fix(vcr-ra): optimized path preserves callee-saved r4-r8 — AAPCS prologue (#490, #242) (#495)

The optimized ARM selection path (the default, non-`--relocatable` self-contained
image) uses r4-r8 as scratch / promoted locals but emitted no prologue, so a
caller's callee-saved registers were silently clobbered — a systemic AAPCS
violation. It was masked because non-leaf callers route through the direct
selector (which over-saves) and the frozen byte gate only covers the
`--relocatable` direct path; it blocks flipping the non-leaf thin-forwarder
prologue lever (#428).

Fix: after range-realloc, wrap a body that genuinely touches a callee-saved
register in a conservative `push {r4-r8,lr}` / `pop {r4-r8,pc}`;
`shrink_callee_saved_saves` then trims the save to the registers actually used.
Two new liveness helpers:

- `body_uses_callee_saved` — the emit-side twin of `shrink_callee_saved_saves`,
  reading the same control-flow allowlist + `reg_effect` classification as
  push-conditions. Conservative on `reg_effect` `None` (an unmodeled op may touch
  a callee-saved register → force the push): under-saving is the AAPCS bug,
  over-saving is harmless (shrink pads it down).
- `ensure_callee_saved_prologue` — wraps the body, no-op when a prologue already
  exists (direct path) or the body is callee-saved-free (a spurious push is
  permanent — shrink never removes one, only pads).

Decided on the POST-realloc body, where realloc has lowered low-pressure r4-r8
scratch back to r0-r3, so a save is added only for genuinely-clobbered registers
(a pre-realloc decision over-saves). Runs in both realloc-on (shrink trims) and
realloc-off (full save kept) configurations.

Scope: callee-saved (r4-r8) only. Caller-saved (r0-r3) preservation across
import calls on the optimized path is a separate known gap (#197). Functions with
>4 params (incoming stack args) decline to the direct selector, so the wrap is a
no-op there and cannot perturb stack-arg offsets (verified byte-identical to
`--no-optimize`). Default-on (a correctness fix is not flag-gated); it adds
save/restore cycles to optimized-path functions that use r4-r8 — a non-shipped
path (gale ships `--relocatable`), so no silicon cost. Frozen byte gate stays
bit-identical (direct path untouched).

Validation:
- New unicorn sentinel oracle (`callee_saved_490_differential.py`, CI-gated):
  sets r4-r8 to sentinels, asserts result == wasmtime AND sentinels restored,
  across 16-bit push and 32-bit PUSH.W (high-register) forms; passes default,
  `SYNTH_RANGE_REALLOC=0`, and `SYNTH_DEAD_FRAME_ELIM=1`. FAILs without the fix.
- 4 new liveness unit tests; full workspace suite + bulk-memory optimized-path
  differential green; frozen byte gate bit-identical.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: avrabe

.github/workflows/ci.yml

@@ -343,3 +343,40 @@ jobs:
         env:
           SYNTH: ./target/debug/synth
         run: python scripts/repro/const_addr_fold_riscv_differential.py
+
+  callee-saved-490-oracle:
+    name: optimized-path callee-saved preservation oracle
+    # VCR-ORACLE-001 (#242, #490): EXECUTE optimized-path functions that use
+    # r4-r8 under unicorn (UC_ARCH_ARM / Thumb) with r4-r8 set to sentinels, and
+    # assert both the result matches wasmtime AND every callee-saved register is
+    # restored at return. This is the runtime gate for the #490 fix (the
+    # optimized path now emits the `push {r4-r8,lr}` / `pop {r4-r8,pc}` it was
+    # missing); the frozen byte gate only covers the `--relocatable` direct path,
+    # so nothing else exercises the optimized path's AAPCS compliance. Covers the
+    # 16-bit push and the 32-bit PUSH.W (high-register) forms. Isolated job:
+    # emulation deps pip-installed here ONLY.
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - uses: dtolnay/rust-toolchain@stable
+      - name: Cache Cargo dependencies
+        uses: actions/cache@v5
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            target/
+          key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-cargo-
+      - name: Build synth
+        run: cargo build -p synth-cli
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.x"
+      - name: Install emulation deps
+        run: pip install wasmtime unicorn pyelftools
+      - name: Run callee-saved preservation oracle
+        env:
+          SYNTH: ./target/debug/synth
+        run: python scripts/repro/callee_saved_490_differential.py

crates/synth-backend/src/arm_backend.rs

@@ -441,9 +441,22 @@ fn compile_wasm_to_arm(
         } else {
             out
         };
+        // #490 (epic #242): the optimized selector uses r4-r8 as scratch /
+        // promoted locals but emits no prologue, silently clobbering a caller's
+        // callee-saved registers. Add the missing `push {r4-r8,lr}` /
+        // `pop {r4-r8,pc}` HERE — on the post-realloc body, where realloc has
+        // lowered low-pressure r4-r8 scratch back to r0-r3, so a save is added
+        // only for registers genuinely clobbered. `shrink_callee_saved_saves`
+        // (next) then trims it to the used set. No-op on the direct path (it
+        // already has its own prologue) and on callee-saved-free leaves.
+        let out = synth_synthesis::liveness::ensure_callee_saved_prologue(&out);
         synth_synthesis::liveness::shrink_callee_saved_saves(&out).unwrap_or(out)
     } else {
-        arm_instrs
+        // Range-realloc off (`SYNTH_RANGE_REALLOC=0`): the optimized path still
+        // must preserve the callee-saved registers it clobbers (#490). No shrink
+        // (it is coupled to the realloc lever), so the conservative full save
+        // stays — correct, just not minimised in this debug configuration.
+        synth_synthesis::liveness::ensure_callee_saved_prologue(&arm_instrs)
     };
 
     // VCR-RA-001 SHADOW ALLOCATION (#209/#242): run the register allocator on

crates/synth-synthesis/src/liveness.rs

@@ -2487,6 +2487,112 @@ pub fn shrink_callee_saved_saves(instrs: &[ArmInstruction]) -> Option<Vec<ArmIns
     Some(out)
 }
 
+/// #490 (epic #242): does this body clobber/read any callee-saved register, so
+/// the optimized path must wrap it in a `push {r4-r8,lr}` / `pop {r4-r8,pc}`
+/// prologue/epilogue to honour AAPCS?
+///
+/// This is the **emit-side twin** of [`shrink_callee_saved_saves`]: it reads the
+/// exact same per-op classification (the control-flow allowlist + `reg_effect`)
+/// as *push-conditions* rather than *decline-conditions*, so the two passes
+/// always agree. [`ensure_callee_saved_prologue`] emits a conservative full
+/// `push {r4-r8,lr}` when this returns `true`; `shrink_callee_saved_saves` then
+/// trims that push to the registers actually touched (or declines, leaving the
+/// full push, on frame/call/unmodeled bodies it cannot prove safe).
+///
+/// **Conservative on `None` (fail-safe).** An op `reg_effect` cannot model
+/// (i64-pair pseudo-ops, calls, anything outside the modeled scope) *might* write
+/// a callee-saved register, so it forces the push. Under-saving is the exact
+/// AAPCS violation #490 fixes; over-saving an unused register is harmless (shrink
+/// pads it down, or — on a body shrink declines — it is a few dead bytes). The
+/// only load-bearing answer is `false`: a spurious push on a callee-saved-free
+/// leaf is permanent (shrink never *removes* a push), so the control-flow
+/// allowlist and the cs-intersection test must be exact, mirroring shrink. The
+/// decision runs on the **post-range-realloc** body, where low-pressure r4-r8
+/// scratch has already been lowered to r0-r3, so only genuine clobbers remain.
+pub fn body_uses_callee_saved(instrs: &[ArmInstruction]) -> bool {
+    use ArmOp::*;
+    const CALLEE_SAVED: [Reg; 5] = [Reg::R4, Reg::R5, Reg::R6, Reg::R7, Reg::R8];
+    for ins in instrs {
+        match &ins.op {
+            // Function-boundary markers and pure label control flow carry no
+            // callee-saved data effect (mirrors shrink's allowlist exactly).
+            Push { .. }
+            | Pop { .. }
+            | Label { .. }
+            | B { .. }
+            | BOffset { .. }
+            | BCondOffset { .. }
+            | Bhs { .. }
+            | Blo { .. }
+            | Bcc { .. } => {}
+            Bx { rm } if *rm == Reg::LR => {}
+            op => match reg_effect(op) {
+                Some(e) => {
+                    if e.defs
+                        .iter()
+                        .chain(e.uses.iter())
+                        .any(|r| CALLEE_SAVED.contains(r))
+                    {
+                        return true;
+                    }
+                }
+                // Unmodeled op → assume it may touch a callee-saved register.
+                None => return true,
+            },
+        }
+    }
+    false
+}
+
+/// #490 (epic #242): give the optimized path the callee-saved prologue/epilogue
+/// it is missing. The optimized selector uses r4-r8 as scratch / promoted locals
+/// but emits no `push`/`pop`, so a caller's r4-r8 are silently clobbered — a
+/// systemic AAPCS violation. When the (post-realloc) body genuinely touches a
+/// callee-saved register, wrap it in a conservative full `push {r4-r8,lr}` /
+/// `pop {r4-r8,pc}`; [`shrink_callee_saved_saves`] (run next) trims that to the
+/// registers actually used, or declines and leaves the full save on a
+/// frame/call/unmodeled body it cannot prove safe.
+///
+/// No-op when a prologue already exists (the direct selector emits its own
+/// `push {…,lr}`) or the body is callee-saved-free (e.g. a pure-r0-r3 leaf —
+/// left byte-identical, which is load-bearing: an added push could never be
+/// removed later). The push is inserted at index 0, landing before any
+/// `sub sp,#frame` the optimized path reserved; each `bx lr` becomes
+/// `pop {…,pc}`, landing after the matching `add sp,#frame`.
+pub fn ensure_callee_saved_prologue(instrs: &[ArmInstruction]) -> Vec<ArmInstruction> {
+    use ArmOp::*;
+    // Already has a prologue (direct path) → leave it for shrink to size.
+    if instrs
+        .iter()
+        .any(|i| matches!(&i.op, Push { regs } if regs.contains(&Reg::LR)))
+    {
+        return instrs.to_vec();
+    }
+    if !body_uses_callee_saved(instrs) {
+        return instrs.to_vec();
+    }
+    let mut out = Vec::with_capacity(instrs.len() + 1);
+    out.push(ArmInstruction {
+        op: Push {
+            regs: vec![Reg::R4, Reg::R5, Reg::R6, Reg::R7, Reg::R8, Reg::LR],
+        },
+        source_line: None,
+    });
+    for ins in instrs {
+        if matches!(&ins.op, Bx { rm } if *rm == Reg::LR) {
+            out.push(ArmInstruction {
+                op: Pop {
+                    regs: vec![Reg::R4, Reg::R5, Reg::R6, Reg::R7, Reg::R8, Reg::PC],
+                },
+                source_line: ins.source_line,
+            });
+        } else {
+            out.push(ins.clone());
+        }
+    }
+    out
+}
+
 /// VCR-RA-002 (#390, epic #242): eliminate a provably-dead stack frame.
 ///
 /// `compute_local_layout` reserves a frame slot — materialized as
@@ -5885,6 +5991,101 @@ mod tests {
         assert_eq!(out[4], seq[4]);
     }
 
+    // ---- ensure_callee_saved_prologue (#490) ----
+
+    #[test]
+    fn ensure_prologue_wraps_body_that_touches_callee_saved() {
+        // Optimized-path shape: no prologue, body writes r4/r5, returns via bx lr.
+        let seq = vec![
+            ins(ArmOp::Movw {
+                rd: Reg::R4,
+                imm16: 100,
+            }),
+            ins(ArmOp::Add {
+                rd: Reg::R5,
+                rn: Reg::R0,
+                op2: Operand2::Reg(Reg::R4),
+            }),
+            ins(ArmOp::Mov {
+                rd: Reg::R0,
+                op2: Operand2::Reg(Reg::R5),
+            }),
+            ins(ArmOp::Bx { rm: Reg::LR }),
+        ];
+        let out = ensure_callee_saved_prologue(&seq);
+        assert_eq!(
+            out[0].op,
+            ArmOp::Push {
+                regs: vec![Reg::R4, Reg::R5, Reg::R6, Reg::R7, Reg::R8, Reg::LR]
+            }
+        );
+        assert_eq!(
+            out.last().unwrap().op,
+            ArmOp::Pop {
+                regs: vec![Reg::R4, Reg::R5, Reg::R6, Reg::R7, Reg::R8, Reg::PC]
+            }
+        );
+        // Body untouched between the new prologue and epilogue.
+        assert_eq!(out[1..=3], seq[0..=2]);
+        // And shrink then sizes it to the registers actually used ({r4,r5} → pad).
+        let shrunk = shrink_callee_saved_saves(&out).expect("shrinks");
+        assert_eq!(
+            shrunk[0].op,
+            ArmOp::Push {
+                regs: vec![Reg::R4, Reg::R5, Reg::R6, Reg::LR]
+            }
+        );
+    }
+
+    #[test]
+    fn ensure_prologue_leaves_callee_saved_free_leaf_untouched() {
+        // Pure r0-r3 leaf: no callee-saved register touched → no push added
+        // (a spurious push would be permanent — shrink never removes one).
+        let seq = vec![
+            ins(ArmOp::Add {
+                rd: Reg::R0,
+                rn: Reg::R0,
+                op2: Operand2::Imm(1),
+            }),
+            ins(ArmOp::Bx { rm: Reg::LR }),
+        ];
+        assert_eq!(ensure_callee_saved_prologue(&seq), seq);
+        assert!(!body_uses_callee_saved(&seq));
+    }
+
+    #[test]
+    fn ensure_prologue_is_conservative_on_unmodeled_ops() {
+        // An op reg_effect cannot model (here a Bl call) might clobber a
+        // callee-saved register → force the push (fail-safe). shrink then
+        // declines on the call and leaves the full save.
+        let seq = vec![
+            ins(ArmOp::Bl {
+                label: "func_1".to_string(),
+            }),
+            ins(ArmOp::Bx { rm: Reg::LR }),
+        ];
+        assert!(body_uses_callee_saved(&seq));
+        let out = ensure_callee_saved_prologue(&seq);
+        assert!(matches!(&out[0].op, ArmOp::Push { regs } if regs.contains(&Reg::LR)));
+        assert_eq!(shrink_callee_saved_saves(&out), None);
+    }
+
+    #[test]
+    fn ensure_prologue_skips_body_that_already_has_one() {
+        // Direct-path output already carries its own `push {…,lr}` → no-op
+        // (shrink owns sizing it; double-wrapping would corrupt the frame).
+        let seq = vec![
+            prologue(),
+            ins(ArmOp::Add {
+                rd: Reg::R5,
+                rn: Reg::R0,
+                op2: Operand2::Imm(1),
+            }),
+            epilogue(),
+        ];
+        assert_eq!(ensure_callee_saved_prologue(&seq), seq);
+    }
+
     // ---- elide_dead_frame (VCR-RA-002, #390) ----
 
     fn frame_sub(n: i32) -> ArmInstruction {

crates/synth-synthesis/src/optimizer_bridge.rs

@@ -5516,6 +5516,13 @@ impl OptimizerBridge {
             arm_instrs.push(ArmOp::Bx { rm: Reg::LR });
         }
 
+        // #490: the callee-saved prologue/epilogue the optimized path needs to
+        // honour AAPCS is added AFTER range-realloc (in `arm_backend`), where the
+        // re-allocated body shows which callee-saved registers are *genuinely*
+        // live — realloc lowers low-pressure r4-r8 scratch back to r0-r3, so a
+        // decision here (pre-realloc) would over-save. See
+        // `liveness::ensure_callee_saved_prologue`.
+
         Ok(arm_instrs)
     }
 }

