diff --git a/changelog.d/8173-buffer-receiver-fused-array-callbacks.md b/changelog.d/8173-buffer-receiver-fused-array-callbacks.md new file mode 100644 index 0000000000..d9831b54c6 --- /dev/null +++ b/changelog.d/8173-buffer-receiver-fused-array-callbacks.md @@ -0,0 +1,88 @@ +Fixed nine fused `js_array_*` callback/reduce entry points that returned +**garbage values** — not empty, not a throw — for a Buffer-backed `Uint8Array` +receiver codegen could not statically prove (#8137). + +`holder.u.map(x => x * 2)` answered `[1.297723e-318, 0, 0]` where node answers +`[6,2,4]`; `holder.u.reduce((a, b) => a + b, 0)` answered `6.4886e-319` where +node answers `6`. Also affected: `filter` (`[]`), `find` (`undefined`), +`findIndex` (`-1`), `some`/`every` (`false`), `reduceRight`, `forEach`, and +`js_array_map_discard`. + +Perry's `new Uint8Array([…])` is a `BufferHeader` (`buffer::js_uint8array_new`), +not a `TypedArrayHeader`, so it is absent from the typed-array registry and the +`lookup_typed_array_kind` re-dispatch each helper performs never answers for it. +The helper then reads the `BufferHeader` as an `ArrayHeader`. Both share the +`{length: u32, capacity: u32}` prefix, so `length` reads CORRECTLY while the +elements — decoded as NaN-boxed f64 slots at `base + 8 + i*8` over a payload of +one byte per element — are raw bytes reinterpreted, `length * 7` bytes past the +real payload. Correct length, garbage values, which is why the symptom is not an +empty result and why a probe that only asks "did we return `[]`?" is blind to it. + +This is a **standing gap**, not the #8041 funnel-ordering regression #8090 / +#8109 / #8119 / #8130 / #8140 have been closing: `normalize_array_receiver` is +permissive for a registered Buffer and returns the raw address rather than null. +The new receiver-kind question is nevertheless asked ABOVE the funnel, because +the sibling funnel `clean_arr_ptr` DOES null the same receiver and every caller +reads that as "empty" — keeping the question above means the ordering cannot rot +back into that shape. + +The new `array::buffer_receiver::buffer_receiver_dispatch` delegates to +`dispatch_uint8_buffer_method`, the shared uint8 `%TypedArray%.prototype` +dispatcher a *statically typed* receiver already reaches through +`dispatch_buffer_method`'s catch-all. That choice is load-bearing, not +incidental: these nine pass the receiver to the callback as the 3rd/4th +argument, and the dispatcher passes the ORIGINAL Buffer. Delegating through +`buffer_receiver_as_uint8_typed_array` instead — whose answer is a COPY, and +whose own doc scopes it to the immutable methods — would have made +`u.forEach((v, i, arr) => { arr[0] = 9 })` silently lose the write. +`the_callbacks_third_argument_is_the_receiver_itself_not_a_copy` fails when the +implementation is swapped to a copy, so the choice is pinned rather than +commented. + +Delegating also closed two statically-typed holes: + +* `reduceRight` was wrong for `const u = new Uint8Array([3,1,2])` too, because + codegen folds that call straight to `js_array_reduce_right` rather than routing + it through `dispatch_buffer_method`. Measured + `z|6.36e-314|5.09e-313|6.49e-319` against node's `z|2|1|3`. +* `findLast` had no arm in the uint8 dispatcher at all, so it threw + `TypeError: (Buffer).findLast is not a function` on BOTH dispatch paths. Its + sibling `findLastIndex` was already served, which is why the hole survived — + the two are always cited together. + +`ArrayBuffer` / `SharedArrayBuffer` / `DataView` are declined by +`is_typed_array_buffer`, the same gate `dispatch_buffer_method`'s catch-all uses, +so the two receiver populations cannot drift apart; none has +`%TypedArray%.prototype` and serving them would have invented iteration node does +not have. (Perry answers `[0,0,0,0]` for `ab.map(cb)` where node throws; that +divergence is pre-existing and byte-identical before and after this change.) + +On the ordinary path the gate opens with +`typedarray::arena_payload_has_gc_type(addr, GC_TYPE_ARRAY)`, so `[1,2,3].map(…)` +reaches NEITHER registry. That predicate rather than a bare header-byte read is +the correction #8142 wrote into `array_receiver_gc_tag`'s doc: a Buffer comes in +both backings, and an EXTERNAL one has no `GcHeader` at all — the eight bytes +below its payload are allocator bookkeeping that can read as any `obj_type`, +`GC_TYPE_ARRAY` included, so a bare tag read would skip the probe for exactly the +receiver this function exists to catch. The fast path is asserted, not assumed: +`a_plain_array_never_reaches_the_buffer_gate` measures a `#[cfg(test)]` probe +counter on `is_typed_array_buffer` and fails if the gate is deleted, even though +every ANSWER stays correct. + +18 tests in `array/typed_array_receiver_tests.rs`, all asserting **observed +element values, never a predicate**. #8137 names the trap and it has been +shipped here once already: `u.every(x => x > 0)` answers `true` under node AND +under the bug, because `1.297723e-318 > 0`. The callbacks record what they +actually saw, so a garbage read fails whatever the predicate says. Five sabotage +arms were run and reverted: gate always declines (12/12 subject tests fail, 4 +controls correctly pass), gate deleted (probe-count test fails), `findLast` arm +removed (its test fails), over-reach to every registered buffer (the +ArrayBuffer/DataView control fails), and delegate-through-a-copy (the +3rd-argument test fails while the values stay correct). Sabotage A caught a +vacuous test of my own — the 3rd-argument test passed under it, because the +pre-fix helper already passed the raw buffer as the 3rd argument while reading +garbage elements — so it now carries an element assertion too. + +Verified byte-for-byte against node `v26.5.1` (`.node-version`) across 36 probe +cases, with plain-array, array-like-object and `Int32Array` controls. +`cargo test -p perry-runtime --lib`: 2436 passed, 0 failed, 4 ignored. diff --git a/crates/perry-runtime/src/array/buffer_receiver.rs b/crates/perry-runtime/src/array/buffer_receiver.rs new file mode 100644 index 0000000000..5ffc0af57c --- /dev/null +++ b/crates/perry-runtime/src/array/buffer_receiver.rs @@ -0,0 +1,116 @@ +//! #8137: resolve an `Array.prototype` iteration receiver that is really a +//! Buffer-backed `Uint8Array`, and run the method on it in place. +//! +//! Perry's `new Uint8Array([…])` is a `BufferHeader` (`buffer::js_uint8array_new`), +//! not a `TypedArrayHeader`. It is therefore absent from the typed-array +//! registry, and the `lookup_typed_array_kind` re-dispatch every fused +//! `js_array_*` callback helper performs never answers for it. The helper then +//! reads the `BufferHeader` as an `ArrayHeader`. Both start with +//! `{length: u32, capacity: u32}`, so `length` is CORRECT while the elements — +//! read as NaN-boxed f64 slots at `base + 8 + i*8` over a payload that is one +//! byte per element — are raw bytes reinterpreted, and the read runs +//! `length * 7` bytes past the real payload. +//! +//! That is why the symptom is *garbage values*, not an empty result or a +//! throw, and why any probe that only asks "did we return `[]`?" is blind to +//! it. It is also why a predicate test is vacuous: `u.every(x => x > 0)` +//! answers `true` under node AND under the bug, because `1.29e-318 > 0`. Every +//! test in this family must assert value identity. +//! +//! The answer is the shared uint8 `%TypedArray%.prototype` dispatcher that the +//! *statically* typed receiver already reaches (`dispatch_buffer_method`'s +//! catch-all → `dispatch_uint8_buffer_method`). It reads elements through +//! `js_buffer_get` and passes the ORIGINAL Buffer as the callback's 3rd/4th +//! argument, so — unlike `buffer_receiver_as_uint8_typed_array`, which hands +//! back a COPY and is scoped to the immutable methods — a write through +//! `arr` in `u.forEach((v, i, arr) => { arr[0] = 9 })` still lands on `u`. + +use super::ArrayHeader; +use crate::closure::ClosureHeader; + +/// NaN-box a callback closure as the `args[0]` the uint8 dispatcher validates. +#[inline] +pub(crate) fn callback_arg(callback: *const ClosureHeader) -> f64 { + f64::from_bits(crate::value::JSValue::pointer(callback as *const u8).bits()) +} + +/// Run `method` on `arr` through the uint8 `%TypedArray%.prototype` dispatcher +/// when `arr` is a Buffer-backed `Uint8Array`. +/// +/// `Some(result)` — the receiver was resolved and the method ran; the caller +/// must return, converting the NaN-boxed result to its own return type. +/// `None` — not our receiver (or the dispatcher does not implement `method`); +/// the caller keeps its ordinary array path. +/// +/// **Call this ABOVE the array-only funnel.** `normalize_array_receiver` is +/// permissive for a registered Buffer (it returns the raw address rather than +/// null), so a re-dispatch below it does still run today — but that is a +/// property of one funnel, and the sibling funnel `clean_arr_ptr` returns NULL +/// for the same receiver, which every caller reads as "empty". Asking the +/// receiver-kind question first is what #8090 / #8119 / #8130 / #8140 each had +/// to restore after it had been placed below one; keeping this call above the +/// funnel means the ordering cannot rot back. +pub(crate) fn buffer_receiver_dispatch( + arr: *const ArrayHeader, + method: &str, + args: &[f64], +) -> Option { + let addr = crate::array::array_receiver_addr(arr as *mut ArrayHeader); + if addr == 0 { + return None; + } + // Cheap negative for the hot path: a receiver that is PROVABLY an + // arena-backed `GC_TYPE_ARRAY` cannot be a registered Buffer, so an + // ordinary `[1,2,3].map(…)` reaches NEITHER registry. + // + // `arena_payload_has_gc_type` rather than a bare header-byte read (or an + // open-coded address floor): a Buffer comes in BOTH backings, and an + // EXTERNAL one — `EXTERNAL_BUFFER_REGISTRY`, `shared_sab::alloc_shared_sab` + // — has no `GcHeader` at all. The eight bytes below its payload are + // allocator bookkeeping and can read as any `obj_type`, `GC_TYPE_ARRAY` + // included. A bare tag read would therefore skip the probe for exactly the + // receiver this function exists to catch, silently and only sometimes. + // The predicate range-checks, rejects `HeapSpace::Unknown` for the HEADER + // address, and validates through `gc_type_info` before trusting the byte; + // it answers `false` for an external buffer, which falls through to the + // registry — the authoritative answer. See `array/header.rs`'s + // `array_receiver_gc_tag` doc (#8142). + if unsafe { crate::typedarray::arena_payload_has_gc_type(addr, crate::gc::GC_TYPE_ARRAY) } { + return None; + } + // `is_typed_array_buffer` is the same gate `dispatch_buffer_method`'s + // catch-all uses to reach this dispatcher, so the two receiver populations + // cannot drift apart. It declines `ArrayBuffer` / `SharedArrayBuffer` / + // `DataView` (none has `%TypedArray%.prototype`, so node throws rather + // than answering elements) and the KeyObject / CryptoKey buffers. + if !crate::object::typed_array_proto_thunks::is_typed_array_buffer(addr) { + return None; + } + // Root the receiver and the callback across the dispatch. Both arrive as + // raw parameters of a `#[no_mangle]` helper, and a callback allocated by a + // FRAMELESS caller — the arrow in `holder.u.map(x => x * 2)` — is reachable + // ONLY through that parameter plus the native stack, which an evacuating + // minor does not scan. Closures are non-movable, so an unrooted one is + // swept in place mid-loop and the next dispatch calls freed memory. This is + // #6081 / gh #6206 exactly; `js_array_map` roots its callback for the same + // reason, and routing through this function must not lose that root. + let scope = crate::gc::RuntimeHandleScope::new(); + let _recv = scope.root_nanbox_f64(f64::from_bits( + crate::value::JSValue::pointer(addr as *const u8).bits(), + )); + let _cb = args + .first() + .map(|callback| scope.root_nanbox_f64(*callback)); + unsafe { + crate::object::typed_array_proto_thunks::dispatch_uint8_buffer_method(addr, method, args) + } +} + +/// The `*mut ArrayHeader` a `map`/`filter` caller returns, from the NaN-boxed +/// pointer the dispatcher answers. The result is a `BufferHeader`, matching +/// node (`u8.map(…)` is a `Uint8Array`, not a plain Array) and matching what +/// the typed-array arm beside it already does with a `TypedArrayHeader`. +#[inline] +pub(crate) fn dispatch_result_as_array(result: f64) -> *mut ArrayHeader { + crate::value::js_nanbox_get_pointer(result) as *mut ArrayHeader +} diff --git a/crates/perry-runtime/src/array/iter_methods.rs b/crates/perry-runtime/src/array/iter_methods.rs index cf4c97ae93..c4dba84b4c 100644 --- a/crates/perry-runtime/src/array/iter_methods.rs +++ b/crates/perry-runtime/src/array/iter_methods.rs @@ -153,6 +153,21 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo if collection_foreach_reroute(arr, callback) { return; } + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if crate::array::buffer_receiver_dispatch( + arr, + "forEach", + &[crate::array::callback_arg(callback)], + ) + .is_some() + { + 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 @@ -219,6 +234,17 @@ pub extern "C" fn js_array_map( arr: *const ArrayHeader, callback: *const ClosureHeader, ) -> *mut ArrayHeader { + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if let Some(result) = + crate::array::buffer_receiver_dispatch(arr, "map", &[crate::array::callback_arg(callback)]) + { + return crate::array::dispatch_result_as_array(result); + } let arr = normalize_array_receiver(arr); if arr.is_null() { return js_array_alloc(0); @@ -312,6 +338,23 @@ pub extern "C" fn js_array_map( /// effects without allocating or filling the result array. #[no_mangle] pub extern "C" fn js_array_map_discard(arr: *const ArrayHeader, callback: *const ClosureHeader) { + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + // `forEach`, not `map`: this entry point exists precisely to preserve + // callback evaluation order and side effects WITHOUT allocating a result. + if crate::array::buffer_receiver_dispatch( + arr, + "forEach", + &[crate::array::callback_arg(callback)], + ) + .is_some() + { + return; + } let arr = normalize_array_receiver(arr); if arr.is_null() { return; @@ -373,6 +416,19 @@ pub extern "C" fn js_array_filter( arr: *const ArrayHeader, callback: *const ClosureHeader, ) -> *mut ArrayHeader { + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if let Some(result) = crate::array::buffer_receiver_dispatch( + arr, + "filter", + &[crate::array::callback_arg(callback)], + ) { + return crate::array::dispatch_result_as_array(result); + } let arr = normalize_array_receiver(arr); if arr.is_null() { return js_array_alloc(0); @@ -446,6 +502,17 @@ pub extern "C" fn js_array_filter( /// Returns the element as f64, or undefined if not found. #[no_mangle] pub extern "C" fn js_array_find(arr: *const ArrayHeader, callback: *const ClosureHeader) -> f64 { + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if let Some(result) = + crate::array::buffer_receiver_dispatch(arr, "find", &[crate::array::callback_arg(callback)]) + { + return result; + } let arr = normalize_array_receiver(arr); if arr.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); @@ -488,6 +555,20 @@ pub extern "C" fn js_array_findIndex( arr: *const ArrayHeader, callback: *const ClosureHeader, ) -> i32 { + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if let Some(result) = crate::array::buffer_receiver_dispatch( + arr, + "findIndex", + &[crate::array::callback_arg(callback)], + ) { + // The dispatcher answers a raw f64 index (or `-1.0`), never a NaN-box. + return result as i32; + } let arr = normalize_array_receiver(arr); if arr.is_null() { return -1; @@ -650,6 +731,18 @@ pub extern "C" fn js_array_at(arr: *const ArrayHeader, index: f64) -> f64 { pub extern "C" fn js_array_some(arr: *const ArrayHeader, callback: *const ClosureHeader) -> f64 { const TAG_TRUE: u64 = 0x7FFC_0000_0000_0004; const TAG_FALSE: u64 = 0x7FFC_0000_0000_0003; + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if let Some(result) = + crate::array::buffer_receiver_dispatch(arr, "some", &[crate::array::callback_arg(callback)]) + { + // Already a NaN-boxed boolean, the same shape this function returns. + return result; + } let arr = normalize_array_receiver(arr); if arr.is_null() { return f64::from_bits(TAG_FALSE); @@ -696,6 +789,19 @@ pub extern "C" fn js_array_some(arr: *const ArrayHeader, callback: *const Closur pub extern "C" fn js_array_every(arr: *const ArrayHeader, callback: *const ClosureHeader) -> f64 { const TAG_TRUE: u64 = 0x7FFC_0000_0000_0004; const TAG_FALSE: u64 = 0x7FFC_0000_0000_0003; + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + if let Some(result) = crate::array::buffer_receiver_dispatch( + arr, + "every", + &[crate::array::callback_arg(callback)], + ) { + return result; + } let arr = normalize_array_receiver(arr); if arr.is_null() { return f64::from_bits(TAG_TRUE); @@ -814,6 +920,22 @@ pub extern "C" fn js_array_reduce( has_initial: i32, initial: f64, ) -> f64 { + // #8137: a Buffer-backed `Uint8Array` receiver. Perry's + // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array + // registry, so the `lookup_typed_array_kind` re-dispatch below never + // answers for it and the `BufferHeader` is read as an `ArrayHeader` — + // correct `length`, GARBAGE elements. Asked above the funnel; see + // `array::buffer_receiver`. + // An ABSENT initial value must stay absent: the dispatcher distinguishes + // `args.len() >= 2` (seed supplied) from a one-element list (seed is the + // first element), and a seedless reduce over an empty receiver must THROW + // rather than answer `undefined`. Passing `initial` unconditionally would + // silently turn every seedless reduce into a seeded one. + let reduce_args = [crate::array::callback_arg(callback), initial]; + let reduce_args = &reduce_args[..if has_initial != 0 { 2 } else { 1 }]; + if let Some(result) = crate::array::buffer_receiver_dispatch(arr, "reduce", reduce_args) { + return result; + } let arr = normalize_array_receiver(arr); if arr.is_null() { if has_initial != 0 { diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 701560cce6..83ab365aec 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -1,5 +1,6 @@ //! Array representation for Perry — split into topical sub-modules. mod alloc; +mod buffer_receiver; mod concat_reverse; mod element_shape; mod fill_extend; @@ -46,6 +47,9 @@ pub use self::alloc::{ js_array_alloc_with_length_longlived, js_array_constructor_single, js_array_create, js_array_from_arraylike_holey_value, js_array_from_f64, }; +pub(crate) use self::buffer_receiver::{ + buffer_receiver_dispatch, callback_arg, dispatch_result_as_array, +}; pub use self::concat_reverse::{ js_array_concat, js_array_concat_new, js_array_fill, js_array_fill_generic, js_array_fill_range, js_array_reverse, js_array_reverse_value, diff --git a/crates/perry-runtime/src/array/reduce_right.rs b/crates/perry-runtime/src/array/reduce_right.rs index d25c29bceb..2b333860a1 100644 --- a/crates/perry-runtime/src/array/reduce_right.rs +++ b/crates/perry-runtime/src/array/reduce_right.rs @@ -22,6 +22,20 @@ pub extern "C" fn js_array_reduce_right( has_initial: i32, initial: f64, ) -> f64 { + // #8137: a Buffer-backed `Uint8Array` receiver reads as an `ArrayHeader` + // below — correct `length`, GARBAGE elements. `reduceRight` is the widest + // case in the family: it is wrong even for a STATICALLY typed + // `const u = new Uint8Array([3,1,2])`, because codegen folds that call + // straight to this helper rather than routing it through + // `dispatch_buffer_method`. Measured `z|6.36e-314|5.09e-315|4.29e-315` + // against node's `z|2|1|3`. + // + // An ABSENT initial value must stay absent — see `js_array_reduce`. + let reduce_args = [crate::array::callback_arg(callback), initial]; + let reduce_args = &reduce_args[..if has_initial != 0 { 2 } else { 1 }]; + if let Some(result) = crate::array::buffer_receiver_dispatch(arr, "reduceRight", reduce_args) { + return result; + } let arr = normalize_array_receiver(arr); if arr.is_null() { if has_initial != 0 { diff --git a/crates/perry-runtime/src/array/typed_array_receiver_tests.rs b/crates/perry-runtime/src/array/typed_array_receiver_tests.rs index 08c2a18b9a..8370e0a516 100644 --- a/crates/perry-runtime/src/array/typed_array_receiver_tests.rs +++ b/crates/perry-runtime/src/array/typed_array_receiver_tests.rs @@ -707,3 +707,469 @@ fn an_array_buffer_or_data_view_receiver_gets_no_element_iterator() { "a DataView receiver must not be served a Uint8Array iterator" ); } + +// --------------------------------------------------------------------------- +// #8137: the nine fused `js_array_*` CALLBACK entry points must resolve a +// Buffer-backed `Uint8Array` receiver before reading it as an `ArrayHeader`. +// +// The precondition is pinned by +// `a_new_uint8array_is_a_buffer_not_a_registry_typed_array` above: perry's +// `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array +// registry, so the `lookup_typed_array_kind` re-dispatch each of these helpers +// performs never answers for it. +// +// Unlike #8140's iterator family, the failure here is NOT an empty result. +// `BufferHeader` and `ArrayHeader` share the `{length, capacity}` prefix, so +// `length` reads CORRECTLY while the elements — decoded as NaN-boxed f64 slots +// at `base + 8 + i*8` over a payload of one byte per element — are raw bytes +// reinterpreted, `length * 7` bytes past the real payload. Measured against +// node v26.5.1 on `{ u: new Uint8Array([3,1,2]) }`: +// +// entry point node perry pre-fix +// map [6,2,4] [1.297723e-318,0,0] +// filter [3,2] [] +// find 1 undefined +// findIndex 1 -1 +// some(x => x === 2) true false +// every(x => x in {3,1,2}) true false +// reduce 6 6.4886e-319 +// reduceRight z|2|1|3 z|0|0|6.4886e-319 +// forEach 3;1;2; 6.4886e-319;0;0; +// +// **Every test below asserts the OBSERVED ELEMENT VALUES, never a predicate.** +// The issue names the trap explicitly: `u.every(x => x > 0)` answers `true` +// under node AND under the bug, because `1.297723e-318 > 0`. A probe of that +// shape reports PASS on the broken path, and that exact vacuity has already +// been shipped here once. `OBSERVED` below records what the callback actually +// saw, so a garbage read fails the assertion whatever the predicate says. +// --------------------------------------------------------------------------- + +thread_local! { + /// Every `(value, index, receiver_bits)` triple a test callback observed. + static OBSERVED: std::cell::RefCell> = + const { std::cell::RefCell::new(Vec::new()) }; +} + +fn observed_values() -> Vec { + OBSERVED.with(|o| o.borrow().iter().map(|(v, _, _)| *v).collect()) +} + +fn observed_indices() -> Vec { + OBSERVED.with(|o| o.borrow().iter().map(|(_, i, _)| *i).collect()) +} + +fn observed_receivers() -> Vec { + OBSERVED.with(|o| o.borrow().iter().map(|(_, _, r)| *r).collect()) +} + +fn reset_observed() { + OBSERVED.with(|o| o.borrow_mut().clear()); +} + +fn record(value: f64, index: f64, receiver: f64) { + OBSERVED.with(|o| o.borrow_mut().push((value, index, receiver.to_bits()))); +} + +/// `(element, index, receiver) -> element * 2` — records, then doubles. +extern "C" fn cb_double( + _closure: *const crate::closure::ClosureHeader, + value: f64, + index: f64, + receiver: f64, +) -> f64 { + record(value, index, receiver); + value * 2.0 +} + +/// Truthy for `3` and `2` — a VALUE-IDENTITY predicate. `x > 1` would also +/// answer "true" for the garbage reads, so it must not be used. +extern "C" fn cb_is_three_or_two( + _closure: *const crate::closure::ClosureHeader, + value: f64, + index: f64, + receiver: f64, +) -> f64 { + record(value, index, receiver); + bool_f64(value == 3.0 || value == 2.0) +} + +/// Truthy only for the literal `1`. +extern "C" fn cb_is_one( + _closure: *const crate::closure::ClosureHeader, + value: f64, + index: f64, + receiver: f64, +) -> f64 { + record(value, index, receiver); + bool_f64(value == 1.0) +} + +/// Truthy for every member of `{3, 1, 2}` — the discriminating `every` +/// predicate. A `x > 0` predicate here is VACUOUS (see the header comment). +extern "C" fn cb_is_a_source_byte( + _closure: *const crate::closure::ClosureHeader, + value: f64, + index: f64, + receiver: f64, +) -> f64 { + record(value, index, receiver); + bool_f64(value == 3.0 || value == 1.0 || value == 2.0) +} + +/// `(accumulator, element, index, receiver) -> accumulator + element`. +extern "C" fn cb_sum( + _closure: *const crate::closure::ClosureHeader, + accumulator: f64, + value: f64, + index: f64, + receiver: f64, +) -> f64 { + record(value, index, receiver); + accumulator + value +} + +fn bool_f64(b: bool) -> f64 { + f64::from_bits(crate::value::JSValue::bool(b).bits()) +} + +fn is_true(v: f64) -> bool { + v.to_bits() == crate::value::TAG_TRUE +} + +fn closure(func: *const u8) -> *const crate::closure::ClosureHeader { + crate::closure::js_closure_alloc(func, 0) as *const crate::closure::ClosureHeader +} + +/// Read a Buffer-backed result (what `map`/`filter` answer for this receiver, +/// matching node — `u8.map(…)` is a `Uint8Array`, not a plain Array). +fn read_uint8_result(result: *mut ArrayHeader) -> Vec { + assert!(!result.is_null(), "the helper must answer a collection"); + let buf = result as *const crate::buffer::BufferHeader; + let len = unsafe { (*buf).length } as usize; + (0..len) + .map(|i| crate::buffer::js_buffer_get(buf, i as i32) as u8) + .collect() +} + +/// The receiver every test in this block uses: `new Uint8Array([3, 1, 2])`. +fn subject() -> *mut ArrayHeader { + reset_observed(); + uint8_buffer(&[3.0, 1.0, 2.0]) +} + +#[test] +fn js_array_map_maps_a_buffer_receivers_bytes() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let result = crate::array::js_array_map(buf, closure(cb_double as *const u8)); + assert_eq!( + observed_values(), + vec![3.0, 1.0, 2.0], + "the callback must see the BYTES. `[1.297723e-318, 0, 0]` is #8137 — \ + the BufferHeader read as an ArrayHeader" + ); + assert_eq!( + read_uint8_result(result), + vec![6, 2, 4], + "node answers Uint8Array [6,2,4]" + ); +} + +#[test] +fn js_array_map_discard_still_runs_the_callbacks_on_a_buffer() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + // `map` whose result is unused: no allocation, but the callback must still + // observe every real byte in order. + crate::array::js_array_map_discard(buf, closure(cb_double as *const u8)); + assert_eq!(observed_values(), vec![3.0, 1.0, 2.0]); + assert_eq!(observed_indices(), vec![0.0, 1.0, 2.0]); +} + +#[test] +fn js_array_filter_filters_a_buffer_receivers_bytes() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let result = crate::array::js_array_filter(buf, closure(cb_is_three_or_two as *const u8)); + assert_eq!(observed_values(), vec![3.0, 1.0, 2.0]); + assert_eq!( + read_uint8_result(result), + vec![3, 2], + "node answers [3,2]; the EMPTY list is #8137" + ); +} + +#[test] +fn js_array_find_finds_a_buffer_receivers_byte() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let found = crate::array::js_array_find(buf, closure(cb_is_one as *const u8)); + assert_eq!(observed_values(), vec![3.0, 1.0]); + assert_eq!(found, 1.0, "node answers 1; `undefined` is #8137"); +} + +#[test] +fn js_array_find_index_finds_a_buffer_receivers_index() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let index = crate::array::js_array_findIndex(buf, closure(cb_is_one as *const u8)); + assert_eq!(observed_values(), vec![3.0, 1.0]); + assert_eq!(index, 1, "node answers 1; `-1` is #8137"); +} + +#[test] +fn js_array_some_sees_a_buffer_receivers_bytes() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let answer = crate::array::js_array_some(buf, closure(cb_is_one as *const u8)); + assert_eq!( + observed_values(), + vec![3.0, 1.0], + "the recorded values are the discriminating measurement — a `some` \ + ANSWER can coincide by luck, the observed bytes cannot" + ); + assert!(is_true(answer), "node answers true; `false` is #8137"); +} + +#[test] +fn js_array_every_sees_a_buffer_receivers_bytes() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + // `cb_is_a_source_byte`, NOT `x > 0`: the garbage reads are also `> 0`, so + // a sign predicate answers `true` on the BROKEN path too (#8137's own + // "vacuous probe to avoid"). + let answer = crate::array::js_array_every(buf, closure(cb_is_a_source_byte as *const u8)); + assert_eq!(observed_values(), vec![3.0, 1.0, 2.0]); + assert!(is_true(answer), "node answers true; `false` is #8137"); +} + +#[test] +fn js_array_for_each_visits_a_buffer_receivers_bytes() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + crate::array::js_array_forEach(buf, closure(cb_double as *const u8)); + assert_eq!( + observed_values(), + vec![3.0, 1.0, 2.0], + "node visits 3;1;2; — `6.4886e-319;0;0;` is #8137" + ); + assert_eq!(observed_indices(), vec![0.0, 1.0, 2.0]); +} + +#[test] +fn js_array_reduce_accumulates_a_buffer_receivers_bytes() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let sum = crate::array::js_array_reduce(buf, closure(cb_sum as *const u8), 1, 0.0); + assert_eq!(observed_values(), vec![3.0, 1.0, 2.0]); + assert_eq!(sum, 6.0, "node answers 6; `9.12e-313` is #8137"); +} + +#[test] +fn js_array_reduce_without_a_seed_starts_at_the_first_byte() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + // `has_initial == 0` must stay seedless through the delegation: the + // dispatcher keys on `args.len() >= 2`, so forwarding `initial` + // unconditionally would silently seed every reduce with 0.0 — the same + // answer here, but the WRONG one for a non-additive callback, and it would + // turn the empty-receiver TypeError into a silent `undefined`. + let sum = crate::array::js_array_reduce(buf, closure(cb_sum as *const u8), 0, 0.0); + assert_eq!( + observed_values(), + vec![1.0, 2.0], + "the first byte becomes the seed, so only indices 1..n reach the callback" + ); + assert_eq!(sum, 6.0); +} + +#[test] +fn js_array_reduce_right_accumulates_a_buffer_receiver_in_reverse() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let sum = crate::array::js_array_reduce_right(buf, closure(cb_sum as *const u8), 1, 0.0); + assert_eq!( + observed_indices(), + vec![2.0, 1.0, 0.0], + "reduceRight must walk right-to-left" + ); + assert_eq!(observed_values(), vec![2.0, 1.0, 3.0]); + assert_eq!(sum, 6.0); + // `reduceRight` is the widest case in the family: it is wrong for a + // STATICALLY typed receiver too, because codegen folds that call straight + // to this helper rather than routing it through `dispatch_buffer_method`. + // Measured `z|6.36e-314|5.09e-313|6.49e-319` against node's `z|2|1|3`. +} + +#[test] +fn the_callbacks_third_argument_is_the_receiver_itself_not_a_copy() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + crate::array::js_array_forEach(buf, closure(cb_double as *const u8)); + + // Non-vacuity: the receiver-identity assertion below ALSO holds on the + // broken path (the pre-fix helper passes `rooted.receiver()`, which is the + // raw buffer, while reading garbage ELEMENTS), so on its own this test + // reports PASS under the bug. It is here to pin the CHOICE of fix, not the + // fix itself — so it must carry the element assertion as well. + assert_eq!(observed_values(), vec![3.0, 1.0, 2.0]); + + let expected = crate::value::JSValue::pointer(buf as *const u8).bits(); + assert_eq!( + observed_receivers(), + vec![expected; 3], + "the spec passes the ORIGINAL receiver as the 3rd argument. This is \ + why the fix delegates to the uint8 dispatcher (which reads through \ + `js_buffer_get`) rather than to `buffer_receiver_as_uint8_typed_array`, \ + whose answer is a COPY: `u.forEach((v, i, arr) => {{ arr[0] = 9 }})` \ + must mutate `u`, and through a copy the write is silently lost" + ); + + // Not just pointer-equal — writable. A copy would swallow this. + crate::buffer::js_buffer_set(buf as *mut crate::buffer::BufferHeader, 0, 9); + assert_eq!( + crate::buffer::js_buffer_get(buf as *const crate::buffer::BufferHeader, 0), + 9 + ); +} + +#[test] +fn find_last_is_served_for_a_buffer_receiver() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + let cb = closure(cb_is_three_or_two as *const u8); + let args = [f64::from_bits( + crate::value::JSValue::pointer(cb as *const u8).bits(), + )]; + // `findLast` had NO arm in the uint8 dispatcher, so it fell through to + // `dispatch_buffer_method`'s catch-all and threw + // `TypeError: (Buffer).findLast is not a function` — for the STATIC + // receiver too. Node answers `2`. Its sibling `findLastIndex` was already + // served, which is why the hole survived: the two are always cited + // together. `None` here is the pre-fix behaviour. + let answer = unsafe { + crate::object::typed_array_proto_thunks::dispatch_uint8_buffer_method( + buf as usize, + "findLast", + &args, + ) + }; + assert_eq!( + answer, + Some(2.0), + "node answers 2; `None` means the arm is gone and the catch-all throws" + ); + assert_eq!( + observed_values(), + vec![2.0], + "findLast must walk right-to-left and stop at the first match" + ); +} + +// ---- controls: the ordinary receivers must be untouched ------------------- + +#[test] +fn a_plain_array_still_maps_through_the_same_helper() { + let _serialized = crate::array::test_serialize(); + reset_observed(); + let mut arr = js_array_alloc(3); + for v in [3.0, 1.0, 2.0] { + arr = js_array_push_f64(arr, v); + } + let result = crate::array::js_array_map(arr, closure(cb_double as *const u8)); + assert_eq!(observed_values(), vec![3.0, 1.0, 2.0]); + let out = crate::array::header::clean_arr_ptr(result); + assert!( + !out.is_null(), + "a plain array must still answer a plain array" + ); + let values: Vec = (0..3) + .map(|i| crate::array::js_array_get_f64(out, i)) + .collect(); + assert_eq!(values, vec![6.0, 2.0, 4.0]); +} + +#[test] +fn a_plain_array_never_reaches_the_buffer_gate() { + let _serialized = crate::array::test_serialize(); + reset_observed(); + let mut arr = js_array_alloc(3); + for v in [3.0, 1.0, 2.0] { + arr = js_array_push_f64(arr, v); + } + // Prime anything built lazily on first touch, then measure ONLY the + // receiver-resolution call — the same discipline #8140's probe-count test + // needed, and for the same reason: reading the result runs its own + // per-element probes and would swamp the window. + crate::array::js_array_map_discard(arr, closure(cb_double as *const u8)); + let before = crate::object::typed_array_proto_thunks::test_buffer_gate_probe_count(); + crate::array::js_array_map_discard(arr, closure(cb_double as *const u8)); + let after = crate::object::typed_array_proto_thunks::test_buffer_gate_probe_count(); + assert_eq!( + after, before, + "a provably arena-backed GC_TYPE_ARRAY receiver must be rejected by \ + `arena_payload_has_gc_type` before any registry probe. Delete that \ + gate at the top of `buffer_receiver_dispatch` and this fails, even \ + though every ANSWER above stays correct" + ); +} + +#[test] +fn a_generic_array_like_object_receiver_still_materializes() { + let _serialized = crate::array::test_serialize(); + reset_observed(); + // `Array.prototype.map.call({length: 3, 0: 3, 1: 1, 2: 2}, cb)` — the + // `normalize_array_receiver` arm below the new gate. The buffer question + // must decline a GC_TYPE_OBJECT receiver and leave it reachable. + let obj = crate::object::js_object_alloc(0, 4); + let key = |k: &str| crate::string::js_string_from_bytes(k.as_ptr(), k.len() as u32); + crate::object::js_object_set_field_by_name(obj, key("length"), 3.0); + crate::object::js_object_set_field_by_name(obj, key("0"), 3.0); + crate::object::js_object_set_field_by_name(obj, key("1"), 1.0); + crate::object::js_object_set_field_by_name(obj, key("2"), 2.0); + crate::array::js_array_map_discard(obj as *const ArrayHeader, closure(cb_double as *const u8)); + assert_eq!( + observed_values(), + vec![3.0, 1.0, 2.0], + "an array-like object receiver must still be materialized and iterated" + ); +} + +#[test] +fn an_array_buffer_or_data_view_receiver_is_not_given_element_semantics() { + let _serialized = crate::array::test_serialize(); + // `ArrayBuffer` / `SharedArrayBuffer` / `DataView` have no + // %TypedArray%.prototype. The gate must decline them so this change cannot + // INVENT iteration node does not have — exactly as + // `buffer_receiver_as_uint8_typed_array` and #8140's iterator arm do. + let ab = crate::buffer::buffer_alloc(4); + unsafe { (*ab).length = 4 }; + crate::buffer::mark_as_array_buffer(ab as usize); + assert!( + crate::array::buffer_receiver_dispatch(ab as *const ArrayHeader, "forEach", &[0.0]) + .is_none(), + "an ArrayBuffer receiver must not be served Uint8Array iteration" + ); + + let dv = crate::buffer::buffer_alloc(4); + unsafe { (*dv).length = 4 }; + crate::buffer::mark_as_data_view(dv as usize); + assert!( + crate::array::buffer_receiver_dispatch(dv as *const ArrayHeader, "reduce", &[0.0]) + .is_none(), + "a DataView receiver must not be served Uint8Array iteration" + ); +} + +#[test] +fn a_method_the_uint8_dispatcher_does_not_implement_falls_through() { + let _serialized = crate::array::test_serialize(); + let buf = subject(); + // `flatMap` is not a %TypedArray%.prototype method (node throws + // `… is not a function`). The gate must answer `None` for it rather than + // inventing a result, so the caller keeps whatever it does today. + assert!( + crate::array::buffer_receiver_dispatch(buf, "flatMap", &[0.0]).is_none(), + "an unimplemented method must fall through, not answer" + ); +} diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 5ea0354eab..ce0d280bc7 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -148,7 +148,7 @@ mod string_proto_thunks; #[cfg(feature = "temporal")] mod temporal_proto; mod typed_array_define; -mod typed_array_proto_thunks; +pub(crate) mod typed_array_proto_thunks; mod util_types; mod weakref_proto_thunks; mod websocket_global; diff --git a/crates/perry-runtime/src/object/typed_array_proto_thunks.rs b/crates/perry-runtime/src/object/typed_array_proto_thunks.rs index 3ee3664294..a5e5144b0f 100644 --- a/crates/perry-runtime/src/object/typed_array_proto_thunks.rs +++ b/crates/perry-runtime/src/object/typed_array_proto_thunks.rs @@ -184,7 +184,27 @@ unsafe fn ta_receiver_or_throw(method: &str) -> TypedArrayProtoReceiver { throw_not_typed_array(method) } -pub(super) fn is_typed_array_buffer(addr: usize) -> bool { +#[cfg(test)] +thread_local! { +/// Every entry into [`is_typed_array_buffer`], i.e. every caller that could not +/// rule a Buffer receiver out more cheaply. Mirrors #8140's +/// `TEST_TA_REGISTRY_PROBES` and exists for the same reason: the +/// `arena_payload_has_gc_type` gate that lets an ordinary array skip this probe +/// is asserted against it, so deleting the gate fails a test even though the +/// ANSWER stays correct. A fast path nobody can prove ran is not a fast path. +/// +/// Per THREAD, not per process: `cargo test` runs each case on its own thread. + static TEST_BUFFER_GATE_PROBES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[cfg(test)] +pub(crate) fn test_buffer_gate_probe_count() -> u64 { + TEST_BUFFER_GATE_PROBES.with(|c| c.get()) +} + +pub(crate) fn is_typed_array_buffer(addr: usize) -> bool { + #[cfg(test)] + TEST_BUFFER_GATE_PROBES.with(|c| c.set(c.get().wrapping_add(1))); crate::buffer::is_registered_buffer(addr) && !crate::buffer::is_any_array_buffer(addr) && !crate::buffer::is_data_view(addr) @@ -519,7 +539,7 @@ fn validate_comparator(args: &[f64]) -> *const crate::closure::ClosureHeader { } } -pub(super) unsafe fn dispatch_uint8_buffer_method( +pub(crate) unsafe fn dispatch_uint8_buffer_method( addr: usize, method: &str, args: &[f64], @@ -621,9 +641,19 @@ pub(super) unsafe fn dispatch_uint8_buffer_method( } bool_value(false) } - "find" => { + // #8137: `findLast` had NO arm, so it fell through to the catch-all + // and threw `TypeError: (Buffer).findLast is not a function` — on the + // STATIC receiver too (`new Uint8Array([3,1,2]).findLast(x => x < 3)`, + // node: `2`). Its sibling `findLastIndex` was already served below, + // which is why the hole survived: the two are always cited together. + "find" | "findLast" => { let cb = validate_callback(args); - for i in 0..len { + let indexes: Box> = if method == "find" { + Box::new(0..len) + } else { + Box::new((0..len).rev()) + }; + for i in indexes { let value = uint8_get(addr, i) as f64; let keep = crate::closure::js_closure_call3(cb, value, i as f64, receiver); if crate::value::js_is_truthy(keep) != 0 { diff --git a/crates/perry-runtime/src/typedarray/mod.rs b/crates/perry-runtime/src/typedarray/mod.rs index 0d2b952d4a..aa419894b2 100644 --- a/crates/perry-runtime/src/typedarray/mod.rs +++ b/crates/perry-runtime/src/typedarray/mod.rs @@ -733,7 +733,7 @@ fn is_arena_backed_addr(addr: usize) -> bool { } #[inline] -unsafe fn arena_payload_has_gc_type(addr: usize, expected_type: u8) -> bool { +pub(crate) unsafe fn arena_payload_has_gc_type(addr: usize, expected_type: u8) -> bool { if addr < crate::gc::GC_HEADER_SIZE + 0x1000 { return false; }