Skip to content

Commit 7ae24aa

Browse files
sbomersbomer
committed
[xabt] Move FixAbstractMethodsStep from ILLink trimmer to post-trim pipeline
Move FixAbstractMethodsStep out of the ILLink MarkStep handler into the PostTrimmingPipeline MSBuild task (runs AfterTargets=ILLink). The step now extends BaseStep instead of BaseMarkHandler, with an internal constructor for dependency injection used by the new thin PostTrimmingFixAbstractMethodsStep wrapper. The no-trim path (LinkAssembliesNoShrink) continues to work via BaseStep.Initialize. Preserve Java.Lang.AbstractMethodError via linker descriptor XML since the step no longer calls Annotations.Mark().
1 parent 6cc34f5 commit 7ae24aa

File tree

9 files changed

+109
-94
lines changed

9 files changed

+109
-94
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
<Compile Include="..\Xamarin.Android.Build.Tasks\Linker\External\Linker\BaseMarkHandler.cs" Link="External\BaseMarkHandler.cs" />
1717
<Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\AndroidLinkConfiguration.cs" Link="MonoDroid.Tuner\AndroidLinkConfiguration.cs" />
1818
<Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\Extensions.cs" Link="MonoDroid.Tuner\Extensions.cs" />
19-
<Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\FixAbstractMethodsStep.cs" Link="MonoDroid.Tuner\FixAbstractMethodsStep.cs" />
2019
<Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" Link="MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" />
2120
<Compile Include="..\Xamarin.Android.Build.Tasks\Linker\MonoDroid.Tuner\LinkDesignerBase.cs" Link="MonoDroid.Tuner\LinkDesignerBase.cs" />
2221
<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‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
<type fullname="Android.Runtime.XmlReaderPullParser" />
3535
-->
3636
<type fullname="Java.Interop.TypeManager*JavaTypeManager" />
37+
<type fullname="Java.Lang.AbstractMethodError">
38+
<method name=".ctor" />
39+
</type>
3740
<type fullname="Java.Lang.Object">
3841
<method name="SetHandleOnDeserialized" />
3942
</type>

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

Lines changed: 38 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -10,68 +10,43 @@
1010
using Mono.Linker.Steps;
1111
using Mono.Tuner;
1212
using Xamarin.Android.Tasks;
13-
14-
#if ILLINK
15-
using Resources = Microsoft.Android.Sdk.ILLink.Properties.Resources;
16-
#else // !ILLINK
1713
using Resources = Xamarin.Android.Tasks.Properties.Resources;
18-
#endif // !ILLINK
1914

