Skip to content

Commit 746a9ec

Browse files
Minor false positive fixes in ChakraCore
1. BitVectors were not allocated as leaf, and this could cause the GC to interpret it's contents as pointer references. Switched to non-leaf 2. Temporary guest-arenas that were being cached could still get scanned, even if they weren't actively being used. Added a mechanism to unregister said arena when it wasn't used.
1 parent c391c55 commit 746a9ec

File tree

6 files changed

+39
-3
lines changed

6 files changed

+39
-3
lines changed

lib/Backend/NativeCodeData.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ NativeCodeData::Allocator::Alloc(size_t requestSize)
6767
return data;
6868
}
6969

70+
char *
71+
NativeCodeData::Allocator::AllocLeaf(size_t requestSize)
72+
{
73+
return Alloc(requestSize);
74+
}
75+
7076
char *
7177
NativeCodeData::Allocator::AllocZero(size_t requestSize)
7278
{

lib/Backend/NativeCodeData.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class NativeCodeData
3838

3939
char * Alloc(DECLSPEC_GUARD_OVERFLOW size_t requestedBytes);
4040
char * AllocZero(DECLSPEC_GUARD_OVERFLOW size_t requestedBytes);
41+
char * AllocLeaf(__declspec(guard(overflow)) size_t requestedBytes);
42+
4143
NativeCodeData * Finalize();
4244
void Free(void * buffer, size_t byteSize);
4345

lib/Common/DataStructures/FixedBitVector.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,14 @@ template <typename TAllocator>
147147
BVFixed * BVFixed::New(TAllocator * alloc, BVFixed * initBv)
148148
{
149149
BVIndex length = initBv->Length();
150-
BVFixed *result = AllocatorNewPlus(TAllocator, alloc, sizeof(BVUnit) * BVFixed::WordCount(length), BVFixed, initBv);
150+
BVFixed *result = AllocatorNewPlusLeaf(TAllocator, alloc, sizeof(BVUnit) * BVFixed::WordCount(length), BVFixed, initBv);
151151
return result;
152152
}
153153

154154
template <typename TAllocator>
155155
BVFixed * BVFixed::New(DECLSPEC_GUARD_OVERFLOW BVIndex length, TAllocator * alloc, bool initialSet)
156156
{
157-
BVFixed *result = AllocatorNewPlus(TAllocator, alloc, sizeof(BVUnit) * BVFixed::WordCount(length), BVFixed, length, initialSet);
157+
BVFixed *result = AllocatorNewPlusLeaf(TAllocator, alloc, sizeof(BVUnit) * BVFixed::WordCount(length), BVFixed, length, initialSet);
158158
return result;
159159
}
160160

lib/Runtime/Base/TempArenaAllocatorObject.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,31 @@ namespace Js
4343
Assert(allocator.AllocatedSize() == 0);
4444
}
4545

46+
template <bool isGuestArena>
47+
void TempArenaAllocatorWrapper<isGuestArena>::AdviseInUse()
48+
{
49+
if (isGuestArena)
50+
{
51+
if (externalGuestArenaRef == nullptr)
52+
{
53+
externalGuestArenaRef = this->recycler->RegisterExternalGuestArena(this->GetAllocator());
54+
}
55+
}
56+
}
57+
58+
template <bool isGuestArena>
59+
void TempArenaAllocatorWrapper<isGuestArena>::AdviseNotInUse()
60+
{
61+
this->allocator.Reset();
62+
63+
if (isGuestArena)
64+
{
65+
Assert(externalGuestArenaRef != nullptr);
66+
this->recycler->UnregisterExternalGuestArena(externalGuestArenaRef);
67+
externalGuestArenaRef = nullptr;
68+
}
69+
}
70+
4671
// Explicit instantiation
4772
template class TempArenaAllocatorWrapper<true>;
4873
template class TempArenaAllocatorWrapper<false>;

lib/Runtime/Base/TempArenaAllocatorObject.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ namespace Js
1818

1919
public:
2020

21+
void AdviseInUse();
22+
void AdviseNotInUse();
2123

2224
static TempArenaAllocatorWrapper* Create(ThreadContext * threadContext);
2325

lib/Runtime/Base/ThreadContext.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2142,6 +2142,7 @@ ThreadContext::GetTemporaryGuestAllocator(LPCWSTR name)
21422142
{
21432143
temporaryGuestArenaAllocatorCount--;
21442144
Js::TempGuestArenaAllocatorObject * allocator = recyclableData->temporaryGuestArenaAllocators[temporaryGuestArenaAllocatorCount];
2145+
allocator->AdviseInUse();
21452146
recyclableData->temporaryGuestArenaAllocators[temporaryGuestArenaAllocatorCount] = nullptr;
21462147
return allocator;
21472148
}
@@ -2154,7 +2155,7 @@ ThreadContext::ReleaseTemporaryGuestAllocator(Js::TempGuestArenaAllocatorObject
21542155
{
21552156
if (temporaryGuestArenaAllocatorCount < MaxTemporaryArenaAllocators)
21562157
{
2157-
tempGuestAllocator->GetAllocator()->Reset();
2158+
tempGuestAllocator->AdviseNotInUse();
21582159
recyclableData->temporaryGuestArenaAllocators[temporaryGuestArenaAllocatorCount] = tempGuestAllocator;
21592160
temporaryGuestArenaAllocatorCount++;
21602161
return;

0 commit comments

Comments
 (0)