fix(typedarray): dispatch, not drop, when a Uint8Array-specialized element helper's receiver is not one

changelog.d/8120-uint8array-specialized-dispatch.md

@@ -0,0 +1,74 @@
+### Fixed
+
+- **Reassigning a `Uint8Array` binding dropped element writes and read
+  `undefined` (#8111).** The `Uint8Array`-specialized twin of #8100.
+  `js_uint8array_get`, `js_uint8array_index_get_value` and `js_uint8array_set`
+  (`typedarray/access.rs`) are a SEPARATE emission path from the two helpers
+  #8109 fixed: codegen picks them from `is_uint8array_receiver`
+  (`perry-codegen/src/expr/index_{get,set}.rs`), which reads
+  `receiver_class_name` rather than the `local_type_hint` predicate #8100 is
+  about — but it fires for a reassigned `Uint8Array` local just the same.
+
+  Each had a three-way shape (registered typed array of the right kind /
+  registered buffer / fall off the end) and TWO of those arms answered for a
+  receiver that is perfectly readable: the trailing arm (a plain array, object
+  or string) and the wrong-KIND arm (a registered typed array that is not
+  `Uint8Array` / `Uint8ClampedArray`). Reads answered `0` / `undefined`; the
+  store was dropped with no trace at all. Unlike #8100 these helpers were
+  memory-SAFE — they validated before dereferencing — they just answered
+  wrongly, silently, in the shipped default configuration.
+
+  Measured against the pinned node oracle (`v26.5.1`, `.node-version`), both
+  arms exit 0:
+
+  ```
+                       node             perry before
+  store:               [5,10]           [9,10]                 <- write DROPPED
+  dyn:                 10               undefined
+  read:                5 10 2 10        undefined undefined 2 10
+  obj store+read:      42 8             undefined undefined
+  string read:         h i              undefined undefined
+  wrong-kind:          77 12 2          undefined undefined 2
+  ```
+
+  Correct in BOTH arms: a real `Uint8Array` (`3 250 2`), a `Buffer`
+  (`200 2`), a plain array (`5 10`), a plain object (`5 10`), a
+  `Uint8ClampedArray` (`255 2`). `Q.length` and `Q.at(1)` were already right, so
+  the binding really did hold the plain array — only the specialized element
+  accessors were wrong.
+
+  The read arms now delegate to `js_typed_array_get`, which owns #8109's
+  `classify_element_read_receiver` dispatch, so this path INHERITS it rather
+  than growing a third classifier — including the property that a
+  `GC_TYPE_TYPED_ARRAY` / `GC_TYPE_NATIVE_TYPED_VIEW` header still wins on a
+  registry miss, so a lookup failure can cost the diversion but never the
+  element read. The store arm asks the same classifier directly, because
+  `js_dyn_index_set` has its own return-value contract and the value arrives as
+  an `i32`. `js_uint8array_get`'s ABI is a byte-typed `i32`, so a recovered
+  value there goes through `ToNumber` rather than having its NaN-box bits
+  reinterpreted, and `undefined` still collapses to the `0` byte sentinel
+  (#6088).
+
+  The wrong-KIND arms are REMOVED rather than kept as the issue suggested:
+  `js_typed_array_{get,set}` are kind-generic (they read `(*ta).kind`), node
+  reads and writes the real element there, and `js_typed_array_set` even
+  performs the spec's `ToBigInt` `TypeError` for a Number written into a bigint
+  view. `Uint8Array` / `Uint8ClampedArray` behaviour is unchanged and pinned by
+  two control tests (`300 -> 44` wrapping, `300 -> 255` clamping).
+
+  RESIDUAL, documented in-code and not introduced here: codegen narrows
+  `js_uint8array_set`'s value to `i32` at the call site (`fptosi`,
+  `perry-codegen/src/expr/arrays_finds.rs`) because the DECLARED receiver is a
+  byte view, so a fractional value written through a reassigned binding arrives
+  already truncated — `Q[0] = 1.5` stores `1` where node stores `1.5`. Closing
+  that needs an f64-valued entry point plus a codegen change, not a
+  runtime-side dispatch. Every integer store is exact.
+
+  Validation: the issue's probe is byte-identical to node post-fix. 7 unit tests
+  in `crates/perry-runtime/src/typedarray/element_read_receiver_tests.rs`, each
+  asserting the recovered VALUE rather than "did not panic";
+  sabotage-verified after a real rebuild — reverting `access.rs` turns 4 of them
+  red, and the 3 that stay green are the intended controls.
+  `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).

crates/perry-runtime/src/typedarray/access.rs

@@ -570,24 +570,36 @@ pub extern "C" fn js_typed_array_copy_within(
     ta
 }
 
+/// `uint8array[i]` / `buffer[i]` read as a raw byte. See
+/// [`js_uint8array_index_get_value`] for the JS-value twin and for the #8111
+/// stale-static-hint dispatch both share.
 #[no_mangle]
 pub extern "C" fn js_uint8array_get(target: *const TypedArrayHeader, index: i32) -> i32 {
     let addr = strip_nanbox(target as u64);
     if addr < 0x1000 || index < 0 {
         return 0;
     }
-    if let Some(kind) = lookup_typed_array_kind(addr) {
-        if !matches!(kind, KIND_UINT8 | KIND_UINT8_CLAMPED) {
-            return 0;
-        }
-        let value = js_typed_array_get(addr as *const TypedArrayHeader, index);
-        if value.to_bits() == crate::value::TAG_UNDEFINED {
-            0
-        } else {
-            value as i32
-        }
+    let value = if lookup_typed_array_kind(addr).is_some() {
+        js_typed_array_get(addr as *const TypedArrayHeader, index)
     } else if crate::buffer::is_registered_buffer(addr) {
-        crate::buffer::js_buffer_get(addr as *const crate::buffer::BufferHeader, index)
+        return crate::buffer::js_buffer_get(addr as *const crate::buffer::BufferHeader, index);
+    } else {
+        // #8111: not a registered typed array and not a registered buffer, so
+        // the `Uint8Array` / `Buffer` static type codegen keyed on was a stale
+        // hint. `js_typed_array_get` performs #8109's
+        // `classify_element_read_receiver` dispatch on our behalf.
+        js_typed_array_get(addr as *const TypedArrayHeader, index)
+    };
+    // This accessor's ABI is a byte-typed i32, so an `undefined` (absent
+    // receiver, or out of range) collapses to the `0` byte sentinel the caller
+    // expects (#6088), and a recovered non-number element takes ToNumber
+    // rather than having its NaN-box bits reinterpreted as an integer.
+    if value.to_bits() == crate::value::TAG_UNDEFINED {
+        return 0;
+    }
+    let n = jsvalue_to_f64(value);
+    if n.is_finite() {
+        n as i32
     } else {
         0
     }
@@ -601,6 +613,14 @@ pub extern "C" fn js_uint8array_get(target: *const TypedArrayHeader, index: i32)
 /// i32 accessor is forced to return (#6088). `js_typed_array_get` and
 /// `js_buffer_index_get_value` both already yield `undefined` for out-of-range,
 /// so each arm forwards directly.
+///
+/// #8111: codegen picks this helper from `is_uint8array_receiver`
+/// (`perry-codegen/src/expr/index_get.rs`), which reads `receiver_class_name`
+/// and so still fires for a local DECLARED `Uint8Array` whose binding was
+/// reassigned — the `Uint8Array`-specialized twin of the #8100 defect. Two
+/// arms used to answer `undefined` for a receiver that is perfectly
+/// readable: a registered typed array of the WRONG kind, and anything the
+/// registry does not know at all. Both now dispatch.
 #[no_mangle]
 pub extern "C" fn js_uint8array_index_get_value(
     target: *const TypedArrayHeader,
@@ -611,15 +631,18 @@ pub extern "C" fn js_uint8array_index_get_value(
     if addr < 0x1000 || index < 0 {
         return undefined;
     }
-    if let Some(kind) = lookup_typed_array_kind(addr) {
-        if !matches!(kind, KIND_UINT8 | KIND_UINT8_CLAMPED) {
-            return undefined;
-        }
+    if lookup_typed_array_kind(addr).is_some() {
         js_typed_array_get(addr as *const TypedArrayHeader, index)
     } else if crate::buffer::is_registered_buffer(addr) {
         crate::buffer::js_buffer_index_get_value(addr as *const crate::buffer::BufferHeader, index)
     } else {
-        undefined
+        // #8111: the `Uint8Array` static type was a stale hint — the binding
+        // was reassigned. Answering `undefined` here dropped the real element.
+        // `js_typed_array_get` owns #8109's `classify_element_read_receiver`
+        // dispatch, so this arm inherits it: header-wins-on-registry-miss for
+        // a real typed array, ordinary `[[Get]]` for a plain array / object /
+        // string, `undefined` for a masked-away non-pointer.
+        js_typed_array_get(addr as *const TypedArrayHeader, index)
     }
 }
 
@@ -631,18 +654,53 @@ pub extern "C" fn js_uint8array_index_get_value(
 static KEEP_JS_UINT8ARRAY_INDEX_GET_VALUE: extern "C" fn(*const TypedArrayHeader, i32) -> f64 =
     js_uint8array_index_get_value;
 
+/// `uint8array[i] = v` / `buffer[i] = v`. See
+/// [`js_uint8array_index_get_value`] for the #8111 stale-static-hint dispatch
+/// this shares, and the trailing arm below for the one residual its i32 value
+/// ABI cannot close.
 #[no_mangle]
 pub extern "C" fn js_uint8array_set(target: *mut TypedArrayHeader, index: i32, value: i32) {
     let addr = strip_nanbox(target as u64);
     if addr < 0x1000 || index < 0 {
         return;
     }
-    if let Some(kind) = lookup_typed_array_kind(addr) {
-        if !matches!(kind, KIND_UINT8 | KIND_UINT8_CLAMPED) {
-            return;
-        }
-        js_typed_array_set(addr as *mut TypedArrayHeader, index, value as f64);
+    if lookup_typed_array_kind(addr).is_some() {
+        js_typed_array_set(addr as *mut TypedArrayHeader, index, f64::from(value));
     } else if crate::buffer::is_registered_buffer(addr) {
         crate::buffer::js_buffer_set(addr as *mut crate::buffer::BufferHeader, index, value);
+    } else {
+        // #8111, store side. The read helpers above recover the element; a
+        // dropped STORE is the same defect one call earlier, and it is the
+        // more damaging half — `Q[0] = 5` left no trace at all.
+        //
+        // The receiver question is the one #8109 already answers, so ask it
+        // the same way rather than adding a third classifier. Only a
+        // POSITIVELY identified receiver is diverted:
+        //
+        // * `TypedArray` — a `GC_TYPE_TYPED_ARRAY` / `GC_TYPE_NATIVE_TYPED_
+        //   VIEW` header the registry missed; `js_typed_array_set` is
+        //   kind-generic (it reads `(*ta).kind`).
+        // * `Ordinary` — a plain array, object, or anything else indexable.
+        //   `js_dyn_index_set` is the `[[Set]]` this access would have taken
+        //   if the stale static hint had never existed.
+        // * `Absent` — nothing indexable; drop the store, which is what a
+        //   write through a non-object primitive does in sloppy mode.
+        //
+        // RESIDUAL (pre-existing, not introduced here): codegen narrows this
+        // helper's `value` to i32 at the call site (`fptosi`, perry-codegen
+        // `expr/arrays_finds.rs`'s `Uint8ArraySet` arm) because the declared
+        // receiver is a byte view. A FRACTIONAL value written through a
+        // reassigned binding therefore arrives already truncated, so
+        // `Q[0] = 1.5` stores `1` where node stores `1.5`. Closing that needs
+        // an f64-valued entry point, not a runtime-side dispatch.
+        match crate::typedarray::classify_element_read_receiver(target as u64) {
+            ElementReadReceiver::TypedArray(ta) => {
+                js_typed_array_set(ta as *mut TypedArrayHeader, index, f64::from(value));
+            }
+            ElementReadReceiver::Ordinary(receiver) => {
+                crate::value::js_dyn_index_set(receiver, f64::from(index), f64::from(value));
+            }
+            ElementReadReceiver::Absent => {}
+        }
     }
 }

