fix(gc): keep live array elements traced and initialize unused array capacity

[jdalton]

Two ways the garbage collector can mishandle arrays, both fixed here. The first one loses data: an array holding object references can have those references silently dropped during a collection, and the next read of that element gets whatever now sits at that address. In practice the program dies later with TypeError: value is not a function, at a place that has nothing to do with the array. The second one reads memory that was never written: the unused slack at the end of an array was left as raw uninitialized bytes, which the collector could mistake for pointers and follow.

Both were found while investigating a crash in a downstream command-line application. Each fix stands on its own, is independently correct, and is tested. Neither one fixes that crash; its root cause is a separate bug, described at the bottom.

Background: how the collector decides what to scan in an array

Perry's collector is generational and moving. Newly allocated objects live in a young generation, and a minor collection walks only that generation, copying whatever is still reachable somewhere else and reclaiming the rest. Copying is what makes a missed reference fatal rather than merely wasteful: an object that the collector never visits does not get its new address recorded, so any reference to it becomes a pointer to memory that has been reused.

To avoid scanning every slot of every array, each object carries a layout state describing where its pointers are. POINTER_FREE means the payload holds no pointers at all, so the collector can skip it entirely. SIDE_MASK means the object has a side table, a bitmask marking which element indices hold pointers, and the collector should consult that mask. Those two states are the ones that matter below.

Fix 1: recording a pointer restores the SIDE_MASK layout state

An array could end up in the POINTER_FREE layout state while still carrying a populated element pointer mask. One way to get there is a numeric-layout rebuild that observed a short or empty prefix and set POINTER_FREE, while the previously populated mask lingered behind it.

The two disagree, and the code that reads them does not notice. heap_payload_slot_selection short-circuits on POINTER_FREE and skips the entire payload without ever consulting the mask. So the evacuating young-generation minor collection dropped live pointer elements that sit within the array's length, and a later read of one of those elements returned a garbage pointer, which surfaces as TypeError: value is not a function.

The fix is to close the disagreement at the point it is created. layout_note_slot now restores SIDE_MASK whenever it records a pointer into an array's existing element mask, because recording a pointer is itself proof that the object is not pointer-free.

This is the same class of bug as #6831.

The diagnostic scan PERRY_GC_FROMSPACE_SCAN counts stale references from an old object into the young generation. Within-length stale array-to-young edges go from 202 to 0 with this fix. The regression test is test_pointer_store_restores_side_mask_from_stale_pointer_free.

Fix 2: initializing the unused capacity of an array

An array allocates room for more elements than it currently holds, so that appending does not reallocate every time. The region between length and capacity is slack, and it was being left as raw arena bytes, meaning whatever the allocator happened to hand back.

Raw bytes can look like a pointer by coincidence. Any scan or trace that reads beyond length follows those bits as if they were a live reference, which segfaults if the address it lands on has been evacuated. Short of a crash, it floods PERRY_GC_FROMSPACE_SCAN with false positives, roughly 4000 per collection cycle.

js_array_alloc and js_array_grow now HOLE-initialize the [length, capacity) slack. HOLE is a non-pointer sentinel value, so the slack can no longer be mistaken for a reference. This matches what js_array_alloc_with_length already did, and it is the same class as issue #323.

Uninitialized-slack false-positive edges go from roughly 4000 per cycle to roughly 0.

Test runs

cargo test -p perry-runtime passes the array suite 73 to 0 and the layout suite 78 to 0, rebased on origin/main.

The gc::tests::teardown::map_set_* failures are pre-existing and order-dependent, and are unrelated to this change.

The crash this does not fix

The value is not a function crash that prompted this investigation has a separate root cause, and it is not addressed here.

There, the evacuating minor collection drops an old object's field[1] reference to a shared young function across a collection-cycle boundary. That is a remembered-set snapshot drop, in the same area as #5029, rather than an array-layout problem. The remembered set is the collector's record of which old objects point into the young generation, and an edge missing from it means the minor collection never scans that reference at all.

The full hand-off, with evidence and the exact next diagnostic step, is pinned at #7154.

Summary by CodeRabbit

• Bug Fixes

  • Improved array memory handling during allocation and growth, preventing stale values in unused capacity.
  • Strengthened garbage collection tracking when arrays receive new pointer values, helping ensure referenced objects remain available.
  • Improved reliability when scanning and processing arrays with mixed value types.

• Tests

  • Added regression coverage for array pointer tracking and memory layout behavior.

