From 5d9aeafb44406f0065b0a901f34278f7641902a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:25:08 +0200 Subject: [PATCH 01/11] diag(gc): make PERRY_STACKMAP_WALKER=verify name the disagreement it finds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first end-to-end `verify` run on aarch64 ELF caught the fp-chain walker and the Itanium unwinder resolving one root 96 bytes apart (#7984). All the gate could say was "1 unique slot versus 1 unique slot" and print the two addresses in decimal — from which the frame, the base register, the function whose prologue was decoded, and therefore *which walker is wrong* are all unrecoverable. Every candidate explanation (a missed trailing `sub sp`, a frame the chain skipped, a CFA one frame out) predicts exactly that output. Both walkers now report a `ResolvedRoot` instead of a bare `MutableRootSlot`: the same address, plus the frame return address it was matched on, the record's function, the map's base register and frame offset, and the base that walker resolved the register to. `verify` prints all of it on a mismatch, calls out an equal-slot-count disagreement as a base disagreement rather than a missed frame, and on aarch64 dumps `fp_to_sp_offset`'s decode and the prologue words it read. The prologue dump is gated on the map vouching for the function address. The first draft was not, and a unit test with a synthetic address turned the diagnostic into a SIGSEGV with no output — which is what would also happen in the field for the failure mode "the map's addresses are wrong". `verify` moves to its own file and the decoder tests to another, because `stack_maps.rs` was eight lines under the 2000-line cap. The workflow's crash path tailed 20 lines of the failing run's stderr, which truncated the report's head. 120. --- .github/workflows/gc-native-roots.yml | 2 +- .../perry-runtime/src/gc/roots/stack_maps.rs | 597 +++--------------- .../src/gc/roots/stack_maps_decode_tests.rs | 470 ++++++++++++++ .../src/gc/roots/stack_maps_verify.rs | 286 +++++++++ 4 files changed, 832 insertions(+), 523 deletions(-) create mode 100644 crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs create mode 100644 crates/perry-runtime/src/gc/roots/stack_maps_verify.rs diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index 08e2fcf788..6a44a1754c 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.github/workflows/gc-native-roots.yml @@ -515,7 +515,7 @@ jobs: "/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; } + tail -120 "/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)) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index 3e6a52877f..2e4e81ddbf 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps.rs @@ -179,6 +179,41 @@ fn walker_mode() -> WalkerMode { }) } +/// One root as a walker resolved it, carrying the provenance that says WHY. +/// +/// The walkers used to hand the collector a bare `MutableRootSlot`, which is +/// all the collector needs and exactly nothing of what a disagreement between +/// two walkers is about. When `PERRY_STACKMAP_WALKER=verify` caught the +/// aarch64-ELF fp-chain walk and the unwinder resolving one root 96 bytes +/// apart (#7984), the panic could say "1 slot versus 1 slot" and print two +/// integers — from which neither the frame, the base register, nor the frame +/// whose base was used could be recovered. Every walker now reports where the +/// address came from, so `verify` names the disagreement instead of posing it. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) struct ResolvedRoot { + /// Address of the slot: `base` displaced by the record's frame offset. + pub(super) address: usize, + /// The frame's return address — what `match_records` was keyed on. + pub(super) ip: usize, + /// Start of the function the matched record belongs to. + pub(super) function_address: usize, + /// The record's base register (29 = FP, 31 = SP on aarch64). + pub(super) dwarf_reg: u16, + /// The record's frame offset from that base. + pub(super) offset: i32, + /// The base the walker resolved that register to for this frame. + pub(super) base: usize, +} + +impl ResolvedRoot { + fn slot(self) -> MutableRootSlot { + MutableRootSlot { + kind: MutableRootSlotKind::NativeStack, + ptr: self.address as *mut u64, + } + } +} + #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub(in crate::gc) struct NativeStackWalkStats { pub(in crate::gc) walks: usize, @@ -461,61 +496,23 @@ pub(super) fn visit_stack_map_root_slots( return NativeStackWalkStats::default(); } match walker_mode() { - WalkerMode::Unwind => unwind::visit(index, visit), + WalkerMode::Unwind => unwind::visit(index, &mut |root: ResolvedRoot| visit(root.slot())), WalkerMode::Fast => { if index.chain_walkable { - if let Some(stats) = fp_chain::visit(index, visit) { + if let Some(stats) = + fp_chain::visit(index, &mut |root: ResolvedRoot| visit(root.slot())) + { return stats; } } - let mut stats = unwind::visit(index, visit); + let mut stats = unwind::visit(index, &mut |root: ResolvedRoot| visit(root.slot())); stats.fallback_walks = 1; stats } - WalkerMode::Verify => verify_visit(index, visit), + WalkerMode::Verify => verify::visit(index, visit), } } -/// Debug-only cross-check: the fast walk reads slot addresses without -/// mutating, then the unwinder performs the real visitation while recording -/// what it reached. Any set difference is a missed or invented frame and -/// panics immediately — this is the liveness gate for the fast walker itself. -fn verify_visit( - index: &StackMapIndex, - visit: &mut impl FnMut(MutableRootSlot), -) -> NativeStackWalkStats { - let mut fast_addresses: Vec = Vec::new(); - let fast_stats = fp_chain::visit(index, &mut |slot: MutableRootSlot| { - fast_addresses.push(slot.ptr as usize); - }); - let Some(fast_stats) = fast_stats else { - panic!( - "PERRY_STACKMAP_WALKER=verify: fast walk unavailable \ - (chain_walkable={}, anomaly or unsupported target)", - index.chain_walkable - ); - }; - let mut unwind_addresses: Vec = Vec::new(); - let mut stats = unwind::visit(index, &mut |slot: MutableRootSlot| { - unwind_addresses.push(slot.ptr as usize); - visit(slot); - }); - fast_addresses.sort_unstable(); - fast_addresses.dedup(); - unwind_addresses.sort_unstable(); - unwind_addresses.dedup(); - assert_eq!( - fast_addresses, - unwind_addresses, - "PERRY_STACKMAP_WALKER=verify: fast walk visited {} unique slots, \ - unwinder visited {}", - fast_addresses.len(), - unwind_addresses.len() - ); - stats.fp_walks = fast_stats.fp_walks; - stats -} - /// Decode every concatenated compact map in the section. /// /// The linker concatenates one blob per object file, so this walks blob by @@ -1009,7 +1006,7 @@ mod unwind { stats: NativeStackWalkStats, } - pub(super) fn visit( + pub(super) fn visit( index: &StackMapIndex, visit: &mut F, ) -> NativeStackWalkStats { @@ -1030,7 +1027,7 @@ mod unwind { state.stats } - unsafe extern "C" fn walk_frame( + unsafe extern "C" fn walk_frame( context: *mut UnwindContext, argument: *mut c_void, ) -> i32 { @@ -1084,9 +1081,13 @@ mod unwind { if address == 0 || address & (std::mem::align_of::() - 1) != 0 { continue; } - (state.visit)(MutableRootSlot { - kind: MutableRootSlotKind::NativeStack, - ptr: address as *mut u64, + (state.visit)(ResolvedRoot { + address, + ip, + function_address: record.function_address, + dwarf_reg: location.dwarf_reg, + offset: location.offset, + base, }); } } @@ -1216,7 +1217,7 @@ mod unwind { (low != 0 && low < high).then_some((low, high)) } - pub(super) fn visit( + pub(super) fn visit( index: &StackMapIndex, visit: &mut F, ) -> NativeStackWalkStats { @@ -1257,9 +1258,13 @@ mod unwind { { return stats; } - visit(MutableRootSlot { - kind: MutableRootSlotKind::NativeStack, - ptr: address as *mut u64, + visit(ResolvedRoot { + address, + ip: context.rip as usize, + function_address: record.function_address, + dwarf_reg: location.dwarf_reg, + offset: location.offset, + base, }); } } @@ -1320,7 +1325,7 @@ mod unwind { pub(super) fn visit( _index: &StackMapIndex, - _visit: &mut impl FnMut(MutableRootSlot), + _visit: &mut impl FnMut(ResolvedRoot), ) -> NativeStackWalkStats { NativeStackWalkStats::default() } @@ -1401,7 +1406,7 @@ mod fp_chain { (addr as usize).saturating_add(size) } - pub(super) fn visit( + pub(super) fn visit( index: &StackMapIndex, visit: &mut F, ) -> Option { @@ -1483,9 +1488,13 @@ mod fp_chain { { continue; } - visit(MutableRootSlot { - kind: MutableRootSlotKind::NativeStack, - ptr: address as *mut u64, + visit(ResolvedRoot { + address, + ip: return_address, + function_address: record.function_address, + dwarf_reg: location.dwarf_reg, + offset: location.offset, + base, }); } } @@ -1510,12 +1519,17 @@ mod fp_chain { pub(super) fn visit( _index: &StackMapIndex, - _visit: &mut impl FnMut(MutableRootSlot), + _visit: &mut impl FnMut(ResolvedRoot), ) -> Option { None } } +// `verify` mode, and the report it prints when the two walkers disagree. Its +// own file because this one is close to the 2000-line cap. +#[path = "stack_maps_verify.rs"] +mod verify; + // 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 @@ -1529,466 +1543,5 @@ mod fp_chain { mod unwind_contract; #[cfg(test)] -mod tests { - use super::*; - - fn push_varint(out: &mut Vec, mut value: u64) { - while value >= 0x80 { - out.push((value as u8 & 0x7F) | 0x80); - value >>= 7; - } - out.push(value as u8); - } - - fn zigzag(value: i32) -> u64 { - ((value << 1) ^ (value >> 31)) as u32 as u64 - } - - /// Build one compact blob, mirroring `perry-codegen/src/gc_map.rs`. - /// `records` is `(instruction_offset, roots)`, roots as `(dwarf_reg, offset)`; - /// an empty root slice with `repeat` set encodes the repeat flag. - fn one_map(function: u64, records: &[(u32, Vec<(u16, i32)>, bool)]) -> Vec { - let mut offsets = Vec::new(); - let mut stream = Vec::new(); - for (instruction_offset, roots, repeat) in records { - offsets.extend_from_slice(&instruction_offset.to_le_bytes()); - if *repeat { - push_varint(&mut stream, 1); - continue; - } - push_varint(&mut stream, (roots.len() as u64) << 1); - let mut last: Option = None; - for (reg, offset) in roots { - let tag = match *reg { - DWARF_REG_FP_AARCH64 => 0u64, - DWARF_REG_SP_AARCH64 => 1, - _ => 2, - }; - let delta = match last { - None => *offset, - Some(previous) => offset.wrapping_sub(previous), - }; - push_varint(&mut stream, (zigzag(delta) << 2) | tag); - if tag == 2 { - push_varint(&mut stream, u64::from(*reg)); - } - last = Some(*offset); - } - } - - // Build for THIS host's pointer width, mirroring the emitter: the - // decoder rejects a blob whose recorded width disagrees with its own. - let ptr64 = std::mem::size_of::() == 8; - let entry = if ptr64 { 16 } else { 12 }; - let total_len = 16 + entry + offsets.len() + stream.len(); - let mut bytes = Vec::new(); - bytes.extend_from_slice(GC_MAP_MAGIC); - bytes.push(GC_MAP_VERSION); - bytes.push(0); - bytes.extend_from_slice(&u16::from(ptr64).to_le_bytes()); - bytes.extend_from_slice(&1u32.to_le_bytes()); - bytes.extend_from_slice(&(total_len as u32).to_le_bytes()); - if ptr64 { - bytes.extend_from_slice(&function.to_le_bytes()); - } else { - bytes.extend_from_slice(&(function as u32).to_le_bytes()); - } - bytes.extend_from_slice(&32u32.to_le_bytes()); - bytes.extend_from_slice(&(records.len() as u32).to_le_bytes()); - bytes.extend_from_slice(&offsets); - bytes.extend_from_slice(&stream); - while bytes.len() % 8 != 0 { - bytes.push(0); - } - bytes - } - - fn simple(function: u64, offset: u32, frame_offset: i32) -> Vec { - one_map(function, &[(offset, vec![(29, frame_offset)], false)]) - } - - #[test] - fn decodes_frame_location() { - let bytes = simple(0x1000, 0x10, -8); - let (records, roots) = parse_gc_map(&bytes).expect("valid map"); - assert_eq!(records.len(), 1); - assert_eq!(records[0].pc, 0x1010); - assert_eq!(records[0].function_address, 0x1000); - assert_eq!(records[0].stack_size, 32); - assert_eq!( - roots, - vec![StackMapLocation { - dwarf_reg: 29, - offset: -8, - }] - ); - } - - #[test] - fn decodes_linker_concatenated_input_sections() { - let mut bytes = simple(0x1000, 0x10, -8); - bytes.extend_from_slice(&simple(0x2000, 0x20, -16)); - let (records, _) = parse_gc_map(&bytes).expect("concatenated maps"); - assert_eq!(records.len(), 2); - assert_eq!(records[0].pc, 0x1010); - assert_eq!(records[1].pc, 0x2020); - } - - #[test] - fn repeated_live_sets_share_one_copy() { - // Three safepoints, the last two repeating the first's live set: the - // whole point of the format, and the reason the in-memory index does - // not hold 154k duplicated entries on a real application. - let bytes = one_map( - 0x1000, - &[ - (0x10, vec![(29, -8), (29, -16)], false), - (0x20, vec![], true), - (0x30, vec![], true), - ], - ); - let (records, roots) = parse_gc_map(&bytes).expect("valid map"); - assert_eq!(records.len(), 3); - assert_eq!(roots.len(), 2, "the repeats must not append new roots"); - for record in &records { - assert_eq!(record.roots_start, 0); - assert_eq!(record.roots_len, 2); - } - } - - #[test] - fn decodes_negative_and_ascending_root_offsets() { - let bytes = one_map(0x1000, &[(0, vec![(29, -64), (29, -8), (31, 24)], false)]); - let (_, roots) = parse_gc_map(&bytes).expect("valid map"); - assert_eq!( - roots, - vec![ - StackMapLocation { - dwarf_reg: 29, - offset: -64 - }, - StackMapLocation { - dwarf_reg: 29, - offset: -8 - }, - StackMapLocation { - dwarf_reg: 31, - offset: 24 - }, - ] - ); - } - - #[test] - fn decodes_an_explicit_base_register() { - // LLVM uses x19 as a frame base pointer in functions with dynamic - // stack allocation — 66 root slots in one real module. A single FP/SP - // bit cannot express that, which is what forced the 2-bit base tag. - let bytes = one_map(0x1000, &[(0x10, vec![(19, -40), (29, -8)], false)]); - let (_, roots) = parse_gc_map(&bytes).expect("valid map"); - assert_eq!( - roots, - vec![ - StackMapLocation { - dwarf_reg: 19, - offset: -40 - }, - StackMapLocation { - dwarf_reg: 29, - offset: -8 - }, - ] - ); - } - - #[test] - fn an_explicit_base_register_disables_the_fast_walk() { - // The x29-chain walker can only recover FP and SP; anything else must - // fall back to the platform unwinder, which can. - let index = index_records( - vec![StackMapRecord { - pc: 0x1000, - function_address: 0x1000, - stack_size: 64, - roots_start: 0, - roots_len: 1, - }], - vec![StackMapLocation { - dwarf_reg: 19, - offset: -40, - }], - ); - assert!(!index.chain_walkable); - } - - #[test] - fn rejects_a_blob_built_for_the_other_pointer_width() { - // The header records the width the emitter used. A blob claiming the - // other width would have every function address misread, so it must be - // refused rather than decoded — watchOS `arm64_32` is ILP32 while every - // other supported target is LP64. - let mut bytes = simple(0x1000, 0x10, -8); - let flags = u16::from_le_bytes([bytes[6], bytes[7]]); - bytes[6..8].copy_from_slice(&(flags ^ 1).to_le_bytes()); - assert!( - parse_gc_map(&bytes).is_none(), - "a map built for the other pointer width must be refused" - ); - } - - #[test] - fn rejects_a_blob_whose_length_cannot_advance_the_cursor() { - // `total_len` comes straight from the header. A zero (or too-small) - // value leaves `base` where it was, and because the magic still - // matches there the resync path never runs — the loop spins forever - // inside `OnceLock::get_or_init`, hanging the process at the first - // collection instead of failing closed. - let mut bytes = simple(0x1000, 0x10, -8); - bytes[12..16].copy_from_slice(&0u32.to_le_bytes()); - assert!( - parse_gc_map(&bytes).is_none(), - "a blob that cannot advance the cursor must be rejected, not looped on" - ); - - // Long enough to look plausible, still short of header + function table. - let mut bytes = simple(0x1000, 0x10, -8); - bytes[12..16].copy_from_slice(&20u32.to_le_bytes()); - assert!(parse_gc_map(&bytes).is_none()); - } - - #[test] - fn rejects_a_truncated_function_table() { - // The record counts size the fixed-width offset array; a short read - // there must not be rounded down to zero, or every later varint is - // decoded from the wrong offset. - let bytes = simple(0x1000, 0x10, -8); - let truncated = &bytes[..20]; - assert!(parse_gc_map(truncated).is_none()); - } - - #[test] - fn rejects_truncated_or_wrong_version_sections() { - assert!(parse_gc_map(&[]).is_none() || parse_gc_map(&[]).unwrap().0.is_empty()); - let mut bytes = simple(0x1000, 0x10, -8); - bytes[4] = GC_MAP_VERSION + 1; - assert!( - parse_gc_map(&bytes).is_none(), - "an unknown version must not be guessed at" - ); - // A total_len that runs past the section must fail rather than read on. - let mut bytes = simple(0x1000, 0x10, -8); - let len = bytes.len(); - bytes[12..16].copy_from_slice(&((len as u32) + 64).to_le_bytes()); - assert!(parse_gc_map(&bytes).is_none()); - } - - #[test] - fn chain_walkable_index_accepts_fp_and_sp_locations_only() { - let rec = |pc: usize| StackMapRecord { - pc, - function_address: pc, - stack_size: 160, - roots_start: 0, - roots_len: 1, - }; - // FP and SP are both walkable: SP resolves per frame by decoding the - // owning function's prologue (#7173). - let walkable = index_records( - vec![rec(0x1000), rec(0x2000)], - vec![ - StackMapLocation { - dwarf_reg: DWARF_REG_FP_AARCH64, - offset: -8, - }, - StackMapLocation { - dwarf_reg: DWARF_REG_SP_AARCH64, - offset: -8, - }, - ], - ); - assert!(walkable.chain_walkable); - assert_eq!(walkable.min_pc, 0x1000); - assert_eq!(walkable.max_pc, 0x2000); - // Any other register disqualifies the whole image. - assert!( - !index_records( - vec![rec(0x1000)], - vec![StackMapLocation { - dwarf_reg: 1, - offset: -8 - }], - ) - .chain_walkable, - "a non-FP/SP register must disable the fast walk" - ); - } - - #[test] - fn rejects_a_record_from_an_adjacent_function() { - // A safepoint at the end of A must not be matched for an `ip` early in - // B just because it falls inside the +-16 window: the walker would use - // A's frame offsets against B's frame. - let index = index_records( - vec![ - StackMapRecord { - pc: 0x1ffc, - function_address: 0x1000, - stack_size: 32, - roots_start: 0, - roots_len: 1, - }, - StackMapRecord { - pc: 0x2040, - function_address: 0x2000, - stack_size: 32, - roots_start: 0, - roots_len: 1, - }, - ], - vec![StackMapLocation { - dwarf_reg: 29, - offset: -8, - }], - ); - // 0x2004 is 8 bytes past A's last safepoint but lives in B. - assert!( - index.match_records(0x2004).is_empty(), - "a record from the previous function must not match" - ); - // A same-function near-match is still accepted — requiring an exact pc - // would drop it, and the measured suite has one. - assert_eq!(index.match_records(0x2038).len(), 1); - } - - #[test] - fn matches_plain_maps_before_and_statepoints_after_unwinder_ips() { - let rec = |pc: usize| StackMapRecord { - pc, - function_address: pc, - stack_size: 32, - roots_start: 0, - roots_len: 0, - }; - let maps = vec![rec(0x1000), rec(0x1020)]; - assert_eq!(closest_record_pc(&maps, 0x1004), Some(0x1000)); - assert_eq!(closest_record_pc(&maps, 0x101c), Some(0x1020)); - assert_eq!(closest_record_pc(&maps, 0x1020), Some(0x1020)); - } -} - -#[cfg(all(test, target_arch = "aarch64"))] -mod fp_offset_trailing_sub_tests { - use super::fp_to_sp_offset; - - /// Assemble a prologue into executable-ish memory and decode it. The - /// decoder only reads words, so a plain aligned buffer is enough. - fn decode(words: &[u32]) -> Option { - let buf = words.to_vec().into_boxed_slice(); - let addr = buf.as_ptr() as usize; - let out = fp_to_sp_offset(addr); - drop(buf); - out - } - - const ADD_X29_SP_0X90: u32 = 0x9102_43FD; // add x29, sp, #0x90 - const SUB_SP_SP_0X170: u32 = 0xD105_C3FF; // sub sp, sp, #0x170 - const RET: u32 = 0xD65F_03C0; - const NOP: u32 = 0xD503_201F; - - // The three prologue words #7394 was measured on, read out of - // `perry_fn_test_gap_gc_call_argument_rooting_ts__run` at +0x20: - // - // 9101c3fd add x29, sp, #0x70 - // d14007ff sub sp, sp, #0x1, lsl #12 - // d12103ff sub sp, sp, #0x840 - const ADD_X29_SP_0X70: u32 = 0x9101_C3FD; - const SUB_SP_SP_1_LSL12: u32 = 0xD140_07FF; - const SUB_SP_SP_0X840: u32 = 0xD121_03FF; - const ADD_X29_SP_2_LSL12: u32 = 0x9140_0BFD; // add x29, sp, #0x2, lsl #12 - - /// #7328: `add x29, sp, #imm` is not always the last stack adjustment. - /// LLVM emits a further `sub sp, sp, #N` after establishing the frame - /// pointer, and reading only the `add` left the fast walker N bytes high - /// on every slot in that frame — a silent wrong answer, since the walker - /// then enumerated addresses the collector treated as roots. - #[test] - fn a_sub_after_the_fp_setup_is_included() { - assert_eq!( - decode(&[ADD_X29_SP_0X90, SUB_SP_SP_0X170, NOP, RET]), - Some(0x90 + 0x170), - "the trailing `sub sp, sp, #0x170` must be added to the fp offset" - ); - } - - /// The common shape — fp established last — must be unchanged. - #[test] - fn a_prologue_with_no_trailing_sub_is_unchanged() { - assert_eq!(decode(&[ADD_X29_SP_0X90, NOP, RET]), Some(0x90)); - } - - /// Only a contiguous run of `sub sp` immediately after the `add` counts. - /// A later `sub sp` is a body operation (dynamic alloca, call-argument - /// area) already accounted for by the stack map's own slot offsets. - #[test] - fn a_sub_after_the_prologue_run_is_not_counted() { - assert_eq!( - decode(&[ADD_X29_SP_0X90, NOP, SUB_SP_SP_0X170, RET]), - Some(0x90), - "a `sub sp` separated from the prologue run must not be folded in" - ); - } - - /// A leaf that never sets up fp still fails closed, so the caller falls - /// back to the platform unwinder rather than inventing an offset. - #[test] - fn a_leaf_without_fp_setup_still_fails_closed() { - assert_eq!(decode(&[NOP, RET]), None); - } - - /// #7394: a trailing `sub sp, sp, #imm, lsl #12` must contribute - /// `imm << 12`. #7328's decoder masked the `sh` bit into the opcode - /// comparison, so a shifted `sub` did not match at all. - #[test] - fn a_shifted_trailing_sub_is_included() { - assert_eq!( - decode(&[ADD_X29_SP_0X70, SUB_SP_SP_1_LSL12, NOP, RET]), - Some(0x70 + 0x1000), - "`sub sp, sp, #0x1, lsl #12` must contribute 4096, not 1" - ); - } - - /// The measured shape. The shifted `sub` is not the last one, so failing - /// to match it also **ended the accumulation run** and dropped the - /// `sub sp, sp, #0x840` behind it: the decoder reported 0x70 for a frame - /// whose body SP is 0x18B0 below the frame pointer, and the walker - /// enumerated — and the collector wrote through — addresses 6208 bytes - /// off. That is CLAUDE.md's fourth gate-failure mode: a live walker - /// visiting the wrong stack. - #[test] - fn a_shifted_sub_does_not_end_the_accumulation_run() { - assert_eq!( - decode(&[ - ADD_X29_SP_0X70, - SUB_SP_SP_1_LSL12, - SUB_SP_SP_0X840, - NOP, - RET - ]), - Some(0x70 + 0x1000 + 0x840), - "every `sub sp` in the contiguous prologue run must be folded in" - ); - } - - /// The `sh` bit is decoded on the `add` that establishes the frame - /// pointer too — the same masking bug applied there, where it would have - /// made the decoder skip the fp setup entirely and report a later - /// instruction's offset (or `None`). - #[test] - fn a_shifted_fp_setup_is_decoded() { - assert_eq!( - decode(&[ADD_X29_SP_2_LSL12, NOP, RET]), - Some(0x2000), - "`add x29, sp, #0x2, lsl #12` establishes fp 8192 above sp" - ); - } -} +#[path = "stack_maps_decode_tests.rs"] +mod decode_tests; diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs b/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs new file mode 100644 index 0000000000..62fb90f403 --- /dev/null +++ b/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs @@ -0,0 +1,470 @@ +//! Decoder and record-matching tests for `stack_maps.rs`. +//! +//! Its own file for the same reason `stack_maps_unwind_contract.rs` is: the +//! parent is at the 2000-line cap, and a test that cannot be added without +//! deleting production commentary is a test that does not get added. + +#[cfg(test)] +mod tests { + use super::super::*; + + fn push_varint(out: &mut Vec, mut value: u64) { + while value >= 0x80 { + out.push((value as u8 & 0x7F) | 0x80); + value >>= 7; + } + out.push(value as u8); + } + + fn zigzag(value: i32) -> u64 { + ((value << 1) ^ (value >> 31)) as u32 as u64 + } + + /// Build one compact blob, mirroring `perry-codegen/src/gc_map.rs`. + /// `records` is `(instruction_offset, roots)`, roots as `(dwarf_reg, offset)`; + /// an empty root slice with `repeat` set encodes the repeat flag. + fn one_map(function: u64, records: &[(u32, Vec<(u16, i32)>, bool)]) -> Vec { + let mut offsets = Vec::new(); + let mut stream = Vec::new(); + for (instruction_offset, roots, repeat) in records { + offsets.extend_from_slice(&instruction_offset.to_le_bytes()); + if *repeat { + push_varint(&mut stream, 1); + continue; + } + push_varint(&mut stream, (roots.len() as u64) << 1); + let mut last: Option = None; + for (reg, offset) in roots { + let tag = match *reg { + DWARF_REG_FP_AARCH64 => 0u64, + DWARF_REG_SP_AARCH64 => 1, + _ => 2, + }; + let delta = match last { + None => *offset, + Some(previous) => offset.wrapping_sub(previous), + }; + push_varint(&mut stream, (zigzag(delta) << 2) | tag); + if tag == 2 { + push_varint(&mut stream, u64::from(*reg)); + } + last = Some(*offset); + } + } + + // Build for THIS host's pointer width, mirroring the emitter: the + // decoder rejects a blob whose recorded width disagrees with its own. + let ptr64 = std::mem::size_of::() == 8; + let entry = if ptr64 { 16 } else { 12 }; + let total_len = 16 + entry + offsets.len() + stream.len(); + let mut bytes = Vec::new(); + bytes.extend_from_slice(GC_MAP_MAGIC); + bytes.push(GC_MAP_VERSION); + bytes.push(0); + bytes.extend_from_slice(&u16::from(ptr64).to_le_bytes()); + bytes.extend_from_slice(&1u32.to_le_bytes()); + bytes.extend_from_slice(&(total_len as u32).to_le_bytes()); + if ptr64 { + bytes.extend_from_slice(&function.to_le_bytes()); + } else { + bytes.extend_from_slice(&(function as u32).to_le_bytes()); + } + bytes.extend_from_slice(&32u32.to_le_bytes()); + bytes.extend_from_slice(&(records.len() as u32).to_le_bytes()); + bytes.extend_from_slice(&offsets); + bytes.extend_from_slice(&stream); + while bytes.len() % 8 != 0 { + bytes.push(0); + } + bytes + } + + fn simple(function: u64, offset: u32, frame_offset: i32) -> Vec { + one_map(function, &[(offset, vec![(29, frame_offset)], false)]) + } + + #[test] + fn decodes_frame_location() { + let bytes = simple(0x1000, 0x10, -8); + let (records, roots) = parse_gc_map(&bytes).expect("valid map"); + assert_eq!(records.len(), 1); + assert_eq!(records[0].pc, 0x1010); + assert_eq!(records[0].function_address, 0x1000); + assert_eq!(records[0].stack_size, 32); + assert_eq!( + roots, + vec![StackMapLocation { + dwarf_reg: 29, + offset: -8, + }] + ); + } + + #[test] + fn decodes_linker_concatenated_input_sections() { + let mut bytes = simple(0x1000, 0x10, -8); + bytes.extend_from_slice(&simple(0x2000, 0x20, -16)); + let (records, _) = parse_gc_map(&bytes).expect("concatenated maps"); + assert_eq!(records.len(), 2); + assert_eq!(records[0].pc, 0x1010); + assert_eq!(records[1].pc, 0x2020); + } + + #[test] + fn repeated_live_sets_share_one_copy() { + // Three safepoints, the last two repeating the first's live set: the + // whole point of the format, and the reason the in-memory index does + // not hold 154k duplicated entries on a real application. + let bytes = one_map( + 0x1000, + &[ + (0x10, vec![(29, -8), (29, -16)], false), + (0x20, vec![], true), + (0x30, vec![], true), + ], + ); + let (records, roots) = parse_gc_map(&bytes).expect("valid map"); + assert_eq!(records.len(), 3); + assert_eq!(roots.len(), 2, "the repeats must not append new roots"); + for record in &records { + assert_eq!(record.roots_start, 0); + assert_eq!(record.roots_len, 2); + } + } + + #[test] + fn decodes_negative_and_ascending_root_offsets() { + let bytes = one_map(0x1000, &[(0, vec![(29, -64), (29, -8), (31, 24)], false)]); + let (_, roots) = parse_gc_map(&bytes).expect("valid map"); + assert_eq!( + roots, + vec![ + StackMapLocation { + dwarf_reg: 29, + offset: -64 + }, + StackMapLocation { + dwarf_reg: 29, + offset: -8 + }, + StackMapLocation { + dwarf_reg: 31, + offset: 24 + }, + ] + ); + } + + #[test] + fn decodes_an_explicit_base_register() { + // LLVM uses x19 as a frame base pointer in functions with dynamic + // stack allocation — 66 root slots in one real module. A single FP/SP + // bit cannot express that, which is what forced the 2-bit base tag. + let bytes = one_map(0x1000, &[(0x10, vec![(19, -40), (29, -8)], false)]); + let (_, roots) = parse_gc_map(&bytes).expect("valid map"); + assert_eq!( + roots, + vec![ + StackMapLocation { + dwarf_reg: 19, + offset: -40 + }, + StackMapLocation { + dwarf_reg: 29, + offset: -8 + }, + ] + ); + } + + #[test] + fn an_explicit_base_register_disables_the_fast_walk() { + // The x29-chain walker can only recover FP and SP; anything else must + // fall back to the platform unwinder, which can. + let index = index_records( + vec![StackMapRecord { + pc: 0x1000, + function_address: 0x1000, + stack_size: 64, + roots_start: 0, + roots_len: 1, + }], + vec![StackMapLocation { + dwarf_reg: 19, + offset: -40, + }], + ); + assert!(!index.chain_walkable); + } + + #[test] + fn rejects_a_blob_built_for_the_other_pointer_width() { + // The header records the width the emitter used. A blob claiming the + // other width would have every function address misread, so it must be + // refused rather than decoded — watchOS `arm64_32` is ILP32 while every + // other supported target is LP64. + let mut bytes = simple(0x1000, 0x10, -8); + let flags = u16::from_le_bytes([bytes[6], bytes[7]]); + bytes[6..8].copy_from_slice(&(flags ^ 1).to_le_bytes()); + assert!( + parse_gc_map(&bytes).is_none(), + "a map built for the other pointer width must be refused" + ); + } + + #[test] + fn rejects_a_blob_whose_length_cannot_advance_the_cursor() { + // `total_len` comes straight from the header. A zero (or too-small) + // value leaves `base` where it was, and because the magic still + // matches there the resync path never runs — the loop spins forever + // inside `OnceLock::get_or_init`, hanging the process at the first + // collection instead of failing closed. + let mut bytes = simple(0x1000, 0x10, -8); + bytes[12..16].copy_from_slice(&0u32.to_le_bytes()); + assert!( + parse_gc_map(&bytes).is_none(), + "a blob that cannot advance the cursor must be rejected, not looped on" + ); + + // Long enough to look plausible, still short of header + function table. + let mut bytes = simple(0x1000, 0x10, -8); + bytes[12..16].copy_from_slice(&20u32.to_le_bytes()); + assert!(parse_gc_map(&bytes).is_none()); + } + + #[test] + fn rejects_a_truncated_function_table() { + // The record counts size the fixed-width offset array; a short read + // there must not be rounded down to zero, or every later varint is + // decoded from the wrong offset. + let bytes = simple(0x1000, 0x10, -8); + let truncated = &bytes[..20]; + assert!(parse_gc_map(truncated).is_none()); + } + + #[test] + fn rejects_truncated_or_wrong_version_sections() { + assert!(parse_gc_map(&[]).is_none() || parse_gc_map(&[]).unwrap().0.is_empty()); + let mut bytes = simple(0x1000, 0x10, -8); + bytes[4] = GC_MAP_VERSION + 1; + assert!( + parse_gc_map(&bytes).is_none(), + "an unknown version must not be guessed at" + ); + // A total_len that runs past the section must fail rather than read on. + let mut bytes = simple(0x1000, 0x10, -8); + let len = bytes.len(); + bytes[12..16].copy_from_slice(&((len as u32) + 64).to_le_bytes()); + assert!(parse_gc_map(&bytes).is_none()); + } + + #[test] + fn chain_walkable_index_accepts_fp_and_sp_locations_only() { + let rec = |pc: usize| StackMapRecord { + pc, + function_address: pc, + stack_size: 160, + roots_start: 0, + roots_len: 1, + }; + // FP and SP are both walkable: SP resolves per frame by decoding the + // owning function's prologue (#7173). + let walkable = index_records( + vec![rec(0x1000), rec(0x2000)], + vec![ + StackMapLocation { + dwarf_reg: DWARF_REG_FP_AARCH64, + offset: -8, + }, + StackMapLocation { + dwarf_reg: DWARF_REG_SP_AARCH64, + offset: -8, + }, + ], + ); + assert!(walkable.chain_walkable); + assert_eq!(walkable.min_pc, 0x1000); + assert_eq!(walkable.max_pc, 0x2000); + // Any other register disqualifies the whole image. + assert!( + !index_records( + vec![rec(0x1000)], + vec![StackMapLocation { + dwarf_reg: 1, + offset: -8 + }], + ) + .chain_walkable, + "a non-FP/SP register must disable the fast walk" + ); + } + + #[test] + fn rejects_a_record_from_an_adjacent_function() { + // A safepoint at the end of A must not be matched for an `ip` early in + // B just because it falls inside the +-16 window: the walker would use + // A's frame offsets against B's frame. + let index = index_records( + vec![ + StackMapRecord { + pc: 0x1ffc, + function_address: 0x1000, + stack_size: 32, + roots_start: 0, + roots_len: 1, + }, + StackMapRecord { + pc: 0x2040, + function_address: 0x2000, + stack_size: 32, + roots_start: 0, + roots_len: 1, + }, + ], + vec![StackMapLocation { + dwarf_reg: 29, + offset: -8, + }], + ); + // 0x2004 is 8 bytes past A's last safepoint but lives in B. + assert!( + index.match_records(0x2004).is_empty(), + "a record from the previous function must not match" + ); + // A same-function near-match is still accepted — requiring an exact pc + // would drop it, and the measured suite has one. + assert_eq!(index.match_records(0x2038).len(), 1); + } + + #[test] + fn matches_plain_maps_before_and_statepoints_after_unwinder_ips() { + let rec = |pc: usize| StackMapRecord { + pc, + function_address: pc, + stack_size: 32, + roots_start: 0, + roots_len: 0, + }; + let maps = vec![rec(0x1000), rec(0x1020)]; + assert_eq!(closest_record_pc(&maps, 0x1004), Some(0x1000)); + assert_eq!(closest_record_pc(&maps, 0x101c), Some(0x1020)); + assert_eq!(closest_record_pc(&maps, 0x1020), Some(0x1020)); + } +} + +#[cfg(all(test, target_arch = "aarch64"))] +mod fp_offset_trailing_sub_tests { + use super::super::fp_to_sp_offset; + + /// Assemble a prologue into executable-ish memory and decode it. The + /// decoder only reads words, so a plain aligned buffer is enough. + fn decode(words: &[u32]) -> Option { + let buf = words.to_vec().into_boxed_slice(); + let addr = buf.as_ptr() as usize; + let out = fp_to_sp_offset(addr); + drop(buf); + out + } + + const ADD_X29_SP_0X90: u32 = 0x9102_43FD; // add x29, sp, #0x90 + const SUB_SP_SP_0X170: u32 = 0xD105_C3FF; // sub sp, sp, #0x170 + const RET: u32 = 0xD65F_03C0; + const NOP: u32 = 0xD503_201F; + + // The three prologue words #7394 was measured on, read out of + // `perry_fn_test_gap_gc_call_argument_rooting_ts__run` at +0x20: + // + // 9101c3fd add x29, sp, #0x70 + // d14007ff sub sp, sp, #0x1, lsl #12 + // d12103ff sub sp, sp, #0x840 + const ADD_X29_SP_0X70: u32 = 0x9101_C3FD; + const SUB_SP_SP_1_LSL12: u32 = 0xD140_07FF; + const SUB_SP_SP_0X840: u32 = 0xD121_03FF; + const ADD_X29_SP_2_LSL12: u32 = 0x9140_0BFD; // add x29, sp, #0x2, lsl #12 + + /// #7328: `add x29, sp, #imm` is not always the last stack adjustment. + /// LLVM emits a further `sub sp, sp, #N` after establishing the frame + /// pointer, and reading only the `add` left the fast walker N bytes high + /// on every slot in that frame — a silent wrong answer, since the walker + /// then enumerated addresses the collector treated as roots. + #[test] + fn a_sub_after_the_fp_setup_is_included() { + assert_eq!( + decode(&[ADD_X29_SP_0X90, SUB_SP_SP_0X170, NOP, RET]), + Some(0x90 + 0x170), + "the trailing `sub sp, sp, #0x170` must be added to the fp offset" + ); + } + + /// The common shape — fp established last — must be unchanged. + #[test] + fn a_prologue_with_no_trailing_sub_is_unchanged() { + assert_eq!(decode(&[ADD_X29_SP_0X90, NOP, RET]), Some(0x90)); + } + + /// Only a contiguous run of `sub sp` immediately after the `add` counts. + /// A later `sub sp` is a body operation (dynamic alloca, call-argument + /// area) already accounted for by the stack map's own slot offsets. + #[test] + fn a_sub_after_the_prologue_run_is_not_counted() { + assert_eq!( + decode(&[ADD_X29_SP_0X90, NOP, SUB_SP_SP_0X170, RET]), + Some(0x90), + "a `sub sp` separated from the prologue run must not be folded in" + ); + } + + /// A leaf that never sets up fp still fails closed, so the caller falls + /// back to the platform unwinder rather than inventing an offset. + #[test] + fn a_leaf_without_fp_setup_still_fails_closed() { + assert_eq!(decode(&[NOP, RET]), None); + } + + /// #7394: a trailing `sub sp, sp, #imm, lsl #12` must contribute + /// `imm << 12`. #7328's decoder masked the `sh` bit into the opcode + /// comparison, so a shifted `sub` did not match at all. + #[test] + fn a_shifted_trailing_sub_is_included() { + assert_eq!( + decode(&[ADD_X29_SP_0X70, SUB_SP_SP_1_LSL12, NOP, RET]), + Some(0x70 + 0x1000), + "`sub sp, sp, #0x1, lsl #12` must contribute 4096, not 1" + ); + } + + /// The measured shape. The shifted `sub` is not the last one, so failing + /// to match it also **ended the accumulation run** and dropped the + /// `sub sp, sp, #0x840` behind it: the decoder reported 0x70 for a frame + /// whose body SP is 0x18B0 below the frame pointer, and the walker + /// enumerated — and the collector wrote through — addresses 6208 bytes + /// off. That is CLAUDE.md's fourth gate-failure mode: a live walker + /// visiting the wrong stack. + #[test] + fn a_shifted_sub_does_not_end_the_accumulation_run() { + assert_eq!( + decode(&[ + ADD_X29_SP_0X70, + SUB_SP_SP_1_LSL12, + SUB_SP_SP_0X840, + NOP, + RET + ]), + Some(0x70 + 0x1000 + 0x840), + "every `sub sp` in the contiguous prologue run must be folded in" + ); + } + + /// The `sh` bit is decoded on the `add` that establishes the frame + /// pointer too — the same masking bug applied there, where it would have + /// made the decoder skip the fp setup entirely and report a later + /// instruction's offset (or `None`). + #[test] + fn a_shifted_fp_setup_is_decoded() { + assert_eq!( + decode(&[ADD_X29_SP_2_LSL12, NOP, RET]), + Some(0x2000), + "`add x29, sp, #0x2, lsl #12` establishes fp 8192 above sp" + ); + } +} diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs b/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs new file mode 100644 index 0000000000..d8469f52c7 --- /dev/null +++ b/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs @@ -0,0 +1,286 @@ +//! `PERRY_STACKMAP_WALKER=verify`: run both walkers and require them to agree. +//! +//! This is the only check that can catch a fast walk that silently skips +//! frames or resolves a root against the wrong frame base. Forced-evacuation +//! verification cannot: it enumerates roots through the same walker, so it +//! never sees a slot the walker never reached, and it has no idea what a root +//! slot is *supposed* to contain — a wrong stack word looks exactly like a +//! right one. +//! +//! # Why the report is this detailed +//! +//! The first end-to-end `verify` run on aarch64 ELF caught the fp-chain walker +//! and the unwinder resolving one root 96 bytes apart (#7984). All the panic +//! could say was +//! +//! ```text +//! fast walk visited 1 unique slots, unwinder visited 1 +//! left: [281474742909688] +//! right: [281474742909592] +//! ``` +//! +//! — two integers, from which the frame, the base register, the function whose +//! prologue was decoded, and therefore *which walker was wrong* are all +//! unrecoverable. Every candidate explanation (a missed trailing `sub sp`, a +//! frame the chain skipped, a CFA that is one frame out) predicts exactly that +//! output, so the gate could prove a bug existed and nothing about its shape. +//! +//! A gate that cannot name what it found sends whoever picks it up back to +//! square one, so this one prints the provenance of every root both walkers +//! resolved: the frame return address it was matched on, the function the +//! record belongs to, the base register and frame offset from the map, the +//! base each walker resolved that register to, and — on aarch64 — the +//! prologue words `fp_to_sp_offset` decoded to derive an SP base. That is +//! enough to say which walker is wrong without a second run. + +use super::{fp_chain, unwind, MutableRootSlot, NativeStackWalkStats, ResolvedRoot, StackMapIndex}; +use std::fmt::Write as _; + +/// Run the fast walk non-mutating, then the unwinder for the real visitation, +/// and panic unless they resolved the identical set of slot addresses. +pub(super) fn visit( + index: &StackMapIndex, + visit: &mut impl FnMut(MutableRootSlot), +) -> NativeStackWalkStats { + let mut fast: Vec = Vec::new(); + let fast_stats = fp_chain::visit(index, &mut |root: ResolvedRoot| fast.push(root)); + let Some(fast_stats) = fast_stats else { + panic!( + "PERRY_STACKMAP_WALKER=verify: fast walk unavailable \ + (chain_walkable={}, anomaly or unsupported target)", + index.chain_walkable + ); + }; + let mut slow: Vec = Vec::new(); + let mut stats = unwind::visit(index, &mut |root: ResolvedRoot| { + slow.push(root); + visit(MutableRootSlot { + kind: super::MutableRootSlotKind::NativeStack, + ptr: root.address as *mut u64, + }); + }); + + if !addresses_agree(&fast, &slow) { + panic!("{}", report(index, &fast, &slow)); + } + + stats.fp_walks = fast_stats.fp_walks; + stats +} + +/// The comparison the gate actually makes: the SETS of slot addresses, since +/// visiting order and duplicate visits of one slot are both immaterial to the +/// collector (rewriting a slot twice is idempotent). +fn addresses_agree(fast: &[ResolvedRoot], slow: &[ResolvedRoot]) -> bool { + unique_addresses(fast) == unique_addresses(slow) +} + +fn unique_addresses(roots: &[ResolvedRoot]) -> Vec { + let mut out: Vec = roots.iter().map(|root| root.address).collect(); + out.sort_unstable(); + out.dedup(); + out +} + +/// The full disagreement, one line per root, plus the prologue evidence for +/// every function whose frame either walker resolved an SP-relative root in. +fn report(index: &StackMapIndex, fast: &[ResolvedRoot], slow: &[ResolvedRoot]) -> String { + let fast_addresses = unique_addresses(fast); + let slow_addresses = unique_addresses(slow); + let mut out = String::new(); + let _ = writeln!( + out, + "PERRY_STACKMAP_WALKER=verify: the fp-chain walker and the platform \ + unwinder resolved different root slots.\n \ + fp-chain: {} unique slot(s) {:#x?}\n unwinder: {} unique slot(s) {:#x?}", + fast_addresses.len(), + fast_addresses, + slow_addresses.len(), + slow_addresses, + ); + // A constant delta between two same-length sets is the signature of a + // frame-base disagreement rather than a missed or invented frame, and it + // is the first thing to know. Say so explicitly instead of leaving it to + // be spotted by subtracting two decimal integers by hand. + if fast_addresses.len() == slow_addresses.len() { + let deltas: Vec = fast_addresses + .iter() + .zip(&slow_addresses) + .map(|(fast, slow)| *fast as i64 - *slow as i64) + .collect(); + let _ = writeln!( + out, + " same slot count, so this is a base disagreement, not a missed \ + frame; fp-chain minus unwinder = {deltas:?} byte(s)" + ); + } + let _ = writeln!(out, "\n fp-chain roots:"); + for root in fast { + describe(&mut out, index, root); + } + let _ = writeln!(out, " unwinder roots:"); + for root in slow { + describe(&mut out, index, root); + } + out +} + +fn describe(out: &mut String, index: &StackMapIndex, root: &ResolvedRoot) { + let register = match root.dwarf_reg { + 29 => "fp/x29", + 31 => "sp", + _ => "reg", + }; + let _ = writeln!( + out, + " slot {:#x} = base {:#x} {:+} | ip {:#x} (fn {:#x} + {:#x}) | \ + map: dwarf {} ({}) offset {:+}{}", + root.address, + root.base, + root.offset, + root.ip, + root.function_address, + root.ip.wrapping_sub(root.function_address), + root.dwarf_reg, + register, + root.offset, + prologue_note(index, root), + ); +} + +/// Whether the map itself vouches for `function_address` as the start of a +/// function it has records for. +/// +/// The report runs on the failure path, where a plausible cause is a map whose +/// addresses are wrong — so it must not dereference an address on the strength +/// of the very data under suspicion. This is the same set the walker's +/// `match_records` containment check consults, so a dump gated on it reads +/// only what the walk already read. +fn map_vouches_for(index: &StackMapIndex, function_address: usize) -> bool { + index + .function_starts + .binary_search(&function_address) + .is_ok() +} + +/// What the fast walker derives an SP base from, spelled out. +/// +/// `fp_to_sp_offset` decodes the owning function's prologue to get +/// `x29 - body_sp`; every SP-relative root in that frame is placed relative to +/// the result. When it is wrong the whole frame is wrong by one constant, +/// which is precisely the shape #7984 presents, so the decoded value and the +/// words it was decoded from belong in the report. +#[cfg(target_arch = "aarch64")] +fn prologue_note(index: &StackMapIndex, root: &ResolvedRoot) -> String { + if root.dwarf_reg != super::DWARF_REG_SP_AARCH64 + || !map_vouches_for(index, root.function_address) + { + return String::new(); + } + let decoded = super::fp_to_sp_offset(root.function_address); + let mut words = String::new(); + for word_index in 0..10usize { + // Reading the prologue is what the walker itself does, from the same + // address it was already gated on above, so this adds no unsafety the + // walk did not already have. + let word = + unsafe { std::ptr::read((root.function_address + word_index * 4) as *const u32) }; + let _ = write!(words, " {word:08x}"); + } + format!("\n fp_to_sp_offset(fn) = {decoded:?}, prologue words:{words}") +} + +#[cfg(not(target_arch = "aarch64"))] +fn prologue_note(_index: &StackMapIndex, _root: &ResolvedRoot) -> String { + String::new() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn root(address: usize, base: usize, offset: i32) -> ResolvedRoot { + ResolvedRoot { + address, + ip: 0x4000, + function_address: 0x3000, + dwarf_reg: 31, + offset, + base, + } + } + + /// An index that vouches for NO function address, so the report never + /// dereferences the synthetic addresses above. + fn empty_index() -> StackMapIndex { + super::super::index_records(Vec::new(), Vec::new()) + } + + #[test] + fn identical_sets_agree_regardless_of_order_or_repeats() { + let fast = vec![root(0x100, 0xF8, 8), root(0x200, 0x1F8, 8)]; + let slow = vec![ + root(0x200, 0x1F8, 8), + root(0x100, 0xF8, 8), + root(0x100, 0xF8, 8), + ]; + assert!( + addresses_agree(&fast, &slow), + "the collector rewrites a slot idempotently, so order and repeats \ + must not fail the gate" + ); + } + + #[test] + fn a_constant_base_delta_is_named_as_one() { + // #7984's exact shape: one slot each, 96 bytes apart. The report has + // to say "base disagreement" and print the delta, because that is the + // fact that separates a wrong frame base from a missed frame — and the + // old message printed neither. + let fast = vec![root(0x1060, 0x1058, 8)]; + let slow = vec![root(0x1000, 0xFF8, 8)]; + assert!(!addresses_agree(&fast, &slow)); + let text = report(&empty_index(), &fast, &slow); + assert!( + text.contains("base disagreement"), + "equal slot counts must be reported as a base disagreement: {text}" + ); + assert!( + text.contains("[96]"), + "the report must print the byte delta: {text}" + ); + assert!( + text.contains("dwarf 31 (sp)"), + "the report must name the base register the map asked for: {text}" + ); + } + + #[test] + fn a_missed_frame_is_not_reported_as_a_base_disagreement() { + let fast = vec![root(0x1000, 0xFF8, 8)]; + let slow = vec![root(0x1000, 0xFF8, 8), root(0x2000, 0x1FF8, 8)]; + let text = report(&empty_index(), &fast, &slow); + assert!( + !text.contains("base disagreement"), + "different slot counts mean a frame was missed or invented: {text}" + ); + } + + /// The report must not dereference an address the map does not vouch for. + /// + /// It runs on the failure path, and one live hypothesis for any such + /// failure is a map whose function addresses are wrong — so reading + /// instructions from one on the strength of that same map turns a + /// diagnostic into a SIGSEGV with no output at all. Measured while writing + /// this file: the first draft did exactly that. + #[test] + fn an_unvouched_function_address_is_never_dereferenced() { + let index = empty_index(); + assert!(!map_vouches_for(&index, 0x3000)); + let text = report(&index, &[root(0x1000, 0xFF8, 8)], &[]); + assert!( + !text.contains("prologue words"), + "no prologue may be dumped for an address the map does not list: {text}" + ); + } +} From 55a3ed6dfa159019e56ab4fb8fbdc5c5a85b1bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:27:33 +0200 Subject: [PATCH 02/11] docs(gc-handoff): #7984 working notes and the changeset for #7997 --- changelog.d/7997-verify-walker-report.md | 37 +++++ gc-handoff/WALKER-NOTES.md | 164 +++++++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 changelog.d/7997-verify-walker-report.md create mode 100644 gc-handoff/WALKER-NOTES.md diff --git a/changelog.d/7997-verify-walker-report.md b/changelog.d/7997-verify-walker-report.md new file mode 100644 index 0000000000..843884c44d --- /dev/null +++ b/changelog.d/7997-verify-walker-report.md @@ -0,0 +1,37 @@ +### `PERRY_STACKMAP_WALKER=verify` now names the disagreement it finds + +The first end-to-end `verify` run on aarch64 ELF caught the fp-chain walker and +the Itanium unwinder resolving the same GC root 96 bytes apart (#7984). The +whole of what the gate could report was `fast walk visited 1 unique slots, +unwinder visited 1` and the two addresses in decimal — not the frame, not the +base register, not the function whose prologue was decoded, and therefore not +*which walker is wrong*. Every candidate explanation predicts exactly that +output: a `sub sp` the prologue decoder's contiguous-run rule missed, a frame +the x29 chain skipped because an intermediate frame carries no frame record +(legal on Linux, not on Darwin), or a CFA one frame out on libgcc. + +Both walkers now hand back a `ResolvedRoot` rather than a bare +`MutableRootSlot`: the same address, plus the frame return address it was +matched on, the record's function, the map's base register and frame offset, +and the base that walker resolved that register to. +`visit_stack_map_root_slots` projects it straight back to a `MutableRootSlot`, +so the collector's view is unchanged. On a mismatch `verify` prints every root +from both walks, states that an equal slot count means a *base* disagreement +rather than a missed frame (with the per-slot byte delta), and on aarch64 dumps +`fp_to_sp_offset`'s decode together with the prologue words it read — the +ground truth for the frame layout the fast walker derives an SP base from. + +The prologue dump is gated on the parsed map vouching for the function address +(`function_starts`, the same set `match_records` consults). The first draft was +not gated, and a unit test with a synthetic address turned the diagnostic into +a SIGSEGV with no output — which is what would happen in the field for the one +failure mode where a report matters most, a map whose addresses are wrong. +`an_unvouched_function_address_is_never_dereferenced` pins it. + +`gc-native-roots.yml`'s crash path tailed 20 lines of the failing run's stderr, +which truncates the report's head; it now tails 120. `verify` and the decoder +tests move into `stack_maps_verify.rs` and `stack_maps_decode_tests.rs` because +`stack_maps.rs` was eight lines under the 2000-line cap. + +This does not fix #7984 — the `ubuntu-24.04-arm` arm stays red. It makes that +arm's next red run diagnostic instead of a riddle. diff --git a/gc-handoff/WALKER-NOTES.md b/gc-handoff/WALKER-NOTES.md new file mode 100644 index 0000000000..989e98d1f0 --- /dev/null +++ b/gc-handoff/WALKER-NOTES.md @@ -0,0 +1,164 @@ +# #7984 — the fp-chain walker and the unwinder disagree by 96 bytes on aarch64 ELF + +Working notes. Written as the investigation runs, so the dead ends are here on +purpose: three of them are the ones the panic message could not distinguish, +and knowing they are *excluded* is most of the value. + +## The claim, restated exactly + +On `ubuntu-24.04-arm`, `PERRY_STACKMAP_WALKER=verify` on `01_nursery_churn`: + +``` +fast walk visited 1 unique slots, unwinder visited 1 + left: [281474742909688] <- fp_chain + right: [281474742909592] <- unwinder +``` + +`left - right = +96`. Same slot count, so this is **not** a missed or invented +frame: it is one root resolved against two different bases. + +## What the two walkers actually compute + +Both are in `crates/perry-runtime/src/gc/roots/stack_maps.rs`. For the frame a +record belongs to: + +| root's base register | `fp_chain` | `unwind` | +|---|---|---| +| DWARF 29 (fp) | `caller_fp`, the word at `[fp]` of the callee's frame record | `_Unwind_GetGR(ctx, 29)` | +| DWARF 31 (sp) | `caller_fp - fp_to_sp_offset(record.function_address)` | `_Unwind_GetCFA(ctx)` | + +`fp_to_sp_offset` decodes the owning function's prologue: the immediate of +`add x29, sp, #imm` plus every `sub sp, sp, #imm` in the *contiguous run* +immediately after it (#7328, #7394). + +Everything else — the parsed map, `match_records`' ±16 window, the record's +`(dwarf_reg, offset)` — is **shared**. A parse bug would move both walkers by +the same amount and produce no divergence at all. So the divergence is provably +one of: + +- (a) `caller_fp` ≠ `_Unwind_GetGR(29)` for that frame, +- (b) `caller_fp - fp_to_sp_offset(F)` ≠ `_Unwind_GetCFA()` for that frame, +- (c) the two walkers attributed the record to *different frames*. + +## Measured: which registers the roots actually use + +Compiled the failing probe on macOS, took the pre-`opt` module out of +`--trace llvm`, and ran perry's own RS4GC pipeline plus `llc` for both triples: + +``` +opt -passes='function(mem2reg),rewrite-statepoints-for-gc' -S _01_nursery_churn_ts.ll +llc -mtriple=aarch64-unknown-linux-gnu -mcpu=neoverse-n2 -O3 # ELF +llc -mtriple=arm64-apple-macosx14.0.0 -mcpu=apple-m1 -O3 # Mach-O +``` + +`llvm-readobj --stackmap` over both objects: + +| | ELF | Mach-O | +|---|---|---| +| root locations | **all `Indirect [R#31 + off]`** | **all `Indirect [R#31 + off]`** | +| functions with records | 2 (stack sizes 96, 176) | 2 (stack sizes 112, 192) | +| the single `[R#31 + 8]` root | the anon-shape constructor, records at +320 and +588 | same | + +So **both platforms take the SP path**, and 96 is exactly the ELF +constructor's `stack size`. The Mach-O twin's frame is 112, which is why the +number is 96 and not something else — it is a property of that one frame. + +## Measured: the prologues + +ELF (the anon-shape constructor — the frame the `[R#31 + 8]` root lives in): + +``` +sub sp, sp, #96 +str d10, [sp, #16] +stp d9, d8, [sp, #24] +stp x29, x30, [sp, #40] <- frame record in the MIDDLE of the frame +str x23, [sp, #56] +stp x22, x21, [sp, #64] +stp x20, x19, [sp, #80] +add x29, sp, #40 <- x29 - body_sp = 40 +.cfi_def_cfa w29, 56 <- CFA = x29 + 56 = body_sp + 96 +``` + +Mach-O, same source function: + +``` +sub sp, sp, #112 +... spills ... +stp x29, x30, [sp, #96] <- frame record at the TOP of the frame +add x29, sp, #96 <- x29 - body_sp = 96 +.cfi_def_cfa w29, 16 +``` + +`fp_to_sp_offset` decodes **40** and **96** respectively, and both are correct. +An audit script that re-implements the decoder over assembly text and compares +it against a full simulation of every prologue `sp` adjustment reports **0 +mismatches / 12 fp functions** across the generated module on both triples, and +0/14 over a hand-built C corpus (small frames, >4 KiB frames needing +`sub sp, sp, #N, lsl #12`, and multi-instruction allocations). + +So (b) is **not** a prologue-decode error for this function. Hypothesis +"the contiguous-run rule missed a `sub sp, sp, #96`" is **refuted for the +observed frame**. + +## Measured: `_Unwind_GetCFA`, and the frame-record-in-the-middle geometry + +Standalone differential harness (`global_asm!` frames with hand-chosen layouts +plus the real `fp_to_sp_offset`), run on **macOS aarch64** and on **aarch64 +Linux (Debian bookworm, libgcc) under colima**: + +| frame shape | fp-chain sp | `_Unwind_GetCFA` | truth | +|---|---|---|---| +| ELF-shaped: record at `body_sp+40` of a 96-byte frame | exact | exact | — | +| Darwin-shaped: record at `body_sp+80` of a 96-byte frame | exact | exact | — | + +`_Unwind_GetCFA` inside an `_Unwind_Backtrace` callback returns the **body +stack pointer of the frame whose return address `_Unwind_GetIP` reports** on +both implementations, which is what `stack_maps_unwind_contract.rs` asserts +(#7392) — confirmed independently here on aarch64 Linux. + +A second harness added a **frameless** intermediate frame (saves `x30`, never +establishes `x29` — legal on Linux, and what any C library built without +`-fno-omit-frame-pointer` emits; not legal on Darwin, where the ABI requires +the chain). Result: the fp chain does pair a return address in the frameless +function with a *different* frame's `x29`, but `fp_to_sp_offset` returns `None` +for a function with no `add x29, sp`, which makes the real walker `return None` +and `verify` panic with "fast walk unavailable" — a different message. Frames +either side of the frameless one still resolve **exactly** in both walkers. +So hypothesis (c) via a frameless frame is **refuted as a producer of this +message**; it produces the other one. + +## Where that leaves it + +Every mechanism reproducible off the target agrees. The divergence needs the +real `ubuntu-24.04-arm` binary, and the gate that found it could not say which +walker was wrong. + +**Step one (PR #7997)**: make the gate say. Both walkers now report a +`ResolvedRoot` — address, the frame return address it was matched on, the +record's function, the map's base register and offset, and the base that walker +resolved that register to — and `verify` prints all of it, calls an +equal-slot-count disagreement a *base* disagreement rather than a missed frame, +and on aarch64 dumps `fp_to_sp_offset`'s decode plus the prologue words it +read. That is enough to settle (a) vs (b) vs (c) from one CI run. + +Gate on the report itself: the prologue dump only runs for a function address +the parsed map vouches for (`function_starts`). Reading instructions from an +address supplied by the data under suspicion is how a diagnostic becomes a +SIGSEGV with no output — measured, in the first draft of this file's tests. + +## Reproduction recipe, for the next person + +Nothing here needs an aarch64 Linux host except the last line: + +```bash +# 1. the module IR, pre-RS4GC +PERRY_RS4GC=1 perry benchmarks/gc_ratchet/probes/01_nursery_churn.ts -o /tmp/p --trace llvm +# 2. perry's own pipeline, then either backend +grep -v '^module asm' .perry-trace/llvm/_01_nursery_churn_ts.ll > m.ll # drops a Mach-O-only .no_dead_strip +opt -passes='function(mem2reg),rewrite-statepoints-for-gc' -S m.ll -o rs.ll +llc -mtriple=aarch64-unknown-linux-gnu -mcpu=neoverse-n2 -O3 rs.ll -o linux.s +clang --target=aarch64-unknown-linux-gnu -mcpu=neoverse-n2 -c linux.s -o linux.o +llvm-readobj --stackmap linux.o # every root's base register and offset +# 3. an arm64 Linux shell, for the unwinder half +colima start --arch aarch64 && docker run --rm --platform linux/arm64 -v "$HOME/x:/x" rust:1-slim-bookworm ... +``` From 91c56b9b11b2513c0331cf807dd28d91702f940e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:34:28 +0200 Subject: [PATCH 03/11] test(gc): run both aarch64 stack-map walkers over a frame we chose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing `cargo test` runs has ever called `fp_chain::visit` or `unwind::visit`. Their only coverage was `PERRY_STACKMAP_WALKER=verify` inside one arm of `gc-native-roots.yml` — a workflow that has never had a successful run on any branch (#7970) and whose aarch64-ELF arm is red on #7984. The unit tests around them cover the decoder and the matcher; the step that turns a `(register, offset)` pair into a stack address had none. Two `global_asm!` probe frames, both copied from real generated code for the same TypeScript function: the aarch64-ELF layout, where LLVM puts the `x29,x30` pair below the other callee-saves so the frame record sits in the MIDDLE of the frame (`sub sp,sp,#96 / stp x29,x30,[sp,#40] / add x29,sp,#40`), and the Mach-O layout, where it sits at the top. Each writes a sentinel into the word its synthetic `Indirect [R#31 + 8]` record names, hands the callback its body SP and the exact return address, and both walkers must resolve it. The assertion is deliberately not `verify`'s set equality: two empty sets are equal and so are two identically-wrong ones, so that is a presence check, not a proof. Each walker must independently land on a word HOLDING the sentinel. `a_wrong_frame_offset_is_caught` is the sabotage arm — a record 8 bytes off must fail the sentinel check, so a green run cannot mean the check is inert. The sample is taken inside the callback, while the probe frame is live. Reading it after the probe returns reads a frame the harness has already reused: measured here, as a sentinel that had become `4`. --- .../perry-runtime/src/gc/roots/stack_maps.rs | 11 + .../gc/roots/stack_maps_walker_agreement.rs | 351 ++++++++++++++++++ 2 files changed, 362 insertions(+) create mode 100644 crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index 2e4e81ddbf..23194fd526 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps.rs @@ -1545,3 +1545,14 @@ mod unwind_contract; #[cfg(test)] #[path = "stack_maps_decode_tests.rs"] mod decode_tests; + +// The only test anywhere that runs BOTH aarch64 walkers over a frame whose +// layout is known, and requires each to land on the word the record names. +// Same platform set as `fp_chain` itself. +#[cfg(all( + test, + any(target_vendor = "apple", target_os = "linux"), + target_arch = "aarch64" +))] +#[path = "stack_maps_walker_agreement.rs"] +mod walker_agreement; diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs b/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs new file mode 100644 index 0000000000..312a2a41e8 --- /dev/null +++ b/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs @@ -0,0 +1,351 @@ +//! The two aarch64 walkers, run against one frame whose layout we chose. +//! +//! # Why this exists +//! +//! Before this file, **nothing `cargo test` runs ever called `fp_chain::visit` +//! or `unwind::visit`.** Their only coverage was `PERRY_STACKMAP_WALKER=verify` +//! inside one arm of `gc-native-roots.yml` — a workflow that had never had a +//! successful run on any branch (#7970), and whose aarch64-ELF arm is red on +//! #7984 precisely because the two walkers resolve one root 96 bytes apart. +//! Every unit test around them tested the *decoder* and the *matcher*; the part +//! that turns a `(register, offset)` pair into a stack address had none. +//! +//! # What it asserts, and why set equality is not enough +//! +//! `verify` compares the two walkers' slot sets. Two empty sets are equal, and +//! so are two identically-wrong sets — that is a presence check, not a proof. +//! The discriminating quantity here is the **contents** of the resolved slot: +//! the probe frame writes a sentinel into the exact word its synthetic +//! stack-map record names, and each walker must independently land on a word +//! holding that sentinel. A walker that visits nothing fails; a walker that +//! visits the wrong word fails even if the other walker agrees with it. +//! +//! `a_wrong_frame_offset_is_caught` is the sabotage arm: it feeds a record +//! whose offset is deliberately 8 bytes off and requires the sentinel check to +//! reject it. Without that, a green run here would only mean the assertions +//! never ran. +//! +//! # The frame layouts +//! +//! Both are copied from real generated code for the same TypeScript function, +//! because they differ per object format and the difference is load-bearing: +//! LLVM's AArch64 **ELF** frame lowering puts the `x29,x30` pair *below* the +//! other callee-saved registers, so the frame record sits in the MIDDLE of the +//! frame and `x29 - body_sp` is small; Mach-O puts it at the TOP. Measured on +//! `benchmarks/gc_ratchet/probes/01_nursery_churn.ts`, the anon-shape +//! constructor: ELF `sub sp,sp,#96 / stp x29,x30,[sp,#40] / add x29,sp,#40`, +//! Mach-O `sub sp,sp,#112 / stp x29,x30,[sp,#96] / add x29,sp,#96`. Both are +//! exercised here on whichever host runs the suite, since the walkers care +//! about the layout and not about the object format that motivated it. + +use super::{ + fp_chain, index_records, unwind, ResolvedRoot, StackMapIndex, StackMapLocation, StackMapRecord, + DWARF_REG_SP_AARCH64, +}; + +/// Written into the slot the synthetic record names, and required to be there +/// when a walker resolves it. Not a round number: a walker that lands on +/// unrelated stack words must not be able to pass by luck. +const SENTINEL: u64 = 0xCAFE_F00D; + +#[cfg(target_vendor = "apple")] +macro_rules! asm_symbol { + ($name:literal) => { + concat!("_", $name) + }; +} +#[cfg(not(target_vendor = "apple"))] +macro_rules! asm_symbol { + ($name:literal) => { + $name + }; +} + +// Each probe: establish the frame, write SENTINEL at [body_sp + 8] — the word +// an `Indirect [R#31 + 8]` record names — then call the Rust callback with +// (body_sp, return_address). `adr x1, 2f` hands the callback the exact PC the +// walkers will match a record on, so the test never has to guess it. +core::arch::global_asm!( + ".p2align 4", + concat!(".globl ", asm_symbol!("perry_walker_probe_elf")), + concat!(asm_symbol!("perry_walker_probe_elf"), ":"), + ".cfi_startproc", + "sub sp, sp, #96", + ".cfi_def_cfa_offset 96", + "stp x29, x30, [sp, #40]", + "add x29, sp, #40", + ".cfi_def_cfa w29, 56", + ".cfi_offset w30, -48", + ".cfi_offset w29, -56", + "mov x2, x0", + "mov x3, #0xF00D", + "movk x3, #0xCAFE, lsl #16", + "str x3, [sp, #8]", + "mov x0, sp", + "adr x1, 2f", + "blr x2", + "2:", + "ldp x29, x30, [sp, #40]", + "add sp, sp, #96", + "ret", + ".cfi_endproc", + ".p2align 4", + concat!(".globl ", asm_symbol!("perry_walker_probe_darwin")), + concat!(asm_symbol!("perry_walker_probe_darwin"), ":"), + ".cfi_startproc", + "sub sp, sp, #112", + ".cfi_def_cfa_offset 112", + "stp x29, x30, [sp, #96]", + "add x29, sp, #96", + ".cfi_def_cfa w29, 16", + ".cfi_offset w30, -8", + ".cfi_offset w29, -16", + "mov x2, x0", + "mov x3, #0xF00D", + "movk x3, #0xCAFE, lsl #16", + "str x3, [sp, #8]", + "mov x0, sp", + "adr x1, 3f", + "blr x2", + "3:", + "ldp x29, x30, [sp, #96]", + "add sp, sp, #112", + "ret", + ".cfi_endproc", +); + +type Probe = extern "C" fn(body_sp: usize, return_address: usize); + +unsafe extern "C" { + fn perry_walker_probe_elf(callback: Probe); + fn perry_walker_probe_darwin(callback: Probe); +} + +/// What the probe frame told us about itself, plus the frame offset the +/// synthetic record should carry. +#[derive(Clone, Copy)] +struct Frame { + function_address: usize, + stack_size: u64, + /// `x29 - body_sp` for this layout, for the message when a walk is wrong. + fp_to_sp: usize, +} + +const ELF_FRAME: Frame = Frame { + function_address: 0, + stack_size: 96, + fp_to_sp: 40, +}; +const DARWIN_FRAME: Frame = Frame { + function_address: 0, + stack_size: 112, + fp_to_sp: 96, +}; + +fn index_for(frame: Frame, return_address: usize, offset: i32) -> StackMapIndex { + index_records( + vec![StackMapRecord { + pc: return_address, + function_address: frame.function_address, + stack_size: frame.stack_size, + roots_start: 0, + roots_len: 1, + }], + vec![StackMapLocation { + dwarf_reg: DWARF_REG_SP_AARCH64, + offset, + }], + ) +} + +/// One resolved slot, sampled WHILE THE PROBE FRAME IS STILL LIVE. +/// +/// Reading the word after the probe returns reads a dead frame that the test +/// harness has already reused — measured while writing this file, as a +/// sentinel that had become `4`. The walk and the sample therefore both happen +/// inside the callback, and only the values travel back out. +#[derive(Clone, Copy, Debug)] +struct Sample { + address: usize, + word: u64, +} + +fn sample(address: usize) -> Sample { + Sample { + address, + // The walkers just handed this address to a collector that would WRITE + // through it, so reading it is strictly weaker than what production + // does with the same value. + word: unsafe { std::ptr::read(address as *const u64) }, + } +} + +fn walk(index: &StackMapIndex) -> (Option>, Vec) { + let mut fast: Vec = Vec::new(); + let fast_stats = fp_chain::visit(index, &mut |root: ResolvedRoot| { + fast.push(sample(root.address)) + }); + let mut slow: Vec = Vec::new(); + unwind::visit(index, &mut |root: ResolvedRoot| { + slow.push(sample(root.address)) + }); + (fast_stats.map(|_| fast), slow) +} + +/// Every walker must land on a word holding `SENTINEL`, and must land on at +/// least one word at all. +fn check(kind: &str, walker: &str, samples: &[Sample], expected: usize, frame: Frame) { + assert!( + !samples.is_empty(), + "{kind}: the {walker} walker resolved NO root. Set equality between \ + two walkers is satisfied by both finding nothing, which is why this \ + asserts the walk reached the probe frame rather than that the two \ + agreed." + ); + for Sample { address, word } in samples { + assert_eq!( + *address, + expected, + "{kind}: the {walker} walker placed the root at {address:#x}, not \ + {expected:#x} ({} bytes out). The probe frame is \ + `x29 - body_sp = {}`; a constant miss is a frame-base error, \ + which is #7984's shape.", + *address as i64 - expected as i64, + frame.fp_to_sp, + ); + // The discriminating quantity. Two walkers agreeing on the wrong word + // is exactly the failure `verify`'s set comparison cannot see. + assert_eq!( + *word, SENTINEL, + "{kind}: the {walker} walker resolved {address:#x}, which does not \ + hold the sentinel the probe frame wrote into the word its record \ + names — the address is a stack word, but not the root's." + ); + } +} + +// The probe frame reports itself through this cell: an `extern "C"` callback +// has nowhere else to put a result, and a thread-local keeps it off `static +// mut` (whose references the 2024 edition rejects). Each test drives one probe +// to completion before reading, so there is no interleaving to reason about. +thread_local! { + static PROBE: std::cell::RefCell = const { + std::cell::RefCell::new(ProbeState { + frame: ELF_FRAME, + offset: 8, + body_sp: 0, + fast: None, + slow: None, + }) + }; +} + +struct ProbeState { + frame: Frame, + offset: i32, + body_sp: usize, + fast: Option>, + slow: Option>, +} + +extern "C" fn run_probe(body_sp: usize, return_address: usize) { + let (frame, offset) = PROBE.with(|cell| { + let state = cell.borrow(); + (state.frame, state.offset) + }); + let index = index_for(frame, return_address, offset); + let (fast, slow) = walk(&index); + let fast = fast.expect( + "the fp-chain walk returned None (an anomaly bail-out). A walker that \ + declines to run cannot be cross-checked against the other, which is \ + the other half of what `verify` reports.", + ); + PROBE.with(|cell| { + let mut state = cell.borrow_mut(); + state.body_sp = body_sp; + state.fast = Some(fast); + state.slow = Some(slow); + }); +} + +fn drive( + probe: unsafe extern "C" fn(Probe), + frame: Frame, + offset: i32, +) -> (usize, Vec, Vec) { + PROBE.with(|cell| { + let mut state = cell.borrow_mut(); + state.frame = frame; + state.offset = offset; + state.fast = None; + state.slow = None; + }); + unsafe { probe(run_probe) }; + PROBE.with(|cell| { + let mut state = cell.borrow_mut(); + let fast = state.fast.take().expect("the probe callback never ran"); + let slow = state.slow.take().expect("the probe callback never ran"); + (state.body_sp, fast, slow) + }) +} + +/// `function_address` is the symbol's runtime address, which only Rust knows. +fn elf_frame() -> Frame { + Frame { + function_address: perry_walker_probe_elf as *const () as usize, + ..ELF_FRAME + } +} + +fn darwin_frame() -> Frame { + Frame { + function_address: perry_walker_probe_darwin as *const () as usize, + ..DARWIN_FRAME + } +} + +#[test] +fn both_walkers_resolve_an_sp_root_in_an_elf_shaped_frame() { + let frame = elf_frame(); + let (body_sp, fast, slow) = drive(perry_walker_probe_elf, frame, 8); + let expected = body_sp + 8; + check("ELF-shaped frame", "fp-chain", &fast, expected, frame); + check("ELF-shaped frame", "unwinder", &slow, expected, frame); +} + +#[test] +fn both_walkers_resolve_an_sp_root_in_a_darwin_shaped_frame() { + let frame = darwin_frame(); + let (body_sp, fast, slow) = drive(perry_walker_probe_darwin, frame, 8); + let expected = body_sp + 8; + check("Mach-O-shaped frame", "fp-chain", &fast, expected, frame); + check("Mach-O-shaped frame", "unwinder", &slow, expected, frame); +} + +/// The sabotage arm: prove the sentinel check can fail. +/// +/// A gate whose assertions have never been violated is a gate nobody has shown +/// can fail. Feed a record whose frame offset is 8 bytes past the real slot and +/// require BOTH walkers to land somewhere that does not hold the sentinel — if +/// this test ever passes by finding the sentinel anyway, the checks above prove +/// nothing. +#[test] +fn a_wrong_frame_offset_is_caught() { + let (body_sp, fast, slow) = drive(perry_walker_probe_elf, elf_frame(), 16); + assert!( + !fast.is_empty() && !slow.is_empty(), + "both walkers must still run" + ); + for Sample { address, word } in fast.iter().chain(slow.iter()) { + assert_eq!( + *address, + body_sp + 16, + "the sabotaged record must resolve to the sabotaged address" + ); + assert_ne!( + *word, SENTINEL, + "the sentinel check cannot distinguish the right slot from a wrong \ + one, so the agreement tests above prove nothing" + ); + } +} From 967dd6bfe38f338b4dbfffd872b62be0f5d239bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:39:29 +0200 Subject: [PATCH 04/11] test(gc): take the walker-agreement verdict outside the extern "C" callback An assertion firing inside the probe callback unwinds out of an `extern "C"` function, which aborts: the harness reported SIGABRT and "panic in a function that cannot unwind" instead of the assertion that fired. Measured while sabotage-verifying the gate. The callback now only records; every verdict is taken by the test. --- .../gc/roots/stack_maps_walker_agreement.rs | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs b/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs index 312a2a41e8..e11f1f8be6 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs @@ -234,8 +234,9 @@ thread_local! { frame: ELF_FRAME, offset: 8, body_sp: 0, + ran: false, fast: None, - slow: None, + slow: Vec::new(), }) }; } @@ -244,8 +245,11 @@ struct ProbeState { frame: Frame, offset: i32, body_sp: usize, + /// Whether the callback ran at all — distinct from "it ran and the + /// fp-chain walk declined", which `fast: None` means. + ran: bool, fast: Option>, - slow: Option>, + slow: Vec, } extern "C" fn run_probe(body_sp: usize, return_address: usize) { @@ -254,17 +258,16 @@ extern "C" fn run_probe(body_sp: usize, return_address: usize) { (state.frame, state.offset) }); let index = index_for(frame, return_address, offset); + // Nothing here may panic: this is an `extern "C"` callback, so unwinding + // out of it aborts the process and the test reports SIGABRT instead of the + // assertion that fired. Every verdict is taken by the caller. let (fast, slow) = walk(&index); - let fast = fast.expect( - "the fp-chain walk returned None (an anomaly bail-out). A walker that \ - declines to run cannot be cross-checked against the other, which is \ - the other half of what `verify` reports.", - ); PROBE.with(|cell| { let mut state = cell.borrow_mut(); state.body_sp = body_sp; - state.fast = Some(fast); - state.slow = Some(slow); + state.ran = true; + state.fast = fast; + state.slow = slow; }); } @@ -277,15 +280,20 @@ fn drive( let mut state = cell.borrow_mut(); state.frame = frame; state.offset = offset; + state.ran = false; state.fast = None; - state.slow = None; + state.slow = Vec::new(); }); unsafe { probe(run_probe) }; PROBE.with(|cell| { let mut state = cell.borrow_mut(); - let fast = state.fast.take().expect("the probe callback never ran"); - let slow = state.slow.take().expect("the probe callback never ran"); - (state.body_sp, fast, slow) + assert!(state.ran, "the probe callback never ran"); + let fast = state.fast.take().expect( + "the fp-chain walk declined (an anomaly bail-out). A walker that \ + will not run cannot be cross-checked against the other, which is \ + the other half of what `verify` reports.", + ); + (state.body_sp, fast, std::mem::take(&mut state.slow)) }) } From 969c59638d9acc2ee8b1447ae6e267ca58f14762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:40:23 +0200 Subject: [PATCH 05/11] ci(gc-native-roots): run the walker-agreement tests on both aarch64 hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They are the only thing that calls `fp_chain::visit` or `unwind::visit` from `cargo test`, and they need neither a compiled probe nor a collection — so they run before the probe matrix, and a walker defect is reported as a walker defect rather than as an oracle diff twenty minutes later. The step requires each test by name: `--lib ` is a substring match, so a rename would select nothing and cargo would still exit 0 having run zero tests. --- .github/workflows/gc-native-roots.yml | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index 6a44a1754c..53f5257bf9 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.github/workflows/gc-native-roots.yml @@ -262,6 +262,42 @@ jobs: run: | export RUSTFLAGS="-C force-frame-pointers=yes -C force-unwind-tables=yes" cargo build --profile perry-dev -p perry -p perry-runtime-static -p perry-stdlib-static + + # The two aarch64 walkers, over a frame this repository wrote, on the host + # that has to walk it. + # + # Until this step, NOTHING `cargo test` runs ever called `fp_chain::visit` + # or `unwind::visit` — their only exercise anywhere was `verify` mode in + # the step further down, inside a workflow that has never had a successful + # run on any branch (#7970). The unit tests cover the decoder and the + # matcher; the step that turns a `(register, offset)` pair into a stack + # address had none, which is why #7984 could only ever be found by + # compiling TypeScript and collecting. + # + # This asks the same question in seconds and without a compiler: given a + # frame whose layout is known — the aarch64-ELF one, where LLVM puts the + # `x29,x30` pair below the other callee-saves, and the Mach-O one, where + # it sits at the top — do both walkers land on the word the record names? + # It runs BEFORE the probe matrix so a walker defect is reported as a + # walker defect rather than as an oracle diff twenty minutes later. + - name: Walker agreement (aarch64 hosts) + if: ${{ !cancelled() && matrix.arch == 'aarch64' && runner.os != 'Windows' }} + run: | + set -euo pipefail + export RUSTFLAGS="-C force-frame-pointers=yes -C force-unwind-tables=yes" + cargo test --profile perry-dev -p perry-runtime --lib gc::roots::stack_maps \ + -- --test-threads=1 | tee /tmp/walker-agreement.log + # `--lib ` is a substring match, so a rename makes it select + # nothing and `cargo test` still exits 0 having run zero tests — a + # gate that cannot fail. Require each test by name, including the + # sabotage arm that proves the sentinel check discriminates. + for name in both_walkers_resolve_an_sp_root_in_an_elf_shaped_frame \ + both_walkers_resolve_an_sp_root_in_a_darwin_shaped_frame \ + a_wrong_frame_offset_is_caught; do + grep -q "walker_agreement::$name ... ok" /tmp/walker-agreement.log \ + || { echo "::error::$name did not run — the filter matched nothing"; exit 1; } + done + - name: Probe matrix, RS4GC mode, forced evacuation if: ${{ !cancelled() }} run: | From 28ec16435f720ab55db71645bf251758ab6bf4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:50:39 +0200 Subject: [PATCH 06/11] docs(gc-handoff): rule out a libgcc-14 CFA difference on the failing distro --- gc-handoff/WALKER-NOTES.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gc-handoff/WALKER-NOTES.md b/gc-handoff/WALKER-NOTES.md index 989e98d1f0..768e5aec9f 100644 --- a/gc-handoff/WALKER-NOTES.md +++ b/gc-handoff/WALKER-NOTES.md @@ -116,6 +116,20 @@ stack pointer of the frame whose return address `_Unwind_GetIP` reports** on both implementations, which is what `stack_maps_unwind_contract.rs` asserts (#7392) — confirmed independently here on aarch64 Linux. +That contract has, as far as CI goes, never run on this target: `cargo-test` +runs on `ubuntu-latest` (x86-64), so `unwind_cfa_is_the_frames_stack_pointer` +has no aarch64-Linux arm. Since the failing runner is Ubuntu 24.04 and the +first measurement above was Debian bookworm, the same binary was re-run against +**both** libgcc lines to rule out a difference: + +| libgcc | CFA vs the frame's real body SP | +|---|---| +| `12.2.0-14+deb12u1` (bookworm) | exact, every frame | +| `14.2.0-4ubuntu2~24.04.1` (noble) | exact, every frame | + +So the unwinder half of the contract holds on the failing distro's own +unwinder. + A second harness added a **frameless** intermediate frame (saves `x30`, never establishes `x29` — legal on Linux, and what any C library built without `-fno-omit-frame-pointer` emits; not legal on Darwin, where the ABI requires From 47031cb02b6040d5b36b89dcecde44ba174ddaee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 12 Aug 2026 23:53:22 +0200 Subject: [PATCH 07/11] diag(gc): print each walk's frame and record counts in the verify report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A walker that stopped early reaches fewer frames, matches fewer records, and takes its roots from the INNER part of the stack — which presents as the same constant address offset a wrong frame base does, and is a completely different bug. These two counts are what separate them, and the report had neither. --- .../src/gc/roots/stack_maps_verify.rs | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs b/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs index d8469f52c7..4e9d4a0e0d 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps_verify.rs @@ -61,7 +61,7 @@ pub(super) fn visit( }); if !addresses_agree(&fast, &slow) { - panic!("{}", report(index, &fast, &slow)); + panic!("{}", report(index, &fast, &slow, fast_stats, stats)); } stats.fp_walks = fast_stats.fp_walks; @@ -84,7 +84,13 @@ fn unique_addresses(roots: &[ResolvedRoot]) -> Vec { /// The full disagreement, one line per root, plus the prologue evidence for /// every function whose frame either walker resolved an SP-relative root in. -fn report(index: &StackMapIndex, fast: &[ResolvedRoot], slow: &[ResolvedRoot]) -> String { +fn report( + index: &StackMapIndex, + fast: &[ResolvedRoot], + slow: &[ResolvedRoot], + fast_stats: NativeStackWalkStats, + slow_stats: NativeStackWalkStats, +) -> String { let fast_addresses = unique_addresses(fast); let slow_addresses = unique_addresses(slow); let mut out = String::new(); @@ -114,6 +120,20 @@ fn report(index: &StackMapIndex, fast: &[ResolvedRoot], slow: &[ResolvedRoot]) - frame; fp-chain minus unwinder = {deltas:?} byte(s)" ); } + // How far each walk got. A walker that stopped early reaches fewer + // frames and therefore fewer records, and its roots come from the INNER + // part of the stack — which presents as a constant offset too, but is a + // completely different bug from a wrong frame base. These two counts are + // what tell them apart. + let _ = writeln!( + out, + " frames visited: fp-chain {}, unwinder {}; records matched: \ + fp-chain {}, unwinder {}", + fast_stats.frames_visited, + slow_stats.frames_visited, + fast_stats.records_matched, + slow_stats.records_matched, + ); let _ = writeln!(out, "\n fp-chain roots:"); for root in fast { describe(&mut out, index, root); @@ -210,6 +230,13 @@ mod tests { } } + fn stats(frames_visited: usize) -> NativeStackWalkStats { + NativeStackWalkStats { + frames_visited, + ..NativeStackWalkStats::default() + } + } + /// An index that vouches for NO function address, so the report never /// dereferences the synthetic addresses above. fn empty_index() -> StackMapIndex { @@ -240,7 +267,7 @@ mod tests { let fast = vec![root(0x1060, 0x1058, 8)]; let slow = vec![root(0x1000, 0xFF8, 8)]; assert!(!addresses_agree(&fast, &slow)); - let text = report(&empty_index(), &fast, &slow); + let text = report(&empty_index(), &fast, &slow, stats(3), stats(4)); assert!( text.contains("base disagreement"), "equal slot counts must be reported as a base disagreement: {text}" @@ -259,7 +286,7 @@ mod tests { fn a_missed_frame_is_not_reported_as_a_base_disagreement() { let fast = vec![root(0x1000, 0xFF8, 8)]; let slow = vec![root(0x1000, 0xFF8, 8), root(0x2000, 0x1FF8, 8)]; - let text = report(&empty_index(), &fast, &slow); + let text = report(&empty_index(), &fast, &slow, stats(3), stats(4)); assert!( !text.contains("base disagreement"), "different slot counts mean a frame was missed or invented: {text}" @@ -277,7 +304,7 @@ mod tests { fn an_unvouched_function_address_is_never_dereferenced() { let index = empty_index(); assert!(!map_vouches_for(&index, 0x3000)); - let text = report(&index, &[root(0x1000, 0xFF8, 8)], &[]); + let text = report(&index, &[root(0x1000, 0xFF8, 8)], &[], stats(3), stats(3)); assert!( !text.contains("prologue words"), "no prologue may be dumped for an address the map does not list: {text}" From e2f7a9208af5436c6daea64dc54dfd7c3f08a735 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 00:49:47 +0200 Subject: [PATCH 08/11] fix(gc): the aarch64 prologue decoder was blind to two real shapes (#7984) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PERRY_STACKMAP_WALKER=verify` caught the fp-chain walker and the Itanium unwinder resolving one root 96 bytes apart on `ubuntu-24.04-arm`. Reproduced end to end and the fp-chain walker is the wrong one, for two reasons in the same prologue. MEASURED on aarch64 Linux, `01_nursery_churn` built by this compiler with `PERRY_TARGET_CPU=neoverse-n2`, function `main` — the module body, which carries 100 stack-map records: 124790: str d10, [sp, #-128]! 124794: stp d9, d8, [sp, #16] 124798: stp x29, x30, [sp, #32] 12479c: add x29, sp, #0x20 <- fp established 1247a0: stp x28, x27, [sp, #48] <- NOT a `sub sp`: the run ended here ... four more callee-save pairs 1247b4: sub sp, sp, #0x50 <- 80 bytes, dropped 1247b8: addvl sp, sp, #-2 <- and 2 x VL more, dropped 1. A callee-save store does not move sp, so it cannot end the prologue's run of stack adjustments. LLVM interleaves them with the frame-pointer setup whenever SVE is on; the decoder read the first one as the end of the prologue and dropped the 80-byte local allocation behind it. Stores through sp with no writeback are now transparent to the run, enumerated by opcode so an unrecognised instruction still ends it. 2. `addvl`/`addpl` adjusts sp in units of the RUNTIME vector length, which is nowhere in the instruction. There is no correct byte count to return, so `fp_to_sp_offset` returns `None` and the walk falls back to the platform unwinder — which reads DWARF CFI and needs no VG for an fp-based frame. Why only this runner: perry tunes a host build with `-mcpu=native`. On a Neoverse-class core that enables SVE and produces the prologue above; the same probe built `-mcpu=neoverse-n1` has neither the interleaved stores nor the `addvl`, and passes. macOS has no SVE, so no Apple arm could ever see it. And 96 is not a constant — it is that frame's missed tail, which scales with the vector length. Under qemu `-cpu max` (VL=64B) the same binary diverges by 208, which is 0x50 + 2 x 64 exactly. Tests use the real instruction words read out of that binary with `objdump -d`, including the encoding of `addvl sp, sp, #-2` (`043f57df`) and the negative case `addvl x8, sp, #2`, which is a body address computation and must not disable the frame. --- .../perry-runtime/src/gc/roots/stack_maps.rs | 92 +++++++++++- .../src/gc/roots/stack_maps_decode_tests.rs | 138 ++++++++++++++++++ .../gc/roots/stack_maps_walker_agreement.rs | 81 ++++++++++ 3 files changed, 306 insertions(+), 5 deletions(-) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index 23194fd526..e3ae9ce5b1 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps.rs @@ -399,11 +399,48 @@ fn fp_to_sp_offset(function_address: usize) -> Option { fp_offset = Some(offset + immediate_of(word)); continue; } - // The prologue's stack adjustments are contiguous; the first - // instruction after them that is not a `sub sp` ends the run. - // Anything later that touches sp is a body operation (a dynamic - // alloca, a call-argument area) which the stack map's own - // offsets already account for. + // #7984: an SVE stack adjustment scales by the RUNTIME vector + // length, which is nowhere in the instruction. There is no + // correct number to return, so return none of one — the + // caller falls back to the platform unwinder, which reads the + // frame's DWARF CFI and does not need VG for an fp-based + // frame. + // + // This is not hypothetical and it is not rare. Perry tunes a + // host build with `-mcpu=native`; on any Neoverse-class core + // that turns SVE on, and LLVM then emits the module body's + // prologue as (measured on `01_nursery_churn`, aarch64 Linux, + // `-mcpu=neoverse-n2`): + // + // add x29, sp, #0x20 <- fp established here + // stp x28, x27, [sp, #48] + // ... four more callee-save pairs ... + // sub sp, sp, #0x50 <- 80 bytes + // addvl sp, sp, #-2 <- and 2 x VL more + // + // The same probe built `-mcpu=neoverse-n1` has neither the + // interleaved stores nor the `addvl`, which is why this was an + // ARM-Linux-runner-only failure that no macOS arm could see. + if writes_sp_by_vector_length(word) { + return None; + } + // A store INTO the frame does not move sp, so it cannot end + // the run of stack adjustments — and LLVM interleaves exactly + // these between the frame-pointer setup and the local-area + // allocation in the shape above. Treating one as the end of + // the prologue is what made the decoder report 0x20 for a + // frame whose body SP is 144 bytes below the frame pointer, + // placing every SP-relative root in it 112 bytes too high. + if is_frame_store_through_sp(word) { + continue; + } + // Anything else ends the prologue. Something later that + // touches sp is a body operation (a dynamic alloca, a + // call-argument area) which the stack map's own offsets + // already account for — and a frame that needs a base pointer + // for either reason records its roots against x19, which + // `chain_walkable` refuses for the whole image, so this walker + // never sees one. break; } } @@ -415,6 +452,51 @@ fn fp_to_sp_offset(function_address: usize) -> Option { fp_offset } +/// `stp`/`str` with SP as the base register and no writeback. +/// +/// These are the callee-save spills LLVM emits, and they do not modify sp — so +/// one appearing after the frame-pointer setup says nothing about whether the +/// prologue's stack adjustments are finished. Enumerated rather than inferred: +/// an instruction this does not recognise ends the run, which is the safe +/// direction. Every opcode below was read out of a real aarch64-Linux binary +/// (`objdump -d`, `01_nursery_churn` built `-mcpu=neoverse-n2`), not from +/// memory. +#[cfg(target_arch = "aarch64")] +fn is_frame_store_through_sp(word: u32) -> bool { + // Base register, bits [9:5]. 31 is SP in a load/store base position (it is + // never XZR there), so no ambiguity to resolve. + if (word >> 5) & 0x1F != u32::from(DWARF_REG_SP_AARCH64) { + return false; + } + matches!( + word & 0xFFC0_0000, + 0xA900_0000 // stp Xt1, Xt2, [sp, #imm] (measured: a9036ffc) + | 0x6D00_0000 // stp Dt1, Dt2, [sp, #imm] (measured: 6d0123e9) + | 0xAD00_0000 // stp Qt1, Qt2, [sp, #imm] + | 0xF900_0000 // str Xt, [sp, #imm] + | 0xFD00_0000 // str Dt, [sp, #imm] + | 0x3D80_0000 // str Qt, [sp, #imm] + ) +} + +/// `addvl`/`addpl` writing SP — an adjustment in units of the runtime SVE +/// vector length. +/// +/// The instruction carries a multiplier, not a byte count, so the frame's real +/// size is unknowable from the text. `fp_to_sp_offset` fails closed on one +/// rather than returning the unscaled figure. +/// +/// Encoding, verified against `043f57df` = `addvl sp, sp, #-2` in a real +/// binary: bits [31:24] `0000_0100`, [23:21] `001`, [20:16] Rn, [15:11] `01010` +/// (`addvl`) or `01011` (`addpl`), [10:5] imm6, [4:0] Rd. +#[cfg(target_arch = "aarch64")] +fn writes_sp_by_vector_length(word: u32) -> bool { + const OPCODE_MASK: u32 = 0xFFE0_F800; + const ADDVL: u32 = 0x0420_5000; + const ADDPL: u32 = 0x0420_5800; + word & 0x1F == u32::from(DWARF_REG_SP_AARCH64) && matches!(word & OPCODE_MASK, ADDVL | ADDPL) +} + #[cfg(not(target_arch = "aarch64"))] fn fp_to_sp_offset(_function_address: usize) -> Option { None diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs b/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs index 62fb90f403..bbd8a30fff 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs @@ -468,3 +468,141 @@ mod fp_offset_trailing_sub_tests { ); } } + +/// #7984: the prologue shape LLVM emits when SVE is on. +/// +/// Every word here was read out of a real aarch64-Linux binary with +/// `objdump -d` — `benchmarks/gc_ratchet/probes/01_nursery_churn.ts` built by +/// this compiler with `PERRY_TARGET_CPU=neoverse-n2`, function `main`, which is +/// the module body and carries 100 stack-map records. The same probe built +/// `-mcpu=neoverse-n1` produces neither the interleaved stores nor the `addvl`, +/// which is why this was an ARM-Linux-runner-only failure. +#[cfg(all(test, target_arch = "aarch64"))] +mod sve_prologue_tests { + use super::super::fp_to_sp_offset; + + fn decode(words: &[u32]) -> Option { + let buf = words.to_vec().into_boxed_slice(); + let out = fp_to_sp_offset(buf.as_ptr() as usize); + drop(buf); + out + } + + // 124790: str d10, [sp, #-128]! + // 124794: stp d9, d8, [sp, #16] + // 124798: stp x29, x30, [sp, #32] + // 12479c: add x29, sp, #0x20 + // 1247a0: stp x28, x27, [sp, #48] + // 1247a4: stp x26, x25, [sp, #64] + // 1247a8: stp x24, x23, [sp, #80] + // 1247ac: stp x22, x21, [sp, #96] + // 1247b0: stp x20, x19, [sp, #112] + // 1247b4: sub sp, sp, #0x50 + // 1247b8: addvl sp, sp, #-2 + // 1247bc: bl js_inline_arena_state + const STR_D10_SP_M128_PRE: u32 = 0xFC18_0FEA; + const STP_D9_D8_SP_16: u32 = 0x6D01_23E9; + const STP_X29_X30_SP_32: u32 = 0xA902_7BFD; + const ADD_X29_SP_0X20: u32 = 0x9100_83FD; + const STP_X28_X27_SP_48: u32 = 0xA903_6FFC; + const STP_X26_X25_SP_64: u32 = 0xA904_67FA; + const STP_X24_X23_SP_80: u32 = 0xA905_5FF8; + const STP_X22_X21_SP_96: u32 = 0xA906_57F6; + const STP_X20_X19_SP_112: u32 = 0xA907_4FF4; + const SUB_SP_SP_0X50: u32 = 0xD101_43FF; + const ADDVL_SP_SP_M2: u32 = 0x043F_57DF; + const BL: u32 = 0x9418_FFE3; + + /// A callee-save store does not move sp, so it cannot end the prologue's + /// run of stack adjustments. + /// + /// Before #7984 the first `stp` after the frame-pointer setup ended the + /// run, so this frame decoded as 0x20 when its body SP is 0x50 further + /// down — placing every SP-relative root in it 80 bytes too high, silently, + /// on the walker that runs when `verify` is off. + #[test] + fn callee_save_stores_do_not_end_the_stack_adjustment_run() { + assert_eq!( + decode(&[ + STR_D10_SP_M128_PRE, + STP_D9_D8_SP_16, + STP_X29_X30_SP_32, + ADD_X29_SP_0X20, + STP_X28_X27_SP_48, + STP_X26_X25_SP_64, + STP_X24_X23_SP_80, + STP_X22_X21_SP_96, + STP_X20_X19_SP_112, + SUB_SP_SP_0X50, + BL, + ]), + Some(0x20 + 0x50), + "the `sub sp, sp, #0x50` behind five callee-save pairs must still \ + be folded into the frame base" + ); + } + + /// An SVE stack adjustment is in units of the runtime vector length, so + /// there is no correct byte count to return. Fail closed and let the + /// platform unwinder — which reads DWARF CFI, and needs no VG for an + /// fp-based frame — answer for this frame. + /// + /// Returning the un-scaled value instead is #7984: `main` decoded as 0x20 + /// against a real `x29 - body_sp` of 0x90, and `PERRY_STACKMAP_WALKER=verify` + /// caught the fp-chain walker and the unwinder 96 bytes apart on + /// `ubuntu-24.04-arm`. + #[test] + fn an_sve_stack_adjustment_fails_closed() { + assert_eq!( + decode(&[ + ADD_X29_SP_0X20, + STP_X28_X27_SP_48, + SUB_SP_SP_0X50, + ADDVL_SP_SP_M2, + BL, + ]), + None, + "a frame whose size depends on the SVE vector length must fall \ + back to the unwinder, not report the part it could read" + ); + } + + /// The whole measured prologue, verbatim: the two defects compose, and the + /// answer is still `None` rather than a partially-correct number. + #[test] + fn the_measured_neoverse_n2_prologue_fails_closed() { + assert_eq!( + decode(&[ + STR_D10_SP_M128_PRE, + STP_D9_D8_SP_16, + STP_X29_X30_SP_32, + ADD_X29_SP_0X20, + STP_X28_X27_SP_48, + STP_X26_X25_SP_64, + STP_X24_X23_SP_80, + STP_X22_X21_SP_96, + STP_X20_X19_SP_112, + SUB_SP_SP_0X50, + ADDVL_SP_SP_M2, + BL, + ]), + None + ); + } + + /// `addvl` into a scratch register is not a stack adjustment and must not + /// disable the frame. LLVM emits `addvl x8, sp, #2` all over an SVE + /// function body to address spill slots; only a write to SP moves the + /// frame. (`043f57df` is `addvl sp,…`; clearing the destination field to + /// x8 gives the body form.) + #[test] + fn addvl_into_a_scratch_register_is_not_a_stack_adjustment() { + let addvl_x8 = (ADDVL_SP_SP_M2 & !0x1F) | 8; + assert_eq!( + decode(&[ADD_X29_SP_0X20, SUB_SP_SP_0X50, addvl_x8, BL]), + Some(0x20 + 0x50), + "only `addvl` writing SP is undecodable; one writing x8 is a body \ + address computation" + ); + } +} diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs b/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs index e11f1f8be6..5647d1b09b 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps_walker_agreement.rs @@ -357,3 +357,84 @@ fn a_wrong_frame_offset_is_caught() { ); } } + +// A frame that saves x30 but never establishes x29. Legal on Linux — it is +// what any C library built without `-fno-omit-frame-pointer` emits — and the +// shape `fp_to_sp_offset` cannot decode, so it stands in for #7984's SVE +// prologue, which cannot be executed on a core without SVE. +core::arch::global_asm!( + ".p2align 4", + concat!(".globl ", asm_symbol!("perry_walker_probe_no_fp")), + concat!(asm_symbol!("perry_walker_probe_no_fp"), ":"), + ".cfi_startproc", + "sub sp, sp, #96", + ".cfi_def_cfa_offset 96", + "str x30, [sp, #88]", + ".cfi_offset w30, -8", + "mov x2, x0", + "mov x3, #0xF00D", + "movk x3, #0xCAFE, lsl #16", + "str x3, [sp, #8]", + "mov x0, sp", + "adr x1, 4f", + "blr x2", + "4:", + "ldr x30, [sp, #88]", + "add sp, sp, #96", + "ret", + ".cfi_endproc", +); + +unsafe extern "C" { + fn perry_walker_probe_no_fp(callback: Probe); +} + +/// When the prologue cannot be decoded, the fast walker declines and the +/// unwinder still resolves the root. +/// +/// This is the fallback #7984's fix rests on: on an SVE host the module body's +/// prologue ends in `addvl sp, sp, #-N`, whose byte count is the runtime vector +/// length, so `fp_to_sp_offset` returns `None` rather than the part it could +/// read — and the walk has to end up on the platform unwinder, which reads +/// DWARF CFI and needs no vector length for an fp-based frame. +/// +/// The probe here is frameless rather than SVE because an `addvl` cannot be +/// executed on a core without SVE, and the two reach the same code path: an +/// undecodable prologue for a matched SP-relative record. The *decoding* half +/// is pinned on the real `addvl` bytes in `stack_maps_decode_tests.rs`. +/// +/// Note what this asserts about the unwinder: not merely that it ran, but that +/// it landed on the word holding the sentinel. A fallback that finds nothing +/// would be a collector with no roots, which is worse than the bug. +#[test] +fn an_undecodable_prologue_declines_the_fast_walk_and_the_unwinder_still_answers() { + let frame = Frame { + function_address: perry_walker_probe_no_fp as *const () as usize, + stack_size: 96, + fp_to_sp: 0, + }; + PROBE.with(|cell| { + let mut state = cell.borrow_mut(); + state.frame = frame; + state.offset = 8; + state.ran = false; + state.fast = None; + state.slow = Vec::new(); + }); + unsafe { perry_walker_probe_no_fp(run_probe) }; + let (body_sp, fast, slow) = PROBE.with(|cell| { + let mut state = cell.borrow_mut(); + assert!(state.ran, "the probe callback never ran"); + ( + state.body_sp, + state.fast.take(), + std::mem::take(&mut state.slow), + ) + }); + assert!( + fast.is_none(), + "a prologue with no `add x29, sp` must abandon the fast walk, not \ + invent a frame base for it" + ); + check("frameless frame", "unwinder", &slow, body_sp + 8, frame); +} From b33110a3fa644cea164752d2de1b19a5e689e1f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 01:05:08 +0200 Subject: [PATCH 09/11] fix(gc): decode the SVE stack adjustment instead of giving up on the frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut of #7984 failed closed on `addvl sp, sp, #-N` and let the platform unwinder answer for the frame. Correct, but on an SVE host that frame is the module body — it is on the stack at every collection — so the fast walker would decline for the whole program. `verify` turns a decline into a panic, so the arm that found the bug would have stayed red on the fix, and the walker whose entire purpose is speed would never run there. The multiplier is in the instruction (`imm6`, signed); only the unit is not. Read it once with `prctl(PR_SVE_GET_VL)` — a syscall, not `rdvl`, which faults on a core without SVE — and cache it. Where it cannot be read, which is every core without SVE including all Apple ones, the decode still fails closed. MEASURED on aarch64 Linux, `01_nursery_churn` built `-mcpu=neoverse-n2`, run under `qemu-aarch64 -cpu max` at two vector lengths so the SCALING is pinned and not one machine's answer: VL = 16 B (sve128) verify passes, output byte-exact VL = 64 B (sve512) verify passes, output byte-exact and the whole probe matrix: 28 verify runs (14 probes x 2 vector lengths), all byte-exact against the pinned Node oracle, no failures. The fast walker is LIVE there, which is the point of decoding rather than declining — `11_collect_at_depth` at VL = 64 B reports fp_walks 12, fallback_walks 0, records_matched 1338, locations_visited 2678, with `verify` green, so all 2678 slots were cross-checked against the unwinder and agreed. The tests derive their expectation from `sve_vector_length_bytes()` rather than hard-coding a byte count, so they pin the scaling on whatever host runs them and still assert the fail-closed answer where there is no SVE. --- .../perry-runtime/src/gc/roots/stack_maps.rs | 82 ++++++++++++- .../src/gc/roots/stack_maps_decode_tests.rs | 45 ++++++- gc-handoff/WALKER-NOTES.md | 113 +++++++++++++++--- 3 files changed, 214 insertions(+), 26 deletions(-) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index e3ae9ce5b1..16c87d51fd 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps.rs @@ -422,7 +422,12 @@ fn fp_to_sp_offset(function_address: usize) -> Option { // interleaved stores nor the `addvl`, which is why this was an // ARM-Linux-runner-only failure that no macOS arm could see. if writes_sp_by_vector_length(word) { - return None; + // The multiplier is in the instruction; the unit is not. + // Read it once from the kernel — `?` fails the whole + // decode where it cannot be read, because half a frame + // size is a wrong answer, not a partial one. + fp_offset = Some(offset + sve_sp_allocation_bytes(word)?); + continue; } // A store INTO the frame does not move sp, so it cannot end // the run of stack adjustments — and LLVM interleaves exactly @@ -491,10 +496,77 @@ fn is_frame_store_through_sp(word: u32) -> bool { /// (`addvl`) or `01011` (`addpl`), [10:5] imm6, [4:0] Rd. #[cfg(target_arch = "aarch64")] fn writes_sp_by_vector_length(word: u32) -> bool { - const OPCODE_MASK: u32 = 0xFFE0_F800; - const ADDVL: u32 = 0x0420_5000; - const ADDPL: u32 = 0x0420_5800; - word & 0x1F == u32::from(DWARF_REG_SP_AARCH64) && matches!(word & OPCODE_MASK, ADDVL | ADDPL) + word & 0x1F == u32::from(DWARF_REG_SP_AARCH64) + && matches!(word & SVE_ADD_OPCODE_MASK, SVE_ADDVL | SVE_ADDPL) +} + +#[cfg(target_arch = "aarch64")] +const SVE_ADD_OPCODE_MASK: u32 = 0xFFE0_F800; +#[cfg(target_arch = "aarch64")] +const SVE_ADDVL: u32 = 0x0420_5000; +#[cfg(target_arch = "aarch64")] +const SVE_ADDPL: u32 = 0x0420_5800; + +/// How many bytes an `addvl`/`addpl` writing SP takes OFF the stack. +/// +/// `addvl Rd, Rn, #imm6` is `Rd = Rn + imm6 * VL`, where VL is the vector +/// length in bytes; `addpl` uses an eighth of it (the predicate length). A +/// prologue allocation is a NEGATIVE multiplier, so a non-negative one is not +/// an allocation and is refused rather than guessed at. +/// +/// `None` — vector length unavailable, or not an allocation — fails the whole +/// decode, which puts the frame on the platform unwinder. Half a frame size is +/// a wrong answer, not a partial one. +#[cfg(target_arch = "aarch64")] +fn sve_sp_allocation_bytes(word: u32) -> Option { + // imm6, bits [10:5], signed. + let raw = ((word >> 5) & 0x3F) as i32; + let multiplier = if raw & 0x20 != 0 { raw - 0x40 } else { raw }; + let allocation = usize::try_from(-multiplier).ok().filter(|n| *n > 0)?; + let vector_length = sve_vector_length_bytes()?; + match word & SVE_ADD_OPCODE_MASK { + SVE_ADDVL => allocation.checked_mul(vector_length), + // `addpl`'s unit is VL/8, and a vector length is always a multiple of + // 16 bytes, so the division is exact. + SVE_ADDPL => allocation.checked_mul(vector_length / 8), + _ => None, + } +} + +/// The calling thread's SVE vector length in bytes. +/// +/// Read from the kernel rather than executed: `rdvl` would be the direct way +/// and it faults on a core without SVE, which is most of them — including +/// every Apple one, where this returns `None` and any `addvl` in a decoded +/// prologue therefore fails closed. `prctl(PR_SVE_GET_VL)` costs one syscall, +/// answers on a thread that has never touched SVE, and is cached for the +/// process because nothing in Perry calls `PR_SVE_SET_VL`. +/// +/// The walking thread is the right thread to ask: the prologue whose `addvl` +/// is being decoded executed on it, with this same length. +#[cfg(all(target_arch = "aarch64", target_os = "linux"))] +fn sve_vector_length_bytes() -> Option { + static VECTOR_LENGTH: OnceLock> = OnceLock::new(); + *VECTOR_LENGTH.get_or_init(|| { + // + const PR_SVE_GET_VL: i32 = 51; + const PR_SVE_VL_LEN_MASK: i32 = 0xffff; + unsafe extern "C" { + fn prctl(option: i32, ...) -> i32; + } + let raw = unsafe { prctl(PR_SVE_GET_VL) }; + // Negative is -1/errno: no SVE, or a kernel without the interface. A + // zero length would be nonsense; refuse it rather than scale by it. + (raw > 0).then(|| (raw & PR_SVE_VL_LEN_MASK) as usize) + }) +} + +#[cfg(all(target_arch = "aarch64", not(target_os = "linux")))] +fn sve_vector_length_bytes() -> Option { + // No non-Linux aarch64 target Perry supports implements SVE, and neither + // backend for them emits `addvl`. Fail closed if one ever does, rather + // than invent a length. + None } #[cfg(not(target_arch = "aarch64"))] diff --git a/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs b/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs index bbd8a30fff..9c071565a3 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps_decode_tests.rs @@ -488,6 +488,15 @@ mod sve_prologue_tests { out } + /// What `addvl sp, sp, #-2` costs on THIS host, and `None` where the + /// vector length cannot be read — which is every core without SVE, + /// including all Apple ones. Deriving the expectation instead of writing a + /// constant is what makes these tests pin the SCALING rather than one + /// machine's answer. + fn two_vector_lengths() -> Option { + super::super::sve_vector_length_bytes().map(|vl| 2 * vl) + } + // 124790: str d10, [sp, #-128]! // 124794: stp d9, d8, [sp, #16] // 124798: stp x29, x30, [sp, #32] @@ -552,7 +561,7 @@ mod sve_prologue_tests { /// caught the fp-chain walker and the unwinder 96 bytes apart on /// `ubuntu-24.04-arm`. #[test] - fn an_sve_stack_adjustment_fails_closed() { + fn an_sve_stack_adjustment_is_scaled_by_the_vector_length_or_fails_closed() { assert_eq!( decode(&[ ADD_X29_SP_0X20, @@ -561,16 +570,38 @@ mod sve_prologue_tests { ADDVL_SP_SP_M2, BL, ]), - None, - "a frame whose size depends on the SVE vector length must fall \ - back to the unwinder, not report the part it could read" + two_vector_lengths().map(|bytes| 0x20 + 0x50 + bytes), + "`addvl sp, sp, #-2` allocates two vector lengths; where the \ + length cannot be read the whole decode must fail so the frame \ + goes to the unwinder, never report the 0x70 it could read" + ); + } + + /// The multiplier is read from the instruction and the unit from the + /// kernel, so the two must be pinned separately — a decoder that ignored + /// `imm6` would still pass the test above on a host with VL = 16 and one + /// `addvl`. + #[test] + fn the_sve_multiplier_comes_from_the_instruction() { + let Some(vl) = super::super::sve_vector_length_bytes() else { + return; // no SVE on this host; the fail-closed arm covers it + }; + assert_eq!( + super::super::sve_sp_allocation_bytes(ADDVL_SP_SP_M2), + Some(2 * vl) ); + // `addvl sp, sp, #-1`: imm6 = -1 in bits [10:5]. + let addvl_m1 = (ADDVL_SP_SP_M2 & !(0x3F << 5)) | (0x3F << 5); + assert_eq!(super::super::sve_sp_allocation_bytes(addvl_m1), Some(vl)); + // A POSITIVE multiplier is a deallocation, not a prologue allocation. + let addvl_p2 = (ADDVL_SP_SP_M2 & !(0x3F << 5)) | (2 << 5); + assert_eq!(super::super::sve_sp_allocation_bytes(addvl_p2), None); } /// The whole measured prologue, verbatim: the two defects compose, and the /// answer is still `None` rather than a partially-correct number. #[test] - fn the_measured_neoverse_n2_prologue_fails_closed() { + fn the_measured_neoverse_n2_prologue_decodes_or_fails_closed() { assert_eq!( decode(&[ STR_D10_SP_M128_PRE, @@ -586,7 +617,9 @@ mod sve_prologue_tests { ADDVL_SP_SP_M2, BL, ]), - None + two_vector_lengths().map(|bytes| 0x20 + 0x50 + bytes), + "the whole measured prologue: 0x20 from the `add`, 0x50 from the \ + `sub` behind five callee-save pairs, and two vector lengths" ); } diff --git a/gc-handoff/WALKER-NOTES.md b/gc-handoff/WALKER-NOTES.md index 768e5aec9f..bacbd15159 100644 --- a/gc-handoff/WALKER-NOTES.md +++ b/gc-handoff/WALKER-NOTES.md @@ -141,27 +141,110 @@ either side of the frameless one still resolve **exactly** in both walkers. So hypothesis (c) via a frameless frame is **refuted as a producer of this message**; it produces the other one. -## Where that leaves it +## ANSWER: the unwinder is right, and the fp-chain walker was blind twice -Every mechanism reproducible off the target agrees. The divergence needs the -real `ubuntu-24.04-arm` binary, and the gate that found it could not say which -walker was wrong. +Reproduced end to end on aarch64 Linux (Ubuntu 24.04 arm64 under colima, LLVM +22.1.8 from apt.llvm.org, `--profile perry-dev`, the issue's own RUSTFLAGS). +The default `-mcpu=native` on that host resolves to an Apple core without SVE +and **passes** — all 14 probes, all of `verify`. Forcing the tuning the GitHub +ARM runner gets reproduces it: -**Step one (PR #7997)**: make the gate say. Both walkers now report a -`ResolvedRoot` — address, the frame return address it was matched on, the -record's function, the map's base register and offset, and the base that walker -resolved that register to — and `verify` prints all of it, calls an -equal-slot-count disagreement a *base* disagreement rather than a missed frame, -and on aarch64 dumps `fp_to_sp_offset`'s decode plus the prologue words it -read. That is enough to settle (a) vs (b) vs (c) from one CI run. +``` +PERRY_TARGET_CPU=neoverse-n2 -> diverges +PERRY_TARGET_CPU=neoverse-n1 -> passes +``` + +That is the whole "why only this runner": Perry tunes a host build with +`-mcpu=native`; a Neoverse-class core turns SVE on, and SVE changes the shape +of the prologue LLVM emits. No Apple arm can ever see it, and no x86-64 arm +either. + +### The frame + +`main` — the module body, 100 stack-map records — built `-mcpu=neoverse-n2`, +read out of the binary with `objdump -d` (`PERRY_DEBUG_SYMBOLS=1`, which +suppresses the final strip): + +``` +124790: fc180fea str d10, [sp, #-128]! +124794: 6d0123e9 stp d9, d8, [sp, #16] +124798: a9027bfd stp x29, x30, [sp, #32] +12479c: 910083fd add x29, sp, #0x20 <- fp established; decoder reads 32 +1247a0: a9036ffc stp x28, x27, [sp, #48] <- NOT a `sub sp`: the run ENDED here +1247a4: a90467fa stp x26, x25, [sp, #64] +1247a8: a9055ff8 stp x24, x23, [sp, #80] +1247ac: a90657f6 stp x22, x21, [sp, #96] +1247b0: a9074ff4 stp x20, x19, [sp, #112] +1247b4: d10143ff sub sp, sp, #0x50 <- 80 bytes, DROPPED +1247b8: 043f57df addvl sp, sp, #-2 <- 2 x VL more, DROPPED +``` + +Two independent defects, both in `fp_to_sp_offset`: + +1. **A callee-save store ended the accumulation run.** It does not move sp, so + it says nothing about whether the prologue's stack adjustments are over — + but the rule was "the first instruction that is not a `sub sp` ends the + run". LLVM interleaves those stores with the frame-pointer setup whenever + SVE is on, so the 80-byte local allocation behind them was dropped. +2. **`addvl sp, sp, #-N` was not decoded at all.** Its unit is the runtime SVE + vector length, which is not in the instruction. + +### Which walker is right, arithmetically + +From the report the new instrument prints, under qemu `-cpu max` (VL = 64 B): + +``` +fp-chain: [0x400000800af0, 0x400000800b08] +unwinder: [0x400000800a20, 0x400000800a38] +same slot count, so this is a base disagreement, not a missed frame; + fp-chain minus unwinder = [208, 208] byte(s) +frames visited: fp-chain 9, unwinder 10; records matched: fp-chain 1, unwinder 1 + slot 0x400000800af0 = base 0x400000800ab0 +64 | ip ... (fn 0xaaaaaabc4790 + 0x22c) + fp_to_sp_offset(fn) = Some(32), prologue words: fc180fea 6d0123e9 a9027bfd + 910083fd a9036ffc a90467fa a9055ff8 a90657f6 a9074ff4 d10143ff +``` + +Same function, same ip, same record, same offsets — so it is a base +disagreement, and the bases are `0x…ab0` (fast) and `0x…9e0` (unwinder). + + caller_fp = fast base + decoded = 0x…ab0 + 32 = 0x…ad0 + true x29-body_sp = 32 + 0x50 + 2*64 = 240 + true body_sp = 0x…ad0 - 240 = 0x…9e0 <- the unwinder's base -Gate on the report itself: the prologue dump only runs for a function address -the parsed map vouches for (`function_starts`). Reading instructions from an -address supplied by the data under suspicion is how a diagnostic becomes a -SIGSEGV with no output — measured, in the first draft of this file's tests. +**The unwinder's base is the frame's real body SP, derived independently from +the prologue the fast walker misread.** That is the proof, not an appeal to the +unwinder being the reference implementation. + +### 96 is not a constant + +It is that frame's missed tail, and it scales with the vector length: + +| host | missed | = | +|---|---|---| +| `ubuntu-24.04-arm` (Cobalt 100, VL = 16 B) | 96 | 0x50 + 1 x 16 | +| qemu `-cpu max` (VL = 64 B) | 208 | 0x50 + 2 x 64 | + +So the issue's open question 2 — "is 96 constant?" — is answered: no, and a fix +keyed on 96 would have been wrong on every other vector length. + +### The fix + +1. Stores through sp with no writeback (`stp`/`str`, enumerated by opcode) are + transparent to the accumulation run. Unrecognised instructions still end it, + so the safe direction is preserved. +2. `addvl`/`addpl` writing sp is decoded: the multiplier from `imm6`, the unit + from `prctl(PR_SVE_GET_VL)`, cached. Where the length cannot be read — every + core without SVE, including all Apple ones — the whole decode fails and the + frame goes to the platform unwinder, which reads DWARF CFI and needs no VG + for an fp-based frame. + +Fail-closed matters here in a way that a "return the part I could read" fallback +would not: reporting 0x70 for a frame whose body SP is 240 bytes down is exactly +the silent wrong answer this bug is. ## Reproduction recipe, for the next person + Nothing here needs an aarch64 Linux host except the last line: ```bash From 53e046fb223cb5c5f499cd062c84bd0c414cfccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 01:06:02 +0200 Subject: [PATCH 10/11] docs(changelog): fold the #7984 fix into the changeset --- changelog.d/7997-verify-walker-report.md | 119 ++++++++++++++++------- 1 file changed, 86 insertions(+), 33 deletions(-) diff --git a/changelog.d/7997-verify-walker-report.md b/changelog.d/7997-verify-walker-report.md index 843884c44d..a0e849371c 100644 --- a/changelog.d/7997-verify-walker-report.md +++ b/changelog.d/7997-verify-walker-report.md @@ -1,37 +1,90 @@ +### aarch64: the fast GC stack-map walker misread every SVE-shaped prologue (#7984) + +`PERRY_STACKMAP_WALKER=verify` caught the frame-pointer-chain walker and the +Itanium unwinder resolving the same GC root 96 bytes apart on +`ubuntu-24.04-arm`. The fp-chain walker is the wrong one — it is also the one +that runs when `verify` is off — and it was blind twice in the same prologue. + +Reproduced end to end on aarch64 Linux. The trigger is the tuning, not the +distro: Perry builds a host binary with `-mcpu=native`, and on a Neoverse-class +core that turns SVE on, which changes the shape LLVM emits. The same probe built +`-mcpu=neoverse-n1` passes; no Apple arm can see it at all. Measured on `main` +— the module body, 100 stack-map records — built `-mcpu=neoverse-n2`: + +``` +12479c: add x29, sp, #0x20 <- fp established; the decoder read 32 +1247a0: stp x28, x27, [sp, #48] <- not a `sub sp`, so the run ended HERE +... four more callee-save pairs +1247b4: sub sp, sp, #0x50 <- 80 bytes, dropped +1247b8: addvl sp, sp, #-2 <- and two vector lengths more, dropped +``` + +1. **A callee-save store ended the accumulation run.** It does not move sp, so + it says nothing about whether the prologue's stack adjustments are finished, + but the rule was "the first instruction that is not a `sub sp` ends the run". + Stores through sp with no writeback are now transparent to it, enumerated by + opcode so an unrecognised instruction still ends the run — the safe + direction. +2. **`addvl`/`addpl` writing sp was not decoded at all.** Its multiplier is in + the instruction; its unit is the runtime SVE vector length, which is not. + That is now read once via `prctl(PR_SVE_GET_VL)` — a syscall rather than + `rdvl`, which faults on a core without SVE — and cached. Where it cannot be + read the whole decode fails and the frame goes to the platform unwinder, + which reads DWARF CFI and needs no VG for an fp-based frame. + +**96 was never a constant.** It is that frame's missed tail and it scales with +the vector length: 96 on the runner (`0x50 + 1 x 16`), 208 under `qemu -cpu max` +at VL = 64 (`0x50 + 2 x 64`). A fix keyed on 96 would have been wrong on every +other vector length. + +Validated at two vector lengths so the scaling is pinned rather than one +machine's answer: 28 `verify` runs (14 probes x VL 16 B and 64 B) under +`qemu-aarch64 -cpu max`, all byte-exact against the pinned Node oracle. The fast +walker is live there — `11_collect_at_depth` reports `fp_walks 12`, +`fallback_walks 0`, `records_matched 1338`, `locations_visited 2678` with +`verify` green, so 2678 slots were cross-checked against the unwinder and +agreed. + ### `PERRY_STACKMAP_WALKER=verify` now names the disagreement it finds -The first end-to-end `verify` run on aarch64 ELF caught the fp-chain walker and -the Itanium unwinder resolving the same GC root 96 bytes apart (#7984). The -whole of what the gate could report was `fast walk visited 1 unique slots, -unwinder visited 1` and the two addresses in decimal — not the frame, not the -base register, not the function whose prologue was decoded, and therefore not -*which walker is wrong*. Every candidate explanation predicts exactly that -output: a `sub sp` the prologue decoder's contiguous-run rule missed, a frame -the x29 chain skipped because an intermediate frame carries no frame record -(legal on Linux, not on Darwin), or a CFA one frame out on libgcc. +All the gate could report was `fast walk visited 1 unique slots, unwinder +visited 1` and two addresses in decimal — not the frame, not the base register, +not the function whose prologue was decoded, and therefore not which walker was +wrong. Every candidate explanation predicts exactly that output. Both walkers now hand back a `ResolvedRoot` rather than a bare -`MutableRootSlot`: the same address, plus the frame return address it was -matched on, the record's function, the map's base register and frame offset, -and the base that walker resolved that register to. -`visit_stack_map_root_slots` projects it straight back to a `MutableRootSlot`, -so the collector's view is unchanged. On a mismatch `verify` prints every root -from both walks, states that an equal slot count means a *base* disagreement -rather than a missed frame (with the per-slot byte delta), and on aarch64 dumps -`fp_to_sp_offset`'s decode together with the prologue words it read — the -ground truth for the frame layout the fast walker derives an SP base from. - -The prologue dump is gated on the parsed map vouching for the function address -(`function_starts`, the same set `match_records` consults). The first draft was -not gated, and a unit test with a synthetic address turned the diagnostic into -a SIGSEGV with no output — which is what would happen in the field for the one -failure mode where a report matters most, a map whose addresses are wrong. -`an_unvouched_function_address_is_never_dereferenced` pins it. - -`gc-native-roots.yml`'s crash path tailed 20 lines of the failing run's stderr, -which truncates the report's head; it now tails 120. `verify` and the decoder -tests move into `stack_maps_verify.rs` and `stack_maps_decode_tests.rs` because -`stack_maps.rs` was eight lines under the 2000-line cap. - -This does not fix #7984 — the `ubuntu-24.04-arm` arm stays red. It makes that -arm's next red run diagnostic instead of a riddle. +`MutableRootSlot`: the address, plus the frame return address it was matched on, +the record's function, the map's base register and frame offset, and the base +that walker resolved that register to. `visit_stack_map_root_slots` projects it +straight back, so the collector's view is unchanged. On a mismatch `verify` +prints every root from both walks, states that an equal slot count means a +*base* disagreement rather than a missed frame (with the per-slot delta), prints +each walk's frame and record counts so an early-terminating walk is +distinguishable, and on aarch64 dumps `fp_to_sp_offset`'s decode together with +the prologue words it read. That report is what identified #7984 in one run. + +The prologue dump is gated on the parsed map vouching for the function address. +The first draft was not, and a unit test with a synthetic address turned the +diagnostic into a SIGSEGV with no output — which is what would happen in the +field for the one failure mode where a report matters most, a map whose +addresses are wrong. + +### The two aarch64 walkers now have unit coverage at all + +Nothing `cargo test` runs had ever called `fp_chain::visit` or `unwind::visit`; +the decoder and the matcher were covered, the step that turns a +`(register, offset)` pair into a stack address was not. +`stack_maps_walker_agreement.rs` drives both over `global_asm!` probe frames in +the two real layouts — the aarch64-ELF one with the frame record in the middle +of the frame, and the Mach-O one with it at the top — and requires each walker +to land on a word **holding a sentinel** the probe wrote into the slot its +record names. Set equality is satisfied by two empty sets and by two identically +wrong ones; the slot's contents are the discriminating quantity. +`a_wrong_frame_offset_is_caught` is the sabotage arm, and a third test pins that +an undecodable prologue declines the fast walk while the unwinder still +resolves the root — the fallback the SVE fix rests on. + +`gc-native-roots.yml` runs them on both aarch64 arms, before the probe matrix, +requiring each test by name (`--lib ` is a substring match, so a rename +would select nothing and cargo would still exit 0). Its crash path tailed 20 +lines of the failing run's stderr, which truncates the report; now 120. From dcee6a4b1e5f7a345406319a4036b906b8220248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 01:18:33 +0200 Subject: [PATCH 11/11] docs(gc-native-roots): the ubuntu-24.04-arm arm is no longer expected red MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header told the next reader that arm must STAY red until #7984 is fixed. It is fixed here, so leaving that would make a green run look like a lost gate and a red one look expected — both wrong. Says what to do instead if it goes red again: read the report before assuming a regression. --- .github/workflows/gc-native-roots.yml | 33 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/.github/workflows/gc-native-roots.yml b/.github/workflows/gc-native-roots.yml index 53f5257bf9..cc8c652810 100644 --- a/.github/workflows/gc-native-roots.yml +++ b/.github/workflows/gc-native-roots.yml @@ -119,11 +119,24 @@ # objects copied, and the whole step passes. The collector was # never the problem. # -# ubuntu-24.04-arm REAL DEFECT, filed as #7984. `PERRY_STACKMAP_WALKER=verify` -# caught the fast fp-chain walker and the unwinder resolving the -# same root to addresses 96 bytes apart. This arm is red because -# it found the bug it was built to find; it must STAY red until -# #7984 is fixed. Do not skip it. +# ubuntu-24.04-arm REAL DEFECT, filed as #7984 and FIXED (#7997). +# `PERRY_STACKMAP_WALKER=verify` caught the fast fp-chain +# walker and the unwinder resolving the same root 96 bytes +# apart, and the fast walker — the one that runs when `verify` +# is off — was the wrong one. Two blind spots in +# `fp_to_sp_offset`, both only reachable with SVE on, which is +# what `-mcpu=native` turns on for a Neoverse-class core and +# nothing on macOS or x86-64 ever does: a callee-save store +# ended the prologue's stack-adjustment run, and +# `addvl sp, sp, #-N` was not decoded at all. The 96 was never +# a constant — it is that frame's missed tail, and it scales +# with the vector length (208 at VL = 64 B). +# +# This arm should now be GREEN. If it goes red again, read the +# report `verify` prints before assuming a regression: it names +# the frame, the base register, both resolved bases and the +# prologue words, which is enough to say which walker is wrong +# without a second run. # # windows-latest REAL DEFECT, filed as #7985 (`perry.exe` cannot link # against the official LLVM 22 release: /MT-vs-/MD CRT mismatch, @@ -133,11 +146,11 @@ # `D:\a\_temp` as a remote host — was a workflow bug and is # fixed here with `--force-local`. # -# So: after #7970 the macOS and ubuntu-latest arms should pass and the other two -# should remain red on their filed defects. This workflow is therefore NOT a -# promotion candidate yet — promoting it while #7984/#7985 are open would block -# every PR. Promote only once all four arms are green, and per CLAUDE.md, run it -# green once BEFORE adding it to branch protection. +# So: after #7970 and #7997 three of the four arms should pass and only +# windows-latest should remain red, on #7985. This workflow is therefore still +# NOT a promotion candidate — promoting it while #7985 is open would block every +# PR. Promote only once all four arms are green, and per CLAUDE.md, run it green +# once BEFORE adding it to branch protection. name: gc-native-roots on: # Must run where it can actually gate something. Branch-scoped triggers were