[clang] [Clang] Overflow Idiom Exclusions (PR #100272)
Justin Stitt via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 23 16:03:53 PDT 2024
https://github.com/JustinStitt created https://github.com/llvm/llvm-project/pull/100272
## Summary
Introduce `-fno-sanitize-overflow-idioms` which disables sanitizer instrumentation for common overflow-dependent code idioms.
## Background
For a wide selection of projects, proper overflow sanitization could help catch bugs and solve security vulnerabilities. Unfortunately, in some cases the integer overflow sanitizers are too noisy for their users and are often left disabled. Providing users with a method to disable sanitizer instrumentation of common patterns could mean more projects actually utilize the sanitizers in the first place.
One such project that has opted to not use integer overflow (or truncation) sanitizers is the Linux Kernel. There has been [some discussion](https://lore.kernel.org/all/202404291502.612E0A10@keescook/) recently concerning mitigation strategies for unexpected arithmetic overflow. This discussion is still ongoing and [a succinct article](https://lwn.net/Articles/979747/) accurately sums up the discussion. In summary, many Kernel developers do not want to introduce more arithmetic wrappers when **most developers understand the code patterns as they are**.
Patterns like:
``` c
if (base + offset < base) { ... }
```
or
``` c
while (i--) { ... }
```
or
``` c
#define SOME -1UL
```
… are extremely common in a code base like the Linux Kernel. It is perhaps too much to ask of kernel developers to use arithmetic wrappers in these cases. For example:
``` c
while (wrapping_post_dec(i)) { ... }
```
… which wraps some builtin would not fly. This would incur too many changes to existing code; the code churn would be too much, at least too much to justify turning on overflow sanitizers.
User Cyberax (from [lwn.net](http://lwn.net/)) probably shares a similar question to you:
> I don’t understand why Linux developers won’t just annotate the expected wraparounds. They are used somewhat frequently, but not so much as to affect the readability. Most source code files in the tree won’t have any annotations at all.
but then realizes:
> “I think, the kernel developers just like to be able to read the overflow-dependent code idioms.”
## Prevailing Sentiment
It'd be nice if we could turn on overflow sanitizers for the Linux Kernel (and other projects) without causing too much noise from the compiler. So, we need to disable instrumentation of the noisy code patterns and generally leave the source code intact. This will improve the signal-to-noise ratio and help us find **real** bugs.
## Examples
Currently, this PR tackles three pervasive idioms:
`if (a + b < a)` or some logically-equivalent re-ordering like `if (a > b + a)`
`while (i--)` (for unsigned) a post-decrement always overflows here
`-1UL, -2UL, etc` negation of unsigned constants will always overflow
All of which are disabled when `-fno-sanitize-overflow-idioms` is ticked on.
## Implementation Details
All three idiom exclusions are handled within Clang itself. Initially the `if ( a + b < a )` idiom was handled as an IR optimization pass but feedback from the [RFC](https://discourse.llvm.org/t/rfc-overflow-idiom-exclusion/80093/7) led me to implementing this in only Clang.
---
CCs: @efriedma-quic @kees @jyknight @fmayer @vitalybuka
>From 06b702cd38943314b2e6f873e64d70baed6f57f7 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 1/2] implement idiom exclusions
Add flag `-fno-sanitize-overflow-idioms` which disables integer
overflow/truncation sanitizer instrumentation for common
overflow-dependent code patterns.
Signed-off-by: Justin Stitt <justinstitt at google.com>
---
clang/include/clang/AST/Expr.h | 5 +
clang/include/clang/Basic/LangOptions.def | 2 +
clang/include/clang/Driver/Options.td | 10 ++
clang/include/clang/Driver/SanitizerArgs.h | 1 +
clang/lib/AST/Expr.cpp | 53 +++++++
clang/lib/CodeGen/CGExprScalar.cpp | 26 +++-
clang/lib/Driver/SanitizerArgs.cpp | 7 +
clang/lib/Driver/ToolChains/Clang.cpp | 7 +
.../CodeGen/overflow-idiom-exclusion-fp.c | 77 ++++++++++
clang/test/CodeGen/overflow-idiom-exclusion.c | 141 ++++++++++++++++++
10 files changed, 327 insertions(+), 2 deletions(-)
create mode 100644 clang/test/CodeGen/overflow-idiom-exclusion-fp.c
create mode 100644 clang/test/CodeGen/overflow-idiom-exclusion.c
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 5b813bfc2faf9..c329ee061cf00 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3860,6 +3860,7 @@ class CStyleCastExpr final
class BinaryOperator : public Expr {
enum { LHS, RHS, END_EXPR };
Stmt *SubExprs[END_EXPR];
+ bool isOverflowIdiom;
public:
typedef BinaryOperatorKind Opcode;
@@ -4018,6 +4019,10 @@ class BinaryOperator : public Expr {
return isShiftAssignOp(getOpcode());
}
+ bool ignoreOverflowSanitizers() const {
+ return isOverflowIdiom;
+ }
+
/// Return true if a binary operator using the specified opcode and operands
/// would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized
/// integer to a pointer.
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 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 <justinstitt at google.com>
Date: Tue, 23 Jul 2024 22:44:47 +0000
Subject: [PATCH 2/2] 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 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)
--------------------------------------------------
More information about the cfe-commits
mailing list