[Bug c/49234] New: -Wstrict-overflow gives obviously unwarranted warning

[Bug c/49234] New: -Wstrict-overflow gives obviously unwarranted warning

[jim at meyering dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

           Summary: -Wstrict-overflow gives obviously unwarranted warning
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jim@meyering.net


Given this code, in file k.c, -O2 -Wstrict-overflow evokes a warning.
However, since the only values assigned to "state" are 0, 1 and 2,
gcc should be able to determine that no overflow is possible,
and hence should issue no warning:

char *
trim2 (char *d)
{
  int state = 0;
  char *r;

  int i;
  for (i = 0; d[i]; i++)
    {
      if (state == 0 && d[i] == ' ')
    continue;

      if (state == 0)              /* line 13 */
    state = 1;

      if (state == 1)
    {
      state = 2;
      r = d + i;
    }
    }

  if (state == 2)
    *r = '\0';

  return d;
}

$ gcc -O2 -Wstrict-overflow -c k.c
k.c: In function 'trim2':
k.c:13:10: warning: assuming signed overflow does not occur when simplifying
conditional to constant [-Wstrict-overflow]

For the record, until recently I would not have bothered enabling
-Wstrict-overflow, due to the high proportion of false-positives,
but since I've learned about the risk of this bug,
  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33498
I am now more inclined to use -Wstrict-overflow in spite of that.

[Bug tree-optimization/49234] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.05.30 20:44:22
          Component|c                           |tree-optimization
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-30 20:44:22 UTC ---
Hum ... VRP ...

Simulating block 15

Visiting statement:
state_31 = ASSERT_EXPR <state_3, state_3 != 0>;

Found new range for state_31: [1, +INF(OVF)]

yeah, well ... !?  We are entering the loop induction variable code
and SCEV computes us this (not sure why we end up with the overflow
flag set though).

[Bug tree-optimization/49234] [4.4/4.5/4.7/4.7 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
   Target Milestone|---                         |4.4.7
            Summary|-Wstrict-overflow gives     |[4.4/4.5/4.7/4.7
                   |obviously unwarranted       |Regression]
                   |warning                     |-Wstrict-overflow gives
                   |                            |obviously unwarranted
                   |                            |warning

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-05-31 08:35:26 UTC ---
Started with http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=139263

[Bug tree-optimization/49234] [4.4/4.5/4.7/4.7 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-05-31 09:02:29 UTC ---
We hit:
163724    rguenth       /* Similarly, if the new maximum is smaller or larger
than
163724    rguenth        the previous one, go all the way to +INF.  */
163724    rguenth       if (cmp_max < 0 || cmp_max > 0)
163724    rguenth       {
163724    rguenth         if (!needs_overflow_infinity (TREE_TYPE
(vr_result.max))
163724    rguenth             || !vrp_var_may_overflow (lhs, phi))
163724    rguenth           vr_result.max = TYPE_MAX_VALUE (TREE_TYPE
(vr_result.max));
163724    rguenth         else if (supports_overflow_infinity (TREE_TYPE
(vr_result.max)))
163724    rguenth           vr_result.max =
163724    rguenth               positive_overflow_infinity (TREE_TYPE
(vr_result.max));
163724    rguenth       }

As the IV (i) might overflow, vrp_var_may_overflow returns true.
In particular, chrec is SCEV_NOT_KNOWN.  Thus it just in case sets
vr_result.max to +INF(OVF) and later on we warn about it.
Before hitting this code, vr_result contains the right range [0, 2], but it
doesn't know it can safely use that.

[Bug tree-optimization/49234] [4.4/4.5/4.7/4.7 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
           Priority|P3                          |P2

[Bug tree-optimization/49234] [4.5/4.7/4.8/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.7                       |4.5.4

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-13 12:47:34 UTC ---
4.4 branch is being closed, moving to 4.5.4 target.

[Bug tree-optimization/49234] [4.5/4.7/4.8/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.4                       |4.6.4

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-02 11:36:12 UTC ---
The 4.5 branch is being closed, adjusting target milestone.

[Bug tree-optimization/49234] [4.5/4.7/4.8/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aldyh at gcc dot gnu.org
         AssignedTo|unassigned at gcc dot       |aldyh at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #6 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-02-28 13:51:36 UTC ---
I'll take a look.

[Bug tree-optimization/49234] [4.5/4.7/4.8/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #7 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-02-28 15:49:34 UTC ---
Created attachment 29555
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29555
proof of concept: untested

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dnovillo at gcc dot
                   |                            |gnu.org, rguenth at gcc dot
                   |                            |gnu.org

--- Comment #8 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-02-28 16:00:26 UTC ---
As Jakub explained in comment 3, this is a problem in the VRP code.

To avoid bouncing from max to max (or slowly increasing to +INF), we
immediately go to +INF(OVF), even if we have correct ranges.

One alternative that I show in the attached patch (comment 7: proof of concept
hack, untested), is to keep track of how many times we are iterating through an
SSA.  If we go past an arbitrary number (say 10), we can then go to +INF.

So basically, keep track of a few states, but if it starts getting ridiculous
step it up to INF.  Similarly for the cmp_min code.  The relevant part of my
patch is shown below.

Does this seem like an approach worth exploring (this silences the warning), or
does anyone have a better suggestion?

-      /* Similarly, if the new maximum is smaller or larger than
-     the previous one, go all the way to +INF.  */
-      if (cmp_max < 0 || cmp_max > 0)
+      /* Adjust to a new maximum if it has changed.  For the first few
+     iterations, adjust the maximum accordingly.  However, if
+     we're iterating too much, go all the way to +INF to avoid
+     either bouncing around or iterating millions of times to
+     reach +INF.  */
+      if ((cmp_max < 0 || cmp_max > 0))
     {
-      if (!needs_overflow_infinity (TREE_TYPE (vr_result.max))
-          || !vrp_var_may_overflow (lhs, phi))
-        vr_result.max = TYPE_MAX_VALUE (TREE_TYPE (vr_result.max));
-      else if (supports_overflow_infinity (TREE_TYPE (vr_result.max)))
-        vr_result.max =
-        positive_overflow_infinity (TREE_TYPE (vr_result.max));
+      lhs_vr->visited++;
+      if (lhs_vr->visited < 10) // Handle the first 10 iterations.
+        vr_result.max = lhs_vr->max;
+      else
+        {
+          if (!needs_overflow_infinity (TREE_TYPE (vr_result.max))
+          || !vrp_var_may_overflow (lhs, phi))
+        vr_result.max = TYPE_MAX_VALUE (TREE_TYPE (vr_result.max));
+          else if (supports_overflow_infinity (TREE_TYPE (vr_result.max)))
+        vr_result.max =
+          positive_overflow_infinity (TREE_TYPE (vr_result.max));
+        }
     }

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #9 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2013-02-28 20:40:55 UTC ---
(In reply to comment #3)
> We hit:
> 163724    rguenth       /* Similarly, if the new maximum is smaller or larger
> than
> 163724    rguenth        the previous one, go all the way to +INF.  */
> 163724    rguenth       if (cmp_max < 0 || cmp_max > 0)
> 163724    rguenth       {
> 163724    rguenth         if (!needs_overflow_infinity (TREE_TYPE
> (vr_result.max))
> 163724    rguenth             || !vrp_var_may_overflow (lhs, phi))
> 163724    rguenth           vr_result.max = TYPE_MAX_VALUE (TREE_TYPE
> (vr_result.max));
> 163724    rguenth         else if (supports_overflow_infinity (TREE_TYPE
> (vr_result.max)))
> 163724    rguenth           vr_result.max =
> 163724    rguenth               positive_overflow_infinity (TREE_TYPE
> (vr_result.max));
> 163724    rguenth       }
> 

(In reply to comment #8)
> 
> Does this seem like an approach worth exploring (this silences the warning), or
> does anyone have a better suggestion?

Isn't the problem that vrp_var_may_overflow returns true even though 'state'
cannot overflow? Jakub says:

> As the IV (i) might overflow, vrp_var_may_overflow returns true.
> In particular, chrec is SCEV_NOT_KNOWN.  Thus it just in case sets
> vr_result.max to +INF(OVF) and later on we warn about it.
> Before hitting this code, vr_result contains the right range [0, 2], but it
> doesn't know it can safely use that.

Couldn't be possible to detect this by the fact that 'state' does not depend on
anything variable?

Also, in such a case, the algorithm cannot iterate more than the number of phi
nodes in the loop (if I understand the VRP correctly, which I most likely
don't).

But I looked around and I honestly don't know how to implement this idea.

In any case, your patch would need to adjust the code for the minimum also, no?
Because the same behaviour can be triggered just by using negative numbers to
trigger a negative overflow infinity.

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |iant at google dot com

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-01 11:45:54 UTC ---
(In reply to comment #9)
> (In reply to comment #3)
> > We hit:
> > 163724    rguenth       /* Similarly, if the new maximum is smaller or larger
> > than
> > 163724    rguenth        the previous one, go all the way to +INF.  */
> > 163724    rguenth       if (cmp_max < 0 || cmp_max > 0)
> > 163724    rguenth       {
> > 163724    rguenth         if (!needs_overflow_infinity (TREE_TYPE
> > (vr_result.max))
> > 163724    rguenth             || !vrp_var_may_overflow (lhs, phi))
> > 163724    rguenth           vr_result.max = TYPE_MAX_VALUE (TREE_TYPE
> > (vr_result.max));
> > 163724    rguenth         else if (supports_overflow_infinity (TREE_TYPE
> > (vr_result.max)))
> > 163724    rguenth           vr_result.max =
> > 163724    rguenth               positive_overflow_infinity (TREE_TYPE
> > (vr_result.max));
> > 163724    rguenth       }
> > 
> 
> (In reply to comment #8)
> > 
> > Does this seem like an approach worth exploring (this silences the warning), or
> > does anyone have a better suggestion?
> 
> Isn't the problem that vrp_var_may_overflow returns true even though 'state'
> cannot overflow? Jakub says:
> 
> > As the IV (i) might overflow, vrp_var_may_overflow returns true.
> > In particular, chrec is SCEV_NOT_KNOWN.  Thus it just in case sets
> > vr_result.max to +INF(OVF) and later on we warn about it.
> > Before hitting this code, vr_result contains the right range [0, 2], but it
> > doesn't know it can safely use that.
> 
> Couldn't be possible to detect this by the fact that 'state' does not depend on
> anything variable?
> 
> Also, in such a case, the algorithm cannot iterate more than the number of phi
> nodes in the loop (if I understand the VRP correctly, which I most likely
> don't).
> 
> But I looked around and I honestly don't know how to implement this idea.
> 
> In any case, your patch would need to adjust the code for the minimum also, no?
> Because the same behaviour can be triggered just by using negative numbers to
> trigger a negative overflow infinity.

I think we should simply not set overflow on this code path in any case.  We
have (OVF) just for the sake of -Wstrict-overflow warnings which in VRP are
implemeted in the worst possible way.

The issue with iterating is that you either can re-create the issue
with a different testcase or you blow up compile-time.

When somebody wants to improve VRP for SSA cycles then the way to go is
to compute the SSA SCC, classify it (in this case we only have assigns
from constants, not increments or decrements) and choose a solving
strathegy.

Oh, and of course strict-overflow warnings are all Ians fault ;)

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[ian at airs dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Ian Lance Taylor <ian at airs dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ian at airs dot com

--- Comment #11 from Ian Lance Taylor <ian at airs dot com> 2013-03-01 14:52:53 UTC ---
I suspect we can handle this case by observing that all the incoming values to
the PHI node are constants.

It's true that strict overflow warnings are my fault but I believe it was a
necessary move to keep people from turning on -fwrapv.

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #12 from Aldy Hernandez <aldyh at redhat dot com> 2013-03-01 19:17:32 UTC ---
> --- Comment #11 from Ian Lance Taylor <ian at airs dot com> 2013-03-01 14:52:53 UTC ---
> I suspect we can handle this case by observing that all the incoming values to
> the PHI node are constants.

Ian.

They're not all constants.  In this particular case we have phis like this:

state_2 = PHI <0(6), state_3(12), 2(5)>

I suppose we could chase the SSA def chain and if all the phi arguments 
are either constants, or known to point to an SSA that has been 
previously analyzed as constant, then we can avoid generating an 
overflow.  But we'd have to keep track of states across calls to 
vrp_visit_phi_node.  Is this what you had in mind, or am I 
misunderstanding something?

Obviously, Richi's idea is much simpler :).  With his suggestion we 
could probably do with:

@@ -8111,11 +8109,9 @@ vrp_visit_phi_node (gimple phi)
        if (cmp_max < 0 || cmp_max > 0)
         {
           if (!needs_overflow_infinity (TREE_TYPE (vr_result.max))
-             || !vrp_var_may_overflow (lhs, phi))
+             || !vrp_var_may_overflow (lhs, phi)
+             || supports_overflow_infinity (TREE_TYPE (vr_result.max)))
             vr_result.max = TYPE_MAX_VALUE (TREE_TYPE (vr_result.max));
-         else if (supports_overflow_infinity (TREE_TYPE (vr_result.max)))
-           vr_result.max =
-               positive_overflow_infinity (TREE_TYPE (vr_result.max));
         }

And similarly for MIN.  In the above, supports_overflow_infinity is just 
there to make sure we have a CONSTANT_CLASS_P.  If that's not needed, we 
could even do:

    if (cmp_max < 0 || cmp_max > 0)
      vr_result.max = TYPE_MAX_VALUE (TREE_TYPE (vr_result.max));

and be done with it.

Let me know.  I am willing to entertain any approach y'all suggest.

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[ian at airs dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #13 from Ian Lance Taylor <ian at airs dot com> 2013-03-01 19:23:00 UTC ---
How hard would it be to test whether the values are all constant or have the
same SSA_NAME_VAR as the value we are setting?

My only concern about richi's suggestion is that it will miss some simple
cases.
  for (i = 1; i >= 0; i *= 2)
    {
      int j = fn1 () ? i : 1;
      if (j >= 0)
        fn ();
    }

Here i overflows and so does j, but I think your patch will teach VRP that j
does not overflow, so we will get no warning for j >= 0.

Or something like that.

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #14 from Aldy Hernandez <aldyh at redhat dot com> 2013-03-01 19:33:47 UTC ---
On 03/01/13 13:23, ian at airs dot com wrote:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234
>
> --- Comment #13 from Ian Lance Taylor <ian at airs dot com> 2013-03-01 19:23:00 UTC ---
> How hard would it be to test whether the values are all constant or have the
> same SSA_NAME_VAR as the value we are setting?

That was actually my first idea, but it seemed too simple :).  I will do 
so and report back.

Thanks.

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #15 from rguenther at suse dot de <rguenther at suse dot de> 2013-03-04 09:57:40 UTC ---
On Fri, 1 Mar 2013, aldyh at redhat dot com wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234
> 
> --- Comment #12 from Aldy Hernandez <aldyh at redhat dot com> 2013-03-01 19:17:32 UTC ---
> > --- Comment #11 from Ian Lance Taylor <ian at airs dot com> 2013-03-01 14:52:53 UTC ---
> > I suspect we can handle this case by observing that all the incoming values to
> > the PHI node are constants.
> 
> Ian.
> 
> They're not all constants.  In this particular case we have phis like this:
> 
> state_2 = PHI <0(6), state_3(12), 2(5)>
> 
> I suppose we could chase the SSA def chain and if all the phi arguments 
> are either constants, or known to point to an SSA that has been 
> previously analyzed as constant, then we can avoid generating an 
> overflow.  But we'd have to keep track of states across calls to 
> vrp_visit_phi_node.  Is this what you had in mind, or am I 
> misunderstanding something?
> 
> Obviously, Richi's idea is much simpler :).  With his suggestion we 
> could probably do with:
> 
> @@ -8111,11 +8109,9 @@ vrp_visit_phi_node (gimple phi)
>         if (cmp_max < 0 || cmp_max > 0)
>          {
>            if (!needs_overflow_infinity (TREE_TYPE (vr_result.max))
> -             || !vrp_var_may_overflow (lhs, phi))
> +             || !vrp_var_may_overflow (lhs, phi)
> +             || supports_overflow_infinity (TREE_TYPE (vr_result.max)))
>              vr_result.max = TYPE_MAX_VALUE (TREE_TYPE (vr_result.max));
> -         else if (supports_overflow_infinity (TREE_TYPE (vr_result.max)))
> -           vr_result.max =
> -               positive_overflow_infinity (TREE_TYPE (vr_result.max));
>          }
> 
> And similarly for MIN.  In the above, supports_overflow_infinity is just 
> there to make sure we have a CONSTANT_CLASS_P.  If that's not needed, we 
> could even do:
> 
>     if (cmp_max < 0 || cmp_max > 0)
>       vr_result.max = TYPE_MAX_VALUE (TREE_TYPE (vr_result.max));
> 
> and be done with it.

That's what I was suggesting.

As for strict-overflow warnings in VRP my suggestion was that we
want to warn only when a folding result _changes_.  Thus, propagate
-fwrapv and -fno-wrapv in "parallel" using two lattices and compare
the final result.  That's way less prone to false positives.

Richard.

[Bug tree-optimization/49234] [4.5/4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #16 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-05 18:00:12 UTC ---
Created attachment 29590
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29590
WIP: proposed patch special casing constant phi arguments

Ian.

Sure, I can handle SSA_NAME_VAR equality, but then we won't be able to handle
self referential operations like the one in gcc.dg/Wstrict-overflow-12.c.  For
example, with your suggested approach (in this attached patch), we fail here:

  for (i = 1, bits = 1; i > 0; i += i) /* { dg-warning "assuming signed
overflow does not occur" "correct warning" } */
    ++bits;

Because we encounter something like this which is perfectly valid with your
approach:

i_1 = PHI <1(2), i_4(3)>

[Bug tree-optimization/49234] [4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.5/4.6/4.7/4.8            |[4.6/4.7/4.8 Regression]
                   |Regression]                 |-Wstrict-overflow gives
                   |-Wstrict-overflow gives     |obviously unwarranted
                   |obviously unwarranted       |warning
                   |warning                     |

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-06 08:55:46 UTC ---
The patch is a hack.  Comparing SSA_NAME_VAR is very fragile, nothing but
debug info cares about SSA_NAME_VAR.

I can't see why constant PHI arguments should be in any way special - in
fact this is just the very usual induction variable PHI with a constant
initial value ...

[Bug tree-optimization/49234] [4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #18 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-06 16:11:22 UTC ---
Created attachment 29599
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29599
do not set overflow on [+-]INF

This is Richi's suggestion from comment 10.  The problem is that it obviously
causes the VRP strict overflow warnings to fail:

$ make check-gcc RUNTESTFLAGS=dg.exp=*strict*
FAIL: gcc.dg/Wstrict-overflow-12.c correct warning (test for warnings, line 13)
FAIL: gcc.dg/Wstrict-overflow-13.c correct warning (test for warnings, line 14)
FAIL: gcc.dg/Wstrict-overflow-14.c  (test for warnings, line 13)
FAIL: gcc.dg/Wstrict-overflow-15.c  (test for warnings, line 13)
FAIL: gcc.dg/Wstrict-overflow-21.c correct warning (test for warnings, line 8)
FAIL: gcc.dg/no-strict-overflow-6.c scan-tree-dump optimized "return bits"
FAIL: c-c++-common/restrict-1.c  -Wc++-compat  (test for excess errors)

If y'all are willing to work with this patch and XFAIL these tests, I'm more
than happy to test and commit this patch, otherwise I may have to drop this for
now, as I am not volunteering to fix VRP for SSA cycles as suggested.  I am
switching gears from 4.8 bugfixing into other duties shortly...

Let me know...

[Bug tree-optimization/49234] [4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[ian at airs dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #19 from Ian Lance Taylor <ian at airs dot com> 2013-03-06 16:18:50 UTC ---
Those tests are more or less the whole point of the strict-overflow warning. 
-Wstrict-overflow exists to have an optional warning that tells you when you
may run into trouble.  For a warning of this type it's much better to have a
false positive than a false negative.  A false positive is just annoying.  A
false negative causes you to miss a potential bug in the program.

Sorry you've put so much time into this, but I don't see how it could be
acceptable to have a false negative on a case like Wstrict-overflow-12.c.

[Bug tree-optimization/49234] [4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #20 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-06 16:28:12 UTC ---
Oh, no worries Ian.  I totally agree.  I just wanted to put all this out there,
since I'm unfortunately about to drop it.

We should probably close this as a WONTFIX, or perhaps just drop this in
priority.  A false positive is not the end of the world, so I don't see how
this merits a P2 for the release.

Thoughts?

[Bug tree-optimization/49234] [4.6/4.7/4.8 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #21 from rguenther at suse dot de <rguenther at suse dot de> 2013-03-07 08:36:43 UTC ---
On Wed, 6 Mar 2013, aldyh at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234
> 
> --- Comment #20 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2013-03-06 16:28:12 UTC ---
> Oh, no worries Ian.  I totally agree.  I just wanted to put all this out there,
> since I'm unfortunately about to drop it.
> 
> We should probably close this as a WONTFIX, or perhaps just drop this in
> priority.  A false positive is not the end of the world, so I don't see how
> this merits a P2 for the release.
> 
> Thoughts?

I'd say we just give up here due to the fact that propagation in
SSA / CFG cycles is imprecise and that it is thus not possible
to avoid either false positives or false negatives.

A P2 regression isn't so bad, we have tons of those.

[Bug tree-optimization/49234] [4.6/4.7/4.8/4.9 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vapier at gentoo dot org

--- Comment #22 from Richard Biener <rguenth at gcc dot gnu.org> 2013-04-12 08:40:46 UTC ---
*** Bug 56928 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/49234] [4.7/4.8/4.9 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.4                       |4.7.4

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 15:16:52 UTC ---
GCC 4.6.4 has been released and the branch has been closed.

[Bug tree-optimization/49234] [4.7/4.8/4.9/4.10 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |4.8.4

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
The 4.7 branch is being closed, moving target milestone to 4.8.4.

[Bug tree-optimization/49234] [4.8/4.9/5 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug tree-optimization/49234] [4.8/4.9/5 Regression] -Wstrict-overflow gives obviously unwarranted warning

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |aldyh at gcc dot gnu.org
         Resolution|---                         |FIXED

--- Comment #26 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Closed.  Fixed with this:

commit b7f05e98d52b950f3422ea5d161a0e1d0642acf0
Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Apr 28 14:07:51 2014 +0000

    2014-04-28  Richard Biener  <rguenther@suse.de>

        * tree-vrp.c (vrp_var_may_overflow): Remove.
        (vrp_visit_phi_node): Rather than bumping to +-INF possibly
        with overflow immediately bump to one before that value and
        let iteration figure out overflow status.

        * gcc.dg/tree-ssa/vrp91.c: New testcase.
        * gcc.dg/Wstrict-overflow-14.c: XFAIL.
        * gcc.dg/Wstrict-overflow-15.c: Likewise.
        * gcc.dg/Wstrict-overflow-18.c: Remove XFAIL.

[Bug tree-optimization/49234] [4.8/4.9 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
      Known to work|                            |5.0
         Resolution|FIXED                       |---
            Summary|[4.8/4.9/5 Regression]      |[4.8/4.9 Regression]
                   |-Wstrict-overflow gives     |-Wstrict-overflow gives
                   |obviously unwarranted       |obviously unwarranted
                   |warning                     |warning

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
Only for trunk.

[Bug tree-optimization/49234] [4.8/4.9 Regression] -Wstrict-overflow gives obviously unwarranted warning

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.5                       |4.9.3

--- Comment #28 from Richard Biener <rguenth at gcc dot gnu.org> ---
The gcc-4_8-branch is being closed, re-targeting regressions to 4.9.3.

[Bug tree-optimization/49234] [4.9 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

--- Comment #29 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.9.3 has been released.

[Bug tree-optimization/49234] [4.9 Regression] -Wstrict-overflow gives obviously unwarranted warning

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49234

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.3                       |4.9.4