[1.8>1.9] [MERGE #4535 @meg-gupta] Fix setting hasBailout when there are inlined functions in try/catch

Merge pull request #4535 from meg-gupta:fixCatchEatingUpEx

Fixes OS#15078638

When we bailout executing trycode from within OP_TryCatch, we complete the execution of the current function enclosing the try/catch in the interpreter.
If there was an exception within the try region, it is caught and handled accordingly in ProcessTryHandlerBailOut which reconstructs try/catch/finally frames
when we bailout midway executing code within OP_TryCatch. If there was an exception outside the try region, the catch of the OP_TryCatch ends up catching it,
because it happens to be on the callstack. For this we use the hasBailOut bit which is per function, so we know that this exception has to be passed above.

When we have inlined functions inside the try, and for bailouts inside the inlined code, we do not set the hasBailedOut bit, so that the enclosing functions catch in OP_TryCatch catches it.

But when we bailout from inlined code inside try, we execute inlined code, as well as the enclosing function's code in the interpreter.
We will be execution code past the try/catch of the current function in the interpreter. Now if any code outside the eh region throws,
we will catch that in OP_TryCatch which happens to be on the callstack. And we will end up handling it instead of passing above because we have not set the hasBailedOutBit from the bailout point.

This change fixes this issue. We pass a pointer to the hasBailedOutBit and set it once we have finished executing the inlined frames and are ready to process the interpreter frame of the current function.

Author: meg-gupta

Diff for: lib/Backend/BailOut.cpp

@@ -1217,6 +1217,12 @@ BailOutRecord::BailOutInlinedCommon(Js::JavascriptCallStackLayout * layout, Bail
     BailOutReturnValue bailOutReturnValue;
     Js::ScriptFunction * innerMostInlinee = nullptr;
     BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, false, branchValue);
+
+    bool * hasBailOutBit = layout->functionObject->GetScriptContext()->GetThreadContext()->GetHasBailedOutBitPtr();
+    if (hasBailOutBit != nullptr && bailOutRecord->ehBailoutData)
+    {
+        *hasBailOutBit = true;
+    }
     Js::Var result = BailOutCommonNoCodeGen(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset, returnAddress, bailOutKind, branchValue,
         registerSaves, &bailOutReturnValue);
     ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
@@ -1255,6 +1261,11 @@ BailOutRecord::BailOutFromLoopBodyInlinedCommon(Js::JavascriptCallStackLayout *
     BailOutReturnValue bailOutReturnValue;
     Js::ScriptFunction * innerMostInlinee = nullptr;
     BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, true, branchValue);
+    bool * hasBailOutBit = layout->functionObject->GetScriptContext()->GetThreadContext()->GetHasBailedOutBitPtr();
+    if (hasBailOutBit != nullptr && bailOutRecord->ehBailoutData)
+    {
+        *hasBailOutBit = true;
+    }
     uint32 result = BailOutFromLoopBodyHelper(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset,
         bailOutKind, nullptr, registerSaves, &bailOutReturnValue);
     ScheduleLoopBodyCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind);

Diff for: lib/Runtime/Base/ThreadContext.cpp

@@ -93,6 +93,7 @@ ThreadContext::ThreadContext(AllocationPolicyManager * allocationPolicyManager,
     stackProber(nullptr),
     isThreadBound(false),
     hasThrownPendingException(false),
+    hasBailedOutBitPtr(nullptr),
     pendingFinallyException(nullptr),
     noScriptScope(false),
     heapEnum(nullptr),

Diff for: lib/Runtime/Base/ThreadContext.h

@@ -638,6 +638,7 @@ class ThreadContext sealed :
     StackProber * stackProber;
     bool isThreadBound;
     bool hasThrownPendingException;
+    bool * hasBailedOutBitPtr;
     bool callDispose;
 #if ENABLE_JS_REENTRANCY_CHECK
     bool noJsReentrancy;
@@ -1531,6 +1532,16 @@ class ThreadContext sealed :
         this->hasThrownPendingException = true;
     }
 
