From da9b4cc619d4d9c17c17b87c9c13f0b027c391a5 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Fri, 13 Mar 2026 15:16:14 -0700 Subject: [PATCH 01/10] adding EnumMethodDefinitionByName etc --- .../ClrDataMethodDefinition.cs | 80 ++++++ .../ClrDataModule.cs | 265 +++++++++++++++++- .../IXCLRData.cs | 6 + 3 files changed, 348 insertions(+), 3 deletions(-) create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs new file mode 100644 index 00000000000000..30cd9192d9566a --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs @@ -0,0 +1,80 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices.Marshalling; +using Microsoft.Diagnostics.DataContractReader.Contracts; + +namespace Microsoft.Diagnostics.DataContractReader.Legacy; + +[GeneratedComClass] +public sealed unsafe partial class ClrDataMethodDefinition : IXCLRDataMethodDefinition +{ + private readonly Target _target; + private readonly TargetPointer _module; + private readonly uint _token; + private readonly TargetPointer _methodDesc; + private readonly IXCLRDataMethodDefinition? _legacyImpl; + public ClrDataMethodDefinition( + Target target, + TargetPointer module, + uint token, + IXCLRDataMethodDefinition? legacyImpl) + { + _target = target; + _module = module; + _token = token; + _methodDesc = TargetPointer.Null; // MethodDesc is lazily initialized in GetRepresentativeEntryAddress + _legacyImpl = legacyImpl; + } + int IXCLRDataMethodDefinition.GetTypeDefinition(DacComNullableByRef typeDefinition) + => _legacyImpl is not null ? _legacyImpl.GetTypeDefinition(typeDefinition) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.StartEnumInstances(IXCLRDataAppDomain? appDomain, ulong* handle) + => _legacyImpl is not null ? _legacyImpl.StartEnumInstances(appDomain, handle) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.EnumInstance(ulong* handle, DacComNullableByRef instance) + => _legacyImpl is not null ? _legacyImpl.EnumInstance(handle, instance) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.EndEnumInstances(ulong handle) + => _legacyImpl is not null ? _legacyImpl.EndEnumInstances(handle) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.GetName(uint flags, uint bufLen, uint* nameLen, char* name) + => _legacyImpl is not null ? _legacyImpl.GetName(flags, bufLen, nameLen, name) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.GetTokenAndScope(uint* token, DacComNullableByRef mod) + => _legacyImpl is not null ? _legacyImpl.GetTokenAndScope(token, mod) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.GetFlags(uint* flags) + => _legacyImpl is not null ? _legacyImpl.GetFlags(flags) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.IsSameObject(IXCLRDataMethodDefinition? method) + => _legacyImpl is not null ? _legacyImpl.IsSameObject(method) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.GetLatestEnCVersion(uint* version) + => _legacyImpl is not null ? _legacyImpl.GetLatestEnCVersion(version) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.StartEnumExtents(ulong* handle) + => _legacyImpl is not null ? _legacyImpl.StartEnumExtents(handle) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.EnumExtent(ulong* handle, ClrDataMethodDefinitionExtent* extent) + => _legacyImpl is not null ? _legacyImpl.EnumExtent(handle, extent) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.EndEnumExtents(ulong handle) + => _legacyImpl is not null ? _legacyImpl.EndEnumExtents(handle) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.GetCodeNotification(uint* flags) + => _legacyImpl is not null ? _legacyImpl.GetCodeNotification(flags) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.SetCodeNotification(uint flags) + => _legacyImpl is not null ? _legacyImpl.SetCodeNotification(flags) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.Request(uint reqCode, uint inBufferSize, byte* inBuffer, uint outBufferSize, byte* outBuffer) + => _legacyImpl is not null ? _legacyImpl.Request(reqCode, inBufferSize, inBuffer, outBufferSize, outBuffer) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.GetRepresentativeEntryAddress(ClrDataAddress* addr) + => _legacyImpl is not null ? _legacyImpl.GetRepresentativeEntryAddress(addr) : HResults.E_NOTIMPL; + + int IXCLRDataMethodDefinition.HasClassOrMethodInstantiation(int* bGeneric) + => _legacyImpl is not null ? _legacyImpl.HasClassOrMethodInstantiation(bGeneric) : HResults.E_NOTIMPL; +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 1e01218e074dbe..0188af12df55f7 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -3,9 +3,13 @@ using System; using System.Diagnostics; +using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; +using System.Reflection.Metadata.Ecma335; +using System.Reflection.Metadata; +using System.Collections.Generic; using Microsoft.Diagnostics.DataContractReader.Contracts; namespace Microsoft.Diagnostics.DataContractReader.Legacy; @@ -60,6 +64,122 @@ CustomQueryInterfaceResult ICustomQueryInterface.GetInterface(ref Guid iid, out return CustomQueryInterfaceResult.NotHandled; } + internal sealed class EnumMethodDefinitions + { + private readonly uint _flags; + private readonly MetadataReader _reader; + private TypeDefinitionHandle? _typeHandle; + private string? _methodName; + public IEnumerator MethodEnumerator = Enumerable.Empty().GetEnumerator(); + public TargetPointer LegacyHandle { get; set; } = TargetPointer.Null; + + public EnumMethodDefinitions(MetadataReader reader, uint flags, TargetPointer legacyHandle = default) + { + _reader = reader; + _flags = flags; + LegacyHandle = legacyHandle; + } + + public void Start(string fullName) + { + // start: find the type. + int parenIndex = fullName.IndexOf('('); + if (parenIndex >= 0) + fullName = fullName[..parenIndex]; + + int lastNestingSep = Math.Max(fullName.LastIndexOf('+'), fullName.LastIndexOf('/')); + int searchFrom = fullName.Length - 1; + + while (searchFrom > 0) + { + int dotPos = fullName.LastIndexOf('.', searchFrom); + if (dotPos <= 0) + break; + + while (dotPos > 0 && fullName[dotPos - 1] == '.') + dotPos--; + + if (dotPos <= lastNestingSep || dotPos <= 0) + break; + + string typePortion = fullName[..dotPos]; + string methodName = fullName[Math.Min(dotPos + 1, fullName.Length - 1)..]; + + _typeHandle = ResolveType(_reader, typePortion); + if (_typeHandle != null) + { + _methodName = methodName; + break; + } + + searchFrom = dotPos - 1; + } + + if (_typeHandle == null) + throw new ArgumentException(); + MethodEnumerator = IterateMethodInstances().GetEnumerator(); + } + + private bool StringEquals(string a, string b) + { + StringComparison comparison = (_flags & (uint)CLRDataByNameFlag.CLRDATA_BYNAME_CASE_INSENSITIVE) != 0 ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + return string.Equals(a, b, comparison); + } + + private IEnumerable IterateMethodInstances() + { + foreach (MethodDefinitionHandle mh in _reader.GetTypeDefinition(_typeHandle!.Value).GetMethods()) + { + if (StringEquals(_reader.GetString(_reader.GetMethodDefinition(mh).Name), _methodName!)) + yield return (uint)MetadataTokens.GetToken(mh); + } + yield break; + } + + private TypeDefinitionHandle? ResolveType(MetadataReader reader, string typeFullName) + { + string[] nestingParts = typeFullName.Split('+', '/'); + + string firstPart = nestingParts[0]; + int lastDot = firstPart.LastIndexOf('.'); + + string ns = lastDot >= 0 ? firstPart[..lastDot] : ""; + string outerName = lastDot >= 0 ? firstPart[(lastDot + 1)..] : firstPart; + + TypeDefinitionHandle? current = FindTopLevelType(reader, ns, outerName); + + for (int i = 1; i < nestingParts.Length && current != null; i++) + current = FindNestedType(reader, current.Value, nestingParts[i]); + + return current; + } + + private TypeDefinitionHandle? FindTopLevelType(MetadataReader reader, string @namespace, string name) + { + foreach (TypeDefinitionHandle handle in reader.TypeDefinitions) + { + TypeDefinition td = reader.GetTypeDefinition(handle); + if (td.IsNested) + continue; + + if ((string.IsNullOrEmpty(@namespace) || StringEquals(reader.GetString(td.Namespace), @namespace)) && + StringEquals(reader.GetString(td.Name), name)) + return handle; + } + return null; + } + + private TypeDefinitionHandle? FindNestedType(MetadataReader reader, TypeDefinitionHandle enclosing, string name) + { + foreach (TypeDefinitionHandle nh in reader.GetTypeDefinition(enclosing).GetNestedTypes()) + { + if (StringEquals(reader.GetString(reader.GetTypeDefinition(nh).Name), name)) + return nh; + } + return null; + } + } + int IXCLRDataModule.StartEnumAssemblies(ulong* handle) => _legacyModule is not null ? _legacyModule.StartEnumAssemblies(handle) : HResults.E_NOTIMPL; int IXCLRDataModule.EnumAssembly(ulong* handle, DacComNullableByRef assembly) @@ -99,12 +219,151 @@ int IXCLRDataModule.GetTypeDefinitionByToken(/*mdTypeDef*/ uint token, DacComNul => _legacyModule is not null ? _legacyModule.GetTypeDefinitionByToken(token, typeDefinition) : HResults.E_NOTIMPL; int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulong* handle) - => _legacyModule is not null ? _legacyModule.StartEnumMethodDefinitionsByName(name, flags, handle) : HResults.E_NOTIMPL; + { + int hr = HResults.S_OK; + *handle = 0; + + // Start the legacy enumeration to keep it in sync with the cDAC enumeration. + ulong handleLocal = default; + int hrLocal = default; + if (_legacyModule is not null) + { + hrLocal = _legacyModule.StartEnumMethodDefinitionsByName(name, flags, &handleLocal); + } + + try + { + if (name == null || *name == '\0') + throw new ArgumentException(); + if ((flags & ~((uint)CLRDataByNameFlag.CLRDATA_BYNAME_CASE_SENSITIVE | (uint)CLRDataByNameFlag.CLRDATA_BYNAME_CASE_INSENSITIVE)) != 0) + throw new ArgumentException(); + string fullName = new string(name); + + // start: find the type. + ILoader loader = _target.Contracts.Loader; + Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_address); + MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!; + + EnumMethodDefinitions emd = new(reader, flags, handleLocal); + GCHandle gcHandle = GCHandle.Alloc(emd); + *handle = (ulong)GCHandle.ToIntPtr(gcHandle).ToInt64(); + emd.Start(fullName); + } + catch (System.Exception ex) + { + hr = ex.HResult; + } + + finally + { + if (hr < 0 && *handle != 0) + { + // Free the GCHandle if we failed to start the enumeration + GCHandle.FromIntPtr((IntPtr)(*handle)).Free(); + *handle = 0; + } + } + +#if DEBUG + if (_legacyModule is not null) + { + Debug.ValidateHResult(hr, hrLocal); + } +#endif + return hr; + } + int IXCLRDataModule.EnumMethodDefinitionByName(ulong* handle, DacComNullableByRef method) - => _legacyModule is not null ? _legacyModule.EnumMethodDefinitionByName(handle, method) : HResults.E_NOTIMPL; + { + int hr = HResults.S_OK; + EnumMethodDefinitions emd; + try + { + GCHandle gcHandle = GCHandle.FromIntPtr((IntPtr)(*handle)); + if (gcHandle.Target is not EnumMethodDefinitions emdLocal) + throw new ArgumentException(); + + emd = emdLocal; + } + catch (System.Exception ex) + { + return ex.HResult; + } + + // Advance the legacy enumeration to keep it in sync with the cDAC enumeration. + IXCLRDataMethodDefinition? legacyMethod = null; + int hrLocal = HResults.S_OK; + if (_legacyModule is not null) + { + ulong legacyHandle = emd.LegacyHandle; + DacComNullableByRef legacyMethodOut = new(isNullRef: false); + hrLocal = _legacyModule.EnumMethodDefinitionByName(&legacyHandle, legacyMethodOut); + legacyMethod = legacyMethodOut.Interface; + emd.LegacyHandle = legacyHandle; + } + + try + { + if (emd.MethodEnumerator.MoveNext()) + { + uint token = emd.MethodEnumerator.Current; + method.Interface = new ClrDataMethodDefinition(_target, _address, token, legacyMethod); + } + else + { + hr = HResults.S_FALSE; + } + } + catch (System.Exception ex) + { + // Fall back to the legacy DAC result when available, otherwise propagate the error. + if (_legacyModule is not null) + { + hr = hrLocal; + method.Interface = legacyMethod; + } + else + { + hr = ex.HResult; + } + } + +#if DEBUG + if (_legacyModule is not null) + { + Debug.ValidateHResult(hr, hrLocal); + } +#endif + + return hr; + } int IXCLRDataModule.EndEnumMethodDefinitionsByName(ulong handle) - => _legacyModule is not null ? _legacyModule.EndEnumMethodDefinitionsByName(handle) : HResults.E_NOTIMPL; + { + int hr = HResults.S_OK; + EnumMethodDefinitions emd; + try + { + GCHandle gcHandle = GCHandle.FromIntPtr((IntPtr)handle); + if (gcHandle.Target is not EnumMethodDefinitions emdLocal) + throw new ArgumentException(); + emd = emdLocal; + emd.MethodEnumerator.Dispose(); + gcHandle.Free(); + } + catch (System.Exception ex) + { + return ex.HResult; + } + + if (_legacyModule != null && emd.LegacyHandle != TargetPointer.Null) + { + int hrLocal = _legacyModule.EndEnumMethodDefinitionsByName(emd.LegacyHandle); + if (hrLocal < 0) + return hrLocal; + } + return hr; + } int IXCLRDataModule.StartEnumMethodInstancesByName(char* name, uint flags, IXCLRDataAppDomain? appDomain, ulong* handle) => _legacyModule is not null ? _legacyModule.StartEnumMethodInstancesByName(name, flags, appDomain, handle) : HResults.E_NOTIMPL; int IXCLRDataModule.EnumMethodInstanceByName(ulong* handle, DacComNullableByRef method) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs index 862d9c6956b8c5..56341e9e655766 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs @@ -29,6 +29,12 @@ public struct DacpGetModuleData public ulong InMemoryPdbSize; } +public enum CLRDataByNameFlag : uint +{ + CLRDATA_BYNAME_CASE_SENSITIVE = 0, + CLRDATA_BYNAME_CASE_INSENSITIVE = 0x1 +} + [GeneratedComInterface] [Guid("88E32849-0A0A-4cb0-9022-7CD2E9E139E2")] public unsafe partial interface IXCLRDataModule From 159ae0376f2485c9f33fdfc94a0e0a45ea48be30 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Fri, 13 Mar 2026 15:55:33 -0700 Subject: [PATCH 02/10] code review --- .../ClrDataMethodDefinition.cs | 2 -- .../ClrDataModule.cs | 29 +++++++------------ 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs index 30cd9192d9566a..d269d804e4b98c 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs @@ -13,7 +13,6 @@ public sealed unsafe partial class ClrDataMethodDefinition : IXCLRDataMethodDefi private readonly Target _target; private readonly TargetPointer _module; private readonly uint _token; - private readonly TargetPointer _methodDesc; private readonly IXCLRDataMethodDefinition? _legacyImpl; public ClrDataMethodDefinition( Target target, @@ -24,7 +23,6 @@ public ClrDataMethodDefinition( _target = target; _module = module; _token = token; - _methodDesc = TargetPointer.Null; // MethodDesc is lazily initialized in GetRepresentativeEntryAddress _legacyImpl = legacyImpl; } int IXCLRDataMethodDefinition.GetTypeDefinition(DacComNullableByRef typeDefinition) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 0188af12df55f7..2d06d956eac310 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -73,11 +73,10 @@ internal sealed class EnumMethodDefinitions public IEnumerator MethodEnumerator = Enumerable.Empty().GetEnumerator(); public TargetPointer LegacyHandle { get; set; } = TargetPointer.Null; - public EnumMethodDefinitions(MetadataReader reader, uint flags, TargetPointer legacyHandle = default) + public EnumMethodDefinitions(MetadataReader reader, uint flags) { _reader = reader; _flags = flags; - LegacyHandle = legacyHandle; } public void Start(string fullName) @@ -226,11 +225,6 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo // Start the legacy enumeration to keep it in sync with the cDAC enumeration. ulong handleLocal = default; int hrLocal = default; - if (_legacyModule is not null) - { - hrLocal = _legacyModule.StartEnumMethodDefinitionsByName(name, flags, &handleLocal); - } - try { if (name == null || *name == '\0') @@ -244,26 +238,21 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_address); MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!; - EnumMethodDefinitions emd = new(reader, flags, handleLocal); + EnumMethodDefinitions emd = new(reader, flags); + emd.Start(fullName); + if (_legacyModule is not null) + { + hrLocal = _legacyModule.StartEnumMethodDefinitionsByName(name, flags, &handleLocal); + } + emd.LegacyHandle = handleLocal; GCHandle gcHandle = GCHandle.Alloc(emd); *handle = (ulong)GCHandle.ToIntPtr(gcHandle).ToInt64(); - emd.Start(fullName); } catch (System.Exception ex) { hr = ex.HResult; } - finally - { - if (hr < 0 && *handle != 0) - { - // Free the GCHandle if we failed to start the enumeration - GCHandle.FromIntPtr((IntPtr)(*handle)).Free(); - *handle = 0; - } - } - #if DEBUG if (_legacyModule is not null) { @@ -279,6 +268,8 @@ int IXCLRDataModule.EnumMethodDefinitionByName(ulong* handle, DacComNullableByRe EnumMethodDefinitions emd; try { + if (method.IsNullRef) + throw new NullReferenceException(); GCHandle gcHandle = GCHandle.FromIntPtr((IntPtr)(*handle)); if (gcHandle.Target is not EnumMethodDefinitions emdLocal) throw new ArgumentException(); From f32e75d77e12fb860fa210cfcd3ddf23aaf8eebc Mon Sep 17 00:00:00 2001 From: rcj1 Date: Sat, 14 Mar 2026 08:19:31 -0700 Subject: [PATCH 03/10] validation --- .../ClrDataModule.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 2d06d956eac310..b7ca57337afc36 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -73,10 +73,11 @@ internal sealed class EnumMethodDefinitions public IEnumerator MethodEnumerator = Enumerable.Empty().GetEnumerator(); public TargetPointer LegacyHandle { get; set; } = TargetPointer.Null; - public EnumMethodDefinitions(MetadataReader reader, uint flags) + public EnumMethodDefinitions(MetadataReader reader, uint flags, TargetPointer legacyHandle) { _reader = reader; _flags = flags; + LegacyHandle = legacyHandle; } public void Start(string fullName) @@ -227,6 +228,10 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo int hrLocal = default; try { + if (_legacyModule is not null) + { + hrLocal = _legacyModule.StartEnumMethodDefinitionsByName(name, flags, &handleLocal); + } if (name == null || *name == '\0') throw new ArgumentException(); if ((flags & ~((uint)CLRDataByNameFlag.CLRDATA_BYNAME_CASE_SENSITIVE | (uint)CLRDataByNameFlag.CLRDATA_BYNAME_CASE_INSENSITIVE)) != 0) @@ -238,13 +243,8 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_address); MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!; - EnumMethodDefinitions emd = new(reader, flags); + EnumMethodDefinitions emd = new(reader, flags, handleLocal); emd.Start(fullName); - if (_legacyModule is not null) - { - hrLocal = _legacyModule.StartEnumMethodDefinitionsByName(name, flags, &handleLocal); - } - emd.LegacyHandle = handleLocal; GCHandle gcHandle = GCHandle.Alloc(emd); *handle = (ulong)GCHandle.ToIntPtr(gcHandle).ToInt64(); } From 9ff1b05d143c413224895b9893ee4051d4839bed Mon Sep 17 00:00:00 2001 From: rcj1 Date: Sat, 14 Mar 2026 11:24:12 -0700 Subject: [PATCH 04/10] adding tests --- .../ClrDataModule.cs | 9 +- .../cdac/tests/EnumMethodDefinitionsTests.cs | 208 ++++++++++++++++++ 2 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index b7ca57337afc36..c44812035889c9 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -64,7 +64,7 @@ CustomQueryInterfaceResult ICustomQueryInterface.GetInterface(ref Guid iid, out return CustomQueryInterfaceResult.NotHandled; } - internal sealed class EnumMethodDefinitions + public sealed class EnumMethodDefinitions { private readonly uint _flags; private readonly MetadataReader _reader; @@ -305,18 +305,13 @@ int IXCLRDataModule.EnumMethodDefinitionByName(ulong* handle, DacComNullableByRe hr = HResults.S_FALSE; } } - catch (System.Exception ex) + catch (System.Exception) { // Fall back to the legacy DAC result when available, otherwise propagate the error. if (_legacyModule is not null) { - hr = hrLocal; method.Interface = legacyMethod; } - else - { - hr = ex.HResult; - } } #if DEBUG diff --git a/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs b/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs new file mode 100644 index 00000000000000..d56f16c449ac25 --- /dev/null +++ b/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs @@ -0,0 +1,208 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using Microsoft.Diagnostics.DataContractReader.Legacy; +using Xunit; + +namespace Microsoft.Diagnostics.DataContractReader.Tests; + +public unsafe class EnumMethodDefinitionsTests +{ + // Synthetic metadata containing a variety of type and method definitions. + // + // System.String → Concat, Join, Concat (overload), .ctor, .ctor (overload) + // MyNs.MyClass → MyMethod, ToString, mymethod (case variant), .ctor + // GlobalType (no namespace)→ Foo + // MyNs.Generic`1 → DoWork + // MyNs.Outer → OuterMethod + // MyNs.Outer+Inner → InnerMethod + // Deep.Nested.Ns.DeepType → DeepMethod + private static readonly byte[] s_metadataBytes = BuildTestMetadata(); + + private static byte[] BuildTestMetadata() + { + var mdBuilder = new MetadataBuilder(); + + mdBuilder.AddModule( + 0, + mdBuilder.GetOrAddString("TestModule"), + mdBuilder.GetOrAddGuid(Guid.NewGuid()), + default, default); + + mdBuilder.AddAssembly( + mdBuilder.GetOrAddString("TestAssembly"), + new Version(1, 0, 0, 0), + default, default, 0, + AssemblyHashAlgorithm.None); + + var sigBlob = new BlobBuilder(); + sigBlob.WriteByte(0x00); // DEFAULT calling convention + sigBlob.WriteCompressedInteger(0); + sigBlob.WriteByte(0x01); // ELEMENT_TYPE_VOID + BlobHandle voidSig = mdBuilder.GetOrAddBlob(sigBlob); + + int methodRow = 1; + + MethodDefinitionHandle NextMethod() => MetadataTokens.MethodDefinitionHandle(methodRow); + + void AddMethod(string name) + { + mdBuilder.AddMethodDefinition( + MethodAttributes.Public, default, + mdBuilder.GetOrAddString(name), voidSig, -1, + MetadataTokens.ParameterHandle(1)); + methodRow++; + } + + // (required first type) + mdBuilder.AddTypeDefinition( + default, default, mdBuilder.GetOrAddString(""), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + + // System.String with overloaded Concat and constructors + mdBuilder.AddTypeDefinition( + TypeAttributes.Public, + mdBuilder.GetOrAddString("System"), + mdBuilder.GetOrAddString("String"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("Concat"); + AddMethod("Join"); + AddMethod("Concat"); + AddMethod(".ctor"); + AddMethod(".ctor"); + + // MyNs.MyClass with case-variant method names and a constructor + mdBuilder.AddTypeDefinition( + TypeAttributes.Public, + mdBuilder.GetOrAddString("MyNs"), + mdBuilder.GetOrAddString("MyClass"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("MyMethod"); + AddMethod("ToString"); + AddMethod("mymethod"); + AddMethod(".ctor"); + + // GlobalType with no namespace + mdBuilder.AddTypeDefinition( + TypeAttributes.Public, default, + mdBuilder.GetOrAddString("GlobalType"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("Foo"); + + // MyNs.Generic`1 (generic type with backtick arity notation) + mdBuilder.AddTypeDefinition( + TypeAttributes.Public, + mdBuilder.GetOrAddString("MyNs"), + mdBuilder.GetOrAddString("Generic`1"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("DoWork"); + + // MyNs.Outer + TypeDefinitionHandle outerHandle = mdBuilder.AddTypeDefinition( + TypeAttributes.Public, + mdBuilder.GetOrAddString("MyNs"), + mdBuilder.GetOrAddString("Outer"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("OuterMethod"); + + // Inner (nested inside Outer) + TypeDefinitionHandle innerHandle = mdBuilder.AddTypeDefinition( + TypeAttributes.NestedPublic, default, + mdBuilder.GetOrAddString("Inner"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("InnerMethod"); + mdBuilder.AddNestedType(innerHandle, outerHandle); + + // Deep.Nested.Ns.DeepType (multi-level namespace) + mdBuilder.AddTypeDefinition( + TypeAttributes.Public, + mdBuilder.GetOrAddString("Deep.Nested.Ns"), + mdBuilder.GetOrAddString("DeepType"), + default, MetadataTokens.FieldDefinitionHandle(1), NextMethod()); + AddMethod("DeepMethod"); + + var rootBuilder = new MetadataRootBuilder(mdBuilder); + var blobBuilder = new BlobBuilder(); + rootBuilder.Serialize(blobBuilder, 0, 0); + return blobBuilder.ToArray(); + } + + [Theory] + // Fully qualified names + [InlineData("System.String.Join", 0u, 1)] + [InlineData("MyNs.MyClass.MyMethod", 0u, 1)] + [InlineData("Deep.Nested.Ns.DeepType.DeepMethod", 0u, 1)] + // Namespaces are optional — omitting the namespace matches any namespace + [InlineData("String.Join", 0u, 1)] + [InlineData("MyClass.MyMethod", 0u, 1)] + [InlineData("GlobalType.Foo", 0u, 1)] + [InlineData("DeepType.DeepMethod", 0u, 1)] + // Overloaded methods enumerate all matching definitions + [InlineData("System.String.Concat", 0u, 2)] + // Case-sensitive (flag 0): exact match required + [InlineData("MyNs.MyClass.mymethod", 0u, 1)] + [InlineData("MyNs.MyClass.MYMETHOD", 0u, 0)] + // Case-insensitive (flag 1): matches both MyMethod and mymethod + [InlineData("MyNs.MyClass.MYMETHOD", 1u, 2)] + [InlineData("myns.myclass.mymethod", 1u, 2)] + [InlineData("myns.myclass.MYMETHOD", 1u, 2)] + // Method parameters in parentheses are stripped and ignored + [InlineData("System.String.Join()", 0u, 1)] + [InlineData("System.String.Concat(System.String, System.String)", 0u, 2)] + [InlineData("MyNs.MyClass.MyMethod(int)", 0u, 1)] + // Generic types use backtick arity notation + [InlineData("MyNs.Generic`1.DoWork", 0u, 1)] + // Nested types with '+' separator + [InlineData("MyNs.Outer+Inner.InnerMethod", 0u, 1)] + // Nested types with '/' separator + [InlineData("MyNs.Outer/Inner.InnerMethod", 0u, 1)] + // Nested types without namespace + [InlineData("Outer+Inner.InnerMethod", 0u, 1)] + [InlineData("Outer/Inner.InnerMethod", 0u, 1)] + // Constructors use the '.ctor' name — the double-dot is handled correctly + [InlineData("System.String..ctor", 0u, 2)] + [InlineData("MyNs.MyClass..ctor", 0u, 1)] + // Non-existent method on an existing type returns zero matches + [InlineData("System.String.NonExistent", 0u, 0)] + // Outer type method is still accessible + [InlineData("MyNs.Outer.OuterMethod", 0u, 1)] + public void EnumMethodsByName_ReturnsExpectedCount(string fullName, uint flags, int expectedCount) + { + fixed (byte* ptr = s_metadataBytes) + { + var reader = new MetadataReader(ptr, s_metadataBytes.Length); + var emd = new ClrDataModule.EnumMethodDefinitions(reader, flags, TargetPointer.Null); + emd.Start(fullName); + + int count = 0; + while (emd.MethodEnumerator.MoveNext()) + count++; + + Assert.Equal(expectedCount, count); + } + } + + [Theory] + // Non-existent type + [InlineData("NonExistent.Method")] + // Generic type without backtick notation is not found + [InlineData("MyNs.Generic.DoWork")] + // Bare method name without any type qualifier + [InlineData("Method")] + // Return type prefix causes resolution failure + [InlineData("void System.String.Join")] + public void EnumMethodsByName_ThrowsForUnresolvableInput(string fullName) + { + fixed (byte* ptr = s_metadataBytes) + { + var reader = new MetadataReader(ptr, s_metadataBytes.Length); + var emd = new ClrDataModule.EnumMethodDefinitions(reader, 0, TargetPointer.Null); + + Assert.Throws(() => emd.Start(fullName)); + } + } +} From 55ab5debbdcdad999a398d9fbb72e5578fe585cc Mon Sep 17 00:00:00 2001 From: rcj1 Date: Sat, 14 Mar 2026 11:51:29 -0700 Subject: [PATCH 05/10] code review --- .../ClrDataModule.cs | 5 +++-- .../Microsoft.Diagnostics.DataContractReader.Legacy.csproj | 3 +++ src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index c44812035889c9..c3cdf69a99489c 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -64,7 +64,7 @@ CustomQueryInterfaceResult ICustomQueryInterface.GetInterface(ref Guid iid, out return CustomQueryInterfaceResult.NotHandled; } - public sealed class EnumMethodDefinitions + internal sealed class EnumMethodDefinitions { private readonly uint _flags; private readonly MetadataReader _reader; @@ -305,13 +305,14 @@ int IXCLRDataModule.EnumMethodDefinitionByName(ulong* handle, DacComNullableByRe hr = HResults.S_FALSE; } } - catch (System.Exception) + catch (System.Exception ex) { // Fall back to the legacy DAC result when available, otherwise propagate the error. if (_legacyModule is not null) { method.Interface = legacyMethod; } + hr = ex.HResult; } #if DEBUG diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Microsoft.Diagnostics.DataContractReader.Legacy.csproj b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Microsoft.Diagnostics.DataContractReader.Legacy.csproj index 4eb5f58cc529dc..237437772770ab 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Microsoft.Diagnostics.DataContractReader.Legacy.csproj +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Microsoft.Diagnostics.DataContractReader.Legacy.csproj @@ -18,4 +18,7 @@ + + + diff --git a/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs b/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs index d56f16c449ac25..7f633e1b31f3f7 100644 --- a/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs +++ b/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs @@ -30,7 +30,7 @@ private static byte[] BuildTestMetadata() mdBuilder.AddModule( 0, mdBuilder.GetOrAddString("TestModule"), - mdBuilder.GetOrAddGuid(Guid.NewGuid()), + mdBuilder.GetOrAddGuid(Guid.Empty), default, default); mdBuilder.AddAssembly( From 40c8990e2a9ff44d9d86a8e77f1b15ceafa04c76 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Sat, 14 Mar 2026 13:35:10 -0700 Subject: [PATCH 06/10] removing unneeded internalsvisibleto --- .../cdac/mscordaccore_universal/mscordaccore_universal.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj index 35615c89eda9c6..1e9b9c83933ed7 100644 --- a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj +++ b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj @@ -23,9 +23,6 @@ - - - From 7931c1a0494ed9b86f4fe4395b4afbe062889c68 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Mon, 16 Mar 2026 14:20:44 -0700 Subject: [PATCH 07/10] code review, adding impl and enum interface --- .../ClrDataMethodDefinition.cs | 51 ++++++++++++++++++- .../ClrDataModule.cs | 12 ++--- .../IEnum.cs | 9 ++++ .../SOSDacImpl.IXCLRDataProcess.cs | 10 ++-- 4 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs index d269d804e4b98c..79d21e24a062b5 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs @@ -3,6 +3,7 @@ using System; using System.Runtime.InteropServices.Marshalling; +using System.Diagnostics; using Microsoft.Diagnostics.DataContractReader.Contracts; namespace Microsoft.Diagnostics.DataContractReader.Legacy; @@ -41,7 +42,55 @@ int IXCLRDataMethodDefinition.GetName(uint flags, uint bufLen, uint* nameLen, ch => _legacyImpl is not null ? _legacyImpl.GetName(flags, bufLen, nameLen, name) : HResults.E_NOTIMPL; int IXCLRDataMethodDefinition.GetTokenAndScope(uint* token, DacComNullableByRef mod) - => _legacyImpl is not null ? _legacyImpl.GetTokenAndScope(token, mod) : HResults.E_NOTIMPL; + { + int hr = HResults.S_OK; + try + { + if (token is not null) + { + *token = _token; + } + if (!mod.IsNullRef) + { + IXCLRDataModule? legacyMod = null; + if (_legacyImpl is not null) + { + DacComNullableByRef legacyModOut = new(isNullRef: false); + int hrLegacy = _legacyImpl.GetTokenAndScope(null, legacyModOut); + if (hrLegacy < 0) + return hrLegacy; + legacyMod = legacyModOut.Interface; + } + + mod.Interface = new ClrDataModule(_module, _target, legacyMod); + } + } + catch (System.Exception ex) + { + hr = ex.HResult; + } + +#if DEBUG + if (_legacyImpl is not null) + { + bool validateToken = token is not null; + bool validateMod = !mod.IsNullRef; + + uint tokenLocal = 0; + DacComNullableByRef legacyModOutLocal = new(isNullRef: !validateMod); + int hrLocal = _legacyImpl.GetTokenAndScope(validateToken ? &tokenLocal : null, legacyModOutLocal); + + Debug.ValidateHResult(hr, hrLocal); + + if (validateToken) + { + Debug.Assert(tokenLocal == *token, $"cDAC: {*token:x}, DAC: {tokenLocal:x}"); + } + } +#endif + + return hr; + } int IXCLRDataMethodDefinition.GetFlags(uint* flags) => _legacyImpl is not null ? _legacyImpl.GetFlags(flags) : HResults.E_NOTIMPL; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index c3cdf69a99489c..75cb9ba353b9db 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -64,13 +64,13 @@ CustomQueryInterfaceResult ICustomQueryInterface.GetInterface(ref Guid iid, out return CustomQueryInterfaceResult.NotHandled; } - internal sealed class EnumMethodDefinitions + internal sealed class EnumMethodDefinitions : IEnum { private readonly uint _flags; private readonly MetadataReader _reader; private TypeDefinitionHandle? _typeHandle; private string? _methodName; - public IEnumerator MethodEnumerator = Enumerable.Empty().GetEnumerator(); + public IEnumerator Enumerator { get; set; } = Enumerable.Empty().GetEnumerator(); public TargetPointer LegacyHandle { get; set; } = TargetPointer.Null; public EnumMethodDefinitions(MetadataReader reader, uint flags, TargetPointer legacyHandle) @@ -117,7 +117,7 @@ public void Start(string fullName) if (_typeHandle == null) throw new ArgumentException(); - MethodEnumerator = IterateMethodInstances().GetEnumerator(); + Enumerator = IterateMethodInstances().GetEnumerator(); } private bool StringEquals(string a, string b) @@ -295,9 +295,9 @@ int IXCLRDataModule.EnumMethodDefinitionByName(ulong* handle, DacComNullableByRe try { - if (emd.MethodEnumerator.MoveNext()) + if (emd.Enumerator.MoveNext()) { - uint token = emd.MethodEnumerator.Current; + uint token = emd.Enumerator.Current; method.Interface = new ClrDataMethodDefinition(_target, _address, token, legacyMethod); } else @@ -334,7 +334,7 @@ int IXCLRDataModule.EndEnumMethodDefinitionsByName(ulong handle) if (gcHandle.Target is not EnumMethodDefinitions emdLocal) throw new ArgumentException(); emd = emdLocal; - emd.MethodEnumerator.Dispose(); + emd.Enumerator.Dispose(); gcHandle.Free(); } catch (System.Exception ex) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs new file mode 100644 index 00000000000000..c79a74c9264977 --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs @@ -0,0 +1,9 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; +namespace Microsoft.Diagnostics.DataContractReader.Legacy; +public interface IEnum +{ + IEnumerator Enumerator { get; } + TargetPointer LegacyHandle { get; } +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs index bb76867fd06a14..718209b0ea79e4 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs @@ -136,7 +136,7 @@ int IXCLRDataProcess.EndEnumModules(ulong handle) int IXCLRDataProcess.GetModuleByAddress(ClrDataAddress address, DacComNullableByRef mod) => _legacyProcess is not null ? _legacyProcess.GetModuleByAddress(address, mod) : HResults.E_NOTIMPL; - internal sealed class EnumMethodInstances + internal sealed class EnumMethodInstances : IEnum { private readonly Target _target; private readonly TargetPointer _mainMethodDesc; @@ -144,7 +144,7 @@ internal sealed class EnumMethodInstances private readonly ILoader _loader; private readonly IRuntimeTypeSystem _rts; private readonly ICodeVersions _cv; - public IEnumerator methodEnumerator = Enumerable.Empty().GetEnumerator(); + public IEnumerator Enumerator { get; set; } = Enumerable.Empty().GetEnumerator(); public TargetPointer LegacyHandle { get; set; } = TargetPointer.Null; public EnumMethodInstances(Target target, TargetPointer methodDesc, TargetPointer appDomain) @@ -174,7 +174,7 @@ public int Start() return HResults.S_FALSE; } - methodEnumerator = IterateMethodInstances().GetEnumerator(); + Enumerator = IterateMethodInstances().GetEnumerator(); return HResults.S_OK; } @@ -402,9 +402,9 @@ int IXCLRDataProcess.EnumMethodInstanceByAddress(ulong* handle, DacComNullableBy try { - if (emi.methodEnumerator.MoveNext()) + if (emi.Enumerator.MoveNext()) { - MethodDescHandle methodDesc = emi.methodEnumerator.Current; + MethodDescHandle methodDesc = emi.Enumerator.Current; method.Interface = new ClrDataMethodInstance(_target, methodDesc, emi._appDomain, legacyMethod); } else From d9ea29e0251f77959e9b1065dad8f52225fcbb30 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Mon, 16 Mar 2026 14:31:44 -0700 Subject: [PATCH 08/10] code review --- .../ClrDataModule.cs | 4 ++-- .../Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs | 2 +- src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 75cb9ba353b9db..45db8dd25aa27c 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -117,7 +117,7 @@ public void Start(string fullName) if (_typeHandle == null) throw new ArgumentException(); - Enumerator = IterateMethodInstances().GetEnumerator(); + Enumerator = IterateMethodDefinitions().GetEnumerator(); } private bool StringEquals(string a, string b) @@ -126,7 +126,7 @@ private bool StringEquals(string a, string b) return string.Equals(a, b, comparison); } - private IEnumerable IterateMethodInstances() + private IEnumerable IterateMethodDefinitions() { foreach (MethodDefinitionHandle mh in _reader.GetTypeDefinition(_typeHandle!.Value).GetMethods()) { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs index c79a74c9264977..5c4a7beb912554 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs @@ -2,7 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; namespace Microsoft.Diagnostics.DataContractReader.Legacy; -public interface IEnum +internal interface IEnum { IEnumerator Enumerator { get; } TargetPointer LegacyHandle { get; } diff --git a/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs b/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs index 7f633e1b31f3f7..45cb8298ede8ff 100644 --- a/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs +++ b/src/native/managed/cdac/tests/EnumMethodDefinitionsTests.cs @@ -179,7 +179,7 @@ public void EnumMethodsByName_ReturnsExpectedCount(string fullName, uint flags, emd.Start(fullName); int count = 0; - while (emd.MethodEnumerator.MoveNext()) + while (emd.Enumerator.MoveNext()) count++; Assert.Equal(expectedCount, count); From 29084f39a274376a93a58e5acc0ff3913166356d Mon Sep 17 00:00:00 2001 From: rcj1 Date: Tue, 24 Mar 2026 15:46:11 -0700 Subject: [PATCH 09/10] code review --- .../ClrDataModule.cs | 10 ++-------- .../IEnum.cs | 8 ++++++++ .../IXCLRData.cs | 4 ++-- .../SOSDacImpl.IXCLRDataProcess.cs | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs index 45db8dd25aa27c..3986b59a2948b8 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -245,8 +245,7 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo EnumMethodDefinitions emd = new(reader, flags, handleLocal); emd.Start(fullName); - GCHandle gcHandle = GCHandle.Alloc(emd); - *handle = (ulong)GCHandle.ToIntPtr(gcHandle).ToInt64(); + *handle = (ulong)((IEnum)emd).GetHandle(); } catch (System.Exception ex) { @@ -307,11 +306,6 @@ int IXCLRDataModule.EnumMethodDefinitionByName(ulong* handle, DacComNullableByRe } catch (System.Exception ex) { - // Fall back to the legacy DAC result when available, otherwise propagate the error. - if (_legacyModule is not null) - { - method.Interface = legacyMethod; - } hr = ex.HResult; } @@ -334,7 +328,7 @@ int IXCLRDataModule.EndEnumMethodDefinitionsByName(ulong handle) if (gcHandle.Target is not EnumMethodDefinitions emdLocal) throw new ArgumentException(); emd = emdLocal; - emd.Enumerator.Dispose(); + ((IEnum)emd).Dispose(); gcHandle.Free(); } catch (System.Exception ex) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs index 5c4a7beb912554..11fe6885a8e29c 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IEnum.cs @@ -1,9 +1,17 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Runtime.InteropServices; namespace Microsoft.Diagnostics.DataContractReader.Legacy; + internal interface IEnum { IEnumerator Enumerator { get; } TargetPointer LegacyHandle { get; } + void Dispose() => Enumerator.Dispose(); + long GetHandle() + { + GCHandle gcHandle = GCHandle.Alloc(this); + return GCHandle.ToIntPtr(gcHandle).ToInt64(); + } } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs index 56341e9e655766..1312c2681a9a2a 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; @@ -32,7 +32,7 @@ public struct DacpGetModuleData public enum CLRDataByNameFlag : uint { CLRDATA_BYNAME_CASE_SENSITIVE = 0, - CLRDATA_BYNAME_CASE_INSENSITIVE = 0x1 + CLRDATA_BYNAME_CASE_INSENSITIVE = 1 } [GeneratedComInterface] diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs index 718209b0ea79e4..ed4bf66bd41ddd 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs @@ -362,7 +362,7 @@ int IXCLRDataProcess.StartEnumMethodInstancesByAddress(ClrDataAddress address, I emi.LegacyHandle = handleLocal; GCHandle gcHandle = GCHandle.Alloc(emi); - *handle = (ulong)GCHandle.ToIntPtr(gcHandle).ToInt64(); + *handle = (ulong)((IEnum)emi).GetHandle(); hr = emi.Start(); } } From 16f190488708aa86f2a0d7e2033d329c20c98cc1 Mon Sep 17 00:00:00 2001 From: Rachel Date: Tue, 24 Mar 2026 16:07:08 -0700 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../IXCLRData.cs | 2 +- .../SOSDacImpl.IXCLRDataProcess.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs index 1312c2681a9a2a..d95f7da708f15f 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs index ed4bf66bd41ddd..7ce1cc45883939 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs @@ -361,7 +361,6 @@ int IXCLRDataProcess.StartEnumMethodInstancesByAddress(ClrDataAddress address, I EnumMethodInstances emi = new(_target, methodDesc, TargetPointer.Null); emi.LegacyHandle = handleLocal; - GCHandle gcHandle = GCHandle.Alloc(emi); *handle = (ulong)((IEnum)emi).GetHandle(); hr = emi.Start(); }