https://github.com/JustinStitt updated
https://github.com/llvm/llvm-project/pull/100272
>From 06b702cd38943314b2e6f873e64d70baed6f57f7 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinst...@google.com>
Date: Tue, 23 Jul 2024 20:21:49 +0000
Subject: [PATCH 1/3] implement idiom exclusions
Add flag `-fno-sanitize-overflow-idioms` which disables integer
overflow/truncation sanitizer instrumentation for common
overflow-dependent code patterns.
Signed-off-by: Justin Stitt <justinst...@google.com>
---
clang/include/clang/AST/Expr.h | 5 +
clang/include/clang/Basic/LangOptions.def | 2 +
clang/include/clang/Driver/Options.td | 10 ++
clang/include/clang/Driver/SanitizerArgs.h | 1 +
clang/lib/AST/Expr.cpp | 53 +++++++
clang/lib/CodeGen/CGExprScalar.cpp | 26 +++-
clang/lib/Driver/SanitizerArgs.cpp | 7 +
clang/lib/Driver/ToolChains/Clang.cpp | 7 +
.../CodeGen/overflow-idiom-exclusion-fp.c | 77 ++++++++++
clang/test/CodeGen/overflow-idiom-exclusion.c | 141 ++++++++++++++++++
10 files changed, 327 insertions(+), 2 deletions(-)
create mode 100644 clang/test/CodeGen/overflow-idiom-exclusion-fp.c
create mode 100644 clang/test/CodeGen/overflow-idiom-exclusion.c
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 5b813bfc2faf9..c329ee061cf00 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3860,6 +3860,7 @@ class CStyleCastExpr final
class BinaryOperator : public Expr {
enum { LHS, RHS, END_EXPR };
Stmt *SubExprs[END_EXPR];
+ bool isOverflowIdiom;
public:
typedef BinaryOperatorKind Opcode;
@@ -4018,6 +4019,10 @@ class BinaryOperator : public Expr {
return isShiftAssignOp(getOpcode());
}
+ bool ignoreOverflowSanitizers() const {
+ return isOverflowIdiom;
+ }
+
/// Return true if a binary operator using the specified opcode and operands
/// would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized
/// integer to a pointer.
diff --git a/clang/include/clang/Basic/LangOptions.def
b/clang/include/clang/Basic/LangOptions.def
index 834a6f6cd43e3..675350a99024a 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -402,6 +402,8 @@ VALUE_LANGOPT(TrivialAutoVarInitMaxSize, 32, 0,
"stop trivial automatic variable initialization if var size
exceeds the specified size (in bytes). Must be greater than 0.")
ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2,
SOB_Undefined,
"signed integer overflow handling")
+LANGOPT(IgnoreNegationOverflow, 1, 0, "ignore overflow caused by negation")
+LANGOPT(SanitizeOverflowIdioms, 1, 1, "enable instrumentation for common
overflow idioms")
ENUM_LANGOPT(ThreadModel , ThreadModelKind, 2, ThreadModelKind::POSIX,
"Thread Model")
BENIGN_LANGOPT(ArrowDepth, 32, 256,
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 69269cf7537b0..81a6baa1a2eae 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2559,6 +2559,16 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats",
"Disable">,
BothFlags<[], [ClangOption], " sanitizer statistics gathering.">>,
Group<f_clang_Group>;
+defm sanitize_overflow_idioms : BoolFOption<"sanitize-overflow-idioms",
+ LangOpts<"SanitizeOverflowIdioms">, DefaultTrue,
+ PosFlag<SetTrue, [], [], "Enable">,
+ NegFlag<SetFalse, [], [], "Disable">,
+ BothFlags<[], [ClangOption], " the instrumentation of common overflow
idioms">>;
+defm sanitize_negation_overflow : BoolFOption<"sanitize-negation-overflow",
+ LangOpts<"IgnoreNegationOverflow">, DefaultFalse,
+ PosFlag<SetFalse, [], [], "Enable">,
+ NegFlag<SetTrue, [], [], "Disable">,
+ BothFlags<[], [ClangOption], " integer overflow sanitizer instrumentation
for negation">>;
def fsanitize_thread_memory_access : Flag<["-"],
"fsanitize-thread-memory-access">,
Group<f_clang_Group>,
HelpText<"Enable memory access
instrumentation in ThreadSanitizer (default)">;
diff --git a/clang/include/clang/Driver/SanitizerArgs.h
b/clang/include/clang/Driver/SanitizerArgs.h
index 47ef175302679..291739e1221d9 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -64,6 +64,7 @@ class SanitizerArgs {
// True if cross-dso CFI support if provided by the system (i.e. Android).
bool ImplicitCfiRuntime = false;
bool NeedsMemProfRt = false;
+ bool SanitizeOverflowIdioms = true;
bool HwasanUseAliases = false;
llvm::AsanDetectStackUseAfterReturnMode AsanUseAfterReturn =
llvm::AsanDetectStackUseAfterReturnMode::Invalid;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 9d5b8167d0ee6..c07560c92100d 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4759,6 +4759,54 @@ ParenListExpr *ParenListExpr::CreateEmpty(const
ASTContext &Ctx,
return new (Mem) ParenListExpr(EmptyShell(), NumExprs);
}
+namespace {
+/// Certain overflow-dependent code idioms can have their integer overflow
+/// sanitization disabled. Check for the common pattern `if (a + b < a)` and
+/// return the resulting BinaryOperator responsible for the addition so we can
+/// elide overflow checks during codegen.
+static std::optional<BinaryOperator *> getOverflowIdiomBinOp(const
BinaryOperator *E) {
+ Expr *Addition, *ComparedTo;
+ if (E->getOpcode() == BO_LT) {
+ Addition = E->getLHS();
+ ComparedTo = E->getRHS();
+ } else if (E->getOpcode() == BO_GT) {
+ Addition = E->getRHS();
+ ComparedTo = E->getLHS();
+ } else {
+ return {};
+ }
+
+ const Expr *AddLHS = nullptr, *AddRHS = nullptr;
+ BinaryOperator *BO = dyn_cast<BinaryOperator>(Addition);
+
+ if (BO && BO->getOpcode() == clang::BO_Add) {
+ // now store addends for lookup on other side of '>'
+ AddLHS = BO->getLHS();
+ AddRHS = BO->getRHS();
+ }
+
+ if (!AddLHS || !AddRHS)
+ return {};
+
+ const Decl *LHSDecl, *RHSDecl, *OtherDecl;
+
+ LHSDecl = AddLHS->IgnoreParenImpCasts()->getReferencedDeclOfCallee();
+ RHSDecl = AddRHS->IgnoreParenImpCasts()->getReferencedDeclOfCallee();
+ OtherDecl = ComparedTo->IgnoreParenImpCasts()->getReferencedDeclOfCallee();
+
+ if (!OtherDecl)
+ return {};
+
+ if (!LHSDecl && !RHSDecl)
+ return {};
+
+ if ((LHSDecl && LHSDecl == OtherDecl && LHSDecl != RHSDecl) ||
+ (RHSDecl && RHSDecl == OtherDecl && RHSDecl != LHSDecl))
+ return BO;
+ return {};
+}
+} // namespace
+
BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
Opcode opc, QualType ResTy, ExprValueKind VK,
ExprObjectKind OK, SourceLocation opLoc,
@@ -4770,6 +4818,11 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx,
Expr *lhs, Expr *rhs,
BinaryOperatorBits.OpLoc = opLoc;
SubExprs[LHS] = lhs;
SubExprs[RHS] = rhs;
+ if (!Ctx.getLangOpts().SanitizeOverflowIdioms) {
+ std::optional<BinaryOperator*> Result = getOverflowIdiomBinOp(this);
+ if (Result.has_value())
+ Result.value()->isOverflowIdiom = true;
+ }
BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index a17d68424bbce..be0307073f3b9 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -25,6 +25,7 @@
#include "clang/AST/DeclObjC.h"
#include "clang/AST/Expr.h"
#include "clang/AST/RecordLayout.h"
+#include "clang/AST/ParentMapContext.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Basic/CodeGenOptions.h"
#include "clang/Basic/TargetInfo.h"
@@ -195,13 +196,22 @@ static bool CanElideOverflowCheck(const ASTContext &Ctx,
const BinOpInfo &Op) {
if (!Op.mayHaveIntegerOverflow())
return true;
+ const UnaryOperator *UO = dyn_cast<UnaryOperator>(Op.E);
+
+ if (UO && UO->getOpcode() == UO_Minus &&
+ Ctx.getLangOpts().IgnoreNegationOverflow)
+ return true;
+
// If a unary op has a widened operand, the op cannot overflow.
- if (const auto *UO = dyn_cast<UnaryOperator>(Op.E))
+ if (UO)
return !UO->canOverflow();
// We usually don't need overflow checks for binops with widened operands.
// Multiplication with promoted unsigned operands is a special case.
const auto *BO = cast<BinaryOperator>(Op.E);
+ if (BO->ignoreOverflowSanitizers())
+ return true;
+
auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS());
if (!OptionalLHSTy)
return false;
@@ -2876,6 +2886,17 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const
UnaryOperator *E, LValue LV,
} else if (type->isIntegerType()) {
QualType promotedType;
bool canPerformLossyDemotionCheck = false;
+
+ // Does this match the pattern of while(i--) {...}? If so, and if
+ // SanitizeOverflowIdioms is disabled, we don't want to enable sanitizer.
+ bool disableSanitizer =
+ (!isInc && !isPre &&
+ !CGF.getContext().getLangOpts().SanitizeOverflowIdioms &&
+ llvm::all_of(CGF.getContext().getParentMapContext().getParents(*E),
+ [&](const DynTypedNode &Parent) -> bool {
+ return Parent.get<WhileStmt>();
+ }));
+
if (CGF.getContext().isPromotableIntegerType(type)) {
promotedType = CGF.getContext().getPromotedIntegerType(type);
assert(promotedType != type && "Shouldn't promote to the same type.");
@@ -2935,7 +2956,8 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const
UnaryOperator *E, LValue LV,
} else if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) {
value = EmitIncDecConsiderOverflowBehavior(E, value, isInc);
} else if (E->canOverflow() && type->isUnsignedIntegerType() &&
- CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) {
+ CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
+ !disableSanitizer) {
value = EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(
E, value, isInc, E->getFPFeaturesInEffect(CGF.getLangOpts())));
} else {
diff --git a/clang/lib/Driver/SanitizerArgs.cpp
b/clang/lib/Driver/SanitizerArgs.cpp
index 1fd870b72286e..567840bcfbbdb 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1069,6 +1069,10 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
options::OPT_fmemory_profile_EQ,
options::OPT_fno_memory_profile, false);
+ SanitizeOverflowIdioms =
+ Args.hasFlag(options::OPT_fsanitize_overflow_idioms,
+ options::OPT_fno_sanitize_overflow_idioms, true);
+
// Finally, initialize the set of available and recoverable sanitizers.
Sanitizers.Mask |= Kinds;
RecoverableSanitizers.Mask |= RecoverableKinds;
@@ -1356,6 +1360,9 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const
llvm::opt::ArgList &Args,
CmdArgs.push_back("+tagged-globals");
}
+ if (!SanitizeOverflowIdioms)
+ CmdArgs.push_back("-fno-sanitize-overflow-idioms");
+
// MSan: Workaround for PR16386.
// ASan: This is mainly to help LSan with cases such as
// https://github.com/google/sanitizers/issues/373
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/ToolChains/Clang.cpp
index 78936fd634f33..198947c2431c3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6800,6 +6800,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction
&JA,
}
}
+ if (Args.hasArg(options::OPT_fno_sanitize_overflow_idioms) &&
+ !Args.hasArg(options::OPT_fsanitize_negation_overflow))
+ CmdArgs.push_back("-fno-sanitize-negation-overflow");
+
if (Args.getLastArg(options::OPT_fapple_kext) ||
(Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
CmdArgs.push_back("-fapple-kext");
@@ -6853,6 +6857,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction
&JA,
Args.addOptInFlag(CmdArgs, options::OPT_mspeculative_load_hardening,
options::OPT_mno_speculative_load_hardening);
+ Args.AddLastArg(CmdArgs, options::OPT_fsanitize_negation_overflow,
+ options::OPT_fno_sanitize_negation_overflow);
+
RenderSSPOptions(D, TC, Args, CmdArgs, KernelOrKext);
RenderSCPOptions(TC, Args, CmdArgs);
RenderTrivialAutoVarInitOptions(D, TC, Args, CmdArgs);
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
new file mode 100644
index 0000000000000..0fa847f733a7f
--- /dev/null
+++ b/clang/test/CodeGen/overflow-idiom-exclusion-fp.c
@@ -0,0 +1,77 @@
+// Check for potential false positives from patterns that _almost_ match
classic overflow idioms
+// RUN: %clang %s -O2
-fsanitize=signed-integer-overflow,unsigned-integer-overflow
-fno-sanitize-overflow-idioms -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang %s -O2
-fsanitize=signed-integer-overflow,unsigned-integer-overflow
-fno-sanitize-overflow-idioms -fwrapv -S -emit-llvm -o - | FileCheck %s
+
+extern unsigned a, b, c;
+extern int u, v, w;
+
+extern unsigned some(void);
+
+// Make sure all these still have handler paths, we shouldn't be excluding
+// instrumentation of any "near" idioms.
+void close_but_not_quite(void) {
+ // CHECK: br i1{{.*}}handler.
+ if (a + b > a)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (a - b < a)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (a + b < a)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (a + b + 1 < a)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (a + b < a + 1)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (b >= a + b)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (a + a < a)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler.
+ if (a + b == a)
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler
+ if (u + v < u) /* matches overflow idiom, but is signed */
+ c = 9;
+
+ // CHECK: br i1{{.*}}handler
+ // Although this can never actually overflow we are still checking that the
+ // sanitizer instruments it.
+ while (--a)
+ some();
+}
+
+// cvise'd kernel code that caused problems during development
+// CHECK: br i1{{.*}}handler
+typedef unsigned size_t;
+typedef enum { FSE_repeat_none } FSE_repeat;
+typedef enum { ZSTD_defaultAllowed } ZSTD_defaultPolicy_e;
+FSE_repeat ZSTD_selectEncodingType_repeatMode;
+ZSTD_defaultPolicy_e ZSTD_selectEncodingType_isDefaultAllowed;
+size_t ZSTD_NCountCost(void);
+
+void ZSTD_selectEncodingType(void) {
+ size_t basicCost =
+ ZSTD_selectEncodingType_isDefaultAllowed ? ZSTD_NCountCost() : 0,
+ compressedCost = 3 + ZSTD_NCountCost();
+ if (basicCost <= compressedCost)
+ ZSTD_selectEncodingType_repeatMode = FSE_repeat_none;
+}
+
+void function_calls(void) {
+ // CHECK: br i1{{.*}}handler
+ if (some() + b < some())
+ c = 9;
+}
diff --git a/clang/test/CodeGen/overflow-idiom-exclusion.c
b/clang/test/CodeGen/overflow-idiom-exclusion.c
new file mode 100644
index 0000000000000..df138fb8f5db8
--- /dev/null
+++ b/clang/test/CodeGen/overflow-idiom-exclusion.c
@@ -0,0 +1,141 @@
+// Ensure some common idioms don't trigger the overflow sanitizers when
+// -fno-sanitize-overflow-idioms is enabled. In many cases, overflow warnings
+// caused by these idioms are seen as "noise" and result in users turning off
+// sanitization all together.
+
+// A pattern like "if (a + b < a)" simply checks for overflow and usually means
+// the user is trying to handle it gracefully.
+
+// Similarly, a pattern resembling "while (i--)" is extremely common and
+// warning on its inevitable overflow can be seen as superfluous. Do note that
+// using "i" in future calculations can be tricky because it will still
+// wrap-around. Using -fno-sanitize-overflow-idioms or not doesn't change this
+// fact -- we just won't warn/trap with sanitizers.
+
+// Another common pattern that, in some cases, is found to be too noisy is
+// unsigned negation, for example:
+// unsigned long A = -1UL;
+
+// RUN: %clang %s -O2
-fsanitize=signed-integer-overflow,unsigned-integer-overflow
-fno-sanitize-overflow-idioms -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang %s -O2
-fsanitize=signed-integer-overflow,unsigned-integer-overflow
-fno-sanitize-overflow-idioms -fwrapv -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang %s -O2
-fsanitize=signed-integer-overflow,unsigned-integer-overflow
-fno-sanitize-overflow-idioms -S -emit-llvm -o - | FileCheck %s
--check-prefix=NEGATION
+// RUN: %clang %s -O2
-fsanitize=signed-integer-overflow,unsigned-integer-overflow
-fno-sanitize-overflow-idioms -fsanitize-negation-overflow -S -emit-llvm -o - |
FileCheck %s --check-prefix=NEGATIONOV
+// CHECK-NOT: br{{.*}}overflow
+
+extern unsigned a, b, c;
+extern unsigned some(void);
+
+void basic_commutativity(void) {
+ if (a + b < a)
+ c = 9;
+ if (a + b < b)
+ c = 9;
+ if (b + a < b)
+ c = 9;
+ if (b + a < a)
+ c = 9;
+ if (a > a + b)
+ c = 9;
+ if (a > b + a)
+ c = 9;
+ if (b > a + b)
+ c = 9;
+ if (b > b + a)
+ c = 9;
+}
+
+void arguments_and_commutativity(unsigned V1, unsigned V2) {
+ if (V1 + V2 < V1)
+ c = 9;
+ if (V1 + V2 < V2)
+ c = 9;
+ if (V2 + V1 < V2)
+ c = 9;
+ if (V2 + V1 < V1)
+ c = 9;
+ if (V1 > V1 + V2)
+ c = 9;
+ if (V1 > V2 + V1)
+ c = 9;
+ if (V2 > V1 + V2)
+ c = 9;
+ if (V2 > V2 + V1)
+ c = 9;
+}
+
+void pointers(unsigned *P1, unsigned *P2, unsigned V1) {
+ if (*P1 + *P2 < *P1)
+ c = 9;
+ if (*P1 + V1 < V1)
+ c = 9;
+ if (V1 + *P2 < *P2)
+ c = 9;
+}
+
+struct OtherStruct {
+ unsigned foo, bar;
+};
+
+struct MyStruct {
+ unsigned base, offset;
+ struct OtherStruct os;
+};
+
+extern struct MyStruct ms;
+
+void structs(void) {
+ if (ms.base + ms.offset < ms.base)
+ c = 9;
+}
+
+void nestedstructs(void) {
+ if (ms.os.foo + ms.os.bar < ms.os.foo)
+ c = 9;
+}
+
+// Normally, this would be folded into a simple call to the overflow handler
+// and a store. Excluding this idiom results in just a store.
+void constants(void) {
+ unsigned base = 4294967295;
+ unsigned offset = 1;
+ if (base + offset < base)
+ c = 9;
+}
+
+void common_while(unsigned i) {
+ // This post-decrement usually causes overflow sanitizers to trip on the very
+ // last operation.
+ while (i--) {
+ some();
+ }
+}
+
+// NEGATION-LABEL,NEGATIONOV-LABEL: define{{.*}}negation_overflow
+// NEGATION-NOT: negate_overflow
+// NEGATIONOV: negate_overflow
+// Normally, these assignments would trip the unsigned overflow sanitizer.
+void negation_overflow(void) {
+#define SOME -1UL
+ unsigned long A = -1UL;
+ unsigned long B = -2UL;
+ unsigned long C = -3UL;
+ unsigned long D = -1337UL;
+ (void)A;(void)B;(void)C;(void)D;
+}
+
+// cvise'd kernel code that caused problems during development due to sign
+// extension
+typedef unsigned long size_t;
+int qnbytes;
+int *key_alloc_key;
+size_t key_alloc_quotalen;
+int *key_alloc(void) {
+ if (qnbytes + key_alloc_quotalen < qnbytes)
+ return key_alloc_key;
+ return key_alloc_key + 3;;
+}
+
+void function_call(void) {
+ if (b + some() < b)
+ c = 9;
+}
>From 1dd1cbb1b13d0b038fa511620af993238a5bb260 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinst...@google.com>
Date: Tue, 23 Jul 2024 22:44:47 +0000
Subject: [PATCH 2/3] add docs
Signed-off-by: Justin Stitt <justinst...@google.com>
---
clang/docs/ReleaseNotes.rst | 26 ++++++++++++++++++
clang/docs/UndefinedBehaviorSanitizer.rst | 32 +++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ac1de0db9ce48..443ab1456cfcb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -230,6 +230,32 @@ Moved checkers
Sanitizers
----------
+- Added the ``-fno-sanitize-overflow-idioms`` flag which disables integer
+ overflow and truncation sanitizer instrumentation for specific
+ overflow-dependent code patterns. The noise created by these idioms is a
+ large reason as to why large projects refuse to turn on arithmetic
+ sanitizers.
+
+ .. code-block:: c++
+
+ void negation_overflow() {
+ unsigned long foo = -1UL; // No longer causes a negation overflow
warning
+ unsigned long bar = -2UL; // and so on...
+ }
+
+ void while_post_decrement() {
+ unsigned char count = 16;
+ while (count--) { /* ... */} // No longer causes
unsigned-integer-overflow sanitizer to trip
+ }
+
+ int common_overflow_check_pattern(unsigned base, unsigned offset) {
+ if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`,
and other re-orderings, won't be instrumented
+ }
+
+ Note that the ``-fsanitize-overflow-idioms`` flag now also exists but has
+ virtually no function other than to disable an already present
+ ``-fno-sanitize-overflow-idioms``.
+
Python Binding Changes
----------------------
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst
b/clang/docs/UndefinedBehaviorSanitizer.rst
index 531d56e313826..b856f601aec81 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -312,6 +312,38 @@ This attribute may not be
supported by other compilers, so consider using it together with
``#if defined(__clang__)``.
+Disabling instrumentation of common overflow idioms
+=====================================================
+
+There are certain overflow-dependent code patterns which produce a lot of noise
+for integer overflow/truncation sanitizers. To disable instrumentation for
+these common patterns one should use ``-fno-sanitize-overflow-idioms``. Its
+inverse ``-fsanitize-overflow-idioms`` also exists but has no function other
+than to disable an already present ``-fno-sanitize-overflow-idioms``.
+
+Currently, this option handles three pervasive overflow-dependent code idioms:
+
+.. code-block:: c++
+
+ unsigned long foo = -1UL; // No longer causes a negation overflow warning
+ unsigned long bar = -2UL; // and so on...
+
+.. code-block:: c++
+
+ unsigned char count = 16;
+ while (count--) { /* ... */ } // No longer causes
unsigned-integer-overflow sanitizer to trip
+
+.. code-block:: c++
+
+ if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and
other re-orderings,
+ // won't be instrumented (same for
signed types)
+
+Negated unsigned constants, post-decrements in a while loop condition and
+simple overflow checks are accepted and pervasive code patterns. Existing
+projects that enable more warnings or sanitizers may find that the compiler is
+too noisy. Now, they can use ``-fno-sanitize-overflow-idioms`` with no source
+modifications.
+
Suppressing Errors in Recompiled Code (Ignorelist)
--------------------------------------------------
>From ac15444b318f38da1e5cc0e8bb7b5af53bc68971 Mon Sep 17 00:00:00 2001
From: Justin Stitt <justinst...@google.com>
Date: Tue, 23 Jul 2024 23:08:23 +0000
Subject: [PATCH 3/3] format
Signed-off-by: Justin Stitt <justinst...@google.com>
---
clang/include/clang/AST/Expr.h | 4 +---
clang/lib/AST/Expr.cpp | 5 +++--
clang/lib/CodeGen/CGExprScalar.cpp | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index c329ee061cf00..e8dc22c51a03d 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4019,9 +4019,7 @@ class BinaryOperator : public Expr {
return isShiftAssignOp(getOpcode());
}
- bool ignoreOverflowSanitizers() const {
- return isOverflowIdiom;
- }
+ bool ignoreOverflowSanitizers() const { return isOverflowIdiom; }
/// Return true if a binary operator using the specified opcode and operands
/// would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index c07560c92100d..d963d473442d8 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4764,7 +4764,8 @@ namespace {
/// sanitization disabled. Check for the common pattern `if (a + b < a)` and
/// return the resulting BinaryOperator responsible for the addition so we can
/// elide overflow checks during codegen.
-static std::optional<BinaryOperator *> getOverflowIdiomBinOp(const
BinaryOperator *E) {
+static std::optional<BinaryOperator *>
+getOverflowIdiomBinOp(const BinaryOperator *E) {
Expr *Addition, *ComparedTo;
if (E->getOpcode() == BO_LT) {
Addition = E->getLHS();
@@ -4819,7 +4820,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx,
Expr *lhs, Expr *rhs,
SubExprs[LHS] = lhs;
SubExprs[RHS] = rhs;
if (!Ctx.getLangOpts().SanitizeOverflowIdioms) {
- std::optional<BinaryOperator*> Result = getOverflowIdiomBinOp(this);
+ std::optional<BinaryOperator *> Result = getOverflowIdiomBinOp(this);
if (Result.has_value())
Result.value()->isOverflowIdiom = true;
}
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index be0307073f3b9..43a3a333fa76f 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -24,8 +24,8 @@
#include "clang/AST/Attr.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/Expr.h"
-#include "clang/AST/RecordLayout.h"
#include "clang/AST/ParentMapContext.h"
+#include "clang/AST/RecordLayout.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Basic/CodeGenOptions.h"
#include "clang/Basic/TargetInfo.h"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
