OS#15992595: On debug reparse, update all entry points associated with all parsed functions, not just the ones associated with function objects. Object may evolve after reparse and use types that are not referenced at reparse time.

Author: pleath

lib/Runtime/Base/FunctionBody.cpp

@@ -2406,17 +2406,10 @@ namespace Js
 
                     if (isDebugOrAsmJsReparse)
                     {
-                        grfscr &= ~fscrWillDeferFncParse; // Disable deferred parsing if not DeferNested, or doing a debug/asm.js re-parse
-                    }
-
-                    if (isDebugOrAsmJsReparse)
-                    {
-                        grfscr |= fscrNoAsmJs; // Disable asm.js when debugging or if linking failed
-                    }
-
-                    if (isDebugOrAsmJsReparse)
-                    {
-                        grfscr &= ~fscrCreateParserState; // Disable parser state cache if we're debugging or reparsing asm.js
+                        // Disable deferred parsing if not DeferNested, or doing a debug/asm.js re-parse
+                        // Disable asm.js when debugging or if linking failed
+                        // Disable parser state cache if we're debugging or reparsing asm.js
+                        grfscr = fscrNoAsmJs | (grfscr & ~(fscrWillDeferFncParse | fscrCreateParserState));
                     }
 
                     BEGIN_TRANSLATE_EXCEPTION_TO_HRESULT
@@ -3520,6 +3513,23 @@ namespace Js
 #endif
     }
 
+    void FunctionBody::UpdateEntryPointsOnDebugReparse()
+    {
+        // Update all function types associated with this function body. Note that we can't rely on updating
+        // types pointed to by function objects, because the type may evolve to one that is not currently referenced.
+
+        ProxyEntryPointInfo * entryPointInfo = this->GetDefaultFunctionEntryPointInfo();
+        JavascriptMethod newEntryPoint = this->GetDirectEntryPoint(entryPointInfo);
+        bool isAsmJS = this->GetIsAsmjsMode();
+
+        auto updateOneType = [&](ScriptFunctionType* functionType) {
+            // Note that the ScriptFunctionType method will handle cross-site thunks correctly.
+            functionType->ChangeEntryPoint(entryPointInfo, newEntryPoint, isAsmJS);
+        };
+
+        this->MapFunctionObjectTypes(updateOneType);
+    }
+
     void FunctionProxy::Finalize(bool isShutdown)
     {
         this->CleanupFunctionProxyCounters();
@@ -5188,10 +5198,6 @@ namespace Js
             }
 
             this->SetOriginalEntryPoint(DefaultDeferredParsingThunk);
-
-            // Abandon the shared type so a new function will get a new one
-            this->deferredPrototypeType = nullptr;
-            this->undeferredFunctionType = nullptr;
             this->SetAttributes((FunctionInfo::Attributes) (this->GetAttributes() | FunctionInfo::Attributes::DeferredParse));
         }
 

lib/Runtime/Base/FunctionBody.h

@@ -3213,6 +3213,8 @@ namespace Js
         void ClearEntryPoints();
         void ResetEntryPoint();
         void CleanupToReparseHelper();
+        void UpdateEntryPointsOnDebugReparse();
+
         void AddDeferParseAttribute();
         void RemoveDeferParseAttribute();
 #if DBG

lib/Runtime/Base/ScriptContext.cpp

@@ -4021,7 +4021,6 @@ namespace Js
         // Wrapped function are not allocated with the EnumClass bit
         Assert(pFunction->GetFunctionInfo() != &JavascriptExternalFunction::EntryInfo::WrappedFunctionThunk);
 
-        JavascriptMethod entryPoint = pFunction->GetEntryPoint();
         FunctionInfo * info = pFunction->GetFunctionInfo();
         FunctionProxy * proxy = info->GetFunctionProxy();
 
@@ -4093,26 +4092,13 @@ namespace Js
         }
 #endif
 
-        ScriptFunction * scriptFunction = ScriptFunction::FromVar(pFunction);
-
 #ifdef ASMJS_PLAT
+        ScriptFunction * scriptFunction = ScriptFunction::FromVar(pFunction);
         scriptContext->TransitionEnvironmentForDebugger(scriptFunction);
 #endif
-
-        JavascriptMethod newEntryPoint;
-        if (CrossSite::IsThunk(entryPoint))
-        {
-            // Can't change from cross-site to non-cross-site, but still need to update the e.p.info on ScriptFunctionType.
-            newEntryPoint = entryPoint;
-        }
-        else
-        {
-            newEntryPoint = pBody->GetDirectEntryPoint(pBody->GetDefaultFunctionEntryPointInfo());
-        }
-
-        scriptFunction->ChangeEntryPoint(pBody->GetDefaultFunctionEntryPointInfo(), newEntryPoint);
     }
-#endif
+
+#endif // ENABLE_SCRIPT_DEBUGGING
 
 #if defined(ENABLE_SCRIPT_PROFILING) || defined(ENABLE_SCRIPT_DEBUGGING)
     void ScriptContext::RecyclerEnumClassEnumeratorCallback(void *address, size_t size)

lib/Runtime/Debug/DebugContext.cpp

@@ -238,8 +238,10 @@ namespace Js
                         {
                             pFuncBody->ReinitializeExecutionModeAndLimits();
                         }
+                        pFuncBody->UpdateEntryPointsOnDebugReparse();
                     });
                 }
+
                 return false;
             }
 
