Fix BackoutJitData (#54711)

* Fix BackoutJitData The RemoveJitData that the BackoutJitData calls requires the code header to be copied to the final location. This change fixes it. I've also found that in one of my previous changes, I've accidentally enabled jitting into a scratch buffer by default by adding the FEATURE_WXORX define unconditionally. So I am removing it in this change for non Apple Silicon, it will be replaced by a dynamic check whether W^X is enabled in the final W^X change.
dotnet · Jun 25, 2021 · 3924d03 · 3924d03
1 parent de5dd0d
commit 3924d03
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 15 deletions.
diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp
@@ -2685,7 +2685,11 @@ void EEJitManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t reserveFo
         pCodeHdr = ((CodeHeader *)pCode) - 1;
 
         *pAllocatedSize = sizeof(CodeHeader) + totalSize;
-#define FEATURE_WXORX        
+
+#if defined(HOST_OSX) && defined(HOST_ARM64)
+#define FEATURE_WXORX
+#endif
+
 #ifdef FEATURE_WXORX
         pCodeHdrRW = (CodeHeader *)new BYTE[*pAllocatedSize];
 #else

diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp
@@ -11211,6 +11211,27 @@ void CEEJitInfo::GetProfilingHandle(bool                      *pbHookFunction,
     *pbIndirectedHandles = false;
 }
 
+/*********************************************************************/
+void CEEJitInfo::WriteCodeBytes()
+{
+    LIMITED_METHOD_CONTRACT;
+
+#ifdef USE_INDIRECT_CODEHEADER
+    if (m_pRealCodeHeader != NULL)
+    {
+        // Restore the read only version of the real code header
+        m_CodeHeaderRW->SetRealCodeHeader(m_pRealCodeHeader);
+        m_pRealCodeHeader = NULL;
+    }
+#endif // USE_INDIRECT_CODEHEADER
+
+    if (m_CodeHeaderRW != m_CodeHeader)
+    {
+        ExecutableWriterHolder<void> codeWriterHolder((void *)m_CodeHeader, m_codeWriteBufferSize);
+        memcpy(codeWriterHolder.GetRW(), m_CodeHeaderRW, m_codeWriteBufferSize);
+    }
+}
+
 /*********************************************************************/
 void CEEJitInfo::BackoutJitData(EEJitManager * jitMgr)
 {
@@ -11219,6 +11240,10 @@ void CEEJitInfo::BackoutJitData(EEJitManager * jitMgr)
         GC_TRIGGERS;
     } CONTRACTL_END;
 
+    // The RemoveJitData call below requires the m_CodeHeader to be valid, so we need to write
+    // the code bytes to the target memory location.
+    WriteCodeBytes();
+
     CodeHeader* pCodeHeader = m_CodeHeader;
     if (pCodeHeader)
         jitMgr->RemoveJitData(pCodeHeader, m_GCinfo_len, m_EHinfo_len);
@@ -11232,20 +11257,7 @@ void CEEJitInfo::WriteCode(EEJitManager * jitMgr)
         GC_TRIGGERS;
     } CONTRACTL_END;
 
-#ifdef USE_INDIRECT_CODEHEADER
-    if (m_pRealCodeHeader != NULL)
-    {
-        // Restore the read only version of the real code header
-        m_CodeHeaderRW->SetRealCodeHeader(m_pRealCodeHeader);
-        m_pRealCodeHeader = NULL;
-    }
-#endif // USE_INDIRECT_CODEHEADER
-
-    if (m_CodeHeaderRW != m_CodeHeader)
-    {
-        ExecutableWriterHolder<void> codeWriterHolder((void *)m_CodeHeader, m_codeWriteBufferSize);
-        memcpy(codeWriterHolder.GetRW(), m_CodeHeaderRW, m_codeWriteBufferSize);
-    }
+    WriteCodeBytes();
 
     // Now that the code header was written to the final location, publish the code via the nibble map
     jitMgr->NibbleMapSet(m_pCodeHeap, m_CodeHeader->GetCodeStartAddress(), TRUE);

diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h
@@ -941,6 +941,8 @@ class CEEJitInfo : public CEEInfo
 
 protected :
 
+    void WriteCodeBytes();
+
 #ifdef FEATURE_PGO
     // PGO data
     struct ComputedPgoData