fix(gc): walk legal 8-mod-16 frame records, and stop moving unwinder roots a frame

.github/workflows/gc-native-roots.yml

@@ -77,8 +77,20 @@
 # CLAUDE.md's GC knob kill-policy: an arm exercising the non-default state, or
 # delete the mode.
 #
-#   PERRY_GC_SAFEPOINT_ONLY  -> native-roots-rs4gc, "safepoint-only" steps
-#   PERRY_STACKMAP_WALKER    -> native-roots-rs4gc, "both non-default walkers"
+#   PERRY_GC_SAFEPOINT_ONLY  -> NOTHING. This entry was false: no step in this
+#                               file, or any other, ever set the variable. Left
+#                               spelled out rather than quietly deleted, because
+#                               a ledger that has been wrong once has to say so.
+#   PERRY_STACKMAP_WALKER    -> native-roots-rs4gc, "Both non-default walkers"
+#                               step. Also false until #7392 — the entry claimed
+#                               an arm that did not exist, and both walkers it
+#                               named were carrying real bugs the whole time:
+#                               `unwind` placed every SP-relative root one frame
+#                               too low, and `verify` could not run at all
+#                               because the fast walk bailed on a legal frame
+#                               record. Measured on aarch64-Linux the day the
+#                               step was added: 2 of 11 probes passed all three
+#                               walkers before the fix, 11 of 11 after.
 #   PERRY_RS4GC              -> native-roots-rs4gc
 #   PERRY_STATEPOINT_REPORT  -> not a knob. It survives as the driver's
 #                               internal handoff to the rayon module workers,
@@ -376,6 +388,113 @@ jobs:
           "$py" scripts/gc_walker_trace_assert.py /tmp/rs4gc-trace.err \
             --require-locations
 
