]> granicus.if.org Git - clang/commitdiff
Add a check for tautological bitwise comparisons to -Wtautological-compare.
authorJordan Rose <jordan_rose@apple.com>
Tue, 20 May 2014 17:31:11 +0000 (17:31 +0000)
committerJordan Rose <jordan_rose@apple.com>
Tue, 20 May 2014 17:31:11 +0000 (17:31 +0000)
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
include/clang/Basic/DiagnosticSemaKinds.td
lib/Analysis/CFG.cpp
lib/Sema/AnalysisBasedWarnings.cpp
test/Sema/warn-bitwise-compare.c [new file with mode: 0644]

index 1230a9662c8f284e107ef2b5e8d629c614b49603..7909fdc09cbbc6b0e0d4f7ef842f21a069815760 100644 (file)
@@ -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() {}
 };
 
index d6d0d0b3c850c7777fe361986153e720811f18e7..00b3b3450503758f03be76b93abeca48ec20c0ab 100644 (file)
@@ -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;
index 3139ae4c5d3b64ab14b87085f42d36c8915c8522..5d2926c0f0e591a2265998040ca7db568fb5a961 100644 (file)
@@ -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,
index d10b5a84960314482dbb8288075d361330b164a5..048019f994a61b17c2e3da0cdb6a4e96ab96d147 100644 (file)
@@ -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 (file)
index 0000000..175f8f5
--- /dev/null
@@ -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) {}
+}