From d668de1c112abc4c708af72bbcbe0ab546bec10d Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 15 Aug 2024 23:42:37 +0000 Subject: [PATCH 1/4] Run FixAbstractMethodsStep before MarkStep --- .../MonoDroid.Tuner/FixAbstractMethodsStep.cs | 73 ++++++++++++++----- .../Microsoft.Android.Sdk.ILLink.targets | 2 +- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs index 70de2500931..a71b65f68c8 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs @@ -23,31 +23,19 @@ namespace MonoDroid.Tuner /// /// NOTE: this step is subclassed so it can be called directly from Xamarin.Android.Build.Tasks /// - public class FixAbstractMethodsStep : -#if ILLINK - BaseMarkHandler -#else // !ILLINK - BaseStep -#endif // !ILLINK + public class FixAbstractMethodsStep : BaseStep { + readonly IMetadataResolver cache; #if ILLINK - public override void Initialize (LinkContext context, MarkContext markContext) - { - this.cache = context; - base.Initialize (context, markContext); - markContext.RegisterMarkTypeAction (type => ProcessType (type)); - } -#else // !ILLINK + readonly List assemblies = new (); +#endif // ILLINK + public FixAbstractMethodsStep (IMetadataResolver cache) { this.cache = cache; } - readonly -#endif // !ILLINK - IMetadataResolver cache; - bool CheckShouldProcessAssembly (AssemblyDefinition assembly) { if (!Annotations.HasAction (assembly)) @@ -74,6 +62,49 @@ void UpdateAssemblyAction (AssemblyDefinition assembly) } #if ILLINK + protected override void ProcessAssembly (AssemblyDefinition assembly) + { + assemblies.Add (assembly); + } + + protected override void EndProcess () + { + foreach (var assembly in GetReferencedAssemblies().ToList()) { + ProcessAssembly_Actual(assembly); + } + + IEnumerable GetReferencedAssemblies () + { + var loaded = new HashSet (assemblies); + var toProcess = new Queue (assemblies); + + while (toProcess.Count > 0) { + var assembly = toProcess.Dequeue (); + foreach (var reference in ResolveReferences (assembly)) { + if (!loaded.Add (reference)) + continue; + yield return reference; + toProcess.Enqueue (reference); + } + } + } + + ICollection ResolveReferences (AssemblyDefinition assembly) + { + List references = new List (); + if (assembly == null) + return references; + + foreach (AssemblyNameReference reference in assembly.MainModule.AssemblyReferences) { + AssemblyDefinition? definition = cache.Resolve (reference); + if (definition != null) + references.Add (definition); + } + + return references; + } + } + protected void ProcessType (TypeDefinition type) { var assembly = type.Module.Assembly; @@ -89,8 +120,13 @@ protected void ProcessType (TypeDefinition type) UpdateAssemblyAction (assembly); MarkAbstractMethodErrorType (); } -#else // !ILLINK +#endif // ILLINK + +#if ILLINK + void ProcessAssembly_Actual (AssemblyDefinition assembly) +#else // !ILLINK protected override void ProcessAssembly (AssemblyDefinition assembly) +#endif // !ILLINK { if (!CheckShouldProcessAssembly (assembly)) return; @@ -111,7 +147,6 @@ internal bool FixAbstractMethods (AssemblyDefinition assembly) } return changed; } -#endif // !ILLINK readonly HashSet warnedAssemblies = new (StringComparer.Ordinal); diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets index e99bfb0e722..d58bb7a0b43 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets @@ -50,6 +50,7 @@ This file contains the .NET 5-specific targets to customize ILLink <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" BeforeStep="MarkStep" Type="Microsoft.Android.Sdk.ILLink.SetupStep" /> + <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" BeforeStep="MarkStep" Type="MonoDroid.Tuner.FixAbstractMethodsStep" /> <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="Microsoft.Android.Sdk.ILLink.PreserveSubStepDispatcher" /> <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="MonoDroid.Tuner.MarkJavaObjects" /> @@ -57,7 +58,6 @@ This file contains the .NET 5-specific targets to customize ILLink <_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" /> <_TrimmerCustomSteps Condition=" '$(_ProguardProjectConfiguration)' != '' " From bc4ac8260b11e0ccec445bc7aff75c3d01d6ddf8 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 16 Aug 2024 17:38:25 +0000 Subject: [PATCH 2/4] Fix build --- .../Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs index a71b65f68c8..0794f045f95 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs @@ -96,7 +96,7 @@ ICollection ResolveReferences (AssemblyDefinition assembly) return references; foreach (AssemblyNameReference reference in assembly.MainModule.AssemblyReferences) { - AssemblyDefinition? definition = cache.Resolve (reference); + AssemblyDefinition? definition = Context.Resolve (reference); if (definition != null) references.Add (definition); } @@ -132,7 +132,9 @@ protected override void ProcessAssembly (AssemblyDefinition assembly) return; if (FixAbstractMethods (assembly)) { +#if !ILLINK Context.SafeReadSymbols (assembly); +#endif // !ILLINK UpdateAssemblyAction (assembly); MarkAbstractMethodErrorType (); } From bee585130d626fb4277f27ec93c4d3f68e4bcae7 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 16 Aug 2024 17:47:52 +0000 Subject: [PATCH 3/4] Fix cache, remove ctor for ILLINK build --- .../Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs index 0794f045f95..eca50839699 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs @@ -25,16 +25,16 @@ namespace MonoDroid.Tuner /// public class FixAbstractMethodsStep : BaseStep { - readonly IMetadataResolver cache; - #if ILLINK - readonly List assemblies = new (); -#endif // ILLINK + IMetadataResolver cache => Context; +#else // !ILLINK + readonly IMetadataResolver cache; public FixAbstractMethodsStep (IMetadataResolver cache) { this.cache = cache; } +#endif // !ILLINK bool CheckShouldProcessAssembly (AssemblyDefinition assembly) { @@ -62,6 +62,8 @@ void UpdateAssemblyAction (AssemblyDefinition assembly) } #if ILLINK + readonly List assemblies = new (); + protected override void ProcessAssembly (AssemblyDefinition assembly) { assemblies.Add (assembly); From 89904e858b2eed371e4b7f6fa3d75415ce896465 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 6 Sep 2024 16:56:37 +0000 Subject: [PATCH 4/4] Check nested types --- .../MonoDroid.Tuner/FixAbstractMethodsStep.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs index eca50839699..bcc78f1cf6f 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs @@ -113,9 +113,6 @@ protected void ProcessType (TypeDefinition type) if (!CheckShouldProcessAssembly (assembly)) return; - if (!MightNeedFix (type)) - return; - if (!FixAbstractMethods (type)) return; @@ -146,10 +143,18 @@ internal bool FixAbstractMethods (AssemblyDefinition assembly) { bool changed = false; foreach (var type in assembly.MainModule.Types) { - if (MightNeedFix (type)) - changed |= FixAbstractMethods (type); + changed |= FixAbstractMethodsNested (type); } return changed; + + bool FixAbstractMethodsNested (TypeDefinition type) + { + bool changed = FixAbstractMethods (type); + foreach (var nested in type.NestedTypes) { + changed |= FixAbstractMethodsNested (nested); + } + return changed; + } } readonly HashSet warnedAssemblies = new (StringComparer.Ordinal); @@ -281,6 +286,9 @@ bool HaveSameSignature (TypeReference iface, MethodDefinition iMethod, MethodDef bool FixAbstractMethods (TypeDefinition type) { + if (!MightNeedFix (type)) + return false; + if (!type.HasInterfaces) return false;