compiler_wrapper: keep FORTIFY if sanitizer is trivial
For sanitizers like `return`, `builtin`, etc., we have no reason to also
drop FORTIFY checks.
BUG=b:231357370
TEST=`go test`
Change-Id: I1e349a4f6743e549b7bc0899a307b10683bb42e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3651188
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Commit-Queue: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
diff --git a/compiler_wrapper/sanitizer_flags.go b/compiler_wrapper/sanitizer_flags.go
index b9eb055..163b0f4 100644
--- a/compiler_wrapper/sanitizer_flags.go
+++ b/compiler_wrapper/sanitizer_flags.go
@@ -8,38 +8,79 @@
"strings"
)
+// Returns whether the flag turns on 'invasive' sanitizers. These are sanitizers incompatible with
+// things like FORTIFY, since they require meaningful runtime support, intercept libc calls, etc.
+func isInvasiveSanitizerFlag(flag string) bool {
+ // There are a few valid spellings here:
+ // -fsanitize=${sanitizer_list}, which enables the given sanitizers
+ // -fsanitize-trap=${sanitizer_list}, which specifies sanitizer behavior _if_ these
+ // sanitizers are already enabled.
+ // -fsanitize-recover=${sanitizer_list}, which also specifies sanitizer behavior _if_
+ // these sanitizers are already enabled.
+ // -fsanitize-ignorelist=/path/to/file, which designates a config file for sanitizers.
+ //
+ // All we care about is the first one, since that's what actually enables sanitizers. Clang
+ // does not accept a `-fsanitize ${sanitizer_list}` spelling of this flag.
+ fsanitize := "-fsanitize="
+ if !strings.HasPrefix(flag, fsanitize) {
+ return false
+ }
+
+ sanitizers := flag[len(fsanitize):]
+ if sanitizers == "" {
+ return false
+ }
+
+ for _, sanitizer := range strings.Split(sanitizers, ",") {
+ // Keep an allowlist of sanitizers known to not cause issues.
+ switch sanitizer {
+ case "alignment", "array-bounds", "bool", "bounds", "builtin", "enum",
+ "float-cast-overflow", "integer-divide-by-zero", "local-bounds",
+ "nullability", "nullability-arg", "nullability-assign",
+ "nullability-return", "null", "return", "returns-nonnull-attribute",
+ "shift-base", "shift-exponent", "shift", "unreachable", "vla-bound":
+ // These sanitizers are lightweight. Ignore them.
+ default:
+ return true
+ }
+ }
+ return false
+}
+
func processSanitizerFlags(builder *commandBuilder) {
hasSanitizeFlags := false
+ // TODO: This doesn't take -fno-sanitize flags into account. This doesn't seem to be an
+ // issue in practice.
for _, arg := range builder.args {
- // TODO: This should probably be -fsanitize= to not match on
- // e.g. -fsanitize-blocklist
- if arg.fromUser {
- if strings.HasPrefix(arg.value, "-fsanitize") {
- hasSanitizeFlags = true
- }
+ if arg.fromUser && isInvasiveSanitizerFlag(arg.value) {
+ hasSanitizeFlags = true
+ break
}
}
- if hasSanitizeFlags {
- // Flags not supported by sanitizers (ASan etc.)
- unsupportedSanitizerFlags := map[string]bool{
- "-D_FORTIFY_SOURCE=1": true,
- "-D_FORTIFY_SOURCE=2": true,
- "-Wl,--no-undefined": true,
- "-Wl,-z,defs": true,
- }
- builder.transformArgs(func(arg builderArg) string {
- // TODO: This is a bug in the old wrapper to not filter
- // non user args for gcc. Fix this once we don't compare to the old wrapper anymore.
- if (builder.target.compilerType != gccType || arg.fromUser) &&
- unsupportedSanitizerFlags[arg.value] {
- return ""
- }
- return arg.value
- })
-
- builder.filterArgPairs(func(arg1, arg2 builderArg) bool {
- return !(arg1.value == "-Wl,-z" && arg2.value == "-Wl,defs")
- })
+ if !hasSanitizeFlags {
+ return
}
+
+ // Flags not supported by sanitizers (ASan etc.)
+ unsupportedSanitizerFlags := map[string]bool{
+ "-D_FORTIFY_SOURCE=1": true,
+ "-D_FORTIFY_SOURCE=2": true,
+ "-Wl,--no-undefined": true,
+ "-Wl,-z,defs": true,
+ }
+
+ builder.transformArgs(func(arg builderArg) string {
+ // TODO: This is a bug in the old wrapper to not filter
+ // non user args for gcc. Fix this once we don't compare to the old wrapper anymore.
+ if (builder.target.compilerType != gccType || arg.fromUser) &&
+ unsupportedSanitizerFlags[arg.value] {
+ return ""
+ }
+ return arg.value
+ })
+
+ builder.filterArgPairs(func(arg1, arg2 builderArg) bool {
+ return !(arg1.value == "-Wl,-z" && arg2.value == "-Wl,defs")
+ })
}
diff --git a/compiler_wrapper/sanitizer_flags_test.go b/compiler_wrapper/sanitizer_flags_test.go
index 551691f..796961e 100644
--- a/compiler_wrapper/sanitizer_flags_test.go
+++ b/compiler_wrapper/sanitizer_flags_test.go
@@ -8,6 +8,28 @@
"testing"
)
+func TestFortifyIsKeptIfSanitizerIsTrivial(t *testing.T) {
+ withTestContext(t, func(ctx *testContext) {
+ cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+ ctx.newCommand(gccX86_64, "-fsanitize=return", "-D_FORTIFY_SOURCE=1", mainCc)))
+ if err := verifyArgCount(cmd, 1, "-D_FORTIFY_SOURCE=1"); err != nil {
+ t.Error(err)
+ }
+
+ cmd = ctx.must(callCompiler(ctx, ctx.cfg,
+ ctx.newCommand(gccX86_64, "-fsanitize=return,address", "-D_FORTIFY_SOURCE=1", mainCc)))
+ if err := verifyArgCount(cmd, 0, "-D_FORTIFY_SOURCE=1"); err != nil {
+ t.Error(err)
+ }
+
+ cmd = ctx.must(callCompiler(ctx, ctx.cfg,
+ ctx.newCommand(gccX86_64, "-fsanitize=address,return", "-D_FORTIFY_SOURCE=1", mainCc)))
+ if err := verifyArgCount(cmd, 0, "-D_FORTIFY_SOURCE=1"); err != nil {
+ t.Error(err)
+ }
+ })
+}
+
func TestFilterUnsupportedSanitizerFlagsIfSanitizeGiven(t *testing.T) {
withTestContext(t, func(ctx *testContext) {
cmd := ctx.must(callCompiler(ctx, ctx.cfg,
