Skip to content
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

Jump to bottom

[cdac] Implement ISOSDacInterface2::GetObjectExceptionData #104343

Merged
merged 8 commits into from
Jul 9, 2024
Merged

[cdac] Implement ISOSDacInterface2::GetObjectExceptionData #104343

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a8c8caf
Make cDAC implement ISOSDacInterface2
elinor-fung Jul 2, 2024
ca65ff1
Make DAC call cDAC for GetObjectExceptionData
elinor-fung Jul 2, 2024
26c7632
Add GetExceptionObjectData to Exception contract, implement ISOSDacIn…
elinor-fung Jul 2, 2024
1e55328
Update Exception contract doc
elinor-fung Jul 2, 2024
4514056
Use exast field names
elinor-fung Jul 8, 2024
86875ba
ExceptionObject -> Exception
elinor-fung Jul 8, 2024
e11102d
Merge remote-tracking branch 'upstream/main' into cdac-exception-object
elinor-fung Jul 8, 2024
1952438
Remove Managed prefix
elinor-fung Jul 9, 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
34 changes: 31 additions & 3 deletions docs/design/datacontracts/Exception.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,56 @@
# Contract Thread
# Contract Exception

This contract is for getting information about exceptions in the process.

## APIs of contract

```csharp
record struct ManagedExceptionData(
TargetPointer Message,
Copy link
Member

Choose a reason for hiding this comment

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

Should we call this just ExceptionData?

I think it would be best to avoid Managed prefix in the contract definitions.

Copy link
Member

Choose a reason for hiding this comment

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

If there is something explicitly unmanaged (like EXCEPTION_INFORMATION - that is unmanaged equivalent of managed exception), I would use a special prefix for that if necessary.

TargetPointer InnerException,
TargetPointer StackTrace,
TargetPointer WatsonBuckets,
TargetPointer StackTraceString,
TargetPointer RemoteStackTraceString,
int HResult,
int XCode);
```

``` csharp
TargetPointer GetExceptionInfo(TargetPointer exception, out TargetPointer nextNestedException);
ManagedExceptionData GetManagedExceptionData(TargetPointer managedException)
```

## Version 1

Data descriptors used:
- `ExceptionInfo`
- `ExceptionObject`

Copy link
Member

Choose a reason for hiding this comment

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

Should we have a consistent naming scheme for data descriptors of managed types? The names of the unmanaged mirrors of managed types are not very consistent.

Here are some examples of managed types that may need data descriptor at some point.

System.Exception
System.Runtime.Loader.AssemblyLoadContext
System.Threading.Thread

One possible naming scheme is to replace . with _. It would produce:
System_Exception
System_Runtime_Loader_AssemblyLoadContext
System_Threading_Thread

Copy link
Member

Choose a reason for hiding this comment

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

Or omit . in the name:
SystemException
SystemRuntimeLoaderAssemblyLoadContext
SystemThreadingThread

Or abbreviate the name space:
S_Exception
SRL_AssemblyLoadContext
ST_Thread

Or drop the namespace completely (I think I like this the most):
Exception
AssemblyLoadContext
Thread

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I like the non-abbreviated . -> _, but I could see those getting a bit long - maybe abbreviated is a reasonable middle. For dropping the namespace completely, we'd have to differentiate between unmanaged and managed types that have the same name (like Thread) and I think I'd prefer the managed ones having the indication of being managed.

Copy link
Member

Choose a reason for hiding this comment

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

we'd have to differentiate between unmanaged and managed types that have the same name (like Thread)

The name collisions have been a problem a few times as we are rewriting more of the native runtime in C#. If the name collisions are the only problem, I would say rename the internal types to avoid them. The ultimate picture can be that nearly all types have managed views, and subset of types have unmanaged mirrors.

The name collisions between managed and unmanaged code are also a source of confusion. Trivia question - when you see ThreadOBJ in SOS, does it mean managed Thread or unmanaged Thread?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `ExceptionObject`
- `Exception`

?

