[MERGE chakra-core#923] add failfast if entryExitRecord list is messed up

MikeHolman · MikeHolman · commit c78fcf6158bb · 2016-05-10T13:51:01.000-07:00
Merge pull request chakra-core#923 from MikeHolman:sehchange
diff --git a/lib/Common/Exceptions/ReportError.cpp b/lib/Common/Exceptions/ReportError.cpp
@@ -106,4 +106,16 @@ __declspec(noinline) void FromDOM_NoScriptScope_fatal_error()
     ReportFatalException(NULL, E_UNEXPECTED, EnterScript_FromDOM_NoScriptScope, scenario);
 }
 
+__declspec(noinline) void EntryExitRecord_Corrupted_fatal_error()
+{
+    int scenario = 6;
+    ReportFatalException(NULL, E_UNEXPECTED, Fatal_EntryExitRecordCorruption, scenario);
+}
+
+__declspec(noinline) void UnexpectedExceptionHandling_fatal_error(EXCEPTION_POINTERS * originalException)
+{
+    int scenario = 7;
+    ReportFatalException(NULL, E_UNEXPECTED, Fatal_UnexpectedExceptionHandling, scenario);
+}
+
 #pragma optimize("",on)
diff --git a/lib/Common/Exceptions/ReportError.h b/lib/Common/Exceptions/ReportError.h
@@ -19,7 +19,9 @@ enum ErrorReason
     MarkStack_OUTOFMEMORY = 11,
     EnterScript_FromDOM_NoScriptScope = 12,
     Fatal_FailedToBox_OUTOFMEMORY = 13,
-    Fatal_Recycler_MemoryCorruption = 14
+    Fatal_Recycler_MemoryCorruption = 14,
+    Fatal_EntryExitRecordCorruption = 15,
+    Fatal_UnexpectedExceptionHandling = 16
 };
 
 extern "C" void ReportFatalException(
@@ -52,6 +54,8 @@ 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);
 
 #ifdef LARGEHEAPBLOCK_ENCODING
 void LargeHeapBlock_Metadata_Corrupted(
diff --git a/lib/Runtime/Base/ThreadContext.cpp b/lib/Runtime/Base/ThreadContext.cpp
@@ -1872,6 +1872,12 @@ ThreadContext::PushEntryExitRecord(Js::ScriptEntryExitRecord * record)
         Assert(lastRecord->leaveForHost || lastRecord->leaveForAsyncHostOperation);
         lastRecord->hasReentered = true;
         record->next = lastRecord;
+
+        // these are on stack, which grows down. if this condition doesn't hold, then the list somehow got messed up
+        if (!IsOnStack(lastRecord) || (uintptr_t)record >= (uintptr_t)lastRecord)
+        {
+            EntryExitRecord_Corrupted_fatal_error();
+        }
     }
 
     this->entryExitRecord = record;
@@ -1881,7 +1887,14 @@ void ThreadContext::PopEntryExitRecord(Js::ScriptEntryExitRecord * record)
 {
     AssertMsg(record && record == this->entryExitRecord, "Mismatch script entry/exit");
 
-    this->entryExitRecord = this->entryExitRecord->next;
+    // these are on stack, which grows down. if this condition doesn't hold, then the list somehow got messed up
+    Js::ScriptEntryExitRecord * next = this->entryExitRecord->next;
+    if (next && (!IsOnStack(next) || (uintptr_t)this->entryExitRecord >= (uintptr_t)next))
+    {
+        EntryExitRecord_Corrupted_fatal_error();
+    }
+
+    this->entryExitRecord = next;
 }
 
 BOOL ThreadContext::ReserveStaticTypeIds(__in int first, __in int last)
diff --git a/lib/Runtime/Library/JavascriptFunction.cpp b/lib/Runtime/Library/JavascriptFunction.cpp
@@ -642,10 +642,8 @@ namespace Js
             // 0xE06D7363 is C++ exception code
             if (exceptionCode != 0 && !IsDebuggerPresent() && exceptionCode != 0xE06D7363 && exceptionAction != EXCEPTION_CONTINUE_EXECUTION)
             {
-                exceptionInfo;
-
                 // ensure that hosts are not doing SEH across Chakra frames, as that can lead to bad state (e.g. destructors not being called)
-                RaiseFailFastException(NULL, NULL, NULL);
+                UnexpectedExceptionHandling_fatal_error(&exceptionInfo);
             }
         }
         //ret should never be null here

Original file line number	Diff line number	Diff line change
`@@ -642,10 +642,8 @@ namespace Js`
`642`	`642`	`// 0xE06D7363 is C++ exception code`
`643`	`643`	`if (exceptionCode != 0 && !IsDebuggerPresent() && exceptionCode != 0xE06D7363 && exceptionAction != EXCEPTION_CONTINUE_EXECUTION)`
`644`	`644`	`{`
`645`		`- exceptionInfo;`
`646`		`-`
`647`	`645`	`// ensure that hosts are not doing SEH across Chakra frames, as that can lead to bad state (e.g. destructors not being called)`
`648`		`- RaiseFailFastException(NULL, NULL, NULL);`
	`646`	`+ UnexpectedExceptionHandling_fatal_error(&exceptionInfo);`
`649`	`647`	`}`
`650`	`648`	`}`
`651`	`649`	`//ret should never be null here`