[cDAC]: Implement GetEnCHangingFieldInfo

[rcj1]

Implement this using several new cDAC RuntimeMutableTypeSystem APIs.

bool DoesEnCFieldDescNeedFixup(TargetPointer encFieldDescPointer);
TargetPointer GetEnCStaticFieldDataAddress(TargetPointer encFieldDescPointer);
TargetPointer GetEnCInstanceFieldAddress(TargetPointer objectAddress, TargetPointer encFieldDescPointer);

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends cDAC’s Edit-and-Continue support by wiring up GetEnCHangingFieldInfo end-to-end: it adds new cDAC data descriptors / managed data models for EnC runtime structures, implements the IRuntimeMutableTypeSystem helpers needed to locate EnC-added field storage, and updates the DAC/DBI surface to consume the new shape.

Changes:

• Implement GetEnCHangingFieldInfo in the managed cDAC DacDbiImpl using new IRuntimeMutableTypeSystem APIs.
• Add new cDAC data types / descriptors for EnC field and syncblock structures, plus contract methods to retrieve EnC field storage addresses.
• Update the native DAC/DBI interface signature (remove pfStatic) and adjust call sites + documentation; add unit tests for the new helpers.

Show a summary per file

File	Description
src/native/managed/cdac/tests/UnitTests/RuntimeMutableTypeSystemTests.cs	Adds unit tests for DoesEnCFieldDescNeedFixup and GetEnCStaticFieldDataAddress.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs	Updates managed COM projection: adds EnCHangingFieldInfo and changes GetEnCHangingFieldInfo signature.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs	Implements GetEnCHangingFieldInfo using cDAC contracts (and debug-compare vs legacy when available).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs	Adds EnC-related DataType entries for new descriptors.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/SyncBlock.cs	Reads optional SyncBlock.EnCInfo pointer when present in the descriptor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EnCSyncBlockInfo.cs	New data model for EnCSyncBlockInfo descriptor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EnCFieldDesc.cs	New data model for EnCFieldDesc descriptor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EnCAddedStaticField.cs	New data model for EnCAddedStaticField including [FieldAddress] storage address.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EnCAddedField.cs	New data model for per-object EnC-added field list entries.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EditAndContinueHelperObject.cs	New managed-type descriptor for System.Diagnostics.EditAndContinueHelper’s _objectReference slot address.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeMutableTypeSystem_1.cs	Implements new IRuntimeMutableTypeSystem APIs for EnC fixup/static/instance address resolution.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CorDbHResults.cs	Adds CORDBG_E_ENC_HANGING_FIELD constant.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeMutableTypeSystem.cs	Adds new public contract methods needed by the implementation.
src/coreclr/vm/syncblk.h	Exposes SyncBlock::m_pEnCInfo offset to cDAC under FEATURE_METADATA_UPDATER.
src/coreclr/vm/encee.h	Adds cDAC access to EnCFieldDesc / EnCSyncBlockInfo members via cdac_data<>.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Adds new cDAC type descriptors for EnC structures + optional SyncBlock.EnCInfo.
src/coreclr/inc/dacdbi.idl	Updates IDacDbiInterface::GetEnCHangingFieldInfo signature (drops pfStatic).
src/coreclr/debug/inc/dacdbistructures.inl	Adjusts EnCHangingFieldInfo::Init to use UINT offset param type.
src/coreclr/debug/inc/dacdbistructures.h	Changes EnCHangingFieldInfo offset storage from SIZE_T to UINT.
src/coreclr/debug/inc/dacdbiinterface.h	Updates virtual method signature for GetEnCHangingFieldInfo.
src/coreclr/debug/di/rsclass.cpp	Updates call site and uses FieldData.m_fFldIsStatic instead of out pfStatic.
src/coreclr/debug/daccess/dacdbiimpl.h	Updates declaration for GetEnCHangingFieldInfo.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Updates definition for GetEnCHangingFieldInfo and removes pfStatic assignment.
docs/design/datacontracts/RuntimeMutableTypeSystem.md	Documents new contract APIs, required contracts, and new data descriptors.

Copilot's findings

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

[Copilot]

Copilot's findings

• Files reviewed: 24/24 changed files
• Comments generated: 4

[github-actions]

Copilot Code Review

Holistic Assessment

Motivation: Justified. This continues the systematic migration of native DAC APIs to the managed cDAC. GetEnCHangingFieldInfo is required for Edit-and-Continue field inspection through the cDAC path. The design doc update in the latest commit incorporates review feedback from @noahfalk.

