Fix unwind info when fake-splitting; add job to runtime-jit-experimental (#69922)
authorAman Khalid <t-amankhalid@microsoft.com>
Tue, 7 Jun 2022 17:47:31 +0000 (10:47 -0700)
committerGitHub <noreply@github.com>
Tue, 7 Jun 2022 17:47:31 +0000 (10:47 -0700)
Fake-splitting currently breaks stack walks by not generating unwind
info for cold code. This commit implements unwind info on x86/64 when
fake-splitting by generating unwind info for the combined hot/cold
section just once, rather than generating unwind info for the separate
sections.

For reasons to be investigated, this implementation does not work when
the code sections are separated by an arbitrary buffer (such as the 4KB
buffer previously used). Thus, the buffer has been removed from the
fake-splitting implementation: Now, the hot and cold sections are
placed contiguously in memory, but the JIT continues to behave as if
they are arbitrarily far away (for example, by using long branches
between sections).

Following this fix, fake-splitting no longer requires the GC to be
suppressed by setting `COMPlus_GCgen0size=1000000`. A test job
has been added to `runtime-jit-experimental` to ensure
fake/stress-splitting does not regress.

eng/pipelines/common/templates/runtimes/run-test-job.yml
src/coreclr/jit/compiler.h
src/coreclr/jit/ee_il_dll.cpp
src/coreclr/jit/emit.cpp
src/coreclr/jit/jitconfigvalues.h
src/coreclr/jit/unwindamd64.cpp
src/coreclr/jit/unwindx86.cpp
src/tests/Common/testenvironment.proj

index f7f310d..9e75af0 100644 (file)
@@ -572,6 +572,7 @@ jobs:
           - jitosr_stress
           - jitosr_pgo
           - jitosr_stress_random
+          - jit_stress_splitting
           - jitpartialcompilation
           - jitpartialcompilation_osr
           - jitpartialcompilation_osr_pgo
index 7779d45..13a7791 100644 (file)
@@ -8000,6 +8000,10 @@ private:
     void unwindReserveFuncHelper(FuncInfoDsc* func, bool isHotCode);
     void unwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode, void* pColdCode, bool isHotCode);
 
+#ifdef DEBUG
+    void fakeUnwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode);
+#endif // DEBUG
+
 #endif // TARGET_AMD64 || (TARGET_X86 && FEATURE_EH_FUNCLETS)
 
     UNATIVE_OFFSET unwindGetCurrentOffset(FuncInfoDsc* func);
