diff options
author | Jiyong Park <jiyong@google.com> | 2019-07-29 21:27:18 +0900 |
---|---|---|
committer | Jiyong Park <jiyong@google.com> | 2019-08-16 13:46:06 +0900 |
commit | d5bd6d36768200e9085c2406cdde718f0b0c72aa (patch) | |
tree | 9ee384e3a1cacca5707240f78aec4c460bf3138b | |
parent | e73a7df1baa243f81797e9529c2ac2dd8a1f5966 (diff) | |
download | soong-d5bd6d36768200e9085c2406cdde718f0b0c72aa.tar.gz |
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
-rw-r--r-- | android/mutator.go | 1 | ||||
-rw-r--r-- | cc/cc.go | 16 | ||||
-rw-r--r-- | cc/llndk_library.go | 2 | ||||
-rw-r--r-- | cc/sanitize.go | 163 |
4 files changed, 92 insertions, 90 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) @@ -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,60 +876,37 @@ 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) - - 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 { - if c.static() { - if !mctx.Device() { - 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 { - cfiStaticLibs := cfiStaticLibs(mctx.Config()) - - 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 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 + + // 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 @@ -932,34 +915,30 @@ func sanitizerMutator(t sanitizerType) func(android.BottomUpMutatorContext) { 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) - } - + // Export the static lib name to make 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 + 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 { + appendStringSync(c.Name(), hwasanStaticLibs(mctx.Config()), + &hwasanStaticLibsMutex) + } } } + } 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 + + // locate the asan libraries under /data/asan + if mctx.Device() && t == asan && isSanitizerEnabled { + modules[0].(*Module).sanitize.Properties.InSanitizerDir = true + } } } c.sanitize.Properties.SanitizeDep = false @@ -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) && |