Fix feature switch IL when feature attributes are removed

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

@@ -2951,7 +2951,16 @@ void MarkMethodCollection (IList<MethodDefinition> methods, in DependencyInfo re
 			if (method == null)
 				return null;
 
-			if (Annotations.GetAction (method) == MethodAction.Nothing)
+			var methodAction = Annotations.GetAction (method);
+			if (methodAction is MethodAction.ConvertToStub) {
+				// CodeRewriterStep runs after sweeping, and may request the stubbed value for any preserved method
+				// with the action ConvertToStub. Ensure we have precomputed any stub value that may be needed by
+				// CodeRewriterStep. This ensures sweeping doesn't change the stub value (which can be determined by
+				// FeatureGuardAttribute or FeatureSwitchDefinitionAttribute that might have been removed).
+				Annotations.TryGetMethodStubValue (method, out _);
+			}
+
+			if (methodAction == MethodAction.Nothing)
 				Annotations.SetAction (method, MethodAction.Parse);
 
 

src/tools/illink/src/linker/Linker/MemberActionStore.cs

@@ -13,12 +13,14 @@ public class MemberActionStore
 	{
 		public SubstitutionInfo PrimarySubstitutionInfo { get; }
 		private readonly Dictionary<AssemblyDefinition, SubstitutionInfo?> _embeddedXmlInfos;
+		private readonly Dictionary<MethodDefinition, bool> _featureCheckValues;
 		readonly LinkContext _context;
 
 		public MemberActionStore (LinkContext context)
 		{
 			PrimarySubstitutionInfo = new SubstitutionInfo ();
 			_embeddedXmlInfos = new Dictionary<AssemblyDefinition, SubstitutionInfo?> ();
+			_featureCheckValues = new Dictionary<MethodDefinition, bool> ();
 			_context = context;
 		}
 
@@ -68,6 +70,9 @@ public bool TryGetMethodStubValue (MethodDefinition method, out object? value)
 
 		internal bool TryGetFeatureCheckValue (MethodDefinition method, out bool value)
 		{
+			if (_featureCheckValues.TryGetValue (method, out value))
+				return true;
+
 			value = false;
 
 			if (!method.IsStatic)
@@ -88,7 +93,11 @@ internal bool TryGetFeatureCheckValue (MethodDefinition method, out bool value)
 
 				// If there's a FeatureSwitchDefinition, don't continue looking for FeatureGuard.
 				// We don't want to infer feature switch settings from FeatureGuard.
-				return _context.FeatureSettings.TryGetValue (switchName, out value);
+				if (_context.FeatureSettings.TryGetValue (switchName, out value)) {
+					_featureCheckValues[method] = value;
+					return true;
+				}
+				return false;
 			}
 
 			if (!_context.IsOptimizationEnabled (CodeOptimizations.SubstituteFeatureGuards, method))
@@ -101,13 +110,16 @@ internal bool TryGetFeatureCheckValue (MethodDefinition method, out bool value)
 				if (featureType.Namespace == "System.Diagnostics.CodeAnalysis") {
 					switch (featureType.Name) {
 					case "RequiresUnreferencedCodeAttribute":
+						_featureCheckValues[method] = value;
 						return true;
 					case "RequiresDynamicCodeAttribute":
 						if (_context.FeatureSettings.TryGetValue (
 								"System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported",
 								out bool isDynamicCodeSupported)
-							&& !isDynamicCodeSupported)
+							&& !isDynamicCodeSupported) {
+							_featureCheckValues[method] = value;
 							return true;
+							}
 						break;
 					}
 				}

src/tools/illink/test/Mono.Linker.Tests.Cases/LinkAttributes/Dependencies/FeatureProperties.cs

@@ -0,0 +1,22 @@
+// 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.Diagnostics.CodeAnalysis;
+
+namespace Mono.Linker.Tests.Cases.LinkAttributes.Dependencies
+{
+	public class FeatureProperties
+	{
+		[FeatureSwitchDefinition ("FeatureSwitch")]
+		public static bool FeatureSwitchDefinition => Removed ();
+
+		[FeatureGuard (typeof (RequiresUnreferencedCodeAttribute))]
+		public static bool FeatureGuard => Removed ();
+
+		[FeatureSwitchDefinition ("StubbedFeatureSwitch")]
+		public static bool StubbedFeatureSwitch => Removed ();
+
+		static bool Removed () => true;
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/LinkAttributes/FeatureAttributeRemovalInCopyAssembly.cs

@@ -0,0 +1,58 @@
+// 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.Diagnostics.CodeAnalysis;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+using Mono.Linker.Tests.Cases.LinkAttributes.Dependencies;
+
+namespace Mono.Linker.Tests.Cases.LinkAttributes
+{
+	[KeptMember (".ctor()")]
+	[ExpectedInstructionSequenceOnMemberInAssembly ("FeatureProperties.dll", typeof (FeatureProperties), "get_StubbedFeatureSwitch()", new[] {
+		"ldc.i4.1",
+		"ret",
+	})]
+	[SetupLinkAttributesFile ("TestRemoveFeatureAttributes.xml")]
+	[SetupLinkerSubstitutionFile ("StubFeatureSwitch.xml")]
+	[RemovedAttributeInAssembly ("FeatureProperties.dll", typeof (FeatureSwitchDefinitionAttribute), typeof (FeatureProperties), nameof (FeatureProperties.FeatureSwitchDefinition))]
+	[RemovedAttributeInAssembly ("FeatureProperties.dll", typeof (FeatureGuardAttribute), typeof (FeatureProperties), nameof (FeatureProperties.FeatureGuard))]
+	[RemovedAttributeInAssembly ("FeatureProperties.dll", typeof (FeatureSwitchDefinitionAttribute), typeof (FeatureProperties), nameof (FeatureProperties.StubbedFeatureSwitch))]
+	[RemovedMemberInAssembly ("FeatureProperties.dll", typeof (FeatureProperties), "Removed()")]
+	[SetupCompileBefore ("FeatureProperties.dll", new[] { "Dependencies/FeatureProperties.cs" })]
+	[SetupLinkerArgument ("--feature", "FeatureSwitch", "false")]
+	[SetupLinkerArgument ("--feature", "StubbedFeatureSwitch", "true")]
+	[SetupLinkerAction ("copy", "test")] // prevent trimming calls to feature switch properties
+	[IgnoreLinkAttributes (false)]
+	[IgnoreSubstitutions (false)]
+	class FeatureAttributeRemovalInCopyAssembly
+	{
+		public static void Main ()
+		{
+			TestFeatureSwitch ();
+			TestFeatureGuard ();
+			TestStubbedFeatureSwitch ();
+		}
+
+		[Kept]
+		static void TestFeatureSwitch () {
+			if (FeatureProperties.FeatureSwitchDefinition)
+				Unused ();
+		}
+
+		[Kept]
+		static void TestFeatureGuard () {
+			if (FeatureProperties.FeatureGuard)
+				Unused ();
+		}
+
+		[Kept]
+		static void TestStubbedFeatureSwitch () {
+			if (FeatureProperties.StubbedFeatureSwitch)
+				Unused ();
+		}
+
+		[Kept]
+		static void Unused () { }
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/LinkAttributes/StubFeatureSwitch.xml

@@ -0,0 +1,7 @@
+<linker xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../../src/ILLink.Shared/ILLink.LinkAttributes.xsd">
+  <assembly fullname="FeatureProperties">
+    <type fullname="Mono.Linker.Tests.Cases.LinkAttributes.Dependencies.FeatureProperties">
+      <method signature="System.Boolean get_StubbedFeatureSwitch()" body="stub" />
+    </type>
+  </assembly>
+</linker>

src/tools/illink/test/Mono.Linker.Tests.Cases/LinkAttributes/TestRemoveFeatureAttributes.xml

@@ -0,0 +1,10 @@
+<linker xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../../src/ILLink.Shared/ILLink.LinkAttributes.xsd">
+  <assembly fullname="System.Private.CoreLib">
+    <type fullname="System.Diagnostics.CodeAnalysis.FeatureSwitchDefinitionAttribute">
+      <attribute internal="RemoveAttributeInstances"/>
+    </type>
+    <type fullname="System.Diagnostics.CodeAnalysis.FeatureGuardAttribute">
+      <attribute internal="RemoveAttributeInstances"/>
+    </type>
+  </assembly>
+</linker>