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

pleath · pleath · commit d5c4f4743d81 · 2017-01-09T10:42:19.000-08:00
Merge pull request chakra-core#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.
diff --git a/lib/Runtime/Base/FunctionBody.cpp b/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
diff --git a/lib/Runtime/Base/FunctionBody.h b/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
diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/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;
 }
 
