Commit 61c263f
* gc: enumerate the runtime-side GC-pointer holders, with a gate (#7231)
A `thread_local!` or `static` in perry-runtime / perry-stdlib that stores a
pointer into the GC heap IS a GC root, and the collector only knows that if
something registers it. Nothing static could find the class: the one static
checker this repo has reads emitted LLVM IR, and a runtime table is not in it.
#7226, #7239, #7268 and #7274 were all found by hand, each re-deriving the same
sweep. This is that sweep as something that can fail.
`scripts/gc_runtime_root_holders.py`:
1. Enumerates every static-shaped declaration in the two crates whose type can
hold a GC pointer. Rule A: the type names a heap header or `JSValue`.
Rule B: an integer/`f64` cell that some function in its own file both names
and allocates in — which is the only way to catch `CACHED_ENV: Cell<f64>`,
the highest-impact holder in the issue's original report.
2. COMPUTES coverage instead of trusting names. Registered scanners are read
from every `gc_register_*root_scanner*(...)` call; a call graph over both
crates is walked from them, and a holder counts as covered when its name
appears in a reachable function DEFINED IN THE SAME FILE. `REGISTRY`,
`SLOTS`, `ROOTS`, `STATES` and `CACHED` each name several different holders
here, so a name-only match certifies the wrong one; the graph walk is what
finds holders a scanner reaches through an accessor (`cp_live_lock()`,
`get_closure_props()`, `buffer_props()`).
3. Requires a written verdict for the rest, in
`scripts/gc_runtime_root_holders.json`. An unclassified holder fails, and so
does an entry that no longer matches — which is what makes a fix delete its
own exemption.
Current state: 81 holders, 47 reached by a registered scanner, 30 classified.
The inventory records 11 `covered_elsewhere` (the gate's known false positives,
each naming the scanner that covers it), 15 `not_a_gc_pointer`, 1 `test_only`,
1 `unverified`, and two `open_gap`s the sweep found and nothing tracked:
* `json/mod.rs` `PARSE_KEY_RING` — a hot-key mirror of the ROOTED
`PARSE_KEY_CACHE`. A move rewrites one copy and not the other. Narrow: the
keys are longlived/old-gen, so only old-gen defrag can move them.
* `perf_hooks.rs` `PERF_ENTRY_KEYS_ARRAY` — a nursery `keys_array` address
compared by identity and never rewritten. Stale ⇒ a silent slow path, or a
match against a newly-allocated array recycled into the address.
Three bugs found while building it, all in the checker rather than the tree,
and all of the "green because it matched nothing" shape:
* string literals were not stripped, so brace counting swallowed
`scan_raw_json_key_root_mut` and reported `RAW_JSON_KEY` — which that scanner
visits three lines below its declaration — as uncovered;
* the registration regex captured only the FIRST argument, so
`gc_register_mutable_root_scanner_named("name", scanner)` registered nothing
and six worker_threads holders read as uncovered;
* rule B keyed on the file rather than the function and reported 544 holders,
four fifths of them counters — a gate nobody would read.
`--self-test` plants a covered holder, one reached only through an accessor, one
uncovered per rule, and a same-named decoy in another file, and asserts each
classification; then asserts the verdict machinery can go red (empty inventory
⇒ everything unclassified; an entry matching nothing ⇒ stale; an entry for a
COVERED holder ⇒ stale). Live sabotage: planting an unrooted
`Cell<*mut ObjectHeader>` into `regex.rs` fails the real scan.
The docstring names what the gate CANNOT see — `RuntimeState`'s fields (not
declarations; a field-count floor makes growth loud), integer holders whose file
never allocates, cross-file scanners, and whether a "covered" holder is covered
CORRECTLY (the #7239 three-of-four-slots shape). It bounds the population; it
does not audit semantics.
* changelog: 7695 runtime root holder gate
* gc: two ways the holder gate could not fail (#7231, CodeRabbit review)
Both are the hazard this script exists to catch, in the script itself.
1. Bare-name reachability could certify the WRONG module's holder.
`bodies` is keyed on the bare function name, so two modules defining
`scan_roots_mut` share a key — and registering one made the other's body
reachable, marking a holder in that module covered when nothing scans it.
Not hypothetical: `scan_tls_roots_mut` is defined in BOTH perry-runtime and
perry-stdlib, and `worker_threads` has several `scan_*_roots_mut` siblings.
The registration text carries the module path
(`crate::json::raw_json::scan_raw_json_key_root_mut`), so the ROOT set is now
resolved to a defining file; a name that resolves to several definitions must
match the path. Deeper hops stay bare-name — nothing in the text says which
module a call resolved to — and the docstring's "cannot see" section now says
that out loud rather than leaving it as an assumption.
Self-test: two modules define `scan_dup_roots_mut`, only one is registered,
and the unregistered module's holder must read UNCOVERED. Sabotage-verified —
reverting to bare-name reachability fails that case and only that case.
2. `apply_inventory` accepted any object carrying a matching (file, name).
No `verdict`, an invented `verdict`, an empty `why`, a `covered_elsewhere`
naming no scanner, an `open_gap` citing no issue — each silenced a holder,
and a suppression whose justification cannot be read or checked is a mute
button rather than a decision record.
`inventory_problems` now validates the vocabulary, requires a `why` long
enough to be a reason, requires `scanner` on `covered_elsewhere` and `issue`
on `open_gap`, rejects duplicates, and caps `unverified` at 2 so the one
verdict that classifies nothing cannot quietly become the whole inventory.
The self-test plants one malformed entry per rule and requires each to be
rejected.
It found seven of my own entries with reasons too thin to check
("Monotonic counter.", "Same six-slot loop.") on its first run. Those are
rewritten.
Gate output is unchanged: 81 holders, 47 reached by a registered scanner, 30
classified.
* gc: delete the SHAPE_CACHE open_gap entry — #7694 fixed it
The gate said so itself: an entry that no longer matches an open gap is stale
and fails the build, which is what makes the inventory shrink-only.
Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
* chore: bump version to 0.5.1402
Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
---------
Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 69c5435 commit 61c263f
7 files changed
Lines changed: 1215 additions & 79 deletions
File tree
- .github/workflows
- changelog.d
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
217 | 217 | | |
218 | 218 | | |
219 | 219 | | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
220 | 237 | | |
221 | 238 | | |
222 | 239 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
251 | 251 | | |
252 | 252 | | |
253 | 253 | | |
254 | | - | |
| 254 | + | |
0 commit comments