Revert "[Clang] Overflow Pattern Exclusions (#100272)"

clang/docs/ReleaseNotes.rst

@@ -395,36 +395,6 @@ Moved checkers
 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
-     }
-
-  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
 ----------------------
 - Fixed an issue that led to crashes when calling ``Type.get_exception_specification_kind``.

clang/docs/UndefinedBehaviorSanitizer.rst

@@ -293,48 +293,6 @@ 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.
 
-Disabling instrumentation for common overflow patterns
-------------------------------------------------------
-
-There are certain overflow-dependent or overflow-prone code patterns which
-produce a lot of noise for integer overflow/truncation sanitizers. Negated
-unsigned constants, post-decrements in a while loop condition and simple
-overflow checks are accepted and pervasive code patterns. However, the signal
-received from sanitizers instrumenting these code patterns may be too noisy for
-some projects. To disable instrumentation for these common patterns one should
-use ``-fsanitize-overflow-pattern-exclusion=``.
-
-Currently, this option supports three overflow-dependent code idioms:
-
-``negated-unsigned-const``
-
-.. 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...
-
-``post-decr-while``
-
-.. 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
-
-``add-overflow-test``
-
-.. 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)
-
-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
 =================
 

clang/include/clang/AST/Expr.h

@@ -4043,15 +4043,6 @@ class BinaryOperator : public Expr {
   void setHasStoredFPFeatures(bool B) { BinaryOperatorBits.HasFPFeatures = B; }
   bool hasStoredFPFeatures() const { return BinaryOperatorBits.HasFPFeatures; }
 
-  /// Set and get the bit that informs arithmetic overflow sanitizers whether
-  /// or not they should exclude certain BinaryOperators from instrumentation
-  void setExcludedOverflowPattern(bool B) {
-    BinaryOperatorBits.ExcludedOverflowPattern = B;
-  }
-  bool hasExcludedOverflowPattern() const {
-    return BinaryOperatorBits.ExcludedOverflowPattern;
-  }
-
   /// Get FPFeatures from trailing storage
   FPOptionsOverride getStoredFPFeatures() const {
     assert(hasStoredFPFeatures());

clang/include/clang/AST/Stmt.h

@@ -650,11 +650,6 @@ class alignas(void *) Stmt {
     LLVM_PREFERRED_TYPE(bool)
     unsigned HasFPFeatures : 1;
 
-    /// Whether or not this BinaryOperator should be excluded from integer
-    /// overflow sanitization.
-    LLVM_PREFERRED_TYPE(bool)
-    unsigned ExcludedOverflowPattern : 1;
-
     SourceLocation OpLoc;
   };
 

clang/include/clang/Basic/LangOptions.def

@@ -406,8 +406,6 @@ VALUE_LANGOPT(TrivialAutoVarInitMaxSize, 32, 0,
              "stop trivial automatic variable initialization if var size exceeds the specified size (in bytes). Must be greater than 0.")
 ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2, SOB_Undefined,
              "signed integer overflow handling")
-LANGOPT(IgnoreNegationOverflow, 1, 0, "ignore overflow caused by negation")
-LANGOPT(SanitizeOverflowIdioms, 1, 1, "enable instrumentation for common overflow idioms")
 ENUM_LANGOPT(ThreadModel  , ThreadModelKind, 2, ThreadModelKind::POSIX, "Thread Model")
 
 BENIGN_LANGOPT(ArrowDepth, 32, 256,

clang/include/clang/Basic/LangOptions.h

@@ -367,21 +367,6 @@ class LangOptionsBase {
     PerThread,
   };
 
-  /// Exclude certain code patterns from being instrumented by arithmetic
-  /// overflow sanitizers
-  enum OverflowPatternExclusionKind {
-    /// Don't exclude any overflow patterns from sanitizers
-    None = 1 << 0,
-    /// Exclude all overflow patterns (below)
-    All = 1 << 1,
-    /// if (a + b < a)
-    AddOverflowTest = 1 << 2,
-    /// -1UL
-    NegUnsignedConst = 1 << 3,
-    /// while (count--)
-    PostDecrInWhile = 1 << 4,
-  };
-
   enum class DefaultVisiblityExportMapping {
     None,
     /// map only explicit default visibilities to exported
@@ -570,11 +555,6 @@ class LangOptions : public LangOptionsBase {
   /// The default stream kind used for HIP kernel launching.
   GPUDefaultStreamKind GPUDefaultStream;
 
-  /// Which overflow patterns should be excluded from sanitizer instrumentation
-  unsigned OverflowPatternExclusionMask = 0;
-
-  std::vector<std::string> OverflowPatternExclusionValues;
-
   /// The seed used by the randomize structure layout feature.
   std::string RandstructSeed;
 
@@ -650,14 +630,6 @@ class LangOptions : public LangOptionsBase {
     return MSCompatibilityVersion >= MajorVersion * 100000U;
   }
 
-  bool isOverflowPatternExcluded(OverflowPatternExclusionKind Kind) const {
-    if (OverflowPatternExclusionMask & OverflowPatternExclusionKind::None)
-      return false;
-    if (OverflowPatternExclusionMask & OverflowPatternExclusionKind::All)
-      return true;
-    return OverflowPatternExclusionMask & Kind;
-  }
-
   /// Reset all of the options that are not considered when building a
   /// module.
   void resetNonModularOptions();

clang/include/clang/Driver/Options.td

@@ -2565,11 +2565,6 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats",
           "Disable">,
   BothFlags<[], [ClangOption], " sanitizer statistics gathering.">>,
   Group<f_clang_Group>;
-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]>,
-  Values<"none,all,add-overflow-test,negated-unsigned-const,post-decr-while">,
-  MarshallingInfoStringVector<LangOpts<"OverflowPatternExclusionValues">>;
 def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
                                      Group<f_clang_Group>,
                                      HelpText<"Enable memory access instrumentation in ThreadSanitizer (default)">;

clang/include/clang/Driver/SanitizerArgs.h

@@ -33,7 +33,6 @@ class SanitizerArgs {
   std::vector<std::string> BinaryMetadataIgnorelistFiles;
   int CoverageFeatures = 0;
   int BinaryMetadataFeatures = 0;
-  int OverflowPatternExclusions = 0;
   int MsanTrackOrigins = 0;
   bool MsanUseAfterDtor = true;
   bool MsanParamRetval = true;

clang/lib/AST/Expr.cpp

@@ -4759,53 +4759,6 @@ ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx,
   return new (Mem) ParenListExpr(EmptyShell(), NumExprs);
 }
 
-/// 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 *>
-getOverflowPatternBinOp(const BinaryOperator *E) {
-  Expr *Addition, *ComparedTo;
-  if (E->getOpcode() == BO_LT) {
-    Addition = E->getLHS();
-    ComparedTo = E->getRHS();
-  } else if (E->getOpcode() == BO_GT) {
-    Addition = E->getRHS();
-    ComparedTo = E->getLHS();
-  } else {
-    return {};
-  }
-
-  const Expr *AddLHS = nullptr, *AddRHS = nullptr;
-  BinaryOperator *BO = dyn_cast<BinaryOperator>(Addition);
-
-  if (BO && BO->getOpcode() == clang::BO_Add) {
-    // now store addends for lookup on other side of '>'
-    AddLHS = BO->getLHS();
-    AddRHS = BO->getRHS();
-  }
-
-  if (!AddLHS || !AddRHS)
-    return {};
-
-  const Decl *LHSDecl, *RHSDecl, *OtherDecl;
-
-  LHSDecl = AddLHS->IgnoreParenImpCasts()->getReferencedDeclOfCallee();
-  RHSDecl = AddRHS->IgnoreParenImpCasts()->getReferencedDeclOfCallee();
-  OtherDecl = ComparedTo->IgnoreParenImpCasts()->getReferencedDeclOfCallee();
-
-  if (!OtherDecl)
-    return {};
-
-  if (!LHSDecl && !RHSDecl)
-    return {};
-
-  if ((LHSDecl && LHSDecl == OtherDecl && LHSDecl != RHSDecl) ||
-      (RHSDecl && RHSDecl == OtherDecl && RHSDecl != LHSDecl))
-    return BO;
-  return {};
-}
-
 BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
                                Opcode opc, QualType ResTy, ExprValueKind VK,
                                ExprObjectKind OK, SourceLocation opLoc,
@@ -4815,15 +4768,8 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
   assert(!isCompoundAssignmentOp() &&
          "Use CompoundAssignOperator for compound assignments");
   BinaryOperatorBits.OpLoc = opLoc;
-  BinaryOperatorBits.ExcludedOverflowPattern = 0;
   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 = 1;
-  }
   BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
   if (hasStoredFPFeatures())
     setStoredFPFeatures(FPFeatures);

clang/lib/CodeGen/CGExprScalar.cpp

@@ -24,7 +24,6 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
-#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -196,24 +195,13 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) {
   if (!Op.mayHaveIntegerOverflow())
     return true;
 
-  const UnaryOperator *UO = dyn_cast<UnaryOperator>(Op.E);
-
-  if (UO && UO->getOpcode() == UO_Minus &&
-      Ctx.getLangOpts().isOverflowPatternExcluded(
-          LangOptions::OverflowPatternExclusionKind::NegUnsignedConst) &&
-      UO->isIntegerConstantExpr(Ctx))
-    return true;
-
   // If a unary op has a widened operand, the op cannot overflow.
-  if (UO)
+  if (const auto *UO = dyn_cast<UnaryOperator>(Op.E))
     return !UO->canOverflow();
 
   // We usually don't need overflow checks for binops with widened operands.
   // Multiplication with promoted unsigned operands is a special case.
   const auto *BO = cast<BinaryOperator>(Op.E);
-  if (BO->hasExcludedOverflowPattern())
-    return true;
-
   auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS());
   if (!OptionalLHSTy)
     return false;
@@ -2778,26 +2766,6 @@ llvm::Value *ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior(
   llvm_unreachable("Unknown SignedOverflowBehaviorTy");
 }
 
-/// For the purposes of overflow pattern exclusion, does this match the
-/// "while(i--)" pattern?
-static bool matchesPostDecrInWhile(const UnaryOperator *UO, bool isInc,
-                                   bool isPre, ASTContext &Ctx) {
-  if (isInc || isPre)
-    return false;
-
-  // -fsanitize-overflow-pattern-exclusion=post-decr-while
-  if (!Ctx.getLangOpts().isOverflowPatternExcluded(
-          LangOptions::OverflowPatternExclusionKind::PostDecrInWhile))
-    return false;
-
-  // all Parents (usually just one) must be a WhileStmt
-  for (const auto &Parent : Ctx.getParentMapContext().getParents(*UO))
-    if (!Parent.get<WhileStmt>())
-      return false;
-
-  return true;
-}
-
 namespace {
 /// Handles check and update for lastprivate conditional variables.
 class OMPLastprivateConditionalUpdateRAII {
@@ -2909,10 +2877,6 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
   } else if (type->isIntegerType()) {
     QualType promotedType;
     bool canPerformLossyDemotionCheck = false;
-
-    bool excludeOverflowPattern =
-        matchesPostDecrInWhile(E, isInc, isPre, CGF.getContext());
-
     if (CGF.getContext().isPromotableIntegerType(type)) {
       promotedType = CGF.getContext().getPromotedIntegerType(type);
       assert(promotedType != type && "Shouldn't promote to the same type.");
@@ -2972,8 +2936,7 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
     } else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
       value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
     } else if (E->canOverflow() && type->isUnsignedIntegerType() &&
-               CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
-               !excludeOverflowPattern) {
+               CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
       value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
           E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
     } else {