From 7549d460f0bdd180f5fd4b314a140ce704254401 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Wed, 25 Aug 2021 16:16:03 +0900 Subject: [PATCH 1/2] crt modules produces cc rules for testing Previously, crt modules didn't produce cc rules because they didn't have any input src files set. This prevented us from having a test which checks the cflags of the crt modules. Fixing that by adding source file to the crt modules. crtbrand is also added as an 'objs' dep, because otherwise partialLd rules won't be generated. Bug: N/A Test: m nothing Change-Id: I731227c20c662c876c40f0c41e1769a271e2c643 --- cc/testing.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/cc/testing.go b/cc/testing.go index 071f1ec3b..d0dca6b53 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -375,26 +375,42 @@ func commonDefaultModules() string { cc_object { name: "crtbegin_so", defaults: ["crt_defaults"], + srcs: ["crtbegin_so.c"], + objs: ["crtbrand"], } cc_object { name: "crtbegin_dynamic", defaults: ["crt_defaults"], + srcs: ["crtbegin.c"], + objs: ["crtbrand"], } cc_object { name: "crtbegin_static", defaults: ["crt_defaults"], + srcs: ["crtbegin.c"], + objs: ["crtbrand"], } cc_object { name: "crtend_so", defaults: ["crt_defaults"], + srcs: ["crtend_so.c"], + objs: ["crtbrand"], } cc_object { name: "crtend_android", defaults: ["crt_defaults"], + srcs: ["crtend.c"], + objs: ["crtbrand"], + } + + cc_object { + name: "crtbrand", + defaults: ["crt_defaults"], + srcs: ["crtbrand.c"], } cc_library { @@ -585,6 +601,11 @@ var PrepareForTestWithCcDefaultModules = android.GroupFixturePreparers( "defaults/cc/common/libm.map.txt": nil, "defaults/cc/common/ndk_libandroid_support": nil, "defaults/cc/common/ndk_libc++_shared": nil, + "defaults/cc/common/crtbegin_so.c": nil, + "defaults/cc/common/crtbegin.c": nil, + "defaults/cc/common/crtend_so.c": nil, + "defaults/cc/common/crtend.c": nil, + "defaults/cc/common/crtbrand.c": nil, }.AddToFixture(), // Place the default cc test modules that are common to all platforms in a location that will not From 5df7bd33f7b64e2b880856e3193419697a8fb693 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Wed, 25 Aug 2021 16:18:46 +0900 Subject: [PATCH 2/2] crt objects for APEX and vendor variants have correct target API levels Previously, crt objects for APEX and vendor variants targetted API level 16 regardless of their context. For example, even if BOARD_VNDK_VERSION is set to 29, or an APEX has `min_sdk_version: "29"`, the target API level was from `min_sdk_version` property of the crt object which is set to 16. The meaning of min_sdk_version is quite different when it comes to crt objects. It means the lowest API level that it CAN target for. It does NOT mean the API level it SHOULD always target. This has caused some other problems like TLS segment underalignment for vendor libraries because the vendor libraries were all built with TLS layout from API level 16. This change fixes the problem by correctly implementing the different semantic of min_sdk_version for crt objects. Bug: N/A Test: m nothing Change-Id: I15328e0b6cbabbe151dd65c7469c6355e167b78a --- apex/apex_test.go | 39 +++++++++++++++++++++++++++++++++++++++ cc/cc.go | 29 +++++++++++++++++++---------- cc/object_test.go | 34 +++++++++++++++++----------------- 3 files changed, 75 insertions(+), 27 deletions(-) diff --git a/apex/apex_test.go b/apex/apex_test.go index f07bf6379..600f6c217 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -1874,6 +1874,45 @@ func TestApexMinSdkVersion_DefaultsToLatest(t *testing.T) { expectNoLink("libx", "shared_apex10000", "libz", "shared") } +func TestApexMinSdkVersion_crtobjectInVendorApex(t *testing.T) { + ctx := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + native_shared_libs: ["mylib"], + updatable: false, + vendor: true, + min_sdk_version: "29", + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + cc_library { + name: "mylib", + vendor_available: true, + system_shared_libs: [], + stl: "none", + apex_available: [ "myapex" ], + min_sdk_version: "29", + } + `) + + vendorVariant := "android_vendor.29_arm64_armv8-a" + + // First check that the correct variant of crtbegin_so is used. + ldRule := ctx.ModuleForTests("mylib", vendorVariant+"_shared_apex29").Rule("ld") + crtBegin := names(ldRule.Args["crtBegin"]) + ensureListContains(t, crtBegin, "out/soong/.intermediates/"+cc.DefaultCcCommonTestModulesDir+"crtbegin_so/"+vendorVariant+"_apex29/crtbegin_so.o") + + // Ensure that the crtbegin_so used by the APEX is targeting 29 + cflags := ctx.ModuleForTests("crtbegin_so", vendorVariant+"_apex29").Rule("cc").Args["cFlags"] + android.AssertStringDoesContain(t, "cflags", cflags, "-target aarch64-linux-android29") +} + func TestPlatformUsesLatestStubsFromApexes(t *testing.T) { ctx := testApex(t, ` apex { diff --git a/cc/cc.go b/cc/cc.go index 243f0a5ae..9c1f5594f 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -975,16 +975,17 @@ func (c *Module) MinSdkVersion() string { return String(c.Properties.Min_sdk_version) } -func (c *Module) SplitPerApiLevel() bool { - if !c.canUseSdk() { - return false - } +func (c *Module) isCrt() bool { if linker, ok := c.linker.(*objectLinker); ok { return linker.isCrt() } return false } +func (c *Module) SplitPerApiLevel() bool { + return c.canUseSdk() && c.isCrt() +} + func (c *Module) AlwaysSdk() bool { return c.Properties.AlwaysSdk || Bool(c.Properties.Sdk_variant_only) } @@ -1446,12 +1447,20 @@ func (ctx *moduleContextImpl) minSdkVersion() string { // 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) + // numbers. However, the non-sdk variant (for apex or platform) of the crt object is left + // untouched. min_sdk_version: 16 doesn't actually mean that the non-sdk variant has to + // support such an old version. The version is set to the later version in case when the + // non-sdk variant is for the platform, or the min_sdk_version of the containing APEX if + // it's for an APEX. + if ctx.mod.isCrt() && !ctx.isSdkVariant() { + if ctx.isForPlatform() { + ver = strconv.Itoa(android.FutureApiLevelInt) + } else { // for apex + ver = ctx.apexSdkVersion().String() + if ver == "" { // in case when min_sdk_version was not set by the APEX + ver = ctx.sdkVersion() + } + } } // Also make sure that minSdkVersion is not greater than sdkVersion, if they are both numbers diff --git a/cc/object_test.go b/cc/object_test.go index 0e5508a4a..259a892ab 100644 --- a/cc/object_test.go +++ b/cc/object_test.go @@ -15,8 +15,9 @@ package cc import ( - "android/soong/android" "testing" + + "android/soong/android" ) func TestMinSdkVersionsOfCrtObjects(t *testing.T) { @@ -27,24 +28,23 @@ func TestMinSdkVersionsOfCrtObjects(t *testing.T) { crt: true, stl: "none", min_sdk_version: "28", - + vendor_available: true, }`) - arch := "android_arm64_armv8-a" - for _, v := range []string{"", "28", "29", "30", "current"} { - var variant string - // platform variant - 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 + " " + variants := []struct { + variant string + num string + }{ + {"android_arm64_armv8-a", "10000"}, + {"android_arm64_armv8-a_sdk_28", "28"}, + {"android_arm64_armv8-a_sdk_29", "29"}, + {"android_arm64_armv8-a_sdk_30", "30"}, + {"android_arm64_armv8-a_sdk_current", "10000"}, + {"android_vendor.29_arm64_armv8-a", "29"}, + } + for _, v := range variants { + cflags := ctx.ModuleForTests("crt_foo", v.variant).Rule("cc").Args["cFlags"] + expected := "-target aarch64-linux-android" + v.num + " " android.AssertStringDoesContain(t, "cflag", cflags, expected) } }