[MERGE #396] Fix stack closure boxing bug

Merge pull request #396 from pleath:boxnative
We need to box frame display and scope slots in a native frame before calling BoxNativeFrame so that the boxed versions can be found and written back to their known locations on the frame.

lib/Runtime/Library/StackScriptFunction.cpp

@@ -276,6 +276,26 @@ namespace Js
                     {
                         hasInlineeToBox = false;
 
+                        if (callerFunctionBody->DoStackFrameDisplay())
+                        {
+                            Js::FrameDisplay *stackFrameDisplay = 
+                                this->GetFrameDisplayFromNativeFrame(walker, callerFunctionBody);
+                            // Local frame display may be null if bailout didn't restore it, which means we don't need it.
+                            if (stackFrameDisplay)
+                            {
+                                this->BoxFrameDisplay(stackFrameDisplay);
+                            }
+                        }
+                        if (callerFunctionBody->DoStackScopeSlots())
+                        {
+                            Var* stackScopeSlots = this->GetScopeSlotsFromNativeFrame(walker, callerFunctionBody);
+                            if (stackScopeSlots)
+                            {
+                                // Scope slot pointer may be null if bailout didn't restore it, which means we don't need it.
+                                this->BoxScopeSlots(stackScopeSlots, ScopeSlots(stackScopeSlots).GetCount());
+                            }
+                        }
+
                         // walk native frame
                         this->BoxNativeFrame(walker, callerFunctionBody);
 
@@ -405,6 +425,62 @@ namespace Js
         }
     }
 
+    uintptr_t StackScriptFunction::BoxState::GetNativeFrameDisplayIndex(FunctionBody * functionBody)
+    {
+#if _M_IX86 || _M_AMD64
+        if (functionBody->GetInParamsCount() == 0)
+        {
+            return (uintptr_t)JavascriptFunctionArgIndex_StackFrameDisplayNoArg;
+        }
+        else
+#endif
+        {
+            return (uintptr_t)JavascriptFunctionArgIndex_StackFrameDisplay;
+        }
+    }
+
+    uintptr_t StackScriptFunction::BoxState::GetNativeScopeSlotsIndex(FunctionBody * functionBody)
+    {
+#if _M_IX86 || _M_AMD64
+        if (functionBody->GetInParamsCount() == 0)
+        {
+            return (uintptr_t)JavascriptFunctionArgIndex_StackScopeSlotsNoArg;
+        }
+        else
+#endif
+        {
+            return (uintptr_t)JavascriptFunctionArgIndex_StackScopeSlots;
+        }
+    }
+
+    FrameDisplay * StackScriptFunction::BoxState::GetFrameDisplayFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody)
+    {
+        uintptr_t frameDisplayIndex = GetNativeFrameDisplayIndex(callerFunctionBody);
+        void **argv = walker.GetCurrentArgv();
+        return (Js::FrameDisplay*)argv[frameDisplayIndex];
+    }
+
+    Var * StackScriptFunction::BoxState::GetScopeSlotsFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody)
+    {
+        uintptr_t scopeSlotsIndex = GetNativeScopeSlotsIndex(callerFunctionBody);
+        void **argv = walker.GetCurrentArgv();
+        return (Var*)argv[scopeSlotsIndex];
+    }
+
+    void StackScriptFunction::BoxState::SetFrameDisplayFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody, FrameDisplay * frameDisplay)
+    {
+        uintptr_t frameDisplayIndex = GetNativeFrameDisplayIndex(callerFunctionBody);
+        void **argv = walker.GetCurrentArgv();
+        ((FrameDisplay**)argv)[frameDisplayIndex] = frameDisplay;
+    }
+
+    void StackScriptFunction::BoxState::SetScopeSlotsFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody, Var * scopeSlots)
+    {
+        uintptr_t scopeSlotsIndex = GetNativeScopeSlotsIndex(callerFunctionBody);
+        void **argv = walker.GetCurrentArgv();
+        ((Var**)argv)[scopeSlotsIndex] = scopeSlots;
+    }
+
     void StackScriptFunction::BoxState::BoxNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody)
     {
         this->ForEachStackNestedFunctionNative(walker, callerFunctionBody, [&](ScriptFunction *curr)
@@ -422,41 +498,24 @@ namespace Js
         });
 
         // Write back the boxed stack closure pointers at the designated stack locations.
