fix(gc): refuse a forwarding walk out of, or into, a non-object, and give every rekeyed table a death story (#8174) - #8196
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 (26)
📝 WalkthroughWalkthroughThis change validates GC forwarding sources and targets, adds registry-driven pruning for rekeyed side tables, introduces an audit manifest and required lint check, removes obsolete async-step closure state, and expands regression coverage. ChangesGC forwarding validation and side-table custody
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GCCollector
participant ForwardingValidator
participant DeadOwnerPruner
participant RekeyAudit
GCCollector->>ForwardingValidator: validate forwarding source and target
ForwardingValidator-->>GCCollector: rewrite address or refuse forwarding
GCCollector->>DeadOwnerPruner: prune registered dead-key side tables
DeadOwnerPruner-->>GCCollector: retain live entries
RekeyAudit->>RekeyAudit: scan rekey sites and validate manifest
RekeyAudit-->>GCCollector: return audit status
Possibly related issues
Possibly related PRs
Suggested reviewers: ✨ 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
force-pushed
the
gc/8174-rekeyed-table-registry
branch
from
August 16, 2026 09:19
d8bd654 to
3052a1a
Compare
proggeramlug
changed the title
This was referenced Aug 16, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Aug 16, 2026
Co-authored-by: Ralph Küpper <ralph@skelpo.com> Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
added 4 commits
August 16, 2026 11:28
`GC_FLAG_FORWARDED` means "the first payload word is where this object moved to". Both forwarding walkers — `rewrite_raw_addr` and `verify::try_rewrite_raw_addr` — trusted that byte for ANY address in a known heap region, and trusted whatever word they found behind it. That is safe for a slot the collector already proved is a live reference. It is not safe for a METADATA KEY: `visit_metadata_*` rewrites a recorded heap address if it moved and deliberately does NOT mark it, so the object can die and the arena can recycle the address. #8040, instrumented: recycled bytes at a dead FUNCTION_CLASS_IDS key presented `gc_flags = 0x86` (FORWARDED set by coincidence) and `obj_type = 104`, a type id no `GcTypeInfo` entry exists for. Its "forwarding pointer" was a NaN-boxed value; the walk could not classify the next hop, stopped, and RETURNED it — and the caller masked it to 48 bits into a live, unrelated survivor. #8168 removed that dead key; this closes the following. Two discriminators, one at each end of the hop (`gc/forwarding.rs`): * `forwarding_walk_header` refuses to read a forwarding pointer out of an address that does not read back as a real arena object header. This is NOT the `self.ptrs.classify()` gate `rewrite_raw_addr` documents as having un-rekeyed legitimate `shapes.entries` keys — that one narrows on SPACE as well; the header test does not. * `accept_forwarding_target` refuses a target that is not the start of a heap object, so a bogus word cannot become the answer by virtue of the walk merely stopping at it. Both apply to the verifier too: it panics whenever it can rewrite a slot the rewrite pass left alone, so tightening one walker alone would have turned a silent corruption into an abort blaming an innocent scanner. Refusals are counted and reported under `PERRY_GC_DIAG=1` only when non-zero. The structural half. `gc::dead_owner` is the real fix for this class — drop the entry before its dead key can be walked — and its fan-out was a hand-written list nothing checked. `DEAD_KEY_PRUNES` is now the registry `fan_out` iterates, and `scripts/gc_rekeyed_key_tables.py` (wired into `lint`) requires a written verdict for all 38 `visit_metadata_*` sites: a `dead_owner:` verdict must name a registered prune, a new site fails, and an exemption matching nothing fails too. Six tables have no prune and no rooting; they are declared and capped (#8190-#8195) so the count can only go down. Co-authored-by: Ralph Küpper <ralph@skelpo.com> Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
The aggregate counter says a stale key reached a rewrite walk. It does not say WHICH TABLE, which is the whole distance between "there is a bug of the #8040 shape somewhere" and a fix — #8040 itself took days to attribute. `pin::CopyingWalkPhaseGuard` already names the scanner around every rewrite-pass walk; tally refusals against it and print the breakdown in the `[gc-forwarding]` line. Co-authored-by: Ralph Küpper <ralph@skelpo.com> Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
) The #8174 registry gate's first run enumerated every `visit_metadata_*` site in the tree and asked each the same question #8168 had to answer by hand: when this key's object dies, what removes the entry? Six tables had no answer. That is live #8040 exposure in six places — a rekeyed table's dead key is not a leak, it is an address the arena recycles and the next rewrite pass reads as a GcHeader. Fixed rather than exempted, so the manifest lands with ZERO declared gaps and MAX_OPEN_GAPS = 0: CONSOLE_INSTANCES prune_dead_console_instance_owners #8190 BOXED_PRIMITIVE_PAYLOADS prune_dead_boxed_primitive_payload_... #8191 TRANSITION_CACHE_GLOBAL prune_dead_transition_cache_entries #8192 ASYNC_STEP_GUARD.last_closure field DELETED #8193 REFLECT_METADATA.target_bits prune_dead_reflect_metadata_targets #8194 SYMBOL_ACCESSOR_PROPERTIES folded into the symbol-property prune #8195 #8193 is not a prune. `last_closure` held the closure that took the last erroring async step, for a same-closure check DELETED when #712/#921/#922 showed a runaway loop alternates between two closures. Nothing has read it since — but it was still a raw heap address the promise scanner rekeyed without marking, and nothing pruned it. Maintaining state nobody reads is its own dead code, so the field goes, and with it the PROMISE_SCAN_ASYNC_STEP_GUARD budgeted phase whose only slot it was. #8195 is not a new prune either: the accessor table shares its owner key with SYMBOL_PROPERTIES and SYMBOL_PROPERTY_ATTRS, both pruned since the 2026-07-09 audit, and was simply left out. It now takes the same pass's memoized owner verdict. That also closes a leak — a dead owner's accessor closures were immortal. Each new prune has a pair of cases: the prune FIRES (dead owner, one collection, the table observably shrinks) and its inverse (a rooted owner's entry survives). The transition-cache case allocates its rooted `next_keys` in OLD-GEN on purpose — a reachable neighbour in the dead array's own nursery block would force-mark it (#7975) and the prune would correctly decline, which would have read as a failure of the prune. cargo test -p perry-runtime --lib: 2514 passed / 0 failed / 4 ignored. Co-authored-by: Ralph Küpper <ralph@skelpo.com> Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
Co-authored-by: Ralph Küpper <ralph@skelpo.com> Claude-Session: https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
proggeramlug
force-pushed
the
gc/8174-rekeyed-table-registry
branch
from
August 16, 2026 09:28
1a000c5 to
d0dd732
Compare
proggeramlug
marked this pull request as ready for review
August 16, 2026 09:29
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 #8174, #8190, #8191, #8192, #8193, #8194, #8195.
What was wrong
GC_FLAG_FORWARDEDmeans "the first payload word is where this object moved to". Both forwarding walkers —CopyingNurseryCollector::rewrite_raw_addrandgc::verify::try_rewrite_raw_addr— trusted that byte for any address in a known heap region, and trusted whatever word they found behind it.For a slot the collector already proved is a live reference, both are safe. For a metadata key neither is.
RuntimeRootVisitor::visit_metadata_usize_slotand its siblings rewrite a recorded raw heap address if a moving collection forwarded it, and deliberately do not mark it — the key is a side table's key, not a reference the program can reach, so rooting it would leak. The price is that the key's object can die and the arena can recycle the address under it.#8040 is what that looks like, instrumented: recycled payload bytes at a dead
FUNCTION_CLASS_IDSkey presentinggc_flags = 0x86(GC_FLAG_FORWARDEDset by coincidence),obj_type = 104— a type id noGcTypeInfoentry exists for — and a "forwarding pointer" that was really a NaN-boxed value (0x7FFF…). The walk followed it, could not classify the next hop, stopped and returned it, andvisit_metadata_nanbox_keymasked it to 48 bits into a live, unrelated survivor. #8168 removed that one dead key. This closes the following, and then removes the remaining dead keys.1. Two discriminators, one at each end of the hop (
gc/forwarding.rs, new)forwarding_walk_headerrefuses to read a forwarding pointer out of an address that does not read back as a real arena object header (plausible_gc_header: registeredobj_type, sane size,GC_FLAG_ARENA). [Next.js/dylib] Full production App Route compatibility tracker #8040's bytes fail onobj_type = 104. Every real forwarding source passes —set_forwarding_addressoverwrites one payload word and ORs one flag bit, and all four production installers (copying::move_young, promotion,gc::oldgendefrag,array::push_pop's growth stub) operate on arena objects.This is not the
self.ptrs.classify()gate thatrewrite_raw_addr's own doc records as having un-rekeyed legitimateshapes.entrieskeys and turned the verifier red. That one additionally narrows on SPACE and resolves the survivor thread-locals; the header test carries none of that.plausible_gc_headeris already the acceptance testCopyingPointerSet::classify_arenaapplies to every arena pointer the collector classifies, so nothing it rejects was ever an object the collector could have moved.accept_forwarding_targetrefuses a target that is not the start of a heap object, so a bogus word can no longer become the answer by virtue of the walk merely stopping at it. Off-arena it still accepts a malloc'd array-growth target, but only above the handle band and belowHEAP_MAX— which is what the0x7FFF…word fails.Both are applied to the verifier too.
try_rewrite_raw_addris whatRuntimeRootVisitMode::Verifyruns, and it panics whenever it can rewrite a slot the rewrite pass left alone; tightening one walker alone would have converted a silent corruption into aPERRY_GC_VERIFY_EVACUATIONabort blaming an innocent scanner.Refusals are counted and, under
PERRY_GC_DIAG=1, reported only when non-zero asThe
by_walkbreakdown comes frompin::CopyingWalkPhaseGuard, which already names the scanner around every rewrite-pass walk. The aggregate says a stale key reached a rewrite walk; the breakdown says whose, which is the whole distance between "there is a bug of the #8040 shape somewhere" and a fix — #8040 itself took days to attribute.2. The structural half
gc::dead_owneris the real fix for this class: drop the entry before its dead key can be walked. Its fan-out covered a dozen tables, #8168 made it thirteen, and nothing checked the list was complete.DEAD_KEY_PRUNES(gc/dead_owner.rs) is now the registryfan_outiterates: 19 entries, each naming the tables it prunes and which of the pass's three deadness predicates it takes.scripts/gc_rekeyed_key_tables.py, wired intolint(a required context), enumerates all 37visit_metadata_*sites inperry-runtime/perry-stdliband requires a written verdict for each inscripts/gc_rekeyed_key_tables.json.What the gate rejects
dead_owner:<fn>naming a prune not inDEAD_KEY_PRUNESself_pruned:<fn>naming a function that does not existopen_gapverdict at all (MAX_OPEN_GAPS = 0)--self-testplants twelve shapes (every row above, plus anopen_gapwithout an issue number, plus a doc comment that must not count as a site, plus a correctly-classified tree that must pass) and requires the checker to adjudicate each. It runs in the samelintstep, before the real scan.3. What the audit found — all six fixed, not exempted
The gate's first run turned up six more rekeyed tables with no death story. Rather than declare them, they are fixed, so the manifest lands with zero gaps and
MAX_OPEN_GAPS = 0.CONSOLE_INSTANCESprune_dead_console_instance_ownersBOXED_PRIMITIVE_PAYLOADSprune_dead_boxed_primitive_payload_ownersTRANSITION_CACHE_GLOBAL(prev_keys,key_ptr)prune_dead_transition_cache_entriesASYNC_STEP_GUARD.last_closureREFLECT_METADATA.target_bitsprune_dead_reflect_metadata_targetsSYMBOL_ACCESSOR_PROPERTIES(owner half)prune_dead_symbol_property_owners#8193 is not a prune.
AsyncStepGuard::last_closureheld the address of the closure that took the last erroring async step, for a same-closure check that was deleted when #712/#921/#922 showed a runaway loop alternates between two closures. Nothing has read it since. It was not inert, though — it was a raw heap address the promise scanner rekeyed without marking, and nothing pruned it. Writing a prune to maintain state nobody reads is its own dead code, so the field goes, and with it thePROMISE_SCAN_ASYNC_STEP_GUARDbudgeted phase whose only slot it was.#8195 is not a new prune either. The accessor table shares its owner key with
SYMBOL_PROPERTIESandSYMBOL_PROPERTY_ATTRS, both pruned since the 2026-07-09 audit, and was simply left out. It now takes the same pass's memoized owner verdict, so all three agree about every owner. That also closes a leak — a dead owner's accessor closures were immortal.Tests
gc/tests/forwarding_target_validation.rs, 5 cases. The two sabotage cases plant #8040's shape verbatim and assert the premise first — the address classifies as heap, the byte carriesGC_FLAG_FORWARDED, and104is not a registered type — so a green run says the discriminator works rather than that nothing was tried. The premise case asserts the opposite direction: a genuine evacuation still rewrites and neither refusal counter moves, which is the property the rejectedclassify()-based tightening broke. The registry case assertsDEAD_KEY_PRUNEShas not shrunk, its labels are unique, and #8168'sFUNCTION_CLASS_IDSentry is still present with itsGC_TYPE_CLOSUREnarrowing.gc/tests/dead_owner_side_tables.rs, 10 new cases. Each new prune gets a pair: the prune fires (dead owner, one collection, the table observably shrinks) and its inverse (a rooted owner's entry survives — a prune that drops live entries is worse than the stale key it removes). The transition-cache case allocates its rootednext_keysin OLD-GEN on purpose: a reachable neighbour in the dead array's own nursery block would force-mark it (#7975) and the prune would correctly decline, which would have read as a failure of the prune.Local validation
End-to-end, under the moving collector. A churn fixture (60 rounds × 120 objects, dropping all but a 3-round window) that exercises exactly the surfaces this touches — varied shapes, symbol-keyed properties, accessor descriptors,
Map/Set+ iterators, synthetic classes via plain-function prototypes, closure dynamic props, proxies with agettrap,Reflect.get, promises — run underretired_set[gc-forwarding]#3604#5319#3604scripts/gc_evacuation_liveness_assert.pypasses on all three, so the subject was live rather than "nothing threw". Zero[gc-forwarding]lines is the load-bearing number: on a healthy workload that hammers every rekeyed surface, the new discriminators refuse nothing, i.e. no legitimate rewrite was lost.Suites (against
bfb0707be):cargo test -p perry-runtime --lib— 2514 passed / 0 failed / 4 ignoredcargo test -p perry --bin perry— 987 passed / 0 failedcargo test -p perry-codegen --no-fail-fast— 1483 passed / 9 failed, the same 9 by name asmain(this crate does not depend onperry-runtime)Gates:
cargo fmt --all -- --check,check_file_size.sh,gc_runtime_root_holders.py,gc_store_site_inventory.py,gc_pin_sites.py(+--self-test),gc_gate_wiring_check.py,raw_handle_debt.py(990, unchanged),shape_descriptor_census.py,addr_class_inventory.py,check_gc_env_knobs.py,check_test_registration.py,class_id_collisions.py,workspace_architecture.py --check,check_gc_doc_claims.py,check_locale_independent_io.py, and the newgc_rekeyed_key_tables.py(+--self-test) — all clean.check_thread_locals.pyis red onmainfor three files this branch does not touch (dyn_eval/interp.rs,module_require.rs,node_vm.rs); the new counters usecrate::perry_thread_local!and add no fourth.scripts/gc_pin_sites.pygains one allowlist entry: the plantedgc_flags = 0x86carries bit 2, but nothing is being pinned — the address is payload interior of a live allocation with no object at it, and rewriting the byte as named flags would misreport what #8040 actually observed.Not fixed here
#8163 is unaffected and stays open. Retested on
53e8a21e3before this branch: the production Next App Route fixture's forced-evacuation arm still fails withTypeError: value is not a function(243 copying minors, 117,579 objects copied, 0 verify panics, normal arm green) — details on the issue. This branch narrows what a stale key can be followed into; the holder losing that closure is a different defect.https://claude.ai/code/session_01AHvBYz7E6wWKv8kmvLLGpj
Summary by CodeRabbit
Bug Fixes
Tests
Chores