Skip to content

fix(gc): resolve the memoized Array.prototype address across relocation (#6981) - #7071

Merged
proggeramlug merged 2 commits into
mainfrom
fix/6981-array-hole-read-recursion
Jul 30, 2026
Merged

fix(gc): resolve the memoized Array.prototype address across relocation (#6981)#7071
proggeramlug merged 2 commits into
mainfrom
fix/6981-array-hole-read-recursion

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Closes #6981.

The recursion frame, symbolised

PERRY_DEBUG_SYMBOLS=1 + the OS crash report + atos gives the frame the issue was missing:

main
  js_array_get_index_or_string
    js_array_get_f64            <- repeated, one call site, unbounded
    js_array_get_f64
    ...
      array_oob_prototype_get
        array_has_own_index
          array_object_flags    <- fault, stack guard page

EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE, "Thread stack size exceeded due to excessive recursion". So the earlier retraction was right: this is not memory corruption. Every dereference on the way down is to valid, mapped memory. It is a two-function cycle that never terminates.

Mechanism

array::indexing memoizes Array.prototype's heap address in a process-global AtomicUsize (ARRAY_PROTO_ADDR; Object.prototype has the twin). That is a raw pointer to a movable object, and nothing maintained it.

Every reader of an array pointer resolves it through clean_arr_ptr, which follows GC_FLAG_FORWARDED chains. The cache did not. And the hole/OOB read fallback's self-recursion guard is an object-identity test:

let proto = array_prototype_addr();          // memoized, UNRESOLVED
if proto != 0 && proto != receiver {          // receiver is clean_arr_ptr-RESOLVED
    ...
    return js_array_get_f64(proto_arr, index); // resolves proto -> different address
}

Once Array.prototype moves, proto and receiver are two different addresses for the same object. The guard stops firing, and:

js_array_get_f64(stub)  --clean_arr_ptr-->  live  --hole-->
array_oob_prototype_get(live)  --cache-->  stub  -->  js_array_get_f64(stub)  -->  ...

That is the unbounded recursion. It also explains the three necessary conditions exactly: the index write is what sets ARRAY_PROTO_HAS_INDEX and arms the fallback (naming the prototype does not); the hole read is what enters the fallback; relocation is what splits the identity.

It was never only a GC bug

Array.prototype relocates two ways, and only one involves the collector:

route reproducer needs GC?
js_array_grow — indexed write past the dense capacity reallocates and forwards the old head Array.prototype[300] = 555 no — dies with PERRY_GEN_GC=0
copying young-gen minor — evacuates the prototype and forwards the corpus histogram under PERRY_GC_HEAP_LIMIT=8 yes

The second is what #6981 was measuring. The first is a GC-free reproducer of the same defect on main, found while reducing this one, and it is the stronger regression test because no collector arm is required to trip it.

Minimal reproducers

GC-free (exit 139 on main, no env at all):

(Array.prototype as any)[300] = 555;   // index write, past capacity -> grow -> forwarding stub
const c: number[] = new Array(4);
c[0] = 1;
console.log("" + c[1], "" + c[300]);   // hole read -> prototype fallback -> recursion

Relocation (exit 139 on main under the pressure knob; correct without it):

(Array.prototype as any)[3] = 555;     // index write, in capacity -> no grow
function histogram(data: number[], size: number): number[] {
  const counts: number[] = new Array(size);       // all holes
  const mask = size - 1;
  for (let i = 0; i < data.length; i++) { const v = data[i] & mask; counts[v] = (counts[v] || 0) + 1; }
  return counts;
}
const data: number[] = []; let seed = 4242;
for (let i = 0; i < 2000; i++) { seed = (seed * 48271) % 2147483647; data.push(seed); }
console.log(histogram(data, 16).join(","));

Both are in the acceptance suite verbatim; both are byte-exact against the pinned Node 26.5.0 oracle.

Fix

Three defences, one per way the address can go stale:

  1. array_prototype_addr / object_prototype_addr heal the cache through the forwarding chain and store the healed address back. This is what covers js_array_grow, which the collector never sees. The not-forwarded path stays call-free: try_read_gc_header is #[inline(always)] and reduces to two range compares plus one load of a permanently-hot gc_flags byte, so note_array_index_write's per-write probe keeps its shape.
  2. scan_prototype_addr_cache_roots_mut is registered in gc_init, so a relocating cycle rewrites the slot like every other address-holding side table (CLASS_PROTOTYPE_OBJECTS, TYPED_ARRAY_VIEW_META, …). Healing alone is not sound here: once the from-space stub is swept and its block recycled, the forwarded bit is gone and the cache would name an unrelated live object.
  3. array_oob_prototype_get resolves the receiver before the identity compare, so the guard is exact by construction instead of contingent on cache freshness.

resolve_forwarding already existed in value/equality.rs — its own doc says it exists "so callers can compare resolved addresses for object identity", which is precisely the invariant this site was violating. It is promoted to pub(crate) and reused rather than duplicated.

Teeth — sabotage-verified in both directions

crates/perry/tests/gc_array_prototype_hole_read_6981.rs — both programs above x 5 collector arms (default, heap_limit_8, precise_roots, force_evacuate, gen_gc_off), each byte-exact vs Node 26.5.0. The suite clears the whole PERRY_GC_* family before each arm so an exported kill switch cannot turn it into a control.

gc::tests::runtime_roots::prototype_addr_cache — 5 unit tests, cargo test --lib -p perry-runtime visible (the per-PR gate), one per defence plus scanner registration and the unset sentinel.

sabotage result
revert the heal (return cached) prototype_addr_reads_through_a_forwarding_stub + ..._multi_hop_... FAIL, other 3 pass
drop the gc_init registration + empty the scanner body ..._is_rewritten_by_the_collector + ..._scanner_is_registered FAIL, other 3 pass
revert the whole runtime fix, rebuild perry and the -static wrappers both e2e tests FAIL: [grow/default] and [relocate/heap_limit_8] die by signal

The last row is the one that matters and it caught me first time round: cargo test -p perry does not rebuild libperry_runtime.a (the -static wrappers do), so the initial sabotage run linked the fixed archive and passed vacuously. Re-run after cargo build --release -p perry -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static, it goes red. Recorded here because the trap is easy to repeat.

Also added test-files/test_gap_array_proto_grow_hole_read.ts (parity file, byte-exact with and without the pressure knob).

Matrix delta

scripts/gc_repsel_matrix.sh --arms all --pressure 8, release, macOS arm64, pinned Node 26.5.0, isolated worktree + isolated CARGO_TARGET_DIR, full -p perry -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static build set. Denominator unchanged: 22 rows x 20 arms = 440 cells.

before   PASS=325  UNVER=100  XFAIL=1  FAIL=14     byte-exact 425/440
after    PASS=339  UNVER=100  XFAIL=1  FAIL=0      byte-exact 439/440

+14 PASS / -14 FAIL, nothing regressed. UNVER is bit-for-bit unchanged at 100 (19 = p4a3_numarray_growth / #7016, 81 = the 9 loop-free rows that went inert per #7059) and the single XFAIL is still the pre-existing repsel_ptr_shape_locals x rep_ptr_shape_off (#6976).

All 14 FAILs were test_gap_repsel_p4a3_numarray_barriers, in exactly the 14 arms with a non-zero copy-minor count — verified per the #7040 distinction ([gc-copy-minor] ran copied_objects=), never the summed moved. That row is now PASS in all 20 arms, and the arm-liveness table is unchanged: evac_minor / force_evac / force_verify / loop_polls / the six rep_*_off arms at copy-minor 21/22, default / verify_evac / cons_scan_off / cons_scan_off_force at 12/22, the five collect arms at 0/22 by construction.

evac_minor and force_verify are put back into PR_ARMS, which is what that script's own comment instructed once #6981 closed. Per-PR now carries the relocating-minor arms.

Full RUST_TEST_THREADS=1 cargo test --lib -p perry-runtime: 1531 passed, 0 failed.

What I could not verify

  • The from-space byte state at the moment the cycle starts. The recursion needs array_has_own_index(stub, index) to answer true once, and I did not instrument which bytes make it do so (dead payload vs a recycled block). It does not change the fix — the fix removes the address split that lets the cycle exist at all — but the entry condition is inferred, not measured.
  • Object.prototype's half is fixed symmetrically but has no observed reproducer. OBJECT_PROTO_ADDR has the identical shape and the identical consumers (note_object_prototype_index_write, object_prototype_addr_matches), so it is fixed alongside; the unit tests cover it, no end-to-end program does.
  • A silent-miss class this also closes, untested end to end. note_array_index_write compares the live receiver against the cache, so before this change a first Array.prototype index write occurring after a relocation would not set ARRAY_PROTO_HAS_INDEX at all — holes would read undefined instead of the inherited value. That is a wrong answer rather than a crash; I did not build a reproducer for it.
  • No per-cell diff against the pre-fix matrix, only totals plus the failing-row/arm identity — the baseline is recorded in gc: a relocating minor with PRECISE roots breaks 14 of 20 representation-corpus files (5 crashes, 9 mismatches) — the conservative stack scan is load-bearing for correctness #6981 as totals.
  • Single host (macOS arm64, M-series mini). Not re-measured on Linux.
  • No perf measurement of the heal probe. The argument above is a static one (inlined, two compares plus one hot load); I did not run the benchmark frontier.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed potential hangs or crashes when reading array holes or out-of-bounds indices after Array.prototype relocation.
    • Restored correct inherited values for array holes following prototype relocation.
    • Improved reliability during moving garbage-collection cycles.
  • Tests

    • Added regression coverage for prototype relocation, array growth, and hole reads.
    • Expanded garbage-collection validation scenarios.

…on (#6981)

`array::indexing` memoizes `Array.prototype`'s (and `Object.prototype`'s)
heap address in a process-global `AtomicUsize`. That is a raw pointer to a
MOVABLE object, and nothing maintained it. Two independent relocations
leave the cache naming a `GC_FLAG_FORWARDED` stub:

  1. `js_array_grow` — an indexed write past the dense capacity
     (`Array.prototype[300] = v`) reallocates the backing store and
     forwards the old head. No GC is involved at all.
  2. the copying young-gen minor — it evacuates the prototype and forwards.

Every reader of an array pointer resolves it through `clean_arr_ptr`, which
follows forwarding chains. The cache did not, and the hole/OOB read
fallback's self-recursion guard is the object-identity test
`proto != receiver`. After a move those are two different addresses for the
SAME object, the guard stops firing, and

    js_array_get_f64  ⇄  array_oob_prototype_get

recurse without bound until the thread's stack guard page:
`EXC_BAD_ACCESS` / `KERN_PROTECTION_FAILURE`, "Thread stack size exceeded
due to excessive recursion", exit 139. It is a control-flow defect, not
memory corruption — every dereference on the way down is to valid, mapped
memory, which is why a 64 MB stack does not help and the evacuation
verifier never fires.

Three defences, matching the two ways the address goes stale:

  * `array_prototype_addr` / `object_prototype_addr` resolve the forwarding
    chain and heal the cache in place. This is what covers `js_array_grow`,
    which the collector never sees. The not-forwarded fast path stays
    call-free (`try_read_gc_header` is `#[inline(always)]`: two range
    compares plus one load of a permanently-hot `gc_flags` byte).
  * `scan_prototype_addr_cache_roots_mut` is registered in `gc_init`, so a
    relocating cycle REWRITES the slot like every other address-holding
    side table. Healing alone is not sufficient: once the from-space stub
    is swept and its block recycled the forwarded bit is gone.
  * `array_oob_prototype_get` resolves the receiver before the identity
    compare, so the guard is exact by construction rather than contingent
    on cache freshness.

Regression coverage, sabotage-verified in both directions:

  * `crates/perry/tests/gc_array_prototype_hole_read_6981.rs` — two
    programs (grow-forwarding with no GC at all, and relocation under a
    heap budget) x 5 collector arms, byte-exact vs node 26.5.0. Against
    the unfixed runtime the grow program dies at the very first arm, with
    the collector switched off.
  * `gc::tests::runtime_roots::prototype_addr_cache` — 5 unit tests, one
    per defence plus registration and the unset sentinel. Removing the
    heal reddens 2; removing the registration/scanner body reddens 2.
  * `test-files/test_gap_array_proto_grow_hole_read.ts` — parity file.

`scripts/gc_repsel_matrix.sh --arms all --pressure 8` over 440 cells goes
from `PASS=325 UNVER=100 XFAIL=1 FAIL=14` to
`PASS=339 UNVER=100 XFAIL=1 FAIL=0`; byte-exact vs node 26.5.0 425/440 ->
439/440. All 14 FAILs were `test_gap_repsel_p4a3_numarray_barriers`, in
exactly the 14 arms that run a copying minor. `evac_minor` and
`force_verify` go back into `PR_ARMS`, as that script's own comment
instructed once #6981 closed.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fcce89eb-b893-4c8e-8945-e198af48a68c

📥 Commits

Reviewing files that changed from the base of the PR and between 5dde57d and fc1d97b.

📒 Files selected for processing (11)
  • changelog.d/7071-array-prototype-addr-relocation.md
  • crates/perry-runtime/src/array/indexing.rs
  • crates/perry-runtime/src/array/mod.rs
  • crates/perry-runtime/src/gc/mod.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots/prototype_addr_cache.rs
  • crates/perry-runtime/src/value/equality.rs
  • crates/perry-runtime/src/value/mod.rs
  • crates/perry/tests/gc_array_prototype_hole_read_6981.rs
  • scripts/gc_repsel_matrix.sh
  • test-files/test_gap_array_proto_grow_hole_read.ts

📝 Walkthrough

Walkthrough

Array and object prototype address caches now heal through forwarding chains and are rewritten by relocating GC. The recursion guard resolves receivers before comparison, with runtime-root and end-to-end tests covering growth, relocation, forwarding chains, and GC configurations.

Changes

Prototype Cache Relocation

Layer / File(s) Summary
Cache healing and recursion guard
crates/perry-runtime/src/value/..., crates/perry-runtime/src/array/indexing.rs
Forwarding resolution is exposed within the crate, cached prototype addresses are healed, and out-of-bounds prototype recursion compares forwarding-resolved addresses.
Mutable GC root registration
crates/perry-runtime/src/array/mod.rs, crates/perry-runtime/src/gc/mod.rs
The prototype cache scanner is re-exported and registered during GC initialization for mutable root rewriting.
Runtime cache scanner tests
crates/perry-runtime/src/gc/tests/runtime_roots/...
Tests verify forwarding-chain resolution, cache healing, collector rewriting, scanner registration, and preservation of the unset sentinel.
End-to-end hole-read regressions
crates/perry/tests/gc_array_prototype_hole_read_6981.rs, test-files/test_gap_array_proto_grow_hole_read.ts, scripts/gc_repsel_matrix.sh, changelog.d/7071-array-prototype-addr-relocation.md
Growth and relocation scenarios run across GC configuration arms, assert termination and exact output, and document the fix and updated matrix coverage.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Suggested reviewers: andrewtdiz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main fix: stale memoized Array.prototype addresses across relocation.
Description check ✅ Passed The description is thorough and covers the issue, fix, related issue, and test evidence, though it doesn't follow the template headings exactly.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/6981-array-hole-read-recursion

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug merged commit 2545abb into main Jul 30, 2026
7 checks passed
@proggeramlug
proggeramlug deleted the fix/6981-array-hole-read-recursion branch July 30, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant