From a09124d784b2f938b772dbb35a6703673469d5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 20:53:39 +0200 Subject: [PATCH 1/8] fix(error): fs diagnostics become own properties of the error, not the message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An fs error's code/errno/syscall/path lived in six side tables keyed by the MESSAGE STRING's address. Two consequences, both fixed here. WRONG ERROR. Any `new Error(m)` built from the same message text picked up the unrelated fs error's fields: new Error(fsErr.message).code // ENOENT, node says undefined .syscall / .errno / .path // stat / -2 / the path The metadata belonged to the string, so anything holding that string answered to it. INVISIBLE TO REFLECTION. In node these are ordinary own properties. Served from a side table behind property getters they were absent from every enumeration path: Object.keys(e) [] -> code,errno,path,syscall hasOwnProperty('code') false -> true getOwnPropertyDescriptor undefined -> {value,writable,enumerable,configurable} JSON.stringify(e) {} -> {"errno":-2,"code":"ENOENT",...} {...e} {} -> same Any code logging or serialising a caught fs error lost its whole payload. Three sites each held a different wrong assumption about errors: * the fs builders keyed on the message string; * JSON.stringify hardcoded "{}" for GC_TYPE_ERROR — right for a plain error (message/name/stack are non-enumerable), wrong once the error has enumerable own props, so it also dropped user-assigned ones; * Object.assign/spread had no Error arm, so it copied nothing. All three now enumerate through exotic_own_keys(.., enumerable_only), the same enumeration Object.keys uses, so they cannot drift apart. ORDER. ERROR_USER_PROPS was a HashMap with an alphabetical sort_by for determinism — stable but not node's. It is insertion-ordered now, with reassignment keeping a key's original position, and the fs fields are installed in node's uvException order (errno, code, syscall, path, dest). The GC root scanner over these props moved to the ordered store. Verified against node on the claude-code host: three repro programs are byte-identical including key order. Suite 2746 passed. --- crates/perry-runtime/src/fs/errors.rs | 66 ++++++++--- crates/perry-runtime/src/json/stringify.rs | 79 +++++++++++-- .../src/node_submodules/diagnostics.rs | 105 ++++++++++++++++-- .../src/node_submodules/diagnostics_gc.rs | 2 +- crates/perry-runtime/src/object/alloc.rs | 41 +++++++ 5 files changed, 259 insertions(+), 34 deletions(-) diff --git a/crates/perry-runtime/src/fs/errors.rs b/crates/perry-runtime/src/fs/errors.rs index 417c979b40..16df1c3226 100644 --- a/crates/perry-runtime/src/fs/errors.rs +++ b/crates/perry-runtime/src/fs/errors.rs @@ -102,6 +102,54 @@ pub(crate) fn io_error_errno(err: &std::io::Error) -> i32 { } } +/// Attach Node's fs diagnostic fields to `err_ptr` as **own properties of the +/// error object**. +/// +/// These used to be registered in six side tables keyed by the MESSAGE +/// STRING's address (`register_error_code_pub` and friends), which produced two +/// defects: +/// +/// * **Wrong error.** Any `new Error(m)` built from the same message text +/// inherited the unrelated fs error's fields — `new Error(e.message).code` +/// returned `ENOENT` where node returns `undefined`, along with `.syscall`, +/// `.errno` and `.path`. Metadata belonged to the string, not the throw. +/// * **Invisible to reflection.** In node these are ordinary own properties: +/// `Object.keys(e)` is `code,errno,path,syscall`, and `JSON.stringify(e)` and +/// `{...e}` carry them. Served from a side table behind property *getters* +/// they were absent from all of it — perry returned `{}` for both, so any +/// code that logs or serialises an fs error silently lost every field. +/// +/// Keying on the error object fixes both at once, and each field then reaches +/// reflection through the same path a user assignment does. +unsafe fn attach_fs_error_props( + err_ptr: *mut crate::error::ErrorHeader, + code: &str, + errno: i32, + syscall: &str, + path: Option<&str>, + dest: Option<&str>, +) { + use crate::node_submodules::set_error_user_prop; + let owner = err_ptr as usize; + let put_str = |key: &str, s: &str| { + let boxed = js_string_from_bytes(s.as_ptr(), s.len() as u32); + set_error_user_prop(owner, key, crate::value::js_nanbox_string(boxed as i64)); + }; + // Insertion order is observable — `Object.keys`, `for…in`, `{...err}` and + // `JSON.stringify` all report it — so install these in the same order + // node's `uvException` does: errno, code, syscall, path, dest. + // `errno` is numeric in node (-2 for ENOENT), not a string. + set_error_user_prop(owner, "errno", errno as f64); + put_str("code", code); + put_str("syscall", syscall); + if let Some(p) = path { + put_str("path", p); + } + if let Some(d) = dest { + put_str("dest", d); + } +} + pub(crate) unsafe fn build_fs_error_value( err: &std::io::Error, syscall: &'static str, @@ -112,13 +160,7 @@ pub(crate) unsafe fn build_fs_error_value( let msg = format!("{}: {}, {} '{}'", code, err, syscall, path); let msg_ptr = js_string_from_bytes(msg.as_ptr(), msg.len() as u32); let err_ptr = crate::error::js_error_new_with_message(msg_ptr); - // Register code/syscall/path in the per-message side tables so the - // `.code`, `.syscall`, `.path` property getters in `field_get_set` - // surface Node-compatible values on caught errors. - crate::node_submodules::register_error_code_pub(msg_ptr, code); - crate::node_submodules::register_error_errno(msg_ptr, errno); - crate::node_submodules::register_error_syscall(msg_ptr, syscall); - crate::node_submodules::register_error_path(msg_ptr, path.to_string()); + attach_fs_error_props(err_ptr, code, errno, syscall, Some(path), None); crate::value::js_nanbox_pointer(err_ptr as i64) } @@ -136,11 +178,7 @@ pub(crate) unsafe fn build_fs_error_value_with_dest( let msg = format!("{}: {}, {} '{}' -> '{}'", code, err, syscall, path, dest); let msg_ptr = js_string_from_bytes(msg.as_ptr(), msg.len() as u32); let err_ptr = crate::error::js_error_new_with_message(msg_ptr); - crate::node_submodules::register_error_code_pub(msg_ptr, code); - crate::node_submodules::register_error_errno(msg_ptr, errno); - crate::node_submodules::register_error_syscall(msg_ptr, syscall); - crate::node_submodules::register_error_path(msg_ptr, path.to_string()); - crate::node_submodules::register_error_dest(msg_ptr, dest.to_string()); + attach_fs_error_props(err_ptr, code, errno, syscall, Some(path), Some(dest)); crate::value::js_nanbox_pointer(err_ptr as i64) } @@ -153,9 +191,7 @@ pub(crate) unsafe fn build_fs_error_value_no_path( let msg = format!("{}: {}, {}", code, err, syscall); let msg_ptr = js_string_from_bytes(msg.as_ptr(), msg.len() as u32); let err_ptr = crate::error::js_error_new_with_message(msg_ptr); - crate::node_submodules::register_error_code_pub(msg_ptr, code); - crate::node_submodules::register_error_errno(msg_ptr, errno); - crate::node_submodules::register_error_syscall(msg_ptr, syscall); + attach_fs_error_props(err_ptr, code, errno, syscall, None, None); crate::value::js_nanbox_pointer(err_ptr as i64) } diff --git a/crates/perry-runtime/src/json/stringify.rs b/crates/perry-runtime/src/json/stringify.rs index b48a9e8ee0..6718792ea6 100644 --- a/crates/perry-runtime/src/json/stringify.rs +++ b/crates/perry-runtime/src/json/stringify.rs @@ -357,6 +357,56 @@ pub(crate) unsafe fn arm_to_json_result_guard(result: f64) { } } +/// Serialize an `Error`'s own ENUMERABLE properties, the way node does. +/// +/// Errors do not have the JSObject keys/values layout, so they cannot go +/// through `stringify_object` — but they are still ordinary property bearers to +/// an observer. `exotic_own_keys(.., enumerable_only = true)` is the same +/// enumeration `Object.keys` uses on an error, so JSON output and `Object.keys` +/// cannot disagree. +/// +/// `depth` is `Some` when called from the depth-tracking variant, so nested +/// values keep the cycle/recursion budget of the caller. +unsafe fn stringify_error_own_props(ptr: *const u8, buf: &mut String, depth: Option) { + let ptr = ptr as usize; + use crate::object::exotic_expando::{exotic_get_own_property, exotic_own_keys, ExoticKind}; + let keys = exotic_own_keys(ExoticKind::Error, ptr, true); + buf.push('{'); + let mut first = true; + for key in keys { + let Some(v) = exotic_get_own_property( + ptr, + ExoticKind::Error, + &key, + f64::from_bits(bits_of_ptr(ptr)), + ) else { + continue; + }; + // `undefined` own properties are omitted from objects, per JSON.stringify. + if v.to_bits() == crate::value::TAG_UNDEFINED { + continue; + } + if !first { + buf.push(','); + } + first = false; + write_escaped_string(buf, &key); + buf.push(':'); + match depth { + Some(d) => stringify_value_depth(v, 0, buf, d + 1), + None => stringify_value(v, 0, buf), + } + } + buf.push('}'); +} + +/// NaN-box `ptr` back into the pointer value an exotic `[[Get]]` wants as its +/// `receiver` (used only to rebind `this` for an accessor property). +#[inline] +fn bits_of_ptr(ptr: usize) -> u64 { + crate::value::js_nanbox_pointer(ptr as i64).to_bits() +} + #[inline] pub(crate) unsafe fn stringify_value(value: f64, type_hint: u32, buf: &mut String) { let bits: u64 = value.to_bits(); @@ -554,15 +604,24 @@ pub(crate) unsafe fn stringify_value(value: f64, type_hint: u32, buf: &mut Strin } } crate::gc::GC_TYPE_ERROR => { - // Issue #928: Built-in Error objects (and subclasses - // like TypeError) have a dedicated `ErrorHeader` layout — - // not the JSObject keys/values layout. Routing them - // through `stringify_object` derefs garbage as a - // `keys_array` pointer and segfaults the process. - // Node's `JSON.stringify(new Error("x"))` returns "{}" - // because Error's intrinsic props (`message`, `name`, - // `stack`) are non-enumerable; mirror that. - buf.push_str("{}"); + // Issue #928: Built-in Error objects (and subclasses like + // TypeError) have a dedicated `ErrorHeader` layout — not the + // JSObject keys/values layout — so they must never reach + // `stringify_object`, which would deref garbage as a + // `keys_array` pointer and segfault. + // + // They are NOT always "{}", though. Node emits an error's own + // ENUMERABLE properties like any other object; `{}` is merely + // what a *plain* error produces, because `message`/`name`/ + // `stack` are non-enumerable: + // + // JSON.stringify(new Error("x")) -> {} + // e.foo = 1; JSON.stringify(e) -> {"foo":1} + // JSON.stringify(fsError) -> {"errno":-2,"code":"ENOENT",…} + // + // Hardcoding "{}" silently dropped every one of those, so any + // code that logs a caught error as JSON lost its whole payload. + stringify_error_own_props(ptr, buf, None); } crate::gc::GC_TYPE_MAP | crate::gc::GC_TYPE_SET => { // Map/Set have a `{size, capacity, entries/elements}` header, @@ -770,7 +829,7 @@ pub(crate) unsafe fn stringify_value_depth( } crate::gc::GC_TYPE_ERROR => { // Issue #928: see the matching branch in `stringify_value`. - buf.push_str("{}"); + stringify_error_own_props(ptr, buf, Some(depth)); } crate::gc::GC_TYPE_MAP | crate::gc::GC_TYPE_SET => { // See the matching branch in `stringify_value` — Map/Set diff --git a/crates/perry-runtime/src/node_submodules/diagnostics.rs b/crates/perry-runtime/src/node_submodules/diagnostics.rs index 87be84f73a..b58cbcd4ca 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics.rs @@ -602,7 +602,19 @@ thread_local! { /// string/primitive own properties. Stale entries after a GC move of the /// error are harmless (same model as the message-keyed tables above): a /// lookup at the new address simply misses. - pub(crate) static ERROR_USER_PROPS: RefCell>> = + /// Insertion-ORDERED per error: a `Vec`, not a `HashMap`. + /// + /// ECMA-262 enumerates an object's own string keys in insertion order, and + /// that order is observable through `Object.keys`, `for…in`, `{...err}` and + /// `JSON.stringify`. Backed by a `HashMap` this list came out in hash order, + /// so `error_user_props` sorted it alphabetically to at least be + /// deterministic — which is stable but still not node's order. A caught fs + /// error serialized as `{"code":…,"errno":…,"path":…,"syscall":…}` where + /// node writes `{"errno":…,"code":…,"syscall":…,"path":…}`. + /// + /// An error carries a handful of properties, so a linear scan is cheaper + /// than hashing and the order falls out for free. + pub(crate) static ERROR_USER_PROPS: RefCell>> = RefCell::new(HashMap::new()); } @@ -629,10 +641,14 @@ pub fn set_error_user_prop(error_ptr: usize, key: &str, value: f64) { ErrUserProp::Bits(value.to_bits()) }; ERROR_USER_PROPS.with(|m| { - m.borrow_mut() - .entry(error_ptr) - .or_default() - .insert(key.to_string(), stored); + let mut map = m.borrow_mut(); + let props = map.entry(error_ptr).or_default(); + // Reassigning an existing key keeps its original position — `o.a=1; + // o.b=2; o.a=3` still enumerates `a,b` in node. + match props.iter_mut().find(|(k, _)| k == key) { + Some(slot) => slot.1 = stored, + None => props.push((key.to_string(), stored)), + } }); } @@ -645,7 +661,7 @@ pub fn error_user_prop(error_ptr: usize, key: &str) -> Option { } ERROR_USER_PROPS.with(|m| { m.borrow().get(&error_ptr).and_then(|props| { - props.get(key).map(|v| match v { + props.iter().find(|(k, _)| k == key).map(|(_, v)| match v { ErrUserProp::Str(s) => { let ptr = js_string_from_bytes(s.as_ptr(), s.len() as u32); f64::from_bits(crate::js_nanbox_string(ptr as i64).to_bits()) @@ -666,7 +682,13 @@ pub fn remove_error_user_prop(error_ptr: usize, key: &str) -> bool { ERROR_USER_PROPS.with(|m| { m.borrow_mut() .get_mut(&error_ptr) - .map(|props| props.remove(key).is_some()) + .map(|props| match props.iter().position(|(k, _)| k == key) { + Some(i) => { + props.remove(i); + true + } + None => false, + }) .unwrap_or(false) }) } @@ -701,7 +723,8 @@ pub fn error_user_props(error_ptr: usize) -> Vec<(String, f64)> { (key, materialized) }) .collect(); - props.sort_by(|a, b| a.0.cmp(&b.0)); + // No sort: the Vec is already in insertion order, which is the order + // ECMA-262 specifies and node emits. props } @@ -1998,3 +2021,69 @@ mod tests { DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); } } + +#[cfg(test)] +mod error_prop_order_tests { + use super::*; + + /// Own string keys enumerate in INSERTION order, not hash or alphabetical + /// order. This is observable through `Object.keys`, `for…in`, `{...err}` + /// and `JSON.stringify`, so a caught fs error must serialize as node's + /// `{"errno":…,"code":…,"syscall":…,"path":…}`. + /// + /// The store was a `HashMap` with an alphabetical `sort_by` bolted on for + /// determinism, which is stable but wrong: it emitted `code` before + /// `errno`. Reverting to any unordered container fails this test. + #[test] + fn user_props_enumerate_in_insertion_order() { + let err = 0x4000_1000usize; + for k in ["errno", "code", "syscall", "path"] { + set_error_user_prop(err, k, 1.0); + } + let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); + assert_eq!( + keys, + vec![ + "errno".to_string(), + "code".to_string(), + "syscall".to_string(), + "path".to_string() + ], + "fs error fields must enumerate in node's insertion order, not sorted" + ); + } + + /// Reassigning an existing key keeps its ORIGINAL position — in node, + /// `o.a=1; o.b=2; o.a=3` still enumerates `a,b`. An implementation that + /// removed-then-appended would report `b,a`. + #[test] + fn reassignment_keeps_original_position() { + let err = 0x4000_2000usize; + set_error_user_prop(err, "a", 1.0); + set_error_user_prop(err, "b", 2.0); + set_error_user_prop(err, "a", 3.0); + let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); + assert_eq!(keys, vec!["a".to_string(), "b".to_string()]); + assert_eq!( + error_user_prop(err, "a"), + Some(3.0), + "reassignment must still update the value" + ); + } + + /// Removing a key must not disturb the order of the survivors. + #[test] + fn removal_preserves_order_of_the_rest() { + let err = 0x4000_3000usize; + for k in ["one", "two", "three"] { + set_error_user_prop(err, k, 0.0); + } + assert!(remove_error_user_prop(err, "two")); + let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); + assert_eq!(keys, vec!["one".to_string(), "three".to_string()]); + assert!( + !remove_error_user_prop(err, "two"), + "second remove is a no-op" + ); + } +} diff --git a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs index b06b7eda7b..c34e48e267 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs @@ -133,7 +133,7 @@ pub(crate) fn finalize_dead_copied_minor_from_space_errors() { pub(crate) fn scan_error_user_props_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { ERROR_USER_PROPS.with(|m| { for props in m.borrow_mut().values_mut() { - for v in props.values_mut() { + for (_, v) in props.iter_mut() { if let ErrUserProp::Bits(bits) = v { visitor.visit_nanbox_u64_slot(bits); } diff --git a/crates/perry-runtime/src/object/alloc.rs b/crates/perry-runtime/src/object/alloc.rs index 482e3af632..79c2d61ca9 100644 --- a/crates/perry-runtime/src/object/alloc.rs +++ b/crates/perry-runtime/src/object/alloc.rs @@ -1543,6 +1543,47 @@ pub unsafe extern "C" fn js_object_assign_one(target_f64: f64, source_f64: f64) // (Stripe's `protoExtend` does `Object.assign(Constructor, Super)` to copy a // resource class's enumerable statics like `.extend`/`.method`; without this // the call hung at `import 'stripe'`.) + // An `Error` source. Like the buffer and closure arms around it, an + // `ErrorHeader` is not the JSObject keys/values layout, so it has no + // `keys_array` for the generic path below to walk — `{...err}` and + // `Object.assign({}, err)` therefore copied NOTHING and produced `{}`. + // + // Node treats an error as an ordinary property bearer here: its own + // ENUMERABLE properties are copied, which for a caught fs error means + // `code`/`errno`/`syscall`/`path`, and for any error means whatever the + // program assigned. `message`/`name`/`stack` stay behind because they are + // non-enumerable — `exotic_own_keys(.., enumerable_only = true)` encodes + // exactly that rule, and is the same enumeration `Object.keys` and + // `JSON.stringify` use, so the three cannot disagree. + if src_raw >= 0x10000 && src_raw.is_multiple_of(8) && { + let src_gc = + (src_raw as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; + (*src_gc).obj_type == crate::gc::GC_TYPE_ERROR + } { + use crate::object::exotic_expando::{exotic_get_own_property, exotic_own_keys, ExoticKind}; + let scope = crate::gc::RuntimeHandleScope::new(); + let tgt_h = scope.root_raw_mut_ptr(target); + let receiver = crate::value::js_nanbox_pointer(src_raw as i64); + for name in exotic_own_keys(ExoticKind::Error, src_raw, true) { + let Some(value) = exotic_get_own_property(src_raw, ExoticKind::Error, &name, receiver) + else { + continue; + }; + 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)); + } + if crate::closure::is_closure_ptr(src_raw) { // #7200: `js_string_from_bytes` and the write funnel both allocate, and // the snapshot's VALUES are heap references held in a plain `Vec` for From 041313d676fba8b9b4fdaeedb187c5ed3e2843f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 20:54:16 +0200 Subject: [PATCH 2/8] changelog: add fragment for #8889 --- changelog.d/8889-error-own-properties.md | 46 ++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 changelog.d/8889-error-own-properties.md diff --git a/changelog.d/8889-error-own-properties.md b/changelog.d/8889-error-own-properties.md new file mode 100644 index 0000000000..fb1dc3fd66 --- /dev/null +++ b/changelog.d/8889-error-own-properties.md @@ -0,0 +1,46 @@ +Fixed an fs error's `code`/`errno`/`syscall`/`path`: they are now own properties +of the **error object** rather than entries in side tables keyed by the error's +**message string address**. + +Two defects followed from that keying, both verified against node. + +**The wrong error got the metadata.** Any `new Error(m)` built from the same +message text inherited an unrelated fs error's fields — `.code` returned +`ENOENT` where node returns `undefined`, along with `.syscall`, `.errno` and +`.path`. The metadata belonged to the string, so anything holding that string +answered to it. + +**They were invisible to reflection.** In node these are ordinary own +properties; served from a side table behind property *getters* they appeared in +none of the enumeration paths: + +| | before | after (= node) | +|---|---|---| +| `Object.keys(e)` | `[]` | `code,errno,path,syscall` | +| `hasOwnProperty('code')` | `false` | `true` | +| `getOwnPropertyDescriptor` | `undefined` | `{value,writable,enumerable,configurable}` | +| `JSON.stringify(e)` | `{}` | `{"errno":-2,"code":"ENOENT",…}` | +| `{...e}` | `{}` | same | + +Any code that logged or serialised a caught fs error lost its whole payload. + +Three sites each held a different wrong assumption about errors. The fs builders +keyed on the message string. `JSON.stringify` hardcoded `"{}"` for +`GC_TYPE_ERROR` — correct for a *plain* error, whose `message`/`name`/`stack` +are non-enumerable, but wrong once an error carries enumerable own properties, +so it also dropped **user-assigned** ones (`e.foo=1; JSON.stringify(e)` gave +`{}` where node gives `{"foo":1}`; that half is independent of fs). +`Object.assign`/spread had no Error arm and copied nothing. All three now +enumerate through `exotic_own_keys(.., enumerable_only = true)` — the same +enumeration `Object.keys` uses — so they cannot drift apart again. + +Property **order** is fixed too. `ERROR_USER_PROPS` was a `HashMap` with an +alphabetical `sort_by` bolted on for determinism: stable, but not node's. Own +string keys enumerate in insertion order per ECMA-262, and that order is +observable through all four paths above. The store is insertion-ordered now, +reassignment keeps a key's original position (`o.a=1; o.b=2; o.a=3` enumerates +`a,b`), and the fs fields install in node's `uvException` order. The GC root +scanner over these properties moved to the ordered store. + +Verified by running three repro programs against node on the same host: +byte-identical output, key order included. From c32643cab9f29824021bff3b2caccadd14c72886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 14:11:41 +0200 Subject: [PATCH 3/8] perf(codegen): brand claimed-array receivers before the guarded plain tier An erased Array declaration admits object-backed Array subclasses (`class Archetype extends Array`) and typed arrays as readily as plain Arrays. The canonical-i32 read split (#8872) committed such a receiver's integer keys to the guarded plain-array tier, whose feedback fallback classifies the receiver out of line on every read; wolf-ecs `packed[sparse[x]]` paid 4-6% of both benchmarks there even after the fallback learned the dense subclass read. The element arm of a claimed-receiver site now reads the GcHeader type byte once: a plain Array keeps the guarded tier, every other heap pointer takes the receiver-unknown numeric tiers (inline typed-array read, dense subclass `arrlike.ic`, complete dispatcher) that the runtime-key arm of the same site already uses, and non-pointers keep the guarded tier's unchanged fallback. Test: `index_get_claim_tests::claimed_array_receiver_brands_before_committing_a_canonical_key_to_the_plain_tier`. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ (cherry picked from commit 8819e362356139322bddb6b2c1734637630bb24d) --- crates/perry-codegen/src/expr/index_get.rs | 85 +++++++++++++++++-- .../src/expr/index_get_claim_tests.rs | 78 +++++++++++++++++ 2 files changed, 154 insertions(+), 9 deletions(-) diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 67bc5db0df..a5232a03cb 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -412,15 +412,82 @@ fn lower_array_index_get_via_canonical_i32_split( .cond_br(&is_canonical_i32, &element_label, &runtime_label); ctx.current_block = element_idx; - let element_value = lower_guarded_array_index_get( - ctx, - arr_box, - &idx_i32, - "aidx.dynamic", - require_numeric_layout, - coerce_numeric_fallback, - receiver_slot, - )?; + let element_value = if preserve_claimed_receiver_fallback { + // An erased Array declaration admits object-backed Array subclasses + // (`class Archetype extends Array` — wolf-ecs `packed[sparse[x]]`) and + // typed arrays as readily as plain Arrays. The guarded plain-array + // tier rejects those on its `GC_TYPE_ARRAY` brand and its feedback + // fallback then classifies the receiver out of line on every read. + // Read the brand once here: a plain Array keeps the guarded tier, + // every other heap pointer takes the receiver-unknown numeric tiers + // (inline typed-array read, dense-subclass `arrlike.ic`, complete + // dispatcher) that the runtime-key arm already uses for the same + // receivers. Non-pointers keep the guarded tier's unchanged fallback. + let brand_idx = ctx.new_block("aidx.claimed.brand"); + let array_idx = ctx.new_block("aidx.claimed.array"); + let other_idx = ctx.new_block("aidx.claimed.other"); + let claimed_merge_idx = ctx.new_block("aidx.claimed.merge"); + let brand_label = ctx.block_label(brand_idx); + let array_label = ctx.block_label(array_idx); + let other_label = ctx.block_label(other_idx); + let claimed_merge_label = ctx.block_label(claimed_merge_idx); + { + let blk = ctx.block(); + let arr_bits = blk.bitcast_double_to_i64(arr_box); + let arr_handle = blk.and(I64, &arr_bits, crate::nanbox::POINTER_MASK_I64); + let tag = blk.lshr(I64, &arr_bits, "48"); + let is_pointer = blk.icmp_eq(I64, &tag, "32765"); // POINTER_TAG + // The same heap band the receiver-unknown tiers dereference in. + let above_handle_band = blk.icmp_ugt(I64, &arr_handle, "1048575"); + let below_heap_limit = blk.icmp_ult(I64, &arr_handle, "140737488355328"); + let in_heap = blk.and(I1, &above_handle_band, &below_heap_limit); + let heap_candidate = blk.and(I1, &is_pointer, &in_heap); + blk.cond_br(&heap_candidate, &brand_label, &array_label); + } + ctx.current_block = brand_idx; + { + let blk = ctx.block(); + let arr_bits = blk.bitcast_double_to_i64(arr_box); + let arr_handle = blk.and(I64, &arr_bits, crate::nanbox::POINTER_MASK_I64); + let gc_type_addr = blk.sub(I64, &arr_handle, "8"); + let gc_type_ptr = blk.inttoptr(I64, &gc_type_addr); + let gc_type = blk.load(I8, &gc_type_ptr); + let is_array = blk.icmp_eq(I8, &gc_type, "1"); // GC_TYPE_ARRAY + blk.cond_br(&is_array, &array_label, &other_label); + } + ctx.current_block = array_idx; + let array_value = lower_guarded_array_index_get( + ctx, + arr_box, + &idx_i32, + "aidx.dynamic", + require_numeric_layout, + coerce_numeric_fallback, + receiver_slot, + )?; + let array_end = ctx.block().label.clone(); + ctx.block().br(&claimed_merge_label); + ctx.current_block = other_idx; + let other_value = + lower_inline_dyn_typed_array_get(ctx, arr_box, idx_double, coerce_numeric_fallback); + let other_end = ctx.block().label.clone(); + ctx.block().br(&claimed_merge_label); + ctx.current_block = claimed_merge_idx; + ctx.block().phi( + DOUBLE, + &[(&array_value, &array_end), (&other_value, &other_end)], + ) + } else { + lower_guarded_array_index_get( + ctx, + arr_box, + &idx_i32, + "aidx.dynamic", + require_numeric_layout, + coerce_numeric_fallback, + receiver_slot, + )? + }; let element_end = ctx.block().label.clone(); ctx.block().br(&merge_label); diff --git a/crates/perry-codegen/src/expr/index_get_claim_tests.rs b/crates/perry-codegen/src/expr/index_get_claim_tests.rs index b2be6a45a2..09e28734bf 100644 --- a/crates/perry-codegen/src/expr/index_get_claim_tests.rs +++ b/crates/perry-codegen/src/expr/index_get_claim_tests.rs @@ -415,6 +415,84 @@ fn any_typed_dynamic_key_takes_the_numeric_tiers_when_it_is_an_array_index() { ); } +/// The canonical-i32 arm of the same `packed[sparse[x]]` site: an erased +/// Array declaration admits object-backed Array subclasses, so a canonical +/// integer key must not be committed to the guarded plain-array tier — whose +/// feedback fallback classifies the receiver out of line on every read (the +/// 2.2× wolf-ecs regression after #8872). The element arm brands the +/// receiver once and sends non-`GC_TYPE_ARRAY` heap pointers to the +/// receiver-unknown numeric tiers instead. +#[test] +fn claimed_array_receiver_brands_before_committing_a_canonical_key_to_the_plain_tier() { + const SPARSE: u32 = 41; + let ir = ir_for( + "claimed_receiver_brand", + vec![ + Stmt::Let { + id: ITEMS, + name: "packed".to_string(), + ty: Type::Array(Box::new(Type::Any)), + mutable: false, + init: Some(Expr::PropertyGet { + object: Box::new(Expr::Object(vec![( + "value".to_string(), + Expr::Array(vec![Expr::Number(7.0)]), + )])), + property: "value".to_string(), + byte_offset: 0, + }), + }, + Stmt::Let { + id: SPARSE, + name: "sparse".to_string(), + ty: Type::Array(Box::new(Type::Any)), + mutable: false, + init: Some(Expr::PropertyGet { + object: Box::new(Expr::Object(vec![( + "value".to_string(), + Expr::Array(vec![Expr::Number(0.0)]), + )])), + property: "value".to_string(), + byte_offset: 0, + }), + }, + Stmt::Let { + id: RESULT, + name: "result".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(ITEMS)), + index: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(SPARSE)), + index: Box::new(Expr::Integer(0)), + }), + }), + }, + ], + ); + assert!( + ir.contains("aidx.canonical") && ir.contains("aidx.claimed.brand"), + "the canonical-i32 arm must brand the claimed receiver before the plain tier:\n{ir}" + ); + let brand = super::class_field_barrier_tests::block_body(&ir, "aidx.claimed.brand") + .expect("the brand block exists"); + assert!( + brand.contains("load i8, ptr") && brand.contains("icmp eq i8") && brand.contains(", 1"), + "the brand block must read the GcHeader type byte and test GC_TYPE_ARRAY:\n{brand}" + ); + assert!( + ir.contains("aidx.claimed.array") && ir.contains("aidx.dynamic.fast"), + "a plain Array keeps the guarded element tier:\n{ir}" + ); + assert!( + ir.contains("aidx.claimed.other") + && ir.matches("arrlike.ic.family_token").count() >= 2 + && ir.matches("tav.get.brand").count() >= 2, + "every other heap receiver must reach the inline typed-array and dense-subclass tiers from BOTH the canonical and the runtime-key arm:\n{ir}" + ); +} + fn dynamic_key_read_ir(name: &str, key_type: Type) -> String { let param = |id, name: &str, ty| Param { id, From fba91ccc002f454a7455d374a02ed1dd538048fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 14:29:53 +0200 Subject: [PATCH 4/8] perf(codegen): gate the guarded store's layout note on the inline classification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `js_gc_note_slot_layout_aware` returns without acting when the old and new slot values share a pointer classification, unless both are pointers and the array carries an element-shape proof (`GC_ARRAY_ELEMENT_SHAPE`). The guarded in-bounds store fast arm still paid the call on every store — 4% of the wolf-ecs add/remove profile, almost all of it `ents[id] = arch` pointer-over- pointer stores into proof-free arrays. The fast arm now stores through a deferred-note variant of the shared slot emitter (old bits loaded, string-addref demote unchanged), classifies both values with an exact codegen mirror of `layout_pointer_bearing_bits`, tests the element-shape bit on the `_reserved` word `deref.live` already loaded, and calls the note only from a gated `laynote` block when it has work: a classification change (which must reach `layout_note_slot`) or a pointer- over-pointer store into a proof-bearing array. Test: `index_set_barrier_tests::the_fast_arm_layout_note_is_gated_on_the_pointer_classification_and_shape_bit`. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ --- .../src/expr/index_set_barrier_tests.rs | 54 +++++++++++++ .../src/expr/index_set_guarded.rs | 79 ++++++++++++++++--- .../perry-codegen/src/expr/write_barrier.rs | 61 ++++++++++++++ 3 files changed, 181 insertions(+), 13 deletions(-) diff --git a/crates/perry-codegen/src/expr/index_set_barrier_tests.rs b/crates/perry-codegen/src/expr/index_set_barrier_tests.rs index 80802025d8..dce9fcb4c4 100644 --- a/crates/perry-codegen/src/expr/index_set_barrier_tests.rs +++ b/crates/perry-codegen/src/expr/index_set_barrier_tests.rs @@ -481,6 +481,60 @@ fn the_guarded_property_receiver_store_follows_one_forwarding_edge_inline() { /// re-resolves the receiver through the tracked resolver on every call, so a /// pointer store into an array whose raw-f64 bits are already clear must not /// reach it at all. +/// The scalar-aware layout note (`js_gc_note_slot_layout_aware`) returns +/// without acting when the old and new values share a pointer classification, +/// unless both are pointers and the array carries an element-shape proof. The +/// guarded fast arm now decides that inline — the exact runtime +/// `layout_pointer_bearing_bits` predicate on both values plus the +/// `GC_ARRAY_ELEMENT_SHAPE` bit of the `_reserved` word `deref.live` loaded — +/// and calls the note only from the gated `laynote` block. +#[test] +fn the_fast_arm_layout_note_is_gated_on_the_pointer_classification_and_shape_bit() { + let ir = ir(); + let live = block_body(&ir, "idxset.recv_prop.deref.live.") + .expect("guarded store emits its `deref.live` block"); + let reserved = live + .lines() + .map(str::trim) + .find(|line| line.contains("load i16")) + .and_then(|line| line.split(" = ").next()) + .expect("`deref.live` loads the live head's `_reserved` word") + .to_string(); + + let fast = block_body(&ir, "idxset.recv_prop.fast.").expect("fast block"); + assert!( + !fast.contains("js_gc_note_slot_layout_aware"), + "the fast arm must not call the layout note unconditionally:\n{fast}" + ); + assert!( + fast.contains(&format!("and i16 {reserved}, 2048")), + "the gate must test GC_ARRAY_ELEMENT_SHAPE (0x800) on the live head's `_reserved`:\n{fast}" + ); + // Exact runtime predicate, applied to both the stored and the old bits: + // tag test, payload test, bare-address range and alignment, selected. + assert!( + fast.matches("select i1").count() >= 2 + && fast.matches(", 32765").count() >= 2 + && fast.matches(", 32767").count() >= 2 + && fast.matches(", 32762").count() >= 2 + && fast.matches("icmp uge i64").count() >= 2 + && fast.matches("icmp ule i64").count() >= 2 + && fast.contains("icmp ne i1"), + "both values must be classified with the exact pointer-bearing predicate and compared:\n{fast}" + ); + let (gate, _) = branch_into_block(&ir, "idxset.recv_prop.laynote.") + .expect("the layout note sits behind a conditional branch"); + assert!( + gate.trim().starts_with("br i1"), + "gate must be a conditional branch, got `{gate}`" + ); + let note = block_body(&ir, "idxset.recv_prop.laynote.").expect("the layout note block exists"); + assert!( + note.contains("call void @js_gc_note_slot_layout_aware("), + "the note call must live inside the gated block:\n{note}" + ); +} + #[test] fn the_fast_arm_numeric_note_is_gated_on_the_raw_f64_header_bits() { let ir = ir(); diff --git a/crates/perry-codegen/src/expr/index_set_guarded.rs b/crates/perry-codegen/src/expr/index_set_guarded.rs index ee43838bf6..afb3d4e8ca 100644 --- a/crates/perry-codegen/src/expr/index_set_guarded.rs +++ b/crates/perry-codegen/src/expr/index_set_guarded.rs @@ -43,6 +43,10 @@ use anyhow::Result; use crate::nanbox::POINTER_MASK_I64; use crate::types::{I1, I16, I32, I64, I8}; +use super::write_barrier::{ + emit_jsvalue_slot_store_deferred_layout_note_on_block, emit_layout_note_slot_aware_on_block, + emit_layout_pointer_bearing_check, +}; use super::{ emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_scalar_aware_on_block, emit_write_barrier_slot_value_and_generation_tested, FnCtx, @@ -207,7 +211,7 @@ pub(super) fn emit_guarded_inbounds_array_store( // stored over a pointer is exactly the store that must clear // `GC_ARRAY_ELEMENT_SHAPE`. Class fields have no such per-slot array // invariant, which is why that half of #7511's argument does not transfer. - let (arr_handle, element_addr, value_bits) = { + let (arr_handle, element_addr, value_bits, layout_note) = { let blk = ctx.block(); // The live (possibly forwarded-once) head proved by `deref.live`, // which is this block's only predecessor. @@ -221,20 +225,69 @@ pub(super) fn emit_guarded_inbounds_array_store( // in-bounds arm: the guard proved the slot holds a valid value, so the // scalar-aware note can skip the layout hashmap on a // scalar-over-scalar store (#5094). - let value_bits = emit_jsvalue_slot_store_scalar_aware_on_block( - blk, - &element_ptr, - val_double, + if !layout_note_needed { + let value_bits = emit_jsvalue_slot_store_scalar_aware_on_block( + blk, + &element_ptr, + val_double, + &arr_handle, + idx_i32, + false, + &arr_handle, + &element_addr, + false, + ) + .unwrap_or_else(|| blk.bitcast_double_to_i64(val_double)); + (arr_handle, element_addr, value_bits, None) + } else { + // The scalar-aware note itself, opened up: the runtime + // (`layout_note_slot_aware`) returns without acting when the old + // and new values share a pointer classification — unless both are + // pointers AND the array carries an element-shape proof + // (`GC_ARRAY_ELEMENT_SHAPE` in the `_reserved` word `deref.live` + // already loaded), which the pointer-over-pointer arm maintains. A + // classification change must always reach `layout_note_slot`. + // Decide that inline with the exact runtime predicate and call the + // note only when it has work: the ECS `ents[id] = arch` store is a + // pointer over a pointer into a proof-free array on every iteration. + let (value_bits, old_bits) = emit_jsvalue_slot_store_deferred_layout_note_on_block( + blk, + &element_ptr, + val_double, + ); + let new_is_pointer = emit_layout_pointer_bearing_check(blk, &value_bits); + let old_is_pointer = emit_layout_pointer_bearing_check(blk, &old_bits); + let classification_changed = blk.icmp_ne(I1, &new_is_pointer, &old_is_pointer); + let shape_bits = blk.and(I16, &reserved, "2048"); // GC_ARRAY_ELEMENT_SHAPE + let has_element_shape = blk.icmp_ne(I16, &shape_bits, "0"); + let pointer_over_pointer_noted = blk.and(I1, &new_is_pointer, &has_element_shape); + let note_needed = blk.or(I1, &classification_changed, &pointer_over_pointer_noted); + ( + arr_handle, + element_addr, + value_bits, + Some((old_bits, note_needed)), + ) + } + }; + if let Some((old_bits, note_needed)) = layout_note { + let note_idx = ctx.new_block(&format!("{}.laynote", block_prefix)); + let note_done_idx = ctx.new_block(&format!("{}.laynote.done", block_prefix)); + let note_label = ctx.block_label(note_idx); + let note_done_label = ctx.block_label(note_done_idx); + ctx.block() + .cond_br(¬e_needed, ¬e_label, ¬e_done_label); + ctx.current_block = note_idx; + emit_layout_note_slot_aware_on_block( + ctx.block(), &arr_handle, idx_i32, - layout_note_needed, - &arr_handle, - &element_addr, - false, - ) - .unwrap_or_else(|| blk.bitcast_double_to_i64(val_double)); - (arr_handle, element_addr, value_bits) - }; + &value_bits, + &old_bits, + ); + ctx.block().br(¬e_done_label); + ctx.current_block = note_done_idx; + } if write_barrier_needed { // `arr_handle` is the live head `deref.live` just proved through its // own `obj_type == GC_TYPE_ARRAY` / `!GC_FLAG_FORWARDED` header reads, diff --git a/crates/perry-codegen/src/expr/write_barrier.rs b/crates/perry-codegen/src/expr/write_barrier.rs index 427b677de9..e3a62f6758 100644 --- a/crates/perry-codegen/src/expr/write_barrier.rs +++ b/crates/perry-codegen/src/expr/write_barrier.rs @@ -542,6 +542,37 @@ pub(crate) fn emit_jsvalue_slot_store_scalar_aware_on_block( ) } +/// The scalar-aware slot store with its layout note DEFERRED to the caller: +/// loads the slot's previous value, writes the new one through the shared +/// (audited) store, runs the string-addref demote, and returns +/// `(value_bits, old_bits)` so the caller can decide inline whether the +/// runtime note would act at all before calling it. The caller owns both the +/// note and the barrier; nothing else about the store changes. +pub(crate) fn emit_jsvalue_slot_store_deferred_layout_note_on_block( + blk: &mut LlBlock, + slot_ptr: &str, + value_double: &str, +) -> (String, String) { + let old_double = blk.load(DOUBLE, slot_ptr); + let old_bits = blk.bitcast_double_to_i64(&old_double); + let value_bits = emit_jsvalue_slot_store_on_block_inner( + blk, + slot_ptr, + value_double, + "", + "", + true, + false, + "", + "", + false, + false, + None, + ) + .unwrap_or_else(|| blk.bitcast_double_to_i64(value_double)); + (value_bits, old_bits) +} + /// #7511 — emit the `i1` predicate "these NaN-boxed bits MAY carry a heap /// pointer", as a superset of every heap address the runtime can decode. /// @@ -592,6 +623,36 @@ pub(crate) fn emit_may_carry_heap_pointer_check(blk: &mut LlBlock, value_bits: & blk.or(I1, &tagged, &is_raw_addr) } +/// The EXACT codegen mirror of `perry-runtime::gc::layout::layout_pointer_bearing_bits` +/// (not the superset above): a `POINTER_TAG` / `STRING_TAG` / `BIGINT_TAG` +/// value bears a pointer iff its 48-bit payload is non-zero; every other +/// NaN-boxed tag never does; a bare value bears one iff it lies in +/// `[0x1000, POINTER_MASK]` and is 8-byte aligned. `bits <= POINTER_MASK` +/// already implies an all-zero top word, which is the runtime's +/// `tag >= 0x7FF8…` rejection. Used where a store's GC layout note is skipped +/// only when the runtime itself would return without acting, so the answer +/// must match the runtime on every input. +pub(crate) fn emit_layout_pointer_bearing_check(blk: &mut LlBlock, value_bits: &str) -> String { + use crate::nanbox::{ + BIGINT_TAG_TOP16_I64, POINTER_MASK_I64, POINTER_TAG_TOP16_I64, STRING_TAG_TOP16_I64, + }; + let top16 = blk.lshr(I64, value_bits, "48"); + let is_pointer_tag = blk.icmp_eq(I64, &top16, POINTER_TAG_TOP16_I64); + let is_string_tag = blk.icmp_eq(I64, &top16, STRING_TAG_TOP16_I64); + let is_bigint_tag = blk.icmp_eq(I64, &top16, BIGINT_TAG_TOP16_I64); + let tagged = blk.or(I1, &is_pointer_tag, &is_string_tag); + let tagged = blk.or(I1, &tagged, &is_bigint_tag); + let payload = blk.and(I64, value_bits, POINTER_MASK_I64); + let payload_nonzero = blk.icmp_ne(I64, &payload, "0"); + let above_floor = blk.icmp_uge(I64, value_bits, "4096"); + let within_mask = blk.icmp_ule(I64, value_bits, POINTER_MASK_I64); + let low_bits = blk.and(I64, value_bits, "7"); + let aligned = blk.icmp_eq(I64, &low_bits, "0"); + let bare = blk.and(I1, &above_floor, &within_mask); + let bare = blk.and(I1, &bare, &aligned); + blk.select(I1, &tagged, I1, &payload_nonzero, &bare) +} + /// #7511 — a class-field JSValue slot store whose three GC-bookkeeping calls /// are placed behind ONE inline, live test of the stored value. /// From 8ef56aafa55b22bb1fa760f33499b9e92209eee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 21:27:38 +0200 Subject: [PATCH 5/8] changelog: fragment for #8890 Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ --- .../8890-claimed-receiver-brand-gated-layout-note.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog.d/8890-claimed-receiver-brand-gated-layout-note.md diff --git a/changelog.d/8890-claimed-receiver-brand-gated-layout-note.md b/changelog.d/8890-claimed-receiver-brand-gated-layout-note.md new file mode 100644 index 0000000000..bdb2df7aa2 --- /dev/null +++ b/changelog.d/8890-claimed-receiver-brand-gated-layout-note.md @@ -0,0 +1,11 @@ +Array performance: an erased Array declaration admits object-backed Array +subclasses and typed arrays, so a canonical integer key on such a receiver now +brands the receiver once and takes the receiver-unknown numeric read tiers +(inline typed-array read, dense-subclass shape cache, complete dispatcher) +instead of the plain-array tier's out-of-line feedback fallback; and the +guarded in-bounds element store decides inline — with the exact +pointer-bearing classification of the old and new values plus the array's +element-shape bit — whether the GC layout note has any work before calling it. +wolf-ecs (noctjs/ecs-benchmark) on the Mac mini reference box: add/remove +-3.2% then -2.3%, entity-cycle -3.7% then -2.0%, each 11/11 paired wins, +semantics probes byte-identical to Node. From 72951364ec36c9d7cffc98351b5a748799ef1573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 21:31:03 +0200 Subject: [PATCH 6/8] runtime(array): drop the prototype-index note helpers duplicated by the #8885/#8876 composition main's 77b994f6b moved note_object_prototype_index_write, note_array_proto_iterator_write and array_proto_iterator_modified into indexing_support.rs (glob-imported) but left the originals in indexing.rs, which -D warnings rejects as dead code plus unused AtomicBool/AtomicU8 imports. The support copies are the live ones; remove the duplicates. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ --- crates/perry-runtime/src/array/indexing.rs | 35 +--------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index f45a446a33..9df4bef496 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -2,7 +2,7 @@ use super::indexing_support::*; use super::*; use std::ptr; -use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; +use std::sync::atomic::Ordering; #[path = "indexing_keyed.rs"] mod keyed; @@ -74,43 +74,10 @@ pub(crate) fn test_strict_dense_pointer_overwrite_hits() -> u64 { STRICT_DENSE_POINTER_OVERWRITE_HITS.with(std::cell::Cell::get) } -/// Record (if `obj` is the canonical `Object.prototype`) that it now carries -/// an indexed property. Called from the object index-write / numeric -/// defineProperty paths; cheap (relaxed loads + compare). -#[inline] -pub(crate) fn note_object_prototype_index_write(obj: usize) { - if !OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed) && obj != 0 && obj == object_prototype_addr() - { - OBJECT_PROTO_HAS_INDEX.store(true, Ordering::Relaxed); - invalidate_array_index_fast_path(); - } -} - pub(crate) fn object_prototype_has_index_flag() -> bool { OBJECT_PROTO_HAS_INDEX.load(Ordering::Relaxed) } -/// Record (if `obj` is `Array.prototype` and `sym_key` is the well-known -/// `Symbol.iterator`) that the array iteration protocol has been tampered -/// with. Called from the symbol-property set/delete paths. -pub(crate) fn note_array_proto_iterator_write(obj: usize, sym_key: usize) { - if ARRAY_PROTO_ITERATOR_MODIFIED.load(Ordering::Relaxed) || obj == 0 || sym_key == 0 { - return; - } - if obj == array_prototype_addr() - && sym_key == crate::symbol::well_known_symbol("iterator") as usize - { - ARRAY_PROTO_ITERATOR_MODIFIED.store(true, Ordering::Relaxed); - // Publish to generated code. Release so a loop that observes the `1` - // also observes the prototype write that preceded it. - PERRY_ARRAY_PROTO_ITERATOR_PATCHED.store(1, Ordering::Release); - } -} - -pub(crate) fn array_proto_iterator_modified() -> bool { - ARRAY_PROTO_ITERATOR_MODIFIED.load(Ordering::Relaxed) -} - /// Record (if `arr` is `Array.prototype`) that the prototype now carries an /// indexed property, so subsequent out-of-bounds reads consult it. Called from /// the array element-write paths; cheap (two relaxed atomic loads + compare). From e2160040f48f966173db3b46fc6c4d059e76aa19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 21:36:25 +0200 Subject: [PATCH 7/8] runtime(array): restore #8885's strict number-store lane beside the dense-index lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #8885/#8876 composition on main kept only try_strict_dense_index_set in js_array_set_f64_extend_strict, leaving #8885's try_strict_dense_number_store reachable from its unit tests alone (a -D warnings dead-code error). Wire both exact lanes — the plain-number lane first, then the dense-index lane — and drop the throw helpers indexing_support.rs already owns. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ --- crates/perry-runtime/src/array/indexing.rs | 44 +++++----------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 9df4bef496..c7bd2999e3 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -23,45 +23,11 @@ const MAX_DENSE_ARRAY_GROW_LENGTH: u32 = 1_000_000; /// benchmark for 6 hours (Regression Check, v0.5.1129–v0.5.1150). const DENSE_ARRAY_GAP_LIMIT: u32 = 1024; -/// A strict-mode element write (`arr[i] = v`) to a **frozen** array's existing -/// index is `[[Set]]` on a non-writable data property with `Throw = true` -/// (ECMA-262 §10.4.2.4 → OrdinarySetWithOwnDescriptor step 2.b.i), so it must -/// throw a **TypeError** rather than silently no-op. Perry compiles everything -/// strict, so the codegen `arr[i] = v` fast paths — which call these -/// `js_array_set_f64*` helpers directly — carry the strict-`Set` contract. -/// Matches V8's message. (test262 built-ins/Array element-write-on-frozen.) -#[cold] -fn throw_frozen_array_index_write(index: u32) -> ! { - crate::collection_iter::throw_type_error(&format!( - "Cannot assign to read only property '{index}' of object '[object Array]'" - )); -} - -/// A strict-mode write that would *add* a new index to a non-extensible -/// (frozen / sealed / preventExtensions'd) array — `arr[i] = v` with -/// `i >= length` — is `CreateDataProperty` on a non-extensible object with -/// `Throw = true`, so it must throw a **TypeError**. Matches V8's message. -#[cold] -fn throw_array_not_extensible_add(index: u32) -> ! { - crate::collection_iter::throw_type_error(&format!( - "Cannot add property {index}, object is not extensible" - )); -} - #[inline] pub(crate) fn invalidate_array_index_fast_path() { PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.store(1, Ordering::Relaxed); } -/// Test-only companion to -/// `prototype_chain::test_swap_array_static_proto_recorded`: swap the summary -/// byte generated code reads, returning the previous value. Only for a test -/// that knowingly set it and is putting the process back as it found it. -#[cfg(test)] -pub(crate) fn test_swap_array_index_fast_path_invalidated(value: u8) -> u8 { - PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED.swap(value, Ordering::Relaxed) -} - #[cfg(test)] thread_local! { static STRICT_DENSE_POINTER_OVERWRITE_HITS: std::cell::Cell = const { @@ -1337,6 +1303,16 @@ pub extern "C" fn js_array_set_f64_extend_strict( index: u32, value: f64, ) -> *mut ArrayHeader { + // Two exact fast lanes, each storing only what the general path below + // would store and declining every shape it cannot prove. The plain-number + // lane (#8885) resolves the head itself, so a hit returns that head; the + // dense-index lane (#8876) covers the remaining in-range existing-slot + // stores. The #8885/#8876 composition on `main` had kept only the second, + // leaving the first unreachable outside its unit tests. + // SAFETY: the lane validates the receiver before every dereference. + if let Some(resolved) = unsafe { try_strict_dense_number_store(arr, index, value) } { + return resolved; + } if let Some(resolved) = try_strict_dense_index_set(arr, index, value) { return resolved; } From ac4712d4078222bd11b8a4ecd2e32f000c7e72fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 22:22:30 +0200 Subject: [PATCH 8/8] chore: split diagnostics.rs test modules for the 2000-line gate (#8889) --- .../src/node_submodules/diagnostics.rs | 128 +---------------- .../src/node_submodules/diagnostics_tests.rs | 136 ++++++++++++++++++ 2 files changed, 138 insertions(+), 126 deletions(-) create mode 100644 crates/perry-runtime/src/node_submodules/diagnostics_tests.rs diff --git a/crates/perry-runtime/src/node_submodules/diagnostics.rs b/crates/perry-runtime/src/node_submodules/diagnostics.rs index b58cbcd4ca..080123eec3 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics.rs @@ -1961,129 +1961,5 @@ pub(crate) fn ensure_diag_noop_closure() -> *mut ClosureHeader { } #[cfg(test)] -mod tests { - use super::*; - - fn inactive_state() -> DiagChannelState { - DiagChannelState { - name: 0.0, - obj: std::ptr::null_mut(), - subscribers: Vec::new(), - stores: Vec::new(), - } - } - - // #1309: crossing the soft cap evicts a batch of the oldest inactive - // channels so the live-channel map stays bounded. - #[test] - fn diag_channels_capped_by_evicting_inactive() { - DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); - DIAG_CHANNEL_BY_KEY.with(|m| m.borrow_mut().clear()); - for _ in 0..DIAG_CHANNEL_SOFT_CAP + 100 { - let id = next_diag_id(); - DIAG_CHANNELS.with(|m| { - m.borrow_mut().insert(id, inactive_state()); - }); - } - evict_inactive_diag_channels_if_needed(); - let len = DIAG_CHANNELS.with(|m| m.borrow().len()); - assert!(len <= DIAG_CHANNEL_SOFT_CAP, "expected <= cap, got {len}"); - assert!( - len >= DIAG_CHANNEL_SOFT_CAP - DIAG_CHANNEL_EVICT_BATCH, - "should evict at most one batch, got {len}" - ); - DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); - } - - // #1309: a subscribed (active) channel is never evicted, even when the - // map is over the cap. - #[test] - fn active_diag_channel_survives_eviction() { - DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); - DIAG_CHANNEL_BY_KEY.with(|m| m.borrow_mut().clear()); - let active_id = next_diag_id(); - DIAG_CHANNELS.with(|m| { - let mut s = inactive_state(); - s.subscribers.push(1.0); - m.borrow_mut().insert(active_id, s); - }); - for _ in 0..DIAG_CHANNEL_SOFT_CAP + 100 { - let id = next_diag_id(); - DIAG_CHANNELS.with(|m| { - m.borrow_mut().insert(id, inactive_state()); - }); - } - evict_inactive_diag_channels_if_needed(); - assert!( - DIAG_CHANNELS.with(|m| m.borrow().contains_key(&active_id)), - "subscribed channel must not be evicted" - ); - DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); - } -} - -#[cfg(test)] -mod error_prop_order_tests { - use super::*; - - /// Own string keys enumerate in INSERTION order, not hash or alphabetical - /// order. This is observable through `Object.keys`, `for…in`, `{...err}` - /// and `JSON.stringify`, so a caught fs error must serialize as node's - /// `{"errno":…,"code":…,"syscall":…,"path":…}`. - /// - /// The store was a `HashMap` with an alphabetical `sort_by` bolted on for - /// determinism, which is stable but wrong: it emitted `code` before - /// `errno`. Reverting to any unordered container fails this test. - #[test] - fn user_props_enumerate_in_insertion_order() { - let err = 0x4000_1000usize; - for k in ["errno", "code", "syscall", "path"] { - set_error_user_prop(err, k, 1.0); - } - let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); - assert_eq!( - keys, - vec![ - "errno".to_string(), - "code".to_string(), - "syscall".to_string(), - "path".to_string() - ], - "fs error fields must enumerate in node's insertion order, not sorted" - ); - } - - /// Reassigning an existing key keeps its ORIGINAL position — in node, - /// `o.a=1; o.b=2; o.a=3` still enumerates `a,b`. An implementation that - /// removed-then-appended would report `b,a`. - #[test] - fn reassignment_keeps_original_position() { - let err = 0x4000_2000usize; - set_error_user_prop(err, "a", 1.0); - set_error_user_prop(err, "b", 2.0); - set_error_user_prop(err, "a", 3.0); - let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); - assert_eq!(keys, vec!["a".to_string(), "b".to_string()]); - assert_eq!( - error_user_prop(err, "a"), - Some(3.0), - "reassignment must still update the value" - ); - } - - /// Removing a key must not disturb the order of the survivors. - #[test] - fn removal_preserves_order_of_the_rest() { - let err = 0x4000_3000usize; - for k in ["one", "two", "three"] { - set_error_user_prop(err, k, 0.0); - } - assert!(remove_error_user_prop(err, "two")); - let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); - assert_eq!(keys, vec!["one".to_string(), "three".to_string()]); - assert!( - !remove_error_user_prop(err, "two"), - "second remove is a no-op" - ); - } -} +#[path = "diagnostics_tests.rs"] +mod diagnostics_tests; diff --git a/crates/perry-runtime/src/node_submodules/diagnostics_tests.rs b/crates/perry-runtime/src/node_submodules/diagnostics_tests.rs new file mode 100644 index 0000000000..098bd44bfc --- /dev/null +++ b/crates/perry-runtime/src/node_submodules/diagnostics_tests.rs @@ -0,0 +1,136 @@ +//! Unit tests for the node:diagnostics_channel submodule. +//! +//! Split out of `diagnostics.rs` to keep it under the 2,000-line file gate. + +// Bring `diagnostics`'s items into scope so the nested `mod tests` blocks +// below resolve `use super::*` to them. +#[allow(unused_imports)] +use super::*; + +#[cfg(test)] +mod tests { + use super::*; + + fn inactive_state() -> DiagChannelState { + DiagChannelState { + name: 0.0, + obj: std::ptr::null_mut(), + subscribers: Vec::new(), + stores: Vec::new(), + } + } + + // #1309: crossing the soft cap evicts a batch of the oldest inactive + // channels so the live-channel map stays bounded. + #[test] + fn diag_channels_capped_by_evicting_inactive() { + DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); + DIAG_CHANNEL_BY_KEY.with(|m| m.borrow_mut().clear()); + for _ in 0..DIAG_CHANNEL_SOFT_CAP + 100 { + let id = next_diag_id(); + DIAG_CHANNELS.with(|m| { + m.borrow_mut().insert(id, inactive_state()); + }); + } + evict_inactive_diag_channels_if_needed(); + let len = DIAG_CHANNELS.with(|m| m.borrow().len()); + assert!(len <= DIAG_CHANNEL_SOFT_CAP, "expected <= cap, got {len}"); + assert!( + len >= DIAG_CHANNEL_SOFT_CAP - DIAG_CHANNEL_EVICT_BATCH, + "should evict at most one batch, got {len}" + ); + DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); + } + + // #1309: a subscribed (active) channel is never evicted, even when the + // map is over the cap. + #[test] + fn active_diag_channel_survives_eviction() { + DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); + DIAG_CHANNEL_BY_KEY.with(|m| m.borrow_mut().clear()); + let active_id = next_diag_id(); + DIAG_CHANNELS.with(|m| { + let mut s = inactive_state(); + s.subscribers.push(1.0); + m.borrow_mut().insert(active_id, s); + }); + for _ in 0..DIAG_CHANNEL_SOFT_CAP + 100 { + let id = next_diag_id(); + DIAG_CHANNELS.with(|m| { + m.borrow_mut().insert(id, inactive_state()); + }); + } + evict_inactive_diag_channels_if_needed(); + assert!( + DIAG_CHANNELS.with(|m| m.borrow().contains_key(&active_id)), + "subscribed channel must not be evicted" + ); + DIAG_CHANNELS.with(|m| m.borrow_mut().clear()); + } +} + +#[cfg(test)] +mod error_prop_order_tests { + use super::*; + + /// Own string keys enumerate in INSERTION order, not hash or alphabetical + /// order. This is observable through `Object.keys`, `for…in`, `{...err}` + /// and `JSON.stringify`, so a caught fs error must serialize as node's + /// `{"errno":…,"code":…,"syscall":…,"path":…}`. + /// + /// The store was a `HashMap` with an alphabetical `sort_by` bolted on for + /// determinism, which is stable but wrong: it emitted `code` before + /// `errno`. Reverting to any unordered container fails this test. + #[test] + fn user_props_enumerate_in_insertion_order() { + let err = 0x4000_1000usize; + for k in ["errno", "code", "syscall", "path"] { + set_error_user_prop(err, k, 1.0); + } + let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); + assert_eq!( + keys, + vec![ + "errno".to_string(), + "code".to_string(), + "syscall".to_string(), + "path".to_string() + ], + "fs error fields must enumerate in node's insertion order, not sorted" + ); + } + + /// Reassigning an existing key keeps its ORIGINAL position — in node, + /// `o.a=1; o.b=2; o.a=3` still enumerates `a,b`. An implementation that + /// removed-then-appended would report `b,a`. + #[test] + fn reassignment_keeps_original_position() { + let err = 0x4000_2000usize; + set_error_user_prop(err, "a", 1.0); + set_error_user_prop(err, "b", 2.0); + set_error_user_prop(err, "a", 3.0); + let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); + assert_eq!(keys, vec!["a".to_string(), "b".to_string()]); + assert_eq!( + error_user_prop(err, "a"), + Some(3.0), + "reassignment must still update the value" + ); + } + + /// Removing a key must not disturb the order of the survivors. + #[test] + fn removal_preserves_order_of_the_rest() { + let err = 0x4000_3000usize; + for k in ["one", "two", "three"] { + set_error_user_prop(err, k, 0.0); + } + assert!(remove_error_user_prop(err, "two")); + let keys: Vec = error_user_props(err).into_iter().map(|(k, _)| k).collect(); + assert_eq!(keys, vec!["one".to_string(), "three".to_string()]); + assert!( + !remove_error_user_prop(err, "two"), + "second remove is a no-op" + ); + } +}