Support attribute trimming opt-in (dotnet/linker#1839)

* Support attribute opt-in

* Update docs

* Rename test attributes to match

* Don't pass native SDK assemblies to tests

on Windows

* Update LinkTask

* PR feedback

- Don't warn on duplicate attributes
- Remove comment
- Fix typos and wording
- Use static array
- Rename CheckIsTrimmable -> IsTrimmable

* PR feedback: rename options

- --trim-action -> --trim-mode
-- --default-action -> --action

* Fix ILLink.Tasks test

* PR feedback

- Add plenty of comments
- RootNonTrimmableAssemblies -> ProcessReferencesStep
- Make -reference order stable using List instead of HashSet

* PR feedback

Make GetInputAssemblyPaths private

Commit migrated from dotnet/linker@44907d9

Author: sbomer

src/tools/illink/docs/error-codes.md

@@ -129,6 +129,10 @@ the error code. For example:
 
 #### `IL1038`: Exported type '{type.Name}' cannot be resolved
 
+#### `IL1039`: Reference assembly '{assemblyPath}' could not be loaded
+
+- A reference assembly input passed via -reference could not be loaded.
+
 ----
 ## Warning Codes
 
@@ -1510,6 +1514,15 @@ This is technically possible if a custom assembly defines `DynamicDependencyAttr
   </linker>
   ```
 
+#### `IL2102`: Invalid AssemblyMetadata("IsTrimmable", ...) attribute in assembly 'assembly'. Value must be "True"
+
+- AssemblyMetadataAttribute may be used at the assembly level to turn on trimming for the assembly. The only supported value is "True", but the attribute contained an unsupported value.
+
+  ``` C#
+  // IL2102: Invalid AssemblyMetadata("IsTrimmable", "False") attribute in assembly 'assembly'. Value must be "True"
+  [assembly: AssemblyMetadata("IsTrimmable", "False")] 
+  ```
+
 ## Single-File Warning Codes
 
 #### `IL3000`: 'member' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'

src/tools/illink/docs/illink-options.md

@@ -58,22 +58,25 @@ The linker can do the following things on all or individual assemblies
 - `delete`- remove them from the output
 - `save` - save them in memory without linking
 
-You can specify an action per assembly using `-p` option like this:
+You can specify an action per assembly using `--action` option like this:
 
-`illink -p link Foo`
+`illink --action link Foo`
 
 or
 
-`illink -p skip System.Windows.Forms`
+`illink --action skip System.Windows.Forms`
 
-Or you can specify what to do for the core assemblies.
+Or you can specify what to do for the trimmed assemblies.
 
-Core assemblies are the assemblies that belong to the base class library,
-like `System.Private.CoreLib.dll`, `System.dll` or `System.Windows.Forms.dll`.
+A trimmable assembly is any assembly that includes the attribute `System.Reflection.AssemblyMetadata("IsTrimmable", "True")`.
 
-You can specify what action to do on the core assemblies with the option:
+You can specify what action to do on the trimmed assemblies with the option:
 
-`-c skip|copy|link`
+`--trim-mode skip|copy|copyused|link`
+
+You can specify what action to do on assemblies without such an attribute with the option:
+
+`--action copy|link`
 
 ### The output directory
 

src/tools/illink/docs/illink-tasks.md

@@ -58,7 +58,7 @@ The linker can be invoked as an MSBuild task, `ILLink`. We recommend not using t
         RootAssemblyNames="@(LinkerRootAssemblies)"
         RootDescriptorFiles="@(LinkerRootDescriptors)"
         OutputDirectory="output"
-        ExtraArgs="-t -c link" />
+        ExtraArgs="-t --trim-mode link" />
 ```
 
 ## Default Linking Behavior

src/tools/illink/src/ILLink.Tasks/LinkTask.cs

@@ -23,15 +23,15 @@ public class ILLink : ToolTask
 		///       UnusedInterfaces
 		///       IPConstProp
 		///       Sealer
-		///   Maps to '-reference', and possibly '-p', '--enable-opt', '--disable-opt'
+		///   Maps to '-reference', and possibly '--action', '--enable-opt', '--disable-opt'
 		/// </summary>
 		[Required]
 		public ITaskItem[] AssemblyPaths { get; set; }
 
 		/// <summary>
 		///    Paths to assembly files that are reference assemblies,
 		///    representing the surface area for compilation.
-		///    Maps to '-reference', with action set to 'skip' via '-p'.
+		///    Maps to '-reference', with action set to 'skip' via '--action'.
 		/// </summary>
 		public ITaskItem[] ReferenceAssemblyPaths { get; set; }
 
@@ -180,11 +180,16 @@ public class ILLink : ToolTask
 		bool? _removeSymbols;
 
 		/// <summary>
-		///   Sets the default action for assemblies.
-		///   Maps to '-c' and '-u'.
+		///   Sets the default action for trimmable assemblies.
+		///   Maps to '--trim-mode'
 		/// </summary>
 		public string TrimMode { get; set; }
 
+		/// <summary>
+		///   Sets the default action for assemblies which have not opted into trimming.
+		///   Maps to '--action'
+		public string DefaultAction { get; set; }
+
 		/// <summary>
 		///   A list of custom steps to insert into the linker pipeline.
 		///   Each ItemSpec should be the path to the assembly containing the custom step.
@@ -296,7 +301,7 @@ protected override string GenerateResponseFileCommands ()
 
 				string trimMode = assembly.GetMetadata ("TrimMode");
 				if (!String.IsNullOrEmpty (trimMode)) {
-					args.Append ("-p ");
+					args.Append ("--action ");
 					args.Append (trimMode);
 					args.Append (' ').AppendLine (Quote (assemblyName));
 				}
@@ -329,7 +334,7 @@ protected override string GenerateResponseFileCommands ()
 					// Treat reference assemblies as "skip". Ideally we
 					// would not even look at the IL, but only use them to
 					// resolve surface area.
-					args.Append ("-p skip ").AppendLine (Quote (assemblyName));
+					args.Append ("--action skip ").AppendLine (Quote (assemblyName));
 				}
 			}
 
@@ -396,7 +401,10 @@ protected override string GenerateResponseFileCommands ()
 				args.AppendLine ("-b");
 
 			if (TrimMode != null)
-				args.Append ("-c ").Append (TrimMode).Append (" -u ").AppendLine (TrimMode);
+				args.Append ("--trim-mode ").AppendLine (TrimMode);
+
+			if (DefaultAction != null)
+				args.Append ("--action ").AppendLine (DefaultAction);
 
 			if (CustomSteps != null) {
 				foreach (var customStep in CustomSteps) {

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

@@ -200,9 +200,6 @@ void Initialize ()
 		{
 			InitializeCorelibAttributeXml ();
 
-			foreach (AssemblyDefinition assembly in _context.GetAssemblies ())
-				InitializeAssembly (assembly);
-
 			ProcessMarkedPending ();
 		}
 
@@ -228,13 +225,6 @@ void InitializeCorelibAttributeXml ()
 				_context.CustomAttributes.PrimaryAttributeInfo.AddInternalAttributes (provider, annotations);
 		}
 
-		protected virtual void InitializeAssembly (AssemblyDefinition assembly)
-		{
-			var action = _context.Annotations.GetAction (assembly);
-			if (IsFullyPreservedAction (action))
-				MarkAssembly (assembly, new DependencyInfo (DependencyKind.AssemblyAction, action));
-		}
-
 		void Complete ()
 		{
 			foreach (var body in _unreachableBodies) {
@@ -386,7 +376,7 @@ bool MarkFullyPreservedAssemblies ()
 			// Fully mark any assemblies with copy/save action.
 
 			// Unresolved references could get the copy/save action if this is the default action.
-			bool scanReferences = IsFullyPreservedAction (_context.CoreAction) || IsFullyPreservedAction (_context.UserAction);
+			bool scanReferences = IsFullyPreservedAction (_context.TrimAction) || IsFullyPreservedAction (_context.DefaultAction);
 
 			if (!scanReferences) {
 				// Unresolved references could get the copy/save action if it was set explicitly
@@ -1312,7 +1302,7 @@ protected void MarkAssembly (AssemblyDefinition assembly, DependencyInfo reason)
 
 			MarkExportedTypesTarget.ProcessAssembly (assembly, _context);
 
-			if (IsFullyPreservedAction (_context.Annotations.GetAction (assembly))) {
+			if (ProcessReferencesStep.IsFullyPreservedAction (_context.Annotations.GetAction (assembly))) {
 				MarkEntireAssembly (assembly);
 				return;
 			}

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

@@ -0,0 +1,75 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.IO;
+
+namespace Mono.Linker.Steps
+{
+	public class ProcessReferencesStep : BaseStep
+	{
+		protected override void Process ()
+		{
+			// Walk over all -reference inputs and resolve any that may need to be rooted.
+
+			// For example:
+			// -reference dir/Unreferenced.dll --action copy --trim-mode copyused
+			//     In this case we need to check whether Unreferenced has the
+			//     IsTrimmable attribute, and root it if not.
+			// -reference dir/Unreferenced.dll --action copy --trim-mode copyused --action link Unreferenced
+			//     The per-assembly action wins over the default --action or --trim-mode,
+			//     so we don't need to load the assembly to check for IsTrimmable attribute.
+			// -reference dir/Unreferenced.dll --action link --trim-mode link
+			//     In this case, we don't need to load the assembly up-front, because it will
+			//     not get the copy/save action, regardless of the IsTrimmable attribute.
+
+			// Note that we don't do the same for assemblies which may be resolved from input directories - such
+			// assemblies will only be rooted if something loads them.
+			foreach (var assemblyPath in GetInputAssemblyPaths ()) {
+				var assemblyName = Path.GetFileNameWithoutExtension (assemblyPath);
+
+				// If there's no way that this reference could have the copy/save action,
+				// we don't need to load it up-front.
+				if (!MaybeIsFullyPreservedAssembly (assemblyName))
+					continue;
+
+				// For the remaining references, we need to resolve them (which looks for IsTrimmable attribute)
+				// to determine the action.
+				var assembly = Context.TryResolve (assemblyName);
+				if (assembly == null) {
+					Context.LogError ($"Reference assembly '{assemblyPath}' could not be loaded", 1039);
+					continue;
+				}
+
+				// If the assigned action (now taking into account the IsTrimmable attribute) requires us
+				// to root the assembly, do so.
+				if (IsFullyPreservedAction (Annotations.GetAction (assembly)))
+					Annotations.Mark (assembly.MainModule, new DependencyInfo (DependencyKind.AssemblyAction, assembly));
+			}
+		}
+
+		IEnumerable<string> GetInputAssemblyPaths ()
+		{
+			var assemblies = new HashSet<string> ();
+			foreach (var referencePath in Context.Resolver.GetReferencePaths ()) {
+				var assemblyName = Path.GetFileNameWithoutExtension (referencePath);
+				if (assemblies.Add (assemblyName))
+					yield return referencePath;
+			}
+		}
+
+		public static bool IsFullyPreservedAction (AssemblyAction action)
+		{
+			return action == AssemblyAction.Copy || action == AssemblyAction.Save;
+		}
+
+		bool MaybeIsFullyPreservedAssembly (string assemblyName)
+		{
+			if (Context.Actions.TryGetValue (assemblyName, out AssemblyAction action))
+				return IsFullyPreservedAction (action);
+
+			return IsFullyPreservedAction (Context.DefaultAction) || IsFullyPreservedAction (Context.TrimAction);
+		}
+	}
+}

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

@@ -30,7 +30,6 @@
 using System.Collections.Generic;
 using System.IO;
 using Mono.Cecil;
-using Mono.Collections.Generic;
 
 namespace Mono.Linker
 {
@@ -42,7 +41,7 @@ public class AssemblyResolver : DirectoryAssemblyResolver
 		HashSet<string> _unresolvedAssemblies;
 		bool _ignoreUnresolved;
 		LinkContext _context;
-		readonly Collection<string> _references;
+		readonly List<string> _references;
 
 
 		public IDictionary<string, AssemblyDefinition> AssemblyCache {
@@ -57,7 +56,7 @@ public AssemblyResolver ()
 		public AssemblyResolver (Dictionary<string, AssemblyDefinition> assembly_cache)
 		{
 			_assemblies = assembly_cache;
-			_references = new Collection<string> () { };
+			_references = new List<string> () { };
 		}
 
 		public bool IgnoreUnresolved {
@@ -80,16 +79,18 @@ public string GetAssemblyFileName (AssemblyDefinition assembly)
 			return assembly.MainModule.FileName;
 		}
 
-		AssemblyDefinition ResolveFromReferences (AssemblyNameReference name, Collection<string> references, ReaderParameters parameters)
+		AssemblyDefinition ResolveFromReferences (AssemblyNameReference name, ReaderParameters parameters)
 		{
-			var fileName = name.Name + ".dll";
-			foreach (var reference in references) {
-				if (Path.GetFileName (reference) != fileName)
-					continue;
-				try {
-					return GetAssembly (reference, parameters);
-				} catch (BadImageFormatException) {
-					continue;
+			foreach (var reference in _references) {
+				foreach (var extension in DirectoryAssemblyResolver.Extensions) {
+					var fileName = name.Name + extension;
+					if (Path.GetFileName (reference) != fileName)
+						continue;
+					try {
+						return GetAssembly (reference, parameters);
+					} catch (BadImageFormatException) {
+						continue;
+					}
 				}
 			}
 
@@ -107,7 +108,7 @@ public override AssemblyDefinition Resolve (AssemblyNameReference name, ReaderPa
 			if (!_assemblies.TryGetValue (name.Name, out AssemblyDefinition asm) && (_unresolvedAssemblies == null || !_unresolvedAssemblies.Contains (name.Name))) {
 				try {
 					// Any full path explicit reference takes precedence over other look up logic
-					asm = ResolveFromReferences (name, _references, parameters);
+					asm = ResolveFromReferences (name, parameters);
 
 					// Fall back to the base class resolution logic
 					if (asm == null)
@@ -139,6 +140,11 @@ public void AddReferenceAssembly (string referencePath)
 			_references.Add (referencePath);
 		}
 
+		public List<string> GetReferencePaths ()
+		{
+			return _references;
+		}
+
 		protected override void Dispose (bool disposing)
 		{
 			foreach (var asm in _assemblies.Values) {

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

@@ -83,11 +83,12 @@ public virtual AssemblyDefinition Resolve (AssemblyNameReference name, ReaderPar
 			throw new AssemblyResolutionException (name, new FileNotFoundException ($"Unable to find '{name.Name}.dll' or '{name.Name}.exe' file"));
 		}
 
+		public static string[] Extensions = new[] { ".dll", ".exe" };
+
 		AssemblyDefinition SearchDirectory (AssemblyNameReference name, IEnumerable<string> directories, ReaderParameters parameters)
 		{
-			var extensions = new[] { ".dll", ".exe" };
 			foreach (var directory in directories) {
-				foreach (var extension in extensions) {
+				foreach (var extension in Extensions) {
 					string file = Path.Combine (directory, name.Name + extension);
 					if (!File.Exists (file))
 						continue;