+    bool * GetHasBailedOutBitPtr()
+    {
+        return this->hasBailedOutBitPtr;
+    }
+
+    void SetHasBailedOutBitPtr(bool *setValue)
+    {
+        this->hasBailedOutBitPtr = setValue;
+    }
+
     void SetRecordedException(Js::JavascriptExceptionObject* exceptionObject, bool propagateToDebugger = false)
     {
         this->recyclableData->exceptionObject = exceptionObject;

Diff for: lib/Runtime/Language/JavascriptExceptionOperators.cpp

@@ -94,10 +94,11 @@ namespace Js
         void *continuation = nullptr;
         JavascriptExceptionObject *exception = nullptr;
         void *tryCatchFrameAddr = nullptr;
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)frame + hasBailedOutOffset));
+
         PROBE_STACK(scriptContext, Constants::MinStackDefault + spillSize + argsSize);
         {
             Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, frame);
-
             try
             {
                 Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
@@ -129,18 +130,22 @@ namespace Js
 
             exception = exception->CloneIfStaticExceptionObject(scriptContext);
             bool hasBailedOut = *(bool*)((char*)frame + hasBailedOutOffset); // stack offsets are negative
+            // If an inlinee bailed out due to some reason, the execution of the current function enclosing the try catch will also continue in the interpreter
+            // During execution in the interpreter, if we throw outside the region enclosed in try/catch, this catch ends up catching that exception because its present on the call stack
             if (hasBailedOut)
             {
                 // If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
                 // it so happens that this catch was on the stack and caught the exception.
                 // Re-throw!
+                scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
                 JavascriptExceptionOperators::DoThrow(exception, scriptContext);
-            }
+            }  
+
             Var exceptionObject = exception->GetThrownObject(scriptContext);
             AssertMsg(exceptionObject, "Caught object is null.");
             continuation = amd64_CallWithFakeFrame(catchAddr, frame, spillSize, argsSize, exceptionObject);
         }
-
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
         return continuation;
     }
 
@@ -154,6 +159,7 @@ namespace Js
     {
         void                      *tryContinuation     = nullptr;
         JavascriptExceptionObject *exception           = nullptr;
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)frame + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackDefault + spillSize + argsSize);
 
@@ -189,6 +195,7 @@ namespace Js
                 // If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
                 // it so happens that this catch was on the stack and caught the exception.
                 // Re-throw!
+                scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
                 JavascriptExceptionOperators::DoThrow(exception, scriptContext);
             }
 
@@ -197,6 +204,7 @@ namespace Js
             return continuation;
         }
 
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
         return tryContinuation;
     }
 
@@ -250,6 +258,7 @@ namespace Js
         void *continuation = nullptr;
         JavascriptExceptionObject *exception = nullptr;
         void * tryCatchFrameAddr = nullptr;
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)localsPtr + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackDefault + argsSize);
         {
@@ -295,8 +304,10 @@ namespace Js
                 // If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
                 // it so happens that this catch was on the stack and caught the exception.
                 // Re-throw!
+                scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
                 JavascriptExceptionOperators::DoThrow(exception, scriptContext);
             }
+
             Var exceptionObject = exception->GetThrownObject(scriptContext);
             AssertMsg(exceptionObject, "Caught object is null.");
 #if defined(_M_ARM)
@@ -306,6 +317,7 @@ namespace Js
 #endif
         }
 
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
         return continuation;
     }
 
@@ -320,9 +332,9 @@ namespace Js
     {
         void                      *tryContinuation     = nullptr;
         JavascriptExceptionObject *exception           = nullptr;
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)localsPtr + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackDefault + argsSize);
-
         try
         {
 #if defined(_M_ARM)
@@ -355,8 +367,10 @@ namespace Js
                 // If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
                 // it so happens that this catch was on the stack and caught the exception.
                 // Re-throw!
+                scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
                 JavascriptExceptionOperators::DoThrow(exception, scriptContext);
             }
