rename some patterns, rewrite docs

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>

Author: JustinStitt

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.

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

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 {

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

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);

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;

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)

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);
     }
   }

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);
 

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