Turn on inlining for functions with trycatch/tryfinally

- Avoid copy prop s0 (return symbol) into inlined functions

- Clear inlinee callinfo : When there is an exception in an inlinee which is inside an EH region, we end up leaving datastructures in stackwalker dirty.
If the EH handler was a try catch, and if the same code is executed in a loop  in the interprerter after BailOnNoException,
the stack walker due to dirty datastructures assumes that there are inlinee frames on the stack.

lib/Backend/BailOut.cpp

@@ -1608,6 +1608,10 @@ BailOutRecord::BailOutHelper(Js::JavascriptCallStackLayout * layout, Js::ScriptF
     {
         newInstance->OrFlags(Js::InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall);
     }
+    if (isInlinee)
+    {
+        newInstance->OrFlags(Js::InterpreterStackFrameFlags_FromInlineeCodeInEHBailOut);
+    }
 
     ThreadContext *threadContext = newInstance->GetScriptContext()->GetThreadContext();
 

lib/Backend/Func.h

@@ -208,7 +208,7 @@ class Func
 
     bool DoInline() const
     {
-        return DoGlobOpt() && !GetTopFunc()->HasTry();
+        return DoGlobOpt();
     }
 
     bool DoOptimizeTry() const

lib/Backend/GlobOpt.cpp

@@ -3674,6 +3674,18 @@ GlobOpt::CopyProp(IR::Opnd *opnd, IR::Instr *instr, Value *val, IR::IndirOpnd *p
 
     ValueInfo *valueInfo = val->GetValueInfo();
 
+
+    if (this->func->HasFinally())
+    {
+        // s0 = undefined was added on functions with early exit in try-finally functions, that can get copy-proped and case incorrect results
+        if (instr->m_opcode == Js::OpCode::ArgOut_A_Inline && valueInfo->GetSymStore() &&
+            valueInfo->GetSymStore()->m_id == 0)
+        {
+            // We don't want to copy-prop s0 (return symbol) into inlinee code
+            return opnd;
+        }
+    }
+
     // Constant prop?
     int32 intConstantValue;
     int64 int64ConstantValue;

lib/Backend/Inline.cpp

@@ -1399,8 +1399,6 @@ bool
 Inline::TryOptimizeCallInstrWithFixedMethod(IR::Instr *callInstr, const FunctionJITTimeInfo * inlineeInfo, bool isPolymorphic, bool isBuiltIn, bool isCtor, bool isInlined, bool &safeThis,
                                             bool dontOptimizeJustCheck, uint i /*i-th inlinee at a polymorphic call site*/)
 {
-    Assert(!callInstr->m_func->GetJITFunctionBody()->HasTry());
-
     if (PHASE_OFF(Js::FixedMethodsPhase, callInstr->m_func))
     {
         return false;

lib/Backend/InliningDecider.cpp

@@ -24,17 +24,6 @@ bool InliningDecider::InlineIntoTopFunc() const
         return false;
     }
 
-    if (this->topFunc->GetHasTry())
-    {
-#if defined(DBG_DUMP) || defined(ENABLE_DEBUG_CONFIG_OPTIONS)
-        char16 debugStringBuffer[MAX_FUNCTION_BODY_DEBUG_STRING_SIZE];
-#endif
-        INLINE_TESTTRACE(_u("INLINING: Skip Inline: Has try\tCaller: %s (%s)\n"), this->topFunc->GetDisplayName(),
-            this->topFunc->GetDebugNumberSet(debugStringBuffer));
-        // Glob opt doesn't run on function with try, so we can't generate bailout for it
-        return false;
-    }
-
     return InlineIntoInliner(topFunc);
 }
 
@@ -218,14 +207,6 @@ Js::FunctionInfo *InliningDecider::Inline(Js::FunctionBody *const inliner, Js::F
             return nullptr;
         }
 
-        if (inlinee->GetHasTry())
-        {
-            INLINE_TESTTRACE(_u("INLINING: Skip Inline: Has try\tInlinee: %s (%s)\tCaller: %s (%s)\n"),
-                inlinee->GetDisplayName(), inlinee->GetDebugNumberSet(debugStringBuffer), inliner->GetDisplayName(),
-                inliner->GetDebugNumberSet(debugStringBuffer2));
-            return nullptr;
-        }
-
         // This is a hard limit as the argOuts array is statically sized.
         if (inlinee->GetInParamsCount() > Js::InlineeCallInfo::MaxInlineeArgoutCount)
         {

lib/Backend/LinearScan.cpp

@@ -1412,6 +1412,14 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
         // To indicate this is a subsequent bailout from an inlinee
         bailOutRecord->bailOutOpcode = Js::OpCode::InlineeEnd;
 #endif
+        if (this->func->HasTry())
+        {
+            RegionType currentRegionType = this->currentRegion->GetType();
+            if (currentRegionType == RegionTypeTry || currentRegionType == RegionTypeCatch || currentRegionType == RegionTypeFinally)
+            {
+                bailOutRecord->ehBailoutData = this->currentRegion->ehBailoutData;
+            }
+        }
         funcBailOutData[funcIndex].bailOutRecord->parent = bailOutRecord;
         funcIndex--;
         funcBailOutData[funcIndex].bailOutRecord = bailOutRecord;

lib/Backend/Lower.cpp

@@ -19995,7 +19995,13 @@ Lowerer::EHBailoutPatchUp()
                 this->InsertReturnThunkForRegion(this->currentRegion, restoreReturnValueFromBailoutLabel);
                 if (instr->HasBailOutInfo())
                 {
-                    this->SetHasBailedOut(instr);
+                    if (instr->GetBailOutInfo()->bailOutFunc == this->m_func)
+                    {
+                        // We dont set this bit for inlined code, if there was a bailout in the inlined code,
+                        // and an exception was thrown, we want the caller's handler to handle the exception accordingly.
+                        // TODO : Revisit when we start inlining functions with try-catch/try-finally
+                        this->SetHasBailedOut(instr);
+                    }
                     tmpInstr = this->EmitEHBailoutStackRestore(instr);
                     this->EmitSaveEHBailoutReturnValueAndJumpToRetThunk(tmpInstr);
                     if (!restoreReturnFromBailoutEmitted)

lib/Runtime/Base/ThreadContext.cpp

@@ -125,6 +125,7 @@ ThreadContext::ThreadContext(AllocationPolicyManager * allocationPolicyManager,
     entryExitRecord(nullptr),
     leafInterpreterFrame(nullptr),
     threadServiceWrapper(nullptr),
+    tryCatchFrameAddr(nullptr),
     temporaryArenaAllocatorCount(0),
     temporaryGuestArenaAllocatorCount(0),
     crefSContextForDiag(0),

lib/Runtime/Base/ThreadContext.h

@@ -678,7 +678,7 @@ class ThreadContext sealed :
     ThreadServiceWrapper* threadServiceWrapper;
     uint functionCount;
     uint sourceInfoCount;
-
+    void * tryCatchFrameAddr;
     enum RedeferralState
     {
         InitialRedeferralState,
@@ -1253,6 +1253,9 @@ class ThreadContext sealed :
     uint EnterScriptStart(Js::ScriptEntryExitRecord *, bool doCleanup);
     void EnterScriptEnd(Js::ScriptEntryExitRecord *, bool doCleanup);
 
+    void * GetTryCatchFrameAddr() { return this->tryCatchFrameAddr; }
+    void SetTryCatchFrameAddr(void * frameAddr) { this->tryCatchFrameAddr = frameAddr; }
+
     template <bool leaveForHost>
     void LeaveScriptStart(void *);
     template <bool leaveForHost>

lib/Runtime/Language/EHBailoutData.h

@@ -33,7 +33,15 @@ namespace Js
             this->parent = parent;
             this->child = nullptr;
         }
-
+        EHBailoutData(EHBailoutData * other)
+        {
+            this->nestingDepth = other->nestingDepth;
+            this->catchOffset = other->catchOffset;
+            this->finallyOffset = other->finallyOffset;
+            this->ht = other->ht;
+            this->parent = other->parent;
+            this->child = nullptr;
+        }
 #if ENABLE_NATIVE_CODEGEN
         void Fixup(NativeCodeData::DataChunk* chunkList)
         {

lib/Runtime/Language/InterpreterStackFrame.cpp

@@ -3284,7 +3284,7 @@ namespace Js
         } autoRestore(this);
 #endif
 
-        if (this->ehBailoutData)
+        if (this->ehBailoutData && !(m_flags & InterpreterStackFrameFlags_FromInlineeCodeInEHBailOut))
         {
             if ((m_flags & Js::InterpreterStackFrameFlags_FromBailOut) && !(m_flags & InterpreterStackFrameFlags_ProcessingBailOutFromEHCode))
             {
@@ -6752,7 +6752,6 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(const byte * ip)
                 // Generator return scenario, so no need to go into the catch block and we must rethrow to propagate the exception to down level
                 JavascriptExceptionOperators::DoThrow(exception, scriptContext);
             }
-
             if (catchOffset != 0)
             {
                 exception = exception->CloneIfStaticExceptionObject(scriptContext);

lib/Runtime/Language/InterpreterStackFrame.h

@@ -27,6 +27,7 @@ namespace Js
         InterpreterStackFrameFlags_FromBailOut = 8,
         InterpreterStackFrameFlags_ProcessingBailOutOnArrayAccessHelperCall = 0x10,
         InterpreterStackFrameFlags_ProcessingBailOutFromEHCode = 0x20,
+        InterpreterStackFrameFlags_FromInlineeCodeInEHBailOut = 0x40,
         InterpreterStackFrameFlags_All = 0xFFFF,
     };
 

lib/Runtime/Language/JavascriptExceptionOperators.cpp

@@ -60,6 +60,18 @@ namespace Js
         m_threadContext->SetIsUserCode(m_previousCatchHandlerToUserCodeStatus);
     }
 
+    JavascriptExceptionOperators::TryCatchFrameAddrStack::TryCatchFrameAddrStack(ScriptContext* scriptContext, void *frameAddr)
+    {
+        m_threadContext = scriptContext->GetThreadContext();
+        m_prevTryCatchFrameAddr = m_threadContext->GetTryCatchFrameAddr();
+        scriptContext->GetThreadContext()->SetTryCatchFrameAddr(frameAddr);
+    }
+
+    JavascriptExceptionOperators::TryCatchFrameAddrStack::~TryCatchFrameAddrStack()
+    {
+        m_threadContext->SetTryCatchFrameAddr(m_prevTryCatchFrameAddr);
+    }
+
     bool JavascriptExceptionOperators::CrawlStackForWER(Js::ScriptContext& scriptContext)
     {
         return Js::Configuration::Global.flags.WERExceptionSupport && !scriptContext.GetThreadContext()->HasCatchHandler();
@@ -87,6 +99,7 @@ namespace Js
         try
         {
             Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
+            Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, frame);
             continuation = amd64_CallWithFakeFrame(tryAddr, frame, spillSize, argsSize);
         }
         catch (const Js::JavascriptException& err)
@@ -215,6 +228,7 @@ namespace Js
         try
         {
             Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
+            Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);
 #if defined(_M_ARM)
             continuation = arm_CallEhFrame(tryAddr, framePtr, localsPtr, argsSize);
 #elif defined(_M_ARM64)
@@ -365,6 +379,7 @@ namespace Js
         try
         {
             Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
+            Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);
 
             // Adjust the frame pointer and call into the try.
             // If the try completes without raising an exception, it will pass back the continuation address.
@@ -875,7 +890,16 @@ namespace Js
 
         JavascriptExceptionOperators::ThrowExceptionObject(exceptionObject, scriptContext, /*considerPassingToDebugger=*/ true, /*returnAddress=*/ nullptr, resetStack);
     }
+#if ENABLE_NATIVE_CODEGEN
+    void JavascriptExceptionOperators::WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress)
+    {
+        JavascriptStackWalker walker(scriptContext, /*useEERContext*/ true, returnAddress);
 
+        // We have to walk the inlinee frames and clear callinfo count on them on an exception
+        // At this point inlinedFrameWalker is closed, so we should build it again by calling InlinedFrameWalker::FromPhysicalFrame
+        walker.WalkAndClearInlineeFrameCallInfoOnException();
+    }
+#endif
     void
         JavascriptExceptionOperators::WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException, bool resetSatck)
     {
@@ -1090,6 +1114,20 @@ namespace Js
                 WalkStackForExceptionContext(*scriptContext, exceptionContext, thrownObject, StackCrawlLimitOnThrow(thrownObject, *scriptContext), returnAddress, /*isThrownException=*/ true, resetStack);
                 exceptionObject->FillError(exceptionContext, scriptContext);
                 AddStackTraceToObject(thrownObject, exceptionContext.GetStackTrace(), *scriptContext, /*isThrownException=*/ true, resetStack);
+
+                // We need to clear callinfo on inlinee virtual frames on an exception.
+                // We now allow inlining of functions into callers that have try-catch/try-finally.
+                // When there is an exception inside the inlinee with caller having a try-catch, clear the inlinee callinfo by walking the stack.
+                // If not, we might have the try-catch inside a loop, and when we execute the loop next time in the interpreter on BailOnException,
+                // we will see inlined frames as being present even though they are not, because we depend on FrameInfo's callinfo to tell if an inlinee is on the stack,
+                // and we haven't cleared those bits due to the exception
+
+#if ENABLE_NATIVE_CODEGEN
+                if (scriptContext->GetThreadContext()->GetTryCatchFrameAddr() != nullptr)
+                {
+                    WalkStackForCleaningUpInlineeInfo(scriptContext, returnAddress);
+                }
+#endif
             }
             Assert(!scriptContext ||
                    // If we disabled implicit calls and we did record an implicit call, do not throw.

lib/Runtime/Language/JavascriptExceptionOperators.h

@@ -43,6 +43,17 @@ namespace Js
             ~AutoCatchHandlerExists();
         };
 
+        class TryCatchFrameAddrStack
+        {
+          private:
+            void * m_prevTryCatchFrameAddr;
+            ThreadContext* m_threadContext;
+
+          public:
+            TryCatchFrameAddrStack(ScriptContext* scriptContext, void *frameAddr);
+            ~TryCatchFrameAddrStack();
+        };
+
         static void __declspec(noreturn) OP_Throw(Var object, ScriptContext* scriptContext);
         static void __declspec(noreturn) Throw(Var object, ScriptContext* scriptContext);
         static void __declspec(noreturn) ThrowExceptionObject(Js::JavascriptExceptionObject* exceptionObject, ScriptContext* scriptContext, bool considerPassingToDebugger = false, PVOID returnAddress = NULL, bool resetStack = false);
@@ -80,6 +91,9 @@ namespace Js
         static Var ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...);
         static Var StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...);
         static void WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException = true, bool resetSatck = false);
+#if ENABLE_NATIVE_CODEGEN
+        static void WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress);
+#endif
         static void AddStackTraceToObject(Var obj, JavascriptExceptionContext::StackTrace* stackTrace, ScriptContext& scriptContext, bool isThrownException = true, bool resetSatck = false);
         static uint64 StackCrawlLimitOnThrow(Var thrownObject, ScriptContext& scriptContext);
 

lib/Runtime/Language/JavascriptStackWalker.cpp

@@ -616,7 +616,35 @@ namespace Js
         }
         return nullptr;
     }
-
+#if ENABLE_NATIVE_CODEGEN
+    void JavascriptStackWalker::WalkAndClearInlineeFrameCallInfoOnException()
+    {
+        // Walk the stack and when we find the first JavascriptFrame, we clear the inlinee's callinfo for this frame
+        // It is sufficient we stop at the first Javascript frame which had the enclosing try-catch
+        // TODO : Revisit when we start inlining functions with try-catch/try-finally
+        while (this->Walk(true))
+        {
+            if (this->IsJavascriptFrame())
+            {
+                if (!this->isNativeLibraryFrame)
+                {
+                    if (HasInlinedFramesOnStack())
+                    {
+                        for (int index = inlinedFrameWalker.GetFrameCount() - 1; index >= 0; index--)
+                        {
+                            auto inlinedFrame = inlinedFrameWalker.GetFrameAtIndex(index);
+                            inlinedFrame->callInfo.Clear();
+                        }
+                    }
+                    if (this->currentFrame.GetFrame() == this->scriptContext->GetThreadContext()->GetTryCatchFrameAddr())
+                    {
+                        break;
+                    }
+                }
+            }
+        }
+    }
+#endif
     // Note: noinline is to make sure that when we unwind to the unwindToAddress, there is at least one frame to unwind.
     _NOINLINE
     JavascriptStackWalker::JavascriptStackWalker(ScriptContext * scriptContext, bool useEERContext, PVOID returnAddress, bool _forceFullWalk /*=false*/) :

lib/Runtime/Language/JavascriptStackWalker.h

@@ -114,6 +114,7 @@ namespace Js
         uint32                  GetBottomMostInlineeOffset() const;
         Js::JavascriptFunction *GetBottomMostFunctionObject() const;
         void                    FinalizeStackValues(__in_ecount(argCount) Js::Var args[], size_t argCount) const;
+        int32                   GetFrameCount() { return frameCount; }
 
     private:
         enum {
@@ -135,11 +136,13 @@ namespace Js
 
         };
 
-        void Initialize(int32 frameCount, __in_ecount(frameCount) InlinedFrame **frames, Js::ScriptFunction *parent);
+    public:
+        InlinedFrame *const     GetFrameAtIndex(signed index) const;
 
+    private:
+        void Initialize(int32 frameCount, __in_ecount(frameCount) InlinedFrame **frames, Js::ScriptFunction *parent);
         void MoveNext();
         InlinedFrame *const GetCurrentFrame() const;
-        InlinedFrame *const GetFrameAtIndex(signed index) const;
 
         Js::ScriptFunction *parentFunction;
         InlinedFrame          **frames;
@@ -233,6 +236,7 @@ namespace Js
         void ClearCachedInternalFrameInfo();
         void SetCachedInternalFrameInfo(InternalFrameType frameType, JavascriptFunction* function, bool hasInlinedFramesOnStack, bool prevIntFrameIsFromBailout);
         InternalFrameInfo GetCachedInternalFrameInfo() const { return this->lastInternalFrameInfo; }
+        void WalkAndClearInlineeFrameCallInfoOnException();
 #endif
         bool IsCurrentPhysicalFrameForLoopBody() const;