Update to new linker custom steps API (#11374)

* Update to new linker custom steps API

* PR feedback

- Fix indentation
- Add Initialize(LinkContext) to ExceptionalMarkHandler
- Remove unnecessary ifdef
- Use IsSetter/IsGetter
- Use [0] instead of Single()
- Avoid allocating empty collections

* Note override issue

* Clean up comments

* Move `DynamicRegistrationSupported` change earlier, along with the
detection code.

This solve the issue that `ILLink` does a similar job _before_ we have
the chance to disable the dynamic registrar.

* ILLink does not support considering other attributes as `[Preserve]`

when it is itself preserved at the assembly-level.

This ignored test is checking that feature so it cannot be enabled
for `NET`

Added to known breaking changes #8900

* Fix removal of the dynamic registrar on legacy

* Fix IntPtr size inlining

Co-authored-by: Sebastien Pouliot <[email protected]>

tests/linker/ios/link all/PreserveTest.cs

@@ -77,6 +77,9 @@ public void PreserveTypeWithoutMembers ()
 		}
 
 		[Test]
+#if NET
+		[Ignore ("This feature is not supported by dotnet's ILLink -> https://github.com/xamarin/xamarin-macios/issues/8900")]
+#endif
 		public void PreserveTypeWithCustomAttribute ()
 		{
 			var t = Type.GetType ("LinkAll.Attributes.MemberWithCustomAttribute" + WorkAroundLinkerHeuristics);

tools/dotnet-linker/ApplyPreserveAttributeBase.cs

@@ -23,7 +23,8 @@ public override SubStepTargets Targets {
 					| SubStepTargets.Field
 					| SubStepTargets.Method
 					| SubStepTargets.Property
-					| SubStepTargets.Event;
+					| SubStepTargets.Event
+					| SubStepTargets.Assembly;
 			}
 		}
 

tools/dotnet-linker/SetupStep.cs

@@ -28,6 +28,17 @@ public List<IStep> Steps {
 			}
 		}
 
+		List<IMarkHandler> _markHandlers;
+		List<IMarkHandler> MarkHandlers {
+			get {
+				if (_markHandlers == null) {
+					var pipeline = typeof (LinkContext).GetProperty ("Pipeline").GetGetMethod ().Invoke (Context, null);
+					_markHandlers = (List<IMarkHandler>) pipeline.GetType ().GetProperty ("MarkHandlers").GetValue (pipeline);
+				}
+				return _markHandlers;
+			}
+		}
+
 		void InsertBefore (IStep step, string stepName)
 		{
 			for (int i = 0; i < Steps.Count; i++) {
@@ -65,28 +76,28 @@ protected override void TryProcess ()
 			// the final decision to remove/keep the dynamic registrar must be done before the linking step
 			InsertBefore (new RegistrarRemovalTrackingStep (), "MarkStep");
 
-			var pre_dynamic_dependency_lookup_substeps = new DotNetSubStepDispatcher ();
-			InsertBefore (pre_dynamic_dependency_lookup_substeps, "MarkStep");
-
-			var prelink_substeps = new DotNetSubStepDispatcher ();
-			InsertBefore (prelink_substeps, "MarkStep");
+			var pre_mark_substeps = new DotNetSubStepDispatcher ();
+			InsertBefore (pre_mark_substeps, "MarkStep");
 
 			var post_sweep_substeps = new DotNetSubStepDispatcher ();
 			InsertAfter (post_sweep_substeps, "SweepStep");
 
 			if (Configuration.LinkMode != LinkMode.None) {
-				pre_dynamic_dependency_lookup_substeps.Add (new PreserveBlockCodeSubStep ());
+				MarkHandlers.Add (new PreserveBlockCodeHandler ());
 
 				// We need to run the ApplyPreserveAttribute step even we're only linking sdk assemblies, because even
 				// though we know that sdk assemblies will never have Preserve attributes, user assemblies may have
 				// [assembly: LinkSafe] attributes, which means we treat them as sdk assemblies and those may have
 				// Preserve attributes.
-				prelink_substeps.Add (new ApplyPreserveAttribute ());
-				prelink_substeps.Add (new OptimizeGeneratedCodeSubStep ());
-				prelink_substeps.Add (new MarkNSObjects ());
-				prelink_substeps.Add (new PreserveSmartEnumConversionsSubStep ());
-				prelink_substeps.Add (new CollectUnmarkedMembersSubStep ());
-				prelink_substeps.Add (new StoreAttributesStep ());
+				MarkHandlers.Add (new DotNetMarkAssemblySubStepDispatcher (new ApplyPreserveAttribute ()));
+				MarkHandlers.Add (new OptimizeGeneratedCodeHandler ());
+				MarkHandlers.Add (new DotNetMarkAssemblySubStepDispatcher (new MarkNSObjects ()));
+				MarkHandlers.Add (new PreserveSmartEnumConversionsHandler ());
+
+				// This step could be run after Mark to avoid tracking all members:
+				// https://github.com/xamarin/xamarin-macios/issues/11447
+				pre_mark_substeps.Add (new CollectUnmarkedMembersSubStep ());
+				pre_mark_substeps.Add (new StoreAttributesStep ());
 
 				post_sweep_substeps.Add (new RemoveAttributesStep ());
 			}

tools/dotnet-linker/Steps/ConfigurationAwareMarkHandler.cs

@@ -0,0 +1,18 @@
+using System;
+using System.Collections.Generic;
+
+using Xamarin.Bundler;
+
+namespace Xamarin.Linker {
+	public abstract class ConfigurationAwareMarkHandler : ExceptionalMarkHandler {
+		protected override void Report (Exception exception)
+		{
+			LinkerConfiguration.Report (context, exception);
+		}
+
+		protected void Report (List<Exception> exceptions)
+		{
+			LinkerConfiguration.Report (context, exceptions);
+		}
+	}
+}

tools/dotnet-linker/Steps/DotNetMarkAssemblyDispatcher.cs

@@ -0,0 +1,11 @@
+using Mono.Linker.Steps;
+
+namespace Xamarin.Linker.Steps {
+	// MarkSubStepsDispatcher is abstract, so create a subclass we can instantiate.
+	// Can be removed when we update to the preview4 linker, which makes MarkSubStepsDispatcher non-abstract.
+	class DotNetMarkAssemblySubStepDispatcher : MarkSubStepsDispatcher {
+		public DotNetMarkAssemblySubStepDispatcher (params BaseSubStep[] subSteps) : base (subSteps)
+		{
+		}
+	}
+}

tools/dotnet-linker/Steps/ExceptionalMarkHandler.cs

@@ -0,0 +1,121 @@
+// Copyright 2016 Xamarin Inc.
+
+using System;
+using Mono.Cecil;
+using Mono.Tuner;
+using Xamarin.Bundler;
+
+using Xamarin.Tuner;
+
+using Mono.Linker;
+using Mono.Linker.Steps;
+
+namespace Xamarin.Linker {
+
+	// Similar to ExceptionalSubStep, but this only runs for marked members
+	// that were registered for handling by the subclass.
+	public abstract class ExceptionalMarkHandler : IMarkHandler
+	{
+		public abstract void Initialize (LinkContext context, MarkContext markContext);
+
+		public virtual void Initialize (LinkContext context)
+		{
+			this.context = context;
+		}
+
+		protected DerivedLinkContext LinkContext => Configuration.DerivedLinkContext;
+
+		protected LinkContext context { get; private set; }
+
+		protected AnnotationStore Annotations => context.Annotations;
+		protected LinkerConfiguration Configuration => LinkerConfiguration.GetInstance (context);
+
+		protected Profile Profile => Configuration.Profile;
+
+		public void ProcessAssembly (AssemblyDefinition assembly)
+		{
+			try {
+				Process (assembly);
+			} catch (Exception e) {
+				Report (Fail (assembly, e));
+			}
+		}
+
+		public void ProcessType (TypeDefinition type)
+		{
+			try {
+				Process (type);
+			} catch (Exception e) {
+				Report (Fail (type, e));
+			}
+		}
+
+		public void ProcessField (FieldDefinition field)
+		{
+			try {
+				Process (field);
+			} catch (Exception e) {
+				Report (Fail (field, e));
+			}
+		}
+
+		public void ProcessMethod (MethodDefinition method)
+		{
+			try {
+				Process (method);
+			} catch (Exception e) {
+				Report (Fail (method, e));
+			}
+		}
+
+		// state-aware versions to be subclassed
+
+		protected virtual void Process (AssemblyDefinition assembly)
+		{
+		}
+
+		protected virtual void Process (TypeDefinition type)
+		{
+		}
+
+		protected virtual void Process (FieldDefinition field)
+		{
+		}
+
+		protected virtual void Process (MethodDefinition method)
+		{
+		}
+
+		// failure overrides, with defaults
+
+		protected virtual Exception Fail (AssemblyDefinition assembly, Exception e)
+		{
+			return ErrorHelper.CreateError (ErrorCode, e, Errors.MX_ExceptionalSubSteps, Name, assembly?.FullName);
+		}
+
+		protected virtual Exception Fail (TypeDefinition type, Exception e)
+		{
+			return ErrorHelper.CreateError (ErrorCode | 1, e, Errors.MX_ExceptionalSubSteps, Name, type?.FullName);
+		}
+
+		protected virtual Exception Fail (FieldDefinition field, Exception e)
+		{
+			return ErrorHelper.CreateError (ErrorCode | 2, e, Errors.MX_ExceptionalSubSteps, Name, field?.FullName);
+		}
+
+		protected virtual Exception Fail (MethodDefinition method, Exception e)
+		{
+			return ErrorHelper.CreateError (ErrorCode | 3, e, Errors.MX_ExceptionalSubSteps, Name, method?.FullName);
+		}
+		protected virtual void Report (Exception e)
+		{
+			throw e;
+		}
+
+		// abstracts
+
+		protected abstract string Name { get; }
+
+		protected abstract int ErrorCode { get; }
+	}
+}

tools/dotnet-linker/Steps/PreserveBlockCodeHandler.cs

@@ -0,0 +1,85 @@
+using System;
+using System.Linq;
+
+using Mono.Cecil;
+
+using Mono.Linker;
+using Mono.Linker.Steps;
+using Mono.Tuner;
+
+using Xamarin.Bundler;
+
+namespace Xamarin.Linker.Steps {
+
+	public class PreserveBlockCodeHandler : ConfigurationAwareMarkHandler {
+
+		protected override string Name { get; } = "Preserve Block Code";
+		protected override int ErrorCode { get; } = 2240;
+
+		public override void Initialize (LinkContext context, MarkContext markContext)
+		{
+			base.Initialize (context);
+			markContext.RegisterMarkTypeAction (ProcessType);
+		}
+
+		protected override void Process (TypeDefinition type)
+		{
+			/* For the following class:
+
+			static internal class SDInnerBlock {
+				// this field is not preserved by other means, but it must not be linked away
+				static internal readonly DInnerBlock Handler = Invoke;
+
+				[MonoPInvokeCallback (typeof (DInnerBlock))]
+				static internal void Invoke (IntPtr block, int magic_number)
+				{
+				}
+			}
+
+			We need to make sure the linker doesn't remove the Handler field
+			and the Invoke method. 
+			*/
+
+			// First make sure we got the right class
+			// The type for the field we're looking for is abstract, sealed and nested and contains exactly 1 field.
+			if (!type.HasFields || !type.IsAbstract || !type.IsSealed || !type.IsNested)
+				return;
+			if (type.Fields.Count != 1)
+				return;
+
+			// The type is also nested inside ObjCRuntime.Trampolines class)
+			var nestingType = type.DeclaringType;
+			if (!nestingType.Is ("ObjCRuntime", "Trampolines"))
+				return;
+
+			// The class has a readonly field named 'Handler'
+			var field = type.Fields [0];
+			if (!field.IsInitOnly)
+				return;
+			if (field.Name != "Handler")
+				return;
+
+			// The class has a parameterless 'Invoke' method with a 'MonoPInvokeCallback' attribute
+			if (!type.HasMethods)
+				return;
+			var method = type.Methods.SingleOrDefault (v => {
+				if (v.Name != "Invoke")
+					return false;
+				if (!v.HasParameters)
+					return false;
+				if (!v.HasCustomAttributes)
+					return false;
+				if (!v.CustomAttributes.Any (v => v.AttributeType.Name == "MonoPInvokeCallbackAttribute"))
+					return false;
+				return true;
+			});
+
+			if (method == null)
+				return;
+
+			// The type was used, so preserve the method and field
+			context.Annotations.Mark (method);
+			context.Annotations.Mark (field);
+		}
+	}
+}