Skip to content

GC: heap values in plain alloca_entry slots are neither marked nor rewritten (inline-ctor this_slot, closure-capture staging array) #7202

Closed

Description

@proggeramlug

Summary

The third way the root-store dominance invariant breaks, and the one neither #7184 nor #7192 addresses: a heap value living in a plain alloca_entry slot that is neither a shadow slot nor a temp root, so the collector neither marks nor rewrites it.

#7184 fixed a root store whose slot index fell outside the pushed shadow frame (silently bounds-checked into a no-op). #7192 fixed a root store emitted in-frame but after a collection point. This one has no root store at all — the value is in unrooted stack memory for the whole window.

The two known instances

1. this_slot on the inline-constructor path — crates/perry-codegen/src/lower_call/new.rs

let this_slot = ctx.func.alloca_entry(DOUBLE);
ctx.block().store(DOUBLE, &obj_box, &this_slot);
ctx.this_stack.push(this_slot);

Every this read inside the inlined constructor body loads from that alloca. alloca_entry (crates/perry-codegen/src/function.rs:421) emits a bare alloca; nothing binds it to a shadow slot and nothing pushes it as a temp root. If the instance relocates part-way through the body — a field initializer that calls a function containing a loop is enough under PERRY_GC_MOVING_LOOP_POLLS=1 — every subsequent this.x = … writes into abandoned from-space memory.

Reachability is the default, not an opt-in. force_ctor_call requires class.constructor.is_some(), so any class with fields or heritage but no own constructor (class C { payload = mk() }, class C extends B {}) takes the inline path with PERRY_INLINE_CTOR unset.

Its sibling ctor_result_slot had the same defect and is fixed in #7198 by never letting it carry an address (it now starts at undefined, so js_ctor_return_override maps fall-through to the re-read this). That fix does not generalise to this_slot, which has to hold the address by construction.

The fix is not local: this is read at arbitrary points through ctx.this_stack, so making it relocation-safe means routing Expr::This through a root rather than an alloca — a representation change for this_stack with its own tests, plus every other consumer (this_slot_for_err, closures capturing this).

2. The [N x i64] closure-capture staging array

Same shape: alloca_entry_array storage holding capture values across the lowering that fills it.

Why it is hard to see

There is no crash and often no wrong answer. this_slot and the old ctor_result_slot were stale together, so the whole construction operated consistently on the from-space copy and the caller received a from-space object with all its fields present. The program is correct-looking and holds a dangling pointer — exactly why #7154's from-space scan only ever saw offenders one or more cycles after the target died, with correct layout coverage.

scripts/gc_root_dominance_check.py does not model it either: it anchors on js_shadow_slot_bind, and there is none. Extending it needs the general stale-register invariant (any register or unrooted alloca holding a heap value that is live across a collecting call and then used without being re-read from a root). A prototype on top of the shipped CFG/dominance layer reported ~1000 raw hits on the sfw-registry corpus before allowlist tuning, so NONCOLLECTING needs extending before it is actionable.

Why it matters

Along with #7200 and #7201, this is why PERRY_GC_MOVING_LOOP_POLLS=1 cannot become the default again — i.e. why #7161 cannot be reverted and #7019's minor-GC RSS win stays reverted.

Refs #7154, #7161, #7184, #7192.

