From 6cf0613bc36b34afa1ff1be6d0da88ba5c683c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 14 Aug 2026 02:24:52 +0200 Subject: [PATCH 1/3] fix(array): route collection reads before array validation --- .../src/array/collection_tag_tests.rs | 8 ++ crates/perry-runtime/src/array/indexing.rs | 105 +++++++++--------- 2 files changed, 60 insertions(+), 53 deletions(-) diff --git a/crates/perry-runtime/src/array/collection_tag_tests.rs b/crates/perry-runtime/src/array/collection_tag_tests.rs index dc62f7e5a0..b14bf68e99 100644 --- a/crates/perry-runtime/src/array/collection_tag_tests.rs +++ b/crates/perry-runtime/src/array/collection_tag_tests.rs @@ -114,6 +114,10 @@ fn a_live_set_receiver_still_reads_its_elements_through_the_registry() { ); let as_array = set as *const ArrayHeader; + assert!( + clean_arr_ptr(as_array).is_null(), + "#8041's array-only funnel must keep rejecting Set layout" + ); let before = probes(); assert_eq!(js_array_length(as_array), 3); assert_eq!(js_array_get_f64(as_array, 0), 5.0); @@ -140,6 +144,10 @@ fn a_live_map_receiver_still_reports_its_size_through_the_registry() { ); let as_array = map as *const ArrayHeader; + assert!( + clean_arr_ptr(as_array).is_null(), + "#8041's array-only funnel must keep rejecting Map layout" + ); let before = probes(); assert_eq!(js_array_length(as_array), 2); assert_eq!(js_array_get_f64(as_array, 0), 1.0, "entry 0's key"); diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index d792385b76..fd3f726a39 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -705,6 +705,8 @@ pub extern "C" fn js_array_numeric_get_f64_unboxed(arr: *mut ArrayHeader, index: /// Get an element from an array by index (returns f64) #[no_mangle] pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { + const TAG_UNDEFINED_F64: f64 = f64::from_bits(0x7FFC_0000_0000_0001u64); + // Issue #179 Phase 5: lazy fast path — must run BEFORE // `clean_arr_ptr` because that helper force-materializes a lazy // pointer into a regular ArrayHeader. For the common read-only @@ -718,17 +720,19 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { // around so the cache persists across calls. Strip the NaN-box // tag manually and check obj_type without going through the // clean-and-validate helper. - unsafe { + let raw_ptr = { let bits = arr as u64; let top16 = bits >> 48; - let raw_ptr = if top16 >= 0x7FF8 { + if top16 >= 0x7FF8 { if top16 == 0x7FFC { return f64::NAN; } (bits & 0x0000_FFFF_FFFF_FFFF) as *const ArrayHeader } else { arr - }; + } + }; + unsafe { if !raw_ptr.is_null() && (raw_ptr as usize) >= crate::gc::GC_HEADER_SIZE + 0x1000 { let gc_header = (raw_ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; @@ -741,6 +745,43 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { } } } + + // #7765: one `GcHeader` read gates both collection registry probes and, + // after the array-only funnel, supplies the descriptor flags which + // `array_object_flags` used to re-derive through another clean/header read. + // A header-less Buffer/TypedArray can expose allocator bookkeeping here, + // but a coincidental collection tag is harmless: the authoritative + // registry answers false, and those receivers are routed below. + // + // #8060: #8041 correctly made `clean_arr_ptr` reject every tracked + // non-array. Map/Set indexed reads are an intentional array-like dispatch, + // though, so classify them before that strict array-only funnel — matching + // `js_array_length`. The managed-header tag only selects which authority to + // ask; the registry remains the liveness/layout proof. + let receiver_tag = array_receiver_gc_tag(raw_ptr); + if receiver_tag.0 == crate::gc::GC_TYPE_SET && crate::set::is_registered_set(raw_ptr as usize) { + let set = raw_ptr as *const crate::set::SetHeader; + unsafe { + let size = (*set).size; + if index >= size { + return TAG_UNDEFINED_F64; + } + let elements = (*set).elements as *const f64; + return std::ptr::read(elements.add(index as usize)); + } + } + if receiver_tag.0 == crate::gc::GC_TYPE_MAP && crate::map::is_registered_map(raw_ptr as usize) { + let map = raw_ptr as *const crate::map::MapHeader; + unsafe { + let size = (*map).size; + if index >= size { + return TAG_UNDEFINED_F64; + } + let entries = (*map).entries as *const f64; + return std::ptr::read(entries.add(index as usize * 2)); + } + } + let cleaned = clean_arr_ptr(arr); if cleaned.is_null() { // #7574: `a[i]` on a `class X extends Array` instance held in a @@ -765,55 +806,14 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { crate::buffer::js_buffer_get(arr as *const crate::buffer::BufferHeader, index as i32); return byte_val as f64; } - // #7765: ONE `GcHeader` read now gates both collection probes below and - // supplies the descriptor flags further down, which `array_object_flags` - // used to re-derive through a second `clean_arr_ptr` and a second header - // read. On `gc-handoff/apps/asyncpipe_big.ts` this call site was 76% of all - // `is_registered_set` samples and 82% of all `is_registered_map` ones — - // both registries are non-empty there, so the #7474 latch is correctly - // armed and each probe really was resolving a thread-local and hashing, on - // every element read of an ordinary array, to prove an array is not a Map. - // - // The tag answers because every registered `Map`/`Set` IS its - // `arena_alloc_gc(_, _, GC_TYPE_MAP|GC_TYPE_SET)` header (one registration - // site each), and it is ABA-proof by construction: it lives INSIDE the - // candidate bytes, so recycling the address into anything else rewrites it - // before the new pointer is handed out. That is exactly what an - // address-keyed negative memo could not offer (#7755). - // - // Correct for a header-LESS receiver too. Buffers and typed arrays are - // `std::alloc`-backed, so their preceding bytes are allocator bookkeeping — - // but both are already routed above, and whichever way those bytes read the - // outcome is unchanged: a bookkeeping byte that happens to read as - // `GC_TYPE_SET`/`GC_TYPE_MAP` still falls through to the authoritative - // registry (which answers `false`), and any other value skips a probe that - // would have answered `false` anyway. - let receiver_tag = array_receiver_gc_tag(arr); - // Check if this is a Set — read from elements pointer (not inline) - if receiver_tag.0 == crate::gc::GC_TYPE_SET && crate::set::is_registered_set(arr as usize) { - let set = arr as *const crate::set::SetHeader; - unsafe { - let size = (*set).size; - if index >= size { - return TAG_UNDEFINED_F64; - } - let elements = (*set).elements as *const f64; - return std::ptr::read(elements.add(index as usize)); - } - } - // Check if this is a Map — return entries as [key, value] pairs - if receiver_tag.0 == crate::gc::GC_TYPE_MAP && crate::map::is_registered_map(arr as usize) { - let map = arr as *const crate::map::MapHeader; - unsafe { - let size = (*map).size; - if index >= size { - return TAG_UNDEFINED_F64; - } - let entries = (*map).entries as *const f64; - // Map entries: key at index*2, return key for simple iteration - return std::ptr::read(entries.add(index as usize * 2)); - } - } + // The usual case cleans to the same address, so reuse the header tag read + // above. A forwarded Array resolves to a different address and needs its + // live head's descriptor flags. + let receiver_tag = if arr == raw_ptr { + receiver_tag + } else { + array_receiver_gc_tag(arr) + }; // #6748 grind: per-array flag, not the process-global gate (see // `array_has_own_index`) — this probe allocated two Strings on EVERY // checked element read once any descriptor existed process-wide, which @@ -833,7 +833,6 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { // JS spec: out-of-bounds array access returns `undefined`, not NaN. // This matters for destructuring defaults (`const [a, b, c = 30] = [1, 2]`) // where the `?? fallback` must see TAG_UNDEFINED, not NaN. - const TAG_UNDEFINED_F64: f64 = f64::from_bits(0x7FFC_0000_0000_0001u64); unsafe { let length = (*arr).length; if index >= length { From a2c90a12bfda16e6044f233cf93f9fefcc25c931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 14 Aug 2026 02:25:25 +0200 Subject: [PATCH 2/3] docs(changelog): note collection index routing fix --- changelog.d/8061-collection-index-before-array-clean.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/8061-collection-index-before-array-clean.md diff --git a/changelog.d/8061-collection-index-before-array-clean.md b/changelog.d/8061-collection-index-before-array-clean.md new file mode 100644 index 0000000000..3219203515 --- /dev/null +++ b/changelog.d/8061-collection-index-before-array-clean.md @@ -0,0 +1,5 @@ +### Fixed + +- Map and Set indexed reads through array-like runtime fallbacks no longer + return `NaN`: tag-selected, registry-confirmed collections are routed before + strict Array pointer validation (#8060). From ef62f0dbb7d4d32f941ed68f35beff82a2458e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 14 Aug 2026 02:28:45 +0200 Subject: [PATCH 3/3] docs(array): retain collection tag dispatch rationale --- crates/perry-runtime/src/array/indexing.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index fd3f726a39..f2e4e52ae1 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -746,9 +746,21 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 { } } - // #7765: one `GcHeader` read gates both collection registry probes and, - // after the array-only funnel, supplies the descriptor flags which - // `array_object_flags` used to re-derive through another clean/header read. + // #7765: ONE `GcHeader` read gates both collection probes and, after the + // array-only funnel, supplies the descriptor flags which + // `array_object_flags` used to re-derive through a second `clean_arr_ptr` + // and a second header read. On `gc-handoff/apps/asyncpipe_big.ts` this call + // site was 76% of all `is_registered_set` samples and 82% of all + // `is_registered_map` ones — both registries are non-empty there, so the + // #7474 latch is armed and each probe really resolves a thread-local and + // hashes on every ordinary-array element read unless this tag gates it. + // + // The tag answers because every registered `Map`/`Set` IS its + // `arena_alloc_gc(_, _, GC_TYPE_MAP|GC_TYPE_SET)` header, and it is + // ABA-proof: recycling the address into anything else rewrites the tag + // before the new pointer is handed out. That is exactly what an + // address-keyed negative memo could not offer (#7755). + // // A header-less Buffer/TypedArray can expose allocator bookkeeping here, // but a coincidental collection tag is harmless: the authoritative // registry answers false, and those receivers are routed below.