GCStress: try to reduce races and tolerate races better

src/inc/CrstTypes.def

@@ -352,6 +352,10 @@ Crst NativeImageCache
     Unordered
 End
 
+Crst GCCover
+    AcquiredBefore LoaderHeap
+End
+
 Crst GCMemoryPressure
 End
 

src/inc/crsttypes.h

@@ -83,111 +83,112 @@ enum CrstType
     CrstFusionPolicyConfigPool = 64,
     CrstFusionSingleUse = 65,
     CrstFusionWarningLog = 66,
-    CrstGCMemoryPressure = 67,
-    CrstGlobalStrLiteralMap = 68,
-    CrstHandleTable = 69,
-    CrstHostAssemblyMap = 70,
-    CrstHostAssemblyMapAdd = 71,
-    CrstIbcProfile = 72,
-    CrstIJWFixupData = 73,
-    CrstIJWHash = 74,
-    CrstILFingerprintCache = 75,
-    CrstILStubGen = 76,
-    CrstInlineTrackingMap = 77,
-    CrstInstMethodHashTable = 78,
-    CrstInterfaceVTableMap = 79,
-    CrstInterop = 80,
-    CrstInteropData = 81,
-    CrstIOThreadpoolWorker = 82,
-    CrstIsJMCMethod = 83,
-    CrstISymUnmanagedReader = 84,
-    CrstJit = 85,
-    CrstJitGenericHandleCache = 86,
-    CrstJitPerf = 87,
-    CrstJumpStubCache = 88,
-    CrstLeafLock = 89,
-    CrstListLock = 90,
-    CrstLoaderAllocator = 91,
-    CrstLoaderAllocatorReferences = 92,
-    CrstLoaderHeap = 93,
-    CrstMda = 94,
-    CrstMetadataTracker = 95,
-    CrstModIntPairList = 96,
-    CrstModule = 97,
-    CrstModuleFixup = 98,
-    CrstModuleLookupTable = 99,
-    CrstMulticoreJitHash = 100,
-    CrstMulticoreJitManager = 101,
-    CrstMUThunkHash = 102,
-    CrstNativeBinderInit = 103,
-    CrstNativeImageCache = 104,
-    CrstNls = 105,
-    CrstNotifyGdb = 106,
-    CrstObjectList = 107,
-    CrstOnEventManager = 108,
-    CrstPatchEntryPoint = 109,
-    CrstPEFileSecurityManager = 110,
-    CrstPEImage = 111,
-    CrstPEImagePDBStream = 112,
-    CrstPendingTypeLoadEntry = 113,
-    CrstPinHandle = 114,
-    CrstPinnedByrefValidation = 115,
-    CrstProfilerGCRefDataFreeList = 116,
-    CrstProfilingAPIStatus = 117,
-    CrstPublisherCertificate = 118,
-    CrstRCWCache = 119,
-    CrstRCWCleanupList = 120,
-    CrstRCWRefCache = 121,
-    CrstReadyToRunEntryPointToMethodDescMap = 122,
-    CrstReDacl = 123,
-    CrstReflection = 124,
-    CrstReJITDomainTable = 125,
-    CrstReJITGlobalRequest = 126,
-    CrstReJITSharedDomainTable = 127,
-    CrstRemoting = 128,
-    CrstRetThunkCache = 129,
-    CrstRWLock = 130,
-    CrstSavedExceptionInfo = 131,
-    CrstSaveModuleProfileData = 132,
-    CrstSecurityPolicyCache = 133,
-    CrstSecurityPolicyInit = 134,
-    CrstSecurityStackwalkCache = 135,
-    CrstSharedAssemblyCreate = 136,
-    CrstSharedBaseDomain = 137,
-    CrstSigConvert = 138,
-    CrstSingleUseLock = 139,
-    CrstSpecialStatics = 140,
-    CrstSqmManager = 141,
-    CrstStackSampler = 142,
-    CrstStressLog = 143,
-    CrstStrongName = 144,
-    CrstStubCache = 145,
-    CrstStubDispatchCache = 146,
-    CrstStubUnwindInfoHeapSegments = 147,
-    CrstSyncBlockCache = 148,
-    CrstSyncHashLock = 149,
-    CrstSystemBaseDomain = 150,
-    CrstSystemDomain = 151,
-    CrstSystemDomainDelayedUnloadList = 152,
-    CrstThreadIdDispenser = 153,
-    CrstThreadpoolEventCache = 154,
-    CrstThreadpoolTimerQueue = 155,
-    CrstThreadpoolWaitThreads = 156,
-    CrstThreadpoolWorker = 157,
-    CrstThreadStaticDataHashTable = 158,
-    CrstThreadStore = 159,
-    CrstTPMethodTable = 160,
-    CrstTypeEquivalenceMap = 161,
-    CrstTypeIDMap = 162,
-    CrstUMEntryThunkCache = 163,
-    CrstUMThunkHash = 164,
-    CrstUniqueStack = 165,
-    CrstUnresolvedClassLock = 166,
-    CrstUnwindInfoTableLock = 167,
-    CrstVSDIndirectionCellLock = 168,
-    CrstWinRTFactoryCache = 169,
-    CrstWrapperTemplate = 170,
-    kNumberOfCrstTypes = 171
+    CrstGCCover = 67,
+    CrstGCMemoryPressure = 68,
+    CrstGlobalStrLiteralMap = 69,
+    CrstHandleTable = 70,
+    CrstHostAssemblyMap = 71,
+    CrstHostAssemblyMapAdd = 72,
+    CrstIbcProfile = 73,
+    CrstIJWFixupData = 74,
+    CrstIJWHash = 75,
+    CrstILFingerprintCache = 76,
+    CrstILStubGen = 77,
+    CrstInlineTrackingMap = 78,
+    CrstInstMethodHashTable = 79,
+    CrstInterfaceVTableMap = 80,
+    CrstInterop = 81,
+    CrstInteropData = 82,
+    CrstIOThreadpoolWorker = 83,
+    CrstIsJMCMethod = 84,
+    CrstISymUnmanagedReader = 85,
+    CrstJit = 86,
+    CrstJitGenericHandleCache = 87,
+    CrstJitPerf = 88,
+    CrstJumpStubCache = 89,
+    CrstLeafLock = 90,
+    CrstListLock = 91,
+    CrstLoaderAllocator = 92,
+    CrstLoaderAllocatorReferences = 93,
+    CrstLoaderHeap = 94,
+    CrstMda = 95,
+    CrstMetadataTracker = 96,
+    CrstModIntPairList = 97,
+    CrstModule = 98,
+    CrstModuleFixup = 99,
+    CrstModuleLookupTable = 100,
+    CrstMulticoreJitHash = 101,
+    CrstMulticoreJitManager = 102,
+    CrstMUThunkHash = 103,
+    CrstNativeBinderInit = 104,
+    CrstNativeImageCache = 105,
+    CrstNls = 106,
+    CrstNotifyGdb = 107,
+    CrstObjectList = 108,
+    CrstOnEventManager = 109,
+    CrstPatchEntryPoint = 110,
+    CrstPEFileSecurityManager = 111,
+    CrstPEImage = 112,
+    CrstPEImagePDBStream = 113,
+    CrstPendingTypeLoadEntry = 114,
+    CrstPinHandle = 115,
+    CrstPinnedByrefValidation = 116,
+    CrstProfilerGCRefDataFreeList = 117,
+    CrstProfilingAPIStatus = 118,
+    CrstPublisherCertificate = 119,
+    CrstRCWCache = 120,
+    CrstRCWCleanupList = 121,
+    CrstRCWRefCache = 122,
+    CrstReadyToRunEntryPointToMethodDescMap = 123,
+    CrstReDacl = 124,
+    CrstReflection = 125,
+    CrstReJITDomainTable = 126,
+    CrstReJITGlobalRequest = 127,
+    CrstReJITSharedDomainTable = 128,
+    CrstRemoting = 129,
+    CrstRetThunkCache = 130,
+    CrstRWLock = 131,
+    CrstSavedExceptionInfo = 132,
+    CrstSaveModuleProfileData = 133,
+    CrstSecurityPolicyCache = 134,
+    CrstSecurityPolicyInit = 135,
+    CrstSecurityStackwalkCache = 136,
+    CrstSharedAssemblyCreate = 137,
+    CrstSharedBaseDomain = 138,
+    CrstSigConvert = 139,
+    CrstSingleUseLock = 140,
+    CrstSpecialStatics = 141,
+    CrstSqmManager = 142,
+    CrstStackSampler = 143,
+    CrstStressLog = 144,
+    CrstStrongName = 145,
+    CrstStubCache = 146,
+    CrstStubDispatchCache = 147,
+    CrstStubUnwindInfoHeapSegments = 148,
+    CrstSyncBlockCache = 149,
+    CrstSyncHashLock = 150,
+    CrstSystemBaseDomain = 151,
+    CrstSystemDomain = 152,
+    CrstSystemDomainDelayedUnloadList = 153,
+    CrstThreadIdDispenser = 154,
+    CrstThreadpoolEventCache = 155,
+    CrstThreadpoolTimerQueue = 156,
+    CrstThreadpoolWaitThreads = 157,
+    CrstThreadpoolWorker = 158,
+    CrstThreadStaticDataHashTable = 159,
+    CrstThreadStore = 160,
+    CrstTPMethodTable = 161,
+    CrstTypeEquivalenceMap = 162,
+    CrstTypeIDMap = 163,
+    CrstUMEntryThunkCache = 164,
+    CrstUMThunkHash = 165,
+    CrstUniqueStack = 166,
+    CrstUnresolvedClassLock = 167,
+    CrstUnwindInfoTableLock = 168,
+    CrstVSDIndirectionCellLock = 169,
+    CrstWinRTFactoryCache = 170,
+    CrstWrapperTemplate = 171,
+    kNumberOfCrstTypes = 172
 };
 
 #endif // __CRST_TYPES_INCLUDED