crates/perry-runtime/src/typedarray/element_read_receiver_tests.rs

@@ -219,3 +219,143 @@ fn classify_element_read_receiver_rejects_garbage_bits() {
         ElementReadReceiver::Absent
     ));
 }
+
+// --------------------------------------------------------------------------
+// #8111: the `Uint8Array`-specialized twin of the same defect.
+//
+// `js_uint8array_get` / `js_uint8array_index_get_value` / `js_uint8array_set`
+// are a SEPARATE emission path: codegen picks them from
+// `is_uint8array_receiver` (`perry-codegen/src/expr/index_{get,set}.rs`),
+// which keys on `receiver_class_name` rather than the `local_type_hint`
+// predicate #8100 is about — but it fires for a reassigned `Uint8Array` local
+// just the same. Each helper had a three-way shape (registered typed array of
+// the right kind / registered buffer / fall off the end) and TWO of those
+// arms answered for a receiver that is perfectly readable:
+//
+//   * the trailing arm — a plain array or object — answered `0` /
+//     `undefined` / dropped the store;
+//   * the wrong-KIND arm — a registered typed array that is not
+//     `Uint8Array` / `Uint8ClampedArray` — did the same, although
+//     `js_typed_array_get` / `js_typed_array_set` are kind-generic and node
+//     reads and writes the real element there.
+//
+// The store half matters most: `Q[0] = 5` left no trace at all.
+//
+// Each test asserts the RECOVERED VALUE, never merely "did not panic". The
+// pre-fix code answers `0` / `undefined` / no-op for every one of them, so
+// none can pass against the old body.
+// --------------------------------------------------------------------------
+
+use crate::typedarray::access::{
+    js_uint8array_get, js_uint8array_index_get_value, js_uint8array_set,
+};
+
+/// A plain array handed to a `Uint8Array`-specialized helper, exactly as
+/// codegen emits it (`unbox_to_i64` masks the NaN-box tag off).
+fn as_u8(arr: *mut ArrayHeader) -> *const TypedArrayHeader {
+    as_typed(arr)
+}
+
+/// A registered typed array with the NaN-box tag masked off, the shape every
+/// one of these helpers is actually handed.
+fn as_recv(ta: *mut TypedArrayHeader) -> *const TypedArrayHeader {
+    ((ta as u64) & POINTER_MASK) as *const TypedArrayHeader
+}
+
+#[test]
+fn js_uint8array_index_get_value_reads_a_plain_array_receiver() {
+    let _serialized = crate::array::test_serialize();
+    let arr = plain_array(&[9.0, 10.0]);
+    assert_eq!(js_uint8array_index_get_value(as_u8(arr), 0), 9.0);
+    assert_eq!(js_uint8array_index_get_value(as_u8(arr), 1), 10.0);
+    // Out of range is `undefined`, the IntegerIndexedExotic answer node
+    // prints — not the `0` byte sentinel.
+    assert!(is_undefined(js_uint8array_index_get_value(as_u8(arr), 2)));
+}
+
+#[test]
+fn js_uint8array_set_stores_into_a_plain_array_receiver() {
+    let _serialized = crate::array::test_serialize();
+    let arr = plain_array(&[9.0, 10.0]);
+    js_uint8array_set(as_u8(arr) as *mut TypedArrayHeader, 0, 5);
+    assert_eq!(
+        js_uint8array_index_get_value(as_u8(arr), 0),
+        5.0,
+        "the store must land in the plain array — it was silently dropped"
+    );
+    // And it is visible through the ordinary array accessor too, i.e. it is a
+    // real `[[Set]]` and not a shadow write somewhere else.
+    assert_eq!(crate::array::js_array_get_element(arr as i64, 0), 5.0);
+    assert_eq!(crate::array::js_array_get_element(arr as i64, 1), 10.0);
+}
+
+#[test]
+fn js_uint8array_get_reads_a_plain_array_receiver_as_a_byte() {
+    let _serialized = crate::array::test_serialize();
+    let arr = plain_array(&[9.0, 10.0]);
+    assert_eq!(js_uint8array_get(as_u8(arr), 0), 9);
+    assert_eq!(js_uint8array_get(as_u8(arr), 1), 10);
+    // This accessor's ABI is a byte-typed i32, so out of range stays the `0`
+    // sentinel (#6088) rather than becoming `undefined`.
+    assert_eq!(js_uint8array_get(as_u8(arr), 2), 0);
+}
+
+#[test]
+fn uint8_helpers_read_and_write_a_wrong_kind_typed_array() {
+    let _serialized = crate::array::test_serialize();
+    // A real Int32Array behind a `Uint8Array` static hint. `js_typed_array_
+    // {get,set}` are kind-generic, so there is nothing unsafe about serving
+    // it — the old `!matches!(kind, UINT8 | UINT8_CLAMPED)` arm just answered
+    // `undefined` and dropped the store. node reads and writes the element.
+    let ta = typed(KIND_INT32, &[11.0, 12.0]);
+    let recv = as_recv(ta);
+
+    assert_eq!(js_uint8array_index_get_value(recv, 1), 12.0);
+    js_uint8array_set(recv as *mut TypedArrayHeader, 0, 77);
+    assert_eq!(js_uint8array_index_get_value(recv, 0), 77.0);
+    // Through the kind-correct accessor as well: the lane really holds 77.
+    assert_eq!(js_typed_array_get(ta, 0), 77.0);
+}
+
+#[test]
+fn js_uint8array_index_get_value_is_undefined_for_a_non_pointer_receiver() {
+    let _serialized = crate::array::test_serialize();
+    // `Q = 42 as any` — codegen masks the tag off, so the helper sees a small
+    // integer. Nothing indexable: `undefined`, and a store is dropped.
+    let bogus = 42u64 as *const TypedArrayHeader;
+    assert!(is_undefined(js_uint8array_index_get_value(bogus, 0)));
+    assert_eq!(js_uint8array_get(bogus, 0), 0);
+    js_uint8array_set(bogus as *mut TypedArrayHeader, 0, 5);
+}
+
+// --------------------------------------------------------------------------
+// Controls: the receivers these helpers were WRITTEN for must be untouched.
+// --------------------------------------------------------------------------
+
+#[test]
+fn uint8_helpers_still_serve_a_real_uint8_array() {
+    let _serialized = crate::array::test_serialize();
+    let ta = typed(KIND_UINT8, &[3.0, 4.0]);
+    let recv = as_recv(ta);
+
+    assert_eq!(js_uint8array_index_get_value(recv, 0), 3.0);
+    assert_eq!(js_uint8array_get(recv, 1), 4);
+    js_uint8array_set(recv as *mut TypedArrayHeader, 1, 250);
+    assert_eq!(js_uint8array_index_get_value(recv, 1), 250.0);
+    // A Uint8Array lane is 1 byte: 300 wraps to 44. If the store had been
+    // diverted to a plain-array `[[Set]]` it would read back 300.
+    js_uint8array_set(recv as *mut TypedArrayHeader, 0, 300);
+    assert_eq!(js_uint8array_index_get_value(recv, 0), 44.0);
+    assert!(is_undefined(js_uint8array_index_get_value(recv, 2)));
+}
+
+#[test]
+fn uint8_helpers_still_serve_a_uint8_clamped_array() {
+    let _serialized = crate::array::test_serialize();
+    let ta = typed(KIND_UINT8_CLAMPED, &[1.0, 2.0]);
+    let recv = as_recv(ta);
+    // Clamped, not wrapped: 300 -> 255. The kind-specific store is still the
+    // one running.
+    js_uint8array_set(recv as *mut TypedArrayHeader, 0, 300);
+    assert_eq!(js_uint8array_index_get_value(recv, 0), 255.0);
+}