From 92fe74067729af7dbfce82cd73fae83758eb8cf2 Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Wed, 15 Jul 2020 13:33:30 -0700 Subject: [PATCH] Stop using prebuilt NDK CRT objects. We don't need the prebuilt versions. The NDK CRT objects are (now) built from the platform sources and the only difference is that the NDK CRT objects also include an ELF note that identifies the NDK version, which isn't helpful for anything built by the platform. Add a `crt` property to cc_object that allows CRT objects to identify themselves. CRT objects, unlike other modules, will have a variant built per-API level they support, rather than just an SDK variant and a platform variant. This is needed because new CRT objects will rely on APIs not available in old libcs and old CRT objects will not support all the features of a modern one. Test: treehugger Bug: http://b/159925977 Change-Id: I6595485fa1bfe0ad4945193d344b863f64eec654 --- cc/binary.go | 30 ++++---------------------- cc/cc.go | 41 +++++++++++++++++++++++++++++------ cc/compiler.go | 2 +- cc/library.go | 17 ++------------- cc/ndk_library.go | 54 +++++++++++++++++++++++++++++++++++------------ cc/object.go | 8 +++++++ cc/testing.go | 44 ++------------------------------------ java/app_test.go | 32 +++++++++++++++------------- rust/rust.go | 5 +++-- rust/testing.go | 1 + 10 files changed, 113 insertions(+), 121 deletions(-) diff --git a/cc/binary.go b/cc/binary.go index 565cb8ae7..63bbd8385 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -130,34 +130,12 @@ func (binary *binaryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps { deps = binary.baseLinker.linkerDeps(ctx, deps) if ctx.toolchain().Bionic() { if !Bool(binary.baseLinker.Properties.Nocrt) { - if !ctx.useSdk() { - if binary.static() { - deps.CrtBegin = "crtbegin_static" - } else { - deps.CrtBegin = "crtbegin_dynamic" - } - deps.CrtEnd = "crtend_android" + if binary.static() { + deps.CrtBegin = "crtbegin_static" } else { - // TODO(danalbert): Add generation of crt objects. - // For `sdk_version: "current"`, we don't actually have a - // freshly generated set of CRT objects. Use the last stable - // version. - version := ctx.sdkVersion() - if version == "current" { - version = getCurrentNdkPrebuiltVersion(ctx) - } - - if binary.static() { - deps.CrtBegin = "ndk_crtbegin_static." + version - } else { - if binary.static() { - deps.CrtBegin = "ndk_crtbegin_static." + version - } else { - deps.CrtBegin = "ndk_crtbegin_dynamic." + version - } - deps.CrtEnd = "ndk_crtend_android." + version - } + deps.CrtBegin = "crtbegin_dynamic" } + deps.CrtEnd = "crtend_android" } if binary.static() { diff --git a/cc/cc.go b/cc/cc.go index 6f1a06d99..cef99eb24 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -681,6 +681,13 @@ func (c *Module) MinSdkVersion() string { return String(c.Properties.Min_sdk_version) } +func (c *Module) SplitPerApiLevel() bool { + if linker, ok := c.linker.(*objectLinker); ok { + return linker.isCrt() + } + return false +} + func (c *Module) AlwaysSdk() bool { return c.Properties.AlwaysSdk || Bool(c.Properties.Sdk_variant_only) } @@ -952,7 +959,7 @@ func (c *Module) canUseSdk() bool { func (c *Module) UseSdk() bool { if c.canUseSdk() { - return String(c.Properties.Sdk_version) != "" + return String(c.Properties.Sdk_version) != "" || c.SplitPerApiLevel() } return false } @@ -1485,8 +1492,11 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { c.Properties.SubName += ramdiskSuffix } else if c.InRecovery() && !c.OnlyInRecovery() { c.Properties.SubName += recoverySuffix - } else if c.Properties.IsSdkVariant && c.Properties.SdkAndPlatformVariantVisibleToMake { + } else if c.IsSdkVariant() && (c.Properties.SdkAndPlatformVariantVisibleToMake || c.SplitPerApiLevel()) { c.Properties.SubName += sdkSuffix + if c.SplitPerApiLevel() { + c.Properties.SubName += "." + c.SdkVersion() + } } ctx := &moduleContext{ @@ -1659,7 +1669,7 @@ func (c *Module) begin(ctx BaseModuleContext) { for _, feature := range c.features { feature.begin(ctx) } - if ctx.useSdk() { + if ctx.useSdk() && c.IsSdkVariant() { version, err := normalizeNdkApiLevel(ctx, ctx.sdkVersion(), ctx.Arch()) if err != nil { ctx.PropertyErrorf("sdk_version", err.Error()) @@ -1762,6 +1772,22 @@ func StubsLibNameAndVersion(name string) (string, string) { return name, "" } +func GetCrtVariations(ctx android.BottomUpMutatorContext, + m LinkableInterface) []blueprint.Variation { + if ctx.Os() != android.Android { + return nil + } + if m.UseSdk() { + return []blueprint.Variation{ + {Mutator: "sdk", Variation: "sdk"}, + {Mutator: "ndk_api", Variation: m.SdkVersion()}, + } + } + return []blueprint.Variation{ + {Mutator: "sdk", Variation: ""}, + } +} + func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { if !c.Enabled() { return @@ -2024,11 +2050,14 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { vendorSnapshotObjects := vendorSnapshotObjects(actx.Config()) + crtVariations := GetCrtVariations(ctx, c) if deps.CrtBegin != "" { - actx.AddVariationDependencies(nil, CrtBeginDepTag, rewriteSnapshotLibs(deps.CrtBegin, vendorSnapshotObjects)) + actx.AddVariationDependencies(crtVariations, CrtBeginDepTag, + rewriteSnapshotLibs(deps.CrtBegin, vendorSnapshotObjects)) } if deps.CrtEnd != "" { - actx.AddVariationDependencies(nil, CrtEndDepTag, rewriteSnapshotLibs(deps.CrtEnd, vendorSnapshotObjects)) + actx.AddVariationDependencies(crtVariations, CrtEndDepTag, + rewriteSnapshotLibs(deps.CrtEnd, vendorSnapshotObjects)) } if deps.LinkerFlagsFile != "" { actx.AddDependency(c, linkerFlagsDepTag, deps.LinkerFlagsFile) @@ -3076,7 +3105,7 @@ func squashRecoverySrcs(m *Module) { } func (c *Module) IsSdkVariant() bool { - return c.Properties.IsSdkVariant + return c.Properties.IsSdkVariant || c.AlwaysSdk() } func getCurrentNdkPrebuiltVersion(ctx DepsContext) string { diff --git a/cc/compiler.go b/cc/compiler.go index d5ea2c3ae..d0b5b46b7 100644 --- a/cc/compiler.go +++ b/cc/compiler.go @@ -565,7 +565,7 @@ func makeDefineString(name string) string { var gnuToCReplacer = strings.NewReplacer("gnu", "c") func ndkPathDeps(ctx ModuleContext) android.Paths { - if ctx.useSdk() { + if ctx.Module().(*Module).IsSdkVariant() { // The NDK sysroot timestamp file depends on all the NDK sysroot files // (headers and libraries). return android.Paths{getNdkBaseTimestampFile(ctx)} diff --git a/cc/library.go b/cc/library.go index 89f480f56..1c2b1ee88 100644 --- a/cc/library.go +++ b/cc/library.go @@ -800,21 +800,8 @@ func (library *libraryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps { deps.ReexportStaticLibHeaders = append(deps.ReexportStaticLibHeaders, library.StaticProperties.Static.Export_static_lib_headers...) } else if library.shared() { if ctx.toolchain().Bionic() && !Bool(library.baseLinker.Properties.Nocrt) { - if !ctx.useSdk() { - deps.CrtBegin = "crtbegin_so" - deps.CrtEnd = "crtend_so" - } else { - // TODO(danalbert): Add generation of crt objects. - // For `sdk_version: "current"`, we don't actually have a - // freshly generated set of CRT objects. Use the last stable - // version. - version := ctx.sdkVersion() - if version == "current" { - version = getCurrentNdkPrebuiltVersion(ctx) - } - deps.CrtBegin = "ndk_crtbegin_so." + version - deps.CrtEnd = "ndk_crtend_so." + version - } + deps.CrtBegin = "crtbegin_so" + deps.CrtEnd = "crtend_so" } deps.WholeStaticLibs = append(deps.WholeStaticLibs, library.SharedProperties.Shared.Whole_static_libs...) deps.StaticLibs = append(deps.StaticLibs, library.SharedProperties.Shared.Static_libs...) diff --git a/cc/ndk_library.go b/cc/ndk_library.go index 22e3ec32d..58e742e5f 100644 --- a/cc/ndk_library.go +++ b/cc/ndk_library.go @@ -110,6 +110,10 @@ func intMax(a int, b int) int { func normalizeNdkApiLevel(ctx android.BaseModuleContext, apiLevel string, arch android.Arch) (string, error) { + if apiLevel == "" { + panic("empty apiLevel not allowed") + } + if apiLevel == "current" { return apiLevel, nil } @@ -136,7 +140,8 @@ func normalizeNdkApiLevel(ctx android.BaseModuleContext, apiLevel string, // supported version here instead. version, err := strconv.Atoi(apiLevel) if err != nil { - return "", fmt.Errorf("API level must be an integer (is %q)", apiLevel) + // Non-integer API levels are codenames. + return apiLevel, nil } version = intMax(version, minVersion) @@ -182,40 +187,61 @@ func shouldUseVersionScript(ctx android.BaseModuleContext, stub *stubDecorator) return version >= unversionedUntil, nil } -func generateStubApiVariants(mctx android.BottomUpMutatorContext, c *stubDecorator) { - platformVersion := mctx.Config().PlatformSdkVersionInt() +func generatePerApiVariants(ctx android.BottomUpMutatorContext, m *Module, + propName string, propValue string, perSplit func(*Module, string)) { + platformVersion := ctx.Config().PlatformSdkVersionInt() - firstSupportedVersion, err := normalizeNdkApiLevel(mctx, String(c.properties.First_version), - mctx.Arch()) + firstSupportedVersion, err := normalizeNdkApiLevel(ctx, propValue, + ctx.Arch()) if err != nil { - mctx.PropertyErrorf("first_version", err.Error()) + ctx.PropertyErrorf(propName, err.Error()) } - firstGenVersion, err := getFirstGeneratedVersion(firstSupportedVersion, platformVersion) + firstGenVersion, err := getFirstGeneratedVersion(firstSupportedVersion, + platformVersion) if err != nil { // In theory this is impossible because we've already run this through // normalizeNdkApiLevel above. - mctx.PropertyErrorf("first_version", err.Error()) + ctx.PropertyErrorf(propName, err.Error()) } var versionStrs []string for version := firstGenVersion; version <= platformVersion; version++ { versionStrs = append(versionStrs, strconv.Itoa(version)) } - versionStrs = append(versionStrs, mctx.Config().PlatformVersionActiveCodenames()...) + versionStrs = append(versionStrs, ctx.Config().PlatformVersionActiveCodenames()...) versionStrs = append(versionStrs, "current") - modules := mctx.CreateVariations(versionStrs...) + modules := ctx.CreateVariations(versionStrs...) for i, module := range modules { - module.(*Module).compiler.(*stubDecorator).properties.ApiLevel = versionStrs[i] + perSplit(module.(*Module), versionStrs[i]) } } -func NdkApiMutator(mctx android.BottomUpMutatorContext) { - if m, ok := mctx.Module().(*Module); ok { +func NdkApiMutator(ctx android.BottomUpMutatorContext) { + if m, ok := ctx.Module().(*Module); ok { if m.Enabled() { if compiler, ok := m.compiler.(*stubDecorator); ok { - generateStubApiVariants(mctx, compiler) + if ctx.Os() != android.Android { + // These modules are always android.DeviceEnabled only, but + // those include Fuchsia devices, which we don't support. + ctx.Module().Disable() + return + } + generatePerApiVariants(ctx, m, "first_version", + String(compiler.properties.First_version), + func(m *Module, version string) { + m.compiler.(*stubDecorator).properties.ApiLevel = + version + }) + } else if m.SplitPerApiLevel() && m.IsSdkVariant() { + if ctx.Os() != android.Android { + return + } + generatePerApiVariants(ctx, m, "min_sdk_version", + m.MinSdkVersion(), func(m *Module, version string) { + m.Properties.Sdk_version = &version + }) } } } diff --git a/cc/object.go b/cc/object.go index 15a529e85..778d131ba 100644 --- a/cc/object.go +++ b/cc/object.go @@ -55,6 +55,10 @@ type ObjectLinkerProperties struct { // if set, the path to a linker script to pass to ld -r when combining multiple object files. Linker_script *string `android:"path,arch_variant"` + + // Indicates that this module is a CRT object. CRT objects will be split + // into a variant per-API level between min_sdk_version and current. + Crt *bool } func newObject() *Module { @@ -162,3 +166,7 @@ func (object *objectLinker) coverageOutputFilePath() android.OptionalPath { func (object *objectLinker) object() bool { return true } + +func (object *objectLinker) isCrt() bool { + return Bool(object.Properties.Crt) +} diff --git a/cc/testing.go b/cc/testing.go index 4113fd2b8..06e5f832c 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -338,6 +338,8 @@ func GatherRequiredDepsForTest(oses ...android.OsType) string { vendor_available: true, native_bridge_supported: true, stl: "none", + min_sdk_version: "16", + crt: true, apex_available: [ "//apex_available:platform", "//apex_available:anyapex", @@ -347,48 +349,26 @@ func GatherRequiredDepsForTest(oses ...android.OsType) string { cc_object { name: "crtbegin_so", defaults: ["crt_defaults"], - recovery_available: true, - vendor_available: true, - native_bridge_supported: true, - min_sdk_version: "29", - stl: "none", } cc_object { name: "crtbegin_dynamic", defaults: ["crt_defaults"], - recovery_available: true, - vendor_available: true, - native_bridge_supported: true, - stl: "none", } cc_object { name: "crtbegin_static", defaults: ["crt_defaults"], - recovery_available: true, - vendor_available: true, - native_bridge_supported: true, - stl: "none", } cc_object { name: "crtend_so", defaults: ["crt_defaults"], - recovery_available: true, - vendor_available: true, - native_bridge_supported: true, - min_sdk_version: "29", - stl: "none", } cc_object { name: "crtend_android", defaults: ["crt_defaults"], - recovery_available: true, - vendor_available: true, - native_bridge_supported: true, - stl: "none", } cc_library { @@ -420,26 +400,6 @@ func GatherRequiredDepsForTest(oses ...android.OsType) string { symbol_file: "libdl.map.txt", } - ndk_prebuilt_object { - name: "ndk_crtbegin_so.27", - sdk_version: "27", - } - - ndk_prebuilt_object { - name: "ndk_crtend_so.27", - sdk_version: "27", - } - - ndk_prebuilt_object { - name: "ndk_crtbegin_dynamic.27", - sdk_version: "27", - } - - ndk_prebuilt_object { - name: "ndk_crtend_android.27", - sdk_version: "27", - } - ndk_prebuilt_shared_stl { name: "ndk_libc++_shared", } diff --git a/java/app_test.go b/java/app_test.go index f50aa3a52..a070318db 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -531,16 +531,6 @@ func TestUpdatableApps_JniLibShouldBeBuiltAgainstMinSdkVersion(t *testing.T) { 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, @@ -553,16 +543,28 @@ func TestUpdatableApps_JniLibShouldBeBuiltAgainstMinSdkVersion(t *testing.T) { inputs := ctx.ModuleForTests("libjni", "android_arm64_armv8-a_sdk_shared").Description("link").Implicits var crtbeginFound, crtendFound bool + expectedCrtBegin := ctx.ModuleForTests("crtbegin_so", + "android_arm64_armv8-a_sdk_29").Rule("partialLd").Output + expectedCrtEnd := ctx.ModuleForTests("crtend_so", + "android_arm64_armv8-a_sdk_29").Rule("partialLd").Output + implicits := []string{} for _, input := range inputs { - switch input.String() { - case "prebuilts/ndk/current/platforms/android-29/arch-arm64/usr/lib/crtbegin_so.o": + implicits = append(implicits, input.String()) + if strings.HasSuffix(input.String(), expectedCrtBegin.String()) { crtbeginFound = true - case "prebuilts/ndk/current/platforms/android-29/arch-arm64/usr/lib/crtend_so.o": + } else if strings.HasSuffix(input.String(), expectedCrtEnd.String()) { crtendFound = true } } - if !crtbeginFound || !crtendFound { - t.Error("should link with ndk_crtbegin_so.29 and ndk_crtend_so.29") + if !crtbeginFound { + t.Error(fmt.Sprintf( + "expected implicit with suffix %q, have the following implicits:\n%s", + expectedCrtBegin, strings.Join(implicits, "\n"))) + } + if !crtendFound { + t.Error(fmt.Sprintf( + "expected implicit with suffix %q, have the following implicits:\n%s", + expectedCrtEnd, strings.Join(implicits, "\n"))) } } diff --git a/rust/rust.go b/rust/rust.go index 17fd04225..fe2851e4a 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -1003,11 +1003,12 @@ func (mod *Module) DepsMutator(actx android.BottomUpMutatorContext) { blueprint.Variation{Mutator: "link", Variation: "static"}), cc.StaticDepTag(), deps.StaticLibs...) + crtVariations := append(cc.GetCrtVariations(ctx, mod), commonDepVariations...) if deps.CrtBegin != "" { - actx.AddVariationDependencies(commonDepVariations, cc.CrtBeginDepTag, deps.CrtBegin) + actx.AddVariationDependencies(crtVariations, cc.CrtBeginDepTag, deps.CrtBegin) } if deps.CrtEnd != "" { - actx.AddVariationDependencies(commonDepVariations, cc.CrtEndDepTag, deps.CrtEnd) + actx.AddVariationDependencies(crtVariations, cc.CrtEndDepTag, deps.CrtEnd) } if mod.sourceProvider != nil { diff --git a/rust/testing.go b/rust/testing.go index 925b02c96..80e414871 100644 --- a/rust/testing.go +++ b/rust/testing.go @@ -99,6 +99,7 @@ func GatherRequiredDepsForTest() string { func CreateTestContext() *android.TestContext { ctx := android.NewTestArchContext() android.RegisterPrebuiltMutators(ctx) + ctx.PreArchMutators(android.RegisterDefaultsPreArchMutators) cc.RegisterRequiredBuildComponentsForTest(ctx) ctx.RegisterModuleType("genrule", genrule.GenRuleFactory) ctx.RegisterModuleType("rust_binary", RustBinaryFactory)