[cDAC]: Implement GetTypedByRefInfo, GetStringData, GetArrayData, GetBasicObjectInfo by rcj1 · Pull Request #129200 · dotnet/runtime

rcj1 · 2026-06-09T23:30:05Z

Add two cDAC APIs on Object contract:

voidGetStringData(TargetPointeraddress,outuintlength,outuintoffsetToFirstChar);ulongGetSize(TargetPointeraddress);

Add one cDAC API on RuntimeTypeSystem:

TypedByRefInfoGetTypedByRefInfo(TargetPointertypedByRef);

Implement DacDbi APIs, slightly modifying out params to be more specific to each API
Moving, renaming, and re-typing DebuggerIPCE_ObjectData
Add unit tests for GetStringData and GetSize to ObjectTests.cs, covering string length/offset and both fixed-size object and variable-size array sizing across architectures

…sicObjectInfo (managed side) Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>

Copilot

Pull request overview

This PR extends the cDAC contracts and legacy DAC/DBI bridge to support additional object/typed-by-ref inspection APIs, and updates the CoreCLR DAC/DBI interface + right-side debugger plumbing to use more specific out-parameters (and a new object-data structure) for string/array/basic object queries.

Changes:

Add TypedByRef data descriptor + IRuntimeTypeSystem.GetTypedByRefInfo(...) (with unit tests).
Add IObject.GetStringData(...) and IObject.GetSize(...), and implement corresponding DAC/DBI entrypoints (GetStringData, GetArrayData, GetBasicObjectInfo, GetTypedByRefInfo) in the managed legacy bridge.
Refactor debugger-side object data plumbing: replace DebuggerIPCE_ObjectData with DacDbiObjectData and update IDL/interface declarations and DI callers accordingly.

Show a summary per file

File	Description
src/native/managed/cdac/tests/UnitTests/MethodTableTests.cs	Adds unit tests validating `GetTypedByRefInfo` reads data/type fields correctly.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs	Updates managed COM projection signatures and introduces `DacDbiArrayInfo`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs	Implements the updated DAC/DBI APIs using cDAC contracts, with DEBUG cross-checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs	Adds `DataType.TypedByRef` descriptor name.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/TypedByRef.cs	New data descriptor for `TypedByRef` (Data + Type fields).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs	Implements `GetTypedByRefInfo` in the RuntimeTypeSystem contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs	Adds `GetStringData` and `GetSize` implementations in the Object contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs	Adds public `TypedByRefInfo` and `GetTypedByRefInfo` API surface.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.cs	Adds public `GetSize` and `GetStringData` API surface.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Adds `TypedByRef` to the CoreCLR cDAC data descriptor list.
src/coreclr/inc/dacdbi.idl	Updates IDL to the new out-parameter shapes and struct names.
src/coreclr/debug/inc/dbgipcevents.h	Removes `DebuggerIPCE_ObjectData` definition.
src/coreclr/debug/inc/dacdbistructures.h	Introduces `DacDbiArrayInfo` and `DacDbiObjectData` structures.
src/coreclr/debug/inc/dacdbiinterface.h	Updates DAC/DBI interface method signatures and associated comments.
src/coreclr/debug/di/rspriv.h	Switches DI-side storage/parameters from `DebuggerIPCE_ObjectData` to `DacDbiObjectData`.
src/coreclr/debug/di/process.cpp	Updates `GetObjectInternal` to use the new `GetBasicObjectInfo` signature.
src/coreclr/debug/di/divalue.cpp	Updates reference/object data initialization to call new DAC/DBI signatures for string/array/typed-byref.
src/coreclr/debug/daccess/dacdbiimpl.h	Updates DAC implementation declarations to new signatures.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Updates DAC implementations for typed-byref/string/array/basic object queries.
docs/design/datacontracts/RuntimeTypeSystem.md	Documents the new `TypedByRef` descriptor + `GetTypedByRefInfo` API.
docs/design/datacontracts/Object.md	Documents `GetStringData` / `GetSize` additions and intended behavior.

Copilot's findings

Files reviewed: 21/21 changed files
Comments generated: 5

Copilot

Copilot's findings

Files reviewed: 21/21 changed files
Comments generated: 6

github-actions · 2026-06-11T18:34:02Z

🤖 Copilot Code Review — PR #129200

Note

This review was generated by Copilot.

Holistic Assessment

Motivation: Well-justified. The monolithic DebuggerIPCE_ObjectData struct forced every DacDbi API to pass the entire struct even though each method only populated a small subset of fields. Decomposing into specific out parameters makes the interface clearer, easier to implement from the cDAC layer, and eliminates the hidden coupling between fields.

