[clang] [Clang] Re-land Overflow Pattern Exclusions (PR #104889)
Bill Wendling via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 20 13:13:32 PDT 2024
https://github.com/bwendling updated https://github.com/llvm/llvm-project/pull/104889
>From 1034d3afd71822bc9975579519abc889276108a1 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 23 Jul 2024 20:21:49 +0000
Subject: [PATCH 01/20] 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 <justinstitt at 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 d454a7ff2f8cf..2e9f2c552aad8 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -406,6 +406,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 6df3a6a5943a9..2d4322b51035b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2565,6 +2565,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 84392745ea614..089f841bb3acd 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;
@@ -2877,6 +2887,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.");
@@ -2936,7 +2957,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 96aa930ea2861..e423267855075 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6861,6 +6861,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");
@@ -6914,6 +6918,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 b7cd3110e86cb1aa40895c63a3a4b5f6f24f61f5 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 23 Jul 2024 22:44:47 +0000
Subject: [PATCH 02/20] add docs
Signed-off-by: Justin Stitt <justinstitt at 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 7c4451d93394c..cd0549bf303be 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -392,6 +392,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
----------------------
- 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 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 531eef6fde8d6e14e41a6d7961784469dfd786f7 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 23 Jul 2024 23:08:23 +0000
Subject: [PATCH 03/20] format
Signed-off-by: Justin Stitt <justinstitt at 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 089f841bb3acd..f069039d3702a 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"
>From c930ff285cd1b4f3e69224ce6aae8fb98d41f3a8 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Fri, 26 Jul 2024 00:06:03 +0000
Subject: [PATCH 04/20] add support for comma-separated pattern specification
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 4 +-
clang/include/clang/Basic/LangOptions.h | 28 ++++++++++++++
clang/include/clang/Driver/Options.td | 10 ++---
clang/include/clang/Driver/SanitizerArgs.h | 1 +
clang/lib/AST/Expr.cpp | 5 ++-
clang/lib/CodeGen/CGExprScalar.cpp | 11 ++++--
clang/lib/Driver/SanitizerArgs.cpp | 37 +++++++++++++++++++
clang/lib/Driver/ToolChains/Clang.cpp | 10 ++---
clang/lib/Frontend/CompilerInvocation.cpp | 16 ++++++++
.../CodeGen/overflow-idiom-exclusion-fp.c | 4 +-
clang/test/CodeGen/overflow-idiom-exclusion.c | 30 ++++++++++-----
11 files changed, 124 insertions(+), 32 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index e8dc22c51a03d..b25a4368aa152 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/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
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 91f1c2f2e6239..d57a9d7f31c73 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
+ 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();
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2d4322b51035b..6fb41cd7de4d7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2570,11 +2570,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)">;
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 291739e1221d9..1e23a6a6f85f1 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/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;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index d963d473442d8..8069cf80ea505 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/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())
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index f069039d3702a..f1e71557fb5ad 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/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>();
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 567840bcfbbdb..3b1c0b645ffbe 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) {
@@ -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) ||
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index e423267855075..f2bc11839edd4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6861,10 +6861,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");
@@ -6918,9 +6914,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);
@@ -7776,6 +7769,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 e3911c281985b..24842b0a769fc 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4267,6 +4267,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);
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
index 0fa847f733a7f..7bd7f94a3f039 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
+++ b/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;
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/overflow-idiom-exclusion.c
index df138fb8f5db8..d09e8c364963c 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion.c
+++ b/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;
}
>From 9a844edb4bd7ac4df541642c2a3b164c5b633618 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 29 Jul 2024 20:19:30 +0000
Subject: [PATCH 05/20] remove more instances of 'idiom'
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/docs/ReleaseNotes.rst | 28 ++++----
clang/docs/UndefinedBehaviorSanitizer.rst | 64 ++++++++++---------
clang/include/clang/Driver/Options.td | 5 --
clang/include/clang/Driver/SanitizerArgs.h | 1 -
clang/lib/AST/Expr.cpp | 6 +-
clang/lib/CodeGen/CGExprScalar.cpp | 4 +-
clang/lib/Driver/SanitizerArgs.cpp | 7 --
.../CodeGen/overflow-idiom-exclusion-fp.c | 20 ++++--
clang/test/CodeGen/overflow-idiom-exclusion.c | 11 ++--
9 files changed, 71 insertions(+), 75 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cd0549bf303be..f5696d6ce15dc 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -392,31 +392,35 @@ 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.
+- 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
}
- 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``.
+ 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 b856f601aec81..2b15d14ff449b 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -293,56 +293,58 @@ 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.
-Issue Suppression
-=================
-
-UndefinedBehaviorSanitizer is not expected to produce false positives.
-If you see one, look again; most likely it is a true positive!
-
-Disabling Instrumentation with ``__attribute__((no_sanitize("undefined")))``
-----------------------------------------------------------------------------
-
-You disable UBSan checks for particular functions with
-``__attribute__((no_sanitize("undefined")))``. You can use all values of
-``-fsanitize=`` flag in this attribute, e.g. if your function deliberately
-contains possible signed integer overflow, you can use
-``__attribute__((no_sanitize("signed-integer-overflow")))``.
-
-This attribute may not be
-supported by other compilers, so consider using it together with
-``#if defined(__clang__)``.
-
-Disabling instrumentation of common overflow idioms
-=====================================================
+Disabling instrumentation for common overflow patterns
+------------------------------------------------------
-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``.
+There are certain overflow-dependent or overflow-prone code patterns which
+produce a lot of noise for integer overflow/truncation sanitizers. To disable
+instrumentation for these common patterns one should use
+``-fsanitize-overflow-pattern-exclusion=``.
-Currently, this option handles three pervasive overflow-dependent code idioms:
+Currently, this option supports three pervasive overflow-dependent code idioms:
.. 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...
.. 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
.. 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)
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.
+simple overflow checks are accepted and pervasive code patterns. 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
+=================
+
+UndefinedBehaviorSanitizer is not expected to produce false positives.
+If you see one, look again; most likely it is a true positive!
+
+Disabling Instrumentation with ``__attribute__((no_sanitize("undefined")))``
+----------------------------------------------------------------------------
+
+You disable UBSan checks for particular functions with
+``__attribute__((no_sanitize("undefined")))``. You can use all values of
+``-fsanitize=`` flag in this attribute, e.g. if your function deliberately
+contains possible signed integer overflow, you can use
+``__attribute__((no_sanitize("signed-integer-overflow")))``.
+
+This attribute may not be
+supported by other compilers, so consider using it together with
+``#if defined(__clang__)``.
Suppressing Errors in Recompiled Code (Ignorelist)
--------------------------------------------------
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 6fb41cd7de4d7..acc1f2fde5397 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>;
-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">>;
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]>,
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 1e23a6a6f85f1..e64ec463ca890 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -65,7 +65,6 @@ 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 8069cf80ea505..ffe10ecb24173 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4760,12 +4760,12 @@ ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx,
}
namespace {
-/// Certain overflow-dependent code idioms can have their integer overflow
+/// 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 *>
-getOverflowIdiomBinOp(const BinaryOperator *E) {
+getOverflowPatternBinOp(const BinaryOperator *E) {
Expr *Addition, *ComparedTo;
if (E->getOpcode() == BO_LT) {
Addition = E->getLHS();
@@ -4821,7 +4821,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
SubExprs[RHS] = rhs;
if (Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::AddOverflowTest)) {
- std::optional<BinaryOperator *> Result = getOverflowIdiomBinOp(this);
+ std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(this);
if (Result.has_value())
Result.value()->ExcludedOverflowPattern = true;
}
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index f1e71557fb5ad..3064e744be066 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -198,9 +198,7 @@ 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 &&
+ if (UO && UO->getOpcode() == UO_Minus && UO->isIntegerConstantExpr(Ctx) &&
Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::NegUnsignedConst))
return true;
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 3b1c0b645ffbe..a63ee944fd1bb 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1080,10 +1080,6 @@ 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;
@@ -1375,9 +1371,6 @@ 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/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
index 7bd7f94a3f039..621d8f43babd8 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
@@ -1,4 +1,4 @@
-// Check for potential false positives from patterns that _almost_ match classic overflow idioms
+// Check for potential false positives from patterns that _almost_ match classic overflow-dependent or overflow-prone code patterns
// 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
@@ -8,7 +8,8 @@ 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.
+// 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)
@@ -26,6 +27,7 @@ void close_but_not_quite(void) {
if (a + b + 1 < a)
c = 9;
+ // CHECK: br i1{{.*}}handler.
// CHECK: br i1{{.*}}handler.
if (a + b < a + 1)
c = 9;
@@ -42,10 +44,6 @@ void close_but_not_quite(void) {
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.
@@ -54,7 +52,6 @@ void close_but_not_quite(void) {
}
// 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;
@@ -62,6 +59,8 @@ 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,
@@ -70,8 +69,15 @@ void ZSTD_selectEncodingType(void) {
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
index d09e8c364963c..e753343542a06 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion.c
@@ -1,6 +1,6 @@
-// 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
+// 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
@@ -9,8 +9,7 @@
// 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.
+// wrap-around.
// Another common pattern that, in some cases, is found to be too noisy is
// unsigned negation, for example:
@@ -107,7 +106,7 @@ void nestedstructs(void) {
}
// Normally, this would be folded into a simple call to the overflow handler
-// and a store. Excluding this idiom results in just a store.
+// and a store. Excluding this pattern results in just a store.
void constants(void) {
unsigned base = 4294967295;
unsigned offset = 1;
>From a994c8f969438a3eb6ae103751affea65a245ed8 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Thu, 1 Aug 2024 21:46:13 +0000
Subject: [PATCH 06/20] default initialize field
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index b25a4368aa152..31572cd4465eb 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/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 ExcludedOverflowPattern;
+ bool ExcludedOverflowPattern = false;
public:
typedef BinaryOperatorKind Opcode;
>From d22332e001f2e4606a513c56f5dda890c3bac76e Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Thu, 1 Aug 2024 23:12:42 +0000
Subject: [PATCH 07/20] update tests to not redefine size_t
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/test/CodeGen/overflow-idiom-exclusion-fp.c | 6 +++---
clang/test/CodeGen/overflow-idiom-exclusion.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
index 621d8f43babd8..e6b729e0c788e 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
@@ -52,17 +52,17 @@ void close_but_not_quite(void) {
}
// cvise'd kernel code that caused problems during development
-typedef unsigned size_t;
+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);
+_size_t ZSTD_NCountCost(void);
// CHECK-LABEL: ZSTD_selectEncodingType
// CHECK: br i1{{.*}}handler
void ZSTD_selectEncodingType(void) {
- size_t basicCost =
+ _size_t basicCost =
ZSTD_selectEncodingType_isDefaultAllowed ? ZSTD_NCountCost() : 0,
compressedCost = 3 + ZSTD_NCountCost();
if (basicCost <= compressedCost)
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/overflow-idiom-exclusion.c
index e753343542a06..c93da342f9149 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion.c
@@ -134,10 +134,10 @@ void negation(void) {
// cvise'd kernel code that caused problems during development due to sign
// extension
-typedef unsigned long size_t;
+typedef unsigned long _size_t;
int qnbytes;
int *key_alloc_key;
-size_t key_alloc_quotalen;
+_size_t key_alloc_quotalen;
int *key_alloc(void) {
if (qnbytes + key_alloc_quotalen < qnbytes)
return key_alloc_key;
>From 6546875b1c5137fde3a1f461815cc013a1c66a76 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Thu, 8 Aug 2024 17:29:00 -0700
Subject: [PATCH 08/20] separate logic into function and prefer StringSwitch
Addressing Bill's feedback:
* separate while(i--) disableSanitizer logic into a function, rename this
field.
* slightly rewrite some docs by adding titles
* Prefer stringswitch to if-else chain
* move comments within tests to after RUN lines
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/docs/UndefinedBehaviorSanitizer.rst | 26 ++++++++++-----
clang/include/clang/Basic/LangOptions.h | 2 +-
clang/lib/CodeGen/CGExprScalar.cpp | 33 +++++++++++++------
clang/lib/Frontend/CompilerInvocation.cpp | 19 +++++------
.../CodeGen/overflow-idiom-exclusion-fp.c | 2 +-
clang/test/CodeGen/overflow-idiom-exclusion.c | 11 ++++---
6 files changed, 56 insertions(+), 37 deletions(-)
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index 2b15d14ff449b..9f3d980eefbea 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -297,11 +297,16 @@ 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. To disable
-instrumentation for these common patterns one should use
-``-fsanitize-overflow-pattern-exclusion=``.
+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 pervasive overflow-dependent code idioms:
+Currently, this option supports three overflow-dependent code idioms:
+
+``negated-unsigned-const``
.. code-block:: c++
@@ -309,23 +314,26 @@ Currently, this option supports three pervasive overflow-dependent code idioms:
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)
-Negated unsigned constants, post-decrements in a while loop condition and
-simple overflow checks are accepted and pervasive code patterns. 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.
+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/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index d57a9d7f31c73..eb4cb4b5a7e93 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -571,7 +571,7 @@ class LangOptions : public LangOptionsBase {
GPUDefaultStreamKind GPUDefaultStream;
/// Which overflow patterns should be excluded from sanitizer instrumentation
- int OverflowPatternExclusionMask = 0;
+ unsigned OverflowPatternExclusionMask = 0;
std::vector<std::string> OverflowPatternExclusionValues;
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 3064e744be066..4695228b1b9e5 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2777,6 +2777,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<WhileStmt>())
+ return false;
+
+ return true;
+}
+
namespace {
/// Handles check and update for lastprivate conditional variables.
class OMPLastprivateConditionalUpdateRAII {
@@ -2889,15 +2909,8 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
QualType promotedType;
bool canPerformLossyDemotionCheck = false;
- // Is the pattern "while (i--)" and overflow exclusion?
- bool disableSanitizer =
- (!isInc && !isPre &&
- CGF.getContext().getLangOpts().isOverflowPatternExcluded(
- LangOptions::OverflowPatternExclusionKind::PostDecrInWhile) &&
- llvm::all_of(CGF.getContext().getParentMapContext().getParents(*E),
- [&](const DynTypedNode &Parent) -> bool {
- return Parent.get<WhileStmt>();
- }));
+ bool excludeOverflowPattern =
+ matchesPostDecrInWhile(E, isInc, isPre, CGF.getContext());
if (CGF.getContext().isPromotableIntegerType(type)) {
promotedType = CGF.getContext().getPromotedIntegerType(type);
@@ -2959,7 +2972,7 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
- !disableSanitizer) {
+ !excludeOverflowPattern) {
value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
} else {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 24842b0a769fc..5a5f5cb79a12f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4269,17 +4269,14 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
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;
+ Opts.OverflowPatternExclusionMask |=
+ llvm::StringSwitch<unsigned>(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);
}
}
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
index e6b729e0c788e..d21405c56beab 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
@@ -1,7 +1,7 @@
-// Check for potential false positives from patterns that _almost_ match classic overflow-dependent or overflow-prone code patterns
// 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;
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/overflow-idiom-exclusion.c
index c93da342f9149..7c8c4af61029d 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion.c
@@ -1,3 +1,9 @@
+// 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
@@ -15,11 +21,6 @@
// unsigned negation, for example:
// unsigned long A = -1UL;
-// 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
>From a2cc243bb1e734fde4517774d75dc11dc5c8d799 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 12 Aug 2024 11:33:36 -0700
Subject: [PATCH 09/20] remove anonymous namespace
remove the anonymous namespace as a static qualifier is enough for this
method.
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
Thanks Bill.
---
clang/lib/AST/Expr.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index ffe10ecb24173..5c8624d77e8c7 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4759,7 +4759,6 @@ ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx,
return new (Mem) ParenListExpr(EmptyShell(), NumExprs);
}
-namespace {
/// 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
@@ -4806,7 +4805,6 @@ getOverflowPatternBinOp(const BinaryOperator *E) {
return BO;
return {};
}
-} // namespace
BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
Opcode opc, QualType ResTy, ExprValueKind VK,
>From 2b6abb964593cc45dab5274e796ccb2d465ec1c3 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 12 Aug 2024 17:57:03 -0700
Subject: [PATCH 10/20] move ExcludedOverflowPattern to BinaryOperatorBits
* move ExcludedOverflowPattern to BinaryOperatorBits
* reorder some logic to benefit from short-circuiting
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 5 +++--
clang/include/clang/AST/Stmt.h | 2 ++
clang/lib/AST/Expr.cpp | 3 ++-
clang/lib/CodeGen/CGExprScalar.cpp | 5 +++--
4 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 31572cd4465eb..db5a07014a994 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3860,7 +3860,6 @@ class CStyleCastExpr final
class BinaryOperator : public Expr {
enum { LHS, RHS, END_EXPR };
Stmt *SubExprs[END_EXPR];
- bool ExcludedOverflowPattern = false;
public:
typedef BinaryOperatorKind Opcode;
@@ -4019,7 +4018,9 @@ class BinaryOperator : public Expr {
return isShiftAssignOp(getOpcode());
}
- bool ignoreOverflowSanitizers() const { return ExcludedOverflowPattern; }
+ bool ignoreOverflowSanitizers() const {
+ return BinaryOperatorBits.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
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index bbd7634bcc3bf..22e17a03f974b 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -649,6 +649,8 @@ class alignas(void *) Stmt {
/// It is 0 otherwise.
LLVM_PREFERRED_TYPE(bool)
unsigned HasFPFeatures : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned ExcludedOverflowPattern : 1;
SourceLocation OpLoc;
};
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 5c8624d77e8c7..57475c66a94e3 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4815,13 +4815,14 @@ 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()->ExcludedOverflowPattern = true;
+ Result.value()->BinaryOperatorBits.ExcludedOverflowPattern = 1;
}
BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
if (hasStoredFPFeatures())
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 4695228b1b9e5..5285fd4c314b0 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -198,9 +198,10 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) {
const UnaryOperator *UO = dyn_cast<UnaryOperator>(Op.E);
- if (UO && UO->getOpcode() == UO_Minus && UO->isIntegerConstantExpr(Ctx) &&
+ if (UO && UO->getOpcode() == UO_Minus &&
Ctx.getLangOpts().isOverflowPatternExcluded(
- LangOptions::OverflowPatternExclusionKind::NegUnsignedConst))
+ LangOptions::OverflowPatternExclusionKind::NegUnsignedConst) &&
+ UO->isIntegerConstantExpr(Ctx))
return true;
// If a unary op has a widened operand, the op cannot overflow.
>From f09e40e65bec8deadf264b390d2673744204e96a Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 13 Aug 2024 14:06:12 -0700
Subject: [PATCH 11/20] add comment to flag
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Stmt.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 22e17a03f974b..f1a2aac0a8b2f 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -649,6 +649,9 @@ class alignas(void *) Stmt {
/// It is 0 otherwise.
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;
>From d714d7b02b8d3e56754efb368f4b78bbb706af7b Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 14 Aug 2024 14:51:12 -0700
Subject: [PATCH 12/20] add ExcludedOverflowPattern bit to Stmt Serialization
Signed-off-by: Justin Stitt <justinstitt at google.com>
Co-authored-by: Bill Wendling <morbo at google.com>
---
clang/include/clang/AST/Expr.h | 6 +++++-
clang/lib/CodeGen/CGExprScalar.cpp | 2 +-
clang/lib/Serialization/ASTReaderStmt.cpp | 1 +
clang/lib/Serialization/ASTWriterStmt.cpp | 1 +
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index db5a07014a994..6aee17461feef 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4018,7 +4018,7 @@ class BinaryOperator : public Expr {
return isShiftAssignOp(getOpcode());
}
- bool ignoreOverflowSanitizers() const {
+ bool hasExcludedOverflowPattern() const {
return BinaryOperatorBits.ExcludedOverflowPattern;
}
@@ -4047,6 +4047,10 @@ class BinaryOperator : public Expr {
void setHasStoredFPFeatures(bool B) { BinaryOperatorBits.HasFPFeatures = B; }
bool hasStoredFPFeatures() const { return BinaryOperatorBits.HasFPFeatures; }
+ void setExcludedOverflowPattern(bool B) {
+ BinaryOperatorBits.ExcludedOverflowPattern = B;
+ }
+
/// Get FPFeatures from trailing storage
FPOptionsOverride getStoredFPFeatures() const {
assert(hasStoredFPFeatures());
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 5285fd4c314b0..6eac2b4c54e1b 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -211,7 +211,7 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) {
// 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())
+ if (BO->hasExcludedOverflowPattern())
return true;
auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS());
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index a33f2a41a6549..8ae07907a04ab 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 038616a675b72..c292d0a789c7c 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());
>From c33ce9fa3ffe2ae6f96e2961e4f2f934790748ec Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 14 Aug 2024 17:04:02 -0700
Subject: [PATCH 13/20] colocate getter and setter together
previously, the getter was a couple dozen lines about the setter, for no
good reason. Let's put these two methods visually next to one another.
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 6aee17461feef..f5863524723a2 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4018,10 +4018,6 @@ class BinaryOperator : public Expr {
return isShiftAssignOp(getOpcode());
}
- bool hasExcludedOverflowPattern() const {
- return BinaryOperatorBits.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
/// integer to a pointer.
@@ -4047,9 +4043,14 @@ 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 {
>From faa5fea682ca0e57876d7b664169e1f0d7753c54 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Wed, 14 Aug 2024 20:10:11 -0700
Subject: [PATCH 14/20] fix tests to use triple
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
.../CodeGen/overflow-idiom-exclusion-fp.c | 4 +-
clang/test/CodeGen/overflow-idiom-exclusion.c | 84 +++++++++++++++----
2 files changed, 68 insertions(+), 20 deletions(-)
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
index d21405c56beab..5dd2a9c2a69c8 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
@@ -1,5 +1,5 @@
-// 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_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -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;
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/overflow-idiom-exclusion.c
index 7c8c4af61029d..fc380db43cee3 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion.c
+++ b/clang/test/CodeGen/overflow-idiom-exclusion.c
@@ -1,9 +1,8 @@
-// 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
-
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all -fwrapv %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -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_cc1 -triple x86_64-unknown-linux-gnu -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_cc1 -triple x86_64-unknown-linux-gnu -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
@@ -24,20 +23,15 @@
// 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);
+// ADD-LABEL: @basic_commutativity
+// WHILE-LABEL: @basic_commutativity
+// NEGATE-LABEL: @basic_commutativity
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
void basic_commutativity(void) {
if (a + b < a)
c = 9;
@@ -57,6 +51,12 @@ void basic_commutativity(void) {
c = 9;
}
+// ADD-LABEL: @arguments_and_commutativity
+// WHILE-LABEL: @arguments_and_commutativity
+// NEGATE-LABEL: @arguments_and_commutativity
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
void arguments_and_commutativity(unsigned V1, unsigned V2) {
if (V1 + V2 < V1)
c = 9;
@@ -76,6 +76,12 @@ void arguments_and_commutativity(unsigned V1, unsigned V2) {
c = 9;
}
+// ADD-LABEL: @pointers
+// WHILE-LABEL: @pointers
+// NEGATE-LABEL: @pointers
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
void pointers(unsigned *P1, unsigned *P2, unsigned V1) {
if (*P1 + *P2 < *P1)
c = 9;
@@ -96,16 +102,34 @@ struct MyStruct {
extern struct MyStruct ms;
+// ADD-LABEL: @structs
+// WHILE-LABEL: @structs
+// NEGATE-LABEL: @structs
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
void structs(void) {
if (ms.base + ms.offset < ms.base)
c = 9;
}
+// ADD-LABEL: @nestedstructs
+// WHILE-LABEL: @nestedstructs
+// NEGATE-LABEL: @nestedstructs
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
void nestedstructs(void) {
if (ms.os.foo + ms.os.bar < ms.os.foo)
c = 9;
}
+// ADD-LABEL: @constants
+// WHILE-LABEL: @constants
+// NEGATE-LABEL: @constants
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
// 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) {
@@ -114,7 +138,12 @@ void constants(void) {
if (base + offset < base)
c = 9;
}
-
+// ADD-LABEL: @common_while
+// NEGATE-LABEL: @common_while
+// WHILE-LABEL: @common_while
+// ADD: usub.with.overflow
+// NEGATE: usub.with.overflow
+// WHILE: %dec = add i32 %0, -1
void common_while(unsigned i) {
// This post-decrement usually causes overflow sanitizers to trip on the very
// last operation.
@@ -123,6 +152,12 @@ void common_while(unsigned i) {
}
}
+// ADD-LABEL: @negation
+// NEGATE-LABEL: @negation
+// WHILE-LABEL @negation
+// ADD: negate_overflow
+// NEGATE-NOT: negate_overflow
+// WHILE: negate_overflow
// Normally, these assignments would trip the unsigned overflow sanitizer.
void negation(void) {
#define SOME -1UL
@@ -133,6 +168,13 @@ void negation(void) {
(void)A;(void)B;(void)C;(void)D;
}
+
+// ADD-LABEL: @key_alloc
+// WHILE-LABEL: @key_alloc
+// NEGATE-LABEL: @key_alloc
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
// cvise'd kernel code that caused problems during development due to sign
// extension
typedef unsigned long _size_t;
@@ -145,6 +187,12 @@ int *key_alloc(void) {
return key_alloc_key + 3;;
}
+// ADD-LABEL: @function_call
+// WHILE-LABEL: @function_call
+// NEGATE-LABEL: @function_call
+// WHILE: handler.add_overflow
+// NEGATE: handler.add_overflow
+// ADD-NOT: handler.add_overflow
void function_call(void) {
if (b + some() < b)
c = 9;
>From 3f452fd478cb1dbf390252edcbb3be3a920283c5 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Thu, 15 Aug 2024 14:33:44 -0700
Subject: [PATCH 15/20] remove langopts that are no longer being used
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/Basic/LangOptions.def | 2 --
1 file changed, 2 deletions(-)
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 2e9f2c552aad8..d454a7ff2f8cf 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,
>From a9ff81b7868ee9d4186127b3d76db3d8fd18cf22 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Fri, 16 Aug 2024 11:40:23 -0700
Subject: [PATCH 16/20] initialize bit, fixing tests
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 1 +
clang/lib/AST/Expr.cpp | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index f5863524723a2..7bacf028192c6 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3888,6 +3888,7 @@ class BinaryOperator : public Expr {
/// Construct an empty binary operator.
explicit BinaryOperator(EmptyShell Empty) : Expr(BinaryOperatorClass, Empty) {
BinaryOperatorBits.Opc = BO_Comma;
+ BinaryOperatorBits.ExcludedOverflowPattern = false;
}
public:
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 57475c66a94e3..25ab6f3b2addf 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4815,14 +4815,14 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
assert(!isCompoundAssignmentOp() &&
"Use CompoundAssignOperator for compound assignments");
BinaryOperatorBits.OpLoc = opLoc;
- BinaryOperatorBits.ExcludedOverflowPattern = 0;
+ BinaryOperatorBits.ExcludedOverflowPattern = false;
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;
+ Result.value()->BinaryOperatorBits.ExcludedOverflowPattern = true;
}
BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
if (hasStoredFPFeatures())
@@ -4836,6 +4836,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
FPOptionsOverride FPFeatures, bool dead2)
: Expr(CompoundAssignOperatorClass, ResTy, VK, OK) {
BinaryOperatorBits.Opc = opc;
+ BinaryOperatorBits.ExcludedOverflowPattern = false;
assert(isCompoundAssignmentOp() &&
"Use CompoundAssignOperator for compound assignments");
BinaryOperatorBits.OpLoc = opLoc;
>From 02f6369e902f03adee328753c80285a7ff4d9cfd Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 19 Aug 2024 16:03:44 -0700
Subject: [PATCH 17/20] rename tests, remove outdated tests
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
....c => ignore-overflow-pattern-false-pos.c} | 20 ----------------
...-exclusion.c => ignore-overflow-pattern.c} | 24 +++----------------
2 files changed, 3 insertions(+), 41 deletions(-)
rename clang/test/CodeGen/{overflow-idiom-exclusion-fp.c => ignore-overflow-pattern-false-pos.c} (67%)
rename clang/test/CodeGen/{overflow-idiom-exclusion.c => ignore-overflow-pattern.c} (90%)
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
similarity index 67%
rename from clang/test/CodeGen/overflow-idiom-exclusion-fp.c
rename to clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
index 5dd2a9c2a69c8..651be942c2b50 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
@@ -45,30 +45,10 @@ void close_but_not_quite(void) {
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
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c b/clang/test/CodeGen/ignore-overflow-pattern.c
similarity index 90%
rename from clang/test/CodeGen/overflow-idiom-exclusion.c
rename to clang/test/CodeGen/ignore-overflow-pattern.c
index fc380db43cee3..734bb04c0251f 100644
--- a/clang/test/CodeGen/overflow-idiom-exclusion.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern.c
@@ -3,6 +3,7 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -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_cc1 -triple x86_64-unknown-linux-gnu -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_cc1 -triple x86_64-unknown-linux-gnu -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
@@ -163,30 +164,11 @@ 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;
+ unsigned long C = -SOME;
+ (void)A;(void)B;(void)C;
}
-// ADD-LABEL: @key_alloc
-// WHILE-LABEL: @key_alloc
-// NEGATE-LABEL: @key_alloc
-// WHILE: handler.add_overflow
-// NEGATE: handler.add_overflow
-// ADD-NOT: handler.add_overflow
-// 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;;
-}
-
// ADD-LABEL: @function_call
// WHILE-LABEL: @function_call
// NEGATE-LABEL: @function_call
>From d19eb43fdbe0c4e6edeed5085f5c49ca51a95c51 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 19 Aug 2024 17:35:55 -0700
Subject: [PATCH 18/20] rename flag to
-fsanitize-undefined-ignore-overflow-pattern=
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/docs/ReleaseNotes.rst | 12 ++++++------
clang/docs/UndefinedBehaviorSanitizer.rst | 14 +++++++-------
clang/include/clang/Driver/Options.td | 2 +-
clang/lib/CodeGen/CGExprScalar.cpp | 2 +-
clang/lib/Driver/SanitizerArgs.cpp | 6 +++---
clang/lib/Driver/ToolChains/Clang.cpp | 2 +-
clang/lib/Frontend/CompilerInvocation.cpp | 3 ++-
.../CodeGen/ignore-overflow-pattern-false-pos.c | 4 ++--
clang/test/CodeGen/ignore-overflow-pattern.c | 10 +++++-----
9 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5696d6ce15dc..0ea2bf1531fae 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -392,27 +392,27 @@ 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
+- Added the ``-fsanitize-undefined-ignore-overflow-pattern`` 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``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=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``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=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``
+ /// specified with ``-fsanitize-undefined-ignore-overflow-pattern=post-decr-while``
void while_post_decrement() {
unsigned char count = 16;
while (count--) { /* ... */} // No longer causes unsigned-integer-overflow sanitizer to trip
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index 9f3d980eefbea..1c92907372f83 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -302,7 +302,7 @@ 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=``.
+use ``-fsanitize-undefined-ignore-overflow-pattern=``.
Currently, this option supports three overflow-dependent code idioms:
@@ -310,7 +310,7 @@ Currently, this option supports three overflow-dependent code idioms:
.. code-block:: c++
- /// -fsanitize-overflow-pattern-exclusion=negated-unsigned-const
+ /// -fsanitize-undefined-ignore-overflow-pattern=negated-unsigned-const
unsigned long foo = -1UL; // No longer causes a negation overflow warning
unsigned long bar = -2UL; // and so on...
@@ -318,7 +318,7 @@ Currently, this option supports three overflow-dependent code idioms:
.. code-block:: c++
- /// -fsanitize-overflow-pattern-exclusion=post-decr-while
+ /// -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
unsigned char count = 16;
while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
@@ -326,14 +326,14 @@ Currently, this option supports three overflow-dependent code idioms:
.. code-block:: c++
- /// -fsanitize-overflow-pattern-exclusion=add-overflow-test
+ /// -fsanitize-undefined-ignore-overflow-pattern=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.
+``-fsanitize-undefined-ignore-overflow-pattern=all`` or disable all exclusions
+with ``-fsanitize-undefined-ignore-overflow-pattern=none``. Specifying ``none``
+has precedence over other values.
Issue Suppression
=================
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index acc1f2fde5397..08563b3f435cb 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2565,7 +2565,7 @@ 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=">,
+def fsanitize_undefined_ignore_overflow_pattern_EQ : CommaJoined<["-"], "fsanitize-undefined-ignore-overflow-pattern=">,
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">,
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 6eac2b4c54e1b..3bda254c86adf 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2785,7 +2785,7 @@ static bool matchesPostDecrInWhile(const UnaryOperator *UO, bool isInc,
if (isInc || isPre)
return false;
- // -fsanitize-overflow-pattern-exclusion=post-decr-while
+ // -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
if (!Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::PostDecrInWhile))
return false;
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index a63ee944fd1bb..edac1e900a771 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -793,7 +793,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
}
for (const auto *Arg :
- Args.filtered(options::OPT_fsanitize_overflow_pattern_exclusion_EQ)) {
+ Args.filtered(options::OPT_fsanitize_undefined_ignore_overflow_pattern_EQ)) {
Arg->claim();
OverflowPatternExclusions |=
parseOverflowPatternExclusionValues(D, Arg, DiagnoseErrors);
@@ -1253,8 +1253,8 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
"-fsanitize-system-ignorelist=", SystemIgnorelistFiles);
if (OverflowPatternExclusions)
- Args.AddAllArgs(CmdArgs,
- options::OPT_fsanitize_overflow_pattern_exclusion_EQ);
+ Args.AddAllArgs(
+ CmdArgs, options::OPT_fsanitize_undefined_ignore_overflow_pattern_EQ);
if (MsanTrackOrigins)
CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index f2bc11839edd4..8ce9f2baf8965 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7770,7 +7770,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
}
Args.AddAllArgs(CmdArgs,
- options::OPT_fsanitize_overflow_pattern_exclusion_EQ);
+ options::OPT_fsanitize_undefined_ignore_overflow_pattern_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 5a5f5cb79a12f..f510d3067d4d5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4267,7 +4267,8 @@ 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)) {
+ if (auto *A =
+ Args.getLastArg(OPT_fsanitize_undefined_ignore_overflow_pattern_EQ)) {
for (int i = 0, n = A->getNumValues(); i != n; ++i) {
Opts.OverflowPatternExclusionMask |=
llvm::StringSwitch<unsigned>(A->getValue(i))
diff --git a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
index 651be942c2b50..40193e0c3e267 100644
--- a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all -fwrapv %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=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;
diff --git a/clang/test/CodeGen/ignore-overflow-pattern.c b/clang/test/CodeGen/ignore-overflow-pattern.c
index 734bb04c0251f..b7d700258f853 100644
--- a/clang/test/CodeGen/ignore-overflow-pattern.c
+++ b/clang/test/CodeGen/ignore-overflow-pattern.c
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=all -fwrapv %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -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_cc1 -triple x86_64-unknown-linux-gnu -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_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-overflow-pattern-exclusion=post-decr-while %s -emit-llvm -o - | FileCheck %s --check-prefix=WHILE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all -fwrapv %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=add-overflow-test %s -emit-llvm -o - | FileCheck %s --check-prefix=ADD
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=negated-unsigned-const %s -emit-llvm -o - | FileCheck %s --check-prefix=NEGATE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=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
>From 7c5dd941a2abc298bf2bf7031cc5eba4cd7592e7 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Mon, 19 Aug 2024 17:55:35 -0700
Subject: [PATCH 19/20] run clang-format
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/lib/Driver/SanitizerArgs.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index edac1e900a771..e2ab17760d97c 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -792,8 +792,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
<< "fsanitize-trap=cfi";
}
- for (const auto *Arg :
- Args.filtered(options::OPT_fsanitize_undefined_ignore_overflow_pattern_EQ)) {
+ for (const auto *Arg : Args.filtered(
+ options::OPT_fsanitize_undefined_ignore_overflow_pattern_EQ)) {
Arg->claim();
OverflowPatternExclusions |=
parseOverflowPatternExclusionValues(D, Arg, DiagnoseErrors);
>From 03f5d81374784377d382eef7286d74eb712c7395 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinstitt at google.com>
Date: Tue, 20 Aug 2024 12:21:25 -0700
Subject: [PATCH 20/20] add comment to parseOverflowPatternExclusionValues
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/lib/Driver/SanitizerArgs.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index e2ab17760d97c..9d9ad79d51d7f 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -119,6 +119,8 @@ static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
static int parseCoverageFeatures(const Driver &D, const llvm::opt::Arg *A,
bool DiagnoseErrors);
+/// Parse -fsanitize-undefined-ignore-overflow-pattern= flag values, diagnosing
+/// any invalid values. Returns a mask of excluded overflow patterns.
static int parseOverflowPatternExclusionValues(const Driver &D,
const llvm::opt::Arg *A,
bool DiagnoseErrors);
More information about the cfe-commits
mailing list