From 061a290dde28c78801d08215641ad4d7b947b269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 22:33:45 +0200 Subject: [PATCH 1/4] fix(gc): walk legal 8-mod-16 frame records, and stop moving unwinder roots a frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent defects in the native-root walkers, both aarch64, both invisible until the ELF arm could finally run a probe (#7392). 1. The x29 chain walk required a 16-byte-aligned frame record. AAPCS64 does not promise that — it fixes the record's contents and leaves its placement in the frame unspecified — and LLVM's AArch64 ELF frame lowering puts the x29,x30 pair below the other callee-saved GPRs, so an odd number of those lands it 8 mod 16. Measured in a real runtime frame (x19..x23 + v8 saved, CFA = x29+56). Darwin pins the record to the top of the frame, which is why this never fired on macOS. The check treated the legal record as corruption and abandoned the walk mid-stack. 2. The unwinder fallback it abandoned into resolved SP-relative roots against `CFA - stack_size`. Inside an `_Unwind_Backtrace` callback the CFA already IS the stack pointer of the frame whose return address `_Unwind_GetIP` reports, so the subtraction placed every such root one whole frame too low. Verified directly on 02_survivor_promotion: at the CFA the slot holds a NaN-boxed pointer (0x7ffd...), one frame below it holds a stack address. Together: on aarch64-Linux the chain walk bailed, the fallback then rewrote the wrong words, the frame's real roots were never updated after an evacuation, and the mutator dereferenced a stale from-space pointer — the SIGSEGV in #7392. The CFA semantics are pinned by a new test that records each frame's real stack pointer and matches it against a live walk; it passes on aarch64 Linux (libgcc), aarch64 macOS (Apple libunwind) and x86-64 Linux, so `CFA_RETURN_ADDRESS_BYTES` had no case left to serve and is deleted. Neither walker was exercised anywhere: nothing in the tree ever set PERRY_STACKMAP_WALKER, though the workflow's knob ledger claimed an arm for it. gc-native-roots now runs every probe under `unwind`, and under `verify` on aarch64 where the fp-chain walk exists, 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. --- .github/workflows/gc-native-roots.yml | 81 ++++++++- .../perry-runtime/src/gc/roots/stack_maps.rs | 100 +++++++---- .../gc/roots/stack_maps_unwind_contract.rs | 158 ++++++++++++++++++ 3 files changed, 304 insertions(+), 35 deletions(-) create mode 100644 crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index ced53f39be..f94705753a 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.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,71 @@ 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; } + # #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 diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index 11d7c97bdb..0a78794b4b 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/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,29 @@ static STACK_MAPS: OnceLock = 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::(); -#[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. +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 +1012,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 +1388,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 +1415,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 +1483,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::*; diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs b/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs new file mode 100644 index 0000000000..3593199b22 --- /dev/null +++ b/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs @@ -0,0 +1,158 @@ +//! What `_Unwind_GetCFA` actually returns, pinned by measurement (#7392). +//! +//! The Itanium fallback walker resolves every SP-relative root against this one +//! value, and the number it hands back is not the number its name suggests. +//! DWARF defines a frame's CFA as the *caller's* stack pointer before the call, +//! which is why `unwind::walk_frame` used to compute `CFA - stack_size` to get +//! back down to the frame's own stack pointer. Measured, that subtraction is +//! wrong: inside an `_Unwind_Backtrace` callback the CFA already IS the stack +//! pointer of the frame whose return address `_Unwind_GetIP` reports, so the +//! subtraction moved every root one whole frame too low — reading, and under +//! evacuation *writing*, words that belong to a dead callee frame. +//! +//! Nothing downstream can catch that: no code knows what a root slot is +//! supposed to contain, so wrong words look exactly like right ones. The bug +//! survived because on aarch64 this walker is only the fallback — the x29 chain +//! normally answers first — and it surfaced on aarch64-Linux only once a legal +//! 8-mod-16 frame record made the chain walk bail into it (#7392). +//! +//! So the contract is asserted here rather than trusted: each frame records its +//! real stack pointer on the way in, the walk records the CFA it is given, and +//! the test requires the second list to contain the first, in order. It runs on +//! every host `cargo test` covers — where the two implementations differ +//! (libgcc on Linux, Apple's libunwind on Darwin) and where the return-address +//! convention differs (x86-64 pushes it, aarch64 does not). + +use std::ffi::c_void; +use std::hint::black_box; + +#[repr(C)] +struct UnwindContext { + _private: [u8; 0], +} + +unsafe extern "C" { + fn _Unwind_Backtrace( + trace: unsafe extern "C" fn(*mut UnwindContext, *mut c_void) -> i32, + argument: *mut c_void, + ) -> i32; + fn _Unwind_GetCFA(context: *mut UnwindContext) -> usize; +} + +fn stack_pointer() -> usize { + let sp: usize; + #[cfg(target_arch = "aarch64")] + unsafe { + std::arch::asm!("mov {sp}, sp", sp = out(reg) sp, options(nomem, nostack)); + } + #[cfg(target_arch = "x86_64")] + unsafe { + std::arch::asm!("mov {sp}, rsp", sp = out(reg) sp, options(nomem, nostack)); + } + sp +} + +unsafe extern "C" fn collect(context: *mut UnwindContext, argument: *mut c_void) -> i32 { + let out = unsafe { &mut *argument.cast::>() }; + out.push(unsafe { _Unwind_GetCFA(context) }); + // Bounded: a runaway walk must fail the assertion, not hang the suite. + i32::from(out.len() > 24) +} + +fn walk_cfas() -> Vec { + let mut cfas: Vec = Vec::new(); + unsafe { + _Unwind_Backtrace(collect, (&mut cfas as *mut Vec).cast::()); + } + cfas +} + +// Three frames deep, each with locals the optimiser cannot fold away, so the +// stack pointers are distinct and the frames cannot be merged into one. +#[inline(never)] +fn innermost(sps: &mut Vec) -> Vec { + let pad = black_box([0u64; 4]); + sps.push(stack_pointer()); + let cfas = walk_cfas(); + black_box(pad); + cfas +} + +#[inline(never)] +fn middle(sps: &mut Vec) -> Vec { + let pad = black_box([0u64; 16]); + sps.push(stack_pointer()); + let cfas = innermost(sps); + black_box(pad); + cfas +} + +#[inline(never)] +fn outermost(sps: &mut Vec) -> Vec { + let pad = black_box([0u64; 32]); + sps.push(stack_pointer()); + let cfas = middle(sps); + black_box(pad); + cfas +} + +/// The CFA a backtrace callback reports is the frame's own stack pointer. +/// +/// Recorded innermost-first, which is the order `_Unwind_Backtrace` visits, and +/// matched as a SUBSEQUENCE: the walk legitimately reports frames these helpers +/// know nothing about (`walk_cfas` itself, the test harness, libc), and adding +/// one of those must not fail the test. What must never happen is a reported +/// CFA that is one frame off from the real stack pointer — the shape of #7392. +#[test] +fn unwind_cfa_is_the_frames_stack_pointer() { + let mut sps: Vec = Vec::new(); + let cfas = outermost(&mut sps); + assert!( + cfas.len() >= sps.len(), + "the walk visited {} frame(s) for {} recorded frames", + cfas.len(), + sps.len() + ); + + // `sps` is outermost-first (each frame pushes on the way in); the walk is + // innermost-first. + let expected: Vec = sps.iter().rev().copied().collect(); + let mut next = 0usize; + for cfa in &cfas { + if next < expected.len() && *cfa == expected[next] { + next += 1; + } + } + assert_eq!( + next, + expected.len(), + "every recorded stack pointer must appear as a reported CFA, in walk \ + order.\n recorded stack pointers (innermost first): {expected:#x?}\n \ + reported CFAs: {cfas:#x?}\nA CFA that is one frame size away from a \ + recorded stack pointer is #7392: `unwind::walk_frame` would then place \ + every SP-relative root in the wrong frame." + ); +} + +/// A frame record is two 64-bit words: eight-byte aligned, not sixteen. +/// +/// 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 pair 8 mod 16 — +/// measured in a real runtime frame in #7392, where the fast walker rejected it +/// as corrupt and bailed into the (then broken) unwinder fallback. +#[test] +fn a_frame_record_needs_only_eight_byte_alignment() { + let mask = super::FRAME_RECORD_ALIGN_MASK; + assert_eq!(0x1000usize & mask, 0, "16-aligned records stay valid"); + assert_eq!( + 0x1008usize & mask, + 0, + "an 8-mod-16 frame record is legal AAPCS64 and must be walked, not \ + rejected — rejecting it is what abandoned the walk in #7392" + ); + assert_ne!( + 0x1004usize & mask, + 0, + "a 4-aligned address is still garbage" + ); +} From 2bd184ae71d528832f87fae637b5729652ab86f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 22:34:52 +0200 Subject: [PATCH 2/4] docs: changelog fragment for #7400 --- .../7400-native-root-walkers-aarch64.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 changelog.d/7400-native-root-walkers-aarch64.md diff --git a/changelog.d/7400-native-root-walkers-aarch64.md b/changelog.d/7400-native-root-walkers-aarch64.md new file mode 100644 index 0000000000..3c32a9a1a4 --- /dev/null +++ b/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. From 610a4424aa44d4361b694bfee3c0b7f0fc2ae6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 22:44:09 +0200 Subject: [PATCH 3/4] test(gc): read the caller's stack pointer at opt-level 0, and silence the mask off aarch64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stack_pointer()` as a function is inlined only once the optimiser is on, so at the `opt-level=0` `cargo test` uses it reported its own frame — every recorded value off by one frame, and the contract test failed for a reason unrelated to the walker. A macro expands at the call site, which removes the question. `FRAME_RECORD_ALIGN_MASK` is read only by the fp-chain walker, which exists on aarch64 Unix alone; carry that exact cfg so an x86-64 or Windows build does not warn on a constant it has no walker for. --- .../perry-runtime/src/gc/roots/stack_maps.rs | 11 +++++ .../gc/roots/stack_maps_unwind_contract.rs | 48 +++++++++++++------ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index 0a78794b4b..215f9588e7 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps.rs @@ -130,6 +130,17 @@ const DWARF_REG_SP_AARCH64: u16 = 31; // 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 diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs b/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs index 3593199b22..db420dd2aa 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps_unwind_contract.rs @@ -39,24 +39,42 @@ unsafe extern "C" { fn _Unwind_GetCFA(context: *mut UnwindContext) -> usize; } -fn stack_pointer() -> usize { - let sp: usize; - #[cfg(target_arch = "aarch64")] - unsafe { - std::arch::asm!("mov {sp}, sp", sp = out(reg) sp, options(nomem, nostack)); - } - #[cfg(target_arch = "x86_64")] - unsafe { - std::arch::asm!("mov {sp}, rsp", sp = out(reg) sp, options(nomem, nostack)); - } - sp +/// The stack pointer of the frame that expands this, which is why it is a macro +/// and not a function: as a function it is only inlined once the optimiser is +/// on, and at `opt-level=0` — the profile `cargo test` uses — it reported its +/// OWN frame instead of its caller's, so every recorded value was wrong by one +/// frame and the test failed for a reason that had nothing to do with the +/// walker. Expanding at the call site takes the question away entirely. +macro_rules! stack_pointer { + () => {{ + let sp: usize; + #[cfg(target_arch = "aarch64")] + unsafe { + std::arch::asm!("mov {sp}, sp", sp = out(reg) sp, options(nomem, nostack)); + } + #[cfg(target_arch = "x86_64")] + unsafe { + std::arch::asm!("mov {sp}, rsp", sp = out(reg) sp, options(nomem, nostack)); + } + sp + }}; } +/// `_URC_NO_REASON`, the only code that continues the walk. +const URC_NO_REASON: i32 = 0; +/// `_URC_END_OF_STACK`. Any non-zero code stops `_Unwind_Backtrace`; this is +/// the one that says "stop, normally". +const URC_END_OF_STACK: i32 = 5; + unsafe extern "C" fn collect(context: *mut UnwindContext, argument: *mut c_void) -> i32 { let out = unsafe { &mut *argument.cast::>() }; out.push(unsafe { _Unwind_GetCFA(context) }); // Bounded: a runaway walk must fail the assertion, not hang the suite. - i32::from(out.len() > 24) + if out.len() > 24 { + URC_END_OF_STACK + } else { + URC_NO_REASON + } } fn walk_cfas() -> Vec { @@ -72,7 +90,7 @@ fn walk_cfas() -> Vec { #[inline(never)] fn innermost(sps: &mut Vec) -> Vec { let pad = black_box([0u64; 4]); - sps.push(stack_pointer()); + sps.push(stack_pointer!()); let cfas = walk_cfas(); black_box(pad); cfas @@ -81,7 +99,7 @@ fn innermost(sps: &mut Vec) -> Vec { #[inline(never)] fn middle(sps: &mut Vec) -> Vec { let pad = black_box([0u64; 16]); - sps.push(stack_pointer()); + sps.push(stack_pointer!()); let cfas = innermost(sps); black_box(pad); cfas @@ -90,7 +108,7 @@ fn middle(sps: &mut Vec) -> Vec { #[inline(never)] fn outermost(sps: &mut Vec) -> Vec { let pad = black_box([0u64; 32]); - sps.push(stack_pointer()); + sps.push(stack_pointer!()); let cfas = middle(sps); black_box(pad); cfas From 337cdb59930470e9915f3da9196c69d62d862b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 22:50:28 +0200 Subject: [PATCH 4/4] ci(gc): assert each non-default walker actually ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oracle-diffing every probe proves a process exited zero and printed the right bytes. It does not prove PERRY_STACKMAP_WALKER selected the walker under test, that the walker reached a mapped frame, or that anything moved — and all three modes are meant to produce identical output, so program output cannot tell them apart. That is the shape of #7392 itself. Per mode, off one traced run of 11_collect_at_depth: fp_walks == 0 proves 'unwind' took effect and > 0 proves 'verify' cross-checked something, --require-locations proves frames were stepped and roots enumerated, and the evacuation assert proves a copying minor moved objects. Measured on aarch64-Linux: unwind fp_walks=0, verify fp_walks=5, both with 663 locations and 6015 objects copied; and the negative control — judging a default-walker trace as if it were 'unwind' — fails, so the assert can. --- .github/workflows/gc-native-roots.yml | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index f94705753a..a0c8bf679b 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.github/workflows/gc-native-roots.yml @@ -453,6 +453,48 @@ jobs: [ "$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