Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiler: improve debugging experience #2434

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Apr 14, 2022

Currently one instruction can correspond to multiple sequence points
because of inlining. This leads to a bad user experience as only
the last one is used. In this commit we create a sequence point for each
inlined call and also make sure that each time a new sequence point is
created the corresponding opcode can easily be seen in code.

The NOPs increase contract size, but not to a large degree. Other
solutions considered:

  1. Discard NOPs if a special flag is provided. Still leads to bad
    debugging experience if deployed contract differs from the debugged
    one.
  2. Create an issue for a debugger. When multiple sequence points are
    provided for a single instruction they can be used to emulate
    non-inline behaviour with pseudo-NOPs. I believe this is what windows
    debugger does (the last paragraph https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugging-optimized-code-and-inline-functions-external )
  3. Emit debug info for inlined functions even without creating a
    function in the NEF itself. This should be done for each called
    instance and would also create overlapping opcode ranges for
    the enclosing function. However this approach can also ensure
    consistent values view for inlined function parameters.

Signed-off-by: Evgeniy Stratonikov evgeniy@nspcc.ru

Currently one instruction can correspond to multiple sequence points
because of inlining. This leads to a bad user experience as only
the last one is used. In this commit we create a sequence point for each
inlined call and also make sure that each time a new sequence point is
created the corresponding opcode can easily be seen in code.

The NOPs increase contract size, but not to a large degree. Other
solutions considered:
1. Discard NOPs if a special flag is provided. Still leads to bad
   debugging experience if deployed contract differs from the debugged
   one.
2. Create an issue for a debugger. When multiple sequence points are
   provided for a single instruction they can be used to emulate
   non-inline behaviour with pseudo-NOPs. I believe this is what windows
   debugger does (the last paragraph https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugging-optimized-code-and-inline-functions-external )
3. Emit debug info for inlined functions even without creating a
   function in the NEF itself. This should be done for each called
   instance and would also create overlapping opcode ranges for
   the enclosing function. However this approach can also ensure
   consistent values view for inlined function parameters.

Signed-off-by: Evgeniy Stratonikov <evgeniy@nspcc.ru>
@roman-khimov
Copy link
Member

Currently one instruction can correspond to multiple sequence points because of inlining.

Maybe we can improve handling this case in the debugger itself?

@fyrchik
Copy link
Contributor Author

fyrchik commented May 16, 2022

Created an issue in the neo-debugger neo-project/neo-debugger#170

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Go smart contract compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants