[PATCH][RFC] Fix PRs 87362/87428

[Jason Merrill]

On Tue, Sep 25, 2018 at 9:55 AM, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 25 Sep 2018, Jason Merrill wrote:
>
>> On Tue, Sep 25, 2018 at 6:34 AM, Richard Biener <rguenther@suse.de> wrote:
>> >
>> > Hi,
>> >
>> > the following two patches both "fix" the immediate issue of gdb
>> > asserting with
>> >
>> > ../../gdb/dwarf2read.c:9730: internal-error: void
>> > dw2_add_symbol_to_list(symbol*, pending**): Assertion `(*listhead) == NULL
>> > || (SYMBOL_LANGUAGE ((*listhead)->symbol[0]) == SYMBOL_LANGUAGE (symbol))'
>> > failed.
>> >
>> > on most LTO compiled objects now.  I tracked this down to us generating
>> > bogus DWARF (as I read it), specifically an inline instance that is
>> > output not as DW_TAG_inlined_subroutine but represented as
>> > DW_TAG_lexical_block (but otherwise with same children and abstract
>> > origin).  This is because we do have (quite a lot :/) inlined calls
>> > with LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION
>> > and thus inlined_function_outer_scope_p returning false on the
>> > BLOCK the inliner generates.
>> >
>> > A third yet untested alternative would be to store the combined
>> > locus|BLOCK location in BLOCK_SOURCE_LOCATION and in
>> > inlined_function_outer_scope_p check for literal UNKNOWN_LOCATION
>> > instead of looking at LOCATION_LOCUS.  We'd have to properly
>> > remap the locus in remap_block (which may be a chicken-and-egg issue)
>> > to not run foul of what I fixed with r196542.  But somehow that
>> > runs into issues with the new debug notes Alex added last year.
>> > Similarly keying on BLOCK_ABSTRACT_ORIGIN rather than
>> > BLOCK_SOURCE_LOCATION in inlined_function_outer_scope_p.
>> >
>> > Any ideas?  The least fallout is probably produced by the
>> > BUILTINS_LOCATION "hack" (but dwarf2out.c assumes that the
>> > locations are sensible and thus will output some "interesting"
>> > src-coords for the lexical block, <built-in>:0 respectively).
>>
>> Is there a reason not to use DECL_SOURCE_LOCATION (fn)?
>
> I guess that would work as well (and iff that's zero, like for
> artificial functions, BUILTINS_LOCATION might again be a valid fallback).
> It would still have src-coords on the lexical block that might be
> different as one would expect, but it might be good enough.
>
> So, would you be happy with the following?

Yes, that seems good enough.

Jason