Skip to content

fix(gc): two root-dominance regressions + restore the corpus's missing rooting IR - #8823

Merged
proggeramlug merged 4 commits into
mainfrom
fix/gc-root-dominance-8809-8810
Aug 25, 2026
Merged

fix(gc): two root-dominance regressions + restore the corpus's missing rooting IR#8823
proggeramlug merged 4 commits into
mainfrom
fix/gc-root-dominance-8809-8810

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #8809 and #8810 together — they are interdependent (the corpus fix makes the corpus complete; the dominance fix makes it clean), and were validated stacked.

Context: the gate has been unable to report since 2026-08-15

gc-root-dominance.yml was last green on main in run 31870805900 (2026-08-15T06:59Z) with 144/144 sources compiled and === violations: 0. Every scheduled run since dies at --audit-poll-reach before reaching the corpus, and PR runs are label-gated behind run-extended-tests so they report skipped. See #8821.

Because 2026-08-15 is a clean 0-violation reference, the violations below are new regressions that landed inside that 10-day window, not pre-existing debt.

#8809 — two root-dominance regressions, both #7192-shaped

Root store emitted after a call that can allocate:

  1. perry-codegen/src/expr/object_literal.rs::lower_by_name_props lowered a this-capturing method closure, installed it via js_object_set_field_by_name, and only then pushed its root. The object handle is re-read from its root; the closure is not. The installer can run a user setter or Proxy trap, and the closure is reachable from the object by then, so an evacuating minor moves it — the push then publishes a moved-from address and the deferred this-patch writes the receiver into from-space. Reachable from TypeScript via perf(hir): lower static method literals directly #8793 (landed in perf: static method literals, captured closure reuse, argument shape facts #8796, 2026-08-25). Fix: the install now happens inside the closure's rooted scope and re-reads it.
  2. perry-codegen/src/lower_call/new.rs::construction_runs_user_code returned false for a class whose only private elements are methods/accessors, while emit_field_inits still emitted js_private_brand_add for it (merge: land #8630 (class semantics tail) with six audit fixes #8643, 2026-08-23). That helper calls js_object_set_field_by_name, and its own body notes the marker-key allocation "can evacuate both the receiver and any live value". Fix: the predicate gains || class.has_private_instance_elements().

gc_root_dominance_check.py gains four POLL_CAPABLE_RUNTIME entries. This strengthens detection — every change is an addition. Three (js_builtin_subclass_construct, js_tls_create_secure_context, js_tls_secure_context_new) are the symbols --audit-poll-reach has been naming, which is what unblocks the gate; the fourth (js_private_brand_add) is a referent no audit can ask for, because it matches no alloc/new/create convention that ALLOC_RE recognises. No allowlist entry was added and no detection was narrowed.

#8810 — two GC-rooting sources silently contributed no IR

The corpus reported 2 of 152 sources failed to compile against MAX_SKIPPED=0, yet both compiled fine standalone. Cause was neither a timeout nor contention: it was a link-stage failure discarding already-emitted IR. The corpus sets PERRY_NO_AUTO_OPTIMIZE=1, and those two are the only sources in the corpus importing a node builtin (node:net, node:http2), so they need libperry_ext_{net,http}.a. perry auto-built each wrapper in its own cargo invocation; cargo resolves features per invocation, so the wrapper bundled a different tokio compilation than the prebuilt libperry_stdlib.a, and the link correctly refused that pair (#507/#7629). perry exited 1 after codegen had written the .ll, and the loop's continue discarded it.

That mattered because the two silently missing files are ..._gc_http2_pending_event_callback_rooting and ..._gc_net_once_flags_rekey — the two whose names say they cover GC callback rooting, which is exactly what the gate detects. The script's own header warns that "IR that was never emitted reads to the checker exactly like IR with no violations in it".

Fix: compile with --no-link. --trace llvm is written during codegen, long before a link line exists, so linking was never this gate's subject. MIN_SOURCES also corrected 131 → 152 (it had drifted, leaving room for 21 sources to vanish before the "corpus shrank" arm could fire), and a failed compile now prints its first error line beside the source name. MAX_SKIPPED stays 0 and nothing was added to any exclusion list.

Validation (merged tree, verified independently)

before after
corpus 150/152, 2 skipped, 177 .ll, rc=1 152/152, 0 skipped, 179 .ll, rc=0
dominance checker 7 violations, rc=1 0 violations, rc=0
  • all 30 lint-job gates pass
  • perry-codegen 1252, perry-runtime 2690 — 0 failed
  • new regression test a_method_closure_is_rooted_before_it_is_installedpasses (it was authored but never compiled, due to a full disk; verified here)
  • gc_root_dominance_check.py --self-testself-test OK, so the checker can still fail
  • seeded-violation arm reported 40 planted / 40 caught / 0 missed at release profile
  • the 177 previously-shared .ll files are byte-identical before/after the --no-link change, so it altered no emitted IR
  • df checked before and after every decisive run; no result here was produced under ENOSPC

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability when creating objects with methods and classes using private methods, including during memory management and garbage collection.
    • Prevented stale references during object construction and method installation.
  • Tests

    • Expanded runtime checks to cover additional code paths that may trigger memory collection.
    • Increased corpus coverage and improved failure diagnostics, compiling all discovered sources successfully.
  • Documentation

    • Added guidance on memory-safety auditing and clarified how to interpret audit failures.

Ralph Küpper added 4 commits August 25, 2026 13:29
…eting its own subjects

`scripts/gc_root_dominance_corpus.sh` reported 150/152 sources compiled, 2
skipped, exit 1 on clean `main` — and the two it dropped were
`test_gap_gc_net_once_flags_rekey` and
`test_gap_gc_http2_pending_event_callback_rooting`, i.e. the two whose names
say they cover GC callback rooting, in the gate that exists to check exactly
that. The script's own header names this hazard: "IR that was never emitted
reads to the checker exactly like IR with no violations in it."

Not a compiler regression. The corpus sets `PERRY_NO_AUTO_OPTIMIZE=1`
deliberately, which forbids the specialized runtime/stdlib rebuild. Those two
sources are the only ones in the corpus that import a node builtin
(`node:net`, `node:http2`), so they need a prebuilt `libperry_ext_{net,http}.a`
that the documented build command does not produce. perry auto-built each
wrapper in its own cargo invocation; cargo resolves features per invocation,
so the wrapper's tokio came out as a different compilation than the prebuilt
`libperry_stdlib.a`'s, and `compile/shared_tokio.rs` refused that pair at link
time — correctly (#507/#7629). perry exited non-zero *after* codegen had
already written the `.ll`, and the loop's `continue` threw that IR away.

Measured, not inferred: the failing invocation leaves
`.perry-trace/llvm/<name>_ts.ll` on disk, and the refusal names the two tokio
ids. The divergence is per-invocation, not per-profile: the two wrappers perry
auto-built are both `--release` with an identical tokio feature set, and carry
`tokio-32c8af8634bd9648` and `tokio-5436513d89214d4a`. `cargo --unit-graph` at
that same profile shows why — a lone `-p perry-ext-net` resolves `rustls`
without `ring`, `bitflags`/`log` without `std`, and so on.

`--no-link` fixes it by removing the failure mode rather than tolerating it.
`--trace llvm` is written during codegen, long before the link line exists, so
the link stage was never this gate's subject; not linking means there is no
link line to be wrong, no ext archive to be missing, and no stale
`libperry_runtime.a` deciding what the corpus contains. It also stops the
script writing 152 executables it never reads.
`scripts/compiler_output_harness/repsel_census.py` compiles with `--no-link`
for the same reason. A source that stops *linking* is still a finding — it is
`./run_parity_tests.sh`'s, which compiles and runs every `test_gap_*.ts` under
the shipping configuration.

Shadow lowering, same compiler both arms:

  before  150/152 sources, 2 skipped, 177 .ll, exit 1
  after   152/152 sources, 0 skipped, 179 .ll, exit 0

The 177 shared `.ll` files are byte-identical across the two runs, so
`--no-link` changed nothing about the emitted IR. The two new modules add 30
functions and 53 root stores and zero new violations: both arms report the
same 7 violations over the same 3 fingerprints (tracked in #8809), and
`--unrooted-allocas` stays at 0, now over 179 files instead of 177.

The gate can still fail, and that is checked rather than assumed: with a
wrapper that makes one source's compile exit 1, the run reports 151/152, 1
skipped, exit 1.

Two follow-ups in the same file. A failed compile now reports its first error
line beside the source name, one skip per line — a skip that names only a file
is a finding you must reproduce locally before you can even read it, which is
how this one read as a compiler regression. And `MIN_SOURCES` is raised
131 → 152, the measured discovery count: it had drifted exactly the way the
old `MIN_COMPILED=90` floor did, leaving room for 21 sources to vanish before
the "corpus shrank" arm could fire.

Fixes #8810
…points (#8809)

`gc-root-dominance` has been red on `main` since 2026-08-15, and every
scheduled run failed at the same step -- `--audit-poll-reach`, which runs
BEFORE the compiler build. None of the four gated arms below it executed for
ten days, and two #7192-shaped rooting regressions landed inside that window.
Neither PR carried `run-extended-tests`, so the opt-in PR arm did not see them
either. Measured at release profile with the workflow's exact invocation:
7 violations, all `MOVING: YES`, over 3 fingerprints; 8 unfiltered.

* `expr/object_literal.rs` -- `lower_by_name_props` installed a
  `this`-capturing method closure with `js_object_set_field_by_name` and only
  THEN pushed its root. That setter is a collection point and the closure is
  reachable from the object by the time it runs, so an evacuating minor moves
  it: the push published a moved-from address into a slot the collector scans,
  and the deferred `this`-patch loop wrote the receiver into abandoned
  from-space. The install now happens inside the closure's rooted scope and
  re-reads it. Unreachable from TypeScript between #809 and #8793, which is why
  the ordering survived the #7192 sweep.

* `lower_call/new.rs` -- `construction_runs_user_code` answered `false` for a
  class whose only private elements are methods or accessors, while
  `emit_field_inits` still emits `js_private_brand_add` for it (#8643). That
  helper allocates the marker key and calls `js_object_set_field_by_name`; its
  own body says the allocation "can evacuate both the receiver and any live
  value". `new WithPrivateMethod()` therefore fed a stale handle to
  `js_gc_init_typed_shape_layout` and published it into the caller's root slot.

* `scripts/gc_root_dominance_check.py` -- `POLL_CAPABLE_RUNTIME` gains the
  three symbols `--audit-poll-reach` had been naming since 2026-08-15, which is
  what unblocks the gate, plus `js_private_brand_add`, which no audit can ask
  for because `ALLOC_RE` does not match it. No allowlist entry, no narrowing;
  `--self-test` still reports its planted violation.

No version bump.
@proggeramlug
proggeramlug merged commit cd5ab38 into main Aug 25, 2026
17 of 18 checks passed
@proggeramlug
proggeramlug deleted the fix/gc-root-dominance-8809-8810 branch August 25, 2026 12:40
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 97b4a96f-8a6b-49a7-bac4-47144424d17c

📥 Commits

Reviewing files that changed from the base of the PR and between 58785ec and 039baf8.

📒 Files selected for processing (8)
  • changelog.d/8809-gc-root-dominance-late-roots.md
  • changelog.d/8810-gc-root-dominance-corpus-no-link.md
  • crates/perry-codegen/src/expr/object_literal.rs
  • crates/perry-codegen/src/lower_call/new.rs
  • crates/perry-codegen/src/rooting/mod.rs
  • docs/src/internals/gc-rooting-invariant.md
  • scripts/gc_root_dominance_check.py
  • scripts/gc_root_dominance_corpus.sh

📝 Walkthrough

Walkthrough

The change fixes object-literal closure rooting, accounts for private-element construction during GC analysis, expands poll-capable runtime coverage, and updates corpus compilation and diagnostic reporting.

Changes

GC root dominance

Layer / File(s) Summary
Object-literal closure rooting
crates/perry-codegen/src/expr/object_literal.rs, crates/perry-codegen/src/rooting/mod.rs, changelog.d/8809-gc-root-dominance-late-roots.md
Method closures are rooted before installation. RootedAcc::as_arg is available within the crate. Regression coverage checks root-store ordering in emitted IR.
Private-element construction audit
crates/perry-codegen/src/lower_call/new.rs, scripts/gc_root_dominance_check.py, docs/src/internals/gc-rooting-invariant.md
Private instance elements mark construction as collection-capable. Poll-capable runtime symbols and audit-gate behavior are documented.
Corpus compilation and reporting
scripts/gc_root_dominance_corpus.sh, changelog.d/8810-gc-root-dominance-corpus-no-link.md
Corpus compilation uses --no-link, raises MIN_SOURCES to 152, preserves generated IR, and reports skipped-source diagnostics separately.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: jdalton

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/gc-root-dominance-8809-8810

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gc-root-dominance: 7/3 gated (8/4 unfiltered) NEW root-dominance regressions landed 2026-08-15..25 while the gate was red

1 participant