diff mbox series

[C] Fix -Wtautological-compare (PR c/82437)

Message ID 20171005203426.GJ18588@tucnak
State New
Headers show
Series [C] Fix -Wtautological-compare (PR c/82437) | expand

Commit Message

Jakub Jelinek Oct. 5, 2017, 8:34 p.m. UTC 
Hi!

In warn_tautological_bitwise_comparison, there is even a comment
mentioning the fact that the types of the two constants might not be the
same (it is called with the original arguments before they are promoted
to common type for the comparison).

On the following testcase, one of the constants was unsigned (because
it has been converted to that when anded with unsigned variable), while
the other was signed and wi::to_widest (bitopcst) & wi::to_widest (cst)
wasn't sign-extended from 32-bit (because bitopcst was unsigned),
while wi::to_widest (cst) was (because cst was int), so we warned even
when we shouldn't.

The following patch instead uses the precision of the larger type and
zero extends from that (because we really don't need to care about bits
beyond that precision).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-10-05  Jakub Jelinek  <jakub@redhat.com>

	PR c/82437
	* c-warn.c (warn_tautological_bitwise_comparison): Instead of
	using to_widest use wide_int with the larger of the two precisions.

	* c-c++-common/Wtautological-compare-6.c: New test.


	Jakub

Comments

Marek Polacek Oct. 5, 2017, 9:05 p.m. UTC | #1 
On Thu, Oct 05, 2017 at 10:34:26PM +0200, Jakub Jelinek wrote:
> Hi!
> 
> In warn_tautological_bitwise_comparison, there is even a comment
> mentioning the fact that the types of the two constants might not be the
> same (it is called with the original arguments before they are promoted
> to common type for the comparison).
> 
> On the following testcase, one of the constants was unsigned (because
> it has been converted to that when anded with unsigned variable), while
> the other was signed and wi::to_widest (bitopcst) & wi::to_widest (cst)
> wasn't sign-extended from 32-bit (because bitopcst was unsigned),
> while wi::to_widest (cst) was (because cst was int), so we warned even
> when we shouldn't.
> 
> The following patch instead uses the precision of the larger type and
> zero extends from that (because we really don't need to care about bits
> beyond that precision).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Looks ok, thanks.

	Marek
diff mbox series

Patch

--- gcc/c-family/c-warn.c.jj	2017-10-02 13:46:57.000000000 +0200
+++ gcc/c-family/c-warn.c	2017-10-05 16:18:20.050207720 +0200
@@ -356,16 +356,24 @@  warn_tautological_bitwise_comparison (lo
     return;
 
   /* Note that the two operands are from before the usual integer
-     conversions, so their types might not be the same.  */
-  widest_int res;
+     conversions, so their types might not be the same.
+     Use the larger of the two precisions and ignore bits outside
+     of that.  */
+  int prec = MAX (TYPE_PRECISION (TREE_TYPE (cst)),
+		  TYPE_PRECISION (TREE_TYPE (bitopcst)));
+
+  wide_int bitopcstw = wide_int::from (bitopcst, prec, UNSIGNED);
+  wide_int cstw = wide_int::from (cst, prec, UNSIGNED);
+
+  wide_int res;
   if (TREE_CODE (bitop) == BIT_AND_EXPR)
-    res = wi::to_widest (bitopcst) & wi::to_widest (cst);
+    res = bitopcstw & cstw;
   else
-    res = wi::to_widest (bitopcst) | wi::to_widest (cst);
+    res = bitopcstw | cstw;
 
   /* For BIT_AND only warn if (CST2 & CST1) != CST1, and
      for BIT_OR only if (CST2 | CST1) != CST1.  */
-  if (res == wi::to_widest (cst))
+  if (res == cstw)
     return;
 
   if (code == EQ_EXPR)
--- gcc/testsuite/c-c++-common/Wtautological-compare-6.c.jj	2017-10-05 16:21:41.799800708 +0200
+++ gcc/testsuite/c-c++-common/Wtautological-compare-6.c	2017-10-05 16:23:36.727429542 +0200
@@ -0,0 +1,11 @@ 
+/* PR c/82437 */
+/* { dg-do compile { target int32plus } } */
+/* { dg-options "-Wtautological-compare" } */
+
+int
+foo (unsigned int x)
+{
+  if ((x & -1879048192) != -1879048192)	/* { dg-bogus "bitwise comparison always evaluates to" } */
+    return 0;
+  return 1;
+}