Fix BailOnNoProfile inside try functions in x86

There were multiple issues with having BailOnNoProfile for functions with try.

1.	BailOnNoProfile  for functions within try can make argouts orphaned. For orphaned argouts, we allocate space in locals area for bailouts.
But when there is a try we expect all live out params at the top of the stack instead of the locals area. We cannot distinguish at Bailout time to
read from the locals area or the top of the stack for out params, because all we depend upon to distinguish between the two are if the offsets are -ve or +ve.

2.	To compute totalStackToBeRestored on return from EH bailout, we use totalOutParamCount.
But with BailOnNoProfile, we need to remove the number of orphaned args and the number of dead out params from this number.

3	We compute startCallArgRestoreAdjustCounts in GlobOptBailOut which consist of offsets from top of the try c++ frame in case of nested calls.
For nested calls, argouts for the outer call need to be restored from an offset of stack-adjustment-done-by-the-inner-call from esp.
If the inner call consisted of orphaned args, this calculation is inaccurate. At this point we do not know how many orphaned args are present.
We know if an arg is orphaned only during lowering.
Similarly if the inner call consisted of dead out params, this calculation will be inaccurate.

This change fixes the above issues.

Fixes OS#14486384

Author: meg-gupta

lib/Backend/BailOut.cpp

@@ -83,7 +83,7 @@ BailOutInfo::NeedsStartCallAdjust(uint i, const IR::Instr * bailOutInstr) const
 }
 
 void
-BailOutInfo::RecordStartCallInfo(uint i, uint argRestoreAdjustCount, IR::Instr *instr)
+BailOutInfo::RecordStartCallInfo(uint i, IR::Instr *instr)
 {
     Assert(i < this->startCallCount);
     Assert(this->startCallInfo);
@@ -92,7 +92,8 @@ BailOutInfo::RecordStartCallInfo(uint i, uint argRestoreAdjustCount, IR::Instr *
 
     this->startCallInfo[i].instr = instr;
     this->startCallInfo[i].argCount = instr->GetArgOutCount(/*getInterpreterArgOutCount*/ true);
-    this->startCallInfo[i].argRestoreAdjustCount = argRestoreAdjustCount;
+    this->startCallInfo[i].argRestoreAdjustCount = 0;
+    this->startCallInfo[i].isOrphanedCall = false;
 }
 
 void
@@ -124,7 +125,7 @@ BailOutInfo::GetStartCallOutParamCount(uint i) const
 }
 
 void