Approach: Sound. The PR systematically replaces DebuggerIPCE_ObjectData with a new DacDbiObjectData (local to the DI layer) and changes each DacDbi API to accept only the parameters it actually populates. The new cDAC contract APIs (GetStringData, GetSize, GetTypedByRefInfo) are clean additions that follow existing patterns. The #if DEBUG validation blocks comparing cDAC and legacy DAC results are a solid correctness safety net.

Summary: ⚠️ Needs Human Review. The code is well-structured and the signature changes are fully propagated across all callers. Two minor inconsistencies merit human attention but are not merge-blocking: (1) GetStringData in DacDbiImpl.cs is the only method of the four that doesn't pre-initialize its out parameters, and (2) no unit tests cover the new GetStringData/GetSize contract implementations. Both were flagged in prior reviews and marked resolved, so a human should confirm the disposition.

Detailed Findings

✅ Signature Propagation — Complete and correct

All four refactored DacDbi methods (GetTypedByRefInfo, GetStringData, GetArrayData, GetBasicObjectInfo) have their new signatures consistently updated across every layer: dacdbi.idl, dacdbiinterface.h, dacdbiimpl.h/.cpp, IDacDbiInterface.cs, DacDbiImpl.cs, and all callers in divalue.cpp and process.cpp. The old DebuggerIPCE_ObjectData type has zero remaining references. Clean removal.

✅ cDAC Contract Implementations — Follow established patterns

GetStringData in Object_1.cs validates identically to the existing GetStringValue (same null-MT and string-MT checks).
GetSize correctly uses componentSize > 0 to detect variable-size objects and reuses Data.Array for reading NumComponents (which shares layout with m_StringLength for strings).
GetTypedByRefInfo in RuntimeTypeSystem_1.cs is a straightforward data descriptor read with proper TypedByRef data type and field definitions in datadescriptor.inc.
New Data/TypedByRef.cs follows existing data class conventions, is in an SDK-style project (auto-globbed), and DataType.TypedByRef is registered in DataType.cs.

✅ Debug Validation — Thorough

All four methods in DacDbiImpl.cs include #if DEBUG blocks that cross-validate cDAC results against the legacy DAC. The assertions cover all output parameters including struct fields (array rank, bounds offsets, element size, etc.).

✅ DacDbiObjectData Replaces DebuggerIPCE_ObjectData — Clean transition

The new DacDbiObjectData in dacdbistructures.h uses CORDB_ADDRESS for objRef (instead of void*), UINT for sizes (instead of SIZE_T), and a well-typed DacDbiArrayInfo struct for array data. All divalue.cpp comparisons are updated to use (CORDB_ADDRESS)NULL appropriately. The PTR_TO_CORDB_ADDRESS conversions are correctly removed where objRef is now already CORDB_ADDRESS.

⚠️ GetStringData in DacDbiImpl.cs — Out parameters not pre-initialized

DacDbiImpl.GetStringData (line 3033) does not initialize *pLength and *pOffsetToStringBase before the try block. The other three methods (GetTypedByRefInfo, GetArrayData, GetBasicObjectInfo) all initialize their out parameters to zero/default before entering the try. In practice this is safe today because the only caller (divalue.cpp:1534) pre-zeroes the entire DacDbiObjectData via PreInitObjectData→memset, and IfFailThrow prevents using the values on failure. However, the inconsistency could be a footgun for future callers. This was flagged in a prior review and resolved — confirming the disposition is intentional would be good.

⚠️ Missing Unit Tests for GetStringData and GetSize

The PR adds unit tests for GetTypedByRefInfo (two Theory test methods in MethodTableTests.cs), but no tests for the new IObject.GetStringData or IObject.GetSize implementations. Existing test infrastructure in ObjectTests.cs covers GetStringValue and GetArrayData and could serve as a template for testing these new APIs. This was also flagged in a prior review and resolved. Not blocking, but worth confirming as an intentional deferral.

💡 Error Message Inconsistency in GetSize

GetSize at Object_1.cs:196 throws "Address represents a free object" while the rest of the file (e.g., GetStringData, GetStringValue, GetArrayData) uses "Address represents a set-free object". Minor inconsistency — not blocking.

Generated by Code Review for issue #129200 · ● 8.4M · ◷

rcj1 · 2026-06-11T19:28:02Z

@copilot please add unit tests for GetStringData and GetSize.

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>

Copilot · 2026-06-11T19:34:24Z

@copilot please add unit tests for GetStringData and GetSize.

Added StringData and Size theory tests to ObjectTests.cs covering GetStringData (length + offset to first char) and GetSize (fixed-size object and variable-size array). Per your preference I did not run the build; please build/run when convenient.

