diff --git a/changelog.d/7079-shadow-stack-runtime-ops.md b/changelog.d/7079-shadow-stack-runtime-ops.md new file mode 100644 index 0000000000..599e1bcdb0 --- /dev/null +++ b/changelog.d/7079-shadow-stack-runtime-ops.md @@ -0,0 +1,51 @@ +Made the two hot shadow-stack runtime operations cheap. A ceiling measurement +had put the shadow stack at +71.5 % instructions on application-shaped code +(`w6_records`, Pi 5) versus not emitting it at all — roughly 65–70 instructions +per shadow op — and located two implementation costs rather than anything +inherent to shadow stacks. + +**`js_shadow_frame_push` did three `Vec::resize` calls per activation.** The +per-slot state lived in three parallel thread-local `Vec`s (`stack: Vec` +values, `slot_ptrs: Vec` bindings, `active: Vec` liveness bits), +so one frame push meant five capacity checks, three length updates, and — read +out of the linked archive's disassembly — up to three `memset` **calls** for a +frame with two to four pointer-typed locals. The three words are now one +16-byte `ShadowEntry { value, meta }`, the frame header packs `prev_frame_top` +and `slot_count` into a single entry, and the slot clear is a constant-size +store. A push is now one capacity check plus `movi`/`stp q0, q0`/`stp q0, q0`, +with no call on the common path; a slot store is one bounds check against one +length instead of three, and writes the whole entry with one `stp`. + +Keeping that clear call-free took three attempts, all verified by +disassembling the shipped archive: LLVM re-forms a `match` on the slot count, +a bounded loop, and a runtime-length `write_bytes` alike into a compare chain +that calls `memset`, and it tail-merged even the constant-size store with the +neighbouring large-frame `memset` into one `csel`-the-length-then-call until +the large path moved behind `#[cold] #[inline(never)]`. + +**`js_shadow_slot_set` / `js_shadow_slot_bind` called the root write barrier on +every store.** That barrier is the incremental-mark root *shading* barrier, not +the generational remembered-set barrier — old→young edges are logged by the +heap-slot barriers, which are untouched. Its entire body reads a thread-local +and returns when no incremental cycle is armed, so the cost was a call plus a +second TLS lookup on every pointer-typed local update. It is now gated inline on +`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT`, the same guard codegen already +emits around `js_write_barrier_root_nanbox` for persistent shadow slots. The +gate is observationally identical rather than a narrowing: +`incremental_mark_barrier_enable` installs the thread-local *and then* +increments the count before returning to the mutator, so a zero count proves +this thread's pointer is null and proves the call would have been a no-op. A +non-zero count is conservative in the harmless direction. The barrier itself is +unchanged and still fires whenever a cycle is in flight. + +The liveness bit now shares a word with the bound compiled-local address (bit 0, +always free on an 8-byte-aligned local slot). An address that would collide with +the tag is recorded as active-but-unbound rather than truncated: the mirrored +value is still marked and still rewritten, and the collector is never handed a +mis-derived address to write a forwarded pointer into. + +New `gc::tests::shadow_stack_ops` covers the four properties an optimisation +here can silently break — liveness, rewritability through a real copying minor, +the value the mutator actually stored, and a fresh frame never inheriting the +recycled buffer tail — and every one was verified to fail under a targeted +sabotage of the thing it covers. diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index d79f45bee3..39bb0c9be5 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -24,6 +24,8 @@ pub use scanner_shims::{ }; pub(crate) use shadow_stack::shadow_stack_has_active_frame; pub(crate) use shadow_stack::SHADOW; +#[allow(unused_imports)] +pub(crate) use shadow_stack::{bound_slot_meta, ShadowEntry, SLOT_ACTIVE, SLOT_PTR_MASK}; pub use shadow_stack::{ js_shadow_frame_pop, js_shadow_frame_push, js_shadow_slot_bind, js_shadow_slot_get, js_shadow_slot_set, shadow_stack_depth, SHADOW_STACK_GROW_RESERVE, SHADOW_STACK_HEADER_SLOTS, @@ -1493,29 +1495,34 @@ impl MutableRootSlot { pub(super) fn visit_shadow_stack_root_slots(mut visit: impl FnMut(MutableRootSlot)) { SHADOW.with(|cell| unsafe { let s = &mut *cell.get(); - if s.stack.is_empty() { + if s.slots.is_empty() { return; } let mut top = s.frame_top; while top != usize::MAX && top >= SHADOW_STACK_HEADER_SLOTS { let header_base = top - SHADOW_STACK_HEADER_SLOTS; - if header_base + 1 >= s.stack.len() { + if header_base >= s.slots.len() { break; } - let slot_count = s.stack[header_base + 1] as usize; + let header = s.slots[header_base]; + let slot_count = header.meta; let slots_end = top + slot_count; - if slots_end > s.stack.len() { + if slots_end > s.slots.len() { break; } - let base = s.stack.as_mut_ptr().add(top); + let base = s.slots.as_mut_ptr().add(top); for i in 0..slot_count { - let slot_idx = top + i; - if !s.active.get(slot_idx).copied().unwrap_or(false) { + let entry = base.add(i); + if !(*entry).is_active() { continue; } - let bound_ptr = s.slot_ptrs.get(slot_idx).copied().unwrap_or(0) as *mut u64; + let bound_ptr = (*entry).bound_ptr(); + // Unbound entries expose the mirror word itself. `ShadowEntry` + // is `#[repr(C)]` with `value` at offset 0, so this is a + // correctly-aligned `*mut u64` into the buffer — the same + // storage the pre-#7079 parallel-`Vec` layout handed out. let ptr = if bound_ptr.is_null() { - base.add(i) + std::ptr::addr_of_mut!((*entry).value) } else { bound_ptr }; @@ -1524,7 +1531,7 @@ pub(super) fn visit_shadow_stack_root_slots(mut visit: impl FnMut(MutableRootSlo ptr, }); } - top = s.stack[header_base] as usize; + top = header.value as usize; } }); } diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index aa107d22e8..31746dfa67 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -1,5 +1,85 @@ -pub const SHADOW_STACK_HEADER_SLOTS: usize = 2; // prev_frame_top + slot_count -pub const SHADOW_STACK_GROW_RESERVE: usize = 1024; // initial capacity (slots) +//! Precise shadow-stack roots. +//! +//! # Layout +//! +//! One thread-local `Vec` holds every frame back-to-back. A frame +//! is `SHADOW_STACK_HEADER_SLOTS` header entries followed by its slots: +//! +//! ```text +//! [ header: value = caller frame_top, meta = slot_count ][ slot 0 ][ slot 1 ] ... +//! ^ frame_handle (base) ^ frame_top +//! ``` +//! +//! Before this rewrite the three per-slot words lived in three parallel `Vec`s +//! (`stack: Vec`, `slot_ptrs: Vec`, `active: Vec`) and the +//! header took two `u64` words. That cost every frame push five separate +//! capacity checks and three `resize` calls — two of which lowered to a +//! `memset` **call** for the 2–4-slot frames codegen actually emits — and cost +//! every slot store three independent bounds checks against three `Vec` +//! headers. Interleaving the three words into one 16-byte entry makes a push +//! one capacity check plus a handful of `stp`s, and a slot store one bounds +//! check against one length. + +use std::cell::UnsafeCell; +use std::sync::atomic::Ordering; + +/// Entries a frame reserves for its header. One [`ShadowEntry`] carries both +/// header words: `value` = the caller's `frame_top`, `meta` = this frame's +/// slot count. +pub const SHADOW_STACK_HEADER_SLOTS: usize = 1; +/// Entries the backing buffer reserves the first time it grows. +pub const SHADOW_STACK_GROW_RESERVE: usize = 1024; + +/// Liveness bit, stored in bit 0 of [`ShadowEntry::meta`]. +/// +/// A bound slot pointer is the address of a compiled `i64`/`double` local +/// slot and is therefore 8-byte aligned, so bit 0 is always free to carry the +/// liveness flag alongside it. [`js_shadow_slot_bind`] refuses to record a +/// pointer that would collide with the tag rather than truncating one. +pub(crate) const SLOT_ACTIVE: usize = 1; +/// Mask recovering the bound compiled-local address from `meta`. +pub(crate) const SLOT_PTR_MASK: usize = !SLOT_ACTIVE; + +/// One shadow-stack entry. +/// +/// `#[repr(C)]` with `value` first is load-bearing: the GC hands the visitor +/// `&mut entry.value` as a `*mut u64` root slot for unbound entries, so the +/// mirrored word must sit at offset 0 and be 8-byte aligned. 16 bytes also +/// makes indexing a shift rather than a multiply. +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct ShadowEntry { + /// The heap word the mutator stored. + /// + /// Raw bits rather than a typed pointer because slots hold NaN-boxed + /// JSValue bits (upper 16 bits are the tag, lower 48 the pointer) — the + /// GC tracer unwraps the NaN-box the same way it already does for closure + /// captures. + pub(crate) value: u64, + /// `bound_slot_address | SLOT_ACTIVE`. + /// + /// The address half is the compiled local/global slot this entry mirrors, + /// or 0 when the entry is unbound. When present, the GC reads and rewrites + /// the original slot, not the stale mirror copy. The `SLOT_ACTIVE` bit is + /// the liveness flag: it lets codegen stop reporting a dead local without + /// mutating the compiled local slot after last use. + pub(crate) meta: usize, +} + +impl ShadowEntry { + pub(crate) const EMPTY: ShadowEntry = ShadowEntry { value: 0, meta: 0 }; + + #[inline(always)] + pub(crate) fn is_active(self) -> bool { + self.meta & SLOT_ACTIVE != 0 + } + + /// The compiled local slot this entry mirrors, or null when unbound. + #[inline(always)] + pub(crate) fn bound_ptr(self) -> *mut u64 { + (self.meta & SLOT_PTR_MASK) as *mut u64 + } +} /// Combined shadow-stack state. Holding both fields in one TLS slot /// halves the macOS `tlv_get_addr` calls in every shadow-stack op @@ -18,58 +98,186 @@ pub const SHADOW_STACK_GROW_RESERVE: usize = 1024; // initial capacity (slots) /// while a GC walk is in progress (no allocation occurs inside the /// scanner/rewriter, and `GC_FLAG_IN_ALLOC` blocks reentrant GC). pub(crate) struct ShadowStackState { - /// `Vec` instead of `Vec<*mut u8>` because slots hold - /// NaN-boxed JSValue bits (upper 16 bits are the tag, lower 48 - /// the pointer) — the GC tracer unwraps the NaN-box the same way - /// it already does for closure captures. - pub(crate) stack: Vec, - /// Optional pointer to the compiled local/global slot represented by - /// each shadow-stack entry. When present, the GC reads and rewrites the - /// original slot, not a stale mirror copy. - pub(crate) slot_ptrs: Vec, - /// Liveness bit for each shadow slot. This lets codegen stop reporting a - /// dead local without mutating the compiled local slot after last use. - pub(crate) active: Vec, - /// Index into `stack` where the current frame's slot_0 lives. + /// Every frame's header + slots, back to back. + pub(crate) slots: Vec, + /// Index into `slots` where the current frame's slot 0 lives. /// `usize::MAX` when no frame is pushed (initial state + after /// the outermost function returns). pub(crate) frame_top: usize, } thread_local! { - pub(crate) static SHADOW: std::cell::UnsafeCell = - std::cell::UnsafeCell::new(ShadowStackState { - stack: Vec::with_capacity(SHADOW_STACK_GROW_RESERVE), - slot_ptrs: Vec::with_capacity(SHADOW_STACK_GROW_RESERVE), - active: Vec::with_capacity(SHADOW_STACK_GROW_RESERVE), + /// `const`-initialized so the access is a plain TLS address computation. + /// The buffer is reserved lazily on the first push instead of eagerly at + /// thread start. + pub(crate) static SHADOW: UnsafeCell = const { + UnsafeCell::new(ShadowStackState { + slots: Vec::new(), frame_top: usize::MAX, - }); + }) + }; +} + +/// Reserve room for `need` more entries. Outlined and `#[cold]` so the push +/// fast path stays a capacity compare and a not-taken branch. +#[cold] +#[inline(never)] +fn grow_for(s: &mut ShadowStackState, need: usize) { + s.slots.reserve(need.max(SHADOW_STACK_GROW_RESERVE)); +} + +/// Slots a push always zeroes, whether or not the frame declares that many. +/// +/// A *constant*-size clear is the point, and it took three attempts to get one +/// that survived the optimizer. Every length-dependent form — a `match` on `n` +/// with a spelled-out arm per size, a bounded loop, `write_bytes` with a +/// runtime `n` — is re-formed by LLVM into a compare chain that computes a byte +/// count and **calls `memset`**, which is the per-activation call this rewrite +/// exists to delete. Even the constant store below was tail-merged with the +/// large-frame `write_bytes` into a single `csel`-the-length-then-`bl memset` +/// until the large path moved behind `#[inline(never)]`. All three shapes were +/// read out of the linked archive's disassembly, not assumed. +/// +/// Four covers the frame sizes codegen actually emits and still lowers to a +/// pair of `stp q0, q0`. +const SHADOW_FRAME_ZERO_MIN: usize = 4; + +/// Zero the `n` freshly-claimed slot entries of a new frame. +/// +/// # Safety +/// `p` must point at `max(n, SHADOW_FRAME_ZERO_MIN)` writable, correctly +/// aligned `ShadowEntry`s. `js_shadow_frame_push` guarantees this by sizing +/// its capacity check with [`frame_zero_span`]; the entries past `n` are +/// inside the buffer's spare capacity and are re-zeroed by whichever push +/// claims them next. +#[inline(always)] +unsafe fn clear_slots(p: *mut ShadowEntry, n: usize) { + if n <= SHADOW_FRAME_ZERO_MIN { + std::ptr::write( + p.cast::<[ShadowEntry; SHADOW_FRAME_ZERO_MIN]>(), + [ShadowEntry::EMPTY; SHADOW_FRAME_ZERO_MIN], + ); + } else { + clear_large_frame_slots(p, n); + } +} + +/// Out-of-line so LLVM cannot tail-merge this variable-length `memset` with the +/// constant-length store in [`clear_slots`]. Frames this wide are rare enough +/// that the call is irrelevant to them and fatal to everything else. +/// +/// # Safety +/// `p` must point at `n` writable, correctly-aligned `ShadowEntry`s. +#[cold] +#[inline(never)] +unsafe fn clear_large_frame_slots(p: *mut ShadowEntry, n: usize) { + std::ptr::write_bytes(p, 0, n); +} + +/// Entries a push must have room for: the header, the declared slots, and the +/// over-zeroed tail [`clear_slots`] writes for small frames. +#[inline(always)] +fn frame_zero_span(slot_count: usize) -> usize { + SHADOW_STACK_HEADER_SLOTS + + if slot_count < SHADOW_FRAME_ZERO_MIN { + SHADOW_FRAME_ZERO_MIN + } else { + slot_count + } +} + +/// Encode a bound compiled-local address into a live [`ShadowEntry::meta`]. +/// +/// Bit 0 carries the liveness flag, so an address whose bit 0 is set cannot be +/// recorded. Rather than truncate it — which would hand the collector a +/// *different* address to write a forwarded pointer into — the binding is +/// dropped and the entry stays active-but-unbound: the mirrored value is still +/// marked and still rewritten, only the write-through to the compiled local is +/// lost. Compiled `i64`/`double` local slots are always 8-byte aligned, so the +/// fallback is unreachable in practice; it exists so a mis-emitted callsite +/// degrades instead of corrupting memory. Branchless (`tst`/`csel`), so the +/// aligned case pays nothing. +#[inline(always)] +pub(crate) fn bound_slot_meta(raw: usize) -> usize { + let bound = if raw & SLOT_ACTIVE == 0 { raw } else { 0 }; + bound | SLOT_ACTIVE +} + +/// Shade a value that was just stored into a shadow-stack root slot, if any +/// incremental mark cycle is in flight. +/// +/// # Why the gate is not a narrowing +/// +/// [`crate::gc::runtime_write_barrier_root_nanbox`] is an incremental-marking +/// (root shading) barrier, **not** the generational remembered-set barrier — +/// old→young edges are logged by the heap-slot barriers +/// (`runtime_write_barrier_slot` and friends), which this change does not +/// touch. Its whole body is `incremental_mark_barrier_value`, which reads the +/// thread-local `INCREMENTAL_MARK_BARRIER_VALID_PTRS` and returns immediately +/// when it is null. +/// +/// `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` counts the threads whose +/// thread-local pointer is currently non-null. `incremental_mark_barrier_enable` +/// installs the thread-local *and then* increments the count, both before +/// returning to the mutator, so on any thread: +/// +/// > this thread's `VALID_PTRS` is non-null ⟹ the count is ≥ 1 +/// +/// Therefore a zero count proves this thread's pointer is null, i.e. proves +/// the call would have returned `false` without doing anything. Skipping it is +/// observationally identical, not a weaker barrier. A non-zero count is +/// conservative in the harmless direction: another thread's cycle makes us +/// take the call, which then observes its own null pointer and returns. +/// +/// This is the same gate codegen already emits inline around +/// `js_write_barrier_root_nanbox` for persistent shadow slots +/// (`perry-codegen/src/expr/shadow_slot.rs::emit_persistent_shadow_root_barrier`); +/// this makes the runtime entry points agree with it. +#[inline(always)] +fn root_shading_barrier(value_bits: u64) { + if crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::SeqCst) != 0 { + shade_root_slot_value(value_bits); + } +} + +#[cold] +#[inline(never)] +fn shade_root_slot_value(value_bits: u64) { + crate::gc::runtime_write_barrier_root_nanbox(value_bits); } /// Push a new shadow-stack frame with `slot_count` live-pointer /// slots. Slots start zero-initialized (codegen fills them with -/// NaN-boxed pointer values via `js_shadow_slot_set`). Returns an -/// opaque `frame_handle` (the pre-push top index) that the matching -/// pop must be passed — lets the GC assert frame balance in debug -/// builds and detects codegen misemission. +/// NaN-boxed pointer values via `js_shadow_slot_set` / +/// `js_shadow_slot_bind`). Returns an opaque `frame_handle` (the pre-push +/// buffer length) that the matching pop must be passed — lets the GC assert +/// frame balance in debug builds and detects codegen misemission. /// -/// Not marked `#[inline(always)]` because it's called once per -/// function entry; the 3-line body inlines naturally. +/// The zero-fill is load-bearing, not hygiene: the buffer beyond the current +/// length still holds the *previous* frame's entries, so a frame that +/// inherited them would report dead slot values — stale addresses of +/// already-freed objects, or live bindings into a stack frame that has since +/// returned — as roots. #[no_mangle] pub extern "C" fn js_shadow_frame_push(slot_count: u32) -> u64 { SHADOW.with(|cell| unsafe { let s = &mut *cell.get(); - let prev_top = s.frame_top; - let base = s.stack.len(); - // Header: prev_frame_top + slot_count. Slots follow, - // initialized to 0 (GC_FLAG_NONE + null pointer). - s.stack.push(prev_top as u64); - s.stack.push(slot_count as u64); - let slots_start = s.stack.len(); - s.stack.resize(slots_start + slot_count as usize, 0); - s.slot_ptrs.resize(s.stack.len(), 0); - s.active.resize(s.stack.len(), false); - s.frame_top = slots_start; + let base = s.slots.len(); + let need = SHADOW_STACK_HEADER_SLOTS + slot_count as usize; + if frame_zero_span(slot_count as usize) > s.slots.capacity() - base { + grow_for(s, frame_zero_span(slot_count as usize)); + } + let header = s.slots.as_mut_ptr().add(base); + std::ptr::write( + header, + ShadowEntry { + value: s.frame_top as u64, + meta: slot_count as usize, + }, + ); + clear_slots(header.add(SHADOW_STACK_HEADER_SLOTS), slot_count as usize); + s.slots.set_len(base + need); + s.frame_top = base + SHADOW_STACK_HEADER_SLOTS; base as u64 }) } @@ -80,40 +288,36 @@ pub extern "C" fn js_shadow_frame_push(slot_count: u32) -> u64 { /// /// Robustness: the bounds check below was previously a `debug_assert!`, /// which is **compiled out in release builds**. A corrupted / out-of-range -/// `frame_handle` therefore reached `s.stack[base]` unchecked and aborted +/// `frame_handle` therefore reached the header entry unchecked and aborted /// the entire process with an out-of-bounds panic. This was observed on /// Windows release builds, where codegen could thread a NaN-boxed value /// (e.g. boxed `undefined`, `0x7FFC_0000_0000_0001`) into this `extern "C"` /// argument instead of the small index `js_shadow_frame_push` returned — -/// `js_shadow_frame_pop(9222246136947933185)` → `s.stack[huge]` → -/// hard crash a few seconds into startup. The shadow stack is Phase A -/// (built but not yet consumed by the GC tracer), so skipping a malformed -/// pop is memory-safe and GC-correctness-neutral; aborting the host -/// program is not. Promote the check to a real release-safe guard and -/// bail out — mirrors the bounds checks `js_shadow_slot_set` / -/// `js_shadow_slot_get` already perform on every access. +/// `js_shadow_frame_pop(9222246136947933185)` → out-of-range header read → +/// hard crash a few seconds into startup. Skipping a malformed pop is +/// memory-safe and GC-correctness-neutral (it leaves the frame installed, +/// which over-approximates the root set); aborting the host program is not. #[no_mangle] pub extern "C" fn js_shadow_frame_pop(frame_handle: u64) { SHADOW.with(|cell| unsafe { let s = &mut *cell.get(); let base = frame_handle as usize; - if base + SHADOW_STACK_HEADER_SLOTS > s.stack.len() { + // `base >= len`, not `base + HEADER_SLOTS > len`: the addition form + // wraps for a handle near `usize::MAX` and lets exactly the corrupted + // handles this guard exists for slip through into an unchecked read. + if base >= s.slots.len() { debug_assert!(false, "shadow-stack pop past end (corrupted frame handle)"); return; } - let prev_top = s.stack[base] as usize; - s.stack.truncate(base); - s.slot_ptrs.truncate(base); - s.active.truncate(base); - s.frame_top = prev_top; + s.frame_top = (*s.slots.as_ptr().add(base)).value as usize; + // `ShadowEntry: Copy`, so shrinking has no drop glue to run. + s.slots.set_len(base); }); } /// Update slot `idx` in the current frame with `value`. /// Codegen emits this at safepoints for each live pointer-typed -/// local. Hot path — compiled code calls this directly or inlines -/// an equivalent sequence; Rust version exists for runtime tests -/// and debug builds. +/// local, and for the `value = 0` "local is dead from here" clear. /// /// # Slot value contract (#6910) /// @@ -139,16 +343,24 @@ pub extern "C" fn js_shadow_slot_set(idx: u32, value: u64) { return; // no frame active — no-op } let slot = top + idx as usize; - if slot < s.stack.len() { - s.stack[slot] = value; - s.active[slot] = value != 0; - if value != 0 { - crate::gc::runtime_write_barrier_root_nanbox(value); - let ptr = s.slot_ptrs[slot] as *mut u64; - if !ptr.is_null() { - *ptr = value; - } - } + if slot >= s.slots.len() { + return; + } + let entry = s.slots.as_mut_ptr().add(slot); + let meta = (*entry).meta; + (*entry).value = value; + if value == 0 { + // Codegen's "dead from here" clear: drop the liveness bit but keep + // the binding, so a later re-activation still writes through to the + // same compiled local slot. + (*entry).meta = meta & SLOT_PTR_MASK; + return; + } + (*entry).meta = meta | SLOT_ACTIVE; + root_shading_barrier(value); + let bound = (meta & SLOT_PTR_MASK) as *mut u64; + if !bound.is_null() { + *bound = value; } }); } @@ -168,18 +380,32 @@ pub extern "C" fn js_shadow_slot_bind(idx: u32, value_slot: *mut u64) { return; } let slot = top + idx as usize; - if slot < s.stack.len() { - s.slot_ptrs[slot] = value_slot as usize; - s.stack[slot] = *value_slot; - s.active[slot] = true; - crate::gc::runtime_write_barrier_root_nanbox(*value_slot); + if slot >= s.slots.len() { + return; } + // Snapshot what the mutator has in the slot right now, and root that + // exact word. Never re-read it later at a safepoint: a re-read can + // observe a *subsequent* store and root the wrong value. + let value = *value_slot; + let raw = value_slot as usize; + debug_assert_eq!( + raw & SLOT_ACTIVE, + 0, + "bound compiled local slot must be 8-byte aligned" + ); + std::ptr::write( + s.slots.as_mut_ptr().add(slot), + ShadowEntry { + value, + meta: bound_slot_meta(raw), + }, + ); + root_shading_barrier(value); }); } -/// Read the current frame's slot `idx` — test-only; Phase B GC -/// tracer walks the raw Vec directly instead of going through a -/// function call per slot. +/// Read the current frame's slot `idx` — test-only; the GC tracer walks the +/// raw buffer directly instead of going through a function call per slot. #[no_mangle] pub extern "C" fn js_shadow_slot_get(idx: u32) -> u64 { SHADOW.with(|cell| unsafe { @@ -189,18 +415,17 @@ pub extern "C" fn js_shadow_slot_get(idx: u32) -> u64 { return 0; } let slot = top + idx as usize; - if slot < s.stack.len() { - if !s.active[slot] { - return 0; - } - let ptr = s.slot_ptrs[slot] as *const u64; - if ptr.is_null() { - s.stack[slot] - } else { - *ptr - } + let Some(entry) = s.slots.get(slot).copied() else { + return 0; + }; + if !entry.is_active() { + return 0; + } + let bound = entry.bound_ptr(); + if bound.is_null() { + entry.value } else { - 0 + *bound } }) } @@ -217,10 +442,10 @@ pub fn shadow_stack_depth() -> usize { while top != usize::MAX && top >= SHADOW_STACK_HEADER_SLOTS { depth += 1; let header_base = top - SHADOW_STACK_HEADER_SLOTS; - if header_base >= s.stack.len() { + if header_base >= s.slots.len() { break; } - top = s.stack[header_base] as usize; + top = s.slots[header_base].value as usize; } depth }) @@ -244,8 +469,8 @@ pub(crate) fn shadow_stack_has_active_frame() -> bool { /// callee frame. Until the unwinding function eventually returns, any GC /// that scans roots (`visit_shadow_stack_root_slots`) would walk those /// orphaned frames, reading — and, on the copying/evacuating path, -/// *writing back into* — `slot_ptrs` that point into stack memory that -/// has already been unwound and is being reused by the catch body. +/// *writing back into* — bound slot pointers that point into stack memory +/// that has already been unwound and is being reused by the catch body. /// /// The same reasoning applies to the temp-root stack (#6951): generated code /// pushes an expression temporary, evaluates something that throws, and never @@ -277,7 +502,7 @@ pub(crate) fn shadow_stack_savepoint() -> ShadowSavepoint { let s = &*cell.get(); ShadowSavepoint { frame_top: s.frame_top, - len: s.stack.len(), + len: s.slots.len(), temp_roots: super::temp_roots::temp_root_depth(), } }) @@ -296,10 +521,8 @@ pub(crate) fn shadow_stack_savepoint() -> ShadowSavepoint { pub(crate) fn shadow_stack_restore(sp: ShadowSavepoint) { SHADOW.with(|cell| unsafe { let s = &mut *cell.get(); - if sp.len <= s.stack.len() { - s.stack.truncate(sp.len); - s.slot_ptrs.truncate(sp.len); - s.active.truncate(sp.len); + if sp.len <= s.slots.len() { + s.slots.truncate(sp.len); } s.frame_top = sp.frame_top; }); diff --git a/crates/perry-runtime/src/gc/tests/debt_pacer.rs b/crates/perry-runtime/src/gc/tests/debt_pacer.rs index 74f2894ab2..3e3dcb0971 100644 --- a/crates/perry-runtime/src/gc/tests/debt_pacer.rs +++ b/crates/perry-runtime/src/gc/tests/debt_pacer.rs @@ -581,8 +581,8 @@ fn atomic_finalize_remark_rescues_pointer_hidden_in_shadow_slot_after_root_scan( SHADOW.with(|cell| unsafe { let st = &mut *cell.get(); let slot = st.frame_top + 1; - st.stack[slot] = string_bits(hidden); - st.active[slot] = true; + st.slots[slot].value = string_bits(hidden); + st.slots[slot].meta |= SLOT_ACTIVE; }); let completed = complete_budgeted_gc_cycle(); diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 33c1fefb32..a5940470cb 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -20,6 +20,7 @@ mod os_tag; mod root_words; mod roots; mod runtime_roots; +mod shadow_stack_ops; mod smoke; pub(super) mod support; mod teardown; diff --git a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs new file mode 100644 index 0000000000..7e103db523 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs @@ -0,0 +1,471 @@ +//! Teeth for the interleaved shadow-stack entry layout and the gated root +//! shading barrier. +//! +//! Every test here is written so that removing the thing it covers makes it +//! fail. The four properties under test are the ones a shadow-stack +//! optimisation can silently break: +//! +//! 1. **Liveness** — the collector marks what is in the slot. +//! 2. **Rewritability** — an evacuating collection updates the slot in place +//! and the *reader* observes the moved address. +//! 3. **Observed value** — the mirrored word is the one the mutator stored, +//! not a re-read taken later. +//! 4. **No stale roots** — a fresh frame never inherits the previous frame's +//! values or bindings out of the recycled buffer tail. + +use super::super::*; +use super::support::*; +use std::sync::atomic::Ordering; + +/// Frame handles for a nest of frames, popped in reverse on drop. +struct FrameNest(Vec); + +impl FrameNest { + fn push(&mut self, slot_count: u32) -> u64 { + let h = js_shadow_frame_push(slot_count); + self.0.push(h); + h + } +} + +impl Drop for FrameNest { + fn drop(&mut self) { + while let Some(h) = self.0.pop() { + js_shadow_frame_pop(h); + } + } +} + +fn scanner_slot_ptrs() -> Vec<*mut u64> { + let mut out = Vec::new(); + visit_shadow_stack_root_slots(|slot| out.push(slot.ptr)); + out +} + +fn scanner_slot_values() -> Vec { + let mut out = Vec::new(); + visit_shadow_stack_root_slots(|slot| out.push(unsafe { slot.read() })); + out +} + +// --------------------------------------------------------------------------- +// 4. No stale roots: a fresh frame must not inherit the recycled buffer tail. +// --------------------------------------------------------------------------- + +/// Sabotage check: delete the `clear_slots` call in `js_shadow_frame_push` +/// and this fails — the second frame reports the first frame's four dead +/// pointer words as live roots. +#[test] +fn fresh_frame_does_not_inherit_popped_frame_slot_values() { + let _guard = GcTestIsolationGuard::new(); + // 4 slots hits the unrolled arm of `clear_slots`, 9 hits the `write_bytes` + // arm. Both must clear. + for slot_count in [1u32, 2, 3, 4, 9, 33] { + reset_shadow_stack(); + let dead = js_shadow_frame_push(slot_count); + for i in 0..slot_count { + js_shadow_slot_set(i, 0x7FFD_0000_DEAD_0000 | u64::from(i)); + } + assert_eq!( + scanner_slot_values().len(), + slot_count as usize, + "{slot_count}-slot frame should report every set slot" + ); + js_shadow_frame_pop(dead); + + let fresh = js_shadow_frame_push(slot_count); + assert_eq!( + scanner_slot_values(), + Vec::::new(), + "a fresh {slot_count}-slot frame must report no roots; it reused the \ + buffer the popped frame wrote" + ); + for i in 0..slot_count { + assert_eq!( + js_shadow_slot_get(i), + 0, + "fresh frame slot {i} of {slot_count} must read back as empty" + ); + } + js_shadow_frame_pop(fresh); + } +} + +/// The `meta` half must be cleared too, not just `value`. A surviving binding +/// points at the *caller's* stack storage, which by then belongs to a +/// different function — the collector would read a root out of it and, on the +/// evacuating path, write a forwarded pointer back into it. +/// +/// Sabotage check: clear only `ShadowEntry::value` in `clear_slots` and the +/// write-through assertion below fails. +#[test] +fn fresh_frame_does_not_inherit_popped_frame_bindings() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + + let mut stale_storage: u64 = 0x7FFD_0000_1111_1111; + let dead = js_shadow_frame_push(2); + js_shadow_slot_bind(0, &mut stale_storage as *mut u64); + assert_eq!(js_shadow_slot_get(0), 0x7FFD_0000_1111_1111); + js_shadow_frame_pop(dead); + + let fresh = js_shadow_frame_push(2); + assert!( + scanner_slot_ptrs().is_empty(), + "fresh frame must not expose the popped frame's bound storage" + ); + // A write into the fresh frame must land in the mirror only. + js_shadow_slot_set(0, 0x7FFD_0000_2222_2222); + assert_eq!( + stale_storage, 0x7FFD_0000_1111_1111, + "fresh frame inherited a stale binding and wrote through it" + ); + assert_eq!(js_shadow_slot_get(0), 0x7FFD_0000_2222_2222); + let ptrs = scanner_slot_ptrs(); + assert_eq!(ptrs.len(), 1); + assert_ne!( + ptrs[0], &mut stale_storage as *mut u64, + "scanner handed out the popped frame's bound storage" + ); + js_shadow_frame_pop(fresh); +} + +// --------------------------------------------------------------------------- +// Liveness bit / binding encoding round-trip. +// --------------------------------------------------------------------------- + +/// The liveness flag and the bound address share one word. Clearing a slot +/// must drop only the flag: codegen re-activates the same slot later and still +/// expects the write-through to reach the original compiled local. +/// +/// Sabotage check: write `meta = 0` instead of `meta & SLOT_PTR_MASK` on the +/// clear path and the final write-through assertion fails. +#[test] +fn clearing_a_bound_slot_keeps_the_binding_for_later_reactivation() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let h = js_shadow_frame_push(1); + let mut storage: u64 = ptr_bits(0x1234_5678); + + js_shadow_slot_bind(0, &mut storage as *mut u64); + assert_eq!(scanner_slot_ptrs(), vec![&mut storage as *mut u64]); + + js_shadow_slot_set(0, 0); + assert_eq!(js_shadow_slot_get(0), 0, "cleared slot reads as empty"); + assert!( + scanner_slot_ptrs().is_empty(), + "cleared slot must not be scanned" + ); + assert_eq!( + storage, + ptr_bits(0x1234_5678), + "clearing must not write through to the compiled local" + ); + + js_shadow_slot_set(0, ptr_bits(0xABCD_EF00)); + assert_eq!( + storage, + ptr_bits(0xABCD_EF00), + "re-activated slot must still write through its retained binding" + ); + assert_eq!( + scanner_slot_ptrs(), + vec![&mut storage as *mut u64], + "re-activated slot must be scanned through the compiled local, not the mirror" + ); + js_shadow_frame_pop(h); +} + +/// Bit 0 of `meta` carries the liveness flag, so an odd address cannot be +/// stored there. Truncating it and letting the collector write a forwarded +/// pointer into `addr & !1` would corrupt whatever lives there; the encoder +/// therefore drops the binding and keeps the entry active-but-unbound, which +/// still marks and still rewrites the mirrored word. +/// +/// Sabotage check: replace the encoder body with `raw | SLOT_ACTIVE` and the +/// odd-address case starts reporting a truncated pointer. +#[test] +fn bound_meta_encoding_rejects_addresses_that_would_clobber_the_liveness_bit() { + for aligned in [0usize, 8, 0x1_0000, usize::MAX & SLOT_PTR_MASK] { + let meta = bound_slot_meta(aligned); + assert_eq!(meta & SLOT_ACTIVE, SLOT_ACTIVE, "must be active"); + assert_eq!( + meta & SLOT_PTR_MASK, + aligned, + "aligned address must round-trip exactly" + ); + } + for misaligned in [1usize, 9, 0x1_0001] { + let meta = bound_slot_meta(misaligned); + assert_eq!(meta & SLOT_ACTIVE, SLOT_ACTIVE, "must still be active"); + assert_eq!( + meta & SLOT_PTR_MASK, + 0, + "misaligned address must be dropped, never truncated" + ); + } +} + +// --------------------------------------------------------------------------- +// 1./2. Liveness and rewritability across a real evacuating collection. +// --------------------------------------------------------------------------- + +/// A value reachable only from a bound shadow slot must survive a copying +/// minor GC, and the *compiled local* — not just the mirror — must be +/// rewritten to the new address. +/// +/// Sabotage check: hand the mirror address to the visitor unconditionally in +/// `visit_shadow_stack_root_slots` and `storage` keeps pointing at from-space. +#[test] +fn bound_slot_survives_and_is_rewritten_by_a_copying_minor() { + let _guard = CopyingNurseryTestGuard::new(1); + let child = young_leaf(); + let mut storage: u64 = ptr_bits(child); + js_shadow_slot_bind(0, &mut storage as *mut u64); + + let _ = gc_collect_minor(); + + let moved = (storage & POINTER_MASK) as usize; + assert_ne!(moved, 0, "bound local was cleared by the collection"); + assert!( + crate::arena::pointer_in_nursery(moved) || crate::arena::pointer_in_old_gen(moved), + "bound local must hold a live heap address after collection" + ); + assert_eq!( + js_shadow_slot_get(0), + storage, + "slot read must observe the rewritten compiled local" + ); + assert_ne!(moved, child, "test did not actually evacuate the object"); +} + +/// The same property for an unbound slot: the mirror word itself is the root +/// slot, so it must be rewritten in place. +#[test] +fn unbound_slot_survives_and_is_rewritten_by_a_copying_minor() { + let _guard = CopyingNurseryTestGuard::new(1); + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + + let _ = gc_collect_minor(); + + let moved = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(moved, child, "test did not actually evacuate the object"); + assert!(crate::arena::pointer_in_nursery(moved) || crate::arena::pointer_in_old_gen(moved)); +} + +/// `js_shadow_slot_bind` must root the word the mutator has in the slot at the +/// moment of the call. Re-reading the compiled local at a later safepoint +/// would root whatever a subsequent store put there — the exact miscompile +/// shape that made `new C(g, bump())` print the post-`bump` value. +#[test] +fn bind_roots_the_value_present_at_the_call_not_a_later_store() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let h = js_shadow_frame_push(2); + + let mut storage: u64 = ptr_bits(0xAAAA_0000); + js_shadow_slot_bind(0, &mut storage as *mut u64); + // Slot 1 is bound to a *different* cell; the mirrors must not alias. + let mut other: u64 = ptr_bits(0xBBBB_0000); + js_shadow_slot_bind(1, &mut other as *mut u64); + + // The mirrored word recorded at bind time is exactly what was there. + SHADOW.with(|cell| unsafe { + let s = &*cell.get(); + let top = s.frame_top; + assert_eq!(s.slots[top].value, ptr_bits(0xAAAA_0000)); + assert_eq!(s.slots[top + 1].value, ptr_bits(0xBBBB_0000)); + }); + + // A bound slot deliberately tracks later mutator stores through the + // binding — that is what the binding is *for* — so the scanner follows the + // compiled local, which is the storage the mutator will read after the + // safepoint. + storage = ptr_bits(0xCCCC_0000); + assert_eq!(storage, ptr_bits(0xCCCC_0000)); + assert_eq!(js_shadow_slot_get(0), ptr_bits(0xCCCC_0000)); + assert_eq!(js_shadow_slot_get(1), ptr_bits(0xBBBB_0000)); + assert_eq!( + scanner_slot_values(), + vec![ptr_bits(0xCCCC_0000), ptr_bits(0xBBBB_0000)] + ); + js_shadow_frame_pop(h); +} + +// --------------------------------------------------------------------------- +// The gated root shading barrier. +// --------------------------------------------------------------------------- + +/// The premise the gate rests on: whenever this thread's incremental mark +/// barrier is armed, `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` is +/// non-zero. If that ever stopped holding, a zero count would no longer prove +/// the barrier call is a no-op and the gate would start dropping shading. +#[test] +fn active_count_is_nonzero_whenever_this_threads_barrier_is_armed() { + let _guard = GcTestIsolationGuard::new(); + incremental_mark_barrier_disable(); + assert!(!incremental_mark_barrier_active()); + + let valid_ptrs = build_valid_pointer_set(); + let armed = IncrementalMarkBarrierTestGuard::new(&valid_ptrs); + assert!(incremental_mark_barrier_active()); + assert_ne!( + PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::SeqCst), + 0, + "armed barrier must be visible in the global gate" + ); + drop(armed); + assert!(!incremental_mark_barrier_active()); +} + +/// With a cycle in flight, a store into a shadow slot must still shade the +/// stored object — the gate only skips the call when no cycle exists. +/// +/// Sabotage check: invert the gate to `== 0` (or delete the barrier call) and +/// both assertions below fail. +#[test] +fn slot_set_and_bind_shade_the_stored_value_while_a_cycle_is_active() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + clear_marks(); + clear_mark_seeds(); + + let set_child = young_leaf(); + let bind_child = young_leaf(); + let valid_ptrs = build_valid_pointer_set(); + let _barrier = IncrementalMarkBarrierTestGuard::new(&valid_ptrs); + + let h = js_shadow_frame_push(2); + js_shadow_slot_set(0, ptr_bits(set_child)); + let mut storage: u64 = ptr_bits(bind_child); + js_shadow_slot_bind(1, &mut storage as *mut u64); + drain_incremental_mark_barrier_seeds(&valid_ptrs); + + assert_marked_user_ptr(set_child, "js_shadow_slot_set child"); + assert_marked_user_ptr(bind_child, "js_shadow_slot_bind child"); + + js_shadow_frame_pop(h); + clear_marks(); + clear_mark_seeds(); +} + +// --------------------------------------------------------------------------- +// Frame bookkeeping under the packed header. +// --------------------------------------------------------------------------- + +/// The header packs `prev_frame_top` and `slot_count` into one entry. Mixed +/// slot counts, including zero-slot frames, must still chain correctly and +/// leave every frame's slots visible to the scanner. +#[test] +fn nested_frames_with_mixed_slot_counts_chain_and_scan_correctly() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let counts: [u32; 8] = [0, 1, 5, 0, 2, 9, 3, 4]; + let mut nest = FrameNest(Vec::new()); + let mut expected: Vec = Vec::new(); + + for (frame, &count) in counts.iter().enumerate() { + nest.push(count); + assert_eq!(shadow_stack_depth(), frame + 1); + for i in 0..count { + let bits = 0x7FFD_0000_0000_0000 | ((frame as u64) << 16) | u64::from(i); + js_shadow_slot_set(i, bits); + expected.push(bits); + } + } + + let mut seen = scanner_slot_values(); + seen.sort_unstable(); + expected.sort_unstable(); + assert_eq!(seen, expected, "every frame's slots must be scanned"); + + // Popping restores each caller's own slot view. + for (frame, &count) in counts.iter().enumerate().rev() { + assert_eq!(shadow_stack_depth(), frame + 1); + for i in 0..count { + assert_eq!( + js_shadow_slot_get(i), + 0x7FFD_0000_0000_0000 | ((frame as u64) << 16) | u64::from(i), + "frame {frame} slot {i} was clobbered by a callee" + ); + } + js_shadow_frame_pop(nest.0.pop().expect("frame handle")); + } + assert_eq!(shadow_stack_depth(), 0); +} + +/// A push that outgrows the buffer must reallocate before the header and slot +/// writes land, and the frame chain must survive the move. +#[test] +fn frame_chain_survives_buffer_growth() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let mut nest = FrameNest(Vec::new()); + // Push well past SHADOW_STACK_GROW_RESERVE entries. + let frames = SHADOW_STACK_GROW_RESERVE; + for i in 0..frames { + nest.push(1); + js_shadow_slot_set(0, 0x7FFD_0000_0000_0000 | i as u64); + } + assert_eq!(shadow_stack_depth(), frames); + assert_eq!(scanner_slot_values().len(), frames); + for i in (0..frames).rev() { + assert_eq!(js_shadow_slot_get(0), 0x7FFD_0000_0000_0000 | i as u64); + js_shadow_frame_pop(nest.0.pop().expect("frame handle")); + } + assert_eq!(shadow_stack_depth(), 0); +} + +/// A savepoint/restore pair (the `longjmp` unwind path) must drop the orphaned +/// frames *and* leave the buffer in a state where the next push still starts +/// from cleared entries. +#[test] +fn savepoint_restore_drops_orphaned_frames_without_leaving_stale_roots() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let outer = js_shadow_frame_push(2); + js_shadow_slot_set(0, 0x7FFD_0000_0000_00AA); + + let sp = shadow_stack_savepoint(); + let mut orphan_storage: u64 = 0x7FFD_0000_0000_00BB; + let _inner = js_shadow_frame_push(3); + js_shadow_slot_bind(0, &mut orphan_storage as *mut u64); + js_shadow_slot_set(1, 0x7FFD_0000_0000_00CC); + assert_eq!(shadow_stack_depth(), 2); + + shadow_stack_restore(sp); + assert_eq!(shadow_stack_depth(), 1); + assert_eq!(scanner_slot_values(), vec![0x7FFD_0000_0000_00AA]); + + // The catch body pushes its own frame over the abandoned storage. + let after = js_shadow_frame_push(3); + assert_eq!(scanner_slot_values(), vec![0x7FFD_0000_0000_00AA]); + js_shadow_slot_set(0, 0x7FFD_0000_0000_00DD); + assert_eq!( + orphan_storage, 0x7FFD_0000_0000_00BB, + "restored frame must not still be bound to the unwound frame's storage" + ); + js_shadow_frame_pop(after); + js_shadow_frame_pop(outer); +} + +/// A malformed `frame_handle` must be ignored rather than panicking the host +/// process (the Windows release crash this guard was added for). +#[test] +fn out_of_range_frame_pop_is_ignored() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let h = js_shadow_frame_push(2); + js_shadow_slot_set(0, 0x7FFD_0000_0000_0001); + // A NaN-boxed `undefined` threaded in where the handle belongs, plus the + // two handles that would wrap a `base + HEADER_SLOTS` bounds check and slip + // past it into an unchecked read. + for bogus in [0x7FFC_0000_0000_0001u64, u64::MAX, u64::MAX - 1] { + js_shadow_frame_pop(bogus); + assert_eq!(shadow_stack_depth(), 1, "frame must still be installed"); + } + assert_eq!(js_shadow_slot_get(0), 0x7FFD_0000_0000_0001); + js_shadow_frame_pop(h); + assert_eq!(shadow_stack_depth(), 0); +} diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index c0a4ef1028..2332b0a657 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -6,9 +6,7 @@ static YOUNG_LEAF_COUNTER: AtomicUsize = AtomicUsize::new(0); pub(super) fn reset_shadow_stack() { SHADOW.with(|cell| unsafe { let s = &mut *cell.get(); - s.stack.clear(); - s.slot_ptrs.clear(); - s.active.clear(); + s.slots.clear(); s.frame_top = usize::MAX; }); }