scripts/repro/callee_saved_490.wat

@@ -0,0 +1,31 @@
+(module
+  (memory 1)
+  ;; #490 fixture (epic #242): functions the OPTIMIZED path lowers using r4-r8 as
+  ;; scratch. Before the fix the optimized path emitted no prologue and silently
+  ;; clobbered a caller's callee-saved registers; the differential sets r4-r8 to
+  ;; sentinels, runs each function, and asserts the sentinels survive AND the
+  ;; result matches wasmtime — proving the push/pop the fix adds preserves them.
+  ;;
+  ;; Pressure is tuned to exercise the callee-saved range WITHOUT exhausting the
+  ;; R0-R8 pool (the optimized path has no spill, so an exhausting fold would hit
+  ;; a separate pre-existing miscompile and conflate two issues). Each function
+  ;; below is differential-verified to compute correctly on the optimized path.
+
+  ;; low: two live temps → 16-bit `push {r4,r5,r6,lr}` (r6 is shrink's even-pad).
+  (func $low (export "low") (param i32) (result i32)
+    (i32.add (local.get 0) (i32.const 100)))
+
+  ;; mid: three live subterms → `push {r4,r5,r6,lr}`.
+  (func $mid (export "mid") (param i32 i32) (result i32)
+    (i32.add (i32.mul (local.get 0) (local.get 1))
+             (i32.sub (local.get 0) (local.get 1))))
+
+  ;; wide: enough simultaneously-live products to reach r8 → 32-bit Thumb-2
+  ;; `push.w {r4-r8,lr}`, exercising the high-register save the 16-bit form
+  ;; cannot encode.
+  (func $wide (export "wide") (param i32 i32 i32) (result i32)
+    (i32.add
+      (i32.add (i32.mul (local.get 0) (local.get 1))
+               (i32.mul (local.get 1) (local.get 2)))
+      (i32.mul (local.get 0) (local.get 2))))
+)