Fix GC stress C bug for arm (#15269)

* Fix GC stress C bug for arm

The OnGcCoverageInterrupt function was not removing the "thumb" bit from
the address before putting back the original instruction that was
previously replaced by an invalid instruction that invokes the
OnGcCoverageInterrupt. That resulted in the original instruction being
restored to incorrect location (to the original address + 1) and later
to the GC stress failure.

* Reflect PR feedback

diff --git a/src/vm/gccover.cpp b/src/vm/gccover.cpp
index 895c176..d61a168 100644 (file)
--- a/src/vm/gccover.cpp
+++ b/src/vm/gccover.cpp
@@ -1293,7 +1293,7 @@ bool IsGcCoverageInterrupt(LPVOID ip)
 // original instruction. Only one instruction must be used, 
 // because multiple threads can be executing the same code stream.
 
-void RemoveGcCoverageInterrupt(Volatile<BYTE>* instrPtr, BYTE * savedInstrPtr)
+void RemoveGcCoverageInterrupt(TADDR instrPtr, BYTE * savedInstrPtr)
 {
 #ifdef _TARGET_ARM_
         if (GetARMInstructionLength(savedInstrPtr) == 2)
@@ -1303,7 +1303,7 @@ void RemoveGcCoverageInterrupt(Volatile<BYTE>* instrPtr, BYTE * savedInstrPtr)
 #elif defined(_TARGET_ARM64_)
         *(DWORD *)instrPtr = *(DWORD *)savedInstrPtr;
 #else
-        *instrPtr = *savedInstrPtr;
+        *(BYTE *)instrPtr = *savedInstrPtr;
 #endif
 }
 
@@ -1315,12 +1315,12 @@ BOOL OnGcCoverageInterrupt(PCONTEXT regs)
     GCcoverCount++;
     forceStack[0]= &regs;                // This is so I can see it fastchecked
 
-    BYTE* pControlPc = (BYTE*)GetIP(regs);
+    PCODE controlPc = GetIP(regs);
+    TADDR instrPtr = PCODEToPINSTR(controlPc);
 
-    Volatile<BYTE>* instrPtr = (Volatile<BYTE>*)pControlPc;
     forceStack[0] = &instrPtr;            // This is so I can see it fastchecked
 
-    EECodeInfo codeInfo((PCODE)pControlPc);
+    EECodeInfo codeInfo(controlPc);
     if (!codeInfo.IsValid())
         return(FALSE);
 
@@ -1384,19 +1384,19 @@ FORCEINLINE void UpdateGCStressInstructionWithoutGC ()
 
 void DoGcStress (PCONTEXT regs, MethodDesc *pMD)
 {
-    BYTE* pControlPc = (BYTE*)GetIP(regs);
-    Volatile<BYTE>* instrPtr = (Volatile<BYTE>*)pControlPc;
+    PCODE controlPc = GetIP(regs);
+    TADDR instrPtr = PCODEToPINSTR(controlPc);
 
     if (!pMD)
     {
-        pMD = ExecutionManager::GetCodeMethodDesc((PCODE)pControlPc);
+        pMD = ExecutionManager::GetCodeMethodDesc(controlPc);
         if (!pMD)
             return;
     }
 
     GCCoverageInfo *gcCover = pMD->m_GcCover;
 
-    EECodeInfo codeInfo((TADDR)instrPtr);
+    EECodeInfo codeInfo(controlPc);
     _ASSERTE(codeInfo.GetMethodDesc() == pMD);
     DWORD offset = codeInfo.GetRelOffset();
 
@@ -1404,7 +1404,7 @@ void DoGcStress (PCONTEXT regs, MethodDesc *pMD)
 
 #if defined(_TARGET_X86_) || defined(_TARGET_AMD64_)
 
-    BYTE instrVal = *instrPtr;
+    BYTE instrVal = *(BYTE *)instrPtr;
     forceStack[6] = &instrVal;            // This is so I can see it fastchecked
     
     if (instrVal != INTERRUPT_INSTR && 
@@ -1419,9 +1419,6 @@ void DoGcStress (PCONTEXT regs, MethodDesc *pMD)
 
 #elif defined(_TARGET_ARM_)
 
-    _ASSERTE(((TADDR)instrPtr) & THUMB_CODE);
-    instrPtr = instrPtr - THUMB_CODE;
-
     WORD instrVal = *(WORD*)instrPtr;
     forceStack[6] = &instrVal;            // This is so I can see it fastchecked
 
@@ -1680,7 +1677,7 @@ void DoGcStress (PCONTEXT regs, MethodDesc *pMD)
         }
 
         // Must flush instruction cache before returning as instruction has been modified.
-        FlushInstructionCache(GetCurrentProcess(), instrPtr, 6);
+        FlushInstructionCache(GetCurrentProcess(), (LPCVOID)instrPtr, 6);
 
         // It's not GC safe point, the GC Stress instruction is 
         // already commited and interrupt is already put at next instruction so we just return.
@@ -1770,7 +1767,7 @@ void DoGcStress (PCONTEXT regs, MethodDesc *pMD)
         UpdateGCStressInstructionWithoutGC ();
 
     // Must flush instruction cache before returning as instruction has been modified.
-    FlushInstructionCache(GetCurrentProcess(), instrPtr, 4);
+    FlushInstructionCache(GetCurrentProcess(), (LPCVOID)instrPtr, 4);
 
     CONSISTENCY_CHECK(!pThread->HasPendingGCStressInstructionUpdate());