Skip to content

Fix GC hole caused by adrp and add instructions on arm64 - #136412

Merged
VSadov merged 2 commits into
mainfrom
copilot/fix-gc-hole-on-arm64
Oct 27, 2026
Merged

VSadov merged 2 commits into
mainfrom
copilot/fix-gc-hole-on-arm64

Conversation

Copilot AI commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.


This section details on the original issue you should resolve

<issue_title>[arm64] GC hole: JIT must kill the destination register right after adrp</issue_title>
<issue_description>Re: #135794 (comment)

Found after enabling GC stress in R2R code.

On arm64, adrp+add (IF_LARGEADR) and adrp+ldr (IF_LARGELDC) are emitted as one emitter instruction (emitOutputLoadLabel/emitOutputLJ). GC liveness of the register written by adrp therefore can't change between the two machine instructions. If it held a live GC ref before the pair, it is still reported as one while it holds a non-GC page address.

In fully interruptible code a thread can be suspended at the second instruction, so this is a real GC hole, not just a GC stress artifact. R2R uses the pair (relocatable address) for the async ResumeInfo address (genAsyncResumeInfo).

Found via GC stress on R2R (#135794, #108249). The arm64 async.0.1 run (MutateImplicitByRef) took a SIGILL at add x0,x0,#0xdd8. x0 was reported as a GC ref holding an image page address, and Object::Validate crashed. This is from a core dump; the GC info was not decoded.

Fix: emit the GC kill for the register right after adrp, like the existing TLS-GD adrp case (emitGCregDeadUpd(REG_R0, dst)).</issue_description>

Comments on the Issue (you are @copilot in this section)

@azure-pipelines

azure-pipelines Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 6 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @anicka-net, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

@VSadov

VSadov commented Oct 24, 2026

Copy link
Copy Markdown
Member

@copilot are there file changes?

Copilot AI commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

@copilot are there file changes?

No. The only commit (625fdf4) is an empty “Initial plan” commit, and the working tree is clean. No ARM64 GC-liveness fix was submitted.

@jkotas jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-GC-coreclr labels Oct 24, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@VSadov

VSadov commented Oct 25, 2026

Copy link
Copy Markdown
Member

@copilot why no files were changed? (trying different model)

@VSadov

VSadov commented Oct 25, 2026

Copy link
Copy Markdown
Member

@copilot why no files were changed? (trying different model)

@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@dotnet dotnet deleted a comment from Copilot AI Oct 25, 2026
@VSadov

VSadov commented Oct 25, 2026

Copy link
Copy Markdown
Member

@copilot implement the fix

Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>

Copilot AI commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot implement the fix

Implemented in 1a898aa7. Both long address and constant-load forms kill the ADRP destination at ADRP+4, including integer scratch registers for vector loads.

Validation: checked ARM64 JIT build passed; the ARM64 R2R repro’s x0 death moves from 0x190 to 0x180; 49,086 CoreLib methods compile with unchanged instruction listings and corrected GC transitions in both paths. The existing repro also passes on x64.

Native ARM64 GC-stress execution was unavailable. Secret scan passed; CodeQL was attempted but skipped because its database is too large.

@VSadov
VSadov marked this pull request as ready for review October 25, 2026 18:11
@VSadov VSadov changed the title [WIP] Fix GC hole caused by adrp and add instructions on arm64 Fix GC hole caused by adrp and add instructions on arm64 Oct 25, 2026
@JulieLeeMSFT

Copy link
Copy Markdown
Member

/azp run runtime-coreclr jitstress, runtime-coreclr jitstressregs, runtime-coreclr gcstress0x3-gcstress0xc

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

@VSadov

VSadov commented Oct 27, 2026

Copy link
Copy Markdown
Member

Assert failure(PID 5640 [0x00001608], Thread: 9548 [0x254c]): *(_UNCHECKED_OBJECTREF *)handle == NULL is #117138

@JulieLeeMSFT JulieLeeMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It fixes a GC hole on ARM64 in fully interruptible code.
If the register destination of an adrp/add address-materialization pair previously held a live object reference, a GC occurring between the two instructions would incorrectly treat the partial page address as a live GC reference, leading to heap corruption or crashes.

It added two calls to the existing emitGCregDeadUpd helper at the precise PC where the register's semantic value changes — mirroring an already-established pattern used for TLS-GD relaxation, so it closes the correctness gap in both the large-address-load and large-constant-load emitter paths.

@VSadov

VSadov commented Oct 27, 2026

Copy link
Copy Markdown
Member

Thanks!

@VSadov
VSadov merged commit 425a54e into main Oct 27, 2026
183 of 187 checks passed
@VSadov
VSadov deleted the copilot/fix-gc-hole-on-arm64 branch October 27, 2026 19:04
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 28, 2026
@VSadov

VSadov commented Oct 29, 2026

Copy link
Copy Markdown
Member

@JulieLeeMSFT - I wonder if we want to backport the fix to net11?


The underlying GC hole appears to be pre-existing, not an async regression. Async R2R introduced or exposed a reliable path to the vulnerable emitter sequence, and R2R GC stress revealed it.

So I’d classify it as a latent JIT correctness bug newly exposed by async/R2R.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[arm64] GC hole: JIT must kill the destination register right after adrp

4 participants