fix(evm): pin MLOAD results to prevent stale memory reads in multipass JIT#364
Open
starwarfan wants to merge 2 commits intoDTVMStack:mainfrom
Open
fix(evm): pin MLOAD results to prevent stale memory reads in multipass JIT#364starwarfan wants to merge 2 commits intoDTVMStack:mainfrom
starwarfan wants to merge 2 commits intoDTVMStack:mainfrom
Conversation
…s JIT MLOAD's convertBytes32ToU256Operand generates load instructions from the EVM memory buffer that are represented as SSA values in the MIR. When the MLOAD result stays on the EVM stack across a memory-writing opcode (CODECOPY, MSTORE, EXTCODECOPY, etc.), the backend may schedule these loads after the write, causing the MLOAD to observe the *new* memory contents instead of the value at the time of the MLOAD. Pin the four uint64 components into local variables immediately after the loads, creating a data dependency that prevents reordering past subsequent function calls. Fixes 2 fuzz crashes (crash-0e10c1dd, crash-cdb6d9e4) where MLOAD returned code bytes written by a later CODECOPY, producing non-zero upper bits that triggered a spurious GasLimitExceeded. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Fixes a multipass JIT miscompile where MLOAD results could observe later memory writes due to backend reordering of the underlying memory loads, leading to incorrect values and fuzz crashes.
Changes:
- In
handleMLoad, “pins” the four 64-bit components produced byconvertBytes32ToU256Operandinto temporaries immediately after loading. - Reconstructs the
UINT256operand from the reloaded components to enforce an ordering/data-dependency barrier against later memory-writing opcodes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 20%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 20%)
Summary: 194 benchmarks, 0 regressions |
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.
MLOAD's
convertBytes32ToU256Operandgenerates load instructions from the EVM memory buffer that are represented as SSA values in the MIR. When the MLOAD result stays on the EVM stack across a memory-writing opcode (CODECOPY, MSTORE, EXTCODECOPY, etc.), the backend may schedule these loads after the write, causing the MLOAD to observe the new memory contents instead of the value at the time of the MLOAD.Pin the four uint64 components into local variables immediately after the loads, creating a data dependency that prevents reordering past subsequent function calls.
Fixes 2 fuzz crashes (
crash-0e10c1dd,crash-cdb6d9e4) where MLOAD returned code bytes written by a later CODECOPY, producing non-zero upper bits that triggered a spuriousGasLimitExceeded.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):
src/compiler/evm_frontend/evm_mir_compiler.cpp
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
Root cause: In the multipass JIT compiler,
handleMLoadconverts the 32-byte memory read into a 4-componentuint256Operand viaconvertBytes32ToU256Operand. This function emits fourLoadInstructionMIR nodes that read directly from the EVM memory buffer pointer. These MIR nodes are SSA values—the Operand merely holds pointers to them on the compile-time stack. When LLVM later compiles these nodes to native code, it may schedule them at the point of use rather than the point of definition, because it sees no data dependency forcing early execution.If a memory-writing opcode (CODECOPY, EXTCODECOPY, MSTORE, etc.) occurs between the MLOAD and the eventual use of its result, LLVM can legally move the loads past the write call, causing the MLOAD to read modified memory. In the fuzz crashes, CODECOPY wrote bytecode to memory at offset 0. A previous MLOAD(0) result was still on the stack. When that result was later consumed by
normalizeOperandU64(for a second CODECOPY's size argument), the loads read the new code bytes instead of zeros. The non-zero upper bits triggeredGasLimitExceeded.Fix: After
convertBytes32ToU256Operand, immediately store each component into a local variable viastoreInstructionInTempand reload it. This creates a register-spill/reload sequence that the backend cannot reorder past function calls, ensuring the memory read is pinned to the MLOAD's position in the instruction stream.4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Provide a description of the tests:
Verified both crash files pass with the fix using evmone-fuzzer in multipass mode. Also verified that no other fixed crash files regressed.
Made with Cursor