fix(compiler): sync non-lifted EVM stack state across block boundaries#507
Open
ZR74 wants to merge 18 commits into
Open
fix(compiler): sync non-lifted EVM stack state across block boundaries#507ZR74 wants to merge 18 commits into
ZR74 wants to merge 18 commits into
Conversation
Add a standalone DTVM-only test target plus extracted fixtures for the historical legacy CALL regression so it can be replayed without external chain snapshots.
Run the standalone legacy CALL regression through the dtvmapi execution path and expand the fixture payloads needed for DTVM-only coverage.
Spill the full tracked stack back to EVMInstance on non-lifted exits, reload stack metadata on non-lifted entries, and avoid re-finalizing dead blocks after a terminator so logical stack provenance stays aligned across block boundaries.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an EVM compiler/runtime state mismatch by ensuring non-lifted EVM stack metadata is synchronized across basic-block boundaries, and adds regression coverage (unit tests + historical mainnet fixtures) to prevent gas/behavior divergence around legacy CALL execution paths.
Changes:
- Update the EVM bytecode visitor to reload stack metadata at non-lifted block entry and to sync stack size back to the instance when materializing the stack at block exit.
- Extend
EVMMirBuilderwith helpers to reload/sync tracked stack metadata from/to theEVMInstance. - Add legacy mainnet repro fixtures and new/expanded regression tests (including a dedicated
evmLegacyCallReproTeststarget).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/evm/fixtures/legacy_call_repro/schema.json | JSON schema for the standalone legacy CALL repro fixture format. |
| tests/evm/fixtures/legacy_call_repro/README.md | Documentation for regenerating and interpreting the legacy CALL fixtures. |
| tests/evm/fixtures/legacy_call_repro/cases.json | Lists the included legacy CALL fixture cases. |
| tests/evm/fixtures/legacy_call_repro/block_254277_tx_0.json | Mainnet fixture capturing a legacy CALL “creation cost” case. |
| tests/evm/fixtures/legacy_call_repro/block_254297_tx_0.json | Mainnet fixture capturing a legacy CALL “guard path” case. |
| src/tests/evm_legacy_call_repro_tests.cpp | New test runner that loads fixtures and executes them in interpreter/multipass (and optionally via dtvmapi). |
| src/tests/evm_jit_frontend_tests.cpp | Enhances the mock builder’s tracked-stack model and adds JIT frontend regression tests around hidden prefixes and non-lifted targets. |
| src/tests/evm_interp_tests.cpp | Adds a regression test that captures the recipient address of an internal CALL and compares interpreter vs multipass. |
| src/tests/CMakeLists.txt | Wires up evmLegacyCallReproTests build + ctest registration (and dtvmapi linkage when available). |
| src/compiler/evm_frontend/evm_mir_compiler.h | Declares new stack metadata reload/sync helpers on EVMMirBuilder. |
| src/compiler/evm_frontend/evm_mir_compiler.cpp | Implements stack metadata reload/sync helpers. |
| src/action/evm_bytecode_visitor.h | Core logic change: reload stack metadata for non-lifted blocks; sync stack size on materialized exits; avoid double-finalization. |
Comments suppressed due to low confidence (1)
src/tests/evm_legacy_call_repro_tests.cpp:226
runFixture()re-parses the fixture JSON to readenv.block_hashbut doesn’t checkstd::ifstreamopen success or RapidJSON parse errors before accessing members. A missing/corrupt fixture would currently lead to undefined behavior. Consider reusing the already-parsed document inloadFixture()(storeblock_hashthere) and/or addASSERT_TRUE(F.is_open())+ parse/type checks before accessingD["env"]["block_hash"].
// one block_hash value for all get_block_hash() queries.
const auto DocPath = std::filesystem::path(Fixture.FixturePath);
std::ifstream F(DocPath);
rapidjson::IStreamWrapper ISW(F);
rapidjson::Document D;
D.ParseStream(ISW);
if (D.IsObject() && D.HasMember("env") && D["env"].IsObject() &&
D["env"].HasMember("block_hash") && D["env"]["block_hash"].IsString()) {
Host->block_hash =
zen::utils::parseBytes32(D["env"]["block_hash"].GetString());
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 1 regressions |
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> (cherry picked from commit 8e57176)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 7a4d2a5)
Keep hidden live-in prefixes from being rematerialized twice, tighten dynamic jump predecessor mapping, and anchor non-lifted entry values so deep stack slots stay aligned across loop iterations. This fixes the legacy CALL repro path that was drifting jump targets and surfacing bad-jump or gas mismatches in multipass JIT.
Run analyzer-based fallback checks for EVM modules in every multipass load path and retry compilation failures in interpreter mode for dtvmapi and recursive test-host submodules. This makes the legacy CALL repro fixtures pass consistently even when a nested bytecode fragment still trips MIR verification during JIT compilation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note