Fixing stack balance under generator/yield

We were caching and restoring the sp in the processtryfinally in order to balance the stack when we are in the middle of call. However in the case of generator we can yield out of the call. In that case we actually haven't finished the try part. So once we comeback after yield we don't need to cache the stack again. Introduced the flag to capture such state and fix the problem.
chakra-core · Aug 26, 2016 · 5cd9eed · 5cd9eed
1 parent cd950a7
commit 5cd9eed
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 3 deletions.
diff --git a/lib/Runtime/Language/InterpreterStackFrame.cpp b/lib/Runtime/Language/InterpreterStackFrame.cpp
@@ -1084,6 +1084,7 @@ namespace Js
         newInstance->m_flags        = InterpreterStackFrameFlags_None;
         newInstance->closureInitDone = false;
         newInstance->isParamScopeDone = false;
+        newInstance->shouldCacheSP = true;
 #if ENABLE_PROFILE_INFO
         newInstance->switchProfileMode = false;
         newInstance->isAutoProfiling = false;
@@ -6744,7 +6745,10 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(const byte * ip)
             // mark the stackFrame as 'in try block'
             this->m_flags |= InterpreterStackFrameFlags_WithinTryBlock;
 
-            CacheSp();
+            if (shouldCacheSP)
+            {
+                CacheSp();
+            }
 
             if (this->IsInDebugMode())
             {
@@ -6772,10 +6776,10 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(const byte * ip)
             this->m_flags &= ~InterpreterStackFrameFlags_WithinTryBlock;
         }
 
+        shouldCacheSP = !skipFinallyBlock;
+
         if (skipFinallyBlock)
         {
-            RestoreSp();
-
             // A leave occurred due to a yield
             return;
         }

diff --git a/lib/Runtime/Language/InterpreterStackFrame.h b/lib/Runtime/Language/InterpreterStackFrame.h
@@ -112,6 +112,7 @@ namespace Js
 
         bool closureInitDone : 1;
         bool isParamScopeDone : 1;
+        bool shouldCacheSP : 1; // Helps in determining if we need to cache the sp in ProcessTryFinally
 #if ENABLE_PROFILE_INFO
         bool switchProfileMode : 1;
         bool isAutoProfiling : 1;

diff --git a/test/es6/iteratorclose.js b/test/es6/iteratorclose.js
@@ -1069,6 +1069,47 @@ var tests = [
             });
 		}
 	},    
+    {
+		name : "BugFix : yielding in the call expression under generator function",
+		body : function () {
+            var val = 0;
+            function bar(a, b, c) { val = b; }
+            function *foo(d) {
+                for (var k of [2, 3]) {
+                    bar(1, d ? yield : d, k);
+                }
+            }
+            var iter = foo(true);
+            iter.next();
+            iter.next();
+            iter.next(10);
+            assert.areEqual(val, 10, "yielding in the call expression under for..of is working correctly");
+		}
+	},    
+    {
+		name : "BugFix : yielding in the call expression under try catch",
+		body : function () {
+            var val = 0;
+            var counter = 0;
+            function bar(a, b, c) { val = b; }
+            function *foo(d) {
+                try {
+                     try {
+                         bar(1, d ? yield : d, 11);
+                     } finally {
+                         counter++;
+                     }
+                } finally {
+                    counter++;
+                }
+            }
+            var iter = foo(true);
+            iter.next();
+            iter.next(10);
+            assert.areEqual(val, 10, "yielding in the call expression under try/catch is working correctly");
+            assert.areEqual(counter, 2, "both finally called after yielding");
+		}
+	},    
 ];
 
 testRunner.runTests(tests, {