From d113a5cb1665ea845027361ecde542df1b0a4ff3 Mon Sep 17 00:00:00 2001 From: Noah Falk Date: Wed, 10 Jun 2026 03:24:30 -0700 Subject: [PATCH 1/5] WIP Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ContractNotAvailableException.cs | 98 +++++++++++++++ .../ContractRegistry.cs | 16 ++- .../CachingContractRegistry.cs | 16 ++- .../ContractDescriptorTarget.cs | 101 ++++++++-------- .../mscordaccore_universal/Entrypoints.cs | 15 +-- .../managed/cdac/scripts/DumpHelpers.cs | 23 ++-- .../cdac/tests/DataGenerator/TestTarget.cs | 9 +- .../ContractDescriptorBuilder.cs | 78 +++++++++++- .../tests/TestInfrastructure/DumpTestBase.cs | 14 +-- .../TestPlaceholderTarget.cs | 14 ++- .../ContractDescriptor/TargetTests.cs | 113 ++++++++++++++++++ 11 files changed, 391 insertions(+), 106 deletions(-) create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs new file mode 100644 index 00000000000000..036fd6108963c4 --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs @@ -0,0 +1,98 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.Diagnostics.DataContractReader; + +/// +/// Base exception for failures to retrieve a data contract from a target. +/// +public abstract class ContractNotAvailableException : Exception +{ + private const int E_NOTIMPL = unchecked((int)0x80004001); + + /// + /// Initializes a new instance of the class. + /// + /// The name of the requested contract. + /// The target-advertised version of the requested contract, or if the target did not advertise the contract. + /// The exception message. + protected ContractNotAvailableException(string contractName, string? contractVersion, string message) + : base(message) + { + ContractName = contractName; + ContractVersion = contractVersion; + HResult = E_NOTIMPL; + } + + /// + /// Gets the name of the requested contract. + /// + public string ContractName { get; } + + /// + /// Gets the target-advertised version of the requested contract, or if the target did not advertise the contract. + /// + public string? ContractVersion { get; } +} + +/// +/// Exception thrown when the target's contract descriptor does not advertise the requested contract. +/// +public sealed class ContractMissingException : ContractNotAvailableException +{ + /// + /// Initializes a new instance of the class. + /// + /// The name of the requested contract. + public ContractMissingException(string contractName) + : base(contractName, null, $"Contract '{contractName}' is not advertised by the target.") + { } +} + +/// +/// Exception thrown when the target advertises the requested contract, but this cDAC cannot provide an implementation for the advertised version. +/// +public abstract class ContractUnsupportedException : ContractNotAvailableException +{ + /// + /// Initializes a new instance of the class. + /// + /// The name of the requested contract. + /// The target-advertised version of the requested contract. + /// The exception message. + protected ContractUnsupportedException(string contractName, string contractVersion, string message) + : base(contractName, contractVersion, message) + { } +} + +/// +/// Exception thrown when the target advertises a contract version that this cDAC does not recognize, typically because the target runtime is newer than this cDAC. +/// +public sealed class ContractUnrecognizedException : ContractUnsupportedException +{ + /// + /// Initializes a new instance of the class. + /// + /// The name of the requested contract. + /// The target-advertised version of the requested contract. + public ContractUnrecognizedException(string contractName, string contractVersion) + : base(contractName, contractVersion, $"Contract '{contractName}' version {contractVersion} is advertised by the target but is not recognized by this cDAC.") + { } +} + +/// +/// Exception thrown when the target advertises a contract version that this cDAC recognizes but intentionally does not implement, typically because the target runtime is too old. +/// +public sealed class ContractObsoleteException : ContractUnsupportedException +{ + /// + /// Initializes a new instance of the class. + /// + /// The name of the requested contract. + /// The target-advertised version of the requested contract. + public ContractObsoleteException(string contractName, string contractVersion) + : base(contractName, contractVersion, $"Contract '{contractName}' version {contractVersion} is advertised by the target and recognized by this cDAC, but is intentionally not implemented.") + { } +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs index 0c2e0aa833e760..27003d9378cc89 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs @@ -157,19 +157,19 @@ public abstract class ContractRegistry /// /// When this method returns , contains the requested contract instance; otherwise, . /// - /// - /// When this method returns , contains a human-readable explanation of why the contract could not be retrieved; otherwise, . + /// + /// When this method returns , contains the exception that describes why the contract could not be retrieved; otherwise, . /// /// /// if the requested contract is present and was retrieved successfully; if the contract is not present or registered"/>. /// - public abstract bool TryGetContract([NotNullWhen(true)] out TContract contract, out string? failureReason) where TContract : IContract; + public abstract bool TryGetContract([NotNullWhen(true)] out TContract contract, [NotNullWhen(false)] out System.Exception? failureException) where TContract : IContract; public TContract GetContract() where TContract : IContract { - if (!TryGetContract(out TContract contract, out string? failureReason)) + if (!TryGetContract(out TContract contract, out System.Exception? failureException)) { - throw new NotImplementedException($"Contract '{typeof(TContract).Name}' is not supported by the target. Reason: {failureReason ?? "no reason provided"}"); + throw failureException ?? new ContractMissingException(TContract.Name); } return contract; } @@ -186,6 +186,12 @@ public bool TryGetContract([NotNullWhen(true)] out TContract contract public abstract void Register(string version, Func creator) where TContract : IContract; + /// + /// Register a contract version that is recognized but intentionally not implemented. + /// + public abstract void RegisterUnsupported(string version) + where TContract : IContract; + /// /// Flush all cached data held by contracts in this registry for the given /// . Called when the target process state may have changed diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs index 4ca7582501f6b5..b5d3618a5936f0 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs @@ -19,6 +19,7 @@ internal sealed class CachingContractRegistry : ContractRegistry private readonly Dictionary _contracts = []; private readonly Dictionary<(Type, string), Func> _creators = []; + private readonly HashSet<(Type, string)> _unsupportedVersions = []; private readonly Target _target; private readonly TryGetContractVersionDelegate _tryGetContractVersion; @@ -38,10 +39,15 @@ public override void Register(string version, Func _creators[(typeof(TContract), version)] = t => creator(t); } - public override bool TryGetContract([NotNullWhen(true)] out TContract contract, out string? failureReason) + public override void RegisterUnsupported(string version) + { + _unsupportedVersions.Add((typeof(TContract), version)); + } + + public override bool TryGetContract([NotNullWhen(true)] out TContract contract, [NotNullWhen(false)] out System.Exception? failureException) { contract = default!; - failureReason = null; + failureException = null; if (_contracts.TryGetValue(typeof(TContract), out IContract? cached)) { contract = (TContract)cached; @@ -53,13 +59,15 @@ public override bool TryGetContract([NotNullWhen(true)] out TContract { if (!_creators.TryGetValue((typeof(TContract), version), out creator)) { - failureReason = $"Target supports contract '{typeof(TContract).Name}' version {version}, but no implementation is registered for that version."; + failureException = _unsupportedVersions.Contains((typeof(TContract), version)) + ? new ContractObsoleteException(TContract.Name, version) + : new ContractUnrecognizedException(TContract.Name, version); return false; } } else if (!_creators.TryGetValue((typeof(TContract), string.Empty), out creator)) { - failureReason = $"Target does not support contract '{typeof(TContract).Name}'."; + failureException = new ContractMissingException(TContract.Name); return false; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs index c9e6f53c922ba4..38d2fad2210551 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs @@ -9,6 +9,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; +using System.Text.Json; using Microsoft.Diagnostics.DataContractReader.Data; using Microsoft.Diagnostics.DataContractReader.Contracts; using System.Collections.Frozen; @@ -73,30 +74,19 @@ private readonly struct Configuration /// A callback to set a thread's context /// A callback to allocate virtual memory in the target /// Registration actions that populate the contract registry (e.g., ) - /// The target object. - /// If a target instance could be created, true; otherwise, false. - public static bool TryCreate( + /// The target object. + public static ContractDescriptorTarget Create( ulong contractDescriptor, ReadFromTargetDelegate readFromTarget, WriteToTargetDelegate writeToTarget, GetTargetThreadContextDelegate getThreadContext, SetTargetThreadContextDelegate setThreadContext, AllocVirtualDelegate allocVirtual, - Action[] contractRegistrations, - [NotNullWhen(true)] out ContractDescriptorTarget? target) + Action[] contractRegistrations) { DataTargetDelegates dataTargetDelegates = new DataTargetDelegates(readFromTarget, writeToTarget, getThreadContext, setThreadContext, allocVirtual); - if (TryReadContractDescriptor( - contractDescriptor, - dataTargetDelegates, - out Descriptor descriptor)) - { - target = new ContractDescriptorTarget(descriptor, dataTargetDelegates, contractRegistrations); - return true; - } - - target = null; - return false; + Descriptor descriptor = ReadContractDescriptor(contractDescriptor, dataTargetDelegates); + return new ContractDescriptorTarget(descriptor, dataTargetDelegates, contractRegistrations); } /// @@ -185,13 +175,8 @@ private void BuildDescriptors(bool forceBuild = false) { _pendingSubDescriptors.RemoveAt(i); - if (TryReadContractDescriptor( - subDescriptorAddress.Value, - _dataTargetDelegates, - out Descriptor subDescriptor)) - { - AddDescriptor(subDescriptor); - } + Descriptor subDescriptor = ReadContractDescriptor(subDescriptorAddress.Value, _dataTargetDelegates); + AddDescriptor(subDescriptor); } } } while (_descriptors.Count > loopDescriptorCount); @@ -216,14 +201,16 @@ private void BuildDescriptors(bool forceBuild = false) { if (descriptor.Config.IsLittleEndian != _config.IsLittleEndian || descriptor.Config.PointerSize != _config.PointerSize) - throw new InvalidOperationException("All descriptors must have the same endianness and pointer size."); + { + throw new FormatException("All descriptors must have the same endianness and pointer size."); + } // Read contracts and add to map foreach ((string name, string version) in descriptor.ContractDescriptor.Contracts ?? []) { if (contracts.ContainsKey(name)) { - throw new InvalidOperationException($"Duplicate contract name '{name}' found in contract descriptor."); + throw new FormatException($"Duplicate contract name '{name}' found in contract descriptor."); } contracts[name] = version; } @@ -249,7 +236,7 @@ private void BuildDescriptors(bool forceBuild = false) if (seenTypeNames.Contains(name)) { - throw new InvalidOperationException($"Duplicate type name '{name}' found in contract descriptor."); + throw new FormatException($"Duplicate type name '{name}' found in contract descriptor."); } seenTypeNames.Add(name); @@ -263,14 +250,14 @@ private void BuildDescriptors(bool forceBuild = false) foreach ((string name, ContractDescriptorParser.GlobalDescriptor global) in descriptor.ContractDescriptor.Globals) { if (seenGlobalNames.Contains(name)) - throw new InvalidOperationException($"Duplicate global name '{name}' found in contract descriptor."); + throw new FormatException($"Duplicate global name '{name}' found in contract descriptor."); seenGlobalNames.Add(name); if (global.Indirect) { if (global.NumericValue.Value >= (ulong)descriptor.PointerData.Length) - throw new InvalidOperationException($"Invalid pointer data index {global.NumericValue.Value}."); + throw new FormatException($"Invalid pointer data index {global.NumericValue.Value}."); globals[name] = new GlobalValue { @@ -318,7 +305,7 @@ private static IEnumerable GetSubDescriptors(Descriptor descripto if (subDescriptor.Value.Indirect) { if (subDescriptor.Value.NumericValue.Value >= (ulong)descriptor.PointerData.Length) - throw new InvalidOperationException($"Invalid pointer data index {subDescriptor.Value.NumericValue.Value}."); + throw new FormatException($"Invalid pointer data index {subDescriptor.Value.NumericValue.Value}."); yield return descriptor.PointerData[(int)subDescriptor.Value.NumericValue]; } @@ -326,28 +313,25 @@ private static IEnumerable GetSubDescriptors(Descriptor descripto } // See docs/design/datacontracts/contract-descriptor.md - private static bool TryReadContractDescriptor( + private static Descriptor ReadContractDescriptor( ulong address, - DataTargetDelegates dataTargetDelegates, - out Descriptor descriptor) + DataTargetDelegates dataTargetDelegates) { - descriptor = default; - // Magic - uint64_t Span buffer = stackalloc byte[sizeof(ulong)]; if (dataTargetDelegates.ReadFromTarget(address, buffer) < 0) - return false; + throw new FormatException($"Failed to read contract descriptor header at 0x{address:x8}."); address += sizeof(ulong); ReadOnlySpan magicLE = "DNCCDAC\0"u8; ReadOnlySpan magicBE = "\0CADCCND"u8; bool isLittleEndian = buffer.SequenceEqual(magicLE); if (!isLittleEndian && !buffer.SequenceEqual(magicBE)) - return false; + throw new FormatException("Contract descriptor has an invalid magic value."); // Flags - uint32_t if (!TryRead(address, isLittleEndian, dataTargetDelegates, out uint flags)) - return false; + throw new FormatException($"Failed to read contract descriptor flags at 0x{address:x8}."); address += sizeof(uint); @@ -358,19 +342,19 @@ private static bool TryReadContractDescriptor( // Descriptor size - uint32_t if (!TryRead(address, config.IsLittleEndian, dataTargetDelegates, out uint descriptorSize)) - return false; + throw new FormatException($"Failed to read contract descriptor size at 0x{address:x8}."); address += sizeof(uint); // Descriptor - char* if (!TryReadPointer(address, config, dataTargetDelegates, out TargetPointer descriptorAddr)) - return false; + throw new FormatException($"Failed to read contract descriptor JSON pointer at 0x{address:x8}."); address += (uint)pointerSize; // Pointer data count - uint32_t if (!TryRead(address, config.IsLittleEndian, dataTargetDelegates, out uint pointerDataCount)) - return false; + throw new FormatException($"Failed to read contract descriptor pointer data count at 0x{address:x8}."); address += sizeof(uint); @@ -379,35 +363,52 @@ private static bool TryReadContractDescriptor( // Pointer data - uintptr_t* if (!TryReadPointer(address, config, dataTargetDelegates, out TargetPointer pointerDataAddr)) - return false; + throw new FormatException($"Failed to read contract descriptor pointer data pointer at 0x{address:x8}."); // Read descriptor + if (descriptorSize > int.MaxValue) + throw new FormatException($"Contract descriptor size {descriptorSize} is too large."); + Span descriptorBuffer = descriptorSize <= StackAllocByteThreshold ? stackalloc byte[(int)descriptorSize] : new byte[(int)descriptorSize]; if (dataTargetDelegates.ReadFromTarget(descriptorAddr.Value, descriptorBuffer) < 0) - return false; + throw new FormatException($"Failed to read contract descriptor JSON at 0x{descriptorAddr.Value:x8}."); - ContractDescriptorParser.ContractDescriptor? contractDescriptor = ContractDescriptorParser.ParseCompact(descriptorBuffer); + ContractDescriptorParser.ContractDescriptor? contractDescriptor; + try + { + contractDescriptor = ContractDescriptorParser.ParseCompact(descriptorBuffer); + } + catch (JsonException ex) + { + throw new FormatException("Failed to parse contract descriptor JSON.", ex); + } + catch (InvalidOperationException ex) + { + throw new FormatException("Failed to parse contract descriptor JSON.", ex); + } if (contractDescriptor is null) - return false; + throw new FormatException("Contract descriptor JSON parsed to null."); // Read pointer data - TargetPointer[] pointerData = new TargetPointer[pointerDataCount]; - for (int i = 0; i < pointerDataCount; i++) + if (pointerDataCount > int.MaxValue) + throw new FormatException($"Contract descriptor pointer data count {pointerDataCount} is too large."); + + int pointerDataLength = (int)pointerDataCount; + TargetPointer[] pointerData = new TargetPointer[pointerDataLength]; + for (int i = 0; i < pointerDataLength; i++) { if (!TryReadPointer(pointerDataAddr.Value + (uint)(i * pointerSize), config, dataTargetDelegates, out pointerData[i])) - return false; + throw new FormatException($"Failed to read contract descriptor pointer data entry {i}."); } - descriptor = new Descriptor + return new Descriptor { Config = config, ContractDescriptor = contractDescriptor, PointerData = pointerData }; - - return true; } public override int PointerSize => _config.PointerSize; diff --git a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs index 26fd7bdec35152..56a67436559542 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs @@ -83,8 +83,7 @@ private static unsafe int Init( }; } - // TODO: [cdac] Better error code/details - if (!ContractDescriptorTarget.TryCreate( + ContractDescriptorTarget target = ContractDescriptorTarget.Create( descriptor, (address, buffer) => { @@ -129,9 +128,7 @@ private static unsafe int Init( }, setThreadContextDelegate, allocDelegate, - [Contracts.CoreCLRContracts.Register], - out ContractDescriptorTarget? target)) - return -1; + [Contracts.CoreCLRContracts.Register]); GCHandle gcHandle = GCHandle.Alloc(target); *handle = GCHandle.ToIntPtr(gcHandle); @@ -321,7 +318,7 @@ private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDat }; } - if (!ContractDescriptorTarget.TryCreate( + ContractDescriptorTarget target = ContractDescriptorTarget.Create( contractAddress, (address, buffer) => { @@ -369,11 +366,7 @@ private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDat } }, allocVirtual, - [Contracts.CoreCLRContracts.Register], - out ContractDescriptorTarget? target)) - { - return -1; - } + [Contracts.CoreCLRContracts.Register]); Legacy.SOSDacImpl impl = new(target, legacyImpl); void* ccw = ComInterfaceMarshaller.ConvertToUnmanaged(impl); diff --git a/src/native/managed/cdac/scripts/DumpHelpers.cs b/src/native/managed/cdac/scripts/DumpHelpers.cs index e4b1c93d7e7326..9f7c6eedaeb7d7 100644 --- a/src/native/managed/cdac/scripts/DumpHelpers.cs +++ b/src/native/managed/cdac/scripts/DumpHelpers.cs @@ -48,19 +48,14 @@ public static ContractDescriptorTarget CreateCdacTarget(DataTarget dt) { ulong contractAddr = FindContractDescriptor(dt); - if (!ContractDescriptorTarget.TryCreate( - contractAddr, - (ulong address, Span buffer) => dt.DataReader.Read(address, buffer) == buffer.Length ? 0 : -1, - (ulong address, Span buffer) => -1, - (uint threadId, uint contextFlags, Span buffer) => - dt.DataReader.GetThreadContext(threadId, contextFlags, buffer) ? 0 : -1, - (uint threadId, ReadOnlySpan context) => -1, - [CoreCLRContracts.Register], - out ContractDescriptorTarget? target)) - { - throw new InvalidOperationException("Failed to create cDAC target."); - } - - return target; + return ContractDescriptorTarget.Create( + contractAddr, + (ulong address, Span buffer) => dt.DataReader.Read(address, buffer) == buffer.Length ? 0 : -1, + (ulong address, Span buffer) => -1, + (uint threadId, uint contextFlags, Span buffer) => + dt.DataReader.GetThreadContext(threadId, contextFlags, buffer) ? 0 : -1, + (uint threadId, ReadOnlySpan context) => -1, + (ulong _, out ulong _) => throw new NotImplementedException("Scripts do not provide AllocVirtual"), + [CoreCLRContracts.Register]); } } diff --git a/src/native/managed/cdac/tests/DataGenerator/TestTarget.cs b/src/native/managed/cdac/tests/DataGenerator/TestTarget.cs index 6d6f6544b17575..da18f2b7abc335 100644 --- a/src/native/managed/cdac/tests/DataGenerator/TestTarget.cs +++ b/src/native/managed/cdac/tests/DataGenerator/TestTarget.cs @@ -271,22 +271,25 @@ public TestContractRegistry(IManagedTypeSource managedTypeSource) public override IManagedTypeSource ManagedTypeSource => _managedTypeSource; - public override bool TryGetContract([NotNullWhen(true)] out TContract contract, out string? failureReason) + public override bool TryGetContract([NotNullWhen(true)] out TContract contract, [NotNullWhen(false)] out System.Exception? failureException) { if (typeof(TContract) == typeof(IManagedTypeSource)) { contract = (TContract)_managedTypeSource; - failureReason = null; + failureException = null; return true; } contract = default!; - failureReason = "Not registered in TestContractRegistry."; + failureException = new ContractMissingException(TContract.Name); return false; } public override void Register(string version, Func creator) => throw new NotImplementedException(); + public override void RegisterUnsupported(string version) + => throw new NotImplementedException(); + public override void Flush(FlushScope scope) { } } diff --git a/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs b/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs index e525d16c6eb865..8c18c5181f679f 100644 --- a/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs +++ b/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs @@ -29,13 +29,16 @@ public class DescriptorBuilder(ContractDescriptorBuilder parent) private bool _created; private readonly ContractDescriptorBuilder _parent = parent; - private IReadOnlyCollection? _contracts; + private IReadOnlyDictionary? _contracts; private IDictionary? _types; private IReadOnlyCollection<(string Name, ulong? Value, uint? IndirectIndex, string? StringValue, string? TypeName)>? _globals; private IReadOnlyCollection<(string Name, ulong? Value, uint? IndirectIndex, string? StringValue, string? TypeName)>? _subDescriptors; private IReadOnlyCollection? _indirectValues; public DescriptorBuilder SetContracts(IReadOnlyCollection contracts) + => SetContracts(contracts.ToDictionary(static c => c, static _ => "c1")); + + public DescriptorBuilder SetContracts(IReadOnlyDictionary contracts) { if (_contracts is not null) throw new InvalidOperationException("Contracts already set"); @@ -139,9 +142,9 @@ private string MakeContractsJson() if (_contracts is null || _contracts.Count == 0) return string.Empty; StringBuilder sb = new(); - foreach (var c in _contracts) + foreach ((string name, string version) in _contracts) { - sb.Append($"\"{c}\": \"c1\","); + sb.Append($"\"{name}\": \"{version}\","); } Debug.Assert(sb.Length > 0); sb.Length--; // remove trailing comma @@ -202,13 +205,78 @@ private string MakeContractsJson() } } - public bool TryCreateTarget(DescriptorBuilder descriptor, [NotNullWhen(true)] out ContractDescriptorTarget? target, Action[]? contractRegistrations = null) + public bool TryCreateTarget( + DescriptorBuilder descriptor, + [NotNullWhen(true)] out ContractDescriptorTarget? target, + params Action[] additionalContractRegistrations) + { + try + { + target = CreateTarget(descriptor, additionalContractRegistrations); + return true; + } + catch (Exception) + { + target = null; + return false; + } + } + + public ContractDescriptorTarget CreateTarget(DescriptorBuilder descriptor, params Action[] additionalContractRegistrations) { if (_created) throw new InvalidOperationException("Context already created"); _created = true; ulong contractDescriptorAddress = descriptor.CreateSubDescriptor(ContractDescriptorAddr, JsonDescriptorAddr, ContractPointerDataAddr); MockMemorySpace.MemoryContext memoryContext = GetMemoryContext(); - return ContractDescriptorTarget.TryCreate(contractDescriptorAddress, memoryContext.ReadFromTarget, memoryContext.WriteToTarget, (_, _, _) => throw new NotImplementedException("Tests do not provide GetTargetThreadContext"), (_, _) => throw new NotImplementedException("Tests do not provide SetTargetThreadContext"), (ulong _, out ulong _) => throw new NotImplementedException("Tests do not provide AllocVirtual"), contractRegistrations ?? [Contracts.CoreCLRContracts.Register], out target); + Action[] contractRegistrations = [Contracts.CoreCLRContracts.Register, .. additionalContractRegistrations]; + + return ContractDescriptorTarget.Create( + contractDescriptorAddress, + memoryContext.ReadFromTarget, + memoryContext.WriteToTarget, + (_, _, _) => throw new NotImplementedException("Tests do not provide GetTargetThreadContext"), + (_, _) => throw new NotImplementedException("Tests do not provide SetTargetThreadContext"), + (ulong _, out ulong _) => throw new NotImplementedException("Tests do not provide AllocVirtual"), + contractRegistrations); + } + + public ContractDescriptorTarget CreateTargetFromRawDescriptor(byte[] descriptor, byte[] descriptorJson, byte[] pointerData) + { + if (_created) + throw new InvalidOperationException("Context already created"); + _created = true; + + AddHeapFragment(new MockMemorySpace.HeapFragment + { + Address = ContractDescriptorAddr, + Data = descriptor, + Name = "ContractDescriptor" + }); + AddHeapFragment(new MockMemorySpace.HeapFragment + { + Address = JsonDescriptorAddr, + Data = descriptorJson, + Name = "JsonDescriptor" + }); + if (pointerData.Length > 0) + { + AddHeapFragment(new MockMemorySpace.HeapFragment + { + Address = ContractPointerDataAddr, + Data = pointerData, + Name = "PointerData" + }); + } + + MockMemorySpace.MemoryContext memoryContext = GetMemoryContext(); + return ContractDescriptorTarget.Create( + ContractDescriptorAddr, + memoryContext.ReadFromTarget, + memoryContext.WriteToTarget, + (_, _, _) => throw new NotImplementedException("Tests do not provide GetTargetThreadContext"), + (_, _) => throw new NotImplementedException("Tests do not provide SetTargetThreadContext"), + (ulong _, out ulong _) => throw new NotImplementedException("Tests do not provide AllocVirtual"), + [Contracts.CoreCLRContracts.Register]); } } diff --git a/src/native/managed/cdac/tests/TestInfrastructure/DumpTestBase.cs b/src/native/managed/cdac/tests/TestInfrastructure/DumpTestBase.cs index c98afbe8b41c96..865402813cb6b9 100644 --- a/src/native/managed/cdac/tests/TestInfrastructure/DumpTestBase.cs +++ b/src/native/managed/cdac/tests/TestInfrastructure/DumpTestBase.cs @@ -108,17 +108,14 @@ protected void InitializeDumpTest(TestConfiguration config, string debuggeeName, _host = ClrMdDumpHost.Open(dumpPath, GetSymbolPaths(debuggeeName, versionDir)); ulong contractDescriptor = _host.FindContractDescriptorAddress(); - bool created = ContractDescriptorTarget.TryCreate( + _target = ContractDescriptorTarget.Create( contractDescriptor, _host.ReadFromTarget, writeToTarget: static (_, _) => -1, _host.GetThreadContext, setThreadContext: static (_, _) => -1, allocVirtual: static (ulong _, out ulong _) => throw new NotImplementedException("Dump tests do not provide AllocVirtual"), - [Contracts.CoreCLRContracts.Register], - out _target); - - Assert.True(created, $"Failed to create ContractDescriptorTarget from dump: {dumpPath}"); + [Contracts.CoreCLRContracts.Register]); } /// @@ -134,17 +131,14 @@ protected void InitializeDumpTestFromPath(string dumpPath) _host = ClrMdDumpHost.Open(dumpPath, []); ulong contractDescriptor = _host.FindContractDescriptorAddress(); - bool created = ContractDescriptorTarget.TryCreate( + _target = ContractDescriptorTarget.Create( contractDescriptor, _host.ReadFromTarget, writeToTarget: static (_, _) => -1, _host.GetThreadContext, setThreadContext: static (_, _) => -1, allocVirtual: static (ulong _, out ulong _) => throw new NotImplementedException("Dump tests do not provide AllocVirtual"), - [Contracts.CoreCLRContracts.Register], - out _target); - - Assert.True(created, $"Failed to create ContractDescriptorTarget from dump: {dumpPath}"); + [Contracts.CoreCLRContracts.Register]); } public void Dispose() diff --git a/src/native/managed/cdac/tests/TestInfrastructure/TestPlaceholderTarget.cs b/src/native/managed/cdac/tests/TestInfrastructure/TestPlaceholderTarget.cs index d064f5eda7d272..f016f625003d72 100644 --- a/src/native/managed/cdac/tests/TestInfrastructure/TestPlaceholderTarget.cs +++ b/src/native/managed/cdac/tests/TestInfrastructure/TestPlaceholderTarget.cs @@ -604,6 +604,7 @@ public sealed class TestContractRegistry : ContractRegistry private readonly Dictionary _versions = new(); private readonly Dictionary _mocks = new(); private readonly Dictionary _resolved = new(); + private readonly HashSet<(Type, string)> _unsupportedVersions = new(); private Target _target = null!; public void SetTarget(Target target) => _target = target; @@ -617,10 +618,13 @@ public void SetMock(TContract mock) where TContract : IContract public override void Register(string version, Func creator) => _creators[(typeof(TContract), version)] = t => creator(t); - public override bool TryGetContract([NotNullWhen(true)] out TContract contract, out string? failureReason) + public override void RegisterUnsupported(string version) + => _unsupportedVersions.Add((typeof(TContract), version)); + + public override bool TryGetContract([NotNullWhen(true)] out TContract contract, [NotNullWhen(false)] out System.Exception? failureException) { contract = default!; - failureReason = null; + failureException = null; if (_resolved.TryGetValue(typeof(TContract), out var cached)) { contract = (TContract)cached; @@ -636,7 +640,9 @@ public override bool TryGetContract([NotNullWhen(true)] out TContract { if (!_creators.TryGetValue((typeof(TContract), version), out var creator)) { - failureReason = $"Target supports contract '{typeof(TContract).Name}' version {version}, but no implementation is registered for that version."; + failureException = _unsupportedVersions.Contains((typeof(TContract), version)) + ? new ContractObsoleteException(TContract.Name, version) + : new ContractUnrecognizedException(TContract.Name, version); return false; } @@ -644,7 +650,7 @@ public override bool TryGetContract([NotNullWhen(true)] out TContract } else { - failureReason = $"Contract '{typeof(TContract).Name}' is not supported by the target."; + failureException = new ContractMissingException(TContract.Name); return false; } diff --git a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs index 64f76a941cfa6e..8f551f7b11b8b8 100644 --- a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs @@ -235,6 +235,119 @@ public void ReadUtf16String(MockTarget.Architecture arch) Assert.Equal(expected, actual); } + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void Create_InvalidDescriptorJson_ThrowsFormatException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + byte[] descriptor = new byte[ContractDescriptorHelpers.Size(targetTestHelpers.Arch.Is64Bit)]; + byte[] descriptorJson = "{ invalid json"u8.ToArray(); + ContractDescriptorHelpers.Fill(descriptor, targetTestHelpers.Arch, descriptorJson.Length, 0xdddddddd, 0, 0xeeeeeeee); + + FormatException ex = Assert.Throws(() => builder.CreateTargetFromRawDescriptor(descriptor, descriptorJson, [])); + Assert.IsType(ex.InnerException); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void Create_InvalidPointerDataIndex_ThrowsFormatException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetGlobals([("invalid", null, (uint?)1, null, "pointer")]) + .SetIndirectValues([0]); + + Assert.Throws(() => builder.CreateTarget(descriptorBuilder)); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void TryCreateTarget_InvalidPointerDataIndex_ReturnsFalse(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetGlobals([("invalid", null, (uint?)1, null, "pointer")]) + .SetIndirectValues([0]); + + Assert.False(builder.TryCreateTarget(descriptorBuilder, out _)); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetContract_MissingContract_ThrowsContractMissingException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts([]); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + ContractMissingException ex = Assert.Throws(() => target.Contracts.RuntimeInfo); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.Null(ex.ContractVersion); + Assert.Equal(unchecked((int)0x80004001), ex.HResult); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetContract_UnrecognizedVersion_ThrowsContractUnrecognizedException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(new Dictionary { ["RuntimeInfo"] = "unsupported-version" }); + + Assert.True(builder.TryCreateTarget( + descriptorBuilder, + out ContractDescriptorTarget? target)); + + ContractUnrecognizedException ex = Assert.Throws(() => target.Contracts.RuntimeInfo); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.Equal("unsupported-version", ex.ContractVersion); + Assert.Equal(unchecked((int)0x80004001), ex.HResult); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetContract_ObsoleteVersion_ThrowsContractObsoleteException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(new Dictionary { ["RuntimeInfo"] = "obsolete-version" }); + + Assert.True(builder.TryCreateTarget( + descriptorBuilder, + out ContractDescriptorTarget? target, + registry => registry.RegisterUnsupported("obsolete-version"))); + + ContractObsoleteException ex = Assert.Throws(() => target.Contracts.RuntimeInfo); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.Equal("obsolete-version", ex.ContractVersion); + Assert.Equal(unchecked((int)0x80004001), ex.HResult); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void TryGetContract_UnrecognizedVersion_ReturnsContractUnrecognizedException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(new Dictionary { ["RuntimeInfo"] = "unsupported-version" }); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + Assert.False(target.Contracts.TryGetContract(out _, out System.Exception? failureException)); + ContractUnrecognizedException ex = Assert.IsType(failureException); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.Equal("unsupported-version", ex.ContractVersion); + } + private static void ValidateGlobals( ContractDescriptorTarget target, (string Name, ulong Value, string? Type)[] globals, From f761f172d1a3021120d95e17a6525ac9f7ee7420 Mon Sep 17 00:00:00 2001 From: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> Date: Sun, 12 Jul 2026 04:41:12 -0700 Subject: [PATCH 2/5] cDAC: eager contract validation with classifiable HRESULTs (WIP checkpoint) Intermediate checkpoint of the cDAC 'policy by support' work: - CdacHResults: CDAC_E_CONTRACT_{NOT_ADVERTISED,UNRECOGNIZED,UNSUPPORTED} and CDAC_E_DESCRIPTOR_{NOT_FOUND,MALFORMED} (customer-bit failure codes). - ContractNotAvailableException hierarchy (Missing/Unrecognized/Obsolete) and ContractValidationException carrying the distinct HRESULT. - ContractRegistry.TryValidate + CachingContractRegistry no-instantiation resolution (safe on partial/triage dumps). - CoreCLRContracts.ValidateForSos eager validation of the SOS surface, run at CLRDataCreateInstance only when no legacy DAC is supplied. - ContractDescriptorTarget descriptor-read failures mapped to CDAC_E_DESCRIPTOR_* via FormatException.HResult. - Design docs: contract-validation.md + consumer guide. - Tests in TargetTests.cs. --- .../CdacHResults.cs | 59 ++++++ .../ContractNotAvailableException.cs | 33 ++++ .../ContractRegistry.cs | 25 +++ .../CoreCLRContracts.cs | 66 +++++++ .../CachingContractRegistry.cs | 65 +++++-- .../ContractDescriptorTarget.cs | 50 +++-- .../mscordaccore_universal/Entrypoints.cs | 16 +- .../ContractDescriptor/TargetTests.cs | 182 +++++++++++++++++- 8 files changed, 461 insertions(+), 35 deletions(-) create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CdacHResults.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CdacHResults.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CdacHResults.cs new file mode 100644 index 00000000000000..bd199b63ca01c4 --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CdacHResults.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Diagnostics.DataContractReader; + +/// +/// cDAC-specific HRESULTs surfaced when creating a data-access interface +/// for a target fails for a reason this cDAC can describe precisely. This covers two stages of +/// creation: locating and reading the target's contract descriptor (the DESCRIPTOR codes), +/// and eager validation that this cDAC can service the contracts the descriptor advertises (the +/// CONTRACT codes, see +/// ). +/// +/// +/// The customer bit (bit 29, 0x20000000) is set on every value so these codes can never +/// collide with system or CLR (corerror.h) HRESULTs. All values are failure codes, so +/// existing FAILED(hr) checks in native callers continue to behave correctly. Native code +/// and tooling that wants to distinguish the failure modes (for example, to decide whether to fall +/// back to the brittle DAC) can compare against the literal values documented here. +/// +public static class CdacHResults +{ + /// + /// A contract required to service the SOS interface is not advertised by the target's + /// contract descriptor. The target runtime predates the contract or does not expose it. + /// + public const int CDAC_E_CONTRACT_NOT_ADVERTISED = unchecked((int)0xA0DAC001); + + /// + /// The target advertises a required contract at a version this cDAC does not recognize, + /// typically because the target runtime is newer than this cDAC. + /// + public const int CDAC_E_CONTRACT_UNRECOGNIZED = unchecked((int)0xA0DAC002); + + /// + /// The target advertises a required contract at a version this cDAC recognizes but + /// intentionally does not implement, typically because the target runtime is too old. + /// + public const int CDAC_E_CONTRACT_UNSUPPORTED = unchecked((int)0xA0DAC003); + + /// + /// No usable cDAC contract descriptor could be located for the target: the contract locator + /// did not produce a descriptor address, the descriptor header could not be read, or the bytes + /// at the descriptor address are not a contract descriptor (bad magic). This typically means the + /// target is not a cDAC-capable .NET runtime (for example, a non-.NET target or a runtime that + /// predates the cDAC contract descriptor), and tooling should fall back to the brittle DAC + /// rather than treat it as a hard error. + /// + public const int CDAC_E_DESCRIPTOR_NOT_FOUND = unchecked((int)0xA0DAC011); + + /// + /// A cDAC contract descriptor was found but could not be consumed: a field could not be read, + /// the embedded JSON failed to parse, or the descriptor content is internally inconsistent + /// (for example, duplicate names or an out-of-range pointer-data index). This indicates a + /// corrupt dump or a descriptor format this cDAC cannot understand, and is distinct from a + /// recognized-but-unsupported contract version (). + /// + public const int CDAC_E_DESCRIPTOR_MALFORMED = unchecked((int)0xA0DAC012); +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs index 036fd6108963c4..ee79bb6394da94 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractNotAvailableException.cs @@ -96,3 +96,36 @@ public ContractObsoleteException(string contractName, string contractVersion) : base(contractName, contractVersion, $"Contract '{contractName}' version {contractVersion} is advertised by the target and recognized by this cDAC, but is intentionally not implemented.") { } } + +/// +/// Exception thrown when eager validation of the contracts required to service an SOS / +/// IXCLRDataProcess interface fails during creation. Unlike the lazy +/// hierarchy (which carries E_NOTIMPL so that +/// individual SOS APIs degrade gracefully), this exception carries a distinct +/// value identifying the failure category so the native loader can decide how to proceed. +/// +public sealed class ContractValidationException : Exception +{ + /// + /// Initializes a new instance of the class. + /// + /// The value describing the validation failure category. + /// The underlying describing which contract failed and why. + public ContractValidationException(int hResult, ContractNotAvailableException inner) + : base(inner.Message, inner) + { + HResult = hResult; + ContractName = inner.ContractName; + ContractVersion = inner.ContractVersion; + } + + /// + /// Gets the name of the contract whose validation failed. + /// + public string ContractName { get; } + + /// + /// Gets the target-advertised version of the contract whose validation failed, or if the target did not advertise the contract. + /// + public string? ContractVersion { get; } +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs index 27003d9378cc89..cccea8cb6724cd 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs @@ -179,6 +179,31 @@ public bool TryGetContract([NotNullWhen(true)] out TContract contract return TryGetContract(out contract, out _); } + /// + /// Determines whether this cDAC can provide the requested contract for the target. Implementations + /// should resolve the target-advertised version against the registered implementations only, without + /// instantiating the contract, so that validation performs no target memory reads (safe on partial or + /// triage dumps) and never triggers contract-to-contract chaining. + /// + /// The contract type to validate. + /// + /// When this method returns , contains the exception describing why the + /// contract cannot be provided; otherwise, . + /// + /// + /// if the contract is advertised by the target and a matching implementation + /// is registered; otherwise, . + /// + /// + /// The default implementation delegates to , + /// which instantiates the contract and therefore does read target memory. Registries that need the + /// no-instantiation guarantee above must override this method (CachingContractRegistry does). + /// + public virtual bool TryValidate([NotNullWhen(false)] out System.Exception? failureException) where TContract : IContract + { + return TryGetContract(out _, out failureException); + } + /// /// Register a contract implementation for a specific version. /// External packages use this to add contract versions or entirely new contract interfaces. diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs index c654da1595ab90..bfea93d140abc8 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs @@ -80,4 +80,70 @@ public static void Register(ContractRegistry registry) registry.Register("c1", static t => new RuntimeMutableTypeSystem_1(t)); } + + /// + /// Eagerly validates that every contract required by the cDAC data-access interfaces can be + /// provided for the target, without instantiating any of them. This is intended to run at + /// interface-creation time so that an unsupported target fails fast with a distinct + /// code instead of failing lazily inside a later data-access call. + /// + /// The contract registry for the target being validated. + /// + /// Thrown for the first required contract that cannot be provided. The exception's + /// is + /// if the target does not advertise the + /// contract, if the advertised version is + /// unknown to this cDAC, or if the advertised + /// version is recognized but intentionally unimplemented. + /// + public static void ValidateForDataAccess(ContractRegistry registry) + { + // Direct contract accesses from SOSDacImpl.cs and SOSDacImpl.IXCLRDataProcess.cs. + // IObjectiveCMarshal is optional because SOS uses TryGetContract for Apple-only support. + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + Validate(registry); + + // Transitive contract accesses from the implementations above. + Validate(registry); // IComWrappers: ComWrappers_1.cs + Validate(registry); // IStackWalk: StackWalk_1.cs + Validate(registry); // IAuxiliarySymbols/IPrecodeStubs: CodePointerUtils.cs, PrecodeStubs_Common.cs + Validate(registry); // ILoader: Loader_1.cs + + static void Validate(ContractRegistry registry) where TContract : IContract + { + if (registry.TryValidate(out System.Exception? failure)) + { + return; + } + + throw failure switch + { + ContractObsoleteException obsolete => new ContractValidationException(CdacHResults.CDAC_E_CONTRACT_UNSUPPORTED, obsolete), + ContractUnrecognizedException unrecognized => new ContractValidationException(CdacHResults.CDAC_E_CONTRACT_UNRECOGNIZED, unrecognized), + ContractNotAvailableException notAvailable => new ContractValidationException(CdacHResults.CDAC_E_CONTRACT_NOT_ADVERTISED, notAvailable), + _ => failure, + }; + } + } } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs index b5d3618a5936f0..f4e7fa0866b4cf 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs @@ -54,20 +54,8 @@ public override bool TryGetContract([NotNullWhen(true)] out TContract return true; } - Func? creator; - if (_tryGetContractVersion(TContract.Name, out string? version)) + if (!TryResolveCreator(typeof(TContract), TContract.Name, out Func? creator, out failureException)) { - if (!_creators.TryGetValue((typeof(TContract), version), out creator)) - { - failureException = _unsupportedVersions.Contains((typeof(TContract), version)) - ? new ContractObsoleteException(TContract.Name, version) - : new ContractUnrecognizedException(TContract.Name, version); - return false; - } - } - else if (!_creators.TryGetValue((typeof(TContract), string.Empty), out creator)) - { - failureException = new ContractMissingException(TContract.Name); return false; } @@ -81,6 +69,57 @@ public override bool TryGetContract([NotNullWhen(true)] out TContract return true; } + public override bool TryValidate([NotNullWhen(false)] out System.Exception? failureException) + { + failureException = null; + + // An already-instantiated contract is, by definition, supported. + if (_contracts.ContainsKey(typeof(TContract))) + { + return true; + } + + // Resolve only — never invoke the creator. Invoking it would read target memory and may + // chain into other contracts, which must not happen during eager validation. + return TryResolveCreator(typeof(TContract), TContract.Name, out _, out failureException); + } + + /// + /// Classifies whether a registered creator exists for the target-advertised version of a + /// contract, without invoking it. Shared by + /// and . + /// + private bool TryResolveCreator( + Type contractType, + string contractName, + [NotNullWhen(true)] out Func? creator, + [NotNullWhen(false)] out System.Exception? failureException) + { + creator = null; + failureException = null; + + if (!_tryGetContractVersion(contractName, out string? version)) + { + if (_creators.TryGetValue((contractType, string.Empty), out creator)) + { + return true; + } + + failureException = new ContractMissingException(contractName); + return false; + } + + if (!_creators.TryGetValue((contractType, version), out creator)) + { + failureException = _unsupportedVersions.Contains((contractType, version)) + ? new ContractObsoleteException(contractName, version) + : new ContractUnrecognizedException(contractName, version); + return false; + } + + return true; + } + public override void Flush(FlushScope scope) { foreach (IContract contract in _contracts.Values) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs index 38d2fad2210551..28d8a19dfe3d17 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs @@ -202,7 +202,7 @@ private void BuildDescriptors(bool forceBuild = false) if (descriptor.Config.IsLittleEndian != _config.IsLittleEndian || descriptor.Config.PointerSize != _config.PointerSize) { - throw new FormatException("All descriptors must have the same endianness and pointer size."); + throw DescriptorMalformed("All descriptors must have the same endianness and pointer size."); } // Read contracts and add to map @@ -210,7 +210,7 @@ private void BuildDescriptors(bool forceBuild = false) { if (contracts.ContainsKey(name)) { - throw new FormatException($"Duplicate contract name '{name}' found in contract descriptor."); + throw DescriptorMalformed($"Duplicate contract name '{name}' found in contract descriptor."); } contracts[name] = version; } @@ -236,7 +236,7 @@ private void BuildDescriptors(bool forceBuild = false) if (seenTypeNames.Contains(name)) { - throw new FormatException($"Duplicate type name '{name}' found in contract descriptor."); + throw DescriptorMalformed($"Duplicate type name '{name}' found in contract descriptor."); } seenTypeNames.Add(name); @@ -250,14 +250,14 @@ private void BuildDescriptors(bool forceBuild = false) foreach ((string name, ContractDescriptorParser.GlobalDescriptor global) in descriptor.ContractDescriptor.Globals) { if (seenGlobalNames.Contains(name)) - throw new FormatException($"Duplicate global name '{name}' found in contract descriptor."); + throw DescriptorMalformed($"Duplicate global name '{name}' found in contract descriptor."); seenGlobalNames.Add(name); if (global.Indirect) { if (global.NumericValue.Value >= (ulong)descriptor.PointerData.Length) - throw new FormatException($"Invalid pointer data index {global.NumericValue.Value}."); + throw DescriptorMalformed($"Invalid pointer data index {global.NumericValue.Value}."); globals[name] = new GlobalValue { @@ -305,7 +305,7 @@ private static IEnumerable GetSubDescriptors(Descriptor descripto if (subDescriptor.Value.Indirect) { if (subDescriptor.Value.NumericValue.Value >= (ulong)descriptor.PointerData.Length) - throw new FormatException($"Invalid pointer data index {subDescriptor.Value.NumericValue.Value}."); + throw DescriptorMalformed($"Invalid pointer data index {subDescriptor.Value.NumericValue.Value}."); yield return descriptor.PointerData[(int)subDescriptor.Value.NumericValue]; } @@ -313,6 +313,16 @@ private static IEnumerable GetSubDescriptors(Descriptor descripto } // See docs/design/datacontracts/contract-descriptor.md + // Failure constructing a target from its contract descriptor surfaces as a FormatException so + // existing callers and tests keep working, but the HResult is set to a cDAC-specific code so + // tooling can distinguish "no descriptor / not a cDAC target" from "descriptor present but + // corrupt". The boundary entry points propagate Exception.HResult when it is a failure code. + private static FormatException DescriptorNotFound(string message) => + new(message) { HResult = CdacHResults.CDAC_E_DESCRIPTOR_NOT_FOUND }; + + private static FormatException DescriptorMalformed(string message, System.Exception? innerException = null) => + new(message, innerException) { HResult = CdacHResults.CDAC_E_DESCRIPTOR_MALFORMED }; + private static Descriptor ReadContractDescriptor( ulong address, DataTargetDelegates dataTargetDelegates) @@ -320,18 +330,18 @@ private static Descriptor ReadContractDescriptor( // Magic - uint64_t Span buffer = stackalloc byte[sizeof(ulong)]; if (dataTargetDelegates.ReadFromTarget(address, buffer) < 0) - throw new FormatException($"Failed to read contract descriptor header at 0x{address:x8}."); + throw DescriptorNotFound($"Failed to read contract descriptor header at 0x{address:x8}."); address += sizeof(ulong); ReadOnlySpan magicLE = "DNCCDAC\0"u8; ReadOnlySpan magicBE = "\0CADCCND"u8; bool isLittleEndian = buffer.SequenceEqual(magicLE); if (!isLittleEndian && !buffer.SequenceEqual(magicBE)) - throw new FormatException("Contract descriptor has an invalid magic value."); + throw DescriptorNotFound("Contract descriptor has an invalid magic value."); // Flags - uint32_t if (!TryRead(address, isLittleEndian, dataTargetDelegates, out uint flags)) - throw new FormatException($"Failed to read contract descriptor flags at 0x{address:x8}."); + throw DescriptorMalformed($"Failed to read contract descriptor flags at 0x{address:x8}."); address += sizeof(uint); @@ -342,19 +352,19 @@ private static Descriptor ReadContractDescriptor( // Descriptor size - uint32_t if (!TryRead(address, config.IsLittleEndian, dataTargetDelegates, out uint descriptorSize)) - throw new FormatException($"Failed to read contract descriptor size at 0x{address:x8}."); + throw DescriptorMalformed($"Failed to read contract descriptor size at 0x{address:x8}."); address += sizeof(uint); // Descriptor - char* if (!TryReadPointer(address, config, dataTargetDelegates, out TargetPointer descriptorAddr)) - throw new FormatException($"Failed to read contract descriptor JSON pointer at 0x{address:x8}."); + throw DescriptorMalformed($"Failed to read contract descriptor JSON pointer at 0x{address:x8}."); address += (uint)pointerSize; // Pointer data count - uint32_t if (!TryRead(address, config.IsLittleEndian, dataTargetDelegates, out uint pointerDataCount)) - throw new FormatException($"Failed to read contract descriptor pointer data count at 0x{address:x8}."); + throw DescriptorMalformed($"Failed to read contract descriptor pointer data count at 0x{address:x8}."); address += sizeof(uint); @@ -363,17 +373,17 @@ private static Descriptor ReadContractDescriptor( // Pointer data - uintptr_t* if (!TryReadPointer(address, config, dataTargetDelegates, out TargetPointer pointerDataAddr)) - throw new FormatException($"Failed to read contract descriptor pointer data pointer at 0x{address:x8}."); + throw DescriptorMalformed($"Failed to read contract descriptor pointer data pointer at 0x{address:x8}."); // Read descriptor if (descriptorSize > int.MaxValue) - throw new FormatException($"Contract descriptor size {descriptorSize} is too large."); + throw DescriptorMalformed($"Contract descriptor size {descriptorSize} is too large."); Span descriptorBuffer = descriptorSize <= StackAllocByteThreshold ? stackalloc byte[(int)descriptorSize] : new byte[(int)descriptorSize]; if (dataTargetDelegates.ReadFromTarget(descriptorAddr.Value, descriptorBuffer) < 0) - throw new FormatException($"Failed to read contract descriptor JSON at 0x{descriptorAddr.Value:x8}."); + throw DescriptorMalformed($"Failed to read contract descriptor JSON at 0x{descriptorAddr.Value:x8}."); ContractDescriptorParser.ContractDescriptor? contractDescriptor; try @@ -382,25 +392,25 @@ private static Descriptor ReadContractDescriptor( } catch (JsonException ex) { - throw new FormatException("Failed to parse contract descriptor JSON.", ex); + throw DescriptorMalformed("Failed to parse contract descriptor JSON.", ex); } catch (InvalidOperationException ex) { - throw new FormatException("Failed to parse contract descriptor JSON.", ex); + throw DescriptorMalformed("Failed to parse contract descriptor JSON.", ex); } if (contractDescriptor is null) - throw new FormatException("Contract descriptor JSON parsed to null."); + throw DescriptorMalformed("Contract descriptor JSON parsed to null."); // Read pointer data if (pointerDataCount > int.MaxValue) - throw new FormatException($"Contract descriptor pointer data count {pointerDataCount} is too large."); + throw DescriptorMalformed($"Contract descriptor pointer data count {pointerDataCount} is too large."); int pointerDataLength = (int)pointerDataCount; TargetPointer[] pointerData = new TargetPointer[pointerDataLength]; for (int i = 0; i < pointerDataLength; i++) { if (!TryReadPointer(pointerDataAddr.Value + (uint)(i * pointerSize), config, dataTargetDelegates, out pointerData[i])) - throw new FormatException($"Failed to read contract descriptor pointer data entry {i}."); + throw DescriptorMalformed($"Failed to read contract descriptor pointer data entry {i}."); } return new Descriptor diff --git a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs index 56a67436559542..f5ed98e4beb0cf 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs @@ -180,6 +180,12 @@ private static unsafe int CreateSosInterface(IntPtr handle, IntPtr legacyImplPtr object? legacyImpl = legacyImplPtr != IntPtr.Zero ? ComInterfaceMarshaller.ConvertToManaged((void*)legacyImplPtr) : null; + + // Without a legacy implementation to absorb individually-unimplemented APIs, validate + // the complete data-access contract set before publishing the interface. + if (legacyImpl is null) + Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts); + Legacy.SOSDacImpl impl = new(target, legacyImpl); nint ptr = (nint)ComInterfaceMarshaller.ConvertToUnmanaged(impl); *obj = ptr; @@ -293,7 +299,10 @@ private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDat if (hr != 0) { throw new InvalidOperationException( - $"{nameof(ICLRContractLocator)} failed to fetch the contract descriptor with HRESULT: 0x{hr:x}."); + $"{nameof(ICLRContractLocator)} failed to fetch the contract descriptor with HRESULT: 0x{hr:x}.") + { + HResult = CdacHResults.CDAC_E_DESCRIPTOR_NOT_FOUND + }; } // Build the allocVirtual delegate if the target supports ICLRDataTarget2 @@ -368,6 +377,11 @@ private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDat allocVirtual, [Contracts.CoreCLRContracts.Register]); + // Without a legacy implementation to absorb individually-unimplemented APIs, validate + // the complete data-access contract set before publishing the interface. + if (legacyImpl is null) + Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts); + Legacy.SOSDacImpl impl = new(target, legacyImpl); void* ccw = ComInterfaceMarshaller.ConvertToUnmanaged(impl); Marshal.QueryInterface((nint)ccw, *pIID, out nint ptrToIface); diff --git a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs index 8f551f7b11b8b8..fecf6b917b5e20 100644 --- a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/TargetTests.cs @@ -247,6 +247,23 @@ public void Create_InvalidDescriptorJson_ThrowsFormatException(MockTarget.Archit FormatException ex = Assert.Throws(() => builder.CreateTargetFromRawDescriptor(descriptor, descriptorJson, [])); Assert.IsType(ex.InnerException); + Assert.Equal(CdacHResults.CDAC_E_DESCRIPTOR_MALFORMED, ex.HResult); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void Create_InvalidMagic_ThrowsDescriptorNotFound(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + byte[] descriptor = new byte[ContractDescriptorHelpers.Size(targetTestHelpers.Arch.Is64Bit)]; + byte[] descriptorJson = "{}"u8.ToArray(); + ContractDescriptorHelpers.Fill(descriptor, targetTestHelpers.Arch, descriptorJson.Length, 0xdddddddd, 0, 0xeeeeeeee); + // Corrupt the magic so the bytes at the descriptor address are not a recognized contract descriptor. + descriptor[0] ^= 0xFF; + + FormatException ex = Assert.Throws(() => builder.CreateTargetFromRawDescriptor(descriptor, descriptorJson, [])); + Assert.Equal(CdacHResults.CDAC_E_DESCRIPTOR_NOT_FOUND, ex.HResult); } [Theory] @@ -259,7 +276,8 @@ public void Create_InvalidPointerDataIndex_ThrowsFormatException(MockTarget.Arch descriptorBuilder.SetGlobals([("invalid", null, (uint?)1, null, "pointer")]) .SetIndirectValues([0]); - Assert.Throws(() => builder.CreateTarget(descriptorBuilder)); + FormatException ex = Assert.Throws(() => builder.CreateTarget(descriptorBuilder)); + Assert.Equal(CdacHResults.CDAC_E_DESCRIPTOR_MALFORMED, ex.HResult); } [Theory] @@ -348,6 +366,168 @@ public void TryGetContract_UnrecognizedVersion_ReturnsContractUnrecognizedExcept Assert.Equal("unsupported-version", ex.ContractVersion); } + // The contracts required by the data-access interfaces, advertised at the versions + // CoreCLRContracts registers. Mirrors CoreCLRContracts.ValidateForDataAccess. + private static readonly string[] s_requiredDataAccessContracts = + [ + "AuxiliarySymbols", "BuiltInCOM", "CodeNotifications", "CodeVersions", "ComWrappers", + "ConditionalWeakTable", "DacStreams", "Debugger", "EcmaMetadata", "Exception", + "ExecutionManager", "GC", "GCInfo", "Loader", "Notifications", "Object", "PlatformMetadata", + "PrecodeStubs", "ReJIT", "RuntimeInfo", "RuntimeTypeSystem", "SHash", "Signature", + "StackWalk", "StressLog", "SyncBlock", "Thread", + ]; + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void TryValidate_RegisteredVersion_ReturnsTrue(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(new Dictionary { ["RuntimeInfo"] = "c1" }); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + Assert.True(target.Contracts.TryValidate(out System.Exception? failure)); + Assert.Null(failure); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void TryValidate_DoesNotInstantiateContract(MockTarget.Architecture arch) + { + // GCInfo's creator reads RuntimeInfo from target memory. Advertise GCInfo but omit RuntimeInfo: + // TryValidate must succeed (a creator is registered) without invoking it, whereas a real + // GetContract would chain into RuntimeInfo and fail. + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(new Dictionary { ["GCInfo"] = "c1" }); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + Assert.True(target.Contracts.TryValidate(out System.Exception? failure)); + Assert.Null(failure); + Assert.Throws(() => target.Contracts.GCInfo); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void TryValidate_MissingContract_ReturnsContractMissingException(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts([]); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + Assert.False(target.Contracts.TryValidate(out System.Exception? failure)); + Assert.IsType(failure); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void ValidateForDataAccess_AllRequiredPresent_DoesNotThrow(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(s_requiredDataAccessContracts); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void ValidateForDataAccess_MissingRequiredContract_ThrowsNotAdvertised(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(s_requiredDataAccessContracts.Where(static c => c != "RuntimeInfo").ToArray()); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + ContractValidationException ex = Assert.Throws( + () => Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts)); + Assert.Equal(CdacHResults.CDAC_E_CONTRACT_NOT_ADVERTISED, ex.HResult); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.IsType(ex.InnerException); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void ValidateForDataAccess_MissingTransitiveContract_ThrowsNotAdvertised(MockTarget.Architecture arch) + { + string[] transitiveDependencies = ["ConditionalWeakTable", "Debugger", "SHash"]; + + foreach (string missingContract in transitiveDependencies) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + descriptorBuilder.SetContracts(s_requiredDataAccessContracts.Where(c => c != missingContract).ToArray()); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + ContractValidationException ex = Assert.Throws( + () => Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts)); + Assert.Equal(CdacHResults.CDAC_E_CONTRACT_NOT_ADVERTISED, ex.HResult); + Assert.Equal(missingContract, ex.ContractName); + Assert.IsType(ex.InnerException); + } + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void ValidateForDataAccess_UnrecognizedRequiredVersion_ThrowsUnrecognized(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + Dictionary contracts = s_requiredDataAccessContracts.ToDictionary(static c => c, static _ => "c1"); + contracts["RuntimeInfo"] = "version-from-the-future"; + descriptorBuilder.SetContracts(contracts); + + Assert.True(builder.TryCreateTarget(descriptorBuilder, out ContractDescriptorTarget? target)); + + ContractValidationException ex = Assert.Throws( + () => Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts)); + Assert.Equal(CdacHResults.CDAC_E_CONTRACT_UNRECOGNIZED, ex.HResult); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.Equal("version-from-the-future", ex.ContractVersion); + Assert.IsType(ex.InnerException); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void ValidateForDataAccess_DeprecatedContractReference_ThrowsUnsupported(MockTarget.Architecture arch) + { + TargetTestHelpers targetTestHelpers = new(arch); + ContractDescriptorBuilder builder = new(targetTestHelpers); + ContractDescriptorBuilder.DescriptorBuilder descriptorBuilder = new(builder); + Dictionary contracts = s_requiredDataAccessContracts.ToDictionary(static c => c, static _ => "c1"); + contracts["RuntimeInfo"] = "deprecated-version"; + descriptorBuilder.SetContracts(contracts); + + // Model a target descriptor that still references a contract version this cDAC recognizes + // as deprecated. Production registrations can use the same mechanism when a version retires. + Assert.True(builder.TryCreateTarget( + descriptorBuilder, + out ContractDescriptorTarget? target, + registry => registry.RegisterUnsupported("deprecated-version"))); + + ContractValidationException ex = Assert.Throws( + () => Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts)); + Assert.Equal(CdacHResults.CDAC_E_CONTRACT_UNSUPPORTED, ex.HResult); + Assert.Equal("RuntimeInfo", ex.ContractName); + Assert.Equal("deprecated-version", ex.ContractVersion); + Assert.IsType(ex.InnerException); + } + private static void ValidateGlobals( ContractDescriptorTarget target, (string Name, ulong Value, string? Type)[] globals, From 6625b43bd31422c9f1284535e7fe4f5973213d15 Mon Sep 17 00:00:00 2001 From: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:44:45 -0700 Subject: [PATCH 3/5] Add DacDbiInterfaceInstance to cdac (#130856) Squash the two commits from dotnet/runtime#130856 into one isolated change so it can be reverted independently. --- .../Dbi/DacDbiImpl.cs | 4 +- .../Dbi/IDacDbiInterface.cs | 14 +++ .../ICLRData.cs | 8 ++ .../mscordaccore_universal/Entrypoints.cs | 96 +++++++++++++++++++ .../cdac/tests/UnitTests/DacDbiImplTests.cs | 11 +++ 5 files changed, 132 insertions(+), 1 deletion(-) 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..7469d332124916 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 @@ -72,7 +72,9 @@ public int FlushCache() } public int DacSetTargetConsistencyChecks(Interop.BOOL fEnableAsserts) - => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.DacSetTargetConsistencyChecks(fEnableAsserts) : HResults.E_NOTIMPL; + => LegacyFallbackHelper.CanFallback() && _legacy is not null + ? _legacy.DacSetTargetConsistencyChecks(fEnableAsserts) + : HResults.S_OK; public int IsLeftSideInitialized(Interop.BOOL* pResult) { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs index 66cdc0ca6c121e..cc96dd9fc20abd 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs @@ -8,6 +8,20 @@ namespace Microsoft.Diagnostics.DataContractReader.Legacy; +[GeneratedComInterface] +[Guid("FE06DC28-49FB-4636-A4A3-E80DB4AE116C")] +public unsafe partial interface ICorDebugDataTarget +{ + [PreserveSig] + int GetPlatform(int* pTargetPlatform); + + [PreserveSig] + int ReadVirtual(ulong address, byte* pBuffer, uint bytesRequested, uint* pBytesRead); + + [PreserveSig] + int GetThreadContext(uint threadId, uint contextFlags, uint contextSize, byte* pContext); +} + [StructLayout(LayoutKind.Sequential)] public struct COR_TYPEID { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs index 838180d4323edd..2e81d2e00a9f63 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs @@ -81,6 +81,14 @@ public unsafe partial interface ICLRDataTarget3 : ICLRDataTarget2 int GetExceptionThreadID(uint* threadID); } +[GeneratedComInterface] +[Guid("b760bf44-9377-4597-8be7-58083bdc5146")] +public unsafe partial interface ICLRRuntimeLocator +{ + [PreserveSig] + int GetRuntimeBase(ulong* baseAddress); +} + [GeneratedComInterface] [Guid("17d5b8c6-34a9-407f-af4f-a930201d4e02")] public unsafe partial interface ICLRContractLocator diff --git a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs index f5ed98e4beb0cf..4cabab42f76197 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs @@ -256,6 +256,53 @@ private static unsafe int CLRDataCreateInstanceWithFallback(Guid* pIID, IntPtr / return CLRDataCreateInstanceImpl(pIID, pLegacyTarget, pLegacyImpl, iface); } + [UnmanagedCallersOnly(EntryPoint = "DacDbiInterfaceInstance")] + private static unsafe int DacDbiInterfaceInstance( + IntPtr /*ICorDebugDataTarget*/ pTarget, + ulong runtimeBase, + IntPtr /*IDacDbiInterface::IAllocator*/ pAllocator, + IntPtr /*IDacDbiInterface::IMetaDataLookup*/ pMetaDataLookup, + void** iface) + { + // Match the native DAC export (DacDbiInterfaceInstance in dacdbiimpl.cpp), which only + // validates the target, base address, and out parameter. The allocator and metadata + // lookup pointers are not used by the managed implementation, so don't require them. + if (pTarget == IntPtr.Zero + || runtimeBase == 0 + || iface == null) + { + return HResults.E_INVALIDARG; + } + + *iface = null; + + try + { + object dataTarget = ComInterfaceMarshaller.ConvertToManaged((void*)pTarget)!; + if (dataTarget is ICLRRuntimeLocator runtimeLocator) + { + ulong locatedRuntimeBase; + int hr = runtimeLocator.GetRuntimeBase(&locatedRuntimeBase); + if (hr < 0) + return hr; + if (locatedRuntimeBase != runtimeBase) + return HResults.E_INVALIDARG; + } + + ContractDescriptorTarget target = CreateTargetFromCorDebugDataTarget(dataTarget); + Legacy.DacDbiImpl impl = new(target, legacyObj: null); + *iface = ComInterfaceMarshaller.ConvertToUnmanaged(impl); + return HResults.S_OK; + } + catch (Exception ex) + { + if (iface != null) + *iface = null; + int hr = ex.HResult; + return hr < 0 ? hr : HResults.E_FAIL; + } + } + // Same export name and signature as DAC CLRDataCreateInstance in daccess.cpp [UnmanagedCallersOnly(EntryPoint = "CLRDataCreateInstance")] private static unsafe int CLRDataCreateInstance(Guid* pIID, IntPtr /*ICLRDataTarget*/ pLegacyTarget, void** iface) @@ -392,4 +439,53 @@ private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDat return 0; } + + private static unsafe ContractDescriptorTarget CreateTargetFromCorDebugDataTarget(object targetObject) + { + ICorDebugDataTarget dataTarget = targetObject as ICorDebugDataTarget ?? throw new ArgumentException( + $"Data target does not implement {nameof(ICorDebugDataTarget)}", nameof(targetObject)); + ICLRContractLocator contractLocator = targetObject as ICLRContractLocator ?? throw new ArgumentException( + $"Data target does not implement {nameof(ICLRContractLocator)}", nameof(targetObject)); + + ulong contractAddress; + int hr = contractLocator.GetContractDescriptor(&contractAddress); + if (hr != 0) + { + throw new InvalidOperationException( + $"{nameof(ICLRContractLocator)} failed to fetch the contract descriptor with HRESULT: 0x{hr:x}."); + } + + if (!ContractDescriptorTarget.TryCreate( + contractAddress, + (address, buffer) => + { + fixed (byte* bufferPtr = buffer) + { + uint bytesRead; + return dataTarget.ReadVirtual(address, bufferPtr, (uint)buffer.Length, &bytesRead); + } + }, + (address, buffer) => HResults.E_NOTIMPL, + (threadId, contextFlags, bufferToFill) => + { + fixed (byte* bufferPtr = bufferToFill) + { + return dataTarget.GetThreadContext(threadId, contextFlags, (uint)bufferToFill.Length, bufferPtr); + } + }, + (threadId, context) => HResults.E_NOTIMPL, + (ulong size, out ulong allocatedAddress) => + { + allocatedAddress = 0; + return HResults.E_NOTIMPL; + }, + [Contracts.CoreCLRContracts.Register], + out ContractDescriptorTarget? target)) + { + throw new InvalidOperationException( + $"Failed to create a {nameof(ContractDescriptorTarget)} from the contract descriptor at 0x{contractAddress:x}."); + } + + return target!; + } } diff --git a/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs b/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs index b5ebff64043f28..1f91dd724dc280 100644 --- a/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs @@ -37,6 +37,17 @@ private static (DacDbiImpl DacDbi, TestPlaceholderTarget Target) CreateDacDbiWit return (dacDbi, target); } + [Fact] + public void DacSetTargetConsistencyChecks_Standalone_ReturnsSuccess() + { + MockTarget.Architecture architecture = new() { IsLittleEndian = true, Is64Bit = true }; + TestPlaceholderTarget target = new TestPlaceholderTarget.Builder(architecture).Build(); + DacDbiImpl dacDbi = new(target, legacyObj: null); + + Assert.Equal(System.HResults.S_OK, dacDbi.DacSetTargetConsistencyChecks(Interop.BOOL.TRUE)); + Assert.Equal(System.HResults.S_OK, dacDbi.DacSetTargetConsistencyChecks(Interop.BOOL.FALSE)); + } + [Theory] [ClassData(typeof(MockTarget.StdArch))] public void SetCompilerFlags_BothFlagsSet_EncCapable(MockTarget.Architecture arch) From 33d9d7baf74061d7b8bae065b8122858e9a319f9 Mon Sep 17 00:00:00 2001 From: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:34:42 -0700 Subject: [PATCH 4/5] Revert "Add DacDbiInterfaceInstance to cdac (#130856)" This reverts commit f1eba34a94ff9238769dccb469bdb942fe0569d1. --- .../Dbi/DacDbiImpl.cs | 4 +- .../Dbi/IDacDbiInterface.cs | 14 --- .../ICLRData.cs | 8 -- .../mscordaccore_universal/Entrypoints.cs | 96 ------------------- .../cdac/tests/UnitTests/DacDbiImplTests.cs | 11 --- 5 files changed, 1 insertion(+), 132 deletions(-) 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 7469d332124916..0481f2c9bd129e 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 @@ -72,9 +72,7 @@ public int FlushCache() } public int DacSetTargetConsistencyChecks(Interop.BOOL fEnableAsserts) - => LegacyFallbackHelper.CanFallback() && _legacy is not null - ? _legacy.DacSetTargetConsistencyChecks(fEnableAsserts) - : HResults.S_OK; + => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.DacSetTargetConsistencyChecks(fEnableAsserts) : HResults.E_NOTIMPL; public int IsLeftSideInitialized(Interop.BOOL* pResult) { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs index cc96dd9fc20abd..66cdc0ca6c121e 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs @@ -8,20 +8,6 @@ namespace Microsoft.Diagnostics.DataContractReader.Legacy; -[GeneratedComInterface] -[Guid("FE06DC28-49FB-4636-A4A3-E80DB4AE116C")] -public unsafe partial interface ICorDebugDataTarget -{ - [PreserveSig] - int GetPlatform(int* pTargetPlatform); - - [PreserveSig] - int ReadVirtual(ulong address, byte* pBuffer, uint bytesRequested, uint* pBytesRead); - - [PreserveSig] - int GetThreadContext(uint threadId, uint contextFlags, uint contextSize, byte* pContext); -} - [StructLayout(LayoutKind.Sequential)] public struct COR_TYPEID { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs index 2e81d2e00a9f63..838180d4323edd 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ICLRData.cs @@ -81,14 +81,6 @@ public unsafe partial interface ICLRDataTarget3 : ICLRDataTarget2 int GetExceptionThreadID(uint* threadID); } -[GeneratedComInterface] -[Guid("b760bf44-9377-4597-8be7-58083bdc5146")] -public unsafe partial interface ICLRRuntimeLocator -{ - [PreserveSig] - int GetRuntimeBase(ulong* baseAddress); -} - [GeneratedComInterface] [Guid("17d5b8c6-34a9-407f-af4f-a930201d4e02")] public unsafe partial interface ICLRContractLocator diff --git a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs index 4cabab42f76197..f5ed98e4beb0cf 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs @@ -256,53 +256,6 @@ private static unsafe int CLRDataCreateInstanceWithFallback(Guid* pIID, IntPtr / return CLRDataCreateInstanceImpl(pIID, pLegacyTarget, pLegacyImpl, iface); } - [UnmanagedCallersOnly(EntryPoint = "DacDbiInterfaceInstance")] - private static unsafe int DacDbiInterfaceInstance( - IntPtr /*ICorDebugDataTarget*/ pTarget, - ulong runtimeBase, - IntPtr /*IDacDbiInterface::IAllocator*/ pAllocator, - IntPtr /*IDacDbiInterface::IMetaDataLookup*/ pMetaDataLookup, - void** iface) - { - // Match the native DAC export (DacDbiInterfaceInstance in dacdbiimpl.cpp), which only - // validates the target, base address, and out parameter. The allocator and metadata - // lookup pointers are not used by the managed implementation, so don't require them. - if (pTarget == IntPtr.Zero - || runtimeBase == 0 - || iface == null) - { - return HResults.E_INVALIDARG; - } - - *iface = null; - - try - { - object dataTarget = ComInterfaceMarshaller.ConvertToManaged((void*)pTarget)!; - if (dataTarget is ICLRRuntimeLocator runtimeLocator) - { - ulong locatedRuntimeBase; - int hr = runtimeLocator.GetRuntimeBase(&locatedRuntimeBase); - if (hr < 0) - return hr; - if (locatedRuntimeBase != runtimeBase) - return HResults.E_INVALIDARG; - } - - ContractDescriptorTarget target = CreateTargetFromCorDebugDataTarget(dataTarget); - Legacy.DacDbiImpl impl = new(target, legacyObj: null); - *iface = ComInterfaceMarshaller.ConvertToUnmanaged(impl); - return HResults.S_OK; - } - catch (Exception ex) - { - if (iface != null) - *iface = null; - int hr = ex.HResult; - return hr < 0 ? hr : HResults.E_FAIL; - } - } - // Same export name and signature as DAC CLRDataCreateInstance in daccess.cpp [UnmanagedCallersOnly(EntryPoint = "CLRDataCreateInstance")] private static unsafe int CLRDataCreateInstance(Guid* pIID, IntPtr /*ICLRDataTarget*/ pLegacyTarget, void** iface) @@ -439,53 +392,4 @@ private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDat return 0; } - - private static unsafe ContractDescriptorTarget CreateTargetFromCorDebugDataTarget(object targetObject) - { - ICorDebugDataTarget dataTarget = targetObject as ICorDebugDataTarget ?? throw new ArgumentException( - $"Data target does not implement {nameof(ICorDebugDataTarget)}", nameof(targetObject)); - ICLRContractLocator contractLocator = targetObject as ICLRContractLocator ?? throw new ArgumentException( - $"Data target does not implement {nameof(ICLRContractLocator)}", nameof(targetObject)); - - ulong contractAddress; - int hr = contractLocator.GetContractDescriptor(&contractAddress); - if (hr != 0) - { - throw new InvalidOperationException( - $"{nameof(ICLRContractLocator)} failed to fetch the contract descriptor with HRESULT: 0x{hr:x}."); - } - - if (!ContractDescriptorTarget.TryCreate( - contractAddress, - (address, buffer) => - { - fixed (byte* bufferPtr = buffer) - { - uint bytesRead; - return dataTarget.ReadVirtual(address, bufferPtr, (uint)buffer.Length, &bytesRead); - } - }, - (address, buffer) => HResults.E_NOTIMPL, - (threadId, contextFlags, bufferToFill) => - { - fixed (byte* bufferPtr = bufferToFill) - { - return dataTarget.GetThreadContext(threadId, contextFlags, (uint)bufferToFill.Length, bufferPtr); - } - }, - (threadId, context) => HResults.E_NOTIMPL, - (ulong size, out ulong allocatedAddress) => - { - allocatedAddress = 0; - return HResults.E_NOTIMPL; - }, - [Contracts.CoreCLRContracts.Register], - out ContractDescriptorTarget? target)) - { - throw new InvalidOperationException( - $"Failed to create a {nameof(ContractDescriptorTarget)} from the contract descriptor at 0x{contractAddress:x}."); - } - - return target!; - } } diff --git a/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs b/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs index 1f91dd724dc280..b5ebff64043f28 100644 --- a/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs +++ b/src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs @@ -37,17 +37,6 @@ private static (DacDbiImpl DacDbi, TestPlaceholderTarget Target) CreateDacDbiWit return (dacDbi, target); } - [Fact] - public void DacSetTargetConsistencyChecks_Standalone_ReturnsSuccess() - { - MockTarget.Architecture architecture = new() { IsLittleEndian = true, Is64Bit = true }; - TestPlaceholderTarget target = new TestPlaceholderTarget.Builder(architecture).Build(); - DacDbiImpl dacDbi = new(target, legacyObj: null); - - Assert.Equal(System.HResults.S_OK, dacDbi.DacSetTargetConsistencyChecks(Interop.BOOL.TRUE)); - Assert.Equal(System.HResults.S_OK, dacDbi.DacSetTargetConsistencyChecks(Interop.BOOL.FALSE)); - } - [Theory] [ClassData(typeof(MockTarget.StdArch))] public void SetCompilerFlags_BothFlagsSet_EncCapable(MockTarget.Architecture arch) From 3200f0568fb0918d999d046ba15df6de968e38b5 Mon Sep 17 00:00:00 2001 From: Juan Hoyos <19413848+hoyosjs@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:39:23 -0700 Subject: [PATCH 5/5] Add dbi validation on top of dacdbi entrypoint --- .../cdac/mscordaccore_universal/Entrypoints.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs index 4cabab42f76197..fae25b4c1a2fba 100644 --- a/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs +++ b/src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs @@ -290,6 +290,7 @@ private static unsafe int DacDbiInterfaceInstance( } ContractDescriptorTarget target = CreateTargetFromCorDebugDataTarget(dataTarget); + Contracts.CoreCLRContracts.ValidateForDataAccess(target.Contracts); Legacy.DacDbiImpl impl = new(target, legacyObj: null); *iface = ComInterfaceMarshaller.ConvertToUnmanaged(impl); return HResults.S_OK; @@ -455,7 +456,7 @@ private static unsafe ContractDescriptorTarget CreateTargetFromCorDebugDataTarge $"{nameof(ICLRContractLocator)} failed to fetch the contract descriptor with HRESULT: 0x{hr:x}."); } - if (!ContractDescriptorTarget.TryCreate( + return ContractDescriptorTarget.Create( contractAddress, (address, buffer) => { @@ -479,13 +480,6 @@ private static unsafe ContractDescriptorTarget CreateTargetFromCorDebugDataTarge allocatedAddress = 0; return HResults.E_NOTIMPL; }, - [Contracts.CoreCLRContracts.Register], - out ContractDescriptorTarget? target)) - { - throw new InvalidOperationException( - $"Failed to create a {nameof(ContractDescriptorTarget)} from the contract descriptor at 0x{contractAddress:x}."); - } - - return target!; + [Contracts.CoreCLRContracts.Register]); } }