From 24cc4b896427e57a3b2584e2360bc034efcf40ef Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 13 Nov 2024 15:25:23 -0800 Subject: [PATCH 1/7] Make TestPlaceholderTarget constructor take a dictionary of type infos --- .../cdacreader/tests/CodeVersionsTests.cs | 4 +--- .../ExecutionManager/ExecutionManagerTests.cs | 3 +-- .../tests/ExecutionManager/HashMapTests.cs | 3 +-- .../cdacreader/tests/PrecodeStubsTests.cs | 3 +-- .../cdacreader/tests/TestPlaceholderTarget.cs | 17 ++++++++--------- 5 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs index a7628695731433..75c96dce23eaf0 100644 --- a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs +++ b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs @@ -241,13 +241,11 @@ public CVTestTarget(MockTarget.Architecture arch, IReadOnlyCollection? modules = null, ReadFromTargetDelegate reader = null, Dictionary? typeInfoCache = null) - : base(arch, reader) + : base(arch, reader, typeInfoCache ?? []) { IExecutionManager mockExecutionManager = new MockExecutionManager(codeBlocks ?? []); IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(this, methodDescs ?? [], methodTables ?? []); ILoader loader = new MockLoader(modules ?? []); - if (typeInfoCache != null) - SetTypeInfoCache(typeInfoCache); IContractFactory cvfactory = new CodeVersionsFactory(); SetContracts(new TestRegistry() { CodeVersionsContract = new (() => cvfactory.CreateContract(this, 1)), diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs index 78f34095132df9..cb4721c1e7ec4c 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs @@ -25,10 +25,9 @@ public static ExecutionManagerTestTarget FromBuilder(ExecutionManagerTestBuilder } public ExecutionManagerTestTarget(int version, MockTarget.Architecture arch, ReadFromTargetDelegate dataReader, TargetPointer topRangeSectionMap, Dictionary typeInfoCache) - : base(arch, dataReader) + : base(arch, dataReader, typeInfoCache) { _topRangeSectionMap = topRangeSectionMap; - SetTypeInfoCache(typeInfoCache); IContractFactory emfactory = new ExecutionManagerFactory(); SetContracts(new TestRegistry() { ExecutionManagerContract = new (() => emfactory.CreateContract(this, version)), diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs index 61b207abad2399..7d4f71687bc648 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs @@ -14,10 +14,9 @@ internal class HashMapTestTarget : TestPlaceholderTarget private readonly (string Name, ulong Value, string? Type)[] _globals; public HashMapTestTarget(MockTarget.Architecture arch, MockMemorySpace.ReadContext readContext, MockDescriptors.HashMap hashMap) - : base (arch, readContext.ReadFromTarget) + : base (arch, readContext.ReadFromTarget, hashMap.Types) { _globals = hashMap.Globals; - SetTypeInfoCache(hashMap.Types); } public override T ReadGlobal(string name) diff --git a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs index 1647d5f2639524..a946c4e1b4cd3c 100644 --- a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs +++ b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs @@ -290,10 +290,9 @@ public static PrecodeTestTarget FromBuilder(PrecodeBuilder precodeBuilder) return new PrecodeTestTarget(arch, reader, precodeBuilder.CodePointerFlags, precodeBuilder.MachineDescriptorAddress, typeInfo); } public PrecodeTestTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, CodePointerFlags codePointerFlags, TargetPointer platformMetadataAddress, Dictionary typeInfoCache) - : base(arch, reader) + : base(arch, reader, typeInfoCache) { PrecodeMachineDescriptorAddress = platformMetadataAddress; - SetTypeInfoCache(typeInfoCache); IContractFactory precodeFactory = new PrecodeStubsFactory(); Mock platformMetadata = new(MockBehavior.Strict); diff --git a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs index 7c94e57b011ffa..2e859301797ac6 100644 --- a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs @@ -16,21 +16,25 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests; internal class TestPlaceholderTarget : Target { private protected ContractRegistry contractRegistry; - private protected Target.IDataCache dataCache; - private protected Dictionary typeInfoCache; + private readonly Target.IDataCache dataCache; + private readonly Dictionary typeInfoCache; internal delegate int ReadFromTargetDelegate(ulong address, Span buffer); - protected ReadFromTargetDelegate _dataReader = (address, buffer) => throw new NotImplementedException(); + private readonly ReadFromTargetDelegate _dataReader; #region Setup public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader) + : this(arch, reader, []) + { } + + public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types) { IsLittleEndian = arch.IsLittleEndian; PointerSize = arch.Is64Bit ? 8 : 4; contractRegistry = new TestRegistry(); dataCache = new DefaultDataCache(this); - typeInfoCache = null; + typeInfoCache = types; _dataReader = reader; } @@ -39,11 +43,6 @@ internal void SetContracts(ContractRegistry contracts) contractRegistry = contracts; } - internal void SetTypeInfoCache(Dictionary cache) - { - typeInfoCache = cache; - } - #endregion Setup public override int PointerSize { get; } From a0e4baaa2c321032028f1807b66f840352599e86 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 13 Nov 2024 15:25:44 -0800 Subject: [PATCH 2/7] Remove unnecessary subclasses of TestPlaceholderTarget --- .../tests/ExecutionManager/NibbleMapTests.cs | 18 ++++++------------ .../ExecutionManager/RangeSectionMapTests.cs | 15 +++------------ 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/NibbleMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/NibbleMapTests.cs index e9c8369ade4104..6a71fbafcbb67b 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/NibbleMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/NibbleMapTests.cs @@ -9,19 +9,13 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager; public class NibbleMapTestsBase { - internal class NibbleMapTestTarget : TestPlaceholderTarget + internal static Target CreateTarget(NibbleMapTestBuilderBase nibbleMapTestBuilder) { - public NibbleMapTestTarget(MockTarget.Architecture arch, MockMemorySpace.ReadContext readContext) - : base(arch, readContext.ReadFromTarget) + MockMemorySpace.ReadContext readContext = new() { - } - } - - internal static NibbleMapTestTarget CreateTarget(NibbleMapTestBuilderBase nibbleMapTestBuilder) - { - return new NibbleMapTestTarget(nibbleMapTestBuilder.Arch, new MockMemorySpace.ReadContext() { HeapFragments = new[] { nibbleMapTestBuilder.NibbleMapFragment } - }); + }; + return new TestPlaceholderTarget(nibbleMapTestBuilder.Arch, readContext.ReadFromTarget); } } @@ -84,7 +78,7 @@ public void NibbleMapOneItemLookupOk(MockTarget.Architecture arch) TargetCodePointer inputPC = new(mapBase + 0x0200u); uint codeSize = 0x80; // doesn't matter builder.AllocateCodeChunk (inputPC, codeSize); - NibbleMapTestTarget target = CreateTarget(builder); + Target target = CreateTarget(builder); // TESTCASE: @@ -144,7 +138,7 @@ public void NibbleMapOneItemLookupOk(MockTarget.Architecture arch) TargetCodePointer inputPC = new(mapBase + 0x0200u); uint codeSize = 0x400; builder.AllocateCodeChunk (inputPC, codeSize); - NibbleMapTestTarget target = CreateTarget(builder); + Target target = CreateTarget(builder); // TESTCASE: diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs index fed8cc942ad442..d5a62ab54430a5 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs @@ -6,26 +6,17 @@ using Microsoft.Diagnostics.DataContractReader.ExecutionManagerHelpers; - namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager; public class RangeSectionMapTests { - internal class RSMTestTarget : TestPlaceholderTarget - { - public RSMTestTarget(MockTarget.Architecture arch, MockMemorySpace.ReadContext readContext) - : base (arch, readContext.ReadFromTarget) - { - } - } - [Theory] [ClassData(typeof(MockTarget.StdArch))] public void TestLookupFail(MockTarget.Architecture arch) { var builder = ExecutionManagerTestBuilder.CreateRangeSection(arch); builder.MarkCreated(); - var target = new RSMTestTarget(arch, builder.GetReadContext()); + var target = new TestPlaceholderTarget(arch, builder.GetReadContext().ReadFromTarget); var rsla = RangeSectionMap.Create(target); @@ -44,7 +35,7 @@ public void TestLookupOne(MockTarget.Architecture arch) var value = 0x0a0a_0a0au; builder.InsertAddressRange(inputPC, length, value); builder.MarkCreated(); - var target = new RSMTestTarget(arch, builder.GetReadContext()); + var target = new TestPlaceholderTarget(arch, builder.GetReadContext().ReadFromTarget); var rsla = RangeSectionMap.Create(target); @@ -59,7 +50,7 @@ public void TestLookupOne(MockTarget.Architecture arch) public void TestGetIndexForLevel(MockTarget.Architecture arch) { // Exhaustively test GetIndexForLevel for all possible values of the byte for each level - var target = new RSMTestTarget(arch, new MockMemorySpace.ReadContext()); + var target = new TestPlaceholderTarget(arch, new MockMemorySpace.ReadContext().ReadFromTarget); var rsla = RangeSectionMap.Create(target); int numLevels = arch.Is64Bit ? 5 : 2; // the bits 0..effectiveRange - 1 are not handled the map and are irrelevant From fab5b6f92ab3ff36d9667c32a1826408ca9011c6 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 13 Nov 2024 16:32:35 -0800 Subject: [PATCH 3/7] Pass globals to TestPlaceholderTarget --- .../cdacreader/tests/CodeVersionsTests.cs | 2 +- .../ExecutionManagerTestBuilder.cs | 17 +++++-- .../ExecutionManager/ExecutionManagerTests.cs | 47 ++--------------- .../tests/ExecutionManager/HashMapTests.cs | 19 +------ .../MockDescriptors.HashMap.cs | 24 +++++---- .../cdacreader/tests/PrecodeStubsTests.cs | 21 +++----- .../cdacreader/tests/TestPlaceholderTarget.cs | 51 ++++++++++++------- 7 files changed, 74 insertions(+), 107 deletions(-) diff --git a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs index 75c96dce23eaf0..fb07c412572a07 100644 --- a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs +++ b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs @@ -241,7 +241,7 @@ public CVTestTarget(MockTarget.Architecture arch, IReadOnlyCollection? modules = null, ReadFromTargetDelegate reader = null, Dictionary? typeInfoCache = null) - : base(arch, reader, typeInfoCache ?? []) + : base(arch, reader, typeInfoCache) { IExecutionManager mockExecutionManager = new MockExecutionManager(codeBlocks ?? []); IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(this, methodDescs ?? [], methodTables ?? []); diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs index 5e2228efd80d95..a74d7429ced614 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Microsoft.Diagnostics.DataContractReader.ExecutionManagerHelpers; using InteriorMapValue = Microsoft.Diagnostics.DataContractReader.ExecutionManagerHelpers.RangeSectionMap.InteriorMapValue; @@ -13,6 +14,7 @@ internal class ExecutionManagerTestBuilder { public const ulong ExecutionManagerCodeRangeMapAddress = 0x000a_fff0; + const bool UseFunclets = true; const int RealCodeHeaderSize = 0x08; // must be big enough for the offsets of RealCodeHeader size in ExecutionManagerTestTarget, below public struct AllocationRange @@ -259,6 +261,7 @@ public static RangeSectionMapTestBuilder CreateRangeSection(MockTarget.Architect internal MockMemorySpace.Builder Builder { get; } internal Dictionary Types { get; } + internal (string Name, ulong Value)[] Globals { get; } private readonly RangeSectionMapTestBuilder _rsmBuilder; @@ -287,11 +290,19 @@ internal ExecutionManagerTestBuilder(int version, MockMemorySpace.Builder builde CodeHeapListNodeFields, RealCodeHeaderFields, RuntimeFunctionFields, - MockDescriptors.HashMap.HashMapFields, - MockDescriptors.HashMap.BucketFields(Builder.TargetTestHelpers), ReadyToRunInfoFields(Builder.TargetTestHelpers), MockDescriptors.ModuleFields, - ]); + ]).Concat(MockDescriptors.HashMap.GetTypes(Builder.TargetTestHelpers)) + .ToDictionary(); + Globals = + [ + (nameof(Constants.Globals.ExecutionManagerCodeRangeMapAddress), ExecutionManagerCodeRangeMapAddress), + (nameof(Constants.Globals.StubCodeBlockLast), 0x0Fu), + (nameof(Constants.Globals.FeatureEHFunclets), UseFunclets ? 1 : 0), + ]; + Globals = Globals + .Concat(MockDescriptors.HashMap.GetGlobals(Builder.TargetTestHelpers)) + .ToArray(); } internal NibbleMapTestBuilderBase CreateNibbleMap(ulong codeRangeStart, uint codeRangeSize) diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs index cb4721c1e7ec4c..7b8562a1e5d003 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs @@ -13,63 +13,22 @@ public class ExecutionManagerTests { internal class ExecutionManagerTestTarget : TestPlaceholderTarget { - private readonly ulong _topRangeSectionMap; - public static ExecutionManagerTestTarget FromBuilder(ExecutionManagerTestBuilder emBuilder) { var arch = emBuilder.Builder.TargetTestHelpers.Arch; ReadFromTargetDelegate reader = emBuilder.Builder.GetReadContext().ReadFromTarget; - var topRangeSectionMap = ExecutionManagerTestBuilder.ExecutionManagerCodeRangeMapAddress; - var typeInfo = emBuilder.Types; - return new ExecutionManagerTestTarget(emBuilder.Version, arch, reader, topRangeSectionMap, typeInfo); + return new ExecutionManagerTestTarget(emBuilder.Version, arch, reader, emBuilder.Types, emBuilder.Globals); } - public ExecutionManagerTestTarget(int version, MockTarget.Architecture arch, ReadFromTargetDelegate dataReader, TargetPointer topRangeSectionMap, Dictionary typeInfoCache) - : base(arch, dataReader, typeInfoCache) + public ExecutionManagerTestTarget(int version, MockTarget.Architecture arch, ReadFromTargetDelegate dataReader, Dictionary typeInfoCache, (string Name, ulong Value)[] globals) + : base(arch, dataReader, typeInfoCache, globals) { - _topRangeSectionMap = topRangeSectionMap; IContractFactory emfactory = new ExecutionManagerFactory(); SetContracts(new TestRegistry() { ExecutionManagerContract = new (() => emfactory.CreateContract(this, version)), PlatformMetadataContract = new (() => new Mock().Object) }); } - public override TargetPointer ReadGlobalPointer(string global) - { - switch (global) - { - case Constants.Globals.ExecutionManagerCodeRangeMapAddress: - return new TargetPointer(_topRangeSectionMap); - default: - return base.ReadGlobalPointer(global); - } - } - - public override T ReadGlobal(string name) - { - switch (name) - { - case Constants.Globals.StubCodeBlockLast: - if (typeof(T) == typeof(byte)) - return (T)(object)(byte)0x0Fu; - break; - case Constants.Globals.FeatureEHFunclets: - if (typeof(T) == typeof(byte)) - return (T)(object)(byte)1; - break; - case Constants.Globals.HashMapValueMask: - if (typeof(T) == typeof(ulong)) - return (T)(object)(PointerSize == 4 ? 0x7FFFFFFFu : 0x7FFFFFFFFFFFFFFFu); - break; - case Constants.Globals.HashMapSlotsPerBucket: - if (typeof(T) == typeof(uint)) - return (T)(object)4u; - break; - default: - break; - } - return base.ReadGlobal(name); - } } [Theory] diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs index 7d4f71687bc648..1c42afca7caeb1 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs @@ -11,24 +11,9 @@ public class HashMapTests { internal class HashMapTestTarget : TestPlaceholderTarget { - private readonly (string Name, ulong Value, string? Type)[] _globals; - public HashMapTestTarget(MockTarget.Architecture arch, MockMemorySpace.ReadContext readContext, MockDescriptors.HashMap hashMap) - : base (arch, readContext.ReadFromTarget, hashMap.Types) - { - _globals = hashMap.Globals; - } - - public override T ReadGlobal(string name) - { - foreach (var global in _globals) - { - if (global.Name == name) - return T.CreateChecked(global.Value); - } - - return base.ReadGlobal(name); - } + : base (arch, readContext.ReadFromTarget, hashMap.Types, hashMap.Globals) + { } } [Theory] diff --git a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs index 597d75cf6001fd..ee5628e6e25a90 100644 --- a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs +++ b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs @@ -33,7 +33,7 @@ public class HashMap }; internal Dictionary Types { get; } - internal (string Name, ulong Value, string? Type)[] Globals { get; } + internal (string Name, ulong Value)[] Globals { get; } private const ulong DefaultAllocationRangeStart = 0x0003_0000; private const ulong DefaultAllocationRangeEnd = 0x0004_0000; @@ -52,24 +52,28 @@ public HashMap(MockMemorySpace.Builder builder, (ulong Start, ulong End) allocat { _builder = builder; _allocator = _builder.CreateAllocator(allocationRange.Start, allocationRange.End); - Types = GetTypes(); - Globals = - [ - (nameof(Constants.Globals.HashMapSlotsPerBucket), HashMapSlotsPerBucket, "uint32"), - (nameof(Constants.Globals.HashMapValueMask), _builder.TargetTestHelpers.PointerSize == 4 ? 0x7FFFFFFFu : 0x7FFFFFFFFFFFFFFFu, "uint64"), - ]; + Types = GetTypes(builder.TargetTestHelpers); + Globals = GetGlobals(builder.TargetTestHelpers); } - private Dictionary GetTypes() + internal static Dictionary GetTypes(TargetTestHelpers helpers) { return GetTypesForTypeFields( - _builder.TargetTestHelpers, + helpers, [ HashMapFields, - BucketFields(_builder.TargetTestHelpers), + BucketFields(helpers), ]); } + internal static (string Name, ulong Value)[] GetGlobals(TargetTestHelpers helpers) + { + return [ + (nameof(Constants.Globals.HashMapSlotsPerBucket), HashMapSlotsPerBucket), + (nameof(Constants.Globals.HashMapValueMask), helpers.PointerSize == 4 ? 0x7FFFFFFFu : 0x7FFFFFFFFFFFFFFFu), + ]; + } + public TargetPointer CreateMap((TargetPointer Key, TargetPointer Value)[] entries) { Target.TypeInfo hashMapType = Types[DataType.HashMap]; diff --git a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs index a946c4e1b4cd3c..1cc8d24986f43b 100644 --- a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs +++ b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs @@ -179,7 +179,8 @@ internal class PrecodeBuilder { public readonly MockMemorySpace.Builder Builder; public readonly MockMemorySpace.BumpAllocator PrecodeAllocator; public readonly MockMemorySpace.BumpAllocator StubDataPageAllocator; - public readonly Dictionary? TypeInfoCache; + + internal Dictionary? Types { get; } public TargetPointer MachineDescriptorAddress; public CodePointerFlags CodePointerFlags {get; private set;} @@ -189,7 +190,7 @@ public PrecodeBuilder(AllocationRange allocationRange, MockMemorySpace.Builder b Builder = builder; PrecodeAllocator = builder.CreateAllocator(allocationRange.PrecodeDescriptorStart, allocationRange.PrecodeDescriptorEnd); StubDataPageAllocator = builder.CreateAllocator(allocationRange.StubDataPageStart, allocationRange.StubDataPageEnd); - TypeInfoCache = typeInfoCache ?? CreateTypeInfoCache(Builder.TargetTestHelpers); + Types = typeInfoCache ?? CreateTypeInfoCache(Builder.TargetTestHelpers); } public Dictionary CreateTypeInfoCache(TargetTestHelpers targetTestHelpers) { @@ -234,7 +235,7 @@ private void SetCodePointerFlags(PrecodeTestDescriptor test) public void AddPlatformMetadata(PrecodeTestDescriptor descriptor) { SetCodePointerFlags(descriptor); - var typeInfo = TypeInfoCache[DataType.PrecodeMachineDescriptor]; + var typeInfo = Types[DataType.PrecodeMachineDescriptor]; var fragment = PrecodeAllocator.Allocate((ulong)typeInfo.Size, $"{descriptor.Name} Precode Machine Descriptor"); Builder.AddHeapFragment(fragment); MachineDescriptorAddress = fragment.Address; @@ -250,7 +251,7 @@ public void AddPlatformMetadata(PrecodeTestDescriptor descriptor) { public TargetCodePointer AddStubPrecodeEntry(string name, PrecodeTestDescriptor test, TargetPointer methodDesc) { // TODO[cdac]: allow writing other kinds of stub precode subtypes ulong stubCodeSize = (ulong)test.StubPrecodeSize; - var stubDataTypeInfo = TypeInfoCache[DataType.StubPrecodeData]; + var stubDataTypeInfo = Types[DataType.StubPrecodeData]; MockMemorySpace.HeapFragment stubDataFragment = StubDataPageAllocator.Allocate((ulong)stubDataTypeInfo.Size, $"Stub data for {name} on {test.Name}"); Builder.AddHeapFragment(stubDataFragment); // allocate the code one page before the stub data @@ -286,11 +287,11 @@ public static PrecodeTestTarget FromBuilder(PrecodeBuilder precodeBuilder) precodeBuilder.MarkCreated(); var arch = precodeBuilder.Builder.TargetTestHelpers.Arch; ReadFromTargetDelegate reader = precodeBuilder.Builder.GetReadContext().ReadFromTarget; - var typeInfo = precodeBuilder.TypeInfoCache; + var typeInfo = precodeBuilder.Types; return new PrecodeTestTarget(arch, reader, precodeBuilder.CodePointerFlags, precodeBuilder.MachineDescriptorAddress, typeInfo); } public PrecodeTestTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, CodePointerFlags codePointerFlags, TargetPointer platformMetadataAddress, Dictionary typeInfoCache) - : base(arch, reader, typeInfoCache) + : base(arch, reader, typeInfoCache, [(Constants.Globals.PlatformMetadata, platformMetadataAddress)]) { PrecodeMachineDescriptorAddress = platformMetadataAddress; IContractFactory precodeFactory = new PrecodeStubsFactory(); @@ -304,14 +305,6 @@ public PrecodeTestTarget(MockTarget.Architecture arch, ReadFromTargetDelegate re PrecodeStubsContract = new (() => precodeFactory.CreateContract(this, 1)), }); } - - public override TargetPointer ReadGlobalPointer (string name) - { - if (name == Constants.Globals.PlatformMetadata) { - return PlatformMetadataAddress; - } - return base.ReadGlobalPointer(name); - } } [Theory] diff --git a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs index 2e859301797ac6..fe63c50543b376 100644 --- a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs @@ -16,26 +16,24 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests; internal class TestPlaceholderTarget : Target { private protected ContractRegistry contractRegistry; - private readonly Target.IDataCache dataCache; - private readonly Dictionary typeInfoCache; + private readonly Target.IDataCache _dataCache; + private readonly Dictionary _typeInfoCache; + private readonly (string Name, ulong Value)[] _globals; internal delegate int ReadFromTargetDelegate(ulong address, Span buffer); private readonly ReadFromTargetDelegate _dataReader; #region Setup - public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader) - : this(arch, reader, []) - { } - - public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types) + public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types = null, (string Name, ulong Value)[] globals = null) { IsLittleEndian = arch.IsLittleEndian; PointerSize = arch.Is64Bit ? 8 : 4; contractRegistry = new TestRegistry(); - dataCache = new DefaultDataCache(this); - typeInfoCache = types; + _dataCache = new DefaultDataCache(this); + _typeInfoCache = types ?? []; _dataReader = reader; + _globals = globals ?? []; } internal void SetContracts(ContractRegistry contracts) @@ -53,14 +51,34 @@ public override bool IsAlignedToPointerSize(TargetPointer pointer) return (pointer.Value & (ulong)(PointerSize - 1)) == 0; } - public override TargetPointer ReadGlobalPointer(string global) => throw new NotImplementedException(); + public override TargetPointer ReadGlobalPointer(string name) + { + foreach (var global in _globals) + { + if (global.Name == name) + return new TargetPointer(global.Value); + } + + throw new NotImplementedException(); + } + public override TargetPointer ReadPointer(ulong address) => DefaultReadPointer(address); public override TargetCodePointer ReadCodePointer(ulong address) => DefaultReadCodePointer(address); public override void ReadBuffer(ulong address, Span buffer) => throw new NotImplementedException(); public override string ReadUtf8String(ulong address) => throw new NotImplementedException(); public override string ReadUtf16String(ulong address) => throw new NotImplementedException(); public override TargetNUInt ReadNUInt(ulong address) => DefaultReadNUInt(address); - public override T ReadGlobal(string name) => throw new NotImplementedException(); + public override T ReadGlobal(string name) + { + foreach (var global in _globals) + { + if (global.Name == name) + return T.CreateChecked(global.Value); + } + + throw new NotImplementedException(); + } + public override T Read(ulong address) => DefaultRead(address); #region subclass reader helpers @@ -177,18 +195,15 @@ protected TargetCodePointer DefaultReadCodePointer(ulong address) public override TargetPointer ReadPointerFromSpan(ReadOnlySpan bytes) => throw new NotImplementedException(); - public override Target.TypeInfo GetTypeInfo(DataType dataType) => typeInfoCache != null ? GetTypeInfoImpl(dataType) : throw new NotImplementedException(); - - private protected virtual Target.TypeInfo GetTypeInfoImpl(DataType dataType) + public override Target.TypeInfo GetTypeInfo(DataType dataType) { - if (typeInfoCache!.TryGetValue(dataType, out var info)) - { + if (_typeInfoCache.TryGetValue(dataType, out var info)) return info; - } + throw new NotImplementedException(); } - public override Target.IDataCache ProcessedData => dataCache; + public override Target.IDataCache ProcessedData => _dataCache; public override ContractRegistry Contracts => contractRegistry; internal class TestRegistry : ContractRegistry From 22fc7bc564f3636ba954873bae2bdd09b36808b7 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 13 Nov 2024 17:16:56 -0800 Subject: [PATCH 4/7] Remove subclasses of TestPlaceholderTarget Remove subclasses - squash --- .../cdacreader/tests/CodeVersionsTests.cs | 58 ++++++++++--------- .../ExecutionManager/ExecutionManagerTests.cs | 54 ++++++----------- .../tests/ExecutionManager/HashMapTests.cs | 17 +++--- .../MockDescriptors.HashMap.cs | 17 +++--- .../cdacreader/tests/PrecodeStubsTests.cs | 47 ++++++--------- .../cdacreader/tests/TestPlaceholderTarget.cs | 3 - 6 files changed, 82 insertions(+), 114 deletions(-) diff --git a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs index fb07c412572a07..f18e417023cc7e 100644 --- a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs +++ b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs @@ -226,41 +226,43 @@ TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer tableAddress, uint } } - internal class CVTestTarget : TestPlaceholderTarget + internal static Target CreateTarget( + MockTarget.Architecture arch, + IReadOnlyCollection methodDescs = null, + IReadOnlyCollection methodTables = null, + IReadOnlyCollection codeBlocks = null, + IReadOnlyCollection modules = null, + MockCodeVersions builder = null) { - public static CVTestTarget FromBuilder(MockTarget.Architecture arch, IReadOnlyCollection methodDescs, IReadOnlyCollection methodTables, IReadOnlyCollection codeBlocks, IReadOnlyCollection modules, MockCodeVersions builder) + TestPlaceholderTarget target; + if (builder != null) { builder.MarkCreated(); - return new CVTestTarget(arch, reader: builder.Builder.GetReadContext().ReadFromTarget, typeInfoCache: builder.Types, - methodDescs: methodDescs, methodTables: methodTables, codeBlocks: codeBlocks, modules: modules); + target = new TestPlaceholderTarget(arch, builder.Builder.GetReadContext().ReadFromTarget, builder.Types); } - - public CVTestTarget(MockTarget.Architecture arch, IReadOnlyCollection? methodDescs = null, - IReadOnlyCollection? methodTables = null, - IReadOnlyCollection? codeBlocks = null, - IReadOnlyCollection? modules = null, - ReadFromTargetDelegate reader = null, - Dictionary? typeInfoCache = null) - : base(arch, reader, typeInfoCache) + else { - IExecutionManager mockExecutionManager = new MockExecutionManager(codeBlocks ?? []); - IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(this, methodDescs ?? [], methodTables ?? []); - ILoader loader = new MockLoader(modules ?? []); - IContractFactory cvfactory = new CodeVersionsFactory(); - SetContracts(new TestRegistry() { - CodeVersionsContract = new (() => cvfactory.CreateContract(this, 1)), - ExecutionManagerContract = new (() => mockExecutionManager), - RuntimeTypeSystemContract = new (() => mockRuntimeTypeSystem), - LoaderContract = new (() => loader), - }); + target = new TestPlaceholderTarget(arch, null); } + + IExecutionManager mockExecutionManager = new MockExecutionManager(codeBlocks ?? []); + IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(target, methodDescs ?? [], methodTables ?? []); + ILoader loader = new MockLoader(modules ?? []); + IContractFactory cvfactory = new CodeVersionsFactory(); + target.SetContracts(new TestPlaceholderTarget.TestRegistry() { + CodeVersionsContract = new (() => cvfactory.CreateContract(target, 1)), + ExecutionManagerContract = new (() => mockExecutionManager), + RuntimeTypeSystemContract = new (() => mockRuntimeTypeSystem), + LoaderContract = new (() => loader), + }); + return target; } [Theory] [ClassData(typeof(MockTarget.StdArch))] public void GetNativeCodeVersion_Null(MockTarget.Architecture arch) { - var target = new CVTestTarget(arch); + var target = CreateTarget(arch); var codeVersions = target.Contracts.CodeVersions; Assert.NotNull(codeVersions); @@ -284,7 +286,7 @@ public void GetNativeCodeVersion_OneVersion_NonVersionable(MockTarget.Architectu MethodDesc = oneMethod, }; - var target = new CVTestTarget(arch, methodDescs: [oneMethod], codeBlocks: [oneBlock]); + var target = CreateTarget(arch, methodDescs: [oneMethod], codeBlocks: [oneBlock]); var codeVersions = target.Contracts.CodeVersions; Assert.NotNull(codeVersions); @@ -318,7 +320,7 @@ public void GetNativeCodeVersion_OneVersion_Versionable(MockTarget.Architecture }; builder.FillNativeCodeVersionNode(nativeCodeVersionNode, methodDesc: oneMethod.Address, nativeCode: codeBlockStart, next: TargetPointer.Null, isActive: false, ilVersionId: default); - var target = CVTestTarget.FromBuilder(arch, [oneMethod], [], [oneBlock], [], builder); + var target = CreateTarget(arch, [oneMethod], [], [oneBlock], [], builder); // TEST @@ -365,7 +367,7 @@ public void GetActiveNativeCodeVersion_DefaultCase(MockTarget.Architecture arch) oneMethod.MethodTable = oneMethodTable; oneMethod.RowId = methodRowId; - var target = CVTestTarget.FromBuilder(arch, [oneMethod], [oneMethodTable], [], [oneModule], builder); + var target = CreateTarget(arch, [oneMethod], [oneMethodTable], [], [oneModule], builder); // TEST @@ -428,7 +430,7 @@ private void GetActiveNativeCodeVersion_IterateVersionNodes_Impl(MockTarget.Arch methodDesc.MethodTable = methodTable; methodDesc.RowId = methodRowId; - var target = CVTestTarget.FromBuilder(arch, [methodDesc], [methodTable], [], [module], builder); + var target = CreateTarget(arch, [methodDesc], [methodTable], [], [module], builder); // TEST @@ -499,7 +501,7 @@ private void GetActiveNativeCodeVersion_ExplicitILCodeVersion_Impl(MockTarget.Ar oneMethod.MethodTable = methodTable; oneMethod.RowId = methodRowId; - var target = CVTestTarget.FromBuilder(arch, [oneMethod], [methodTable], [], [module], builder); + var target = CreateTarget(arch, [oneMethod], [methodTable], [], [module], builder); // TEST diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs index 7b8562a1e5d003..c4e0d0f6548b37 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs @@ -11,24 +11,18 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager; public class ExecutionManagerTests { - internal class ExecutionManagerTestTarget : TestPlaceholderTarget + private static Target CreateTarget(ExecutionManagerTestBuilder emBuilder) { - public static ExecutionManagerTestTarget FromBuilder(ExecutionManagerTestBuilder emBuilder) - { - var arch = emBuilder.Builder.TargetTestHelpers.Arch; - ReadFromTargetDelegate reader = emBuilder.Builder.GetReadContext().ReadFromTarget; - return new ExecutionManagerTestTarget(emBuilder.Version, arch, reader, emBuilder.Types, emBuilder.Globals); - } - - public ExecutionManagerTestTarget(int version, MockTarget.Architecture arch, ReadFromTargetDelegate dataReader, Dictionary typeInfoCache, (string Name, ulong Value)[] globals) - : base(arch, dataReader, typeInfoCache, globals) - { - IContractFactory emfactory = new ExecutionManagerFactory(); - SetContracts(new TestRegistry() { - ExecutionManagerContract = new (() => emfactory.CreateContract(this, version)), - PlatformMetadataContract = new (() => new Mock().Object) - }); - } + emBuilder.MarkCreated(); + var arch = emBuilder.Builder.TargetTestHelpers.Arch; + TestPlaceholderTarget.ReadFromTargetDelegate reader = emBuilder.Builder.GetReadContext().ReadFromTarget; + var target = new TestPlaceholderTarget(arch, reader, emBuilder.Types, emBuilder.Globals); + IContractFactory emfactory = new ExecutionManagerFactory(); + target.SetContracts(new TestPlaceholderTarget.TestRegistry() { + ExecutionManagerContract = new (() => emfactory.CreateContract(target, emBuilder.Version)), + PlatformMetadataContract = new (() => new Mock().Object) + }); + return target; } [Theory] @@ -36,8 +30,7 @@ public ExecutionManagerTestTarget(int version, MockTarget.Architecture arch, Rea public void GetCodeBlockHandle_Null(int version, MockTarget.Architecture arch) { ExecutionManagerTestBuilder emBuilder = new (version, arch, ExecutionManagerTestBuilder.DefaultAllocationRange); - emBuilder.MarkCreated(); - var target = ExecutionManagerTestTarget.FromBuilder (emBuilder); + var target = CreateTarget(emBuilder); var em = target.Contracts.ExecutionManager; Assert.NotNull(em); @@ -50,8 +43,7 @@ public void GetCodeBlockHandle_Null(int version, MockTarget.Architecture arch) public void GetCodeBlockHandle_NoRangeSections(int version, MockTarget.Architecture arch) { ExecutionManagerTestBuilder emBuilder = new (version, arch, ExecutionManagerTestBuilder.DefaultAllocationRange); - emBuilder.MarkCreated(); - var target = ExecutionManagerTestTarget.FromBuilder (emBuilder); + var target = CreateTarget(emBuilder); var em = target.Contracts.ExecutionManager; Assert.NotNull(em); @@ -83,9 +75,7 @@ public void GetMethodDesc_OneRangeOneMethod(int version, MockTarget.Architecture TargetPointer rangeSectionAddress = emBuilder.AddRangeSection(jittedCode, jitManagerAddress: jitManagerAddress, codeHeapListNodeAddress: codeHeapListNodeAddress); TargetPointer rangeSectionFragmentAddress = emBuilder.AddRangeSectionFragment(jittedCode, rangeSectionAddress); - emBuilder.MarkCreated(); - - var target = ExecutionManagerTestTarget.FromBuilder(emBuilder); + var target = CreateTarget(emBuilder); var em = target.Contracts.ExecutionManager; Assert.NotNull(em); @@ -127,9 +117,7 @@ public void GetCodeBlockHandle_OneRangeZeroMethod(int version, MockTarget.Archit TargetPointer rangeSectionAddress = emBuilder.AddRangeSection(jittedCode, jitManagerAddress: jitManagerAddress, codeHeapListNodeAddress: codeHeapListNodeAddress); TargetPointer rangeSectionFragmentAddress = emBuilder.AddRangeSectionFragment(jittedCode, rangeSectionAddress); - emBuilder.MarkCreated(); - - var target = ExecutionManagerTestTarget.FromBuilder(emBuilder); + var target = CreateTarget(emBuilder); var em = target.Contracts.ExecutionManager; Assert.NotNull(em); @@ -170,9 +158,7 @@ public void GetCodeBlockHandle_R2R_NoRuntimeFunctionMatch(int version, MockTarge TargetPointer rangeSectionAddress = emBuilder.AddReadyToRunRangeSection(jittedCode, jitManagerAddress, r2rModule); _ = emBuilder.AddRangeSectionFragment(jittedCode, rangeSectionAddress); - emBuilder.MarkCreated(); - - Target target = ExecutionManagerTestTarget.FromBuilder(emBuilder); + Target target = CreateTarget(emBuilder); IExecutionManager em = target.Contracts.ExecutionManager; Assert.NotNull(em); @@ -207,9 +193,7 @@ public void GetMethodDesc_R2R_OneRuntimeFunction(int version, MockTarget.Archite TargetPointer rangeSectionAddress = emBuilder.AddReadyToRunRangeSection(jittedCode, jitManagerAddress, r2rModule); _ = emBuilder.AddRangeSectionFragment(jittedCode, rangeSectionAddress); - emBuilder.MarkCreated(); - - Target target = ExecutionManagerTestTarget.FromBuilder(emBuilder); + Target target = CreateTarget(emBuilder); IExecutionManager em = target.Contracts.ExecutionManager; Assert.NotNull(em); @@ -258,9 +242,7 @@ public void GetMethodDesc_R2R_MultipleRuntimeFunctions(int version, MockTarget.A TargetPointer rangeSectionAddress = emBuilder.AddReadyToRunRangeSection(jittedCode, jitManagerAddress, r2rModule); _ = emBuilder.AddRangeSectionFragment(jittedCode, rangeSectionAddress); - emBuilder.MarkCreated(); - - Target target = ExecutionManagerTestTarget.FromBuilder(emBuilder); + Target target = CreateTarget(emBuilder); IExecutionManager em = target.Contracts.ExecutionManager; Assert.NotNull(em); diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs index 1c42afca7caeb1..eb54dec9e21757 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs @@ -9,11 +9,11 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager; public class HashMapTests { - internal class HashMapTestTarget : TestPlaceholderTarget + private static Target CreateTarget(MockDescriptors.HashMap hashMap) { - public HashMapTestTarget(MockTarget.Architecture arch, MockMemorySpace.ReadContext readContext, MockDescriptors.HashMap hashMap) - : base (arch, readContext.ReadFromTarget, hashMap.Types, hashMap.Globals) - { } + MockMemorySpace.Builder builder = hashMap.Builder; + builder.MarkCreated(); + return new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals); } [Theory] @@ -31,9 +31,8 @@ public void GetValue(MockTarget.Architecture arch) ]; TargetPointer mapAddress = hashMap.CreateMap(entries); TargetPointer ptrMapAddress = hashMap.CreatePtrMap(entries); - builder.MarkCreated(); - Target target = new HashMapTestTarget(arch, builder.GetReadContext(), hashMap); + Target target = CreateTarget(hashMap); var lookup = HashMapLookup.Create(target); var ptrLookup = PtrHashMapLookup.Create(target); @@ -68,9 +67,8 @@ public void GetValue_Collision(MockTarget.Architecture arch) ]; TargetPointer mapAddress = hashMap.CreateMap(entries); TargetPointer ptrMapAddress = hashMap.CreatePtrMap(entries); - builder.MarkCreated(); - Target target = new HashMapTestTarget(arch, builder.GetReadContext(), hashMap); + Target target = CreateTarget(hashMap); var lookup = HashMapLookup.Create(target); var ptrLookup = PtrHashMapLookup.Create(target); @@ -94,9 +92,8 @@ public void GetValue_NoMatch(MockTarget.Architecture arch) (TargetPointer Key, TargetPointer Value)[] entries = [(0x100, 0x010)]; TargetPointer mapAddress = hashMap.CreateMap(entries); TargetPointer ptrMapAddress = hashMap.CreatePtrMap(entries); - builder.MarkCreated(); - Target target = new HashMapTestTarget(arch, builder.GetReadContext(), hashMap); + Target target = CreateTarget(hashMap); { var lookup = HashMapLookup.Create(target); diff --git a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs index ee5628e6e25a90..6bb809e2eaa9ef 100644 --- a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs +++ b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs @@ -35,13 +35,14 @@ public class HashMap internal Dictionary Types { get; } internal (string Name, ulong Value)[] Globals { get; } + internal MockMemorySpace.Builder Builder { get; } + private const ulong DefaultAllocationRangeStart = 0x0003_0000; private const ulong DefaultAllocationRangeEnd = 0x0004_0000; // See g_rgPrimes in hash.cpp private static readonly uint[] PossibleSizes = [5, 11, 17, 23, 29, 37]; - private readonly MockMemorySpace.Builder _builder; private readonly MockMemorySpace.BumpAllocator _allocator; public HashMap(MockMemorySpace.Builder builder) @@ -50,8 +51,8 @@ public HashMap(MockMemorySpace.Builder builder) public HashMap(MockMemorySpace.Builder builder, (ulong Start, ulong End) allocationRange) { - _builder = builder; - _allocator = _builder.CreateAllocator(allocationRange.Start, allocationRange.End); + Builder = builder; + _allocator = Builder.CreateAllocator(allocationRange.Start, allocationRange.End); Types = GetTypes(builder.TargetTestHelpers); Globals = GetGlobals(builder.TargetTestHelpers); } @@ -78,14 +79,14 @@ public TargetPointer CreateMap((TargetPointer Key, TargetPointer Value)[] entrie { Target.TypeInfo hashMapType = Types[DataType.HashMap]; MockMemorySpace.HeapFragment map = _allocator.Allocate(hashMapType.Size!.Value, "HashMap"); - _builder.AddHeapFragment(map); + Builder.AddHeapFragment(map); PopulateMap(map.Address, entries); return map.Address; } public void PopulateMap(TargetPointer mapAddress, (TargetPointer Key, TargetPointer Value)[] entries) { - TargetTestHelpers helpers = _builder.TargetTestHelpers; + TargetTestHelpers helpers = Builder.TargetTestHelpers; // HashMap::NewSize int requiredSlots = entries.Length * 3 / 2; @@ -99,7 +100,7 @@ public void PopulateMap(TargetPointer mapAddress, (TargetPointer Key, TargetPoin uint numBuckets = size + 1; uint totalBucketsSize = bucketSize * numBuckets; MockMemorySpace.HeapFragment buckets = _allocator.Allocate(totalBucketsSize, $"Buckets[{numBuckets}]"); - _builder.AddHeapFragment(buckets); + Builder.AddHeapFragment(buckets); helpers.Write(buckets.Data.AsSpan().Slice(0, sizeof(uint)), size); const int maxRetry = 8; @@ -124,7 +125,7 @@ public void PopulateMap(TargetPointer mapAddress, (TargetPointer Key, TargetPoin // Update the map to point at the buckets Target.TypeInfo hashMapType = Types[DataType.HashMap]; - Span map = _builder.BorrowAddressRange(mapAddress, (int)hashMapType.Size!.Value); + Span map = Builder.BorrowAddressRange(mapAddress, (int)hashMapType.Size!.Value); helpers.WritePointer(map.Slice(hashMapType.Fields[nameof(Data.HashMap.Buckets)].Offset, helpers.PointerSize), buckets.Address); } @@ -148,7 +149,7 @@ public void PopulatePtrMap(TargetPointer mapAddress, (TargetPointer Key, TargetP private bool TryAddEntryToBucket(Span bucket, TargetPointer key, TargetPointer value) { - TargetTestHelpers helpers = _builder.TargetTestHelpers; + TargetTestHelpers helpers = Builder.TargetTestHelpers; Target.TypeInfo bucketType = Types[DataType.Bucket]; for (int i = 0; i < HashMapSlotsPerBucket; i++) { diff --git a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs index 1cc8d24986f43b..6f6c60b8d9c662 100644 --- a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs +++ b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs @@ -277,34 +277,25 @@ public TargetCodePointer AddStubPrecodeEntry(string name, PrecodeTestDescriptor public void MarkCreated() => Builder.MarkCreated(); } - internal class PrecodeTestTarget : TestPlaceholderTarget + private static Target CreateTarget(PrecodeBuilder precodeBuilder) { - internal readonly TargetPointer PrecodeMachineDescriptorAddress; + var arch = precodeBuilder.Builder.TargetTestHelpers.Arch; + TestPlaceholderTarget.ReadFromTargetDelegate reader = precodeBuilder.Builder.GetReadContext().ReadFromTarget; // hack for this test put the precode machine descriptor at the same address as the PlatformMetadata - internal TargetPointer PlatformMetadataAddress => PrecodeMachineDescriptorAddress; - public static PrecodeTestTarget FromBuilder(PrecodeBuilder precodeBuilder) - { - precodeBuilder.MarkCreated(); - var arch = precodeBuilder.Builder.TargetTestHelpers.Arch; - ReadFromTargetDelegate reader = precodeBuilder.Builder.GetReadContext().ReadFromTarget; - var typeInfo = precodeBuilder.Types; - return new PrecodeTestTarget(arch, reader, precodeBuilder.CodePointerFlags, precodeBuilder.MachineDescriptorAddress, typeInfo); - } - public PrecodeTestTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, CodePointerFlags codePointerFlags, TargetPointer platformMetadataAddress, Dictionary typeInfoCache) - : base(arch, reader, typeInfoCache, [(Constants.Globals.PlatformMetadata, platformMetadataAddress)]) - { - PrecodeMachineDescriptorAddress = platformMetadataAddress; - IContractFactory precodeFactory = new PrecodeStubsFactory(); - - Mock platformMetadata = new(MockBehavior.Strict); - platformMetadata.Setup(p => p.GetCodePointerFlags()).Returns(codePointerFlags); - platformMetadata.Setup(p => p.GetPrecodeMachineDescriptor()).Returns(PrecodeMachineDescriptorAddress); - - SetContracts(new TestRegistry() { - PlatformMetadataContract = new (() => platformMetadata.Object), - PrecodeStubsContract = new (() => precodeFactory.CreateContract(this, 1)), - }); - } + (string Name, ulong Value)[] globals = [(Constants.Globals.PlatformMetadata, precodeBuilder.MachineDescriptorAddress)]; + precodeBuilder.MarkCreated(); + var target = new TestPlaceholderTarget(arch, reader, precodeBuilder.Types, globals); + + IContractFactory precodeFactory = new PrecodeStubsFactory(); + Mock platformMetadata = new(MockBehavior.Strict); + platformMetadata.Setup(p => p.GetCodePointerFlags()).Returns(precodeBuilder.CodePointerFlags); + platformMetadata.Setup(p => p.GetPrecodeMachineDescriptor()).Returns(precodeBuilder.MachineDescriptorAddress); + + target.SetContracts(new TestPlaceholderTarget.TestRegistry() { + PlatformMetadataContract = new (() => platformMetadata.Object), + PrecodeStubsContract = new (() => precodeFactory.CreateContract(target, 1)), + }); + return target; } [Theory] @@ -317,7 +308,7 @@ public void TestPrecodeStubPrecodeExpectedMethodDesc(PrecodeTestDescriptor test) TargetPointer expectedMethodDesc = new TargetPointer(0xeeee_eee0u); // arbitrary TargetCodePointer stub1 = builder.AddStubPrecodeEntry("Stub 1", test, expectedMethodDesc); - var target = PrecodeTestTarget.FromBuilder(builder); + var target = CreateTarget(builder); Assert.NotNull(target); var precodeContract = target.Contracts.PrecodeStubs; @@ -326,7 +317,5 @@ public void TestPrecodeStubPrecodeExpectedMethodDesc(PrecodeTestDescriptor test) var actualMethodDesc = precodeContract.GetMethodDescFromStubAddress(stub1); Assert.Equal(expectedMethodDesc, actualMethodDesc); - - } } diff --git a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs index fe63c50543b376..0bb0aca72c920d 100644 --- a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs @@ -24,7 +24,6 @@ internal class TestPlaceholderTarget : Target private readonly ReadFromTargetDelegate _dataReader; -#region Setup public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegate reader, Dictionary types = null, (string Name, ulong Value)[] globals = null) { IsLittleEndian = arch.IsLittleEndian; @@ -41,8 +40,6 @@ internal void SetContracts(ContractRegistry contracts) contractRegistry = contracts; } -#endregion Setup - public override int PointerSize { get; } public override bool IsLittleEndian { get; } From 0d3445ab85b2a2504f6db767a0cd1c2bbfd66ddb Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 14 Nov 2024 11:00:36 -0800 Subject: [PATCH 5/7] Stop requiring builder to be marked as created when getting a read context for it squash - mark created --- .../managed/cdacreader/tests/CodeVersionsTests.cs | 13 +++---------- .../ExecutionManager/ExecutionManagerTestBuilder.cs | 6 ------ .../tests/ExecutionManager/ExecutionManagerTests.cs | 1 - .../tests/ExecutionManager/HashMapTests.cs | 13 +++---------- .../tests/ExecutionManager/RangeSectionMapTests.cs | 2 -- .../MockDescriptors/MockDescriptors.CodeVersions.cs | 2 -- .../managed/cdacreader/tests/MockMemorySpace.cs | 11 +---------- .../managed/cdacreader/tests/PrecodeStubsTests.cs | 3 --- .../cdacreader/tests/TestPlaceholderTarget.cs | 2 +- 9 files changed, 8 insertions(+), 45 deletions(-) diff --git a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs index f18e417023cc7e..9819c24efa1fb6 100644 --- a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs +++ b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs @@ -234,16 +234,9 @@ internal static Target CreateTarget( IReadOnlyCollection modules = null, MockCodeVersions builder = null) { - TestPlaceholderTarget target; - if (builder != null) - { - builder.MarkCreated(); - target = new TestPlaceholderTarget(arch, builder.Builder.GetReadContext().ReadFromTarget, builder.Types); - } - else - { - target = new TestPlaceholderTarget(arch, null); - } + TestPlaceholderTarget target = builder != null + ? new TestPlaceholderTarget(arch, builder.Builder.GetReadContext().ReadFromTarget, builder.Types) + : new TestPlaceholderTarget(arch, null); IExecutionManager mockExecutionManager = new MockExecutionManager(codeBlocks ?? []); IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(target, methodDescs ?? [], methodTables ?? []); diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs index a74d7429ced614..67f18342b6e5cc 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs @@ -161,10 +161,6 @@ public void InsertAddressRange(TargetCodePointer start, uint length, ulong value cur = new TargetCodePointer(cur.Value + (ulong)BytesAtLastLevel); // FIXME: round ? } while (cur.Value < end); } - public void MarkCreated() - { - _builder.MarkCreated(); - } public MockMemorySpace.ReadContext GetReadContext() { @@ -480,6 +476,4 @@ public TargetPointer AddReadyToRunModule(TargetPointer r2rInfo) return r2rModule.Address; } - - public void MarkCreated() => Builder.MarkCreated(); } diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs index c4e0d0f6548b37..85aa18bbb82844 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs @@ -13,7 +13,6 @@ public class ExecutionManagerTests { private static Target CreateTarget(ExecutionManagerTestBuilder emBuilder) { - emBuilder.MarkCreated(); var arch = emBuilder.Builder.TargetTestHelpers.Arch; TestPlaceholderTarget.ReadFromTargetDelegate reader = emBuilder.Builder.GetReadContext().ReadFromTarget; var target = new TestPlaceholderTarget(arch, reader, emBuilder.Types, emBuilder.Globals); diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs index eb54dec9e21757..53c7b2ffcb8623 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs @@ -9,13 +9,6 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager; public class HashMapTests { - private static Target CreateTarget(MockDescriptors.HashMap hashMap) - { - MockMemorySpace.Builder builder = hashMap.Builder; - builder.MarkCreated(); - return new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals); - } - [Theory] [ClassData(typeof(MockTarget.StdArch))] public void GetValue(MockTarget.Architecture arch) @@ -32,7 +25,7 @@ public void GetValue(MockTarget.Architecture arch) TargetPointer mapAddress = hashMap.CreateMap(entries); TargetPointer ptrMapAddress = hashMap.CreatePtrMap(entries); - Target target = CreateTarget(hashMap); + Target target = new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals); var lookup = HashMapLookup.Create(target); var ptrLookup = PtrHashMapLookup.Create(target); @@ -68,7 +61,7 @@ public void GetValue_Collision(MockTarget.Architecture arch) TargetPointer mapAddress = hashMap.CreateMap(entries); TargetPointer ptrMapAddress = hashMap.CreatePtrMap(entries); - Target target = CreateTarget(hashMap); + Target target = new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals); var lookup = HashMapLookup.Create(target); var ptrLookup = PtrHashMapLookup.Create(target); @@ -93,7 +86,7 @@ public void GetValue_NoMatch(MockTarget.Architecture arch) TargetPointer mapAddress = hashMap.CreateMap(entries); TargetPointer ptrMapAddress = hashMap.CreatePtrMap(entries); - Target target = CreateTarget(hashMap); + Target target = new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals); { var lookup = HashMapLookup.Create(target); diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs index d5a62ab54430a5..914c5ec93223aa 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/RangeSectionMapTests.cs @@ -15,7 +15,6 @@ public class RangeSectionMapTests public void TestLookupFail(MockTarget.Architecture arch) { var builder = ExecutionManagerTestBuilder.CreateRangeSection(arch); - builder.MarkCreated(); var target = new TestPlaceholderTarget(arch, builder.GetReadContext().ReadFromTarget); var rsla = RangeSectionMap.Create(target); @@ -34,7 +33,6 @@ public void TestLookupOne(MockTarget.Architecture arch) var length = 0x1000u; var value = 0x0a0a_0a0au; builder.InsertAddressRange(inputPC, length, value); - builder.MarkCreated(); var target = new TestPlaceholderTarget(arch, builder.GetReadContext().ReadFromTarget); var rsla = RangeSectionMap.Create(target); diff --git a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.CodeVersions.cs b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.CodeVersions.cs index a6b833044abd7f..05df0bace8d6e5 100644 --- a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.CodeVersions.cs +++ b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.CodeVersions.cs @@ -90,8 +90,6 @@ public CodeVersions(MockMemorySpace.Builder builder, (ulong Start, ulong End) al ]); } - public void MarkCreated() => Builder.MarkCreated(); - public TargetPointer AddMethodDescVersioningState(TargetPointer nativeCodeVersionNode, bool isDefaultVersionActive) { Target.TypeInfo info = Types[DataType.MethodDescVersioningState]; diff --git a/src/native/managed/cdacreader/tests/MockMemorySpace.cs b/src/native/managed/cdacreader/tests/MockMemorySpace.cs index f7cbacf2515227..59a4026bc2454f 100644 --- a/src/native/managed/cdacreader/tests/MockMemorySpace.cs +++ b/src/native/managed/cdacreader/tests/MockMemorySpace.cs @@ -216,21 +216,12 @@ private ulong CreateDescriptorFragments() if (pointerData.Data.Length > 0) AddHeapFragment(pointerData); - MarkCreated(); - return descriptor.Address; - } - - internal void MarkCreated() - { - if (_created) - throw new InvalidOperationException("Context already created"); _created = true; + return descriptor.Address; } internal ReadContext GetReadContext() { - if (!_created) - throw new InvalidOperationException("Context not created"); ReadContext context = new ReadContext { HeapFragments = _heapFragments, diff --git a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs index 6f6c60b8d9c662..b2c61747a58b28 100644 --- a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs +++ b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs @@ -273,8 +273,6 @@ public TargetCodePointer AddStubPrecodeEntry(string name, PrecodeTestDescriptor } return address; } - - public void MarkCreated() => Builder.MarkCreated(); } private static Target CreateTarget(PrecodeBuilder precodeBuilder) @@ -283,7 +281,6 @@ private static Target CreateTarget(PrecodeBuilder precodeBuilder) TestPlaceholderTarget.ReadFromTargetDelegate reader = precodeBuilder.Builder.GetReadContext().ReadFromTarget; // hack for this test put the precode machine descriptor at the same address as the PlatformMetadata (string Name, ulong Value)[] globals = [(Constants.Globals.PlatformMetadata, precodeBuilder.MachineDescriptorAddress)]; - precodeBuilder.MarkCreated(); var target = new TestPlaceholderTarget(arch, reader, precodeBuilder.Types, globals); IContractFactory precodeFactory = new PrecodeStubsFactory(); diff --git a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs index 0bb0aca72c920d..ea4f87923193f7 100644 --- a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs @@ -11,7 +11,7 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests; /// -/// A base class implementation of Target that throws NotImplementedException for all methods. +/// A mock implementation of Target that has basic implementations of getting types/globals and reading data /// internal class TestPlaceholderTarget : Target { From a6134bedccc7b27e385d651f3ceb2bed73691a87 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 14 Nov 2024 12:01:19 -0800 Subject: [PATCH 6/7] Use Moq for ContractRegistry --- .../cdacreader/tests/CodeVersionsTests.cs | 13 ++++--- .../ExecutionManager/ExecutionManagerTests.cs | 8 ++-- .../cdacreader/tests/PrecodeStubsTests.cs | 13 ++++--- .../cdacreader/tests/TestPlaceholderTarget.cs | 38 ++----------------- 4 files changed, 23 insertions(+), 49 deletions(-) diff --git a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs index 9819c24efa1fb6..69e39c597feda8 100644 --- a/src/native/managed/cdacreader/tests/CodeVersionsTests.cs +++ b/src/native/managed/cdacreader/tests/CodeVersionsTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Microsoft.Diagnostics.DataContractReader.Contracts; +using Moq; using Xunit; namespace Microsoft.Diagnostics.DataContractReader.UnitTests; @@ -242,12 +243,12 @@ internal static Target CreateTarget( IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(target, methodDescs ?? [], methodTables ?? []); ILoader loader = new MockLoader(modules ?? []); IContractFactory cvfactory = new CodeVersionsFactory(); - target.SetContracts(new TestPlaceholderTarget.TestRegistry() { - CodeVersionsContract = new (() => cvfactory.CreateContract(target, 1)), - ExecutionManagerContract = new (() => mockExecutionManager), - RuntimeTypeSystemContract = new (() => mockRuntimeTypeSystem), - LoaderContract = new (() => loader), - }); + ContractRegistry reg = Mock.Of( + c => c.CodeVersions == cvfactory.CreateContract(target, 1) + && c.ExecutionManager == mockExecutionManager + && c.RuntimeTypeSystem == mockRuntimeTypeSystem + && c.Loader == loader); + target.SetContracts(reg); return target; } diff --git a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs index 85aa18bbb82844..f222e0149604b4 100644 --- a/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs +++ b/src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs @@ -17,10 +17,10 @@ private static Target CreateTarget(ExecutionManagerTestBuilder emBuilder) TestPlaceholderTarget.ReadFromTargetDelegate reader = emBuilder.Builder.GetReadContext().ReadFromTarget; var target = new TestPlaceholderTarget(arch, reader, emBuilder.Types, emBuilder.Globals); IContractFactory emfactory = new ExecutionManagerFactory(); - target.SetContracts(new TestPlaceholderTarget.TestRegistry() { - ExecutionManagerContract = new (() => emfactory.CreateContract(target, emBuilder.Version)), - PlatformMetadataContract = new (() => new Mock().Object) - }); + ContractRegistry reg = Mock.Of( + c => c.ExecutionManager == emfactory.CreateContract(target, emBuilder.Version) + && c.PlatformMetadata == new Mock().Object); + target.SetContracts(reg); return target; } diff --git a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs index b2c61747a58b28..e62f42e033d407 100644 --- a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs +++ b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs @@ -284,14 +284,17 @@ private static Target CreateTarget(PrecodeBuilder precodeBuilder) var target = new TestPlaceholderTarget(arch, reader, precodeBuilder.Types, globals); IContractFactory precodeFactory = new PrecodeStubsFactory(); - Mock platformMetadata = new(MockBehavior.Strict); + Mock platformMetadata = new(); platformMetadata.Setup(p => p.GetCodePointerFlags()).Returns(precodeBuilder.CodePointerFlags); platformMetadata.Setup(p => p.GetPrecodeMachineDescriptor()).Returns(precodeBuilder.MachineDescriptorAddress); - target.SetContracts(new TestPlaceholderTarget.TestRegistry() { - PlatformMetadataContract = new (() => platformMetadata.Object), - PrecodeStubsContract = new (() => precodeFactory.CreateContract(target, 1)), - }); + // Creating the PrecodeStubs contract depends on the PlatformMetadata contract, so we need + // to set it up such that it will only be created after the target's targets are set up + Mock reg = new(); + reg.SetupGet(c => c.PlatformMetadata).Returns(platformMetadata.Object); + reg.SetupGet(c => c.PrecodeStubs).Returns(() => precodeFactory.CreateContract(target, 1)); + target.SetContracts(reg.Object); + return target; } diff --git a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs index ea4f87923193f7..11f68f27847c97 100644 --- a/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs +++ b/src/native/managed/cdacreader/tests/TestPlaceholderTarget.cs @@ -15,7 +15,7 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests; /// internal class TestPlaceholderTarget : Target { - private protected ContractRegistry contractRegistry; + private ContractRegistry _contractRegistry; private readonly Target.IDataCache _dataCache; private readonly Dictionary _typeInfoCache; private readonly (string Name, ulong Value)[] _globals; @@ -28,7 +28,7 @@ public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegat { IsLittleEndian = arch.IsLittleEndian; PointerSize = arch.Is64Bit ? 8 : 4; - contractRegistry = new TestRegistry(); + _contractRegistry = new Mock().Object; _dataCache = new DefaultDataCache(this); _typeInfoCache = types ?? []; _dataReader = reader; @@ -37,7 +37,7 @@ public TestPlaceholderTarget(MockTarget.Architecture arch, ReadFromTargetDelegat internal void SetContracts(ContractRegistry contracts) { - contractRegistry = contracts; + _contractRegistry = contracts; } public override int PointerSize { get; } @@ -201,37 +201,7 @@ public override Target.TypeInfo GetTypeInfo(DataType dataType) } public override Target.IDataCache ProcessedData => _dataCache; - public override ContractRegistry Contracts => contractRegistry; - - internal class TestRegistry : ContractRegistry - { - public TestRegistry() { } - internal Lazy? ExceptionContract { get; set; } - internal Lazy? LoaderContract { get; set; } - internal Lazy? EcmaMetadataContract { get; set; } - internal Lazy? ObjectContract { get; set; } - internal Lazy? ThreadContract { get; set; } - internal Lazy? RuntimeTypeSystemContract { get; set; } - internal Lazy? DacStreamsContract { get; set; } - internal Lazy ExecutionManagerContract { get; set; } - internal Lazy? CodeVersionsContract { get; set; } - internal Lazy? PlatformMetadataContract { get; set; } - internal Lazy? PrecodeStubsContract { get; set; } - internal Lazy? ReJITContract { get; set; } - - public override Contracts.IException Exception => ExceptionContract.Value ?? throw new NotImplementedException(); - public override Contracts.ILoader Loader => LoaderContract.Value ?? throw new NotImplementedException(); - public override Contracts.IEcmaMetadata EcmaMetadata => EcmaMetadataContract.Value ?? throw new NotImplementedException(); - public override Contracts.IObject Object => ObjectContract.Value ?? throw new NotImplementedException(); - public override Contracts.IThread Thread => ThreadContract.Value ?? throw new NotImplementedException(); - public override Contracts.IRuntimeTypeSystem RuntimeTypeSystem => RuntimeTypeSystemContract.Value ?? throw new NotImplementedException(); - public override Contracts.IDacStreams DacStreams => DacStreamsContract.Value ?? throw new NotImplementedException(); - public override Contracts.IExecutionManager ExecutionManager => ExecutionManagerContract.Value ?? throw new NotImplementedException(); - public override Contracts.ICodeVersions CodeVersions => CodeVersionsContract.Value ?? throw new NotImplementedException(); - public override Contracts.IPlatformMetadata PlatformMetadata => PlatformMetadataContract.Value ?? throw new NotImplementedException(); - public override Contracts.IPrecodeStubs PrecodeStubs => PrecodeStubsContract.Value ?? throw new NotImplementedException(); - public override Contracts.IReJIT ReJIT => ReJITContract.Value ?? throw new NotImplementedException(); - } + public override ContractRegistry Contracts => _contractRegistry; // A data cache that stores data in a dictionary and calls IData.Create to construct the data. private class DefaultDataCache : Target.IDataCache From 61acd5223bd04319d0bc3f4eabc46f09ab3d5bcc Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Tue, 19 Nov 2024 12:52:24 -0800 Subject: [PATCH 7/7] PR feedback --- .../MockDescriptors/MockDescriptors.HashMap.cs | 2 +- .../cdacreader/tests/PrecodeStubsTests.cs | 18 +++++++----------- .../cdacreader/tests/TargetTestHelpers.cs | 3 +++ 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs index 6bb809e2eaa9ef..499c5da0ef7b3d 100644 --- a/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs +++ b/src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs @@ -71,7 +71,7 @@ internal static (string Name, ulong Value)[] GetGlobals(TargetTestHelpers helper { return [ (nameof(Constants.Globals.HashMapSlotsPerBucket), HashMapSlotsPerBucket), - (nameof(Constants.Globals.HashMapValueMask), helpers.PointerSize == 4 ? 0x7FFFFFFFu : 0x7FFFFFFFFFFFFFFFu), + (nameof(Constants.Globals.HashMapValueMask), helpers.MaxSignedTargetAddress), ]; } diff --git a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs index e62f42e033d407..c8ad543c5e2227 100644 --- a/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs +++ b/src/native/managed/cdacreader/tests/PrecodeStubsTests.cs @@ -180,7 +180,7 @@ internal class PrecodeBuilder { public readonly MockMemorySpace.BumpAllocator PrecodeAllocator; public readonly MockMemorySpace.BumpAllocator StubDataPageAllocator; - internal Dictionary? Types { get; } + internal Dictionary Types { get; } public TargetPointer MachineDescriptorAddress; public CodePointerFlags CodePointerFlags {get; private set;} @@ -190,16 +190,11 @@ public PrecodeBuilder(AllocationRange allocationRange, MockMemorySpace.Builder b Builder = builder; PrecodeAllocator = builder.CreateAllocator(allocationRange.PrecodeDescriptorStart, allocationRange.PrecodeDescriptorEnd); StubDataPageAllocator = builder.CreateAllocator(allocationRange.StubDataPageStart, allocationRange.StubDataPageEnd); - Types = typeInfoCache ?? CreateTypeInfoCache(Builder.TargetTestHelpers); + Types = typeInfoCache ?? GetTypes(Builder.TargetTestHelpers); } - public Dictionary CreateTypeInfoCache(TargetTestHelpers targetTestHelpers) { - var typeInfo = new Dictionary(); - AddToTypeInfoCache(typeInfo, targetTestHelpers); - return typeInfo; - } - - public void AddToTypeInfoCache(Dictionary typeInfoCache, TargetTestHelpers targetTestHelpers) { + public Dictionary GetTypes(TargetTestHelpers targetTestHelpers) { + Dictionary types = new(); var layout = targetTestHelpers.LayoutFields([ new(nameof(Data.PrecodeMachineDescriptor.StubCodePageSize), DataType.uint32), new(nameof(Data.PrecodeMachineDescriptor.OffsetOfPrecodeType), DataType.uint8), @@ -211,7 +206,7 @@ public void AddToTypeInfoCache(Dictionary typeInfoCac new(nameof(Data.PrecodeMachineDescriptor.FixupPrecodeType), DataType.uint8), new(nameof(Data.PrecodeMachineDescriptor.ThisPointerRetBufPrecodeType), DataType.uint8), ]); - typeInfoCache[DataType.PrecodeMachineDescriptor] = new Target.TypeInfo() { + types[DataType.PrecodeMachineDescriptor] = new Target.TypeInfo() { Fields = layout.Fields, Size = layout.Stride, }; @@ -219,10 +214,11 @@ public void AddToTypeInfoCache(Dictionary typeInfoCac new(nameof(Data.StubPrecodeData.Type), DataType.uint8), new(nameof(Data.StubPrecodeData.MethodDesc), DataType.pointer), ]); - typeInfoCache[DataType.StubPrecodeData] = new Target.TypeInfo() { + types[DataType.StubPrecodeData] = new Target.TypeInfo() { Fields = layout.Fields, Size = layout.Stride, }; + return types; } private void SetCodePointerFlags(PrecodeTestDescriptor test) diff --git a/src/native/managed/cdacreader/tests/TargetTestHelpers.cs b/src/native/managed/cdacreader/tests/TargetTestHelpers.cs index 41e47ca83766b7..4496edd8305a9a 100644 --- a/src/native/managed/cdacreader/tests/TargetTestHelpers.cs +++ b/src/native/managed/cdacreader/tests/TargetTestHelpers.cs @@ -19,6 +19,9 @@ public TargetTestHelpers(MockTarget.Architecture arch) } public int PointerSize => Arch.Is64Bit ? sizeof(ulong) : sizeof(uint); + + public ulong MaxSignedTargetAddress => (ulong)(Arch.Is64Bit ? long.MaxValue : int.MaxValue); + public int ContractDescriptorSize => ContractDescriptor.Size(Arch.Is64Bit);