diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 02971fbde2b8c..73bb2a0b247a6 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -348,6 +348,8 @@ class CodeGen final : public CodeGenInterface void genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pInitRegZeroed, regMaskTP maskArgRegsLiveIn); + void genPoisonFrame(regMaskTP bbRegLiveIn); + #if defined(TARGET_ARM) bool genInstrWithConstant( diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index e99f047c061a5..cfbdf1e9033b8 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -12528,3 +12528,65 @@ void CodeGenInterface::VariableLiveKeeper::dumpLvaVariableLiveRanges() const } #endif // DEBUG #endif // USING_VARIABLE_LIVE_RANGE + +//----------------------------------------------------------------------------- +// 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 0xcdcdcdcd. +void CodeGen::genPoisonFrame(regMaskTP regLiveIn) +{ + assert(compiler->compShouldPoisonFrame()); + assert((regLiveIn & genRegMask(REG_SCRATCH)) == 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 + genSetRegToIcon(REG_SCRATCH, (ssize_t)0xcdcdcdcdcdcdcdcd, TYP_LONG); +#else + genSetRegToIcon(REG_SCRATCH, (ssize_t)0xcdcdcdcd, TYP_INT); +#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 % 8) == 0 && end - offs >= 8) + { + GetEmitter()->emitIns_S_R(ins_Store(TYP_LONG), EA_8BYTE, REG_SCRATCH, (int)varNum, offs - addr); + offs += 8; + continue; + } +#endif + + assert((offs % 4) == 0 && end - offs >= 4); + GetEmitter()->emitIns_S_R(ins_Store(TYP_INT), EA_4BYTE, REG_SCRATCH, (int)varNum, offs - addr); + offs += 4; + } + } +} diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 796c128b98eac..8bf1f852ffd4d 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -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; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index fe3f987667d38..47cff55ced8c5 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9828,6 +9828,16 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX return (info.compUnmanagedCallCountWithGCTransition > 0); } + // Returns true if address-exposed user variables should be poisoned with a recognizable value + bool compShouldPoisonFrame() + { +#ifdef FEATURE_ON_STACK_REPLACEMENT + if (opts.IsOSR()) + return false; +#endif + return !info.compInitMem && opts.compDbgCode; + } + #if defined(DEBUG) void compDispLocalVars(); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index e7a281cdecaa1..04faed33f50a8 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2603,8 +2603,8 @@ void Compiler::fgAddInternal() noway_assert(!compIsForInlining()); // 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()) + // required. Similarly, we need a scratch BB for poisoning. Create it here. + if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame()) { fgEnsureFirstBBisScratch(); fgFirstBB->bbFlags |= BBF_DONT_REMOVE; diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index dcea881311919..6bb36a7ef1e40 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -2333,6 +2333,15 @@ void LinearScan::buildIntervals() // assert(block->isRunRarely()); } + // For frame poisoning we generate code into scratch BB right after prolog since + // otherwise the prolog might become too large. In this case we will put the poison immediate + // into the scratch register, so it will be killed here. + if (compiler->compShouldPoisonFrame() && compiler->fgFirstBBisScratch() && block == compiler->fgFirstBB) + { + addRefsForPhysRegMask(genRegMask(REG_SCRATCH), currentLoc + 1, RefTypeKill, true); + currentLoc += 2; + } + LIR::Range& blockRange = LIR::AsRange(block); for (GenTree* node : blockRange) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs b/src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs index d56ce8ebb7fc7..a966b35b83c4a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs @@ -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(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) @@ -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; @@ -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) { @@ -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; @@ -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); @@ -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)); @@ -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; @@ -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); @@ -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)); diff --git a/src/tests/JIT/Directed/debugging/poison.cs b/src/tests/JIT/Directed/debugging/poison.cs new file mode 100644 index 0000000000000..463894a4e6a35 --- /dev/null +++ b/src/tests/JIT/Directed/debugging/poison.cs @@ -0,0 +1,56 @@ +using System; +using System.Runtime.CompilerServices; + +public class Program +{ + [SkipLocalsInit] + public static unsafe int Main() + { + bool result = true; + + int poisoned; + Unsafe.SkipInit(out poisoned); + result &= VerifyPoison(&poisoned, sizeof(int)); + + GCRef zeroed; + Unsafe.SkipInit(out zeroed); + result &= VerifyZero(Unsafe.AsPointer(ref zeroed), Unsafe.SizeOf()); + + WithoutGCRef poisoned2; + Unsafe.SkipInit(out poisoned2); + result &= VerifyPoison(&poisoned2, sizeof(WithoutGCRef)); + + return result ? 100 : 101; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static unsafe bool VerifyPoison(void* val, int size) + => AllEq(new Span(val, size), 0xCD); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static unsafe bool VerifyZero(void* val, int size) + => AllEq(new Span(val, size), 0); + + private static unsafe bool AllEq(Span span, byte byteVal) + { + foreach (byte b in span) + { + if (b != byteVal) + return false; + } + + return true; + } + + private struct GCRef + { + public object ARef; + } + + private struct WithoutGCRef + { + public double ADouble; + public int ANumber; + public float AFloat; + } +} diff --git a/src/tests/JIT/Directed/debugging/poison.csproj b/src/tests/JIT/Directed/debugging/poison.csproj new file mode 100644 index 0000000000000..b2691ab4cab22 --- /dev/null +++ b/src/tests/JIT/Directed/debugging/poison.csproj @@ -0,0 +1,11 @@ + + + Exe + PdbOnly + False + True + + + + + diff --git a/src/tests/issues.targets b/src/tests/issues.targets index bac25dd48638a..6e16937b7f74a 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1668,6 +1668,9 @@ needs triage + + Tests coreclr JIT's debug poisoning of address taken variables +