From d5bd6d36768200e9085c2406cdde718f0b0c72aa Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 29 Jul 2019 21:27:18 +0900 Subject: [PATCH 1/2] Fix sanitizer dep This change fixes a problem in sanitizerMutator where a module is linked with of non-sanitized variant of a lib at build-time, but is linked with the sanitized variant of the lib at run-time. This happened because, for each sanitizer type, every shared libs are split into non-sanitized and sanitized variants, and then either of the variants are suppressed from Make so that it isn't installed to the device. This change fixes the problem by NOT splitting for shared libs; only the sanitized variant is created if needed. Header libs, static libs and shared libs for a few sanitizer types (asan/fuzzer) are however split into two. This is because the static and headers libs become part of the depending module, and asan/fuzzer require that the depending module and the dependant module should be compiled for the same sanitizer. Bug: 138103882 Bug: 138426065 Test: m com.android.runtime.debug Check that libziparchive exists under both /system/apex/com.android.runtime/[lib|lib64] Merged-In: Ia447785c485c0d049e19477b32bc638bfe6f1608 (cherry picked from commit 1d1119f4bd20368dc686c6980d80a3ea34311716) Change-Id: Ia447785c485c0d049e19477b32bc638bfe6f1608 --- android/mutator.go | 1 + cc/cc.go | 16 ++++ cc/llndk_library.go | 2 +- cc/sanitize.go | 173 ++++++++++++++++++++------------------------ 4 files changed, 97 insertions(+), 95 deletions(-) diff --git a/android/mutator.go b/android/mutator.go index 71237a1cd..e003f0bbe 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -160,6 +160,7 @@ type BottomUpMutatorContext interface { CreateVariations(...string) []blueprint.Module CreateLocalVariations(...string) []blueprint.Module SetDependencyVariation(string) + SetDefaultDependencyVariation(*string) AddVariationDependencies([]blueprint.Variation, blueprint.DependencyTag, ...string) AddFarVariationDependencies([]blueprint.Variation, blueprint.DependencyTag, ...string) AddInterVariantDependency(tag blueprint.DependencyTag, from, to blueprint.Module) diff --git a/cc/cc.go b/cc/cc.go index 0089dc71c..ebe6b3957 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -51,6 +51,8 @@ func init() { ctx.TopDown("hwasan_deps", sanitizerDepsMutator(hwasan)) ctx.BottomUp("hwasan", sanitizerMutator(hwasan)).Parallel() + // cfi mutator shouldn't run before sanitizers that return true for + // incompatibleWithCfi() ctx.TopDown("cfi_deps", sanitizerDepsMutator(cfi)) ctx.BottomUp("cfi", sanitizerMutator(cfi)).Parallel() @@ -239,6 +241,7 @@ type VendorProperties struct { type ModuleContextIntf interface { static() bool staticBinary() bool + header() bool toolchain() config.Toolchain useSdk() bool sdkVersion() string @@ -662,6 +665,10 @@ func (ctx *moduleContextImpl) staticBinary() bool { return ctx.mod.staticBinary() } +func (ctx *moduleContextImpl) header() bool { + return ctx.mod.header() +} + func (ctx *moduleContextImpl) useSdk() bool { if ctx.ctx.Device() && !ctx.useVndk() && !ctx.inRecovery() && !ctx.ctx.Fuchsia() { return String(ctx.mod.Properties.Sdk_version) != "" @@ -1925,6 +1932,15 @@ func (c *Module) staticBinary() bool { return false } +func (c *Module) header() bool { + if h, ok := c.linker.(interface { + header() bool + }); ok { + return h.header() + } + return false +} + func (c *Module) getMakeLinkType() string { if c.useVndk() { if inList(c.Name(), vndkCoreLibraries) || inList(c.Name(), vndkSpLibraries) || inList(c.Name(), llndkLibraries) { diff --git a/cc/llndk_library.go b/cc/llndk_library.go index 56ef2b67b..52c58eb78 100644 --- a/cc/llndk_library.go +++ b/cc/llndk_library.go @@ -221,7 +221,7 @@ func llndkHeadersFactory() android.Module { &library.MutatedProperties, &library.flagExporter.Properties) - android.InitAndroidArchModule(module, android.DeviceSupported, android.MultilibBoth) + module.Init() return module } diff --git a/cc/sanitize.go b/cc/sanitize.go index b7a36a60c..c1b055afe 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -120,6 +120,10 @@ func (t sanitizerType) name() string { } } +func (t sanitizerType) incompatibleWithCfi() bool { + return t == asan || t == hwasan +} + type SanitizeProperties struct { // enable AddressSanitizer, ThreadSanitizer, or UndefinedBehaviorSanitizer Sanitize struct { @@ -543,16 +547,18 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { } func (sanitize *sanitize) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkData) { - // Add a suffix for CFI-enabled static libraries to allow surfacing both to make without a - // name conflict. - if ret.Class == "STATIC_LIBRARIES" && Bool(sanitize.Properties.Sanitize.Cfi) { - ret.SubName += ".cfi" - } - if ret.Class == "STATIC_LIBRARIES" && Bool(sanitize.Properties.Sanitize.Hwaddress) { - ret.SubName += ".hwasan" - } - if ret.Class == "STATIC_LIBRARIES" && Bool(sanitize.Properties.Sanitize.Scs) { - ret.SubName += ".scs" + // Add a suffix for cfi/hwasan/scs-enabled static/header libraries to allow surfacing + // both the sanitized and non-sanitized variants to make without a name conflict. + if ret.Class == "STATIC_LIBRARIES" || ret.Class == "HEADER_LIBRARIES" { + if Bool(sanitize.Properties.Sanitize.Cfi) { + ret.SubName += ".cfi" + } + if Bool(sanitize.Properties.Sanitize.Hwaddress) { + ret.SubName += ".hwasan" + } + if Bool(sanitize.Properties.Sanitize.Scs) { + ret.SubName += ".scs" + } } } @@ -841,7 +847,7 @@ func sanitizerRuntimeMutator(mctx android.BottomUpMutatorContext) { {Mutator: "image", Variation: c.imageVariation()}, {Mutator: "arch", Variation: mctx.Target().String()}, }, staticDepTag, runtimeLibrary) - } else if !c.static() { + } else if !c.static() && !c.header() { // dynamic executable and shared libs get shared runtime libs mctx.AddFarVariationDependencies([]blueprint.Variation{ {Mutator: "link", Variation: "shared"}, @@ -870,95 +876,68 @@ func sanitizerMutator(t sanitizerType) func(android.BottomUpMutatorContext) { modules := mctx.CreateVariations(t.variationName()) modules[0].(*Module).sanitize.SetSanitizer(t, true) } else if c.sanitize.isSanitizerEnabled(t) || c.sanitize.Properties.SanitizeDep { - // Save original sanitizer status before we assign values to variant - // 0 as that overwrites the original. isSanitizerEnabled := c.sanitize.isSanitizerEnabled(t) + if mctx.Device() && t.incompatibleWithCfi() { + // TODO: Make sure that cfi mutator runs "after" any of the sanitizers that + // are incompatible with cfi + c.sanitize.SetSanitizer(cfi, false) + } + if c.static() || c.header() || t == asan { + // Static and header libs are split into non-sanitized and sanitized variants. + // Shared libs are not split. However, for asan, we split even for shared + // libs because a library sanitized for asan can't be linked from a library + // that isn't sanitized for asan. + // + // Note for defaultVariation: since we don't split for shared libs but for static/header + // libs, it is possible for the sanitized variant of a static/header lib to depend + // on non-sanitized variant of a shared lib. Such unfulfilled variation causes an + // error when the module is split. defaultVariation is the name of the variation that + // will be used when such a dangling dependency occurs during the split of the current + // module. By setting it to the name of the sanitized variation, the dangling dependency + // is redirected to the sanitized variant of the dependent module. + defaultVariation := t.variationName() + mctx.SetDefaultDependencyVariation(&defaultVariation) + modules := mctx.CreateVariations("", t.variationName()) + modules[0].(*Module).sanitize.SetSanitizer(t, false) + modules[1].(*Module).sanitize.SetSanitizer(t, true) + modules[0].(*Module).sanitize.Properties.SanitizeDep = false + modules[1].(*Module).sanitize.Properties.SanitizeDep = false - modules := mctx.CreateVariations("", t.variationName()) - modules[0].(*Module).sanitize.SetSanitizer(t, false) - modules[1].(*Module).sanitize.SetSanitizer(t, true) - - modules[0].(*Module).sanitize.Properties.SanitizeDep = false - modules[1].(*Module).sanitize.Properties.SanitizeDep = false - - // We don't need both variants active for anything but CFI-enabled - // target static libraries, so suppress the appropriate variant in - // all other cases. - if t == cfi { + // For cfi/scs/hwasan, we can export both sanitized and un-sanitized variants + // to Make, because the sanitized version has a different suffix in name. + // For other types of sanitizers, suppress the variation that is disabled. + if t != cfi && t != scs && t != hwasan { + if isSanitizerEnabled { + modules[0].(*Module).Properties.PreventInstall = true + modules[0].(*Module).Properties.HideFromMake = true + } else { + modules[1].(*Module).Properties.PreventInstall = true + modules[1].(*Module).Properties.HideFromMake = true + } + } + // Export the static lib name to make if c.static() { - if !mctx.Device() { - if isSanitizerEnabled { - modules[0].(*Module).Properties.PreventInstall = true - modules[0].(*Module).Properties.HideFromMake = true + if t == cfi { + appendStringSync(c.Name(), cfiStaticLibs(mctx.Config()), &cfiStaticLibsMutex) + } else if t == hwasan { + if c.useVndk() { + appendStringSync(c.Name(), hwasanVendorStaticLibs(mctx.Config()), + &hwasanStaticLibsMutex) } else { - modules[1].(*Module).Properties.PreventInstall = true - modules[1].(*Module).Properties.HideFromMake = true + appendStringSync(c.Name(), hwasanStaticLibs(mctx.Config()), + &hwasanStaticLibsMutex) } - } else { - cfiStaticLibs := cfiStaticLibs(mctx.Config()) + } + } + } else { + // Shared libs are not split. Only the sanitized variant is created. + modules := mctx.CreateVariations(t.variationName()) + modules[0].(*Module).sanitize.SetSanitizer(t, true) + modules[0].(*Module).sanitize.Properties.SanitizeDep = false - cfiStaticLibsMutex.Lock() - *cfiStaticLibs = append(*cfiStaticLibs, c.Name()) - cfiStaticLibsMutex.Unlock() - } - } else { - modules[0].(*Module).Properties.PreventInstall = true - modules[0].(*Module).Properties.HideFromMake = true - } - } else if t == asan { - if mctx.Device() { - // CFI and ASAN are currently mutually exclusive so disable - // CFI if this is an ASAN variant. - modules[1].(*Module).sanitize.Properties.InSanitizerDir = true - modules[1].(*Module).sanitize.SetSanitizer(cfi, false) - } - if isSanitizerEnabled { - modules[0].(*Module).Properties.PreventInstall = true - modules[0].(*Module).Properties.HideFromMake = true - } else { - modules[1].(*Module).Properties.PreventInstall = true - modules[1].(*Module).Properties.HideFromMake = true - } - } else if t == scs { - // We don't currently link any static libraries built with make into - // libraries built with SCS, so we don't need logic for propagating - // SCSness of dependencies into make. - if !c.static() { - if isSanitizerEnabled { - modules[0].(*Module).Properties.PreventInstall = true - modules[0].(*Module).Properties.HideFromMake = true - } else { - modules[1].(*Module).Properties.PreventInstall = true - modules[1].(*Module).Properties.HideFromMake = true - } - } - } else if t == hwasan { - if mctx.Device() { - // CFI and HWASAN are currently mutually exclusive so disable - // CFI if this is an HWASAN variant. - modules[1].(*Module).sanitize.SetSanitizer(cfi, false) - } - - if c.static() { - if c.useVndk() { - hwasanVendorStaticLibs := hwasanVendorStaticLibs(mctx.Config()) - hwasanStaticLibsMutex.Lock() - *hwasanVendorStaticLibs = append(*hwasanVendorStaticLibs, c.Name()) - hwasanStaticLibsMutex.Unlock() - } else { - hwasanStaticLibs := hwasanStaticLibs(mctx.Config()) - hwasanStaticLibsMutex.Lock() - *hwasanStaticLibs = append(*hwasanStaticLibs, c.Name()) - hwasanStaticLibsMutex.Unlock() - } - } else { - if isSanitizerEnabled { - modules[0].(*Module).Properties.PreventInstall = true - modules[0].(*Module).Properties.HideFromMake = true - } else { - modules[1].(*Module).Properties.PreventInstall = true - modules[1].(*Module).Properties.HideFromMake = true - } + // locate the asan libraries under /data/asan + if mctx.Device() && t == asan && isSanitizerEnabled { + modules[0].(*Module).sanitize.Properties.InSanitizerDir = true } } } @@ -994,6 +973,12 @@ func hwasanVendorStaticLibs(config android.Config) *[]string { }).(*[]string) } +func appendStringSync(item string, list *[]string, mutex *sync.Mutex) { + mutex.Lock() + *list = append(*list, item) + mutex.Unlock() +} + func enableMinimalRuntime(sanitize *sanitize) bool { if !Bool(sanitize.Properties.Sanitize.Address) && !Bool(sanitize.Properties.Sanitize.Hwaddress) && From 105e166581d314d8229c903e3d23611acfcbf3dd Mon Sep 17 00:00:00 2001 From: Jaewoong Jung Date: Wed, 4 Sep 2019 13:26:18 -0700 Subject: [PATCH 2/2] Add NOTICE file path to apex bundle base rule. The NOTICE files are missing from prebuilt apexes, and it turns out they were excluded when building bundles. Bug: 140317706 Test: Ran build_mainline_modules.sh and checked bundle base modules. Change-Id: I92c4231f2007e1d8cd9c2bd044201458803c0fd7 --- apex/apex.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apex/apex.go b/apex/apex.go index 69a50f1c8..e5d431edf 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -86,7 +86,8 @@ var ( Command: `${zip2zip} -i $in -o $out ` + `apex_payload.img:apex/${abi}.img ` + `apex_manifest.json:root/apex_manifest.json ` + - `AndroidManifest.xml:manifest/AndroidManifest.xml`, + `AndroidManifest.xml:manifest/AndroidManifest.xml ` + + `assets/NOTICE.html.gz:assets/NOTICE.html.gz`, CommandDeps: []string{"${zip2zip}"}, Description: "app bundle", }, "abi")