Activity

  1. proggeramlug commented on Aug 1, 2026

    @proggeramlug
    ContributorAuthor

    The enumeration this issue asks for is done, and the named site is fixed in #7207.

    The enumeration

    140 bare-alloca sites across crates/perry-codegen at origin/main (3a983ca6a) — 63 alloca_entry, 42 alloca_entry_array, 2 alloca_entry_bytes_aligned, 19 blk.alloca, 14 raw emit_raw. No other alloca-emitting spelling exists. Classified 23 HAZARD / 117 SAFE; the full table with per-site disposition is in changelog.d/7207-residual-root-store-holes.md.

    class n disposition
    inline-ctor this slot (lower_call/new.rs:1109) 1 fixed in #7207 — shadow slot
    closure this / new.target slots 2 HAZARD → #7208
    catch (e) parameter slot 1 HAZARD → #7209
    class-keys pointer caches, staging arrays, inlined-callee param slots 19 HAZARD → #7210
    explicitly rooted 15 SAFE
    no collection window 46 SAFE
    non-GC (i32/i1/index/counter/handle/POD) 52 SAFE
    PTR-into-heap, safe by a runtime invariant 4 SAFE conditionally — see #7210 §5

    Two corrections to this issue's text worth recording:

    The this_slot fix

    Bound as a shadow slot via expr::scalar_slot_root::root_entry_alloca (this issue's own suggestion — #6968's grow-on-demand machinery, generalized). Binding rather than routing Expr::This through a temp root is what makes it a one-line change at ~30 read sites: every reader already does load DOUBLE, ptr <slot>, and js_shadow_slot_bind records slot_ptrs[idx] = alloca, so evacuation rewrites it in place. Gated on construction_runs_user_code — the same predicate that decided the instance needed a temp root — so a class with no constructor, fields or heritage keeps its previous IR exactly, frame size included.

    No runtime reproducer, and why

    test-files/test_gap_gc_inline_ctor_this_rooting.ts is green at base, and ships saying so. The stale write is currently absorbed: the class-field inline guard reads obj_type/class_id/keys_array off the stale pointer, the evacuation's forwarding record fails that guard, and the runtime fallback resolves forwarding — so the write lands on the moved object after all. That masking holds only while the from-space block still carries the forwarding record, which is exactly #7154's latency, and #7196's mprotect quarantine is the instrument that would collapse it (still open at the time of writing).

    So this rests on the static argument, and is gated two ways instead: a codegen regression test whose two conjuncts sabotage-check red independently, plus a new gc_root_dominance_check.py --unrooted-allocas mode.

    The static gate this issue asked for

    The shipped check anchors on js_shadow_slot_bind, so a value that is never bound produces no triple — which is why #7192's checker read the corpus at 1 violation while this_slot was live. The new mode reports a gc-capable alloca that no bind names, holding a value whose provenance is a heap source, loaded below a collecting call on a real CFG path; allocas whose address escapes to a call are excluded rather than guessed at.

    A/B on the reproducer's IR under PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_INLINE_SHADOW_SLOT=0: 2 violations → 1, moving-minor reachable 1 → 0. The survivor is function.rs:521's perry_class_keys_* cache (#7210 §1).

    This issue's worry that the general stale-register invariant needs NONCOLLECTING extending before it is actionable is correct and separate; #7206 is doing that as --stale-registers.

  2. proggeramlug commented on Aug 2, 2026

    @proggeramlug
    ContributorAuthor

    Re-verified on main = d4ac65f21. Fixed — closing, with the residual explicitly relocated.

    Both named instances are closed:

    1. The inline-ctor this_slot — fixed by fix(gc): close the three residual root-store holes — the PutValueSet key, js_object_assign_one, and the inline-ctor this slot #7207. lower_call/new.rs:1148-1164 now seeds the alloca to undefined in entry_allocas and calls crate::expr::root_entry_alloca(ctx, &this_slot), gated on construction_runs_user_code (the same predicate that decides the instance needs a temp root), so a class with no constructor, fields or heritage keeps its previous IR and frame size exactly. Binding it — rather than routing Expr::This through a temp root, as this issue proposed — left all ~30 ctx.this_stack.last() readers untouched, because they already load from the alloca and js_shadow_slot_bind records slot_ptrs[idx] = alloca.
    2. The closure-capture this / new.target slots — split out as GC: a closure's captured this / new.target slots are unrooted allocas (closure.rs reserves no +1 slot where method.rs does) #7208 and fixed by fix(gc): bind the catch parameter and the closure this/new.target slots, and snapshot the Object.assign string source #7216. Verified green with a runtime witness: test_gap_gc_closure_this_capture_rooting 10/10 byte-exact on the allocation-point arm at 5 copying minors / 17 325 objects copied.

    Evidence

    The instrument this issue called for was built (--unrooted-allocas, added in #7207) and now reports clean over a freshly generated corpus:

    === files: 138   gc-capable allocas: 5779   unrooted-alloca violations: 0   (moving-minor reachable: 0)
    

    with the bind-anchored mode also at 0 and --seeded-violations 40 catching 40/40, so neither zero is a parse failure.

    The residual is tracked, not dropped. The [N x i64] staging-array shape in §2 of this issue is codegen/helpers.rs:1275, still a bare emit_raw("alloca [N x double]") filled interleaved with lowering — that is #7210 §2 and it is open. The general stale-register invariant this issue asked for is likewise #7210's subject.

    Part of the sweep in #7262.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions