From 4917049f6efe2d08e67eb26ec77e21dcce5172c5 Mon Sep 17 00:00:00 2001 From: Zhizhou Yang Date: Thu, 8 Feb 2018 18:32:11 -0800 Subject: Fix llvm-ar error caused by using lto and sanitizer together LLVM-AR does not allow passing --plugin options more than once. The --plugin ARFLAGS that lto want to add, may already exist if sanitizer is also turned on. Fixed this by adding a new bool Flags.ArGoldPlugin. Set this variable to true whenever LLVM gold plugin is needed for ArFlags. In function TransformObjToStaticLib(), add this option to arFlags using global value ${config.LLVMGoldPlugin} if the bool value is true. Bug: http://b/73160350 Test: build the image with make and succeeded. Change-Id: I62785829b0a4b663225926e4aed98defc1b6da2c --- cc/sanitize.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index ac6cb778..c47c3194 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -38,7 +38,6 @@ 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 cfiStaticLibsMutex sync.Mutex @@ -407,7 +406,7 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { // See b/72706604 or https://github.com/android-ndk/ndk/issues/498. flags.LdFlags = append(flags.LdFlags, "-Wl,-plugin-opt,-emulated-tls") } - flags.ArFlags = append(flags.ArFlags, cfiArflags...) + flags.ArGoldPlugin = true if Bool(sanitize.Properties.Sanitize.Diag.Cfi) { diagSanitizers = append(diagSanitizers, "cfi") } -- cgit v1.2.3 From 954f430e9795a8e4a6d2f43cc356129da46fc0f8 Mon Sep 17 00:00:00 2001 From: Ivan Lozano Date: Wed, 21 Feb 2018 15:49:20 -0800 Subject: Add minimal-runtime support for integer overflows. Adds Soong support for -fsanitze-minimal-runtime when using the integer overflow sanitizers. This makes the crashes due to these sanitizers less mysterious. Bug: 64091660 Test: Compiled and checked the generated compiler commands Test: Checked program that overflows for the abort reason Change-Id: Ieeceaf6c35c8371592952d3b8b977aefc11601c5 Merged-In: Ieeceaf6c35c8371592952d3b8b977aefc11601c5 (cherry picked from commit 30c5db2f47e0305cd50f0cc90047e9ac9c5f676e) --- cc/sanitize.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index c47c3194..535d28ff 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -42,7 +42,8 @@ var ( cfiExportsMap android.Path cfiStaticLibsMutex sync.Mutex - intOverflowCflags = []string{"-fsanitize-blacklist=build/soong/cc/config/integer_overflow_blacklist.txt"} + intOverflowCflags = []string{"-fsanitize-blacklist=build/soong/cc/config/integer_overflow_blacklist.txt"} + minimalRuntimeFlags = []string{"-fsanitize-minimal-runtime", "-fno-sanitize-trap=integer", "-fno-sanitize-recover=integer"} ) type sanitizerType int @@ -112,9 +113,10 @@ type SanitizeProperties struct { Blacklist *string } `android:"arch_variant"` - SanitizerEnabled bool `blueprint:"mutated"` - SanitizeDep bool `blueprint:"mutated"` - InSanitizerDir bool `blueprint:"mutated"` + SanitizerEnabled bool `blueprint:"mutated"` + SanitizeDep bool `blueprint:"mutated"` + MinimalRuntimeDep bool `blueprint:"mutated"` + InSanitizerDir bool `blueprint:"mutated"` } type sanitize struct { @@ -301,6 +303,11 @@ func (sanitize *sanitize) deps(ctx BaseModuleContext, deps Deps) Deps { } func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { + minimalRuntimePath := "${config.ClangAsanLibDir}/" + config.UndefinedBehaviorSanitizerMinimalRuntimeLibrary(ctx.toolchain()) + ".a" + + if ctx.Device() && sanitize.Properties.MinimalRuntimeDep { + flags.LdFlags = append(flags.LdFlags, minimalRuntimePath) + } if !sanitize.Properties.SanitizerEnabled { return flags } @@ -431,6 +438,7 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { if len(sanitizers) > 0 { sanitizeArg := "-fsanitize=" + strings.Join(sanitizers, ",") + flags.CFlags = append(flags.CFlags, sanitizeArg) if ctx.Host() { flags.CFlags = append(flags.CFlags, "-fno-sanitize-recover=all") @@ -440,6 +448,11 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { _, flags.LdFlags = removeFromList("-Wl,--no-undefined", flags.LdFlags) } else { flags.CFlags = append(flags.CFlags, "-fsanitize-trap=all", "-ftrap-function=abort") + + if enableMinimalRuntime(sanitize) { + flags.CFlags = append(flags.CFlags, strings.Join(minimalRuntimeFlags, " ")) + flags.libFlags = append([]string{minimalRuntimePath}, flags.libFlags...) + } } } @@ -589,6 +602,24 @@ func sanitizerDepsMutator(t sanitizerType) func(android.TopDownMutatorContext) { } } +// Propagate the ubsan minimal runtime dependency when there are integer overflow sanitized static dependencies. +func minimalRuntimeDepsMutator() func(android.TopDownMutatorContext) { + return func(mctx android.TopDownMutatorContext) { + if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil { + mctx.VisitDepsDepthFirst(func(module android.Module) { + if d, ok := module.(*Module); ok && d.static() && d.sanitize != nil { + + // If a static dependency will be built with the minimal runtime, + // make sure we include the ubsan minimal runtime. + if enableMinimalRuntime(d.sanitize) { + c.sanitize.Properties.MinimalRuntimeDep = true + } + } + }) + } + } +} + // Create sanitized variants for modules that need them func sanitizerMutator(t sanitizerType) func(android.BottomUpMutatorContext) { return func(mctx android.BottomUpMutatorContext) { @@ -659,6 +690,18 @@ func cfiStaticLibs(config android.Config) *[]string { }).(*[]string) } +func enableMinimalRuntime(sanitize *sanitize) bool { + if !Bool(sanitize.Properties.Sanitize.Address) && + (Bool(sanitize.Properties.Sanitize.Integer_overflow) || + len(sanitize.Properties.Sanitize.Misc_undefined) > 0) && + !(Bool(sanitize.Properties.Sanitize.Diag.Integer_overflow) || + Bool(sanitize.Properties.Sanitize.Diag.Cfi) || + len(sanitize.Properties.Sanitize.Diag.Misc_undefined) > 0) { + return true + } + return false +} + func cfiMakeVarsProvider(ctx android.MakeVarsContext) { cfiStaticLibs := cfiStaticLibs(ctx.Config()) sort.Strings(*cfiStaticLibs) -- cgit v1.2.3 From fe6edc9a70c9fa4551f4e793f89f380529f084cf Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 28 Mar 2018 14:50:27 -0700 Subject: Fix data race on cfiExportsMap cfiExportsMap was reinitialized for every module, which caused data races. Create the path from the string on each use instead. Bug: 77234104 Test: m nothing with race detector turned on Change-Id: Ibca3149dcbe8a9d4d9f7ec6dd0b164697e7ae5cd Merged-In: Ibca3149dcbe8a9d4d9f7ec6dd0b164697e7ae5cd (cherry picked from commit 1218a19f0d2217500a1efa5fffde7465df2e4419) --- cc/sanitize.go | 3 --- 1 file changed, 3 deletions(-) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index c47c3194..0bcbb125 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -39,7 +39,6 @@ var ( cfiLdflags = []string{"-flto", "-fsanitize-cfi-cross-dso", "-fsanitize=cfi", "-Wl,-plugin-opt,O1"} 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"} @@ -282,8 +281,6 @@ 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 { -- cgit v1.2.3 From f267f715ebcde8f0dec9bfe168dc8c5dcfb93bc7 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Thu, 10 May 2018 15:29:24 -0700 Subject: Allow abi diffs sanitized variants of vndk libraries on production devices. Previously abi diffs were allowed only on unsanitized variants of vndk libraries. This CL allows them on all sanitized variants which go onto production devices, eg: cfi variants. Bug: 66301104 Test: Without this change, for arm64 libstagefright_foundation doesn't get an lsdump file since we don't build an unsanitized variant (aosp_arm64_ab). Test: With this change, for arm64 libstagefright_foundation does get an lsdump file (aosp_arm64_ab) Merged-In: I94f82fd84fc898e4980c3f3619df9677ed723c32 Change-Id: I94f82fd84fc898e4980c3f3619df9677ed723c32 (cherry picked from commit b7e08ca83000f14653ffdd0bc4195067bb902dfc) --- cc/sanitize.go | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index ee549bc0..86537347 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -535,6 +535,11 @@ func (sanitize *sanitize) isUnsanitizedVariant() bool { !sanitize.isSanitizerEnabled(cfi) } +func (sanitize *sanitize) isVariantOnProductionDevice() bool { + return !sanitize.isSanitizerEnabled(asan) && + !sanitize.isSanitizerEnabled(tsan) +} + func (sanitize *sanitize) SetSanitizer(t sanitizerType, b bool) { switch t { case asan: -- cgit v1.2.3 From 1838ef9be44938a476a6ecaff153b338e28f8e35 Mon Sep 17 00:00:00 2001 From: Ivan Lozano Date: Thu, 10 May 2018 14:17:22 -0700 Subject: Don't export UBSan minimal runtime symbols. When linking in the UBSan minimal runtime, don't export the symbols. This was resulting in an edge case where symbols were sometimes undefined at runtime on address sanitized builds if static library dependencies were integer overflow sanitized. Bug: 78766744 Test: readelf on libraries show either inclusion of the shared library or no undefined symbols related to the minimal runtime. Change-Id: Ide85c8c6b53b400ce9166ccaf96d250797fe4b24 Merged-In: Ide85c8c6b53b400ce9166ccaf96d250797fe4b24 (cherry picked from commit 59fdea2ac2ded9190eaa9ce81252cd809a2985cb) --- cc/sanitize.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index ee549bc0..23a7be44 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -300,10 +300,12 @@ func (sanitize *sanitize) deps(ctx BaseModuleContext, deps Deps) Deps { } func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { - minimalRuntimePath := "${config.ClangAsanLibDir}/" + config.UndefinedBehaviorSanitizerMinimalRuntimeLibrary(ctx.toolchain()) + ".a" + minimalRuntimeLib := config.UndefinedBehaviorSanitizerMinimalRuntimeLibrary(ctx.toolchain()) + ".a" + minimalRuntimePath := "${config.ClangAsanLibDir}/" + minimalRuntimeLib if ctx.Device() && sanitize.Properties.MinimalRuntimeDep { flags.LdFlags = append(flags.LdFlags, minimalRuntimePath) + flags.LdFlags = append(flags.LdFlags, "-Wl,--exclude-libs,"+minimalRuntimeLib) } if !sanitize.Properties.SanitizerEnabled { return flags @@ -449,6 +451,7 @@ func (sanitize *sanitize) flags(ctx ModuleContext, flags Flags) Flags { if enableMinimalRuntime(sanitize) { flags.CFlags = append(flags.CFlags, strings.Join(minimalRuntimeFlags, " ")) flags.libFlags = append([]string{minimalRuntimePath}, flags.libFlags...) + flags.LdFlags = append(flags.LdFlags, "-Wl,--exclude-libs,"+minimalRuntimeLib) } } } -- cgit v1.2.3 From 7589c82eecd4c25279f64464609708ff8400dcb0 Mon Sep 17 00:00:00 2001 From: Vishwath Mohan Date: Wed, 23 May 2018 19:29:55 -0700 Subject: Disable CFI for vendor variants of VNDK libraries Enabling CFI changes the VNDK ABI from the frozen snapshot, so the only solution for now is to temporarily disable CFI on all vendor variants for the sake of compatibility. Bug: 66301104 Test: ABI is consistent. Change-Id: Ie7dca41e0f647808b08adede09a30f3c746e6bda --- cc/sanitize.go | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index fc44eaf1..205b2a20 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -256,6 +256,13 @@ func (sanitize *sanitize) begin(ctx BaseModuleContext) { s.Diag.Cfi = nil } + // Also disable CFI for VNDK variants of components in the + // include paths + if ctx.isVndk() && ctx.useVndk() && ctx.Config().CFIEnabledForPath(ctx.ModuleDir()) { + s.Cfi = nil + s.Diag.Cfi = nil + } + if ctx.staticBinary() { s.Address = nil s.Coverage = nil -- cgit v1.2.3 From 1c54f66bd56d00592b6ce55826411ae6af9e2697 Mon Sep 17 00:00:00 2001 From: Vishwath Mohan Date: Thu, 24 May 2018 18:36:18 -0700 Subject: Restrict CFI_INCLUDE_PATHS to Arm64 This CL restricts CFI_INCLUDE_PATHS and PRODUCT_CFI_INCLUDE_PATHS to Arm64 only. Bug: 66301104 Test: x86 targets do not respect the include paths variables Change-Id: I66ec2fb05435535aaf5d59fdfc7a170a4fdd4f26 --- cc/sanitize.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index 205b2a20..881a5a01 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -224,8 +224,8 @@ func (sanitize *sanitize) begin(ctx BaseModuleContext) { } } - // Enable CFI for all components in the include paths - if s.Cfi == nil && ctx.Config().CFIEnabledForPath(ctx.ModuleDir()) { + // Enable CFI for all components in the include paths (for Aarch64 only) + if s.Cfi == nil && ctx.Config().CFIEnabledForPath(ctx.ModuleDir()) && ctx.Arch().ArchType == android.Arm64 { s.Cfi = boolPtr(true) if inList("cfi", ctx.Config().SanitizeDeviceDiag()) { s.Diag.Cfi = boolPtr(true) -- cgit v1.2.3 From 9ccbba0200cf56cc6b1b09bc45fa2efb1ea9a411 Mon Sep 17 00:00:00 2001 From: Vishwath Mohan Date: Mon, 28 May 2018 13:54:48 -0700 Subject: Disable CFI for vendor variants of all components The current VNDK ABI snapshot expects that CFI is disabled for components that either in the include paths (from cfi-common.mk) OR enabled directly in the makefile/blueprint. However, on non-arm64 architectures, the build system was only disabling CFI for vendor components in the include paths. This topic corrects it by (a) removing the include paths check to disable CFI for vendor variants (this CL), and (b) inheriting the CFI include paths in all product configs to ensure that when we update the ABI we're able to move to relying exclusively on (PRODUCT_)CFI_INCLUDE_PATHS. Bug: 66301104 Test: ABI matches for all architectures. Change-Id: I648edf13346b18fd88b623682e8590ed44709e0d --- cc/sanitize.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'cc/sanitize.go') diff --git a/cc/sanitize.go b/cc/sanitize.go index 881a5a01..de970352 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -256,9 +256,8 @@ func (sanitize *sanitize) begin(ctx BaseModuleContext) { s.Diag.Cfi = nil } - // Also disable CFI for VNDK variants of components in the - // include paths - if ctx.isVndk() && ctx.useVndk() && ctx.Config().CFIEnabledForPath(ctx.ModuleDir()) { + // Also disable CFI for VNDK variants of components + if ctx.isVndk() && ctx.useVndk() { s.Cfi = nil s.Diag.Cfi = nil } -- cgit v1.2.3