[clang] 81b4b89 - [Sanitizer] Support -fwrapv with -fsanitize=signed-integer-overflow (#82432)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 21 13:00:11 PST 2024
Author: Justin Stitt
Date: 2024-02-21T21:00:08Z
New Revision: 81b4b89197a6be5f19f907b558540bb3cb70f064
URL: https://github.com/llvm/llvm-project/commit/81b4b89197a6be5f19f907b558540bb3cb70f064
DIFF: https://github.com/llvm/llvm-project/commit/81b4b89197a6be5f19f907b558540bb3cb70f064.diff
LOG: [Sanitizer] Support -fwrapv with -fsanitize=signed-integer-overflow (#82432)
Clang has a `signed-integer-overflow` sanitizer to catch arithmetic
overflow; however, most of its instrumentation [fails to
apply](https://godbolt.org/z/ee41rE8o6) when `-fwrapv` is enabled; this
is by design.
The Linux kernel enables `-fno-strict-overflow` which implies `-fwrapv`.
This means we are [currently unable to detect signed-integer
wrap-around](https://github.com/KSPP/linux/issues/26). All the while,
the root cause of many security vulnerabilities in the Linux kernel is
[arithmetic overflow](https://cwe.mitre.org/data/definitions/190.html).
To work around this and enhance the functionality of
`-fsanitize=signed-integer-overflow`, we instrument signed arithmetic
even if the signed overflow behavior is defined.
Co-authored-by: Justin Stitt <justinstitt at google.com>
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/docs/UndefinedBehaviorSanitizer.rst
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/integer-overflow.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dd217e16f1f1ae..ef2d9b8e46ae4e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -408,6 +408,14 @@ Moved checkers
Sanitizers
----------
+- ``-fsanitize=signed-integer-overflow`` now instruments signed arithmetic even
+ when ``-fwrapv`` is enabled. Previously, only division checks were enabled.
+
+ Users with ``-fwrapv`` as well as a sanitizer group like
+ ``-fsanitize=undefined`` or ``-fsanitize=integer`` enabled may want to
+ manually disable potentially noisy signed integer overflow checks with
+ ``-fno-sanitize=signed-integer-overflow``
+
Python Binding Changes
----------------------
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index b8ad3804f18903..8f58c92bd2a163 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -190,10 +190,11 @@ Available checks are:
- ``-fsanitize=signed-integer-overflow``: Signed integer overflow, where the
result of a signed integer computation cannot be represented in its type.
This includes all the checks covered by ``-ftrapv``, as well as checks for
- signed division overflow (``INT_MIN/-1``), but not checks for
- lossy implicit conversions performed before the computation
- (see ``-fsanitize=implicit-conversion``). Both of these two issues are
- handled by ``-fsanitize=implicit-conversion`` group of checks.
+ signed division overflow (``INT_MIN/-1``). Note that checks are still
+ added even when ``-fwrapv`` is enabled. This sanitizer does not check for
+ lossy implicit conversions performed before the computation (see
+ ``-fsanitize=implicit-conversion``). Both of these two issues are handled
+ by ``-fsanitize=implicit-conversion`` group of checks.
- ``-fsanitize=unreachable``: If control flow reaches an unreachable
program point.
- ``-fsanitize=unsigned-integer-overflow``: Unsigned integer overflow, where
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 576734e460b9c1..10b74575220443 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -723,7 +723,9 @@ class ScalarExprEmitter
if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
- return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
+ if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
+ return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
+ [[fallthrough]];
case LangOptions::SOB_Undefined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
@@ -2568,7 +2570,9 @@ llvm::Value *ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior(
StringRef Name = IsInc ? "inc" : "dec";
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
- return Builder.CreateAdd(InVal, Amount, Name);
+ if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
+ return Builder.CreateAdd(InVal, Amount, Name);
+ [[fallthrough]];
case LangOptions::SOB_Undefined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
return Builder.CreateNSWAdd(InVal, Amount, Name);
@@ -3913,7 +3917,9 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
if (op.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
- return Builder.CreateAdd(op.LHS, op.RHS, "add");
+ if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
+ return Builder.CreateAdd(op.LHS, op.RHS, "add");
+ [[fallthrough]];
case LangOptions::SOB_Undefined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
return Builder.CreateNSWAdd(op.LHS, op.RHS, "add");
@@ -4067,7 +4073,9 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
if (op.Ty->isSignedIntegerOrEnumerationType()) {
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
- return Builder.CreateSub(op.LHS, op.RHS, "sub");
+ if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
+ return Builder.CreateSub(op.LHS, op.RHS, "sub");
+ [[fallthrough]];
case LangOptions::SOB_Undefined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
return Builder.CreateNSWSub(op.LHS, op.RHS, "sub");
diff --git a/clang/test/CodeGen/integer-overflow.c b/clang/test/CodeGen/integer-overflow.c
index 9a3107c0b52926..461b026d39615b 100644
--- a/clang/test/CodeGen/integer-overflow.c
+++ b/clang/test/CodeGen/integer-overflow.c
@@ -1,7 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s --check-prefix=DEFAULT
// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -fwrapv | FileCheck %s --check-prefix=WRAPV
// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -ftrapv | FileCheck %s --check-prefix=TRAPV
-// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -fsanitize=signed-integer-overflow | FileCheck %s --check-prefix=CATCH_UB
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -fsanitize=signed-integer-overflow | FileCheck %s --check-prefixes=CATCH_UB,CATCH_UB_POINTER
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -fsanitize=signed-integer-overflow -fwrapv | FileCheck %s --check-prefixes=CATCH_UB,NOCATCH_UB_POINTER
// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -ftrapv -ftrapv-handler foo | FileCheck %s --check-prefix=TRAPV_HANDLER
@@ -62,7 +63,8 @@ void test1(void) {
// DEFAULT: getelementptr inbounds i32, ptr
// WRAPV: getelementptr i32, ptr
// TRAPV: getelementptr inbounds i32, ptr
- // CATCH_UB: getelementptr inbounds i32, ptr
+ // CATCH_UB_POINTER: getelementptr inbounds i32, ptr
+ // NOCATCH_UB_POINTER: getelementptr i32, ptr
// PR9350: char pre-increment never overflows.
extern volatile signed char PR9350_char_inc;
More information about the cfe-commits
mailing list