From fd215ab385a705d27f7b72103dacab57b3305072 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Tue, 12 Aug 2025 10:00:05 -0700 Subject: [PATCH 1/8] Adding TraverseModuleMap cDAC API --- docs/design/datacontracts/Loader.md | 28 ++++++++++ .../Contracts/ILoader.cs | 1 + .../Contracts/Loader_1.cs | 27 ++++++++++ .../Legacy/ISOSDacInterface.cs | 6 +++ .../Legacy/SOSDacImpl.cs | 51 ++++++++++++++++++- 5 files changed, 112 insertions(+), 1 deletion(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index be25e4cba9787d..25602dca4cdfe9 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -74,6 +74,7 @@ TargetPointer GetILBase(ModuleHandle handle); TargetPointer GetAssemblyLoadContext(ModuleHandle handle); ModuleLookupTables GetLookupTables(ModuleHandle handle); TargetPointer GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags); +IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table); bool IsCollectible(ModuleHandle handle); bool IsAssemblyLoaded(ModuleHandle handle); TargetPointer GetGlobalLoaderAllocator(); @@ -474,6 +475,33 @@ TargetPointer GetModuleLookupMapElement(TargetPointer table, uint token, out Tar return TargetPointer.Null; } +IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table) +{ + Data.ModuleLookupMap lookupMap = new Data.ModuleLookupMap(table); + // have to read lookupMap an extra time upfront because only the first map + // has valid supportedFlagsMask + TargetNUInt supportedFlagsMask = target.ReadNUInt(table + /* ModuleLookupMap::SupportedFlagsMask */); + uint index = 1; + do + { + uint count = target.Read(table + /*ModuleLookupMap::Count*/); + if (index < count) + { + TargetPointer entryAddress = target.ReadPointer(table + /*ModuleLookupMap::TableData*/) + (ulong)(index * target.PointerSize); + TargetPointer rawValue = target.ReadPointer(entryAddress); + ulong maskedValue = rawValue & ~(supportedFlagsMask.Value); + if (maskedValue != 0) + yield return (new TargetPointer(maskedValue), index); + index++; + } + else + { + table = target.ReadPointer(table + /*ModuleLookupMap::Next*/); + index -= count; + } + } while (table != TargetPointer.Null); +} + bool IsCollectible(ModuleHandle handle) { TargetPointer assembly = target.ReadPointer(handle.Address + /*Module::Assembly*/); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs index 8822dfee7ba384..f25dc59c8b6ae0 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs @@ -97,6 +97,7 @@ public interface ILoader : IContract TargetPointer GetAssemblyLoadContext(ModuleHandle handle) => throw new NotImplementedException(); ModuleLookupTables GetLookupTables(ModuleHandle handle) => throw new NotImplementedException(); TargetPointer GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags) => throw new NotImplementedException(); + IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table) => throw new NotImplementedException(); bool IsCollectible(ModuleHandle handle) => throw new NotImplementedException(); bool IsAssemblyLoaded(ModuleHandle handle) => throw new NotImplementedException(); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index 2d2562625e4688..b25fa100dd4b79 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -383,6 +383,33 @@ TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, return TargetPointer.Null; } + IEnumerable<(TargetPointer, uint)> ILoader.IterateModuleLookupMap(TargetPointer table) + { + Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); + // have to read lookupMap an extra time upfront because only the first map + // has valid supportedFlagsMask + TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; + uint index = 1; + do + { + lookupMap = _target.ProcessedData.GetOrAdd(table); + if (index < lookupMap.Count) + { + TargetPointer entryAddress = lookupMap.TableData + (ulong)(index * _target.PointerSize); + TargetPointer rawValue = _target.ReadPointer(entryAddress); + ulong maskedValue = rawValue & ~(supportedFlagsMask.Value); + if (maskedValue != 0) + yield return (new TargetPointer(maskedValue), index); + index++; + } + else + { + table = lookupMap.Next; + index -= lookupMap.Count; + } + } while (table != TargetPointer.Null); + } + bool ILoader.IsCollectible(ModuleHandle handle) { Data.Module module = _target.ProcessedData.GetOrAdd(handle.Address); diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs index 3d3e2774d22db5..74b19286157ca4 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs @@ -123,6 +123,12 @@ internal struct DacpModuleData public ulong dwModuleIndex; // Always 0 - .NET no longer has this } +internal enum ModuleMapType +{ + TYPEDEFTOMETHODTABLE = 0x0, + TYPEREFTOMETHODTABLE = 0x1 +} + internal struct DacpMethodTableData { public int bIsFree; // everything else is NULL if this is true. diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs index b1cc4cc1ac9d24..6802c201675b97 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs @@ -2143,7 +2143,56 @@ int ISOSDacInterface.TraverseEHInfo(ClrDataAddress ip, void* pCallback, void* to int ISOSDacInterface.TraverseLoaderHeap(ClrDataAddress loaderHeapAddr, void* pCallback) => _legacyImpl is not null ? _legacyImpl.TraverseLoaderHeap(loaderHeapAddr, pCallback) : HResults.E_NOTIMPL; int ISOSDacInterface.TraverseModuleMap(int mmt, ClrDataAddress moduleAddr, void* pCallback, void* token) - => _legacyImpl is not null ? _legacyImpl.TraverseModuleMap(mmt, moduleAddr, pCallback, token) : HResults.E_NOTIMPL; + { + int hr = HResults.S_OK; + if (moduleAddr == 0) + hr = HResults.E_INVALIDARG; + else + { + try + { + Contracts.ILoader loader = _target.Contracts.Loader; + TargetPointer moduleAddrPtr = moduleAddr.ToTargetPointer(_target); + Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(moduleAddrPtr); + Contracts.ModuleLookupTables lookupTables = loader.GetLookupTables(moduleHandle); + IEnumerable<(TargetPointer, uint)>? elements = null; + switch ((ModuleMapType)mmt) + { + case ModuleMapType.TYPEDEFTOMETHODTABLE: + elements = loader.IterateModuleLookupMap(lookupTables.TypeDefToMethodTable); + break; + case ModuleMapType.TYPEREFTOMETHODTABLE: + elements = loader.IterateModuleLookupMap(lookupTables.TypeRefToMethodTable); + break; + default: + hr = HResults.E_INVALIDARG; + break; + } + if (elements != null && hr == HResults.S_OK) + { + foreach ((TargetPointer element, uint index) in elements) + { + // Call the callback with each element + var callback = (delegate* unmanaged[Stdcall])pCallback; + callback(index, element.ToClrDataAddress(_target), token); + } + } + } + catch (System.Exception ex) + { + hr = ex.HResult; + } +#if DEBUG + if (_legacyImpl is not null) + { + // side effects here so dumpmodule -mt will print twice in debug, ensure that they match + int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, pCallback, token); + Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); + } + } +#endif + return hr; + } int ISOSDacInterface.TraverseRCWCleanupList(ClrDataAddress cleanupListPtr, void* pCallback, void* token) => _legacyImpl is not null ? _legacyImpl.TraverseRCWCleanupList(cleanupListPtr, pCallback, token) : HResults.E_NOTIMPL; int ISOSDacInterface.TraverseVirtCallStubHeap(ClrDataAddress pAppDomain, int heaptype, void* pCallback) From bd9906cb98718ade88d51ede08f957616b986194 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Tue, 12 Aug 2025 11:32:21 -0700 Subject: [PATCH 2/8] comment --- docs/design/datacontracts/Loader.md | 2 +- .../Contracts/Loader_1.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index 25602dca4cdfe9..086bc75d3b32c9 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -481,7 +481,7 @@ IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table) // have to read lookupMap an extra time upfront because only the first map // has valid supportedFlagsMask TargetNUInt supportedFlagsMask = target.ReadNUInt(table + /* ModuleLookupMap::SupportedFlagsMask */); - uint index = 1; + uint index = 1; // zero is invalid do { uint count = target.Read(table + /*ModuleLookupMap::Count*/); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index b25fa100dd4b79..4f1d9b83c7149b 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -389,7 +389,7 @@ TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, // have to read lookupMap an extra time upfront because only the first map // has valid supportedFlagsMask TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; - uint index = 1; + uint index = 1; // zero is invalid do { lookupMap = _target.ProcessedData.GetOrAdd(table); From 1794d349c923405a7ce0d007d42fc56258a87578 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Tue, 12 Aug 2025 16:55:15 -0700 Subject: [PATCH 3/8] restructuring debug statement and module lookup --- .../Contracts/Loader_1.cs | 54 +++++++++---------- .../Legacy/ISOSDacInterface.cs | 2 +- .../Legacy/SOSDacImpl.cs | 40 +++++++++++--- 3 files changed, 60 insertions(+), 36 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index 4f1d9b83c7149b..ed484c07bd9dea 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -352,26 +352,17 @@ ModuleLookupTables ILoader.GetLookupTables(ModuleHandle handle) module.MethodDefToILCodeVersioningStateMap); } - TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags) + private TargetPointer GetModuleLookupMapAtIndex(ref TargetPointer table, ref uint index, ref TargetNUInt flags, TargetNUInt supportedFlagsMask) { - uint rid = EcmaMetadataUtils.GetRowId(token); - ArgumentOutOfRangeException.ThrowIfZero(rid); - flags = new TargetNUInt(0); - if (table == TargetPointer.Null) - return TargetPointer.Null; - uint index = rid; - Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); - // have to read lookupMap an extra time upfront because only the first map - // has valid supportedFlagsMask - TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; do { - lookupMap = _target.ProcessedData.GetOrAdd(table); + Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); if (index < lookupMap.Count) { TargetPointer entryAddress = lookupMap.TableData + (ulong)(index * _target.PointerSize); TargetPointer rawValue = _target.ReadPointer(entryAddress); flags = new TargetNUInt(rawValue & supportedFlagsMask.Value); + index++; return rawValue & ~(supportedFlagsMask.Value); } else @@ -383,29 +374,38 @@ TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, return TargetPointer.Null; } + TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags) + { + if (table == TargetPointer.Null) + { + flags = new TargetNUInt(0); + return TargetPointer.Null; + } + + Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); + TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; + + uint rid = EcmaMetadataUtils.GetRowId(token); + ArgumentOutOfRangeException.ThrowIfZero(rid); + flags = new TargetNUInt(0); + uint index = rid; + return GetModuleLookupMapAtIndex(ref table, ref index, ref flags, supportedFlagsMask); + } + IEnumerable<(TargetPointer, uint)> ILoader.IterateModuleLookupMap(TargetPointer table) { + if (table == TargetPointer.Null) + yield break; Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); - // have to read lookupMap an extra time upfront because only the first map - // has valid supportedFlagsMask TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; + TargetNUInt flags = new TargetNUInt(0); uint index = 1; // zero is invalid do { - lookupMap = _target.ProcessedData.GetOrAdd(table); - if (index < lookupMap.Count) + TargetPointer nxt = GetModuleLookupMapAtIndex(ref table, ref index, ref flags, supportedFlagsMask); + if (nxt != TargetPointer.Null) { - TargetPointer entryAddress = lookupMap.TableData + (ulong)(index * _target.PointerSize); - TargetPointer rawValue = _target.ReadPointer(entryAddress); - ulong maskedValue = rawValue & ~(supportedFlagsMask.Value); - if (maskedValue != 0) - yield return (new TargetPointer(maskedValue), index); - index++; - } - else - { - table = lookupMap.Next; - index -= lookupMap.Count; + yield return (new TargetPointer(nxt), index-1); } } while (table != TargetPointer.Null); } diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs index 74b19286157ca4..240b50931ec7a8 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs @@ -280,7 +280,7 @@ internal unsafe partial interface ISOSDacInterface [PreserveSig] int GetModuleData(ClrDataAddress moduleAddr, DacpModuleData* data); [PreserveSig] - int TraverseModuleMap(/*ModuleMapType*/ int mmt, ClrDataAddress moduleAddr, /*MODULEMAPTRAVERSE*/ void* pCallback, void* token); + int TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token); [PreserveSig] int GetAssemblyModuleList(ClrDataAddress assembly, uint count, [In, Out, MarshalUsing(CountElementName = nameof(count))] ClrDataAddress[] modules, uint* pNeeded); [PreserveSig] diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs index 6802c201675b97..eea795d0b195d2 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs @@ -2142,9 +2142,25 @@ int ISOSDacInterface.TraverseEHInfo(ClrDataAddress ip, void* pCallback, void* to => _legacyImpl is not null ? _legacyImpl.TraverseEHInfo(ip, pCallback, token) : HResults.E_NOTIMPL; int ISOSDacInterface.TraverseLoaderHeap(ClrDataAddress loaderHeapAddr, void* pCallback) => _legacyImpl is not null ? _legacyImpl.TraverseLoaderHeap(loaderHeapAddr, pCallback) : HResults.E_NOTIMPL; - int ISOSDacInterface.TraverseModuleMap(int mmt, ClrDataAddress moduleAddr, void* pCallback, void* token) + + [UnmanagedFunctionPointer(CallingConvention.StdCall)] + private delegate void TraverseModuleMapCallback(uint index, ClrDataAddress moduleAddr, void* expectedElements); + private static void TraverseModuleMapCallbackImpl(uint index, ClrDataAddress moduleAddr, void* expectedElements) + { + var expectedElementsDict = (Dictionary)GCHandle.FromIntPtr((nint)expectedElements).Target!; + if (expectedElementsDict.TryGetValue(moduleAddr, out uint expectedIndex) && expectedIndex == index) + { + expectedElementsDict[default]++; // Increment the count for verification + } + else + { + Debug.Assert(false, $"Unexpected module address {moduleAddr:x} at index {index}"); + } + } + int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token) { int hr = HResults.S_OK; + IEnumerable<(TargetPointer Address, uint Index)> elements = Enumerable.Empty<(TargetPointer, uint)>(); if (moduleAddr == 0) hr = HResults.E_INVALIDARG; else @@ -2155,8 +2171,7 @@ int ISOSDacInterface.TraverseModuleMap(int mmt, ClrDataAddress moduleAddr, void* TargetPointer moduleAddrPtr = moduleAddr.ToTargetPointer(_target); Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(moduleAddrPtr); Contracts.ModuleLookupTables lookupTables = loader.GetLookupTables(moduleHandle); - IEnumerable<(TargetPointer, uint)>? elements = null; - switch ((ModuleMapType)mmt) + switch (mmt) { case ModuleMapType.TYPEDEFTOMETHODTABLE: elements = loader.IterateModuleLookupMap(lookupTables.TypeDefToMethodTable); @@ -2168,13 +2183,12 @@ int ISOSDacInterface.TraverseModuleMap(int mmt, ClrDataAddress moduleAddr, void* hr = HResults.E_INVALIDARG; break; } - if (elements != null && hr == HResults.S_OK) + if (hr == HResults.S_OK) { foreach ((TargetPointer element, uint index) in elements) { // Call the callback with each element - var callback = (delegate* unmanaged[Stdcall])pCallback; - callback(index, element.ToClrDataAddress(_target), token); + pCallback(index, element.ToClrDataAddress(_target), token); } } } @@ -2185,9 +2199,19 @@ int ISOSDacInterface.TraverseModuleMap(int mmt, ClrDataAddress moduleAddr, void* #if DEBUG if (_legacyImpl is not null) { - // side effects here so dumpmodule -mt will print twice in debug, ensure that they match - int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, pCallback, token); + Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target), tuple => tuple.Index); + expectedElements.Add(default, 0); + void* tokenDebug = GCHandle.ToIntPtr(GCHandle.Alloc(expectedElements)).ToPointer(); + + TraverseModuleMapCallback callbackDebug = new TraverseModuleMapCallback(TraverseModuleMapCallbackImpl); + GCHandle gch = GCHandle.Alloc(callbackDebug); + var callbackDebugPtr = (delegate* unmanaged[Stdcall])Marshal.GetFunctionPointerForDelegate(callbackDebug); + + int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, callbackDebugPtr, tokenDebug); Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); + Debug.Assert(expectedElements[default] == elements.Count(), $"cDAC: {elements.Count()} elements, DAC: {expectedElements[default]} elements"); + GCHandle.FromIntPtr((nint)tokenDebug).Free(); + gch.Free(); } } #endif From f2773a5937d6e72812d5f600c32e3531748da0ec Mon Sep 17 00:00:00 2001 From: Rachel Date: Tue, 12 Aug 2025 18:34:57 -0700 Subject: [PATCH 4/8] Update SOSDacImpl.cs --- .../Legacy/SOSDacImpl.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs index eea795d0b195d2..10c6d973196f34 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs @@ -2196,23 +2196,23 @@ int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleA { hr = ex.HResult; } + } #if DEBUG - if (_legacyImpl is not null) - { - Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target), tuple => tuple.Index); - expectedElements.Add(default, 0); - void* tokenDebug = GCHandle.ToIntPtr(GCHandle.Alloc(expectedElements)).ToPointer(); + if (_legacyImpl is not null) + { + Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target), tuple => tuple.Index); + expectedElements.Add(default, 0); + void* tokenDebug = GCHandle.ToIntPtr(GCHandle.Alloc(expectedElements)).ToPointer(); - TraverseModuleMapCallback callbackDebug = new TraverseModuleMapCallback(TraverseModuleMapCallbackImpl); - GCHandle gch = GCHandle.Alloc(callbackDebug); - var callbackDebugPtr = (delegate* unmanaged[Stdcall])Marshal.GetFunctionPointerForDelegate(callbackDebug); + TraverseModuleMapCallback callbackDebug = new TraverseModuleMapCallback(TraverseModuleMapCallbackImpl); + GCHandle gch = GCHandle.Alloc(callbackDebug); + var callbackDebugPtr = (delegate* unmanaged[Stdcall])Marshal.GetFunctionPointerForDelegate(callbackDebug); - int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, callbackDebugPtr, tokenDebug); - Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); - Debug.Assert(expectedElements[default] == elements.Count(), $"cDAC: {elements.Count()} elements, DAC: {expectedElements[default]} elements"); - GCHandle.FromIntPtr((nint)tokenDebug).Free(); - gch.Free(); - } + int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, callbackDebugPtr, tokenDebug); + Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); + Debug.Assert(expectedElements[default] == elements.Count(), $"cDAC: {elements.Count()} elements, DAC: {expectedElements[default]} elements"); + GCHandle.FromIntPtr((nint)tokenDebug).Free(); + gch.Free(); } #endif return hr; From 7338bcd793f1b0eb11c376556fc3eab8b64e3388 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Wed, 13 Aug 2025 16:14:05 -0700 Subject: [PATCH 5/8] code review --- docs/design/datacontracts/Loader.md | 4 +- .../Contracts/ILoader.cs | 2 +- .../Contracts/Loader_1.cs | 39 ++++++++++--------- .../Legacy/SOSDacImpl.cs | 38 +++++++++--------- 4 files changed, 42 insertions(+), 41 deletions(-) diff --git a/docs/design/datacontracts/Loader.md b/docs/design/datacontracts/Loader.md index 086bc75d3b32c9..d1c2aa430fae3e 100644 --- a/docs/design/datacontracts/Loader.md +++ b/docs/design/datacontracts/Loader.md @@ -74,7 +74,7 @@ TargetPointer GetILBase(ModuleHandle handle); TargetPointer GetAssemblyLoadContext(ModuleHandle handle); ModuleLookupTables GetLookupTables(ModuleHandle handle); TargetPointer GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags); -IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table); +IEnumerable<(TargetPointer, uint)> EnumerateModuleLookupMap(TargetPointer table); bool IsCollectible(ModuleHandle handle); bool IsAssemblyLoaded(ModuleHandle handle); TargetPointer GetGlobalLoaderAllocator(); @@ -475,7 +475,7 @@ TargetPointer GetModuleLookupMapElement(TargetPointer table, uint token, out Tar return TargetPointer.Null; } -IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table) +IEnumerable<(TargetPointer, uint)> EnumerateModuleLookupMap(TargetPointer table) { Data.ModuleLookupMap lookupMap = new Data.ModuleLookupMap(table); // have to read lookupMap an extra time upfront because only the first map diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs index f25dc59c8b6ae0..0ef7c19a2f7c61 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs @@ -97,7 +97,7 @@ public interface ILoader : IContract TargetPointer GetAssemblyLoadContext(ModuleHandle handle) => throw new NotImplementedException(); ModuleLookupTables GetLookupTables(ModuleHandle handle) => throw new NotImplementedException(); TargetPointer GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags) => throw new NotImplementedException(); - IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table) => throw new NotImplementedException(); + IEnumerable<(TargetPointer, uint)> EnumerateModuleLookupMap(TargetPointer table) => throw new NotImplementedException(); bool IsCollectible(ModuleHandle handle) => throw new NotImplementedException(); bool IsAssemblyLoaded(ModuleHandle handle) => throw new NotImplementedException(); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index ed484c07bd9dea..4f72e76b456e54 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.Diagnostics.DataContractReader.Data; @@ -352,8 +353,12 @@ ModuleLookupTables ILoader.GetLookupTables(ModuleHandle handle) module.MethodDefToILCodeVersioningStateMap); } - private TargetPointer GetModuleLookupMapAtIndex(ref TargetPointer table, ref uint index, ref TargetNUInt flags, TargetNUInt supportedFlagsMask) + private static (bool Done, uint NextIndex) IterateLookupMap(uint index) => (false, index + 1); + private static (bool Done, uint NextIndex) SearchLookupMap(uint index) => (true, index); + private delegate (bool Done, uint NextIndex) Delegate(uint index); + private IEnumerable<(TargetPointer, uint)> IterateModuleLookupMap(TargetPointer table, uint index, Delegate iterator) { + bool doneIterating; do { Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); @@ -361,9 +366,10 @@ private TargetPointer GetModuleLookupMapAtIndex(ref TargetPointer table, ref uin { TargetPointer entryAddress = lookupMap.TableData + (ulong)(index * _target.PointerSize); TargetPointer rawValue = _target.ReadPointer(entryAddress); - flags = new TargetNUInt(rawValue & supportedFlagsMask.Value); - index++; - return rawValue & ~(supportedFlagsMask.Value); + yield return (rawValue, index); + (doneIterating, index) = iterator(index); + if (doneIterating) + yield break; } else { @@ -371,7 +377,6 @@ private TargetPointer GetModuleLookupMapAtIndex(ref TargetPointer table, ref uin index -= lookupMap.Count; } } while (table != TargetPointer.Null); - return TargetPointer.Null; } TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, out TargetNUInt flags) @@ -383,31 +388,29 @@ TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer table, uint token, } Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); - TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; + ulong supportedFlagsMask = lookupMap.SupportedFlagsMask.Value; uint rid = EcmaMetadataUtils.GetRowId(token); ArgumentOutOfRangeException.ThrowIfZero(rid); - flags = new TargetNUInt(0); - uint index = rid; - return GetModuleLookupMapAtIndex(ref table, ref index, ref flags, supportedFlagsMask); + (TargetPointer rval, uint _) = IterateModuleLookupMap(table, rid, SearchLookupMap).FirstOrDefault(); + flags = new TargetNUInt(rval & supportedFlagsMask); + return rval & ~supportedFlagsMask; } - IEnumerable<(TargetPointer, uint)> ILoader.IterateModuleLookupMap(TargetPointer table) + IEnumerable<(TargetPointer, uint)> ILoader.EnumerateModuleLookupMap(TargetPointer table) { if (table == TargetPointer.Null) yield break; Data.ModuleLookupMap lookupMap = _target.ProcessedData.GetOrAdd(table); - TargetNUInt supportedFlagsMask = lookupMap.SupportedFlagsMask; + ulong supportedFlagsMask = lookupMap.SupportedFlagsMask.Value; TargetNUInt flags = new TargetNUInt(0); uint index = 1; // zero is invalid - do + foreach ((TargetPointer targetPointer, uint idx) in IterateModuleLookupMap(table, index, IterateLookupMap)) { - TargetPointer nxt = GetModuleLookupMapAtIndex(ref table, ref index, ref flags, supportedFlagsMask); - if (nxt != TargetPointer.Null) - { - yield return (new TargetPointer(nxt), index-1); - } - } while (table != TargetPointer.Null); + TargetPointer rval = targetPointer & ~supportedFlagsMask; + if (rval != TargetPointer.Null) + yield return (rval, idx); + } } bool ILoader.IsCollectible(ModuleHandle handle) diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs index eea795d0b195d2..95b24eca251718 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Linq; using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices.Marshalling; using System.Text; @@ -2143,9 +2144,9 @@ int ISOSDacInterface.TraverseEHInfo(ClrDataAddress ip, void* pCallback, void* to int ISOSDacInterface.TraverseLoaderHeap(ClrDataAddress loaderHeapAddr, void* pCallback) => _legacyImpl is not null ? _legacyImpl.TraverseLoaderHeap(loaderHeapAddr, pCallback) : HResults.E_NOTIMPL; - [UnmanagedFunctionPointer(CallingConvention.StdCall)] - private delegate void TraverseModuleMapCallback(uint index, ClrDataAddress moduleAddr, void* expectedElements); - private static void TraverseModuleMapCallbackImpl(uint index, ClrDataAddress moduleAddr, void* expectedElements) +#if DEBUG + [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] + private static void TraverseModuleMapCallback(uint index, ClrDataAddress moduleAddr, void* expectedElements) { var expectedElementsDict = (Dictionary)GCHandle.FromIntPtr((nint)expectedElements).Target!; if (expectedElementsDict.TryGetValue(moduleAddr, out uint expectedIndex) && expectedIndex == index) @@ -2157,6 +2158,7 @@ private static void TraverseModuleMapCallbackImpl(uint index, ClrDataAddress mod Debug.Assert(false, $"Unexpected module address {moduleAddr:x} at index {index}"); } } +#endif int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token) { int hr = HResults.S_OK; @@ -2174,10 +2176,10 @@ int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleA switch (mmt) { case ModuleMapType.TYPEDEFTOMETHODTABLE: - elements = loader.IterateModuleLookupMap(lookupTables.TypeDefToMethodTable); + elements = loader.EnumerateModuleLookupMap(lookupTables.TypeDefToMethodTable); break; case ModuleMapType.TYPEREFTOMETHODTABLE: - elements = loader.IterateModuleLookupMap(lookupTables.TypeRefToMethodTable); + elements = loader.EnumerateModuleLookupMap(lookupTables.TypeRefToMethodTable); break; default: hr = HResults.E_INVALIDARG; @@ -2196,23 +2198,19 @@ int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleA { hr = ex.HResult; } + } #if DEBUG - if (_legacyImpl is not null) - { - Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target), tuple => tuple.Index); - expectedElements.Add(default, 0); - void* tokenDebug = GCHandle.ToIntPtr(GCHandle.Alloc(expectedElements)).ToPointer(); - - TraverseModuleMapCallback callbackDebug = new TraverseModuleMapCallback(TraverseModuleMapCallbackImpl); - GCHandle gch = GCHandle.Alloc(callbackDebug); - var callbackDebugPtr = (delegate* unmanaged[Stdcall])Marshal.GetFunctionPointerForDelegate(callbackDebug); + if (_legacyImpl is not null) + { + Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target), tuple => tuple.Index); + expectedElements.Add(default, 0); + void* tokenDebug = GCHandle.ToIntPtr(GCHandle.Alloc(expectedElements)).ToPointer(); + delegate* unmanaged[Stdcall] callbackDebugPtr = &TraverseModuleMapCallback; - int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, callbackDebugPtr, tokenDebug); - Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); - Debug.Assert(expectedElements[default] == elements.Count(), $"cDAC: {elements.Count()} elements, DAC: {expectedElements[default]} elements"); - GCHandle.FromIntPtr((nint)tokenDebug).Free(); - gch.Free(); - } + int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, callbackDebugPtr, tokenDebug); + Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); + Debug.Assert(expectedElements[default] == elements.Count(), $"cDAC: {elements.Count()} elements, DAC: {expectedElements[default]} elements"); + GCHandle.FromIntPtr((nint)tokenDebug).Free(); } #endif return hr; From 4e727363111c2da70dd2f84ed1e7647dc6a9afed Mon Sep 17 00:00:00 2001 From: Rachel Date: Wed, 13 Aug 2025 16:21:08 -0700 Subject: [PATCH 6/8] Update Loader_1.cs --- .../Contracts/Loader_1.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs index 4f72e76b456e54..d2cfe716f6d723 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.Diagnostics.DataContractReader.Data; From f37017ae1534e79827b0ef6535b1b142bed8cc21 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Wed, 20 Aug 2025 08:54:51 -0700 Subject: [PATCH 7/8] codereview --- .../Legacy/ISOSDacInterface.cs | 2 +- .../cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs index e813fccb9abc05..57804d170af873 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs @@ -286,7 +286,7 @@ internal unsafe partial interface ISOSDacInterface [PreserveSig] int GetModuleData(ClrDataAddress moduleAddr, DacpModuleData* data); [PreserveSig] - int TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token); + int TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token); [PreserveSig] int GetAssemblyModuleList(ClrDataAddress assembly, uint count, [In, Out, MarshalUsing(CountElementName = nameof(count))] ClrDataAddress[] modules, uint* pNeeded); [PreserveSig] diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs index a10d78946fcee4..fe0749c21d6017 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs @@ -2146,9 +2146,9 @@ int ISOSDacInterface.TraverseLoaderHeap(ClrDataAddress loaderHeapAddr, void* pCa #if DEBUG [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] - private static void TraverseModuleMapCallback(uint index, ClrDataAddress moduleAddr, void* expectedElements) + private static void TraverseModuleMapCallback(uint index, ulong moduleAddr, void* expectedElements) { - var expectedElementsDict = (Dictionary)GCHandle.FromIntPtr((nint)expectedElements).Target!; + var expectedElementsDict = (Dictionary)GCHandle.FromIntPtr((nint)expectedElements).Target!; if (expectedElementsDict.TryGetValue(moduleAddr, out uint expectedIndex) && expectedIndex == index) { expectedElementsDict[default]++; // Increment the count for verification @@ -2159,7 +2159,7 @@ private static void TraverseModuleMapCallback(uint index, ClrDataAddress moduleA } } #endif - int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token) + int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token) { int hr = HResults.S_OK; IEnumerable<(TargetPointer Address, uint Index)> elements = Enumerable.Empty<(TargetPointer, uint)>(); @@ -2190,7 +2190,7 @@ int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleA foreach ((TargetPointer element, uint index) in elements) { // Call the callback with each element - pCallback(index, element.ToClrDataAddress(_target), token); + pCallback(index, element.ToClrDataAddress(_target).Value, token); } } } @@ -2202,10 +2202,10 @@ int ISOSDacInterface.TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleA #if DEBUG if (_legacyImpl is not null) { - Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target), tuple => tuple.Index); + Dictionary expectedElements = elements.ToDictionary(tuple => tuple.Address.ToClrDataAddress(_target).Value, tuple => tuple.Index); expectedElements.Add(default, 0); void* tokenDebug = GCHandle.ToIntPtr(GCHandle.Alloc(expectedElements)).ToPointer(); - delegate* unmanaged[Stdcall] callbackDebugPtr = &TraverseModuleMapCallback; + delegate* unmanaged[Stdcall] callbackDebugPtr = &TraverseModuleMapCallback; int hrLocal = _legacyImpl.TraverseModuleMap(mmt, moduleAddr, callbackDebugPtr, tokenDebug); Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); From 8dd82dc9db4494304d58dddd5083a88d2eef96b0 Mon Sep 17 00:00:00 2001 From: Rachel Date: Mon, 25 Aug 2025 14:45:39 -0700 Subject: [PATCH 8/8] Update src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs Co-authored-by: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> --- .../cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs index 57804d170af873..6a5d3aa40157a1 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Legacy/ISOSDacInterface.cs @@ -286,7 +286,7 @@ internal unsafe partial interface ISOSDacInterface [PreserveSig] int GetModuleData(ClrDataAddress moduleAddr, DacpModuleData* data); [PreserveSig] - int TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token); + int TraverseModuleMap(ModuleMapType mmt, ClrDataAddress moduleAddr, delegate* unmanaged[Stdcall] pCallback, void* token); [PreserveSig] int GetAssemblyModuleList(ClrDataAddress assembly, uint count, [In, Out, MarshalUsing(CountElementName = nameof(count))] ClrDataAddress[] modules, uint* pNeeded); [PreserveSig]