public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-6476] c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR handling [PR107465]
@ 2023-03-04  9:22 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2023-03-04  9:22 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:3ec9a8728086ad86a2d421e067329f305f40e005

commit r13-6476-g3ec9a8728086ad86a2d421e067329f305f40e005
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Mar 4 10:21:45 2023 +0100

    c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR handling [PR107465]
    
    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).
    
    2023-03-04  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.

Diff:
---
 gcc/c-family/c-warn.cc                       | 52 +++++++++++++---------------
 gcc/testsuite/c-c++-common/Wsign-compare-2.c |  6 ++++
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index 70d9bb20097..ccbede95ea4 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -2345,13 +2345,33 @@ warn_for_sign_compare (location_t location,
      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)
@@ -2361,6 +2381,7 @@ warn_for_sign_compare (location_t location,
 	{
 	  std::swap (op0, op1);
 	  std::swap (unsignedp0, unsignedp1);
+	  std::swap (bits0, bits1);
 	}
 
       int unsignedp;
@@ -2379,16 +2400,8 @@ warn_for_sign_compare (location_t location,
 	      && 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)
@@ -2406,24 +2419,7 @@ warn_for_sign_compare (location_t location,
 		< 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");
diff --git a/gcc/testsuite/c-c++-common/Wsign-compare-2.c b/gcc/testsuite/c-c++-common/Wsign-compare-2.c
index 23933d09cdc..71a7c9899fb 100644
--- a/gcc/testsuite/c-c++-common/Wsign-compare-2.c
+++ b/gcc/testsuite/c-c++-common/Wsign-compare-2.c
@@ -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" } */
+}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-03-04  9:22 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-04  9:22 [gcc r13-6476] c-family: Incremental fix for -Wsign-compare BIT_NOT_EXPR handling [PR107465] Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).