From 1d1119f4bd20368dc686c6980d80a3ea34311716 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 29 Jul 2019 21:27:18 +0900 Subject: [PATCH] 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] Change-Id: Ia447785c485c0d049e19477b32bc638bfe6f1608 --- android/mutator.go | 5 ++ cc/cc.go | 16 ++++ cc/llndk_library.go | 2 +- cc/sanitize.go | 185 +++++++++++++++++++------------------------- 4 files changed, 101 insertions(+), 107 deletions(-) diff --git a/android/mutator.go b/android/mutator.go index 82376e475..8e4343d0f 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -143,6 +143,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) @@ -292,6 +293,10 @@ func (b *bottomUpMutatorContext) SetDependencyVariation(variation string) { b.bp.SetDependencyVariation(variation) } +func (b *bottomUpMutatorContext) SetDefaultDependencyVariation(variation *string) { + b.bp.SetDefaultDependencyVariation(variation) +} + func (b *bottomUpMutatorContext) AddVariationDependencies(variations []blueprint.Variation, tag blueprint.DependencyTag, names ...string) { diff --git a/cc/cc.go b/cc/cc.go index cc2e65faf..cba191fd5 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -56,6 +56,8 @@ func init() { ctx.TopDown("fuzzer_deps", sanitizerDepsMutator(fuzzer)) ctx.BottomUp("fuzzer", sanitizerMutator(fuzzer)).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() @@ -255,6 +257,7 @@ type VendorProperties struct { type ModuleContextIntf interface { static() bool staticBinary() bool + header() bool toolchain() config.Toolchain useSdk() bool sdkVersion() string @@ -715,6 +718,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) != "" @@ -2034,6 +2041,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(actx android.ModuleContext) string { name := actx.ModuleName() if c.useVndk() { diff --git a/cc/llndk_library.go b/cc/llndk_library.go index 82901037b..4d59975f4 100644 --- a/cc/llndk_library.go +++ b/cc/llndk_library.go @@ -232,7 +232,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 b238b7e53..10e0f0e21 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -124,6 +124,10 @@ func (t sanitizerType) name() string { } } +func (t sanitizerType) incompatibleWithCfi() bool { + return t == asan || t == fuzzer || t == hwasan +} + type SanitizeProperties struct { // enable AddressSanitizer, ThreadSanitizer, or UndefinedBehaviorSanitizer Sanitize struct { @@ -554,16 +558,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" + } } } @@ -868,7 +874,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"}, @@ -897,108 +903,69 @@ 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 || t == fuzzer { + // Static and header libs are split into non-sanitized and sanitized variants. + // Shared libs are not split. However, for asan and fuzzer, we split even for shared + // libs because a library sanitized for asan/fuzzer can't be linked from a library + // that isn't sanitized for asan/fuzzer. + // + // 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) + // 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 + } + } - 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 { + // 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 == fuzzer { - // TODO(b/131771163): CFI and fuzzer support are mutually incompatible - // as CFI pulls in LTO. - if mctx.Device() { - 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 == 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 } } } @@ -1034,6 +1001,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) &&