Re: [PING #4][PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028)

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Nov 30, 2018 at 3:02 AM Jeff Law <law@redhat.com> wrote:
>
> On 11/29/18 4:43 PM, Martin Sebor wrote:
> >> The fallout on existing tests is minimal.  What's more concerning is
> >> that it doesn't actually pass the new test from Martin's original
> >> submission.  We get bogus warnings.
> >>
> >> At least part of the problem is weakness in maybe_diag_stxncpy_trunc.
> >> It can't handle something like this:
> >>
> >> test_literal (char * d, struct S * s)
> >> {
> >>    strncpy (d, "1234567890", 3);
> >>    _1 = d + 3;
> >>    *_1 = 0;
> >> }
> >>
> >>
> >> Note the pointer arithmetic between the strncpy and storing the NUL
> >> terminator.

I already said the way the code looks for the next stmt is totally backwards...

Oh, and I also already voiced my concerns about emitting warnings
from folding code, did I? ...

So, let's suppose we delay folding of (builtin [string]) calls to some
first special pass that also diagnoses those issues.  One obvious
place to place this pass would be where we now do the
early object-size pass.  In fact we might want to merge
the object-size pass with the strlen pass because they sound so
much related.  This pass would then fold the calls and set some
cfun->gimple_df->after_call_warnings flag we could test in the
folder (similar to how we have avoid_folding_inline_builtin ()).

This placement ensures that we already got functions early
inlined (albeit in "early optimized" form but with their diagnostics
already been emitted).

This is of course all GCC 10 material.

> > Right.  I'm less concerned about this case because it involves
> > a literal that's obviously longer than the destination but it
> > would be nice if the suppression worked here as well in case
> > the literal comes from macro expansion.  It will require
> > another tweak.
> OK.  If this isn't at the core of the regression BZ, then xfailing those
> particular cases and coming back to them is fine.
>
> >
> > But the test from my patch passes with the changes to calls.c
> > from my patch, so that's an improvement.
> >
> > I have done the test suite cleanup in the attached patch.  It
> > was indeed minimal -- not sure why I saw so many failures with
> > my initial approach.
> Richi's does the folding as part of gimple lowering.  So it's still
> pretty early -- basically it ends up hitting just a few tests that are
> looking for folded stuff in the .gimple dump.
>
> I had actually targeted this patch as one to work through and try to get
> resolved today.  Kind of funny that we were poking at it at the same time.
>
>
> >
> > I can submit a patch to handle the literal case above as
> > a followup unless you would prefer it done at the same time.
> Follow-up is fine by me.
>
> jeff