Fix feature switch IL when feature attributes are removed (#104995)

When removing `FeatureSwitchDefinition` and `FeatureGuard`
attributes, we need to ensure that the body rewriting logic still
respects these attributes even though the rewriting happens after
attribute removal. Caching the substituted constant value fixes
this.

More detail:

When feature switches or feature guards are substituted with a
constant, the IL rewriting of the feature property happens after
SweepStep, during CodeRewriterStep. At this point, any attributes
marked for removal by RemoveAttributeInstancesAttribute have
already been swept. When removing
`FeatureSwitchDefinitionAttribute` and `FeatureGuardAttribute`,
as we do with AggressiveAttributeTrimming, this means that
CodeRewriterStep fails to substitute a constant for attributed
feature check or feature guard properties.

Normally, the attributed feature properties would be removed
entirely when substituted, so CodeRewriterStep wouldn't even
visit the property. However, it's possible that the caller of the
property was a `copy` assembly where the callsite can't be
rewritten to return a constant.

When this happens, we can end up with a property that's not
stubbed out during CodeRewriterStep, even though we didn't mark a
callee of the property (because MarkStep did treat the property
as a constant and didn't mark its instructions). When trying to
write out the property's IL this results in a failure because we
see a reference to a method that was removed.

Fixes #104945

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

@@ -2891,7 +2891,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>