Move FixAbstractMethodsStep out of ILLink process

src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj

@@ -16,7 +16,6 @@
     <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\External\Linker\BaseMarkHandler.cs" Link="External\BaseMarkHandler.cs" />
     <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\AndroidLinkConfiguration.cs" Link="MonoDroid.Tuner\AndroidLinkConfiguration.cs" />
     <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\Extensions.cs" Link="MonoDroid.Tuner\Extensions.cs" />
-    <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\FixAbstractMethodsStep.cs" Link="MonoDroid.Tuner\FixAbstractMethodsStep.cs" />
     <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" Link="MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" />
     <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\LinkDesignerBase.cs" Link="MonoDroid.Tuner\LinkDesignerBase.cs" />
     <Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\RemoveResourceDesignerStep.cs" Link="MonoDroid.Tuner\RemoveResourceDesignerStep.cs" />

src/Microsoft.Android.Sdk.ILLink/PreserveLists/Mono.Android.xml

@@ -34,6 +34,9 @@
 		<type fullname="Android.Runtime.XmlReaderPullParser" />
 		-->
 		<type fullname="Java.Interop.TypeManager*JavaTypeManager" />
+		<type fullname="Java.Lang.AbstractMethodError">
+			<method name=".ctor" />
+		</type>
 		<type fullname="Java.Lang.Object">
 			<method name="SetHandleOnDeserialized" />
 		</type>

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs

@@ -10,68 +10,43 @@
 using Mono.Linker.Steps;
 using Mono.Tuner;
 using Xamarin.Android.Tasks;
-
-#if ILLINK
-using Resources = Microsoft.Android.Sdk.ILLink.Properties.Resources;
-#else   // !ILLINK
 using Resources = Xamarin.Android.Tasks.Properties.Resources;
