[cDAC] Implement GetMethodDefinitionByToken in ClrDataModule#127091
Open
max-charlamb wants to merge 2 commits intodotnet:mainfrom
Open
[cDAC] Implement GetMethodDefinitionByToken in ClrDataModule#127091max-charlamb wants to merge 2 commits intodotnet:mainfrom
max-charlamb wants to merge 2 commits intodotnet:mainfrom
Conversation
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements IXCLRDataModule.GetMethodDefinitionByToken in the managed cDAC ClrDataModule wrapper so it no longer relies solely on legacy DAC delegation, enabling progress toward stricter “no legacy fallback” coverage.
Changes:
- Replace the legacy-only delegation stub with a cDAC implementation that validates the token type and constructs a
ClrDataMethodDefinition. - When a legacy module is available, also invokes the legacy method to capture its result for DEBUG HRESULT parity validation.
Implement IXCLRDataModule.GetMethodDefinitionByToken in the cDAC ClrDataModule wrapper instead of delegating entirely to the legacy DAC. The implementation validates the token is an mdtMethodDef, retrieves the legacy method definition for DEBUG parity validation, and creates a ClrDataMethodDefinition with the token and module address. This follows the same pattern used by EnumMethodDefinitionByName and enables removing GetMethodDefinitionByToken from the cDAC no-fallback allowlist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that GetMethodDefinitionByToken is implemented in the cDAC, it no longer needs to be in the LegacyFallbackHelper allowlist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
max-charlamb
force-pushed
the
dev/max-charlamb/cdac-getmethoddefbytoken
branch
from
fb386ce to
e67c1fb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR was generated with the assistance of GitHub Copilot.
Summary
Implement
IXCLRDataModule.GetMethodDefinitionByTokenin the cDACClrDataModulewrapper instead of delegating entirely to the legacy DAC.Changes
ClrDataModule.cs— Replaced the one-line legacy delegation stub with a full cDAC implementation that:mdtMethodDef(throwsArgumentException→E_INVALIDARG)ClrDataMethodDefinitionwith the token, module address, and legacy method definition#if DEBUGValidateHResultcheck against the legacy resultPattern
Follows the same pattern used by
EnumMethodDefinitionByName(line 302) which already createsClrDataMethodDefinitionobjects in the same way.The C++ legacy implementation (
task.cpp:2215-2251) validates the token type, then callsClrDataMethodDefinition::NewFromModulewhich simply creates aClrDataMethodDefinition— the cDAC version mirrors this behavior.Motivation
This enables removing
GetMethodDefinitionByTokenfrom the cDAC no-fallback allowlist in #126752.