Simple RuntimeTypeSystem DacDbi APIs by rcj1 · Pull Request #126732 · dotnet/runtime

rcj1 · 2026-04-09T23:08:57Z

Implementing DacDbi APIs HasTypeParams, GetTypeHandle and GetThreadStaticAddress that use and require minimal modifications to the RuntimeTypeSystem cDAC contract.

dotnet-policy-service · 2026-04-09T23:10:07Z

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

Adds minimal RuntimeTypeSystem contract surface and corresponding Legacy DacDbi/SOS implementations to support a few simple DacDbi APIs (HasTypeParams, GetTypeHandle, GetThreadStaticAddress) using cDAC data.

Changes:

Implement DacDbiImpl.HasTypeParams, GetTypeHandle, and GetThreadStaticAddress against cDAC contracts (with DEBUG cross-validation vs legacy DAC when available).
Extend RuntimeTypeSystem contract with ContainsGenericVariables and GetFieldDescThreadStaticAddress, plus supporting MethodTable flag plumbing.
Consolidate ECMA token helpers into EcmaMetadataUtils and update docs/HResults accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs	Replaces local metadata token enum usage with shared `EcmaMetadataUtils` token helpers.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs	Implements the three requested DacDbi APIs via cDAC contracts with DEBUG parity checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.cs	Adds `ContainsGenericVariables` MethodTable flag bit support.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/EcmaMetadataUtils.cs	Makes token utilities publicly reusable (token type enum + mask).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs	Implements `ContainsGenericVariables` and adds thread-static address computation API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CorDbHResults.cs	Adds HRESULT constant for “class not loaded”.
src/native/managed/cdac/docs/design/datacontracts/RuntimeTypeSystem.md	Documents the new RuntimeTypeSystem APIs and static/thread-static address helpers.

Copilot · 2026-04-09T23:14:53Z

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CorDbHResults.cs

    public const int CORDBG_E_NOTREADY = unchecked((int)0x80131c10);
    public const int CORDBG_E_READVIRTUAL_FAILURE = unchecked((int)0x80131c49);
    public const int ERROR_BUFFER_OVERFLOW = unchecked((int)0x8007006F); // HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW)
+    public const int CLASS_NOT_LOADED = unchecked((int)0x80131303);


CorDbgHResults constants use CORDBG_E_* naming (e.g., CORDBG_E_NOTREADY, CORDBG_E_READVIRTUAL_FAILURE). The newly added CLASS_NOT_LOADED breaks that convention and is easy to misread as a generic error; consider renaming it to CORDBG_E_CLASS_NOT_LOADED (and update call sites) to match the actual HRESULT name used in CoreCLR (CORDBG_E_CLASS_NOT_LOADED).

Suggested change

public const int CLASS_NOT_LOADED = unchecked((int)0x80131303);

public const int CORDBG_E_CLASS_NOT_LOADED = unchecked((int)0x80131303);

Copilot · 2026-04-09T23:14:53Z

docs/design/datacontracts/RuntimeTypeSystem.md

@@ -80,6 +80,7 @@ partial interface IRuntimeTypeSystem : IContract
    public virtual bool IsGenericTypeDefinition(TypeHandle typeHandle);

    public virtual bool IsCollectible(TypeHandle typeHandle);


ContainsGenericVariables is introduced as a contract API here, and the implementation relies on the MethodTable flag bit (enum_flag_ContainsGenericVariables = 0x20000000). The “Version 1” section’s WFLAGS_HIGH listing below should be updated to include this flag (and its value) so the contract documentation matches the contract implementation.

Suggested change

public virtual bool IsCollectible(TypeHandle typeHandle);

public virtual bool IsCollectible(TypeHandle typeHandle);

// True if the MethodTable has enum_flag_ContainsGenericVariables (0x20000000) set in WFLAGS_HIGH.

Copilot · 2026-04-09T23:14:54Z

...ged/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs

    public bool IsGenericTypeDefinition(TypeHandle typeHandle) => !typeHandle.IsMethodTable() ? false : _methodTables[typeHandle.Address].Flags.IsGenericTypeDefinition;
