[Sema] Fix tautological bounds check warning with -fwrapv

[nathanchance]

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.

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Nathan Chancellor (nathanchance)

Changes

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.

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/120480.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaExpr.cpp (+4-3)
• (modified) clang/test/Sema/tautological-pointer-comparison.c (+41-9)

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@+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@+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@+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@+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@+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@+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@+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@+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@+2 {{pointer comparison always evaluates to false}} +#endif + return ptr + index < ptr; } // Negative tests with wrong predicate. 

[nikic]

LGTM

[pinskia]

Shouldn't this be part of -Wstrict-overflow instead of -Wtautological-compare ?