[cDAC] Add custom marshallers for COM interface parameters by max-charlamb · Pull Request #125132 · dotnet/runtime

max-charlamb · 2026-03-03T17:21:43Z

No description provided.

dotnet-policy-service · 2026-03-03T17:22:59Z

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR introduces DacComInterfaceIn<T> and DacComInterfaceOut<T> custom marshaller types to replace raw void*/void** and typed out IFoo? patterns for COM interface parameters in the cDAC legacy COM interface definitions. This improves type safety while correctly handling null COM interface pointers that the [GeneratedComInterface] source generator cannot natively accommodate.

Changes:

New DacComInterfaceIn<T>, DacComInterfaceOut<T> wrapper types and their [CustomMarshaller] implementations in a new Marshalling/ subfolder; ClrDataAddress.cs moved there as well.
All COM interface declarations in ISOSDacInterface.cs and IXCLRData.cs migrated from void*/void**/out IFoo? to the new typed wrappers.
Implementations updated in SOSDacImpl, ClrDataFrame, ClrDataModule, ClrDataTask, ClrDataStackWalk, and ClrDataMethodInstance, with ThrowIfNull()/IsNull null-safety guards added to match native E_POINTER/E_INVALIDARG behavior.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`Marshalling/DacComInterfaceOut.cs`	New `DacComInterfaceOut<T>` wrapper and marshaller for output COM interface parameters
`Marshalling/DacComInterfaceIn.cs`	New `DacComInterfaceIn<T>` wrapper and marshaller for input COM interface parameters
`Marshalling/ClrDataAddress.cs`	`ClrDataAddress` type moved to `Marshalling/` subfolder; no logic change
`ISOSDacInterface.cs`	Updated all COM interface parameter types + added `ISOSStackRefEnum`/`ISOSMemoryEnum` interface declarations
`IXCLRData.cs`	Updated all COM interface parameter types across all `IXCLRData*` interfaces
`SOSDacImpl.cs`	Updated implementations for `GetHandleEnum`, `GetHandleEnumForTypes`, `GetHandleEnumForGC`, `GetModule`, `GetStackReferences`, ISOSDacInterface13 passthrough methods, `GetMethodTableSlotEnumerator`
`SOSDacImpl.IXCLRDataProcess.cs`	Updated IXCLRDataProcess implementations with new typed parameters and try-catch wrapping
`ClrDataTask.cs`	Updated IXCLRDataTask implementations
`ClrDataStackWalk.cs`	Updated `GetFrame` implementation
`ClrDataModule.cs`	Updated IXCLRDataModule implementations
`ClrDataFrame.cs`	Updated IXCLRDataFrame implementations
`ClrDataMethodInstance.cs`	Updated IXCLRDataMethodInstance implementations, cleaned up COM interop code

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ISOSDacInterface.cs

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataTask.cs

...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodInstance.cs

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

.../managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

...anaged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComInterfaceIn.cs

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataStackWalk.cs

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs

...naged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComInterfaceOut.cs

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

...naged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComInterfaceOut.cs

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ISOSDacInterface.cs

.../managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs

jkoritzinsky

LGTM

...naged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComInterfaceOut.cs

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComNullableByRef.cs:1

The marshaller only registers MarshalMode.UnmanagedToManagedIn and MarshalMode.ManagedToUnmanagedIn modes. For an output parameter in [GeneratedComInterface]-decorated interfaces, the source generator requires MarshalMode.UnmanagedToManagedIn for the receiver (managed implementation called by native code) but may also need MarshalMode.ManagedToUnmanagedOut or MarshalMode.Default for certain call directions. More specifically, ManagedToUnmanagedIn is for passing a value from managed into native — but this parameter is an output from the callee's perspective. Using ManagedToUnmanagedIn when passing a DacComNullableByRef<T> to a legacy native call (as done in SOSDacImpl.cs) means the source generator is treating the wrapper as an input, not an output sink. Verify that the registered marshal modes produce the correct generated code for both call directions (native→managed and managed→native).

// Licensed to the .NET Foundation under one or more agreements.

...aged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComNullableByRef.cs

.../managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs

Replace raw void*/void** and out T? COM interface parameters with DacComNullableByRef<T> across the cDAC legacy layer: - Add DacComNullableByRef<T> with stateful custom marshallers for both UnmanagedToManaged and ManagedToUnmanaged directions. Setting Interface when IsNullRef is true throws NullReferenceException, mirroring the native behavior of writing through a null void**. - Convert all out T? COM interface output parameters in IXCLRData.cs and ISOSDacInterface.cs to DacComNullableByRef<T>. - Update all implementation files to match new signatures: pass-through methods become expression-bodied, cDAC implementations set .Interface directly (NRE on null ref) or guard with IsNullRef for optional params. - Fully define ISOSStackRefEnum and ISOSMemoryEnum interfaces with their Next methods, supporting structs (SOSStackRefData, SOSStackRefError, SOSMemoryRegion), enums (SOSStackSourceType), and ISOSStackRefErrorEnum. - Move ClrDataAddress.cs to the Marshalling folder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

