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. 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. 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, 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. /// diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index f45a446a33..c7bd2999e3 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; @@ -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 { @@ -74,43 +40,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). @@ -1370,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; } 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..080123eec3 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 } @@ -1938,63 +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()); - } -} +#[path = "diagnostics_tests.rs"] +mod diagnostics_tests; 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/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" + ); + } +} diff --git a/crates/perry-runtime/src/object/alloc.rs b/crates/perry-runtime/src/object/alloc.rs index e16b01a218..e92664b44d 100644 --- a/crates/perry-runtime/src/object/alloc.rs +++ b/crates/perry-runtime/src/object/alloc.rs @@ -1552,6 +1552,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