[PING #4] [PATCH] look harder for MEM_REF operand equality to avoid -Wstringop-truncation (PR 84561)

[Richard Biener]

On Mon, Nov 19, 2018 at 5:27 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 11/16/2018 01:46 AM, Richard Biener wrote:
> > On Fri, Nov 16, 2018 at 4:09 AM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01934.html
> >>
> >> Do I need to change something in this patch to make it acceptable
> >> or should I assume we will leave this bug in GCC unfixed?
> >
> > So the issue is the next_stmt handling because for the _next_ stmt
> > we did not yet replace uses with lattice values.  This information
> > was missing all the time along (and absent from patch context).
> >
> > I notice the next_stmt handling is incredibly fragile and it doesn't
> > even check the store you identify as thouching the same object
> > writes a '\0', nor does it verify the store writes to a position after
> > the strncpy accessed area (but eventually anywhere is OK...).
>
> Yes, this is being tracked in bug 84396.  I have been planing
> to tighten it up to check that it is, in fact, the NUL character
> being inserted but other things have been getting in the way (like
> trying to fix this bug).
>
> > So I really wonder why there's the restriction on 1:1 equality of the
> > base.  That relies on proper CSE (as you saw and tried to work-around
> > in your patch) and more.
> >
> > So what I'd do is the attached.  Apply more leeway (and less at the
> > same time) and restrict it to stores from zero but allow any aliasing
> > one.  One could build up more precision by, instead of using
> > ptr_may_alias_ref_p use refs_may_alias_p and build up a ao_ref
> > for the strncpy destination using ao_ref_from_ptr_and_size, but
> > I didn't bother to figure out what constraint on len the function
> > computed up to this point.
> >
> > The patch fixes the testcase.
>
> It does, but it also introduces two regressions into the test
> suite (false negatives).

The patch was only for suggestion and not meant to be complete ....

>  The code your patch removes is there
> to handle these cases.  I'll look into your suggestion to use
> refs_may_alias_p to avoid these regressions.
>
> Martin
>
> PS With the suggested patch GCC fails to detect the following:

eventually fixed with my suggestion to use ao_ref_from_ptr_and_size.

>    struct A { char str[3]; };
>
>    struct B { struct A a[3]; int i; };
>    struct C { struct B b[3]; int i; };
>
>    void f (struct B *p, const char *s)
>    {
>      // write into p->a[0]:
>      __builtin_strncpy (p->a[0].str, s, sizeof p->a[0].str);
>
>      // write into p->a[1]:
>      p->a[1].str[sizeof p->a[1].str - 1] = 0;
>    }