Skip to content

GC: this holds a stale from-space pointer after a moving collection inside a method body #8495

Closed

Description

@proggeramlug

cargo test -p perry --test gc_property_key_operand_rooting_6935 fails 2 of 3 on main, under the forced-evacuation env the suite exists to exercise (PERRY_GC_FORCE_EVACUATE=1, PERRY_GC_VERIFY_EVACUATION=1):

proxy_target_set_survives_forced_evacuation ......... FAILED
property_key_stored_values_survive_forced_evacuation  FAILED
property_key_receivers_survive_forced_evacuation ....  ok

The compiled binaries exit 1 with values that came back missing across a collection:

FAIL proxy-forward-set.tag got=undefined want=30
TypeError: Cannot convert undefined or null to object

FAIL dyn-index-set payload missing
FAIL dyn-index-set-toprimitive payload missing
FAIL class-static-computed.tag got=undefined want=6

That is the unrooted-value-across-collection signature this suite was written for (#6935).

Bisected to f14a9e262 — "perf(compile): reduce generated bundle bloat" (#8418).

git bisect run over 526e0b502..74e806502 with a build-and-run script, 4 steps, converged cleanly on that commit. Reproduces locally on macOS (no Linux needed).

Reproduce:

cargo build --profile perry-dev -p perry -p perry-runtime-static -p perry-stdlib-static
PERRY_RUNTIME_DIR=$PWD/target/perry-dev \
  cargo test --profile perry-dev -p perry --test gc_property_key_operand_rooting_6935

Ruled out:

The likeliest remaining suspects inside #8418 are the ~304-line run_pipeline.rs restructuring and the collect_modules / json_module split, rather than the opt-in PERRY_LL_PREOPT_OPTNONE_INSTRS path (default 0 = disabled).

Why this is release-gating and why it appeared only now: cargo-test stops at its first failing suite, and the long-standing bun_ffi_stage1 abort (#8479, fixed in #8488) was ahead of it — so this has been masked. With that abort fixed, this is now the visible cargo-test failure, and cargo-test is required by full-suite-gate.

Activity

  1. proggeramlug commented on Aug 20, 2026

    @proggeramlug
    ContributorAuthor

    Correction: my bisect verdict was wrong — do NOT revert #8418.

    I reverted f14a9e262 (#8418) locally and the failures persist. The revert covered everything of substance — inprocess.rs (all 134 lines, including the optnone stamping), run_pipeline.rs, collect_modules/json_module, object_cache/build_cache — keeping only linker.rs/linker_tests.rs at main, because #8457 later builds on the size_optimization_requested helper #8418 introduced. And that remainder is just the -Os plumbing, which is independently ruled out below. So #8418 is effectively exonerated.

    Where my bisect went wrong. The run script treated any non-ok outcome as "bad":

    grep -q "test result: ok" log && exit 0
    [  -eq 124 ] && exit 125
    exit 1          # <-- build/link failures land here too

    Only a timeout mapped to . Any commit where the suite failed to build or link — plausible mid-range, given the runtime/archive churn in that window — was scored as a reproduction of the bug, which can walk the search to an innocent commit. A correct script must distinguish "the suite ran and failed the assertion" from "the suite never ran", e.g. by requiring a test result: line at all and skipping otherwise.

    What IS solid (re-verified with genuine recompiles):

    • Reproduces on main, macOS, ~4 s per run — the suite passes --no-cache, so there is no object-cache confound and iteration is cheap.
    • Not -Os: PERRY_LL_SIZE_OPT=0 fails identically, confirmed on a 221 s cold run (an earlier 4 s run of mine looked like cache reuse; it was not — that is simply the real time once cargo has nothing to rebuild).
    • Not the extern "C-unwind" work: fails at 74e806502, which predates fix(runtime): remove the C-unwind abort guards from the JS throw path (#8479) #8488.
    • The failing writes are o1[heavyKey(...)] = payload(n) — the STORED VALUE must survive an allocating key coercion (the runtime: audit ToPropertyKey receiver/value rooting — raw receiver and stored value held across GC-capable key coercions #6935 contract). property_key_receivers_survive_forced_evacuation still passes, so receiver rooting is intact; it is specifically the stored value and the proxy-forwarded set.
    • The dynamic write now lowers to js_put_value_set_dyn_ic. Its miss path (js_put_value_set_dyn_ic_miss) does root target/key/value in a RuntimeHandleScope before js_put_value_set, and the hit path (dyn_ic_try_store) cannot allocate — so the runtime side looks correct, which points at caller-side rooting in the emitted IR rather than the helper.

    Given --no-cache and a 4 s cycle, a corrected git bisect run over the same range is cheap and is the obvious next step — with a script that skips (125) whenever no test result: line is produced.

  2. proggeramlug commented on Aug 20, 2026

    @proggeramlug
    ContributorAuthor

    Second correction: #8418 is fully exonerated, and this is NOT a recent regression. Both of my bisects were invalid.

    I verified the endpoints directly, which I should have done first:

    commit result
    f14a9e262 (#8418, the "first bad commit") 2 failed
    f14a9e262~1 (its parent — the bisect's "good" side) 2 failed
    526e0b502 (the range's assumed-good endpoint) 2 failed

    The good endpoint was never good. I started git bisect by asserting 526e0b502 was good instead of testing it, so both runs converged on noise — the corrected skip-rule script from my previous comment fixed a real flaw, but it could not rescue a bisect whose premise was wrong. Apologies for the churn; please disregard the #8418 attribution entirely.

    What this changes. These failures predate the whole recent merge window, so there is no fresh regression to revert. They surface now only because cargo-test stops at its first failing suite and the long-standing bun_ffi_stage1 abort (#8479) sat ahead of them; fixing that in #8488 uncovered these. full-suite-gate has been red since June, so this suite has had no visible gate for months.

    What is solid, all re-verified with genuine recompiles:

    For whoever picks this up: bisecting is still the right tool, but bound it below with a commit you have tested — the natural candidate is 086049732 (#6941), which introduced this suite. I am testing that now and will post whether the suite has ever passed on this host.

  3. changed the title [-]gc: property-key/proxy stored values lost across forced evacuation since #8418 — cargo-test blocker (bisected)[/-] [+]gc: property-key stored values and proxy-forwarded sets lost across forced evacuation (long-standing; cargo-test blocker)[/+] on Aug 20, 2026
  4. proggeramlug commented on Aug 21, 2026

    @proggeramlug
    ContributorAuthor

    Valid bisect (both endpoints verified this time): first bad commit is 2c687ad19 — #7552, "perf(hir): type a for-initializer declaration instead of registering it as Any" (Aug 7).

    Endpoints, each confirmed by running the suite rather than assumed:

    commit result
    086049732 (#6941, added this suite) 3 passed ✅
    2c687ad19 (#7552) 2 failed
    526e0b502 2 failed
    origin/main 2 failed

    So the suite did work once — this is a real regression, not a test that never passed here — and it broke on Aug 7.

    Why #7552 is a credible cause. Its own description explains the mechanism: a for-initializer declaration used to register Type::Any, and Any is pointer-bearing. Typing it means shapes derived from the loop variable stop being handed to the collector as traced pointer slots and become eligible for POINTER_FREE / raw-f64 store paths. If a field that actually holds a GC pointer ends up classified non-pointer, the collector will not update it across an evacuation — which is exactly the observed got=undefined / "payload missing".

    But reverting #7552 on current main does NOT fix it. I did the revert (resolving the conflicts: version-bump noise taken from main, and both body_stmt.rs hunks resolved to the reverted Type::Any side), rebuilt, and the same 2 tests still fail. Combined with the bisect being sound, that means there is more than one independent cause: #7552 introduced the failure on Aug 7, and at least one later change reproduces it on its own, so removing the first alone changes nothing today.

    That is consistent with how long this went unwatched — full-suite-gate has been red since June and cargo-test stops at its first failing suite, so nothing has guarded this file for months and causes could accumulate silently.

    Suggested approach for whoever takes it (iteration is cheap — the suite passes --no-cache and a run is ~5 s once warm):

    1. Bisect again from 2c687ad19 forward with 2c687ad19 as the bad endpoint's ancestor, to enumerate the second cause; repeat until a revert of the identified set actually restores green.
    2. Or attack it directly from the shape side, which may be faster: dump the anon-shape pointer mask for payload(t) ({tag: number, arr: [...], s: string}) and for the loop-minted {i} shapes, and check whether arr/s are being marked non-pointer. PERRY_GC_VERIFY_EVACUATION=1 already fails loudly, so a shape-level assertion should localise it quickly.

    Ruled out (all re-verified with genuine recompiles): -Os / #8457; #8418 in full (see my earlier correction — that attribution came from a bisect with an unverified good endpoint, my error); the extern "C-unwind" work; and the js_put_value_set_dyn_ic helper itself, whose miss path roots target/key/value in a RuntimeHandleScope and whose hit path cannot allocate. property_key_receivers_survive_forced_evacuation passes throughout, so receiver rooting is intact — the defect is specific to the stored value and the proxy-forwarded set.

  5. proggeramlug commented on Aug 21, 2026

    @proggeramlug
    ContributorAuthor

    Further narrowing — the obvious suspects are all clean, so the defect is upstream of the store.

    The failing receiver is receiver() → { seed: 1 }: an all-numeric object literal, so it is declared GC_LAYOUT_POINTER_FREE at allocation. It then gains a pointer property (o1[key] = payload). The natural hypothesis is that this dynamic add fails to downgrade the layout, leaving the collector blind to the new slot. I checked that path and it appears correct:

    • store_object_field_slot → runtime_store_jsvalue_slot, which does call layout_note_slot(parent, slot, bits) and runtime_write_barrier_slot.
    • canonicalize_typed_slot_store_bits only rewrites INT32_TAG values into raw-f64-typed slots; a pointer is returned unchanged, so it cannot mangle the stored payload.
    • js_put_value_set_dyn_ic's hit path (dyn_ic_try_store) re-validates the receiver's live shape token and cannot allocate; its miss path roots target/key/value in a RuntimeHandleScope before js_put_value_set.

    So the store itself, the IC, and the per-slot layout note all look sound. That pushes suspicion upstream: the shape/property-add transition that allocates the new inline slot, or the declared typed-layout descriptor's inline-slot accounting for an object that was born POINTER_FREE — i.e. whether the slot index being noted matches the slot the collector actually traces after the transition.

    This fits #7552 as the first bad commit: typing loop-derived declarations means more object literals mint concrete numeric shapes, so more receivers are born POINTER_FREE and reach this transition — it widens exposure to a latent defect rather than creating a new store bug. It also fits the finding that reverting #7552 alone no longer helps, if later work independently widened the same exposure.

    Concrete next probe: instrument or assert the declared pointer mask / inline-slot count for o1 immediately after o1[heavyKey("k1")] = payload(1) and before churnAndCollect(), and compare against what the tracer visits. PERRY_GC_VERIFY_EVACUATION=1 is already on in this suite, so a mismatch should be assertable at that point rather than inferred from the undefined several statements later.

  6. proggeramlug commented on Aug 21, 2026

    @proggeramlug
    ContributorAuthor

    Resolution, part 1 — a real GC bug, fixed in #8499.

    js_put_value_set rooted target/key/value/receiver and then bound all four to plain f64 locals before js_to_property_key, which runs user code and can evacuate. The handles kept the objects alive; the locals kept pre-collection addresses. Isolated to a 16-line reproduction: a static key survives, the same write with an allocating key coercion loses the value. Fixing it takes this suite 1/3 → 2/3 — proxy_target_set_survives_forced_evacuation now passes, as does the dyn-index-set case.

    Resolution, part 2 — the remaining sub-case is NOT a GC bug.

    dyn-index-set-toprimitive fails because Perry writes the wrong key:

    Perry: keys: ["seed","undefined"]
    Node:  keys: ["seed","k1"]
    

    So the store lands under "undefined" and o.k1 is legitimately absent. Nothing is lost across the collection — the property was never created.

    Narrowing, all measured:

    • A dynamically-assigned Symbol.toPrimitive that does not collect works fine — {n} + obj[Symbol.toPrimitive] = function(){ return String(this.n) } coerces correctly, and so does the object-literal method form. Both match Node.
    • It only misbehaves when the method collects before reading this (churnAndCollect(); return this.n;), i.e. this.n evaluates to undefined after a moving collection.
    • IMPLICIT_THIS itself is fine: scan_implicit_this_roots_mut exists precisely for this (SIGSEGV race in js_native_call_method via @perryts/mysql nativeScramble during concurrent Pool.acquire handshakes (0.5.1029 / c720d2a4) #1813) and is registered via reg_scanner! in gc/mod.rs, so the cell is rewritten on evacuation.
    • js_to_primitive also re-reads its operands from handles after each allocation.

    That leaves the generated code for the method body: a plain function expression assigned to a symbol property appears to materialise this into a stack slot that is neither rooted nor reloaded across the collection point, so the post-churn this.n reads a stale receiver. The object-literal method form does not show this, which suggests the two forms take different this materialisation paths — that difference is the thing to look at.

    This is the "root store must dominate every subsequent collection point" class from CLAUDE.md, on the this operand, and scripts/gc_root_dominance_check.py over --trace llvm output is the intended instrument for it.

    Repro (~5 s, --no-cache, macOS reproduces it):

    const k: any = { n: "k1" };
    k[Symbol.toPrimitive] = function (): string { churn(); return this.n; };  // churn() collects
    const o: any = { seed: 1 };
    o[k] = { tag: 1 };
    console.log(Object.keys(o));   // Perry: ["seed","undefined"]   Node: ["seed","k1"]
  7. proggeramlug commented on Aug 21, 2026

    @proggeramlug
    ContributorAuthor

    Correction: the remaining failure is NOT a rooting bug, and not a this-binding bug. The receiver survives; its PROPERTIES do not.

    I chased this as a stale-receiver problem and was wrong twice. The measurement that settles it — identity and contents probed at the same instant, inside the method body, after a forced-evacuation collection:

    FORCE_EVACUATE : isGlobal=false isV3=true n=undefined keys=[]
    plain          : isGlobal=false isV3=true n=V3        keys=["n","f"]
    node           : isGlobal=false isV3=true n=V3        keys=["n","f"]
    

    isV3=true in the failing arm. The receiver pointer is correct and was rewritten correctly by the collector, so this is rooted and scan_implicit_this_roots_mut is doing its job. What is gone is the object's property storage: Object.keys(this) is [] where it should be ["n","f"]. The value isn't lost across a store — the object is emptied.

    That also explains every earlier observation, without any of the this-binding theories:

    • String(this.n) printing undefined rather than throwing — the receiver is a live object, it just has no properties.
    • Reading this.n before the collection making a later read work (V5/V5) — an earlier read populates a cache that survives; a first read after the collection has nothing to fall back on.
    • const me = this before the churn working — same masking.

    Repro (no Symbol.toPrimitive, no proxy, no property-key coercion — those were all incidental):

    const keep: any[] = [];
    function churnGc(): void { (globalThis as any).gc?.(); }
    const v3: any = { n: "V3" };
    v3.f = function (): string { churnGc(); return String(Object.keys(this)); };
    keep.push(v3);
    console.log(v3.f());     // Perry FORCE_EVACUATE: ""   Node: "n,f"

    Scope, measured. The same object at top level keeps its properties across gc() under PERRY_GC_FORCE_EVACUATE=1 (literal, literal+dynamic-add, and all-dynamic shapes all survive). It only empties when the object is young and the collection happens deeper in the stack, inside a method call. Identity intact + shape empty is the signature of an address-keyed side table that is not re-keyed when the object moves — the same class as #8393, and worth checking against #7510's layout tables first.

    This is what property_key_stored_values_survive_forced_evacuation has been asserting all along; the test name is accurate and my earlier reading of it was not.

    Two claims of mine on this issue are withdrawn: that the remaining sub-case was a wrong-key coercion (the key is fine; the object is empty), and that it was a stale-this restore. I also swept all 135 unrooted js_implicit_this_set save/restore pairs in the runtime onto the rooted form and rebuilt: it changed nothing, which is the evidence that the save/restore population is not implicated here. That sweep is NOT part of #8499 — it is unvalidated and I reverted it rather than ship a 135-site runtime change on a release branch.

    #8499 remains a separate, verified fix for a real stale-operand bug in js_put_value_set and does not close this issue.

  8. changed the title [-]gc: property-key stored values and proxy-forwarded sets lost across forced evacuation (long-standing; cargo-test blocker)[/-] [+]GC: object loses its property storage when evacuated during a nested method call (identity survives, Object.keys empties)[/+] on Aug 21, 2026
  9. proggeramlug commented on Aug 21, 2026

    @proggeramlug
    ContributorAuthor

    Root-cause progress: this is a stale from-space this, confirmed by the quarantine instrument. Handing off with the search narrowed to one contradiction.

    Confirmed by measurement

    1. The value read as this is a stale from-space pointer, not an emptied object. With PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800:

    [gc-fromspace-protect] FAULT: signal 10 at 0x2395db64570
      This address is RETIRED FROM-SPACE. The evacuating minor moved or
      freed the object here and the holder kept the pre-collection address.
      block=0x2395dae0000 +542064 retired_bytes=542432 retired_by_minor=#0
      last-known object: user_ptr=0x2395db64578 obj_type=2 size=40
      The faulting instruction IS the stale use.
    

    Corroborated from JS: in the failing arm Object.getPrototypeOf(this) === Object.prototype is false (it is true when healthy) and typeof this === "object" — i.e. garbage read as an object, not a live object that lost its properties. My earlier "the object is emptied" reading was wrong; that was the symptom, not the defect.

    2. Only the receiver is affected. Equally young, equally rooted objects observed at the same instant are intact — including one holding a closure-valued property, which rules out the closure-property theory:

    recv=[]  plain=["n"]  withFn=["n","g"]  nested=["n","o"]
    

    3. === masks it. this === recv reports true while probe(this) and probe(recv) — the same helper, one callsite, so no per-callsite cache — return [] and ["n","f"]. Equality evidently resolves forwarding; ordinary reads do not. Do not trust === to decide whether you are holding a moved object.

    4. The IMPLICIT_THIS scanner is NOT the bug. Instrumented, it runs on the right cell and rewrites correctly:

    [this-scan] cell@0x81d400098 thread=ThreadId(1) before=0x…82e02a0 rewrote=true after=0x…97002a0
    

    The contradiction to resolve next

    Same cell address, same thread, and between the last scan and the read there is no write at all — every js_implicit_this_set call was logged, and none occurs in that window:

    [this-scan] cell@0x81d400098 … after=0x7ffd03b7597002a0     <- scanner leaves this
    [this-get]  cell@0x81d400098 … value=0x7ffd03b758364578     <- read returns this
    

    this_binding.rs has exactly three access points (get:121, set:170, scanner:218), so on the code as written this transition should be impossible. Either a fourth writer exists that I have not found, or the TLS slot is being written by something that is not going through the cell's API. That is where the next session should start; it is a much smaller question than the one this issue opened with.

    Ruled out, so nobody re-walks them

    • Wrong-key coercion in the property-key path (the key is fine).
    • A stale-this restore in the runtime save/restore population: I converted all 135 unrooted js_implicit_this_set save/restore pairs to the rooted form and rebuilt — behaviour identical. Reverted; not part of any PR.
    • The Object.keys callsite cache (defeated with a shared helper, still fails).
    • Closure-valued properties, Symbol.toPrimitive, proxies, Object.defineProperty — all incidental to the original repro.
    • scan_implicit_this_roots_mut being unregistered or ineffective.

    Repro (~5 s, macOS reproduces)

    const keep: any[] = [];
    function churnGc(): void { (globalThis as any).gc?.(); }
    const recv: any = { n: "RECV" };
    recv.f = function (): string {
      churnGc();
      return String(Object.getPrototypeOf(this) === Object.prototype) + " " + JSON.stringify(Object.keys(this));
    };
    keep.push(recv);
    console.log(recv.f());
    // PERRY_GC_FORCE_EVACUATE=1 -> "false []"      (stale from-space `this`)
    // without the knob / node   -> "true [\"n\",\"f\"]"

    Needs PERRY_GC_FORCE_EVACUATE=1; a plain run and a minor-only churn (keep.push(...)) both pass, so the trigger is a moving collection reached from inside a method body.

    Masking to beware: reading this.n before the collection makes a later read succeed, so any probe that touches this early proves nothing.

  10. changed the title [-]GC: object loses its property storage when evacuated during a nested method call (identity survives, Object.keys empties)[/-] [+]GC: `this` holds a stale from-space pointer after a moving collection inside a method body[/+] on Aug 21, 2026
  11. proggeramlug commented on Aug 21, 2026

    @proggeramlug
    ContributorAuthor

    Addendum — two more hypotheses eliminated, and the exact ordering anomaly to start from.

    Duplicate runtime: ruled out. libperry_runtime.a and libperry_stdlib.a both export _js_implicit_this_set as a global text symbol and both carry IMPLICIT_THIS TLS symbols, so "two copies of the cell, scanner fixes one and the mutator reads the other" looked compelling. It is wrong: instrumenting all three access points to print the cell address shows exactly one cell for the whole run (cell@0x…c009c8 in set, scan, and get alike), on ThreadId(1) throughout. The archive-level duplication is real but does not produce two live cells here.

    The anomaly, stated precisely. With every write, scan and read logged on that one cell:

    [this-set]  cell@0x994c009c8 value=0x…ff6e02a0 prev=undefined      <- bind receiver
    [this-set]  cell@0x994c009c8 value=undefined    prev=0x…ff6e02a0   <- cell := undefined
    [this-scan] cell@0x994c009c8 before=0x…ff6e02a0 rewrote=true after=0x…00a802a0
    [this-get]  cell@0x994c009c8 value=0x…ff764578
    

    Two things cannot both be true on a single-threaded run with one cell and three access points (this_binding.rs get:121, set:170, scanner:218):

    1. the scan observes the receiver immediately after a write that stored undefined; and
    2. the read returns …ff764578 — a third value, neither what the scan left (…00a802a0) nor anything ever written — and it is the stale from-space address the quarantine faults on (user_ptr low bits 4578).

    So either stderr ordering is misleading me about when the collection runs relative to the bind/restore pair, or the cell is written by something that is not one of those three access points. Resolving that is the whole remaining question. A good first move is to log a monotonic sequence number plus the collection ordinal at all three sites, so the interleaving is established from a counter rather than from print order.

    Cumulative ruled-out list for this issue (all by measurement): wrong-key coercion; a stale-this restore anywhere in the 135-pair runtime save/restore population (converted them all to the rooted form — behaviour identical, reverted); the Object.keys callsite cache; closure-valued properties, Symbol.toPrimitive, proxies, defineProperty; scan_implicit_this_roots_mut being unregistered or ineffective; and now duplicate runtime copies.

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