Skip to content

Commit b8920ed

Browse files
committed
OS#17384939: avoid race condition when writing callback info IDL
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.
1 parent 34967a4 commit b8920ed

File tree

2 files changed

+28
-19
lines changed

2 files changed

+28
-19
lines changed

lib/Backend/JITTimeProfileInfo.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,20 +93,22 @@ JITTimeProfileInfo::InitializeJITProfileData(
9393
else
9494
{
9595
data->profiledCallbackCount = static_cast<Js::ProfileId>(callbackInfoList->Count());
96-
data->callbackData = AnewArrayZ(alloc, CallbackInfoIDL, data->profiledCallbackCount);
97-
98-
size_t callbackInfoIndex = 0;
99-
FOREACH_SLIST_ENTRY(Field(Js::CallbackInfo *), callbackInfo, callbackInfoList)
96+
if (data->profiledCallbackCount > 0)
10097
{
101-
memcpy_s(
102-
&data->callbackData[callbackInfoIndex],
103-
sizeof(CallbackInfoIDL),
104-
callbackInfo,
105-
sizeof(Js::CallbackInfo)
106-
);
107-
++callbackInfoIndex;
98+
data->callbackData = AnewArrayZ(alloc, CallbackInfoIDL, data->profiledCallbackCount);
99+
Js::CallbackInfoList::Iterator iter = callbackInfoList->GetIterator();
100+
for (Js::ProfileId callbackInfoIndex = 0; callbackInfoIndex < data->profiledCallbackCount; ++callbackInfoIndex)
101+
{
102+
iter.Next();
103+
Js::CallbackInfo * info = iter.Data();
104+
memcpy_s(
105+
&data->callbackData[callbackInfoIndex],
106+
sizeof(CallbackInfoIDL),
107+
info,
108+
sizeof(Js::CallbackInfo)
109+
);
110+
}
108111
}
109-
NEXT_SLIST_ENTRY
110112
}
111113

112114
CompileAssert(sizeof(BVUnitIDL) == sizeof(BVUnit));

lib/Runtime/Language/DynamicProfileInfo.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -506,16 +506,23 @@ namespace Js
506506
funcBody->SetCallbackInfoList(list);
507507
}
508508

509-
CallbackInfo * info = FindCallbackInfo(funcBody, callSiteId);
510-
if (info == nullptr)
509+
CallbackInfoList::EditingIterator iter = list->GetEditingIterator();
510+
while (iter.Next())
511511
{
512-
info = RecyclerNewStructZ(funcBody->GetScriptContext()->GetRecycler(), CallbackInfo);
513-
info->callSiteId = callSiteId;
514-
info->sourceId = NoSourceId;
515-
info->canInlineCallback = true;
516-
list->Prepend(info);
512+
Field(CallbackInfo*) callbackInfo = iter.Data();
513+
if (callbackInfo->callSiteId == callSiteId)
514+
{
515+
return callbackInfo;
516+
}
517517
}
518518

519+
// Callsite is not already in the list, so add it to the end.
520+
CallbackInfo * info = info = RecyclerNewStructZ(funcBody->GetScriptContext()->GetRecycler(), CallbackInfo);
521+
info->callSiteId = callSiteId;
522+
info->sourceId = NoSourceId;
523+
info->canInlineCallback = true;
524+
525+
iter.InsertBefore(info);
519526
return info;
520527
}
521528

0 commit comments

Comments
 (0)