Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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 @@ -116,7 +116,59 @@ int IXCLRDataExceptionState.GetPrevious(DacComNullableByRef<IXCLRDataExceptionSt
}

int IXCLRDataExceptionState.GetManagedObject(DacComNullableByRef<IXCLRDataValue> value)
=> LegacyFallbackHelper.CanFallback() && _legacyImpl is not null ? _legacyImpl.GetManagedObject(value) : HResults.E_NOTIMPL;
{
int hr = HResults.S_OK, hrLocal = HResults.S_OK;
IXCLRDataValue? legacyValue = null;

if (_legacyImpl is not null)

@noahfalk noahfalk Jul 24, 2026

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.

We shouldn't fallback to the legacy implementation without first testing LegacyFallbackHelper.CanFallback(). Is it useful to have a partial implementation of this method that still relies on the fallback? It seems a little clearer if we left the previous fallback behavior in place until we had an implementation that didn't rely on any fallback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like we didn’t update the debug validation to test CanFallback() anywhere, I will do that here and then put up a PR to do that everywhere

{
DacComNullableByRef<IXCLRDataValue> legacyValueOut = new(value.IsNullRef);
hrLocal = _legacyImpl.GetManagedObject(legacyValueOut);
legacyValue = legacyValueOut.Interface;
}

try
{
if (_thrownObjectHandle == TargetPointer.Null)
throw new ArgumentException();

TargetPointer exceptionObject;
try
{
exceptionObject = _target.ReadPointer(_thrownObjectHandle);
}
catch (VirtualReadException)
{
throw new ArgumentException();
}

ulong objectSize = _target.Contracts.Object.GetSize(exceptionObject);
Comment on lines +132 to +145
value.Interface = new ClrDataValue(
_target,
(uint)ClrDataValueFlag.DEFAULT,
[
new NativeVarLocation
{
AddressOrValue = exceptionObject.ToClrDataAddress(_target),
Size = objectSize,
IsRegisterValue = false,
},
],
legacyValue);
}
catch (System.Exception ex)
{
hr = ex.HResult;
}
#if DEBUG
if (_legacyImpl is not null)
{
Debug.ValidateHResult(hr, hrLocal);
}
#endif

return hr;
}

int IXCLRDataExceptionState.GetBaseType(/*CLRDataBaseExceptionType*/ uint* type) => HResults.E_NOTIMPL;

Expand Down
113 changes: 110 additions & 3 deletions src/native/managed/cdac/tests/UnitTests/ClrDataExceptionStateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,25 @@ namespace Microsoft.Diagnostics.DataContractReader.Tests;

