Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ internal TargetPointer CanonMT
}
}
}

internal readonly bool ValidateReadable() => ValidateDataReadable<MethodTable>(_target, Address);
}

internal struct NonValidatedEEClass
Expand All @@ -84,6 +86,8 @@ internal NonValidatedEEClass(Target target, TargetPointer eeClassPointer)
}

internal TargetPointer MethodTable => _target.ReadPointer(Address + (ulong)_type.Fields[nameof(MethodTable)].Offset);

internal readonly bool ValidateReadable() => ValidateDataReadable<EEClass>(_target, Address);
}

internal static NonValidatedMethodTable GetMethodTableData(Target target, TargetPointer methodTablePointer)
Expand All @@ -108,6 +112,11 @@ private bool ValidateMethodTablePointer(NonValidatedMethodTable umt)
{
try
{
// Make sure that we can read the method table's data.
if (!umt.ValidateReadable())
{
return false;
}
if (!ValidateThrowing(umt))
{
return false;
Expand Down Expand Up @@ -146,6 +155,10 @@ private bool ValidateThrowing(NonValidatedMethodTable methodTable)
if (eeClassPtr != TargetPointer.Null)
{
NonValidatedEEClass eeClass = GetEEClassData(_target, eeClassPtr);
if (!eeClass.ValidateReadable())
{
return false;
}
TargetPointer methodTablePtrFromClass = eeClass.MethodTable;
if (methodTable.Address == methodTablePtrFromClass)
{
Expand All @@ -154,6 +167,10 @@ private bool ValidateThrowing(NonValidatedMethodTable methodTable)
if (methodTable.Flags.HasInstantiation || methodTable.Flags.IsArray)
{
NonValidatedMethodTable methodTableFromClass = GetMethodTableData(_target, methodTablePtrFromClass);
if (!methodTableFromClass.ValidateReadable())
{
return false;
}
TargetPointer classFromMethodTable = GetClassThrowing(methodTableFromClass);
return classFromMethodTable == eeClassPtr;
}
Expand All @@ -174,6 +191,19 @@ private bool ValidateMethodTableAdHoc(NonValidatedMethodTable methodTable)
return true;
}

private static bool ValidateDataReadable<T>(Target target, TargetPointer dataAddress) where T : IData<T>
{
try
{
T dataClass = T.Create(target, dataAddress);
return true;
}
catch (VirtualReadException)
{
return false;
}
}

private TargetPointer GetClassThrowing(NonValidatedMethodTable methodTable)
{
TargetPointer eeClassOrCanonMT = methodTable.EEClassOrCanonMT;
Expand All @@ -186,6 +216,10 @@ private TargetPointer GetClassThrowing(NonValidatedMethodTable methodTable)
{
TargetPointer canonicalMethodTablePtr = methodTable.CanonMT;
NonValidatedMethodTable umt = GetMethodTableData(_target, canonicalMethodTablePtr);
if (!umt.ValidateReadable())
{
throw new InvalidOperationException("canon MT is not readable");
}
return umt.EEClass;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

<ItemGroup>
<Compile Include="$(LibrariesProjectRoot)Common\src\System\HResults.cs" Link="Common\System\HResults.cs" />
<Compile Include="..\TestHelpers.cs" Link="TestHelpers.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Diagnostics.DataContractReader.Contracts;
using Microsoft.Diagnostics.DataContractReader.Legacy;
using Xunit;
using static Microsoft.Diagnostics.DataContractReader.Tests.TestHelpers;

namespace Microsoft.Diagnostics.DataContractReader.DumpTests;

Expand Down Expand Up @@ -95,7 +96,7 @@ public unsafe void VarargPInvoke_GetMethodTableDataForILStubFrame(TestConfigurat
TargetPointer mt = rts.GetMethodTable(mdHandle);
DacpMethodTableData mtData;
int hr = sosDac.GetMethodTableData(new ClrDataAddress(mt), &mtData);
Assert.Equal(HResults.S_OK, hr);
AssertHResult(HResults.S_OK, hr);

return;
}
Expand Down Expand Up @@ -135,7 +136,7 @@ public unsafe void VarargPInvoke_GetILAddressMapForILStub_ReturnsEFail(TestConfi
Target, mdHandle, TargetPointer.Null, legacyImpl: null);
uint mapNeeded;
int hr = methodInstance.GetILAddressMap(0, &mapNeeded, null);
Assert.Equal(HResults.E_FAIL, hr);
AssertHResult(HResults.E_FAIL, hr);

return;
}
Expand Down
112 changes: 112 additions & 0 deletions src/native/managed/cdac/tests/MethodTableTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

using System;
using Microsoft.Diagnostics.DataContractReader.Contracts;
using Microsoft.Diagnostics.DataContractReader.Legacy;
using Microsoft.Diagnostics.DataContractReader.RuntimeTypeSystemHelpers;
using Moq;
using Xunit;
using static Microsoft.Diagnostics.DataContractReader.Tests.TestHelpers;

namespace Microsoft.Diagnostics.DataContractReader.Tests;

Expand Down Expand Up @@ -226,4 +228,114 @@ public void ValidateArrayInstMethodTable(MockTarget.Architecture arch)
});

}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public unsafe void GetMethodTableDataReturnsEInvalidArgWhenEEClassPartiallyReadable(MockTarget.Architecture arch)
{
// Reproduces the HResult mismatch from dotnet/diagnostics CI where the cDAC returned
// CORDBG_E_READVIRTUAL_FAILURE (0x80131c49) instead of E_INVALIDARG (0x80070057)
// when GetMethodTableData was called with an address whose MethodTable validation
// passes but subsequent EEClass reads fail.
TargetPointer methodTablePtr = default;

RTSContractHelper(arch,
(rtsBuilder) =>
{
TargetTestHelpers helpers = rtsBuilder.Builder.TargetTestHelpers;

// Create a full MethodTable that will pass validation
methodTablePtr = rtsBuilder.AddMethodTable("PartialEEClass MT",
mtflags: default, mtflags2: default, baseSize: helpers.ObjectBaseSize,
module: TargetPointer.Null, parentMethodTable: TargetPointer.Null,
numInterfaces: 0, numVirtuals: 3);

// Create a tiny EEClass fragment — only the MethodTable pointer field.
// Previously, validation only read NonValidatedEEClass.MethodTable (at offset 0),
// so this minimal fragment was sufficient for validation to pass while later
// EEClass reads (for MethodDescChunk, NumMethods, etc.) at subsequent offsets
// would fail with VirtualReadException. After the TypeValidation fix, the full
// EEClass is eagerly read during validation, so this now correctly reports
// E_INVALIDARG instead of surfacing VirtualReadException from those later reads.
int pointerSize = helpers.PointerSize;
MockMemorySpace.HeapFragment tinyEEClass = rtsBuilder.TypeSystemAllocator.Allocate(
(ulong)pointerSize, "Tiny EEClass (MethodTable field only)");
helpers.WritePointer(tinyEEClass.Data, methodTablePtr);
rtsBuilder.Builder.AddHeapFragment(tinyEEClass);

// Point the MethodTable's EEClassOrCanonMT at the tiny EEClass
rtsBuilder.SetMethodTableEEClassOrCanonMTRaw(methodTablePtr, tinyEEClass.Address);
},
(target) =>
{
ISOSDacInterface sosDac = new SOSDacImpl(target, legacyObj: null);

DacpMethodTableData mtData = default;
int hr = sosDac.GetMethodTableData(new ClrDataAddress(methodTablePtr), &mtData);

// Should return E_INVALIDARG to match legacy DAC behavior
AssertHResult(HResults.E_INVALIDARG, hr);
});
}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public unsafe void GetMethodTableDataReturnsEInvalidArgWhenMethodTablePartiallyReadable(MockTarget.Architecture arch)
{
// Analogous to the EEClass test above but for the MethodTable itself.
// If a MethodTable is only partially readable (enough for validation fields
// like MTFlags, BaseSize, MTFlags2, EEClassOrCanonMT) but not the full
// structure (Module, ParentMethodTable, etc.), validation currently passes
// because NonValidatedMethodTable reads fields lazily. The subsequent
// Data.MethodTable construction then fails with VirtualReadException,
// surfacing CORDBG_E_READVIRTUAL_FAILURE instead of E_INVALIDARG.
TargetPointer tinyMethodTableAddr = default;

RTSContractHelper(arch,
(rtsBuilder) =>
{
TargetTestHelpers helpers = rtsBuilder.Builder.TargetTestHelpers;
Target.TypeInfo mtTypeInfo = rtsBuilder.Types[DataType.MethodTable];

// Create a valid EEClass that will point back to our tiny MethodTable
TargetPointer eeClassPtr = rtsBuilder.AddEEClass("PartialMT EEClass",
attr: 0, numMethods: 0, numNonVirtualSlots: 0);

// Allocate a tiny MethodTable fragment — only enough for the fields that
// validation reads (MTFlags, BaseSize, MTFlags2, EEClassOrCanonMT) but
// not the full MethodTable. Fields like Module, ParentMethodTable, etc.
// at subsequent offsets will be unreadable.
int eeClassOrCanonMTOffset = mtTypeInfo.Fields[nameof(Data.MethodTable.EEClassOrCanonMT)].Offset;
ulong partialSize = (ulong)(eeClassOrCanonMTOffset + helpers.PointerSize);
MockMemorySpace.HeapFragment tinyMT = rtsBuilder.TypeSystemAllocator.Allocate(
partialSize, "Tiny MethodTable (validation fields only)");

Span<byte> dest = tinyMT.Data;
helpers.Write(dest.Slice(mtTypeInfo.Fields[nameof(Data.MethodTable.MTFlags)].Offset), (uint)0);
helpers.Write(dest.Slice(mtTypeInfo.Fields[nameof(Data.MethodTable.BaseSize)].Offset), (uint)helpers.ObjectBaseSize);
helpers.Write(dest.Slice(mtTypeInfo.Fields[nameof(Data.MethodTable.MTFlags2)].Offset), (uint)0);
helpers.WritePointer(dest.Slice(eeClassOrCanonMTOffset), eeClassPtr);

rtsBuilder.Builder.AddHeapFragment(tinyMT);
tinyMethodTableAddr = tinyMT.Address;

// Point the EEClass back at the tiny MethodTable to pass validation
Target.TypeInfo eeClassTypeInfo = rtsBuilder.Types[DataType.EEClass];
Span<byte> eeClassBytes = rtsBuilder.Builder.BorrowAddressRange(
eeClassPtr, (int)eeClassTypeInfo.Size.Value);
helpers.WritePointer(
eeClassBytes.Slice(eeClassTypeInfo.Fields[nameof(Data.EEClass.MethodTable)].Offset,
helpers.PointerSize), tinyMethodTableAddr);
},
(target) =>
{
ISOSDacInterface sosDac = new SOSDacImpl(target, legacyObj: null);

DacpMethodTableData mtData = default;
int hr = sosDac.GetMethodTableData(new ClrDataAddress(tinyMethodTableAddr), &mtData);

// Should return E_INVALIDARG to match legacy DAC behavior
AssertHResult(HResults.E_INVALIDARG, hr);
});
}
}
31 changes: 31 additions & 0 deletions src/native/managed/cdac/tests/TestHelpers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Reflection;
using Xunit;

namespace Microsoft.Diagnostics.DataContractReader.Tests;

internal static class TestHelpers
{
internal static string FormatHResult(int hr)
{
string hex = $"0x{unchecked((uint)hr):X8}";
foreach (Type type in new[] { typeof(HResults), typeof(CorDbgHResults) })
{
foreach (FieldInfo field in type.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static))
{
if (field.FieldType == typeof(int) && (int)field.GetValue(null)! == hr)
return $"{field.Name} ({hex})";
}
}
return hex;
}

internal static void AssertHResult(int expected, int actual)
{
Assert.True(expected == actual,
$"Expected: {FormatHResult(expected)}, Actual: {FormatHResult(actual)}");
}
}
Loading