gc: fix root-dominance phi false positives + 3 of 5 real hits, lower --max-unrooted to 2 (#7664) - #7724
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds phi-aware native statepoint hazard analysis and tests, roots receivers during static class-method dispatch, documents remaining unrooted captures, lowers the enforcement budget to 2, and updates the project version to ChangesGC root analysis and dispatch rooting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LLVMIR
participant RootDominanceChecker
participant StatepointFixtures
LLVMIR->>RootDominanceChecker: provide phi incoming operands and predecessors
RootDominanceChecker->>RootDominanceChecker: propagate taint across all incoming edges
RootDominanceChecker->>RootDominanceChecker: evaluate predecessor-edge statepoint hazards
StatepointFixtures->>RootDominanceChecker: run safe and hazardous phi-edge cases
RootDominanceChecker-->>StatepointFixtures: report zero or one unrooted hazard
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
added 5 commits
August 9, 2026 19:53
…receiver hazard (#7664) scripts/gc_root_dominance_check.py: the native/--statepoints chain treated a phi as unconditionally transparent, so one tainted incoming edge blanket- tainted the phi's result and a downstream use was checked against ANY CFG path between source and use (between_blocks is deliberately path-insensitive, sound for an ordinary register but not for a phi, whose dynamic value depends on which edge was actually taken). All four reported unmasked hits were the same &&/|| short-circuit join: the tainted edge never crosses a safepoint, the OTHER edge does, and the checker reported that. _cast_closure gains phi_all_edges: a phi joins `chain` only once every incoming edge is independently in it. That closes the false positive and deliberately excludes the case of a single tainted edge with its own intervening safepoint before its predecessor's terminator; _phi_edge_hazard covers that separately, checking each edge's own window. Two new self-test fixtures (phi_safe_edge / phi_hazard_edge) pin both directions, each verified against a sabotaged copy of the checker to confirm it can still fail. lower_call/property_get/static_dispatch.rs: (Lexer as any).lex(...) reads a module-global receiver, then held it raw across arg-bundling logic that can allocate (a rest-param bundle always allocates; an object-literal argument can too) before implicit_this_save/js_static_this_arm_value read the stale copy -- the same #6969/#6986 shape #7719 just fixed in lower_call/builtin.rs, here on the receiver. Wrapped it in RootedGroup::adopt/reread. Re-verified against the current corpus: the checker fix eliminates exactly the four phi false positives with nothing else changing. The static-dispatch fix was not yet re-verified against a fresh corpus run after this rebase (disk pressure and box load made prior corpus runs unreliable) -- see the PR description for exactly what is and isn't confirmed.
…7664) Re-verifying the checker fix found 9 real+false hits, not the 8 the prior snapshot recorded -- test_gap_static_method_value_name_collision joined the population after #7691 without the budget being re-measured. Of the 9: 4 were the checker's own phi-edge false positives (fixed in the prior commit), 3 were unrooted:global (2 already fixed upstream by #7719, 1 fixed in the prior commit's static_dispatch.rs change), and 2 are unrooted:capture -- real, diagnosed, and tracked as this budget's referent rather than rushed. Measured on the native corpus, both arms of --moving-only, stale still 0.
proggeramlug
force-pushed
the
gc/7664-native-unrooted-residue
branch
from
August 9, 2026 18:16
1be0b67 to
d849ff5
Compare
Contributor
Author
Merging as v0.5.1420 — and I verified the one thing you flagged you couldn'tThe headline finding is that the prior triage's own numbers were stale. "8 = 4 phi false positives + 4 real" was itself out of date: a fresh native corpus read 9, because a fifth real hit ( Dropping your own I re-ran the confirmation you explicitly said you had notYou noted your "2 residual hits" came from a build containing your own now-dropped The budget of 2 holds against #7719's fix, not just yours. Two things about my own run worth recording, since both are traps:
Scope, honestly statedThe 2 remaining The checker fix ( Shadow-lowering read is noted as unchecked in your test plan rather than glossed — correct, given the box was briefly disk-full and at load 30-55. Gates 19/19. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/gc_root_dominance_check.py (1)
4356-4368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a "still a live subject" assertion for
phi_safe_edge, as the roundtrip fixture has.The safe-edge arm asserts only that the scan reports 0. That arm passes for the right reason and for several wrong reasons: if
%r2ever stops being classified as a heap source, if the phi stops parsing, or ifphi_incomingreturns an empty list, the fixture still reports 0 and the arm stays green.Lines 4340-4349 already apply the counter-measure to the roundtrip fixture: they assert the fixture's own registers still classify as expected. Apply the same shape here. Assert that
phi_incomingon the join's phi returns two edges, and that one of them names%r2.
phi_hazard_edgedoes not need this, because a non-zero count already proves the subject is live.💚 Proposed addition after the safe-edge arm
ok = False + # ...and the safe fixture must still be a live subject: the join must + # parse as a two-edge phi that really does carry the tainted register, + # or the 0 above is a green for the wrong reason. + sf = parse_file(paths["phi_safe_edge"])[0] + sf_phis = [i for b in sf.blocks for i in sf.insns[b] if _is_phi(i)] + sf_edges = [e for p in sf_phis for e in phi_incoming(p)] + if len(sf_edges) != 2 or not any(v.strip() == "%r2" for v, _p in sf_edges): + print("self-test FAIL: the phi_safe_edge fixture's join must parse " + "as a two-edge phi carrying %r2, or the 0 above proves " + f"nothing. Parsed edges: {sf_edges!r}", file=sys.stderr) + ok = False hits = _scan_statepoints([paths["phi_hazard_edge"]], moving_only=True)🤖 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 4356 - 4368, Strengthen the `phi_safe_edge` self-test by asserting before or alongside the zero-hit check that `phi_incoming` for the join phi returns exactly two incoming edges and includes one naming `%r2`, matching the existing roundtrip fixture validation. Keep the current zero-result assertion unchanged; do not add this assertion to `phi_hazard_edge`.crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs (1)
135-149: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSkip rooting when the receiver is a compile-time constant.
The
_arm producescrate::nanbox::double_literal(...), a literal NaN-boxed INT32 class id. That value is not a heap reference and cannot go stale. Whenhas_restis true,collectsis true, sogroup.adoptroots the literal andgroup.rereadreloads it. The result is a root slot and a store/load pair that protect a constant.The
Expr::ClassRef(_)arm has the same property: the comment at line 99-100 stateslower_expryields the INT32-NaN-boxed class id.This is IR noise, not a bug. The comment at lines 142-146 states the intent to keep the common case free of rooting traffic, so the same reasoning applies here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs` around lines 135 - 149, Update receiver rooting in the static-dispatch lowering around group.adopt so compile-time class-reference receivers produced by the Expr::ClassRef(_) and synthesized _ arms are never marked as collecting. Preserve has_rest rooting for non-constant receivers, but bypass group.adopt rooting and reread traffic for these literal INT32 NaN-box values.
🤖 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 `@crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs`:
- Around line 220-230: Update the rest-bundle construction in the static
dispatch lowering path to use the existing lower_rest_call_args_rooted rooting
pattern. Keep the rest-array accumulator and already-lowered values rooted
across lower_expr calls, reread the accumulator before each push, and append
boxed bundles afterward; include synthetic arguments as an additional array
within the same root group.
---
Nitpick comments:
In `@crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs`:
- Around line 135-149: Update receiver rooting in the static-dispatch lowering
around group.adopt so compile-time class-reference receivers produced by the
Expr::ClassRef(_) and synthesized _ arms are never marked as collecting.
Preserve has_rest rooting for non-constant receivers, but bypass group.adopt
rooting and reread traffic for these literal INT32 NaN-box values.
In `@scripts/gc_root_dominance_check.py`:
- Around line 4356-4368: Strengthen the `phi_safe_edge` self-test by asserting
before or alongside the zero-hit check that `phi_incoming` for the join phi
returns exactly two incoming edges and includes one naming `%r2`, matching the
existing roundtrip fixture validation. Keep the current zero-result assertion
unchanged; do not add this assertion to `phi_hazard_edge`.
🪄 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: 96868c7f-a938-4df3-ab1d-606cb4f9cb39
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/gc-root-dominance.ymlCLAUDE.mdCargo.tomlchangelog.d/7724-native-unrooted-residue.mdcrates/perry-codegen/src/lower_call/property_get/static_dispatch.rsscripts/gc_root_dominance_check.py
Comment on lines
+220
to
+230
| let rest_count = args.len().saturating_sub(fixed_count); | ||
| let cap = (rest_count as u32).to_string(); | ||
| let mut current = ctx.block().call(I64, "js_array_alloc", &[(I32, &cap)]); | ||
| for a in args.iter().skip(fixed_count) { | ||
| let v = lower_expr(ctx, a)?; | ||
| let blk = ctx.block(); | ||
| current = | ||
| blk.call(I64, "js_array_push_f64", &[(I64, ¤t), (DOUBLE, &v)]); | ||
| } | ||
| let rest_box = nanbox_pointer_inline(ctx.block(), ¤t); | ||
| lowered.push(rest_box); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find how the spread accumulator was rooted in `#7690`, for reuse here.
set -euo pipefail
rg -n -C8 'js_array_alloc' crates/perry-codegen/src --type=rust -g '!**/tests/**' \
| rg -n -C8 'RootedGroup|guard_store_operand|StoreOperandGuard|root_operands_begin' || true
rg -n -C10 'accumulator|spread' crates/perry-codegen/src/lower_call --type=rust | rg -n -C10 'Rooted|guard' || trueRepository: PerryTS/perry
Length of output: 17159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant static-dispatch rest-bundle code and the rooted helper if present.
fd -a 'static_dispatch.rs|mod.rs|rooting.rs' crates/perry-codegen/src | sed 's#^\./##' | sort | rg 'crates/perry-codegen/src/(lower_call/(mod\.rs|static_dispatch\.rs)|rooting\.rs)$' || true
echo '--- static_dispatch outline ---'
ast-grep outline crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs --view compact 2>/dev/null || true
echo '--- static_dispatch relevant section ---'
sed -n '160,250p' crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs | nl -ba -v160
echo '--- mod lower_rest_call_args_rooted relevant section ---'
sed -n '280,355p' crates/perry-codegen/src/lower_call/mod.rs | nl -ba -v280
echo '--- all uses of lower_rest_call_args_rooted ---'
rg -n "lower_rest_call_args_rooted" crates/perry-codegen/src -g '*.rs'Repository: PerryTS/perry
Length of output: 327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- static_dispatch relevant section ---'
awk 'NR>=160 && NR<=250 { printf "%5d %s\n", NR, $0 }' crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs
echo '--- mod lower_rest_call_args_rooted relevant section ---'
awk 'NR>=280 && NR<=355 { printf "%5d %s\n", NR, $0 }' crates/perry-codegen/src/lower_call/mod.rs
echo '--- all uses of lower_rest_call_args_rooted ---'
rg -n "lower_rest_call_args_rooted" crates/perry-codegen/src -g '*.rs' || trueRepository: PerryTS/perry
Length of output: 12461
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- static_dispatch imports/outline around lower/static property_get ---'
sed -n '1,120p' crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs
echo '--- relevant surrounding property_get static call path ---'
sed -n '50,280p' crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs
echo '--- all uses of lower_call in static_dispatch / property_get ---'
rg -n "lower_call\\(|PropertyGet|StaticMethod|property_get" crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs || true
echo '--- check for is_static_method_call related symbols in static_dispatch ---'
rg -n "is_static_method_call|lowered |lower_call|is_synth_args|has_rest|declared" crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs || true
echo '--- diff stat/name-only for context ---'
git diff --stat -- crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs || true
git diff --name-only -- crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs || trueRepository: PerryTS/perry
Length of output: 23989
Root the rest-array accumulator while building the rest bundle.
current is the js_array_alloc result held in a raw i64 SSA register. In the non-synthetic rest path, lower_expr runs inside the push loop, so user code can trigger a moving collection while current and already-pushed values remain unrooted. Use the existing lower_rest_call_args_rooted pattern: lower operands under a rooted group, build each rest array after reread, then append the boxed bundles; handle the synthetic arguments bundle as an additional array in the same root group.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs` around
lines 220 - 230, Update the rest-bundle construction in the static dispatch
lowering path to use the existing lower_rest_call_args_rooted rooting pattern.
Keep the rest-array accumulator and already-lowered values rooted across
lower_expr calls, reread the accumulator before each push, and append boxed
bundles afterward; include synthetic arguments as an additional array within the
same root group.
Source: Learnings
proggeramlug
pushed a commit
that referenced
this pull request
Aug 9, 2026
… no-op crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs's three tests were red on main: js_generator_attach_prototype and js_generator_attach_closure_prototype no longer moved their receiver under an alloc-point copying minor, and the shipped-default witness never saw its trigger armed. warm_generator_intrinsics() called js_generator_attach_prototype(TAG_UNDEFINED, 0) to pre-build the generator intrinsic tower before the timed call under test. That never worked: js_generator_attach_prototype returns at its very first line for any non-pointer obj, so the "warm-up" touched nothing. It went unnoticed because GENERATOR_FUNCTION_INTRINSIC_PTR and its five siblings were plain process-global AtomicI64s pre-#7723 - some earlier test in the same binary had almost always already built the tower, so the real call under test found it cached regardless of what warm_generator_intrinsics() did. #7723 converted those six statics to per_test_global! specifically so each test starts from a guaranteed first-touch state (crates/perry-runtime/src/gc/tests/lazy_intrinsic_towers.rs's whole point). That is a correct, deliberate change - it took away the accidental cross-test priming these three tests had been relying on. With nothing pre-built, the real call now pays the dozens-of-allocations tower build itself, inside build_generator_tower's GcSuppressScope (#7251's no-move window for that build). That suppression window swallows the arena trigger the test injected via arm_collection_on_next_block for the rest of the call: no copying minor ever runs before the tower build's own scope closes, and by then intermediate's own allocation no longer needs a new arena block, so the trigger is never serviced. Confirmed with instrumented gc_check_trigger / GcSuppressScope traces comparing the last-good commit against #7723: on the last-good commit the real call's first allocation reaches gc_check_trigger unsuppressed and services the trigger directly; on #7723 the entire ~1800-call tower build runs suppressed first and nothing ever re-triggers afterward. Fix warm_generator_intrinsics() to call crate::object::ensure_generator_intrinsics() directly - the same builder lazy_intrinsic_towers.rs uses - so it does what its name and doc comment always claimed. This does not touch the liveness/deferral assertions those tests make; it only repairs the test's own setup helper. Bisected via git checkout of each of today's three merges in an isolated worktree: c907953 (pre-#7721) passes; ca8c0d6 (#7721, moving-loop poll default flip) passes; cbb682d (#7723, no-move window + per_test_global towers) is the first commit where all three fail. #7724 is uninvolved.
proggeramlug
added a commit
that referenced
this pull request
Aug 9, 2026
… no-op (#7731) * fix(gc): warm_generator_intrinsics must call the tower builder, not a no-op crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs's three tests were red on main: js_generator_attach_prototype and js_generator_attach_closure_prototype no longer moved their receiver under an alloc-point copying minor, and the shipped-default witness never saw its trigger armed. warm_generator_intrinsics() called js_generator_attach_prototype(TAG_UNDEFINED, 0) to pre-build the generator intrinsic tower before the timed call under test. That never worked: js_generator_attach_prototype returns at its very first line for any non-pointer obj, so the "warm-up" touched nothing. It went unnoticed because GENERATOR_FUNCTION_INTRINSIC_PTR and its five siblings were plain process-global AtomicI64s pre-#7723 - some earlier test in the same binary had almost always already built the tower, so the real call under test found it cached regardless of what warm_generator_intrinsics() did. #7723 converted those six statics to per_test_global! specifically so each test starts from a guaranteed first-touch state (crates/perry-runtime/src/gc/tests/lazy_intrinsic_towers.rs's whole point). That is a correct, deliberate change - it took away the accidental cross-test priming these three tests had been relying on. With nothing pre-built, the real call now pays the dozens-of-allocations tower build itself, inside build_generator_tower's GcSuppressScope (#7251's no-move window for that build). That suppression window swallows the arena trigger the test injected via arm_collection_on_next_block for the rest of the call: no copying minor ever runs before the tower build's own scope closes, and by then intermediate's own allocation no longer needs a new arena block, so the trigger is never serviced. Confirmed with instrumented gc_check_trigger / GcSuppressScope traces comparing the last-good commit against #7723: on the last-good commit the real call's first allocation reaches gc_check_trigger unsuppressed and services the trigger directly; on #7723 the entire ~1800-call tower build runs suppressed first and nothing ever re-triggers afterward. Fix warm_generator_intrinsics() to call crate::object::ensure_generator_intrinsics() directly - the same builder lazy_intrinsic_towers.rs uses - so it does what its name and doc comment always claimed. This does not touch the liveness/deferral assertions those tests make; it only repairs the test's own setup helper. Bisected via git checkout of each of today's three merges in an isolated worktree: c907953 (pre-#7721) passes; ca8c0d6 (#7721, moving-loop poll default flip) passes; cbb682d (#7723, no-move window + per_test_global towers) is the first commit where all three fail. #7724 is uninvolved. * changelog: add fragment for #7731 (generator-attach-pacing) * chore: bump version to 0.5.1422 Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
7 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.
Summary
Closes the checker half of #7664 and two of the four real hits underneath it. Re-verification found the population was 5 real hits, not 4 (an off-by-one against the prior triage — see below), and this PR fixes 3 of them plus both checker false positives, leaving 2 open as a follow-up rather than rushed.
What I verified vs. what I'm inferring
--self-test(deterministic, no build needed) passes, including two new sabotage-tested fixtures (phi_safe_edgemust report 0,phi_hazard_edge— byte-identical except the safepoint moves onto the tainted edge — must report exactly 1; each was confirmed to fail when the corresponding fix is reverted). Also ran against the real native corpus (149 modules) on a build that included this fix: the fourunmaskedphi false positives from the prior snapshot are gone, nothing else changed.static_dispatch.rsfix: verified against the real corpus (0 hits fortest_gap_static_method_value_name_collision, down from 1) and a functional smoke test ((Lexer as any).lex(...)/Parser.parsestill produce correct output, no crash).builtin.rs(the other 2unrooted:globalhits): I wrote a fix for these independently, then discoveredorigin/mainhad moved to include fix(gc): root builtin.rs's constructor arguments across each other's lowering (#6986) #7719, which fixes the identical shape (module-global constructor argument held across a sibling argument's allocation) across a superset oflower_call/builtin.rs's arms via the sameRootedGroupmechanism. I dropped my version in favor of it and rebased cleanly (no conflicts). I have not re-run the corpus against the exact post-rebase commit — the corpus run showing 2 residual hits (bothunrooted:capture) used a build with my own now-droppedbuiltin.rsfix, not fix(gc): root builtin.rs's constructor arguments across each other's lowering (#6986) #7719's. Since fix(gc): root builtin.rs's constructor arguments across each other's lowering (#6986) #7719 covers the same shape with the same mechanism, I expect the same reading post-rebase, but this PR's owngc-root-dominance-statepointsCI run is the actual confirmation, not something I'm asserting here.unrooted:capturehits: diagnosed in detail (see the workflow comment and the changelog fragment), not fixed.js_closure_get_capture_bits's return value is never re-entered into either the RS4GC-tracked domain or a temp root by the generic "read a captured value" call sites — narrower than the original triage's "signature/ABI change" guess, but still a real slice of work (root_reload.rs'sFactsonly models loads as reloadable sources today, not calls). Filing as a follow-up issue rather than rushing it.The checker fix
scripts/gc_root_dominance_check.py'schain(the untracked cast-closure a stale use is searched in) treatedphias unconditionally transparent — one tainted incoming edge blanket-tainted the phi's result, and a downstream use of that result was checked against any CFG path between source and use. That's sound for an ordinary register but not a phi, whose dynamic value depends on which edge was actually taken. All four false positives were the same&&/||short-circuit join: the tainted edge never crosses a safepoint, the other edge does._cast_closuregainedphi_all_edges: a phi joinschainonly once every incoming edge is independently in it (a worklist retry on each operand's own arrival, so admission order doesn't matter). That deliberately gives up the single-tainted-edge-with-its-own-safepoint case;_phi_edge_hazardcovers that separately, checking each edge's window against its own predecessor's terminator instead of the join.Budget
gc-root-dominance-statepoints'--max-unrootedgoes 8 → 2. The full accounting (all 9 hits a fresh corpus actually reads before this PR, why the prior "8" was already stale, and the diagnosis for both remaining hits) is in the workflow comment and the changelog fragment.Test plan
python3 scripts/gc_root_dominance_check.py --self-test— clean, including two new fixtures each confirmed via a sabotaged copy of the checker to still be able to failcargo check -p perry-codegen— clean--statepoints --moving-only: 4 phi false positives gone,test_gap_static_method_value_name_collisionhit gone, 2unrooted:capturehits remain exactly as diagnosed (run predates the fix(gc): root builtin.rs's constructor arguments across each other's lowering (#6986) #7719 rebase — see above)static_dispatch.rschange (test_gap_static_method_value_name_collision.ts, no oracle diff run — output shape looked correct, not byte-compared against node)gc-root-dominance-statepointsCI run)--lowering shadowcorpus (attempted twice; both runs hit unrelated compile flakiness on a heavily-loaded, briefly disk-full shared box — did not get a clean read before wrap-up)Summary by CodeRabbit
Bug Fixes
Documentation
Chores