public unsafe class ExceptionStateTests
{
private const ulong ExceptionObjectSize = 0x40;
private static readonly TargetPointer s_exceptionObjectAddress = new(0xAA00_0000);

private static (TestPlaceholderTarget Target, TargetPointer ThrownObjectHandle) CreateTargetWithException(
MockTarget.Architecture arch,
TargetPointer messageAddr,
string? messageString)
{
TargetPointer exceptionObjectAddr = new TargetPointer(0x5000);
TargetTestHelpers helpers = new(arch);
var targetBuilder = new TestPlaceholderTarget.Builder(arch);
var allocator = targetBuilder.MemoryBuilder.CreateAllocator(0x1_0000, 0x2_0000);

MockMemorySpace.HeapFragment handleFragment = allocator.Allocate((ulong)helpers.PointerSize, "ThrownObjectHandle");
helpers.WritePointer(handleFragment.Data, exceptionObjectAddr);
helpers.WritePointer(handleFragment.Data, s_exceptionObjectAddress);

TargetPointer thrownObjectHandle = new TargetPointer(handleFragment.Address);

var mockException = new Mock<IException>();
mockException.Setup(e => e.GetExceptionData(exceptionObjectAddr)).Returns(new ExceptionData(
mockException.Setup(e => e.GetExceptionData(s_exceptionObjectAddress)).Returns(new ExceptionData(
Message: messageAddr,
InnerException: TargetPointer.Null,
StackTrace: TargetPointer.Null,
Expand All @@ -40,6 +42,7 @@ private static (TestPlaceholderTarget Target, TargetPointer ThrownObjectHandle)
XCode: 0));

var mockObject = new Mock<IObject>();
mockObject.Setup(o => o.GetSize(s_exceptionObjectAddress)).Returns(ExceptionObjectSize);
if (messageAddr != TargetPointer.Null && messageString is not null)
mockObject.Setup(o => o.GetStringValue(messageAddr)).Returns(messageString);

Expand Down Expand Up @@ -228,6 +231,110 @@ public void GetFlags(uint inputFlags, bool hasNestedException, uint expectedFlag
AssertFlags(exceptionState, expectedFlags);
}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public void GetManagedObject(MockTarget.Architecture arch)
{
(TestPlaceholderTarget target, TargetPointer thrownObjectHandle) =
CreateTargetWithException(arch, TargetPointer.Null, null);
IXCLRDataExceptionState exceptionState = new ClrDataExceptionState(
target,
new TargetPointer(0x1000),
(uint)CLRDataExceptionStateFlag.CLRDATA_EXCEPTION_DEFAULT,
TargetPointer.Null,
thrownObjectHandle,
TargetPointer.Null,
null);
DacComNullableByRef<IXCLRDataValue> value = new(isNullRef: false);

int hr = exceptionState.GetManagedObject(value);

Assert.Equal(HResults.S_OK, hr);
Assert.NotNull(value.Interface);

uint flags;
Assert.Equal(HResults.S_OK, value.Interface.GetFlags(&flags));
Assert.Equal((uint)ClrDataValueFlag.DEFAULT, flags);

ClrDataAddress address;
Assert.Equal(HResults.S_OK, value.Interface.GetAddress(&address));
Assert.Equal(s_exceptionObjectAddress.ToClrDataAddress(target), address);

ulong size;
Assert.Equal(HResults.S_OK, value.Interface.GetSize(&size));
Assert.Equal(ExceptionObjectSize, size);

uint locationFlags;
ClrDataAddress location;
Assert.Equal(HResults.S_OK, value.Interface.GetLocationByIndex(0, &locationFlags, &location));
Assert.Equal(ClrDataVLocFlag.CLRDATA_VLOC_MEMORY, locationFlags);
Assert.Equal(s_exceptionObjectAddress.ToClrDataAddress(target), location);
Comment on lines +267 to +271
}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public void GetManagedObject_InvalidHandle(MockTarget.Architecture arch)
{
TestPlaceholderTarget target = new TestPlaceholderTarget.Builder(arch)
.UseReader((ulong _, Span<byte> _) => -1)
.Build();
IXCLRDataExceptionState exceptionState = new ClrDataExceptionState(
target,
new TargetPointer(0x1000),
(uint)CLRDataExceptionStateFlag.CLRDATA_EXCEPTION_DEFAULT,
TargetPointer.Null,
new TargetPointer(0x2000),
TargetPointer.Null,
null);
DacComNullableByRef<IXCLRDataValue> value = new(isNullRef: false);

int hr = exceptionState.GetManagedObject(value);

Assert.Equal(HResults.E_INVALIDARG, hr);
Assert.Null(value.Interface);
}

[Fact]
public void GetManagedObject_NullHandle()
{
IXCLRDataExceptionState exceptionState = new ClrDataExceptionState(
null!,
new TargetPointer(0x1000),
(uint)CLRDataExceptionStateFlag.CLRDATA_EXCEPTION_DEFAULT,
TargetPointer.Null,
TargetPointer.Null,
TargetPointer.Null,
null);
DacComNullableByRef<IXCLRDataValue> value = new(isNullRef: false);

int hr = exceptionState.GetManagedObject(value);

Assert.Equal(HResults.E_INVALIDARG, hr);
Assert.Null(value.Interface);
}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public void GetManagedObject_NullOutput(MockTarget.Architecture arch)
{
(TestPlaceholderTarget target, TargetPointer thrownObjectHandle) =
CreateTargetWithException(arch, TargetPointer.Null, null);
IXCLRDataExceptionState exceptionState = new ClrDataExceptionState(
target,
new TargetPointer(0x1000),
(uint)CLRDataExceptionStateFlag.CLRDATA_EXCEPTION_DEFAULT,
TargetPointer.Null,
thrownObjectHandle,
TargetPointer.Null,
null);
DacComNullableByRef<IXCLRDataValue> value = new(isNullRef: true);

int hr = exceptionState.GetManagedObject(value);

Assert.Equal(HResults.E_POINTER, hr);
Assert.Null(value.Interface);
}

[Theory]
[ClassData(typeof(MockTarget.StdArch))]
public void GetString_WithMessage(MockTarget.Architecture arch)
Expand Down
Loading