Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix recursive issues for large Regions GC heap enumeration #3333

Merged
2 commits merged into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions src/SOS/Strike/sos.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,10 +586,10 @@ namespace sos
mCurrObj = mStart < TO_TADDR(mSegment.mem) ? TO_TADDR(mSegment.mem) : mStart;
mSegmentEnd = TO_TADDR(mSegment.highAllocMark);

CheckSegmentRange();
TryAlignToObjectInRange();
}

bool ObjectIterator::NextSegment()
bool ObjectIterator::TryMoveNextSegment()
{
if (mCurrHeap >= mNumHeaps)
{
Expand Down Expand Up @@ -648,16 +648,29 @@ namespace sos
mLastObj = 0;
mCurrObj = mStart < TO_TADDR(mSegment.mem) ? TO_TADDR(mSegment.mem) : mStart;
mSegmentEnd = TO_TADDR(mSegment.highAllocMark);
return CheckSegmentRange();
return true;
}

bool ObjectIterator::CheckSegmentRange()
bool ObjectIterator::TryMoveToObjectInNextSegmentInRange()
{
CheckInterrupt();
if (TryMoveNextSegment())
{
return TryAlignToObjectInRange();
}

return false;
}

bool ObjectIterator::TryAlignToObjectInRange()
{
while (!MemOverlap(mStart, mEnd, TO_TADDR(mSegment.mem), mSegmentEnd))
if (!NextSegment())
{
CheckInterrupt();
mikem8361 marked this conversation as resolved.
Show resolved Hide resolved
if (!TryMoveNextSegment())
{
return false;
}
}

// At this point we know that the current segment contains objects in
// the correct range. However, there's no telling if the user gave us
Expand Down Expand Up @@ -724,7 +737,7 @@ namespace sos
}
catch(const sos::Exception &)
{
NextSegment();
TryMoveToObjectInNextSegmentInRange();
}
}

Expand Down Expand Up @@ -773,7 +786,9 @@ namespace sos
}

if (mCurrObj > mEnd || mCurrObj >= mSegmentEnd)
NextSegment();
{
TryMoveToObjectInNextSegmentInRange();
}
}

SyncBlkIterator::SyncBlkIterator()
Expand Down
23 changes: 21 additions & 2 deletions src/SOS/Strike/sos.h
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,27 @@ namespace sos
void BuildError(__out_ecount(count) char *out, size_t count, const char *format, ...) const;

void AssertSanity() const;
bool NextSegment();
bool CheckSegmentRange();

/*
This function moves to the next segment/region without checking any restrictions
on the range. Returns true if it was able to move to a new segment/region.
*/
bool TryMoveNextSegment();

/*
Aligns the iterator to the object that falls in the requested range, moving to
the next segment/region as necessary. The iterator state doesn't change if the
current object already lies in the requested range. Returns true if aligning
to such an object was possible.
*/
bool TryAlignToObjectInRange();

/*
Moves to the next segment/region that contains an object in the requested
range and align it to such object. This operation always moves the iterator.
Returns false if no such move was possible.
*/
bool TryMoveToObjectInNextSegmentInRange();
void MoveToNextObject();

private:
Expand Down
30 changes: 12 additions & 18 deletions src/SOS/Strike/strike.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11039,6 +11039,7 @@ DECLARE_API(GCWhere)
}
}

DWORD heapIdx = 0;
if (!IsServerBuild())
{
DacpGcHeapDetails heapDetails;
Expand All @@ -11048,13 +11049,7 @@ DECLARE_API(GCWhere)
return Status;
}

if (GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge))
{
ExtOut("Address " WIN64_8SPACES " Gen Heap segment " WIN64_8SPACES " begin " WIN64_8SPACES " allocated " WIN64_8SPACES " size\n");
ExtOut("%p %d %2d %p %p %p 0x%x(%d)\n",
SOS_PTR(taddrObj), gen, 0, SOS_PTR(trngSeg.segAddr), SOS_PTR(trngSeg.start), SOS_PTR(trngSeg.end), size, size);
bFound = TRUE;
}
bFound = GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge);
}
else
{
Expand All @@ -11080,31 +11075,30 @@ DECLARE_API(GCWhere)
return Status;
}

for (DWORD n = 0; n < dwNHeaps; n ++)
for (heapIdx = 0; heapIdx < dwNHeaps && !bFound; heapIdx++)
{
DacpGcHeapDetails dacHeapDetails;
if (dacHeapDetails.Request(g_sos, heapAddrs[n]) != S_OK)
if (dacHeapDetails.Request(g_sos, heapAddrs[heapIdx]) != S_OK)
{
ExtOut("Error requesting details\n");
return Status;
}

GCHeapDetails heapDetails(dacHeapDetails, heapAddrs[n]);
if (GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge))
{
ExtOut("Address " WIN64_8SPACES " Gen Heap segment " WIN64_8SPACES " begin " WIN64_8SPACES " allocated" WIN64_8SPACES " size\n");
ExtOut("%p %d %2d %p %p %p 0x%x(%d)\n",
SOS_PTR(taddrObj), gen, n, SOS_PTR(trngSeg.segAddr), SOS_PTR(trngSeg.start), SOS_PTR(trngSeg.end), size, size);
bFound = TRUE;
break;
}
GCHeapDetails heapDetails(dacHeapDetails, heapAddrs[heapIdx]);
bFound = GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge);
}
}

if (!bFound)
{
ExtOut("Address %#p not found in the managed heap.\n", SOS_PTR(taddrObj));
}
else
{
ExtOut("Address " WIN64_8SPACES " Gen Heap segment " WIN64_8SPACES " begin " WIN64_8SPACES " allocated" WIN64_8SPACES " size\n");
ExtOut("%p %d %2d %p %p %p 0x%x(%d)\n",
SOS_PTR(taddrObj), gen, heapIdx, SOS_PTR(trngSeg.segAddr), SOS_PTR(trngSeg.start), SOS_PTR(trngSeg.end), size, size);
}

return Status;
}
Expand Down