fix(ci): pin the root-dominance corpus to the shadow-stack lowering - #7415
Merged
Conversation
added 2 commits
August 5, 2026 07:25
The GC Root Dominance gate has been unable to return a verdict since #7370 made statepoints the default. The checker's entire vocabulary is `call void @js_shadow_slot_bind(...)`. Under the stack-map lowering the final IR pass resolves those indices to native allocas and REMOVES the calls (FunctionCodegen::stack_map_slot_count), so the corpus compiled 144 modules containing zero root stores. The gate reported violations: 0 and then refused to pass, because its --min-binds floor caught that its own subject never ran. That is CLAUDE.md's fourth hazard working exactly as designed. Pinning PERRY_RS4GC=0 is sound rather than a dodge: #7340 split the root-set analysis from its lowering, this gate is about the analysis, and both backends share it. The shadow stack is also still the production lowering wherever the runtime cannot walk frames. Measured, same binary and source, only the knob differing: arm=default js_shadow_slot_bind calls = 0 arm=rs4gc0 js_shadow_slot_bind calls = 9 #7370 already fixed this for the unit tests -- helpers.rs records that eight broke when the default flipped and were given NativeRootsPin::shadow(). The corpus shell scripts were the same breakage in another idiom, missed.
|
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 changelog records the GC Root Dominance corpus configuration. It documents pinning ChangesGC Root Dominance corpus
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
✨ 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 |
This was referenced Aug 5, 2026
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
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.
GC Root Dominancehas been unable to return a verdict since #7370 made statepoints the default. Last greena3b35664c, first red148f97bc2= #7370.It is gate breakage, not a dominance violation
The checker's entire vocabulary is
call void @js_shadow_slot_bind(...). Under the stack-map lowering, the final IR pass resolves those indices to native allocas and removes the calls —FunctionCodegen::stack_map_slot_countsays so directly. So the corpus compiled fine and contained zero of the thing being counted.The
--min-bindsfloor did exactly its job: it caught that the gate's own subject never ran, rather than reporting a cleanviolations: 0. CLAUDE.md's fourth hazard, working as designed.The fix, and why pinning is sound
Both corpus scripts now set
PERRY_RS4GC=0. #7340 split the root-set analysis from its lowering; this gate is about the analysis, which both backends share. The shadow stack also remains the production lowering wherever the runtime cannot walk frames.PERRY_RS4GCis already in the object-cache key, so no stale-object risk.Measured — same binary, same source, only the knob differing:
This exact breakage was already fixed once
#7370 handled it for the unit tests —
helpers.rsrecords that eight tests broke when the default flipped and were givenNativeRootsPin::shadow(). The corpus shell scripts were the same breakage in a different idiom, and were missed.Summary by CodeRabbit