add support for comma-separated pattern specification

Signed-off-by: Justin Stitt <[email protected]>

Author: JustinStitt

clang/include/clang/AST/Expr.h

@@ -3860,7 +3860,7 @@ class CStyleCastExpr final
 class BinaryOperator : public Expr {
   enum { LHS, RHS, END_EXPR };
   Stmt *SubExprs[END_EXPR];
-  bool isOverflowIdiom;
+  bool ExcludedOverflowPattern;
 
 public:
   typedef BinaryOperatorKind Opcode;
@@ -4019,7 +4019,7 @@ class BinaryOperator : public Expr {
     return isShiftAssignOp(getOpcode());
   }
 
-  bool ignoreOverflowSanitizers() const { return isOverflowIdiom; }
+  bool ignoreOverflowSanitizers() const { return ExcludedOverflowPattern; }
 
   /// Return true if a binary operator using the specified opcode and operands
   /// would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized

clang/include/clang/Basic/LangOptions.h

@@ -367,6 +367,21 @@ 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
@@ -555,6 +570,11 @@ class LangOptions : public LangOptionsBase {
   /// The default stream kind used for HIP kernel launching.
   GPUDefaultStreamKind GPUDefaultStream;
 
+  /// Which overflow patterns should be excluded from sanitizer instrumentation
+  int OverflowPatternExclusionMask = 0;
+
+  std::vector<std::string> OverflowPatternExclusionValues;
+
   /// The seed used by the randomize structure layout feature.
   std::string RandstructSeed;
 
@@ -630,6 +650,14 @@ 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

@@ -2563,11 +2563,11 @@ defm sanitize_overflow_idioms : BoolFOption<"sanitize-overflow-idioms",
   PosFlag<SetTrue, [], [], "Enable">,
   NegFlag<SetFalse, [], [], "Disable">,
   BothFlags<[], [ClangOption], " the instrumentation of common overflow idioms">>;
-defm sanitize_negation_overflow : BoolFOption<"sanitize-negation-overflow",
-  LangOpts<"IgnoreNegationOverflow">, DefaultFalse,
-  PosFlag<SetFalse, [], [], "Enable">,
-  NegFlag<SetTrue, [], [], "Disable">,
-  BothFlags<[], [ClangOption], " integer overflow sanitizer instrumentation for negation">>;
+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,6 +33,7 @@ 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

@@ -4819,10 +4819,11 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
   BinaryOperatorBits.OpLoc = opLoc;
   SubExprs[LHS] = lhs;
   SubExprs[RHS] = rhs;
-  if (!Ctx.getLangOpts().SanitizeOverflowIdioms) {
+  if (Ctx.getLangOpts().isOverflowPatternExcluded(
+          LangOptions::OverflowPatternExclusionKind::AddOverflowTest)) {
     std::optional<BinaryOperator *> Result = getOverflowIdiomBinOp(this);
     if (Result.has_value())
-      Result.value()->isOverflowIdiom = true;
+      Result.value()->ExcludedOverflowPattern = true;
   }
   BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
   if (hasStoredFPFeatures())

clang/lib/CodeGen/CGExprScalar.cpp

@@ -198,8 +198,11 @@ 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 &&
-      Ctx.getLangOpts().IgnoreNegationOverflow)
+      Ctx.getLangOpts().isOverflowPatternExcluded(
+          LangOptions::OverflowPatternExclusionKind::NegUnsignedConst))
     return true;
 
   // If a unary op has a widened operand, the op cannot overflow.
@@ -2888,11 +2891,11 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
     QualType promotedType;
     bool canPerformLossyDemotionCheck = false;
 
-    // Does this match the pattern of while(i--) {...}? If so, and if
-    // SanitizeOverflowIdioms is disabled, we don't want to enable sanitizer.
+    // Is the pattern "while (i--)" and overflow exclusion?
     bool disableSanitizer =
         (!isInc && !isPre &&
-         !CGF.getContext().getLangOpts().SanitizeOverflowIdioms &&
+         CGF.getContext().getLangOpts().isOverflowPatternExcluded(
+             LangOptions::OverflowPatternExclusionKind::PostDecrInWhile) &&
          llvm::all_of(CGF.getContext().getParentMapContext().getParents(*E),
                       [&](const DynTypedNode &Parent) -> bool {
                         return Parent.get<WhileStmt>();

clang/lib/Driver/SanitizerArgs.cpp

@@ -119,6 +119,10 @@ static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
 static int parseCoverageFeatures(const Driver &D, const llvm::opt::Arg *A,
                                  bool DiagnoseErrors);
 
+static int parseOverflowPatternExclusionValues(const Driver &D,
+                                               const llvm::opt::Arg *A,
+                                               bool DiagnoseErrors);
+
 /// Parse -f(no-)?sanitize-metadata= flag values, diagnosing any invalid
 /// components. Returns OR of members of \c BinaryMetadataFeature enumeration.
 static int parseBinaryMetadataFeatures(const Driver &D, const llvm::opt::Arg *A,
@@ -788,6 +792,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
           << "fsanitize-trap=cfi";
   }
 
+  for (const auto *Arg :
+       Args.filtered(options::OPT_fsanitize_overflow_pattern_exclusion_EQ)) {
+    Arg->claim();
+    OverflowPatternExclusions |=
+        parseOverflowPatternExclusionValues(D, Arg, DiagnoseErrors);
+  }
+
   // Parse -f(no-)?sanitize-coverage flags if coverage is supported by the
   // enabled sanitizers.
   for (const auto *Arg : Args) {
@@ -1245,6 +1256,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
   addSpecialCaseListOpt(Args, CmdArgs,
                         "-fsanitize-system-ignorelist=", SystemIgnorelistFiles);
 
+  if (OverflowPatternExclusions)
+    Args.AddAllArgs(CmdArgs,
+                    options::OPT_fsanitize_overflow_pattern_exclusion_EQ);
+
   if (MsanTrackOrigins)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +
                                          Twine(MsanTrackOrigins)));
@@ -1433,6 +1448,28 @@ SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
   return Kinds;
 }
 
+static int parseOverflowPatternExclusionValues(const Driver &D,
+                                               const llvm::opt::Arg *A,
+                                               bool DiagnoseErrors) {
+  int Exclusions = 0;
+  for (int i = 0, n = A->getNumValues(); i != n; ++i) {
+    const char *Value = A->getValue(i);
+    int E =
+        llvm::StringSwitch<int>(Value)
+            .Case("none", LangOptionsBase::None)
+            .Case("all", LangOptionsBase::All)
+            .Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
+            .Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
+            .Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
+            .Default(0);
+    if (E == 0)
+      D.Diag(clang::diag::err_drv_unsupported_option_argument)
+          << A->getSpelling() << Value;
+    Exclusions |= E;
+  }
+  return Exclusions;
+}
+
 int parseCoverageFeatures(const Driver &D, const llvm::opt::Arg *A,
                           bool DiagnoseErrors) {
   assert(A->getOption().matches(options::OPT_fsanitize_coverage) ||

clang/lib/Driver/ToolChains/Clang.cpp

@@ -6844,10 +6844,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
-  if (Args.hasArg(options::OPT_fno_sanitize_overflow_idioms) &&
-      !Args.hasArg(options::OPT_fsanitize_negation_overflow))
-    CmdArgs.push_back("-fno-sanitize-negation-overflow");
-
   if (Args.getLastArg(options::OPT_fapple_kext) ||
       (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
     CmdArgs.push_back("-fapple-kext");
@@ -6901,9 +6897,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   Args.addOptInFlag(CmdArgs, options::OPT_mspeculative_load_hardening,
                     options::OPT_mno_speculative_load_hardening);
 
-  Args.AddLastArg(CmdArgs, options::OPT_fsanitize_negation_overflow,
-                  options::OPT_fno_sanitize_negation_overflow);
-
   RenderSSPOptions(D, TC, Args, CmdArgs, KernelOrKext);
   RenderSCPOptions(TC, Args, CmdArgs);
   RenderTrivialAutoVarInitOptions(D, TC, Args, CmdArgs);
@@ -7753,6 +7746,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     Args.AddLastArg(CmdArgs, options::OPT_fgpu_default_stream_EQ);
   }
 
+  Args.AddAllArgs(CmdArgs,
+                  options::OPT_fsanitize_overflow_pattern_exclusion_EQ);
+
   Args.AddLastArg(CmdArgs, options::OPT_foffload_uniform_block,
                   options::OPT_fno_offload_uniform_block);
 

clang/lib/Frontend/CompilerInvocation.cpp

@@ -4248,6 +4248,22 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
 
+  if (auto *A = Args.getLastArg(OPT_fsanitize_overflow_pattern_exclusion_EQ)) {
+    for (int i = 0, n = A->getNumValues(); i != n; ++i) {
+      StringRef Value = A->getValue(i);
+      if (Value == "none")
+        Opts.OverflowPatternExclusionMask |= LangOptionsBase::None;
+      else if (Value == "all")
+        Opts.OverflowPatternExclusionMask |= LangOptionsBase::All;
+      else if (Value == "add-overflow-test")
+        Opts.OverflowPatternExclusionMask |= LangOptionsBase::AddOverflowTest;
+      else if (Value == "negated-unsigned-const")
+        Opts.OverflowPatternExclusionMask |= LangOptionsBase::NegUnsignedConst;
+      else if (Value == "post-decr-while")
+        Opts.OverflowPatternExclusionMask |= LangOptionsBase::PostDecrInWhile;
+    }
+  }
+
   // Parse -fsanitize= arguments.
   parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
                       Diags, Opts.Sanitize);

clang/test/CodeGen/overflow-idiom-exclusion-fp.c

@@ -1,6 +1,6 @@
 // Check for potential false positives from patterns that _almost_ match classic overflow idioms
-// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fno-sanitize-overflow-idioms -S -emit-llvm -o - | FileCheck %s
-// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fno-sanitize-overflow-idioms -fwrapv -S -emit-llvm -o - | FileCheck %s
+// 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
 
 extern unsigned a, b, c;
 extern int u, v, w;

clang/test/CodeGen/overflow-idiom-exclusion.c

@@ -16,12 +16,25 @@
 // unsigned negation, for example:
 // unsigned long A = -1UL;
 
-// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fno-sanitize-overflow-idioms -S -emit-llvm -o - | FileCheck %s
-// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fno-sanitize-overflow-idioms -fwrapv -S -emit-llvm -o - | FileCheck %s
-// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fno-sanitize-overflow-idioms -S -emit-llvm -o - | FileCheck %s --check-prefix=NEGATION
-// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fno-sanitize-overflow-idioms -fsanitize-negation-overflow -S -emit-llvm -o - | FileCheck %s --check-prefix=NEGATIONOV
-// CHECK-NOT: br{{.*}}overflow
+// 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
+// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=add-overflow-test -S -emit-llvm -o - | FileCheck %s --check-prefix=ADD
+// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=negated-unsigned-const -S -emit-llvm -o - | FileCheck %s --check-prefix=NEGATE
+// RUN: %clang %s -O2 -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=post-decr-while -S -emit-llvm -o - | FileCheck %s --check-prefix=WHILE
 
+// CHECK-NOT: handle{{.*}}overflow
+
+// ADD: usub.with.overflow
+// ADD: negate_overflow
+// ADD-NOT: handler.add_overflow
+
+// NEGATE: handler.add_overflow
+// NEGATE: usub.with.overflow
+// NEGATE-NOT: negate_overflow
+
+// WHILE: handler.add_overflow
+// WHILE: negate_overflow
+// WHILE-NOT: usub.with.overflow
 extern unsigned a, b, c;
 extern unsigned some(void);
 
@@ -110,16 +123,13 @@ void common_while(unsigned i) {
   }
 }
 
-// NEGATION-LABEL,NEGATIONOV-LABEL: define{{.*}}negation_overflow
-// NEGATION-NOT: negate_overflow
-// NEGATIONOV: negate_overflow
 // Normally, these assignments would trip the unsigned overflow sanitizer.
-void negation_overflow(void) {
+void negation(void) {
 #define SOME -1UL
   unsigned long A = -1UL;
   unsigned long B = -2UL;
   unsigned long C = -3UL;
-  unsigned long D = -1337UL;
+  unsigned long D = -SOME;
   (void)A;(void)B;(void)C;(void)D;
 }