fix(compiler): prevent phi crashes in EVM SSA stack-lift path

docs/changes/2026-06-09-evm-ssa-lift-phi-crashes/README.md

@@ -0,0 +1,115 @@
+# Change: Fix phi-materialization crashes in the EVM SSA stack-lift path
+
+- **Status**: Implemented
+- **Date**: 2026-06-09
+- **Tier**: Light
+
+## Overview
+
+The EVM multipass JIT has an optional SSA stack-lifting path, gated by the
+build flag `ZEN_ENABLE_EVM_STACK_SSA_LIFT` (default **OFF**, introduced in
+#395, fed by the range analyzer from #493). When that flag is enabled the
+JIT emitted invalid MIR that the verifier rejects and that aborts the
+compiler (SIGABRT) on common real-world control flow. This change fixes
+three independent defects in that path. All edits are reachable only when a
+block is lifted, so the default (flag-off) build is unaffected.
+
+## Motivation
+
+With `ZEN_ENABLE_EVM_STACK_SSA_LIFT=ON`, compiling ordinary contracts trips
+the MIR verifier's phi invariants and then aborts:
+
+1. **Phi contiguity.** `createPendingPhi` appended each phi at the block end.
+   A stack merge with two or more slots emits, per slot, four phi components
+   followed by four temp-store `dassign`s; a later slot's phis therefore
+   landed *after* an earlier slot's `dassign`s, violating the
+   "phis contiguous at block start" invariant
+   (`mir/pass/verifier.h`). Multi-slot merge blocks — e.g. any loop header
+   carrying more than one stack value — aborted.
+
+2. **Dynamic-jump-target phi undersize.** Codegen's indirect-jump dispatch
+   wires an edge from every block that performs a computed `JUMP`/`JUMPI` to
+   every `JUMPDEST`. The analyzer's shape-class predecessor enumeration
+   (`getPotentialEntryPredecessorsForBlock`), which sizes a merge block's
+   phis, is a strict subset of that set. So a dynamic-jump-target merge block
+   received phis with fewer incoming slots than its actual MIR predecessor
+   count; a later pass indexing phi incomings by predecessor then ran out of
+   bounds (silent SIGABRT in a release build).
+
+3. **Loop back-edge phi incoming block.** A loop header's merge phi froze its
+   back-edge incoming *block* before the `JUMPI` terminator wired the real CFG
+   edge into the header. The forward walk that resolves the incoming block
+   could not yet reach the true predecessor and recorded the source EVM
+   block's *entry* MIR basic block instead of the basic block that actually
+   branches back. The verifier rejected the phi
+   ("phi incoming block must be a predecessor of the current block").
+
+These are latent correctness/robustness bugs in merged code. Fixing them is
+worthwhile independent of whether the feature is enabled.
+
+## Approach
+
+1. **Phi contiguity** — `createPendingPhi` (evm_mir_compiler.cpp): create the
+   phi without appending, scan the block's leading run of phis, and insert the
+   new phi immediately after it. All phis stay grouped at the block start
+   regardless of how many slots or interleaved `dassign`s precede.
+
+2. **Dynamic-jump-target exclusion** — `finalizeLiftability` (evm_analyzer.h):
+   a new coverage check
+   (`dynamicJumpTargetPredecessorsCoverCodegenEdges`) sets
+   `CanLiftStack = false` for any dynamic-jump-target candidate whose static
+   predecessor enumeration does not account for every dispatch source codegen
+   will wire. Such blocks fall back to the existing memory-backed stack path —
+   the correct conservative behavior: never lift a block whose phi predecessor
+   set cannot be modeled exactly.
+
+3. **Deferred incoming-block re-resolution** —
+   `finalizeStackMergePhiIncomingBlocks` (evm_mir_compiler.cpp), called at the
+   end of `finalizeEVMBase` once the whole CFG exists: for each stack-merge phi,
+   any incoming block that is not yet a real predecessor of the owning block is
+   re-resolved by a forward walk to the actual predecessor. Only the block
+   pointer is corrected; the incoming **value** is preserved.
+
+The fixes touch only phi *placement* and *predecessor bookkeeping*; no
+arithmetic lowering or value semantics change.
+
+## Impact
+
+- **Modules**: `src/compiler/evm_frontend/` only (`evm_mir_compiler.{h,cpp}`,
+  `evm_analyzer.h`).
+- **Default build (flag OFF)**: unaffected. `createPendingPhi`,
+  `finalizeStackMergePhiIncomingBlocks`, and the new analyzer code are
+  reachable only on the lift path; with the flag off the bookkeeping vector
+  stays empty and the lift-only paths are never entered.
+- **Lift build (flag ON)**: the three abort/verifier failures above no longer
+  occur on the validated workloads.
+
+## Validation
+
+- **Default-path neutrality** (build rebuilt from this change, flag OFF):
+  - multipass `evmone-unittests`: **223/223**
+  - multipass `evmone-statetest -k fork_Cancun` (EEST `state_tests`):
+    **2723/2723**
+- **Lift path** (flag ON):
+  - multipass `evmone-unittests`: **223/223** (was 222/223 before fix 3;
+    `memory_grow_mstore8` now passes)
+  - 228-fixture mainnet-replay corpus: JIT-compiles crash-free, verifier-clean
+    (was SIGABRT before fixes 1–2)
+- `tools/format.sh check`: passes.
+
+## Known limitation (disclosed)
+
+This change does **not** make SSA stack-lifting production-ready, and the flag
+stays **OFF**. With the flag ON, at least one further crash remains: the EEST
+`stEIP1153_transientStorage/transStorageOK` state test still aborts under
+SSA-lift (confirmed in isolation; passes with the flag off). That defect is
+independent of the three fixed here and will be addressed separately. CI does
+not build with `ZEN_ENABLE_EVM_STACK_SSA_LIFT=ON`, so it does not exercise this
+path; the lift-path results above were produced manually.
+
+## Checklist
+
+- [x] Implementation complete
+- [x] Default-path tests pass (unittests 223/223, statetest 2723/2723)
+- [x] Build and format checks pass
+- [ ] SSA-lift made fully correct (tracked separately — see Known limitation)

src/compiler/evm_frontend/evm_analyzer.h

@@ -15,6 +15,7 @@
 #include <map>
 #include <queue>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -1090,6 +1091,56 @@ class EVMAnalyzer {
     return SourceBlockPCs;
   }
 
