Fix SOS CLRMA crash in the native (non-hosted) !analyze path - #5946
Open
hoyosjs wants to merge 6 commits into
Open
Fix SOS CLRMA crash in the native (non-hosted) !analyze path#5946hoyosjs wants to merge 6 commits into
hoyosjs wants to merge 6 commits into
Conversation
The exported CLRMACreateInstance entrypoint runs outside any SOS command, but the native Target/Runtime/DataTarget/Host fallback objects read the command-scoped g_Ext* globals, which are null outside a command. !analyze hit this and crashed in Target::GetOperatingSystem. These objects now use the session-lifetime IDebuggerServices (backed by DbgEngServices/LLDBServices) instead of g_Ext*, so they are valid regardless of command scope. OS-dependent choices, including the CLRMA module-name separator, now come from ITarget::GetOperatingSystem(). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exercises the DAC CLRMA provider through the SOS native Target/DataTarget/Runtime objects under cdb with no managed host, verifying provider association and the native stack walk resolve through the session-lifetime IDebuggerServices. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in SOS CLRMA’s native (non-hosted) path by removing reliance on command-scoped g_Ext* globals and instead using session-lifetime IDebuggerServices for OS detection, module enumeration, memory access, and thread context operations—so the exported CLRMACreateInstance entrypoint can run safely outside SOS command scope.
Changes:
- Refactors SOS native
Host/Target/DataTarget/Runtimefallback implementations to useIDebuggerServicesinstead ofg_Ext*globals. - Moves OS-dependent behavior (notably CLRMA module-name path separator selection) to use
ITarget::GetOperatingSystem(). - Adds a unit test script coverage scenario to exercise the native CLRMA path under
cdbwhen no managed host is present.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/SOS.UnitTests/Scripts/StackAndOtherTests.script | Adds coverage to exercise the non-hosted CLRMA path under cdb and validate OS/thread-context dependent behavior. |
| src/SOS/Strike/sosextensions.cpp | Passes session-lifetime debugger services into the local Host singleton creation. |
| src/SOS/Strike/platform/targetimpl.h | Updates Target singleton API to require debugger services; adjusts runtime switching behavior. |
| src/SOS/Strike/platform/targetimpl.cpp | Implements Target to use IDebuggerServices for OS detection and process ID queries; manages debugger-service lifetime. |
| src/SOS/Strike/platform/runtimeimpl.cpp | Replaces g_Ext* usage with IDebuggerServices for module enumeration and memory reads in runtime discovery. |
| src/SOS/Strike/platform/hostimpl.h | Updates Host singleton API to accept/store IDebuggerServices. |
| src/SOS/Strike/platform/hostimpl.cpp | Implements Host storing IDebuggerServices and using it to create the local Target. |
| src/SOS/Strike/platform/datatarget.h | Stores IDebuggerServices in DataTarget and adds destructor for releasing it. |
| src/SOS/Strike/platform/datatarget.cpp | Routes DataTarget operations (machine type, modules, memory, thread context, alloc/free) through IDebuggerServices/IRemoteMemoryService. |
| src/SOS/Strike/clrma/managedanalysis.cpp | Chooses CLRMA path separator based on ITarget::GetOperatingSystem() instead of command-scoped target checks. |
Suppressed comments (2)
src/SOS/Strike/platform/hostimpl.cpp:40
- Host::GetInstance can construct the singleton with a null debuggerServices pointer, which will later crash when Host uses it (and currently crashes immediately in Host::Host). Validate debuggerServices when creating the singleton and fail fast if it's missing.
IHost* Host::GetInstance(IDebuggerServices* debuggerServices)
{
if (s_host == nullptr)
{
s_host = new Host(debuggerServices);
}
s_host->AddRef();
return s_host;
}
src/SOS/Strike/platform/hostimpl.cpp:107
- Host::GetCurrentTarget returns S_OK even if Target::GetInstance fails and returns nullptr (e.g., if debugger services were not available when the Host was created). Return an error in that case so callers don't dereference a null target.
HRESULT Host::GetCurrentTarget(ITarget** ppTarget)
{
if (ppTarget == nullptr)
{
return E_INVALIDARG;
}
*ppTarget = Target::GetInstance(m_debuggerServices);
return S_OK;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
SwitchRuntime only asserted s_target, so -netfx/-netcore switching could fail when no command had created a target yet. Lazily create the local target using the session-lifetime debugger services, matching the prior contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The clrma command prints frames in dbgeng module-name form (SymbolTestApp!Type.Method+0xNN), not the DAC !clrstack form (SymbolTestApp.dll!Type.Method(sig) + N [source]). Match the clrma frame line so the native-fallback coverage passes across configs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
clrma with no argument reports the current thread's current exception, which is not stable at this script point: both the exception it picks and the module-name format it prints (Module.dll!Type vs dbgeng Module!Type) vary across runs. Assert only the provider line and OSThreadId, which require AssociateClient -> QueryDebugClient -> Target::GetOperatingSystem to run without faulting - the exact path that crashed - and are stable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
noahfalk
approved these changes
Jul 31, 2026
tommcdon
approved these changes
Jul 31, 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.
The exported
CLRMACreateInstanceentrypoint runs outside any SOS command, but the nativeTarget/Runtime/DataTarget/Hostfallback objects read the command-scopedg_Ext*globals, which are null outside a command.!analyzehit this and crashed inTarget::GetOperatingSystem.These objects now use the session-lifetime
IDebuggerServices(backed byDbgEngServices/LLDBServices) instead ofg_Ext*, so they are valid regardless of command scope. OS-dependent choices, including the CLRMA module-name separator, now come fromITarget::GetOperatingSystem().Rejected repopulating
g_Ext*for the exported path: it preserves the command-scoped-global coupling instead of fixing the lifetime mismatch.Behavior change:
DataTarget::GetMachineTypenow normalizesARM64ECtoAMD64(viaDbgEngServices::GetProcessorType), matching the rest of SOS; AMD64/ARM64 are unaffected.