diff mbox series

c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR handling [PR107465]

Message ID Y3331bxiUwkukHjb@tucnak
State New
Headers show
Series c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR handling [PR107465] | expand

Commit Message

Jakub Jelinek Nov. 23, 2022, 10:37 a.m. UTC 
Hi!

There can be too many extensions and seems I didn't get everything right in
the previously posted patch.

The following incremental patch ought to fix that.
The code can deal with quite a few sign/zero extensions at various spots
and it is important to deal with all of them right.
On the argument that contains BIT_NOT_EXPR we have:
MSB bits#4 bits#3 BIT_NOT_EXPR bits#2 bits#1 LSB
where bits#1 is one or more bits (TYPE_PRECISION (TREE_TYPE (arg0))
at the end of the function) we don't know anything about, for the purposes
of this warning it is VARYING that is inverted with BIT_NOT_EXPR to some other
VARYING bits;
bits#2 is one or more bits (TYPE_PRECISION (TREE_TYPE (op0)) -
TYPE_PRECISION (TREE_TYPE (arg0)) at the end of the function)
which are known to be 0 before the BIT_NOT_EXPR and 1 after it.
bits#3 is zero or more bits from the TYPE_PRECISION (TREE_TYPE (op0))
at the end of function to the TYPE_PRECISION (TREE_TYPE (op0)) at the
end of the function to TYPE_PRECISION (TREE_TYPE (op0)) at the start
of the function, which are either zero extension or sign extension.
And bits#4 is zero or more bits from the TYPE_PRECISION (TREE_TYPE (op0))
at the start of the function to TYPE_PRECISION (result_type), which
again can be zero or sign extension.

Now, vanilla trunk as well as the previously posted patch mishandles the
case where bits#3 are sign extended (as bits#2 are known to be all set,
that means bits#3 are all set too) but bits#4 are zero extended and are
thus all 0.

The patch fixes it by tracking the lowest bit which is known to be clear
above the known to be set bits (if any, otherwise it is precision of
result_type).

Ok for trunk if it passes bootstrap/regtest?

2022-11-23  Jakub Jelinek  <jakub@redhat.com>

	PR c/107465
	* c-warn.cc (warn_for_sign_compare): Don't warn for unset bits
	above innermost zero extension of BIT_NOT_EXPR result.

	* c-c++-common/Wsign-compare-2.c (f18): New test.


	Jakub
diff mbox series

Patch

