diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 596f3b5142d66..9961797704b04 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -683,6 +683,16 @@ Improvements to Clang's diagnostics views.push_back(std::string("123")); // warning } +- Clang now emits a ``-Wtautological-compare`` diagnostic when a check for + pointer addition overflow is always true or false, because overflow would + be undefined behavior. + + .. code-block:: c++ + + bool incorrect_overflow_check(const char *ptr, size_t index) { + return ptr + index < ptr; // warning + } + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 9344b620779b8..d67a81f8564a8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10246,7 +10246,7 @@ def warn_dangling_reference_captured_by_unknown : Warning< // should result in a warning, since these always evaluate to a constant. // Array comparisons have similar warnings def warn_comparison_always : Warning< - "%select{self-|array }0comparison always evaluates to " + "%select{self-|array |pointer }0comparison always evaluates to " "%select{a constant|true|false|'std::strong_ordering::equal'}1">, InGroup; def warn_comparison_bitwise_always : Warning< diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 20bf6f7f6f28f..e06a092177ef0 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -11786,6 +11786,50 @@ static bool checkForArray(const Expr *E) { return D->getType()->isArrayType() && !D->isWeak(); } +/// Detect patterns ptr + size >= ptr and ptr + size < ptr, where ptr is a +/// pointer and size is an unsigned integer. Return whether the result is +/// always true/false. +static std::optional isTautologicalBoundsCheck(const Expr *LHS, + const Expr *RHS, + BinaryOperatorKind Opc) { + if (!LHS->getType()->isPointerType()) + return std::nullopt; + + // Canonicalize to >= or < predicate. + switch (Opc) { + case BO_GE: + case BO_LT: + break; + case BO_GT: + std::swap(LHS, RHS); + Opc = BO_LT; + break; + case BO_LE: + std::swap(LHS, RHS); + Opc = BO_GE; + break; + default: + return std::nullopt; + } + + auto *BO = dyn_cast(LHS); + if (!BO || BO->getOpcode() != BO_Add) + return std::nullopt; + + Expr *Other; + if (Expr::isSameComparisonOperand(BO->getLHS(), RHS)) + Other = BO->getRHS(); + else if (Expr::isSameComparisonOperand(BO->getRHS(), RHS)) + Other = BO->getLHS(); + else + return std::nullopt; + + if (!Other->getType()->isUnsignedIntegerType()) + return std::nullopt; + + return Opc == BO_GE; +} + /// Diagnose some forms of syntactically-obvious tautological comparison. static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc, Expr *LHS, Expr *RHS, @@ -11895,6 +11939,12 @@ static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc, S.PDiag(diag::warn_comparison_always) << 1 /*array comparison*/ << Result); + } else if (std::optional Res = + isTautologicalBoundsCheck(LHS, RHS, Opc)) { + S.DiagRuntimeBehavior(Loc, nullptr, + S.PDiag(diag::warn_comparison_always) + << 2 /*pointer comparison*/ + << (*Res ? AlwaysTrue : AlwaysFalse)); } } diff --git a/clang/test/Sema/tautological-pointer-comparison.c b/clang/test/Sema/tautological-pointer-comparison.c new file mode 100644 index 0000000000000..19cd20e5f7d21 --- /dev/null +++ b/clang/test/Sema/tautological-pointer-comparison.c @@ -0,0 +1,74 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +int add_ptr_idx_ult_ptr(const char *ptr, unsigned index) { + return ptr + index < ptr; // expected-warning {{pointer comparison always evaluates to false}} +} + +int add_idx_ptr_ult_ptr(const char *ptr, unsigned index) { + return index + ptr < ptr; // expected-warning {{pointer comparison always evaluates to false}} +} + +int ptr_ugt_add_ptr_idx(const char *ptr, unsigned index) { + return ptr > ptr + index; // expected-warning {{pointer comparison always evaluates to false}} +} + +int ptr_ugt_add_idx_ptr(const char *ptr, unsigned index) { + return ptr > index + ptr; // expected-warning {{pointer comparison always evaluates to false}} +} + +int add_ptr_idx_uge_ptr(const char *ptr, unsigned index) { + return ptr + index >= ptr; // expected-warning {{pointer comparison always evaluates to true}} +} + +int add_idx_ptr_uge_ptr(const char *ptr, unsigned index) { + return index + ptr >= ptr; // expected-warning {{pointer comparison always evaluates to true}} +} + +int ptr_ule_add_ptr_idx(const char *ptr, unsigned index) { + return ptr <= ptr + index; // expected-warning {{pointer comparison always evaluates to true}} +} + +int ptr_ule_add_idx_ptr(const char *ptr, unsigned index) { + return ptr <= index + ptr; // expected-warning {{pointer comparison always evaluates to true}} +} + +int add_ptr_idx_ult_ptr_array(unsigned index) { + char ptr[10]; + return ptr + index < ptr; // expected-warning {{pointer comparison always evaluates to false}} +} + +// Negative tests with wrong predicate. + +int add_ptr_idx_ule_ptr(const char *ptr, unsigned index) { + return ptr + index <= ptr; +} + +int add_ptr_idx_ugt_ptr(const char *ptr, unsigned index) { + return ptr + index > ptr; +} + +int ptr_uge_add_idx_ptr(const char *ptr, unsigned index) { + return ptr >= index + ptr; +} + +int ptr_ult_add_idx_ptr(const char *ptr, unsigned index) { + return ptr < index + ptr; +} + +// Negative test with signed index. + +int add_ptr_idx_ult_ptr_signed(const char *ptr, int index) { + return ptr + index < ptr; +} + +// Negative test with unrelated pointers. + +int add_ptr_idx_ult_ptr2(const char *ptr, const char *ptr2, unsigned index) { + return ptr + index < ptr2; +} + +// Negative test with non-pointer operands. + +int add_ptr_idx_ult_ptr_not_pointer(unsigned ptr, unsigned index) { + return ptr + index < ptr; +}