From 03b5185b888f2d75b61916e37c8f366e10e7fd94 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Wed, 26 Feb 2020 22:45:42 +0900 Subject: [PATCH] apex: choose stub according to min_sdk_version Native modules within APEX should be linked with proper stub version according to its min_sdk_version. For example, when min_sdk_version is set to "29", libfoo in the apex would be linked to libbar of version 29 from platform, even if it has a newer version like 30. Bug: 145796956 Test: m nothing (soong tests) Change-Id: I4a0b2002587bc24b7deeb5d59b6eeba5e1db5b1f --- android/apex.go | 29 +++++ apex/apex.go | 23 +++- apex/apex_test.go | 291 ++++++++++++++++++++++++++++++++++++++++++++++ cc/cc.go | 42 +++++-- cc/cc_test.go | 1 + cc/linkable.go | 1 + rust/rust.go | 4 + 7 files changed, 381 insertions(+), 10 deletions(-) diff --git a/android/apex.go b/android/apex.go index 026d685ae..a4de6dd7f 100644 --- a/android/apex.go +++ b/android/apex.go @@ -15,7 +15,9 @@ package android import ( + "fmt" "sort" + "strconv" "sync" ) @@ -25,6 +27,8 @@ type ApexInfo struct { // Whether this apex variant needs to target Android 10 LegacyAndroid10Support bool + + MinSdkVersion int } // ApexModule is the interface that a module type is expected to implement if @@ -86,6 +90,13 @@ type ApexModule interface { // DepIsInSameApex tests if the other module 'dep' is installed to the same // APEX as this module DepIsInSameApex(ctx BaseModuleContext, dep Module) bool + + // Returns the highest version which is <= min_sdk_version. + // For example, with min_sdk_version is 10 and versionList is [9,11] + // it returns 9. + ChooseSdkVersion(versionList []string, useLatest bool) (string, error) + + ShouldSupportAndroid10() bool } type ApexProperties struct { @@ -181,6 +192,24 @@ func (m *ApexModuleBase) DepIsInSameApex(ctx BaseModuleContext, dep Module) bool return true } +func (m *ApexModuleBase) ChooseSdkVersion(versionList []string, useLatest bool) (string, error) { + if useLatest { + return versionList[len(versionList)-1], nil + } + minSdkVersion := m.ApexProperties.Info.MinSdkVersion + for i := range versionList { + ver, _ := strconv.Atoi(versionList[len(versionList)-i-1]) + if ver <= minSdkVersion { + return versionList[len(versionList)-i-1], nil + } + } + return "", fmt.Errorf("min_sdk_version is set %v, but not found in %v", minSdkVersion, versionList) +} + +func (m *ApexModuleBase) ShouldSupportAndroid10() bool { + return !m.IsForPlatform() && (m.ApexProperties.Info.MinSdkVersion <= 29 || m.ApexProperties.Info.LegacyAndroid10Support) +} + func (m *ApexModuleBase) checkApexAvailableProperty(mctx BaseModuleContext) { for _, n := range m.ApexProperties.Apex_available { if n == AvailableToPlatform || n == availableToAnyApex { diff --git a/apex/apex.go b/apex/apex.go index bef4e4254..de18e5ccd 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -19,6 +19,7 @@ import ( "path" "path/filepath" "sort" + "strconv" "strings" "sync" @@ -1028,7 +1029,15 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) { var apexBundles []android.ApexInfo var directDep bool if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex { - apexBundles = []android.ApexInfo{{mctx.ModuleName(), proptools.Bool(a.properties.Legacy_android10_support)}} + minSdkVersion := a.minSdkVersion(mctx) + + apexBundles = []android.ApexInfo{ + android.ApexInfo{ + ApexName: mctx.ModuleName(), + LegacyAndroid10Support: proptools.Bool(a.properties.Legacy_android10_support), + MinSdkVersion: minSdkVersion, + }, + } directDep = true } else if am, ok := mctx.Module().(android.ApexModule); ok { apexBundles = am.ApexVariations() @@ -1967,6 +1976,18 @@ func (a *apexBundle) walkPayloadDeps(ctx android.ModuleContext, }) } +func (a *apexBundle) minSdkVersion(ctx android.BaseModuleContext) int { + ver := proptools.StringDefault(a.properties.Min_sdk_version, "current") + if ver != "current" { + minSdkVersion, err := strconv.Atoi(ver) + if err != nil { + ctx.PropertyErrorf("min_sdk_version", "should be \"current\" or , but %q", ver) + } + return minSdkVersion + } + return android.FutureApiLevel +} + // Ensures that the dependencies are marked as available for this APEX func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) { // Let's be practical. Availability for test, host, and the VNDK apex isn't important diff --git a/apex/apex_test.go b/apex/apex_test.go index 674446af2..2b1319741 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -984,6 +984,297 @@ func TestApexWithSystemLibsStubs(t *testing.T) { ensureContains(t, libFlags, "libdl/android_arm64_armv8-a_shared/libdl.so") } +func TestApexUseStubsAccordingToMinSdkVersionInUnbundledBuild(t *testing.T) { + // there are three links between liba --> libz + // 1) myapex -> libx -> liba -> libz : this should be #2 link, but fallback to #1 + // 2) otherapex -> liby -> liba -> libz : this should be #3 link + // 3) (platform) -> liba -> libz : this should be non-stub link + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libx"], + min_sdk_version: "2", + } + + apex { + name: "otherapex", + key: "myapex.key", + native_shared_libs: ["liby"], + min_sdk_version: "3", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "libx", + shared_libs: ["liba"], + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + } + + cc_library { + name: "liby", + shared_libs: ["liba"], + system_shared_libs: [], + stl: "none", + apex_available: [ "otherapex" ], + } + + cc_library { + name: "liba", + shared_libs: ["libz"], + system_shared_libs: [], + stl: "none", + apex_available: [ + "//apex_available:anyapex", + "//apex_available:platform", + ], + } + + cc_library { + name: "libz", + system_shared_libs: [], + stl: "none", + stubs: { + versions: ["1", "3"], + }, + } + `, withUnbundledBuild) + + expectLink := func(from, from_variant, to, to_variant string) { + ldArgs := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld").Args["libFlags"] + ensureContains(t, ldArgs, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + expectNoLink := func(from, from_variant, to, to_variant string) { + ldArgs := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld").Args["libFlags"] + ensureNotContains(t, ldArgs, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + // platform liba is linked to non-stub version + expectLink("liba", "shared", "libz", "shared") + // liba in myapex is linked to #1 + expectLink("liba", "shared_myapex", "libz", "shared_1") + expectNoLink("liba", "shared_myapex", "libz", "shared_3") + expectNoLink("liba", "shared_myapex", "libz", "shared") + // liba in otherapex is linked to #3 + expectLink("liba", "shared_otherapex", "libz", "shared_3") + expectNoLink("liba", "shared_otherapex", "libz", "shared_1") + expectNoLink("liba", "shared_otherapex", "libz", "shared") +} + +func TestApexMinSdkVersionDefaultsToLatest(t *testing.T) { + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libx"], + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "libx", + shared_libs: ["libz"], + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + } + + cc_library { + name: "libz", + system_shared_libs: [], + stl: "none", + stubs: { + versions: ["1", "2"], + }, + } + `) + + expectLink := func(from, from_variant, to, to_variant string) { + ldArgs := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld").Args["libFlags"] + ensureContains(t, ldArgs, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + expectNoLink := func(from, from_variant, to, to_variant string) { + ldArgs := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld").Args["libFlags"] + ensureNotContains(t, ldArgs, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + expectLink("libx", "shared_myapex", "libz", "shared_2") + expectNoLink("libx", "shared_myapex", "libz", "shared_1") + expectNoLink("libx", "shared_myapex", "libz", "shared") +} + +func TestPlatformUsesLatestStubsFromApexes(t *testing.T) { + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libx"], + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "libx", + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + stubs: { + versions: ["1", "2"], + }, + } + + cc_library { + name: "libz", + shared_libs: ["libx"], + system_shared_libs: [], + stl: "none", + } + `) + + expectLink := func(from, from_variant, to, to_variant string) { + ldArgs := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld").Args["libFlags"] + ensureContains(t, ldArgs, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + expectNoLink := func(from, from_variant, to, to_variant string) { + ldArgs := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld").Args["libFlags"] + ensureNotContains(t, ldArgs, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + expectLink("libz", "shared", "libx", "shared_2") + expectNoLink("libz", "shared", "libz", "shared_1") + expectNoLink("libz", "shared", "libz", "shared") +} + +func TestQApexesUseLatestStubsInBundledBuilds(t *testing.T) { + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libx"], + min_sdk_version: "29", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "libx", + shared_libs: ["libbar"], + apex_available: [ "myapex" ], + } + + cc_library { + name: "libbar", + stubs: { + versions: ["29", "30"], + }, + } + `) + expectLink := func(from, from_variant, to, to_variant string) { + ld := ctx.ModuleForTests(from, "android_arm64_armv8-a_"+from_variant).Rule("ld") + libFlags := ld.Args["libFlags"] + ensureContains(t, libFlags, "android_arm64_armv8-a_"+to_variant+"/"+to+".so") + } + expectLink("libx", "shared_myapex", "libbar", "shared_30") +} + +func TestQTargetApexUseStaticUnwinder(t *testing.T) { + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libx"], + min_sdk_version: "29", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "libx", + apex_available: [ "myapex" ], + } + + `, withUnbundledBuild) + + // ensure apex variant of c++ is linked with static unwinder + cm := ctx.ModuleForTests("libc++", "android_arm64_armv8-a_shared_myapex").Module().(*cc.Module) + ensureListContains(t, cm.Properties.AndroidMkStaticLibs, "libgcc_stripped") + // note that platform variant is not. + cm = ctx.ModuleForTests("libc++", "android_arm64_armv8-a_shared").Module().(*cc.Module) + ensureListNotContains(t, cm.Properties.AndroidMkStaticLibs, "libgcc_stripped") + + libFlags := ctx.ModuleForTests("libx", "android_arm64_armv8-a_shared_myapex").Rule("ld").Args["libFlags"] + ensureContains(t, libFlags, "android_arm64_armv8-a_shared_myapex/libc++.so") + ensureContains(t, libFlags, "android_arm64_armv8-a_shared_29/libc.so") // min_sdk_version applied +} + +func TestInvalidMinSdkVersion(t *testing.T) { + testApexError(t, `"libz" .*: min_sdk_version is set 29.*`, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["libx"], + min_sdk_version: "29", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "libx", + shared_libs: ["libz"], + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + } + + cc_library { + name: "libz", + system_shared_libs: [], + stl: "none", + stubs: { + versions: ["30"], + }, + } + `, withUnbundledBuild) + + testApexError(t, `"myapex" .*: min_sdk_version: should be .*`, ` + apex { + name: "myapex", + key: "myapex.key", + min_sdk_version: "R", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + `) +} + func TestFilesInSubDir(t *testing.T) { ctx, _ := testApex(t, ` apex { diff --git a/cc/cc.go b/cc/cc.go index e5e724e33..8b3c77202 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -631,6 +631,15 @@ func (c *Module) SetStubsVersions(version string) { panic(fmt.Errorf("SetStubsVersions called on non-library module: %q", c.BaseModuleName())) } +func (c *Module) StubsVersion() string { + if c.linker != nil { + if library, ok := c.linker.(*libraryDecorator); ok { + return library.MutatedProperties.StubsVersion + } + } + panic(fmt.Errorf("StubsVersion called on non-library module: %q", c.BaseModuleName())) +} + func (c *Module) SetStatic() { if c.linker != nil { if library, ok := c.linker.(libraryInterface); ok { @@ -1821,16 +1830,17 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { } actx.AddVariationDependencies(variations, depTag, name) - // If the version is not specified, add dependency to the latest stubs library. + // If the version is not specified, add dependency to all stubs libraries. // The stubs library will be used when the depending module is built for APEX and // the dependent module is not in the same APEX. - latestVersion := LatestStubsVersionFor(actx.Config(), name) - if version == "" && latestVersion != "" && versionVariantAvail { - actx.AddVariationDependencies([]blueprint.Variation{ - {Mutator: "link", Variation: "shared"}, - {Mutator: "version", Variation: latestVersion}, - }, depTag, name) - // Note that depTag.ExplicitlyVersioned is false in this case. + if version == "" && versionVariantAvail { + for _, ver := range stubsVersionsFor(actx.Config())[name] { + // Note that depTag.ExplicitlyVersioned is false in this case. + actx.AddVariationDependencies([]blueprint.Variation{ + {Mutator: "link", Variation: "shared"}, + {Mutator: "version", Variation: ver}, + }, depTag, name) + } } } @@ -2178,7 +2188,8 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } if depTag == staticUnwinderDepTag { - if c.ApexProperties.Info.LegacyAndroid10Support { + // Use static unwinder for legacy (min_sdk_version = 29) apexes (b/144430859) + if c.ShouldSupportAndroid10() { depTag = StaticDepTag } else { return @@ -2230,6 +2241,19 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { useThisDep = (depInSameApex != depIsStubs) } + // when to use (unspecified) stubs, check min_sdk_version and choose the right one + if useThisDep && depIsStubs && !explicitlyVersioned { + useLatest := c.IsForPlatform() || (c.ShouldSupportAndroid10() && !ctx.Config().UnbundledBuild()) + versionToUse, err := c.ChooseSdkVersion(ccDep.StubsVersions(), useLatest) + if err != nil { + ctx.OtherModuleErrorf(dep, err.Error()) + return + } + if versionToUse != ccDep.StubsVersion() { + useThisDep = false + } + } + if !useThisDep { return // stop processing this dep } diff --git a/cc/cc_test.go b/cc/cc_test.go index 8acc8175d..56b36df57 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -2530,6 +2530,7 @@ func TestRuntimeLibsNoVndk(t *testing.T) { } func checkStaticLibs(t *testing.T, expected []string, module *Module) { + t.Helper() actual := module.Properties.AndroidMkStaticLibs if !reflect.DeepEqual(actual, expected) { t.Errorf("incorrect static_libs"+ diff --git a/cc/linkable.go b/cc/linkable.go index e4f034c7f..80cd6b8a2 100644 --- a/cc/linkable.go +++ b/cc/linkable.go @@ -26,6 +26,7 @@ type LinkableInterface interface { BuildStubs() bool SetBuildStubs() SetStubsVersions(string) + StubsVersion() string HasStubsVariants() bool SelectedStl() string ApiLevel() string diff --git a/rust/rust.go b/rust/rust.go index de6512cf2..f446ef039 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -325,6 +325,10 @@ func (mod *Module) SetStubsVersions(string) { panic("SetStubsVersions not yet implemented for rust modules") } +func (mod *Module) StubsVersion() string { + panic("SetStubsVersions not yet implemented for rust modules") +} + func (mod *Module) BuildStaticVariant() bool { if mod.compiler != nil { if library, ok := mod.compiler.(libraryInterface); ok {