Author: Justin Stitt Date: 2024-08-23T23:33:23-07:00 New Revision: 76236fafda19ff3760443196edcd3cd9610ed733
URL: https://github.com/llvm/llvm-project/commit/76236fafda19ff3760443196edcd3cd9610ed733 DIFF: https://github.com/llvm/llvm-project/commit/76236fafda19ff3760443196edcd3cd9610ed733.diff LOG: [Clang] Overflow Pattern Exclusion - rename some patterns, enhance docs (#105709) >From @vitalybuka's review on https://github.com/llvm/llvm-project/pull/104889: - [x] remove unused variable in tests - [x] rename `post-decr-while` --> `unsigned-post-decr-while` - [x] split `add-overflow-test` into `add-unsigned-overflow-test` and `add-signed-overflow-test` - [x] be more clear about defaults within docs - [x] add table to docs Here's a screenshot of the rendered table so you don't have to build the html docs yourself to inspect the layout:  CCs: @vitalybuka --------- Signed-off-by: Justin Stitt <justinst...@google.com> Co-authored-by: Vitaly Buka <vitalyb...@google.com> Added: Modified: clang/docs/ReleaseNotes.rst clang/docs/UndefinedBehaviorSanitizer.rst clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/Options.td clang/lib/AST/Expr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/Driver/SanitizerArgs.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/ignore-overflow-pattern-false-pos.c clang/test/CodeGen/ignore-overflow-pattern.c Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 798f59009af3c3..0ced2f779f7058 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -466,28 +466,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. diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst index 1c92907372f83c..0d1010b7dcb338 100644 --- a/clang/docs/UndefinedBehaviorSanitizer.rst +++ b/clang/docs/UndefinedBehaviorSanitizer.rst @@ -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 ================= diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index eb4cb4b5a7e93f..1c80ee89837cb3 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -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 { diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 4bf604d46a0f70..1b9b3f2c6600a3 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2570,7 +2570,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>, diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 25ab6f3b2addfb..3309619850f34a 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -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, @@ -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); diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 2a726bba2dd304..af11bc20a3b639 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-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; diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 09262f40b5b50c..18bb35a563167e 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -1457,9 +1457,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) diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index f510d3067d4d58..0bb4175dd021ee 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -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); } } diff --git a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c index 40193e0c3e2671..b4811443b95192 100644 --- a/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c +++ b/clang/test/CodeGen/ignore-overflow-pattern-false-pos.c @@ -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); diff --git a/clang/test/CodeGen/ignore-overflow-pattern.c b/clang/test/CodeGen/ignore-overflow-pattern.c index b7d700258f8538..c4a9d07b07aaac 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-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 @@ -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 @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
