Fix missing call to write barrier in static constructors of ref structs#125418
Fix missing call to write barrier in static constructors of ref structs#125418janvorli wants to merge 6 commits intodotnet:mainfrom
Conversation
The JIT was incorrectly handling initialization of static fields of ref structs. It was not adding call to JIT_ByRefWriteBarrier when setting these fields even though these statics live in GC heap. This change fixes it and adds a regression test that causes fatal error when run with DOTNET_HeapVerify=1. Close dotnet#125169
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
Fixes CoreCLR JIT handling of addresses for fields whose owning type is byref-like (ref struct), ensuring static fields aren’t incorrectly treated as “not on heap” (which can suppress required write barriers). Adds a new JIT regression test covering the scenario.
Changes:
- Update JIT importer logic to only mark byref-like instance field owners as
GTF_IND_TGT_NOT_HEAP(exclude static fields). - Add a new JIT regression test intended to reproduce missing write-barrier behavior for ref struct statics.
- Wire the new test into
Regression_ro_2.csproj.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/tests/JIT/Regression/Regression_ro_2.csproj | Adds the new regression test source file to the test project. |
| src/tests/JIT/Regression/JitBlue/Runtime_125169/Runtime_125169.cs | New regression test exercising ref struct static initialization + GC. |
| src/coreclr/jit/importer.cpp | Adjusts heap-ness classification so static fields on byref-like owners aren’t treated as not-heap. |
You can also share your feedback on Copilot code review. Take the survey.
src/tests/JIT/Regression/JitBlue/Runtime_125169/Runtime_125169.cs
Outdated
Show resolved
Hide resolved
src/tests/JIT/Regression/JitBlue/Runtime_125169/Runtime_125169.cs
Outdated
Show resolved
Hide resolved
|
@EgorBo PTAL |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
src/tests/JIT/Regression/JitBlue/Runtime_125169/Runtime_125169.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
You can also share your feedback on Copilot code review. Take the survey.
| ReadOnlySpan<byte> testSpan = new(RefWrapper._data._innerArray); | ||
| // Without the fix, the following GC.Collect call causes fatal error during heap verification with DOTNET_HeapVerify=1 | ||
| GC.Collect(0); | ||
|
|
||
| // Prevent the compiler from optimizing out the loop above. | ||
| GC.KeepAlive(arr); |
There was a problem hiding this comment.
testSpan is never used, so the JIT may dead-code-eliminate the ReadOnlySpan<byte> construction (and potentially the RefWrapper._data access), which can make the test non-deterministic or ineffective at triggering the ref-struct cctor path. Please make the access observably used (e.g., read a byte / assert a value, or GC.KeepAlive(testSpan) / GC.KeepAlive(RefWrapper._data._innerArray), or explicitly force the cctor via RuntimeHelpers.RunClassConstructor).
The JIT was incorrectly handling initialization of static fields of ref structs. It was not adding call to JIT_ByRefWriteBarrier when setting these fields even though these statics live in GC heap.
This issue was introduced in .NET 10 in #111733
This change fixes it and adds a regression test that causes fatal error when run with DOTNET_HeapVerify=1.
Close #125169