From 7aaba05f4aa4d48cb32c0faee0ded95da1e2abe8 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Thu, 16 Jul 2026 14:25:08 -0400 Subject: [PATCH 1/5] [cDAC] Return E_FAIL when module metadata is unavailable When a module's metadata is unavailable, several ISOS/IXCLRData/DacDbi COM methods relied on a null-forgiving MetadataReader (GetMetadata(...)!), which surfaced as a NullReferenceException (E_POINTER) rather than the E_FAIL the native DAC returns. Return E_FAIL at the high-confidence COM-boundary sites where the enclosing method already catches exceptions and returns ex.HResult, and where the native DAC returns E_FAIL when metadata is unavailable: - IXCLRDataModule.StartEnumMethodDefinitionsByName - ISOSDacInterface.GetFieldDescData - DacDbi GetObjectFields (enclosing-class metadata resolution) The metadata read now throws Marshal.GetExceptionForHR(HResults.E_FAIL) when GetMetadata returns null. Signature formatting / type-name building (SigFormat, TypeNameBuilder), the ClrDataFrame local-signature reader (whose null result is converted to a DEFAULT value flag), and the contract-layer metadata reads are intentionally left unchanged to keep this change to well-understood COM boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2aa5dc4-0c99-43f0-af14-734143849c4a --- .../ClrDataModule.cs | 3 +- .../Dbi/DacDbiImpl.cs | 3 +- .../SOSDacImpl.cs | 3 +- .../UnitTests/ClrDataModuleMetadataTests.cs | 42 +++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.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 6e194d4b19155c..2bc05918d619b7 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -298,7 +298,8 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo // start: find the type. ILoader loader = _target.Contracts.Loader; Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_address); - MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!; + MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; EnumMethodDefinitions emd = new(reader, flags, (nuint)handleLocal); emd.Start(fullName); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 0481f2c9bd129e..4c687050621112 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -4770,7 +4770,8 @@ public int GetObjectFields(ulong id, uint celt, COR_FIELD* layout, uint* pceltFe TypeHandle enclosingTypeHandle = rts.GetTypeHandle(enclosingMT); TargetPointer enclosingModulePtr = rts.GetModule(enclosingTypeHandle); Contracts.ModuleHandle enclosingModuleHandle = _target.Contracts.Loader.GetModuleHandleFromModulePtr(enclosingModulePtr); - MetadataReader enclosingMdReader = ecmaMetadataContract.GetMetadata(enclosingModuleHandle)!; + MetadataReader enclosingMdReader = ecmaMetadataContract.GetMetadata(enclosingModuleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; FieldDefinitionHandle fieldDefHandle = (FieldDefinitionHandle)MetadataTokens.Handle((int)memberDef); FieldDefinition fieldDef = enclosingMdReader.GetFieldDefinition(fieldDefHandle); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index 03576016dfc336..46e992caa35f91 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -1070,7 +1070,8 @@ int ISOSDacInterface.GetFieldDescData(ClrDataAddress fieldDesc, DacpFieldDescDat TypeHandle ctx = rtsContract.GetTypeHandle(enclosingMT); TargetPointer modulePtr = rtsContract.GetModule(ctx); Contracts.ModuleHandle moduleHandle = _target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePtr); - MetadataReader mdReader = ecmaMetadataContract.GetMetadata(moduleHandle)!; + MetadataReader mdReader = ecmaMetadataContract.GetMetadata(moduleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; FieldDefinition fieldDef = mdReader.GetFieldDefinition(fieldHandle); TypeHandle foundTypeHandle = rtsContract.GetFieldDescApproxTypeHandle(fieldDescTargetPtr); diff --git a/src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs b/src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs new file mode 100644 index 00000000000000..3e1ccba0aeb530 --- /dev/null +++ b/src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs @@ -0,0 +1,42 @@ +// 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.Metadata; +using Microsoft.Diagnostics.DataContractReader.Contracts; +using Microsoft.Diagnostics.DataContractReader.Legacy; +using Microsoft.Diagnostics.DataContractReader.TestInfrastructure; +using Moq; +using Xunit; +using ModuleHandle = Microsoft.Diagnostics.DataContractReader.Contracts.ModuleHandle; + +namespace Microsoft.Diagnostics.DataContractReader.Tests; + +public unsafe class ClrDataModuleMetadataTests +{ + private static readonly MockTarget.Architecture s_arch = new() { IsLittleEndian = true, Is64Bit = true }; + private static readonly TargetPointer s_modulePointer = new(0x1000); + private static readonly ModuleHandle s_moduleHandle = new(new TargetPointer(0x2000)); + + [Fact] + public void StartEnumMethodDefinitionsByName_MissingMetadataReturnsEFail() + { + var loader = new Mock(); + loader.Setup(l => l.GetModuleHandleFromModulePtr(s_modulePointer)).Returns(s_moduleHandle); + var ecmaMetadata = new Mock(); + ecmaMetadata.Setup(e => e.GetMetadata(s_moduleHandle)).Returns((MetadataReader?)null); + + TestPlaceholderTarget target = new TestPlaceholderTarget.Builder(s_arch) + .UseReader((ulong _, Span _) => -1) + .AddMockContract(loader) + .AddMockContract(ecmaMetadata) + .Build(); + IXCLRDataModule module = new ClrDataModule(s_modulePointer, target, legacyImpl: null); + + ulong handle; + fixed (char* name = "Foo") + { + Assert.Equal(HResults.E_FAIL, module.StartEnumMethodDefinitionsByName(name, 0, &handle)); + } + } +} From af949172f9a50680ba180752f57f07b1977cfcfa Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Fri, 17 Jul 2026 11:26:48 -0400 Subject: [PATCH 2/5] Handle unavailable metadata consistently Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2aa5dc4-0c99-43f0-af14-734143849c4a --- .../ClrDataFrame.cs | 9 ++-- .../ClrDataMethodDefinition.cs | 2 +- .../Dbi/DacDbiImpl.cs | 2 +- .../SigFormat.cs | 10 +++-- .../TypeNameBuilder.cs | 13 ++++-- .../UnitTests/ClrDataModuleMetadataTests.cs | 42 ------------------- 6 files changed, 24 insertions(+), 54 deletions(-) delete mode 100644 src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs index 1052e9519020cd..a8ab53555eae11 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs @@ -536,7 +536,8 @@ private ClrDataValue CreateValueFromDebugInfo( return null; } - mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!; + mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; StandaloneSignatureHandle localSigHandle = MetadataTokens.StandaloneSignatureHandle(localToken); BlobHandle localSigBlob = mdReader.GetStandaloneSignature(localSigHandle).Signature; return mdReader.GetBlobReader(localSigBlob); @@ -571,7 +572,8 @@ private uint GetLocalVariableCount(MethodDescHandle mdh, Contracts.ModuleHandle { try { - MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) ?? throw new NotImplementedException(); + MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; uint token = _target.Contracts.RuntimeTypeSystem.GetMethodToken(mdh); MethodDefinition methodDef = mdReader.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle((int)EcmaMetadataUtils.GetRowId(token))); FlagSignatureTypeProvider provider = new(_target, moduleHandle); @@ -789,7 +791,8 @@ private static (uint Flags, int Size) CheckEnumFromTypeDef(MetadataReader reader { // For TypeRefs, try to resolve in the same module's TypeDef table. TypeReference typeRef = reader.GetTypeReference(handle); - MetadataReader moduleReader = _target.Contracts.EcmaMetadata.GetMetadata(_moduleHandle)!; + MetadataReader moduleReader = _target.Contracts.EcmaMetadata.GetMetadata(_moduleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; foreach (TypeDefinitionHandle tdh in moduleReader.TypeDefinitions) { TypeDefinition td = moduleReader.GetTypeDefinition(tdh); 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 0fe4e2641d91b0..60bb12fdab754d 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs @@ -70,7 +70,7 @@ private string GetFullMethodNameFromMetadata() Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_module); IEcmaMetadata ecmaMetadata = _target.Contracts.EcmaMetadata; MetadataReader reader = ecmaMetadata.GetMetadata(moduleHandle) - ?? throw new InvalidOperationException("Failed to get metadata reader"); + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; int rowId = (int)(_token & 0x00FFFFFF); MethodDefinitionHandle methodDefHandle = MetadataTokens.MethodDefinitionHandle(rowId); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 4c687050621112..81073f91a2df16 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -2325,7 +2325,7 @@ public int GetILCodeAndSig(ulong vmAssembly, uint functionToken, DacDbiTargetBuf Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromAssemblyPtr(new TargetPointer(vmAssembly)); MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) - ?? throw new InvalidOperationException("Module has no metadata."); + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; MethodDefinitionHandle mdMethodHandle = MetadataTokens.MethodDefinitionHandle((int)EcmaMetadataUtils.GetRowId(functionToken)); MethodDefinition methodDef = mdReader.GetMethodDefinition(mdMethodHandle); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs index 6e41501fab87f7..1aa7337ba6eada 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs @@ -4,6 +4,7 @@ using System; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; +using System.Runtime.InteropServices; using System.Text; using Microsoft.Diagnostics.DataContractReader.Contracts; @@ -180,7 +181,8 @@ private static unsafe void AddTypeString(Target target, uint typeDefToken = runtimeTypeSystem.GetTypeDefToken(th); TargetPointer modulePointer = target.Contracts.RuntimeTypeSystem.GetModule(th); Contracts.ModuleHandle module = target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePointer); - MetadataReader internalTypeMetadata = target.Contracts.EcmaMetadata.GetMetadata(module)!; + MetadataReader internalTypeMetadata = target.Contracts.EcmaMetadata.GetMetadata(module) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; TypeDefinition internalTypeDef = internalTypeMetadata.GetTypeDefinition((TypeDefinitionHandle)MetadataTokens.Handle((int)typeDefToken)); _namespace = internalTypeMetadata.GetString(internalTypeDef.Namespace); @@ -346,7 +348,8 @@ private static void AddType(Target target, StringBuilder stringBuilder, TypeHand uint typeDefToken = runtimeTypeSystem.GetTypeDefToken(typeHandle); TargetPointer modulePointer = target.Contracts.RuntimeTypeSystem.GetModule(typeHandle); Contracts.ModuleHandle module = target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePointer); - MetadataReader metadata = target.Contracts.EcmaMetadata.GetMetadata(module)!; + MetadataReader metadata = target.Contracts.EcmaMetadata.GetMetadata(module) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; TypeDefinition typeDef = metadata.GetTypeDefinition((TypeDefinitionHandle)MetadataTokens.Handle((int)typeDefToken)); string _namespace = metadata.GetString(typeDef.Namespace); string name = metadata.GetString(typeDef.Name); @@ -391,7 +394,8 @@ private static void AddType(Target target, StringBuilder stringBuilder, TypeHand case CorElementType.Var: runtimeTypeSystem.IsGenericVariable(typeHandle, out TargetPointer genericVariableModulePointer, out uint typeVarToken); Contracts.ModuleHandle genericVariableModule = target.Contracts.Loader.GetModuleHandleFromModulePtr(genericVariableModulePointer); - MetadataReader generatedVariableMetadata = target.Contracts.EcmaMetadata.GetMetadata(genericVariableModule)!; + MetadataReader generatedVariableMetadata = target.Contracts.EcmaMetadata.GetMetadata(genericVariableModule) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; GenericParameter genericVariable = generatedVariableMetadata.GetGenericParameter((GenericParameterHandle)MetadataTokens.Handle((int)typeVarToken)); stringBuilder.Append(generatedVariableMetadata.GetString(genericVariable.Name)); return; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs index e481d0cc471fe3..4a01096298f73b 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; +using System.Runtime.InteropServices; using System.Text; using Microsoft.Diagnostics.DataContractReader; using Microsoft.Diagnostics.DataContractReader.Contracts; @@ -123,7 +124,8 @@ public static void AppendMethodImpl(Target target, StringBuilder stringBuilder, if (rowId != 0) { Contracts.ModuleHandle module = loader.GetModuleHandleFromModulePtr(runtimeTypeSystem.GetModule(th)); - MetadataReader reader = target.Contracts.EcmaMetadata.GetMetadata(module)!; + MetadataReader reader = target.Contracts.EcmaMetadata.GetMetadata(module) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; MethodDefinition methodDef = reader.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle((int)rowId)); stringBuilder.Append(reader.GetString(methodDef.Name)); } @@ -224,7 +226,8 @@ private static void AppendTypeCore(ref TypeNameBuilder tnb, Contracts.TypeHandle else if (typeSystemContract.IsGenericVariable(typeHandle, out TargetPointer modulePointer, out uint genericParamToken)) { Contracts.ModuleHandle module = tnb.Target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePointer); - MetadataReader reader = tnb.Target.Contracts.EcmaMetadata.GetMetadata(module)!; + MetadataReader reader = tnb.Target.Contracts.EcmaMetadata.GetMetadata(module) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; var handle = (GenericParameterHandle)MetadataTokens.Handle((int)genericParamToken); GenericParameter genericParam = reader.GetGenericParameter(handle); if (format.HasFlag(TypeNameFormat.FormatGenericParam)) @@ -291,7 +294,8 @@ private static void AppendTypeCore(ref TypeNameBuilder tnb, Contracts.TypeHandle } else { - MetadataReader reader = tnb.Target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!; + MetadataReader reader = tnb.Target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; AppendNestedTypeDef(ref tnb, reader, (TypeDefinitionHandle)MetadataTokens.EntityHandle((int)typeDefToken), format); } @@ -318,7 +322,8 @@ private static void AppendTypeCore(ref TypeNameBuilder tnb, Contracts.TypeHandle Contracts.ModuleHandle module = tnb.Target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePtr); // NOTE: The DAC variant of assembly name generation is different than the runtime version. The DAC variant is simpler, and only uses SimpleName - MetadataReader mr = tnb.Target.Contracts.EcmaMetadata.GetMetadata(module)!; + MetadataReader mr = tnb.Target.Contracts.EcmaMetadata.GetMetadata(module) + ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; string assemblySimpleName = mr.GetString(mr.GetAssemblyDefinition().Name); tnb.AddAssemblySpec(assemblySimpleName); diff --git a/src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs b/src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs deleted file mode 100644 index 3e1ccba0aeb530..00000000000000 --- a/src/native/managed/cdac/tests/UnitTests/ClrDataModuleMetadataTests.cs +++ /dev/null @@ -1,42 +0,0 @@ -// 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.Metadata; -using Microsoft.Diagnostics.DataContractReader.Contracts; -using Microsoft.Diagnostics.DataContractReader.Legacy; -using Microsoft.Diagnostics.DataContractReader.TestInfrastructure; -using Moq; -using Xunit; -using ModuleHandle = Microsoft.Diagnostics.DataContractReader.Contracts.ModuleHandle; - -namespace Microsoft.Diagnostics.DataContractReader.Tests; - -public unsafe class ClrDataModuleMetadataTests -{ - private static readonly MockTarget.Architecture s_arch = new() { IsLittleEndian = true, Is64Bit = true }; - private static readonly TargetPointer s_modulePointer = new(0x1000); - private static readonly ModuleHandle s_moduleHandle = new(new TargetPointer(0x2000)); - - [Fact] - public void StartEnumMethodDefinitionsByName_MissingMetadataReturnsEFail() - { - var loader = new Mock(); - loader.Setup(l => l.GetModuleHandleFromModulePtr(s_modulePointer)).Returns(s_moduleHandle); - var ecmaMetadata = new Mock(); - ecmaMetadata.Setup(e => e.GetMetadata(s_moduleHandle)).Returns((MetadataReader?)null); - - TestPlaceholderTarget target = new TestPlaceholderTarget.Builder(s_arch) - .UseReader((ulong _, Span _) => -1) - .AddMockContract(loader) - .AddMockContract(ecmaMetadata) - .Build(); - IXCLRDataModule module = new ClrDataModule(s_modulePointer, target, legacyImpl: null); - - ulong handle; - fixed (char* name = "Foo") - { - Assert.Equal(HResults.E_FAIL, module.StartEnumMethodDefinitionsByName(name, 0, &handle)); - } - } -} From 6770fb164bd159374dceadb2a4434a4f5c4cba99 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Fri, 17 Jul 2026 11:38:17 -0400 Subject: [PATCH 3/5] Preserve existing metadata exceptions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2aa5dc4-0c99-43f0-af14-734143849c4a --- .../ClrDataFrame.cs | 3 +-- .../ClrDataMethodDefinition.cs | 2 +- .../Dbi/DacDbiImpl.cs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs index a8ab53555eae11..1020348ac5d146 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs @@ -572,8 +572,7 @@ private uint GetLocalVariableCount(MethodDescHandle mdh, Contracts.ModuleHandle { try { - MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) ?? throw new NotImplementedException(); uint token = _target.Contracts.RuntimeTypeSystem.GetMethodToken(mdh); MethodDefinition methodDef = mdReader.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle((int)EcmaMetadataUtils.GetRowId(token))); FlagSignatureTypeProvider provider = new(_target, moduleHandle); 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 60bb12fdab754d..0fe4e2641d91b0 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs @@ -70,7 +70,7 @@ private string GetFullMethodNameFromMetadata() Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_module); IEcmaMetadata ecmaMetadata = _target.Contracts.EcmaMetadata; MetadataReader reader = ecmaMetadata.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Failed to get metadata reader"); int rowId = (int)(_token & 0x00FFFFFF); MethodDefinitionHandle methodDefHandle = MetadataTokens.MethodDefinitionHandle(rowId); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 81073f91a2df16..4c687050621112 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -2325,7 +2325,7 @@ public int GetILCodeAndSig(ulong vmAssembly, uint functionToken, DacDbiTargetBuf Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromAssemblyPtr(new TargetPointer(vmAssembly)); MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); MethodDefinitionHandle mdMethodHandle = MetadataTokens.MethodDefinitionHandle((int)EcmaMetadataUtils.GetRowId(functionToken)); MethodDefinition methodDef = mdReader.GetMethodDefinition(mdMethodHandle); From cd7d839f94f8ed619cddb99955c81c4bc1e5f291 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Fri, 17 Jul 2026 11:50:47 -0400 Subject: [PATCH 4/5] Clarify unavailable metadata failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2aa5dc4-0c99-43f0-af14-734143849c4a --- .../ClrDataFrame.cs | 2 +- .../ClrDataModule.cs | 2 +- .../Dbi/DacDbiImpl.cs | 2 +- .../SOSDacImpl.cs | 2 +- .../SigFormat.cs | 7 +++---- .../TypeNameBuilder.cs | 9 ++++----- 6 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs index 1020348ac5d146..fc69b2fba6a15b 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs @@ -537,7 +537,7 @@ private ClrDataValue CreateValueFromDebugInfo( } mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); StandaloneSignatureHandle localSigHandle = MetadataTokens.StandaloneSignatureHandle(localToken); BlobHandle localSigBlob = mdReader.GetStandaloneSignature(localSigHandle).Signature; return mdReader.GetBlobReader(localSigBlob); 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 2bc05918d619b7..565092b4b60531 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -299,7 +299,7 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo ILoader loader = _target.Contracts.Loader; Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(_address); MetadataReader reader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); EnumMethodDefinitions emd = new(reader, flags, (nuint)handleLocal); emd.Start(fullName); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 4c687050621112..87b4e10672136e 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -4771,7 +4771,7 @@ public int GetObjectFields(ulong id, uint celt, COR_FIELD* layout, uint* pceltFe TargetPointer enclosingModulePtr = rts.GetModule(enclosingTypeHandle); Contracts.ModuleHandle enclosingModuleHandle = _target.Contracts.Loader.GetModuleHandleFromModulePtr(enclosingModulePtr); MetadataReader enclosingMdReader = ecmaMetadataContract.GetMetadata(enclosingModuleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); FieldDefinitionHandle fieldDefHandle = (FieldDefinitionHandle)MetadataTokens.Handle((int)memberDef); FieldDefinition fieldDef = enclosingMdReader.GetFieldDefinition(fieldDefHandle); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index 46e992caa35f91..0be9f10f0408cb 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -1071,7 +1071,7 @@ int ISOSDacInterface.GetFieldDescData(ClrDataAddress fieldDesc, DacpFieldDescDat TargetPointer modulePtr = rtsContract.GetModule(ctx); Contracts.ModuleHandle moduleHandle = _target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePtr); MetadataReader mdReader = ecmaMetadataContract.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); FieldDefinition fieldDef = mdReader.GetFieldDefinition(fieldHandle); TypeHandle foundTypeHandle = rtsContract.GetFieldDescApproxTypeHandle(fieldDescTargetPtr); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs index 1aa7337ba6eada..710c91149ae1dd 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs @@ -4,7 +4,6 @@ using System; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; -using System.Runtime.InteropServices; using System.Text; using Microsoft.Diagnostics.DataContractReader.Contracts; @@ -182,7 +181,7 @@ private static unsafe void AddTypeString(Target target, TargetPointer modulePointer = target.Contracts.RuntimeTypeSystem.GetModule(th); Contracts.ModuleHandle module = target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePointer); MetadataReader internalTypeMetadata = target.Contracts.EcmaMetadata.GetMetadata(module) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); TypeDefinition internalTypeDef = internalTypeMetadata.GetTypeDefinition((TypeDefinitionHandle)MetadataTokens.Handle((int)typeDefToken)); _namespace = internalTypeMetadata.GetString(internalTypeDef.Namespace); @@ -349,7 +348,7 @@ private static void AddType(Target target, StringBuilder stringBuilder, TypeHand TargetPointer modulePointer = target.Contracts.RuntimeTypeSystem.GetModule(typeHandle); Contracts.ModuleHandle module = target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePointer); MetadataReader metadata = target.Contracts.EcmaMetadata.GetMetadata(module) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); TypeDefinition typeDef = metadata.GetTypeDefinition((TypeDefinitionHandle)MetadataTokens.Handle((int)typeDefToken)); string _namespace = metadata.GetString(typeDef.Namespace); string name = metadata.GetString(typeDef.Name); @@ -395,7 +394,7 @@ private static void AddType(Target target, StringBuilder stringBuilder, TypeHand runtimeTypeSystem.IsGenericVariable(typeHandle, out TargetPointer genericVariableModulePointer, out uint typeVarToken); Contracts.ModuleHandle genericVariableModule = target.Contracts.Loader.GetModuleHandleFromModulePtr(genericVariableModulePointer); MetadataReader generatedVariableMetadata = target.Contracts.EcmaMetadata.GetMetadata(genericVariableModule) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); GenericParameter genericVariable = generatedVariableMetadata.GetGenericParameter((GenericParameterHandle)MetadataTokens.Handle((int)typeVarToken)); stringBuilder.Append(generatedVariableMetadata.GetString(genericVariable.Name)); return; diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs index 4a01096298f73b..1b8b31cfd25166 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; -using System.Runtime.InteropServices; using System.Text; using Microsoft.Diagnostics.DataContractReader; using Microsoft.Diagnostics.DataContractReader.Contracts; @@ -125,7 +124,7 @@ public static void AppendMethodImpl(Target target, StringBuilder stringBuilder, { Contracts.ModuleHandle module = loader.GetModuleHandleFromModulePtr(runtimeTypeSystem.GetModule(th)); MetadataReader reader = target.Contracts.EcmaMetadata.GetMetadata(module) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); MethodDefinition methodDef = reader.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle((int)rowId)); stringBuilder.Append(reader.GetString(methodDef.Name)); } @@ -227,7 +226,7 @@ private static void AppendTypeCore(ref TypeNameBuilder tnb, Contracts.TypeHandle { Contracts.ModuleHandle module = tnb.Target.Contracts.Loader.GetModuleHandleFromModulePtr(modulePointer); MetadataReader reader = tnb.Target.Contracts.EcmaMetadata.GetMetadata(module) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); var handle = (GenericParameterHandle)MetadataTokens.Handle((int)genericParamToken); GenericParameter genericParam = reader.GetGenericParameter(handle); if (format.HasFlag(TypeNameFormat.FormatGenericParam)) @@ -295,7 +294,7 @@ private static void AppendTypeCore(ref TypeNameBuilder tnb, Contracts.TypeHandle else { MetadataReader reader = tnb.Target.Contracts.EcmaMetadata.GetMetadata(moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); AppendNestedTypeDef(ref tnb, reader, (TypeDefinitionHandle)MetadataTokens.EntityHandle((int)typeDefToken), format); } @@ -323,7 +322,7 @@ private static void AppendTypeCore(ref TypeNameBuilder tnb, Contracts.TypeHandle // NOTE: The DAC variant of assembly name generation is different than the runtime version. The DAC variant is simpler, and only uses SimpleName MetadataReader mr = tnb.Target.Contracts.EcmaMetadata.GetMetadata(module) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); string assemblySimpleName = mr.GetString(mr.GetAssemblyDefinition().Name); tnb.AddAssemblySpec(assemblySimpleName); From 885848fa1b883ac1b25b90fbb38879b531add0b7 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Mon, 20 Jul 2026 12:20:37 -0400 Subject: [PATCH 5/5] Address Copilot review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2aa5dc4-0c99-43f0-af14-734143849c4a --- .../ClrDataFrame.cs | 2 +- .../ClrDataModule.cs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs index fc69b2fba6a15b..579f74d2f6eda1 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs @@ -791,7 +791,7 @@ private static (uint Flags, int Size) CheckEnumFromTypeDef(MetadataReader reader // For TypeRefs, try to resolve in the same module's TypeDef table. TypeReference typeRef = reader.GetTypeReference(handle); MetadataReader moduleReader = _target.Contracts.EcmaMetadata.GetMetadata(_moduleHandle) - ?? throw Marshal.GetExceptionForHR(HResults.E_FAIL)!; + ?? throw new InvalidOperationException("Module has no metadata."); foreach (TypeDefinitionHandle tdh in moduleReader.TypeDefinitions) { TypeDefinition td = moduleReader.GetTypeDefinition(tdh); 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 565092b4b60531..63fa9c372a7892 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs @@ -304,11 +304,22 @@ int IXCLRDataModule.StartEnumMethodDefinitionsByName(char* name, uint flags, ulo EnumMethodDefinitions emd = new(reader, flags, (nuint)handleLocal); emd.Start(fullName); *handle = (ulong)((IEnum)emd).GetHandle(); + // Legacy handle ownership transferred to emd. + handleLocal = default; } catch (System.Exception ex) { hr = ex.HResult; } + finally + { + // The legacy enumeration is started before the cDAC work. If that work fails, + // the caller receives a null handle and cannot end the legacy enumeration. + if (_legacyModule is not null && handleLocal != default) + { + _legacyModule.EndEnumMethodDefinitionsByName(handleLocal); + } + } #if DEBUG if (_legacyModule is not null)