diff --git a/changelog.d/10387-groupby-json-raw-f64-layout.md b/changelog.d/10387-groupby-json-raw-f64-layout.md new file mode 100644 index 0000000000..a606568d71 --- /dev/null +++ b/changelog.d/10387-groupby-json-raw-f64-layout.md @@ -0,0 +1,28 @@ +### Fixed + +- `JSON.stringify` of an array built by a runtime helper that writes its element + slots directly no longer emits `null` for every non-number element. + `JSON.stringify([...Map.groupBy("aba", ch => ch).entries()])` answered + `[["a",[null,null]],["b",[null]]]` instead of `[["a",["a","a"]],["b",["b"]]]`, + while `arr[0]` and `String(arr)` on the same array stayed correct — so only a + JSON round-trip could see it. + + `js_array_alloc` stamps `GC_ARRAY_RAW_F64_LAYOUT` ("every live slot is an + unboxed double") on the fresh, empty array, where it is vacuously true. A + producer such as `groupby.rs`'s `group_by_make_array` then sets `length` and + `std::ptr::write`s the element words itself, bypassing every noting store + helper that would have cleared the flag. That mislabelling was harmless until + `json::stringify_primitive_array` started taking the flag as proof and + emitting each slot through `write_number`: a NaN-boxed string read as a double + is non-finite, and JSON renders non-finite as `null`. + `test_gap_2899_2779_2777_static_helpers` went red on `main` in the window that + introduced that reader. + + `object::gc_slots::rebuild_array_layout_from_slots` — the choke point every + such producer already calls to repair the GC pointer bitmap — now re-derives + the numeric-layout flag from the same slots it just walked. The reclassify is + clear-only, so an array that really is all-numbers keeps its fast path, and no + receiver that used to decline now passes. `test_gap_2899_2779_2777_static_helpers` + gained JSON coverage for string, boolean, object, mixed and numeric group + values, plus the non-JSON readback beside it so a future failure says which + half broke. diff --git a/changelog.d/10387-iterator-prototype-next-spread-arms.md b/changelog.d/10387-iterator-prototype-next-spread-arms.md new file mode 100644 index 0000000000..1fbc500691 --- /dev/null +++ b/changelog.d/10387-iterator-prototype-next-spread-arms.md @@ -0,0 +1,52 @@ +### Fixed + +- A replaced `%ArrayIteratorPrototype%.next` (and its Map / Set / String + siblings) now drives spread, `Array.from` and call-spread, not just `for…of` + and a manual `.next()`. `[...[4, 5]]` under a patch that doubles each value + printed `4,5`; `[...new Set([1, 2])]` printed `1,2`; `[..."ab"]` printed + `a,b`; `f(...[13, 14])` passed the raw elements. + + Each of these ends in a runtime element-COPY arm — `dense_spread_source`'s + memcpy (#7533), `js_set_to_array` / `js_map_entries`, + `js_string_to_char_array`, `js_array_like_to_array`'s array fast path — that + materializes the result without ever calling `.next()`, so the per-call + "is `next` still the builtin?" proof in `object/iterator_prototypes.rs` cannot + be reached from inside them. This is the same hole #10086 closed for the + `for…of` index loop and the array-destructuring fast arm, and it is closed the + same way: the family prototype object can only be patched after it has escaped + to user code through `Object.getPrototypeOf` / `Reflect.getPrototypeOf`, so + that escape is the choke point. `note_iterator_prototype_exposed` (was + `note_array_iterator_prototype_exposed`) now recognises the Map, Set and String + family prototypes and `%IteratorPrototype%` itself as well as the array one, + and the copy arms decline on their family's signal and run the real protocol. + + The array arms move from the narrow `array_proto_iterator_modified` (the + `Symbol.iterator` slot was written) to the broader + `array_iteration_not_pristine`. The narrow fact implies the broad one, so + every receiver that declined before still declines. Nothing changes for a + program that never introspects a built-in iterator: all four signals stay + false until `Object.getPrototypeOf` hands the prototype out. + + `test_gap_iterator_prototype_next_patch` has been red on `main` since it + landed on 2026-09-06 (gap-suite shard log for `87dc334920` — the first run that + contained it — already reported `pass -> parity_fail`), so this is a + first-time fix of a fixture that over-specified the implementation, not a + regression repair. The fixture and its + `crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs` companion + gained `Array.from(array)`, call-spread, multi-operand spread, + `Array.from(set)`, `Array.from(map)`, `[...map]` and `Array.from(string)` + cases under the same patches. + +The fixture also could not pass for a reason that was not Perry's: it called +`console.log` while a built-in iterator prototype was patched. Node builds +`SafeMap` out of `internal/per_context/primordials` lazily, and the parity +harness runs the oracle under `FORCE_COLOR=0`, which is the path that defers +that construction into the patched window — so the ORACLE died with + + node:internal/per_context/primordials:449 + class SafeMap extends Map {}, + +leaving `Node exit: 1, Perry exit: 0` however the runtime behaved. Output is now +buffered inside each patched window and flushed after the prototype is restored; +every value is still computed inside the window, which is the subject, and the +emitted text is byte-identical to the unbuffered run. diff --git a/changelog.d/10387-suppressed-error-heritage.md b/changelog.d/10387-suppressed-error-heritage.md new file mode 100644 index 0000000000..abea62321c --- /dev/null +++ b/changelog.d/10387-suppressed-error-heritage.md @@ -0,0 +1,26 @@ +### Fixed + +- `new SuppressedError(...) instanceof Error` is `true` again, and + `Object.getPrototypeOf(SuppressedError.prototype) === Error.prototype` / + `Object.getPrototypeOf(SuppressedError) === Error` now hold as ECMA-262 + requires. + + `SuppressedError` was missing from `is_native_error_subclass_constructor`, so + the `globalThis` population loop never linked its prototype pair into the + Error family — `SuppressedError.prototype`'s `[[Prototype]]` stayed + `Object.prototype`. That was latent while `instanceof Error` answered from the + class registry (`extends_builtin_error(CLASS_ID_SUPPRESSED_ERROR)`, which + `js_suppressed_error_new` registers). It stopped being latent when + `js_instanceof` began consulting the instance's RECORDED prototype chain FIRST + for `class_id == CLASS_ID_ERROR`: the walk reached a chain with no + `Error.prototype` in it, returned `Some(false)`, and short-circuited the + registry fact that used to carry the answer. `test_gap_disposablestack_2875` + went red on `main` between `87dc334920` and `f207cf6618` — the window + containing that change — and stayed red. + + The fix is the missing link itself, not a special case in `instanceof`: the + recorded chain is now correct, so the walk answers `true` on its own and + `Error.prototype.toString` is inherited (`String(err)` is + `"SuppressedError: both failed"`). `test_gap_disposablestack_2875` gained + direct assertions for both prototype links, for the `TypeError` control, and + for the inherited `toString`. diff --git a/crates/perry-runtime/src/array/flat_clone.rs b/crates/perry-runtime/src/array/flat_clone.rs index 2365a2c250..9ec05b827d 100644 --- a/crates/perry-runtime/src/array/flat_clone.rs +++ b/crates/perry-runtime/src/array/flat_clone.rs @@ -50,9 +50,16 @@ unsafe fn receiver_gc_type(ptr: *const ArrayHeader) -> u8 { /// `Array.prototype` / `Object.prototype` index properties shadowing the /// dense slots, and no live indices past the dense backing store — the three /// cases where `arr[i]` is not the raw slot. -/// - `array_proto_iterator_modified`: user code replaced or deleted -/// `Array.prototype[Symbol.iterator]`, so the builtin walk is no longer what -/// a spread must run. +/// - `array_iteration_not_pristine`: array iteration can no longer be PROVEN +/// to be the pristine builtin protocol — either user code replaced or +/// deleted `Array.prototype[Symbol.iterator]`, or `%ArrayIteratorPrototype%` +/// itself escaped through `Object.getPrototypeOf` and its `next` may have +/// been replaced (#10086's escape signal; #9846). Both mean the builtin walk +/// is no longer what a spread must run. This gate used to read only the +/// narrower `array_proto_iterator_modified`, so `[...[4, 5]]` under a patched +/// `%ArrayIteratorPrototype%.next` memcpy'd the raw elements and never called +/// the patch. The narrow fact implies the broad one, so nothing that used to +/// decline now passes. /// - `object_static_prototype`: `Object.setPrototypeOf(array, custom)` can /// replace the inherited iterator without touching Array.prototype. /// - `has_own_symbol_property`: the instance carries its OWN `[Symbol.iterator]`, @@ -85,7 +92,7 @@ pub(crate) fn dense_spread_source(value: f64) -> Option<*const ArrayHeader> { if crate::array::array_iteration_is_exotic(arr) { return None; } - if crate::array::array_proto_iterator_modified() { + if crate::array::array_iteration_not_pristine() { return None; } if crate::object::prototype_chain::object_static_prototype(arr as usize).is_some() { diff --git a/crates/perry-runtime/src/array/from_concat.rs b/crates/perry-runtime/src/array/from_concat.rs index a2c5ae57b7..d61076e1c5 100644 --- a/crates/perry-runtime/src/array/from_concat.rs +++ b/crates/perry-runtime/src/array/from_concat.rs @@ -74,6 +74,32 @@ fn throw_map_fn_not_callable(map_fn: f64) -> ! { /// /// Takes the raw NaN-boxed f64 value (NOT a pre-unboxed pointer) so it can /// inspect the tag bits before stripping. +/// #9846: is `value` a String / Set / Map whose iterator prototype has escaped +/// to user code, so its `Array.from` element copy is no longer unobservable? +/// See the call site in [`js_array_from_value`]. +fn array_from_family_not_pristine(value: f64) -> bool { + // Sticky-flag loads FIRST. The classification below costs two registry + // lookups, and `Array.from` is hot: in the overwhelmingly common program no + // iterator prototype has ever escaped, so this must cost three relaxed + // loads and nothing else. + let string_dirty = crate::object::iterator_prototypes::string_iteration_not_pristine(); + let set_dirty = crate::object::iterator_prototypes::set_iteration_not_pristine(); + let map_dirty = crate::object::iterator_prototypes::map_iteration_not_pristine(); + if !(string_dirty || set_dirty || map_dirty) { + return false; + } + let jsv = crate::value::JSValue::from_bits(value.to_bits()); + if jsv.is_any_string() { + return string_dirty; + } + let raw = crate::value::js_nanbox_get_pointer(value) as usize; + if raw == 0 { + return false; + } + (set_dirty && crate::set::is_registered_set(raw)) + || (map_dirty && crate::map::is_registered_map(raw)) +} + #[no_mangle] pub extern "C" fn js_array_from_value(boxed: f64) -> *mut ArrayHeader { let bits = boxed.to_bits(); @@ -91,16 +117,31 @@ pub extern "C" fn js_array_from_value(boxed: f64) -> *mut ArrayHeader { // `js_array_clone` then behaves exactly as it does for a literal. // #7542: `Array.from(arr)` is `GetIterator(arr)` + drain, so a patched // `Array.prototype[Symbol.iterator]` drives it exactly as it drives spread. + // #9846 widened the condition from the `Symbol.iterator` write alone to the + // whole "array iteration is no longer provably pristine" fact, so a patched + // `%ArrayIteratorPrototype%.next` drives `Array.from(arr)` too. // This function's array arm ends in a raw `js_array_clone` — a shallow // element copy that never consults the protocol — so the guard has to be // here rather than downstream. `array_from_spread_value` already routes the // patched case through `js_get_iterator`; reuse it so the two entry points // cannot answer differently for the same receiver. - if crate::array::array_proto_iterator_modified() + if crate::array::array_iteration_not_pristine() && crate::array::js_array_is_array(boxed).to_bits() == crate::value::TAG_TRUE { return crate::array::js_array_clone_for_spread(boxed); } + // #9846: the Map / Set / String families have the same hole. Their arms end + // in `js_array_clone`'s element copies (the Set backing, the Map entry + // pairs, the string's codepoint cut), none of which calls `.next()`, so a + // patched `%SetIteratorPrototype%.next` was invisible to `Array.from(set)` + // exactly as it was to `[...set]`. `array_from_spread_value` is the one + // implementation that declines those copies on the escape signal; delegate + // to it rather than restate the decision here. Every receiver this admits + // is iterable, so the delegation cannot turn an accepted `Array.from` into + // the "not iterable" throw. + if array_from_family_not_pristine(boxed) { + return crate::array::js_array_clone_for_spread(boxed); + } let jsval = crate::value::JSValue::from_bits(bits); if jsval.is_short_string() { diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 5c4ca69e2d..c388728b2a 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -1297,6 +1297,33 @@ pub(crate) unsafe fn clear_array_numeric_layout(arr: *const ArrayHeader) { clear_array_raw_f64_layout_flag(arr); } +/// Re-derive the raw-f64 numeric layout flag from the slots that are actually +/// there, CLEARING it when any live slot is not a number. Never sets it. +/// +/// `js_array_alloc` stamps `GC_ARRAY_RAW_F64_LAYOUT` on the fresh (length 0) +/// array, where it is vacuously true. A producer that fills the array through +/// the noting store helpers keeps the flag honest; one that sets `length` and +/// `std::ptr::write`s the element words directly — the documented "internal +/// scratch array" shape called out on [`mark_array_raw_f64_holes_fresh`] — +/// bypasses every clear and leaves a NaN-boxed pointer sitting in a slot the +/// flag promises is a plain double. +/// +/// That used to be merely latent. `json::stringify_primitive_array` (#9849) +/// now takes the flag as proof and emits every slot through `write_number`, so +/// a mislabelled array serializes its strings as `null` +/// (`JSON.stringify([...Map.groupBy("aba", ch => ch).entries()])`). This is the +/// repair for the choke point those producers already call, +/// `object::gc_slots::rebuild_array_layout_from_slots`. +#[inline] +pub(crate) unsafe fn reclassify_array_numeric_layout_from_slots(arr: *mut ArrayHeader) { + if arr.is_null() || !array_has_raw_f64_layout_flag(arr) { + return; + } + if !array_slots_are_numeric(arr) { + clear_array_numeric_layout(arr); + } +} + #[inline] pub(crate) fn clear_array_numeric_layout_ptr(user_ptr: usize) { if user_ptr == 0 { diff --git a/crates/perry-runtime/src/array/indexing_support.rs b/crates/perry-runtime/src/array/indexing_support.rs index b9a94f8453..ecbd291f8b 100644 --- a/crates/perry-runtime/src/array/indexing_support.rs +++ b/crates/perry-runtime/src/array/indexing_support.rs @@ -162,7 +162,7 @@ pub(super) static ARRAY_PROTO_ITERATOR_MODIFIED: AtomicBool = AtomicBool::new(fa /// HIR lowering, which never consults the iteration protocol, so such a /// patch was ignored there even after the spread paths were fixed (#7542). /// * The array-iterator PROTOTYPE object was handed to user code (#10086; see -/// `object::iterator_prototypes::note_array_iterator_prototype_exposed`). +/// `object::iterator_prototypes::note_iterator_prototype_exposed`). /// A replaced `%ArrayIteratorPrototype%.next` is detected per `.next()` call /// (`prototype_next_is_canonical`), which a fast arm that never calls /// `.next()` cannot observe — and the only way to reach that object in order @@ -172,10 +172,15 @@ pub(super) static ARRAY_PROTO_ITERATOR_MODIFIED: AtomicBool = AtomicBool::new(fa /// introspect array iterators, while under-approximating would silently /// return unpatched elements. /// -/// Both consumers — the `for…of` index loop and #10086's array-destructuring -/// arm — branch on it ONCE, which is also what the spec wants: iteration -/// performs GetIterator exactly once, so a patch landing mid-loop must not -/// change the iterator already in hand. +/// The two GENERATED consumers — the `for…of` index loop and #10086's +/// array-destructuring arm — branch on it ONCE, which is also what the spec +/// wants: iteration performs GetIterator exactly once, so a patch landing +/// mid-loop must not change the iterator already in hand. +/// +/// #9846 added a third, RUNTIME-side consumer through +/// [`array_iteration_not_pristine`]: `dense_spread_source`'s element copy and +/// `array_from_spread_value`'s `Array.prototype[Symbol.iterator]` delegation, +/// which are the spread / `Array.from` equivalents of the same hole. /// /// A separate `u8` global rather than exposing the `AtomicBool`: codegen emits /// a plain volatile `i8` load, the same shape as @@ -184,9 +189,12 @@ pub(super) static ARRAY_PROTO_ITERATOR_MODIFIED: AtomicBool = AtomicBool::new(fa /// itself is emitted byte-identically to before. /// /// NOTE the asymmetry with [`ARRAY_PROTO_ITERATOR_MODIFIED`]: that bool keeps -/// its exact original meaning (the `Symbol.iterator` slot was written) and still -/// gates the Rust-side spread / `js_get_iterator` delegation. Only this byte -/// carries the broader "not provably pristine" fact. +/// its exact original meaning (the `Symbol.iterator` slot was written). This +/// byte carries the broader "not provably pristine" fact, and since #9846 it +/// is what the Rust-side spread / `js_get_iterator` delegation gates on too — +/// the narrow bool implies it (`note_array_proto_iterator_write` sets both), +/// so widening those call sites strictly grows the set of receivers that take +/// the real protocol. #[no_mangle] pub static PERRY_ARRAY_ITERATION_NOT_PRISTINE: AtomicU8 = AtomicU8::new(0); @@ -198,6 +206,13 @@ pub(crate) fn note_array_iteration_not_pristine() { PERRY_ARRAY_ITERATION_NOT_PRISTINE.store(1, Ordering::Release); } +/// Rust-side reader for [`PERRY_ARRAY_ITERATION_NOT_PRISTINE`]. Acquire-ordered +/// to pair with `note_array_iteration_not_pristine`'s release. +#[inline] +pub(crate) fn array_iteration_not_pristine() -> bool { + PERRY_ARRAY_ITERATION_NOT_PRISTINE.load(Ordering::Acquire) != 0 +} + /// Record (if `obj` is `Array.prototype` and `sym_key` is the well-known /// `Symbol.iterator`) that the array iteration protocol has been tampered /// with. Called from the symbol-property set/delete paths. diff --git a/crates/perry-runtime/src/array/iterator.rs b/crates/perry-runtime/src/array/iterator.rs index 79ed386779..78ea691b39 100644 --- a/crates/perry-runtime/src/array/iterator.rs +++ b/crates/perry-runtime/src/array/iterator.rs @@ -966,6 +966,14 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { throw_not_iterable(value()); } if jsv.is_any_string() { + // #9846: `[..."ab"]` is `GetIterator(str)` + drain per spec, and + // `js_string_to_char_array` is an element cut that never calls + // `%StringIteratorPrototype%.next`. That is unobservable — until the + // prototype object escapes to user code, after which the cut would + // silently ignore a patched `next`. Decline then, and run the protocol. + if crate::object::iterator_prototypes::string_iteration_not_pristine() { + return js_iterator_to_array(crate::symbol::js_get_iterator(value())); + } let str_ptr = crate::value::js_get_string_pointer_unified(value()); let str_bits = crate::value::STRING_TAG | (str_ptr as u64 & POINTER_MASK); return crate::string::js_string_to_char_array(str_bits as i64) as *mut ArrayHeader; @@ -1038,7 +1046,7 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { // not preempt the walk below for that case: `js_get_iterator`'s patched // branch reads the PROTOTYPE only, and would throw "not iterable" for an // array carrying its own method once the prototype slot has been deleted. - if crate::array::array_proto_iterator_modified() + if crate::array::array_iteration_not_pristine() && crate::array::js_array_is_array(value()).to_bits() == crate::value::TAG_TRUE && !array_has_own_iterator(value()) { @@ -1051,10 +1059,21 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { if crate::buffer::is_registered_buffer(raw_ptr()) { return crate::buffer::buffer_to_array(raw_ptr() as *const crate::buffer::BufferHeader); } + // #9846: the Set / Map arms below copy the backing store instead of + // driving `%SetIteratorPrototype%.next` / `%MapIteratorPrototype%.next`. + // Same trade as the array dense arm and the string cut above: free while + // the family prototype has never escaped to user code, wrong the moment it + // has, so decline on the escape signal and run the real protocol. if crate::set::is_registered_set(raw_ptr()) { + if crate::object::iterator_prototypes::set_iteration_not_pristine() { + return js_iterator_to_array(crate::symbol::js_get_iterator(value())); + } return crate::set::js_set_to_array(raw_ptr() as *const crate::set::SetHeader); } if crate::map::is_registered_map(raw_ptr()) { + if crate::object::iterator_prototypes::map_iteration_not_pristine() { + return js_iterator_to_array(crate::symbol::js_get_iterator(value())); + } return crate::map::js_map_entries(raw_ptr() as *const crate::map::MapHeader); } // `class X extends Map | Set` instance — spread (`[...container]`, diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 6141572233..2af3af130c 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -176,7 +176,7 @@ pub(crate) use self::indexing::{ #[cfg(test)] pub(crate) use self::indexing_support::test_keys_array_slot_fallbacks; pub(crate) use self::indexing_support::{ - array_proto_iterator_modified, invalidate_array_index_fast_path, + array_iteration_not_pristine, array_proto_iterator_modified, invalidate_array_index_fast_path, keys_array_len_capped_to_capacity, keys_array_slot, note_array_index_write, note_array_iteration_not_pristine, note_array_proto_iterator_write, note_object_prototype_index_write, object_prototype_has_index_flag, @@ -289,9 +289,10 @@ pub(crate) use self::header::{ gc_element_slot_range, mark_array_layout_unknown, mark_array_raw_f64_holes_fresh, normalize_array_receiver, note_array_slot, note_array_slot_layout_only, note_array_slot_resolved_flags, rebuild_array_layout, rebuild_array_layout_exact, - refresh_array_numeric_layout, replay_array_growth_write_barriers, set_array_numeric_layout, - store_array_slot, store_array_slot_resolved, transfer_array_numeric_layout, - typed_array_receiver, value_bits_to_number, NumericArrayLayout, MIN_ARRAY_CAPACITY, + reclassify_array_numeric_layout_from_slots, refresh_array_numeric_layout, + replay_array_growth_write_barriers, set_array_numeric_layout, store_array_slot, + store_array_slot_resolved, transfer_array_numeric_layout, typed_array_receiver, + value_bits_to_number, NumericArrayLayout, MIN_ARRAY_CAPACITY, }; pub(crate) use self::named_props::{ array_has_named_properties_resolved, array_has_sparse_index_properties_resolved, diff --git a/crates/perry-runtime/src/object/arguments.rs b/crates/perry-runtime/src/object/arguments.rs index 41cf23d17f..5d8992e36c 100644 --- a/crates/perry-runtime/src/object/arguments.rs +++ b/crates/perry-runtime/src/object/arguments.rs @@ -713,11 +713,13 @@ pub extern "C" fn js_array_like_to_array(value: f64) -> *mut ArrayHeader { // #7542: unless `Array.prototype[Symbol.iterator]` was replaced, in // which case call-spread (`f(...arr)`) must drive the patched method — // it decides how many arguments the callee receives, so `f(...[1,2,3])` - // passed 3 where node passes 1. `array_proto_iterator_modified()` is a - // sticky flag that is false until user code writes the prototype slot, - // so the fast path is untouched in every ordinary program. + // passed 3 where node passes 1. #9846 widened the condition to the + // whole `array_iteration_not_pristine` fact, so a replaced + // `%ArrayIteratorPrototype%.next` drives `f(...arr)` as well; both are + // sticky flags that stay false until user code touches the prototype + // tower, so the fast path is untouched in every ordinary program. if crate::array::js_array_is_array(value).to_bits() == crate::value::TAG_TRUE { - if crate::array::array_proto_iterator_modified() + if crate::array::array_iteration_not_pristine() || crate::array::array_ptr_as_proxy(raw as *const ArrayHeader).is_some() { return crate::array::js_array_clone_for_spread(value); diff --git a/crates/perry-runtime/src/object/gc_slots.rs b/crates/perry-runtime/src/object/gc_slots.rs index 4dd728aeef..dc71be5988 100644 --- a/crates/perry-runtime/src/object/gc_slots.rs +++ b/crates/perry-runtime/src/object/gc_slots.rs @@ -86,6 +86,15 @@ pub(crate) unsafe fn rebuild_array_layout_from_slots(arr: *mut ArrayHeader) { let len = (*arr).length as usize; let slots = crate::array::array_elements_ptr(arr as *const ArrayHeader) as *mut u64; crate::gc::layout_rebuild_from_slots(arr as *mut u8, slots, len); + // The GC pointer bitmap is not the only per-array fact a direct slot write + // invalidates. Every caller here reached this function because it set + // `length` and `std::ptr::write`-d the element words itself, bypassing the + // noting store helpers — and `js_array_alloc` births the array carrying + // `GC_ARRAY_RAW_F64_LAYOUT` (vacuously true at length 0). Re-derive that + // flag from the same slots this rebuild just walked, or a NaN-boxed pointer + // sits in a slot the flag promises is a plain double. Clear-only, so an + // array that really is all-numbers keeps its fast path. + crate::array::reclassify_array_numeric_layout_from_slots(arr); if crate::arena::pointer_in_old_gen(arr as usize) { for i in 0..len { let slot = slots.add(i); diff --git a/crates/perry-runtime/src/object/global_this/array_error.rs b/crates/perry-runtime/src/object/global_this/array_error.rs index b88cea18bf..b035c5934e 100644 --- a/crates/perry-runtime/src/object/global_this/array_error.rs +++ b/crates/perry-runtime/src/object/global_this/array_error.rs @@ -213,6 +213,17 @@ pub(crate) extern "C" fn object_prototype_is_prototype_of_thunk( /// #4533: native error subclass constructors whose `[[Prototype]]` is `Error` /// (their `.prototype.[[Prototype]]` already links to `Error.prototype`). +/// +/// `SuppressedError` belongs here for the same reason the NativeErrors do — +/// ECMA-262 (explicit resource management) gives `%SuppressedError%` the +/// `[[Prototype]]` `%Error%` and `%SuppressedError.prototype%` the +/// `[[Prototype]]` `%Error.prototype%`. Omitting it did not merely lose +/// `Object.getPrototypeOf(SuppressedError.prototype) === Error.prototype`: once +/// `js_instanceof` started answering `x instanceof Error` from the instance's +/// RECORDED prototype chain (#9940), the missing link made that walk answer +/// `false` and short-circuit the `extends_builtin_error(CLASS_ID_SUPPRESSED_ERROR)` +/// registry fact that used to carry it, so `new SuppressedError(…) instanceof +/// Error` regressed to `false`. pub(crate) fn is_native_error_subclass_constructor(name: &str) -> bool { matches!( name, @@ -223,6 +234,7 @@ pub(crate) fn is_native_error_subclass_constructor(name: &str) -> bool { | "EvalError" | "URIError" | "AggregateError" + | "SuppressedError" ) } diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index e13891f145..0afd4ca9b7 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -36,7 +36,7 @@ use super::{ install_proto_method, js_object_alloc, set_builtin_property_attrs, ObjectHeader, PropertyAttrs, }; use crate::value::JSValue; -use std::sync::atomic::{AtomicI64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicI64, Ordering}; // GC-rooted singleton slots. Each realm builds its own tower in its own arena; // the process-global handles resolve to per-agent atomics and are scanned in @@ -66,9 +66,50 @@ pub(crate) static REGEXP_STRING_ITERATOR_PROTOTYPE_PTR: super::RealmAtomicI64 = pub(crate) static ITERATOR_HELPER_PROTOTYPE_PTR: super::RealmAtomicI64 = super::RealmAtomicI64::new(&ITERATOR_HELPER_PROTOTYPE_PTR_SLOT); -/// #10086: the array-iterator prototype object is about to be handed to user -/// code, so `%ArrayIteratorPrototype%.next` may be replaced at any point after -/// this. Publish that to generated code. +/// Sticky per-family siblings of `PERRY_ARRAY_ITERATION_NOT_PRISTINE`: the +/// Map / Set / String iterator PROTOTYPE object escaped to user code, so +/// `%MapIteratorPrototype%.next` (etc.) may be patched from here on. +/// +/// The array byte is exported because GENERATED code reads it (the `for…of` +/// index loop, #10086's destructuring arm). These three are read only from +/// Rust — `array_from_spread_value`'s Map / Set / string element-copy arms — +/// so a plain `AtomicBool` is enough. `AtomicBool` holds no heap pointer, so +/// none of them is a GC root (`scripts/gc_runtime_root_holders.py`). +static MAP_ITERATION_NOT_PRISTINE: AtomicBool = AtomicBool::new(false); +static SET_ITERATION_NOT_PRISTINE: AtomicBool = AtomicBool::new(false); +static STRING_ITERATION_NOT_PRISTINE: AtomicBool = AtomicBool::new(false); + +/// Has `%MapIteratorPrototype%` escaped to user code? See +/// [`note_iterator_prototype_exposed`]. +#[inline] +pub(crate) fn map_iteration_not_pristine() -> bool { + MAP_ITERATION_NOT_PRISTINE.load(Ordering::Acquire) +} + +/// Has `%SetIteratorPrototype%` escaped to user code? See +/// [`note_iterator_prototype_exposed`]. +#[inline] +pub(crate) fn set_iteration_not_pristine() -> bool { + SET_ITERATION_NOT_PRISTINE.load(Ordering::Acquire) +} + +/// Has `%StringIteratorPrototype%` escaped to user code? See +/// [`note_iterator_prototype_exposed`]. +#[inline] +pub(crate) fn string_iteration_not_pristine() -> bool { + STRING_ITERATION_NOT_PRISTINE.load(Ordering::Acquire) +} + +/// #10086: a built-in iterator prototype object is about to be handed to user +/// code, so `%ArrayIteratorPrototype%.next` (or its Map / Set / String +/// sibling) may be replaced at any point after this. Publish that. +/// +/// #9846 widened this from the array family alone. The array byte covers the +/// two GENERATED fast arms; the three booleans cover the RUNTIME element-copy +/// arms in `array_from_spread_value`, which are the same kind of hole — +/// `[...new Set([1, 2])]` memcpy'd the Set's backing and `[..."ab"]` cut the +/// string into chars, so a patched `%SetIteratorPrototype%.next` / +/// `%StringIteratorPrototype%.next` never ran. /// /// A replaced `next` is detected per `.next()` call by /// [`prototype_next_is_canonical`] — which a non-iterator fast arm (the @@ -86,7 +127,7 @@ pub(crate) static ITERATOR_HELPER_PROTOTYPE_PTR: super::RealmAtomicI64 = /// So the flag is set when the object escapes, patched or not. The cost lands /// only on programs that introspect an array iterator — and those are exactly /// the programs about to patch one. -pub(crate) fn note_array_iterator_prototype_exposed(value: f64) { +pub(crate) fn note_iterator_prototype_exposed(value: f64) { let jv = JSValue::from_bits(value.to_bits()); if !jv.is_pointer() { return; @@ -97,6 +138,29 @@ pub(crate) fn note_array_iterator_prototype_exposed(value: f64) { } if ARRAY_ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == addr { crate::array::note_array_iteration_not_pristine(); + return; + } + if MAP_ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == addr { + MAP_ITERATION_NOT_PRISTINE.store(true, Ordering::Release); + return; + } + if SET_ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == addr { + SET_ITERATION_NOT_PRISTINE.store(true, Ordering::Release); + return; + } + if STRING_ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == addr { + STRING_ITERATION_NOT_PRISTINE.store(true, Ordering::Release); + return; + } + // `%IteratorPrototype%` itself is the parent of all four families, and a + // patch there is inherited by every one of them. Reaching it needs a + // second `getPrototypeOf` hop off a family prototype, so this arm is + // strictly rarer than the four above — mark them all. + if ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == addr { + crate::array::note_array_iteration_not_pristine(); + MAP_ITERATION_NOT_PRISTINE.store(true, Ordering::Release); + SET_ITERATION_NOT_PRISTINE.store(true, Ordering::Release); + STRING_ITERATION_NOT_PRISTINE.store(true, Ordering::Release); } } diff --git a/crates/perry-runtime/src/object/object_ops/prototype.rs b/crates/perry-runtime/src/object/object_ops/prototype.rs index 2b8370f710..a5d0a0bfd6 100644 --- a/crates/perry-runtime/src/object/object_ops/prototype.rs +++ b/crates/perry-runtime/src/object/object_ops/prototype.rs @@ -192,7 +192,7 @@ pub extern "C" fn js_object_get_prototype_of(obj_value: f64) -> f64 { // patched. Publishing here is what lets the `for…of` index loop and the // array-destructuring fast arm decline a possibly-patched // `%ArrayIteratorPrototype%.next`, which neither can observe otherwise. - crate::object::iterator_prototypes::note_array_iterator_prototype_exposed(proto); + crate::object::iterator_prototypes::note_iterator_prototype_exposed(proto); proto } diff --git a/crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs b/crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs index 18bcdae86e..d51fcd48a7 100644 --- a/crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs +++ b/crates/perry/tests/issue_9846_iterator_prototype_next_patch.rs @@ -14,6 +14,15 @@ //! (`test-files/test_gap_iterator_prototype_next_patch.ts`), captured //! 2026-09-06. The discriminating lines are: //! +//! * `A-spread 8,10` / `A-from-array 22,24` / `A-call-spread 26,28` / +//! `A-multi-spread 30,32` / `D-set s1,s2` / `D-map-spread [["z",105]]` / +//! `E-string A,B` — every one of these is a runtime element-COPY fast arm +//! that materializes the result without ever calling `.next()`, so the +//! per-call proof above cannot see the patch from inside them. They decline +//! on the #10086 escape signal instead (the family prototype object reached +//! user code through `Object.getPrototypeOf`), which is the only event that +//! can precede a patch. Without that the fixture printed the raw elements — +//! `A-spread 4,5`, `D-set 1,2`, `E-string a,b`. //! * `F-bound-copy 100,200` — `orig.bind(other)` has the SAME native entry as //! the builtin thunk but a different `this`. A proof that compared by native //! entry alone, without first reading the prototype's own slot, would call @@ -39,6 +48,9 @@ const SOURCE: &str = include_str!("../../../test-files/test_gap_iterator_prototy const EXPECTED: &str = "A-forof 2,4,6\n\ A-spread 8,10\n\ A-from 12\n\ +A-from-array 22,24\n\ +A-call-spread 26,28\n\ +A-multi-spread 30,32\n\ A-manual 14 16 true\n\ B-forof 1,2,3\n\ B-spread 4,5\n\ @@ -46,10 +58,14 @@ B-manual 7 8 true\n\ C-forof-empty 0\n\ C-restored 9\n\ D-map a=101,b=102\n\ +D-map-spread [[\"z\",105]]\n\ +D-map-from [[\"y\",106]]\n\ D-map-restored a,1\n\ D-set s1,s2\n\ +D-set-from s3,s4\n\ D-set-restored 3\n\ E-string A,B\n\ +E-string-from C,D\n\ E-string-restored c,d\n\ F-same-object 1,2\n\ F-bound-copy 100,200\n\ diff --git a/test-files/test_gap_2899_2779_2777_static_helpers.ts b/test-files/test_gap_2899_2779_2777_static_helpers.ts index 1ddbc01060..1432d78b0c 100644 --- a/test-files/test_gap_2899_2779_2777_static_helpers.ts +++ b/test-files/test_gap_2899_2779_2777_static_helpers.ts @@ -46,6 +46,28 @@ console.log(JSON.stringify(mgSym.get(symM))); const mgNum = Map.groupBy([10, 20, 11], (v) => Math.floor(v / 10)); console.log(JSON.stringify([...mgNum.entries()])); +// ---- group arrays survive JSON.stringify (#9849 raw-f64 layout) ---- +// The group arrays are built by the runtime with direct slot writes, so the +// "every slot is an unboxed double" layout flag a fresh array is born with has +// to be re-derived from what actually landed in them. When it was not, +// `JSON.stringify` trusted the flag, read each NaN-boxed string as a double and +// emitted `null` — while `arr[0]` and `String(arr)` stayed correct, so only a +// JSON round-trip could see it. Cover every element kind, and pin the +// non-JSON readback beside it so a future failure says which half broke. +const jsonStr = Map.groupBy(["a", "b", "a"], (v) => v); +console.log(JSON.stringify(jsonStr.get("a")), String(jsonStr.get("a")), jsonStr.get("a")![0]); +const jsonBool = Map.groupBy([true, false, true], (v) => v); +console.log(JSON.stringify(jsonBool.get(true)), String(jsonBool.get(true))); +const jsonObj = Map.groupBy([{ a: 1 }, { a: 2 }], (v) => "k"); +console.log(JSON.stringify(jsonObj.get("k"))); +const jsonMixed = Map.groupBy([1, "two", null, undefined, 3.5], () => "m"); +console.log(JSON.stringify(jsonMixed.get("m"))); +const jsonNum = Map.groupBy([1, 2, 3], () => "n"); +console.log(JSON.stringify(jsonNum.get("n"))); +const ogJson = Object.groupBy(["a", "b", "a"], (v) => v); +console.log(JSON.stringify(ogJson.a), JSON.stringify(ogJson)); +console.log(JSON.stringify([...Map.groupBy("aba", (ch) => ch).values()])); + // ---- TypeError cases ---- try { Object.groupBy(null, (x) => x); diff --git a/test-files/test_gap_disposablestack_2875.ts b/test-files/test_gap_disposablestack_2875.ts index 01076f943c..c2ba1f8889 100644 --- a/test-files/test_gap_disposablestack_2875.ts +++ b/test-files/test_gap_disposablestack_2875.ts @@ -37,3 +37,18 @@ console.log((err.error as Error).message); console.log((err.suppressed as Error).message); console.log(err instanceof Error); console.log(err instanceof SuppressedError); + +// 5) The `instanceof Error` above must hold because the CHAIN says so, not +// because a registry happened to answer first: ECMA-262 gives +// `%SuppressedError%` the [[Prototype]] `%Error%` and +// `%SuppressedError.prototype%` the [[Prototype]] `%Error.prototype%`, exactly +// like the NativeErrors. Assert both links directly, and assert the inherited +// `Error.prototype.toString` behaviour that only a real chain can produce — +// without them `instanceof` regressed to `false` the moment it started +// answering from the recorded prototype walk. +console.log(Object.getPrototypeOf(SuppressedError) === Error); +console.log(Object.getPrototypeOf(SuppressedError.prototype) === Error.prototype); +console.log(Object.getPrototypeOf(TypeError.prototype) === Error.prototype); +console.log(Object.prototype.toString.call(err)); +console.log(String(err)); +console.log(JSON.stringify(new SuppressedError(new Error("a"), new Error("b")).message)); diff --git a/test-files/test_gap_iterator_prototype_next_patch.ts b/test-files/test_gap_iterator_prototype_next_patch.ts index 7f66bdac27..1da3e62299 100644 --- a/test-files/test_gap_iterator_prototype_next_patch.ts +++ b/test-files/test_gap_iterator_prototype_next_patch.ts @@ -11,6 +11,28 @@ const mapProto: any = Object.getPrototypeOf(new Map().entries()); const setProto: any = Object.getPrototypeOf(new Set().values()); const stringProto: any = Object.getPrototypeOf(""[Symbol.iterator]()); +// Output is BUFFERED inside a patched window and flushed after the prototype is +// restored. `console.log` is not safe to call while a built-in iterator +// prototype is patched: node's formatter builds `SafeMap` out of +// `internal/per_context/primordials` lazily, and the harness runs the oracle +// with `FORCE_COLOR=0`, which is exactly the path that defers that +// construction into the window. Node then dies with +// +// node:internal/per_context/primordials:449 +// class SafeMap extends Map {}, +// +// on this fixture — the ORACLE crashes, not Perry, so the test could never +// pass however the runtime behaved. Buffering keeps every value computed +// inside the window (which is the subject) while moving the printing out. +const pending: string[] = []; +function log(...parts: unknown[]) { + pending.push(parts.map((p) => String(p)).join(" ")); +} +function flush() { + for (const line of pending) console.log(line); + pending.length = 0; +} + function withPatched(proto: any, patch: (orig: any) => any, body: () => void) { const orig = proto.next; proto.next = patch(orig); @@ -18,6 +40,7 @@ function withPatched(proto: any, patch: (orig: any) => any, body: () => void) { body(); } finally { proto.next = orig; + flush(); } } @@ -33,11 +56,17 @@ withPatched( () => { const got: number[] = []; for (const v of [1, 2, 3]) got.push(v); - console.log("A-forof", got.join(",")); - console.log("A-spread", [...[4, 5]].join(",")); - console.log("A-from", Array.from([6].values()).join(",")); + log("A-forof", got.join(",")); + log("A-spread", [...[4, 5]].join(",")); + log("A-from", Array.from([6].values()).join(",")); + // `Array.from(array)` and the CALL / multi-operand spread forms reach + // different runtime entry points from `[...array]` — each one has its own + // element-copy fast arm, and each has to decline it here. + log("A-from-array", Array.from([11, 12]).join(",")); + log("A-call-spread", ((...xs: number[]) => xs.join(","))(...[13, 14])); + log("A-multi-spread", [...[15], ...[16]].join(",")); const it = [7, 8].values(); - console.log("A-manual", it.next().value, it.next().value, it.next().done); + log("A-manual", it.next().value, it.next().value, it.next().done); }, ); @@ -62,7 +91,7 @@ withPatched( () => { const got: number[] = []; for (const v of [1, 2]) got.push(v); - console.log("C-forof-empty", got.length); + log("C-forof-empty", got.length); }, ); console.log("C-restored", [...[9]].join(",")); @@ -79,7 +108,9 @@ withPatched( () => { const got: string[] = []; for (const [k, v] of new Map([["a", 1], ["b", 2]])) got.push(k + "=" + v); - console.log("D-map", got.join(",")); + log("D-map", got.join(",")); + log("D-map-spread", JSON.stringify([...new Map([["z", 5]])])); + log("D-map-from", JSON.stringify(Array.from(new Map([["y", 6]])))); }, ); console.log("D-map-restored", [...new Map([["a", 1]])].join(",")); @@ -92,7 +123,8 @@ withPatched( return r; }, () => { - console.log("D-set", [...new Set([1, 2])].join(",")); + log("D-set", [...new Set([1, 2])].join(",")); + log("D-set-from", Array.from(new Set([3, 4])).join(",")); }, ); console.log("D-set-restored", [...new Set([3])].join(",")); @@ -107,7 +139,8 @@ withPatched( return r; }, () => { - console.log("E-string", [..."ab"].join(",")); + log("E-string", [..."ab"].join(",")); + log("E-string-from", Array.from("cd").join(",")); }, ); console.log("E-string-restored", [..."cd"].join(",")); @@ -123,9 +156,10 @@ console.log("E-string-restored", [..."cd"].join(",")); const other = [100, 200].values(); arrayProto.next = orig.bind(other); try { - console.log("F-bound-copy", [...[1, 2]].join(",")); + log("F-bound-copy", [...[1, 2]].join(",")); } finally { arrayProto.next = orig; + flush(); } console.log("F-restored", [...[3]].join(",")); } @@ -142,7 +176,7 @@ console.log("E-string-restored", [..."cd"].join(",")); }, }); try { - console.log("G-accessor", [...[1, 2]].join(","), gets > 0); + log("G-accessor", [...[1, 2]].join(","), gets > 0); } finally { Object.defineProperty(arrayProto, "next", { value: orig, @@ -150,6 +184,7 @@ console.log("E-string-restored", [..."cd"].join(",")); enumerable: false, configurable: true, }); + flush(); } console.log("G-restored", [...[4]].join(",")); } @@ -163,11 +198,12 @@ console.log("E-string-restored", [..."cd"].join(",")); for (const _v of [1]) { console.log("H-unexpected"); } - console.log("H", "no-throw"); + log("H", "no-throw"); } catch (e: any) { - console.log("H", e instanceof TypeError); + log("H", e instanceof TypeError); } finally { arrayProto.next = orig; + flush(); } console.log("H-restored", [...[5, 6]].join(",")); } @@ -182,11 +218,12 @@ for (const bad of [42, "not a function", undefined, null, {}]) { for (const _v of [1]) { console.log("I-unexpected"); } - console.log("I", typeof bad, "no-throw"); + log("I", typeof bad, "no-throw"); } catch (e: any) { - console.log("I", typeof bad, e instanceof TypeError); + log("I", typeof bad, e instanceof TypeError); } finally { arrayProto.next = orig; + flush(); } } console.log("I-restored", [...[7, 8]].join(","));