Re: [PATCH] PR tree-optimization/38572 (VRP and C++ enums)

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Mon, Dec 22, 2008 at 4:56 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> While exposed by my patch, this bug was most likely latent before.
>
> It is a latent bug in the handling of out-of-bounds values.  In the
> testcase, values 256 and 257 are assigned to an enum whose maximum value
> is 6 (and the TYPE_MAX_VALUE is 7).
>
> During VRP a phi's value range changes from [256, 256] to [256, 257] and
> VRP then forces the upper bound to the max-value of the type.  This
> however generates the invalid range [256, 7].  After the patch we punt
> and give VARYING.
>
> Ok?  What about 4.3?  No known test case there, but one could probably
> be found with enough patience...

Ok for trunk.

Thanks,
Richard.

> Paolo
>
> 2008-12-22  Paolo Bonzini  <bonzini@gnu.org>
>
>        PR tree-optimization/38572
>        * tree-vrp.c (vrp_visit_phi_node): Look out for invalid ranges
>        and change them to VARYING.
>
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      (revision 142865)
> +++ gcc/tree-vrp.c      (working copy)
> @@ -6361,9 +6361,12 @@ vrp_visit_phi_node (gimple phi)
>             minimums.  */
>          if (cmp_min > 0 || cmp_min < 0)
>            {
> -             /* If we will end up with a (-INF, +INF) range, set it
> -                to VARYING.  */
> -             if (vrp_val_is_max (vr_result.max))
> +             /* If we will end up with a (-INF, +INF) range, set it to
> +                VARYING.  Same if the previous max value was invalid for
> +                the type and we'd end up with vr_result.min > vr_result.max.  */
> +             if (vrp_val_is_max (vr_result.max)
> +                 || compare_values (TYPE_MIN_VALUE (TREE_TYPE (vr_result.min)),
> +                                    vr_result.max) > 0)
>                goto varying;
>
>              if (!needs_overflow_infinity (TREE_TYPE (vr_result.min))
> @@ -6380,9 +6383,12 @@ vrp_visit_phi_node (gimple phi)
>             the previous one, go all the way to +INF.  */
>          if (cmp_max < 0 || cmp_max > 0)
>            {
> -             /* If we will end up with a (-INF, +INF) range, set it
> -                to VARYING.  */
> -             if (vrp_val_is_min (vr_result.min))
> +             /* If we will end up with a (-INF, +INF) range, set it to
> +                VARYING.  Same if the previous min value was invalid for
> +                the type and we'd end up with vr_result.max < vr_result.min.  */
> +             if (vrp_val_is_min (vr_result.min)
> +                 || compare_values (TYPE_MAX_VALUE (TREE_TYPE (vr_result.max)),
> +                                    vr_result.min) < 0)
>                goto varying;
>
>              if (!needs_overflow_infinity (TREE_TYPE (vr_result.max))
>
>
>
> Warning: No xauth data; using fake authentication data for X11 forwarding.
> // PR tree-optimization/38572
> // { dg-do compile }
> // { dg-options "-O2" }
>
> // Crash caused by the out-of-bounds enum values (all the remaining cruft
> // is needed only to trigger the appropriate code path in tree-vrp.c).
> enum JSOp
> {
>  JSOP_GETELEM = 5,
>  JSOP_LIMIT
> };
> extern void g ();
> void f (char *pc, char *endpc, int format, char ***fp, enum JSOp op)
> {
>  while (pc <= endpc)
>    {
>      if ((fp && *fp && pc == **fp) || pc == endpc)
>        {
>          if (format == 1)
>            op = (JSOp) 256;
>          else if (format == 2)
>            op = (JSOp) 257;
>          else
>            op = JSOP_GETELEM;
>        }
>      if (op >= JSOP_LIMIT)
>        {
>          if (format)
>            g ();
>        }
>    }
> }
> enum JSOp
> {
>  JSOP_GETELEM = 5,
>  JSOP_LIMIT
> };
>
>