Skip to content

Commit cfcbe3b

Browse files
committed
[1.8>1.9] [MERGE #4560 @leirocks] simplify xData deletion
Merge pull request #4560 from leirocks:ftfix change to not using jit code deletion job to do the work, instead always do it in side channel and before new xData registration
2 parents 1d521f5 + c60cf51 commit cfcbe3b

17 files changed

+87
-79
lines changed

lib/Backend/CodeGenWorkItem.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,22 @@ void CodeGenWorkItem::OnWorkItemProcessFail(NativeCodeGenerator* codeGen)
205205
#if DBG
206206
this->allocation->allocation->isNotExecutableBecauseOOM = true;
207207
#endif
208-
codeGen->FreeNativeCodeGenAllocation(this->allocation->allocation->address, nullptr);
208+
209+
#if PDATA_ENABLED & defined(_WIN32)
210+
if (this->entryPointInfo && this->entryPointInfo->GetXDataInfo())
211+
{
212+
void* functionTable = this->entryPointInfo->GetXDataInfo()->functionTable;
213+
if (functionTable)
214+
{
215+
if (!DelayDeletingFunctionTable::AddEntry(functionTable))
216+
{
217+
PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("OnWorkItemProcessFail: Failed to add to slist, table: %llx\n"), functionTable);
218+
DelayDeletingFunctionTable::DeleteFunctionTable(functionTable);
219+
}
220+
}
221+
}
222+
#endif
223+
codeGen->FreeNativeCodeGenAllocation(this->allocation->allocation->address);
209224
}
210225
}
211226

lib/Backend/EmitBuffer.cpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ template <typename TAlloc, typename TPreReservedAlloc, class SyncObject>
5555
void
5656
EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocations(bool release)
5757
{
58+
DelayDeletingFunctionTable::Clear();
5859
AutoRealOrFakeCriticalSection<SyncObject> autoCs(&this->criticalSection);
5960

6061
#if DBG_DUMP
@@ -191,9 +192,10 @@ EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::NewAllocation(size_t b
191192

192193
template <typename TAlloc, typename TPreReservedAlloc, class SyncObject>
193194
bool
194-
EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocation(void* address, void** functionTable)
195+
EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocation(void* address)
195196
{
196197
AutoRealOrFakeCriticalSection<SyncObject> autoCs(&this->criticalSection);
198+
197199
#if _M_ARM
198200
address = (void*)((uintptr_t)address & ~0x1); // clear the thumb bit
199201
#endif
@@ -243,14 +245,6 @@ EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocation(void* a
243245
}
244246
VerboseHeapTrace(_u("Freeing 0x%p, allocation: 0x%p\n"), address, allocation->allocation->address);
245247

246-
#if PDATA_ENABLED && defined(_WIN32)
247-
if (functionTable && *functionTable)
248-
{
249-
NtdllLibrary::Instance->DeleteGrowableFunctionTable(*functionTable);
250-
*functionTable = nullptr;
251-
}
252-
#endif
253-
254248
this->allocationHeap.Free(allocation->allocation);
255249
this->allocator->Free(allocation, sizeof(TEmitBufferAllocation));
256250

lib/Backend/EmitBuffer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class EmitBufferManager
4646
bool ProtectBufferWithExecuteReadWriteForInterpreter(TEmitBufferAllocation* allocation);
4747
bool CommitBufferForInterpreter(TEmitBufferAllocation* allocation, _In_reads_bytes_(bufferSize) BYTE* pBuffer, _In_ size_t bufferSize);
4848
void CompletePreviousAllocation(TEmitBufferAllocation* allocation);
49-
bool FreeAllocation(void* address, void** functionTable);
49+
bool FreeAllocation(void* address);
5050
//Ends here
5151

5252
bool IsInHeap(void* address);

lib/Backend/NativeCodeGenerator.cpp

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,7 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor
11171117
}
11181118

11191119
#if defined(TARGET_64)
1120+
DelayDeletingFunctionTable::Clear();
11201121
XDataAllocation * xdataInfo = HeapNewZ(XDataAllocation);
11211122
xdataInfo->address = (byte*)jitWriteData.xdataAddr;
11221123
XDataAllocator::Register(xdataInfo, jitWriteData.codeAddress, jitWriteData.codeSize);
@@ -3214,14 +3215,14 @@ NativeCodeGenerator::EnterScriptStart()
32143215
}
32153216

32163217
void
3217-
FreeNativeCodeGenAllocation(Js::ScriptContext *scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress, void** functionTable)
3218+
FreeNativeCodeGenAllocation(Js::ScriptContext *scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress)
32183219
{
32193220
if (!scriptContext->GetNativeCodeGenerator())
32203221
{
32213222
return;
32223223
}
32233224

3224-
scriptContext->GetNativeCodeGenerator()->QueueFreeNativeCodeGenAllocation((void*)codeAddress, (void*)thunkAddress, functionTable);
3225+
scriptContext->GetNativeCodeGenerator()->QueueFreeNativeCodeGenAllocation((void*)codeAddress, (void*)thunkAddress);
32253226
}
32263227

32273228
bool TryReleaseNonHiPriWorkItem(Js::ScriptContext* scriptContext, CodeGenWorkItem* workItem)
@@ -3252,30 +3253,22 @@ bool NativeCodeGenerator::TryReleaseNonHiPriWorkItem(CodeGenWorkItem* workItem)
32523253
}
32533254

