From 8b535b9051423cef3ad367a8afff7a20dd497f18 Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Wed, 13 May 2026 01:42:03 -0400 Subject: [PATCH] [cdac] Move DataType from Abstractions to Contracts Moves the DataType enum out of the Abstractions assembly and into Contracts, since it is part of the descriptor schema rather than the abstract target API. Target.GetTypeInfo(DataType) is replaced with a string-based abstract Target.GetTypeInfo(string) plus an extension method DataTypeTargetExtensions.GetTypeInfo(this Target, DataType) that calls .ToString(). Target.FieldInfo.Type is removed; TypeName is the sole identifier. TestPlaceholderTarget storage is now string-keyed internally. Builder.AddTypes(Dictionary) is kept for caller convenience. Also removes the unused TargetTestHelpers.SizeOfTypeInfo helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Target.cs | 8 +--- .../DataType.cs | 6 +++ .../ContractDescriptorTarget.cs | 40 +++---------------- .../TargetTests.SubDescriptors.cs | 4 +- .../tests/ContractDescriptor/TargetTests.cs | 4 +- .../managed/cdac/tests/DebuggerTests.cs | 2 +- .../tests/ExecutionManager/HashMapTests.cs | 13 +++--- .../managed/cdac/tests/GCMemoryRegionTests.cs | 2 +- .../managed/cdac/tests/MethodDescTests.cs | 2 +- .../managed/cdac/tests/TargetTestHelpers.cs | 14 +------ .../cdac/tests/TestPlaceholderTarget.cs | 17 +++++--- 11 files changed, 39 insertions(+), 73 deletions(-) rename src/native/managed/cdac/{Microsoft.Diagnostics.DataContractReader.Abstractions => Microsoft.Diagnostics.DataContractReader.Contracts}/DataType.cs (95%) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs index fc994d02521f0e..d8b01775a3107a 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Target.cs @@ -234,9 +234,9 @@ public virtual TargetPointer AllocateMemory(uint size) /// /// Returns the information about the given well-known data type in the target process /// - /// The name of the well known type + /// The name of the well known type /// The information about the given type in the target process - public abstract TypeInfo GetTypeInfo(DataType type); + public abstract TypeInfo GetTypeInfo(string typeName); /// /// Get the data cache for the target @@ -304,10 +304,6 @@ public readonly record struct FieldInfo /// public int Offset { get; init; } /// - /// The well known data type of the field in the target process - /// - public readonly DataType Type { get; init; } - /// /// The name of the well known data type of the field in the target process, or null /// if the target data descriptor did not record a name /// diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs similarity index 95% rename from src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs rename to src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs index b3802a8bcc03bc..4cf582abcdd40d 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs @@ -197,3 +197,9 @@ public enum DataType CardTableInfo, RegionFreeList, } + +public static class DataTypeTargetExtensions +{ + public static Target.TypeInfo GetTypeInfo(this Target target, DataType type) + => target.GetTypeInfo(type.ToString()); +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs index 47eb8bff46456c..e6be36f80343a8 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/ContractDescriptorTarget.cs @@ -38,7 +38,6 @@ private readonly struct Configuration private readonly DataTargetDelegates _dataTargetDelegates; private readonly Dictionary _contracts = []; private readonly IReadOnlyDictionary _globals = new Dictionary(); - private readonly Dictionary _knownTypes = []; private readonly Dictionary _types = []; public override ContractRegistry Contracts { get; } @@ -133,7 +132,7 @@ private ContractDescriptorTarget(Descriptor[] descriptors, DataTargetDelegates d _contracts = []; // Set pointer type size - _knownTypes[DataType.pointer] = new TypeInfo { Size = (uint)_config.PointerSize }; + _types[DataType.pointer.ToString()] = new TypeInfo { Size = (uint)_config.PointerSize }; HashSet seenTypeNames = new HashSet(); HashSet seenGlobalNames = new HashSet(); @@ -170,7 +169,6 @@ private ContractDescriptorTarget(Descriptor[] descriptors, DataTargetDelegates d fieldInfos[fieldName] = new Target.FieldInfo() { Offset = field.Offset, - Type = field.Type is null ? DataType.Unknown : GetDataType(field.Type), TypeName = field.Type }; } @@ -183,15 +181,7 @@ private ContractDescriptorTarget(Descriptor[] descriptors, DataTargetDelegates d } seenTypeNames.Add(name); - DataType dataType = GetDataType(name); - if (dataType is not DataType.Unknown) - { - _knownTypes[dataType] = typeInfo; - } - else - { - _types[name] = typeInfo; - } + _types[name] = typeInfo; } } @@ -382,14 +372,6 @@ private static bool TryReadContractDescriptor( return true; } - private static DataType GetDataType(string type) - { - if (Enum.TryParse(type, false, out DataType dataType) && Enum.IsDefined(dataType)) - return dataType; - - return DataType.Unknown; - } - public override int PointerSize => _config.PointerSize; public override bool IsLittleEndian => _config.IsLittleEndian; @@ -583,7 +565,7 @@ public override TargetPointer ReadPointerFromSpan(ReadOnlySpan bytes) public override TargetCodePointer ReadCodePointer(ulong address) { - TypeInfo codePointerTypeInfo = GetTypeInfo(DataType.CodePointer); + TypeInfo codePointerTypeInfo = this.GetTypeInfo(DataType.CodePointer); if (codePointerTypeInfo.Size is sizeof(uint)) { return new TargetCodePointer(Read(address)); @@ -597,7 +579,7 @@ public override TargetCodePointer ReadCodePointer(ulong address) public override bool TryReadCodePointer(ulong address, out TargetCodePointer value) { - TypeInfo codePointerTypeInfo = GetTypeInfo(DataType.CodePointer); + TypeInfo codePointerTypeInfo = this.GetTypeInfo(DataType.CodePointer); if (codePointerTypeInfo.Size is sizeof(uint)) { if (TryRead(address, out uint val)) @@ -821,23 +803,11 @@ public bool TryReadStringGlobal(string name, [NotNullWhen(true)] out string? val #endregion - public override TypeInfo GetTypeInfo(DataType type) - { - if (!_knownTypes.TryGetValue(type, out Target.TypeInfo typeInfo)) - throw new InvalidOperationException($"Failed to get type info for '{type}'"); - - return typeInfo; - } - - public Target.TypeInfo GetTypeInfo(string type) + public override Target.TypeInfo GetTypeInfo(string type) { if (_types.TryGetValue(type, out Target.TypeInfo typeInfo)) return typeInfo; - DataType dataType = GetDataType(type); - if (dataType is not DataType.Unknown) - return GetTypeInfo(dataType); - throw new InvalidOperationException($"Failed to get type info for '{type}'"); } diff --git a/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.SubDescriptors.cs b/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.SubDescriptors.cs index d5f0b0880229bb..960569882c104c 100644 --- a/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.SubDescriptors.cs +++ b/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.SubDescriptors.cs @@ -23,8 +23,8 @@ public unsafe partial class TargetTests { Size = 56, Fields = new Dictionary { - { "Field1", new(){ Offset = 8, Type = DataType.uint16, TypeName = DataType.uint16.ToString() }}, - { "Field2", new(){ Offset = 16, Type = DataType.ObjectHandle, TypeName = DataType.ObjectHandle.ToString() }}, + { "Field1", new(){ Offset = 8, TypeName = DataType.uint16.ToString() }}, + { "Field2", new(){ Offset = 16, TypeName = DataType.ObjectHandle.ToString() }}, { "Field3", new(){ Offset = 32 }} } }, diff --git a/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.cs b/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.cs index 43cc38592d1157..b42b9492aa5386 100644 --- a/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.cs +++ b/src/native/managed/cdac/tests/ContractDescriptor/TargetTests.cs @@ -19,8 +19,8 @@ public unsafe partial class TargetTests { Size = 56, Fields = new Dictionary { - { "Field1", new(){ Offset = 8, Type = DataType.uint16, TypeName = DataType.uint16.ToString() }}, - { "Field2", new(){ Offset = 16, Type = DataType.ObjectHandle, TypeName = DataType.ObjectHandle.ToString() }}, + { "Field1", new(){ Offset = 8, TypeName = DataType.uint16.ToString() }}, + { "Field2", new(){ Offset = 16, TypeName = DataType.ObjectHandle.ToString() }}, { "Field3", new(){ Offset = 32 }} } }, diff --git a/src/native/managed/cdac/tests/DebuggerTests.cs b/src/native/managed/cdac/tests/DebuggerTests.cs index ef2dc7f8c71af9..b50d489787cbaf 100644 --- a/src/native/managed/cdac/tests/DebuggerTests.cs +++ b/src/native/managed/cdac/tests/DebuggerTests.cs @@ -65,7 +65,7 @@ private static TestPlaceholderTarget BuildTarget( TargetTestHelpers.LayoutResult debuggerLayout = GetDebuggerLayout(helpers); TargetTestHelpers.LayoutResult debuggerRcThreadLayout = GetDebuggerRCThreadLayout(helpers); - builder.AddTypes(new() + builder.AddTypes(new Dictionary { [DataType.Debugger] = new Target.TypeInfo() { Fields = debuggerLayout.Fields, Size = debuggerLayout.Stride }, [DataType.DebuggerRCThread] = new Target.TypeInfo() { Fields = debuggerRcThreadLayout.Fields, Size = debuggerRcThreadLayout.Stride }, diff --git a/src/native/managed/cdac/tests/ExecutionManager/HashMapTests.cs b/src/native/managed/cdac/tests/ExecutionManager/HashMapTests.cs index e3a34815667563..499f74c8411f21 100644 --- a/src/native/managed/cdac/tests/ExecutionManager/HashMapTests.cs +++ b/src/native/managed/cdac/tests/ExecutionManager/HashMapTests.cs @@ -28,15 +28,14 @@ private static Target CreateTarget( MockTarget.Architecture arch, Action configure) { - MockMemorySpace.Builder builder = new(new TargetTestHelpers(arch)); - MockHashMapBuilder hashMap = new(builder); + TestPlaceholderTarget.Builder builder = new(arch); + MockHashMapBuilder hashMap = new(builder.MemoryBuilder); configure(hashMap); - return new TestPlaceholderTarget( - builder.TargetTestHelpers.Arch, - builder.GetMemoryContext().ReadFromTarget, - CreateContractTypes(hashMap), - CreateContractGlobals(hashMap)); + return builder + .AddTypes(CreateContractTypes(hashMap)) + .AddGlobals(CreateContractGlobals(hashMap)) + .Build(); } [Theory] diff --git a/src/native/managed/cdac/tests/GCMemoryRegionTests.cs b/src/native/managed/cdac/tests/GCMemoryRegionTests.cs index 3a8357b90ac8ee..f64c994e47e5bd 100644 --- a/src/native/managed/cdac/tests/GCMemoryRegionTests.cs +++ b/src/native/managed/cdac/tests/GCMemoryRegionTests.cs @@ -61,7 +61,7 @@ private static (string Name, ulong Value)[] BuildGlobals( { var result = new Dictionary(); foreach (var (name, offset, type) in fields) - result[name] = new Target.FieldInfo { Offset = offset, Type = type }; + result[name] = new Target.FieldInfo { Offset = offset, TypeName = type.ToString() }; return result; } diff --git a/src/native/managed/cdac/tests/MethodDescTests.cs b/src/native/managed/cdac/tests/MethodDescTests.cs index 3963750e6e26e6..31365e735fbc93 100644 --- a/src/native/managed/cdac/tests/MethodDescTests.cs +++ b/src/native/managed/cdac/tests/MethodDescTests.cs @@ -30,7 +30,7 @@ public class MethodDescTests Size = methodDescBuilder.AsyncMethodDataSize, Fields = new Dictionary { - [nameof(Data.AsyncMethodData.Flags)] = new Target.FieldInfo { Offset = 0, Type = DataType.Unknown }, + [nameof(Data.AsyncMethodData.Flags)] = new Target.FieldInfo { Offset = 0 }, }, }, [DataType.ArrayMethodDesc] = new Target.TypeInfo { Size = methodDescBuilder.ArrayMethodDescSize }, diff --git a/src/native/managed/cdac/tests/TargetTestHelpers.cs b/src/native/managed/cdac/tests/TargetTestHelpers.cs index 4f8daf5bed49e7..5438d11b412cf8 100644 --- a/src/native/managed/cdac/tests/TargetTestHelpers.cs +++ b/src/native/managed/cdac/tests/TargetTestHelpers.cs @@ -147,17 +147,6 @@ internal int SizeOfPrimitive(DataType type) }; } - internal int SizeOfTypeInfo(Target.TypeInfo info) - { - int size = 0; - foreach (var (_, field) in info.Fields) - { - size = Math.Max(size, field.Offset + SizeOfPrimitive(field.Type)); - } - - return size; - } - #endregion Mock memory initialization private static int AlignUp(int offset, int align) @@ -215,7 +204,7 @@ private LayoutResult LayoutFieldsWorker(FieldLayout style, Field[] fields, ref i }; fieldInfos[name] = new Target.FieldInfo { Offset = offset, - Type = type, + TypeName = type.ToString(), }; offset += size; } @@ -245,7 +234,6 @@ internal static Target.TypeInfo CreateTypeInfo(Layout layout) fields[field.Name] = new Target.FieldInfo { Offset = field.Offset, - Type = DataType.Unknown, }; } diff --git a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs index 0dfa59aed222fd..af56f5c7dc8291 100644 --- a/src/native/managed/cdac/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdac/tests/TestPlaceholderTarget.cs @@ -19,7 +19,7 @@ internal class TestPlaceholderTarget : Target { private ContractRegistry _contractRegistry; private readonly Target.IDataCache _dataCache; - private readonly Dictionary _typeInfoCache; + private readonly Dictionary _typeInfoCache; private readonly (string Name, ulong Value)[] _globals; private readonly (string Name, string Value)[] _globalStrings; @@ -33,7 +33,7 @@ internal class TestPlaceholderTarget : Target private static readonly UTF8Encoding strictUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); private static readonly UTF8Encoding looseUTF8Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: false); - public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types = null, (string Name, ulong Value)[] globals = null, (string Name, string Value)[] globalStrings = null, WriteToTargetDelegate? writer = null, AllocateMemoryDelegate? allocateMemory = null) + public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types = null, (string Name, ulong Value)[] globals = null, (string Name, string Value)[] globalStrings = null, WriteToTargetDelegate? writer = null, AllocateMemoryDelegate? allocateMemory = null) { IsLittleEndian = arch.IsLittleEndian; PointerSize = arch.Is64Bit ? 8 : 4; @@ -77,7 +77,7 @@ internal class Builder { private readonly MockTarget.Architecture _arch; private readonly MockMemorySpace.Builder _memBuilder; - private readonly Dictionary _types = new(); + private readonly Dictionary _types = new(); private readonly List<(string Name, ulong Value)> _globals = new(); private readonly List<(string Name, string Value)> _globalStrings = new(); private readonly List> _contractSetups = new(); @@ -94,6 +94,13 @@ public Builder(MockTarget.Architecture arch) internal MockMemorySpace.Builder MemoryBuilder => _memBuilder; public Builder AddTypes(Dictionary types) + { + foreach (var kvp in types) + _types[kvp.Key.ToString()] = kvp.Value; + return this; + } + + public Builder AddTypes(Dictionary types) { foreach (var kvp in types) _types[kvp.Key] = kvp.Value; @@ -499,9 +506,9 @@ protected TargetCodePointer DefaultReadCodePointer(ulong address) public override TargetPointer ReadPointerFromSpan(ReadOnlySpan bytes) => throw new NotImplementedException(); - public override Target.TypeInfo GetTypeInfo(DataType dataType) + public override Target.TypeInfo GetTypeInfo(string typeName) { - if (_typeInfoCache.TryGetValue(dataType, out var info)) + if (_typeInfoCache.TryGetValue(typeName, out var info)) return info; throw new NotImplementedException();