From 6e65a50daaaef548e2e0d658f2eedd5ed99a6393 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Wed, 31 Aug 2022 03:20:13 -0700 Subject: [PATCH 1/2] Fix recursive issues for large Regions GC heap enumeration --- src/SOS/Strike/sos.cpp | 31 +++++++++++++++++++++++-------- src/SOS/Strike/sos.h | 23 +++++++++++++++++++++-- src/SOS/Strike/strike.cpp | 30 ++++++++++++------------------ 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/SOS/Strike/sos.cpp b/src/SOS/Strike/sos.cpp index 5764a86dd4..68f243a9dc 100644 --- a/src/SOS/Strike/sos.cpp +++ b/src/SOS/Strike/sos.cpp @@ -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) { @@ -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(); + 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 @@ -724,7 +737,7 @@ namespace sos } catch(const sos::Exception &) { - NextSegment(); + TryMoveToObjectInNextSegmentInRange(); } } @@ -773,7 +786,9 @@ namespace sos } if (mCurrObj > mEnd || mCurrObj >= mSegmentEnd) - NextSegment(); + { + TryMoveToObjectInNextSegmentInRange(); + } } SyncBlkIterator::SyncBlkIterator() diff --git a/src/SOS/Strike/sos.h b/src/SOS/Strike/sos.h index 2a58703c85..e4335daa28 100644 --- a/src/SOS/Strike/sos.h +++ b/src/SOS/Strike/sos.h @@ -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: diff --git a/src/SOS/Strike/strike.cpp b/src/SOS/Strike/strike.cpp index f5156d38a9..e7751f1eec 100644 --- a/src/SOS/Strike/strike.cpp +++ b/src/SOS/Strike/strike.cpp @@ -11039,6 +11039,7 @@ DECLARE_API(GCWhere) } } + DWORD heapIdx = 0; if (!IsServerBuild()) { DacpGcHeapDetails heapDetails; @@ -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 { @@ -11080,24 +11075,17 @@ 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); } } @@ -11105,6 +11093,12 @@ DECLARE_API(GCWhere) { 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; } From 55e1c04890f596ee9db875c78b6e85ee8926fca8 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Wed, 31 Aug 2022 14:56:59 -0700 Subject: [PATCH 2/2] Ensure check interrupt in other calls of GC algorithms --- src/SOS/Strike/sos.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/SOS/Strike/sos.cpp b/src/SOS/Strike/sos.cpp index 68f243a9dc..4040947155 100644 --- a/src/SOS/Strike/sos.cpp +++ b/src/SOS/Strike/sos.cpp @@ -591,6 +591,8 @@ namespace sos bool ObjectIterator::TryMoveNextSegment() { + CheckInterrupt(); + if (mCurrHeap >= mNumHeaps) { return false; @@ -663,6 +665,7 @@ namespace sos bool ObjectIterator::TryAlignToObjectInRange() { + CheckInterrupt(); while (!MemOverlap(mStart, mEnd, TO_TADDR(mSegment.mem), mSegmentEnd)) { CheckInterrupt(); @@ -755,6 +758,8 @@ namespace sos void ObjectIterator::MoveToNextObject() { + CheckInterrupt(); + // Object::GetSize can be unaligned, so we must align it ourselves. size_t size = (bLarge || bPinned) ? AlignLarge(mCurrObj.GetSize()) : Align(mCurrObj.GetSize());