index c7ac7b3..f8c437e 100644 (file)
@@ -1125,14 +1125,13 @@ void Compiler::eeDispLineInfos()
 void Compiler::eeAllocMem(AllocMemArgs* args)
 {
 #ifdef DEBUG
-    // Fake splitting implementation: hot section = hot code + 4K buffer + cold code
-    const UNATIVE_OFFSET hotSizeRequest      = args->hotCodeSize;
-    const UNATIVE_OFFSET coldSizeRequest     = args->coldCodeSize;
-    const UNATIVE_OFFSET fakeSplittingBuffer = 4096;
+    const UNATIVE_OFFSET hotSizeRequest  = args->hotCodeSize;
+    const UNATIVE_OFFSET coldSizeRequest = args->coldCodeSize;
 
+    // Fake splitting implementation: place hot/cold code in contiguous section
     if (JitConfig.JitFakeProcedureSplitting() && (coldSizeRequest > 0))
     {
-        args->hotCodeSize  = hotSizeRequest + fakeSplittingBuffer + coldSizeRequest;
+        args->hotCodeSize  = hotSizeRequest + coldSizeRequest;
         args->coldCodeSize = 0;
     }
 #endif
@@ -1143,8 +1142,8 @@ void Compiler::eeAllocMem(AllocMemArgs* args)
     if (JitConfig.JitFakeProcedureSplitting() && (coldSizeRequest > 0))
     {
         // Fix up hot/cold code pointers
-        args->coldCodeBlock   = ((BYTE*)args->hotCodeBlock) + hotSizeRequest + fakeSplittingBuffer;
-        args->coldCodeBlockRW = ((BYTE*)args->hotCodeBlockRW) + hotSizeRequest + fakeSplittingBuffer;
+        args->coldCodeBlock   = ((BYTE*)args->hotCodeBlock) + hotSizeRequest;
+        args->coldCodeBlockRW = ((BYTE*)args->hotCodeBlockRW) + hotSizeRequest;
 
         // Reset args' hot/cold code sizes in case caller reads them later
         args->hotCodeSize  = hotSizeRequest;
@@ -1161,13 +1160,6 @@ void Compiler::eeReserveUnwindInfo(bool isFunclet, bool isColdCode, ULONG unwind
         printf("reserveUnwindInfo(isFunclet=%s, isColdCode=%s, unwindSize=0x%x)\n", isFunclet ? "true" : "false",
                isColdCode ? "true" : "false", unwindSize);
     }
-
-    // Fake splitting currently does not handle unwind info for cold code
-    if (isColdCode && JitConfig.JitFakeProcedureSplitting())
-    {
-        JITDUMP("reserveUnwindInfo for cold code with JitFakeProcedureSplitting enabled: ignoring cold unwind info\n");
-        return;
-    }
 #endif // DEBUG
 
     if (info.compMatchedVM)
@@ -1207,13 +1199,6 @@ void Compiler::eeAllocUnwindInfo(BYTE*          pHotCode,
         }
         printf(")\n");
     }
-
-    // Fake splitting currently does not handle unwind info for cold code
-    if (pColdCode && JitConfig.JitFakeProcedureSplitting())
-    {
-        JITDUMP("allocUnwindInfo for cold code with JitFakeProcedureSplitting enabled: ignoring cold unwind info\n");
-        return;
-    }
 #endif // DEBUG
 
     if (info.compMatchedVM)
index cd309ea..b9d0130 100644 (file)
@@ -6638,10 +6638,13 @@ unsigned emitter::emitEndCodeGen(Compiler* comp,
             {
                 // The allocated chunk is bigger than used, fill in unused space in it.
                 unsigned unusedSize = allocatedHotCodeSize - emitCurCodeOffs(cp);
+                BYTE*    cpRW       = cp + writeableOffset;
                 for (unsigned i = 0; i < unusedSize; ++i)
                 {
-                    *cp++ = DEFAULT_CODE_BUFFER_INIT;
+                    *cpRW++ = DEFAULT_CODE_BUFFER_INIT;
                 }
+
+                cp = cpRW - writeableOffset;
                 assert(allocatedHotCodeSize == emitCurCodeOffs(cp));
             }
         }
index 29e977c..dbf4b60 100644 (file)
@@ -196,11 +196,6 @@ CONFIG_INTEGER(JitDumpInlinePhases, W("JitDumpInlinePhases"), 1) // Dump inline
 CONFIG_METHODSET(JitEHDump, W("JitEHDump")) // Dump the EH table for the method, as reported to the VM
 CONFIG_METHODSET(JitExclude, W("JitExclude"))
 CONFIG_INTEGER(JitFakeProcedureSplitting, W("JitFakeProcedureSplitting"), 0) // Do code splitting independent of VM.
-                                                                             // For now, this disables unwind info for
-                                                                             // cold sections, breaking stack walks.
-                                                                             // Set COMPlus_GCgen0size=1000000 to avoid
-                                                                             // running the GC, which requires
-                                                                             // stack-walking.
 CONFIG_METHODSET(JitForceProcedureSplitting, W("JitForceProcedureSplitting"))
 CONFIG_METHODSET(JitGCDump, W("JitGCDump"))
 CONFIG_METHODSET(JitDebugDump, W("JitDebugDump"))
index caaf7c2..2c8e90f 100644 (file)
@@ -656,11 +656,21 @@ void Compiler::unwindReserve()
 //
 void Compiler::unwindReserveFunc(FuncInfoDsc* func)
 {
-    unwindReserveFuncHelper(func, true);
-
-    if (fgFirstColdBlock != nullptr)
+#ifdef DEBUG
+    if (JitConfig.JitFakeProcedureSplitting() && (fgFirstColdBlock != nullptr))
     {
-        unwindReserveFuncHelper(func, false);
+        assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.
+        unwindReserveFuncHelper(func, true);
+    }
+    else
+#endif // DEBUG
+    {
+        unwindReserveFuncHelper(func, true);
+
+        if (fgFirstColdBlock != nullptr)
+        {
+            unwindReserveFuncHelper(func, false);
+        }
     }
 }
 
@@ -880,12 +890,43 @@ void Compiler::unwindEmitFunc(FuncInfoDsc* func, void* pHotCode, void* pColdCode
     static_assert_no_msg(FUNC_HANDLER == (FuncKind)CORJIT_FUNC_HANDLER);
     static_assert_no_msg(FUNC_FILTER == (FuncKind)CORJIT_FUNC_FILTER);
 
-    unwindEmitFuncHelper(func, pHotCode, pColdCode, true);
+#ifdef DEBUG
+    if (JitConfig.JitFakeProcedureSplitting() && (pColdCode != nullptr))
+    {
+        fakeUnwindEmitFuncHelper(func, pHotCode);
+    }
+    else
+#endif // DEBUG
+    {
+        unwindEmitFuncHelper(func, pHotCode, pColdCode, true);
+
+        if (pColdCode != nullptr)
+        {
+            unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
+        }
+    }
+}
+
+#ifdef DEBUG
+void Compiler::fakeUnwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode)
+{
+    assert(fgFirstColdBlock != nullptr);
+    assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.
 
-    if (pColdCode != nullptr)
+    const UNATIVE_OFFSET startOffset     = 0;
+    const UNATIVE_OFFSET endOffset       = info.compNativeCodeSize;
+    const DWORD          unwindCodeBytes = sizeof(func->unwindCodes) - func->unwindCodeSlot;
+    BYTE*                pUnwindBlock    = &func->unwindCodes[func->unwindCodeSlot];
+
+    if (opts.dspUnwind)
     {
-        unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
+        DumpUnwindInfo(true, startOffset, endOffset, (const UNWIND_INFO* const)pUnwindBlock);
     }
+
+    // Pass pColdCode = nullptr; VM allocs unwind info for combined hot/cold section
+    eeAllocUnwindInfo((BYTE*)pHotCode, nullptr, startOffset, endOffset, unwindCodeBytes, pUnwindBlock,
+                      (CorJitFuncKind)func->funKind);
 }