+      # #7392. `PERRY_STACKMAP_WALKER` selects between three walks over the same
+      # roots, and until this step nothing anywhere set it — the ledger at the
+      # top of this file said otherwise for months. Both non-default walks were
+      # broken the whole time, on every platform, and could not have been
+      # noticed:
+      #
+      #   unwind  resolved SP-relative roots against `CFA - stack_size`, but the
+      #           CFA an `_Unwind_Backtrace` callback reports IS the frame's
+      #           stack pointer, so every such root landed one frame too low. A
+      #           wrong stack word looks exactly like a right one to everything
+      #           downstream — no code knows what a root slot should contain.
+      #   verify  runs both walks and compares the slot sets, i.e. it is the
+      #           only check that can catch the above. It could not run: the
+      #           fast walk rejected a legal 8-mod-16 frame record (which is
+      #           what AArch64 ELF frame lowering produces whenever an odd
+      #           number of callee-saved GPRs sits below the pair) and returned
+      #           "unavailable", which verify turns into a panic.
+      #
+      # So the default walker was the only one anyone exercised, and on
+      # aarch64-Linux its bail-out landed in the broken fallback: the roots of
+      # that frame were never rewritten after an evacuation, and the mutator
+      # dereferenced a stale from-space pointer (`02_survivor_promotion`,
+      # SIGSEGV). Measured on aarch64-Linux before the fix: 2 of 11 probes
+      # passed all three walkers. After: 11 of 11.
+      #
+      # `verify` needs the fp-chain walk to exist, which is aarch64-only, so it
+      # is gated on the arch rather than skipped quietly. Windows has neither
+      # walker (`RtlVirtualUnwind` is its own module) and is excluded outright.
+      - name: Both non-default walkers
+        if: ${{ !cancelled() && runner.os != 'Windows' }}
+        run: |
+          set -euo pipefail
+          modes="unwind"
+          if [ "${{ matrix.arch }}" = "aarch64" ]; then
+            modes="unwind verify"
+          fi
+          echo "walkers under test: $modes"
+          checked=0
+          for probe in benchmarks/gc_ratchet/probes/*.ts; do
+            name=$(basename "$probe" .ts)
+            # Binaries and oracles come from the matrix step above, same job and
+            # same runner — as the walker-liveness assert already does. Missing
+            # ones are a hard error: silently checking nothing is the failure
+            # mode this whole step exists to close.
+            [ -x "/tmp/rs4gc-$name" ] \
+              || { echo "::error::$name has no binary from the probe matrix step"; exit 1; }
+            [ -s "/tmp/rs4gc-$name.oracle" ] \
+              || { echo "::error::$name has no pinned oracle from the probe matrix step"; exit 1; }
+            for mode in $modes; do
+              PERRY_STACKMAP_WALKER="$mode" \
+              PERRY_RS4GC=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 \
+              PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \
+                "/tmp/rs4gc-$name" > "/tmp/walker-$name-$mode.out" \
+                                  2> "/tmp/walker-$name-$mode.err" \
+                || { echo "::error::$name crashed under PERRY_STACKMAP_WALKER=$mode"; \
+                     tail -20 "/tmp/walker-$name-$mode.err"; exit 1; }
+              diff "/tmp/rs4gc-$name.oracle" "/tmp/walker-$name-$mode.out" \
+                || { echo "::error::$name diverged from the pinned oracle under PERRY_STACKMAP_WALKER=$mode"; exit 1; }
+              checked=$((checked+1))
+            done
+          done
+          echo "non-default walker runs, all oracle-diffed: $checked"
+          [ "$checked" -gt 0 ] \
+            || { echo "::error::no probe ran under a non-default walker — the step measured nothing"; exit 1; }
+
+          # Everything above proves a process exited zero and printed what the
+          # oracle printed. It does NOT prove `PERRY_STACKMAP_WALKER=$mode`
+          # selected that walker, that the walker reached a mapped frame, or
+          # that anything was evacuated — and all three modes are supposed to
+          # produce identical output, so program output cannot tell them apart.
+          # That is CLAUDE.md's fourth hazard, and the very shape of #7392: the
+          # walker under test read the wrong words for months while every probe
+          # stayed green.
+          #
+          # So assert the subject was live, per mode, off one traced run of
+          # `11_collect_at_depth` (deep stack, a live root in every frame, so
+          # the telemetry is non-trivial on every arm):
+          #
+          #   fp_walks == 0  proves `unwind` took effect — nonzero means the
+          #                  chain walk ran anyway and the mode did nothing.
+          #   fp_walks > 0   proves `verify` cross-checked something rather
+          #                  than quietly not running the chain walk.
+          #   --require-locations   the walker stepped frames, matched
+          #                  safepoints and enumerated roots, rather than
+          #                  visiting nothing while other root sources covered.
+          #   evacuation liveness   a copying minor ran and MOVED an object, so
+          #                  the roots being enumerated were roots that had to
+          #                  be rewritten (#6942/#6946, #7336).
+          #
+          # python3 unqualified: this step never runs on Windows, which is the
+          # only runner where the toolcache spells it `python`.
+          for mode in $modes; do
+            case "$mode" in
+              unwind) fp_flag="--forbid-fp-walks" ;;
+              verify) fp_flag="--require-fp-walks" ;;
+              *) echo "::error::no liveness assert defined for walker $mode"; exit 1 ;;
+            esac
+            PERRY_GC_TRACE=1 PERRY_GC_DIAG=1 PERRY_STACKMAP_WALKER="$mode" \
+            PERRY_RS4GC=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 \
+            PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \
+              /tmp/rs4gc-11_collect_at_depth > /dev/null 2> "/tmp/walker-trace-$mode.err"
+            python3 scripts/gc_walker_trace_assert.py "/tmp/walker-trace-$mode.err" \
+              --require-locations $fp_flag
+            python3 scripts/gc_evacuation_liveness_assert.py "/tmp/walker-trace-$mode.err" \
+              --probe "11_collect_at_depth (PERRY_STACKMAP_WALKER=$mode)"
+          done
+
       # #7327. Everything above pins PERRY_LLVM_OPT + PERRY_LLVM_CLANG to one
       # brew install, because RS4GC piped IR through an external `opt` and a
       # newer `opt` emits attributes an older `clang` cannot parse. That made

changelog.d/7400-native-root-walkers-aarch64.md

@@ -0,0 +1,37 @@
+**Fixed** two aarch64 native-root walker defects that together crashed the
+`PERRY_RS4GC` probe matrix on aarch64-Linux — `02_survivor_promotion` took a
+SIGSEGV under forced evacuation (#7392). Neither was in statepoint lowering.
+
+The x29 chain walk demanded a 16-byte-aligned frame record. AAPCS64 fixes what a
+frame record *contains* and leaves where it sits in the frame unspecified; only
+SP must be 16-byte aligned. LLVM's AArch64 **ELF** frame lowering puts the
+`x29,x30` pair below the other callee-saved GPRs, so an odd number of those
+lands the record 8 mod 16 — measured in the runtime frame that tripped it, which
+saves x19..x23 and v8 and whose CFI reads `CFA = x29+56`. Darwin pins the record
+to the top of the frame, which is why this could never fire on macOS. The walk
+read a legal record as a corrupt chain and abandoned the stack mid-walk.
+
+It abandoned into the platform unwinder, which resolved SP-relative roots
+against `CFA - stack_size`. That follows DWARF's definition of a CFA and is
+wrong for what `_Unwind_GetCFA` returns: inside an `_Unwind_Backtrace` callback
+the CFA already *is* the stack pointer of the frame whose return address
+`_Unwind_GetIP` reports, so every such root landed one whole frame too low.
+Measured in-process on the failing probe: at the CFA the slot holds a NaN-boxed
+pointer (`0x7ffd…`); 240 bytes lower — that frame's `stack_size` — it holds a
+stack address. So the frame's real roots were never rewritten after an
+evacuation, and the mutator dereferenced a stale from-space pointer.
+
+`CFA_RETURN_ADDRESS_BYTES` is deleted: a standalone probe recording each frame's
+real stack pointer and matching it against a live walk gives the same answer on
+aarch64 Linux (libgcc), aarch64 macOS (Apple libunwind) and x86-64 Linux, so the
+return-address convention never entered into it. That probe ships as
+`unwind_cfa_is_the_frames_stack_pointer`, in `cargo-test` on every host.
+
+`PERRY_STACKMAP_WALKER` had no arm anywhere in the tree, while the knob ledger
+in `gc-native-roots.yml` claimed one — which is how both non-default walkers
+carried bugs indefinitely, `verify` being the only check that can catch a wrong
+root base at all (nothing downstream knows what a root slot should contain).
+The workflow now runs every probe under `unwind`, and under `verify` on the
+aarch64 arms, each byte-diffed against the pinned Node oracle. Measured on
+aarch64-Linux: 2 of 11 probes passed all three walkers before this change,
+11 of 11 after.

crates/perry-runtime/src/gc/roots/stack_maps.rs

@@ -51,6 +51,14 @@ struct StackMapRecord {
     /// location needs the FP-to-SP offset (see `fp_to_sp_offset`).
     function_address: usize,
     /// The containing function's total frame size from the function table.
+    ///
+    /// Decoded because the map carries it and `parse_gc_map`'s tests pin that
+    /// the field is read at the right offset — NOT because a walker may build
+    /// a root's base out of it. The unwinder fallback used to compute
+    /// `CFA - stack_size` and that was #7392: the CFA a backtrace callback
+    /// reports IS the frame's stack pointer, so the subtraction put every
+    /// SP-relative root one frame too low.
+    #[allow(dead_code)]
     stack_size: u64,
     /// Half-open range into `StackMapIndex::roots`.
     ///
@@ -100,25 +108,40 @@ static STACK_MAPS: OnceLock<StackMapIndex> = OnceLock::new();
 const DWARF_REG_FP_AARCH64: u16 = 29;
 const DWARF_REG_SP_AARCH64: u16 = 31;
 
-// How far below the CFA this frame's return address sits, if it sits on the
-// stack at all. This is NOT a constant across architectures and getting it
-// wrong shifts every SP-relative root by a word:
+// A frame record is two 64-bit words, so it needs EIGHT-byte alignment, not
+// sixteen.
 //
-//   x86-64: `call` PUSHES the return address, so CFA is the caller's SP before
-//           the call and the callee's SP starts one slot lower.
-//   aarch64: `bl` writes the return address to x30. Nothing is pushed, so the
-//           frame's SP is simply CFA - stack_size.
+// The stack pointer is 16-byte aligned at a public interface on both supported
+// ABIs, and on Darwin the frame record sits at the top of the frame, so there
+// x29 is always 16-aligned as well and a `fp & 0xF` test never fires. AAPCS64
+// does not promise that: §6.4.6 fixes the record's CONTENTS and leaves its
+// location within the frame unspecified, and LLVM's AArch64 **ELF** frame
+// lowering puts the `x29,x30` pair *below* the other callee-saved GPRs. With an
+// odd number of those, the pair lands 8 mod 16.
 //
-// The aarch64 case is easy to miss because `chain_walkable` is true there, so
-// the fast x29 walker normally runs and this path is only the fallback — an
-// eight-byte error would stay latent until the fast walk bailed.
-// Unused on Windows: `RtlVirtualUnwind` hands back the frame's real `Rsp`, so
-// the Windows walker never derives SP from a CFA (there is no CFA query).
-#[cfg(target_arch = "x86_64")]
-#[cfg_attr(target_os = "windows", allow(dead_code))]
-const CFA_RETURN_ADDRESS_BYTES: usize = std::mem::size_of::<usize>();
-#[cfg(not(target_arch = "x86_64"))]
-const CFA_RETURN_ADDRESS_BYTES: usize = 0;
+// Measured on aarch64-unknown-linux-gnu (#7392), from the `.eh_frame` of a
+// runtime frame that saves x19..x23 and v8:
+//
+//     LOC        CFA      x19  x20  x21  x22  x23  x29  ra   v8
+//     ...        x29+56   c-8  c-16 c-24 c-32 c-40 c-56 c-48 c-64
+//
+// x29 = CFA - 56, and CFA is 16-aligned, so x29 ≡ 8 (mod 16) — a legal frame
+// record the 16-byte test rejected. That abandoned the fast walk mid-stack and
+// fell back to the unwinder, which had its own SP-base bug (see
+// `unwind::walk_frame`), so the frame's roots were never rewritten after an
+// evacuation and the mutator then dereferenced a stale from-space pointer.
+//
+// Only the fp-chain walker reads it, and that walker exists on aarch64 Unix
+// alone — the same cfg, spelled out rather than approximated, so an x86-64 or
+// Windows build does not warn on a constant it has no walker for.
+#[cfg_attr(
+    not(all(
+        any(target_vendor = "apple", target_os = "linux"),
+        target_arch = "aarch64"
+    )),
+    allow(dead_code)
+)]
+const FRAME_RECORD_ALIGN_MASK: usize = 0x7;
 
 // Which DWARF register is the stack pointer on the machine this runtime was
 // built for. Distinct from the format constants above and used only to choose
@@ -1000,21 +1023,31 @@ mod unwind {
         for record in matched {
             for location in state.index.locations(record) {
                 state.stats.locations_visited = state.stats.locations_visited.saturating_add(1);
-                // SP-relative roots derive their base from the CFA. By the
-                // SysV/AAPCS definition the CFA is the caller's stack pointer
-                // immediately before the call, so this frame's body stack
-                // pointer sits one return-address slot plus this function's own
-                // frame below it — and `stack_size` is exactly that frame,
-                // recorded per function in the map.
+                // SP-relative roots take the CFA as their base VERBATIM.
+                //
+                // Not `CFA - stack_size`, which is what the DWARF definition of
+                // a CFA suggests and what this code used to compute. What
+                // `_Unwind_GetCFA` returns inside an `_Unwind_Backtrace`
+                // callback is the body stack pointer of the frame whose return
+                // address `_Unwind_GetIP` just reported, so subtracting the
+                // frame size lands one whole frame too low.
+                //
+                // MEASURED (#7392) by `unwind_cfa_is_the_frames_stack_pointer`
+                // below, which records each frame's real SP and matches it
+                // against the walk: the identity holds on aarch64 Linux
+                // (libgcc), aarch64 macOS (Apple libunwind) and x86-64 Linux
+                // alike — so there is no return-address adjustment to make and
+                // no per-architecture constant left to get wrong.
+                //
+                // It stayed invisible because this is the FALLBACK path: on
+                // aarch64 the x29 chain walk normally answers, and wherever it
+                // bailed this read unrelated words instead of the roots, which
+                // nothing downstream can notice — no code knows what a root slot
+                // is supposed to contain. Cross-checked directly on
+                // `02_survivor_promotion`: at the CFA the slot holds a NaN-boxed
+                // pointer (`0x7ffd…`); one frame lower it holds a stack address.
                 let base = if location.dwarf_reg == ARCH_DWARF_SP {
-                    let cfa = _Unwind_GetCFA(context);
-                    match cfa
-                        .checked_sub(CFA_RETURN_ADDRESS_BYTES)
-                        .and_then(|v| v.checked_sub(record.stack_size as usize))
-                    {
-                        Some(sp) => sp,
-                        None => continue,
-                    }
+                    _Unwind_GetCFA(context)
                 } else {
                     _Unwind_GetGR(context, i32::from(location.dwarf_reg))
                 };
@@ -1366,7 +1399,7 @@ mod fp_chain {
         let high_pc = index.max_pc.saturating_add(MAX_SAFEPOINT_RETURN_DELTA);
         let mut fp = current_frame_pointer();
         while fp != 0 {
-            if fp & 0xF != 0 || fp.checked_add(16)? > top {
+            if fp & FRAME_RECORD_ALIGN_MASK != 0 || fp.checked_add(16)? > top {
                 return None;
             }
             let return_address = unsafe { *((fp + 8) as *const usize) };
@@ -1393,7 +1426,7 @@ mod fp_chain {
                         // outside the stack that the collector then reads and
                         // rewrites. Fail closed to the platform unwinder.
                         if caller_fp == 0
-                            || caller_fp & 0xF != 0
+                            || caller_fp & FRAME_RECORD_ALIGN_MASK != 0
                             || caller_fp <= fp
                             || caller_fp.checked_add(16)? > top
                         {
@@ -1461,6 +1494,18 @@ mod fp_chain {
     }
 }
 
+// The contract the Itanium fallback rests on, asserted against a real walk
+// rather than against DWARF's definition of a CFA — the two disagree, and
+// believing the definition was #7392. Its own file because this one is close to
+// the 2000-line cap.
+#[cfg(all(
+    test,
+    any(target_vendor = "apple", target_os = "linux"),
+    any(target_arch = "aarch64", target_arch = "x86_64")
+))]
+#[path = "stack_maps_unwind_contract.rs"]
+mod unwind_contract;
+
 #[cfg(test)]
 mod tests {
     use super::*;