Port #19780 to the 2.1 branch. #19803
Conversation
* Fix callKillSet for CORINFO_HELP_ASSIGN_BYREF. on x64. * Fix typos.
sandreenko
added
os-linux
|
@jkotas How do we usually test and prove such changes for porting into a release branch? Is it possible to ask our customers to apply the fix before the merge? |
|
https://github.com/dotnet/coreclr/issues/19361#issuecomment-414716191 has a standalone repro for the crash that you can run. This repro was distilled out of the real workload hitting the crash. If you can check that you see the crash without your fix on your machine and it goes away with your fix, I would consider it good enough proof that it is fixing the bug. |
|
@corruptmem @legigor This is fix for #19361 and #19796. Are you are interested in trying it before it is released? The instructions how to build a private version of CoreCLR and use it in your app are at https://github.com/dotnet/coreclr/blob/release/2.1/Documentation/workflow/UsingYourBuild.md (make sure to build release version - using "./build.sh release skiptests"). |
I have checked that the standalone repro crashes without the fix and works with it. |
|
PTAL @RussKeldorph @dotnet/jit-contrib |
|
@RussKeldorph could you please take a look? |
jamshedd
added
Servicing-approved
|
Approved for 2.1.5 |
|
@sandreenko @RussKeldorph this needs to get committed today or possibly tomorrow or it will miss 2.1.5 and need to go to shiproom again. Is it possible to get CI green and code reviews and merge today? |
|
@dotnet-bot test |
|
@danmosemsft I was told that "Big shiproom will merge when branch is open.". Do I need to merge it? @CarolEidt, @jashook could you please review this? This is the change from #19780 for the release branch. |
@sandreenko Yes this is true, but only if CI is green and the PR has got code review signoff. Probably that was why they did not merge. In this case, once you have got those, please do feel free to merge yourself so we can get it in today. |
|
Arm64 Failure looks like infrastructure problems in the branch. Seeing that windows arm64 in this branch is unsupported, CI is clean and this seems ready to go. |
|
Thanks! |
…otnet#19803) * Fix callKillSet for CORINFO_HELP_ASSIGN_BYREF. on x64.
Description
Fix callKillSet for CORINFO_HELP_ASSIGN_BYREF for Unix x64. The bug caused segfault if GC happened between the point where we reported a struct as dead and the correct last use.
Fixes #19796 and #19361 in the release branch.
Customer Impact
Currently they can get segmentation faults in programs with many threads and big memory consumption.
Regression?
Not a regression.
Risk
Low, it changes behaviour only on x64 Linux. The change for Linux x64 is to keep the same set of registers as Windows does for the helper that shares implementation between them.