32543255
void
3255-
NativeCodeGenerator::FreeNativeCodeGenAllocation(void* codeAddress, void** functionTable)
3256+
NativeCodeGenerator::FreeNativeCodeGenAllocation(void* codeAddress)
32563257
{
32573258
if (JITManager::GetJITManager()->IsOOPJITEnabled())
32583259
{
3259-
// function table delete in content process
3260-
#if PDATA_ENABLED && defined(_WIN32)
3261-
if (functionTable && *functionTable)
3262-
{
3263-
NtdllLibrary::Instance->DeleteGrowableFunctionTable(*functionTable);
3264-
*functionTable = nullptr;
3265-
}
3266-
#endif
32673260
ThreadContext * context = this->scriptContext->GetThreadContext();
32683261
HRESULT hr = JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)codeAddress);
32693262
JITManager::HandleServerCallResult(hr, RemoteCallType::MemFree);
32703263
}
32713264
else if(this->backgroundAllocators)
32723265
{
3273-
this->backgroundAllocators->emitBufferManager.FreeAllocation(codeAddress, functionTable);
3266+
this->backgroundAllocators->emitBufferManager.FreeAllocation(codeAddress);
32743267
}
32753268
}
32763269

32773270
void
3278-
NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* codeAddress, void * thunkAddress, void** functionTable)
3271+
NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* codeAddress, void * thunkAddress)
32793272
{
32803273
ASSERT_THREAD();
32813274

@@ -3305,24 +3298,24 @@ NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* codeAddress, void *
33053298
// OOP JIT will always queue a job
33063299

33073300
// The foreground allocators may have been used
3308-
if (this->foregroundAllocators && this->foregroundAllocators->emitBufferManager.FreeAllocation(codeAddress, functionTable))
3301+
if (this->foregroundAllocators && this->foregroundAllocators->emitBufferManager.FreeAllocation(codeAddress))
33093302
{
33103303
return;
33113304
}
33123305

33133306
// The background allocators were used. Queue a job to free the allocation from the background thread.
3314-
this->freeLoopBodyManager.QueueFreeLoopBodyJob(codeAddress, thunkAddress, functionTable);
3307+
this->freeLoopBodyManager.QueueFreeLoopBodyJob(codeAddress, thunkAddress);
33153308
}
33163309