Note

This reply was generated by GitHub Copilot.

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>

noahfalk

LGTM!

rcj1 · 2026-06-12T13:05:34Z

/ba-g #129297

…BasicObjectInfo (#129200) * Add two cDAC APIs on Object contract: ```csharp void GetStringData(TargetPointer address, out uint length, out uint offsetToFirstChar); ulong GetSize(TargetPointer address); ``` * Add one cDAC API on RuntimeTypeSystem: ```csharp TypedByRefInfo GetTypedByRefInfo(TargetPointer typedByRef); ``` * Implement DacDbi APIs, slightly modifying out params to be more specific to each API * Moving, renaming, and re-typing DebuggerIPCE_ObjectData * Add unit tests for `GetStringData` and `GetSize` to `ObjectTests.cs`, covering string length/offset and both fixed-size object and variable-size array sizing across architectures --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

cdac: implement GetTypedByRefInfo, GetStringData, GetArrayData, GetBa…
122b2c6
…sicObjectInfo (managed side) Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>

CopilotAI review requested due to automatic review settings June 9, 2026 23:30

Copilotstarted reviewing on behalf of rcj1 June 9, 2026 23:30 View session

rcj1 added the area-Diagnostics-cdac label Jun 9, 2026

This comment has been minimized.
Sign in to view

CopilotAI reviewed Jun 9, 2026
View reviewed changes

fix build
9243933

rcj1 requested review from barosiak, max-charlamb and noahfalk June 10, 2026 01:54

This was referenced Jun 10, 2026
slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883
Open
The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008
Open

This comment has been minimized.
Sign in to view

Update divalue.cpp
7cedf6d

CopilotAI review requested due to automatic review settings June 10, 2026 02:16

Copilotstarted reviewing on behalf of rcj1 June 10, 2026 02:16 View session

This comment has been minimized.
Sign in to view

CopilotAI reviewed Jun 10, 2026
View reviewed changes

max-charlamb reviewed Jun 10, 2026
View reviewed changes

Comment threadsrc/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Outdated
Comment thread...native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs Outdated

jkotas reviewed Jun 10, 2026
View reviewed changes

Comment threaddocs/design/datacontracts/Object.md Outdated

code review
74672e3

Copilotstarted work on behalf of rcj1 June 11, 2026 19:28 View session

Add unit tests for GetStringData and GetSize
efaf3d4
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>

CopilotAI review requested due to automatic review settings June 11, 2026 19:34

rcj1 removed the request for review from Copilot June 11, 2026 19:34

Improve readability of Size test
e4a861f
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>

CopilotAI review requested due to automatic review settings June 11, 2026 19:35

rcj1 removed the request for review from Copilot June 11, 2026 19:35

Copilotfinished work on behalf of rcj1 June 11, 2026 19:35

build-analysisBot mentioned this pull request Jun 12, 2026
[browser][coreCLR] Wasm.Console.Node.Sample - undefined symbol: SystemInteropJS_GetManagedStackTrace #129229
Closed

noahfalk approved these changes Jun 12, 2026
View reviewed changes

rcj1 merged commit d22168c into mainJun 12, 2026
127 of 132 checks passed

rcj1 deleted the copilot/cdac-implement-dacdbi-apis branch June 12, 2026 13:06

dotnet-maestroBot mentioned this pull request Jun 13, 2026
[main] Source code updates from dotnet/runtime dotnet/dotnet#7196
Merged

dotnet-milestone-botBot added this to the 11.0-preview6 milestone Jun 17, 2026

github-actionsBot locked and limited conversation to collaborators Jul 18, 2026

Uh oh!

Conversation

rcj1 commented Jun 9, 2026• edited by Copilot AILoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

CopilotAI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actionsBot commented Jun 11, 2026

🤖 Copilot Code Review — PR #129200

Holistic Assessment

Detailed Findings

✅ Signature Propagation — Complete and correct

✅ cDAC Contract Implementations — Follow established patterns

✅ Debug Validation — Thorough

✅ DacDbiObjectData Replaces DebuggerIPCE_ObjectData — Clean transition

⚠️ GetStringData in DacDbiImpl.cs — Out parameters not pre-initialized

⚠️ Missing Unit Tests for GetStringData and GetSize

💡 Error Message Inconsistency in GetSize

Uh oh!

rcj1 commented Jun 11, 2026

Uh oh!

CopilotAI commented Jun 11, 2026

Uh oh!

noahfalk left a comment

Choose a reason for hiding this comment

Uh oh!

rcj1 commented Jun 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rcj1 commented Jun 9, 2026•
edited by Copilot AI
Loading