[coderabbitai]

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: f40e4b61-fac9-4652-a9fd-5fb41b469e77

📥 Commits

Reviewing files that changed from the base of the PR and between e047966 and 4de9ad9.

📒 Files selected for processing (3)

• crates/perry-runtime/src/array/alloc.rs
• crates/perry-runtime/src/array/push_pop.rs
• crates/perry-runtime/src/gc/tests/layout_trace.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• crates/perry-runtime/src/gc/tests/layout_trace.rs
• crates/perry-runtime/src/array/alloc.rs
• crates/perry-runtime/src/array/push_pop.rs

---

📝 Walkthrough

Walkthrough

This change hardens array GC behavior. New capacity slots are initialized with TAG_HOLE. Pointer-mask updates restore GC_LAYOUT_SIDE_MASK. A regression test verifies tracing of existing and newly stored pointers.

Changes

GC array soundness hardening

Layer / File(s)	Summary
Initialize array capacity slack crates/perry-runtime/src/array/alloc.rs, crates/perry-runtime/src/array/push_pop.rs	Array allocation and growth fill unused capacity slots with TAG_HOLE.
Restore pointer-mask layout state crates/perry-runtime/src/gc/layout.rs, crates/perry-runtime/src/gc/tests/layout_trace.rs, changelog.d/gc-pointer-free-mask-invariant.md	Pointer recording restores GC_LAYOUT_SIDE_MASK. The regression test verifies both pointer-mask entries and child tracing.

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

Possibly related issues

• GC: evacuating minor drops an old-to-young field[1] edge, crashing with 'value is not a function' #7154: The array layout and slack initialization fixes address the array-related objectives, but not the separate object-field remembered-set loss.

Possibly related PRs

• PerryTS/perry#6849: Shares GC layout and array pointer-mask tracing logic.
• PerryTS/perry#7041: Shares from-space scanning and GC verification work.
• PerryTS/perry#7043: Also modifies js_array_grow, but addresses forwarding-stub diagnostics instead of capacity-slot initialization.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies both garbage collector fixes for live array tracing and unused array capacity.
Description check	✅ Passed	The description clearly explains both fixes, scope, related issues, diagnostics, and test results, although it does not use the template headings or checklist.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[proggeramlug]

Thanks — this is a real soundness fix and it lands as-is. I've pushed the missing regression coverage on top of your commit (your commit is untouched) and I'm merging.

On the fix itself

Correct, and correct for the reason you give: recording a pointer proves the object is not pointer-free, and heap_payload_slot_selection skips the whole payload on POINTER_FREE without ever consulting the mask, so a populated mask under that state is not a cosmetic desync — it is an invisible payload. The sibling branch (no mask yet, state POINTER_FREE) already restored SIDE_MASK; this one just didn't. Strictly conservative, as you say: it can only add tracing.

I also checked the one interaction that could have made it non-orthogonal — your new set_layout_state(header, GC_LAYOUT_SIDE_MASK) against the #6893 typed-layout eviction machinery (layout_set_typed_unknown is one-way, GC_OBJ_TYPED_LAYOUT_INTACT is bit 12 of GcHeader._reserved). It is orthogonal, three ways: set_layout_state masks only GC_LAYOUT_STATE_MASK | GC_LAYOUT_ALL_POINTERS (0xE000), so bit 12 is preserved and can never be set by it; an evicted object has state GC_LAYOUT_UNKNOWN, which layout_note_slot early-returns on well before your branch; and eviction also removes the object's LAYOUT_SLOT_MASKS entry, so the masks.get_mut(..) guard your code sits behind is None for exactly the objects eviction has claimed. Lock discipline is likewise unchanged: your call sits inside the same LAYOUT_SLOT_MASKS.with(|m| { let mut masks = m.borrow_mut(); … }) borrow as the pre-existing set_layout_state in the sibling else if branch, and set_layout_state is a plain _reserved bit write that takes no lock and touches no thread-local, so there is no re-entrant borrow and no double-take of the global side-table lock.

Your beyond-length finding — you reproduced an established result

This is the part worth your time, because it's context you couldn't have had from the issue tracker alone.

Your second finding — 21,888 remaining scan offenders at element indices ≥ length, and the observation that a diagnostic which evacuated them turned the crash into a SIGSEGV — independently reproduces the fromspace-scan instrument's own validation from #7050. Measured there by extending the scan with two extra axes (is the offender's owner itself GC_FLAG_FORWARDED, and is the offending slot inside visit_gc_rewrite_slots for that owner), on every cycle of every run:

