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

[Release/5.0] Fix covariant returns when overriding method of non-parent ancestor #47937

Merged

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Feb 5, 2021

Port #47846 to release/5.0

There is a problem in the
ClassLoader::ValidateMethodsWithCovariantReturnTypes that results in
failed verification of valid override in case the return type of the
method being overriden is generic in canonical form and it is defined
in an ancestor class that is not the parent.

The problem is that we attempt to use instantiation of the parent class
instead of the ancestor class that contains definition of the method
being overriden.

This change fixes it by locating the proper ancestor MethodTable and
using it.

Customer Impact

Valid application that uses multiple generic type arguments in covariant return override where some of them but not all are canonical crashes at runtime with System.TypeLoadException when attempting to use the underlying type.

Testing

CoreCLR pri1 tests, including the new test added that specifically target the problematic cases (copy of the repro provided by our customer that have found the issue)

Risk

Low

Regression

No

@janvorli janvorli added Servicing-consider Issue for next servicing release review area-TypeSystem-coreclr labels Feb 5, 2021
@janvorli janvorli added this to the 5.0.x milestone Feb 5, 2021
@janvorli janvorli self-assigned this Feb 5, 2021
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. Please get a cr and I will take it for consideration.

@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 9, 2021
@leecow leecow modified the milestones: 5.0.x, 5.0.4 Feb 9, 2021
@Anipik
Copy link
Contributor

Anipik commented Feb 10, 2021

@janvorli can you address the merge conflicts here ?

@janvorli
Copy link
Member Author

Sure, I'll resolve it in a minute.

…ent ancestor

There is a problem in the
ClassLoader::ValidateMethodsWithCovariantReturnTypes that results in
failed verification of valid override in case the return type of the
method being overriden is generic in canonical form and it is defined
in an ancestor class that is not the parent.

The problem is that we attempt to use instantiation of the parent class
instead of the ancestor class that contains definition of the method
being overriden.

This change fixes it by locating the proper ancestor MethodTable and
using it.
@janvorli janvorli force-pushed the port-fix-covariant-returns-issue-4 branch from 7384f76 to 43a8b88 Compare February 10, 2021 16:00
@janvorli
Copy link
Member Author

@Anipik done

@janvorli
Copy link
Member Author

It looks like brew installation of dependencies is broken on OSX / iOS / tvOS:
Error: protected method `declared_runtime_dependencies' called for #Formulary::FormulaNamespace713c9f9d45fae5ea9f8b69224785c4fa::Automake:0x00007f9b0b46a0e8

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-TypeSystem-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants