[cDAC] Implement IsValidObject for cDAC#128457
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR extends the cDAC runtime type system contract (and its tests/mocks) to support additional MethodTable/EEClass queries, then uses those APIs to implement DacDbiImpl.IsValidObject. It also adds a new CoreCLR cDAC global (ContinuationSingletonEEClass) to enable continuation subtype identification.
Changes:
- Adds a new CoreCLR cDAC global pointer (
ContinuationSingletonEEClass) and wires it into the managed reader contracts/tests. - Extends
IRuntimeTypeSystem/RuntimeTypeSystem_1with APIs to query EEClass pointers, canonical MethodTables, and continuation-without-metadata detection. - Implements
Microsoft.Diagnostics.DataContractReader.Legacy.DacDbiImpl.IsValidObjectusing the cDAC contracts and adds new MethodTable tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs | Adds mock global storage + setter for the continuation singleton EEClass pointer. |
| src/native/managed/cdac/tests/MethodTableTests.cs | Adds globals wiring and new theory tests for IsContinuationWithoutMetadata and IsCanonicalMethodTable. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs | Implements IsValidObject using cDAC contracts (replacing legacy fallback-only behavior). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs | Reads new global and implements new RTS APIs (GetClassPointer, IsCanonicalMethodTable, IsContinuationWithoutMetadata). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs | Adds the new global name constant. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs | Adds new public contract members used by IsValidObject. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Exposes g_singletonContinuationEEClass via a new cDAC global pointer. |
Comments suppressed due to low confidence (1)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs:548
- GetClassPointer is now a public IRuntimeTypeSystem API, but it doesn't guard against non-MethodTable TypeHandles (it indexes _methodTables[typeHandle.Address] unconditionally). Other public APIs in this type return TargetPointer.Null / 0 for non-MethodTable handles. Please add a typeHandle.IsMethodTable() check (and return TargetPointer.Null or throw ArgumentException consistently) to avoid KeyNotFound/undefined behavior when callers pass a TypeDesc handle.
public TargetPointer GetClassPointer(TypeHandle typeHandle)
{
MethodTable methodTable = _methodTables[typeHandle.Address];
switch (MethodTableFlags_1.GetEEClassOrCanonMTBits(methodTable.EEClassOrCanonMT))
{
| #if DEBUG | ||
| if (_legacy is not null) | ||
| { | ||
| Interop.BOOL* pResultLocal = stackalloc Interop.BOOL[1]; |
There was a problem hiding this comment.
No need to explicitly stack alloc, please follow the style of
|
Might want to have a dump test for this, see |
Summary
Implement IsValidObject in the cDAC.
Changes
Note to copilot code review: we don't care about compat with older dumps or runtimes or api review