...aged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComNullableByRef.cs

max-charlamb · 2026-03-06T17:46:35Z

/ba-g "cDAC only change and cDAC tests pass"

Copilot AI review requested due to automatic review settings March 3, 2026 17:21

github-actions bot added the area-Diagnostics-coreclr label Mar 3, 2026

dotnet-policy-service bot assigned max-charlamb Mar 3, 2026

Copilot started reviewing on behalf of max-charlamb March 3, 2026 17:22 View session

max-charlamb requested review from jkoritzinsky and rcj1 March 3, 2026 17:27

Copilot AI reviewed Mar 3, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

Copilot AI review requested due to automatic review settings March 3, 2026 18:05

Copilot started reviewing on behalf of max-charlamb March 3, 2026 18:07 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

max-charlamb requested a review from Copilot March 3, 2026 20:35

Copilot started reviewing on behalf of max-charlamb March 3, 2026 20:36 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

max-charlamb mentioned this pull request Mar 3, 2026

[cDAC] Implement GetCurrentAppDomain on IXCLRDataTask #124996

Merged

This comment was marked as outdated.

Sign in to view

build-analysis bot mentioned this pull request Mar 4, 2026

"We stopped hearing from agent Azure Pipelines 32. Verify the agent machine is running and has a healthy network connection" dotnet/dnceng#1886

Open

3 tasks

Copilot AI review requested due to automatic review settings March 4, 2026 23:08

max-charlamb force-pushed the dev/maxcharlamb/runtime-cdac-interface-marshalling branch from b01d788 to ecb58d5 Compare March 4, 2026 23:08

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Copilot started reviewing on behalf of max-charlamb March 4, 2026 23:15 View session

max-charlamb marked this pull request as draft March 4, 2026 23:44

jkoritzinsky reviewed Mar 4, 2026

View reviewed changes

...naged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComInterfaceOut.cs Outdated Show resolved Hide resolved

noahfalk reviewed Mar 5, 2026

View reviewed changes

...naged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComInterfaceOut.cs Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings March 5, 2026 16:36

Copilot AI reviewed Mar 5, 2026

View reviewed changes

max-charlamb marked this pull request as ready for review March 5, 2026 17:07

Copilot AI review requested due to automatic review settings March 5, 2026 17:07

Copilot AI reviewed Mar 5, 2026

View reviewed changes

max-charlamb closed this Mar 5, 2026

max-charlamb reopened this Mar 5, 2026

Copilot AI review requested due to automatic review settings March 5, 2026 21:42

Copilot started reviewing on behalf of max-charlamb March 5, 2026 21:44 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 5, 2026 22:36

max-charlamb force-pushed the dev/maxcharlamb/runtime-cdac-interface-marshalling branch from 61ac4b0 to f57d8a7 Compare March 5, 2026 22:36

Copilot AI reviewed Mar 5, 2026

View reviewed changes

noahfalk approved these changes Mar 6, 2026

View reviewed changes

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataModule.cs Show resolved Hide resolved

Max Charlamb and others added 2 commits March 6, 2026 10:56

fix spelling

08ff1af

Copilot AI review requested due to automatic review settings March 6, 2026 16:01

max-charlamb force-pushed the dev/maxcharlamb/runtime-cdac-interface-marshalling branch from 5e77fcc to 08ff1af Compare March 6, 2026 16:01

Copilot started reviewing on behalf of max-charlamb March 6, 2026 16:03 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

...aged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComNullableByRef.cs Show resolved Hide resolved

...aged/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Marshalling/DacComNullableByRef.cs Show resolved Hide resolved

max-charlamb enabled auto-merge (squash) March 6, 2026 17:50

max-charlamb merged commit 6bf2076 into dotnet:main Mar 6, 2026
52 of 56 checks passed

max-charlamb deleted the dev/maxcharlamb/runtime-cdac-interface-marshalling branch March 6, 2026 17:52

dotnet-maestro bot mentioned this pull request Mar 7, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#5301

Open

Conversation

max-charlamb commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkoritzinsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

max-charlamb commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

max-charlamb commented Mar 3, 2026 •

edited

Loading

max-charlamb commented Mar 6, 2026 •

edited

Loading