diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3c2e0282d1c72..b03d748307b75 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -294,6 +294,36 @@ 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 ---------------------- diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst index 531d56e313826..9f3d980eefbea 100644 --- a/clang/docs/UndefinedBehaviorSanitizer.rst +++ b/clang/docs/UndefinedBehaviorSanitizer.rst @@ -293,6 +293,48 @@ 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 ================= diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 5b813bfc2faf9..f5863524723a2 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4043,6 +4043,15 @@ 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()); diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index bbd7634bcc3bf..f1a2aac0a8b2f 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -650,6 +650,11 @@ 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; }; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 0035092ce0d86..4619e2d8c4fb7 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -400,6 +400,8 @@ 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, diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 91f1c2f2e6239..eb4cb4b5a7e93 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/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 + unsigned OverflowPatternExclusionMask = 0; + + std::vector 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(); diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c8c56dbb51b28..6bb9a0cfe7083 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2558,6 +2558,11 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats", "Disable">, BothFlags<[], [ClangOption], " sanitizer statistics gathering.">>, 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>; def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">, Group, HelpText<"Enable memory access instrumentation in ThreadSanitizer (default)">; diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index 47ef175302679..e64ec463ca890 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -33,6 +33,7 @@ class SanitizerArgs { std::vector BinaryMetadataIgnorelistFiles; int CoverageFeatures = 0; int BinaryMetadataFeatures = 0; + int OverflowPatternExclusions = 0; int MsanTrackOrigins = 0; bool MsanUseAfterDtor = true; bool MsanParamRetval = true; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 9d5b8167d0ee6..57475c66a94e3 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4759,6 +4759,53 @@ 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 +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(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, @@ -4768,8 +4815,15 @@ 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 Result = getOverflowPatternBinOp(this); + if (Result.has_value()) + Result.value()->BinaryOperatorBits.ExcludedOverflowPattern = 1; + } BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage(); if (hasStoredFPFeatures()) setStoredFPFeatures(FPFeatures); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 84392745ea614..6eac2b4c54e1b 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -24,6 +24,7 @@ #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" @@ -195,13 +196,24 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) { if (!Op.mayHaveIntegerOverflow()) return true; + const UnaryOperator *UO = dyn_cast(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 (const auto *UO = dyn_cast(Op.E)) + if (UO) 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(Op.E); + if (BO->hasExcludedOverflowPattern()) + return true; + auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS()); if (!OptionalLHSTy) return false; @@ -2766,6 +2778,26 @@ 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()) + return false; + + return true; +} + namespace { /// Handles check and update for lastprivate conditional variables. class OMPLastprivateConditionalUpdateRAII { @@ -2877,6 +2909,10 @@ 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."); @@ -2936,7 +2972,8 @@ 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)) { + CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && + !excludeOverflowPattern) { value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec( E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts()))); } else { diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 1fd870b72286e..a63ee944fd1bb 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/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) { @@ -1241,6 +1252,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))); @@ -1426,6 +1441,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(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) || diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 843d68c85bc59..6b88c5e833440 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -7746,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); diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index f6b6c44a4cab6..8ab7b69c00312 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -4248,6 +4248,19 @@ 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) { + Opts.OverflowPatternExclusionMask |= + llvm::StringSwitch(A->getValue(i)) + .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); + } + } + // Parse -fsanitize= arguments. parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ), Diags, Opts.Sanitize); diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index e1cba9e612be3..b14653540b85e 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1128,6 +1128,7 @@ void ASTStmtReader::VisitBinaryOperator(BinaryOperator *E) { (BinaryOperator::Opcode)CurrentUnpackingBits->getNextBits(/*Width=*/6)); bool hasFP_Features = CurrentUnpackingBits->getNextBit(); E->setHasStoredFPFeatures(hasFP_Features); + E->setExcludedOverflowPattern(CurrentUnpackingBits->getNextBit()); E->setLHS(Record.readSubExpr()); E->setRHS(Record.readSubExpr()); E->setOperatorLoc(readSourceLocation()); diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index ec667b58337ff..62041c1090e96 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -1063,6 +1063,7 @@ void ASTStmtWriter::VisitBinaryOperator(BinaryOperator *E) { CurrentPackingBits.addBits(E->getOpcode(), /*Width=*/6); bool HasFPFeatures = E->hasStoredFPFeatures(); CurrentPackingBits.addBit(HasFPFeatures); + CurrentPackingBits.addBit(E->hasExcludedOverflowPattern()); Record.AddStmt(E->getLHS()); Record.AddStmt(E->getRHS()); Record.AddSourceLocation(E->getOperatorLoc()); diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c new file mode 100644 index 0000000000000..d21405c56beab --- /dev/null +++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c @@ -0,0 +1,83 @@ +// 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 + +// 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); + +// Make sure all these still have handler paths, we shouldn't be excluding +// 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) + c = 9; + + // CHECK: br i1{{.*}}handler. + if (a - b < a) + c = 9; + + // CHECK: br i1{{.*}}handler. + if (a + b < a) + c = 9; + + // CHECK: br i1{{.*}}handler. + if (a + b + 1 < a) + c = 9; + + // CHECK: br i1{{.*}}handler. + // CHECK: br i1{{.*}}handler. + if (a + b < a + 1) + c = 9; + + // CHECK: br i1{{.*}}handler. + if (b >= a + b) + c = 9; + + // CHECK: br i1{{.*}}handler. + if (a + a < a) + c = 9; + + // CHECK: br i1{{.*}}handler. + if (a + b == a) + c = 9; + + // CHECK: br i1{{.*}}handler + // Although this can never actually overflow we are still checking that the + // sanitizer instruments it. + while (--a) + some(); +} + +// cvise'd kernel code that caused problems during development +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, + compressedCost = 3 + ZSTD_NCountCost(); + if (basicCost <= compressedCost) + 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 new file mode 100644 index 0000000000000..7c8c4af61029d --- /dev/null +++ b/clang/test/CodeGen/overflow-idiom-exclusion.c @@ -0,0 +1,151 @@ +// 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 + +// 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 +// the user is trying to handle it gracefully. + +// 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. + +// Another common pattern that, in some cases, is found to be too noisy is +// unsigned negation, for example: +// unsigned long A = -1UL; + + +// 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); + +void basic_commutativity(void) { + if (a + b < a) + c = 9; + if (a + b < b) + c = 9; + if (b + a < b) + c = 9; + if (b + a < a) + c = 9; + if (a > a + b) + c = 9; + if (a > b + a) + c = 9; + if (b > a + b) + c = 9; + if (b > b + a) + c = 9; +} + +void arguments_and_commutativity(unsigned V1, unsigned V2) { + if (V1 + V2 < V1) + c = 9; + if (V1 + V2 < V2) + c = 9; + if (V2 + V1 < V2) + c = 9; + if (V2 + V1 < V1) + c = 9; + if (V1 > V1 + V2) + c = 9; + if (V1 > V2 + V1) + c = 9; + if (V2 > V1 + V2) + c = 9; + if (V2 > V2 + V1) + c = 9; +} + +void pointers(unsigned *P1, unsigned *P2, unsigned V1) { + if (*P1 + *P2 < *P1) + c = 9; + if (*P1 + V1 < V1) + c = 9; + if (V1 + *P2 < *P2) + c = 9; +} + +struct OtherStruct { + unsigned foo, bar; +}; + +struct MyStruct { + unsigned base, offset; + struct OtherStruct os; +}; + +extern struct MyStruct ms; + +void structs(void) { + if (ms.base + ms.offset < ms.base) + c = 9; +} + +void nestedstructs(void) { + if (ms.os.foo + ms.os.bar < ms.os.foo) + c = 9; +} + +// Normally, this would be folded into a simple call to the overflow handler +// and a store. Excluding this pattern results in just a store. +void constants(void) { + unsigned base = 4294967295; + unsigned offset = 1; + if (base + offset < base) + c = 9; +} + +void common_while(unsigned i) { + // This post-decrement usually causes overflow sanitizers to trip on the very + // last operation. + while (i--) { + some(); + } +} + +// Normally, these assignments would trip the unsigned overflow sanitizer. +void negation(void) { +#define SOME -1UL + unsigned long A = -1UL; + unsigned long B = -2UL; + unsigned long C = -3UL; + unsigned long D = -SOME; + (void)A;(void)B;(void)C;(void)D; +} + +// cvise'd kernel code that caused problems during development due to sign +// extension +typedef unsigned long _size_t; +int qnbytes; +int *key_alloc_key; +_size_t key_alloc_quotalen; +int *key_alloc(void) { + if (qnbytes + key_alloc_quotalen < qnbytes) + return key_alloc_key; + return key_alloc_key + 3;; +} + +void function_call(void) { + if (b + some() < b) + c = 9; +}