From 5e1b87f0ceb4081991f05cd32eacd2c03f0ef2b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 01:05:34 +0200 Subject: [PATCH 1/2] fix(buffer): ask the receiver-kind question above the byte and toLocaleString arms Closes #8149. Closes #8139 (part 1). Perry backs four distinct JS types with the same `BufferHeader` and the same `BUFFER_REGISTRY` entry: a node `Buffer`, a `Uint8Array`, an `ArrayBuffer`/`SharedArrayBuffer`, and a `DataView`. Every consumer that triaged a receiver as "registered buffer => byte indexable" served all four, so a `DataView` and a raw `ArrayBuffer` answered bytes for `[i]`, `in`, `.length` and `hasOwnProperty`, an index STORE overwrote a byte where node creates an ordinary property, and `JSON.stringify` leaked the backing bytes as `{"type":"Buffer","data":[...]}`. The enumeration paths had no registered-buffer arm at all and walked a `BufferHeader` as an `ObjectHeader`, reading payload bytes as `keys_array`. That answered `[]` when those bytes were zero -- which is why `Object.keys(Buffer)` looked like a mere wrong answer -- and SIGBUS'd in `js_array_length` when they were not. Separately (#8139), the HIR folds the zero-arg `x.toLocaleString()` on ANY receiver to `Expr::DateToLocaleString`, whose runtime tail had no array / typed-array / buffer arm and rendered `[object Array]`. New `buffer::exotic_view` owns the discrimination; every call site asks it ABOVE the arm it guards, never below. --- .../perry-runtime/src/buffer/exotic_view.rs | 95 ++++ .../src/buffer/exotic_view_tests.rs | 482 ++++++++++++++++++ crates/perry-runtime/src/buffer/mod.rs | 15 +- crates/perry-runtime/src/buffer/own_props.rs | 25 + .../src/json/stringify_buffer.rs | 24 + crates/perry-runtime/src/object/alloc.rs | 36 ++ .../src/object/buffer_dispatch.rs | 23 +- .../perry-runtime/src/object/descriptors.rs | 18 + .../perry-runtime/src/object/field_get_set.rs | 2 +- .../src/object/field_get_set/enumeration.rs | 136 +++++ .../object/field_get_set/get_field_by_name.rs | 7 + .../field_get_set/get_field_by_name_tail.rs | 10 +- .../src/object/field_get_set/has_property.rs | 35 ++ .../src/object/has_own_helpers.rs | 12 + crates/perry-runtime/src/object/mod.rs | 2 +- .../src/object/native_call_method.rs | 3 + .../object/native_call_method/object_proto.rs | 68 +++ .../to_locale_string_tests.rs | 156 ++++++ .../object/object_ops/descriptor_helpers.rs | 14 +- .../src/object/polymorphic_index.rs | 26 + crates/perry-runtime/src/proxy.rs | 10 + crates/perry-runtime/src/typed_feedback.rs | 21 + crates/perry-runtime/src/value/dyn_index.rs | 26 + .../perry-runtime/src/value/dynamic_object.rs | 10 + 24 files changed, 1248 insertions(+), 8 deletions(-) create mode 100644 crates/perry-runtime/src/buffer/exotic_view.rs create mode 100644 crates/perry-runtime/src/buffer/exotic_view_tests.rs create mode 100644 crates/perry-runtime/src/object/native_call_method/to_locale_string_tests.rs diff --git a/crates/perry-runtime/src/buffer/exotic_view.rs b/crates/perry-runtime/src/buffer/exotic_view.rs new file mode 100644 index 0000000000..7f5450c328 --- /dev/null +++ b/crates/perry-runtime/src/buffer/exotic_view.rs @@ -0,0 +1,95 @@ +//! #8149: which registered buffers carry integer-indexed own properties. +//! +//! Perry backs FOUR distinct JS types with the same `BufferHeader` + the same +//! `BUFFER_REGISTRY` entry: a node `Buffer`, a `Uint8Array`, an +//! `ArrayBuffer`/`SharedArrayBuffer`, and a `DataView`. Only the first two are +//! *integer-indexed exotic objects*. The other two have **no** integer-indexed +//! own properties at all: +//! +//! ```js +//! const dv = new DataView(new ArrayBuffer(8)); +//! dv[0] // undefined — not the byte +//! dv.length // undefined — DataView has byteLength, not length +//! 0 in dv // false +//! dv[0] = 7 // an ORDINARY own property "0"; the byte stays 0 +//! Object.keys(dv) // ["0"] — the expando, never the bytes +//! ``` +//! +//! Every consumer that triaged a receiver as "registered buffer ⇒ byte +//! indexable" served all four, so a `DataView` and a raw `ArrayBuffer` answered +//! bytes for `[i]`, `in`, `.length` and `hasOwnProperty`, and an index STORE +//! overwrote a byte where node creates a property. +//! +//! The discriminating question is asked ABOVE the byte arm, never below it: +//! the byte arm answers unconditionally, so a re-check placed after it is dead +//! code. This is the same ordering rule #8090 / #8109 / #8119 / #8120 / #8124 / +//! #8140 / #8141 / #8148 / #8173 each had to restore. +//! +//! Both backings are covered by construction. `is_registered_buffer` is a +//! side-table membership test, not an address-range or GC-header probe, so an +//! EXTERNAL buffer (no `GcHeader` at all — see `array/header.rs`'s +//! `array_receiver_gc_tag` doc, #8142) is classified by exactly the same +//! lookups as an arena-backed one. + +/// `true` when `addr` is a registered buffer whose integer indices really are +/// byte slots — a node `Buffer`, a `Uint8Array`, or another buffer-backed typed +/// array. +/// +/// `false` both for a non-buffer and for the three registered buffers that are +/// NOT integer-indexed exotic objects: `ArrayBuffer`, `SharedArrayBuffer` and +/// `DataView`. Use [`is_non_indexed_buffer_view`] to tell those two `false` +/// cases apart — a non-buffer must keep falling through to the generic object +/// walk, while a `DataView` must answer `undefined`/`false` right here. +/// +/// The KeyObject / CryptoKey buffers are byte-indexed today and stay that way: +/// this predicate is about the `ArrayBuffer`/`DataView` split, and silently +/// changing a crypto receiver's indexing would be an unrelated behaviour +/// change. `object::typed_array_proto_thunks::is_typed_array_buffer` is the +/// stricter sibling that also declines those, because it selects a +/// `%TypedArray%.prototype` METHOD population rather than an element read. +#[inline] +pub fn is_byte_indexed_buffer(addr: usize) -> bool { + super::is_registered_buffer(addr) && !is_non_indexed_buffer_view(addr) +} + +/// `true` for the registered buffers with no integer-indexed own properties: +/// `ArrayBuffer`, `SharedArrayBuffer`, `DataView`. +/// +/// Does NOT itself check registration — both underlying sets are address-keyed +/// and only ever populated for registered buffers, and both are latch-guarded +/// so a program that never constructs one pays two relaxed atomic loads. Call +/// it inside an arm that has already established `is_registered_buffer`, or use +/// [`is_byte_indexed_buffer`] for the combined question. +#[inline] +pub fn is_non_indexed_buffer_view(addr: usize) -> bool { + super::is_any_array_buffer(addr) || super::is_data_view(addr) +} + +/// The own-property key a NUMERIC computed key names on a non-indexed buffer +/// view, or `None` when the key is not a number at all. +/// +/// `dv[0] = 7` stores under `"0"` and `Object.keys(dv)` must report `"0"` — +/// the ordinary `ToPropertyKey` string, not the raw double. The argument is the +/// NaN-BOXED key as the index paths carry it, so both the `INT32_TAG` form +/// (`dv[0]` with a loop counter) and the plain-double form reach the same +/// answer; a string / symbol / pointer key answers `None` so the caller keeps +/// its existing by-name route. +/// +/// Scope: CANONICAL array indices only. A numeric key that is not one (`-1`, +/// `1.5`, `NaN`) is also a legitimate ordinary property key in node +/// (`dv[-1] = 1` then `Object.keys(dv)` is `["-1"]`), but perry drops that +/// store today for a Buffer as well as a DataView, so it is left alone here +/// rather than fixed only for one of the four buffer-backed types. The caller +/// falls through to its existing route, whose answer for those keys is +/// unchanged. +pub fn canonical_index_key(index: f64) -> Option { + let jv = crate::value::JSValue::from_bits(index.to_bits()); + let n = if jv.is_int32() { + f64::from(jv.as_int32()) + } else if jv.is_number() { + index + } else { + return None; + }; + (n >= 0.0 && n.fract() == 0.0 && n <= u32::MAX as f64).then(|| (n as u32).to_string()) +} diff --git a/crates/perry-runtime/src/buffer/exotic_view_tests.rs b/crates/perry-runtime/src/buffer/exotic_view_tests.rs new file mode 100644 index 0000000000..716e3daef5 --- /dev/null +++ b/crates/perry-runtime/src/buffer/exotic_view_tests.rs @@ -0,0 +1,482 @@ +//! #8149: an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` is a registered +//! buffer that is NOT integer-indexed exotic. +//! +//! ## Why these tests assert VALUES, never predicates +//! +//! Every case below pins the exact answer node gives, measured against node +//! `26.5.1` (the `.node-version` pin). A `DataView`'s bytes are zero-filled, so +//! a probe that only asked "is `dv[0]` falsy?" would pass under the bug — +//! `0` and `undefined` are both falsy. The tests therefore compare the NaN-box +//! tag (`is_undefined`), not truthiness, and the store-side tests assert BOTH +//! halves: that the expando exists AND that the byte is still zero. A fix that +//! wrote the byte *and* recorded the expando would pass the first half alone. +//! +//! ## What each control proves +//! +//! * `..._buffer_receiver_...` cases keep a real `Buffer` / `Uint8Array` on the +//! byte path. That is the population the new predicate must NOT capture, and +//! the sabotage arm "decline every registered buffer" fails exactly here. +//! * the plain-array and plain-object cases prove the new arm did not divert a +//! receiver that never was a buffer. + +use crate::value::JSValue; + +fn undefined() -> f64 { + f64::from_bits(crate::value::TAG_UNDEFINED) +} + +fn boxed(addr: usize) -> f64 { + f64::from_bits(JSValue::pointer(addr as *const u8).bits()) +} + +fn is_undefined(v: f64) -> bool { + v.to_bits() == crate::value::TAG_UNDEFINED +} + +/// A registered `BufferHeader` holding `bytes`, marked as `mark` dictates. +fn buffer_with(bytes: &[u8]) -> *mut crate::buffer::BufferHeader { + let buf = crate::buffer::buffer_alloc(bytes.len() as u32); + unsafe { + (*buf).length = bytes.len() as u32; + std::ptr::copy_nonoverlapping( + bytes.as_ptr(), + crate::buffer::buffer_data_mut(buf), + bytes.len(), + ); + } + buf +} + +fn data_view(bytes: &[u8]) -> usize { + let buf = buffer_with(bytes) as usize; + crate::buffer::mark_as_data_view(buf); + buf +} + +fn array_buffer(bytes: &[u8]) -> usize { + let buf = buffer_with(bytes) as usize; + crate::buffer::mark_as_array_buffer(buf); + buf +} + +/// A node `Buffer`: registered, marked nothing. (`Buffer.from` does not call +/// `mark_as_uint8array` — that mark distinguishes the `Uint8Array` CONSTRUCTOR +/// path, which is why `Object.keys` was right for one and `[]` for the other.) +fn node_buffer(bytes: &[u8]) -> usize { + buffer_with(bytes) as usize +} + +fn string_keys(arr: *mut crate::array::ArrayHeader) -> Vec { + let n = crate::array::js_array_length(arr); + (0..n) + .filter_map(|i| { + let v = crate::array::js_array_get(arr, i); + let ptr = crate::value::js_get_string_pointer_unified(f64::from_bits(v.bits())) + as *const crate::StringHeader; + if ptr.is_null() { + return None; + } + unsafe { + let len = (*ptr).byte_len as usize; + let data = (ptr as *const u8).add(std::mem::size_of::()); + std::str::from_utf8(std::slice::from_raw_parts(data, len)) + .ok() + .map(str::to_owned) + } + }) + .collect() +} + +fn key(name: &str) -> f64 { + let s = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + f64::from_bits(JSValue::string_ptr(s).bits()) +} + +// --------------------------------------------------------------------------- +// The predicate itself +// --------------------------------------------------------------------------- + +#[test] +fn only_array_buffer_and_data_view_are_non_indexed_views() { + let dv = data_view(&[1, 2, 3, 4]); + let ab = array_buffer(&[1, 2, 3, 4]); + let b = node_buffer(&[1, 2, 3]); + let u8a = node_buffer(&[1, 2, 3]); + crate::buffer::mark_as_uint8array(u8a); + + assert!(crate::buffer::is_non_indexed_buffer_view(dv)); + assert!(crate::buffer::is_non_indexed_buffer_view(ab)); + assert!(!crate::buffer::is_non_indexed_buffer_view(b)); + assert!(!crate::buffer::is_non_indexed_buffer_view(u8a)); + + assert!(!crate::buffer::is_byte_indexed_buffer(dv)); + assert!(!crate::buffer::is_byte_indexed_buffer(ab)); + assert!(crate::buffer::is_byte_indexed_buffer(b)); + assert!(crate::buffer::is_byte_indexed_buffer(u8a)); + + // A plain array is not a buffer at all — the two `false` answers above are + // NOT the same answer as this one, which is why `is_non_indexed_buffer_view` + // exists separately from `!is_byte_indexed_buffer`. + let arr = crate::array::js_array_alloc(2); + assert!(!crate::buffer::is_byte_indexed_buffer(arr as usize)); + assert!(!crate::buffer::is_non_indexed_buffer_view(arr as usize)); +} + +// --------------------------------------------------------------------------- +// Element READ — the three index funnels named in #8149 +// --------------------------------------------------------------------------- + +#[test] +fn a_data_view_index_read_is_undefined_through_every_index_funnel() { + let dv = data_view(&[7, 8, 9, 10]); + let recv = boxed(dv); + // node: `new DataView(new ArrayBuffer(4))[0]` === undefined, for EVERY + // in-bounds index. `0` (the byte) and `undefined` are both falsy, so the + // assertion is on the NaN-box tag. + for idx in 0..4u32 { + let i = f64::from(idx); + assert!( + is_undefined(crate::value::js_dyn_index_get(recv, i)), + "js_dyn_index_get answered a byte for dv[{idx}]" + ); + assert!( + is_undefined(crate::object::js_object_get_index_polymorphic(dv as i64, i)), + "js_object_get_index_polymorphic answered a byte for dv[{idx}]" + ); + assert!( + is_undefined( + crate::typed_feedback::js_typed_feedback_array_index_get_fallback_boxed(0, recv, i,) + ), + "the typed-feedback fallback answered a byte for dv[{idx}]" + ); + } +} + +#[test] +fn an_array_buffer_index_read_is_undefined() { + let ab = array_buffer(&[7, 8]); + let recv = boxed(ab); + assert!(is_undefined(crate::value::js_dyn_index_get(recv, 0.0))); + assert!(is_undefined( + crate::object::js_object_get_index_polymorphic(ab as i64, 1.0) + )); +} + +#[test] +fn a_buffer_receiver_still_reads_its_bytes() { + // CONTROL. `Buffer.from([1,2,3])[1]` is `2` in node, and the sabotage arm + // "decline every registered buffer" fails here. + let b = node_buffer(&[1, 2, 3]); + let recv = boxed(b); + assert_eq!(crate::value::js_dyn_index_get(recv, 1.0), 2.0); + assert_eq!( + crate::object::js_object_get_index_polymorphic(b as i64, 2.0), + 3.0 + ); + assert_eq!( + crate::typed_feedback::js_typed_feedback_array_index_get_fallback_boxed(0, recv, 0.0), + 1.0 + ); +} + +#[test] +fn a_plain_array_receiver_is_untouched_by_the_view_arm() { + // CONTROL: the new question is asked of buffers only. + let arr = crate::array::js_array_alloc(3); + for v in [10.0, 20.0, 30.0] { + crate::array::js_array_push_f64(arr, v); + } + assert_eq!( + crate::value::js_dyn_index_get(boxed(arr as usize), 1.0), + 20.0 + ); + assert_eq!( + crate::object::js_object_get_index_polymorphic(arr as i64, 2.0), + 30.0 + ); +} + +// --------------------------------------------------------------------------- +// Element STORE — an ordinary own property, not a byte +// --------------------------------------------------------------------------- + +#[test] +fn a_data_view_index_store_creates_an_own_property_and_leaves_the_byte() { + let dv = data_view(&[0, 0, 0, 0]); + // node: `dv[0] = 7` → `dv[0] === 7`, `new Uint8Array(ab)[0] === 0`. + crate::object::js_object_set_index_polymorphic(dv as i64, 0.0, 7.0); + // BOTH halves. A fix that recorded the expando AND wrote the byte passes + // the first assertion alone. + assert_eq!( + crate::buffer::buffer_get_own_prop(dv, "0"), + Some(7.0), + "the store must land as the own property \"0\"" + ); + assert_eq!( + crate::buffer::js_buffer_get(dv as *const crate::buffer::BufferHeader, 0), + 0, + "the store must NOT have written byte 0" + ); + assert_eq!(crate::value::js_dyn_index_get(boxed(dv), 0.0), 7.0); +} + +#[test] +fn a_data_view_index_store_through_dyn_index_set_creates_an_own_property() { + let dv = data_view(&[0, 0]); + crate::value::js_dyn_index_set(boxed(dv), 1.0, 5.0); + assert_eq!(crate::buffer::buffer_get_own_prop(dv, "1"), Some(5.0)); + assert_eq!( + crate::buffer::js_buffer_get(dv as *const crate::buffer::BufferHeader, 1), + 0 + ); +} + +#[test] +fn a_buffer_index_store_still_writes_the_byte() { + // CONTROL. `Buffer.from([1,2,3])[0] = 9` writes the byte in node. + let b = node_buffer(&[1, 2, 3]); + crate::object::js_object_set_index_polymorphic(b as i64, 0.0, 9.0); + assert_eq!( + crate::buffer::js_buffer_get(b as *const crate::buffer::BufferHeader, 0), + 9 + ); + assert_eq!(crate::buffer::buffer_get_own_prop(b, "0"), None); +} + +// --------------------------------------------------------------------------- +// `in` / `hasOwnProperty` / `.length` +// --------------------------------------------------------------------------- + +#[test] +fn in_and_has_own_are_false_for_a_data_view_index() { + let dv = data_view(&[1, 2, 3, 4]); + let recv = boxed(dv); + // node: `0 in dv` === false, `Object.prototype.hasOwnProperty.call(dv,"0")` + // === false. + assert_eq!( + crate::object::js_object_has_property(recv, 0.0).to_bits(), + crate::value::TAG_FALSE + ); + assert_eq!( + crate::object::js_object_has_property(recv, key("0")).to_bits(), + crate::value::TAG_FALSE + ); + assert_eq!( + crate::object::js_object_has_own(recv, key("0")).to_bits(), + crate::value::TAG_FALSE + ); + // `length` is a %TypedArray% slot; a DataView has only `byteLength`. + assert_eq!( + crate::object::js_object_has_property(recv, key("length")).to_bits(), + crate::value::TAG_FALSE + ); + + // After a store the index IS an own property — node agrees. + crate::object::js_object_set_index_polymorphic(dv as i64, 0.0, 7.0); + assert_eq!( + crate::object::js_object_has_property(recv, 0.0).to_bits(), + crate::value::TAG_TRUE + ); + assert_eq!( + crate::object::js_object_has_own(recv, key("0")).to_bits(), + crate::value::TAG_TRUE + ); +} + +#[test] +fn in_and_has_own_still_answer_true_for_a_buffer_index() { + // CONTROL. + let b = node_buffer(&[1, 2, 3]); + let recv = boxed(b); + assert_eq!( + crate::object::js_object_has_property(recv, 1.0).to_bits(), + crate::value::TAG_TRUE + ); + assert_eq!( + crate::object::js_object_has_own(recv, key("1")).to_bits(), + crate::value::TAG_TRUE + ); + assert_eq!( + crate::object::js_object_has_property(recv, 3.0).to_bits(), + crate::value::TAG_FALSE, + "an out-of-bounds index is still absent" + ); + assert_eq!( + crate::object::js_object_has_property(recv, key("length")).to_bits(), + crate::value::TAG_TRUE + ); +} + +#[test] +fn length_is_undefined_for_a_view_and_the_byte_count_for_a_buffer() { + let dv = data_view(&[1, 2, 3, 4]); + let ab = array_buffer(&[1, 2, 3, 4, 5]); + let b = node_buffer(&[1, 2, 3]); + unsafe { + // node: `dv.length` / `ab.length` === undefined; `byteLength` is the + // count. `0` would ALSO be wrong-but-falsy, hence the tag assertion. + assert!(is_undefined(crate::value::js_dynamic_object_get_property( + boxed(dv), + b"length".as_ptr() as *const i8, + 6 + ))); + assert!(is_undefined(crate::value::js_dynamic_object_get_property( + boxed(ab), + b"length".as_ptr() as *const i8, + 6 + ))); + assert_eq!( + crate::value::js_dynamic_object_get_property( + boxed(dv), + b"byteLength".as_ptr() as *const i8, + 10 + ), + 4.0 + ); + // CONTROL: a Buffer keeps both spellings. + assert_eq!( + crate::value::js_dynamic_object_get_property( + boxed(b), + b"length".as_ptr() as *const i8, + 6 + ), + 3.0 + ); + } +} + +// --------------------------------------------------------------------------- +// Enumeration — `Object.keys` / `.values` / `.entries` / `getOwnPropertyNames` +// +// This is also the memory-safety half: before the buffer arm existed these +// walked a `BufferHeader` as an `ObjectHeader`, reading payload bytes as +// `keys_array`. `Object.keys(new DataView(new ArrayBuffer(8)))` SIGBUS'd in +// `js_array_length` in any program that had also allocated a `Buffer` (exit +// 138); it answered `[]` only when those bytes happened to be zero. +// --------------------------------------------------------------------------- + +#[test] +fn object_keys_of_a_buffer_lists_its_byte_indices() { + let b = node_buffer(&[1, 2, 3]); + // node: `Object.keys(Buffer.from([1,2,3]))` === ["0","1","2"]. Perry + // answered `[]`. + assert_eq!( + string_keys(crate::object::js_object_keys_value(boxed(b))), + vec!["0", "1", "2"] + ); + let values = crate::object::js_object_values_value(boxed(b)); + let n = crate::array::js_array_length(values); + let got: Vec = (0..n) + .map(|i| f64::from_bits(crate::array::js_array_get(values, i).bits())) + .collect(); + assert_eq!(got, vec![1.0, 2.0, 3.0]); +} + +#[test] +fn object_keys_of_a_data_view_is_empty_then_reports_the_expando() { + let dv = data_view(&[1, 2, 3, 4, 5, 6, 7, 8]); + // node: `Object.keys(dv)` === []; after `dv[0] = 7` it is ["0"]. + assert!(string_keys(crate::object::js_object_keys_value(boxed(dv))).is_empty()); + crate::object::js_object_set_index_polymorphic(dv as i64, 0.0, 7.0); + assert_eq!( + string_keys(crate::object::js_object_keys_value(boxed(dv))), + vec!["0"] + ); +} + +/// The memory-safety half, with the poison the field measurement supplied. +/// +/// The pre-fix walk read the `BufferHeader`'s PAYLOAD as +/// `ObjectHeader.keys_array` and then called `js_array_length` on it. It +/// answered `[]` whenever those bytes happened to be zero — which is what made +/// `Object.keys(Buffer)` look like a mere wrong answer — and SIGBUS'd when they +/// did not. The `.ts` repro was +/// `new ArrayBuffer(8); new DataView(ab); Buffer.from([1,2,3]); +/// Object.keys(D)`, exit 138, `js_array_length` ← `js_object_keys`. +/// +/// Filling the payload with a non-zero, non-heap word makes the hazard +/// deterministic instead of allocation-order-dependent: with the arm removed +/// this reads that word as a pointer. The assertion is still on the ANSWER — +/// the poison is a stressor, not the subject. +#[test] +fn object_keys_of_a_poison_filled_view_neither_reads_nor_crashes() { + let dv = data_view(&[0xAA; 64]); + assert!(string_keys(crate::object::js_object_keys_value(boxed(dv))).is_empty()); + let names = crate::object::js_object_get_own_property_names(boxed(dv)); + let arr = (names.to_bits() & crate::value::POINTER_MASK) as *mut crate::array::ArrayHeader; + assert!(string_keys(arr).is_empty()); + + let b = node_buffer(&[0xAA; 3]); + assert_eq!( + string_keys(crate::object::js_object_keys_value(boxed(b))), + vec!["0", "1", "2"] + ); +} + +#[test] +fn get_own_property_names_of_a_buffer_lists_its_byte_indices() { + let b = node_buffer(&[4, 5]); + let names = crate::object::js_object_get_own_property_names(boxed(b)); + let arr = (names.to_bits() & crate::value::POINTER_MASK) as *mut crate::array::ArrayHeader; + assert_eq!(string_keys(arr), vec!["0", "1"]); + + let ab = array_buffer(&[4, 5]); + let names = crate::object::js_object_get_own_property_names(boxed(ab)); + let arr = (names.to_bits() & crate::value::POINTER_MASK) as *mut crate::array::ArrayHeader; + assert!(string_keys(arr).is_empty()); +} + +#[test] +fn object_keys_of_a_plain_object_and_array_are_unchanged() { + // CONTROL: the buffer arm sits at the very top of all three walks, so this + // is what proves it declined a receiver that is not a buffer. + let arr = crate::array::js_array_alloc(2); + crate::array::js_array_push_f64(arr, 1.0); + crate::array::js_array_push_f64(arr, 2.0); + assert_eq!( + string_keys(crate::object::js_object_keys_value(boxed(arr as usize))), + vec!["0", "1"] + ); + + let obj = crate::object::js_object_alloc(0, 2); + let k = crate::string::js_string_from_bytes(b"a".as_ptr(), 1); + crate::object::js_object_set_field_by_name(obj, k, 1.0); + assert_eq!( + string_keys(crate::object::js_object_keys_value(boxed(obj as usize))), + vec!["a"] + ); +} + +// --------------------------------------------------------------------------- +// JSON +// --------------------------------------------------------------------------- + +#[test] +fn json_stringify_of_a_view_is_an_empty_object_not_buffer_bytes() { + let dv = data_view(&[1, 2, 3, 4]); + let ab = array_buffer(&[9, 9]); + // node: `JSON.stringify(new DataView(new ArrayBuffer(4)))` === "{}". + // Perry emitted `{"type":"Buffer","data":[1,2,3,4]}` — a shape node never + // produces here, and one that leaks the backing bytes. + assert_eq!(json_of(boxed(dv)), "{}"); + assert_eq!(json_of(boxed(ab)), "{}"); + + // CONTROL: a real Buffer keeps `Buffer.prototype.toJSON`'s shape. + let b = node_buffer(&[1, 2]); + assert_eq!(json_of(boxed(b)), r#"{"type":"Buffer","data":[1,2]}"#); +} + +fn json_of(value: f64) -> String { + let bits = + unsafe { crate::json::js_json_stringify_full(value, undefined(), undefined()) } as u64; + let ptr = crate::value::js_get_string_pointer_unified(f64::from_bits(bits)) + as *const crate::StringHeader; + if ptr.is_null() { + return String::new(); + } + unsafe { + let len = (*ptr).byte_len as usize; + let data = (ptr as *const u8).add(std::mem::size_of::()); + String::from_utf8_lossy(std::slice::from_raw_parts(data, len)).into_owned() + } +} diff --git a/crates/perry-runtime/src/buffer/mod.rs b/crates/perry-runtime/src/buffer/mod.rs index e740df5440..5f6e254ad1 100644 --- a/crates/perry-runtime/src/buffer/mod.rs +++ b/crates/perry-runtime/src/buffer/mod.rs @@ -15,6 +15,11 @@ mod copy_write; mod dataview; mod detach; mod encode; +mod exotic_view; +/// #8149: `ArrayBuffer` / `SharedArrayBuffer` / `DataView` are registered +/// buffers with no integer-indexed own properties. +#[cfg(test)] +mod exotic_view_tests; mod from; mod header; mod iter; @@ -65,10 +70,16 @@ pub(crate) use detach::{array_buffer_transfer, detach_array_buffer}; #[cfg(test)] pub(crate) use own_props::test_buffer_own_props_owner_count; pub use own_props::{ - buffer_get_own_prop, buffer_has_own_prop, buffer_own_props_possible, buffer_set_own_prop, - clear_buffer_own_props, scan_buffer_own_props_roots_mut, + buffer_get_own_prop, buffer_has_own_prop, buffer_own_prop_names, buffer_own_props_possible, + buffer_set_own_prop, clear_buffer_own_props, scan_buffer_own_props_roots_mut, }; +// ---- Re-exports: #8149 integer-indexed-exotic discrimination ---- +// `ArrayBuffer` / `SharedArrayBuffer` / `DataView` share `BufferHeader` and the +// buffer registry with `Buffer` / `Uint8Array` but have NO integer-indexed own +// properties. See `exotic_view`. +pub use exotic_view::{canonical_index_key, is_byte_indexed_buffer, is_non_indexed_buffer_view}; + // ---- Re-exports: Buffer.from / alloc / concat (FFI) ---- pub use from::{ js_array_buffer_new, js_array_buffer_new_value, js_buffer_alloc, js_buffer_alloc_fill_value, diff --git a/crates/perry-runtime/src/buffer/own_props.rs b/crates/perry-runtime/src/buffer/own_props.rs index c4017a5748..703f16208e 100644 --- a/crates/perry-runtime/src/buffer/own_props.rs +++ b/crates/perry-runtime/src/buffer/own_props.rs @@ -76,6 +76,31 @@ pub fn buffer_get_own_prop(addr: usize, prop: &str) -> Option { .map(f64::from_bits) } +/// Every own dynamic prop key recorded for `addr`, in insertion-independent +/// (sorted) order. +/// +/// #8149: `Object.keys` / `getOwnPropertyNames` / `for…in` need these. Before, +/// the enumeration paths had no registered-buffer arm at all and walked a +/// `BufferHeader` as an `ObjectHeader` — reading payload bytes as the +/// `keys_array` pointer, which returned `[]` when those bytes happened to be +/// zero and SIGBUS'd in `js_array_length` when they did not. +/// +/// Integer-index keys come back as the canonical decimal strings they were +/// stored under (`buffer::canonical_index_key`); the caller is responsible for +/// the ECMA-262 ordering rule that puts array indices first, ascending. +pub fn buffer_own_prop_names(addr: usize) -> Vec { + if addr == 0 || !buffer_own_props_possible() { + return Vec::new(); + } + let mut names: Vec = buffer_props() + .lock() + .ok() + .and_then(|props| props.get(&addr).map(|m| m.keys().cloned().collect())) + .unwrap_or_default(); + names.sort(); + names +} + /// Whether the buffer carries any own dynamic prop under `prop`. pub fn buffer_has_own_prop(addr: usize, prop: &str) -> bool { buffer_get_own_prop(addr, prop).is_some() diff --git a/crates/perry-runtime/src/json/stringify_buffer.rs b/crates/perry-runtime/src/json/stringify_buffer.rs index 1d7e8ede8d..70a27da57f 100644 --- a/crates/perry-runtime/src/json/stringify_buffer.rs +++ b/crates/perry-runtime/src/json/stringify_buffer.rs @@ -23,6 +23,23 @@ pub(crate) unsafe fn stringify_buffer(ptr: *const u8, buf: &mut String) { buf.push_str("null"); return; } + // #8149: an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` is a + // registered buffer, but it is NOT a `Buffer` and NOT a `Uint8Array`. + // Neither `Buffer.prototype.toJSON` nor the integer-indexed own-property + // shape applies: node serializes both as `{}` because they have no own + // enumerable properties at all. Perry answered + // `{"type":"Buffer","data":[…]}` — a shape node never produces for these, + // and one that leaks the backing bytes. Asked ABOVE the + // Buffer/`Uint8Array` split, which claims every registered buffer. + // + // Own expandos (`dv.foo = 1`, which node WOULD serialize) are not emitted: + // that needs the generic object serializer, and this arm exists to stop the + // byte leak. `{}` is node's answer for every `DataView`/`ArrayBuffer` that + // carries none, which is all of them in practice. + if crate::buffer::is_non_indexed_buffer_view(ptr as usize) { + buf.push_str("{}"); + return; + } let len = (*buf_ptr).length as usize; let data = (buf_ptr as *const u8).add(std::mem::size_of::()); let bytes = std::slice::from_raw_parts(data, len); @@ -135,6 +152,13 @@ pub(crate) unsafe fn stringify_buffer_pretty( buf.push_str("null"); return; } + // #8149: see `stringify_buffer` — an `ArrayBuffer` / `SharedArrayBuffer` / + // `DataView` is neither a `Buffer` nor a `Uint8Array`, and node serializes + // all three as `{}`. + if crate::buffer::is_non_indexed_buffer_view(ptr as usize) { + buf.push_str("{}"); + return; + } let len = (*buf_ptr).length as usize; let data = (buf_ptr as *const u8).add(std::mem::size_of::()); let bytes = std::slice::from_raw_parts(data, len); diff --git a/crates/perry-runtime/src/object/alloc.rs b/crates/perry-runtime/src/object/alloc.rs index cf0f612022..397b3b0456 100644 --- a/crates/perry-runtime/src/object/alloc.rs +++ b/crates/perry-runtime/src/object/alloc.rs @@ -1506,6 +1506,42 @@ pub unsafe extern "C" fn js_object_assign_one(target_f64: f64, source_f64: f64) return target_f64; } + // #8149: a registered BUFFER source — a node `Buffer` / `Uint8Array` (whose + // own enumerable properties ARE its byte indices, so + // `{...Buffer.from([1,2,3])}` is `{"0":1,"1":2,"2":3}` in node), or an + // `ArrayBuffer` / `DataView` (which own only whatever the user assigned). + // A `BufferHeader` is not an `ObjectHeader`; the walk below reached the + // `try_read_gc_header` triage and answered `{}` for an arena-backed buffer, + // and an EXTERNAL one has no `GcHeader` at all, so the byte it reads there + // is allocator bookkeeping that can classify as anything. Enumerate through + // the shared buffer own-key helper instead. + if let Some(keys) = + crate::object::field_get_set::enumeration::registered_buffer_own_keys(src_raw) + { + // The key string and the write funnel both allocate, so the target can + // move on every iteration: read it through the handle AT the call + // (`with_mut_ptr`) rather than binding a pre-loop copy. + let scope = crate::gc::RuntimeHandleScope::new(); + let tgt_h = scope.root_raw_mut_ptr(target); + for name in keys { + let value = crate::object::field_get_set::enumeration::registered_buffer_own_value( + src_raw, &name, + ); + let value_h = scope.root_nanbox_f64(value); + let key_ptr = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + tgt_h.with_mut_ptr::(|tgt| { + object_assign_set_string_key( + tgt, + target_is_array, + key_ptr, + value_h.get_nanbox_f64(), + ) + }); + } + return tgt_h + .with_mut_ptr::(|tgt| crate::value::js_nanbox_pointer(tgt as i64)); + } + // A function/closure source is NOT an `ObjectHeader`: reading `keys_array` // off it dereferences a bogus field, yielding a garbage `key_count` and a // runaway copy loop. Enumerate the closure's own *enumerable* dynamic props diff --git a/crates/perry-runtime/src/object/buffer_dispatch.rs b/crates/perry-runtime/src/object/buffer_dispatch.rs index 1d00e806af..4d569198e2 100644 --- a/crates/perry-runtime/src/object/buffer_dispatch.rs +++ b/crates/perry-runtime/src/object/buffer_dispatch.rs @@ -1101,7 +1101,28 @@ pub unsafe fn dispatch_buffer_method( "valueOf" => f64::from_bits(JSValue::pointer(addr as *mut u8).bits()), // `buf.toLocaleString()` — Node delegates to toString() with no // args, which yields the utf8 decode. Match that. - "toLocaleString" => { + // + // #8139: `Buffer.prototype.toLocaleString` is an OWN override on + // `Buffer`. A plain `Uint8Array` inherits + // `%TypedArray%.prototype.toLocaleString`, which is the element JOIN — + // `new Uint8Array([3,1,2]).toLocaleString()` is `"3,1,2"` in node, not + // the three raw bytes. Perry backs both with the same `BufferHeader`, + // so this arm claimed the plain `Uint8Array` too. Ask the + // receiver-kind question ABOVE the arm by declining here: the catch-all + // then delegates to `dispatch_uint8_buffer_method`, whose + // `toLocaleString` is already `uint8_join` (correct for bytes, which + // are all below the first digit-grouping boundary). + // + // `is_uint8array_buffer` is the mark the `Uint8Array` CONSTRUCTOR path + // sets and `Buffer.from` does not. It is not a perfect brand — the + // `KeyObject.export()` arm below marks its *Buffer* result so + // `instanceof Uint8Array` holds — so `secretKey.export() + // .toLocaleString()` now joins where node decodes. That is the + // Buffer-vs-`Uint8Array` identity conflation #8139 also names for + // `toString`; splitting it properly needs a real `Buffer` brand, which + // is deliberately NOT attempted here. Before this change the same call + // answered `"[object Uint8Array]"`, so no spelling regressed. + "toLocaleString" if !crate::buffer::is_uint8array_buffer(addr) => { let str_ptr = crate::buffer::js_buffer_to_string(buf_ptr, 0); f64::from_bits(JSValue::string_ptr(str_ptr).bits()) } diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index e802184b4f..f058caf7d0 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1120,6 +1120,24 @@ pub extern "C" fn js_object_get_own_property_names(obj_value: f64) -> f64 { ); return f64::from_bits((result as u64) | 0x7FFD_0000_0000_0000); } + // #8149: a registered BUFFER receiver the arm above did not claim — a + // node `Buffer` (never `mark_as_uint8array`-tagged, so absent from the + // typed-array owner set), an `ArrayBuffer`, or a `DataView`. Without + // this the generic walk below reads a `BufferHeader` as an + // `ObjectHeader`; `Object.getOwnPropertyNames(Buffer.from([1,2,3]))` + // answered `[]` where node lists the byte indices. + if obj_jv.is_pointer() { + let addr = crate::value::js_nanbox_get_pointer(obj_value) as usize; + if let Some(keys) = super::field_get_set::enumeration::registered_buffer_own_keys(addr) + { + let mut out = crate::array::js_array_alloc(keys.len().max(1) as u32); + for name in keys { + let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + out = crate::array::js_array_push(out, crate::value::JSValue::string_ptr(key)); + } + return f64::from_bits((out as u64) | 0x7FFD_0000_0000_0000); + } + } // Date / RegExp / Error exotic instances: expando keys (including // non-enumerable ones) + per-kind builtin own slots. if let Some((addr, kind)) = super::exotic_expando::exotic_expando_kind_of_value(obj_value) { diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index 060577cdcd..edda42ad7f 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -257,7 +257,7 @@ pub(crate) use accessors::scan_accessor_receiver_override_root_mut; mod buffer_own_prop; mod class_object_props; mod crypto_key; -mod enumeration; +pub(crate) mod enumeration; mod field_ops; mod get_field_by_name; #[cfg(test)] diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration.rs b/crates/perry-runtime/src/object/field_get_set/enumeration.rs index 27914f4924..fb91554dc6 100644 --- a/crates/perry-runtime/src/object/field_get_set/enumeration.rs +++ b/crates/perry-runtime/src/object/field_get_set/enumeration.rs @@ -826,11 +826,129 @@ pub(crate) unsafe fn keys_contain_array_index(keys: *const ArrayHeader) -> bool false } +/// The raw heap address behind a possibly still-NaN-boxed `ObjectHeader` +/// pointer, as the enumeration entry points receive it. +#[inline] +fn strip_nanbox_addr(obj: *const ObjectHeader) -> usize { + let bits = obj as u64; + let top16 = bits >> 48; + if top16 == 0x7FFD || top16 >= 0x7FF8 { + (bits & 0x0000_FFFF_FFFF_FFFF) as usize + } else { + bits as usize + } +} + +/// #8149: the own property keys of a REGISTERED BUFFER receiver, in +/// `OrdinaryOwnPropertyKeys` order — canonical array indices ascending, then +/// the string keys. +/// +/// `None` when `addr` is not a registered buffer at all, so the caller keeps +/// its ordinary walk. `Some` for all four buffer-backed types, and the four do +/// NOT answer the same thing: +/// +/// * a node `Buffer` / `Uint8Array` IS an integer-indexed exotic object, so its +/// byte indices are own properties — `Object.keys(Buffer.from([1,2,3]))` is +/// `["0","1","2"]`; +/// * an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` has NONE — only +/// whatever the user assigned (`dv.foo = 1`, or `dv[0] = 7`, which creates an +/// ordinary property rather than writing a byte). +/// +/// Before this arm existed the enumeration paths had no registered-buffer case +/// and fell through to the generic `ObjectHeader` walk, reading buffer payload +/// bytes as the `keys_array` pointer. That answered `[]` when those bytes +/// happened to be zero — which is why `Object.keys(Buffer)` looked merely wrong +/// — and SIGBUS'd in `js_array_length` when they did not, e.g. +/// `Object.keys(new DataView(new ArrayBuffer(8)))` in any program that had also +/// allocated a `Buffer`. +/// +/// Expando ordering among the non-index keys is alphabetical, not insertion +/// order: `buffer::own_props` is a `HashMap`, so insertion order was never +/// recorded. Node uses insertion order. Deterministic-but-different beats the +/// previous nondeterministic-and-crashing. +pub(crate) fn registered_buffer_own_keys(addr: usize) -> Option> { + if addr == 0 || !crate::buffer::is_registered_buffer(addr) { + return None; + } + let mut indices: Vec = Vec::new(); + if crate::buffer::is_byte_indexed_buffer(addr) { + let len = crate::buffer::js_buffer_length(addr as *const crate::buffer::BufferHeader); + indices.extend(0..len.max(0) as u32); + } + let mut names: Vec = Vec::new(); + for name in crate::buffer::buffer_own_prop_names(addr) { + match canonical_array_index(&name) { + Some(idx) if !indices.contains(&idx) => indices.push(idx), + Some(_) => {} + None => names.push(name), + } + } + indices.sort_unstable(); + let mut keys: Vec = indices.into_iter().map(|i| i.to_string()).collect(); + keys.append(&mut names); + Some(keys) +} + +/// The value each key of [`registered_buffer_own_keys`] names: the byte for an +/// in-bounds index of a byte-indexed buffer, else the stored own property. +pub(crate) fn registered_buffer_own_value(addr: usize, key: &str) -> f64 { + if let Some(v) = crate::buffer::buffer_get_own_prop(addr, key) { + return v; + } + if crate::buffer::is_byte_indexed_buffer(addr) { + if let Some(idx) = canonical_array_index(key) { + let buf = addr as *const crate::buffer::BufferHeader; + if (idx as i32) < crate::buffer::js_buffer_length(buf) { + return f64::from(crate::buffer::js_buffer_get(buf, idx as i32)); + } + } + } + f64::from_bits(crate::value::TAG_UNDEFINED) +} + +/// Build the `Object.keys` / `.values` / `.entries` answer for a registered +/// buffer from [`registered_buffer_own_keys`]. +fn registered_buffer_enum(addr: usize, what: MapSetEnum) -> Option<*mut ArrayHeader> { + let keys = registered_buffer_own_keys(addr)?; + let mut out = crate::array::js_array_alloc(keys.len().max(1) as u32); + for key in keys { + let key_str = || crate::string::js_string_from_bytes(key.as_ptr(), key.len() as u32); + match what { + MapSetEnum::Keys => { + out = crate::array::js_array_push(out, JSValue::string_ptr(key_str())); + } + MapSetEnum::Values => { + out = crate::array::js_array_push_f64(out, registered_buffer_own_value(addr, &key)); + } + MapSetEnum::Entries => { + let pair = crate::array::js_array_alloc(2); + let pair = crate::array::js_array_push(pair, JSValue::string_ptr(key_str())); + let pair = + crate::array::js_array_push_f64(pair, registered_buffer_own_value(addr, &key)); + out = crate::array::js_array_push( + out, + JSValue::from_bits(JSValue::pointer(pair as *const u8).bits()), + ); + } + } + } + Some(out) +} + /// Get the keys of an object as an array of strings. /// If any key has a per-property descriptor with `enumerable: false`, that key is filtered out. /// Otherwise (the common case), this returns the stored keys array directly. #[no_mangle] pub extern "C" fn js_object_keys(obj: *const ObjectHeader) -> *mut ArrayHeader { + // #8149: a registered BUFFER receiver — node `Buffer`, `Uint8Array`, + // `ArrayBuffer`, `SharedArrayBuffer` or `DataView`. Asked FIRST, above the + // `is_valid_obj_ptr` guard: a `BufferHeader` is not an `ObjectHeader`, and + // the generic walk below reads its payload bytes as `keys_array` — `[]` + // when they are zero, SIGBUS in `js_array_length` when they are not. + // See `registered_buffer_own_keys`. + if let Some(result) = registered_buffer_enum(strip_nanbox_addr(obj), MapSetEnum::Keys) { + return result; + } if obj.is_null() || !is_valid_obj_ptr(obj as *const u8) { // Issue #893: defensive sibling of `js_object_entries`'s // is_valid_obj_ptr filter — `Object.keys(undefined)` / @@ -1182,6 +1300,15 @@ pub(crate) unsafe fn descriptor_marks_non_enumerable( /// Returns an array of the object's field values #[no_mangle] pub extern "C" fn js_object_values(obj: *const ObjectHeader) -> *mut ArrayHeader { + // #8149: a registered BUFFER receiver — node `Buffer`, `Uint8Array`, + // `ArrayBuffer`, `SharedArrayBuffer` or `DataView`. Asked FIRST, above the + // `is_valid_obj_ptr` guard: a `BufferHeader` is not an `ObjectHeader`, and + // the generic walk below reads its payload bytes as `keys_array` — `[]` + // when they are zero, SIGBUS in `js_array_length` when they are not. + // See `registered_buffer_own_keys`. + if let Some(result) = registered_buffer_enum(strip_nanbox_addr(obj), MapSetEnum::Values) { + return result; + } let stripped = { let bits = obj as u64; let top16 = bits >> 48; @@ -1331,6 +1458,15 @@ pub extern "C" fn js_object_values(obj: *const ObjectHeader) -> *mut ArrayHeader /// Returns an array where each element is a 2-element array [key, value] #[no_mangle] pub extern "C" fn js_object_entries(obj: *const ObjectHeader) -> *mut ArrayHeader { + // #8149: a registered BUFFER receiver — node `Buffer`, `Uint8Array`, + // `ArrayBuffer`, `SharedArrayBuffer` or `DataView`. Asked FIRST, above the + // `is_valid_obj_ptr` guard: a `BufferHeader` is not an `ObjectHeader`, and + // the generic walk below reads its payload bytes as `keys_array` — `[]` + // when they are zero, SIGBUS in `js_array_length` when they are not. + // See `registered_buffer_own_keys`. + if let Some(result) = registered_buffer_enum(strip_nanbox_addr(obj), MapSetEnum::Entries) { + return result; + } let stripped = { let bits = obj as u64; let top16 = bits >> 48; diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index f76d0821b3..98c1b10265 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -658,6 +658,13 @@ pub extern "C" fn js_object_get_field_by_name( } else { let buf = addr as *const crate::buffer::BufferHeader; match key_bytes { + // #8149: `length` is a `%TypedArray%` slot. An + // `ArrayBuffer` / `SharedArrayBuffer` / `DataView` + // exposes only `byteLength`, so node answers + // `undefined` for `dv.length` / `ab.length`. + b"length" if crate::buffer::is_non_indexed_buffer_view(addr) => { + return JSValue::undefined(); + } b"length" | b"byteLength" => { return JSValue::number(crate::buffer::js_buffer_length(buf) as f64); } diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index 3783f356ed..6e33da0c89 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -256,7 +256,15 @@ pub(crate) fn get_field_by_name_object_tail( if let Some(value) = crypto_key_property_value(obj as usize, key_bytes) { return value; } - if key_bytes == b"length" || key_bytes == b"byteLength" { + // #8149: `length` is a `%TypedArray%` slot. An `ArrayBuffer` / + // `SharedArrayBuffer` / `DataView` has only `byteLength`, so + // node answers `undefined` for `dv.length` / `ab.length`. Asked + // ABOVE the shared arm, which answers the byte count for both + // spellings. + if key_bytes == b"byteLength" + || (key_bytes == b"length" + && !crate::buffer::is_non_indexed_buffer_view(obj as usize)) + { let b = obj as *const crate::buffer::BufferHeader; return JSValue::number(crate::buffer::js_buffer_length(b) as f64); } diff --git a/crates/perry-runtime/src/object/field_get_set/has_property.rs b/crates/perry-runtime/src/object/field_get_set/has_property.rs index 36985b9ad8..b7793d69ee 100644 --- a/crates/perry-runtime/src/object/field_get_set/has_property.rs +++ b/crates/perry-runtime/src/object/field_get_set/has_property.rs @@ -561,6 +561,41 @@ pub extern "C" fn js_object_has_property(obj: f64, key: f64) -> f64 { // buffer (not `TYPED_ARRAY_REGISTRY`), so the typed-array arm above misses // them. A Buffer is a `Uint8Array`, so `in` consults numeric indices // (bounds) and the own/inherited members property-get can resolve. + // #8149: an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` is a + // registered buffer with NO integer-indexed own properties, and no + // `length` / `BYTES_PER_ELEMENT` slot either — node's `0 in dv` and + // `"length" in dv` are both `false`. Asked ABOVE the byte-bounds arm, + // which answers `true` for every in-range index unconditionally. An + // index STORE does create an ordinary own property, so consult the + // expando table before answering `false`. + if crate::buffer::is_registered_buffer(obj_addr as usize) + && crate::buffer::is_non_indexed_buffer_view(obj_addr as usize) + { + if let Some(name) = crate::buffer::canonical_index_key(f64::from_bits(key_val.bits())) { + return if crate::buffer::buffer_has_own_prop(obj_addr as usize, &name) { + nanbox_true + } else { + nanbox_false + }; + } + if key_val.is_any_string() { + let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + if let Some(name) = unsafe { crate::string::js_string_key_bytes(key_val, &mut sso) } + .and_then(|b| std::str::from_utf8(b).ok()) + { + if matches!(name, "length" | "BYTES_PER_ELEMENT") { + return nanbox_false; + } + if is_canonical_numeric_index_string(name) { + return if crate::buffer::buffer_has_own_prop(obj_addr as usize, name) { + nanbox_true + } else { + nanbox_false + }; + } + } + } + } if crate::buffer::is_registered_buffer(obj_addr as usize) { let buf = obj_addr as *const crate::buffer::BufferHeader; let len = crate::buffer::js_buffer_length(buf); diff --git a/crates/perry-runtime/src/object/has_own_helpers.rs b/crates/perry-runtime/src/object/has_own_helpers.rs index 2d07124496..bd6b1d185c 100644 --- a/crates/perry-runtime/src/object/has_own_helpers.rs +++ b/crates/perry-runtime/src/object/has_own_helpers.rs @@ -144,6 +144,18 @@ pub(super) unsafe fn buffer_own_key_present( let Some(key_name) = string_header_as_str(key) else { return false; }; + // #8149: an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` is a registered + // buffer with NO integer-indexed own properties, so + // `Object.prototype.hasOwnProperty.call(dv, "0")` is `false` in node. Asked + // ABOVE the bounds test, which answers `true` for every in-range index. An + // index STORE does create an ordinary own property, so consult the expando + // table rather than answering a flat `false`. + if crate::buffer::is_non_indexed_buffer_view(buf as usize) { + return crate::buffer::buffer_has_own_prop(buf as usize, key_name); + } + if crate::buffer::buffer_has_own_prop(buf as usize, key_name) { + return true; + } let Some(index) = super::canonical_array_index(key_name) else { return false; }; diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index c9fde372e9..4564e26448 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -72,7 +72,7 @@ mod delete_rest; mod descriptors; mod disposable_proto_thunks; pub(crate) mod exotic_expando; -mod field_get_set; +pub(crate) mod field_get_set; pub(crate) use field_get_set::scan_accessor_receiver_override_root_mut; mod field_set_by_name; mod gc_slots; diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 1a3ab82587..4c84a6ff41 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -21,6 +21,9 @@ mod string_methods; mod dispatch_arg_coercion_tests; #[cfg(test)] mod probe_dispatch_tests; +#[cfg(test)] +/// #8139: `toLocaleString` on an array / typed-array / buffer receiver. +mod to_locale_string_tests; mod typed_array; use disposal::{ diff --git a/crates/perry-runtime/src/object/native_call_method/object_proto.rs b/crates/perry-runtime/src/object/native_call_method/object_proto.rs index 71460c4a45..3bc83349b8 100644 --- a/crates/perry-runtime/src/object/native_call_method/object_proto.rs +++ b/crates/perry-runtime/src/object/native_call_method/object_proto.rs @@ -158,6 +158,74 @@ pub(crate) unsafe fn js_object_default_to_locale_string(receiver: f64) -> f64 { ) }; } + // #8139: an ARRAY, TYPED ARRAY or BUFFER receiver. + // + // The HIR folds the zero-arg `x.toLocaleString()` on ANY receiver to + // `Expr::DateToLocaleString` (`lower/expr_call/url_date_instance.rs`), so + // this function — not the method-dispatch tower — is where every such call + // is answered. It had arms for number / Date / Temporal / BigInt / + // primitive and then fell through to `Object.prototype.toLocaleString`'s + // `Invoke(O, "toString")`, which for these three receivers renders + // `"[object Array]"` / `"[object Int32Array]"` / `"[object Uint8Array]"`. + // Node joins the per-element `toLocaleString`s: `"3,1,2"`. + // + // The plain-array row is the tell that this is not a receiver-reroute bug + // in the array helpers — `js_array_to_locale_string` was always correct and + // is what the ARGUMENT-bearing form (`arr.toLocaleString("en-US")`, which + // does NOT fold and goes down the ordinary tower) has always reached. What + // was missing is this path's arm, so the two spellings disagreed. + // + // Each case delegates to the exact helper the tower would have used, rather + // than re-implementing the join: `js_native_call_method` is NOT an option + // here, because its `common_methods` `toLocaleString` arm calls straight + // back into this function. + if jsval.is_pointer() { + let addr = jsval.as_pointer::() as usize; + if crate::value::addr_class::is_above_handle_band(addr) { + // Buffer / `Uint8Array`: the buffer dispatcher owns the + // Buffer-vs-`%TypedArray%` split (a `Buffer` decodes its bytes, a + // `Uint8Array` joins them), so ask it rather than deciding here. + // + // `is_byte_indexed_buffer` (#8149) and not `is_registered_buffer`: + // an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` is a + // registered buffer with NO `toLocaleString` of its own, so node + // gives it `Object.prototype.toLocaleString` → + // `"[object ArrayBuffer]"` / `"[object DataView]"`. Routing them to + // the dispatcher would hand back the utf8 DECODE of their backing + // bytes — a new wrong answer, and a byte leak. + if crate::buffer::is_byte_indexed_buffer(addr) { + return crate::object::dispatch_buffer_method( + addr, + "toLocaleString", + std::ptr::null(), + 0, + ); + } + if crate::typedarray::lookup_typed_array_kind(addr).is_some() { + if let Some(result) = super::dispatch_typed_array_method( + addr as *mut crate::typedarray::TypedArrayHeader, + "toLocaleString", + std::ptr::null(), + 0, + ) { + return result; + } + } + let obj_type = crate::value::addr_class::try_read_gc_header(addr) + .map(|header| header.obj_type) + .unwrap_or(0); + if obj_type == crate::gc::GC_TYPE_ARRAY || obj_type == crate::gc::GC_TYPE_LAZY_ARRAY { + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + let s = crate::array::js_array_to_locale_string( + addr as *const crate::array::ArrayHeader, + undef, + undef, + ); + return f64::from_bits(JSValue::string_ptr(s).bits()); + } + } + } + // An own `toLocaleString` closure wins over the default rendering — // notably `%TypedArray%.prototype.toLocaleString()` invoked as a method ON // the prototype object itself must run the installed brand-check thunk diff --git a/crates/perry-runtime/src/object/native_call_method/to_locale_string_tests.rs b/crates/perry-runtime/src/object/native_call_method/to_locale_string_tests.rs new file mode 100644 index 0000000000..604dd87ffa --- /dev/null +++ b/crates/perry-runtime/src/object/native_call_method/to_locale_string_tests.rs @@ -0,0 +1,156 @@ +//! #8139: `toLocaleString` on an array / typed-array / buffer receiver. +//! +//! ## Why the tests go through `js_value_to_locale_string` +//! +//! That is the ONLY entry point the source-level zero-arg call reaches. The +//! HIR folds `x.toLocaleString()` on ANY receiver to `Expr::DateToLocaleString` +//! (`lower/expr_call/url_date_instance.rs`), and codegen lowers the +//! non-Number case to `js_value_to_locale_string`. The method-dispatch tower — +//! which has been answering correctly all along — is only reached by the +//! ARGUMENT-bearing spelling, which does not fold. Testing the tower would +//! therefore have passed before the fix. +//! +//! ## Why the assertions are exact strings, not "not [object …]" +//! +//! `join()` and `toLocaleString()` differ ONLY in digit grouping: node's +//! `new Int32Array([1234567, 2]).toLocaleString()` is `"1,234,567,2"` while +//! `.join()` is `"1234567,2"`. A test that only checked "did we stop saying +//! `[object Int32Array]`?" would pass against a `join` delegation, which is +//! the wrong answer and the one an obvious implementation reaches for. Every +//! expectation below was measured against node `26.5.1` (the `.node-version` +//! pin). + +use crate::value::JSValue; + +fn locale_string(receiver: f64) -> String { + let result = crate::object::js_value_to_locale_string(receiver); + let ptr = crate::value::js_get_string_pointer_unified(result) as *const crate::StringHeader; + if ptr.is_null() { + return String::new(); + } + unsafe { + let len = (*ptr).byte_len as usize; + let data = (ptr as *const u8).add(std::mem::size_of::()); + String::from_utf8_lossy(std::slice::from_raw_parts(data, len)).into_owned() + } +} + +fn boxed(addr: usize) -> f64 { + f64::from_bits(JSValue::pointer(addr as *const u8).bits()) +} + +fn plain_array(values: &[f64]) -> f64 { + let arr = crate::array::js_array_alloc(values.len() as u32); + let mut arr = arr; + for v in values { + arr = crate::array::js_array_push_f64(arr, *v); + } + boxed(arr as usize) +} + +fn typed(kind: u8, values: &[f64]) -> f64 { + let ta = crate::typedarray::typed_array_alloc(kind, values.len() as u32); + for (i, v) in values.iter().enumerate() { + crate::typedarray::js_typed_array_set(ta, i as i32, *v); + } + boxed(ta as usize) +} + +fn buffer(bytes: &[u8]) -> *mut crate::buffer::BufferHeader { + let buf = crate::buffer::buffer_alloc(bytes.len() as u32); + unsafe { + (*buf).length = bytes.len() as u32; + std::ptr::copy_nonoverlapping( + bytes.as_ptr(), + crate::buffer::buffer_data_mut(buf), + bytes.len(), + ); + } + buf +} + +// --------------------------------------------------------------------------- + +#[test] +fn a_plain_array_joins_its_elements_locale_strings() { + // node: `[3,1,2].toLocaleString()` === "3,1,2". + assert_eq!(locale_string(plain_array(&[3.0, 1.0, 2.0])), "3,1,2"); + // The grouping is the discriminator against a `join()` delegation. + // node: `[1234567,2].toLocaleString()` === "1,234,567,2". + assert_eq!(locale_string(plain_array(&[1234567.0, 2.0])), "1,234,567,2"); + // node: `[].toLocaleString()` === "". + assert_eq!(locale_string(plain_array(&[])), ""); +} + +#[test] +fn a_typed_array_joins_its_elements_locale_strings() { + // node: `new Int32Array([3,1,2]).toLocaleString()` === "3,1,2". + assert_eq!( + locale_string(typed(crate::typedarray::KIND_INT32, &[3.0, 1.0, 2.0])), + "3,1,2" + ); + // node: "1,234,567,2" — NOT `join()`'s "1234567,2". + assert_eq!( + locale_string(typed(crate::typedarray::KIND_INT32, &[1234567.0, 2.0])), + "1,234,567,2" + ); + // node: `new Float64Array([1.5, 1234567.25]).toLocaleString()` === + // "1.5,1,234,567.25". + assert_eq!( + locale_string(typed(crate::typedarray::KIND_FLOAT64, &[1.5, 1234567.25])), + "1.5,1,234,567.25" + ); +} + +#[test] +fn a_uint8array_joins_its_bytes_and_a_buffer_decodes_them() { + // The Buffer-vs-`Uint8Array` split. Both are the same `BufferHeader` in + // perry, and `Buffer.prototype.toLocaleString` is an OWN override that a + // plain `Uint8Array` does not inherit. + let u8a = buffer(&[3, 1, 2]) as usize; + crate::buffer::mark_as_uint8array(u8a); + // node: `new Uint8Array([3,1,2]).toLocaleString()` === "3,1,2". + assert_eq!(locale_string(boxed(u8a)), "3,1,2"); + + // node: `Buffer.from([104,105]).toLocaleString()` === "hi" (it delegates to + // `toString()`, i.e. the utf8 decode). The bytes are printable so the + // assertion can name the exact string. + let buf = buffer(&[104, 105]) as usize; + assert_eq!(locale_string(boxed(buf)), "hi"); +} + +#[test] +fn the_existing_receivers_are_unchanged() { + // CONTROLS. The new arms sit above the `Object.prototype.toLocaleString` + // tail, so these prove they declined every receiver that already worked. + // node: 12345 → "12,345"; ({}) → "[object Object]"; "hi" → "hi"; + // true → "true". + assert_eq!(locale_string(12345.0), "12,345"); + let obj = crate::object::js_object_alloc(0, 1); + assert_eq!(locale_string(boxed(obj as usize)), "[object Object]"); + let s = crate::string::js_string_from_bytes(b"hi".as_ptr(), 2); + assert_eq!( + locale_string(f64::from_bits(JSValue::string_ptr(s).bits())), + "hi" + ); + assert_eq!( + locale_string(f64::from_bits(crate::value::TAG_TRUE)), + "true" + ); +} + +#[test] +fn an_array_buffer_and_data_view_keep_the_object_tag() { + // CONTROL + scope note. node: `new ArrayBuffer(2).toLocaleString()` === + // "[object ArrayBuffer]" (Object.prototype.toLocaleString → toString). + // Perry renders "[object Uint8Array]" for both, which is a pre-existing + // `Symbol.toStringTag` gap, NOT something the new buffer arm introduced — + // the arm routes them to `dispatch_buffer_method`, whose `toLocaleString` + // decodes, so the assertion here is that they are NOT served the decode. + let ab = buffer(&[104, 105]) as usize; + crate::buffer::mark_as_array_buffer(ab); + assert_ne!(locale_string(boxed(ab)), "hi"); + let dv = buffer(&[104, 105]) as usize; + crate::buffer::mark_as_data_view(dv); + assert_ne!(locale_string(boxed(dv)), "hi"); +} diff --git a/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs b/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs index b90db0a1f6..49d6ed21d7 100644 --- a/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs +++ b/crates/perry-runtime/src/object/object_ops/descriptor_helpers.rs @@ -106,8 +106,18 @@ pub(crate) unsafe fn registered_buffer_index_own_property_present( // `typedarray_props` registry — returning `Some(false)` for them would // shadow that check (`typed_array_has_own_property`) and wrongly report // a defined own property as absent. Fall through with `None` instead. - let idx = super::super::has_own_helpers::str_from_string_header(key_str) - .and_then(super::super::canonical_array_index)?; + let name = super::super::has_own_helpers::str_from_string_header(key_str)?; + let idx = super::super::canonical_array_index(name)?; + // #8149: an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` has NO + // integer-indexed own properties — `Object.prototype.hasOwnProperty + // .call(dv, "0")` and `getOwnPropertyDescriptor(dv, "0")` are `false` / + // `undefined` in node. Asked ABOVE the bounds test, which answers `true` + // for every in-range index. An index STORE does create an ordinary own + // property, so consult the expando table rather than answering a flat + // `false`. + if crate::buffer::is_non_indexed_buffer_view(raw_buffer_addr) { + return Some(crate::buffer::buffer_has_own_prop(raw_buffer_addr, name)); + } let buf = raw_buffer_addr as *const crate::buffer::BufferHeader; Some(idx < (*buf).length) } diff --git a/crates/perry-runtime/src/object/polymorphic_index.rs b/crates/perry-runtime/src/object/polymorphic_index.rs index 6a90272cf0..2f483c6f76 100644 --- a/crates/perry-runtime/src/object/polymorphic_index.rs +++ b/crates/perry-runtime/src/object/polymorphic_index.rs @@ -193,6 +193,19 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> { return value; } + // #8149: `ArrayBuffer` / `SharedArrayBuffer` / `DataView` share the buffer + // registry with `Buffer`/`Uint8Array` but have NO integer-indexed own + // properties — node answers `undefined` for `dv[0]`. Asked ABOVE the byte + // arm, which answers unconditionally. An index store put an ordinary own + // property there (`js_object_set_index_polymorphic`), so read that first. + if crate::buffer::is_registered_buffer(raw as usize) + && crate::buffer::is_non_indexed_buffer_view(raw as usize) + { + if let Some(key) = crate::buffer::canonical_index_key(idx) { + return crate::buffer::buffer_get_own_prop(raw as usize, &key) + .unwrap_or_else(|| f64::from_bits(crate::value::TAG_UNDEFINED)); + } + } if crate::buffer::is_registered_buffer(raw as usize) { let Some(index) = numeric_key_i32_index(idx) else { // A NON-numeric computed key on a Buffer (`buf[k]` where `k` is a @@ -383,6 +396,19 @@ pub extern "C" fn js_object_set_index_polymorphic(obj_handle: i64, idx: f64, val } } + // #8149: an index STORE on an `ArrayBuffer` / `SharedArrayBuffer` / + // `DataView` creates an ordinary own property in node — `dv[0] = 7` leaves + // the byte at 0 and makes `Object.keys(dv)` report `"0"`. Asked ABOVE the + // byte-store arm for the same reason the read side is: that arm writes + // unconditionally. + if crate::buffer::is_registered_buffer(raw as usize) + && crate::buffer::is_non_indexed_buffer_view(raw as usize) + { + if let Some(key) = crate::buffer::canonical_index_key(idx) { + crate::buffer::buffer_set_own_prop(raw as usize, &key, value); + return; + } + } if crate::buffer::is_registered_buffer(raw as usize) { if let Some(index) = numeric_key_i32_index(idx) { crate::buffer::js_buffer_set( diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index 5ffcce253b..e23161f1fe 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -995,6 +995,16 @@ fn set_integer_indexed_exotic(target: f64, key: f64, value: f64) -> bool { let Some(raw) = raw_ptr_from_value(target) else { return false; }; + // #8149: `ArrayBuffer` / `SharedArrayBuffer` / `DataView` are registered + // buffers, but they are NOT integer-indexed exotic objects — `dv[0] = 7` + // creates an ORDINARY own property in node and leaves the byte at 0. + // Answering `false` here hands the write back to `js_put_value_set`'s + // ordinary `[[Set]]` walk, which would bit-cast the `BufferHeader`, so + // store the expando directly and claim the write. + if crate::buffer::is_non_indexed_buffer_view(raw) { + crate::buffer::buffer_set_own_prop(raw, &index.to_string(), value); + return true; + } if crate::buffer::is_registered_buffer(raw) { crate::buffer::js_buffer_set(raw as *mut crate::buffer::BufferHeader, index, value as i32); return true; diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index eebff17426..8a3500dee1 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -2085,6 +2085,17 @@ pub extern "C" fn js_typed_feedback_array_index_get_fallback_boxed( ); } + // #8149: `ArrayBuffer` / `SharedArrayBuffer` / `DataView` are registered + // buffers with NO integer-indexed own properties — node answers `undefined` + // for `dv[0]`. Asked ABOVE the byte arm, which answers unconditionally. + if crate::buffer::is_registered_buffer(raw_addr) + && crate::buffer::is_non_indexed_buffer_view(raw_addr) + { + if let Some(key) = crate::buffer::canonical_index_key(index) { + return crate::buffer::buffer_get_own_prop(raw_addr, &key) + .unwrap_or_else(|| f64::from_bits(TAG_UNDEFINED)); + } + } if crate::buffer::is_registered_buffer(raw_addr) { let Some(index) = finite_nonnegative_i32_index(index) else { return f64::from_bits(TAG_UNDEFINED); @@ -2413,6 +2424,16 @@ pub extern "C" fn js_typed_feedback_array_index_set_fallback_boxed( return receiver; } + // #8149: an index store on an `ArrayBuffer` / `SharedArrayBuffer` / + // `DataView` creates an ordinary own property, it does not write a byte. + if crate::buffer::is_registered_buffer(raw_addr) + && crate::buffer::is_non_indexed_buffer_view(raw_addr) + { + if let Some(key) = crate::buffer::canonical_index_key(index) { + crate::buffer::buffer_set_own_prop(raw_addr, &key, value); + return receiver; + } + } if crate::buffer::is_registered_buffer(raw_addr) { if let Some(index) = finite_nonnegative_i32_index(index) { crate::buffer::js_buffer_set( diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index 40e3530338..491311bdc2 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -271,6 +271,20 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { index, ); } + // #8149: an `ArrayBuffer` / `SharedArrayBuffer` / `DataView` is a registered + // buffer too, but it is NOT an integer-indexed exotic object — node answers + // `undefined` for `dv[0]`, never the byte. Ask that ABOVE the byte arm: the + // arm below answers unconditionally, so a re-check placed after it is dead + // code. An index STORE created an ordinary own property (see + // `js_object_set_index_polymorphic`), so consult it before giving up. + if crate::buffer::is_registered_buffer(raw_ptr) + && crate::buffer::is_non_indexed_buffer_view(raw_ptr) + { + if let Some(key) = crate::buffer::canonical_index_key(index) { + return crate::buffer::buffer_get_own_prop(raw_ptr, &key) + .unwrap_or_else(|| f64::from_bits(TAG_UNDEFINED)); + } + } if crate::buffer::is_registered_buffer(raw_ptr) { let buf = raw_ptr as *const crate::buffer::BufferHeader; if let Some(idx_i32) = finite_nonnegative_i32_index(index) { @@ -613,6 +627,18 @@ pub extern "C" fn js_dyn_index_set(obj: f64, index: f64, value: f64) -> f64 { ); return value; } + // #8149: an index STORE on an `ArrayBuffer` / `SharedArrayBuffer` / + // `DataView` creates an ORDINARY own property — `dv[0] = 7` leaves the byte + // at 0, and `Object.keys(dv)` afterwards is `["0"]`. Asked above the + // byte-store arm, which writes unconditionally. + if crate::buffer::is_registered_buffer(raw_ptr) + && crate::buffer::is_non_indexed_buffer_view(raw_ptr) + { + if let Some(key) = crate::buffer::canonical_index_key(index) { + crate::buffer::buffer_set_own_prop(raw_ptr, &key, value); + return value; + } + } if crate::buffer::is_registered_buffer(raw_ptr) { if let Some(idx_i32) = finite_nonnegative_i32_index(index) { crate::buffer::js_buffer_set( diff --git a/crates/perry-runtime/src/value/dynamic_object.rs b/crates/perry-runtime/src/value/dynamic_object.rs index abc40620f3..c8872c9eab 100644 --- a/crates/perry-runtime/src/value/dynamic_object.rs +++ b/crates/perry-runtime/src/value/dynamic_object.rs @@ -446,6 +446,16 @@ pub unsafe extern "C" fn js_dynamic_object_get_property( if crate::buffer::is_registered_buffer(ptr as usize) { let buf = ptr as *const crate::buffer::BufferHeader; match property_name { + // #8149: `length` is a `%TypedArray%` slot. An `ArrayBuffer` / + // `SharedArrayBuffer` / `DataView` exposes only `byteLength`, so + // node answers `undefined` for `dv.length` and `ab.length`. Asked + // ABOVE the shared arm, which answers the byte count for both + // spellings. (`js_value_length_f64`, the deliberately-numeric + // sibling above, is left alone: its callers feed the result through + // `ToLength`, where `undefined` and `0` are the same answer.) + "length" if crate::buffer::is_non_indexed_buffer_view(ptr as usize) => { + return f64::from_bits(TAG_UNDEFINED); + } "length" | "byteLength" => { return (*buf).length as f64; } From 7326be47bc2ea3a06b93b5242c5de8eb9127c16a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 16 Aug 2026 01:07:37 +0200 Subject: [PATCH 2/2] docs(changelog): add the #8186 fragment for #8149 / #8139 --- changelog.d/8186-buffer-view-receiver-kind.md | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 changelog.d/8186-buffer-view-receiver-kind.md diff --git a/changelog.d/8186-buffer-view-receiver-kind.md b/changelog.d/8186-buffer-view-receiver-kind.md new file mode 100644 index 0000000000..647ac4c4b6 --- /dev/null +++ b/changelog.d/8186-buffer-view-receiver-kind.md @@ -0,0 +1,94 @@ +Fixed two independent conformance bugs that both live one receiver-kind question +away from a buffer fast path (#8149, #8139). + +**#8149 — `DataView` and raw `ArrayBuffer` were byte-indexable.** Perry backs +FOUR distinct JS types with the same `BufferHeader` and the same +`BUFFER_REGISTRY` entry: a node `Buffer`, a `Uint8Array`, an +`ArrayBuffer`/`SharedArrayBuffer`, and a `DataView`. Only the first two are +integer-indexed exotic objects, but every consumer that triaged a receiver as +"registered buffer ⇒ byte indexable" served all four. `dv[0]` answered the byte +where node answers `undefined`; `dv.length` / `ab.length` answered the byte +count where node answers `undefined`; `0 in dv` and +`hasOwnProperty.call(dv,"0")` answered `true` where node answers `false`; and +`dv[0] = 7` OVERWROTE a byte where node creates an ordinary own property and +leaves the byte at 0. + +The `Object.keys` half was a memory-safety bug, not merely a wrong answer. The +enumeration paths had no registered-buffer arm at all and fell through to the +generic walk, which reads a `BufferHeader`'s PAYLOAD as +`ObjectHeader.keys_array` and then calls `js_array_length` on it. That answered +`[]` whenever those bytes happened to be zero — which is all +`Object.keys(Buffer.from([1,2,3]))` looked like — and SIGBUS'd (exit 138, +`js_array_length` ← `js_object_keys`) when they did not: + +```ts +const ab = new ArrayBuffer(8); +const D = new DataView(ab); +const b = Buffer.from([1, 2, 3]); // required: it changes the allocation layout +console.log(JSON.stringify(Object.keys(D))); +``` + +`Object.entries(b)` and `Object.values(b)` reached the same walk. Also fixed +along the same arm: `Object.getOwnPropertyNames(b)`, `{...b}` / +`Object.assign({}, b)` (both were `{}`), and `JSON.stringify(dv)` / +`JSON.stringify(ab)`, which emitted `{"type":"Buffer","data":[…]}` — a shape +node never produces for those receivers, and one that leaks the backing bytes. + +New `buffer::exotic_view` owns the discrimination +(`is_non_indexed_buffer_view`, `is_byte_indexed_buffer`, +`canonical_index_key`), and every call site asks it ABOVE the arm it guards, +never below: the byte arm answers unconditionally, so a re-check placed after +it is dead code. That is the ordering #8090 / #8109 / #8119 / #8120 / #8124 / +#8140 / #8141 / #8148 / #8173 each had to restore. Both buffer backings are +covered by construction — `is_registered_buffer` is a side-table membership +test, not an address-range or GC-header probe, so an EXTERNAL buffer (no +`GcHeader` at all; see `array/header.rs`'s `array_receiver_gc_tag` doc, #8142) +is classified by exactly the same lookups as an arena-backed one. + +**#8139 — `toLocaleString` rendered `[object Array]` for every array.** A +different cause, one level further out. The HIR folds the zero-arg +`x.toLocaleString()` on ANY receiver to `Expr::DateToLocaleString` +(`lower/expr_call/url_date_instance.rs`), so `js_object_default_to_locale_string` +— not the method-dispatch tower — is where every such call is answered. It had +arms for number / `Date` / `Temporal` / `BigInt` / primitives and then fell +through to `Object.prototype.toLocaleString`'s `Invoke(O, "toString")`. The +plain-array row is the tell that this is NOT a reroute bug in the array +helpers: `js_array_to_locale_string` was always correct, and the +argument-bearing spelling (`arr.toLocaleString("en-US")`, which does not fold) +has always reached it — the two spellings simply disagreed. Each new arm +delegates to the exact helper the tower would have used; `js_native_call_method` +is deliberately not used, because its `common_methods` `toLocaleString` arm +calls straight back into this function. `dispatch_buffer_method`'s +`toLocaleString` additionally declines a `is_uint8array_buffer`-marked receiver +so it reaches `uint8_join`: `Buffer.prototype.toLocaleString` is an OWN +override, and a plain `Uint8Array` inherits the `%TypedArray%` join +(`new Uint8Array([3,1,2]).toLocaleString()` is `"3,1,2"`, not three raw bytes). + +Verified byte-for-byte against node `26.5.1` (the `.node-version` pin) across 7 +probe programs / 96 rows. 22 tests in `buffer/exotic_view_tests.rs` and +`object/native_call_method/to_locale_string_tests.rs`, all asserting OBSERVED +VALUES, never predicates: a `DataView`'s bytes are zero-filled, so a probe +asking "is `dv[0]` falsy?" passes under the bug, and the typed-array +`toLocaleString` cases assert the digit GROUPING (`"1,234,567,2"`) because that +is the only thing separating a correct implementation from a `join()` +delegation. The store-side tests assert BOTH halves — the expando exists AND +the byte is still zero — since a fix doing both would pass either alone. Six +sabotage arms were run and reverted: predicate always false (10 subject tests +fail, 6 controls pass), predicate over-reaching to every registered buffer (8 +controls fail), enumeration arm deleted (exactly the 3 enumeration tests fail), +`toLocaleString` arms deleted (3 of 5 fail), the `!is_uint8array_buffer` gate +removed (the `Uint8Array` test fails), and the typed-array arm delegating to +`join()` (the grouping test fails). One of my own measurements was vacuous and +was replaced: an assertion read `Object.values(buffer)` through the string-key +helper, which would have passed either way. + +`cargo test -p perry-runtime --lib`: 2467 passed, 0 failed, 4 ignored. + +Deliberately out of scope, each with its reason recorded in the code: +`for…in` over a `Buffer` (node also enumerates `Buffer.prototype`'s ~100 +enumerable methods); `new Uint8Array([3,1,2]).toString()` (#8139 part 2 — the +gate is one line, but `is_uint8array_buffer` is not a real `Buffer` brand: +`KeyObject.export()` marks its Buffer result so `instanceof Uint8Array` holds, +and unlike `toLocaleString` the pre-state here is a widely-relied-on decode); +own expandos inside `JSON.stringify(dv)`; and `dv[-1] = 1`, whose store perry +drops for a `Buffer` as well.