-BailOutInfo::RecordStartCallInfo(uint i, uint argRestoreAdjust, IR::Instr *instr)
+BailOutInfo::RecordStartCallInfo(uint i, IR::Instr *instr)
 {
     Assert(i < this->startCallCount);
     Assert(this->startCallInfo);
@@ -1028,6 +1029,7 @@ BailOutRecord::RestoreValues(IR::BailOutKind bailOutKind, Js::JavascriptCallStac
     bool fromLoopBody, Js::Var * registerSaves, Js::InterpreterStackFrame * newInstance, Js::Var* pArgumentsObject, void * argoutRestoreAddress) const
 {
     bool isLocal = offsets == nullptr;
+
     if (isLocal == true)
     {
         globalBailOutRecordTable->IterateGlobalBailOutRecordTableRows(m_bailOutRecordId, [=](GlobalBailOutRecordDataRow *row) {
@@ -1071,7 +1073,15 @@ BailOutRecord::RestoreValues(IR::BailOutKind bailOutKind, Js::JavascriptCallStac
             this->IsOffsetNativeIntOrFloat(i, argOutSlotStart, &isFloat64, &isInt32,
                                            &isSimd128F4, &isSimd128I4, &isSimd128I8, &isSimd128I16,
                                            &isSimd128U4, &isSimd128U8, &isSimd128U16, &isSimd128B4, &isSimd128B8, &isSimd128B16);
-
+#ifdef _M_IX86
+            if (this->argOutOffsetInfo->isOrphanedArgSlot->Test(argOutSlotStart + i))
+            {
+                RestoreValue(bailOutKind, layout, values, scriptContext, fromLoopBody, registerSaves, newInstance, pArgumentsObject,
+                    nullptr, i, offset, /* isLocal */ true, isFloat64, isInt32, isSimd128F4, isSimd128I4, isSimd128I8,
+                    isSimd128I16, isSimd128U4, isSimd128U8, isSimd128U16, isSimd128B4, isSimd128B8, isSimd128B16);
+                continue;
+            }
+#endif
             RestoreValue(bailOutKind, layout, values, scriptContext, fromLoopBody, registerSaves, newInstance, pArgumentsObject,
                          argoutRestoreAddress, i, offset, false, isFloat64, isInt32, isSimd128F4, isSimd128I4, isSimd128I8,
                          isSimd128I16, isSimd128U4, isSimd128U8, isSimd128U16, isSimd128B4, isSimd128B8, isSimd128B16);

lib/Backend/BailOut.h

@@ -18,6 +18,7 @@ class BailOutInfo
         IR::Instr * instr;
         uint argCount;
         uint argRestoreAdjustCount;
+        bool isOrphanedCall;
     } StartCallInfo;
 #else
     typedef uint StartCallInfo;
@@ -55,7 +56,7 @@ class BailOutInfo
     void FinalizeOffsets(__in_ecount(count) int * offsets, uint count, Func *func, BVSparse<JitArenaAllocator> *bvInlinedArgSlot);
 #endif
 
-    void RecordStartCallInfo(uint i, uint argRestoreAdjustCount, IR::Instr *instr);
+    void RecordStartCallInfo(uint i, IR::Instr *instr);
     uint GetStartCallOutParamCount(uint i) const;
 #ifdef _M_IX86
     bool NeedsStartCallAdjust(uint i, const IR::Instr * instr) const;
@@ -288,6 +289,9 @@ class BailOutRecord
     {
         BVFixed * argOutFloat64Syms;        // Used for float-type-specialized ArgOut symbols. Index = [0 .. BailOutInfo::totalOutParamCount].
         BVFixed * argOutLosslessInt32Syms;  // Used for int-type-specialized ArgOut symbols (which are native int and for bailout we need tagged ints).
+#ifdef _M_IX86
+        BVFixed * isOrphanedArgSlot;
+#endif
 #ifdef ENABLE_SIMDJS
         // SIMD_JS
         BVFixed * argOutSimd128F4Syms;
@@ -310,6 +314,9 @@ class BailOutRecord
         {
             FixupNativeDataPointer(argOutFloat64Syms, chunkList);
             FixupNativeDataPointer(argOutLosslessInt32Syms, chunkList);
+#ifdef _M_IX86
+            FixupNativeDataPointer(isOrphanedArgSlot, chunkList);
+#endif
 #ifdef ENABLE_SIMDJS
             FixupNativeDataPointer(argOutSimd128F4Syms, chunkList);
             FixupNativeDataPointer(argOutSimd128I4Syms, chunkList);

lib/Backend/GlobOptBailOut.cpp

@@ -952,7 +952,6 @@ GlobOpt::FillBailOutInfo(BasicBlock *block, BailOutInfo * bailOutInfo)
         bailOutInfo->totalOutParamCount = totalOutParamCount;
         bailOutInfo->argOutSyms = JitAnewArrayZ(this->func->m_alloc, StackSym *, totalOutParamCount);
 
-        uint argRestoreAdjustCount = 0;
         FOREACH_SLISTBASE_ENTRY(IR::Opnd *, opnd, block->globOptData.callSequence)
         {
             if(opnd->GetStackSym()->HasArgSlotNum())
@@ -999,25 +998,6 @@ GlobOpt::FillBailOutInfo(BasicBlock *block, BailOutInfo * bailOutInfo)
 
                 bailOutInfo->startCallFunc[startCallNumber] = sym->m_instrDef->m_func;
 #ifdef _M_IX86
-                if (this->currentRegion && (this->currentRegion->GetType() == RegionTypeTry || this->currentRegion->GetType() == RegionTypeFinally))
-                {
-                    // For a bailout in argument evaluation from an EH region, the esp is offset by the TryCatch helper's frame. So, the argouts are not actually pushed at the
-                    // offsets stored in the bailout record, which are relative to ebp. Need to restore the argouts from the actual value of esp before calling the Bailout helper.
-                    // For nested calls, argouts for the outer call need to be restored from an offset of stack-adjustment-done-by-the-inner-call from esp.
-                    if (startCallNumber + 1 == bailOutInfo->startCallCount)
-                    {
-                        argRestoreAdjustCount = 0;
-                    }
-                    else
-                    {
-                        argRestoreAdjustCount = bailOutInfo->startCallInfo[startCallNumber + 1].argRestoreAdjustCount + bailOutInfo->startCallInfo[startCallNumber + 1].argCount;
-                        if ((Math::Align<int32>(bailOutInfo->startCallInfo[startCallNumber + 1].argCount * MachPtr, MachStackAlignment) - (bailOutInfo->startCallInfo[startCallNumber + 1].argCount * MachPtr)) != 0)
-                        {
-                            argRestoreAdjustCount++;
-                        }
-                    }
-                }
-
                 if (sym->m_isInlinedArgSlot)
                 {
                     bailOutInfo->inlinedStartCall->Set(startCallNumber);
@@ -1027,10 +1007,9 @@ GlobOpt::FillBailOutInfo(BasicBlock *block, BailOutInfo * bailOutInfo)
                 Assert(totalOutParamCount >= argOutCount);
                 Assert(argOutCount >= currentArgOutCount);
 
-                bailOutInfo->RecordStartCallInfo(startCallNumber, argRestoreAdjustCount, sym->m_instrDef);
+                bailOutInfo->RecordStartCallInfo(startCallNumber, sym->m_instrDef);
                 totalOutParamCount -= argOutCount;
                 currentArgOutCount = 0;
-
             }
         }
         NEXT_SLISTBASE_ENTRY;

lib/Backend/LinearScan.cpp

@@ -1740,6 +1740,9 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
         NativeCodeData::AllocatorNoFixup<BVFixed>* allocatorT = (NativeCodeData::AllocatorNoFixup<BVFixed>*)allocator;
         BVFixed * argOutFloat64Syms = BVFixed::New(bailOutInfo->totalOutParamCount, allocatorT);
         BVFixed * argOutLosslessInt32Syms = BVFixed::New(bailOutInfo->totalOutParamCount, allocatorT);
+#ifdef _M_IX86
+        BVFixed * isOrphanedArgSlot = BVFixed::New(bailOutInfo->totalOutParamCount, allocatorT);
+#endif
         // SIMD_JS
         BVFixed * argOutSimd128F4Syms = BVFixed::New(bailOutInfo->totalOutParamCount, allocatorT);
         BVFixed * argOutSimd128I4Syms = BVFixed::New(bailOutInfo->totalOutParamCount, allocatorT);
@@ -1772,7 +1775,8 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
             uint outParamCount = bailOutInfo->GetStartCallOutParamCount(i);
             startCallOutParamCounts[i] = outParamCount;
 #ifdef _M_IX86
-            startCallArgRestoreAdjustCounts[i] = bailOutInfo->startCallInfo[i].argRestoreAdjustCount;
+            uint orphanedArgCount = 0;
+            uint deadArgCount = 0;
             // Only x86 has a progression of pushes of out args, with stack alignment.
             bool fDoStackAdjust = false;
             if (!bailOutInfo->inlinedStartCall->Test(i))
@@ -1833,6 +1837,7 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
                 currentBailOutRecord->argOutOffsetInfo->startCallIndex = i;
                 currentBailOutRecord->argOutOffsetInfo->startCallOutParamCounts = &startCallOutParamCounts[i];
 #ifdef _M_IX86
+                currentBailOutRecord->argOutOffsetInfo->isOrphanedArgSlot = isOrphanedArgSlot;
                 currentBailOutRecord->startCallArgRestoreAdjustCounts = &startCallArgRestoreAdjustCounts[i];
 #endif
                 currentBailOutRecord->argOutOffsetInfo->outParamOffsets = &outParamOffsets[outParamStart];
@@ -1842,13 +1847,13 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
 
 #ifdef ENABLE_SIMDJS
                 // SIMD_JS
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128F4Syms  = argOutSimd128F4Syms;
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128I4Syms  = argOutSimd128I4Syms  ;
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128I8Syms  = argOutSimd128I8Syms  ;
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128I16Syms = argOutSimd128I16Syms ;
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128U4Syms  = argOutSimd128U4Syms  ;
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128U8Syms  = argOutSimd128U8Syms  ;
-                currentBailOutRecord->argOutOffsetInfo->argOutSimd128U16Syms = argOutSimd128U16Syms ;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128F4Syms = argOutSimd128F4Syms;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128I4Syms = argOutSimd128I4Syms;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128I8Syms = argOutSimd128I8Syms;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128I16Syms = argOutSimd128I16Syms;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128U4Syms = argOutSimd128U4Syms;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128U8Syms = argOutSimd128U8Syms;
+                currentBailOutRecord->argOutOffsetInfo->argOutSimd128U16Syms = argOutSimd128U16Syms;
                 currentBailOutRecord->argOutOffsetInfo->argOutSimd128B4Syms = argOutSimd128U4Syms;
                 currentBailOutRecord->argOutOffsetInfo->argOutSimd128B8Syms = argOutSimd128U8Syms;
                 currentBailOutRecord->argOutOffsetInfo->argOutSimd128B16Syms = argOutSimd128U16Syms;
@@ -1866,6 +1871,11 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
                 StackSym * sym = bailOutInfo->argOutSyms[argOutSlot];
                 if (sym == nullptr)
                 {
+#ifdef _M_IX86
+#if DBG
+                    deadArgCount++;
+#endif
+#endif
                     // This can happen when instr with bailout occurs before all ArgOuts for current call instr are processed.
                     continue;
                 }
@@ -2014,6 +2024,13 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
 #else
                                     // Stack offset are negative, includes the PUSH EBP and return address
                                     outParamOffsets[outParamOffsetIndex] = sym->m_offset - (2 * MachPtr);
+#endif
+#ifdef _M_IX86
+                                    isOrphanedArgSlot->Set(outParamOffsetIndex);
+                                    bailOutInfo->startCallInfo[i].isOrphanedCall = true;
+#if DBG
+                                    orphanedArgCount++;
+#endif
 #endif
                                 }
 #ifdef _M_IX86
@@ -2135,6 +2152,41 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
 #endif
                 }
             }
+#ifdef _M_IX86
+            uint liveArgCount = bailOutInfo->startCallInfo[i /*startCallCount*/].argCount - orphanedArgCount - deadArgCount;
+            if (liveArgCount == 0)
+            {
+                bailOutInfo->startCallInfo[i].isOrphanedCall = true;
+            }
+#endif
+        }
+
+        for (int i = startCallCount - 1; i >= 0; i--)
+        {
+#ifdef _M_IX86
+            uint argRestoreAdjustCount = 0;
+            if (this->currentRegion && (this->currentRegion->GetType() == RegionTypeTry || this->currentRegion->GetType() == RegionTypeFinally))
+            {
+                // For a bailout in argument evaluation from an EH region, the esp is offset by the TryCatch helper's frame. So, the argouts are not actually pushed at the
+                // offsets stored in the bailout record, which are relative to ebp. Need to restore the argouts from the actual value of esp before calling the Bailout helper.
+                // For nested calls, argouts for the outer call need to be restored from an offset of stack-adjustment-done-by-the-inner-call from esp.
+                if ((unsigned)(i + 1) == bailOutInfo->startCallCount)
+                {
+                    argRestoreAdjustCount = 0;
+                }
+                else
+                {
+                    uint argCount = bailOutInfo->startCallInfo[i + 1].isOrphanedCall ? 0 : bailOutInfo->startCallInfo[i + 1].argCount;
+                    argRestoreAdjustCount = bailOutInfo->startCallInfo[i + 1].argRestoreAdjustCount + argCount;
+                    if ((Math::Align<int32>(argCount * MachPtr, MachStackAlignment) - (argCount * MachPtr)) != 0)
+                    {
+                        argRestoreAdjustCount++;
+                    }
+                }
+                bailOutInfo->startCallInfo[i].argRestoreAdjustCount = argRestoreAdjustCount;
+            }
+            startCallArgRestoreAdjustCounts[i] = bailOutInfo->startCallInfo[i].argRestoreAdjustCount;
+#endif
         }
     }
     else

lib/Backend/Lower.cpp

@@ -24982,19 +24982,21 @@ Lowerer::EmitEHBailoutStackRestore(IR::Instr * bailoutInstr)
 
 #ifdef _M_IX86
     BailOutInfo * bailoutInfo = bailoutInstr->GetBailOutInfo();
+    uint totalLiveArgCount = 0;
     if (bailoutInfo->startCallCount != 0)
     {
         uint totalStackToBeRestored = 0;
         uint stackAlignmentAdjustment = 0;
         for (uint i = 0; i < bailoutInfo->startCallCount; i++)
         {
-            uint startCallOutParamCount = bailoutInfo->GetStartCallOutParamCount(i);
-            if ((Math::Align<int32>(startCallOutParamCount * MachPtr, MachStackAlignment) - (startCallOutParamCount * MachPtr)) != 0)
+            uint startCallLiveArgCount = bailoutInfo->startCallInfo[i].isOrphanedCall ? 0 : bailoutInfo->GetStartCallOutParamCount(i);
+            if ((Math::Align<int32>(startCallLiveArgCount * MachPtr, MachStackAlignment) - (startCallLiveArgCount * MachPtr)) != 0)
             {
                 stackAlignmentAdjustment++;
             }
+            totalLiveArgCount += startCallLiveArgCount;
         }
-        totalStackToBeRestored = (bailoutInfo->totalOutParamCount + stackAlignmentAdjustment) * MachPtr;
+        totalStackToBeRestored = (totalLiveArgCount + stackAlignmentAdjustment) * MachPtr;
 
         IR::RegOpnd * espOpnd = IR::RegOpnd::New(NULL, LowererMD::GetRegStackPointer(), TyMachReg, this->m_func);
         IR::Opnd * opnd = IR::IndirOpnd::New(espOpnd, totalStackToBeRestored, TyMachReg, this->m_func);