Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] fix Hybrid AOT
Browse files Browse the repository at this point in the history
Fixes: dotnet#4818

Use of Hybrid AOT currently results in assemblies that have not been
CIL-stripped.

In 77ab240, a change was made to fix how `Mono.Android.dll` was
treated during AOT. `Mono.Android.dll` used to be the only assembly
copied to the `shrunk` directory, and we had two copies of the this
assembly passed to the AOT compiler. This change unintentionally made
`<CilStrip/>` strip the wrong set of assemblies...

The initial fix would be:

    <CilStrip
        ...
    -   ResolvedAssemblies="@(_ResolvedAssemblies)">
    +   ResolvedAssemblies="@(_ShrunkAssemblies)">

Unfortunately, this causes a crash:

    06-11 16:19:08.557 E/AndroidRuntime(18806): java.lang.UnsatisfiedLinkError: No implementation found for void mono.android.TypeManager.n_activate(java.lang.String, java.lang.String, java.lang.Object, java.lang.Object[]) (tried Java_mono_android_TypeManager_n_1activate and Java_mono_android_TypeManager_n_1activate__Ljava_lang_String_2Ljava_lang_String_2Ljava_lang_Object_2_3Ljava_lang_Object_2)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at mono.android.TypeManager.n_activate(Native Method)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at mono.android.TypeManager.Activate(:7)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at crc64446c24ccf511bf5f.SplashScreenActivity.<init>(:25)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at java.lang.Class.newInstance(Native Method)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.app.Instrumentation.newActivity(Instrumentation.java:1086)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2809)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2988)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.app.ActivityThread.-wrap14(ActivityThread.java)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1631)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.os.Handler.dispatchMessage(Handler.java:102)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.os.Looper.loop(Looper.java:154)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at android.app.ActivityThread.main(ActivityThread.java:6682)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at java.lang.reflect.Method.invoke(Native Method)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1520)
    06-11 16:19:08.557 E/AndroidRuntime(18806): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1410)

Thinking about how things "used to work", we weren't stripping the
*correct* `Mono.Android.dll`. So we could run `<CilStrip/>` on every
assembly *besides* `Mono.Android.dll`?

I think this could be improved further if we could strip
`Mono.Android.dll`, but this at least gets things back to working the
way they used to.

I updated the `BuildIncrementalAot` test that had a `//TODO` comment,
which works properly now. I also added a new test to check that method
bodies are stripped.
  • Loading branch information
jonathanpeppers committed Jun 22, 2020
1 parent 8859a2e commit 1f8bbae
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4331,5 +4331,37 @@ public void XA4310 ([Values ("apk", "aab")] string packageFormat)

}
}

[Test]
public void HybridAOT ()
{
var proj = new XamarinAndroidApplicationProject () {
IsRelease = true,
AotAssemblies = true,
};
proj.SetProperty ("AndroidAotMode", "Hybrid");
// So we can use Mono.Cecil to open assemblies directly
proj.SetProperty ("AndroidEnableAssemblyCompression", "False");

using (var b = CreateApkBuilder ()) {
b.Build (proj);

var apk = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath, $"{proj.PackageName}.apk");
FileAssert.Exists (apk);
using (var zip = ZipHelper.OpenZip (apk)) {
var entry = zip.ReadEntry ($"assemblies/{proj.ProjectName}.dll");
Assert.IsNotNull (entry, $"{proj.ProjectName}.dll should exist in apk!");
using (var stream = new MemoryStream ()) {
entry.Extract (stream);
stream.Position = 0;
using (var assembly = AssemblyDefinition.ReadAssembly (stream)) {
var type = assembly.MainModule.GetType ($"{proj.ProjectName}.MainActivity");
var method = type.Methods.First (m => m.Name == "OnCreate");
Assert.LessOrEqual (method.Body.Instructions.Count, 1, "OnCreate should have stripped method bodies!");
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -929,11 +929,6 @@ public void BuildIncrementalAot (string supportedAbis, string androidAotMode, bo
Assert.IsFalse (b.Output.IsTargetSkipped (target), $"`{target}` should *not* be skipped on first build!");
}

if (androidAotMode == "Hybrid") {
// FIXME: with Hybrid AOT, <CilStrip/> modifies assemblies in-place
Assert.Ignore ("Ignoring, Hybrid AOT triggers _BuildApkEmbed.");
}

b.BuildLogFile = "second.log";
b.CleanupAfterSuccessfulBuild = false;
b.CleanupOnDispose = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2151,13 +2151,17 @@ because xbuild doesn't support framework reference assemblies.
<Output TaskParameter="NativeLibrariesReferences" ItemName="_AdditionalNativeLibraryReferences" />
</Aot>

<ItemGroup Condition=" '$(AndroidAotMode)' == 'Hybrid' And '$(AotAssemblies)' == 'True' ">
<_CilStripAssemblies Include="@(_ShrunkAssemblies)" Condition=" '%(FileName)' != 'Mono.Android' " />
</ItemGroup>

<!-- Strip the IL code of the resolved managed assemblies -->
<CilStrip
Condition=" '$(AndroidAotMode)' == 'Hybrid' And '$(AotAssemblies)' == 'True' "
AndroidAotMode="$(AndroidAotMode)"
ToolPath="$(_MonoAndroidToolsDirectory)"
ApkOutputPath="$(_BuildApkEmbedOutputs)"
ResolvedAssemblies="@(_ResolvedAssemblies)">
ResolvedAssemblies="@(_CilStripAssemblies)">
</CilStrip>

<!-- Bundle the assemblies into native libraries in the apk -->
Expand Down

0 comments on commit 1f8bbae

Please sign in to comment.