Update SOS for unified ISOSDacInterface18 GCInfo API - #5898
Conversation
leculver
left a comment
There was a problem hiding this comment.
This is a great start. I know it's a draft, but I had a few comments to help steer. Thanks!
491a060 to
26a0263
Compare
> [!NOTE] > This content was generated with AI assistance. ## ISOSDacInterface18 for GCInfo inspection APIs Add `ISOSDacInterface18` with 4 COM methods exposing GC info data through the cDAC for the SOS `!GCInfo` command: - `GetGCInfoHeader` -- header fields (version, code size, prolog, stack base register, GS cookie, PSP sym, generics context) - `GetGCInfoInterruptibleRanges` -- code regions where GC can interrupt execution - `GetGCInfoSafePoints` -- specific offsets with point-in-time GC slot liveness - `GetGCInfoSlotLifetimes` -- unified slot lifetime ranges (register + stack combined via `SOSGCSlotLifetime` with `IsRegister` discriminator) ### IGCInfo contract changes - Add `GetHeader`, `GetSafePoints`, `GetSlotLifetimes` to `IGCInfo`/`IGCInfoDecoder` - Consolidate `GetStackBaseRegister` and `GetSizeOfStackParameterArea` into `GCInfoHeader` (accessed via `GetHeader()`). These fields require the same decode depth (`DecodePoints.ReversePInvoke`) as the rest of the header, so there's no performance benefit to keeping them separate. - Keep `GetCodeLength` as a standalone method -- it only decodes to `DecodePoints.CodeLength`, which is significantly cheaper than the full header decode. Used on the hot path by `EEJitManager`/`InterpreterJitManager`/`ReadyToRunJitManager` for method size lookups. - Keep `GetCalleePoppedArgumentsSize` as a standalone method -- it returns 0 via a default interface method on non-x86 platforms without any decoding. Only x86 reads the `ArgCount` header field. - Eagerly decode safe points in `DecodeSafePoints`, stored denormalized - Single-pass chunk decoder for interruptible slot lifetimes (O(transitions), not O(codeLength)) - Safe-point-only methods emit `[safePointOffset, safePointOffset+1)` per live slot per safe point - Implement x86 GCInfo support (`GetHeader`/`GetSafePoints`/`GetSlotLifetimes`) - Add `GenericsContextKind.None` as default enum value - Fix `GSCookieValidRange` -- only set when GSCookie is present - Native DAC returns `E_NOINTERFACE` for `ISOSDacInterface18` (cDAC-only interface) ### Bug fix: ARM thumb bit in relative offset computation `EEJitManager.GetMethodInfo` and `InterpreterJitManager.GetMethodInfo` computed `relativeOffset` as `jittedCodeAddress.Value - codeStart.Value`. On ARM (Thumb-2), `TargetCodePointer.Value` includes the thumb bit (bit 0 = 1), but `codeStart` (a `TargetPointer` from the nibble map) does not. This produced a relative offset that was off by 1, causing `FindSafePoint` and `EnumerateLiveSlots` to look up the wrong offset. Fixed by stripping the thumb bit via `CodePointerUtils.AddressFromCodePointer` before computing the difference. `ReadyToRunJitManager` was already doing this correctly. ### Companion PR - dotnet/diagnostics#5898 -- SOS `!GCInfo` command updated to use `ISOSDacInterface18` with legacy-compatible timeline output format --------- Co-authored-by: Max Charlamb <[email protected]> Co-authored-by: Copilot <[email protected]>
> [!NOTE] > This content was generated with AI assistance. ## ISOSDacInterface18 for GCInfo inspection APIs Add `ISOSDacInterface18` with 4 COM methods exposing GC info data through the cDAC for the SOS `!GCInfo` command: - `GetGCInfoHeader` -- header fields (version, code size, prolog, stack base register, GS cookie, PSP sym, generics context) - `GetGCInfoInterruptibleRanges` -- code regions where GC can interrupt execution - `GetGCInfoSafePoints` -- specific offsets with point-in-time GC slot liveness - `GetGCInfoSlotLifetimes` -- unified slot lifetime ranges (register + stack combined via `SOSGCSlotLifetime` with `IsRegister` discriminator) ### IGCInfo contract changes - Add `GetHeader`, `GetSafePoints`, `GetSlotLifetimes` to `IGCInfo`/`IGCInfoDecoder` - Consolidate `GetStackBaseRegister` and `GetSizeOfStackParameterArea` into `GCInfoHeader` (accessed via `GetHeader()`). These fields require the same decode depth (`DecodePoints.ReversePInvoke`) as the rest of the header, so there's no performance benefit to keeping them separate. - Keep `GetCodeLength` as a standalone method -- it only decodes to `DecodePoints.CodeLength`, which is significantly cheaper than the full header decode. Used on the hot path by `EEJitManager`/`InterpreterJitManager`/`ReadyToRunJitManager` for method size lookups. - Keep `GetCalleePoppedArgumentsSize` as a standalone method -- it returns 0 via a default interface method on non-x86 platforms without any decoding. Only x86 reads the `ArgCount` header field. - Eagerly decode safe points in `DecodeSafePoints`, stored denormalized - Single-pass chunk decoder for interruptible slot lifetimes (O(transitions), not O(codeLength)) - Safe-point-only methods emit `[safePointOffset, safePointOffset+1)` per live slot per safe point - Implement x86 GCInfo support (`GetHeader`/`GetSafePoints`/`GetSlotLifetimes`) - Add `GenericsContextKind.None` as default enum value - Fix `GSCookieValidRange` -- only set when GSCookie is present - Native DAC returns `E_NOINTERFACE` for `ISOSDacInterface18` (cDAC-only interface) ### Bug fix: ARM thumb bit in relative offset computation `EEJitManager.GetMethodInfo` and `InterpreterJitManager.GetMethodInfo` computed `relativeOffset` as `jittedCodeAddress.Value - codeStart.Value`. On ARM (Thumb-2), `TargetCodePointer.Value` includes the thumb bit (bit 0 = 1), but `codeStart` (a `TargetPointer` from the nibble map) does not. This produced a relative offset that was off by 1, causing `FindSafePoint` and `EnumerateLiveSlots` to look up the wrong offset. Fixed by stripping the thumb bit via `CodePointerUtils.AddressFromCodePointer` before computing the difference. `ReadyToRunJitManager` was already doing this correctly. ### Companion PR - dotnet/diagnostics#5898 -- SOS `!GCInfo` command updated to use `ISOSDacInterface18` with legacy-compatible timeline output format --------- Co-authored-by: Max Charlamb <[email protected]> Co-authored-by: Copilot <[email protected]>
3a2da7b to
f88333c
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates SOS !GCInfo to prefer the unified ISOSDacInterface18-based GCInfo API (cDAC path) and introduces a new GCInfoData provider to format the output (including “timeline” style entries), with fallbacks to the legacy raw GC-info blob decoding for older runtimes. It also updates the SOS private interfaces/IDL to add ISOSDacInterface18 and related data structs, and refreshes repo dependency versions to a newer .NET 11 RC build.
Changes:
- Add
GCInfoDataabstraction and use it from!GCInfoand from disassembly’s-gcinfopath before falling back to legacy decoding. - Extend SOS private interface definitions with
ISOSDacInterface18and GCInfo-related structs. - Add a unit test script check for
!GCInfooutput and update dependency versions to .NET 11 RC build numbers.
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/OtherCommands.script | Adds a basic !GCInfo invocation/verification to unit test scripts. |
| src/SOS/Strike/strike.cpp | Switches !GCInfo and disassembly GC-info capture to prefer GCInfoData (Interface18) with legacy fallback; adds -legacy option. |
| src/SOS/Strike/gcinfoprovider.h | Declares GCInfoData container/formatter for Interface18-derived GCInfo. |
| src/SOS/Strike/gcinfoprovider.cpp | Implements Interface18 queries and formats GCInfo output. |
| src/SOS/Strike/CMakeLists.txt | Adds gcinfoprovider.cpp to SOS build inputs. |
| src/shared/pal/prebuilt/inc/sospriv.h | Updates prebuilt SOS private header with Interface18 + supporting structs and minor enum/IDL regen deltas. |
| src/shared/pal/prebuilt/idl/sospriv_i.cpp | Adds IID definition for ISOSDacInterface18. |
| src/shared/inc/sospriv.idl | Adds ISOSDacInterface16/17/18 and GCInfo struct definitions to the IDL. |
| eng/Version.Details.xml | Updates dotnet/dotnet dependency versions/SHA to an RC build. |
| eng/Version.Details.props | Updates derived dependency version properties to the RC build. |
Suppressed comments (1)
src/SOS/Strike/gcinfoprovider.cpp:202
- This comment is inaccurate: the formatting emitted by GCInfoData::DumpToOutput does not match the legacy GcInfoDumper output exactly (e.g., legacy prints slot flags like "(pinned)/(interior)/(untracked)" and uses "caller.sp" for caller-SP-relative slots). Either update the output to match legacy, or soften/remove the "exactly" claim here.
// Display format depends on whether the method is fully interruptible or safe-point-only.
// This matches the legacy GcInfoDumper output exactly.
bool isInterruptible = !InterruptibleRanges.empty();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
fyi @hoyosjs |
Co-authored-by: Copilot <[email protected]> Copilot-Session: 3ebf2a2d-da27-41b5-9f45-4558240207ec
c157b79 to
b704927
Compare
Co-authored-by: Copilot <[email protected]> Copilot-Session: 3ebf2a2d-da27-41b5-9f45-4558240207ec
Co-authored-by: Copilot <[email protected]> Copilot-Session: 3ebf2a2d-da27-41b5-9f45-4558240207ec
Co-authored-by: Copilot <[email protected]> Copilot-Session: 3ebf2a2d-da27-41b5-9f45-4558240207ec
Note
This content was generated with AI assistance.
Update SOS !GCInfo command to use unified ISOSDacInterface18 API with timeline output format.
Depends on: dotnet/runtime#129761