Skip to content

[cDAC] Implement DacDbi APIs ParseContinuation and GetAsyncLocals#129147

Merged
rcj1 merged 6 commits into
mainfrom
copilot/cdac-parse-continuation-get-async-locals
Jun 22, 2026
Merged

[cDAC] Implement DacDbi APIs ParseContinuation and GetAsyncLocals#129147
rcj1 merged 6 commits into
mainfrom
copilot/cdac-parse-continuation-get-async-locals

Conversation

@rcj1

@rcj1 rcj1 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
  • Add cDAC API to read continuation
  • Add cDAC API to read async debug info
  • Implement the DacDbi APIs using those

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 9, 2026 01:03

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 extends the cDAC (DataContractReader) stack to expose continuation-object data and async debug-info data, then wires those into the legacy DacDbi surface by implementing ParseContinuation and replacing the old “return a list” async-locals API with a callback-based enumeration API.

Changes:

  • Add new cDAC Object contract support for reading ContinuationObject + AsyncResumeInfo and expose GetContinuationInfo.
  • Add DebugInfo contract support for decoding the AsyncInfo chunk into suspension points + captured locals.
  • Update the DacDbi interface and implementations to use ParseContinuation and EnumerateAsyncLocals (callback-based), and update the RS consumer to accumulate results.
Show a summary per file
File Description
src/native/managed/cdac/tests/ObjectTests.cs Adds unit tests for IObject.GetContinuationInfo behavior (resume info present/absent).
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.cs Adds mock layouts + builders for ContinuationObject and AsyncResumeInfo test objects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Introduces AsyncLocalData and switches from GetAsyncLocals to callback-based EnumerateAsyncLocals.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Implements ParseContinuation and EnumerateAsyncLocals using cDAC contracts, with DEBUG cross-checking vs legacy DAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs Adds AsyncResumeInfo data type.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ContinuationObject.cs Defines cDAC data descriptor-backed reader for ContinuationObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/AsyncResumeInfo.cs Defines cDAC data descriptor-backed reader for AsyncResumeInfo (DiagnosticIP).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs Implements GetContinuationInfo in the Object contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfoHelpers.cs Adds decoding for async suspension points + locals from the AsyncInfo chunk.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfo_2.cs Exposes IDebugInfo.GetAsyncSuspensionPoints by decoding the AsyncInfo chunk.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.cs Adds ContinuationInfo + GetContinuationInfo to the Object contract surface.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IDebugInfo.cs Adds public async suspension/local data structures and GetAsyncSuspensionPoints.
src/coreclr/vm/object.h Adds cdac_data<ContinuationObject> offsets for descriptor generation.
src/coreclr/vm/datadescriptor/datadescriptor.inc Adds descriptors for ContinuationObject fields and new AsyncResumeInfo type (DiagnosticIP).
src/coreclr/inc/dacdbi.idl Updates COM IDL to replace GetAsyncLocals with EnumerateAsyncLocals callback form.
src/coreclr/debug/inc/dacdbiinterface.h Updates the native interface contract docs and signature for EnumerateAsyncLocals.
src/coreclr/debug/di/rsthread.cpp Updates RS to call EnumerateAsyncLocals and accumulate results into a quick array list.
src/coreclr/debug/di/rspriv.h Switches CordbAsyncFrame async locals storage to CQuickArrayList<AsyncLocalData>.
src/coreclr/debug/daccess/dacdbiimpl.h Renames GetAsyncLocals to EnumerateAsyncLocals in the DAC implementation declaration.
src/coreclr/debug/daccess/dacdbiimpl.cpp Implements callback-based EnumerateAsyncLocals in the DAC implementation.
docs/design/datacontracts/Object.md Documents the new GetContinuationInfo contract API and involved descriptors.
docs/design/datacontracts/DebugInfo.md Documents async suspension point APIs and the AsyncInfo encoding format.

Copilot's findings

