Enable GC stress for ReadyToRun code

docs/design/coreclr/jit/investigate-stress.md

@@ -24,6 +24,8 @@ Enabling GC Hole Stress causes GCs to always occur in specific locations and tha
 - **0x8** – GC on every allowable R2R instr.
 - **0x10** – GC only on a unique stack trace.
 
+R2R instruction stress is not supported on Apple platforms, where write/execute restrictions can prevent patching image-backed code.
+
 ### Common combinations
 
 **0x1 | 0x2** – 0x3 are "in the VM". Failures in 0x1 or 0x2 can be due to VM-related reasons, like lack of GC reporting/pinning in interop frames.

src/coreclr/vm/gccover.cpp

@@ -8,7 +8,7 @@
 
 /* This file holds code that is designed to test GC pointer tracking in
    fully interruptible code.  We basically do a GC everywhere we can in
-   jitted code
+   managed code
  */
 /****************************************************************************/
 
@@ -32,6 +32,18 @@
 
 /****************************************************************************/
 
+GCStressCodeWriter::GCStressCodeWriter(BYTE* address, size_t size)
+    : m_addressRW(address)
+{
+    WRAPPER_NO_CONTRACT;
+
+    if (!ExecutionManager::IsReadyToRunCode(PINSTRToPCODE((TADDR)address)))
+    {
+        m_writer.AssignExecutableWriterHolder(address, size);
+        m_addressRW = m_writer.GetRW();
+    }
+}
+
 bool IsGcCoverageInterruptInstruction(PBYTE instrPtr)
 {
     UINT32 instrVal;
@@ -93,9 +105,6 @@ void SetupAndSprinkleBreakpoints(
 {
     _ASSERTE(!nativeCodeVersion.IsNull());
 
-    // CONSIDER: does anyone call this with fZapped == true ? are there plans?
-    _ASSERTE(!fZapped);
-
     // Allocate room for the GCCoverageInfo and copy of the method instructions
     MethodDesc *pMD = nativeCodeVersion.GetMethodDesc();
     size_t memSize = sizeof(GCCoverageInfo) + methodRegionInfo.hotSize + methodRegionInfo.coldSize;
@@ -130,7 +139,7 @@ void SetupAndSprinkleBreakpoints(
     nativeCodeVersion.SetGCCoverageInfo(gcCover);
 }
 
-void SetupAndSprinkleBreakpointsForJittedMethod(NativeCodeVersion               nativeCodeVersion,
+void SetupAndSprinkleBreakpointsForMethod(NativeCodeVersion                     nativeCodeVersion,
                                                 PCODE                           codeStart
                                                )
 {
@@ -151,12 +160,12 @@ void SetupAndSprinkleBreakpointsForJittedMethod(NativeCodeVersion
     SetupAndSprinkleBreakpoints(nativeCodeVersion,
                                 &codeInfo,
                                 methodRegionInfo,
-                                FALSE
+                                ExecutionManager::IsReadyToRunCode(codeStart)
                                );
 }
 
 /****************************************************************************/
-/* called when a method is first jitted when GCStress level 4 or 8 is on */
+/* called before publishing a method when GCStress level 4 or 8 is on */
 
 void SetupGcCoverage(NativeCodeVersion nativeCodeVersion, BYTE* methodStartPtr)
 {
@@ -192,7 +201,7 @@ void SetupGcCoverage(NativeCodeVersion nativeCodeVersion, BYTE* methodStartPtr)
     }
 
     PCODE codeStart = (PCODE) methodStartPtr;
-    SetupAndSprinkleBreakpointsForJittedMethod(nativeCodeVersion, codeStart);
+    SetupAndSprinkleBreakpointsForMethod(nativeCodeVersion, codeStart);
 }
 
 // There are some code path in DoGcStress to return without doing a GC but
@@ -373,11 +382,19 @@ static PBYTE getTargetOfCall(PBYTE instrPtr, PCONTEXT regs, PBYTE* nextInstr) {
 
 // There is a call target instruction, try to find the MethodDesc for where target points to.
 // Returns nullptr if it can't find it.
-static MethodDesc* getTargetMethodDesc(PCODE target)
+static MethodDesc* getTargetMethodDesc(PCODE target, PCONTEXT regs)
 {
     MethodDesc* targetMD = ExecutionManager::GetCodeMethodDesc(target);
     if (targetMD != nullptr)
     {
+        // Interior targets such as callfinally do not follow the parent's return convention
+        // and can return into a no-GC region.
+        EECodeInfo targetCodeInfo(target);
+        if (targetCodeInfo.IsValid() && targetCodeInfo.GetRelOffset() != 0)
+        {
+            return nullptr;
+        }
+
         // It is JIT/NGened call.
         return targetMD;
     }
@@ -395,6 +412,19 @@ static MethodDesc* getTargetMethodDesc(PCODE target)
             // It is a VSD stub manager.
             DispatchToken token(VirtualCallStubManager::GetTokenFromStubQuick(pVSDStubManager, target, stubKind));
             _ASSERTE(token.IsValid());
+            if (token.IsThisToken())
+            {
+                // Class tokens encode only a slot. The receiver is available in ECX at the call,
+                // but not while sprinkling breakpoints, and null receivers must fault in the stub.
+                if (regs == nullptr || regs->Ecx == 0)
+                {
+                    return nullptr;
+                }
+
+                Object* receiver = reinterpret_cast<Object*>(regs->Ecx);
+                return VirtualCallStubManager::GetRepresentativeMethodDescFromToken(token, receiver->GetMethodTable());
+            }
+
             return VirtualCallStubManager::GetInterfaceMethodDescFromToken(token);
         }
     }
@@ -486,7 +516,8 @@ void GCCoverageInfo::SprinkleBreakpoints(
     if (fZapped)
     {
         DWORD oldProtect;
-        ClrVirtualProtect(codeStart, codeSize, PAGE_EXECUTE_READWRITE, &oldProtect);
+        if (!ClrVirtualProtect(codeStart, codeSize, PAGE_EXECUTE_READWRITE, &oldProtect))
+            EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_EXECUTIONENGINE, W("Failed to change code protection for GC stress"));
         writeableOffset = 0;
     }
     else
@@ -559,7 +590,7 @@ void GCCoverageInfo::SprinkleBreakpoints(
 
                 if (target != 0)
                 {
-                    targetMD = getTargetMethodDesc((PCODE)target);
+                    targetMD = getTargetMethodDesc((PCODE)target, nullptr);
                 }
             }
             break;
@@ -813,7 +844,7 @@ void DoGcStress (PCONTEXT regs, NativeCodeVersion nativeCodeVersion)
         PBYTE target = getTargetOfCall((BYTE*) instrPtr, regs, (BYTE**)&nextInstr);
         if (target != 0)
         {
-            ExecutableWriterHolder<BYTE> nextInstrWriterHolder(nextInstr, sizeof(DWORD));
+            GCStressCodeWriter nextInstrWriterHolder(nextInstr, sizeof(DWORD));
             if (!pThread->PreemptiveGCDisabled())
             {
                 // We are in preemptive mode in JITTed code. This implies that we are into IL stub
@@ -822,7 +853,7 @@ void DoGcStress (PCONTEXT regs, NativeCodeVersion nativeCodeVersion)
             }
             else
             {
-                MethodDesc* targetMD = getTargetMethodDesc((PCODE)target);
+                MethodDesc* targetMD = getTargetMethodDesc((PCODE)target, regs);
 
                 if (targetMD != 0)
                 {
@@ -956,7 +987,7 @@ void replaceSafePointInstructionWithGcStressInstr(GcInfoDecoder* decoder, UINT32
     // The instruction about to be replaced cannot already be a gcstress instruction
     _ASSERTE(!IsGcCoverageInterruptInstruction(instrPtr));
 
-    ExecutableWriterHolder<BYTE> instrPtrWriterHolder(instrPtr, sizeof(DWORD));
+    GCStressCodeWriter instrPtrWriterHolder(instrPtr, sizeof(DWORD));
 #if defined(TARGET_ARM)
     size_t instrLen = GetARMInstructionLength(instrPtr);
 
@@ -1031,7 +1062,7 @@ bool replaceInterruptibleRangesWithGcStressInstr (UINT32 startOffset, UINT32 sto
     // Need to do two iterations if interruptible range spans across hot & cold region
     while(acrossHotRegion--)
     {
-        ExecutableWriterHolder<BYTE> instrPtrWriterHolder(rangeStart, rangeStop - rangeStart);
+        GCStressCodeWriter instrPtrWriterHolder(rangeStart, rangeStop - rangeStart);
         PBYTE instrPtrRW =  instrPtrWriterHolder.GetRW();
         PBYTE rangeStopRW = instrPtrRW + (rangeStop - rangeStart);
         while(instrPtrRW < rangeStopRW)
@@ -1099,11 +1130,13 @@ void GCCoverageInfo::SprinkleBreakpoints(
     if (fZapped)
     {
         DWORD oldProtect;
-        ClrVirtualProtect((BYTE*)methodRegion.hotStartAddress, methodRegion.hotSize, PAGE_EXECUTE_READWRITE, &oldProtect);
+        if (!ClrVirtualProtect((BYTE*)methodRegion.hotStartAddress, methodRegion.hotSize, PAGE_EXECUTE_READWRITE, &oldProtect))
+            EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_EXECUTIONENGINE, W("Failed to change code protection for GC stress"));
 
         if (methodRegion.coldSize > 0)
         {
-            ClrVirtualProtect((BYTE*)methodRegion.coldStartAddress, methodRegion.coldSize, PAGE_EXECUTE_READWRITE, &oldProtect);
+            if (!ClrVirtualProtect((BYTE*)methodRegion.coldStartAddress, methodRegion.coldSize, PAGE_EXECUTE_READWRITE, &oldProtect))
+                EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_EXECUTIONENGINE, W("Failed to change code protection for GC stress"));
         }
     }
 
@@ -1258,7 +1291,7 @@ bool IsGcCoverageInterrupt(LPVOID ip)
 
 void RemoveGcCoverageInterrupt(TADDR instrPtr, BYTE * savedInstrPtr, GCCoverageInfo* gcCover, DWORD offset)
 {
-    ExecutableWriterHolder<void> instrPtrWriterHolder((void*)instrPtr, 4);
+    GCStressCodeWriter instrPtrWriterHolder((BYTE*)instrPtr, 4);
 #ifdef TARGET_ARM
     if (GetARMInstructionLength(savedInstrPtr) == 2)
         *(WORD *)instrPtrWriterHolder.GetRW()  = *(WORD *)savedInstrPtr;

src/coreclr/vm/gccover.h

@@ -8,6 +8,19 @@
 
 #ifdef HAVE_GCCOVER
 
+#ifndef DACCESS_COMPILE
+// R2R image pages are not owned by the JIT executable allocator.
+class GCStressCodeWriter
+{
+    ExecutableWriterHolderNoLog<BYTE> m_writer;
+    BYTE* m_addressRW;
+
+public:
+    GCStressCodeWriter(BYTE* address, size_t size);
+    BYTE* GetRW() const { return m_addressRW; }
+};
+#endif // !DACCESS_COMPILE
+
 /****************************************************************************/
 /* GCCOverageInfo holds the state of which instructions have been visited by
    a GC and which ones have not */

src/coreclr/vm/jitinterface.cpp

@@ -15597,6 +15597,15 @@ NativeCodeVersion EECodeInfo::GetNativeCodeVersion() const
         return NativeCodeVersion();
     }
 
+#if defined(HAVE_GCCOVER) && defined(FEATURE_READYTORUN)
+    // R2R code is the default version, even when its entry point was registered
+    // by a different instantiation or by diagnostic enumeration before publication.
+    if (m_pJM == ExecutionManager::GetReadyToRunJitManager())
+    {
+        return NativeCodeVersion(pMD);
+    }
+#endif // HAVE_GCCOVER && FEATURE_READYTORUN
+
 #ifdef FEATURE_CODE_VERSIONING
     if (pMD->IsVersionable())
     {

src/coreclr/vm/method.hpp

@@ -2377,6 +2377,9 @@ class MethodDesc
 
 public:
     static void Init();
+#ifdef FEATURE_READYTORUN
+    static void SetupGcCoverageForPrecompiledCode(PCODE pCode);
+#endif // FEATURE_READYTORUN
 #endif
 
     friend struct ::cdac_data<MethodDesc>;

src/coreclr/vm/prestub.cpp

@@ -65,6 +65,26 @@ void MethodDesc::Init()
     m_GCCoverCrst.Init(CrstGCCover);
 }
 
+#ifdef FEATURE_READYTORUN
+void MethodDesc::SetupGcCoverageForPrecompiledCode(PCODE pCode)
+{
+    STANDARD_VM_CONTRACT;
+
+    // Image-backed code may not be patchable under Apple's write/execute restrictions.
+#ifndef HOST_APPLE
+    if (GCStress<cfg_instr_ngen>::IsEnabled())
+    {
+        CrstHolder gcCoverLock(&m_GCCoverCrst);
+        EECodeInfo codeInfo(pCode);
+        _ASSERTE(codeInfo.IsValid());
+
+        // Shared R2R code must use the same MethodDesc as the interrupt handler.
+        SetupGcCoverage(NativeCodeVersion(codeInfo.GetMethodDesc()), (BYTE*)pCode);
+    }
+#endif // !HOST_APPLE
+}
+#endif // FEATURE_READYTORUN
+
 #endif
 
 #define LOG_USING_R2R_CODE(method)  LOG((LF_ZAP, LL_INFO10000,                                                            \

src/coreclr/vm/readytoruninfo.cpp

@@ -1276,7 +1276,12 @@ void ReadyToRunInfo::RegisterResumptionStub(PCODE stubEntryPoint)
 
     // Use the entry point hashtable to check if another thread already registered a MethodDesc
     if (m_pCompositeInfo->GetMethodDescForEntryPointInNativeImage(stubEntryPoint) != NULL)
+    {
+#ifdef HAVE_GCCOVER
+        MethodDesc::SetupGcCoverageForPrecompiledCode(stubEntryPoint);
+#endif // HAVE_GCCOVER
         return;
+    }
 
     MethodDesc* pStubMD = pStubCache->CreateR2RBackedILStub(
         pLoaderAllocator,
@@ -1300,6 +1305,9 @@ void ReadyToRunInfo::RegisterResumptionStub(PCODE stubEntryPoint)
     {
         amTracker.SuppressRelease();
     }
+#ifdef HAVE_GCCOVER
+    MethodDesc::SetupGcCoverageForPrecompiledCode(stubEntryPoint);
+#endif // HAVE_GCCOVER
 }
 
 PCODE ReadyToRunInfo::GetEntryPoint(MethodDesc * pMD, PrepareCodeConfig* pConfig, BOOL fFixups)
@@ -1444,6 +1452,11 @@ PCODE ReadyToRunInfo::GetEntryPoint(MethodDesc * pMD, PrepareCodeConfig* pConfig
 #endif
 #endif
 
+#ifdef HAVE_GCCOVER
+    // Instrument before any caller can publish or execute the entry point.
+    MethodDesc::SetupGcCoverageForPrecompiledCode(pEntryPoint);
+#endif // HAVE_GCCOVER
+
 #ifdef PROFILING_SUPPORTED
         {
             BEGIN_PROFILER_CALLBACK(CORProfilerTrackCacheSearches());

src/coreclr/vm/threadsuspend.cpp

@@ -18,6 +18,7 @@
 #include <minipal/time.h>
 
 #include "exinfo.h"
+#include "gccover.h"
 
 #define HIJACK_NONINTERRUPTIBLE_THREADS
 
@@ -3449,7 +3450,7 @@ void Thread::CommitGCStressInstructionUpdate()
         assert(pbDestCode != NULL);
         assert(pbSrcCode != NULL);
 
-        ExecutableWriterHolder<BYTE> destCodeWriterHolder(pbDestCode, sizeof(DWORD));
+        GCStressCodeWriter destCodeWriterHolder(pbDestCode, sizeof(DWORD));
 
 #if defined(TARGET_X86) || defined(TARGET_AMD64)