From a72cfb0ee2669abab031c5095a670678fd0b7861 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 30 Oct 2024 13:22:44 -0700 Subject: [PATCH] [cdac] Implement `IXCLRDataTask::CreateStackWalk` (#109350) - Add `ClrDataStackWalk` to cDAC as its implementation of `IXCLRDataStackWalk` - Currently delegates everything to legacy implementation - Implement `IXCLRDataTask::CreateStackWalk` - Checks that the thread has been started - Gets the result from the legacy DAC and passes it to the cDAC `ClrDataStackWalk` - Switch `ISOSDacInterface.GetModule` to use `out IXCLRDataModule?` instead of `void**` --- .../cdacreader/src/Legacy/ClrDataModule.cs | 14 +++-- .../cdacreader/src/Legacy/ClrDataStackWalk.cs | 42 +++++++++++++++ .../cdacreader/src/Legacy/ClrDataTask.cs | 53 +++++++++++++------ .../cdacreader/src/Legacy/ISOSDacInterface.cs | 2 +- .../cdacreader/src/Legacy/IXCLRData.cs | 33 +++++++++++- .../cdacreader/src/Legacy/SOSDacImpl.cs | 29 +++------- 6 files changed, 127 insertions(+), 46 deletions(-) create mode 100644 src/native/managed/cdacreader/src/Legacy/ClrDataStackWalk.cs diff --git a/src/native/managed/cdacreader/src/Legacy/ClrDataModule.cs b/src/native/managed/cdacreader/src/Legacy/ClrDataModule.cs index 05178f474ce422..21d8daa9a6fdd7 100644 --- a/src/native/managed/cdacreader/src/Legacy/ClrDataModule.cs +++ b/src/native/managed/cdacreader/src/Legacy/ClrDataModule.cs @@ -13,17 +13,23 @@ internal sealed unsafe partial class ClrDataModule : ICustomQueryInterface, IXCL { private readonly TargetPointer _address; private readonly Target _target; - private readonly nint _legacyModulePointer; private readonly IXCLRDataModule? _legacyModule; private readonly IXCLRDataModule2? _legacyModule2; - public ClrDataModule(TargetPointer address, Target target, nint legacyModulePointer, object? legacyImpl) + // This is an IUnknown pointer for the legacy implementation + private readonly nint _legacyModulePointer; + + public ClrDataModule(TargetPointer address, Target target, IXCLRDataModule? legacyImpl) { _address = address; _target = target; - _legacyModulePointer = legacyModulePointer; - _legacyModule = legacyImpl as IXCLRDataModule; + _legacyModule = legacyImpl; _legacyModule2 = legacyImpl as IXCLRDataModule2; + if (legacyImpl is not null && ComWrappers.TryGetComInstance(legacyImpl, out _legacyModulePointer)) + { + // Release the AddRef from TryGetComInstance. We rely on the ref-count from holding on to IXCLRDataModule. + Marshal.Release(_legacyModulePointer); + } } private static readonly Guid IID_IMetaDataImport = Guid.Parse("7DAC8207-D3AE-4c75-9B67-92801A497D44"); diff --git a/src/native/managed/cdacreader/src/Legacy/ClrDataStackWalk.cs b/src/native/managed/cdacreader/src/Legacy/ClrDataStackWalk.cs new file mode 100644 index 00000000000000..0e543c87fccdff --- /dev/null +++ b/src/native/managed/cdacreader/src/Legacy/ClrDataStackWalk.cs @@ -0,0 +1,42 @@ +// 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.Runtime.InteropServices; +using System.Runtime.InteropServices.Marshalling; + +namespace Microsoft.Diagnostics.DataContractReader.Legacy; + +[GeneratedComClass] +internal sealed unsafe partial class ClrDataStackWalk : IXCLRDataStackWalk +{ + private readonly TargetPointer _threadAddr; + private readonly uint _flags; + private readonly Target _target; + private readonly IXCLRDataStackWalk? _legacyImpl; + + public ClrDataStackWalk(TargetPointer threadAddr, uint flags, Target target, IXCLRDataStackWalk? legacyImpl) + { + _threadAddr = threadAddr; + _flags = flags; + _target = target; + _legacyImpl = legacyImpl; + } + + int IXCLRDataStackWalk.GetContext(uint contextFlags, uint contextBufSize, uint* contextSize, [MarshalUsing(CountElementName = "contextBufSize"), Out] byte[] contextBuf) + => _legacyImpl is not null ? _legacyImpl.GetContext(contextFlags, contextBufSize, contextSize, contextBuf) : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.GetFrame(void** frame) + => _legacyImpl is not null ? _legacyImpl.GetFrame(frame) : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.GetFrameType(uint* simpleType, uint* detailedType) + => _legacyImpl is not null ? _legacyImpl.GetFrameType(simpleType, detailedType) : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.GetStackSizeSkipped(ulong* stackSizeSkipped) + => _legacyImpl is not null ? _legacyImpl.GetStackSizeSkipped(stackSizeSkipped) : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.Next() + => _legacyImpl is not null ? _legacyImpl.Next() : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.Request(uint reqCode, uint inBufferSize, byte* inBuffer, uint outBufferSize, byte* outBuffer) + => _legacyImpl is not null ? _legacyImpl.Request(reqCode, inBufferSize, inBuffer, outBufferSize, outBuffer) : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.SetContext(uint contextSize, [In, MarshalUsing(CountElementName = "contextSize")] byte[] context) + => _legacyImpl is not null ? _legacyImpl.SetContext(contextSize, context) : HResults.E_NOTIMPL; + int IXCLRDataStackWalk.SetContext2(uint flags, uint contextSize, [In, MarshalUsing(CountElementName = "contextSize")] byte[] context) + => _legacyImpl is not null ? _legacyImpl.SetContext2(flags, contextSize, context) : HResults.E_NOTIMPL; +} diff --git a/src/native/managed/cdacreader/src/Legacy/ClrDataTask.cs b/src/native/managed/cdacreader/src/Legacy/ClrDataTask.cs index b2a05958acb5e3..3c189701e57fd5 100644 --- a/src/native/managed/cdacreader/src/Legacy/ClrDataTask.cs +++ b/src/native/managed/cdacreader/src/Legacy/ClrDataTask.cs @@ -20,36 +20,55 @@ public ClrDataTask(TargetPointer address, Target target, IXCLRDataTask? legacyIm _legacyImpl = legacyImpl; } - public int GetProcess(/*IXCLRDataProcess*/ void** process) + int IXCLRDataTask.GetProcess(/*IXCLRDataProcess*/ void** process) => _legacyImpl is not null ? _legacyImpl.GetProcess(process) : HResults.E_NOTIMPL; - public int GetCurrentAppDomain(/*IXCLRDataAppDomain*/ void** appDomain) + int IXCLRDataTask.GetCurrentAppDomain(/*IXCLRDataAppDomain*/ void** appDomain) => _legacyImpl is not null ? _legacyImpl.GetCurrentAppDomain(appDomain) : HResults.E_NOTIMPL; - public int GetUniqueID(ulong* id) + int IXCLRDataTask.GetUniqueID(ulong* id) => _legacyImpl is not null ? _legacyImpl.GetUniqueID(id) : HResults.E_NOTIMPL; - public int GetFlags(uint* flags) + int IXCLRDataTask.GetFlags(uint* flags) => _legacyImpl is not null ? _legacyImpl.GetFlags(flags) : HResults.E_NOTIMPL; - public int IsSameObject(IXCLRDataTask* task) + int IXCLRDataTask.IsSameObject(IXCLRDataTask* task) => _legacyImpl is not null ? _legacyImpl.IsSameObject(task) : HResults.E_NOTIMPL; - public int GetManagedObject(/*IXCLRDataValue*/ void** value) + int IXCLRDataTask.GetManagedObject(/*IXCLRDataValue*/ void** value) => _legacyImpl is not null ? _legacyImpl.GetManagedObject(value) : HResults.E_NOTIMPL; - public int GetDesiredExecutionState(uint* state) + int IXCLRDataTask.GetDesiredExecutionState(uint* state) => _legacyImpl is not null ? _legacyImpl.GetDesiredExecutionState(state) : HResults.E_NOTIMPL; - public int SetDesiredExecutionState(uint state) + int IXCLRDataTask.SetDesiredExecutionState(uint state) => _legacyImpl is not null ? _legacyImpl.SetDesiredExecutionState(state) : HResults.E_NOTIMPL; - public int CreateStackWalk(uint flags, /*IXCLRDataStackWalk*/ void** stackWalk) - => _legacyImpl is not null ? _legacyImpl.CreateStackWalk(flags, stackWalk) : HResults.E_NOTIMPL; - public int GetOSThreadID(uint* id) + + int IXCLRDataTask.CreateStackWalk(uint flags, out IXCLRDataStackWalk? stackWalk) + { + stackWalk = default; + + Contracts.ThreadData threadData = _target.Contracts.Thread.GetThreadData(_address); + if (threadData.State.HasFlag(Contracts.ThreadState.Unstarted)) + return HResults.E_FAIL; + + IXCLRDataStackWalk? legacyStackWalk = null; + if (_legacyImpl is not null) + { + int hr = _legacyImpl.CreateStackWalk(flags, out legacyStackWalk); + if (hr < 0) + return hr; + } + + stackWalk = new ClrDataStackWalk(_address, flags, _target, legacyStackWalk); + return HResults.S_OK; + } + + int IXCLRDataTask.GetOSThreadID(uint* id) => _legacyImpl is not null ? _legacyImpl.GetOSThreadID(id) : HResults.E_NOTIMPL; - public int GetContext(uint contextFlags, uint contextBufSize, uint* contextSize, byte* contextBuffer) + int IXCLRDataTask.GetContext(uint contextFlags, uint contextBufSize, uint* contextSize, byte* contextBuffer) => _legacyImpl is not null ? _legacyImpl.GetContext(contextFlags, contextBufSize, contextSize, contextBuffer) : HResults.E_NOTIMPL; - public int SetContext(uint contextSize, byte* context) + int IXCLRDataTask.SetContext(uint contextSize, byte* context) => _legacyImpl is not null ? _legacyImpl.SetContext(contextSize, context) : HResults.E_NOTIMPL; - public int GetCurrentExceptionState(/*IXCLRDataExceptionState*/ void** exception) + int IXCLRDataTask.GetCurrentExceptionState(/*IXCLRDataExceptionState*/ void** exception) => _legacyImpl is not null ? _legacyImpl.GetCurrentExceptionState(exception) : HResults.E_NOTIMPL; - public int Request(uint reqCode, uint inBufferSize, byte* inBuffer, uint outBufferSize, byte* outBuffer) + int IXCLRDataTask.Request(uint reqCode, uint inBufferSize, byte* inBuffer, uint outBufferSize, byte* outBuffer) => _legacyImpl is not null ? _legacyImpl.Request(reqCode, inBufferSize, inBuffer, outBufferSize, outBuffer) : HResults.E_NOTIMPL; - public int GetName(uint bufLen, uint* nameLen, char* nameBuffer) + int IXCLRDataTask.GetName(uint bufLen, uint* nameLen, char* nameBuffer) => _legacyImpl is not null ? _legacyImpl.GetName(bufLen, nameLen, nameBuffer) : HResults.E_NOTIMPL; - public int GetLastExceptionState(/*IXCLRDataExceptionState*/ void** exception) + int IXCLRDataTask.GetLastExceptionState(/*IXCLRDataExceptionState*/ void** exception) => _legacyImpl is not null ? _legacyImpl.GetLastExceptionState(exception) : HResults.E_NOTIMPL; } diff --git a/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs b/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs index e1d42466b5bcb4..b2495bfb839fc2 100644 --- a/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs +++ b/src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs @@ -210,7 +210,7 @@ internal unsafe partial interface ISOSDacInterface // Modules [PreserveSig] - int GetModule(ulong addr, /*IXCLRDataModule*/ void** mod); + int GetModule(ulong addr, out IXCLRDataModule? mod); [PreserveSig] int GetModuleData(ulong moduleAddr, DacpModuleData* data); [PreserveSig] diff --git a/src/native/managed/cdacreader/src/Legacy/IXCLRData.cs b/src/native/managed/cdacreader/src/Legacy/IXCLRData.cs index c3f8a50be7bee3..3c1374f6243580 100644 --- a/src/native/managed/cdacreader/src/Legacy/IXCLRData.cs +++ b/src/native/managed/cdacreader/src/Legacy/IXCLRData.cs @@ -310,6 +310,37 @@ internal unsafe partial interface IXCLRDataProcess2 : IXCLRDataProcess int SetGcNotification(GcEvtArgs gcEvtArgs); } +[GeneratedComInterface] +[Guid("E59D8D22-ADA7-49a2-89B5-A415AFCFC95F")] +internal unsafe partial interface IXCLRDataStackWalk +{ + [PreserveSig] + int GetContext( + uint contextFlags, + uint contextBufSize, + uint* contextSize, + [Out, MarshalUsing(CountElementName = nameof(contextBufSize))] byte[] contextBuf); + [PreserveSig] + int SetContext(uint contextSize, [In, MarshalUsing(CountElementName = nameof(contextSize))] byte[] context); + + [PreserveSig] + int Next(); + + [PreserveSig] + int GetStackSizeSkipped(ulong* stackSizeSkipped); + + [PreserveSig] + int GetFrameType(/*CLRDataSimpleFrameType*/ uint* simpleType, /*CLRDataDetailedFrameType*/ uint* detailedType); + [PreserveSig] + int GetFrame(/*IXCLRDataFrame*/ void** frame); + + [PreserveSig] + int Request(uint reqCode, uint inBufferSize, byte* inBuffer, uint outBufferSize, byte* outBuffer); + + [PreserveSig] + int SetContext2(uint flags, uint contextSize, [In, MarshalUsing(CountElementName = nameof(contextSize))] byte[] context); +} + [GeneratedComInterface] [Guid("A5B0BEEA-EC62-4618-8012-A24FFC23934C")] internal unsafe partial interface IXCLRDataTask @@ -339,7 +370,7 @@ internal unsafe partial interface IXCLRDataTask int SetDesiredExecutionState(uint state); [PreserveSig] - int CreateStackWalk(uint flags, /*IXCLRDataStackWalk*/ void** stackWalk); + int CreateStackWalk(uint flags, out IXCLRDataStackWalk? stackWalk); [PreserveSig] int GetOSThreadID(uint* id); diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs index aad3accde2fbeb..4e7c70f563fd2e 100644 --- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs @@ -465,37 +465,20 @@ int ISOSDacInterface.GetMethodTableSlot(ulong mt, uint slot, ulong* value) => _legacyImpl is not null ? _legacyImpl.GetMethodTableSlot(mt, slot, value) : HResults.E_NOTIMPL; int ISOSDacInterface.GetMethodTableTransparencyData(ulong mt, void* data) => _legacyImpl is not null ? _legacyImpl.GetMethodTableTransparencyData(mt, data) : HResults.E_NOTIMPL; - int ISOSDacInterface.GetModule(ulong addr, /*IXCLRDataModule*/ void** mod) + int ISOSDacInterface.GetModule(ulong addr, out IXCLRDataModule? mod) { - ComWrappers cw = new StrategyBasedComWrappers(); + mod = default; - int hr; - nint legacyModulePointer = 0; - object? legacyModule = null; + IXCLRDataModule? legacyModule = null; if (_legacyImpl is not null) { - hr = _legacyImpl.GetModule(addr, (void**)&legacyModulePointer); + int hr = _legacyImpl.GetModule(addr, out legacyModule); if (hr < 0) return hr; - - legacyModule = cw.GetOrCreateObjectForComInstance(legacyModulePointer, CreateObjectFlags.None); } - ClrDataModule module = new(addr, _target, legacyModulePointer, legacyModule); - - // Lifetime is now managed via ClrDataModule - if (legacyModulePointer != 0) - Marshal.Release(legacyModulePointer); - - nint iunknownPtr = cw.GetOrCreateComInterfaceForObject(module, CreateComInterfaceFlags.None); - hr = Marshal.QueryInterface(iunknownPtr, typeof(IXCLRDataModule).GUID, out nint modPtr); - if (iunknownPtr != 0) - Marshal.Release(iunknownPtr); - - if (hr == HResults.S_OK) - *mod = (IXCLRDataModule*)modPtr; - - return hr; + mod = new ClrDataModule(addr, _target, legacyModule); + return HResults.S_OK; } int ISOSDacInterface.GetModuleData(ulong moduleAddr, DacpModuleData* data)