-#endif  // !ILLINK
 
 namespace MonoDroid.Tuner
 {
-	/// <summary>
-	/// NOTE: this step is subclassed so it can be called directly from Xamarin.Android.Build.Tasks
-	/// </summary>
-	public class FixAbstractMethodsStep : BaseMarkHandler
-#if !ILLINK
-		, IAssemblyModifierPipelineStep
-#endif  // !ILLINK
+	public class FixAbstractMethodsStep : BaseStep, IAssemblyModifierPipelineStep
 	{
-		public override void Initialize (LinkContext context, MarkContext markContext)
+		readonly IMetadataResolver _injectedCache;
+		readonly Func<AssemblyDefinition> _injectedGetMonoAndroidAssembly;
+		readonly Action<string> _injectedLogMessage;
+
+		/// <summary>
+		/// Used by LinkAssembliesNoShrink and tests. Call <see cref="BaseStep.Initialize(LinkContext)"/> before use.
+		/// </summary>
+		public FixAbstractMethodsStep () { }
+
+		/// <summary>
+		/// Used by <see cref="PostTrimmingFixAbstractMethodsStep"/> when no <see cref="LinkContext"/> is available.
+		/// </summary>
+		internal FixAbstractMethodsStep (
+			IMetadataResolver cache,
+			Func<AssemblyDefinition> getMonoAndroidAssembly,
+			Action<string> logMessage)
 		{
-			base.Initialize (context, markContext);
-			markContext.RegisterMarkTypeAction (type => ProcessType (type));
+			_injectedCache = cache;
+			_injectedGetMonoAndroidAssembly = getMonoAndroidAssembly;
+			_injectedLogMessage = logMessage;
 		}
 
-		bool CheckShouldProcessAssembly (AssemblyDefinition assembly)
-		{
-			if (!Annotations.HasAction (assembly))
-				Annotations.SetAction (assembly, AssemblyAction.Skip);
-
-			if (MonoAndroidHelper.IsFrameworkAssembly (assembly))
-				return false;
-
-			CheckAppDomainUsage (assembly, (string msg) =>
-#if ILLINK
-				Context.LogMessage (MessageContainer.CreateCustomWarningMessage (Context, msg, 6200, new MessageOrigin (), WarnVersion.ILLink5))
-#else   // !ILLINK
-				Context.LogWarning ("XA2000", msg)
-#endif  // !ILLINK
-			);
+		IMetadataResolver TypeCache => _injectedCache ?? Context;
 
-			return assembly.MainModule.HasTypeReference ("Java.Lang.Object");
-		}
-
-		void UpdateAssemblyAction (AssemblyDefinition assembly)
-		{
-			if (Annotations.GetAction (assembly) == AssemblyAction.Copy)
-				Annotations.SetAction (assembly, AssemblyAction.Save);
-		}
-
-		void ProcessType (TypeDefinition type)
+		public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
 		{
-			var assembly = type.Module.Assembly;
-			if (!CheckShouldProcessAssembly (assembly))
-				return;
-
-			if (!MightNeedFix (type))
-				return;
-
-			if (!FixAbstractMethods (type))
+			// Only run this step on non-main user Android assemblies
+			if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
 				return;
 
-			UpdateAssemblyAction (assembly);
-			MarkAbstractMethodErrorType ();
+			context.IsAssemblyModified |= FixAbstractMethods (assembly);
 		}
 
 		internal bool FixAbstractMethods (AssemblyDefinition assembly)
@@ -84,17 +59,6 @@ internal bool FixAbstractMethods (AssemblyDefinition assembly)
 			return changed;
 		}
 
-#if !ILLINK
-		public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
-		{
-			// Only run this step on non-main user Android assemblies
-			if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
-				return;
-
-			context.IsAssemblyModified |= FixAbstractMethods (assembly);
-		}
-#endif  // !ILLINK
-
 		readonly HashSet<string> warnedAssemblies = new (StringComparer.Ordinal);
 
 		internal void CheckAppDomainUsage (AssemblyDefinition assembly, Action<string> warn)
@@ -114,7 +78,7 @@ internal void CheckAppDomainUsage (AssemblyDefinition assembly, Action<string> w
 
 		bool MightNeedFix (TypeDefinition type)
 		{
-			return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", cache);
+			return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", TypeCache);
 		}
 
 		bool CompareTypes (TypeReference iType, TypeReference tType)
@@ -140,11 +104,11 @@ bool CompareTypes (TypeReference iType, TypeReference tType)
 			if (iType.Namespace != tType.Namespace)
 				return false;
 
-			TypeDefinition iTypeDef = cache.Resolve (iType);
+			TypeDefinition iTypeDef = TypeCache.Resolve (iType);
 			if (iTypeDef == null)
 				return false;
 
-			TypeDefinition tTypeDef = cache.Resolve (tType);
+			TypeDefinition tTypeDef = TypeCache.Resolve (tType);
 			if (tTypeDef == null)
 				return false;
 
@@ -174,7 +138,7 @@ bool IsInOverrides (MethodDefinition iMethod, MethodDefinition tMethod)
 				return false;
 
 			foreach (var o in tMethod.Overrides)
-				if (o != null && iMethod.Name == o.Name && iMethod == cache.Resolve (o))
+				if (o != null && iMethod.Name == o.Name && iMethod == TypeCache.Resolve (o))
 					return true;
 
 			return false;
@@ -223,12 +187,12 @@ bool FixAbstractMethods (TypeDefinition type)
 
 			bool rv = false;
 			List<MethodDefinition> typeMethods = new List<MethodDefinition> (type.Methods);
-			foreach (var baseType in type.GetBaseTypes (cache))
+			foreach (var baseType in type.GetBaseTypes (TypeCache))
 				typeMethods.AddRange (baseType.Methods);
 
 			foreach (var ifaceInfo in type.Interfaces) {
 				var iface    = ifaceInfo.InterfaceType;
-				var ifaceDef = cache.Resolve (iface);
+				var ifaceDef = TypeCache.Resolve (iface);
 				if (ifaceDef == null) {
 					LogMessage ($"Unable to unresolve interface: {iface.FullName}");
 					continue;
@@ -308,33 +272,20 @@ MethodDefinition AbstractMethodErrorConstructor {
 			}
 		}
 
-		bool markedAbstractMethodErrorType;
-
-		void MarkAbstractMethodErrorType ()
+		public override void LogMessage (string message)
 		{
-			if (markedAbstractMethodErrorType)
+			if (_injectedLogMessage != null) {
+				_injectedLogMessage (message);
 				return;
-			markedAbstractMethodErrorType = true;
-
-
-			var td = AbstractMethodErrorConstructor.DeclaringType;
-			Annotations.Mark (td);
-			Annotations.SetPreserve (td, TypePreserve.Nothing);
-			Annotations.AddPreservedMethod (td, AbstractMethodErrorConstructor);
-		}
-
-		public virtual void LogMessage (string message)
-		{
-			Context.LogMessage (message);
+			}
+			base.LogMessage (message);
 		}
 
 		AssemblyDefinition GetMonoAndroidAssembly ()
 		{
-#if !ILLINK
+			if (_injectedGetMonoAndroidAssembly != null)
+				return _injectedGetMonoAndroidAssembly ();
 			return Context.Resolver.GetAssembly ("Mono.Android.dll");
-#else   // ILLINK
-			return Context.GetLoadedAssembly ("Mono.Android");
-#endif  // ILLINK
 		}
 	}
 }

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/PostTrimmingFixAbstractMethodsStep.cs

@@ -0,0 +1,44 @@
+#nullable enable
+
+using System;
+using Java.Interop.Tools.Cecil;
+using Microsoft.Android.Build.Tasks;
+using Mono.Cecil;
+using Xamarin.Android.Tasks;
+
+namespace MonoDroid.Tuner;
+
+/// <summary>
+/// Post-trimming version of FixAbstractMethodsStep that delegates to the core logic
+/// in <see cref="FixAbstractMethodsStep"/>. Skips framework assemblies, checks for
+/// AppDomain.CreateDomain usage, and fixes missing abstract method implementations
+/// on Java.Lang.Object subclasses.
+/// </summary>
+class PostTrimmingFixAbstractMethodsStep : IAssemblyModifierPipelineStep
+{
+	readonly FixAbstractMethodsStep _step;
+	readonly Action<string> _warn;
+
+	public PostTrimmingFixAbstractMethodsStep (
+		IMetadataResolver cache,
+		Func<AssemblyDefinition?> getMonoAndroidAssembly,
+		Action<string> logMessage,
+		Action<string> warn)
+	{
+		_step = new FixAbstractMethodsStep (cache, getMonoAndroidAssembly, logMessage);
+		_warn = warn;
+	}
+
+	public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+	{
+		if (MonoAndroidHelper.IsFrameworkAssembly (assembly))
+			return;
+
+		_step.CheckAppDomainUsage (assembly, _warn);
+
+		if (!assembly.MainModule.HasTypeReference ("Java.Lang.Object"))
+			return;
+
+		context.IsAssemblyModified |= _step.FixAbstractMethods (assembly);
+	}
+}

src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets

@@ -195,7 +195,6 @@
       <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="MonoDroid.Tuner.PreserveApplications" />
       <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="Microsoft.Android.Sdk.ILLink.PreserveRegistrations" />
       <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="Microsoft.Android.Sdk.ILLink.PreserveJavaInterfaces" />
-      <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="MonoDroid.Tuner.FixAbstractMethodsStep" />
       <!-- Custom steps that run after MarkStep -->
       <!-- Custom steps that run after CleanStep -->
       <_TrimmerCustomSteps

src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs

@@ -1,7 +1,6 @@
 #nullable enable
 
 using System;
-using Mono.Linker.Steps;
 using MonoDroid.Tuner;
 
 namespace Xamarin.Android.Tasks
@@ -22,7 +21,7 @@ protected override void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkCon
 		{
 			// FixAbstractMethodsStep
 			var fixAbstractMethodsStep = new FixAbstractMethodsStep ();
-			fixAbstractMethodsStep.Initialize (context, new EmptyMarkContext ());
+			fixAbstractMethodsStep.Initialize (context);
 			pipeline.Steps.Add (fixAbstractMethodsStep);
 
 			// FixLegacyResourceDesignerStep

src/Xamarin.Android.Build.Tasks/Tasks/PostTrimmingPipeline.cs

@@ -47,6 +47,22 @@ public override bool RunTask ()
 
 		var steps = new List<IAssemblyModifierPipelineStep> ();
 		steps.Add (new StripEmbeddedLibrariesStep (Log));
+
+		// FixAbstractMethods — resolve Mono.Android once up front. If resolution fails, log
+		// the error and skip running the fix step entirely to avoid later unhandled exceptions.
+		AssemblyDefinition? monoAndroidAssembly = null;
+		try {
+			monoAndroidAssembly = resolver.Resolve (AssemblyNameReference.Parse ("Mono.Android"));
+		} catch (AssemblyResolutionException ex) {
+			Log.LogErrorFromException (ex, showStackTrace: false);
+		}
+		if (monoAndroidAssembly != null) {
+			steps.Add (new PostTrimmingFixAbstractMethodsStep (cache,
+				() => monoAndroidAssembly,
+				(msg) => Log.LogDebugMessage (msg),
+				(msg) => Log.LogCodedWarning ("XA2000", msg)));
+		}
+
 		if (AddKeepAlives) {
 			// Memoize the corlib resolution so the attempt (and any error logging) happens at most once,
 			// regardless of how many assemblies/methods need KeepAlive injection.

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

@@ -8,7 +8,6 @@
 using Mono.Cecil;
 using Mono.Cecil.Cil;
 using Mono.Linker;
-using Mono.Linker.Steps;
 using Mono.Tuner;
 using MonoDroid.Tuner;
 using NUnit.Framework;
@@ -33,7 +32,7 @@ public void FixAbstractMethodsStep_SkipDimMembers ()
 
 			using (var resolver = new DirectoryAssemblyResolver (Logger, false))
 			using (var context = new LinkContext (resolver)) {
-				step.Initialize (context, new EmptyMarkContext ());
+				step.Initialize (context);
 				resolver.SearchDirectories.Add (path);
 
 				var myAssemblyPath = Path.Combine (path, "MyAssembly.dll");
@@ -92,7 +91,7 @@ public void FixAbstractMethodsStep_Explicit ()
 
 			using (var resolver = new DirectoryAssemblyResolver (Logger, false))
 			using (var context = new LinkContext (resolver)) {
-				step.Initialize (context, new EmptyMarkContext ());
+				step.Initialize (context);
 				resolver.SearchDirectories.Add (path);
 
 				var myAssemblyPath = Path.Combine (path, "MyAssembly.dll");

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj

@@ -56,6 +56,7 @@
     <Compile Include="Linker\MonoDroid.Tuner\AndroidLinkConfiguration.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\Extensions.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\FixAbstractMethodsStep.cs" />
+    <Compile Include="Linker\MonoDroid.Tuner\PostTrimmingFixAbstractMethodsStep.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\LinkDesignerBase.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\RemoveResourceDesignerStep.cs" />