From 70cd7ede82e8ba9192bbd8e63fd7c46cbeba0c21 Mon Sep 17 00:00:00 2001 From: Vitek Karas Date: Wed, 17 Nov 2021 20:34:14 +0100 Subject: [PATCH 1/2] Adds a test for multiple nested forwarders in a row with copy/link (#2372) The problematic scenario is if there's a "copy" assembly with a forwarder to a "link" assembly with a forwarder. The forwarder is for a nested class. Test for https://github.com/dotnet/linker/issues/2359. Also improved the test infra to print out errors from ilasm if they happen. --- .../Dependencies/ImplementationLibrary.cs | 11 +++++ .../Dependencies/NestedForwarderLibrary.il | 31 +++++++++++++ .../Dependencies/NestedForwarderLibrary_2.il | 31 +++++++++++++ .../ReferenceImplementationLibrary.cs | 11 +++++ .../MultiForwardedTypesWithCopyUsed.cs | 44 +++++++++++++++++++ .../MultiForwardedTypesWithLink.cs | 44 +++++++++++++++++++ .../TestCasesRunner/ILCompiler.cs | 6 ++- 7 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il create mode 100644 test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il create mode 100644 test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs create mode 100644 test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ImplementationLibrary.cs b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ImplementationLibrary.cs index 211e2d14ffe9..6ba55597001c 100644 --- a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ImplementationLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ImplementationLibrary.cs @@ -25,6 +25,10 @@ public class ImplementationLibraryNestedType public static int PropertyOnNestedType { get; set; } } + public class ForwardedNestedType + { + } + public static int someField = 42; public string GetSomeValue () @@ -33,6 +37,13 @@ public string GetSomeValue () } } + public class AnotherImplementationClass + { + public class ForwardedNestedType + { + } + } + [AttributeUsage (AttributeTargets.All)] public class ImplementationLibraryAttribute : Attribute { diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il new file mode 100644 index 000000000000..edb8ded478bc --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il @@ -0,0 +1,31 @@ +.assembly extern mscorlib +{ +} + +.assembly extern Implementation +{ +} + +.assembly NestedForwarderLibrary +{ +} + +.class extern forwarder Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary +{ + .assembly extern 'Implementation' +} +.class extern ForwardedNestedType +{ + .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary' +} + +.class extern forwarder Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.AnotherImplementationClass +{ + .assembly extern 'Implementation' +} +.class extern ForwardedNestedType +{ + .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary' +} + +.module 'NestedForwarderLibrary.dll' diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il new file mode 100644 index 000000000000..2b0faefc622e --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il @@ -0,0 +1,31 @@ +.assembly extern mscorlib +{ +} + +.assembly extern NestedForwarderLibrary +{ +} + +.assembly NestedForwarderLibrary_2 +{ +} + +.class extern forwarder Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary +{ + .assembly extern 'NestedForwarderLibrary' +} +.class extern ForwardedNestedType +{ + .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary' +} + +.class extern forwarder Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.AnotherImplementationClass +{ + .assembly extern 'NestedForwarderLibrary' +} +.class extern ForwardedNestedType +{ + .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary' +} + +.module 'NestedForwarderLibrary_2.dll' diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ReferenceImplementationLibrary.cs b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ReferenceImplementationLibrary.cs index 4ec4203724d1..ac08d817a4bc 100644 --- a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ReferenceImplementationLibrary.cs +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/ReferenceImplementationLibrary.cs @@ -27,6 +27,10 @@ public class ImplementationLibraryNestedType public static int PropertyOnNestedType { get; set; } } + public class ForwardedNestedType + { + } + public static int someField = 0; public string GetSomeValue () @@ -35,6 +39,13 @@ public string GetSomeValue () } } + public class AnotherImplementationClass + { + public class ForwardedNestedType + { + } + } + [AttributeUsage (AttributeTargets.All)] public class ImplementationLibraryAttribute : Attribute { diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs new file mode 100644 index 000000000000..e2e5c39bfbbd --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; +using Mono.Linker.Tests.Cases.TypeForwarding.Dependencies; + +namespace Mono.Linker.Tests.Cases.TypeForwarding +{ + [SkipUnresolved (true)] + + [SetupCompileBefore ("NestedForwarderLibrary_2.dll", new[] { "Dependencies/ReferenceImplementationLibrary.cs" }, defines: new[] { "INCLUDE_REFERENCE_IMPL" })] + + // After compiling the test case we then replace the reference impl with implementation + type forwarder + [SetupCompileAfter ("Implementation.dll", new[] { "Dependencies/ImplementationLibrary.cs" })] + [SetupCompileAfter ("NestedForwarderLibrary.dll", new[] { "Dependencies/NestedForwarderLibrary.il" }, references: new[] { "Implementation.dll" })] + [SetupCompileAfter ("NestedForwarderLibrary_2.dll", new[] { "Dependencies/NestedForwarderLibrary_2.il" }, references: new[] { "NestedForwarderLibrary.dll" })] + + [SetupLinkerAction ("copy", "test")] + [SetupLinkerAction ("copy", "NestedForwarderLibrary_2")] + [SetupLinkerAction ("copyused", "NestedForwarderLibrary")] + [SetupLinkerAction ("copyused", "Implementation")] + + // https://github.com/dotnet/linker/issues/2359 + // One of the type forwarders in NestedForwarderLibrary will not be kept. + // Which one depends on order. + //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + [KeptTypeInAssembly ("Implementation.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + [KeptTypeInAssembly ("Implementation.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + + [KeptMember (".ctor()")] + class MultiForwardedTypesWithCopyUsed + { + static void Main () + { + Console.WriteLine (typeof (ImplementationLibrary.ForwardedNestedType).FullName); + Console.WriteLine (typeof (AnotherImplementationClass.ForwardedNestedType).FullName); + } + } +} diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs new file mode 100644 index 000000000000..2d61e1e6b6d4 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; +using Mono.Linker.Tests.Cases.TypeForwarding.Dependencies; + +namespace Mono.Linker.Tests.Cases.TypeForwarding +{ + [SkipUnresolved (true)] + + [SetupCompileBefore ("NestedForwarderLibrary_2.dll", new[] { "Dependencies/ReferenceImplementationLibrary.cs" }, defines: new[] { "INCLUDE_REFERENCE_IMPL" })] + + // After compiling the test case we then replace the reference impl with implementation + type forwarder + [SetupCompileAfter ("Implementation.dll", new[] { "Dependencies/ImplementationLibrary.cs" })] + [SetupCompileAfter ("NestedForwarderLibrary.dll", new[] { "Dependencies/NestedForwarderLibrary.il" }, references: new[] { "Implementation.dll" })] + [SetupCompileAfter ("NestedForwarderLibrary_2.dll", new[] { "Dependencies/NestedForwarderLibrary_2.il" }, references: new[] { "NestedForwarderLibrary.dll" })] + + [SetupLinkerAction ("copy", "test")] + [SetupLinkerAction ("copy", "NestedForwarderLibrary_2")] + [SetupLinkerAction ("link", "NestedForwarderLibrary")] + [SetupLinkerAction ("link", "Implementation")] + + // https://github.com/dotnet/linker/issues/2359 + // One of the type forwarders in NestedForwarderLibrary will not be kept. + // Which one depends on order. + //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + [KeptTypeInAssembly ("Implementation.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + [KeptTypeInAssembly ("Implementation.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + + [KeptMember (".ctor()")] + class MultiForwardedTypesWithLink + { + static void Main () + { + Console.WriteLine (typeof (ImplementationLibrary.ForwardedNestedType).FullName); + Console.WriteLine (typeof (AnotherImplementationClass.ForwardedNestedType).FullName); + } + } +} diff --git a/test/Mono.Linker.Tests/TestCasesRunner/ILCompiler.cs b/test/Mono.Linker.Tests/TestCasesRunner/ILCompiler.cs index 52e876694d48..f2aba174ddf6 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/ILCompiler.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/ILCompiler.cs @@ -15,16 +15,20 @@ public class ILCompiler public NPath Compile (CompilerOptions options) { var capturedOutput = new List (); + var capturedError = new List (); var process = new Process (); SetupProcess (process, options); process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; process.OutputDataReceived += (sender, args) => capturedOutput.Add (args.Data); + process.ErrorDataReceived += (sender, args) => capturedError.Add (args.Data); process.Start (); process.BeginOutputReadLine (); + process.BeginErrorReadLine (); process.WaitForExit (); if (process.ExitCode != 0) { - Assert.Fail ($"Failed to compile IL assembly : {options.OutputPath}\n{capturedOutput.Aggregate ((buff, s) => buff + Environment.NewLine + s)}"); + Assert.Fail ($"Failed to compile IL assembly : {options.OutputPath}\n{capturedOutput.Aggregate ((buff, s) => buff + Environment.NewLine + s)}{capturedError.Aggregate ((buff, s) => buff + Environment.NewLine + s)}"); } return options.OutputPath; From 0e2d10c0412279d246f89e316b61d2584f456545 Mon Sep 17 00:00:00 2001 From: Vitek Karas Date: Mon, 22 Nov 2021 14:50:31 +0100 Subject: [PATCH 2/2] Fix marking of nested type forwarders (#2385) When create a type reference for the target of a type forwarder, if the type forwarder is for a nested type, we have to build a whole tree of type references for all of the declaring types and not just the final nested type. Enabled tests which were already added for this case (and fixed a bug in them) --- src/linker/Linker.Steps/MarkStep.cs | 14 +++++++++++++- .../Dependencies/NestedForwarderLibrary.il | 2 +- .../Dependencies/NestedForwarderLibrary_2.il | 2 +- .../MultiForwardedTypesWithCopyUsed.cs | 11 ++++------- .../TypeForwarding/MultiForwardedTypesWithLink.cs | 11 ++++------- 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 524c9195e300..adbd13b7be86 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -1441,7 +1441,7 @@ protected override void ProcessTypeReference (TypeReference type) protected override void ProcessExportedType (ExportedType exportedType) { markingHelpers.MarkExportedType (exportedType, assembly.MainModule, new DependencyInfo (DependencyKind.ExportedType, assembly)); - markingHelpers.MarkForwardedScope (new TypeReference (exportedType.Namespace, exportedType.Name, assembly.MainModule, exportedType.Scope)); + markingHelpers.MarkForwardedScope (CreateTypeReferenceForExportedTypeTarget (exportedType)); } protected override void ProcessExtra () @@ -1454,6 +1454,18 @@ protected override void ProcessExtra () markingHelpers.MarkForwardedScope (typeReference); } } + + TypeReference CreateTypeReferenceForExportedTypeTarget (ExportedType exportedType) + { + TypeReference? declaringTypeReference = null; + if (exportedType.DeclaringType != null) { + declaringTypeReference = CreateTypeReferenceForExportedTypeTarget (exportedType.DeclaringType); + } + + return new TypeReference (exportedType.Namespace, exportedType.Name, assembly.MainModule, exportedType.Scope) { + DeclaringType = declaringTypeReference + }; + } } void ProcessModuleType (AssemblyDefinition assembly) diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il index edb8ded478bc..572248fab454 100644 --- a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary.il @@ -25,7 +25,7 @@ } .class extern ForwardedNestedType { - .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary' + .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.AnotherImplementationClass' } .module 'NestedForwarderLibrary.dll' diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il index 2b0faefc622e..84c16d84cf43 100644 --- a/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/Dependencies/NestedForwarderLibrary_2.il @@ -25,7 +25,7 @@ } .class extern ForwardedNestedType { - .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.ImplementationLibrary' + .class extern 'Mono.Linker.Tests.Cases.TypeForwarding.Dependencies.AnotherImplementationClass' } .module 'NestedForwarderLibrary_2.dll' diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs index e2e5c39bfbbd..a75bce83325f 100644 --- a/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithCopyUsed.cs @@ -22,13 +22,10 @@ namespace Mono.Linker.Tests.Cases.TypeForwarding [SetupLinkerAction ("copyused", "NestedForwarderLibrary")] [SetupLinkerAction ("copyused", "Implementation")] - // https://github.com/dotnet/linker/issues/2359 - // One of the type forwarders in NestedForwarderLibrary will not be kept. - // Which one depends on order. - //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (ImplementationLibrary.ForwardedNestedType))] - //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] - //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (ImplementationLibrary.ForwardedNestedType))] - //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] [KeptTypeInAssembly ("Implementation.dll", typeof (ImplementationLibrary.ForwardedNestedType))] [KeptTypeInAssembly ("Implementation.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] diff --git a/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs index 2d61e1e6b6d4..6500443d5855 100644 --- a/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs +++ b/test/Mono.Linker.Tests.Cases/TypeForwarding/MultiForwardedTypesWithLink.cs @@ -22,13 +22,10 @@ namespace Mono.Linker.Tests.Cases.TypeForwarding [SetupLinkerAction ("link", "NestedForwarderLibrary")] [SetupLinkerAction ("link", "Implementation")] - // https://github.com/dotnet/linker/issues/2359 - // One of the type forwarders in NestedForwarderLibrary will not be kept. - // Which one depends on order. - //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (ImplementationLibrary.ForwardedNestedType))] - //[KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] - //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (ImplementationLibrary.ForwardedNestedType))] - //[KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (ImplementationLibrary.ForwardedNestedType))] + [KeptTypeInAssembly ("NestedForwarderLibrary_2.dll", typeof (AnotherImplementationClass.ForwardedNestedType))] [KeptTypeInAssembly ("Implementation.dll", typeof (ImplementationLibrary.ForwardedNestedType))] [KeptTypeInAssembly ("Implementation.dll", typeof (AnotherImplementationClass.ForwardedNestedType))]