Comments suppressed due to low confidence (2)

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

  • EnumerateAsyncLocals compares state by casting to int. For values >= 0x8000_0000 this becomes negative, the (int)state < suspensionPoints.Count check passes, and the subsequent index uses a negative value, causing an exception (and an unintended HRESULT) instead of the intended "out of range => S_OK with no callbacks" behavior. The native DAC implementation treats state as an unsigned index (state >= NumSuspensionPoints).
        try
        {
            if (vmObject == 0)
            {

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

  • ParseContinuation doesn't initialize the output pointers on the success path until after reading target state. If an exception is thrown after the null checks (e.g., invalid address / read failure), the method returns a failing HRESULT but leaves the out values uninitialized. Other methods in this file zero-init outputs early (e.g., GetAssemblyFromModule) to avoid callers observing garbage.
#if DEBUG
        if (_legacy is not null)
        {
            ulong resultLocal;
            int hrLocal = _legacy.GetNativeCodeVersionNode(vmMethod, codeStartAddress, &resultLocal);
  • Files reviewed: 22/22 changed files
  • Comments generated: 1

@github-actions

This comment has been minimized.

@noahfalk noahfalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@rcj1 rcj1 self-assigned this Jun 22, 2026
Copilot AI review requested due to automatic review settings June 22, 2026 16:48

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.

Copilot's findings

  • Files reviewed: 22/22 changed files
  • Comments generated: 5

Comment thread docs/design/datacontracts/Object.md
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 22, 2026 16:59
@github-actions

Copy link
Copy Markdown
Contributor

Copilot Code Review

Holistic Assessment

Motivation: This PR fills a clear gap in the cDAC async debugging pipeline by implementing ParseContinuation (Object contract) and GetAsyncSuspensionPoints (DebugInfo contract), then wiring them into the DacDbi layer via callback-based EnumerateAsyncLocals. The work is well-motivated and necessary for runtime-async diagnostics.

Approach: The approach follows established cDAC patterns throughout — data descriptors in datadescriptor.inc, IData<T> types, contract implementations, DacDbi callback enumeration, and #if DEBUG cross-validation against the legacy DAC. The GetAsyncLocalsEnumerateAsyncLocals rename to a callback pattern is consistent with existing DacDbi enumeration APIs (EnumerateTypeHandleParams, field enumeration, etc.).

Summary: ⚠️ Needs Human Review. The implementation is well-structured and my analysis shows the encoding/decoding logic matches the native runtime. I have two items I'd like a human to verify: (1) the async-info nibble encoding semantics (format-sensitive code that I verified by comparing against debuginfostore.cpp, but a human eye on the delta-to-uint accumulation would be valuable), and (2) whether dedicated unit tests for DoAsyncInfo should be added before merge.


Detailed Findings

Detailed Findings

✅ Correctness — Encoding/decoding matches native runtime

I compared DebugInfoHelpers.DoAsyncInfo against the native RestoreAsyncDebugInfo / DoAsyncSuspensionPoints / DoAsyncVars in debuginfostore.cpp and confirmed the following correspondences:

cDAC (DoAsyncInfo) Native (CompressDebugInfo)
ReadUInt() for NumSuspensionPoints ReadEncodedU32
ReadUInt() for total var count (discarded) ReadEncodedU32*pNumAsyncVars (used for allocation)
ReadInt() for signed delta DoEncodedDeltaU32NonMonotonicReadEncodedI32
ReadUInt() + MAX_ILNUM for var number DoEncodedAdjustedU32(var->VarNumber, MAX_ILNUM)
ReadUInt() for offset DoEncodedU32

The MAX_ILNUM constant (unchecked((uint)-6)) matches ICorDebugInfo::MAX_ILNUM. The total var count is correctly discarded since the per-suspension-point counts fully partition the flat var list.

✅ Object contract — GetContinuationInfo correctly reads continuation fields

Object_1.GetContinuationInfo reads Next, ResumeInfo, and State from the ContinuationObject and conditionally reads DiagnosticIP from AsyncResumeInfo when ResumeInfo is non-null. The data descriptors use offsetof against the native ContinuationObject class and CORINFO_AsyncResumeInfo struct, which is correct. The CDAC_TYPE_INDETERMINATE for AsyncResumeInfo is appropriate since only the field offset is needed.

✅ DacDbi interface change — Callback pattern properly implemented

The rename from GetAsyncLocals (allocating into DacDbiArrayList) to EnumerateAsyncLocals (callback-based) follows the established CallbackAccumulator<T> pattern. The consumer in rsthread.cpp correctly accumulates via CallbackAccumulator<AsyncLocalData>::PushCallback, checks acc.hrError for allocation failures, and copies into CQuickArrayList. The m_nNumberOfVars field was correctly removed (it had no remaining uses) and DacDbiArrayList<AsyncLocalData> was replaced with CQuickArrayList<AsyncLocalData>, with all .Count() calls updated to .Size().

✅ Data descriptors — Properly defined

datadescriptor.inc adds ContinuationObject fields (Next, ResumeInfo, State) using cdac_data<ContinuationObject> offsets and AsyncResumeInfo::DiagnosticIP using offsetof(CORINFO_AsyncResumeInfo, DiagnosticIP). The cdac_data<ContinuationObject> template specialization in object.h correctly declares the offsets with friend access.

✅ DEBUG validation — Thorough cross-validation

Both ParseContinuation and EnumerateAsyncLocals in DacDbiImpl.cs include #if DEBUG blocks that compare every output field between the cDAC and legacy DAC implementations, including HRESULT comparison via Debug.ValidateHResult. The EnumerateAsyncLocals validation uses a [ThreadStatic] list with [UnmanagedCallersOnly] callback to collect legacy results, which is a clean approach.

✅ Unit tests — Good coverage for Object contract

Three test cases (GetContinuationInfo_WithResumeInfo, GetContinuationInfo_NullResumeInfo, GetContinuationInfo_NullDiagnosticIP) cover the key scenarios. The mock data structures (MockContinuationObjectData, MockAsyncResumeInfoData) correctly model the native field layouts including the Flags field between ResumeInfo and State.

✅ Previous review issues addressed

The two follow-up commits correctly fixed:

  1. Missing closing brace in Object.md pseudocode (commit 6c70183)
  2. Signed-to-unsigned cast: (int)state < suspensionPoints.Countstate < (uint)suspensionPoints.Count (commit 49d81c7), which prevents underflow for state > int.MaxValue

💡 Observation — No dedicated unit tests for DoAsyncInfo decoder

The DebugInfoHelpers.DoAsyncInfo encoding/decoding path has no targeted unit test with known nibble-encoded data. The Object contract has tests, and the #if DEBUG cross-validation provides integration-level coverage, but a unit test exercising the nibble reader directly would provide stronger coverage for this format-sensitive code (e.g., edge cases with 0 suspension points, non-monotonic offset deltas, mixed var counts). This is non-blocking — the DEBUG validation and native decoder matching provide reasonable confidence.

✅ Contract versioning — Default implementation is correct

IDebugInfo.GetAsyncSuspensionPoints has a default implementation returning Array.Empty<AsyncSuspensionInfo>(), so DebugInfo_1 (which doesn't support async info) correctly returns an empty list without needing modification.

✅ Documentation — Clear and accurate

The additions to DebugInfo.md and Object.md accurately document the new API types, encoding format, and pseudocode. The encoding documentation faithfully describes the nibble layout.

Note

This review was generated by GitHub Copilot.

Generated by Code Review for issue #129147 · ● 72.6M ·

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.

Copilot's findings

  • Files reviewed: 22/22 changed files
  • Comments generated: 3

@rcj1

rcj1 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 22, 2026 17:58
@rcj1 rcj1 removed the request for review from Copilot June 22, 2026 17:58

Copilot AI commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@copilot please fix the unit test issues: https://github.com/dotnet/runtime/pull/129147/checks?check_run_id=82776061889

Fixed in 49d81c7. The CdacUnitTests failure was a duplicate ContinuationObject key: after the latest main merge, MethodTableTests.CreateContractTypes now adds a (size-only) DataType.ContinuationObject entry, which collided with the entry ObjectTests.CreateContractTypes added via .Concat(...).ToDictionary(). I restructured ObjectTests.CreateContractTypes to start from the MethodTableTests dictionary and override with the Object-specific full-field layouts (so the ContinuationObject layout with field offsets wins). All 2583 cdac unit tests pass locally (16 skipped, 0 failed).

Note

This comment was generated by AI (GitHub Copilot).

Copilot finished work on behalf of rcj1 June 22, 2026 17:59
@rcj1

rcj1 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

/ba-g x86 is down

@rcj1 rcj1 merged commit bed6b9b into main Jun 22, 2026
129 of 138 checks passed
@rcj1 rcj1 deleted the copilot/cdac-parse-continuation-get-async-locals branch June 22, 2026 20:55
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone Jun 23, 2026
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