From a008fb08cf5e50baef8ee612feba5104de536393 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Tue, 16 Mar 2021 17:15:53 +0900 Subject: [PATCH 1/2] Always respect min_sdk_version Background: When compiling cc_* modules, min_sdk_version determines the "version" part of the clang triple: -target -linux-android. The version part is used to make sure that calls to the APIs that are added after the version are guarded with a runtime check (i.e. __builtin_available). Previously, min_sdk_version was used as the version part only when the cc_* module is unbundled (e.g. built for an APEX or built with SDK). In other words, min_sdk_version has been ignored for the platform variants. They were built with the version number 10000. This was problematic for Mainline module tests. Since they are neither part of an APEX nor built with SDK (because they need to have access to some of the module-only APIs), they are built with the version number 10000. As a side effect, __builtin_available macro are expanded to 1 at build time - because 10000 is higher than any API versions. When such a test built in the latest platform source tree runs on a device running an old platform, it tries to call an API that might not be available on the platform and experience a crash, due to the lack of the runtime check. This change fixes the problem by using min_sdk_version as the "version" part of the clang triple, regardless of the module's variant. Then __builtin_available macro in the tests doesn't expand to 1, but to a function call to the libclang_rt.builtin library which checks the system property ro.build.version.sdk. Bug: N/A Test: run resolv_stress_test Change-Id: I88f64c5a35f1b5276c3350e177b116932011a940 --- cc/cc_test.go | 12 ++++++++++++ cc/compiler.go | 7 +------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/cc/cc_test.go b/cc/cc_test.go index 719661598..781778714 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -3628,6 +3628,18 @@ func TestAidlFlagsPassedToTheAidlCompiler(t *testing.T) { } } +func TestMinSdkVersionInClangTriple(t *testing.T) { + ctx := testCc(t, ` + cc_library_shared { + name: "libfoo", + srcs: ["foo.c"], + min_sdk_version: "29", + }`) + + cFlags := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_shared").Rule("cc").Args["cFlags"] + android.AssertStringDoesContain(t, "min sdk version", cFlags, "-target aarch64-linux-android29") +} + type MemtagNoteType int const ( diff --git a/cc/compiler.go b/cc/compiler.go index bcad1ad2e..2729e61c5 100644 --- a/cc/compiler.go +++ b/cc/compiler.go @@ -411,12 +411,7 @@ func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags, deps target := "-target " + tc.ClangTriple() if ctx.Os().Class == android.Device { - // When built for the non-updateble part of platform, minSdkVersion doesn't matter. - // It matters only when building we are building for modules that can be unbundled. - version := "current" - if !ctx.isForPlatform() || ctx.isSdkVariant() { - version = ctx.minSdkVersion() - } + version := ctx.minSdkVersion() if version == "" || version == "current" { target += strconv.Itoa(android.FutureApiLevelInt) } else { From fdaa5f71648b8e3c21a867db0a6751f5e1a2069b Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 19 Mar 2021 22:18:04 +0900 Subject: [PATCH 2/2] Versioned CRT objects are built with correct __ANDROID_API__ Background: `min_sdk_version` of a crt object is the oldest SDK version that the crt object supports. When it's set to for example 16, Soong creates a number of versioned variants of the crt object starting from 16 to the latest version. The variant for version X is provided to NDK clients having `min_sdk_version` set to X. Problem: all versioned variants of a crt object were built with `-target -linux-android16`. Therefore they all have been with `#define __ANDROID_API__ 16`. This is because the mutated variants still have the same min_sdk_version property and the clang triple follows min_sdk_version, not sdk_version. This is too conservative and against our intention to provide the latest crt object that matches with the min_sdk_version of the client. In the other hand, the platform(non-sdk) variant of the crt object doesn't have such a problem. min_sdk_version is completely ignored. However, this is a bug and will be fixed by aosp/1640364. As a side effect of the fixing, the platform variant will begin to built with a very old __ANDROID_API__ which unnecessarily turns off the new platform features like the TLS segment over-alignment. This change fixes the problems: * For the versioned variants of crt objects, we set both `min_sdk_version` and `sdk_versio` to the version that the variant is created for. * For the platform variant of crt objects, `min_sdk_version` is force reset to "current". Bug: 183191008 Test: m Change-Id: I8c9d0fcea816de8cd1532dac4a47eee4f726c037 --- cc/cc.go | 20 ++++++++++++++++++- cc/cc_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ cc/library.go | 1 + cc/linkable.go | 1 + rust/rust.go | 4 ++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/cc/cc.go b/cc/cc.go index cab7459ed..cb845a42f 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1363,6 +1363,19 @@ func (ctx *moduleContextImpl) minSdkVersion() string { if ver == "apex_inherit" || ver == "" { ver = ctx.sdkVersion() } + // For crt objects, the meaning of min_sdk_version is very different from other types of + // module. For them, min_sdk_version defines the oldest version that the build system will + // create versioned variants for. For example, if min_sdk_version is 16, then sdk variant of + // the crt object has local variants of 16, 17, ..., up to the latest version. sdk_version + // and min_sdk_version properties of the variants are set to the corresponding version + // numbers. However, the platform (non-sdk) variant of the crt object is left untouched. + // min_sdk_version: 16 doesn't actually mean that the platform variant has to support such + // an old version. Since the variant is for the platform, it's preferred to target the + // latest version. + if ctx.mod.SplitPerApiLevel() && !ctx.isSdkVariant() { + ver = strconv.Itoa(android.FutureApiLevelInt) + } + // Also make sure that minSdkVersion is not greater than sdkVersion, if they are both numbers sdkVersionInt, err := strconv.Atoi(ctx.sdkVersion()) minSdkVersionInt, err2 := strconv.Atoi(ver) @@ -1933,9 +1946,14 @@ func GetCrtVariations(ctx android.BottomUpMutatorContext, return nil } if m.UseSdk() { + // Choose the CRT that best satisfies the min_sdk_version requirement of this module + minSdkVersion := m.MinSdkVersion() + if minSdkVersion == "" || minSdkVersion == "apex_inherit" { + minSdkVersion = m.SdkVersion() + } return []blueprint.Variation{ {Mutator: "sdk", Variation: "sdk"}, - {Mutator: "version", Variation: m.SdkVersion()}, + {Mutator: "version", Variation: minSdkVersion}, } } return []blueprint.Variation{ diff --git a/cc/cc_test.go b/cc/cc_test.go index 781778714..7489163cc 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -3640,6 +3640,59 @@ func TestMinSdkVersionInClangTriple(t *testing.T) { android.AssertStringDoesContain(t, "min sdk version", cFlags, "-target aarch64-linux-android29") } +func TestMinSdkVersionsOfCrtObjects(t *testing.T) { + ctx := testCc(t, ` + cc_object { + name: "crt_foo", + srcs: ["foo.c"], + crt: true, + stl: "none", + min_sdk_version: "28", + + }`) + + arch := "android_arm64_armv8-a" + for _, v := range []string{"", "28", "29", "30", "current"} { + var variant string + if v == "" { + variant = arch + } else { + variant = arch + "_sdk_" + v + } + cflags := ctx.ModuleForTests("crt_foo", variant).Rule("cc").Args["cFlags"] + vNum := v + if v == "current" || v == "" { + vNum = "10000" + } + expected := "-target aarch64-linux-android" + vNum + " " + android.AssertStringDoesContain(t, "cflag", cflags, expected) + } +} + +func TestUseCrtObjectOfCorrectVersion(t *testing.T) { + ctx := testCc(t, ` + cc_binary { + name: "bin", + srcs: ["foo.c"], + stl: "none", + min_sdk_version: "29", + sdk_version: "current", + } + `) + + // Sdk variant uses the crt object of the matching min_sdk_version + variant := "android_arm64_armv8-a_sdk" + crt := ctx.ModuleForTests("bin", variant).Rule("ld").Args["crtBegin"] + android.AssertStringDoesContain(t, "crt dep of sdk variant", crt, + variant+"_29/crtbegin_dynamic.o") + + // platform variant uses the crt object built for platform + variant = "android_arm64_armv8-a" + crt = ctx.ModuleForTests("bin", variant).Rule("ld").Args["crtBegin"] + android.AssertStringDoesContain(t, "crt dep of platform variant", crt, + variant+"/crtbegin_dynamic.o") +} + type MemtagNoteType int const ( diff --git a/cc/library.go b/cc/library.go index 22a36c6b9..8efde1ec5 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1914,6 +1914,7 @@ func createPerApiVersionVariations(mctx android.BottomUpMutatorContext, minSdkVe for i, module := range modules { module.(*Module).Properties.Sdk_version = StringPtr(versionStrs[i]) + module.(*Module).Properties.Min_sdk_version = StringPtr(versionStrs[i]) } } diff --git a/cc/linkable.go b/cc/linkable.go index 58919a060..13b732bf4 100644 --- a/cc/linkable.go +++ b/cc/linkable.go @@ -111,6 +111,7 @@ type LinkableInterface interface { InProduct() bool SdkVersion() string + MinSdkVersion() string AlwaysSdk() bool IsSdkVariant() bool diff --git a/rust/rust.go b/rust/rust.go index 8ebdb727c..17ff6252f 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -256,6 +256,10 @@ func (mod *Module) SdkVersion() string { return "" } +func (mod *Module) MinSdkVersion() string { + return "" +} + func (mod *Module) AlwaysSdk() bool { return false }