test(gc): handle hygiene in the #6910 root-word tests

changelog.d/6932-gc-root-word-test-handle-hygiene.md

@@ -0,0 +1,9 @@
+**Tests: handle hygiene in the #6910 root-word GC tests (follow-up to #6929).**
+
+A GC test that holds raw pointers across GC-capable calls is the same unrooted-handle bug class as #6655, inside tests whose job is validating GC correctness — an unsound GC test can pass for the wrong reason and mask the behavior it exists to pin. Audited all four tests landed in #6929.
+
+`mutable_root_mark_and_rewrite_accept_the_same_word_forms` held `mark_target` / `rewrite_target` as raw `usize` across two further arena allocations with no trigger suppression; evacuation moves arena objects, so either could have named from-space by the time the walks ran. Fixed with `GcTriggerThresholdTestGuard::suppress_automatic_triggers()` rather than a `RuntimeHandleScope` — deliberately, and the code says why: this test hand-builds a `ValidPointerSet` snapshot and hand-sets a forwarding address, neither of which survives a real collection however well the objects are rooted, so the hazard is the collection itself. `build_valid_pointer_set()` also moved after the last allocation (behavior-identical, but "snapshot the heap, then allocate into it" read wrong).
+
+`bare_address_in_shadow_slot_survives_a_real_collection` and `bare_address_in_global_root_survives_a_real_collection` were sound but brittle — `live` is `gc_malloc`-backed and the collector marks malloc objects in place without relocating (`CopyingPointerKind::Malloc => Some(addr)`). They now read the survivor back out of the root slot instead of reusing the pre-cycle raw pointer, which drops the dependence on that policy and additionally proves the slot itself was maintained across the collection. `decode_root_word_round_trips_each_representation` needed no change (pure bit math, no allocation).
+
+Detection power re-verified rather than assumed: with `mark_mutable_root_bits` temporarily reverted to the pre-#6910 NaN-box-only mark, the hardened tests still fail with the original signature. Stress matrix 9/9 green (`PERRY_GC_FORCE_EVACUATE=1`, `PERRY_GC_VERIFY_EVACUATION=1`, `PERRY_GEN_GC=0`, `PERRY_WRITE_BARRIERS=0` and combinations). Tests only — no runtime or codegen change.

crates/perry-runtime/src/gc/tests/root_words.rs

@@ -112,15 +112,30 @@ fn root_walk_rewrites(kind: ProbeSlot, bits: u64, valid_ptrs: &ValidPointerSet)
 fn mutable_root_mark_and_rewrite_accept_the_same_word_forms() {
     let _guard = GcTestIsolationGuard::new();
     let _scan = ConservativeScanDisabledGuard::new();
+    // This test hand-builds its own collector state: a `ValidPointerSet`
+    // snapshot and a hand-set forwarding address, then drives the mark and
+    // rewrite walks directly. A collection firing between any two lines below
+    // would invalidate all of that — and the three arena allocations are each
+    // a GC point, with the targets held as raw addresses across them.
+    //
+    // Note that rooting the targets would NOT be sufficient here, which is why
+    // this is trigger suppression rather than a `RuntimeHandleScope`: keeping
+    // the objects alive does not keep the pre-built `valid_ptrs` snapshot
+    // accurate, nor protect the forwarding bit this test sets by hand. The
+    // hazard is the collection itself, so the fix is to make the region
+    // collection-free.
+    let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
     clear_marks();
     clear_mark_seeds();
 
     // Two distinct targets: the mark probe must see an un-forwarded object,
     // while the rewrite probe needs a forwarded one.
     let mark_target = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize;
     let rewrite_target = crate::arena::arena_alloc_gc(40, 8, GC_TYPE_OBJECT) as usize;
-    let valid_ptrs = build_valid_pointer_set();
     let moved = crate::arena::arena_alloc_gc_old(40, 8, GC_TYPE_OBJECT) as usize;
+    // Snapshot AFTER every allocation, so the set describes the heap the walks
+    // below actually run against.
+    let valid_ptrs = build_valid_pointer_set();
     unsafe {
         set_forwarding_address(
             header_from_user_ptr(rewrite_target as *const u8),
@@ -223,14 +238,24 @@ fn bare_address_in_shadow_slot_survives_a_real_collection() {
         0,
         "the sweep must actually have run for this test to mean anything"
     );
+    // Read the survivor back out of the ROOT rather than reusing the raw
+    // pointer captured before the cycle. `live` is `gc_malloc`-backed and the
+    // collector marks malloc objects in place (`CopyingPointerKind::Malloc`
+    // returns the address unchanged), so the two agree today — but depending
+    // on that makes the test quietly wrong the day malloc objects become
+    // relocatable, and reloading is the stronger assertion anyway: it also
+    // proves the slot itself was maintained across the collection.
+    let survivor = js_shadow_slot_get(0);
+    assert_ne!(survivor, 0, "the shadow slot must still hold the survivor");
+    let survivor = survivor as *mut u8;
     assert!(
-        malloc_user_ptr_tracked(live),
+        malloc_user_ptr_tracked(survivor),
         "an object reachable only through a bare address in a shadow-stack \
          slot must be marked, not swept (#6910)"
     );
     unsafe {
         assert_eq!(
-            (*(live as *mut crate::closure::ClosureHeader)).type_tag,
+            (*(survivor as *mut crate::closure::ClosureHeader)).type_tag,
             crate::closure::CLOSURE_MAGIC,
             "surviving object must still be intact"
         );
@@ -270,14 +295,29 @@ fn bare_address_in_global_root_survives_a_real_collection() {
         0,
         "the sweep must actually have run for this test to mean anything"
     );
+    // Same discipline as the shadow-slot case: the root slot, not the raw
+    // pointer captured before the cycle, is the authority on where the object
+    // is now.
+    assert_ne!(
+        global_slot, 0,
+        "the global root must still hold the survivor"
+    );
     assert!(
-        malloc_user_ptr_tracked(live),
+        malloc_user_ptr_tracked(global_slot as *mut u8),
         "a bare address in a registered global root must still be marked"
     );
     assert_eq!(
         global_slot, live as u64,
-        "an unmoved target must leave the bare global slot untouched"
+        "a malloc-backed target is never relocated, so the bare global slot \
+         must come back byte-identical"
     );
+    unsafe {
+        assert_eq!(
+            (*(global_slot as *mut crate::closure::ClosureHeader)).type_tag,
+            crate::closure::CLOSURE_MAGIC,
+            "surviving object must still be intact"
+        );
+    }
 }
 
 /// The decoder is the single place that knows the word forms; assert the two