Skip to content

[cDAC] Implement IXCLRDataProcess.EnumMethodInstancesByAddress #115131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 47 commits into from
Aug 11, 2025

Conversation

max-charlamb
Copy link
Contributor

@max-charlamb max-charlamb commented Apr 28, 2025

Contributes to #114336

Implements:
IXCLRDataProcess.StartEnumMethodInstancesByAddress
IXCLRDataProcess.EnumMethodInstancesByAddress
IXCLRDataProcess.EndEnumMethodInstancesByAddress
IXCLRDataMethodInstance.GetTokenAndScope
IXCLRDataMethodInstance.GetRepresentativeEntryAddress

These final two APIs allow me to test that enumerating the method instances return the correct COM object/data.

In order to implement these APIs, the following contracts were updated:

  • ILoader
    • IEnumerable<TargetPointer> GetAvailableTypeParams(ModuleHandle handle)
    • IEnumerable<TargetPointer> GetInstantiatedMethods(ModuleHandle handle)
  • IRuntimeTypeSystem
    • TargetPointer GetMethodDescForSlot(TypeHandle methodTable, ushort slot)
    • Refactored MethodDesc validation to allow skipping validation for known good MethodDesc pointers.

and the following data types were added:

  • EETypeHashTable to read available type parameters on a module
  • InstMethodHashTable to read instantiated methods in a module

Fixed small bug in DAC where status wasn't properly returned in ClrDataAccess::StartEnumMethodInstancesByAddress

Questions

  1. Where should I add MethodDesc::s_ClassificationSizeTable to the DAC Enum memory regions so it is present in all mini dumps?
    • Given the DAC already uses this when creating a minidump, it should be present.
  2. Should the RuntimeTypeSystem contract be updated here, or in another PR regarding the async changes? Do we need to do a contract bump?
    • Added extra flags for to support async

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 28, 2025
@max-charlamb max-charlamb added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 28, 2025
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

@Copilot Copilot AI review requested due to automatic review settings August 4, 2025 20:11
Copy link
Contributor

@Copilot 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 implements IXCLRDataProcess.EnumMethodInstancesByAddress and related APIs to support enumerating method instances for debugging scenarios. The implementation introduces new data contracts for reading instantiated methods and type parameters from modules, along with the necessary COM wrappers.

  • Implements the core enumeration APIs for method instances by address
  • Adds IXCLRDataMethodInstance COM interface with token/scope and entry address methods
  • Extends contracts to support reading hash tables for instantiated methods and available type parameters

Reviewed Changes

Copilot reviewed 27 out of 29 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs Adds new Module fields for AvailableTypeParams and InstMethodHashTable
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.MethodDescriptors.cs Adds dummy MethodDescSizeTable global for tests
src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.IXCLRDataProcess.cs Implements core enumeration logic and EnumMethodInstances helper class
src/native/managed/cdac/mscordaccore_universal/Legacy/IXCLRData.cs Defines IXCLRDataMethodInstance interface and updates method signatures
src/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataModule.cs Updates signature for EnumMethodInstanceByName
src/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataMethodInstance.cs Implements IXCLRDataMethodInstance COM wrapper
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs Minor formatting fix
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodDescFlags_1.cs Adds HasAsyncMethodData flag
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Module.cs Adds AvailableTypeParams and InstMethodHashTable fields
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/MethodDescChunk.cs Adds FirstMethodDesc calculated field
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InstMethodHashTable.cs New data type for reading instantiated method hash tables
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EETypeHashTable.cs New data type for reading available type parameter hash tables
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EEClass.cs Adds MethodDescChunk field
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/DacEnumerableHash.cs Base implementation for reading DAC enumerable hash tables
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs Implements GetMethodDescForSlot and GetIntroducedMethods
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs Implements GetAvailableTypeParams and GetInstantiatedMethods
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs Adds MethodDescSizeTable global
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs Adds new data types EETypeHashTable and InstMethodHashTable
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs Adds GetMethodDescForSlot method
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ILoader.cs Adds GetAvailableTypeParams and GetInstantiatedMethods methods
src/coreclr/vm/typehash.h Adds cdac_data template specialization for EETypeHashTable
src/coreclr/vm/instmethhash.h Adds cdac_data template specialization for InstMethodHashTable
src/coreclr/vm/dacenumerablehash.h Fixes typo and adds friend declaration for cdac_data
src/coreclr/vm/class.h Adds MethodDescChunk field to cdac_data template
src/coreclr/debug/runtimeinfo/datadescriptor.inc Adds data descriptors for new types and fields
docs/design/datacontracts/RuntimeTypeSystem.md Documents new APIs and data structures
docs/design/datacontracts/Loader.md Documents new hash table reading functionality
Comments suppressed due to low confidence (1)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.IXCLRDataProcess.cs:143

  • The field _appDomain should not be readonly since it's assigned in the constructor but accessed publicly. Either make it a property or remove readonly.
        public readonly TargetPointer _appDomain;

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.

4 participants