From fd2c7d3932c7db798ba6dc0ac8aba5500524ef48 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 28 Jul 2023 23:03:49 +0200 Subject: [PATCH 1/2] SPMI: Gracefully handle recorded EE-side exceptions during replay Various JIT-EE APIs are expected to be able to throw exceptions, which SPMI will record and rethrow during replay. This change rethrows these exceptions using a known exception code so that we can gracefully handle these exceptions during replay; this makes us no longer consider these as replay failures. Also do a small amount of cleanup for the EH in SPMI. --- .../superpmi-shared/errorhandling.cpp | 31 ++++++++++++------- .../superpmi/superpmi-shared/errorhandling.h | 5 +-- .../tools/superpmi/superpmi-shared/logging.h | 2 +- .../tools/superpmi/superpmi/icorjitinfo.cpp | 12 +++---- .../tools/superpmi/superpmi/jitinstance.cpp | 16 +++++++++- 5 files changed, 45 insertions(+), 21 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp index b051ffdb32a29..058837c86ae0d 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp @@ -7,30 +7,39 @@ #include "runtimedetails.h" #include "spmiutil.h" -void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode) -{ - RaiseException(exceptionCode, 0, 0, nullptr); -} - // Allocating memory here seems moderately dangerous: we'll probably leak like a sieve... -void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode, va_list args, const char* message) +void MSC_ONLY(__declspec(noreturn)) ThrowSpmiException(DWORD exceptionCode, va_list args, const char* message) { + assert(IsSuperPMIException(exceptionCode)); + char* buffer = new char[8192]; - ULONG_PTR* ptr = new ULONG_PTR(); - *ptr = (ULONG_PTR)buffer; _vsnprintf_s(buffer, 8192, 8191, message, args); if (BreakOnException()) __debugbreak(); - RaiseException(exceptionCode, 0, 1, ptr); + ULONG_PTR exArgs[1]; + exArgs[0] = (ULONG_PTR)buffer; + RaiseException(exceptionCode, 0, ArrLen(exArgs), exArgs); } -void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode, const char* msg, ...) +void MSC_ONLY(__declspec(noreturn)) ThrowSpmiException(DWORD exceptionCode, const char* msg, ...) { va_list ap; va_start(ap, msg); - ThrowException(exceptionCode, ap, msg); + ThrowSpmiException(exceptionCode, ap, msg); +} + +// Throw an exception that indicates that the EE side threw an exception during recording. +// These exceptions do not result in replay errors; see JitInstance::CompileMethod. +void MSC_ONLY(__declspec(noreturn)) ThrowRecordedException(DWORD innerExceptionCode) +{ + if (BreakOnException()) + __debugbreak(); + + ULONG_PTR args[1]; + args[0] = (ULONG_PTR)innerExceptionCode; + RaiseException(EXCEPTIONCODE_RECORDED_EXCEPTION, 0, ArrLen(args), args); } SpmiException::SpmiException(FilterSuperPMIExceptionsParam_CaptureException* e) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h index 0d9aca331fb49..3f34d9743ba38 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h @@ -19,11 +19,12 @@ #define EXCEPTIONCODE_LWM 0xe0423000 #define EXCEPTIONCODE_CALLUTILS 0xe0426000 #define EXCEPTIONCODE_TYPEUTILS 0xe0427000 +#define EXCEPTIONCODE_RECORDED_EXCEPTION 0xe0428000 #define EXCEPTIONCODE_ASSERT 0xe0440000 // RaiseException wrappers -void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode); -void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode, const char* message, ...); +void MSC_ONLY(__declspec(noreturn)) ThrowSpmiException(DWORD exceptionCode, const char* message, ...); +void MSC_ONLY(__declspec(noreturn)) ThrowRecordedException(DWORD innerExceptionCode); // Assert stuff #define AssertCodeMsg(expr, exCode, msg, ...) \ diff --git a/src/coreclr/tools/superpmi/superpmi-shared/logging.h b/src/coreclr/tools/superpmi/superpmi-shared/logging.h index b8afe63e312c3..957da040885ea 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/logging.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/logging.h @@ -30,7 +30,7 @@ do \ { \ Logger::LogExceptionMessage(__FUNCTION__, __FILE__, __LINE__, exCode, msg, __VA_ARGS__); \ - ThrowException(exCode, msg, __VA_ARGS__); \ + ThrowSpmiException(exCode, msg, __VA_ARGS__); \ } while (0) // These are specified as flags so subsets of the logging functionality can be enabled/disabled at once diff --git a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp index f8f84b24cd6cd..6f34204924b8a 100644 --- a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp @@ -77,7 +77,7 @@ bool MyICJI::getMethodInfo(CORINFO_METHOD_HANDLE ftn, /* IN */ DWORD exceptionCode = 0; bool value = jitInstance->mc->repGetMethodInfo(ftn, info, context, &exceptionCode); if (exceptionCode != 0) - ThrowException(exceptionCode); + ThrowRecordedException(exceptionCode); return value; } @@ -97,7 +97,7 @@ CorInfoInline MyICJI::canInline(CORINFO_METHOD_HANDLE callerHnd, /* IN */ DWORD exceptionCode = 0; CorInfoInline result = jitInstance->mc->repCanInline(callerHnd, calleeHnd, &exceptionCode); if (exceptionCode != 0) - ThrowException(exceptionCode); + ThrowRecordedException(exceptionCode); return result; } @@ -298,7 +298,7 @@ void MyICJI::resolveToken(/* IN, OUT */ CORINFO_RESOLVED_TOKEN* pResolvedToken) jitInstance->mc->cr->AddCall("resolveToken"); jitInstance->mc->repResolveToken(pResolvedToken, &exceptionCode); if (exceptionCode != 0) - ThrowException(exceptionCode); + ThrowRecordedException(exceptionCode); } // Signature information about the call sig @@ -1056,7 +1056,7 @@ CorInfoTypeWithMod MyICJI::getArgType(CORINFO_SIG_INFO* sig, /* IN */ jitInstance->mc->cr->AddCall("getArgType"); CorInfoTypeWithMod value = jitInstance->mc->repGetArgType(sig, args, vcTypeRet, &exceptionCode); if (exceptionCode != 0) - ThrowException(exceptionCode); + ThrowRecordedException(exceptionCode); return value; } @@ -1078,7 +1078,7 @@ CORINFO_CLASS_HANDLE MyICJI::getArgClass(CORINFO_SIG_INFO* sig, /* IN */ jitInstance->mc->cr->AddCall("getArgClass"); CORINFO_CLASS_HANDLE value = jitInstance->mc->repGetArgClass(sig, args, &exceptionCode); if (exceptionCode != 0) - ThrowException(exceptionCode); + ThrowRecordedException(exceptionCode); return value; } @@ -1360,7 +1360,7 @@ void MyICJI::getCallInfo( jitInstance->mc->repGetCallInfo(pResolvedToken, pConstrainedResolvedToken, callerHandle, flags, pResult, &exceptionCode); if (exceptionCode != 0) - ThrowException(exceptionCode); + ThrowRecordedException(exceptionCode); } // returns the class's domain ID for accessing shared statics diff --git a/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp b/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp index c2b50459b5c43..0d3e3f09d5085 100644 --- a/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp +++ b/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp @@ -414,7 +414,7 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i pParam->pThis->mc->cr->recAllocMemCapture(); pParam->pThis->mc->cr->recAllocGCInfoCapture(); - pParam->pThis->mc->cr->recMessageLog("Successful Compile"); + pParam->pThis->mc->cr->recMessageLog(jitResult == CORJIT_OK ? "Successful Compile" : "Successful Compile (BADCODE)"); pParam->metrics->NumCodeBytes += NCodeSizeBlock; } @@ -435,6 +435,20 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i e.DeleteMessage(); param.result = RESULT_MISSING; } + else if (e.GetCode() == EXCEPTIONCODE_RECORDED_EXCEPTION) + { + // Exception thrown by EE during recording, for example a managed + // MissingFieldException thrown by resolveToken. Several JIT-EE + // APIs can throw this and the recorder expects and rethrows their + // exceptions faithfully. We do not consider these a replay + // failure. + + // Call these methods to capture that no code/GC info was generated. + mc->cr->recAllocMemCapture(); + mc->cr->recAllocGCInfoCapture(); + + mc->cr->recMessageLog("Successful Compile (EE API exception)"); + } else { e.ShowAndDeleteMessage(); From c0c21f6ded732772c603ea726f4adb6c6a028db3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 29 Jul 2023 21:41:31 +0200 Subject: [PATCH 2/2] Revise --- .../superpmi-shared/errorhandling.cpp | 20 ++++++++----------- .../superpmi/superpmi-shared/errorhandling.h | 5 +++-- .../tools/superpmi/superpmi/icorjitinfo.cpp | 12 +++++------ .../tools/superpmi/superpmi/jitinstance.cpp | 14 +++++++------ 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp index 058837c86ae0d..9b918353c4b60 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp @@ -7,6 +7,14 @@ #include "runtimedetails.h" #include "spmiutil.h" +void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode) +{ + if (BreakOnException()) + __debugbreak(); + + RaiseException(exceptionCode, 0, 0, nullptr); +} + // Allocating memory here seems moderately dangerous: we'll probably leak like a sieve... void MSC_ONLY(__declspec(noreturn)) ThrowSpmiException(DWORD exceptionCode, va_list args, const char* message) { @@ -30,18 +38,6 @@ void MSC_ONLY(__declspec(noreturn)) ThrowSpmiException(DWORD exceptionCode, cons ThrowSpmiException(exceptionCode, ap, msg); } -// Throw an exception that indicates that the EE side threw an exception during recording. -// These exceptions do not result in replay errors; see JitInstance::CompileMethod. -void MSC_ONLY(__declspec(noreturn)) ThrowRecordedException(DWORD innerExceptionCode) -{ - if (BreakOnException()) - __debugbreak(); - - ULONG_PTR args[1]; - args[0] = (ULONG_PTR)innerExceptionCode; - RaiseException(EXCEPTIONCODE_RECORDED_EXCEPTION, 0, ArrLen(args), args); -} - SpmiException::SpmiException(FilterSuperPMIExceptionsParam_CaptureException* e) : exCode(e->exceptionCode), exMessage(e->exceptionMessage) { diff --git a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h index 3f34d9743ba38..978a40c50a68e 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h @@ -8,6 +8,7 @@ #define _ErrorHandling #include "logging.h" +#include "corexcep.h" // EXCEPTIONCODE_DebugBreakorAV is just the base exception number; calls to DebugBreakorAV() // pass a unique number to add to this. EXCEPTIONCODE_DebugBreakorAV_MAX is the maximum number @@ -19,12 +20,12 @@ #define EXCEPTIONCODE_LWM 0xe0423000 #define EXCEPTIONCODE_CALLUTILS 0xe0426000 #define EXCEPTIONCODE_TYPEUTILS 0xe0427000 -#define EXCEPTIONCODE_RECORDED_EXCEPTION 0xe0428000 #define EXCEPTIONCODE_ASSERT 0xe0440000 +#define EXCEPTIONCODE_COMPLUS EXCEPTION_COMPLUS // RaiseException wrappers +void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode); void MSC_ONLY(__declspec(noreturn)) ThrowSpmiException(DWORD exceptionCode, const char* message, ...); -void MSC_ONLY(__declspec(noreturn)) ThrowRecordedException(DWORD innerExceptionCode); // Assert stuff #define AssertCodeMsg(expr, exCode, msg, ...) \ diff --git a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp index 6f34204924b8a..f8f84b24cd6cd 100644 --- a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp @@ -77,7 +77,7 @@ bool MyICJI::getMethodInfo(CORINFO_METHOD_HANDLE ftn, /* IN */ DWORD exceptionCode = 0; bool value = jitInstance->mc->repGetMethodInfo(ftn, info, context, &exceptionCode); if (exceptionCode != 0) - ThrowRecordedException(exceptionCode); + ThrowException(exceptionCode); return value; } @@ -97,7 +97,7 @@ CorInfoInline MyICJI::canInline(CORINFO_METHOD_HANDLE callerHnd, /* IN */ DWORD exceptionCode = 0; CorInfoInline result = jitInstance->mc->repCanInline(callerHnd, calleeHnd, &exceptionCode); if (exceptionCode != 0) - ThrowRecordedException(exceptionCode); + ThrowException(exceptionCode); return result; } @@ -298,7 +298,7 @@ void MyICJI::resolveToken(/* IN, OUT */ CORINFO_RESOLVED_TOKEN* pResolvedToken) jitInstance->mc->cr->AddCall("resolveToken"); jitInstance->mc->repResolveToken(pResolvedToken, &exceptionCode); if (exceptionCode != 0) - ThrowRecordedException(exceptionCode); + ThrowException(exceptionCode); } // Signature information about the call sig @@ -1056,7 +1056,7 @@ CorInfoTypeWithMod MyICJI::getArgType(CORINFO_SIG_INFO* sig, /* IN */ jitInstance->mc->cr->AddCall("getArgType"); CorInfoTypeWithMod value = jitInstance->mc->repGetArgType(sig, args, vcTypeRet, &exceptionCode); if (exceptionCode != 0) - ThrowRecordedException(exceptionCode); + ThrowException(exceptionCode); return value; } @@ -1078,7 +1078,7 @@ CORINFO_CLASS_HANDLE MyICJI::getArgClass(CORINFO_SIG_INFO* sig, /* IN */ jitInstance->mc->cr->AddCall("getArgClass"); CORINFO_CLASS_HANDLE value = jitInstance->mc->repGetArgClass(sig, args, &exceptionCode); if (exceptionCode != 0) - ThrowRecordedException(exceptionCode); + ThrowException(exceptionCode); return value; } @@ -1360,7 +1360,7 @@ void MyICJI::getCallInfo( jitInstance->mc->repGetCallInfo(pResolvedToken, pConstrainedResolvedToken, callerHandle, flags, pResult, &exceptionCode); if (exceptionCode != 0) - ThrowRecordedException(exceptionCode); + ThrowException(exceptionCode); } // returns the class's domain ID for accessing shared statics diff --git a/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp b/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp index 0d3e3f09d5085..426d9ccd6c0f1 100644 --- a/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp +++ b/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp @@ -435,13 +435,15 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i e.DeleteMessage(); param.result = RESULT_MISSING; } - else if (e.GetCode() == EXCEPTIONCODE_RECORDED_EXCEPTION) + else if (e.GetCode() == EXCEPTIONCODE_COMPLUS) { - // Exception thrown by EE during recording, for example a managed - // MissingFieldException thrown by resolveToken. Several JIT-EE - // APIs can throw this and the recorder expects and rethrows their - // exceptions faithfully. We do not consider these a replay - // failure. + // We assume that managed exceptions are never JIT bugs and were + // thrown by the EE during recording. Various EE APIs can throw + // managed exceptions and replay will faithfully rethrow these. The + // JIT itself will sometimes catch them (e.g. during inlining), but + // if they make it out of the JIT then we assume that they are not + // JIT bugs. The typical scenario is something like + // MissingFieldException thrown from resolveToken. // Call these methods to capture that no code/GC info was generated. mc->cr->recAllocMemCapture();