remove more instances of 'idiom'

JustinStitt · JustinStitt · commit 1f15671ad9f2 · 2024-08-01T21:44:25.000Z
Signed-off-by: Justin Stitt &lt;justinstitt@google.com&gt;
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -294,31 +294,35 @@ Moved checkers
 Sanitizers
 ----------
 
-- Added the ``-fno-sanitize-overflow-idioms`` flag which disables integer
-  overflow and truncation sanitizer instrumentation for specific
-  overflow-dependent code patterns. The noise created by these idioms is a
-  large reason as to why large projects refuse to turn on arithmetic
-  sanitizers.
+- Added the ``-fsanitize-overflow-pattern-exclusion=`` 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``.
 
   .. code-block:: c++
 
+     /// specified with ``-fsanitize-overflow-pattern-exclusion=add-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-overflow-pattern-exclusion=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-overflow-pattern-exclusion=post-decr-while``
      void while_post_decrement() {
        unsigned char count = 16;
        while (count--) { /* ... */} // No longer causes unsigned-integer-overflow sanitizer to trip
      }
 
-     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
-     }
-
-  Note that the ``-fsanitize-overflow-idioms`` flag now also exists but has
-  virtually no function other than to disable an already present
-  ``-fno-sanitize-overflow-idioms``.
+  Many existing projects have a large amount of these code patterns present.
+  This new flag should allow those projects to enable integer sanitizers with
+  less noise.
 
 Python Binding Changes
 ----------------------
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -293,56 +293,58 @@ To silence reports from unsigned integer overflow, you can set
 ``-fsanitize-recover=unsigned-integer-overflow``, is particularly useful for
 providing fuzzing signal without blowing up logs.
 
-Issue Suppression
-=================
-
-UndefinedBehaviorSanitizer is not expected to produce false positives.
-If you see one, look again; most likely it is a true positive!
-
-Disabling Instrumentation with ``__attribute__((no_sanitize("undefined")))``
-----------------------------------------------------------------------------
-
-You disable UBSan checks for particular functions with
-``__attribute__((no_sanitize("undefined")))``. You can use all values of
-``-fsanitize=`` flag in this attribute, e.g. if your function deliberately
-contains possible signed integer overflow, you can use
-``__attribute__((no_sanitize("signed-integer-overflow")))``.
-
-This attribute may not be
-supported by other compilers, so consider using it together with
-``#if defined(__clang__)``.
-
-Disabling instrumentation of common overflow idioms
-=====================================================
+Disabling instrumentation for common overflow patterns
+------------------------------------------------------
 
-There are certain overflow-dependent code patterns which produce a lot of noise
-for integer overflow/truncation sanitizers. To disable instrumentation for
-these common patterns one should use ``-fno-sanitize-overflow-idioms``. Its
-inverse ``-fsanitize-overflow-idioms`` also exists but has no function other
-than to disable an already present ``-fno-sanitize-overflow-idioms``.
+There are certain overflow-dependent or overflow-prone code patterns which
+produce a lot of noise for integer overflow/truncation sanitizers. To disable
+instrumentation for these common patterns one should use
+``-fsanitize-overflow-pattern-exclusion=``.
 
-Currently, this option handles three pervasive overflow-dependent code idioms:
+Currently, this option supports three pervasive overflow-dependent code idioms:
 
 .. code-block:: c++
 
+    /// -fsanitize-overflow-pattern-exclusion=negated-unsigned-const
     unsigned long foo = -1UL; // No longer causes a negation overflow warning
     unsigned long bar = -2UL; // and so on...
 
 .. code-block:: c++
 
+    /// -fsanitize-overflow-pattern-exclusion=post-decr-while
     unsigned char count = 16;
     while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
 
 .. code-block:: c++
 
+    /// -fsanitize-overflow-pattern-exclusion=add-overflow-test
     if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings,
                                             // won't be instrumented (same for signed types)
 
 Negated unsigned constants, post-decrements in a while loop condition and
