Add a check for tautological bitwise comparisons to -Wtautological-compare.

This catches issues like:

if ((x & 8) == 4) { ... }
if ((x | 4) != 3) { ... }

Patch by Anders Rönnholm!

llvm-svn: 209221

5 files changed, 77 insertions, 8 deletions

diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h
index 1230a9662c8..7909fdc09cb 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/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/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d6d0d0b3c85..00b3b345050 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/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<TautologicalCompare>;
+def warn_comparison_bitwise_always : Warning<
+  "bitwise comparison always evaluates to %select{false|true}0">,
+  InGroup<TautologicalCompare>;
 def warn_tautological_overlap_comparison : Warning<
   "overlapping comparisons always evaluate to %select{false|true}0">,
   InGroup<TautologicalOverlapCompare>, DefaultIgnore;
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 3139ae4c5d3..5d2926c0f0e 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/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<BinaryOperator>(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<IntegerLiteral>(LHSExpr2);
+
+      if (!IntLiteral2)
+        IntLiteral2 = dyn_cast<IntegerLiteral>(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/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index d10b5a84960..048019f994a 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/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/clang/test/Sema/warn-bitwise-compare.c b/clang/test/Sema/warn-bitwise-compare.c
new file mode 100644
index 00000000000..175f8f5367f
--- /dev/null
+++ b/clang/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) {}
+}