SOS: load DBI through dbgshim by hoyosjs · Pull Request #5980 · dotnet/diagnostics

hoyosjs · 2026-08-25T23:15:37Z

Depends on #5966.

Route CoreCLR ICorDebugProcess activation through dbgshim so the bundled universal DBI and cDAC are tried first, with SOS's existing DAC/DBI resolver retained as the provider fallback. Keep desktop CLR on its existing direct mscordbi path because the public dbgshim factory is CoreCLR-SKU-specific.

Ship mscordbi_universal from the cDAC transport package beside dbgshim and mscordaccore_universal in the shared SOS packaging used by dotnet-sos, dotnet-dump, and the WinDbg SOS package.

Validation:

managed SOS.Hosting build
native x64 build
DbgShim.UnitTests: 36/36
published dotnet-dump clrstack -i with forced cDAC used the bundled DBI
provider fallback clrstack -i used the matching runtime DAC/DBI
win-x64 dotnet-sos, dotnet-dump, and Microsoft.NETCore.SOS packages contain dbgshim, universal cDAC, and universal DBI

Copilot

Pull request overview

This PR updates SOS’s DBI activation path so CoreCLR ICorDebugProcess creation is routed through dbgshim (preferring the bundled universal DBI + cDAC first), while keeping the existing direct mscordbi activation path for Desktop CLR. It also updates managed hosting to use the same dbgshim-based activation and packages the universal DBI alongside existing SOS assets.

Changes:

Native SOS: add dbgshim-based ICorDebugProcess activation (CoreCLR) and factor Desktop CLR activation into a dedicated helper.
SOS.Hosting: introduce a managed ICLRDebuggingLibraryProvider2 implementation and route CorDebug activation through a new IClrDataProcessActivator.CreateCorDebugProcess API.
Packaging: include mscordbi_universal in the shared SOS packaging (gated by PackageWithCDac).

Reviewed changes

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

Show a summary per file

File	Description
src/SOS/Strike/platform/runtimeimpl.h	Adds `CreateDesktopCorDebugProcess` declaration to separate Desktop CLR activation path.
src/SOS/Strike/platform/runtimeimpl.cpp	Implements Desktop CLR DBI activation helper and routes CoreCLR DBI activation through dbgshim + library provider.
src/SOS/SOS.Hosting/RuntimeWrapper.cs	Updates managed DBI activation to use `IClrDataProcessActivator` for CoreCLR and improves HRESULT propagation.
src/SOS/SOS.Hosting/RuntimeLibraryProvider.cs	Adds managed `ICLRDebuggingLibraryProvider2` implementation to resolve DBI/DAC paths for dbgshim.
src/SOS/SOS.Hosting/ClrDataProcessActivator.cs	Refactors dbgshim initialization and adds `CreateCorDebugProcess` to activate `ICorDebugProcess` via dbgshim.
src/sos-packaging.props	Packages `mscordbi_universal` for supported RIDs when `PackageWithCDac` is enabled.
src/Microsoft.Diagnostics.DebugServices/IClrDataProcessActivator.cs	Extends the activator interface with a new CorDebug activation API.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

+    ArrayHolder<WCHAR> dacModulePath = new WCHAR[MAX_LONGPATH + 1];
+    if (MultiByteToWideChar(CP_ACP, 0, dacFilePath, -1, dacModulePath, MAX_LONGPATH) <= 0)
+    {
+        return HRESULT_FROM_WIN32(GetLastError());
+    }


+            modulePath = IntPtr.Zero;
+
+            string path = fileName?.IndexOf("mscordbi", StringComparison.OrdinalIgnoreCase) >= 0
+                ? _getDbiPath()
+                : _getDacPath();
+            if (string.IsNullOrEmpty(path))
+            {
+                Trace.TraceError($"RuntimeLibraryProvider: could not resolve {fileName}");
+                return HResult.E_NOINTERFACE;
+            }
+
+            modulePath = Marshal.StringToCoTaskMemUni(path);
+            Trace.TraceInformation($"RuntimeLibraryProvider: resolved {fileName} to {path}");
+            return HResult.S_OK;


noahfalk

LGTM modulo the policy handling

noahfalk · 2026-08-28T23:28:17Z

