Implement ISOSDacInterface::GetLoaderAllocatorHeapNames and GetLoaderAllocatorHeaps in cDAC SOSDacImpl#124818
Implement ISOSDacInterface::GetLoaderAllocatorHeapNames and GetLoaderAllocatorHeaps in cDAC SOSDacImpl#124818
Conversation
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
| private static readonly string[] s_loaderAllocatorHeapNames = | ||
| [ | ||
| "LowFrequencyHeap", | ||
| "HighFrequencyHeap", | ||
| "StaticsHeap", | ||
| "StubHeap", | ||
| "ExecutableHeap", | ||
| "FixupPrecodeHeap", | ||
| "NewStubPrecodeHeap", | ||
| "IndcellHeap", | ||
| "CacheEntryHeap", | ||
| ]; | ||
|
|
||
| private static readonly nint[] s_loaderAllocatorHeapNamePtrs = InitializeHeapNamePtrs(); | ||
|
|
||
| private static nint[] InitializeHeapNamePtrs() | ||
| { | ||
| nint[] ptrs = new nint[s_loaderAllocatorHeapNames.Length]; | ||
| for (int i = 0; i < s_loaderAllocatorHeapNames.Length; i++) | ||
| { | ||
| ptrs[i] = Marshal.StringToHGlobalAnsi(s_loaderAllocatorHeapNames[i]); | ||
| } | ||
| return ptrs; | ||
| } | ||
|
|
||
| int ISOSDacInterface13.GetLoaderAllocatorHeapNames(int count, char** ppNames, int* pNeeded) | ||
| => _legacyImpl13 is not null ? _legacyImpl13.GetLoaderAllocatorHeapNames(count, ppNames, pNeeded) : HResults.E_NOTIMPL; | ||
| { |
There was a problem hiding this comment.
@copilot , this is not quite correct. If you look at the runtime implementation, there are #defines that remove some elements from the list under certain conditions. We will need to do something similar here.
While we are at it, lets expand the scope of this PR to include implementing GetLoaderAllocatorHeaps is SOSDacImpl.cs since they are very related.
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
|
@copilot, try again |
…dd missing using Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Implementation reworked in commits up to 0554c43. Changes: Feature-conditional heap names: GetLoaderAllocatorHeaps: Fully implemented in SOSDacImpl.cs, reading heap pointers through the Native data descriptors: Added Tests: 32 new test cases (8 test methods × 4 architectures) covering both methods. All 864 cDAC tests pass. |
max-charlamb
left a comment
There was a problem hiding this comment.
@copilot , address feedback
| IReadOnlyList<string> GetLoaderAllocatorHeapNames() => throw new NotImplementedException(); | ||
| IReadOnlyList<TargetPointer> GetLoaderAllocatorHeaps(TargetPointer loaderAllocatorPointer) => throw new NotImplementedException(); |
There was a problem hiding this comment.
Instead of hardcoding these, lets have a single API that returns a dictionary. If the ISOSDacInterface APIs only want the names, we can query the global loader allocator and return the values. That way we don't care about ordering.
…ps API Replace separate GetLoaderAllocatorHeapNames() and GetLoaderAllocatorHeaps(TargetPointer) with a single GetLoaderAllocatorHeaps(TargetPointer) returning IReadOnlyDictionary<string, TargetPointer>. SOSDacImpl.GetLoaderAllocatorHeapNames now queries the global loader allocator to get names from the dictionary. GetLoaderAllocatorHeaps iterates dictionary values. Tests updated to use dictionary-based mock and verify unordered name/address sets. Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
max-charlamb
left a comment
There was a problem hiding this comment.
@copilot , address feedback
...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs
Show resolved
Hide resolved
...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs
Outdated
Show resolved
Hide resolved
...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs
Outdated
Show resolved
Hide resolved
…tionary - Use nameof(Data.LoaderAllocator.X) instead of hardcoded strings for all dictionary keys in GetLoaderAllocatorHeaps - Make FixupPrecodeHeap and NewStubPrecodeHeap conditional on ContainsKey (exclude from dictionary when field absent from data descriptor) - Remove else branch that padded IndcellHeap/CacheEntryHeap with null when VirtualCallStubManager is null - only include them when VcsManager exists - All 864 cDAC tests pass Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Addressed all three review comments in e0e9cd5 —
Addressed all three review comments in e0e9cd5 — |
|
@copilot , please fix build break |
datadescriptor.h did not include virtualcallstub.h, so the cdac_data<VirtualCallStubManager> template specialization was not visible when datadescriptor.inc was compiled. This caused a build failure referencing cdac_data<VirtualCallStubManager>::IndcellHeap and cdac_data<VirtualCallStubManager>::CacheEntryHeap. Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Fixed in 3f63f2f. The issue was that Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| TargetPointer GetDynamicIL(ModuleHandle handle, uint token) => throw new NotImplementedException(); | ||
| IReadOnlyList<string> GetLoaderAllocatorHeapNames() => throw new NotImplementedException(); | ||
| IReadOnlyList<TargetPointer> GetLoaderAllocatorHeaps(TargetPointer loaderAllocatorPointer) => throw new NotImplementedException(); | ||
| IReadOnlyDictionary<string, TargetPointer> GetLoaderAllocatorHeaps(TargetPointer loaderAllocatorPointer) => throw new NotImplementedException(); |
There was a problem hiding this comment.
I don't see any docs for this in the contract doc. I assume we need to add it.
Description
Implements
ISOSDacInterface13.GetLoaderAllocatorHeapNamesandISOSDacInterface13.GetLoaderAllocatorHeapsin the managed cDACSOSDacImpl.cs, replacing the legacy DAC delegation stubs. Based onClrDataAccess::GetLoaderAllocatorHeapNamesandClrDataAccess::GetLoaderAllocatorHeapsinrequest.cpp.Native Data Descriptors
cdac_data<LoaderAllocator>inloaderallocator.hpp: StaticsHeap, ExecutableHeap, FixupPrecodeHeap (#ifdef HAS_FIXUP_PRECODE), NewStubPrecodeHeap (#ifndef FEATURE_PORTABLE_ENTRYPOINTS), DynamicHelpersStubHeap (FEATURE_READYTORUN && FEATURE_STUBPRECODE_DYNAMIC_HELPERS), VirtualCallStubManagercdac_data<VirtualCallStubManager>invirtualcallstub.h: IndcellHeap (unconditional), CacheEntryHeap (#ifdef FEATURE_VIRTUAL_STUB_DISPATCH)datadescriptor.incwith matching feature guardsVirtualCallStubManagertoDataTypeenum#include "virtualcallstub.h"todatadescriptor.hsocdac_data<VirtualCallStubManager>is visible whendatadescriptor.incis compiledManaged Data Classes
Data/LoaderAllocator.cswith optional heap fields (FixupPrecodeHeap, NewStubPrecodeHeap, DynamicHelpersStubHeap as nullable) and always-present fields (StaticsHeap, ExecutableHeap, VirtualCallStubManager)Data/VirtualCallStubManager.cswith IndcellHeap and optional CacheEntryHeapILoader Contract
GetLoaderAllocatorHeaps(TargetPointer)method toILoaderinterface returningIReadOnlyDictionary<string, TargetPointer>— dictionary keys are heap names, values are heap pointersLoader_1.cswith dynamic dictionary building based on data descriptor field presence viaContainsKeychecks, matching the native#ifdefconditional compilationnameof(Data.LoaderAllocator.X)andnameof(Data.VirtualCallStubManager.X)instead of hardcoded stringsSOSDacImpl
GetLoaderAllocatorHeapNames: Queries global loader allocator viaILoader.GetLoaderAllocatorHeapsto get heap names from dictionary keys. Lazily initializes ANSI string pointers viaMarshal.StringToHGlobalAnsi, cached on the instance. Standard cDAC pattern with try/catch,S_FALSEwhen buffer is insufficient,#if DEBUGlegacy count comparisonGetLoaderAllocatorHeaps: Reads heap pointers through the ILoader contract dictionary, fills LoaderHeapKindNormal for all heapsTests
LoaderTests.cs× 4 architectures = 32 new test casesGetLoaderAllocatorHeapNames: Count query, full name retrieval, insufficient buffer, null pNeededGetLoaderAllocatorHeaps: Count query, full heap retrieval, insufficient buffer, null address💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.