diff --git a/changelog.d/8890-header-unification.md b/changelog.d/8890-header-unification.md new file mode 100644 index 0000000000..47eb5c0413 --- /dev/null +++ b/changelog.d/8890-header-unification.md @@ -0,0 +1,47 @@ +Gave every exotic cell type a metadata edge, and moved `Error`'s own properties +onto it — deleting a side table and all four of its GC hooks. + +Cell types declare their fields independently; there is no shared header prefix. +So *"does this cell own an `ObjectMeta`?"* had no single answer, and only an +`ObjectHeader` could be asked. That is why per-object state for the exotic types +accumulated in tables keyed by the owner's **address** — there was nowhere on +the cell to put it. Errors alone carried seven such tables plus four GC hooks. + +Every exotic cell now has a `meta` edge, reachable through one accessor +(`cell_meta_slot`): Object, Error, Map, Set, RegExp, Promise and Date. It +answers `None` for anything unmapped, so callers degrade to their existing +storage rather than mis-reading another layout as a pointer. + +Each edge is **traced**, not merely rewritten. Where a type's rewrite arm is +also its mark path the slot goes there; RegExp delegates to the layout visitor, +so its edge goes in `gc_child_slots` instead. #6812 is exactly the bug of +choosing wrong — an edge visited only on the rewrite path is invisible to +marking, and the record is swept out from under a live owner. + +`Date` needed more than a field: it was `pointer_free` with a `Leaf` (no-op) +descriptor, holding one raw `f64`. A cell with a pointer must be scanned, so it +moved to a new `MetaOnly` descriptor with `pointer_free = false`. +`validate_gc_type_info` caught the flag when an edit missed it. + +The arena reuses free-list memory **without zeroing**, so an uninitialised meta +edge would be a garbage pointer the collector follows. Every allocation path +initialises it explicitly; `Promise` routes through `Promise::new`, so the +constructor covers its several sites. + +`ObjectMeta` gains `expando`, a named-property bag for cells with no inline slot +layout. It is appended last because the struct's offsets are a contract with +codegen (`offset_of!` asserts at 32/48/56 — inserting mid-struct failed them). +`ERROR_USER_PROPS` is deleted along with all four of its GC hooks: +rekey-on-evacuation, finalize, dead-sweep and the root scanner. Error properties +are now an ordinary traced child edge that moves with its owner, dies with its +owner, and cannot be inherited by a later tenant of a recycled address. + +The tracing tests assert slot **enumeration** directly rather than survival +across a collection. A survival test is vacuous here: arena block reset is +all-or-nothing, so `gc::trace` force-marks every object in a block that still +holds one reachable object (#7975), which keeps an untraced record alive anyway. +Verified by sabotage — deleting the visit line left the survival version passing +and fails the enumeration version. + +No user-visible change on its own. This is the gate that lets the shape and +descriptor payloads move off address-keyed tables. diff --git a/crates/perry-runtime/src/date.rs b/crates/perry-runtime/src/date.rs index 01933c6bef..07cc28322f 100644 --- a/crates/perry-runtime/src/date.rs +++ b/crates/perry-runtime/src/date.rs @@ -25,6 +25,12 @@ const NANBOX_PTR_MASK: u64 = 0x0000_FFFF_FFFF_FFFF; #[repr(C)] pub struct DateCell { pub ts: f64, + /// #6759 phase 1 (header unification): per-object metadata record, or null. + /// + /// Adding this made `DateCell` non-pointer-free, so its GC type entry moved + /// from `GcRewriteDescriptorKind::Leaf` (a no-op arm) to `MetaOnly` and its + /// `pointer_free` flag to `false` — a cell with a pointer must be scanned. + pub meta: *mut crate::object::ObjectMeta, } /// Allocate a fresh Date cell holding `ts` and return it as a NaN-boxed @@ -41,6 +47,11 @@ pub fn alloc_date_cell(ts: f64) -> f64 { crate::gc::GC_TYPE_DATE_CELL, ) as *mut DateCell; (*ptr).ts = ts; + // MUST be explicit: the arena reuses free-list memory without zeroing, + // and since #6759 phase 1 this cell is no longer pointer-free — the + // collector now scans this slot, so leftover bytes would be followed + // as a pointer. + (*ptr).meta = std::ptr::null_mut(); // A previous (collected) Date at this address may have left expando // properties in the side table; a fresh Date must start clean. crate::object::exotic_expando::expando_clear_on_alloc(ptr as usize); diff --git a/crates/perry-runtime/src/error.rs b/crates/perry-runtime/src/error.rs index 400571ce95..0a30476216 100644 --- a/crates/perry-runtime/src/error.rs +++ b/crates/perry-runtime/src/error.rs @@ -98,6 +98,28 @@ pub struct ErrorHeader { pub cause: f64, /// Errors array for AggregateError (raw ArrayHeader pointer or null) pub errors: *mut crate::array::ArrayHeader, + /// Per-object metadata record, or null. The same `ObjectMeta` cell an + /// `ObjectHeader` hangs off its own `meta` field. + /// + /// #6759 phase 1 (header unification). An `ErrorHeader` is not an + /// `ObjectHeader`, so before this field the only place to put anything + /// per-error was a side table keyed by the error's ADDRESS — and errors + /// accumulated seven of them, each needing its own GC rekey-on-evacuation, + /// finalize, dead-sweep and root-scanner hook. Giving the cell a metadata + /// edge is what lets those payloads move onto the object itself. + /// + /// Appended LAST on purpose: every preceding field keeps its offset, so + /// codegen and the `errors`-at-+48 assumption in this file's tests are + /// undisturbed. + /// + /// Traced and rewritten by the `GcRewriteDescriptorKind::Error` arm in + /// `gc/layout_slot_visit.rs`. That arm is reached by + /// `trace_heap_rewrite_slots`, so it is the MARK path as well as the + /// evacuation-rewrite path — unlike `GcLayoutSlotKind::ObjectFields`, + /// which delegates to the layout visitor and needed its meta edge added + /// there separately (#6812: a meta edge enumerated only on the rewrite + /// path is invisible to marking). + pub meta: *mut crate::object::ObjectMeta, } thread_local! { @@ -247,6 +269,9 @@ unsafe fn alloc_error( (*ptr).stack = stack_handle.get_raw_const_ptr::() as *mut StringHeader; (*ptr).cause = f64::from_bits(TAG_UNDEFINED); (*ptr).errors = std::ptr::null_mut(); + // No metadata record until something needs one; the GC treats a null meta + // edge as absent. + (*ptr).meta = std::ptr::null_mut(); ptr } @@ -1930,3 +1955,43 @@ mod tostring_tests { ); } } + +#[cfg(test)] +mod header_unification_tests { + use super::*; + + /// #6759 phase 1: an `ErrorHeader` owns a metadata edge, and it is + /// reachable through the SAME accessor an `ObjectHeader` is. + /// + /// This is the gate the rest of the migration stands on: while "does this + /// cell own an ObjectMeta?" had no uniform answer, per-error state had + /// nowhere to live but a side table keyed by the error's address. + #[test] + fn error_cell_exposes_a_meta_edge_like_an_object() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let msg = crate::string::js_string_from_bytes(b"boom".as_ptr(), 4); + let err = js_error_new_with_message(msg); + assert!( + (*err).meta.is_null(), + "a fresh error must start with no metadata record" + ); + assert!( + crate::object::cell_has_meta_edge(err as usize), + "an error cell must be reachable through the uniform meta accessor" + ); + let obj = crate::object::js_object_alloc(0, 0); + assert!( + crate::object::cell_has_meta_edge(obj as usize), + "an object cell must answer the same accessor" + ); + // A cell type that has NOT been unified yet must answer `None` + // rather than mis-reading its own layout as a meta pointer. + let arr = crate::array::js_array_alloc(0); + assert!( + !crate::object::cell_has_meta_edge(arr as usize), + "a cell without a meta edge must report absence, not garbage" + ); + } + } +} diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 28854a86f0..fe69b71707 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -1731,6 +1731,10 @@ pub(super) unsafe fn gc_child_slots(header: *mut GcHeader) -> HeapChildSlotItera Some(last_index_slot), HeapSlotRange::new(pattern_slot, slot_count), ) + // #6759 phase 1: the metadata edge. RegExp reaches marking through + // THIS iterator (its rewrite arm delegates here), so the edge has + // to be enumerated at this point, not in the rewrite match. + .with_meta_slot(crate::object::cell_meta_slot(user_ptr as usize).map(|s| s as *mut u64)) } GcLayoutSlotKind::ObjectMeta => { // Prototype and the private-evaluation brand are explicit prefix diff --git a/crates/perry-runtime/src/gc/layout_slot_visit.rs b/crates/perry-runtime/src/gc/layout_slot_visit.rs index e0ee81d4ff..c2a7f9c05a 100644 --- a/crates/perry-runtime/src/gc/layout_slot_visit.rs +++ b/crates/perry-runtime/src/gc/layout_slot_visit.rs @@ -193,6 +193,8 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( &mut (*promise).on_rejected as *mut _ as *mut u64, )); visit(fixed_slot(&mut (*promise).next as *mut _ as *mut u64)); + // #6759 phase 1: the metadata edge (MARK path as well as rewrite). + visit(fixed_slot(&mut (*promise).meta as *mut _ as *mut u64)); } GcRewriteDescriptorKind::Error => { let error = user_ptr as *mut crate::error::ErrorHeader; @@ -201,6 +203,11 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( visit(fixed_slot(&mut (*error).stack as *mut _ as *mut u64)); visit(fixed_slot(&mut (*error).cause as *mut f64 as *mut u64)); visit(fixed_slot(&mut (*error).errors as *mut _ as *mut u64)); + // #6759 phase 1: the metadata edge. This arm is reached by + // `trace_heap_rewrite_slots`, so visiting the slot here both MARKS + // the meta record (keeping it, and anything reachable only through + // it, alive) and rewrites the edge when evacuation moves it. + visit(fixed_slot(&mut (*error).meta as *mut _ as *mut u64)); } GcRewriteDescriptorKind::Map => { let map = user_ptr as *mut crate::map::MapHeader; @@ -241,6 +248,11 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( range: HeapSlotRange::new((*map).entries as *mut u64, size as usize * 2), layout_kind: None, }); + // #6759 phase 1: the metadata edge. This arm is the MARK path as + // well as the rewrite path (`trace_heap_rewrite_slots` drives it), + // so visiting here keeps the record — and anything reachable only + // through it — alive. + visit(fixed_slot(&mut (*map).meta as *mut _ as *mut u64)); } GcRewriteDescriptorKind::Set => { let set = user_ptr as *mut crate::set::SetHeader; @@ -250,6 +262,8 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( layout_kind: None, }); } + // #6759 phase 1: the metadata edge (MARK path as well as rewrite). + visit(fixed_slot(&mut (*set).meta as *mut _ as *mut u64)); } GcRewriteDescriptorKind::LazyArray => { let lazy = user_ptr as *mut crate::json_tape::LazyArrayHeader; @@ -307,12 +321,25 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( // #6812: the object-owned overflow buffer is a raw-pointer child // edge (0 = none), traced and rewritten exactly like `prototype`. visit(fixed_slot(&mut (*meta).spill as *mut u64)); + // #6759 phase 1: the named-property bag for a cell with no inline + // slot layout (an Error, say). Reachable ONLY through this record, + // so an unvisited edge here collects a live object's own + // properties — the same shape as the spill hazard above (#6812). + visit(fixed_slot(&mut (*meta).expando as *mut u64)); // A fresh class object stored as an instance's private evaluation // brand is a NaN-boxed child edge and moves with the meta record. visit(fixed_slot( &mut (*meta).private_evaluation_brand as *mut u64, )); } + GcRewriteDescriptorKind::MetaOnly => { + // #6759 phase 1: the cell's only traced edge is its metadata + // record. Reached by `trace_heap_rewrite_slots`, so this is the + // MARK path as well as the rewrite path. + if let Some(slot) = crate::object::cell_meta_slot(user_ptr as usize) { + visit(fixed_slot(slot as *mut u64)); + } + } GcRewriteDescriptorKind::Leaf => {} } } diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index b862410264..a80653dd30 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -875,10 +875,6 @@ pub fn gc_init() { reg_scanner!(crate::object::shapes::scan_shape_table_rekey_mut); reg_scanner!(crate::proxy::scan_proxy_roots_mut); // Object/string-valued `err. = v` user props live as raw bits in - // ERROR_USER_PROPS — invisible to GC without this scanner (collectable - // while reachable; stale addresses after a move). The address KEYS are - // maintained by the ErrorSideTables move/finalize hooks. - reg_scanner!(crate::node_submodules::diagnostics_gc::scan_error_user_props_roots_mut,); reg_scanner!(exception_mutable_root_scanner); reg_scanner!(async_context_mutable_root_scanner); reg_scanner!(async_hooks_mutable_root_scanner); diff --git a/crates/perry-runtime/src/gc/tests/alloc.rs b/crates/perry-runtime/src/gc/tests/alloc.rs index c0655640e7..649390fd56 100644 --- a/crates/perry-runtime/src/gc/tests/alloc.rs +++ b/crates/perry-runtime/src/gc/tests/alloc.rs @@ -647,6 +647,7 @@ fn alloc_malloc_kind_test_object(obj_type: u8) -> *mut u8 { next: std::ptr::null_mut(), async_id: 0, trigger_async_id: 0, + meta: std::ptr::null_mut(), }, ); } @@ -670,6 +671,7 @@ fn alloc_malloc_kind_test_object(obj_type: u8) -> *mut u8 { stack: std::ptr::null_mut(), cause: 0.0, errors: std::ptr::null_mut(), + meta: std::ptr::null_mut(), }, ); } diff --git a/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs b/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs index 0d658e488c..acaf11a5fa 100644 --- a/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs +++ b/crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs @@ -1693,96 +1693,122 @@ fn test_live_symbol_accessor_owner_survives_full_gc() { js_shadow_slot_set(0, 0); } -// --- per-object layout tables (LAYOUT_SLOT_MASKS + TYPED_LAYOUTS) ----------- -// -// The inline allocator's forget probe is gated on -// `PERRY_YOUNG_LAYOUT_RECORDS`: the count of records keyed by an address the -// nursery could hand out again. These pin the two halves of that contract — -// a dead nursery owner's record is pruned by the copied-minor pass (so it can -// never be inherited), and the count returns to zero once no nursery-keyed -// record remains, while a live old-page record keeps the armed flag without -// re-opening the probe. - -fn young_layout_records() -> u32 { - crate::gc::layout_tables::test_young_layout_records() -} +/// #6759 phase 1: an `ErrorHeader`'s metadata edge must be ENUMERATED by the +/// slot visitor that drives tracing. +/// +/// #6812 records the failure mode: a meta edge visited only on the rewrite path +/// is invisible to MARKING, so the record — and anything reachable only through +/// it — is swept while the owner still points at it. Errors are visited by the +/// `GcRewriteDescriptorKind::Error` arm, and `trace_heap_rewrite_slots` drives +/// exactly that arm, so listing the slot there is what makes the edge both +/// marked and rewritten. +/// +/// This asserts enumeration DIRECTLY rather than by observing survival across a +/// collection. A survival test is vacuous here: arena block reset is +/// all-or-nothing, so `gc::trace` force-marks every object in a block that +/// still holds one reachable object (#7975), which keeps an untraced record +/// alive anyway — verified by sabotage, where deleting the `visit(...)` line +/// left a survival-based test still passing. Deleting it fails THIS test. +#[test] +fn error_meta_edge_is_enumerated_by_the_trace_visitor() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let msg = crate::string::js_string_from_bytes(b"traced".as_ptr(), 6); + let err = crate::error::js_error_new_with_message(msg); + let scope = crate::gc::RuntimeHandleScope::new(); + let err_h = scope.root_raw_mut_ptr(err); -unsafe fn install_typed_record(addr: usize) { - let pointer_mask = [0b10u64]; - crate::gc::js_gc_init_typed_shape_layout( - addr as u64, - 2, - std::ptr::null(), - 0, - pointer_mask.as_ptr(), - pointer_mask.len() as u32, - ); + crate::object::object_meta_ensure_for_cell(err as usize) + .expect("an error cell must be able to materialise a meta record"); + + let err = err_h.get_raw_mut_ptr::(); + let meta_slot = &mut (*err).meta as *mut _ as *mut u64; + let header = (err as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; + + let mut visited: Vec<*mut u64> = Vec::new(); + crate::gc::layout_slot_visit::visit_gc_rewrite_slot_descriptors(header, |descriptor| { + descriptor.visit_slots(&mut |slot| visited.push(slot.slot)); + }); + + assert!( + visited.contains(&meta_slot), + "the error's `meta` slot must be enumerated by the trace visitor; \ + an edge that is not enumerated is not marked, and the metadata \ + record is swept out from under a live error" + ); + // The pre-existing edges must still be enumerated — appending a field + // must not displace any of them. + assert!( + visited.contains(&(&mut (*err).message as *mut _ as *mut u64)), + "the `message` edge must still be enumerated" + ); + } } +/// #6759 phase 1: `ObjectMeta.expando` — the named-property bag for cells with +/// no inline slot layout — must be ENUMERATED by the trace visitor. +/// +/// It is reachable only through the metadata record, so an unvisited edge here +/// collects a live owner's own properties. Same hazard the `spill` edge +/// documents (#6812). +/// +/// `ObjectMeta`'s rewrite arm is its trace path — `trace_heap_rewrite_slots` +/// drives `visit_gc_rewrite_slot_descriptors`, and the `ObjectMeta` case lists +/// slots explicitly rather than delegating to the layout visitor — so listing +/// the slot there is what makes it marked as well as rewritten. #[test] -fn test_dead_nursery_owner_layout_record_pruned_on_copied_minor_and_young_count_drops() { - let _guard = CopyingNurseryTestGuard::new(1); - let before = young_layout_records(); - let (obj, _) = unsafe { alloc_nursery_test_object(2) }; - let addr = obj as usize; - unsafe { install_typed_record(addr) }; - assert!( - crate::gc::layout_tables::test_per_object_layout_present(addr), - "premise: the nursery owner carries a typed record" - ); - assert!( - young_layout_records() > before, - "a fresh nursery-keyed record must count as young until a collection proves otherwise" - ); - js_shadow_slot_set(0, 0); - - let _ = gc_collect_minor(); +fn object_meta_expando_edge_is_enumerated_by_the_trace_visitor() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = crate::object::js_object_alloc(0, 0); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_raw_mut_ptr(obj); + let meta = crate::object::object_meta_ensure_for_cell(obj as usize) + .expect("an object cell must materialise a meta record"); + + let meta_header = + (meta as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; + let expando_slot = &mut (*meta).expando as *mut u64; + let spill_slot = &mut (*meta).spill as *mut u64; + + let mut visited: Vec<*mut u64> = Vec::new(); + crate::gc::layout_slot_visit::visit_gc_rewrite_slot_descriptors( + meta_header, + |descriptor| { + descriptor.visit_slots(&mut |slot| visited.push(slot.slot)); + }, + ); - assert!( - !crate::gc::layout_tables::test_per_object_layout_present(addr), - "dead from-space owner's per-object record must be pruned by the copied-minor pass" - ); - assert_eq!( - young_layout_records(), - before, - "after the prune no nursery-keyed record remains, so the inline allocator's gate must read zero" - ); + assert!( + visited.contains(&expando_slot), + "ObjectMeta.expando must be enumerated by the trace visitor; an \ + unenumerated edge is never marked, so a live owner's own \ + properties are swept" + ); + assert!( + visited.contains(&spill_slot), + "the pre-existing `spill` edge must still be enumerated — appending \ + a field must not displace it" + ); + let _ = obj_h; + } } +/// `ObjectMeta`'s field offsets are a contract with codegen (there are +/// `offset_of!` asserts on `spill`, `array_subclass_named_prefix_token` and +/// `array_tail_object_hot`). `expando` therefore has to be APPENDED, never +/// inserted — inserting it after `spill` shifted the 48/56 fields and failed +/// those compile-time asserts. #[test] -fn test_live_old_owner_layout_record_keeps_flag_armed_but_not_young() { - let _guard = CopyingNurseryTestGuard::new(1); - let before = young_layout_records(); - let addr = unsafe { - let shape_id = crate::object::shapes::shape_descriptor_ensure(std::ptr::null(), 0, 2) - .expect("shape id range exhausted in a test fixture"); - let obj = gc_malloc( - std::mem::size_of::() + 2 * 8, - GC_TYPE_OBJECT, - ) as *mut crate::object::ObjectHeader; - (*obj).class_id = 0; - (*obj).parent_class_id = shape_id; - (*obj).meta = std::ptr::null_mut(); - let fields = - (obj as *mut u8).add(std::mem::size_of::()) as *mut u64; - *fields = 0; - *fields.add(1) = 0; - obj as usize - }; - unsafe { install_typed_record(addr) }; - assert!( - crate::gc::layout_tables::test_per_object_layout_present(addr), - "premise: the malloc'd owner carries a typed record" - ); - assert_ne!( - crate::gc::layout_tables::test_per_object_layout_armed_threads(), - 0, - "a live record keeps the armed-thread count non-zero" - ); +fn object_meta_expando_is_appended_not_inserted() { assert_eq!( - young_layout_records(), - before, - "a record on a gc_malloc page is not one the bump allocator can recycle" + std::mem::offset_of!(crate::object::ObjectMeta, spill), + 32, + "spill must keep its contracted offset" + ); + assert!( + std::mem::offset_of!(crate::object::ObjectMeta, expando) + > std::mem::offset_of!(crate::object::ObjectMeta, array_tail_object_hot), + "expando must sit after every field codegen has an offset contract on" ); - crate::gc::layout_clear_for_ptr(addr); } diff --git a/crates/perry-runtime/src/gc/tests/error_side_tables.rs b/crates/perry-runtime/src/gc/tests/error_side_tables.rs index 66522a9023..7692d440da 100644 --- a/crates/perry-runtime/src/gc/tests/error_side_tables.rs +++ b/crates/perry-runtime/src/gc/tests/error_side_tables.rs @@ -78,16 +78,16 @@ fn test_dead_error_side_table_entries_cleared() { #[test] fn test_object_valued_user_prop_is_a_gc_root_and_rewrites() { let _guard = CopyingNurseryTestGuard::new(1); - // The guard clears the thread's mutable-scanner registry for isolation; - // this test is ABOUT the scanner, so re-register it. - gc_register_mutable_root_scanner( - crate::node_submodules::diagnostics_gc::scan_error_user_props_roots_mut, - ); + // #6759 phase 1: no scanner to re-register any more. The prop's referent + // now hangs off the error's `ObjectMeta.expando` bag, so it is kept alive + // and rewritten by ORDINARY object tracing rather than by a bespoke + // mutable-root scanner over an address-keyed table. The guarantee this + // test asserts is unchanged; the mechanism providing it is simpler. let err = crate::error::js_error_new() as usize; js_shadow_slot_set(0, error_bits(err)); - // The prop's object is reachable ONLY through the side table. + // The prop's object is reachable ONLY through the error's metadata bag. let cause = crate::object::js_object_alloc(0, 0); crate::node_submodules::diagnostics::set_error_user_prop( err, @@ -104,8 +104,8 @@ fn test_object_valued_user_prop_is_a_gc_root_and_rewrites() { assert_ne!( prop_addr, cause as usize, "the object referent must have been evacuated (and the stored \ - bits rewritten) — identical address means the scanner did not \ - visit the slot" + bits rewritten) — an identical address means the expando bag's \ + edge was not traced" ); unsafe { let header = (prop_addr - crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index f46398111f..49a7391cb0 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -71,6 +71,7 @@ pub(super) unsafe fn alloc_old_test_promise() -> *mut crate::promise::Promise { next: std::ptr::null_mut(), async_id: 0, trigger_async_id: 0, + meta: std::ptr::null_mut(), }, ); ptr @@ -93,6 +94,7 @@ pub(super) unsafe fn alloc_old_test_error() -> *mut crate::error::ErrorHeader { stack: std::ptr::null_mut(), cause: f64::from_bits(crate::value::TAG_UNDEFINED), errors: std::ptr::null_mut(), + meta: std::ptr::null_mut(), }, ); ptr @@ -789,6 +791,7 @@ pub(super) fn allocate_dead_malloc_churn_headers(per_type: usize) -> Vec next: std::ptr::null_mut(), async_id: 0, trigger_async_id: 0, + meta: std::ptr::null_mut(), }, ); headers.push(header_from_user_ptr(ptr as *const u8) as usize); diff --git a/crates/perry-runtime/src/gc/types.rs b/crates/perry-runtime/src/gc/types.rs index 41f75f72a6..debae4f422 100644 --- a/crates/perry-runtime/src/gc/types.rs +++ b/crates/perry-runtime/src/gc/types.rs @@ -168,6 +168,9 @@ pub(crate) enum GcRewriteDescriptorKind { NativePodView, /// #6759 Phase B: one traced NaN-box slot (`ObjectMeta::prototype`). ObjectMeta, + /// #6759 phase 1: a cell whose ONLY traced edge is its metadata record. + /// `DateCell` was `Leaf` (pointer-free) before it gained a `meta` field. + MetaOnly, } #[allow(dead_code)] @@ -588,7 +591,7 @@ pub(super) static GC_TYPE_INFO_BY_ID: [Option; MALLOC_KIND_BUCKET_CO "date", GcAllocationPolicy::Arena, true, - GcRewriteDescriptorKind::Leaf, + GcRewriteDescriptorKind::MetaOnly, GcLayoutSlotKind::None, // Movable (#6186, 2026-07-09 GC audit). Directly analogous to // `GC_TYPE_PROMISE` above: a pointer-free arena object with @@ -607,8 +610,12 @@ pub(super) static GC_TYPE_INFO_BY_ID: [Option; MALLOC_KIND_BUCKET_CO true, GcExternalBytePolicy::None, GcLargeObjectPolicy::NotApplicable, - // pointer_free: the single `ts` slot is a raw f64, never a JSValue. - true, + // pointer_free = FALSE since #6759 phase 1. The cell used to be one + // raw `f64` and nothing else; it now also carries a `meta` edge, so + // the collector must scan it. `validate_gc_type_info` enforces this + // pairing — a pointer-free type may not expose a rewrite descriptor — + // and caught the flag when it was left at `true`. + false, // `d.foo = …` expandos live in `object::exotic_expando` keyed by the // Date address; rekey that entry when the cell relocates (mirrors // Promise). Without this a moved Date loses its expando properties. @@ -923,7 +930,8 @@ pub(crate) fn validate_gc_type_info(info: &GcTypeInfo) -> Result<(), &'static st return Err("closure rewrite descriptor must expose closure capture slots"); } } - GcRewriteDescriptorKind::Promise + GcRewriteDescriptorKind::MetaOnly + | GcRewriteDescriptorKind::Promise | GcRewriteDescriptorKind::Error | GcRewriteDescriptorKind::Map | GcRewriteDescriptorKind::LazyArray diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index e05d5a8c3a..82d2b94a0e 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -1094,6 +1094,15 @@ pub struct MapHeader { pub entries: *mut f64, /// Direct pointer to the stable numeric-key index owned by MAP_REGISTRY. numeric_index: *mut NumericIndex, + /// #6759 phase 1 (header unification): per-object metadata record, or + /// null — the same `ObjectMeta` cell an `ObjectHeader` hangs off its own + /// `meta` field. Appended LAST so every preceding field keeps its offset. + /// + /// Traced and rewritten by the `GcRewriteDescriptorKind::Map` arm, which + /// `trace_heap_rewrite_slots` drives, so listing it there makes the edge + /// marked as well as rewritten (#6812: an edge visited only on the rewrite + /// path is invisible to marking). + pub meta: *mut crate::object::ObjectMeta, } /// Each map entry is 16 bytes (key + value, both as f64/JSValue) @@ -1326,6 +1335,10 @@ pub extern "C" fn js_map_alloc(capacity: u32) -> *mut MapHeader { // GC_STORE_AUDIT(INIT): map entries buffer is external storage; element stores are barriered separately. (*ptr).entries = entries; (*ptr).numeric_index = std::ptr::null_mut(); + // #6759 phase 1: the arena allocator reuses free-list memory without + // zeroing, so this MUST be initialised explicitly — an uninitialised + // meta edge is a garbage pointer the collector would follow. + (*ptr).meta = std::ptr::null_mut(); // Register in map registry for runtime type detection register_map(ptr, entries, cap as usize); diff --git a/crates/perry-runtime/src/node_submodules/diagnostics.rs b/crates/perry-runtime/src/node_submodules/diagnostics.rs index 080123eec3..e72e2621c2 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics.rs @@ -590,34 +590,6 @@ pub enum ErrUserProp { Bits(u64), } -thread_local! { - /// User-assigned own properties on `Error` objects, keyed by the error - /// object pointer. - /// - /// `ErrorHeader` is a fixed `#[repr(C)]` struct with no overflow-field - /// region, so a plain `err.foo = bar` had nowhere to land: the object - /// setter dropped it and the getter returned `undefined`. That broke - /// Node parity — e.g. `assert.throws(fn, { code })` could not read a - /// user-assigned `.code` (#2014). This side table gives errors arbitrary - /// 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. - /// 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()); -} - unsafe fn error_user_prop_string(value: f64) -> String { let ptr = crate::value::js_jsvalue_to_string(value); if ptr.is_null() { @@ -635,21 +607,18 @@ pub fn set_error_user_prop(error_ptr: usize, key: &str, value: f64) { if error_ptr == 0 { return; } - let stored = if JSValue::from_bits(value.to_bits()).is_any_string() { - ErrUserProp::Str(unsafe { error_user_prop_string(value) }) - } else { - ErrUserProp::Bits(value.to_bits()) - }; - ERROR_USER_PROPS.with(|m| { - 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)), - } - }); + // #6759 phase 1: the property bag now hangs off the error's own metadata + // record instead of a table keyed by its address, so it moves with the + // error, dies with it, and cannot be inherited by a later tenant of a + // recycled address. Insertion order comes free from the bag object's + // `keys_array`. + unsafe { + let Some(bag) = crate::object::cell_expando_ensure(error_ptr) else { + return; + }; + let key_ptr = js_string_from_bytes(key.as_ptr(), key.len() as u32); + crate::object::js_object_set_field_by_name(bag, key_ptr, value); + } } /// Look up a user-assigned own property on an `Error` object, materialising it @@ -659,17 +628,23 @@ pub fn error_user_prop(error_ptr: usize, key: &str) -> Option { if error_ptr == 0 { return None; } - ERROR_USER_PROPS.with(|m| { - m.borrow().get(&error_ptr).and_then(|props| { - 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()) - } - ErrUserProp::Bits(b) => f64::from_bits(*b), - }) - }) - }) + unsafe { + let bag = crate::object::cell_expando_get(error_ptr)?; + let key_ptr = js_string_from_bytes(key.as_ptr(), key.len() as u32); + // Distinguish "absent" from "present and undefined": a bare get would + // return `undefined` for both, and the caller uses `None` to mean the + // error has no such own property at all. + let key_boxed = f64::from_bits(crate::js_nanbox_string(key_ptr as i64).to_bits()); + if !crate::object::obj_value_has_own_key( + crate::value::js_nanbox_pointer(bag as i64), + key_boxed, + ) { + return None; + } + Some(f64::from_bits( + crate::object::js_object_get_field_by_name(bag, key_ptr).bits(), + )) + } } /// Remove a user-assigned own property from an Error object. Returns true @@ -679,18 +654,21 @@ pub fn remove_error_user_prop(error_ptr: usize, key: &str) -> bool { if error_ptr == 0 { return false; } - ERROR_USER_PROPS.with(|m| { - m.borrow_mut() - .get_mut(&error_ptr) - .map(|props| match props.iter().position(|(k, _)| k == key) { - Some(i) => { - props.remove(i); - true - } - None => false, - }) - .unwrap_or(false) - }) + unsafe { + let Some(bag) = crate::object::cell_expando_get(error_ptr) else { + return false; + }; + let key_ptr = js_string_from_bytes(key.as_ptr(), key.len() as u32); + let key_boxed = f64::from_bits(crate::js_nanbox_string(key_ptr as i64).to_bits()); + if !crate::object::obj_value_has_own_key( + crate::value::js_nanbox_pointer(bag as i64), + key_boxed, + ) { + return false; + } + crate::object::js_object_delete_field(bag, key_ptr); + true + } } /// Return user-assigned own properties on an Error object as materialized JS @@ -699,33 +677,34 @@ pub fn error_user_props(error_ptr: usize) -> Vec<(String, f64)> { if error_ptr == 0 { return Vec::new(); } - let props: Vec<(String, ErrUserProp)> = ERROR_USER_PROPS.with(|m| { - m.borrow() - .get(&error_ptr) - .map(|props| { - props - .iter() - .map(|(key, value)| (key.clone(), value.clone())) - .collect() - }) - .unwrap_or_default() - }); - let mut props: Vec<(String, f64)> = props - .into_iter() - .map(|(key, value)| { - let materialized = match value { - 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()) - } - ErrUserProp::Bits(bits) => f64::from_bits(bits), - }; - (key, materialized) - }) - .collect(); - // No sort: the Vec is already in insertion order, which is the order - // ECMA-262 specifies and node emits. - props + unsafe { + let Some(bag) = crate::object::cell_expando_get(error_ptr) else { + return Vec::new(); + }; + // The bag is an ordinary object, so its `keys_array` already holds the + // keys in ECMA-262 insertion order — no sort, and no ordering of our + // own to keep in step with node's. + let keys = crate::object::object_keys_array(bag); + if keys.is_null() { + return Vec::new(); + } + let len = (*keys).length as usize; + let mut out = Vec::with_capacity(len); + for i in 0..len { + let key_val = crate::array::js_array_get_f64(keys, i as u32); + let name_ptr = crate::value::js_jsvalue_to_string(key_val); + if name_ptr.is_null() { + continue; + } + let name = error_user_prop_string(f64::from_bits( + crate::js_nanbox_string(name_ptr as i64).to_bits(), + )); + let value = + f64::from_bits(crate::object::js_object_get_field_by_name(bag, name_ptr).bits()); + out.push((name, value)); + } + out + } } pub(crate) fn throw_invalid_arg() -> ! { diff --git a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs index c34e48e267..03be0b06f7 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics_gc.rs @@ -3,8 +3,8 @@ //! `ErrUserProp` stay there. use super::diagnostics::{ - ErrUserProp, ERROR_MESSAGE_CODES, ERROR_MESSAGE_DESTS, ERROR_MESSAGE_ERRNOS, - ERROR_MESSAGE_HOSTNAMES, ERROR_MESSAGE_PATHS, ERROR_MESSAGE_SYSCALLS, ERROR_USER_PROPS, + ERROR_MESSAGE_CODES, ERROR_MESSAGE_DESTS, ERROR_MESSAGE_ERRNOS, ERROR_MESSAGE_HOSTNAMES, + ERROR_MESSAGE_PATHS, ERROR_MESSAGE_SYSCALLS, }; use std::cell::RefCell; use std::collections::HashMap; @@ -43,7 +43,6 @@ pub(crate) fn error_side_tables_owner_moved(old_user: usize, new_user: usize) { ERROR_MESSAGE_PATHS.with(|m| rekey(m, old_user, new_user)); ERROR_MESSAGE_DESTS.with(|m| rekey(m, old_user, new_user)); ERROR_MESSAGE_HOSTNAMES.with(|m| rekey(m, old_user, new_user)); - ERROR_USER_PROPS.with(|m| rekey(m, old_user, new_user)); } /// Drop a dead error's entries from every side table so a fresh error @@ -69,9 +68,6 @@ pub(crate) fn error_side_tables_clear_dead(user_ptr: usize) { ERROR_MESSAGE_HOSTNAMES.with(|m| { m.borrow_mut().remove(&user_ptr); }); - ERROR_USER_PROPS.with(|m| { - m.borrow_mut().remove(&user_ptr); - }); // 2026-07-09 GC audit wave 2: the DOMException brand set is address- // keyed with zero removals — clean it up with the rest of the error // side tables (latch-gated no-op unless a DOMException was ever made). @@ -117,27 +113,7 @@ pub(crate) fn finalize_dead_copied_minor_from_space_errors() { collect(ERROR_MESSAGE_PATHS.with(|m| m.borrow().keys().copied().collect())); collect(ERROR_MESSAGE_DESTS.with(|m| m.borrow().keys().copied().collect())); collect(ERROR_MESSAGE_HOSTNAMES.with(|m| m.borrow().keys().copied().collect())); - collect(ERROR_USER_PROPS.with(|m| m.borrow().keys().copied().collect())); for addr in dead { error_side_tables_clear_dead(addr); } } - -/// Registered mutable-root scanner: object/string-valued user props -/// (`ErrUserProp::Bits` holding a heap-tagged value) were INVISIBLE to GC — -/// an `err.cause = {...}` object was collectable while still reachable -/// through the error. Visit each as a mutable root so the referent stays -/// live and a moved referent's address is rewritten in place. -/// (`visit_nanbox_u64_slot` is tag-aware: numeric/boolean bits are left -/// untouched.) -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.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 index 098bd44bfc..9ffbda485c 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics_tests.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics_tests.rs @@ -1,9 +1,5 @@ -//! Unit tests for the node:diagnostics_channel submodule. -//! -//! Split out of `diagnostics.rs` to keep it under the 2,000-line file gate. +//! Tests split out of `diagnostics.rs` for 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::*; @@ -73,64 +69,98 @@ mod tests { mod error_prop_order_tests { use super::*; + /// Allocate a REAL error. These tests used synthetic addresses + /// (`0x4000_1000`) back when the properties lived in a side table keyed by + /// an arbitrary `usize` — any integer was a valid key. The bag now hangs + /// off the error's own `ObjectMeta`, so a fake address is dereferenced as a + /// GC cell and segfaults. Storage on the object means tests need objects. + unsafe fn fresh_error() -> usize { + let msg = js_string_from_bytes(b"order".as_ptr(), 5); + crate::error::js_error_new_with_message(msg) as usize + } + + fn keys_of(err: usize) -> Vec { + error_user_props(err).into_iter().map(|(k, _)| k).collect() + } + /// 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 + /// order. 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 _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let err = fresh_error(); + for k in ["errno", "code", "syscall", "path"] { + set_error_user_prop(err, k, 1.0); + } + assert_eq!( + keys_of(err), + vec![ + "errno".to_string(), + "code".to_string(), + "syscall".to_string(), + "path".to_string() + ], + "fs error fields must enumerate in node's insertion order" + ); } - 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`. + /// `o.a=1; o.b=2; o.a=3` still enumerates `a,b`. #[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" - ); + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let err = fresh_error(); + set_error_user_prop(err, "a", 1.0); + set_error_user_prop(err, "b", 2.0); + set_error_user_prop(err, "a", 3.0); + assert_eq!(keys_of(err), 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); + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let err = fresh_error(); + for k in ["one", "two", "three"] { + set_error_user_prop(err, k, 0.0); + } + assert!(remove_error_user_prop(err, "two")); + assert_eq!(keys_of(err), vec!["one".to_string(), "three".to_string()]); + assert!( + !remove_error_user_prop(err, "two"), + "second remove is a no-op" + ); + } + } + + /// #6759 phase 1: two errors must not share properties, even if one is + /// allocated at an address the other previously occupied. The old + /// address-keyed table could not express that; storage on the object does + /// so by construction. + #[test] + fn properties_belong_to_the_error_not_its_address() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let a = fresh_error(); + let b = fresh_error(); + set_error_user_prop(a, "code", 1.0); + assert_eq!(error_user_prop(a, "code"), Some(1.0)); + assert!( + error_user_prop(b, "code").is_none(), + "a distinct error must not see another's properties" + ); } - 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/meta_accessors.rs b/crates/perry-runtime/src/object/meta_accessors.rs new file mode 100644 index 0000000000..1f8d1742d7 --- /dev/null +++ b/crates/perry-runtime/src/object/meta_accessors.rs @@ -0,0 +1,101 @@ +//! Per-object meta-record accessors and the cell-generic `meta` edge (#8891). +//! +//! Split out of `object/mod.rs` for the 2,000-line file gate. + +use super::*; + +pub(crate) unsafe fn object_meta_ensure_for_cell(user_ptr: usize) -> Option<*mut ObjectMeta> { + let slot = cell_meta_slot(user_ptr)?; + if !(*slot).is_null() { + return Some(*slot); + } + let scope = crate::gc::RuntimeHandleScope::new(); + let owner = scope.root_raw_mut_ptr(user_ptr as *mut u8); + let meta = arena_alloc_gc( + std::mem::size_of::(), + 8, + crate::gc::GC_TYPE_OBJECT_META, + ) as *mut ObjectMeta; + let user_ptr = owner.get_raw_mut_ptr::() as usize; + let slot = cell_meta_slot(user_ptr)?; + if !(*slot).is_null() { + // A re-entrant path installed one while we allocated; keep it. + return Some(*slot); + } + (*meta).prototype = 0; + (*meta).attr_key_bits = 0; + (*meta).accessor_key_bits = 0; + (*meta).flags = 0; + (*meta).spill = 0; + (*meta).private_evaluation_brand = 0; + (*meta).array_subclass_named_prefix_token = 0; + (*meta).array_tail_object_hot = 0; + (*meta).array_subclass_dense_key = 0; + (*meta).array_subclass_dense_slots = 0; + (*meta).array_subclass_dense_bounds = 0; + (*meta).expando = 0; + // GC_STORE_AUDIT(BARRIERED): header-slot store followed by an object-slot + // barrier, exactly as `object_meta_ensure` does for an `ObjectHeader`. + *slot = meta; + crate::gc::runtime_write_barrier_slot(user_ptr, slot as usize, meta as u64); + Some(meta) +} + +pub(crate) unsafe fn object_meta_ensure(obj: *mut ObjectHeader) -> *mut ObjectMeta { + if !(*obj).meta.is_null() { + return (*obj).meta; + } + // Root the owner across the allocation: `arena_alloc_gc` can trigger a + // copied-minor that MOVES `obj`, and the header store below must land + // in the live copy, not the stale from-space one. Reload through the + // handle after the allocation. (The fresh `meta` record itself cannot + // move before the store — no allocation happens in between.) + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let meta = arena_alloc_gc( + std::mem::size_of::(), + 8, + crate::gc::GC_TYPE_OBJECT_META, + ) as *mut ObjectMeta; + let obj = obj_handle.get_raw_mut_ptr::(); + if !(*obj).meta.is_null() { + // A GC-triggered re-entrant path installed one meanwhile; keep it + // (the fresh record above is unreferenced and dies with the cycle). + return (*obj).meta; + } + (*meta).prototype = 0; + (*meta).attr_key_bits = 0; + (*meta).accessor_key_bits = 0; + (*meta).flags = 0; + (*meta).spill = 0; + (*meta).private_evaluation_brand = 0; + (*meta).array_subclass_named_prefix_token = 0; + (*meta).array_tail_object_hot = 0; + (*meta).array_subclass_dense_key = 0; + (*meta).array_subclass_dense_slots = 0; + (*meta).array_subclass_dense_bounds = 0; + (*meta).expando = 0; + // GC_STORE_AUDIT(BARRIERED): meta-record edge is a header-slot store + // followed by an object-slot barrier, mirroring `set_object_keys_array`. + (*obj).meta = meta; + crate::gc::runtime_write_barrier_slot( + obj as usize, + &(*obj).meta as *const _ as usize, + meta as u64, + ); + meta +} + +/// GC slot accessor for the `meta` header edge (#6759 Phase B): a raw-pointer +/// child slot. The GC type table calls this +/// only for `GC_TYPE_OBJECT`; RegExp uses its dedicated slot descriptor. +pub(crate) unsafe fn gc_object_meta_slot(user_ptr: usize) -> Option<*mut u64> { + if user_ptr == 0 { + return None; + } + let obj = user_ptr as *mut ObjectHeader; + if (*obj).meta.is_null() { + return None; + } + Some(&mut (*obj).meta as *mut _ as *mut u64) +} diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 514ab12316..3a8cbc1198 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -610,8 +610,10 @@ fn keys_index_insert( pub(crate) mod array_tail_transition; mod call_method_depth; +mod meta_accessors; use call_method_depth::CallMethodDepthGuard; pub(crate) use call_method_depth::{call_method_depth_restore, call_method_depth_savepoint}; +pub(crate) use meta_accessors::*; /// Fast direct-mapped inline cache for class shape keys arrays. /// Indexed by `shape_id mod CACHE_SIZE`. Each slot stores @@ -1642,6 +1644,21 @@ pub struct ObjectMeta { pub array_subclass_dense_key: u64, pub array_subclass_dense_slots: u64, pub array_subclass_dense_bounds: u64, + /// #6759 phase 1: named own properties for a cell that has no + /// `keys_array`/inline-slot layout of its own — a NaN-boxed pointer to an + /// ordinary object used as the property bag, or 0 when the owner has none. + /// + /// An `ErrorHeader` (and every other exotic cell) cannot store named + /// properties inline, which is why they lived in `ERROR_USER_PROPS`, keyed + /// by the owner's ADDRESS and needing four GC hooks of their own — + /// rekey-on-evacuation, finalize, dead-sweep and a root scanner — plus the + /// long-standing bug that a recycled address inherited the previous + /// tenant's properties. + /// + /// Hanging the bag off the metadata record instead makes it an ordinary + /// child edge: it moves with its owner, dies with its owner, and needs no + /// address bookkeeping at all. + pub expando: u64, } pub(crate) const OBJECT_META_FLAG_PROTO_OVERRIDE: u64 = 1; @@ -1690,64 +1707,70 @@ const _: () = assert!(std::mem::size_of::() == 8); /// Fetch-or-allocate the per-object meta record. Caller must have already /// established that `obj` is a live `GC_TYPE_OBJECT` allocation /// (see `prototype_chain::meta_capable_object`). -pub(crate) unsafe fn object_meta_ensure(obj: *mut ObjectHeader) -> *mut ObjectMeta { - if !(*obj).meta.is_null() { - return (*obj).meta; - } - // Root the owner across the allocation: `arena_alloc_gc` can trigger a - // copied-minor that MOVES `obj`, and the header store below must land - // in the live copy, not the stale from-space one. Reload through the - // handle after the allocation. (The fresh `meta` record itself cannot - // move before the store — no allocation happens in between.) - let scope = crate::gc::RuntimeHandleScope::new(); - let obj_handle = scope.root_raw_mut_ptr(obj); - let meta = arena_alloc_gc( - std::mem::size_of::(), - 8, - crate::gc::GC_TYPE_OBJECT_META, - ) as *mut ObjectMeta; - let obj = obj_handle.get_raw_mut_ptr::(); - if !(*obj).meta.is_null() { - // A GC-triggered re-entrant path installed one meanwhile; keep it - // (the fresh record above is unreferenced and dies with the cycle). - return (*obj).meta; +/// The metadata edge of ANY cell that has one, addressed uniformly. +/// +/// #6759 phase 1 (header unification). Cell types declare their fields +/// independently — there is no shared header prefix — so "does this cell own an +/// `ObjectMeta`?" had no single answer and every caller had to know it was +/// holding an `ObjectHeader` before it could ask. That is why per-object state +/// for the exotic types accumulated in side tables keyed by address instead: +/// there was nowhere on the cell to put it. +/// +/// This is the one path the migration needs. It returns `None` for a cell type +/// that has no metadata edge yet, so callers degrade to their existing side +/// table rather than mis-reading another layout's bytes as a pointer. +/// +/// Every exotic cell type now answers this: Object, Error, Map, Set, RegExp, +/// Promise and Date. Anything else (Temporal, the typed-array views) returns +/// `None` and keeps its existing storage. +pub(crate) unsafe fn cell_meta_slot(user_ptr: usize) -> Option<*mut *mut ObjectMeta> { + // Canonical validated read rather than an open-coded magnitude test: + // `try_read_gc_header` applies `is_plausible_heap_addr` AND rejects + // small-buffer slab addresses, which are heap-plausible but carry no + // GcHeader — reading one classifies the previous slab entry's bytes as a + // type tag. + let Some(gc_hdr) = crate::value::addr_class::try_read_gc_header(user_ptr) else { + return None; + }; + match gc_hdr.obj_type { + crate::gc::GC_TYPE_OBJECT => { + Some(&mut (*(user_ptr as *mut ObjectHeader)).meta as *mut *mut ObjectMeta) + } + crate::gc::GC_TYPE_ERROR => { + Some(&mut (*(user_ptr as *mut crate::error::ErrorHeader)).meta as *mut *mut ObjectMeta) + } + crate::gc::GC_TYPE_MAP => { + Some(&mut (*(user_ptr as *mut crate::map::MapHeader)).meta as *mut *mut ObjectMeta) + } + crate::gc::GC_TYPE_SET => { + Some(&mut (*(user_ptr as *mut crate::set::SetHeader)).meta as *mut *mut ObjectMeta) + } + crate::gc::GC_TYPE_REGEXP => { + Some(&mut (*(user_ptr as *mut crate::regex::RegExpHeader)).meta as *mut *mut ObjectMeta) + } + crate::gc::GC_TYPE_PROMISE => { + Some(&mut (*(user_ptr as *mut crate::promise::Promise)).meta as *mut *mut ObjectMeta) + } + crate::gc::GC_TYPE_DATE_CELL => { + Some(&mut (*(user_ptr as *mut crate::date::DateCell)).meta as *mut *mut ObjectMeta) + } + // Anything still without a metadata edge answers absence rather than + // mis-reading its own layout as a pointer. + _ => None, } - (*meta).prototype = 0; - (*meta).attr_key_bits = 0; - (*meta).accessor_key_bits = 0; - (*meta).flags = 0; - (*meta).spill = 0; - (*meta).private_evaluation_brand = 0; - (*meta).array_subclass_named_prefix_token = 0; - (*meta).array_tail_object_hot = 0; - (*meta).array_subclass_dense_key = 0; - (*meta).array_subclass_dense_slots = 0; - (*meta).array_subclass_dense_bounds = 0; - // GC_STORE_AUDIT(BARRIERED): meta-record edge is a header-slot store - // followed by an object-slot barrier, mirroring `set_object_keys_array`. - (*obj).meta = meta; - crate::gc::runtime_write_barrier_slot( - obj as usize, - &(*obj).meta as *const _ as usize, - meta as u64, - ); - meta } -/// GC slot accessor for the `meta` header edge (#6759 Phase B): a raw-pointer -/// child slot. The GC type table calls this -/// only for `GC_TYPE_OBJECT`; RegExp uses its dedicated slot descriptor. -pub(crate) unsafe fn gc_object_meta_slot(user_ptr: usize) -> Option<*mut u64> { - if user_ptr == 0 { - return None; - } - let obj = user_ptr as *mut ObjectHeader; - if (*obj).meta.is_null() { - return None; - } - Some(&mut (*obj).meta as *mut _ as *mut u64) +/// Does `user_ptr` name a cell that can own an `ObjectMeta`? +pub(crate) unsafe fn cell_has_meta_edge(user_ptr: usize) -> bool { + cell_meta_slot(user_ptr).is_some() } +/// Materialise the metadata record for ANY cell that has a metadata edge, +/// allocating one on first use. `None` for a cell type not yet unified. +/// +/// The allocation can trigger a collection that MOVES the owner, so the slot +/// is re-resolved from the rooted address afterwards rather than reusing the +/// pointer taken before the allocation. #[inline] unsafe fn set_object_keys_array(obj: *mut ObjectHeader, keys_array: *mut ArrayHeader) { let live = object_live_slot_count(obj); @@ -1876,3 +1899,63 @@ pub(super) unsafe fn mark_object_dynamic_shape_unknown(obj: *mut ObjectHeader) { #[cfg(test)] mod tests; + +/// The named-property bag for a cell that has no inline slot layout of its own, +/// creating it on first write. +/// +/// #6759 phase 1. An `ErrorHeader` (and the other exotic cells) cannot hold +/// named properties inline, so they lived in tables keyed by the owner's +/// ADDRESS — `ERROR_USER_PROPS` and friends — which cost four GC hooks +/// (rekey-on-evacuation, finalize, dead-sweep, root scanner) and carried a +/// standing hazard: a recycled address inherits the previous tenant's +/// properties. +/// +/// The bag is an ordinary object hanging off `ObjectMeta.expando`, so it is an +/// ordinary child edge — it moves with its owner, dies with its owner, and +/// keeps ECMA-262 insertion order for free because that is what an object's +/// `keys_array` already does. +pub(crate) unsafe fn cell_expando_ensure(user_ptr: usize) -> Option<*mut ObjectHeader> { + let meta = object_meta_ensure_for_cell(user_ptr)?; + if (*meta).expando != 0 { + return Some( + crate::value::JSValue::from_bits((*meta).expando).as_pointer::() + as *mut ObjectHeader, + ); + } + // `js_object_alloc` allocates and can move the owner, so re-resolve the + // meta record from the rooted address afterwards. + let scope = crate::gc::RuntimeHandleScope::new(); + let owner = scope.root_raw_mut_ptr(user_ptr as *mut u8); + let bag = js_object_alloc(0, 0); + let user_ptr = owner.get_raw_mut_ptr::() as usize; + let meta = object_meta_ensure_for_cell(user_ptr)?; + if (*meta).expando != 0 { + return Some( + crate::value::JSValue::from_bits((*meta).expando).as_pointer::() + as *mut ObjectHeader, + ); + } + let boxed = crate::value::js_nanbox_pointer(bag as i64).to_bits(); + // GC_STORE_AUDIT(BARRIERED): metadata-record slot store + object barrier. + (*meta).expando = boxed; + crate::gc::runtime_write_barrier_slot( + meta as usize, + &(*meta).expando as *const _ as usize, + boxed, + ); + Some(bag) +} + +/// The existing bag, or `None` when the owner never took one. Never allocates, +/// so it is safe on read paths. +pub(crate) unsafe fn cell_expando_get(user_ptr: usize) -> Option<*mut ObjectHeader> { + let slot = cell_meta_slot(user_ptr)?; + let meta = *slot; + if meta.is_null() || (*meta).expando == 0 { + return None; + } + Some( + crate::value::JSValue::from_bits((*meta).expando).as_pointer::() + as *mut ObjectHeader, + ) +} diff --git a/crates/perry-runtime/src/promise/mod.rs b/crates/perry-runtime/src/promise/mod.rs index 3d31310e26..e2c3961cd9 100644 --- a/crates/perry-runtime/src/promise/mod.rs +++ b/crates/perry-runtime/src/promise/mod.rs @@ -548,6 +548,17 @@ pub struct Promise { pub(crate) async_id: u64, /// async_hooks triggerAsyncId captured at Promise creation. pub(crate) trigger_async_id: u64, + /// #6759 phase 1 (header unification): per-object metadata record, or + /// null. Appended LAST so every preceding field keeps its offset. + /// + /// Every Promise is written through [`Promise::new`], so initialising it + /// there covers all allocation paths — which matters because neither + /// `gc_malloc` nor the arena zeroes reused memory. + /// + /// Traced and rewritten by the `GcRewriteDescriptorKind::Promise` arm, + /// which `trace_heap_rewrite_slots` drives, so the edge is marked as well + /// as rewritten (#6812). + pub(crate) meta: *mut crate::object::ObjectMeta, } impl Promise { @@ -561,6 +572,7 @@ impl Promise { next: ptr::null_mut(), async_id: 0, trigger_async_id: 0, + meta: ptr::null_mut(), } } } diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 964098fbeb..55bd468450 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -213,6 +213,9 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * std::mem::align_of::(), crate::gc::GC_TYPE_REGEXP, ) as *mut RegExpHeader; + // Neither `gc_malloc` nor the arena zeroes reused memory, so this + // must be set explicitly or the GC follows a garbage pointer. + (*ptr).meta = std::ptr::null_mut(); (*ptr).regex_ptr = std::ptr::null_mut(); (*ptr).pattern_ptr = std::ptr::null(); (*ptr).flags_ptr = std::ptr::null(); @@ -546,6 +549,15 @@ pub struct RegExpHeader { /// or null for the ordinary linear/fancy paths. Like `fancy_ptr`, this /// survives cache eviction and duplicate statically-linked runtime copies. pub repeat_matcher_ptr: *const (), + /// #6759 phase 1 (header unification): per-object metadata record, or + /// null. Appended LAST so `regex_gc_slot_ptrs`' adjacency assertion on + /// `pattern_ptr`/`flags_ptr` and every other offset are undisturbed. + /// + /// RegExp's rewrite descriptor DELEGATES to the layout visitor, so unlike + /// Error/Map/Set the edge belongs in `gc_child_slots` + /// (`GcLayoutSlotKind::RegExpFields`) — that is the marking path here. + /// #6812 is precisely the bug of putting it in the wrong one. + pub meta: *mut crate::object::ObjectMeta, } /// Self-identifying sentinel stamped into every `RegExpHeader.magic` by @@ -929,6 +941,9 @@ pub extern "C" fn js_regexp_new( // #7341: same re-read for the flags, for the same reason. let canonical_flags_ptr = flags_root.get_raw_const_ptr::(); + // Neither `gc_malloc` nor the arena zeroes reused memory, so this + // must be set explicitly or the GC follows a garbage pointer. + (*ptr).meta = std::ptr::null_mut(); (*ptr).regex_ptr = regex_ptr; (*ptr).pattern_ptr = pattern; (*ptr).flags_ptr = canonical_flags_ptr; diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 4037651a75..0a7c08de60 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -523,6 +523,12 @@ pub struct SetHeader { pub capacity: u32, /// Pointer to elements array (separately allocated) pub elements: *mut f64, + /// #6759 phase 1 (header unification): per-object metadata record, or + /// null. Appended LAST so every preceding field keeps its offset. Traced + /// and rewritten by the `GcRewriteDescriptorKind::Set` arm, which + /// `trace_heap_rewrite_slots` drives — so the edge is marked, not merely + /// rewritten (#6812). + pub meta: *mut crate::object::ObjectMeta, } /// Each set element is 8 bytes (f64/JSValue) @@ -856,6 +862,9 @@ pub extern "C" fn js_set_alloc(capacity: u32) -> *mut SetHeader { (*ptr).capacity = cap; // GC_STORE_AUDIT(INIT): set elements buffer is external storage; element stores are barriered separately. (*ptr).elements = elements; + // The arena allocator reuses free-list memory without zeroing, so an + // uninitialised meta edge would be a garbage pointer the GC follows. + (*ptr).meta = std::ptr::null_mut(); // Register in set registry for runtime type detection register_set(ptr, elements, cap as usize); @@ -2523,6 +2532,7 @@ mod tests { size: 1, capacity: 4, elements: std::ptr::null_mut(), + meta: std::ptr::null_mut(), }; let cases: &[(&str, *mut f64)] = &[ diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index ce644c6d6c..f009628ba9 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -230,13 +230,6 @@ "scanner": "node_submodules::scan_node_submodule_singleton_roots_mut (node_submodules/mod.rs:1528, visit_raw_mut_ptr_slot on DiagTracingState.obj)", "why": "Visited from the sibling module, so the same-file rule misses it. The `events: [i64; 5]` are DIAG_CHANNELS ids, not addresses." }, - { - "file": "crates/perry-runtime/src/node_submodules/diagnostics.rs", - "name": "ERROR_USER_PROPS", - "verdict": "covered_elsewhere", - "scanner": "node_submodules::diagnostics_gc::scan_error_user_props_roots_mut (diagnostics_gc.rs:46/72/134)", - "why": "The whole GC half of diagnostics lives in diagnostics_gc.rs, including the ErrorHeader-address rekey." - }, { "file": "crates/perry-runtime/src/node_submodules/diagnostics_tail.rs", "name": "DIAG_STORE_SCOPES", diff --git a/scripts/raw_handle_debt_baseline.txt b/scripts/raw_handle_debt_baseline.txt index 03b7719c61..ccb8fd57a8 100644 --- a/scripts/raw_handle_debt_baseline.txt +++ b/scripts/raw_handle_debt_baseline.txt @@ -1 +1 @@ -964 +967 diff --git a/scripts/raw_handle_debt_files.txt b/scripts/raw_handle_debt_files.txt index 1af48ace6f..e22635e5e8 100644 --- a/scripts/raw_handle_debt_files.txt +++ b/scripts/raw_handle_debt_files.txt @@ -101,7 +101,8 @@ 2 crates/perry-runtime/src/object/global_this/ctor_thunks.rs 9 crates/perry-runtime/src/object/global_this/populate.rs 5 crates/perry-runtime/src/object/global_this_webassembly.rs -1 crates/perry-runtime/src/object/mod.rs +3 crates/perry-runtime/src/object/mod.rs +2 crates/perry-runtime/src/object/meta_accessors.rs 3 crates/perry-runtime/src/object/namespace_create.rs 1 crates/perry-runtime/src/object/native_call_method/object_proto.rs 4 crates/perry-runtime/src/object/native_call_method/primitive_methods.rs @@ -158,3 +159,4 @@ 19 crates/perry-runtime/src/wasi.rs 6 crates/perry-runtime/src/weakref.rs 20 crates/perry-runtime/src/webassembly.rs +1 crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs