Protect SuperPMI from crashes calling jitStartup
authorBruce Forstall <brucefo@microsoft.com>
Thu, 12 Apr 2018 20:55:15 +0000 (13:55 -0700)
committerBruce Forstall <brucefo@microsoft.com>
Thu, 12 Apr 2018 20:55:15 +0000 (13:55 -0700)
When we call jitStartup, we pass a JitHost interface that the JIT
calls to query for data. These queries look up in the recorded
MCH data, and could fail (and throw an exception) if data is
missing, which it can be for running non-matching altjit against
a collection. Protect these calls with exception handling.

src/ToolBox/superpmi/superpmi/jitinstance.cpp
src/ToolBox/superpmi/superpmi/jitinstance.h

index 52f2b90d22a901cf5a31436e4b1692f109d09732..ee589f12ea0c47d990efd53e4f9fed78270c42a7 100644 (file)
@@ -190,7 +190,11 @@ HRESULT JitInstance::StartUp(char*          PathToJit,
     {
         mc      = firstContext;
         jitHost = new JitHost(*this);
-        pnjitStartup(jitHost);
+        if (!callJitStartup(jitHost))
+        {
+            LogError("jitStartup failed");
+            return -1;
+        }
     }
 
     pJitInstance = pngetJit();
@@ -256,7 +260,11 @@ bool JitInstance::reLoad(MethodContext* firstContext)
     {
         mc      = firstContext;
         jitHost = new JitHost(*this);
-        pnjitStartup(jitHost);
+        if (!callJitStartup(jitHost))
+        {
+            LogError("jitStartup failed");
+            return false;
+        }
     }
 
     pJitInstance = pngetJit();
@@ -465,17 +473,56 @@ void JitInstance::freeLongLivedArray(void* array)
     HeapFree(ourHeap, 0, array);
 }
 
+// Helper for calling pnjitStartup. Needed to allow SEH here.
+bool JitInstance::callJitStartup(ICorJitHost* jithost)
+{
+    // Calling into the collection, which could fail, especially
+    // for altjits. So protect the call.
+
+    struct Param : FilterSuperPMIExceptionsParam_CaptureException
+    {
+        JitInstance* pThis;
+        ICorJitHost* jithost;
+        bool         result;
+    } param;
+    param.pThis   = this;
+    param.jithost = jithost;
+    param.result  = false;
+
+    PAL_TRY(Param*, pParam, &param)
+    {
+        pParam->pThis->pnjitStartup(pParam->jithost);
+        pParam->result = true;
+    }
+    PAL_EXCEPT_FILTER(FilterSuperPMIExceptions_CaptureExceptionAndStop)
+    {
+        SpmiException e(&param.exceptionPointers);
+
+        LogError("failed to call jitStartup.");
+        e.ShowAndDeleteMessage();
+    }
+    PAL_ENDTRY
+
+    return param.result;
+}
+
 // Reset JitConfig, that stores Enviroment variables.
 bool JitInstance::resetConfig(MethodContext* firstContext)
 {
-    if (pnjitStartup != nullptr)
+    if (pnjitStartup == nullptr)
+    {
+        return false;
+    }
+
+    mc = firstContext;
+    ICorJitHost* newHost = new JitHost(*this);
+
+    if (!callJitStartup(newHost))
     {
-        mc                   = firstContext;
-        ICorJitHost* newHost = new JitHost(*this);
-        pnjitStartup(newHost);
-        delete static_cast<JitHost*>(jitHost);
-        jitHost = newHost;
-        return true;
+        return false;
     }
-    return false;
-}
+
+    delete static_cast<JitHost*>(jitHost);
+    jitHost = newHost;
+    return true;
+}
\ No newline at end of file
index eeb2ff70818dac8cd8d9b068e22600fbeb1cc253..e7b5fdb034a387213f99e9207e2f55dd0a096b2b 100644 (file)
@@ -54,6 +54,8 @@ public:
     HRESULT StartUp(char* PathToJit, bool copyJit, bool breakOnDebugBreakorAV, MethodContext* firstContext);
     bool reLoad(MethodContext* firstContext);
 
+    bool callJitStartup(ICorJitHost* newHost);
+
     bool resetConfig(MethodContext* firstContext);
 
     Result CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput);