Re: [RFC] fixing addr2line inline info

[Josh Stone <jistone at redhat dot com>]

On 11/21/2014 07:52 AM, Mark Wielaard wrote:
> On Tue, 2014-11-18 at 18:04 -0800, Josh Stone wrote:
>> I have a simple patch attached to improve the output addr2line -fi.  The
>> problem I had is it failed to identify the parent function name if there
>> was any lexical_block in the scope hierarchy.  For example, try the
>> following, compiled as the attached testfile-lex-inlines.bz2:
>>
>>   1 // g++ x.cpp -g -fPIC -olibx.so -shared -O3 -fvisibility=hidden
>>   2
>>   3 void foobar()
>>   4 {
>>   5   __asm__ ( "nop" ::: );
>>   6 }
>>   7
>>   8 void foo()
>>   9 {
>>  10   {
>>  11     void (*bar) () = foobar;
>>  12     bar();
>>  13   }
>>  14 }
>>
>> $ eu-addr2line -f -i -e testfile-lex-inlines.bz2 0x690
>> foobar inlined at /tmp/x.cpp:12 in _Z3foov
>> /tmp/x.cpp:9
>> ??
>> /tmp/x.cpp:12
>>
>> With my patch:
>>
>> $ ./src/addr2line -fi -e testfile-lex-inlines.bz2 0x690
>> foobar inlined at /tmp/x.cpp:12 in _Z3foov
>> /tmp/x.cpp:9
>> _Z3foov
>> /tmp/x.cpp:12
>>
>> So the name is resolved, and I'm happier.
> 
> Yes, I think your patch is correct. Going "up" till you find the first
> subprogram/inlined_subroutine is also what eu-stack does.

Great.

>>   But, as I went to make this a
>> testcase, I noticed that the ":9" line isn't really correct for "foobar"
>> -- that's the start of "foo".
>>
>> There are two candidate lines for 0x690:
>>  [    33] special opcode 246: address+16 = +0x690 <_Z3foov>, line+4 = 9
>>  [    34] special opcode 14: address+0 = +0x690 <_Z3foov>, line-4 = 5
> 
> You can also see it with:
> $ eu-readelf --debug-dump=decodedline libx.so 
> 
> DWARF section [27] '.debug_line' at offset 0x11c2:
> 
>  CU [b] x.cpp
>   line:col SBPE* disc isa op address (Statement Block Prologue Epilogue *End)
>   /tmp/x.cpp (mtime: 0, length: 0)
>      4:0   S        0   0  0 +0x0000000000000680 <_Z6foobarv>
>      5:0   S        0   0  0 +0x0000000000000680 <_Z6foobarv>
>      9:0   S        0   0  0 +0x0000000000000690 <_Z3foov>
>      5:0   S        0   0  0 +0x0000000000000690 <_Z3foov>
>      5:0   S   *    0   0  0 +0x0000000000000691 <_Z3foov+0x1>
> 
>> So what happens AFAICT, the name is found with dwarf_getscopes(),
>> which will find the innermost address match.  The line is found with
>> dwfl_module_getsrc, which uses a binary search, making no guarantees
>> if there are multiple matches.  The bsearch happened to pick the first
>> one in this case.  Perhaps this should choose the last matching
>> address to be approximately innermost?
> 
> It isn't entirely clear to me what the producer is trying to tell us
> here. Is an address increase of zero actually legal? It seems that is
> only legal if the next line is an end_sequence marker. But then there is
> also that magic zero address increase sequence at the start.

I think this is useful if you were trying to answer the other direction,
like a debugger placing breakpoints.  Lines 9 and 5 are effectively at
the same address in the binary, since the inline function sits at the
site of the inline call, with no parameter setup in this case.

It's only ambiguous for mapping an address to a line. :)

> I don't know how to mark these zero address incremented lines in a way
> that they keep sorted correctly. But if you can figure that out, then
> picking the last one seems to be the right idea. That way there is at
> least a change the line covers multiple addresses and not just one that
> happens to overlap with the next line.

Ah, I didn't realize libdw sorted the lines internally, and with qsort
which is not stable.  It would have been ok IMO if equal lines were left
in their original order.  Hmm...

Binutils' addr2line does choose the better x.cpp:5 to associate with
"foobar", but I don't know if it's just lucky or more clever.

>> I also noticed that addr2line's print_dwarf_function() doesn't try to
>> read linkage_name, although that wouldn't matter for this foobar inline.
> 
> Yes, it would be nice if it would. That is just
> s/dwarf_diename/print_diesym/ ?
> If it does that, it would also be nice if it would demangle the name if
> possible, like eu-stack does.

Yes, I think that will work.  I'd make demangling optional though.

>>  It also looks like it's trying to walk up the inline / subprogram
>> stack, but since dwarf_getscopes chases the abstract_origin, it doesn't
>> have any of that.  So print_dwarf_function() returns false, and it falls
>> back to dwfl_module_addrname to get the outer name.
>>
>> With dwarf_getscopes_die this would work, including multiple layers of
>> inlines, as it later does for -i.  But actually, I think having -i makes
>> this "inlined at ..." message redundant.  Binutils addr2line -fi doesn't
>> print anything like this.  Should we just kill that part?
> 
> I must admit I am not really following. Could you give an example? I
> don't immediate understand what you think is confusing/redundant and/or
> how your proposed output would look.

I'll try to rephrase.  And here's an example from the existing test,
which might be helpful since it actually inlines multiple layers:

$ src/addr2line -f -i -e ../tests/testfile-inlines.bz2 0x5e1
fubar inlined at /tmp/x.cpp:20 in _Z3foov
/tmp/x.cpp:10
baz
/tmp/x.cpp:20
_Z3foov
/tmp/x.cpp:26

So for that first line with "inlined at ...", print_dwarf_functions
*tries* to walk up the chain of [inline, inline, ..., subprogram].  That
is, it doesn't return until sees a subprogram (return true), or runs out
of scopes (return false).

But with dwarf_getscopes, it only ever sees two scopes, first die offset
0x151 for the inlined_subroutine, internally followed to the
abstract_origin 0x36, then second is die offset 0xb for the CU.  So it
always returns false and falls back to dwfl_module_addrname to complete
the line with "_Z3foov".

If it used dwarf_getscopes_die to get the real hierarchy, as it seems to
think it has, then the scopes would be [0x151, 0x13a, 0xe5, 0xb], and
that line would get filled out something like:
  fubar inlined at x.cpp:20 in baz inlined at x.cpp:26 in _Z3foov

And this duplicates what the rest of "-f -i" will print later.

So I'm saying print_dwarf_functions is buggy now, and if fixed it would
be redundant.

For comparison, binutils' addr2line just prints the function name for
that first line, which I think is better.

$ addr2line -f -i -e ../tests/testfile-inlines 0x5e1
fubar
/tmp/x.cpp:10
baz
/tmp/x.cpp:20
_Z3foov
/tmp/x.cpp:26