fix(array): ask the typed-array question before clean_arr_ptr rejects it, for sort/toSorted/toReversed/with

changelog.d/8119-typed-array-immutables-and-sort.md

@@ -0,0 +1,110 @@
+### Fixed
+
+- **Typed-array `sort`, `toSorted`, `toReversed` and `with` were silent no-ops
+  or returned an empty array (#8096).** #8090 fixed `fill`, ranged `fill`,
+  `reverse` and `copyWithin` by asking `array::header::typed_array_receiver()`
+  BEFORE the `clean_arr_ptr` funnel, and named these six helpers as carrying the
+  identical dead delegation:
+
+  | helper | file | clean at | delegation at |
+  |---|---|---|---|
+  | `js_array_to_reversed` | `array/immutable.rs:32` | 33 | 37 |
+  | `js_array_to_sorted_default` | `array/immutable.rs:59` | 60 | 61 |
+  | `js_array_to_sorted_with_comparator` | `array/immutable.rs:96` | 105 | 106 |
+  | `js_array_with` | `array/immutable.rs:232` | 242 | 246 |
+  | `js_array_sort_default` | `array/sort.rs:542` | 551 | 561 |
+  | `js_array_sort_with_comparator` | `array/sort.rs:635` | 651 | 658 |
+
+  Codegen routes statically-typed typed-array receivers through the generic
+  `js_array_*` helpers on purpose (#3148 / #654 — `is_array_expr` answers `true`
+  for `Int32Array` &co.) on the contract that each helper re-dispatches on
+  `lookup_typed_array_kind`. `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. So every delegation written below the clean was
+  unreachable.
+
+  Two distinct silent failures followed. In-place `sort` returned the null'd
+  pointer having sorted nothing; `toReversed` / `toSorted` / `with` returned
+  `js_array_alloc(0)` — an EMPTY PLAIN ARRAY, so `.constructor.name` was
+  `"Array"` and `JSON.stringify(Array.from(x))` was `[]`.
+
+  Measured against the pinned node oracle (`v26.5.1`, `.node-version`), both
+  arms exit 0:
+
+  ```
+                          node                perry before
+  i32 sort default    1 2 9 10            10 9 2 1        (no-op)
+  f64 sort cmp desc   10.5 9.25 2 1       1 10.5 2 9.25   (no-op)
+  i32 toSorted        1 2 9 10            undefined x4    (empty array)
+  f64 toReversed      4.5 3.5 2.5 1.5     undefined x4
+  i32 with            1 99 3 4            undefined x4
+  i8  with wraps      -56                 undefined
+  ctor names          Int32Array x3       Array x3
+  ```
+
+  The plain-`Array` controls in the same program are correct in BOTH arms
+  (`plain sort default: 1 10 2 9`, the ToString ordering), which is what makes
+  the defect typed-array-specific rather than a general mutator break.
+
+  `sort` and `toSorted` have to reach the typed helper for the ORDER as well as
+  the layout: `%TypedArray%.prototype.sort` compares NUMERICALLY (§23.2.3.29
+  CompareTypedArrayElements) where `Array.prototype.sort` compares by `ToString`.
+  `[10, 9, 2, 1]` separates all three possible answers — numeric `1, 2, 9, 10`,
+  string `1, 10, 2, 9`, no-op `10, 9, 2, 1` — and is what the new tests feed.
+  The comparator cases feed `[1, 10, 2, 9]` for the same reason: an
+  already-descending input would let a no-op pass a descending-sort assertion.
+
+- **`Uint8Array` and `Buffer` `toReversed` / `toSorted` answered an empty array
+  on every dispatch path (#8096, second receiver shape).** Perry's
+  `new Uint8Array([…])` is not a registry `TypedArrayHeader` at all —
+  `buffer::js_uint8array_new` returns a `BufferHeader`, registered as a buffer
+  and marked `mark_as_uint8array`. So `typed_array_receiver`, which is
+  registry-backed, legitimately answers `None` for the most common typed array
+  in the language, while `clean_arr_ptr` still rejects it as a tracked
+  non-array. Verified directly on the real constructor path:
+
+  ```
+  addr=0x20000b00008   is_registered_buffer=true   lookup_typed_array_kind=None
+  gc_header_obj_type=Some(10)   clean_arr_ptr_null=true
+  js_array_to_reversed(...).length = 0
+  ```
+
+  Most `Array.prototype` entry points never see this: `sort` / `with` /
+  `reverse` / `fill` on that shape resolve through the dynamic method
+  dispatcher, and `copyWithin` grew its own Buffer arm in #8090. But
+  `toReversed` and `toSorted` fold unconditionally in HIR
+  (`lower/expr_call/local_array_methods.rs` — no receiver-type guard), and the
+  dynamic tower's own `toReversed` / `toSorted` arms
+  (`object/native_call_method/handle_methods.rs`) call straight back into these
+  same helpers, so those two were wrong for `Uint8Array` and `Buffer` on every
+  dispatch path, static and dynamic:
+
+  ```
+  ann u8 toReversed:  node 4 9 2 10 1 [object Uint8Array]
+                     perry 0 undefined undefined undefined undefined [object Array]
+  ```
+
+  New `buffer_receiver_as_uint8_typed_array` (`array/header.rs`) resolves that
+  shape into a fresh `KIND_UINT8` %TypedArray% copy. It is sound only for the
+  IMMUTABLE methods — a copy-based in-place `sort` would sort the copy and leave
+  the receiver untouched, a different wrong answer — so `toReversed`,
+  `toSorted` and `with` use it and `js_array_sort_*` deliberately does not. It
+  declines `ArrayBuffer` / `SharedArrayBuffer` / `DataView` receivers, which
+  have no `%TypedArray%.prototype` in node.
+
+  Validation: five probes byte-identical to node `v26.5.1`, all exit 0 in both
+  arms. 12 tests added to
+  `crates/perry-runtime/src/array/typed_array_receiver_tests.rs` (19 in the file
+  with #8090's), sabotage-verified in two stages after real rebuilds —
+  reverting `immutable.rs` + `sort.rs` turns 6 of them red, neutering
+  `buffer_receiver_as_uint8_typed_array` turns the other 3 red. The 3 that stay
+  green in both arms are the intended controls: the plain-`Array` control, the
+  `ArrayBuffer` / `DataView` decline, and the registry precondition the Buffer
+  arm exists for (#8090's `clean_arr_ptr_still_rejects_a_typed_array_receiver`
+  keeps #7574 pinned alongside them). `cargo test -p perry-runtime --lib`: 2380 passed / 0 failed / 4 ignored
+  (`main`: 2361 / 0 / 4). `perry-codegen`: 1434 passed / 11 failed, identical to
+  the `main` baseline (#8092).
+
+  `js_array_to_spliced` needs no equivalent — `%TypedArray%.prototype` has no
+  `toSpliced`.

crates/perry-runtime/src/array/header.rs

@@ -718,6 +718,69 @@ pub(crate) fn typed_array_receiver(
         .map(|_| addr as *mut crate::typedarray::TypedArrayHeader)
 }
 
+/// #8096: resolve a receiver that is a registered `Buffer` / `Uint8Array`
+/// into a fresh `KIND_UINT8` %TypedArray% COPY, so an `Array.prototype`
+/// helper can delegate to a `js_typed_array_*` twin that only accepts a
+/// `TypedArrayHeader`.
+///
+/// [`typed_array_receiver`] does not answer for the most common typed array
+/// in the language. Perry's `new Uint8Array([…])` returns a `BufferHeader`
+/// (`buffer::js_uint8array_new`), registered as a BUFFER and marked
+/// `mark_as_uint8array` — it is not in the typed-array registry at all. The
+/// receiver is still a tracked non-`GC_TYPE_ARRAY` allocation, so
+/// `clean_arr_ptr` rejects it exactly as it rejects a real
+/// `GC_TYPE_TYPED_ARRAY`, and the caller answers an EMPTY plain array.
+///
+/// Most `Array.prototype` entry points never see that: `sort` / `with` /
+/// `reverse` / `fill` on a Buffer-backed `Uint8Array` resolve through the
+/// dynamic method dispatcher, and `copyWithin` grew its own Buffer arm in
+/// #8090. `toReversed` and `toSorted` fold unconditionally in HIR
+/// (`lower/expr_call/local_array_methods.rs` — no receiver-type guard), and
+/// the dynamic tower's own `toReversed` / `toSorted` arms
+/// (`object/native_call_method/handle_methods.rs`) call straight back into
+/// these same helpers, so those two were wrong on EVERY dispatch path.
+///
+/// **Only sound for the IMMUTABLE methods.** The answer is a copy, so an
+/// in-place mutator delegating to it would sort/reverse the copy and leave
+/// the receiver untouched — a different wrong answer. `toReversed`,
+/// `toSorted` and `with` all return a new collection, which is why they can
+/// use it; `js_array_sort_*` deliberately does not.
+///
+/// `None` for an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` receiver:
+/// none of those has `%TypedArray%.prototype`, so node throws
+/// `TypeError: … is not a function` rather than answering elements.
+#[inline]
+pub(crate) fn buffer_receiver_as_uint8_typed_array(
+    arr: *mut ArrayHeader,
+) -> Option<*mut crate::typedarray::TypedArrayHeader> {
+    let addr = array_receiver_addr(arr);
+    if addr == 0
+        || !crate::buffer::is_registered_buffer(addr)
+        || crate::buffer::is_any_array_buffer(addr)
+        || crate::buffer::is_data_view(addr)
+    {
+        return None;
+    }
+    // Copy the bytes out BEFORE allocating: `typed_array_alloc` can collect,
+    // and a raw payload pointer read across it is exactly the borrowed-heap-
+    // slice shape rooting cannot fix.
+    let buf = addr as *const crate::buffer::BufferHeader;
+    let bytes: Vec<u8> = unsafe {
+        let len = (*buf).length as usize;
+        if len == 0 {
+            Vec::new()
+        } else {
+            std::slice::from_raw_parts(crate::buffer::buffer_data(buf), len).to_vec()
+        }
+    };
+    let ta =
+        crate::typedarray::typed_array_alloc(crate::typedarray::KIND_UINT8, bytes.len() as u32);
+    for (i, byte) in bytes.iter().enumerate() {
+        crate::typedarray::js_typed_array_set(ta, i as i32, f64::from(*byte));
+    }
+    Some(ta)
+}
+
 /// 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`,

crates/perry-runtime/src/array/immutable.rs

@@ -30,15 +30,29 @@ fn throw_invalid_index(index: f64) -> ! {
 /// `arr.toReversed()` — return a new reversed copy (immutable)
 #[no_mangle]
 pub extern "C" fn js_array_to_reversed(arr: *const ArrayHeader) -> *mut ArrayHeader {
+    // #3148/#2879/#8096: %TypedArray% receiver — reverse over element-typed
+    // storage. Asked BEFORE `clean_arr_ptr` for the reason
+    // `typed_array_receiver` documents: written after the clean this branch is
+    // unreachable, because since #7574 the funnel returns null for every
+    // tracked non-`GC_TYPE_ARRAY` object and every typed array is a tracked
+    // `GC_TYPE_TYPED_ARRAY` one. Reached that way the helper answered an EMPTY
+    // plain array, not the receiver's reversed elements.
+    if let Some(ta) = typed_array_receiver(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_to_reversed(ta) as *mut ArrayHeader;
+    }
+    // #8096: the Buffer-backed `Uint8Array` shape — the SAME symptom from a
+    // different receiver. `new Uint8Array([…])` is a registered BufferHeader,
+    // so `typed_array_receiver` legitimately answers `None` and the clean
+    // below rejects it as a tracked non-array. See
+    // `buffer_receiver_as_uint8_typed_array` for why this arm is sound only
+    // for the immutable methods.
+    if let Some(ta) = buffer_receiver_as_uint8_typed_array(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_to_reversed(ta) as *mut ArrayHeader;
+    }
     let arr = clean_arr_ptr(arr);
     if arr.is_null() {
         return js_array_alloc(0);
     }
-    if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() {
-        return crate::typedarray::js_typed_array_to_reversed(
-            arr as *const crate::typedarray::TypedArrayHeader,
-        ) as *mut ArrayHeader;
-    }
     unsafe {
         let len = (*arr).length as usize;
         let new_arr = js_array_alloc(len as u32);
@@ -57,12 +71,30 @@ pub extern "C" fn js_array_to_reversed(arr: *const ArrayHeader) -> *mut ArrayHea
 /// `arr.toSorted()` — return a new sorted copy (default string sort, immutable)
 #[no_mangle]
 pub extern "C" fn js_array_to_sorted_default(arr: *const ArrayHeader) -> *mut ArrayHeader {
-    let arr = clean_arr_ptr(arr);
-    if !arr.is_null() && crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() {
-        return crate::typedarray::js_typed_array_to_sorted_default(
-            arr as *const crate::typedarray::TypedArrayHeader,
-        ) as *mut ArrayHeader;
+    // #3148/#2879/#8096: %TypedArray% receiver — sort over element-typed
+    // storage. Asked BEFORE `clean_arr_ptr` for the reason
+    // `typed_array_receiver` documents: written after the clean this branch is
+    // unreachable, because since #7574 the funnel returns null for every
+    // tracked non-`GC_TYPE_ARRAY` object and every typed array is a tracked
+    // `GC_TYPE_TYPED_ARRAY` one.
+    // The typed twin also carries the right ORDER: `%TypedArray%.prototype.
+    // toSorted` with no comparator sorts NUMERICALLY (§23.2.3.32 ->
+    // CompareTypedArrayElements), where `Array.prototype.toSorted` sorts by
+    // ToString. Falling through to the plain-array body would be the string
+    // order even if the slots were readable (#8096).
+    if let Some(ta) = typed_array_receiver(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_to_sorted_default(ta) as *mut ArrayHeader;
     }
+    // #8096: the Buffer-backed `Uint8Array` shape — the SAME symptom from a
+    // different receiver. `new Uint8Array([…])` is a registered BufferHeader,
+    // so `typed_array_receiver` legitimately answers `None` and the clean
+    // below rejects it as a tracked non-array. See
+    // `buffer_receiver_as_uint8_typed_array` for why this arm is sound only
+    // for the immutable methods.
+    if let Some(ta) = buffer_receiver_as_uint8_typed_array(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_to_sorted_default(ta) as *mut ArrayHeader;
+    }
+    let arr = clean_arr_ptr(arr);
     if arr.is_null() {
         return js_array_alloc(0);
     }
@@ -102,13 +134,27 @@ pub extern "C" fn js_array_to_sorted_with_comparator(
     if comparator.is_null() {
         return js_array_to_sorted_default(arr);
     }
-    let arr = clean_arr_ptr(arr);
-    if !arr.is_null() && crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() {
-        return crate::typedarray::js_typed_array_to_sorted_with_comparator(
-            arr as *const crate::typedarray::TypedArrayHeader,
-            comparator,
-        ) as *mut ArrayHeader;
+    // #3148/#2879/#8096: %TypedArray% receiver — sort over element-typed
+    // storage. Asked BEFORE `clean_arr_ptr` for the reason
+    // `typed_array_receiver` documents: written after the clean this branch is
+    // unreachable, because since #7574 the funnel returns null for every
+    // tracked non-`GC_TYPE_ARRAY` object and every typed array is a tracked
+    // `GC_TYPE_TYPED_ARRAY` one.
+    if let Some(ta) = typed_array_receiver(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_to_sorted_with_comparator(ta, comparator)
+            as *mut ArrayHeader;
+    }
+    // #8096: the Buffer-backed `Uint8Array` shape — the SAME symptom from a
+    // different receiver. `new Uint8Array([…])` is a registered BufferHeader,
+    // so `typed_array_receiver` legitimately answers `None` and the clean
+    // below rejects it as a tracked non-array. See
+    // `buffer_receiver_as_uint8_typed_array` for why this arm is sound only
+    // for the immutable methods.
+    if let Some(ta) = buffer_receiver_as_uint8_typed_array(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_to_sorted_with_comparator(ta, comparator)
+            as *mut ArrayHeader;
     }
+    let arr = clean_arr_ptr(arr);
     if arr.is_null() {
         return js_array_alloc(0);
     }
@@ -239,17 +285,30 @@ pub extern "C" fn js_array_with(
     // source local can't corrupt it. No-op for SSO / non-string. The cloned
     // elements come from `arr` and are already shared. (Mirrors #5548.)
     crate::string::js_string_addref_if_heap_string(value);
+    // #3148/#2879/#8096: %TypedArray% receiver — replace one element over
+    // element-typed storage. Asked BEFORE `clean_arr_ptr` for the reason
+    // `typed_array_receiver` documents: written after the clean this branch is
+    // unreachable, because since #7574 the funnel returns null for every
+    // tracked non-`GC_TYPE_ARRAY` object and every typed array is a tracked
+    // `GC_TYPE_TYPED_ARRAY` one.
+    // The typed twin also owns the per-kind coercion of `value` (a `300` into
+    // a `Uint8Array` must read back `44`) and the typed RangeError text.
+    if let Some(ta) = typed_array_receiver(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_with(ta, index, value) as *mut ArrayHeader;
+    }
+    // #8096: the Buffer-backed `Uint8Array` shape — the SAME symptom from a
+    // different receiver. `new Uint8Array([…])` is a registered BufferHeader,
+    // so `typed_array_receiver` legitimately answers `None` and the clean
+    // below rejects it as a tracked non-array. See
+    // `buffer_receiver_as_uint8_typed_array` for why this arm is sound only
+    // for the immutable methods.
+    if let Some(ta) = buffer_receiver_as_uint8_typed_array(arr as *mut ArrayHeader) {
+        return crate::typedarray::js_typed_array_with(ta, index, value) as *mut ArrayHeader;
+    }
     let arr = clean_arr_ptr(arr);
     if arr.is_null() {
         return js_array_alloc(0);
     }
-    if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() {
-        return crate::typedarray::js_typed_array_with(
-            arr as *const crate::typedarray::TypedArrayHeader,
-            index,
-            value,
-        ) as *mut ArrayHeader;
-    }
     unsafe {
         let len = (*arr).length as isize;
         // ECMA ToIntegerOrInfinity: NaN coerces to 0, ±Infinity stay infinite.

crates/perry-runtime/src/array/mod.rs

@@ -206,13 +206,13 @@ pub(crate) use self::header::{
     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,
     array_object_flags_from_tag, array_ptr_as_proxy, array_receiver_addr, array_receiver_gc_tag,
-    canonicalize_array_numeric_store_value, clean_arr_ptr, clean_arr_ptr_mut,
-    clear_array_numeric_layout, clear_array_numeric_layout_ptr, 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, 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, typed_array_receiver, value_bits_to_number,
-    NumericArrayLayout, MIN_ARRAY_CAPACITY,
+    buffer_receiver_as_uint8_typed_array, canonicalize_array_numeric_store_value, clean_arr_ptr,
+    clean_arr_ptr_mut, clear_array_numeric_layout, clear_array_numeric_layout_ptr,
+    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, 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,
+    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