diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -196,7 +196,11 @@ used in comparison operators. Fixes `Issue 56560 `_. - Fix that ``if consteval`` could evaluate to ``true`` at runtime because it was incorrectly constant folded. Fixes `Issue 55638 `_. +- ``-Wtautological-compare`` missed warnings for tautological comparisons + involving a negative integer literal. This fixes + `Issue 42918 `_. + Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - ``-Wliteral-range`` will warn on floating-point equality comparisons with 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 @@ -55,12 +55,14 @@ #include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include #include #include #include +using namespace std; using namespace clang; static SourceLocation GetEndLoc(Decl *D) { @@ -964,33 +966,39 @@ const Expr *LHSExpr = B->getLHS()->IgnoreParens(); const Expr *RHSExpr = B->getRHS()->IgnoreParens(); - const IntegerLiteral *IntLiteral = dyn_cast(LHSExpr); - const Expr *BoolExpr = RHSExpr; + const Expr *SubExpr = nullptr; + const Expr *BoolExpr = nullptr; - if (!IntLiteral) { - IntLiteral = dyn_cast(RHSExpr); + if (IsIntegerLiteralConstantExpr(LHSExpr)) { + SubExpr = LHSExpr; // LHS is a constant expression. + BoolExpr = RHSExpr; + } else if (IsIntegerLiteralConstantExpr(RHSExpr)) { + SubExpr = RHSExpr; // RHS is a constant expression. BoolExpr = LHSExpr; - } - - if (!IntLiteral) + } else return TryResult(); + // Get the Value of the constant expression. + llvm::APInt L1 = getIntegerLiteralSubexpressionValue(SubExpr); + const BinaryOperator *BitOp = dyn_cast(BoolExpr); if (BitOp && (BitOp->getOpcode() == BO_And || BitOp->getOpcode() == BO_Or)) { const Expr *LHSExpr2 = BitOp->getLHS()->IgnoreParens(); const Expr *RHSExpr2 = BitOp->getRHS()->IgnoreParens(); - const IntegerLiteral *IntLiteral2 = dyn_cast(LHSExpr2); + const Expr *SubExpr2 = nullptr; - if (!IntLiteral2) - IntLiteral2 = dyn_cast(RHSExpr2); + if (IsIntegerLiteralConstantExpr(LHSExpr2)) + SubExpr2 = LHSExpr2; // LHS is a constant expression. + else if (IsIntegerLiteralConstantExpr(RHSExpr2)) + SubExpr2 = RHSExpr2; // RHS is a constant expression. + else + return TryResult(); // Neither is a constant expression, bail out. - if (!IntLiteral2) - return TryResult(); + // Get the Value of the constant expression. + llvm::APInt L2 = getIntegerLiteralSubexpressionValue(SubExpr2); - 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 (BuildOpts.Observer) @@ -999,7 +1007,7 @@ TryResult(B->getOpcode() != BO_EQ); } } else if (BoolExpr->isKnownToHaveBooleanValue()) { - llvm::APInt IntValue = IntLiteral->getValue(); + llvm::APInt IntValue = L1; if ((IntValue == 1) || (IntValue == 0)) { return TryResult(); } @@ -1009,6 +1017,30 @@ return TryResult(); } + // The sub expression will be given and accordingly the Integer literal value + // will be returned. Will returned the Integer literal as 12 or even the unary + // as -12. + llvm::APInt getIntegerLiteralSubexpressionValue(const Expr *E) { + // UnaryOperator identification. + const UnaryOperator *UnOp = dyn_cast(E->IgnoreParens()); + + // If is unary operator then evaluating + if (UnOp) { + // To save the integer Literal. + Expr::EvalResult IntLiteralUnary; + E->EvaluateAsInt(IntLiteralUnary, *Context); + return IntLiteralUnary.Val.getInt(); + } + + // If not unary use the cast to find the value. + const IntegerLiteral *IntLiteral = + dyn_cast(E->IgnoreParens()); + if (IntLiteral) + return IntLiteral->getValue(); + + return llvm::APInt(); + } + 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,67 @@ 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 & 8) == 8) {} if ((x & 8) != 8) {} if ((x | 4) == 4) {} if ((x | 4) != 4) {} + 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) {} + } void g(int x) { 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,15 +396,16 @@ 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();