Re: [RFC] fixing addr2line inline info

[Mark Wielaard <mjw@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4149 bytes --]

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.

>   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 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.

> 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.

>  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.

Thanks,

Mark