From e727fba91f40096313200b52d0fb20e9bdebfc47 Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Fri, 27 May 2022 14:11:31 -0700 Subject: [PATCH] Fix unwind info when fake-splitting 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. --- .../templates/runtimes/run-test-job.yml | 1 + src/coreclr/jit/compiler.h | 4 ++ src/coreclr/jit/ee_il_dll.cpp | 27 ++------- src/coreclr/jit/emit.cpp | 5 +- src/coreclr/jit/jitconfigvalues.h | 5 -- src/coreclr/jit/unwindamd64.cpp | 55 ++++++++++++++++--- src/coreclr/jit/unwindx86.cpp | 54 +++++++++++++++--- src/tests/Common/testenvironment.proj | 3 + 8 files changed, 112 insertions(+), 42 deletions(-) diff --git a/eng/pipelines/common/templates/runtimes/run-test-job.yml b/eng/pipelines/common/templates/runtimes/run-test-job.yml index f7f310dfb9742..9e75af0ac8f6b 100644 --- a/eng/pipelines/common/templates/runtimes/run-test-job.yml +++ b/eng/pipelines/common/templates/runtimes/run-test-job.yml @@ -572,6 +572,7 @@ jobs: - jitosr_stress - jitosr_pgo - jitosr_stress_random + - jit_stress_splitting - jitpartialcompilation - jitpartialcompilation_osr - jitpartialcompilation_osr_pgo diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b4cfe60a692d9..9656fed89dbfb 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7966,6 +7966,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX 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); diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index c7ac7b32e5e99..f8c437e326694 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -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) diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 10c5c096f8934..bd5dccdfabc74 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -6637,10 +6637,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)); } } diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 29e977cf1d8c9..dbf4b6069fc40 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -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")) diff --git a/src/coreclr/jit/unwindamd64.cpp b/src/coreclr/jit/unwindamd64.cpp index caaf7c2dbe29b..2c8e90fa5a944 100644 --- a/src/coreclr/jit/unwindamd64.cpp +++ b/src/coreclr/jit/unwindamd64.cpp @@ -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 diff --git a/src/coreclr/jit/unwindx86.cpp b/src/coreclr/jit/unwindx86.cpp index 0cd88fd29ba1e..bd27e46cbef49 100644 --- a/src/coreclr/jit/unwindx86.cpp +++ b/src/coreclr/jit/unwindx86.cpp @@ -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 diff --git a/src/tests/Common/testenvironment.proj b/src/tests/Common/testenvironment.proj index fcc3e3926febc..4cc62162bc2f2 100644 --- a/src/tests/Common/testenvironment.proj +++ b/src/tests/Common/testenvironment.proj @@ -42,7 +42,9 @@ COMPlus_HeapVerify; COMPlus_JITMinOpts; COMPlus_JitELTHookEnabled; + COMPlus_JitFakeProcedureSplitting; COMPlus_JitStress; + COMPlus_JitStressProcedureSplitting; COMPlus_JitStressRegs; COMPlus_TailcallStress; COMPlus_ReadyToRun; @@ -188,6 +190,7 @@ +