Preserve all InternalsVisibleTo attributes in ILLink Trimmer (#98910)

Removes previous special handling to only keep IVT attributes for
assemblies that were resolvable and marked at link time. This behaviour
could cause a noticeable difference in behaviour at runtime without emitting
any trim analysis warnings while providing no mechanism to retain the
attributes. The removal of these attributes provides only a neglible
size reduction.

Fixes #92582

Author: Xtansia

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

@@ -62,7 +62,6 @@ protected LinkContext Context {
 		protected Queue<(MethodDefinition, DependencyInfo, MessageOrigin)> _methods;
 		protected Dictionary<MethodDefinition, MarkScopeStack.Scope> _virtual_methods;
 		protected Queue<AttributeProviderPair> _assemblyLevelAttributes;
-		readonly List<AttributeProviderPair> _ivt_attributes;
 		protected Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> _lateMarkedAttributes;
 		protected List<(TypeDefinition, MarkScopeStack.Scope)> _typesWithInterfaces;
 		protected HashSet<AssemblyDefinition> _dynamicInterfaceCastableImplementationTypesDiscovered;
@@ -222,7 +221,6 @@ public MarkStep ()
 			_methods = new Queue<(MethodDefinition, DependencyInfo, MessageOrigin)> ();
 			_virtual_methods = new Dictionary<MethodDefinition, MarkScopeStack.Scope> ();
 			_assemblyLevelAttributes = new Queue<AttributeProviderPair> ();
-			_ivt_attributes = new List<AttributeProviderPair> ();
 			_lateMarkedAttributes = new Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> ();
 			_typesWithInterfaces = new List<(TypeDefinition, MarkScopeStack.Scope)> ();
 			_dynamicInterfaceCastableImplementationTypesDiscovered = new HashSet<AssemblyDefinition> ();
@@ -288,42 +286,6 @@ protected virtual void Complete ()
 			}
 		}
 
-		bool ProcessInternalsVisibleAttributes ()
-		{
-			bool marked_any = false;
-			foreach (var attr in _ivt_attributes) {
-
-				var provider = attr.Provider;
-				Debug.Assert (attr.Provider is ModuleDefinition or AssemblyDefinition);
-				var assembly = (provider is ModuleDefinition module) ? module.Assembly : provider as AssemblyDefinition;
-
-				using var assemblyScope = ScopeStack.PushLocalScope (new MessageOrigin (assembly));
-
-				if (!Annotations.IsMarked (attr.Attribute) && IsInternalsVisibleAttributeAssemblyMarked (attr.Attribute)) {
-					MarkCustomAttribute (attr.Attribute, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, attr.Provider));
-					marked_any = true;
-				}
-			}
-
-			return marked_any;
-
-			bool IsInternalsVisibleAttributeAssemblyMarked (CustomAttribute ca)
-			{
-				System.Reflection.AssemblyName an;
-				try {
-					an = new System.Reflection.AssemblyName ((string) ca.ConstructorArguments[0].Value);
-				} catch {
-					return false;
-				}
-
-				var assembly = Context.GetLoadedAssembly (an.Name!);
-				if (assembly == null)
-					return false;
-
-				return Annotations.IsMarked (assembly.MainModule);
-			}
-		}
-
 		static bool TypeIsDynamicInterfaceCastableImplementation (TypeDefinition type)
 		{
 			if (!type.IsInterface || !type.HasInterfaces || !type.HasCustomAttributes)
@@ -416,8 +378,7 @@ void Process ()
 				ProcessMarkedPending () ||
 				ProcessLazyAttributes () ||
 				ProcessLateMarkedAttributes () ||
-				MarkFullyPreservedAssemblies () ||
-				ProcessInternalsVisibleAttributes ()) ;
+				MarkFullyPreservedAssemblies ()) ;
 
 			ProcessPendingTypeChecks ();
 		}
@@ -1169,6 +1130,9 @@ protected virtual bool ShouldMarkCustomAttribute (CustomAttribute ca, ICustomAtt
 				case "System.Runtime.InteropServices.InterfaceTypeAttribute":
 				case "System.Runtime.InteropServices.GuidAttribute":
 					return true;
+				// May be implicitly used by the runtime
+				case "System.Runtime.CompilerServices.InternalsVisibleToAttribute":
+					return true;
 				}
 
 				TypeDefinition? type = Context.Resolve (attr_type);
@@ -1546,10 +1510,7 @@ bool ProcessLazyAttributes ()
 				if (IsAttributeRemoved (customAttribute, resolved.DeclaringType) && Annotations.GetAction (CustomAttributeSource.GetAssemblyFromCustomAttributeProvider (assemblyLevelAttribute.Provider)) == AssemblyAction.Link)
 					continue;
 
