Skip to content

[Clang] Overflow Pattern Exclusion - rename some patterns, enhance docs #105709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -451,28 +451,34 @@ Sanitizers

- 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``.
patterns are: ``add-signed-overflow-test``, ``add-unsigned-overflow-test``,
``negated-unsigned-const``, and ``unsigned-post-decr-while``. The sanitizer
instrumentation can be toggled off for all available patterns by specifying
``all``. Conversely, you may disable all exclusions with ``none`` which is
the default.

.. code-block:: c++

/// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-overflow-test``
/// specified with ``-fsanitize-undefined-ignore-overflow-pattern=add-unsigned-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-undefined-ignore-overflow-pattern=add-signed-overflow-test``
int common_overflow_check_pattern_signed(signed int base, signed int offset) {
if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings, won't be instrumented
}

/// 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-undefined-ignore-overflow-pattern=post-decr-while``
/// specified with ``-fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while``
void while_post_decrement() {
unsigned char count = 16;
while (count--) { /* ... */} // No longer causes unsigned-integer-overflow sanitizer to trip
while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip
}

Many existing projects have a large amount of these code patterns present.
Expand Down
37 changes: 30 additions & 7 deletions clang/docs/UndefinedBehaviorSanitizer.rst
Original file line number Diff line number Diff line change
Expand Up @@ -314,26 +314,49 @@ Currently, this option supports three 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``
``unsigned-post-decr-while``

.. code-block:: c++

/// -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
/// -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while
unsigned char count = 16;
while (count--) { /* ... */ } // No longer causes unsigned-integer-overflow sanitizer to trip

``add-overflow-test``
``add-signed-overflow-test,add-unsigned-overflow-test``

.. code-block:: c++

/// -fsanitize-undefined-ignore-overflow-pattern=add-overflow-test
/// -fsanitize-undefined-ignore-overflow-pattern=add-(signed|unsigned)-overflow-test
if (base + offset < base) { /* ... */ } // The pattern of `a + b < a`, and other re-orderings,
// won't be instrumented (same for signed types)
// won't be instrumented (signed or unsigned types)

.. list-table:: Overflow Pattern Types
:widths: 30 50
:header-rows: 1

* - Pattern
- Sanitizer
* - negated-unsigned-const
- unsigned-integer-overflow
* - unsigned-post-decr-while
- unsigned-integer-overflow
* - add-unsigned-overflow-test
- unsigned-integer-overflow
* - add-signed-overflow-test
- signed-integer-overflow



Note: ``add-signed-overflow-test`` suppresses only the check for Undefined
Behavior. Eager Undefined Behavior optimizations are still possible. One may
remedy this with ``-fwrapv`` or ``-fno-strict-overflow``.

You can enable all exclusions with
``-fsanitize-undefined-ignore-overflow-pattern=all`` or disable all exclusions
with ``-fsanitize-undefined-ignore-overflow-pattern=none``. Specifying ``none``
has precedence over other values.
with ``-fsanitize-undefined-ignore-overflow-pattern=none``. If
``-fsanitize-undefined-ignore-overflow-pattern`` is not specified ``none`` is
implied. Specifying ``none`` alongside other values also implies ``none`` as
``none`` has precedence over other values -- including ``all``.

Issue Suppression
=================
Expand Down
8 changes: 5 additions & 3 deletions clang/include/clang/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,11 +375,13 @@ class LangOptionsBase {
/// Exclude all overflow patterns (below)
All = 1 << 1,
/// if (a + b < a)
AddOverflowTest = 1 << 2,
AddSignedOverflowTest = 1 << 2,
/// if (a + b < a)
AddUnsignedOverflowTest = 1 << 3,
/// -1UL
NegUnsignedConst = 1 << 3,
NegUnsignedConst = 1 << 4,
/// while (count--)
PostDecrInWhile = 1 << 4,
PostDecrInWhile = 1 << 5,
};