-simple overflow checks are accepted and pervasive code patterns. Existing
-projects that enable more warnings or sanitizers may find that the compiler is
-too noisy. Now, they can use ``-fno-sanitize-overflow-idioms`` with no source
-modifications.
+simple overflow checks are accepted and pervasive code patterns. You can enable
+all exclusions with ``-fsanitize-overflow-pattern-exclusion=all`` or disable
+all exclusions with ``-fsanitize-overflow-pattern-exclusion=none``. Specifying
+``none`` has precedence over other values.
+
+Issue Suppression
+=================
+
+UndefinedBehaviorSanitizer is not expected to produce false positives.
+If you see one, look again; most likely it is a true positive!
+
+Disabling Instrumentation with ``__attribute__((no_sanitize("undefined")))``
+----------------------------------------------------------------------------
+
+You disable UBSan checks for particular functions with
+``__attribute__((no_sanitize("undefined")))``. You can use all values of
+``-fsanitize=`` flag in this attribute, e.g. if your function deliberately
+contains possible signed integer overflow, you can use
+``__attribute__((no_sanitize("signed-integer-overflow")))``.
+
+This attribute may not be
+supported by other compilers, so consider using it together with
+``#if defined(__clang__)``.
 
 Suppressing Errors in Recompiled Code (Ignorelist)
 --------------------------------------------------
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -2558,11 +2558,6 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats",
           "Disable">,
   BothFlags<[], [ClangOption], " sanitizer statistics gathering.">>,
   Group<f_clang_Group>;
