Use ISOSDacInterface17 for DumpLog when available#5873
Merged
max-charlamb merged 1 commit intoJun 22, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates SOS !DumpLog to prefer a new cDAC-based stress log path via ISOSDacInterface17 when available, falling back to the existing raw-memory stress log dump for older runtimes. It also wires up unit tests to ensure stress log data is present in dumps and adds a script verification for DumpLog.
Changes:
- Add a new
DumpStressLogViaInterface17implementation instressLogDump.cppand call it first from!DumpLog. - Introduce
ISOSDacInterface17+ stress-log enumerator interfaces/structs insospriv.hand corresponding IIDs insospriv_i.cpp. - Enable stress logging for test debuggees and add
DumpLogverification toOtherCommands.script.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/SOS.UnitTests/SOSRunner.cs | Enables stress logging config for dump-generation test runs so DumpLog has data. |
| src/tests/SOS.UnitTests/Scripts/OtherCommands.script | Adds DumpLog invocation and verifies the success banner. |
| src/SOS/Strike/strike.cpp | Tries the new Interface17-based dump path first, then falls back to legacy StressLog::Dump. |
| src/SOS/Strike/stressLogDump.cpp | Implements the Interface17-based stress log dump and message merging logic. |
| src/shared/pal/prebuilt/inc/sospriv.h | Adds the new Interface17 and stress log enum interfaces + data structs. |
| src/shared/pal/prebuilt/idl/sospriv_i.cpp | Adds IIDs for the new interfaces. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
max-charlamb
force-pushed
the
maxcharlamb/stresslog-isosdac17
branch
from
3319aa6 to
cea50bc
Compare
noahfalk
previously approved these changes
Jun 12, 2026
max-charlamb
added a commit
to dotnet/runtime
that referenced
this pull request
Jun 19, 2026
> [!NOTE] > This PR was created with assistance from GitHub Copilot. ## Summary Add a new `ISOSDacInterface17` COM interface that exposes the cDAC's `IStressLog` contract to SOS and clrmd consumers. This enables reading stress logs from both CoreCLR and NativeAOT processes without hardcoded struct offsets. ## Motivation SOS (`!DumpLog`) and clrmd currently read stress logs via raw memory reads with hardcoded struct offsets matching the CoreCLR layout. NativeAOT's `StressLog` struct has a different layout (different lock type, no module table, no padding sentinel), so these tools cannot read NativeAOT stress logs. The cDAC already has a working `IStressLog` contract (`StressLog_1`/`StressLog_2`) that reads stress logs from both runtimes using `datadescriptor.inc` field offsets. This PR bridges that contract to the COM interface layer so SOS and clrmd can consume it. ## Changes ### cDAC contract updates - Add `StartTime` (wall-clock FILETIME) to data descriptors (CoreCLR + NativeAOT), `Data/StressLog.cs`, `StressLogData` record, and contract implementation - Add `Address` field to `ThreadStressLogData` for thread identification across the COM boundary - Add `StressLog` to `ContractRegistry` ### ISOSDacInterface17 definition - Data structs: `SOSStressLogData`, `SOSThreadStressLogData`, `SOSStressMsgData` (defined inline in IDL following modern convention) - Enumerator interfaces: `ISOSStressLogThreadEnum`, `ISOSStressLogMsgEnum` - `ISOSDacInterface17`: `GetStressLogData`, `GetStressLogThreadEnumerator`, `GetStressLogMessageEnumerator` - All APIs return `S_FALSE` when stress log is not enabled (no separate availability check needed) ### SOSDacImpl bridge - Thread enumerator: materialized array with try/catch COM boundary protection - Message enumerator: eagerly materialized with full `Reset`/`GetCount` support and `GetArguments` for per-batch arg retrieval - The native DAC does **not** implement this interface -- `QueryInterface` for the IID will fail on the legacy DAC, and only the cDAC handles it ### IDL + testing - Full IDL definitions in `sospriv.idl` - StressLog debuggee + dump-based integration tests validating the contract and the COM interface layer ## Test Results All dump tests pass: - `StressLogIsAvailable` -- PASSED - `StressLogDataIsValid` -- PASSED - `CanEnumerateThreadsAndMessages` -- PASSED ## Related PRs - **Diagnostics**: dotnet/diagnostics#5873 -- SOS `!DumpLog` updated to use `ISOSDacInterface17` with fallback - **CI run**: [runtime-diagnostics build 1458939](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1458939) --------- Co-authored-by: Max Charlamb <maxcharlamb@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
max-charlamb
force-pushed
the
maxcharlamb/stresslog-isosdac17
branch
3 times, most recently
from
4864ad2 to
d93970d
Compare
steveisok
previously approved these changes
Jun 22, 2026
max-charlamb
force-pushed
the
maxcharlamb/stresslog-isosdac17
branch
2 times, most recently
from
b3b0c61 to
89ddb26
Compare
Add StressLog::DumpViaInterface17 which queries for ISOSDacInterface17 to enumerate stress log threads and messages via the cDAC contract path. DumpLog tries this first, falling back to the legacy raw-read path if the interface is unavailable. A -legacy flag opts out explicitly. - Add ISOSDacInterface17, ISOSStressLogThreadEnum, ISOSStressLogMsgEnum and supporting SOS* data structs to the prebuilt sospriv headers - Fix legacy StressLog::Dump incorrectly returning a failure HRESULT when a format string ReadVirtual failed mid-loop, even though all entries were successfully processed - Gate stress log enablement in tests to only OtherCommands (DumpLog) - Add DumpLog CI test coverage with VERIFY Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
max-charlamb
force-pushed
the
maxcharlamb/stresslog-isosdac17
branch
from
89ddb26 to
5c793ec
Compare
rcj1
approved these changes
Jun 22, 2026
steveisok
approved these changes
Jun 22, 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.
Note
This PR was created with assistance from GitHub Copilot.
Summary
Update
!DumpLogto prefer the cDAC-basedISOSDacInterface17for stress log reading when available, falling back to the existing raw-memory path for older runtimes. This enables reading stress logs from both CoreCLR and NativeAOT processes.Companion to dotnet/runtime#129272 which defines
ISOSDacInterface17and implements the cDAC bridge.Changes
SOS
!DumpLogupdateStressLog::DumpViaInterface17()instressLogDump.cpp-- a new code path that:QueryInterfaceforISOSDacInterface17GetStressLogData,GetStressLogThreadEnumerator, andGetStressLogMessageEnumeratorto read stress log data through the cDAC contractformatOutputfor special format specifiers like%pM/%pT)E_NOINTERFACEif the interface is not available!DumpLoginstrike.cppto try the new path first, falling back toStressLog::Dump()ifE_NOINTERFACE-legacyflag to force the legacy raw-read pathInterface definitions
ISOSDacInterface17,ISOSStressLogThreadEnum,ISOSStressLogMsgEnuminterfaces and SOS* data structs tosospriv.hsospriv_i.cppBug fix in legacy
StressLog::DumpFixed a pre-existing bug where the legacy
StressLog::Dumpcould return a failure HRESULT even after successfully processing all stress log entries. The issue was thathrwas being overwritten byReadVirtualcalls for format strings inside the message loop. If the last format string read failed (e.g., unmapped memory page in the dump on macOS), the error propagated as the function's return value despite all messages being processed correctly. The fix avoids clobberinghrin the loop and explicitly sets it based on whether messages were processed.Test coverage
!DumpLogto theOtherCommands.scripttest withVERIFY:SUCCESSDOTNET_StressLog=1) to only the OtherCommands test viaEnableStressLogproperty onTestInformationBackward Compatibility
ISOSDacInterface17(Add ISOSDacInterface17 for StressLog enumeration via cDAC runtime#129272): uses the new cDAC-based path, which supports both CoreCLR and NativeAOTISOSDacInterface17:QueryInterfacereturnsE_NOINTERFACE, falls back to the existingStressLog::Dump()raw-memory path -- zero behavior changeRelated PRs
ISOSDacInterface17and implements the cDAC bridge