Bug 71031 - [6 Regression] ICE in extract_range_from_binary_expr_1, at tree-vrp.c:2535 w/ -Os
Summary: [6 Regression] ICE in extract_range_from_binary_expr_1, at tree-vrp.c:2535 w/...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 7.0
: P2 normal
Target Milestone: 6.2
Assignee: Marek Polacek
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2016-05-09 19:18 UTC by Arseny Solokha
Modified: 2016-05-19 18:44 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 5.3.0
Known to fail:
Last reconfirmed: 2016-05-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arseny Solokha 2016-05-09 19:18:13 UTC
gcc-7.0.0-alpha20160508 snapshot ICEs when compiling the following reduced testcase w/ -Os:

int zj;
int **yr;

void
nn (void)
{
  unsigned int od = 4;

  for (;;)
    {
      int lk;

      for (lk = 0; lk < 2; ++lk)
        {
          static int cm;

          zj = 0;
          if (od == 0)
            return;
          ++od;
          for (cm = 0; cm < 2; ++cm)
            {
              --od;
              **yr = 0;
            }
        }
    }
}

% gcc-7.0.0-alpha20160508 -c -Os z5y81wfl.c                            
z5y81wfl.c: In function 'nn':
z5y81wfl.c:5:1: internal compiler error: in extract_range_from_binary_expr_1, at tree-vrp.c:2535
 nn (void)
 ^~
Comment 1 Richard Biener 2016-05-10 08:57:00 UTC
Confirmed.

#1  0x00000000011690fc in extract_range_from_binary_expr_1 (vr=0x7fffffffd8b0, 
    code=PLUS_EXPR, expr_type=<integer_type 0x7ffff688b888 unsigned int>, 
    vr0_=0x7fffffffd790, vr1_=0x7fffffffd7b0)
    at /space/rguenther/src/svn/trunk3/gcc/tree-vrp.c:2534
