Mark .overrides from marked methods in a copy assembly (#82197)

Fixes #81746

Static abstract interface methods that aren't used are expected to be removed, and any references to them in the metadata of the overriders are expected to be removed in SweepStep. However, overrides may come from a 'copy' assembly, so their metadata won't get swept at all. We would still remove the static abstract method, but would leave the reference to it in the overrider's metadata, creating invalid metadata.

This fixes the issue by making sure all methods referenced in the .overrides metadata of method X are marked if method X is in a copy assembly, where previously we would postpone marking the .overrides if the base was static abstract.

diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
index ce7a505..4e7bda6 100644 (file)
--- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
+++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@@ -2196,7 +2196,7 @@ namespace Mono.Linker.Steps
 
                                        // Remove ",nq" suffix if present
                                        // (it asks the expression evaluator to remove the quotes when displaying the final value)
-                                       if (ContainsNqSuffixRegex ().IsMatch(realMatch)) {
+                                       if (ContainsNqSuffixRegex ().IsMatch (realMatch)) {
                                                realMatch = realMatch.Substring (0, realMatch.LastIndexOf (','));
                                        }
 
@@ -3204,11 +3204,15 @@ namespace Mono.Linker.Steps
                        }
 
                        if (method.HasOverrides) {
+                               var assembly = Context.Resolve (method.DeclaringType.Scope);
+                               // If this method is in a Copy or CopyUsed assembly, .overrides won't get swept and we need to keep all of them
+                               bool markAllOverrides = assembly != null && Annotations.GetAction (assembly) is AssemblyAction.Copy or AssemblyAction.CopyUsed;
                                foreach (MethodReference @base in method.Overrides) {
                                        // Method implementing a static interface method will have an override to it - note instance methods usually don't unless they're explicit.
                                        // Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method.
                                        // Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round.
-                                       if (Context.Resolve (@base) is MethodDefinition baseDefinition
+                                       if (!markAllOverrides &&
+                                               Context.Resolve (@base) is MethodDefinition baseDefinition
                                                && new OverrideInformation.OverridePair (baseDefinition, method).IsStaticInterfaceMethodPair ())
                                                continue;
                                        MarkMethod (@base, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin);

diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs
index 77157ac..e8999de 100644 (file)
--- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs
+++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs
@@ -8,6 +8,12 @@ namespace ILLink.RoslynAnalyzer.Tests.Inheritance.Interfaces
        {
 
                [Fact]
+               public Task OverrideInCopyAssembly ()
+               {
+                       return RunTest (allowMissingWarnings: true);
+               }
+
+               [Fact]
                public Task VarianceBasic ()
                {
                        return RunTest (allowMissingWarnings: true);

diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/OverrideInCopyAssembly.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/OverrideInCopyAssembly.cs
new file mode 100644 (file)
index 0000000..baa6785
--- /dev/null
+++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/OverrideInCopyAssembly.cs
@@ -0,0 +1,57 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+using Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods.Dependencies;
+
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods
+{
+       [KeptMemberInAssembly ("library", typeof (IStaticAbstractMethods), "Method()", "get_Property()", "Property", "set_Property(System.Int32)")]
+       [SetupCompileBefore ("library.dll", new[] { "Dependencies/Library.cs" })]
+       [SetupLinkerAction ("link", "library")]
+       [SetupLinkerAction ("copy", "test")]
+       /// <summary>
+       ///     Regression test for issue: https://github.com/dotnet/runtime/issues/81746
+       ///     OverridesStaticInterfaceMethods.Method() (and Property.set/get) has an entry in .overrides pointing to ISataticAbstractMethods.Method.
+       ///     IStaticAbstractMethods.Method() isn't referenced anywhere else and isn't otherwise needed.
+       ///     Usually the interface method could be removed, and the pointer to it in the .overrides metadata would be removed
+       ///     However, since OverridesStaticInterfaceMethods is in a 'copy' assembly, the .overrides metadata isn't swept. If we remove the method from the interface,
+       ///     we have a "dangling reference" which makes the metadata invalid.
+       /// </summary>
+       static class OverrideInCopyAssembly
+       {
+               [Kept]
+               public static void Main ()
+               {
+                       OverridesStaticInterfaceMethods.Property = OverridesStaticInterfaceMethods.Method ();
+                       var x = OverridesStaticInterfaceMethods.Property;
+               }
+               [Kept]
+               [KeptMember (".ctor()")]
+               [KeptInterface (typeof (IStaticAbstractMethods))]
+               class OverridesStaticInterfaceMethods : IStaticAbstractMethods
+               {
+                       [Kept]
+                       public static int Property {
+                               [Kept]
+                               [KeptOverride (typeof (IStaticAbstractMethods))]
+                               get => throw new NotImplementedException ();
+                               [Kept]
+                               [KeptOverride (typeof (IStaticAbstractMethods))]
+                               set => throw new NotImplementedException ();
+                       }
+                       [Kept]
+                       [KeptOverride (typeof (IStaticAbstractMethods))]
+
+                       public static int Method () => throw new NotImplementedException ();
+                       [Kept]
+                       public int InstanceMethod () => throw new NotImplementedException ();
+               }
+       }
+}