-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JIT: Add a stress mode that poisons implicit byrefs #80691
JIT: Add a stress mode that poisons implicit byrefs #80691
Conversation
This stress mode poisons all implicit byrefs before returns from the method. GC pointers are nulled out and other parts of the structs are filled with 0xcd bytes. This should help expose incorrectly elided copies in the recently added last-use copy elision optimization.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsThis stress mode poisons all implicit byrefs before returns from the method. GC pointers are nulled out and other parts of the structs are filled with 0xcd bytes. This should help expose incorrectly elided copies in the recently added last-use copy elision optimization.
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
src/coreclr/jit/importer.cpp
Outdated
GenTree* addr; | ||
if (start > 0) | ||
{ | ||
addr = gtNewLclFldAddrNode(lclNum, start, TYP_BYREF); | ||
} | ||
else | ||
{ | ||
addr = gtNewLclVarAddrNode(lclNum, TYP_BYREF); | ||
} | ||
|
||
GenTree* blk = new (this, GT_BLK) GenTreeBlk(GT_BLK, TYP_STRUCT, addr, typGetBlkLayout(count)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GLOB_REF
s are missing for this assignment and the one below.
Nit: this can use block-based TYP_STRUCT
LCL_FLD
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, addressed I think. Can you take another look?
src/coreclr/jit/importer.cpp
Outdated
//------------------------------------------------------------------------ | ||
// impPoisonImplicitByrefsBeforeReturn: | ||
// Spill the stack and insert IR that poisons all implicit byrefs. | ||
// | ||
void Compiler::impPoisonImplicitByrefsBeforeReturn() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: a little comment for why do we want to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
The failures are known or will be fixed by #80734. cc @dotnet/jit-contrib PTAL @AndyAyersMS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this, it looks like a good safeguard.
This stress mode poisons all implicit byrefs before returns from the method. GC pointers are nulled out and other parts of the structs are filled with 0xcd bytes.
This should help expose incorrectly elided copies in the recently added last-use copy elision optimization.