[MERGE chakra-core#2493 @MikeHolman] change how we do our seh failfast

MikeHolman · MikeHolman · commit e64ae9a0e290 · 2017-02-08T15:51:42.000-08:00
Merge pull request chakra-core#2493 from MikeHolman:changesehfailfast We were seeing on jscript9 our SEH failfast was misbehaving, which led to bad misattribution of Watson crashes. It is fixed there, but same may apply to ChakraCore
diff --git a/lib/Common/Exceptions/ReportError.cpp b/lib/Common/Exceptions/ReportError.cpp
@@ -126,7 +126,7 @@ _NOINLINE void EntryExitRecord_Corrupted_fatal_error()
     ReportFatalException(NULL, E_UNEXPECTED, Fatal_EntryExitRecordCorruption, scenario);
 }
 
-_NOINLINE void UnexpectedExceptionHandling_fatal_error(EXCEPTION_POINTERS * originalException)
+_NOINLINE void UnexpectedExceptionHandling_fatal_error()
 {
     int scenario = 7;
     ReportFatalException(NULL, E_UNEXPECTED, Fatal_UnexpectedExceptionHandling, scenario);
diff --git a/lib/Common/Exceptions/ReportError.h b/lib/Common/Exceptions/ReportError.h
@@ -59,7 +59,7 @@ void MarkStack_OOM_fatal_error();
 void Binary_Inconsistency_fatal_error();
 void Version_Inconsistency_fatal_error();
 void EntryExitRecord_Corrupted_fatal_error();
-void UnexpectedExceptionHandling_fatal_error(EXCEPTION_POINTERS * originalException);
+void UnexpectedExceptionHandling_fatal_error();
 
 #ifdef LARGEHEAPBLOCK_ENCODING
 void LargeHeapBlock_Metadata_Corrupted(
diff --git a/lib/Runtime/Base/ThreadContext.cpp b/lib/Runtime/Base/ThreadContext.cpp
@@ -186,6 +186,7 @@ ThreadContext::ThreadContext(AllocationPolicyManager * allocationPolicyManager,
     jsrtRuntime(nullptr),
     propertyMap(nullptr),
     rootPendingClose(nullptr),
+    exceptionCode(0),
     isProfilingUserCode(true),
     loopDepth(0),
     redeferralState(InitialRedeferralState),
diff --git a/lib/Runtime/Base/ThreadContext.h b/lib/Runtime/Base/ThreadContext.h
@@ -710,6 +710,8 @@ class ThreadContext sealed :
     int stackProbeCount;
     // Count stack probes and poll for continuation every n probes
     static const int StackProbePollThreshold = 1000;
+    EXCEPTION_POINTERS exceptionInfo;
+    uint32 exceptionCode;
 
     ArenaAllocator inlineCacheThreadInfoAllocator;
     ArenaAllocator isInstInlineCacheThreadInfoAllocator;
@@ -866,6 +868,10 @@ class ThreadContext sealed :
 #endif
 #endif
 
+    void SetAbnormalExceptionRecord(EXCEPTION_POINTERS *exceptionInfo) { this->exceptionInfo = *exceptionInfo; }
+    void SetAbnormalExceptionCode(uint32 exceptionInfo) { this->exceptionCode = exceptionInfo; }
+    uint32 GetAbnormalExceptionCode() const { return this->exceptionCode; }
+
 #ifdef ENABLE_BASIC_TELEMETRY
     GUID activityId;
 #endif
diff --git a/lib/Runtime/Library/JavascriptFunction.cpp b/lib/Runtime/Library/JavascriptFunction.cpp
@@ -670,9 +670,14 @@ namespace Js
         // SEH and ResumeForOutOfBoundsArrayRefs are not needed.
         ret = CallRootFunctionInternal(args, scriptContext, inScript);
 #else
+        if (scriptContext->GetThreadContext()->GetAbnormalExceptionCode() != 0)
+        {
+            // ensure that hosts are not doing SEH across Chakra frames, as that can lead to bad state (e.g. destructors not being called)
+            UnexpectedExceptionHandling_fatal_error();
+        }
+
         // mark volatile, because otherwise VC will incorrectly optimize away load in the finally block
         volatile uint32 exceptionCode = 0;
-        volatile int exceptionAction = EXCEPTION_CONTINUE_SEARCH;
         EXCEPTION_POINTERS exceptionInfo = {0};
         __try
         {
@@ -683,18 +688,18 @@ namespace Js
             __except (
                 exceptionInfo = *GetExceptionInformation(),
                 exceptionCode = GetExceptionCode(),
-                exceptionAction = CallRootEventFilter(exceptionCode, GetExceptionInformation()))
+                CallRootEventFilter(exceptionCode, GetExceptionInformation()))
             {
                 Assert(UNREACHED);
             }
         }
         __finally
         {
             // 0xE06D7363 is C++ exception code
-            if (exceptionCode != 0 && !IsDebuggerPresent() && exceptionCode != 0xE06D7363 && exceptionAction != EXCEPTION_CONTINUE_EXECUTION)
+            if (exceptionCode != 0 && exceptionCode != 0xE06D7363 && AbnormalTermination() && !IsDebuggerPresent() )
             {
-                // ensure that hosts are not doing SEH across Chakra frames, as that can lead to bad state (e.g. destructors not being called)
-                UnexpectedExceptionHandling_fatal_error(&exceptionInfo);
+                scriptContext->GetThreadContext()->SetAbnormalExceptionCode(exceptionCode);
+                scriptContext->GetThreadContext()->SetAbnormalExceptionRecord(&exceptionInfo);
             }
         }
 #endif

Original file line number	Diff line number	Diff line change
`@@ -126,7 +126,7 @@ _NOINLINE void EntryExitRecord_Corrupted_fatal_error()`
`126`	`126`	`ReportFatalException(NULL, E_UNEXPECTED, Fatal_EntryExitRecordCorruption, scenario);`
`127`	`127`	`}`
`128`	`128`
`129`		`-_NOINLINE void UnexpectedExceptionHandling_fatal_error(EXCEPTION_POINTERS * originalException)`
	`129`	`+_NOINLINE void UnexpectedExceptionHandling_fatal_error()`
`130`	`130`	`{`
`131`	`131`	`int scenario = 7;`
`132`	`132`	`ReportFatalException(NULL, E_UNEXPECTED, Fatal_UnexpectedExceptionHandling, scenario);`
Original file line number	Diff line number	Diff line change
`@@ -670,9 +670,14 @@ namespace Js`
`670`	`670`	`// SEH and ResumeForOutOfBoundsArrayRefs are not needed.`
`671`	`671`	`ret = CallRootFunctionInternal(args, scriptContext, inScript);`
`672`	`672`	`#else`
	`673`	`+ if (scriptContext->GetThreadContext()->GetAbnormalExceptionCode() != 0)`
	`674`	`+ {`
	`675`	`+ // ensure that hosts are not doing SEH across Chakra frames, as that can lead to bad state (e.g. destructors not being called)`
	`676`	`+ UnexpectedExceptionHandling_fatal_error();`
	`677`	`+ }`
	`678`	`+`
`673`	`679`	`// mark volatile, because otherwise VC will incorrectly optimize away load in the finally block`
`674`	`680`	`volatile uint32 exceptionCode = 0;`
`675`		`- volatile int exceptionAction = EXCEPTION_CONTINUE_SEARCH;`
`676`	`681`	`EXCEPTION_POINTERS exceptionInfo = {0};`
`677`	`682`	`__try`
`678`	`683`	`{`
`@@ -683,18 +688,18 @@ namespace Js`
`683`	`688`	`__except (`
`684`	`689`	`exceptionInfo = *GetExceptionInformation(),`
`685`	`690`	`exceptionCode = GetExceptionCode(),`
`686`		`- exceptionAction = CallRootEventFilter(exceptionCode, GetExceptionInformation()))`
	`691`	`+ CallRootEventFilter(exceptionCode, GetExceptionInformation()))`
`687`	`692`	`{`
`688`	`693`	`Assert(UNREACHED);`
`689`	`694`	`}`
`690`	`695`	`}`
`691`	`696`	`__finally`
`692`	`697`	`{`
`693`	`698`	`// 0xE06D7363 is C++ exception code`
`694`		`- if (exceptionCode != 0 && !IsDebuggerPresent() && exceptionCode != 0xE06D7363 && exceptionAction != EXCEPTION_CONTINUE_EXECUTION)`
	`699`	`+ if (exceptionCode != 0 && exceptionCode != 0xE06D7363 && AbnormalTermination() && !IsDebuggerPresent() )`
`695`	`700`	`{`
`696`		`- // ensure that hosts are not doing SEH across Chakra frames, as that can lead to bad state (e.g. destructors not being called)`
`697`		`- UnexpectedExceptionHandling_fatal_error(&exceptionInfo);`
	`701`	`+ scriptContext->GetThreadContext()->SetAbnormalExceptionCode(exceptionCode);`
	`702`	`+ scriptContext->GetThreadContext()->SetAbnormalExceptionRecord(&exceptionInfo);`
`698`	`703`	`}`
`699`	`704`	`}`
`700`	`705`	`#endif`