[Merge chakra-core/ChakraCore@951b23f750] [MERGE #3932 @boingoing] Lo…

…osen an assert in `FindOrAddPidRef` Merge pull request #3932 from boingoing:RemovePidRefStackAssert It is possible for the pid ref stack to be out-of-order when we try to add the var decl for special names if we had to backtrack and reparse lambda params. Fixes: https://microsoft.visualstudio.com/web/wi.aspx?pcguid=cb55739e-4afe-46a3-970f-1b49d8ee7564&id=13976845
nodejs · Oct 13, 2017 · b54e978 · b54e978
1 parent 8fe5594
commit b54e978
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 4 deletions.
diff --git a/deps/chakrashim/core/lib/Parser/Hash.h b/deps/chakrashim/core/lib/Parser/Hash.h
@@ -243,7 +243,11 @@ struct Ident
         return prevRef;
     }
 
-    PidRefStack * FindOrAddPidRef(ArenaAllocator *alloc, int scopeId, Js::LocalFunctionId funcId)
+    PidRefStack * FindOrAddPidRef(ArenaAllocator *alloc, int scopeId, Js::LocalFunctionId funcId
+#if DBG
+        , bool isReparseLambdaParams = false
+#endif
+    )
     {
         // If the stack is empty, or we are pushing to the innermost scope already,
         // we can go ahead and push a new PidRef on the stack.
@@ -305,7 +309,7 @@ struct Ident
                 return newRef;
             }
 
-            Assert(ref->prev->id <= ref->id);
+            Assert(ref->prev->id <= ref->id || isReparseLambdaParams);
             prevRef = ref;
             ref = ref->prev;
         }

diff --git a/deps/chakrashim/core/lib/Parser/Parse.cpp b/deps/chakrashim/core/lib/Parser/Parse.cpp
@@ -9032,7 +9032,11 @@ PidRefStack* Parser::PushPidRef(IdentPtr pid)
         // NOTE: the phase check is here to protect perf. See OSG 1020424.
         // In some LS AST-rewrite cases we lose a lot of perf searching the PID ref stack rather
         // than just pushing on the top. This hasn't shown up as a perf issue in non-LS benchmarks.
-        return pid->FindOrAddPidRef(&m_nodeAllocator, GetCurrentBlock()->sxBlock.blockId, GetCurrentFunctionNode()->sxFnc.functionId);
+        return pid->FindOrAddPidRef(&m_nodeAllocator, GetCurrentBlock()->sxBlock.blockId, GetCurrentFunctionNode()->sxFnc.functionId
+#if DBG
+            , this->m_reparsingLambdaParams
+#endif
+        );
     }
 
     Assert(GetCurrentBlock() != nullptr);
@@ -9063,7 +9067,11 @@ PidRefStack* Parser::PushPidRef(IdentPtr pid)
 
 PidRefStack* Parser::FindOrAddPidRef(IdentPtr pid, int scopeId, Js::LocalFunctionId funcId)
 {
-    PidRefStack *ref = pid->FindOrAddPidRef(&m_nodeAllocator, scopeId, funcId);
+    PidRefStack *ref = pid->FindOrAddPidRef(&m_nodeAllocator, scopeId, funcId
+#if DBG
+        , this->m_reparsingLambdaParams
+#endif
+    );
     if (ref == NULL)
     {
         Error(ERRnoMemory);

diff --git a/deps/chakrashim/core/test/Basics/SpecialSymbolCapture.js b/deps/chakrashim/core/test/Basics/SpecialSymbolCapture.js
@@ -884,6 +884,13 @@ var tests = [
             `);
         }
     },
+    {
+        name: "PidRefStack might be out-of-order when we try to add special symbol var decl",
+        body: function() {
+            // Failure causes an assert to fire
+            WScript.LoadScript(`(a = function() { this }, b = (this)) => {}`);
+        }
+    }
 ]
 
 testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });