diff mbox

-Wshift-overflow: Warn for shifting sign bit out of a negative number

Message ID 1448538778-38407-1-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Nov. 26, 2015, 11:52 a.m. UTC 
maybe_warn_shift_overflow is checking for patterns such as (1 << 31)
and not warning for them.

However, if the shifted value is negative, a shift by a non-zero
amount will always shift *out* of the sign bit rather than into it.
Thus it should be warned about, even if the value only requires
one bit more than the precision of the LHS.

Ok for trunk?

Paolo

gcc:
* c-family/c-common.c (maybe_warn_shift_overflow): Warn on all
overflows if shifting 1 out of the sign bit.

gcc/testsuite:
* c-c++-common/Wshift-overflow-1.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-2.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-3.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-4.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-6.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-7.c: Test shifting 1 out of the sign bit.

Comments

Jeff Law Dec. 2, 2015, 5:51 p.m. UTC | #1 
On 11/26/2015 04:52 AM, Paolo Bonzini wrote:
> maybe_warn_shift_overflow is checking for patterns such as (1 << 31)
> and not warning for them.
>
> However, if the shifted value is negative, a shift by a non-zero
> amount will always shift *out* of the sign bit rather than into it.
> Thus it should be warned about, even if the value only requires
> one bit more than the precision of the LHS.
>
> Ok for trunk?
>
> Paolo
>
> gcc:
> * c-family/c-common.c (maybe_warn_shift_overflow): Warn on all
> overflows if shifting 1 out of the sign bit.
>
> gcc/testsuite:
> * c-c++-common/Wshift-overflow-1.c: Test shifting 1 out of the sign bit.
> * c-c++-common/Wshift-overflow-2.c: Test shifting 1 out of the sign bit.
> * c-c++-common/Wshift-overflow-3.c: Test shifting 1 out of the sign bit.
> * c-c++-common/Wshift-overflow-4.c: Test shifting 1 out of the sign bit.
> * c-c++-common/Wshift-overflow-6.c: Test shifting 1 out of the sign bit.
> * c-c++-common/Wshift-overflow-7.c: Test shifting 1 out of the sign bit.
OK.
jeff
diff mbox

