c-family PATCH to improve -Wtautological-compare (PR c/81783)
Marek Polacek
polacek@redhat.com
Wed Aug 16 15:20:00 GMT 2017
This patch improves -Wtautological-compare so that it also detects
bitwise comparisons involving & and | that are always true or false, e.g.
if ((a & 16) == 10)
return 1;
can never be true. Note that e.g. "(a & 9) == 8" is *not* always false
or true.
I think it's pretty straightforward with one snag: we shouldn't warn if
the constant part of the bitwise operation comes from a macro, but currently
that's not possible, so I XFAILed this in the new test.
This has found one issue in the GCC codebase and it's a genuine bug:
<https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00757.html>.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-08-16 Marek Polacek <polacek@redhat.com>
PR c/81783
* c-warn.c (warn_tautological_bitwise_comparison): New function.
(warn_tautological_cmp): Call it.
* doc/invoke.texi: Update -Wtautological-compare documentation.
* c-c++-common/Wtautological-compare-5.c: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 9c3073444cf..0749d16a50f 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -321,6 +321,59 @@ find_array_ref_with_const_idx_r (tree *expr_p, int *, void *)
return NULL_TREE;
}
+/* Subroutine of warn_tautological_cmp. Warn about bitwise comparison
+ that always evaluate to true or false. LOC is the location of the
+ ==/!= comparison specified by CODE; LHS and RHS are the usual operands
+ of this comparison. */
+
+static void
+warn_tautological_bitwise_comparison (location_t loc, tree_code code,
+ tree lhs, tree rhs)
+{
+ if (code != EQ_EXPR && code != NE_EXPR)
+ return;
+
+ /* Extract the operands from e.g. (x & 8) == 4. */
+ tree bitop;
+ tree cst;
+ if ((TREE_CODE (lhs) == BIT_AND_EXPR
+ || TREE_CODE (lhs) == BIT_IOR_EXPR)
+ && TREE_CODE (rhs) == INTEGER_CST)
+ bitop = lhs, cst = rhs;
+ else if ((TREE_CODE (rhs) == BIT_AND_EXPR
+ || TREE_CODE (rhs) == BIT_IOR_EXPR)
+ && TREE_CODE (lhs) == INTEGER_CST)
+ bitop = rhs, cst = lhs;
+ else
+ return;
+
+ tree bitopcst;
+ if (TREE_CODE (TREE_OPERAND (bitop, 0)) == INTEGER_CST)
+ bitopcst = TREE_OPERAND (bitop, 0);
+ else if (TREE_CODE (TREE_OPERAND (bitop, 1)) == INTEGER_CST)
+ bitopcst = TREE_OPERAND (bitop, 1);
+ else
+ return;
+
+ wide_int res;
+ if (TREE_CODE (bitop) == BIT_AND_EXPR)
+ res = wi::bit_and (bitopcst, cst);
+ else
+ res = wi::bit_or (bitopcst, cst);
+
+ /* For BIT_AND only warn if (CST2 & CST1) != CST1, and
+ for BIT_OR only if (CST2 | CST1) != CST1. */
+ if (res == cst)
+ return;
+
+ if (code == EQ_EXPR)
+ warning_at (loc, OPT_Wtautological_compare,
+ "bitwise comparison always evaluates to false");
+ else
+ warning_at (loc, OPT_Wtautological_compare,
+ "bitwise comparison always evaluates to true");
+}
+
/* Warn if a self-comparison always evaluates to true or false. LOC
is the location of the comparison with code CODE, LHS and RHS are
operands of the comparison. */
@@ -337,6 +390,8 @@ warn_tautological_cmp (location_t loc, enum tree_code code, tree lhs, tree rhs)
|| from_macro_expansion_at (EXPR_LOCATION (rhs)))
return;
+ warn_tautological_bitwise_comparison (loc, code, lhs, rhs);
+
/* We do not warn for constants because they are typical of macro
expansions that test for features, sizeof, and similar. */
if (CONSTANT_CLASS_P (fold_for_warn (lhs))
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index ec29f1d629e..72a16a19711 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5484,6 +5484,14 @@ int i = 1;
@dots{}
if (i > i) @{ @dots{} @}
@end smallexample
+
+This warning also warns about bitwise comparisons that always evaluate
+to true or false, for instance:
+@smallexample
+if ((a & 16) == 10) @{ @dots{} @}
+@end smallexample
+will always be false.
+
This warning is enabled by @option{-Wall}.
@item -Wtrampolines
diff --git gcc/testsuite/c-c++-common/Wtautological-compare-5.c gcc/testsuite/c-c++-common/Wtautological-compare-5.c
index e69de29bb2d..4664bfdeae6 100644
--- gcc/testsuite/c-c++-common/Wtautological-compare-5.c
+++ gcc/testsuite/c-c++-common/Wtautological-compare-5.c
@@ -0,0 +1,106 @@
+/* PR c/81783 */
+/* { dg-do compile } */
+/* { dg-options "-Wtautological-compare" } */
+
+enum E { FOO = 128 };
+
+int
+f (int a)
+{
+ if ((a & 16) == 10) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if ((16 & a) == 10) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if (10 == (a & 16)) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if (10 == (16 & a)) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+
+ if ((a & 16) != 10) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((16 & a) != 10) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if (10 != (a & 16)) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if (10 != (16 & a)) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+
+ if ((a & 9) == 8)
+ return 1;
+ if ((9 & a) == 8)
+ return 1;
+ if (8 == (a & 9))
+ return 1;
+ if (8 == (9 & a))
+ return 1;
+
+ if ((a & 9) != 8)
+ return 1;
+ if ((9 & a) != 8)
+ return 1;
+ if (8 != (a & 9))
+ return 1;
+ if (8 != (9 & a))
+ return 1;
+
+ if ((a | 16) == 10) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if ((16 | a) == 10) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if (10 == (a | 16)) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if (10 == (16 | a)) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+
+ if ((a | 16) != 10) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((16 | a) != 10) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if (10 != (a | 16)) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if (10 != (16 | a)) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+
+ if ((a | 9) == 8) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if ((9 | a) == 8) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if (8 == (a | 9)) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if (8 == (9 | a)) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+
+ if ((a | 9) != 8) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((9 | a) != 8) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if (8 != (a | 9)) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if (8 != (9 | a)) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+
+ if ((a & 128) != 1) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((128 & a) != 1) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((a & FOO) != 1) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((FOO & a) != 1) /* { dg-warning "bitwise comparison always evaluates to true" } */
+ return 1;
+ if ((a & 128) == 1) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if ((128 & a) == 1) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if ((a & FOO) == 1) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+ if ((FOO & a) == 1) /* { dg-warning "bitwise comparison always evaluates to false" } */
+ return 1;
+
+#define N 0x10
+ if ((a & N) == 10) /* { dg-bogus "bitwise comparison always evaluates to false" "" { xfail *-*-* } } */
+ return 1;
+ if ((a | N) == 10) /* { dg-bogus "bitwise comparison always evaluates to false" "" { xfail *-*-* } } */
+ return 1;
+
+ return 0;
+}
Marek
More information about the Gcc-patches
mailing list