[clang] [Sema] Fix tautological bounds check warning with -fwrapv (PR #120480)
Nathan Chancellor via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 18 13:09:45 PST 2024
https://github.com/nathanchance created https://github.com/llvm/llvm-project/pull/120480
The tautological bounds check warning added in #120222 does not take into account whether signed integer overflow is well defined or not, which could result in a developer removing a bounds check that may not actually be always false because of different overflow semantics.
```c
int check(const int* foo, unsigned int idx)
{
return foo + idx < foo;
}
```
```
$ clang -O2 -c test.c
test.c:3:19: warning: pointer comparison always evaluates to false [-Wtautological-compare]
3 | return foo + idx < foo;
| ^
1 warning generated.
# Bounds check is eliminated without -fwrapv, warning was correct
$ llvm-objdump -dr test.o
...
0000000000000000 <check>:
0: 31 c0 xorl %eax, %eax
2: c3 retq
```
```
$ clang -O2 -fwrapv -c test.c
test.c:3:19: warning: pointer comparison always evaluates to false [-Wtautological-compare]
3 | return foo + idx < foo;
| ^
1 warning generated.
# Bounds check remains, warning was wrong
$ llvm-objdump -dr test.o
0000000000000000 <check>:
0: 89 f0 movl %esi, %eax
2: 48 8d 0c 87 leaq (%rdi,%rax,4), %rcx
6: 31 c0 xorl %eax, %eax
8: 48 39 f9 cmpq %rdi, %rcx
b: 0f 92 c0 setb %al
e: c3 retq
```
Prevent the warning from firing when `-fwrapv` is enabled.
>From 0eb68a5e438701a92dcedefc26a99c8dd48d0bed Mon Sep 17 00:00:00 2001
From: Nathan Chancellor <nathan at kernel.org>
Date: Wed, 18 Dec 2024 14:03:14 -0700
Subject: [PATCH] [Sema] Fix tautological bounds check warning with -fwrapv
The tautological bounds check warning added in #120222 does not take
into account whether signed integer overflow is well defined or not,
which could result in a developer removing a bounds check that may not
actually be always false because of different overflow semantics.
$ cat test.c
int check(const int* foo, unsigned int idx)
{
return foo + idx < foo;
}
$ clang -O2 -c test.c
test.c:3:19: warning: pointer comparison always evaluates to false [-Wtautological-compare]
3 | return foo + idx < foo;
| ^
1 warning generated.
# Bounds check is eliminated without -fwrapv, warning was correct
$ llvm-objdump -dr test.o
...
0000000000000000 <check>:
0: 31 c0 xorl %eax, %eax
2: c3 retq
$ clang -O2 -fwrapv -c test.c
test.c:3:19: warning: pointer comparison always evaluates to false [-Wtautological-compare]
3 | return foo + idx < foo;
| ^
1 warning generated.
# Bounds check remains, warning was wrong
$ llvm-objdump -dr test.o
0000000000000000 <check>:
0: 89 f0 movl %esi, %eax
2: 48 8d 0c 87 leaq (%rdi,%rax,4), %rcx
6: 31 c0 xorl %eax, %eax
8: 48 39 f9 cmpq %rdi, %rcx
b: 0f 92 c0 setb %al
e: c3 retq
Prevent the warning from firing when -fwrapv is enabled.
---
clang/lib/Sema/SemaExpr.cpp | 7 +--
.../Sema/tautological-pointer-comparison.c | 50 +++++++++++++++----
2 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e06a092177ef02..24f7d27c691154 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11789,10 +11789,11 @@ 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(const Expr *LHS,
+static std::optional<bool> isTautologicalBoundsCheck(Sema &S, const Expr *LHS,
const Expr *RHS,
BinaryOperatorKind Opc) {
- if (!LHS->getType()->isPointerType())
+ if (!LHS->getType()->isPointerType() ||
+ S.getLangOpts().isSignedOverflowDefined())
return std::nullopt;
// Canonicalize to >= or < predicate.
@@ -11940,7 +11941,7 @@ static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
<< 1 /*array comparison*/
<< Result);
} else if (std::optional<bool> Res =
- isTautologicalBoundsCheck(LHS, RHS, Opc)) {
+ isTautologicalBoundsCheck(S, LHS, RHS, Opc)) {
S.DiagRuntimeBehavior(Loc, nullptr,
S.PDiag(diag::warn_comparison_always)
<< 2 /*pointer comparison*/
diff --git a/clang/test/Sema/tautological-pointer-comparison.c b/clang/test/Sema/tautological-pointer-comparison.c
index 19cd20e5f7d21c..22734ecab6a2f3 100644
--- a/clang/test/Sema/tautological-pointer-comparison.c
+++ b/clang/test/Sema/tautological-pointer-comparison.c
@@ -1,40 +1,72 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DFWRAPV -fwrapv -verify %s
+
+#ifdef FWRAPV
+// expected-no-diagnostics
+#endif
int add_ptr_idx_ult_ptr(const char *ptr, unsigned index) {
- return ptr + index < ptr; // expected-warning {{pointer comparison always evaluates to false}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to false}}
+#endif
+ return ptr + index < ptr;
}
int add_idx_ptr_ult_ptr(const char *ptr, unsigned index) {
- return index + ptr < ptr; // expected-warning {{pointer comparison always evaluates to false}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to false}}
+#endif
+ return index + ptr < ptr;
}
int ptr_ugt_add_ptr_idx(const char *ptr, unsigned index) {
- return ptr > ptr + index; // expected-warning {{pointer comparison always evaluates to false}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to false}}
+#endif
+ return ptr > ptr + index;
}
int ptr_ugt_add_idx_ptr(const char *ptr, unsigned index) {
- return ptr > index + ptr; // expected-warning {{pointer comparison always evaluates to false}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to false}}
+#endif
+ return ptr > index + ptr;
}
int add_ptr_idx_uge_ptr(const char *ptr, unsigned index) {
- return ptr + index >= ptr; // expected-warning {{pointer comparison always evaluates to true}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to true}}
+#endif
+ return ptr + index >= ptr;
}
int add_idx_ptr_uge_ptr(const char *ptr, unsigned index) {
- return index + ptr >= ptr; // expected-warning {{pointer comparison always evaluates to true}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to true}}
+#endif
+ return index + ptr >= ptr;
}
int ptr_ule_add_ptr_idx(const char *ptr, unsigned index) {
- return ptr <= ptr + index; // expected-warning {{pointer comparison always evaluates to true}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to true}}
+#endif
+ return ptr <= ptr + index;
}
int ptr_ule_add_idx_ptr(const char *ptr, unsigned index) {
- return ptr <= index + ptr; // expected-warning {{pointer comparison always evaluates to true}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to true}}
+#endif
+ return ptr <= index + ptr;
}
int add_ptr_idx_ult_ptr_array(unsigned index) {
char ptr[10];
- return ptr + index < ptr; // expected-warning {{pointer comparison always evaluates to false}}
+#ifndef FWRAPV
+ // expected-warning at +2 {{pointer comparison always evaluates to false}}
+#endif
+ return ptr + index < ptr;
}
// Negative tests with wrong predicate.
More information about the cfe-commits
mailing list