Author: Nathan Chancellor Date: 2024-12-18T20:23:50-07:00 New Revision: f0dcf3240dffe3767c7f3a2e2da5b92ae9fd1bef
URL: https://github.com/llvm/llvm-project/commit/f0dcf3240dffe3767c7f3a2e2da5b92ae9fd1bef DIFF: https://github.com/llvm/llvm-project/commit/f0dcf3240dffe3767c7f3a2e2da5b92ae9fd1bef.diff LOG: [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 ``` Added: Modified: clang/lib/Sema/SemaExpr.cpp clang/test/Sema/tautological-pointer-comparison.c Removed: ################################################################################ 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..1c5973b01a30d0 100644 --- a/clang/test/Sema/tautological-pointer-comparison.c +++ b/clang/test/Sema/tautological-pointer-comparison.c @@ -1,4 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -fwrapv -verify=fwrapv %s + +// fwrapv-no-diagnostics int add_ptr_idx_ult_ptr(const char *ptr, unsigned index) { return ptr + index < ptr; // expected-warning {{pointer comparison always evaluates to false}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
