[clang] Sanitizer: Support -fwrapv with -fsanitize=signed-integer-overflow (PR #82432)
Justin Stitt via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 20 14:49:39 PST 2024
https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/82432
>From b02b09b9eb4f9a8ac60dd077d95c67b959db3b70 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 20 Feb 2024 22:21:02 +0000
Subject: [PATCH 1/3] support fwrapv with signed int overflow sanitizer
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/docs/UndefinedBehaviorSanitizer.rst | 9 +++++----
clang/lib/CodeGen/CGExprScalar.cpp | 16 ++++++++++++----
clang/test/CodeGen/integer-overflow.c | 12 ++++++++++++
4 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5bca2c965c866b..685b19cabeb82c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -399,6 +399,9 @@ Moved checkers
Sanitizers
----------
+- ``-fsanitize=signed-integer-overflow`` now instruments signed arithmetic even
+ when ``-fwrapv`` is enabled. Previously, only division checks were enabled.
+
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..7621d9bcdec991 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..b6d1a9fc4cf744 100644
--- a/clang/test/CodeGen/integer-overflow.c
+++ b/clang/test/CodeGen/integer-overflow.c
@@ -2,6 +2,7 @@
// 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 -fwrapv | FileCheck %s --check-prefix=CATCH_WRAP
// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - -ftrapv -ftrapv-handler foo | FileCheck %s --check-prefix=TRAPV_HANDLER
@@ -16,6 +17,7 @@ void test1(void) {
// WRAPV: add i32
// TRAPV: llvm.sadd.with.overflow.i32
// CATCH_UB: llvm.sadd.with.overflow.i32
+ // CATCH_WRAP: llvm.sadd.with.overflow.i32
// TRAPV_HANDLER: foo(
f11G = a + b;
@@ -23,6 +25,7 @@ void test1(void) {
// WRAPV: sub i32
// TRAPV: llvm.ssub.with.overflow.i32
// CATCH_UB: llvm.ssub.with.overflow.i32
+ // CATCH_WRAP: llvm.ssub.with.overflow.i32
// TRAPV_HANDLER: foo(
f11G = a - b;
@@ -30,6 +33,7 @@ void test1(void) {
// WRAPV: mul i32
// TRAPV: llvm.smul.with.overflow.i32
// CATCH_UB: llvm.smul.with.overflow.i32
+ // CATCH_WRAP: llvm.smul.with.overflow.i32
// TRAPV_HANDLER: foo(
f11G = a * b;
@@ -37,6 +41,7 @@ void test1(void) {
// WRAPV: sub i32 0,
// TRAPV: llvm.ssub.with.overflow.i32(i32 0
// CATCH_UB: llvm.ssub.with.overflow.i32(i32 0
+ // CATCH_WRAP: llvm.ssub.with.overflow.i32(i32 0
// TRAPV_HANDLER: foo(
f11G = -a;
@@ -46,6 +51,7 @@ void test1(void) {
// WRAPV: add i32 {{.*}}, 1
// TRAPV: llvm.sadd.with.overflow.i32({{.*}}, i32 1)
// CATCH_UB: llvm.sadd.with.overflow.i32({{.*}}, i32 1)
+ // CATCH_WRAP: llvm.sadd.with.overflow.i32({{.*}}, i32 1)
// TRAPV_HANDLER: foo(
++a;
@@ -53,6 +59,7 @@ void test1(void) {
// WRAPV: add i32 {{.*}}, -1
// TRAPV: llvm.ssub.with.overflow.i32({{.*}}, i32 1)
// CATCH_UB: llvm.ssub.with.overflow.i32({{.*}}, i32 1)
+ // CATCH_WRAP: llvm.ssub.with.overflow.i32({{.*}}, i32 1)
// TRAPV_HANDLER: foo(
--a;
@@ -70,6 +77,7 @@ void test1(void) {
// WRAPV: add i8 {{.*}}, 1
// TRAPV: add i8 {{.*}}, 1
// CATCH_UB: add i8 {{.*}}, 1
+ // CATCH_WRAP: add i8 {{.*}}, 1
++PR9350_char_inc;
// PR9350: char pre-decrement never overflows.
@@ -78,6 +86,7 @@ void test1(void) {
// WRAPV: add i8 {{.*}}, -1
// TRAPV: add i8 {{.*}}, -1
// CATCH_UB: add i8 {{.*}}, -1
+ // CATCH_WRAP: add i8 {{.*}}, -1
--PR9350_char_dec;
// PR9350: short pre-increment never overflows.
@@ -86,6 +95,7 @@ void test1(void) {
// WRAPV: add i16 {{.*}}, 1
// TRAPV: add i16 {{.*}}, 1
// CATCH_UB: add i16 {{.*}}, 1
+ // CATCH_WRAP: add i16 {{.*}}, 1
++PR9350_short_inc;
// PR9350: short pre-decrement never overflows.
@@ -94,6 +104,7 @@ void test1(void) {
// WRAPV: add i16 {{.*}}, -1
// TRAPV: add i16 {{.*}}, -1
// CATCH_UB: add i16 {{.*}}, -1
+ // CATCH_WRAP: add i16 {{.*}}, -1
--PR9350_short_dec;
// PR24256: don't instrument __builtin_frame_address.
@@ -102,4 +113,5 @@ void test1(void) {
// WRAPV: call ptr @llvm.frameaddress.p0(i32 0)
// TRAPV: call ptr @llvm.frameaddress.p0(i32 0)
// CATCH_UB: call ptr @llvm.frameaddress.p0(i32 0)
+ // CATCH_WRAP: call ptr @llvm.frameaddress.p0(i32 0)
}
>From e5e92e6c07a9fbbac698a3b6bb4422f26ea06583 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 20 Feb 2024 22:31:41 +0000
Subject: [PATCH 2/3] run git clang-format
---
clang/lib/CodeGen/CGExprScalar.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 7621d9bcdec991..10b74575220443 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -724,7 +724,7 @@ class ScalarExprEmitter
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
- return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
+ return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
[[fallthrough]];
case LangOptions::SOB_Undefined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
@@ -2571,7 +2571,7 @@ llvm::Value *ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior(
switch (CGF.getLangOpts().getSignedOverflowBehavior()) {
case LangOptions::SOB_Defined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
- return Builder.CreateAdd(InVal, Amount, Name);
+ return Builder.CreateAdd(InVal, Amount, Name);
[[fallthrough]];
case LangOptions::SOB_Undefined:
if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow))
>From 018269881647673b530b8cb4611c7a380a5a1b5c Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 20 Feb 2024 22:49:28 +0000
Subject: [PATCH 3/3] add release note regarding noise
---
clang/docs/ReleaseNotes.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 685b19cabeb82c..1b1f10de0c8c5e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -402,6 +402,11 @@ 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
----------------------
More information about the cfe-commits
mailing list