+
             scriptContext->GetThreadContext()->SetPendingFinallyException(exception);
 #if defined(_M_ARM)
             void * finallyContinuation = arm_CallEhFrame(finallyAddr, framePtr, localsPtr, argsSize);
@@ -366,6 +380,7 @@ namespace Js
             return finallyContinuation;
         }
 
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
         return tryContinuation;
     }
 
@@ -429,6 +444,7 @@ namespace Js
         void* continuationAddr = NULL;
         Js::JavascriptExceptionObject* pExceptionObject = NULL;
         void *tryCatchFrameAddr = nullptr;
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)framePtr + hasBailedOutOffset));
 
         PROBE_STACK(scriptContext, Constants::MinStackDefault);
         {
@@ -526,8 +542,10 @@ namespace Js
                 // If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
                 // it so happens that this catch was on the stack and caught the exception.
                 // Re-throw!
+                scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
                 JavascriptExceptionOperators::DoThrow(pExceptionObject, scriptContext);
             }
+
             Var catchObject = pExceptionObject->GetThrownObject(scriptContext);
             AssertMsg(catchObject, "Caught object is NULL");
 #ifdef _M_IX86
@@ -581,14 +599,15 @@ namespace Js
 #endif
         }
 
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
         return continuationAddr;
     }
 
     void* JavascriptExceptionOperators::OP_TryFinally(void* tryAddr, void* handlerAddr, void* framePtr, int hasBailedOutOffset, ScriptContext *scriptContext)
     {
         Js::JavascriptExceptionObject* pExceptionObject = NULL;
         void* continuationAddr = NULL;
-
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)framePtr + hasBailedOutOffset));
         PROBE_STACK(scriptContext, Constants::MinStackDefault);
 
         try
@@ -676,8 +695,10 @@ namespace Js
                 // If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
                 // it so happens that this catch was on the stack and caught the exception.
                 // Re-throw!
+                scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
                 JavascriptExceptionOperators::DoThrow(pExceptionObject, scriptContext);
             }
+
             scriptContext->GetThreadContext()->SetPendingFinallyException(pExceptionObject);
 
             void* newContinuationAddr = NULL;
@@ -733,6 +754,8 @@ namespace Js
 #endif
             return newContinuationAddr;
         }
+
+        scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
         return continuationAddr;
     }
 

Diff for: test/EH/hasBailedOutBug.baseline

@@ -0,0 +1 @@
+TypeError: Assignment to read-only properties is not allowed in strict mode

Diff for: test/EH/hasBailedOutBug.js

@@ -0,0 +1,48 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var shouldBailout = false;
+function test0() {
+  var obj0 = {};
+  var func0 = function () {
+  };
+  var func1 = function () {
+    (function () {
+      'use strict';
+      try {
+        function func8() {
+          obj0.prop2;
+        }
+        var uniqobj4 = func8();
+      } catch (ex) {
+        return 'somestring';
+      } finally {
+      }
+      func0(ary.push(ary.unshift(Object.prototype.length = protoObj0)));
+    }(shouldBailout ? (Object.defineProperty(Object.prototype, 'length', {
+      get: function () {
+      }
+    })) : arguments));
+  };
+  var ary = Array();
+  var protoObj0 = Object();
+  ({
+    prop7: shouldBailout ? (Object.defineProperty(obj0, 'prop2', {
+      set: function () {
+      }
+    })) : Object
+  });
+  for (; func1(); ) {
+  }
+}
+test0();
+test0();
+shouldBailout = true;
+try {
+  test0();
+}
+catch(ex) {
+  print(ex);
+}

Diff for: test/EH/rlexe.xml

@@ -176,4 +176,10 @@
       <compile-flags> -force:inline </compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+       <files>hasBailedOutBug.js</files>
+       <baseline>hasBailedOutBug.baseline</baseline>
+  </default>
+  </test>
 </regress-exe>