Merge pull request dotnet#16533 from briansull/default-fill

Implement JitDefaultFill

src/jit/alloc.h

@@ -131,7 +131,7 @@ inline void* ArenaAllocator::allocateMemory(size_t size)
     }
 
 #if defined(DEBUG)
-    memset(block, UninitializedWord<char>(), size);
+    memset(block, UninitializedWord<char>(nullptr), size);
 #endif
 
     return block;

src/jit/codegencommon.cpp

@@ -152,7 +152,7 @@ CodeGen::CodeGen(Compiler* theCompiler) : CodeGenInterface(theCompiler)
     genTrnslLocalVarCount = 0;
 
     // Shouldn't be used before it is set in genFnProlog()
-    compiler->compCalleeRegsPushed = UninitializedWord<unsigned>();
+    compiler->compCalleeRegsPushed = UninitializedWord<unsigned>(compiler);
 
 #if defined(_TARGET_XARCH_) && !FEATURE_STACK_FP_X87
     // Shouldn't be used before it is set in genFnProlog()

src/jit/compiler.cpp

@@ -2194,6 +2194,44 @@ void Compiler::compDoComponentUnitTestsOnce()
         BitSetSupport::TestSuite(getAllocatorDebugOnly());
     }
 }
+
+//------------------------------------------------------------------------
+// compGetJitDefaultFill:
+//
+// Return Value:
+//    An unsigned char value used to initizalize memory allocated by the JIT.
+//    The default value is taken from COMPLUS_JitDefaultFill,  if is not set
+//    the value will be 0xdd.  When JitStress is active a random value based
+//    on the method hash is used.
+//
+// Notes:
+//    Note that we can't use small values like zero, because we have some
+//    asserts that can fire for such values.
+//
+unsigned char Compiler::compGetJitDefaultFill()
+{
+    unsigned char defaultFill = (unsigned char)JitConfig.JitDefaultFill();
+
+    if ((this != nullptr) && (compStressCompile(STRESS_GENERIC_VARN, 50)))
+    {
+        unsigned temp;
+        temp = info.compMethodHash();
+        temp = (temp >> 16) ^ temp;
+        temp = (temp >> 8) ^ temp;
+        temp = temp & 0xff;
+        // asserts like this: assert(!IsUninitialized(stkLvl));
+        // mean that small values for defaultFill are problematic
+        // so we make the value larger in that case.
+        if (temp < 0x20)
+        {
+            temp |= 0x80;
+        }
+        defaultFill = (unsigned char)temp;
+    }
+
+    return defaultFill;
+}
+
 #endif // DEBUG
 
 /*****************************************************************************

src/jit/compiler.h

@@ -8960,6 +8960,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     bool compDonotInline();
 
 #ifdef DEBUG
+    unsigned char compGetJitDefaultFill(); // Get the default fill char value
+                                           // we randomize this value when JitStress is enabled
+
     const char* compLocalVarName(unsigned varNum, unsigned offs);
     VarName compVarName(regNumber reg, bool isFloatReg = false);
     const char* compRegVarName(regNumber reg, bool displayVar = false, bool isFloatReg = false);

src/jit/compiler.hpp

@@ -1715,12 +1715,9 @@ inline unsigned Compiler::lvaGrabTemp(bool shortLifetime DEBUGARG(const char* re
             new (&newLvaTable[i], jitstd::placement_t()) LclVarDsc(this); // call the constructor.
         }
 
-#if 0
-        // TODO-Cleanup: Enable this and test.
 #ifdef DEBUG
         // Fill the old table with junks. So to detect the un-intended use.
-        memset(lvaTable, fDefaultFill2.val_DontUse_(CLRConfig::INTERNAL_JitDefaultFill, 0xFF), lvaCount * sizeof(*lvaTable));
-#endif
+        memset(lvaTable, JitConfig.JitDefaultFill(), lvaCount * sizeof(*lvaTable));
 #endif
 
         lvaTableCnt = newLvaTableCnt;
@@ -1792,12 +1789,9 @@ inline unsigned Compiler::lvaGrabTemps(unsigned cnt DEBUGARG(const char* reason)
             new (&newLvaTable[i], jitstd::placement_t()) LclVarDsc(this); // call the constructor.
         }
 
-#if 0
 #ifdef DEBUG
-        // TODO-Cleanup: Enable this and test.
         // Fill the old table with junks. So to detect the un-intended use.
-        memset(lvaTable, fDefaultFill2.val_DontUse_(CLRConfig::INTERNAL_JitDefaultFill, 0xFF), lvaCount * sizeof(*lvaTable));
-#endif
+        memset(lvaTable, JitConfig.JitDefaultFill(), lvaCount * sizeof(*lvaTable));
 #endif
 
         lvaTableCnt = newLvaTableCnt;

src/jit/gschecks.cpp

@@ -399,6 +399,9 @@ void Compiler::gsParamsToShadows()
         }
 
         int shadowVar = lvaGrabTemp(false DEBUGARG("shadowVar"));
+        // reload varDsc as lvaGrabTemp may realloc the lvaTable[]
+        varDsc = &lvaTable[lclNum];
+
         // Copy some info
 
         var_types type             = varTypeIsSmall(varDsc->TypeGet()) ? TYP_INT : varDsc->TypeGet();

src/jit/jit.h

@@ -591,29 +591,13 @@ struct JitOptions
 
 extern JitOptions jitOpts;
 
-/*****************************************************************************
-*
-*  Returns a word filled with the JITs allocator CHK fill value.
-*
-*/
+// Forward declarations for UninitializedWord and IsUninitialized are needed by alloc.h
 template <typename T>
