From bbc3fb780b859ccd2c4550a8bc6b87f36d64dfd9 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Wed, 29 Apr 2020 14:01:06 +0900 Subject: [PATCH] Reland "enforce sdk_version for JNI libs for updatable APKs" JNI libs for "updatable" APKs or APKs in "updatable" APEXes should set sdk_version which is equal to or less than APK's min_sdk_version. In fact, we'd better check if min_sdk_version of JNI libs matches(or is earlier than) min_sdk_version of the APK. But for now the build system can't handle sdk_version/min_sdk_version correctly for JNI libs. That's why sdk_version of JNI libs is enforced to match with min_sdk_version of APK in this change. (original commit: 98c4750f39577abd42e70dfd777ea6f5e1af3111) Bug: 145796956 Test: m Change-Id: I08543ccee7dfda0559a1fca108ceb5c28f84943f --- java/app.go | 30 +++++++++++- java/app_test.go | 121 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) diff --git a/java/app.go b/java/app.go index f1af2adf4..e08a1aeb9 100755 --- a/java/app.go +++ b/java/app.go @@ -286,19 +286,47 @@ func (a *AndroidApp) GenerateAndroidBuildActions(ctx android.ModuleContext) { } func (a *AndroidApp) checkAppSdkVersions(ctx android.ModuleContext) { - if Bool(a.appProperties.Updatable) { + if Bool(a.appProperties.Updatable) || a.ApexModuleBase.Updatable() { if !a.sdkVersion().stable() { ctx.PropertyErrorf("sdk_version", "Updatable apps must use stable SDKs, found %v", a.sdkVersion()) } if String(a.deviceProperties.Min_sdk_version) == "" { ctx.PropertyErrorf("updatable", "updatable apps must set min_sdk_version.") } + if minSdkVersion, err := a.minSdkVersion().effectiveVersion(ctx); err == nil { + a.checkJniLibsSdkVersion(ctx, minSdkVersion) + } else { + ctx.PropertyErrorf("min_sdk_version", "%s", err.Error()) + } } a.checkPlatformAPI(ctx) a.checkSdkVersions(ctx) } +// If an updatable APK sets min_sdk_version, min_sdk_vesion of JNI libs should match with it. +// This check is enforced for "updatable" APKs (including APK-in-APEX). +// b/155209650: until min_sdk_version is properly supported, use sdk_version instead. +// because, sdk_version is overridden by min_sdk_version (if set as smaller) +// and linkType is checked with dependencies so we can be sure that the whole dependency tree +// will meet the requirements. +func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVersion sdkVersion) { + // It's enough to check direct JNI deps' sdk_version because all transitive deps from JNI deps are checked in cc.checkLinkType() + ctx.VisitDirectDeps(func(m android.Module) { + if !IsJniDepTag(ctx.OtherModuleDependencyTag(m)) { + return + } + dep, _ := m.(*cc.Module) + jniSdkVersion, err := android.ApiStrToNum(ctx, dep.SdkVersion()) + if err != nil || int(minSdkVersion) < jniSdkVersion { + ctx.OtherModuleErrorf(dep, "sdk_version(%v) is higher than min_sdk_version(%v) of the containing android_app(%v)", + dep.SdkVersion(), minSdkVersion, ctx.ModuleName()) + return + } + + }) +} + // Returns true if the native libraries should be stored in the APK uncompressed and the // extractNativeLibs application flag should be set to false in the manifest. func (a *AndroidApp) useEmbeddedNativeLibs(ctx android.ModuleContext) bool { diff --git a/java/app_test.go b/java/app_test.go index 4bcfa5a81..4249abd77 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -385,6 +385,127 @@ func TestUpdatableApps(t *testing.T) { } } +func TestUpdatableApps_JniLibsShouldShouldSupportMinSdkVersion(t *testing.T) { + testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` + android_app { + name: "foo", + srcs: ["a.java"], + updatable: true, + sdk_version: "current", + min_sdk_version: "current", + jni_libs: ["libjni"], + } + + cc_library { + name: "libjni", + stl: "none", + system_shared_libs: [], + sdk_version: "current", + } + `) +} + +func TestUpdatableApps_JniLibShouldBeBuiltAgainstMinSdkVersion(t *testing.T) { + bp := cc.GatherRequiredDepsForTest(android.Android) + ` + android_app { + name: "foo", + srcs: ["a.java"], + updatable: true, + sdk_version: "current", + min_sdk_version: "29", + jni_libs: ["libjni"], + } + + cc_library { + name: "libjni", + stl: "none", + system_shared_libs: [], + sdk_version: "29", + } + + ndk_prebuilt_object { + name: "ndk_crtbegin_so.29", + sdk_version: "29", + } + + ndk_prebuilt_object { + name: "ndk_crtend_so.29", + sdk_version: "29", + } + ` + fs := map[string][]byte{ + "prebuilts/ndk/current/platforms/android-29/arch-arm64/usr/lib/crtbegin_so.o": nil, + "prebuilts/ndk/current/platforms/android-29/arch-arm64/usr/lib/crtend_so.o": nil, + "prebuilts/ndk/current/platforms/android-29/arch-arm/usr/lib/crtbegin_so.o": nil, + "prebuilts/ndk/current/platforms/android-29/arch-arm/usr/lib/crtend_so.o": nil, + } + + ctx, _ := testJavaWithConfig(t, testConfig(nil, bp, fs)) + + inputs := ctx.ModuleForTests("libjni", "android_arm64_armv8-a_sdk_shared").Description("link").Implicits + var crtbeginFound, crtendFound bool + for _, input := range inputs { + switch input.String() { + case "prebuilts/ndk/current/platforms/android-29/arch-arm64/usr/lib/crtbegin_so.o": + crtbeginFound = true + case "prebuilts/ndk/current/platforms/android-29/arch-arm64/usr/lib/crtend_so.o": + crtendFound = true + } + } + if !crtbeginFound || !crtendFound { + t.Error("should link with ndk_crtbegin_so.29 and ndk_crtend_so.29") + } +} + +func TestUpdatableApps_ErrorIfJniLibDoesntSupportMinSdkVersion(t *testing.T) { + bp := cc.GatherRequiredDepsForTest(android.Android) + ` + android_app { + name: "foo", + srcs: ["a.java"], + updatable: true, + sdk_version: "current", + min_sdk_version: "29", // this APK should support 29 + jni_libs: ["libjni"], + } + + cc_library { + name: "libjni", + stl: "none", + sdk_version: "current", + } + ` + testJavaError(t, `"libjni" .*: sdk_version\(current\) is higher than min_sdk_version\(29\)`, bp) +} + +func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) { + bp := cc.GatherRequiredDepsForTest(android.Android) + ` + android_app { + name: "foo", + srcs: ["a.java"], + updatable: true, + sdk_version: "current", + min_sdk_version: "29", // this APK should support 29 + jni_libs: ["libjni"], + } + + cc_library { + name: "libjni", + stl: "none", + shared_libs: ["libbar"], + system_shared_libs: [], + sdk_version: "27", + } + + cc_library { + name: "libbar", + stl: "none", + system_shared_libs: [], + sdk_version: "current", + } + ` + testJavaError(t, `"libjni" .*: links "libbar" built against newer API version "current"`, bp) +} + func TestResourceDirs(t *testing.T) { testCases := []struct { name string