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
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. */
