From f5d4c48aca3c2ed8512aea2c6a7195c58e7b7996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 07:48:06 +0200 Subject: [PATCH 1/2] fix(array): route a Map/Set receiver before the array-only funnel in the fused forEach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codegen fuses a 1-argument `.forEach(cb)` to the ARRAY entry point `js_array_forEach` whenever it cannot prove the receiver is a collection — `obj.someSet.forEach(cb)` is the ordinary shape, and it is the shape react-server-dom uses for `request.abortableTasks`. #5989 put a Set/Map reroute inside that helper, but placed it AFTER `normalize_array_receiver` and its `if arr.is_null() { return; }` early-out. #8041 then widened `clean_arr_ptr` — the funnel `normalize_array_receiver` ends in — from "reject GC_TYPE_OBJECT / GC_TYPE_CLOSURE" to "reject every tracked non-array". That is correct for the array-layout question, but it nulls a GC_TYPE_SET / GC_TYPE_MAP receiver, so the reroute became unreachable and every fused `set.forEach(cb)` / `map.forEach(cb)` silently iterated nothing. Not a crash: an empty result where node yields elements. Hoist the reroute into `collection_foreach_reroute`, called as the first statement of `js_array_forEach`. Gated on `array_receiver_gc_tag` (the #7765 idiom `js_array_get_f64` already uses), so an ordinary array is excluded by one already-warm header byte and never reaches a registry probe; the registry stays the liveness/layout proof. Same ordering fix #8060/#8061 applied to the indexed read and #8090/#8119/#8109/#8120 applied to the typed-array questions. The 2-argument form `.forEach(cb, thisArg)` lowers to `js_arraylike_forEach`, which already reroutes before any array validation, and was never affected — which is why only the 1-arg lines of the two gap tests were red. Fixes the two `pass -> parity_fail` entries catalogued in #8117: `test_gap_collection_foreach_member_receiver_thisarg` and `test_gap_set_map_foreach_fused_receiver`. Both reproduce standalone and are now byte-identical to node v26.5.1 with exit 0. Tests: three added to `array/collection_tag_tests.rs`, sabotage-verified twice. Restoring the pre-fix ordering fails the Set/Map cases with `left: []` — the exact production symptom — while the plain-array control stays green; deleting the receiver-tag gate fails the control on the registry probe counters (`left: (3, 3) right: (2, 2)`) while the Set/Map cases stay green. --- .../src/array/collection_tag_tests.rs | 126 ++++++++++++++++++ .../perry-runtime/src/array/iter_methods.rs | 66 ++++++--- 2 files changed, 172 insertions(+), 20 deletions(-) diff --git a/crates/perry-runtime/src/array/collection_tag_tests.rs b/crates/perry-runtime/src/array/collection_tag_tests.rs index b14bf68e99..e429f2be20 100644 --- a/crates/perry-runtime/src/array/collection_tag_tests.rs +++ b/crates/perry-runtime/src/array/collection_tag_tests.rs @@ -328,3 +328,129 @@ fn every_registered_collection_address_carries_its_own_type_tag() { assert!(crate::set::is_registered_set(set as usize)); } } + +// --------------------------------------------------------------------------- +// #8117: the fused ARRAY `forEach` entry point must still reach a Map/Set. +// +// Codegen fuses a 1-argument `.forEach(cb)` to `js_array_forEach` +// whenever it cannot prove the receiver is a collection (`obj.someSet` is the +// ordinary shape). #5989 put a Set/Map reroute inside that helper, but AFTER +// `normalize_array_receiver`. #8041 then widened `clean_arr_ptr` from "reject +// GC_TYPE_OBJECT / GC_TYPE_CLOSURE" to "reject every tracked non-array", which +// nulls a Set/Map receiver — so the reroute became unreachable and the fused +// call silently iterated nothing. +// +// These assert THE SUBJECT the way this file's #7765 tests do: the Set/Map case +// asserts the visited VALUES (an empty visit list is exactly the bug), and the +// plain-array case asserts the registry probe counters do not move, so deleting +// the tag gate fails even though the answer would stay correct. +// --------------------------------------------------------------------------- + +use crate::closure::{js_closure_alloc, ClosureHeader}; +use std::cell::RefCell; + +thread_local! { + static FOREACH_VISITS: RefCell> = const { RefCell::new(Vec::new()) }; +} + +extern "C" fn record_first_arg( + _closure: *const ClosureHeader, + value: f64, + _index: f64, + _receiver: f64, +) -> f64 { + FOREACH_VISITS.with(|v| v.borrow_mut().push(value.to_bits())); + f64::from_bits(crate::value::TAG_UNDEFINED) +} + +fn take_visits() -> Vec { + FOREACH_VISITS.with(|v| { + v.borrow_mut() + .drain(..) + .map(f64::from_bits) + .collect::>() + }) +} + +fn recording_callback() -> *const ClosureHeader { + take_visits(); + js_closure_alloc(record_first_arg as *const u8, 0) +} + +#[test] +fn the_fused_array_foreach_still_visits_a_set_receiver() { + let (_map, _set) = arm_both_registries(); + let set = js_set_alloc(4); + js_set_add(set, 10.0); + js_set_add(set, 20.0); + assert_eq!(js_set_size(set), 2); + + // The precondition that made this a regression rather than a latent gap: + // the array-only funnel refuses this receiver, so any reroute placed after + // it is dead code. + assert!( + normalize_array_receiver(set as *const ArrayHeader).is_null(), + "#8041's array-only funnel must still refuse a Set receiver — if this \ + starts passing the reroute is no longer the thing under test" + ); + + let cb = recording_callback(); + js_array_forEach(set as *const ArrayHeader, cb); + assert_eq!( + take_visits(), + vec![10.0, 20.0], + "a Set reaching the fused array forEach must run Set.prototype.forEach; \ + an EMPTY list is #8117 — the reroute ran after clean_arr_ptr nulled it" + ); +} + +#[test] +fn the_fused_array_foreach_still_visits_a_map_receiver() { + let (_map, _set) = arm_both_registries(); + let map = js_map_alloc(4); + js_map_set(map, 1.0, 100.0); + js_map_set(map, 2.0, 200.0); + assert_eq!(js_map_size(map), 2); + + assert!( + normalize_array_receiver(map as *const ArrayHeader).is_null(), + "#8041's array-only funnel must still refuse a Map receiver" + ); + + let cb = recording_callback(); + js_array_forEach(map as *const ArrayHeader, cb); + // Map.prototype.forEach passes (value, key, map) — the first argument is + // the VALUE. + assert_eq!( + take_visits(), + vec![100.0, 200.0], + "a Map reaching the fused array forEach must run Map.prototype.forEach" + ); +} + +#[test] +fn a_plain_array_foreach_iterates_without_probing_the_collection_registries() { + let (_map, _set) = arm_both_registries(); + let arr = dense(&[1.0, 2.0, 3.0]); + let cb = recording_callback(); + + // Prime anything lazily built on first touch, then measure. + js_array_forEach(arr, cb); + let _ = take_visits(); + + let before = probes(); + js_array_forEach(arr, cb); + let after = probes(); + + assert_eq!( + take_visits(), + vec![1.0, 2.0, 3.0], + "the control receiver must keep iterating its own elements" + ); + assert_eq!( + after, before, + "a GC_TYPE_ARRAY receiver must never reach is_registered_map / \ + is_registered_set — delete the receiver-tag gate in \ + collection_foreach_reroute and this is what fails" + ); +} diff --git a/crates/perry-runtime/src/array/iter_methods.rs b/crates/perry-runtime/src/array/iter_methods.rs index 944a897a68..cf4c97ae93 100644 --- a/crates/perry-runtime/src/array/iter_methods.rs +++ b/crates/perry-runtime/src/array/iter_methods.rs @@ -103,10 +103,56 @@ impl Drop for DenseThisGuard { } } +/// #5989/#8117: `.forEach` on a receiver codegen could not prove is a +/// collection is statically fused to the ARRAY entry point below, so a native +/// `Set`/`Map` arrives there. Run the collection's own `forEach` and report +/// `true`; a genuine array-like receiver reports `false` and falls through. +/// +/// This MUST run before `normalize_array_receiver`. #8041 made `clean_arr_ptr` +/// — which `normalize_array_receiver` funnels into — reject every *tracked +/// non-array*, where it previously rejected only `GC_TYPE_OBJECT` / +/// `GC_TYPE_CLOSURE`. That is correct for the array layout question, but it +/// nulls a `GC_TYPE_SET` / `GC_TYPE_MAP` receiver, and #5989's reroute sat +/// AFTER the normalize call, behind `if arr.is_null() { return; }`. The reroute +/// therefore became unreachable and every fused `set.forEach(cb)` / +/// `map.forEach(cb)` silently iterated nothing. Same ordering fix #8060 applied +/// to the indexed read and #8090/#8119 applied to the typed-array question. +/// +/// Tag-gated exactly as `js_array_get_f64` is (#7765): every registered +/// `Map`/`Set` IS its `arena_alloc_gc(_, _, GC_TYPE_MAP|GC_TYPE_SET)` header, +/// so an ordinary array is excluded by one already-warm header byte and never +/// reaches a registry probe. The registry remains the liveness/layout proof. +#[inline] +fn collection_foreach_reroute(arr: *const ArrayHeader, callback: *const ClosureHeader) -> bool { + let addr = crate::array::array_receiver_addr(arr as *mut ArrayHeader); + let tag = crate::array::array_receiver_gc_tag(addr as *const ArrayHeader).0; + if tag != crate::gc::GC_TYPE_SET && tag != crate::gc::GC_TYPE_MAP { + return false; + } + let cb_value = f64::from_bits(crate::value::JSValue::pointer(callback as *const u8).bits()); + let undef = undefined_value(); + if tag == crate::gc::GC_TYPE_SET && crate::set::is_registered_set(addr) { + crate::set::js_set_foreach(addr as *mut crate::set::SetHeader, cb_value, undef); + return true; + } + if tag == crate::gc::GC_TYPE_MAP && crate::map::is_registered_map(addr) { + crate::map::js_map_foreach(addr as *mut crate::map::MapHeader, cb_value, undef); + return true; + } + false +} + /// forEach - call callback(element, index) for each element /// Returns nothing (void) #[no_mangle] pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const ClosureHeader) { + // #5989: a native Set/Map reaching this fused array entry point runs its + // own `forEach`. Ordered before `normalize_array_receiver` because that + // funnel nulls every tracked non-array (#8041) — see + // `collection_foreach_reroute`. + if collection_foreach_reroute(arr, callback) { + return; + } // #7574: `normalize_array_receiver` materializes an array-like OBJECT // receiver — a `class X extends Array` instance among them — into a fresh // dense snapshot. The spec passes the RECEIVER as the callback's 3rd @@ -130,26 +176,6 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo ); return; } - // #5989: `.forEach` on an unknown-typed receiver is statically fused to - // this array entry point, but the receiver may be a native Set/Map — - // react-server-dom iterates `request.abortableTasks` (a Set read back off - // the request object) exactly this way. Treating a SetHeader as an - // ArrayHeader feeds hash-table internals to the callback as elements and - // segfaults on the first property read. `forEach` is the ONLY method name - // the fused array methods share with Set/Map, so this single reroute — - // mirroring the typed-array reroute above — covers the hazard class. - { - let cb_value = f64::from_bits(crate::value::JSValue::pointer(callback as *const u8).bits()); - let undef = f64::from_bits(crate::value::TAG_UNDEFINED); - if crate::set::is_registered_set(arr as usize) { - crate::set::js_set_foreach(arr as *mut crate::set::SetHeader, cb_value, undef); - return; - } - if crate::map::is_registered_map(arr as usize) { - crate::map::js_map_foreach(arr as *mut crate::map::MapHeader, cb_value, undef); - return; - } - } unsafe { let length = (*arr).length; let scope = crate::gc::RuntimeHandleScope::new(); From f7bb1d0627e2280876fcd2744954e88f86d68e52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 07:49:07 +0200 Subject: [PATCH 2/2] docs(changelog): note the fused forEach collection reroute fix --- changelog.d/8130-fused-foreach-collection-reroute.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog.d/8130-fused-foreach-collection-reroute.md diff --git a/changelog.d/8130-fused-foreach-collection-reroute.md b/changelog.d/8130-fused-foreach-collection-reroute.md new file mode 100644 index 0000000000..e4477784c6 --- /dev/null +++ b/changelog.d/8130-fused-foreach-collection-reroute.md @@ -0,0 +1,11 @@ +### Fixed + +- A 1-argument `.forEach(cb)` on a `Map` or `Set` no longer iterates nothing + when codegen could not statically prove the receiver was a collection + (`obj.someSet.forEach(cb)`, react-server-dom's `request.abortableTasks`). + Codegen fuses that shape to the array entry point `js_array_forEach`, whose + #5989 collection reroute sat behind `normalize_array_receiver`; #8041 widened + `clean_arr_ptr` to reject every tracked non-array, which nulls a + `GC_TYPE_SET`/`GC_TYPE_MAP` receiver and left the reroute unreachable. The + reroute now runs first, receiver-tag gated so an ordinary array still never + reaches a registry probe (#8117).