Skip to content
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

Fix crossgen2 incorrect handling of ldftn to external managed methods #38236

Merged
merged 5 commits into from
Jun 24, 2020

Conversation

davidwrighton
Copy link
Member

Fix issue in crossgen2 where external managed methods which are used as part of an ldftn instruction are referred to through a delay load import instead of a precode import.

The issue was that the special handling of external methods as a seperate import type stepped around the handling of external methods via precode or delay load.

This change

  • Renames LocalMethodImport to DelayLoadMethodImport
  • Removes ExternalMethodImport
  • Uses DelayLoadMethodImport and PrecodeMethodImport for all method importation
  • Enables DelayLoadMethodImport and PrecodeMethodImport to be used to refer to methods which are external.
  • Adds tests to cover these external sorts of imports to for creation of both delegates and function pointers

@davidwrighton
Copy link
Member Author

@dotnet/crossgen-contrib

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks great, thanks David for setting this right! I also appreciate your diligence in adding tons of new tests everywhere you hit a snag :-).

@cshung
Copy link
Member

cshung commented Jun 23, 2020

Perhaps we can close this one when this is done? #13395

@davidwrighton davidwrighton merged commit 59015b0 into dotnet:master Jun 24, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
@davidwrighton davidwrighton deleted the other_dll_ldftn branch April 20, 2021 17:42
Sign up for free to 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.

3 participants