[clang] [Sema] Diagnose tautological bounds checks (PR #120222)
Nikita Popov via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 17 04:40:45 PST 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/120222
>From 66b5b0d72b49539206794c4472ee6fb14f00c5a7 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 17 Dec 2024 13:10:30 +0100
Subject: [PATCH 1/4] [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;
+}
>From d974240640b5132206ab25a96c1f4e3e51330d5e Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 17 Dec 2024 13:32:46 +0100
Subject: [PATCH 2/4] Const qualifiers
---
clang/lib/Sema/SemaExpr.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4e814972d5c978..e06a092177ef02 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11789,7 +11789,8 @@ static bool checkForArray(const Expr *E) {
/// 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,
+static std::optional<bool> isTautologicalBoundsCheck(const Expr *LHS,
+ const Expr *RHS,
BinaryOperatorKind Opc) {
if (!LHS->getType()->isPointerType())
return std::nullopt;
>From 9c432bac9041ec30f3cc200be151ac44b91c83d5 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 17 Dec 2024 13:37:07 +0100
Subject: [PATCH 3/4] add changelog entry
---
clang/docs/ReleaseNotes.rst | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 596f3b5142d663..d6dacd87de7652 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 additional 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
----------------------------------
>From 28050288215f278f77f3b07e63334b24cb40f4eb Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 17 Dec 2024 13:39:03 +0100
Subject: [PATCH 4/4] Add test with a C array
---
clang/test/Sema/tautological-pointer-comparison.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang/test/Sema/tautological-pointer-comparison.c b/clang/test/Sema/tautological-pointer-comparison.c
index f3f7c7da8d6907..19cd20e5f7d21c 100644
--- a/clang/test/Sema/tautological-pointer-comparison.c
+++ b/clang/test/Sema/tautological-pointer-comparison.c
@@ -32,6 +32,11 @@ 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) {
More information about the cfe-commits
mailing list