[clang] 76236fa - [Clang] Overflow Pattern Exclusion - rename some patterns, enhance docs (#105709)

via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 23 23:33:26 PDT 2024


Author: Justin Stitt
Date: 2024-08-23T23:33:23-07:00
New Revision: 76236fafda19ff3760443196edcd3cd9610ed733

URL: https://github.com/llvm/llvm-project/commit/76236fafda19ff3760443196edcd3cd9610ed733
DIFF: https://github.com/llvm/llvm-project/commit/76236fafda19ff3760443196edcd3cd9610ed733.diff

LOG: [Clang] Overflow Pattern Exclusion - rename some patterns, enhance docs (#105709)

>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:

![image](https://github.com/user-attachments/assets/5d3497c4-5f5a-4579-b29b-96a0fd192faa)


CCs: @vitalybuka

---------

Signed-off-by: Justin Stitt <justinstitt at google.com>
Co-authored-by: Vitaly Buka <vitalybuka at google.com>

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/docs/UndefinedBehaviorSanitizer.rst
    clang/include/clang/Basic/LangOptions.h
    clang/include/clang/Driver/Options.td
    clang/lib/AST/Expr.cpp
    clang/lib/CodeGen/CGExprScalar.cpp
    clang/lib/Driver/SanitizerArgs.cpp
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
    clang/test/CodeGen/ignore-overflow-pattern.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 798f59009af3c3..0ced2f779f7058 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -466,28 +466,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..0d1010b7dcb338 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -314,26 +314,49 @@ 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)
+
+.. 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
+
+
+
+Note: ``add-signed-overflow-test`` suppresses only the check for Undefined
+Behavior. Eager Undefined Behavior optimizations are still possible. 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 4bf604d46a0f70..1b9b3f2c6600a3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2570,7 +2570,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..3309619850f34a 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4806,6 +4806,26 @@ 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) {
+  std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(E);
+  if (!Result.has_value())
+    return;
+  QualType AdditionResultType = Result.value()->getType();
+
+  if ((AdditionResultType->isSignedIntegerType() &&
+       Ctx.getLangOpts().isOverflowPatternExcluded(
+           LangOptions::OverflowPatternExclusionKind::AddSignedOverflowTest)) ||
+      (AdditionResultType->isUnsignedIntegerType() &&
+       Ctx.getLangOpts().isOverflowPatternExcluded(
+           LangOptions::OverflowPatternExclusionKind::AddUnsignedOverflowTest)))
+    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 +4838,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 2a726bba2dd304..af11bc20a3b639 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 09262f40b5b50c..18bb35a563167e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1457,9 +1457,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


        


More information about the cfe-commits mailing list