enum class DefaultVisiblityExportMapping {
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -2568,7 +2568,7 @@ defm sanitize_stats : BoolOption<"f", "sanitize-stats",
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">,
Values<"none,all,add-unsigned-overflow-test,add-signed-overflow-test,negated-unsigned-const,unsigned-post-decr-while">,
MarshallingInfoStringVector<LangOpts<"OverflowPatternExclusionValues">>;
def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
Group<f_clang_Group>,
Expand Down
27 changes: 21 additions & 6 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4806,6 +4806,26 @@ getOverflowPatternBinOp(const BinaryOperator *E) {
return {};
}

/// Compute and set the OverflowPatternExclusion bit based on whether the
/// BinaryOperator expression matches an overflow pattern being ignored by
/// -fsanitize-undefined-ignore-overflow-pattern=add-signed-overflow-test or
/// -fsanitize-undefined-ignore-overflow-pattern=add-unsigned-overflow-test
static void computeOverflowPatternExclusion(const ASTContext &Ctx,
const BinaryOperator *E) {
std::optional<BinaryOperator *> Result = getOverflowPatternBinOp(E);
if (!Result.has_value())
return;
QualType AdditionResultType = Result.value()->getType();

if ((AdditionResultType->isSignedIntegerType() &&
Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::AddSignedOverflowTest)) ||
(AdditionResultType->isUnsignedIntegerType() &&
Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::AddUnsignedOverflowTest)))
Result.value()->setExcludedOverflowPattern(true);
}

BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
Opcode opc, QualType ResTy, ExprValueKind VK,
ExprObjectKind OK, SourceLocation opLoc,
Expand All @@ -4818,12 +4838,7 @@ BinaryOperator::BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs,
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 = true;
}
computeOverflowPatternExclusion(Ctx, this);
BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
if (hasStoredFPFeatures())
setStoredFPFeatures(FPFeatures);
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/CGExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2785,7 +2785,7 @@ static bool matchesPostDecrInWhile(const UnaryOperator *UO, bool isInc,
if (isInc || isPre)
return false;

// -fsanitize-undefined-ignore-overflow-pattern=post-decr-while
// -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while
if (!Ctx.getLangOpts().isOverflowPatternExcluded(
LangOptions::OverflowPatternExclusionKind::PostDecrInWhile))
return false;
Expand Down
7 changes: 5 additions & 2 deletions clang/lib/Driver/SanitizerArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1453,9 +1453,12 @@ static int parseOverflowPatternExclusionValues(const Driver &D,
llvm::StringSwitch<int>(Value)
.Case("none", LangOptionsBase::None)
.Case("all", LangOptionsBase::All)
.Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
.Case("add-unsigned-overflow-test",
LangOptionsBase::AddUnsignedOverflowTest)
.Case("add-signed-overflow-test",
LangOptionsBase::AddSignedOverflowTest)
.Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
.Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
.Case("unsigned-post-decr-while", LangOptionsBase::PostDecrInWhile)
.Default(0);
if (E == 0)
D.Diag(clang::diag::err_drv_unsupported_option_argument)
Expand Down
8 changes: 6 additions & 2 deletions clang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4274,9 +4274,13 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
llvm::StringSwitch<unsigned>(A->getValue(i))
.Case("none", LangOptionsBase::None)
.Case("all", LangOptionsBase::All)
.Case("add-overflow-test", LangOptionsBase::AddOverflowTest)
.Case("add-unsigned-overflow-test",
LangOptionsBase::AddUnsignedOverflowTest)
.Case("add-signed-overflow-test",
LangOptionsBase::AddSignedOverflowTest)
.Case("negated-unsigned-const", LangOptionsBase::NegUnsignedConst)
.Case("post-decr-while", LangOptionsBase::PostDecrInWhile)
.Case("unsigned-post-decr-while",
LangOptionsBase::PostDecrInWhile)
.Default(0);
}
}
Expand Down
1 change: 0 additions & 1 deletion clang/test/CodeGen/ignore-overflow-pattern-false-pos.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

// 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;

extern unsigned some(void);

Expand Down
7 changes: 5 additions & 2 deletions clang/test/CodeGen/ignore-overflow-pattern.c
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// 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=add-signed-overflow-test,add-unsigned-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
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsanitize=signed-integer-overflow,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-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
Expand All @@ -25,6 +25,7 @@
// CHECK-NOT: handle{{.*}}overflow

extern unsigned a, b, c;
extern int u, v;
extern unsigned some(void);

// ADD-LABEL: @basic_commutativity
Expand All @@ -50,6 +51,8 @@ void basic_commutativity(void) {
c = 9;
if (b > b + a)
c = 9;
if (u + v < u)
c = 9;
}

// ADD-LABEL: @arguments_and_commutativity
Expand Down
Loading