Re: [PATCH] have -Wnonnull print inlining stack (PR 83369)

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2017-12-11 at 15:18 -0700, Martin Sebor wrote:
> On 12/11/2017 02:08 PM, David Malcolm wrote:
> > On Mon, 2017-12-11 at 09:51 -0700, Martin Sebor wrote:
> > > Bug 83369 - Missing diagnostics during inlining, notes that when
> > > -Wnonnull is issued for an inlined call to a built-in function,
> > > GCC doesn't print the inlining stack, making it hard to debug
> > > where the problem comes from.
> > > 
> > > When the -Wnonnull warning was introduced into the middle-end
> > > the diagnostic machinery provided no way to print the inlining
> > > stack (analogous to %K for trees).  Since then GCC has gained
> > > support for the %G directive which does just that.  The attached
> > > patch makes use of the directive to print the inlining context
> > > for -Wnonnull.
> > > 
> > > The patch doesn't include a test because the DejaGnu framework
> > > provides no mechanism to validate this part of GCC output (see
> > > also bug 83336).
> > > 
> > > Tested on x86_64-linux with no regressions.
> > > 
> > > Martin
> > 
> > I'm wondering if we should eliminate %K and %G altogether, and make
> > tree-diagnostic.c and friends automatically print the inlining
> > stack
> > -they just need a location_t (the issue is with system headers, I
> > suppose, but maybe we can just make that smarter: perhaps only
> > suppress
> > if every location in the chain is in a system header?).  I wonder
> > if
> > that would be GCC 9 material at this point though?
> 
> Getting rid of %G and %K sounds fine to me.  I can't think of
> a use case for suppressing middle end diagnostics in system
> headers so unless someone else can it might be a non-issue.
> Since the change would fix a known bug it seems to me that it
> should be acceptable even at this stage.

There's the "artificial" attribute, which as far as I can tell was
introduced as a kind of workaround for the "inlined code now looks like
it's in a system header" issue (but may also related to debuginfo???)

> > Coming back to this patch: regarding tests, would you be able to
> > use
> > the techniques of:
> >   https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00646.html
> > to build a test case?
> 
> I think so.  If I'm reading it right, it depends on the prune.exp
> changes and could be as simple as
> testsuite/gcc.dg/plugin/diagnostic-test-inlining-4.c, right?
> 
> Does it need to be  in the plugin directory or can any test
> use this approach?

Any test could use that approach, without needing to use the plugin;
that plugin exists purely to separate the testing of the "print
inlining information for middle-end warnings" out from any particular
middle-end warning.

Now that I think about it a bit more, that approach has an issue, in
that it hardcodes a lot of what we expect the output format to be.

That's OK for the patch I linked to, since that's the purpose of that
test.

But for your purposes, I think that all you need is the approach Aldy
used in gcc.dg/tm/pr52141.c, which has just a:

  /* { dg-message "inlined from \'f\'" "" { target *-*-* } 0 } */

to verify that *something* was printed about the inlining, without
overspecifying exactly what.

Dave