[MERGE #5208 @boingoing] OS#16855035 - Properly support redeferral for serialized defer-parsed functions

Merge pull request #5208 from boingoing:redefer_for_serialized_defer_parse

When we redeferred and subsequently reparsed a function which was defer-parsed and deserialized from bytecode, we took the null captured names set off of the new ParseNode and used it to generate a set of empty deferred stubs for the function. When we skipped scanning nested functions we didn't push references to their captured names. This caused us to try and load from invalid slots at runtime. We also lost the set of deferred stubs stored in ParseableFunctionInfo AuxPtrs during one of the duplication hops when we duplicate a FunctionProxy.

Fixes:
https://microsoft.visualstudio.com/OS/_workitems/edit/16855035

Author: boingoing

lib/Runtime/Base/FunctionBody.cpp

@@ -728,7 +728,6 @@ namespace Js
         SetCountField(CounterFields::ConstantCount, 1);
 
         proxy->UpdateFunctionBodyImpl(this);
-        this->SetDeferredStubs(proxy->GetDeferredStubs());
 
         this->m_defaultEntryPointInfo = RecyclerNewFinalized(scriptContext->GetRecycler(),
             FunctionEntryPointInfo, this, scriptContext->CurrentThunk, scriptContext->GetThreadContext());
@@ -1456,6 +1455,7 @@ namespace Js
         CopyDeferParseField(m_inParamCount);
         CopyDeferParseField(m_grfscr);
         other->SetScopeInfo(this->GetScopeInfo());
+        other->SetDeferredStubs(this->GetDeferredStubs());
         CopyDeferParseField(m_utf8SourceHasBeenSet);
 #if DBG
         CopyDeferParseField(deferredParseNextFunctionId);
@@ -4993,7 +4993,6 @@ namespace Js
         this->SetStatementMaps(nullptr);
         this->SetCodeGenGetSetRuntimeData(nullptr);
         this->SetPropertyIdOnRegSlotsContainer(nullptr);
-        this->SetDeferredStubs(nullptr);
         this->profiledLdLenCount = 0;
         this->profiledLdElemCount = 0;
         this->profiledStElemCount = 0;

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -2770,7 +2770,7 @@ void ByteCodeGenerator::EmitOneFunction(ParseNodeFnc *pnodeFnc)
 
     if (funcInfo->root->pnodeBody == nullptr)
     {
-        if (!PHASE_OFF1(Js::SkipNestedDeferredPhase) && (this->GetFlags() & fscrCreateParserState) == fscrCreateParserState)
+        if (!PHASE_OFF1(Js::SkipNestedDeferredPhase) && (this->GetFlags() & fscrCreateParserState) == fscrCreateParserState && deferParseFunction->GetCompileCount() == 0)
         {
             deferParseFunction->BuildDeferredStubs(funcInfo->root);
         }
@@ -2779,10 +2779,6 @@ void ByteCodeGenerator::EmitOneFunction(ParseNodeFnc *pnodeFnc)
     }
 
     Js::FunctionBody* byteCodeFunction = funcInfo->GetParsedFunctionBody();
-    // We've now done a full parse of this function, so we no longer need to remember the extents
-    // and attributes of the top-level nested functions. (The above code has run for all of those,
-    // so they have pointers to the stub sub-trees they need.)
-    byteCodeFunction->SetDeferredStubs(nullptr);
 
     try
     {

lib/Runtime/Language/AsmJsModule.cpp

@@ -554,7 +554,7 @@ namespace Js
 
         if (fncNode->pnodeBody == NULL)
         {
-            if (!PHASE_OFF1(Js::SkipNestedDeferredPhase) && (grfscr & fscrCreateParserState) == fscrCreateParserState)
+            if (!PHASE_OFF1(Js::SkipNestedDeferredPhase) && (grfscr & fscrCreateParserState) == fscrCreateParserState && deferParseFunction->GetCompileCount() == 0)
             {
                 deferParseFunction->BuildDeferredStubs(fncNode);
             }

test/Basics/bug_os16855035.js

@@ -0,0 +1,33 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+// With -useparserstatecache -force:deferparse -force:redeferral -CollectGarbage we should not hit an AV
+
+var glo;
+function test()
+{
+    function nested1(param2)
+    {
+        function nested2()
+        {
+            return  param2;
+        }
+
+        escape();
+        return 'pass';
+    }
+    WScript.Echo(nested1());
+
+    function blah() { return 'pass' }
+    function escape() { glo = blah; }
+}
+
+
+test("test3" )
+WScript.Echo(glo());
+
+CollectGarbage();
+
+test("test1");

test/Basics/rlexe.xml

@@ -421,4 +421,11 @@
       <tags>exclude_test,exclude_jshost</tags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>bug_os16855035.js</files>
+      <compile-flags>-UseParserStateCache -ParserStateCache -Force:DeferParse -Force:Redeferral -CollectGarbage</compile-flags>
+      <tags>exclude_test,exclude_jshost</tags>
+    </default>
+  </test>
 </regress-exe>