Avoid reporting frozen segments in GetGenerationBounds (#85727)

cshung · noahfalk · web-flow · commit d64c06fd1db5 · 2023-07-01T13:04:28.000-07:00
Co-authored-by: Noah Falk &lt;noahfalk@users.noreply.github.com&gt;
diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp
@@ -46166,15 +46166,15 @@ void gc_heap::descr_generations_to_profiler (gen_walk_fn fn, void *context)
         for (int curr_gen_number = total_generation_count-1; curr_gen_number >= 0; curr_gen_number--)
         {
             generation* gen = hp->generation_of (curr_gen_number);
-            heap_segment* seg = generation_start_segment (gen);
+            heap_segment* seg = heap_segment_rw (generation_start_segment (gen));
 #ifdef USE_REGIONS
             while (seg)
             {
                 fn(context, curr_gen_number, heap_segment_mem (seg),
                                               heap_segment_allocated (seg),
                                               heap_segment_reserved (seg));
 
-                seg = heap_segment_next (seg);
+                seg = heap_segment_next_rw (seg);
             }
 #else
             while (seg && (seg != hp->ephemeral_heap_segment))
@@ -46190,7 +46190,7 @@ void gc_heap::descr_generations_to_profiler (gen_walk_fn fn, void *context)
                                               (curr_gen_number > max_generation) ?
                                                 heap_segment_reserved (seg) : heap_segment_allocated (seg));
 
-                seg = heap_segment_next (seg);
+                seg = heap_segment_next_rw (seg);
             }
 
             if (seg)
diff --git a/src/coreclr/gc/gcee.cpp b/src/coreclr/gc/gcee.cpp
@@ -62,6 +62,7 @@ void GCHeap::ReportGenerationBounds()
     {
         g_theGCHeap->DiagDescrGenerations([](void*, int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)
         {
+            ASSERT((0 <= generation) && (generation <= poh_generation));
             uint64_t range = static_cast<uint64_t>(rangeEnd - rangeStart);
             uint64_t rangeReserved = static_cast<uint64_t>(rangeEndReserved - rangeStart);
             FIRE_EVENT(GCGenerationRange, (uint8_t)generation, rangeStart, range, rangeReserved);
diff --git a/src/coreclr/gc/gceventstatus.h b/src/coreclr/gc/gceventstatus.h
@@ -174,9 +174,9 @@ class GCEventStatus
             fprintf(stderr, "GCHeapSurvivalAndMovement ");
         }
 
-        if (keyword & GCEventKeyword_GCHeapCollect)
+        if (keyword & GCEventKeyword_ManagedHeapCollect)
         {
-            fprintf(stderr, "GCHeapCollect ");
+            fprintf(stderr, "ManagedHeapCollect ");
         }
 
         if (keyword & GCEventKeyword_GCHeapAndTypeNames)
diff --git a/src/coreclr/gc/gcinterface.h b/src/coreclr/gc/gcinterface.h
@@ -277,7 +277,7 @@ enum GCEventKeyword
     GCEventKeyword_GCHeapDump                    =  0x100000,
     GCEventKeyword_GCSampledObjectAllocationHigh =  0x200000,
     GCEventKeyword_GCHeapSurvivalAndMovement     =  0x400000,
-    GCEventKeyword_GCHeapCollect                 =  0x800000,
+    GCEventKeyword_ManagedHeapCollect            =  0x800000,
     GCEventKeyword_GCHeapAndTypeNames            = 0x1000000,
     GCEventKeyword_GCSampledObjectAllocationLow  = 0x2000000,
     GCEventKeyword_All = GCEventKeyword_GC
@@ -287,7 +287,7 @@ enum GCEventKeyword
       | GCEventKeyword_GCHeapDump
       | GCEventKeyword_GCSampledObjectAllocationHigh
       | GCEventKeyword_GCHeapSurvivalAndMovement
-      | GCEventKeyword_GCHeapCollect
+      | GCEventKeyword_ManagedHeapCollect
       | GCEventKeyword_GCHeapAndTypeNames
       | GCEventKeyword_GCSampledObjectAllocationLow
 };
diff --git a/src/coreclr/nativeaot/Runtime/eventtrace.cpp b/src/coreclr/nativeaot/Runtime/eventtrace.cpp
@@ -417,7 +417,7 @@ void ETW::GCLog::EndMovedReferences(size_t profilingContext,
 }
 
 /***************************************************************************/
-/* This implements the public runtime provider's GCHeapCollectKeyword.  It
+/* This implements the public runtime provider's ManagedHeapCollectKeyword.  It
    performs a full, gen-2, blocking GC.
 /***************************************************************************/
 void ETW::GCLog::ForceGC(LONGLONG l64ClientSequenceNumber)
diff --git a/src/coreclr/nativeaot/Runtime/gcrhenv.cpp b/src/coreclr/nativeaot/Runtime/gcrhenv.cpp
@@ -87,12 +87,12 @@ uint32_t EtwCallback(uint32_t IsEnabled, RH_ETW_CONTEXT * pContext)
         GCHeapUtilities::GetGCHeap()->DiagTraceGCSegments();
     }
 
-    // Special check for the runtime provider's GCHeapCollectKeyword.  Profilers
+    // Special check for the runtime provider's ManagedHeapCollectKeyword.  Profilers
     // flick this to force a full GC.
     if (IsEnabled &&
         (pContext->RegistrationHandle == Microsoft_Windows_Redhawk_GC_PublicHandle) &&
         GCHeapUtilities::IsGCHeapInitialized() &&
-        ((pContext->MatchAnyKeyword & CLR_GCHEAPCOLLECT_KEYWORD) != 0))
+        ((pContext->MatchAnyKeyword & CLR_MANAGEDHEAPCOLLECT_KEYWORD) != 0))
     {
         // Profilers may (optionally) specify extra data in the filter parameter
         // to log with the GCStart event.
diff --git a/src/coreclr/nativeaot/Runtime/rheventtrace.h b/src/coreclr/nativeaot/Runtime/rheventtrace.h
@@ -40,7 +40,7 @@
 #define CLR_GCHEAPDUMP_KEYWORD 0x100000
 #define CLR_GCHEAPALLOC_KEYWORD 0x200000
 #define CLR_GCHEAPSURVIVALANDMOVEMENT_KEYWORD 0x400000
-#define CLR_GCHEAPCOLLECT_KEYWORD 0x800000
+#define CLR_MANAGEDHEAPCOLLECT_KEYWORD 0x800000
 #define CLR_GCHEAPANDTYPENAMES_KEYWORD 0x1000000
 #define CLR_PERFTRACK_KEYWORD 0x20000000
 #define CLR_STACK_KEYWORD 0x40000000
diff --git a/src/coreclr/vm/ClrEtwAll.man b/src/coreclr/vm/ClrEtwAll.man
@@ -57,8 +57,8 @@
                              message="$(string.RuntimePublisher.GCSampledObjectAllocationHighKeywordMessage)" symbol="CLR_GCHEAPALLOCHIGH_KEYWORD"/>
                     <keyword name="GCHeapSurvivalAndMovementKeyword" mask="0x400000"
                              message="$(string.RuntimePublisher.GCHeapSurvivalAndMovementKeywordMessage)" symbol="CLR_GCHEAPSURVIVALANDMOVEMENT_KEYWORD"/>
-                    <keyword name="GCHeapCollectKeyword" mask="0x800000"
-                             message="$(string.RuntimePublisher.GCHeapCollectKeyword)" symbol="CLR_GCHEAPCOLLECT_KEYWORD"/>
+                    <keyword name="ManagedHeapCollectKeyword" mask="0x800000"
+                             message="$(string.RuntimePublisher.ManagedHeapCollectKeyword)" symbol="CLR_MANAGEDHEAPCOLLECT_KEYWORD"/>
                     <keyword name="GCHeapAndTypeNamesKeyword" mask="0x1000000"
                              message="$(string.RuntimePublisher.GCHeapAndTypeNamesKeyword)" symbol="CLR_GCHEAPANDTYPENAMES_KEYWORD"/>
                     <keyword name="GCSampledObjectAllocationLowKeyword" mask="0x2000000"
@@ -3415,7 +3415,7 @@
                            symbol="GCBulkRootStaticVar" message="$(string.RuntimePublisher.GCBulkRootStaticVarEventMessage)"/>
 
                     <event value="39" version="0" level="win:LogAlways" template="GCDynamicEvent"
-                           keywords= "GCKeyword GCHandleKeyword GCHeapDumpKeyword GCSampledObjectAllocationHighKeyword GCHeapSurvivalAndMovementKeyword GCHeapCollectKeyword GCHeapAndTypeNamesKeyword GCSampledObjectAllocationLowKeyword"
+                           keywords= "GCKeyword GCHandleKeyword GCHeapDumpKeyword GCSampledObjectAllocationHighKeyword GCHeapSurvivalAndMovementKeyword ManagedHeapCollectKeyword GCHeapAndTypeNamesKeyword GCSampledObjectAllocationLowKeyword"
                            opcode="GCDynamicEvent"
                            task="GarbageCollection"
                            symbol="GCDynamicEvent"/>
@@ -9074,7 +9074,7 @@
                 <string id="RuntimePublisher.GCSampledObjectAllocationHighKeywordMessage" value="GCSampledObjectAllocationHigh" />
                 <string id="RuntimePublisher.GCSampledObjectAllocationLowKeywordMessage" value="GCSampledObjectAllocationLow" />
                 <string id="RuntimePublisher.GCHeapSurvivalAndMovementKeywordMessage" value="GCHeapSurvivalAndMovement" />
-                <string id="RuntimePublisher.GCHeapCollectKeyword" value="GCHeapCollect" />
+                <string id="RuntimePublisher.ManagedHeapCollectKeyword" value="ManagedHeapCollect" />
                 <string id="RuntimePublisher.GCHeapAndTypeNamesKeyword" value="GCHeapAndTypeNames" />
                 <string id="RuntimePublisher.GCHandleKeywordMessage" value="GCHandle" />
                 <string id="RuntimePublisher.ThreadTransferKeywordMessage" value="ThreadTransfer" />
diff --git a/src/coreclr/vm/eventtrace.cpp b/src/coreclr/vm/eventtrace.cpp
@@ -851,7 +851,7 @@ VOID ETW::GCLog::EndMovedReferences(size_t profilingContext, BOOL fAllowProfApiN
 }
 
 /***************************************************************************/
-/* This implements the public runtime provider's GCHeapCollectKeyword.  It
+/* This implements the public runtime provider's ManagedHeapCollectKeyword.  It
    performs a full, gen-2, blocking GC. */
 /***************************************************************************/
 VOID ETW::GCLog::ForceGC(LONGLONG l64ClientSequenceNumber)
@@ -4326,10 +4326,10 @@ void InitializeEventTracing()
 // that ultimately funnels them all into a common handler.
 
 #if defined(HOST_UNIX)
-// CLR_GCHEAPCOLLECT_KEYWORD is defined by the generated ETW manifest on Windows.
+// CLR_MANAGEDHEAPCOLLECT_KEYWORD is defined by the generated ETW manifest on Windows.
 // On non-Windows, we need to make sure that this is defined.  Given that we can't change
 // the value due to compatibility, we specify it here rather than generating defines based on the manifest.
-#define CLR_GCHEAPCOLLECT_KEYWORD 0x800000
+#define CLR_MANAGEDHEAPCOLLECT_KEYWORD 0x800000
 #endif // defined(HOST_UNIX)
 
 // CallbackProviderIndex provides a quick identification of which provider triggered the
@@ -4417,10 +4417,10 @@ VOID EtwCallbackCommon(
         GCHeapUtilities::RecordEventStateChange(bIsPublicTraceHandle, keywords, level);
     }
 
-    // Special check for the runtime provider's GCHeapCollectKeyword.  Profilers
+    // Special check for the runtime provider's ManagedHeapCollectKeyword.  Profilers
     // flick this to force a full GC.
     if (g_fEEStarted && !g_fEEShutDown && bIsPublicTraceHandle &&
-        ((MatchAnyKeyword & CLR_GCHEAPCOLLECT_KEYWORD) != 0))
+        ((MatchAnyKeyword & CLR_MANAGEDHEAPCOLLECT_KEYWORD) != 0))
     {
         // Profilers may (optionally) specify extra data in the filter parameter
         // to log with the GCStart event.
diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/vm/proftoeeinterfaceimpl.cpp
@@ -7689,7 +7689,7 @@ HRESULT ProfToEEInterfaceImpl::GetNonGCHeapBounds(ULONG cObjectRanges,
             ranges[segIdx].rangeStart = (ObjectID)firstObj;
 
             // Total size reserved for a segment
-            ranges[segIdx].rangeLengthReserved = (UINT_PTR)segments[segIdx]->m_Size;
+            ranges[segIdx].rangeLengthReserved = (UINT_PTR)segments[segIdx]->m_Size - sizeof(ObjHeader);
 
             // Size of the segment that is currently in use
             ranges[segIdx].rangeLength = (UINT_PTR)(segments[segIdx]->m_pCurrent - firstObj);
diff --git a/src/tests/profiler/native/nongcheap/nongcheap.cpp b/src/tests/profiler/native/nongcheap/nongcheap.cpp
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 #include "nongcheap.h"
+#include <vector>
 
 GUID NonGcHeapProfiler::GetClsid()
 {
@@ -59,46 +60,106 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished()
 
     _garbageCollections++;
 
-    const int MAX_NON_GC_HEAP_SEGMENTS = 16;
-    COR_PRF_NONGC_HEAP_RANGE segments[MAX_NON_GC_HEAP_SEGMENTS];
+    std::vector<ULONG> segment_starts;
+    std::vector<ULONG> segment_ends;
+    const int MAX_SEGMENTS = 16;
+    COR_PRF_NONGC_HEAP_RANGE nongc_segments[MAX_SEGMENTS];
+    COR_PRF_GC_GENERATION_RANGE gc_segments[MAX_SEGMENTS];
     ULONG segCount;
     ObjectID firstObj = 0;
-    HRESULT hr = pCorProfilerInfo->GetNonGCHeapBounds(MAX_NON_GC_HEAP_SEGMENTS, &segCount, segments);
+    HRESULT hr = pCorProfilerInfo->GetNonGCHeapBounds(MAX_SEGMENTS, &segCount, nongc_segments);
     if (FAILED(hr))
     {
         printf("GetNonGCHeapBounds returned an error\n!");
         _failures++;
     }
-    else if (segCount == 0 || segCount > MAX_NON_GC_HEAP_SEGMENTS)
+    else if (segCount == 0 || segCount > MAX_SEGMENTS)
     {
         printf("GetNonGCHeapBounds: invalid segCount (%u)\n!", segCount);
         _failures++;
     }
     else
     {
         // Save very first object ID to compare with EnumerateNonGCObjects
-        firstObj = segments[0].rangeStart;
+        firstObj = nongc_segments[0].rangeStart;
 
         printf("\nGetNonGCHeapBounds (segCount = %u):\n", segCount);
         for (ULONG i = 0; i < segCount; i++)
         {
             printf("\tseg#%u, rangeStart=%p, rangeLength=%u, rangeLengthReserved=%u\n",
-                i, (void*)segments[i].rangeStart, (ULONG)segments[i].rangeLength, (ULONG)segments[i].rangeLengthReserved);
+                i, (void*)nongc_segments[i].rangeStart, (ULONG)nongc_segments[i].rangeLength, (ULONG)nongc_segments[i].rangeLengthReserved);
 
-            if ((ULONG)segments[i].rangeLength > (ULONG)segments[i].rangeLengthReserved)
+            if ((ULONG)nongc_segments[i].rangeLength > (ULONG)nongc_segments[i].rangeLengthReserved)
             {
                 printf("GetNonGCHeapBounds: rangeLength > rangeLengthReserved");
                 _failures++;
             }
 
-            if (!segments[i].rangeStart)
+            if (!nongc_segments[i].rangeStart)
             {
                 printf("GetNonGCHeapBounds: rangeStart is null");
                 _failures++;
             }
+            segment_starts.emplace_back((ULONG)nongc_segments[i].rangeStart);
+            segment_ends.emplace_back((ULONG)nongc_segments[i].rangeStart + (ULONG)nongc_segments[i].rangeLengthReserved);
         }
         printf("\n");
     }
+    hr = pCorProfilerInfo->GetGenerationBounds(MAX_SEGMENTS, &segCount, gc_segments);
+    if (FAILED(hr))
+    {
+        printf("GetGenerationBounds returned an error\n!");
+        _failures++;
+    }
+    else if (segCount == 0 || segCount > MAX_SEGMENTS)
+    {
+        printf("GetGenerationBounds: invalid segCount (%u)\n!", segCount);
+        _failures++;
+    }
+    else
+    {
+        printf("\nGetGenerationBounds (segCount = %u):\n", segCount);
+        for (ULONG i = 0; i < segCount; i++)
+        {
+            printf("\tseg#%u, rangeStart=%p, rangeLength=%u, rangeLengthReserved=%u\n",
+                i, (void*)gc_segments[i].rangeStart, (ULONG)gc_segments[i].rangeLength, (ULONG)gc_segments[i].rangeLengthReserved);
+
+            if ((ULONG)gc_segments[i].rangeLength > (ULONG)gc_segments[i].rangeLengthReserved)
+            {
+                printf("GetGenerationBounds: rangeLength > rangeLengthReserved");
+                _failures++;
+            }
+
+            if (!gc_segments[i].rangeStart)
+            {
+                printf("GetGenerationBounds: rangeStart is null");
+                _failures++;
+            }
+            segment_starts.emplace_back((ULONG)gc_segments[i].rangeStart);
+            segment_ends.emplace_back((ULONG)gc_segments[i].rangeStart + (ULONG)gc_segments[i].rangeLengthReserved);
+        }
+        printf("\n");
+    }
+    sort(segment_starts.begin(), segment_starts.end());
+    sort(segment_ends.begin(), segment_ends.end());
+    for (size_t i = 0; i < segment_starts.size() - 1; i++)
+    {
+        if (segment_starts[i] == segment_starts[i+1])
+        {
+            printf("Duplicated segment starts");
+            _failures++;
+        }
+        if (segment_ends[i] == segment_ends[i+1])
+        {
+            printf("Duplicated segment ends");
+            _failures++;
+        }
+        if (segment_ends[i] > segment_starts[i+1])
+        {
+            printf("Overlapping segments\n");
+            _failures++;
+        }
+    }
 
     // Let's make sure we got the same number of objects as we got from the callback
     // by testing the EnumerateNonGCObjects API.

Original file line number	Diff line number	Diff line change
`@@ -46166,15 +46166,15 @@ void gc_heap::descr_generations_to_profiler (gen_walk_fn fn, void *context)`
`46166`	`46166`	`for (int curr_gen_number = total_generation_count-1; curr_gen_number >= 0; curr_gen_number--)`
`46167`	`46167`	`{`
`46168`	`46168`	`generation* gen = hp->generation_of (curr_gen_number);`
`46169`		`- heap_segment* seg = generation_start_segment (gen);`
	`46169`	`+ heap_segment* seg = heap_segment_rw (generation_start_segment (gen));`
`46170`	`46170`	`#ifdef USE_REGIONS`
`46171`	`46171`	`while (seg)`
`46172`	`46172`	`{`
`46173`	`46173`	`fn(context, curr_gen_number, heap_segment_mem (seg),`
`46174`	`46174`	`heap_segment_allocated (seg),`
`46175`	`46175`	`heap_segment_reserved (seg));`
`46176`	`46176`
`46177`		`- seg = heap_segment_next (seg);`
	`46177`	`+ seg = heap_segment_next_rw (seg);`
`46178`	`46178`	`}`
`46179`	`46179`	`#else`
`46180`	`46180`	`while (seg && (seg != hp->ephemeral_heap_segment))`
`@@ -46190,7 +46190,7 @@ void gc_heap::descr_generations_to_profiler (gen_walk_fn fn, void *context)`
`46190`	`46190`	`(curr_gen_number > max_generation) ?`
`46191`	`46191`	`heap_segment_reserved (seg) : heap_segment_allocated (seg));`
`46192`	`46192`
`46193`		`- seg = heap_segment_next (seg);`
	`46193`	`+ seg = heap_segment_next_rw (seg);`
`46194`	`46194`	`}`
`46195`	`46195`
`46196`	`46196`	`if (seg)`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ void GCHeap::ReportGenerationBounds()`
`62`	`62`	`{`
`63`	`63`	`g_theGCHeap->DiagDescrGenerations([](void, int generation, uint8_t rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)`
`64`	`64`	`{`
	`65`	`+ ASSERT((0 <= generation) && (generation <= poh_generation));`
`65`	`66`	`uint64_t range = static_cast<uint64_t>(rangeEnd - rangeStart);`
`66`	`67`	`uint64_t rangeReserved = static_cast<uint64_t>(rangeEndReserved - rangeStart);`
`67`	`68`	`FIRE_EVENT(GCGenerationRange, (uint8_t)generation, rangeStart, range, rangeReserved);`
Original file line number	Diff line number	Diff line change
`@@ -174,9 +174,9 @@ class GCEventStatus`
`174`	`174`	`fprintf(stderr, "GCHeapSurvivalAndMovement ");`
`175`	`175`	`}`
`176`	`176`
`177`		`- if (keyword & GCEventKeyword_GCHeapCollect)`
	`177`	`+ if (keyword & GCEventKeyword_ManagedHeapCollect)`
`178`	`178`	`{`
`179`		`- fprintf(stderr, "GCHeapCollect ");`
	`179`	`+ fprintf(stderr, "ManagedHeapCollect ");`
`180`	`180`	`}`
`181`	`181`
`182`	`182`	`if (keyword & GCEventKeyword_GCHeapAndTypeNames)`
Original file line number	Diff line number	Diff line change
`@@ -417,7 +417,7 @@ void ETW::GCLog::EndMovedReferences(size_t profilingContext,`
`417`	`417`	`}`
`418`	`418`
`419`	`419`	`/***************************************************************************/`
`420`		`-/* This implements the public runtime provider's GCHeapCollectKeyword. It`
	`420`	`+/* This implements the public runtime provider's ManagedHeapCollectKeyword. It`
`421`	`421`	`performs a full, gen-2, blocking GC.`
`422`	`422`	`/***************************************************************************/`
`423`	`423`	`void ETW::GCLog::ForceGC(LONGLONG l64ClientSequenceNumber)`
Original file line number	Diff line number	Diff line change
`@@ -87,12 +87,12 @@ uint32_t EtwCallback(uint32_t IsEnabled, RH_ETW_CONTEXT * pContext)`
`87`	`87`	`GCHeapUtilities::GetGCHeap()->DiagTraceGCSegments();`
`88`	`88`	`}`
`89`	`89`
`90`		`- // Special check for the runtime provider's GCHeapCollectKeyword. Profilers`
	`90`	`+ // Special check for the runtime provider's ManagedHeapCollectKeyword. Profilers`
`91`	`91`	`// flick this to force a full GC.`
`92`	`92`	`if (IsEnabled &&`
`93`	`93`	`(pContext->RegistrationHandle == Microsoft_Windows_Redhawk_GC_PublicHandle) &&`
`94`	`94`	`GCHeapUtilities::IsGCHeapInitialized() &&`
`95`		`- ((pContext->MatchAnyKeyword & CLR_GCHEAPCOLLECT_KEYWORD) != 0))`
	`95`	`+ ((pContext->MatchAnyKeyword & CLR_MANAGEDHEAPCOLLECT_KEYWORD) != 0))`
`96`	`96`	`{`
`97`	`97`	`// Profilers may (optionally) specify extra data in the filter parameter`
`98`	`98`	`// to log with the GCStart event.`