[RFC] proof-of-concept: warning for a bit comparison that is always true/false

Daniel Marjamäki daniel.marjamaki@gmail.com
Wed Apr 16 15:29:00 GMT 2014


Hello!

I am new to GCC.

I want to add a warning to GCC when bit comparison is always true/false.

Example:

    if ((x&4)==0) {}  // <- no warning

    if ((x&4)==4) {}  // <- no warning

    if ((x&4)==5) {}  // <- warn!

When this warning is triggered, the most common cause is that somebody
made a mistake when using bitmasks.

I attach a proof-of-concept patch. I would like comments.

The patch needs some cleanup before it's applied.. I would like it to
handle at least != also and not just ==. And I would like it to be
less strict about where integer constants are located.

I wonder where I should put this code. Is gcc/c/c-typeck.c a good file
to put this in? Should I put it in somewhere else?

What warning flags should be used to enable this? Is some
-Wcondition-bitop a good idea? Can this be added by -Wall?

I wrote this check for Cppcheck years ago. In my experience this
warning has a good signal/noise ratio.

Best regards,
Daniel Marjamäki
-------------- next part --------------
Index: gcc/testsuite/c-c++-common/Wcondition-bitop.c
===================================================================
--- gcc/testsuite/c-c++-common/Wcondition-bitop.c	(revision 0)
+++ gcc/testsuite/c-c++-common/Wcondition-bitop.c	(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+void f(int x) {
+  if ((x&4)==0){}
+  if ((x&4)==4){}
+  if ((x&4)==2){}  /* { dg-warning "Comparison is always false" } */
+}
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 209347)
+++ gcc/c/c-typeck.c	(arbetskopia)
@@ -9980,6 +9980,15 @@
   bool op0_int_operands, op1_int_operands;
   bool int_const, int_const_or_overflow, int_operands;
 
+  if ((code                                      == EQ_EXPR     ) &&
+      (TREE_CODE(orig_op0)                       == BIT_AND_EXPR) &&
+      (TREE_CODE(orig_op1)                       == INTEGER_CST ) &&
+      (TREE_INT_CST_LOW(orig_op1)                != 0           ) &&
+      (TREE_CODE(TREE_OPERAND_CHECK(orig_op0,1)) == INTEGER_CST ) &&
+      (((TREE_INT_CST_LOW(TREE_OPERAND_CHECK(orig_op0,1))  & TREE_INT_CST_LOW(orig_op1))  != TREE_INT_CST_LOW(orig_op1)) ||
+       ((TREE_INT_CST_HIGH(TREE_OPERAND_CHECK(orig_op0,1)) & TREE_INT_CST_HIGH(orig_op1)) != TREE_INT_CST_HIGH(orig_op1))))
+    warning_at(location, 0, "Comparison is always false");
+
   /* Expression code to give to the expression when it is built.
      Normally this is CODE, which is what the caller asked for,
      but in some special cases we change it.  */


More information about the Gcc-patches mailing list