3317-
void NativeCodeGenerator::FreeLoopBodyJobManager::QueueFreeLoopBodyJob(void* codeAddress, void * thunkAddress, void** functionTable)
3310+
void NativeCodeGenerator::FreeLoopBodyJobManager::QueueFreeLoopBodyJob(void* codeAddress, void * thunkAddress)
33183311
{
33193312
Assert(!this->isClosed);
33203313

3321-
FreeLoopBodyJob* job = HeapNewNoThrow(FreeLoopBodyJob, this, codeAddress, thunkAddress, *functionTable);
3314+
FreeLoopBodyJob* job = HeapNewNoThrow(FreeLoopBodyJob, this, codeAddress, thunkAddress);
33223315

33233316
if (job == nullptr)
33243317
{
3325-
FreeLoopBodyJob stackJob(this, codeAddress, thunkAddress, *functionTable, false /* heapAllocated */);
3318+
FreeLoopBodyJob stackJob(this, codeAddress, thunkAddress, false /* heapAllocated */);
33263319

33273320
{
33283321
AutoOptionalCriticalSection lock(Processor()->GetCriticalSection());
@@ -3346,9 +3339,6 @@ void NativeCodeGenerator::FreeLoopBodyJobManager::QueueFreeLoopBodyJob(void* cod
33463339
HeapDelete(job);
33473340
}
33483341
}
3349-
3350-
// function table successfully transferred to background job
3351-
*functionTable = nullptr;
33523342
}
33533343

33543344
#ifdef PROFILE_EXEC

lib/Backend/NativeCodeGenerator.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@ void SetProfileMode(BOOL fSet);
104104
void UpdateQueueForDebugMode();
105105
bool IsBackgroundJIT() const;
106106
void EnterScriptStart();
107-
void FreeNativeCodeGenAllocation(void* codeAddress, void** functionTable);
107+
void FreeNativeCodeGenAllocation(void* codeAddress);
108108
bool TryReleaseNonHiPriWorkItem(CodeGenWorkItem* workItem);
109109

110-
void QueueFreeNativeCodeGenAllocation(void* codeAddress, void* thunkAddress, void** functionTable);
110+
void QueueFreeNativeCodeGenAllocation(void* codeAddress, void* thunkAddress);
111111

112112
bool IsClosed() { return isClosed; }
113113
void AddWorkItem(CodeGenWorkItem* workItem);
@@ -201,19 +201,17 @@ void SetProfileMode(BOOL fSet);
201201
class FreeLoopBodyJob: public JsUtil::Job
202202
{
203203
public:
204-
FreeLoopBodyJob(JsUtil::JobManager *const manager, void* codeAddress, void* thunkAddress, void* functionTable, bool isHeapAllocated = true):
204+
FreeLoopBodyJob(JsUtil::JobManager *const manager, void* codeAddress, void* thunkAddress, bool isHeapAllocated = true):
205205
JsUtil::Job(manager),
206206
codeAddress(codeAddress),
207207
thunkAddress(thunkAddress),
208-
functionTable(functionTable),
209208
heapAllocated(isHeapAllocated)
210209
{
211210
}
212211

213212
bool heapAllocated;
214213
void* codeAddress;
215214
void* thunkAddress;
216-
void* functionTable;
217215
};
218216

219217
class FreeLoopBodyJobManager sealed: public WaitableJobManager
@@ -281,9 +279,8 @@ void SetProfileMode(BOOL fSet);
281279
{
282280
FreeLoopBodyJob* freeLoopBodyJob = static_cast<FreeLoopBodyJob*>(job);
283281

284-
void* functionTable = freeLoopBodyJob->functionTable;
285282
// Free Loop Body
286-
nativeCodeGen->FreeNativeCodeGenAllocation(freeLoopBodyJob->codeAddress, &functionTable);
283+
nativeCodeGen->FreeNativeCodeGenAllocation(freeLoopBodyJob->codeAddress);
287284

288285
return true;
289286
}
@@ -306,7 +303,7 @@ void SetProfileMode(BOOL fSet);
306303
}
307304
}
308305

309-
void QueueFreeLoopBodyJob(void* codeAddress, void* thunkAddress, void** functionTable);
306+
void QueueFreeLoopBodyJob(void* codeAddress, void* thunkAddress);
310307

311308
private:
312309
NativeCodeGenerator* nativeCodeGen;

