Set hasBailedOutBitPtr correctly for nested finally blocks

JITed functions use a stack of bits to determine whether a bailout
occured within a try-catch or try-finally block. When a bailout occurs
within a finally, the corresponding entry in the bit stack has already
been popped, but we still need to set the bit correctly for the
containing try block, if one exists.

Author: Kevin Smith

lib/Backend/BailOut.cpp

@@ -1193,14 +1193,7 @@ BailOutRecord::BailOutInlinedCommon(Js::JavascriptCallStackLayout * layout, Bail
     BailOutReturnValue bailOutReturnValue;
     Js::ScriptFunction * innerMostInlinee = nullptr;
     BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, false, branchValue);
-
-    bool * hasBailedOutBitPtr = layout->functionObject->GetScriptContext()->GetThreadContext()->GetHasBailedOutBitPtr();
-    Assert(!bailOutRecord->ehBailoutData || hasBailedOutBitPtr ||
-        bailOutRecord->ehBailoutData->ht == Js::HandlerType::HT_Finally /* When we bailout from inlinee in non exception finally, we maynot see hasBailedOutBitPtr*/);
-    if (hasBailedOutBitPtr && bailOutRecord->ehBailoutData && bailOutRecord->ehBailoutData->ht != Js::HandlerType::HT_Finally)
-    {
-        *hasBailedOutBitPtr = true;
-    }
+    SetHasBailedOutBit(bailOutRecord, layout->functionObject->GetScriptContext());
     Js::Var result = BailOutCommonNoCodeGen(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset, returnAddress, bailOutKind, branchValue,
         registerSaves, &bailOutReturnValue);
     ScheduleFunctionCodeGen(Js::VarTo<Js::ScriptFunction>(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
@@ -1239,20 +1232,40 @@ BailOutRecord::BailOutFromLoopBodyInlinedCommon(Js::JavascriptCallStackLayout *
     BailOutReturnValue bailOutReturnValue;
     Js::ScriptFunction * innerMostInlinee = nullptr;
     BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, true, branchValue);
-    bool * hasBailedOutBitPtr = layout->functionObject->GetScriptContext()->GetThreadContext()->GetHasBailedOutBitPtr();
-    Assert(!bailOutRecord->ehBailoutData || hasBailedOutBitPtr ||
-        bailOutRecord->ehBailoutData->ht == Js::HandlerType::HT_Finally /* When we bailout from inlinee in non exception finally, we maynot see hasBailedOutBitPtr*/);
-    if (hasBailedOutBitPtr && bailOutRecord->ehBailoutData)
-    {
-        *hasBailedOutBitPtr = true;
-    }
-
+    SetHasBailedOutBit(bailOutRecord, layout->functionObject->GetScriptContext());
     uint32 result = BailOutFromLoopBodyHelper(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset,
         bailOutKind, nullptr, registerSaves, &bailOutReturnValue);
     ScheduleLoopBodyCodeGen(Js::VarTo<Js::ScriptFunction>(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind);
     return result;
 }
 
+void
+BailOutRecord::SetHasBailedOutBit(BailOutRecord const * bailOutRecord, Js::ScriptContext * scriptContext)
+{
+    Js::EHBailoutData * ehBailoutData = bailOutRecord->ehBailoutData;
+    if (!ehBailoutData)
+    {
+        return;
+    }
+
+    // When a bailout occurs within a finally region, the hasBailedOutBitPtr associated with the
+    // try-catch-finally or try-finally has already been removed from the stack. In that case,
+    // we set the hasBailedOutBitPtr for the nearest enclosing try or catch region within the
+    // function.
+    while (ehBailoutData->ht == Js::HandlerType::HT_Finally)
+    {
+        if (!ehBailoutData->parent || ehBailoutData->parent->nestingDepth < 0)
+        {
+            return;
+        }
+        ehBailoutData = ehBailoutData->parent;
+    }
+
+    bool * hasBailedOutBitPtr = scriptContext->GetThreadContext()->GetHasBailedOutBitPtr();
+    Assert(hasBailedOutBitPtr);
+    *hasBailedOutBitPtr = true;
+}
+
 void
 BailOutRecord::BailOutInlinedHelper(Js::JavascriptCallStackLayout * layout, BailOutRecord const *& currentBailOutRecord,
     uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Var * registerSaves, BailOutReturnValue * bailOutReturnValue, Js::ScriptFunction ** innerMostInlinee, bool isInLoopBody, Js::Var branchValue)
@@ -3041,4 +3054,3 @@ void  GlobalBailOutRecordDataTable::AddOrUpdateRow(JitArenaAllocator *allocator,
     rowToInsert->regSlot = regSlot;
     *lastUpdatedRowIndex = length++;
 }
-

lib/Backend/BailOut.h

@@ -277,6 +277,8 @@ class BailOutRecord
     static uint32 BailOutFromLoopBodyHelper(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
         uint32 bailOutOffset, IR::BailOutKind bailOutKind, Js::Var branchValue, Js::Var * registerSaves, BailOutReturnValue * returnValue = nullptr);
 
+    static void SetHasBailedOutBit(BailOutRecord const * bailOutRecord, Js::ScriptContext * scriptContext);
+
     static void UpdatePolymorphicFieldAccess(Js::JavascriptFunction *  function, BailOutRecord const * bailOutRecord);
 
     static void ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind,

test/EH/hasBailedOutBug3.js

@@ -32,4 +32,3 @@ test0();
 test0();
 test0();
 print("Passed\n");
-

test/EH/hasBailedOutBug4.js

@@ -0,0 +1,38 @@
+var shouldBailout = false;
+var caught = false;
+
+function test0() {
+  function func0() {
+    if (shouldBailout) {
+      throw new Error('oops');
+    }
+  }
+
+  function func1() { func0() }
+  function func2() { func1() }
+  function func3() { shouldBailout ? obj0 : null }
+
+  var obj0 = { method0: func1 };
+  var obj1 = { method0: func2 };
+
+  try {
+    try {} finally { func3(); }
+  } catch {
+    caught = true;
+  }
+
+  func2();
+}
+
+// generate profile
+test0();
+test0();
+
+// run code with bailouts enabled
+shouldBailout = true;
+try {
+  test0();
+} catch {}
+if (!caught) {
+  print('Passed');
+}

test/EH/rlexe.xml

@@ -194,6 +194,11 @@
        <files>hasBailedOutBug3.js</files>
     </default>
   </test>
+  <test>
+    <default>
+       <files>hasBailedOutBug4.js</files>
+    </default>
+  </test>
   <test>
     <default>
        <files>StackOverflow.js</files>