From b85e94189da016398fa784172171b9157183273c Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 20 May 2014 17:31:11 +0000 Subject: [PATCH] Add a check for tautological bitwise comparisons to -Wtautological-compare. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This catches issues like: if ((x & 8) == 4) { ... } if ((x | 4) != 3) { ... } Patch by Anders Rönnholm! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@209221 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Analysis/CFG.h | 2 + include/clang/Basic/DiagnosticSemaKinds.td | 3 ++ lib/Analysis/CFG.cpp | 43 ++++++++++++++++++---- lib/Sema/AnalysisBasedWarnings.cpp | 9 +++++ test/Sema/warn-bitwise-compare.c | 28 ++++++++++++++ 5 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 test/Sema/warn-bitwise-compare.c diff --git a/include/clang/Analysis/CFG.h b/include/clang/Analysis/CFG.h index 1230a9662c..7909fdc09c 100644 --- a/include/clang/Analysis/CFG.h +++ b/include/clang/Analysis/CFG.h @@ -706,6 +706,8 @@ class CFGCallback { public: CFGCallback() {} virtual void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {} + virtual void compareBitwiseEquality(const BinaryOperator *B, + bool isAlwaysTrue) {} virtual ~CFGCallback() {} }; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d6d0d0b3c8..00b3b34505 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6404,6 +6404,9 @@ def note_ref_subobject_of_member_declared_here : Note< def warn_comparison_always : Warning< "%select{self-|array }0comparison always evaluates to %select{false|true|a constant}1">, InGroup; +def warn_comparison_bitwise_always : Warning< + "bitwise comparison always evaluates to %select{false|true}0">, + InGroup; def warn_tautological_overlap_comparison : Warning< "overlapping comparisons always evaluate to %select{false|true}0">, InGroup, DefaultIgnore; diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp index 3139ae4c5d..5d2926c0f0 100644 --- a/lib/Analysis/CFG.cpp +++ b/lib/Analysis/CFG.cpp @@ -534,9 +534,10 @@ private: } } - /// Find a equality comparison with an expression evaluating to a boolean and - /// a constant other than 0 and 1. - /// e.g. if (!x == 10) + /// Find an incorrect equality comparison. Either with an expression + /// evaluating to a boolean and a constant other than 0 and 1. + /// e.g. if (!x == 10) or a bitwise and/or operation that always evaluates to + /// true/false e.q. (x & 8) == 4. TryResult checkIncorrectEqualityOperator(const BinaryOperator *B) { const Expr *LHSExpr = B->getLHS()->IgnoreParens(); const Expr *RHSExpr = B->getRHS()->IgnoreParens(); @@ -549,15 +550,41 @@ private: BoolExpr = LHSExpr; } - if (!IntLiteral || !BoolExpr->isKnownToHaveBooleanValue()) + if (!IntLiteral) return TryResult(); - llvm::APInt IntValue = IntLiteral->getValue(); - if ((IntValue == 1) || (IntValue == 0)) { - return TryResult(); + 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); + + if (!IntLiteral2) + IntLiteral2 = dyn_cast(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 (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)) { + return TryResult(); + } + return TryResult(B->getOpcode() != BO_EQ); } - return TryResult(B->getOpcode() != BO_EQ); + return TryResult(); } TryResult analyzeLogicOperatorCondition(BinaryOperatorKind Relation, diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index d10b5a8496..048019f994 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -146,6 +146,15 @@ public: S.Diag(B->getExprLoc(), diag::warn_tautological_overlap_comparison) << DiagRange << isAlwaysTrue; } + + void compareBitwiseEquality(const BinaryOperator *B, bool isAlwaysTrue) { + if (HasMacroID(B)) + return; + + SourceRange DiagRange = B->getSourceRange(); + S.Diag(B->getExprLoc(), diag::warn_comparison_bitwise_always) + << DiagRange << isAlwaysTrue; + } }; diff --git a/test/Sema/warn-bitwise-compare.c b/test/Sema/warn-bitwise-compare.c new file mode 100644 index 0000000000..175f8f5367 --- /dev/null +++ b/test/Sema/warn-bitwise-compare.c @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-compare %s + +#define mydefine 2 + +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 ((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 | 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 ((0x23 | x) == 0x155){} // 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 & 9) == 8) {} + if ((x & 9) != 8) {} + if ((x | 4) == 5) {} + if ((x | 4) != 5) {} + + if ((x & mydefine) == 8) {} + if ((x | mydefine) == 4) {} +} -- 2.40.0