On 6/19/19 5:11 AM, Vladislav Ivanishin wrote:
Hi,
This patch (partially) adds displaying inlining context for
-W{maybe,}uninitialized warnings. This is not as trivial to enable as
simply supplying the "%G" format specifier, so I have some questions
below.
I need this hunk
void
percent_K_format (text_info *text, location_t loc, tree block)
{
- text->set_location (0, loc, SHOW_RANGE_WITH_CARET);
+ if (loc != UNKNOWN_LOCATION)
+ text->set_location (0, loc, SHOW_RANGE_WITH_CARET);
for two reasons:
- The gimple return stmt has UNKNOWN_LOCATION due to this code in
lower_gimple_return ():
if (gimple_return_retval (stmt) == gimple_return_retval (tmp_rs.stmt))
{
/* Remove the line number from the representative return statement.
It now fills in for many such returns. Failure to remove this
will result in incorrect results for coverage analysis. */
gimple_set_location (tmp_rs.stmt, UNKNOWN_LOCATION);
This code is also causing quite a few non-trivial instances of
-Wreturn-local-addr to have no location after two (or more) such
statements have been merged (I raised PR 90735 for it), independent
of inlining. I wonder if using the location of the closing curly
brace of the function definition (if it's available somewhere)
would work without messing up the coverage analysis.
(In case you are wondering, the code and the comment were
added in 2004.)
- When percent_K_format is called, TEXT might already hold precise
location information in case its (indirect) caller is
warning_at/error_at. So it seems to me, this function lacks
distinguishing the two cases: being called from plain warning/error
functions vs. their *_at counterparts (the location shouldn't be
updated in the latter case).
Martin, you did the necessary work for percent_G_format to accept
arbitrary gimple statements rather than just calls for PR86650, but
left the PR open. Do you remember running into that sort of problem,
or was it something else?
I'm not sure it was the same problem. I described what I was seeing
when I posted the patch:
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01286.html
I think it's worth revisiting this with your patch in place. It would
be nice to get the inlining context in place for -Warray-bounds too.
David Malcolm (CC'd) is the expert on locations and the diagnostic
subsystem so he will have a much better insight than me into all of
this than me.
Martin
Sometimes inlining context is still lost (with the current patch), as
can be seen e.g. with a version of the test with 's/unsigned yo/int yo/'
substitution applied. (The chain of block = BLOCK_SUPERCONTEXT (block)
is broken - it ends with 0 rather than a FUNCTION_DECL.) Is this known
and expected (e.g. because pass_late_warn_uninitialized runs very late),
or something to be investigated and fixed?
The patch was bootstrapped and regtested on x86_64-pc-linux-gnu. Shall
it be applied?
