SPMI: Throw distinguished exception for recorded exceptions (#89978)
authorJakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
Mon, 7 Aug 2023 21:02:32 +0000 (23:02 +0200)
committerGitHub <noreply@github.com>
Mon, 7 Aug 2023 21:02:32 +0000 (23:02 +0200)
Switch back to this scheme, since only looking for the managed exception
code does not handle crossgen2/ilc (they throw exceptions across the
JIT-EE interface as normal C++ exceptions).

I've verified that the JIT does not look closely at the exception codes
thrown in any of the places it does EH, which comes down to
`impJitErrorTrap` and `runWithErrorTrap`. So changing the exception code
should not result in any behavior differences.

src/coreclr/tools/superpmi/superpmi-shared/errorhandling.cpp
src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h
src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp
src/coreclr/tools/superpmi/superpmi/jitinstance.cpp

index 9b91835..ceba2ef 100644 (file)
@@ -7,14 +7,6 @@
 #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)
 {
@@ -38,6 +30,25 @@ 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.
+// Note that we rely on the fact that the JIT do not look closely at exceptions thrown by the EE;
+// otherwise this could cause issues since it changes the exception code from what was actually
+// thrown.
+//
+// However, do note that crossgen2/ilc and the VM already do not use the same exception code when
+// throwing exceptions.
+//
+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)
 {
index 978a40c..3c418b3 100644 (file)
@@ -8,7 +8,6 @@
 #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
 #define EXCEPTIONCODE_LWM 0xe0423000
 #define EXCEPTIONCODE_CALLUTILS 0xe0426000
 #define EXCEPTIONCODE_TYPEUTILS 0xe0427000
+// Special exception code for rethrowing a recorded exception. This is NOT
+// considered an SPMI exception (see IsSuperPMIException), but it also does not
+// result in a replay failure (see JitInstance::CompileMethod).
+#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, ...)                                                                          \
index 561f245..af7be09 100644 (file)
@@ -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
@@ -1060,7 +1060,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;
 }
 
@@ -1082,7 +1082,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;
 }
 
@@ -1364,7 +1364,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
index 426d9cc..f8c81c4 100644 (file)
@@ -435,15 +435,13 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i
             e.DeleteMessage();
             param.result = RESULT_MISSING;
         }
-        else if (e.GetCode() == EXCEPTIONCODE_COMPLUS)
+        else if (e.GetCode() == EXCEPTIONCODE_RECORDED_EXCEPTION)
         {
-            // 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.
+            // Exception thrown by EE during recording, for example a managed
+            // MissingFieldException thrown by resolveToken. Several JIT-EE
+            // APIs can throw exceptions and the recorder expects and rethrows
+            // their exceptions under this exception code. We do not consider
+            // these a replay failure.
 
             // Call these methods to capture that no code/GC info was generated.
             mc->cr->recAllocMemCapture();