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 merged 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
janvorli
added
the
area-CodeGen-coreclr
|
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.
|
/backport to release/10.0 |
|
Started backporting to |
|
@janvorli backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix missing call to write barrier in static constructors of ref structs
Using index info to reconstruct a base tree...
M src/coreclr/jit/importer.cpp
A src/tests/JIT/Regression/Regression_ro_2.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/jit/importer.cpp
CONFLICT (modify/delete): src/tests/JIT/Regression/Regression_ro_2.csproj deleted in HEAD and modified in Fix missing call to write barrier in static constructors of ref structs. Version Fix missing call to write barrier in static constructors of ref structs of src/tests/JIT/Regression/Regression_ro_2.csproj left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix missing call to write barrier in static constructors of ref structs
Error: The process '/usr/bin/git' failed with exit code 128 |
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