gc: whole-heap from-space scan, independent of the rewrite pass (#7035)

changelog.d/7041-gc-whole-heap-fromspace-scan.md

@@ -0,0 +1,44 @@
+### GC: whole-heap from-space scan (`PERRY_GC_FROMSPACE_SCAN`)
+
+Adds a debug-only verification pass that answers, after the copying minor's
+rewrite pass and before from-space is reset:
+
+> does any live object OUTSIDE from-space still contain a word that decodes to a
+> from-space address?
+
+It walks the whole heap — the arena census plus the malloc-tracked registry —
+and decodes every payload word through `decode_root_word`, the decoder the mark
+and rewrite paths already share (#6910). Objects that are themselves in
+from-space are skipped: a dead object legitimately still points at its dead
+peers.
+
+**Why this is not redundant with `PERRY_GC_VERIFY_EVACUATION` (#7035).** The
+existing verifier walks the same surfaces the *rewrite pass* walks, so it can
+only check that the rewrite pass agreed with itself; a holder the rewrite pass
+never enumerated is invisible to both. On the #7022 reproducer the verifier
+reports clean while the program dies of malloc-heap corruption. This pass does
+not depend on any root enumeration, and on that same reproducer it names the
+offending holders on the second collection.
+
+Each offender is classified along two axes that have different fixes:
+
+- **missing rewrite** (target carries `GC_FLAG_FORWARDED` — it moved and this
+  reference was not updated) vs **dangling** (target was never evacuated and is
+  about to be recycled);
+- remembered-set coverage of the slot's page — `never_dirty` (a store path
+  skipped the write barrier), `lost_dirty` (the edge was recorded and then
+  lost), `dirty_but_missed` (the page is dirty and the slot was still missed).
+  This reuses the existing `EVER_DIRTY_OLD_PAGES` tracking, which is now enabled
+  by this scan as well as by `PERRY_GC_VERIFY_EVACUATION`.
+
+`PERRY_GC_FROMSPACE_SCAN_ABORT=1` additionally aborts on the first offender so a
+crash report captures the offending cycle rather than the downstream corruption.
+
+O(live heap) per collection, so it is strictly opt-in and off by default.
+
+Three unit tests cover both directions — that the scan finds a planted
+un-rewritten old→young reference, that it stops reporting once the reference is
+removed, that it separates dangling from missing-rewrite, and that it ignores
+references held *by* from-space objects. Teeth verified by sabotage: blinding
+the from-space predicate turns two of the three red with the exact
+"planted=0" signature.

crates/perry-runtime/src/gc/barrier.rs

@@ -1296,7 +1296,10 @@ thread_local! {
 fn ever_dirty_tracking_enabled() -> bool {
     use std::sync::OnceLock;
     static CACHED: OnceLock<bool> = OnceLock::new();
-    *CACHED.get_or_init(|| std::env::var_os("PERRY_GC_VERIFY_EVACUATION").is_some())
+    *CACHED.get_or_init(|| {
+        std::env::var_os("PERRY_GC_VERIFY_EVACUATION").is_some()
+            || super::fromspace_scan::fromspace_scan_enabled()
+    })
 }
 
 fn ever_dirty_note(page: usize) {
@@ -1859,3 +1862,17 @@ pub fn remembered_set_clear() {
     let mut state = RememberedSetClearState::new();
     while !state.step(usize::MAX) {}
 }
+
+/// #7035: is `addr`'s old page currently in the remembered set?
+pub(super) fn dirty_now_for_addr(addr: usize) -> bool {
+    DIRTY_OLD_PAGES.with(|s| {
+        s.borrow()
+            .contains(&crate::arena::generation_page_for_addr(addr))
+    })
+}
+
+/// #7035: was `addr`'s old page EVER dirtied? Distinguishes "barrier never ran"
+/// from "edge was recorded then lost".
+pub(super) fn ever_dirty_for_addr(addr: usize) -> bool {
+    ever_dirty_old_page(crate::arena::generation_page_for_addr(addr))
+}

crates/perry-runtime/src/gc/copying.rs

@@ -1192,6 +1192,11 @@ pub(super) fn gc_collect_minor_copying_fast_path_with_eligibility(
         super::verify::verify_marked_heap_report_nonfatal("copying-minor");
     }
 
+    // #7035: whole-heap from-space scan. MUST run here — after the rewrite
+    // pass, before from-space is reset — and it is deliberately independent of
+    // the root enumeration the rewrite pass and the evacuation verifier share.
+    super::fromspace_scan::run_fromspace_scan();
+
     crate::promise::cleanup_copied_minor_promise_contexts_for_gc();
     finalize_dead_copied_minor_from_space_side_allocations();
     let reset = crate::arena::copying_reset_from_spaces_and_flip();

crates/perry-runtime/src/gc/fromspace_scan.rs

@@ -0,0 +1,305 @@
+//! Whole-heap from-space scan (#7035).
+//!
+//! # Why this exists
+//!
+//! `PERRY_GC_VERIFY_EVACUATION` walks the same surfaces the *rewrite pass*
+//! walks — `visit_shadow_stack_root_slots`, `visit_global_root_slots`, and the
+//! registered side-table scanners. Both derive from one enumeration, so the
+//! verifier can only check that the rewrite pass agreed with itself: a holder
+//! the rewrite pass never knew about is invisible to the verifier too. On the
+//! #7022 reproducer the verifier reports clean while the program dies of malloc
+//! heap corruption, which is exactly that blind spot.
+//!
+//! This pass takes the opposite approach and asks a question that does not
+//! depend on any root enumeration:
+//!
+//! > After the rewrite pass, does any live object OUTSIDE from-space still
+//! > contain a word that decodes to a from-space address?
+//!
+//! It answers it by walking the whole heap — the arena census (`Address`-order
+//! `ArenaObjectCursor`, covering Eden / both survivor semispaces / old / long-
+//! lived) plus the malloc-tracked registry — and decoding every payload word
+//! through `decode_root_word`, the same decoder the mark and rewrite paths
+//! share (#6910). Objects that are themselves in from-space are skipped: they
+//! are about to be reclaimed, and a dead object legitimately still points at
+//! its dead peers.
+//!
+//! A word whose target carries `GC_FLAG_FORWARDED` is an unambiguous **missing
+//! rewrite**: the object moved this cycle and this reference was not updated.
+//! A word pointing at a non-forwarded from-space object is a **dangling**
+//! reference — the target was not evacuated at all, so it is about to be
+//! recycled underneath this holder.
+//!
+//! # Cost and placement
+//!
+//! O(live heap) per collection, so it is strictly a debug instrument behind
+//! `PERRY_GC_FROMSPACE_SCAN=1`. It must run after the rewrite pass and
+//! **before** `copying_reset_from_spaces_and_flip`, while from-space is still
+//! intact and page-registered — the same window the evacuation verifier uses.
+//!
+//! `PERRY_GC_FROMSPACE_SCAN_ABORT=1` additionally aborts on the first offender
+//! so a debugger/crash report captures the offending cycle rather than the
+//! downstream corruption.
+
+use super::*;
+
+/// One offending word found by the scan.
+#[derive(Clone, Copy)]
+pub(crate) struct FromSpaceRef {
+    /// Header of the object that CONTAINS the stale word.
+    pub(super) owner_header: usize,
+    pub(super) owner_obj_type: u8,
+    pub(super) owner_space: crate::arena::HeapSpace,
+    /// Byte offset of the stale word within the owner's payload.
+    pub(super) slot_offset: usize,
+    /// The from-space address the word decodes to.
+    pub(super) target: usize,
+    pub(super) target_space: crate::arena::HeapSpace,
+    /// True when the target carries `GC_FLAG_FORWARDED` — i.e. it MOVED and
+    /// this reference was simply not rewritten.
+    pub(super) target_forwarded: bool,
+    /// True when the word was NaN-boxed, false when it was a bare address.
+    pub(super) nanboxed: bool,
+    /// Remembered-set coverage of THIS SLOT's page, the decisive split:
+    /// `ever_dirty == false` -> a store path skipped the write barrier;
+    /// `ever_dirty && !dirty_now` -> the edge was recorded and then LOST.
+    pub(super) slot_dirty_now: bool,
+    pub(super) slot_ever_dirty: bool,
+    /// Owner's raw `gc_flags`, reported uninterpreted. NOTE: during a minor,
+    /// old-gen parents are NOT marked even when their slots are scanned through
+    /// the remembered set, so `GC_FLAG_MARKED` here does not mean "was scanned".
+    pub(super) owner_flags: u8,
+}
+
+#[derive(Default)]
+pub(crate) struct FromSpaceScanReport {
+    pub(crate) objects_scanned: usize,
+    pub(crate) words_scanned: usize,
+    pub(crate) missing_rewrites: usize,
+    pub(crate) dangling: usize,
+    /// Offending slots whose page was NEVER dirtied -> a store path skipped the
+    /// write barrier entirely.
+    pub(crate) never_dirty: usize,
+    /// Offending slots whose page was dirtied at some point but is not dirty
+    /// now -> the edge was recorded and then lost by a clear/restore gap.
+    pub(crate) lost_dirty: usize,
+    /// Offending slots whose page IS currently dirty -> the remembered set had
+    /// it and the scan still missed the slot.
+    pub(crate) dirty_but_missed: usize,
+    pub(crate) distinct_owners: crate::fast_hash::PtrHashSet<usize>,
+    pub(crate) samples: Vec<FromSpaceRef>,
+}
+
+const MAX_SAMPLES: usize = 32;
+
+pub(super) fn fromspace_scan_enabled() -> bool {
+    use std::sync::OnceLock;
+    static CACHED: OnceLock<bool> = OnceLock::new();
+    *CACHED.get_or_init(|| {
+        matches!(
+            std::env::var("PERRY_GC_FROMSPACE_SCAN").as_deref(),
+            Ok("1") | Ok("on") | Ok("true")
+        )
+    })
+}
+
+fn fromspace_scan_abort() -> bool {
+    use std::sync::OnceLock;
+    static CACHED: OnceLock<bool> = OnceLock::new();
+    *CACHED.get_or_init(|| {
+        matches!(
+            std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT").as_deref(),
+            Ok("1") | Ok("on") | Ok("true")
+        )
+    })
+}
+
+/// Is `space` a from-space for the copying minor that just ran? Eden always is;
+/// so is whichever survivor semispace was active going INTO the cycle (the
+/// flip happens later, inside `copying_reset_from_spaces_and_flip`).
+#[inline]
+fn is_from_space(space: crate::arena::HeapSpace) -> bool {
+    space == crate::arena::HeapSpace::NurseryEden || space == crate::arena::active_survivor_space()
+}
+
+/// Scan one object's payload for from-space references.
+///
+/// # Safety
+/// `header` must point at a live, walkable `GcHeader` whose `size` field covers
+/// the allocation, which is what both the arena cursor and the malloc registry
+/// guarantee.
+unsafe fn scan_object(header: *mut GcHeader, report: &mut FromSpaceScanReport) {
+    let total = (*header).size as usize;
+    if total <= GC_HEADER_SIZE {
+        return;
+    }
+    let user = (header as *mut u8).add(GC_HEADER_SIZE);
+    let owner_space = crate::arena::classify_heap_space(user as usize);
+    // A from-space object is about to be reclaimed; it may legitimately still
+    // reference its dead peers. Only holders that SURVIVE the cycle matter.
+    if is_from_space(owner_space) {
+        return;
+    }
+    report.objects_scanned += 1;
+
+    let payload_words = (total - GC_HEADER_SIZE) / 8;
+    let words = user as *const u64;
+    for i in 0..payload_words {
+        let bits = *words.add(i);
+        report.words_scanned += 1;
+        let Some(word) = super::root_words::decode_root_word(bits) else {
+            continue;
+        };
+        let target = word.addr();
+        let target_space = crate::arena::classify_heap_space(target);
+        if !is_from_space(target_space) {
+            continue;
+        }
+        // The target is in from-space. Did it move (missing rewrite) or was it
+        // never evacuated (dangling)?
+        let target_forwarded = if target >= GC_HEADER_SIZE {
+            let th = (target - GC_HEADER_SIZE) as *const GcHeader;
+            (*th).gc_flags & GC_FLAG_FORWARDED != 0
+        } else {
+            false
+        };
+        if target_forwarded {
+            report.missing_rewrites += 1;
+        } else {
+            report.dangling += 1;
+        }
+        report.distinct_owners.insert(header as usize);
+        let slot_addr = words.add(i) as usize;
+        let dirty_now = super::barrier::dirty_now_for_addr(slot_addr);
+        let ever_dirty = super::barrier::ever_dirty_for_addr(slot_addr);
+        if !ever_dirty {
+            report.never_dirty += 1;
+        } else if !dirty_now {
+            report.lost_dirty += 1;
+        } else {
+            report.dirty_but_missed += 1;
+        }
+        if report.samples.len() < MAX_SAMPLES {
+            report.samples.push(FromSpaceRef {
+                owner_header: header as usize,
+                owner_obj_type: (*header).obj_type,
+                owner_space,
+                slot_offset: i * 8,
+                target,
+                target_space,
+                target_forwarded,
+                nanboxed: matches!(word, super::root_words::RootWord::Nanboxed { .. }),
+                slot_dirty_now: super::barrier::dirty_now_for_addr(words.add(i) as usize),
+                slot_ever_dirty: super::barrier::ever_dirty_for_addr(words.add(i) as usize),
+                owner_flags: (*header).gc_flags,
+            });
+        }
+        if fromspace_scan_abort() {
+            report_and_abort(report);
+        }
+    }
+}
+
+/// Walk the whole heap and report every surviving reference into from-space.
+pub(crate) fn scan_heap_for_fromspace_refs() -> FromSpaceScanReport {
+    let mut report = FromSpaceScanReport::default();
+
+    // --- arena census (Eden / survivors / old / longlived) ------------------
+    let mut builder =
+        crate::arena::ArenaObjectCursorBuilder::new(crate::arena::ArenaWalkOrder::Address);
+    let mut cursor = loop {
+        let mut budget = usize::MAX;
+        if let Some(cursor) = builder.step(&mut budget) {
+            break cursor;
+        }
+    };
+    loop {
+        let mut budget = usize::MAX;
+        match cursor.next_budgeted(&mut budget) {
+            Some((header_ptr, _block_idx)) => unsafe {
+                scan_object(header_ptr as *mut GcHeader, &mut report);
+            },
+            None => {
+                if cursor.is_finished() {
+                    break;
+                }
+            }
+        }
+    }
+
+    // --- malloc-tracked objects --------------------------------------------
+    let malloc_headers: Vec<*mut GcHeader> =
+        MALLOC_STATE.with(|s| s.borrow().objects.iter().copied().collect());
+    for header in malloc_headers {
+        if header.is_null() {
+            continue;
+        }
+        unsafe {
+            scan_object(header, &mut report);
+        }
+    }
+
+    report
+}
+
+fn describe(r: &FromSpaceRef) -> String {
+    format!(
+        "  owner={:#x} type={} space={:?} +{} {} -> {:#x} ({:?}) {} [slot dirty_now={} ever_dirty={} owner_flags={:#x} marked={}]",
+        r.owner_header,
+        r.owner_obj_type,
+        r.owner_space,
+        r.slot_offset,
+        if r.nanboxed { "nanbox" } else { "bare" },
+        r.target,
+        r.target_space,
+        if r.target_forwarded {
+            "MISSING-REWRITE (target moved)"
+        } else {
+            "DANGLING (target not evacuated)"
+        },
+        r.slot_dirty_now,
+        r.slot_ever_dirty,
+        r.owner_flags,
+        r.owner_flags & GC_FLAG_MARKED != 0
+    )
+}
+
+fn report_and_abort(report: &FromSpaceScanReport) -> ! {
+    emit_report(report, "abort");
+    panic!(
+        "gc from-space scan: {} missing rewrite(s), {} dangling reference(s) survived the rewrite pass",
+        report.missing_rewrites, report.dangling
+    );
+}
+
+pub(super) fn emit_report(report: &FromSpaceScanReport, phase: &str) {
+    eprintln!(
+        "[gc-fromspace-scan {}] objects={} words={} missing_rewrites={} dangling={} owners={} | never_dirty={} lost_dirty={} dirty_but_missed={}",
+        phase,
+        report.objects_scanned,
+        report.words_scanned,
+        report.missing_rewrites,
+        report.dangling,
+        report.distinct_owners.len(),
+        report.never_dirty,
+        report.lost_dirty,
+        report.dirty_but_missed
+    );
+    for sample in &report.samples {
+        eprintln!("{}", describe(sample));
+    }
+}
+
+/// Entry point called from the copying minor, after the rewrite pass and before
+/// `copying_reset_from_spaces_and_flip`.
+pub(super) fn run_fromspace_scan() {
+    if !fromspace_scan_enabled() {
+        return;
+    }
+    let report = scan_heap_for_fromspace_refs();
+    if report.missing_rewrites > 0 || report.dangling > 0 {
+        emit_report(&report, "OFFENDERS");
+    } else {
+        emit_report(&report, "clean");
+    }
+}