gc: teach the root-dominance checker to read statepoint relocation bundles - #7663
Merged
Conversation
📝 WalkthroughWalkthroughThe PR adds native LLVM statepoint support to the GC root-dominance checker and corpus generator. It adds statepoint analysis, native corpus validation, an independent CI job, failure artifacts, gate wiring, and documentation for shadow and native modes. ChangesNative Statepoint Validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CIJob
participant CorpusScript
participant Compiler
participant LLVMOpt
participant StatepointChecker
CIJob->>CorpusScript: generate native corpus
CorpusScript->>Compiler: compile with native RS4GC lowering
Compiler-->>CorpusScript: traced LLVM IR
CorpusScript->>LLVMOpt: apply production statepoint rewrite
LLVMOpt-->>CorpusScript: native statepoint IR
CIJob->>StatepointChecker: validate corpus and hazard budgets
StatepointChecker-->>CIJob: validation results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
scripts/gc_root_dominance_check.py (4)
3269-3287: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe class census counts pre-allowlist hits; state that in the output.
Lines 3284-3287 build
per_class,per_kindandper_sinkfromfound, while Lines 3338-3339 gate onremaining. The header at Line 3304 readsstatepoint hazards: Nwith no qualifier, so an allowlisted corpus prints a non-zero census next to a passing verdict. Label the census line as unfiltered.📝 Proposed fix
- print(f"=== statepoint hazards: {len(found)} " + print(f"=== statepoint hazards (unfiltered, before allowlist): " + f"{len(found)} " f"(unrooted: {per_class['unrooted']}, stale: {per_class['stale']})")🤖 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 `@scripts/gc_root_dominance_check.py` around lines 3269 - 3287, Update the statepoint census output in run_statepoints to label the hazard count as unfiltered, since per_class, per_kind, and per_sink are computed from found before apply_allowlist while gating uses remaining. Preserve the existing counts and passing verdict behavior.
3404-3453: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_sp_seed_siteshas a dead variable and a redundant branch.
block_startis assigned at Lines 3420 and 3423 and never read. The two yields at Lines 3448-3453 are identical, so theelifadds no behaviour.♻️ Proposed cleanup
- block_start = 0 fn_hi = 0 for i, line in enumerate(lines): if line.startswith("define "): fn_hi = next((k for k in range(i + 1, len(lines)) if lines[k].startswith("}")), len(lines)) - block_start = i + 1 continue if LABEL_RE.match(line) or line.startswith("}"): - block_start = i + 1 continue @@ - if use_at is not None and use_at > i + 1: - # Splice ABOVE the use but BELOW the source, so the safepoint lands - # strictly inside the window. - yield use_at, reg - elif use_at == i + 1: - yield use_at, reg + if use_at is not None: + # Splice ABOVE the use but BELOW the source, so the safepoint lands + # strictly inside the window. + yield use_at, reg🤖 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 `@scripts/gc_root_dominance_check.py` around lines 3404 - 3453, Clean up _sp_seed_sites by removing the unused block_start assignments and collapsing the final if/elif yield logic into a single condition that yields use_at and reg whenever use_at is not None. Preserve the existing site-selection behavior.
4871-4876: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe disarmed-knob guard cannot see an explicitly passed default value.
The guard compares each option against its default.
--max-unrooted 0and--min-statepoints 1are therefore accepted and ignored when--statepointsis absent. That is the exact shape the surrounding comments reject: a knob that reads as a check that ran.Detect the flag rather than the value.
♻️ Proposed fix: sentinel defaults
- ap.add_argument("--min-statepoints", type=int, default=1, metavar="N", + ap.add_argument("--min-statepoints", type=int, default=None, metavar="N",(and the same for
--min-live-bundles,--min-relocates,--max-unrooted)- for flag, val in (("--min-statepoints", ns.min_statepoints != 1), - ("--min-live-bundles", ns.min_live_bundles != 1), - ("--min-relocates", ns.min_relocates != 1), - ("--max-unrooted", ns.max_unrooted != 0)): - if val and not ns.statepoints: + for flag, val in (("--min-statepoints", ns.min_statepoints), + ("--min-live-bundles", ns.min_live_bundles), + ("--min-relocates", ns.min_relocates), + ("--max-unrooted", ns.max_unrooted)): + if val is not None and not ns.statepoints: ap.error(f"{flag} requires --statepoints") + if ns.min_statepoints is None: + ns.min_statepoints = 1 + if ns.min_live_bundles is None: + ns.min_live_bundles = 1 + if ns.min_relocates is None: + ns.min_relocates = 1 + if ns.max_unrooted is None: + ns.max_unrooted = 0Add a self-test arm for
["--max-unrooted", "0", path]expecting exit 2, so the guard covers the default value too.🤖 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 `@scripts/gc_root_dominance_check.py` around lines 4871 - 4876, Update the argument definitions for --min-statepoints, --min-live-bundles, --min-relocates, and --max-unrooted to use sentinel defaults, then make the disarmed-knob guard detect whether each option was explicitly supplied rather than comparing its value with the normal default. Preserve normal defaults when --statepoints is enabled, and add a self-test for ["--max-unrooted", "0", path] expecting exit 2.
3310-3319: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the exemption report; it is duplicated verbatim.
Lines 3310-3319 and Lines 5120-5129 print the same block with the same
by_keyconstruction and the same knob suffix. Extract one helper and call it from both paths, so a change to the wording or the knob hint cannot diverge between the two modes.♻️ Proposed refactor
def print_exempt_counts(exempt_counts): if not exempt_counts: print("=== suppressed by an IMMOVABLE_SOURCES exemption: none") return print("=== suppressed by an IMMOVABLE_SOURCES exemption " "(`#7210`: rewritten location, immovable object):") by_key = {s.key: s for s in IMMOVABLE_SOURCES} for k, n in sorted(exempt_counts.items(), key=lambda kv: -kv[1]): src = by_key.get(k) knob = f" (--{src.knob.replace('_', '-')} re-reports these)" if src else "" print(f" {n:6d} {k}{knob}")🤖 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 `@scripts/gc_root_dominance_check.py` around lines 3310 - 3319, Extract the duplicated IMMOVABLE_SOURCES exemption-reporting block into a shared print_exempt_counts helper, preserving its empty and non-empty output, by_key construction, sorting, and knob suffix behavior. Replace both existing report paths with calls to this helper so the wording and formatting remain consistent.
🤖 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 @.github/workflows/gc-root-dominance.yml:
- Around line 377-379: Update the branch protection or required-status-check
configuration associated with the gc-root-dominance workflow to include
gc-root-dominance-statepoints for main. Ensure it is required alongside the
existing checks before relying on this workflow gate.
In `@scripts/gc_root_dominance_check.py`:
- Around line 300-322: Update CALL_RE and the callee extraction in the call
parsing flow to accept quoted LLVM callee names such as "foo bar" as well as
existing bare names, preserving the captured name without its surrounding
quotes. Ensure the statepoint handling in the visible call-initialization block
still recognizes wrapped calls and sets is_statepoint, live, token, and callee
consistently for quoted and unquoted forms.
- Around line 3240-3247: Update the docstring for statepoint_corpus_stats to
document the five-value return tuple, including unreadable alongside
statepoints, live_bundles, relocates, and live_roots; leave the function’s
return values and caller unchanged.
- Around line 233-243: Update statepoint_callee to parse quoted wrapped callees
such as @"name$suffix" in addition to bare identifiers, returning the unquoted
callee name while preserving indirect-callee handling. Add a self-test fixture
containing a quoted wrapped callee to exercise this path and verify the callee
is retained for moving-only hazard detection.
- Around line 3352-3362: Ensure the native statepoint job invokes the checker
with --max-stale 0, or make run_statepoints default max_stale to 0 whenever
--statepoints is enabled. Preserve explicit --max-stale overrides, and remove
any stale allowlist entries that remain unnecessary after the verdict is
enforced.
In `@scripts/gc_root_dominance_corpus.sh`:
- Around line 259-273: Update the native rewrite failure handling in the
corpus-generation flow to fail the run when the OPT invocation fails, rather
than removing the output and continuing. Preserve the existing diagnostic
capture, but ensure opt_failed causes the overall script to exit nonzero after
processing or immediately; alternatively, enforce an explicit reviewed allowlist
for known failures.
- Around line 307-315: Update the statepoint and live-bundle count pipelines in
the corpus validation block around sp and live so no-match grep results become
zero under set -euo pipefail, while preserving failures from other grep errors.
Ensure the existing empty-corpus diagnostic and exit path still runs when either
count is zero.
---
Nitpick comments:
In `@scripts/gc_root_dominance_check.py`:
- Around line 3269-3287: Update the statepoint census output in run_statepoints
to label the hazard count as unfiltered, since per_class, per_kind, and per_sink
are computed from found before apply_allowlist while gating uses remaining.
Preserve the existing counts and passing verdict behavior.
- Around line 3404-3453: Clean up _sp_seed_sites by removing the unused
block_start assignments and collapsing the final if/elif yield logic into a
single condition that yields use_at and reg whenever use_at is not None.
Preserve the existing site-selection behavior.
- Around line 4871-4876: Update the argument definitions for --min-statepoints,
--min-live-bundles, --min-relocates, and --max-unrooted to use sentinel
defaults, then make the disarmed-knob guard detect whether each option was
explicitly supplied rather than comparing its value with the normal default.
Preserve normal defaults when --statepoints is enabled, and add a self-test for
["--max-unrooted", "0", path] expecting exit 2.
- Around line 3310-3319: Extract the duplicated IMMOVABLE_SOURCES
exemption-reporting block into a shared print_exempt_counts helper, preserving
its empty and non-empty output, by_key construction, sorting, and knob suffix
behavior. Replace both existing report paths with calls to this helper so the
wording and formatting remain consistent.
🪄 Autofix
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: b5c30feb-bb12-45de-9712-5aa328335215
📒 Files selected for processing (4)
.github/workflows/gc-root-dominance.ymldocs/src/internals/gc-rooting-invariant.mdscripts/gc_root_dominance_check.pyscripts/gc_root_dominance_corpus.sh
Comment on lines
+377
to
+379
| gc-root-dominance-statepoints: | ||
| runs-on: macos-14 | ||
| timeout-minutes: 90 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
repo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner')"
gh api "repos/$repo/branches/main/protection/required_status_checks" \
--jq '.contexts[]' \
| grep -Fx 'gc-root-dominance-statepoints'Repository: PerryTS/perry
Length of output: 205
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Workflow file list:"
git ls-files .github/workflows
echo
echo "Target workflow line count:"
wc -l .github/workflows/gc-root-dominance.yml
echo
echo "Relevant required-status context and job declarations:"
rg -n "required status checks|not required|required|gc-root-dominance|statepoints|required_contexts|branches:" .github/workflows/gc-root-dominance.yml || true
echo
echo "Status check related wording in other workflow files:"
rg -n "required status checks|required_contexts|gc-root-dominance|statepoints" .github/workflows || trueRepository: PerryTS/perry
Length of output: 5194
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Top and relevant comments:"
sed -n '1,90p' .github/workflows/gc-root-dominance.yml
echo
sed -n '350,385p' .github/workflows/gc-root-dominance.yml
echo
echo "Workflow context names around job declarations:"
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/gc-root-dominance.yml')
lines = p.read_text().splitlines()
depth = None
for i, line in enumerate(lines, 1):
if i < 60 or i > 530:
continue
stripped = line.lstrip()
if stripped.startswith('runs-on:') or stripped.startswith(('gc-', 'statepoint', 'gc-root-dominance')) or stripped.startswith('#'):
pass
d = len(line) - len(stripped)
if stripped.startswith(' ') and stripped[2:14] == 'gc-root-dominance:':
depth = d
print(f"{i}: depth {d}: {stripped}")
elif depth is not None and d == depth and line.strip() and not line.strip().startswith('#'):
print(f"{i}: depth {d}: {stripped}")
PYRepository: PerryTS/perry
Length of output: 6827
Add gc-root-dominance-statepoints to main required status checks.
This workflow declares gc-root-dominance-statepoints as not required, and the comments call out that promotion must be completed as the second step. Add gc-root-dominance-statepoints to the required status checks before relying on this gate to block regressions.
🤖 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 @.github/workflows/gc-root-dominance.yml around lines 377 - 379, Update the
branch protection or required-status-check configuration associated with the
gc-root-dominance workflow to include gc-root-dominance-statepoints for main.
Ensure it is required alongside the existing checks before relying on this
workflow gate.
Source: Coding guidelines
Comment on lines
+233
to
+243
| def statepoint_callee(text): | ||
| """The `@name` after the balanced `elementtype(...)`, or None if indirect.""" | ||
| got = _balanced_group(text, "elementtype(") | ||
| if got is None: | ||
| return None | ||
| _inner, end = got | ||
| after = text[end:].lstrip() | ||
| if not after.startswith("@"): | ||
| return None # indirect callee: a `%reg` sits here instead | ||
| name = re.match(r"[\w.$]+", after[1:]) | ||
| return name.group(0) if name else None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
statepoint_callee cannot read a QUOTED wrapped callee.
LLVM quotes any identifier it cannot print bare, which is why DEFINE_RE was widened at Line 114 for @"name$suffix". The wrapped callee operand is printed the same way:
ptr elementtype(i64 (i64, i64)) @"perry_fn_get$typed_f64"
Line 242 matches [\w.$]+ against "perry_fn_get$typed_f64". The first character is ", so re.match returns None and the callee becomes <indirect>.
Effect: every statepoint that wraps a repsel specialisation loses its callee. movers then returns empty, moving is False, and --moving-only — the arm the gate runs — drops those hazards. That is the same 7% population this PR widened DEFINE_RE for, and the population the docs name as where a representation change moves the rooting obligation.
🐛 Proposed fix: accept the quoted form
after = text[end:].lstrip()
if not after.startswith("@"):
return None # indirect callee: a `%reg` sits here instead
- name = re.match(r"[\w.$]+", after[1:])
- return name.group(0) if name else None
+ # LLVM quotes an identifier it cannot print bare -- every repsel
+ # specialisation (`…$typed_f64`). Read the quoted form too, or the
+ # callee reads `<indirect>` and --moving-only drops the hazard.
+ name = re.match(r'"([^"]*)"|([-\w.$]+)', after[1:])
+ if not name:
+ return None
+ return name.group(1) if name.group(1) is not None else name.group(2)Add a fixture whose wrapped callee is quoted, so the self-test covers this arm.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def statepoint_callee(text): | |
| """The `@name` after the balanced `elementtype(...)`, or None if indirect.""" | |
| got = _balanced_group(text, "elementtype(") | |
| if got is None: | |
| return None | |
| _inner, end = got | |
| after = text[end:].lstrip() | |
| if not after.startswith("@"): | |
| return None # indirect callee: a `%reg` sits here instead | |
| name = re.match(r"[\w.$]+", after[1:]) | |
| return name.group(0) if name else None | |
| def statepoint_callee(text): | |
| """The `@name` after the balanced `elementtype(...)`, or None if indirect.""" | |
| got = _balanced_group(text, "elementtype(") | |
| if got is None: | |
| return None | |
| _inner, end = got | |
| after = text[end:].lstrip() | |
| if not after.startswith("@"): | |
| return None # indirect callee: a `%reg` sits here instead | |
| # LLVM quotes an identifier it cannot print bare -- every repsel | |
| # specialisation (`…$typed_f64`). Read the quoted form too, or the | |
| # callee reads `<indirect>` and --moving-only drops the hazard. | |
| name = re.match(r'"([^"]*)"|([-\w.$]+)', after[1:]) | |
| if not name: | |
| return None | |
| return name.group(1) if name.group(1) is not None else name.group(2) |
🤖 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 `@scripts/gc_root_dominance_check.py` around lines 233 - 243, Update
statepoint_callee to parse quoted wrapped callees such as @"name$suffix" in
addition to bare identifiers, returning the unquoted callee name while
preserving indirect-callee handling. Add a self-test fixture containing a quoted
wrapped callee to exercise this path and verify the callee is retained for
moving-only hazard detection.
Comment on lines
300
to
+322
| c = CALL_RE.search(text) | ||
| self.callee = c.group(1) if c else None | ||
| self.is_statepoint = False | ||
| self.live = () | ||
| self.token = None | ||
| if self.callee is not None and STATEPOINT_MARK[1:] in self.callee: | ||
| # ★ `callee` becomes the WRAPPED callee, deliberately. | ||
| # | ||
| # Every consumer in this file — `is_collecting`, | ||
| # `compute_poll_reaching`, the `--moving-only` classification, the | ||
| # `RECEIVER_SINKS` ranking — asks "what function does this call?". | ||
| # Under RS4GC the textual callee is always | ||
| # `llvm.experimental.gc.statepoint.p0`, so leaving it alone would | ||
| # make every safepoint in the corpus indistinguishable and | ||
| # `--moving-only` would classify nothing. Rewriting it here means | ||
| # those consumers keep working unmodified against both lowerings, | ||
| # which is the entire argument for this being a mode rather than a | ||
| # second script. | ||
| self.is_statepoint = True | ||
| self.live = statepoint_live_set(text) | ||
| self.token = self.result | ||
| wrapped = statepoint_callee(text) | ||
| self.callee = wrapped if wrapped is not None else "<indirect>" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect CALL_RE, ALLOC_RE, BIND_RE and other name-matching regexes for quote support.
rg -n -C2 '^(CALL_RE|ALLOC_RE|BIND_RE|GLOBAL_ROOT_RE|RECEIVER_SINKS|REWRITTEN_LOAD_RE)\s*=' scripts/gc_root_dominance_check.pyRepository: PerryTS/perry
Length of output: 1615
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
CALL_RE = re.compile(r"\b(?:call|invoke)\s+[^@]*@([\w.$]+)\(")
samples = [
'call noundef i32 @"main.__static_initialization_and_destruction_0"(i32 noundef 1, i32 noundef 1) `#7`',
'call i32 `@llvm.experimental.gc.statepoint.p0.i32`(i32 1, i32 2)',
'call void @"quoted call"(ptr %0)',
'call void `@quoted` call(ptr %0)',
]
for s in samples:
c = CALL_RE.search(s)
print([s, c.group(1) if c else None])
PYRepository: PerryTS/perry
Length of output: 454
Handle quoted LLVM callee names in CALL_RE.
CALL_RE is bare-only (@([\\w.$]+)), so LLVM calls like call void @"foo bar"(...) leave self.callee = None. In that case the RS4GC wrapper in this block also leaves self.is_statepoint = False, so related consumers can miss collecting calls and statepoints.
🤖 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 `@scripts/gc_root_dominance_check.py` around lines 300 - 322, Update CALL_RE
and the callee extraction in the call parsing flow to accept quoted LLVM callee
names such as "foo bar" as well as existing bare names, preserving the captured
name without its surrounding quotes. Ensure the statepoint handling in the
visible call-initialization block still recognizes wrapped calls and sets
is_statepoint, live, token, and callee consistently for quoted and unquoted
forms.
Comment on lines
+3240
to
+3247
| def statepoint_corpus_stats(parsed): | ||
| """`(statepoints, live_bundles, relocates, live_roots)` over a corpus. | ||
|
|
||
| The subject-liveness numbers. `statepoints` alone is not enough: a corpus | ||
| can be full of safepoints that record nothing, which is what an unrooted | ||
| build looks like, so `live_bundles` and `relocates` are counted separately | ||
| and floored separately. | ||
| """ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Docstring states a 4-tuple; the function returns 5 values.
Line 3266 returns (n_sp, n_live, n_reloc, n_roots, unreadable). The caller at Line 4983 unpacks five. Update the docstring so the return contract is correct.
📝 Proposed fix
- """`(statepoints, live_bundles, relocates, live_roots)` over a corpus.
+ """`(statepoints, live_bundles, relocates, live_roots, unreadable)` over a
+ corpus. `unreadable` is the set of `"gc-live"` operands `AS1_DEF_RE` could
+ not classify; a non-empty set means the predicate is narrower than the IR.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def statepoint_corpus_stats(parsed): | |
| """`(statepoints, live_bundles, relocates, live_roots)` over a corpus. | |
| The subject-liveness numbers. `statepoints` alone is not enough: a corpus | |
| can be full of safepoints that record nothing, which is what an unrooted | |
| build looks like, so `live_bundles` and `relocates` are counted separately | |
| and floored separately. | |
| """ | |
| def statepoint_corpus_stats(parsed): | |
| """`(statepoints, live_bundles, relocates, live_roots, unreadable)` over a | |
| corpus. `unreadable` is the set of `"gc-live"` operands `AS1_DEF_RE` could | |
| not classify; a non-empty set means the predicate is narrower than the IR. | |
| The subject-liveness numbers. `statepoints` alone is not enough: a corpus | |
| can be full of safepoints that record nothing, which is what an unrooted | |
| build looks like, so `live_bundles` and `relocates` are counted separately | |
| and floored separately. | |
| """ |
🤖 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 `@scripts/gc_root_dominance_check.py` around lines 3240 - 3247, Update the
docstring for statepoint_corpus_stats to document the five-value return tuple,
including unreadable alongside statepoints, live_bundles, relocates, and
live_roots; leave the function’s return values and caller unchanged.
Comment on lines
+3352
to
+3362
| if max_stale is not None: | ||
| if len(stale_remaining) > max_stale: | ||
| if not verbose: | ||
| print("\n".join(render(v) for v in stale_remaining)) | ||
| print(f"error: {len(stale_remaining)} stale-across-safepoint " | ||
| f"use(s), budget is {max_stale}. Lower the count or raise " | ||
| "--max-stale deliberately.", file=sys.stderr) | ||
| rc = rc or 1 | ||
| else: | ||
| print(f"within budget: stale {len(stale_remaining)} <= {max_stale}") | ||
| return rc |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how CI invokes the statepoint mode, and whether --max-stale is passed.
fd -e yml -e yaml . .github/workflows --exec rg -n -C6 'gc_root_dominance_check|--statepoints' {}
rg -n -C4 -- '--statepoints' docs scripts 2>/dev/nullRepository: PerryTS/perry
Length of output: 30121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant argument/validation ---'
sed -n '4828,4882p' scripts/gc_root_dominance_check.py | cat -n
printf '%s\n' '--- stale result handling ---'
sed -n '3342,3370p' scripts/gc_root_dominance_check.py | cat -n
printf '%s\n' '--- stale self-test budget case ---'
sed -n '3988,4008p' scripts/gc_root_dominance_check.py | cat -n
printf '%s\n' '--- deterministic parser behavior probe (no repo execution) ---'
python3 - <<'PY'
import ast, inspect
from pathlib import Path
p = Path('scripts/gc_root_dominance_check.py')
src = p.read_text()
mod = ast.parse(src)
fn = next(n for n in mod.body if isinstance(n, ast.FunctionDef) and n.name == 'run_statepoints')
print('run_statepoints line:', fn.lineno)
for n in ast.walk(fn):
if isinstance(n, ast.FunctionDef) or isinstance(n, ast.FunctionDef):
pass
if isinstance(n, ast.Name) and n.id == 'max_stale':
print('max_stale reference line:', n.lineno)
if isinstance(n, ast.Attribute) and isinstance(n.value, ast.Name) and n.value.id == 'parser' and n.attr == 'add_argument':
for call in ast.walk(n):
if isinstance(call, ast.Call) and isinstance(call.keywords, list):
kw = {k.arg: ast.literal_eval(k.value) for k in call.keywords}
if kw.get('dest') == 'max_stale':
print('max_stale add_argument:', kw)
print('stale_remaining branch:', any('len(stale_remaining) > max_stale' in ast.get_source_segment(src, n) for n in ast.walk(fn) if hasattr(ast, 'get_source_segment') and isinstance(n, ast.BoolOp)))
cfg_path = Path('.github/workflows')
print('statepoints workflow calls without max-stale:')
for path in cfg_path.glob('*'):
if not path.name.endswith(('.yml','.yaml')):
continue
text = path.read_text()
for i,line in enumerate(text.splitlines(),1):
if '--statepoints' in line and 'scripts/gc_root_dominance_check.py' in text[max(0,text.find(line,i,i+800)):min(len(text),text.find(line,i)+800)]:
# narrow to nearby line
print(f'{path}:{max(1,i-1)}:{min(len(text.splitlines()),i+8)}')
PYRepository: PerryTS/perry
Length of output: 7559
Gate the statepoint stale verdict with --max-stale 0.
--statepoints has no max_stale default, so run_statepoints skips the stale check. The native corpus job only passes --max-unrooted 21, not --max-stale; an uncovered stale hazard prints but exits 0. Add --max-stale 0 to the native statepoint job, or default max_stale to 0 when --statepoints is active, and remove stale allowlist entries if any stale hazards still need triage.
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 3354-3354: Avoid HTML built in strings
Context: render(v)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(html-string-from-parameters)
🤖 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 `@scripts/gc_root_dominance_check.py` around lines 3352 - 3362, Ensure the
native statepoint job invokes the checker with --max-stale 0, or make
run_statepoints default max_stale to 0 whenever --statepoints is enabled.
Preserve explicit --max-stale overrides, and remove any stale allowlist entries
that remain unnecessary after the verdict is enforced.
Source: Coding guidelines
Comment on lines
+259
to
+273
| out="$OUTDIR/${name}__$(basename "$ll")" | ||
| if [ "$LOWERING" = "native" ]; then | ||
| # The production rewrite. A module that `opt` refuses is SKIPPED rather | ||
| # than copied through unrewritten: unrewritten IR parses fine and | ||
| # contains no statepoints, so it would dilute the corpus with files the | ||
| # checker reads as clean. | ||
| if ! "$OPT_BIN" -passes="$RS4GC_PASSES" -S "$ll" -o "$out" 2>"$scratch/opt.err"; then | ||
| rm -f "$out" | ||
| opt_failed=$((opt_failed + 1)) | ||
| opt_failed_names+=("$(basename "$ll") -- $(head -1 "$scratch/opt.err")") | ||
| continue | ||
| fi | ||
| else | ||
| cp "$ll" "$out" | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail the corpus when a native rewrite fails.
Lines 265-269 remove a module from the native corpus and continue. A new opt failure can therefore exclude a changed module from the gated analysis while the aggregate floors still pass. Fail the run, or add an explicit reviewed allowlist for known rewrite failures.
Proposed fix
if [ "$opt_failed" -gt 0 ]; then
printf ' rewrite failed: %s\n' "${opt_failed_names[*]}"
+ exit 1
fi🤖 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 `@scripts/gc_root_dominance_corpus.sh` around lines 259 - 273, Update the
native rewrite failure handling in the corpus-generation flow to fail the run
when the OPT invocation fails, rather than removing the output and continuing.
Preserve the existing diagnostic capture, but ensure opt_failed causes the
overall script to exit nonzero after processing or immediately; alternatively,
enforce an explicit reviewed allowlist for known failures.
Comment on lines
+307
to
+315
| sp="$(grep -ho 'gc\.statepoint\.p0(' "$OUTDIR"/*.ll 2>/dev/null | wc -l | tr -d ' ')" | ||
| live="$(grep -ho '"gc-live"(' "$OUTDIR"/*.ll 2>/dev/null | wc -l | tr -d ' ')" | ||
| echo " statepoints: $sp non-empty live bundles: $live" | ||
| if [ "$sp" -eq 0 ] || [ "$live" -eq 0 ]; then | ||
| echo "::error::the native corpus contains $sp statepoint(s) and $live live" >&2 | ||
| echo "bundle(s). The rewrite ran and produced nothing to check. Either" >&2 | ||
| echo "codegen stopped marking functions gc \"statepoint-example\", or" >&2 | ||
| echo "PERRY_RS4GC=1 no longer selects native roots for this target." >&2 | ||
| exit 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
: > "$tmp/empty.ll"
if bash -c '
set -euo pipefail
count="$(grep -ho "gc\.statepoint\.p0(" "$1"/*.ll 2>/dev/null | wc -l | tr -d " ")"
printf "%s\n" "$count"
' _ "$tmp"; then
echo "expected the current pipeline to exit before its zero-count diagnostic" >&2
exit 1
fiRepository: PerryTS/perry
Length of output: 151
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- file exists and size ---\n'
git ls-files -- scripts/gc_root_dominance_corpus.sh | while IFS= read -r f; do
wc -l "$f"
done
printf '\n--- context lines 290-325 ---\n'
sed -n '290,325p' scripts/gc_root_dominance_corpus.sh | cat -n -v
printf '\n--- shell behavior probe ---\n'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
: > "$tmp/empty.ll"
bash -c '
set -euo pipefail
count="$(grep -ho "gc\.statepoint\.p0(" "$1"/*.ll 2>/dev/null | wc -l | tr -d " ")"
printf "%s\n" "$count"
' _ "$tmp"
printf 'pipeline exit_code=%s\n' "$?"
echo "--- grep -c behavior probe ---"
count="$(grep -hc "gc\.statepoint\.p0(" "$tmp"/*.ll 2>/dev/null || :)"
printf "%s" "$count"
printf "\npipe_status: ${PIPESTATUS[@]} "Repository: PerryTS/perry
Length of output: 242
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- file exists and size ---'
git ls-files -- scripts/gc_root_dominance_corpus.sh | while IFS= read -r f; do
wc -l "$f"
done
printf '%s\n' ''
printf '%s\n' '--- context lines 290-325 ---'
sed -n '290,325p' scripts/gc_root_dominance_corpus.sh | cat -n -v
printf '%s\n' ''
printf '%s\n' '--- shell behavior probe ---'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
: > "$tmp/empty.ll"
bash -c '
set -euo pipefail
count="$(grep -ho "gc\.statepoint\.p0(" "$1"/*.ll 2>/dev/null | wc -l | tr -d " ")"
printf "%s\n" "$count"
' _ "$tmp"
printf '%s\n' "pipeline_exit_code=$?"
printf '%s\n' ''
printf '%s\n' '--- grep -c behavior probe ---'
count="$(grep -hc "gc\.statepoint\.p0(" "$tmp"/*.ll 2>/dev/null || :)"
printf '%s' "$count"
printf '%s\n' " grep_count_exit_code=$?"Repository: PerryTS/perry
Length of output: 2366
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- shell behavior probe ---'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
: > "$tmp/empty.ll"
bash -c '
set -euo pipefail
count="$(grep -ho "gc\.statepoint\.p0(" "$1"/*.ll 2>/dev/null | wc -l | tr -d " ")"
printf "%s\n" "$count"
' _ "$tmp"
printf '%s\n' "pipeline_exit_code=$?"
printf '%s\n' ''
printf '%s\n' '--- grep -c behavior probe ---'
count="$(grep -hc "gc\.statepoint\.p0(" "$tmp"/*.ll 2>/dev/null || :)"
printf '%s' "$count"
printf '%s\n' "grep_count_exit_code=$?"
printf '%s\n' ''
printf '%s\n' '--- grep -hc with no matching files ---'
tmp2="$(mktemp -d)"
trap 'rm -rf "$tmp" "$tmp2"' EXIT
count="$(grep -hc "gc\.statepoint\.p0(" "$tmp2"/*.ll 2>/dev/null || :)"
printf '%s' "$count"
printf '%s\n' "grep_count_nomatchfiles_exit_code=$?"Repository: PerryTS/perry
Length of output: 180
🏁 Script executed:
#!/usr/bin/env bash
set -uo pipefail
printf '%s\n' '--- shell behavior probe ---'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
: > "$tmp/empty.ll"
(
set -euo pipefail
count="$(grep -ho "gc\.statepoint\.p0(" "$tmp"/*.ll 2>/dev/null | wc -l | tr -d ' ')"
printf '%s\n' "$count"
)
status="$?"
printf '%s\n' "pipeline_exit_code=$status"
if [ "$status" -ne 0 ]; then
printf '%s\n' 'pipeline did not finish assignment and printing; it exits under set -euo pipefail'
fi
printf '%s\n' ''
printf '%s\n' '--- grep -hc behavior probe ---'
count="$(grep -hc "gc\.statepoint\.p0(" "$tmp"/*.ll 2>/dev/null || :)"
printf '%s\n' "$count"
printf '%s\n' "grep_count_exit_code=$?"
printf '%s\n' ''
printf '%s\n' '--- grep -hc with no matching files ---'
tmp2="$(mktemp -d)"
: > "$tmp2/other.ll"
(
set -euo pipefail
count="$(grep -hc "gc\.statepoint\.p0(" "$tmp2"/*.ll 2>/dev/null || :)"
printf '%s\n' "$count"
) || printf '%s\n' "grep_count_nomatchfiles_exit_code=$?"Repository: PerryTS/perry
Length of output: 384
Preserve the empty-corpus diagnostic.
With set -euo pipefail, grep -ho exits 1 when the corpus has no matches, so Lines 307-308 exit before the ::error:: message is emitted. Map grep status 1 to a zero count, and retain other grep failures.
🤖 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 `@scripts/gc_root_dominance_corpus.sh` around lines 307 - 315, Update the
statepoint and live-bundle count pipelines in the corpus validation block around
sp and live so no-match grep results become zero under set -euo pipefail, while
preserving failures from other grep errors. Ensure the existing empty-corpus
diagnostic and exit path still runs when either count is zero.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/gc-root-dominance.yml (1)
374-380:⚠️ Potential issue | 🟠 MajorMake
gc-root-dominance-statepointsa required status check before relying on it.The workflow explicitly marks this job
NOT REQUIRED. A native statepoint regression can merge while the required shadow job remains green. The entry inscripts/gc_gate_wiring_check.pyregisters the job for static wiring checks; it does not updatemainbranch protection.Verify the required contexts:
#!/usr/bin/env bash set -euo pipefail repo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner')" gh api "repos/$repo/branches/main/protection/required_status_checks" \ --jq '.contexts[]?' | grep -Fx 'gc-root-dominance-statepoints'As per coding guidelines, CI gates must be included in required branch-protection contexts.
🤖 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 @.github/workflows/gc-root-dominance.yml around lines 374 - 380, Make the gc-root-dominance-statepoints workflow job a required main-branch protection status check, removing the NOT REQUIRED designation and updating repository branch-protection configuration as needed. Verify that the exact context gc-root-dominance-statepoints appears in main’s required status-check contexts; do not rely solely on scripts/gc_gate_wiring_check.py.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.
Duplicate comments:
In @.github/workflows/gc-root-dominance.yml:
- Around line 374-380: Make the gc-root-dominance-statepoints workflow job a
required main-branch protection status check, removing the NOT REQUIRED
designation and updating repository branch-protection configuration as needed.
Verify that the exact context gc-root-dominance-statepoints appears in main’s
required status-check contexts; do not rely solely on
scripts/gc_gate_wiring_check.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7f37b1b-c8b1-4c5d-b1a2-38e6303f174b
📒 Files selected for processing (6)
.github/workflows/gc-root-dominance.ymlchangelog.d/7663-statepoint-root-dominance-checker.mddocs/src/internals/gc-rooting-invariant.mdscripts/gc_gate_wiring_check.pyscripts/gc_root_dominance_check.pyscripts/gc_root_dominance_corpus.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/gc_root_dominance_corpus.sh
- docs/src/internals/gc-rooting-invariant.md
- scripts/gc_root_dominance_check.py
added 5 commits
August 8, 2026 22:40
…ion bundles WIP: --statepoints mode, native-lowering corpus, CI job. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/gc-root-dominance.yml (1)
380-382:⚠️ Potential issue | 🟠 MajorMake
gc-root-dominance-statepointsa requiredmainstatus check.The native statepoint job is not included in
mainbranch protection. A failed native audit can therefore be merged without blocking the branch. Add the exactgc-root-dominance-statepointscontext to the required status checks before relying on this gate.This repeats the unresolved finding from the previous review.
As per coding guidelines,
.github/workflows/*.ymlCI gates must be included in required branch-protection contexts.Verify the repository setting with:
#!/usr/bin/env bash set -euo pipefail repo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner')" gh api "repos/$repo/branches/main/protection/required_status_checks" \ --jq '.contexts[]' | grep -Fx 'gc-root-dominance-statepoints'🤖 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 @.github/workflows/gc-root-dominance.yml around lines 380 - 382, Add the exact gc-root-dominance-statepoints context to the main branch’s required status-check protection settings, ensuring the workflow job is included among mandatory CI gates. Verify the repository branch-protection configuration contains this context.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.
Duplicate comments:
In @.github/workflows/gc-root-dominance.yml:
- Around line 380-382: Add the exact gc-root-dominance-statepoints context to
the main branch’s required status-check protection settings, ensuring the
workflow job is included among mandatory CI gates. Verify the repository
branch-protection configuration contains this context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 179a9b1e-8b22-4037-9bc7-2bc47190fa29
📒 Files selected for processing (3)
.github/workflows/gc-root-dominance.ymlchangelog.d/7663-statepoint-root-dominance-checker.mddocs/src/internals/gc-rooting-invariant.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/src/internals/gc-rooting-invariant.md
- changelog.d/7663-statepoint-root-dominance-checker.md
Contributor
Author
Audit — merging as v0.5.1376This closes plan item 8, and it found a hole in the existing gate on the way. The pre-existing parser bug is the headline
I sabotage-verified it rather than taking the count: reverting
An error message that states the consequence rather than the symptom is worth as much as the fix. A parser that silently skips 7% of a corpus and reports the rest clean is the purest form of the failure this repo keeps paying for — and it was found only because a seeded violation went unreported in a function nothing was parsing. That is the seeded-violations arm doing precisely the job it exists for. The other parser repairs are the same class: My brief was wrong on its central premiseI wrote "under the default it has statepoints and no binds". The The tracked/untracked lineThat LLVM relocates Two of your own sabotages were vacuous when written and you fixed them — the tracked/untracked line had no fixture at all, and The 21 violationsNot allowlisted, ratcheted at Verified here
|
proggeramlug
force-pushed
the
gc/7660-statepoint-static-checker
branch
from
August 8, 2026 20:44
0864c4f to
d81b4d4
Compare
proggeramlug
pushed a commit
that referenced
this pull request
Aug 8, 2026
…#7664) `gc-root-dominance-statepoints`' `--max-unrooted` ratchet goes 21 -> 7. #7663 pointed the root-dominance rule at the NATIVE root lowering -- the one that ships since #7370 -- and reported 21 `unrooted` hazards. Fourteen were shapes `root_reload.rs` looked straight through, because its rule is stated over the load's own register and in both shapes the value at risk lives somewhere else. 1. The root is a GLOBAL, not an alloca (10 hits). A string literal lowers to `load double, ptr @<mod>_.str.N.handle`; the handle global is a registered root, so the string is never swept, and an evacuating cycle REWRITES the global while a register loaded beforehand keeps the pre-move address. #7240's shape, whose fix covered call operands only. 2. The stale register is DERIVED from the load (3 of 7 unmasked receivers). `this.count++` holds the unmasked receiver across the property GET; the load's only use is the bitcast ABOVE the collecting call, so the window was empty and the function took zero reloads. 3. `new.target`'s saved previous value (1 hit). `new.rs` saved `js_new_target_get()` in a bare register across the whole constructor body; the cell is a registered mutable root, so the restore publishes a pre-move address back INTO a root the collector scans. #7226's `prev_this` bug for `new.target`. The reload rule is restated over the value's derivation rather than its register: for a value read out of a collector-rewritten location -- a shadow slot or a string-handle global -- and any value derived from it by pure bit ops, every use a collection point can reach re-materialises the whole derivation. A recipe is extended only through ops that are pure functions of their operands and whose every register operand is already in the same single root's recipe, which makes it self-contained and materialisable anywhere. Each value's window is anchored at its own defining instruction, not at the root load. `new.target` gets `new_target_save`/`new_target_restore` in `crate::rooting`, structurally `implicit_this_save`/`implicit_this_restore`. Re-reading the cell would be the wrong repair: `js_new_target_set` has already overwritten it. Measured on `Counter__increment`: before, all three statepoints carried an EMPTY live set, so the receiver was marked by nothing; after, each carries a "gc-live" bundle and a `gc.relocate`, and the SET reads a mask re-derived from the relocated pointer plus a fresh load of the handle global. Remaining 7, each its own slice: 4 unmasked are phi-mediated (the reload has to go in the predecessor, on the edge); 2 `@perry_global_*` are module-level variables the program assigns, so they need rooting rather than reloading (pinned by `a_module_global_is_not_a_reload_source`); 1 capture read. #7664 stays open as the budget's referent. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
…#7664) `gc-root-dominance-statepoints`' `--max-unrooted` ratchet goes 21 -> 7. #7663 pointed the root-dominance rule at the NATIVE root lowering -- the one that ships since #7370 -- and reported 21 `unrooted` hazards. Fourteen were shapes `root_reload.rs` looked straight through, because its rule is stated over the load's own register and in both shapes the value at risk lives somewhere else. 1. The root is a GLOBAL, not an alloca (10 hits). A string literal lowers to `load double, ptr @<mod>_.str.N.handle`; the handle global is a registered root, so the string is never swept, and an evacuating cycle REWRITES the global while a register loaded beforehand keeps the pre-move address. #7240's shape, whose fix covered call operands only. 2. The stale register is DERIVED from the load (3 of 7 unmasked receivers). `this.count++` holds the unmasked receiver across the property GET; the load's only use is the bitcast ABOVE the collecting call, so the window was empty and the function took zero reloads. 3. `new.target`'s saved previous value (1 hit). `new.rs` saved `js_new_target_get()` in a bare register across the whole constructor body; the cell is a registered mutable root, so the restore publishes a pre-move address back INTO a root the collector scans. #7226's `prev_this` bug for `new.target`. The window is anchored at the ROOT LOAD, not at the derived value. Anchoring at the derivation looks more precise and is wrong: `main`'s class-object read has the scope-end shadow-slot clear landing between the load and the mask, so a walk starting at the mask never sees it and re-read a slot the program had just nulled -- `(makeAnon(77) as any).v` became `undefined`. Caught by an A/B against the branch point on `test_gap_class_expr_identity`, not by the dominance checker, which cannot see a value-correctness bug. The reload rule is restated over the value's derivation rather than its register: for a value read out of a collector-rewritten location -- a shadow slot or a string-handle global -- and any value derived from it by pure bit ops, every use a collection point can reach re-materialises the whole derivation. A recipe is extended only through ops that are pure functions of their operands and whose every register operand is already in the same single root's recipe, which makes it self-contained and materialisable anywhere. Grouping by root load also puts the cost back at O(blocks x loads). `new.target` gets `new_target_save`/`new_target_restore` in `crate::rooting`, structurally `implicit_this_save`/`implicit_this_restore`. Re-reading the cell would be the wrong repair: `js_new_target_set` has already overwritten it. Measured on `Counter__increment`: before, all three statepoints carried an EMPTY live set, so the receiver was marked by nothing; after, each carries a "gc-live" bundle and a `gc.relocate`, and the SET reads a mask re-derived from the relocated pointer plus a fresh load of the handle global. Remaining 7, each its own slice: 4 unmasked are phi-mediated (the reload has to go in the predecessor, on the edge); 2 `@perry_global_*` are module-level variables the program assigns, so they need rooting rather than reloading (pinned by `a_module_global_is_not_a_reload_source`); 1 capture read. #7664 stays open as the budget's referent. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
…#7664) `gc-root-dominance-statepoints`' `--max-unrooted` ratchet goes 21 -> 7. #7663 pointed the root-dominance rule at the NATIVE root lowering -- the one that ships since #7370 -- and reported 21 `unrooted` hazards. Fourteen were shapes `root_reload.rs` looked straight through, because its rule is stated over the load's own register and in both shapes the value at risk lives somewhere else. 1. The root is a GLOBAL, not an alloca (10 hits). A string literal lowers to `load double, ptr @<mod>_.str.N.handle`; the handle global is a registered root, so the string is never swept, and an evacuating cycle REWRITES the global while a register loaded beforehand keeps the pre-move address. #7240's shape, whose fix covered call operands only. 2. The stale register is DERIVED from the load (3 of 7 unmasked receivers). `this.count++` holds the unmasked receiver across the property GET; the load's only use is the bitcast ABOVE the collecting call, so the window was empty and the function took zero reloads. 3. `new.target`'s saved previous value (1 hit). `new.rs` saved `js_new_target_get()` in a bare register across the whole constructor body; the cell is a registered mutable root, so the restore publishes a pre-move address back INTO a root the collector scans. #7226's `prev_this` bug for `new.target`. The window is anchored at the ROOT LOAD, not at the derived value. Anchoring at the derivation looks more precise and is wrong: `main`'s class-object read has the scope-end shadow-slot clear landing between the load and the mask, so a walk starting at the mask never sees it and re-read a slot the program had just nulled -- `(makeAnon(77) as any).v` became `undefined`. Caught by an A/B against the branch point on `test_gap_class_expr_identity`, not by the dominance checker, which cannot see a value-correctness bug. The reload rule is restated over the value's derivation rather than its register: for a value read out of a collector-rewritten location -- a shadow slot or a string-handle global -- and any value derived from it by pure bit ops, every use a collection point can reach re-materialises the whole derivation. A recipe is extended only through ops that are pure functions of their operands and whose every register operand is already in the same single root's recipe, which makes it self-contained and materialisable anywhere. Grouping by root load also puts the cost back at O(blocks x loads). `new.target` gets `new_target_save`/`new_target_restore` in `crate::rooting`, structurally `implicit_this_save`/`implicit_this_restore`. Re-reading the cell would be the wrong repair: `js_new_target_set` has already overwritten it. Measured on `Counter__increment`: before, all three statepoints carried an EMPTY live set, so the receiver was marked by nothing; after, each carries a "gc-live" bundle and a `gc.relocate`, and the SET reads a mask re-derived from the relocated pointer plus a fresh load of the handle global. Remaining 7, each its own slice: 4 unmasked are phi-mediated (the reload has to go in the predecessor, on the edge); 2 `@perry_global_*` are module-level variables the program assigns, so they need rooting rather than reloading (pinned by `a_module_global_is_not_a_reload_source`); 1 capture read. #7664 stays open as the budget's referent. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
proggeramlug
added a commit
that referenced
this pull request
Aug 9, 2026
… in the native lowering (#7664) (#7667) * gc: close the strhandle, derived-mask and new.target unrooted hazards (#7664) `gc-root-dominance-statepoints`' `--max-unrooted` ratchet goes 21 -> 7. #7663 pointed the root-dominance rule at the NATIVE root lowering -- the one that ships since #7370 -- and reported 21 `unrooted` hazards. Fourteen were shapes `root_reload.rs` looked straight through, because its rule is stated over the load's own register and in both shapes the value at risk lives somewhere else. 1. The root is a GLOBAL, not an alloca (10 hits). A string literal lowers to `load double, ptr @<mod>_.str.N.handle`; the handle global is a registered root, so the string is never swept, and an evacuating cycle REWRITES the global while a register loaded beforehand keeps the pre-move address. #7240's shape, whose fix covered call operands only. 2. The stale register is DERIVED from the load (3 of 7 unmasked receivers). `this.count++` holds the unmasked receiver across the property GET; the load's only use is the bitcast ABOVE the collecting call, so the window was empty and the function took zero reloads. 3. `new.target`'s saved previous value (1 hit). `new.rs` saved `js_new_target_get()` in a bare register across the whole constructor body; the cell is a registered mutable root, so the restore publishes a pre-move address back INTO a root the collector scans. #7226's `prev_this` bug for `new.target`. The window is anchored at the ROOT LOAD, not at the derived value. Anchoring at the derivation looks more precise and is wrong: `main`'s class-object read has the scope-end shadow-slot clear landing between the load and the mask, so a walk starting at the mask never sees it and re-read a slot the program had just nulled -- `(makeAnon(77) as any).v` became `undefined`. Caught by an A/B against the branch point on `test_gap_class_expr_identity`, not by the dominance checker, which cannot see a value-correctness bug. The reload rule is restated over the value's derivation rather than its register: for a value read out of a collector-rewritten location -- a shadow slot or a string-handle global -- and any value derived from it by pure bit ops, every use a collection point can reach re-materialises the whole derivation. A recipe is extended only through ops that are pure functions of their operands and whose every register operand is already in the same single root's recipe, which makes it self-contained and materialisable anywhere. Grouping by root load also puts the cost back at O(blocks x loads). `new.target` gets `new_target_save`/`new_target_restore` in `crate::rooting`, structurally `implicit_this_save`/`implicit_this_restore`. Re-reading the cell would be the wrong repair: `js_new_target_set` has already overwritten it. Measured on `Counter__increment`: before, all three statepoints carried an EMPTY live set, so the receiver was marked by nothing; after, each carries a "gc-live" bundle and a `gc.relocate`, and the SET reads a mask re-derived from the relocated pointer plus a fresh load of the handle global. Remaining 7, each its own slice: 4 unmasked are phi-mediated (the reload has to go in the predecessor, on the edge); 2 `@perry_global_*` are module-level variables the program assigns, so they need rooting rather than reloading (pinned by `a_module_global_is_not_a_reload_source`); 1 capture read. #7664 stays open as the budget's referent. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix * chore: bump version to 0.5.1382 Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
The checker reported the verbatim pre-#7453 code as clean in every GATED mode — including `--statepoints`, added in #7663 precisely because the other three could not read the lowering that ships. Re-planting #7453's exact code in `expr/url_main.rs` and running every mode over the 16 URL-lowering sources, both lowerings: mode clean sabotaged --moving-only (dominance) 0 0 --unrooted-allocas --moving-only 0 0 --stale-registers --moving-only 2 2 --statepoints --moving-only 2 2 --stale-registers (unfiltered) 24 35 --statepoints (unfiltered) 15 21 Every gated arm blind, both unfiltered arms not: `js_url_coerce_string` is in ALLOC_RE but not in POLL_CAPABLE_RUNTIME, so `--moving-only` drops the window. The one name takes the sabotaged arms to 13 and 8. `--audit-alloc-re` and `--audit-poll-capable` both hunt a NAME WITH NO REFERENT. The new `--audit-poll-reach` hunts a REFERENT WITH NO NAME: a symbol ALLOC_RE matches whose runtime body reaches a POLL_CAPABLE_RUNTIME symbol without being listed. Not "every poll-capable symbol must be listed" (297 call one directly — a coverage change with its own hit count), but "the checker's two lists must not disagree about the same symbol". 77 found, all listed. Shown able to fail: deleting js_url_coerce_string reddens the audit; neutering _strip_noncode reddens the decoy fixture; making the reach one-hop instead of a fixpoint reddens the transitive fixture. Both non-vacuity floors are asserted. Budgets: curated --stale-registers --moving-only 9 -> 13 (pinned 39, unchanged); native --statepoints --moving-only 7 -> 11, re-pinned with all four new hits named. No hit disappeared.
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
The checker reported the verbatim pre-#7453 code as clean in every GATED mode — including `--statepoints`, added in #7663 precisely because the other three could not read the lowering that ships. Re-planting #7453's exact code in `expr/url_main.rs` and running every mode over the 16 URL-lowering sources, both lowerings: mode clean sabotaged --moving-only (dominance) 0 0 --unrooted-allocas --moving-only 0 0 --stale-registers --moving-only 2 2 --statepoints --moving-only 2 2 --stale-registers (unfiltered) 24 35 --statepoints (unfiltered) 15 21 Every gated arm blind, both unfiltered arms not: `js_url_coerce_string` is in ALLOC_RE but not in POLL_CAPABLE_RUNTIME, so `--moving-only` drops the window. The one name takes the sabotaged arms to 13 and 8. `--audit-alloc-re` and `--audit-poll-capable` both hunt a NAME WITH NO REFERENT. The new `--audit-poll-reach` hunts a REFERENT WITH NO NAME: a symbol ALLOC_RE matches whose runtime body reaches a POLL_CAPABLE_RUNTIME symbol without being listed. Not "every poll-capable symbol must be listed" (297 call one directly — a coverage change with its own hit count), but "the checker's two lists must not disagree about the same symbol". 77 found, all listed. Shown able to fail: deleting js_url_coerce_string reddens the audit; neutering _strip_noncode reddens the decoy fixture; making the reach one-hop instead of a fixpoint reddens the transitive fixture. Both non-vacuity floors are asserted. Budgets: curated --stale-registers --moving-only 9 -> 13 (pinned 39, unchanged); native --statepoints --moving-only 7 -> 11, re-pinned with all four new hits named. No hit disappeared.
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
The checker reported the verbatim pre-#7453 code as clean in every GATED mode — including `--statepoints`, added in #7663 precisely because the other three could not read the lowering that ships. Re-planting #7453's exact code in `expr/url_main.rs` and running every mode over the 16 URL-lowering sources, both lowerings: mode clean sabotaged --moving-only (dominance) 0 0 --unrooted-allocas --moving-only 0 0 --stale-registers --moving-only 2 2 --statepoints --moving-only 2 2 --stale-registers (unfiltered) 24 35 --statepoints (unfiltered) 15 21 Every gated arm blind, both unfiltered arms not: `js_url_coerce_string` is in ALLOC_RE but not in POLL_CAPABLE_RUNTIME, so `--moving-only` drops the window. The one name takes the sabotaged arms to 13 and 8. `--audit-alloc-re` and `--audit-poll-capable` both hunt a NAME WITH NO REFERENT. The new `--audit-poll-reach` hunts a REFERENT WITH NO NAME: a symbol ALLOC_RE matches whose runtime body reaches a POLL_CAPABLE_RUNTIME symbol without being listed. Not "every poll-capable symbol must be listed" (297 call one directly — a coverage change with its own hit count), but "the checker's two lists must not disagree about the same symbol". 77 found, all listed. Shown able to fail: deleting js_url_coerce_string reddens the audit; neutering _strip_noncode reddens the decoy fixture; making the reach one-hop instead of a fixpoint reddens the transitive fixture. Both non-vacuity floors are asserted. Budgets: curated --stale-registers --moving-only 9 -> 13 (pinned 39, unchanged); native --statepoints --moving-only 7 -> 11, re-pinned with all four new hits named. No hit disappeared.
proggeramlug
added a commit
that referenced
this pull request
Aug 9, 2026
…7679) * gc: teach gc-root-dominance the referent-with-no-name hole (#7616) The checker reported the verbatim pre-#7453 code as clean in every GATED mode — including `--statepoints`, added in #7663 precisely because the other three could not read the lowering that ships. Re-planting #7453's exact code in `expr/url_main.rs` and running every mode over the 16 URL-lowering sources, both lowerings: mode clean sabotaged --moving-only (dominance) 0 0 --unrooted-allocas --moving-only 0 0 --stale-registers --moving-only 2 2 --statepoints --moving-only 2 2 --stale-registers (unfiltered) 24 35 --statepoints (unfiltered) 15 21 Every gated arm blind, both unfiltered arms not: `js_url_coerce_string` is in ALLOC_RE but not in POLL_CAPABLE_RUNTIME, so `--moving-only` drops the window. The one name takes the sabotaged arms to 13 and 8. `--audit-alloc-re` and `--audit-poll-capable` both hunt a NAME WITH NO REFERENT. The new `--audit-poll-reach` hunts a REFERENT WITH NO NAME: a symbol ALLOC_RE matches whose runtime body reaches a POLL_CAPABLE_RUNTIME symbol without being listed. Not "every poll-capable symbol must be listed" (297 call one directly — a coverage change with its own hit count), but "the checker's two lists must not disagree about the same symbol". 77 found, all listed. Shown able to fail: deleting js_url_coerce_string reddens the audit; neutering _strip_noncode reddens the decoy fixture; making the reach one-hop instead of a fixpoint reddens the transitive fixture. Both non-vacuity floors are asserted. Budgets: curated --stale-registers --moving-only 9 -> 13 (pinned 39, unchanged); native --statepoints --moving-only 7 -> 11, re-pinned with all four new hits named. No hit disappeared. * chore: bump version to 0.5.1391 Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the gap
docs/src/internals/gc-rooting-invariant.mdrecords as its firstblind spot and
gc_root_dominance_corpus.shapologises for in a paragraph:scripts/gc_root_dominance_check.pygains a--statepointsmode that readsgc.statepointrelocation bundles, andgc_root_dominance_corpus.shgains--lowering nativeto emit a corpus it can read. Both shadow modes areunchanged and still gated.
One correction to the premise
The brief and the existing in-tree comment both say the corpus "contains 1251
statepoints" under the default lowering. It contains zero.
--trace llvmdumps what codegen emitted, and codegen does not emit statepoints — it emits
ptr addrspace(1)root allocas plus agc "statepoint-example"functionattribute, and LLVM's
rewrite-statepoints-for-gcinserts the safepoints later,in the linker step (
perry-codegen/src/linker.rs::maybe_rs4gc_preprocess).Measured on a default-lowering trace: 0
gc.statepointinstructions, 0"gc-live"bundles, only thedeclarelines. (1251 is roughly the count ofgc "statepoint-example"function attributes, which is a different thing.)So the native corpus is the traced IR plus the production rewrite. The
corpus script runs
optwith the pass string it reads out ofSTATEPOINT_REWRITE_PASSESinperry-codegen/src/inprocess.rs, and refusesto run if it cannot read that const — a reproduction of a pipeline that has
silently drifted is a corpus about nothing. That single-sourcing is the price of
not adding a compiler flag; the alternative (a new
--tracestage that dumpspost-RS4GC IR) was rejected because it only fires on the non-in-process path and
puts corpus generation inside the object cache.
Design, and what was rejected
A mode, not a second script. The statepoint IR is structurally different —
no binds, roots declared per-callsite, explicit relocation — but the CFG
builder, Cooper/Harvey/Kennedy dominance,
between_blocks,ALLOC_RE,POLL_CAPABLE_RUNTIME,ROOT_READ_CALLS,REWRITTEN_LOAD_RE, theIMMOVABLE_SOURCESexemptions, the allowlist and the liveness floors are allthe same question asked of a different lowering. This file already records what
a second derivation costs:
REWRITTEN_LOAD_REwas effectively defined twice,the two modes disagreed about what a collector-rewritten load is, and the
narrower one was wrong — which is #7240. Measured here too: wiring the #7210
exemptions through removed 187 of 326 hits on a probe corpus, every one of them
a population #7210 had already adjudicated.
The invariant. Under native roots a value is a root at a safepoint iff it is
in that
gc.statepoint's"gc-live"bundle, and its identity below is thegc.relocateresult. So the rule becomes no register naming a GC object may beused below a safepoint unless it is the relocated value.
The line that does the work is tracked vs untracked. LLVM relocates
ptr addrspace(1)SSA values and rewrites their dominated uses, so those arenever stale. Everything else is invisible to it — and Perry NaN-boxes, so a
JSValue spends most of its life as a
double. Getting that line wrong in theloose direction is not a false positive but a wall of them: a cast-closure that
walked through
inttoptr … to ptr addrspace(1)reached the relocationphiat the top of every loop and came back out below it, reporting the relocated
value as stale — 21 of the first 29 hits, all correct code.
Two verdict classes, because they have two different fixes:
unrootedstaleTwo things this mode gets that the shadow modes cannot.
NONCOLLECTINGisnot consulted at all — LLVM already decided which calls are safepoints and put
the answer in the IR, so a wrong entry in that hand-kept list cannot hide a
hazard here (a statepoint over
js_write_barrier_root_nanbox, which genuinelycannot collect and is in
NONCOLLECTING, still counts). And every safepointnames its wrapped callee, so
--moving-onlyclassifies against the real symbol.A pre-existing parser bug, found by a seeded violation that went unreported
DEFINE_REwas^define\s+.*?@([\w.$]+)\(. LLVM quotes an identifier itcannot print bare, and
$forces it — so every representation-selectionspecialisation is printed
@"name$typed_f64". The regex could not match acrossthe quotes,
curstayedNone, and the entire function body was skippedwithout a word, in a parser that raises
MalformedIRelsewhere precisely toprevent silent skips.
175 of 2452 defines (7.1%) in the native corpus, and not a random 7%: repsel
specialisations are where this page says a representation change moves the
rooting obligation. Zero in the shadow corpus — perry's own writer prints those
names bare — so the existing shadow arms were never affected. Fixed, and a
definethe parser cannot name now raises instead of being skipped.Found only because
--seeded-violationsreported40 planted, 39 caught, 1 MISSEDand the missed one was in a function nothing was parsing.Proof the gate can fail
--seeded-violations 40on the real corpus. Splices a safepoint between areal
ptrtoint ptr addrspace(1)and its real use, in perry-plus-RS4GC output,and requires every one to be reported. 40 planted, 40 caught, 0 missed.
The spliced callee is
js_gc_loop_safepoint, so the planted hazard survives--moving-only— the configuration the gate ships with, not a laxer one.It runs unconditionally, not after an early return on real violations:
the run whose "can it still fail" arm you most want is the one already telling
you something. (The bind-anchored path's seeder still sits below its return;
--seeded-violationsis now a usage error on the two modes that have noseeder, which it was silently ignored by before.)
--self-testred and naming itsown arm: the callee reader, the live-set paren nesting, invoke-continuation
folding, quoted define names, the tracked/untracked line, comment stripping,
the GC: the remaining unrooted-alloca hazards after #7207 — class-keys pointer caches, interleaved staging arrays, inlined-callee param slots #7210 exemption carry-over, the rooted/unrooted split,
AS1_DEF_RE's phialternative, and the
--min-statepointsfloor. Two arms were vacuous whenfirst written and are now not — the tracked/untracked line had no fixture at
all (added: a relocation-phi round trip), and the
--min-statepointsfloor waspassing via
--min-relocates, so it is now asserted with the other two floorsrelaxed to 0.
"gc-live"operand isptr addrspace(1)by construction, so one whose definitionAS1_DEF_REcannotclassify is a parser gap and is an error. It fired immediately on first run:
262 unrecognised operands, all
%.N = phi ptr addrspace(1) […], becauseaddrspace\(1\)\bhas no word boundary between)and the following space.Exactly the dead-alternative failure
--audit-alloc-reexists for, in adifferent regex.
Liveness floors, and why each number
Corpus as of this commit: 149 modules, 2452 functions, 30033 safepoints,
17478 with a non-empty live bundle, 40759 relocations, 34511 (safepoint, root)
pairs. Floors sit well below that and far above "something compiled":
--min-statepointsgc "statepoint-example"--min-relocatesoptexits 0 on a module with nothing to do)--min-live-bundles--min-files/--min-funcs--min-bindscannot do this job: the native corpus has zero binds byconstruction, so a bind floor rejects every valid corpus for this mode and
accepts none. Each mode refuses the other's corpus, asserted both ways in
--self-testand rehearsed against both real corpora:The corpus script asserts its own subject too: zero statepoints or zero live
bundles is an error at generation time, not a clean verdict downstream.
What it found: 21 real hazards, and they are known shapes
--statepoints --moving-onlyover the curated corpus: 21 hazards, allunrooted, 0stale. Unfiltered (the RECLAIM half of the invariant as wellas the MOVE half): 1444 — 1123
unrooted, 321stale.The 21 fall into four shapes, every one of which this repo has a name for:
strhandle— a string-literal handle global loaded into a register andheld across
js_closure_call1or the generic property-GET dispatch. This isfix(gc): root every argument of a cross-module direct call — registry --help 28/30 → 30/30 #7240's exact shape; the fix landed for the shadow lowering
(
OperandProtection::Reload) and these say it does not cover the native one.unmasked— a receiver unmasked out ofptr addrspace(1)anddereferenced below a property GET. This is GC: #7154's residual is NOT fixed — the loop-polls config is red 0/30, and stock zod alone fails 5/40 #7280 / zod's
clone, the shapethat faults deterministically under
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800.global— a@perry_global_*module variable read into a register andheld across
js_closure_call1.rootread—js_new_target_get()→js_new_target_set()across aconstructor call. Structurally identical to fix(gc): root the interned typeof strings, the rawJSON key, and every saved implicit
this#7226'sjs_implicit_this_setprev_this, fornew.targetinstead ofthis.capture— a closure capture cell read held acrossjs_number_coerce.Full
-voutput is in the job log and is uploaded as an artifact on failure.These are not suppressed into an allowlist. They are a population under
triage, so the gate carries
--max-unrooted 21— a ratchet that can only belowered, the same call
--stale-registersmakes and for the same stated reason(21 tombstones with no issue numbers would be worse documentation than one
number that can only go down).
--max-stale 0holds the other class at zero.#7664 enumerates all 21 by shape and is the budget's referent — a number
with nothing behind it is exactly what CLAUDE.md says a threshold decays into.
Each shape wants its own fix and its own decrement before promotion.
Not promoted to required
gc-root-dominance-statepointsis a separate job — separate corpus,separate floors, and, the reason that matters, a separate branch-protection
context, so it can be promoted without dragging the shadow arms along. It is
deliberately not in branch protection: a gate that has never been green
blocks every open PR the day it becomes required, which is the corollary in
CLAUDE.md this repo has already paid for once. Let it run on
mainfirst.Promotion is an admin action and a separate decision; the doc says so.
The one
|| truein the file is on the unfiltered census step, which is adiagnostic and says so where it runs — it keeps the filtered number honest, so
that "the gated arm reads zero" can be told apart from "the filter ate
everything".
Rehearsal of the exact CI commands
Both jobs' command lines, run locally against freshly built corpora, exit status
taken from the checker itself and never through a pipe:
The new job is registered in
scripts/gc_gate_wiring_check.py'sGATESin thissame change — an unregistered sibling is precisely how the job above spent
months being the checked one while the lowering it reads stopped shipping. The
census step carries an explicit
continue-on-error: trueso that gate reads its|| trueas a declared opt-out rather than a swallowed failure.Gates
22 lint commands extracted from
test.yml, pluscargo fmt --all -- --check,the checker's four static arms, the workflow YAML parse and the corpus script's
syntax check: 28 commands, 0 failing.
cargo check --all-targetsrc=0,cargo test -p perry-codegen --lib --no-fail-fast725 passed / 0 failed(including all 14
native_root_coveragetests),cargo test -p perry-runtime --lib --no-fail-fast1917 passed / 0 failed.error[count 0 in every log andRunning unittestspresent in both test logs.No regression in the shadow arms, rehearsed against a freshly built shadow
corpus at this commit: bind-anchored
--moving-only0 violations over 2452functions / 149 modules / 9799 root stores;
--unrooted-allocas --moving-only0 over 7862 gc-capable allocas;
--stale-registers --moving-only23,inside the existing budget of 39.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation