Address review comments

chakra-core · Oct 10, 2017 · c17aa6b · c17aa6b
1 parent 10f22e9
commit c17aa6b
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 5 deletions.
diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp
@@ -3675,11 +3675,15 @@ GlobOpt::CopyProp(IR::Opnd *opnd, IR::Instr *instr, Value *val, IR::IndirOpnd *p
     ValueInfo *valueInfo = val->GetValueInfo();
 
 
-    if (instr->m_opcode == Js::OpCode::ArgOut_A_Inline && valueInfo->GetSymStore() &&
-        valueInfo->GetSymStore()->m_id == 0)
+    if (this->func->HasFinally())
     {
-        // We don't want to copy-prop s0 (return symbol) into inlinee code
-        return opnd;
+        // 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?

diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp
@@ -19999,6 +19999,7 @@ Lowerer::EHBailoutPatchUp()
                     {
                         // 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);

diff --git a/lib/Runtime/Language/JavascriptExceptionOperators.cpp b/lib/Runtime/Language/JavascriptExceptionOperators.cpp
@@ -893,7 +893,7 @@ namespace Js
 #if ENABLE_NATIVE_CODEGEN
     void JavascriptExceptionOperators::WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress)
     {
-        JavascriptStackWalker walker(scriptContext, true, 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

diff --git a/lib/Runtime/Language/JavascriptStackWalker.cpp b/lib/Runtime/Language/JavascriptStackWalker.cpp
@@ -621,6 +621,7 @@ namespace Js
     {
         // 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())