[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**`
dotnet · Oct 30, 2024 · a72cfb0 · a72cfb0
1 parent 95bae2b
commit a72cfb0
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 46 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)