From b02b09b9eb4f9a8ac60dd077d95c67b959db3b70 Mon Sep 17 00:00:00 2001 From: Justin Stitt Date: Tue, 20 Feb 2024 22:21:02 +0000 Subject: [PATCH 1/4] 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 Date: Tue, 20 Feb 2024 22:31:41 +0000 Subject: [PATCH 2/4] 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 Date: Tue, 20 Feb 2024 22:49:28 +0000 Subject: [PATCH 3/4] 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 ---------------------- From 1d9cb0aca8985aa1636780b3ff9a863962cc2d57 Mon Sep 17 00:00:00 2001 From: Justin Stitt Date: Tue, 20 Feb 2024 23:30:47 +0000 Subject: [PATCH 4/4] update tests to use common prefix --- clang/test/CodeGen/integer-overflow.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/clang/test/CodeGen/integer-overflow.c b/clang/test/CodeGen/integer-overflow.c index b6d1a9fc4cf744..461b026d39615b 100644 --- a/clang/test/CodeGen/integer-overflow.c +++ b/clang/test/CodeGen/integer-overflow.c @@ -1,8 +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 -fwrapv | FileCheck %s --check-prefix=CATCH_WRAP +// 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 @@ -17,7 +17,6 @@ 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; @@ -25,7 +24,6 @@ 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; @@ -33,7 +31,6 @@ 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; @@ -41,7 +38,6 @@ 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; @@ -51,7 +47,6 @@ 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; @@ -59,7 +54,6 @@ 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; @@ -69,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; @@ -77,7 +72,6 @@ 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. @@ -86,7 +80,6 @@ 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. @@ -95,7 +88,6 @@ 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. @@ -104,7 +96,6 @@ 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. @@ -113,5 +104,4 @@ 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) }