[cDAC] Implement GetMethodDefinitionByToken in ClrDataModule#127091
Merged
max-charlamb merged 3 commits intodotnet:mainfrom Apr 18, 2026
Merged
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
rcj1
approved these changes
Apr 17, 2026
noahfalk
approved these changes
Apr 17, 2026
GetMethodDefinitionByToken now returns a cDAC-managed ClrDataMethodDefinition wrapper. When SOS calls methods on it, those delegate to legacy but are blocked by the no-fallback check. Add the four downstream methods that are not yet implemented in the cDAC: - StartEnumInstances - GetName - SetCodeNotification - HasClassOrMethodInstantiation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/ba-g cDAC only change and cDAC legs pass |
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. Remove it from the no-fallback allowlist and add newly-exposed downstream methods.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 resultLegacyFallbackHelper.cs— RemovedGetMethodDefinitionByTokenfrom the no-fallback allowlist. Added fourIXCLRDataMethodDefinitionmethods that are now exposed as blocked fallbacks becauseGetMethodDefinitionByTokenreturns a cDAC-managed wrapper instead of a legacy object:StartEnumInstancesGetNameSetCodeNotificationHasClassOrMethodInstantiationThese methods are not yet implemented in the cDAC and must remain on the allowlist until they are.
Pattern
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 (#126752).