https://github.com/nikic created https://github.com/llvm/llvm-project/pull/120222
This diagnoses comparisons like `ptr + unsigned_index < ptr` and `ptr + unsigned_index >= ptr`, which are always false/true because addition of a pointer and an unsigned index cannot wrap (or the behavior is undefined). This warning is intended to help find broken bounds checks (which must be implemented in terms of uintptr_t instead). Fixes https://github.com/llvm/llvm-project/issues/120214. >From 66b5b0d72b49539206794c4472ee6fb14f00c5a7 Mon Sep 17 00:00:00 2001 From: Nikita Popov <npo...@redhat.com> Date: Tue, 17 Dec 2024 13:10:30 +0100 Subject: [PATCH] [Sema] Diagnose tautological bounds checks This diagnoses comparisons like `ptr + unsigned_index < ptr` and `ptr + unsigned_index >= ptr`, which are always false/true because addition of a pointer and an unsigned index cannot wrap (or the behavior is undefined). This warning is intended to help find broken bounds checks (which must be implemented in terms of uintptr_t instead). Fixes https://github.com/llvm/llvm-project/issues/120214. --- .../clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaExpr.cpp | 49 +++++++++++++ .../Sema/tautological-pointer-comparison.c | 69 +++++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/tautological-pointer-comparison.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 9344b620779b84..d67a81f8564a8e 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<TautologicalCompare>; def warn_comparison_bitwise_always : Warning< diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 20bf6f7f6f28ff..4e814972d5c978 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -11786,6 +11786,49 @@ 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<bool> isTautologicalBoundsCheck(Expr *LHS, 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<BinaryOperator>(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 +11938,12 @@ static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc, S.PDiag(diag::warn_comparison_always) << 1 /*array comparison*/ << Result); + } else if (std::optional<bool> 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 00000000000000..f3f7c7da8d6907 --- /dev/null +++ b/clang/test/Sema/tautological-pointer-comparison.c @@ -0,0 +1,69 @@ +// 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}} +} + +// 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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
