GC rooting: measure the dependency-scale corpus, then close the rule that dominates it (#7280)

.github/workflows/gc-root-dominance.yml

@@ -157,6 +157,19 @@ jobs:
       - name: Install Rust toolchain
         uses: dtolnay/rust-toolchain@stable
 
+      # ★ The dependency-scale corpus needs the dependency.
+      #
+      # `zod` is this repo's own package.json devDependency, pinned by
+      # package-lock.json and governed by the same soak window as everything
+      # else in that file -- not a fixture invented for this job.
+      # `--ignore-scripts` because nothing here needs a lifecycle script to
+      # run, and a corpus generator is a bad place to execute one.
+      - uses: actions/setup-node@v6
+        with:
+          node-version-file: .node-version
+      - name: Install the npm dependencies the dep corpus compiles
+        run: npm ci --ignore-scripts --no-audit --no-fund
+
       - name: Cache cargo
         uses: actions/cache@v6
         with:
@@ -242,10 +255,95 @@ jobs:
             --allowlist scripts/gc_root_dominance_allowlist.json \
             -v
 
+      # ★★ The DEPENDENCY-SCALE corpus (#7280).
+      #
+      # Everything above this line runs over ~124 hand-written `test-files/`
+      # sources. That corpus read ZERO in both gated modes while twenty lines of
+      # stock `zod` faulted deterministically under the from-space protector,
+      # and #7280 records the gap in one sentence: 25 curated files pass while
+      # 20 lines of stock zod fail.
+      #
+      # It is not a size problem, it is a distribution problem. Measured with
+      # `--stale-registers --moving-only` on the same compiler:
+      #
+      #   curated (124 sources, 144 modules):   116 stale uses, and what
+      #     dominates is property-GET helper windows and js_number_coerce
+      #   dependency-scale (81 modules, 62 MB): 370 stale uses, and what
+      #     dominates is js_object_assign_one (object spread, 137) and
+      #     js_new_function_construct (102) -- populations the curated corpus
+      #     produces 12 and 1 of
+      #
+      # Nothing is sampled away: all 81 modules and all 62 MB are checked. The
+      # cost is ~8s to emit and ~4s for the two gated arms below, because those
+      # arms are linear in instruction count. The `--stale-registers` ratchet is
+      # the expensive one (~5 min) and says so where it runs.
+      - name: Emit the dependency-scale IR corpus
+        run: ./scripts/gc_root_dominance_dep_corpus.sh ir-corpus-dep
+
+      - name: Check root-store dominance (dependency-scale)
+        run: |
+          set -euo pipefail
+          # Floors from the corpus as of this commit (81 modules, ~12900
+          # functions, ~7700 root stores), set below that with room for the
+          # dependency's own churn. `zod` growing is fine; `zod` no longer
+          # compiling natively is the finding, and these are what make it one.
+          python3 scripts/gc_root_dominance_check.py ir-corpus-dep \
+            --moving-only \
+            --min-files 60 --min-binds 4000 --min-funcs 6000 \
+            --allowlist scripts/gc_root_dominance_allowlist.json \
+            --seeded-violations 40 \
+            -v
+
+      - name: Check that every GC value in an alloca has a root store (dependency-scale)
+        run: |
+          set -euo pipefail
+          python3 scripts/gc_root_dominance_check.py ir-corpus-dep \
+            --unrooted-allocas \
+            --moving-only \
+            --min-files 60 --min-binds 4000 --min-funcs 6000 \
+            --allowlist scripts/gc_root_dominance_allowlist.json \
+            -v
+
+      # ★ The stale-register RATCHET, on both corpora.
+      #
+      # `--stale-registers` asks the third question: not "is the root store
+      # late" and not "is there a root store at all", but "is a register holding
+      # a rooted value used below a collection point". It is the mode that found
+      # #7206's two bugs and the mode #7280's zod fault lives in, and until now
+      # it ran only by hand -- so its number could move in either direction
+      # between one investigation and the next with nothing to say so.
+      #
+      # It is a BUDGET rather than an allowlist because the residual is a
+      # population, not a list of triaged sites: the remaining uses are the ones
+      # whose slot the program itself reassigns inside the window, which need a
+      # temp root rather than a re-read (see crate::root_reload). A budget can
+      # only be lowered, and lowering it is the ratchet.
+      #
+      # This step is minutes, not seconds -- the scan is superlinear in
+      # instruction count and the dependency corpus is 62 MB. That is the price
+      # of checking the population that actually breaks.
+      - name: Stale-register budget (curated)
+        run: |
+          set -euo pipefail
+          python3 scripts/gc_root_dominance_check.py ir-corpus \
+            --stale-registers --moving-only \
+            --min-files 90 --min-binds 1500 --min-funcs 1200 \
+            --max-stale 39
+
+      - name: Stale-register budget (dependency-scale)
+        run: |
+          set -euo pipefail
+          python3 scripts/gc_root_dominance_check.py ir-corpus-dep \
+            --stale-registers --moving-only \
+            --min-files 60 --min-binds 4000 --min-funcs 6000 \
+            --max-stale 118
+
       - name: Upload the IR corpus on failure
         if: failure()
         uses: actions/upload-artifact@v7
         with:
           name: gc-root-dominance-ir
-          path: ir-corpus
+          path: |
+            ir-corpus
+            ir-corpus-dep
           retention-days: 7

changelog.d/7311-dep-scale-corpus-and-root-reload.md

@@ -0,0 +1,166 @@
+### GC rooting: measure the right corpus, then fix the one rule that dominates it
+
+Two linked changes: the gate's corpus was measuring the wrong population, and
+once it measured the right one, 73% of what it reported was a single rule.
+
+#### The corpus (#7280)
+
+`scripts/gc_root_dominance_corpus.sh` compiles ~124 hand-written `test-files/`
+sources. It reads **zero** in both modes the CI gate runs, and it read zero
+while twenty lines of stock `zod` faulted deterministically under the from-space
+protector. #7280 puts the gap in one sentence: *25 curated files pass while 20
+lines of stock zod fail.*
+
+That is a distribution problem, not a size problem. Both corpora, same compiler,
+`--stale-registers --moving-only`:
+
+| corpus | stale uses | dominant population |
+|---|---|---|
+| curated — 124 sources, 144 modules, 2378 functions | 116 | property-GET helper windows, `js_number_coerce`, `js_closure_callN` |
+| dependency-scale — 81 modules, 62 MB, 12899 functions | 371 | `js_object_assign_one` 137, `js_new_function_construct` 102, `js_closure_call*` 30 |
+
+The curated corpus produces 12 of the first population and 1 of the second. A
+hand-written test allocates a couple of objects and calls a couple of helpers; a
+library spreads objects into objects, boxes every mutable capture because its
+closures outlive their frames, and builds values field by field out of data.
+The rooting hazards live in the *shapes*.
+
+So `scripts/gc_root_dominance_dep_corpus.sh` generates the second corpus from a
+real npm dependency — `zod`, this repo's own `package.json` devDependency,
+pinned by `package-lock.json` and governed by the same soak window as everything
+else in that file — imported **by source path** so its modules compile natively
+rather than falling back to V8 and emitting no IR to check.
+
+**Nothing is sampled away.** All 81 modules and all 62 MB are checked. Emitting
+costs ~8s; the two gated arms cost ~3s each, because they are linear in
+instruction count. The `--stale-registers` budget is the expensive one (~5 min)
+and the workflow says so where it runs.
+
+`test-files/gc-dep-corpus/main.ts` is the only entry point and the rest of that
+directory reaches the compiler by being imported from it, so the generator
+**asserts that every `.ts` in the directory produced a module** — the check a
+size floor could not be (#7278), since ~90 modules of `zod` swamp any count a
+missing 40-line source would cross.
+
+#### The rule (`crate::root_reload`)
+
+> A load out of a shadow slot is a **copy** of a root. An evacuating minor
+> rewrites the slot; it cannot rewrite the register. So every use a collection
+> point can reach must re-read the slot — unless a store to that slot can also
+> run on the way, in which case re-reading would observe an assignment the
+> program made and the register is left alone.
+
+Verbatim from `zod`'s object-spread lowering, before:
+
+```llvm
+  call void @js_shadow_slot_bind(i32 0, ptr %r7)   ; %r7 IS a root
+  %r8  = load double, ptr %r7                      ; a COPY of the root
+  %r9  = call double @baseFields()                 ; evacuates; rewrites %r7
+  %r10 = call double @js_object_assign_one(double %r8, double %r9)  ; from-space
+  %r11 = load double, ptr %r7                      ; the NEXT statement re-reads
+```
+
+Codegen was never unable to re-read the slot — the following statement does it,
+because a fresh lowering emits a fresh load. The bug is that *within* one
+lowering the load happens once, at the top, and the register is carried across
+everything after it. That is why this is a pass and not another point fix:
+`index_set.rs` alone lowers `object` before `value` at fifteen separate arms,
+and the shape also appears in the object-literal spread, the inline class-field
+store, `new`, property define, `instanceof` and the closure-call family.
+
+The soundness half is the half `expr/temp_root.rs` already documents: re-reading
+a *local* is not unconditionally safe, because `new C(g, bump())` where `bump()`
+assigns `g` must pass the pre-`bump()` value. At IR level that objection is
+decidable — an assignment to a shadow-slotted local is a `store` to its alloca
+in the same function, and a local captured *and* mutated by a closure is boxed
+instead, so it has no plain shadow slot to reload. Both halves are path
+questions over the real CFG, answered the way the checker answers them: a
+back-edge round trip is not an intra-iteration path, because re-entering the
+load's block re-executes the load.
+
+Where the reload was not needed — no call between the two points — LLVM's
+EarlyCSE/GVN forwards it away, since nothing can clobber the alloca. Where it
+was needed the intervening call is opaque and the load stays. The pass is close
+to free exactly where it is redundant.
+
+Recording is at the choke point, not at the thirteen `js_shadow_slot_bind` emit
+sites: `LlBlock::call_void` and `LlFunction::entry_setup_call_void` see every
+bind form, including the slow arm of #7088's inline diamond (which emits the
+same call), so a fourteenth site cannot be added without the set noticing. And
+the pass runs in `compile_module` before **any** rendering path, so the text
+renderer and the in-process constructor (#7301) see the same IR — a pass living
+inside `to_ir` would silently not apply to the other.
+
+#### What it closes, on both corpora
+
+| arm | curated | dependency-scale |
+|---|---|---|
+| `--stale-registers --moving-only`, total | 116 → **39** | 371 → **118** |
+| …of which `source=slotload` | 102 → **25** | 272 → **19** |
+| bind-anchored `--moving-only` | 0 → 0 | 0 → 0 |
+| `--unrooted-allocas --moving-only` | 0 → 0 | 0 → 0 |
+
+`js_object_assign_one` disappears from the dependency-scale report entirely
+(137 → 0); `js_new_function_construct` goes 102 → 29.
+
+Both budgets are now ratchets in `gc-root-dominance.yml` (`--max-stale 39` and
+`--max-stale 118`), because until now this mode ran only by hand — its number
+could move in either direction between one investigation and the next with
+nothing to say so. It is a budget rather than an allowlist because the residual
+is a *population*, not a list of triaged sites: what is left is the uses whose
+slot the program itself reassigns inside the window, which need a temp root
+rather than a re-read.
+
+#### And one runtime-Rust rooting bug the corpus led to
+
+`js_regexp_new` takes `pattern` as a raw `StringHeader*` and then allocates
+twice — `js_string_from_str` for the canonical flags, `gc_malloc` for the header
+— before storing that pointer into `RegExpHeader::pattern_ptr`. Either
+allocation can run an evacuating minor, after which the argument names retired
+from-space and the header keeps a **permanently** dangling `pattern_ptr`; the
+borrowed `pattern_str` had the same exposure and fed the owned `.source` copy.
+
+That is the runtime-Rust half of the invariant (#7249), which
+`gc_root_dominance_check.py` is structurally blind to — it reads emitted IR and
+cannot see a Rust local. Reproduction:
+
+```
+PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \
+PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 \
+  ./<gc-dep-corpus-main>
+```
+
+Before: faults deterministically, `RETIRED FROM-SPACE`, `js_regexp_new + 3920`
+← `js_regexp_construct` ← `perry_closure_…zod…regexes_ts__9`. The same fault
+appears with the codegen pass alone, so it is not caused by it. After: clean.
+
+No unit-test witness ships with it, and that is a deliberate statement rather
+than an omission: forcing a *copying* minor inside a runtime function needs a
+collection at an allocation point, and `gc/zeal.rs` documents why that level
+does not exist (an allocation-point collection takes `force_full_scan`, which
+makes the copying minor ineligible, so it would move nothing). A test was
+written, its liveness assert refused to pass, and it was deleted rather than
+shipped as a test that cannot fail for the right reason.
+
+#### Measured after #7301 and #7305, not before
+
+The backend rewrite (typed `LlInst`, two consumers of one finalized-item
+visitor) and the move from setjmp/longjmp to `invoke`/`landingpad` both landed
+while this was in flight, so every number above is re-measured on `b50e857c2`,
+both sides, over identical paths. The dependency-scale parent count moved by
+exactly one (370 → 371); nothing else changed.
+
+Two consequences for the pass, both load-bearing:
+
+* It inserts a **typed** `LlInst::Load` and rewrites operands on typed variants
+  in place, never text, so `native_emit`'s `(typed, raw)` migration ratchet moves
+  in the intended direction. And it runs in `compile_module` before *any*
+  rendering path, so `to_ir` and the native C-API builder consume the same
+  stream — a pass inside `to_ir` would have applied to one consumer only.
+* An `invoke` is modelled as **both** a call and a two-successor terminator.
+  Missing the call half would classify a throwing helper's window as
+  non-collecting and drop every reload inside a `try`; missing the successor half
+  would hide the unwind edge. The unwind edge is also why the rule is "reload at
+  the USE" and not "reload after the call": a load from the slot is valid
+  wherever it sits and reads whatever the collector last wrote, so it is correct
+  on both edges without a placement decision.

crates/perry-codegen/src/block.rs

@@ -9,8 +9,8 @@
 //! sorts out the registers. Explicit `phi` nodes are still emitted for
 //! control-flow merges (if/else value context, short-circuit logical ops).
 
-use std::cell::{Cell, RefCell};
-use std::collections::HashMap;
+use std::cell::{Cell, Ref, RefCell};
+use std::collections::{HashMap, HashSet};
 use std::rc::Rc;
 
 use crate::codegen::FpContractMode;
@@ -71,16 +71,59 @@ pub struct RegCounter {
     /// catch/finally bodies see the *enclosing* scope, which is exactly
     /// where a throw escaping them lands at runtime.
     eh_unwind_labels: RefCell<Vec<String>>,
+    /// Every alloca this function has published to the precise-root collector
+    /// with `js_shadow_slot_bind(idx, ptr)`.
+    ///
+    /// Recorded at the choke points rather than at the thirteen emit sites,
+    /// because a fourteenth site is exactly the thing that gets added without
+    /// anyone remembering a register list somewhere else.
+    ///
+    /// A shadow slot is the one class of alloca whose contents the collector
+    /// writes behind generated code's back: an evacuating minor rewrites the
+    /// slot to the object's new address. [`crate::root_reload`] reads this set
+    /// to find the loads that rewrite makes stale. See
+    /// `docs/src/internals/gc-rooting-invariant.md`.
+    shadow_slot_allocas: RefCell<HashSet<String>>,
 }
 
 impl RegCounter {
     pub fn new() -> Self {
         Self {
             value: Cell::new(0),
             eh_unwind_labels: RefCell::new(Vec::new()),
+            shadow_slot_allocas: RefCell::new(HashSet::new()),
         }
     }
 
+    /// Record `ptr` when `callee` is the precise-root bind.
+    ///
+    /// Called from the two choke points every bind form passes through:
+    /// [`LlBlock::call_void`] — which carries the direct call AND the slow arm
+    /// of #7088's inline diamond, since that arm emits the same call — and
+    /// `LlFunction::entry_setup_call_void`, which carries the persistent-slot
+    /// bind hoisted into the entry prelude.
+    pub(crate) fn note_shadow_slot_bind(&self, callee: &str, second_arg: Option<&str>) {
+        if callee != "js_shadow_slot_bind" {
+            return;
+        }
+        // `js_shadow_slot_bind(i32 idx, ptr %slot)`: the slot is argument two.
+        // A non-register operand means the bind was built some other way, and
+        // the conservative answer is to record nothing — a slot this set does
+        // not name is simply never reloaded.
+        if let Some(ptr) = second_arg {
+            if ptr.starts_with('%') {
+                self.shadow_slot_allocas
+                    .borrow_mut()
+                    .insert(ptr.to_string());
+            }
+        }
+    }
+
+    /// The shadow slots bound in this function. See [`crate::root_reload`].
+    pub(crate) fn shadow_slot_allocas(&self) -> Ref<'_, HashSet<String>> {
+        self.shadow_slot_allocas.borrow()
+    }
+
     /// Enter an invoke-EH handler scope: calls emitted from here until the
     /// matching pop unwind to `lpad_label`.
     pub fn push_eh_scope(&self, lpad_label: String) {
@@ -244,6 +287,14 @@ impl LlBlock {
         &self.instructions
     }
 
+    /// Mutable instruction list, for the whole-function passes that run after
+    /// lowering. See [`crate::root_reload`]. Not for emitters — they go through
+    /// [`LlBlock::push_inst`] / [`LlBlock::emit`], which keep the terminator
+    /// discipline and the try-region bookkeeping.
+    pub(crate) fn insts_mut(&mut self) -> &mut Vec<crate::inst::LlInst> {
+        &mut self.instructions
+    }
+
     // -------- Arithmetic (double) --------
     //
     // FP ops are emitted with no LLVM fast-math flags by default. Setting
@@ -1136,6 +1187,8 @@ impl LlBlock {
     pub fn call_void(&mut self, func_name: &str, args: &[(LlvmType, &str)]) {
         // #835 + #846: same registry hook as `call` — see comment there.
         crate::ext_registry::record_ffi_call(func_name);
+        self.counter
+            .note_shadow_slot_bind(func_name, args.get(1).map(|(_, v)| *v));
         if let Some((cont, lpad)) = self.eh_invoke_suffix(func_name) {
             let arg_str = format_args(args);
             self.emit(format!(

crates/perry-codegen/src/codegen/mod.rs

@@ -2566,6 +2566,15 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
         .native_rep_records
         .extend(typed_clone_rejection_records);
 
+    // #7280: re-read every shadow slot below the collection points that can run
+    // under it. Whole-function, so it runs here rather than inside a lowering —
+    // the shape it fixes is spread over dozens of lowerings, fifteen arms of
+    // `index_set.rs` alone. It runs BEFORE any rendering path so the text
+    // renderer and the in-process constructor see the same IR; a pass living in
+    // one of them would silently not apply to the other.
+    // See `crate::root_reload`.
+    crate::root_reload::apply_to_module(&mut llmod);
+
     let verify_native_regions = opts.verify_native_regions
         || std::env::var("PERRY_VERIFY_NATIVE_REGIONS").ok().as_deref() == Some("1");
     if verify_native_regions {