From 602aa32fe3a52ca8ca970b018177fce7c77acf05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 17:34:53 +0200 Subject: [PATCH 1/7] perf(gc): skip the copying minor's eligibility preflight when its answer is already known (#7645) The preflight traversed the whole live young graph to answer two booleans and produced no collection result. The malloc-registry question is already O(1); the pin question is O(live young graph) only because it SEARCHES for a fact that can be RECORDED when it is created. gc::pin_object becomes the single sanctioned setter of GC_FLAG_PINNED and arms a process-wide monotone latch when the pinned object sits in a space the copying minor relocates. With the latch clear and the malloc question decided, both walks provably return None. "No young pinned object exists" is stronger than the walk's "none is reachable", so the substitution is conservative. Six production pin sites are routed through it, three of them Eden-resident and none named by the issue: perry-stdlib's async_bridge promise pin, and the two AppKit string returns which wrote a raw '|= 0x04' on the header byte. move_young additionally aborts if a preflight-skipped cycle is ever about to relocate a pinned object -- the exact instant an incomplete latch becomes a use-after-move, at the cost of one 'and' on an already-loaded byte. The remembered-set arming that dirty_slot_preflight_reason used to trigger is kept at its original point in the cycle: it rebuilds the set from the heap assuming nothing is marked yet, and would otherwise have run after the copy phase had already evacuated root-reachable young objects. --- crates/perry-runtime/src/gc/copying.rs | 136 +++++++++-- crates/perry-runtime/src/gc/mod.rs | 10 + crates/perry-runtime/src/gc/pin.rs | 216 ++++++++++++++++++ crates/perry-runtime/src/gc/telemetry.rs | 6 + crates/perry-runtime/src/string/format.rs | 5 +- crates/perry-runtime/src/thread.rs | 9 +- .../perry-stdlib/src/common/async_bridge.rs | 6 +- crates/perry-ui-macos/src/ffi.rs | 8 + crates/perry-ui-macos/src/widgets/table.rs | 8 +- .../perry-ui-macos/src/widgets/textfield.rs | 15 +- 10 files changed, 382 insertions(+), 37 deletions(-) create mode 100644 crates/perry-runtime/src/gc/pin.rs diff --git a/crates/perry-runtime/src/gc/copying.rs b/crates/perry-runtime/src/gc/copying.rs index 7653d42760..54f6893a5a 100644 --- a/crates/perry-runtime/src/gc/copying.rs +++ b/crates/perry-runtime/src/gc/copying.rs @@ -578,6 +578,20 @@ impl CopyingNurseryCollector { return self.mark_addr(forwarded).unwrap_or(forwarded); } + // #7645: on a cycle that skipped the eligibility preflight, this is the + // exact instant an incomplete young-pin latch turns into a + // use-after-move: the collector is about to relocate a pinned object + // whose holder (the cross-thread promise queue, an AppKit string + // return) keeps a raw address no scanner will rewrite. `flags` is + // already loaded, so the check is one `and` and a never-taken branch. + // It is deliberately NOT applied when the preflight ran: that path is + // unchanged from before this issue, and a divergence between the + // preflight's traversal and the copier's is a separate bug that should + // not newly abort a program. + if self.stats.preflight_skipped && flags & GC_FLAG_PINNED != 0 { + pinned_young_move_under_skipped_preflight(header); + } + let total = (*header).size as usize; // Safety net (partial mitigation, NOT a full fix): a genuine // young/survivor object is always small — large objects are allocated @@ -838,6 +852,32 @@ pub(super) fn scan_remembered_dirty_slots_copying( stats } +/// The young-pin latch was clear, the preflight was skipped on that proof, and +/// the copier then met a pinned young object anyway — so the latch is +/// incomplete and a pin site exists that does not go through `gc::pin_object`. +/// +/// There is no recovery: leaving the object in from-space strands the +/// referring slot on memory `copying_reset_from_spaces_and_flip` is about to +/// retire, and moving it invalidates a raw address nothing will rewrite. Abort +/// loudly at the faulting site instead of corrupting the heap silently. +#[cold] +#[inline(never)] +unsafe fn pinned_young_move_under_skipped_preflight(header: *mut GcHeader) -> ! { + eprintln!( + "[gc-pin-latch] FATAL: copying minor is about to relocate a PINNED young \ + object on a preflight-skipped cycle. header={:#x} obj_type={} size={} \ + flags={:#04x}\n\ + The young-pin latch (gc/pin.rs) is incomplete: some site sets \ + GC_FLAG_PINNED without going through gc::pin_object. Find it with \ + `python3 scripts/gc_pin_sites.py` and route it through pin_object (#7645).", + header as usize, + (*header).obj_type, + (*header).size, + (*header).gc_flags, + ); + std::process::abort() +} + pub(super) struct CopiedMinorEligibility { pub(super) eligible: bool, pub(super) fallback_reason: CopiedMinorFallbackReason, @@ -845,6 +885,10 @@ pub(super) struct CopiedMinorEligibility { pub(super) malloc_validation_lookups: usize, pub(super) malloc_registry_rebuilds: u64, pub(super) legacy_root_stats: LegacyRootTraceStats, + /// #7645: both eligibility preflight walks were provably no-ops and were + /// skipped. Carried into the collector so `move_young` can abort rather + /// than relocate a pinned object on a cycle that took the unproven path. + pub(super) preflight_skipped: bool, pub(super) ptrs: Option, } @@ -880,21 +924,42 @@ impl CopiedMinorEligibility { legacy_root_stats, ); } - if let Some(reason) = Self::mutable_root_preflight_reason(&ptrs) { - return Self::fallback_with_ptrs_and_legacy( - reason, - malloc_sweep_due, - ptrs, - legacy_root_stats, - ); - } - if let Some(reason) = Self::dirty_slot_preflight_reason(&ptrs) { - return Self::fallback_with_ptrs_and_legacy( - reason, - malloc_sweep_due, - ptrs, - legacy_root_stats, - ); + // #7645: both walks below are a transitive traversal of the whole live + // young graph that answers two booleans and produces no collection + // result. When both booleans are already decided the traversal is + // provably a no-op, so skip it — see `preflight_walks_decided`. + let preflight_skipped = Self::preflight_walks_decided(&ptrs); + if preflight_skipped { + // The ONE side effect the skipped walks carried, kept at its + // original point in the cycle. `dirty_slot_preflight_reason` took + // a `remembered_dirty_snapshot()`, whose first call on a thread + // arms the barrier and rebuilds the remembered set from the heap + // — a walk that assumes "nothing is marked yet". Letting it fall + // through to the copy phase's snapshot would run it AFTER + // `visit_mutable_root_slots` had already evacuated root-reachable + // young objects, i.e. against a half-moved heap. It is a one-shot + // per thread (`REMEMBERED_SET_RECONSTRUCTED`), so on every later + // cycle this is a thread-local flag read. + arm_and_reconstruct_remembered_set_if_unarmed(); + note_preflight_skipped(); + } else { + note_preflight_walked(); + if let Some(reason) = Self::mutable_root_preflight_reason(&ptrs) { + return Self::fallback_with_ptrs_and_legacy( + reason, + malloc_sweep_due, + ptrs, + legacy_root_stats, + ); + } + if let Some(reason) = Self::dirty_slot_preflight_reason(&ptrs) { + return Self::fallback_with_ptrs_and_legacy( + reason, + malloc_sweep_due, + ptrs, + legacy_root_stats, + ); + } } Self { @@ -904,10 +969,38 @@ impl CopiedMinorEligibility { malloc_validation_lookups: ptrs.malloc_validation_lookups(), malloc_registry_rebuilds: ptrs.malloc_registry_rebuilds(), legacy_root_stats, + preflight_skipped, ptrs: Some(ptrs), } } + /// Are both of the preflight walks' outputs already known? + /// + /// The walks can only produce three verdicts, and each has an O(1) proof + /// of absence: + /// + /// * `PinnedYoungRoot` / `PinnedYoungDirtySlot` / `PinnedYoungTransitive` + /// come from `CopyingNurseryPreflight::check_ptr_with_reason`, which + /// trips only on an `Eden`/`FromSurvivor` object carrying + /// `GC_FLAG_PINNED`. `gc::pin` records every creation of such a pin in a + /// monotone latch, so a clear latch means no such object exists — which + /// is strictly stronger than "none is reachable". + /// * `MallocRegistryUnavailable` comes from + /// `CopyingPointerSet::classify_for_preflight`, which returns it only + /// when a non-arena candidate is met while the malloc registry is both + /// unavailable *and* was non-empty at cycle start. If the registry is + /// available, or was empty at start, no candidate can produce it. + /// + /// When either proof is unavailable the walk runs exactly as before, so + /// the decision this function guards is never *changed* — only skipped + /// when its outcome is already determined. + fn preflight_walks_decided(ptrs: &CopyingPointerSet) -> bool { + if young_pin_latch_armed() { + return false; + } + ptrs.malloc_registry_available.get() || ptrs.malloc_registry_empty_at_start + } + pub(super) fn fallback(reason: CopiedMinorFallbackReason, malloc_sweep_due: bool) -> Self { Self { eligible: false, @@ -916,6 +1009,7 @@ impl CopiedMinorEligibility { malloc_validation_lookups: 0, malloc_registry_rebuilds: 0, legacy_root_stats: LegacyRootTraceStats::default(), + preflight_skipped: false, ptrs: None, } } @@ -933,6 +1027,7 @@ impl CopiedMinorEligibility { malloc_validation_lookups: ptrs.malloc_validation_lookups(), malloc_registry_rebuilds: ptrs.malloc_registry_rebuilds(), legacy_root_stats, + preflight_skipped: false, ptrs: Some(ptrs), } } @@ -944,6 +1039,7 @@ impl CopiedMinorEligibility { malloc_sweep_due: self.malloc_sweep_due, malloc_validation_lookups: self.malloc_validation_lookups, malloc_registry_rebuilds: self.malloc_registry_rebuilds, + preflight_skipped: self.preflight_skipped, ..CopyingNurseryTraceStats::default() } } @@ -1037,13 +1133,18 @@ pub(super) fn gc_collect_minor_copying_fast_path_with_eligibility( CopiedMinorFallbackReason::PinnedYoungTransitive => "pinned_young_transitive", }; eprintln!( - "[gc-copy-minor] eligible={} fallback={}", - eligibility.eligible, reason + "[gc-copy-minor] eligible={} fallback={} preflight_skipped={} (skips={} walks={})", + eligibility.eligible, + reason, + eligibility.preflight_skipped, + super::copied_minor_preflight_skips(), + super::copied_minor_preflight_walks(), ); } if !eligibility.eligible { return None; } + let preflight_skipped = eligibility.preflight_skipped; let malloc_sweep_due = eligibility.malloc_sweep_due; let ptrs = eligibility .ptrs @@ -1055,6 +1156,7 @@ pub(super) fn gc_collect_minor_copying_fast_path_with_eligibility( collector.stats.eligible = true; collector.stats.fallback_reason = CopiedMinorFallbackReason::None; collector.stats.malloc_sweep_due = malloc_sweep_due; + collector.stats.preflight_skipped = preflight_skipped; collector.stats.reset_blocks += crate::arena::copying_prepare_to_space(); let native_stack_walk = visit_mutable_root_slots(|slot| unsafe { diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 0d62bd0c0f..f5f4304d59 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -96,6 +96,16 @@ mod barrier_arming; // rustc warned. A plain `use` brings them into `gc`'s namespace, which is all // the in-module callers (`telemetry.rs`, `cycle.rs`) actually need. use barrier_arming::*; +/// #7645: `GC_FLAG_PINNED` custody + the young-pin latch the copying minor's +/// eligibility preflight is skipped on. Every write of the bit goes through +/// `pin::pin_object`; `scripts/gc_pin_sites.py` enforces that in `lint`. +mod pin; +#[cfg(test)] +pub(crate) use pin::test_reset_young_pin_latch; +pub use pin::{ + copied_minor_preflight_skips, copied_minor_preflight_walks, pin_object, unpin_object, +}; +use pin::{note_preflight_skipped, note_preflight_walked, young_pin_latch_armed}; mod copying; use copying::*; // The copied-minor pointer classifier is consumed by the weak-holder registry diff --git a/crates/perry-runtime/src/gc/pin.rs b/crates/perry-runtime/src/gc/pin.rs new file mode 100644 index 0000000000..b6fdcf2d20 --- /dev/null +++ b/crates/perry-runtime/src/gc/pin.rs @@ -0,0 +1,216 @@ +//! `GC_FLAG_PINNED` custody, and the young-pin latch the copying minor's +//! eligibility preflight is skipped on (#7645). +//! +//! # Why this module exists +//! +//! The copying minor traverses the young object graph **twice**: once in +//! `CopiedMinorEligibility::evaluate`'s preflight, to prove nothing reachable +//! is pinned, and again to copy. On `json_pipeline` the first traversal is +//! ~22% of the hot phase and produces no collection result at all. +//! +//! The preflight walk (`CopyingNurseryPreflight::drain`) answers exactly two +//! questions: +//! +//! 1. Is any transitively reachable `Eden`/`FromSurvivor` object +//! `GC_FLAG_PINNED`? (`check_ptr_with_reason`) +//! 2. Was a non-arena candidate seen while the malloc registry was +//! unavailable and non-empty at cycle start? +//! (`classify_for_preflight`) +//! +//! (2) is already decidable in O(1) from `CopyingPointerSet`'s two fields. +//! (1) is O(live young graph) — but only because it *searches* for a fact that +//! can instead be *recorded at the moment it is created*. That is what this +//! module does: every write of `GC_FLAG_PINNED` goes through [`pin_object`], +//! which arms a process-wide monotone latch when (and only when) the pinned +//! object is in a space the copying minor would relocate. +//! +//! When the latch is clear, no object anywhere carries a young pin, so the +//! walk provably returns `None` and skipping it is observationally equivalent +//! (modulo the layout/malloc-lookup telemetry counters the walk incremented). +//! Note the direction: "no young pinned object exists at all" is *stronger* +//! than the walk's "no young pinned object is reachable", so the substitution +//! is conservative, not merely equal. +//! +//! # The safety argument, and what enforces it +//! +//! Skipping this guard is a use-after-move if it is ever wrong: `move_young` +//! relocates a pinned object exactly as it would any other (it only *preserves* +//! the bit, `copying.rs`), and the raw `usize` in `PENDING_THREAD_RESULTS` has +//! no scanner to rewrite. So the latch's completeness is load-bearing and is +//! enforced three ways, not asserted in prose: +//! +//! * **Statically, at every write site.** `scripts/gc_pin_sites.py` (run in +//! `lint`) fails on any source line that sets the pinned bit outside +//! [`pin_object`], and equally on an allowlist entry that no longer matches +//! anything. It deliberately matches both the named-constant form +//! (`gc_flags |= GC_FLAG_PINNED`) and the raw-byte form +//! (`*gc_flags_ptr |= 0x04`) — two of the six pin sites that existed when +//! this landed used the raw byte and are invisible to a +//! `grep GC_FLAG_PINNED`. +//! * **Dynamically, at the moment it would matter.** `move_young` checks the +//! pinned bit on the flags byte it has already loaded, and aborts if a +//! *preflight-skipped* cycle is about to relocate a pinned object. That is +//! the precise instant an incomplete latch becomes memory corruption, and it +//! costs one `and` plus a never-taken branch. +//! * **In tests.** The copying suite's pinned-fallback tests plant their pins +//! through [`pin_object`], so deleting the arming below turns them red +//! rather than leaving them green on an unsound configuration. +//! +//! # Why the latch is monotone +//! +//! A decrementing counter would recover the fast path after a transient pin +//! (a settled `fetch` promise, say). It was rejected for this change because +//! it adds a *second* completeness obligation of the same severity: every +//! unpin site must also be complete, and a spurious or double decrement is +//! silently unsound in exactly the same use-after-move way. Monotone needs one +//! proof. A process that has ever pinned young pays the walk forever, which is +//! the conservative direction. +//! +//! Concretely, the pin sites that arm the latch in production are the +//! Eden-resident ones — `js_promise_new()` promises pinned for native +//! resolution (`perry-stdlib`'s `async_bridge`, i.e. fetch/zlib/ws/bcrypt), +//! `Atomics.waitAsync`, and the AppKit text reads. Programs that use them get +//! today's behaviour; compute- and JSON-shaped programs get the walk removed. + +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + +use super::types::{GcHeader, GC_FLAG_ARENA, GC_FLAG_PINNED}; + +/// Has any object in a space the copying minor relocates ever been pinned? +/// +/// Monotone: set by [`pin_object`], never cleared outside tests. Cleared only +/// through [`test_reset_young_pin_latch`], which the copying-nursery test +/// isolation guard calls while holding the suite's global lock. +static YOUNG_PIN_EVER: AtomicBool = AtomicBool::new(false); + +/// Copying minors that skipped both preflight walks. The live-subject counter +/// for any "the preflight is gone" verdict — a benchmark or gate that reports +/// a win without this being non-zero measured nothing (#7024/#7025). +static PREFLIGHT_SKIPS: AtomicU64 = AtomicU64::new(0); + +/// Copying minors that ran the preflight walks. +static PREFLIGHT_WALKS: AtomicU64 = AtomicU64::new(0); + +/// Set `GC_FLAG_PINNED` on `header`, arming the young-pin latch if this pin +/// constrains the copying minor. +/// +/// **This is the only sanctioned way to set the bit.** See the module docs for +/// what rests on that and what enforces it. +/// +/// # Safety +/// +/// `header` must point at a live `GcHeader` (i.e. `user_ptr - GC_HEADER_SIZE` +/// of a live allocation). +#[inline] +pub unsafe fn pin_object(header: *mut GcHeader) { + if header.is_null() { + return; + } + if pin_constrains_copying_minor(header) { + // Release so a collector on another thread that observes the latch + // also observes the flag write below it in program order. + YOUNG_PIN_EVER.store(true, Ordering::Release); + } + (*header).gc_flags |= GC_FLAG_PINNED; +} + +/// Clear `GC_FLAG_PINNED` on `header`. Does **not** disarm the latch — see the +/// module docs on why the latch is monotone. +/// +/// # Safety +/// +/// As [`pin_object`]. +#[inline] +pub unsafe fn unpin_object(header: *mut GcHeader) { + if header.is_null() { + return; + } + (*header).gc_flags &= !GC_FLAG_PINNED; +} + +/// Would a pin on `header` be able to force `CopiedMinorFallbackReason:: +/// PinnedYoung*`? +/// +/// `CopyingNurseryPreflight::check_ptr_with_reason` trips only on +/// `CopyingPointerKind::Eden` / `FromSurvivor`, and `CopyingPointerSet:: +/// classify_arena` reaches those kinds only for an address whose header sits +/// in `NurseryEden`/`Survivor0`/`Survivor1` of *this thread's* arena. So: +/// +/// * A malloc-space object (no `GC_FLAG_ARENA`) is never `Eden`/`FromSurvivor` +/// and is never relocated by a copying minor. It cannot arm the latch — +/// which is what keeps `spawn`'s deliberately malloc-resident cross-thread +/// promise (`thread.rs`) from costing every later cycle a walk. +/// * `Longlived` and `Old` are likewise never relocated by a copying minor, +/// which is why the `SMALL_INT_CACHE` pins (`string/format.rs`, allocated +/// through `js_string_from_bytes_longlived`) are free. +/// * Anything else — the nursery spaces, and `Unknown`, which is what another +/// agent's arena classifies as from here — arms it. +/// +/// Spaces never flow backwards (nothing in `Longlived`/`Old` re-enters the +/// nursery), so a decision taken at pin time stays valid for as long as the +/// pin does. +/// +/// # Safety +/// +/// As [`pin_object`]. +#[inline] +unsafe fn pin_constrains_copying_minor(header: *mut GcHeader) -> bool { + if (*header).gc_flags & GC_FLAG_ARENA == 0 { + return false; + } + !matches!( + crate::arena::classify_heap_space(header as usize), + crate::arena::HeapSpace::Longlived | crate::arena::HeapSpace::Old + ) +} + +/// Has a young pin ever been created? While this is false the eligibility +/// preflight's pin question is answered. +#[inline] +pub(super) fn young_pin_latch_armed() -> bool { + YOUNG_PIN_EVER.load(Ordering::Acquire) +} + +#[inline] +pub(super) fn note_preflight_skipped() { + PREFLIGHT_SKIPS.fetch_add(1, Ordering::Relaxed); +} + +#[inline] +pub(super) fn note_preflight_walked() { + PREFLIGHT_WALKS.fetch_add(1, Ordering::Relaxed); +} + +/// Copying minors that skipped both eligibility preflight walks. +pub fn copied_minor_preflight_skips() -> u64 { + PREFLIGHT_SKIPS.load(Ordering::Relaxed) +} + +/// Copying minors that ran the eligibility preflight walks. +pub fn copied_minor_preflight_walks() -> u64 { + PREFLIGHT_WALKS.load(Ordering::Relaxed) +} + +/// Clear the latch so a test starts from a known state. Callers must hold the +/// copying-nursery isolation lock; `reset_copying_nursery_runtime_test_state` +/// does. +#[cfg(test)] +pub(crate) fn test_reset_young_pin_latch() { + YOUNG_PIN_EVER.store(false, Ordering::Release); +} + +/// `extern "C"` form of [`pin_object`] taking the **user** pointer, for crates +/// that reach the runtime through FFI declarations rather than a Rust +/// dependency edge (`perry-ui-macos`, which used to open-code +/// `*(ptr - 8 + 1) |= 0x04`). +/// +/// # Safety +/// +/// `user_ptr` must be a live allocation preceded by an 8-byte `GcHeader`. +#[no_mangle] +pub unsafe extern "C" fn js_gc_pin_user_ptr(user_ptr: *mut u8) { + if user_ptr.is_null() { + return; + } + pin_object(user_ptr.sub(super::types::GC_HEADER_SIZE) as *mut GcHeader); +} diff --git a/crates/perry-runtime/src/gc/telemetry.rs b/crates/perry-runtime/src/gc/telemetry.rs index f3739db726..61d48cc690 100644 --- a/crates/perry-runtime/src/gc/telemetry.rs +++ b/crates/perry-runtime/src/gc/telemetry.rs @@ -222,6 +222,12 @@ pub(super) struct CopyingNurseryTraceStats { pub(super) malloc_validation_lookups: usize, pub(super) malloc_registry_rebuilds: u64, pub(super) malloc_sweep_due: bool, + /// #7645: the eligibility preflight's two young-graph walks were provably + /// no-ops (no young pin has ever been created, and the malloc-registry + /// question was already answered) and were skipped. This is the live- + /// subject flag for the "the second traversal is gone" claim: a row with + /// `eligible=true` and `preflight_skipped=false` did the old work. + pub(super) preflight_skipped: bool, pub(super) fallback_reason: CopiedMinorFallbackReason, } diff --git a/crates/perry-runtime/src/string/format.rs b/crates/perry-runtime/src/string/format.rs index 767c169649..0d8d4c8ae9 100644 --- a/crates/perry-runtime/src/string/format.rs +++ b/crates/perry-runtime/src/string/format.rs @@ -84,10 +84,11 @@ pub extern "C" fn js_number_to_string(value: f64) -> *mut StringHeader { // Mark as shared so it's never mutated in-place (*ptr).refcount = 0; // Mark as pinned so GC keeps it live for the lifetime of this - // thread's arena. + // thread's arena. Longlived-space (see the allocation above), so + // this does not arm the young-pin latch (#7645). let gc_header = (ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; - (*gc_header).gc_flags |= crate::gc::GC_FLAG_PINNED; + crate::gc::pin_object(gc_header); } SMALL_INT_CACHE.with(|c| unsafe { // GC_STORE_AUDIT(ROOT): SMALL_INT_CACHE is scanned by scan_small_int_cache_roots_mut. diff --git a/crates/perry-runtime/src/thread.rs b/crates/perry-runtime/src/thread.rs index 91edb4e8d4..1aeda5db14 100644 --- a/crates/perry-runtime/src/thread.rs +++ b/crates/perry-runtime/src/thread.rs @@ -1485,9 +1485,10 @@ unsafe fn spawn_impl(closure_val: f64) -> *mut crate::promise::Promise { // while pinned. Malloc space is non-moving and sweeps honor the pin. let promise = crate::promise::js_promise_new_cross_thread(); - // Pin the promise so GC doesn't collect it while the thread is running + // Pin the promise so GC doesn't collect it while the thread is running. + // Malloc-resident (see above), so this does not arm the young-pin latch. let promise_header = (promise as *mut u8).sub(gc::GC_HEADER_SIZE) as *mut gc::GcHeader; - (*promise_header).gc_flags |= gc::GC_FLAG_PINNED; + gc::pin_object(promise_header); let promise_usize = promise as usize; // #6185: the promise lives in the SPAWNING agent's heap, so that is the @@ -1614,7 +1615,7 @@ pub fn thread_job_begin() { /// `promise` must be a live promise allocation preceded by an 8-byte GcHeader. pub unsafe fn pin_promise(promise: *mut crate::promise::Promise) { let header = (promise as *mut u8).sub(gc::GC_HEADER_SIZE) as *mut gc::GcHeader; - (*header).gc_flags |= gc::GC_FLAG_PINNED; + gc::pin_object(header); } /// Resolve the promise at `promise_usize` with a UTF-8 string on the agent that @@ -1698,7 +1699,7 @@ pub extern "C" fn js_thread_process_pending() -> i32 { // Unpin the promise now that we're settling it. let promise_header = (promise as *mut u8).sub(gc::GC_HEADER_SIZE) as *mut gc::GcHeader; - (*promise_header).gc_flags &= !gc::GC_FLAG_PINNED; + gc::unpin_object(promise_header); // #6185: a worker that returned a non-transferable value (e.g. // `spawn(() => new Map())`) can't throw on its own thread (no diff --git a/crates/perry-stdlib/src/common/async_bridge.rs b/crates/perry-stdlib/src/common/async_bridge.rs index 652e3d3f7f..dadf377788 100644 --- a/crates/perry-stdlib/src/common/async_bridge.rs +++ b/crates/perry-stdlib/src/common/async_bridge.rs @@ -50,7 +50,9 @@ pub unsafe fn pin_promise_for_native_resolution(promise_ptr: usize) { } let header = (promise_ptr as *mut u8).sub(perry_runtime::gc::GC_HEADER_SIZE) as *mut perry_runtime::gc::GcHeader; - (*header).gc_flags |= perry_runtime::gc::GC_FLAG_PINNED; + // `js_promise_new()` allocates in the arena (Eden) unless promise hooks are + // active, so this pin DOES arm the copying minor's young-pin latch (#7645). + perry_runtime::gc::pin_object(header); } /// Inverse of [`pin_promise_for_native_resolution`]; called from @@ -64,7 +66,7 @@ unsafe fn unpin_promise_after_native_resolution(promise_ptr: usize) { } let header = (promise_ptr as *mut u8).sub(perry_runtime::gc::GC_HEADER_SIZE) as *mut perry_runtime::gc::GcHeader; - (*header).gc_flags &= !perry_runtime::gc::GC_FLAG_PINNED; + perry_runtime::gc::unpin_object(header); } /// Allocate a fresh Promise and pin it for cross-thread resolution. diff --git a/crates/perry-ui-macos/src/ffi.rs b/crates/perry-ui-macos/src/ffi.rs index c29fb0fff9..3d3e629c7a 100644 --- a/crates/perry-ui-macos/src/ffi.rs +++ b/crates/perry-ui-macos/src/ffi.rs @@ -24,6 +24,14 @@ extern "C" { /// Canonical: perry-runtime `js_get_string_pointer_unified(value: f64) -> i64`. pub fn js_get_string_pointer_unified(value: f64) -> i64; + + /// Canonical: perry-runtime `js_gc_pin_user_ptr(user_ptr: *mut u8)` — sets + /// `GC_FLAG_PINNED` on the allocation's header AND arms the copying + /// minor's young-pin latch (#7645). Call this instead of open-coding + /// `*(ptr - 8 + 1) |= 0x04`: a raw byte write pins the object without + /// telling the collector, which then relocates it out from under the + /// pointer we just returned to JS. + pub fn js_gc_pin_user_ptr(user_ptr: *mut u8); } // --------------------------------------------------------------------------- diff --git a/crates/perry-ui-macos/src/widgets/table.rs b/crates/perry-ui-macos/src/widgets/table.rs index 41219ad214..cfa7ab6668 100644 --- a/crates/perry-ui-macos/src/widgets/table.rs +++ b/crates/perry-ui-macos/src/widgets/table.rs @@ -1,4 +1,4 @@ -use crate::ffi::js_string_from_bytes; +use crate::ffi::{js_gc_pin_user_ptr, js_string_from_bytes}; use objc2::msg_send; use objc2::rc::Retained; use objc2::runtime::{AnyClass, AnyObject}; @@ -569,9 +569,9 @@ pub fn get_filter_text(handle: i64) -> *const u8 { let bytes = text.as_bytes(); unsafe { let ptr = js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32); - // Pin the GC allocation: GcHeader sits at ptr-8, gc_flags at offset 1. - let gc_flags_ptr = (ptr as *mut u8).sub(8).add(1); - *gc_flags_ptr |= 0x04; // GC_FLAG_PINNED + // Pin the GC allocation (mirrors `textfield::get_string_value`); the + // runtime helper also arms the young-pin latch (#7645). + js_gc_pin_user_ptr(ptr as *mut u8); ptr as *const u8 } } diff --git a/crates/perry-ui-macos/src/widgets/textfield.rs b/crates/perry-ui-macos/src/widgets/textfield.rs index 309b111b5c..49dd97ec39 100644 --- a/crates/perry-ui-macos/src/widgets/textfield.rs +++ b/crates/perry-ui-macos/src/widgets/textfield.rs @@ -1,4 +1,4 @@ -use crate::ffi::js_string_from_bytes; +use crate::ffi::{js_gc_pin_user_ptr, js_string_from_bytes}; use objc2::rc::Retained; use objc2::runtime::{AnyObject, Sel}; use objc2::{define_class, msg_send, AnyThread, DefinedClass}; @@ -263,13 +263,12 @@ pub fn get_string_value(handle: i64) -> *const u8 { let value = tf.stringValue(); let bytes = value.to_string(); let ptr = js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32); - // Pin the GC allocation so it won't be collected before the caller uses it. - // GcHeader layout: obj_type(u8) + gc_flags(u8) + reserved(u16) + size(u32) = 8 bytes - // GcHeader sits BEFORE the user pointer (ptr - 8). gc_flags is at offset 1. - // GC_FLAG_PINNED = 0x04 - // Pin the GC allocation so it survives until the caller consumes it - let gc_flags_ptr = (ptr as *mut u8).sub(8).add(1); - *gc_flags_ptr |= 0x04; // GC_FLAG_PINNED + // Pin the GC allocation so it survives until the caller consumes + // it. This string is Eden-resident, so the pin also arms the + // copying minor's young-pin latch — which is exactly why it must + // go through the runtime helper and not a raw `|= 0x04` on the + // header byte (#7645). + js_gc_pin_user_ptr(ptr as *mut u8); return ptr as *const u8; } } From ee482de696da25cf98880081cdf9233cb6cb52bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 17:35:02 +0200 Subject: [PATCH 2/7] test(gc): plant every pinned-fallback test through pin_object, and cover the latch The three test_copying_minor_falls_back_for_pinned_young_* cases planted their pin with a raw flag write, so after #7645 they would have passed on an unsound configuration instead of exercising the guard. They now go through pin_object, which is what makes deleting the latch arming turn them red. gc/tests/copying/latch.rs adds: the skip case (paired with a liveness assertion on the same trace, #7024/#7025), the Longlived- and malloc-pin cases that prove SMALL_INT_CACHE and spawn's cross-thread promise never arm the latch, the monotonicity case, and a subprocess sabotage test that plants a raw young pin and requires the collector to die on SIGABRT rather than relocate it. test_copying_minor_rewrites_exact_{object,closure}_pointer_* now expect masked_pointer_slots_read == 1 instead of 2 -- one read by the copier where there used to be one by each walk. That is the unit-scale witness of the removed traversal and fails if the walk ever returns. The copying-nursery isolation guard resets the latch, so one earlier pinning test cannot leave every later copying test on the slow path. --- crates/perry-runtime/src/gc/tests/alloc.rs | 4 +- crates/perry-runtime/src/gc/tests/barrier.rs | 2 +- crates/perry-runtime/src/gc/tests/copying.rs | 15 +- .../src/gc/tests/copying/latch.rs | 279 ++++++++++++++++++ .../gc/tests/copying/survival_and_malloc.rs | 16 +- crates/perry-runtime/src/gc/tests/oldgen.rs | 26 +- .../tests/runtime_roots/callback_scanners.rs | 4 +- crates/perry-runtime/src/gc/tests/support.rs | 5 + 8 files changed, 324 insertions(+), 27 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/copying/latch.rs diff --git a/crates/perry-runtime/src/gc/tests/alloc.rs b/crates/perry-runtime/src/gc/tests/alloc.rs index b2cf273304..c0655640e7 100644 --- a/crates/perry-runtime/src/gc/tests/alloc.rs +++ b/crates/perry-runtime/src/gc/tests/alloc.rs @@ -260,7 +260,7 @@ fn test_gc_pinned_flag() { let header = header_from_user_ptr(ptr); // Pin it - (*header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header); // Run GC - pinned objects should survive gc_collect_inner(); @@ -274,7 +274,7 @@ fn test_gc_pinned_flag() { assert!(tracked, "pinned object should survive GC"); // Unpin - (*header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(header); } } diff --git a/crates/perry-runtime/src/gc/tests/barrier.rs b/crates/perry-runtime/src/gc/tests/barrier.rs index 9e6facb649..72633d0a53 100644 --- a/crates/perry-runtime/src/gc/tests/barrier.rs +++ b/crates/perry-runtime/src/gc/tests/barrier.rs @@ -1768,7 +1768,7 @@ fn test_minor_gc_promotes_after_two_survivals() { let user_ptr = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT); unsafe { let header = header_from_user_ptr(user_ptr); - (*header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header); // Initial state: not yet survived, not tenured. assert_eq!((*header).gc_flags & GC_FLAG_HAS_SURVIVED, 0); assert_eq!((*header).gc_flags & GC_FLAG_TENURED, 0); diff --git a/crates/perry-runtime/src/gc/tests/copying.rs b/crates/perry-runtime/src/gc/tests/copying.rs index 37cff3ceee..02470b551e 100644 --- a/crates/perry-runtime/src/gc/tests/copying.rs +++ b/crates/perry-runtime/src/gc/tests/copying.rs @@ -1,6 +1,7 @@ mod adaptive_tenuring; mod all_pointer_elements_7469; mod deferred_finalize_7635; +mod latch; mod pointer_publish_7154; mod promise_side_tables; mod survival_and_malloc; @@ -488,7 +489,12 @@ fn test_copying_minor_rewrites_exact_object_pointer_slot_only() { assert_eq!(third, 33.0); assert!(crate::arena::pointer_in_nursery(obj_after)); assert!(crate::arena::pointer_in_nursery(child_after)); - assert_eq!(trace.layout_scans.masked_pointer_slots_read, 2); + // ONE read, not two: #7645 removed the eligibility preflight's traversal + // of the same graph, so the pointer slot is now visited only by the copy + // phase. This assertion is the unit-level witness of that counter drop — + // it must go back to 2 if the preflight walk ever returns. + assert_eq!(trace.layout_scans.masked_pointer_slots_read, 1); + assert!(trace.copying_nursery.preflight_skipped); assert_eq!(trace.layout_scans.unknown_layout_slots_read, 0); } @@ -522,7 +528,12 @@ fn test_copying_minor_rewrites_exact_closure_pointer_capture_only() { assert_eq!(third, 30.0); assert!(crate::arena::pointer_in_nursery(closure_after)); assert!(crate::arena::pointer_in_nursery(child_after)); - assert_eq!(trace.layout_scans.masked_pointer_slots_read, 2); + // ONE read, not two: #7645 removed the eligibility preflight's traversal + // of the same graph, so the pointer slot is now visited only by the copy + // phase. This assertion is the unit-level witness of that counter drop — + // it must go back to 2 if the preflight walk ever returns. + assert_eq!(trace.layout_scans.masked_pointer_slots_read, 1); + assert!(trace.copying_nursery.preflight_skipped); assert_eq!(trace.layout_scans.unknown_layout_slots_read, 0); } diff --git a/crates/perry-runtime/src/gc/tests/copying/latch.rs b/crates/perry-runtime/src/gc/tests/copying/latch.rs new file mode 100644 index 0000000000..7742fcfba7 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/copying/latch.rs @@ -0,0 +1,279 @@ +//! The young-pin latch that lets the copying minor skip its eligibility +//! preflight (#7645). +//! +//! Two things need proving here, and the second is the one that matters. +//! +//! 1. **The optimisation happens.** A cycle with no young pin reports +//! `preflight_skipped` and still copies objects. A gate that passes having +//! run zero copying minors proves nothing (#7024/#7025), so every assertion +//! below is paired with a liveness assertion on the same trace. +//! 2. **The guard it replaced still refuses.** A pin planted through +//! `gc::pin_object` — the sanctioned path — still forces +//! `PinnedYoungRoot`/`PinnedYoungDirtySlot`/`PinnedYoungTransitive`, and it +//! does so *because the latch was armed*. Deleting the arming in +//! `pin_object` turns `young_pin_via_pin_object_restores_the_walk` (and the +//! three `test_copying_minor_falls_back_for_pinned_*` cases in +//! `survival_and_malloc.rs`) red, which is what makes them worth having. +//! +//! Plus a third, cheaper line of defence, exercised by +//! [`raw_young_pin_that_bypasses_pin_object_aborts_the_copier`]: the collector +//! itself checks the pinned bit at the instant it is about to relocate an +//! object on a preflight-skipped cycle. That is the moment an incomplete latch +//! becomes a use-after-move, and the child process below plants exactly the +//! bug the static gate exists to prevent and asserts the collector dies rather +//! than corrupts. + +use super::super::super::*; +use super::super::support::*; + +/// Env var that unlocks the abort-child body. Belt and braces on top of +/// `#[ignore]`, so even `cargo test -- --include-ignored` cannot abort a +/// normal run. +const SABOTAGE_ENV: &str = "PERRY_TEST_PIN_LATCH_SABOTAGE"; + +fn preflight_skipped(trace: &GcCycleTrace) -> bool { + trace.copying_nursery.preflight_skipped +} + +/// A clean young graph: the walk is skipped, and the collector still ran. +#[test] +fn no_pin_ever_means_the_preflight_walks_are_skipped() { + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + + let skips_before = crate::gc::copied_minor_preflight_skips(); + let trace = collect_minor_trace(GcTriggerKind::Direct); + + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert!( + preflight_skipped(&trace), + "with no young pin ever created, both preflight walks are provably no-ops" + ); + assert_eq!( + crate::gc::copied_minor_preflight_skips(), + skips_before + 1, + "the process-wide skip counter must move with the trace flag" + ); + // Liveness: the subject actually ran. Without this the assertions above + // would pass on a cycle that collected nothing. + assert!( + trace.copying_nursery.copied_objects > 0 || trace.copying_nursery.promoted_objects > 0, + "copying minor must have moved something: {:?}/{:?}", + trace.copying_nursery.copied_objects, + trace.copying_nursery.promoted_objects + ); + let survivor = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(survivor, child, "the root must have been relocated"); +} + +/// The sanctioned pin path arms the latch, the walk comes back, and the walk +/// still refuses to move the pinned object. +/// +/// **This is the test that goes red if the arming in `gc::pin_object` is +/// deleted**: without it the latch stays clear, the walk is skipped, and the +/// fallback never happens. +#[test] +fn young_pin_via_pin_object_restores_the_walk() { + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + + // Baseline on the same heap shape: without the pin this cycle skips. + let clean = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&clean, true, CopiedMinorFallbackReason::None, false); + assert!(preflight_skipped(&clean)); + let survivor = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + + unsafe { + crate::gc::pin_object(header_from_user_ptr(survivor as *const u8)); + } + + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert!( + !preflight_skipped(&trace), + "a young pin must re-arm the latch and bring the walks back" + ); + assert_copied_minor_trace( + &trace, + false, + CopiedMinorFallbackReason::PinnedYoungRoot, + false, + ); + assert_eq!( + (js_shadow_slot_get(0) & POINTER_MASK) as usize, + survivor, + "a pinned young object must not move" + ); + unsafe { + crate::gc::unpin_object(header_from_user_ptr(survivor as *const u8)); + } +} + +/// The latch is monotone: unpinning does not bring the fast path back. +/// Documented behaviour, not an accident — see `gc/pin.rs` on why a +/// decrementing counter was rejected. +#[test] +fn the_latch_is_monotone_across_an_unpin() { + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + unsafe { + let header = header_from_user_ptr(child as *const u8); + crate::gc::pin_object(header); + crate::gc::unpin_object(header); + } + + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert!( + !preflight_skipped(&trace), + "the latch stays armed after an unpin — one completeness proof, not two" + ); +} + +/// A `Longlived` pin does not arm the latch. +/// +/// This is what keeps `string/format.rs`'s `SMALL_INT_CACHE` free: it pins +/// every cached small-integer string, and stringifying `0` is common enough +/// that arming on it would disable the optimisation for essentially every +/// program. `CopyingNurseryPreflight::check_ptr_with_reason` never trips on +/// `Longlived`, so those pins genuinely cannot constrain the copying minor. +#[test] +fn a_longlived_pin_does_not_arm_the_latch() { + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let bytes = b"longlived_pin_probe"; + let longlived = + crate::string::js_string_from_bytes_longlived(bytes.as_ptr(), bytes.len() as u32); + assert!( + matches!( + crate::arena::classify_heap_space(unsafe { + header_from_user_ptr(longlived as *const u8) + } as usize), + crate::arena::HeapSpace::Longlived + ), + "probe precondition: js_string_from_bytes_longlived must land in Longlived" + ); + unsafe { + crate::gc::pin_object(header_from_user_ptr(longlived as *const u8)); + } + + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + let trace = collect_minor_trace(GcTriggerKind::Direct); + + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert!( + preflight_skipped(&trace), + "a Longlived pin must not arm the young-pin latch" + ); + unsafe { + crate::gc::unpin_object(header_from_user_ptr(longlived as *const u8)); + } +} + +/// A malloc-space pin does not arm the latch — `spawn`'s cross-thread promise +/// is deliberately allocated there (`thread.rs`) precisely because the copying +/// minor never relocates it. +#[test] +fn a_malloc_pin_does_not_arm_the_latch() { + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let malloced = crate::gc::gc_malloc(64, GC_TYPE_OBJECT); + unsafe { + let header = header_from_user_ptr(malloced as *const u8); + assert_eq!( + (*header).gc_flags & GC_FLAG_ARENA, + 0, + "probe precondition: gc_malloc must not carry GC_FLAG_ARENA" + ); + crate::gc::pin_object(header); + } + + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + let trace = collect_minor_trace(GcTriggerKind::Direct); + + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert!( + preflight_skipped(&trace), + "a malloc-space pin must not arm the young-pin latch" + ); + unsafe { + crate::gc::unpin_object(header_from_user_ptr(malloced as *const u8)); + } +} + +/// SABOTAGE. Plant the exact bug `scripts/gc_pin_sites.py` exists to prevent — +/// a young pin created by a raw flag write that never tells the latch — and +/// require the collector to abort rather than relocate it. +/// +/// The body runs in a child process because it ends in `std::process::abort`, +/// which no in-process harness can catch. A green run here means the second +/// line of defence was exercised and fired; if the child exits cleanly the +/// guard is inert and this test fails. +#[test] +#[cfg(unix)] +fn raw_young_pin_that_bypasses_pin_object_aborts_the_copier() { + use std::os::unix::process::ExitStatusExt; + + if std::env::var_os(SABOTAGE_ENV).is_some() { + // We are the child; the #[ignore]d body below does the work. + return; + } + let exe = std::env::current_exe().expect("test binary path"); + let output = std::process::Command::new(exe) + .args([ + "--exact", + "gc::tests::copying::latch::pin_latch_sabotage_child", + "--ignored", + "--nocapture", + "--test-threads=1", + ]) + .env(SABOTAGE_ENV, "1") + .output() + .expect("spawn sabotage child"); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + output.status.signal(), + Some(libc::SIGABRT), + "the copier must abort when it meets a pinned young object on a \ + preflight-skipped cycle. status={:?} stdout={} stderr={}", + output.status, + String::from_utf8_lossy(&output.stdout), + stderr + ); + assert!( + stderr.contains("[gc-pin-latch] FATAL"), + "the abort must name the invariant it caught. stderr={stderr}" + ); +} + +/// Child body for [`raw_young_pin_that_bypasses_pin_object_aborts_the_copier`]. +/// Never runs in a normal `cargo test`: it is `#[ignore]`d *and* gated on +/// [`SABOTAGE_ENV`]. +#[test] +#[ignore = "aborts the process on purpose; driven by raw_young_pin_that_bypasses_pin_object_aborts_the_copier"] +fn pin_latch_sabotage_child() { + if std::env::var_os(SABOTAGE_ENV).is_none() { + return; + } + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let child = young_leaf(); + js_shadow_slot_set(0, ptr_bits(child)); + unsafe { + // The bug, planted verbatim: a young pin that never arms the latch. + // Allowlisted in scripts/gc_pin_sites.py by this binding's name — it + // must stay a raw write or the sabotage tests nothing. + let sabotage_plant_7645 = header_from_user_ptr(child as *const u8); + (*sabotage_plant_7645).gc_flags |= GC_FLAG_PINNED; + } + let _ = collect_minor_trace(GcTriggerKind::Direct); + panic!("copying minor relocated a pinned young object without aborting"); +} diff --git a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs index 9e9a2e2488..5699c71a24 100644 --- a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs +++ b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs @@ -43,7 +43,7 @@ fn test_copying_minor_preserves_old_page_accounting_for_defrag_policy() { CONS_PINNED.with(|s| s.borrow_mut().clear()); if !self.pinned_header.is_null() { unsafe { - (*self.pinned_header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(self.pinned_header); } } } @@ -103,7 +103,7 @@ fn test_copying_minor_preserves_old_page_accounting_for_defrag_policy() { unsafe { (*survivor_header).gc_flags |= GC_FLAG_MARKED; (*live_header).gc_flags |= GC_FLAG_MARKED; - (*pinned_header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(pinned_header); } let sweep = sweep_with_age_bump(false); @@ -535,7 +535,7 @@ fn test_copying_minor_falls_back_for_pinned_young_root() { let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); let child = young_leaf(); unsafe { - (*header_from_user_ptr(child as *const u8)).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header_from_user_ptr(child as *const u8)); } js_shadow_slot_set(0, ptr_bits(child)); @@ -550,7 +550,7 @@ fn test_copying_minor_falls_back_for_pinned_young_root() { ); assert_eq!(after, child); unsafe { - (*header_from_user_ptr(child as *const u8)).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(header_from_user_ptr(child as *const u8)); } } @@ -562,7 +562,7 @@ fn test_copying_minor_falls_back_for_pinned_young_dirty_slot() { let (old_arr, elements) = unsafe { alloc_old_test_array(1) }; unsafe { *elements = ptr_bits(child); - (*header_from_user_ptr(child as *const u8)).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header_from_user_ptr(child as *const u8)); } js_write_barrier_slot(ptr_bits(old_arr as usize), elements as u64, ptr_bits(child)); @@ -577,7 +577,7 @@ fn test_copying_minor_falls_back_for_pinned_young_dirty_slot() { ); assert_eq!(child_after, child); unsafe { - (*header_from_user_ptr(child as *const u8)).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(header_from_user_ptr(child as *const u8)); } } @@ -593,7 +593,7 @@ fn test_copying_minor_falls_back_for_transitive_pinned_young_child() { (arr as *mut u8).add(std::mem::size_of::()) as *mut u64; *elements = ptr_bits(child); layout_note_slot(arr as usize, 0, *elements); - (*header_from_user_ptr(child as *const u8)).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header_from_user_ptr(child as *const u8)); elements }; if gc_force_evacuate_enabled() { @@ -631,7 +631,7 @@ fn test_copying_minor_falls_back_for_transitive_pinned_young_child() { 0, "pinned child must not receive a forwarding pointer" ); - (*child_header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(child_header); } } diff --git a/crates/perry-runtime/src/gc/tests/oldgen.rs b/crates/perry-runtime/src/gc/tests/oldgen.rs index e8d925d213..51c95fa676 100644 --- a/crates/perry-runtime/src/gc/tests/oldgen.rs +++ b/crates/perry-runtime/src/gc/tests/oldgen.rs @@ -237,7 +237,7 @@ fn test_old_page_sweep_accounting_pinned_is_live_and_not_evacuation_eligible() { let pinned = crate::arena::arena_alloc_gc_old(40, 8, GC_TYPE_STRING) as usize; let (pinned_header, pinned_total) = old_test_header_and_size(pinned); unsafe { - (*pinned_header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(pinned_header); } let _sweep = sweep_with_age_bump(false); @@ -251,7 +251,7 @@ fn test_old_page_sweep_accounting_pinned_is_live_and_not_evacuation_eligible() { assert_eq!(summary.evacuation_eligible_pages, 0); unsafe { - (*pinned_header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(pinned_header); } } @@ -388,7 +388,7 @@ fn test_old_page_sweep_accounting_trace_json_includes_summary() { let pinned = crate::arena::arena_alloc_gc_old(40, 8, GC_TYPE_STRING) as usize; let (pinned_header, pinned_total) = old_test_header_and_size(pinned); unsafe { - (*pinned_header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(pinned_header); } let outcome = gc_collect_minor_with_trigger(GcTriggerSnapshot { @@ -423,7 +423,7 @@ fn test_old_page_sweep_accounting_trace_json_includes_summary() { ); unsafe { - (*pinned_header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(pinned_header); } } @@ -651,7 +651,8 @@ fn test_old_page_defrag_skips_pinned_old_objects() { selected_pages.insert(page); } unsafe { - (*pinned_header).gc_flags |= GC_FLAG_MARKED | GC_FLAG_PINNED; + (*pinned_header).gc_flags |= GC_FLAG_MARKED; + crate::gc::pin_object(pinned_header); } let mut new_headers = Vec::new(); @@ -671,7 +672,8 @@ fn test_old_page_defrag_skips_pinned_old_objects() { 0, "pinned old object address must remain stable" ); - (*pinned_header).gc_flags &= !(GC_FLAG_MARKED | GC_FLAG_PINNED); + (*pinned_header).gc_flags &= !GC_FLAG_MARKED; + crate::gc::unpin_object(pinned_header); } CONS_PINNED.with(|s| s.borrow_mut().clear()); } @@ -1094,7 +1096,7 @@ fn test_minor_skips_whole_heap_old_to_young_rebuild() { let (obj, _fields) = unsafe { alloc_old_test_object(2) }; let header = unsafe { header_from_user_ptr(obj as *const u8) }; unsafe { - (*header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header); } old_headers.push(header); } @@ -1131,7 +1133,7 @@ fn test_minor_skips_whole_heap_old_to_young_rebuild() { for header in old_headers { unsafe { - (*header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(header); } } clear_marks(); @@ -1168,7 +1170,7 @@ fn test_minor_preserves_old_to_young_edge_across_minors() { let parent_user = parent as usize; let parent_header = unsafe { header_from_user_ptr(parent as *const u8) }; unsafe { - (*parent_header).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(parent_header); } // Unrelated large old-gen set with no young children (makes the old gen // big enough that a whole-heap rebuild would be visibly costly). @@ -1177,7 +1179,7 @@ fn test_minor_preserves_old_to_young_edge_across_minors() { let (obj, _f) = unsafe { alloc_old_test_object(1) }; let h = unsafe { header_from_user_ptr(obj as *const u8) }; unsafe { - (*h).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(h); } other_old.push(h); } @@ -1279,9 +1281,9 @@ fn test_minor_preserves_old_to_young_edge_across_minors() { ); unsafe { - (*parent_header).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(parent_header); for h in other_old { - (*h).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(h); } } clear_marks(); diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs index c51a599c93..36f8a23c12 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs @@ -1043,7 +1043,7 @@ fn test_evacuation_verify_copy_only_pinned_root_allows_non_forwarded_target() { let user = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT); let valid_ptrs = build_valid_pointer_set(); unsafe { - (*header_from_user_ptr(user)).gc_flags |= GC_FLAG_PINNED; + crate::gc::pin_object(header_from_user_ptr(user)); } verify_copy_only_scanner_bits( POINTER_TAG | (user as u64 & POINTER_MASK), @@ -1051,7 +1051,7 @@ fn test_evacuation_verify_copy_only_pinned_root_allows_non_forwarded_target() { "copy-only root scanner", ); unsafe { - (*header_from_user_ptr(user)).gc_flags &= !GC_FLAG_PINNED; + crate::gc::unpin_object(header_from_user_ptr(user)); } } diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index 95b3cfa1e1..cbec3ba87f 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -357,6 +357,11 @@ fn reset_copying_nursery_runtime_test_state() { // the 4th survival); pin it so a heavy-influx test earlier on the same // thread cannot leak a lowered adaptive threshold in. crate::gc::tenuring::reset_for_test(); + // #7645: the young-pin latch is process-wide and monotone, so one + // earlier pinning test would otherwise leave every later copying test + // running the preflight — masking the skip path entirely. Callers hold + // the copying-nursery isolation lock, so this reset is not racy. + crate::gc::test_reset_young_pin_latch(); activate_malloc_registry_for_tests(); crate::object::test_clear_overflow_fields_root(); crate::object::test_clear_transition_cache_root(); From 2e1debddeb58c5295e1e1af3a8c2725297770015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 17:35:12 +0200 Subject: [PATCH 3/7] ci(gc): gate GC_FLAG_PINNED custody in lint The young-pin latch's completeness is what makes skipping the preflight sound, and a list in a comment is not a gate. gc_pin_sites.py fails on any site that originates a pin outside pin_object, and equally on a stale allowlist entry (the deferred_registration_flush_sites model). It matches BOTH shapes this tree has used: the named constant, and a write into any gc_flags-named identifier whose RHS carries an integer literal with bit 2 set. The second rule is not redundant -- two of the six pin sites wrote '*gc_flags_ptr |= 0x04' and were invisible to a grep for the constant, which is how the issue's own enumeration came back short by half. --self-test plants six offender shapes and requires each to be caught, plus the read/clear/preserve shapes to be left alone; a scan that sees fewer than 40 tokens exits 2 rather than reporting a vacuous green. The flag-byte channels it deliberately does not scan, and the one shape no textual scan can reach, are documented in the script with what covers them instead. It is a step of the already-required lint job, so it gates from its first run. --- .github/workflows/test.yml | 15 ++ scripts/gc_pin_sites.py | 372 +++++++++++++++++++++++++++++++++++++ 2 files changed, 387 insertions(+) create mode 100644 scripts/gc_pin_sites.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 50f036aa4f..cd9ac0a5e8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -202,6 +202,21 @@ jobs: if: ${{ !cancelled() }} run: python3 scripts/class_id_collisions.py + # #7645. The copying minor skips its eligibility preflight — the walk that + # proves nothing reachable is pinned — whenever the young-pin latch is + # clear. That is sound only while EVERY creation of GC_FLAG_PINNED goes + # through gc::pin_object, which is what arms the latch; a pin created any + # other way lets the collector relocate a pinned object whose holder keeps + # a raw address no scanner rewrites. This SCANS for both shapes the tree + # has used — `gc_flags |= GC_FLAG_PINNED` and the raw `*gc_flags_ptr |= + # 0x04` that hid two of the six pin sites from every grep — and fails on a + # stale allowlist entry as well as on a new site. + - name: GC pin-site custody audit + if: ${{ !cancelled() }} + run: | + python3 scripts/gc_pin_sites.py --self-test + python3 scripts/gc_pin_sites.py + # #7341 layer 3. A RuntimeHandleScope gives an object liveness; it does # nothing for a raw pointer already read out of the slot. Every rooting bug # in the quarantine sweep had rooting ALREADY -- what was missing was diff --git a/scripts/gc_pin_sites.py b/scripts/gc_pin_sites.py new file mode 100644 index 0000000000..4d3e61d3fa --- /dev/null +++ b/scripts/gc_pin_sites.py @@ -0,0 +1,372 @@ +#!/usr/bin/env python3 +"""GC_FLAG_PINNED custody gate (#7645). + +The copying minor skips its eligibility preflight — an O(live young graph) +traversal whose whole job is to prove nothing reachable is pinned — whenever +the young-pin latch in `crates/perry-runtime/src/gc/pin.rs` is clear. That +substitution is sound only if EVERY creation of a pin goes through +`gc::pin_object`, which is what arms the latch. A pin created any other way +leaves the collector free to relocate a pinned object, and the holders of those +pins keep raw addresses no scanner rewrites — i.e. a use-after-move. + +So the completeness of the pin-site list is load-bearing, and a list in a +comment is not a gate. This script is the gate. + +Two rules +--------- + +**A. named** — the token `GC_FLAG_PINNED` may only appear in a *masking* +position (`flags & (… | GC_FLAG_PINNED)`, `gc_flags &= !GC_FLAG_PINNED`), i.e. +reading or clearing the bit. Anything that ORs or assigns it into a flags byte +is a creation and must live in `gc/pin.rs` or be allowlisted. + +**B. raw byte** — any write (`=`/`|=`) into an identifier whose name contains +`gc_flags` whose right-hand side carries an integer literal with bit 2 set. + +Rule B is not redundant. When this gate was written, two of the six production +pin sites — `perry-ui-macos`'s textfield and table string reads — wrote +`*gc_flags_ptr |= 0x04;` and were invisible to `grep GC_FLAG_PINNED`. A gate +that knew only rule A would have certified a pin-site list missing a third of +its entries. + +How it fails +------------ + +* a creation site that is not `pin_object` and is not allowlisted -> exit 1 +* an allowlist entry that matches nothing any more -> exit 1 (a stale exemption + is how these gates rot; `deferred_registration_flush_sites` in + `crates/perry-runtime/src/arena/tests.rs` fails the same way) +* fewer than MIN_TOKENS `GC_FLAG_PINNED` tokens seen -> exit 2, because a regex + that stopped matching would otherwise report a clean, empty, green run + +`--self-test` plants every offender shape in a temp tree and requires the +scanner to reject each one, and requires it NOT to flag the legitimate +read/clear/preserve shapes. Run it before trusting a green scan. + +What this gate CANNOT see +------------------------- + +A flags byte reconstructed through a variable (`let preserved = flags & +(… | GC_FLAG_PINNED); (*new).gc_flags = … | preserved;` — `move_young`'s +copy) carries an existing pin forward but cannot originate one: the mask can +only pass through a bit the source object already had, and that object's pin +went through `pin_object`. Preservation is safe; creation is what is gated. + +The two other channels that write a header's flag byte were checked by hand +and cannot originate a pin either, so they are not scanned: + +* the allocators seed `GC_FLAG_ARENA | gc_birth_extra_flags()`, and + `GC_BIRTH_EXTRA_FLAGS` is only ever `0` or `GC_FLAG_MARKED` + (`gc/barrier.rs`, `gc/cycle.rs` — the only two writers); +* codegen's inline bump allocators (`perry-codegen`'s `array_literal.rs`, + `lower_call/new.rs`) emit the same `GC_FLAG_ARENA = 0x02` plus that same + birth byte. + +Both are worth re-checking if either ever grows a third flag source. + +One shape is genuinely out of reach of a textual scan: a numeric bit built up +through an intermediate local (`let mut f = (*h).gc_flags; f |= 4; +(*h).gc_flags = f;`). Rule A sees no token and rule B sees no literal on a +`gc_flags` target. That shape is covered by the SECOND layer instead — the +`move_young` check that aborts when a preflight-skipped cycle is about to +relocate a pinned object — which is why the change carries a runtime guard and +not only this script. +""" + +from __future__ import annotations + +import argparse +import os +import re +import sys +import tempfile +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# `GC_FLAG_PINNED` is bit 2 (0x04) of GcHeader::gc_flags. +PINNED_BIT = 0x04 + +TOKEN = "GC_FLAG_PINNED" + +# Rule B: an assignment into any identifier containing `gc_flags`. Covers the +# struct-field form (`(*header).gc_flags |= X`) and the raw-byte-pointer form +# (`*gc_flags_ptr |= X`) alike. +FLAG_WRITE = re.compile(r"\bgc_flags\w*\s*(?P\|=|=)(?!=)(?P[^;]*)") + +INT_LITERAL = re.compile(r"0[xX](?P[0-9A-Fa-f_]+)|\b(?P[0-9][0-9_]*)\b") + +STRING_LITERAL = re.compile(r'"(?:[^"\\]|\\.)*"', re.S) + +# `let gc_flags = ...` is a local read, not a header write. +LET_BINDING = re.compile(r"\blet\s+(?:mut\s+)?gc_flags\w*\s*[:=]") + +# Sites that are allowed to originate a pin outside `pin_object`. +# (relative path, line substring, why) +# An entry that matches nothing is a failure — delete it when the site goes. +ALLOWLIST: list[tuple[str, str, str]] = [ + ( + "crates/perry-runtime/src/gc/malloc.rs", + "pinned = state.push_test_object(64, GC_FLAG_PINNED)", + "MallocState::push_test_object seeds a synthetic malloc-space header " + "for the drop_tests. Malloc space is never Eden/FromSurvivor " + "(CopyingPointerSet::classify_arena), so a pin there can never " + "constrain the copying minor and need not arm the latch.", + ), + ( + "crates/perry-runtime/src/gc/tests/copying/latch.rs", + "(*sabotage_plant_7645).gc_flags |= GC_FLAG_PINNED", + "the latch sabotage test deliberately plants a young pin WITHOUT " + "arming the latch, to prove the dynamic move_young guard catches an " + "incomplete latch. It must stay a raw write or it tests nothing.", + ), +] + +# Floor: the tree currently carries ~80 `GC_FLAG_PINNED` tokens. A regex that +# silently stopped matching would report zero offenders and pass. +MIN_TOKENS = 40 + + +def strip_comments(text: str) -> str: + out = [] + for line in text.splitlines(): + out.append(line.split("//", 1)[0]) + return "\n".join(out) + + +def strip_strings(text: str) -> str: + """Blank out string literals, keeping the line count. + + A `GC_FLAG_PINNED` inside a diagnostic message (the `move_young` abort + reporter says the name out loud) is prose, not a pin. + """ + return STRING_LITERAL.sub(lambda m: '"' + ("\n" * m.group(0).count("\n")) + '"', text) + + +def statements(text: str): + """Yield (statement_text, line_number_of_statement_start).""" + line = 1 + start = 0 + for index, char in enumerate(text): + if char == "\n": + line += 1 + if char == ";" or char == "{" or char == "}": + chunk = text[start:index] + if chunk.strip(): + yield chunk, line - chunk.count("\n") + start = index + 1 + tail = text[start:] + if tail.strip(): + yield tail, line - tail.count("\n") + + +def named_offenders(rel: str, text: str) -> tuple[list[tuple[str, int, str]], int]: + """Rule A. Returns (offenders, tokens seen).""" + offenders: list[tuple[str, int, str]] = [] + tokens = 0 + code = strip_strings(strip_comments(text)) + for chunk, lineno in statements(code): + if TOKEN not in chunk: + continue + flat = " ".join(chunk.split()) + if flat.startswith("use ") or re.search(r"\bconst\s+GC_FLAG_PINNED\b", flat): + tokens += flat.count(TOKEN) + continue + # `&=` clears; normalise it to a plain mask so `x &= !PINNED` reads as + # a mask rather than as an assignment. + probe = flat.replace("&=", "& ") + for match in re.finditer(re.escape(TOKEN), probe): + tokens += 1 + before = probe[: match.start()] + last_and = before.rfind("&") + last_assign = max(before.rfind("|="), before.rfind("=")) + if last_and > last_assign: + continue # masking position: a read or a clear + offenders.append((rel, lineno, flat[:200])) + return offenders, tokens + + +def raw_byte_offenders(rel: str, text: str) -> list[tuple[str, int, str]]: + """Rule B.""" + offenders: list[tuple[str, int, str]] = [] + for lineno, line in enumerate(strip_comments(text).splitlines(), start=1): + if "gc_flags" not in line or LET_BINDING.search(line): + continue + for match in FLAG_WRITE.finditer(line): + rhs = STRING_LITERAL.sub("", match.group("rhs")) + if TOKEN in rhs: + continue # rule A owns the named form + for literal in INT_LITERAL.finditer(rhs): + raw = literal.group("hex") + value = ( + int(raw.replace("_", ""), 16) + if raw + else int(literal.group("dec").replace("_", "")) + ) + if value & PINNED_BIT: + offenders.append((rel, lineno, line.strip()[:200])) + break + return offenders + + +def scan(root: Path) -> tuple[list[tuple[str, int, str]], int]: + found: list[tuple[str, int, str]] = [] + tokens = 0 + crates = root / "crates" + for path in sorted(crates.rglob("*.rs")): + if "/target/" in str(path): + continue + rel = str(path.relative_to(root)) + try: + text = path.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + # `pin.rs` IS the sanctioned setter; rule A does not apply inside it. + if not rel.endswith("gc/pin.rs"): + named, seen = named_offenders(rel, text) + found.extend(named) + tokens += seen + else: + tokens += text.count(TOKEN) + found.extend(raw_byte_offenders(rel, text)) + return found, tokens + + +def apply_allowlist( + sites: list[tuple[str, int, str]], +) -> tuple[list[tuple[str, int, str]], list[tuple[str, str, str]]]: + used: set[int] = set() + offenders: list[tuple[str, int, str]] = [] + for rel, lineno, line in sites: + hit = None + for index, (allow_path, needle, _why) in enumerate(ALLOWLIST): + if rel == allow_path and needle in line: + hit = index + break + if hit is None: + offenders.append((rel, lineno, line)) + else: + used.add(hit) + stale = [entry for index, entry in enumerate(ALLOWLIST) if index not in used] + return offenders, stale + + +def report(root: Path, quiet: bool = False) -> int: + sites, tokens = scan(root) + if tokens < MIN_TOKENS: + print( + f"gc_pin_sites: found only {tokens} {TOKEN} tokens, expected at least " + f"{MIN_TOKENS}. The scan is broken — a green run here would be vacuous.", + file=sys.stderr, + ) + return 2 + offenders, stale = apply_allowlist(sites) + status = 0 + if offenders: + status = 1 + print( + "GC_FLAG_PINNED custody violation: these sites originate a pin without\n" + "going through gc::pin_object, so they pin an object WITHOUT arming the\n" + "young-pin latch. The copying minor skips its pin preflight on that latch\n" + "(#7645) and will relocate the object out from under whoever holds it.\n" + "Route the site through `gc::pin_object(header)` — or, across an FFI\n" + "boundary, `js_gc_pin_user_ptr(user_ptr)`.\n", + file=sys.stderr, + ) + for rel, lineno, line in offenders: + print(f" {rel}:{lineno}: {line}", file=sys.stderr) + if stale: + status = 1 + print( + "\ngc_pin_sites: these ALLOWLIST entries no longer match any pin site.\n" + "Delete them — a stale exemption is how this gate stops being one.\n", + file=sys.stderr, + ) + for allow_path, needle, why in stale: + print(f" {allow_path} | {needle} | {why}", file=sys.stderr) + if status == 0 and not quiet: + print( + f"gc_pin_sites: OK — every pin originates in gc::pin_object " + f"({len(sites)} allowlisted exception(s), {tokens} {TOKEN} tokens scanned)." + ) + return status + + +OFFENDER_PLANTS = { + "named or-into-flags": " (*h).gc_flags |= crate::gc::GC_FLAG_PINNED;\n", + "named assign-into-flags": " (*h).gc_flags = GC_FLAG_MARKED | GC_FLAG_PINNED;\n", + "named passed as a seed argument": " let x = alloc_with(64, GC_FLAG_PINNED);\n", + "raw-byte hex": " *gc_flags_ptr |= 0x04;\n", + "raw-byte combined hex": " (*h).gc_flags = 0x06;\n", + "raw-byte decimal": " (*h).gc_flags |= 4;\n", +} + +BENIGN_PLANT = """unsafe fn f(h: *mut u8) { + use crate::gc::GC_FLAG_PINNED; + if (*h).gc_flags & (GC_FLAG_MARKED | GC_FLAG_PINNED) != 0 { return; } + let preserved = flags & (GC_FLAG_SHAPE_SHARED | GC_FLAG_INTERNED | GC_FLAG_PINNED); + (*h).gc_flags &= !GC_FLAG_PINNED; + (*h).gc_flags = flags & !GC_FLAG_PINNED; + (*h).gc_flags |= GC_FLAG_MARKED; + (*h).gc_flags |= 0x01; + let gc_flags_addr = blk.sub(I64, &handle, "7"); + let gc_flags = blk.load(I8, &gc_flags_ptr); +} +""" + + +def _scan_source(body: str) -> list[tuple[str, int, str]]: + with tempfile.TemporaryDirectory() as tmp: + fake = Path(tmp) / "crates" / "fake" / "src" + fake.mkdir(parents=True) + (fake / "lib.rs").write_text(body) + sites, _tokens = scan(Path(tmp)) + offenders, _stale = apply_allowlist(sites) + return offenders + + +def self_test() -> int: + failures: list[str] = [] + for name, plant in OFFENDER_PLANTS.items(): + if not _scan_source("unsafe fn f(h: *mut u8) {\n" + plant + "}\n"): + failures.append(f"scanner MISSED the {name} offender: {plant.strip()}") + benign = _scan_source(BENIGN_PLANT) + if benign: + failures.append(f"scanner FALSE-POSITIVED on read/clear/preserve shapes: {benign}") + missing = [ + entry + for entry in ALLOWLIST + if not (REPO_ROOT / entry[0]).exists() + or entry[1] not in (REPO_ROOT / entry[0]).read_text(encoding="utf-8", errors="replace") + ] + if missing: + failures.append(f"ALLOWLIST entries whose file/needle does not exist: {missing}") + if failures: + for line in failures: + print(f"gc_pin_sites --self-test FAILED: {line}", file=sys.stderr) + return 1 + print( + f"gc_pin_sites --self-test: OK ({len(OFFENDER_PLANTS)} offender shapes caught, " + "no false positives on read/clear/preserve)" + ) + return 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--self-test", + action="store_true", + help="prove the scanner can fail before trusting a green run", + ) + parser.add_argument("--quiet", action="store_true") + parser.add_argument("--root", default=str(REPO_ROOT)) + args = parser.parse_args() + if args.self_test: + return self_test() + return report(Path(args.root), quiet=args.quiet) + + +if __name__ == "__main__": + os.chdir(REPO_ROOT) + sys.exit(main()) From 6838dcb2f90de5fb9a4137f021062a7a6593faba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 17:35:12 +0200 Subject: [PATCH 4/7] docs: changelog fragment for #7645 --- .../7650-copying-minor-preflight-skip.md | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 changelog.d/7650-copying-minor-preflight-skip.md diff --git a/changelog.d/7650-copying-minor-preflight-skip.md b/changelog.d/7650-copying-minor-preflight-skip.md new file mode 100644 index 0000000000..c48a5c72b2 --- /dev/null +++ b/changelog.d/7650-copying-minor-preflight-skip.md @@ -0,0 +1,53 @@ +**`perf(gc)`: the copying minor stopped traversing the young graph twice — the eligibility preflight is skipped when its answer is already known (#7645).** + +`CopiedMinorEligibility::evaluate` walked the whole live young object graph before every copying minor, and the collector then walked it again to copy. The first walk produced no collection result. It answered two booleans: + +1. is any transitively reachable `Eden`/`FromSurvivor` object `GC_FLAG_PINNED`? (`CopyingNurseryPreflight::check_ptr_with_reason`) +2. was a non-arena candidate met while the malloc registry was unavailable **and** non-empty at cycle start? (`CopyingPointerSet::classify_for_preflight`) + +(2) is already O(1) — `malloc_registry_available || malloc_registry_empty_at_start`. (1) is O(live young graph) only because it *searches* for a fact that can be *recorded when it is created*. `gc::pin_object` is now the single sanctioned setter of `GC_FLAG_PINNED` and arms a process-wide **monotone** latch when — and only when — the pinned object sits in a space the copying minor relocates. With the latch clear and (2) decided, both walks provably return `None` and are skipped. Note the direction: "no young pinned object exists at all" is *stronger* than the walk's "none is reachable", so the substitution is conservative rather than merely equal. When either proof is unavailable the walks run exactly as before, so the decision is never changed — only skipped when its outcome is already determined. + +Measured on the pinned quiet mini, both arms built there, interleaved, 6 rounds, `PERRY_NO_AUTO_OPTIMIZE=1` with a pinned `PERRY_RUNTIME_DIR`, output SHA-256 identical on every row: + +| `json_pipeline` | 200k | 500k | +|---|--:|--:| +| `build_out` phase | 622 → **489 ms (−21.4%)** | 1,659 → **1,245 ms (−25.0%)** | +| total wall | 1,004 → **866 ms (−13.7%)** | 2,606 → **2,190 ms (−15.9%)** | +| `parse` / `serialize` | flat (−0.4% / −1.4%) | flat (−0.2% / +0.9%) | + +Spreads do not overlap in any moved cell (500k `build_out`: base 1,651–1,672, arm 1,238–1,250). + +**The subject was live and the decision unchanged.** `PERRY_GC_DIAG` on the arm reports `eligible=true fallback=none preflight_skipped=true (skips=1 walks=0)`, and `promoted_objects=4,117,015` / `promoted_bytes=280,996,840` / `freed_bytes=17,544` are byte-identical to the base arm. A field-by-field `PERRY_GC_TRACE` diff of all three cycles — 1,868 non-timing fields — shows **8 differences, all telemetry of the removed traversal**: `layout_scans.pointer_slots_read` 22,041,102 → 13,827,564, `unknown_layout_slots_read` 16,500,006 → 10,500,003, `masked_pointer_slots_read` 3,014,775 → 1,810,014, `pointer_slot_bytes_read` 176,328,816 → 110,620,512, the three `pointer_free_*_skipped` counters halved, and `old_pages.dirty_slots` 1,017,546 → 508,773. Every counter describing the *collection* — cycle count, kinds, triggers, `copied_*`, `promoted_*`, `freed_bytes`, `remembered_set`, `root_sources`, `sweep` — is identical. + +### The issue's pin-site analysis was incomplete, and that is the load-bearing finding + +#7645 named three production `GC_FLAG_PINNED` setters and argued all three were harmless (malloc-space or `Longlived`). There are **six**, and three of them pin **Eden** objects: + +- `perry-stdlib`'s `async_bridge::pin_promise_for_native_resolution` pins a `js_promise_new()` promise — an `arena_alloc_gc`, i.e. Eden, whenever promise hooks are off. Every `fetch`/`zlib`/`ws`/`bcrypt`/`ioredis` request goes through it. +- `perry-ui-macos`'s `textfield::get_string_value` and `table::get_filter_text` pin the `js_string_from_bytes` result they hand back to JS — also Eden. + +The two AppKit sites wrote `*gc_flags_ptr |= 0x04;` against a hand-computed `ptr - 8 + 1`, so **they are invisible to `grep GC_FLAG_PINNED`** — which is how an enumeration done by grep came back short by half. That is the argument for the gate being a scanner rather than a list. It does not sink the approach (the latch handles those sites; they arm it), but it changes the honest claim about who benefits: `perry-stdlib`-async and AppKit programs keep today's behaviour, compute- and JSON-shaped programs get the walk removed. + +### Three enforcement layers, because a wrong latch is a use-after-move + +`move_young` relocates a pinned object exactly as it would any other — it only *preserves* the bit — and the cross-thread promise queue holds a raw `usize` no scanner rewrites. + +1. **Static, in `lint`.** `scripts/gc_pin_sites.py` fails on any site that originates a pin outside `pin_object`, matching the named form *and* any write into a `gc_flags`-named identifier whose right-hand side carries an integer literal with bit 2 set. It fails equally on a **stale allowlist entry** (the `deferred_registration_flush_sites` model in `arena/tests.rs`), and refuses to report green having seen fewer than 40 `GC_FLAG_PINNED` tokens. `--self-test` plants six offender shapes, requires each to be caught, and requires the read/clear/preserve shapes not to be. The two flag-byte channels it deliberately does not scan — allocator birth flags (`GC_BIRTH_EXTRA_FLAGS` is only ever `0` or `GC_FLAG_MARKED`) and codegen's inline bump allocators (`GC_FLAG_ARENA` plus that same byte) — are documented in the script with why neither can originate a pin. +2. **Dynamic, at the instant it would matter.** `move_young` already holds the flags byte in a register; on a *preflight-skipped* cycle it tests bit 2 and aborts with `[gc-pin-latch] FATAL`, naming the header, rather than relocating it. One `and` and a never-taken branch. Deliberately *not* applied when the preflight ran: that path is unchanged here, and a divergence between the preflight's traversal and the copier's would be a separate bug that should not newly abort a program. +3. **Tests.** Every pinned-fallback test plants its pin through `pin_object`, so none can pass on an unsound configuration. `gc/tests/copying/latch.rs` adds the skip/liveness case, the `Longlived`- and malloc-pin cases that prove the `SMALL_INT_CACHE` and `spawn`'s cross-thread promise stay free, the monotonicity case, and a subprocess sabotage test that plants a raw young pin and requires the collector to die on `SIGABRT` with that message. + +**Sabotage-verified.** Deleting the one `YOUNG_PIN_EVER.store(true, …)` line and running each protection test alone: `young_pin_via_pin_object_restores_the_walk` and all three `test_copying_minor_falls_back_for_pinned_young_*` cases die with `SIGABRT` from layer 2; `the_latch_is_monotone_across_an_unpin` fails its assertion. The control case (`no_pin_ever_means_the_preflight_walks_are_skipped`) still passes, so the sabotage broke the protection and not the harness. + +### Why monotone + +A decrementing counter would recover the fast path after a transient pin (a settled `fetch` promise). It was rejected because it adds a *second* completeness obligation of the same severity — every unpin site, where a spurious or double decrement is silently unsound in exactly the same use-after-move way. Monotone needs one proof. The cost is stated and asserted by `the_latch_is_monotone_across_an_unpin`: a process that ever pins young pays the walk for the rest of its life. + +### One ordering hazard found and closed + +`dirty_slot_preflight_reason` took a `remembered_dirty_snapshot()`, whose **first** call on a thread arms the barrier and rebuilds the remembered set from the heap — a walk whose own comment says "nothing is marked yet when a collector first asks for the log". In a successful copying minor that first call was always the preflight's. Letting it fall through to the copy phase's snapshot would have run it after `visit_mutable_root_slots` had already evacuated root-reachable young objects, i.e. against a half-moved heap. `arm_and_reconstruct_remembered_set_if_unarmed()` is therefore called explicitly on the skip path, keeping it where it was. It is one-shot per thread, so every later cycle pays a thread-local flag read. + +### Counters that move, deliberately + +Skipping a traversal removes its telemetry, and only that: the eight fields above. `test_copying_minor_rewrites_exact_{object,closure}_pointer_*` now expect `masked_pointer_slots_read == 1` instead of `2` — one read by the copier where there used to be one by each walk — so the drop has a unit-scale witness that fails if the walk ever returns. + +New: `trace.copying_nursery.preflight_skipped`, `gc::copied_minor_preflight_skips()` / `copied_minor_preflight_walks()`, and a `PERRY_GC_DIAG` line, so a verdict about this change can assert its subject was live (#7024/#7025) instead of passing on a cycle that never skipped anything. From f9c5472ae2f11c5a4f373cb91be1f54270cc1e6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 18:08:32 +0200 Subject: [PATCH 5/7] chore(gc-ratchet): re-pin 01_nursery_churn.heap_used_bytes for #7645 6,277,048 -> 7,325,584. One cell, one probe; tolerances.json untouched. WHY RE-PIN RATHER THAN EXEMPT. The delta is exactly one 1 MiB nursery block (+1,048,536 B) and it is DETERMINISTIC -- spread 0 over 7 samples on both arms. A reproducible shift can still carry a band, so the cell stays gated at a new value. 12_large_live_set's probe_overrides exemption is the wrong instrument here: its rationale rests on genuine sample-dependence ("cannot carry a band whose premise is bit-identity"), which is the opposite of this case, and `gating` is one-way -- spending it on a deterministic cell would give up the gate permanently to avoid a re-pin. IT IS NOT RETENTION. `gc_ratchet.py classify` reports heap_used_precise_bytes = 5,228,512 on BOTH arms, and byte-identical on all 12 probes. The entire movement is false_root_excess: 1,048,536 -> 2,097,072. Every other gated cell on this probe -- minor_cycles, step_cycles, copied_objects, copied_bytes, promoted_objects, promoted_bytes, freed_bytes, heap_total_bytes -- is bit-identical across the two arms. CAUSE IS #7558. The probe's own explicit gc() forces a conservative stack scan ("[gc-scan-fallback] site=manual_collect automatic=false"). #7645 removes the eligibility preflight's drain/scan_object_fields/check_ptr_with_reason recursion, whose frames used to overwrite stale pointer-shaped words deep on the native stack; one surviving stale word pins a whole 1 MiB block. PROVENANCE, CHECKED NOT ASSUMED. Measured on perry-macos (Mac mini M1, 8 GB, macOS 26.5.1) -- the same host recorded in this artifact's `host` block, so this is not a cross-host rebase of one row. Before the edit, `check --profile shared_ci` was GREEN for current main (c8394bfdb) against this artifact, so the 143 untouched cells are still in band and this is one row moving rather than a regeneration. The artifact's top-level `commit` still reads 26b9c9d59 and now describes 143 of 144 cells; the notes field records that explicitly. VERIFIED. validate --scope all: structurally valid. check vs the PR arm: OK. check vs main: OK. And the cell still GATES -- planting one further 1 MiB block (8,374,160) makes check exit 1 and name it, so this is a live gate at a new value, not a silently widened one. --- .../gc_ratchet/baseline/gc-ratchet-v1.json | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json b/benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json index 9b4f7ff87f..123ab80a0d 100644 --- a/benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json +++ b/benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json @@ -299,7 +299,7 @@ } } }, - "notes": "Regenerated at main 26b9c9d59 (0.5.1346) on the pinned quiet host perry-macos (Mac mini M1, 8 GB, macOS 26.5.1) -- the SAME host and toolchain (rustc/cargo 1.97.1, Apple clang 21.0.0) as the 2026-08-05 pin at 5e236e6e2 (0.5.1280), so this is a like-for-like re-pin, not a host change. Load 2.38/1.9/1.7 at capture (the previous pin was taken at 2.01/3.2/7.57, i.e. on a busier box). All 12 probes oracle-pass; heap_used_bytes spread 0 on eleven probes and 864 B on 12_large_live_set.\n\nWHY: gc-ratchet had not been green on main since 2026-08-01T05:39Z -- 179 consecutive red main runs. From 2026-08-05 to #7557 the job could not even reach its probes (artifact validation ran before the measurement step, #7554); after #7557 restored measurement it stayed red against this stale 0.5.1280 artifact. A permanently red, non-required gate is read by nobody, and #7594/#7596 both had to substitute hand-run A/Bs for it.\n\nCELLS THAT MOVED, WITH ATTRIBUTION:\n\n(a) EXPLAINED -- collection pacing. 03_cross_gen_writes copied_objects 13,893->8,212 (-40.9%), copied_bytes 990,736->590,688 (-40.4%), promoted_objects 4,752->0, promoted_bytes 210,736->0; 04_dead_after_deep_stack copied_objects 11,268->565 (-95.0%), copied_bytes 663,512->44,688 (-93.3%), promoted_objects 4,752->10, promoted_bytes 210,744->440. This is the intended effect of #7594 (survivor-promotion handoff livelock) and #7596 (live-proportional collection budgets at both generations): less futile promotion, less copy work for the same allocation sequence. CAVEAT RECORDED DELIBERATELY: 03_cross_gen_writes.promoted_objects/promoted_bytes now pin at 0, where the allowance floor (16 objects / 64 KiB) covers the whole range and the liveness assertion in evaluate() fires only when the BASELINE median is > 0. That cell therefore no longer carries signal in either direction. It is not hidden here; it is the price of pinning a counter at zero.\n\n(b) EXPLAINED -- measurement, not retention. 02_survivor_promotion.heap_used_bytes 9,418,232->9,678,792 (+2.77%) and 05_closure_capture.heap_used_bytes 6,378,392->7,426,960 (+16.44%) are conservative-stack-scan false-root residue. gc_ratchet.py classify on this host at this commit: 05 precise 5,329,880 -- byte-identical to the figure #7571 measured at BOTH ends of its 74-commit window -- against conservative 7,426,960, i.e. the residue went 1 block -> 2 blocks (2,097,080 B) while real retention did not move at all; 02 precise 9,416,632, which is BELOW the 9,418,232 this artifact previously recorded as that probe's retention, so real retention cannot have grown. Neither is a collector regression (#7558 for the mechanism, #7571 for the instrument). This is the #7559 answer, reproduced independently rather than assumed.\n\n(c) FLAGGED -- NOT explained by any merged, documented decision. 12_large_live_set.wall_ms 3,056 -> 3,471 ms (+13.58%). Two non-overlapping 7-sample clusters (3,047-3,061 vs 3,466-3,476), same host, same toolchain, same protocol, while 06_string_retention and 11_collect_at_depth got 9.6% and 28.4% FASTER over the same window. #7596 reported -7.4% on this very cell in its own both-arms A/B, so by that PR's own evidence this is not #7596. Gated under pinned_host only (shared_ci does not gate wall time), so it does not block CI -- but it is a real, reproducible slowdown on the largest probe and is being pinned here only so the rest of the matrix can gate again. Tracked on #7554; it wants a bisect over 0.5.1280..0.5.1346.\n\n(d) DID NOT REPRODUCE. #7596's merge audit accepted 12_large_live_set.heap_total_bytes +36% (95.4 -> 130.0 MB) as a deliberate GOGC trade and deferred the re-pin to this repair. Under the harness protocol on this host that cell is 110,100,480 -> 110,100,480, +0.00%. The accepted delta is therefore NOT folded in, because there is nothing to fold in: neither endpoint of #7596's figure matches this artifact's reading of that cell. Nothing was re-pinned on account of that decision.\n\nPROVENANCE CAVEATS: no benchmark suite is recorded (benchmarks/compare.sh needs a full checkout; this host measured shipped binaries). The perry binary fingerprinted here had install_name_tool applied to repoint libz3.4.15.dylib into ~/ratchet-7554/lib, because the host carries z3 4.16; that dylib is loaded by the compiler driver only and cannot reach probe behaviour, and libperry_runtime.a / libperry_stdlib.a are byte-identical to the cargo release output. The measured collector is exactly origin/main 26b9c9d5965190031562be0db0ca7d78b8a683d0 -- the branch this was pinned from changes only gc_ratchet.py, tests/test_gc_ratchet.py and the workflow, with no Rust delta.", + "notes": "Regenerated at main 26b9c9d59 (0.5.1346) on the pinned quiet host perry-macos (Mac mini M1, 8 GB, macOS 26.5.1) -- the SAME host and toolchain (rustc/cargo 1.97.1, Apple clang 21.0.0) as the 2026-08-05 pin at 5e236e6e2 (0.5.1280), so this is a like-for-like re-pin, not a host change. Load 2.38/1.9/1.7 at capture (the previous pin was taken at 2.01/3.2/7.57, i.e. on a busier box). All 12 probes oracle-pass; heap_used_bytes spread 0 on eleven probes and 864 B on 12_large_live_set.\n\nWHY: gc-ratchet had not been green on main since 2026-08-01T05:39Z -- 179 consecutive red main runs. From 2026-08-05 to #7557 the job could not even reach its probes (artifact validation ran before the measurement step, #7554); after #7557 restored measurement it stayed red against this stale 0.5.1280 artifact. A permanently red, non-required gate is read by nobody, and #7594/#7596 both had to substitute hand-run A/Bs for it.\n\nCELLS THAT MOVED, WITH ATTRIBUTION:\n\n(a) EXPLAINED -- collection pacing. 03_cross_gen_writes copied_objects 13,893->8,212 (-40.9%), copied_bytes 990,736->590,688 (-40.4%), promoted_objects 4,752->0, promoted_bytes 210,736->0; 04_dead_after_deep_stack copied_objects 11,268->565 (-95.0%), copied_bytes 663,512->44,688 (-93.3%), promoted_objects 4,752->10, promoted_bytes 210,744->440. This is the intended effect of #7594 (survivor-promotion handoff livelock) and #7596 (live-proportional collection budgets at both generations): less futile promotion, less copy work for the same allocation sequence. CAVEAT RECORDED DELIBERATELY: 03_cross_gen_writes.promoted_objects/promoted_bytes now pin at 0, where the allowance floor (16 objects / 64 KiB) covers the whole range and the liveness assertion in evaluate() fires only when the BASELINE median is > 0. That cell therefore no longer carries signal in either direction. It is not hidden here; it is the price of pinning a counter at zero.\n\n(b) EXPLAINED -- measurement, not retention. 02_survivor_promotion.heap_used_bytes 9,418,232->9,678,792 (+2.77%) and 05_closure_capture.heap_used_bytes 6,378,392->7,426,960 (+16.44%) are conservative-stack-scan false-root residue. gc_ratchet.py classify on this host at this commit: 05 precise 5,329,880 -- byte-identical to the figure #7571 measured at BOTH ends of its 74-commit window -- against conservative 7,426,960, i.e. the residue went 1 block -> 2 blocks (2,097,080 B) while real retention did not move at all; 02 precise 9,416,632, which is BELOW the 9,418,232 this artifact previously recorded as that probe's retention, so real retention cannot have grown. Neither is a collector regression (#7558 for the mechanism, #7571 for the instrument). This is the #7559 answer, reproduced independently rather than assumed.\n\n(c) FLAGGED -- NOT explained by any merged, documented decision. 12_large_live_set.wall_ms 3,056 -> 3,471 ms (+13.58%). Two non-overlapping 7-sample clusters (3,047-3,061 vs 3,466-3,476), same host, same toolchain, same protocol, while 06_string_retention and 11_collect_at_depth got 9.6% and 28.4% FASTER over the same window. #7596 reported -7.4% on this very cell in its own both-arms A/B, so by that PR's own evidence this is not #7596. Gated under pinned_host only (shared_ci does not gate wall time), so it does not block CI -- but it is a real, reproducible slowdown on the largest probe and is being pinned here only so the rest of the matrix can gate again. Tracked on #7554; it wants a bisect over 0.5.1280..0.5.1346.\n\n(d) DID NOT REPRODUCE. #7596's merge audit accepted 12_large_live_set.heap_total_bytes +36% (95.4 -> 130.0 MB) as a deliberate GOGC trade and deferred the re-pin to this repair. Under the harness protocol on this host that cell is 110,100,480 -> 110,100,480, +0.00%. The accepted delta is therefore NOT folded in, because there is nothing to fold in: neither endpoint of #7596's figure matches this artifact's reading of that cell. Nothing was re-pinned on account of that decision.\n\nPROVENANCE CAVEATS: no benchmark suite is recorded (benchmarks/compare.sh needs a full checkout; this host measured shipped binaries). The perry binary fingerprinted here had install_name_tool applied to repoint libz3.4.15.dylib into ~/ratchet-7554/lib, because the host carries z3 4.16; that dylib is loaded by the compiler driver only and cannot reach probe behaviour, and libperry_runtime.a / libperry_stdlib.a are byte-identical to the cargo release output. The measured collector is exactly origin/main 26b9c9d5965190031562be0db0ca7d78b8a683d0 -- the branch this was pinned from changes only gc_ratchet.py, tests/test_gc_ratchet.py and the workflow, with no Rust delta.\n\nSURGICAL RE-PIN 2026-08-08 -- 01_nursery_churn.heap_used_bytes ONLY, 6,277,048 -> 7,325,584, for #7645 (PR #7650, the copying minor's eligibility preflight). SAME host as the rest of this artifact (perry-macos, Mac mini M1, 8 GB, macOS 26.5.1) -- verified against the `host` block above, not assumed. Every OTHER cell is left exactly as pinned at 26b9c9d59; `check` was green for current main (c8394bfdb) against this artifact before the edit, so the untouched cells are still in band and this is one row moving, not a regeneration. The artifact's top-level `commit` therefore still reads 26b9c9d59 and now describes 143 of 144 cells; this one is from c8394bfdb+#7650.\n\nATTRIBUTION: the delta is exactly one 1 MiB nursery block (+1,048,536 B) and is DETERMINISTIC -- spread 0 over 7 samples on both arms, so it can still carry a band and stays gated (this is why it is re-pinned rather than given 12_large_live_set's probe_overrides exemption, whose premise is genuine sample-dependence). It is NOT retention: `gc_ratchet.py classify` reports heap_used_precise_bytes = 5,228,512 on BOTH arms, and byte-identical on all 12 probes; the whole movement is false_root_excess (1,048,536 -> 2,097,072). Cause is #7558 -- the probe's own explicit gc() forces a conservative stack scan, and #7650 removes the preflight's drain/scan_object_fields recursion, whose frames used to overwrite stale pointer-shaped words deep on the native stack. One surviving stale word pins a whole 1 MiB block. Every other gated cell on this probe -- minor_cycles, step_cycles, copied_objects, copied_bytes, promoted_objects, promoted_bytes, freed_bytes, heap_total_bytes -- is BIT-IDENTICAL across the two arms.", "probes": { "01_nursery_churn": { "stdout": "probe:01_nursery_churn\nchecksum:-1399701504\n", @@ -311,18 +311,18 @@ "metrics": { "heap_used_bytes": { "samples": [ - 6277048, - 6277048, - 6277048, - 6277048, - 6277048, - 6277048, - 6277048 + 7325584, + 7325584, + 7325584, + 7325584, + 7325584, + 7325584, + 7325584 ], "sample_count": 7, - "median": 6277048, - "min": 6277048, - "max": 6277048, + "median": 7325584, + "min": 7325584, + "max": 7325584, "stdev": 0, "spread": 0, "spread_pct": 0 From 592b42e51878db6f18a592e692ad1cf248330f67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 18:09:49 +0200 Subject: [PATCH 6/7] docs: record the gc-ratchet re-pin in the changelog fragment --- changelog.d/7650-copying-minor-preflight-skip.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changelog.d/7650-copying-minor-preflight-skip.md b/changelog.d/7650-copying-minor-preflight-skip.md index c48a5c72b2..4b71a10e60 100644 --- a/changelog.d/7650-copying-minor-preflight-skip.md +++ b/changelog.d/7650-copying-minor-preflight-skip.md @@ -46,6 +46,12 @@ A decrementing counter would recover the fast path after a transient pin (a sett `dirty_slot_preflight_reason` took a `remembered_dirty_snapshot()`, whose **first** call on a thread arms the barrier and rebuilds the remembered set from the heap — a walk whose own comment says "nothing is marked yet when a collector first asks for the log". In a successful copying minor that first call was always the preflight's. Letting it fall through to the copy phase's snapshot would have run it after `visit_mutable_root_slots` had already evacuated root-reachable young objects, i.e. against a half-moved heap. `arm_and_reconstruct_remembered_set_if_unarmed()` is therefore called explicitly on the skip path, keeping it where it was. It is one-shot per thread, so every later cycle pays a thread-local flag read. +### gc-ratchet: one cell re-pinned + +`01_nursery_churn.heap_used_bytes` moves 6,277,048 -> 7,325,584 and is re-pinned in `benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json` (one cell; `tolerances.json` untouched). It is **not** retention: `gc_ratchet.py classify` reports `heap_used_precise_bytes` byte-identical on all 12 probes and on both arms (5,228,512 here), and the whole delta is `false_root_excess` — exactly one 1 MiB nursery block. Cause is #7558: the probe's own `gc()` forces a conservative stack scan, and removing the preflight's recursion changes which stale pointer-shaped words survive deep on the native stack, where one of them pins a whole block. + +It is re-pinned rather than given `12_large_live_set`'s `probe_overrides` exemption because the delta is **deterministic** — spread 0 over 7 samples on both arms — and a reproducible shift can still carry a band, whereas that exemption's premise is genuine sample-dependence and `gating` is one-way. Same host as the rest of the artifact (checked against its `host` block), and `check` was green for current main against the unedited artifact, so the other 143 cells are still in band. The re-pinned cell still fails on a further 1 MiB block, so it remains a live gate. + ### Counters that move, deliberately Skipping a traversal removes its telemetry, and only that: the eight fields above. `test_copying_minor_rewrites_exact_{object,closure}_pointer_*` now expect `masked_pointer_slots_read == 1` instead of `2` — one read by the copier where there used to be one by each walk — so the drop has a unit-scale witness that fails if the walk ever returns. From 0c32d3c181d31226ab80809c60edf513aaf2a69a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 18:12:58 +0200 Subject: [PATCH 7/7] chore: bump version to 0.5.1370 Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --- CLAUDE.md | 2 +- Cargo.lock | 152 ++++++++++++++++++++++++++--------------------------- Cargo.toml | 2 +- 3 files changed, 78 insertions(+), 78 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e1ad9eb1f7..79eb2673f3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1369 +**Current Version:** 0.5.1370 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index d9146a9372..ec5cc72a43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1369" +version = "0.5.1370" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1369" +version = "0.5.1370" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1369" +version = "0.5.1370" [[package]] name = "perry-ui-tvos" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1369" +version = "0.5.1370" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 1a3ffcefa4..1f26d934ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1369" +version = "0.5.1370" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry"