From 4d8d8fec4a8c3c152aa2d9e9e5252d8612d9afc3 Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Mon, 1 Jun 2020 21:53:49 +0900 Subject: [PATCH] Refine logic choosing vendor snapshot modules This refines the vendor snapshot codes in order to fix logic errors. - Capture toolchain_library and cc_library_headers correctly. - Redirect unwind static library correctly. - Filter out sanitize / coverage / lto by looking at HideFromMake. - Add binary() function for clear and shorter codes. - Include test modules. - Add more tests to prevent further snapshot breakages. Bug: 157106227 Test: m vendor-snapshot Test: m nothing for all available targets Test: EMMA_INSTRUMENT=true EMMA_INSTRUMENT_FRAMEWORK=true \ NATIVE_COVERAGE=true COVERAGE_PATHS="*" m nothing Change-Id: Id90082b5ab730f928582ad24f022ba410855400e --- cc/binary.go | 4 +++ cc/cc.go | 16 +++++++++- cc/cc_test.go | 72 +++++++++++++++++++++++++++++++------------ cc/prebuilt.go | 4 +++ cc/testing.go | 1 + cc/vendor_snapshot.go | 45 ++++++++++++++------------- 6 files changed, 99 insertions(+), 43 deletions(-) diff --git a/cc/binary.go b/cc/binary.go index 661264eef..251b7f0c4 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -231,6 +231,10 @@ func (binary *binaryDecorator) staticBinary() bool { return binary.static() } +func (binary *binaryDecorator) binary() bool { + return true +} + func (binary *binaryDecorator) linkerFlags(ctx ModuleContext, flags Flags) Flags { flags = binary.baseLinker.linkerFlags(ctx, flags) diff --git a/cc/cc.go b/cc/cc.go index 01bf5f1ef..150843cd5 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -309,6 +309,7 @@ type ModuleContextIntf interface { static() bool staticBinary() bool header() bool + binary() bool toolchain() config.Toolchain canUseSdk() bool useSdk() bool @@ -1114,6 +1115,10 @@ func (ctx *moduleContextImpl) header() bool { return ctx.mod.header() } +func (ctx *moduleContextImpl) binary() bool { + return ctx.mod.binary() +} + func (ctx *moduleContextImpl) canUseSdk() bool { return ctx.mod.canUseSdk() } @@ -1896,7 +1901,7 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { if deps.StaticUnwinderIfLegacy { actx.AddVariationDependencies([]blueprint.Variation{ {Mutator: "link", Variation: "static"}, - }, staticUnwinderDepTag, staticUnwinder(actx)) + }, staticUnwinderDepTag, rewriteSnapshotLibs(staticUnwinder(actx), vendorSnapshotStaticLibs)) } for _, lib := range deps.LateStaticLibs { @@ -2713,6 +2718,15 @@ func (c *Module) header() bool { return false } +func (c *Module) binary() bool { + if b, ok := c.linker.(interface { + binary() bool + }); ok { + return b.binary() + } + return false +} + func (c *Module) getMakeLinkType(actx android.ModuleContext) string { if c.UseVndk() { if lib, ok := c.linker.(*llndkStubDecorator); ok { diff --git a/cc/cc_test.go b/cc/cc_test.go index 67eb248d2..0a25b5e02 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -258,9 +258,7 @@ func checkVndkModule(t *testing.T, ctx *android.TestContext, name, subDir string } } -func checkSnapshot(t *testing.T, ctx *android.TestContext, singletonName, moduleName, snapshotFilename, subDir, variant string) { - snapshotSingleton := ctx.SingletonForTests(singletonName) - +func checkSnapshot(t *testing.T, ctx *android.TestContext, singleton android.TestingSingleton, moduleName, snapshotFilename, subDir, variant string) { mod, ok := ctx.ModuleForTests(moduleName, variant).Module().(android.OutputFileProducer) if !ok { t.Errorf("%q must have output\n", moduleName) @@ -273,7 +271,7 @@ func checkSnapshot(t *testing.T, ctx *android.TestContext, singletonName, module } snapshotPath := filepath.Join(subDir, snapshotFilename) - out := snapshotSingleton.Output(snapshotPath) + out := singleton.Output(snapshotPath) if out.Input.String() != outputFiles[0].String() { t.Errorf("The input of snapshot %q must be %q, but %q", moduleName, out.Input.String(), outputFiles[0]) } @@ -398,16 +396,18 @@ func TestVndk(t *testing.T) { variant := "android_vendor.VER_arm64_armv8-a_shared" variant2nd := "android_vendor.VER_arm_armv7-a-neon_shared" - checkSnapshot(t, ctx, "vndk-snapshot", "libvndk", "libvndk.so", vndkCoreLibPath, variant) - checkSnapshot(t, ctx, "vndk-snapshot", "libvndk", "libvndk.so", vndkCoreLib2ndPath, variant2nd) - checkSnapshot(t, ctx, "vndk-snapshot", "libvndk_sp", "libvndk_sp-x.so", vndkSpLibPath, variant) - checkSnapshot(t, ctx, "vndk-snapshot", "libvndk_sp", "libvndk_sp-x.so", vndkSpLib2ndPath, variant2nd) + snapshotSingleton := ctx.SingletonForTests("vndk-snapshot") + + checkSnapshot(t, ctx, snapshotSingleton, "libvndk", "libvndk.so", vndkCoreLibPath, variant) + checkSnapshot(t, ctx, snapshotSingleton, "libvndk", "libvndk.so", vndkCoreLib2ndPath, variant2nd) + checkSnapshot(t, ctx, snapshotSingleton, "libvndk_sp", "libvndk_sp-x.so", vndkSpLibPath, variant) + checkSnapshot(t, ctx, snapshotSingleton, "libvndk_sp", "libvndk_sp-x.so", vndkSpLib2ndPath, variant2nd) snapshotConfigsPath := filepath.Join(snapshotVariantPath, "configs") - checkSnapshot(t, ctx, "vndk-snapshot", "llndk.libraries.txt", "llndk.libraries.txt", snapshotConfigsPath, "") - checkSnapshot(t, ctx, "vndk-snapshot", "vndkcore.libraries.txt", "vndkcore.libraries.txt", snapshotConfigsPath, "") - checkSnapshot(t, ctx, "vndk-snapshot", "vndksp.libraries.txt", "vndksp.libraries.txt", snapshotConfigsPath, "") - checkSnapshot(t, ctx, "vndk-snapshot", "vndkprivate.libraries.txt", "vndkprivate.libraries.txt", snapshotConfigsPath, "") + checkSnapshot(t, ctx, snapshotSingleton, "llndk.libraries.txt", "llndk.libraries.txt", snapshotConfigsPath, "") + checkSnapshot(t, ctx, snapshotSingleton, "vndkcore.libraries.txt", "vndkcore.libraries.txt", snapshotConfigsPath, "") + checkSnapshot(t, ctx, snapshotSingleton, "vndksp.libraries.txt", "vndksp.libraries.txt", snapshotConfigsPath, "") + checkSnapshot(t, ctx, snapshotSingleton, "vndkprivate.libraries.txt", "vndkprivate.libraries.txt", snapshotConfigsPath, "") checkVndkOutput(t, ctx, "vndk/vndk.libraries.txt", []string{ "LLNDK: libc.so", @@ -839,6 +839,12 @@ func TestVendorSnapshot(t *testing.T) { vendor_available: true, nocrt: true, } + + toolchain_library { + name: "libb", + vendor_available: true, + src: "libb.a", + } ` config := TestConfig(buildDir, android.Android, nil, bp, nil) config.TestProductVariables.DeviceVndkVersion = StringPtr("current") @@ -849,6 +855,9 @@ func TestVendorSnapshot(t *testing.T) { snapshotDir := "vendor-snapshot" snapshotVariantPath := filepath.Join(buildDir, snapshotDir, "arm64") + snapshotSingleton := ctx.SingletonForTests("vendor-snapshot") + + var jsonFiles []string for _, arch := range [][]string{ []string{"arm64", "armv8-a"}, @@ -861,22 +870,45 @@ func TestVendorSnapshot(t *testing.T) { // For shared libraries, only non-VNDK vendor_available modules are captured sharedVariant := fmt.Sprintf("android_vendor.VER_%s_%s_shared", archType, archVariant) sharedDir := filepath.Join(snapshotVariantPath, archDir, "shared") - checkSnapshot(t, ctx, "vendor-snapshot", "libvendor", "libvendor.so", sharedDir, sharedVariant) - checkSnapshot(t, ctx, "vendor-snapshot", "libvendor_available", "libvendor_available.so", sharedDir, sharedVariant) + checkSnapshot(t, ctx, snapshotSingleton, "libvendor", "libvendor.so", sharedDir, sharedVariant) + checkSnapshot(t, ctx, snapshotSingleton, "libvendor_available", "libvendor_available.so", sharedDir, sharedVariant) + jsonFiles = append(jsonFiles, + filepath.Join(sharedDir, "libvendor.so.json"), + filepath.Join(sharedDir, "libvendor_available.so.json")) // For static libraries, all vendor:true and vendor_available modules (including VNDK) are captured. staticVariant := fmt.Sprintf("android_vendor.VER_%s_%s_static", archType, archVariant) staticDir := filepath.Join(snapshotVariantPath, archDir, "static") - checkSnapshot(t, ctx, "vendor-snapshot", "libvndk", "libvndk.a", staticDir, staticVariant) - checkSnapshot(t, ctx, "vendor-snapshot", "libvendor", "libvendor.a", staticDir, staticVariant) - checkSnapshot(t, ctx, "vendor-snapshot", "libvendor_available", "libvendor_available.a", staticDir, staticVariant) + checkSnapshot(t, ctx, snapshotSingleton, "libb", "libb.a", staticDir, staticVariant) + checkSnapshot(t, ctx, snapshotSingleton, "libvndk", "libvndk.a", staticDir, staticVariant) + checkSnapshot(t, ctx, snapshotSingleton, "libvendor", "libvendor.a", staticDir, staticVariant) + checkSnapshot(t, ctx, snapshotSingleton, "libvendor_available", "libvendor_available.a", staticDir, staticVariant) + jsonFiles = append(jsonFiles, + filepath.Join(staticDir, "libb.a.json"), + filepath.Join(staticDir, "libvndk.a.json"), + filepath.Join(staticDir, "libvendor.a.json"), + filepath.Join(staticDir, "libvendor_available.a.json")) - // For binary libraries, all vendor:true and vendor_available modules are captured. + // For binary executables, all vendor:true and vendor_available modules are captured. if archType == "arm64" { binaryVariant := fmt.Sprintf("android_vendor.VER_%s_%s", archType, archVariant) binaryDir := filepath.Join(snapshotVariantPath, archDir, "binary") - checkSnapshot(t, ctx, "vendor-snapshot", "vendor_bin", "vendor_bin", binaryDir, binaryVariant) - checkSnapshot(t, ctx, "vendor-snapshot", "vendor_available_bin", "vendor_available_bin", binaryDir, binaryVariant) + checkSnapshot(t, ctx, snapshotSingleton, "vendor_bin", "vendor_bin", binaryDir, binaryVariant) + checkSnapshot(t, ctx, snapshotSingleton, "vendor_available_bin", "vendor_available_bin", binaryDir, binaryVariant) + jsonFiles = append(jsonFiles, + filepath.Join(binaryDir, "vendor_bin.json"), + filepath.Join(binaryDir, "vendor_available_bin.json")) + } + + // For header libraries, all vendor:true and vendor_available modules are captured. + headerDir := filepath.Join(snapshotVariantPath, archDir, "header") + jsonFiles = append(jsonFiles, filepath.Join(headerDir, "libvendor_headers.json")) + } + + for _, jsonFile := range jsonFiles { + // verify all json files exist + if snapshotSingleton.MaybeOutput(jsonFile).Rule == nil { + t.Errorf("%q expected but not found", jsonFile) } } } diff --git a/cc/prebuilt.go b/cc/prebuilt.go index 2ef31950e..d22838add 100644 --- a/cc/prebuilt.go +++ b/cc/prebuilt.go @@ -306,6 +306,10 @@ func (p *prebuiltBinaryLinker) link(ctx ModuleContext, return nil } +func (p *prebuiltBinaryLinker) binary() bool { + return true +} + // cc_prebuilt_binary installs a precompiled executable in srcs property in the // device's directory. func prebuiltBinaryFactory() android.Module { diff --git a/cc/testing.go b/cc/testing.go index 611992070..d92309f3d 100644 --- a/cc/testing.go +++ b/cc/testing.go @@ -457,6 +457,7 @@ func TestConfig(buildDir string, os android.OsType, env map[string]string, "my_include": nil, "foo.map.txt": nil, "liba.so": nil, + "libb.a": nil, } GatherRequiredFilesForTest(mockFS) diff --git a/cc/vendor_snapshot.go b/cc/vendor_snapshot.go index c79a653cd..13a6b055d 100644 --- a/cc/vendor_snapshot.go +++ b/cc/vendor_snapshot.go @@ -429,7 +429,7 @@ func isVendorProprietaryPath(dir string) bool { // depend on newer VNDK) So they are captured as vendor snapshot To build older vendor // image and newer system image altogether. func isVendorSnapshotModule(m *Module, moduleDir string) bool { - if !m.Enabled() { + if !m.Enabled() || m.Properties.HideFromMake { return false } // skip proprietary modules, but include all VNDK (static) @@ -443,37 +443,37 @@ func isVendorSnapshotModule(m *Module, moduleDir string) bool { return false } // the module must be installed in /vendor - if !m.installable() || m.isSnapshotPrebuilt() || !m.inVendor() { - return false - } - // exclude test modules - if _, ok := m.linker.(interface{ gtest() bool }); ok { - return false - } - // TODO(b/65377115): add full support for sanitizer - if m.sanitize != nil && !m.sanitize.isUnsanitizedVariant() { + if !m.IsForPlatform() || m.isSnapshotPrebuilt() || !m.inVendor() { return false } // Libraries if l, ok := m.linker.(snapshotLibraryInterface); ok { + // TODO(b/65377115): add full support for sanitizer + if m.sanitize != nil { + // cfi, scs and hwasan export both sanitized and unsanitized variants for static and header + // Always use unsanitized variants of them. + for _, t := range []sanitizerType{cfi, scs, hwasan} { + if !l.shared() && m.sanitize.isSanitizerEnabled(t) { + return false + } + } + } if l.static() { - return proptools.BoolDefault(m.VendorProperties.Vendor_available, true) + return m.outputFile.Valid() && proptools.BoolDefault(m.VendorProperties.Vendor_available, true) } if l.shared() { - return !m.IsVndk() + return m.outputFile.Valid() && !m.IsVndk() } return true } // Binaries - _, ok := m.linker.(*binaryDecorator) - if !ok { - if _, ok := m.linker.(*prebuiltBinaryLinker); !ok { - return false - } + if m.binary() { + return m.outputFile.Valid() && proptools.BoolDefault(m.VendorProperties.Vendor_available, true) } - return proptools.BoolDefault(m.VendorProperties.Vendor_available, true) + + return false } func (c *vendorSnapshotSingleton) GenerateBuildActions(ctx android.SingletonContext) { @@ -620,7 +620,7 @@ func (c *vendorSnapshotSingleton) GenerateBuildActions(ctx android.SingletonCont } propOut = filepath.Join(snapshotArchDir, targetArch, libType, stem+".json") - } else { + } else if m.binary() { // binary flags prop.Symlinks = m.Symlinks() prop.SharedLibs = m.Properties.SnapshotSharedLibs @@ -630,6 +630,9 @@ func (c *vendorSnapshotSingleton) GenerateBuildActions(ctx android.SingletonCont snapshotBinOut := filepath.Join(snapshotArchDir, targetArch, "binary", binPath.Base()) ret = append(ret, copyFile(ctx, binPath, snapshotBinOut)) propOut = snapshotBinOut + ".json" + } else { + ctx.Errorf("unknown module %q in vendor snapshot", m.String()) + return nil } j, err := json.Marshal(prop) @@ -815,9 +818,7 @@ func VendorSnapshotSourceMutator(ctx android.BottomUpMutatorContext) { // header snapshotMap = vendorSnapshotHeaderLibs(ctx.Config()) } - } else if _, ok := module.linker.(*binaryDecorator); ok { - snapshotMap = vendorSnapshotBinaries(ctx.Config()) - } else if _, ok := module.linker.(*prebuiltBinaryLinker); ok { + } else if module.binary() { snapshotMap = vendorSnapshotBinaries(ctx.Config()) } else { return