Repository navigation
GC: the remaining unrooted-alloca hazards after #7207 — class-keys pointer caches, interleaved staging arrays, inlined-callee param slots #7210
Description
Summary
The remaining #7202-class hazards after PR #7207, from the same bare-alloca enumeration (140 sites, 23 HAZARD / 117 SAFE). #7207 fixed the inline-constructor this slot; #7208 and #7209 own the closure this/new.target slots and the catch parameter. This tracks the rest so they stop being invisible.
1. An unrooted cached copy of a value whose original is a registered root (3 sites)
-
function.rs:521(entry_init_load_global) — caches the@perry_class_keys_*pointer in an entry alloca for the whole function.codegen/string_pool.rs:464-477(GC: old-page defrag unsound on conservative-scan cycles — un-rewritten referrer of moved old objects (class-keys global suspected) #5042) explicitly says that array "lives in the longlived (old-gen) arena … so old-page defrag (C4b) can relocate it", which is why the global was registered withjs_gc_register_global_root. This second copy silently opts out. Split by consumer:new.rs:621,new.rs:705,scalar_method.rs:553dereference it (passing it askeystojs_object_alloc_class_inline_keys) — real hazard;method_override.rs:213andproperty_get/dynamic_dispatch.rs:105onlyicmp eqit as a shape token — fail-safe.This is the one violation
gc_root_dominance_check.py --unrooted-allocasstill reports after fix(gc): close the three residual root-store holes — the PutValueSet key, js_object_assign_one, and the inline-ctor this slot #7207 on its reproducer corpus (not moving-reachable there, becausejs_array_allocis the only call in that window). -
stmt/loops.rs:1505— adoublecache of a module global aliased intoctx.locals[gid]for both copies of a packed-f64 range loop. -
stmt/loops.rs:4766— a hoistedarr.length; narrow, but the hoist classifier imposes no type gate, so aNamed-typed receiver whoselengthis a string lands a pointer here.
2. Staging arrays filled interleaved with lowering (9 sites)
Shape: for a in args { let v = lower_expr(ctx, a)?; store v into buf[i] } — argument i's NaN-boxed pointer is already sitting in the unrooted buffer while argument i+1 is lowered, and lowering allocates.
- ★
codegen/helpers.rs:1275—vals_bufstages every exported binding's value forjs_create_namespace.js_closure_alloc_singleton(:1300,:1341) and an arbitrary user getter (:1315) run inside the loop; N GC pointers accumulate across N allocating calls, thenjs_create_namespace(:1366) allocates while reading all N. expr/call_spread.rs:473,expr/instance_misc1.rs:1035,lower_call/extern_func.rs:1161/:1197/:1249,lower_call/namespace_call.rs:68/:107/:141.
setTimeout(resolve, d, someObject) with two or more heap arguments is the reachable shape. expr::temp_root::lower_exprs_rooted (#6951) exists for exactly this and has ~7 callers repo-wide.
3. Staging arrays with a call between the stores and the consumer (3 sites)
lower_call/property_get/dynamic_dispatch.rs:1315—js_object_get_own_field_or_undef(:1351, a prototype/shape-walking read) sits between the stores andjs_native_call_value/js_native_call_method. Widest blast radius: the codegen: split large modules into parallel codegen units (bound clang peak memory; remove the single-TU wall) #5391 collapsed instance-dispatch outline.lower_call/console_promise.rs:1336—js_closure_unbox_callee_checked_rebind(:1358), which rebinds a closure's bakedthiscapture slot.lower_call/early_branches.rs:244—unbox_str_handle(:262-265), string-key branch only.
4. Inlined-callee locals — structurally unrootable today (4 sites)
lower_call/new_ctor_args.rs:54, stmt/let_stmt.rs:974/:997/:367. These hold arbitrary JS values and go straight into ctx.locals, but their HIR ids belong to a different function, so collect_pointer_typed_locals — which walks only this frame's params and Stmt::Lets — never assigns them a slot. Nothing can bind them even in principle; closing these needs a representation change, not a call.
#6969's temp-root scope keeps the arguments alive across construction, so this is not a premature-sweep bug — it is the "rooted slot, stale copy" split. refresh_rooted_args (new.rs:199-217) does exactly the needed re-read for lowered_args; nothing does it for the param slots.
5. Conditionally safe, worth a standing note (4 sites)
codegen/function.rs:928/:991, expr/buffer_views.rs:272, stmt/let_buffer_views.rs:54 cache handle+8/handle+16 data pointers for a whole function. The in-tree argument (function.rs:962-976) is liveness via the caller's rooted binding plus address stability, because typed arrays are old-arena GC_FLAG_TENURED and gc_type_is_movable(GC_TYPE_TYPED_ARRAY) == false. Sound as written. If any admitted construction form ever nursery-allocates, all four become hazards silently — that invariant deserves an assertion rather than a comment.
Why this matters
Together with #7208 and #7209 these are the reason the #7161 revert cannot be called safe on the strength of #7192/#7198/#7207 alone: they have never been exercised under a moving minor, and gc_root_dominance_check.py --unrooted-allocas (added in #7207) is the instrument that can now find them without waiting for a crash.
Refs #7202, #7207, #7208, #7209, #7154, #7161, #6951, #6968, #6969.
Activity
docs(gc): correct the post-PerryTS#7207 staleness in the rooting-inva…
docs(gc): correct the post-PerryTS#7207 staleness in the rooting-inva…
ci(gc): make the root-dominance gate able to fail, baseline it honest…
proggeramlug commentedon Aug 2, 2026
The 66 triaged — all of them are false positives, and the reason is one line in the checker
gc_root_dominance_check.py --unrooted-allocas --moving-onlyover a freshly generated 128-module corpus at7d1dc9ca2reports 66, every one moving-reachable. Classified by the provenance of the stored value (script: re-runcheck_func_unrooted_allocasand resolve each store'sprovenance()back to its heap source):load i64, ptr @perry_class_keys_*call @js_box_alloc_bits43 of the 66 are in
main; the rest are spread over 20 functions. There is no third class:#7210's §2/§3/§4 sites do not appear here at all (they are excluded by the checker'sescapedrule atgc_root_dominance_check.py:1488-1495, because a staging buffer's address is passed to its consuming call).The 64:
@perry_class_keys_*cannot move under any shipped or matrix configuration#7210§1 calls these "real hazard". They are not, today, and the argument is short:js_build_class_keys_arrayallocates the array withjs_array_alloc_with_length_longlived(object/alloc.rs:337,:319) — the old/longlived arena, never the nursery. The nursery copying minor only relocates nursery objects.select_old_page_defrag_pagesreturnsdefault()unlessPERRY_GC_OLD_DEFRAG=1(gc/oldgen.rs:229-259), disabled under GC evacuation leaves a stale reference to a moved array element (wild-pointer crash / corrupt cached value) #6206 pending a rewrite-contract fix. Not one matrix arm sets it.The
#5042comment that motivated registering the global as a root ("old-page defrag (C4b) can relocate it") is still the right reason to keep that registration — it is future-proofing against #6206 being resolved. But it does not make the cached copy a live hazard, and rooting 64 of them today buys a shadow slot per (class, function) for zero behaviour change.Condition, stated so it can be re-checked: if
PERRY_GC_OLD_DEFRAGever becomes the default, orjs_build_class_keys_arrayever stops using the_longlivedallocator, all 64 become real andentry_init_load_global's slot must be bound (expr::root_entry_allocaworks on it —gc/roots.rs:1344-1349records that both root-slot kinds "accept a heap reference either NaN-boxed or bare", so a raw i64 keys pointer binds correctly).The 2: a box is never relocated
js_box_alloc_bitsusesstd::alloc::alloc(box.rs:69-88), the box is never freed, andscan_box_roots_mutrewrites the JSValue inside the box, not the box's address.expr/literals_vars.rs:706-717already states this invariant in prose for a sibling site.What the checker should learn from this
_is_heap_source(gc_root_dominance_check.py:1456) conflates two different things: "the collector rewrites this location" and "the object this names can move."REWRITTEN_LOAD_REincludesperry_class_keys_\w+because the global is rewritten — but the array it points at is old-arena and immovable, so the copy can never go stale. Until that distinction exists in the tool,--unrooted-allocas --moving-onlycannot reach 0, and #7198's "promote the gate to required after a clean week" clock cannot start. That is the actionable item here, not 66 roots.§2/§3/§4, which the checker cannot see
Audited separately at source level. The headline: the issue predicted staleness and the reproducer produced a crash, for a different operand than the one named.
nextTick— real, and the callback register is the live defect, not (only) the staging buffer.cb_boxwas lowered first and read atjs_timer_validate_callbackafter every argument'slower_expr.setTimeout(cb, 0, {…}, churn())is red 3/3 at base underPERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_HEAP_LIMIT=8:TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received an instance of Object.The buffer is the second window and is worse in kind — a premature sweep, not staleness. Fixed in fix(gc): root the callback and every staged argument of the timer family #7230 with a witness.
setImmediate,process.nextTickand the spread-call buffer were probed with the same shape and are green at base — 1 of 4 reproduced, so the static "9 HAZARDs" reading was over-called.dynamic_dispatch.rs: the cited intervening calljs_object_get_own_field_or_undefis a pure own-keys walk that does not allocate, so the stated claim is refuted; a genuinely unprotected sibling loop exists one frame up (for a in args { push(lower_expr(ctx, a)?) }).console_promise.rs: already fixed by fix(codegen): root the callee,thisand every argument of js_closure_callN #7214'sRootedOperands.early_branches.rs: confirmed,unbox_str_handle→js_get_string_pointer_unifiedallocates on the SSO path.root_entry_allocadoes not go throughcollect_pointer_typed_locals; it callsctx.func.reserve_shadow_slot(), which takes no HIR id and grows the frame on demand at the store site (function.rs:341-376). GC: heap values in plainalloca_entryslots are neither marked nor rewritten (inline-ctorthis_slot, closure-capture staging array) #7202 already used it on the sibling slot in the same inlined-constructor window (lower_call/new.rs:1164). All four are ordinary fixable sites.let_stmt.rs:367additionally has no "different function" problem at all — it is this function's ownStmt::Let, in a branch that early-returns past the normal bind.Deferred from #7230 and worth their own change: the §4 ctor-param slots,
call_spread.rs's regular-arg buffer (needs the callee lowered inside the rooted group without disturbing evaluation order), andcodegen/helpers.rs'svals_buf.fix(gc): root the callback and every staged argument of the timer family
fix(gc): scan setInterval's trailing arguments
fix(gc): root the callback and every staged argument of the timer fam…
proggeramlug commentedon Aug 2, 2026
All 66 are false positives, and the tool's own predicate is the actionable finding.
Triaged over a fresh 128-module corpus at
7d1dc9ca2, classified by each store's heap-source provenance:load i64, ptr @perry_class_keys_*js_build_class_keys_arrayusesjs_array_alloc_with_length_longlived(old arena, never the nursery). Only old-page defrag relocates those, andselect_old_page_defrag_pagesreturns empty unlessPERRY_GC_OLD_DEFRAG=1(off under #6206; no matrix arm sets it).call @js_box_alloc_bitsstd::alloc::alloc, never freed;scan_box_roots_mutrewrites the JSValue inside the box, not its address.§1's "real hazard" label does not survive contact with the allocator. §2/§3/§4 don't appear in the 66 at all (excluded by the checker's
escapedrule) and were audited separately: §3'sdynamic_dispatchclaim is refuted (js_object_get_own_field_or_undefdoes not allocate);console_promisewas already fixed by #7214; §4's "structurally unrootable, needs a representation change" is wrong —root_entry_allocabypassescollect_pointer_typed_localsviareserve_shadow_slot, and #7202 already used exactly that on the sibling slot in the same window.★ The actionable item
One line in
scripts/gc_root_dominance_check.py:_is_heap_sourceconflates "the collector rewrites this location" with "the object this names can move." Until that distinction exists,--unrooted-allocas --moving-onlycannot reach 0, so #7198's promote-to-required clock cannot start. Fixing the predicate — not rooting 66 sites — is what closes this issue.Two conditions worth re-checking if either changes: the 64 become real hazards if
PERRY_GC_OLD_DEFRAG=1ever ships on, and the 2 if boxes ever become GC-heap allocations.fix(gc): root the ten unrooted runtime-side caches, and the scanner t…
fix(gc): root the ten unrooted runtime-side caches, and the scanner t…
fix(gc): split the dominance checker's heap-source predicate by movab…
proggeramlug commentedon Aug 2, 2026
Re-verified on
main=d4ac65f21as part of the sweep in #7262. Still open, with one correction to §1.Confirmed still open
§2's flagship site is unchanged:
codegen/helpers.rs:1275is still a bareblk.emit_raw(format!("{} = alloca [{} x double]", ...)), still filled interleaved withlower_exprand still consumed byjs_create_namespaceat:1374.expr::temp_root::lower_exprs_rootedis not applied to it.Correction to §1: the class-keys bullet is now a known FALSE POSITIVE class, not a hazard
§1 leads with
function.rs:521(entry_init_load_globalcaching the@perry_class_keys_*pointer) and calls it "the one violation--unrooted-allocasstill reports". That is no longer the reading. On a fresh 138-module corpus the mode reports:The 95 class-keys reports are suppressed by an exemption that cites this issue by number, on the ground #7239 established independently:
CLASS_KEYS_BY_IDis safe because every write site uses a_longlivedallocator. So the cached copy cannot go stale under the collector as it exists today.That is a narrower claim than "not a hazard", and the exemption is honest about it —
--assume-old-defragre-reports all 95. The residual exposure is precisely old-page defrag (C4b) relocating the long-lived keys array, which is the scenariocodegen/string_pool.rs:464-477warned about and which motivated registering the global as a root. If C4b defrag is ever enabled for that arena, these 95 sites become live hazards in one step — which is worth an assertion rather than an exemption comment, in the same spirit as §5's standing note about the typed-array data pointers.Net: §1's first bullet should be re-scoped from "real hazard, dereferenced at
new.rs:621/:705/scalar_method.rs:553" to "conditionally exempt, contingent on_longlivedallocation and on C4b defrag staying off". §§2–4 are untouched by this and remain open.Not fixing here — read-only sweep.
proggeramlug commentedon Aug 9, 2026
Re-adjudication against
8d27032c7— every cited line number was staleSource-only. Per-site verdict; paths relative to the repo root.
codegen/function.rs:528entry_init_load_global; derefslower_call/new_alloc.rs:249,:333,lower_call/scalar_method.rs:553; icmp-onlymethod_override.rs:212,property_get/dynamic_dispatch.rs:105new.rs:621/:705→new_alloc.rs:249/:333)gc_root_dominance_check.py:2638IMMOVABLE_SOURCES["class-keys"]suppresses it behind 3 machine probes, re-enabled by--assume-old-defrag, audited in CI atgc-root-dominance.yml:185and:451. §1's own ask — "an assertion rather than an exemption comment" — is already satisfied.stmt/loops.rs:1505alloca_entry(DOUBLE)cachingload double, ptr @perry_global_*intoctx.locals[gid]. Producerpacked_f64_range_loop_invariant_global_reads(:1001-1031) tests onlymodule_globals.contains_key(id)— no type gate.stmt/loops.rs:4777arr.length.classify_for_length_hoist_impl(:5530-5564) matchesproperty == "length"syntactically;array_length_receiver_is_loop_local(:5714-5720) checks slot/box/global/scalar-replacement only — still no type gate.codegen/helpers.rs:1470vals_buf, consumed at:1565alloca [N x double]filled at:1551-1553in a loop callingjs_closure_alloc_singleton(:1494,:1535) and an arbitrary user getter (:1516). Nocrate::rootinganywhere in the function.expr/call_spread.rsregular-arg bufferalloca_entry_array(DOUBLE, regular_count)filled perlower_expr. Pluscb_boxheld across the whole marshal, plus the rawacc_handleaccumulator loop. The accumulator half is fixed by #7696; the buffer andcb_boxare not.expr/instance_misc1.rs:1138rooting::with_operands_rooted; buffer filled in a second lowering-free pass. Comment at:1130cites this issue.lower_call/extern_func.rs→lower_call/extern_timers.rs:213,:245,:283(sharedfill_arg_buffer:73)with_operands_rooted. The migration also found and fixed the 2-argument siblings #7210's own fix missed (extern_timers.rs:20-40).lower_call/namespace_call.rs:113,:146,:178with_operands_rooted+fill_arg_buffer; theunbox_to_i64now sits below the group.property_get/dynamic_dispatch.rs:1341js_object_get_own_field_or_undef(object/object_ops/accessors.rs:109-181) is a pure own-keys walk —js_array_length,js_array_get,js_string_key_matches_bytes(byte compare,string/compare.rs:245-272),js_object_get_field. Zero allocations, so there is no collection point between the stores and the consumer.property_get/dynamic_dispatch.rs:275,285-288recv_box = lower_expr(object)thenfor a in args { static_user_args.push(lower_expr(a)?) }— receiver and each arg in bare registers across every later arg's lowering.lower_call/console_promise.rs:1382with_rooted_group(ctx, args.len() + 2, …); comment at:1361-1370namesjs_closure_unbox_callee_checked_rebind→clone_closure_rebind_thisand re-reads below it.lower_call/early_branches.rs:244alloca_entry_arrayfilled:245-250;unbox_str_handle(:264) isjs_get_string_pointer_unified, which the checker explicitly declines to call non-collecting because its SSO branch callsjs_string_materialize_to_heap(gc_root_dominance_check.py:2170). Also unprotected arg loop:236-238and barerecv_box/key_box:234-235.lower_call/new_ctor_args.rs:52-54;stmt/let_stmt.rs:978,:1001,:371codegen/function.rs:932,:995;expr/buffer_views.rs:267;stmt/let_buffer_views.rs:54§4 is closable today, and the source says so in its own words
The claim was that these locals' HIR ids belong to a different function so
collect_pointer_typed_localscan never slot them. Butreserve_shadow_slot(codegen/function.rs:352-377) takes no HIR id and grows the frame at the store site, androot_entry_alloca(expr/scalar_slot_root.rs:102-142) keys on the alloca's LLVM name string (scalar_slot_shadow_slots: HashMap<String, u32>).let_stmt.rs:472and:861already callroot_scalar_replaced_sloton id-less slots in the same function, and:861is inside the sameifbranch as sites 4b/4c.Site 4d is not even an inlined-callee local:
idis this frame's ownStmt::Let, socollect_pointer_typed_localsdoes reserve it a slot — the POD-record branch justreturn Ok(())s at:432past the bind at:1810, while storing a NaN-boxedjs_object_allochandle (expr/pod_record.rs:360). One line:emit_shadow_slot_bind_for_local(ctx, id)after the store.The only per-site wrinkle is 4a's
boxed_paramarm, which stores anI64box pointer and wants the box-slot treatment rather than theDOUBLEone.§5's premise holds — but not for the reason the in-tree comment gives
gc_type_is_movable(gc/types.rs:619) does returnfalseforGC_TYPE_TYPED_ARRAY/GC_TYPE_BUFFER, buttypes.rs:517states that flag only gates old-page defrag — the nursery copying minor evacuates eden objects regardless. The whole load is carried by old-arena residency, defended by exactly two choke points:typed_array_alloc(typedarray/mod.rs:~97) andbuffer_alloc(buffer/header.rs:478), botharena_alloc_gc_old(...)followed by a hand-writtengc_flags |= GC_FLAG_TENURED. All ~147 construction sites funnel through those two, and nothing nursery-allocates today.Two fragilities worth the assertion §5 asks for:
arena_alloc_gc_olddoes not itself setGC_FLAG_TENURED(a purpose-builtarena_alloc_gc_born_tenuredexists and is unused), and every old-gen residency test is on the large-object path — no test asserts a small typed array is old-gen.IMMOVABLE_SOURCEShas exactly two entries (class-keys,box); there is no typed-array entry, and typed arrays are structurally outside the checker's reach anyway (getelementptris absent fromTRANSPARENT_OPSandtyped_array_alloc\w*was deliberately removed fromALLOC_RE). So the assertion exists for §1 and does not exist for §5.Note on the 0/0
--unrooted-allocas --moving-onlyreadingIt is not evidence for §§2–4: those sites are excluded by the checker's
escapedrule (a staging buffer's address is passed to its consumer), and §4's are unreached because the corpus does not route a movable heap value through an inlined-ctor param under--moving-only.fix(gc): close #7210's two remaining unrooted-alloca sites + #7640 A/…
fix(gc): close #7210's two remaining unrooted-alloca sites + #7640 A/…
fix(gc): close #7210's two remaining unrooted-alloca sites + #7640 A/…
fix(turnloop): root two objects held across allocations; clear three …