Skip to content

[cdac] Simplify usage of TestPlaceholderTarget in tests #109873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elinor-fung merged 7 commits into from
Nov 20, 2024
Merged

[cdac] Simplify usage of TestPlaceholderTarget in tests #109873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24cc4b8
Make TestPlaceholderTarget constructor take a dictionary of type infos
elinor-fung Nov 13, 2024
a0e4baa
Remove unnecessary subclasses of TestPlaceholderTarget
elinor-fung Nov 13, 2024
fab5b6f
Pass globals to TestPlaceholderTarget
elinor-fung Nov 14, 2024
22fc7bc
Remove subclasses of TestPlaceholderTarget
elinor-fung Nov 14, 2024
0d3445a
Stop requiring builder to be marked as created when getting a read co…
elinor-fung Nov 14, 2024
a6134be
Use Moq for ContractRegistry
elinor-fung Nov 14, 2024
61acd52
PR feedback
elinor-fung Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 29 additions & 35 deletions src/native/managed/cdacreader/tests/CodeVersionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using Microsoft.Diagnostics.DataContractReader.Contracts;
using Moq;
using Xunit;

namespace Microsoft.Diagnostics.DataContractReader.UnitTests;
Expand Down Expand Up @@ -226,43 +227,36 @@ TargetPointer ILoader.GetModuleLookupMapElement(TargetPointer tableAddress, uint
}
}

internal class CVTestTarget : TestPlaceholderTarget
internal static Target CreateTarget(
MockTarget.Architecture arch,
IReadOnlyCollection<MockMethodDesc> methodDescs = null,
IReadOnlyCollection<MockMethodTable> methodTables = null,
IReadOnlyCollection<MockCodeBlockStart> codeBlocks = null,
IReadOnlyCollection<MockModule> modules = null,
MockCodeVersions builder = null)
{
public static CVTestTarget FromBuilder(MockTarget.Architecture arch, IReadOnlyCollection<MockMethodDesc> methodDescs, IReadOnlyCollection<MockMethodTable> methodTables, IReadOnlyCollection<MockCodeBlockStart> codeBlocks, IReadOnlyCollection<MockModule> modules, MockCodeVersions builder)
{
builder.MarkCreated();
return new CVTestTarget(arch, reader: builder.Builder.GetReadContext().ReadFromTarget, typeInfoCache: builder.Types,
methodDescs: methodDescs, methodTables: methodTables, codeBlocks: codeBlocks, modules: modules);
}

public CVTestTarget(MockTarget.Architecture arch, IReadOnlyCollection<MockMethodDesc>? methodDescs = null,
IReadOnlyCollection<MockMethodTable>? methodTables = null,
IReadOnlyCollection<MockCodeBlockStart>? codeBlocks = null,
IReadOnlyCollection<MockModule>? modules = null,
ReadFromTargetDelegate reader = null,
Dictionary<DataType, TypeInfo>? typeInfoCache = null)
: base(arch, reader)
{
IExecutionManager mockExecutionManager = new MockExecutionManager(codeBlocks ?? []);
IRuntimeTypeSystem mockRuntimeTypeSystem = new MockRuntimeTypeSystem(this, methodDescs ?? [], methodTables ?? []);
ILoader loader = new MockLoader(modules ?? []);
if (typeInfoCache != null)
SetTypeInfoCache(typeInfoCache);
IContractFactory<ICodeVersions> cvfactory = new CodeVersionsFactory();
SetContracts(new TestRegistry() {
CodeVersionsContract = new (() => cvfactory.CreateContract(this, 1)),
ExecutionManagerContract = new (() => mockExecutionManager),
RuntimeTypeSystemContract = new (() => mockRuntimeTypeSystem),
LoaderContract = new (() => loader),
});
}
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 ?? []);
ILoader loader = new MockLoader(modules ?? []);
IContractFactory<ICodeVersions> cvfactory = new CodeVersionsFactory();
ContractRegistry reg = Mock.Of<ContractRegistry>(
c => c.CodeVersions == cvfactory.CreateContract(target, 1)
&& c.ExecutionManager == mockExecutionManager
&& c.RuntimeTypeSystem == mockRuntimeTypeSystem
&& c.Loader == loader);
target.SetContracts(reg);
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);
Expand All @@ -286,7 +280,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);
Expand Down Expand Up @@ -320,7 +314,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

Expand Down Expand Up @@ -367,7 +361,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

Expand Down Expand Up @@ -430,7 +424,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

Expand Down Expand Up @@ -501,7 +495,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

