From d4b484b0707c2dbbe1e3ac096f953344aea2b1dc Mon Sep 17 00:00:00 2001 From: Vishwath Mohan Date: Tue, 18 Jul 2017 19:07:30 -0700 Subject: [PATCH] Build system changes for CFI (Soong) This CL makes multiples changes in preparation for platform-wide CFI. (a) Adds a second -version-script=... to the command line when building components that use a version script. This ensures that __cfi_check is also exported, and allows CFI to be enabled for these components. (b) Adds both topdown and bottom up mutators for CFI to help propagate dependencies correctly for components that may need CFI disabled. (c) Fixes an issue with the mutators to correctly apply settings to both generated variants (d) Fixes issues when components have more than a single visibility flag. Bug: 30227045 Test: SANITIZE_TARGET=cfi m -j40 # dependencies are correctly built # with/without CFI Change-Id: I44793cc03bcbcdaa957cc49c7240b87d7c9db327 --- cc/androidmk.go | 5 ++ cc/cc.go | 19 ++++--- cc/config/cfi_exports.map | 4 ++ cc/library.go | 4 ++ cc/sanitize.go | 108 +++++++++++++++++++++++++++++++------- 5 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 cc/config/cfi_exports.map diff --git a/cc/androidmk.go b/cc/androidmk.go index 194faabac..d028f8c79 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -86,6 +86,11 @@ func (c *Module) AndroidMk() android.AndroidMkData { c.subAndroidMk(&ret, c.linker) if c.sanitize != nil { c.subAndroidMk(&ret, c.sanitize) + if Bool(c.sanitize.Properties.Sanitize.Cfi) { + ret.SubName += ".cfi" + } else if Bool(c.sanitize.Properties.Sanitize.Address) { + ret.SubName += ".asan" + } } c.subAndroidMk(&ret, c.installer) diff --git a/cc/cc.go b/cc/cc.go index cdbe43e5a..02aaf1991 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -46,6 +46,9 @@ func init() { ctx.TopDown("asan_deps", sanitizerDepsMutator(asan)) ctx.BottomUp("asan", sanitizerMutator(asan)).Parallel() + ctx.TopDown("cfi_deps", sanitizerDepsMutator(cfi)) + ctx.BottomUp("cfi", sanitizerMutator(cfi)).Parallel() + ctx.TopDown("tsan_deps", sanitizerDepsMutator(tsan)) ctx.BottomUp("tsan", sanitizerMutator(tsan)).Parallel() @@ -437,12 +440,7 @@ func (ctx *moduleContextImpl) toolchain() config.Toolchain { } func (ctx *moduleContextImpl) static() bool { - if static, ok := ctx.mod.linker.(interface { - static() bool - }); ok { - return static.static() - } - return false + return ctx.mod.static() } func (ctx *moduleContextImpl) staticBinary() bool { @@ -1281,6 +1279,15 @@ func (c *Module) Srcs() android.Paths { return android.Paths{} } +func (c *Module) static() bool { + if static, ok := c.linker.(interface { + static() bool + }); ok { + return static.static() + } + return false +} + // // Defaults // diff --git a/cc/config/cfi_exports.map b/cc/config/cfi_exports.map new file mode 100644 index 000000000..3d8f3e0da --- /dev/null +++ b/cc/config/cfi_exports.map @@ -0,0 +1,4 @@ +{ + global: + __cfi_check; +}; diff --git a/cc/library.go b/cc/library.go index 25872c6cb..9d1801747 100644 --- a/cc/library.go +++ b/cc/library.go @@ -513,6 +513,10 @@ func (library *libraryDecorator) linkShared(ctx ModuleContext, if versionScript.Valid() { flags.LdFlags = append(flags.LdFlags, "-Wl,--version-script,"+versionScript.String()) linkerDeps = append(linkerDeps, versionScript.Path()) + if library.sanitize.isSanitizerEnabled(cfi) { + flags.LdFlags = append(flags.LdFlags, "-Wl,--version-script,"+cfiExportsMap.String()) + linkerDeps = append(linkerDeps, cfiExportsMap) + } } if unexportedSymbols.Valid() { ctx.PropertyErrorf("unexported_symbols_list", "Only supported on Darwin") diff --git a/cc/sanitize.go b/cc/sanitize.go index d5535cb68..53de14bd3 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -32,12 +32,14 @@ var ( asanLdflags = []string{"-Wl,-u,__asan_preinit"} asanLibs = []string{"libasan"} - cfiCflags = []string{"-flto", "-fsanitize-cfi-cross-dso", "-fvisibility=default", + cfiCflags = []string{"-flto", "-fsanitize-cfi-cross-dso", "-fsanitize-blacklist=external/compiler-rt/lib/cfi/cfi_blacklist.txt"} // FIXME: revert the __cfi_check flag when clang is updated to r280031. cfiLdflags = []string{"-flto", "-fsanitize-cfi-cross-dso", "-fsanitize=cfi", "-Wl,-plugin-opt,O1 -Wl,-export-dynamic-symbol=__cfi_check"} - cfiArflags = []string{"--plugin ${config.ClangBin}/../lib64/LLVMgold.so"} + cfiArflags = []string{"--plugin ${config.ClangBin}/../lib64/LLVMgold.so"} + cfiExportsMapPath = "build/soong/cc/config/cfi_exports.map" + cfiExportsMap android.Path intOverflowCflags = []string{"-fsanitize-blacklist=build/soong/cc/config/integer_overflow_blacklist.txt"} ) @@ -56,6 +58,7 @@ const ( asan sanitizerType = iota + 1 tsan intOverflow + cfi ) func (t sanitizerType) String() string { @@ -66,6 +69,8 @@ func (t sanitizerType) String() string { return "tsan" case intOverflow: return "intOverflow" + case cfi: + return "cfi" default: panic(fmt.Errorf("unknown sanitizerType %d", t)) } @@ -251,6 +256,8 @@ func (sanitize *sanitize) begin(ctx BaseModuleContext) { ctx.ModuleErrorf(`Use of "coverage" also requires "address"`) } } + + cfiExportsMap = android.PathForSource(ctx, cfiExportsMapPath) } func (sanitize *sanitize) deps(ctx BaseModuleContext, deps Deps) Deps { @@ -362,12 +369,23 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { flags.LdFlags = append(flags.LdFlags, "-march=armv7-a") } sanitizers = append(sanitizers, "cfi") + flags.CFlags = append(flags.CFlags, cfiCflags...) + // Only append the default visibility flag if -fvisibility has not already been set + // to hidden. + if !inList("-fvisibility=hidden", flags.CFlags) { + flags.CFlags = append(flags.CFlags, "-fvisibility=default") + } flags.LdFlags = append(flags.LdFlags, cfiLdflags...) flags.ArFlags = append(flags.ArFlags, cfiArflags...) if Bool(sanitize.Properties.Sanitize.Diag.Cfi) { diagSanitizers = append(diagSanitizers, "cfi") } + + if ctx.staticBinary() { + _, flags.CFlags = removeFromList("-fsanitize-cfi-cross-dso", flags.CFlags) + _, flags.LdFlags = removeFromList("-fsanitize-cfi-cross-dso", flags.LdFlags) + } } if Bool(sanitize.Properties.Sanitize.Integer_overflow) { @@ -451,18 +469,16 @@ func (sanitize *sanitize) inSanitizerDir() bool { return sanitize.Properties.InSanitizerDir } -func (sanitize *sanitize) Sanitizer(t sanitizerType) bool { - if sanitize == nil { - return false - } - +func (sanitize *sanitize) getSanitizerBoolPtr(t sanitizerType) *bool { switch t { case asan: - return Bool(sanitize.Properties.Sanitize.Address) + return sanitize.Properties.Sanitize.Address case tsan: - return Bool(sanitize.Properties.Sanitize.Thread) + return sanitize.Properties.Sanitize.Thread case intOverflow: - return Bool(sanitize.Properties.Sanitize.Integer_overflow) + return sanitize.Properties.Sanitize.Integer_overflow + case cfi: + return sanitize.Properties.Sanitize.Cfi default: panic(fmt.Errorf("unknown sanitizerType %d", t)) } @@ -479,6 +495,9 @@ func (sanitize *sanitize) SetSanitizer(t sanitizerType, b bool) { sanitize.Properties.Sanitize.Thread = boolPtr(b) case intOverflow: sanitize.Properties.Sanitize.Integer_overflow = boolPtr(b) + case cfi: + sanitize.Properties.Sanitize.Cfi = boolPtr(b) + sanitize.Properties.Sanitize.Diag.Cfi = boolPtr(b) default: panic(fmt.Errorf("unknown sanitizerType %d", t)) } @@ -487,40 +506,89 @@ func (sanitize *sanitize) SetSanitizer(t sanitizerType, b bool) { } } +// Check if the sanitizer is explicitly disabled (as opposed to nil by +// virtue of not being set). +func (sanitize *sanitize) isSanitizerExplicitlyDisabled(t sanitizerType) bool { + if sanitize == nil { + return false + } + + sanitizerVal := sanitize.getSanitizerBoolPtr(t) + return sanitizerVal != nil && *sanitizerVal == false +} + +// There isn't an analog of the method above (ie:isSanitizerExplicitlyEnabled) +// because enabling a sanitizer either directly (via the blueprint) or +// indirectly (via a mutator) sets the bool ptr to true, and you can't +// distinguish between the cases. It isn't needed though - both cases can be +// treated identically. +func (sanitize *sanitize) isSanitizerEnabled(t sanitizerType) bool { + if sanitize == nil { + return false + } + + sanitizerVal := sanitize.getSanitizerBoolPtr(t) + return sanitizerVal != nil && *sanitizerVal == true +} + // Propagate asan requirements down from binaries func sanitizerDepsMutator(t sanitizerType) func(android.TopDownMutatorContext) { return func(mctx android.TopDownMutatorContext) { - if c, ok := mctx.Module().(*Module); ok && c.sanitize.Sanitizer(t) { + if c, ok := mctx.Module().(*Module); ok && c.sanitize.isSanitizerEnabled(t) { mctx.VisitDepsDepthFirst(func(module android.Module) { - if d, ok := mctx.Module().(*Module); ok && c.sanitize != nil && - !c.sanitize.Properties.Sanitize.Never { - d.sanitize.Properties.SanitizeDep = true + if d, ok := module.(*Module); ok && d.sanitize != nil && + !d.sanitize.Properties.Sanitize.Never && + !d.sanitize.isSanitizerExplicitlyDisabled(t) { + if (t == cfi && d.static()) || t != cfi { + d.sanitize.Properties.SanitizeDep = true + } } }) } } } -// Create asan variants for modules that need them +// Create sanitized variants for modules that need them func sanitizerMutator(t sanitizerType) func(android.BottomUpMutatorContext) { return func(mctx android.BottomUpMutatorContext) { if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil { - if c.isDependencyRoot() && c.sanitize.Sanitizer(t) { + if c.isDependencyRoot() && c.sanitize.isSanitizerEnabled(t) { modules := mctx.CreateVariations(t.String()) modules[0].(*Module).sanitize.SetSanitizer(t, true) - } else if c.sanitize.Properties.SanitizeDep { + } 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.String()) 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 if mctx.Device() { - modules[1].(*Module).sanitize.Properties.InSanitizerDir = true + // CFI and ASAN are currently mutually exclusive (CFI defers + // to ASAN if you try enabling both), so disable CFI is this + // is an ASAN variant. + if t == asan { + modules[1].(*Module).sanitize.Properties.InSanitizerDir = true + modules[1].(*Module).sanitize.SetSanitizer(cfi, false) + } } else { - modules[0].(*Module).Properties.PreventInstall = true + if isSanitizerEnabled { + modules[0].(*Module).Properties.PreventInstall = true + } else { + modules[1].(*Module).Properties.PreventInstall = true + } } if mctx.AConfig().EmbeddedInMake() { - modules[0].(*Module).Properties.HideFromMake = true + if !mctx.Device() { + if isSanitizerEnabled { + modules[0].(*Module).Properties.HideFromMake = true + } else { + modules[1].(*Module).Properties.HideFromMake = true + } + } } } c.sanitize.Properties.SanitizeDep = false