2015
namespace MonoDroid.Tuner
2116
{
22-
/// <summary>
23-
/// NOTE: this step is subclassed so it can be called directly from Xamarin.Android.Build.Tasks
24-
/// </summary>
25-
public class FixAbstractMethodsStep : BaseMarkHandler
26-
#if !ILLINK
27-
, IAssemblyModifierPipelineStep
28-
#endif // !ILLINK
17+
public class FixAbstractMethodsStep : BaseStep, IAssemblyModifierPipelineStep
2918
{
30-
public override void Initialize (LinkContext context, MarkContext markContext)
19+
readonly IMetadataResolver _injectedCache;
20+
readonly Func<AssemblyDefinition> _injectedGetMonoAndroidAssembly;
21+
readonly Action<string> _injectedLogMessage;
22+
23+
/// <summary>
24+
/// Used by LinkAssembliesNoShrink and tests. Call <see cref="BaseStep.Initialize(LinkContext)"/> before use.
25+
/// </summary>
26+
public FixAbstractMethodsStep () { }
27+
28+
/// <summary>
29+
/// Used by <see cref="PostTrimmingFixAbstractMethodsStep"/> when no <see cref="LinkContext"/> is available.
30+
/// </summary>
31+
internal FixAbstractMethodsStep (
32+
IMetadataResolver cache,
33+
Func<AssemblyDefinition> getMonoAndroidAssembly,
34+
Action<string> logMessage)
3135
{
32-
base.Initialize (context, markContext);
33-
markContext.RegisterMarkTypeAction (type => ProcessType (type));
36+
_injectedCache = cache;
37+
_injectedGetMonoAndroidAssembly = getMonoAndroidAssembly;
38+
_injectedLogMessage = logMessage;
3439
}
3540

36-
bool CheckShouldProcessAssembly (AssemblyDefinition assembly)
37-
{
38-
if (!Annotations.HasAction (assembly))
39-
Annotations.SetAction (assembly, AssemblyAction.Skip);
40-
41-
if (MonoAndroidHelper.IsFrameworkAssembly (assembly))
42-
return false;
43-
44-
CheckAppDomainUsage (assembly, (string msg) =>
45-
#if ILLINK
46-
Context.LogMessage (MessageContainer.CreateCustomWarningMessage (Context, msg, 6200, new MessageOrigin (), WarnVersion.ILLink5))
47-
#else // !ILLINK
48-
Context.LogWarning ("XA2000", msg)
49-
#endif // !ILLINK
50-
);
41+
IMetadataResolver TypeCache => _injectedCache ?? Context;
5142

52-
return assembly.MainModule.HasTypeReference ("Java.Lang.Object");
53-
}
54-
55-
void UpdateAssemblyAction (AssemblyDefinition assembly)
56-
{
57-
if (Annotations.GetAction (assembly) == AssemblyAction.Copy)
58-
Annotations.SetAction (assembly, AssemblyAction.Save);
59-
}
60-
61-
void ProcessType (TypeDefinition type)
43+
public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
6244
{
63-
var assembly = type.Module.Assembly;
64-
if (!CheckShouldProcessAssembly (assembly))
65-
return;
66-
67-
if (!MightNeedFix (type))
68-
return;
69-
70-
if (!FixAbstractMethods (type))
45+
// Only run this step on non-main user Android assemblies
46+
if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
7147
return;
7248

73-
UpdateAssemblyAction (assembly);
74-
MarkAbstractMethodErrorType ();
49+
context.IsAssemblyModified |= FixAbstractMethods (assembly);
7550
}
7651

7752
internal bool FixAbstractMethods (AssemblyDefinition assembly)
@@ -84,17 +59,6 @@ internal bool FixAbstractMethods (AssemblyDefinition assembly)
8459
return changed;
8560
}
8661

87-
#if !ILLINK
88-
public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
89-
{
90-
// Only run this step on non-main user Android assemblies
91-
if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
92-
return;
93-
94-
context.IsAssemblyModified |= FixAbstractMethods (assembly);
95-
}
96-
#endif // !ILLINK
97-
9862
readonly HashSet<string> warnedAssemblies = new (StringComparer.Ordinal);
9963

10064
internal void CheckAppDomainUsage (AssemblyDefinition assembly, Action<string> warn)
@@ -114,7 +78,7 @@ internal void CheckAppDomainUsage (AssemblyDefinition assembly, Action<string> w
11478

11579
bool MightNeedFix (TypeDefinition type)
11680
{
117-
return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", cache);
81+
return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", TypeCache);
11882
}
11983

12084
bool CompareTypes (TypeReference iType, TypeReference tType)
@@ -140,11 +104,11 @@ bool CompareTypes (TypeReference iType, TypeReference tType)
140104
if (iType.Namespace != tType.Namespace)
141105
return false;
142106

143-
TypeDefinition iTypeDef = cache.Resolve (iType);
107+
TypeDefinition iTypeDef = TypeCache.Resolve (iType);
144108
if (iTypeDef == null)
145109
return false;
146110

147-
TypeDefinition tTypeDef = cache.Resolve (tType);
111+
TypeDefinition tTypeDef = TypeCache.Resolve (tType);
148112
if (tTypeDef == null)
149113
return false;
150114

@@ -174,7 +138,7 @@ bool IsInOverrides (MethodDefinition iMethod, MethodDefinition tMethod)
174138
return false;
175139

176140
foreach (var o in tMethod.Overrides)
177-
if (o != null && iMethod.Name == o.Name && iMethod == cache.Resolve (o))
141+
if (o != null && iMethod.Name == o.Name && iMethod == TypeCache.Resolve (o))
178142
return true;
179143

180144
return false;
@@ -223,12 +187,12 @@ bool FixAbstractMethods (TypeDefinition type)
223187

224188
bool rv = false;
225189
List<MethodDefinition> typeMethods = new List<MethodDefinition> (type.Methods);
226-
foreach (var baseType in type.GetBaseTypes (cache))
190+
foreach (var baseType in type.GetBaseTypes (TypeCache))
227191
typeMethods.AddRange (baseType.Methods);
228192

229193
foreach (var ifaceInfo in type.Interfaces) {
230194
var iface = ifaceInfo.InterfaceType;
231-
var ifaceDef = cache.Resolve (iface);
195+
var ifaceDef = TypeCache.Resolve (iface);
232196
if (ifaceDef == null) {
233197
LogMessage ($"Unable to unresolve interface: {iface.FullName}");
234198
continue;
@@ -308,33 +272,20 @@ MethodDefinition AbstractMethodErrorConstructor {
308272
}
309273
}
310274

311-
bool markedAbstractMethodErrorType;
312-
313-
void MarkAbstractMethodErrorType ()
275+
public override void LogMessage (string message)
314276
{
315-
if (markedAbstractMethodErrorType)
277+
if (_injectedLogMessage != null) {
278+
_injectedLogMessage (message);
316279
return;
317-
markedAbstractMethodErrorType = true;
318-
319-
320-
var td = AbstractMethodErrorConstructor.DeclaringType;
321-
Annotations.Mark (td);
322-
Annotations.SetPreserve (td, TypePreserve.Nothing);
323-
Annotations.AddPreservedMethod (td, AbstractMethodErrorConstructor);
324-
}
325-
326-
public virtual void LogMessage (string message)
327-
{
328-
Context.LogMessage (message);
280+
}
281+
base.LogMessage (message);
329282
}
330283

331284
AssemblyDefinition GetMonoAndroidAssembly ()
332285
{
333-
#if !ILLINK
286+
if (_injectedGetMonoAndroidAssembly != null)
287+
return _injectedGetMonoAndroidAssembly ();
334288
return Context.Resolver.GetAssembly ("Mono.Android.dll");
335-
#else // ILLINK
336-
return Context.GetLoadedAssembly ("Mono.Android");
337-
#endif // ILLINK
338289
}
339290
}
340291
}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#nullable enable
2+
3+
using System;
4+
using Java.Interop.Tools.Cecil;
5+
using Microsoft.Android.Build.Tasks;
6+
using Mono.Cecil;
7+
using Xamarin.Android.Tasks;
8+
9+
namespace MonoDroid.Tuner;
10+
11+
/// <summary>
12+
/// Post-trimming version of FixAbstractMethodsStep that delegates to the core logic
13+
/// in <see cref="FixAbstractMethodsStep"/>. Skips framework assemblies, checks for
14+
/// AppDomain.CreateDomain usage, and fixes missing abstract method implementations
15+
/// on Java.Lang.Object subclasses.
16+
/// </summary>
17+
class PostTrimmingFixAbstractMethodsStep : IAssemblyModifierPipelineStep
18+
{
19+
readonly FixAbstractMethodsStep _step;
20+
readonly Action<string> _warn;
21+
22+
public PostTrimmingFixAbstractMethodsStep (
23+
IMetadataResolver cache,
24+
Func<AssemblyDefinition?> getMonoAndroidAssembly,
25+
Action<string> logMessage,
26+
Action<string> warn)
27+
{
28+
_step = new FixAbstractMethodsStep (cache, getMonoAndroidAssembly, logMessage);
29+
_warn = warn;
30+
}
31+
32+
public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
33+
{
34+
if (MonoAndroidHelper.IsFrameworkAssembly (assembly))
35+
return;
36+
37+
_step.CheckAppDomainUsage (assembly, _warn);
38+
39+
if (!assembly.MainModule.HasTypeReference ("Java.Lang.Object"))
40+
return;
41+
42+
context.IsAssemblyModified |= _step.FixAbstractMethods (assembly);
43+
}
44+
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@
195195
<_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="MonoDroid.Tuner.PreserveApplications" />
196196
<_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="Microsoft.Android.Sdk.ILLink.PreserveRegistrations" />
197197
<_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="Microsoft.Android.Sdk.ILLink.PreserveJavaInterfaces" />
198-
<_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" Type="MonoDroid.Tuner.FixAbstractMethodsStep" />
199198
<!-- Custom steps that run after MarkStep -->
200199
<!-- Custom steps that run after CleanStep -->
201200
<_TrimmerCustomSteps

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#nullable enable
22

33
using System;
4-
using Mono.Linker.Steps;
54
using MonoDroid.Tuner;
65

76
namespace Xamarin.Android.Tasks
@@ -22,7 +21,7 @@ protected override void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkCon
2221
{
2322
// FixAbstractMethodsStep
2423
var fixAbstractMethodsStep = new FixAbstractMethodsStep ();
25-
fixAbstractMethodsStep.Initialize (context, new EmptyMarkContext ());
24+
fixAbstractMethodsStep.Initialize (context);
2625
pipeline.Steps.Add (fixAbstractMethodsStep);
2726

2827
// FixLegacyResourceDesignerStep

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,26 @@ public override bool RunTask ()
4747

4848
var steps = new List<IAssemblyModifierPipelineStep> ();
4949
steps.Add (new StripEmbeddedLibrariesStep (Log));
50+
51+
// FixAbstractMethods — memoize Mono.Android resolution so the attempt (and any error
52+
// logging) happens at most once, regardless of how many assemblies need fixing.
53+
AssemblyDefinition? monoAndroidAssembly = null;
54+
bool monoAndroidResolutionAttempted = false;
55+
steps.Add (new PostTrimmingFixAbstractMethodsStep (cache,
56+
() => {
57+
if (!monoAndroidResolutionAttempted) {
58+
monoAndroidResolutionAttempted = true;
59+
try {
60+
monoAndroidAssembly = resolver.Resolve (AssemblyNameReference.Parse ("Mono.Android"));
61+
} catch (AssemblyResolutionException ex) {
62+
Log.LogErrorFromException (ex, showStackTrace: false);
63+
}
64+
}
65+
return monoAndroidAssembly;
66+
},
67+
(msg) => Log.LogDebugMessage (msg),
68+
(msg) => Log.LogCodedWarning ("XA2000", msg)));
69+
5070
if (AddKeepAlives) {
5171
// Memoize the corlib resolution so the attempt (and any error logging) happens at most once,
5272
// regardless of how many assemblies/methods need KeepAlive injection.

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using Mono.Cecil;
99
using Mono.Cecil.Cil;
1010
using Mono.Linker;
11-
using Mono.Linker.Steps;
1211
using Mono.Tuner;
1312
using MonoDroid.Tuner;
1413
using NUnit.Framework;
@@ -33,7 +32,7 @@ public void FixAbstractMethodsStep_SkipDimMembers ()
3332

3433
using (var resolver = new DirectoryAssemblyResolver (Logger, false))
3534
using (var context = new LinkContext (resolver)) {
36-
step.Initialize (context, new EmptyMarkContext ());
35+
step.Initialize (context);
3736
resolver.SearchDirectories.Add (path);
3837

3938
var myAssemblyPath = Path.Combine (path, "MyAssembly.dll");
@@ -92,7 +91,7 @@ public void FixAbstractMethodsStep_Explicit ()
9291

9392
using (var resolver = new DirectoryAssemblyResolver (Logger, false))
9493
using (var context = new LinkContext (resolver)) {
95-
step.Initialize (context, new EmptyMarkContext ());
94+
step.Initialize (context);
9695
resolver.SearchDirectories.Add (path);
9796

9897
var myAssemblyPath = Path.Combine (path, "MyAssembly.dll");

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
<Compile Include="Linker\MonoDroid.Tuner\AndroidLinkConfiguration.cs" />
5757
<Compile Include="Linker\MonoDroid.Tuner\Extensions.cs" />
5858
<Compile Include="Linker\MonoDroid.Tuner\FixAbstractMethodsStep.cs" />
59+
<Compile Include="Linker\MonoDroid.Tuner\PostTrimmingFixAbstractMethodsStep.cs" />
5960
<Compile Include="Linker\MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" />
6061
<Compile Include="Linker\MonoDroid.Tuner\LinkDesignerBase.cs" />
6162
<Compile Include="Linker\MonoDroid.Tuner\RemoveResourceDesignerStep.cs" />

0 commit comments

Comments
 (0)