gc: native roots beyond aarch64-macOS — x86-64, iOS, iPadOS, tvOS - #7349
Merged
Conversation
PERRY_RS4GC=1 was refused off aarch64 (#7324). The refusal was right — the collector segfaulted rather than reporting anything — but the cause was one unsupported call, not anything architectural. On x86-64 every root is Indirect [RSP + off], DWARF register 7, and the unwinder path resolved bases with _Unwind_GetGR(context, reg). _Unwind_GetGR is not a supported query for the stack-pointer column; it returned garbage the collector wrote through. _Unwind_GetCFA is the supported way. SP-relative roots now derive from the CFA: by the SysV/AAPCS definition it is the caller's stack pointer immediately before the call, so the body stack pointer sits one return-address slot plus this function's frame below it, and stack_size is exactly that frame, already in the map. The architecture's SP register number is a runtime-local constant, deliberately separate from the format's base tags — those stay aarch64-literal so the compiler's idea of the target and the runtime's target_arch cannot disagree. Measured on real x86-64 Linux: 10/10 probes byte-match the pinned oracle under forced evacuation with verification, .perry_gcmap present in every binary. The walker ran rather than passing vacuously — telemetry reports walks=1, frames_visited=10, records_matched=1, locations_visited=2 with fp_walks=0, and evacuation moved objects (retained_forwarded_stub_objects=5). aarch64 telemetry has the same shape (7/0/1/1 vs 10/0/1/1), which makes this an equivalence result. Those counts are low on BOTH platforms because the probes end in a manual gc() from a shallow stack — a pre-existing gate weakness worth naming, not something this introduces.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR enables pointer-width-aware compact GC maps and CFA-based x86-64 stack-pointer root resolution. It expands native GC-root support to 64-bit Apple targets and Linux, updates CI coverage, and documents validation results. ChangesNative GC-root support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@crates/perry-runtime/src/gc/roots/stack_maps.rs`:
- Around line 878-889: The SP-relative base calculation in the stack-map
handling must use an architecture-specific return-address adjustment: update the
base logic around _Unwind_GetCFA so x86-64 subtracts one pointer-sized
return-address slot while AArch64 subtracts zero before applying
record.stack_size. Update changelog.d/7350-statepoints-x86-64.md lines 13-20 to
document the separate CFA-to-SP calculations for both architectures, then
validate with cargo check -p perry --profile perry-dev.
🪄 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: 1e0ab59f-316d-4095-82e1-721225512af9
📒 Files selected for processing (3)
changelog.d/7350-statepoints-x86-64.mdcrates/perry-codegen/src/gc_map.rscrates/perry-runtime/src/gc/roots/stack_maps.rs
statepoints-refuse-x86 asserted that native roots REFUSE on x86-64. They work now, so that job would fail on its own success message — which said exactly what to do: 'add the x86-64 host to native-roots-rs4gc-aarch64 (rename it) and delete this job'. native-roots-rs4gc is now a two-host matrix: macos-14 for aarch64 + Mach-O, and ubuntu-latest for x86-64 + ELF. The toolchain step picks brew LLVM or the system/apt pair per host, and the liveness assertions read otool or readelf per object format. The Mach-O-only in-process step is gated to the macOS arm. ELF matters more than the arch here: every object-format bug in this design was ELF-only and invisible on Mach-O — SHF_GNU_RETAIN or --gc-sections drops the section, SHF_WRITE or the relocated addresses force a DT_TEXTREL, and eh_walker's asm used the Mach-O underscore convention. ARM64 Linux would cover the fourth corner, but those runners queue for hours here and its two components are each covered now.
There was a problem hiding this comment.
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 @.github/workflows/gc-native-roots.yml:
- Around line 53-57: Complete the explanatory comment around the x86-64 matrix
entry by adding the missing action before “binary that crashes under
collection,” so the sentence is grammatically complete and accurately describes
what the old note claimed the run would do.
🪄 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: 551f1935-0a06-4e7d-b6b2-7b07a3ddbd01
📒 Files selected for processing (1)
.github/workflows/gc-native-roots.yml
Comment on lines
+53
to
+57
| # x86-64 was refused outright until #7349 taught the runtime to derive an | ||
| # SP-relative base from the CFA; it is a first-class arm of the matrix now. The | ||
| # old note said (#7324) that a run there would | ||
| # binary that crashes under collection, so an x86-64 run of this matrix would | ||
| # test nothing but the refusal — which is what `statepoints-refuse-x86` is for. | ||
| # test nothing but the refusal — no longer true, and that job is gone. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the incomplete documentation sentence.
Line 55 omits the action before “binary that crashes under collection.”
Proposed fix
-# old note said (`#7324`) that a run there would
+# old note said (`#7324`) that a run there would produce a📝 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
| # x86-64 was refused outright until #7349 taught the runtime to derive an | |
| # SP-relative base from the CFA; it is a first-class arm of the matrix now. The | |
| # old note said (#7324) that a run there would | |
| # binary that crashes under collection, so an x86-64 run of this matrix would | |
| # test nothing but the refusal — which is what `statepoints-refuse-x86` is for. | |
| # test nothing but the refusal — no longer true, and that job is gone. | |
| # x86-64 was refused outright until `#7349` taught the runtime to derive an | |
| # SP-relative base from the CFA; it is a first-class arm of the matrix now. The | |
| # old note said (`#7324`) that a run there would produce a | |
| # binary that crashes under collection, so an x86-64 run of this matrix would | |
| # test nothing but the refusal — no longer true, and that job is gone. |
🤖 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 @.github/workflows/gc-native-roots.yml around lines 53 - 57, Complete the
explanatory comment around the x86-64 matrix entry by adding the missing action
before “binary that crashes under collection,” so the sentence is grammatically
complete and accurately describes what the old note claimed the run would do.
iOS and iPadOS are aarch64 + Mach-O, the same shape as macOS. They did not work because the Mach-O loader, the unwinder module, the fp-chain walker and stack_top were each cfg(target_os = "macos"). On any other Apple platform that selected the no-section stub: loaded_stack_map_section() returned None, the index was empty, and the collector ran with NO native roots — silently, on the platforms hardest to debug. The compiler emitted the map; nothing read it. All four gates are now the same predicate: 64-bit Apple, or Linux. pthread_get_stackaddr_np is Apple-wide, and the mach2 dependency was widened to match the code using it — declaring it for fewer targets than the loader compiles on is how this stayed hidden. watchOS is refused deliberately: arm64_32 has 32-bit pointers while the map stores u64 addresses and the runtime does usize arithmetic on them. The check is ordered before the arm64 prefix test so it actually fires. Verified by building perry-runtime for aarch64-apple-ios, -ios-sim and -tvos. That is what found the hole: stack_top did not exist on iOS, so the build failed outright instead of quietly picking the stub. visionOS still fails in the third-party psm build script, unrelated. A device/simulator run is the verification this does not yet have.
proggeramlug
changed the title
…tchOS Two changes, one of them a bug CodeRabbit caught in the CFA derivation I added for x86-64. The return-address adjustment is NOT architecture-independent. x86-64 `call` pushes the return address, so the body stack pointer is CFA - 8 - stack_size. aarch64 `bl` writes it to x30 and pushes nothing, so it is CFA - stack_size. Subtracting the slot unconditionally shifted every SP-relative root by a word on aarch64 — and it would have stayed latent there, because chain_walkable is true on aarch64 so the fast x29 walker runs and this path is only the fallback. The probes passed 10/10 without ever exercising it. watchOS is no longer refused. The blocker was the map's function-address field being a fixed u64 while arm64_32 is ILP32; it now follows the target's pointer width, and the header's previously-reserved flags field records which width was used. The decoder asserts that against its own usize and refuses a mismatch, so a map built for the other width fails loudly instead of misreading every function address. Entries are 16 bytes on LP64, 12 on ILP32. Tests cover both widths: the emitter must produce .long/.quad and the matching flag, and the decoder must reject a blob whose recorded width disagrees. Local compile-verification for arm64_32 is still blocked by the third-party psm build script, which does not cross-compile in this environment — that is unrelated to this code, and the same failure blocks visionOS. Also fixes the incomplete sentence CodeRabbit flagged in the workflow header.
There was a problem hiding this comment.
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/gc/roots/stack_maps.rs (1)
896-913: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftDo not silently drop roots from unresolved CFA arithmetic.
checked_sub(...).and_then(...)rejects invalid SP base calculation withcontinue;walk_framecan then report successful statistics for the rest of native stack-map roots. A malformed map, ABI mismatch, or invalid frame can remove a live root from the scan. Fail the native-stack-walk path instead of continuing with a partial root set.🤖 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/gc/roots/stack_maps.rs` around lines 896 - 913, Update the SP-relative base calculation in walk_frame so failed CFA arithmetic propagates a native stack-walk failure instead of using continue. Preserve the checked subtraction, but return or propagate the function’s existing error result when it yields None, ensuring malformed frame data cannot produce successful statistics with a partial root set.Source: Coding guidelines
🧹 Nitpick comments (1)
crates/perry-runtime/src/gc/roots/stack_maps.rs (1)
848-851: 🩺 Stability & Availability | 🔵 TrivialVerify
_Unwind_GetCFAon every enabled Apple target.The new declaration is compiled for the expanded 64-bit Apple set. Build success does not prove that the symbol links and returns the expected CFA on iOS, tvOS, or visionOS. The supplied objectives also state that device and simulator execution remains unverified. Add a forced-evacuation probe that confirms nonzero unwinder walks and successful root relocation on each enabled target. The published
mach20.6.0 platform table lists macOS and iOS coverage, but not tvOS, visionOS, or watchOS. (docs.rs)As per coding guidelines, validate the Rust change with
cargo check -p perry --profile perry-dev.🤖 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/gc/roots/stack_maps.rs` around lines 848 - 851, Add a forced-evacuation GC probe covering every enabled Apple target, including device and simulator variants, that verifies _Unwind_GetCFA produces nonzero unwinder walks and that roots are relocated successfully; ensure the probe executes on iOS, tvOS, and visionOS rather than relying only on compilation. Then validate the Rust changes with cargo check -p perry --profile perry-dev.Sources: Coding guidelines, MCP tools
🤖 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/gc/roots/stack_maps.rs`:
- Around line 896-913: Update the SP-relative base calculation in walk_frame so
failed CFA arithmetic propagates a native stack-walk failure instead of using
continue. Preserve the checked subtraction, but return or propagate the
function’s existing error result when it yields None, ensuring malformed frame
data cannot produce successful statistics with a partial root set.
---
Nitpick comments:
In `@crates/perry-runtime/src/gc/roots/stack_maps.rs`:
- Around line 848-851: Add a forced-evacuation GC probe covering every enabled
Apple target, including device and simulator variants, that verifies
_Unwind_GetCFA produces nonzero unwinder walks and that roots are relocated
successfully; ensure the probe executes on iOS, tvOS, and visionOS rather than
relying only on compilation. Then validate the Rust changes with cargo check -p
perry --profile perry-dev.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 363ccef5-36c5-404b-8c75-072815f029e6
📒 Files selected for processing (4)
changelog.d/7349-apple-platforms.mdcrates/perry-codegen/src/gc_map.rscrates/perry-runtime/Cargo.tomlcrates/perry-runtime/src/gc/roots/stack_maps.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry-codegen/src/gc_map.rs
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
#7360) * test(gc): collect at stack depth, and gate walker liveness on every arm The native-root walker had no probe that made it work. Every probe in the suite calls `gc()` at the end, from a shallow stack, so on macOS and Linux `04_dead_after_deep_stack` reported 7 frames visited and **zero** root locations. Both arms would have passed unchanged with a walker that visited nothing at all -- other root sources covered the probes. Windows only walked a deep stack (5,626 frames) by accident of heap sizing, which is why the `--require-locations` gate could be applied there and nowhere else. `11_collect_at_depth` makes that coverage deliberate. `descend` holds a heap value live ACROSS its recursive call and collects at the deepest point, so at collection time there is one live root per frame, all of them mid-frame rather than in the leaf. Every slot is read after the collection returns, so a walker that stops early -- or a map with a wrong base register -- produces a wrong checksum, not merely a slower run. Under `PERRY_GC_FORCE_EVACUATE=1` every survivor moves, so a stale pointer cannot be accidentally right. Measured, byte-matching the pinned Node oracle: macOS aarch64 228 frames, 222 records, 221 locations (was 7 / 0) x86-64 Linux 231 frames, 221 locations (was 7 / 0) With both Unix arms now walking a real stack, `--require-locations` moves from the Windows-only branch to the shared path and gates all three. Full ratchet suite: 11/11 byte-identical to the oracle under `PERRY_RS4GC=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1`. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF * docs: changelog fragment for #7359 Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF * ci(gc): add the aarch64-Linux arm, the one uncovered corner of the map The matrix covered aarch64+Mach-O, x86-64+ELF and x86-64+PE, and the note explaining the gap said ARM64 Linux was skipped because "its two components are each covered above." That is the exact compositional fallacy `word_width_for` in `gc_map.rs` exists to warn about. `.word` is not a fixed size -- GNU `as` defines it as the target's natural machine word -- so LLVM's AArch64 ELF backend spells every 32-bit stack-map field `.word`, while both covered arms spell it `.long`: Mach-O uses `.long` on aarch64, and on x86 `.word` means *two* bytes so LLVM will not use it for a 32-bit field. The directive width is a property of the intersection, not of either component. What this arm does and does not add, stated precisely, because overclaiming in this file is how #7321's wrong explanation survived into an issue and a job name: the `.word` spelling is already unit-tested on every arm, against a hand-written sample. What no arm has ever exercised is the end-to-end chain on this target -- real LLVM asm output, real ELF linking, real runtime walking -- where the failure mode is not a parse error but a wrong answer. `ubuntu-24.04-arm` is already in use in release-packages.yml and the repo is public, so the runners are available; the "queue for hours" half of the old rationale is stale too. Also repairs a garbled sentence in the header comment, left by an edit that spliced two clauses about the pre-#7349 x86-64 refusal. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF * docs(ci): the knob ledger said PERRY_STATEPOINT_REPORT was deleted; it is internal plumbing The env *spelling* was deleted under the kill policy (#7314) and the flag is the only entry point -- but the variable itself is still how the driver hands the format to the rayon module workers, and run_pipeline.rs remove_var's it when the flag is absent so an inherited value cannot switch reporting on. That block is a knob ledger. An entry reading "deleted" for a name still greppable in the tree makes the whole list look stale to the next auditor. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…tform matrix (#7367) The plan still said statepoints were aarch64-only (#7321), that the matrix "therefore runs on macos-14", that `statepoints-refuse-x86` pinned the refusal, and it spelled the knob `PERRY_STATEPOINTS` four times. None of that is true now, and this document is what the adoption decision gets made from. What actually changed: - x86-64 is unblocked. `_Unwind_GetGR(ctx, 7)` does segfault and cannot be fixed as stated -- libgcc tracks only the columns CFI restores and RSP is derived, not tracked. #7349 stopped asking for it and derives the SP-relative base from `_Unwind_GetCFA`, with a per-arch return-address adjustment (x86-64 `call` pushes one, aarch64 `bl` does not). x86-64 Linux is a first-class arm. - Windows works via RtlVirtualUnwind (#7355), the one walker with no Itanium unwinder beneath it. - aarch64+ELF is now covered too (#7360) -- the only shape where LLVM spells 32-bit stack-map fields `.word`. - One mechanism, not two: PERRY_STATEPOINTS and the plain-map bridge are deleted, so the kill-policy line about "a mode that still exists" no longer applies to this pair. - The gate proves something now. Until today the Unix arms reported 7 frames and ZERO locations -- they would have passed with a walker that visited nothing. #7359's deep-collect probe took them to 221 locations. - watchOS/visionOS are not blocked by Perry: they build on stable without `dyn-eval`, and fail three crates away in psm's Mach-O guard. So the remaining adoption gate is `llvm-inprocess` becoming a default cargo feature, plus sequencing step 2 (root density) -- adopting today would regress binary size on root-dense code. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…#7369) * fix(lint): split index_set.rs, over the 2000-line cap since #7342 (#7366) `scripts/check_file_size.sh` exits 1 on main HEAD: `crates/perry-codegen/src/expr/index_set.rs` is 2035 lines against a 2000 cap. It crossed in #7342. That script runs inside the `lint` job, which is a REQUIRED context -- so this is the second independent way `lint` was red on main today (the first was rustfmt on linker.rs, #7361). A required check that is red on main blocks nothing; it means every merge is a bypass. The split follows the recipe in the script's own failure message: extract a topical group into a sibling module. `lower_inline_dyn_typed_array_set` and its `emit_inline_ta_int_store` helper are the guarded inline typed-array store for a type-erased receiver -- one coherent unit, moved verbatim to `index_set_typed_array.rs`. index_set.rs drops to 1749 lines, leaving real headroom rather than landing one line under the cap. Mechanical move: the two functions are byte-identical, only the imports they need travelled with them and `lower_inline_dyn_typed_array_set` became `pub(super)` so its one caller can still reach it. cargo test -p perry-codegen --lib: 609 passed. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com> * docs(plan): both statepoint adoption gates are closed; record the platform matrix (#7367) The plan still said statepoints were aarch64-only (#7321), that the matrix "therefore runs on macos-14", that `statepoints-refuse-x86` pinned the refusal, and it spelled the knob `PERRY_STATEPOINTS` four times. None of that is true now, and this document is what the adoption decision gets made from. What actually changed: - x86-64 is unblocked. `_Unwind_GetGR(ctx, 7)` does segfault and cannot be fixed as stated -- libgcc tracks only the columns CFI restores and RSP is derived, not tracked. #7349 stopped asking for it and derives the SP-relative base from `_Unwind_GetCFA`, with a per-arch return-address adjustment (x86-64 `call` pushes one, aarch64 `bl` does not). x86-64 Linux is a first-class arm. - Windows works via RtlVirtualUnwind (#7355), the one walker with no Itanium unwinder beneath it. - aarch64+ELF is now covered too (#7360) -- the only shape where LLVM spells 32-bit stack-map fields `.word`. - One mechanism, not two: PERRY_STATEPOINTS and the plain-map bridge are deleted, so the kill-policy line about "a mode that still exists" no longer applies to this pair. - The gate proves something now. Until today the Unix arms reported 7 frames and ZERO locations -- they would have passed with a walker that visited nothing. #7359's deep-collect probe took them to 221 locations. - watchOS/visionOS are not blocked by Perry: they build on stable without `dyn-eval`, and fail three crates away in psm's Mach-O guard. So the remaining adoption gate is `llvm-inprocess` becoming a default cargo feature, plus sequencing step 2 (root density) -- adopting today would regress binary size on root-dense code. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com> * gc: admit two provably-leaf helpers, and measure that it buys nothing js_gc_register_global_root was the most frequent non-leaf callee in the probe suite (148 call sites) and is provably GC-leaf: its whole body is runtime_write_barrier_root_heap_word -- which js_write_barrier_root_heap_word, already CannotCollect, wraps in one line -- plus a TLS Vec::push. The "malloc count threshold" trigger does not apply to that push: the counter is MALLOC_STATE.objects.len(), a registry of Perry GC objects, and the #[global_allocator] is plain mimalloc/System with no GC hook. js_typed_feedback_maybe_dump_trace joins its already-admitted family siblings. Measured A/B on the same tree, and the result is a null: probe safepoints roots total bytes __text 06_string_retention 105 -> 100 27=27 0 -4 B 09_try_catch_roots 343 -> 339 259=259 0 -4 B 11_collect_at_depth 120 -> 117 36=36 0 -4 B Root counts are IDENTICAL. The 40 safepoints removed across the suite were all rootless, and a rootless safepoint costs essentially nothing -- which is what docs/engine-plan.md already says: "the axis is not 'statepoints are bigger', it is 'roots are bigger'". Recording it as evidence: the safepoint-count lever is not the binary-size lever, so sequencing step 2 must attack live-root SETS. Two tests come with it. One pins the wrapper's classification to the barrier it wraps. The other pins js_nanbox_string OUT of the allowlist: at 120 call sites it is the obvious next candidate and reads as pure bit manipulation, but its null guard calls js_string_from_bytes to allocate an empty string. Probe suite 11/11 byte-identical under forced evacuation + verification. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…7368) * fix(codegen): statepoint report counted zero safepoints since #7348 #7348 deleted the explicit bridge and with it the only callers of note_statepoint and note_skipped -- they lived in the bridge, which counted safepoints as it emitted them. The methods survived with no callers, so statepoints, relocations, max_live_roots, skipped_non_safepoints, live_roots_histogram and both by-callee maps went structurally zero in production. A real compile printed "0 statepoints emitted" while its binary carried 120. Counting at IR-emission time cannot work any more, and that is the lesson: Perry no longer decides which calls become safepoints -- RewriteStatepointsForGC does, inside LLVM. The only honest source is the compact-map rewrite, which already parses the assembly LLVM emitted and computed these exact numbers before dropping them into log::debug!. The report reads from there now: 120 safepoints across 6 function(s) in 1 module(s) 36 live roots recorded, 0.30 per safepoint An absent measurement no longer renders as a measured zero: gc_map.modules == 0 means "never reported", the text report says UNAVAILABLE rather than printing zeros, and JSON carries gc_map separately from totals so a consumer can tell them apart. schema_version -> 2. The CI gate now asserts the counts, not just the label. --only-backend rs4gc passed throughout the regression -- the label was right, the numbers were fiction. It now also requires records > 0 and roots > 0; verified against a synthetic report with the #7348 shape, where the label check still reports 9 functions green while the count checks exit 1. Second round of dead counters here (#7362 removed four that never had a writer at all). The new test documents why the first invariant missed this one: every_rendered_counter_has_a_writer called the mutators itself, so "has a writer" passed while "is written" was false. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF * gc: admit two provably-leaf helpers, and measure that it buys nothing (#7369) * fix(lint): split index_set.rs, over the 2000-line cap since #7342 (#7366) `scripts/check_file_size.sh` exits 1 on main HEAD: `crates/perry-codegen/src/expr/index_set.rs` is 2035 lines against a 2000 cap. It crossed in #7342. That script runs inside the `lint` job, which is a REQUIRED context -- so this is the second independent way `lint` was red on main today (the first was rustfmt on linker.rs, #7361). A required check that is red on main blocks nothing; it means every merge is a bypass. The split follows the recipe in the script's own failure message: extract a topical group into a sibling module. `lower_inline_dyn_typed_array_set` and its `emit_inline_ta_int_store` helper are the guarded inline typed-array store for a type-erased receiver -- one coherent unit, moved verbatim to `index_set_typed_array.rs`. index_set.rs drops to 1749 lines, leaving real headroom rather than landing one line under the cap. Mechanical move: the two functions are byte-identical, only the imports they need travelled with them and `lower_inline_dyn_typed_array_set` became `pub(super)` so its one caller can still reach it. cargo test -p perry-codegen --lib: 609 passed. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com> * docs(plan): both statepoint adoption gates are closed; record the platform matrix (#7367) The plan still said statepoints were aarch64-only (#7321), that the matrix "therefore runs on macos-14", that `statepoints-refuse-x86` pinned the refusal, and it spelled the knob `PERRY_STATEPOINTS` four times. None of that is true now, and this document is what the adoption decision gets made from. What actually changed: - x86-64 is unblocked. `_Unwind_GetGR(ctx, 7)` does segfault and cannot be fixed as stated -- libgcc tracks only the columns CFI restores and RSP is derived, not tracked. #7349 stopped asking for it and derives the SP-relative base from `_Unwind_GetCFA`, with a per-arch return-address adjustment (x86-64 `call` pushes one, aarch64 `bl` does not). x86-64 Linux is a first-class arm. - Windows works via RtlVirtualUnwind (#7355), the one walker with no Itanium unwinder beneath it. - aarch64+ELF is now covered too (#7360) -- the only shape where LLVM spells 32-bit stack-map fields `.word`. - One mechanism, not two: PERRY_STATEPOINTS and the plain-map bridge are deleted, so the kill-policy line about "a mode that still exists" no longer applies to this pair. - The gate proves something now. Until today the Unix arms reported 7 frames and ZERO locations -- they would have passed with a walker that visited nothing. #7359's deep-collect probe took them to 221 locations. - watchOS/visionOS are not blocked by Perry: they build on stable without `dyn-eval`, and fail three crates away in psm's Mach-O guard. So the remaining adoption gate is `llvm-inprocess` becoming a default cargo feature, plus sequencing step 2 (root density) -- adopting today would regress binary size on root-dense code. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com> * gc: admit two provably-leaf helpers, and measure that it buys nothing js_gc_register_global_root was the most frequent non-leaf callee in the probe suite (148 call sites) and is provably GC-leaf: its whole body is runtime_write_barrier_root_heap_word -- which js_write_barrier_root_heap_word, already CannotCollect, wraps in one line -- plus a TLS Vec::push. The "malloc count threshold" trigger does not apply to that push: the counter is MALLOC_STATE.objects.len(), a registry of Perry GC objects, and the #[global_allocator] is plain mimalloc/System with no GC hook. js_typed_feedback_maybe_dump_trace joins its already-admitted family siblings. Measured A/B on the same tree, and the result is a null: probe safepoints roots total bytes __text 06_string_retention 105 -> 100 27=27 0 -4 B 09_try_catch_roots 343 -> 339 259=259 0 -4 B 11_collect_at_depth 120 -> 117 36=36 0 -4 B Root counts are IDENTICAL. The 40 safepoints removed across the suite were all rootless, and a rootless safepoint costs essentially nothing -- which is what docs/engine-plan.md already says: "the axis is not 'statepoints are bigger', it is 'roots are bigger'". Recording it as evidence: the safepoint-count lever is not the binary-size lever, so sequencing step 2 must attack live-root SETS. Two tests come with it. One pins the wrapper's classification to the barrier it wraps. The other pins js_nanbox_string OUT of the allowlist: at 120 call sites it is the obvious next candidate and reads as pure bit manipulation, but its null guard calls js_string_from_bytes to allocate an empty string. Probe suite 11/11 byte-identical under forced evacuation + verification. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com> * fix(ci): report assertion pinned to a probe Windows cannot compile Three review fixes on #7368. The report assertion ran on 09_try_catch_roots, which contains four `try` blocks. RS4GC cannot rewrite WinEH funclet pads, so linker.rs's rs4gc_funclet_refusal rejects that probe on windows-msvc -- the probe loop above tolerates it by grepping the compile log for "funclet", but this step did not. A gate pinned to a probe that cannot compile on one arm fails for a reason unrelated to its subject. The portable assertion now uses 11_collect_at_depth (no `try`, compiles on all four arms); 09_try_catch_roots keeps its own non-Windows step so the try-specific coverage that justified deleting the bridge is not lost. The gc_map doc claimed records/roots would be ABSENT when unmeasured. They are plain u64 fields on a plain derive and always serialise; `modules` is the sentinel. Fixed to describe what the code actually does -- the same class of comment-vs-code drift this PR exists to clean up. The "map never reported" guard fired for any --require-*/--print, including fields that live in `totals` and are counted at IR-emission time whether or not the rewrite ran. Now scoped to map-backed fields: --require-positive textual_calls is answered from its measured value (verified exit 0) while --require-positive records still fails on an unreported map (exit 1). Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…fork (#7371) Three corrections, one of which is a number the plan explicitly warns against quoting and was carrying anyway. 1. THE SIZE FIGURE. Only +18.95% appears on main -- a synthetic worst case with three heap values live across an allocation in EVERY one of 2000 functions. The dependency-scale measurement is +1.86% (zod, 81 native modules, 29 MB binary), an order of magnitude lower. The correction was written when the synthetic was retracted but never reached main: #7345 squash-merged as 24 insertions, the first commit only, so the follow-up correction commit was dropped. That is the same failure mode this document records for #7321 -- a wrong explanation outliving its own disproof -- so the real number now leads and the worst case is explicitly marked do-not-quote. 2. SEQUENCING STEP 2 said root density was a PREREQUISITE for adoption, reasoning from that retracted figure. Adoption shipped in #7370 without it. Still worth doing, and still the same lever #7296 proved worth 9.9x, but it gates nothing. 3. THE ADOPTION FORK IS CLOSED. Every gate shut: llvm-inprocess default (#7353), x86-64 (#7349), Windows (#7355), bridge deleted (#7348), and the 479-test suite with no env matching the shadow baseline exactly. The target-aware shape is recorded because it is the part that generalises: native roots where the runtime can walk, shadow stack where it cannot. Also: layer 2 now reads THE DEFAULT rather than landed opt-in, layer 3's count is 41 rather than 54 after #7363, and the 2026-08-03 status header no longer says 'not yet adopted'. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PERRY_RS4GC=1was refused on anything but aarch64 (#7324). That refusal was correct — the collector segfaulted rather than reporting anything — but the cause turned out to be one unsupported call, not anything architectural.The bug
On x86-64 every root is
Indirect [RSP + off]— DWARF register 7. The unwinder path resolved bases with_Unwind_GetGR(context, reg), and_Unwind_GetGRis not a supported query for the stack-pointer column. It returned garbage, and the collector wrote through it._Unwind_GetCFAis the supported way to reach a frame's stack pointer.SP-relative roots now derive their base from the CFA: by the SysV/AAPCS definition the CFA is the caller's stack pointer immediately before the call, so this frame's body stack pointer sits one return-address slot plus the function's own frame below it — and
stack_sizeis exactly that frame, already recorded per function in the map.What this deliberately does not change
The format's base tags stay aarch64-literal. My first attempt made them architecture-relative;
gc_map.rsalready documents why that's wrong:That reasoning holds, and x86-64 roots already round-trip correctly through the explicit-register tag. So the architecture's SP number is a runtime-local constant used only to pick a base-resolution strategy — no format change, no agreement hazard.
Measured on real x86-64 Linux hardware
All ten gc-ratchet probes byte-match the pinned Node oracle under
PERRY_RS4GC=1 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1, with.perry_gcmappresent in every binary.The walker demonstrably ran rather than passing vacuously:
fp_walks=0is correct — the fp-chain walker is aarch64-only, so the unwinder is the right path here. Evacuation genuinely moved objects (retained_forwarded_stub_objects=5).The same telemetry on aarch64 has the same shape —
7/0/1/1vs10/0/1/1on the same probes, and2locations on09_try_catch_rootson both. That equivalence is what makes this a result rather than a green light of unknown provenance.Caveat I want on the record
Those location counts are low on both platforms. The probes end in an explicit
gc()— a manual collection from a shallow stack — so precise native roots are lightly exercised by this suite regardless of architecture. That is a pre-existing gate weakness, not something this change introduces, and it means the honest claim is "x86-64 behaves identically to aarch64" rather than "x86-64 is heavily exercised".Strengthening that suite is worth doing separately: a probe that collects from a deep stack with many live roots, without a manual
gc(), would exercise the precise path on every platform.Update: Apple platforms (iOS, iPadOS, tvOS)
Same story as x86-64, different gate. iOS and iPadOS are aarch64 + Mach-O — the same shape as macOS, which already worked. They didn't, because the Mach-O loader, the unwinder module, the fp-chain walker and
stack_topwere each#[cfg(target_os = "macos")].On any other Apple platform that selected the no-section stub:
loaded_stack_map_section()returnedNone, the index came out empty, and the collector ran with no native roots at all — silently, on the platforms hardest to debug. The compiler emitted the map; nothing read it.All four gates are now one predicate: 64-bit Apple, or Linux.
pthread_get_stackaddr_npis Apple-wide, and themach2dependency was widened to match the code that uses it — declaring it for fewer targets than the loader compiles on is exactly how this stayed hidden.watchOS is refused, deliberately.
arm64_32has 32-bit pointers while the map stores function addresses asu64and the runtime doesusizearithmetic on them. The check is ordered before thearm64prefix test so it actually fires, and it refuses rather than emitting a map nothing can read.Verified by building
perry-runtimefor each target —aarch64-apple-ios,aarch64-apple-ios-sim,aarch64-apple-tvosall compile. That is what found the hole:stack_topdid not exist on iOS, so the build failed outright rather than quietly selecting the stub.aarch64-apple-visionosstill fails inside the third-partypsmbuild script, unrelated to this change.Honest limit: this is compile-level verification. A device or simulator run is the evidence it does not yet have, and I'd want that before anyone calls iOS supported rather than plumbed.
Summary by CodeRabbit
New Features
Bug Fixes