feat(gc): from-space protection, GC zeal, and verify-roots gap closures for #7154

[jdalton]

When Perry's code generator forgets to tell the garbage collector about a value that is still in use, the compiled program keeps pointing at memory the collector has already recycled. The maddening part is that nothing goes wrong at that moment. The nursery resets, the bump allocator hands the same bytes to the next allocation, and the stale pointer now reads a perfectly valid — but completely unrelated — object. The program carries on and dies one or more collection cycles later, in a different function, as TypeError: value is not a function. Ten rounds of investigation on #7154 went into that gap between cause and symptom.

This PR adds three opt-in instruments that collapse the gap. On the known-broken reproducer, the strongest combination turns "wrong answer on 9 of 400 iterations, exit code 0" into a hard fault on iteration 1, at the instruction that misuses the stale pointer, with the holder still live on the stack. On the known-fixed build the same combination stays clean, so it is a detector rather than a noise generator.

All three are default-off and inert when off; no collector behaviour changes on a normal build. Nobody compiling or running a Perry program sees any difference unless they set one of these environment variables.

Instrument 1: from-space protection — PERRY_GC_PROTECT_FROMSPACE makes the stale read fault at the faulting instruction

New crates/perry-runtime/src/arena/quarantine.rs. After an evacuating minor collection, the retired Eden and active-survivor blocks are detached from the arena into a bounded quarantine ring (each vacated slot becomes a data = null tombstone, the exact shape C4b-δ dealloc already leaves, so block-index semantics — active_survivor_block_index_range, block_has_live, every walker — are untouched), poison-filled, and mprotect(PROT_NONE)d over their page-aligned interior.

A stale dereference now faults immediately:

[gc-fromspace-protect] FAULT: signal 10 at 0x377ddea0178
  This address is RETIRED FROM-SPACE. The evacuating minor moved or
  freed the object here and the holder kept the pre-collection address.
  block=0x377ddea0000 +376 retired_bytes=664 retired_by_minor=#0
  last-known object: user_ptr=0x377ddea00f0 obj_type=1 size=144
  The faulting instruction IS the stale use. Backtrace:
  ...

The reporter formats into a stack buffer and write(2)s it (no format! on the signal path), reads the census with try_lock so it can never block, uses the async-signal-safe backtrace / backtrace_symbols_fd pair, then restores SIG_DFL and returns so the instruction re-faults — a core file, debugger or crash reporter still sees the real site. The obj_type and size come from a census taken by walking each block before poisoning.

• =poison selects poison without mprotect. That is also what the (at most two) sub-page block edges mprotect cannot cover always get — arena blocks are alloc'd at 16-byte alignment. Protected and poisoned byte totals are counted separately, so a run can never claim page protection it did not get.
• PERRY_GC_PROTECT_FROMSPACE_DEPTH (default 4, 0 clamped to 1) bounds memory. Expired sets are restored to PROT_READ|PROT_WRITE and recycled back into Eden rather than freed, so the quarantine is a ring, footprint is depth × from-space bytes, and nothing that was ever mprotected is handed to dealloc.

Instrument 2: GC zeal — PERRY_GC_ZEAL=1 moves the value on its first exposure, not whenever pressure happens to line up

Forces an evacuating minor at every GC safepoint (loop back-edge polls and the outermost microtask-pump boundary) instead of only when pressure is due, so an unrooted value moves on its first exposure rather than whenever an unrelated allocation burst happens to line up. Modelled on V8 --stress-scavenge / SpiderMonkey gcZeal.

Zeal implies gc_force_evacuate_enabled() — a zealous minor that left survivors in place would move nothing and could not surface the bug, i.e. a gate that cannot fail — while still losing to an explicit PERRY_GEN_GC_EVACUATE=0, so the two knobs can never silently disagree about whether objects move.

There is deliberately no level 2. An "every allocation" zeal would go through the alloc-point arm, which takes ManualGcScanGuard::force_full_scan; a forced conservative scan makes the copying minor ineligible (CopiedMinorFallbackReason::ConservativeStack), so level 2 would run many non-moving minors and move nothing. That is exactly the PERRY_GC_FORCE_EVACUATE inertness this project already paid for in #6942 / #6946, so the level does not exist rather than existing untrustworthy.

Instrument 3: verify-roots gap closures — the abort switch was completely inert on its own

PERRY_GC_FROMSPACE_SCAN_ABORT=1 now implies PERRY_GC_FROMSPACE_SCAN=1. On its own it was completely inert: run_fromspace_scan returned at the enablement gate, the scan never ran, there was nothing to abort, and the run reported success — an investigator reaching for the abort switch mid-hunt got a green run and no scan.

The abort path now also prints a collector backtrace, and every offender sample reports the target's obj_type alongside the owner's (the field that separates a dead closure, 4, from a dead object, 2, when triaging value is not a function).

Validation on the known-broken / known-fixed pair — detection moves from iteration 9-of-400-and-silent to iteration 1-and-fatal

Ran. main is currently without #7192's lower_new_impl_inner fix, so it is the known-broken build. Same binary, same program (#7192's test_gap_gc_new_instance_rooting.ts), compiled and run with PERRY_GC_MOVING_LOOP_POLLS=1:

arm	result	detection
broken, no instruments	bad 9 / 400, exit 0	none — silent wrong answer after all 400 iterations
broken, ZEAL=1	bad 400 / 400, exit 0	100% of iterations instead of 2.25%, still silent
broken, ZEAL=1 PROTECT_FROMSPACE=1 DEPTH=800	SIGBUS, exit 138	iteration 1 of 400, first use of an address retired by collection #0
fixed (#7192 applied), no instruments	bad 0	—
fixed, ZEAL=1	bad 0	—
fixed, ZEAL=1 PROTECT_FROMSPACE=1 DEPTH=800	bad 0, exit 0	clean — no false positive

Depth matters and is now documented. The default 4 misses this reproducer silently: under zeal the constructor body crosses 600 back-edge polls, so the caller's stale register is 600 retirements old by the time js_ctor_return_override publishes it. Rule of thumb, now in CLAUDE.md and the memory-model page: depth >= the number of safepoints the suspect value survives.

Why this is not an instrument artifact. The quarantine covers exactly the bytes the ordinary reset declares reusable — every block of Eden and the active survivor, nothing more. Pinned young objects make the copying fast path ineligible (CopiedMinorFallbackReason::PinnedYoung*), so from-space is entirely dead when this runs. And in poison mode a program that reads retired from-space produces a wrong answer rather than a fault, which is a direct positive control: it proves the bytes were genuinely being consumed.

What each knob does NOT gate — two documented ways a protected run can be vacuously green

CLAUDE.md and docs/src/internals/memory-model.md gain a table stating what each knob gates exactly and — as importantly — what it does not, because prior rounds were misled by knobs whose real effect differed from their name.

Knob	The trap
PERRY_GC_PROTECT_FROMSPACE	Gates only the copying minor's from-space reset — not arena_reset_empty_blocks, not the full mark-sweep, not old-gen defrag, not the malloc sweep. A run with zero copying minors protects nothing; check for the [gc-fromspace-protect] retired_set=#N line under PERRY_GC_DIAG=1.
PERRY_GC_ZEAL	Cannot emit loop polls codegen never produced (compile-time PERRY_GC_MOVING_LOOP_POLLS=1, default off since #7161). crate::gc::zeal_forced_collections() is the live-subject counter.

Also noted: quarantined bytes leave ARENA_TOTAL_BYTES, so a protected run under-reports arena bytes and RSS is genuinely higher. Do not benchmark under it.

Test coverage — every knob asserts both states, and the detector is sabotage-tested rather than merely exercised

crates/perry-runtime/src/gc/tests/fromspace_protect.rs, 10 tests. Every knob asserts both states as the kill-policy requires — the OFF arm proves the collector is unchanged when the instrument is off, and each ON arm asserts its subject was live (an object actually moved, or the safepoint was genuinely idle) before believing the result. Knob parsing is factored into pure functions so both states are covered without mutating process environment, since the live readers cache in a OnceLock.

quarantine_catches_a_planted_stale_from_space_deref is the sabotage arm: it plants a #7184/#7192-shaped stale from-space pointer and asserts the instrument distinguishes it from the live object that would otherwise be recycled into those bytes. A green protected run therefore means the detector works, not that nothing was tried.

New CI arm (gating): scripts/gc_instrument_smoke.sh, wired into .github/workflows/test.yml. It asserts both defaults — inert with the knobs unset, live with them set — and refuses to pass unless the zeal arm forced strictly more collections than the pressure-only arm, so it cannot go green having run zero copying minors (the #6942 / #7024 / #7025 failure mode).

Test-suite parity — order-flaky on both trees, so the failing set was characterised rather than compared once

cargo test -p perry-runtime is order-flaky on both trees, so the failing set was characterised rather than compared once: 33 runs on origin/main, 19 on this branch. Both produce 0–5 failures per run drawn from the same rotating pool, and every name observed on this branch was also observed on main (global_this_webassembly::namespace_members_exist_with_expected_shapes, native_module_stream::stream_constructors_expose_static_method_values, prop_plan::*, teardown::map_set_*, closure::dynamic_props::tests_1802::*, object::tests::closure_name_and_length_ignore_plain_assignment, gc::tests::runtime_roots::test_class_inheritance_side_table_roots_mark_and_rewrite). All pass in isolation. No name is unique to this branch.

scripts/check_file_size.sh passes. scripts/addr_class_inventory.py output is identical to main's (the two handle-floor ratchet failures in child_process/value_util.rs and fs/dirent.rs are pre-existing); the one new gcheader-cast — the quarantine census, which walks a detached block linearly exactly as arena/walk.rs does and can never see a NaN-box payload — has a justified allowlist entry.

Coordination — runtime-side only; nothing from #7192 is included here

Runtime-side only (crates/perry-runtime/src/gc/*, arena/*). No overlap with #7192's codegen work or scripts/gc_root_dominance_check.py. #7192's fix and gap test were used for validation and then reverted out of this branch; nothing from that PR is included here.

Summary by CodeRabbit

• New Features

  • Added opt-in GC rooting diagnostics with from-space quarantine, poisoning, and stale-pointer fault reporting.
  • Added GC zeal mode to force evacuating collections at safepoints.
  • Enhanced stale-reference scans with object details and optional abort diagnostics.
  • Added configurable quarantine depth, protection modes, diagnostic statistics, and platform fallbacks.

• Documentation

  • Documented diagnostic settings, interactions, limitations, and recommended configurations.

• Tests

  • Added automated coverage and smoke tests for GC instrumentation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added default-off GC rooting diagnostics for from-space quarantine, poisoning, page protection, forced evacuating minors, and stale-reference scanning. Added runtime tests, an end-to-end smoke test, CI coverage, documentation, and a changelog.

Changes

GC rooting-bug instruments

Layer / File(s)	Summary
From-space quarantine implementation crates/perry-runtime/src/arena/*, crates/perry-runtime/src/arena/reset.rs, scripts/addr_class_allowlist.txt	Added configurable quarantine, poisoning, Unix page protection, bounded recycling, statistics, signal reporting, and quarantine-aware copying-minor reset behavior.
Safepoint-driven GC zeal crates/perry-runtime/src/gc/zeal.rs, crates/perry-runtime/src/gc/mod.rs, crates/perry-runtime/src/gc/policy.rs	Added PERRY_GC_ZEAL, safepoint-triggered evacuating minors, test overrides, and forced-collection telemetry.
Stale-reference diagnostics crates/perry-runtime/src/gc/fromspace_scan.rs	Unified scan and abort knob resolution, recorded target object types, and added collector backtraces to abort reports.
Instrumentation validation and documentation crates/perry-runtime/src/gc/tests/*, scripts/gc_instrument_smoke.sh, .github/workflows/test.yml, docs/src/internals/memory-model.md, CLAUDE.md, changelog.d/7196-gc-rooting-bug-instruments.md, Cargo.toml	Added unit, integration, smoke, and CI tests. Documented configuration, platform behavior, compile-time poll requirements, diagnostic combinations, and the workspace version update.

Estimated code review effort: 5 (Critical) | ~90+ minutes

Possibly related PRs

• PerryTS/perry#7041: Modifies related fromspace_scan.rs behavior.
• PerryTS/perry#7019: Relates to moving-minor GC behavior and safepoint polling.
• PerryTS/perry#7020: Modifies moving-loop polling and related GC tests.

Suggested labels: bug

Suggested reviewers: proggeramlug

Sequence Diagram(s)

sequenceDiagram
  participant MovingLoopPoll
  participant GCPolicy
  participant ArenaReset
  participant Quarantine
  MovingLoopPoll->>GCPolicy: reach enabled safepoint
  GCPolicy->>ArenaReset: force copying minor under zeal
  ArenaReset->>Quarantine: retire evacuated from-space
  Quarantine-->>GCPolicy: update quarantine statistics

Loading

sequenceDiagram
  participant StaleReference
  participant FromSpaceScan
  participant Collector
  participant DiagnosticReporter
  StaleReference->>FromSpaceScan: inspect from-space target
  FromSpaceScan->>Collector: resolve scan and abort knobs
  FromSpaceScan->>DiagnosticReporter: report target type and address
  DiagnosticReporter-->>Collector: emit backtrace before abort

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the three main changes: from-space protection, GC zeal, and verify-roots improvements.
Description check	✅ Passed	The description is detailed and covers the changes, related issue, validation, tests, limitations, and coordination, but it does not use the template headings or checklist.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 8

🧹 Nitpick comments (3)

crates/perry-runtime/src/gc/tests/fromspace_protect.rs (1)

159-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Read the sentinel through GcHeader.

The raw *const u8 read assumes obj_type is the first field of GcHeader. That holds today, but a field reorder would silently change what this test asserts.

♻️ Proposed change

-    let header_obj_type = unsafe { *((from_space_addr - GC_HEADER_SIZE) as *const u8) };
+    let header_obj_type =
+        unsafe { (*((from_space_addr - GC_HEADER_SIZE) as *const GcHeader)).obj_type };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/gc/tests/fromspace_protect.rs` around lines 159 -
164, Update the retired-header assertion in the from-space protection test to
read the sentinel through the `GcHeader` field representing the object type,
rather than dereferencing a raw `*const u8`; preserve the existing
`QUARANTINE_POISON_OBJ_TYPE` comparison and failure message.

crates/perry-runtime/src/gc/fromspace_scan.rs (2)

318-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The sentinel prints as 255, not 0xFF.

target_obj_type uses {}, so the no-header sentinel appears as decimal 255. The field documentation at Lines 65-69 states the value is "Reported as 0xFF". Align the two so an investigator can grep for the documented spelling.

♻️ Proposed change

-        "  owner={:`#x`} type={} space={:?} +{} {} -> {:`#x`} (type={} {:?}) {} [slot dirty_now={} ever_dirty={} owner_flags={:`#x`} marked={}]",
+        "  owner={:`#x`} type={} space={:?} +{} {} -> {:`#x`} (type={:`#x`} {:?}) {} [slot dirty_now={} ever_dirty={} owner_flags={:`#x`} marked={}]",

Note that this also changes the rendering of real types (4 becomes 0x4), so update the field documentation accordingly if you take this option.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/gc/fromspace_scan.rs` around lines 318 - 325, Update
the format specifier for target_obj_type in the fromspace scan log to
hexadecimal so the no-header sentinel is rendered as 0xFF and matches its
documented spelling. Revise the target_obj_type field documentation to describe
hexadecimal rendering for real object types as well.

---

134-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Cache the resolved knob pair once.

Both readers duplicate the same two std::env::var calls in separate OnceLocks. If the environment changes between the first call to fromspace_scan_enabled() and the first call to fromspace_scan_abort(), the two caches can disagree about the same knobs. A single cached pair removes the duplication and makes the pair atomic.

♻️ Proposed refactor

-pub(super) fn fromspace_scan_enabled() -> bool {
-    use std::sync::OnceLock;
-    static CACHED: OnceLock<bool> = OnceLock::new();
-    *CACHED.get_or_init(|| {
-        resolve_scan_knobs(
-            std::env::var("PERRY_GC_FROMSPACE_SCAN").ok().as_deref(),
-            std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT")
-                .ok()
-                .as_deref(),
-        )
-        .0
-    })
-}
-
-fn fromspace_scan_abort() -> bool {
-    use std::sync::OnceLock;
-    static CACHED: OnceLock<bool> = OnceLock::new();
-    *CACHED.get_or_init(|| {
-        resolve_scan_knobs(
-            std::env::var("PERRY_GC_FROMSPACE_SCAN").ok().as_deref(),
-            std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT")
-                .ok()
-                .as_deref(),
-        )
-        .1
-    })
-}
+fn scan_knobs() -> (bool, bool) {
+    use std::sync::OnceLock;
+    static CACHED: OnceLock<(bool, bool)> = OnceLock::new();
+    *CACHED.get_or_init(|| {
+        resolve_scan_knobs(
+            std::env::var("PERRY_GC_FROMSPACE_SCAN").ok().as_deref(),
+            std::env::var("PERRY_GC_FROMSPACE_SCAN_ABORT")
+                .ok()
+                .as_deref(),
+        )
+    })
+}
+
+pub(super) fn fromspace_scan_enabled() -> bool {
+    scan_knobs().0
+}
+
+fn fromspace_scan_abort() -> bool {
+    scan_knobs().1
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/gc/fromspace_scan.rs` around lines 134 - 158,
Introduce one shared OnceLock-cached pair for the resolved from-space scan
knobs, and have both fromspace_scan_enabled and fromspace_scan_abort read their
respective values from that pair. Move the duplicated environment reads and
resolve_scan_knobs call into the shared initializer so both readers observe one
atomic result.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CLAUDE.md`:
- Line 143: Update the PERRY_GC_ZEAL row in CLAUDE.md to state that zeal only
bypasses the gc_budgeted_due_trigger() requirement, while
gc_safepoint_moving_minor entry guards still apply for in-allocation,
suppressed, unsafe FFI, non-zero root-lock depth, and budgeted-cycle states.
Revise the “does NOT” cell to include these guards and avoid implying every
safepoint always collects.

In `@crates/perry-runtime/src/arena/mod.rs`:
- Around line 16-18: Gate the `quarantine` module and its publicly exposed APIs
in `arena::mod.rs` to supported Unix targets, preventing unconditional
compilation of `libc` calls such as `sysconf`, `mprotect`, and `sigaction` on
Windows. For unsupported targets, add inert fallbacks for
`protect_fromspace_enabled` and `copying_quarantine_from_spaces_and_flip` that
preserve the existing API without performing quarantine actions.

In `@crates/perry-runtime/src/arena/quarantine.rs`:
- Around line 688-694: Update the census coverage filter in the entry lookup to
end at user_offset + size - GC_HEADER_SIZE, matching build_census’s user_offset
and size semantics; keep the existing reverse search and lower-bound check
unchanged so offsets in a following object’s header are not attributed to the
preceding object.
- Around line 392-409: Update push_set_and_evict so a poisoned REGISTRY lock
returns the incoming blocks in evicted for immediate recycling instead of
dropping them. Move SETS_RETIRED.fetch_add into the successful lock branch,
ensuring it increments only when the QuarantinedSet is actually registered.
- Around line 494-500: Repair the survivor arena’s current block before flipping
instead of assigning the first non-null position with unwrap_or(0). Update or
invoke ensure_usable_current_block for the arena handled by
with_survivor_arena_mut, ensuring a fresh usable block is installed when no
empty survivor block remains and the inactive-survivor metadata range is updated
consistently.

In `@crates/perry-runtime/src/gc/tests/fromspace_protect.rs`:
- Around line 107-123: Serialize the exact quarantine counter checks in
protection_off_retires_no_from_space by acquiring the existing
copying_nursery_isolation_lock before capturing the baseline and holding it
through collection and assertions. Apply the same lock to any related tests in
fromspace_protect.rs that compare quarantine_stats deltas, while leaving the
thread-local ProtectionModeGuard behavior unchanged.
- Around line 281-295: Update zeal_implies_forced_evacuation to read the
pre-zeal evacuation state without the redundant nested _zeal guard, then split
the assertion into separate enabled and disabled cases. When
gen_gc_evacuate_enabled() is true, assert that enabling zeal makes
gc_force_evacuate_enabled() true; when it is false, explicitly assert the
expected disabled-state behavior so the test reports that branch instead of
passing vacuously.

In `@docs/src/internals/memory-model.md`:
- Line 136: Update the PERRY_GC_ZEAL=1 documentation to state that an explicit
PERRY_GEN_GC_EVACUATE=0 takes precedence, disables survivor evacuation, and
prevents zeal from forcing objects to move. Keep the existing description of
zeal and its implied PERRY_GC_FORCE_EVACUATE behavior unchanged.

---

Nitpick comments:
In `@crates/perry-runtime/src/gc/fromspace_scan.rs`:
- Around line 318-325: Update the format specifier for target_obj_type in the
fromspace scan log to hexadecimal so the no-header sentinel is rendered as 0xFF
and matches its documented spelling. Revise the target_obj_type field
documentation to describe hexadecimal rendering for real object types as well.
- Around line 134-158: Introduce one shared OnceLock-cached pair for the
resolved from-space scan knobs, and have both fromspace_scan_enabled and
fromspace_scan_abort read their respective values from that pair. Move the
duplicated environment reads and resolve_scan_knobs call into the shared
initializer so both readers observe one atomic result.

In `@crates/perry-runtime/src/gc/tests/fromspace_protect.rs`:
- Around line 159-164: Update the retired-header assertion in the from-space
protection test to read the sentinel through the `GcHeader` field representing
the object type, rather than dereferencing a raw `*const u8`; preserve the
existing `QUARANTINE_POISON_OBJ_TYPE` comparison and failure message.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 31a2e1bf-975a-4764-8f9a-18ac6937cdba

📥 Commits

Reviewing files that changed from the base of the PR and between 42ce572 and 2f659c6.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (14)

• CLAUDE.md
• Cargo.toml
• changelog.d/7154-gc-rooting-bug-instruments.md
• crates/perry-runtime/src/arena/mod.rs
• crates/perry-runtime/src/arena/quarantine.rs
• crates/perry-runtime/src/arena/reset.rs
• crates/perry-runtime/src/gc/fromspace_scan.rs
• crates/perry-runtime/src/gc/mod.rs
• crates/perry-runtime/src/gc/policy.rs
• crates/perry-runtime/src/gc/tests/fromspace_protect.rs
• crates/perry-runtime/src/gc/tests/mod.rs
• crates/perry-runtime/src/gc/zeal.rs
• docs/src/internals/memory-model.md
• scripts/addr_class_allowlist.txt

[proggeramlug]

Completion pass — review resolved, kill-policy settled, instruments proven against the known bugs

Merged main in (no rebase, no force-push; through #7199). All eight review threads answered inline — three fixed, two refuted with evidence, three doc/test corrections.

One blocker found, and it was real

mod quarantine; was ungated and perry-runtime is genuinely compiled for x86_64-pc-windows-msvc — test.yml's windows-build job per-PR, and release-packages.yml via perry-ui-windows → perry-runtime. A crate depending only on libc, calling exactly the surface quarantine.rs used, fails on that target for every symbol: sysconf, _SC_PAGESIZE, mprotect, PROT_NONE/READ/WRITE, sigaction, SA_SIGINFO, SA_ONSTACK, SIGBUS, sigemptyset.

Fixed by gating the syscall helpers (not the whole module), so QuarantineStats, the knob parsers and their tests keep one definition and an identical public API everywhere. Off Unix ProtectPages degrades to poison-only visibly — bytes_protected stays 0 while bytes_poisoned counts the whole retired range, which is what the separately-counted totals were built to reveal. Precedent is pty/mod.rs's #[cfg(unix)] mod native;.

Kill-policy: resolved by option (a), not by amending the policy

Three dark knobs was the right thing to push back on. Both directions are now covered by arms that can fail:

• Sabotage test, quarantine_catches_a_planted_stale_from_space_deref, in the required cargo-test gate. It plants the fix(codegen): never let a shadow-frame slot index escape the pushed frame (#7154) #7184/fix(codegen): make every GC root store dominate the collection points after it #7192 shape (a mutator keeping a pre-collection address across an evacuating minor) and asserts the instrument reports poison where the un-instrumented control reads a valid recycled object. That control is what makes the verdict mean anything — it demonstrates the bug is genuinely invisible without the instrument.
• scripts/gc_instrument_smoke.sh, gating in gc-stress, for the integrated path a unit test cannot reach: codegen emitting back-edge polls → zeal firing → copying minor → quarantine retirement in a real compiled program. It fails unless the zeal arm forces strictly more collections than the pressure-only arm, so it cannot go green having run zero copying minors. Locally: off = 0 retirements, zeal = 1230, all arms correct, ~20s.

Verified the gates can fail. With the quarantine branch disabled, 4 of 13 tests go red — including the sabotage test, at its live-subject assertion — while the knob-parse and OFF-arm tests correctly stay green.

Detect-the-known-bugs: red-then-green on the reverted fix

Same instrumented binary, test_gap_gc_new_instance_rooting.ts, compiled and run with PERRY_GC_MOVING_LOOP_POLLS=1:

arm	result	detection
#7192 reverted, no instruments	bad 9/400, exit 0	none — silent wrong answer
#7192 reverted, ZEAL=1 PROTECT_FROMSPACE=1 DEPTH=800	SIGBUS, exit 138	fault at the stale deref, retired_by_minor=#0
fixed, no instruments	bad 0, exit 0	—
fixed, ZEAL=1	bad 0, exit 0 (220s)	—
fixed, ZEAL=1 PROTECT_FROMSPACE=1 DEPTH=800	bad 0, exit 0 (343s)	clean — no false positive

That run also surfaced a second census bug the first fix had masked. The faulting address is a raw header address (#7192 publishes %inst, the pre-header pointer), and bounding coverage at the payload end left it attributed to nobody — (no census entry covers this offset). Coverage now matches the object's whole extent, header included, with census_lookup extracted and pinned for all four cases: payload, last payload byte, own header, stride padding.

Also

• Version bump reverted, then re-resolved to main's 0.5.1278 by the merge — Cargo.toml/Cargo.lock are now byte-identical to main, nothing for the maintainer to strip. Changelog fragment renamed to the PR-keyed 7196-.
• The Critical survivor-current finding is refuted with a regression test: a tombstone has size == 0, so ArenaBlock::alloc fails bumped > self.size and returns None without dereferencing data. The ordinary reset_region_to_zero path sets current = 0 unconditionally, so tombstone-tolerant allocation is a property the allocator has always needed.
• docs/CLAUDE.md: zeal does not bypass gc_safepoint_moving_minor's entry guards, and loses to an explicit PERRY_GEN_GC_EVACUATE=0.

Note for the maintainer, unrelated to this PR

scripts/check_file_size.sh fails on pristine origin/main (97e843f62) — 16 files over the 2000-line cap, none touched here, byte-identical to main, script unmodified. lint is red on main independently of this branch.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/src/internals/memory-model.md (1)

139-140: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the caveat count.

The lead-in says "Two caveats", but the list contains four bullets: the copying-minor gating (Line 142), the depth guidance (Line 145), the loop-poll requirement (Line 154), and the Unix-only page protection (Line 158). Use a count-free lead-in so later additions cannot make it wrong again.

📝 Proposed wording

-Two caveats these instruments are explicit about, because both have burned
-prior investigations:
+Caveats these instruments are explicit about, because each has burned prior
+investigations:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/src/internals/memory-model.md` around lines 139 - 140, Update the
lead-in before the caveat list to remove the hard-coded “Two” count and use
count-free wording, leaving the four caveat bullets unchanged.

🧹 Nitpick comments (1)

CLAUDE.md (1)

139-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider trimming the knob table and pointing to the docs page.

Lines 141-143 restate docs/src/internals/memory-model.md Lines 133-162 and the changelog fragment nearly in full. Three copies of the same knob semantics will drift. Keep the "gates EXACTLY / does NOT" rows short here and link to the memory-model section for the long form.

As per coding guidelines: "Keep CLAUDE.md concise; put detailed change history in changelog.d/ fragments".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 139 - 146, Trim the knob table in CLAUDE.md to
concise “gates EXACTLY / does NOT” summaries for PERRY_GC_PROTECT_FROMSPACE,
PERRY_GC_PROTECT_FROMSPACE_DEPTH, PERRY_GC_ZEAL, and
PERRY_GC_FROMSPACE_SCAN_ABORT. Remove the duplicated implementation details and
changelog/reproducer history, and add a link to the relevant
docs/src/internals/memory-model.md section for the long-form semantics.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/src/internals/memory-model.md`:
- Around line 139-140: Update the lead-in before the caveat list to remove the
hard-coded “Two” count and use count-free wording, leaving the four caveat
bullets unchanged.

---

Nitpick comments:
In `@CLAUDE.md`:
- Around line 139-146: Trim the knob table in CLAUDE.md to concise “gates
EXACTLY / does NOT” summaries for PERRY_GC_PROTECT_FROMSPACE,
PERRY_GC_PROTECT_FROMSPACE_DEPTH, PERRY_GC_ZEAL, and
PERRY_GC_FROMSPACE_SCAN_ABORT. Remove the duplicated implementation details and
changelog/reproducer history, and add a link to the relevant
docs/src/internals/memory-model.md section for the long-form semantics.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f983474-4699-4acd-86ad-bf52c58bfde2

📥 Commits

Reviewing files that changed from the base of the PR and between 2f659c6 and 4e319a6.

📒 Files selected for processing (9)

• .github/workflows/test.yml
• CLAUDE.md
• changelog.d/7196-gc-rooting-bug-instruments.md
• crates/perry-runtime/src/arena/mod.rs
• crates/perry-runtime/src/arena/quarantine.rs
• crates/perry-runtime/src/gc/tests/fromspace_protect.rs
• crates/perry-runtime/src/gc/tests/mod.rs
• docs/src/internals/memory-model.md
• scripts/gc_instrument_smoke.sh

🚧 Files skipped from review as they are similar to previous changes (2)

• crates/perry-runtime/src/gc/tests/mod.rs
• crates/perry-runtime/src/arena/mod.rs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Cargo.toml`:
- Line 295: Restore the [workspace.package] version in Cargo.toml to 0.5.1278,
leaving release/version metadata unchanged otherwise.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f1cdb427-b753-4060-ac36-17f3539c8624

📥 Commits

Reviewing files that changed from the base of the PR and between 4e319a6 and 846ef23.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (16)

• .github/workflows/test.yml
• CLAUDE.md
• Cargo.toml
• changelog.d/7196-gc-rooting-bug-instruments.md
• crates/perry-runtime/src/arena/mod.rs
• crates/perry-runtime/src/arena/quarantine.rs
• crates/perry-runtime/src/arena/reset.rs
• crates/perry-runtime/src/gc/fromspace_scan.rs
• crates/perry-runtime/src/gc/mod.rs
• crates/perry-runtime/src/gc/policy.rs
• crates/perry-runtime/src/gc/tests/fromspace_protect.rs
• crates/perry-runtime/src/gc/tests/mod.rs
• crates/perry-runtime/src/gc/zeal.rs
• docs/src/internals/memory-model.md
• scripts/addr_class_allowlist.txt
• scripts/gc_instrument_smoke.sh

🚧 Files skipped from review as they are similar to previous changes (13)

• scripts/addr_class_allowlist.txt
• crates/perry-runtime/src/gc/tests/mod.rs
• .github/workflows/test.yml
• CLAUDE.md
• crates/perry-runtime/src/gc/policy.rs
• crates/perry-runtime/src/arena/mod.rs
• crates/perry-runtime/src/gc/zeal.rs
• crates/perry-runtime/src/gc/tests/fromspace_protect.rs
• crates/perry-runtime/src/arena/reset.rs
• crates/perry-runtime/src/gc/mod.rs
• docs/src/internals/memory-model.md
• crates/perry-runtime/src/gc/fromspace_scan.rs
• crates/perry-runtime/src/arena/quarantine.rs

[proggeramlug]

The three deferred gates, run on the idle mini (darwin-arm64, the gc-ratchet pinned-baseline host)

Built from this PR head (4e319a6c4) in its own worktree + isolated target dir. Oracle node 26.5.1 matches .node-version exactly.

1. Matrix --arms all --pressure 8 — EXONERATED by in-place A/B

Branch arm: PASS=296 UNVER=242 XFAIL=1 FAIL=70.

FAIL=70 is the number #7198 identified as the contended signature, so I did not trust it. I reverted only this PR's nine runtime files to origin/main in the same worktree and same target dir (true in-place A/B — same host, same package set, only the collector differing), rebuilt, and re-ran:

arm	summary
this PR	PASS=296 UNVER=242 XFAIL=1 FAIL=70
origin/main collector	PASS=296 UNVER=242 XFAIL=1 FAIL=70

Byte-identical — and the failing sets are identical too, not just the counts:

10  test_gap_repsel_pshape_tower_delete        10  test_gap_repsel_p4a_logical_numeric
10  test_gap_repsel_p4b_field_store_elision    10  test_gap_repsel_p4a_inline_tiers
10  test_gap_repsel_p4a3_ptr_numarray          10  test_gap_repsel_module_init_canonical
10  test_gap_repsel_loop_bounded_i32            1  test_gap_repsel_ptr_shape_locals

#7196 causes none of them. Worth noting for #7194: the red is broader than that issue records. #7194 documents test_gap_repsel_p4a3_ptr_numarray across ten requires=move arms; in fact seven tests fail that way, all with evacuated=0. Same family, 7× the surface.

2. gc-ratchet — red for a demonstrably main-side reason, and I can prove the cause

Both profiles fail: check --profile pinned_host → exit 1, --profile shared_ci → exit 1, 86 gated regressions (heap_used_bytes +5371%, minor_cycles 80 → 0, copied_objects 18,294 → 0, rss +275%).

Every regression is an evacuation counter going to zero. Rather than assume, I re-measured this PR's own binary with PERRY_GC_MOVING_LOOP_POLLS=1 — the pre-#7161 default:

probe	baseline	current (default)	with POLLS=1
01_nursery_churn	14	0	14
02_survivor_promotion	10	0	10
03_cross_gen_writes	22	0	22
04_dead_after_deep_stack	104	0	104
05_closure_capture	26	0	26
06_string_retention	64	0	64
07_array_grow_evacuate	80	0	80
08_map_set_sidetables	84	0	84

minor_cycles reproduces the baseline exactly on this PR's build, and copied_objects to within ~1% (allocation-order noise). The pinned baseline was captured at 88dcee83b, 2026-07-30, when loop polls were still on by default; #7161's flip is the entire delta. This is #7205's territory, not a verdict on #7196 — and per that issue the ratchet has not executed on main for three merges, so I am flagging rather than claiming a pass. The baseline needs re-pinning before the ratchet can gate anything again.

That table doubles as the non-vacuity evidence the instruments-off case needs: with the knobs unset and polls restored, this PR's collector reproduces the baseline collector's evacuation counters exactly.

3. Full runtime suite, both scan modes — clean

PERRY_CONSERVATIVE_STACK_SCAN	result	failures
0 (production / disabled)	1626 passed, 3 failed	teardown::map_set_owner_records_follow_growth, teardown::map_set_side_allocations_release_exactly_once, webassembly::namespace_members_exist_with_expected_shapes
1 (Full)	1628 passed, 1 failed	webassembly::namespace_members_exist_with_expected_shapes

All are the documented order-flaky pool, and all three pass in isolation (verified individually with --test-threads=1). Different subsets fail in the two modes, which is the signature of ordering rather than of either scan mode.

Verdict

All three gates are clean with respect to this PR. The two that are red are red on origin/main for reasons proven main-side — one by in-place A/B, one by reproducing the baseline exactly once the pre-#7161 default is restored.