Skip to content

Use ISOSDacInterface17 for DumpLog when available#5873

Open
max-charlamb wants to merge 1 commit into
dotnet:mainfrom
max-charlamb:maxcharlamb/stresslog-isosdac17
Open

Use ISOSDacInterface17 for DumpLog when available#5873
max-charlamb wants to merge 1 commit into
dotnet:mainfrom
max-charlamb:maxcharlamb/stresslog-isosdac17

Conversation

@max-charlamb

@max-charlamb max-charlamb commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Note

This PR was created with assistance from GitHub Copilot.

Summary

Update !DumpLog to prefer the cDAC-based ISOSDacInterface17 for 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 ISOSDacInterface17 and implements the cDAC bridge.

Changes

SOS !DumpLog update

  • Added DumpStressLogViaInterface17() in stressLogDump.cpp -- a new code path that:
    • QueryInterface for ISOSDacInterface17
    • If available: uses GetStressLogData, GetStressLogThreadEnumerator, and GetStressLogMessageEnumerator to read stress log data through the cDAC contract
    • Produces identical output format to the legacy path (header block, merged timestamp-ordered messages, formatOutput for special format specifiers like %pM/%pT)
    • Returns E_NOINTERFACE if the interface is not available
  • Updated !DumpLog in strike.cpp to try the new path first, falling back to StressLog::Dump() if E_NOINTERFACE

Interface definitions

  • Added ISOSDacInterface17, ISOSStressLogThreadEnum, ISOSStressLogMsgEnum, ISOSStressLogMemoryEnum interfaces and data structs to sospriv.h
  • Added IID GUIDs to sospriv_i.cpp

Testing

  • Enabled stress logging environment variables (DOTNET_StressLog=1, etc.) for all CreateDump debuggees so stress log data is present in dumps
  • Added DumpLog command + verification to OtherCommands.script

Backward Compatibility

  • When running against a runtime with ISOSDacInterface17 (Add ISOSDacInterface17 for StressLog enumeration via cDAC runtime#129272): uses the new cDAC-based path, which supports both CoreCLR and NativeAOT
  • When running against a runtime without ISOSDacInterface17: QueryInterface returns E_NOINTERFACE, falls back to the existing StressLog::Dump() raw-memory path -- zero behavior change

Related PRs

@max-charlamb max-charlamb requested a review from a team as a code owner June 11, 2026 02:55
Copilot AI review requested due to automatic review settings June 11, 2026 02:55
@max-charlamb max-charlamb mentioned this pull request Jun 11, 2026
Merged
Copilot AI reviewed Jun 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 DumpStressLogViaInterface17 implementation in stressLogDump.cpp and call it first from !DumpLog.
  • Introduce ISOSDacInterface17 + stress-log enumerator interfaces/structs in sospriv.h and corresponding IIDs in sospriv_i.cpp.
  • Enable stress logging for test debuggees and add DumpLog verification to OtherCommands.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.

Comment thread src/SOS/Strike/stressLogDump.cpp Outdated
Comment thread src/SOS/Strike/stressLogDump.cpp Outdated
Comment thread src/SOS/Strike/stressLogDump.cpp Outdated
Comment thread src/SOS/Strike/stressLogDump.cpp Outdated
Comment thread src/SOS/Strike/stressLogDump.cpp
Comment thread src/SOS/Strike/stressLogDump.cpp
Comment thread src/SOS/Strike/stressLogDump.cpp
Comment thread src/SOS/Strike/stressLogDump.cpp Outdated
Comment thread src/SOS/Strike/stressLogDump.cpp Outdated
Comment thread src/tests/SOS.UnitTests/SOSRunner.cs Outdated
@max-charlamb max-charlamb force-pushed the maxcharlamb/stresslog-isosdac17 branch from 3319aa6 to cea50bc Compare June 11, 2026 03:16
@max-charlamb max-charlamb mentioned this pull request Jun 11, 2026
Closed
noahfalk
noahfalk previously approved these changes Jun 12, 2026
View reviewed changes
max-charlamb added a commit to dotnet/runtime that referenced this pull request Jun 19, 2026
@max-charlamb
Add ISOSDacInterface17 for StressLog enumeration via cDAC (#129272)
ecbe39a 
> [!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 max-charlamb marked this pull request as draft June 21, 2026 22:36
@max-charlamb max-charlamb marked this pull request as ready for review June 21, 2026 22:37
@max-charlamb max-charlamb force-pushed the maxcharlamb/stresslog-isosdac17 branch from cea50bc to 006232e Compare June 22, 2026 02:05
Use ISOSDacInterface17 for DumpLog when available
4864ad2 
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
- Gate stress log enablement in tests to only OtherCommands (DumpLog)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb max-charlamb force-pushed the maxcharlamb/stresslog-isosdac17 branch from 006232e to 4864ad2 Compare June 22, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@noahfalk noahfalk noahfalk left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@max-charlamb @noahfalk