-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Changes from 4 commits
6bed9c3
22f1bc5
6cfa28b
ade7a3a
a959b75
95f1c84
ebb3102
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10887,6 +10887,12 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) | |
{ | ||
assert(lvaInlineeReturnSpillTemp != BAD_VAR_NUM); | ||
} | ||
|
||
if (!compIsForInlining() && ((prefixFlags & (PREFIX_TAILCALL_EXPLICIT | PREFIX_TAILCALL_STRESS)) == 0) && | ||
compStressCompile(STRESS_POISON_IMPLICIT_BYREFS, 25)) | ||
{ | ||
impPoisonImplicitByrefsBeforeReturn(); | ||
} | ||
#endif // DEBUG | ||
|
||
GenTree* op2 = nullptr; | ||
|
@@ -11204,6 +11210,87 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) | |
return true; | ||
} | ||
|
||
#ifdef DEBUG | ||
//------------------------------------------------------------------------ | ||
// impPoisonImplicitByrefsBeforeReturn: | ||
// Spill the stack and insert IR that poisons all implicit byrefs. | ||
// | ||
void Compiler::impPoisonImplicitByrefsBeforeReturn() | ||
{ | ||
bool spilled = false; | ||
for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) | ||
{ | ||
if (!lvaIsImplicitByRefLocal(lclNum)) | ||
{ | ||
continue; | ||
} | ||
|
||
compPoisoningAnyImplicitByrefs = true; | ||
|
||
if (!spilled) | ||
{ | ||
for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) | ||
{ | ||
impSpillStackEntry(level, BAD_VAR_NUM DEBUGARG(true) DEBUGARG("Stress poisoning byrefs before return")); | ||
} | ||
|
||
spilled = true; | ||
} | ||
|
||
LclVarDsc* dsc = lvaGetDesc(lclNum); | ||
// Be conservative about this local to ensure we do not eliminate the poisoning. | ||
lvaSetVarAddrExposed(lclNum, AddressExposedReason::STRESS_POISON_IMPLICIT_BYREFS); | ||
|
||
assert(varTypeIsStruct(dsc)); | ||
ClassLayout* layout = dsc->GetLayout(); | ||
assert(layout != nullptr); | ||
|
||
auto poisonBlock = [this, lclNum](unsigned start, unsigned count) { | ||
if (count <= 0) | ||
{ | ||
return; | ||
} | ||
|
||
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 commentThe reason will be displayed to describe this comment to others. Learn more.
Nit: this can use block-based There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, addressed I think. Can you take another look? |
||
GenTree* op = gtNewBlkOpNode(blk, gtNewIconNode(0xcd)); | ||
impAppendTree(op, CHECK_SPILL_NONE, DebugInfo()); | ||
}; | ||
|
||
unsigned startOffs = 0; | ||
unsigned numSlots = layout->GetSlotCount(); | ||
for (unsigned curSlot = 0; curSlot < numSlots; curSlot++) | ||
{ | ||
unsigned offs = curSlot * TARGET_POINTER_SIZE; | ||
var_types gcPtr = layout->GetGCPtrType(curSlot); | ||
if (!varTypeIsGC(gcPtr)) | ||
{ | ||
continue; | ||
} | ||
|
||
poisonBlock(startOffs, offs - startOffs); | ||
|
||
GenTree* zeroField = gtNewAssignNode(gtNewLclFldNode(lclNum, gcPtr, offs), gtNewZeroConNode(gcPtr)); | ||
impAppendTree(zeroField, CHECK_SPILL_NONE, DebugInfo()); | ||
|
||
startOffs = offs + TARGET_POINTER_SIZE; | ||
} | ||
|
||
assert(startOffs <= lvaLclExactSize(lclNum)); | ||
poisonBlock(startOffs, lvaLclExactSize(lclNum) - startOffs); | ||
} | ||
} | ||
#endif | ||
|
||
/***************************************************************************** | ||
* Mark the block as unimported. | ||
* Note that the caller is responsible for calling impImportBlockPending(), | ||
|
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!