Revert "Revert "Build system changes for CFI (Soong)""

This reverts commit 33c252c2f1.

I have a fix to the crashes that this CL set was causing, and have
uploaded it as a patchset to this revert.

This also contains a partial fix that was initially submitted
separately as
https://android-review.googlesource.com/#/c/platform/build/soong/+/524295/

Bug: 30227045
Test: ./art/test/testrunner/run_build_test_target.py -j110 art-asan #no
build errors
Test: m -j50 ASAN_OPTIONS=detect_leaks=0 SANITIZE_HOST=address # no
build errors

Change-Id: I3e53549fa03413d35d9a952f04de1e7629e1f06d

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 {
+	return ctx.mod.static()
-		static() bool
-	}); ok {
-		return static.static()
-	}
-	return false
 }
 
 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"}
+	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) {
@@ -445,24 +463,30 @@ 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"
+		}
+	}
+
 }
 
 func (sanitize *sanitize) inSanitizerDir() bool {
 	return sanitize.Properties.InSanitizerDir
 }
 
-func (sanitize *sanitize) Sanitizer(t sanitizerType) bool {
+func (sanitize *sanitize) getSanitizerBoolPtr(t sanitizerType) *bool {
-	if sanitize == nil {
-		return false
-	}
-
 	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 +503,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 +514,87 @@ 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 &&
+				if d, ok := module.(*Module); ok && d.sanitize != nil &&
-					!c.sanitize.Properties.Sanitize.Never {
+					!d.sanitize.Properties.Sanitize.Never &&
-					d.sanitize.Properties.SanitizeDep = true
+					!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 so disable
+					// CFI if 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
-				if mctx.AConfig().EmbeddedInMake() {
+					} else {
-					modules[0].(*Module).Properties.HideFromMake = true
+						modules[1].(*Module).Properties.PreventInstall = true
+					}
+
+					if mctx.AConfig().EmbeddedInMake() {
+						if isSanitizerEnabled {
+							modules[0].(*Module).Properties.HideFromMake = true
+						} else {
+							modules[1].(*Module).Properties.HideFromMake = true
+						}
+					}
 				}
 			}
 			c.sanitize.Properties.SanitizeDep = false