[MERGE #5339 @MikeHolman] avoid calling GetCallSiteInfo from background

Merge pull request #5339 from MikeHolman:calleeinfobug

GetCallSiteInfo isn't thread safe, so avoid calling it from the background.

It was only ever called in the background from FunctionBody::HasNonBuiltInCallee, which only needs some of the info. So I've made a new method that only reads what we need and put it/updates to CallSiteInfo behind a critical section.

OS: 17904582

Author: MikeHolman

lib/Runtime/Base/FunctionBody.cpp

@@ -7627,10 +7627,7 @@ namespace Js
         for (ProfileId i = 0; i < profiledCallSiteCount; i++)
         {
             Assert(HasDynamicProfileInfo());
-            bool ctor;
-            bool isPolymorphic;
-            FunctionInfo *info = dynamicProfileInfo->GetCallSiteInfo(this, i, &ctor, &isPolymorphic);
-            if (info == nullptr || info->HasBody())
+            if (dynamicProfileInfo->MayHaveNonBuiltinCallee(i))
             {
                 return true;
             }

lib/Runtime/Base/FunctionBody.h

@@ -2754,6 +2754,7 @@ namespace Js
 #if ENABLE_NATIVE_CODEGEN
         void SetPolymorphicCallSiteInfoHead(PolymorphicCallSiteInfo *polyCallSiteInfo) { this->SetAuxPtr<AuxPointerType::PolymorphicCallSiteInfoHead>(polyCallSiteInfo); }
         PolymorphicCallSiteInfo * GetPolymorphicCallSiteInfoHead() { return this->GetAuxPtr<AuxPointerType::PolymorphicCallSiteInfoHead>(); }
+        PolymorphicCallSiteInfo * GetPolymorphicCallSiteInfoWithLock() { return this->GetAuxPtrWithLock<AuxPointerType::PolymorphicCallSiteInfoHead>(); }
 
         void SetCallbackInfoList(CallbackInfoList * callbackInfoList) { this->SetAuxPtr<AuxPointerType::CallbackArgOutInfoList>(callbackInfoList); }
         CallbackInfoList * GetCallbackInfoList() { return this->GetAuxPtr<AuxPointerType::CallbackArgOutInfoList>(); }

lib/Runtime/Language/DynamicProfileInfo.cpp

@@ -53,6 +53,8 @@ namespace Js
         bits = NotNativeIntBit | NotNativeFloatBit;
     }
 
+    CriticalSection DynamicProfileInfo::callSiteInfoCS;
+
     DynamicProfileInfo* DynamicProfileInfo::New(Recycler* recycler, FunctionBody* functionBody, bool persistsAcrossScriptContexts)
     {
         size_t totalAlloc = 0;
@@ -541,12 +543,14 @@ namespace Js
 #ifdef ASMJS_PLAT
     void DynamicProfileInfo::RecordAsmJsCallSiteInfo(FunctionBody* callerBody, ProfileId callSiteId, FunctionBody* calleeBody)
     {
+        AutoCriticalSection cs(&this->callSiteInfoCS);
+
         if (!callerBody || !callerBody->GetIsAsmjsMode() || !calleeBody || !calleeBody->GetIsAsmjsMode())
         {
             AssertMsg(UNREACHED, "Call to RecordAsmJsCallSiteInfo without two asm.js/wasm FunctionBody");
             return;
         }
-        
+
 #if DBG_DUMP || defined(DYNAMIC_PROFILE_STORAGE) || defined(RUNTIME_DATA_COLLECTION)
         // If we persistsAcrossScriptContext, the dynamic profile info may be referred to by multiple function body from
         // different script context
@@ -688,6 +692,8 @@ namespace Js
 
     void DynamicProfileInfo::RecordCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, FunctionInfo* calleeFunctionInfo, JavascriptFunction* calleeFunction, uint actualArgCount, bool isConstructorCall, InlineCacheIndex ldFldInlineCacheId)
     {
+        AutoCriticalSection cs(&this->callSiteInfoCS);
+
 #if DBG_DUMP || defined(DYNAMIC_PROFILE_STORAGE) || defined(RUNTIME_DATA_COLLECTION)
         // If we persistsAcrossScriptContext, the dynamic profile info may be referred to by multiple function body from
         // different script context
@@ -788,6 +794,7 @@ namespace Js
             localPolyCallSiteInfo->functionIds[i] = CallSiteNoInfo;
         }
 
+        Assert(this->callSiteInfoCS.IsLocked());
         callSiteInfo[callSiteId].isPolymorphic = true;
         callSiteInfo[callSiteId].u.polymorphicCallSiteInfo = localPolyCallSiteInfo;
         funcBody->SetPolymorphicCallSiteInfoHead(localPolyCallSiteInfo);
@@ -797,6 +804,8 @@ namespace Js
     {
         if (dynamicProfileFunctionInfo)
         {
+            AutoCriticalSection cs(&this->callSiteInfoCS);
+
             for (ProfileId i = 0; i < dynamicProfileFunctionInfo->callSiteInfoCount; i++)
             {
                 if (callSiteInfo[i].isPolymorphic)
@@ -809,6 +818,8 @@ namespace Js
 
     void DynamicProfileInfo::ResetPolymorphicCallSiteInfo(ProfileId callSiteId, Js::LocalFunctionId functionId)
     {
+        Assert(this->callSiteInfoCS.IsLocked());
+
         callSiteInfo[callSiteId].isPolymorphic = false;
         callSiteInfo[callSiteId].u.functionData.sourceId = CurrentSourceId;
         callSiteInfo[callSiteId].u.functionData.functionId = functionId;
@@ -991,6 +1002,7 @@ namespace Js
 
     FunctionInfo * DynamicProfileInfo::GetFunctionInfo(FunctionBody* functionBody, Js::SourceId sourceId, Js::LocalFunctionId functionId)
     {
+        Assert(ThreadContext::GetContextForCurrentThread());
         if (sourceId == BuiltInSourceId)
         {
             return JavascriptBuiltInFunction::GetFunctionInfo(functionId);
@@ -1048,8 +1060,30 @@ namespace Js
         return nullptr;
     }
 
+    bool DynamicProfileInfo::MayHaveNonBuiltinCallee(ProfileId callSiteId)
+    {
+        AutoCriticalSection cs(&this->callSiteInfoCS);
+
+        if (this->callSiteInfo[callSiteId].dontInline)
+        {
+            return true;
+        }
+
+        if (!this->callSiteInfo[callSiteId].isPolymorphic)
+        {
+            Js::SourceId sourceId = this->callSiteInfo[callSiteId].u.functionData.sourceId;
+            if (sourceId == BuiltInSourceId)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     FunctionInfo * DynamicProfileInfo::GetCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, bool *isConstructorCall, bool *isPolymorphicCall)
     {
+        Assert(ThreadContext::GetContextForCurrentThread());
         Assert(functionBody);
         const auto callSiteCount = functionBody->GetProfiledCallSiteCount();
         Assert(callSiteId < callSiteCount);

lib/Runtime/Language/DynamicProfileInfo.h

@@ -464,6 +464,7 @@ namespace Js
         static bool HasCallSiteInfo(FunctionBody* functionBody);
         bool HasCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId); // Does a particular callsite have ProfileInfo?
         FunctionInfo * GetCallbackInfo(FunctionBody * functionBody, ProfileId callSiteId);
+        bool MayHaveNonBuiltinCallee(ProfileId callSiteId);
         FunctionInfo * GetCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, bool *isConstructorCall, bool *isPolymorphicCall);
         CallSiteInfo * GetCallSiteInfo() const { return callSiteInfo; }
         uint16 GetConstantArgInfo(ProfileId callSiteId);
@@ -607,6 +608,8 @@ namespace Js
         Field(bool) persistsAcrossScriptContexts;
 #endif
 
+        static CriticalSection callSiteInfoCS;
+
         static JavascriptMethod EnsureDynamicProfileInfo(Js::ScriptFunction * function);
 #if DBG_DUMP
         static void DumpList(DynamicProfileInfoList * profileInfoList, ArenaAllocator * dynamicProfileInfoAllocator);
@@ -629,6 +632,7 @@ namespace Js
         void ResetPolymorphicCallSiteInfo(ProfileId callSiteId, Js::LocalFunctionId functionId);
         void SetFunctionIdSlotForNewPolymorphicCall(ProfileId callSiteId, Js::LocalFunctionId curFunctionId, Js::SourceId curSourceId, Js::FunctionBody *inliner);
         void RecordPolymorphicCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, FunctionInfo * calleeFunctionInfo);
+
 #ifdef RUNTIME_DATA_COLLECTION
         static CriticalSection s_csOutput;
         template <typename T>