fix(gc): tombstone shape publish arms old_carrier — deleted receivers no longer lose their keys array under evacuating GC (#9200)

changelog.d/9200-tombstone-oldgen-keys-root.md

@@ -0,0 +1,74 @@
+### Fixed
+
+- **A tombstone delete on a promoted receiver no longer loses the object's
+  keys array under an evacuating collection** (#9200). On
+  `test_gap_repsel_pshape_tower_delete.ts` with `PERRY_OBJECT_TOMBSTONES=1
+  PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1`, a deleted receiver came
+  out of the churn with `Object.keys()` empty and a previously-live field
+  reading `NaN` — silently, exit 0, stderr empty. This is the corruption that
+  forced #9038's default-on tombstone deletes back to opt-in in #9212; the
+  default flip is deliberately NOT part of this change.
+
+  The mechanism, confirmed by tracing the descriptor lifecycle on the fixture
+  (not by code reading — one earlier confident hypothesis had already failed
+  against it):
+
+  1. The delete's clone fork published an intermediate descriptor for the
+     receiver's freshly cloned, nursery-young keys array and correctly armed
+     its `old_carrier` gate (`set_object_keys_array_with_live` has done that
+     since #8256 — the receiver had been promoted by the pre-delete churn, so
+     no minor would ever enumerate it again).
+  2. `publish_object_shape_holes` then minted the hole successor with
+     `old_carrier: false`, stamped it, and **retired the armed intermediate**
+     in its keys-address sweep (the #9064 descriptor-pile-up fix removes
+     every other id under the owned keys address). Net effect: an old
+     receiver stamped with an unarmed descriptor naming a young keys array.
+  3. The next minor walks a non-carrier record metadata-only
+     (`scan_shape_table_rekey_mut`) and never enumerates the old receiver,
+     so the keys array had **no root at all**: it was swept while live,
+     `prune_dead_shape_keys` dropped the descriptor as dead, and the
+     receiver's stamp dangled. `object_keys_array()` resolves through the
+     descriptor (#8047), so the receiver was shapeless from then on.
+     `PERRY_GC_VERIFY_EVACUATION` cannot see any of this — the only edge
+     lives in table metadata, and it is gone by sweep.
+
+  The fix is structural, not a patched call site:
+  `shapes::stamp_object_shape_id_with_carrier_note` is now the one post-birth
+  publication point for a ShapeId into a receiver's header word — it stamps
+  and, for any receiver outside the nursery, arms the descriptor's
+  old-carrier gate in the same breath, mirroring the note
+  `visit_gc_layout_slot_descriptors` makes at trace time. Every post-birth
+  publish routes through it: `publish_object_shape_holes` (the bug),
+  `try_update_stable_tombstone_shape`, `publish_object_shape_from`,
+  `stamp_object_shape`, `birth_stamp_object_shape`,
+  `transition_object_shape_semantics`, `transition_object_shape_to_class`,
+  the reserved-floor stamp, and the plain cached-shape install (which
+  hand-rolled the same note; the cache-carried install keeps its documented
+  skip — `cache_carrier` is the stronger registration). The arming
+  `set_object_keys_array_with_live` carried at its tail moved into the
+  funnel. Over-arming costs one rooted record for at most one full trace —
+  the epoch contract `old_carrier` already lives by (#8112).
+
+  Affected files:
+
+  - `crates/perry-runtime/src/object/shapes.rs` — the funnel, and the
+    stamp sites in it.
+  - `crates/perry-runtime/src/object/shapes_slot_list.rs` —
+    `publish_object_shape_holes` / `try_update_stable_tombstone_shape` stamp
+    through the funnel.
+  - `crates/perry-runtime/src/object/mod.rs` — tail arming folded into the
+    funnel.
+  - `crates/perry-runtime/src/object/reserved_floor.rs` — floor stamp through
+    the funnel.
+
+  Validation: the tower fixture flag-on is byte-identical to node 26.5.1
+  5/5 runs on `HEAP_LIMIT=8 + FORCE_EVACUATE`, 5/5 on the tighter
+  `HEAP_LIMIT=4`, and 5/5 with `VERIFY_EVACUATION` added; the trace shows the
+  hole descriptors as carriers with their keys arrays rewritten (evacuated
+  live) instead of pruned. Flag-off is byte-identical to node before and
+  after. `gc_repsel_matrix.sh --arms force_verify --filter tower_delete` with
+  the flag exported: PASS with the arm live (moved-objects 1/1, copy-minor
+  1/1). New witnesses: `test_gap_repsel_pshape_tombstone_oldgen_delete.ts`
+  (the minimized non-tower trigger, registered in the corpus) and a unit pin
+  (`tombstone_publish_on_untraced_receiver_arms_old_carrier`) that fails on
+  the unfixed runtime and passes with the funnel.

crates/perry-runtime/src/object/mod.rs

@@ -1819,16 +1819,12 @@ unsafe fn set_object_keys_array_with_live(
     // #8067/#8113: every visible ShapeId resolves to the exact rooted
     // ordered-keys/live-slot descriptor. Same-pointer appends are versioned
     // inside the helper.
-    let successor_shape_id =
-        shapes::publish_object_shape_from(obj, predecessor, keys_array, live_inline_slot_count);
-    // An old receiver is invisible to an ordinary minor root walk. Arm the
-    // shared descriptor edge at publication time so its keys array is copied
-    // during the same first minor, rather than relying on a later pass over a
-    // stale from-space address. Exact object-start validation deliberately
-    // rejects that stale address once the nursery block is reset (#8256).
-    if !crate::arena::pointer_in_nursery(obj as usize) {
-        shapes::note_old_generation_carrier(shapes::shape_descriptor_by_id(successor_shape_id));
-    }
+    // An old receiver is invisible to an ordinary minor root walk (#8256).
+    // #9200: the arming that used to live here moved into the stamp funnel
+    // (`shapes::stamp_object_shape_id_with_carrier_note`), which
+    // `publish_object_shape_from` and every other post-birth publish now
+    // route through — this call site no longer needs to remember the note.
+    shapes::publish_object_shape_from(obj, predecessor, keys_array, live_inline_slot_count);
 }
 
 #[inline]

crates/perry-runtime/src/object/reserved_floor.rs

@@ -122,7 +122,7 @@ unsafe fn stamp_reserved_floor_shape(
     let id = shapes::publish_shape_result(shapes::shape_descriptor_ensure_with_holes(
         keys, floor, live, generation, kind, floor,
     ));
-    (*obj).parent_class_id = id;
+    shapes::stamp_object_shape_id_with_carrier_note(obj, id);
     shapes::debug_assert_object_shape_parity_for_keys(obj, keys as *mut ArrayHeader);
     id
 }

crates/perry-runtime/src/object/shapes.rs

@@ -709,6 +709,42 @@ pub(crate) unsafe fn note_cache_carrier(descriptor: Option<ShapeDescriptor>) {
     (*record).cache_carrier = true;
 }
 
+/// The post-birth publication point for a ShapeId into a receiver's header
+/// word: stamp, then register the carrier duty the stamp just created.
+///
+/// #9200: a receiver a MINOR WILL NOT ENUMERATE (old-gen, `gc_malloc`'d
+/// large, immortal bootstrap) can be stamped with a descriptor whose keys
+/// array is nursery-young. The receiver is invisible to the next minor, so
+/// the descriptor's record is the ONLY path that can keep that keys array
+/// alive — and an unarmed record is walked metadata-only by
+/// `scan_shape_table_rekey_mut`. The keys array is then swept while live,
+/// `prune_dead_shape_keys` drops the descriptor as dead, and the receiver
+/// comes back shapeless: `Object.keys()` empty, every fixed-slot read
+/// `undefined`. The tombstone-delete publish hit exactly this: it minted a
+/// fresh unarmed descriptor for an already-promoted receiver and then
+/// retired the armed predecessor in its keys-address sweep.
+///
+/// Arming here — in the same breath as the header store — makes the
+/// old-carrier gate hold BY CONSTRUCTION for every publish routed through
+/// this funnel, instead of relying on each publish site to remember the
+/// note. The nursery test mirrors `visit_gc_layout_slot_descriptors`'
+/// carrier note: "not in the nursery", never "in old-gen".
+///
+/// Over-approximation is the designed cost model: the gate is sticky within
+/// an epoch and recomputed by every full trace, so arming a receiver that
+/// dies young roots one record for at most one full collection — exactly
+/// the generational contract (#8112).
+#[inline]
+pub(crate) unsafe fn stamp_object_shape_id_with_carrier_note(
+    obj: *mut crate::object::ObjectHeader,
+    id: u32,
+) {
+    (*obj).parent_class_id = id;
+    if !crate::arena::pointer_in_nursery(obj as usize) {
+        note_old_generation_carrier(shape_descriptor_by_id(id));
+    }
+}
+
 /// Clear every `cache_carrier` bit ahead of the post-full-trace recompute.
 pub(crate) fn clear_all_cache_carriers() {
     let mut inner = crate::state::state().shapes.inner.borrow_mut();
@@ -921,9 +957,14 @@ unsafe fn install_cached_object_shape_version_impl(
     // Match `set_object_keys_array_with_live`: representation feedback must be
     // invalidated while the predecessor stamp is still authoritative.
     super::mark_object_dynamic_shape_unknown(obj);
-    (*obj).parent_class_id = target_shape_id;
-    if !target_is_cache_carried && !crate::arena::pointer_in_nursery(obj as usize) {
-        note_old_generation_carrier(shape_descriptor_by_id(target_shape_id));
+    if target_is_cache_carried {
+        // `cache_carrier` already roots the target descriptor for the
+        // cache's lifetime — strictly stronger than the epoch-scoped
+        // old-carrier note, so this stamp deliberately skips the funnel's
+        // descriptor probe (see the function doc above).
+        (*obj).parent_class_id = target_shape_id;
+    } else {
+        stamp_object_shape_id_with_carrier_note(obj, target_shape_id);
     }
 
     #[cfg(debug_assertions)]
@@ -976,7 +1017,7 @@ pub(crate) unsafe fn stamp_object_shape(
         crate::array::clear_array_subclass_named_prefix_token(obj);
         let id = shape_descriptor_ensure(keys, key_count, live_inline_slot_count)
             .unwrap_or_else(|error| shape_descriptor_error_abort(error));
-        (*obj).parent_class_id = id;
+        stamp_object_shape_id_with_carrier_note(obj, id);
         debug_assert_object_shape_parity(obj);
         return id;
     };
@@ -997,7 +1038,7 @@ pub(crate) unsafe fn stamp_object_shape(
         // an actual structural identity change.
         crate::array::clear_array_subclass_named_prefix_token(obj);
     }
-    (*obj).parent_class_id = id;
+    stamp_object_shape_id_with_carrier_note(obj, id);
     debug_assert_object_shape_parity(obj);
     id
 }
@@ -1044,7 +1085,7 @@ pub(crate) unsafe fn birth_stamp_object_shape(
                 live_inline_slot_count,
             );
     if supplied_id_is_local {
-        (*obj).parent_class_id = runtime_shape_id;
+        stamp_object_shape_id_with_carrier_note(obj, runtime_shape_id);
         debug_assert_object_shape_parity(obj);
     } else {
         // `current` was just published from the newborn's explicit keys edge
@@ -1290,7 +1331,7 @@ pub(crate) unsafe fn publish_object_shape_from(
         object_kind,
         hole_count,
     ));
-    (*obj).parent_class_id = id;
+    stamp_object_shape_id_with_carrier_note(obj, id);
     debug_assert_object_shape_parity_for_keys(obj, keys);
     id
 }
@@ -1323,7 +1364,7 @@ pub(crate) unsafe fn transition_object_shape_semantics(
         generation,
         current.object_kind,
     ));
-    (*obj).parent_class_id = id;
+    stamp_object_shape_id_with_carrier_note(obj, id);
     debug_assert_object_shape_parity(obj);
     id
 }
@@ -1372,7 +1413,7 @@ pub(crate) unsafe fn transition_object_shape_to_class(
         current.semantic_generation,
         ShapeObjectKind::Class,
     ));
-    (*obj).parent_class_id = id;
+    stamp_object_shape_id_with_carrier_note(obj, id);
     debug_assert_object_shape_parity(obj);
     id
 }

crates/perry-runtime/src/object/shapes_slot_list.rs

@@ -420,7 +420,11 @@ pub(crate) unsafe fn rekey_stable_tombstone_shape_after_squeeze(
     inner.descriptors.insert(new_id, record);
     drop(inner);
 
-    (*obj).parent_class_id = new_id;
+    // #9200: the funnel re-arms the preserved record for a non-nursery
+    // receiver. The record kept its flags across the id move, but a receiver
+    // promoted since the last trace has no other arming opportunity before
+    // the next minor.
+    super::stamp_object_shape_id_with_carrier_note(obj, new_id);
     super::debug_assert_object_shape_parity(obj);
     Some(new_id)
 }
@@ -464,7 +468,15 @@ pub(crate) unsafe fn publish_object_shape_holes(
         current.object_kind,
         hole_count,
     ));
