From 48adc33c159bc63c2e7cf52b083930984232059a Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Wed, 24 Jun 2026 20:15:32 -0400 Subject: [PATCH] [cdac] Add default (empty-version) contract registration logic Establishes a convention for registering a contract implementation that is used when the target does not advertise a version for that contract: - Register("", creator) registers an implementation under the empty-string version. - In TryGetContract, when the target does not advertise a version for the contract, the registry falls back to the empty-string registration if one is present. - When the target does advertise a version, an exact-version match is still required and a missing implementation fails (the fallback does not kick in), preserving version-skew detection. This lets host-side contracts (which have no target-side data and are not advertised by the target's contract descriptor) be wired into the existing ContractRegistry.Register(version, creator) API without adding a new method. Adds unit tests that exercise the production CachingContractRegistry through a real ContractDescriptorTarget (extending ContractDescriptorBuilder.TryCreateTarget to accept custom contract registrations). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CachingContractRegistry.cs | 15 ++- .../ContractDescriptorBuilder.cs | 4 +- .../ContractRegistrationTests.cs | 116 ++++++++++++++++++ 3 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs index 376b680f35966c..4ca7582501f6b5 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs @@ -48,15 +48,18 @@ public override bool TryGetContract([NotNullWhen(true)] out TContract return true; } - if (!_tryGetContractVersion(TContract.Name, out string? version)) + Func? creator; + if (_tryGetContractVersion(TContract.Name, out string? version)) { - failureReason = $"Target does not support contract '{typeof(TContract).Name}'."; - return false; + 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."; + return false; + } } - - if (!_creators.TryGetValue((typeof(TContract), version), out Func? creator)) + else if (!_creators.TryGetValue((typeof(TContract), string.Empty), out creator)) { - failureReason = $"Target supports contract '{typeof(TContract).Name}' version {version}, but no implementation is registered for that version."; + failureReason = $"Target does not support contract '{typeof(TContract).Name}'."; return false; } diff --git a/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs b/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs index 411d7203087de4..93ca07995d1b8f 100644 --- a/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs +++ b/src/native/managed/cdac/tests/TestInfrastructure/ContractDescriptor/ContractDescriptorBuilder.cs @@ -202,13 +202,13 @@ private string MakeContractsJson() } } - public bool TryCreateTarget(DescriptorBuilder descriptor, [NotNullWhen(true)] out ContractDescriptorTarget? target) + public bool TryCreateTarget(DescriptorBuilder descriptor, [NotNullWhen(true)] out ContractDescriptorTarget? target, Action[]? contractRegistrations = null) { 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"), (ulong _, out ulong _) => throw new NotImplementedException("Tests do not provide AllocVirtual"), [Contracts.CoreCLRContracts.Register], out target); + return ContractDescriptorTarget.TryCreate(contractDescriptorAddress, memoryContext.ReadFromTarget, memoryContext.WriteToTarget, (_, _, _) => throw new NotImplementedException("Tests do not provide GetTargetThreadContext"), (ulong _, out ulong _) => throw new NotImplementedException("Tests do not provide AllocVirtual"), contractRegistrations ?? [Contracts.CoreCLRContracts.Register], out target); } } diff --git a/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs new file mode 100644 index 00000000000000..69d207051d87c5 --- /dev/null +++ b/src/native/managed/cdac/tests/UnitTests/ContractDescriptor/ContractRegistrationTests.cs @@ -0,0 +1,116 @@ +// 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.Collections.Generic; +using Microsoft.Diagnostics.DataContractReader.Contracts; +using Microsoft.Diagnostics.DataContractReader.TestInfrastructure; +using Microsoft.Diagnostics.DataContractReader.TestInfrastructure.ContractDescriptor; +using Xunit; + +namespace Microsoft.Diagnostics.DataContractReader.Tests.ContractDescriptor; + +/// +/// Tests contract-version resolution through a real +/// (and thus the production CachingContractRegistry), in particular the +/// empty-string "default" registration used as a fallback when the target does not +/// advertise a version for a contract. +/// +public class ContractRegistrationTests +{ + private interface IFakeContract : IContract + { + static string IContract.Name { get; } = "FakeContract"; + string Tag { get; } + } + + private sealed class FakeContract(string tag) : IFakeContract + { + public string Tag => tag; + } + + private static ContractDescriptorTarget CreateTarget( + MockTarget.Architecture arch, + string[] advertisedContracts, + Action registerFake) + { + TargetTestHelpers helpers = new(arch); + ContractDescriptorBuilder builder = new(helpers); + ContractDescriptorBuilder.DescriptorBuilder descriptor = new(builder); + descriptor.SetTypes(new Dictionary()) + .SetGlobals(Array.Empty<(string, ulong, string?)>()) + .SetContracts(advertisedContracts); + + bool created = builder.TryCreateTarget( + descriptor, + out ContractDescriptorTarget? target, + [Contracts.CoreCLRContracts.Register, registerFake]); + Assert.True(created); + return target!; + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void NoAdvertisedVersion_FallsBackToDefaultRegistration(MockTarget.Architecture arch) + { + // Target advertises no version for FakeContract -> the empty-string + // "default" registration is used. + ContractDescriptorTarget target = CreateTarget( + arch, + advertisedContracts: [], + registerFake: static r => r.Register(string.Empty, static t => new FakeContract("default"))); + + Assert.True(target.Contracts.TryGetContract(out IFakeContract? contract)); + Assert.Equal("default", contract.Tag); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void NoAdvertisedVersion_NoDefaultRegistration_Fails(MockTarget.Architecture arch) + { + // FakeContract is neither advertised nor registered. + ContractDescriptorTarget target = CreateTarget( + arch, + advertisedContracts: [], + registerFake: static _ => { }); + + Assert.False(target.Contracts.TryGetContract(out IFakeContract? contract)); + Assert.Null(contract); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void AdvertisedVersion_UsesVersionedRegistration_NotDefault(MockTarget.Architecture arch) + { + // Target advertises FakeContract (the builder emits version "c1"); both a + // versioned and a default registration exist, and the advertised version + // must win. + ContractDescriptorTarget target = CreateTarget( + arch, + advertisedContracts: ["FakeContract"], + registerFake: static r => + { + r.Register("c1", static t => new FakeContract("v1")); + r.Register(string.Empty, static t => new FakeContract("default")); + }); + + Assert.True(target.Contracts.TryGetContract(out IFakeContract? contract)); + Assert.Equal("v1", contract.Tag); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void AdvertisedVersion_NoMatchingRegistration_DoesNotFallBackToDefault(MockTarget.Architecture arch) + { + // Target advertises FakeContract (version "c1"), but only a default ("") + // registration exists. This is a version-skew failure and must NOT + // silently use the default registration. + ContractDescriptorTarget target = CreateTarget( + arch, + advertisedContracts: ["FakeContract"], + registerFake: static r => r.Register(string.Empty, static t => new FakeContract("default"))); + + Assert.False(target.Contracts.TryGetContract(out IFakeContract? contract)); + Assert.Null(contract); + } +}