Fix SuperPMI SEH exception handling (#48447)
authorBruce Forstall <brucefo@microsoft.com>
Thu, 18 Feb 2021 19:13:18 +0000 (09:13 -1000)
committerGitHub <noreply@github.com>
Thu, 18 Feb 2021 19:13:18 +0000 (11:13 -0800)
Capture data in the filter; don't save a PEXCEPTION_POINTERS
that points to data that is is transient to the filter.

src/coreclr/ToolBox/superpmi/superpmi-shared/errorhandling.cpp
src/coreclr/ToolBox/superpmi/superpmi-shared/errorhandling.h
src/coreclr/ToolBox/superpmi/superpmi/jitinstance.cpp
src/coreclr/ToolBox/superpmi/superpmi/superpmi.cpp

index bb3d847..6b11771 100644 (file)
@@ -29,14 +29,8 @@ void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode, const ch
     ThrowException(exceptionCode, ap, msg);
 }
 
-SpmiException::SpmiException(PEXCEPTION_POINTERS exp) : exCode(exp->ExceptionRecord->ExceptionCode)
-{
-    exMessage =
-        (exp->ExceptionRecord->NumberParameters != 1) ? nullptr : (char*)exp->ExceptionRecord->ExceptionInformation[0];
-}
-
-SpmiException::SpmiException(DWORD exceptionCode, char* exceptionMessage)
-    : exCode(exceptionCode), exMessage(exceptionMessage)
+SpmiException::SpmiException(FilterSuperPMIExceptionsParam_CaptureException* e)
+    : exCode(e->exceptionCode), exMessage(e->exceptionMessage)
 {
 }
 
@@ -58,13 +52,13 @@ void SpmiException::ShowAndDeleteMessage()
     if (exMessage != nullptr)
     {
         LogError("Exception thrown: %s", exMessage);
-        delete[] exMessage;
-        exMessage = nullptr;
     }
     else
     {
         LogError("Unexpected exception was thrown.");
     }
+
+    DeleteMessage();
 }
 
 void SpmiException::DeleteMessage()
