llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver Author: Vitaly Buka (vitalybuka) <details> <summary>Changes</summary> This reverts commit 9a666deecb9ff6ca3a6b12e6c2877e19b74b54da. --- Patch is 24.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/104472.diff 16 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (-30) - (modified) clang/docs/UndefinedBehaviorSanitizer.rst (-42) - (modified) clang/include/clang/AST/Expr.h (-9) - (modified) clang/include/clang/AST/Stmt.h (-5) - (modified) clang/include/clang/Basic/LangOptions.def (-2) - (modified) clang/include/clang/Basic/LangOptions.h (-28) - (modified) clang/include/clang/Driver/Options.td (-5) - (modified) clang/include/clang/Driver/SanitizerArgs.h (-1) - (modified) clang/lib/AST/Expr.cpp (-54) - (modified) clang/lib/CodeGen/CGExprScalar.cpp (+2-39) - (modified) clang/lib/Driver/SanitizerArgs.cpp (-37) - (modified) clang/lib/Driver/ToolChains/Clang.cpp (-3) - (modified) clang/lib/Frontend/CompilerInvocation.cpp (-13) - (modified) clang/lib/Serialization/ASTReaderStmt.cpp (-1) - (modified) clang/lib/Serialization/ASTWriterStmt.cpp (-1) - (removed) clang/test/CodeGen/overflow-idiom-exclusion-fp.c (-83) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b6b7dd5705637a..4872dbb7a556ad 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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``. diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst index 9f3d980eefbea7..531d56e313826c 100644 --- a/clang/docs/UndefinedBehaviorSanitizer.rst +++ b/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 ================= diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index f5863524723a2e..5b813bfc2faf90 100644 --- a/clang/include/clang/AST/Expr.h +++ b/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()); diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index f1a2aac0a8b2f8..bbd7634bcc3bfb 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/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; }; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 2e9f2c552aad8a..d454a7ff2f8cf4 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/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, diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index eb4cb4b5a7e93f..91f1c2f2e6239e 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/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(); diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 73e19b65dededb..40df91dc3fe0e3 100644 --- a/clang/include/clang/Driver/Options.td +++ b/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)">; diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index e64ec463ca8907..47ef175302679f 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/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; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 57475c66a94e35..9d5b8167d0ee62 100644 --- a/clang/lib/AST/Expr.cpp +++ b/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); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 6eac2b4c54e1ba..84392745ea6144 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/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 { diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index a63ee944fd1bb4..1fd870b72286e5 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -119,10 +119,6 @@ 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, @@ -792,13 +788,6 @@ 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) { @@ -1252,10 +1241,6 @@ 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))); @@ -1441,28 +1426,6 @@ 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) || diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index f2bc11839edd4d..96aa930ea28612 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -7769,9 +7769,6 @@ 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 5a5f5cb79a12f2..e3911c281985b7 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -4267,19 +4267,6 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, Diags.Report(diag::err_drv_invalid_value... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/104472 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
