From f0f6a79df4e2058939b6ad504f73643799fcf47b Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 31 Jul 2026 21:06:26 -0400 Subject: [PATCH 1/3] fix(gc): restore SIDE_MASK when recording a pointer into an array's existing element mask layout_note_slot's pointer branch set the mask bit but left a stale POINTER_FREE layout state in place. heap_payload_slot_selection treats POINTER_FREE as pointer-free and skips the ENTIRE payload without ever consulting the mask, so the evacuating young-gen minor never traced the recorded pointer elements and reclaimed/relocated the live child out from under the slot -> 'TypeError: value is not a function' when later called. Same class as #6831. Recording a pointer proves the object is not pointer-free, so restore SIDE_MASK. Adds a regression test and a changelog fragment. Verified: PERRY_GC_FROMSPACE_SCAN within-length stale array->young edges drop 202 -> 0; cargo test -p perry-runtime green. --- changelog.d/gc-pointer-free-mask-invariant.md | 7 ++ crates/perry-runtime/src/gc/layout.rs | 14 ++++ .../src/gc/tests/layout_trace.rs | 71 +++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 changelog.d/gc-pointer-free-mask-invariant.md diff --git a/changelog.d/gc-pointer-free-mask-invariant.md b/changelog.d/gc-pointer-free-mask-invariant.md new file mode 100644 index 0000000000..8492f2f991 --- /dev/null +++ b/changelog.d/gc-pointer-free-mask-invariant.md @@ -0,0 +1,7 @@ +fix(gc): two moving-collector soundness hardenings for arrays. + +1. `layout_note_slot` now restores `SIDE_MASK` state whenever it records a pointer into an array's existing element mask. Previously a stale `POINTER_FREE` state could linger with a populated mask (e.g. after an array was truncated to a numeric/empty prefix), and `heap_payload_slot_selection` treats `POINTER_FREE` as "no pointers" — skipping the whole payload without consulting the mask. The evacuating young-gen minor then dropped live within-length pointer elements, later read as a garbage pointer (`TypeError: value is not a function`). Same class as #6831. + +2. `js_array_alloc` / `js_array_grow` now HOLE-initialize the `[length, capacity)` slack instead of leaving it as raw arena bytes. Uninitialized slack could hold stale pointer-shaped bits that any beyond-`length` scan/trace would follow into freed/relocated memory; HOLE is a non-pointer sentinel (matching `js_array_alloc_with_length`). + +Verified with `PERRY_GC_FROMSPACE_SCAN`: within-length stale array→young edges drop 202→0; uninitialized-slack false-positive edges drop from ~4000/cycle to ~0. `cargo test -p perry-runtime` array/layout suites green. diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index aff1978b64..1e88fca8fe 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -748,6 +748,20 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits if pointer { if let Some(mask) = masks.get_mut(&parent_user) { mask.set_slot(slot_index); + // A non-empty pointer mask MUST be reflected by SIDE_MASK + // state: `heap_payload_slot_selection` treats POINTER_FREE + // as "no pointers" and skips the WHOLE payload without ever + // consulting the mask. If a stale POINTER_FREE lingers here + // (an array truncated to a numeric/empty prefix flips to + // POINTER_FREE while its element mask is retained), recording + // a pointer would leave every masked element untraced — the + // evacuating minor then reclaims/relocates the child out from + // under the live slot, later read+called as a garbage pointer + // ("value is not a function"). Recording a pointer proves the + // object is not pointer-free, so restore SIDE_MASK. + if (*header)._reserved & GC_LAYOUT_STATE_MASK != GC_LAYOUT_SIDE_MASK { + set_layout_state(header, GC_LAYOUT_SIDE_MASK); + } } else if (*header)._reserved & GC_LAYOUT_STATE_MASK == GC_LAYOUT_POINTER_FREE { let mut mask = LayoutSlotMask::Inline(0); mask.set_slot(slot_index); diff --git a/crates/perry-runtime/src/gc/tests/layout_trace.rs b/crates/perry-runtime/src/gc/tests/layout_trace.rs index 1f52121ece..aabd3fcdce 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace.rs +++ b/crates/perry-runtime/src/gc/tests/layout_trace.rs @@ -259,6 +259,77 @@ fn test_layout_mask_small_mixed_array_scans_exact_pointer_slot() { clear_mark_seeds(); } +#[test] +fn test_pointer_store_restores_side_mask_from_stale_pointer_free() { + // #sfw-registry regression: an array can end up in POINTER_FREE layout + // state while still carrying a populated element pointer-mask (e.g. after a + // numeric-layout rebuild observed a short/empty prefix while the mask + // lingered). `heap_payload_slot_selection` short-circuits POINTER_FREE and + // skips the WHOLE payload without consulting the mask, so the evacuating + // minor would drop live pointer elements. `layout_note_slot` must restore + // SIDE_MASK whenever it records a pointer into an existing mask. + clear_marks(); + clear_mark_seeds(); + + let child0 = crate::string::js_string_from_bytes(b"c0".as_ptr(), 2) as *mut u8; + let child1 = crate::string::js_string_from_bytes(b"c1".as_ptr(), 2) as *mut u8; + let child0_h = unsafe { header_from_user_ptr(child0) }; + let child1_h = unsafe { header_from_user_ptr(child1) }; + + let arr = crate::array::js_array_alloc_with_length(2); + // Store a pointer at index 0 -> SIDE_MASK + mask{0}. + crate::array::js_array_set_f64( + arr, + 0, + f64::from_bits(STRING_TAG | (child0 as u64 & POINTER_MASK)), + ); + assert_eq!(test_layout_pointer_slot_count(arr as usize, 2), Some(1)); + + // Reproduce the stale-state hazard: force POINTER_FREE while the mask{0} + // entry is still present (`set_layout_state` only touches the state bits). + let arr_header = unsafe { header_from_user_ptr(arr as *mut u8) }; + unsafe { + set_layout_state(arr_header, GC_LAYOUT_POINTER_FREE); + } + + // Store a pointer at index 1 through the normal array store path. + crate::array::js_array_set_f64( + arr, + 1, + f64::from_bits(STRING_TAG | (child1 as u64 & POINTER_MASK)), + ); + + // The fix: recording a pointer must have restored SIDE_MASK, so the mask is + // consulted and BOTH pointer slots are visible to the tracer. + unsafe { + assert_eq!( + (*arr_header)._reserved & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_SIDE_MASK, + "recording a pointer into an existing mask must restore SIDE_MASK" + ); + } + assert_eq!(test_layout_pointer_slot_count(arr as usize, 2), Some(2)); + + let valid_ptrs = build_valid_pointer_set(); + let mut worklist = Vec::new(); + unsafe { + trace_array(arr as *mut u8, &valid_ptrs, &mut worklist); + assert_ne!( + (*child0_h).gc_flags & GC_FLAG_MARKED, + 0, + "index 0 pointer must be traced" + ); + assert_ne!( + (*child1_h).gc_flags & GC_FLAG_MARKED, + 0, + "index 1 pointer (stored under stale POINTER_FREE) must be traced" + ); + } + + clear_marks(); + clear_mark_seeds(); +} + #[test] fn test_layout_mask_heap_conversion_keeps_sparse_words_zeroed() { clear_marks(); From e0479662138835ef96c727a5c0f1ff9e22a76052 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 31 Jul 2026 21:06:26 -0400 Subject: [PATCH 2/3] fix(gc): HOLE-initialize array capacity slack in js_array_alloc/js_array_grow The [length, capacity) slack was left as raw arena bytes; stale pointer-shaped bits there are followed as live pointers by any beyond-length scan/trace (SIGSEGV if evacuated) and flood PERRY_GC_FROMSPACE_SCAN with false positives (~4000/cycle). HOLE-initialize the slack so it is a non-pointer sentinel, matching js_array_alloc_with_length. --- crates/perry-runtime/src/array/alloc.rs | 7 +++++++ crates/perry-runtime/src/array/push_pop.rs | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/crates/perry-runtime/src/array/alloc.rs b/crates/perry-runtime/src/array/alloc.rs index bb9ee19329..433ef29a80 100644 --- a/crates/perry-runtime/src/array/alloc.rs +++ b/crates/perry-runtime/src/array/alloc.rs @@ -47,6 +47,13 @@ pub extern "C" fn js_array_alloc(capacity: u32) -> *mut ArrayHeader { // Initialize header (*ptr).length = 0; (*ptr).capacity = actual_capacity; + // EXPERIMENT (#sfw-registry slack): HOLE-initialize the whole capacity so + // the unused [length, capacity) slack never holds stale arena bits that + // the whole-heap from-space scan misreads as live from-space pointers. + let elements_ptr = (ptr as *mut u8).add(std::mem::size_of::()) as *mut u64; + for i in 0..actual_capacity as usize { + std::ptr::write(elements_ptr.add(i), crate::value::TAG_HOLE); + } set_array_numeric_layout(ptr, NumericArrayLayout::RawF64); crate::gc::layout_init_pointer_free(ptr as *mut u8); } diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index f622d069a4..07d17fff6f 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -91,6 +91,16 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu ptr::copy_nonoverlapping(arr as *const u8, new_ptr as *mut u8, old_size); (*new_ptr).capacity = new_capacity; + // EXPERIMENT (#sfw-registry slack): HOLE-initialize the newly added + // [old_capacity, new_capacity) slack so it never holds stale arena bits + // the whole-heap from-space scan misreads as live from-space pointers. + { + let new_elems = + (new_ptr as *mut u8).add(std::mem::size_of::()) as *mut u64; + for i in old_capacity as usize..new_capacity as usize { + ptr::write(new_elems.add(i), crate::value::TAG_HOLE); + } + } let old_header = (arr as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; let new_header = From 4de9ad9277e4688f3e5ca8a30ac3e7dab098e6a4 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 31 Jul 2026 23:13:15 -0400 Subject: [PATCH 3/3] chore(gc): drop internal tracker tags from array and layout comments --- crates/perry-runtime/src/array/alloc.rs | 6 +++--- crates/perry-runtime/src/array/push_pop.rs | 6 +++--- crates/perry-runtime/src/gc/tests/layout_trace.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/perry-runtime/src/array/alloc.rs b/crates/perry-runtime/src/array/alloc.rs index 433ef29a80..9286cf2ce1 100644 --- a/crates/perry-runtime/src/array/alloc.rs +++ b/crates/perry-runtime/src/array/alloc.rs @@ -47,9 +47,9 @@ pub extern "C" fn js_array_alloc(capacity: u32) -> *mut ArrayHeader { // Initialize header (*ptr).length = 0; (*ptr).capacity = actual_capacity; - // EXPERIMENT (#sfw-registry slack): HOLE-initialize the whole capacity so - // the unused [length, capacity) slack never holds stale arena bits that - // the whole-heap from-space scan misreads as live from-space pointers. + // HOLE-initialize the whole capacity so the unused [length, capacity) + // slack never holds stale arena bits that the whole-heap from-space + // scan misreads as live from-space pointers. let elements_ptr = (ptr as *mut u8).add(std::mem::size_of::()) as *mut u64; for i in 0..actual_capacity as usize { std::ptr::write(elements_ptr.add(i), crate::value::TAG_HOLE); diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 07d17fff6f..0f4daba730 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -91,9 +91,9 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu ptr::copy_nonoverlapping(arr as *const u8, new_ptr as *mut u8, old_size); (*new_ptr).capacity = new_capacity; - // EXPERIMENT (#sfw-registry slack): HOLE-initialize the newly added - // [old_capacity, new_capacity) slack so it never holds stale arena bits - // the whole-heap from-space scan misreads as live from-space pointers. + // HOLE-initialize the newly added [old_capacity, new_capacity) slack + // so it never holds stale arena bits the whole-heap from-space scan + // misreads as live from-space pointers. { let new_elems = (new_ptr as *mut u8).add(std::mem::size_of::()) as *mut u64; diff --git a/crates/perry-runtime/src/gc/tests/layout_trace.rs b/crates/perry-runtime/src/gc/tests/layout_trace.rs index aabd3fcdce..f4288f36bc 100644 --- a/crates/perry-runtime/src/gc/tests/layout_trace.rs +++ b/crates/perry-runtime/src/gc/tests/layout_trace.rs @@ -261,8 +261,8 @@ fn test_layout_mask_small_mixed_array_scans_exact_pointer_slot() { #[test] fn test_pointer_store_restores_side_mask_from_stale_pointer_free() { - // #sfw-registry regression: an array can end up in POINTER_FREE layout - // state while still carrying a populated element pointer-mask (e.g. after a + // Regression: an array can end up in POINTER_FREE layout state while + // still carrying a populated element pointer-mask (e.g. after a // numeric-layout rebuild observed a short/empty prefix while the mask // lingered). `heap_payload_slot_selection` short-circuits POINTER_FREE and // skips the WHOLE payload without consulting the mask, so the evacuating