[1.10>master] [MERGE #5581 @pleath] OS#15992595: On debug reparse, update entry points associated with all parsed functions, not just the ones associated with function objects

Merge pull request #5581 from pleath:15992595

Object may evolve after reparse and use types that are not referenced at reparse time.

Author: pleath

lib/Runtime/Base/FunctionBody.cpp

@@ -2438,17 +2438,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
@@ -3555,6 +3548,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();
@@ -5227,10 +5237,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

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

lib/Runtime/Base/ScriptContext.cpp

@@ -4101,7 +4101,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();
 
@@ -4173,26 +4172,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,