Repository navigation
Consolidate cDAC entrypoint MethodDesc resolution - #135450
Conversation
Route RuntimeTypeSystem and MethodDesc validation through ExecutionManager.NonVirtualEntry2MethodDesc and remove the portable-only PrecodeStubs contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removed redundant information about using ExecutionManager for resolving entry points.
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pcshrosbree
left a comment
There was a problem hiding this comment.
I compared this head's reader with the merge base's on six linux-x64 dumps (mini, heap and full; R2R and JIT) from a Checked build at e2d92b2. GetMethodDescForSlot on every vtable slot of every type in the CoreLib and test-app TypeDef maps gave 33,092 identical observations, including minidump slots where both readers throw.
Nit: the description could separate temporary-entry-point validation, which now prefers PrecodeStubs when it is advertised, from the paths that go through NonVirtualEntry2MethodDesc.
Test patch (3 test files; applies to e2d92b2 with git apply; 3324/3324 pass)
diff --git a/src/native/managed/cdac/tests/UnitTests/ExecutionManager/ExecutionManagerTests.cs b/src/native/managed/cdac/tests/UnitTests/ExecutionManager/ExecutionManagerTests.cs
index b8b3d9152b9..8977d747123 100644
--- a/src/native/managed/cdac/tests/UnitTests/ExecutionManager/ExecutionManagerTests.cs
+++ b/src/native/managed/cdac/tests/UnitTests/ExecutionManager/ExecutionManagerTests.cs
@@ -1928,6 +1928,97 @@ public void NonVirtualEntry2MethodDesc_PortableEntryPoint_ReturnsOwningMethodDes
Assert.Equal(expectedMethodDesc, target.Contracts.ExecutionManager.NonVirtualEntry2MethodDesc(new TargetCodePointer(entryPoint.Address)));
}
+ // The non-portable paths of NonVirtualEntry2MethodDesc, mirroring native NonVirtualEntry2MethodDesc
+ // (src/coreclr/vm/method.cpp): jitted code resolves through the jit manager, a precode range list
+ // resolves through the precode, and an address in no range section resolves to null.
+
+ [Theory]
+ [MemberData(nameof(StdArchAllVersions))]
+ public void NonVirtualEntry2MethodDesc_JittedCode_ReturnsMethodDesc(string version, MockTarget.Architecture arch)
+ {
+ const ulong codeRangeStart = 0x0a0a_0000u;
+ const uint codeRangeSize = 0xc000u;
+ const uint methodSize = 0x450;
+ const ulong jitManagerAddress = 0x000b_ff00;
+ const ulong expectedMethodDescAddress = 0x0101_aaa0;
+
+ MockExecutionManagerBuilder emBuilder = new(version, arch, MockExecutionManagerBuilder.DefaultAllocationRange);
+ MockExecutionManagerBuilder.JittedCodeRange jittedCode = emBuilder.AllocateJittedCodeRange(codeRangeStart, codeRangeSize);
+ ulong methodStart = emBuilder.AddJittedMethod(jittedCode, methodSize, expectedMethodDescAddress).CodeAddress;
+ NibbleMapTestBuilderBase nibBuilder = emBuilder.CreateNibbleMap(codeRangeStart, codeRangeSize);
+ nibBuilder.AllocateCodeChunk(new TargetCodePointer(methodStart), methodSize);
+ MockCodeHeapListNode codeHeapListNode = emBuilder.AddCodeHeapListNode(0, codeRangeStart, codeRangeStart + codeRangeSize, codeRangeStart, nibBuilder.NibbleMapFragment.Address);
+ MockRangeSection rangeSection = emBuilder.AddRangeSection(jittedCode, jitManagerAddress, codeHeapListNode.Address);
+ _ = emBuilder.AddRangeSectionFragment(jittedCode, rangeSection.Address);
+
+ // Strict: resolving jitted code must not consult the precode decoder.
+ Mock<IPrecodeStubs> precodeStubs = new(MockBehavior.Strict);
+ Target target = CreateTarget(emBuilder, configureTarget: targetBuilder => targetBuilder
+ .AddContract<IFeatureFlags>(version: "c1")
+ .AddMockContract(precodeStubs));
+
+ Assert.Equal(
+ new TargetPointer(expectedMethodDescAddress),
+ target.Contracts.ExecutionManager.NonVirtualEntry2MethodDesc(new TargetCodePointer(methodStart)));
+ }
+
+ [Theory]
+ [MemberData(nameof(StdArchAllVersions))]
+ public void NonVirtualEntry2MethodDesc_PrecodeRangeList_ReturnsPrecodeMethodDesc(string version, MockTarget.Architecture arch)
+ {
+ TargetCodePointer precode = new(PrecodeRangeStart + 0x100);
+ TargetPointer expectedMethodDesc = new(0x0101_aaa0);
+ Mock<IPrecodeStubs> precodeStubs = new(MockBehavior.Strict);
+ precodeStubs.Setup(p => p.GetMethodDescFromStubAddress(precode)).Returns(expectedMethodDesc);
+
+ Target target = CreatePrecodeRangeListTarget(version, arch, precodeStubs);
+
+ Assert.Equal(expectedMethodDesc, target.Contracts.ExecutionManager.NonVirtualEntry2MethodDesc(precode));
+ }
+
+ [Theory]
+ [MemberData(nameof(StdArchAllVersions))]
+ public void NonVirtualEntry2MethodDesc_RangeListAddressThatIsNotAPrecode_ReturnsNull(string version, MockTarget.Architecture arch)
+ {
+ TargetCodePointer notAPrecode = new(PrecodeRangeStart + 0x100);
+ Mock<IPrecodeStubs> precodeStubs = new(MockBehavior.Strict);
+ precodeStubs.Setup(p => p.GetMethodDescFromStubAddress(notAPrecode)).Throws(new InvalidOperationException("Invalid precode type"));
+
+ Target target = CreatePrecodeRangeListTarget(version, arch, precodeStubs);
+
+ Assert.Equal(TargetPointer.Null, target.Contracts.ExecutionManager.NonVirtualEntry2MethodDesc(notAPrecode));
+ }
+
+ [Theory]
+ [MemberData(nameof(StdArchAllVersions))]
+ public void NonVirtualEntry2MethodDesc_AddressInNoRangeSection_ReturnsNullWithoutDecoding(string version, MockTarget.Architecture arch)
+ {
+ // Strict with no setups: native returns NULL here without reading the address as a precode.
+ Mock<IPrecodeStubs> precodeStubs = new(MockBehavior.Strict);
+
+ Target target = CreatePrecodeRangeListTarget(version, arch, precodeStubs);
+
+ Assert.Equal(TargetPointer.Null, target.Contracts.ExecutionManager.NonVirtualEntry2MethodDesc(new TargetCodePointer(PrecodeRangeStart + PrecodeRangeSize + 0x100)));
+ }
+
+ private const ulong PrecodeRangeStart = 0x0b0b_0000u;
+ private const uint PrecodeRangeSize = 0x1000u;
+
+ private static Target CreatePrecodeRangeListTarget(string version, MockTarget.Architecture arch, Mock<IPrecodeStubs> precodeStubs)
+ {
+ const ulong jitManagerAddress = 0x000b_ff00;
+ const int stubCodeBlockKindFixupPrecode = 4; // STUB_CODE_BLOCK_FIXUPPRECODE
+
+ MockExecutionManagerBuilder emBuilder = new(version, arch, MockExecutionManagerBuilder.DefaultAllocationRange);
+ MockExecutionManagerBuilder.JittedCodeRange precodeRange = emBuilder.AllocateJittedCodeRange(PrecodeRangeStart, PrecodeRangeSize);
+ MockRangeSection rangeSection = emBuilder.AddRangeListRangeSection(precodeRange, jitManagerAddress, stubCodeBlockKindFixupPrecode);
+ _ = emBuilder.AddRangeSectionFragment(precodeRange, rangeSection.Address);
+
+ return CreateTarget(emBuilder, configureTarget: targetBuilder => targetBuilder
+ .AddContract<IFeatureFlags>(version: "c1")
+ .AddMockContract(precodeStubs));
+ }
+
private sealed class PortableEntryPointFixture
{
public required Target Target { get; init; }
diff --git a/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs b/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs
index c5971f3370a..c0871c9a158 100644
--- a/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs
+++ b/src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs
@@ -77,7 +77,8 @@ private static uint GetMethodDescBaseSize(MockDescriptors.MockMethodDescriptorsB
private static IRuntimeTypeSystem CreateRuntimeTypeSystemContract(
MockTarget.Architecture arch,
Action<MockDescriptors.MockMethodDescriptorsBuilder> configure,
- Mock<IExecutionManager>? mockExecutionManager = null)
+ Mock<IExecutionManager>? mockExecutionManager = null,
+ Action<TestPlaceholderTarget.Builder>? configureTarget = null)
{
var targetBuilder = new TestPlaceholderTarget.Builder(arch);
MockDescriptors.RuntimeTypeSystem rtsBuilder = new(targetBuilder.MemoryBuilder);
@@ -87,15 +88,15 @@ private static IRuntimeTypeSystem CreateRuntimeTypeSystemContract(
configure(methodDescBuilder);
mockExecutionManager ??= new Mock<IExecutionManager>();
- var target = targetBuilder
+ targetBuilder
.AddTypes(CreateContractTypes(methodDescBuilder))
.AddGlobals(CreateContractGlobals(methodDescBuilder))
.AddContract<IRuntimeTypeSystem>(version: "c1")
.AddContract<ILoader>(version: "c1")
.AddMockContract(new Mock<IPlatformMetadata>())
- .AddMockContract(mockExecutionManager)
- .Build();
- return target.Contracts.RuntimeTypeSystem;
+ .AddMockContract(mockExecutionManager);
+ configureTarget?.Invoke(targetBuilder);
+ return targetBuilder.Build().Contracts.RuntimeTypeSystem;
}
[Theory]
@@ -957,6 +958,122 @@ public void Validation_NativeCodeSlot_PrecodeFallback_WrongMethodDesc_Fails(Mock
Assert.Throws<ArgumentException>(() => rts.GetMethodDescHandle(methodDescAddress));
}
+ // A populated vtable slot resolves through ExecutionManager.NonVirtualEntry2MethodDesc, as in native
+ // MethodTable::GetMethodDescForSlot_NoThrow, whatever kind of entry point the slot holds.
+ [Theory]
+ [ClassData(typeof(MockTarget.StdArch))]
+ public void GetMethodDescForSlot_PopulatedVtableSlot_ResolvesThroughNonVirtualEntry2MethodDesc(MockTarget.Architecture arch)
+ {
+ TargetCodePointer precodeEntryPoint = new(0x0789_ab00);
+ TargetCodePointer jittedEntryPoint = new(0x0789_cd00);
+ TargetPointer precodeMethodDesc = new(0x0101_aaa0);
+ TargetPointer jittedMethodDesc = new(0x0101_bbb0);
+ TargetPointer methodTable = TargetPointer.Null;
+ Mock<IExecutionManager> mockExecutionManager = new();
+ mockExecutionManager.Setup(em => em.NonVirtualEntry2MethodDesc(precodeEntryPoint)).Returns(precodeMethodDesc);
+ mockExecutionManager.Setup(em => em.NonVirtualEntry2MethodDesc(jittedEntryPoint)).Returns(jittedMethodDesc);
+
+ IRuntimeTypeSystem rts = CreateRuntimeTypeSystemContract(arch, methodDescBuilder =>
+ {
+ MockDescriptors.RuntimeTypeSystem rtsBuilder = methodDescBuilder.RTSBuilder;
+ MockEEClass eeClass = rtsBuilder.AddEEClass(string.Empty);
+ eeClass.NumNonVirtualSlots = 0;
+ MockMethodTable mt = rtsBuilder.AddMethodTableWithVtable(string.Empty, [precodeEntryPoint.Value, jittedEntryPoint.Value]);
+ mt.BaseSize = rtsBuilder.Builder.TargetTestHelpers.ObjectBaseSize;
+ eeClass.MethodTable = mt.Address;
+ mt.EEClassOrCanonMT = eeClass.Address;
+ methodTable = mt.Address;
+ }, mockExecutionManager);
+
+ ITypeHandle typeHandle = rts.GetTypeHandle(methodTable);
+ Assert.Equal(precodeMethodDesc, rts.GetMethodDescForSlot(typeHandle, 0));
+ Assert.Equal(jittedMethodDesc, rts.GetMethodDescForSlot(typeHandle, 1));
+ }
+
+ public static IEnumerable<object[]> TemporaryEntryPointValidationData()
+ {
+ foreach (object[] arch in new MockTarget.StdArch())
+ {
+ foreach (bool portableEntrypoints in new[] { false, true })
+ {
+ foreach (bool resolvesToSelf in new[] { true, false })
+ {
+ yield return [arch[0], portableEntrypoints, resolvesToSelf];
+ }
+ }
+ }
+ }
+
+ // ValidateMethodDescPointer requires a MethodDesc's temporary entry point to resolve back to it.
+ // Without portable entry points it decodes the precode directly, like the legacy DAC's DacValidateMD
+ // (MethodDesc::GetMethodDescFromPrecode), so it does not depend on the ExecutionManager range lookup.
+ // With portable entry points no PrecodeStubs contract is advertised and NonVirtualEntry2MethodDesc is used.
+ [Theory]
+ [MemberData(nameof(TemporaryEntryPointValidationData))]
+ public void Validation_TemporaryEntryPoint_MustResolveToMethodDesc(MockTarget.Architecture arch, bool portableEntrypoints, bool resolvesToSelf)
+ {
+ TargetPointer methodDescAddress = TargetPointer.Null;
+ TargetCodePointer temporaryEntryPoint = new(0x0789_abc0);
+ TargetPointer otherMethodDesc = new(0xDEAD_BEE0);
+ Mock<IExecutionManager> mockExecutionManager = new();
+ Mock<IPrecodeStubs> mockPrecodeStubs = new(MockBehavior.Strict);
+ Target.TypeInfo codeDataType = default;
+
+ IRuntimeTypeSystem rts = CreateRuntimeTypeSystemContract(arch, methodDescBuilder =>
+ {
+ TargetTestHelpers helpers = methodDescBuilder.Builder.TargetTestHelpers;
+ TargetPointer methodTable = AddMethodTable(methodDescBuilder.RTSBuilder);
+ byte chunkSize = (byte)(methodDescBuilder.MethodDescLayout.Size / methodDescBuilder.MethodDescAlignment);
+ MockMethodDescChunk chunk = methodDescBuilder.AddMethodDescChunk(string.Empty, chunkSize);
+ chunk.MethodTable = methodTable.Value;
+ chunk.Size = chunkSize;
+ chunk.Count = 1;
+
+ // MethodDescCodeData holds only the temporary entry point here.
+ codeDataType = new Target.TypeInfo
+ {
+ Fields = new Dictionary<string, Target.FieldInfo> { [nameof(Data.MethodDescCodeData.TemporaryEntryPoint)] = new() { Offset = 0 } },
+ Size = (uint)helpers.PointerSize,
+ };
+ MockMemorySpace.HeapFragment codeData = methodDescBuilder.Builder.CreateAllocator(0x0444_0000, 0x0444_1000)
+ .Allocate((ulong)helpers.PointerSize, "MethodDescCodeData");
+ helpers.WritePointer(codeData.Data.AsSpan(), temporaryEntryPoint);
+
+ MockMethodDesc methodDesc = chunk.GetMethodDescAtChunkIndex(0, methodDescBuilder.MethodDescLayout);
+ methodDesc.Flags = (ushort)MethodClassification.IL;
+ methodDesc.EntryPointFlags = (byte)MethodDescFlags_1.MethodDescEntryPointFlags.TemporaryEntryPointAssigned;
+ methodDesc.CodeData = codeData.Address;
+ methodDescAddress = new TargetPointer(methodDesc.Address);
+ }, mockExecutionManager, targetBuilder =>
+ {
+ targetBuilder.AddTypes(new Dictionary<DataType, Target.TypeInfo> { [DataType.MethodDescCodeData] = codeDataType });
+ if (!portableEntrypoints)
+ targetBuilder.AddMockContract(mockPrecodeStubs);
+ });
+
+ TargetPointer resolved = resolvesToSelf ? methodDescAddress : otherMethodDesc;
+ if (portableEntrypoints)
+ {
+ mockExecutionManager.Setup(em => em.NonVirtualEntry2MethodDesc(temporaryEntryPoint)).Returns(resolved);
+ }
+ else
+ {
+ // The ExecutionManager reports nothing for this address (for example, its range section is not in a
+ // minidump); validation must still succeed through the precode.
+ mockExecutionManager.Setup(em => em.NonVirtualEntry2MethodDesc(temporaryEntryPoint)).Returns(TargetPointer.Null);
+ mockPrecodeStubs.Setup(p => p.GetMethodDescFromStubAddress(temporaryEntryPoint)).Returns(resolved);
+ }
+
+ if (resolvesToSelf)
+ {
+ Assert.Equal(methodDescAddress, rts.GetMethodDescHandle(methodDescAddress).Address);
+ }
+ else
+ {
+ Assert.Throws<ArgumentException>(() => rts.GetMethodDescHandle(methodDescAddress));
+ }
+ }
+
private static TargetPointer AddMethodTable(MockDescriptors.RuntimeTypeSystem rtsBuilder, ushort numVirtuals = 5)
{
MockEEClass eeClass = rtsBuilder.AddEEClass(string.Empty);
diff --git a/src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs b/src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs
index 81b04ccfe7c..1b865fac4e6 100644
--- a/src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs
+++ b/src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs
@@ -658,6 +658,32 @@ private TView Add<TView>(Layout<TView> layout, ulong size, string name)
return layout.Create(fragment.Data.AsMemory(), fragment.Address);
}
+ /// <summary>
+ /// Allocates a method table followed by one vtable indirection and its chunk of virtual slots
+ /// (MethodTable::GetSlotPtrRaw: the indirections start immediately after the MethodTable).
+ /// Sets <c>NumVirtuals</c> to the number of slots given.
+ /// </summary>
+ internal MockMethodTable AddMethodTableWithVtable(string name, ulong[] vtableSlots)
+ {
+ const int SlotsPerIndirection = 8;
+ if (vtableSlots.Length > SlotsPerIndirection)
+ throw new ArgumentOutOfRangeException(nameof(vtableSlots));
+
+ int pointerSize = Builder.TargetTestHelpers.PointerSize;
+ int indirectionOffset = MethodTableLayout.Size;
+ int chunkOffset = indirectionOffset + pointerSize;
+ MockMemorySpace.HeapFragment fragment = TypeSystemAllocator.Allocate(
+ (ulong)(chunkOffset + SlotsPerIndirection * pointerSize), $"MethodTable+vtable '{name}'");
+
+ Builder.TargetTestHelpers.WritePointer(fragment.Data.AsSpan(indirectionOffset, pointerSize), fragment.Address + (ulong)chunkOffset);
+ for (int i = 0; i < vtableSlots.Length; i++)
+ Builder.TargetTestHelpers.WritePointer(fragment.Data.AsSpan(chunkOffset + i * pointerSize, pointerSize), vtableSlots[i]);
+
+ MockMethodTable mt = MethodTableLayout.Create(fragment.Data.AsMemory(0, MethodTableLayout.Size), fragment.Address);
+ mt.NumVirtuals = (ushort)vtableSlots.Length;
+ return mt;
+ }
+
/// <summary>
/// Allocates a method table together with a CGCDesc immediately before it in memory.
/// </summary>Note
This review was drafted with AI assistance (Anthropic Claude, reviewed with OpenAI Codex). The builds, dumps and test runs behind it were run locally.
|
/ba-g unrelated stress fails |
|
Follow-up with the tests: #135470 |
Follow-up to #135450. Every populated vtable slot now resolves through `NonVirtualEntry2MethodDesc`, but its non-portable paths had no unit coverage. Neither did the temporary-entry-point check in `ValidateMethodDescPointer`. Before this change, breaking any of the following still passed every test: - the jitted-code branch, the precode branch, or the catch; - `GetMethodDescForSlot`; - either variant of the validation choice. **Tests added:** - **`ExecutionManagerTests`:** `NonVirtualEntry2MethodDesc` for - jitted code; - a `FIXUPPRECODE` range list; - a range-list address that is not a precode (null); - an address in no range section (null, and never decoded). - **`MethodDescTests`:** - `GetMethodDescForSlot` resolving a populated vtable slot; - `ValidateMethodDescPointer` requiring the temporary entry point to resolve back to the MethodDesc, over portable × resolves-to-self. - **`MockDescriptors`:** `AddMethodTableWithVtable`. With these tests, each of 12 single-guard mutations fails, and each fails in one test method: - each `NonVirtualEntry2MethodDesc` branch; - the slot path; - both validation variants; - the validation compare; - the `ValidateForDataAccess` guards. UnitTests: 3324/3324 pass on linux-x64, osx-arm64 and windows-x64. On osx-arm64 and windows-x64 the mutation that makes `NonVirtualEntry2MethodDesc` return null for a precode address was also re-run, and its test fails on both. > [!NOTE] > This PR was prepared with AI assistance (Anthropic Claude, reviewed with OpenAI Codex). The tests and the mutation runs were executed locally.
Route RuntimeTypeSystem through ExecutionManager.NonVirtualEntry2MethodDesc and remove the portable-only PrecodeStubs contract.