lib/Backend/PDataManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ void PDataManager::UnregisterPdata(RUNTIME_FUNCTION* pdata)
4646
if (AutoSystemInfo::Data.IsWin8OrLater())
4747
{
4848
// TODO: need to move to background?
49-
NtdllLibrary::Instance->DeleteGrowableFunctionTable(pdata);
49+
DelayDeletingFunctionTable::DeleteFunctionTable(pdata);
5050
}
5151
else
5252
{

lib/Common/BackendApi.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ void UpdateNativeCodeGeneratorForDebugMode(NativeCodeGenerator* nativeCodeGen);
6262
CriticalSection *GetNativeCodeGenCriticalSection(NativeCodeGenerator *pNativeCodeGen);
6363
bool TryReleaseNonHiPriWorkItem(Js::ScriptContext* scriptContext, CodeGenWorkItem* workItem);
6464
void NativeCodeGenEnterScriptStart(NativeCodeGenerator * nativeCodeGen);
65-
void FreeNativeCodeGenAllocation(Js::ScriptContext* scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress, void** functionTable);
65+
void FreeNativeCodeGenAllocation(Js::ScriptContext* scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress);
6666
InProcCodeGenAllocators* GetForegroundAllocator(NativeCodeGenerator * nativeCodeGen, PageAllocator* pageallocator);
6767
void GenerateFunction(NativeCodeGenerator * nativeCodeGen, Js::FunctionBody * functionBody, Js::ScriptFunction * function = NULL);
6868
void GenerateLoopBody(NativeCodeGenerator * nativeCodeGen, Js::FunctionBody * functionBody, Js::LoopHeader * loopHeader, Js::EntryPointInfo* entryPointInfo, uint localCount, Js::Var localSlots[]);

lib/Common/ConfigFlagsList.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ PHASE(All)
371371
PHASE(DeferSourceLoad)
372372
PHASE(ObjectMutationBreakpoint)
373373
PHASE(NativeCodeData)
374+
PHASE(XData)
374375
#undef PHASE
375376
#endif
376377

lib/Common/Core/DelayLoadLibrary.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,17 @@ DWORD NtdllLibrary::AddGrowableFunctionTable( _Out_ PVOID * DynamicTable,
8282
return 1;
8383
}
8484
}
85-
return addGrowableFunctionTable(DynamicTable,
85+
DWORD status = addGrowableFunctionTable(DynamicTable,
8686
FunctionTable,
8787
EntryCount,
8888
MaximumEntryCount,
8989
RangeBase,
9090
RangeEnd);
91+
#if _M_X64
92+
PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("Register: Begin: %llx, End: %x, Unwind: %llx, RangeBase: %llx, RangeEnd: %llx, table: %llx, Status: %x\n"),
93+
FunctionTable->BeginAddress, FunctionTable->EndAddress, FunctionTable->UnwindInfoAddress, RangeBase, RangeEnd, *DynamicTable, status);
94+
#endif
95+
return status;
9196
}
9297
return 1;
9398
}
@@ -107,6 +112,8 @@ VOID NtdllLibrary::DeleteGrowableFunctionTable( _In_ PVOID DynamicTable )
107112
}
108113
}
109114
deleteGrowableFunctionTable(DynamicTable);
115+
116+
PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("UnRegister: table: %llx\n"), DynamicTable);
110117
}
111118
}
112119

lib/Common/Memory/DelayDeletingFunctionTable.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void DelayDeletingFunctionTable::Clear()
6464
while (entry)
6565
{
6666
FunctionTableNode* list = (FunctionTableNode*)entry;
67-
NtdllLibrary::Instance->DeleteGrowableFunctionTable(list->functionTable);
67+
DeleteFunctionTable(list->functionTable);
6868
_aligned_free(entry);
6969
entry = InterlockedPopEntrySList(Head);
7070
}
@@ -79,4 +79,11 @@ bool DelayDeletingFunctionTable::IsEmpty()
7979
#else
8080
return true;
8181
#endif
82+
}
83+
84+
void DelayDeletingFunctionTable::DeleteFunctionTable(void* functionTable)
85+
{
86+
#if PDATA_ENABLED && defined(_WIN32)
87+
NtdllLibrary::Instance->DeleteGrowableFunctionTable(functionTable);
88+
#endif
8289
}

0 commit comments

Comments
 (0)