+    public bool ContainsGenericVariables(TypeHandle typeHandle)
+    {
+        if (typeHandle.IsTypeDesc())
+        {
+            CorElementType type = GetSignatureCorElementType(typeHandle);
+            if (type == CorElementType.Var || type == CorElementType.MVar)
+                return true;
+
+            else if (HasTypeParam(typeHandle))
+            {
+                return ContainsGenericVariables(GetRootTypeParam(typeHandle));
+            }
+
+            else if (type == CorElementType.FnPtr)
+            {
+                _ = IsFunctionPointer(typeHandle, out ReadOnlySpan<TypeHandle> signatureTypeArgs, out _);
+                foreach (TypeHandle sigTypeArg in signatureTypeArgs)
+                {
+                    if (ContainsGenericVariables(sigTypeArg))
+                        return true;
+                }
+            }
+
+            return false;
+        }
+        return _methodTables[typeHandle.Address].Flags.ContainsGenericVariables;
+    }


This adds a new ContainsGenericVariables API with non-trivial recursion over TypeDescs (Var/MVar, parameterized types, FnPtr signatures) and MethodTable flags, but there are no unit tests covering these new cases. Please add targeted tests (likely in TypeDescTests/MethodTableTests) to validate expected results for Var/MVar, Ptr/ByRef chains, FnPtr signatures containing generic variables, and a MethodTable with/without the flag.

Copilot · 2026-04-09T23:14:54Z

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

    public int HasTypeParams(ulong vmTypeHandle, Interop.BOOL* pResult)
-        => _legacy is not null ? _legacy.HasTypeParams(vmTypeHandle, pResult) : HResults.E_NOTIMPL;
+    {
+        *pResult = Interop.BOOL.FALSE;
+        int hr = HResults.S_OK;
+        try
+        {
+            Contracts.IRuntimeTypeSystem rts = _target.Contracts.RuntimeTypeSystem;
+            TypeHandle typeHandle = rts.GetTypeHandle(new TargetPointer(vmTypeHandle));
+            *pResult = rts.ContainsGenericVariables(typeHandle) ? Interop.BOOL.TRUE : Interop.BOOL.FALSE;
+        }


These new cDAC-backed implementations should have dump-based integration coverage to ensure they match real runtime behavior. Please add tests under src/native/managed/cdac/tests/DumpTests/DacDbi/ that validate HasTypeParams for at least one open generic type and one closed generic type (and verify HRESULT behavior).

Copilot · 2026-04-09T23:14:54Z

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

    public int GetTypeHandle(ulong vmModule, uint metadataToken, ulong* pRetVal)
-        => _legacy is not null ? _legacy.GetTypeHandle(vmModule, metadataToken, pRetVal) : HResults.E_NOTIMPL;
+    {
+        *pRetVal = 0;
+        int hr = HResults.S_OK;
+        try
+        {
+            Contracts.ILoader loader = _target.Contracts.Loader;
+            TargetPointer module = new TargetPointer(vmModule);
+            Contracts.ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(module);
+            Contracts.ModuleLookupTables lookupTables = loader.GetLookupTables(moduleHandle);
+            switch ((EcmaMetadataUtils.TokenType)(metadataToken & EcmaMetadataUtils.TokenTypeMask))
+            {
+                case EcmaMetadataUtils.TokenType.mdtTypeDef:
+                    *pRetVal = loader.GetModuleLookupMapElement(lookupTables.TypeDefToMethodTable, metadataToken, out var _).Value;
+                    break;
+                case EcmaMetadataUtils.TokenType.mdtTypeRef:
+                    *pRetVal = loader.GetModuleLookupMapElement(lookupTables.TypeRefToMethodTable, metadataToken, out var _).Value;
+                    break;
+                default:
+                    throw Marshal.GetExceptionForHR(CorDbgHResults.CLASS_NOT_LOADED)!;
+            }


GetTypeHandle is now implemented via Loader lookup tables and returns CLASS_NOT_LOADED for unsupported token types. Please add dump-based integration tests (under DumpTests/DacDbi) that exercise a known mdTypeDef and (if applicable) a mdTypeRef, and validate that an unloaded/invalid token produces the expected HRESULT.

Copilot · 2026-04-09T23:14:54Z

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

+                throw new ArgumentException("vmRuntimeThread cannot be null for thread static fields");
+            if (!rts.IsFieldDescThreadStatic(fd))
+            {
+                throw new NotImplementedException();
+            }


GetThreadStaticAddress now depends on the new IRuntimeTypeSystem.GetFieldDescThreadStaticAddress contract API and has distinct failure modes (E_INVALIDARG for null thread, E_NOTIMPL for non-thread-static field). Please add dump-based integration tests to cover a real thread-static field, plus the failure cases, to prevent regressions across contract versions.

Suggested change

throw new ArgumentException("vmRuntimeThread cannot be null for thread static fields");

if (!rts.IsFieldDescThreadStatic(fd))

{

throw new NotImplementedException();

}

throw new COMException("vmRuntimeThread cannot be null for thread static fields", HResults.E_INVALIDARG);

if (!rts.IsFieldDescThreadStatic(fd))

{

throw new COMException("Field is not thread static", HResults.E_NOTIMPL);

}

Copilot · 2026-04-09T23:14:54Z

...ged/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs

        TargetPointer @base;
        if (type == CorElementType.Class || type == CorElementType.ValueType)
        {
-            @base = GetGCStaticsBasePointer(ctx);
+            if (thread.HasValue)
+            {
+                @base = GetGCThreadStaticsBasePointer(ctx, thread.Value);
+            }
+            else
+            {
+                @base = GetGCStaticsBasePointer(ctx);
+            }
        }
        else
        {
-            @base = GetNonGCStaticsBasePointer(ctx);
+            if (thread.HasValue)
+            {
+                @base = GetNonGCThreadStaticsBasePointer(ctx, thread.Value);
+            }
+            else
+            {
+                @base = GetNonGCStaticsBasePointer(ctx);
+            }
        }


GetGCThreadStaticsBasePointer/GetNonGCThreadStaticsBasePointer (and the non-thread equivalents) can legitimately return TargetPointer.Null when the inspection-only static storage hasn’t been allocated. This helper currently proceeds even when @base is null; later it will add the field offset to 0, which can surface as a bogus non-zero address to callers (notably for thread statics). Consider short-circuiting when @base == TargetPointer.Null (and the field isn’t an RVA) to return TargetPointer.Null, matching DAC behavior (“may commonly return NULL”).

noahfalk

Copilot had some good feedback but other than that it looked good to me

simple runtimetypesystem APIs

acdd115

rcj1 requested review from barosiak, Copilot, max-charlamb and noahfalk April 9, 2026 23:08

github-actions bot added the area-Diagnostics-coreclr label Apr 9, 2026

dotnet-policy-service bot assigned rcj1 Apr 9, 2026

Copilot started reviewing on behalf of rcj1 April 9, 2026 23:10 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

noahfalk approved these changes Apr 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simple RuntimeTypeSystem DacDbi APIs#126732

Simple RuntimeTypeSystem DacDbi APIs#126732
rcj1 wants to merge 1 commit intodotnet:mainfrom
rcj1:simple-rts

rcj1 commented Apr 9, 2026

Uh oh!

dotnet-policy-service bot commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

noahfalk left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	public const int CLASS_NOT_LOADED = unchecked((int)0x80131303);
	public const int CORDBG_E_CLASS_NOT_LOADED = unchecked((int)0x80131303);

		@@ -80,6 +80,7 @@ partial interface IRuntimeTypeSystem : IContract
		public virtual bool IsGenericTypeDefinition(TypeHandle typeHandle);

		public virtual bool IsCollectible(TypeHandle typeHandle);

	public virtual bool IsCollectible(TypeHandle typeHandle);
	public virtual bool IsCollectible(TypeHandle typeHandle);
	// True if the MethodTable has enum_flag_ContainsGenericVariables (0x20000000) set in WFLAGS_HIGH.

Conversation

rcj1 commented Apr 9, 2026

Uh oh!

dotnet-policy-service bot commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

noahfalk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants