[clang] fd874e5 - Missing tautological compare warnings due to unary operators
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 19 07:48:09 PDT 2022
Author: Muhammad Usman Shahid
Date: 2022-08-19T10:46:29-04:00
New Revision: fd874e5fb119e1d9f427a299ffa5bbabaeba9455
URL: https://github.com/llvm/llvm-project/commit/fd874e5fb119e1d9f427a299ffa5bbabaeba9455
DIFF: https://github.com/llvm/llvm-project/commit/fd874e5fb119e1d9f427a299ffa5bbabaeba9455.diff
LOG: Missing tautological compare warnings due to unary operators
The patch mainly focuses on the no warnings for -Wtautological-compare.
It work fine for the positive numbers but doesn't for the negative
numbers. This is because the warning explicitly checks for an
IntegerLiteral AST node, but -1 is represented by a UnaryOperator with
an IntegerLiteral sub-Expr.
Fixes #42918
Differential Revision: https://reviews.llvm.org/D130510
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-bitwise-compare.c
clang/test/SemaCXX/warn-bitwise-compare.cpp
clang/test/SemaCXX/warn-unreachable.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ccbe7a510ea80..e313f006b514e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -75,6 +75,9 @@ Bug Fixes
This fixes `Issue 56094 <https://github.com/llvm/llvm-project/issues/56094>`_.
- Fix `#57151 <https://github.com/llvm/llvm-project/issues/57151>`_.
``-Wcomma`` is emitted for void returning functions.
+- ``-Wtautological-compare`` missed warnings for tautological comparisons
+ involving a negative integer literal. This fixes
+ `Issue 42918 <https://github.com/llvm/llvm-project/issues/42918>`_.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 2b99b8e680805..31655e43e8993 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -72,6 +72,10 @@ static SourceLocation GetEndLoc(Decl *D) {
/// Returns true on constant values based around a single IntegerLiteral.
/// Allow for use of parentheses, integer casts, and negative signs.
+/// FIXME: it would be good to unify this function with
+/// getIntegerLiteralSubexpressionValue at some point given the similarity
+/// between the functions.
+
static bool IsIntegerLiteralConstantExpr(const Expr *E) {
// Allow parentheses
E = E->IgnoreParens();
@@ -964,15 +968,16 @@ class CFGBuilder {
const Expr *LHSExpr = B->getLHS()->IgnoreParens();
const Expr *RHSExpr = B->getRHS()->IgnoreParens();
- const IntegerLiteral *IntLiteral = dyn_cast<IntegerLiteral>(LHSExpr);
+ Optional<llvm::APInt> IntLiteral1 =
+ getIntegerLiteralSubexpressionValue(LHSExpr);
const Expr *BoolExpr = RHSExpr;
- if (!IntLiteral) {
- IntLiteral = dyn_cast<IntegerLiteral>(RHSExpr);
+ if (!IntLiteral1) {
+ IntLiteral1 = getIntegerLiteralSubexpressionValue(RHSExpr);
BoolExpr = LHSExpr;
}
- if (!IntLiteral)
+ if (!IntLiteral1)
return TryResult();
const BinaryOperator *BitOp = dyn_cast<BinaryOperator>(BoolExpr);
@@ -981,26 +986,26 @@ class CFGBuilder {
const Expr *LHSExpr2 = BitOp->getLHS()->IgnoreParens();
const Expr *RHSExpr2 = BitOp->getRHS()->IgnoreParens();
- const IntegerLiteral *IntLiteral2 = dyn_cast<IntegerLiteral>(LHSExpr2);
+ Optional<llvm::APInt> IntLiteral2 =
+ getIntegerLiteralSubexpressionValue(LHSExpr2);
if (!IntLiteral2)
- IntLiteral2 = dyn_cast<IntegerLiteral>(RHSExpr2);
+ IntLiteral2 = getIntegerLiteralSubexpressionValue(RHSExpr2);
if (!IntLiteral2)
return TryResult();
- llvm::APInt L1 = IntLiteral->getValue();
- llvm::APInt L2 = IntLiteral2->getValue();
- if ((BitOp->getOpcode() == BO_And && (L2 & L1) != L1) ||
- (BitOp->getOpcode() == BO_Or && (L2 | L1) != L1)) {
+ if ((BitOp->getOpcode() == BO_And &&
+ (*IntLiteral2 & *IntLiteral1) != *IntLiteral1) ||
+ (BitOp->getOpcode() == BO_Or &&
+ (*IntLiteral2 | *IntLiteral1) != *IntLiteral1)) {
if (BuildOpts.Observer)
BuildOpts.Observer->compareBitwiseEquality(B,
B->getOpcode() != BO_EQ);
TryResult(B->getOpcode() != BO_EQ);
}
} else if (BoolExpr->isKnownToHaveBooleanValue()) {
- llvm::APInt IntValue = IntLiteral->getValue();
- if ((IntValue == 1) || (IntValue == 0)) {
+ if ((*IntLiteral1 == 1) || (*IntLiteral1 == 0)) {
return TryResult();
}
return TryResult(B->getOpcode() != BO_EQ);
@@ -1009,6 +1014,46 @@ class CFGBuilder {
return TryResult();
}
+ // Helper function to get an APInt from an expression. Supports expressions
+ // which are an IntegerLiteral or a UnaryOperator and returns the value with
+ // all operations performed on it.
+ // FIXME: it would be good to unify this function with
+ // IsIntegerLiteralConstantExpr at some point given the similarity between the
+ // functions.
+ Optional<llvm::APInt> getIntegerLiteralSubexpressionValue(const Expr *E) {
+
+ // If unary.
+ if (const auto *UnOp = dyn_cast<UnaryOperator>(E->IgnoreParens())) {
+ // Get the sub expression of the unary expression and get the Integer
+ // Literal.
+ const Expr *SubExpr = UnOp->getSubExpr()->IgnoreParens();
+
+ if (const auto *IntLiteral = dyn_cast<IntegerLiteral>(SubExpr)) {
+
+ llvm::APInt Value = IntLiteral->getValue();
+
+ // Perform the operation manually.
+ switch (UnOp->getOpcode()) {
+ case UO_Plus:
+ return Value;
+ case UO_Minus:
+ return -Value;
+ case UO_Not:
+ return ~Value;
+ case UO_LNot:
+ return llvm::APInt(Value.getBitWidth(), !Value);
+ default:
+ assert(false && "Unexpected unary operator!");
+ return llvm::None;
+ }
+ }
+ } else if (const auto *IntLiteral =
+ dyn_cast<IntegerLiteral>(E->IgnoreParens()))
+ return IntLiteral->getValue();
+
+ return llvm::None;
+ }
+
TryResult analyzeLogicOperatorCondition(BinaryOperatorKind Relation,
const llvm::APSInt &Value1,
const llvm::APSInt &Value2) {
diff --git a/clang/test/Sema/warn-bitwise-compare.c b/clang/test/Sema/warn-bitwise-compare.c
index 08a8b084fe795..ccd83613b8065 100644
--- a/clang/test/Sema/warn-bitwise-compare.c
+++ b/clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
#define mydefine 2
+#define mydefine2 -2
enum {
ZERO,
@@ -11,29 +12,85 @@ enum {
void f(int x) {
if ((8 & x) == 3) {} // expected-warning {{bitwise comparison always evaluates to false}}
if ((x & 8) == 4) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((-8 & x) == 3) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((x & -8) == 4) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
if ((x & 8) != 4) {} // expected-warning {{bitwise comparison always evaluates to true}}
if ((2 & x) != 4) {} // expected-warning {{bitwise comparison always evaluates to true}}
+ if ((x & -8) != 4) {} // expected-warning {{bitwise comparison always evaluates to true}}
+ if ((-2 & x) != 3) {} // expected-warning {{bitwise comparison always evaluates to true}}
+
if ((x | 4) == 3) {} // expected-warning {{bitwise comparison always evaluates to false}}
if ((x | 3) != 4) {} // expected-warning {{bitwise comparison always evaluates to true}}
if ((5 | x) != 3) {} // expected-warning {{bitwise comparison always evaluates to true}}
+
+ if ((x | -4) == 3) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((x | -3) != 4) {} // expected-warning {{bitwise comparison always evaluates to true}}
+ if ((-5 | x) != 3) {} // expected-warning {{bitwise comparison always evaluates to true}}
+
if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
if (!!((8 & x) == 3)) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if (!!((-8 & x) == 3)) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
int y = ((8 & x) == 3) ? 1 : 2; // expected-warning {{bitwise comparison always evaluates to false}}
+ y = ((-8 & x) == 3) ? 1 : 2; // expected-warning {{bitwise comparison always evaluates to false}}
+ y = ((3 | x) != 5) ? 1 : 2; // expected-warning {{bitwise comparison always evaluates to true}}
+ y = ((-3 | x) != 5) ? 1 : 2; // expected-warning {{bitwise comparison always evaluates to true}}
+
+ if ((x & 0) != !0) {} // expected-warning {{bitwise comparison always evaluates to true}}
+ if ((x & 0) == !0) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((x & 2) == !0) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
+ if ((x | 4) == +3) {} // expected-warning {{bitwise comparison always evaluates to false}}
+ if ((x | 3) != +4) {} // expected-warning {{bitwise comparison always evaluates to true}}
+
+ if ((x & 8) != ~4) {} // expected-warning {{bitwise comparison always evaluates to true}}
+ if ((x & 0) == ~4) {} // expected-warning {{bitwise comparison always evaluates to false}}
if ((x & 8) == 8) {}
if ((x & 8) != 8) {}
if ((x | 4) == 4) {}
if ((x | 4) != 4) {}
+ if ((x | 4) == +4) {}
+ if ((x | 4) != +4) {}
+
+ if ((x & 1) == !12) {}
+ if ((x | 0) == !0) {}
+
+ if ((x | 1) == ~4) {}
+ if ((x | 1) != ~0) {}
+
+ if ((-2 & x) != 4) {}
+ if ((x & -8) == -8) {}
+ if ((x & -8) != -8) {}
+ if ((x | -4) == -4) {}
+ if ((x | -4) != -4) {}
+
if ((x & 9) == 8) {}
if ((x & 9) != 8) {}
if ((x | 4) == 5) {}
if ((x | 4) != 5) {}
+ if ((x & -9) == -10) {}
+ if ((x & -9) != -10) {}
+ if ((x | -4) == -3) {}
+ if ((x | -4) != -3) {}
+
+ if ((x^0) == 0) {}
+
if ((x & mydefine) == 8) {}
if ((x | mydefine) == 4) {}
+
+ if ((x & mydefine2) == 8) {}
+ if ((x | mydefine2) == 4) {}
+
+ if ((x & 1) == 1L) {}
}
void g(int x) {
diff --git a/clang/test/SemaCXX/warn-bitwise-compare.cpp b/clang/test/SemaCXX/warn-bitwise-compare.cpp
index 0fc3a6ac2d0c5..28b926ec1f0c6 100644
--- a/clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ b/clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -11,3 +11,14 @@ void test(int x) {
bool b4 = !!(x | 5);
// expected-warning at -1 {{bitwise or with non-zero value always evaluates to true}}
}
+
+template <int I, class T> // silence
+void foo(int x) {
+ bool b1 = (x & sizeof(T)) == 8;
+ bool b2 = (x & I) == 8;
+ bool b3 = (x & 4) == 8; // expected-warning {{bitwise comparison always evaluates to false}}
+}
+
+void run(int x) {
+ foo<4, int>(8); // expected-note {{in instantiation of function template specialization 'foo<4, int>' requested here}}
+}
diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp
index c664c1912899d..e6f5bc5ef8e12 100644
--- a/clang/test/SemaCXX/warn-unreachable.cpp
+++ b/clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,18 @@ void tautological_compare(bool x, int y) {
if (y == -1 && y != -1) // expected-note {{silence}}
calledFun(); // expected-warning {{will never be executed}}
- // TODO: Extend warning to the following code:
- if (x < -1)
- calledFun();
- if (x == -1)
- calledFun();
+ if (x == -1) // expected-note {{silence}}
+ calledFun(); // expected-warning {{will never be executed}}
- if (x != -1)
+ if (x != -1) // expected-note {{silence}}
calledFun();
else
+ calledFun(); // expected-warning {{will never be executed}}
+
+ // TODO: Extend warning to the following code:
+ if (x < -1)
calledFun();
+
if (-1 > x)
calledFun();
else
More information about the cfe-commits
mailing list