https://github.com/YutongZhuu updated https://github.com/llvm/llvm-project/pull/126846
>From 106a982e3c6bcfa3ee7c26133f0919791699f31a Mon Sep 17 00:00:00 2001 From: Yutong Zhu <y25...@uwaterloo.ca> Date: Sun, 23 Feb 2025 18:16:06 -0500 Subject: [PATCH 1/5] Fix signed-unsigned comparison with UO_Not and UO_Minus --- 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 307edf77ebb58..03058205e61cd 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -204,6 +204,9 @@ Improvements to Clang's diagnostics as function arguments or return value respectively. Note that :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The feature will be default-enabled with ``-Wthread-safety`` in a future release. +- 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 f9926c6b4adab..4d5a318f7a01f 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -10619,6 +10619,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 b8f947a4cc2795451d6c4f8912859e9ce20502dd Mon Sep 17 00:00:00 2001 From: Yutong Zhu <y25...@uwaterloo.ca> Date: Sun, 2 Mar 2025 14:24:01 -0500 Subject: [PATCH 2/5] Add logic for UO_Minus --- clang/lib/Sema/SemaChecking.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 9f6897d8affa185eac0b4ef790870b06ae97d110 Mon Sep 17 00:00:00 2001 From: Yutong Zhu <y25...@uwaterloo.ca> Date: Sun, 2 Mar 2025 14:24:01 -0500 Subject: [PATCH 3/5] Fix incorrect range for UO_Minus >From b9081bbb6ba446a9158d5f6eebd9c442fbcf666b Mon Sep 17 00:00:00 2001 From: Yutong Zhu <y25...@uwaterloo.ca> Date: Sun, 2 Mar 2025 20:27:19 -0500 Subject: [PATCH 4/5] 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 9b7b1a1a33da2a6f022a7f7b31b2ee09e19c7135 Mon Sep 17 00:00:00 2001 From: Yutong Zhu <y25...@uwaterloo.ca> Date: Mon, 3 Mar 2025 23:08:39 -0500 Subject: [PATCH 5/5] 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits