From 01c785d46f8ea42894a670528cd67f2181cd0e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 15:52:24 +0200 Subject: [PATCH 1/2] fix(gc): resolve the memoized Array.prototype address across relocation (#6981) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `array::indexing` memoizes `Array.prototype`'s (and `Object.prototype`'s) heap address in a process-global `AtomicUsize`. That is a raw pointer to a MOVABLE object, and nothing maintained it. Two independent relocations leave the cache naming a `GC_FLAG_FORWARDED` stub: 1. `js_array_grow` — an indexed write past the dense capacity (`Array.prototype[300] = v`) reallocates the backing store and forwards the old head. No GC is involved at all. 2. the copying young-gen minor — it evacuates the prototype and forwards. Every reader of an array pointer resolves it through `clean_arr_ptr`, which follows forwarding chains. The cache did not, and the hole/OOB read fallback's self-recursion guard is the object-identity test `proto != receiver`. After a move those are two different addresses for the SAME object, the guard stops firing, and js_array_get_f64 ⇄ array_oob_prototype_get recurse without bound until the thread's stack guard page: `EXC_BAD_ACCESS` / `KERN_PROTECTION_FAILURE`, "Thread stack size exceeded due to excessive recursion", exit 139. It is a control-flow defect, not memory corruption — every dereference on the way down is to valid, mapped memory, which is why a 64 MB stack does not help and the evacuation verifier never fires. Three defences, matching the two ways the address goes stale: * `array_prototype_addr` / `object_prototype_addr` resolve the forwarding chain and heal the cache in place. This is what covers `js_array_grow`, which the collector never sees. The not-forwarded fast path stays call-free (`try_read_gc_header` is `#[inline(always)]`: two range compares plus one load of a permanently-hot `gc_flags` byte). * `scan_prototype_addr_cache_roots_mut` is registered in `gc_init`, so a relocating cycle REWRITES the slot like every other address-holding side table. Healing alone is not sufficient: once the from-space stub is swept and its block recycled the forwarded bit is gone. * `array_oob_prototype_get` resolves the receiver before the identity compare, so the guard is exact by construction rather than contingent on cache freshness. Regression coverage, sabotage-verified in both directions: * `crates/perry/tests/gc_array_prototype_hole_read_6981.rs` — two programs (grow-forwarding with no GC at all, and relocation under a heap budget) x 5 collector arms, byte-exact vs node 26.5.0. Against the unfixed runtime the grow program dies at the very first arm, with the collector switched off. * `gc::tests::runtime_roots::prototype_addr_cache` — 5 unit tests, one per defence plus registration and the unset sentinel. Removing the heal reddens 2; removing the registration/scanner body reddens 2. * `test-files/test_gap_array_proto_grow_hole_read.ts` — parity file. `scripts/gc_repsel_matrix.sh --arms all --pressure 8` over 440 cells goes from `PASS=325 UNVER=100 XFAIL=1 FAIL=14` to `PASS=339 UNVER=100 XFAIL=1 FAIL=0`; byte-exact vs node 26.5.0 425/440 -> 439/440. All 14 FAILs were `test_gap_repsel_p4a3_numarray_barriers`, in exactly the 14 arms that run a copying minor. `evac_minor` and `force_verify` go back into `PR_ARMS`, as that script's own comment instructed once #6981 closed. --- crates/perry-runtime/src/array/indexing.rs | 95 ++++++- crates/perry-runtime/src/array/mod.rs | 3 + crates/perry-runtime/src/gc/mod.rs | 6 + .../src/gc/tests/runtime_roots.rs | 1 + .../runtime_roots/prototype_addr_cache.rs | 232 ++++++++++++++++++ crates/perry-runtime/src/value/equality.rs | 2 +- crates/perry-runtime/src/value/mod.rs | 1 + .../gc_array_prototype_hole_read_6981.rs | 213 ++++++++++++++++ scripts/gc_repsel_matrix.sh | 27 +- .../test_gap_array_proto_grow_hole_read.ts | 32 +++ 10 files changed, 596 insertions(+), 16 deletions(-) create mode 100644 crates/perry-runtime/src/gc/tests/runtime_roots/prototype_addr_cache.rs create mode 100644 crates/perry/tests/gc_array_prototype_hole_read_6981.rs create mode 100644 test-files/test_gap_array_proto_grow_hole_read.ts diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 4265d69a93..92618fe1c1 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -48,6 +48,24 @@ fn throw_array_not_extensible_add(index: u32) -> ! { /// real code nobody adds numeric indices to `Array.prototype`, so the hot OOB /// path stays a single relaxed atomic load until the (rare) write flips the /// flag. `usize::MAX` marks the address as not-yet-computed. +/// +/// ***THIS IS A RAW ADDRESS OF A MOVABLE OBJECT*** (#6981). `Array.prototype` +/// relocates two different ways, and BOTH leave this cache pointing at a +/// `GC_FLAG_FORWARDED` stub while every reader resolves its own receiver +/// through `clean_arr_ptr` (which follows forwarding): +/// +/// 1. `js_array_grow` — an indexed write past the dense capacity +/// (`Array.prototype[300] = v`) reallocates and forwards the old head; +/// 2. the copying young-gen minor — it evacuates the prototype and forwards. +/// +/// A stale cache is not merely a wrong value: `array_oob_prototype_get`'s +/// self-recursion guard is `proto != receiver`, and after a move those are two +/// different addresses **for the same object**, so the guard stops firing and +/// `js_array_get_f64` ⇄ `array_oob_prototype_get` recurse until the stack guard +/// page (SIGSEGV, "excessive recursion"). Hence the two defences below: +/// `array_prototype_addr` resolves the forwarding chain and self-heals, and +/// `scan_prototype_addr_cache_roots_mut` lets the collector rewrite the slot so +/// the address stays live even once the from-space stub is recycled. static ARRAY_PROTO_ADDR: AtomicUsize = AtomicUsize::new(usize::MAX); static ARRAY_PROTO_HAS_INDEX: AtomicBool = AtomicBool::new(false); @@ -72,10 +90,74 @@ pub(crate) fn invalidate_array_index_fast_path() { PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.store(1, Ordering::Relaxed); } +/// GC root scanner for the two memoized prototype addresses (#6981). +/// +/// `ARRAY_PROTO_ADDR` / `OBJECT_PROTO_ADDR` hold raw addresses of movable +/// objects, so a relocating cycle must REWRITE them exactly like the other +/// address-holding side tables (`CLASS_PROTOTYPE_OBJECTS`, +/// `TYPED_ARRAY_VIEW_META`, …). Forwarding-chain healing alone is not +/// sufficient: once the from-space stub is swept and its block recycled the +/// `GC_FLAG_FORWARDED` bit is gone, and the cache would then name an unrelated +/// live object. Both intrinsics are reachable from `globalThis`, so the marking +/// half of this visit is redundant; the rewriting half is the point. +pub fn scan_prototype_addr_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { + for cache in [&ARRAY_PROTO_ADDR, &OBJECT_PROTO_ADDR] { + let cached = cache.load(Ordering::Relaxed); + if cached == usize::MAX || cached == 0 { + continue; + } + let mut addr = cached; + if visitor.visit_usize_slot(&mut addr) { + cache.store(addr, Ordering::Relaxed); + } + } +} + +/// Test-only handles on the two memoized prototype addresses, so the #6981 +/// regression tests can install a synthetic forwarded stub without touching the +/// realm's real intrinsics. +#[cfg(test)] +pub(crate) fn test_array_proto_addr_cache() -> &'static AtomicUsize { + &ARRAY_PROTO_ADDR +} + +#[cfg(test)] +pub(crate) fn test_object_proto_addr_cache() -> &'static AtomicUsize { + &OBJECT_PROTO_ADDR +} + +/// Re-read a memoized prototype address through the GC forwarding chain and +/// write the healed address back, so every caller compares (and dereferences) +/// the object's CURRENT location. See the `ARRAY_PROTO_ADDR` doc for why an +/// unresolved cache is a hang, not just a wrong answer (#6981). +/// +/// `note_array_index_write` calls this on every indexed array write until the +/// prototype is polluted, so the not-forwarded case must stay call-free: the +/// `try_read_gc_header` probe is `#[inline(always)]` and reduces to two range +/// compares plus one load of a `gc_flags` byte at a fixed, permanently-hot +/// address. It also classifies the address band before dereferencing, so the +/// not-yet-resolved sentinel (`usize::MAX`) and any non-heap value fall +/// straight through. +#[inline] +fn heal_prototype_addr(cache: &AtomicUsize, cached: usize) -> usize { + let forwarded = unsafe { + crate::value::addr_class::try_read_gc_header(cached) + .is_some_and(|header| header.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0) + }; + if !forwarded { + return cached; + } + let resolved = crate::value::resolve_forwarding(cached); + if resolved != cached { + cache.store(resolved, Ordering::Relaxed); + } + resolved +} + pub(crate) fn object_prototype_addr() -> usize { let cached = OBJECT_PROTO_ADDR.load(Ordering::Relaxed); if cached != usize::MAX { - return cached; + return heal_prototype_addr(&OBJECT_PROTO_ADDR, cached); } let ctor = crate::object::js_get_global_this_builtin_value(b"Object".as_ptr(), 6); let ctor_value = crate::value::JSValue::from_bits(ctor.to_bits()); @@ -149,7 +231,7 @@ pub(crate) fn array_proto_iterator_modified() -> bool { pub(crate) fn array_prototype_addr() -> usize { let cached = ARRAY_PROTO_ADDR.load(Ordering::Relaxed); if cached != usize::MAX { - return cached; + return heal_prototype_addr(&ARRAY_PROTO_ADDR, cached); } let ctor = crate::object::js_get_global_this_builtin_value(b"Array".as_ptr(), 5); let ctor_value = crate::value::JSValue::from_bits(ctor.to_bits()); @@ -190,6 +272,13 @@ pub(crate) fn note_array_index_write(arr: usize) { /// prototype has indexed properties (see `ARRAY_PROTO_HAS_INDEX`). Returns the /// inherited value, or `undefined` if absent. Skipped entirely when the /// receiver IS `Array.prototype` (avoids self-recursion) or the flag is unset. +/// +/// #6981: the `proto != receiver` self-recursion guard is an OBJECT IDENTITY +/// test, so both sides must be forwarding-resolved. `js_array_get_f64` resolves +/// its receiver through `clean_arr_ptr`; the prototype address comes from a +/// memoized cache, so it is healed here too. Comparing a stale address against +/// a resolved one makes the guard silently stop firing and +/// `js_array_get_f64` ⇄ this function recurse without bound. #[inline] unsafe fn array_oob_prototype_get(receiver: usize, index: u32) -> f64 { const TAG_UNDEFINED_F64: f64 = f64::from_bits(0x7FFC_0000_0000_0001u64); @@ -204,7 +293,7 @@ unsafe fn array_oob_prototype_get(receiver: usize, index: u32) -> f64 { } if ARRAY_PROTO_HAS_INDEX.load(Ordering::Relaxed) { let proto = array_prototype_addr(); - if proto != 0 && proto != receiver { + if proto != 0 && proto != crate::value::resolve_forwarding(receiver) { let proto_arr = proto as *const ArrayHeader; if index < (*proto_arr).length && array_has_own_index(proto_arr, index) { return js_array_get_f64(proto_arr, index); diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index d7e4451ef4..b18c5e8c08 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -93,7 +93,10 @@ pub use self::indexing::{ js_array_numeric_set_f64_unboxed, js_array_set_f64, js_array_set_f64_extend, js_array_set_f64_extend_strict, js_array_set_f64_unchecked, js_array_set_index_or_string, js_array_set_index_or_string_strict, js_array_set_string_key, + scan_prototype_addr_cache_roots_mut, }; +#[cfg(test)] +pub(crate) use self::indexing::{test_array_proto_addr_cache, test_object_proto_addr_cache}; pub use self::is_array::js_array_is_array; pub(crate) use self::iter_methods::throw_reduce_of_empty; pub use self::iter_methods::{ diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 6c98904171..e6ebfa8821 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -453,6 +453,12 @@ pub fn gc_init() { gc_register_mutable_root_scanner(crate::regex::scan_last_exec_groups_root_mut); gc_register_mutable_root_scanner(crate::object::scan_exotic_expando_roots_mut); gc_register_mutable_root_scanner(crate::array::scan_template_raw_roots_mut); + // #6981: the memoized `Array.prototype` / `Object.prototype` addresses in + // `array::indexing`. Raw addresses of movable objects — a relocating cycle + // that does not rewrite them leaves the hole/OOB read fallback comparing a + // stale address against a forwarding-resolved receiver, which defeats its + // own self-recursion guard and drives the mutator into unbounded recursion. + gc_register_mutable_root_scanner(crate::array::scan_prototype_addr_cache_roots_mut); gc_register_mutable_root_scanner(crate::map::scan_map_iterator_array_roots_mut); gc_register_mutable_root_scanner(crate::set::scan_set_iterator_array_roots_mut); gc_register_mutable_root_scanner(crate::perf_hooks::scan_perf_entries_roots_mut); diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots.rs b/crates/perry-runtime/src/gc/tests/runtime_roots.rs index bbe852fa37..6ba510feca 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots.rs @@ -2,6 +2,7 @@ use super::super::*; use super::support::*; use std::cell::Cell; mod callback_scanners; +mod prototype_addr_cache; mod string_slice; fn assert_panics_with(expected: &str, f: impl FnOnce()) { diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/prototype_addr_cache.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/prototype_addr_cache.rs new file mode 100644 index 0000000000..237b133688 --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/prototype_addr_cache.rs @@ -0,0 +1,232 @@ +//! #6981 — the memoized `Array.prototype` / `Object.prototype` addresses are +//! raw pointers to MOVABLE objects, so they must survive relocation. +//! +//! `array::indexing` memoizes both intrinsic addresses in process-global +//! `AtomicUsize` caches. `Array.prototype` relocates two ways, and both leave a +//! `GC_FLAG_FORWARDED` stub at the memoized address: +//! +//! * `js_array_grow` — `Array.prototype[300] = v` reallocates the dense +//! backing store and forwards the old head (no GC involved at all); +//! * the copying young-gen minor — it evacuates the prototype and forwards. +//! +//! Every *reader* of an array pointer resolves it through `clean_arr_ptr`, +//! which follows forwarding chains. The cache did not. That mismatch is not a +//! cosmetic staleness: `array_oob_prototype_get`'s self-recursion guard is the +//! object-identity test `proto != receiver`, so once the two sides disagree +//! about the same object the guard stops firing and +//! `js_array_get_f64` ⇄ `array_oob_prototype_get` recurse without bound until +//! the thread's stack guard page (`SIGSEGV`, "excessive recursion"). +//! +//! Two independent defences, one test each: +//! +//! 1. `array_prototype_addr` / `object_prototype_addr` heal the cache through +//! the forwarding chain. This is what covers `js_array_grow`, which the +//! collector never sees. +//! 2. `scan_prototype_addr_cache_roots_mut` is a registered mutable root +//! scanner, so a relocating cycle REWRITES the slot. Healing alone is not +//! enough here: once the from-space stub is swept and its block recycled +//! the forwarded bit is gone, and the cache would name an unrelated live +//! object. +//! +//! The tests install a *synthetic* stub in the cache and restore the previous +//! value on the way out, so they never disturb the realm's real intrinsics. + +use super::*; +use std::sync::atomic::{AtomicUsize, Ordering}; + +/// Save/restore both prototype-address caches around a test body. +struct ProtoAddrCacheGuard { + array: usize, + object: usize, +} + +impl ProtoAddrCacheGuard { + fn new() -> Self { + Self { + array: crate::array::test_array_proto_addr_cache().load(Ordering::Relaxed), + object: crate::array::test_object_proto_addr_cache().load(Ordering::Relaxed), + } + } +} + +impl Drop for ProtoAddrCacheGuard { + fn drop(&mut self) { + crate::array::test_array_proto_addr_cache().store(self.array, Ordering::Relaxed); + crate::array::test_object_proto_addr_cache().store(self.object, Ordering::Relaxed); + } +} + +/// Allocate a nursery object to stand in for the intrinsic. +fn nursery_stand_in() -> *mut u8 { + crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY) +} + +/// Evacuate `from`: allocate an old-gen destination and forward `from` → `to`. +fn evacuate(from: *mut u8) -> usize { + let to = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_ARRAY); + unsafe { + set_forwarding_address(header_from_user_ptr(from) as *mut GcHeader, to); + } + to as usize +} + +/// Allocate `from` and `to`, forward `from` → `to`, and return the pair. +fn forwarded_pair() -> (usize, usize) { + let from = nursery_stand_in(); + let to = evacuate(from); + (from as usize, to) +} + +fn cache_of(which: &str) -> &'static AtomicUsize { + if which == "array" { + crate::array::test_array_proto_addr_cache() + } else { + crate::array::test_object_proto_addr_cache() + } +} + +fn read_addr(which: &str) -> usize { + if which == "array" { + crate::array::array_prototype_addr() + } else { + crate::array::object_prototype_addr() + } +} + +/// DEFENCE 1. A memoized address that has been forwarded — by `js_array_grow` +/// or by an evacuating cycle — must read back as the object's CURRENT address, +/// and the cache must be healed in place so the next reader pays nothing. +/// +/// Without the heal this returns the from-space stub, which is a different +/// address for the same object than every `clean_arr_ptr`-resolved receiver — +/// the exact condition that defeats `array_oob_prototype_get`'s +/// self-recursion guard. +#[test] +fn prototype_addr_reads_through_a_forwarding_stub() { + let _guard = ProtoAddrCacheGuard::new(); + + for which in ["array", "object"] { + let (from, to) = forwarded_pair(); + cache_of(which).store(from, Ordering::Relaxed); + + assert_eq!( + read_addr(which), + to, + "{which}_prototype_addr must resolve the GC forwarding chain: a stale \ + from-space address is a DIFFERENT address for the SAME object than \ + every clean_arr_ptr-resolved receiver, which defeats the \ + `proto != receiver` self-recursion guard in the hole/OOB read \ + fallback and hangs the mutator (#6981)" + ); + assert_eq!( + cache_of(which).load(Ordering::Relaxed), + to, + "{which}_prototype_addr must write the healed address back so the \ + hot path stays a single relaxed load" + ); + } +} + +/// Multi-hop chains (grow, then grow again, then evacuate) must resolve all the +/// way to the live head. +#[test] +fn prototype_addr_reads_through_a_multi_hop_forwarding_chain() { + let _guard = ProtoAddrCacheGuard::new(); + + let first = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY); + let second = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY); + let final_user = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_ARRAY); + unsafe { + set_forwarding_address(header_from_user_ptr(first) as *mut GcHeader, second); + set_forwarding_address(header_from_user_ptr(second) as *mut GcHeader, final_user); + } + + crate::array::test_array_proto_addr_cache().store(first as usize, Ordering::Relaxed); + assert_eq!( + crate::array::array_prototype_addr(), + final_user as usize, + "every forwarding hop must be followed (#6981)" + ); +} + +/// DEFENCE 2. The collector must REWRITE the cache, not merely leave it +/// resolvable — from-space is reset and handed back to the mutator at the end +/// of the cycle, after which the forwarded bit is gone and healing cannot +/// recover the address. +#[test] +fn prototype_addr_cache_is_rewritten_by_the_collector() { + let _guard = ProtoAddrCacheGuard::new(); + + // The from-space objects must exist before the valid-pointer set is built — + // that set is what tells the rewrite visitor an address is a real heap + // object, exactly as in a real cycle. + let array_from = nursery_stand_in(); + let object_from = nursery_stand_in(); + let valid_ptrs = build_valid_pointer_set(); + let array_to = evacuate(array_from); + let object_to = evacuate(object_from); + crate::array::test_array_proto_addr_cache().store(array_from as usize, Ordering::Relaxed); + crate::array::test_object_proto_addr_cache().store(object_from as usize, Ordering::Relaxed); + + crate::array::scan_prototype_addr_cache_roots_mut(&mut RuntimeRootVisitor::for_rewrite( + &valid_ptrs, + )); + + assert_eq!( + crate::array::test_array_proto_addr_cache().load(Ordering::Relaxed), + array_to, + "the ARRAY_PROTO_ADDR slot must be rewritten by the relocating cycle — \ + it is a raw address of a movable object, exactly like the other \ + registered side tables (#6981)" + ); + assert_eq!( + crate::array::test_object_proto_addr_cache().load(Ordering::Relaxed), + object_to, + "the OBJECT_PROTO_ADDR slot must be rewritten by the relocating cycle \ + (#6981)" + ); +} + +/// …and it must actually be REGISTERED. The scanner above can be invoked +/// directly from a test whether or not `gc_init` ever mentions it, so assert +/// the wiring separately: an unregistered scanner is a no-op in production. +#[test] +fn prototype_addr_cache_scanner_is_registered() { + crate::gc::gc_init(); + let registered = crate::gc::roots::MUTABLE_ROOT_SCANNERS.with(|scanners| { + scanners.borrow().iter().any(|entry| { + entry.scanner as usize + == crate::array::scan_prototype_addr_cache_roots_mut as MutableRootScanner as usize + }) + }); + assert!( + registered, + "scan_prototype_addr_cache_roots_mut must be registered in gc_init — \ + otherwise the collector never rewrites ARRAY_PROTO_ADDR and the cache \ + is left naming from-space after the block is recycled (#6981)" + ); +} + +/// The not-yet-computed sentinel is not a heap address and must be left alone — +/// a scanner that rewrote it would pin a bogus prototype for the whole process. +#[test] +fn prototype_addr_cache_scanner_leaves_the_unset_sentinel_alone() { + let _guard = ProtoAddrCacheGuard::new(); + let valid_ptrs = build_valid_pointer_set(); + + crate::array::test_array_proto_addr_cache().store(usize::MAX, Ordering::Relaxed); + crate::array::test_object_proto_addr_cache().store(usize::MAX, Ordering::Relaxed); + + crate::array::scan_prototype_addr_cache_roots_mut(&mut RuntimeRootVisitor::for_rewrite( + &valid_ptrs, + )); + + assert_eq!( + crate::array::test_array_proto_addr_cache().load(Ordering::Relaxed), + usize::MAX + ); + assert_eq!( + crate::array::test_object_proto_addr_cache().load(Ordering::Relaxed), + usize::MAX + ); +} diff --git a/crates/perry-runtime/src/value/equality.rs b/crates/perry-runtime/src/value/equality.rs index eac900798f..cc6cc30034 100644 --- a/crates/perry-runtime/src/value/equality.rs +++ b/crates/perry-runtime/src/value/equality.rs @@ -188,7 +188,7 @@ pub extern "C" fn js_jsvalue_equals(a: f64, b: f64) -> i32 { /// callers can compare resolved addresses for object identity. `try_read_gc_- /// header` performs the band/heap classification (never dereferencing a /// non-heap id). Depth-capped to defend against corrupted GC cycles. -fn resolve_forwarding(mut addr: usize) -> usize { +pub(crate) fn resolve_forwarding(mut addr: usize) -> usize { unsafe { let mut steps = 0u32; while let Some(header) = crate::value::addr_class::try_read_gc_header(addr) { diff --git a/crates/perry-runtime/src/value/mod.rs b/crates/perry-runtime/src/value/mod.rs index 8ba91621f9..ebd0b2c457 100644 --- a/crates/perry-runtime/src/value/mod.rs +++ b/crates/perry-runtime/src/value/mod.rs @@ -123,6 +123,7 @@ pub use to_string::{ }; // ----- Equality, comparison, SameValueZero, dynamic string equality ----- +pub(crate) use equality::resolve_forwarding; pub use equality::{ js_dynamic_string_equals, js_jsvalue_compare, js_jsvalue_equals, js_jsvalue_loose_equals, js_jsvalue_same_value_zero, diff --git a/crates/perry/tests/gc_array_prototype_hole_read_6981.rs b/crates/perry/tests/gc_array_prototype_hole_read_6981.rs new file mode 100644 index 0000000000..fced6442a7 --- /dev/null +++ b/crates/perry/tests/gc_array_prototype_hole_read_6981.rs @@ -0,0 +1,213 @@ +//! #6981 — a hole read through a polluted `Array.prototype` must terminate +//! after the prototype has RELOCATED. +//! +//! `array::indexing` memoizes `Array.prototype`'s heap address in a global +//! `AtomicUsize`. Every reader of an array pointer resolves it through +//! `clean_arr_ptr`, which follows `GC_FLAG_FORWARDED` chains; the cache did +//! not. `array_oob_prototype_get`'s self-recursion guard is the object-identity +//! test `proto != receiver`, so the moment the prototype moves — leaving a +//! forwarding stub at the memoized address — the two sides name the same object +//! by two different addresses, the guard stops firing, and +//! `js_array_get_f64` ⇄ `array_oob_prototype_get` recurse until the thread's +//! stack guard page. The OS reports `EXC_BAD_ACCESS` / `KERN_PROTECTION_FAILURE` +//! with "Thread stack size exceeded due to excessive recursion"; the process +//! dies with SIGSEGV (exit 139). It is a control-flow defect, not memory +//! corruption: every dereference on the way down is to valid, mapped memory. +//! +//! Three conditions are individually necessary, and both programs below carry +//! all three: +//! +//! 1. `Array.prototype` has RELOCATED, so the memoized address is a stub; +//! 2. an `Array.prototype` **index write** exists, which is what sets +//! `ARRAY_PROTO_HAS_INDEX` and arms the prototype-consulting fallback +//! (merely *naming* `Array.prototype` is not enough); +//! 3. a **hole read** — reading an index that was never assigned — which is +//! what enters the fallback at all. Pre-filling the array removes the +//! crash entirely. +//! +//! # Why two programs +//! +//! There are two independent ways for `Array.prototype` to relocate, and they +//! need different defences, so a single arm could pass while the other stayed +//! broken: +//! +//! * `grow` — `Array.prototype[300] = v` runs `js_array_grow`, which +//! reallocates the dense backing store and forwards the old head. **No GC +//! is involved**, so no collector fix can reach it; the memoized address +//! has to resolve the chain itself. This arm reproduces with the collector +//! switched off entirely, which is also what proves the bug was never +//! "a GC bug". +//! * `relocate` — the copying young-gen minor evacuates the prototype. Here +//! the collector must REWRITE the cache, because from-space is reset and +//! handed back to the mutator at the end of the cycle, after which the +//! forwarded bit is gone and chain-following cannot recover the address. +//! +//! Measured against the unfixed compiler at `c0d98624a`: `grow` exits 139 in +//! every arm including `PERRY_GEN_GC=0`; `relocate` exits 139 in every arm that +//! runs a copying minor and passes in the arms that never relocate. With the +//! fix both are node-exact everywhere. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// GROW arm. `Array.prototype[300] = 555` is past the dense capacity, so +/// `js_array_grow` reallocates and leaves a forwarding stub at the memoized +/// address. `c[1]` is then a hole read on a 4-element `new Array(4)`, which +/// consults the (relocated) prototype. Needs no GC at all. +const SOURCE_GROW: &str = r#" +(Array.prototype as any)[300] = 555; +const c: number[] = new Array(4); +c[0] = 1; +console.log("" + c[1], "" + c[300]); +delete (Array.prototype as any)[300]; +"#; + +const ORACLE_GROW: &str = "undefined 555\n"; + +/// RELOCATE arm. The prototype write is in-capacity (no grow), so the ONLY way +/// the memoized address goes stale is a relocating collection. `histogram` +/// allocates enough to trip one under a heap budget and reads `counts[v]` as a +/// hole on every first touch of a bucket. +const SOURCE_RELOCATE: &str = r#" +(Array.prototype as any)[3] = 555; +function histogram(data: number[], size: number): number[] { + const counts: number[] = new Array(size); + const mask = size - 1; + for (let i = 0; i < data.length; i++) { + const v = data[i] & mask; + counts[v] = (counts[v] || 0) + 1; + } + return counts; +} +const data: number[] = []; +let seed = 4242; +for (let i = 0; i < 2000; i++) { + seed = (seed * 48271) % 2147483647; + data.push(seed); +} +console.log(histogram(data, 16).join(",")); +"#; + +/// node 26.5.0 (`.node-version`). Index 3 is the inherited 555, so its bucket +/// count is 555 higher than the others — the oracle would still be byte-exact +/// if the inherited value were dropped, which is why the assertion is on the +/// whole line and not just on termination. +const ORACLE_RELOCATE: &str = "120,105,125,679,142,125,133,128,115,135,126,109,121,117,134,141\n"; + +/// The test runner's own environment is inherited by `Command`, so a developer +/// (or a bisect script) exporting a collector kill switch would silently turn +/// every arm into a never-relocates control and the suite would pass against +/// the unfixed compiler. Clear the whole family, then apply the arm's own vars. +const GC_ENV_OVERRIDES: &[&str] = &[ + "PERRY_GEN_GC", + "PERRY_GEN_GC_EVACUATE", + "PERRY_GC_SCAVENGE", + "PERRY_GC_SCAVENGE_NURSERY_MB", + "PERRY_GC_MOVING_SAFEPOINT", + "PERRY_GC_MOVING_LOOP_POLLS", + "PERRY_GC_FORCE_EVACUATE", + "PERRY_GC_VERIFY_EVACUATION", + "PERRY_CONSERVATIVE_STACK_SCAN", + "PERRY_WRITE_BARRIERS", + "PERRY_GC_INCREMENTAL", + "PERRY_GC_HEAP_LIMIT", +]; + +fn compile(dir: &std::path::Path, source: &str, name: &str) -> PathBuf { + let entry = dir.join(format!("{name}.ts")); + let output = dir.join(format!("{name}_bin")); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed for {name}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + output +} + +fn run_arms(binary: &std::path::Path, dir: &std::path::Path, oracle: &str, what: &str) { + // Runtime-only knobs, so every arm runs exactly the same generated code. + let arms: Vec<(&str, Vec<(&str, &str)>)> = vec![ + ("default", vec![]), + ("heap_limit_8", vec![("PERRY_GC_HEAP_LIMIT", "8")]), + ( + "precise_roots", + vec![ + ("PERRY_GC_HEAP_LIMIT", "8"), + ("PERRY_GC_INCREMENTAL", "0"), + ("PERRY_CONSERVATIVE_STACK_SCAN", "off"), + ], + ), + ( + "force_evacuate", + vec![ + ("PERRY_GC_HEAP_LIMIT", "8"), + ("PERRY_GC_INCREMENTAL", "0"), + ("PERRY_CONSERVATIVE_STACK_SCAN", "off"), + ("PERRY_GC_FORCE_EVACUATE", "1"), + ], + ), + // Control: full mark-sweep never runs a copying minor. The `grow` arm + // must still fail here on the unfixed compiler (its relocation is + // `js_array_grow`, not the collector), so a green control is not + // inertness for that program. + ("gen_gc_off", vec![("PERRY_GEN_GC", "0")]), + ]; + + for (label, arm) in &arms { + let mut cmd = Command::new(binary); + cmd.current_dir(dir); + for key in GC_ENV_OVERRIDES { + cmd.env_remove(key); + } + for (k, v) in arm { + cmd.env(k, v); + } + let run = cmd.output().expect("run compiled binary"); + assert!( + run.status.success(), + "[{what}/{label}] compiled binary died (exit {:?}). Exit 139 here is \ + the #6981 stack overflow: the memoized `Array.prototype` address \ + went stale across a relocation, so the hole-read fallback's \ + `proto != receiver` identity guard compared a from-space address \ + against a forwarding-resolved one, never fired, and \ + `js_array_get_f64` ⇄ `array_oob_prototype_get` recursed until the \ + stack guard page.\nstderr:\n{}", + run.status.code(), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + oracle, + "[{what}/{label}] output must be byte-exact vs node 26.5.0" + ); + } +} + +#[test] +fn hole_read_through_a_grown_array_prototype_terminates() { + let dir = tempfile::tempdir().expect("tempdir"); + let binary = compile(dir.path(), SOURCE_GROW, "grow"); + run_arms(&binary, dir.path(), ORACLE_GROW, "grow"); +} + +#[test] +fn hole_read_through_a_relocated_array_prototype_terminates() { + let dir = tempfile::tempdir().expect("tempdir"); + let binary = compile(dir.path(), SOURCE_RELOCATE, "relocate"); + run_arms(&binary, dir.path(), ORACLE_RELOCATE, "relocate"); +} diff --git a/scripts/gc_repsel_matrix.sh b/scripts/gc_repsel_matrix.sh index f0af2f96be..2377a52306 100755 --- a/scripts/gc_repsel_matrix.sh +++ b/scripts/gc_repsel_matrix.sh @@ -206,18 +206,21 @@ ARMS=( # and the %E% arms could produce before. #6981's redness now reaches the arm # named after the shipped configuration. # -# `evac_minor` / `force_verify` stay out for the original reason, unchanged: -# they are RED for a real, filed reason (#6981), and per-PR redness on an -# unrelated PR is how a gate stops being read. They ARE in `--arms all`, which -# push / workflow_dispatch runs. -# -# WHEN #6981 CLOSES, PUT `evac_minor` AND `force_verify` BACK IN THIS LIST. -# That is the point at which "a representation regressed GC correctness under -# relocation" becomes a per-PR signal, which is the whole reason this matrix -# exists. Do not instead add triage entries for those cells: -# test-parity/gc_repsel_triage.txt is for redness that is provably NOT a -# representation defect, and #6981's redness may well be exactly that. -PR_ARMS="default,verify_evac,cons_scan_off,shipped_default" +# `evac_minor` AND `force_verify` ARE BACK IN, as the previous revision of this +# comment instructed. They were held out only while #6981 was red. It is fixed: +# the memoized `Array.prototype` address is a raw pointer to a MOVABLE object, +# so a relocation (`js_array_grow` or the copying minor) left the hole-read +# fallback's `proto != receiver` self-recursion guard comparing a from-space +# address against a forwarding-resolved one; the guard stopped firing and the +# mutator recursed until the stack guard page. Measured on the fix, +# `--arms all --pressure 8`: PASS=339 UNVER=100 XFAIL=1 FAIL=0 over 440 cells, +# with both arms at copy-minor 21/22 (was PASS=325 … FAIL=14). So "a +# representation regressed GC correctness under relocation" is now a per-PR +# signal on the arms that actually relocate, which is the whole reason this +# matrix exists. The single XFAIL is the pre-existing +# `repsel_ptr_shape_locals x rep_ptr_shape_off` entry (#6976), not in this +# subset. +PR_ARMS="default,evac_minor,verify_evac,force_verify,cons_scan_off,shipped_default" arm_field() { # $1 = arm record, $2 = 1..5 printf '%s' "$1" | cut -d'|' -f"$2" diff --git a/test-files/test_gap_array_proto_grow_hole_read.ts b/test-files/test_gap_array_proto_grow_hole_read.ts new file mode 100644 index 0000000000..d9d5e31faa --- /dev/null +++ b/test-files/test_gap_array_proto_grow_hole_read.ts @@ -0,0 +1,32 @@ +// #6981: a hole read must still terminate after `Array.prototype` RELOCATED. +// +// Writing an index past `Array.prototype`'s dense capacity reallocates its +// backing store (`js_array_grow`) and leaves a GC forwarding stub at the old +// head — with no GC involved at all. The runtime memoizes `Array.prototype`'s +// address, and the hole/OOB read fallback guards against self-recursion with +// the object-identity test `proto != receiver`. Readers resolve their receiver +// through the forwarding chain, so a memoized address that does not resolve +// names the same object by a different address, the guard stops firing, and the +// element read recurses until the stack guard page (SIGSEGV). +// +// Three conditions, all present below and each individually necessary: an +// `Array.prototype` INDEX WRITE (naming it is not enough), a HOLE READ (an +// index never assigned), and a prototype that has MOVED. + +const proto: any = Array.prototype; +proto[300] = 555; + +const c: number[] = new Array(4); +c[0] = 1; +console.log("" + c[1]); +console.log("" + c[300]); +console.log((c[1] as any) || -1); + +// A hole read on an index the prototype *does* carry inherits its value. +proto[2] = 777; +console.log("" + c[2]); +console.log(c[0] + ((c[2] as any) || 0)); + +delete proto[300]; +delete proto[2]; +console.log("" + c[2], "" + c[300]); From fc1d97bc4b05f331819b88e2f1b9e921f6e4e14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 30 Jul 2026 15:54:54 +0200 Subject: [PATCH 2/2] docs(changelog): #6981 Array.prototype address relocation fragment --- .../7071-array-prototype-addr-relocation.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 changelog.d/7071-array-prototype-addr-relocation.md diff --git a/changelog.d/7071-array-prototype-addr-relocation.md b/changelog.d/7071-array-prototype-addr-relocation.md new file mode 100644 index 0000000000..15d3389718 --- /dev/null +++ b/changelog.d/7071-array-prototype-addr-relocation.md @@ -0,0 +1,16 @@ +### Fixed + +- **`Array.prototype` hole reads no longer hang after the prototype relocates (#6981).** Reading an index that was never assigned, on a program that has installed an indexed property on `Array.prototype`, could drive the runtime into unbounded recursion and die with `SIGSEGV` on the thread's stack guard page ("Thread stack size exceeded due to excessive recursion"). It was not memory corruption — every dereference on the way down was to valid, mapped memory. + + `array::indexing` memoizes `Array.prototype`'s (and `Object.prototype`'s) heap address in a process-global `AtomicUsize`. That is a raw pointer to a **movable** object, and nothing maintained it. Every reader of an array pointer resolves it through `clean_arr_ptr`, which follows `GC_FLAG_FORWARDED` chains; the cache did not. The hole/out-of-bounds read fallback guards against self-recursion with the object-identity test `proto != receiver`, so once the prototype moved, the two sides named the same object by two different addresses, the guard stopped firing, and `js_array_get_f64` ⇄ `array_oob_prototype_get` called each other forever. + + Two independent relocations reach it, and only one is the collector: + + - **`js_array_grow`** — an indexed write past the dense capacity (`Array.prototype[300] = v`) reallocates the backing store and forwards the old head. **No GC is involved**: this reproduces with `PERRY_GEN_GC=0`, in the shipped configuration, with no environment overrides at all. + - **the copying young-gen minor** — it evacuates the prototype and forwards. This is the form #6981 measured, reachable in the shipped collector under a heap budget. + + Fixed with three defences: `array_prototype_addr` / `object_prototype_addr` resolve the forwarding chain and heal the cache in place (covering `js_array_grow`, which the collector never sees, with a call-free not-forwarded fast path); `scan_prototype_addr_cache_roots_mut` is registered in `gc_init` so a relocating cycle rewrites the slot like every other address-holding side table (needed because a swept, recycled from-space stub no longer carries the forwarded bit); and `array_oob_prototype_get` resolves the receiver before the identity compare, making the guard exact by construction. + + Same root cause also closes a silent-miss class: before this, a *first* `Array.prototype` index write occurring after a relocation would not set the pollution flag at all, so holes read `undefined` instead of the inherited value. + + `scripts/gc_repsel_matrix.sh --arms all --pressure 8` goes from `PASS=325 UNVER=100 XFAIL=1 FAIL=14` to `PASS=339 UNVER=100 XFAIL=1 FAIL=0` over 440 cells (byte-exact vs Node 26.5.0: 425/440 → 439/440), with nothing regressed. All 14 failures were `test_gap_repsel_p4a3_numarray_barriers`, in exactly the 14 arms whose `[gc-copy-minor]` count is non-zero. `evac_minor` and `force_verify` are back in the per-PR arm subset, so "a representation regressed GC correctness under relocation" is now a per-PR signal.