Skip to content

[cDAC] refactor ObjectHandle IData#127115

Merged
rcj1 merged 3 commits intodotnet:mainfrom
rcj1:refactor-oh
Apr 21, 2026
Merged

[cDAC] refactor ObjectHandle IData#127115
rcj1 merged 3 commits intodotnet:mainfrom
rcj1:refactor-oh

Conversation

@rcj1
Copy link
Copy Markdown
Contributor

@rcj1 rcj1 commented Apr 19, 2026

ObjectHandle is a strange IData in that it expects the address parameter to be the address field of the OBJECTHANDLE as opposed to the address of the structure like in every other IData. This is annoying because it means we have to first ReadPointer from the address of the ObjectHandle before we construct one, which means these fields have to be typed as pointers instead of TYPE(ObjectHandle). With this refactoring, the parameter to ObjectHandle is now the address of the ObjectHandle, and we can type ObjectHandle fields properly and do ReadDataField where appropriate.

@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.

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

Refactors cDAC’s ObjectHandle IData so its constructor parameter is the address of the OBJECTHANDLE field (consistent with other IData), enabling strongly-typed ObjectHandle fields and use of ReadDataField<ObjectHandle> instead of extra pointer reads.

Changes:

  • Rename the VM data type from GCHandle to ObjectHandle and update contract descriptors accordingly.
  • Update multiple contract data models/contracts to read OBJECTHANDLE fields via ReadDataField<ObjectHandle> and expose ObjectHandle-typed properties.
  • Adjust tests and descriptor definitions to reflect the new ObjectHandle type.

Reviewed changes

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

Show a summary per file
File Description
src/native/managed/cdac/tests/ContractDescriptor/TargetTests.cs Updates test type/field expectations to DataType.ObjectHandle.
src/native/managed/cdac/tests/ContractDescriptor/TargetTests.SubDescriptors.cs Same as above for sub-descriptor tests.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Thread.cs Reads thread OBJECTHANDLE fields via ReadDataField<ObjectHandle> and updates property types.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/SyncBlock.cs Reads Lock as ObjectHandle inline data and stores it as such.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ObjectHandle.cs Changes ObjectHandle to interpret the input as field-address and performs the dereferences internally.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/LoaderAllocator.cs Reads ObjectHandle field via ReadDataField<ObjectHandle>.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InflightTLSData.cs Reads TLSData as ObjectHandle via ReadDataField<ObjectHandle>.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs Reads and exposes ThrownObjectHandle as ObjectHandle.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/AssemblyBinder.cs Reads and exposes AssemblyLoadContext as ObjectHandle.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs Updates contract logic to use ObjectHandle.Handle / .Object instead of raw pointers.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs Adjusts static valuetype access path to directly read the OBJECTREF from the slot.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs Uses ObjectHandle.Object to return the ALC object directly.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs Returns both handle and object from the new ObjectHandle representation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ConditionalWeakTable_1.cs Treats depHnd as an inline ObjectHandle field address; uses .Handle for extra info lookup.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ComWrappers_1.cs Treats mow as the address of an inline ObjectHandle field.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs Renames GCHandle enum member to ObjectHandle.
src/coreclr/vm/datadescriptor/datadescriptor.inc Updates data descriptor types/fields to declare OBJECTHANDLE-bearing fields as TYPE(ObjectHandle) and renames the type entry.

Copy link
Copy Markdown
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

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

LGTM, any comments @noahfalk

nit: There are some remaining references to GCHandle in docs/design/datacontracts/data_descriptor.md which should be converted to ObjectHandle

Comment thread docs/design/datacontracts/data_descriptor.md Outdated
Copilot AI review requested due to automatic review settings April 20, 2026 20:32
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 2 comments.

Copy link
Copy Markdown
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM!

@rcj1
Copy link
Copy Markdown
Contributor Author

rcj1 commented Apr 21, 2026

/ba-g wasm

@rcj1 rcj1 merged commit 81b1c76 into dotnet:main Apr 21, 2026
126 of 129 checks passed
@rcj1 rcj1 deleted the refactor-oh branch April 21, 2026 04:43
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.

6 participants