Skip to content

GC: a closure's captured this / new.target slots are unrooted allocas (closure.rs reserves no +1 slot where method.rs does) #7208

Closed

Description

@proggeramlug

Summary

codegen/closure.rs allocates the closure's captured this (:704) and new.target (:687) into plain alloca_entry slots and never binds either. They hold a heap receiver for the whole closure body, across every this_stack.last() reader (~14 sites), so an evacuating minor neither marks nor rewrites them and every load below a collection point names from-space.

Found by the #7202 bare-alloca enumeration (PR #7207), which fixed the sibling this slot on the inline-constructor path.

It is a structural asymmetry, not one missing line

codegen/method.rs:316   lf.enable_shadow_frame(m.len() as u32 + 1);   // the +1 IS the `this` slot
codegen/method.rs:1344  lf.enable_shadow_frame(m.len() as u32 + 1);
codegen/closure.rs:589  lf.enable_shadow_frame(m.len() as u32);       // no slot reserved

method.rs:334/:1372 then bind that reserved slot (js_shadow_slot_bind(this_shadow_slot_idx, this_slot)). Closures reserve nothing, so there is no index to bind.

The in-tree comment at codegen/closure.rs:815-817 — "The this / new.target capture reads are exempt … they run in the entry-block prologue, ahead of any statement that could collect" — justifies the timing of the read, not the lifetime of the slot, which spans the whole body.

Fix shape

expr::scalar_slot_root::root_entry_alloca (generalized in #7207 from #6968's machinery) is the mechanism: seed the alloca to undefined in entry_allocas, reserve_shadow_slot(), bind once in entry setup. It grows the frame on demand, so enable_shadow_frame's pre-lowering count does not have to predict it.

A reproducer needs a closure whose body allocates between two this uses, with the closure's receiver reachable only from the capture cell. It is worth writing one before fixing: #7207's inline-ctor equivalent turned out to be masked at runtime (the class-field inline guard fails on the forwarded header and the runtime fallback resolves forwarding), so a static fix without a witness should say so.

Refs #7202, #7154, #7161, #7207.

Activity

  1. proggeramlug commented on Aug 1, 2026

    @proggeramlug
    ContributorAuthor

    Reproduced and fixed in #7216.

    Stronger than the issue predicted: red at origin/main 1679e22b4 as a SIGSEGV (exit=139, 3/3 deterministic) under plain PERRY_GC_MOVING_LOOP_POLLS=1, and bad 4 of 400 on the evacuating arm. Green 5/5 / 3/3 / 3/3 with the fix across polls, the evacuating arm and the shipped default, byte-exact against Node 26.5.1.

    The reproducer is the shape the issue asked for: an arrow returned from a method, whose receiver is a temporary reachable only from the closure capture cell after make() returns — so an evacuating minor moves it rather than freeing it, rewrites the capture cell, and leaves the prologue's alloca copy naming from-space.

    Fix is the +1 asymmetry this issue identified: closure.rs now reserves one index per captured this / new.target and binds each inline in the entry block right after its store, matching method.rs:316/:1344.

  2. proggeramlug commented on Aug 2, 2026

    @proggeramlug
    ContributorAuthor

    Re-verified on main = d4ac65f21. Fixed — closing.

    Fixed by #7216 (dd83c7e92): codegen/closure.rs now reserves one shadow index per captured this / new.target and binds each inline in the entry block immediately after its store — the +1 that method.rs:316/:1344 always had and closure.rs:589 did not.

    This issue asked for a witness before the fix, on the grounds that #7207's inline-ctor equivalent turned out to be masked at runtime. That witness exists and it was not masked:

    Evidence

    test-files/test_gap_gc_closure_this_capture_rooting.ts (shipped with #7216), compiled with no compile-time GC env, run on the allocation-point arm:

    PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off \
      ./test_gap_gc_closure_this_capture_rooting
    
    result
    at 1679e22b4 (#7216's base) exit=139 3/3 under polls, bad 4 on the evacuating arm
    d4ac65f21 bad 0, 10/10, byte-exact vs node 26.5.1

    Movement confirmed: 5 copying minors, 17 325 objects copied on the same binary and arm.

    #7066 is the same defect reported separately and is closed with it. 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