diff --git a/changelog.d/8041-array-growth-forwarding.md b/changelog.d/8041-array-growth-forwarding.md new file mode 100644 index 0000000000..c62ffb751b --- /dev/null +++ b/changelog.d/8041-array-growth-forwarding.md @@ -0,0 +1 @@ +**Fixed: growing arrays at valid low arena addresses now retain stale-reference forwarding across repeated growth and moving minor GC (#8035).** Array header access now requires allocator ownership before dereferencing, rejects invalid forwarding targets, and installs growth forwarding through a checked raw-header pointer. diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 4c586410d5..aba6685dc0 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -537,15 +537,11 @@ pub(crate) fn test_template_raw_roots() -> (usize, usize) { /// Strip NaN-boxing tags from an array pointer and guard against invalid values. /// -/// Issue #73 follow-up: the `> 0x1000` (4 KB) floor is too permissive -/// for the macOS ARM64 heap layout. A corrupted NaN-box whose 48-bit -/// handle lands in the 1 TB — 2 TB window (e.g. `0x00FF_0000_0000` — -/// a `BufferHeader { length: 0, capacity: 255 }` read as u64) clears -/// the old floor and segfaults `(*arr).length` / SIMD memcpy inside -/// `js_array_slice` / `js_array_length` / etc. Real mimalloc + arena -/// allocations on Darwin consistently land in the 3-5 TB range; -/// constraining to `>= 2 TB && < 128 TB` rejects the observed -/// corruption patterns without cutting off any real heap pointer. +/// Issue #73/#8035 follow-up: address magnitude is not proof of ownership. +/// Corrupted NaN-box payloads can land in a plausible heap window, while real +/// macOS mimalloc arena allocations can land below the former 2 TiB floor. +/// Registry-handle bands are rejected first; GC headers are read only after +/// arena page metadata or the malloc registry proves ownership. /// /// v0.5.85 follow-up: also validate the GC header byte + length/capacity /// sanity. A pointer that passes the range check but points into the @@ -553,52 +549,11 @@ pub(crate) fn test_template_raw_roots() -> (usize, usize) { /// e.g. decoded PostgreSQL text column data) reads garbage length /// values — witnessed `len=775370038 cap=926234674` (both the ASCII /// bytes of `"6+2.2017"`) flowing through `js_array_slice` and -/// triggering 22GB-wide memcpy segfaults. Post-check: obj_type at -/// `handle-8` must equal GC_TYPE_ARRAY (1), and length must be -/// <= capacity <= 16M (same bound as the GC tracer's sanity guard). +/// triggering 22GB-wide memcpy segfaults. The post-check therefore requires +/// `GC_TYPE_ARRAY` and validates length/capacity before any element access +/// (with the registered Buffer/TypedArray and sparse-array exceptions below). #[inline(always)] pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { - // Heap window varies by allocator and run: macOS mimalloc can land well - // below 2 TB (observed around 45 GB in the Rust test harness); - // Android scudo + Linux glibc also allocate MUCH lower (often < 1 TB); Windows - // mimalloc lands well under 1 TB (often in the GB-to-tens-of-GB range). - // iOS / tvOS / watchOS / visionOS *device* targets use libsystem_malloc - // (mimalloc is host-side only) and allocate in the same low range — - // #1136's `for…of` over `split()` reproed empty because the array - // pointer landed below 2 TB and `clean_arr_ptr` silently null-ed it. - // Using the macOS-tight 2 TB floor on Android / Windows / iOS-family - // silently null-s every real array pointer, turning js_array_set_f64 - // into a no-op and — at the read side via js_array_map etc. — - // returning empty arrays for legitimate inputs (issues #385/#386/#387 - // for non-macOS hosts; #1136 for iOS device). - // - // The iOS *simulator* runs on the macOS host's mimalloc and lands in - // the 3-5 TB range like macOS itself; lowering the floor to 4 KB does - // not weaken the guard there because the actual liveness check is the - // GcHeader / obj_type validation downstream. - #[cfg(any( - target_os = "android", - target_os = "macos", - target_os = "linux", - target_os = "windows", - target_os = "ios", - target_os = "tvos", - target_os = "watchos", - target_os = "visionos", - ))] - const HEAP_MIN: u64 = 0x1000; // 4 KB (classic user-space floor) - #[cfg(not(any( - target_os = "android", - target_os = "macos", - target_os = "linux", - target_os = "windows", - target_os = "ios", - target_os = "tvos", - target_os = "watchos", - target_os = "visionos", - )))] - const HEAP_MIN: u64 = 0x200_0000_0000; // 2 TB — retained for unlisted targets - const HEAP_MAX: u64 = 0x8000_0000_0000; // 47-bit userspace cap let bits = arr as u64; let top16 = bits >> 48; let cleaned = if top16 >= 0x7FF8 { @@ -606,24 +561,14 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { return std::ptr::null(); } let cleaned_bits = bits & 0x0000_FFFF_FFFF_FFFF; - if !(HEAP_MIN..HEAP_MAX).contains(&cleaned_bits) { - return std::ptr::null(); - } cleaned_bits as *const ArrayHeader } else { - if !(HEAP_MIN..HEAP_MAX).contains(&bits) { - return std::ptr::null(); - } arr }; - // #5432: reject small-handle ids (fetch/zlib/proxy/common-registry) that - // reached an array helper as the receiver. On non-macOS hosts HEAP_MIN is - // 0x1000 — below the handle band — so a handle like a fetch Headers id - // (0x40000) passes the window check above, and the forwarding-chain / - // obj_type derefs below (`cleaned - 8`) would read unmapped low memory and - // SIGSEGV. Magnitude-classify before any deref and null it (safe - // empty-result no-op), mirroring the addr_class band-map contract. - if crate::value::addr_class::is_handle_band(cleaned as usize) { + // Preserve the permissive window needed by registered Buffer/TypedArray + // receivers, but centralize its platform policy in addr_class. Actual GC + // header reads below require allocator ownership as well. + if !crate::value::addr_class::is_plausible_heap_addr(cleaned as usize) { return std::ptr::null(); } // Issue #233: follow GC_FLAG_FORWARDED forwarding chains. When @@ -639,19 +584,28 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { // practice (1-2 grows) but cap depth at 64 to defend against // cycles from corrupted GC state. let mut cleaned = cleaned; + let mut tracked_header = + unsafe { crate::value::addr_class::try_read_tracked_gc_header(cleaned as usize) }; unsafe { let mut steps = 0u32; - while (cleaned as usize) >= crate::gc::GC_HEADER_SIZE + 0x1000 { - let gc_header = - (cleaned as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; - if (*gc_header).gc_flags & crate::gc::GC_FLAG_FORWARDED == 0 { + while let Some(gc_header) = tracked_header { + let gc_header = gc_header.as_ptr(); + if (*gc_header).obj_type != crate::gc::GC_TYPE_ARRAY + || (*gc_header).gc_flags & crate::gc::GC_FLAG_FORWARDED == 0 + { break; } - let new_user = crate::gc::forwarding_address(gc_header) as u64; - if !(HEAP_MIN..HEAP_MAX).contains(&new_user) { + let new_user = crate::gc::forwarding_address(gc_header) as usize; + let Some(target_header) = + crate::value::addr_class::try_read_tracked_gc_header(new_user) + else { + return std::ptr::null(); + }; + if (*target_header.as_ptr()).obj_type != crate::gc::GC_TYPE_ARRAY { return std::ptr::null(); } cleaned = new_user as *const ArrayHeader; + tracked_header = Some(target_header); steps += 1; if steps > 64 { return std::ptr::null(); @@ -665,56 +619,33 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { // into a real ArrayHeader and substitute the materialized // pointer for every downstream accessor. O(1) on subsequent // calls (idempotent via the `materialized` cache). - unsafe { - if (cleaned as usize) >= crate::gc::GC_HEADER_SIZE + 0x1000 { - let gc_header = - (cleaned as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; - let obj_type = (*gc_header).obj_type; - if obj_type == crate::gc::GC_TYPE_LAZY_ARRAY { + let addr = cleaned as usize; + let tracked_obj_type = + tracked_header.map(|gc_header| unsafe { (*gc_header.as_ptr()).obj_type }); + if let Some(obj_type) = tracked_obj_type { + if obj_type == crate::gc::GC_TYPE_LAZY_ARRAY { + unsafe { let lazy = cleaned as *mut crate::json_tape::LazyArrayHeader; if (*lazy).magic == crate::json_tape::LAZY_ARRAY_MAGIC { let materialized = crate::json_tape::force_materialize_lazy(lazy); return materialized as *const ArrayHeader; } } - // #7574: a `GC_TYPE_OBJECT` / `GC_TYPE_CLOSURE` allocation is NOT - // an `ArrayHeader`, and the two layouts overlay field for field — - // `ArrayHeader.length` reads `ObjectHeader.object_type` (= 1), - // `.capacity` reads `class_id`, and the element slots at +8/+16/+24 - // are `parent_class_id ‖ field_count`, `keys_array` and `meta`. The - // sanity check below waves that through (1 <= class_id <= 100M), so - // an element WRITE overwrites two live GC child edges with - // arbitrary doubles and the collector then traces them: `class X - // extends Array` in a `T[]`-annotated binding SIGSEGVs on its - // second `.push()`. - // - // A declared TypeScript type is a hint, never a layout fact - // (CLAUDE.md, *Known Limitations*), so this is reachable from every - // binding form. Refusing it here makes ALL ~190 `clean_arr_ptr` - // call sites fail-closed at once — each degrades through its - // existing null branch instead of dereferencing a forged header — - // and it is the same "resolve at the shared runtime funnel, not at - // one codegen predicate at a time" shape #7573 used for Map/Set. - // - // Correctness (rather than mere safety) for the entry points the - // declared-type tiers actually reach is layered on top: those null - // branches re-enter through `array::subclass::array_object_*`, - // which runs the operation on the spec-generic array-like engine. - // - // Costs one compare on a byte this block already loaded. Buffers - // and typed arrays are `std::alloc`-backed with no `GcHeader`, so - // their preceding bytes are allocator bookkeeping that can read as - // any value — confirm against the registries before nulling, in the - // cold arm only. - if obj_type == crate::gc::GC_TYPE_OBJECT || obj_type == crate::gc::GC_TYPE_CLOSURE { - let addr = cleaned as usize; - if !crate::buffer::is_registered_buffer(addr) - && crate::typedarray::lookup_typed_array_kind(addr).is_none() - { - return std::ptr::null(); - } - } } + // A declared TypeScript type is a hint, never a layout fact. Reject + // every tracked non-array at this shared funnel before treating its + // payload as an ArrayHeader (#7574). + if obj_type != crate::gc::GC_TYPE_ARRAY { + return std::ptr::null(); + } + } else if !crate::buffer::is_registered_buffer(addr) + && crate::typedarray::lookup_typed_array_kind(addr).is_none() + { + // Handles, synthetic pointers, and unrelated allocations must be + // rejected before any GcHeader or ArrayHeader dereference. Registered + // Buffer/TypedArray receivers intentionally use the compatible + // length/capacity prefix and carry no GcHeader. + return std::ptr::null(); } // Length/capacity sanity: dense arrays have length <= capacity and // length below 100M (800 MB of element payload — well above legitimate @@ -732,15 +663,9 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { // wave them through; everything else at this size is // almost certainly corrupted. let addr = cleaned as usize; - let sparse_array_shape = if addr >= crate::gc::GC_HEADER_SIZE + 0x1000 { - let gc_header = (cleaned as *const u8).sub(crate::gc::GC_HEADER_SIZE) - as *const crate::gc::GcHeader; - (*gc_header).obj_type == crate::gc::GC_TYPE_ARRAY - && hdr.length > hdr.capacity - && hdr.capacity <= 1_000_000 - } else { - false - }; + let sparse_array_shape = tracked_obj_type == Some(crate::gc::GC_TYPE_ARRAY) + && hdr.length > hdr.capacity + && hdr.capacity <= 1_000_000; if sparse_array_shape { return cleaned; } diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 8d69c32eab..3de073abfb 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -35,6 +35,33 @@ fn throw_non_writable_length() -> ! { ); } +/// Install an array-growth forwarding stub after `tracked_header_for` proves +/// allocator ownership of `old_user_addr`. Keeping the classifier injectable +/// makes the below-2-TiB macOS case deterministic without mapping a fixed low +/// virtual address in the test process. +/// +/// # Safety +/// The classifier must return the live header for `old_user_addr`, and +/// `new_user_addr` must be a live array allocation. +#[inline] +pub(super) unsafe fn install_array_growth_forwarding_with( + old_user_addr: usize, + new_user_addr: *mut u8, + tracked_header_for: impl FnOnce(usize) -> Option>, +) -> bool { + let Some(header) = tracked_header_for(old_user_addr) else { + return false; + }; + let header = header.as_ptr(); + if (*header).obj_type != crate::gc::GC_TYPE_ARRAY + || (*header).gc_flags & crate::gc::GC_FLAG_ARENA == 0 + { + return false; + } + crate::gc::set_forwarding_address(header, new_user_addr); + true +} + #[inline] pub(crate) fn guard_writable_length(arr: *const ArrayHeader) { if array_length_is_non_writable(arr) { @@ -135,46 +162,20 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu // ptr. Unlike GC-evacuation originals, array-growth stubs stay // retained because stale array references rely on clean_arr_ptr // following this chain. - // Only valid for arena-allocated arrays (which have a GcHeader - // 8 bytes before the user pointer); guard with a heap-bounds - // check that mirrors clean_arr_ptr's HEAP_MIN to skip pointers - // that don't have a real GcHeader behind them (e.g. test-mode - // synthetic pointers, longlived-arena edge cases). - // #1136: iOS family device allocates via libsystem_malloc in the - // same low range as Android/Linux; mirror `clean_arr_ptr`'s - // platform split so growth forwarding can install a stub for - // arrays that live below 2 TB. - #[cfg(any( - target_os = "android", - target_os = "linux", - target_os = "windows", - target_os = "ios", - target_os = "tvos", - target_os = "watchos", - target_os = "visionos", - ))] - const HEAP_MIN: usize = 0x1000; - #[cfg(not(any( - target_os = "android", - target_os = "linux", - target_os = "windows", - target_os = "ios", - target_os = "tvos", - target_os = "watchos", - target_os = "visionos", - )))] - const HEAP_MIN: usize = 0x200_0000_0000; - if (arr as usize) >= HEAP_MIN + crate::gc::GC_HEADER_SIZE { - // Only forward arrays that came from the GC arena. A - // non-array obj_type would mean something has gone wrong - // upstream; bail out without forwarding rather than corrupt - // an unrelated allocation's header. - if (*old_header).obj_type == crate::gc::GC_TYPE_ARRAY { - crate::gc::set_forwarding_address(old_header, new_ptr as *mut u8); - } - } else { - report_growth_stub_skipped_below_heap_min(arr as usize); - } + // Ownership comes from the same canonical arena/malloc classifier + // that clean_arr_ptr uses while following this stub. In particular, + // valid low-address macOS arena allocations are accepted, while + // handles, synthetic pointers, and unrelated allocations are rejected + // before a header dereference. + let installed = + install_array_growth_forwarding_with(arr as usize, new_ptr as *mut u8, |addr| { + crate::value::addr_class::try_read_tracked_gc_header(addr) + }); + assert!( + installed, + "array growth could not install a forwarding stub for the tracked source at {:#x}", + arr as usize + ); new_ptr } @@ -1109,38 +1110,3 @@ pub extern "C" fn js_array_unshift_variadic( #[used] static KEEP_UNSHIFT_VARIADIC: extern "C" fn(*mut ArrayHeader, *const f64, u32) -> *mut ArrayHeader = js_array_unshift_variadic; - -/// The `HEAP_MIN` guard above is an **address-conditional silent divergence**: -/// whether a growth forwarding stub is installed depends on where the allocator -/// happened to place the array. Below the floor the stub is skipped, so a stale -/// pre-grow reference stops resolving (issue #233's whole mechanism) — with no -/// signal at all. -/// -/// That silence has already cost real debugging time. While investigating -/// #7022 an experiment replaced arena blocks with `mmap`'d, guard-paged blocks; -/// `mmap` with a NULL hint lands well below macOS's 2 TB floor, so this branch -/// silently disabled every growth stub and the experiment came back **falsely -/// clean**. It was only caught by re-running with a high `MAP_FIXED` hint. -/// -/// Emit once per process so the next person gets a signal instead of a silent -/// behaviour change. One line on stderr, so it cannot perturb a stdout parity -/// comparison. -#[cold] -fn report_growth_stub_skipped_below_heap_min(arr_addr: usize) { - use std::sync::atomic::{AtomicBool, Ordering}; - static REPORTED: AtomicBool = AtomicBool::new(false); - debug_assert!( - false, - "array-growth forwarding stub skipped: array at {arr_addr:#x} is below the platform heap floor" - ); - if REPORTED.swap(true, Ordering::Relaxed) { - return; - } - eprintln!( - "[perry-gc] array-growth forwarding stub SKIPPED for an array at {arr_addr:#x}: \ -address is below this platform's heap floor. Stale pre-grow array references \ -will no longer resolve through the growth chain (issue #233). This is normally \ -unreachable; it usually means the arena is being backed by an allocator that \ -places blocks outside the expected range. Reported once per process." - ); -} diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index 8cf29cf12e..6248c7a8eb 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -538,6 +538,139 @@ fn test_array_push_f64_grow_path_preserves_value_and_forwarding() { ); } +#[test] +fn stale_array_reference_survives_three_growths_and_forced_minor_gc() { + let _copying_nursery = crate::gc::CopyingNurseryTestGuard::new(0); + let _triggers = crate::gc::GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _force_evacuation = crate::gc::knob_overrides::ForcedEvacuationTestGuard::on(); + let _verify_evacuation = crate::gc::knob_overrides::VerifyEvacuationTestGuard::on(); + crate::gc::register_runtime_handle_root_scanner_for_tests(); + let initial = js_array_alloc(0); + let mut head = initial; + + // Capacity progresses 16 -> 32 -> 64 -> 128. Keeping `initial` unchanged + // makes every assertion exercise the complete three-stub chain. + for i in 0..65u32 { + head = js_array_push_f64(head, i as f64); + } + assert_ne!(head, initial); + assert_eq!(clean_arr_ptr_mut(initial), head); + assert_eq!(js_array_length(initial), 65); + for i in 0..65u32 { + assert_eq!(js_array_get_f64(initial, i), i as f64); + } + + // Root the current head, not the deliberately stale first allocation: the + // handle must prove that evacuation moved the live array itself, while + // `initial` independently exercises the three growth stubs afterward. + let scope = crate::gc::RuntimeHandleScope::new(); + let root = scope.root_raw_mut_ptr(head); + let pre_gc_head = head; + let cycles_before = crate::gc::copying_minor_cycles(); + let (_, rooted_head) = root.across_mut::(|| { + let _ = crate::gc::gc_collect_minor(); + }); + let cycles_after = crate::gc::copying_minor_cycles(); + assert!( + cycles_after > cycles_before, + "forced collection must complete a copying minor" + ); + assert_ne!( + rooted_head, pre_gc_head, + "forced evacuation must move the live array head" + ); + assert_eq!( + clean_arr_ptr_mut(initial), + rooted_head, + "the stale three-stub chain must resolve to the relocated rooted head" + ); + assert_eq!(js_array_length(initial), 65); + for i in 0..65u32 { + assert_eq!(js_array_get_f64(initial, i), i as f64); + } +} + +#[test] +fn install_array_growth_forwarding_with_installs_stub_for_injected_header() { + // Actual low-address classification is covered by + // value::addr_class::tests::tracked_gc_classifier_accepts_injected_low_arena_membership. + // This test proves the install path uses the injected tracked header. + const LOW_USER: usize = 0x1_0000_0008; + const { assert!(LOW_USER < 0x200_0000_0000) }; + let old = js_array_alloc(0); + let new = js_array_alloc(0); + unsafe { + let old_header = + crate::value::addr_class::try_read_tracked_gc_header(old as usize).unwrap(); + let header_ptr = old_header.as_ptr(); + let flags = (*header_ptr).gc_flags; + let payload = *(old as *const u64); + + let installed = super::push_pop::install_array_growth_forwarding_with( + LOW_USER, + new as *mut u8, + |candidate| { + assert_eq!(candidate, LOW_USER); + Some(old_header) + }, + ); + let resolved = clean_arr_ptr(old); + + *(old as *mut u64) = payload; + (*header_ptr).gc_flags = flags; + assert!(installed); + assert_eq!(resolved, new); + } +} + +#[test] +fn clean_arr_ptr_rejects_forwarding_cycle() { + let first = js_array_alloc(0); + let second = js_array_alloc(0); + unsafe { + let first_header = crate::value::addr_class::try_read_tracked_gc_header(first as usize) + .unwrap() + .as_ptr(); + let second_header = crate::value::addr_class::try_read_tracked_gc_header(second as usize) + .unwrap() + .as_ptr(); + let first_flags = (*first_header).gc_flags; + let second_flags = (*second_header).gc_flags; + let first_payload = *(first as *const u64); + let second_payload = *(second as *const u64); + crate::gc::set_forwarding_address(first_header, second as *mut u8); + crate::gc::set_forwarding_address(second_header, first as *mut u8); + + let resolved = clean_arr_ptr(first); + + *(first as *mut u64) = first_payload; + *(second as *mut u64) = second_payload; + (*first_header).gc_flags = first_flags; + (*second_header).gc_flags = second_flags; + assert!(resolved.is_null()); + } +} + +#[test] +fn clean_arr_ptr_rejects_untracked_forwarding_target_without_deref() { + let array = js_array_alloc(0); + let unrelated = 0x20_0000usize as *mut u8; + unsafe { + let header = crate::value::addr_class::try_read_tracked_gc_header(array as usize) + .unwrap() + .as_ptr(); + let flags = (*header).gc_flags; + let payload = *(array as *const u64); + crate::gc::set_forwarding_address(header, unrelated); + + let resolved = clean_arr_ptr(array); + + *(array as *mut u64) = payload; + (*header).gc_flags = flags; + assert!(resolved.is_null()); + } +} + #[test] fn test_numeric_array_layout_metadata_preserves_and_downgrades_on_writes() { let mut arr = js_array_alloc(4); diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 268963a084..bc4748ca16 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -1415,3 +1415,8 @@ mod tests; /// on a parallel test thread can wipe its entries mid-test. #[cfg(test)] pub(crate) use tests::support::copying_nursery_isolation_lock as global_side_table_test_lock; +#[cfg(test)] +pub(crate) use tests::support::{ + register_runtime_handle_root_scanner_for_tests, CopyingNurseryTestGuard, + GcTriggerThresholdTestGuard, +}; diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index 9b022a22cd..38bb9da2e5 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -222,7 +222,7 @@ pub(super) fn root_scanner_registry_counts() -> (usize, usize, usize, usize) { /// `RuntimeHandleScope` inside such a test is decorative — its handles are /// neither marked nor rewritten, so a raw pointer held across a GC-capable /// call is silently unrooted and the test can pass for the wrong reason. -pub(super) fn register_runtime_handle_root_scanner_for_tests() { +pub(crate) fn register_runtime_handle_root_scanner_for_tests() { gc_register_budgeted_mutable_root_scanner_with_source( scan_runtime_handle_roots_mut, scan_runtime_handle_roots_mut_step, @@ -403,7 +403,7 @@ impl Drop for GcTestIsolationGuard { } } -pub(super) struct CopyingNurseryTestGuard { +pub(crate) struct CopyingNurseryTestGuard { frame: u64, _scanner_guard: ScopedRootScannerRegistryGuard, _lock: std::sync::MutexGuard<'static, ()>, @@ -447,7 +447,7 @@ pub(super) fn reset_copying_nursery_runtime_test_state() { } impl CopyingNurseryTestGuard { - pub(super) fn new(slot_count: u32) -> Self { + pub(crate) fn new(slot_count: u32) -> Self { let lock = copying_nursery_isolation_lock(); let scanner_guard = ScopedRootScannerRegistryGuard::new(); reset_copying_nursery_runtime_test_state(); @@ -475,14 +475,14 @@ impl Drop for CopyingNurseryTestGuard { } } -pub(super) struct GcTriggerThresholdTestGuard { +pub(crate) struct GcTriggerThresholdTestGuard { next_arena_trigger: usize, next_malloc_trigger: usize, malloc_step: usize, } impl GcTriggerThresholdTestGuard { - pub(super) fn suppress_automatic_triggers() -> Self { + pub(crate) fn suppress_automatic_triggers() -> Self { let next_arena_trigger = GC_NEXT_TRIGGER_BYTES.with(|trigger| { let previous = trigger.get(); trigger.set(usize::MAX); diff --git a/crates/perry-runtime/src/value/addr_class.rs b/crates/perry-runtime/src/value/addr_class.rs index afdea46ec8..da3a83a575 100644 --- a/crates/perry-runtime/src/value/addr_class.rs +++ b/crates/perry-runtime/src/value/addr_class.rs @@ -218,6 +218,80 @@ pub(crate) unsafe fn try_read_gc_header(addr: usize) -> Option<&'static GcHeader Some(&*((addr - GC_HEADER_SIZE) as *const GcHeader)) } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum TrackedGcStorage { + Arena, + Malloc, +} + +/// Classify a candidate user address without dereferencing it. +/// +/// The injected lookups keep the safety policy independently testable: a +/// low-address arena range must win on allocator membership, while every +/// handle-band value must be rejected before either lookup runs. +#[inline] +fn classify_tracked_gc_header_with( + addr: usize, + arena_range_base: impl FnOnce(usize) -> Option, + malloc_header_is_tracked: impl FnOnce(*const GcHeader) -> bool, +) -> Option<(usize, TrackedGcStorage)> { + if is_handle_band(addr) { + return None; + } + let header_addr = addr.checked_sub(GC_HEADER_SIZE)?; + if let Some(range_base) = arena_range_base(addr) { + // The payload and its header must belong to the same registered + // range. This prevents a candidate at the first bytes of a mapped + // range from back-reading the preceding page (#7742). + return (header_addr >= range_base).then_some((header_addr, TrackedGcStorage::Arena)); + } + malloc_header_is_tracked(header_addr as *const GcHeader) + .then_some((header_addr, TrackedGcStorage::Malloc)) +} + +/// Locate a `GcHeader` only after allocator-owned metadata proves that `addr` +/// is a Perry GC allocation. Unlike [`try_read_gc_header`], this does not use +/// an address-magnitude window as evidence of ownership: arena page membership +/// or an exact malloc-registry hit is required before the first header byte is +/// touched. The header's type, size, and arena flag are then validated. +/// +/// This is the canonical gate for code that must distinguish live Perry GC +/// allocations from registry handles, synthetic pointers, and unrelated +/// allocations before dereferencing a header. +/// +/// # Safety +/// The returned pointer is valid only while the allocation remains live and on +/// the current runtime thread. Callers must not dereference it after an +/// allocation or collection safepoint. Returning a raw pointer is deliberate: +/// some checked callers install forwarding metadata, so this gate must not +/// manufacture a shared reference and then write through a cast of it. +#[inline] +pub(crate) unsafe fn try_read_tracked_gc_header( + addr: usize, +) -> Option> { + let (header_addr, storage) = classify_tracked_gc_header_with( + addr, + |candidate| crate::arena::classify_heap_space_in_range(candidate).map(|(_, base)| base), + crate::gc::gc_malloc_header_is_tracked, + )?; + if header_addr % std::mem::align_of::() != 0 { + return None; + } + let header = std::ptr::NonNull::new(header_addr as *mut GcHeader)?; + let header_ptr = header.as_ptr(); + if crate::gc::gc_type_info((*header_ptr).obj_type).is_none() { + return None; + } + if ((*header_ptr).size as usize) < GC_HEADER_SIZE { + return None; + } + let header_is_arena = (*header_ptr).gc_flags & crate::gc::GC_FLAG_ARENA != 0; + if header_is_arena != matches!(storage, TrackedGcStorage::Arena) { + return None; + } + Some(header) +} + #[cfg(test)] mod tests { use super::*; @@ -264,6 +338,79 @@ mod tests { assert!(unsafe { try_read_gc_header(0x7FFD_0000_0000_0000) }.is_none()); } + #[test] + fn tracked_gc_classifier_accepts_injected_low_arena_membership() { + use std::cell::Cell; + + // 4 GiB is well below the legacy 2 TiB macOS floor. Membership in a + // registered arena range, not this magnitude, is the ownership proof. + // The array install-path test injects this candidate because mapping a + // fixed low address in the test process would be platform-dependent. + const LOW_USER: usize = 0x1_0000_0008; + const LOW_RANGE_BASE: usize = LOW_USER - GC_HEADER_SIZE; + const { assert!(LOW_USER < 0x200_0000_0000) }; + let malloc_lookup_ran = Cell::new(false); + assert_eq!( + classify_tracked_gc_header_with( + LOW_USER, + |_| Some(LOW_RANGE_BASE), + |_| { + malloc_lookup_ran.set(true); + false + }, + ), + Some((LOW_RANGE_BASE, TrackedGcStorage::Arena)) + ); + assert!(!malloc_lookup_ran.get()); + } + + #[test] + fn tracked_gc_classifier_rejects_handle_boundaries_before_lookup() { + for addr in [0, 1, COMMON_HANDLE_BAND_END, HANDLE_BAND_MAX - 1] { + assert_eq!( + classify_tracked_gc_header_with( + addr, + |_| panic!("handle must not reach the arena classifier"), + |_| panic!("handle must not reach the malloc registry"), + ), + None + ); + } + + // The first address outside the handle band is still rejected when + // neither allocator owns it, without a header dereference. + assert_eq!( + classify_tracked_gc_header_with(HANDLE_BAND_MAX, |_| None, |_| false), + None + ); + } + + #[test] + fn try_read_tracked_gc_header_rejects_unrelated_allocation() { + #[repr(C)] + struct SyntheticAllocation { + header: GcHeader, + payload: u64, + } + + let make_synthetic = || SyntheticAllocation { + header: GcHeader { + obj_type: crate::gc::GC_TYPE_ARRAY, + gc_flags: 0, + _reserved: 0, + size: std::mem::size_of::() as u32, + }, + payload: 0, + }; + let stack_synthetic = make_synthetic(); + let stack_user = &stack_synthetic.payload as *const u64 as usize; + assert!(unsafe { try_read_tracked_gc_header(stack_user) }.is_none()); + + let heap_synthetic = Box::new(make_synthetic()); + let heap_user = &heap_synthetic.payload as *const u64 as usize; + assert!(unsafe { try_read_tracked_gc_header(heap_user) }.is_none()); + } + #[test] fn stream_id_band_is_above_pointer_handles() { assert!(is_stream_id_band(STREAM_ID_BAND_START));