fix(gc): root globalThis across its own lazy builtin population (#6982)

[proggeramlug]

Fixes the largest slice of #6982 / #6981.

First: the premise in #6982 was wrong, and the symbolized stacks say so

#6982 recorded that the five crashers faulted on ldrb w8, [x21, #-0x8]! and concluded this was "a GC-header obj_type read at addr-8 — the collector validating a pointer that should never have reached it". It also flagged that as an n=2 hypothesis matched by offset into an unnamed symbol, and asked for a symbols build before any fix. That was the right call.

No special build was needed: post_link.rs::strip_final_binary skips strip when PERRY_DEBUG_SYMBOLS=1 is set. With real symbols, all five:

repsel_canonical_i32 / repsel_ptr_shape_locals   SIGBUS  0x18400001fc
  0  js_array_length + 488
  1  js_object_set_field_by_name + 5964
  2  populate_global_this_builtins + 45592
  3  js_get_global_this + 156
  4  js_get_global_this_builtin_value + 72

repsel_proven_this_frozen                        SIGSEGV 0x434c4f5300000010
  0  descriptor_state::get_accessor_descriptor + 136
  1  js_object_set_field_by_name + 5728
  2  populate_global_this_builtins + 45592

ta_param_numeric_read                            SIGSEGV 0x004e614e00000008
  0  descriptor_state::set_builtin_property_attrs + 456
  1  populate_global_this_builtins + 46820

typedarray_param_read                            SIGBUS  0x300000020b
  0  get_field_by_name_tail::get_field_by_name_object_tail + 10444
  1  js_object_get_field_by_name + 8048
  2  js_get_global_this_builtin_value + 396

There is not a single GC frame in any of them. No mark, no trace, no copying, no evacuate. The faulting ldrb [x21, #-0x8]! is js_array_length reading an array header at arr-8 — an ordinary mutator read, not collector validation. The collector is not the faulting code; it is the code that moved the object out from under the mutator.

The faulting addresses are the tell: their high halves are ASCII — 0x434c4f53 is "CLOS", 0x004e614e is "NaN" — relocated string payload read where a pointer/descriptor was expected.

Root cause

js_get_global_this registers two root slots for the freshly allocated singleton (THREAD_GLOBAL_THIS, GLOBAL_THIS_PTR), with a long comment explaining that this exists precisely so an evacuating collector rewrites them on a move. It then hands the raw, pre-GC pointer by value to populate_global_this_builtins, which installs several hundred builtins and allocates on nearly every step:

crate::gc::js_gc_register_global_root(cache_slot as i64);           // slot IS rewritten
crate::gc::runtime_store_root_atomic_raw_i64(&GLOBAL_THIS_PTR, ..); // slot IS rewritten
populate_global_this_builtins(new_ptr as *mut ObjectHeader);        // <-- raw, never re-read

Those allocations are exactly the 4 579–5 965 objects the copying minor reports. When it relocates the singleton mid-population, the two registered slots are rewritten and the by-value argument is not, so every later js_object_set_field_by_name(singleton, ..) / set_builtin_property_attrs(singleton as usize, ..) addresses the dead from-space copy whose bytes have been recycled. js_get_global_this then returns that stale address too.

The GC diagnostics for the cycle immediately before each crash confirm the shape: compiled_shadow and shadow_roots entirely zero, native_stack_fallback.decision = "skip_disabled". Nothing kept that pointer alive or current.

Same class as #6951/#6972 (a raw reference held across a collection point), one layer further out. Only reachable with the conservative native-stack scan off — production's Auto -> SkipDisabled — which is #6981's "the scan is load-bearing for correctness": it was pinning the argument register.

The fix

Root the singleton in a RuntimeHandleScope and re-read it through the handle at every use (root_raw_mut_ptr / get_raw_mut_ptr — the idiom array/header.rs already uses), and return the value re-read from the registered cache slot. The two helpers that take the singleton (alias_number_static_to_global_function, alias_typed_array_proto_to_string) had the same window and get the same treatment.

singleton is rebound as a closure rather than a value, so the conversion is exhaustive by construction: any use not converted fails to compile.

Measurements

macOS arm64, pinned Node 26.5.0, release build from aa1c15028, evacuating precise-roots arm PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off, with copied_objects > 0 verified on every run (4 579 / 4 640 / 4 614 / 5 948 / 5 965 before; 9 672 / 15 368 / 15 409 after, on the files that now run to completion).

Control arms pin causality — on the unfixed build each of the five passes with the conservative scan on, passes with precise roots and no pressure, and crashes only with both.

corpus file	before	after
repsel_canonical_i32	SIGBUS	OK (byte-identical to Node)
ta_param_numeric_read	SIGSEGV	mismatch (no longer crashes)
typedarray_param_read	SIGBUS	mismatch (no longer crashes)
repsel_ptr_shape_barriers	SIGSEGV	mismatch (no longer crashes)
repsel_ptr_shape_locals	SIGBUS	SIGSEGV — different, now-unmasked defect (#6991)
repsel_proven_this_frozen	SIGSEGV	TypeError: bump is not a function (#6992)

Crashes on that arm: 6 -> 2. All 21 corpus files remain byte-identical to Node on the as-shipped configuration — no regression.

One defect or several? (the question #6982 asked)

The five crashes shared one blocking defect, and it was masking at least two more. All five faulted downstream of the same js_get_global_this_builtin_value -> populate_global_this_builtins spine, at four different sites, purely depending on where the recycled bytes landed. Removing that shared first hurdle made two independent defects observable:

• gc: compiled constructor receiver goes stale across the globalThis-population collection (unmasked by #6982) #6991 — repsel_ptr_shape_locals now crashes with a completely different 2-frame stack (..._Pt_constructor + 236 -> js_object_set_field_by_name + 3044): a compiled constructor's receiver going stale across the same collection. That is the ctor-receiver rooting class fix(gc): operand temporaries are precise roots — collection methods, constructor args, string-method receiver (#6970, #6969, #6971) #6983 is already addressing, so it is deliberately untouched here.
• gc: proven-this method value is lost across a relocating minor (TypeError: bump is not a function) #6992 — repsel_proven_this_frozen no longer crashes but loses a method value.

What I did NOT do, and one thing reviewers should know

No cargo-test-visible unit test. I verified the fix on the real reproducer in both directions, but I did not land a Rust unit test, and I would rather say so than imply coverage that does not exist. The guards needed to force a relocating minor at a controlled point (CopyingNurseryTestGuard, ConservativeScanDisabledGuard, GcTriggerThresholdTestGuard::make_arena_trigger_due) are pub(super) inside gc::tests, while the functions under test are private to object::global_this; bridging them is real work and I did not want to ship a test whose teeth I had not verified in both directions.

The corpus does not gate this class on PRs — filed as #6993. Measured directly on the unfixed binaries, every PR-gated arm is green:

                        default  verify_evac  cons_scan_off  shipped_default
repsel_canonical_i32    0        0            0              0
typedarray_param_read   0        0            0              0

PR_ARMS is default,verify_evac,cons_scan_off,shipped_default, and the evacuating base needs PERRY_GC_INCREMENTAL=0 and PERRY_CONSERVATIVE_STACK_SCAN=off together — a combination that appears only in --arms all, which runs on push, not on pull_request. I did not simply add the arm to PR_ARMS because 13 corpus cells are still red on it and that would block every unrelated PR.

Verified on the quiet mini, pinned Node 26.5.0, cargo fmt --all -- --check clean.

Summary by CodeRabbit

• Bug Fixes
  • Fixed a GC-related correctness issue during globalThis lazy singleton population that could lead to stale pointers and crashes while initializing global built-ins.
  • Made global built-in installation and related aliasing logic relocation-safe, ensuring globalThis reads return the currently valid object after garbage collection moves it.
• Documentation
  • Updated the changelog with the fix details and validation results, noting any remaining separately tracked discrepancies.

[coderabbitai]

📝 Walkthrough

Walkthrough

Global builtin population now roots globalThis and re-reads it after allocations that may trigger evacuating GC. Alias helpers root their referenced objects, and js_get_global_this returns the collector-updated cached pointer.

Changes

GlobalThis GC safety

Layer / File(s)	Summary
Root globalThis during builtin population changelog.d/6994-gc-globalthis-population-rooting.md, crates/perry-runtime/src/object/global_this/populate.rs	populate_global_this_builtins uses a rooted handle and rewires builtin, namespace, accessor, constant, and constructor installations to re-read the moved singleton.
Root pointer-sensitive global aliases crates/perry-runtime/src/object/global_this/populate.rs	Typed-array and Number alias helpers root globalThis and resolved values before performing pointer-sensitive property writes.
Return the updated globalThis pointer crates/perry-runtime/src/object/global_this/fetch_globals.rs	js_get_global_this prefers the thread-local cache updated by GC and falls back to the initially allocated pointer when the cache is empty.

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

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names the GC fix for rooting globalThis during lazy builtin population and matches the main change.
Description check	✅ Passed	It covers the summary, root cause, fix, related issues, measurements, and verification, though it doesn't use the template headings verbatim.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/6982-collector-pointer-validation

---

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.}

[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/object/global_this/populate.rs (1)

216-237: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Root live closure/object pointers across the intervening allocations in populate_global_this_builtins.

singleton() no longer holds a stale address, but this function still derives and reuses un-rooted sibling pointers across allocation-heavy sections:

• The Storage/WebCrypto path captures closure_ptr/proto_obj/ctor_value before many string allocations, then passes them to install_storage_globals(...) and install_global_webcrypto(singleton()). install_storage_globals installs methods/accessors and creates storage objects without rooting the passed pointers.
• Global function closures are allocated at line 499, then name_key is allocated at line 523 before fn_value is created from the same raw closure_ptr at line 524 and written at line 525.
• Namespace objects are allocated at 564, then namespace installers run before ns_value is re-derived from raw ns_obj at 603.
• install_global_webcrypto(singleton()) hands an address into native_module.rs, whose body allocates the crypto key string before dereferencing that value.

Root these with crate::gc::RuntimeHandleScope and reload from the handles before the following allocations/usages to avoid dangling references from GC evacuation.

🤖 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/object/global_this/populate.rs` around lines 216 -
237, Root all live closure, prototype, constructor, storage, WebCrypto,
global-function, and namespace object pointers in populate_global_this_builtins
with crate::gc::RuntimeHandleScope before allocation-heavy operations. Reload
pointer values from their handles immediately before install_storage_globals,
install_global_webcrypto, property writes, and namespace installation; also
update install_global_webcrypto to receive and reload a rooted handle before its
crypto-key string allocation.

Source: Learnings

🤖 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/object/global_this/populate.rs`:
- Around line 216-237: Root all live closure, prototype, constructor, storage,
WebCrypto, global-function, and namespace object pointers in
populate_global_this_builtins with crate::gc::RuntimeHandleScope before
allocation-heavy operations. Reload pointer values from their handles
immediately before install_storage_globals, install_global_webcrypto, property
writes, and namespace installation; also update install_global_webcrypto to
receive and reload a rooted handle before its crypto-key string allocation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ff9c6949-7103-4e29-8bd5-9b5f7fa2c031

📥 Commits

Reviewing files that changed from the base of the PR and between aa1c150 and bc998a2.

📒 Files selected for processing (3)

• changelog.d/6994-gc-globalthis-population-rooting.md
• crates/perry-runtime/src/object/global_this/fetch_globals.rs
• crates/perry-runtime/src/object/global_this/populate.rs

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@changelog.d/6994-gc-globalthis-population-rooting.md`:
- Line 1: Remove the ### Fixed heading from the changelog fragment, leaving only
the entry body.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 29ca2d51-79d6-4515-be84-289d63fd4aa9

📥 Commits

Reviewing files that changed from the base of the PR and between bc998a2 and 2415771.

📒 Files selected for processing (3)

• changelog.d/6994-gc-globalthis-population-rooting.md
• crates/perry-runtime/src/object/global_this/fetch_globals.rs
• crates/perry-runtime/src/object/global_this/populate.rs

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

• crates/perry-runtime/src/object/global_this/fetch_globals.rs

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the changelog heading.

This fragment must contain only the entry body; ### Fixed is a section header.

Proposed fix

-### Fixed
-

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

### Fixed

🤖 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 `@changelog.d/6994-gc-globalthis-population-rooting.md` at line 1, Remove the
### Fixed heading from the changelog fragment, leaving only the entry body.

Sources: Coding guidelines, Learnings