Skip to content

Add DbgShimCreateInstanceFromContractDescriptor cDAC export#131357

Merged
hoyosjs merged 1 commit into
dotnet:mainfrom
hoyosjs:juhoyosa/cdac-contract-create-instance
Jul 25, 2026
Merged

Add DbgShimCreateInstanceFromContractDescriptor cDAC export#131357
hoyosjs merged 1 commit into
dotnet:mainfrom
hoyosjs:juhoyosa/cdac-contract-create-instance

Conversation

@hoyosjs

@hoyosjs hoyosjs commented Jul 24, 2026

Copy link
Copy Markdown
Member

Add a new cdac export that provides DAC/SOS interfaces but takes the contract descriptor address directly instead of requiring the data target to implement ICLRContractLocator. This is an internal API for dbgshim.

Refactor CLRDataCreateInstanceCore into a descriptor-discovery step plus a shared CreateInstanceFromContractDescriptorCore helper, and fix a latent bug where the QueryInterface HRESULT was ignored and S_OK was returned even on QI failure.

Note

This PR description was drafted with GitHub Copilot.

Add a new cdac export that provides DAC/SOS interfaces but takes the contract descriptor address directly instead of requiring the data
target to implement ICLRContractLocator. This is an internal API for dbgshim.

Refactor CLRDataCreateInstanceCore into a descriptor-discovery step plus a shared
CreateInstanceFromContractDescriptorCore helper, and fix a latent bug where the
QueryInterface HRESULT was ignored and S_OK was returned even on QI failure.
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

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.

Pull request overview

Adds a new unmanaged export in the managed cDAC entrypoint layer to create DAC/SOS interfaces directly from a known contract-descriptor address (avoiding an ICLRContractLocator requirement on the data target), while also refactoring the existing CLRDataCreateInstance path to share the instance-creation core and correcting HRESULT handling for QueryInterface.

Changes:

  • Add DbgShimCreateInstanceFromContractDescriptor unmanaged export to create an instance given an explicit contract descriptor address.
  • Refactor CLRDataCreateInstanceCore into descriptor discovery + shared CreateInstanceFromContractDescriptorCore.
  • Fix a correctness bug by returning the actual QueryInterface HRESULT on failure instead of returning success.

Comment thread src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs
@hoyosjs
hoyosjs enabled auto-merge (squash) July 25, 2026 00:01
hoyosjs added a commit to hoyosjs/diagnostics that referenced this pull request Jul 25, 2026
When servicing a data-access interface (`IXCLRDataProcess` / `ISOSDacInterface`), dbgshim now resolves the target runtime module's `DotNetRuntimeContractDescriptor` export itself and activates the cDAC through the universal binary's new `DbgShimCreateInstanceFromContractDescriptor` entry point (dotnet/runtime#131357), passing the descriptor address directly.

This is needed because dbgshim's data targets don't always implement `ICLRContractLocator`, so the existing `CLRDataCreateInstance` path can't source the descriptor on them.

Behavior:
- Descriptor not resolvable -> cDAC can't succeed, so it's skipped and we fall through to the legacy DAC.
- Descriptor resolvable but the cDAC binary predates the new export -> falls back to `CLRDataCreateInstance` on the same binary (temporary while cDAC gets updated in the tools).
- `LegacyDacOnly` policy and the public dbgshim API are unchanged; this is purely additive.
-  On Windows-host targets the descriptor is read via a small PE export-directory walk (`TryGetPEExportSymbol`) that we already use in runtime; ELF/Mach-O use the existing `TryGetSymbol`.

## Testing

`DbgShim.UnitTests` 104/104 pass. The contract path itself can't be exercised until a `mscordaccore_universal` carrying the new export flows in; it degrades gracefully until then.
@hoyosjs
hoyosjs merged commit 1741008 into dotnet:main Jul 25, 2026
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants