Skip to content

Commit af4ac8e

Browse files
author
Jianchun Xu
committed
[2.0>master] [MERGE #2660 @jianchun] remove global operator new/delete override
Merge pull request #2660 from jianchun:newdel Global operator new/delete override replaces the default used by hosts unintendedly (not sure if there is reliable way to avoid that). This change removes the global overrides and replaces all ChakraCore library new/delete calls with HeapNew/HeapDelete. HeapDelete: Added an optional flag "AllocatorDeleteFlags::UnknownSize" for cases of unknown size to avoid size check. Default still checks size. JITType.h: Add "const" to avoid an unintended copy constructor call, which causes software write barrier access failure in JIT thread (not expecting write barrier access on stack value).
2 parents edd1d35 + 04eb321 commit af4ac8e

19 files changed

+213
-105
lines changed

‎lib/Backend/BackendApi.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,5 +197,8 @@ SetProfilerFromNativeCodeGen(NativeCodeGenerator * toNativeCodeGen, NativeCodeGe
197197

198198
void DeleteNativeCodeData(NativeCodeData * data)
199199
{
200-
delete data;
200+
if (data)
201+
{
202+
HeapDelete(data);
203+
}
201204
}

‎lib/Backend/JITType.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class JITTypeHolderBase
3939
JITTypeHolderBase(JITType * t);
4040

4141
template <class S>
42-
JITTypeHolderBase(JITTypeHolderBase<S> other) : t(PointerValue(other.t)) {}
42+
JITTypeHolderBase(const JITTypeHolderBase<S>& other) : t(PointerValue(other.t)) {}
4343

4444
template <class S>
4545
void operator =(const JITTypeHolderBase<S> &other) { t = other.t; }

‎lib/Backend/NativeCodeData.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//-------------------------------------------------------------------------------------------------------
55
#include "Backend.h"
66

7-
NativeCodeData::NativeCodeData(DataChunk * chunkList)
7+
NativeCodeData::NativeCodeData(DataChunk * chunkList)
88
: chunkList(chunkList)
99
{
1010
#ifdef PERF_COUNTERS
@@ -172,7 +172,7 @@ NativeCodeData::VerifyExistFixupEntry(void* targetAddr, void* addrToFixup, void*
172172
{
173173
if (entry->addrOffset == offset)
174174
{
175-
// The following assertions can be false positive in case a data field happen to
175+
// The following assertions can be false positive in case a data field happen to
176176
// have value fall into NativeCodeData memory range
177177
AssertMsg(entry->targetTotalOffset == targetChunk->offset, "Missing fixup");
178178
return;
@@ -191,12 +191,14 @@ NativeCodeData::DeleteChunkList(DataChunkT * chunkList)
191191
{
192192
DataChunkT * current = next;
193193
next = next->next;
194-
delete current;
194+
195+
// TODO: Should be HeapDeletePlus, but we don't know plusSize
196+
HeapDelete(current, AllocatorDeleteFlags::UnknownSize);
195197
}
196198
}
197199

198-
NativeCodeData::Allocator::Allocator()
199-
: chunkList(nullptr),
200+
NativeCodeData::Allocator::Allocator()
201+
: chunkList(nullptr),
200202
lastChunkList(nullptr),
201203
isOOPJIT(JITManager::GetJITManager()->IsJITServer())
202204
{
@@ -229,7 +231,7 @@ char *
229231
NativeCodeData::Allocator::Alloc(DECLSPEC_GUARD_OVERFLOW size_t requestSize)
230232
{
231233
Assert(!finalized);
232-
char * data = nullptr;
234+
char * data = nullptr;
233235
requestSize = Math::Align(requestSize, sizeof(void*));
234236

235237
if (isOOPJIT)
@@ -240,9 +242,9 @@ NativeCodeData::Allocator::Alloc(DECLSPEC_GUARD_OVERFLOW size_t requestSize)
240242
// positive while verifying missing fixup entries
241243
// Allocation without zeroing out, and with bool field in the structure
242244
// will increase the chance of false positive because of reusing memory
243-
// without zeroing, and the bool field is set to false, makes the garbage
244-
// memory not changed, and the garbage memory might be just pointing to the
245-
// same range of NativeCodeData memory, the checking tool will report false
245+
// without zeroing, and the bool field is set to false, makes the garbage
246+
// memory not changed, and the garbage memory might be just pointing to the
247+
// same range of NativeCodeData memory, the checking tool will report false
246248
// poisitive, see NativeCodeData::VerifyExistFixupEntry for more
247249
DataChunk * newChunk = HeapNewStructPlusZ(requestSize, DataChunk);
248250
#else
@@ -304,7 +306,7 @@ NativeCodeData::Allocator::AllocZero(DECLSPEC_GUARD_OVERFLOW size_t requestSize)
304306
// Allocated with HeapNewStructPlusZ for chk build
305307
memset(data, 0, requestSize);
306308
#else
307-
if (!isOOPJIT)
309+
if (!isOOPJIT)
308310
{
309311
memset(data, 0, requestSize);
310312
}

‎lib/Backend/NativeCodeGenerator.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -953,13 +953,13 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor
953953
pNumberAllocator,
954954
#endif
955955
codeGenProfiler, !foreground);
956-
956+
957957
if (!this->scriptContext->GetThreadContext()->GetPreReservedVirtualAllocator()->IsInRange((void*)jitWriteData.codeAddress))
958958
{
959959
this->scriptContext->GetJitFuncRangeCache()->AddFuncRange((void*)jitWriteData.codeAddress, jitWriteData.codeSize);
960960
}
961961
}
962-
962+
963963
if (JITManager::GetJITManager()->IsOOPJITEnabled() && PHASE_VERBOSE_TRACE(Js::BackEndPhase, workItem->GetFunctionBody()))
964964
{
965965
LARGE_INTEGER freq;
@@ -1200,7 +1200,7 @@ void NativeCodeGenerator::LogCodeGenStart(CodeGenWorkItem * workItem, LARGE_INTE
12001200
size_t sizeInChars = workItem->GetDisplayName(displayName, 256);
12011201
if (sizeInChars > 256)
12021202
{
1203-
displayName = new WCHAR[sizeInChars];
1203+
displayName = HeapNewArray(WCHAR, sizeInChars);
12041204
workItem->GetDisplayName(displayName, 256);
12051205
}
12061206
JS_ETW(EventWriteJSCRIPT_FUNCTION_JIT_START(
@@ -1215,7 +1215,7 @@ void NativeCodeGenerator::LogCodeGenStart(CodeGenWorkItem * workItem, LARGE_INTE
12151215

12161216
if (displayName != displayNameBuffer)
12171217
{
1218-
delete[] displayName;
1218+
HeapDeleteArray(sizeInChars, displayName);
12191219
}
12201220
}
12211221
}
@@ -1317,7 +1317,7 @@ void NativeCodeGenerator::LogCodeGenDone(CodeGenWorkItem * workItem, LARGE_INTEG
13171317
size_t sizeInChars = workItem->GetDisplayName(displayName, 256);
13181318
if (sizeInChars > 256)
13191319
{
1320-
displayName = new WCHAR[sizeInChars];
1320+
displayName = HeapNewArray(WCHAR, sizeInChars);
13211321
workItem->GetDisplayName(displayName, 256);
13221322
}
13231323
void* entryPoint;
@@ -1333,7 +1333,7 @@ void NativeCodeGenerator::LogCodeGenDone(CodeGenWorkItem * workItem, LARGE_INTEG
13331333

13341334
if (displayName != displayNameBuffer)
13351335
{
1336-
delete[] displayName;
1336+
HeapDeleteArray(sizeInChars, displayName);
13371337
}
13381338
}
13391339
}
@@ -3196,7 +3196,7 @@ NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* address)
31963196
//DeRegister Entry Point for CFG
31973197
ThreadContext::GetContextForCurrentThread()->SetValidCallTargetForCFG(address, false);
31983198
}
3199-
3199+
32003200
if ((!JITManager::GetJITManager()->IsOOPJITEnabled() && !this->scriptContext->GetThreadContext()->GetPreReservedVirtualAllocator()->IsInRange((void*)address)) ||
32013201
(JITManager::GetJITManager()->IsOOPJITEnabled() && !PreReservedVirtualAllocWrapper::IsInRange((void*)this->scriptContext->GetThreadContext()->GetPreReservedRegionAddr(), (void*)address)))
32023202
{

‎lib/Common/Core/StackBackTrace.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ template <class T, LONG count, bool useStatic>
141141
struct _TraceRingBuffer
142142
{
143143
T* buf;
144-
_TraceRingBuffer() { buf = new T[count]; }
145-
~_TraceRingBuffer() { delete[] buf; }
144+
_TraceRingBuffer() { buf = HeapNewArray(T, count); }
145+
~_TraceRingBuffer() { HeapDeleteArray(count, buf); }
146146
};
147147
template <class T, LONG count>
148148
struct _TraceRingBuffer<T, count, true>

‎lib/Common/DataStructures/DictionaryStats.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ DictionaryStats* DictionaryStats::Create(const char* name, uint bucketCount)
1515
Js::Configuration::Global.flags.ProfileDictionary < 0)
1616
return NULL;
1717

18-
return ::new DictionaryStats(name, bucketCount);
18+
return NoCheckHeapNew(DictionaryStats, name, bucketCount);
1919
}
2020

2121
DictionaryStats* DictionaryStats::Clone()
@@ -238,10 +238,10 @@ void DictionaryStats::ClearStats()
238238
{
239239
DictionaryStats *pCurrent = pNext;
240240
pNext = pNext->pNext;
241-
delete pCurrent;
241+
NoCheckHeapDelete(pCurrent);
242242
}
243243
current = current->pNext;
244-
delete type;
244+
NoCheckHeapDelete(type);
245245
}
246246
dictionaryTypes = NULL;
247247
}

‎lib/Common/DataStructures/ImmutableList.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,16 @@ void regex::ImmutableStringBuilder<chunkSize>::AppendWithCopy(_In_z_ LPCWSTR str
5050
AssertMsg(str != nullptr, "str != nullptr");
5151
size_t strLength = wcslen(str) + 1; // include null-terminated
5252

53-
WCHAR* buffer = new WCHAR[strLength];
53+
WCHAR* buffer = HeapNewNoThrowArray(WCHAR, strLength);
5454
IfNullThrowOutOfMemory(buffer);
5555
wcsncpy_s(buffer, strLength, str, strLength);
5656

5757
// append in front of the tracked allocated strings
58-
AllocatedStringChunk* newAllocatedString = new AllocatedStringChunk();
58+
AllocatedStringChunk* newAllocatedString = HeapNewNoThrow(AllocatedStringChunk);
5959
if (newAllocatedString == nullptr)
6060
{
6161
// cleanup
62-
delete[] buffer;
62+
HeapDeleteArray(strLength, buffer);
6363
Js::Throw::OutOfMemory();
6464
}
6565

‎lib/Common/DataStructures/ImmutableList.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ namespace regex
678678
T ** ToReferenceArrayInHeap(size_t size)
679679
{
680680
Assert(size == Count());
681-
auto result = new T *[size];
681+
auto result = HeapNewNoThrowArray(T*, size);
682682
IfNullThrowOutOfMemory(result);
683683

684684
auto current = this;
@@ -717,7 +717,7 @@ namespace regex
717717
current = current->next;
718718
}
719719
current->next = nullptr;
720-
delete []arr;
720+
HeapDeleteArray(size, arr);
721721
return result;
722722
}
723723

@@ -744,7 +744,7 @@ namespace regex
744744
current->next = result;
745745
result = current;
746746
}
747-
delete []arr;
747+
HeapDeleteArray(size, arr);
748748
return result;
749749
}
750750

@@ -946,8 +946,11 @@ namespace regex
946946
{
947947
nextAllocatedStringChunk = allocatedStringChunk->next;
948948

949-
delete[] allocatedStringChunk->dataPtr;
950-
delete allocatedStringChunk;
949+
// TODO: Should be HeapDeleteArray, but don't know size
950+
HeapDelete(allocatedStringChunk->dataPtr,
951+
AllocatorDeleteFlags::UnknownSize);
952+
953+
HeapDelete(allocatedStringChunk);
951954

952955
allocatedStringChunk = nextAllocatedStringChunk;
953956
}
@@ -956,7 +959,7 @@ namespace regex
956959
{
957960
auto current = head;
958961
head = head->next;
959-
delete current;
962+
HeapDelete(current);
960963
}
961964
}
962965

@@ -987,7 +990,7 @@ namespace regex
987990
// Generate new chunk
988991
if (currentIndex == chunkSize)
989992
{
990-
StringChunk *newChunk = new StringChunk();
993+
StringChunk *newChunk = HeapNewNoThrow(StringChunk);
991994
IfNullThrowOutOfMemory(newChunk);
992995

993996
if (tail == nullptr)

0 commit comments

Comments
 (0)