-
Notifications
You must be signed in to change notification settings - Fork 383
Closed
Closed
Copy link
Description
This potential issue is found by code review.
The VerifyObject call here is used to power the !VerifyObj command.
diagnostics/src/SOS/Strike/gcroot.cpp
Lines 1703 to 1716 in 24cfdb2
| BOOL VerifyObject(const GCHeapDetails &heap, DWORD_PTR objAddr, DWORD_PTR MTAddr, size_t objSize, BOOL bVerifyMember) | |
| { | |
| // This is only used by the other VerifyObject function if bVerifyMember is true, | |
| // so we only initialize it if we need it for verifying object members. | |
| DacpHeapSegmentData seg; | |
| if (bVerifyMember) | |
| { | |
| // if we fail to find the segment, we cannot verify the object's members | |
| bVerifyMember = FindSegment(heap, seg, objAddr); | |
| } | |
| return VerifyObject(heap, seg, objAddr, MTAddr, objSize, bVerifyMember); | |
| } |
If I read the code correctly, bVerifyMember is reset to FALSE if FindSegment fails. Now FindSegment searches only from the small object heap segments.
diagnostics/src/SOS/Strike/gcroot.cpp
Lines 1642 to 1701 in 24cfdb2
| BOOL FindSegment(const GCHeapDetails &heap, DacpHeapSegmentData &seg, CLRDATA_ADDRESS addr) | |
| { | |
| if (heap.has_regions) | |
| { | |
| CLRDATA_ADDRESS dwAddrSeg; | |
| for (UINT n = 0; n <= GetMaxGeneration(); n++) | |
| { | |
| dwAddrSeg = (DWORD_PTR)heap.generation_table[n].start_segment; | |
| while (dwAddrSeg != 0) | |
| { | |
| if (IsInterrupt()) | |
| return FALSE; | |
| if (seg.Request(g_sos, dwAddrSeg, heap.original_heap_details) != S_OK) | |
| { | |
| ExtOut("Error requesting heap segment %p\n", SOS_PTR(dwAddrSeg)); | |
| return FALSE; | |
| } | |
| if (addr >= TO_TADDR(seg.mem) && | |
| addr < (dwAddrSeg == heap.ephemeral_heap_segment ? heap.alloc_allocated : TO_TADDR(seg.allocated))) | |
| { | |
| break; | |
| } | |
| dwAddrSeg = (DWORD_PTR)seg.next; | |
| } | |
| if (dwAddrSeg != 0) | |
| break; | |
| } | |
| } | |
| else | |
| { | |
| CLRDATA_ADDRESS dwAddrSeg = heap.generation_table[GetMaxGeneration()].start_segment; | |
| // Request the inital segment. | |
| if (seg.Request(g_sos, dwAddrSeg, heap.original_heap_details) != S_OK) | |
| { | |
| ExtOut("Error requesting heap segment %p.\n", SOS_PTR(dwAddrSeg)); | |
| return FALSE; | |
| } | |
| // Loop while the object is not in range of the segment. | |
| while (addr < TO_TADDR(seg.mem) || | |
| addr >= (dwAddrSeg == heap.ephemeral_heap_segment ? heap.alloc_allocated : TO_TADDR(seg.allocated))) | |
| { | |
| // get the next segment | |
| dwAddrSeg = seg.next; | |
| // We reached the last segment without finding the object. | |
| if (dwAddrSeg == NULL) | |
| return FALSE; | |
| if (seg.Request(g_sos, dwAddrSeg, heap.original_heap_details) != S_OK) | |
| { | |
| ExtOut("Error requesting heap segment %p.\n", SOS_PTR(dwAddrSeg)); | |
| return FALSE; | |
| } | |
| } | |
| } | |
| return TRUE; | |
| } |
Yes, the region case has a bug, ignore it for now ...
As GetMaxGeneration() returns 2, this will never search for the large object heap or the pinned object heap. I do not believe this restriction is intentional, since gc_heap::verify_heap in gc.cpp explicitly includes them.