Skip to content

GC: the two lazy intrinsic-tower builders need #7217's no-move window, and need a gate that can fail first #7251

Closed

Description

@proggeramlug

Summary

#7217 established the invariant and fixed the one site that had a runtime witness:

A bootstrap that builds an IMMORTAL object graph through raw pointers held across its own allocations must run in a NO-MOVE WINDOW.

populate_global_this_builtins now runs in a GcSuppressScope, gated by a cargo-test-visible unit test that arms a collection, shows the bootstrap does not service it, and shows the same armed request IS serviced by ordinary allocation once the window closes.

Two sibling builders have the identical shape and are NOT covered, because I could not build a gate for them that can fail. Filing rather than shipping ungated, per CLAUDE.md's knob-kill policy ("shipping a GC-trigger change with no test that can fail without it is the thing [it] exists to stop", quoted verbatim at crates/perry-runtime/src/string/alloc.rs:49).

The two sites

builder the raw locals also reachable ahead of the bootstrap from
object/global_this/generator.rs::ensure_generator_intrinsics → build_generator_tower *mut ObjectHeader / *mut ClosureHeader ctor+proto locals threaded across every allocating install in the tower generator_function_proto_of, generator_function_prototype_of, generator_prototype_ptr — i.e. any program that reflects on a generator without touching globalThis
object/global_this/typed_array.rs::ensure_typed_array_intrinsic ctor and proto, held across ~a dozen set_builtin_property_attrs / set_field installs typedarray_props.rs:812

Both build an immortal graph (the towers hang off process-global intrinsic slots), so the same argument applies verbatim: a collection inside them frees nothing and can only move the holders out from under them.

When they are reached from populate_global_this_builtins they inherit its window and are safe today. The exposure is the lazy-ahead-of-bootstrap path only.

Why #7217 could not gate them, measured

populate_global_this_builtins is gateable behaviourally — arm a pending collection, run it, assert nothing collected — because it allocates ~1.15 MB and therefore crosses several arena block boundaries, and arena_alloc_gc calls gc_check_trigger() on each. A tower is three orders of magnitude smaller and fits inside one block's tail, so it can reach no trigger at all:

  • a first version of the gate arming a collection around ensure_generator_intrinsics() passed with the GcSuppressScope deleted;
  • a second version pre-filling to a block boundary, and re-execing the test binary so the child process is the first to touch a tower (the slots are process-global AtomicI64s, so libtest's name ordering meant array::tests's js_get_global_this() built them first and the test was measuring nothing) — also passed with the scope deleted;
  • a third version recording gc_is_suppressed() from inside the builder under #[cfg(test)] reported suppressed even with the scope removed, which is unexplained and is itself a reason not to trust it as a gate. (Worth a look: something else may already be suppressing across part of the tower build — symbol/get.rs calls the manual gc_suppress()/gc_unsuppress() pair at three sites, and well_known_symbol is on this path.)

What would close this

Either of:

  1. A workload witness. A gap test that reflects on a generator (or touches a typed array) under enough allocation pressure to put a collection inside the tower build, run on the allocation-point arm (PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off) with PERRY_GC_PROTECT_FROMSPACE=1. If the quarantine faults inside build_generator_tower, the window is justified and the witness is its gate.
  2. A trigger the tower's size can reach. GcTriggerThresholdTestGuard::make_malloc_sweep_due() arms the malloc-count trigger, which gc_check_trigger sees on every gc_malloc rather than once per arena block. If the tower's installs go through gc_malloc at all, that gives a lever a few-hundred-object build can actually trip.

Resolve the third bullet's anomaly first — if something already suppresses across these builds, the windows may be redundant rather than missing, and that is a better answer than adding them.

Refs #7217, #7196.