-				if (customAttribute.AttributeType.IsTypeOf ("System.Runtime.CompilerServices", "InternalsVisibleToAttribute") && !Annotations.IsMarked (customAttribute)) {
-					_ivt_attributes.Add (assemblyLevelAttribute);
-					continue;
-				} else if (!ShouldMarkTopLevelCustomAttribute (assemblyLevelAttribute, resolved)) {
+				if (!ShouldMarkTopLevelCustomAttribute (assemblyLevelAttribute, resolved)) {
 					skippedItems.Add (assemblyLevelAttribute);
 					continue;
 				}

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/TestCaseUtils.cs

@@ -90,7 +90,7 @@ private static IEnumerable<string> GetTestDependencies (string testCaseDir, Synt
 				switch (attributeName) {
 				case "SetupCompileBefore": {
 						var arrayExpression = args["#1"];
-						if (arrayExpression is not (ArrayCreationExpressionSyntax or ImplicitArrayCreationExpressionSyntax))
+						if (arrayExpression is not (ArrayCreationExpressionSyntax or ImplicitArrayCreationExpressionSyntax or CollectionExpressionSyntax))
 							throw new InvalidOperationException ();
 						foreach (var sourceFile in args["#1"].DescendantNodes ().OfType<LiteralExpressionSyntax> ())
 							yield return Path.Combine (testCaseDir, LinkerTestBase.GetStringFromExpression (sourceFile));

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/AttributesTests.g.cs

@@ -123,6 +123,12 @@ public Task IVTUnused ()
 			return RunTest (allowMissingWarnings: true);
 		}
 
+		[Fact]
+		public Task IVTUnusedKeptWhenKeepingUsedAttributesOnly ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
 		[Fact]
 		public Task IVTUsed ()
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases/Attributes/Dependencies/IVTUnused_Lib.cs

@@ -0,0 +1,8 @@
+#if IVT
+[assembly: System.Runtime.CompilerServices.InternalsVisibleTo ("missing")]
+[assembly: System.Runtime.CompilerServices.InternalsVisibleTo ("test-with-key, PublicKey=00240000")]
+#endif
+
+namespace Mono.Linker.Tests.Cases.Attributes.Dependencies;
+
+public class IVTUnusedLib;

src/tools/illink/test/Mono.Linker.Tests.Cases/Attributes/IVTUnused.cs

@@ -1,20 +1,18 @@
 using System.Runtime.CompilerServices;
+using Mono.Linker.Tests.Cases.Attributes.Dependencies;
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
 using Mono.Linker.Tests.Cases.Expectations.Metadata;
 
-#if IVT
-[assembly: InternalsVisibleTo ("missing")]
-[assembly: InternalsVisibleTo ("test-with-key, PublicKey=00240000")]
+namespace Mono.Linker.Tests.Cases.Attributes;
 
-#endif
-
-namespace Mono.Linker.Tests.Cases.Attributes
+[SetupCompileBefore ("lib.dll", ["Dependencies/IVTUnused_Lib.cs"], defines: ["IVT"])]
+[KeptAssembly ("lib.dll")]
+[KeptTypeInAssembly ("lib.dll", typeof(IVTUnusedLib))]
+[KeptAttributeInAssembly ("lib.dll", typeof (InternalsVisibleToAttribute))]
+class IVTUnused
 {
-	[Define ("IVT")]
-	class IVTUnused
+	static void Main ()
 	{
-		static void Main ()
-		{
-		}
+		_ = new IVTUnusedLib ();
 	}
 }

src/tools/illink/test/Mono.Linker.Tests.Cases/Attributes/IVTUnusedKeptWhenKeepingUsedAttributesOnly.cs

@@ -0,0 +1,20 @@
+using System.Runtime.CompilerServices;
+using Mono.Linker.Tests.Cases.Attributes.Dependencies;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.Attributes;
+
+[SetupCompileBefore ("lib.dll", ["Dependencies/IVTUnused_Lib.cs"], defines: ["IVT"])]
+[KeptAssembly ("lib.dll")]
+[KeptTypeInAssembly ("lib.dll", typeof(IVTUnusedLib))]
+[KeptAttributeInAssembly ("lib.dll", typeof (InternalsVisibleToAttribute))]
+
+[SetupLinkerArgument("--used-attrs-only", "true")]
+class IVTUnusedKeptWhenKeepingUsedAttributesOnly
+{
+	static void Main ()
+	{
+		_ = new IVTUnusedLib ();
+	}
+}