Skip to content

[cDAC] Implement SetDebugState for cDAC#127778

Merged
barosiak merged 8 commits into
dotnet:mainfrom
barosiak:barosiak/SetDebugState
May 13, 2026
Merged

[cDAC] Implement SetDebugState for cDAC#127778
barosiak merged 8 commits into
dotnet:mainfrom
barosiak:barosiak/SetDebugState

Conversation

@barosiak
Copy link
Copy Markdown
Member

@barosiak barosiak commented May 5, 2026

Summary

Implement SetDebugState in the cDAC. Separates debugger-controlled thread state from ThreadStateNoConcurrency into a new Volatile field to avoid read-modify-write races, and exposes it through the Thread contract.

Changes

  • Added DebuggerControlledThreadState enum with Volatile<> to native Thread class
  • Migrated TSNC_DebuggerUserSuspend to the new enum
  • Exposed through data descriptor and managed contract
  • Implemented DacDbiImpl.SetDebugState with #if DEBUG legacy validation
  • Updated Thread.md contract documentation
  • Added parameterized tests for set/reset/idempotency

Test Results

16 new test runs across all architectures. Existing tests pass.

@barosiak barosiak requested review from max-charlamb, noahfalk and rcj1 May 5, 2026 00:10
@barosiak barosiak self-assigned this May 5, 2026
Copilot AI review requested due to automatic review settings May 5, 2026 00:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Implements cDAC-side support for debugger thread state updates by exposing Thread::m_StateNC through the Thread contract and wiring DacDbiImpl.SetDebugState to set/reset the debugger-suspend bit, with matching tests and contract docs.

Changes:

  • Added StateNC to the native and managed thread data descriptors and documented the new contract dependency.
  • Added ThreadStateNoConcurrency plus SetThreadState / ResetThreadState on the cDAC IThread contract and used them from DacDbiImpl.SetDebugState.
  • Added contract-level unit tests for set/reset/idempotent behavior and updated Thread contract documentation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/native/managed/cdac/tests/ThreadTests.cs Adds contract-level tests for setting/resetting StateNC.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs Extends mock thread layout with StateNC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Adds managed projection enum for debug thread state values.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Implements SetDebugState via the Thread contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Thread.cs Adds StateNC member for descriptor name binding.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs Implements SetThreadState / ResetThreadState and reuses cached thread type info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IThread.cs Expands the public Thread contract with the new enum and mutating methods.
src/coreclr/vm/threads.h Annotates native enum values as contract-dependent and exports StateNC offset.
src/coreclr/vm/datadescriptor/datadescriptor.inc Publishes Thread.StateNC in the cDAC data descriptor.
docs/design/datacontracts/Thread.md Documents the new enum, APIs, and descriptor dependency.
Comments suppressed due to low confidence (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs:706

  • SetDebugState adds new DacDbi behavior, but the PR only adds contract-level ThreadTests. This leaves the DacDbi layer untested for the behaviors this method owns (notably HRESULT mapping for invalid states and the end-to-end state change through the legacy-facing API). The cDAC test guidance in src/native/managed/cdac/tests/README.md:21-32 calls for API-implementation-level tests when adding a new wrapper method backed by an existing contract.
            Contracts.ThreadData threadData = _target.Contracts.Thread.GetThreadData(new TargetPointer(vmThread));
            if ((threadData.State & (Contracts.ThreadState.Stopped | Contracts.ThreadState.Unstarted | Contracts.ThreadState.Detached)) != 0)
                throw Marshal.GetExceptionForHR(CorDbgHResults.CORDBG_E_BAD_THREAD_STATE)!;

            *pRetVal = threadData.ExposedObjectHandle.Value;
        }
        catch (System.Exception ex)
        {
            hr = ex.HResult;
        }
#if DEBUG
        if (_legacy is not null)
        {
            ulong retValLocal;
            int hrLocal = _legacy.GetThreadObject(vmThread, &retValLocal);
            Debug.ValidateHResult(hr, hrLocal);
            if (hr == HResults.S_OK)
                Debug.Assert(*pRetVal == retValLocal, $"cDAC: {*pRetVal:x}, DAC: {retValLocal:x}");

Comment thread docs/design/datacontracts/Thread.md Outdated
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot AI review requested due to automatic review settings May 11, 2026 21:16
@barosiak barosiak requested a review from jkotas May 11, 2026 21:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Comment thread src/coreclr/vm/threads.h Outdated
Comment thread docs/design/datacontracts/Thread.md
Comment thread src/coreclr/vm/threads.h Outdated
Copilot AI review requested due to automatic review settings May 12, 2026 21:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Comment thread src/coreclr/debug/daccess/dacdbiimpl.cpp Outdated
@barosiak barosiak enabled auto-merge (squash) May 12, 2026 21:36
Copilot AI review requested due to automatic review settings May 12, 2026 23:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

@barosiak barosiak merged commit 5f319f4 into dotnet:main May 13, 2026
121 of 126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants