Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

Ping^4.

On Fri, Jun 26, 2015 at 10:08:51AM +0200, Marek Polacek wrote:
> I'm pinging the C++ parts.
> 
> On Fri, Jun 19, 2015 at 12:44:36PM +0200, Marek Polacek wrote:
> > Ping.
> > 
> > On Fri, Jun 12, 2015 at 11:07:29AM +0200, Marek Polacek wrote:
> > > Ping.
> > > 
> > > On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:
> > > > On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> > > > > The C changes are OK.
> > > > 
> > > > Jason, do you want to approve the C++ parts?

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

Ping^5.

On Fri, Jul 03, 2015 at 09:42:39AM +0200, Marek Polacek wrote:
> Ping^4.
> 
> On Fri, Jun 26, 2015 at 10:08:51AM +0200, Marek Polacek wrote:
> > I'm pinging the C++ parts.
> > 
> > On Fri, Jun 19, 2015 at 12:44:36PM +0200, Marek Polacek wrote:
> > > Ping.
> > > 
> > > On Fri, Jun 12, 2015 at 11:07:29AM +0200, Marek Polacek wrote:
> > > > Ping.
> > > > 
> > > > On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:
> > > > > On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> > > > > > The C changes are OK.
> > > > > 
> > > > > Jason, do you want to approve the C++ parts?

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

Ping^6.

On Fri, Jul 10, 2015 at 03:23:43PM +0200, Marek Polacek wrote:
> Ping^5.
> 
> On Fri, Jul 03, 2015 at 09:42:39AM +0200, Marek Polacek wrote:
> > Ping^4.
> > 
> > On Fri, Jun 26, 2015 at 10:08:51AM +0200, Marek Polacek wrote:
> > > I'm pinging the C++ parts.
> > > 
> > > On Fri, Jun 19, 2015 at 12:44:36PM +0200, Marek Polacek wrote:
> > > > Ping.
> > > > 
> > > > On Fri, Jun 12, 2015 at 11:07:29AM +0200, Marek Polacek wrote:
> > > > > Ping.
> > > > > 
> > > > > On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:
> > > > > > On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> > > > > > > The C changes are OK.
> > > > > > 
> > > > > > Jason, do you want to approve the C++ parts?

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Jeff Law]

On 07/17/2015 02:09 AM, Marek Polacek wrote:

Ping^6.

On Fri, Jul 10, 2015 at 03:23:43PM +0200, Marek Polacek wrote:

Ping^5.

On Fri, Jul 03, 2015 at 09:42:39AM +0200, Marek Polacek wrote:

Ping^4.

On Fri, Jun 26, 2015 at 10:08:51AM +0200, Marek Polacek wrote:

I'm pinging the C++ parts.

On Fri, Jun 19, 2015 at 12:44:36PM +0200, Marek Polacek wrote:

Ping.

On Fri, Jun 12, 2015 at 11:07:29AM +0200, Marek Polacek wrote:

Ping.

On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:

On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:

The C changes are OK.


Jason, do you want to approve the C++ parts?

I'll approve the C++ parts given how simple they are :-)

jeff

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

On Fri, Jul 17, 2015 at 03:51:33PM -0600, Jeff Law wrote:
> I'll approve the C++ parts given how simple they are :-)

Thanks, I've committed the patch now after another regtest/bootstrap.

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[David Edelsohn]

This seems to have caused a number of new failures in the PPC
testsuite for vmx/unpack.

- David

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

On Mon, Jul 20, 2015 at 04:23:08PM -0400, David Edelsohn wrote:
> This seems to have caused a number of new failures in the PPC
> testsuite for vmx/unpack.

Sorry about that.  Should be fixed with this patch I'm about to commit.

2015-07-21  Marek Polacek  

* gcc.dg/vmx/unpack-be-order.c: Use -Wno-shift-overflow.
* gcc.dg/vmx/unpack.c: Likewise.
* gcc.target/powerpc/quad-atomic.c: Likewise.

diff --git gcc/testsuite/gcc.dg/vmx/unpack-be-order.c 
gcc/testsuite/gcc.dg/vmx/unpack-be-order.c
index e174433..6eb98f4 100644
--- gcc/testsuite/gcc.dg/vmx/unpack-be-order.c
+++ gcc/testsuite/gcc.dg/vmx/unpack-be-order.c
@@ -1,4 +1,4 @@
-/* { dg-options "-maltivec=be -mabi=altivec -std=gnu99 -mno-vsx" } */
+/* { dg-options "-maltivec=be -mabi=altivec -std=gnu99 -mno-vsx 
-Wno-shift-overflow" } */
 
 #include "harness.h"
 
diff --git gcc/testsuite/gcc.dg/vmx/unpack.c gcc/testsuite/gcc.dg/vmx/unpack.c
index 3c13163..e71a5a6 100644
--- gcc/testsuite/gcc.dg/vmx/unpack.c
+++ gcc/testsuite/gcc.dg/vmx/unpack.c
@@ -1,3 +1,5 @@
+/* { dg-options "-Wno-shift-overflow" } */
+
 #include "harness.h"
 
 #define BIG 4294967295
diff --git gcc/testsuite/gcc.target/powerpc/quad-atomic.c 
gcc/testsuite/gcc.target/powerpc/quad-atomic.c
index 0d7089b..dc0e3a8 100644
--- gcc/testsuite/gcc.target/powerpc/quad-atomic.c
+++ gcc/testsuite/gcc.target/powerpc/quad-atomic.c
@@ -3,7 +3,7 @@
 /* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */
 /* { dg-require-effective-target p8vector_hw } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { 
"-mcpu=power8" } } */
-/* { dg-options "-mcpu=power8 -O2" } */
+/* { dg-options "-mcpu=power8 -O2 -Wno-shift-overflow" } */
 
 /* Test whether we get the right bits for quad word atomic instructions.  */
 #include 

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[David Edelsohn]

On Tue, Jul 21, 2015 at 5:59 AM, Marek Polacek  wrote:
> On Mon, Jul 20, 2015 at 04:23:08PM -0400, David Edelsohn wrote:
>> This seems to have caused a number of new failures in the PPC
>> testsuite for vmx/unpack.
>
> Sorry about that.  Should be fixed with this patch I'm about to commit.
>
> 2015-07-21  Marek Polacek  
>
> * gcc.dg/vmx/unpack-be-order.c: Use -Wno-shift-overflow.
> * gcc.dg/vmx/unpack.c: Likewise.

This doesn't fully fix the failures.

> * gcc.target/powerpc/quad-atomic.c: Likewise.

> diff --git gcc/testsuite/gcc.dg/vmx/unpack.c gcc/testsuite/gcc.dg/vmx/unpack.c
> index 3c13163..e71a5a6 100644
> --- gcc/testsuite/gcc.dg/vmx/unpack.c
> +++ gcc/testsuite/gcc.dg/vmx/unpack.c
> @@ -1,3 +1,5 @@
> +/* { dg-options "-Wno-shift-overflow" } */
> +

Should this be dg-additional-options ?

>  #include "harness.h"
>
>  #define BIG 4294967295

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

On Wed, Jul 22, 2015 at 08:35:12AM -0400, David Edelsohn wrote:
> On Tue, Jul 21, 2015 at 5:59 AM, Marek Polacek  wrote:
> > On Mon, Jul 20, 2015 at 04:23:08PM -0400, David Edelsohn wrote:
> >> This seems to have caused a number of new failures in the PPC
> >> testsuite for vmx/unpack.
> >
> > Sorry about that.  Should be fixed with this patch I'm about to commit.
> >
> > 2015-07-21  Marek Polacek  
> >
> > * gcc.dg/vmx/unpack-be-order.c: Use -Wno-shift-overflow.
> > * gcc.dg/vmx/unpack.c: Likewise.
> 
> This doesn't fully fix the failures.
 
Ouch.  What other failures do you see?  I've tried the patch on ppc64-linux
and didn't see any others.

It'd be very weird to see -Wshift-overflow warnings when -Wno-shift-overflow
is in effect.

> > --- gcc/testsuite/gcc.dg/vmx/unpack.c
> > +++ gcc/testsuite/gcc.dg/vmx/unpack.c
> > @@ -1,3 +1,5 @@
> > +/* { dg-options "-Wno-shift-overflow" } */
> > +
> 
> Should this be dg-additional-options ?

I did what was in gcc.dg/vmx/unpack-be-order.c, i.e. dg-options.  Or does
using dg-additional-options help?

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Segher Boessenkool]

On Wed, Jul 22, 2015 at 02:39:14PM +0200, Marek Polacek wrote:
> On Wed, Jul 22, 2015 at 08:35:12AM -0400, David Edelsohn wrote:
> > On Tue, Jul 21, 2015 at 5:59 AM, Marek Polacek  wrote:
> > > On Mon, Jul 20, 2015 at 04:23:08PM -0400, David Edelsohn wrote:
> > >> This seems to have caused a number of new failures in the PPC
> > >> testsuite for vmx/unpack.
> > >
> > > Sorry about that.  Should be fixed with this patch I'm about to commit.
> > >
> > > 2015-07-21  Marek Polacek  
> > >
> > > * gcc.dg/vmx/unpack-be-order.c: Use -Wno-shift-overflow.
> > > * gcc.dg/vmx/unpack.c: Likewise.
> > 
> > This doesn't fully fix the failures.
>  
> Ouch.  What other failures do you see?  I've tried the patch on ppc64-linux
> and didn't see any others.

vmx.exp sets a bunch of options and the test overrides that now.  Options
like -maltivec are pretty important for this test to work -- it #includes
altivec.h, which does #error unless -maltivec is set, and things go downhill
from that.  unpack-be-order.c works, unpack.c blows up.

Does your compiler maybe default to -maltivec?


Segher

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

On Wed, Jul 22, 2015 at 07:48:47AM -0500, Segher Boessenkool wrote:
> vmx.exp sets a bunch of options and the test overrides that now.  Options
> like -maltivec are pretty important for this test to work -- it #includes
> altivec.h, which does #error unless -maltivec is set, and things go downhill
> from that.  unpack-be-order.c works, unpack.c blows up.

Ah, right.
 
> Does your compiler maybe default to -maltivec?

I suppose -- I didn't see any failures on cfarm 112, but I was able to
reproduce the unpack.c fail on cfarm 110.  Turned out I should've used
dg-additional-options.  Thanks.

Tested vmx.exp on powerpc64-unknown-linux-gnu, applying to trunk.

2015-07-22  Marek Polacek  

* gcc.dg/vmx/unpack.c: Use dg-additional-options rather than
dg-options.

diff --git gcc/testsuite/gcc.dg/vmx/unpack.c gcc/testsuite/gcc.dg/vmx/unpack.c
index e71a5a6..b3ec93a 100644
--- gcc/testsuite/gcc.dg/vmx/unpack.c
+++ gcc/testsuite/gcc.dg/vmx/unpack.c
@@ -1,4 +1,4 @@
-/* { dg-options "-Wno-shift-overflow" } */
+/* { dg-additional-options "-Wno-shift-overflow" } */
 
 #include "harness.h"
 

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

On Tue, Jun 02, 2015 at 09:25:32PM +0200, Marek Polacek wrote:
> On Fri, May 29, 2015 at 08:49:58PM +, Joseph Myers wrote:
> > On Mon, 25 May 2015, Marek Polacek wrote:
> > 
> > > +/* Warn if signed left shift overflows.  Note that we don't warn
> > > +   about left-shifting 1 into the sign bit; cf.
> > > +   
> > > 
> > > +   for C++ and 
> > > 
> > > +   for C.  LOC is a location of the shift; OP0 and OP1 are the operands.
> > > +   Return true if an overflow is detected, false otherwise.  */
> > 
> > But for C that was declared not a defect.  See 
> > .  So 
> > for C99 and later we *should* consider this an overflow (for the purposes 
> > of pedwarns-if-pedantic in contexts where an integer constant expression 
> > is required; maybe -Wshift-overflow=2 for other warnings?).  If then a 
> > future C standard changes things here (in the list of issues to be 
> > considered for a future C standard in Standing Document 3, 
> > ), then, as a 
> > non-defect change, it should be considered a non-overflow in GCC only for 
> > future C standard versions as well as C90.
> > 
> > (Although treating something as not an integer constant expression does 
> > have consequences beyond pedwarns-if-pedantic - a zero derived from that 
> > expression is not a null pointer constant and that can affect the types of 
> > conditional expressions - I don't expect any significant breakage of real 
> > code from that.)
> 
> Well, ok then.  This new version incorporates Richard S.'s suggestion,
> and warns even for 1 << 31 in C99/C11 (also in C90 when -Wshift-overflow
> is explicitely specified).  For C++, it warns about 1 << 31 by default
> only in C++11 mode, in C++14 never, otherwise only if -Wshift-overflow
> is specified.
> 
> But the fallout seems to be nonnegligible.  So I think the default should
> be -Wshift-overflow=1 that doesn't warn about 1 << 31, but still rejects
> e.g. enum { A = 1 << 31 };.  And -Wshift-overflow=2 would warn even about
> 1 << 31.  (Perhaps this is exactly what you had in mind, but I'm not sure.)

Done in the following.  I think this is the best approach.  The documentation
hopefully makes it clear what's the intended behavior.

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

2015-06-03  Marek Polacek  
Richard Sandiford  

PR c++/55095
* c-common.c (c_fully_fold_internal): Warn about left shift overflows.
Use EXPR_LOC_OR_LOC.
(maybe_warn_shift_overflow): New function.
* c-common.h (maybe_warn_shift_overflow): Declare.
* c-opts.c (c_common_post_options): Set warn_shift_overflow.
* c.opt (Wshift-overflow): New option.

* c-typeck.c (digest_init): Pass OPT_Wpedantic to pedwarn_init.
(build_binary_op): Warn about left shift overflows.

* typeck.c (cp_build_binary_op): Warn about left shift overflows.

* doc/invoke.texi: Document -Wshift-overflow and -Wshift-overflow=.

* c-c++-common/Wshift-overflow-1.c: New test.
* c-c++-common/Wshift-overflow-2.c: New test.
* c-c++-common/Wshift-overflow-3.c: New test.
* c-c++-common/Wshift-overflow-4.c: New test.
* c-c++-common/Wshift-overflow-5.c: New test.
* g++.dg/cpp1y/left-shift-1.C: New test.
* gcc.dg/c90-left-shift-2.c: New test.
* gcc.dg/c90-left-shift-3.c: New test.
* gcc.dg/c99-left-shift-2.c: New test.
* gcc.dg/c99-left-shift-3.c: New test.
* gcc.dg/pr40501.c: Use -Wno-shift-overflow.
* gcc.c-torture/execute/pr40386.c: Likewise.
* gcc.dg/vect/pr33373.c: Likewise.
* gcc.dg/vect/vect-shift-2-big-array.c: Likewise.
* gcc.dg/vect/vect-shift-2.c: Likewise.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 36c984c..dc31fad 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1371,7 +1371,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
   if (TREE_OVERFLOW_P (ret)
  && !TREE_OVERFLOW_P (op0)
  && !TREE_OVERFLOW_P (op1))
-   overflow_warning (EXPR_LOCATION (expr), ret);
+   overflow_warning (EXPR_LOC_OR_LOC (expr, input_location), ret);
   if (code == LSHIFT_EXPR
  && TREE_CODE (orig_op0) != INTEGER_CST
  && TREE_CODE (TREE_TYPE (orig_op0)) == INTEGER_TYPE
@@ -1401,6 +1401,18 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
 ? G_("left shift count >= width of type")
 : G_("right shift count >= width of type")));
}
+  if (code == LSHIFT_EXPR
+ /* If either OP0 has been folded to INTEGER_CST...  */
+ && ((TREE_CODE (orig_op0) != INTEGER_CST
+  && TREE_CODE

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Joseph Myers]

On Wed, 3 Jun 2015, Marek Polacek wrote:

> > Well, ok then.  This new version incorporates Richard S.'s suggestion,
> > and warns even for 1 << 31 in C99/C11 (also in C90 when -Wshift-overflow
> > is explicitely specified).  For C++, it warns about 1 << 31 by default
> > only in C++11 mode, in C++14 never, otherwise only if -Wshift-overflow
> > is specified.
> > 
> > But the fallout seems to be nonnegligible.  So I think the default should
> > be -Wshift-overflow=1 that doesn't warn about 1 << 31, but still rejects
> > e.g. enum { A = 1 << 31 };.  And -Wshift-overflow=2 would warn even about
> > 1 << 31.  (Perhaps this is exactly what you had in mind, but I'm not sure.)
> 
> Done in the following.  I think this is the best approach.  The documentation
> hopefully makes it clear what's the intended behavior.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

The C changes are OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> The C changes are OK.

Jason, do you want to approve the C++ parts?

Thanks,

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

Ping.

On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:
> On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> > The C changes are OK.
> 
> Jason, do you want to approve the C++ parts?
> 
> Thanks,

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

Ping.

On Fri, Jun 12, 2015 at 11:07:29AM +0200, Marek Polacek wrote:
> Ping.
> 
> On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:
> > On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> > > The C changes are OK.
> > 
> > Jason, do you want to approve the C++ parts?

Marek

Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) (take 3)

[Marek Polacek]

I'm pinging the C++ parts.

On Fri, Jun 19, 2015 at 12:44:36PM +0200, Marek Polacek wrote:
> Ping.
> 
> On Fri, Jun 12, 2015 at 11:07:29AM +0200, Marek Polacek wrote:
> > Ping.
> > 
> > On Fri, Jun 05, 2015 at 10:55:08AM +0200, Marek Polacek wrote:
> > > On Thu, Jun 04, 2015 at 09:04:19PM +, Joseph Myers wrote:
> > > > The C changes are OK.
> > > 
> > > Jason, do you want to approve the C++ parts?

Marek