From 16ddadbd5b08f5bcb5cb3ab3c447b8802e610d74 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 31 Aug 2017 09:41:00 -0400 Subject: [PATCH] [Xamarin.Android.Build.Tasks] AOT+LLVM needs minSdkVersion (#795) Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=58029 Scenario: Build a project with: * `$(Configuration)`=Release * `$(AotAssemblies)`=True * `$(EnableLLVM)`=True * `$(TargetFrameworkVersion)`=v7.1 (API-25) * `//uses-sdk/@android:minSdkVersion`=10 (in `AndroidManifest.xml`) * with Android NDK r12b or later * on particular hardware devices, e.g. a Nexus 5. Actual results: the app runs, but the AOT'd images aren't used: AOT: image 'Xamarin.Android.Support.v7.AppCompat.dll.so' not found: dlopen failed: cannot locate symbol "__aeabi_memset" referenced by "/data/app/com.companyname.App1-1/lib/arm/libaot-Xamarin.Android.Support.v7.AppCompat.dll.so"... The `__aeabi_memset` symbol can't be found, preventing e.g. `Xamarin.Android.Support.v7.AppCompat.dll.so` from being used. Meaning the app pays the build overhead and size penalty of AOT+LLVM, but doesn't get anything out of it; only the JIT is used. The [cause of the missing `__aeabi_memset` symbol][0] is that we're using the NDK paths which corresponds with `$(TargetFrameworkVersion)`, *not* the NDK paths which correspond with `//uses-sdk/@android:minSdkVersion`. Because of this, if you use the `.apk` on a platform which is >= `minSdkVersion` but less than `$(TargetFrameworkVersion)`, the AOT images won't be used. [0]: https://github.com/android-ndk/ndk/issues/126 Fix this by updating the `` task to instead use the `//uses-sdk/@android:minSdkVersion` value. This ensures that we use NDK paths which correspond to the app's minimum supported API level, which should allow the AOT images to be loaded on downlevel devices. --- src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs | 20 +++++++++++++++++-- .../Xamarin.Android.Build.Tests/BuildTest.cs | 18 +++++++++++++++++ .../Xamarin.Android.Common.targets | 1 + 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs index 6c66bdd696d..261f9b506a5 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs @@ -44,6 +44,9 @@ public class Aot : AsyncTask [Required] public string SdkBinDirectory { get; set; } + [Required] + public ITaskItem ManifestFile { get; set; } + [Required] public ITaskItem[] ResolvedAssemblies { get; set; } @@ -168,9 +171,22 @@ static bool ValidateAotConfiguration (TaskLoggingHelper log, AndroidTargetArch a return true; } - static int GetNdkApiLevel(string androidNdkPath, string androidApiLevel, AndroidTargetArch arch) + int GetNdkApiLevel(string androidNdkPath, string androidApiLevel, AndroidTargetArch arch) { - int level = int.Parse(androidApiLevel); + var manifest = AndroidAppManifest.Load (ManifestFile.ItemSpec); + + int level; + if (manifest.MinSdkVersion.HasValue) { + level = manifest.MinSdkVersion.Value; + } + else if (int.TryParse (androidApiLevel, out level)) { + // level already set + } + else { + // Probably not ideal! + level = AndroidVersion.MaxApiLevel; + } + // Some Android API levels do not exist on the NDK level. Workaround this my mapping them to the // most appropriate API level that does exist. if (level == 6 || level == 7) level = 5; diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs index 79eeb7eac32..5fbc8f2f8c3 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs @@ -2,6 +2,7 @@ using System.IO; using System.Linq; using System.Text; +using System.Text.RegularExpressions; using System.Xml.Linq; using Microsoft.Build.Framework; using NUnit.Framework; @@ -209,12 +210,29 @@ public void BuildAotApplication (string supportedAbis, bool enableLLVM, bool exp proj.SetProperty (KnownProperties.TargetFrameworkVersion, "v5.1"); proj.SetProperty (KnownProperties.AndroidSupportedAbis, supportedAbis); proj.SetProperty ("EnableLLVM", enableLLVM.ToString ()); + if (enableLLVM) { + // Set //uses-sdk/@android:minSdkVersion so that LLVM uses the right libc.so + proj.AndroidManifest = $@" + + + + +"; + } using (var b = CreateApkBuilder (path)) { b.ThrowOnBuildFailure = false; b.Verbosity = LoggerVerbosity.Diagnostic; Assert.AreEqual (expectedResult, b.Build (proj), "Build should have {0}.", expectedResult ? "succeeded" : "failed"); if (!expectedResult) return; + if (enableLLVM) { + // LLVM passes a direct path to libc.so, and we need to use the libc.so + // which corresponds to the *minimum* SDK version specified in AndroidManifest.xml + // Since we overrode minSdkVersion=10, that means we should use libc.so from android-9. + var rightLibc = new Regex (@"^\s*\[AOT\].*cross-.*--llvm.*,ld-flags=.*android-9.arch-.*.usr.lib.libc\.so", RegexOptions.Multiline); + var m = rightLibc.Match (b.LastBuildOutput); + Assert.IsTrue (m.Success, "AOT+LLVM should use libc.so from minSdkVersion!"); + } foreach (var abi in supportedAbis.Split (new char [] { ';' })) { var libapp = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "bundles", abi, "libmonodroid_bundle_app.so"); diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index b2eeafd4e0e..36eabbf3d07 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -2145,6 +2145,7 @@ because xbuild doesn't support framework reference assemblies. AndroidNdkDirectory="$(_AndroidNdkDirectory)" AndroidApiLevel="$(_AndroidApiLevel)" SdkBinDirectory="$(MonoAndroidBinDirectory)" + ManifestFile="$(IntermediateOutputPath)android\AndroidManifest.xml" SupportedAbis="$(_BuildTargetAbis)" AndroidSequencePointsMode="$(_SequencePointsMode)" AotAdditionalArguments="$(AndroidAotAdditionalArguments)"