fix(evm): reload the cached memory base whenever the cached size is reloaded - #607
Open
abmcar wants to merge 1 commit into
Open
fix(evm): reload the cached memory base whenever the cached size is reloaded#607abmcar wants to merge 1 commit into
abmcar wants to merge 1 commit into
Conversation
…eloaded The multipass JIT caches EVM memory base and size in function-entry locals and refreshes them on the memory-expansion branch. It also refreshes the size alone after every runtime helper that can grow memory - the CALL family, CREATE, CREATE2, LOG, KECCAK256, CALLDATACOPY, CODECOPY, EXTCODECOPY, RETURNDATACOPY. The base was never refreshed there, and that is unsound, because those helpers can move the base. EVM memory is allocated lazily: a frame starts with MemoryBase == nullptr and the 16 MB buffer is created by ensureMemoryBuffer() on the frame's first real growth. When that first growth happens inside a runtime helper rather than through expandMemoryIR, the helper allocates the buffer and sets MemoryBase, the JIT reloads the size, and the cached base keeps the entry-time null. A later memory access then combines the two: the reloaded size is large enough that the expansion branch - the only other place the base is refreshed - is not taken, and a precheck-covered access reads the cached base and stores through null. The write lands on page zero, which the trap handler reports as an internal error and which, before it was reclassified, was laundered into an EVMC_INVALID_MEMORY_ACCESS halt and a wrong gas number. Make the pairing structural instead of incidental: reloadMemoryBaseFromInstance() is now one helper used by both the expansion branch and the post-helper reload, and reloadMemorySizeFromInstance() becomes reloadMemoryCachesFromInstance(), which refreshes both halves of the snapshot. The cost is one 8-byte load after a call that has already gone out to a runtime helper. The added differential test is a 22-byte contract with no nested call: a CALLDATACOPY with a dynamic length grows memory inside the generic helper, and two constant-offset stores in the next block share a block precheck whose expansion is already satisfied, so they address memory through the cached base. It segfaults under multipass without this change and matches the interpreter with it. Verified on mainnet blocks 25818502 and 25818530, the two blocks out of a 1000-block window that failed under DTVM only in JIT mode. Both now complete with post-state-root verification. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hg7u14XK36yqjEidE3o2rY
16 tasks
Contributor
Author
|
For anyone reviewing this alongside the other two open EVM PRs — they are separable, which is why they are three, but a reviewer seeing only this one may reasonably ask why a null store through a stale base was ever silent. That is the missing half:
Merging this one alone is sufficient to fix the defect. Merging #604 alone is sufficient to stop the class from being silent. Neither depends on the other. |
16 tasks
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is well-scoped, consistently applied across affected call sites, and includes a targeted regression test that exercises the previously failing helper-growth scenario.
Pull request overview
This PR fixes a multipass JIT correctness bug where the EVM memory base pointer cache could remain stale (including null) after runtime helpers that grow/allocate memory, by ensuring the cached memory base is reloaded whenever the cached memory size is reloaded. This aligns multipass JIT behavior with the interpreter and prevents null dereferences / incorrect memory addressing after helper-driven lazy memory allocation.
Changes:
- Introduce
reloadMemoryBaseFromInstance()and replace size-only refreshes withreloadMemoryCachesFromInstance()to reload both base + size as a single snapshot. - Update all post-runtime-helper “memory cache reload” sites to use the new paired reload helper.
- Add a differential regression test that reproduces helper-driven first memory growth followed by later-block memory stores.
File summaries
| File | Description |
|---|---|
src/compiler/evm_frontend/evm_mir_compiler.h |
Declares new paired memory-cache reload helpers and documents the base+size snapshot invariant. |
src/compiler/evm_frontend/evm_mir_compiler.cpp |
Implements base reload and updates all relevant helper-return paths to reload both base and size; reuses the base reload in expandMemoryIR. |
src/tests/evm_differential_tests.cpp |
Adds a differential regression test that fails on unfixed multipass JIT but matches the interpreter after the fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
⚡ 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, 0 regressions |
abmcar
pushed a commit
to abmcar/reth
that referenced
this pull request
Aug 29, 2026
Two corrections to §10 now that the campaign has run. The defect table listed the JIT null dereference as open and told the reader to run on a 998-block subset. It is fixed upstream (DTVMStack/DTVM#607) and a full rescan under the fixed libraries returns 1000 OK, 0 failures. All 46 originally failing blocks are closed and the four-engine intersection is the whole corpus. The table's description of that defect was also wrong in a way worth fixing: it is not about nested calls. EVM memory is allocated lazily, and the failure needs a frame's first growth to happen inside a runtime helper rather than through expandMemoryIR; depth 0 on a fresh instance qualifies. New §10.5 says what the measured number actually contains, because the obvious reading of it is wrong in two ways. First, the tight boundary is available after all — replay-batch fills rethSubjectRunExecLoop, replay-block does not, which is why it reads as null if you check the wrong tool — and moving to it makes DTVM look worse rather than better, since the fixed overhead is a larger fraction of REVM's smaller figure. Second, neither boundary isolates code generation: a metrics build counts 2,248 module-cache lookups per block against 186 top-level executions, each running a full bytecode memcmp under the mandatory strict validation, and nested frames never reopen the timing window. The figure is DTVM-through-the-bridge against native REVM, and the document now says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018osheVEXr1SywFtbYFjHAc
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".):
Related, but independent and separately mergeable: #604 reclassifies the fault
this defect produces, so that a null dereference in JIT'd code is no longer
reported as a consensus
EVMC_INVALID_MEMORY_ACCESShalt. This PR removes thedereference itself. Neither depends on the other.
2. What is the scope of this PR (e.g. component or file name):
Multipass JIT memory-cache invalidation in
src/compiler/evm_frontend/evm_mir_compiler.{cpp,h}, plus a differentialregression test in
src/tests/evm_differential_tests.cpp.3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
The multipass JIT caches the EVM memory base and size in function-entry locals.
It refreshes both on the memory-expansion branch, but after a runtime helper
that can grow memory it refreshed only the size — the CALL family, CREATE,
CREATE2, LOG, KECCAK256, CALLDATACOPY, CODECOPY, EXTCODECOPY, RETURNDATACOPY.
The base was never refreshed there, and those helpers can move it.
The reason that is not merely theoretical is lazy allocation. A frame starts
with
MemoryBase == nullptr; the buffer is created byensureMemoryBuffer()onthat frame's first real growth. When the first growth happens inside a runtime
helper rather than through
expandMemoryIR, the helper allocates the buffer andsets the instance's base, the JIT reloads only the size, and the cached base
keeps the entry-time null.
A later access then combines the two halves: the reloaded size is large enough
that the expansion branch — the only other place the base is refreshed — is not
taken, so a precheck-covered access reads the cached base and stores through
null.
Two consequences worth stating plainly:
trigger is "first growth happened in a helper", not call depth.
EVMC_INVALID_MEMORY_ACCESShalt — a consensus status — and executioncontinued to a wrong gas number rather than failing.
The fix makes the pairing structural rather than incidental:
reloadMemoryBaseFromInstance()becomes one helper used by both the expansionbranch and the post-helper reload, and
reloadMemorySizeFromInstance()becomesreloadMemoryCachesFromInstance(), refreshing both halves of the snapshot. All15 post-helper reload sites go through it. The cost is one 8-byte load after a
call that has already gone out to a runtime helper.
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
No API, ABI or interface change. Code that previously faulted on a null store —
surfacing as an
invalid memory accessstatus, and before #604 as a consensushalt with a wrong gas number — now executes correctly.
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
Regression test —
EVMMemoryBaseCacheDifferential.HelperGrownMemoryIsAddressableFromALaterBlock,a 22-byte contract with no nested call. A
CALLDATACOPYwith a dynamic copylength stays on the generic memory-growing helper and performs the lazy
allocation; two constant-offset
MSTOREs in the next block share one blockprecheck whose expansion is already satisfied by the reloaded size, so they
address memory through the cached base without taking the expansion branch.
The two stores are load-bearing — please keep them. A single-store version
passes even against unfixed code. The block precheck that makes the stores skip
the expansion branch is only formed when it covers at least two ops
(
evm_memory_grouping.h,if (CoveredOps < 2) { RejectReason = TooFewOps; });with one store there is no shared precheck, the expansion branch is taken, and
the base gets refreshed by the very path the defect avoids. Anyone simplifying
this test will get a false green.
The test is a real negative control — verified, not assumed. I built a third
tree carrying only the test file from this commit on top of pristine
338d123,with the compiler left unfixed, and ran it:
So against unfixed code the multipass JIT reports
invalid memory accesswherethe interpreter succeeds — which is exactly the laundering #604 addresses,
reproduced in a unit test. With this commit applied the same test passes.
Full suite, GCC 12 / LLVM 15 Release multipass build (
ZEN_ENABLE_EVM,ZEN_ENABLE_MULTIPASS_JIT,ZEN_ENABLE_VIRTUAL_STACK,ZEN_ENABLE_CPU_EXCEPTIONON):clang-formatcheck oversrc/: clean338d123: passctest: 11 of 12 binaries passThe one failure,
solidityContractTests, issolc not foundin thisenvironment. It fails identically on a pristine
338d123build that I built andran side by side for exactly this comparison, so it is environmental rather than
a regression.
Mainnet blocks. The two blocks in a 1000-block window (25817835–25818834)
that failed under DTVM in JIT mode only. Both now verify pre- and post-state
roots, on an independent re-run:
27644811is the figure the crash previously corrupted to27730760, so the gascommitment is what most directly closes the defect. The subject's database access
count matches the reference execution exactly on both blocks. (Access counts are
our replay harness's internal gate, offered as corroboration rather than as
upstream-relevant evidence.)
6. Release note