Fix marking of saved assemblies (#86474)

This ensures the fix from
#82197 works also for
assemblies which get the `save` action (which isn't supported
from the command-line, but may happen in custom steps).

It also ensures that `save` assemblies get fully marked
regardless of `DisableMarkingOfCopyAssemblies`.

---------

Co-authored-by: Jackson Schuster <36744439+jtschuster@users.noreply.github.com>

src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

@@ -1410,10 +1410,13 @@ protected void MarkAssembly (AssemblyDefinition assembly, DependencyInfo reason)
 
 			MarkExportedTypesTarget.ProcessAssembly (assembly, Context);
 
-			if (ProcessReferencesStep.IsFullyPreservedAction (Annotations.GetAction (assembly))) {
-				if (!Context.TryGetCustomData ("DisableMarkingOfCopyAssemblies", out string? disableMarkingOfCopyAssembliesValue) ||
-					disableMarkingOfCopyAssembliesValue != "true")
+			var action = Annotations.GetAction (assembly);
+			if (ProcessReferencesStep.IsFullyPreservedAction (action)) {
+				if (action != AssemblyAction.Copy ||
+					!Context.TryGetCustomData ("DisableMarkingOfCopyAssemblies", out string? disableMarkingOfCopyAssembliesValue) ||
+					disableMarkingOfCopyAssembliesValue != "true") {
 					MarkEntireAssembly (assembly);
+				}
 				return;
 			}
 
@@ -3213,8 +3216,8 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 
 			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;
+				// If this method is in a Copy, CopyUsed, or Save 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 or AssemblyAction.Save;
 				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.

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -13,6 +13,12 @@ public Task OverrideInCopyAssembly ()
 			return RunTest (allowMissingWarnings: true);
 		}
 
+		[Fact]
+		public Task OverrideInSaveAssembly ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
 		[Fact]
 		public Task VarianceBasic ()
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/Dependencies/CustomStepSaveAssembly.cs

@@ -0,0 +1,13 @@
+using System;
+using Mono.Cecil;
+using Mono.Linker;
+using Mono.Linker.Steps;
+
+public class CustomStepSaveAssembly : BaseStep
+{
+    protected override void ProcessAssembly (AssemblyDefinition assembly)
+    {
+        if (assembly.Name.Name == "test")
+            Context.Annotations.SetAction (assembly, AssemblyAction.Save);
+    }
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/OverrideInCopyAssembly.cs

@@ -18,7 +18,7 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods
 	[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.
+	///	OverridesStaticInterfaceMethods.Method() (and Property.set/get) has an entry in .overrides pointing to IStaticAbstractMethods.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,

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/OverrideInSaveAssembly.cs

@@ -0,0 +1,59 @@
+// 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)")]
+    // Add a custom step which sets the assembly action of the test to "save"    
+    [SetupCompileBefore ("CustomStepSaveAssembly.dll", new[] { "Dependencies/CustomStepSaveAssembly.cs" }, new[] { "illink.dll", "Mono.Cecil.dll", "netstandard.dll" })]
+    [SetupLinkerArgument ("--custom-step", "-MarkStep:CustomStepSaveAssembly,CustomStepSaveAssembly.dll")]
+	[SetupCompileBefore ("library.dll", new[] { "Dependencies/Library.cs" })]
+	[SetupLinkerAction ("link", "library")]
+	/// <summary>
+	///	Regression test for issue: https://github.com/dotnet/runtime/issues/86242
+	///	OverridesStaticInterfaceMethods.Method() (and Property.set/get) has an entry in .overrides pointing to IStaticAbstractMethods.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 'save' 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 OverrideInSaveAssembly
+	{
+		[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 ();
+		}
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <DefineConstants>$(DefineConstants);INCLUDE_EXPECTATIONS;SUPPORTS_DEFAULT_INTERFACE_METHODS</DefineConstants>
@@ -12,6 +12,7 @@
   <ItemGroup>
     <Compile Remove="Attributes\OnlyKeepUsed\Dependencies\UnusedAttributeWithTypeForwarderIsRemoved_Forwarder.cs" />
     <Compile Remove="CppCLI\Dependencies\CallCppCLIFromManaged.cs" />
+    <Compile Remove="Inheritance.Interfaces\StaticInterfaceMethods\Dependencies\CustomStepSaveAssembly.cs" />
     <Compile Remove="LinkXml\Dependencies\CanPreserveAnExportedType_Forwarder.cs" />
     <Compile Remove="LinkXml\Dependencies\UsedNonRequiredExportedTypeIsKept_fwd.cs" />
     <Compile Remove="LinkXml\Dependencies\UsedNonRequiredExportedTypeIsKeptWhenRooted_fwd.cs" />