From 1d370eacb083b601a8ac4acab43ca3f79bc85e1f Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Wed, 5 Apr 2023 15:20:35 -0700 Subject: [PATCH 01/27] Add ISOSMemoryEnum and HandleTable enum --- src/coreclr/debug/daccess/daccess.cpp | 81 +++++++++++- src/coreclr/debug/daccess/dacimpl.h | 35 ++++++ src/coreclr/debug/daccess/request.cpp | 49 ++++++++ src/coreclr/gc/gcinterface.dac.h | 22 +++- src/coreclr/gc/handletableconstants.h | 117 ++++++++++++++++++ src/coreclr/gc/handletablepriv.h | 118 +----------------- src/coreclr/gc/objecthandle.cpp | 2 + src/coreclr/inc/sospriv.idl | 31 ++++- src/coreclr/pal/prebuilt/idl/sospriv_i.cpp | 11 +- src/coreclr/pal/prebuilt/inc/sospriv.h | 137 ++++++++++++++++++++- src/coreclr/vm/decodemd.cpp | 1 - 11 files changed, 482 insertions(+), 122 deletions(-) create mode 100644 src/coreclr/gc/handletableconstants.h diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 3a0ceb80e6057f..a4083027c63c8e 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -7654,7 +7654,6 @@ void DacHandleWalker::WalkHandles() { if (mask & 1) { - dac_handle_table *pTable = hTable; PTR_AppDomain pDomain = AppDomain::GetCurrentDomain(); param.AppDomain = TO_CDADDR(pDomain.GetAddr()); param.Type = handleType; @@ -8285,3 +8284,83 @@ HRESULT DacStackReferenceErrorEnum::Next(unsigned int count, SOSStackRefError re *pFetched = i; return i < count ? S_FALSE : S_OK; } + + +HRESULT DacMemoryEnumerator::Skip(unsigned int count) +{ + mIteratorIndex += count; + return S_OK; +} + +HRESULT DacMemoryEnumerator::Reset() +{ + mIteratorIndex = 0; + return S_OK; +} + +HRESULT DacMemoryEnumerator::GetCount(unsigned int* pCount) +{ + if (!pCount) + return E_POINTER; + + mRegions.GetCount(); + return S_OK; +} + +HRESULT DacMemoryEnumerator::Next(unsigned int count, SOSMemoryRegion regions[], unsigned int* pFetched) +{ + if (!pFetched) + return E_POINTER; + + if (!regions) + return E_POINTER; + + unsigned int i = 0; + while (i < count && mIteratorIndex < mRegions.GetCount()) + { + regions[i++] = mRegions.Get(mIteratorIndex++); + } + + *pFetched = i; + return i < count ? S_FALSE : S_OK; +} + +HRESULT DacHandleTableMemoryEnumerator::Init() +{ + int max_slots = 1; + +#ifdef FEATURE_SVR_GC + if (GCHeapUtilities::IsServerHeap()) + max_slots = GCHeapCount(); +#endif // FEATURE_SVR_GC + + for (dac_handle_table_map *map = g_gcDacGlobals->handle_table_map; map; map = map->pNext) + { + for (int i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; ++i) + { + if (map->pBuckets[i] != NULL) + { + for (int j = 0; j < max_slots ; ++j) + { + DPTR(dac_handle_table) pTable = map->pBuckets[i]->pTable[j]; + DPTR(dac_handle_table_segment) pFirstSegment = pTable->pSegmentList; + DPTR(dac_handle_table_segment) curr = pFirstSegment; + + do + { + SOSMemoryRegion mem = {0}; + mem.Start = curr.GetAddr(); + mem.Size = HANDLE_SEGMENT_SIZE; + mem.ExtraData = j; // heap number + + mRegions.Add(mem); + + curr = curr->pNextSegment; + } while (curr != nullptr && curr != pFirstSegment); + } + } + } + } + + return S_OK; +} diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index 52ef9cf5979d9c..da108be41f3b0a 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -1211,6 +1211,10 @@ class ClrDataAccess virtual HRESULT STDMETHODCALLTYPE GetDomainLoaderAllocator(CLRDATA_ADDRESS domainAddress, CLRDATA_ADDRESS *pLoaderAllocator); virtual HRESULT STDMETHODCALLTYPE GetLoaderAllocatorHeapNames(int count, const char **ppNames, int *pNeeded); virtual HRESULT STDMETHODCALLTYPE GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocator, int count, CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded); + virtual HRESULT STDMETHODCALLTYPE GetHandleTableMemoryRegions(ISOSMemoryEnum **ppEnum); + virtual HRESULT STDMETHODCALLTYPE GetGCBookkeepingMemoryRegions(ISOSMemoryEnum **ppEnum); + virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions(ISOSMemoryEnum **ppEnum); + virtual HRESULT STDMETHODCALLTYPE LockedFlush(); // // ClrDataAccess. @@ -1954,6 +1958,37 @@ class DacReferenceList unsigned int _capacity; }; + +class DacMemoryEnumerator : public DefaultCOMImpl +{ +public: + DacMemoryEnumerator() + : mIteratorIndex(0) + { + } + + virtual ~DacMemoryEnumerator() {} + virtual HRESULT Init() = 0; + + HRESULT STDMETHODCALLTYPE Skip(unsigned int count); + HRESULT STDMETHODCALLTYPE Reset(); + HRESULT STDMETHODCALLTYPE GetCount(unsigned int *pCount); + HRESULT STDMETHODCALLTYPE Next(unsigned int count, + SOSMemoryRegion regions[], + unsigned int *pFetched); + +protected: + DacReferenceList mRegions; + +private: + unsigned int mIteratorIndex; +}; + +class DacHandleTableMemoryEnumerator : public DacMemoryEnumerator +{ + virtual HRESULT Init(); +}; + struct DacGcReference; /* DacStackReferenceWalker. */ diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index b5a0aa1986bd21..5c40f3d6daebfc 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -5295,3 +5295,52 @@ HRESULT ClrDataAccess::GetGlobalAllocationContext( SOSDacLeave(); return hr; } + +HRESULT ClrDataAccess::GetHandleTableMemoryRegions(ISOSMemoryEnum** ppEnum) +{ + if (!ppEnum) + return E_POINTER; + + SOSDacEnter(); + + DacHandleTableMemoryEnumerator* htEnum = new (nothrow) DacHandleTableMemoryEnumerator(); + if (htEnum) + { + hr = htEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); + if (FAILED(hr)) + delete ppEnum; + } + else + { + hr = E_OUTOFMEMORY; + } + + SOSDacLeave(); + return hr; +} + +HRESULT ClrDataAccess::GetGCBookkeepingMemoryRegions(ISOSMemoryEnum** ppEnum) +{ + if (!ppEnum) + return E_POINTER; + + return E_NOTIMPL; +} + +HRESULT ClrDataAccess::GetGCFreeRegions(ISOSMemoryEnum** ppEnum) +{ + if (!ppEnum) + return E_POINTER; + + return E_NOTIMPL; +} + +HRESULT ClrDataAccess::LockedFlush() +{ + SOSDacEnter(); + + Flush(); + + SOSDacLeave(); + return hr; +} diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index cfe968b74624bc..e79d1de79a3b7a 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -22,10 +22,10 @@ // This value cannot change and should not be used in new DAC APIs. New APIs can query GcDacVars.total_generation_count // variable which is dynamically initialized at runtime - #define NUMBERGENERATIONS 4 -#define INITIAL_HANDLE_TABLE_ARRAY_SIZE 10 -#define HANDLE_MAX_INTERNAL_TYPES 12 + + +#include "handletableconstants.h" // Analogue for the GC heap_segment class, containing information regarding a single // heap segment. @@ -64,6 +64,21 @@ class dac_finalize_queue { uint8_t** m_FillPointers[NUMBERGENERATIONS + ExtraSegCount]; }; +class dac_handle_table_segment { +public: + uint8_t rgGeneration[HANDLE_BLOCKS_PER_SEGMENT * sizeof(uint32_t) / sizeof(uint8_t)]; + uint8_t rgAllocation[HANDLE_BLOCKS_PER_SEGMENT]; + uint32_t rgFreeMask[HANDLE_MASKS_PER_SEGMENT]; + uint8_t rgBlockType[HANDLE_BLOCKS_PER_SEGMENT]; + uint8_t rgUserData[HANDLE_BLOCKS_PER_SEGMENT]; + uint8_t rgLocks[HANDLE_BLOCKS_PER_SEGMENT]; + uint8_t rgTail[HANDLE_MAX_INTERNAL_TYPES]; + uint8_t rgHint[HANDLE_MAX_INTERNAL_TYPES]; + uint32_t rgFreeCount[HANDLE_MAX_INTERNAL_TYPES]; + DPTR(dac_handle_table_segment) pNextSegment; + }; + + class dac_handle_table { public: // We do try to keep everything that the DAC knows about as close to the @@ -71,6 +86,7 @@ class dac_handle_table { // HandleTable has rgTypeFlags at offset 0 for performance reasons and // we don't want to disrupt that. uint32_t padding[HANDLE_MAX_INTERNAL_TYPES]; + DPTR(dac_handle_table_segment) pSegmentList; }; class dac_handle_table_bucket { diff --git a/src/coreclr/gc/handletableconstants.h b/src/coreclr/gc/handletableconstants.h new file mode 100644 index 00000000000000..0d6e678f2aebc0 --- /dev/null +++ b/src/coreclr/gc/handletableconstants.h @@ -0,0 +1,117 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +#ifndef __HANDLETABLECONSTANTS_H__ +#define __HANDLETABLECONSTANTS_H__ + + // Build support for async pinned handles into standalone GC to make it usable with older runtimes +#if defined(BUILD_AS_STANDALONE) && !defined(FEATURE_NATIVEAOT) +#define FEATURE_ASYNC_PINNED_HANDLES +#endif + +#define INITIAL_HANDLE_TABLE_ARRAY_SIZE 10 +#define HANDLE_MAX_INTERNAL_TYPES 12 + +/*--------------------------------------------------------------------------*/ + +//@TODO: find a home for this in a project-level header file +#ifndef BITS_PER_BYTE +#define BITS_PER_BYTE (8) +#endif +/*--------------------------------------------------------------------------*/ + + + +/**************************************************************************** + * + * MAJOR TABLE DEFINITIONS THAT CHANGE DEPENDING ON THE WEATHER + * + ****************************************************************************/ + +// 64k reserved per segment with 4k as header. +#define HANDLE_SEGMENT_SIZE (0x10000) // MUST be a power of 2 (and currently must be 64K due to VirtualAlloc semantics) +#define HANDLE_HEADER_SIZE (0x1000) // SHOULD be <= OS page size + +#define HANDLE_SEGMENT_ALIGNMENT HANDLE_SEGMENT_SIZE + + +#if !BIGENDIAN + + // little-endian write barrier mask manipulation + #define GEN_CLUMP_0_MASK (0x000000FF) + #define NEXT_CLUMP_IN_MASK(dw) ((dw) >> BITS_PER_BYTE) + +#else + + // big-endian write barrier mask manipulation + #define GEN_CLUMP_0_MASK (0xFF000000) + #define NEXT_CLUMP_IN_MASK(dw) ((dw) << BITS_PER_BYTE) + +#endif + + +// if the above numbers change than these will likely change as well +#define HANDLE_HANDLES_PER_CLUMP (16) // segment write-barrier granularity +#define HANDLE_HANDLES_PER_BLOCK (64) // segment suballocation granularity +#define HANDLE_OPTIMIZE_FOR_64_HANDLE_BLOCKS // flag for certain optimizations + +// number of types allowed for public callers +#define HANDLE_MAX_PUBLIC_TYPES (HANDLE_MAX_INTERNAL_TYPES - 1) // reserve one internal type + +// internal block types +#define HNDTYPE_INTERNAL_DATABLOCK (HANDLE_MAX_INTERNAL_TYPES - 1) // reserve last type for data blocks + +// max number of generations to support statistics on +#define MAXSTATGEN (5) + +/*--------------------------------------------------------------------------*/ + + + +/**************************************************************************** + * + * MORE DEFINITIONS + * + ****************************************************************************/ + +// fast handle-to-segment mapping +#define HANDLE_SEGMENT_CONTENT_MASK (HANDLE_SEGMENT_SIZE - 1) +#define HANDLE_SEGMENT_ALIGN_MASK (~HANDLE_SEGMENT_CONTENT_MASK) + +// table layout metrics +#define HANDLE_SIZE sizeof(_UNCHECKED_OBJECTREF) +#define HANDLE_HANDLES_PER_SEGMENT ((HANDLE_SEGMENT_SIZE - HANDLE_HEADER_SIZE) / HANDLE_SIZE) +#define HANDLE_BLOCKS_PER_SEGMENT (HANDLE_HANDLES_PER_SEGMENT / HANDLE_HANDLES_PER_BLOCK) +#define HANDLE_CLUMPS_PER_SEGMENT (HANDLE_HANDLES_PER_SEGMENT / HANDLE_HANDLES_PER_CLUMP) +#define HANDLE_CLUMPS_PER_BLOCK (HANDLE_HANDLES_PER_BLOCK / HANDLE_HANDLES_PER_CLUMP) +#define HANDLE_BYTES_PER_BLOCK (HANDLE_HANDLES_PER_BLOCK * HANDLE_SIZE) +#define HANDLE_HANDLES_PER_MASK (sizeof(uint32_t) * BITS_PER_BYTE) +#define HANDLE_MASKS_PER_SEGMENT (HANDLE_HANDLES_PER_SEGMENT / HANDLE_HANDLES_PER_MASK) +#define HANDLE_MASKS_PER_BLOCK (HANDLE_HANDLES_PER_BLOCK / HANDLE_HANDLES_PER_MASK) +#define HANDLE_CLUMPS_PER_MASK (HANDLE_HANDLES_PER_MASK / HANDLE_HANDLES_PER_CLUMP) + +// We use this relation to check for free mask per block. +C_ASSERT (HANDLE_HANDLES_PER_MASK * 2 == HANDLE_HANDLES_PER_BLOCK); + + +// cache layout metrics +#define HANDLE_CACHE_TYPE_SIZE 128 // 128 == 63 handles per bank +#define HANDLES_PER_CACHE_BANK ((HANDLE_CACHE_TYPE_SIZE / 2) - 1) + +// cache policy defines +#define REBALANCE_TOLERANCE (HANDLES_PER_CACHE_BANK / 3) +#define REBALANCE_LOWATER_MARK (HANDLES_PER_CACHE_BANK - REBALANCE_TOLERANCE) +#define REBALANCE_HIWATER_MARK (HANDLES_PER_CACHE_BANK + REBALANCE_TOLERANCE) + +// bulk alloc policy defines +#define SMALL_ALLOC_COUNT (HANDLES_PER_CACHE_BANK / 10) + +// misc constants +#define MASK_FULL (0) +#define MASK_EMPTY (0xFFFFFFFF) +#define MASK_LOBYTE (0x000000FF) +#define TYPE_INVALID ((uint8_t)0xFF) +#define BLOCK_INVALID ((uint8_t)0xFF) + +/*--------------------------------------------------------------------------*/ + +#endif // __HANDLETABLECONSTANTS_H__ diff --git a/src/coreclr/gc/handletablepriv.h b/src/coreclr/gc/handletablepriv.h index 4c055389963430..086ef2e018b6fa 100644 --- a/src/coreclr/gc/handletablepriv.h +++ b/src/coreclr/gc/handletablepriv.h @@ -14,113 +14,7 @@ #include "handletable.h" - - // Build support for async pinned handles into standalone GC to make it usable with older runtimes -#if defined(BUILD_AS_STANDALONE) && !defined(FEATURE_NATIVEAOT) -#define FEATURE_ASYNC_PINNED_HANDLES -#endif - - -/*--------------------------------------------------------------------------*/ - -//@TODO: find a home for this in a project-level header file -#define BITS_PER_BYTE (8) -/*--------------------------------------------------------------------------*/ - - - -/**************************************************************************** - * - * MAJOR TABLE DEFINITIONS THAT CHANGE DEPENDING ON THE WEATHER - * - ****************************************************************************/ - -// 64k reserved per segment with 4k as header. -#define HANDLE_SEGMENT_SIZE (0x10000) // MUST be a power of 2 (and currently must be 64K due to VirtualAlloc semantics) -#define HANDLE_HEADER_SIZE (0x1000) // SHOULD be <= OS page size - -#define HANDLE_SEGMENT_ALIGNMENT HANDLE_SEGMENT_SIZE - - -#if !BIGENDIAN - - // little-endian write barrier mask manipulation - #define GEN_CLUMP_0_MASK (0x000000FF) - #define NEXT_CLUMP_IN_MASK(dw) ((dw) >> BITS_PER_BYTE) - -#else - - // big-endian write barrier mask manipulation - #define GEN_CLUMP_0_MASK (0xFF000000) - #define NEXT_CLUMP_IN_MASK(dw) ((dw) << BITS_PER_BYTE) - -#endif - - -// if the above numbers change than these will likely change as well -#define HANDLE_HANDLES_PER_CLUMP (16) // segment write-barrier granularity -#define HANDLE_HANDLES_PER_BLOCK (64) // segment suballocation granularity -#define HANDLE_OPTIMIZE_FOR_64_HANDLE_BLOCKS // flag for certain optimizations - -// number of types allowed for public callers -#define HANDLE_MAX_PUBLIC_TYPES (HANDLE_MAX_INTERNAL_TYPES - 1) // reserve one internal type - -// internal block types -#define HNDTYPE_INTERNAL_DATABLOCK (HANDLE_MAX_INTERNAL_TYPES - 1) // reserve last type for data blocks - -// max number of generations to support statistics on -#define MAXSTATGEN (5) - -/*--------------------------------------------------------------------------*/ - - - -/**************************************************************************** - * - * MORE DEFINITIONS - * - ****************************************************************************/ - -// fast handle-to-segment mapping -#define HANDLE_SEGMENT_CONTENT_MASK (HANDLE_SEGMENT_SIZE - 1) -#define HANDLE_SEGMENT_ALIGN_MASK (~HANDLE_SEGMENT_CONTENT_MASK) - -// table layout metrics -#define HANDLE_SIZE sizeof(_UNCHECKED_OBJECTREF) -#define HANDLE_HANDLES_PER_SEGMENT ((HANDLE_SEGMENT_SIZE - HANDLE_HEADER_SIZE) / HANDLE_SIZE) -#define HANDLE_BLOCKS_PER_SEGMENT (HANDLE_HANDLES_PER_SEGMENT / HANDLE_HANDLES_PER_BLOCK) -#define HANDLE_CLUMPS_PER_SEGMENT (HANDLE_HANDLES_PER_SEGMENT / HANDLE_HANDLES_PER_CLUMP) -#define HANDLE_CLUMPS_PER_BLOCK (HANDLE_HANDLES_PER_BLOCK / HANDLE_HANDLES_PER_CLUMP) -#define HANDLE_BYTES_PER_BLOCK (HANDLE_HANDLES_PER_BLOCK * HANDLE_SIZE) -#define HANDLE_HANDLES_PER_MASK (sizeof(uint32_t) * BITS_PER_BYTE) -#define HANDLE_MASKS_PER_SEGMENT (HANDLE_HANDLES_PER_SEGMENT / HANDLE_HANDLES_PER_MASK) -#define HANDLE_MASKS_PER_BLOCK (HANDLE_HANDLES_PER_BLOCK / HANDLE_HANDLES_PER_MASK) -#define HANDLE_CLUMPS_PER_MASK (HANDLE_HANDLES_PER_MASK / HANDLE_HANDLES_PER_CLUMP) - -// We use this relation to check for free mask per block. -C_ASSERT (HANDLE_HANDLES_PER_MASK * 2 == HANDLE_HANDLES_PER_BLOCK); - - -// cache layout metrics -#define HANDLE_CACHE_TYPE_SIZE 128 // 128 == 63 handles per bank -#define HANDLES_PER_CACHE_BANK ((HANDLE_CACHE_TYPE_SIZE / 2) - 1) - -// cache policy defines -#define REBALANCE_TOLERANCE (HANDLES_PER_CACHE_BANK / 3) -#define REBALANCE_LOWATER_MARK (HANDLES_PER_CACHE_BANK - REBALANCE_TOLERANCE) -#define REBALANCE_HIWATER_MARK (HANDLES_PER_CACHE_BANK + REBALANCE_TOLERANCE) - -// bulk alloc policy defines -#define SMALL_ALLOC_COUNT (HANDLES_PER_CACHE_BANK / 10) - -// misc constants -#define MASK_FULL (0) -#define MASK_EMPTY (0xFFFFFFFF) -#define MASK_LOBYTE (0x000000FF) -#define TYPE_INVALID ((uint8_t)0xFF) -#define BLOCK_INVALID ((uint8_t)0xFF) - -/*--------------------------------------------------------------------------*/ +#include "handletableconstants.h" @@ -483,6 +377,11 @@ struct HandleTable */ uint32_t rgTypeFlags[HANDLE_MAX_INTERNAL_TYPES]; + /* + * head of segment list for this table + */ + PTR_TableSegment pSegmentList; + /* * lock for this table */ @@ -499,11 +398,6 @@ struct HandleTable */ uint32_t dwCount; - /* - * head of segment list for this table - */ - PTR_TableSegment pSegmentList; - /* * information on current async scan (if any) */ diff --git a/src/coreclr/gc/objecthandle.cpp b/src/coreclr/gc/objecthandle.cpp index 52b25b88adf51f..43e09789bab815 100644 --- a/src/coreclr/gc/objecthandle.cpp +++ b/src/coreclr/gc/objecthandle.cpp @@ -1811,6 +1811,8 @@ void PopulateHandleTableDacVars(GcDacVars* gcDacVars) static_assert(offsetof(HandleTableMap, dwMaxIndex) == offsetof(dac_handle_table_map, dwMaxIndex), "handle table map DAC layout mismatch"); static_assert(offsetof(HandleTableBucket, pTable) == offsetof(dac_handle_table_bucket, pTable), "handle table bucket DAC layout mismatch"); static_assert(offsetof(HandleTableBucket, HandleTableIndex) == offsetof(dac_handle_table_bucket, HandleTableIndex), "handle table bucket DAC layout mismatch"); + static_assert(offsetof(HandleTable, pSegmentList) == offsetof(dac_handle_table, pSegmentList), "handle table bucket DAC layout mismatch"); + static_assert(offsetof(_TableSegmentHeader, pNextSegment) == offsetof(dac_handle_table_segment, pNextSegment), "handle table bucket DAC layout mismatch"); #ifndef DACCESS_COMPILE gcDacVars->handle_table_map = reinterpret_cast(&g_HandleTableMap); diff --git a/src/coreclr/inc/sospriv.idl b/src/coreclr/inc/sospriv.idl index b1a3b18e06b8b9..c84eb88e7a0d12 100644 --- a/src/coreclr/inc/sospriv.idl +++ b/src/coreclr/inc/sospriv.idl @@ -172,6 +172,31 @@ interface ISOSStackRefEnum : ISOSEnum } +cpp_quote("#ifndef _SOS_MemoryRegion_") +cpp_quote("#define _SOS_MemoryRegion_") + +typedef struct _SOSMemoryRegion +{ + CLRDATA_ADDRESS Start; + CLRDATA_ADDRESS Size; + CLRDATA_ADDRESS ExtraData; // API specific extra data. +} SOSMemoryRegion; + +cpp_quote("#endif // _SOS_MemoryRegion_") + +[ + object, + local, + uuid(E4B860EC-337A-40C0-A591-F09A9680690F) +] +interface ISOSMemoryEnum : ISOSEnum +{ + HRESULT Next([in] unsigned int count, + [out, size_is(count), length_is(*pNeeded)] SOSMemoryRegion memRegion[], + [out] unsigned int *pNeeded); +} + + [ object, local, @@ -465,7 +490,7 @@ interface ISOSDacInterface12 : IUnknown [ object, local, - uuid(3176a8ed-597b-4f54-a71f-83695c6a8c5d) + uuid(3176a8ed-597b-4f54-a71f-83695c6a8c5e) ] interface ISOSDacInterface13 : IUnknown { @@ -473,4 +498,8 @@ interface ISOSDacInterface13 : IUnknown HRESULT GetDomainLoaderAllocator(CLRDATA_ADDRESS domainAddress, CLRDATA_ADDRESS *pLoaderAllocator); HRESULT GetLoaderAllocatorHeapNames(int count, const char **ppNames, int *pNeeded); HRESULT GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocator, int count, CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded); + HRESULT GetHandleTableMemoryRegions(ISOSMemoryEnum **ppEnum); + HRESULT GetGCBookkeepingMemoryRegions(ISOSMemoryEnum **ppEnum); + HRESULT GetGCFreeRegions(ISOSMemoryEnum **ppEnum); + HRESULT LockedFlush(); } diff --git a/src/coreclr/pal/prebuilt/idl/sospriv_i.cpp b/src/coreclr/pal/prebuilt/idl/sospriv_i.cpp index 07f02d061e86c4..141ec62612e48f 100644 --- a/src/coreclr/pal/prebuilt/idl/sospriv_i.cpp +++ b/src/coreclr/pal/prebuilt/idl/sospriv_i.cpp @@ -5,9 +5,11 @@ /* link this file in with the server and any clients */ - /* File created by MIDL compiler version 8.01.0626 */ + /* File created by MIDL compiler version 8.01.0622 */ +/* at Mon Jan 18 19:14:07 2038 + */ /* Compiler settings for sospriv.idl: - Oicf, W1, Zp8, env=Win64 (32b run), target_arch=AMD64 8.01.0626 + Oicf, W1, Zp8, env=Win64 (32b run), target_arch=AMD64 8.01.0622 protocol : dce , ms_ext, c_ext, robust error checks: allocation ref bounds_check enum stub_data VC __declspec() decoration level: @@ -77,6 +79,9 @@ MIDL_DEFINE_GUID(IID, IID_ISOSStackRefErrorEnum,0x774F4E1B,0xFB7B,0x491B,0x97,0x MIDL_DEFINE_GUID(IID, IID_ISOSStackRefEnum,0x8FA642BD,0x9F10,0x4799,0x9A,0xA3,0x51,0x2A,0xE7,0x8C,0x77,0xEE); +MIDL_DEFINE_GUID(IID, IID_ISOSMemoryEnum,0xE4B860EC,0x337A,0x40C0,0xA5,0x91,0xF0,0x9A,0x96,0x80,0x69,0x0F); + + MIDL_DEFINE_GUID(IID, IID_ISOSDacInterface,0x436f00f2,0xb42a,0x4b9f,0x87,0x0c,0xe7,0x3d,0xb6,0x6a,0xe9,0x30); @@ -113,7 +118,7 @@ MIDL_DEFINE_GUID(IID, IID_ISOSDacInterface11,0x96BA1DB9,0x14CD,0x4492,0x80,0x65, MIDL_DEFINE_GUID(IID, IID_ISOSDacInterface12,0x1b93bacc,0x8ca4,0x432d,0x94,0x3a,0x3e,0x6e,0x7e,0xc0,0xb0,0xa3); -MIDL_DEFINE_GUID(IID, IID_ISOSDacInterface13,0x3176a8ed,0x597b,0x4f54,0xa7,0x1f,0x83,0x69,0x5c,0x6a,0x8c,0x5d); +MIDL_DEFINE_GUID(IID, IID_ISOSDacInterface13,0x3176a8ed,0x597b,0x4f54,0xa7,0x1f,0x83,0x69,0x5c,0x6a,0x8c,0x5e); #undef MIDL_DEFINE_GUID diff --git a/src/coreclr/pal/prebuilt/inc/sospriv.h b/src/coreclr/pal/prebuilt/inc/sospriv.h index cc344809965608..4b930aa871263a 100644 --- a/src/coreclr/pal/prebuilt/inc/sospriv.h +++ b/src/coreclr/pal/prebuilt/inc/sospriv.h @@ -73,6 +73,13 @@ typedef interface ISOSStackRefEnum ISOSStackRefEnum; #endif /* __ISOSStackRefEnum_FWD_DEFINED__ */ +#ifndef __ISOSMemoryEnum_FWD_DEFINED__ +#define __ISOSMemoryEnum_FWD_DEFINED__ +typedef interface ISOSMemoryEnum ISOSMemoryEnum; + +#endif /* __ISOSMemoryEnum_FWD_DEFINED__ */ + + #ifndef __ISOSDacInterface_FWD_DEFINED__ #define __ISOSDacInterface_FWD_DEFINED__ typedef interface ISOSDacInterface ISOSDacInterface; @@ -489,6 +496,17 @@ typedef struct _SOS_StackRefError extern RPC_IF_HANDLE __MIDL_itf_sospriv_0000_0002_v0_0_c_ifspec; extern RPC_IF_HANDLE __MIDL_itf_sospriv_0000_0002_v0_0_s_ifspec; +#ifndef _SOS_MemoryRegion_ +#define _SOS_MemoryRegion_ +typedef struct _SOSMemoryRegion + { + CLRDATA_ADDRESS Start; + CLRDATA_ADDRESS Size; + CLRDATA_ADDRESS ExtraData; + } SOSMemoryRegion; + +#endif // _SOS_MemoryRegion_ + #ifndef __ISOSStackRefErrorEnum_INTERFACE_DEFINED__ #define __ISOSStackRefErrorEnum_INTERFACE_DEFINED__ @@ -594,6 +612,112 @@ EXTERN_C const IID IID_ISOSStackRefErrorEnum; #endif /* __ISOSStackRefErrorEnum_INTERFACE_DEFINED__ */ + +#ifndef __ISOSMemoryEnum_INTERFACE_DEFINED__ +#define __ISOSMemoryEnum_INTERFACE_DEFINED__ + +/* interface ISOSMemoryEnum */ +/* [uuid][local][object] */ + + +EXTERN_C const IID IID_ISOSMemoryEnum; + +#if defined(__cplusplus) && !defined(CINTERFACE) + + MIDL_INTERFACE("E4B860EC-337A-40C0-A591-F09A9680690F") + ISOSMemoryEnum : public ISOSEnum + { + public: + virtual HRESULT STDMETHODCALLTYPE Next( + /* [in] */ unsigned int count, + /* [length_is][size_is][out] */ SOSMemoryRegion memRegion[ ], + /* [out] */ unsigned int *pNeeded) = 0; + + }; + + +#else /* C style interface */ + + typedef struct ISOSMemoryEnumVtbl + { + BEGIN_INTERFACE + + HRESULT ( STDMETHODCALLTYPE *QueryInterface )( + ISOSMemoryEnum * This, + /* [in] */ REFIID riid, + /* [annotation][iid_is][out] */ + _COM_Outptr_ void **ppvObject); + + ULONG ( STDMETHODCALLTYPE *AddRef )( + ISOSMemoryEnum * This); + + ULONG ( STDMETHODCALLTYPE *Release )( + ISOSMemoryEnum * This); + + HRESULT ( STDMETHODCALLTYPE *Skip )( + ISOSMemoryEnum * This, + /* [in] */ unsigned int count); + + HRESULT ( STDMETHODCALLTYPE *Reset )( + ISOSMemoryEnum * This); + + HRESULT ( STDMETHODCALLTYPE *GetCount )( + ISOSMemoryEnum * This, + /* [out] */ unsigned int *pCount); + + HRESULT ( STDMETHODCALLTYPE *Next )( + ISOSMemoryEnum * This, + /* [in] */ unsigned int count, + /* [length_is][size_is][out] */ SOSMemoryRegion memRegion[ ], + /* [out] */ unsigned int *pNeeded); + + END_INTERFACE + } ISOSMemoryEnumVtbl; + + interface ISOSMemoryEnum + { + CONST_VTBL struct ISOSMemoryEnumVtbl *lpVtbl; + }; + + + +#ifdef COBJMACROS + + +#define ISOSMemoryEnum_QueryInterface(This,riid,ppvObject) \ + ( (This)->lpVtbl -> QueryInterface(This,riid,ppvObject) ) + +#define ISOSMemoryEnum_AddRef(This) \ + ( (This)->lpVtbl -> AddRef(This) ) + +#define ISOSMemoryEnum_Release(This) \ + ( (This)->lpVtbl -> Release(This) ) + + +#define ISOSMemoryEnum_Skip(This,count) \ + ( (This)->lpVtbl -> Skip(This,count) ) + +#define ISOSMemoryEnum_Reset(This) \ + ( (This)->lpVtbl -> Reset(This) ) + +#define ISOSMemoryEnum_GetCount(This,pCount) \ + ( (This)->lpVtbl -> GetCount(This,pCount) ) + + +#define ISOSMemoryEnum_Next(This,count,memRegion,pNeeded) \ + ( (This)->lpVtbl -> Next(This,count,memRegion,pNeeded) ) + +#endif /* COBJMACROS */ + + +#endif /* C style interface */ + + + + +#endif /* __ISOSMemoryEnum_INTERFACE_DEFINED__ */ + + #ifndef __ISOSStackRefEnum_INTERFACE_DEFINED__ #define __ISOSStackRefEnum_INTERFACE_DEFINED__ @@ -3084,7 +3208,7 @@ EXTERN_C const IID IID_ISOSDacInterface13; #if defined(__cplusplus) && !defined(CINTERFACE) - MIDL_INTERFACE("3176a8ed-597b-4f54-a71f-83695c6a8c5d") + MIDL_INTERFACE("3176a8ed-597b-4f54-a71f-83695c6a8c5e") ISOSDacInterface13 : public IUnknown { public: @@ -3108,6 +3232,17 @@ EXTERN_C const IID IID_ISOSDacInterface13; CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded) = 0; + + virtual HRESULT STDMETHODCALLTYPE GetHandleTableMemoryRegions( + ISOSMemoryEnum **ppEnum) = 0; + + virtual HRESULT STDMETHODCALLTYPE GetGCBookkeepingMemoryRegions( + ISOSMemoryEnum **ppEnum) = 0; + + virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions( + ISOSMemoryEnum **ppEnum) = 0; + + virtual HRESULT STDMETHODCALLTYPE LockedFlush( void) = 0; }; diff --git a/src/coreclr/vm/decodemd.cpp b/src/coreclr/vm/decodemd.cpp index 6def9757c9b26a..e1f7e8e3877a70 100644 --- a/src/coreclr/vm/decodemd.cpp +++ b/src/coreclr/vm/decodemd.cpp @@ -73,7 +73,6 @@ const BYTE decoded_10[2] = {10, END_DECODED }; #define DECODING_ERROR ((unsigned) -1) #define MASK(len) (~(~0u <<(len))) #define MASK64(len) ((~((~((unsigned __int64)0))<<(len)))) -#define BITS_PER_BYTE (sizeof(BYTE)*8) const Decoder::Decode emptyDecode = {decoded_end, DECODING_HEADER(0)}; From d6b07606751ebf705600796a994b514ecc47beee Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Wed, 5 Apr 2023 15:23:23 -0700 Subject: [PATCH 02/27] Update comment --- src/coreclr/gc/gcinterface.dac.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index e79d1de79a3b7a..e4ea21fec8b0b0 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -127,7 +127,7 @@ enum oom_reason /*!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!*/ /* If you modify failure_get_memory and */ /* oom_reason be sure to make the corresponding */ -/* changes in tools\sos\strike\strike.cpp. */ +/* changes in ClrMD. */ /*!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!*/ enum failure_get_memory { From 2b51bf662cd5cf27b9712f4eddd317becd51ae98 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 10:34:58 -0700 Subject: [PATCH 03/27] Add GC Bookkeeping data to the dac - bookkeeping_covered_start is now compiled into all versions of the GC, instead of just with USE_REGIONS. This allows us to find the base address of the allocated memory for GC Bookkeeping. - Added dac enumeration of GC Bookkeeping. --- src/coreclr/debug/daccess/daccess.cpp | 36 ++++++++++++++++++++++++++ src/coreclr/debug/daccess/dacimpl.h | 7 +++++ src/coreclr/debug/daccess/request.cpp | 27 +++++++++++++++++-- src/coreclr/gc/gc.cpp | 26 ++++++++++++------- src/coreclr/gc/gcinterface.dac.h | 8 ++++++ src/coreclr/gc/gcinterface.dacvars.def | 2 ++ src/coreclr/gc/gcpriv.h | 2 +- 7 files changed, 95 insertions(+), 13 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index a4083027c63c8e..7bab13ea78e6b0 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8325,6 +8325,42 @@ HRESULT DacMemoryEnumerator::Next(unsigned int count, SOSMemoryRegion regions[], return i < count ? S_FALSE : S_OK; } + +HRESULT DacGCBookkeepingEnumerator::Init() +{ + TADDR ctiAddr = TO_TADDR(*g_gcDacGlobals->bookkeeping_covered_start); + DPTR(dac_card_table_info) card_table_info(ctiAddr); + + SOSMemoryRegion mem = {0}; + mem.Start = card_table_info.GetAddr(); + mem.Size = card_table_info->size; + mRegions.Add(mem); + + DPTR(size_t) layout = g_gcDacGlobals->card_table_element_layout; + size_t card_table_info_size = layout[0]; + TADDR nextStart = card_table_info->next_card_table; + TADDR next = nextStart; + + _ASSERTE(sizeof(dac_card_table_info) <= card_table_info_size); + + // The while loop is effectively "while (next != 0)" but with an added check to make + // sure we don't underflow next when subtracting card_table_info_size on a bad pointer. + while (next > card_table_info_size) + { + DPTR(dac_card_table_info) ct(next - card_table_info_size); + + mem = {0}; + mem.Start = ct.GetAddr(); + mem.Size = ct->size; + mRegions.Add(mem); + + next = ct->next_card_table; + } + + return S_OK; +} + + HRESULT DacHandleTableMemoryEnumerator::Init() { int max_slots = 1; diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index da108be41f3b0a..baf5b4734dcfce 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -1986,6 +1986,13 @@ class DacMemoryEnumerator : public DefaultCOMImplQueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); + hr = htEnum->Init(); + + if (SUCCEEDED(hr)) + hr = htEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); + if (FAILED(hr)) delete ppEnum; } @@ -5324,7 +5328,26 @@ HRESULT ClrDataAccess::GetGCBookkeepingMemoryRegions(ISOSMemoryEnum** ppEnum) if (!ppEnum) return E_POINTER; - return E_NOTIMPL; + SOSDacEnter(); + + DacGCBookkeepingEnumerator* htEnum = new (nothrow) DacGCBookkeepingEnumerator(); + if (htEnum) + { + hr = htEnum->Init(); + + if (SUCCEEDED(hr)) + hr = htEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); + + if (FAILED(hr)) + delete ppEnum; + } + else + { + hr = E_OUTOFMEMORY; + } + + SOSDacLeave(); + return hr; } HRESULT ClrDataAccess::GetGCFreeRegions(ISOSMemoryEnum** ppEnum) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 474e041974cf9b..5c6654d2cc46fb 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2228,8 +2228,8 @@ size_t gc_heap::g_bpromoted; #endif //MULTIPLE_HEAPS size_t gc_heap::card_table_element_layout[total_bookkeeping_elements + 1]; -#ifdef USE_REGIONS uint8_t* gc_heap::bookkeeping_covered_start = nullptr; +#ifdef USE_REGIONS uint8_t* gc_heap::bookkeeping_covered_committed = nullptr; size_t gc_heap::bookkeeping_sizes[total_bookkeeping_elements]; #endif //USE_REGIONS @@ -8378,6 +8378,9 @@ class card_table_info { public: unsigned recount; + size_t size; + uint32_t* next_card_table; + uint8_t* lowest_address; uint8_t* highest_address; short* brick_table; @@ -8391,11 +8394,12 @@ class card_table_info #ifdef BACKGROUND_GC uint32_t* mark_array; #endif //BACKGROUND_GC - - size_t size; - uint32_t* next_card_table; }; +static_assert(offsetof(dac_card_table_info, size) == offsetof(card_table_info, size), "DAC card_table_info layout mismatch"); +static_assert(offsetof(dac_card_table_info, next_card_table) == offsetof(card_table_info, next_card_table), "DAC card_table_info layout mismatch"); +static_assert(card_table_element == 0, "DAC takes a dependency on card_table_element_layout[0] == sizeof(card_table_info)"); + //These are accessors on untranslated cardtable inline unsigned& card_table_refcount (uint32_t* c_table) @@ -8996,9 +9000,7 @@ uint32_t* gc_heap::make_card_table (uint8_t* start, uint8_t* end) size_t alloc_size = card_table_element_layout[total_bookkeeping_elements]; uint8_t* mem = (uint8_t*)GCToOSInterface::VirtualReserve (alloc_size, 0, virtual_reserve_flags); -#ifdef USE_REGIONS bookkeeping_covered_start = mem; -#endif //USE_REGIONS if (!mem) return 0; @@ -49291,11 +49293,13 @@ void PopulateDacVars(GcDacVars *gcDacVars) assert(gcDacVars != nullptr); *gcDacVars = {}; - // Note: these version numbers are not actually checked by SOS, so if you change - // the GC in a way that makes it incompatible with SOS, please change - // SOS_BREAKING_CHANGE_VERSION in both the runtime and the diagnostics repo - gcDacVars->major_version_number = 1; + // Note: These version numbers do not need to be checked in the .Net dac/SOS because + // we always match the compiled dac and GC to the version used. NativeAOT's SOS may + // work differently than .Net SOS. When making breaking changes here you may need to + // find NativeAOT's equivalent of SOS_BREAKING_CHANGE_VERSION and increment it. + gcDacVars->major_version_number = 2; gcDacVars->minor_version_number = 0; + gcDacVars->total_bookkeeping_elements = total_bookkeeping_elements; #ifdef USE_REGIONS gcDacVars->minor_version_number |= 1; #endif //USE_REGIONS @@ -49357,4 +49361,6 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->gc_heap_field_offsets = reinterpret_cast(&gc_heap_field_offsets); #endif // MULTIPLE_HEAPS gcDacVars->generation_field_offsets = reinterpret_cast(&generation_field_offsets); + gcDacVars->bookkeeping_covered_start = &gc_heap::bookkeeping_covered_start; + gcDacVars->card_table_element_layout = reinterpret_cast(&gc_heap::card_table_element_layout); } diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index e4ea21fec8b0b0..a4602832830f9e 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -102,6 +102,13 @@ class dac_handle_table_map { uint32_t dwMaxIndex; }; +class dac_card_table_info { +public: + unsigned recount; + size_t size; + TADDR next_card_table; +}; + // Possible values of the current_c_gc_state dacvar, indicating the state of // a background GC. enum c_gc_state @@ -225,6 +232,7 @@ struct GcDacVars { uint8_t minor_version_number; size_t generation_size; size_t total_generation_count; + int total_bookkeeping_elements; #ifdef DACCESS_COMPILE #define GC_DAC_VAR(type, name) DPTR(type) name; #define GC_DAC_PTR_VAR(type, name) DPTR(type*) name; diff --git a/src/coreclr/gc/gcinterface.dacvars.def b/src/coreclr/gc/gcinterface.dacvars.def index a78852a09a220d..ab6aa6a66b2279 100644 --- a/src/coreclr/gc/gcinterface.dacvars.def +++ b/src/coreclr/gc/gcinterface.dacvars.def @@ -62,6 +62,8 @@ GC_DAC_ARRAY_VAR (size_t, interesting_mechanism_bits_per_heap) GC_DAC_VAR (dac_handle_table_map, handle_table_map) GC_DAC_ARRAY_VAR (int, gc_heap_field_offsets) GC_DAC_ARRAY_VAR (int, generation_field_offsets) +GC_DAC_PTR_VAR (uint8_t, bookkeeping_covered_start) +GC_DAC_ARRAY_VAR (size_t, card_table_element_layout) #undef GC_DAC_VAR #undef GC_DAC_ARRAY_VAR diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 3ca72125148d22..ed3abd78a0e50c 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -4192,10 +4192,10 @@ class gc_heap #endif //BGC_SERVO_TUNING #endif //BACKGROUND_GC + PER_HEAP_ISOLATED_FIELD_INIT_ONLY uint8_t* bookkeeping_covered_start; #ifdef USE_REGIONS PER_HEAP_ISOLATED_FIELD_INIT_ONLY size_t regions_range; PER_HEAP_ISOLATED_FIELD_INIT_ONLY bool enable_special_regions_p; - PER_HEAP_ISOLATED_FIELD_INIT_ONLY uint8_t* bookkeeping_covered_start; #else //USE_REGIONS PER_HEAP_ISOLATED_FIELD_INIT_ONLY size_t eph_gen_starts_size; PER_HEAP_ISOLATED_FIELD_INIT_ONLY size_t min_segment_size; From 08a7c690b1fb4b21d5cc3a16287c7df735274e1e Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 10:43:51 -0700 Subject: [PATCH 04/27] Add assert --- src/coreclr/gc/gc.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 5c6654d2cc46fb..837fe732168d3a 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -49363,4 +49363,8 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->generation_field_offsets = reinterpret_cast(&generation_field_offsets); gcDacVars->bookkeeping_covered_start = &gc_heap::bookkeeping_covered_start; gcDacVars->card_table_element_layout = reinterpret_cast(&gc_heap::card_table_element_layout); + + // Dac takes a dependency on card_table_element_layout[0] being the size of card_table_info for walking + // backwards from card_table_info::next_card_table to the beginning of the next card_table_info. + assert(gc_heap::card_table_element_layout[0] == sizeof(card_table_info)); } From f8cc6a17fd53edd386764c71479781e263a3b522 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 15:15:05 -0700 Subject: [PATCH 05/27] Add support for free gc regions --- src/coreclr/debug/daccess/daccess.cpp | 6 +++ src/coreclr/debug/daccess/dacimpl.h | 5 +- src/coreclr/debug/daccess/request.cpp | 59 ++++++++++++++++++++-- src/coreclr/debug/daccess/request_common.h | 12 ++++- src/coreclr/debug/daccess/request_svr.cpp | 15 ++++++ src/coreclr/gc/dac_gcheap_fields.h | 14 ++++- src/coreclr/gc/gc.cpp | 8 +-- src/coreclr/gc/gcinterface.dac.h | 18 ++++++- src/coreclr/gc/gcinterface.dacvars.def | 3 ++ src/coreclr/gc/gcpriv.h | 2 + src/coreclr/inc/sospriv.idl | 2 +- src/coreclr/pal/prebuilt/inc/sospriv.h | 45 +++++++++++++++-- 12 files changed, 170 insertions(+), 19 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 7bab13ea78e6b0..bc66a8db81e056 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8328,7 +8328,13 @@ HRESULT DacMemoryEnumerator::Next(unsigned int count, SOSMemoryRegion regions[], HRESULT DacGCBookkeepingEnumerator::Init() { + if (g_gcDacGlobals->bookkeeping_covered_start == nullptr) + return E_FAIL; + TADDR ctiAddr = TO_TADDR(*g_gcDacGlobals->bookkeeping_covered_start); + if (ctiAddr == 0) + return E_FAIL; + DPTR(dac_card_table_info) card_table_info(ctiAddr); SOSMemoryRegion mem = {0}; diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index baf5b4734dcfce..38420877df75d0 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -1213,7 +1213,7 @@ class ClrDataAccess virtual HRESULT STDMETHODCALLTYPE GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocator, int count, CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded); virtual HRESULT STDMETHODCALLTYPE GetHandleTableMemoryRegions(ISOSMemoryEnum **ppEnum); virtual HRESULT STDMETHODCALLTYPE GetGCBookkeepingMemoryRegions(ISOSMemoryEnum **ppEnum); - virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions(ISOSMemoryEnum **ppEnum); + virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS region[], unsigned int *pNeeded); virtual HRESULT STDMETHODCALLTYPE LockedFlush(); // @@ -1338,6 +1338,9 @@ class ClrDataAccess HRESULT EnumMemDumpAllThreadsStack(CLRDataEnumMemoryFlags flags); HRESULT EnumMemCLRMainModuleInfo(); + void AddFreeRegion(DPTR(dac_region_free_list) free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); + void GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); + bool ReportMem(TADDR addr, TSIZE_T size, bool fExpectSuccess = true); bool DacUpdateMemoryRegion(TADDR addr, TSIZE_T bufferSize, BYTE* buffer); diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index e6de8cc07ca7d9..c4eac389fecc0a 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -5350,12 +5350,65 @@ HRESULT ClrDataAccess::GetGCBookkeepingMemoryRegions(ISOSMemoryEnum** ppEnum) return hr; } -HRESULT ClrDataAccess::GetGCFreeRegions(ISOSMemoryEnum** ppEnum) +void ClrDataAccess::AddFreeRegion(DPTR(dac_region_free_list) free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int& index) { - if (!ppEnum) + if (free_list != nullptr) + { + DPTR(dac_heap_segment) seg = free_list->head_free_region; + while (seg != nullptr) + { + if (regions && index < count) + regions[index++] = seg.GetAddr(); + else + index++; + + seg = seg->next; + if (seg == free_list->head_free_region) + break; + } + } +} + +HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int *pNeeded) +{ + if (!pNeeded) return E_POINTER; - return E_NOTIMPL; + SOSDacEnter(); + + unsigned int index = 0; + if (g_gcDacGlobals->global_free_huge_regions != nullptr) + { + AddFreeRegion(DPTR(dac_region_free_list)(TO_TADDR(*g_gcDacGlobals->global_free_huge_regions)), count, regions, index); + } + + if (g_gcDacGlobals->global_regions_to_decommit != nullptr) + { + DPTR(dac_region_free_list) regionList(g_gcDacGlobals->global_regions_to_decommit); + for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) + AddFreeRegion(regionList, count, regions, index); + } + +#if defined(FEATURE_SVR_GC) + if (GCHeapUtilities::IsServerHeap()) + { + GetServerFreeRegions(count, regions, index); + } + else +#endif //FEATURE_SVR_GC + { + DPTR(dac_region_free_list) regionList(g_gcDacGlobals->free_regions); + for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) + AddFreeRegion(regionList, count, regions, index); + } + + if (regions) + *pNeeded = min(index, count); + else + *pNeeded = index; + + SOSDacLeave(); + return hr; } HRESULT ClrDataAccess::LockedFlush() diff --git a/src/coreclr/debug/daccess/request_common.h b/src/coreclr/debug/daccess/request_common.h index 623761ba250687..588e50cf03b58a 100644 --- a/src/coreclr/debug/daccess/request_common.h +++ b/src/coreclr/debug/daccess/request_common.h @@ -129,9 +129,11 @@ LoadGcHeapData(TADDR heap) #define DEFINE_FIELD(field_name, field_type) LOAD(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) LOAD_DPTR(field_name, field_type) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) LOAD_ARRAY(field_name, field_type, array_length); +#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_gcheap_fields.h" - + +#undef DEFINE_MISSING_FIELD #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD @@ -151,9 +153,11 @@ inline void EnumGcHeap(TADDR heap) #define DEFINE_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) ENUM_ARRAY(field_name, field_type, array_length) +#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_gcheap_fields.h" - + +#undef DEFINE_MISSING_FIELD #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD @@ -175,9 +179,11 @@ LoadGeneration(TADDR generation) #define ALL_FIELDS #define DEFINE_FIELD(field_name, field_type) LOAD(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) LOAD_DPTR(field_name, field_type) +#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_generation_fields.h" +#undef DEFINE_MISSING_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD #undef ALL_FIELDS @@ -196,9 +202,11 @@ inline void EnumGeneration(TADDR generation) #define DEFINE_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) ENUM_ARRAY(field_name, field_type, array_length) +#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_generation_fields.h" +#undef DEFINE_MISSING_FIELD #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index e59255a4e5a68f..ab4958214f703b 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -459,4 +459,19 @@ HRESULT DacHeapWalker::InitHeapDataSvr(HeapData *&pHeaps, size_t &pCount) return S_OK; } +void ClrDataAccess::GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index) +{ + for (int i = 0; i < GCHeapCount(); i++) + { + TADDR heapAddress = (TADDR)HeapTableIndex(g_gcDacGlobals->g_heaps, i); + if (heapAddress == 0) + continue; + + dac_gc_heap heap = LoadGcHeapData(heapAddress); + DPTR(dac_region_free_list) regionList = heap.free_regions; + for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) + AddFreeRegion(regionList, count, regions, index); + } +} + #endif // defined(FEATURE_SVR_GC) diff --git a/src/coreclr/gc/dac_gcheap_fields.h b/src/coreclr/gc/dac_gcheap_fields.h index 104157261635a8..7dddb714bf1aad 100644 --- a/src/coreclr/gc/dac_gcheap_fields.h +++ b/src/coreclr/gc/dac_gcheap_fields.h @@ -18,10 +18,19 @@ DEFINE_FIELD (background_saved_highest_address, uint8_t*) #if defined(ALL_FIELDS) || !defined(USE_REGIONS) DEFINE_DPTR_FIELD (saved_sweep_ephemeral_seg, dac_heap_segment) DEFINE_FIELD (saved_sweep_ephemeral_start, uint8_t*) + +#if defined(ALL_FIELDS) +DEFINE_DPTR_FIELD (free_regions, dac_region_free_list) +#else +DEFINE_MISSING_FIELD(free_regions) +#endif // ALL_FIELDS + #else DEFINE_MISSING_FIELD(saved_sweep_ephemeral_seg) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_start) -#endif +DEFINE_ARRAY_FIELD (free_regions, dac_region_free_list, count_free_region_kinds) +#endif // defined(ALL_FIELDS) || !defined(USE_REGIONS) + #else DEFINE_MISSING_FIELD(mark_array) DEFINE_MISSING_FIELD(next_sweep_obj) @@ -29,4 +38,5 @@ DEFINE_MISSING_FIELD(background_saved_lowest_address) DEFINE_MISSING_FIELD(background_saved_highest_address) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_seg) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_start) -#endif +DEFINE_MISSING_FIELD(free_regions) +#endif // defined(ALL_FIELDS) || defined(BACKGROUND_GC) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 837fe732168d3a..90bc632861ae23 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -49302,6 +49302,9 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->total_bookkeeping_elements = total_bookkeeping_elements; #ifdef USE_REGIONS gcDacVars->minor_version_number |= 1; + gcDacVars->count_free_region_kinds = count_free_region_kinds; + gcDacVars->global_regions_to_decommit = reinterpret_cast(&gc_heap::global_regions_to_decommit); + gcDacVars->global_free_huge_regions = reinterpret_cast(&gc_heap::global_free_huge_regions); #endif //USE_REGIONS #ifndef BACKGROUND_GC gcDacVars->minor_version_number |= 2; @@ -49327,6 +49330,7 @@ void PopulateDacVars(GcDacVars *gcDacVars) #ifdef USE_REGIONS gcDacVars->saved_sweep_ephemeral_seg = 0; gcDacVars->saved_sweep_ephemeral_start = 0; + gcDacVars->free_regions = reinterpret_cast(&gc_heap::free_regions); #else gcDacVars->saved_sweep_ephemeral_seg = reinterpret_cast(&gc_heap::saved_sweep_ephemeral_seg); gcDacVars->saved_sweep_ephemeral_start = &gc_heap::saved_sweep_ephemeral_start; @@ -49363,8 +49367,4 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->generation_field_offsets = reinterpret_cast(&generation_field_offsets); gcDacVars->bookkeeping_covered_start = &gc_heap::bookkeeping_covered_start; gcDacVars->card_table_element_layout = reinterpret_cast(&gc_heap::card_table_element_layout); - - // Dac takes a dependency on card_table_element_layout[0] being the size of card_table_info for walking - // backwards from card_table_info::next_card_table to the beginning of the next card_table_info. - assert(gc_heap::card_table_element_layout[0] == sizeof(card_table_info)); } diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index a4602832830f9e..3627dc0adbf181 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -42,6 +42,17 @@ class dac_heap_segment { class dac_gc_heap* heap; }; +class dac_region_free_list { +public: + size_t num_free_regions; + size_t size_free_regions; + size_t size_committed_in_free_regions; + size_t num_free_regions_added; + size_t num_free_regions_removed; + DPTR(dac_heap_segment) head_free_region; + DPTR(dac_heap_segment) tail_free_region; +}; + // Analogue for the GC generation class, containing information about the start segment // of a generation and its allocation context. class dac_generation { @@ -49,12 +60,14 @@ class dac_generation { #define ALL_FIELDS #define DEFINE_FIELD(field_name, field_type) field_type field_name; #define DEFINE_DPTR_FIELD(field_name, field_type) DPTR(field_type) field_name; +#define DEFINE_MISSING_FIELD(field_name) #include "dac_generation_fields.h" #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD #undef ALL_FIELDS +#undef DEFINE_MISSING_FIELD }; // Analogue for the GC CFinalize class, containing information about the finalize queue. @@ -169,6 +182,7 @@ class dac_gc_heap { #define DEFINE_FIELD(field_name, field_type) field_type field_name; #define DEFINE_DPTR_FIELD(field_name, field_type) DPTR(field_type) field_name; #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) field_type field_name[array_length]; +#define DEFINE_MISSING_FIELD(field_name) #include "dac_gcheap_fields.h" @@ -176,6 +190,7 @@ class dac_gc_heap { #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD #undef ALL_FIELDS +#undef DEFINE_MISSING_FIELD // The generation table must always be last, because the size of this array // (stored inline in the gc_heap class) can vary. @@ -191,7 +206,7 @@ class dac_gc_heap { dac_generation generation_table[1]; }; -#define GENERATION_TABLE_FIELD_INDEX 18 +#define GENERATION_TABLE_FIELD_INDEX 19 // Unlike other DACized structures, these types are loaded manually in the debugger. // To avoid misuse, pointers to them are explicitly casted to these unused type. @@ -233,6 +248,7 @@ struct GcDacVars { size_t generation_size; size_t total_generation_count; int total_bookkeeping_elements; + int count_free_region_kinds; #ifdef DACCESS_COMPILE #define GC_DAC_VAR(type, name) DPTR(type) name; #define GC_DAC_PTR_VAR(type, name) DPTR(type*) name; diff --git a/src/coreclr/gc/gcinterface.dacvars.def b/src/coreclr/gc/gcinterface.dacvars.def index ab6aa6a66b2279..b6ebc3963bedc8 100644 --- a/src/coreclr/gc/gcinterface.dacvars.def +++ b/src/coreclr/gc/gcinterface.dacvars.def @@ -64,6 +64,9 @@ GC_DAC_ARRAY_VAR (int, gc_heap_field_offsets) GC_DAC_ARRAY_VAR (int, generation_field_offsets) GC_DAC_PTR_VAR (uint8_t, bookkeeping_covered_start) GC_DAC_ARRAY_VAR (size_t, card_table_element_layout) +GC_DAC_ARRAY_VAR (dac_region_free_list, global_regions_to_decommit) +GC_DAC_PTR_VAR (dac_region_free_list, global_free_huge_regions) +GC_DAC_ARRAY_VAR (dac_region_free_list, free_regions) #undef GC_DAC_VAR #undef GC_DAC_ARRAY_VAR diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index ed3abd78a0e50c..c334c05331c77b 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1379,6 +1379,8 @@ class region_free_list void sort_by_committed_and_age(); static bool is_on_free_list (heap_segment* region, region_free_list free_list[count_free_region_kinds]); }; + +static_assert(sizeof(region_free_list) == sizeof(dac_region_free_list)); #endif enum bookkeeping_element diff --git a/src/coreclr/inc/sospriv.idl b/src/coreclr/inc/sospriv.idl index c84eb88e7a0d12..6cc243ac651cac 100644 --- a/src/coreclr/inc/sospriv.idl +++ b/src/coreclr/inc/sospriv.idl @@ -500,6 +500,6 @@ interface ISOSDacInterface13 : IUnknown HRESULT GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocator, int count, CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded); HRESULT GetHandleTableMemoryRegions(ISOSMemoryEnum **ppEnum); HRESULT GetGCBookkeepingMemoryRegions(ISOSMemoryEnum **ppEnum); - HRESULT GetGCFreeRegions(ISOSMemoryEnum **ppEnum); + HRESULT GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS region[], unsigned int *pNeeded); HRESULT LockedFlush(); } diff --git a/src/coreclr/pal/prebuilt/inc/sospriv.h b/src/coreclr/pal/prebuilt/inc/sospriv.h index 4b930aa871263a..777e6cc5d399f3 100644 --- a/src/coreclr/pal/prebuilt/inc/sospriv.h +++ b/src/coreclr/pal/prebuilt/inc/sospriv.h @@ -3240,7 +3240,9 @@ EXTERN_C const IID IID_ISOSDacInterface13; ISOSMemoryEnum **ppEnum) = 0; virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions( - ISOSMemoryEnum **ppEnum) = 0; + unsigned int count, + CLRDATA_ADDRESS region[ ], + unsigned int *pNeeded) = 0; virtual HRESULT STDMETHODCALLTYPE LockedFlush( void) = 0; }; @@ -3248,32 +3250,65 @@ EXTERN_C const IID IID_ISOSDacInterface13; #else /* C style interface */ + typedef struct ISOSDacInterface13Vtbl { BEGIN_INTERFACE - DECLSPEC_XFGVIRT(IUnknown, QueryInterface) HRESULT ( STDMETHODCALLTYPE *QueryInterface )( ISOSDacInterface13 * This, /* [in] */ REFIID riid, /* [annotation][iid_is][out] */ _COM_Outptr_ void **ppvObject); - DECLSPEC_XFGVIRT(IUnknown, AddRef) ULONG ( STDMETHODCALLTYPE *AddRef )( ISOSDacInterface13 * This); - DECLSPEC_XFGVIRT(IUnknown, Release) ULONG ( STDMETHODCALLTYPE *Release )( ISOSDacInterface13 * This); - DECLSPEC_XFGVIRT(ISOSDacInterface13, TraverseLoaderHeap) HRESULT ( STDMETHODCALLTYPE *TraverseLoaderHeap )( ISOSDacInterface13 * This, CLRDATA_ADDRESS loaderHeapAddr, LoaderHeapKind kind, VISITHEAP pCallback); + HRESULT ( STDMETHODCALLTYPE *GetDomainLoaderAllocator )( + ISOSDacInterface13 * This, + CLRDATA_ADDRESS domainAddress, + CLRDATA_ADDRESS *pLoaderAllocator); + + HRESULT ( STDMETHODCALLTYPE *GetLoaderAllocatorHeapNames )( + ISOSDacInterface13 * This, + int count, + const unsigned char **ppNames, + int *pNeeded); + + HRESULT ( STDMETHODCALLTYPE *GetLoaderAllocatorHeaps )( + ISOSDacInterface13 * This, + CLRDATA_ADDRESS loaderAllocator, + int count, + CLRDATA_ADDRESS *pLoaderHeaps, + LoaderHeapKind *pKinds, + int *pNeeded); + + HRESULT ( STDMETHODCALLTYPE *GetHandleTableMemoryRegions )( + ISOSDacInterface13 * This, + ISOSMemoryEnum **ppEnum); + + HRESULT ( STDMETHODCALLTYPE *GetGCBookkeepingMemoryRegions )( + ISOSDacInterface13 * This, + ISOSMemoryEnum **ppEnum); + + HRESULT ( STDMETHODCALLTYPE *GetGCFreeRegions )( + ISOSDacInterface13 * This, + unsigned int count, + CLRDATA_ADDRESS region[ ], + unsigned int *pNeeded); + + HRESULT ( STDMETHODCALLTYPE *LockedFlush )( + ISOSDacInterface13 * This); + END_INTERFACE } ISOSDacInterface13Vtbl; From 15ec2307b6f74a5bab88ec9de4db01e484abf353 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 15:29:30 -0700 Subject: [PATCH 06/27] Don't require card_table_element_layout --- src/coreclr/debug/daccess/daccess.cpp | 8 +++----- src/coreclr/gc/gc.cpp | 7 +++++-- src/coreclr/gc/gcinterface.dac.h | 1 + src/coreclr/gc/gcinterface.dacvars.def | 1 - 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index bc66a8db81e056..643c3a15158717 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8342,15 +8342,13 @@ HRESULT DacGCBookkeepingEnumerator::Init() mem.Size = card_table_info->size; mRegions.Add(mem); - DPTR(size_t) layout = g_gcDacGlobals->card_table_element_layout; - size_t card_table_info_size = layout[0]; + size_t card_table_info_size = g_gcDacGlobals->card_table_info_size; TADDR nextStart = card_table_info->next_card_table; TADDR next = nextStart; - _ASSERTE(sizeof(dac_card_table_info) <= card_table_info_size); - // The while loop is effectively "while (next != 0)" but with an added check to make - // sure we don't underflow next when subtracting card_table_info_size on a bad pointer. + // sure we don't underflow next when subtracting card_table_info_size if we encounter + // a bad pointer. while (next > card_table_info_size) { DPTR(dac_card_table_info) ct(next - card_table_info_size); diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 90bc632861ae23..87af1b64a08479 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -8398,7 +8398,6 @@ class card_table_info static_assert(offsetof(dac_card_table_info, size) == offsetof(card_table_info, size), "DAC card_table_info layout mismatch"); static_assert(offsetof(dac_card_table_info, next_card_table) == offsetof(card_table_info, next_card_table), "DAC card_table_info layout mismatch"); -static_assert(card_table_element == 0, "DAC takes a dependency on card_table_element_layout[0] == sizeof(card_table_info)"); //These are accessors on untranslated cardtable inline @@ -8627,6 +8626,9 @@ void gc_heap::clear_mark_array (uint8_t* from, uint8_t* end) inline uint32_t*& card_table_next (uint32_t* c_table) { + // NOTE: The dac takes a dependency on card_table_info being right before c_table. + // It's 100% ok to change this implementation detail as long as a matching change + // is made to DacGCBookkeepingEnumerator::Init in daccess.cpp. return ((card_table_info*)((uint8_t*)c_table - sizeof (card_table_info)))->next_card_table; } @@ -49300,6 +49302,8 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->major_version_number = 2; gcDacVars->minor_version_number = 0; gcDacVars->total_bookkeeping_elements = total_bookkeeping_elements; + gcDacVars->card_table_info_size = sizeof(card_table_info); + #ifdef USE_REGIONS gcDacVars->minor_version_number |= 1; gcDacVars->count_free_region_kinds = count_free_region_kinds; @@ -49366,5 +49370,4 @@ void PopulateDacVars(GcDacVars *gcDacVars) #endif // MULTIPLE_HEAPS gcDacVars->generation_field_offsets = reinterpret_cast(&generation_field_offsets); gcDacVars->bookkeeping_covered_start = &gc_heap::bookkeeping_covered_start; - gcDacVars->card_table_element_layout = reinterpret_cast(&gc_heap::card_table_element_layout); } diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index 3627dc0adbf181..f095d0d56a598f 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -249,6 +249,7 @@ struct GcDacVars { size_t total_generation_count; int total_bookkeeping_elements; int count_free_region_kinds; + size_t card_table_info_size; #ifdef DACCESS_COMPILE #define GC_DAC_VAR(type, name) DPTR(type) name; #define GC_DAC_PTR_VAR(type, name) DPTR(type*) name; diff --git a/src/coreclr/gc/gcinterface.dacvars.def b/src/coreclr/gc/gcinterface.dacvars.def index b6ebc3963bedc8..dc57c51a3cd243 100644 --- a/src/coreclr/gc/gcinterface.dacvars.def +++ b/src/coreclr/gc/gcinterface.dacvars.def @@ -63,7 +63,6 @@ GC_DAC_VAR (dac_handle_table_map, handle_table_map) GC_DAC_ARRAY_VAR (int, gc_heap_field_offsets) GC_DAC_ARRAY_VAR (int, generation_field_offsets) GC_DAC_PTR_VAR (uint8_t, bookkeeping_covered_start) -GC_DAC_ARRAY_VAR (size_t, card_table_element_layout) GC_DAC_ARRAY_VAR (dac_region_free_list, global_regions_to_decommit) GC_DAC_PTR_VAR (dac_region_free_list, global_free_huge_regions) GC_DAC_ARRAY_VAR (dac_region_free_list, free_regions) From 880e4282e68a0ed8e2be89db8092e5f551064fcf Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 17:11:40 -0700 Subject: [PATCH 07/27] Fix issue with naming in request --- src/coreclr/debug/daccess/request.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index c4eac389fecc0a..eef7199550bff8 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -5312,7 +5312,7 @@ HRESULT ClrDataAccess::GetHandleTableMemoryRegions(ISOSMemoryEnum** ppEnum) hr = htEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); if (FAILED(hr)) - delete ppEnum; + delete htEnum; } else { @@ -5330,16 +5330,16 @@ HRESULT ClrDataAccess::GetGCBookkeepingMemoryRegions(ISOSMemoryEnum** ppEnum) SOSDacEnter(); - DacGCBookkeepingEnumerator* htEnum = new (nothrow) DacGCBookkeepingEnumerator(); - if (htEnum) + DacGCBookkeepingEnumerator* bkEnum = new (nothrow) DacGCBookkeepingEnumerator(); + if (bkEnum) { - hr = htEnum->Init(); + hr = bkEnum->Init(); if (SUCCEEDED(hr)) - hr = htEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); + hr = bkEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); if (FAILED(hr)) - delete ppEnum; + delete bkEnum; } else { From 04b0127baca369aad2e70b1923347132c795812d Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 17:21:06 -0700 Subject: [PATCH 08/27] Remote DEFINE_MISSING_FIELD from dac We should never have missing fields in the dac. This is a leftover from previous code. --- src/coreclr/debug/daccess/request_common.h | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/coreclr/debug/daccess/request_common.h b/src/coreclr/debug/daccess/request_common.h index 588e50cf03b58a..3168bf914e3773 100644 --- a/src/coreclr/debug/daccess/request_common.h +++ b/src/coreclr/debug/daccess/request_common.h @@ -129,11 +129,9 @@ LoadGcHeapData(TADDR heap) #define DEFINE_FIELD(field_name, field_type) LOAD(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) LOAD_DPTR(field_name, field_type) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) LOAD_ARRAY(field_name, field_type, array_length); -#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_gcheap_fields.h" - -#undef DEFINE_MISSING_FIELD + #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD @@ -153,11 +151,9 @@ inline void EnumGcHeap(TADDR heap) #define DEFINE_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) ENUM_ARRAY(field_name, field_type, array_length) -#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_gcheap_fields.h" -#undef DEFINE_MISSING_FIELD #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD @@ -179,11 +175,9 @@ LoadGeneration(TADDR generation) #define ALL_FIELDS #define DEFINE_FIELD(field_name, field_type) LOAD(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) LOAD_DPTR(field_name, field_type) -#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_generation_fields.h" -#undef DEFINE_MISSING_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD #undef ALL_FIELDS @@ -202,11 +196,9 @@ inline void EnumGeneration(TADDR generation) #define DEFINE_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_DPTR_FIELD(field_name, field_type) ENUM(field_name, field_type) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) ENUM_ARRAY(field_name, field_type, array_length) -#define DEFINE_MISSING_FIELD(field_name) #include "../../gc/dac_generation_fields.h" -#undef DEFINE_MISSING_FIELD #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD From dc1e10f2a27614e2b6513c94135c80fc9fefd438 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 17:23:05 -0700 Subject: [PATCH 09/27] Whitespace fix --- src/coreclr/debug/daccess/request_common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/request_common.h b/src/coreclr/debug/daccess/request_common.h index 3168bf914e3773..623761ba250687 100644 --- a/src/coreclr/debug/daccess/request_common.h +++ b/src/coreclr/debug/daccess/request_common.h @@ -153,7 +153,7 @@ inline void EnumGcHeap(TADDR heap) #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) ENUM_ARRAY(field_name, field_type, array_length) #include "../../gc/dac_gcheap_fields.h" - + #undef DEFINE_ARRAY_FIELD #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD From 28a193dc02a86be296a440df82fd887e7a585e75 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 17:48:51 -0700 Subject: [PATCH 10/27] Break if we loop --- src/coreclr/debug/daccess/daccess.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 643c3a15158717..4e568c483b6d76 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8359,6 +8359,8 @@ HRESULT DacGCBookkeepingEnumerator::Init() mRegions.Add(mem); next = ct->next_card_table; + if (next == nextStart) + break; } return S_OK; From 464d65dfdde953148a85ddc81ad4c8e226a0ef62 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 6 Apr 2023 17:50:57 -0700 Subject: [PATCH 11/27] Fix static_assert --- src/coreclr/gc/gcpriv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index c334c05331c77b..8f3c156e999d82 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1380,7 +1380,7 @@ class region_free_list static bool is_on_free_list (heap_segment* region, region_free_list free_list[count_free_region_kinds]); }; -static_assert(sizeof(region_free_list) == sizeof(dac_region_free_list)); +static_assert(sizeof(region_free_list) == sizeof(dac_region_free_list), "The DAC relies on the size of these two types matching for pointer arithmetic."); #endif enum bookkeeping_element From 216a213eeb203137617599c30e5f86a4448768b9 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 7 Apr 2023 14:29:55 -0700 Subject: [PATCH 12/27] Segment fixes When using segments, update bookkeeping_covered_start and card_table_info.size when we update the card table. --- src/coreclr/gc/gc.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 87af1b64a08479..17452c1521b940 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -9499,6 +9499,8 @@ void gc_heap::copy_brick_card_table() uint32_t* ct = &g_gc_card_table[card_word (gcard_of (g_gc_lowest_address))]; own_card_table (ct); card_table = translate_card_table (ct); + bookkeeping_covered_start = (uint8_t*)ct - sizeof(card_table_info); + card_table_size(ct) = card_table_element_layout[total_bookkeeping_elements]; /* End of global lock */ highest_address = card_table_highest_address (ct); lowest_address = card_table_lowest_address (ct); From ae71dd00114a5f3086fd91a665c1e1dc929eebb5 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 7 Apr 2023 14:48:02 -0700 Subject: [PATCH 13/27] Defensive coding Check how many times we loop through memory to guard against heap corruption. --- src/coreclr/debug/daccess/daccess.cpp | 13 ++++++++++++- src/coreclr/debug/daccess/request.cpp | 19 ++++++++++++++++--- src/coreclr/debug/daccess/request_svr.cpp | 5 +++-- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 4e568c483b6d76..204d352a44cf9d 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8346,6 +8346,10 @@ HRESULT DacGCBookkeepingEnumerator::Init() TADDR nextStart = card_table_info->next_card_table; TADDR next = nextStart; + // Cap the number of regions we will walk in case we have run into some kind of + // memory corruption. We shouldn't have more than 2 linked card tables anyway. + int maxRegions = 8; + // The while loop is effectively "while (next != 0)" but with an added check to make // sure we don't underflow next when subtracting card_table_info_size if we encounter // a bad pointer. @@ -8361,6 +8365,9 @@ HRESULT DacGCBookkeepingEnumerator::Init() next = ct->next_card_table; if (next == nextStart) break; + + if (--maxRegions <= 0) + break; } return S_OK; @@ -8376,7 +8383,11 @@ HRESULT DacHandleTableMemoryEnumerator::Init() max_slots = GCHeapCount(); #endif // FEATURE_SVR_GC - for (dac_handle_table_map *map = g_gcDacGlobals->handle_table_map; map; map = map->pNext) + // Cap the number of regions we will walk in case we hit an infinite loop due + // to memory corruption + int maxRegions = 8192; + + for (dac_handle_table_map *map = g_gcDacGlobals->handle_table_map; map && maxRegions >= 0; map = map->pNext, maxRegions--) { for (int i = 0; i < INITIAL_HANDLE_TABLE_ARRAY_SIZE; ++i) { diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index eef7199550bff8..a6f59592b4921d 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -5354,6 +5354,10 @@ void ClrDataAccess::AddFreeRegion(DPTR(dac_region_free_list) free_list, unsigned { if (free_list != nullptr) { + // Cap the number of regions we will walk in case we hit memory corruption that + // puts us into an infinite loop. + int maxRegions = 4096; + DPTR(dac_heap_segment) seg = free_list->head_free_region; while (seg != nullptr) { @@ -5365,6 +5369,9 @@ void ClrDataAccess::AddFreeRegion(DPTR(dac_region_free_list) free_list, unsigned seg = seg->next; if (seg == free_list->head_free_region) break; + + if (--maxRegions <= 0) + break; } } } @@ -5376,6 +5383,11 @@ HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regi SOSDacEnter(); + // Cap the number of free regions we will walk at a sensible number. This is to protect against + // memory corruption, un-initialized data, or just a bug. + int count_free_region_kinds = g_gcDacGlobals->count_free_region_kinds; + count_free_region_kinds = min(count_free_region_kinds, 16); + unsigned int index = 0; if (g_gcDacGlobals->global_free_huge_regions != nullptr) { @@ -5385,7 +5397,7 @@ HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regi if (g_gcDacGlobals->global_regions_to_decommit != nullptr) { DPTR(dac_region_free_list) regionList(g_gcDacGlobals->global_regions_to_decommit); - for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) + for (int i = 0; i < count_free_region_kinds; i++, regionList++) AddFreeRegion(regionList, count, regions, index); } @@ -5398,8 +5410,9 @@ HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regi #endif //FEATURE_SVR_GC { DPTR(dac_region_free_list) regionList(g_gcDacGlobals->free_regions); - for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) - AddFreeRegion(regionList, count, regions, index); + if (regionList != nullptr) + for (int i = 0; i < count_free_region_kinds; i++, regionList++) + AddFreeRegion(regionList, count, regions, index); } if (regions) diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index ab4958214f703b..74ef56adccdc37 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -469,8 +469,9 @@ void ClrDataAccess::GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS reg dac_gc_heap heap = LoadGcHeapData(heapAddress); DPTR(dac_region_free_list) regionList = heap.free_regions; - for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) - AddFreeRegion(regionList, count, regions, index); + if (regionList != nullptr) + for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) + AddFreeRegion(regionList, count, regions, index); } } From ffa86c72db0d6dd1cc0bb3c7f6c16ccb1c6a948a Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 7 Apr 2023 17:03:25 -0700 Subject: [PATCH 14/27] free_region fixes --- src/coreclr/debug/daccess/daccess.cpp | 7 ++-- src/coreclr/debug/daccess/dacimpl.h | 2 +- src/coreclr/debug/daccess/request.cpp | 42 +++++++++++------------ src/coreclr/debug/daccess/request_svr.cpp | 14 +++++--- src/coreclr/gc/dac_gcheap_fields.h | 17 ++++----- src/coreclr/gc/gc.cpp | 4 ++- src/coreclr/gc/gcinterface.dac.h | 1 + src/coreclr/gc/gcpriv.h | 2 ++ 8 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 204d352a44cf9d..89d284fe7fb854 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8343,14 +8343,13 @@ HRESULT DacGCBookkeepingEnumerator::Init() mRegions.Add(mem); size_t card_table_info_size = g_gcDacGlobals->card_table_info_size; - TADDR nextStart = card_table_info->next_card_table; - TADDR next = nextStart; + TADDR next = card_table_info->next_card_table; // Cap the number of regions we will walk in case we have run into some kind of // memory corruption. We shouldn't have more than 2 linked card tables anyway. int maxRegions = 8; - // The while loop is effectively "while (next != 0)" but with an added check to make + // This loop is effectively "while (next != 0)" but with an added check to make // sure we don't underflow next when subtracting card_table_info_size if we encounter // a bad pointer. while (next > card_table_info_size) @@ -8363,7 +8362,7 @@ HRESULT DacGCBookkeepingEnumerator::Init() mRegions.Add(mem); next = ct->next_card_table; - if (next == nextStart) + if (next == card_table_info->next_card_table) break; if (--maxRegions <= 0) diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index 38420877df75d0..0358671bb009ba 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -1338,7 +1338,7 @@ class ClrDataAccess HRESULT EnumMemDumpAllThreadsStack(CLRDataEnumMemoryFlags flags); HRESULT EnumMemCLRMainModuleInfo(); - void AddFreeRegion(DPTR(dac_region_free_list) free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); + void AddFreeRegion(const dac_region_free_list &free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); void GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); bool ReportMem(TADDR addr, TSIZE_T size, bool fExpectSuccess = true); diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index a6f59592b4921d..422ef264d18040 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -5350,29 +5350,26 @@ HRESULT ClrDataAccess::GetGCBookkeepingMemoryRegions(ISOSMemoryEnum** ppEnum) return hr; } -void ClrDataAccess::AddFreeRegion(DPTR(dac_region_free_list) free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int& index) +void ClrDataAccess::AddFreeRegion(const dac_region_free_list &free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int& index) { - if (free_list != nullptr) - { - // Cap the number of regions we will walk in case we hit memory corruption that - // puts us into an infinite loop. - int maxRegions = 4096; + // Cap the number of regions we will walk in case we hit memory corruption that + // puts us into an infinite loop. + int maxRegions = 4096; - DPTR(dac_heap_segment) seg = free_list->head_free_region; - while (seg != nullptr) - { - if (regions && index < count) - regions[index++] = seg.GetAddr(); - else - index++; + DPTR(dac_heap_segment) seg = free_list.head_free_region; + while (seg != nullptr) + { + if (regions && index < count) + regions[index++] = seg.GetAddr(); + else + index++; - seg = seg->next; - if (seg == free_list->head_free_region) - break; + seg = seg->next; + if (seg == free_list.head_free_region) + break; - if (--maxRegions <= 0) - break; - } + if (--maxRegions <= 0) + break; } } @@ -5391,14 +5388,15 @@ HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regi unsigned int index = 0; if (g_gcDacGlobals->global_free_huge_regions != nullptr) { - AddFreeRegion(DPTR(dac_region_free_list)(TO_TADDR(*g_gcDacGlobals->global_free_huge_regions)), count, regions, index); + DPTR(dac_region_free_list) global_free_huge_regions(g_gcDacGlobals->global_free_huge_regions); + AddFreeRegion(*global_free_huge_regions, count, regions, index); } if (g_gcDacGlobals->global_regions_to_decommit != nullptr) { DPTR(dac_region_free_list) regionList(g_gcDacGlobals->global_regions_to_decommit); for (int i = 0; i < count_free_region_kinds; i++, regionList++) - AddFreeRegion(regionList, count, regions, index); + AddFreeRegion(*regionList, count, regions, index); } #if defined(FEATURE_SVR_GC) @@ -5412,7 +5410,7 @@ HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regi DPTR(dac_region_free_list) regionList(g_gcDacGlobals->free_regions); if (regionList != nullptr) for (int i = 0; i < count_free_region_kinds; i++, regionList++) - AddFreeRegion(regionList, count, regions, index); + AddFreeRegion(*regionList, count, regions, index); } if (regions) diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index 74ef56adccdc37..c28c25ee9500eb 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -461,17 +461,23 @@ HRESULT DacHeapWalker::InitHeapDataSvr(HeapData *&pHeaps, size_t &pCount) void ClrDataAccess::GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index) { + // Cap the number of free regions we will walk at a sensible number. This is to protect against + // memory corruption, un-initialized data, or just a bug. + int count_free_region_kinds = g_gcDacGlobals->count_free_region_kinds; + count_free_region_kinds = min(count_free_region_kinds, 16); + for (int i = 0; i < GCHeapCount(); i++) { TADDR heapAddress = (TADDR)HeapTableIndex(g_gcDacGlobals->g_heaps, i); if (heapAddress == 0) continue; + + dac_gc_heap heap = LoadGcHeapData(heapAddress); - DPTR(dac_region_free_list) regionList = heap.free_regions; - if (regionList != nullptr) - for (int i = 0; i < g_gcDacGlobals->count_free_region_kinds; i++, regionList++) - AddFreeRegion(regionList, count, regions, index); + if (heap.free_regions != nullptr) + for (int i = 0; i < count_free_region_kinds; i++) + AddFreeRegion(heap.free_regions[i], count, regions, index); } } diff --git a/src/coreclr/gc/dac_gcheap_fields.h b/src/coreclr/gc/dac_gcheap_fields.h index 7dddb714bf1aad..c166f8dda9eb5f 100644 --- a/src/coreclr/gc/dac_gcheap_fields.h +++ b/src/coreclr/gc/dac_gcheap_fields.h @@ -10,6 +10,7 @@ DEFINE_FIELD (internal_root_array, uint8_t*) DEFINE_FIELD (internal_root_array_index, size_t) DEFINE_FIELD (heap_analyze_success, BOOL) DEFINE_FIELD (card_table, uint32_t*) + #if defined(ALL_FIELDS) || defined(BACKGROUND_GC) DEFINE_FIELD (mark_array, uint32_t*) DEFINE_FIELD (next_sweep_obj, uint8_t*) @@ -18,19 +19,10 @@ DEFINE_FIELD (background_saved_highest_address, uint8_t*) #if defined(ALL_FIELDS) || !defined(USE_REGIONS) DEFINE_DPTR_FIELD (saved_sweep_ephemeral_seg, dac_heap_segment) DEFINE_FIELD (saved_sweep_ephemeral_start, uint8_t*) - -#if defined(ALL_FIELDS) -DEFINE_DPTR_FIELD (free_regions, dac_region_free_list) -#else -DEFINE_MISSING_FIELD(free_regions) -#endif // ALL_FIELDS - #else DEFINE_MISSING_FIELD(saved_sweep_ephemeral_seg) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_start) -DEFINE_ARRAY_FIELD (free_regions, dac_region_free_list, count_free_region_kinds) #endif // defined(ALL_FIELDS) || !defined(USE_REGIONS) - #else DEFINE_MISSING_FIELD(mark_array) DEFINE_MISSING_FIELD(next_sweep_obj) @@ -38,5 +30,10 @@ DEFINE_MISSING_FIELD(background_saved_lowest_address) DEFINE_MISSING_FIELD(background_saved_highest_address) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_seg) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_start) -DEFINE_MISSING_FIELD(free_regions) #endif // defined(ALL_FIELDS) || defined(BACKGROUND_GC) + +#if defined(ALL_FIELDS) || defined(USE_REGIONS) +DEFINE_ARRAY_FIELD (free_regions, dac_region_free_list, 4) +#else +DEFINE_MISSING_FIELD(free_regions) +#endif // ALL_FIELDS diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 17452c1521b940..19e1357303fc56 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -49328,6 +49328,9 @@ void PopulateDacVars(GcDacVars *gcDacVars) #endif //BACKGROUND_GC #ifndef MULTIPLE_HEAPS gcDacVars->ephemeral_heap_segment = reinterpret_cast(&gc_heap::ephemeral_heap_segment); +#ifdef USE_REGIONS + gcDacVars->free_regions = reinterpret_cast(&gc_heap::free_regions); +#endif #ifdef BACKGROUND_GC gcDacVars->mark_array = &gc_heap::mark_array; gcDacVars->background_saved_lowest_address = &gc_heap::background_saved_lowest_address; @@ -49336,7 +49339,6 @@ void PopulateDacVars(GcDacVars *gcDacVars) #ifdef USE_REGIONS gcDacVars->saved_sweep_ephemeral_seg = 0; gcDacVars->saved_sweep_ephemeral_start = 0; - gcDacVars->free_regions = reinterpret_cast(&gc_heap::free_regions); #else gcDacVars->saved_sweep_ephemeral_seg = reinterpret_cast(&gc_heap::saved_sweep_ephemeral_seg); gcDacVars->saved_sweep_ephemeral_start = &gc_heap::saved_sweep_ephemeral_start; diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index f095d0d56a598f..ca9b944a906c26 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -16,6 +16,7 @@ #define MAX_EXPAND_MECHANISMS_COUNT 6 #define MAX_GC_MECHANISM_BITS_COUNT 2 #define MAX_GLOBAL_GC_MECHANISMS_COUNT 6 +#define FREE_REGION_KINDS 3 // The number of generations is hardcoded in to the dac APIS (DacpGcHeapDetails hard codes the size of its arrays) // The number of generations is hardcoded into some older dac APIS (for example DacpGcHeapDetails hard codes the size of its arrays) diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 8f3c156e999d82..f50a896b7dfe8e 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1343,6 +1343,8 @@ enum free_region_kind count_free_region_kinds, }; +static_assert(count_free_region_kinds == FREE_REGION_KINDS, "Keep count_free_region_kinds in sync with FREE_REGION_KINDS, changing this is not a version breaking change."); + class region_free_list { size_t num_free_regions; From 8afc64efbdd7a3955304fd4b2429e2a853cddf69 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 7 Apr 2023 17:18:57 -0700 Subject: [PATCH 15/27] Remove hardcoding of region list --- src/coreclr/gc/dac_gcheap_fields.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/dac_gcheap_fields.h b/src/coreclr/gc/dac_gcheap_fields.h index c166f8dda9eb5f..399bee5b44a272 100644 --- a/src/coreclr/gc/dac_gcheap_fields.h +++ b/src/coreclr/gc/dac_gcheap_fields.h @@ -33,7 +33,7 @@ DEFINE_MISSING_FIELD(saved_sweep_ephemeral_start) #endif // defined(ALL_FIELDS) || defined(BACKGROUND_GC) #if defined(ALL_FIELDS) || defined(USE_REGIONS) -DEFINE_ARRAY_FIELD (free_regions, dac_region_free_list, 4) +DEFINE_ARRAY_FIELD (free_regions, dac_region_free_list, FREE_REGION_KINDS) #else DEFINE_MISSING_FIELD(free_regions) #endif // ALL_FIELDS From eaae614cf85335fdf13eed16550321b7a5fbbb92 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 7 Apr 2023 17:36:39 -0700 Subject: [PATCH 16/27] Fix gcc warnings --- src/coreclr/debug/daccess/request_svr.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index c28c25ee9500eb..33ab789aa9580d 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -472,12 +472,9 @@ void ClrDataAccess::GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS reg if (heapAddress == 0) continue; - - dac_gc_heap heap = LoadGcHeapData(heapAddress); - if (heap.free_regions != nullptr) - for (int i = 0; i < count_free_region_kinds; i++) - AddFreeRegion(heap.free_regions[i], count, regions, index); + for (int i = 0; i < count_free_region_kinds; i++) + AddFreeRegion(heap.free_regions[i], count, regions, index); } } From 2d57280aff2538c307c3cbf69ae5cf6c6ae046e8 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 7 Apr 2023 17:40:58 -0700 Subject: [PATCH 17/27] Remove unneeded #define --- src/coreclr/gc/gcinterface.dac.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index ca9b944a906c26..da79f126d65955 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -183,7 +183,6 @@ class dac_gc_heap { #define DEFINE_FIELD(field_name, field_type) field_type field_name; #define DEFINE_DPTR_FIELD(field_name, field_type) DPTR(field_type) field_name; #define DEFINE_ARRAY_FIELD(field_name, field_type, array_length) field_type field_name[array_length]; -#define DEFINE_MISSING_FIELD(field_name) #include "dac_gcheap_fields.h" @@ -191,7 +190,6 @@ class dac_gc_heap { #undef DEFINE_DPTR_FIELD #undef DEFINE_FIELD #undef ALL_FIELDS -#undef DEFINE_MISSING_FIELD // The generation table must always be last, because the size of this array // (stored inline in the gc_heap class) can vary. From de347e8c7c213fbc72166b511dc64bb12d3a3d01 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 13 Apr 2023 12:52:28 -0700 Subject: [PATCH 18/27] Add freeable_soh_segment/freeable_uoh_segment --- src/coreclr/debug/daccess/daccess.cpp | 91 +++++++++++++++++++++++ src/coreclr/debug/daccess/dacimpl.h | 17 ++++- src/coreclr/debug/daccess/request.cpp | 66 +++------------- src/coreclr/debug/daccess/request_svr.cpp | 10 ++- src/coreclr/gc/dac_gcheap_fields.h | 4 + src/coreclr/gc/gc.cpp | 4 + src/coreclr/gc/gcinterface.dac.h | 2 +- src/coreclr/gc/gcinterface.dacvars.def | 2 + src/coreclr/inc/sospriv.idl | 3 +- src/coreclr/pal/prebuilt/inc/sospriv.h | 9 +-- 10 files changed, 139 insertions(+), 69 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 89d284fe7fb854..b8f64be3adf351 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8416,3 +8416,94 @@ HRESULT DacHandleTableMemoryEnumerator::Init() return S_OK; } + +void DacFreeRegionEnumerator::AddSingleSegment(const dac_heap_segment &curr, FreeRegionKind kind) +{ + SOSMemoryRegion mem = {0}; + mem.Start = TO_CDADDR(curr.mem); + mem.ExtraData = (CLRDATA_ADDRESS)kind; + + if (curr.mem < curr.committed) + mem.Size = TO_CDADDR(curr.committed) - mem.Start; + + if (mem.Start) + mRegions.Add(mem); +} + +void DacFreeRegionEnumerator::AddSegmentList(DPTR(dac_heap_segment) start, FreeRegionKind kind) +{ + int iterationMax = 2048; + + DPTR(dac_heap_segment) curr = start; + while (curr != nullptr) + { + AddSingleSegment(*curr, kind); + + curr = curr->next; + if (curr == start) + break; + + if (iterationMax-- <= 0) + break; + } +} + +void DacFreeRegionEnumerator::AddFreeList(DPTR(dac_region_free_list) free_list, FreeRegionKind kind) +{ + if (free_list != nullptr) + { + AddSegmentList(free_list->head_free_region, kind); + } +} + +HRESULT DacFreeRegionEnumerator::Init() +{ + // Cap the number of free regions we will walk at a sensible number. This is to protect against + // memory corruption, un-initialized data, or just a bug. + int count_free_region_kinds = g_gcDacGlobals->count_free_region_kinds; + count_free_region_kinds = min(count_free_region_kinds, 16); + + unsigned int index = 0; + if (g_gcDacGlobals->global_free_huge_regions != nullptr) + { + DPTR(dac_region_free_list) global_free_huge_regions(g_gcDacGlobals->global_free_huge_regions); + AddFreeList(global_free_huge_regions, FreeRegionKind::FreeGlobalHugeRegion); + } + + if (g_gcDacGlobals->global_regions_to_decommit != nullptr) + { + DPTR(dac_region_free_list) regionList(g_gcDacGlobals->global_regions_to_decommit); + if (regionList != nullptr) + for (int i = 0; i < count_free_region_kinds; i++, regionList++) + AddFreeList(regionList, FreeRegionKind::FreeGlobalRegion); + } + +#if defined(FEATURE_SVR_GC) + if (GCHeapUtilities::IsServerHeap()) + { + AddServerRegions(); + } + else +#endif //FEATURE_SVR_GC + { + DPTR(dac_region_free_list) regionList(g_gcDacGlobals->free_regions); + if (regionList != nullptr) + for (int i = 0; i < count_free_region_kinds; i++, regionList++) + AddFreeList(regionList, FreeRegionKind::FreeRegion); + + + if (g_gcDacGlobals->freeable_soh_segment != nullptr) + { + DPTR(dac_heap_segment) freeable_soh_segment(*g_gcDacGlobals->freeable_soh_segment); + AddSegmentList(freeable_soh_segment, FreeRegionKind::FreeSohSegment); + } + + if (g_gcDacGlobals->freeable_uoh_segment != nullptr) + { + DPTR(dac_heap_segment) freeable_uoh_segment(*g_gcDacGlobals->freeable_uoh_segment); + AddSegmentList(freeable_uoh_segment, FreeRegionKind::FreeUohSegment); + } + } + + return S_OK; +} diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index 0358671bb009ba..c4a934d7937a17 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -1213,7 +1213,7 @@ class ClrDataAccess virtual HRESULT STDMETHODCALLTYPE GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocator, int count, CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded); virtual HRESULT STDMETHODCALLTYPE GetHandleTableMemoryRegions(ISOSMemoryEnum **ppEnum); virtual HRESULT STDMETHODCALLTYPE GetGCBookkeepingMemoryRegions(ISOSMemoryEnum **ppEnum); - virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS region[], unsigned int *pNeeded); + virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions(ISOSMemoryEnum **ppEnum); virtual HRESULT STDMETHODCALLTYPE LockedFlush(); // @@ -1338,9 +1338,6 @@ class ClrDataAccess HRESULT EnumMemDumpAllThreadsStack(CLRDataEnumMemoryFlags flags); HRESULT EnumMemCLRMainModuleInfo(); - void AddFreeRegion(const dac_region_free_list &free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); - void GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index); - bool ReportMem(TADDR addr, TSIZE_T size, bool fExpectSuccess = true); bool DacUpdateMemoryRegion(TADDR addr, TSIZE_T bufferSize, BYTE* buffer); @@ -1999,6 +1996,18 @@ class DacGCBookkeepingEnumerator : public DacMemoryEnumerator virtual HRESULT Init(); }; +class DacFreeRegionEnumerator : public DacMemoryEnumerator +{ +public: + virtual HRESULT Init(); + +private: + void AddSingleSegment(const dac_heap_segment &seg, FreeRegionKind kind); + void AddSegmentList(DPTR(dac_heap_segment) seg, FreeRegionKind kind); + void AddFreeList(DPTR(dac_region_free_list) freeList, FreeRegionKind kind); + void AddServerRegions(); +}; + struct DacGcReference; /* DacStackReferenceWalker. */ diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 422ef264d18040..9ccb7d6272442a 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -5350,74 +5350,30 @@ HRESULT ClrDataAccess::GetGCBookkeepingMemoryRegions(ISOSMemoryEnum** ppEnum) return hr; } -void ClrDataAccess::AddFreeRegion(const dac_region_free_list &free_list, unsigned int count, CLRDATA_ADDRESS regions[], unsigned int& index) -{ - // Cap the number of regions we will walk in case we hit memory corruption that - // puts us into an infinite loop. - int maxRegions = 4096; - - DPTR(dac_heap_segment) seg = free_list.head_free_region; - while (seg != nullptr) - { - if (regions && index < count) - regions[index++] = seg.GetAddr(); - else - index++; - seg = seg->next; - if (seg == free_list.head_free_region) - break; - - if (--maxRegions <= 0) - break; - } -} - -HRESULT ClrDataAccess::GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int *pNeeded) +HRESULT ClrDataAccess::GetGCFreeRegions(ISOSMemoryEnum **ppEnum) { - if (!pNeeded) + if (!ppEnum) return E_POINTER; SOSDacEnter(); - // Cap the number of free regions we will walk at a sensible number. This is to protect against - // memory corruption, un-initialized data, or just a bug. - int count_free_region_kinds = g_gcDacGlobals->count_free_region_kinds; - count_free_region_kinds = min(count_free_region_kinds, 16); - - unsigned int index = 0; - if (g_gcDacGlobals->global_free_huge_regions != nullptr) + DacFreeRegionEnumerator* frEnum = new (nothrow) DacFreeRegionEnumerator(); + if (frEnum) { - DPTR(dac_region_free_list) global_free_huge_regions(g_gcDacGlobals->global_free_huge_regions); - AddFreeRegion(*global_free_huge_regions, count, regions, index); - } + hr = frEnum->Init(); - if (g_gcDacGlobals->global_regions_to_decommit != nullptr) - { - DPTR(dac_region_free_list) regionList(g_gcDacGlobals->global_regions_to_decommit); - for (int i = 0; i < count_free_region_kinds; i++, regionList++) - AddFreeRegion(*regionList, count, regions, index); - } - -#if defined(FEATURE_SVR_GC) - if (GCHeapUtilities::IsServerHeap()) - { - GetServerFreeRegions(count, regions, index); + if (SUCCEEDED(hr)) + hr = frEnum->QueryInterface(__uuidof(ISOSMemoryEnum), (void**)ppEnum); + + if (FAILED(hr)) + delete frEnum; } else -#endif //FEATURE_SVR_GC { - DPTR(dac_region_free_list) regionList(g_gcDacGlobals->free_regions); - if (regionList != nullptr) - for (int i = 0; i < count_free_region_kinds; i++, regionList++) - AddFreeRegion(*regionList, count, regions, index); + hr = E_OUTOFMEMORY; } - if (regions) - *pNeeded = min(index, count); - else - *pNeeded = index; - SOSDacLeave(); return hr; } diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index 33ab789aa9580d..100f79912a4b47 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -459,7 +459,7 @@ HRESULT DacHeapWalker::InitHeapDataSvr(HeapData *&pHeaps, size_t &pCount) return S_OK; } -void ClrDataAccess::GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS regions[], unsigned int &index) +void DacFreeRegionEnumerator::AddServerRegions() { // Cap the number of free regions we will walk at a sensible number. This is to protect against // memory corruption, un-initialized data, or just a bug. @@ -474,7 +474,13 @@ void ClrDataAccess::GetServerFreeRegions(unsigned int count, CLRDATA_ADDRESS reg dac_gc_heap heap = LoadGcHeapData(heapAddress); for (int i = 0; i < count_free_region_kinds; i++) - AddFreeRegion(heap.free_regions[i], count, regions, index); + AddSegmentList(heap.free_regions[i].head_free_region, FreeRegionKind::FreeRegion); + + AddSingleSegment(heap.freeable_soh_segment, FreeRegionKind::FreeSohSegment); + AddSegmentList(heap.freeable_soh_segment.next, FreeRegionKind::FreeSohSegment); + + AddSingleSegment(heap.freeable_uoh_segment, FreeRegionKind::FreeUohSegment); + AddSegmentList(heap.freeable_uoh_segment.next, FreeRegionKind::FreeUohSegment); } } diff --git a/src/coreclr/gc/dac_gcheap_fields.h b/src/coreclr/gc/dac_gcheap_fields.h index 399bee5b44a272..7eb03d4af28373 100644 --- a/src/coreclr/gc/dac_gcheap_fields.h +++ b/src/coreclr/gc/dac_gcheap_fields.h @@ -16,6 +16,8 @@ DEFINE_FIELD (mark_array, uint32_t*) DEFINE_FIELD (next_sweep_obj, uint8_t*) DEFINE_FIELD (background_saved_lowest_address, uint8_t*) DEFINE_FIELD (background_saved_highest_address, uint8_t*) +DEFINE_FIELD (freeable_soh_segment, dac_heap_segment) +DEFINE_FIELD (freeable_uoh_segment, dac_heap_segment) #if defined(ALL_FIELDS) || !defined(USE_REGIONS) DEFINE_DPTR_FIELD (saved_sweep_ephemeral_seg, dac_heap_segment) DEFINE_FIELD (saved_sweep_ephemeral_start, uint8_t*) @@ -28,6 +30,8 @@ DEFINE_MISSING_FIELD(mark_array) DEFINE_MISSING_FIELD(next_sweep_obj) DEFINE_MISSING_FIELD(background_saved_lowest_address) DEFINE_MISSING_FIELD(background_saved_highest_address) +DEFINE_MISSING_FIELD(freeable_soh_segment) +DEFINE_MISSING_FIELD(freeable_uoh_segment) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_seg) DEFINE_MISSING_FIELD(saved_sweep_ephemeral_start) #endif // defined(ALL_FIELDS) || defined(BACKGROUND_GC) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 19e1357303fc56..aebdba00e98e4d 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -49335,6 +49335,8 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->mark_array = &gc_heap::mark_array; gcDacVars->background_saved_lowest_address = &gc_heap::background_saved_lowest_address; gcDacVars->background_saved_highest_address = &gc_heap::background_saved_highest_address; + gcDacVars->freeable_soh_segment = reinterpret_cast(&gc_heap::freeable_soh_segment); + gcDacVars->freeable_uoh_segment = reinterpret_cast(&gc_heap::freeable_uoh_segment); gcDacVars->next_sweep_obj = &gc_heap::next_sweep_obj; #ifdef USE_REGIONS gcDacVars->saved_sweep_ephemeral_seg = 0; @@ -49347,6 +49349,8 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->mark_array = 0; gcDacVars->background_saved_lowest_address = 0; gcDacVars->background_saved_highest_address = 0; + gcDacVars->freeable_soh_segment = 0; + gcDacVars->freeable_uoh_segment = 0; gcDacVars->next_sweep_obj = 0; gcDacVars->saved_sweep_ephemeral_seg = 0; gcDacVars->saved_sweep_ephemeral_start = 0; diff --git a/src/coreclr/gc/gcinterface.dac.h b/src/coreclr/gc/gcinterface.dac.h index da79f126d65955..3eb66a61a003ad 100644 --- a/src/coreclr/gc/gcinterface.dac.h +++ b/src/coreclr/gc/gcinterface.dac.h @@ -205,7 +205,7 @@ class dac_gc_heap { dac_generation generation_table[1]; }; -#define GENERATION_TABLE_FIELD_INDEX 19 +#define GENERATION_TABLE_FIELD_INDEX 21 // Unlike other DACized structures, these types are loaded manually in the debugger. // To avoid misuse, pointers to them are explicitly casted to these unused type. diff --git a/src/coreclr/gc/gcinterface.dacvars.def b/src/coreclr/gc/gcinterface.dacvars.def index dc57c51a3cd243..17c32ebd56b085 100644 --- a/src/coreclr/gc/gcinterface.dacvars.def +++ b/src/coreclr/gc/gcinterface.dacvars.def @@ -42,6 +42,8 @@ GC_DAC_PTR_VAR (uint32_t, mark_array) GC_DAC_VAR (c_gc_state, current_c_gc_state) GC_DAC_PTR_VAR (dac_heap_segment, ephemeral_heap_segment) GC_DAC_PTR_VAR (dac_heap_segment, saved_sweep_ephemeral_seg) +GC_DAC_PTR_VAR (dac_heap_segment, freeable_soh_segment) +GC_DAC_PTR_VAR (dac_heap_segment, freeable_uoh_segment) GC_DAC_PTR_VAR (uint8_t, saved_sweep_ephemeral_start) GC_DAC_PTR_VAR (uint8_t, background_saved_lowest_address) GC_DAC_PTR_VAR (uint8_t, background_saved_highest_address) diff --git a/src/coreclr/inc/sospriv.idl b/src/coreclr/inc/sospriv.idl index 6cc243ac651cac..69a267e2af4c8d 100644 --- a/src/coreclr/inc/sospriv.idl +++ b/src/coreclr/inc/sospriv.idl @@ -49,6 +49,7 @@ cpp_quote("#endif") cpp_quote("typedef enum { TYPEDEFTOMETHODTABLE, TYPEREFTOMETHODTABLE } ModuleMapType;") cpp_quote("typedef enum {IndcellHeap, LookupHeap, ResolveHeap, DispatchHeap, CacheEntryHeap, VtableHeap} VCSHeapType;") cpp_quote("typedef enum {LoaderHeapKindNormal = 0, LoaderHeapKindExplicitControl = 1} LoaderHeapKind;") +cpp_quote("typedef enum {FreeUnknownRegion = 0, FreeGlobalHugeRegion = 1, FreeGlobalRegion = 2, FreeRegion = 3, FreeSohSegment = 4, FreeUohSegment = 5 } FreeRegionKind;") typedef void (*MODULEMAPTRAVERSE)(UINT index, CLRDATA_ADDRESS methodTable,LPVOID token); typedef void (*VISITHEAP)(CLRDATA_ADDRESS blockData,size_t blockSize,BOOL blockIsCurrentBlock); @@ -500,6 +501,6 @@ interface ISOSDacInterface13 : IUnknown HRESULT GetLoaderAllocatorHeaps(CLRDATA_ADDRESS loaderAllocator, int count, CLRDATA_ADDRESS *pLoaderHeaps, LoaderHeapKind *pKinds, int *pNeeded); HRESULT GetHandleTableMemoryRegions(ISOSMemoryEnum **ppEnum); HRESULT GetGCBookkeepingMemoryRegions(ISOSMemoryEnum **ppEnum); - HRESULT GetGCFreeRegions(unsigned int count, CLRDATA_ADDRESS region[], unsigned int *pNeeded); + HRESULT GetGCFreeRegions(ISOSMemoryEnum **ppEnum); HRESULT LockedFlush(); } diff --git a/src/coreclr/pal/prebuilt/inc/sospriv.h b/src/coreclr/pal/prebuilt/inc/sospriv.h index 777e6cc5d399f3..4893710a98e498 100644 --- a/src/coreclr/pal/prebuilt/inc/sospriv.h +++ b/src/coreclr/pal/prebuilt/inc/sospriv.h @@ -205,6 +205,7 @@ typedef int VCSHeapType; typedef enum { TYPEDEFTOMETHODTABLE, TYPEREFTOMETHODTABLE } ModuleMapType; typedef enum {IndcellHeap, LookupHeap, ResolveHeap, DispatchHeap, CacheEntryHeap, VtableHeap} VCSHeapType; typedef enum {LoaderHeapKindNormal = 0, LoaderHeapKindExplicitControl = 1} LoaderHeapKind; +typedef enum {FreeUnknownRegion = 0, FreeGlobalHugeRegion = 1, FreeGlobalRegion = 2, FreeRegion = 3, FreeSohSegment = 4, FreeUohSegment = 5 } FreeRegionKind; typedef void ( *MODULEMAPTRAVERSE )( UINT index, CLRDATA_ADDRESS methodTable, @@ -3240,9 +3241,7 @@ EXTERN_C const IID IID_ISOSDacInterface13; ISOSMemoryEnum **ppEnum) = 0; virtual HRESULT STDMETHODCALLTYPE GetGCFreeRegions( - unsigned int count, - CLRDATA_ADDRESS region[ ], - unsigned int *pNeeded) = 0; + ISOSMemoryEnum **ppEnum) = 0; virtual HRESULT STDMETHODCALLTYPE LockedFlush( void) = 0; }; @@ -3302,9 +3301,7 @@ EXTERN_C const IID IID_ISOSDacInterface13; HRESULT ( STDMETHODCALLTYPE *GetGCFreeRegions )( ISOSDacInterface13 * This, - unsigned int count, - CLRDATA_ADDRESS region[ ], - unsigned int *pNeeded); + ISOSMemoryEnum **ppEnum); HRESULT ( STDMETHODCALLTYPE *LockedFlush )( ISOSDacInterface13 * This); From cec79406244c2bb527bfbb4def68f4cf94188b1f Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 13 Apr 2023 15:49:29 -0700 Subject: [PATCH 19/27] Add more card table checks --- src/coreclr/debug/daccess/daccess.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index b8f64be3adf351..bb126bc1a0c44d 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8338,16 +8338,19 @@ HRESULT DacGCBookkeepingEnumerator::Init() DPTR(dac_card_table_info) card_table_info(ctiAddr); SOSMemoryRegion mem = {0}; - mem.Start = card_table_info.GetAddr(); - mem.Size = card_table_info->size; - mRegions.Add(mem); + if (card_table_info->recount && card_table_info->size) + { + mem.Start = card_table_info.GetAddr(); + mem.Size = card_table_info->size; + mRegions.Add(mem); + } size_t card_table_info_size = g_gcDacGlobals->card_table_info_size; TADDR next = card_table_info->next_card_table; // Cap the number of regions we will walk in case we have run into some kind of - // memory corruption. We shouldn't have more than 2 linked card tables anyway. - int maxRegions = 8; + // memory corruption. We shouldn't have more than a few linked card tables anyway. + int maxRegions = 32; // This loop is effectively "while (next != 0)" but with an added check to make // sure we don't underflow next when subtracting card_table_info_size if we encounter @@ -8356,10 +8359,13 @@ HRESULT DacGCBookkeepingEnumerator::Init() { DPTR(dac_card_table_info) ct(next - card_table_info_size); - mem = {0}; - mem.Start = ct.GetAddr(); - mem.Size = ct->size; - mRegions.Add(mem); + if (ct->recount && ct->size) + { + mem = {0}; + mem.Start = ct.GetAddr(); + mem.Size = ct->size; + mRegions.Add(mem); + } next = ct->next_card_table; if (next == card_table_info->next_card_table) From 4a9c4a21df06cb83ff9607bced6a300305f4cba5 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Thu, 13 Apr 2023 15:49:52 -0700 Subject: [PATCH 20/27] Fix field definition issue --- src/coreclr/debug/daccess/request_svr.cpp | 7 ++----- src/coreclr/gc/dac_gcheap_fields.h | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index 100f79912a4b47..eccafc0d229c89 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -476,11 +476,8 @@ void DacFreeRegionEnumerator::AddServerRegions() for (int i = 0; i < count_free_region_kinds; i++) AddSegmentList(heap.free_regions[i].head_free_region, FreeRegionKind::FreeRegion); - AddSingleSegment(heap.freeable_soh_segment, FreeRegionKind::FreeSohSegment); - AddSegmentList(heap.freeable_soh_segment.next, FreeRegionKind::FreeSohSegment); - - AddSingleSegment(heap.freeable_uoh_segment, FreeRegionKind::FreeUohSegment); - AddSegmentList(heap.freeable_uoh_segment.next, FreeRegionKind::FreeUohSegment); + AddSegmentList(heap.freeable_soh_segment, FreeRegionKind::FreeSohSegment); + AddSegmentList(heap.freeable_uoh_segment, FreeRegionKind::FreeUohSegment); } } diff --git a/src/coreclr/gc/dac_gcheap_fields.h b/src/coreclr/gc/dac_gcheap_fields.h index 7eb03d4af28373..37b6389ff1ea18 100644 --- a/src/coreclr/gc/dac_gcheap_fields.h +++ b/src/coreclr/gc/dac_gcheap_fields.h @@ -16,8 +16,8 @@ DEFINE_FIELD (mark_array, uint32_t*) DEFINE_FIELD (next_sweep_obj, uint8_t*) DEFINE_FIELD (background_saved_lowest_address, uint8_t*) DEFINE_FIELD (background_saved_highest_address, uint8_t*) -DEFINE_FIELD (freeable_soh_segment, dac_heap_segment) -DEFINE_FIELD (freeable_uoh_segment, dac_heap_segment) +DEFINE_DPTR_FIELD (freeable_soh_segment, dac_heap_segment) +DEFINE_DPTR_FIELD (freeable_uoh_segment, dac_heap_segment) #if defined(ALL_FIELDS) || !defined(USE_REGIONS) DEFINE_DPTR_FIELD (saved_sweep_ephemeral_seg, dac_heap_segment) DEFINE_FIELD (saved_sweep_ephemeral_start, uint8_t*) From 86c1e5a7bf26716e634bdb9c355c45b20010a1cd Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 14 Apr 2023 10:40:12 -0700 Subject: [PATCH 21/27] Fix pointer issue --- src/coreclr/debug/daccess/daccess.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index bb126bc1a0c44d..5b12b454e813c5 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8500,14 +8500,16 @@ HRESULT DacFreeRegionEnumerator::Init() if (g_gcDacGlobals->freeable_soh_segment != nullptr) { - DPTR(dac_heap_segment) freeable_soh_segment(*g_gcDacGlobals->freeable_soh_segment); - AddSegmentList(freeable_soh_segment, FreeRegionKind::FreeSohSegment); + DPTR(DPTR(dac_heap_segment)) freeable_soh_segment_ptr(g_gcDacGlobals->freeable_soh_segment); + if (freeable_soh_segment_ptr != nullptr) + AddSegmentList(*freeable_soh_segment, FreeRegionKind::FreeSohSegment); } if (g_gcDacGlobals->freeable_uoh_segment != nullptr) { - DPTR(dac_heap_segment) freeable_uoh_segment(*g_gcDacGlobals->freeable_uoh_segment); - AddSegmentList(freeable_uoh_segment, FreeRegionKind::FreeUohSegment); + DPTR(DPTR(dac_heap_segment)) freeable_uoh_segment_ptr(g_gcDacGlobals->freeable_uoh_segment); + if (freeable_uoh_segment_ptr != nullptr) + AddSegmentList(*freeable_uoh_segment_ptr, FreeRegionKind::FreeUohSegment); } } From 33566b08ce2550927fa03264570534fa8742e22d Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Fri, 14 Apr 2023 12:02:40 -0700 Subject: [PATCH 22/27] Fix compile issue --- src/coreclr/debug/daccess/daccess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 5b12b454e813c5..15e9f50d30998e 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8502,7 +8502,7 @@ HRESULT DacFreeRegionEnumerator::Init() { DPTR(DPTR(dac_heap_segment)) freeable_soh_segment_ptr(g_gcDacGlobals->freeable_soh_segment); if (freeable_soh_segment_ptr != nullptr) - AddSegmentList(*freeable_soh_segment, FreeRegionKind::FreeSohSegment); + AddSegmentList(*freeable_soh_segment_ptr, FreeRegionKind::FreeSohSegment); } if (g_gcDacGlobals->freeable_uoh_segment != nullptr) From 47abfdc67b2b9ab98f57a626f610637052937e08 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Fri, 14 Apr 2023 21:15:08 -0700 Subject: [PATCH 23/27] Rename bookkeeping_covered_start to bookkeeping_start --- src/coreclr/debug/daccess/daccess.cpp | 4 ++-- src/coreclr/gc/gc.cpp | 18 ++++++++---------- src/coreclr/gc/gcinterface.dacvars.def | 2 +- src/coreclr/gc/gcpriv.h | 2 +- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 15e9f50d30998e..2290784a652379 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8328,10 +8328,10 @@ HRESULT DacMemoryEnumerator::Next(unsigned int count, SOSMemoryRegion regions[], HRESULT DacGCBookkeepingEnumerator::Init() { - if (g_gcDacGlobals->bookkeeping_covered_start == nullptr) + if (g_gcDacGlobals->bookkeeping_start == nullptr) return E_FAIL; - TADDR ctiAddr = TO_TADDR(*g_gcDacGlobals->bookkeeping_covered_start); + TADDR ctiAddr = TO_TADDR(*g_gcDacGlobals->bookkeeping_start); if (ctiAddr == 0) return E_FAIL; diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index aebdba00e98e4d..358f803887e35f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2228,7 +2228,7 @@ size_t gc_heap::g_bpromoted; #endif //MULTIPLE_HEAPS size_t gc_heap::card_table_element_layout[total_bookkeeping_elements + 1]; -uint8_t* gc_heap::bookkeeping_covered_start = nullptr; +uint8_t* gc_heap::bookkeeping_start = nullptr; #ifdef USE_REGIONS uint8_t* gc_heap::bookkeeping_covered_committed = nullptr; size_t gc_heap::bookkeeping_sizes[total_bookkeeping_elements]; @@ -8913,21 +8913,21 @@ bool gc_heap::inplace_commit_card_table (uint8_t* from, uint8_t* to) uint8_t* commit_end = nullptr; if (initial_commit) { - required_begin = bookkeeping_covered_start + ((i == card_table_element) ? 0 : card_table_element_layout[i]); - required_end = bookkeeping_covered_start + card_table_element_layout[i] + new_sizes[i]; + required_begin = bookkeeping_start + ((i == card_table_element) ? 0 : card_table_element_layout[i]); + required_end = bookkeeping_start + card_table_element_layout[i] + new_sizes[i]; commit_begin = align_lower_page(required_begin); } else { assert (additional_commit); - required_begin = bookkeeping_covered_start + card_table_element_layout[i] + bookkeeping_sizes[i]; + required_begin = bookkeeping_start + card_table_element_layout[i] + bookkeeping_sizes[i]; required_end = required_begin + new_sizes[i] - bookkeeping_sizes[i]; commit_begin = align_on_page(required_begin); } assert (required_begin <= required_end); commit_end = align_on_page(required_end); - commit_end = min (commit_end, align_lower_page(bookkeeping_covered_start + card_table_element_layout[i + 1])); + commit_end = min (commit_end, align_lower_page(bookkeeping_start + card_table_element_layout[i + 1])); commit_begin = min (commit_begin, commit_end); assert (commit_begin <= commit_end); @@ -9002,7 +9002,7 @@ uint32_t* gc_heap::make_card_table (uint8_t* start, uint8_t* end) size_t alloc_size = card_table_element_layout[total_bookkeeping_elements]; uint8_t* mem = (uint8_t*)GCToOSInterface::VirtualReserve (alloc_size, 0, virtual_reserve_flags); - bookkeeping_covered_start = mem; + bookkeeping_start = mem; if (!mem) return 0; @@ -9499,7 +9499,7 @@ void gc_heap::copy_brick_card_table() uint32_t* ct = &g_gc_card_table[card_word (gcard_of (g_gc_lowest_address))]; own_card_table (ct); card_table = translate_card_table (ct); - bookkeeping_covered_start = (uint8_t*)ct - sizeof(card_table_info); + bookkeeping_start = (uint8_t*)ct - sizeof(card_table_info); card_table_size(ct) = card_table_element_layout[total_bookkeeping_elements]; /* End of global lock */ highest_address = card_table_highest_address (ct); @@ -13734,8 +13734,6 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size, &g_gc_lowest_address, &g_gc_highest_address)) return E_OUTOFMEMORY; - bookkeeping_covered_start = global_region_allocator.get_start(); - if (!allocate_initial_regions(number_of_heaps)) return E_OUTOFMEMORY; } @@ -49377,5 +49375,5 @@ void PopulateDacVars(GcDacVars *gcDacVars) gcDacVars->gc_heap_field_offsets = reinterpret_cast(&gc_heap_field_offsets); #endif // MULTIPLE_HEAPS gcDacVars->generation_field_offsets = reinterpret_cast(&generation_field_offsets); - gcDacVars->bookkeeping_covered_start = &gc_heap::bookkeeping_covered_start; + gcDacVars->bookkeeping_start = &gc_heap::bookkeeping_start; } diff --git a/src/coreclr/gc/gcinterface.dacvars.def b/src/coreclr/gc/gcinterface.dacvars.def index 17c32ebd56b085..f9e0eb4be7e3b8 100644 --- a/src/coreclr/gc/gcinterface.dacvars.def +++ b/src/coreclr/gc/gcinterface.dacvars.def @@ -64,7 +64,7 @@ GC_DAC_ARRAY_VAR (size_t, interesting_mechanism_bits_per_heap) GC_DAC_VAR (dac_handle_table_map, handle_table_map) GC_DAC_ARRAY_VAR (int, gc_heap_field_offsets) GC_DAC_ARRAY_VAR (int, generation_field_offsets) -GC_DAC_PTR_VAR (uint8_t, bookkeeping_covered_start) +GC_DAC_PTR_VAR (uint8_t, bookkeeping_start) GC_DAC_ARRAY_VAR (dac_region_free_list, global_regions_to_decommit) GC_DAC_PTR_VAR (dac_region_free_list, global_free_huge_regions) GC_DAC_ARRAY_VAR (dac_region_free_list, free_regions) diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index f50a896b7dfe8e..d72c9495dd8eab 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -4196,7 +4196,7 @@ class gc_heap #endif //BGC_SERVO_TUNING #endif //BACKGROUND_GC - PER_HEAP_ISOLATED_FIELD_INIT_ONLY uint8_t* bookkeeping_covered_start; + PER_HEAP_ISOLATED_FIELD_INIT_ONLY uint8_t* bookkeeping_start; #ifdef USE_REGIONS PER_HEAP_ISOLATED_FIELD_INIT_ONLY size_t regions_range; PER_HEAP_ISOLATED_FIELD_INIT_ONLY bool enable_special_regions_p; From 3aff8552d96ac18f64658259e40f73467b92ded7 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Mon, 17 Apr 2023 10:12:01 -0700 Subject: [PATCH 24/27] Update src/coreclr/debug/daccess/daccess.cpp Co-authored-by: Andrew Au --- src/coreclr/debug/daccess/daccess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 2290784a652379..609a364741fd24 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8350,7 +8350,7 @@ HRESULT DacGCBookkeepingEnumerator::Init() // Cap the number of regions we will walk in case we have run into some kind of // memory corruption. We shouldn't have more than a few linked card tables anyway. - int maxRegions = 32; + const int maxRegions = 32; // This loop is effectively "while (next != 0)" but with an added check to make // sure we don't underflow next when subtracting card_table_info_size if we encounter From 7974efb917c882bcdae250bf87cbad0fc5bdca36 Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Mon, 17 Apr 2023 10:38:32 -0700 Subject: [PATCH 25/27] Code review feedback --- src/coreclr/debug/daccess/daccess.cpp | 12 ++++++------ src/coreclr/debug/daccess/dacimpl.h | 6 +++--- src/coreclr/debug/daccess/request_svr.cpp | 6 +++--- src/coreclr/inc/sospriv.idl | 3 ++- src/coreclr/pal/prebuilt/inc/sospriv.h | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 609a364741fd24..59a9815008e8cd 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8350,7 +8350,7 @@ HRESULT DacGCBookkeepingEnumerator::Init() // Cap the number of regions we will walk in case we have run into some kind of // memory corruption. We shouldn't have more than a few linked card tables anyway. - const int maxRegions = 32; + int maxRegions = 32; // This loop is effectively "while (next != 0)" but with an added check to make // sure we don't underflow next when subtracting card_table_info_size if we encounter @@ -8423,11 +8423,12 @@ HRESULT DacHandleTableMemoryEnumerator::Init() return S_OK; } -void DacFreeRegionEnumerator::AddSingleSegment(const dac_heap_segment &curr, FreeRegionKind kind) +void DacFreeRegionEnumerator::AddSingleSegment(const dac_heap_segment &curr, FreeRegionKind kind, int heap) { SOSMemoryRegion mem = {0}; mem.Start = TO_CDADDR(curr.mem); mem.ExtraData = (CLRDATA_ADDRESS)kind; + mem.Heap = heap; if (curr.mem < curr.committed) mem.Size = TO_CDADDR(curr.committed) - mem.Start; @@ -8436,14 +8437,14 @@ void DacFreeRegionEnumerator::AddSingleSegment(const dac_heap_segment &curr, Fre mRegions.Add(mem); } -void DacFreeRegionEnumerator::AddSegmentList(DPTR(dac_heap_segment) start, FreeRegionKind kind) +void DacFreeRegionEnumerator::AddSegmentList(DPTR(dac_heap_segment) start, FreeRegionKind kind, int heap) { int iterationMax = 2048; DPTR(dac_heap_segment) curr = start; while (curr != nullptr) { - AddSingleSegment(*curr, kind); + AddSingleSegment(*curr, kind, heap); curr = curr->next; if (curr == start) @@ -8454,7 +8455,7 @@ void DacFreeRegionEnumerator::AddSegmentList(DPTR(dac_heap_segment) start, FreeR } } -void DacFreeRegionEnumerator::AddFreeList(DPTR(dac_region_free_list) free_list, FreeRegionKind kind) +void DacFreeRegionEnumerator::AddFreeList(DPTR(dac_region_free_list) free_list, FreeRegionKind kind, int heap) { if (free_list != nullptr) { @@ -8497,7 +8498,6 @@ HRESULT DacFreeRegionEnumerator::Init() for (int i = 0; i < count_free_region_kinds; i++, regionList++) AddFreeList(regionList, FreeRegionKind::FreeRegion); - if (g_gcDacGlobals->freeable_soh_segment != nullptr) { DPTR(DPTR(dac_heap_segment)) freeable_soh_segment_ptr(g_gcDacGlobals->freeable_soh_segment); diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index c4a934d7937a17..3631f7bcf07760 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -2002,9 +2002,9 @@ class DacFreeRegionEnumerator : public DacMemoryEnumerator virtual HRESULT Init(); private: - void AddSingleSegment(const dac_heap_segment &seg, FreeRegionKind kind); - void AddSegmentList(DPTR(dac_heap_segment) seg, FreeRegionKind kind); - void AddFreeList(DPTR(dac_region_free_list) freeList, FreeRegionKind kind); + void AddSingleSegment(const dac_heap_segment &seg, FreeRegionKind kind, int heap); + void AddSegmentList(DPTR(dac_heap_segment) seg, FreeRegionKind kind, int heap = 0); + void AddFreeList(DPTR(dac_region_free_list) freeList, FreeRegionKind kind, int heap = 0); void AddServerRegions(); }; diff --git a/src/coreclr/debug/daccess/request_svr.cpp b/src/coreclr/debug/daccess/request_svr.cpp index eccafc0d229c89..3955d8f05db5c1 100644 --- a/src/coreclr/debug/daccess/request_svr.cpp +++ b/src/coreclr/debug/daccess/request_svr.cpp @@ -474,10 +474,10 @@ void DacFreeRegionEnumerator::AddServerRegions() dac_gc_heap heap = LoadGcHeapData(heapAddress); for (int i = 0; i < count_free_region_kinds; i++) - AddSegmentList(heap.free_regions[i].head_free_region, FreeRegionKind::FreeRegion); + AddSegmentList(heap.free_regions[i].head_free_region, FreeRegionKind::FreeRegion, i); - AddSegmentList(heap.freeable_soh_segment, FreeRegionKind::FreeSohSegment); - AddSegmentList(heap.freeable_uoh_segment, FreeRegionKind::FreeUohSegment); + AddSegmentList(heap.freeable_soh_segment, FreeRegionKind::FreeSohSegment, i); + AddSegmentList(heap.freeable_uoh_segment, FreeRegionKind::FreeUohSegment, i); } } diff --git a/src/coreclr/inc/sospriv.idl b/src/coreclr/inc/sospriv.idl index 69a267e2af4c8d..ac7b93a9a7bc12 100644 --- a/src/coreclr/inc/sospriv.idl +++ b/src/coreclr/inc/sospriv.idl @@ -180,7 +180,8 @@ typedef struct _SOSMemoryRegion { CLRDATA_ADDRESS Start; CLRDATA_ADDRESS Size; - CLRDATA_ADDRESS ExtraData; // API specific extra data. + CLRDATA_ADDRESS ExtraData; + int Heap; } SOSMemoryRegion; cpp_quote("#endif // _SOS_MemoryRegion_") diff --git a/src/coreclr/pal/prebuilt/inc/sospriv.h b/src/coreclr/pal/prebuilt/inc/sospriv.h index 4893710a98e498..e6f2b7b4f049ea 100644 --- a/src/coreclr/pal/prebuilt/inc/sospriv.h +++ b/src/coreclr/pal/prebuilt/inc/sospriv.h @@ -504,8 +504,8 @@ typedef struct _SOSMemoryRegion CLRDATA_ADDRESS Start; CLRDATA_ADDRESS Size; CLRDATA_ADDRESS ExtraData; + int Heap; } SOSMemoryRegion; - #endif // _SOS_MemoryRegion_ #ifndef __ISOSStackRefErrorEnum_INTERFACE_DEFINED__ From 05a6c2c86e590d628cc8ba934c60037d9ab124ee Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Mon, 17 Apr 2023 10:43:50 -0700 Subject: [PATCH 26/27] Remove unused parameter --- src/coreclr/debug/daccess/daccess.cpp | 2 +- src/coreclr/debug/daccess/dacimpl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 59a9815008e8cd..bf786988d18f74 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8455,7 +8455,7 @@ void DacFreeRegionEnumerator::AddSegmentList(DPTR(dac_heap_segment) start, FreeR } } -void DacFreeRegionEnumerator::AddFreeList(DPTR(dac_region_free_list) free_list, FreeRegionKind kind, int heap) +void DacFreeRegionEnumerator::AddFreeList(DPTR(dac_region_free_list) free_list, FreeRegionKind kind) { if (free_list != nullptr) { diff --git a/src/coreclr/debug/daccess/dacimpl.h b/src/coreclr/debug/daccess/dacimpl.h index 3631f7bcf07760..ddf61370f416e9 100644 --- a/src/coreclr/debug/daccess/dacimpl.h +++ b/src/coreclr/debug/daccess/dacimpl.h @@ -2004,7 +2004,7 @@ class DacFreeRegionEnumerator : public DacMemoryEnumerator private: void AddSingleSegment(const dac_heap_segment &seg, FreeRegionKind kind, int heap); void AddSegmentList(DPTR(dac_heap_segment) seg, FreeRegionKind kind, int heap = 0); - void AddFreeList(DPTR(dac_region_free_list) freeList, FreeRegionKind kind, int heap = 0); + void AddFreeList(DPTR(dac_region_free_list) freeList, FreeRegionKind kind); void AddServerRegions(); }; From 9abf2f3e8f036c8cd200a926f334acc7f0305feb Mon Sep 17 00:00:00 2001 From: Lee Culver Date: Mon, 17 Apr 2023 10:48:38 -0700 Subject: [PATCH 27/27] Add heap number --- src/coreclr/debug/daccess/daccess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index bf786988d18f74..88003b88935009 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -8409,7 +8409,7 @@ HRESULT DacHandleTableMemoryEnumerator::Init() SOSMemoryRegion mem = {0}; mem.Start = curr.GetAddr(); mem.Size = HANDLE_SEGMENT_SIZE; - mem.ExtraData = j; // heap number + mem.Heap = j; // heap number mRegions.Add(mem);