illink: Unhandled exception. Mono.Linker.InternalErrorException: Type ShareExtensionTest.ShareViewController already has applied All.

[rolfbjarne]

Description

Running illink on a specific .NET for macOS project crashes illink.

Reproduction Steps

Test repro: https://www.dropbox.com/scl/fi/hq6y8bgq6hdasi0tk0en2/illinkrepro-6708de8.zip?rlkey=9qcxbxkzmnyevo2t44p9euszy&dl=0

Download and run test.sh.

You'll probably have to open test.sh and modify the DOTNET and ILLINK paths at the top of the file for your local paths.

Expected behavior

Successful execution.

Actual behavior

Fatal error in IL Linker
Unhandled exception. Mono.Linker.InternalErrorException: Type ShareExtensionTest.ShareViewController already has applied All.
   at Mono.Linker.Steps.MarkStep.ApplyPreserveInfo(TypeDefinition type) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs:line 2798
   at Mono.Linker.Steps.MarkStep.ProcessType(TypeDefinition type) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs:line 2116
   at Mono.Linker.Steps.MarkStep.TypeDefinitionNode.GetStaticDependencies(NodeFactory context) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.TypeDefinitionNode.cs:line 31
   at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node) in /Users/rolf/work/dotnet/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 182
   at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependencies(DependencyNodeCore`1 node) in /Users/rolf/work/dotnet/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 222
   at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack() in /Users/rolf/work/dotnet/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 257
   at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /Users/rolf/work/dotnet/runtime/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 308
   at Mono.Linker.Steps.MarkStep.Process() in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs:line 371
   at Mono.Linker.Steps.MarkStep.Process(LinkContext context) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs:line 245
   at Mono.Linker.Pipeline.ProcessStep(LinkContext context, IStep step) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker/Pipeline.cs:line 178
   at Mono.Linker.Pipeline.Process(LinkContext context) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker/Pipeline.cs:line 167
   at Mono.Linker.Driver.Run(ILogger customLogger) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker/Driver.cs:line 837
   at Mono.Linker.Driver.Main(String[] args) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker/Driver.cs:line 68
   at Mono.Linker.Driver.Main(String[] args) in /Users/rolf/work/dotnet/runtime/src/tools/illink/src/linker/Linker/Driver.cs:line 72

Full output is inside the repro zip in testrunwithdebugstatement.txt.

Regression?

Yes.

It started happening in fbb67a6.

CC @jtschuster

Known Workarounds

No response

Configuration

No response

Other information

No response

[dotnet-policy-service]

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

[sbomer]

@jtschuster could you look into this?

This looks like a bad interaction with custom steps. Here's what I think is happening:

• Custom step somewhere calls SetPreserve on a marked type (https://github.com/search?q=repo%3Axamarin%2Fxamarin-macios%20SetPreserve&type=code)
• The type is marked, but not yet processed (we haven't called ProcessType yet).
• ProcessMarkedPending sees the preserve info and applies it for the type (sets applied to true).
• Later we call ProcessType for the TypeDefinitionNode - that's when we throw an exception, because the preserve info was already applied.

Before fbb67a6, MarkType would immediately process the type so there was no way for a custom step to "inject" preserve info between marking and processing of a type.

I think it's safe to change the throw to return early - it's now expected that we might try to re-apply preserve info for a type since either ProcessMarkedPending or ProcessType could get there first.