On 12/14/2017 06:53 PM, David Malcolm wrote:
On Thu, 2017-12-14 at 16:07 -0700, Martin Sebor wrote:
On 12/14/2017 12:24 PM, Jeff Law wrote:
On 12/11/2017 03:18 PM, 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.
My recollection is we suppress warnings from bits in system headers
because neither we nor the end users necessarily have control over
the
contents of the system headers -- in which case we'd be issuing
warnings
for something that only the vendor can fix.
I believe default suppression of warnings from system headers
probably
needs to stay.
I agree. I wasn't suggesting to get rid of -Wsystem altogether.
Rather, I meant that unlike front-end warnings, I don't know
and can't think of middle-end warnings (and only those) that
should be suppressed for system header code. They indicate
bugs in the emitted and (in most cases) reachable object code.
There should be no such code in system headers, and the middle
end warnings I've worked on go out of their way to make sure
they are emitted regardless. (It's easy to forget this part
and end up with Glibc macros like strncpy suppressing warnings
for serious bugs in user code.)
> Is there any kind of consensus on what we want to do here -- do we
want
> to try to tackle %G/%K removal for gcc-8 or defer it? If we defer
it,
> how do we want to handle printing the inline stack in the interm?
I would support it but I don't expect to have the cycles to
do the work myself at this stage. I view it as a nice design
improvement but give its minimal impact on functionality (IIUC)
it's lower priority than the bugs I'd like to fix.
Yes. The %K and %G feel a design wart to me, but doing it right is
fiddly, and I think something to punt to gcc 9.
I don't
have the impression that improving the detail included in
the inlining stack is predicated on this API change, but I'll
let David speak to that.
The patch is fine; I'd just prefer it if there was a test case. I
think you can reuse the approach used by 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.
Yes, I'm planning on add something like before committing.
Thanks for pointing out the way to do it within the framework.
I didn't think it was possible.
Martin
