fix(codegen): close #7192's own residual root-store hole, and make the dominance checker able to fail

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

@@ -0,0 +1,157 @@
+name: GC Root Dominance
+
+# Static gate for the invariant a GC-managed value's root store must DOMINATE
+# every subsequent site that can trigger a collection (#7154).
+#
+# Two bugs of this class shipped before there was an instrument for it. #7184:
+# the root store was emitted but its shadow-slot index fell outside the pushed
+# frame, so `js_shadow_slot_bind` bounds-checked it into a silent no-op. #7192:
+# the store was emitted in-frame but AFTER a call that allocates. Both present
+# identically — a *rooted* slot holding a dangling pointer, surfacing cycles
+# later as "TypeError: value is not a function" — and neither is visible to any
+# runtime GC probe, because at the moment of the collection there is nothing
+# for the collector to find. A static pass over the emitted IR is the only
+# instrument that sees them before they crash, which is why this is a gate and
+# not a benchmark.
+#
+# THIS JOB IS DESIGNED TO BE ABLE TO FAIL, and is checked against all four ways
+# a gate can be unable to (CLAUDE.md):
+#
+#   1. no `continue-on-error`, no `|| true`, no pipe between the checker and
+#      the shell's exit status;
+#   2. NOT yet in branch protection's required contexts — deliberately, because
+#      a new gate has never been green and promoting it immediately blocks every
+#      open PR. Promote after one clean week on `main`;
+#   3. `concurrency` cancels pull-request runs only, never `main` runs;
+#   4. the subject is ASSERTED live, not assumed. `--self-test` proves the
+#      checker still reports a planted violation and still clears the control,
+#      and `--min-files` / `--min-binds` refuse a clean verdict over a corpus
+#      that contained no modules or no root stores. An empty `.perry-trace/llvm`
+#      is a routine outcome of a failed compile, so "0 violations" over 0 files
+#      must be an error rather than a pass.
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+concurrency:
+  # Per-event groups, cancelling PR runs only. A shared group with an
+  # unconditional cancel-in-progress starves `main`: on a deep runner queue
+  # every merge cancels the previous main run before it reaches a runner, and a
+  # gate that is always cancelled never fails. Same reasoning as gc-ratchet.yml.
+  group: gc-root-dominance-${{ github.event_name }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+env:
+  CARGO_TERM_COLOR: always
+  MACOSX_DEPLOYMENT_TARGET: "13.0"
+
+jobs:
+  gc-root-dominance:
+    runs-on: macos-14
+    timeout-minutes: 90
+    steps:
+      - uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+
+      # Fast structural failure first: prove the checker can still fail before
+      # spending a compiler build on it. This is the arm that would have caught
+      # `PERRY_GC_FORCE_EVACUATE` being inert for every test that "exercised"
+      # it (#6942/#6946).
+      - name: Checker self-test (can this gate still fail?)
+        run: python3 scripts/gc_root_dominance_check.py --self-test
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Cache cargo
+        uses: actions/cache@v6
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            target
+          key: ${{ runner.os }}-cargo-gcdom-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: ${{ runner.os }}-cargo-
+
+      - name: Build perry and the runtime archives
+        run: |
+          set -euo pipefail
+          # perry-runtime and perry-stdlib are rlib-only; the .a files come from
+          # the -static wrapper crates. The package set is fixed so cargo
+          # feature unification matches every other job that builds the
+          # compiler.
+          cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static
+          for artifact in perry libperry_runtime.a libperry_stdlib.a; do
+            test -s "target/release/$artifact" \
+              || { echo "::error::target/release/$artifact was not produced"; exit 1; }
+          done
+
+      - name: Emit the IR corpus
+        env:
+          # PERRY_GC_MOVING_LOOP_POLLS=1 is what puts `js_gc_loop_safepoint` in
+          # the IR, which is what the MOVING classification keys on. It is off
+          # by default (#7161 stopgap), so without it this gate would run over
+          # IR that cannot express the bug — hazard 4 again.
+          PERRY_GC_MOVING_LOOP_POLLS: "1"
+          # Makes every root store the @js_shadow_slot_bind call form. The #7088
+          # inline diamond is equivalent but harder to anchor on.
+          PERRY_INLINE_SHADOW_SLOT: "0"
+          PERRY_NO_AUTO_OPTIMIZE: "1"
+        run: |
+          set -euo pipefail
+          mkdir -p ir-corpus
+          # A spread of shapes that exercise the lowerings this invariant runs
+          # through: construction, object/array literals and spreads, class
+          # expressions with statics, property and element stores, closures.
+          # Kept to test-files/ so the corpus is versioned with the repo rather
+          # than depending on a private workload.
+          shopt -s nullglob
+          sources=(
+            test-files/test_gap_gc_*.ts
+            test-files/test_gap_class*.ts
+            test-files/test_gap_object*.ts
+            test-files/test_gap_static*.ts
+            test-files/test_gap_prop*.ts
+          )
+          if [ "${#sources[@]}" -eq 0 ]; then
+            echo "::error::no corpus sources matched; the glob is stale"
+            exit 1
+          fi
+          for src in "${sources[@]}"; do
+            name="$(basename "$src" .ts)"
+            rm -rf .perry-trace/llvm
+            # A source that fails to compile must not silently shrink the
+            # corpus: --min-files below is the backstop, but say so here too.
+            if ! ./target/release/perry compile "$src" -o "/tmp/$name" --trace llvm >/dev/null 2>&1; then
+              echo "::warning::$src did not compile; skipping"
+              continue
+            fi
+            for ll in .perry-trace/llvm/*.ll; do
+              cp "$ll" "ir-corpus/${name}__$(basename "$ll")"
+            done
+          done
+          echo "corpus: $(find ir-corpus -name '*.ll' | wc -l) .ll files"
+
+      - name: Check root-store dominance
+        run: |
+          set -euo pipefail
+          # No pipe: the checker's own exit status is the job's. --min-binds
+          # asserts the corpus actually contained root stores, so a green
+          # verdict cannot come from IR that never had a subject.
+          python3 scripts/gc_root_dominance_check.py ir-corpus \
+            --moving-only --min-files 5 --min-binds 50 -v
+
+      - name: Upload the IR corpus on failure
+        if: failure()
+        uses: actions/upload-artifact@v7
+        with:
+          name: gc-root-dominance-ir
+          path: ir-corpus
+          retention-days: 7

CLAUDE.md

@@ -237,3 +237,4 @@ Corollary: a *new* gate has never been green, so promoting it to required immedi
 - **Async-to-generator transform, body locals.** It boxes every body local into a shared mutable cell typed `Any`. Two consequences seen in the wild: per-iteration `let`/`const` bindings collapse for closures created in a loop, and computed numeric-key calls (`arr[i](x)`) lose their type proof and silently resolve by *method name*, evaporating the call.
 - **Native base-class subclassing.** A native base's surface is installed at `super()` time and its parent edge lives in the class registry; keying any of that on a literal `extends` name loses it for fieldless classes, indirect subclasses, and class expressions.
 - **Two prototype-resolution paths.** `CLASS_PROTOTYPE_OBJECTS` (synthetic: `Object.create`, plain-function ctors) vs `CLASS_DECL_PROTOTYPE_OBJECTS` (declared classes). `in`/`for…in` and `getPrototypeOf` have disagreed about the same chain.
+- **Root-store dominance in codegen.** *A GC-managed value's root store must **dominate** every subsequent site that can collect.* Three ways it has broken, all shipped: the store's slot index fell outside the pushed shadow frame so `js_shadow_slot_bind` bounds-checked it into a silent no-op (#7184); the store was emitted in-frame but **after** a call that allocates (#7192); and the value lives in a plain `alloca_entry` that is neither a shadow slot nor a temp root, so the collector never rewrites it (`lower_call/new.rs`'s inline-ctor `this_slot`, still open). All three present identically — a *rooted* slot holding a dangling pointer, surfacing cycles later as `TypeError: value is not a function` — and **none is visible to any runtime GC probe**, because at the moment of the collection there is nothing for the collector to find. That is why #7154's from-space scan only ever saw offenders whose targets had already died. The instrument is static: `scripts/gc_root_dominance_check.py` over `--trace llvm` output (`--self-test` proves it can still fail). Only bites under `PERRY_GC_MOVING_LOOP_POLLS=1`, off by default since #7161 — so a green default run says nothing about this class.

changelog.d/7198-root-store-dominance-followup.md

@@ -0,0 +1,25 @@
+### Fixed
+
+- **codegen: three more root-store dominance holes, and one that defeated #7192's own fix at its last instruction (#7154 partial)**. Post-merge follow-up to #7192, adjudicating its review round reproducer-first.
+  - **the inline-constructor result slot (`lower_call/new.rs`)** — the load-bearing one. `ctor_result_slot` is a plain `alloca_entry`: not a shadow slot, not a temp root, never rewritten by the collector. Seeding it with `obj_box` parked the **pre**-constructor instance address in unrooted memory for the whole body, and on fall-through (no explicit `return`) `js_ctor_return_override` saw an *object* in `raw` and returned **that** — discarding the instance `reload_instance` had just re-read. #7192's dominance fix was defeated one instruction after it landed. It is **not** gated on `PERRY_INLINE_CTOR`: `force_ctor_call` requires `class.constructor.is_some()`, so any class with fields or heritage but no own constructor (`class C { payload = mk() }`, `class C extends B {}`) takes the inline path by default. And #7192's own regression test could not see it — `the_new_instance_is_rooted_across_the_constructor_body` walks the *first* operand of the override, which is the re-read one. The slot now starts at `undefined`, exactly equivalent on all four paths (fall-through, bare `return;`, `return <expr>`, inherited-symbol ctor) and carrying no address. It also removes a latent spurious `TypeError`: a *derived* class whose instance's GC type is outside `constructor_return_overrides_this`'s set previously fell through to "Derived constructors may only return object or undefined" on a plain fall-through.
+  - **the computed key of a property/element store (`expr/index_set.rs`, `expr/static_field_meta.rs`)** — `o[k] = f()` and a class expression's `[sym]: init` lower the key *before* the value, leaving it in an SSA register across exactly the window #7192 closed for the receiver. A non-literal string key is an ordinary heap string with no registered root of its own, so `unbox_str_handle` below the call would hand the setter a pre-move `StringHeader*`. Rooted with the same guard, pushed after the receiver's and released before it so the `temp_root_truncate` cuts nest. The guard family is renamed `ReceiverGuard`/`guard_store_receiver` → `StoreOperandGuard`/`guard_store_operand`, because rooting a key through something called "receiver" is the naming drift that let #7114's two predicates diverge.
+  - **`Expr::ObjectSpread` and `Expr::ClassExprFresh` protection predicates (`expr/logical_collections.rs`, `expr/static_field_meta.rs`)** — both were computed from the operand expressions alone, so a construct whose parts are all inert pushed no root even though the lowering itself emits a user-code boundary. A **spread part** now forces protection on its own (`js_object_copy_own_fields` reads every own key of the source, so an accessor there runs arbitrary JS *inside* the helper), and so does a **`static { … }` block** (its body is user code by definition) — which is why `block_fns` moves above `rooted_handle_begin`. Verified in the emitted IR: a class expression with one inert named static, one static block, no captures and no symbol statics answered `false` to every term of the old predicate and emitted no instance root at all; it now pushes one and re-reads it before each block call, before the capture snapshot and before the final `nanbox_pointer_inline`.
+
+  The rest of both predicates stays byte-identical to `Expr::Object`'s, deliberately. An allocation inside a runtime helper provably cannot *initiate* a moving collection: `gc_check_trigger()`'s minor arm defers to the loop safepoint under `PERRY_GC_MOVING_LOOP_POLLS=1` (`gc/policy.rs`, `GC_SAFEPOINT_PENDING`), falls back to a conservative-scanned non-moving minor with polls off, and reaches a budgeted `MutatorAssist` step with `evacuation_policy_allowed = false` on the shipped default; C4b skips non-tenured nursery objects outright. "N `js_object_set_field_by_name` calls, therefore force the root" is not a sound reason, and forking these predicates away from `Expr::Object`'s on it would recreate the two-copies-of-one-decision shape that produced #7114.
+
+### Fixed (tooling)
+
+- **`scripts/gc_root_dominance_check.py` could not fail in four ways.** It is a gate, so it is now audited against all four (CLAUDE.md): it exited 0 on no arguments, on `--help`, on a typo'd flag and on a directory holding no `.ll` — and the corpus is *generated*, so "the trace directory is empty" is a routine outcome of a failed compile rather than an exotic one (now argparse + `--min-files`, exit 2). It had no liveness assertion, so a clean verdict over **zero root stores** was indistinguishable from a clean verdict over the real corpus (now `--min-binds`, with `root stores: N` printed in the summary so a green run carries its own evidence — this is the arm that catches a forgotten `PERRY_INLINE_SHADOW_SLOT=0`). A function whose body had no basic-block label parsed to **zero blocks and was silently skipped**: a planted violation vanished and the run exited 0 (now `MalformedIR`, as is any label-shaped line the strict regex declines, which used to be appended to the previous block and merge two blocks into a fabricated intra-block path). And `--self-test` now plants a violation of exactly this class — same-block and cross-block-through-a-diamond forms — and asserts the checker reports both, clears an otherwise-identical control, and raises on the malformed fixture.
+
+  While there: LLVM's own printed label form (`if.then.1:   ; preds = %entry.0`) is now accepted rather than mis-parsed, so pointing the tool at `llvm-dis` / `opt -S` output either works or says why not. The `%entry.implicit` synthetic block a reviewer described does not exist and was never the defect.
+
+- **`PERRY_SAVE_LL` / `--trace llvm` silently emitted nothing for split modules.** `codegen/mod.rs` forces `n_units = 1` only for `emit_ir_only`, and the `n_units > 1` path `return`s before the `PERRY_SAVE_LL` write — so every module past `MIN_CALLABLES_TO_SPLIT` (8000 callables) was absent from `.perry-trace/llvm`, i.e. exactly the largest modules, which is where a static IR audit most needs to look. The comment above it claimed the opposite. The split path now writes one `<module>.unitN.ll` per codegen unit, at no extra peak because the units are already materialized there. A corpus that quietly omits its biggest members makes a clean verdict meaningless.
+
+### Added
+
+- **`gc-root-dominance` workflow** — runs the checker's `--self-test` first (fast structural failure before a compiler build), then emits an IR corpus from `test-files/` under `PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_INLINE_SHADOW_SLOT=0` and checks it with both liveness floors. No `continue-on-error`, no `|| true`, no pipe between the checker and the job's exit status; `concurrency` cancels pull-request runs only so a busy `main` queue cannot starve it. **Deliberately not a required context yet** — a gate that has never been green blocks every open PR the day it is promoted; promote after a clean week on `main`.
+
+### Notes
+
+- #7192's fragment claimed `Expr::ObjectSpread` is where zod's 269-key spread lowers. It is not: since #809 an object literal containing a spread lowers to a source-ordered IIFE built on `js_object_assign_one` (`lower/expr_object.rs:844`), and `Expr::ObjectSpread`'s **sole** construction site is a JSX spread attribute (`crates/perry-hir/src/jsx.rs:67`). The fix is still right; its blast radius is JSX.
+- Three residuals of this class remain, all reproduced at `73a9084ea` (before #7184/#7192, so inherited rather than caused) and all clean under the shipped default. They are why #7161 cannot be reverted yet: heap values in plain `alloca_entry` slots the collector never rewrites (the inline-ctor `this_slot`, the `[N x i64]` closure-capture staging array); `{ ...src, k: v }` with an accessor source, which **SIGSEGVs** (`exit=139`) under polls on the `js_object_assign_one` path; and a class expression with a `static { … }` block, which **SIGSEGVs** under polls because the value `js_static_this_arm_value` parks in the runtime's static-`this` cell is not rooted.

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

@@ -2653,9 +2653,9 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
     // #5391 codegen units: large modules split their object compilation into N
     // independently-compiled units so clang's peak RSS stays ~whole/N instead of
     // OOMing on one giant TU. Gated to large modules (default 1 unit = unchanged
-    // behavior). `emit_ir_only` and `PERRY_SAVE_LL` want the whole-module text,
-    // so they take the single-text path; the split path avoids materializing the
-    // full ~1GB IR string at all (which would defeat the memory win).
+    // behavior). `emit_ir_only` wants the whole-module text, so it takes the
+    // single-text path; the split path avoids materializing the full ~1GB IR
+    // string at all (which would defeat the memory win).
     let n_units = if opts.emit_ir_only {
         1
     } else {
@@ -2668,6 +2668,21 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
             hir.name,
             units.len()
         );
+        // #7154: dump the units. The comment above used to claim `PERRY_SAVE_LL`
+        // took the single-text path — it never did; this `return` fires before
+        // the `PERRY_SAVE_LL` write below. So `--trace llvm` silently emitted
+        // NOTHING for any module past `MIN_CALLABLES_TO_SPLIT`, i.e. exactly the
+        // largest modules, which is where a static IR audit
+        // (`scripts/gc_root_dominance_check.py`) most needs to look — a corpus
+        // that quietly omits its biggest members makes a clean verdict
+        // meaningless. One file per unit, not one concatenation: the units are
+        // already materialized here, so this adds no peak.
+        if let Ok(save_dir) = std::env::var("PERRY_SAVE_LL") {
+            for (i, unit) in units.iter().enumerate() {
+                let filename = format!("{}/{}.unit{}.ll", save_dir, module_prefix, i);
+                let _ = std::fs::write(&filename, unit);
+            }
+        }
         return crate::linker::compile_units_to_object(&units, opts.target.as_deref());
     }