-        uintptr_t             frameDisplayIndex;
-        uintptr_t             scopeSlotsIndex;
-#if _M_IX86 || _M_AMD64
-        if (callerFunctionBody->GetInParamsCount() == 0)
-        {
-            frameDisplayIndex = (uintptr_t)JavascriptFunctionArgIndex_StackFrameDisplayNoArg;
-            scopeSlotsIndex = (uintptr_t)JavascriptFunctionArgIndex_StackScopeSlotsNoArg;
-        }
-        else
-#endif
-        {
-            frameDisplayIndex = (uintptr_t)JavascriptFunctionArgIndex_StackFrameDisplay;
-            scopeSlotsIndex = (uintptr_t)JavascriptFunctionArgIndex_StackScopeSlots;
-        }
-
-        void **argv = walker.GetCurrentArgv();
-        Js::FrameDisplay *stackFrameDisplay = (Js::FrameDisplay*)argv[frameDisplayIndex];
-
+        Js::FrameDisplay *stackFrameDisplay = this->GetFrameDisplayFromNativeFrame(walker, callerFunctionBody);
         if (ThreadContext::IsOnStack(stackFrameDisplay))
         {
             Js::FrameDisplay *boxedFrameDisplay;
             if (boxedValues.TryGetValue(stackFrameDisplay, (void**)&boxedFrameDisplay))
             {
-                argv[frameDisplayIndex] = boxedFrameDisplay;
+                this->SetFrameDisplayFromNativeFrame(walker, callerFunctionBody, boxedFrameDisplay);
                 callerFunctionBody->GetScriptContext()->GetThreadContext()->AddImplicitCallFlags(ImplicitCall_Accessor);
             }
         }
 
-        Var              *stackScopeSlots = (Var*)argv[scopeSlotsIndex];
+        Var              *stackScopeSlots = this->GetScopeSlotsFromNativeFrame(walker, callerFunctionBody);
         if (ThreadContext::IsOnStack(stackScopeSlots))
         {
             Var              *boxedScopeSlots;
             if (boxedValues.TryGetValue(stackScopeSlots, (void**)&boxedScopeSlots))
             {
-                argv[scopeSlotsIndex] = boxedScopeSlots;
+                this->SetScopeSlotsFromNativeFrame(walker, callerFunctionBody, boxedScopeSlots);
                 callerFunctionBody->GetScriptContext()->GetThreadContext()->AddImplicitCallFlags(ImplicitCall_Accessor);
             }
         }

lib/Runtime/Library/StackScriptFunction.h

@@ -55,11 +55,15 @@ namespace Js
             ScriptContext * scriptContext;
             void * returnAddress;
 
-            Js::Var * BoxScopeSlots(Js::Var * scopeSlots, uint count);
+            Var * BoxScopeSlots(Var * scopeSlots, uint count);
             bool NeedBoxFrame(FunctionBody * functionBody);
             bool NeedBoxScriptFunction(ScriptFunction * scriptFunction);
             ScriptFunction * BoxStackFunction(StackScriptFunction * scriptFunction);
             FrameDisplay * BoxFrameDisplay(FrameDisplay * frameDisplay);
+            FrameDisplay * GetFrameDisplayFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody);
+            Var * GetScopeSlotsFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody);
+            void SetFrameDisplayFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody, FrameDisplay * frameDisplay);
+            void SetScopeSlotsFromNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody, Var * scopeSlots);
             void BoxNativeFrame(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody);
             void UpdateFrameDisplay(ScriptFunction *nestedFunc);
             void Finish();
@@ -70,6 +74,9 @@ namespace Js
             void ForEachStackNestedFunctionInterpreted(InterpreterStackFrame *interpreterFrame, FunctionBody * callerFunctionBody, Fn fn);
             template<class Fn>
             void ForEachStackNestedFunctionNative(JavascriptStackWalker const& walker, FunctionBody * callerFunctionBody, Fn fn);
+
+            static uintptr_t GetNativeFrameDisplayIndex(FunctionBody * functionBody);
+            static uintptr_t GetNativeScopeSlotsIndex(FunctionBody * functionBody);
         };
 
         ScriptFunction * boxedScriptFunction;

test/stackfunc/box_native_emptyframe.baseline

@@ -0,0 +1 @@
+3,4,5,3,4,5

test/stackfunc/box_native_emptyframe.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.
+//-------------------------------------------------------------------------------------------------------
+
+function test0() {
+    var GiantPrintArray = [];
+    function makeArrayLength() {
+    }
+    var obj0 = {};
+    var obj1 = {};
+    var func0 = function () {
+        protoObj0 = obj0;
+        var __loopvar2 = 3;
+        for (; ; __loopvar2++) {
+            if (__loopvar2 === 3 + 3) {
+                break;
+            }
+            function __f() {
+                if (obj0.prop0) {
+                    GiantPrintArray.push(__loopvar2);
+                    Math.sin(Error());
+                } else {
+                    litObj1 = obj0;
+                }
+            }
+            function __g() {
+                __f();
+            }
+            __f();
+        }
+    };
+    var func1 = function () {
+        litObj1.prop0 = obj1;
+    };
+    var func2 = function () {
+        return func0();
+    };
+    var func3 = function () {
+        ary.push(func1(), func0() ? (uniqobj3) : func2());
+    };
+    obj0.method1 = func3;
+    var ary = Array();
+    makeArrayLength(func2());
+    protoObj0.method1();
+    WScript.Echo(GiantPrintArray);
+}
+test0();

test/stackfunc/rlexe.xml

@@ -683,4 +683,11 @@
       <tags>exclude_fre,exclude_arm,exclude_dynapogo</tags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>box_native_emptyframe.js</files>
+      <baseline>box_native_emptyframe.baseline</baseline>
+      <compile-flags>-lic:1</compile-flags>
+    </default>
+  </test>
 </regress-exe>