Skip to content

[cDAC] implement IXCLRDataMethodInstance::GetName and associated APIs - #118966

Merged
max-charlamb merged 14 commits into
dotnet:mainfrom
max-charlamb:cdac-methodinstance-getname
Sep 15, 2025
Merged

[cDAC] implement IXCLRDataMethodInstance::GetName and associated APIs#118966
max-charlamb merged 14 commits into
dotnet:mainfrom
max-charlamb:cdac-methodinstance-getname

Conversation

@max-charlamb

@max-charlambmax-charlamb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Implements:

  • IXCLRDataStackWalk::GetFrame
  • IXCLRDataFrame::GetMethodInstance
  • IXCLRDataMethodInstance::GetName

The former two are required to verify GetName as SOS uses IXCLRDataStackWalk::GetFrame -> IXCLRDataFrame::GetMethodInstance to get the instantiation of IXCLRDataMethodInstance it calls GetName` on.

Modifies:

  • IDacStreams_1.StringFromEEAddress to not throw on read failure. Previously we wrapped this API in a try/catch, given it can return a null value, I believe it is acceptable to return null on read failures.

CopilotAI review requested due to automatic review settings August 21, 2025 18:38

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 the IXCLRDataMethodInstance::GetName method in the cDAC (contract-based Data Access Component) implementation, replacing a previously unimplemented stub that delegated to the legacy implementation.

Key changes:

  • Implements method name generation using TypeNameBuilder.AppendMethodInternal with fallback mechanisms
  • Adds error handling and debug validation against the legacy implementation
  • Improves robustness of string retrieval from EE addresses

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

FileDescription
ClrDataMethodInstance.csImplements GetName method with proper name formatting, fallback logic, and debug validation
SOSDacImpl.csRemoves nested try-catch block in GetMethodTableName method
DacStreams_1.csAdds exception handling for VirtualReadException in StringFromEEAddress method

@max-charlamb
max-charlamb marked this pull request as draft August 22, 2025 14:29
@max-charlamb
max-charlambforce-pushed the cdac-methodinstance-getname branch from 54a82b9 to 84bc116CompareAugust 22, 2025 14:34
@max-charlambmax-charlamb changed the title [cDAC] implement IXCLRDataMethodInstance::GetName[cDAC] implement IXCLRDataMethodInstance::GetName and associated APIsSep 2, 2025
Comment threaddocs/design/datacontracts/StackWalk.md Outdated
Comment threadsrc/native/managed/cdac/mscordaccore_universal/Legacy/ClrDataFrame.cs Outdated
max-charlamband others added 2 commits September 2, 2025 19:53
Co-authored-by: Noah Falk <noahfalk@users.noreply.github.com>
@max-charlamb
max-charlamb marked this pull request as draft September 10, 2025 14:12
@max-charlamb
max-charlamb marked this pull request as ready for review September 10, 2025 16:54

@rcj1rcj1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise

@max-charlamb
max-charlamb merged commit 365fdd0 into dotnet:mainSep 15, 2025
50 checks passed
@max-charlamb
max-charlamb deleted the cdac-methodinstance-getname branch September 15, 2025 21:41
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Oct 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@max-charlamb@noahfalk@rcj1