Avoid copy action for root assemblies (#96363)

Changes TrimmerRootAssembly (and the underlying option -a) to
root all members of the assembly, without setting the action to
copy. This allows us to perform branch removal and attribute
removal for the assembly.

This includes changes to MarkStep to ensure that rooted
assemblies are marked visible to reflection. This for example
ensures that parameter names don't get removed.

With the change to MarkEntireType that calls MarkType instead of
Annotations.Mark, we are now reaching MarkType for the dependency
from the copy assembly to the attribute it contains, producing an
unexpected IL2045. Silence this by checking the DependencyKind.

Also fixes a bug in constant method analysis that this uncovered:

BitVector32.CreateMask(int.MinValue) was incorrectly being treated
as returning constant 1. The IL from the beginning of that method:

    IL_0000:  ldarg.0
    IL_0001:  brtrue.s   IL_0005
    IL_0003:  ldc.i4.1
    IL_0004:  ret

The brtrue.s branch was being analyzed as not taken because the
argument was not 1. The branch should be analyzed as taken for
any non-zero argument.

src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs

@@ -280,6 +280,7 @@ public static bool IsMetadataTokenSupported
         // Changed to `true` when trimming
         public static bool IsBuiltWithAggressiveTrimming => IsNativeAot;
         public static bool IsNotBuiltWithAggressiveTrimming => !IsBuiltWithAggressiveTrimming;
+        public static bool IsTrimmedWithILLink => IsBuiltWithAggressiveTrimming && !IsNativeAot;
 
         // Windows - Schannel supports alpn from win8.1/2012 R2 and higher.
         // Linux - OpenSsl supports alpn from openssl 1.0.2 and higher.

src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj

@@ -72,19 +72,20 @@
     <TrimmerRootDescriptor Include="$(MSBuildThisFileDirectory)ILLink.Descriptors.xml" />
   </ItemGroup>
 
-  <Target Name="PreserveEnCAssembliesFromLinking" Condition="'$(TargetOS)' == 'browser' and '$(EnableAggressiveTrimming)' == 'true'" BeforeTargets="ConfigureTrimming">
+  <Target Name="PreserveEnCAssembliesFromLinking" Condition="'$(TargetOS)' == 'browser' and '$(EnableAggressiveTrimming)' == 'true'" BeforeTargets="PrepareForILLink">
     <ItemGroup>
-      <!-- want to compute the intersection: apply update test assemblies that are also resolved to publish.
+      <!-- want to compute the intersection: apply update test assemblies that are also resolved to link.
         -->
       <ApplyUpdateTestProjectReference Include="@(ProjectReference)" Condition="$([System.String]::new('%(FileName)').StartsWith('$(ApplyUpdateTestPrefix)'))" />
       <ApplyUpdateTestAssemblyName Include="@(ApplyUpdateTestProjectReference->'%(FileName).dll')" />
-      <ResolvedFileToPublishNoDirName Include="%(ResolvedFileToPublish.FileName)%(ResolvedFileToPublish.Extension)">
-        <ResolvedFileToPublish>%(ResolvedFileToPublish.FullPath)</ResolvedFileToPublish>
-      </ResolvedFileToPublishNoDirName>
-      <ResolvedApplyUpdateAssembly Include="@(ResolvedFileToPublishNoDirName)" Condition="'@(ResolvedFileToPublishNoDirName)' == '@(ApplyUpdateTestAssemblyName)' and %(Identity) != ''" />
       <!-- Don't let the IL linker modify EnC test assemblies -->
-      <TrimmerRootAssembly Include="%(ResolvedApplyUpdateAssembly.ResolvedFileToPublish)" />
-    </ItemGroup>
+      <ManagedAssemblyToLink Condition="'%(FileName)%(Extension)' == '@(ApplyUpdateTestAssemblyName)'">
+        <TrimMode>copy</TrimMode>
+      </ManagedAssemblyToLink>
+      <ManagedAssemblyToLink Condition="'%(FileName)%(Extension)' == '@(IntermediateAssembly->'%(FileName)%(Extension)')'">
+        <TrimMode>copy</TrimMode>
+      </ManagedAssemblyToLink>
+    </ItemGroup> 
   </Target>
 
   <Target Name="IncludeDeltasInWasmBundle" BeforeTargets="PrepareForWasmBuildApp" Condition="'$(TargetOS)' == 'browser'">

src/libraries/System.Runtime/tests/System.Reflection.Tests/MemberInfoTests.cs

@@ -166,7 +166,8 @@ public void MethodInfoReflectedTypeDoesNotSurviveRuntimeHandles()
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtimelab/issues/830", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
+        [ActiveIssue("https://github.com/dotnet/linker/issues/2078", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming)),
+            /* Stripping ComVisible because descriptors tell us so */]
         public void GetCustomAttributesData()
         {
             MemberInfo[] m = typeof(MemberInfoTests).GetMember("SampleClass");

src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj

@@ -14,6 +14,8 @@
 
     <!-- The test is looking for debugger attributes we would have stripped with NativeAOT -->
     <IlcKeepManagedDebuggerSupport>true</IlcKeepManagedDebuggerSupport>
+    <!-- Same for ILLink -->
+    <DebuggerSupport>true</DebuggerSupport>
   </PropertyGroup>
 
   <!--

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/AttributeTests.cs

@@ -154,7 +154,7 @@ class ChildAttributeWithField : ParentAttribute
             public int Field = 0;
         }
 
