JIT: fix x86 GC hole in stack-to-stack struct copies#128805
Open
EgorBo wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a CoreCLR JIT (x86 JIT32_GCENCODER) GC safety hole when lowering stack-to-stack struct copies containing byrefs/GC pointers, ensuring the copy path remains GC-aware instead of using an unrolled copy sequence that can create an unreported byref temporary.
Changes:
- In
LowerCopyBlockStore, prevents theisNotHeapshortcut from disabling CpObj onJIT32_GCENCODER, keeping x86 on the GC-aware decomposition/bulk-helper path. - Removes an assert in
LowerBlockStoreAsGcBulkCopyCallthat unnecessarily restricted using the bulk write-barrier helper for off-heap destinations. - Adds a new JIT regression test for the scenario (Span copy + Task.WhenAll stress) under
JitBlue/Runtime_128801.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/coreclr/jit/lower.cpp | Keeps x86 JIT32 on the GC-aware struct-copy path; relaxes an overly strict assert for bulk write-barrier lowering. |
| src/tests/JIT/Regression/JitBlue/Runtime_128801/Runtime_128801.csproj | Adds a new JitBlue test project with process isolation and env var configuration (TieredCompilation/GCStress). |
| src/tests/JIT/Regression/JitBlue/Runtime_128801/Runtime_128801.cs | Adds the regression test body (Span copy micro-test + Task.WhenAll stress). |
EgorBo
force-pushed
the
fix-x86-gc-hole-128801
branch
from
dd7f641 to
4873b96
Compare
In LowerCopyBlockStore, the 'isNotHeap' optimization unconditionally set doCpObj=false but only set gtBlkOpGcUnsafe=true on non-JIT32 targets. On x86 (JIT32_GCENCODER) this routed stack-to-stack struct copies to the plain unroll path, which emits the copy as INS_mov through a scratch register without reporting the register as a byref and without disabling GC. A GC between the load and store of the byref slot left the scratch register stale; the stale value was then stored to the local, which the GC info correctly reports as a byref, pointing into a free object. Guard the 'isNotHeap' shortcut behind !JIT32_GCENCODER so x86 stays on the GC-aware CpObj path (per-slot decomposition for small structs, bulk write-barrier helper for large ones). Drop the !IsAddressNotOnHeap assert in LowerBlockStoreAsGcBulkCopyCall: the helper already handles off-heap destinations correctly (its notInHeap check skips GCHeapMemoryBarrier), so it is a valid fallback for large stack-target copies. Fixes dotnet#128801
EgorBo
force-pushed
the
fix-x86-gc-hole-128801
branch
from
4873b96 to
2baf9bb
Compare
filipnavara
approved these changes
May 30, 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
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.
On x86 (
JIT32_GCENCODER),LowerCopyBlockStoreunconditionally setdoCpObj = falsefor stack-target struct copies, but only set the matchinggtBlkOpGcUnsafe = trueon non-JIT32 targets. The struct copy then took the plain unroll path (genCodeForCpBlkUnroll), which emits the copy asINS_movthrough a scratch register without reporting the register as a byref and without disabling GC. A GC between the load and store of the byref slot left the scratch register stale; the stale value was then stored to the local, which the GC info correctly reports as a byref, now pointing into a free object.Fix: guard the
isNotHeapshortcut behind!JIT32_GCENCODERso x86 stays on the GC-aware CpObj path (per-slot decomposition for small structs, bulk write-barrier helper for large ones). Also drop the!IsAddressNotOnHeapassert inLowerBlockStoreAsGcBulkCopyCallsince the helper already handles off-heap destinations correctly (itsnotInHeapcheck skipsGCHeapMemoryBarrier), so it is a valid fallback for large stack-target copies.Fixes #128801