-    (*obj).parent_class_id = id;
+    // #9200 THE FIX: stamp through the carrier-note funnel. This publish is
+    // the one that minted a fresh (old_carrier=false) descriptor for an
+    // already-promoted receiver and then RETIRED the armed predecessor in the
+    // keys-address sweep below — leaving the receiver's nursery-young keys
+    // array with no root a minor can see. The evacuating minor then swept the
+    // keys array while live, `prune_dead_shape_keys` dropped this descriptor,
+    // and the receiver came back shapeless (`Object.keys()` empty, fixed-slot
+    // reads undefined — the #9200 gap fixture's exact wrong answer).
+    super::stamp_object_shape_id_with_carrier_note(obj, id);
     // Retire the predecessor. Its keys array is OWNED (the tombstone path is
     // gated on that), so this object is the only carrier of the old stamp and
     // the id becomes unreachable the moment the header word above is written:

crates/perry-runtime/src/object/tombstone_tests.rs

@@ -346,3 +346,90 @@ fn small_churn_first_delete_forks_owned_tombstone() {
         );
     }
 }
+
+/// #9200 pin: a flag-on tombstone publish onto a receiver a minor will not
+/// enumerate must arm the successor descriptor's old-carrier gate in the same
+/// breath as the stamp.
+///
+/// The failure this pins, traced on the gap fixture
+/// (`test_gap_repsel_pshape_tower_delete.ts` under `PERRY_OBJECT_TOMBSTONES=1
+/// PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1`):
+/// `publish_object_shape_holes` minted a fresh (`old_carrier = false`)
+/// descriptor for an already-promoted receiver, stamped it, and then retired
+/// the ARMED predecessor in its keys-address sweep. The receiver is invisible
+/// to a minor, and a non-carrier record is walked metadata-only
+/// (`scan_shape_table_rekey_mut`), so the nursery-young owned keys array had
+/// no root at all: the next evacuating minor swept it while live,
+/// `prune_dead_shape_keys` dropped the descriptor, and the receiver came back
+/// shapeless — `Object.keys()` empty, fixed-slot reads `undefined`, silently.
+///
+/// A LARGE allocation is born outside the nursery through the public
+/// allocator — the same "no minor ever enumerates me" population the gap
+/// fixture reaches by churn-promotion, with no synthetic promotion machinery.
+#[test]
+fn tombstone_publish_on_untraced_receiver_arms_old_carrier() {
+    super::delete_rest::test_set_tombstone_deletes(Some(true));
+    let _restore = scopeguard_tombstone_flag();
+    let _global = crate::gc::global_side_table_test_lock();
+    unsafe {
+        // Born OLD through the arena's old-gen allocator — the same "no minor
+        // ever enumerates me" population the gap fixture reaches by
+        // churn-promotion. (`js_object_alloc` routes through the nursery, so
+        // the public allocator cannot produce this receiver in a unit test.)
+        // Initialization mirrors `js_object_alloc_with_parent` exactly.
+        let slots = 24usize;
+        let header_size = std::mem::size_of::<crate::object::ObjectHeader>();
+        let obj = crate::arena::arena_alloc_gc_old(
+            header_size + slots * std::mem::size_of::<u64>(),
+            8,
+            crate::gc::GC_TYPE_OBJECT,
+        ) as *mut crate::object::ObjectHeader;
+        (*obj).class_id = 0;
+        (*obj).parent_class_id = 0;
+        (*obj).meta = std::ptr::null_mut();
+        let fields = (obj as *mut u8).add(header_size) as *mut u64;
+        for i in 0..slots {
+            // GC_STORE_AUDIT(INIT): fresh unpublished storage, pointer-free.
+            std::ptr::write(fields.add(i), crate::value::TAG_UNDEFINED);
+        }
+        crate::gc::layout_init_pointer_free(obj as *mut u8);
+        super::shapes::birth_publish_object_shape(obj, slots as u32);
+        assert!(
+            !crate::arena::pointer_in_nursery(obj as usize),
+            "precondition: the receiver must be born outside the nursery"
+        );
+        for i in 0..20 {
+            let name = format!("key_number_{i:02}");
+            let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32);
+            js_object_set_field_by_name(obj, key, i as f64);
+        }
+        // First delete: the keys array is transition-cache-shared and 20 keys
+        // wide, so this clones + compacts (ownership transfer, no tombstone).
+        let first = crate::string::js_string_from_bytes(b"key_number_11".as_ptr(), 13)
+            as *const crate::StringHeader;
+        assert_eq!(super::delete_rest::js_object_delete_field(obj, first), 1);
+        assert_eq!(super::shapes::object_shape_hole_count(obj), 0);
+        // Second delete: owned keys array, 19 >= 16 keys, holes below the
+        // squeeze threshold — the O(1) tombstone lane and its
+        // `publish_object_shape_holes` mint-and-retire publish.
+        let second = crate::string::js_string_from_bytes(b"key_number_03".as_ptr(), 13)
+            as *const crate::StringHeader;
+        assert_eq!(
+            super::delete_rest::js_object_delete_field(obj, second),
+            1
+        );
+        let descriptor = super::shapes::object_shape_descriptor(obj)
+            .expect("the tombstone publish must leave a resolvable descriptor");
+        assert_eq!(
+            descriptor.hole_count, 1,
+            "the second delete must take the tombstone lane"
+        );
+        assert!(
+            descriptor.old_carrier,
+            "#9200: the tombstone publish stamped a fresh descriptor onto a \
+             receiver no minor enumerates without arming the old-carrier \
+             gate; its young keys array has no root the shape-table scan can \
+             see, and the next evacuating minor sweeps it while live"
+        );
+    }
+}