--- gcc/c-family/c-warn.cc.jj	2022-11-23 10:04:53.000000000 +0100
+++ gcc/c-family/c-warn.cc	2022-11-23 11:19:38.928113842 +0100
@@ -2344,13 +2344,33 @@  warn_for_sign_compare (location_t locati
      have all bits set that are set in the ~ operand when it is
      extended.  */
 
+  /* bits0 is the bit index of op0 extended to result_type, which will
+     be always 0 and so all bits above it.  If there is a BIT_NOT_EXPR
+     in that operand possibly sign or zero extended to op0 and then
+     possibly further sign or zero extended to result_type, bits0 will
+     be the precision of result type if all the extensions involved
+     if any are sign extensions, and will be the place of the innermost
+     zero extension otherwise.  We warn only if BIT_NOT_EXPR's operand is
+     zero extended from some even smaller precision, in that case after
+     BIT_NOT_EXPR some bits below bits0 will be guaranteed to be set.
+     Similarly for bits1.  */
+  int bits0 = TYPE_PRECISION (result_type);
+  if (TYPE_UNSIGNED (TREE_TYPE (op0)))
+    bits0 = TYPE_PRECISION (TREE_TYPE (op0));
   tree arg0 = c_common_get_narrower (op0, &unsignedp0);
   if (TYPE_PRECISION (TREE_TYPE (arg0)) == TYPE_PRECISION (TREE_TYPE (op0)))
     unsignedp0 = TYPE_UNSIGNED (TREE_TYPE (op0));
+  else if (unsignedp0)
+    bits0 = TYPE_PRECISION (TREE_TYPE (arg0));
   op0 = arg0;
+  int bits1 = TYPE_PRECISION (result_type);
+  if (TYPE_UNSIGNED (TREE_TYPE (op1)))
+    bits1 = TYPE_PRECISION (TREE_TYPE (op1));
   tree arg1 = c_common_get_narrower (op1, &unsignedp1);
   if (TYPE_PRECISION (TREE_TYPE (arg1)) == TYPE_PRECISION (TREE_TYPE (op1)))
     unsignedp1 = TYPE_UNSIGNED (TREE_TYPE (op1));
+  else if (unsignedp1)
+    bits1 = TYPE_PRECISION (TREE_TYPE (arg1));
   op1 = arg1;
 
   if ((TREE_CODE (op0) == BIT_NOT_EXPR)
@@ -2360,6 +2380,7 @@  warn_for_sign_compare (location_t locati
 	{
 	  std::swap (op0, op1);
 	  std::swap (unsignedp0, unsignedp1);
+	  std::swap (bits0, bits1);
 	}
 
       int unsignedp;
@@ -2378,16 +2399,8 @@  warn_for_sign_compare (location_t locati
 	      && bits < HOST_BITS_PER_WIDE_INT)
 	    {
 	      HOST_WIDE_INT mask = HOST_WIDE_INT_M1U << bits;
-	      if (unsignedp0)
-		{
-		  bits = TYPE_PRECISION (TREE_TYPE (op0));
-		  if (bits < TYPE_PRECISION (result_type)
-		      && bits < HOST_BITS_PER_WIDE_INT)
-		    mask &= ~(HOST_WIDE_INT_M1U << bits);
-		}
-	      bits = TYPE_PRECISION (result_type);
-	      if (bits < HOST_BITS_PER_WIDE_INT)
-		mask &= ~(HOST_WIDE_INT_M1U << bits);
+	      if (bits0 < HOST_BITS_PER_WIDE_INT)
+		mask &= ~(HOST_WIDE_INT_M1U << bits0);
 	      if ((mask & constant) != mask)
 		{
 		  if (constant == 0)
@@ -2405,24 +2418,7 @@  warn_for_sign_compare (location_t locati
 		< TYPE_PRECISION (TREE_TYPE (op0)))
 	       && unsignedp
 	       && unsignedp1
-	       /* If unsignedp0, the BIT_NOT_EXPR result is
-		  zero extended, so say if op0 is unsigned char
-		  variable, BIT_NOT_EXPR is unsigned short and
-		  result type int and op0 has value 0x55, the
-		  int value will be 0xffaa, or for op0 0xaa it
-		  will be 0xff55.  In these cases, warn if
-		  op1 is unsigned and narrower than unsigned short.
-		  While if unsignedp0 is false, the BIT_NOT_EXPR
-		  result is sign extended and because of the
-		  above TYPE_PRECISION comparison we know the
-		  MSB of BIT_NOT_EXPR is set (perhaps with some
-		  further bits below it).  The sign extension will
-		  then ensure all bits above BIT_NOT_EXPR up to
-		  result_type's precision are set.  */
-	       && (TYPE_PRECISION (TREE_TYPE (op1))
-		   < TYPE_PRECISION (unsignedp0
-				     ? TREE_TYPE (op0)
-				     : result_type)))
+	       && TYPE_PRECISION (TREE_TYPE (op1)) < bits0)
 	warning_at (location, OPT_Wsign_compare,
 		    "comparison of promoted bitwise complement "
 		    "of an unsigned value with unsigned");
--- gcc/testsuite/c-c++-common/Wsign-compare-2.c.jj	2022-11-23 10:04:53.000000000 +0100
+++ gcc/testsuite/c-c++-common/Wsign-compare-2.c	2022-11-23 11:05:10.497862219 +0100
@@ -104,3 +104,9 @@  f17 (unsigned char x, unsigned short y)
 {
   return (unsigned short) (~(unsigned short) x) == y;		/* { dg-bogus "comparison of promoted bitwise complement of an unsigned value with unsigned" } */
 }
+
+int
+f18 (unsigned char x)
+{
+  return (unsigned int) (short) (~(unsigned short) x) == 0xffffff05ULL;	/* { dg-bogus "comparison of promoted bitwise complement of an unsigned value with constant" } */
+}