@@ -265,6 +266,7 @@ int g_rgCrstLevelMap[] =
     4,			// CrstFusionPolicyConfigPool
     5,			// CrstFusionSingleUse
     6,			// CrstFusionWarningLog
+    3,			// CrstGCCover
     0,			// CrstGCMemoryPressure
     11,			// CrstGlobalStrLiteralMap
     1,			// CrstHandleTable
@@ -441,6 +443,7 @@ LPCSTR g_rgCrstNameMap[] =
     "CrstFusionPolicyConfigPool",
     "CrstFusionSingleUse",
     "CrstFusionWarningLog",
+    "CrstGCCover",
     "CrstGCMemoryPressure",
     "CrstGlobalStrLiteralMap",
     "CrstHandleTable",

src/inc/switches.h

@@ -114,6 +114,12 @@
 #define HAVE_GCCOVER
 #endif
 
+// Some platforms may see spurious AVs when GcCoverage is enabled because of races.
+// Enable further processing to see if they recur.
+#if defined(HAVE_GCCOVER) && (defined(_TARGET_X86_) || defined(_TARGET_AMD64_)) && !defined(FEATURE_PAL)
+#define GCCOVER_TOLERATE_SPURIOUS_AV
+#endif
+
 //Turns on a startup delay to allow simulation of slower and faster startup times.
 #define ENABLE_STARTUP_DELAY
 