Expand Down
23 changes: 14 additions & 9 deletions src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTestBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -13,6 +14,7 @@ internal class ExecutionManagerTestBuilder
{
public const ulong ExecutionManagerCodeRangeMapAddress = 0x000a_fff0;

const bool UseFunclets = true;

@AaronRobinsonMSFT AaronRobinsonMSFT Nov 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? A const bool seems odd to me. If this is important, let's add a comment what this is trying to capture.

@elinor-fung elinor-fung Nov 19, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing why that is odd? It's just indicating that the test builder (currently) always specifies to use funclets and it is not configurable on the instance.

@elinor-fung elinor-fung Nov 20, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move it to something local in a follow-up,.

const int RealCodeHeaderSize = 0x08; // must be big enough for the offsets of RealCodeHeader size in ExecutionManagerTestTarget, below

public struct AllocationRange
Expand Down Expand Up @@ -159,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()
{
Expand Down Expand Up @@ -259,6 +257,7 @@ public static RangeSectionMapTestBuilder CreateRangeSection(MockTarget.Architect

internal MockMemorySpace.Builder Builder { get; }
internal Dictionary<DataType, Target.TypeInfo> Types { get; }
internal (string Name, ulong Value)[] Globals { get; }

private readonly RangeSectionMapTestBuilder _rsmBuilder;

Expand Down Expand Up @@ -287,11 +286,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)
Expand Down Expand Up @@ -469,6 +476,4 @@ public TargetPointer AddReadyToRunModule(TargetPointer r2rInfo)

return r2rModule.Address;
}

public void MarkCreated() => Builder.MarkCreated();
}
95 changes: 17 additions & 78 deletions src/native/managed/cdacreader/tests/ExecutionManager/ExecutionManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,75 +11,25 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager;

public class ExecutionManagerTests
{
internal class ExecutionManagerTestTarget : TestPlaceholderTarget
private static Target CreateTarget(ExecutionManagerTestBuilder emBuilder)
{
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);
}

public ExecutionManagerTestTarget(int version, MockTarget.Architecture arch, ReadFromTargetDelegate dataReader, TargetPointer topRangeSectionMap, Dictionary<DataType, TypeInfo> typeInfoCache)
: base(arch, dataReader)
{
_topRangeSectionMap = topRangeSectionMap;
SetTypeInfoCache(typeInfoCache);
IContractFactory<IExecutionManager> emfactory = new ExecutionManagerFactory();
SetContracts(new TestRegistry() {
ExecutionManagerContract = new (() => emfactory.CreateContract(this, version)),
PlatformMetadataContract = new (() => new Mock<IPlatformMetadata>().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<T>(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<T>(name);
}
var arch = emBuilder.Builder.TargetTestHelpers.Arch;
TestPlaceholderTarget.ReadFromTargetDelegate reader = emBuilder.Builder.GetReadContext().ReadFromTarget;
var target = new TestPlaceholderTarget(arch, reader, emBuilder.Types, emBuilder.Globals);
IContractFactory<IExecutionManager> emfactory = new ExecutionManagerFactory();
ContractRegistry reg = Mock.Of<ContractRegistry>(
c => c.ExecutionManager == emfactory.CreateContract(target, emBuilder.Version)
&& c.PlatformMetadata == new Mock<IPlatformMetadata>().Object);
target.SetContracts(reg);
return target;
}

[Theory]
[MemberData(nameof(StdArchAllVersions))]
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);
Expand All @@ -92,8 +42,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);
Expand Down Expand Up @@ -125,9 +74,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);
Expand Down Expand Up @@ -169,9 +116,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);
Expand Down Expand Up @@ -212,9 +157,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);
Expand Down Expand Up @@ -249,9 +192,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);
Expand Down Expand Up @@ -300,9 +241,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);
Expand Down
32 changes: 3 additions & 29 deletions src/native/managed/cdacreader/tests/ExecutionManager/HashMapTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,6 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests.ExecutionManager;

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)
{
_globals = hashMap.Globals;
SetTypeInfoCache(hashMap.Types);
}

public override T ReadGlobal<T>(string name)
{
foreach (var global in _globals)
{
if (global.Name == name)
return T.CreateChecked(global.Value);
}

return base.ReadGlobal<T>(name);
}
}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public void GetValue(MockTarget.Architecture arch)
Expand All @@ -47,9 +24,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 = new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals);

var lookup = HashMapLookup.Create(target);
var ptrLookup = PtrHashMapLookup.Create(target);
Expand Down Expand Up @@ -84,9 +60,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 = new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals);

var lookup = HashMapLookup.Create(target);
var ptrLookup = PtrHashMapLookup.Create(target);
Expand All @@ -110,9 +85,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 = new TestPlaceholderTarget(builder.TargetTestHelpers.Arch, builder.GetReadContext().ReadFromTarget, hashMap.Types, hashMap.Globals);

{
var lookup = HashMapLookup.Create(target);
Expand Down
Loading