From ffb0ee274d65c11f9056555d5f93ffbe187c7a74 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 2 Jun 2021 10:54:24 +0200 Subject: [PATCH 1/2] Fix JIT debugger notification location My recent change to enable jitting into a scratch buffer has broken debugging due to the notification to debugger being sent too early. This change moves the notification after the call to WriteCode and also removes the CallCompileMethodWithSEHWrapper completely, since its only purpose was to call the notification in PAL_FINALLY and only when the JITting succeeded. --- src/coreclr/vm/jitinterface.cpp | 121 +++++++++----------------------- 1 file changed, 33 insertions(+), 88 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 44dfe8fe73b28..8c82941a06a93 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12734,83 +12734,6 @@ CorJitResult invokeCompileMethod(EEJitManager *jitMgr, return ret; } -CorJitResult CallCompileMethodWithSEHWrapper(EEJitManager *jitMgr, - CEEInfo *comp, - struct CORINFO_METHOD_INFO *info, - CORJIT_FLAGS flags, - BYTE **nativeEntry, - uint32_t *nativeSizeOfCode, - NativeCodeVersion nativeCodeVersion) -{ - // no dynamic contract here because SEH is used, with a finally clause - STATIC_CONTRACT_NOTHROW; - STATIC_CONTRACT_GC_TRIGGERS; - - MethodDesc* ftn = nativeCodeVersion.GetMethodDesc(); - - LOG((LF_CORDB, LL_EVERYTHING, "CallCompileMethodWithSEHWrapper called...\n")); - - struct Param - { - EEJitManager *jitMgr; - CEEInfo *comp; - struct CORINFO_METHOD_INFO *info; - CORJIT_FLAGS flags; - BYTE **nativeEntry; - uint32_t *nativeSizeOfCode; - MethodDesc *ftn; - CorJitResult res; - }; Param param; - param.jitMgr = jitMgr; - param.comp = comp; - param.info = info; - param.flags = flags; - param.nativeEntry = nativeEntry; - param.nativeSizeOfCode = nativeSizeOfCode; - param.ftn = ftn; - param.res = CORJIT_INTERNALERROR; - - PAL_TRY(Param *, pParam, ¶m) - { - // - // Call out to the JIT-compiler - // - - pParam->res = invokeCompileMethod( pParam->jitMgr, - pParam->comp, - pParam->info, - pParam->flags, - pParam->nativeEntry, - pParam->nativeSizeOfCode); - } - PAL_FINALLY - { -#if defined(DEBUGGING_SUPPORTED) && !defined(CROSSGEN_COMPILE) - if (!flags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_IMPORT_ONLY) && - !flags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MCJIT_BACKGROUND) -#ifdef FEATURE_STACK_SAMPLING - && !flags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_SAMPLING_JIT_BACKGROUND) -#endif // FEATURE_STACK_SAMPLING - ) - { - // - // Notify the debugger that we have successfully jitted the function - // - if (g_pDebugInterface) - { - if (param.res == CORJIT_OK && !((CEEJitInfo*)param.comp)->JitAgain()) - { - g_pDebugInterface->JITComplete(nativeCodeVersion, (TADDR)*nativeEntry); - } - } - } -#endif // DEBUGGING_SUPPORTED && !CROSSGEN_COMPILE - } - PAL_ENDTRY - - return param.res; -} - /*********************************************************************/ // Figures out the compile flags that are used by both JIT and NGen @@ -13309,17 +13232,16 @@ PCODE UnsafeJitFunction(PrepareCodeConfig* config, QueryPerformanceCounter (&methodJitTimeStart); #endif - // Note on debuggerTrackInfo arg: if we're only importing (ie, verifying/ - // checking to make sure we could JIT, but not actually generating code ( - // eg, for inlining), then DON'T TELL THE DEBUGGER about this. - res = CallCompileMethodWithSEHWrapper(jitMgr, - &jitInfo, - &methodInfo, - flags, - &nativeEntry, - &sizeOfCode, - nativeCodeVersion); - LOG((LF_CORDB, LL_EVERYTHING, "Got through CallCompile MethodWithSEHWrapper\n")); + LOG((LF_CORDB, LL_EVERYTHING, "Calling invokeCompileMethod...\n")); + + res = invokeCompileMethod(jitMgr, + &jitInfo, + &methodInfo, + flags, + &nativeEntry, + &sizeOfCode); + + LOG((LF_CORDB, LL_EVERYTHING, "Got through invokeCompileMethod\n")); #if FEATURE_PERFMAP // Save the code size so that it can be reported to the perfmap. @@ -13355,6 +13277,29 @@ PCODE UnsafeJitFunction(PrepareCodeConfig* config, if (SUCCEEDED(res)) { jitInfo.WriteCode(jitMgr); +#if defined(DEBUGGING_SUPPORTED) + // Note: if we're only importing (ie, verifying/ + // checking to make sure we could JIT, but not actually generating code ( + // eg, for inlining), then DON'T TELL THE DEBUGGER about this. + if (!flags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_IMPORT_ONLY) && + !flags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MCJIT_BACKGROUND) +#ifdef FEATURE_STACK_SAMPLING + && !flags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_SAMPLING_JIT_BACKGROUND) +#endif // FEATURE_STACK_SAMPLING + ) + { + // + // Notify the debugger that we have successfully jitted the function + // + if (g_pDebugInterface) + { + if (!jitInfo.JitAgain()) + { + g_pDebugInterface->JITComplete(nativeCodeVersion, (TADDR)nativeEntry); + } + } + } +#endif // DEBUGGING_SUPPORTED && !CROSSGEN_COMPILE } else { From ceda64ff8ad670c370b2a8095257e3447c6f37f9 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 2 Jun 2021 17:36:04 +0200 Subject: [PATCH 2/2] Fix #endif comment Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 8c82941a06a93..02fae7f99f2bb 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -13299,7 +13299,7 @@ PCODE UnsafeJitFunction(PrepareCodeConfig* config, } } } -#endif // DEBUGGING_SUPPORTED && !CROSSGEN_COMPILE +#endif // DEBUGGING_SUPPORTED } else {