https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/100272
>From 06b702cd38943314b2e6f873e64d70baed6f57f7 Mon Sep 17 00:00:00 2001 From: Justin Stitt <justinst...@google.com> Date: Tue, 23 Jul 2024 20:21:49 +0000 Subject: [PATCH 1/3] implement idiom exclusions Add flag `-fno-sanitize-overflow-idioms` which disables integer overflow/truncation sanitizer instrumentation for common overflow-dependent code patterns. Signed-off-by: Justin Stitt <justinst...@google.com> --- clang/include/clang/AST/Expr.h | 5 + clang/include/clang/Basic/LangOptions.def | 2 + clang/include/clang/Driver/Options.td | 10 ++ clang/include/clang/Driver/SanitizerArgs.h | 1 + clang/lib/AST/Expr.cpp | 53 +++++++ clang/lib/CodeGen/CGExprScalar.cpp | 26 +++- clang/lib/Driver/SanitizerArgs.cpp | 7 + clang/lib/Driver/ToolChains/Clang.cpp | 7 + .../CodeGen/overflow-idiom-exclusion-fp.c | 77 ++++++++++ clang/test/CodeGen/overflow-idiom-exclusion.c | 141 ++++++++++++++++++ 10 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 clang/test/CodeGen/overflow-idiom-exclusion-fp.c create mode 100644 clang/test/CodeGen/overflow-idiom-exclusion.c diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 5b813bfc2faf9..c329ee061cf00 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -3860,6 +3860,7 @@ class CStyleCastExpr final class BinaryOperator : public Expr { enum { LHS, RHS, END_EXPR }; Stmt *SubExprs[END_EXPR]; + bool isOverflowIdiom; public: typedef BinaryOperatorKind Opcode; @@ -4018,6 +4019,10 @@ class BinaryOperator : public Expr { return isShiftAssignOp(getOpcode()); } + bool ignoreOverflowSanitizers() const { + return isOverflowIdiom; + } + /// 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 /// integer to a pointer. diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 834a6f6cd43e3..675350a99024a 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -402,6 +402,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/Driver/Options.td b/clang/include/clang/Driver/Options.td index 69269cf7537b0..81a6baa1a2eae 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2559,6 +2559,16 @@ 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">>; +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_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 47ef175302679..291739e1221d9 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -64,6 +64,7 @@ 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 index 9d5b8167d0ee6..c07560c92100d 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4759,6 +4759,54 @@ ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx, return new (Mem) ParenListExpr(EmptyShell(), NumExprs); } +namespace { +/// Certain overflow-dependent code idioms 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) { + 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 {}; +} +} // namespace + BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy, ExprValueKind VK, ExprObjectKind OK, SourceLocation opLoc, @@ -4770,6 +4818,11 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, BinaryOperatorBits.OpLoc = opLoc; SubExprs[LHS] = lhs; SubExprs[RHS] = rhs; + if (!Ctx.getLangOpts().SanitizeOverflowIdioms) { + std::optional<BinaryOperator*> Result = getOverflowIdiomBinOp(this); + if (Result.has_value()) + Result.value()->isOverflowIdiom = true; + } BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage(); if (hasStoredFPFeatures()) setStoredFPFeatures(FPFeatures); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index a17d68424bbce..be0307073f3b9 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -25,6 +25,7 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" #include "clang/AST/RecordLayout.h" +#include "clang/AST/ParentMapContext.h" #include "clang/AST/StmtVisitor.h" #include "clang/Basic/CodeGenOptions.h" #include "clang/Basic/TargetInfo.h" @@ -195,13 +196,22 @@ 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().IgnoreNegationOverflow) + return true; + // If a unary op has a widened operand, the op cannot overflow. - if (const auto *UO = dyn_cast<UnaryOperator>(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<BinaryOperator>(Op.E); + if (BO->ignoreOverflowSanitizers()) + return true; + auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS()); if (!OptionalLHSTy) return false; @@ -2876,6 +2886,17 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, } else if (type->isIntegerType()) { 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. + bool disableSanitizer = + (!isInc && !isPre && + !CGF.getContext().getLangOpts().SanitizeOverflowIdioms && + llvm::all_of(CGF.getContext().getParentMapContext().getParents(*E), + [&](const DynTypedNode &Parent) -> bool { + return Parent.get<WhileStmt>(); + })); + if (CGF.getContext().isPromotableIntegerType(type)) { promotedType = CGF.getContext().getPromotedIntegerType(type); assert(promotedType != type && "Shouldn't promote to the same type."); @@ -2935,7 +2956,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) && + !disableSanitizer) { 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..567840bcfbbdb 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -1069,6 +1069,10 @@ 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; @@ -1356,6 +1360,9 @@ 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/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 78936fd634f33..198947c2431c3 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6800,6 +6800,10 @@ 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"); @@ -6853,6 +6857,9 @@ 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); 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..0fa847f733a7f --- /dev/null +++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c @@ -0,0 +1,77 @@ +// 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 + +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" idioms. +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. + 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 + 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. + while (--a) + some(); +} + +// 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); + +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; +} + +void function_calls(void) { + // CHECK: br i1{{.*}}handler + if (some() + b < some()) + c = 9; +} diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/overflow-idiom-exclusion.c new file mode 100644 index 0000000000000..df138fb8f5db8 --- /dev/null +++ b/clang/test/CodeGen/overflow-idiom-exclusion.c @@ -0,0 +1,141 @@ +// 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 +// 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. Using -fno-sanitize-overflow-idioms or not doesn't change this +// fact -- we just won't warn/trap with sanitizers. + +// Another common pattern that, in some cases, is found to be too noisy is +// 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 + +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 idiom 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(); + } +} + +// 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) { +#define SOME -1UL + unsigned long A = -1UL; + unsigned long B = -2UL; + unsigned long C = -3UL; + unsigned long D = -1337UL; + (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; +} >From 1dd1cbb1b13d0b038fa511620af993238a5bb260 Mon Sep 17 00:00:00 2001 From: Justin Stitt <justinst...@google.com> Date: Tue, 23 Jul 2024 22:44:47 +0000 Subject: [PATCH 2/3] add docs Signed-off-by: Justin Stitt <justinst...@google.com> --- clang/docs/ReleaseNotes.rst | 26 ++++++++++++++++++ clang/docs/UndefinedBehaviorSanitizer.rst | 32 +++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ac1de0db9ce48..443ab1456cfcb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -230,6 +230,32 @@ 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. + + .. code-block:: c++ + + void negation_overflow() { + unsigned long foo = -1UL; // No longer causes a negation overflow warning + unsigned long bar = -2UL; // and so on... + } + + 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``. + Python Binding Changes ---------------------- diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst index 531d56e313826..b856f601aec81 100644 --- a/clang/docs/UndefinedBehaviorSanitizer.rst +++ b/clang/docs/UndefinedBehaviorSanitizer.rst @@ -312,6 +312,38 @@ This attribute may not be supported by other compilers, so consider using it together with ``#if defined(__clang__)``. +Disabling instrumentation of common overflow idioms +===================================================== + +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``. + +Currently, this option handles three pervasive overflow-dependent code idioms: + +.. code-block:: c++ + + unsigned long foo = -1UL; // No longer causes a negation overflow warning + unsigned long bar = -2UL; // and so on... + +.. code-block:: c++ + + unsigned char count = 16; + while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip + +.. code-block:: c++ + + 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. + Suppressing Errors in Recompiled Code (Ignorelist) -------------------------------------------------- >From ac15444b318f38da1e5cc0e8bb7b5af53bc68971 Mon Sep 17 00:00:00 2001 From: Justin Stitt <justinst...@google.com> Date: Tue, 23 Jul 2024 23:08:23 +0000 Subject: [PATCH 3/3] format Signed-off-by: Justin Stitt <justinst...@google.com> --- clang/include/clang/AST/Expr.h | 4 +--- clang/lib/AST/Expr.cpp | 5 +++-- clang/lib/CodeGen/CGExprScalar.cpp | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index c329ee061cf00..e8dc22c51a03d 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4019,9 +4019,7 @@ class BinaryOperator : public Expr { return isShiftAssignOp(getOpcode()); } - bool ignoreOverflowSanitizers() const { - return isOverflowIdiom; - } + bool ignoreOverflowSanitizers() const { return isOverflowIdiom; } /// 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 diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index c07560c92100d..d963d473442d8 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4764,7 +4764,8 @@ namespace { /// 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) { +static std::optional<BinaryOperator *> +getOverflowIdiomBinOp(const BinaryOperator *E) { Expr *Addition, *ComparedTo; if (E->getOpcode() == BO_LT) { Addition = E->getLHS(); @@ -4819,7 +4820,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, SubExprs[LHS] = lhs; SubExprs[RHS] = rhs; if (!Ctx.getLangOpts().SanitizeOverflowIdioms) { - std::optional<BinaryOperator*> Result = getOverflowIdiomBinOp(this); + std::optional<BinaryOperator *> Result = getOverflowIdiomBinOp(this); if (Result.has_value()) Result.value()->isOverflowIdiom = true; } diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index be0307073f3b9..43a3a333fa76f 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -24,8 +24,8 @@ #include "clang/AST/Attr.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" -#include "clang/AST/RecordLayout.h" #include "clang/AST/ParentMapContext.h" +#include "clang/AST/RecordLayout.h" #include "clang/AST/StmtVisitor.h" #include "clang/Basic/CodeGenOptions.h" #include "clang/Basic/TargetInfo.h" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits