On 6/19/19 8:57 AM, Martin Sebor wrote:
> 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.
You might also want to bring in Andreas K who added the original bits to
help with debug info generation and Eric B who extended that code in
2014 due to impacts on profiling.
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00769.html
https://gcc.gnu.org/ml/gcc-patches/2011-02/msg00421.html
Jeff
