From 394ab1c6eaffd3385af9535c1bf0474ab181bf1a Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 14 Aug 2026 08:26:08 -0700 Subject: [PATCH 1/3] fix(array): ask the typed-array question before clean_arr_ptr rejects it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fill`, `fill_range`, `reverse` and `copyWithin` each carried a #3148 TypedArray delegation, and all four sat AFTER `clean_arr_ptr_mut`. Since #7574 that funnel returns null for every tracked non-`GC_TYPE_ARRAY` object, and since the 2026-07-09 typed-array audit every typed array is a tracked `GC_TYPE_TYPED_ARRAY` allocation — so all four delegations were unreachable and the mutators silently returned the receiver unmutated. New `typed_array_receiver` (array/header.rs) answers from the raw, possibly NaN-boxed argument, before the clean. `copyWithin` also gains the Buffer/`Uint8Array` arm, which reaches it through the HIR fold whenever the receiver's static type is unknown. Unit tests: crates/perry-runtime/src/array/typed_array_receiver_tests.rs (8 tests, green). End-to-end matrix verification follows in the next commit. --- .../perry-runtime/src/array/concat_reverse.rs | 49 ++--- crates/perry-runtime/src/array/header.rs | 42 ++++ crates/perry-runtime/src/array/immutable.rs | 48 +++-- crates/perry-runtime/src/array/mod.rs | 9 +- .../src/array/typed_array_receiver_tests.rs | 201 ++++++++++++++++++ 5 files changed, 307 insertions(+), 42 deletions(-) create mode 100644 crates/perry-runtime/src/array/typed_array_receiver_tests.rs diff --git a/crates/perry-runtime/src/array/concat_reverse.rs b/crates/perry-runtime/src/array/concat_reverse.rs index 7d41884759..116042335e 100644 --- a/crates/perry-runtime/src/array/concat_reverse.rs +++ b/crates/perry-runtime/src/array/concat_reverse.rs @@ -210,16 +210,15 @@ pub extern "C" fn js_array_reverse(arr: *mut ArrayHeader) -> *mut ArrayHeader { crate::array::js_array_reverse_value(recv); return arr; } + // #3148/#2879: TypedArray receiver — reverse over element-typed storage. + // Pre-clean for the reason `typed_array_receiver` documents. + if let Some(ta) = typed_array_receiver(arr) { + return crate::typedarray::js_typed_array_reverse(ta) as *mut ArrayHeader; + } let arr = clean_arr_ptr_mut(arr); if arr.is_null() { return arr; } - // #3148: TypedArray receiver — reverse over element-typed storage. - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { - return crate::typedarray::js_typed_array_reverse( - arr as *mut crate::typedarray::TypedArrayHeader, - ) as *mut ArrayHeader; - } unsafe { let len = (*arr).length as usize; if len <= 1 { @@ -362,21 +361,20 @@ pub extern "C" fn js_array_fill(arr: *mut ArrayHeader, value: f64) -> *mut Array // other, so a later `s += x` corrupts them all. Demote once to shared (no-op // for SSO / non-string; mirrors `js_array_push_f64`, #5548). crate::string::js_string_addref_if_heap_string(value); + // #3148/#2879: TypedArray receiver — fill the whole array, element-typed. + // Asked BEFORE `clean_arr_ptr_mut`, which rejects a `TypedArrayHeader` + // outright (see `typed_array_receiver`); as a post-clean branch this was + // unreachable and `ta.fill(v)` silently did nothing. Only the 1-/0-arg + // form reaches here — codegen sends 2-/3-arg fills to + // `js_array_fill_range`, which passes the real range on. + if let Some(ta) = typed_array_receiver(arr) { + return crate::typedarray::js_typed_array_fill(ta, value, 0, 0.0, 0, 0.0) + as *mut ArrayHeader; + } let arr = clean_arr_ptr_mut(arr); if arr.is_null() { return arr; } - // #3148: TypedArray receiver — fill the whole array, element-typed. - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { - return crate::typedarray::js_typed_array_fill( - arr as *mut crate::typedarray::TypedArrayHeader, - value, - 0, - 0.0, - 0, - 0.0, - ) as *mut ArrayHeader; - } unsafe { let len = (*arr).length as usize; if len == 0 { @@ -406,21 +404,16 @@ pub extern "C" fn js_array_fill_range( // #5552: demote a uniquely-owned source string once before it fills the // range (no-op for SSO / non-string). See `js_array_fill`. crate::string::js_string_addref_if_heap_string(value); + // #3148/#2879: TypedArray receiver — fill [start, end) over element-typed + // storage. Pre-clean for the reason `typed_array_receiver` documents. + if let Some(ta) = typed_array_receiver(arr) { + return crate::typedarray::js_typed_array_fill(ta, value, 1, start, 1, end) + as *mut ArrayHeader; + } let arr = clean_arr_ptr_mut(arr); if arr.is_null() { return arr; } - // #3148: TypedArray receiver — fill [start, end) over element-typed storage. - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { - return crate::typedarray::js_typed_array_fill( - arr as *mut crate::typedarray::TypedArrayHeader, - value, - 1, - start, - 1, - end, - ) as *mut ArrayHeader; - } // ECMA-262 §23.1.3.6: ToIntegerOrInfinity(start) then (end) run BEFORE the // length==0 early-out, and each fires `valueOf` / `Symbol.toPrimitive` // (propagating abrupt completions — test262 fill/return-abrupt-from-start/ diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index aba6685dc0..638f91eb2c 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -685,6 +685,48 @@ pub(crate) fn clean_arr_ptr_mut(arr: *mut ArrayHeader) -> *mut ArrayHeader { clean_arr_ptr(arr as *const ArrayHeader) as *mut ArrayHeader } +/// Resolve a receiver a plain-array helper was handed but that is really a +/// registered %TypedArray%, so the helper can delegate to its element-typed +/// `js_typed_array_*` twin. +/// +/// **Ask this BEFORE `clean_arr_ptr`, never after.** Codegen routes +/// statically-typed typed-array receivers through the generic `js_array_*` +/// helpers on purpose (#3148 / #654 — perry-codegen `is_array_expr` answers +/// `true` for `Int32Array` &co.) on the contract that each helper +/// re-dispatches on `lookup_typed_array_kind`. But `clean_arr_ptr` *rejects* +/// those receivers, and must: since #7574 it returns null for every tracked +/// non-`GC_TYPE_ARRAY` object, because a `TypedArrayHeader`'s raw per-kind +/// storage is not boxed-f64 `ArrayHeader` slots. Since the 2026-07-09 +/// typed-array audit gave every typed array a real `GC_TYPE_TYPED_ARRAY` +/// header, that rejection fires for all of them — so a delegation written +/// after the clean is unreachable code, and its helper silently returns the +/// receiver unmutated. That is exactly how `fill`/`fill_range`/`reverse`/ +/// `copyWithin` became no-ops with no error and no diagnostic (#2879). +/// +/// Strips the NaN-box tag itself (callers may hold a `POINTER_TAG`-boxed +/// value) and never dereferences the address: the side-table probe is the +/// whole test, which also keeps the header-less legacy shapes safe. +#[inline] +pub(crate) fn typed_array_receiver( + arr: *mut ArrayHeader, +) -> Option<*mut crate::typedarray::TypedArrayHeader> { + let addr = array_receiver_addr(arr); + if addr == 0 { + return None; + } + crate::typedarray::lookup_typed_array_kind(addr) + .map(|_| addr as *mut crate::typedarray::TypedArrayHeader) +} + +/// The de-NaN-boxed address of an `Array.prototype` receiver, for side-table +/// probes only. Says nothing about what lives there — never dereference it +/// without one of the registry answers (`typed_array_receiver`, +/// `buffer::is_registered_buffer`) or a `clean_arr_ptr` round trip. +#[inline] +pub(crate) fn array_receiver_addr(arr: *mut ArrayHeader) -> usize { + crate::typedarray::strip_nanbox(arr as u64) +} + /// #5135: detect a Proxy id arriving where an `ArrayHeader` pointer is /// expected. immer's array drafts are Proxies typed (statically) as plain /// arrays, so `draft.push(x)` / `draft.length` reach the native array helpers diff --git a/crates/perry-runtime/src/array/immutable.rs b/crates/perry-runtime/src/array/immutable.rs index 616f47df61..9e45da8d31 100644 --- a/crates/perry-runtime/src/array/immutable.rs +++ b/crates/perry-runtime/src/array/immutable.rs @@ -286,25 +286,49 @@ pub extern "C" fn js_array_copy_within( has_end: i32, end: f64, ) -> *mut ArrayHeader { - let arr = clean_arr_ptr_mut(arr); - if arr.is_null() { - return arr; - } - // #3148: TypedArray receiver — copy over element-typed storage. The typed - // impl treats an undefined `end` as "to length", so pass TAG_UNDEFINED - // when no end argument was provided. - if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() { + // #3148/#2879: TypedArray receiver — copy over element-typed storage. The + // typed impl treats an undefined `end` as "to length", so pass + // TAG_UNDEFINED when no end argument was provided. Asked BEFORE + // `clean_arr_ptr_mut` for the reason `typed_array_receiver` documents — as + // a post-clean branch this was unreachable, so `ta.copyWithin(…)` was a + // silent no-op for BOTH a statically-typed receiver (codegen's + // `lower_array_method` arm) and an `any`-typed one (whose HIR + // `Expr::ArrayCopyWithin` fold lands on this same helper). + if let Some(ta) = typed_array_receiver(arr) { let end_value = if has_end != 0 { end } else { f64::from_bits(crate::value::TAG_UNDEFINED) }; - return crate::typedarray::js_typed_array_copy_within( - arr as *mut crate::typedarray::TypedArrayHeader, + return crate::typedarray::js_typed_array_copy_within(ta, target, start, end_value) + as *mut ArrayHeader; + } + // #2879: the Buffer/`Uint8Array` shape lands here too — its elements are + // single bytes in a `BufferHeader`, so the buffer dispatcher owns the copy + // (`object/buffer_dispatch.rs`'s `copyWithin` arm). Reached whenever the + // receiver's static type is unknown, because the HIR `copyWithin` fold + // (`local_array_methods.rs`) declines only for a *known* typed array and + // otherwise lowers straight to this helper. + let addr = array_receiver_addr(arr); + if crate::buffer::is_registered_buffer(addr) { + let args = [ target, start, - end_value, - ) as *mut ArrayHeader; + if has_end != 0 { + end + } else { + f64::from_bits(crate::value::TAG_UNDEFINED) + }, + ]; + let len = if has_end != 0 { 3 } else { 2 }; + unsafe { + crate::object::dispatch_buffer_method(addr, "copyWithin", args.as_ptr(), len); + } + return arr; + } + let arr = clean_arr_ptr_mut(arr); + if arr.is_null() { + return arr; } // Spec order (ECMA-262 §23.1.3.4): ToIntegerOrInfinity(target), then // (start), then (end). Each coerces via ToNumber → fires `valueOf` / diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index e820a19203..d820e6e192 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -33,6 +33,10 @@ mod spread_dense_tests; mod subclass_tests; #[cfg(test)] mod tests; +/// #2879: the in-place mutators against a %TypedArray% receiver — the shape +/// codegen actually emits for a statically-typed `Int32Array` local. +#[cfg(test)] +mod typed_array_receiver_tests; pub(crate) use self::alloc::{ array_length_range_error, js_array_alloc_pointer_elements, js_array_alloc_with_length_exact, @@ -198,6 +202,7 @@ pub(crate) use self::alloc::{js_array_from_arraylike, js_array_from_string_codep pub(crate) use self::flat_clone::{dense_spread_copy, dense_spread_source, flattenable_array_ptr}; pub(crate) use self::header::{ array_byte_size, array_is_frozen, array_is_sealed_or_no_extend, array_named_property_delete, + array_receiver_addr, array_named_property_get, array_named_property_get_by_name, array_named_property_has, array_named_property_names, array_named_property_set, array_numeric_raw_f64_get, array_numeric_raw_f64_push_inbounds, array_numeric_raw_f64_set_inbounds, array_object_flags, @@ -207,8 +212,8 @@ pub(crate) use self::header::{ mark_array_layout_unknown, mark_array_raw_f64_holes_fresh, normalize_array_receiver, note_array_slot, note_array_slot_layout_only, rebuild_array_layout, rebuild_array_layout_exact, refresh_array_numeric_layout, replay_array_growth_write_barriers, set_array_numeric_layout, - store_array_slot, transfer_array_numeric_layout, value_bits_to_number, NumericArrayLayout, - MIN_ARRAY_CAPACITY, + store_array_slot, transfer_array_numeric_layout, typed_array_receiver, + value_bits_to_number, NumericArrayLayout, MIN_ARRAY_CAPACITY, }; // Sole caller is the regex-engine-gated `regex::exec_array`, so the helper and diff --git a/crates/perry-runtime/src/array/typed_array_receiver_tests.rs b/crates/perry-runtime/src/array/typed_array_receiver_tests.rs new file mode 100644 index 0000000000..72e5771517 --- /dev/null +++ b/crates/perry-runtime/src/array/typed_array_receiver_tests.rs @@ -0,0 +1,201 @@ +//! #2879 / #7574: a %TypedArray% receiver that reaches an `Array.prototype` +//! **in-place mutator** must land on the element-typed `js_typed_array_*` +//! implementation, not fall off the end of the plain-array helper. +//! +//! ## Why this file exists +//! +//! Codegen deliberately routes typed-array receivers through the generic +//! `js_array_*` helpers — `is_array_expr` (perry-codegen +//! `type_analysis/predicates.rs`) answers `true` for `Int32Array` &co. on the +//! #3148 contract that each helper re-dispatches on +//! `lookup_typed_array_kind`. Around forty helpers in this module implement +//! their half of that contract. +//! +//! Those delegations sat **after** the shared `clean_arr_ptr` funnel, which +//! since #7574 rejects every *tracked non-array* GC object, and since the +//! 2026-07-09 typed-array audit every typed array is a tracked +//! `GC_TYPE_TYPED_ARRAY` allocation. So the four in-place mutators returned at +//! `arr.is_null()` and their typed branch became unreachable code: `fill`, +//! `reverse` and `copyWithin` silently did nothing at all, with no error and +//! no diagnostic. +//! +//! `clean_arr_ptr`'s rejection is correct and stays — a `TypedArrayHeader`'s +//! raw storage must never be read as boxed f64 `ArrayHeader` slots. What was +//! wrong is the ORDER. `typed_array_receiver` now answers the "is this a +//! typed array?" question up front, from the raw (possibly NaN-boxed) argument. +//! +//! ## What each test asserts, and how it can fail +//! +//! * `clean_arr_ptr_still_rejects_a_typed_array_receiver` pins the +//! *precondition*. If it ever goes green-by-accident (clean starts accepting +//! typed arrays) the fix below is redundant and the type-confusion #7574 +//! closed is back — so this test failing is a signal to re-read the guard, +//! not to delete the test. +//! * every mutator test asserts a **narrower-than-f64 element width** was +//! used, by storing a value that only survives per-kind truncation +//! (`70000 & 0xFFFF == 4464` for `Uint16Array`). A regression that +//! memcpy'd raw f64 slots, or that no-op'd, fails that assertion — so the +//! test cannot pass while merely "not throwing" (CLAUDE.md's fourth way a +//! gate cannot fail). +//! * the plain-`Array` controls prove the typed pre-check did not hijack the +//! ordinary path. + +use super::*; +use crate::array::{js_array_alloc, js_array_copy_within, js_array_fill, js_array_fill_range, + js_array_push_f64, js_array_reverse}; +use crate::typedarray::{TypedArrayHeader, js_typed_array_get, js_typed_array_set}; + +/// `Uint16Array` (kind 4 per `elem_size_for_kind`) — 2-byte elements, so a +/// value above 0xFFFF proves the store went through the per-kind accessor. +const UINT16: u8 = crate::typedarray::KIND_UINT16; +/// `Int32Array` — 4-byte elements, and signed, so 0x8000_0000 reads back +/// negative only if the element width was honoured. +const INT32: u8 = crate::typedarray::KIND_INT32; + +fn typed(kind: u8, values: &[f64]) -> *mut TypedArrayHeader { + let ta = crate::typedarray::typed_array_alloc(kind, values.len() as u32); + for (i, v) in values.iter().enumerate() { + js_typed_array_set(ta, i as i32, *v); + } + ta +} + +fn read_back(ta: *mut TypedArrayHeader, len: usize) -> Vec { + (0..len).map(|i| js_typed_array_get(ta, i as i32)).collect() +} + +/// A typed array handed to a plain-array helper, exactly as codegen emits it. +fn as_array(ta: *mut TypedArrayHeader) -> *mut ArrayHeader { + ta as *mut ArrayHeader +} + +#[test] +fn clean_arr_ptr_still_rejects_a_typed_array_receiver() { + let _serialized = crate::array::test_serialize(); + let ta = typed(UINT16, &[1.0, 2.0, 3.0, 4.0]); + + // The registry probe the #3148 delegations key on works fine — so a dead + // delegation is never "the typed array wasn't registered". + assert!( + crate::typedarray::lookup_typed_array_kind(ta as usize).is_some(), + "a freshly allocated typed array must be registered, or the probe \ + below proves nothing about ordering" + ); + + // ...and yet the shared receiver funnel rejects it, because it is a + // tracked GC_TYPE_TYPED_ARRAY object rather than a GC_TYPE_ARRAY one. + // This is WHY every post-clean typed branch was unreachable. + assert!( + crate::array::header::clean_arr_ptr_mut(as_array(ta)).is_null(), + "clean_arr_ptr must keep rejecting a TypedArrayHeader (#7574) — the \ + typed pre-check exists precisely because it does" + ); +} + +#[test] +fn js_array_fill_fills_a_typed_array_receiver_element_typed() { + let _serialized = crate::array::test_serialize(); + let ta = typed(UINT16, &[1.0, 2.0, 3.0, 4.0]); + let out = js_array_fill(as_array(ta), 70000.0); + assert!(!out.is_null(), "fill must return its receiver, not null"); + // 70000 truncated to 16 bits == 4464: proof the per-kind store ran. + assert_eq!(read_back(ta, 4), vec![4464.0, 4464.0, 4464.0, 4464.0]); +} + +#[test] +fn js_array_fill_range_fills_only_the_requested_range() { + let _serialized = crate::array::test_serialize(); + let ta = typed(UINT16, &[1.0, 2.0, 3.0, 4.0]); + js_array_fill_range(as_array(ta), 9.0, 0.0, 2.0); + assert_eq!( + read_back(ta, 4), + vec![9.0, 9.0, 3.0, 4.0], + "a 3-arg fill must respect [start, end) — filling the whole array is \ + the failure mode the range plumbing exists to prevent" + ); + + // Negative indices count from the end, and +Infinity (codegen's absent-end + // sentinel) clamps to length. + let ta = typed(UINT16, &[1.0, 2.0, 3.0, 4.0]); + js_array_fill_range(as_array(ta), 7.0, -2.0, f64::INFINITY); + assert_eq!(read_back(ta, 4), vec![1.0, 2.0, 7.0, 7.0]); +} + +#[test] +fn js_array_reverse_reverses_a_typed_array_receiver() { + let _serialized = crate::array::test_serialize(); + let ta = typed(INT32, &[1.0, 2.0, 3.0, 4.0]); + let out = js_array_reverse(as_array(ta)); + assert!(!out.is_null(), "reverse must return its receiver, not null"); + assert_eq!(read_back(ta, 4), vec![4.0, 3.0, 2.0, 1.0]); + + // Odd length: the middle element stays put. + let ta = typed(INT32, &[1.0, 2.0, 3.0, 4.0, 5.0]); + js_array_reverse(as_array(ta)); + assert_eq!(read_back(ta, 5), vec![5.0, 4.0, 3.0, 2.0, 1.0]); +} + +#[test] +fn js_array_copy_within_copies_typed_elements() { + let _serialized = crate::array::test_serialize(); + // `c.copyWithin(0, 2)` — no end argument (has_end == 0 means "to length"). + let ta = typed(UINT16, &[1.0, 2.0, 3.0, 4.0]); + let out = js_array_copy_within(as_array(ta), 0.0, 2.0, 0, 0.0); + assert!(!out.is_null(), "copyWithin must return its receiver, not null"); + assert_eq!(read_back(ta, 4), vec![3.0, 4.0, 3.0, 4.0]); + + // Negative target/start, and an out-of-range end that clamps to length. + let ta = typed(UINT16, &[1.0, 2.0, 3.0, 4.0]); + js_array_copy_within(as_array(ta), -2.0, -4.0, 1, 99.0); + assert_eq!(read_back(ta, 4), vec![1.0, 2.0, 1.0, 2.0]); +} + +#[test] +fn typed_mutators_honour_the_element_width_not_raw_f64_slots() { + let _serialized = crate::array::test_serialize(); + // 0x8000_0000 into an Int32Array reads back as i32::MIN. A plain-array + // f64-slot path would return 2147483648, and a no-op would return 0 — + // both distinguishable, which is what makes this a live-subject check. + let ta = typed(INT32, &[0.0, 0.0]); + js_array_fill(as_array(ta), 2147483648.0); + assert_eq!(read_back(ta, 2), vec![-2147483648.0, -2147483648.0]); +} + +// -------------------------------------------------------------------------- +// Controls: the ordinary plain-Array path must be untouched. +// -------------------------------------------------------------------------- + +fn plain(values: &[f64]) -> *mut ArrayHeader { + let mut arr = js_array_alloc(values.len() as u32); + for v in values { + arr = js_array_push_f64(arr, *v); + } + arr +} + +fn plain_read(arr: *mut ArrayHeader, len: usize) -> Vec { + (0..len) + .map(|i| crate::array::js_array_get_element(arr as i64, i as i64)) + .collect() +} + +#[test] +fn plain_array_mutators_are_unchanged_by_the_typed_pre_check() { + let _serialized = crate::array::test_serialize(); + + let arr = plain(&[1.0, 2.0, 3.0, 4.0]); + js_array_fill(arr, 9.0); + assert_eq!(plain_read(arr, 4), vec![9.0, 9.0, 9.0, 9.0]); + + let arr = plain(&[1.0, 2.0, 3.0, 4.0]); + js_array_fill_range(arr, 9.0, 0.0, 2.0); + assert_eq!(plain_read(arr, 4), vec![9.0, 9.0, 3.0, 4.0]); + + let arr = plain(&[1.0, 2.0, 3.0, 4.0, 5.0]); + js_array_reverse(arr); + assert_eq!(plain_read(arr, 5), vec![5.0, 4.0, 3.0, 2.0, 1.0]); + + let arr = plain(&[1.0, 2.0, 3.0, 4.0]); + js_array_copy_within(arr, 0.0, 2.0, 0, 0.0); + assert_eq!(plain_read(arr, 4), vec![3.0, 4.0, 3.0, 4.0]); +} From 0c73ccbea7bdbcd77555bd154c6969999676fa66 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 14 Aug 2026 08:37:26 -0700 Subject: [PATCH 2/3] docs(2879): changelog fragment + correct the HIR fold's stale destination note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the verified matrix (330 lines, 164 wrong before, 0 after) and names the same-class cases still open (sort / with / toReversed). The HIR `copyWithin` comment said typed receivers fall through "so they reach the runtime js_typed_array_copy_within arm in js_native_call_method". They do not — a typed-array NAME satisfies codegen's `is_array_expr`, so they land in `lower_array_method` and `js_array_copy_within`. Declining the fold is about the slot LAYOUT, not about reaching a particular dispatcher. --- .../2879-typed-array-mutators-no-op.md | 80 +++++++++++++++++++ .../lower/expr_call/local_array_methods.rs | 13 ++- 2 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 changelog.d/2879-typed-array-mutators-no-op.md diff --git a/changelog.d/2879-typed-array-mutators-no-op.md b/changelog.d/2879-typed-array-mutators-no-op.md new file mode 100644 index 0000000000..fcce4a2316 --- /dev/null +++ b/changelog.d/2879-typed-array-mutators-no-op.md @@ -0,0 +1,80 @@ +Fixed `fill`, `reverse` and `copyWithin` silently doing nothing on a typed array +whose type is statically known. No error, no diagnostic, the array simply came +back unchanged: + +```ts +const f = new Uint16Array(4); f[0]=1;f[1]=2;f[2]=3;f[3]=4 +f.fill(9,0,2) // was 1,2,3,4 — node gives 9,9,3,4 +const r = new Uint16Array(4); r[0]=1;r[1]=2;r[2]=3;r[3]=4 +r.reverse() // was 1,2,3,4 — node gives 4,3,2,1 +const c = new Uint16Array(4); c[0]=1;c[1]=2;c[2]=3;c[3]=4 +c.copyWithin(0,2) // was 1,2,3,4 — node gives 3,4,3,4 +``` + +**Root cause — a delegation that stopped being reachable.** Codegen routes a +statically-typed typed-array receiver through the *generic* `js_array_*` helpers +on purpose (#3148/#654: `is_array_expr` in +`crates/perry-codegen/src/type_analysis/predicates.rs` answers `true` for +`Int32Array` &co.), on the contract that each helper re-dispatches on +`lookup_typed_array_kind`. Around forty helpers in `crates/perry-runtime/src/array/` +implement their half of that contract, and the four in-place mutators wrote +theirs *after* the shared `clean_arr_ptr` receiver funnel: + +| helper | delegation was at | +|---|---| +| `js_array_fill` | `array/concat_reverse.rs` | +| `js_array_fill_range` | `array/concat_reverse.rs` | +| `js_array_reverse` | `array/concat_reverse.rs` | +| `js_array_copy_within` | `array/immutable.rs` | + +Two later changes turned that ordering into dead code. #7574 made +`clean_arr_ptr` reject every *tracked non-`GC_TYPE_ARRAY`* object — correctly, a +`TypedArrayHeader`'s raw per-kind storage is not boxed-f64 `ArrayHeader` slots — +and the 2026-07-09 typed-array audit gave every typed array a real +`GC_TYPE_TYPED_ARRAY` header (they used to be header-less side-table +allocations, which the funnel let through). From then on all four mutators +returned at their `arr.is_null()` early-out, and the typed branch below it could +never run. The failure is invisible: `fill`/`reverse`/`copyWithin` all return +their receiver, so a no-op and a success are indistinguishable at the call site. + +**The fix is the ordering, not the routing.** `clean_arr_ptr`'s rejection stays +exactly as #7574 left it. A new `typed_array_receiver` (`array/header.rs`) +answers "is this a registered typed array?" from the raw, possibly NaN-boxed +argument — a side-table probe that never dereferences the address — and each of +the four mutators consults it *before* the clean. Fixing it in codegen instead +(declining `is_array_expr` for typed arrays) was rejected: it would strand the +other ~40 delegations that the same contract depends on, and it would not fix +the `any`-typed receiver at all, because HIR's `copyWithin` fold +(`crates/perry-hir/src/lower/expr_call/local_array_methods.rs`) declines only +for a *known* typed array and otherwise lowers straight to the same runtime +helper. + +`js_array_copy_within` additionally gained the Buffer/`Uint8Array` arm it was +missing, delegating to `object/buffer_dispatch.rs`'s byte-granularity +`copyWithin`. That shape reached the helper through the same HIR fold, so +`copyWithin` on an opaquely-typed `Uint8Array` was a no-op too, even though +every statically-typed `Uint8Array` case already worked. + +**Verification.** A generated matrix of 5 programs × 66 checks — +`fill`/`reverse`/`copyWithin` × `Uint8Array`/`Uint16Array`/`Int32Array`/`Float64Array`/plain +`Array` × statically-typed / `any`-annotated / laundered-through-`any` receiver × +function scope / module scope, with 0-, 1-, 2- and 3-argument `fill`, an +odd-length `reverse`, and negative plus out-of-range `copyWithin` indices — is +now byte-identical to `node` 26.5.1 on all 330 lines. 164 of them were wrong +before: 52 each for `Uint16Array`/`Int32Array`/`Float64Array` (every mutator, +every scope, every typedness) and 8 for `Uint8Array` (opaque `copyWithin` only). +The plain-`Array` program was correct before and after. +The unit coverage is `crates/perry-runtime/src/array/typed_array_receiver_tests.rs`: +it pins the precondition (`clean_arr_ptr` still rejects a typed array, so the +pre-check is load-bearing), asserts the plain-`Array` path is untouched, and +proves the *element-typed* store actually ran by filling values that only +survive per-kind truncation (`70000` → `4464` in a `Uint16Array`, +`2147483648` → `-2147483648` in an `Int32Array`) — so no test here can pass +merely by not throwing. + +**Same class, still open.** `sort`, `with` and `toReversed` on a +statically-typed typed array are wrong for the same reason (`array/sort.rs`, +`array/immutable.rs` — post-clean delegations): `sort` leaves the array +unsorted, `with`/`toReversed` return an empty array. They are not part of this +change because each needs its own oracle matrix (comparator forms, and a +NEW-array return rather than in-place mutation). diff --git a/crates/perry-hir/src/lower/expr_call/local_array_methods.rs b/crates/perry-hir/src/lower/expr_call/local_array_methods.rs index e685077d39..09e4f8f4eb 100644 --- a/crates/perry-hir/src/lower/expr_call/local_array_methods.rs +++ b/crates/perry-hir/src/lower/expr_call/local_array_methods.rs @@ -802,9 +802,16 @@ pub(super) fn try_local_array_methods( // `Expr::ArrayCopyWithin` — that path treats the // receiver as an `ArrayHeader` with boxed f64 slots, // which is invalid for `TypedArrayHeader` raw - // storage. Fall through so they reach the runtime - // `js_typed_array_copy_within` arm in - // `js_native_call_method`. + // storage. Falling through hands them to codegen's + // `lower_array_method` `copyWithin` arm (typed-array + // names satisfy `is_array_expr`), whose + // `js_array_copy_within` re-dispatches on + // `typed_array_receiver` to the element-typed impl. + // Declining this fold is therefore about the SLOT + // LAYOUT, not about reaching a particular dispatcher: + // the un-declined `any`-typed case lands on that same + // helper, which is why the receiver check there must + // run before `clean_arr_ptr` rejects it. let is_typed_array = ctx .lookup_local_type(&arr_name) .map(|ty| { From 5cfc3fac80abcf9983fd9c12af0b3c97e48b31e8 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 14 Aug 2026 10:40:50 -0700 Subject: [PATCH 3/3] chore(changelog): key the fragment on the PR number, not the issue --- ...array-mutators-no-op.md => 8090-typed-array-mutators-no-op.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{2879-typed-array-mutators-no-op.md => 8090-typed-array-mutators-no-op.md} (100%) diff --git a/changelog.d/2879-typed-array-mutators-no-op.md b/changelog.d/8090-typed-array-mutators-no-op.md similarity index 100% rename from changelog.d/2879-typed-array-mutators-no-op.md rename to changelog.d/8090-typed-array-mutators-no-op.md