Skip to content

Commit de658ad

Browse files
committed
[MERGE #731] Fixing debug-only race condition during RecordNativeCodeSize
Merge pull request #731 from satheeshravi:BugFixPA The VirtualQuery in CodeGenWorkItem is unprotected without lock. Hence the assert that is checking for the protect state of the page might not always be true. Fix: Moved the VirtualQuery to EmitBuffer under a lock. This query is still debug-only.
2 parents 4888e27 + 8e82c37 commit de658ad

File tree

3 files changed

+16
-6
lines changed

3 files changed

+16
-6
lines changed

lib/Backend/CodeGenWorkItem.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,7 @@ void CodeGenWorkItem::RecordNativeCodeSize(Func *func, size_t bytes, ushort pdat
206206
bool canAllocInPreReservedHeapPageSegment = func->CanAllocInPreReservedHeapPageSegment();
207207
#endif
208208
EmitBufferAllocation *allocation = func->GetEmitBufferManager()->AllocateBuffer(bytes, &buffer, pdataCount, xdataSize, canAllocInPreReservedHeapPageSegment, true);
209-
210-
#if DBG
211-
MEMORY_BASIC_INFORMATION memBasicInfo;
212-
size_t resultBytes = VirtualQuery(allocation->allocation->address, &memBasicInfo, sizeof(memBasicInfo));
213-
Assert(resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE);
214-
#endif
209+
Assert(func->GetEmitBufferManager()->IsBufferExecuteReadOnly(allocation));
215210

216211
Assert(allocation != nullptr);
217212
if (buffer == nullptr)

lib/Backend/EmitBuffer.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,17 @@ bool EmitBufferManager<SyncObject>::CheckCommitFaultInjection()
327327

328328
#endif
329329

330+
#if DBG
331+
template <typename SyncObject>
332+
bool EmitBufferManager<SyncObject>::IsBufferExecuteReadOnly(EmitBufferAllocation * allocation)
333+
{
334+
AutoRealOrFakeCriticalSection<SyncObject> autoCs(&this->criticalSection);
335+
MEMORY_BASIC_INFORMATION memBasicInfo;
336+
size_t resultBytes = VirtualQuery(allocation->allocation->address, &memBasicInfo, sizeof(memBasicInfo));
337+
return resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE;
338+
}
339+
#endif
340+
330341
template <typename SyncObject>
331342
bool EmitBufferManager<SyncObject>::ProtectBufferWithExecuteReadWriteForInterpreter(EmitBufferAllocation* allocation)
332343
{

lib/Backend/EmitBuffer.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ class EmitBufferManager
5555
void CheckBufferPermissions(EmitBufferAllocation *allocation);
5656
#endif
5757

58+
#if DBG
59+
bool IsBufferExecuteReadOnly(EmitBufferAllocation * allocation);
60+
#endif
61+
5862
EmitBufferAllocation * allocations;
5963

6064
private:

0 commit comments

Comments
 (0)