From e712879d5dbdb06da48cf054f0a9c5d582a8387e Mon Sep 17 00:00:00 2001 From: Vishwath Mohan Date: Fri, 17 Nov 2017 11:08:10 -0800 Subject: [PATCH] Reduce how often both mutated variants are needed. This CL rolls back how often we bubble up both sanitized and un-sanitized variants of a component. With this change only CFI-enabled target static libraries will do this, all other cases suppress one of the two variants (both from being installed and from being exposed to Make for make-embedded builds). This means we shouldn't need a separate sanitizer suffix for ASAN at all (.asan), and similarly for non static-lib CFI components (.cfi), so this CL changes that as well. Lastly, because the version of ar meant for the host is not built with plugin support (which CFI requires), this CL disables CFI for host targets. This CL should fix the following 2 issues: (1) Removing warnings about multiple rules existing for the same installable target. (2) Fixing VTS packaging, which had been broken by the generation of the .asan suffix. Bug: 69172424, 69059192, 67507323 Test: m -j40 # Soong generated .mk file does not have duplicate rules. Test: SANITIZE_TARGET="address" m -j40 libstagefright # installed correctly. Change-Id: Ib90fdbc8a6ad3924fc2a691b7277a8a1bc67cda8 --- cc/sanitize.go | 85 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/cc/sanitize.go b/cc/sanitize.go index 70aa41238..fcea4efb0 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -18,6 +18,7 @@ import ( "fmt" "io" "strings" + "sync" "android/soong/android" "android/soong/cc/config" @@ -36,9 +37,10 @@ var ( "-fsanitize-blacklist=external/compiler-rt/lib/cfi/cfi_blacklist.txt"} cfiLdflags = []string{"-flto", "-fsanitize-cfi-cross-dso", "-fsanitize=cfi", "-Wl,-plugin-opt,O1"} - cfiArflags = []string{"--plugin ${config.ClangBin}/../lib64/LLVMgold.so"} - cfiExportsMapPath = "build/soong/cc/config/cfi_exports.map" - cfiExportsMap android.Path + cfiArflags = []string{"--plugin ${config.ClangBin}/../lib64/LLVMgold.so"} + cfiExportsMapPath = "build/soong/cc/config/cfi_exports.map" + cfiExportsMap android.Path + cfiStaticLibsMutex sync.Mutex intOverflowCflags = []string{"-fsanitize-blacklist=build/soong/cc/config/integer_overflow_blacklist.txt"} ) @@ -122,6 +124,10 @@ type sanitize struct { androidMkRuntimeLibrary string } +func init() { + android.RegisterMakeVarsProvider(pctx, cfiMakeVarsProvider) +} + func (sanitize *sanitize) props() []interface{} { return []interface{}{&sanitize.Properties} } @@ -243,6 +249,12 @@ func (sanitize *sanitize) begin(ctx BaseModuleContext) { s.Diag.Cfi = nil } + // Also disable CFI for host builds. + if ctx.Host() { + s.Cfi = nil + s.Diag.Cfi = nil + } + if ctx.staticBinary() { s.Address = nil s.Coverage = nil @@ -474,12 +486,11 @@ func (sanitize *sanitize) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMk fmt.Fprintln(w, "LOCAL_SHARED_LIBRARIES += "+sanitize.androidMkRuntimeLibrary) } }) - if ctx.Target().Os.Class == android.Device { - if Bool(sanitize.Properties.Sanitize.Cfi) { - ret.SubName += ".cfi" - } else if Bool(sanitize.Properties.Sanitize.Address) { - ret.SubName += ".asan" - } + + // 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" } } @@ -584,27 +595,44 @@ func sanitizerMutator(t sanitizerType) func(android.BottomUpMutatorContext) { modules[0].(*Module).sanitize.Properties.SanitizeDep = false modules[1].(*Module).sanitize.Properties.SanitizeDep = false - if mctx.Device() { - // CFI and ASAN are currently mutually exclusive so disable - // CFI if this is an ASAN variant. - if t == asan { + + // 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.AConfig()) + + 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) } - } else { - if mctx.AConfig().EmbeddedInMake() { - if isSanitizerEnabled { - modules[0].(*Module).Properties.HideFromMake = true - } else { - modules[1].(*Module).Properties.HideFromMake = true - } - } - } - if !mctx.AConfig().EmbeddedInMake() || !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 } } } @@ -612,3 +640,14 @@ func sanitizerMutator(t sanitizerType) func(android.BottomUpMutatorContext) { } } } + +func cfiStaticLibs(config android.Config) *[]string { + return config.Once("cfiStaticLibs", func() interface{} { + return &[]string{} + }).(*[]string) +} + +func cfiMakeVarsProvider(ctx android.MakeVarsContext) { + cfiStaticLibs := cfiStaticLibs(ctx.Config()) + ctx.Strict("SOONG_CFI_STATIC_LIBRARIES", strings.Join(*cfiStaticLibs, " ")) +}