fix(compiler): materialize ADDMOD operand limbs before fast/slow branch#544
Merged
zoowii merged 1 commit intoJun 26, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a multipass miscompile in the EVM MIR compiler where ADDMOD operands could remain as raw tree IR across a fast/slow branch, leading to undefined virtual-register uses on one successor path.
Changes:
- Materialize (
protectUnsafeValue) allADDMODoperand limbs before branching inhandleAddMod. - Add regression tests that compare interpreter vs multipass output for
ADDMODfed by prior inline fast/slow ops (e.g.,MOD,DIV).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/compiler/evm_frontend/evm_mir_compiler.cpp | Forces limb materialization before the fast/slow BrIf in handleAddMod to prevent cross-block hazards. |
| src/tests/evm_interp_tests.cpp | Adds regression tests ensuring multipass output matches interpreter for crafted ADDMOD sequences. |
💡 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, 0 regressions |
handleAddMod extracts the raw tree IR of its Augend/Addend/Modulus limbs and then branches into FastBB/SlowBB. The low limbs are first referenced inside those two non-dominating successor blocks, so the CgIR lowering can place a shared computation into only one branch, leaving the sibling path with an undefined virtual register -- a silent multipass miscompile. This is the same cross-block hazard fixed for handleDivModGeneral in DTVMStack#540; handleAddMod is its untwinned sibling. It surfaces when an ADDMOD operand is the raw tree of a preceding inline fast/slow op (e.g. a MOD result): ORIGIN DUP1 DUP1 PUSH0 MOD ADDMOD returns 0 in the interpreter but garbage in multipass before this fix. Fix: protectUnsafeValue all operand limbs before the branch, matching the DTVMStack#540 pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
abmcar
force-pushed
the
fix/evm-addmod-cross-block-materialization
branch
from
9eb47d6 to
0be4d8d
Compare
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.
ADDMOD miscompiles in multipass when an operand is a raw fast/slow tree; this materializes the operand limbs before the branch so each limb is defined in the dominating block.
Problem
handleAddModextracts the raw tree IR of its Augend/Addend/Modulus limbs and then branches intoFastBB/SlowBB. The low limbs are first referenced inside those two non-dominating successor blocks. The CgIR lowering memoizes each tree node's virtual register in a function-global map that is never cleared between blocks, so a shared limb lowered in the first-walked branch leaves the sibling path reading a virtual register defined in a block that does not dominate it — undefined on that execution path. The result is a silent multipass miscompile (wrong ADDMOD result, occasionally a crash).This is the same cross-block materialization hazard fixed for
handleDivModGeneralin #540.handleAddModhas the same structure but never received the #540 fix on the ADDMOD path.Trigger
The hazard surfaces when an ADDMOD operand is the raw tree of a preceding inline fast/slow op (e.g. a MOD result). Minimal reproducer:
The interpreter returns 0; multipass returns an incorrect result before this fix.
Fix
Materialize all operand limbs with
protectUnsafeValuebefore the branch, matching the #540 pattern. This forces immediate evaluation so each limb's virtual register is defined in the dominating block.Tests
Adds
EVMRegressionTest.AddModCrossBlockMaterialization, a differential interpreter-vs-multipass regression test over MOD-fed and DIV-fed ADDMOD inputs.Validated locally: the new regression test passes; multipass
evmone-unittests(223) andevmone-statetest -k fork_Cancun(2723) pass with the fix;tools/format.sh checkclean.