[MERGE #5213 @sigatrev] OS#17384939: avoid race condition when writing callback info IDL

Merge pull request #5213 from sigatrev:callbackrace

original setup would add elements to the front of the list when profiling instructions, and would iterator over the whole list while writing the IDL. If an element was added between creating the IDL array and beginning the iteration, too many elements would be written and we'd AV.

This changes the setup to add new elements to the end of the list, and limit the number of iterations based on the size of the IDL array created. This should allow elements to be safely added to the list after creating the array. Elements are never removed from the list.

Author: sigatrev

lib/Backend/JITTimeProfileInfo.cpp

@@ -93,20 +93,22 @@ JITTimeProfileInfo::InitializeJITProfileData(
     else
     {
         data->profiledCallbackCount = static_cast<Js::ProfileId>(callbackInfoList->Count());
-        data->callbackData = AnewArrayZ(alloc, CallbackInfoIDL, data->profiledCallbackCount);
-
-        size_t callbackInfoIndex = 0;
-        FOREACH_SLIST_ENTRY(Field(Js::CallbackInfo *), callbackInfo, callbackInfoList)
+        if (data->profiledCallbackCount > 0)
         {
-            memcpy_s(
-                &data->callbackData[callbackInfoIndex],
-                sizeof(CallbackInfoIDL),
-                callbackInfo,
-                sizeof(Js::CallbackInfo)
-            );
-            ++callbackInfoIndex;
+            data->callbackData = AnewArrayZ(alloc, CallbackInfoIDL, data->profiledCallbackCount);
+            Js::CallbackInfoList::Iterator iter = callbackInfoList->GetIterator();
+            for (Js::ProfileId callbackInfoIndex = 0; callbackInfoIndex < data->profiledCallbackCount; ++callbackInfoIndex)
+            {
+                iter.Next();
+                Js::CallbackInfo * info = iter.Data();
+                memcpy_s(
+                    &data->callbackData[callbackInfoIndex],
+                    sizeof(CallbackInfoIDL),
+                    info,
+                    sizeof(Js::CallbackInfo)
+                );
+            }
         }
-        NEXT_SLIST_ENTRY
     }
 
     CompileAssert(sizeof(BVUnitIDL) == sizeof(BVUnit));

lib/Runtime/Language/DynamicProfileInfo.cpp

@@ -476,6 +476,20 @@ namespace Js
         }
     }
 
+    CallbackInfoList::EditingIterator TryFindCallbackInfoIterator(CallbackInfoList * list, ProfileId callSiteId)
+    {
+        CallbackInfoList::EditingIterator iter = list->GetEditingIterator();
+        while (iter.Next())
+        {
+            if (iter.Data()->callSiteId == callSiteId)
+            {
+                return iter;
+            }
+        }
+
+        return iter;
+    }
+
     CallbackInfo * DynamicProfileInfo::FindCallbackInfo(FunctionBody * funcBody, ProfileId callSiteId)
     {
         CallbackInfoList * list = funcBody->GetCallbackInfoList();
@@ -484,14 +498,11 @@ namespace Js
             return nullptr;
         }
 
-        FOREACH_SLIST_ENTRY(Field(CallbackInfo *), callbackInfo, list)
+        CallbackInfoList::EditingIterator iter = TryFindCallbackInfoIterator(list, callSiteId);
+        if (iter.IsValid())
         {
-            if (callbackInfo->callSiteId == callSiteId)
-            {
-                return callbackInfo;
-            }
+            return iter.Data();
         }
-        NEXT_SLIST_ENTRY;
 
         return nullptr;
     }
@@ -506,16 +517,19 @@ namespace Js
             funcBody->SetCallbackInfoList(list);
         }
 
-        CallbackInfo * info = FindCallbackInfo(funcBody, callSiteId);
-        if (info == nullptr)
+        CallbackInfoList::EditingIterator iter = TryFindCallbackInfoIterator(list, callSiteId);
+        if (iter.IsValid())
         {
-            info = RecyclerNewStructZ(funcBody->GetScriptContext()->GetRecycler(), CallbackInfo);
-            info->callSiteId = callSiteId;
-            info->sourceId = NoSourceId;
-            info->canInlineCallback = true;
-            list->Prepend(info);
+            return iter.Data();
         }
 
+        // Callsite is not already in the list, so add it to the end.
+        CallbackInfo * info = info = RecyclerNewStructZ(funcBody->GetScriptContext()->GetRecycler(), CallbackInfo);
+        info->callSiteId = callSiteId;
+        info->sourceId = NoSourceId;
+        info->canInlineCallback = true;
+
+        iter.InsertBefore(info);
         return info;
     }