-            // This also verifies the DAC signature before the DBI is passed the DAC path or handle.
+            CDacLoadPolicy policy = _runtime.RuntimeType == RuntimeType.Desktop
+                ? CDacLoadPolicy.UseLegacyDac
+                : _services.GetService<ISettingsService>()?.CDacLoadPolicy ?? CDacLoadPolicy.PreferCDac;


I don't think SOS should ever be changing the policy, we should insert some logic in the implementation of CreateCorDebugProcess so that it generates the expected result for each policy option on desktop.
cDacOnly -> loading fails
other policies -> load DAC as usual

noahfalk · 2026-08-28T23:37:55Z

+
+    DbgShimCDacLoadPolicy loadPolicy = GetRuntimeConfiguration() == IRuntime::WindowsDesktop
+        ? DbgShimCDacLoadPolicy::LegacyDacOnly
+        : (DbgShimCDacLoadPolicy)GetCDacLoadPolicy();


Same as above. We should handle the policy we are given rather than changing it which implies cDacOnly on desktop is expected to fail.

yeah - I had the same change locally. Trying to clean up this change.

Teach dbgshim to recognize Desktop CLR through the standard ICLRDebugging path and apply the configured cDAC policy consistently to data access and DBI activation. Verify provider-resolved DAC and DBI binaries with WinTrust, Microsoft root policy, and the .NET DAC OID before loading them. Keep bundled universal DBI on the tool-asset trust path, preserve DAC-before-DBI ordering, and contain provider callback failures.

noahfalk · 2026-09-02T10:49:35Z

-    public static bool ShouldTryCDac(CDacLoadPolicy policy)
+    /// <param name="policy">The configured activation policy.</param>
+    /// <returns>The policy to use for activation.</returns>
+    public static CDacLoadPolicy GetEffectiveLoadPolicy(CDacLoadPolicy policy)


Can we avoid this and instead have tests that set DOTNET_ENABLE_CDAC also change the SOS policy explicitly via command if they need to? If it isn't straightforward for the tests to set the policy via commands we could also have some SOS_CDAC_LOAD_POLICY env var that overrides the default policy that we initialize at startup.

High level my aim is:

Keep the SOS policy as simple as possible. Having both a "configured" policy and an "effective" policy makes it harder to reason about what SOS is going to do.

Keep test env vars narrowly focused. Ideally any given test env var only modifies the behavior of one component.

noahfalk · 2026-09-02T10:56:39Z

-# resolves the cDAC from its own native binaries directory, so this is the only spot it is picked up
-# from. Used by the cdac DacMode to exercise the runtime-under-test's own cDAC instead of the copy
-# restored from a referenced runtime package.
+# Overlay an externally-provided cDAC transport next to the freshly built sos.dll. SOS resolves the


cDAC transport = cDAC transport package?

Assuming cdacPath refers to the binary and not the package then I'd just say 'cDAC'

noahfalk · 2026-09-02T10:59:59Z

 usage_list+=("-classfilter: pass class filter to xunit runner (Namespace.ClassName)")
 usage_list+=("-dacmode: which DAC/cDAC the SOS tests load: cdac, cdacverify, or dac.")
-usage_list+=("-cdacpath: path to an mscordaccore_universal to overlay next to sos.dll (only with -dacmode cdac).")
+usage_list+=("-cdacpath: path to an mscordaccore_universal whose sibling universal DBI is overlaid next to SOS (only with -dacmode cdac).")


The new comment suggests only DBI is overlayed but I think want to say that both cDAC and universal DBI are overlayed.

noahfalk · 2026-09-02T11:00:43Z

-# Overlay an externally-provided cDAC (libmscordaccore_universal) next to the freshly built sos so
-# SOS resolves it from its own native binaries directory. Used by the cdac DacMode to exercise the
-# runtime-under-test's own cDAC instead of the copy restored from a referenced runtime package.
+# Overlay an externally-provided cDAC transport next to the freshly built sos. SOS resolves the cDAC


Suggested change

# Overlay an externally-provided cDAC transport next to the freshly built sos. SOS resolves the cDAC

# Overlay an externally-provided cDAC next to the freshly built sos. SOS resolves the cDAC

binary or package?

noahfalk · 2026-09-02T11:13:26Z

-        // The signature needs to match m_skuId exactly, except for m_skuId=CLR_ID_ONECORE_CLR which is
-        // also compatible with the older CLR_ID_PHONE_CLR signature.
-        if (SUCCEEDED(hr) && (debugResource.signature != m_skuId) && !( (debugResource.signature == CLR_ID_PHONE_CLR) && (m_skuId == CLR_ID_ONECORE_CLR) ))
+        // The OneCore CLR debugging object supports CoreCLR and Desktop CLR targets. Other debugging


When we chatted earlier I'm worried I didn't think it through very well.

I'm worried this might be a breaking change. Debuggers could previously rely on CLRDebugging from dbgshim to only bind to CoreCLR binaries and now its going to bind to desktop binaries too. Although both runtimes provide ICorDebug as output the behavior of the APIs isn't perfectly identical. I'm pretty sure VS has various places in their code which special case how they interact with the API depending on CoreCLR vs. Desktop.

Seperate from the back-compat issue it also means now we'd have a testing/compat burden for dbgshim to work with desktop that would be difficult to unwind. Ideally we want new stuff to be migrating away from desktop coupling rather than towards it. For example at some point in the future we'll probably want to have a version of SOS that stops supporting desktop runtime.

noahfalk · 2026-09-02T11:21:00Z

 VERIFY:\s+<HEXVAL>\s+<HEXVAL>\s+[Dd]iv[Zz]ero.*!C\.F2(\(.*\))?\+0x<HEXVAL>\s+
 VERIFY:\[.*[\\|/]Debuggees[\\|/].*DivZero[\\|/]DivZero\.cs @ 36\s*\]\s*
 VERIFY:\s+<HEXVAL>\s+<HEXVAL>\s+[Dd]iv[Zz]ero.*!C\.Main(\(.*\))?\+0x<HEXVAL>\s+
+IFDEF:DESKTOP


Were desktop tests not previously running on these scripts or does this represent a change in SOS behavior when running against desktop?

hoyosjs added 2 commits August 26, 2026 18:45

SOS: load DBI through dbgshim

4a6642c

SOS: separate cDAC and DBI activators

3e724e8

hoyosjs force-pushed the juhoyosa/dbi-through-dbgshim branch from 5d53e54 to 3e724e8 Compare August 27, 2026 01:48

max-charlamb marked this pull request as ready for review August 27, 2026 15:41

Copilot AI lite review requested due to automatic review settings August 27, 2026 15:41

max-charlamb requested a review from a team as a code owner August 27, 2026 15:41

Copilot started reviewing on behalf of max-charlamb August 27, 2026 15:42 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

noahfalk approved these changes Aug 28, 2026

View reviewed changes

hoyosjs added 2 commits August 28, 2026 16:58

SOS: extract dbgshim DBI activation

f6e6724

SOS: unify DBI activation through dbgshim

a1ceb4a

max-charlamb mentioned this pull request Aug 31, 2026

Add cDAC dump collection provider dotnet/runtime#132584

Open

hoyosjs added 6 commits August 31, 2026 15:51

SOS: reuse ClrMD DAC signature verification

9e0a208

Build: overlay matched cDAC transport

1cb6ff9

SOS: inline DAC provider path delegate

50795ce

SOS: centralize effective cDAC policy

5c35ca5

Tests: account for Desktop CLR behavior

587f273

build-analysis Bot mentioned this pull request Sep 2, 2026

[Tests] LogsPipelineUnitTests TestLogsPipeline fails with Assert.Equal() Failure #5659

Open

noahfalk reviewed Sep 2, 2026

View reviewed changes

Navigation Menu

Uh oh!

SOS: load DBI through dbgshim - #5980

SOS: load DBI through dbgshim#5980
hoyosjs wants to merge 10 commits into
dotnet:mainfrom
hoyosjs:juhoyosa/dbi-through-dbgshim

hoyosjs commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

noahfalk left a comment

Uh oh!

noahfalk Aug 28, 2026

Uh oh!

noahfalk Aug 28, 2026

Uh oh!

hoyosjs Aug 28, 2026

Uh oh!

noahfalk Sep 2, 2026

Uh oh!

noahfalk Sep 2, 2026

Uh oh!

noahfalk Sep 2, 2026

Uh oh!

noahfalk Sep 2, 2026

Uh oh!

noahfalk Sep 2, 2026

Uh oh!

noahfalk Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# Overlay an externally-provided cDAC transport next to the freshly built sos. SOS resolves the cDAC
	# Overlay an externally-provided cDAC next to the freshly built sos. SOS resolves the cDAC

Uh oh!

Conversation

hoyosjs commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

noahfalk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants