fix(compiler): harden EVM stack-shape analysis before JIT admission#560
Open
abmcar wants to merge 20 commits into
Open
fix(compiler): harden EVM stack-shape analysis before JIT admission#560abmcar wants to merge 20 commits into
abmcar wants to merge 20 commits into
Conversation
…fix offset A materializing exit of a lifted block spilled its logical stack at byte offset Hidden*32 and set StackSize = Hidden + size. But the lifted logical stack already spans the full absolute entry depth (FullEntryStateDepth = ResolvedEntryStackDepth): the lifter builds entry operands for every absolute slot including the hidden-prefix slots, and handleBeginBlock restores all of them onto the logical stack. Draining that stack at exit therefore yields the full absolute stack, so spilling it at Hidden*32 wrote the stack Hidden slots too high and inflated the recorded depth by Hidden. Top-relative contents stayed consistent, but the inflated StackSize caused spurious stack-overflow traps near the 1024 limit and suppressed underflow traps in later blocks. Under the full-absolute-depth entry-state model the correct spill prefix is 0 (the stack bottom). Fix the lifted branch of finalizeBlockExit to spill at prefix 0 and remove the now-dead CurrentBlockHiddenLiveInPrefixDepth tracking. The non-lifted branch is unchanged, so lift-OFF codegen is byte-identical. Add a differential regression test: a lifted block with Hidden=1 takes a materializing constant JUMP to a non-lifted block that pops two slots while only the prefix slot is live. The interpreter underflows; before the fix the JIT's inflated StackSize let the underflow check pass and the block ran to STOP, diverging in status. Also complete the MockOperand test double with isEmpty() so evmJitFrontendTests builds under ZEN_ENABLE_EVM_STACK_SSA_LIFT. This is Stage 0 of the SSA-lift dynamic-boundary soundness change; the design doc is included. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
…xits of lifted blocks A dynamic dispatch can land on any JUMPDEST via the jump table, so the runtime stack must be valid at every dynamic (non-constant) exit of a lifted block. Previously a dynamic JUMP skipped materialization whenever the compatible-dynamic target set was non-empty, and a dynamic-dest JUMPI decided materialization only from fallthrough liftability, ignoring the taken edge. Both left non-lifted and out-of-model dynamic targets reading a stale runtime stack. Force NeedsRuntimeMaterialization for any dynamic dest at both lifted exit-decision points (handleJumpOpcode, handleJumpIOpcode partial path). SSA entry-state assignment is kept and stays additive: lifted compatible targets retain their zero-reload SSA entry while the materialized memory image backs non-lifted and out-of-model targets. Statically wired lifted->lifted skip sites (constant JUMP/JUMPI, fallthrough, already-begun entry) are unchanged. Lift-OFF codegen is unaffected (changes are inside CurrentBlockLifted branches). This is Stage 1 of the SSA-lift dynamic-boundary soundness change; lifted dynamic targets are closed separately by the Stage 2 liftability rules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
Make the SSA stack-lift dynamic boundary sound so lifted-block codegen matches the interpreter on modules with dynamic jumps or hidden live-in prefixes. - Exclude every jump-table-enterable JUMPDEST from lifting: whenever a module has any dynamic jump the runtime dispatch can land on any JUMPDEST, and lifted SSA entry state does not model that runtime stack. The exclusion equals codegen's indirect switch-emission set. - Assert at the visitor that a non-constant JUMP/JUMPI dest (which drives codegen's indirect dispatch) implies the analyzer marked the block dynamic; a mismatch fails the compile loudly instead of miscompiling. - Do not lift a block whose static predecessor has an unresolved exit depth (its runtime-entry assignment would be silently skipped). - Do not lift a hidden-live-in-prefix block that reconciles its prefix with the runtime stack at a lifted/non-lifted materialization boundary or a dynamic exit; iterate to a fixpoint. Pure-SSA-island hidden blocks keep lifting. - Add debug-only structural invariants: a lifted block is never an indirect-dispatch landing; each static predecessor's exit depth equals the lifted block's full entry-state depth; the logical entry state is fully defined at lifted block begin. Validated interpreter-vs-multipass equality under forced JIT on the Sourcify corpus (56/56), 8 evmone kernels, and EEST fork_Cancun statetests (2723/2723) in both lift-ON and lift-OFF builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
…uards Remove hasUnresolvedCompatibleDynamicReturnTrampoline and hasUnresolvedNonLiftedDeepEntryRisk from the ShouldFallbackToInterp computation, plus the now-unused analyzer helpers they relied on. Add the deep-entry differential reproducer as a unit test. State at this commit: lift-OFF is fully green (2723/2723 EEST fork_Cancun). lift-ON exposes 4 residual wallet miscompiles (logs_hash + state_root divergence): stWalletTest.dayLimitSetDailyLimit, dayLimitSetDailyLimitNoData, multiOwnedChangeOwner, walletChangeOwnerRemovePendingTransaction. These modules previously fell back to the interpreter behind the guards, masking a real lift-ON dynamic-boundary unsoundness that the next commit addresses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
…their pops An internal-function return continuation is reached only by a dynamic return JUMP, so propagateEntryDepths (which flows depths along static edges from the function entry) never assigns its depth; the dynamic-jump region uniform-entry heuristic supplies one that under-counts the caller's frame. The shortfall propagates into static successors, which can then hold ResolvedEntryStackDepth + MinStackHeight < 0 -- popping below their own resolved bottom. Lifting such a block sizes its logical entry state to the too-shallow depth and validateLiftedBlockStackBounds emits a spurious EVMStackUnderflow, reverting the call. This miscompiled four fork_Cancun stWalletTest cases under ZEN_ENABLE_EVM_STACK_SSA_LIFT once the whole-module fallback guards were removed (empty logs_hash, wrong state_root): dayLimitSetDailyLimit, dayLimitSetDailyLimitNoData, multiOwnedChangeOwner, walletChangeOwnerRemovePendingTransaction. Add a finalizeLiftability clause that refuses to lift any block with ResolvedEntryStackDepth + MinStackHeight < 0. The sum is a local proof the resolved depth is under-resolved (or the block genuinely underflows); either way the lifted entry state cannot cover the block's pops. The non-lifted path reads the real runtime stack via its -MinStackHeight check, so unlifting is behaviorally neutral -- a genuine underflow still traps. Update DynamicJumpForcesReachableJumpDestsToFallback: its dynamic-jump source block used CALLDATALOAD at entry depth 0 (a genuine underflow) that the new clause correctly unlifts; give it the CALLDATALOAD offset so the test still exercises reachable-JUMPDEST fallback. Add UnderResolvedEntryDepthBlockIsNot- Lifted guarding the new clause. Gates (lift-ON and lift-OFF): full EEST fork_Cancun 2723/2723, evmDifferentialTests 46/46, evmJitFrontendTests 28/28, evmone-unittests 223/223, evm_asm 200/200; corpus interp-vs-multipass differential (28 contracts x2 calldata incl 0x9649BF5d and 0x6a877e5A, 8 benchmark kernels) zero divergence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
Main-suite geomean -65.1% (lift-OFF, byte-identical flags, 12 reps, median/IQR); 6/7 recovered kernels improve, weierstrudel regresses +19-22% (real, flagged for profiling follow-up); micro suite null; control clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
Close three review findings on the EVM multipass lift-boundary path. 1. Under-resolved-depth revocation had an `== 0` hole. A block whose dynamic-jump region heuristic depth was under-counted such that ResolvedEntryStackDepth + MinStackHeight == 0 still lifted with a too-shallow absolute depth; its materializing exit spill sets StackSize absolutely and truncates the caller's deeper frame slots, diverging from the interpreter. Root fix: taint-track heuristic-derived depths. EntryDepthFromRegionHeuristic is set where the region uniform-entry machinery seeds a depth and inherited along every static propagation edge; finalizeLiftability never lifts a tainted block. Only depths propagated purely from the function entry (seed depth 0) stay trusted. The prior `< 0` rule remains as a subsumed sanity net. 2. assertDynamicJumpConsistency used ZEN_ASSERT (abort() in release), so an analyzer/visitor constant-tracking mismatch would kill the loading process. It now throws a Compilation-phase Error (EVMDynamicJumpConsistencyFailed); the EVM compile path already catches std::exception and degrades to interpreter fallback with a logged error. The check stays active in every build. 3. The hidden-prefix fixpoint keyed its materializing-exit test on HasDynamicJump, missing a JUMPI whose constant destination is not a valid JUMPDEST (HasDynamicJump stays unset, yet codegen forces materialization). The fixpoint now uses blockExitMaterializesRuntimeStack, true for HasDynamicJump || (HasConditionalJump && !HasConstantJump), matching codegen's materialization truth independent of successor liftability. Also simplify the JUMP exit materialization predicate to !HasKnownLiftedSucc. Add an analyzer-level test asserting a region-heuristic-resolved continuation and its static successor are both tainted and unlifted (red-verified: the successor lifts without the taint rule). Update the change-doc Findings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhdGjJ1rKgZXz9sUQJZys
Contributor
There was a problem hiding this comment.
Pull request overview
Removes structural multipass whole-module interpreter fallbacks by making SSA stack-lift “dynamic boundary” handling sound (spill depth correctness, always-materialize rules, and stricter liftability gating), so real-world Solidity contracts route to the multipass JIT instead of the interpreter.
Changes:
- Fix lifted-block materializing-exit spilling to spill from stack bottom (avoids hidden-prefix depth inflation) and always materialize the runtime stack for dynamic jump exits.
- Add liftability safety rules (region-heuristic depth tainting, JUMPDEST/dynamic-target exclusions, unresolved-predecessor depth gating) plus debug invariants to assert analyzer/codegen consistency.
- Remove the two structural
ShouldFallbackToInterpguards so only size/complexity thresholds remain, and add regression tests + change doc.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/evm_jit_frontend_tests.cpp | Adds analyzer unit tests for new liftability rules (under-resolved entry depth, region-heuristic taint propagation). |
| src/tests/evm_differential_tests.cpp | Adds differential regressions for hidden-prefix spill bug and deep-entry “internal-call continuation” JIT/no-fallback behavior. |
| src/runtime/evm_module.cpp | Drops structural whole-module fallback guards; keeps suitability size/complexity threshold only. |
| src/compiler/evm_frontend/evm_analyzer.h | Introduces heuristic-depth taint tracking, new lift-gating rules, hidden-prefix fixpoint boundary logic, and debug structural invariants. |
| src/common/errors.def | Adds a compilation error code for analyzer/codegen dynamic-jump dispatch inconsistency. |
| src/action/evm_bytecode_visitor.h | Fixes spill prefix handling, forces runtime materialization at dynamic exits, and adds a hard consistency guard that throws into existing compile-failure fallback. |
| docs/changes/2026-07-02-evm-lift-dynamic-boundary-soundness/README.md | Full design/evidence/change proposal documenting the hazard and staged fixes. |
💡 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 |
Add a static, theme-aware SVG (per-kernel steady-state change; recovered kernels and the weierstrudel regression shown) embedded by relative path with a caption. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QrsLkrG8HezyC5bxmjMUeA
GitHub PR descriptions do not reliably render committed SVG via camo; add a PNG raster alongside the theme-aware SVG so the figure shows in the PR body. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QrsLkrG8HezyC5bxmjMUeA
Invalidate stack depths that cannot satisfy a block's own pops before lift and range analysis, and conservatively disable range-derived transforms if shape propagation still disagrees. Let frontend consistency errors reach the eager compiler fallback instead of aborting in Release builds.
Restore the analyzer predicate and module routing guard for unresolved non-lifted blocks that consume deeper stack entries. Lifted boundary fixes do not cover this path when stack SSA lifting is disabled. Update the nested internal-call differential regression to assert both the predicate and interpreter fallback.
abmcar
changed the title
added 4 commits
July 16, 2026 15:54
Pass fallthrough liveness explicitly into MIR JUMPDEST construction so dead post-RETURN and post-REVERT continuation blocks cannot become fake phi predecessors. Harden lifted entry-state identity and dynamic-depth handling, add gas-aware differential regressions, and record the final lift-OFF/lift-ON mainnet correctness gate.
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.
Summary
JUMPDESTpredecessors aligned with analyzer metadata by excluding dead post-RETURNand post-REVERTfallthroughs;Root cause and behavior
A block reached through runtime jump dispatch may carry caller-frame slots that are absent from its statically inferred entry depth. Lifting that block can materialize a stack that is too shallow and diverge from interpreter execution. This change prevents runtime-dispatch targets and their affected static closure from using an unproven lifted entry state. It also retains interpreter admission for reachable non-lifted blocks that consume unresolved caller-frame slots.
The MIR builder previously inferred linear fallthrough into
JUMPDESTfrom the current insertion block. A terminating opcode can leave an empty post-termination block with no live predecessor, but the builder still connected it to the next destination. MIR then had one more predecessor than the analyzer's phi model. The visitor now passes fallthrough liveness explicitly, and the builder adds the edge only when it is live.Frontend consistency errors already reach the shared EVM compile boundary. If compilation fails, the module now records an atomic interpreter fallback so eager and profile-guided background paths do not retry the same failed compilation.
Validation
Current post-review source (
629bb22)evmJitFrontendTests, Lift OFFevmJitFrontendTests, Lift ONevmDifferentialTests, Lift OFFevmDifferentialTests, Lift ONA 252-bytecode mainnet-v2 analyzer scan matched the pre-hardening PR head on every non-timing field, including block count, liftability, dynamic-entry taint, and interpreter admission.
Frozen replay evidence
The earlier frozen Lift OFF and Lift ON source snapshots passed 300/300 mainnet-replay-v2 Cancun fixtures and the continuous replay of blocks
21800000..21800099: 100 blocks, 14,150 transactions, 1,825,295,016 gas, and matching final block hash0xaa5e1274f0125381d5a07cd4521b71dd808dd581f46587f8eb82083b1052a812.Those replay binaries predate the post-review work-queue and sticky-fallback hardening. Their source hashes and evidence boundaries are recorded in
docs/changes/2026-07-02-evm-lift-dynamic-boundary-soundness/README.md.Performance and conservative scope
On the same synthetic 20,003-byte analyzer probe with 4,000 chained hidden-prefix blocks, the work-queue implementation reduced a representative Release run from 462,992 us to 3,908 us while preserving the analyzer result. This isolates the repeated-rescan mechanism and is not an end-to-end execution benchmark.
Stack SSA lifting remains disabled by default. In the frozen 300-fixture load trace, the PR introduced no JIT admission loss relative to the exact-base predicate. Among the seven modules admitted by both versions, load-weighted lifted-block coverage changed from 72 to 63, a 12.5% reduction in optimization coverage. This is not a measured 12.5% execution slowdown.
The full correctness evidence, limitations, and follow-up options are in the committed change document.