[cDAC] Implement DacDbi API GetUserState#130208
Open
rcj1 wants to merge 1 commit into
Open
Conversation
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
rcj1
changed the title
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the cDAC DacDbi GetUserState API by composing existing thread/stack-walk contracts with a new GC-safepoint query, and adds cross-validation coverage in dump tests.
Changes:
- Implement
DacDbiImpl.GetUserStateby combining existing partial user state with a new “GC-unsafe point” bit derived from the thread IP. - Add
IsGcSafesupport to the ExecutionManager + GCInfo contract layers (including x86 and the general GCInfo decoder), and document the new APIs. - Add a dump test that cross-validates
GetUserStateagainst the contracts’ computed GC-safepoint status.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiThreadDumpTests.cs | Adds dump-based cross-validation test for GetUserState vs contract-derived GC-safepoint state. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs | Adds CorDebugUserState.USER_UNSAFE_POINT flag used by GetUserState. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs | Implements GetUserState using contracts + stack context IP GC-safety check. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/X86/GCInfo.cs | Adds x86 IsGcSafe implementation and lazy decode of no-GC regions. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/IGCInfoDecoder.cs | Extends internal decoder interface with IsGcSafe. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoX86_1.cs | Wires IsGcSafe through the x86 IGCInfo implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoDecoder.cs | Implements IsGcSafe for the general GCInfo decoder path. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfo_1.cs | Wires IsGcSafe through the generic IGCInfo implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs | Adds IExecutionManager.IsGcSafe using CodeBlock resolution + GCInfo decode. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs | Exposes IsGcSafe via contract version 1 wrapper. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs | Exposes IsGcSafe via contract version 2 wrapper. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IGCInfo.cs | Adds new public IGCInfo.IsGcSafe API. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IExecutionManager.cs | Adds new public IExecutionManager.IsGcSafe API. |
| docs/design/datacontracts/GCInfo.md | Documents IGCInfo.IsGcSafe. |
| docs/design/datacontracts/ExecutionManager.md | Documents IExecutionManager.IsGcSafe. |
Copilot's findings
- Files reviewed: 15/15 changed files
- Comments generated: 4
max-charlamb
approved these changes
Jul 6, 2026
| public int GetUserState(ulong vmThread, int* pRetVal) | ||
| => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetUserState(vmThread, pRetVal) : HResults.E_NOTIMPL; | ||
| { | ||
| *pRetVal = default; |
Member
There was a problem hiding this comment.
This write should be in the try catch to handle NRE
| IPlatformAgnosticContext context = IPlatformAgnosticContext.GetContextForPlatform(_target); | ||
| byte[] contextBytes = _target.Contracts.StackWalk.GetContext(threadData, ThreadContextSource.Debugger, context.FullContextFlags); | ||
| context.FillFromBuffer(contextBytes); | ||
| if (!_target.Contracts.ExecutionManager.IsGcSafe(context.InstructionPointer)) |
Member
There was a problem hiding this comment.
This misses some logic that the native has. See:
runtime/src/coreclr/debug/daccess/dacdbiimpl.cpp
Lines 5314 to 5321 in 22af16a
Is this intended?
Contributor
Author
There was a problem hiding this comment.
What in particular am I missing here?
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.
Implement this API with the use of GC-safety APIs on ExecutionManager (as a relatively thin wrapper) and GCInfo contracts.