test-files/probe9200.ts

@@ -0,0 +1,38 @@
+// #9200 discriminator. The original fixture reads rows only through the
+// cross-module dispatch tower (`pickAll`), so a wrong answer cannot be
+// attributed to the DATA vs the TOWER. This reads every row BOTH ways at every
+// stage: directly (`r.a`, `r.c`, Object.keys) and through the tower.
+//
+// If the direct reads stay correct while the tower reads go NaN, the object is
+// intact and the tower's inline keys-pointer guard is wrongly passing.
+// If the direct reads ALSO break, the object itself lost its keys array.
+import { Row, pickAll } from "./_helpers/repsel_pshape_tower_rows.ts";
+
+function churn(n: number): number {
+    const sink: { x: number }[] = [];
+    for (let i = 0; i < n; i++) sink.push({ x: i });
+    return sink.length;
+}
+
+const rows: Row[] = [];
+for (let i = 0; i < 4; i++) rows.push(new Row(i + 1, (i + 1) * 10, (i + 1) * 3));
+
+function report(tag: string): void {
+    const direct: string[] = [];
+    for (let i = 0; i < rows.length; i++) {
+        const r: any = rows[i];
+        direct.push(r.a + "/" + r.c + "[" + Object.keys(r).join("") + "]");
+    }
+    console.log(tag + " direct: " + direct.join(" "));
+    console.log(tag + " tower : " + pickAll(rows).join(","));
+}
+
+report("S0");
+delete (rows[1] as any).b;
+report("S1-after-delete-row1");
+churn(200_000);
+report("S2-after-churn");
+delete (rows[3] as any).a;
+report("S3-after-delete-row3");
+churn(200_000);
+report("S4-after-churn2");