SOS: activate data access through dbgshim using cDAC support policy - #5966
Open
hoyosjs wants to merge 7 commits into
Open
SOS: activate data access through dbgshim using cDAC support policy#5966hoyosjs wants to merge 7 commits into
hoyosjs wants to merge 7 commits into
Conversation
SOS enumerates the runtime and hands the module base to dbgshim, which prefers the co-located cDAC and falls back to the legacy DAC. The resulting IXCLRDataProcess is registered with ClrMD (managed host, via DataTarget.AddLoadedRuntime) or returned to native SOS, replacing the direct cDAC LoadLibrary path. Remove the runtime-version gate from the cDAC load policy: every runtime attempts the cDAC and the cDAC's own contract-support validation decides. Rework Runtime.CreateRuntime so a cDAC that loads but declines falls back to the in-box DAC; a forced cDAC (UseCDac) stays a hard failure rather than silently using the DAC. Extract TryCreateRuntimeFromLibrary. Move the dbgshim managed interop (ICLRDebugging, ICLRDebuggingPolicy) from the test project into SOS.Hosting as the single product copy. Add IClrDataProcessActivator (DebugServices) implemented by ClrDataProcessActivator (SOS.Hosting), which loads the co-located dbgshim, sets the prefer-cDAC policy, builds a runtime-bound DataTargetWrapper, and returns the IXCLRDataProcess from OpenVirtualProcess.
Add greppable 'data-access:' trace lines at every branch of the cDAC/DAC decision in both the managed ClrMD path (Runtime.CreateRuntime) and the native SOS path (RuntimeWrapper.GetClrDataProcess): seam tried, seam activated the cDAC, seam declined, no seam in host, direct cDAC tried/declined, forced-cDAC hard failure, and in-box DAC fallback. A log now shows exactly which engine served each runtime.
Remove the remaining native runtime-version check and let the cDAC validate whether it can service each target. Carry the effective cDAC policy through the hosted dbgshim activator, including a cDAC-only mode that cannot silently fall back to the DAC. Preserve forced-cDAC rejection across the RuntimeWrapper boundary so native SOS and CLRMA do not replace it with DbgEng's DAC. Serialize per-activation dbgshim policy changes and keep the default prefer-cDAC fallback behavior for unsupported targets.
Make the IXCLRDataProcess service contract implementation-neutral and remove comments that describe callers, fallback policy, or historical behavior. Remove the direct ClrMD cDAC load and the separate cDAC path resolver from Runtime. cDAC activation now only occurs through the registered service; ClrMD loads only the DAC fallback.
Keep the host-owned IXCLRDataProcess reference alive until the Runtime and its ClrMD services are disposed. ClrMD adds and releases its own independent reference while retaining a factory that may return the host pointer. Allow native SOS and CLRMA to use DbgEng DAC fallback only when cDAC is not enforced.
Replace native SOS's direct cDAC LoadLibrary path with CLRCreateInstance and ICLRDebugging::OpenVirtualProcess against the dbgshim next to SOS. The native path requests cDAC-only activation and retains the legacy DAC fallback only when policy allows it. This keeps cDAC activation available when managed hosting is disabled or fails while preserving the same support-based policy behavior.
Contributor
There was a problem hiding this comment.
Pull request overview
Routes SOS cDAC activation through dbgshim with policy-based DAC fallback and ClrMD runtime registration.
Changes:
- Adds native and managed dbgshim activation paths.
- Centralizes cDAC policy handling and removes direct cDAC loading.
- Registers activated runtimes with ClrMD.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Review summary |
|---|---|
src/SOS/Strike/util.cpp |
Reviewed; no final comments. |
src/SOS/Strike/platform/runtimeimpl.h |
Reviewed; no final comments. |
src/SOS/Strike/platform/runtimeimpl.cpp |
Moderate: restore CLR notification setup for live targets. |
src/SOS/Strike/clrma/managedanalysis.cpp |
Reviewed; no final comments. |
src/SOS/SOS.Hosting/RuntimeWrapper.cs |
Moderate: honor cDAC environment gates while preserving forced-policy behavior. |
src/SOS/SOS.Hosting/ICLRDebuggingPolicy.cs |
Reviewed; no final comments. |
src/SOS/SOS.Hosting/ICLRDebugging.cs |
Reviewed; no final comments. |
src/SOS/SOS.Hosting/ClrDataProcessActivator.cs |
Moderate: release retained wrappers and services after failed activation and teardown. |
src/SOS/inc/runtime.h |
Reviewed; no final comments. |
src/Microsoft.Diagnostics.TestHelpers/TestHost/TestDataWriter.cs |
Reviewed; no final comments. |
src/Microsoft.Diagnostics.DebugServices/IRuntime.cs |
Reviewed; no final comments. |
src/Microsoft.Diagnostics.DebugServices/IClrDataProcessActivator.cs |
Reviewed; no final comments. |
src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs |
Reviewed; no final comments. |
Suppressed comments (1)
src/SOS/Strike/platform/runtimeimpl.cpp:546
- The new
CDacOnlyflag is not considered byGetClrDataProcess; the failure and fallback checks below consult onlys_cdacLoadPolicy. A caller passingCDacOnlywhile the global policy isDefaulttherefore falls back to the DAC when dbgshim declines, despite the flag's documented no-fallback semantics (and the managed wrapper treating it as forced). Include the flag in both the activation decision and the fallback suppression checks.
HRESULT hr = CreateClrDataProcessWithDbgShim(&m_cdacDataProcess);
if (FAILED(hr) && s_cdacLoadPolicy == CDacLoadPolicy::UseCDac)
{
*ppClrDataProcess = nullptr;
return hr;
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+61
to
+62
| DataTargetWrapper dataTarget = new(runtime.Services, runtime); | ||
| _dataTargets.Add(dataTarget); |
Comment on lines
+244
to
+246
| bool useCDac = | ||
| configuredPolicy != CDacLoadPolicy.UseLegacyDac && | ||
| (flags & ClrDataProcessFlags.UseCDac) != 0; |
Comment on lines
+746
to
+747
| *ppClrDataProcess = (IXCLRDataProcess*)process; | ||
| return S_OK; |
noahfalk
approved these changes
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use dbgshim as SOS's only cDAC activation path. SOS supplies the runtime module and data target, receives an
IXCLRDataProcess, and registers it with ClrMD throughAddLoadedRuntime.Remove the runtime-version checks that selected cDAC. The cDAC now determines target support through its contract validation:
Design
IClrDataProcessActivatorkeeps DebugServices independent from the hosting and COM implementation.ICLRDebugging::OpenVirtualProcessdirectly when managed hosting is navailable.IXCLRDataProcessreference for the registered runtime lifetime; ClrMD owns a separate reference.Validation
Verified with dotnet-dump and CDB against .NET 10 and .NET 11 dumps:
Commands verified:
runtimes,clrstack, anddumpheap -stat.Native-only CDB was also verified with
!sethostruntime -none: