Allow DAC fallback for non-CoreCLR runtimes - #6016
Open
max-charlamb wants to merge 1 commit into
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b2ca30d-742d-402c-8ba5-afec8b6d69a1
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The native effective-policy mapping currently forces cDAC for RuntimeConfiguration.Unknown, which can incorrectly block DAC fallback for non-CoreCLR runtime types.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new coreclr cDAC load policy across SOS hosting so cDAC is required for CoreCLR runtimes while still allowing legacy DAC fallback for other runtime types (notably desktop .NET Framework), and updates the SOS cdac test mode to use the new policy.
Changes:
- Introduces
CDacLoadPolicy.OnlyUseCDacForCoreClrand wires it through both managed and native SOS hosting paths. - Extends the
runtimes --usecdacoption (managed + native) and settings display to accept/showcoreclr. - Updates SOS unit test runner
cdacmode to useruntimes --usecdac coreclrrather than forcing cDAC globally.
File summaries
| File | Description |
|---|---|
| src/tests/SOS.UnitTests/SOSRunner.cs | Switches test cdac mode to --usecdac coreclr to avoid blocking inspection of non-CoreCLR runtimes. |
| src/SOS/Strike/strike.cpp | Adds coreclr parsing/output for --usecdac in native SOS. |
| src/SOS/Strike/platform/runtimeimpl.cpp | Implements effective policy mapping for OnlyUseCDacForCoreClr on the native hosting path. |
| src/SOS/SOS.Hosting/RuntimeWrapper.cs | Centralizes the managed “effective policy” mapping and uses it for cDAC policy queries and CorDebug creation. |
| src/SOS/inc/runtime.h | Extends native CDacLoadPolicy enum with OnlyUseCDacForCoreClr. |
| src/Microsoft.Diagnostics.ExtensionCommands/Host/RuntimesCommand.cs | Adds coreclr to the managed runtimes --usecdac option mapping and help text. |
| src/Microsoft.Diagnostics.ExtensionCommands/Host/CommandFormatHelpers.cs | Updates settings display formatting to print coreclr. |
| src/Microsoft.Diagnostics.DebugServices/CDacLoadPolicy.cs | Adds the new public managed enum value and XML doc. |
| src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs | Applies the “effective policy” mapping when creating a ClrRuntime. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+598
to
+605
| CDacLoadPolicy policy = GetConfiguredCDacLoadPolicy(); | ||
| if (policy == CDacLoadPolicy::OnlyUseCDacForCoreClr) | ||
| { | ||
| return GetRuntimeConfiguration() == IRuntime::WindowsDesktop | ||
| ? CDacLoadPolicy::PreferCDac | ||
| : CDacLoadPolicy::OnlyUseCDac; | ||
| } | ||
| return policy; |
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.
Summary
Add a
coreclrcDAC load policy that:true,false, andpreferpolicy behaviorThe policy is implemented consistently in the managed and native SOS hosting paths. The SOS
cdactest mode now selectsruntimes --usecdac coreclrinstead of globally requiring cDAC.Motivation
A process may contain both CoreCLR and desktop .NET Framework runtimes. Requiring standalone cDAC globally prevents SOS from inspecting Framework after switching runtimes because Framework does not support cDAC.
This was exposed by
SOSScenarioTests.DualRuntimesin: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1587921The unrelated interpreter build fix is isolated in #6015.
Validation
SOSScenarioTests.DualRuntimespasses incdacmode without a test-specific policy change or runtime-path overrideNote
This pull request description was generated with GitHub Copilot.