Repository navigation
fix(gc-check): loads from constant globals are not heap-value sources - #13020
Merged
Merged
Conversation
A string-literal handle global is defined `constant` and holds a pointer to an executable string image the collector never moves. After -Os hoists such a load above a safepoint, the checker reported the register as unrooted: 97 findings on tssrc's Project unit, identical before and after #12999. Loads whose pointer operand is a module-level `constant` global are no longer treated as heap-value sources; `global` handles keep the #7240 classification. The self-test adds positive cases and a negative control (the same handle declared `global` is still reported).
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 1
✨ Finishing Touches
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 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.
Problem
After #13017, running
scripts/gc_root_dominance_check.pyon tssrc's fully optimizedProjectunit with zero budgets still reported 97 unrooted values and 0 stale ones. The findings are identical on main before and after #12999.Cause
All 97 are
strhandleloads from string-literal handle globals. Each handle global is definedconstantand points at an executable string image, which the collector never moves or reclaims (string_pool/image.rs, runtimeis_image_string).-Oshoists these loads above safepoints:i64hoisted out of a loop or regiondoublehoisted the same wayA register loaded from a
constantglobal can't go stale relative to that global. If the object could move, the global itself would be stale. So these registers aren't heap-value sources. The standard pmdom corpora don't hit this because they run only always-inline, mem2reg, sccp and dce before RS4GC.Change
The checker collects each module's
constantglobal definitions and doesn't treat a load whose pointer operand is one of them as a heap-value source. This applies in both statepoint and stale-register modes.globalhandles (dylib, staticlib, Mach-O runtime-boxed images, TLS strings) keep the #7240 classification.Verification
--self-testpasses. The new positive cases fail without the fix. The negative control, the same handle declaredglobal, still reports 1 unrootedstrhandle.Not covered: on macOS the handles are mutable globals pointing to the same immovable images, so a fully optimized Mac IR check would still flag them. That needs a separate exemption grounded in the runtime's guarantee.