[gc-fromspace-scan OFFENDERS] missing_rewrites=2054 dangling=0 owners=3 | never_dirty=6 lost_dirty=0 dirty_but_missed=2048
[... split] live_owner_refs=6 (enumerated=0 not_enumerated=6) live_owners=2  stub_owner_refs=2048 stub_owners=1 stub_forwarding_words=0

enumerated=0 throughout. 99%+ of those offenders are (1) payload words of GC_FLAG_FORWARDED array growth stubs (#233) — dead storage by construction, only word 0 is live — and (2) uninitialized bytes inside an array's unused capacity: arena blocks are recycled rather than zeroed and move_young copies the whole payload, so a len=1 cap=16 array carries 15 slots of the previous occupant's bytes into to-space, complete with plausible-looking GcHeaders and forwarding addresses. That is exactly why evacuating them SIGSEGVs — they aren't references, they're garbage that happens to decode. The consequence recorded in #7050 is that "PERRY_GC_FROMSPACE_SCAN → zero offenders" is not an attainable acceptance criterion and never was; what is zero on every cycle is the population that would be a genuine collector defect (live owner, enumerated slot).

So your conclusion is the same one that campaign reached, arrived at independently: fix it at the producers, never by tracing more in the collector. Keeping length in sync with the filled/live extent and clearing vacated tail slots in pop / shift / unshift is the right shape — js_array_pop_f64 today just decrements length and leaves the slot bits in place, which is precisely the producer half of what you measured. That follow-up is real, wanted, and yours if you want it.

What I added on top

Your PR shipped fix + changelog only, so on top of your commit:

• crates/perry-runtime/src/gc/tests/layout_pointer_free_mask.rs — two tests. test_pointer_store_into_retained_mask_restores_side_mask pins the invariant; test_retained_mask_pointer_elements_survive_copying_minor drives the same store end to end through a real evacuating young-gen minor, asserts liveness first (copied_objects > 0, array actually relocated) so an inert collection can't produce a green cell, then asserts both masked children were evacuated and their slots rewritten, then reads a field back off each relocated child.

  Verified red against the unfixed runtime — your commit reverted locally, separate CARGO_TARGET_DIR per arm, test binaries hashed and confirmed different:

  test_pointer_store_into_retained_mask_restores_side_mask ... FAILED
    left: Some(0)   right: Some(2)
  test_retained_mask_pointer_elements_survive_copying_minor ... FAILED
    slot 0 still points at the from-space child: the minor skipped this
    array's payload, so the live child was never evacuated (#7138)
    left: 2199037411368   right: 2199037411368
  

  Both pass with your fix. Note the second one reaches the collector assertion on the unfixed arm — i.e. the copying minor ran and moved objects in both arms, so the red is the payload being skipped, not an inert collector.

• test-files/test_gap_gc_array_truncate_append.ts, registered in test-parity/gc_repsel_corpus.txt — the TypeScript-level truncate-then-append shape (length = 0, truncation to a numeric prefix, pop/shift/splice, callable elements, and a long-lived array), sized to be GC-live so the relocating arms of scripts/gc_repsel_matrix.sh aren't inert against it.

• Renamed the changelog fragment to the PR-keyed convention (changelog.d/7138-gc-pointer-free-mask-invariant.md); content is yours, unchanged.

Gates (run locally on the pinned macOS arm64 host — the CI runner backlog is hours deep)

scripts/gc_repsel_matrix.sh --arms all --pressure 8, 24 corpus files × 20 arms = 480 cells:

byte-exact vs node 26.5.1: 479/480 cells
summary: PASS=379 UNVER=100 XFAIL=1 FAIL=0

FAIL=0, and the single XFAIL is the pre-existing repsel_ptr_shape_locals × rep_ptr_shape_off entry (#6976). Liveness holds — every requires=move arm reports copy-minor 23/24, every requires=scavenge arm 14/24. The new row is PASS on all 20 arms, UNVER on none, and it is the most GC-live member of the corpus:

default          cycles=196  scavenged=8 756 303
evac_minor       cycles=149  scavenged=8 682 359
cons_scan_off    cycles=148  scavenged=8 682 359
shipped_default  cycles=274  scavenged=12 766 892   <- no pressure knob, no GC env at all

main moved three corpus rows underneath this PR while it was open, so the matrix was re-run on the merged state (current main + this PR, 27 files × 21 arms = 567 cells):

byte-exact vs node 26.5.1: 566/567 cells
summary: PASS=447 UNVER=119 XFAIL=1 FAIL=0

Still FAIL=0, same single pre-existing XFAIL, new row PASS on all 21 arms, and main's three new rows — including gc_string_literal_operand_rooting (#7114), the other evacuation-sensitive one — are unaffected by the fix.

gc-ratchet (measure --repeats 7 then check), against the pinned darwin-arm64 baseline 88dcee83b, on the same host it was captured on:

• --profile shared_ci → OK, exit 0
• --profile pinned_host → OK, exit 0 (this profile additionally gates RSS and wall time)

Gated columns (heap_used/heap_total/minor_cycles/step_cycles/copied_/promoted_/freed_bytes, 72 rows) are +0.00% on retention and cycles and within ±0.06% on the evacuation counters. That is the right answer, but bit-identical gated counters are also exactly what a vacuous measurement looks like, so the ungated columns are the proof a fresh binary was actually measured: rss_bytes −0.28%…+1.48%, wall_ms −13.17%…+3.61% across the 8 probes. Runtime archives rebuilt via -p perry -p perry-runtime-static -p perry-stdlib-static with .a mtimes confirmed to have moved (the crates are rlib-only, so building them alone links a stale archive and makes the whole measurement vacuous).

cargo test --release -p perry-runtime --lib -- --test-threads=1: 1576 passed, 0 failed, 3 ignored — including both new tests. (Worth knowing if you run the suite locally: in the dev profile it aborts on gc::tests::shadow_stack_ops::out_of_range_frame_pop_is_ignored, because js_shadow_frame_pop's corrupted-handle guard uses debug_assert!(false, …) inside an extern "C" fn — a non-unwinding panic, so SIGABRT. Pre-existing on main, invisible to CI because CI runs --release, where debug_assert! compiles out. Nothing to do with this PR; use --release.)

cargo fmt --all -- --check clean. lint's other gates (check_file_size.sh, gc_store_site_inventory.py, addr_class_inventory.py) are red — but they are red with identical offender counts on pristine origin/main (16 oversize files, 17 store sites, 4 addr-class sites), and none of the offenders is in a file this PR touches. Pre-existing repo debt, not introduced here.

One note on the fixture: it builds the desynced state with a single set_layout_state call rather than replaying a producer sequence. That's deliberate and documented in the module header — every in-crate writer that sets POINTER_FREE today also drops the LAYOUT_SLOT_MASKS entry, so no single call sequence in this crate reproduces it; the masks table is keyed by address, so the 202 occurrences you measured come from side-table reuse. The invariant under test ("a populated mask entry must never be left under POINTER_FREE") is address-local and doesn't depend on how the state got there, and pinning the test to one speculative producer route would have made it weaker.

Thanks again — good find, good writeup, and the draft framing (partial fix, named remaining cause, explicit "do not fix this in the collector") is exactly the right way to hand off a partially-understood soundness bug.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/perry-runtime/src/array/alloc.rs (1)

116-137: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Initialize js_array_alloc_with_length capacity slack.

For capacity < MIN_ARRAY_CAPACITY, this function allocates actual_capacity slots but only writes 0..capacity. A small new Array(n) with n < 16, small Array.from materialization targets, small splice removed arrays, and small sort scratch buffers can therefore leave raw arena bytes in [length, actual_capacity) while the constructor path marks the array as raw-f64 holes. Match js_array_alloc and write TAG_HOLE through actual_capacity.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/array/alloc.rs` around lines 116 - 137, Update
js_array_alloc_with_length to initialize every allocated element slot through
actual_capacity, not just the requested capacity. Keep length set to capacity
while writing TAG_HOLE across the full slack range, matching js_array_alloc and
preserving the existing layout initialization.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/perry-runtime/src/array/alloc.rs`:
- Around line 116-137: Update js_array_alloc_with_length to initialize every
allocated element slot through actual_capacity, not just the requested capacity.
Keep length set to capacity while writing TAG_HOLE across the full slack range,
matching js_array_alloc and preserving the existing layout initialization.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 77001956-d8b1-436e-b01e-2bf3221cb704

📥 Commits

Reviewing files that changed from the base of the PR and between a3b31c0 and e047966.

📒 Files selected for processing (5)

• changelog.d/gc-pointer-free-mask-invariant.md
• crates/perry-runtime/src/array/alloc.rs
• crates/perry-runtime/src/array/push_pop.rs
• crates/perry-runtime/src/gc/layout.rs
• crates/perry-runtime/src/gc/tests/layout_trace.rs