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

@@ -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)
 

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
 //

cc/config/cfi_exports.map

@@ -0,0 +1,4 @@
+{
+  global:
+    __cfi_check;
+};

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")

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"}
+	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 {
+				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() {
+					// 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 {
+					if isSanitizerEnabled {
 						modules[0].(*Module).Properties.PreventInstall = true
+					} else {
+						modules[1].(*Module).Properties.PreventInstall = true
+					}
 				}
 				if mctx.AConfig().EmbeddedInMake() {
+					if !mctx.Device() {
+						if isSanitizerEnabled {
 							modules[0].(*Module).Properties.HideFromMake = true
+						} else {
+							modules[1].(*Module).Properties.HideFromMake = true
+						}
+					}
 				}
 			}
 			c.sanitize.Properties.SanitizeDep = false