diff --git a/changelog.d/10552-residual-prototype-relocation.md b/changelog.d/10552-residual-prototype-relocation.md new file mode 100644 index 0000000000..5cc6654e99 --- /dev/null +++ b/changelog.d/10552-residual-prototype-relocation.md @@ -0,0 +1,9 @@ +### Fixed + +- **A relocated non-object owner lost its explicit `[[Prototype]]` (#10493).** `Object.setPrototypeOf` on a receiver that is not meta-capable records the prototype in the residual address-keyed registry (#9304), and relocation owes that entry two things. Neither happened outside arrays and ordinary objects: `layout_transfer` reached the rekey only *below* its layout-kind early return, which a `GcLayoutSlotKind::None` cell never passes, and the recorded value was emitted as a child edge from the Array and Object arms alone. So a lazy JSON array, Map, Set, Error, Promise, Date, RegExp, Temporal cell or `dyn_eval` closure silently lost its prototype at its first relocation — correct before a collection, wrong after, exit code 0 and no warning — and the prototype value itself was neither retained nor rewritten. Fixing only the rekey is worse than fixing neither: it turns "prototype lost" into "entry names a stale address", a state measured between the two halves. + + Both obligations now follow the registry's population, stated once in `prototype_chain::residual_prototype_owner_type` (everything except strings, bigints, meta records and compiled regex programs) rather than being wired to two kinds by hand. The rekey runs before the layout-kind return for every owner kind, latch-gated, with the move in a `#[cold]` call; the array-arm and move-hook copies are deleted. The recorded value is emitted ahead of the kind arms, so no arm's early return can skip it. + + Each half has its own sabotage witness: removing the rekey fails at "the registry entry did not follow its owner", and restricting the value visit back to arrays and objects fails at "the recorded prototype still names its pre-collection address". Cost is +0.04% to +0.16% instructions where the registry is never armed, and +0.68% on a fixture that arms it and churns Errors/Maps/Dates — the per-owner cost arrays and objects have always paid. + + Not a regression: the funnel before #10381 returns on the same check. Found from a CodeRabbit review comment on #10381 that landed unactioned; the population turned out to be six kinds wider than the one it named. diff --git a/changelog.d/10584-inline-mask-walk.md b/changelog.d/10584-inline-mask-walk.md new file mode 100644 index 0000000000..e90f191b7c --- /dev/null +++ b/changelog.d/10584-inline-mask-walk.md @@ -0,0 +1,44 @@ +Walk an inline slot mask directly instead of re-entering the slot iterator once +per slot. `visit_gc_layout_slot_descriptors` called +`HeapChildSlotIterator::next` for every payload slot; for the common case — a +`Masked` selection whose mask is `LayoutSlotMask::Inline` — each of those calls +re-dispatched the selection, re-decoded the mask's niche and rebuilt the limit +and cursor masks, for about eight instructions of work. + +The mask's set bits are the slot indices, in ascending order, so the arm takes +the word once and walks it with `trailing_zeros` and `word &= word - 1`. Every +other selection, including a `Heap` mask (more than 64 payload slots), keeps the +iterator. The helper carries the iterator's two side conditions with it: the +one-shot raw-numeric accounting that `next`'s first call performs, and the +cursor, left at the end so a later `next` yields nothing. The prefix and meta +edges belong to the caller, which takes them before the payload; the helper +asserts they are gone rather than arguing it. + +Measured on a control whose pointer fields target DISTINCT objects, because the +older shared-child control let the collector's one-entry address memo answer +83.3% of its classifications against 0.0% on the real fixtures, and so hid the +cost of everything downstream of that memo. On it, `next` costs 75.8 of the +417.1 instructions a pointer-slot visit costs, and the walk removes 69.7 of +them. On the same control the shared-child version reports 75.9 — the iterator's +own cost is what the blind control did NOT distort. + +The descriptor walk serves the copying minor, the full mark and the +remembered-set rebuild. Inclusive instructions for the walk on gc3, exact, by +caller: copying minor -7.95%, full mark -16.62%, remembered-set rebuild -17.59%, +dirty scan unchanged. On `oldyoung`, whose masked population is mostly one +`Heap` mask, the remembered-set rebuild and the dirty-coverage restore each pay +one failed `take_inline_mask_word` dispatch per visit: +0.25% and +0.33%, about +two instructions per object visit, against -4.49% on that fixture's copying +minor and -1.07% on the program. + +Whole program, instructions:u, min of 5: gc3 -6.75%, w20000 -5.86%, w5000 +-4.90%, w1000 -2.36%, oldyoung -1.06%, and an allocation-only fixture flat to +298 instructions in 320 million. No fixture regresses in instructions, peak RSS +or max GC pause. + +The equivalence between the walk and the iterator is a property, and is tested +as one: identical index sequences for every mask word (empty, one bit at each +end, full width, both alternations, and 64 pseudo-random words) crossed with +every live slot count from 0 to 128, with a sabotaged twin that drops the mask's +top bit and must be caught, plus a real collection whose only young child hangs +off the highest masked slot and its own sabotaged twin. diff --git a/crates/perry-runtime/src/gc/layout/transfer.rs b/crates/perry-runtime/src/gc/layout/transfer.rs index b99cce0cf2..e8eb7bbd98 100644 --- a/crates/perry-runtime/src/gc/layout/transfer.rs +++ b/crates/perry-runtime/src/gc/layout/transfer.rs @@ -17,13 +17,27 @@ //! `GC_OBJ_TYPED_LAYOUT_INTACT`. What cannot ride a header is a record keyed //! by the object's ADDRESS, and that is all this funnel moves: //! +//! * the residual static-prototype owner registry (#9304), gated by its +//! process-global latch (#7733/#7737) — for EVERY kind that can own an +//! entry, not only the layout kinds (see below); //! * the element-shape proof record (#7480), gated by the header bit that is //! authoritative for it; -//! * the residual static-prototype owner registry (#9304), gated by its -//! process-global latch (#7733/#7737); //! * the per-object `TYPED_LAYOUTS` and `LAYOUT_SLOT_MASKS` entries, gated by //! #7510's emptiness flag and address filter. //! +//! # The prototype registry is not layout metadata +//! +//! Its population is every receiver `object::prototype_chain:: +//! meta_capable_object` turns away — a Map, Set, Error, Promise, Date, RegExp, +//! Temporal cell, lazy JSON array or closure as much as an array — and most of +//! those kinds have no layout slots at all. The rekey used to sit in the array +//! arm of the record path, below the layout-kind return, and in the ordinary +//! object's move hook; every other movable owner kept its entry under the +//! address it had just left, and the dead-owner prune then dropped it. So it +//! runs first, keyed on `prototype_chain::residual_prototype_owner_type`, the +//! same population predicate the collector's value visit uses +//! (`gc/layout_slot_visit.rs`). +//! //! Until #10362 the funnel re-derived the header half too — rewriting bits //! that were already equal, and re-resolving the intact bit through a //! ShapeId-keyed `SHAPE_LAYOUTS` probe — once per relocated object. Measured @@ -64,14 +78,33 @@ use crate::gc::layout_tables::per_object_layouts_may_hold_either; /// `old_user` and `new_user` are user pointers of live allocations, and the /// caller has already made the destination header a copy of the source's (see /// the module docs). The precondition is asserted in test and debug builds. -#[inline] +/// +/// `inline(always)`: every relocation of every object runs this and all it +/// keeps inline is gates — each record move is a cold out-of-line call — but +/// three gates are enough for the heuristic to outline it from `move_young`, +/// and then the call costs more than the gates. +#[inline(always)] pub(crate) unsafe fn layout_transfer(old_user: *mut u8, new_user: *mut u8) { if old_user.is_null() || new_user.is_null() || old_user == new_user { return; } + if (old_user as usize) < GC_HEADER_SIZE + 0x1000 { + return; + } + // Before the layout-kind return, because the registry's owners are not the + // layout kinds (module docs). The latch first: it is one byte load, false + // for any process that never re-prototyped a non-object, and the move is + // out of line. + if crate::object::prototype_chain::object_static_prototypes_maybe_nonempty() + && crate::object::prototype_chain::residual_prototype_owner_type( + (*header_from_user_ptr(old_user as *const u8)).obj_type, + ) + { + transfer_residual_prototype(old_user as usize, new_user as usize); + } // Kinds with no layout metadata at all (strings, meta records, RegExps) - // leave before anything else, exactly as before #10362. The destination - // carries the same `obj_type`, so one classification answers for both. + // have no layout record to move. The destination carries the same + // `obj_type`, so one classification answers for both. let Some(old_header) = layout_header_for_user(old_user as usize) else { return; }; @@ -79,15 +112,13 @@ pub(crate) unsafe fn layout_transfer(old_user: *mut u8, new_user: *mut u8) { let reserved = (*old_header)._reserved; let is_array = (*old_header).obj_type == GC_TYPE_ARRAY; - // Three gates, all answered from words already in registers or in the one + // Two gates, both answered from words already in registers or in the one // hot thread-local slot #7510 keeps them in. Each is the same question the // record mover behind it asks first, hoisted so the common case — no // record anywhere near either address — never leaves this function. let per_object = per_object_layouts_may_hold_either(old_user as usize, new_user as usize); let element_shape = is_array && reserved & GC_ARRAY_ELEMENT_SHAPE != 0; - let static_prototype = - is_array && crate::object::prototype_chain::object_static_prototypes_maybe_nonempty(); - if per_object || element_shape || static_prototype { + if per_object || element_shape { transfer_address_keyed_records( old_user as usize, new_user as usize, @@ -102,9 +133,18 @@ pub(crate) unsafe fn layout_transfer(old_user: *mut u8, new_user: *mut u8) { header_clear_typed_layout_intact(old_header); } -/// The record moves themselves. Cold: on a workload holding no per-object -/// layout record, no element-shape proof and no re-prototyped array — the -/// steady state of every monomorphic program — it is never reached. +/// The residual prototype registry's rekey. Cold and out of line so the funnel +/// stays small enough to inline into every relocation site: it is reached only +/// once something in the process has been re-prototyped. +#[cold] +#[inline(never)] +fn transfer_residual_prototype(old_user: usize, new_user: usize) { + crate::object::prototype_chain::object_static_prototype_owner_moved(old_user, new_user); +} + +/// The layout record moves themselves. Cold: on a workload holding no +/// per-object layout record and no element-shape proof — the steady state of +/// every monomorphic program — it is never reached. #[cold] #[inline(never)] unsafe fn transfer_address_keyed_records( @@ -119,10 +159,6 @@ unsafe fn transfer_address_keyed_records( // from both headers and fails closed — it clears the destination bit // when no record follows the move. crate::array::transfer_element_shape(old_user, new_user); - // #9304: a real array keeps an explicit [[Prototype]] in the residual - // address-keyed registry; moving GC and growth both replace the owner - // allocation through this hook. - crate::object::prototype_chain::object_static_prototype_owner_moved(old_user, new_user); } // #7510's two per-object maps. Both re-test the gate above for their own // address pair, so calling them when only a sibling gate fired costs one diff --git a/crates/perry-runtime/src/gc/layout_slot_visit.rs b/crates/perry-runtime/src/gc/layout_slot_visit.rs index 68ef1bccb0..b14865203b 100644 --- a/crates/perry-runtime/src/gc/layout_slot_visit.rs +++ b/crates/perry-runtime/src/gc/layout_slot_visit.rs @@ -10,6 +10,71 @@ fn fixed_slot(slot: *mut u64) -> GcMutableSlotDescriptor { GcMutableSlotDescriptor::Slot(GcMutableSlot::new(slot, None)) } +impl HeapChildSlotIterator { + /// The payload mask WORD for a `Masked` selection whose mask is + /// [`LayoutSlotMask::Inline`]: exactly the slot indices [`Self::next`] + /// would yield, in the same ascending order, so the caller can walk them + /// with `trailing_zeros` and `word &= word - 1` instead of re-entering the + /// iterator once per slot. `None` for every other selection, which keeps + /// iterating through `next`. + /// + /// `next` re-dispatched the selection, re-decoded the mask's niche and + /// rebuilt the limit and cursor masks FOR EVERY SLOT, for about eight + /// instructions of work. + /// + /// Equivalence, since this replaces the whole iteration: + /// * `next` stops at `slot_count` and at 64 (an inline mask holds no bit + /// above 63), so the eligible set is the mask under both limits — which + /// is what this returns; + /// * it takes the ONE-SHOT raw-numeric accounting with it, exactly as + /// `next`'s first call performs it, so the counters see one record per + /// traced object either way; and + /// * it leaves the cursor at the end, so a later `next` yields nothing. + /// + /// The prefix and meta slots are NOT its business: every caller takes them + /// with `take_prefix_child_slot` / `take_meta_child_slot{,2}` before it + /// reaches the payload, so they are already `None` here. The debug + /// assertion below is what keeps that true. + pub(super) fn take_inline_mask_word(&mut self) -> Option { + // Silent loss of a prefix/meta edge is the one way this can go wrong + // without disagreeing with `next` on any payload index, so it is + // asserted rather than argued. + debug_assert!( + self.prefix_slot.is_none() && self.meta_slot.is_none() && self.meta_slot2.is_none(), + "the inline walk covers the PAYLOAD only; the caller takes the prefix and meta edges first" + ); + let slot_count = self.payload.slot_count(); + let HeapPayloadSlotSelection::Masked { + mask: LayoutSlotMask::Inline(bits), + cursor, + raw_numeric_object_slots, + raw_numeric_recorded, + } = &mut self.selection + else { + return None; + }; + debug_assert_eq!(*cursor, 0, "the inline walk replaces the whole iteration"); + if !*raw_numeric_recorded { + *raw_numeric_recorded = true; + if *raw_numeric_object_slots != 0 { + record_layout_raw_numeric_object_field_range_skipped(*raw_numeric_object_slots); + } + } + let bits = *bits; + *cursor = slot_count; + let limit = slot_count.min(64); + let limit_mask = if limit == 64 { + u64::MAX + } else { + (1u64 << limit) - 1 + }; + let word = bits & limit_mask; + #[cfg(test)] + let word = inline_mask_sabotage::perturb(word); + Some(word) + } +} + pub(super) unsafe fn visit_gc_layout_slot_descriptors( header: *mut GcHeader, visit: &mut dyn FnMut(GcMutableSlotDescriptor), @@ -117,15 +182,32 @@ pub(super) unsafe fn visit_gc_layout_slot_descriptors( }); } HeapPayloadSlotScan::Masked => { - // Iterate by reference: `for .. in child_slots` moves the iterator - // into the loop, a copy per traced object (#10362). - for child_slot in &mut child_slots { - if let HeapChildSlot::Child(slot, layout_kind) = child_slot { + // An inline mask's set bits ARE the slot indices, in ascending + // order: take the word once and walk it, instead of re-entering + // `next` per slot to re-dispatch the selection and rebuild the + // same two masks. Every other mask — `Heap`, i.e. more than 64 + // payload slots — keeps the iterator. + if let Some(mut word) = child_slots.take_inline_mask_word() { + let payload = child_slots.payload; + while word != 0 { + let index = word.trailing_zeros() as usize; + word &= word - 1; visit(GcMutableSlotDescriptor::Slot(GcMutableSlot::new( - slot, - Some(layout_kind), + payload.slot(index), + Some(HeapChildSlotReadKind::Masked), ))); } + } else { + // Iterate by reference: `for .. in child_slots` moves the + // iterator into the loop, a copy per traced object (#10362). + for child_slot in &mut child_slots { + if let HeapChildSlot::Child(slot, layout_kind) = child_slot { + visit(GcMutableSlotDescriptor::Slot(GcMutableSlot::new( + slot, + Some(layout_kind), + ))); + } + } } } HeapPayloadSlotScan::All(range) => visit(GcMutableSlotDescriptor::Range { @@ -155,7 +237,24 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( if header.is_null() || (*header).gc_flags & GC_FLAG_FORWARDED != 0 { return; } + let obj_type = (*header).obj_type; let user_ptr = (header as *mut u8).add(GC_HEADER_SIZE); + // An explicit `Object.setPrototypeOf` value recorded in the residual + // registry is a child edge of its owner, whatever the owner's kind: marking + // retains it and a moving collection rewrites it after + // `gc/layout/transfer.rs` rekeyed the entry. It used to be emitted from the + // array and ordinary-object arms only, so a Map, Set, Error, Promise, Date, + // RegExp, Temporal cell, lazy JSON array or closure owner kept a stale + // prototype address once the prototype moved. First, ahead of the kind + // arms, so no arm's early return can skip it. + if crate::object::prototype_chain::object_static_prototypes_maybe_nonempty() + && crate::object::prototype_chain::residual_prototype_owner_type(obj_type) + { + crate::object::prototype_chain::visit_object_static_prototype_slot_mut( + user_ptr as usize, + |slot| visit(fixed_slot(slot)), + ); + } match gc_type_rewrite_descriptor_kind((*header).obj_type) { GcRewriteDescriptorKind::Array => { visit_gc_layout_slot_descriptors(header, &mut visit); @@ -172,16 +271,6 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( |slot| visit(fixed_slot(slot)), ); } - // #9304: unlike shaped objects, real arrays keep an explicit - // [[Prototype]] in the residual side table. Treat that value as - // the array's child edge so collection retains and rewrites a - // movable custom prototype after layout_transfer rekeys its owner. - crate::object::prototype_chain::visit_object_static_prototype_slot_mut( - user_ptr as usize, - |slot| { - visit(fixed_slot(slot)); - }, - ); } GcRewriteDescriptorKind::Object => { // #6759 Phase B / #6812: the per-object meta record is a raw- @@ -195,14 +284,6 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( crate::object::visit_overflow_field_slots_mut(user_ptr as usize, |slot| { visit(fixed_slot(slot)); }); - // #2820: the recorded `Object.setPrototypeOf` value is a live - // reference; rewrite it if the prototype object moved. - crate::object::prototype_chain::visit_object_static_prototype_slot_mut( - user_ptr as usize, - |slot| { - visit(fixed_slot(slot)); - }, - ); } GcRewriteDescriptorKind::RegExp => { visit_gc_layout_slot_descriptors(header, &mut visit); @@ -403,3 +484,45 @@ pub(super) unsafe fn visit_gc_rewrite_slots( descriptor.visit_slots(&mut visit); }); } + +/// Test-only sabotage for the inline mask walk +/// ([`HeapChildSlotIterator::take_inline_mask_word`]): a fast path that +/// enumerates a DIFFERENT set than the iterator it replaces must be caught, so +/// the witnesses arm this and REQUIRE the failure. Its witnesses are +/// `gc::tests::layout_inline_mask`. +#[cfg(test)] +pub(crate) mod inline_mask_sabotage { + use std::cell::Cell; + + /// Forget the mask's highest slot — the one a cursor-or-limit mistake + /// loses, and the one no `0..slot_count` spot check would look at. + pub(crate) const DROP_TOP: u8 = 1; + + thread_local! { + static PERTURB: Cell = const { Cell::new(0) }; + } + + #[inline] + pub(crate) fn perturb(word: u64) -> u64 { + let armed = PERTURB.with(Cell::get); + if armed & DROP_TOP != 0 && word != 0 { + return word & !(1u64 << (63 - word.leading_zeros())); + } + word + } + + pub(crate) struct Guard(u8); + + impl Guard { + pub(crate) fn arm(what: u8) -> Self { + Self(PERTURB.with(|p| p.replace(p.get() | what))) + } + } + + impl Drop for Guard { + fn drop(&mut self) { + let prior = self.0; + PERTURB.with(|p| p.set(prior)); + } + } +} diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index dc006c52c8..d429ab4c7f 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -1042,7 +1042,7 @@ pub(in crate::gc) fn per_object_layouts_maybe_nonempty() -> bool { /// where the two `transfer_*` entry points below each resolve the hot slot /// again for their own pair (#10362). Same answer, one thread-local /// resolution: the flag and the filter live in the same slot. -#[inline] +#[inline(always)] pub(in crate::gc) fn per_object_layouts_may_hold_either(old_user: usize, new_user: usize) -> bool { let hint = hot_per_object_layout_hint(); hint.nonempty.get() && (hint_may_hold(hint, old_user) || hint_may_hold(hint, new_user)) diff --git a/crates/perry-runtime/src/gc/tests/layout_inline_mask.rs b/crates/perry-runtime/src/gc/tests/layout_inline_mask.rs new file mode 100644 index 0000000000..5512b0a7ec --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/layout_inline_mask.rs @@ -0,0 +1,220 @@ +//! The inline-mask walk that replaced the per-slot `HeapChildSlotIterator::next` +//! call in `visit_gc_layout_slot_descriptors` must enumerate EXACTLY what the +//! iterator enumerates, for every mask shape it claims. That is a property, so +//! it is tested as one — over every edge case of the mask word and of the live +//! slot count, and over a deterministic pseudo-random sample — and each +//! property has a sabotaged twin that must fail. + +use super::super::layout::{ + HeapChildSlot, HeapChildSlotIterator, HeapPayloadSlotSelection, HeapSlotRange, LayoutSlotMask, +}; +use super::super::layout_slot_visit::inline_mask_sabotage; +use super::super::*; +use super::support::*; + +/// Every mask word worth a case: empty, one bit at each end, full width, both +/// alternations, a few hand-picked sparse shapes, and 64 pseudo-random words +/// from a fixed LCG so the sample is the same on every run. +fn mask_words() -> Vec { + let mut words = vec![ + 0, + 1, + 0b10, + 0b1011, + 1 << 31, + 1 << 62, + 1 << 63, + (1 << 63) | 1, + u64::MAX, + u64::MAX >> 1, + 0xAAAA_AAAA_AAAA_AAAA, + 0x5555_5555_5555_5555, + 0xFFFF_FFFF, + 0xFFFF_FFFF_0000_0000, + ]; + let mut state: u64 = 0x2545_F491_4F6C_DD1D; + for _ in 0..64 { + state = state + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + words.push(state); + } + words +} + +/// Live payload slot counts: zero, the small shapes, both sides of every word +/// boundary, the maximum inline width (64) and past it. +fn slot_counts() -> Vec { + vec![0, 1, 2, 3, 7, 8, 31, 32, 33, 63, 64, 65, 96, 128] +} + +fn iterator_for(bits: u64, slots: &mut [u64]) -> HeapChildSlotIterator { + HeapChildSlotIterator { + prefix_slot: None, + meta_slot: None, + meta_slot2: None, + payload: HeapSlotRange::new(slots.as_mut_ptr(), slots.len()), + selection: HeapPayloadSlotSelection::Masked { + mask: LayoutSlotMask::Inline(bits), + cursor: 0, + raw_numeric_object_slots: 0, + raw_numeric_recorded: true, + }, + object_shape: None, + } +} + +/// What the iterator yields: the slot INDEX of every `Child`, in order. +fn iterated_indices(bits: u64, slots: &mut [u64]) -> Vec { + let base = slots.as_mut_ptr(); + let mut out = Vec::new(); + for child in &mut iterator_for(bits, slots) { + match child { + HeapChildSlot::Child(slot, kind) => { + assert_eq!(kind, HeapChildSlotReadKind::Masked, "masked payload slot"); + out.push((slot as usize - base as usize) / std::mem::size_of::()); + } + other => panic!("a masked payload yields only children, got {other:?}"), + } + } + out +} + +/// What the walk yields: the set bits of the word it hands the visitor, in the +/// order `trailing_zeros` + `word &= word - 1` produces. +fn walked_indices(bits: u64, slots: &mut [u64]) -> Vec { + let mut word = iterator_for(bits, slots) + .take_inline_mask_word() + .expect("an inline mask must take the walk"); + let mut out = Vec::new(); + while word != 0 { + out.push(word.trailing_zeros() as usize); + word &= word - 1; + } + out +} + +fn disagreements() -> Vec<(u64, usize)> { + let mut bad = Vec::new(); + for bits in mask_words() { + for count in slot_counts() { + let mut slots = vec![0u64; count]; + if iterated_indices(bits, &mut slots) != walked_indices(bits, &mut slots) { + bad.push((bits, count)); + } + } + } + bad +} + +#[test] +fn the_inline_walk_enumerates_exactly_what_the_iterator_enumerates() { + let bad = disagreements(); + assert!( + bad.is_empty(), + "the inline walk and `next` must agree for every mask and live slot \ + count; they disagreed on {} of {} cases, first {:?}", + bad.len(), + mask_words().len() * slot_counts().len(), + bad.first() + ); +} + +#[test] +fn every_index_the_walk_yields_is_live_and_set_in_the_mask() { + for bits in mask_words() { + for count in slot_counts() { + let mut slots = vec![0u64; count]; + for index in walked_indices(bits, &mut slots) { + assert!( + index < count, + "walked slot {index} is past the live count {count}" + ); + assert!( + bits & (1u64 << index) != 0, + "walked slot {index} is not in the mask" + ); + } + } + } +} + +#[test] +fn a_sabotaged_walk_is_caught_by_the_property() { + let _sabotage = inline_mask_sabotage::Guard::arm(inline_mask_sabotage::DROP_TOP); + let bad = disagreements(); + assert!( + !bad.is_empty(), + "with the mask's top slot dropped the walk must disagree with `next`; \ + a property that cannot see that proves nothing" + ); +} + +/// The walk is the arm a real collection takes: a young string reachable only +/// through the HIGHEST masked slot of a rooted young array must survive a +/// copying minor, and must not when that slot is dropped. +fn top_masked_slot_child_survives(sabotaged: bool) -> bool { + std::thread::spawn(move || { + let _guard = CopyingNurseryTestGuard::new(1); + let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _scan = ConservativeScanDisabledGuard::new(); + let _roots = ShadowAndGlobalRootResetGuard; + const LEN: usize = 6; + let top = LEN - 1; + let arr = crate::array::js_array_alloc_with_length(LEN as u32); + let child = young_leaf(); + // Numbers everywhere else, so the mask holds exactly the top slot: a + // walk that loses its top bit loses this child and nothing else. + for index in 0..top { + crate::array::js_array_set_f64(arr, index as u32, index as f64); + } + crate::array::js_array_set_f64(arr, top as u32, f64::from_bits(string_bits(child))); + js_shadow_slot_set(0, ptr_bits(arr as usize)); + + // Premise: this array's payload really is an inline-masked selection + // whose only bit is the top slot. Without it the collection below takes + // the general arm and proves nothing about the walk. + let word = unsafe { + let header = header_from_user_ptr(arr as *const u8) as *mut GcHeader; + crate::gc::layout::gc_child_slots(header).take_inline_mask_word() + }; + assert_eq!( + word, + Some(1u64 << top), + "premise: the fixture must produce an inline mask holding only slot {top}" + ); + + { + let _sabotage = + sabotaged.then(|| inline_mask_sabotage::Guard::arm(inline_mask_sabotage::DROP_TOP)); + let _ = gc_collect_minor(); + } + let arr_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!(arr_after, arr as usize, "premise: the rooted array moved"); + let slot = unsafe { + *crate::array::gc_element_slot_range(arr_after as *mut crate::array::ArrayHeader) + .expect("the array must still enumerate its elements") + .slot(top) + }; + (slot & POINTER_MASK) as usize != child + }) + .join() + .expect("inline-mask collection test thread must not panic") +} + +#[test] +fn the_top_masked_slots_child_is_evacuated_through_the_walk() { + assert!( + top_masked_slot_child_survives(false), + "the child in the highest masked slot must be evacuated and the slot rewritten" + ); +} + +#[test] +fn sabotaging_the_walks_top_slot_strands_its_child() { + assert!( + !top_masked_slot_child_survives(true), + "with the mask's top slot dropped the child is never visited, so the \ + slot still names from-space" + ); +} diff --git a/crates/perry-runtime/src/gc/tests/mod.rs b/crates/perry-runtime/src/gc/tests/mod.rs index 1ffdcb9893..33ca843b2e 100644 --- a/crates/perry-runtime/src/gc/tests/mod.rs +++ b/crates/perry-runtime/src/gc/tests/mod.rs @@ -47,6 +47,7 @@ mod inline_generation_gate_contract; mod inline_pointer_bearing_contract; mod json_parse_scalar; mod json_stringify_output; +mod layout_inline_mask; mod layout_pointer_free_hazard; mod layout_residue_histogram; mod layout_trace; @@ -59,6 +60,7 @@ mod os_tag; mod promote_in_place; mod promoted_cohort; mod proxy_registry; +mod residual_prototype_relocation; mod restore_coverage; mod retention_9628_9629; mod root_words; diff --git a/crates/perry-runtime/src/gc/tests/residual_prototype_relocation.rs b/crates/perry-runtime/src/gc/tests/residual_prototype_relocation.rs new file mode 100644 index 0000000000..2340ea7f0a --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/residual_prototype_relocation.rs @@ -0,0 +1,282 @@ +//! An explicit `[[Prototype]]` recorded in the residual registry must survive +//! its owner's relocation, whatever the owner's kind. +//! +//! `Object.setPrototypeOf` keeps a shaped object's prototype in its meta record +//! and everything else — every receiver `meta_capable_object` turns away — in +//! the residual address-keyed registry (`object::prototype_chain`). That entry +//! carries two collector obligations: rekey it when the owner moves, and treat +//! its value as a child edge so the prototype is retained and rewritten. Both +//! were wired to arrays and ordinary objects only, the rekey below +//! `layout_transfer`'s layout-kind return. Every other movable owner — a lazy +//! JSON array, Map, Set, Error, Promise, Date, RegExp — lost its prototype at +//! its first copying minor, and with the rekey alone would have kept a stale +//! address to a prototype that had moved or died. +//! +//! These tests have to be able to fail, so every premise they rest on — the +//! entry landed in the registry, the owner and the prototype really moved — is +//! asserted before the verdict. + +use super::super::*; +use super::support::*; + +/// Retargeting any array-like latches the process-wide "an array somewhere has +/// a custom `[[Prototype]]`" flag and stands the index fast paths down for the +/// rest of the binary (see `ArrayPrototypeLatchGuard` in `dyn_eval/tests.rs`). +/// Restore what the test found. +struct ArrayPrototypeLatchRestore { + latch: bool, + invalidated: u8, +} + +impl ArrayPrototypeLatchRestore { + fn capture() -> Self { + Self { + latch: crate::object::prototype_chain::array_static_proto_recorded(), + invalidated: crate::array::PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED + .load(std::sync::atomic::Ordering::Relaxed), + } + } +} + +impl Drop for ArrayPrototypeLatchRestore { + fn drop(&mut self) { + crate::object::prototype_chain::test_swap_array_static_proto_recorded(self.latch); + crate::array::test_swap_array_index_fast_path_invalidated(self.invalidated); + } +} + +/// A small `JSON.parse`-shaped lazy array: small enough that its cluster is +/// born in the nursery (`json_tape::lazy_cluster_is_old`), so a copying minor +/// moves it. +fn nursery_lazy_array(input: &[u8]) -> usize { + let text = crate::string::js_string_from_bytes(input.as_ptr(), input.len() as u32); + crate::json_tape::with_built_tape(input, |tape| unsafe { + crate::json_tape::alloc_lazy_array( + tape, + 0, + crate::json_tape::count_array_length(tape, 0), + text, + ) + }) + .expect("valid JSON should build a tape") as usize +} + +fn obj_type_at(user: usize) -> u8 { + unsafe { (*header_from_user_ptr(user as *const u8)).obj_type } +} + +const MARKER: f64 = 10362.0; + +fn marked_prototype() -> usize { + let proto = crate::object::js_object_alloc(0, 1); + crate::object::js_object_set_field(proto, 0, crate::value::JSValue::number(MARKER)); + proto as usize +} + +fn forget_owners(owners: &[usize]) { + crate::object::prototype_chain::prune_dead_object_prototype_owners(&|owner| { + owners.contains(&owner) + }); +} + +/// The claim as reported against #10381: a lazy array's prototype, set through +/// the real `Object.setPrototypeOf` entry, across a copying minor that moves +/// both the array and its prototype. +#[test] +fn test_lazy_array_explicit_prototype_survives_a_copying_minor() { + let _serialized = crate::array::test_serialize(); + let _feedback = crate::typed_feedback::typed_feedback_test_lock(); + let _latch = ArrayPrototypeLatchRestore::capture(); + // Two rooted values — the lazy array and its prototype — so two shadow + // slots: a store outside the pushed frame is a silent no-op (#7184). + let _guard = CopyingNurseryTestGuard::new(2); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + + let lazy = nursery_lazy_array(b"[1,2,3]"); + assert_eq!( + obj_type_at(lazy), + GC_TYPE_LAZY_ARRAY, + "premise: a real lazy array" + ); + assert!( + crate::arena::pointer_in_nursery(lazy), + "premise: the lazy cluster is nursery-born, so a copying minor can move it" + ); + assert!(crate::gc::gc_type_is_movable(GC_TYPE_LAZY_ARRAY)); + assert!( + unsafe { crate::object::prototype_chain::meta_capable_object(lazy) }.is_none(), + "premise: a lazy array has no meta record, so its prototype goes to the registry" + ); + + let proto = marked_prototype(); + js_shadow_slot_set(0, ptr_bits(lazy)); + js_shadow_slot_set(1, ptr_bits(proto)); + + // The real user-facing entry, not the recorder beneath it. + crate::object::js_object_set_prototype_of( + f64::from_bits(ptr_bits(lazy)), + f64::from_bits(ptr_bits(proto)), + ); + let lazy = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + let proto = (js_shadow_slot_get(1) & POINTER_MASK) as usize; + assert_eq!( + obj_type_at(lazy), + GC_TYPE_LAZY_ARRAY, + "premise: setPrototypeOf left the receiver a lazy array" + ); + assert!( + crate::object::prototype_chain::test_prototype_registry_latch_armed(), + "premise: the residual registry holds an entry" + ); + assert_eq!( + crate::object::prototype_chain::object_static_prototype(lazy), + Some(ptr_bits(proto)), + "premise: the prototype is recorded in the residual registry under the lazy \ + header's own address" + ); + assert_eq!( + crate::object::js_object_get_prototype_of(f64::from_bits(ptr_bits(lazy))).to_bits(), + ptr_bits(proto), + "premise: Object.getPrototypeOf resolves it before anything moves" + ); + + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + let lazy_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + let proto_after = (js_shadow_slot_get(1) & POINTER_MASK) as usize; + assert_ne!( + lazy_after, lazy, + "the copying minor must actually relocate the lazy header — an unmoved \ + receiver proves nothing" + ); + assert_ne!( + proto_after, proto, + "the prototype must move too, or a stale recorded address would go unseen" + ); + assert_eq!(obj_type_at(lazy_after), GC_TYPE_LAZY_ARRAY); + + let recorded = crate::object::prototype_chain::object_static_prototype(lazy_after); + let resolved = + crate::object::js_object_get_prototype_of(f64::from_bits(ptr_bits(lazy_after))).to_bits(); + // Leave no entry behind for a later test to trip over, whatever the verdict. + forget_owners(&[lazy, lazy_after]); + + assert_eq!( + recorded, + Some(ptr_bits(proto_after)), + "the registry entry must follow the lazy header to its new address and name \ + the prototype at ITS current address" + ); + assert_eq!( + resolved, + ptr_bits(proto_after), + "Object.getPrototypeOf on the relocated lazy array must still return the \ + prototype it was given" + ); +} + +/// Every movable receiver kind that keeps its prototype in the residual +/// registry, with the prototype held by NOTHING but that entry. One copying +/// minor has to rekey the entry, retain the prototype through it, and rewrite +/// the recorded address — the three obligations the registry's population +/// shares regardless of kind. Arrays and ordinary objects are the controls: +/// they were covered before. +#[test] +fn test_residual_prototype_owners_of_every_movable_kind_survive_a_copying_minor() { + let _serialized = crate::array::test_serialize(); + let _feedback = crate::typed_feedback::typed_feedback_test_lock(); + let _latch = ArrayPrototypeLatchRestore::capture(); + // One rooted owner at a time; its prototype is deliberately unrooted. + let _guard = CopyingNurseryTestGuard::new(1); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + + type Alloc = Box usize>; + let mut owners: Vec<(&str, Alloc)> = vec![ + ( + "array", + Box::new(|| crate::array::js_array_alloc(4) as usize), + ), + ( + "object", + Box::new(|| crate::object::js_object_alloc(0, 1) as usize), + ), + ("lazy array", Box::new(|| nursery_lazy_array(b"[1,2,3]"))), + ("Map", Box::new(|| crate::map::js_map_alloc(0) as usize)), + ("Set", Box::new(|| crate::set::js_set_alloc(0) as usize)), + ("Error", Box::new(|| crate::error::js_error_new() as usize)), + ( + "Promise", + Box::new(|| crate::promise::js_promise_new() as usize), + ), + ( + "Date", + Box::new(|| (crate::date::js_date_new().to_bits() & POINTER_MASK) as usize), + ), + ]; + #[cfg(feature = "regex-engine")] + owners.push(( + "RegExp", + Box::new(|| { + let pattern = crate::string::js_string_from_bytes(b"a+".as_ptr(), 2); + let flags = crate::string::js_string_from_bytes(b"g".as_ptr(), 1); + crate::regex::js_regexp_new(pattern, flags) as usize + }), + )); + + for (kind, alloc) in owners { + let owner = alloc(); + let obj_type = obj_type_at(owner); + assert!( + crate::arena::pointer_in_nursery(owner) && crate::gc::gc_type_is_movable(obj_type), + "{kind}: premise: a nursery owner of a movable kind" + ); + js_shadow_slot_set(0, ptr_bits(owner)); + let proto = marked_prototype(); + crate::object::js_object_set_prototype_of( + f64::from_bits(ptr_bits(owner)), + f64::from_bits(ptr_bits(proto)), + ); + let owner = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_eq!( + crate::object::prototype_chain::object_static_prototype(owner), + Some(ptr_bits(proto)), + "{kind}: premise: the prototype is recorded for this owner" + ); + + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + let owner_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize; + assert_ne!( + owner_after, owner, + "{kind}: premise: the owner must actually move" + ); + + let recorded = crate::object::prototype_chain::object_static_prototype(owner_after); + forget_owners(&[owner, owner_after]); + js_shadow_slot_set(0, 0); + + let Some(recorded) = recorded else { + panic!("{kind}: the registry entry did not follow its owner to the new address"); + }; + let proto_after = (recorded & POINTER_MASK) as usize; + assert_ne!( + proto_after, proto, + "{kind}: the recorded prototype still names its pre-collection address — the \ + prototype was either not retained through the entry or not rewritten" + ); + assert!( + crate::arena::pointer_in_nursery(proto_after) + || crate::arena::pointer_in_old_gen(proto_after), + "{kind}: the recorded prototype must be a live heap address" + ); + let marker = crate::object::js_object_get_field( + proto_after as *const crate::object::ObjectHeader, + 0, + ); + assert_eq!( + marker.bits(), + MARKER.to_bits(), + "{kind}: the recorded address must hold the prototype that was set" + ); + } +} diff --git a/crates/perry-runtime/src/gc/types.rs b/crates/perry-runtime/src/gc/types.rs index b81e84120c..835cb92074 100644 --- a/crates/perry-runtime/src/gc/types.rs +++ b/crates/perry-runtime/src/gc/types.rs @@ -917,10 +917,9 @@ pub(crate) fn gc_type_after_payload_move(obj_type: u8, old_user: usize, new_user GcMoveHookKind::None => {} GcMoveHookKind::ObjectOverflowFields => { crate::object::overflow_fields_owner_moved(old_user, new_user); - // #2820: migrate any recorded `Object.setPrototypeOf` entry for - // this ordinary object so getPrototypeOf/inherited reads still - // resolve after evacuation. - crate::object::prototype_chain::object_static_prototype_owner_moved(old_user, new_user); + // The residual `Object.setPrototypeOf` registry is rekeyed by the + // relocation funnel for every owner kind (`gc/layout/transfer.rs`), + // which runs before this hook on every move. crate::object::module_wrapper_owner_moved(old_user, new_user); } GcMoveHookKind::ClosureDynamicProps => { diff --git a/crates/perry-runtime/src/object/prototype_chain.rs b/crates/perry-runtime/src/object/prototype_chain.rs index c61ab25901..3376c3a07b 100644 --- a/crates/perry-runtime/src/object/prototype_chain.rs +++ b/crates/perry-runtime/src/object/prototype_chain.rs @@ -511,6 +511,34 @@ pub(crate) fn object_static_prototypes_maybe_nonempty() -> bool { OBJECT_PROTOTYPES_NONEMPTY.load(Ordering::Acquire) } +/// Can a cell of `obj_type` own an entry in the residual registry? +/// +/// The registry's population is every owner [`meta_capable_object`] turns +/// away, and the recorder is reached with whatever the caller holds: +/// `Object.setPrototypeOf` with an array, lazy JSON array, Map, Set, Error, +/// Promise, Date, RegExp or Temporal cell, `dyn_eval` with a closure. Only the +/// kinds that can never be a receiver stand outside it: strings and bigints +/// are primitives, meta records and compiled regex programs are internal. +/// `GC_TYPE_OBJECT` stays inside — its prototypes live in its meta record, and +/// the registry's obligations were always met for it too. +/// +/// The collector keys both of the registry's per-owner obligations on this +/// one predicate: the relocation rekey (`gc/layout/transfer.rs`) and the +/// value visit (`gc/layout_slot_visit.rs`). Both used to be wired to arrays +/// and ordinary objects by hand, so every other movable owner lost its +/// explicit prototype at its first relocation, and none had the prototype +/// value traced or rewritten. +#[inline] +pub(crate) fn residual_prototype_owner_type(obj_type: u8) -> bool { + !matches!( + obj_type, + crate::gc::GC_TYPE_STRING + | crate::gc::GC_TYPE_BIGINT + | crate::gc::GC_TYPE_OBJECT_META + | crate::gc::GC_TYPE_REGEX_PROGRAM + ) +} + /// Migrate the residual side-table entry when an owner's allocation address /// changes, either through moving GC or an `ArrayHeader` growth replacement. /// Mirrors `closure_dynamic_props_owner_moved`.