-inline T UninitializedWord()
-{
-    __int64 word = 0x0101010101010101LL * (JitConfig.JitDefaultFill() & 0xFF);
-    return (T)word;
-}
-
-/*****************************************************************************
-*
-*  Determines whether this value is coming from uninitialized JIT memory
-*
-*/
+inline T UninitializedWord(Compiler* comp);
 
 template <typename T>
-inline bool IsUninitialized(T data)
-{
-    return data == UninitializedWord<T>();
-}
+inline bool IsUninitialized(T data);
+
 #endif // DEBUG
 
 /*****************************************************************************/
@@ -731,7 +715,6 @@ class Histogram
 #pragma warning(push)
 #pragma warning(default : 4820) // 'bytes' bytes padding added after construct 'member_name'
 #endif                          // CHECK_STRUCT_PADDING
-
 #include "alloc.h"
 #include "target.h"
 
@@ -947,9 +930,49 @@ class JitTls
 };
 
 #if defined(DEBUG)
-
+//  Include the definition of Compiler for use by these template functions
+//
 #include "compiler.h"
 
+//****************************************************************************
+//
+//  Returns a word filled with the JITs allocator default fill value.
+//
+template <typename T>
+inline T UninitializedWord(Compiler* comp)
+{
+    unsigned char defaultFill = 0xdd;
+    if (comp == nullptr)
+    {
+        comp = JitTls::GetCompiler();
+    }
+    defaultFill = comp->compGetJitDefaultFill();
+    assert(defaultFill <= 0xff);
+    __int64 word = 0x0101010101010101LL * defaultFill;
+    return (T)word;
+}
+
+//****************************************************************************
+//
+//  Tries to determine if this value is coming from uninitialized JIT memory
+//    - Returns true if the value matches what we initialized the memory to.
+//
+//  Notes:
+//    - Asserts that use this are assuming that the UninitializedWord value
+//      isn't a legal value for 'data'.  Thus using a default fill value of
+//      0x00 will often trigger such asserts.
+//
+template <typename T>
+inline bool IsUninitialized(T data)
+{
+    return data == UninitializedWord<T>(JitTls::GetCompiler());
+}
+
+//****************************************************************************
+//
+//  Debug template definitions for dspPtr, dspOffset
+//    - Used to format pointer/offset values for diffable Disasm
+//
 template <typename T>
 T dspPtr(T p)
 {
@@ -964,6 +987,11 @@ T dspOffset(T o)
 
 #else // !defined(DEBUG)
 
+//****************************************************************************
+//
+//  Non-Debug template definitions for dspPtr, dspOffset
+//    - This is a nop in non-Debug builds
+//
 template <typename T>
 T dspPtr(T p)
 {

src/jit/jitconfigvalues.h

@@ -40,7 +40,7 @@ CONFIG_INTEGER(JitCanUseSSE2, W("JitCanUseSSE2"), -1)
 CONFIG_INTEGER(JitCloneLoops, W("JitCloneLoops"), 1) // If 0, don't clone. Otherwise clone loops for optimizations.
 CONFIG_INTEGER(JitDebugLogLoopCloning, W("JitDebugLogLoopCloning"), 0) // In debug builds log places where loop cloning
                                                                        // optimizations are performed on the fast path.
-CONFIG_INTEGER(JitDefaultFill, W("JitDefaultFill"), 0xff) // In debug builds, initialize the memory allocated by the nra
+CONFIG_INTEGER(JitDefaultFill, W("JitDefaultFill"), 0xdd) // In debug builds, initialize the memory allocated by the nra
                                                           // with this byte.
 CONFIG_INTEGER(JitDirectAlloc, W("JitDirectAlloc"), 0)
 CONFIG_INTEGER(JitDoubleAlign, W("JitDoubleAlign"), 1)

src/jit/lclvars.cpp

@@ -1942,8 +1942,9 @@ void Compiler::lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* Stru
         }
 #endif
 
-        unsigned varNum = lvaGrabTemp(false DEBUGARG(bufp)); // Lifetime of field locals might span multiple BBs, so
-                                                             // they are long lifetime temps.
+        unsigned varNum = lvaGrabTemp(false DEBUGARG(bufp)); // Lifetime of field locals might span multiple BBs,
+                                                             // so they must be long lifetime temps.
+        varDsc = &lvaTable[lclNum];                          // lvaGrabTemp can reallocate the lvaTable
 
         LclVarDsc* fieldVarDsc       = &lvaTable[varNum];
         fieldVarDsc->lvType          = pFieldInfo->fldType;

src/jit/lower.cpp

@@ -2149,10 +2149,12 @@ void Lowering::LowerFastTailCall(GenTreeCall* call)
                     // Create tmp and use it in place of callerArgDsc
                     if (tmpLclNum == BAD_VAR_NUM)
                     {
+                        // Set tmpType first before calling lvaGrabTemp, as that call invalidates callerArgDsc
+                        tmpType   = genActualType(callerArgDsc->lvaArgType());
                         tmpLclNum = comp->lvaGrabTemp(
                             true DEBUGARG("Fast tail call lowering is creating a new local variable"));
+
                         comp->lvaSortAgain                          = true;
-                        tmpType                                     = genActualType(callerArgDsc->lvaArgType());
                         comp->lvaTable[tmpLclNum].lvType            = tmpType;
                         comp->lvaTable[tmpLclNum].lvRefCnt          = 1;
                         comp->lvaTable[tmpLclNum].lvDoNotEnregister = comp->lvaTable[lcl->gtLclNum].lvDoNotEnregister;