From c5a967630ef38406da5143497136e21c356324d5 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Fri, 4 Feb 2022 11:54:50 +0900 Subject: [PATCH 1/2] use_vndk_as_stable APEX shouldn't include VNDK lib Even though a vendor APEX sets use_vndk_as_stable:true it was possible to include a VNDK lib by directly depending on it with native_shared_libs. But it's contradictory to have a VNDK lib while declaring not to include VNDK libs. It was missing since pruning dependencies on VNDK libs was done only for transitive deps. Added a check to reject this. Bug: 216847402 Test: m nothing(running soong tests) Change-Id: I8d79a434b1bfe8e563cf8968fa76830b0e582f66 --- apex/apex.go | 15 ++++++++++++--- apex/apex_test.go | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 0ac6eaa89..d12a7865b 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -888,9 +888,18 @@ func (a *apexBundle) ApexInfoMutator(mctx android.TopDownMutatorContext) { // APEX, but shared across APEXes via the VNDK APEX. useVndk := a.SocSpecific() || a.DeviceSpecific() || (a.ProductSpecific() && mctx.Config().EnforceProductPartitionInterface()) excludeVndkLibs := useVndk && proptools.Bool(a.properties.Use_vndk_as_stable) - if !useVndk && proptools.Bool(a.properties.Use_vndk_as_stable) { - mctx.PropertyErrorf("use_vndk_as_stable", "not supported for system/system_ext APEXes") - return + if proptools.Bool(a.properties.Use_vndk_as_stable) { + if !useVndk { + mctx.PropertyErrorf("use_vndk_as_stable", "not supported for system/system_ext APEXes") + } + mctx.VisitDirectDepsWithTag(sharedLibTag, func(dep android.Module) { + if c, ok := dep.(*cc.Module); ok && c.IsVndk() { + mctx.PropertyErrorf("use_vndk_as_stable", "Trying to include a VNDK library(%s) while use_vndk_as_stable is true.", dep.Name()) + } + }) + if mctx.Failed() { + return + } } continueApexDepsWalk := func(child, parent android.Module) bool { diff --git a/apex/apex_test.go b/apex/apex_test.go index c546fa13c..067501ecb 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -2713,6 +2713,23 @@ func TestVendorApex(t *testing.T) { ensureListNotContains(t, requireNativeLibs, ":vndk") } +func TestVendorApex_use_vndk_as_stable_TryingToIncludeVNDKLib(t *testing.T) { + testApexError(t, `Trying to include a VNDK library`, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libc++"], // libc++ is a VNDK lib + vendor: true, + use_vndk_as_stable: true, + updatable: false, + } + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + }`) +} + func TestVendorApex_use_vndk_as_stable(t *testing.T) { ctx := testApex(t, ` apex { From 91f9203af42d029de2e025bcedc1044a4a1e5dcc Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Fri, 4 Feb 2022 12:36:33 +0900 Subject: [PATCH 2/2] VNDK libs use "unique" APEX variants In case there are two vendor apexes(one with "use_vndk_as_stable:true", and the other with "use_vndk_as_stable:false") a VNDK lib used by both will have "APEX" variant and the former APEX will use "apex" variation. For example, apex1(use_vndk_as_stable) -> foo -> libvndk apex2 -> bar -> libvndk Since foo, bar and libvndk are mutated into two APEX variations("", "apex10000"), foo will use the apex variation of libvndk. To fix this, VNDK libs can use "unique" APEX variations. Then, in the above example, foo will have "myapex1" variation and libvndk will have two APEX variations("" and "apex2"). So foo will link to ""(non-APEX) variation as fallback. Bug: 216847402 Test: m nothing (soong tests) Change-Id: I116932860ef79e22dc338a58b251e3ca693ab4f3 --- apex/apex_test.go | 110 +++++++++++++++++++++++++++++++++++++++------- cc/cc.go | 8 ++++ 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/apex/apex_test.go b/apex/apex_test.go index 067501ecb..6d77b0623 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -2731,6 +2731,15 @@ func TestVendorApex_use_vndk_as_stable_TryingToIncludeVNDKLib(t *testing.T) { } func TestVendorApex_use_vndk_as_stable(t *testing.T) { + // myapex myapex2 + // | | + // mybin ------. mybin2 + // \ \ / | + // (stable) .---\--------` | + // \ / \ | + // \ / \ / + // libvndk libvendor + // (vndk) ctx := testApex(t, ` apex { name: "myapex", @@ -2761,28 +2770,95 @@ func TestVendorApex_use_vndk_as_stable(t *testing.T) { cc_library { name: "libvendor", vendor: true, + stl: "none", + } + apex { + name: "myapex2", + key: "myapex.key", + binaries: ["mybin2"], + vendor: true, + use_vndk_as_stable: false, + updatable: false, + } + cc_binary { + name: "mybin2", + vendor: true, + shared_libs: ["libvndk", "libvendor"], } `) vendorVariant := "android_vendor.29_arm64_armv8-a" - ldRule := ctx.ModuleForTests("mybin", vendorVariant+"_apex10000").Rule("ld") - libs := names(ldRule.Args["libFlags"]) - // VNDK libs(libvndk/libc++) as they are - ensureListContains(t, libs, "out/soong/.intermediates/libvndk/"+vendorVariant+"_shared/libvndk.so") - ensureListContains(t, libs, "out/soong/.intermediates/"+cc.DefaultCcCommonTestModulesDir+"libc++/"+vendorVariant+"_shared/libc++.so") - // non-stable Vendor libs as APEX variants - ensureListContains(t, libs, "out/soong/.intermediates/libvendor/"+vendorVariant+"_shared_apex10000/libvendor.so") + for _, tc := range []struct { + name string + apexName string + moduleName string + moduleVariant string + libs []string + contents []string + requireVndkNamespace bool + }{ + { + name: "use_vndk_as_stable", + apexName: "myapex", + moduleName: "mybin", + moduleVariant: vendorVariant + "_apex10000", + libs: []string{ + // should link with vendor variants of VNDK libs(libvndk/libc++) + "out/soong/.intermediates/libvndk/" + vendorVariant + "_shared/libvndk.so", + "out/soong/.intermediates/" + cc.DefaultCcCommonTestModulesDir + "libc++/" + vendorVariant + "_shared/libc++.so", + // unstable Vendor libs as APEX variant + "out/soong/.intermediates/libvendor/" + vendorVariant + "_shared_apex10000/libvendor.so", + }, + contents: []string{ + "bin/mybin", + "lib64/libvendor.so", + // VNDK libs (libvndk/libc++) are not included + }, + requireVndkNamespace: true, + }, + { + name: "!use_vndk_as_stable", + apexName: "myapex2", + moduleName: "mybin2", + moduleVariant: vendorVariant + "_myapex2", + libs: []string{ + // should link with "unique" APEX(myapex2) variant of VNDK libs(libvndk/libc++) + "out/soong/.intermediates/libvndk/" + vendorVariant + "_shared_myapex2/libvndk.so", + "out/soong/.intermediates/" + cc.DefaultCcCommonTestModulesDir + "libc++/" + vendorVariant + "_shared_myapex2/libc++.so", + // unstable vendor libs have "merged" APEX variants + "out/soong/.intermediates/libvendor/" + vendorVariant + "_shared_apex10000/libvendor.so", + }, + contents: []string{ + "bin/mybin2", + "lib64/libvendor.so", + // VNDK libs are included as well + "lib64/libvndk.so", + "lib64/libc++.so", + }, + requireVndkNamespace: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Check linked libs + ldRule := ctx.ModuleForTests(tc.moduleName, tc.moduleVariant).Rule("ld") + libs := names(ldRule.Args["libFlags"]) + for _, lib := range tc.libs { + ensureListContains(t, libs, lib) + } + // Check apex contents + ensureExactContents(t, ctx, tc.apexName, "android_common_"+tc.apexName+"_image", tc.contents) - // VNDK libs are not included when use_vndk_as_stable: true - ensureExactContents(t, ctx, "myapex", "android_common_myapex_image", []string{ - "bin/mybin", - "lib64/libvendor.so", - }) - - apexManifestRule := ctx.ModuleForTests("myapex", "android_common_myapex_image").Rule("apexManifestRule") - requireNativeLibs := names(apexManifestRule.Args["requireNativeLibs"]) - ensureListContains(t, requireNativeLibs, ":vndk") + // Check "requireNativeLibs" + apexManifestRule := ctx.ModuleForTests(tc.apexName, "android_common_"+tc.apexName+"_image").Rule("apexManifestRule") + requireNativeLibs := names(apexManifestRule.Args["requireNativeLibs"]) + if tc.requireVndkNamespace { + ensureListContains(t, requireNativeLibs, ":vndk") + } else { + ensureListNotContains(t, requireNativeLibs, ":vndk") + } + }) + } } func TestProductVariant(t *testing.T) { @@ -2813,7 +2889,7 @@ func TestProductVariant(t *testing.T) { ) cflags := strings.Fields( - ctx.ModuleForTests("foo", "android_product.29_arm64_armv8-a_apex10000").Rule("cc").Args["cFlags"]) + ctx.ModuleForTests("foo", "android_product.29_arm64_armv8-a_myapex").Rule("cc").Args["cFlags"]) ensureListContains(t, cflags, "-D__ANDROID_VNDK__") ensureListContains(t, cflags, "-D__ANDROID_APEX__") ensureListContains(t, cflags, "-D__ANDROID_PRODUCT__") diff --git a/cc/cc.go b/cc/cc.go index 31babc2da..4a0e7d49a 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -3464,6 +3464,14 @@ func (c *Module) AlwaysRequiresPlatformApexVariant() bool { return c.IsStubs() || c.Target().NativeBridge == android.NativeBridgeEnabled } +// Overrides android.ApexModuleBase.UniqueApexVariations +func (c *Module) UniqueApexVariations() bool { + // When a vendor APEX needs a VNDK lib in it (use_vndk_as_stable: false), it should be a unique + // APEX variation. Otherwise, another vendor APEX with use_vndk_as_stable:true may use a wrong + // variation of the VNDK lib because APEX variations are merged/grouped. + return c.UseVndk() && c.IsVndk() +} + var _ snapshot.RelativeInstallPath = (*Module)(nil) // ConvertWithBp2build converts Module to Bazel for bp2build.