Skip to content

Add GetApproxTypeHandle dump tests#128921

Draft
Copilot wants to merge 8 commits into
mainfrom
copilot/implement-getapproxtypehandle
Draft

Add GetApproxTypeHandle dump tests#128921
Copilot wants to merge 8 commits into
mainfrom
copilot/implement-getapproxtypehandle

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 2, 2026

No description provided.

Copilot AI and others added 2 commits June 2, 2026 15:30
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
@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.

…y+metadataToken

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 3, 2026 00:35
Copilot AI changed the title Add DacDbi approximate type handle dump tests Add GetApproxTypeHandle dump tests Jun 3, 2026
Copilot finished work on behalf of rcj1 June 3, 2026 00:35
Copilot AI requested a review from rcj1 June 3, 2026 00:35
Copilot AI review requested due to automatic review settings June 3, 2026 02:46
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

This PR adds dump-based coverage for DacDbiImpl.GetApproxTypeHandle by mirroring the existing exact-type-handle dump tests: it enumerates heap objects reachable from GC handles in the ExactTypeHandle debuggee, constructs the flattened DebuggerIPCE_TypeArgData[] tree (a managed port of CordbType::GatherTypeData), round-trips that through GetApproxTypeHandle, and asserts the returned vmTypeHandle matches an independently canonicalized expected MethodTable. To support this, it also adds a managed TypeDataWalk implementation and refactors some type lookup logic into a helper.

Changes:

  • Add a new dump test suite DacDbiApproxTypeHandleDumpTests implementing the right-side “gather type data” flattening and expected-handle canonicalization.
  • Implement managed GetApproxTypeHandle support via a new TypeDataWalk port and supporting helper types.
  • Replace IsObjRef(TypeHandle) usage with a new IsCorElementTypeObjRef(CorElementType) helper across tests, the contract, and documentation.
Show a summary per file
File Description
src/native/managed/cdac/tests/MethodTableTests.cs Updates objref classification assertions to use IsCorElementTypeObjRef(GetInternalCorElementType(...)).
src/native/managed/cdac/tests/DumpTests/RuntimeTypeSystemDumpTests.cs Updates dump-based RTS “objref consistency” test to use the new helper.
src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiObjectDumpTests.cs Updates reference-type offset logic to use the new helper.
src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiApproxTypeHandleDumpTests.cs New dump test covering round-trip behavior for GetApproxTypeHandle across reachable handle objects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/TypeDataWalk.cs New managed port of the native flattened-type-data walker used by GetApproxTypeHandle.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Adds DebuggerIPCE_TypeArgData / TypeInfoList and tightens GetApproxTypeHandle signature to a typed pointer.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DbiHelpers.cs New helper for resolving typeDef/typeRef MethodTables from an assembly+token via loader lookup tables.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Implements managed GetApproxTypeHandle and switches some ref-type checks to IsCorElementTypeObjRef.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs Replaces IsObjRef with IsCorElementTypeObjRef implementation used by the contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs Changes the contract surface by removing IsObjRef(TypeHandle) and adding IsCorElementTypeObjRef(CorElementType).
docs/design/datacontracts/RuntimeTypeSystem.md Updates contract documentation to match the new helper.

Copilot's findings

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

Comment thread docs/design/datacontracts/RuntimeTypeSystem.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 03:50
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.

Copilot's findings

  • Files reviewed: 11/11 changed files
  • Comments generated: 7

Comment on lines +290 to +294
[StructLayout(LayoutKind.Sequential, Size = 48)]
public struct DebuggerIPCE_TypeArgData
{
public DebuggerIPCE_ExpandedTypeData data;
public uint numTypeArgs; // Portable<UINT>
Comment on lines +297 to +301
[StructLayout(LayoutKind.Sequential)]
public unsafe struct TypeInfoList
{
public DebuggerIPCE_TypeArgData* m_pList;
public int m_nEntries;
Comment thread docs/design/datacontracts/RuntimeTypeSystem.md Outdated
Comment on lines +251 to +254
Assert.True(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(objectHandle)));
Assert.True(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(stringHandle)));
Assert.True(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(objectArrayHandle)));
Assert.False(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(intPtrHandle)));
Comment on lines +705 to +708
Assert.True(contract.IsCorElementTypeObjRef(contract.GetInternalCorElementType(contract.GetTypeHandle(objectTypePtr))));
Assert.True(contract.IsCorElementTypeObjRef(contract.GetInternalCorElementType(contract.GetTypeHandle(stringTypePtr))));
Assert.True(contract.IsCorElementTypeObjRef(contract.GetInternalCorElementType(contract.GetTypeHandle(szArrayTypePtr))));
Assert.False(contract.IsCorElementTypeObjRef(contract.GetInternalCorElementType(contract.GetTypeHandle(truePrimitiveTypePtr))));
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 04:02
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.

Copilot's findings

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

Comment thread docs/design/datacontracts/RuntimeTypeSystem.md Outdated
Comment on lines +160 to +176
case CorElementType.Array:
case CorElementType.SzArray:
case CorElementType.Ptr:
case CorElementType.Byref:
pSelf->numTypeArgs = 1;
FillTypeNodes(dbi, rts, rts.GetTypeParam(th), nodes, ref idx);
break;

case CorElementType.Class:
case CorElementType.ValueType:
{
ReadOnlySpan<TypeHandle> inst = rts.GetInstantiation(th);
pSelf->numTypeArgs = (uint)inst.Length;
for (int i = 0; i < inst.Length; i++)
FillTypeNodes(dbi, rts, inst[i], nodes, ref idx);
break;
}
Comment on lines 131 to 135
bool IsObject(TypeHandle typeHandle) => throw new NotImplementedException();
bool IsString(TypeHandle typeHandle) => throw new NotImplementedException();
bool IsObjRef(TypeHandle typeHandle) => throw new NotImplementedException();
// True if the CorElementType represents a GC-collectable object reference.
bool IsCorElementTypeObjRef(CorElementType elementType) => throw new NotImplementedException();
// True if the MethodTable represents a type that contains managed references
Comment on lines +251 to +254
Assert.True(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(objectHandle)));
Assert.True(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(stringHandle)));
Assert.True(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(objectArrayHandle)));
Assert.False(rts.IsCorElementTypeObjRef(rts.GetInternalCorElementType(intPtrHandle)));
rcj1 and others added 2 commits June 2, 2026 23:05
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 06:05
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.

Copilot's findings

  • Files reviewed: 11/11 changed files
  • Comments generated: 2

Comment on lines +133 to 135
// True if the CorElementType represents a GC-collectable object reference.
bool IsCorElementTypeObjRef(CorElementType elementType) => throw new NotImplementedException();
// True if the MethodTable represents a type that contains managed references
Comment on lines +157 to +181
CorElementType et = GetElementType(rts, th);
switch (et)
{
case CorElementType.Array:
case CorElementType.SzArray:
case CorElementType.Ptr:
case CorElementType.Byref:
pSelf->numTypeArgs = 1;
FillTypeNodes(dbi, rts, rts.GetTypeParam(th), nodes, ref idx);
break;

case CorElementType.Class:
case CorElementType.ValueType:
{
ReadOnlySpan<TypeHandle> inst = rts.GetInstantiation(th);
pSelf->numTypeArgs = (uint)inst.Length;
for (int i = 0; i < inst.Length; i++)
FillTypeNodes(dbi, rts, inst[i], nodes, ref idx);
break;
}

default:
pSelf->numTypeArgs = 0;
break;
}
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.

3 participants