[Driver] -fsanitize=undefined: don't expand to signed-integer-overflow if -fwrapv

[MaskRay]

Linux kernel uses -fwrapv to change signed integer overflows from
undefined behaviors to defined behaviors. However, the security folks
still want -fsanitize=signed-integer-overflow diagnostics. Their
intention can be expressed with -fwrapv
-fsanitize=signed-integer-overflow (#80089). This mode by default
reports recoverable errors while still making signed integer overflows
defined (most UBSan checks are recoverable by default: you get errors in
stderr, but the program is not halted).

-fsanitize=undefined -fwrapv users likely want to suppress
signed-integer-overflow, unless signed-integer-overflow is explicitly
enabled. Implement this suppression.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Fangrui Song (MaskRay)

Changes

Linux kernel uses -fwrapv to change signed integer overflows from
undefined behaviors to defined behaviors. However, the security folks
still want -fsanitize=signed-integer-overflow diagnostics. Their
intention can be expressed with -fwrapv
-fsanitize=signed-integer-overflow (#80089). This mode by default
reports recoverable errors while still making signed integer overflows
defined (most UBSan checks are recoverable by default: you get errors in
stderr, but the program is not halted).

-fsanitize=undefined -fwrapv users likely want to suppress
signed-integer-overflow, unless signed-integer-overflow is explicitly
enabled. Implement this suppression.

---

Full diff: https://github.com/llvm/llvm-project/pull/85501.diff

2 Files Affected:

• (modified) clang/lib/Driver/SanitizerArgs.cpp (+8)
• (added) clang/test/Driver/fsanitize-signed-integer-overflow.c (+28)

diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 56d497eb4c32b8..8bfe9f02a091d1 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -487,6 +487,14 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
         Add &= ~NotAllowedWithExecuteOnly;
       if (CfiCrossDso)
         Add &= ~SanitizerKind::CFIMFCall;
+      // -fsanitize=undefined does not expand to signed-integer-overflow in
+      // -fwrapv (implied by -fno-strict-overflow) mode.
+      if (Add & SanitizerKind::UndefinedGroup) {
+        bool S = Args.hasFlagNoClaim(options::OPT_fno_strict_overflow,
+                                     options::OPT_fstrict_overflow, false);
+        if (Args.hasFlagNoClaim(options::OPT_fwrapv, options::OPT_fno_wrapv, S))
+          Add &= ~SanitizerKind::SignedIntegerOverflow;
+      }
       Add &= Supported;
 
       if (Add & SanitizerKind::Fuzzer)
diff --git a/clang/test/Driver/fsanitize-signed-integer-overflow.c b/clang/test/Driver/fsanitize-signed-integer-overflow.c
new file mode 100644
index 00000000000000..4a9345493b6aa8
--- /dev/null
+++ b/clang/test/Driver/fsanitize-signed-integer-overflow.c
@@ -0,0 +1,28 @@
+/// When -fwrapv (implied by -fno-strict-overflow) is enabled,
+/// -fsanitize=undefined does not expand to signed-integer-overflow.
+/// -fsanitize=signed-integer-overflow is unaffected by -fwrapv.
+
+// RUN: %clang -### --target=x86_64-linux -fwrapv -fsanitize=signed-integer-overflow %s 2>&1 | FileCheck %s
+// CHECK: -fsanitize=signed-integer-overflow
+// CHECK: -fsanitize-recover=signed-integer-overflow
+
+// RUN: %clang -### --target=x86_64-linux -fno-strict-overflow -fsanitize=undefined %s 2>&1 | FileCheck %s --check-prefix=EXCLUDE
+// RUN: %clang -### --target=x86_64-linux -fstrict-overflow -fwrapv -fsanitize=undefined %s 2>&1 | FileCheck %s --check-prefix=EXCLUDE
+// EXCLUDE:     -fsanitize=alignment,array-bounds,
+// EXCLUDE-NOT: signed-integer-overflow,
+// EXCLUDE:      -fsanitize-recover=alignment,array-bounds,
+// EXCLUDE-SAME: signed-integer-overflow
+
+// RUN: %clang -### --target=x86_64-linux -fwrapv -fsanitize=undefined -fsanitize=signed-integer-overflow %s 2>&1 | FileCheck %s --check-prefix=INCLUDE
+// RUN: %clang -### --target=x86_64-linux -fno-strict-overflow -fno-sanitize=signed-integer-overflow -fsanitize=undefined -fsanitize=signed-integer-overflow %s 2>&1 | FileCheck %s --check-prefix=INCLUDE
+// INCLUDE:      -fsanitize=alignment,array-bounds,
+// INCLUDE-SAME: signed-integer-overflow
+// INCLUDE:      -fsanitize-recover=alignment,array-bounds,
+// INCLUDE-SAME: signed-integer-overflow
+
+/// -fsanitize-trap=undefined expands to signed-integer-overflow regardless of -fwrapv.
+// RUN: %clang -### --target=x86_64-linux -fwrapv -fsanitize=undefined -fsanitize=signed-integer-overflow -fsanitize-trap=undefined %s 2>&1 | FileCheck %s --check-prefix=INCLUDE-TRAP
+// INCLUDE-TRAP:      -fsanitize=alignment,array-bounds,
+// INCLUDE-TRAP-SAME: signed-integer-overflow
+// INCLUDE-TRAP:      -fsanitize-trap=alignment,array-bounds,
+// INCLUDE-TRAP-SAME: signed-integer-overflow

[nickdesaulniers]

What's up with the commit message on this PR? Why is it so different from the PR description?

[MaskRay]

"Squash and merge" uses the description instead of the commit message, so the commit message is not useful. The commit message is due to spr.