``` csharp
TargetPointer GetExceptionInfo(TargetPointer exception, out TargetPointer nextNestedException)
{
if (exception == TargetPointer.Null)
throw new InvalidArgumentException();

nextNestedException = target.ReadPointer(address + /* ExceptionInfo::PreviousNestedInfo offset*/);
TargetPointer thrownObjHandle = target.ReadPointer(address + /* ExceptionInfo::ThrownObject offset */);
nextNestedException = target.ReadPointer(exception + /* ExceptionInfo::PreviousNestedInfo offset*/);
TargetPointer thrownObjHandle = target.ReadPointer(exception + /* ExceptionInfo::ThrownObject offset */);
return = thrownObjHandle != TargetPointer.Null
? target.ReadPointer(thrownObjHandle)
: TargetPointer.Null;
}

ManagedExceptionData GetManagedExceptionData(TargetPointer managedException)
{
return new ManagedExceptionData(
target.ReadPointer(objectAddress + /* Exception::Message offset */),
target.ReadPointer(objectAddress + /* Exception::InnerException offset */),
target.ReadPointer(objectAddress + /* Exception::StackTrace offset */),
target.ReadPointer(objectAddress + /* Exception::WatsonBuckets offset */),
target.ReadPointer(objectAddress + /* Exception::StackTraceString offset */),
target.ReadPointer(objectAddress + /* Exception::RemoteStackTraceString offset */),
target.Read<int>(objectAddress + /* Exception::HResult offset */),
target.Read<int>(objectAddress + /* Exception::XCode offset */),
);
}
```
1 change: 1 addition & 0 deletions src/coreclr/debug/daccess/daccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5512,6 +5512,7 @@ ClrDataAccess::Initialize(void)
// Get SOS interfaces from the cDAC if available.
IUnknown* unk = m_cdac.SosInterface();
(void)unk->QueryInterface(__uuidof(ISOSDacInterface), (void**)&m_cdacSos);
(void)unk->QueryInterface(__uuidof(ISOSDacInterface2), (void**)&m_cdacSos2);
(void)unk->QueryInterface(__uuidof(ISOSDacInterface9), (void**)&m_cdacSos9);
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/debug/daccess/dacimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1234,6 +1234,7 @@ class ClrDataAccess
HRESULT GetNestedExceptionDataImpl(CLRDATA_ADDRESS exception, CLRDATA_ADDRESS *exceptionObject, CLRDATA_ADDRESS *nextNestedException);
HRESULT GetMethodTableDataImpl(CLRDATA_ADDRESS mt, struct DacpMethodTableData *data);
HRESULT GetMethodTableForEEClassImpl (CLRDATA_ADDRESS eeClassReallyMT, CLRDATA_ADDRESS *value);
HRESULT GetObjectExceptionDataImpl(CLRDATA_ADDRESS objAddr, struct DacpExceptionObjectData *data);

BOOL IsExceptionFromManagedCode(EXCEPTION_RECORD * pExceptionRecord);
#ifndef TARGET_UNIX
Expand Down Expand Up @@ -1423,6 +1424,7 @@ class ClrDataAccess

CDAC m_cdac;
NonVMComHolder<ISOSDacInterface> m_cdacSos;
NonVMComHolder<ISOSDacInterface2> m_cdacSos2;
NonVMComHolder<ISOSDacInterface9> m_cdacSos9;

#ifdef FEATURE_MINIMETADATA_IN_TRIAGEDUMPS
Expand Down
41 changes: 36 additions & 5 deletions src/coreclr/debug/daccess/request.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4600,8 +4600,42 @@ HRESULT ClrDataAccess::GetObjectExceptionData(CLRDATA_ADDRESS objAddr, struct Da

SOSDacEnter();

PTR_ExceptionObject pObj = dac_cast<PTR_ExceptionObject>(TO_TADDR(objAddr));
if (m_cdacSos2 != NULL)
{
hr = m_cdacSos2->GetObjectExceptionData(objAddr, data);
if (FAILED(hr))
{
hr = GetObjectExceptionDataImpl(objAddr, data);
}
#ifdef _DEBUG
else
{
DacpExceptionObjectData dataLocal;
HRESULT hrLocal = GetObjectExceptionDataImpl(objAddr, &dataLocal);
_ASSERTE(hr == hrLocal);
_ASSERTE(data->Message == dataLocal.Message);
_ASSERTE(data->InnerException == dataLocal.InnerException);
_ASSERTE(data->StackTrace == dataLocal.StackTrace);
_ASSERTE(data->WatsonBuckets == dataLocal.WatsonBuckets);
_ASSERTE(data->StackTraceString == dataLocal.StackTraceString);
_ASSERTE(data->RemoteStackTraceString == dataLocal.RemoteStackTraceString);
_ASSERTE(data->HResult == dataLocal.HResult);
_ASSERTE(data->XCode == dataLocal.XCode);
}
#endif
}
else
{
hr = GetObjectExceptionDataImpl(objAddr, data);
}

SOSDacLeave();
return hr;
}

