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

Add test case for #3112 with pseudo-circular reference when base provides iface method #3120

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

jtschuster
Copy link
Member

Adds a minimal repro for #3112.

The scenario looks like this:

Types;
Base type B has method M.
Derived type D1 derives from B, and implements interface I1 with methods from B.
Derived type D2 derives from B, and implements interface I2 with methods from B.

Assembly A1 has B.
Assembly A2 has D1.
Assembly A3 has I1 and D2.
I2 can be in any assembly.

In the trimmer:
D1 is marked instantiated.
-> B is marked instantiated (as a base type)
-> D1's interfaces are marked, and the implementation methods are marked
-> B.M() is marked
-> I.M() is added as a base method for B.M()

Later, B.M() is passed to IsMethodNeededByInstantiatedTypeDueToPreservedScope()
-> Base methods of B.M() are iterated over
-> I.M() is one of the base methods, and is passed to IsMethodNeedeByTypeDueToPreservedScope()
-> Base methods of I.M() are required, triggering the entire assembly A3 to be mapped.
-> D2 is mapped
-> I2.M() is added as a base method of B.M() -- While base methods of B.M() is being iterated over

There is a psuedo-circular assembly reference with the way we build out TypeMapInfo
A1 (psuedo-)refers to A3 (when A2 is referenced) because B.M() impls I1.M()
A3 refers to A1 because D2 derives from B.

In main the same exact situation doesn't cause issues, and #3094 fixes this repro for .Net 7, but I don't think a similar situation is guaranteed to never happen. Since base methods can provide interface members, the list of base methods of a type is dependent on which assemblies have been processed by TypeMapInfo at the time GetBaseMethods is called.

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

I assume you tested that this breaks the compiler with shipping 7.0.

Looks good.

We should add this test to #3094 to validate that it really fixes it (and prevent future regressions).

@jtschuster
Copy link
Member Author

I assume you tested that this breaks the compiler with shipping 7.0.

Yes, it crashes the release/7.0 linker.

@jtschuster jtschuster merged commit 2172c79 into dotnet:main Nov 17, 2022
tlakollo pushed a commit to tlakollo/linker that referenced this pull request Dec 20, 2022
…e provides iface method (dotnet#3120)

Add test case for dotnet#3112 with pseudo-circular reference with ifaces

Commit migrated from dotnet@2172c79
tlakollo pushed a commit to tlakollo/runtime that referenced this pull request Dec 22, 2022
…hen base provides iface method (dotnet/linker#3120)

Add test case for dotnet/linker#3112 with pseudo-circular reference with ifaces

Commit migrated from dotnet/linker@2172c79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants