diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -75,6 +75,9 @@ This fixes `Issue 56094 `_. - Fix `#57151 `_. ``-Wcomma`` is emitted for void returning functions. +- ``-Wtautological-compare`` missed warnings for tautological comparisons + involving a negative integer literal. This fixes + `Issue 42918 `_. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -72,6 +72,10 @@ /// 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 @@ const Expr *LHSExpr = B->getLHS()->IgnoreParens(); const Expr *RHSExpr = B->getRHS()->IgnoreParens(); - const IntegerLiteral *IntLiteral = dyn_cast(LHSExpr); + Optional IntLiteral1 = + getIntegerLiteralSubexpressionValue(LHSExpr); const Expr *BoolExpr = RHSExpr; - if (!IntLiteral) { - IntLiteral = dyn_cast(RHSExpr); + if (!IntLiteral1) { + IntLiteral1 = getIntegerLiteralSubexpressionValue(RHSExpr); BoolExpr = LHSExpr; } - if (!IntLiteral) + if (!IntLiteral1) return TryResult(); const BinaryOperator *BitOp = dyn_cast(BoolExpr); @@ -981,26 +986,26 @@ const Expr *LHSExpr2 = BitOp->getLHS()->IgnoreParens(); const Expr *RHSExpr2 = BitOp->getRHS()->IgnoreParens(); - const IntegerLiteral *IntLiteral2 = dyn_cast(LHSExpr2); + Optional IntLiteral2 = + getIntegerLiteralSubexpressionValue(LHSExpr2); if (!IntLiteral2) - IntLiteral2 = dyn_cast(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 @@ 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 getIntegerLiteralSubexpressionValue(const Expr *E) { + + // If unary. + if (const auto *UnOp = dyn_cast(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(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(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 --- 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 @@ 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 --- a/clang/test/SemaCXX/warn-bitwise-compare.cpp +++ b/clang/test/SemaCXX/warn-bitwise-compare.cpp @@ -11,3 +11,14 @@ bool b4 = !!(x | 5); // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}} } + +template // 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 --- a/clang/test/SemaCXX/warn-unreachable.cpp +++ b/clang/test/SemaCXX/warn-unreachable.cpp @@ -396,16 +396,18 @@ 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