+#endif // DEBUG
 
 #endif // TARGET_AMD64
index 0cd88fd..bd27e46 100644 (file)
@@ -113,11 +113,21 @@ void Compiler::unwindEmit(void* pHotCode, void* pColdCode)
 //
 void Compiler::unwindReserveFunc(FuncInfoDsc* func)
 {
-    unwindReserveFuncHelper(func, true);
-
-    if (fgFirstColdBlock != nullptr)
+#ifdef DEBUG
+    if (JitConfig.JitFakeProcedureSplitting() && (fgFirstColdBlock != nullptr))
     {
-        unwindReserveFuncHelper(func, false);
+        assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.
+        unwindReserveFuncHelper(func, true);
+    }
+    else
+#endif // DEBUG
+    {
+        unwindReserveFuncHelper(func, true);
+
+        if (fgFirstColdBlock != nullptr)
+        {
+            unwindReserveFuncHelper(func, false);
+        }
     }
 }
 
@@ -154,11 +164,20 @@ void Compiler::unwindEmitFunc(FuncInfoDsc* func, void* pHotCode, void* pColdCode
     static_assert_no_msg(FUNC_HANDLER == (FuncKind)CORJIT_FUNC_HANDLER);
     static_assert_no_msg(FUNC_FILTER == (FuncKind)CORJIT_FUNC_FILTER);
 
-    unwindEmitFuncHelper(func, pHotCode, pColdCode, true);
-
-    if (pColdCode != nullptr)
+#ifdef DEBUG
+    if (JitConfig.JitFakeProcedureSplitting() && (pColdCode != nullptr))
+    {
+        fakeUnwindEmitFuncHelper(func, pHotCode);
+    }
+    else
+#endif // DEBUG
     {
-        unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
+        unwindEmitFuncHelper(func, pHotCode, pColdCode, true);
+
+        if (pColdCode != nullptr)
+        {
+            unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
+        }
     }
 }
 
@@ -256,4 +275,23 @@ void Compiler::unwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode, void* pCo
     eeAllocUnwindInfo((BYTE*)pHotCode, (BYTE*)pColdCode, startOffset, endOffset, sizeof(UNWIND_INFO),
                       (BYTE*)&unwindInfo, (CorJitFuncKind)func->funKind);
 }
+
+#ifdef DEBUG
+void Compiler::fakeUnwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode)
+{
+    assert(fgFirstColdBlock != nullptr);
+    assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.
+
+    const UNATIVE_OFFSET startOffset = 0;
+    const UNATIVE_OFFSET endOffset   = info.compNativeCodeSize;
+
+    UNWIND_INFO unwindInfo;
+    unwindInfo.FunctionLength = (ULONG)(endOffset);
+
+    // Pass pColdCode = nullptr; VM allocs unwind info for combined hot/cold section
+    eeAllocUnwindInfo((BYTE*)pHotCode, nullptr, startOffset, endOffset, sizeof(UNWIND_INFO), (BYTE*)&unwindInfo,
+                      (CorJitFuncKind)func->funKind);
+}
+#endif // DEBUG
+
 #endif // FEATURE_EH_FUNCLETS
index fcc3e39..4cc6216 100644 (file)
@@ -42,7 +42,9 @@
       COMPlus_HeapVerify;
       COMPlus_JITMinOpts;
       COMPlus_JitELTHookEnabled;
+      COMPlus_JitFakeProcedureSplitting;
       COMPlus_JitStress;
+      COMPlus_JitStressProcedureSplitting;
       COMPlus_JitStressRegs;
       COMPlus_TailcallStress;
       COMPlus_ReadyToRun;
     <TestEnvironment Include="jitosr" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TieredCompilation="1" />
     <TestEnvironment Include="jitosr_stress" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TC_OnStackReplacement_InitialCounter="1" OSR_HitLimit="1" TieredCompilation="1" />
     <TestEnvironment Include="jitosr_stress_random" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TC_OnStackReplacement_InitialCounter="1" OSR_HitLimit="2" TieredCompilation="1" JitRandomOnStackReplacement="15"/>
+    <TestEnvironment Include="jit_stress_splitting" JitFakeProcedureSplitting="1" JitStressProcedureSplitting="1" />
     <TestEnvironment Include="jitosr_pgo" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TieredCompilation="1" TieredPGO="1" />
     <TestEnvironment Include="jitpartialcompilation" TC_PartialCompilation="1" TC_QuickJitForLoops="1" TieredCompilation="1" />
     <TestEnvironment Include="jitpartialcompilation_pgo" TC_PartialCompilation="1" TC_QuickJitForLoops="1" TieredCompilation="1" TieredPGO="1" />