[cdac] Extract ITypeHandle interface and add synthetic type handles by max-charlamb · Pull Request #129800 · dotnet/runtime

max-charlamb · 2026-06-24T14:31:07Z

Summary

Convert TypeHandle from a readonly struct to the ITypeHandle interface, enabling synthetic (reader-fabricated) TypeHandles for unloaded constructed types (Ptr, Byref, SzArray, Array).

Changes

Rename TypeHandle -> ITypeHandle across entire cDAC codebase
ITypeHandle interface in Abstractions with Address, IsNull, IsSynthetic
NullTypeHandle singleton in Abstractions (ITypeHandle.Null)
TargetTypeHandle struct in Contracts (real target-backed handles)
SyntheticTypeHandle internal class in Contracts (unloaded constructed types)
10 RTS methods with synthetic-aware branches
GetConstructedType synthesizes on miss for Ptr/Byref/SzArray/Array instead of returning null
ReadOnlySpan<TypeHandle> replaced with ITypeHandle[] in public API

Motivation

When decoding method signatures, the runtime frequently has no loaded TypeHandle for constructed types like int*, ref T, or T[]. Previously GetConstructedType returned null, forcing consumers into ad-hoc side-channels. Now it returns a synthetic handle that downstream contracts can query uniformly.

Note

This content was generated with the assistance of GitHub Copilot.

dotnet-policy-service · 2026-06-24T14:37:54Z

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 refactors cDAC’s type identity from a concrete TypeHandle struct to an ITypeHandle abstraction, enabling “synthetic” (reader-fabricated) handles for unloaded constructed types (Ptr/Byref/SZArray/Array) and updating the contracts/legacy layers/tests accordingly.

Changes:

Introduces ITypeHandle (+ null/synthetic/target-backed implementations) and updates contracts to traffic in ITypeHandle.
Updates RuntimeTypeSystem_1.GetConstructedType to synthesize constructed types on miss for Ptr/Byref/SZArray/Array.
Updates legacy SOS/DacDbi code and cDAC unit/dump tests to consume the new handle abstraction.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/native/managed/cdac/tests/UnitTests/TypeDescTests.cs	Updates unit tests to use `ITypeHandle` and array-returning APIs.
src/native/managed/cdac/tests/UnitTests/SOSDacInterface5Tests.cs	Switches test setup to `TargetTypeHandle`.
src/native/managed/cdac/tests/UnitTests/RuntimeMutableTypeSystemTests.cs	Updates Moq setups and APIs to `ITypeHandle`.
src/native/managed/cdac/tests/UnitTests/ObjectTests.cs	Updates object contract tests to use `TargetTypeHandle`.
src/native/managed/cdac/tests/UnitTests/MethodTableTests.cs	Replaces `TypeHandle` usage with `ITypeHandle` across MT tests.
src/native/managed/cdac/tests/UnitTests/MethodDescTests.cs	Updates generic-instantiation test assertions to use `ITypeHandle[]`.
src/native/managed/cdac/tests/UnitTests/ExceptionTests.cs	Updates exception contract tests to use `TargetTypeHandle`.
src/native/managed/cdac/tests/UnitTests/DacDbiImplTests.cs	Updates DacDbi tests to use `TargetTypeHandle` and new APIs.
src/native/managed/cdac/tests/UnitTests/CodeVersionsTests.cs	Updates code versions test helpers to use `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/RuntimeTypeSystemDumpTests.cs	Updates dump tests to the `ITypeHandle` API surface.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.cs	Updates dump test to `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/IXCLRDataValueDumpTests.cs	Updates commentary/type references for `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/IXCLRDataMethodDefinitionDumpTests.cs	Updates dump test to `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiObjectDumpTests.cs	Updates DacDbi dump tests and helpers to `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiLoaderDumpTests.cs	Updates DacDbi loader dump test to `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiExactTypeHandleDumpTests.cs	Updates exact-typehandle roundtrip dump test to `ITypeHandle`.
src/native/managed/cdac/tests/DumpTests/CCWDumpTests.cs	Updates CCW dump test wording and `ITypeHandle` usage.
src/native/managed/cdac/tests/DumpTests/AsyncContinuationDumpTests.cs	Updates continuation dump tests to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/TypeNameBuilder.cs	Updates type/method name formatting to consume `ITypeHandle` and array-returning instantiation APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs	Updates IXCLRDataProcess implementation to `ITypeHandle` and array-returning APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs	Updates SOS DAC implementation to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SigFormat.cs	Updates signature formatting helpers to use `ITypeHandle` and array-returning APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs	Updates interop struct comments to refer to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/Helpers/HeapWalk.cs	Updates heap-walk helper to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs	Updates DacDbi implementation to use `ITypeHandle` and constructed-type synthesis.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodInstance.cs	Updates method instance implementation to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodDefinition.cs	Updates generic-instantiation checks to `ITypeHandle[]`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs	Updates frame/type resolution paths to use `ITypeHandle` and array-returning instantiations.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/ExtensionMethods.cs	Adapts type-handle helper extensions to operate on `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcSignatureTypeProvider.cs	Updates GC signature decoding context and lookups to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameHelpers.cs	Updates frame helper logic to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/SignatureTypeProvider.cs	Updates signature type provider to produce/consume `ITypeHandle` and constructed types.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs	Updates signature contract implementation to decode to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/IRuntimeSignatureTypeProvider.cs	Updates documentation for “internal type” handling to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem/TypeHandleImplementations.cs	Adds `TargetTypeHandle` and `SyntheticTypeHandle` implementations of `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs	Core RuntimeTypeSystem changes: `ITypeHandle` plumbing + synthetic constructed-type behavior.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeMutableTypeSystem_1.cs	Updates mutable type system APIs to accept `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs	Updates object sizing/array logic to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ManagedTypeSource_1.cs	Updates managed type resolution/caching to `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs	Updates exception-clause reading path to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs	Updates exception contract implementation to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ConditionalWeakTable_1.cs	Updates CWT contract implementation to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ComWrappers_1.cs	Updates RCW detection to use the renamed generated type-handle accessor.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CodeVersions_1.cs	Updates code-versions contract to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs	Changes signature decoding contract to return `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs	Introduces `ITypeHandle`/`NullTypeHandle` and updates the RuntimeTypeSystem contract signatures.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeMutableTypeSystem.cs	Updates mutable-type-system contract signature to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IManagedTypeSource.cs	Updates managed type source contract to use `ITypeHandle`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CdacAttributes.cs	Updates generator-related docs to refer to `ITypeHandle`.
src/native/managed/cdac/gen/TypeNameResolverSource.cs	Updates generated helper to return `ITypeHandle`.
src/native/managed/cdac/gen/Emitter.cs	Updates generated output to reference `ITypeHandle`.
src/native/managed/cdac/gen/CdacGenerator.cs	Updates generator docs to refer to `ITypeHandle`.

+/// <summary>
+/// An opaque handle to a runtime type. May represent a loaded type (backed by a
+/// target-process MethodTable or TypeDesc address) or a synthetic type fabricated
+/// by the reader for unloaded constructed types.
+/// </summary>
+public interface ITypeHandle : IEquatable<ITypeHandle>
 {
-    // TODO-Layering: These members should be accessible only to contract implementations.
-    public TypeHandle(TargetPointer address)
-    {
-        Address = address;
-    }
-
-    public TargetPointer Address { get; }
+    TargetPointer Address { get; }
+    bool IsNull { get; }
+    bool IsSynthetic { get; }
+    static ITypeHandle Null { get; } = NullTypeHandle.Instance;
+}


+    public bool IsNull => Address == 0;
+    public bool IsSynthetic => false;
+
+    public bool Equals(ITypeHandle? other)
+        => other is TargetTypeHandle t && Address == t.Address;
+    public bool Equals(TargetTypeHandle other) => Address == other.Address;


@@ -474,14 +474,14 @@ public TypeHandle GetTypeHandle(TargetPointer typeHandlePointer)
        // if we already validated this address, return a handle
        if (_methodTables.ContainsKey(typeHandlePointer))
        {
-            return new TypeHandle(typeHandlePointer);
+            return new TargetTypeHandle(typeHandlePointer);


+        // No loaded match found. For Ptr/Byref/SzArray/Array, synthesize a handle
+        // so signature-decoding contracts can treat unloaded constructed types uniformly.
+        if (corElementType is CorElementType.Ptr or CorElementType.Byref or CorElementType.SzArray or CorElementType.Array)
+        {
+            var synthetic = new SyntheticTypeHandle(corElementType, typeHandle, rank);
+            _ = _typeHandles.TryAdd(new TypeKey(typeHandle, corElementType, rank, typeArguments), synthetic);
+            return synthetic;
+        }


rcj1 · 2026-06-24T14:52:10Z

ad-hoc side-channels

Am I missing something, or where are you removing said side channels with this abstraction? If this is a helper for a PR that you have in progress, what does it look like before and after?

max-charlamb · 2026-06-24T15:34:00Z

ad-hoc side-channels

Am I missing something, or where are you removing said side channels with this abstraction? If this is a helper for a PR that you have in progress, what does it look like before and after?

I needed extra data for implementing the ArgIterator: https://github.com/dotnet/runtime/pull/129769/changes#r3468318817

I believe adding synthetic typehandles aligns with our long-term goal to not have to load things in the runtime just for diagnostics.

rcj1 · 2026-06-24T15:35:55Z

ad-hoc side-channels

Am I missing something, or where are you removing said side channels with this abstraction? If this is a helper for a PR that you have in progress, what does it look like before and after?

I needed extra data for implementing the ArgIterator: https://github.com/dotnet/runtime/pull/129769/changes#r3468318817

I believe adding synthetic typehandles aligns with our long-term goal to not have to load things in the runtime just for diagnostics.

Could we accomplish this by adding (optional) fields and properties to the TypeHandle struct?

max-charlamb · 2026-06-24T15:56:50Z

ad-hoc side-channels

Am I missing something, or where are you removing said side channels with this abstraction? If this is a helper for a PR that you have in progress, what does it look like before and after?

I needed extra data for implementing the ArgIterator: https://github.com/dotnet/runtime/pull/129769/changes#r3468318817
I believe adding synthetic typehandles aligns with our long-term goal to not have to load things in the runtime just for diagnostics.

Could we accomplish this by adding (optional) fields and properties to the TypeHandle struct?

The TypeHandle was always meant to be an opaque handle, moving to an interface better accomplishes that and matches the pattern in other contracts.

runtime/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs

Lines 11 to 23 in 440e6cb

    
           // an opaque handle to a type handle.  See IMetadata.GetMethodTableData 
        
           public readonly struct TypeHandle 
        
           { 
        
               // TODO-Layering: These members should be accessible only to contract implementations. 
        
               public TypeHandle(TargetPointer address) 
        
               { 
        
                   Address = address; 
        
               } 
        
               public TargetPointer Address { get; } 
        
               public bool IsNull => Address == 0; 
        
           }

Copilot

Pull request overview

Copilot reviewed 54 out of 54 changed files in this pull request and generated 6 comments.

+public sealed class NullTypeHandle : ITypeHandle
+{
+    public static readonly NullTypeHandle Instance = new();
+    private NullTypeHandle() { }


+    public bool Equals(ITypeHandle? other)
+        => other is TargetTypeHandle t && Address == t.Address;
+    public bool Equals(TargetTypeHandle other) => Address == other.Address;


+        if (corElementType is CorElementType.Ptr or CorElementType.Byref or CorElementType.SzArray or CorElementType.Array)
+        {
+            var synthetic = new SyntheticTypeHandle(corElementType, typeHandle, rank);
+            _ = _typeHandles.TryAdd(new TypeKey(typeHandle, corElementType, rank, typeArguments), synthetic);
+            return synthetic;


+        foreach (ITypeHandle arg in retAndArgTypes)
        {
            if (arg.Address == TargetPointer.Null)
                continue;


                // that we can return to SOS for pretty-printing.
-                // In the future we may want to return a TypeHandle instead of a MethodTable, and modify SOS to do more complete pretty-printing.
-                // DAC equivalent: src/coreclr/vm/typehandle.inl TypeHandle::GetMethodTable
+                // In the future we may want to return a ITypeHandle instead of a MethodTable, and modify SOS to do more complete pretty-printing.
+                // DAC equivalent: src/coreclr/vm/typehandle.inl ITypeHandle::GetMethodTable
                if (rtsContract.IsFunctionPointer(foundTypeHandle, out _, out _) || rtsContract.IsPointer(foundTypeHandle))


    // Shared core implementation for TypeHandleToExpandedTypeInfo and GetObjectExpandedTypeInfo.
-    private void TypeHandleToExpandedTypeInfoImpl(IRuntimeTypeSystem rts, AreValueTypesBoxed boxed, TypeHandle typeHandle, DebuggerIPCE_ExpandedTypeData* pTypeInfo)
+    private void ITypeHandleToExpandedTypeInfoImpl(IRuntimeTypeSystem rts, AreValueTypesBoxed boxed, ITypeHandle typeHandle, DebuggerIPCE_ExpandedTypeData* pTypeInfo)
    {


rcj1 · 2026-06-24T20:24:21Z

The TypeHandle was always meant to be an opaque handle

Ok, sounds good

Rename the TypeHandle struct to ITypeHandle across the entire cDAC codebase in preparation for converting it to an interface that supports synthetic type handles for unloaded constructed types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…peHandle Convert ITypeHandle from a readonly struct to an interface, enabling polymorphic type handles. Add NullTypeHandle (Abstractions) and TargetTypeHandle (Contracts) implementations. Replace ReadOnlySpan with ITypeHandle[] in public API signatures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add SyntheticTypeHandle (internal) for Ptr, Byref, SzArray, and Array types that the runtime has not loaded. GetConstructedType now returns a synthetic handle on miss instead of null, enabling signature-decoding contracts to treat all parameter types uniformly. Synthetic-aware branches added to 10 RTS methods: - GetSignatureCorElementType, HasTypeParam, GetTypeParam - IsPointer, IsArray, ContainsGenericVariables - GetModule, GetLoaderModule, IsLoaded - GetConstructedType (synthesis on miss) Also fixes GcSignatureTypeProvider to use IsNull instead of Address comparison, so synthetics get properly classified. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/CallingConvention_1.cs:245

Using Address==TargetPointer.Null to detect “null handle” is no longer equivalent now that synthetic handles have Address==0 but IsNull==false. This should use IsNull (and preferably avoid Address comparisons for ITypeHandle entirely) to prevent misclassifying synthetic handles as missing/uncached handles.

                    ITypeHandle probe = methodSig.ParameterTypes[argIndex];
                    if (probe.Address == TargetPointer.Null)
                        probe = paramInfo[argIndex].OpenGenericType;
                    if (probe.Address != TargetPointer.Null)
                    {


        Assert.Fail("Could not find CrashInExceptionHandler on the crashing thread's stack");
-        return default;
+        return ITypeHandle.Null;


+/// <summary>
+/// An ITypeHandle backed by a real target-process address (MethodTable* or TypeDesc*).
+/// </summary>
+public readonly struct TargetTypeHandle : ITypeHandle, IEquatable<TargetTypeHandle>
+{
+    public TargetTypeHandle(TargetPointer address)
+    {
+        Address = address;
+    }
+
+    public TargetPointer Address { get; }
+    public bool IsNull => Address == 0;
+    public bool IsSynthetic => false;
+


        // if we already validated this address, return a handle
        if (_methodTables.ContainsKey(typeHandlePointer))
        {
-            return new TypeHandle(typeHandlePointer);
+            return new TargetTypeHandle(typeHandlePointer);
        }


+        // The class advertises a managed identity (ITypeHandle) when HasTypeHandle is set.
        if (model.HasTypeHandle)
        {
-            sb.AppendLine($"    public static {TypeHandleType} TypeHandle({Target} target)");
+            sb.AppendLine($"    public static {ITypeHandleType} ITypeHandle({Target} target)");
            sb.AppendLine($"        => TypeNameResolver.GetTypeHandle(target, _typeNames);");
            sb.AppendLine();


+/// <summary>
+/// An opaque handle to a runtime type. May represent a loaded type (backed by a
+/// target-process MethodTable or TypeDesc address) or a synthetic type fabricated
+/// by the reader for unloaded constructed types.
+/// </summary>
+public interface ITypeHandle : IEquatable<ITypeHandle>
 {
-    // TODO-Layering: These members should be accessible only to contract implementations.
-    public TypeHandle(TargetPointer address)
-    {
-        Address = address;
-    }
-
-    public TargetPointer Address { get; }
+    TargetPointer Address { get; }
+    bool IsNull { get; }
+    bool IsSynthetic { get; }
+    static ITypeHandle Null { get; } = NullTypeHandle.Instance;
+}

-    public bool IsNull => Address == 0;
+/// <summary>
+/// Singleton ITypeHandle representing the absence of a type.
+/// </summary>
+public sealed class NullTypeHandle : ITypeHandle
+{
+    public static readonly NullTypeHandle Instance = new();
+    private NullTypeHandle() { }
+    public TargetPointer Address => TargetPointer.Null;
+    public bool IsNull => true;
+    public bool IsSynthetic => false;
+    public bool Equals(ITypeHandle? other) => other is not null && other.IsNull && !other.IsSynthetic;
+    public override bool Equals(object? obj) => obj is ITypeHandle th && Equals(th);
+    public override int GetHashCode() => 0;
 }


Copilot

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated 6 comments.

        Assert.Fail("Could not find CrashInExceptionHandler on the crashing thread's stack");
-        return default;
+        return ITypeHandle.Null;


+                // In the future we may want to return a ITypeHandle instead of a MethodTable, and modify SOS to do more complete pretty-printing.
+                // DAC equivalent: src/coreclr/vm/typehandle.inl ITypeHandle::GetMethodTable


+public interface ITypeHandle : IEquatable<ITypeHandle>
 {
-    // TODO-Layering: These members should be accessible only to contract implementations.
-    public TypeHandle(TargetPointer address)
-    {
-        Address = address;
-    }
-
-    public TargetPointer Address { get; }
+    TargetPointer Address { get; }
+    bool IsNull { get; }
+    bool IsSynthetic { get; }
+    static ITypeHandle Null { get; } = NullTypeHandle.Instance;
+}




-    ReadOnlySpan<TypeHandle> GetInstantiation(TypeHandle typeHandle) => throw new NotImplementedException();
-    public bool IsClassInited(TypeHandle typeHandle) => throw new NotImplementedException();
-    public bool IsInitError(TypeHandle typeHandle) => throw new NotImplementedException();
-    bool IsGenericTypeDefinition(TypeHandle typeHandle) => throw new NotImplementedException();
-    bool ContainsGenericVariables(TypeHandle typeHandle) => throw new NotImplementedException();
-    bool IsCollectible(TypeHandle typeHandle) => throw new NotImplementedException();
+    ITypeHandle[] GetInstantiation(ITypeHandle typeHandle) => throw new NotImplementedException();
+    public bool IsClassInited(ITypeHandle typeHandle) => throw new NotImplementedException();
+    public bool IsInitError(ITypeHandle typeHandle) => throw new NotImplementedException();


+/// <summary>
+/// An opaque handle to a runtime type. May represent a loaded type (backed by a
+/// target-process MethodTable or TypeDesc address) or a synthetic type fabricated
+/// by the reader for unloaded constructed types.
+/// </summary>
+public interface ITypeHandle : IEquatable<ITypeHandle>
 {
-    // TODO-Layering: These members should be accessible only to contract implementations.
-    public TypeHandle(TargetPointer address)
-    {
-        Address = address;
-    }
-
-    public TargetPointer Address { get; }
+    TargetPointer Address { get; }
+    bool IsNull { get; }
+    bool IsSynthetic { get; }
+    static ITypeHandle Null { get; } = NullTypeHandle.Instance;
+}

-    public bool IsNull => Address == 0;
+/// <summary>
+/// Singleton ITypeHandle representing the absence of a type.
+/// </summary>
+public sealed class NullTypeHandle : ITypeHandle
+{
+    public static readonly NullTypeHandle Instance = new();
+    private NullTypeHandle() { }
+    public TargetPointer Address => TargetPointer.Null;
+    public bool IsNull => true;
+    public bool IsSynthetic => false;
+    public bool Equals(ITypeHandle? other) => other is not null && other.IsNull && !other.IsSynthetic;
+    public override bool Equals(object? obj) => obj is ITypeHandle th && Equals(th);
+    public override int GetHashCode() => 0;
 }


+        // The class advertises a managed identity (ITypeHandle) when HasTypeHandle is set.
        if (model.HasTypeHandle)
        {
-            sb.AppendLine($"    public static {TypeHandleType} TypeHandle({Target} target)");
+            sb.AppendLine($"    public static {ITypeHandleType} ITypeHandle({Target} target)");
            sb.AppendLine($"        => TypeNameResolver.GetTypeHandle(target, _typeNames);");
            sb.AppendLine();
        }


Now that GetConstructedType returns synthetic ITypeHandles for unloaded Ptr/Byref/SzArray/Array types, the CallingConvention contract no longer needs the second signature decode pass or the _kindOverride workaround. Removed: - CdacTypeHandle._kindOverride field and 3-arg constructor - All _kindOverride checks in IsNull, IsPointerType, GetSize, GetCorElementType - ParamTypeInfo struct (was IsByRef + OutermostKind + OpenGenericType) - TrackedType struct - ParamMetadataProvider class (entire second signature decoder) - DecodeParamTypeInfo method - ArgumentLocation.OpenGenericType field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-01T20:38:05Z

Copilot Code Review

Holistic Assessment

Motivation: Justified. When decoding method signatures, GetConstructedType previously returned null for unloaded constructed types (pointers, byrefs, arrays), forcing consumers into ad-hoc workarounds. The CallingConvention code had ~250 lines of side-channel infrastructure (ParamTypeInfo, TrackedType, ParamMetadataProvider) to recover information the signature provider was dropping. Making the type handle polymorphic and synthesizing handles on miss eliminates this workaround cleanly.

Approach: The interface extraction (TypeHandle → ITypeHandle) with a synthetic handle subtype is a reasonable design for this problem. The CallingConvention simplification that follows is a strong signal the abstraction is pulling its weight. However, the struct-to-interface change introduces per-call boxing allocations for the common TargetTypeHandle path, and the return-type changes from ReadOnlySpan<TypeHandle> to ITypeHandle[] expose cached mutable arrays.

Summary: ⚠️ Needs Human Review. The overall direction is sound and the CallingConvention cleanup is a clear win. However, the boxing allocation concern on every GetTypeHandle call, the mutable-array exposure, and missing unit tests for synthetic handle behavior warrant human judgment on whether to address now or track as follow-ups. The code is correct for the flows I traced, but the performance characteristics have changed.

Detailed Findings

⚠️ Boxing allocation on every `GetTypeHandle` call

TargetTypeHandle is a struct, but every return new TargetTypeHandle(...) in GetTypeHandle() boxes it to the ITypeHandle interface. This is the most frequently called RTS method — it's invoked on every type lookup during signature decoding, stack walking, DacDbi operations, etc. The previous TypeHandle was a readonly struct returned by value with zero allocations.

In a diagnostic/debugging context this may be acceptable, but it's a measurable regression for dump-analysis scenarios that process many types. Consider either:

Making TargetTypeHandle a class (with a per-address cache in RuntimeTypeSystem_1 to avoid redundant allocations), or
Keeping the struct but caching boxed ITypeHandle instances per TargetPointer in the existing _methodTables dictionary (or a parallel one)

This affects RuntimeTypeSystem_1.cs lines 477, 492, 500, 508, 517 (all GetTypeHandle return sites).

⚠️ `GetInstantiation` / `GetGenericMethodInstantiation` return mutable cached arrays

The return type changed from ReadOnlySpan<TypeHandle> to ITypeHandle[]. The underlying arrays are cached in TypeInstantiation.TypeHandles and InstantiatedMethodDesc.Instantiation. Returning these directly allows consumers to mutate cached data, silently corrupting subsequent reads. The previous ReadOnlySpan return prevented this.

(IRuntimeTypeSystem.cs line 201, 247; RuntimeTypeSystem_1.cs lines 810, 371)

Consider returning ReadOnlyMemory<ITypeHandle>, ImmutableArray<ITypeHandle>, or at minimum documenting that the returned array must be treated as immutable.

⚠️ Generator emits method named `ITypeHandle`

The code generator (Emitter.cs line 65) now emits:

public static ITypeHandle ITypeHandle(Target target)

A method named ITypeHandle is visually identical to the type name and reads oddly at call sites (Data.Foo.ITypeHandle(target)). The mechanical rename from TypeHandle to ITypeHandle applied too broadly here — the method name should remain TypeHandle or be renamed to GetTypeHandle while only the return type changes to ITypeHandle.

⚠️ Missing unit tests for synthetic type handles

The synthetic handle creation in GetConstructedType (line 1188–1193) and the synthetic-aware branches in GetSignatureCorElementType, IsArray, GetTypeParam, HasTypeParam, IsPointer, ContainsGenericVariables, GetModule, and GetLoaderModule have no dedicated unit test coverage. The existing tests in MethodTableTests.cs and TypeDescTests.cs only exercise real target-backed handles.

Key scenarios to test:

Synthetic handle reports IsSynthetic == true, IsNull == false, Address == 0
GetSignatureCorElementType returns the correct CorElementType for each synthetic kind
GetTypeParam returns the element handle
IsArray reports correct rank for SzArray and Array synthetics
IsPointer returns true for Ptr synthetics
Round-trip: GetConstructedType → synthetic → query methods all agree

💡 Comment references to native `ITypeHandle::GetMethodTable`

Several comments in SOSDacImpl.cs (line 1086–1087) and DacDbiImpl.cs now reference ITypeHandle::GetMethodTable from typehandle.inl, but the native CoreCLR type is still named TypeHandle. The cDAC rename shouldn't leak into native source cross-references — these are meant to help maintainers find the corresponding native code.

✅ Synthetic-aware method routing is correct

I traced the synthetic handle flow through all 10 RTS methods with synthetic-aware branches. The pattern is consistent: check for SyntheticTypeHandle first, delegate to stored properties (Kind, Element, Rank), or recurse to the element. Methods without explicit synthetic handling fall through to IsMethodTable()/IsTypeDesc() checks, which correctly return false for synthetic handles (since Address == 0), causing those methods to return safe defaults (0, false, null).

✅ CallingConvention cleanup is a clear improvement

Removing ParamTypeInfo, TrackedType, ParamMetadataProvider, and the DecodeParamTypeInfo double-decode eliminates ~250 lines of complex workaround code. The CdacTypeHandle adapter is also simplified by removing the _kindOverride field and all associated branching. The synthetic handle approach is a better solution to the same problem.

✅ Equality implementation is symmetric

TargetTypeHandle.Equals(ITypeHandle) and NullTypeHandle.Equals(ITypeHandle) both handle the cross-type null case (zero-address TargetTypeHandle ↔ NullTypeHandle) symmetrically, and GetHashCode returns 0 in both cases. SyntheticTypeHandle equality is value-based on Kind, Rank, and Element, which is correct for the cache key semantics.

✅ cDAC API surface changes do not require API review

Per the cDAC folder-specific guidance for .NET 11 dev branches, new/changed APIs on IContract implementations under src/native/managed/cdac/ do not need to go through API review and must not be described as breaking changes.

Note

This review was generated by GitHub Copilot.

Generated by Code Review for issue #129800 · ● 91.1M · ◷

Copilot AI review requested due to automatic review settings June 24, 2026 14:31

github-actions Bot added the area-Diagnostics-coreclr label Jun 24, 2026

Copilot started reviewing on behalf of max-charlamb June 24, 2026 14:31 View session

dotnet-policy-service Bot assigned max-charlamb Jun 24, 2026

max-charlamb added area-Diagnostics-cdac and removed area-Diagnostics-coreclr labels Jun 24, 2026

Copilot AI reviewed Jun 24, 2026

View reviewed changes

max-charlamb force-pushed the dev/max-charlamb/rts-synthetic-typehandles branch from 1c85305 to 2050b89 Compare June 24, 2026 14:50

Copilot AI review requested due to automatic review settings June 24, 2026 16:03

max-charlamb force-pushed the dev/max-charlamb/rts-synthetic-typehandles branch from 2050b89 to 6522cf8 Compare June 24, 2026 16:03

Copilot started reviewing on behalf of max-charlamb June 24, 2026 16:03 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

max-charlamb force-pushed the dev/max-charlamb/rts-synthetic-typehandles branch from 6522cf8 to 791163a Compare June 24, 2026 19:40

build-analysis Bot mentioned this pull request Jun 25, 2026

SOSStackAndOtherTests are failing #129820

Closed

Max Charlamb and others added 3 commits July 1, 2026 12:45

Copilot AI review requested due to automatic review settings July 1, 2026 17:17

max-charlamb force-pushed the dev/max-charlamb/rts-synthetic-typehandles branch from 791163a to ee5fa4e Compare July 1, 2026 17:17

Copilot started reviewing on behalf of max-charlamb July 1, 2026 17:18 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Copilot AI review requested due to automatic review settings July 1, 2026 19:26

max-charlamb force-pushed the dev/max-charlamb/rts-synthetic-typehandles branch from c6446df to 5c47052 Compare July 1, 2026 19:26

Copilot started reviewing on behalf of max-charlamb July 1, 2026 19:27 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

max-charlamb force-pushed the dev/max-charlamb/rts-synthetic-typehandles branch from 5c47052 to 0424998 Compare July 1, 2026 20:26

		// In the future we may want to return a ITypeHandle instead of a MethodTable, and modify SOS to do more complete pretty-printing.
		// DAC equivalent: src/coreclr/vm/typehandle.inl ITypeHandle::GetMethodTable

Uh oh!

Conversation

max-charlamb commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Motivation

Uh oh!

dotnet-policy-service Bot commented Jun 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

rcj1 commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

max-charlamb commented Jun 24, 2026

Uh oh!

rcj1 commented Jun 24, 2026

Uh oh!

max-charlamb commented Jun 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

rcj1 commented Jun 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

This comment has been minimized.

github-actions Bot commented Jul 1, 2026

Copilot Code Review

Holistic Assessment

Detailed Findings

⚠️ Boxing allocation on every GetTypeHandle call

⚠️ GetInstantiation / GetGenericMethodInstantiation return mutable cached arrays

⚠️ Generator emits method named ITypeHandle

⚠️ Missing unit tests for synthetic type handles

💡 Comment references to native ITypeHandle::GetMethodTable

✅ Synthetic-aware method routing is correct

✅ CallingConvention cleanup is a clear improvement

✅ Equality implementation is symmetric

✅ cDAC API surface changes do not require API review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

max-charlamb commented Jun 24, 2026 •

edited

Loading

rcj1 commented Jun 24, 2026 •

edited

Loading

⚠️ Boxing allocation on every `GetTypeHandle` call

⚠️ `GetInstantiation` / `GetGenericMethodInstantiation` return mutable cached arrays

⚠️ Generator emits method named `ITypeHandle`

💡 Comment references to native `ITypeHandle::GetMethodTable`