Repository navigation
Fix double GC relocation of overlapping byrefs - #135890
Merged
jkoritzinsky merged 3 commits intoOct 17, 2026
Merged
jkoritzinsky merged 3 commits into
jkoritzinsky merged 3 commits into
Conversation
Deduplicate physical byref slots in explicit-layout value types without allocating during GC. Preserve non-loading field lookups and document the rare-layout quadratic scan. Add a targeted by-value regression in the existing ref-field tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @agocke |
Nest ValueClassByRefSearchContext inside ByRefPointerOffsetsReporter without changing its scanning behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Oct 16, 2026
Closed
jkotas
reviewed
Oct 16, 2026
Reuse the value-type reporter in overlap-probe mode without building nested overlap lists or recursively checking overlaps. Add nested-layout regression coverage with distinct and shared backing arrays. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
jkotas
approved these changes
Oct 16, 2026
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Safety-critical GC scanning changes need runtime-owner review of scanner invariants and platform coverage beyond Windows x86.
No open issues 🎉
What changed in this PR
Fixes double GC relocation by reporting each physical byref slot once in overlapping ref-struct layouts, independently of debugger functionality.
Changes:
- Deduplicates overlapping slots using allocation-free traversal.
- Uses already-loaded field type handles during GC scanning.
- Adds regression coverage for nested overlaps and shared backing arrays.
Validation reported in the PR covers Windows x86; no builds or tests were run during this review.
| File | Description |
|---|---|
| src/tests/Loader/classloader/RefFields/Validate.cs | Tests overlapping and nested byrefs through by-value arguments. |
| src/coreclr/vm/siginfo.cpp | Prevents duplicate slot reporting and avoids type loading during scanning. |
Review effort: Balanced
Member
Author
|
/ba-g failures unrelated |
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.
Summary
Report each physical byref slot only once when scanning an explicit-layout byref-like value type. Overlapping fields currently cause the shared scanner to report the same slot multiple times, so a compacting GC can relocate an already-relocated interior pointer again.
This is the standalone GC fix requested in review of #133659. It is not debugger-specific: an ordinary by-value ref-struct argument reproduces the corruption on Windows x86 with
DOTNET_GCStress=1.No debugger-specific or cDAC changes are included.
Validation
Windows x86 Checked runtime, Release libraries, at main base
6b1cd4b85c4b:1 2 3 41 2 3 4DOTNET_GCStress=10 3801155 7536732 29491731 2 3 4DOTNET_GCStress=11, actual6881312(exit 101)DOTNET_GCStress=1The regression assembly and CoreLib are byte-identical between the unfixed and fixed runs.
Build commands and results:
Tests were run with
CORE_ROOTset toartifacts\tests\coreclr\windows.x86.Checked\Tests\Core_Root, using& "$env:CORE_ROOT\corerun.exe"withValidate.dllandInlineArrayValid.dllfrom their respective output directories, once withoutDOTNET_GCStressand once with it set to1. Other architectures/platforms were not run.Resolves #135245
Note
This implementation and PR description were prepared with GitHub Copilot.