[MERGE #5851 @boingoing] Parser m_nextFunctionId is set incorrectly when skipping over nested function with functions in parameter scope

Merge pull request #5851 from boingoing:skipped_function_correct_id

Parser m_nextFunctionId is set incorrectly when skipping over nested function with functions in parameter scope

We use the deferred stubs to skip over nested functions and as part of skipping them, we adjust m_nextFunctionId so that other nested functions following the one we just skipped will have their function ids set correctly. We use the RestorePoint in the deferred stub to advance m_nextFunctionId by the function id increment amount.

That's all fine unless the function we want to skip has nested functions in the parameter scope. Default argument assignments, for example. In that case, parsing or skipping the functions in the parameter scope would have already advanced m_nextFunctionId and so we end up setting it too high here. When we subsequently try and undefer one of the functions below the skipped one (one of the functions with a wrong function id), it might have a function id greater than the count of functions in the bytecode cache. Executing that function hits an assert in the bitvector we use to mark functions executed.

Author: boingoing

lib/Parser/Parse.cpp

@@ -6175,6 +6175,11 @@ void Parser::ParseTopLevelDeferredFunc(ParseNodeFnc * pnodeFnc, ParseNodeFnc * p
 
         this->GetScanner()->SeekTo(stub->restorePoint, m_nextFunctionId);
 
+        // If we already incremented m_nextFunctionId when we saw some functions in the parameter scope
+        // (in default argument assignment, for example), we want to remove the count of those so the
+        // function ids following the one we are skipping right now are correct.
+        *m_nextFunctionId -= pnodeFnc->nestedCount;
+
         for (uint i = 0; i < stub->capturedNameCount; i++)
         {
             int stringId = stub->capturedNameSerializedIds[i];

test/Bugs/rlexe.xml

@@ -496,6 +496,13 @@
       <compile-flags>-force:deferparse -parserstatecache -useparserstatecache</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>skipping_nested_deferred_incorrect_function_id.js</files>
+      <tags>exclude_jshost</tags>
+      <compile-flags>-force:deferparse -parserstatecache -useparserstatecache</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>withSplitScope.js</files>

test/Bugs/skipping_nested_deferred_incorrect_function_id.js

@@ -0,0 +1,13 @@
+//-------------------------------------------------------------------------------------------------------
+// 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(){
+  function a(b = function c() {}) { return () => { return 6; } };
+  [0].reduce(function d() {}, 0);
+  a()();
+}
+test0();
+
+console.log("pass");