Patch

Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 230930)
+++ c-family/c-common.c	(working copy)
@@ -12631,8 +12631,11 @@  maybe_warn_shift_overflow (location_t loc, tree op
 
   unsigned int min_prec = (wi::min_precision (op0, SIGNED)
 			   + TREE_INT_CST_LOW (op1));
-  /* Handle the left-shifting 1 into the sign bit case.  */
-  if (min_prec == prec0 + 1)
+  /* Handle the case of left-shifting 1 into the sign bit.
+   * However, shifting 1 _out_ of the sign bit, as in
+   * INT_MIN << 1, is considered an overflow.
+   */
+  if (!tree_int_cst_sign_bit(op0) && min_prec == prec0 + 1)
     {
       /* Never warn for C++14 onwards.  */
       if (cxx_dialect >= cxx14)
Index: testsuite/c-c++-common/Wshift-overflow-1.c
===================================================================
--- testsuite/c-c++-common/Wshift-overflow-1.c	(revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-1.c	(working copy)
@@ -8,6 +8,9 @@ 
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1; /* { dg-warning "requires 36 bits to represent" } */
 int i3 = 10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
@@ -18,6 +21,7 @@ 
 int i8 = -10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
 int i9 = -__INT_MAX__ << 2; /* { dg-warning "requires 34 bits to represent" } */
 int i10 = -__INT_MAX__ << INTM1; /* { dg-warning "requires 63 bits to represent" } */
+int i11 = INT_MIN << 1; /* { dg-warning "requires 33 bits to represent" } */
 
 int r1 = 1 >> INTM1;
 int r2 = 9 >> INTM1;
@@ -46,6 +50,7 @@ 
 long long int l8 = -10LL << LLONGM2; /* { dg-warning "requires 67 bits to represent" } */
 long long int l9 = -__LONG_LONG_MAX__ << 2; /* { dg-warning "requires 66 bits to represent" } */
 long long int l10 = -__LONG_LONG_MAX__ << LLONGM1; /* { dg-warning "requires 127 bits to represent" } */
+long long int l11 = LONG_LONG_MIN << 1; /* { dg-warning "requires 65 bits to represent" } */
 
 void
 fn (void)
Index: testsuite/c-c++-common/Wshift-overflow-2.c
===================================================================
--- testsuite/c-c++-common/Wshift-overflow-2.c	(revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-2.c	(working copy)
@@ -8,6 +8,9 @@ 
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1;
 int i3 = 10 << INTM2;
@@ -18,6 +21,7 @@ 
 int i8 = -10 << INTM2;
 int i9 = -__INT_MAX__ << 2;
 int i10 = -__INT_MAX__ << INTM1;
+int i11 = INT_MIN << 1;
 
 int r1 = 1 >> INTM1;
 int r2 = 9 >> INTM1;
@@ -46,6 +50,7 @@ 
 long long int l8 = -10LL << LLONGM2;
 long long int l9 = -__LONG_LONG_MAX__ << 2;
 long long int l10 = -__LONG_LONG_MAX__ << LLONGM1;
+long long int l11 = LONG_LONG_MIN << 1;
 
 void
 fn (void)
Index: testsuite/c-c++-common/Wshift-overflow-3.c
===================================================================
--- testsuite/c-c++-common/Wshift-overflow-3.c	(revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-3.c	(working copy)
@@ -9,6 +9,9 @@ 
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1; /* { dg-warning "requires 36 bits to represent" } */
 int i3 = 10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
@@ -19,6 +22,7 @@ 
 int i8 = -10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
 int i9 = -__INT_MAX__ << 2; /* { dg-warning "requires 34 bits to represent" } */
 int i10 = -__INT_MAX__ << INTM1; /* { dg-warning "requires 63 bits to represent" } */
+int i11 = INT_MIN << 1; /* { dg-warning "requires 33 bits to represent" } */
 
 int r1 = 1 >> INTM1;
 int r2 = 9 >> INTM1;
@@ -47,6 +51,7 @@ 
 long long int l8 = -10LL << LLONGM2; /* { dg-warning "requires 67 bits to represent" } */
 long long int l9 = -__LONG_LONG_MAX__ << 2; /* { dg-warning "requires 66 bits to represent" } */
 long long int l10 = -__LONG_LONG_MAX__ << LLONGM1; /* { dg-warning "requires 127 bits to represent" } */
+long long int l11 = LONG_LONG_MIN << 1; /* { dg-warning "requires 65 bits to represent" } */
 
 void
 fn (void)
Index: testsuite/c-c++-common/Wshift-overflow-4.c
===================================================================
--- testsuite/c-c++-common/Wshift-overflow-4.c	(revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-4.c	(working copy)
@@ -9,6 +9,9 @@ 
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1;
 int i3 = 10 << INTM2;
@@ -19,6 +22,7 @@ 
 int i8 = -10 << INTM2;
 int i9 = -__INT_MAX__ << 2;
 int i10 = -__INT_MAX__ << INTM1;
+int i11 = INT_MIN << 1;
 
 int r1 = 1 >> INTM1;
 int r2 = 9 >> INTM1;
@@ -47,6 +51,7 @@ 
 long long int l8 = -10LL << LLONGM2;
 long long int l9 = -__LONG_LONG_MAX__ << 2;
 long long int l10 = -__LONG_LONG_MAX__ << LLONGM1;
+long long int l11 = LONG_LONG_MIN << 1;
 
 void
 fn (void)
Index: testsuite/c-c++-common/Wshift-overflow-6.c
===================================================================
--- testsuite/c-c++-common/Wshift-overflow-6.c	(revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-6.c	(working copy)
@@ -34,3 +34,4 @@ 
 int i28 = 0b10000000000000000000000000000 << 3;
 int i29 = 0b100000000000000000000000000000 << 2;
 int i30 = 0b1000000000000000000000000000000 << 1;
+int i31 = (int) 0b10000000000000000000000000000000u << 1; /* { dg-warning "requires 33 bits to represent" } */
Index: testsuite/c-c++-common/Wshift-overflow-7.c
===================================================================
--- testsuite/c-c++-common/Wshift-overflow-7.c	(revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-7.c	(working copy)
@@ -34,3 +34,4 @@ 
 int i28 = 0b10000000000000000000000000000 << 3; /* { dg-warning "requires 33 bits to represent" } */
 int i29 = 0b100000000000000000000000000000 << 2; /* { dg-warning "requires 33 bits to represent" } */
 int i30 = 0b1000000000000000000000000000000 << 1; /* { dg-warning "requires 33 bits to represent" } */
+int i31 = (int) 0b10000000000000000000000000000000u << 1; /* { dg-warning "requires 33 bits to represent" } */