Skip to content

Commit 365e8ed

Browse files
committed
[MERGE #5403 @sanketj] Scan interior pointers from GC stack roots when hosted by a native GC host
Merge pull request #5403 from sanketj:user/sajos/scan_interior_pointers_from_stack_for_native_gc_host Currently, we skip scanning interior pointers from GC stack roots if we are not running script (or if we are in mem protect mode). When host objects are also placed on the Chakra heap for GC purposes, we must scan interior pointers from the stack even when not running script. To scope this fix to just native GC host scenarios, and to avoid regressing JSRT scenarios, we are introducing a bit on the recycler that will be set for native GC hosts. If this bit is set, we will always scan interior pointers from the stack, and if it is not set, we will continue doing what we do currently. In order to set this bit, an API is being exposed on the JS thread service that is published and can be called by the host.
2 parents 210bbe3 + e6e7b3d commit 365e8ed

File tree

2 files changed

+20
-4
lines changed

2 files changed

+20
-4
lines changed

lib/Common/Memory/Recycler.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ Recycler::Recycler(AllocationPolicyManager * policyManager, IdleDecommitPageAllo
191191
allowDispose(false),
192192
inDisposeWrapper(false),
193193
hasDisposableObject(false),
194+
hasNativeGCHost(false),
194195
tickCountNextDispose(0),
195196
transientPinnedObject(nullptr),
196197
pinnedObjectMap(1024, HeapAllocator::GetNoMemProtectInstance()),
@@ -937,6 +938,18 @@ Recycler::SetIsInScript(bool isInScript)
937938
this->isInScript = isInScript;
938939
}
939940

941+
bool
942+
Recycler::HasNativeGCHost() const
943+
{
944+
return this->hasNativeGCHost;
945+
}
946+
947+
void
948+
Recycler::SetHasNativeGCHost()
949+
{
950+
this->hasNativeGCHost = true;
951+
}
952+
940953
bool
941954
Recycler::NeedOOMRescan() const
942955
{
@@ -1680,7 +1693,7 @@ Recycler::ScanStack()
16801693

16811694
BEGIN_DUMP_OBJECT(this, _u("Registers"));
16821695
// We will not scan interior pointers on stack if we are not in script or we are in mem-protect mode.
1683-
if (!this->isInScript || this->IsMemProtectMode())
1696+
if (!this->HasNativeGCHost() && (!this->isInScript || this->IsMemProtectMode()))
16841697
{
16851698
if (doSpecialMark)
16861699
{
@@ -1699,7 +1712,7 @@ Recycler::ScanStack()
16991712
{
17001713
// We may have interior pointers on the stack such as pointers in the middle of the character buffers backing a JavascriptString or SubString object.
17011714
// To prevent UAFs of these buffers after the GC we will always do MarkInterior for the pointers on stack. This is necessary only when we are doing a
1702-
// GC while running a script as that is when the possiblity of a UAF after GC exists.
1715+
// GC while running a script or when we have a host who allocates objects on the Chakra heap.
17031716
if (doSpecialMark)
17041717
{
17051718
ScanMemoryInline<true, true /* forceInterior */>(this->savedThreadContext.GetRegisters(), sizeof(void*) * SavedRegisterState::NumRegistersToSave
@@ -1715,7 +1728,7 @@ Recycler::ScanStack()
17151728

17161729
BEGIN_DUMP_OBJECT(this, _u("Stack"));
17171730
// We will not scan interior pointers on stack if we are not in script or we are in mem-protect mode.
1718-
if (!this->isInScript || this->IsMemProtectMode())
1731+
if (!this->HasNativeGCHost() && (!this->isInScript || this->IsMemProtectMode()))
17191732
{
17201733
if (doSpecialMark)
17211734
{
@@ -1732,7 +1745,7 @@ Recycler::ScanStack()
17321745
{
17331746
// We may have interior pointers on the stack such as pointers in the middle of the character buffers backing a JavascriptString or SubString object.
17341747
// To prevent UAFs of these buffers after the GC we will always do MarkInterior for the pointers on stack. This is necessary only when we are doing a
1735-
// GC while running a script as that is when the possiblity of a UAF after GC exists.
1748+
// GC while running a script or when we have a host who allocates objects on the Chakra heap.
17361749
if (doSpecialMark)
17371750
{
17381751
ScanMemoryInline<true, true /* forceInterior */>((void**)stackTop, stackScanned

lib/Common/Memory/Recycler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,7 @@ class Recycler
908908
bool inDisposeWrapper;
909909
bool needOOMRescan;
910910
bool hasDisposableObject;
911+
bool hasNativeGCHost;
911912
DWORD tickCountNextDispose;
912913
bool inExhaustiveCollection;
913914
bool hasExhaustiveCandidate;
@@ -1168,6 +1169,8 @@ class Recycler
11681169
void SetIsThreadBound();
11691170
void SetIsScriptActive(bool isScriptActive);
11701171
void SetIsInScript(bool isInScript);
1172+
bool HasNativeGCHost() const;
1173+
void SetHasNativeGCHost();
11711174
bool ShouldIdleCollectOnExit();
11721175
void ScheduleNextCollection();
11731176

0 commit comments

Comments
 (0)