@@ -330,6 +332,7 @@ namespace Js
                     {
                         pFuncBody->ReinitializeExecutionModeAndLimits();
                     }
+                    pFuncBody->UpdateEntryPointsOnDebugReparse();
                 });
             }
 

lib/Runtime/Library/ScriptFunction.cpp

@@ -210,48 +210,15 @@ using namespace Js;
 
     void ScriptFunction::ChangeEntryPoint(ProxyEntryPointInfo* entryPointInfo, JavascriptMethod entryPoint)
     {
-        Assert(entryPoint != nullptr);
         Assert(this->GetTypeId() == TypeIds_Function);
 #if ENABLE_NATIVE_CODEGEN
         Assert(!IsCrossSiteObject() || entryPoint != (Js::JavascriptMethod)checkCodeGenThunk);
 #endif
 
         Assert((entryPointInfo != nullptr && this->GetFunctionProxy() != nullptr));
-        if (this->GetEntryPoint() == entryPoint && this->GetScriptFunctionType()->GetEntryPointInfo() == entryPointInfo)
-        {
-            return;
-        }
 
         bool isAsmJS = HasFunctionBody() && this->GetFunctionBody()->GetIsAsmjsMode();
-
-        // ASMJS:- for asmjs we don't need to update the entry point here as it updates the types entry point
-        if (!isAsmJS)
-        {
-            // We can't go from cross-site to non-cross-site. Update only in the non-cross site case
-            if (!CrossSite::IsThunk(this->GetEntryPoint()))
-            {
-                this->SetEntryPoint(entryPoint);
-            }
-        }
-        // instead update the address in the function entrypoint info
-        else
-        {
-            entryPointInfo->jsMethod = entryPoint;
-        }
-
-        ProxyEntryPointInfo* oldEntryPointInfo = this->GetScriptFunctionType()->GetEntryPointInfo();
-        if (oldEntryPointInfo
-            && oldEntryPointInfo != entryPointInfo
-            && oldEntryPointInfo->SupportsExpiration())
-        {
-            // The old entry point could be executing so we need root it to make sure
-            // it isn't prematurely collected. The rooting is done by queuing it up on the threadContext
-            ThreadContext* threadContext = ThreadContext::GetContextForCurrentThread();
-
-            threadContext->QueueFreeOldEntryPointInfoIfInScript((FunctionEntryPointInfo*)oldEntryPointInfo);
-        }
-
-        this->GetScriptFunctionType()->SetEntryPointInfo(entryPointInfo);
+        this->GetScriptFunctionType()->ChangeEntryPoint(entryPointInfo, entryPoint, isAsmJS);
     }
 
     FunctionProxy * ScriptFunction::GetFunctionProxy() const

lib/Runtime/Types/ScriptFunctionType.cpp

@@ -34,4 +34,43 @@ namespace Js
             library->ScriptFunctionTypeHandler(!proxy->IsConstructor(), proxy->GetIsAnonymousFunction()),
             isShared, isShared);
     }
+
+    void ScriptFunctionType::ChangeEntryPoint(ProxyEntryPointInfo * entryPointInfo, JavascriptMethod entryPoint, bool isAsmJS)
+    {
+        Assert(entryPoint != nullptr);
+        Assert(entryPointInfo != nullptr);
+        if (this->GetEntryPoint() == entryPoint && this->GetEntryPointInfo() == entryPointInfo)
+        {
+            return;
+        }
+
+        // ASMJS:- for asmjs we don't need to update the entry point here as it updates the types entry point
+        if (!isAsmJS)
+        {
+            // We can't go from cross-site to non-cross-site. Update only in the non-cross site case
+            if (!CrossSite::IsThunk(this->GetEntryPoint()))
+            {
+                this->SetEntryPoint(entryPoint);
+            }
+        }
+        // instead update the address in the function entrypoint info
+        else
+        {
+            entryPointInfo->jsMethod = entryPoint;
+        }
+
+        ProxyEntryPointInfo* oldEntryPointInfo = this->GetEntryPointInfo();
+        if (oldEntryPointInfo
+            && oldEntryPointInfo != entryPointInfo
+            && oldEntryPointInfo->SupportsExpiration())
+        {
+            // The old entry point could be executing so we need root it to make sure
+            // it isn't prematurely collected. The rooting is done by queuing it up on the threadContext
+            ThreadContext* threadContext = ThreadContext::GetContextForCurrentThread();
+
+            threadContext->QueueFreeOldEntryPointInfoIfInScript((FunctionEntryPointInfo*)oldEntryPointInfo);
+        }
+
+        this->SetEntryPointInfo(entryPointInfo);
+    }
 };

lib/Runtime/Types/ScriptFunctionType.h

@@ -13,6 +13,7 @@ namespace Js
         static DWORD GetEntryPointInfoOffset() { return offsetof(ScriptFunctionType, entryPointInfo); }
         ProxyEntryPointInfo * GetEntryPointInfo() const { return entryPointInfo; }
         void SetEntryPointInfo(ProxyEntryPointInfo * entryPointInfo) { this->entryPointInfo = entryPointInfo; }
+        void ChangeEntryPoint(ProxyEntryPointInfo * entryPointInfo, JavascriptMethod entryPoint, bool isAsmJS);
     private:
         ScriptFunctionType(ScriptFunctionType * type);
         ScriptFunctionType(ScriptContext* scriptContext, RecyclableObject* prototype,