Activity

  1. proggeramlug commented on Aug 2, 2026

    @proggeramlug
    ContributorAuthor

    Confirmed still open on main = d4ac65f21. Re-verified in the sweep #7262.

    Neither builder has a no-move window:

    • crates/perry-runtime/src/object/global_this/generator.rs — no GcSuppressScope; build_generator_tower at :490, ensure_generator_intrinsics at :609.
    • crates/perry-runtime/src/object/global_this/typed_array.rs — no GcSuppressScope; ensure_typed_array_intrinsic at :418.

    This matches #7249's own "Known issues" note, which recorded that windows for both were written and then deliberately dropped rather than shipped ungated, per the knob-kill policy. Nothing since has added them.

    The sibling site is now verified fixed: populate_global_this_builtins's window is real and load-bearing — #6991 and #6992 both closed on it in this sweep, each green at ~3.4× the movement level their crashes were originally observed at. So the invariant this issue is built on ("a bootstrap that builds an IMMORTAL object graph through raw pointers held across its own allocations must run in a NO-MOVE WINDOW") now has two independent confirmations behind it, which strengthens the case for the two remaining towers.

    The open question is still the gate, not the fix, and the issue's third bullet — gc_is_suppressed() reporting suppressed from inside the builder even with the scope removed — remains the right thing to resolve first. symbol/get.rs's manual gc_suppress()/gc_unsuppress() pair at three sites is still present in the tree and well_known_symbol is still on this path, so that hypothesis is intact and untested.

  2. proggeramlug commented on Aug 3, 2026

    @proggeramlug
    ContributorAuthor

    Heads-up before anyone invests here — this issue is in a class that a planned architectural change would make unrepresentable, and I don't want someone building new machinery that gets deleted.

    See docs/src/internals/rfc-rooting-by-construction.md ("Where this sits in the one GC correctness plan", landed in #7292). Short version: this is layer 2 — a GC-managed pointer live in emitted machine code across a point where the collector can run. Statepoints/stack maps (#7108, #7174, enabled by #7241) would have LLVM track that liveness precisely, so the whole shape stops being expressible.

    This is not "don't fix it". The architectural change is weeks away, the bug is real now, and correctness comes first — please do fix it.

    It is "fix the instance, don't build new mechanism." Specifically:

    • A targeted root/RuntimeHandleScope/GcSuppressScope at the site: good, lands now, survives.
    • A new predicate, a new shadow-slot allocation scheme, or a new hand-maintained table deciding what to root: please raise it first. codegen: ClassExprFresh leaves the class object unrooted across its own js_object_set_field_by_name stores #7211 is the cautionary case — a four-clause predicate written by someone actively thinking about rooting, still wrong, because it asked about the user's expressions rather than the lowering's own emitted calls. Statepoints delete that entire category of judgement call.

    Also worth knowing when you verify: PERRY_GC_MOVING_LOOP_POLLS=1 set at run time against a default-compiled binary is clean 10/10 and proves nothing — the poll codegen isn't in the binary. Compile-time and run-time env must both be set (#7280). And a green run that performed zero copying minors proves nothing either (#7255) — assert copied_objects > 0.

  3. proggeramlug commented on Aug 9, 2026

    @proggeramlug
    ContributorAuthor

    Resolving the third bullet first, as the issue asks

    The issue's own hypothesis is wrong, and the windows are genuinely missing rather than redundant. Source-only investigation on 8d27032c7.

    The symbol/get.rs lead is a dead end

    The three gc_suppress()/gc_unsuppress() pairs at symbol/get.rs:1019/1043, 1057/1069, 1104/1123 are all inside #[cfg(test)] mod handle_meta_share_tests, which begins at symbol/get.rs:966. They are test scaffolding, not production code. well_known_symbol (symbol.rs:167-207) uses Box::new plus two Mutex side tables and calls no GC function at all.

    What actually explains "reported suppressed with the scope removed"

    The intrinsic slots are process-global AtomicI64 statics (object/mod.rs:223-224, 233-238), so each tower is built exactly once per process. In a test binary the first caller is essentially always js_get_global_this() (fetch_globals.rs:54) → populate_global_this_builtins (line 93), which opens its window at populate.rs:79 and calls ensure_typed_array_intrinsic() at 111 and ensure_generator_intrinsics() at 115 — both inside it. A #[cfg(test)] probe therefore records true from the bootstrap-driven build and is never re-entered. This is the same ordering hazard the issue's second bullet already hit; it was not fixed by the re-exec, and asserting GENERATOR_FUNCTION_INTRINSIC_PTR.load() == 0 before the subject call is the check that would have caught it.

    A. build_generator_tower — coverage is a sliver, not the tower

    generator.rs:574-689. No GcSuppressScope. ctor/proto/gen_proto are bound at lines 590-592 and last used at the atomic stores on 686-688.

    The only suppression touching any of it is transient and lives in a callee: note_meta_descriptor_key (descriptor_state.rs:505-522) opens a GcSuppressScope at 512 around object_meta_ensure and closes it immediately. And it does not exist at all for closure owners — meta_capable_object (prototype_chain.rs:132-146) rejects anything whose GC header is not GC_TYPE_OBJECT, and a ClosureHeader is GC_TYPE_CLOSURE.

    Outside any window: js_closure_alloc (590), two js_object_alloc (591-592), set_bound_native_closure_name (601, allocates a name string), the two set_builtin_property_attrs on the closure (603-604, no-ops for the window), set_intrinsic_to_string_tag (625, 684), three install_proto_method (647-649, each a closure + a key string), install_proto_symbol_self_method (683).

    B. ensure_typed_array_intrinsic — same, an order of magnitude larger

    typed_array.rs:418-508. ctor (428) and proto (429) held raw across install_typed_array_proto_accessors (467, 4 closures + 4 getters), install_typed_array_to_string_tag (468), and install_typed_array_proto_methods (491) — 44 methods (TYPED_ARRAY_PROTO_METHODS, typed_array_proto_thunks.rs:45), each a closure plus ≥2 strings plus 3 attribute installs.

    D. Route 2 is not constructible

    make_malloc_sweep_due (gc/tests/support.rs:447-450) sets GC_NEXT_MALLOC_TRIGGER, which is consulted only inside gc_budgeted_due_trigger() (policy.rs:2075-2079) — i.e. only once gc_check_trigger has already been reached. The premise that makes it attractive is that gc_malloc calls gc_check_trigger per call (gc/malloc.rs:234). The towers never call gc_malloc. Every allocator on both paths bottoms out in arena_alloc_gc:

    tower call backing
    js_object_alloc arena_alloc_gc (object/alloc.rs:138)
    js_closure_alloc arena_alloc_gc — the gc_malloc arm (closure/alloc.rs:81) needs payload > 16 KiB; a 0-capture tower closure is 16 bytes
    js_string_from_bytes arena_alloc_gc (string/mod.rs:502-508)
    js_array_alloc arena_alloc_gc (array/alloc.rs:37-46)
    object_meta_ensure arena_alloc_gc (object/mod.rs:1566)
    well_known_symbol Rust Box — no GC path

    So gc_check_trigger is reachable during a tower build only via arena_cell_alloc when the current 1 MiB block fills (arena/block.rs:651). Arming the malloc trigger changes which condition fires, not whether the check runs.

    A route the source does support: arena::arena_start_fresh_general_block() is pub (arena/inline.rs:122-137), so a gate can force a fresh block, fill it to within a few hundred bytes of BLOCK_SIZE, arm, then call the builder.

    E. The exposure is much wider than "reflects on a generator"

    generator_prototype_ptr and generator_function_prototype_of are called from the two #[no_mangle] helpers codegen emits for every generator instantiation — js_generator_attach_prototype → generator_prototype_ptr (generator.rs:452, 466) and js_generator_attach_closure_prototype → generator_function_prototype_of (generator.rs:544).

    So the real exposure is "the first gen() call in any program that has not yet touched globalThis", not "a program that reflects on a generator". Both helpers were carefully rooted in #7577 (RuntimeHandleScope at generator.rs:440 and 516, with the hazard spelled out at 396-430) — and the tower they trigger on line 452 rebuilds the same class of unrooted raw locals two frames down.

    That is the strongest available lever for route 1: a .ts witness whose first generator call happens under allocation pressure, on the allocation-point arm with PERRY_GC_PROTECT_FROMSPACE=1.

    One footgun found in passing

    GC_FLAG_SUPPRESSED is a single bit (policy.rs:6), and gc_unsuppress() clears it unconditionally (policy.rs:1044-1046). The manual pair is therefore not nesting-safe: any gc_unsuppress() reached inside an outer GcSuppressScope ends that outer window early. Not exploitable on the tower path, but live at json/parse_api.rs:56/61, 137/161, 322/338, 436/457, 530/537 and child_process/v8_serde.rs:87/93.

  4. proggeramlug commented on Aug 9, 2026

    @proggeramlug
    ContributorAuthor

    Addendum: route 1 attempted and it did NOT produce the witness

    I ran the whole 507-test gap suite on the allocation-point arm under the quarantine (#7341's route), specifically hoping a generator test would fault inside the tower.

    test_gap_7577_generator_prototype_rooting does fault, deterministically 3/3, obj_type=2 OBJECT size=72, retired_by_minor=#2 — and I initially attributed it here. Symbolicating it refutes that: the stale use is in object::field_get_set::get_field_by_name_tail::get_field_by_name_object_tail, reached through the recursive object::prototype_chain::resolve_inherited_field_from_prototype / resolve_inherited_field pair from js_object_get_field_by_name_f64. That is #7341's OBJECT-72 property-read cluster, not build_generator_tower.

    So the census gives no evidence either way about the two tower windows, and this issue's gate is still unbuilt.

    That does not weaken the source finding above — the windows are real and uncovered — but it does say the obvious workload witness is not free. Two things the attempt showed that a next attempt should use:

    • the tower is built once per process, so any witness must be the process's first generator or typed-array touch. test_gap_7577_generator_prototype_rooting almost certainly builds the tower long before its interesting part runs, and once built it is never re-entered;
    • the exposure is every generator instantiation, not reflection — js_generator_attach_prototype → generator_prototype_ptr (generator.rs:452, 466) and js_generator_attach_closure_prototype → generator_function_prototype_of (generator.rs:544). A witness therefore wants a program whose very first statement allocates enough to arm a collection and whose second is a gen() call, with nothing touching globalThis in between.
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