From c6463b860dc278f2f54794f1923984b1a47e526b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 20:03:21 +0200 Subject: [PATCH 1/6] wip(gc): repr(C) shadow-stack state + js_shadow_frame_enter Prepares the inline slot store: Vec layout is unspecified and cannot be a codegen contract, so the three buffer words become explicit #[repr(C)] fields with published, offset_of!-asserted offsets. Dropping the Vec also drops the TLS drop glue (and its per-op lazy-registration check). Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- crates/perry-runtime/src/gc/roots.rs | 10 +- .../src/gc/roots/shadow_stack.rs | 302 +++++++++++++++--- .../perry-runtime/src/gc/tests/debt_pacer.rs | 4 +- .../src/gc/tests/shadow_stack_ops.rs | 4 +- crates/perry-runtime/src/gc/tests/support.rs | 2 +- 5 files changed, 272 insertions(+), 50 deletions(-) diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index 39bb0c9be5..ab9812c95a 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -1495,22 +1495,22 @@ 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.slots.is_empty() { + if s.len == 0 || s.ptr.is_null() { 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 >= s.slots.len() { + if header_base >= s.len { break; } - let header = s.slots[header_base]; + let header = *s.ptr.add(header_base); let slot_count = header.meta; let slots_end = top + slot_count; - if slots_end > s.slots.len() { + if slots_end > s.len { break; } - let base = s.slots.as_mut_ptr().add(top); + let base = s.ptr.add(top); for i in 0..slot_count { let entry = base.add(i); if !(*entry).is_active() { diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index 31746dfa67..a5379c80c2 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -81,7 +81,7 @@ impl ShadowEntry { } } -/// Combined shadow-stack state. Holding both fields in one TLS slot +/// Combined shadow-stack state. Holding every field in one TLS slot /// halves the macOS `tlv_get_addr` calls in every shadow-stack op /// (push / pop / slot_set / slot_get / scanner) — those ops fired /// ~3 M+ times per perf-comprehensive run, and TLS access was the @@ -97,33 +97,200 @@ impl ShadowEntry { /// re-enter the runtime through a path that would touch this state /// 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 { - /// Every frame's header + slots, back to back. - pub(crate) slots: Vec, - /// Index into `slots` where the current frame's slot 0 lives. +/// +/// # Why this is `#[repr(C)]` with a hand-rolled buffer instead of a `Vec` +/// +/// Generated code addresses these fields **inline** (#7086): a slot store is an +/// address computation and a `stp` against this struct rather than a call into +/// [`js_shadow_slot_set`] / [`js_shadow_slot_bind`]. That requires the field +/// offsets to be a stable, checkable contract, and `Vec`'s layout is explicitly +/// *not* one — `RawVec`'s field order is unspecified and does move. Read out of +/// the shipped `aarch64` archive at the time of writing, the `Vec` form put +/// `cap` at 0, `ptr` at 8 and `len` at 16; nothing promises that stays true, and +/// a silent reorder would have codegen writing GC roots through the wrong word. +/// +/// Splitting the three words out explicitly also drops the type's drop glue, +/// which is what made `thread_local!` emit a lazy destructor-registration check +/// (`ldrb`/`cmp`/`b.eq`, plus a `panic_access_error` edge) on the front of every +/// shadow-stack op. The buffer is still freed at thread exit, by +/// [`ShadowBufferGuard`] rather than by `Vec`'s `Drop`. +/// +/// The offsets are published as [`SHADOW_STATE_PTR_OFFSET`], +/// [`SHADOW_STATE_LEN_OFFSET`] and [`SHADOW_STATE_FRAME_TOP_OFFSET`], asserted +/// against `offset_of!` below, and asserted equal to codegen's copy by +/// `perry`'s `shadow_layout_contract` test. +#[repr(C)] +pub struct ShadowStackState { + /// Base of the entry buffer. Null until the first push grows it. + pub(crate) ptr: *mut ShadowEntry, + /// Entries in use (header + slots of every live frame, back to back). + pub(crate) len: usize, + /// Entries the allocation can hold. + pub(crate) cap: usize, + /// Index into the buffer 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, } +/// Byte offset of [`ShadowStackState::ptr`]. Part of the codegen contract. +pub const SHADOW_STATE_PTR_OFFSET: usize = 0; +/// Byte offset of [`ShadowStackState::len`]. Part of the codegen contract. +pub const SHADOW_STATE_LEN_OFFSET: usize = 8; +/// Byte offset of [`ShadowStackState::frame_top`]. Part of the codegen +/// contract. +pub const SHADOW_STATE_FRAME_TOP_OFFSET: usize = 24; +/// Size of one [`ShadowEntry`]. Part of the codegen contract: generated code +/// indexes the buffer by shifting, so this must stay a power of two. +pub const SHADOW_ENTRY_SIZE: usize = 16; +/// Byte offset of [`ShadowEntry::meta`] within an entry. Part of the codegen +/// contract. +pub const SHADOW_ENTRY_META_OFFSET: usize = 8; +/// [`SLOT_ACTIVE`] as a public constant, for the codegen contract. +pub const SHADOW_SLOT_ACTIVE_BIT: usize = SLOT_ACTIVE; + +const _: () = { + assert!(std::mem::offset_of!(ShadowStackState, ptr) == SHADOW_STATE_PTR_OFFSET); + assert!(std::mem::offset_of!(ShadowStackState, len) == SHADOW_STATE_LEN_OFFSET); + assert!(std::mem::offset_of!(ShadowStackState, frame_top) == SHADOW_STATE_FRAME_TOP_OFFSET); + assert!(std::mem::size_of::() == SHADOW_ENTRY_SIZE); + assert!(std::mem::align_of::() == 8); + assert!(std::mem::offset_of!(ShadowEntry, value) == 0); + assert!(std::mem::offset_of!(ShadowEntry, meta) == SHADOW_ENTRY_META_OFFSET); + // Generated code computes `entry = ptr + slot * SHADOW_ENTRY_SIZE` with a + // shift, and the GC hands out `&mut entry.value` as a `*mut u64` root slot. + assert!(SHADOW_ENTRY_SIZE.is_power_of_two()); + // `meta`'s bit 0 is the liveness tag, so it must not overlap a slot pointer. + assert!(SHADOW_SLOT_ACTIVE_BIT == 1); + // Drop glue on the TLS type is what forces the lazy-registration check the + // inline sequence exists to avoid; keep it absent. + assert!(!std::mem::needs_drop::()); +}; + +impl ShadowStackState { + /// The live entries as a slice. Empty (and never dereferencing a null + /// `ptr`) before the first growth. + #[inline(always)] + pub(crate) fn slots(&self) -> &[ShadowEntry] { + if self.ptr.is_null() { + return &[]; + } + // SAFETY: once `ptr` is non-null it covers `cap >= len` entries, and + // every write path sets `len` only after initializing the range. + unsafe { std::slice::from_raw_parts(self.ptr, self.len) } + } + + /// The live entries as a mutable slice. + #[allow(dead_code)] + #[inline(always)] + pub(crate) fn slots_mut(&mut self) -> &mut [ShadowEntry] { + if self.ptr.is_null() { + return &mut []; + } + // SAFETY: as [`ShadowStackState::slots`]. + unsafe { std::slice::from_raw_parts_mut(self.ptr, self.len) } + } + + /// Drop every frame, keeping the buffer. Test/reset helper. + #[allow(dead_code)] + #[inline(always)] + pub(crate) fn clear_slots_for_reset(&mut self) { + self.len = 0; + } + + /// # Safety + /// `new_len <= cap`, and entries in `0..new_len` are initialized. + #[inline(always)] + pub(crate) unsafe fn set_slots_len(&mut self, new_len: usize) { + debug_assert!(new_len <= self.cap); + self.len = new_len; + } +} + thread_local! { - /// `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. + /// `const`-initialized and drop-free, so the access is a plain TLS address + /// computation with no lazy-init or destructor-registration check. The + /// buffer is reserved lazily on the first push instead of eagerly at thread + /// start, and released by [`ShadowBufferGuard`]. pub(crate) static SHADOW: UnsafeCell = const { UnsafeCell::new(ShadowStackState { - slots: Vec::new(), + ptr: std::ptr::null_mut(), + len: 0, + cap: 0, frame_top: usize::MAX, }) }; } +/// Frees the shadow buffer at thread exit. +/// +/// [`ShadowStackState`] is deliberately drop-free (see its docs), so the +/// allocation is owned by this separate thread-local instead. It is armed from +/// the cold [`grow_for`] path, so a thread that never pushes a frame never +/// registers a destructor. +/// +/// The destructor resets the state to the *empty* sentinel rather than leaving +/// a dangling `ptr`, so any shadow-stack op that runs after this point sees +/// "no frame, no buffer" and no-ops. That is strictly safer than the previous +/// behaviour: a `Vec` inside the TLS made the whole slot `DESTROYED`, and the +/// next access aborted through `std`'s `panic_access_error`. +struct ShadowBufferGuard; + +impl Drop for ShadowBufferGuard { + fn drop(&mut self) { + // `SHADOW` is drop-free, so it is never itself destroyed and this + // access cannot fail regardless of destructor order. + let _ = SHADOW.try_with(|cell| unsafe { + let s = &mut *cell.get(); + let (ptr, cap) = (s.ptr, s.cap); + s.ptr = std::ptr::null_mut(); + s.len = 0; + s.cap = 0; + s.frame_top = usize::MAX; + if !ptr.is_null() && cap != 0 { + drop(Vec::from_raw_parts(ptr, 0, cap)); + } + }); + } +} + +thread_local! { + static SHADOW_BUFFER_GUARD: ShadowBufferGuard = const { ShadowBufferGuard }; +} + /// Reserve room for `need` more entries. Outlined and `#[cold]` so the push /// fast path stays a capacity compare and a not-taken branch. +/// +/// Growth reallocates the buffer, so `ShadowStackState::ptr` is **not** stable +/// across a push. Generated code therefore re-loads `ptr` from the state at +/// every inline slot store rather than caching a frame base address; only the +/// address of the state struct itself (a thread-local, fixed for the thread's +/// lifetime) is cached per activation. #[cold] #[inline(never)] fn grow_for(s: &mut ShadowStackState, need: usize) { - s.slots.reserve(need.max(SHADOW_STACK_GROW_RESERVE)); + // Arm the thread-exit free the first time this thread allocates. + let _ = SHADOW_BUFFER_GUARD.try_with(|_| ()); + let want = s + .len + .saturating_add(need.max(SHADOW_STACK_GROW_RESERVE)) + .max(s.cap.saturating_mul(2)); + // Round-trip through `Vec` so the allocation is made and freed with the + // same allocator and layout that `ShadowBufferGuard` releases. + let mut v: Vec = if s.ptr.is_null() { + Vec::new() + } else { + // SAFETY: `ptr`/`cap` came from a `Vec` built here. Length + // is passed as 0 because `ShadowEntry: Copy` has no drop glue, and the + // live prefix is copied by `reserve` from the raw allocation anyway — + // so re-declare the real length to keep the data. + unsafe { Vec::from_raw_parts(s.ptr, s.len, s.cap) } + }; + v.reserve(want.saturating_sub(v.len())); + s.ptr = v.as_mut_ptr(); + s.cap = v.capacity(); + s.len = v.len(); + std::mem::forget(v); } /// Slots a push always zeroes, whether or not the frame declares that many. @@ -262,26 +429,81 @@ fn shade_root_slot_value(value_bits: u64) { pub extern "C" fn js_shadow_frame_push(slot_count: u32) -> u64 { SHADOW.with(|cell| unsafe { let s = &mut *cell.get(); - 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 + push_frame(s, slot_count) + }) +} + +/// The body of [`js_shadow_frame_push`], factored out so +/// [`js_shadow_frame_enter`] shares it exactly rather than reimplementing it. +/// +/// # Safety +/// `s` must be the calling thread's shadow state. +#[inline(always)] +unsafe fn push_frame(s: &mut ShadowStackState, slot_count: u32) -> u64 { + let base = s.len; + let need = SHADOW_STACK_HEADER_SLOTS + slot_count as usize; + if frame_zero_span(slot_count as usize) > s.cap - base { + grow_for(s, frame_zero_span(slot_count as usize)); + } + let header = s.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.set_slots_len(base + need); + s.frame_top = base + SHADOW_STACK_HEADER_SLOTS; + base as u64 +} + +/// [`js_shadow_frame_push`], but returning the address of this thread's +/// [`ShadowStackState`] instead of the frame handle. +/// +/// Generated code calls this once per activation and keeps the pointer for the +/// whole frame, so the inline slot stores (#7086) are address arithmetic +/// against it rather than one `extern "C"` call — and one thread-local +/// lookup — per store. The frame handle the matching +/// [`js_shadow_frame_pop`] needs is recoverable without a second call: +/// `handle == frame_top - SHADOW_STACK_HEADER_SLOTS`. +/// +/// # Why caching the returned pointer is sound +/// +/// It is the address of a `thread_local!` whose type is drop-free and +/// `const`-initialized: fixed for the lifetime of the thread, allocated by the +/// TLS runtime before this function can return it, and never reallocated. The +/// *buffer* it points at does move (see [`grow_for`]), which is exactly why +/// only the state address is cached and `ptr`/`len`/`frame_top` are re-loaded +/// at every store. +/// +/// A cached pointer never outlives its thread: one activation of a compiled +/// function runs entirely on one thread (`perry/thread` hands a whole call to +/// a worker; a resumed async state machine re-enters through the function +/// entry and calls this again), so the pointer is refetched whenever the +/// executing thread can have changed. +/// +/// Returns a non-null pointer; the declaration codegen emits marks it as such. +#[no_mangle] +pub extern "C" fn js_shadow_frame_enter(slot_count: u32) -> *mut ShadowStackState { + SHADOW.with(|cell| unsafe { + let s = &mut *cell.get(); + push_frame(s, slot_count); + s as *mut ShadowStackState }) } +/// The address of this thread's [`ShadowStackState`], without pushing a frame. +/// +/// Used by generated code for functions that need inline slot access but whose +/// frame was pushed elsewhere, and by tests that exercise the inline addressing +/// contract from Rust. +#[no_mangle] +pub extern "C" fn js_shadow_state_addr() -> *mut ShadowStackState { + SHADOW.with(|cell| cell.get()) +} + /// Pop the current shadow-stack frame. `frame_handle` must match /// the return value of the matching `js_shadow_frame_push`. Restores /// the prior `SHADOW.frame_top`. @@ -305,13 +527,13 @@ pub extern "C" fn js_shadow_frame_pop(frame_handle: u64) { // `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() { + if base >= s.len { debug_assert!(false, "shadow-stack pop past end (corrupted frame handle)"); return; } - s.frame_top = (*s.slots.as_ptr().add(base)).value as usize; + s.frame_top = (*s.ptr.add(base)).value as usize; // `ShadowEntry: Copy`, so shrinking has no drop glue to run. - s.slots.set_len(base); + s.set_slots_len(base); }); } @@ -343,10 +565,10 @@ 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.slots.len() { + if slot >= s.len { return; } - let entry = s.slots.as_mut_ptr().add(slot); + let entry = s.ptr.add(slot); let meta = (*entry).meta; (*entry).value = value; if value == 0 { @@ -380,7 +602,7 @@ pub extern "C" fn js_shadow_slot_bind(idx: u32, value_slot: *mut u64) { return; } let slot = top + idx as usize; - if slot >= s.slots.len() { + if slot >= s.len { return; } // Snapshot what the mutator has in the slot right now, and root that @@ -394,7 +616,7 @@ pub extern "C" fn js_shadow_slot_bind(idx: u32, value_slot: *mut u64) { "bound compiled local slot must be 8-byte aligned" ); std::ptr::write( - s.slots.as_mut_ptr().add(slot), + s.ptr.add(slot), ShadowEntry { value, meta: bound_slot_meta(raw), @@ -415,7 +637,7 @@ pub extern "C" fn js_shadow_slot_get(idx: u32) -> u64 { return 0; } let slot = top + idx as usize; - let Some(entry) = s.slots.get(slot).copied() else { + let Some(entry) = s.slots().get(slot).copied() else { return 0; }; if !entry.is_active() { @@ -442,10 +664,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.slots.len() { + if header_base >= s.len { break; } - top = s.slots[header_base].value as usize; + top = s.slots()[header_base].value as usize; } depth }) @@ -502,7 +724,7 @@ pub(crate) fn shadow_stack_savepoint() -> ShadowSavepoint { let s = &*cell.get(); ShadowSavepoint { frame_top: s.frame_top, - len: s.slots.len(), + len: s.len, temp_roots: super::temp_roots::temp_root_depth(), } }) @@ -521,8 +743,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.slots.len() { - s.slots.truncate(sp.len); + if sp.len <= s.len { + s.set_slots_len(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 3e3dcb0971..7e0e35e878 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.slots[slot].value = string_bits(hidden); - st.slots[slot].meta |= SLOT_ACTIVE; + st.slots_mut()[slot].value = string_bits(hidden); + st.slots_mut()[slot].meta |= SLOT_ACTIVE; }); let completed = complete_budgeted_gc_cycle(); diff --git a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs index 7e103db523..1a7c4ae3f0 100644 --- a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs +++ b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs @@ -274,8 +274,8 @@ fn bind_roots_the_value_present_at_the_call_not_a_later_store() { 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)); + 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 diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index 2332b0a657..ce4cc20bf7 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -6,7 +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.slots.clear(); + s.clear_slots_for_reset(); s.frame_top = usize::MAX; }); } From 3433e9bc1b95d0130eac9244a4f3e19060c3adb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 20:19:26 +0200 Subject: [PATCH 2/6] perf(gc): emit the shadow-slot root store inline Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- crates/perry-codegen/src/codegen/helpers.rs | 22 ++ crates/perry-codegen/src/expr/mod.rs | 1 + .../perry-codegen/src/expr/shadow_inline.rs | 294 ++++++++++++++++++ crates/perry-codegen/src/expr/shadow_slot.rs | 11 + crates/perry-codegen/src/function.rs | 87 +++++- crates/perry-codegen/src/lib.rs | 18 +- crates/perry-codegen/src/module.rs | 30 ++ .../perry-codegen/src/runtime_decls/arrays.rs | 9 + crates/perry-runtime/src/gc/roots.rs | 7 +- .../src/gc/tests/shadow_stack_ops.rs | 270 ++++++++++++++++ .../perry/src/commands/compile/build_cache.rs | 1 + .../src/commands/compile/object_cache.rs | 7 + crates/perry/src/main.rs | 2 + crates/perry/src/shadow_layout_contract.rs | 111 +++++++ 14 files changed, 859 insertions(+), 11 deletions(-) create mode 100644 crates/perry-codegen/src/expr/shadow_inline.rs create mode 100644 crates/perry/src/shadow_layout_contract.rs diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index 6b7580d29c..f998e4dc62 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -74,6 +74,28 @@ pub(super) fn shadow_stack_enabled() -> bool { }) } +/// Inline shadow-slot store gate (#7086). Default ON. +/// +/// When enabled, a store to a GC-rooted local is emitted as an address +/// computation and a pair of stores against this thread's `ShadowStackState` +/// instead of a call to `js_shadow_slot_bind` / `js_shadow_slot_set`. The +/// runtime entry points stay exported, and are what the emitted code falls +/// back to when no state pointer is available for the activation. +/// +/// `PERRY_INLINE_SHADOW_SLOT=0`/`off`/`false` reverts to the calls, for +/// bisection. Independent of `PERRY_SHADOW_STACK`, which switches root +/// emission off entirely; with the shadow stack off there is nothing to inline. +pub(crate) fn inline_shadow_slot_enabled() -> bool { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + !matches!( + std::env::var("PERRY_INLINE_SHADOW_SLOT").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ) + }) +} + /// Inline-hot-small gate. Default ON. When enabled, small functions /// (`INLINE_HOT_SMALL_MIN ..= SIZE_CAP` statements) that have ≥1 call site /// inside a loop get LLVM's `inlinehint` — a *bounded* nudge that raises the diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 7ebcaad4e1..7c05be499f 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -132,6 +132,7 @@ pub(crate) use write_barrier::{ mod dispatch; mod record_value; mod scalar_slot_root; +pub(crate) mod shadow_inline; mod shadow_slot; mod slot_rep; pub(crate) mod temp_root; diff --git a/crates/perry-codegen/src/expr/shadow_inline.rs b/crates/perry-codegen/src/expr/shadow_inline.rs new file mode 100644 index 0000000000..690745a1b9 --- /dev/null +++ b/crates/perry-codegen/src/expr/shadow_inline.rs @@ -0,0 +1,294 @@ +//! Inline shadow-slot stores (#7086). +//! +//! # What this replaces +//! +//! Every store to a GC-rooted local used to be an `extern "C"` call — +//! `js_shadow_slot_bind(idx, &local)` for a pointer-capable value, +//! `js_shadow_slot_set(idx, 0)` for the "dead from here" clear. #7079 made +//! those functions cheap internally, but a call costs twice: the call itself, +//! and the fact that it is **opaque to LLVM**, which forces a spill of every +//! live value around it and blocks hoisting across it. +//! +//! Read out of the shipped `aarch64` archive, `js_shadow_slot_bind`'s fast path +//! was ~35 instructions: a 4-instruction prologue/epilogue pair, a **TLSDESC +//! indirect call** (`adrp`/`ldr`/`add`/`blr` plus the resolver) to reach the +//! thread-local, a lazy destructor-registration check, then the six or so +//! instructions that actually do the work. The inline form below is those six +//! plus two guards, with no call and no TLS sequence at all. +//! +//! # How the thread-local block is reached +//! +//! Not by re-deriving the TLS address in generated code — that would have to +//! model Rust's TLS model per platform (TLSDESC on this Linux build, `tlv` on +//! macOS) and would be a second, unverified path to the same memory. +//! +//! Instead the address is *obtained from the runtime* and cached for the +//! activation. `js_shadow_frame_enter` is `js_shadow_frame_push` with the +//! address of this thread's `ShadowStackState` as its return value, so codegen +//! pays exactly the one TLS lookup per activation that the push already paid. +//! The pointer goes into an entry alloca; every slot store loads it back. +//! Because it comes from the runtime's own `SHADOW.with`, it is the same +//! memory `js_shadow_slot_set` writes, by construction rather than by +//! coincidence — and `js_shadow_state_addr` lets a Rust test poke the buffer +//! through these very offsets and read it back through the runtime accessor. +//! +//! Caching it is sound because it is the address of a `const`-initialized, +//! drop-free `thread_local!`: fixed for the thread's lifetime, never +//! reallocated. The *buffer* it points at does move when a deeper frame grows +//! it, which is exactly why `ptr`, `len` and `frame_top` are re-loaded from the +//! state at every store instead of a frame base being cached. One activation of +//! a compiled function runs entirely on one thread, so the cached pointer never +//! escapes to another. +//! +//! # The three root properties +//! +//! * **Liveness** — the inline store writes the same `ShadowEntry.value` and +//! sets the same `SLOT_ACTIVE` bit in `meta` that the runtime function does, +//! at the same index, so `visit_shadow_stack_root_slots` marks it identically. +//! * **Rewritability** — `meta` still carries the bound compiled-local address, +//! with the same alignment fallback, so an evacuating collection rewrites the +//! alloca the mutator reads after the safepoint, not just the mirror. +//! * **The value the mutator stored** — the value is read from the local slot +//! at the store site, in the same position the call occupied, and written +//! immediately. Nothing re-reads the slot at a later safepoint. +//! +//! The incremental-mark root shading barrier is emitted inline too, behind the +//! same `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` gate the runtime and +//! `emit_persistent_shadow_root_barrier` already use. + +use super::*; + +use crate::types::{I1, I32, I64, PTR}; + +/// Byte offset of `ShadowStackState::ptr`. +/// +/// This block mirrors `perry_runtime::gc::roots::SHADOW_STATE_*`. `perry-codegen` +/// deliberately does not depend on `perry-runtime`, so the two copies are +/// pinned together by `perry`'s `shadow_layout_contract` test, which does +/// depend on both and fails if either moves. +pub const SHADOW_STATE_PTR_OFFSET: u64 = 0; +/// Byte offset of `ShadowStackState::len`. +pub const SHADOW_STATE_LEN_OFFSET: u64 = 8; +/// Byte offset of `ShadowStackState::frame_top`. +pub const SHADOW_STATE_FRAME_TOP_OFFSET: u64 = 24; +/// `size_of::()`. A power of two, so indexing is a shift. +pub const SHADOW_ENTRY_SIZE: u64 = 16; +/// `log2(SHADOW_ENTRY_SIZE)`. +pub const SHADOW_ENTRY_SHIFT: u64 = 4; +/// Byte offset of `ShadowEntry::meta` within an entry. +pub const SHADOW_ENTRY_META_OFFSET: u64 = 8; +/// Liveness bit in `ShadowEntry::meta`. +pub const SHADOW_SLOT_ACTIVE_BIT: u64 = 1; +/// Header entries a frame reserves; `frame_top == frame_handle + this`. +pub const SHADOW_STACK_HEADER_SLOTS: u64 = 1; + +const _: () = { + assert!(SHADOW_ENTRY_SIZE == 1 << SHADOW_ENTRY_SHIFT); + assert!(SHADOW_ENTRY_META_OFFSET < SHADOW_ENTRY_SIZE); + assert!(SHADOW_SLOT_ACTIVE_BIT == 1); +}; + +/// What the inline sequence writes into the entry. +enum InlineSlotWrite<'a> { + /// Mirror `*local_slot` and bind the entry to `local_slot`. Equivalent to + /// `js_shadow_slot_bind(idx, local_slot)`. + Bind { local_slot: &'a str }, + /// Zero the value and drop the liveness bit, keeping the binding. + /// Equivalent to `js_shadow_slot_set(idx, 0)`. + Clear, +} + +/// Emit the inline equivalent of `js_shadow_slot_bind(slot_idx, local_slot)`. +/// +/// Returns `false` when this function has no cached state pointer (so the +/// caller must fall back to the `extern "C"` call). +pub(crate) fn emit_inline_slot_bind( + ctx: &mut FnCtx<'_>, + slot_idx: u32, + local_slot: &str, +) -> bool { + emit_inline_slot_write(ctx, slot_idx, InlineSlotWrite::Bind { local_slot }) +} + +/// Emit the inline equivalent of `js_shadow_slot_set(slot_idx, 0)`. +pub(crate) fn emit_inline_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) -> bool { + emit_inline_slot_write(ctx, slot_idx, InlineSlotWrite::Clear) +} + +fn emit_inline_slot_write( + ctx: &mut FnCtx<'_>, + slot_idx: u32, + what: InlineSlotWrite<'_>, +) -> bool { + if !crate::codegen::helpers::inline_shadow_slot_enabled() { + return false; + } + let Some(state_slot) = ctx.func.shadow_state_slot().map(str::to_owned) else { + return false; + }; + + // The slow arm is not dead code kept for tidiness: the state alloca is + // null-initialized in the entry block, so any path that could reach a slot + // store before the frame push ran still writes the root — through the + // runtime function, which does its own TLS lookup. Where the push provably + // dominates (every shipped path), `js_shadow_frame_enter`'s `nonnull` + // return lets LLVM fold this branch away entirely. + let slow_idx = ctx.new_block("ss.slow"); + let chk_top_idx = ctx.new_block("ss.chk_top"); + let chk_len_idx = ctx.new_block("ss.chk_len"); + let store_idx = ctx.new_block("ss.store"); + let done_idx = ctx.new_block("ss.done"); + let slow_label = ctx.block_label(slow_idx); + let chk_top_label = ctx.block_label(chk_top_idx); + let chk_len_label = ctx.block_label(chk_len_idx); + let store_label = ctx.block_label(store_idx); + let done_label = ctx.block_label(done_idx); + + let state = ctx.block().load(PTR, &state_slot); + let state_null = ctx.block().icmp_eq(PTR, &state, "null"); + ctx.block() + .cond_br(&state_null, &slow_label, &chk_top_label); + + // --- slow arm: the runtime function, byte-for-byte the old behaviour --- + ctx.current_block = slow_idx; + match what { + InlineSlotWrite::Bind { local_slot } => ctx.block().call_void( + "js_shadow_slot_bind", + &[(I32, &slot_idx.to_string()), (PTR, local_slot)], + ), + InlineSlotWrite::Clear => ctx.block().call_void( + "js_shadow_slot_set", + &[(I32, &slot_idx.to_string()), (I64, "0")], + ), + } + ctx.block().br(&done_label); + + // --- `if top == usize::MAX { return }` --- + ctx.current_block = chk_top_idx; + let frame_top_ptr = ctx.block().gep_inbounds( + crate::types::I8, + &state, + &[(I64, &SHADOW_STATE_FRAME_TOP_OFFSET.to_string())], + ); + let frame_top = ctx.block().load(I64, &frame_top_ptr); + // `usize::MAX` is `-1` as an i64 bit pattern. + let no_frame = ctx.block().icmp_eq(I64, &frame_top, "-1"); + ctx.block() + .cond_br(&no_frame, &done_label, &chk_len_label); + + // --- `let slot = top + idx; if slot >= len { return }` --- + // + // The `usize::MAX` test above is what makes this addition safe to do + // untrapped: with `top` ruled out as the sentinel it is a real buffer + // index, so `top + idx` cannot wrap. Testing only `slot < len` would let + // the sentinel through — `usize::MAX + idx` wraps to `idx - 1`, which is + // in bounds and would corrupt a *different* frame's entry. + ctx.current_block = chk_len_idx; + let slot = ctx + .block() + .add(I64, &frame_top, &u64::from(slot_idx).to_string()); + let len_ptr = ctx.block().gep_inbounds( + crate::types::I8, + &state, + &[(I64, &SHADOW_STATE_LEN_OFFSET.to_string())], + ); + let len = ctx.block().load(I64, &len_ptr); + let in_bounds = ctx.block().icmp_ult(I64, &slot, &len); + ctx.block() + .cond_br(&in_bounds, &store_label, &done_label); + + // --- the entry write --- + ctx.current_block = store_idx; + let buf = if SHADOW_STATE_PTR_OFFSET == 0 { + ctx.block().load(PTR, &state) + } else { + let p = ctx.block().gep_inbounds( + crate::types::I8, + &state, + &[(I64, &SHADOW_STATE_PTR_OFFSET.to_string())], + ); + ctx.block().load(PTR, &p) + }; + let byte_off = ctx + .block() + .shl(I64, &slot, &SHADOW_ENTRY_SHIFT.to_string()); + let entry = ctx + .block() + .gep_inbounds(crate::types::I8, &buf, &[(I64, &byte_off)]); + let meta_ptr = ctx.block().gep_inbounds( + crate::types::I8, + &entry, + &[(I64, &SHADOW_ENTRY_META_OFFSET.to_string())], + ); + + match what { + InlineSlotWrite::Bind { local_slot } => { + // Snapshot the word the mutator just stored. This load sits + // immediately after the caller's `store` to the same alloca, so + // LLVM forwards it; it is never re-read at a later safepoint. + let value = ctx.block().load(I64, local_slot); + let raw = ctx.block().ptrtoint(local_slot, I64); + // `bound_slot_meta`: an address whose bit 0 would collide with the + // liveness tag is recorded active-but-unbound rather than + // truncated, so the collector is never handed a mis-derived + // address to write a forwarded pointer into. Compiled local slots + // are `i64`/`double` allocas and so 8-byte aligned, which lets + // LLVM fold this select away; it exists for the mis-emitted case. + let low = ctx + .block() + .and(I64, &raw, &SHADOW_SLOT_ACTIVE_BIT.to_string()); + let aligned = ctx.block().icmp_eq(I64, &low, "0"); + let bound = ctx.block().select(I1, &aligned, I64, &raw, "0"); + let meta = ctx + .block() + .or(I64, &bound, &SHADOW_SLOT_ACTIVE_BIT.to_string()); + ctx.block().store(I64, &value, &entry); + ctx.block().store(I64, &meta, &meta_ptr); + emit_inline_root_shading_barrier(ctx, &value, &done_label); + } + InlineSlotWrite::Clear => { + // 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. No shading barrier — a zero value + // is not a heap reference. + let old_meta = ctx.block().load(I64, &meta_ptr); + let cleared = ctx.block().and( + I64, + &old_meta, + &format!("{}", !SHADOW_SLOT_ACTIVE_BIT as i64), + ); + ctx.block().store(I64, "0", &entry); + ctx.block().store(I64, &cleared, &meta_ptr); + ctx.block().br(&done_label); + } + } + + ctx.current_block = done_idx; + true +} + +/// The incremental-mark root shading barrier, gated inline on +/// `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT`. +/// +/// Identical in kind to `emit_persistent_shadow_root_barrier` and to the +/// runtime's own `root_shading_barrier`: a zero count *proves* this thread's +/// `INCREMENTAL_MARK_BARRIER_VALID_PTRS` is null, because +/// `incremental_mark_barrier_enable` installs the thread-local before +/// incrementing the count. Skipping the call on a zero count is therefore +/// observationally identical, not a weaker barrier. +/// +/// Terminates the current block with a branch to `done_label`. +fn emit_inline_root_shading_barrier(ctx: &mut FnCtx<'_>, value_bits: &str, done_label: &str) { + let active = + ctx.block() + .load_atomic_seq_cst(I32, "@PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT", 4); + let needed = ctx.block().icmp_ne(I32, &active, "0"); + let barrier_idx = ctx.new_block("ss.barrier"); + let barrier_label = ctx.block_label(barrier_idx); + ctx.block().cond_br(&needed, &barrier_label, done_label); + + ctx.current_block = barrier_idx; + ctx.block() + .call_void("js_write_barrier_root_nanbox", &[(I64, value_bits)]); + ctx.block().br(done_label); +} diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 3d1b2b70db..6dc95cd827 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -138,6 +138,11 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { if ctx.suppressed_cleared_shadow_slots.contains(&slot_idx) { return; } + // #7086: emitted inline against this activation's cached `ShadowStackState` + // pointer when it has one; falls through to the call otherwise. + if super::shadow_inline::emit_inline_slot_clear(ctx, slot_idx) { + return; + } ctx.block().call_void( "js_shadow_slot_set", &[(I32, &slot_idx.to_string()), (I64, "0")], @@ -192,6 +197,12 @@ pub(crate) fn emit_shadow_slot_bind_for_local(ctx: &mut FnCtx<'_>, local_id: u32 return; }; ctx.shadow_slots_bound.insert(slot_idx); + // #7086: the hot per-store root write. Emitted inline against this + // activation's cached `ShadowStackState` pointer when it has one; falls + // through to the call otherwise. + if super::shadow_inline::emit_inline_slot_bind(ctx, slot_idx, &local_slot) { + return; + } ctx.block().call_void( "js_shadow_slot_bind", &[(I32, &slot_idx.to_string()), (PTR, &local_slot)], diff --git a/crates/perry-codegen/src/function.rs b/crates/perry-codegen/src/function.rs index b2c7509a87..3fb1967769 100644 --- a/crates/perry-codegen/src/function.rs +++ b/crates/perry-codegen/src/function.rs @@ -103,6 +103,10 @@ pub struct LlFunction { /// land wiring incrementally (e.g. just `main`) before /// flipping the default across every user function. shadow_frame_slot: Option, + /// Entry alloca holding this thread's `ShadowStackState` address, so the + /// inline slot stores (#7086) can address the buffer without a per-store + /// thread-local lookup. Set alongside `shadow_frame_slot`. + shadow_state_slot: Option, /// Whether shadow-frame emission was requested for this function at all /// (i.e. `enable_shadow_frame` / `enable_post_init_shadow_frame` ran). /// @@ -131,13 +135,50 @@ pub struct LlFunction { /// Render the frame-push instruction. Kept in one place so the eager /// emission and the later count rewrite cannot drift. -fn shadow_frame_push_line(handle_reg: &str, slot_count: u32) -> String { +/// +/// `js_shadow_frame_enter` is `js_shadow_frame_push` returning the address of +/// this thread's `ShadowStackState` instead of the frame handle, so the inline +/// slot stores (#7086) get their base pointer without a second thread-local +/// lookup. The handle the matching pop needs is recovered from the state by +/// [`shadow_frame_handle_lines`] — `handle == frame_top - HEADER_SLOTS` — so +/// the pop side is untouched. +fn shadow_frame_push_line(state_reg: &str, slot_count: u32) -> String { format!( - " {} = call i64 @js_shadow_frame_push(i32 {})", - handle_reg, slot_count + " {} = call ptr @js_shadow_frame_enter(i32 {})", + state_reg, slot_count ) } +/// The lines following the push: stash the state pointer for the inline slot +/// stores, then recover the frame handle from `ShadowStackState::frame_top`. +/// +/// Offsets mirror `perry_runtime::gc::roots::SHADOW_STATE_FRAME_TOP_OFFSET` +/// and `SHADOW_STACK_HEADER_SLOTS`; `perry`'s `shadow_layout_contract` test +/// pins them to the runtime's. +fn shadow_frame_handle_lines( + state_reg: &str, + state_slot: &str, + handle_slot: &str, + top_ptr_reg: &str, + top_reg: &str, + handle_reg: &str, +) -> Vec { + use crate::expr::shadow_inline::{SHADOW_STACK_HEADER_SLOTS, SHADOW_STATE_FRAME_TOP_OFFSET}; + vec![ + format!(" store ptr {}, ptr {}", state_reg, state_slot), + format!( + " {} = getelementptr inbounds i8, ptr {}, i64 {}", + top_ptr_reg, state_reg, SHADOW_STATE_FRAME_TOP_OFFSET + ), + format!(" {} = load i64, ptr {}", top_reg, top_ptr_reg), + format!( + " {} = sub i64 {}, {}", + handle_reg, top_reg, SHADOW_STACK_HEADER_SLOTS + ), + format!(" store i64 {}, ptr {}", handle_reg, handle_slot), + ] +} + /// Location of a function's `js_shadow_frame_push` line, so its slot-count /// operand can be rewritten in place after the fact. struct ShadowFramePush { @@ -181,6 +222,7 @@ impl LlFunction { entry_post_init_setup: Vec::new(), entry_init_boundary: None, shadow_frame_slot: None, + shadow_state_slot: None, shadow_frame_requested: false, shadow_frame_post_init_region: false, shadow_frame_push: None, @@ -241,11 +283,31 @@ impl LlFunction { } fn emit_shadow_frame_push(&mut self, slot_count: u32, post_init: bool) { - use crate::types::I64; + use crate::types::{I64, PTR}; let handle_slot = self.alloca_entry(I64); + let state_slot = self.alloca_entry(PTR); + // Null-initialize in `entry_allocas`, which is always spliced at the + // very top of block 0 — so the slot is initialized even when the push + // itself lives in `entry_post_init_setup` (spliced later, after the + // runtime init prelude). An inline slot store that somehow ran before + // the push would then read null and take its runtime-call arm rather + // than an undef pointer. Where the push dominates, LLVM sees the later + // store of a `nonnull` return and folds that arm away. + self.entry_allocas + .push(format!(" store ptr null, ptr {}", state_slot)); + let state_reg = format!("%r{}", self.reg_counter.next()); + let top_ptr_reg = format!("%r{}", self.reg_counter.next()); + let top_reg = format!("%r{}", self.reg_counter.next()); let handle_reg = format!("%r{}", self.reg_counter.next()); - let push_line = shadow_frame_push_line(&handle_reg, slot_count); - let store_line = format!(" store i64 {}, ptr {}", handle_reg, handle_slot); + let push_line = shadow_frame_push_line(&state_reg, slot_count); + let rest = shadow_frame_handle_lines( + &state_reg, + &state_slot, + &handle_slot, + &top_ptr_reg, + &top_reg, + &handle_reg, + ); let region = if post_init { &mut self.entry_post_init_setup } else { @@ -253,14 +315,23 @@ impl LlFunction { }; let line_idx = region.len(); region.push(push_line); - region.push(store_line); + region.extend(rest); self.shadow_frame_push = Some(ShadowFramePush { post_init, line_idx, - handle_reg, + handle_reg: state_reg, }); self.shadow_frame_slot_count = slot_count; self.shadow_frame_slot = Some(handle_slot); + self.shadow_state_slot = Some(state_slot); + } + + /// The entry alloca holding this thread's `ShadowStackState` address, when + /// this function pushed a shadow frame. `None` means the inline slot + /// stores have no base to work from and callers must use the `extern "C"` + /// entry points. + pub fn shadow_state_slot(&self) -> Option<&str> { + self.shadow_state_slot.as_deref() } /// Reserve one more GC-root slot in this function's shadow frame and diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 0f8ba5e185..955df6a8bf 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -8,7 +8,7 @@ pub mod block; pub(crate) mod boxed_vars; pub mod codegen; pub(crate) mod collectors; -pub(crate) mod expr; +pub mod expr; pub mod ext_registry; pub mod function; pub mod linker; @@ -41,6 +41,22 @@ pub use codegen::{ ImportedClass, NamespaceEntry, NamespaceEntryKind, }; +/// The shadow-stack field offsets generated code bakes into its inline root +/// stores (#7086). +/// +/// Exported so `perry`'s `shadow_layout_contract` test can compare them with +/// `perry-runtime`'s copy. `perry-codegen` does not depend on `perry-runtime`, +/// so nothing else can catch the two drifting apart — and drift is silent: +/// the emitted code would store live GC roots through the wrong offset rather +/// than fail to build. +pub mod expr_shadow_layout { + pub use crate::expr::shadow_inline::{ + SHADOW_ENTRY_META_OFFSET, SHADOW_ENTRY_SHIFT, SHADOW_ENTRY_SIZE, SHADOW_SLOT_ACTIVE_BIT, + SHADOW_STACK_HEADER_SLOTS, SHADOW_STATE_FRAME_TOP_OFFSET, SHADOW_STATE_LEN_OFFSET, + SHADOW_STATE_PTR_OFFSET, + }; +} + /// One row of the native-module dispatch table, projected to just /// the manifest-relevant fields (module / method / has_receiver / /// class_filter / arg-kind summary / return-kind summary). Exposed so diff --git a/crates/perry-codegen/src/module.rs b/crates/perry-codegen/src/module.rs index 807bb70f46..11aca0f298 100644 --- a/crates/perry-codegen/src/module.rs +++ b/crates/perry-codegen/src/module.rs @@ -277,6 +277,36 @@ impl LlModule { )); } + /// [`Self::declare_function`] with LLVM *return* parameter attributes + /// (`nonnull`, `noalias`, …), which sit before the return type and so + /// cannot be expressed through the trailing attribute-group string. + /// + /// Used for `js_shadow_frame_enter`, whose `nonnull` return is what lets + /// LLVM fold away the null-state fallback arm that every inline shadow-slot + /// store emits (#7086). The attribute is true by construction: the runtime + /// returns the address of a `thread_local!`. + pub fn declare_function_with_ret_attrs( + &mut self, + name: &str, + return_type: LlvmType, + param_types: &[LlvmType], + ret_attrs: &str, + ) { + if self.declared_names.contains(name) { + return; + } + self.declared_names.insert(name.to_string()); + let param_str = param_types.join(", "); + let attrs = helper_decl_attrs(name); + self.declarations.push(( + name.to_string(), + format!( + "declare {} {} @{}({}){}", + ret_attrs, return_type, name, param_str, attrs + ), + )); + } + pub fn is_declared(&self, name: &str) -> bool { self.declared_names.contains(name) } diff --git a/crates/perry-codegen/src/runtime_decls/arrays.rs b/crates/perry-codegen/src/runtime_decls/arrays.rs index 64131dcacd..735d910285 100644 --- a/crates/perry-codegen/src/runtime_decls/arrays.rs +++ b/crates/perry-codegen/src/runtime_decls/arrays.rs @@ -96,6 +96,15 @@ pub fn declare_phase_b_arrays(module: &mut LlModule) { // js_shadow_frame_pop(frame_handle: u64) // js_shadow_slot_set(idx: u32, value: u64) // js_shadow_slot_bind(idx: u32, value_slot: *mut u64) + // js_shadow_frame_enter(slot_count: u32) -> *mut ShadowStackState + // + // `js_shadow_frame_enter` is `js_shadow_frame_push` returning the address + // of this thread's shadow state rather than the frame handle, so the + // inline slot stores (#7086) get a base pointer without a second + // thread-local lookup per activation. It is the entry point shadow-frame + // emission actually uses; `js_shadow_frame_push` stays declared (and + // exported) for stale cached objects and out-of-tree callers. + module.declare_function_with_ret_attrs("js_shadow_frame_enter", PTR, &[I32], "nonnull"); module.declare_function("js_shadow_frame_push", I64, &[I32]); module.declare_function("js_shadow_frame_pop", VOID, &[I64]); module.declare_function("js_shadow_slot_set", VOID, &[I32, I64]); diff --git a/crates/perry-runtime/src/gc/roots.rs b/crates/perry-runtime/src/gc/roots.rs index ab9812c95a..02169b39a4 100644 --- a/crates/perry-runtime/src/gc/roots.rs +++ b/crates/perry-runtime/src/gc/roots.rs @@ -27,8 +27,11 @@ 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, + js_shadow_frame_enter, js_shadow_frame_pop, js_shadow_frame_push, js_shadow_slot_bind, + js_shadow_slot_get, js_shadow_slot_set, js_shadow_state_addr, shadow_stack_depth, + ShadowStackState, SHADOW_ENTRY_META_OFFSET, SHADOW_ENTRY_SIZE, SHADOW_SLOT_ACTIVE_BIT, + SHADOW_STACK_GROW_RESERVE, SHADOW_STACK_HEADER_SLOTS, SHADOW_STATE_FRAME_TOP_OFFSET, + SHADOW_STATE_LEN_OFFSET, SHADOW_STATE_PTR_OFFSET, }; pub(crate) use shadow_stack::{shadow_stack_restore, shadow_stack_savepoint, ShadowSavepoint}; #[cfg(test)] diff --git a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs index 1a7c4ae3f0..9f5754946c 100644 --- a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs +++ b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs @@ -469,3 +469,273 @@ fn out_of_range_frame_pop_is_ignored() { js_shadow_frame_pop(h); assert_eq!(shadow_stack_depth(), 0); } + +// --------------------------------------------------------------------------- +// #7086: the inline slot-store addressing contract. +// +// Generated code no longer calls `js_shadow_slot_bind` / `js_shadow_slot_set` +// for the hot per-store root write. It computes the entry address itself from +// the `ShadowStackState` pointer `js_shadow_frame_enter` returned, using the +// published `SHADOW_*` offsets, and stores the two words directly. +// +// The helpers below are a *faithful Rust transcription of the emitted LLVM IR* +// -- same offsets, same guards, same order -- so these tests exercise the +// contract codegen depends on: if an offset, the entry size or the liveness +// bit moves, or if the state pointer stops naming the same memory the runtime +// functions write, they fail. What they cannot check is that codegen emits +// this exact sequence; `shadow_inline`'s IR-shape test covers that half. +// --------------------------------------------------------------------------- + +/// Byte-offset load helper matching the emitted `getelementptr inbounds i8`. +unsafe fn state_word(state: *mut ShadowStackState, byte_off: usize) -> usize { + *(state.cast::().add(byte_off).cast::()) +} + +/// The emitted inline bind, transcribed. Returns `false` when a guard fired, +/// i.e. when the emitted code would have skipped the write. +unsafe fn inline_bind_as_codegen_emits( + state: *mut ShadowStackState, + idx: u32, + local_slot: *mut u64, +) -> bool { + let frame_top = state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET); + if frame_top == usize::MAX { + return false; + } + let slot = frame_top + idx as usize; + if slot >= state_word(state, SHADOW_STATE_LEN_OFFSET) { + return false; + } + let buf = state_word(state, SHADOW_STATE_PTR_OFFSET) as *mut u8; + let entry = buf.add(slot * SHADOW_ENTRY_SIZE); + let raw = local_slot as usize; + let bound = if raw & SHADOW_SLOT_ACTIVE_BIT == 0 { raw } else { 0 }; + *entry.cast::() = *local_slot; + *entry.add(SHADOW_ENTRY_META_OFFSET).cast::() = bound | SHADOW_SLOT_ACTIVE_BIT; + true +} + +/// The emitted inline clear, transcribed. +unsafe fn inline_clear_as_codegen_emits(state: *mut ShadowStackState, idx: u32) -> bool { + let frame_top = state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET); + if frame_top == usize::MAX { + return false; + } + let slot = frame_top + idx as usize; + if slot >= state_word(state, SHADOW_STATE_LEN_OFFSET) { + return false; + } + let buf = state_word(state, SHADOW_STATE_PTR_OFFSET) as *mut u8; + let entry = buf.add(slot * SHADOW_ENTRY_SIZE); + let meta_ptr = entry.add(SHADOW_ENTRY_META_OFFSET).cast::(); + *entry.cast::() = 0; + *meta_ptr &= !SHADOW_SLOT_ACTIVE_BIT; + true +} + +/// `js_shadow_frame_enter` must push exactly the frame `js_shadow_frame_push` +/// pushes, and hand back a state whose `frame_top` yields the same handle. +/// +/// Sabotage check: drop the `- SHADOW_STACK_HEADER_SLOTS` from codegen's +/// handle recovery and the recovered handle stops matching, so every emitted +/// `js_shadow_frame_pop` would unbalance the stack. +#[test] +fn frame_enter_pushes_the_same_frame_and_yields_the_same_handle() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + + let before = shadow_stack_depth(); + let state = js_shadow_frame_enter(3); + assert!(!state.is_null(), "frame_enter must return the state address"); + assert_eq!(shadow_stack_depth(), before + 1); + assert_eq!( + state as usize, + js_shadow_state_addr() as usize, + "frame_enter and state_addr must name the same thread-local" + ); + + let frame_top = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) }; + let recovered_handle = (frame_top - SHADOW_STACK_HEADER_SLOTS) as u64; + + js_shadow_frame_pop(recovered_handle); + assert_eq!( + shadow_stack_depth(), + before, + "handle recovered from frame_top must pop the frame frame_enter pushed" + ); +} + +/// The inline write and the runtime accessor must address the same memory, in +/// both directions. +/// +/// Sabotage check: change any `SHADOW_STATE_*` offset, `SHADOW_ENTRY_SIZE` or +/// `SHADOW_ENTRY_META_OFFSET` and this fails. +#[test] +fn inline_write_and_runtime_accessor_address_the_same_entry() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let state = js_shadow_frame_enter(2); + let handle = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; + + // inline write -> runtime read + let mut storage: u64 = 0x7FFD_0000_DEAD_BEEF; + assert!(unsafe { inline_bind_as_codegen_emits(state, 1, &mut storage as *mut u64) }); + assert_eq!( + js_shadow_slot_get(1), + 0x7FFD_0000_DEAD_BEEF, + "runtime accessor must observe the inline write" + ); + + // runtime write -> inline read + let mut other: u64 = 0x7FFF_0000_0000_00AA; + js_shadow_slot_bind(0, &mut other as *mut u64); + let buf = unsafe { state_word(state, SHADOW_STATE_PTR_OFFSET) } as *const u8; + let frame_top = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) }; + let entry = unsafe { buf.add(frame_top * SHADOW_ENTRY_SIZE) }; + assert_eq!( + unsafe { *entry.cast::() }, + 0x7FFF_0000_0000_00AA, + "inline addressing must observe the runtime write" + ); + assert_eq!( + unsafe { *entry.add(SHADOW_ENTRY_META_OFFSET).cast::() }, + (&mut other as *mut u64 as usize) | SHADOW_SLOT_ACTIVE_BIT, + "inline addressing must see the binding the runtime recorded" + ); + + js_shadow_frame_pop(handle as u64); +} + +/// The inline clear must leave the binding in place with the liveness bit +/// dropped -- the same state `js_shadow_slot_set(idx, 0)` leaves, so a later +/// re-activation still writes through to the same compiled local. +/// +/// Sabotage check: make the inline clear zero `meta` outright and the +/// re-activated slot stops writing through to `storage`. +#[test] +fn inline_clear_matches_the_runtime_clear_and_keeps_the_binding() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let state = js_shadow_frame_enter(1); + let handle = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; + + let mut storage: u64 = 0x7FFD_0000_0000_1111; + js_shadow_slot_bind(0, &mut storage as *mut u64); + assert_eq!(scanner_slot_values().len(), 1, "slot starts live"); + + assert!(unsafe { inline_clear_as_codegen_emits(state, 0) }); + assert!( + scanner_slot_values().is_empty(), + "cleared slot must not be reported as a root" + ); + assert_eq!(js_shadow_slot_get(0), 0, "cleared slot reads as dead"); + + // Re-activation still writes through the retained binding. + js_shadow_slot_set(0, 0x7FFD_0000_0000_2222); + assert_eq!( + storage, 0x7FFD_0000_0000_2222, + "inline clear must keep the binding so re-activation writes through" + ); + + js_shadow_frame_pop(handle as u64); +} + +/// A value written by the inline sequence must be marked and rewritten by an +/// evacuating collection exactly as one written through `js_shadow_slot_bind`. +/// +/// This is the liveness + rewritability property for the inline path. Sabotage +/// check: drop the `| SHADOW_SLOT_ACTIVE_BIT` from the inline meta write and +/// the scanner skips the entry, so the object is collected and `storage` no +/// longer points into the heap. +#[test] +fn inline_bound_slot_survives_and_is_rewritten_by_a_copying_minor() { + let _guard = CopyingNurseryTestGuard::new(1); + reset_shadow_stack(); + let state = js_shadow_frame_enter(1); + + let child = young_leaf(); + let mut storage: u64 = ptr_bits(child); + assert!(unsafe { inline_bind_as_codegen_emits(state, 0, &mut storage as *mut u64) }); + + let _ = gc_collect_minor(); + + let moved = (storage & POINTER_MASK) as usize; + assert_ne!(moved, 0, "inline-bound local was cleared by the collection"); + 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), + "inline-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" + ); +} + +/// The `frame_top == usize::MAX` guard is load-bearing, not defensive noise. +/// +/// Without it `top + idx` wraps and lands on entry `idx - 1` of a *different* +/// frame -- an in-bounds address, so a `slot < len` test alone would let the +/// write through and corrupt a live root. This is the same wrap-around class +/// as the `base + HEADER_SLOTS > len` bug #7079 fixed in `frame_pop`. +/// +/// Sabotage check: delete the sentinel test from +/// `inline_bind_as_codegen_emits` (and from the emitter it transcribes) and +/// the write lands instead of being skipped. +#[test] +fn inline_write_with_no_frame_installed_is_skipped_not_wrapped() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + // Give the buffer a live frame with a known value, then unwind it so + // `frame_top` is the sentinel while the buffer still holds entries. + let state = js_shadow_frame_enter(2); + let handle = + unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; + js_shadow_frame_pop(handle as u64); + assert_eq!( + unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) }, + usize::MAX, + "no frame should be installed" + ); + + let mut storage: u64 = 0x7FFD_0000_0000_9999; + assert!( + !unsafe { inline_bind_as_codegen_emits(state, 1, &mut storage as *mut u64) }, + "inline write must be skipped when no frame is installed" + ); + assert!( + !unsafe { inline_clear_as_codegen_emits(state, 1) }, + "inline clear must be skipped when no frame is installed" + ); + assert!( + scanner_slot_values().is_empty(), + "a skipped write must not have produced a root" + ); +} + +/// An out-of-range slot index must be skipped, matching the runtime +/// functions' `slot >= len` guard, rather than writing past the frame. +#[test] +fn inline_write_past_the_frame_is_skipped() { + let _guard = GcTestIsolationGuard::new(); + reset_shadow_stack(); + let state = js_shadow_frame_enter(1); + let handle = + unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; + + let len_before = unsafe { state_word(state, SHADOW_STATE_LEN_OFFSET) }; + let mut storage: u64 = 0x7FFD_0000_0000_7777; + // Slot 5 in a 1-slot frame is past the end of the buffer. + assert!( + !unsafe { inline_bind_as_codegen_emits(state, 5, &mut storage as *mut u64) }, + "out-of-range inline write must be skipped" + ); + assert_eq!( + unsafe { state_word(state, SHADOW_STATE_LEN_OFFSET) }, + len_before, + "a skipped write must not disturb the buffer" + ); + + js_shadow_frame_pop(handle as u64); +} diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 1fe063e8e2..5d84ff3a1d 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -35,6 +35,7 @@ const BUILD_CACHE_ENV_VARS: &[&str] = &[ "PERRY_LLVM_CLANG", "PERRY_WRITE_BARRIERS", "PERRY_SHADOW_STACK", + "PERRY_INLINE_SHADOW_SLOT", "PERRY_DISABLE_BUFFER_FAST_PATH", "PERRY_VERIFY_NATIVE_REGIONS", "PERRY_UNBOXED_OBJECT_FIELDS", diff --git a/crates/perry/src/commands/compile/object_cache.rs b/crates/perry/src/commands/compile/object_cache.rs index 424c29c907..750eb1f8e5 100644 --- a/crates/perry/src/commands/compile/object_cache.rs +++ b/crates/perry/src/commands/compile/object_cache.rs @@ -794,6 +794,13 @@ fn compute_object_cache_key_with_env( "env_shadow_stack", env_var("PERRY_SHADOW_STACK").as_deref().unwrap_or(""), ); + // #7086: flips the shadow-slot store between an inline sequence and the + // `js_shadow_slot_*` calls. Two arms that shared a cached object would + // silently measure the same code. + h.field( + "env_inline_shadow_slot", + env_var("PERRY_INLINE_SHADOW_SLOT").as_deref().unwrap_or(""), + ); h.field( "env_disable_buffer_fast_path", env_var("PERRY_DISABLE_BUFFER_FAST_PATH") diff --git a/crates/perry/src/main.rs b/crates/perry/src/main.rs index 4754f15638..09ad05e08e 100644 --- a/crates/perry/src/main.rs +++ b/crates/perry/src/main.rs @@ -3,6 +3,8 @@ //! CLI driver for compiling TypeScript to native executables. mod commands; +#[cfg(test)] +mod shadow_layout_contract; mod compat_reports; mod telemetry; #[cfg(test)] diff --git a/crates/perry/src/shadow_layout_contract.rs b/crates/perry/src/shadow_layout_contract.rs new file mode 100644 index 0000000000..fd63d65e51 --- /dev/null +++ b/crates/perry/src/shadow_layout_contract.rs @@ -0,0 +1,111 @@ +//! The shadow-stack layout contract between `perry-codegen` and +//! `perry-runtime` (#7086). +//! +//! Generated code writes GC roots *inline*: it computes the address of a +//! `ShadowEntry` from the `ShadowStackState` pointer `js_shadow_frame_enter` +//! returned, using hardcoded field offsets, and stores the two words itself. +//! `perry-codegen` deliberately does not depend on `perry-runtime`, so those +//! offsets exist twice. +//! +//! Nothing in either crate's own build fails if one copy moves. The result +//! would not be a compile error or a crash: the emitted code would write a +//! live root into the wrong word of the wrong structure, and the collector +//! would read a stale or bogus one — a silent wrong-answer bug of exactly the +//! kind this campaign keeps finding. +//! +//! `perry` depends on both crates, so this is where the two copies can be +//! compared. It runs in `cargo-test`, i.e. per PR, unlike the integration +//! suites under `crates/*/tests/`. + +#[cfg(test)] +mod tests { + use perry_codegen::expr_shadow_layout as cg; + use perry_runtime::gc as rt; + + /// Every offset codegen bakes into the emitted store must equal the + /// runtime's. + /// + /// Sabotage check: change any one constant in either crate and this fails. + #[test] + fn shadow_layout_contract_matches_the_runtime() { + assert_eq!( + cg::SHADOW_STATE_PTR_OFFSET as usize, + rt::SHADOW_STATE_PTR_OFFSET, + "ShadowStackState::ptr offset drifted between codegen and runtime" + ); + assert_eq!( + cg::SHADOW_STATE_LEN_OFFSET as usize, + rt::SHADOW_STATE_LEN_OFFSET, + "ShadowStackState::len offset drifted between codegen and runtime" + ); + assert_eq!( + cg::SHADOW_STATE_FRAME_TOP_OFFSET as usize, + rt::SHADOW_STATE_FRAME_TOP_OFFSET, + "ShadowStackState::frame_top offset drifted between codegen and runtime" + ); + assert_eq!( + cg::SHADOW_ENTRY_SIZE as usize, + rt::SHADOW_ENTRY_SIZE, + "ShadowEntry size drifted between codegen and runtime" + ); + assert_eq!( + cg::SHADOW_ENTRY_META_OFFSET as usize, + rt::SHADOW_ENTRY_META_OFFSET, + "ShadowEntry::meta offset drifted between codegen and runtime" + ); + assert_eq!( + cg::SHADOW_SLOT_ACTIVE_BIT as usize, + rt::SHADOW_SLOT_ACTIVE_BIT, + "slot liveness bit drifted between codegen and runtime" + ); + assert_eq!( + cg::SHADOW_STACK_HEADER_SLOTS as usize, + rt::SHADOW_STACK_HEADER_SLOTS, + "frame header size drifted; codegen recovers the frame handle as \ + frame_top - HEADER_SLOTS, so a mismatch unbalances every pop" + ); + } + + /// Codegen indexes the entry buffer with a shift, so the entry size must + /// stay a power of two and the shift must match it. + #[test] + fn shadow_entry_shift_matches_the_entry_size() { + assert_eq!( + cg::SHADOW_ENTRY_SIZE, + 1u64 << cg::SHADOW_ENTRY_SHIFT, + "emitted `shl` amount does not match the entry size" + ); + } + + /// The state pointer codegen caches per activation must be the address the + /// runtime's own accessors use — that is the whole basis for the inline + /// store touching the same memory as `js_shadow_slot_set`. + /// + /// Sabotage check: have `js_shadow_frame_enter` return anything other than + /// the thread-local's address and this fails. + #[test] + fn frame_enter_returns_the_runtime_thread_local_address() { + let state = rt::js_shadow_frame_enter(1); + assert!(!state.is_null()); + assert_eq!( + state as usize, + rt::js_shadow_state_addr() as usize, + "cached state pointer must name the runtime's own thread-local" + ); + + // And the handle codegen recovers from it must pop that frame. + let frame_top = unsafe { + *(state + .cast::() + .add(rt::SHADOW_STATE_FRAME_TOP_OFFSET) + .cast::()) + }; + let depth_before = rt::shadow_stack_depth(); + rt::js_shadow_frame_pop((frame_top - rt::SHADOW_STACK_HEADER_SLOTS) as u64); + assert_eq!( + rt::shadow_stack_depth(), + depth_before - 1, + "handle recovered the way codegen recovers it must balance the frame" + ); + } +} From 7024fd169268708f56af829d96fbddef0e9d164b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 20:35:13 +0200 Subject: [PATCH 3/6] test: teeth for the inline shadow-slot store Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- crates/perry-codegen/src/codegen/closure.rs | 2 +- .../perry-codegen/src/expr/shadow_inline.rs | 280 ++++++++++++++++++ .../tests/shadow_slot_hygiene.rs | 31 +- 3 files changed, 303 insertions(+), 10 deletions(-) diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index aef884acbf..42ee3aea75 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -1266,7 +1266,7 @@ mod tests { .find(|f| { let name_starts_here = f.starts_with("double @perry_closure_") || f.starts_with("internal double @perry_closure_"); - name_starts_here && f.contains("@js_shadow_frame_push") + name_starts_here && f.contains("@js_shadow_frame_enter") }) .unwrap_or_else(|| panic!("no shadow-framed closure body in IR:\n{ir}")); diff --git a/crates/perry-codegen/src/expr/shadow_inline.rs b/crates/perry-codegen/src/expr/shadow_inline.rs index 690745a1b9..5b25ed2ce4 100644 --- a/crates/perry-codegen/src/expr/shadow_inline.rs +++ b/crates/perry-codegen/src/expr/shadow_inline.rs @@ -292,3 +292,283 @@ fn emit_inline_root_shading_barrier(ctx: &mut FnCtx<'_>, value_bits: &str, done_ .call_void("js_write_barrier_root_nanbox", &[(I64, value_bits)]); ctx.block().br(done_label); } + +#[cfg(test)] +mod tests { + use super::*; + use perry_hir::types::Type; + use perry_hir::{Expr, Function, Module as HirModule, Param, Stmt}; + + /// A function with one pointer-typed local, reassigned first from another + /// pointer-typed local (forcing a root bind) and then from a number + /// (forcing the "dead from here" clear). + fn rooted_local_ir() -> String { + let mut hir = HirModule::new("inline_shadow_slot_test"); + hir.functions.push(Function { + id: 0, + name: "roots".to_string(), + type_params: Vec::new(), + params: vec![Param { + id: 0, + name: "o".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + return_type: Type::Any, + body: vec![ + Stmt::Let { + id: 1, + name: "a".to_string(), + ty: Type::Any, + mutable: true, + init: Some(Expr::LocalGet(0)), + }, + // pointer-capable store -> root bind + Stmt::Expr(Expr::LocalSet(1, Box::new(Expr::LocalGet(0)))), + // proven-non-pointer store -> root clear + Stmt::Expr(Expr::LocalSet(1, Box::new(Expr::Number(1.0)))), + Stmt::Return(Some(Expr::LocalGet(1))), + ], + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }); + let opts = crate::CompileOptions { + emit_ir_only: true, + ..Default::default() + }; + let bytes = crate::compile_module(&hir, opts).expect("test module compiles"); + String::from_utf8(bytes).expect("LLVM IR is UTF-8") + } + + fn roots_body(ir: &str) -> String { + ir.split("\ndefine ") + .find(|f| f.contains("@perry_fn_inline_shadow_slot_test__roots(")) + .unwrap_or_else(|| panic!("no `roots` body in IR:\n{ir}")) + .to_string() + } + + /// Every emitted inline-store block, in order. + fn store_blocks(body: &str) -> Vec { + let mut blocks: Vec = Vec::new(); + let mut current: Option = None; + for line in body.lines() { + if line.starts_with("ss.store.") { + current = Some(String::new()); + continue; + } + if let Some(buf) = current.as_mut() { + if line.trim().starts_with("br ") { + blocks.push(std::mem::take(buf)); + current = None; + } else { + buf.push_str(line); + buf.push('\n'); + } + } + } + assert!( + !blocks.is_empty(), + "no inline shadow-slot store block emitted; body:\n{body}" + ); + blocks + } + + /// The register holding the entry address in a store block: the operand of + /// the `shl`-indexed `getelementptr` into the entry buffer. + fn entry_reg(blk: &str) -> String { + for line in blk.lines() { + // ` %rN = getelementptr inbounds i8, ptr %rBUF, i64 %rSHIFTED` + if line.contains("getelementptr inbounds i8, ptr %") && line.trim_end().ends_with(|c: char| c.is_ascii_digit()) + && line.contains(", i64 %") + { + if let Some(name) = line.trim().split(" = ").next() { + return name.trim_start_matches('%').to_string(); + } + } + } + panic!("no entry-address getelementptr in store block:\n{blk}"); + } + + /// The block that performs a *bind* (mirrors a value and sets the liveness + /// bit), located by content so the fixture's statement order can change + /// without silently testing the wrong block. + fn bind_block(body: &str) -> String { + store_blocks(body) + .into_iter() + .find(|b| b.contains("select i1 ") && b.contains("ptrtoint ptr ")) + .unwrap_or_else(|| panic!("no inline bind block in:\n{body}")) + } + + /// The block that performs a *clear* (zeroes the mirror, masks the + /// liveness bit out of meta). + fn clear_block(body: &str) -> String { + store_blocks(body) + .into_iter() + .find(|b| b.contains("store i64 0, ptr %")) + .unwrap_or_else(|| panic!("no inline clear block in:\n{body}")) + } + + /// The frame push must go through `js_shadow_frame_enter` and derive the + /// pop handle from `frame_top`, because the state pointer — not the handle + /// — is what the inline stores need. + /// + /// Sabotage check: point `shadow_frame_push_line` back at + /// `js_shadow_frame_push` and the first two assertions fail; drop the + /// `sub` in `shadow_frame_handle_lines` and the last one does. + #[test] + fn frame_push_uses_frame_enter_and_derives_the_handle() { + let body = roots_body(&rooted_local_ir()); + assert!( + body.contains("call ptr @js_shadow_frame_enter(i32 "), + "frame push must go through js_shadow_frame_enter; body:\n{body}" + ); + assert!( + !body.contains("@js_shadow_frame_push("), + "the handle-returning push must no longer be emitted; body:\n{body}" + ); + assert!( + body.contains(&format!( + "getelementptr inbounds i8, ptr %r3, i64 {}", + SHADOW_STATE_FRAME_TOP_OFFSET + )) || body.contains(&format!(", i64 {}\n", SHADOW_STATE_FRAME_TOP_OFFSET)), + "handle recovery must load ShadowStackState::frame_top at offset \ + {SHADOW_STATE_FRAME_TOP_OFFSET}; body:\n{body}" + ); + assert!( + body.contains(&format!("sub i64 %r5, {}", SHADOW_STACK_HEADER_SLOTS)), + "pop handle must be frame_top - {SHADOW_STACK_HEADER_SLOTS}; body:\n{body}" + ); + } + + /// The hot per-store root write must be a store, not a call. + /// + /// Sabotage check: make `emit_inline_slot_bind` return `false` and there is + /// no `ss.store` block at all. + #[test] + fn pointer_store_roots_inline_with_the_runtime_entry_layout() { + let body = roots_body(&rooted_local_ir()); + let blk = bind_block(&body); + assert!( + blk.contains(&format!(", {}\n", SHADOW_ENTRY_SHIFT)) && blk.contains("shl i64 %"), + "entry address must index the buffer by shifting the slot index by \ + {SHADOW_ENTRY_SHIFT}; block:\n{blk}" + ); + assert!( + blk.contains(&format!( + "getelementptr inbounds i8, ptr %{}, i64 {}", + entry_reg(&blk), + SHADOW_ENTRY_META_OFFSET + )), + "meta word must sit at offset {SHADOW_ENTRY_META_OFFSET} of the \ + entry; block:\n{blk}" + ); + assert!( + blk.contains(&format!(", {}\n", SHADOW_SLOT_ACTIVE_BIT)) && blk.contains("or i64 %"), + "inline bind must set the liveness bit in meta, or the scanner \ + skips the root; block:\n{blk}" + ); + assert!( + blk.contains("select i1 %") && blk.contains(", i64 0\n"), + "inline bind must keep `bound_slot_meta`'s alignment fallback so a \ + tag-colliding address is recorded unbound, never truncated; \ + block:\n{blk}" + ); + // Both words written, value first. + assert!( + blk.matches("store i64 %").count() == 2, + "inline bind must write both the mirrored value and meta; \ + block:\n{blk}" + ); + } + + /// Both guards must be present. Dropping the sentinel test is not a missing + /// safety net but a corruption: `usize::MAX + idx` wraps to `idx - 1`, + /// which passes a `slot < len` test and overwrites a *live* entry of + /// another frame — the same wrap-around class #7079 fixed in `frame_pop`. + /// + /// Sabotage check: delete either guard from `emit_inline_slot_write`. + #[test] + fn inline_store_keeps_the_sentinel_and_bounds_guards() { + let body = roots_body(&rooted_local_ir()); + assert!( + body.contains("ss.chk_top") && body.contains("ss.chk_len"), + "inline store must keep both guards; body:\n{body}" + ); + assert!( + body.contains("icmp eq i64 %r13, -1"), + "frame_top must be tested against the usize::MAX no-frame sentinel; \ + body:\n{body}" + ); + assert!( + body.contains("icmp ult i64 %r15, %r17"), + "slot index must be bounds-checked against ShadowStackState::len; \ + body:\n{body}" + ); + assert!( + body.contains(&format!( + "getelementptr inbounds i8, ptr %r10, i64 {}", + SHADOW_STATE_LEN_OFFSET + )), + "the bounds check must read len at offset {SHADOW_STATE_LEN_OFFSET}; \ + body:\n{body}" + ); + } + + /// The incremental-mark root shading barrier must survive inlining, gated + /// on the counter the runtime uses. + /// + /// Sabotage check: drop the `emit_inline_root_shading_barrier` call — a + /// pointer written into a root after the collector scanned roots is then + /// never shaded, and an in-flight incremental cycle frees a live object. + #[test] + fn inline_bind_keeps_the_gated_root_shading_barrier() { + let body = roots_body(&rooted_local_ir()); + assert!( + body.contains( + "load atomic i32, ptr @PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT seq_cst" + ), + "inline bind must gate on the incremental-mark active count; \ + body:\n{body}" + ); + assert!( + body.contains("call void @js_write_barrier_root_nanbox(i64 %r23)"), + "inline bind must shade the value it just stored when a cycle is in \ + flight; body:\n{body}" + ); + } + + /// The clear must drop the liveness bit while keeping the binding, so a + /// later re-activation still writes through to the same compiled local. + /// + /// Sabotage check: zero the whole meta word instead of masking and the + /// `-2` constant disappears. + #[test] + fn dead_local_clear_is_inline_and_preserves_the_binding() { + let body = roots_body(&rooted_local_ir()); + let blk = clear_block(&body); + let mask = !(SHADOW_SLOT_ACTIVE_BIT as i64); + assert!( + blk.contains(&format!(", {}\n", mask)) && blk.contains("and i64 %"), + "inline clear must AND meta with !SLOT_ACTIVE ({mask}), keeping the \ + bound address; block:\n{blk}" + ); + assert!( + blk.contains("store i64 0, ptr %"), + "inline clear must zero the mirrored value; block:\n{blk}" + ); + assert!( + !blk.contains("@js_write_barrier_root_nanbox"), + "a zero value is not a heap reference and needs no shading; \ + block:\n{blk}" + ); + } +} diff --git a/crates/perry-codegen/tests/shadow_slot_hygiene.rs b/crates/perry-codegen/tests/shadow_slot_hygiene.rs index 84aa1689d2..2c60d3d07c 100644 --- a/crates/perry-codegen/tests/shadow_slot_hygiene.rs +++ b/crates/perry-codegen/tests/shadow_slot_hygiene.rs @@ -1,3 +1,15 @@ +//! NOTE (#7086): the hot shadow-slot stores are now emitted **inline** against +//! the `ShadowStackState` pointer `js_shadow_frame_enter` returns, not as +//! `js_shadow_slot_bind` / `js_shadow_slot_set` calls. Each inline site still +//! emits one of those calls on its null-state fallback arm, at exactly the +//! position the old unconditional call occupied — LLVM folds the arm away +//! because the push's return is `nonnull`. The searches below therefore still +//! locate the right program point, and the *ordering* properties they assert +//! (bind before clear, clear before the next allocation, slot indices not +//! shifted by a numeric local) are unchanged. What they no longer prove is +//! that a call is what executes; `expr::shadow_inline`'s unit tests cover the +//! emitted shape. + use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::types::Type; use perry_hir::{Expr, Function, Module, ModuleInitKind, Stmt}; @@ -572,7 +584,7 @@ fn function_shadow_slots_clear_dead_values_and_skip_numeric_roots() { .expect("LLVM IR should be UTF-8"); assert!( - ir.contains("call i64 @js_shadow_frame_push(i32 2)"), + ir.contains("call ptr @js_shadow_frame_enter(i32 2)"), "known numeric Any local must not reserve a shadow slot" ); @@ -611,7 +623,7 @@ fn entry_module_top_level_shadow_frame_starts_after_init_prelude() { .find("__perry_init_strings_") .expect("entry main should initialize module strings before user code"); let frame_push = main_ir - .find("call i64 @js_shadow_frame_push(i32 2)") + .find("call ptr @js_shadow_frame_enter(i32 2)") .expect("entry main should push a top-level shadow frame"); let user_alloc = main_ir .find("call i64 @js_map_alloc") @@ -639,7 +651,7 @@ fn entry_module_top_level_shadow_slots_update_and_clear() { let main_ir = function_slice(&ir, "main"); assert!( - main_ir.contains("call i64 @js_shadow_frame_push(i32 2)"), + main_ir.contains("call ptr @js_shadow_frame_enter(i32 2)"), "known numeric top-level Any local must not reserve a shadow slot" ); @@ -680,7 +692,7 @@ fn non_entry_module_init_body_gets_post_init_shadow_frame() { .find("__perry_init_strings_") .expect("non-entry init body should initialize strings before user code"); let frame_push = init_ir - .find("call i64 @js_shadow_frame_push(i32 2)") + .find("call ptr @js_shadow_frame_enter(i32 2)") .expect("non-entry init body should push a top-level shadow frame"); let user_alloc = init_ir .find("call i64 @js_map_alloc") @@ -765,8 +777,9 @@ fn flat_const_row_aliases_do_not_reserve_shadow_slots() { let main_ir = function_slice(&ir, "main"); assert!( - main_ir.contains("call i64 @js_shadow_frame_push(i32 1)"), - "only the flat-const table root should reserve a shadow slot" + main_ir.contains("call ptr @js_shadow_frame_enter(i32 1)"), + "only the flat-const table root should reserve a shadow slot; main:\n{}", + main_ir.lines().filter(|l| l.contains("shadow")).collect::>().join("\n") ); assert!( !main_ir.contains("call void @js_shadow_slot_set(i32 1"), @@ -782,7 +795,7 @@ fn reassigned_any_from_number_to_pointer_reserves_and_updates_shadow_slot() { let fn_ir = function_slice(&ir, "perry_fn_reassigned_any_shadow_ts__probe_reassign"); assert!( - fn_ir.contains("call i64 @js_shadow_frame_push(i32 1)"), + fn_ir.contains("call ptr @js_shadow_frame_enter(i32 1)"), "Any local with a later pointer write must reserve a shadow slot" ); let array_alloc = fn_ir @@ -806,7 +819,7 @@ fn mixed_any_writes_keep_alias_shadow_slots_precise() { ); assert!( - fn_ir.contains("call i64 @js_shadow_frame_push(i32 3)"), + fn_ir.contains("call ptr @js_shadow_frame_enter(i32 3)"), "mixed Any writes must keep source, alias, and later reserved as shadow slots" ); for slot_idx in 0..3 { @@ -831,7 +844,7 @@ fn closure_body_write_to_captured_outer_local_is_visible_to_shadow_analysis() { ); assert!( - fn_ir.contains("call i64 @js_shadow_frame_push(i32 2)"), + fn_ir.contains("call ptr @js_shadow_frame_enter(i32 2)"), "captured Any local written to a pointer inside a closure must keep its outer slot" ); assert!( From 2732fa9d40bf2541be388fe1d299f57c35591c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 20:40:27 +0200 Subject: [PATCH 4/6] test: force the sentinel-guard hazard state so the guard test has teeth The first version passed with the guard removed: after a balanced pop len is 0, so the bounds check alone skips the write. Forcing frame_top = usize::MAX while the frame's entries are still live reproduces the wrap the guard exists for -- idx-1 lands on the frame header -- and the test now fails when the guard is deleted. Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- .../tests/shadow_slot_hygiene.rs | 9 ++- .../src/gc/tests/shadow_stack_ops.rs | 67 +++++++++++++------ 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/crates/perry-codegen/tests/shadow_slot_hygiene.rs b/crates/perry-codegen/tests/shadow_slot_hygiene.rs index 2c60d3d07c..f0b7b63087 100644 --- a/crates/perry-codegen/tests/shadow_slot_hygiene.rs +++ b/crates/perry-codegen/tests/shadow_slot_hygiene.rs @@ -776,10 +776,15 @@ fn flat_const_row_aliases_do_not_reserve_shadow_slots() { .expect("LLVM IR should be UTF-8"); let main_ir = function_slice(&ir, "main"); + // PRE-EXISTING RED, not caused by #7086: verified by running this suite + // against pristine `origin/main` sources, where the same assertion fails + // with three reserved slots instead of one. Two row aliases now take a + // persistent shadow slot each. This suite runs nightly/at-tag rather than + // per-PR, which is how it went red unnoticed. Kept asserting the intended + // property, with the string updated for `js_shadow_frame_enter`. assert!( main_ir.contains("call ptr @js_shadow_frame_enter(i32 1)"), - "only the flat-const table root should reserve a shadow slot; main:\n{}", - main_ir.lines().filter(|l| l.contains("shadow")).collect::>().join("\n") + "only the flat-const table root should reserve a shadow slot" ); assert!( !main_ir.contains("call void @js_shadow_slot_set(i32 1"), diff --git a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs index 9f5754946c..3fd143ca11 100644 --- a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs +++ b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs @@ -673,31 +673,43 @@ fn inline_bound_slot_survives_and_is_rewritten_by_a_copying_minor() { ); } -/// The `frame_top == usize::MAX` guard is load-bearing, not defensive noise. +/// The `frame_top == usize::MAX` sentinel guard, mirroring the one in +/// `js_shadow_slot_set` / `js_shadow_slot_bind`. /// -/// Without it `top + idx` wraps and lands on entry `idx - 1` of a *different* -/// frame -- an in-bounds address, so a `slot < len` test alone would let the -/// write through and corrupt a live root. This is the same wrap-around class -/// as the `base + HEADER_SLOTS > len` bug #7079 fixed in `frame_pop`. +/// Honest scope: in a *balanced* program the guard is unreachable, because +/// `frame_top == usize::MAX` implies `len == 0` (the outermost pop restores +/// both), so the bounds check alone would already skip the write. It is kept +/// because the emitted sequence must be observably identical to the runtime +/// function it replaces, and because if the two ever *can* diverge the failure +/// mode is silent corruption rather than a skip: `usize::MAX + idx` wraps to +/// `idx - 1`, which for `idx >= 1` is an in-bounds index into the frame +/// *header* — overwriting `prev_frame_top` and `slot_count` and unlinking every +/// outer frame from the root scan. /// -/// Sabotage check: delete the sentinel test from -/// `inline_bind_as_codegen_emits` (and from the emitter it transcribes) and -/// the write lands instead of being skipped. +/// So the test forces exactly that state rather than pretending a balanced +/// program reaches it. Sabotage check: drop the sentinel test from +/// `inline_bind_as_codegen_emits` (and from the emitter it transcribes) and the +/// header is overwritten instead of the write being skipped. #[test] fn inline_write_with_no_frame_installed_is_skipped_not_wrapped() { let _guard = GcTestIsolationGuard::new(); reset_shadow_stack(); - // Give the buffer a live frame with a known value, then unwind it so - // `frame_top` is the sentinel while the buffer still holds entries. let state = js_shadow_frame_enter(2); let handle = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; - js_shadow_frame_pop(handle as u64); - assert_eq!( - unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) }, - usize::MAX, - "no frame should be installed" - ); + + // Snapshot the frame header, then force the no-frame sentinel while the + // buffer still holds this frame's entries. + let buf = unsafe { state_word(state, SHADOW_STATE_PTR_OFFSET) } as *mut u8; + let header_before = unsafe { *buf.cast::() }; + let header_meta_before = + unsafe { *buf.add(SHADOW_ENTRY_META_OFFSET).cast::() }; + unsafe { + *(state + .cast::() + .add(SHADOW_STATE_FRAME_TOP_OFFSET) + .cast::()) = usize::MAX; + } let mut storage: u64 = 0x7FFD_0000_0000_9999; assert!( @@ -708,10 +720,27 @@ fn inline_write_with_no_frame_installed_is_skipped_not_wrapped() { !unsafe { inline_clear_as_codegen_emits(state, 1) }, "inline clear must be skipped when no frame is installed" ); - assert!( - scanner_slot_values().is_empty(), - "a skipped write must not have produced a root" + assert_eq!( + unsafe { *buf.cast::() }, + header_before, + "a skipped write must not have wrapped into the frame header's \ + prev_frame_top word" + ); + assert_eq!( + unsafe { *buf.add(SHADOW_ENTRY_META_OFFSET).cast::() }, + header_meta_before, + "a skipped write must not have wrapped into the frame header's \ + slot_count word" ); + + // Restore a coherent state so the frame can be popped. + unsafe { + *(state + .cast::() + .add(SHADOW_STATE_FRAME_TOP_OFFSET) + .cast::()) = handle + SHADOW_STACK_HEADER_SLOTS; + } + js_shadow_frame_pop(handle as u64); } /// An out-of-range slot index must be skipped, matching the runtime From 33152a466d469e9e2fab2b1c3998e16646f2bba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 21:00:54 +0200 Subject: [PATCH 5/6] docs: changelog fragment for the inline shadow-slot store Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- changelog.d/7086-inline-shadow-slot-store.md | 99 ++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 changelog.d/7086-inline-shadow-slot-store.md diff --git a/changelog.d/7086-inline-shadow-slot-store.md b/changelog.d/7086-inline-shadow-slot-store.md new file mode 100644 index 0000000000..562379beff --- /dev/null +++ b/changelog.d/7086-inline-shadow-slot-store.md @@ -0,0 +1,99 @@ +**Perf (GC): the shadow-slot root store is emitted inline instead of calling into the runtime.** + +Codegen roots every pointer-capable local by mirroring it into a shadow-stack +slot. Until now each such store was an `extern "C"` call — +`js_shadow_slot_bind(idx, &local)`, or `js_shadow_slot_set(idx, 0)` for the +"dead from here" clear. #7079 made those functions cheap *internally*, but a +call costs twice: the call itself, and the fact that it is **opaque to LLVM**, +which forces a spill of every live value around it and blocks hoisting across +it. That reading — that what remained was per-store calls only codegen could +touch — is what this change tests. + +Read out of the shipped `aarch64` archive, `js_shadow_slot_bind`'s fast path +was ~35 instructions, of which only about six did any work: + +| | before | after | +|---|---|---| +| call/return | `bl` + 4-instruction prologue/epilogue pair | — | +| reach the thread-local | TLSDESC `adrp`/`ldr`/`add` + **indirect `blr`** into the resolver, `mrs tpidr_el0` | — (state pointer already in a register) | +| lazy TLS destructor check | `ldrb`/`cmp`/`b.eq`, plus a `panic_access_error` edge | — (the TLS type is now drop-free) | +| the actual work | 2 guards, address computation, `stp`, gated barrier | unchanged | + +**How the thread-local is reached.** Not by re-deriving the TLS address in +generated code — that would mean modelling Rust's TLS model per platform and +would be a second, unverified path to the same memory. Instead the address is +obtained *from the runtime*: `js_shadow_frame_enter` is `js_shadow_frame_push` +returning the address of this thread's `ShadowStackState` instead of the frame +handle, so codegen pays exactly the one thread-local lookup per activation that +the push already paid. The pointer is cached in an entry alloca; each store +loads it back and re-reads `ptr`/`len`/`frame_top` from it. Caching is sound +because it is the address of a `const`-initialized, drop-free `thread_local!` — +fixed for the thread's lifetime, never reallocated — while the *buffer* it +points at does move when a deeper frame grows it, which is why no frame base is +cached. The handle the matching `js_shadow_frame_pop` needs is recovered as +`frame_top - SHADOW_STACK_HEADER_SLOTS`, so the pop side is untouched. + +**`ShadowStackState` is now `#[repr(C)]` with an explicit buffer.** Generated +code addresses its fields by hardcoded offset, and `Vec`'s layout is explicitly +not a stable contract — in the archive read at the time of writing it happened +to place `cap` at 0, `ptr` at 8 and `len` at 16, and a silent reorder would have +codegen writing live GC roots through the wrong word. Splitting the three words +out also drops the type's drop glue, which is what forced the per-op lazy +destructor-registration check; the buffer is now freed at thread exit by a +separate guard thread-local that is armed only from the cold growth path, and +that resets the state to the empty sentinel rather than leaving `std` to mark +the slot `DESTROYED` (whose next access aborted through `panic_access_error`). + +**Soundness, per root property.** + +*Liveness* — the inline store writes the same `ShadowEntry.value` and sets the +same `SLOT_ACTIVE` bit of `meta`, at the same index, so +`visit_shadow_stack_root_slots` marks it identically. + +*Rewritability* — `meta` still carries the bound compiled-local address, with +`bound_slot_meta`'s alignment fallback intact (a tag-colliding address is +recorded active-but-unbound, never truncated), so an evacuating collection +rewrites the alloca the mutator reads after the safepoint, not just the mirror. + +*The value the mutator stored* — the value is read from the local slot at the +store site, in the position the call occupied, and written immediately. Nothing +re-reads a slot at a later safepoint. + +The incremental-mark root shading barrier is emitted inline behind the same +`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` gate the runtime and +`emit_persistent_shadow_root_barrier` already use, so a pointer stored into a +root after the collector scanned roots is still shaded. + +**Guards.** Both of the runtime function's guards are emitted: the +`frame_top == usize::MAX` sentinel test and the `slot < len` bounds check. The +sentinel is unreachable in a balanced program (that state implies `len == 0`, +so the bounds check alone would skip), but it is kept for exact parity with the +function it replaces, because the failure mode if the two ever diverge is silent +corruption rather than a skip: `usize::MAX + idx` wraps to `idx - 1`, an +*in-bounds* index into the frame header, unlinking every outer frame from the +root scan. That is the same wrap-around class #7079 fixed in `frame_pop`. + +Each store also emits a null-state fallback arm that calls the original runtime +function. `js_shadow_frame_enter` is declared with a `nonnull` return, so LLVM +folds that arm away wherever the push dominates — verified in the linked +binary, where `js_shadow_slot_set` no longer appears at all. + +**Not inlined, deliberately:** the frame push/pop pair (per activation, not per +store), the parameter and closure-prologue binds, and the persistent +entry-setup binds. Those are emitted before the lowering context exists and +would need the guard/barrier control flow restructured; they remain the largest +identified per-activation cost, and `alwaysinline` leaf functions turn them +into per-iteration cost, so they are the natural next step. + +`PERRY_INLINE_SHADOW_SLOT=0`/`off`/`false` reverts to the calls for bisection. +It is part of both the build-level and per-object cache keys, so the two arms +can never share a cached `.o`. + +Tests: 6 new runtime cases pinning the addressing contract (inline write +visible through `js_shadow_slot_get` and vice versa, liveness and rewriting +across a real copying minor, both guards, the clear preserving its binding), 5 +new codegen cases pinning the emitted IR shape, and a `shadow_layout_contract` +test in `perry` — the only crate depending on both — asserting codegen's +offsets equal the runtime's, since nothing else would catch that drift and the +result would be silent. Every one was verified to fail under a targeted +sabotage of the thing it covers. From 53ad6f62f95401a598a40286a4f0c5f1e10dbead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 21:01:55 +0200 Subject: [PATCH 6/6] chore: renumber to #7088 and apply rustfmt Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- ...re.md => 7088-inline-shadow-slot-store.md} | 0 crates/perry-codegen/src/codegen/helpers.rs | 2 +- .../perry-codegen/src/expr/shadow_inline.rs | 27 ++++++------------- crates/perry-codegen/src/expr/shadow_slot.rs | 4 +-- crates/perry-codegen/src/function.rs | 4 +-- crates/perry-codegen/src/lib.rs | 2 +- crates/perry-codegen/src/module.rs | 2 +- .../perry-codegen/src/runtime_decls/arrays.rs | 2 +- .../tests/shadow_slot_hygiene.rs | 4 +-- .../src/gc/roots/shadow_stack.rs | 4 +-- .../src/gc/tests/shadow_stack_ops.rs | 22 ++++++++++----- .../src/commands/compile/object_cache.rs | 2 +- crates/perry/src/main.rs | 2 +- crates/perry/src/shadow_layout_contract.rs | 2 +- 14 files changed, 38 insertions(+), 41 deletions(-) rename changelog.d/{7086-inline-shadow-slot-store.md => 7088-inline-shadow-slot-store.md} (100%) diff --git a/changelog.d/7086-inline-shadow-slot-store.md b/changelog.d/7088-inline-shadow-slot-store.md similarity index 100% rename from changelog.d/7086-inline-shadow-slot-store.md rename to changelog.d/7088-inline-shadow-slot-store.md diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index f998e4dc62..f13844aee3 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -74,7 +74,7 @@ pub(super) fn shadow_stack_enabled() -> bool { }) } -/// Inline shadow-slot store gate (#7086). Default ON. +/// Inline shadow-slot store gate (#7088). Default ON. /// /// When enabled, a store to a GC-rooted local is emitted as an address /// computation and a pair of stores against this thread's `ShadowStackState` diff --git a/crates/perry-codegen/src/expr/shadow_inline.rs b/crates/perry-codegen/src/expr/shadow_inline.rs index 5b25ed2ce4..0c264bae40 100644 --- a/crates/perry-codegen/src/expr/shadow_inline.rs +++ b/crates/perry-codegen/src/expr/shadow_inline.rs @@ -1,4 +1,4 @@ -//! Inline shadow-slot stores (#7086). +//! Inline shadow-slot stores (#7088). //! //! # What this replaces //! @@ -102,11 +102,7 @@ enum InlineSlotWrite<'a> { /// /// Returns `false` when this function has no cached state pointer (so the /// caller must fall back to the `extern "C"` call). -pub(crate) fn emit_inline_slot_bind( - ctx: &mut FnCtx<'_>, - slot_idx: u32, - local_slot: &str, -) -> bool { +pub(crate) fn emit_inline_slot_bind(ctx: &mut FnCtx<'_>, slot_idx: u32, local_slot: &str) -> bool { emit_inline_slot_write(ctx, slot_idx, InlineSlotWrite::Bind { local_slot }) } @@ -115,11 +111,7 @@ pub(crate) fn emit_inline_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) -> bool emit_inline_slot_write(ctx, slot_idx, InlineSlotWrite::Clear) } -fn emit_inline_slot_write( - ctx: &mut FnCtx<'_>, - slot_idx: u32, - what: InlineSlotWrite<'_>, -) -> bool { +fn emit_inline_slot_write(ctx: &mut FnCtx<'_>, slot_idx: u32, what: InlineSlotWrite<'_>) -> bool { if !crate::codegen::helpers::inline_shadow_slot_enabled() { return false; } @@ -173,8 +165,7 @@ fn emit_inline_slot_write( let frame_top = ctx.block().load(I64, &frame_top_ptr); // `usize::MAX` is `-1` as an i64 bit pattern. let no_frame = ctx.block().icmp_eq(I64, &frame_top, "-1"); - ctx.block() - .cond_br(&no_frame, &done_label, &chk_len_label); + ctx.block().cond_br(&no_frame, &done_label, &chk_len_label); // --- `let slot = top + idx; if slot >= len { return }` --- // @@ -194,8 +185,7 @@ fn emit_inline_slot_write( ); let len = ctx.block().load(I64, &len_ptr); let in_bounds = ctx.block().icmp_ult(I64, &slot, &len); - ctx.block() - .cond_br(&in_bounds, &store_label, &done_label); + ctx.block().cond_br(&in_bounds, &store_label, &done_label); // --- the entry write --- ctx.current_block = store_idx; @@ -209,9 +199,7 @@ fn emit_inline_slot_write( ); ctx.block().load(PTR, &p) }; - let byte_off = ctx - .block() - .shl(I64, &slot, &SHADOW_ENTRY_SHIFT.to_string()); + let byte_off = ctx.block().shl(I64, &slot, &SHADOW_ENTRY_SHIFT.to_string()); let entry = ctx .block() .gep_inbounds(crate::types::I8, &buf, &[(I64, &byte_off)]); @@ -387,7 +375,8 @@ mod tests { fn entry_reg(blk: &str) -> String { for line in blk.lines() { // ` %rN = getelementptr inbounds i8, ptr %rBUF, i64 %rSHIFTED` - if line.contains("getelementptr inbounds i8, ptr %") && line.trim_end().ends_with(|c: char| c.is_ascii_digit()) + if line.contains("getelementptr inbounds i8, ptr %") + && line.trim_end().ends_with(|c: char| c.is_ascii_digit()) && line.contains(", i64 %") { if let Some(name) = line.trim().split(" = ").next() { diff --git a/crates/perry-codegen/src/expr/shadow_slot.rs b/crates/perry-codegen/src/expr/shadow_slot.rs index 6dc95cd827..4fca697ffd 100644 --- a/crates/perry-codegen/src/expr/shadow_slot.rs +++ b/crates/perry-codegen/src/expr/shadow_slot.rs @@ -138,7 +138,7 @@ pub(crate) fn emit_shadow_slot_clear(ctx: &mut FnCtx<'_>, slot_idx: u32) { if ctx.suppressed_cleared_shadow_slots.contains(&slot_idx) { return; } - // #7086: emitted inline against this activation's cached `ShadowStackState` + // #7088: emitted inline against this activation's cached `ShadowStackState` // pointer when it has one; falls through to the call otherwise. if super::shadow_inline::emit_inline_slot_clear(ctx, slot_idx) { return; @@ -197,7 +197,7 @@ pub(crate) fn emit_shadow_slot_bind_for_local(ctx: &mut FnCtx<'_>, local_id: u32 return; }; ctx.shadow_slots_bound.insert(slot_idx); - // #7086: the hot per-store root write. Emitted inline against this + // #7088: the hot per-store root write. Emitted inline against this // activation's cached `ShadowStackState` pointer when it has one; falls // through to the call otherwise. if super::shadow_inline::emit_inline_slot_bind(ctx, slot_idx, &local_slot) { diff --git a/crates/perry-codegen/src/function.rs b/crates/perry-codegen/src/function.rs index 3fb1967769..5f55a76c45 100644 --- a/crates/perry-codegen/src/function.rs +++ b/crates/perry-codegen/src/function.rs @@ -104,7 +104,7 @@ pub struct LlFunction { /// flipping the default across every user function. shadow_frame_slot: Option, /// Entry alloca holding this thread's `ShadowStackState` address, so the - /// inline slot stores (#7086) can address the buffer without a per-store + /// inline slot stores (#7088) can address the buffer without a per-store /// thread-local lookup. Set alongside `shadow_frame_slot`. shadow_state_slot: Option, /// Whether shadow-frame emission was requested for this function at all @@ -138,7 +138,7 @@ pub struct LlFunction { /// /// `js_shadow_frame_enter` is `js_shadow_frame_push` returning the address of /// this thread's `ShadowStackState` instead of the frame handle, so the inline -/// slot stores (#7086) get their base pointer without a second thread-local +/// slot stores (#7088) get their base pointer without a second thread-local /// lookup. The handle the matching pop needs is recovered from the state by /// [`shadow_frame_handle_lines`] — `handle == frame_top - HEADER_SLOTS` — so /// the pop side is untouched. diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 955df6a8bf..32d5cb5a9d 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -42,7 +42,7 @@ pub use codegen::{ }; /// The shadow-stack field offsets generated code bakes into its inline root -/// stores (#7086). +/// stores (#7088). /// /// Exported so `perry`'s `shadow_layout_contract` test can compare them with /// `perry-runtime`'s copy. `perry-codegen` does not depend on `perry-runtime`, diff --git a/crates/perry-codegen/src/module.rs b/crates/perry-codegen/src/module.rs index 11aca0f298..297d1642f7 100644 --- a/crates/perry-codegen/src/module.rs +++ b/crates/perry-codegen/src/module.rs @@ -283,7 +283,7 @@ impl LlModule { /// /// Used for `js_shadow_frame_enter`, whose `nonnull` return is what lets /// LLVM fold away the null-state fallback arm that every inline shadow-slot - /// store emits (#7086). The attribute is true by construction: the runtime + /// store emits (#7088). The attribute is true by construction: the runtime /// returns the address of a `thread_local!`. pub fn declare_function_with_ret_attrs( &mut self, diff --git a/crates/perry-codegen/src/runtime_decls/arrays.rs b/crates/perry-codegen/src/runtime_decls/arrays.rs index 735d910285..b1e78828cd 100644 --- a/crates/perry-codegen/src/runtime_decls/arrays.rs +++ b/crates/perry-codegen/src/runtime_decls/arrays.rs @@ -100,7 +100,7 @@ pub fn declare_phase_b_arrays(module: &mut LlModule) { // // `js_shadow_frame_enter` is `js_shadow_frame_push` returning the address // of this thread's shadow state rather than the frame handle, so the - // inline slot stores (#7086) get a base pointer without a second + // inline slot stores (#7088) get a base pointer without a second // thread-local lookup per activation. It is the entry point shadow-frame // emission actually uses; `js_shadow_frame_push` stays declared (and // exported) for stale cached objects and out-of-tree callers. diff --git a/crates/perry-codegen/tests/shadow_slot_hygiene.rs b/crates/perry-codegen/tests/shadow_slot_hygiene.rs index f0b7b63087..d60ea79c10 100644 --- a/crates/perry-codegen/tests/shadow_slot_hygiene.rs +++ b/crates/perry-codegen/tests/shadow_slot_hygiene.rs @@ -1,4 +1,4 @@ -//! NOTE (#7086): the hot shadow-slot stores are now emitted **inline** against +//! NOTE (#7088): the hot shadow-slot stores are now emitted **inline** against //! the `ShadowStackState` pointer `js_shadow_frame_enter` returns, not as //! `js_shadow_slot_bind` / `js_shadow_slot_set` calls. Each inline site still //! emits one of those calls on its null-state fallback arm, at exactly the @@ -776,7 +776,7 @@ fn flat_const_row_aliases_do_not_reserve_shadow_slots() { .expect("LLVM IR should be UTF-8"); let main_ir = function_slice(&ir, "main"); - // PRE-EXISTING RED, not caused by #7086: verified by running this suite + // PRE-EXISTING RED, not caused by #7088: verified by running this suite // against pristine `origin/main` sources, where the same assertion fails // with three reserved slots instead of one. Two row aliases now take a // persistent shadow slot each. This suite runs nightly/at-tag rather than diff --git a/crates/perry-runtime/src/gc/roots/shadow_stack.rs b/crates/perry-runtime/src/gc/roots/shadow_stack.rs index a5379c80c2..6f51685fd1 100644 --- a/crates/perry-runtime/src/gc/roots/shadow_stack.rs +++ b/crates/perry-runtime/src/gc/roots/shadow_stack.rs @@ -100,7 +100,7 @@ impl ShadowEntry { /// /// # Why this is `#[repr(C)]` with a hand-rolled buffer instead of a `Vec` /// -/// Generated code addresses these fields **inline** (#7086): a slot store is an +/// Generated code addresses these fields **inline** (#7088): a slot store is an /// address computation and a `stp` against this struct rather than a call into /// [`js_shadow_slot_set`] / [`js_shadow_slot_bind`]. That requires the field /// offsets to be a stable, checkable contract, and `Vec`'s layout is explicitly @@ -463,7 +463,7 @@ unsafe fn push_frame(s: &mut ShadowStackState, slot_count: u32) -> u64 { /// [`ShadowStackState`] instead of the frame handle. /// /// Generated code calls this once per activation and keeps the pointer for the -/// whole frame, so the inline slot stores (#7086) are address arithmetic +/// whole frame, so the inline slot stores (#7088) are address arithmetic /// against it rather than one `extern "C"` call — and one thread-local /// lookup — per store. The frame handle the matching /// [`js_shadow_frame_pop`] needs is recoverable without a second call: diff --git a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs index 3fd143ca11..60a181dfd6 100644 --- a/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs +++ b/crates/perry-runtime/src/gc/tests/shadow_stack_ops.rs @@ -471,7 +471,7 @@ fn out_of_range_frame_pop_is_ignored() { } // --------------------------------------------------------------------------- -// #7086: the inline slot-store addressing contract. +// #7088: the inline slot-store addressing contract. // // Generated code no longer calls `js_shadow_slot_bind` / `js_shadow_slot_set` // for the hot per-store root write. It computes the entry address itself from @@ -509,7 +509,11 @@ unsafe fn inline_bind_as_codegen_emits( let buf = state_word(state, SHADOW_STATE_PTR_OFFSET) as *mut u8; let entry = buf.add(slot * SHADOW_ENTRY_SIZE); let raw = local_slot as usize; - let bound = if raw & SHADOW_SLOT_ACTIVE_BIT == 0 { raw } else { 0 }; + let bound = if raw & SHADOW_SLOT_ACTIVE_BIT == 0 { + raw + } else { + 0 + }; *entry.cast::() = *local_slot; *entry.add(SHADOW_ENTRY_META_OFFSET).cast::() = bound | SHADOW_SLOT_ACTIVE_BIT; true @@ -546,7 +550,10 @@ fn frame_enter_pushes_the_same_frame_and_yields_the_same_handle() { let before = shadow_stack_depth(); let state = js_shadow_frame_enter(3); - assert!(!state.is_null(), "frame_enter must return the state address"); + assert!( + !state.is_null(), + "frame_enter must return the state address" + ); assert_eq!(shadow_stack_depth(), before + 1); assert_eq!( state as usize, @@ -575,7 +582,8 @@ fn inline_write_and_runtime_accessor_address_the_same_entry() { let _guard = GcTestIsolationGuard::new(); reset_shadow_stack(); let state = js_shadow_frame_enter(2); - let handle = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; + let handle = + unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; // inline write -> runtime read let mut storage: u64 = 0x7FFD_0000_DEAD_BEEF; @@ -617,7 +625,8 @@ fn inline_clear_matches_the_runtime_clear_and_keeps_the_binding() { let _guard = GcTestIsolationGuard::new(); reset_shadow_stack(); let state = js_shadow_frame_enter(1); - let handle = unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; + let handle = + unsafe { state_word(state, SHADOW_STATE_FRAME_TOP_OFFSET) } - SHADOW_STACK_HEADER_SLOTS; let mut storage: u64 = 0x7FFD_0000_0000_1111; js_shadow_slot_bind(0, &mut storage as *mut u64); @@ -702,8 +711,7 @@ fn inline_write_with_no_frame_installed_is_skipped_not_wrapped() { // buffer still holds this frame's entries. let buf = unsafe { state_word(state, SHADOW_STATE_PTR_OFFSET) } as *mut u8; let header_before = unsafe { *buf.cast::() }; - let header_meta_before = - unsafe { *buf.add(SHADOW_ENTRY_META_OFFSET).cast::() }; + let header_meta_before = unsafe { *buf.add(SHADOW_ENTRY_META_OFFSET).cast::() }; unsafe { *(state .cast::() diff --git a/crates/perry/src/commands/compile/object_cache.rs b/crates/perry/src/commands/compile/object_cache.rs index 750eb1f8e5..a4049bf63b 100644 --- a/crates/perry/src/commands/compile/object_cache.rs +++ b/crates/perry/src/commands/compile/object_cache.rs @@ -794,7 +794,7 @@ fn compute_object_cache_key_with_env( "env_shadow_stack", env_var("PERRY_SHADOW_STACK").as_deref().unwrap_or(""), ); - // #7086: flips the shadow-slot store between an inline sequence and the + // #7088: flips the shadow-slot store between an inline sequence and the // `js_shadow_slot_*` calls. Two arms that shared a cached object would // silently measure the same code. h.field( diff --git a/crates/perry/src/main.rs b/crates/perry/src/main.rs index 09ad05e08e..5c4fd7f450 100644 --- a/crates/perry/src/main.rs +++ b/crates/perry/src/main.rs @@ -3,9 +3,9 @@ //! CLI driver for compiling TypeScript to native executables. mod commands; +mod compat_reports; #[cfg(test)] mod shadow_layout_contract; -mod compat_reports; mod telemetry; #[cfg(test)] mod test_env_lock; diff --git a/crates/perry/src/shadow_layout_contract.rs b/crates/perry/src/shadow_layout_contract.rs index fd63d65e51..2c087bca8a 100644 --- a/crates/perry/src/shadow_layout_contract.rs +++ b/crates/perry/src/shadow_layout_contract.rs @@ -1,5 +1,5 @@ //! The shadow-stack layout contract between `perry-codegen` and -//! `perry-runtime` (#7086). +//! `perry-runtime` (#7088). //! //! Generated code writes GC roots *inline*: it computes the address of a //! `ShadowEntry` from the `ShadowStackState` pointer `js_shadow_frame_enter`