On Wed, 26 Feb 2025, Jakub Jelinek wrote:

> On Wed, Feb 26, 2025 at 10:58:26AM +0100, Richard Biener wrote:
> > > This PR is about ubsan error on the c - cx1 + cy1 evaluation in the first
> > > hunk.
> > > 
> > > The following patch hopefully fixes that by doing the 
> > > additions/subtractions
> > > in poly_uint64 rather than poly_int64.  Or shall we instead perform it in
> > > offset_int and watch for overflows and punt somehow for those?
> > 
> > I think when we have the offset computation overflow ignoring such
> > overflow will make the memrefs_conflict_p give possibly wrong
> > answers.  In the PR you say cselib now has those
> > -9223372036854775807ish offsets from sp, why does it do alias queries
> > with those clearly invalid offsets?
> 
> It wants to have a MEM which overlaps anything below the stack.
> So, uses for stack grows down and non-biased stack sp - PTRDIFF_MAX with
> PTRDIFF_MAX MEM_SIZE as an approximation to that.

I see.  Wouldn't setting MEM_OFFSET_KNOWN_P and MEM_SIZE_KNOWN_P
to false work as well?

> > So yes, I think we need to punt on overflow.  Using poly_offset_int
> > should work but it comes at a cost.  Does poly_wide_int have the same
> > wide-int like overflow overloads?
> 
> poly_offset_int will be definitely cheaper than other poly_wide_int, it
> would need to be at least 66 bits and so 128 bit is certainly cheaper then.
> 
> So like this if it passes full bootstrap/regtest?

Yes, that looks good to me.

Thanks,
Richard.

> 2025-02-26  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR middle-end/118819
>       * alias.cc (memrefs_conflict_p): Perform arithmetics on c, xsize and
>       ysize in poly_offset_int and return -1 if it is not representable in
>       poly_int64.
> 
> --- gcc/alias.cc.jj   2025-01-02 11:23:24.000000000 +0100
> +++ gcc/alias.cc      2025-02-26 12:31:52.860341105 +0100
> @@ -2535,19 +2535,39 @@ memrefs_conflict_p (poly_int64 xsize, rt
>           return memrefs_conflict_p (xsize, x1, ysize, y1, c);
>         if (poly_int_rtx_p (x1, &cx1))
>           {
> +           poly_offset_int co = c;
> +           co -= cx1;
>             if (poly_int_rtx_p (y1, &cy1))
> -             return memrefs_conflict_p (xsize, x0, ysize, y0,
> -                                        c - cx1 + cy1);
> +             {
> +               co += cy1;
> +               if (!co.to_shwi (&c))
> +                 return -1;
> +               return memrefs_conflict_p (xsize, x0, ysize, y0, c);
> +             }
> +           else if (!co.to_shwi (&c))
> +             return -1;
>             else
> -             return memrefs_conflict_p (xsize, x0, ysize, y, c - cx1);
> +             return memrefs_conflict_p (xsize, x0, ysize, y, c);
>           }
>         else if (poly_int_rtx_p (y1, &cy1))
> -         return memrefs_conflict_p (xsize, x, ysize, y0, c + cy1);
> +         {
> +           poly_offset_int co = c;
> +           co += cy1;
> +           if (!co.to_shwi (&c))
> +             return -1;
> +           return memrefs_conflict_p (xsize, x, ysize, y0, c);
> +         }
>  
>         return -1;
>       }
>        else if (poly_int_rtx_p (x1, &cx1))
> -     return memrefs_conflict_p (xsize, x0, ysize, y, c - cx1);
> +     {
> +       poly_offset_int co = c;
> +       co -= cx1;
> +       if (!co.to_shwi (&c))
> +         return -1;
> +       return memrefs_conflict_p (xsize, x0, ysize, y, c);
> +     }
>      }
>    else if (GET_CODE (y) == PLUS)
>      {
> @@ -2563,7 +2583,13 @@ memrefs_conflict_p (poly_int64 xsize, rt
>  
>        poly_int64 cy1;
>        if (poly_int_rtx_p (y1, &cy1))
> -     return memrefs_conflict_p (xsize, x, ysize, y0, c + cy1);
> +     {
> +       poly_offset_int co = c;
> +       co += cy1;
> +       if (!co.to_shwi (&c))
> +         return -1;
> +       return memrefs_conflict_p (xsize, x, ysize, y0, c);
> +     }
>        else
>       return -1;
>      }
> @@ -2616,8 +2642,16 @@ memrefs_conflict_p (poly_int64 xsize, rt
>         if (maybe_gt (xsize, 0))
>           xsize = -xsize;
>         if (maybe_ne (xsize, 0))
> -         xsize += sc + 1;
> -       c -= sc + 1;
> +         {
> +           poly_offset_int xsizeo = xsize;
> +           xsizeo += sc + 1;
> +           if (!xsizeo.to_shwi (&xsize))
> +             return -1;
> +         }
> +       poly_offset_int co = c;
> +       co -= sc + 1;
> +       if (!co.to_shwi (&c))
> +         return -1;
>         return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)),
>                                    ysize, y, c);
>       }
> @@ -2631,8 +2665,16 @@ memrefs_conflict_p (poly_int64 xsize, rt
>         if (maybe_gt (ysize, 0))
>           ysize = -ysize;
>         if (maybe_ne (ysize, 0))
> -         ysize += sc + 1;
> -       c += sc + 1;
> +         {
> +           poly_offset_int ysizeo = ysize;
> +           ysizeo += sc + 1;
> +           if (!ysizeo.to_shwi (&ysize))
> +             return -1;
> +         }
> +       poly_offset_int co = c;
> +       co += sc + 1;
> +       if (!co.to_shwi (&c))
> +         return -1;
>         return memrefs_conflict_p (xsize, x,
>                                    ysize, canon_rtx (XEXP (y, 0)), c);
>       }
> @@ -2643,7 +2685,11 @@ memrefs_conflict_p (poly_int64 xsize, rt
>        poly_int64 cx, cy;
>        if (poly_int_rtx_p (x, &cx) && poly_int_rtx_p (y, &cy))
>       {
> -       c += cy - cx;
> +       poly_offset_int co = c;
> +       co += cy;
> +       co -= cx;
> +       if (!co.to_shwi (&c))
> +         return -1;
>         return offset_overlap_p (c, xsize, ysize);
>       }
>  
> 
> 
>       Jakub
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to