[Clang] Overflow Pattern Exclusion - rename some patterns, enhance docs #105709
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
From Vitaly's review on llvm#104889: * remove unused variable in tests * rename `post-decr-while` --> `unsigned-post-decr-while` * split `add-overflow-test` into `add-unsigned-overflow-test` and `add-signed-overflow-test` * be more clear about defaults within docs * add table to docs Signed-off-by: Justin Stitt <justinstitt@google.com>
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Justin Stitt (JustinStitt) ChangesFrom @vitalybuka's review on #104889:
Here's a screenshot of the rendered table so you don't have to build the html docs yourself to inspect the layout: CCs: @vitalybuka Full diff: https://github.com/llvm/llvm-project/pull/105709.diff 10 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5c156a9c073a9c..562bf95663581e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -451,28 +451,34 @@ Sanitizers
- Added the ``-fsanitize-undefined-ignore-overflow-pattern`` flag which can be
used to disable specific overflow-dependent code patterns. The supported
- patterns are: ``add-overflow-test``, ``negated-unsigned-const``, and
- ``post-decr-while``. The sanitizer instrumentation can be toggled off for all
- available patterns by specifying ``all``. Conversely, you can disable all
- exclusions with ``none``.
+ patterns are: ``add-signed-overflow-test``, ``add-unsigned-overflow-test``,
+ ``negated-unsigned-const``, and ``unsigned-post-decr-while``. The sanitizer
+ instrumentation can be toggled off for all available patterns by specifying
+ ``all``. Conversely, you may disable all exclusions with ``none`` which is
+ the default.
.. code-block:: c++
- /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-overflow-test``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-unsigned-overflow-test``
int common_overflow_check_pattern(unsigned base, unsigned offset) {
if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings, won't be instrumented
}
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test``
+ int common_overflow_check_pattern_signed(signed int base, signed int offset) {
+ if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings, won't be instrumented
+ }
+
/// specified with ``-fsanitize-undefined-ignore-overflow-pattern=negated-unsigned-const``
void negation_overflow() {
unsigned long foo = -1UL; // No longer causes a negation overflow warning
unsigned long bar = -2UL; // and so on...
}
- /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=post-decr-while``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while``
void while_post_decrement() {
unsigned char count = 16;
- while (count--) { /* ... */} // No longer causes unsigned-integer-overflow sanitizer to trip
+ while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
}
Many existing projects have a large amount of these code patterns present.
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index 1c92907372f83c..973a6f39f296b3 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -314,26 +314,55 @@ Currently, this option supports three overflow-dependent code idioms:
unsigned long foo = -1UL; // No longer causes a negation overflow warning
unsigned long bar = -2UL; // and so on...
-``post-decr-while``
+``unsigned-post-decr-while``
.. code-block:: c++
- /// -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
+ /// -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while
unsigned char count = 16;
while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
-``add-overflow-test``
+``add-signed-overflow-test,add-unsigned-overflow-test``
.. code-block:: c++
- /// -fsanitize-undefined-ignore-overflow-pattern=add-overflow-test
+ /// -fsanitize-undefined-ignore-overflow-pattern=add-(signed|unsigned)-overflow-test
if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings,
- // won't be instrumented (same for signed types)
+ // won't be instrumented (signed or unsigned types)
+
+Of the two arithmetic overflow sanitizer kinds ``unsigned-integer-overflow``
+and ``signed-integer-overflow``, ignored overflow patterns exclude
+instrumentation from one of them.
+
+.. list-table:: Overflow Pattern Types
+ :widths: 30 50
+ :header-rows: 1
+
+ * - Pattern
+ - Sanitizer
+ * - negated-unsigned-const
+ - unsigned-integer-overflow
+ * - unsigned-post-decr-while
+ - unsigned-integer-overflow
+ * - add-unsigned-overflow-test
+ - unsigned-integer-overflow
+ * - add-signed-overflow-test
+ - signed-integer-overflow
+
+
+
+Ignoring overflow patterns has no effect on the definedness of the arithmetic
+within the pattern. Since ``add-signed-overflow-test`` works with the
+``signed-integer-overflow`` sanitizer instrumentation for code matching this
+overflow pattern is ommitted which may cause eager UB optimizations. One may
+remedy this with ``-fwrapv`` or ``-fno-strict-overflow``.
You can enable all exclusions with
``-fsanitize-undefined-ignore-overflow-pattern=all`` or disable all exclusions
-with ``-fsanitize-undefined-ignore-overflow-pattern=none``. Specifying ``none``
-has precedence over other values.
+with ``-fsanitize-undefined-ignore-overflow-pattern=none``. If
+``-fsanitize-undefined-ignore-overflow-pattern`` is not specified ``none`` is
+implied. Specifying ``none`` alongside other values also implies ``none`` as
+``none`` has precedence over other values -- including ``all``.
Issue Suppression
=================
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index eb4cb4b5a7e93f..1c80ee89837cb3 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -375,11 +375,13 @@ class LangOptionsBase {
/// Exclude all overflow patterns (below)
All = 1 << 1,
/// if (a + b < a)
- AddOverflowTest = 1 << 2,
+ AddSignedOverflowTest = 1 << 2,
+ /// if (a + b < a)
+ AddUnsignedOverflowTest = 1 << 3,
/// -1UL
- NegUnsignedConst = 1 << 3,
+ NegUnsignedConst = 1 << 4,
/// while (count--)
- PostDecrInWhile = 1 << 4,
+ PostDecrInWhile = 1 << 5,
};
enum class DefaultVisiblityExportMapping {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c204062b4f7353..2a6bf8fbc7fbf7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2568,7 +2568,7 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats",
def fsanitize_undefined_ignore_overflow_pattern_EQ : CommaJoined<["-"], "fsanitize-undefined-ignore-overflow-pattern=">,
HelpText<"Specify the overflow patterns to exclude from artihmetic sanitizer instrumentation">,
Visibility<[ClangOption, CC1Option]>,
- Values<"none,all,add-overflow-test,negated-unsigned-const,post-decr-while">,
+ Values<"none,all,add-unsigned-overflow-test,add-signed-overflow-test,negated-unsigned-const,unsigned-post-decr-while">,
MarshallingInfoStringVector<LangOpts<"OverflowPatternExclusionValues">>;
def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
Group<f_clang_Group>,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 25ab6f3b2addfb..b3ef70cf386c4e 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4806,6 +4806,34 @@ getOverflowPatternBinOp(const BinaryOperator *E) {
return {};
}
+/// Compute and set the OverflowPatternExclusion bit based on whether the
+/// BinaryOperator expression matches an overflow pattern being ignored by
+/// -fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test or
+/// -fsanitize-undefined-ignore-overflow-pattern=add-unsigned-overflow-test
+static void computeOverflowPatternExclusion(const ASTContext &Ctx,
+ const BinaryOperator *E) {
+ bool AddSignedOverflowTest = Ctx.getLangOpts().isOverflowPatternExcluded(
+ LangOptions::OverflowPatternExclusionKind::AddSignedOverflowTest);
+ bool AddUnsignedOverflowTest = Ctx.getLangOpts().isOverflowPatternExcluded(
+ LangOptions::OverflowPatternExclusionKind::AddUnsignedOverflowTest);
+
+ if (!AddSignedOverflowTest && !AddUnsignedOverflowTest)
+ return;
+
+ std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(E);
+
+ if (!Result.has_value())
+ return;
+
+ QualType AdditionResultType = Result.value()->getType();
+
+ if (AddSignedOverflowTest && AdditionResultType->isSignedIntegerType())
+ Result.value()->setExcludedOverflowPattern(true);
+ else if (AddUnsignedOverflowTest &&
+ AdditionResultType->isUnsignedIntegerType())
+ Result.value()->setExcludedOverflowPattern(true);
+}
+
BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
Opcode opc, QualType ResTy, ExprValueKind VK,
ExprObjectKind OK, SourceLocation opLoc,
@@ -4818,12 +4846,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
BinaryOperatorBits.ExcludedOverflowPattern = false;
SubExprs[LHS] = lhs;
SubExprs[RHS] = rhs;
- if (Ctx.getLangOpts().isOverflowPatternExcluded(
- LangOptions::OverflowPatternExclusionKind::AddOverflowTest)) {
- std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(this);
- if (Result.has_value())
- Result.value()->BinaryOperatorBits.ExcludedOverflowPattern = true;
- }
+ computeOverflowPatternExclusion(Ctx, this);
BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 3bda254c86adf6..ae600a3d8489c8 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2785,7 +2785,7 @@ static bool matchesPostDecrInWhile(const UnaryOperator *UO, bool isInc,
if (isInc || isPre)
return false;
- // -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
+ // -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while
if (!Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::PostDecrInWhile))
return false;
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 9d9ad79d51d7f8..4d05375348da54 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1453,9 +1453,12 @@ static int parseOverflowPatternExclusionValues(const Driver &D,
llvm::StringSwitch<int>(Value)
.Case("none", LangOptionsBase::None)
.Case("all", LangOptionsBase::All)
- .Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
+ .Case("add-unsigned-overflow-test",
+ LangOptionsBase::AddUnsignedOverflowTest)
+ .Case("add-signed-overflow-test",
+ LangOptionsBase::AddSignedOverflowTest)
.Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
- .Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
+ .Case("unsigned-post-decr-while", LangOptionsBase::PostDecrInWhile)
.Default(0);
if (E == 0)
D.Diag(clang::diag::err_drv_unsupported_option_argument)
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index f510d3067d4d58..0bb4175dd021ee 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4274,9 +4274,13 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
llvm::StringSwitch<unsigned>(A->getValue(i))
.Case("none", LangOptionsBase::None)
.Case("all", LangOptionsBase::All)
- .Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
+ .Case("add-unsigned-overflow-test",
+ LangOptionsBase::AddUnsignedOverflowTest)
+ .Case("add-signed-overflow-test",
+ LangOptionsBase::AddSignedOverflowTest)
.Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
- .Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
+ .Case("unsigned-post-decr-while",
+ LangOptionsBase::PostDecrInWhile)
.Default(0);
}
}
diff --git a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
index 40193e0c3e2671..b4811443b95192 100644
--- a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
@@ -3,7 +3,6 @@
// Check for potential false positives from patterns that _almost_ match classic overflow-dependent or overflow-prone code patterns
extern unsigned a, b, c;
-extern int u, v, w;
extern unsigned some(void);
diff --git a/clang/test/CodeGen/ignore-overflow-pattern.c b/clang/test/CodeGen/ignore-overflow-pattern.c
index b7d700258f8538..c4a9d07b07aaac 100644
--- a/clang/test/CodeGen/ignore-overflow-pattern.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern.c
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all %s -emit-llvm -o - | FileCheck %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all -fwrapv %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=add-overflow-test %s -emit-llvm -o - | FileCheck %s --check-prefix=ADD
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test,add-unsigned-overflow-test %s -emit-llvm -o - | FileCheck %s --check-prefix=ADD
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=negated-unsigned-const %s -emit-llvm -o - | FileCheck %s --check-prefix=NEGATE
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=post-decr-while %s -emit-llvm -o - | FileCheck %s --check-prefix=WHILE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while %s -emit-llvm -o - | FileCheck %s --check-prefix=WHILE
// Ensure some common overflow-dependent or overflow-prone code patterns don't
// trigger the overflow sanitizers. In many cases, overflow warnings caused by
@@ -25,6 +25,7 @@
// CHECK-NOT: handle{{.*}}overflow
extern unsigned a, b, c;
+extern int u, v;
extern unsigned some(void);
// ADD-LABEL: @basic_commutativity
@@ -50,6 +51,8 @@ void basic_commutativity(void) {
c = 9;
if (b > b + a)
c = 9;
+ if (u + v < u)
+ c = 9;
}
// ADD-LABEL: @arguments_and_commutativity
|
|
@llvm/pr-subscribers-clang-driver Author: Justin Stitt (JustinStitt) ChangesFrom @vitalybuka's review on #104889:
Here's a screenshot of the rendered table so you don't have to build the html docs yourself to inspect the layout: CCs: @vitalybuka Full diff: https://github.com/llvm/llvm-project/pull/105709.diff 10 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5c156a9c073a9c..562bf95663581e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -451,28 +451,34 @@ Sanitizers
- Added the ``-fsanitize-undefined-ignore-overflow-pattern`` flag which can be
used to disable specific overflow-dependent code patterns. The supported
- patterns are: ``add-overflow-test``, ``negated-unsigned-const``, and
- ``post-decr-while``. The sanitizer instrumentation can be toggled off for all
- available patterns by specifying ``all``. Conversely, you can disable all
- exclusions with ``none``.
+ patterns are: ``add-signed-overflow-test``, ``add-unsigned-overflow-test``,
+ ``negated-unsigned-const``, and ``unsigned-post-decr-while``. The sanitizer
+ instrumentation can be toggled off for all available patterns by specifying
+ ``all``. Conversely, you may disable all exclusions with ``none`` which is
+ the default.
.. code-block:: c++
- /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-overflow-test``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-unsigned-overflow-test``
int common_overflow_check_pattern(unsigned base, unsigned offset) {
if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings, won't be instrumented
}
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test``
+ int common_overflow_check_pattern_signed(signed int base, signed int offset) {
+ if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings, won't be instrumented
+ }
+
/// specified with ``-fsanitize-undefined-ignore-overflow-pattern=negated-unsigned-const``
void negation_overflow() {
unsigned long foo = -1UL; // No longer causes a negation overflow warning
unsigned long bar = -2UL; // and so on...
}
- /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=post-decr-while``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while``
void while_post_decrement() {
unsigned char count = 16;
- while (count--) { /* ... */} // No longer causes unsigned-integer-overflow sanitizer to trip
+ while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
}
Many existing projects have a large amount of these code patterns present.
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index 1c92907372f83c..973a6f39f296b3 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -314,26 +314,55 @@ Currently, this option supports three overflow-dependent code idioms:
unsigned long foo = -1UL; // No longer causes a negation overflow warning
unsigned long bar = -2UL; // and so on...
-``post-decr-while``
+``unsigned-post-decr-while``
.. code-block:: c++
- /// -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
+ /// -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while
unsigned char count = 16;
while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
-``add-overflow-test``
+``add-signed-overflow-test,add-unsigned-overflow-test``
.. code-block:: c++
- /// -fsanitize-undefined-ignore-overflow-pattern=add-overflow-test
+ /// -fsanitize-undefined-ignore-overflow-pattern=add-(signed|unsigned)-overflow-test
if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings,
- // won't be instrumented (same for signed types)
+ // won't be instrumented (signed or unsigned types)
+
+Of the two arithmetic overflow sanitizer kinds ``unsigned-integer-overflow``
+and ``signed-integer-overflow``, ignored overflow patterns exclude
+instrumentation from one of them.
+
+.. list-table:: Overflow Pattern Types
+ :widths: 30 50
+ :header-rows: 1
+
+ * - Pattern
+ - Sanitizer
+ * - negated-unsigned-const
+ - unsigned-integer-overflow
+ * - unsigned-post-decr-while
+ - unsigned-integer-overflow
+ * - add-unsigned-overflow-test
+ - unsigned-integer-overflow
+ * - add-signed-overflow-test
+ - signed-integer-overflow
+
+
+
+Ignoring overflow patterns has no effect on the definedness of the arithmetic
+within the pattern. Since ``add-signed-overflow-test`` works with the
+``signed-integer-overflow`` sanitizer instrumentation for code matching this
+overflow pattern is ommitted which may cause eager UB optimizations. One may
+remedy this with ``-fwrapv`` or ``-fno-strict-overflow``.
You can enable all exclusions with
``-fsanitize-undefined-ignore-overflow-pattern=all`` or disable all exclusions
-with ``-fsanitize-undefined-ignore-overflow-pattern=none``. Specifying ``none``
-has precedence over other values.
+with ``-fsanitize-undefined-ignore-overflow-pattern=none``. If
+``-fsanitize-undefined-ignore-overflow-pattern`` is not specified ``none`` is
+implied. Specifying ``none`` alongside other values also implies ``none`` as
+``none`` has precedence over other values -- including ``all``.
Issue Suppression
=================
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index eb4cb4b5a7e93f..1c80ee89837cb3 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -375,11 +375,13 @@ class LangOptionsBase {
/// Exclude all overflow patterns (below)
All = 1 << 1,
/// if (a + b < a)
- AddOverflowTest = 1 << 2,
+ AddSignedOverflowTest = 1 << 2,
+ /// if (a + b < a)
+ AddUnsignedOverflowTest = 1 << 3,
/// -1UL
- NegUnsignedConst = 1 << 3,
+ NegUnsignedConst = 1 << 4,
/// while (count--)
- PostDecrInWhile = 1 << 4,
+ PostDecrInWhile = 1 << 5,
};
enum class DefaultVisiblityExportMapping {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c204062b4f7353..2a6bf8fbc7fbf7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2568,7 +2568,7 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats",
def fsanitize_undefined_ignore_overflow_pattern_EQ : CommaJoined<["-"], "fsanitize-undefined-ignore-overflow-pattern=">,
HelpText<"Specify the overflow patterns to exclude from artihmetic sanitizer instrumentation">,
Visibility<[ClangOption, CC1Option]>,
- Values<"none,all,add-overflow-test,negated-unsigned-const,post-decr-while">,
+ Values<"none,all,add-unsigned-overflow-test,add-signed-overflow-test,negated-unsigned-const,unsigned-post-decr-while">,
MarshallingInfoStringVector<LangOpts<"OverflowPatternExclusionValues">>;
def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
Group<f_clang_Group>,
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 25ab6f3b2addfb..b3ef70cf386c4e 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4806,6 +4806,34 @@ getOverflowPatternBinOp(const BinaryOperator *E) {
return {};
}
+/// Compute and set the OverflowPatternExclusion bit based on whether the
+/// BinaryOperator expression matches an overflow pattern being ignored by
+/// -fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test or
+/// -fsanitize-undefined-ignore-overflow-pattern=add-unsigned-overflow-test
+static void computeOverflowPatternExclusion(const ASTContext &Ctx,
+ const BinaryOperator *E) {
+ bool AddSignedOverflowTest = Ctx.getLangOpts().isOverflowPatternExcluded(
+ LangOptions::OverflowPatternExclusionKind::AddSignedOverflowTest);
+ bool AddUnsignedOverflowTest = Ctx.getLangOpts().isOverflowPatternExcluded(
+ LangOptions::OverflowPatternExclusionKind::AddUnsignedOverflowTest);
+
+ if (!AddSignedOverflowTest && !AddUnsignedOverflowTest)
+ return;
+
+ std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(E);
+
+ if (!Result.has_value())
+ return;
+
+ QualType AdditionResultType = Result.value()->getType();
+
+ if (AddSignedOverflowTest && AdditionResultType->isSignedIntegerType())
+ Result.value()->setExcludedOverflowPattern(true);
+ else if (AddUnsignedOverflowTest &&
+ AdditionResultType->isUnsignedIntegerType())
+ Result.value()->setExcludedOverflowPattern(true);
+}
+
BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
Opcode opc, QualType ResTy, ExprValueKind VK,
ExprObjectKind OK, SourceLocation opLoc,
@@ -4818,12 +4846,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
BinaryOperatorBits.ExcludedOverflowPattern = false;
SubExprs[LHS] = lhs;
SubExprs[RHS] = rhs;
- if (Ctx.getLangOpts().isOverflowPatternExcluded(
- LangOptions::OverflowPatternExclusionKind::AddOverflowTest)) {
- std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(this);
- if (Result.has_value())
- Result.value()->BinaryOperatorBits.ExcludedOverflowPattern = true;
- }
+ computeOverflowPatternExclusion(Ctx, this);
BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 3bda254c86adf6..ae600a3d8489c8 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2785,7 +2785,7 @@ static bool matchesPostDecrInWhile(const UnaryOperator *UO, bool isInc,
if (isInc || isPre)
return false;
- // -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
+ // -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while
if (!Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::PostDecrInWhile))
return false;
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 9d9ad79d51d7f8..4d05375348da54 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1453,9 +1453,12 @@ static int parseOverflowPatternExclusionValues(const Driver &D,
llvm::StringSwitch<int>(Value)
.Case("none", LangOptionsBase::None)
.Case("all", LangOptionsBase::All)
- .Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
+ .Case("add-unsigned-overflow-test",
+ LangOptionsBase::AddUnsignedOverflowTest)
+ .Case("add-signed-overflow-test",
+ LangOptionsBase::AddSignedOverflowTest)
.Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
- .Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
+ .Case("unsigned-post-decr-while", LangOptionsBase::PostDecrInWhile)
.Default(0);
if (E == 0)
D.Diag(clang::diag::err_drv_unsupported_option_argument)
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index f510d3067d4d58..0bb4175dd021ee 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4274,9 +4274,13 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
llvm::StringSwitch<unsigned>(A->getValue(i))
.Case("none", LangOptionsBase::None)
.Case("all", LangOptionsBase::All)
- .Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
+ .Case("add-unsigned-overflow-test",
+ LangOptionsBase::AddUnsignedOverflowTest)
+ .Case("add-signed-overflow-test",
+ LangOptionsBase::AddSignedOverflowTest)
.Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
- .Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
+ .Case("unsigned-post-decr-while",
+ LangOptionsBase::PostDecrInWhile)
.Default(0);
}
}
diff --git a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
index 40193e0c3e2671..b4811443b95192 100644
--- a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
@@ -3,7 +3,6 @@
// Check for potential false positives from patterns that _almost_ match classic overflow-dependent or overflow-prone code patterns
extern unsigned a, b, c;
-extern int u, v, w;
extern unsigned some(void);
diff --git a/clang/test/CodeGen/ignore-overflow-pattern.c b/clang/test/CodeGen/ignore-overflow-pattern.c
index b7d700258f8538..c4a9d07b07aaac 100644
--- a/clang/test/CodeGen/ignore-overflow-pattern.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern.c
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all %s -emit-llvm -o - | FileCheck %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all -fwrapv %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=add-overflow-test %s -emit-llvm -o - | FileCheck %s --check-prefix=ADD
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test,add-unsigned-overflow-test %s -emit-llvm -o - | FileCheck %s --check-prefix=ADD
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=negated-unsigned-const %s -emit-llvm -o - | FileCheck %s --check-prefix=NEGATE
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=post-decr-while %s -emit-llvm -o - | FileCheck %s --check-prefix=WHILE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while %s -emit-llvm -o - | FileCheck %s --check-prefix=WHILE
// Ensure some common overflow-dependent or overflow-prone code patterns don't
// trigger the overflow sanitizers. In many cases, overflow warnings caused by
@@ -25,6 +25,7 @@
// CHECK-NOT: handle{{.*}}overflow
extern unsigned a, b, c;
+extern int u, v;
extern unsigned some(void);
// ADD-LABEL: @basic_commutativity
@@ -50,6 +51,8 @@ void basic_commutativity(void) {
c = 9;
if (b > b + a)
c = 9;
+ if (u + v < u)
+ c = 9;
}
// ADD-LABEL: @arguments_and_commutativity
|
With this table, pattern affect only one sanitizer check, so splitting checks, as I suggested is possible. |
vitalybuka
reviewed
Aug 24, 2024
vitalybuka
reviewed
Aug 24, 2024
vitalybuka
approved these changes
Aug 24, 2024
5chmidti
pushed a commit
that referenced
this pull request
Aug 24, 2024
…cs (#105709) From @vitalybuka's review on #104889: - [x] remove unused variable in tests - [x] rename `post-decr-while` --> `unsigned-post-decr-while` - [x] split `add-overflow-test` into `add-unsigned-overflow-test` and `add-signed-overflow-test` - [x] be more clear about defaults within docs - [x] add table to docs Here's a screenshot of the rendered table so you don't have to build the html docs yourself to inspect the layout:  CCs: @vitalybuka --------- Signed-off-by: Justin Stitt <justinstitt@google.com> Co-authored-by: Vitaly Buka <vitalybuka@google.com>
dmpolukhin
pushed a commit
to dmpolukhin/llvm-project
that referenced
this pull request
Sep 2, 2024
…cs (llvm#105709) From @vitalybuka's review on llvm#104889: - [x] remove unused variable in tests - [x] rename `post-decr-while` --> `unsigned-post-decr-while` - [x] split `add-overflow-test` into `add-unsigned-overflow-test` and `add-signed-overflow-test` - [x] be more clear about defaults within docs - [x] add table to docs Here's a screenshot of the rendered table so you don't have to build the html docs yourself to inspect the layout:  CCs: @vitalybuka --------- Signed-off-by: Justin Stitt <justinstitt@google.com> Co-authored-by: Vitaly Buka <vitalybuka@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From @vitalybuka's review on #104889:
post-decr-while-->unsigned-post-decr-whileadd-overflow-testintoadd-unsigned-overflow-testandadd-signed-overflow-testHere's a screenshot of the rendered table so you don't have to build the html docs yourself to inspect the layout:
CCs: @vitalybuka