[MERGE #2330 @pleath] Solve an AST anomaly in rare redeferral cases.

Merge pull request #2330 from pleath:9871933

We may redefer a nested function but choose not to defer it when we recompile the enclosing function. In such a case, the existing nested FunctionProxy is a compact ParseableFunctionInfo, not the full FunctionBody the front end expects to generate. We were keeping the compact structure and discarding the AST subtree belonging to the nested function, but this seems to be producing anomalous AST's that cause problems downstream. Generate the full FunctionBody on the fly instead.

Author: pleath

lib/Runtime/Base/FunctionBody.cpp

@@ -2132,28 +2132,7 @@ namespace Js
         bool isDebugOrAsmJsReparse = false;
         FunctionBody* funcBody = nullptr;
 
-        // If we throw or fail with the function body in an unfinished state, make sure the function info is still
-        // pointing to the old ParseableFunctionInfo and has the right attributes.
-        class AutoRestoreFunctionInfo {
-        public:
-            AutoRestoreFunctionInfo(ParseableFunctionInfo *pfi) : pfi(pfi), funcBody(nullptr) {}
-            ~AutoRestoreFunctionInfo() {
-                if (this->pfi != nullptr && this->pfi->GetFunctionInfo()->GetFunctionProxy() != this->pfi)
-                {
-                    FunctionInfo *functionInfo = this->pfi->functionInfo;
-                    functionInfo->SetAttributes(
-                        (FunctionInfo::Attributes)(functionInfo->GetAttributes() | FunctionInfo::Attributes::DeferredParse));
-                    functionInfo->SetFunctionProxy(this->pfi);
-                    functionInfo->SetOriginalEntryPoint(DefaultEntryThunk);
-                }
-
-                Assert(this->pfi == nullptr || (this->pfi->GetFunctionInfo()->GetFunctionProxy() == this->pfi && !this->pfi->IsFunctionBody()));
-            }
-            void Clear() { pfi = nullptr; funcBody = nullptr; }
-
-            ParseableFunctionInfo * pfi;
-            FunctionBody          * funcBody;
-        } autoRestoreFunctionInfo(this);
+        AutoRestoreFunctionInfo autoRestoreFunctionInfo(this, DefaultEntryThunk);
 
         // If m_hasBeenParsed = true, one of the following things happened:
         // - We had multiple function objects which were all defer-parsed, but with the same function body and one of them

lib/Runtime/Base/FunctionBody.h

@@ -3669,6 +3669,30 @@ namespace Js
         StatementAdjustmentRecordList* GetStatementAdjustmentRecords();
     };
 
+    class AutoRestoreFunctionInfo {
+    public:
+        AutoRestoreFunctionInfo(ParseableFunctionInfo *pfi, const JavascriptMethod originalEntryPoint) : pfi(pfi), funcBody(nullptr), originalEntryPoint(originalEntryPoint) {}
+        ~AutoRestoreFunctionInfo() {
+            if (this->pfi != nullptr && this->pfi->GetFunctionInfo()->GetFunctionProxy() != this->pfi)
+            {
+                FunctionInfo *functionInfo = this->pfi->GetFunctionInfo();
+                functionInfo->SetAttributes(
+                    (FunctionInfo::Attributes)(functionInfo->GetAttributes() | FunctionInfo::Attributes::DeferredParse));
+                functionInfo->SetFunctionProxy(this->pfi);
+                functionInfo->SetOriginalEntryPoint(originalEntryPoint);
+            }
+
+            Assert(this->pfi == nullptr || (this->pfi->GetFunctionInfo()->GetFunctionProxy() == this->pfi && !this->pfi->IsFunctionBody()));
+        }
+        void Clear() { pfi = nullptr; funcBody = nullptr; }
+
+        ParseableFunctionInfo * pfi;
+        FunctionBody          * funcBody;
+        const JavascriptMethod originalEntryPoint;
+    };
+
+    // If we throw or fail with the function body in an unfinished state, make sure the function info is still
+    // pointing to the old ParseableFunctionInfo and has the right attributes.
     typedef SynchronizableList<FunctionBody*, JsUtil::List<FunctionBody*, ArenaAllocator, false, Js::FreeListedRemovePolicy> > FunctionBodyList;
 
     struct ScopeSlots

lib/Runtime/ByteCode/ByteCodeGenerator.cpp

@@ -1168,6 +1168,8 @@ FuncInfo * ByteCodeGenerator::StartBindFunction(const char16 *name, uint nameLen
     bool funcExprWithName;
     Js::ParseableFunctionInfo* parseableFunctionInfo = nullptr;
 
+    Js::AutoRestoreFunctionInfo autoRestoreFunctionInfo(reuseNestedFunc, reuseNestedFunc ? reuseNestedFunc->GetOriginalEntryPoint() : nullptr);
+
     if (this->pCurrentFunction &&
         this->pCurrentFunction->IsFunctionParsed())
     {
@@ -1228,7 +1230,7 @@ FuncInfo * ByteCodeGenerator::StartBindFunction(const char16 *name, uint nameLen
         // Create a function body if:
         //  1. The parse node is not defer parsed
         //  2. Or creating function proxies is disallowed
-        bool createFunctionBody = (pnode->sxFnc.pnodeBody != nullptr) && (!reuseNestedFunc || reuseNestedFunc->IsFunctionBody());
+        bool createFunctionBody = (pnode->sxFnc.pnodeBody != nullptr);
         if (!CONFIG_FLAG(CreateFunctionProxy)) createFunctionBody = true;
 
         Js::FunctionInfo::Attributes attributes = Js::FunctionInfo::Attributes::None;
@@ -1278,8 +1280,17 @@ FuncInfo * ByteCodeGenerator::StartBindFunction(const char16 *name, uint nameLen
             propertyRecordList = EnsurePropertyRecordList();
             if (reuseNestedFunc)
             {
-                Assert(reuseNestedFunc->IsFunctionBody());
-                parseableFunctionInfo = reuseNestedFunc->GetFunctionBody();
+                if (!reuseNestedFunc->IsFunctionBody())
+                {
+                    Js::FunctionBody * parsedFunctionBody =
+                        Js::FunctionBody::NewFromParseableFunctionInfo(reuseNestedFunc->GetParseableFunctionInfo());
+                    autoRestoreFunctionInfo.funcBody = parsedFunctionBody;
+                    parseableFunctionInfo = parsedFunctionBody;
+                }
+                else
+                {
+                    parseableFunctionInfo = reuseNestedFunc->GetFunctionBody();
+                }
             }
             else
             {
@@ -1309,7 +1320,7 @@ FuncInfo * ByteCodeGenerator::StartBindFunction(const char16 *name, uint nameLen
             if (reuseNestedFunc)
             {
                 Assert(!reuseNestedFunc->IsFunctionBody() || reuseNestedFunc->GetFunctionBody()->GetByteCode() != nullptr);
-                pnode->sxFnc.pnodeBody = nullptr;
+                Assert(pnode->sxFnc.pnodeBody == nullptr);
                 parseableFunctionInfo = reuseNestedFunc;
             }
             else
@@ -1437,6 +1448,8 @@ FuncInfo * ByteCodeGenerator::StartBindFunction(const char16 *name, uint nameLen
         this->maxAstSize = currentAstSize;
     }
 
+    autoRestoreFunctionInfo.Clear();
+
     return funcInfo;
 }