-
Notifications
You must be signed in to change notification settings - Fork 285
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
baseline: Improve code analysis API #941
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #941 +/- ##
=======================================
Coverage 93.89% 93.89%
=======================================
Files 146 147 +1
Lines 15487 15507 +20
=======================================
+ Hits 14541 14561 +20
Misses 946 946
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Converted to draft, because I have better idea... |
cb8bbd8
to
e380a81
Compare
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, only minor nits
e4adf18
to
3016caf
Compare
I changed the name for the "full unmodified code" to
|
|
lib/evmone/baseline_execution.cpp
Outdated
@@ -335,19 +335,21 @@ evmc_result execute(evmc_vm* c_vm, const evmc_host_interface* host, evmc_host_co | |||
{ | |||
auto vm = static_cast<VM*>(c_vm); | |||
const bytes_view container{code, code_size}; | |||
const auto eof_enabled = rev >= EVMC_PRAGUE; |
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.
Can you use REV_EOF1
here?
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.
Added as instr::REV_EOF1
but revision handling can be improved further. I wonder if we can make the revision opaque for EVMC.
For code analysis context only pass a `bool` value if EOF is enabled. This was the only information the analysis extracted from the previously used full EVM revision.
Expose more information from the `CodeAnalysis` class via new methods. Make fields private and hide some implementation details.
Take the EOF data section contents from the `CodeAnalysis` object instead of complicating the `ExecutionState`.
3016caf
to
6e2f5d9
Compare
Passing the
evmc_revision
to the code analysis is too fine-grained for this public API. On Ethereum Mainnet EOF can be always enabled, but the options is left for other chain configurations.