[clang] [Clang] Overflow Pattern Exclusion - rename some patterns, enhance docs (PR #105709)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 22 11:11:08 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Justin Stitt (JustinStitt)
<details>
<summary>Changes</summary>
>From @<!-- -->vitalybuka's review on https://github.com/llvm/llvm-project/pull/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
---
Full diff: https://github.com/llvm/llvm-project/pull/105709.diff
10 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+13-7)
- (modified) clang/docs/UndefinedBehaviorSanitizer.rst (+36-7)
- (modified) clang/include/clang/Basic/LangOptions.h (+5-3)
- (modified) clang/include/clang/Driver/Options.td (+1-1)
- (modified) clang/lib/AST/Expr.cpp (+29-6)
- (modified) clang/lib/CodeGen/CGExprScalar.cpp (+1-1)
- (modified) clang/lib/Driver/SanitizerArgs.cpp (+5-2)
- (modified) clang/lib/Frontend/CompilerInvocation.cpp (+6-2)
- (modified) clang/test/CodeGen/ignore-overflow-pattern-false-pos.c (-1)
- (modified) clang/test/CodeGen/ignore-overflow-pattern.c (+5-2)
``````````diff
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/105709
More information about the cfe-commits
mailing list