Skip to content

Commit

Permalink
Poison address exposed user variables in debug code
Browse files Browse the repository at this point in the history
  • Loading branch information
jakobbotsch committed Jun 25, 2021
1 parent fc4a427 commit 4f161de
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 17 deletions.
3 changes: 3 additions & 0 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,9 @@ class CodeGen final : public CodeGenInterface

void genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pInitRegZeroed, regMaskTP maskArgRegsLiveIn);

void genSetRegsModifiedForPoisoning();
void genPoisonFrame(regMaskTP bbRegLiveIn);

#if defined(TARGET_ARM)

bool genInstrWithConstant(
Expand Down
88 changes: 87 additions & 1 deletion src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4842,7 +4842,7 @@ void CodeGen::genCheckUseBlockInit()
}
}

// Record number of 4 byte slots that need zeroing.
// Record number of stack slots that need zeroing.
genInitStkLclCnt = initStkLclCnt;

// Decide if we will do block initialization in the prolog, or use
Expand Down Expand Up @@ -6733,6 +6733,12 @@ void CodeGen::genFinalizeFrame()
}
#endif // TARGET_ARM

// When poisoning on ARM we need to use callee-preserved registers.
if (compiler->compShouldPoisonFrame())
{
genSetRegsModifiedForPoisoning();
}

#ifdef DEBUG
if (verbose)
{
Expand Down Expand Up @@ -12528,3 +12534,83 @@ void CodeGenInterface::VariableLiveKeeper::dumpLvaVariableLiveRanges() const
}
#endif // DEBUG
#endif // USING_VARIABLE_LIVE_RANGE

void CodeGen::genSetRegsModifiedForPoisoning()
{
// For ARM32 we need a callee-reserved register since the scratch registers may have args.
#if defined(TARGET_ARM)
regSet.rsSetRegsModified(RBM_R4);
#endif
}

//-----------------------------------------------------------------------------
// genPoisonFrame: Generate code that places a recognizable value into address exposed variables.
//
// Remarks:
// This function emits code to poison address exposed non-zero-inited local variables. We expect this function
// to be called when emitting code for the scratch BB that comes right after the prolog.
// The variables are poisoned using 0xcccccccc. This value is stored in EAX/RAX on xarch and r/x0 on ARM,
// so we expect that this reg is not live.
void CodeGen::genPoisonFrame(regMaskTP regLiveIn)
{
#if defined(TARGET_XARCH)
const regNumber immRegNum = REG_EAX;
#elif defined(TARGET_ARM)
const regNumber immRegNum = REG_R4;
#elif defined(TARGET_ARM64)
const regNumber immRegNum = REG_R9;
#else
return;
#endif

assert(compiler->compShouldPoisonFrame());
assert((regLiveIn & genRegMask(immRegNum)) == 0);

// The first time we need to poison something we will initialize a register to the largest immediate cccccccc that we can fit.
bool hasPoisonImm = false;
for (unsigned varNum = 0; varNum < compiler->info.compLocalsCount; varNum++)
{
LclVarDsc* varDsc = compiler->lvaGetDesc(varNum);
if (varDsc->lvIsParam || varDsc->lvMustInit || !varDsc->lvAddrExposed)
{
continue;
}

assert(varDsc->lvOnFrame);

if (!hasPoisonImm)
{
#ifdef TARGET_64BIT
instGen_Set_Reg_To_Imm(EA_8BYTE, immRegNum, (ssize_t)0xcccccccccccccccc);
#else
instGen_Set_Reg_To_Imm(EA_4BYTE, immRegNum, (ssize_t)0xcccccccc);
#endif
hasPoisonImm = true;
}

// For 64-bit we check if the local is 8-byte aligned. For 32-bit, we assume everything is always 4-byte aligned.
#ifdef TARGET_64BIT
bool fpBased;
int addr = compiler->lvaFrameAddress((int)varNum, &fpBased);
#else
int addr = 0;
#endif
int size = (int)compiler->lvaLclSize(varNum);
int end = addr + size;
for (int offs = addr; offs < end;)
{
#ifdef TARGET_64BIT
if ((offs & 7) == 0 && end - offs >= 8)
{
GetEmitter()->emitIns_S_R(ins_Store(TYP_LONG), EA_8BYTE, immRegNum, (int)varNum, offs - addr);
offs += 8;
continue;
}
#endif

assert((offs & 3) == 0 && end - offs >= 4);
GetEmitter()->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, immRegNum, (int)varNum, offs - addr);
offs += 4;
}
}
}
7 changes: 7 additions & 0 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,13 @@ void CodeGen::genCodeForBBlist()
compiler->compCurStmt = nullptr;
compiler->compCurLifeTree = nullptr;

// Emit poisoning into scratch BB that comes right after prolog.
// We cannot emit this code in the prolog as it might make the prolog too large.
if (compiler->compShouldPoisonFrame() && compiler->fgBBisScratch(block))
{
genPoisonFrame(newLiveRegSet);
}