-        [ActiveIssue("https://github.com/dotnet/runtimelab/issues/803", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
+        [ActiveIssue("https://github.com/dotnet/runtimelab/issues/803", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming))]
         [Fact]
         [StringValue("\uDFFF")]
         public static void StringArgument_InvalidCodeUnits_FallbackUsed()

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Attributes.cs

@@ -44,7 +44,8 @@ public void TestIsDefined()
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/linker/issues/2078", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot) /* Stripping ComVisible because descriptors tell us so */)]
+        [ActiveIssue("https://github.com/dotnet/linker/issues/2078", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming)
+            /* Stripping ComVisible because descriptors tell us so */)]
         public void IsDefined_PropertyInfo()
         {
             PropertyInfo pi = typeof(TestBase).GetProperty("PropBase3");
@@ -327,6 +328,8 @@ public static class GetCustomAttribute
     {
 
         [Fact]
+        [ActiveIssue("https://github.com/dotnet/linker/issues/2078", typeof(PlatformDetection), nameof(PlatformDetection.IsTrimmedWithILLink))
+            /* Stripping security attributes removes UnverifiableCodeAttribute */]
         public static void customAttributeCount()
         {
             List<CustomAttributeData> customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList();

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Reflection/CustomAttributeDataTests.cs

@@ -75,7 +75,8 @@ public static void Test_CustomAttribute_Constructor_CrossAssembly1()
 
         [Fact]
         [ComVisible(false)]
-        [ActiveIssue("https://github.com/dotnet/linker/issues/2078", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot)) /* Descriptors tell us to remove ComVisibleAttribute */]
+        [ActiveIssue("https://github.com/dotnet/linker/issues/2078", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming))
+            /* Descriptors tell us to remove ComVisibleAttribute */]
         public static void Test_CustomAttribute_Constructor_CrossAssembly2()
         {
             MethodInfo m = (MethodInfo)MethodBase.GetCurrentMethod();

src/tools/illink/src/linker/CompatibilitySuppressions.xml

@@ -1147,6 +1147,12 @@
     <Left>ref/net8.0/illink.dll</Left>
     <Right>lib/net8.0/illink.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Mono.Linker.AnnotationStore.IsRootAssembly(Mono.Cecil.AssemblyDefinition)</Target>
+    <Left>ref/net8.0/illink.dll</Left>
+    <Right>lib/net8.0/illink.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0002</DiagnosticId>
     <Target>M:Mono.Linker.AnnotationStore.Mark(Mono.Cecil.CustomAttribute,Mono.Linker.DependencyInfo@)</Target>
@@ -1243,6 +1249,12 @@
     <Left>ref/net8.0/illink.dll</Left>
     <Right>lib/net8.0/illink.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Mono.Linker.AnnotationStore.SetRootAssembly(Mono.Cecil.AssemblyDefinition)</Target>
+    <Left>ref/net8.0/illink.dll</Left>
+    <Right>lib/net8.0/illink.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0002</DiagnosticId>
     <Target>M:Mono.Linker.AnnotationStore.SetSubstitutedInit(Mono.Cecil.TypeDefinition)</Target>

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

@@ -153,6 +153,7 @@ internal DynamicallyAccessedMembersTypeHierarchy DynamicallyAccessedMembersTypeH
 			DependencyKind.InterfaceImplementationInterfaceType,
 			DependencyKind.Ldtoken,
 			DependencyKind.ModifierType,
+			DependencyKind.NestedType,
 			DependencyKind.InstructionTypeRef,
 			DependencyKind.ParameterType,
 			DependencyKind.ReferencedBySpecialAttribute,
@@ -373,7 +374,7 @@ internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason)
 					MarkEntireType (nested, new DependencyInfo (DependencyKind.NestedType, type));
 			}
 
