Skip to content

Commit f0dcf32

Browse files
authored
[Sema] Fix tautological bounds check warning with -fwrapv (#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 ```
1 parent dc72ec8 commit f0dcf32

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

clang/lib/Sema/SemaExpr.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11789,10 +11789,11 @@ static bool checkForArray(const Expr *E) {
1178911789
/// Detect patterns ptr + size >= ptr and ptr + size < ptr, where ptr is a
1179011790
/// pointer and size is an unsigned integer. Return whether the result is
1179111791
/// always true/false.
11792-
static std::optional<bool> isTautologicalBoundsCheck(const Expr *LHS,
11792+
static std::optional<bool> isTautologicalBoundsCheck(Sema &S, const Expr *LHS,
1179311793
const Expr *RHS,
1179411794
BinaryOperatorKind Opc) {
11795-
if (!LHS->getType()->isPointerType())
11795+
if (!LHS->getType()->isPointerType() ||
11796+
S.getLangOpts().isSignedOverflowDefined())
1179611797
return std::nullopt;
1179711798

1179811799
// Canonicalize to >= or < predicate.
@@ -11940,7 +11941,7 @@ static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
1194011941
<< 1 /*array comparison*/
1194111942
<< Result);
1194211943
} else if (std::optional<bool> Res =
11943-
isTautologicalBoundsCheck(LHS, RHS, Opc)) {
11944+
isTautologicalBoundsCheck(S, LHS, RHS, Opc)) {
1194411945
S.DiagRuntimeBehavior(Loc, nullptr,
1194511946
S.PDiag(diag::warn_comparison_always)
1194611947
<< 2 /*pointer comparison*/

clang/test/Sema/tautological-pointer-comparison.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
// RUN: %clang_cc1 -fsyntax-only -verify %s
2+
// RUN: %clang_cc1 -fsyntax-only -fwrapv -verify=fwrapv %s
3+
4+
// fwrapv-no-diagnostics
25

36
int add_ptr_idx_ult_ptr(const char *ptr, unsigned index) {
47
return ptr + index < ptr; // expected-warning {{pointer comparison always evaluates to false}}

0 commit comments

Comments
 (0)