@@ -88,19 +82,15 @@ LONG FilterSuperPMIExceptions_CatchMC(PEXCEPTION_POINTERS pExceptionPointers, LP
 // This filter function captures the exception pointers and continues searching.
 LONG FilterSuperPMIExceptions_CaptureExceptionAndContinue(PEXCEPTION_POINTERS pExceptionPointers, LPVOID lpvParam)
 {
-    FilterSuperPMIExceptionsParam_CaptureException* pSPMIEParam =
-        (FilterSuperPMIExceptionsParam_CaptureException*)lpvParam;
-    pSPMIEParam->exceptionPointers = *pExceptionPointers; // Capture the exception pointers for use later
-    pSPMIEParam->exceptionCode     = pSPMIEParam->exceptionPointers.ExceptionRecord->ExceptionCode;
+    FilterSuperPMIExceptionsParam_CaptureException* pSPMIEParam = (FilterSuperPMIExceptionsParam_CaptureException*)lpvParam;
+    pSPMIEParam->Initialize(pExceptionPointers);
     return EXCEPTION_CONTINUE_SEARCH;
 }
 
 LONG FilterSuperPMIExceptions_CaptureExceptionAndStop(PEXCEPTION_POINTERS pExceptionPointers, LPVOID lpvParam)
 {
-    FilterSuperPMIExceptionsParam_CaptureException* pSPMIEParam =
-        (FilterSuperPMIExceptionsParam_CaptureException*)lpvParam;
-    pSPMIEParam->exceptionPointers = *pExceptionPointers; // Capture the exception pointers for use later
-    pSPMIEParam->exceptionCode     = pSPMIEParam->exceptionPointers.ExceptionRecord->ExceptionCode;
+    FilterSuperPMIExceptionsParam_CaptureException* pSPMIEParam = (FilterSuperPMIExceptionsParam_CaptureException*)lpvParam;
+    pSPMIEParam->Initialize(pExceptionPointers);
     return EXCEPTION_EXECUTE_HANDLER;
 }
 
index 65919d9..506218f 100644 (file)
@@ -43,26 +43,6 @@ void MSC_ONLY(__declspec(noreturn)) ThrowException(DWORD exceptionCode, const ch
 #define AssertMsg(expr, msg, ...) AssertCodeMsg(expr, EXCEPTIONCODE_ASSERT, msg, ##__VA_ARGS__)
 #define Assert(expr) AssertCode(expr, EXCEPTIONCODE_ASSERT)
 
-class SpmiException
-{
-private:
-    DWORD exCode;
-    char* exMessage;
-
-public:
-    SpmiException(PEXCEPTION_POINTERS exp);
-    SpmiException(DWORD exceptionCode, char* exceptionMessage);
-#if 0
-    ~SpmiException();
-#endif
-
-    char* GetExceptionMessage();
-    DWORD GetCode();
-
-    void ShowAndDeleteMessage();
-    void DeleteMessage();
-};
-
 //
 // Functions and types used by PAL_TRY-related macros.
 //
@@ -71,13 +51,22 @@ extern LONG FilterSuperPMIExceptions_CatchMC(PEXCEPTION_POINTERS pExceptionPoint
 
 struct FilterSuperPMIExceptionsParam_CaptureException
 {
-    EXCEPTION_POINTERS exceptionPointers;
-    DWORD              exceptionCode;
+    DWORD exceptionCode;
+    char* exceptionMessage; // 'new' memory passed from ThrowException()
 
-    FilterSuperPMIExceptionsParam_CaptureException() : exceptionCode(0)
+    FilterSuperPMIExceptionsParam_CaptureException()
+        : exceptionCode(0)
+        , exceptionMessage(nullptr)
     {
-        exceptionPointers.ExceptionRecord = nullptr;
-        exceptionPointers.ContextRecord   = nullptr;
+    }
+
+    // Note: this is called during an SEH filter; the data pointed to by PEXCEPTION_POINTERS is not valid after
+    // calling this function, so anything we want to safe must be copied.
+    // The exception message string is 'new' memory, allocated in the ThrowException() function.
+    void Initialize(PEXCEPTION_POINTERS pExceptionPointers)
+    {
+        exceptionCode    = pExceptionPointers->ExceptionRecord->ExceptionCode;
+        exceptionMessage = (pExceptionPointers->ExceptionRecord->NumberParameters != 1) ? nullptr : (char*)pExceptionPointers->ExceptionRecord->ExceptionInformation[0];
     }
 };
 
@@ -87,4 +76,23 @@ extern LONG FilterSuperPMIExceptions_CaptureExceptionAndStop(PEXCEPTION_POINTERS
 
 extern bool RunWithErrorTrap(void (*function)(void*), void* param);
 
+class SpmiException
+{
+private:
+    DWORD exCode;
+    char* exMessage;
+
+public:
+    SpmiException(FilterSuperPMIExceptionsParam_CaptureException* e);
+#if 0
+    ~SpmiException();
+#endif
+
+    char* GetExceptionMessage();
+    DWORD GetCode();
+
+    void ShowAndDeleteMessage();
+    void DeleteMessage();
+};
+
 #endif
index cd32e4d..dd04e5f 100644 (file)
@@ -350,9 +350,9 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i
     }
     PAL_EXCEPT_FILTER(FilterSuperPMIExceptions_CaptureExceptionAndStop)
     {
-        SpmiException e(&param.exceptionPointers);
+        SpmiException e(&param);
 
-        if (param.exceptionCode == EXCEPTIONCODE_MC) // Can't check e.GetCode() because of https://github.com/dotnet/runtime/issues/48356
+        if (e.GetCode() == EXCEPTIONCODE_MC)
         {
             char* message = e.GetExceptionMessage();
             LogMissing("Method context %d failed to replay: %s", mcIndex, message);
@@ -503,7 +503,7 @@ bool JitInstance::callJitStartup(ICorJitHost* jithost)
     }
     PAL_EXCEPT_FILTER(FilterSuperPMIExceptions_CaptureExceptionAndStop)
     {
-        SpmiException e(&param.exceptionPointers);
+        SpmiException e(&param);
 
         LogError("failed to call jitStartup.");
         e.ShowAndDeleteMessage();
index 1e7f889..a2d9b2d 100644 (file)
@@ -128,7 +128,7 @@ void InvokeNearDiffer(NearDiffer*           nearDiffer,
     }
     PAL_EXCEPT_FILTER(FilterSuperPMIExceptions_CaptureExceptionAndStop)
     {
-        SpmiException e(&param.exceptionPointers);
+        SpmiException e(&param);
 
         LogError("main method %d of size %d failed to load and compile correctly.", (*reader)->GetMethodContextIndex(),
                  (*mc)->methodSize);