[clang] [Clang] Re-land Overflow Pattern Exclusions (PR #104889)
Justin Stitt via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 19 17:53:34 PDT 2024
https://github.com/JustinStitt created https://github.com/llvm/llvm-project/pull/104889
[[Clang] Overflow Pattern Exclusions #100272
](https://github.com/llvm/llvm-project/pull/100272) was merged last week but had some test failures caused by an uninitialized bit within a bitfield. These failures have been fixed so let's try to re-land this.
After speaking with Vitaly and Kees, we've decided to switch the flag naming to better match the existing conventions of sanitizer options: `-fsanitize-<sanitizer_kind>-<option>`
old flag:
`-fsanitize-overflow-pattern-exclusion=(all|none|...)`
new flag:
`-fsanitize-undefined-ignore-overflow-pattern=(all|none|...)`
cc: @vitalybuka @kees @efriedma-quic @bwendling
>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/18] 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 5b813bfc2faf90..c329ee061cf008 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 d454a7ff2f8cf4..2e9f2c552aad8a 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 6df3a6a5943a97..2d4322b51035be 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 47ef175302679f..291739e1221d96 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 9d5b8167d0ee62..c07560c92100d7 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 84392745ea6144..089f841bb3acdf 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 1fd870b72286e5..567840bcfbbdb6 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 96aa930ea28612..e423267855075a 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 00000000000000..0fa847f733a7fb
--- /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 00000000000000..df138fb8f5db84
--- /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/18] 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 7c4451d93394c3..cd0549bf303be5 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 531d56e313826c..b856f601aec81f 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/18] 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 c329ee061cf008..e8dc22c51a03dd 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 c07560c92100d7..d963d473442d8e 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 089f841bb3acdf..f069039d3702aa 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/18] 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 e8dc22c51a03dd..b25a4368aa1527 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 91f1c2f2e6239e..d57a9d7f31c733 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 2d4322b51035be..6fb41cd7de4d78 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 291739e1221d96..1e23a6a6f85f12 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 d963d473442d8e..8069cf80ea5058 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 f069039d3702aa..f1e71557fb5add 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 567840bcfbbdb6..3b1c0b645ffbe0 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 e423267855075a..f2bc11839edd4d 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 e3911c281985b7..24842b0a769fc6 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 0fa847f733a7fb..7bd7f94a3f039c 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 df138fb8f5db84..d09e8c364963c5 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/18] 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 cd0549bf303be5..f5696d6ce15dc7 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 b856f601aec81f..2b15d14ff449be 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 6fb41cd7de4d78..acc1f2fde53979 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 1e23a6a6f85f12..e64ec463ca8907 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 8069cf80ea5058..ffe10ecb241734 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 f1e71557fb5add..3064e744be0666 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 3b1c0b645ffbe0..a63ee944fd1bb4 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 7bd7f94a3f039c..621d8f43babd8b 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 d09e8c364963c5..e753343542a061 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/18] 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 b25a4368aa1527..31572cd4465ebc 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/18] 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 621d8f43babd8b..e6b729e0c788e6 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 e753343542a061..c93da342f91491 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/18] 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 2b15d14ff449be..9f3d980eefbea7 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 d57a9d7f31c733..eb4cb4b5a7e93f 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 3064e744be0666..4695228b1b9e57 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 24842b0a769fc6..5a5f5cb79a12f2 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 e6b729e0c788e6..d21405c56beab3 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 c93da342f91491..7c8c4af61029de 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/18] 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 ffe10ecb241734..5c8624d77e8c7e 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/18] 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 31572cd4465ebc..db5a07014a9942 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 bbd7634bcc3bfb..22e17a03f974ba 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 5c8624d77e8c7e..57475c66a94e35 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 4695228b1b9e57..5285fd4c314b07 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/18] 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 22e17a03f974ba..f1a2aac0a8b2f8 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/18] 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 db5a07014a9942..6aee17461feeff 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 5285fd4c314b07..6eac2b4c54e1ba 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 a33f2a41a65497..8ae07907a04aba 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 038616a675b727..c292d0a789c7cd 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/18] 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 6aee17461feeff..f5863524723a2e 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/18] 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 d21405c56beab3..5dd2a9c2a69c8a 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 7c8c4af61029de..fc380db43cee32 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/18] 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 2e9f2c552aad8a..d454a7ff2f8cf4 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -406,8 +406,6 @@ VALUE_LANGOPT(TrivialAutoVarInitMaxSize, 32, 0,
"stop trivial automatic variable initialization if var size exceeds the specified size (in bytes). Must be greater than 0.")
ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2, SOB_Undefined,
"signed integer overflow handling")
-LANGOPT(IgnoreNegationOverflow, 1, 0, "ignore overflow caused by negation")
-LANGOPT(SanitizeOverflowIdioms, 1, 1, "enable instrumentation for common overflow idioms")
ENUM_LANGOPT(ThreadModel , ThreadModelKind, 2, ThreadModelKind::POSIX, "Thread Model")
BENIGN_LANGOPT(ArrowDepth, 32, 256,
>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/18] 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 f5863524723a2e..7bacf028192c65 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 57475c66a94e35..25ab6f3b2addfb 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/18] 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 5dd2a9c2a69c8a..651be942c2b50a 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 fc380db43cee32..734bb04c0251f9 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/18] 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 f5696d6ce15dc7..0ea2bf1531fae1 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 9f3d980eefbea7..1c92907372f83c 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 acc1f2fde53979..08563b3f435cb3 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 6eac2b4c54e1ba..3bda254c86adf6 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 a63ee944fd1bb4..edac1e900a7710 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 f2bc11839edd4d..8ce9f2baf8965e 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 5a5f5cb79a12f2..f510d3067d4d58 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 651be942c2b50a..40193e0c3e2671 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 734bb04c0251f9..b7d700258f8538 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
More information about the cfe-commits
mailing list