// Traverse the block in linear order, generating code for each node as we
// as we encounter it.
CLANG_FORMAT_COMMENT_ANCHOR;
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -9828,6 +9828,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
return (info.compUnmanagedCallCountWithGCTransition > 0);
}

// Returns true if address-exposed user variables should be poisoned with a recognizable value
bool compShouldPoisonFrame()
{
return !info.compInitMem && opts.MinOpts() && opts.compDbgCode;
}

#if defined(DEBUG)

void compDispLocalVars();
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2604,7 +2604,8 @@ void Compiler::fgAddInternal()

// The backend requires a scratch BB into which it can safely insert a P/Invoke method prolog if one is
// required. Create it here.
if (compMethodRequiresPInvokeFrame())
// Similarly, for poisoning we also need a scratch BB.
if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame())
{
fgEnsureFirstBBisScratch();
fgFirstBB->bbFlags |= BBF_DONT_REMOVE;
Expand Down
15 changes: 0 additions & 15 deletions src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,6 @@ private ulong Low64
1e80
};

// Used to fill uninitialized stack variables with non-zero pattern in debug builds
[Conditional("DEBUG")]
private static unsafe void DebugPoison<T>(ref T s) where T : unmanaged
{
MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref s, 1)).Fill(0xCD);
}

#region Decimal Math Helpers

private static unsafe uint GetExponent(float f)
Expand Down Expand Up @@ -990,7 +983,6 @@ internal static unsafe void DecAddSub(ref DecCalc d1, ref DecCalc d2, bool sign)
// Have to scale by a bunch. Move the number to a buffer where it has room to grow as it's scaled.
//
Unsafe.SkipInit(out Buf24 bufNum);
DebugPoison(ref bufNum);

bufNum.Low64 = low64;
bufNum.Mid64 = tmp64;
Expand Down Expand Up @@ -1339,7 +1331,6 @@ internal static unsafe void VarDecMul(ref DecCalc d1, ref DecCalc d2)
ulong tmp;
uint hiProd;
Unsafe.SkipInit(out Buf24 bufProd);
DebugPoison(ref bufProd);

if ((d1.High | d1.Mid) == 0)
{
Expand Down Expand Up @@ -1920,7 +1911,6 @@ internal static int GetHashCode(in decimal d)
internal static unsafe void VarDecDiv(ref DecCalc d1, ref DecCalc d2)
{
Unsafe.SkipInit(out Buf12 bufQuo);
DebugPoison(ref bufQuo);

uint power;
int curScale;
Expand Down Expand Up @@ -2021,7 +2011,6 @@ internal static unsafe void VarDecDiv(ref DecCalc d1, ref DecCalc d2)
// Shift both dividend and divisor left by curScale.
//
Unsafe.SkipInit(out Buf16 bufRem);
DebugPoison(ref bufRem);

bufRem.Low64 = d1.Low64 << curScale;
bufRem.High64 = (d1.Mid + ((ulong)d1.High << 32)) >> (32 - curScale);
Expand Down Expand Up @@ -2090,7 +2079,6 @@ internal static unsafe void VarDecDiv(ref DecCalc d1, ref DecCalc d2)
// Start by finishing the shift left by curScale.
//
Unsafe.SkipInit(out Buf12 bufDivisor);
DebugPoison(ref bufDivisor);

bufDivisor.Low64 = divisor;
bufDivisor.U2 = (uint)((d2.Mid + ((ulong)d2.High << 32)) >> (32 - curScale));
Expand Down Expand Up @@ -2243,7 +2231,6 @@ internal static void VarDecMod(ref DecCalc d1, ref DecCalc d2)
d1.uflags = d2.uflags;
// Try to scale up dividend to match divisor.
Unsafe.SkipInit(out Buf12 bufQuo);
DebugPoison(ref bufQuo);

bufQuo.Low64 = d1.Low64;
bufQuo.U2 = d1.High;
Expand Down Expand Up @@ -2303,7 +2290,6 @@ private static unsafe void VarDecModFull(ref DecCalc d1, ref DecCalc d2, int sca
int shift = BitOperations.LeadingZeroCount(tmp);

Unsafe.SkipInit(out Buf28 b);
DebugPoison(ref b);

b.Buf24.Low64 = d1.Low64 << shift;
b.Buf24.Mid64 = (d1.Mid + ((ulong)d1.High << 32)) >> (32 - shift);
Expand Down Expand Up @@ -2358,7 +2344,6 @@ private static unsafe void VarDecModFull(ref DecCalc d1, ref DecCalc d2, int sca
else
{
Unsafe.SkipInit(out Buf12 bufDivisor);
DebugPoison(ref bufDivisor);

bufDivisor.Low64 = d2.Low64 << shift;
bufDivisor.U2 = (uint)((d2.Mid + ((ulong)d2.High << 32)) >> (32 - shift));
Expand Down

0 comments on commit 4f161de

Please sign in to comment.