Approach: The decomposition into three IRuntimeMutableTypeSystem building-block APIs (DoesEnCFieldDescNeedFixup, GetEnCStaticFieldDataAddress, GetEnCInstanceFieldAddress) is clean and mirrors the native code's internal structure. The DacDbi interface simplification (removing pfStatic, decomposing DebuggerIPCE_ObjectData into smaller output types) is a well-motivated cleanup. Per cdac.instructions.md for .NET 11 preview, the DacDbi interface is internal/unstable and these changes do not require breaking change documentation.

Summary: ⚠️ Needs Human Review. The implementation is well-structured, follows cDAC patterns, and includes comprehensive DEBUG cross-validation. Three issues from prior reviews remain unaddressed in the latest commits: (1) a missing defensive IsFieldDescEnCNew check matching the native DAC, (2) no unit tests for GetEnCInstanceFieldAddress, and (3) uninitialized output on error paths. A human reviewer should decide whether these are blocking.

---

Detailed Findings

Detailed Findings

✅ Data Descriptors & Contract Implementation — Correct

The new data types (EnCFieldDesc, EnCAddedField, EnCAddedStaticField, EnCSyncBlockInfo, EditAndContinueHelperObject) are properly defined. The [FieldAddress] attributes on EnCAddedStaticField.FieldData and EditAndContinueHelperObject.ObjectReferenceAddress correctly return target addresses rather than values, matching the native semantics (e.g., EnCAddedStaticField::m_FieldData is a variable-sized byte field whose address is the static field storage location). The SyncBlock.EnCInfo field is properly guarded behind FEATURE_METADATA_UPDATER on the native side and handled gracefully via type.Fields.ContainsKey(nameof(EnCInfo)) on the managed side.

✅ DacDbi Interface Refactoring — Clean

The decomposition of DebuggerIPCE_ObjectData into individual parameters (GetTypedByRefInfo, GetStringData, GetArrayData, GetBasicObjectInfo) with a new caller-side DacDbiObjectData struct is a clean separation that makes the DacDbi COM boundary more explicit. The EnCHangingFieldInfo.m_offsetToVars narrowing from SIZE_T to UINT is safe — the value originates as UINT in DebugObjectInfo.

✅ Design Doc Update (latest commit) — Addresses review feedback

The latest commit consolidates the System.Diagnostics.EditAndContinueHelper managed type into the main data types table (removing the separate "Managed types used" section), per @noahfalk's resolved review comment.

⚠️ Missing IsFieldDescEnCNew defensive check — Still open

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs line 2684

The native DAC asserts _ASSERTE(pFD->IsEnCNew()) (dacdbiimpl.cpp:3697) before interpreting the FieldDesc as an EnCFieldDesc. The cDAC implementation directly calls mrts.DoesEnCFieldDescNeedFixup(fieldDescPointer) without verifying the FieldDesc is EnC-new. If a non-EnC FieldDesc were passed, reading at the EnCFieldDesc::NeedsFixup offset would interpret memory beyond the base FieldDesc layout. The contract already provides IsFieldDescEnCNew — adding Debug.Assert(mrts.IsFieldDescEnCNew(fieldDescPointer)) before line 2684 would match the native DAC's defensive posture. Low risk since callers guarantee EnC fields, and the DEBUG cross-check catches mismatches.

⚠️ Uninitialized pFieldData on error paths — Still open

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs line 2669

*pFieldData = default; only executes on the success path (line 2704). If an exception occurs before this (e.g., during LookupTypeDefOrRefInAssembly or GetModuleLookupMapElement), the output struct remains uninitialized. Other DacDbi methods zero outputs up-front. Moving *pFieldData = default; to immediately after the try { would be consistent.

⚠️ No unit tests for GetEnCInstanceFieldAddress — Still open

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeMutableTypeSystem_1.cs lines 101-166

This is the most complex method: SyncBlock → EnCSyncBlockInfo linked list walk → dependent handle secondary → managed helper object → type-dependent address computation (three branches: ValueType unbox, Class OBJECTREF slot, primitive array data). The other two new APIs have dedicated unit tests. While the DEBUG cross-check in DacDbiImpl validates against the legacy DAC at runtime, unit tests covering the three CorElementType branches and the null/missing-EnCInfo early exits would improve regression protection.

💡 EnumerateMonitorEventWaitList removed — Out of PR scope

The native DAC implementation of EnumerateMonitorEventWaitList was replaced with return E_NOTIMPL. The managed cDAC already returned E_NOTIMPL for this method, so this aligns the native side. However, this method IS called by CordbHeapValue3Impl::GetMonitorEventWaitList in divalue.cpp (implementing the public ICorDebugValue3 API). While the change is consistent and the comment states the API is not believed to be called by current consumers, this is functionally unrelated to the EnC hanging field implementation and could be separated into its own PR.

Note

This review was created by GitHub Copilot.

Generated by Code Review for issue #129411 · ● 110.4M · ◷

[rcj1]

/ba-g timeouts