Skip to content

gc: js_closure_get_capture_bits's result is never re-entered into a protected domain (2 unrooted:capture hits, gc-root-dominance-statepoints' --max-unrooted referent) #7725

Description

@proggeramlug

js_closure_get_capture_bits's result is never re-entered into a protected domain (2 unrooted:capture hits, --max-unrooted referent)

Split out of #7664, which fixed the checker's phi false positives and the 3
unrooted:global hits (module-global receivers/arguments held across a
sibling operand's allocation). The remaining --max-unrooted 2 on
gc-root-dominance-statepoints is these two hits, and this issue is their
referent.

The shape

js_closure_get_capture_bits(closure_ptr, idx) returns a raw i64 — the
captured value's bit pattern, which may be a NaN-boxed heap reference. Two
things protect a value like that under the native (RS4GC/statepoint)
lowering: re-entering the ptr addrspace(1) tracked domain (what
codegen/closure.rs already does for %this_closure itself — or …, POINTER_TAG then inttoptr … to ptr addrspace(1)), or a temp root. The
generic "read a captured value" call sites do neither:

  • expr/mod.rs::load_boxed_local_pointer and its siblings in
    literals_vars.rs / array_push.rs / instance_misc1.rs /
    lower_array_method.rs return the bare i64/double and leave it exactly
    that across whatever the calling expression's evaluation does next.
  • The synthesized dynamic-parent implicit constructor closure
    (constructor(...args) { super(...args); … } for a class expression whose
    parent is a runtime value with no statically-known ctor,
    test_gap_class_expr_dynamic_parent_ctor.ts) reads a captured value at the
    top of the closure and uses it ~60 lines later as the callee of a second
    js_new_function_construct, across js_object_alloc_class_inline_keys
    AND the class's own user constructor body.

Confirmed register-by-register on both corpus hits:

test_gap_class_expr_dynamic_parent_ctor_ts::__closure_21
  source (capture): %r61 = call i64 @llvm.experimental.gc.result.i64(token %statepoint_token)
  stale use        : js_new_function_construct's own statepoint, ~60 lines below
  across safepoint : js_closure_get_capture_bits, js_ctor_return_override,
                      js_new_function_construct, js_object_alloc_class_inline_keys,
                      <AnonShape>_constructor (user code)

test_gap_computed_key_method_nested_this_ts::__closure_9
  source (capture): %r145 = call i64 @llvm.experimental.gc.result.i64(token %statepoint_token4)
  stale use        : %r17 = fadd double %r167, %r15
  across safepoint : js_number_coerce

Neither register ever appears in a "gc-live" bundle anywhere in its
function.

Why this isn't a signature/ABI change (correction to the #7664 triage)

The prior triage read this as needing "the callee's own closure pointer to
be a tracked root first, which is a signature/ABI change." Closer reading:
the closure pointer itself (%this_closure) is fine here — it's already
correctly re-entered into the tracked domain and relocated
(%rs4gc.s2/.relocated chains appear in both traces). What's unprotected
is the return value of js_closure_get_capture_bits, which is a completely
separate register with no relationship to the parameter.

Proposed fix

Mirror the string-handle precedent already in crates/perry-codegen/src/root_reload.rs:
treat a js_closure_get_capture_bits call as a reloadable source — re-emit
the same call (same closure pointer, same index) rather than re-deriving from
a load. Soundness argument is the same shape as the existing handle-global
one: re-calling it can only observe a different value if a
js_closure_set_capture_bits to the same index ran in the window, which is
the same store side-condition the pass already tracks for shadow slots and
handle globals (stores_to in root_reload.rs's Facts).

The engineering gap: Facts currently only models loads
(load_of: Option<(String, LlvmType, String)>) as reloadable sources, not
calls. Extending it to recognize a specific call pattern as reloadable (and
re-emitting a call rather than a load at each protected use) is a real
slice of work, not a one-line change — hence splitting it out rather than
rushing it into #7664.

Scope check before starting

Confirm via the corpus (scripts/gc_root_dominance_corpus.sh … --lowering native + the checker's -v output) whether other unrooted:capture shapes
exist beyond these two before generalizing the fix — the two here are both
"read once, held long, no phi, no reassignment in window," but the fix
should be verified against the full corpus rather than just these two
fixtures.

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