2534                      gcc_assert ((min_ovf == -1 && max_ovf == 0)
(gdb) l
2529                    {
2530                      /* Min underflow or max overflow.  The range kind
2531                         changes to VR_ANTI_RANGE.  */
2532                      bool covers = false;
2533                      wide_int tem = tmin;
2534                      gcc_assert ((min_ovf == -1 && max_ovf == 0)
2535                                  || (max_ovf == 1 && min_ovf == 0));
2536                      type = VR_ANTI_RANGE;
2537                      tmin = tmax + 1;
2538                      if (wi::cmp (tmin, tmax, sgn) < 0)
(gdb) p min_ovf
$1 = 1
(gdb) p max_ovf
$2 = 0

also ICEs on the GCC 6 branch.
Comment 2 Marek Polacek 2016-05-10 09:05:29 UTC
Started with r231098.
Comment 3 Marek Polacek 2016-05-10 09:16:10 UTC
I'd like to give this a try.
Comment 4 Marek Polacek 2016-05-18 10:27:19 UTC
Here, we have [1, od_5] + UINT_MAX on type unsigned, which wraps, so we try to turn this into an anti-range [*].  But that is not possible, the anti-range doesn't exist (is empty) so we should drop to varying, I think.

[*] Same as e.g. if we have unsigned i with range [2,4], then for i - 4 we have an anti-range ~[1, UINT_MAX - 2].
Comment 5 Marek Polacek 2016-05-18 11:28:33 UTC
I don't understand how could r231098 cause this though.  If I put breakpoints on type_in_anonymous_namespace_p / type_with_linkage_p / odr_type_p, it doesn't stop at them so...
Comment 6 Marek Polacek 2016-05-18 12:45:18 UTC
The real culprit is r231097; our cc1 in bisect seed must be wrong.
Comment 7 Marek Polacek 2016-05-18 17:20:03 UTC
To expand a bit on Comment 4: we have [1, od_5] + UINT_MAX on type unsigned and are in extract_range_from_binary_expr_1.  We combine the lower bounds, which is 1 + UINT_MAX = 0(OVF).  We  then combine the upper bounds, because the max_op0 is not a constant, the result of that is UINT_MAX.  So WMIN is 0(OVF) and WMAX is UINT_MAX.  These are equal to TYPE_MIN and TYPE_MAX, respectively.  What to do with that, drop to VR_VARYING if we have min overflow or max underflow?

Richi/Jakub, any ideas?  I'm kinda stuck here :(.
Comment 8 rguenther@suse.de 2016-05-19 07:29:14 UTC
On Wed, 18 May 2016, mpolacek at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71031
> 
> Marek Polacek <mpolacek at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org,
>                    |                            |rguenth at gcc dot gnu.org
> 
> --- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
> To expand a bit on Comment 4: we have [1, od_5] + UINT_MAX on type unsigned and
> are in extract_range_from_binary_expr_1.  We combine the lower bounds, which is
> 1 + UINT_MAX = 0(OVF).  We  then combine the upper bounds, because the max_op0
> is not a constant, the result of that is UINT_MAX.  So WMIN is 0(OVF) and WMAX
> is UINT_MAX.  These are equal to TYPE_MIN and TYPE_MAX, respectively.  What to
> do with that, drop to VR_VARYING if we have min overflow or max underflow?
> 
> Richi/Jakub, any ideas?  I'm kinda stuck here :(.

The situation (+ UINT_MAX) is equal to [1, od_5] - 1 which would mean
a result range of ideally [0, od_5 - 1].

I think Eric added this code so he may want to have a look here to see
what general solution is appropriate.

But yes, instead of asserting we can always drop to VR_VARYING ...
Comment 9 Eric Botcazou 2016-05-19 10:13:12 UTC
> The situation (+ UINT_MAX) is equal to [1, od_5] - 1 which would mean
> a result range of ideally [0, od_5 - 1].

Right.

> I think Eric added this code so he may want to have a look here to see
> what general solution is appropriate.
> 
> But yes, instead of asserting we can always drop to VR_VARYING ...

That's what the block of code just above was added for:

	  /* If we have overflow for the constant part and the resulting
	     range will be symbolic, drop to VR_VARYING.  */
	  if ((min_ovf && sym_min_op0 != sym_min_op1)
	      || (max_ovf && sym_max_op0 != sym_max_op1))
	    {
	      set_value_range_to_varying (vr);
	      return;
	    }

so a safe fix is to extend it to catch this case:

	  /* If we have overflow for the constant part and the resulting
	     range will be symbolic, drop to VR_VARYING.  */
	  if ((min_ovf || max_ovf)
              && (sym_min_op0 != sym_min_op1 || sym_max_op0 != sym_max_op1))
	    {
	      set_value_range_to_varying (vr);
	      return;
	    }

If we want to compute the above expected range, we need to enter the business of symbolic ranges with TYPE_OVERFLOW_WRAPS, which is rather delicate.  And we also need to short circuit the final call to compare_values, which will drop to VR_VARYING if I'm not mistaken.
Comment 10 Marek Polacek 2016-05-19 10:48:02 UTC
(In reply to Eric Botcazou from comment #9)
> > The situation (+ UINT_MAX) is equal to [1, od_5] - 1 which would mean
> > a result range of ideally [0, od_5 - 1].
> 
> Right.
> 
> > I think Eric added this code so he may want to have a look here to see
> > what general solution is appropriate.
> > 
> > But yes, instead of asserting we can always drop to VR_VARYING ...
> 
> That's what the block of code just above was added for:
> 
> 	  /* If we have overflow for the constant part and the resulting
> 	     range will be symbolic, drop to VR_VARYING.  */
> 	  if ((min_ovf && sym_min_op0 != sym_min_op1)
> 	      || (max_ovf && sym_max_op0 != sym_max_op1))
> 	    {
> 	      set_value_range_to_varying (vr);
> 	      return;
> 	    }
> 
> so a safe fix is to extend it to catch this case:
> 
> 	  /* If we have overflow for the constant part and the resulting
> 	     range will be symbolic, drop to VR_VARYING.  */
> 	  if ((min_ovf || max_ovf)
>               && (sym_min_op0 != sym_min_op1 || sym_max_op0 != sym_max_op1))
> 	    {
> 	      set_value_range_to_varying (vr);
> 	      return;
> 	    }

That makes sense.  I'll do that.

> If we want to compute the above expected range, we need to enter the
> business of symbolic ranges with TYPE_OVERFLOW_WRAPS, which is rather
> delicate.  And we also need to short circuit the final call to
> compare_values, which will drop to VR_VARYING if I'm not mistaken.

Yea, it is (at least for me ;).

Thanks Eric & Richi!
Comment 11 Eric Botcazou 2016-05-19 11:10:33 UTC
> That makes sense.  I'll do that.

On the other hand, this might be too brutal, i.e. pessimize for types without TYPE_OVERFLOW_WRAPS because the overflows are silently accepted for them, so a better safe fix could be to drop to VR_VARYING for TYPE_OVERFLOW_WRAPS only:

	      else if (min_ovf == -1 && max_ovf == 1)
		{
		  /* Underflow and overflow, drop to VR_VARYING.  */
		  set_value_range_to_varying (vr);
		  return;
		}
Comment 12 Marek Polacek 2016-05-19 11:44:40 UTC
I'm testing this then:

--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -2525,14 +2525,19 @@ extract_range_from_binary_expr_1 (value_range *vr,
          set_value_range_to_varying (vr);
          return;
        }
+         else if ((min_ovf == 1 && max_ovf == 0)
+              || (min_ovf == 0 && max_ovf == -1))
+       {
+         /* Min overflow or max underflow., drop to VR_VARYING.  */
+         set_value_range_to_varying (vr);
+         return;
+       }
          else
        {
          /* Min underflow or max overflow.  The range kind
             changes to VR_ANTI_RANGE.  */
          bool covers = false;
          wide_int tem = tmin;
-         gcc_assert ((min_ovf == -1 && max_ovf == 0)
-                 || (max_ovf == 1 && min_ovf == 0));
          type = VR_ANTI_RANGE;
          tmin = tmax + 1;
          if (wi::cmp (tmin, tmax, sgn) < 0)
Comment 13 Marek Polacek 2016-05-19 15:46:09 UTC
Author: mpolacek
Date: Thu May 19 15:45:35 2016
New Revision: 236477

URL: https://gcc.gnu.org/viewcvs?rev=236477&root=gcc&view=rev
Log:
	PR tree-optimization/71031
	* tree-vrp.c (extract_range_from_binary_expr_1): Turn assert into a
	condition and adjust the code a bit.

	* gcc.dg/tree-ssa/vrp100.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/vrp100.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c
Comment 14 Marek Polacek 2016-05-19 15:50:30 UTC
Fixed on the trunk so far.
Comment 15 Marek Polacek 2016-05-19 18:43:03 UTC
Author: mpolacek
Date: Thu May 19 18:42:31 2016
New Revision: 236484

URL: https://gcc.gnu.org/viewcvs?rev=236484&root=gcc&view=rev
Log:
	PR tree-optimization/71031
	* tree-vrp.c (extract_range_from_binary_expr_1): Turn assert into a
	condition and adjust the code a bit.

	* gcc.dg/tree-ssa/vrp100.c: New test.

Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/tree-ssa/vrp100.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
    branches/gcc-6-branch/gcc/tree-vrp.c
Comment 16 Marek Polacek 2016-05-19 18:44:18 UTC
Fixed.