-
-
Notifications
You must be signed in to change notification settings - Fork 863
feat[tool]: add extra debugging output #4718
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
base: master
Are you sure you want to change the base?
Conversation
This commit contains two main changes. The first are two new output modes: the `symbol_map` and `symbol_map_runtime`. These contain both contain the raw "symbol map" produced during the respective assembly of the deployment and runtime code into bytecode. This information is crucial for determining the exact starting position of internal functions in the assembled bytecode. The second component is a change to the existing metadata output. When the user selects the experimental codegen, the metadata for internal functions will contain a list of the parameter names which are passed via the stack in `venom_via_stack` key. Similarly, if the function returns its value via the stack, the "venom_return_via_stack" key records this as a boolean. These keys are omitted if experimental codegen is not selected. (Credit where credit is due: this feature was primarily developed by Charles Coop). Co-Authored-By: Charles Cooper <[email protected]>
i think this looks good. could we add a test for this to protect against regressions? the easiest would be to just copy/paste the symbol map output and check that the literal contents are produced in the test. |
Won't this be extremely sensitive w.r.t. codegen changes? If there is any perturbation in how something is compiled in it will throw off the symbol locations. Perhaps regenning the expected constants isn't so bad? |
@charles-cooper I added a regression test, but didn't have the courage to test for an exact constant. It's easy enough to add in to the sketched test here. Let me know if you want me to push further on testing different output scenarios. |
Uses new way to compute symbol map, adjust to existence of Label type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. thanks! @cyberthirst to take a look
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4718 +/- ##
=======================================
Coverage 93.22% 93.22%
=======================================
Files 136 136
Lines 19190 19207 +17
Branches 3293 3296 +3
=======================================
+ Hits 17889 17905 +16
Misses 884 884
- Partials 417 418 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vyper/compiler/output.py
Outdated
return {k.label: v for (k, v) in sym.items()} | ||
|
||
|
||
def buld_symbol_map_runtime(compiler_data: CompilerData) -> dict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
assume this PR broke after merging #4744 |
…nto jtoman/debug-info-ext
Point resolve_symbols to its new home
Yep, pointed resolve_symbols to its new home. |
just a lint remaining. @cyberthirst does it look good for you? |
What I did
This commit contains two main changes.
The first are two new output modes: the
symbol_map
andsymbol_map_runtime
. These contain both contain the raw "symbol map" produced during the respective assembly of the deployment and runtime code into bytecode. This information is crucial for determining the exact starting position of internal functions in the assembled bytecode.The second component is a change to the existing metadata output. When the user selects the experimental codegen, the metadata for internal functions will contain a list of the parameter names which are passed via the stack in
venom_via_stack
key. Similarly, if the function returns its value via the stack, the "venom_return_via_stack" key records this as a boolean. These keys are omitted if experimental codegen is not selected. (Credit where credit is due: this feature was primarily developed by Charles Coop).How I did it
The
symbol_map
outputs are generated by calling theassembly_to_evm_with_symbol_map
with the correct assembly and then throwing away all of the output except for the symbol map. I confess I cargo-culted the other arguments (pc_ofst
andcompiler_metadata
) with their declared defaults; if these are indeed the correct values to use and they should always just be whatever the defaults are, we can remove their explicit inclusion.The "calling convention" changes simply queries the same functions which determines the calling convention used during codegen and records that during metadata generation.
How to verify it
The
test_output_json
test ensures that the output machinery works as intended.To verify the metadata and symbol map generation works correctly, we can look at the following simple contract:
When using
-f symbol_map_runtime,metadata,opcodes_runtime --experimental-codegen
we get the following output for thesymbol_map
This tells us that
int
starts at PC 119. Looking at the opcodes:We can see that 0x0077 (aka 119) is the PC of the start of int.
Further, we see in the metadata output:
which reflects that
a
is passed via the stack and the return value is on the stack, as we expect.Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Adds additional debugging output for
metadata
and accessing internal mappings generated by the compiler.Cute Animal Picture