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

SweepStep doesn't process all type references for forwarded types #1576

Closed
rolfbjarne opened this issue Oct 20, 2020 · 0 comments · Fixed by #1683
Closed

SweepStep doesn't process all type references for forwarded types #1576

rolfbjarne opened this issue Oct 20, 2020 · 0 comments · Fixed by #1683

Comments

@rolfbjarne
Copy link
Member

Test case:
linkertestcase-aedcbaa.zip

Extract and execute testcase.sh (not test.sh):

$ ./testcase.sh
++ dirname ./testcase.sh
+ cd .
+ LINKER=/usr/local/share/dotnet/sdk/5.0.100-rc.2.20480.7/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/illink.dll
+ /usr/local/share/dotnet/dotnet /usr/local/share/dotnet/sdk/5.0.100-rc.2.20480.7/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/illink.dll @testcase.rsp --custom-step -LoadReferencesStep:Xamarin.SetupStep,/Users/rolf/test/linkertestcase/dotnet-linker.dll --custom-data LinkerOptionsFile=/Users/rolf/test/linkertestcase/custom-linker-options.txt
[...]
+ monodis './obj/iPhoneSimulator/Debug/net5.0-ios/ios-x64/linked/link all.dll'
+ grep BROKEN
	IL_0020:  call instance void [Xamarin.iOS]ObjCRuntime.BlockLiteral::SetupBlockImpl(<BROKEN CLASS token_ 1000024 due to Could not resolve type with token 01000024 from typeref (expected class 'System.Delegate' in assembly 'System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a') assembly:System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a type:System.Delegate member:(null)>, <BROKEN CLASS token_ 1000024 due to Could not resolve type with token 01000024 from typeref (expected class 'System.Delegate' in assembly 'System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a') assembly:System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a type:System.Delegate member:(null)>, bool, string)
	IL_0020:  call instance void [Xamarin.iOS]ObjCRuntime.BlockLiteral::SetupBlockImpl(<BROKEN CLASS token_ 1000024 due to Could not resolve type with token 01000024 from typeref (expected class 'System.Delegate' in assembly 'System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a') assembly:System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a type:System.Delegate member:(null)>, <BROKEN CLASS token_ 1000024 due to Could not resolve type with token 01000024 from typeref (expected class 'System.Delegate' in assembly 'System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a') assembly:System.Collections, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a type:System.Delegate member:(null)>, bool, string)
[...]
+ echo '❌ FAILED'
❌ FAILED
+ exit 1

The problem occurs when one of our custom linker step rewrites IL to call a different method: https://github.com/xamarin/xamarin-macios/blob/699c1f98fbfb9ac1c5d790a99812aa2f2b1a001b/tools/linker/CoreOptimizeGeneratedCode.cs#L911-L918

The different method is imported into the module in question here: https://github.com/xamarin/xamarin-macios/blob/699c1f98fbfb9ac1c5d790a99812aa2f2b1a001b/tools/linker/CoreOptimizeGeneratedCode.cs#L1058-L1073

The problem occurs because the imported method has a parameter whose type is System.Delegate, from System.Runtime.dll, and when System.Runtime.dll is linked away, the corresponding TypeReference in this method's parameter's ParameterType is not updated in SweepStep to reference System.Private.CoreLib.dll instead.

The end result is an invalid assembly (as can be seen from the monodis output) where the imported method's parameter's type is apparently System.Delegate from System.Collections.dll.

Here is an ugly hack that fixes the problem: https://gist.github.com/rolfbjarne/45e8e6d001f5fe9c9c1cf4036ad212f6

@marek-safar marek-safar added this to the .NET 6.0 milestone Oct 20, 2020
marek-safar added a commit to marek-safar/linker that referenced this issue Dec 10, 2020
The previous logic only removed assembly reference for linked assembly
if the reference didn't exist in the closure of input assemblies. This
was wrong because it kept unused references in one assembly if the
another assembly used same reference.

Fixes dotnet#1296
Fixes dotnet#1576
marek-safar added a commit to marek-safar/linker that referenced this issue Dec 10, 2020
The previous logic only removed assembly reference for linked assembly
if the reference didn't exist in the closure of input assemblies. This
was wrong because it kept unused references in one assembly if the
another assembly used same reference.

Fixes dotnet#1296
Fixes dotnet#1576
marek-safar added a commit to marek-safar/linker that referenced this issue Dec 10, 2020
The previous logic only removed assembly reference for linked assembly
if the reference didn't exist in the closure of input assemblies. This
was wrong because it kept unused references in one assembly if the
another assembly used same reference.

Fixes dotnet#1296
Fixes dotnet#1576
marek-safar added a commit to marek-safar/linker that referenced this issue Dec 10, 2020
The previous logic only removed assembly reference for linked assembly
if the reference didn't exist in the closure of input assemblies. This
was wrong because it kept unused references in one assembly if the
another assembly used same reference.

Fixes dotnet#1296
Fixes dotnet#1576
marek-safar added a commit to marek-safar/linker that referenced this issue Dec 13, 2020
The previous logic only removed assembly reference for linked assembly
if the reference didn't exist in the closure of input assemblies. This
was wrong because it kept unused references in one assembly if the
another assembly used same reference.

Fixes dotnet#1296
Fixes dotnet#1576
marek-safar added a commit to marek-safar/linker that referenced this issue Dec 16, 2020
The previous logic only removed assembly reference for linked assembly
if the reference didn't exist in the closure of input assemblies. This
was wrong because it kept unused references in one assembly if the
another assembly used same reference.

Fixes dotnet#1296
Fixes dotnet#1576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants