[clang] [Clang] Force expressions with UO_Not to not be non-negative (PR #126846)
Yutong Zhu via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 7 20:41:49 PST 2025
https://github.com/YutongZhuu updated https://github.com/llvm/llvm-project/pull/126846
>From d7404029e8998c8c8945cfaa34cf99b743ec2b70 Mon Sep 17 00:00:00 2001
From: Yutong Zhu <y25zhu at uwaterloo.ca>
Date: Sun, 23 Feb 2025 18:16:06 -0500
Subject: [PATCH 1/4] Fix no warning for comparison of integers of different
signs
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/Sema/SemaChecking.cpp | 18 ++++++++++++++++++
clang/test/Sema/compare.c | 8 ++++++++
3 files changed, 29 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6344c4b36e357..e8de334c93a2e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -135,6 +135,9 @@ Improvements to Clang's diagnostics
- Fixed a bug where Clang's Analysis did not correctly model the destructor behavior of ``union`` members (#GH119415).
- A statement attribute applied to a ``case`` label no longer suppresses
'bypassing variable initialization' diagnostics (#84072).
+- The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers
+ except for the case where the operand is an unsigned integer
+ and throws warning if they are compared with unsigned integers (##18878).
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 66c233de4ef30..8dd586f8ab2a8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10069,6 +10069,24 @@ static std::optional<IntRange> TryGetExprRange(ASTContext &C, const Expr *E,
case UO_AddrOf: // should be impossible
return IntRange::forValueOfType(C, GetExprType(E));
+ case UO_Minus:
+ case UO_Not: {
+ if (E->getType()->isUnsignedIntegerType()) {
+ return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
+ Approximate);
+ }
+
+ std::optional<IntRange> SubRange = TryGetExprRange(
+ C, UO->getSubExpr(), MaxWidth, InConstantContext, Approximate);
+
+ if (!SubRange)
+ return std::nullopt;
+
+ // The width increments by 1 if the sub-expression cannot be negative
+ // since it now can be.
+ return IntRange(SubRange->Width + (int)SubRange->NonNegative, false);
+ }
+
default:
return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
Approximate);
diff --git a/clang/test/Sema/compare.c b/clang/test/Sema/compare.c
index 17cf0351ef4f5..950793631c38c 100644
--- a/clang/test/Sema/compare.c
+++ b/clang/test/Sema/compare.c
@@ -419,3 +419,11 @@ void pr36008(enum PR36008EnumTest lhs) {
if (x == y) x = y; // no warning
if (y == x) y = x; // no warning
}
+
+int test13(unsigned a, int b) {
+ return a > ~(95 != b); // expected-warning {{comparison of integers of different signs}}
+}
+
+int test14(unsigned a, int b) {
+ return a > -(95 != b); // expected-warning {{comparison of integers of different signs}}
+}
>From 3dfac146b0af7a7d41cb4ec3f8409bc3fc9ebaf8 Mon Sep 17 00:00:00 2001
From: Yutong Zhu <y25zhu at uwaterloo.ca>
Date: Sun, 2 Mar 2025 14:24:01 -0500
Subject: [PATCH 2/4] Fix incorrect range for UO_Minus
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/Sema/SemaChecking.cpp | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c73e99919b838..db53121c03a51 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -201,6 +201,7 @@ Improvements to Clang's diagnostics
under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
``-Wno-error=parentheses``.
+
- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-pointer``,
which enables warning on passing or returning pointers to guarded variables
as function arguments or return value respectively. Note that
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 4d5a318f7a01f..082613ab07102 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10619,7 +10619,20 @@ static std::optional<IntRange> TryGetExprRange(ASTContext &C, const Expr *E,
case UO_AddrOf: // should be impossible
return IntRange::forValueOfType(C, GetExprType(E));
- case UO_Minus:
+ case UO_Minus: {
+ if (E->getType()->isUnsignedIntegerType()) {
+ return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
+ Approximate);
+ }
+
+ std::optional<IntRange> SubRange = TryGetExprRange(
+ C, UO->getSubExpr(), MaxWidth, InConstantContext, Approximate);
+
+ if (!SubRange)
+ return std::nullopt;
+
+ return IntRange(SubRange->Width + 1, false);
+ }
case UO_Not: {
if (E->getType()->isUnsignedIntegerType()) {
return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
>From 19503398a0d06e923c5ec792bf12ee51ae5d54be Mon Sep 17 00:00:00 2001
From: Yutong Zhu <y25zhu at uwaterloo.ca>
Date: Sun, 2 Mar 2025 20:27:19 -0500
Subject: [PATCH 3/4] Add additional test and comment
---
clang/lib/Sema/SemaChecking.cpp | 5 +++++
clang/test/Sema/compare.c | 18 +++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 082613ab07102..84d4341e323d3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10631,8 +10631,13 @@ static std::optional<IntRange> TryGetExprRange(ASTContext &C, const Expr *E,
if (!SubRange)
return std::nullopt;
+ // If the range was previously non-negative, we need an extra bit for the
+ // sign bit. If the range was not non-negative, we need an extra bit
+ // because the negation of the most-negative value is one bit wider than
+ // that value.
return IntRange(SubRange->Width + 1, false);
}
+
case UO_Not: {
if (E->getType()->isUnsignedIntegerType()) {
return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
diff --git a/clang/test/Sema/compare.c b/clang/test/Sema/compare.c
index 950793631c38c..4234a43379620 100644
--- a/clang/test/Sema/compare.c
+++ b/clang/test/Sema/compare.c
@@ -424,6 +424,22 @@ int test13(unsigned a, int b) {
return a > ~(95 != b); // expected-warning {{comparison of integers of different signs}}
}
-int test14(unsigned a, int b) {
+int test14(unsigned a, unsigned b) {
+ return a > ~b; // no-warning
+}
+
+int test15(unsigned a, int b) {
return a > -(95 != b); // expected-warning {{comparison of integers of different signs}}
}
+
+int test16(unsigned a, unsigned b) {
+ return a > -b; // no-warning
+}
+
+int test17(int a, unsigned b) {
+ return a > --b; // expected-warning {{comparison of integers of different signs}}
+}
+
+int test_negation(int a) {
+ return a == -(-2147483648); // expected-warning {{result of comparison of constant 2147483648 with expression of type 'int' is always false}}
+}
>From bb3dea2bb5ee658805008f030b6922d2b3f4a638 Mon Sep 17 00:00:00 2001
From: Yutong Zhu <y25zhu at uwaterloo.ca>
Date: Mon, 3 Mar 2025 23:08:39 -0500
Subject: [PATCH 4/4] Add more tests for range checking
---
clang/test/Sema/compare.c | 54 +++++++++++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 8 deletions(-)
diff --git a/clang/test/Sema/compare.c b/clang/test/Sema/compare.c
index 4234a43379620..fdae3bc19841e 100644
--- a/clang/test/Sema/compare.c
+++ b/clang/test/Sema/compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-in-range-compare %s -Wno-unreachable-code
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtype-limits %s -Wno-unreachable-code
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-in-range-compare %s -Wno-unreachable-code -DTEST=1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtype-limits %s -Wno-unreachable-code -DTEST=2
int test(char *C) { // nothing here should warn.
return C != ((void*)0);
@@ -421,25 +421,63 @@ void pr36008(enum PR36008EnumTest lhs) {
}
int test13(unsigned a, int b) {
- return a > ~(95 != b); // expected-warning {{comparison of integers of different signs}}
+ return a > ~(95 != b); // expected-warning {{comparison of integers of different signs}}
}
int test14(unsigned a, unsigned b) {
- return a > ~b; // no-warning
+ return a > ~b; // no-warning
}
int test15(unsigned a, int b) {
- return a > -(95 != b); // expected-warning {{comparison of integers of different signs}}
+ return a > -(95 != b); // expected-warning {{comparison of integers of different signs}}
}
int test16(unsigned a, unsigned b) {
- return a > -b; // no-warning
+ return a > -b; // no-warning
}
int test17(int a, unsigned b) {
- return a > --b; // expected-warning {{comparison of integers of different signs}}
+ return a > -(-b); // expected-warning {{comparison of integers of different signs}}
}
-int test_negation(int a) {
+int test18(int a) {
return a == -(-2147483648); // expected-warning {{result of comparison of constant 2147483648 with expression of type 'int' is always false}}
}
+
+int test19(int n) {
+ return -(n & 15) <= -15; // no-warning
+}
+
+#if TEST == 1
+int test20(int n) {
+ return -(n & 15) <= -17; // expected-warning {{result of comparison of 5-bit signed value <= -17 is always false}}
+}
+#endif
+
+int test21(short n) {
+ return -n == 32768; // no-warning
+}
+
+#if TEST == 1
+int test22(short n) {
+ return -n == 65536; // expected-warning {{result of comparison of 17-bit signed value == 65536 is always false}}
+}
+#endif
+
+int test23(unsigned short n) {
+ return ~n == 32768; // no-warning
+}
+
+int test24(short n) {
+ return ~n == 32767; // no-warning
+}
+
+#if TEST == 1
+int test25(unsigned short n) {
+ return ~n == 65536; // expected-warning {{result of comparison of 17-bit signed value == 65536 is always false}}
+}
+
+int test26(short n) {
+ return ~n == 32768; // expected-warning {{result of comparison of 16-bit signed value == 32768 is always false}}
+}
+#endif
More information about the cfe-commits
mailing list