gc: refuse native roots off aarch64, and stop the ELF map forcing DT_TEXTREL - #7324
Merged
Conversation
|
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 (3)
📝 WalkthroughWalkthroughThe compiler now rejects native GC map assembly on non-AArch64 targets and emits writable, retained ELF GC map sections. The workflow and changelog document ARM64 validation and the target restriction. ChangesNative GC ARM64 support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
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.
🧹 Nitpick comments (1)
crates/perry-codegen/src/gc_map.rs (1)
65-76: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd regression assertions for the ELF hardening properties.
The
"awR"declaration addsSHF_WRITEandSHF_GNU_RETAIN. The ARM gate currently checks only section presence and removal of.llvm_stackmaps. A regression to read-only flags could pass while reintroducingDT_TEXTREL. Add checks for a writable.perry_gcmapsection and the absence ofTEXTREL.As per coding guidelines, the CI gate must assert that the behavior it measures actually executed.
🤖 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-codegen/src/gc_map.rs` around lines 65 - 76, Extend the ARM ELF regression gate around the existing section-presence and .llvm_stackmaps checks to verify that .perry_gcmap has writable flags and that the linked binary contains no TEXTREL entry. Ensure the gate also asserts that these inspection checks actually ran, rather than passing when the relevant ELF metadata or tool output is missing.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@crates/perry-codegen/src/gc_map.rs`:
- Around line 65-76: Extend the ARM ELF regression gate around the existing
section-presence and .llvm_stackmaps checks to verify that .perry_gcmap has
writable flags and that the linked binary contains no TEXTREL entry. Ensure the
gate also asserts that these inspection checks actually ran, rather than passing
when the relevant ELF metadata or tool output is missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3345b1f6-4620-4fdd-890f-0099b4334714
📒 Files selected for processing (3)
.github/workflows/gc-native-roots.ymlchangelog.d/7321-statepoints-aarch64-only.mdcrates/perry-codegen/src/gc_map.rs
…TEXTREL The gc-native-roots gate went red on main with a SIGSEGV rather than a missing section: SHF_GNU_RETAIN did keep .perry_gcmap through --gc-sections and .llvm_stackmaps was gone, so the ELF retention fix worked. Two defects behind the crash. The backend is aarch64-only and did not say so. Cross-compiled a probe to x86_64-unknown-linux-gnu and decoded the emitted map: all 178 root slots are Indirect [RSP + off], DWARF register 7. chain_walkable admits only aarch64's 29/31, so every frame falls back to the unwinder, which resolves the base with _Unwind_GetGR(ctx, 7) — that does not reliably return the stack pointer (_Unwind_GetCFA is the supported way). Wild addresses, then a segfault when the collector writes through them. The mode is opt-in, so refuse rather than ship a binary that crashes under collection. The ELF section was read-only while holding relocated function addresses: ld warned 'relocation against main in read-only section .perry_gcmap' and created a DT_TEXTREL in a PIE. Now "awR". Gate moves to an ARM64 runner — on x86-64 it would now test only the refusal. macOS arms remain 9/9; x86-64 Linux now fails the compile with a message naming the target instead of segfaulting at collection time.
proggeramlug
force-pushed
the
gc/statepoints-aarch64-only
branch
from
August 3, 2026 18:52
571cad9 to
806e379
Compare
This was referenced Aug 3, 2026
proggeramlug
added a commit
that referenced
this pull request
Aug 3, 2026
…at it found (#7334) The diagnostic job was merged with #7331 before it had served its purpose; it has now, so it comes out. Its answer contradicts the narrative still at the top of this file: the compact-map rewriter parses x86-64 stack maps fine. Every root is Indirect [RSP + off] (DWARF 7), round-tripping through the explicit- register tag, and no clang version or -march setting reproduced a parse failure. The x86-64 defect is at collection time, which #7324 refuses for. Replace the wrong explanation rather than leave two contradictory ones in the same header. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This was referenced Aug 3, 2026
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
) * gc: native roots on x86-64 — derive the SP base from the CFA 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. * ci(gc): make x86-64 a real arm instead of a refusal assertion 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. * gc: native roots on iOS, iPadOS and tvOS — widen the Apple gates 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. * gc: per-arch CFA adjustment, and a pointer-sized address field for watchOS 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. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.
The
gc-native-rootsgate went red onmainafter #7314. Good news first: the ELF question it existed to answer is resolved —SHF_GNU_RETAINdid keep.perry_gcmapthrough--gc-sections, and.llvm_stackmapswas gone. Both section assertions passed.It failed with exit 139 — SIGSEGV on
02_survivor_promotion, the evacuation-heaviest probe. Two separate defects.The backend is aarch64-only and did not say so
The runner is x86-64. I cross-compiled a probe to
x86_64-unknown-linux-gnuand decoded the emitted stack map rather than guessing:Every root is
Indirect [RSP + off]— DWARF register 7. The runtime'schain_walkableadmits only aarch64's FP/SP (29 and 31), so on x86-64 it is always false and every frame falls back to the unwinder, which resolves the base with_Unwind_GetGR(ctx, 7). That call does not reliably return the stack pointer —_Unwind_GetCFAis the supported way to obtain it. So the walker computed wild addresses and the collector segfaulted writing through them.The mode is opt-in, so the compiler now refuses the combination with a message naming the target, rather than emitting a binary that crashes under collection. Verified:
--target linuxwithPERRY_STATEPOINTS=1now fails the compile cleanly.Making x86-64 actually work means per-architecture base resolution (
_Unwind_GetCFAfor SP, per-arch DWARF constants) — real work, tracked under #7173, and not something to fake in a refusal.The ELF section was read-only but holds relocated addresses
The section carries
.quad <function symbol>entries the loader must relocate. WithoutSHF_WRITEthat forces a text relocation in a PIE — a hardening regression and a portability hazard independent of the segfault. Now"awR"(ALLOC | WRITE | RETAIN).The gate moves to ARM64
ubuntu-24.04-arm. On x86-64 the gate would now exercise nothing but the refusal; on ARM64 it tests the configuration that is actually supported and still answers what it was built for — whether the compact map survives ELF linking.Verification
macOS arms unchanged: explicit bridge, RS4GC and the default shadow stack all 9/9 against the pinned Node oracle, statepoint arms under forced evacuation with the verifying walker.
Summary by CodeRabbit
Bug Fixes
Documentation