Skip to content

Commit

Permalink
[linker] Update to new linker custom steps API (#5748)
Browse files Browse the repository at this point in the history
Context: dotnet/linker#1953
Context: dotnet/linker#1774
Context: dotnet/linker#1774 (comment)
Context: dotnet/linker#1979
Context: dotnet/linker#2019
Context: dotnet/linker#2045
Context: dotnet/java-interop@2573dc8
Context: #5870
Context: #5878

Update the custom linker steps to use the new `IMarkHandler` API
which runs logic during "MarkStep".

(See [this list][0] of pipeline steps for additional context.)

As part of this, I've removed the workaround that loads all referenced
assemblies up-front and simplified some of the linker MSBuild targets.
Some of the `MonoDroid.Tuner` steps were duplicated and changed to
implement the new `IMarkHandler` interface, to avoid touching the
`MonoDroid.Tuner` code.

In my analysis of the custom steps, most of them "just work" if we
call them only for marked members, because they ultimately either:

  - Just call `AddPreserved*()` to conditionally keep members of types
    (which with the new API will just happen when the type is marked)

  - In the case of `FixAbstractMethodsStep()`, inject missing interface
    implementations which will only be kept if they are marked for some
    other reason.

Most of the steps have been updated in a straightforward way based on
the above.

The exceptions are:

  - `AddKeepAlivesStep` needs to run on all code that survived
    linking, and can run as a normal step.

  - `ApplyPreserveAttribute`: this step globally marks members with
    `PreserveAttribute`.
    - The step is only active for non-SDK link assemblies.  Usually we
      root non-SDK assemblies, but it will be a problem if linking all
      assemblies.
    - For now, I updated it to use the new custom step API on assembly
      mark -- so it will scan for the attribute in all marked
      assemblies -- but we should investigate whether this is good
      enough.
    - This can't be migrated to use `IMarkHandler` because it needs
      to scan code for attributes, including unmarked code.

  - `PreserveExportedTypes`: similar to the above, this globally marks
    members with `Export*Attribute`.  It's used for java interop in
    cases where the java methods aren't referenced statically, like
    from xml, or for special serialization methods.
    - It's only active for non-SDK assemblies, so like above it will
      be a problem only if linking all assemblies.
    - Like above, I've made it scan marked assemblies.

  - `PreserveApplications`: globally scans for `ApplicationAttribute`
    on types/assemblies, and sets the `TypePreserve` annotation for
    any types referenced by the attribute.
    - This step technically does a global scan, but it's likely to work
      on "mark type"/"mark assembly", as `ApplicationAttribute` is only
      used on types/assembies that are already kept.
    - I've updated it to use the new `IMarkHandler` API.

Additionally, as per dotnet/java-interop@2573dc8c, stop using
`TypeDefinitionCache` everywhere and instead use `IMetadataResolver`
to support caching `TypeDefinition`s across shared steps.
For .NET 6, `LinkContextMetadataResolver` implements the
`IMetadataResolver` interface in terms of `LinkContext`.

[0]: https://github.com/mono/linker/blob/main/src/linker/Linker/Driver.cs#L714-L730
  • Loading branch information
sbomer authored Jun 2, 2021
1 parent 6a5df16 commit 45a9adc
Show file tree
Hide file tree
Showing 16 changed files with 321 additions and 194 deletions.
19 changes: 19 additions & 0 deletions src/Microsoft.Android.Sdk.ILLink/BaseMarkHandler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Microsoft.Android.Sdk.ILLink;
using Mono.Cecil;
using Mono.Linker.Steps;

namespace Mono.Linker
{
public class BaseMarkHandler : IMarkHandler
{
protected LinkContext Context;
protected AnnotationStore Annotations => Context?.Annotations;
protected IMetadataResolver cache;

public virtual void Initialize (LinkContext context, MarkContext markContext)
{
Context = context;
cache = context;
}
}
}
18 changes: 13 additions & 5 deletions src/Microsoft.Android.Sdk.ILLink/PreserveJavaInterfaces.cs
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
using Mono.Cecil;
using Mono.Linker;
using Mono.Linker.Steps;
using MonoDroid.Tuner;

namespace Microsoft.Android.Sdk.ILLink
{
class PreserveJavaInterfaces : BaseSubStep
class PreserveJavaInterfaces : BaseMarkHandler
{
public override bool IsActiveFor (AssemblyDefinition assembly)
public override void Initialize (LinkContext context, MarkContext markContext)
{
return assembly.Name.Name == "Mono.Android" || assembly.MainModule.HasTypeReference ("Android.Runtime.IJavaObject");
base.Initialize (context, markContext);
markContext.RegisterMarkTypeAction (type => ProcessType (type));
}

public override SubStepTargets Targets { get { return SubStepTargets.Type; } }
bool IsActiveFor (AssemblyDefinition assembly)
{
return assembly.Name.Name == "Mono.Android" || assembly.MainModule.HasTypeReference ("Android.Runtime.IJavaObject");
}

public override void ProcessType (TypeDefinition type)
void ProcessType (TypeDefinition type)
{
if (!IsActiveFor (type.Module.Assembly))
return;

// If we are preserving a Mono.Android interface,
// preserve all members on the interface.
if (!type.IsInterface)
Expand Down
35 changes: 13 additions & 22 deletions src/Microsoft.Android.Sdk.ILLink/PreserveRegistrations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,19 @@

namespace Microsoft.Android.Sdk.ILLink
{
class PreserveRegistrations : BaseSubStep
class PreserveRegistrations : BaseMarkHandler
{
delegate void AddPreservedMethodDelegate (AnnotationStore store, MethodDefinition key, MethodDefinition method);

static readonly AddPreservedMethodDelegate addPreservedMethod;

readonly TypeDefinitionCache cache;

public PreserveRegistrations (TypeDefinitionCache cache) => this.cache = cache;

static PreserveRegistrations ()
public override void Initialize (LinkContext context, MarkContext markContext)
{
// temporarily use reflection to get void AnnotationStore::AddPreservedMethod (MethodDefinition key, MethodDefinition method)
// this can be removed once we have newer Microsoft.NET.ILLink containing https://github.com/mono/linker/commit/e6dadc995a834603e1178f9a1918f0ae38056b29
var method = typeof (AnnotationStore).GetMethod ("AddPreservedMethod", new Type [] { typeof (MethodDefinition), typeof (MethodDefinition) });
addPreservedMethod = (AddPreservedMethodDelegate)(method != null ? Delegate.CreateDelegate (typeof (AddPreservedMethodDelegate), null, method, false) : null);
base.Initialize (context, markContext);
markContext.RegisterMarkMethodAction (method => ProcessMethod (method));
}

public override bool IsActiveFor (AssemblyDefinition assembly)
bool IsActiveFor (AssemblyDefinition assembly)
{
return addPreservedMethod != null && (assembly.Name.Name == "Mono.Android" || assembly.MainModule.HasTypeReference ("Android.Runtime.RegisterAttribute"));
return assembly.Name.Name == "Mono.Android" || assembly.MainModule.HasTypeReference ("Android.Runtime.RegisterAttribute");
}

public override SubStepTargets Targets { get { return SubStepTargets.Method; } }

bool PreserveJniMarshalMethods ()
{
if (Context.TryGetCustomData ("XAPreserveJniMarshalMethods", out var boolValue))
Expand Down Expand Up @@ -78,13 +66,16 @@ void PreserveRegisteredMethod (TypeDefinition type, string member, MethodDefinit
type = type.BaseType.Resolve ();
}

public override void ProcessMethod (MethodDefinition method)
void ProcessMethod (MethodDefinition method)
{
if (!IsActiveFor (method.Module.Assembly))
return;

bool preserveJniMarshalMethodOnly = false;
if (!method.TryGetRegisterMember (out var member, out var nativeMethod, out var signature)) {
if (PreserveJniMarshalMethods () &&
method.DeclaringType.GetMarshalMethodsType () != null &&
method.TryGetBaseOrInterfaceRegisterMember (cache, out member, out nativeMethod, out signature)) {
method.DeclaringType.GetMarshalMethodsType () != null &&
method.TryGetBaseOrInterfaceRegisterMember (cache, out member, out nativeMethod, out signature)) {
preserveJniMarshalMethodOnly = true;
} else {
return;
Expand All @@ -105,7 +96,7 @@ public override void ProcessMethod (MethodDefinition method)

void AddPreservedMethod (MethodDefinition key, MethodDefinition method)
{
addPreservedMethod.Invoke (Context.Annotations, key, method);
Annotations.AddPreservedMethod (key, method);
}
}
}
19 changes: 19 additions & 0 deletions src/Microsoft.Android.Sdk.ILLink/PreserveSubStepDispatcher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;
using System.Collections.Generic;
using System.Text;
using Mono.Linker.Steps;
using Mono.Tuner;

namespace Microsoft.Android.Sdk.ILLink
{
public class PreserveSubStepDispatcher : MarkSubStepsDispatcher
{
public PreserveSubStepDispatcher ()
: base (new ISubStep[] {
new ApplyPreserveAttribute (),
new PreserveExportedTypes ()
})
{
}
}
}
69 changes: 1 addition & 68 deletions src/Microsoft.Android.Sdk.ILLink/SetupStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,77 +12,10 @@ namespace Microsoft.Android.Sdk.ILLink
{
class SetupStep : BaseStep
{
List<IStep> _steps;

List<IStep> Steps {
get {
if (_steps == null) {
var pipeline = typeof (LinkContext).GetProperty ("Pipeline").GetGetMethod ().Invoke (Context, null);
_steps = (List<IStep>) pipeline.GetType ().GetField ("_steps", BindingFlags.Instance | BindingFlags.NonPublic).GetValue (pipeline);
//foreach (var step in _steps)
// Console.WriteLine ($"step: {step.GetType ().Name}");
}
return _steps;
}
}

static MethodInfo getReferencedAssembliesMethod = typeof (LinkContext).GetMethod ("GetReferencedAssemblies", BindingFlags.Public | BindingFlags.Instance);

protected override void Process ()
{
string tfmPaths;
if (Context.TryGetCustomData ("XATargetFrameworkDirectories", out tfmPaths))
if (Context.TryGetCustomData ("XATargetFrameworkDirectories", out string tfmPaths))
Xamarin.Android.Tasks.MonoAndroidHelper.TargetFrameworkDirectories = tfmPaths.Split (new char [] { ';' });

var subSteps1 = new SubStepDispatcher ();
subSteps1.Add (new ApplyPreserveAttribute ());

var cache = new TypeDefinitionCache ();
var subSteps2 = new SubStepDispatcher ();
subSteps2.Add (new PreserveExportedTypes ());
subSteps2.Add (new MarkJavaObjects ());
subSteps2.Add (new PreserveJavaExceptions ());
subSteps2.Add (new PreserveApplications ());
subSteps2.Add (new PreserveRegistrations (cache));
subSteps2.Add (new PreserveJavaInterfaces ());

InsertAfter (new FixAbstractMethodsStep (cache), "SetupStep");
InsertAfter (subSteps2, "SetupStep");
InsertAfter (subSteps1, "SetupStep");

// temporary workaround: this call forces illink to process all the assemblies
if (getReferencedAssembliesMethod == null)
throw new InvalidOperationException ($"Temporary linker workaround failed, {nameof (getReferencedAssembliesMethod)} is null.");

foreach (var assembly in (IEnumerable<AssemblyDefinition>)getReferencedAssembliesMethod.Invoke (Context, null))
Context.LogMessage ($"Reference assembly to process: {assembly}");

string proguardPath;
if (Context.TryGetCustomData ("ProguardConfiguration", out proguardPath))
InsertAfter (new GenerateProguardConfiguration (proguardPath), "CleanStep");

string addKeepAlivesStep;
if (Context.TryGetCustomData ("AddKeepAlivesStep", out addKeepAlivesStep) && bool.TryParse (addKeepAlivesStep, out var bv) && bv)
InsertAfter (new AddKeepAlivesStep (cache), "CleanStep");

string androidLinkResources;
if (Context.TryGetCustomData ("AndroidLinkResources", out androidLinkResources) && bool.TryParse (androidLinkResources, out var linkResources) && linkResources) {
InsertAfter (new RemoveResourceDesignerStep (), "CleanStep");
InsertAfter (new GetAssembliesStep (), "CleanStep");
}
InsertAfter (new StripEmbeddedLibraries (), "CleanStep");
}

void InsertAfter (IStep step, string stepName)
{
for (int i = 0; i < Steps.Count;) {
if (Steps [i++].GetType ().Name == stepName) {
Steps.Insert (i, step);
return;
}
}

throw new InvalidOperationException ($"Could not insert {step} after {stepName}.");
}
}
}
10 changes: 0 additions & 10 deletions src/Microsoft.Android.Sdk.ILLink/SubStepDispatcher.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,30 @@
using Mono.Linker.Steps;

using Mono.Cecil.Cil;
#if NET5_LINKER
using Microsoft.Android.Sdk.ILLink;
#endif // NET5_LINKER

namespace MonoDroid.Tuner
{
public class AddKeepAlivesStep : BaseStep
{
readonly TypeDefinitionCache cache;

public AddKeepAlivesStep (TypeDefinitionCache cache)
#if NET5_LINKER
protected override void Process ()
{
cache = Context;
}
#else // !NET5_LINKER
public AddKeepAlivesStep (IMetadataResolver cache)
{
this.cache = cache;
}

readonly
#endif // !NET5_LINKER
IMetadataResolver cache;

protected override void ProcessAssembly (AssemblyDefinition assembly)
{
var action = Annotations.HasAction (assembly) ? Annotations.GetAction (assembly) : AssemblyAction.Skip;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@

using Mono.Cecil;

#if NET5_LINKER
using Microsoft.Android.Sdk.ILLink;
#endif

namespace MonoDroid.Tuner {

public class ApplyPreserveAttribute : ApplyPreserveAttributeBase {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ public static TypeDefinition GetMarshalMethodsType (this TypeDefinition type)
return null;
}

public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition method, TypeDefinitionCache cache, out string member, out string nativeMethod, out string signature)
public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition method, IMetadataResolver resolver, out string member, out string nativeMethod, out string signature)
{
var type = method.DeclaringType;

Expand All @@ -264,45 +264,45 @@ public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition me
if (method.IsConstructor || type == null || !type.HasNestedTypes)
return false;

var m = method.GetBaseDefinition (cache);
var m = method.GetBaseDefinition (resolver);

while (m != null) {
if (m == method)
break;
break;

method = m;

if (m.TryGetRegisterMember (out member, out nativeMethod, out signature))
return true;

m = m.GetBaseDefinition (cache);
m = m.GetBaseDefinition (resolver);
}

if (!method.DeclaringType.HasInterfaces || !method.IsNewSlot)
return false;

foreach (var iface in method.DeclaringType.Interfaces) {
if (iface.InterfaceType.IsGenericInstance)
continue;
continue;

var itype = iface.InterfaceType.Resolve ();
if (itype == null || !itype.HasMethods)
continue;

foreach (var im in itype.Methods)
if (im.IsEqual (method, cache))
if (im.IsEqual (method, resolver))
return im.TryGetRegisterMember (out member, out nativeMethod, out signature);
}
}

return false;
return false;
}

public static bool IsEqual (this MethodDefinition m1, MethodDefinition m2, TypeDefinitionCache cache)
public static bool IsEqual (this MethodDefinition m1, MethodDefinition m2, IMetadataResolver resolver)
{
if (m1.Name != m2.Name || m1.ReturnType.Name != m2.ReturnType.Name)
return false;

return m1.Parameters.AreParametersCompatibleWith (m2.Parameters, cache);
return m1.Parameters.AreParametersCompatibleWith (m2.Parameters, resolver);
}

public static bool TryGetMarshalMethod (this MethodDefinition method, string nativeMethod, string signature, out MethodDefinition marshalMethod)
Expand Down
Loading

0 comments on commit 45a9adc

Please sign in to comment.