HRESULT ClrDataAccess::GetObjectExceptionDataImpl(CLRDATA_ADDRESS objAddr, struct DacpExceptionObjectData *data)
{
PTR_ExceptionObject pObj = dac_cast<PTR_ExceptionObject>(TO_TADDR(objAddr));
data->Message = TO_CDADDR(dac_cast<TADDR>(pObj->GetMessage()));
data->InnerException = TO_CDADDR(dac_cast<TADDR>(pObj->GetInnerException()));
data->StackTrace = TO_CDADDR(dac_cast<TADDR>(pObj->GetStackTraceArrayObject()));
Expand All @@ -4610,10 +4644,7 @@ HRESULT ClrDataAccess::GetObjectExceptionData(CLRDATA_ADDRESS objAddr, struct Da
data->RemoteStackTraceString = TO_CDADDR(dac_cast<TADDR>(pObj->GetRemoteStackTraceString()));
data->HResult = pObj->GetHResult();
data->XCode = pObj->GetXCode();

SOSDacLeave();

return hr;
return S_OK;
}

HRESULT ClrDataAccess::IsRCWDCOMProxy(CLRDATA_ADDRESS rcwAddr, BOOL* isDCOMProxy)
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/debug/runtimeinfo/datadescriptor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ CDAC_TYPE_FIELD(GCAllocContext, /*pointer*/, Pointer, offsetof(gc_alloc_context,
CDAC_TYPE_FIELD(GCAllocContext, /*pointer*/, Limit, offsetof(gc_alloc_context, alloc_limit))
CDAC_TYPE_END(GCAllocContext)

// Exception

// Use exact managed type field names for the descriptor as field names often can't change due to binary serialization or implicit diagnostic contracts
CDAC_TYPE_BEGIN(Exception)
CDAC_TYPE_INDETERMINATE(Exception)
CDAC_TYPE_FIELD(Exception, /*pointer*/, _message, cdac_offsets<ExceptionObject>::_message)
CDAC_TYPE_FIELD(Exception, /*pointer*/, _innerException, cdac_offsets<ExceptionObject>::_innerException)
CDAC_TYPE_FIELD(Exception, /*pointer*/, _stackTrace, cdac_offsets<ExceptionObject>::_stackTrace)
CDAC_TYPE_FIELD(Exception, /*pointer*/, _watsonBuckets, cdac_offsets<ExceptionObject>::_watsonBuckets)
CDAC_TYPE_FIELD(Exception, /*pointer*/, _stackTraceString, cdac_offsets<ExceptionObject>::_stackTraceString)
CDAC_TYPE_FIELD(Exception, /*pointer*/, _remoteStackTraceString, cdac_offsets<ExceptionObject>::_remoteStackTraceString)
CDAC_TYPE_FIELD(Exception, /*int32*/, _HResult, cdac_offsets<ExceptionObject>::_HResult)
CDAC_TYPE_FIELD(Exception, /*int32*/, _xcode, cdac_offsets<ExceptionObject>::_xcode)
CDAC_TYPE_END(Exception)

CDAC_TYPE_BEGIN(ExceptionInfo)
CDAC_TYPE_INDETERMINATE(ExceptionInfo)
#if FEATURE_EH_FUNCLETS
Expand All @@ -152,6 +167,7 @@ CDAC_TYPE_FIELD(PreviousNestedInfo, /*pointer*/, PreviousNestedInfo, offsetof(Ex
#endif
CDAC_TYPE_END(ExceptionInfo)


CDAC_TYPE_BEGIN(GCHandle)
CDAC_TYPE_SIZE(sizeof(OBJECTHANDLE))
CDAC_TYPE_END(GCHandle)
Expand Down
17 changes: 16 additions & 1 deletion src/coreclr/vm/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2096,7 +2096,7 @@ class LoaderAllocatorObject : public Object
INT32 GetSlotsUsed();
void SetSlotsUsed(INT32 newSlotsUsed);
#endif // DACCESS_COMPILE

void SetNativeLoaderAllocator(LoaderAllocator * pLoaderAllocator)
{
LIMITED_METHOD_CONTRACT;
Expand Down Expand Up @@ -2340,6 +2340,21 @@ class ExceptionObject : public Object
void* _xptrs;
INT32 _xcode;
INT32 _HResult;

template<typename T> friend struct ::cdac_offsets;
};

template<>
struct cdac_offsets<ExceptionObject>
{
static constexpr size_t _message = offsetof(ExceptionObject, _message);
static constexpr size_t _innerException = offsetof(ExceptionObject, _innerException);
static constexpr size_t _stackTrace = offsetof(ExceptionObject, _stackTrace);
static constexpr size_t _watsonBuckets = offsetof(ExceptionObject, _watsonBuckets);
static constexpr size_t _stackTraceString = offsetof(ExceptionObject, _stackTraceString);
static constexpr size_t _remoteStackTraceString = offsetof(ExceptionObject, _remoteStackTraceString);
static constexpr size_t _HResult = offsetof(ExceptionObject, _HResult);
static constexpr size_t _xcode = offsetof(ExceptionObject, _xcode);
};

// Defined in Contracts.cs
Expand Down
11 changes: 11 additions & 0 deletions src/native/managed/cdacreader/src/Contracts/Exception.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@

namespace Microsoft.Diagnostics.DataContractReader.Contracts;

internal record struct ManagedExceptionData(
TargetPointer Message,
TargetPointer InnerException,
TargetPointer StackTrace,
TargetPointer WatsonBuckets,
TargetPointer StackTraceString,
TargetPointer RemoteStackTraceString,
int HResult,
int XCode);

internal interface IException : IContract
{
static string IContract.Name { get; } = nameof(Exception);
Expand All @@ -18,6 +28,7 @@ static IContract IContract.Create(Target target, int version)
}

public virtual TargetPointer GetExceptionInfo(TargetPointer exception, out TargetPointer nextNestedException) => throw new NotImplementedException();
public virtual ManagedExceptionData GetManagedExceptionData(TargetPointer managedException) => throw new NotImplementedException();
}

internal readonly struct Exception : IException
Expand Down
14 changes: 14 additions & 0 deletions src/native/managed/cdacreader/src/Contracts/Exception_1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,18 @@ TargetPointer IException.GetExceptionInfo(TargetPointer exception, out TargetPoi
nextNestedException = exceptionInfo.PreviousNestedInfo;
return exceptionInfo.ThrownObject.Object;
}

ManagedExceptionData IException.GetManagedExceptionData(TargetPointer managedException)
{
Data.Exception exceptionObject = _target.ProcessedData.GetOrAdd<Data.Exception>(managedException);
return new ManagedExceptionData(
exceptionObject.Message,
exceptionObject.InnerException,
exceptionObject.StackTrace,
exceptionObject.WatsonBuckets,
exceptionObject.StackTraceString,
exceptionObject.RemoteStackTraceString,
exceptionObject.HResult,
exceptionObject.XCode);
}
}
33 changes: 33 additions & 0 deletions src/native/managed/cdacreader/src/Data/Exception.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.Diagnostics.DataContractReader.Data;

internal sealed class Exception : IData<Exception>
{
static Exception IData<Exception>.Create(Target target, TargetPointer address)
=> new Exception(target, address);

public Exception(Target target, TargetPointer address)
{
Target.TypeInfo type = target.GetTypeInfo(DataType.Exception);

Message = target.ReadPointer(address + (ulong)type.Fields["_message"].Offset);
InnerException = target.ReadPointer(address + (ulong)type.Fields["_innerException"].Offset);
StackTrace = target.ReadPointer(address + (ulong)type.Fields["_stackTrace"].Offset);
WatsonBuckets = target.ReadPointer(address + (ulong)type.Fields["_watsonBuckets"].Offset);
StackTraceString = target.ReadPointer(address + (ulong)type.Fields["_stackTraceString"].Offset);
RemoteStackTraceString = target.ReadPointer(address + (ulong)type.Fields["_remoteStackTraceString"].Offset);
HResult = target.Read<int>(address + (ulong)type.Fields["_HResult"].Offset);
XCode = target.Read<int>(address + (ulong)type.Fields["_xcode"].Offset);
}

public TargetPointer Message { get; init; }
public TargetPointer InnerException { get; init; }
public TargetPointer StackTrace { get; init; }
public TargetPointer WatsonBuckets { get; init; }
public TargetPointer StackTraceString { get; init; }
public TargetPointer RemoteStackTraceString { get; init; }
public int HResult { get; init; }
public int XCode { get; init; }
}
1 change: 1 addition & 0 deletions src/native/managed/cdacreader/src/DataType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public enum DataType
Thread,
ThreadStore,
GCAllocContext,
Exception,
ExceptionInfo,
RuntimeThreadLocals,

Expand Down
24 changes: 24 additions & 0 deletions src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,30 @@ internal unsafe partial interface ISOSDacInterface
int GetFailedAssemblyDisplayName(ulong assembly, uint count, char* name, uint* pNeeded);
};

#pragma warning disable CS0649 // Field is never assigned to, and will always have its default value
internal struct DacpExceptionObjectData
{
public ulong Message;
public ulong InnerException;
public ulong StackTrace;
public ulong WatsonBuckets;
public ulong StackTraceString;
public ulong RemoteStackTraceString;
public int HResult;
public int XCode;
}
#pragma warning restore CS0649 // Field is never assigned to, and will always have its default value

[GeneratedComInterface]
[Guid("A16026EC-96F4-40BA-87FB-5575986FB7AF")]
internal unsafe partial interface ISOSDacInterface2
{
[PreserveSig]
int GetObjectExceptionData(ulong objectAddress, DacpExceptionObjectData* data);
[PreserveSig]
int IsRCWDCOMProxy(ulong rcwAddress, int* inDCOMProxy);
}

[GeneratedComInterface]
[Guid("4eca42d8-7e7b-4c8a-a116-7bfbf6929267")]
internal partial interface ISOSDacInterface9
Expand Down
27 changes: 26 additions & 1 deletion src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.Diagnostics.DataContractReader.Legacy;
/// corresponding error code.
/// </remarks>
[GeneratedComClass]
internal sealed partial class SOSDacImpl : ISOSDacInterface, ISOSDacInterface9
internal sealed partial class SOSDacImpl : ISOSDacInterface, ISOSDacInterface2, ISOSDacInterface9
{
private readonly Target _target;

Expand Down Expand Up @@ -172,6 +172,30 @@ public unsafe int GetNestedExceptionData(ulong exception, ulong* exceptionObject

public unsafe int GetObjectClassName(ulong obj, uint count, char* className, uint* pNeeded) => HResults.E_NOTIMPL;
public unsafe int GetObjectData(ulong objAddr, void* data) => HResults.E_NOTIMPL;

public unsafe int GetObjectExceptionData(ulong objectAddress, DacpExceptionObjectData* data)
{
try
{
Contracts.IException contract = _target.Contracts.Exception;
Contracts.ManagedExceptionData exceptionData = contract.GetManagedExceptionData(objectAddress);
data->Message = exceptionData.Message;
data->InnerException = exceptionData.InnerException;
data->StackTrace = exceptionData.StackTrace;
data->WatsonBuckets = exceptionData.WatsonBuckets;
data->StackTraceString = exceptionData.StackTraceString;
data->RemoteStackTraceString = exceptionData.RemoteStackTraceString;
data->HResult = exceptionData.HResult;
data->XCode = exceptionData.XCode;
}
catch (Exception ex)
{
return ex.HResult;
}

return HResults.S_OK;
}

public unsafe int GetObjectStringData(ulong obj, uint count, char* stringData, uint* pNeeded) => HResults.E_NOTIMPL;
public unsafe int GetOOMData(ulong oomAddr, void* data) => HResults.E_NOTIMPL;
public unsafe int GetOOMStaticData(void* data) => HResults.E_NOTIMPL;
Expand Down Expand Up @@ -255,6 +279,7 @@ public unsafe int GetThreadStoreData(DacpThreadStoreData* data)
public unsafe int GetTLSIndex(uint* pIndex) => HResults.E_NOTIMPL;
public unsafe int GetUsefulGlobals(void* data) => HResults.E_NOTIMPL;
public unsafe int GetWorkRequestData(ulong addrWorkRequest, void* data) => HResults.E_NOTIMPL;
public unsafe int IsRCWDCOMProxy(ulong rcwAddress, int* inDCOMProxy) => HResults.E_NOTIMPL;
public unsafe int TraverseEHInfo(ulong ip, void* pCallback, void* token) => HResults.E_NOTIMPL;
public unsafe int TraverseLoaderHeap(ulong loaderHeapAddr, void* pCallback) => HResults.E_NOTIMPL;
public unsafe int TraverseModuleMap(int mmt, ulong moduleAddr, void* pCallback, void* token) => HResults.E_NOTIMPL;
Expand Down
Loading