-defm sanitize_overflow_idioms : BoolFOption<"sanitize-overflow-idioms",
-  LangOpts<"SanitizeOverflowIdioms">, DefaultTrue,
-  PosFlag<SetTrue, [], [], "Enable">,
-  NegFlag<SetFalse, [], [], "Disable">,
-  BothFlags<[], [ClangOption], " the instrumentation of common overflow idioms">>;
 def fsanitize_overflow_pattern_exclusion_EQ : CommaJoined<["-"], "fsanitize-overflow-pattern-exclusion=">,
   HelpText<"Specify the overflow patterns to exclude from artihmetic sanitizer instrumentation">,
   Visibility<[ClangOption, CC1Option]>,
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
@@ -65,7 +65,6 @@ class SanitizerArgs {
   // True if cross-dso CFI support if provided by the system (i.e. Android).
   bool ImplicitCfiRuntime = false;
   bool NeedsMemProfRt = false;
-  bool SanitizeOverflowIdioms = true;
   bool HwasanUseAliases = false;
   llvm::AsanDetectStackUseAfterReturnMode AsanUseAfterReturn =
       llvm::AsanDetectStackUseAfterReturnMode::Invalid;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
@@ -4760,12 +4760,12 @@ ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx,
 }
 
 namespace {
-/// Certain overflow-dependent code idioms can have their integer overflow
+/// Certain overflow-dependent code patterns can have their integer overflow
 /// sanitization disabled. Check for the common pattern `if (a + b < a)` and
 /// return the resulting BinaryOperator responsible for the addition so we can
 /// elide overflow checks during codegen.
 static std::optional<BinaryOperator *>
-getOverflowIdiomBinOp(const BinaryOperator *E) {
+getOverflowPatternBinOp(const BinaryOperator *E) {
   Expr *Addition, *ComparedTo;
   if (E->getOpcode() == BO_LT) {
     Addition = E->getLHS();
@@ -4821,7 +4821,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
   SubExprs[RHS] = rhs;
   if (Ctx.getLangOpts().isOverflowPatternExcluded(
           LangOptions::OverflowPatternExclusionKind::AddOverflowTest)) {
-    std::optional<BinaryOperator *> Result = getOverflowIdiomBinOp(this);
+    std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(this);
     if (Result.has_value())
       Result.value()->ExcludedOverflowPattern = true;
   }
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -198,9 +198,7 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) {
 
   const UnaryOperator *UO = dyn_cast<UnaryOperator>(Op.E);
 
-  // TODO: rename or rework NegUnsignedConst because it doesn't only work on
-  // constants.
-  if (UO && UO->getOpcode() == UO_Minus &&
+  if (UO && UO->getOpcode() == UO_Minus && UO->isIntegerConstantExpr(Ctx) &&
       Ctx.getLangOpts().isOverflowPatternExcluded(
           LangOptions::OverflowPatternExclusionKind::NegUnsignedConst))
     return true;
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1080,10 +1080,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
                                 options::OPT_fmemory_profile_EQ,
                                 options::OPT_fno_memory_profile, false);
 
-  SanitizeOverflowIdioms =
-      Args.hasFlag(options::OPT_fsanitize_overflow_idioms,
-                   options::OPT_fno_sanitize_overflow_idioms, true);
-
   // Finally, initialize the set of available and recoverable sanitizers.
   Sanitizers.Mask |= Kinds;
   RecoverableSanitizers.Mask |= RecoverableKinds;
@@ -1375,9 +1371,6 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
     CmdArgs.push_back("+tagged-globals");
   }
 
-  if (!SanitizeOverflowIdioms)
-    CmdArgs.push_back("-fno-sanitize-overflow-idioms");
-
   // MSan: Workaround for PR16386.
   // ASan: This is mainly to help LSan with cases such as
   // https://github.com/google/sanitizers/issues/373
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
@@ -1,4 +1,4 @@
-// Check for potential false positives from patterns that _almost_ match classic overflow idioms
+// Check for potential false positives from patterns that _almost_ match classic overflow-dependent or overflow-prone code patterns
 // RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all -fwrapv -S -emit-llvm -o - | FileCheck %s
 
@@ -8,7 +8,8 @@ extern int u, v, w;
 extern unsigned some(void);
 
 // Make sure all these still have handler paths, we shouldn't be excluding
-// instrumentation of any "near" idioms.
+// instrumentation of any "near" patterns.
+// CHECK-LABEL: close_but_not_quite
 void close_but_not_quite(void) {
   // CHECK: br i1{{.*}}handler.
   if (a + b > a)
@@ -26,6 +27,7 @@ void close_but_not_quite(void) {
   if (a + b + 1 < a)
     c = 9;
 
+  // CHECK: br i1{{.*}}handler.
   // CHECK: br i1{{.*}}handler.
   if (a + b < a + 1)
     c = 9;
@@ -42,10 +44,6 @@ void close_but_not_quite(void) {
   if (a + b == a)
     c = 9;
 
-  // CHECK: br i1{{.*}}handler
-  if (u + v < u) /* matches overflow idiom, but is signed */
-    c = 9;
-
   // CHECK: br i1{{.*}}handler
   // Although this can never actually overflow we are still checking that the
   // sanitizer instruments it.
@@ -54,14 +52,15 @@ void close_but_not_quite(void) {
 }
 
 // cvise'd kernel code that caused problems during development
-// CHECK: br i1{{.*}}handler
 typedef unsigned size_t;
 typedef enum { FSE_repeat_none } FSE_repeat;
 typedef enum { ZSTD_defaultAllowed } ZSTD_defaultPolicy_e;
 FSE_repeat ZSTD_selectEncodingType_repeatMode;
 ZSTD_defaultPolicy_e ZSTD_selectEncodingType_isDefaultAllowed;
 size_t ZSTD_NCountCost(void);
 
+// CHECK-LABEL: ZSTD_selectEncodingType
+// CHECK: br i1{{.*}}handler
 void ZSTD_selectEncodingType(void) {
   size_t basicCost =
              ZSTD_selectEncodingType_isDefaultAllowed ? ZSTD_NCountCost() : 0,
@@ -70,8 +69,15 @@ void ZSTD_selectEncodingType(void) {
     ZSTD_selectEncodingType_repeatMode = FSE_repeat_none;
 }
 
+// CHECK-LABEL: function_calls
 void function_calls(void) {
   // CHECK: br i1{{.*}}handler
   if (some() + b < some())
     c = 9;
 }
+
+// CHECK-LABEL: not_quite_a_negated_unsigned_const
+void not_quite_a_negated_unsigned_const(void) {
+  // CHECK: br i1{{.*}}handler
+  a = -b;
+}
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/overflow-idiom-exclusion.c
@@ -1,6 +1,6 @@
-// Ensure some common idioms don't trigger the overflow sanitizers when
-// -fno-sanitize-overflow-idioms is enabled. In many cases, overflow warnings
-// caused by these idioms are seen as "noise" and result in users turning off
+// Ensure some common overflow-dependent or overflow-prone code patterns don't
+// trigger the overflow sanitizers. In many cases, overflow warnings caused by
+// these patterns are seen as "noise" and result in users turning off
 // sanitization all together.
 
 // A pattern like "if (a + b < a)" simply checks for overflow and usually means
@@ -9,8 +9,7 @@
 // Similarly, a pattern resembling "while (i--)" is extremely common and
 // warning on its inevitable overflow can be seen as superfluous. Do note that
 // using "i" in future calculations can be tricky because it will still
-// wrap-around. Using -fno-sanitize-overflow-idioms or not doesn't change this
-// fact -- we just won't warn/trap with sanitizers.
+// wrap-around.
 
 // Another common pattern that, in some cases, is found to be too noisy is
 // unsigned negation, for example:
@@ -107,7 +106,7 @@ void nestedstructs(void) {
 }
 
 // Normally, this would be folded into a simple call to the overflow handler
-// and a store. Excluding this idiom results in just a store.
+// and a store. Excluding this pattern results in just a store.
 void constants(void) {
   unsigned base = 4294967295;
   unsigned offset = 1;