src/vm/ceemain.cpp

@@ -1124,6 +1124,10 @@ void EEStartupHelper(COINITIEE fFlags)
 
 #endif // _DEBUG
 
+#ifdef HAVE_GCCOVER
+        MethodDesc::Init();
+#endif
+
 #endif // !CROSSGEN_COMPILE
 
 ErrExit: ;

src/vm/dynamicmethod.cpp

@@ -280,6 +280,11 @@ DynamicMethodDesc* DynamicMethodTable::GetDynamicMethod(BYTE *psig, DWORD sigSiz
     pNewMD->m_pszDebugClassName  = (LPUTF8)"dynamicclass";
     pNewMD->m_pszDebugMethodSignature = "DynamicMethod Signature not available";
 #endif // _DEBUG
+
+#ifdef HAVE_GCCOVER
+    pNewMD->m_GcCover = NULL;
+#endif
+
     pNewMD->SetNotInline(TRUE);
     pNewMD->GetLCGMethodResolver()->Reset();
 

src/vm/excep.cpp

@@ -6811,34 +6811,39 @@ DWORD GetGcMarkerExceptionCode(LPVOID ip)
 }
 
 // Did we hit an DO_A_GC_HERE marker in JITted code?
-bool IsGcMarker(DWORD exceptionCode, CONTEXT *pContext)
+bool IsGcMarker(CONTEXT* pContext, EXCEPTION_RECORD *pExceptionRecord)
 {
+    DWORD exceptionCode = pExceptionRecord->ExceptionCode;
 #ifdef HAVE_GCCOVER
     WRAPPER_NO_CONTRACT;
 
     if (GCStress<cfg_any>::IsEnabled())
     {
-#ifdef _TARGET_X86_
-        // on x86 we can't suspend EE to update the GC marker instruction so
-        // we update it directly without suspending.  this can sometimes yield
-        // a STATUS_ACCESS_VIOLATION instead of STATUS_CLR_GCCOVER_CODE.  in
-        // this case we let the AV through and retry the instruction.  we'll
-        // track the IP of the instruction that generated an AV so we don't
-        // mix up a real AV with a "fake" AV.
-        // see comments in function DoGcStress for more details on this race.
-        // also make sure that the thread is actually in managed code since AVs
-        // outside of of JIT code will never be potential GC markers
+#if defined(GCCOVER_TOLERATE_SPURIOUS_AV)
+
+        // We sometimes can't suspend the EE to update the GC marker instruction so
+        // we update it directly without suspending.  This can sometimes yield
+        // a STATUS_ACCESS_VIOLATION instead of STATUS_CLR_GCCOVER_CODE.  In
+        // this case we let the AV through and retry the instruction as hopefully
+        // the race will have resolved.  We'll track the IP of the instruction
+        // that generated an AV so we don't mix up a real AV with a "fake" AV.
+        //
+        // See comments in function DoGcStress for more details on this race.
+        //
+        // Note these "fake" AVs will be reported by the kernel as reads from
+        // address 0xF...F so we also use that as a screen.
         Thread* pThread = GetThread();
         if (exceptionCode == STATUS_ACCESS_VIOLATION &&
             GCStress<cfg_instr>::IsEnabled() &&
+            pExceptionRecord->ExceptionInformation[0] == 0 &&
+            pExceptionRecord->ExceptionInformation[1] == ~0 &&
             pThread->GetLastAVAddress() != (LPVOID)GetIP(pContext) &&
-            pThread->PreemptiveGCDisabled() &&
             !IsIPInEE((LPVOID)GetIP(pContext)))
         {
             pThread->SetLastAVAddress((LPVOID)GetIP(pContext));
             return true;
         }
-#endif // _TARGET_X86_
+#endif // defined(GCCOVER_TOLERATE_SPURIOUS_AV)
 
         if (exceptionCode == STATUS_CLR_GCCOVER_CODE)
         {
@@ -7737,7 +7742,7 @@ VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase3(PEXCEPTION_POINTERS pExcepti
     // NOTE: this is effectively ifdef (_TARGET_AMD64_ || _TARGET_ARM_), and does not actually trigger
     // a GC.  This will redirect the exception context to a stub which will
     // push a frame and cause GC.
-    if (IsGcMarker(exceptionCode, pContext))
+    if (IsGcMarker(pContext, pExceptionRecord))
     {
         return VEH_CONTINUE_EXECUTION;;
     }
@@ -8161,11 +8166,11 @@ LONG WINAPI CLRVectoredExceptionHandlerShim(PEXCEPTION_POINTERS pExceptionInfo)
 
 #ifdef USE_REDIRECT_FOR_GCSTRESS
     // This is AMD64 & ARM specific as the macro above is defined for AMD64 & ARM only
-    bIsGCMarker = IsGcMarker(dwCode, pExceptionInfo->ContextRecord);
+    bIsGCMarker = IsGcMarker(pExceptionInfo->ContextRecord, pExceptionInfo->ExceptionRecord);
 #elif defined(_TARGET_X86_) && defined(HAVE_GCCOVER)
     // This is the equivalent of the check done in COMPlusFrameHandler, incase the exception is
     // seen by VEH first on x86.
-    bIsGCMarker = IsGcMarker(dwCode, pExceptionInfo->ContextRecord);
+    bIsGCMarker = IsGcMarker(pExceptionInfo->ContextRecord, pExceptionInfo->ExceptionRecord);
 #endif // USE_REDIRECT_FOR_GCSTRESS
 
     // Do not update the TLS with exception details for exceptions pertaining to GCStress

src/vm/excep.h

@@ -766,7 +766,7 @@ void CPFH_AdjustContextForThreadSuspensionRace(T_CONTEXT *pContext, Thread *pThr
 #endif // _TARGET_X86_
 
 DWORD GetGcMarkerExceptionCode(LPVOID ip);
-bool IsGcMarker(DWORD exceptionCode, T_CONTEXT *pContext);
+bool IsGcMarker(T_CONTEXT *pContext, EXCEPTION_RECORD *pExceptionRecord);
 
 void InitSavedExceptionInfo();
 

src/vm/exceptionhandling.cpp

@@ -936,7 +936,7 @@ ProcessCLRException(IN     PEXCEPTION_RECORD   pExceptionRecord
     //
     {
 #ifndef USE_REDIRECT_FOR_GCSTRESS
-        if (IsGcMarker(pExceptionRecord->ExceptionCode, pContextRecord))
+        if (IsGcMarker(pContextRecord, pExceptionRecord))
         {
             returnDisposition = ExceptionContinueExecution;
             goto lExit;
@@ -5227,7 +5227,7 @@ BOOL HandleHardwareException(PAL_SEHException* ex)
         // A hardware exception is handled only if it happened in a jitted code or 
         // in one of the JIT helper functions (JIT_MemSet, ...)
         PCODE controlPc = GetIP(ex->GetContextRecord());
-        if (ExecutionManager::IsManagedCode(controlPc) && IsGcMarker(ex->GetExceptionRecord()->ExceptionCode, ex->GetContextRecord()))
+        if (ExecutionManager::IsManagedCode(controlPc) && IsGcMarker(ex->GetContextRecord(), ex->GetExceptionRecord()))
         {
             // Exception was handled, let the signal handler return to the exception context. Some registers in the context can
             // have been modified by the GC.