+  // Codegen's indirect-jump dispatch wires an edge from every block that emits
+  // a dynamic (computed) JUMP/JUMPI to every JUMPDEST in the global jump-dest
+  // table (see implementIndirectJump / getOrCreateIndirectJumpBB). The set of
+  // such dispatch source blocks is therefore independent of the analyzer's
+  // shape-class partitioning. Enumerate it so the lifter can size phis to match
+  // the edges codegen actually wires.
+  std::vector<uint64_t> collectAllDynamicJumpDispatchSourceBlocks() const {
+    std::vector<uint64_t> SourceBlockPCs;
+    for (const auto &[EntryPC, Info] : BlockInfos) {
+      // Codegen emits the indirect-jump dispatch for every block whose JUMP /
+      // JUMPI target is non-constant (Info.HasDynamicJump), independent of
+      // whether the analyzer could resolve that block's entry stack depth. The
+      // depth filter used elsewhere would drop sources codegen still wires, so
+      // it must not be applied here.
+      if (!Info.HasDynamicJump) {
+        continue;
+      }
+      appendUniqueBlockPC(SourceBlockPCs, EntryPC);
+    }
+    return SourceBlockPCs;
+  }
+
+  // A dynamic-jump-target block is only safe to lift when the static
+  // predecessor enumeration that sizes its merge phis
+  // (getPotentialEntryPredecessorsForBlock) already accounts for every
+  // dynamic-jump dispatch source codegen will wire. Otherwise the phi is
+  // undersized relative to the block's actual MIR predecessor count and the
+  // verifier/regalloc path breaks.
+  // DispatchSources is the codegen dispatch-source set, computed once by the
+  // caller (collectAllDynamicJumpDispatchSourceBlocks) and reused across blocks
+  // to avoid rescanning all BlockInfos per block.
+  bool dynamicJumpTargetPredecessorsCoverCodegenEdges(
+      uint64_t BlockPC, const std::vector<uint64_t> &DispatchSources) const {
+    auto It = BlockInfos.find(BlockPC);
+    if (It == BlockInfos.end() || !It->second.IsDynamicJumpTargetCandidate) {
+      return true;
+    }
+
+    const std::vector<uint64_t> EnumeratedSources =
+        collectDynamicJumpSourceBlocksForInfo(It->second);
+    const std::unordered_set<uint64_t> EnumeratedSet(EnumeratedSources.begin(),
+                                                     EnumeratedSources.end());
+    for (uint64_t DispatchSourcePC : DispatchSources) {
+      if (EnumeratedSet.find(DispatchSourcePC) == EnumeratedSet.end()) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   bool getUniformDynamicJumpEntryDepthForRegion(uint64_t RegionEntryPC,
                                                 int32_t &EntryDepth) const {
     bool SawDynamicJump = false;
@@ -1321,6 +1372,10 @@ class EVMAnalyzer {
   }
 
   void finalizeLiftability() {
+    // Computed once and reused across blocks; independent of the per-block
+    // loop.
+    const std::vector<uint64_t> DispatchSources =
+        collectAllDynamicJumpDispatchSourceBlocks();
     for (auto &[EntryPC, Info] : BlockInfos) {
       (void)EntryPC;
       bool EntryKnown = Info.IsEntryStateCompatible;
@@ -1344,6 +1399,16 @@ class EVMAnalyzer {
           getPotentialEntryPredecessorsForBlock(EntryPC).size() > 4) {
         Info.CanLiftStack = false;
       }
+      // Never lift a dynamic-jump-target block whose statically enumerated
+      // predecessor set (which sizes its stack-merge phis) cannot account for
+      // every dynamic-jump dispatch edge codegen wires. The shape-class source
+      // enumeration is a strict subset of codegen's "all JUMPDESTs reachable
+      // from every dynamic jump" wiring, so such a block's phi would be
+      // undersized relative to its actual MIR predecessor count.
+      if (Info.CanLiftStack && !dynamicJumpTargetPredecessorsCoverCodegenEdges(
+                                   EntryPC, DispatchSources)) {
+        Info.CanLiftStack = false;
+      }
     }
   }
 

src/compiler/evm_frontend/evm_mir_compiler.cpp

@@ -165,7 +165,21 @@ MBasicBlock *EVMMirBuilder::resolveReachablePhiIncomingPredecessorBB(
     return CandidateBB;
   }
 
-  MBasicBlock *TargetBB = TargetIt->second;
+  return resolveReachablePredecessorBB(TargetIt->second, CandidateBB);
+}
+
+// Resolve CandidateBB to a real predecessor of TargetBB. If CandidateBB is
+// already a predecessor it is returned unchanged; otherwise the CFG is walked
+// forward from CandidateBB to the first reachable block that is a predecessor
+// of TargetBB. Returns CandidateBB when no such block is found, so callers can
+// detect failure by checking predecessor membership of the result.
+MBasicBlock *
+EVMMirBuilder::resolveReachablePredecessorBB(MBasicBlock *TargetBB,
+                                             MBasicBlock *CandidateBB) const {
+  if (TargetBB == nullptr || CandidateBB == nullptr) {
+    return CandidateBB;
+  }
+
   auto PredRange = TargetBB->predecessors();
   if (std::find(PredRange.begin(), PredRange.end(), CandidateBB) !=
       PredRange.end()) {
@@ -196,6 +210,50 @@ MBasicBlock *EVMMirBuilder::resolveReachablePhiIncomingPredecessorBB(
   return CandidateBB;
 }
 
+void EVMMirBuilder::finalizeStackMergePhiIncomingBlocks() {
+  // Stack-merge phi incoming blocks are resolved eagerly at the time each
+  // predecessor edge's stack state is assigned. For a loop back-edge that
+  // assignment runs before the predecessor block's terminator wires the real
+  // CFG edge into the loop header, so the recorded incoming block can be the
+  // predecessor EVM block's entry MIR block instead of the MIR block that
+  // actually branches into the loop header. Now that the full CFG is built,
+  // walk forward from each recorded incoming block to the real predecessor of
+  // the phi's owning block. Only the incoming-block pointer is corrected; the
+  // incoming value is preserved.
+  for (const auto &[Phi, OwnerBB] : StackMergePhiBlocks) {
+    if (Phi == nullptr || OwnerBB == nullptr) {
+      continue;
+    }
+    auto PredRange = OwnerBB->predecessors();
+    for (size_t Index = 0; Index < Phi->getNumIncoming(); ++Index) {
+      MBasicBlock *IncomingBB = Phi->getIncomingBlock(Index);
+      if (IncomingBB == nullptr) {
+        continue;
+      }
+      // Already a real predecessor: nothing to fix.
+      if (std::find(PredRange.begin(), PredRange.end(), IncomingBB) !=
+          PredRange.end()) {
+        continue;
+      }
+      // Walk forward from the recorded block to the actual predecessor of the
+      // phi's owning block, reusing the shared reachability resolver.
+      MBasicBlock *ResolvedBB =
+          resolveReachablePredecessorBB(OwnerBB, IncomingBB);
+      // The walk must land on a real predecessor; otherwise the phi would keep
+      // an incoming block that is not a predecessor and the verifier would
+      // reject it. Make that failure explicit in debug builds.
+      ZEN_ASSERT(std::find(PredRange.begin(), PredRange.end(), ResolvedBB) !=
+                     PredRange.end() &&
+                 "stack-merge phi incoming block did not resolve to a real "
+                 "predecessor");
+      if (ResolvedBB != IncomingBB) {
+        // Only the incoming-block pointer changes; the value is preserved.
+        Phi->setIncomingBlock(Index, ResolvedBB);
+      }
+    }
+  }
+}
+
 void EVMMirBuilder::loadEVMInstanceAttr() {
   InstanceAddr = createInstruction<ConversionInstruction>(
       false, OP_ptrtoint, &Ctx.I64Type,
@@ -501,6 +559,10 @@ void EVMMirBuilder::finalizeEVMBase() {
     CurFunc->deleteMBasicBlock(ReturnBB);
     ReturnBB = nullptr;
   }
+
+  // Correct loop back-edge merge phi incoming blocks against the now-complete
+  // CFG. No-op when no stack-merge phis were built (e.g. SSA stack-lift off).
+  finalizeStackMergePhiIncomingBlocks();
 }
 
 LoadInstruction *EVMMirBuilder::getInstanceElement(MType *ValueType,
@@ -1096,6 +1158,10 @@ typename EVMMirBuilder::Operand EVMMirBuilder::materializeStackMergeOperand(
                        IncomingComponents[ComponentIndex]);
     }
     PhiComponents[ComponentIndex] = Phi;
+    // Record the phi against its loop-header block so its incoming blocks can
+    // be re-resolved once the full CFG (including back-edge terminators) is
+    // built. See finalizeStackMergePhiIncomingBlocks().
+    StackMergePhiBlocks.emplace_back(Phi, CurBB);
   }
 
   for (size_t ComponentIndex = 0; ComponentIndex < EVM_ELEMENTS_COUNT;
@@ -5234,7 +5300,24 @@ MInstruction *EVMMirBuilder::loadVariable(Variable *Var) {
 
 PhiInstruction *EVMMirBuilder::createPendingPhi(MType *Type,
                                                 size_t NumIncoming) {
-  return createInstruction<PhiInstruction>(true, Type, NumIncoming);
+  // Create the phi without appending it to the block end. Phi instructions
+  // must be contiguous at the block start (verified by the MIR verifier).
+  // When merging multiple stack slots, each slot emits phis followed by
+  // non-phi temp-store dassigns; appending at the end would interleave a
+  // later slot's phis after an earlier slot's dassigns and break the
+  // phi-contiguity invariant. Insert the phi right after the existing leading
+  // phis instead so all phis stay grouped at the front.
+  PhiInstruction *Phi =
+      createInstruction<PhiInstruction>(false, Type, NumIncoming);
+  size_t InsertIdx = 0;
+  for (MInstruction *Inst : *CurBB) {
+    if (Inst->getKind() != MInstruction::PHI) {
+      break;
+    }
+    ++InsertIdx;
+  }
+  CurBB->addStatement(InsertIdx, Phi);
+  return Phi;
 }
 
 size_t EVMMirBuilder::getPhiIncomingSlot(PhiInstruction *Phi,

src/compiler/evm_frontend/evm_mir_compiler.h

@@ -1152,6 +1152,8 @@ class EVMMirBuilder final {
   MBasicBlock *
   resolveReachablePhiIncomingPredecessorBB(uint64_t TargetBlockPC,
                                            MBasicBlock *CandidateBB) const;
+  MBasicBlock *resolveReachablePredecessorBB(MBasicBlock *TargetBB,
+                                             MBasicBlock *CandidateBB) const;
 
   CompilerContext &Ctx;
   MFunction *CurFunc = nullptr;
@@ -1202,6 +1204,17 @@ class EVMMirBuilder final {
   std::map<PhiInstruction *, std::map<uint64_t, size_t>> PhiIncomingSlotMap;
   std::map<VariableIdx, PhiInstruction *> StackMergePhiVarMap;
 
+  // Stack-merge phis and the loop-header block they belong to. A merge phi's
+  // incoming block is resolved eagerly when each predecessor edge's stack
+  // state is assigned (materializeStackMergeOperand / assignStackMergeOperand).
+  // For a loop back-edge this assignment happens before the predecessor's
+  // terminator wires the real CFG edge into the loop header, so the resolved
+  // incoming block can be the predecessor EVM block's entry MIR block rather
+  // than its terminator MIR block. finalizeStackMergePhiIncomingBlocks()
+  // re-resolves every recorded incoming block against the now-complete CFG.
+  std::vector<std::pair<PhiInstruction *, MBasicBlock *>> StackMergePhiBlocks;
+  void finalizeStackMergePhiIncomingBlocks();
+
   struct MemoryCompileStats {
     uint64_t MLoadExpandCount = 0;
     uint64_t MStoreExpandCount = 0;

src/compiler/mir/instructions.h

@@ -203,6 +203,14 @@ class PhiInstruction : public DynamicOperandInstruction {
     }
   }
 
+  // Update only the incoming block for an edge, leaving the incoming value
+  // unchanged. Used when the CFG edge an incoming block represents is resolved
+  // after the value has already been wired.
+  void setIncomingBlock(size_t Index, MBasicBlock *Block) {
+    ZEN_ASSERT(Index < Blocks.size());
+    Blocks[Index] = Block;
+  }
+
 private:
   friend class DynamicOperandInstruction;