-			Annotations.Mark (type, reason, ScopeStack.CurrentScope.Origin);
+			MarkTypeVisibleToReflection (type, type, reason, ScopeStack.CurrentScope.Origin);
 			MarkCustomAttributes (type, new DependencyInfo (DependencyKind.CustomAttribute, type));
 			MarkTypeSpecialCustomAttributes (type);
 
@@ -386,26 +387,27 @@ internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason)
 
 			if (type.HasFields) {
 				foreach (FieldDefinition field in type.Fields) {
-					MarkField (field, new DependencyInfo (DependencyKind.MemberOfType, type), ScopeStack.CurrentScope.Origin);
+					MarkFieldVisibleToReflection (field, new DependencyInfo (DependencyKind.MemberOfType, type), ScopeStack.CurrentScope.Origin);
 				}
 			}
 
 			if (type.HasMethods) {
 				foreach (MethodDefinition method in type.Methods) {
-					Annotations.SetAction (method, MethodAction.ForceParse);
-					MarkMethod (method, new DependencyInfo (DependencyKind.MemberOfType, type), ScopeStack.CurrentScope.Origin);
+					if (IsFullyPreservedAction (Annotations.GetAction (type.Module.Assembly)))
+						Annotations.SetAction (method, MethodAction.ForceParse);
+					MarkMethodVisibleToReflection (method, new DependencyInfo (DependencyKind.MemberOfType, type), ScopeStack.CurrentScope.Origin);
 				}
 			}
 
 			if (type.HasProperties) {
 				foreach (var property in type.Properties) {
-					MarkProperty (property, new DependencyInfo (DependencyKind.MemberOfType, type));
+					MarkPropertyVisibleToReflection (property, new DependencyInfo (DependencyKind.MemberOfType, type), ScopeStack.CurrentScope.Origin);
 				}
 			}
 
 			if (type.HasEvents) {
 				foreach (var ev in type.Events) {
-					MarkEvent (ev, new DependencyInfo (DependencyKind.MemberOfType, type));
+					MarkEventVisibleToReflection (ev, new DependencyInfo (DependencyKind.MemberOfType, type), ScopeStack.CurrentScope.Origin);
 				}
 			}
 		}
@@ -1422,6 +1424,8 @@ protected virtual void MarkAssembly (AssemblyDefinition assembly, DependencyInfo
 					MarkEntireAssembly (assembly);
 				}
 				return;
+			} else if (Annotations.IsRootAssembly (assembly)) {
+				MarkEntireAssembly (assembly);
 			}
 
 			ProcessModuleType (assembly);
@@ -2033,7 +2037,9 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in
 				// will call MarkType on the attribute type itself).
 				// If for some reason we do keep the attribute type (could be because of previous reference which would cause IL2045
 				// or because of a copy assembly with a reference and so on) then we should not spam the warnings due to the type itself.
-				if (!(reason.Source is IMemberDefinition sourceMemberDefinition && sourceMemberDefinition.DeclaringType == type))
+				// Also don't warn when the type is marked due to an assembly being rooted.
+				if (!(reason.Source is IMemberDefinition sourceMemberDefinition && sourceMemberDefinition.DeclaringType == type) &&
+					reason.Kind is not DependencyKind.TypeInAssembly)
 					Context.LogWarning (ScopeStack.CurrentScope.Origin, DiagnosticId.AttributeIsReferencedButTrimmerRemoveAllInstances, type.GetDisplayName ());
 			}
 

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

@@ -87,7 +87,8 @@ protected override void Process ()
 				Context.MetadataTrimming = MetadataTrimming.None;
 				break;
 			case AssemblyRootMode.AllMembers:
-				Context.Annotations.SetAction (assembly, AssemblyAction.Copy);
+				Annotations.SetRootAssembly (assembly);
+				Annotations.Mark (assembly.MainModule, di, origin);
 				return;
 			}
 		}

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

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// 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;
@@ -1583,7 +1583,7 @@ public bool Analyze (in CalleePayload callee, Stack<MethodDefinition> callStack)
 								return false;
 
 							if (operand is int oint) {
-								if (oint == 1)
+								if (oint != 0)
 									jmpTarget = (Instruction) instr.Operand;
 
 								continue;

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

@@ -59,6 +59,7 @@ public partial class AnnotationStore
 		protected readonly Dictionary<TypeDefinition, TypePreserveMembers> preserved_type_members = new ();
 		protected readonly Dictionary<ExportedType, TypePreserveMembers> preserved_exportedtype_members = new ();
 		protected readonly Dictionary<IMemberDefinition, List<MethodDefinition>> preserved_methods = new Dictionary<IMemberDefinition, List<MethodDefinition>> ();
+		readonly HashSet<AssemblyDefinition> assemblies_with_root_all_members = new ();
 		protected readonly HashSet<IMetadataTokenProvider> public_api = new HashSet<IMetadataTokenProvider> ();
 		protected readonly Dictionary<AssemblyDefinition, ISymbolReader> symbol_readers = new Dictionary<AssemblyDefinition, ISymbolReader> ();
 		readonly Dictionary<IMemberDefinition, LinkerAttributesInformation> linker_attributes = new Dictionary<IMemberDefinition, LinkerAttributesInformation> ();
@@ -399,6 +400,16 @@ public bool TryGetPreservedMembers (ExportedType type, out TypePreserveMembers p
 			return preserved_exportedtype_members.TryGetValue (type, out preserve);
 		}
 
+		public void SetRootAssembly (AssemblyDefinition assembly)
+		{
+			assemblies_with_root_all_members.Add (assembly);
+		}
+
+		public bool IsRootAssembly (AssemblyDefinition assembly)
+		{
+			return assemblies_with_root_all_members.Contains (assembly);
+		}
+
 		public bool TryGetMethodStubValue (MethodDefinition method, out object? value)
 		{
 			return MemberActions.TryGetMethodStubValue (method, out value);

src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Metadata/SetupLinkerLinkAllAttribute.cs

@@ -0,0 +1,12 @@
+// 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;
+
+namespace Mono.Linker.Tests.Cases.Expectations.Metadata
+{
+	[AttributeUsage (AttributeTargets.Class, AllowMultiple = false)]
+	public class SetupLinkerLinkAllAttribute : BaseMetadataAttribute
+	{
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/DefaultLibraryLinkBehavior.cs

@@ -1,3 +1,6 @@
+// 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 Mono.Linker.Tests.Cases.Expectations.Assertions;
 using Mono.Linker.Tests.Cases.Expectations.Metadata;
 
@@ -6,12 +9,12 @@ namespace Mono.Linker.Tests.Cases.Libraries
 	[IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)]
 	[KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)]
 	[SetupCompileAsLibrary]
-	[SetupLinkerArgument ("-a", "test.dll")]
+	[SetupLinkerLinkAll]
 	[Kept]
 	[KeptMember (".ctor()")]
 	public class DefaultLibraryLinkBehavior
 	{
-		// Kept because by default libraries their action set to copy
+		// Kept because by default libraries have all members rooted
 		[Kept]
 		public static void Main ()
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/RootAllLibrary.cs

@@ -0,0 +1,56 @@
+// 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 System.Runtime.CompilerServices;
+using Mono.Linker.Tests.Cases.Libraries.Dependencies;
+
+#if RootAllLibrary
+[assembly: TypeForwardedTo (typeof (RootAllLibrary_ExportedType))]
+#endif
+
+namespace Mono.Linker.Tests.Cases.Libraries.Dependencies
+{
+	public class RootAllLibrary
+	{
+		public static void Public ()
+		{
+		}
+
+		private static void Private ()
+		{
+		}
+
+		private class NestedType
+		{
+		}
+
+		public static void RemovedBranch ()
+		{
+			if (SubstitutedProperty)
+				RootAllLibrary_OptionalDependency.Use ();
+		}
+
+		// Substituted to false in RootAllLibrary_Substitutions.xml
+		static bool SubstitutedProperty {
+			get {
+				RequiresUnreferencedCode ();
+				return true;
+			}
+		}
+
+		[RequiresUnreferencedCode (nameof (RequiresUnreferencedCode))]
+		static void RequiresUnreferencedCode ()
+		{
+		}
+
+		[RootAllLibrary_RemovedAttribute]
+		class TypeWithRemovedAttribute
+		{
+		}
+	}
+
+	class NonPublicType
+	{
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/RootAllLibrary_ExportedType.cs

@@ -0,0 +1,9 @@
+// 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.
+
+namespace Mono.Linker.Tests.Cases.Libraries.Dependencies
+{
+	public class RootAllLibrary_ExportedType
+	{
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/RootAllLibrary_LinkAttributes.xml

@@ -0,0 +1,7 @@
+<linker>
+    <assembly fullname="removedattribute">
+        <type fullname="Mono.Linker.Tests.Cases.Libraries.Dependencies.RootAllLibrary_RemovedAttribute">
+            <attribute internal="RemoveAttributeInstances" />
+        </type>
+    </assembly>
+</linker>

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/RootAllLibrary_OptionalDependency.cs

@@ -0,0 +1,12 @@
+using System.Diagnostics.CodeAnalysis;
+
+namespace Mono.Linker.Tests.Cases.Libraries.Dependencies
+{
+	public class RootAllLibrary_OptionalDependency
+	{
+		[RequiresUnreferencedCode (nameof (RootAllLibrary_OptionalDependency))]
+		public static void Use ()
+		{
+		}
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/RootAllLibrary_RemovedAttribute.cs

@@ -0,0 +1,8 @@
+using System;
+
+namespace Mono.Linker.Tests.Cases.Libraries.Dependencies
+{
+	public class RootAllLibrary_RemovedAttribute : Attribute
+	{
+	}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/RootAllLibrary_Substitutions.xml

@@ -0,0 +1,7 @@
+<linker>
+    <assembly fullname="library">
+        <type fullname="Mono.Linker.Tests.Cases.Libraries.Dependencies.RootAllLibrary">
+            <method signature="System.Boolean get_SubstitutedProperty()" body="stub" value="false" />
+        </type>
+    </assembly>
+</linker>