public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC] fixing addr2line inline info
@ 2014-11-19  2:04 Josh Stone
  0 siblings, 0 replies; 4+ messages in thread
From: Josh Stone @ 2014-11-19  2:04 UTC (permalink / raw)
  To: elfutils-devel

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

Hi,

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

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?

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

Patch review and other thoughts appreciated...

Josh

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: addr2line-inline-parent.patch --]
[-- Type: text/x-patch, Size: 791 bytes --]

diff --git a/src/addr2line.c b/src/addr2line.c
index 50fc2b38c367..eea39da97e2c 100644
--- a/src/addr2line.c
+++ b/src/addr2line.c
@@ -672,7 +672,22 @@ handle_address (const char *string, Dwfl *dwfl)
 			continue;
 
 		      if (show_functions)
-			print_diesym (&scopes[i + 1]);
+			{
+			  /* Search for the parent inline or function.  It
+			     might not be directly above this inline -- e.g.
+			     there could be a lexical_block in between.  */
+			  for (int j = i + 1; j < nscopes; j++)
+			    {
+			      Dwarf_Die *parent = &scopes[j];
+			      int tag = dwarf_tag (parent);
+			      if (tag == DW_TAG_inlined_subroutine
+				  || tag == DW_TAG_subprogram)
+				{
+				  print_diesym (parent);
+				  break;
+				}
+			    }
+			}
 
 		      src = NULL;
 		      lineno = 0;

[-- Attachment #3: testfile-lex-inlines.bz2 --]
[-- Type: application/x-bzip, Size: 2599 bytes --]

[-- Attachment #4: readelf-w-testfile-lex-inlines.txt --]
[-- Type: text/plain, Size: 9470 bytes --]


Call frame search table section [13] '.eh_frame_hdr':
 version:          1
 eh_frame_ptr_enc: 0x1b (sdata4 pcrel)
 fde_count_enc:    0x3 (udata4)
 table_enc:        0x3b (sdata4 datarel)
 eh_frame_ptr:     0x24 (offset: 0x6c8)
 fde_count:        3
 Table:
  0xfffffec0 (offset:  0x560) -> 0x40 fde=[    18]
  0xffffffe0 (offset:  0x680) -> 0x68 fde=[    40]
  0xfffffff0 (offset:  0x690) -> 0x80 fde=[    58]

Call frame information section [14] '.eh_frame' at offset 0x6c8:

 [     0] CIE length=20
   CIE_id:                   0
   version:                  1
   augmentation:             "zR"
   code_alignment_factor:    1
   data_alignment_factor:    -8
   return_address_register:  16
   Augmentation data:        0x1b (FDE address encoding: sdata4 pcrel)

   Program:
     def_cfa r7 (rsp) at offset 8
     offset r16 (rip) at cfa-8
     nop
     nop

 [    18] FDE length=36 cie=[     0]
   CIE_pointer:              28
   initial_location:         +0x0000000000000560 (offset: 0x560)
   address_range:            0x30 (end offset: 0x590)

   Program:
     def_cfa_offset 16
     advance_loc 6 to 0x566
     def_cfa_offset 24
     advance_loc 10 to 0x570
     def_cfa_expression 11
          [   0] breg7 8
          [   2] breg16 0
          [   4] lit15
          [   5] and
          [   6] lit11
          [   7] ge
          [   8] lit3
          [   9] shl
          [  10] plus
     nop
     nop
     nop
     nop

 [    40] FDE length=20 cie=[     0]
   CIE_pointer:              68
   initial_location:         +0x0000000000000680 <_Z6foobarv> (offset: 0x680)
   address_range:            0x2 (end offset: 0x682)

   Program:
     nop
     nop
     nop
     nop
     nop
     nop
     nop

 [    58] FDE length=20 cie=[     0]
   CIE_pointer:              92
   initial_location:         +0x0000000000000690 <_Z3foov> (offset: 0x690)
   address_range:            0x2 (end offset: 0x692)

   Program:
     nop
     nop
     nop
     nop
     nop
     nop
     nop

 [    70] Zero terminator

DWARF section [24] '.debug_aranges' at offset 0x1054:

Table at offset 0:

 Length:            44
 DWARF version:      2
 CU offset:          0
 Address size:       8
 Segment size:       0

   +0x0000000000000680 <_Z6foobarv>..+0x0000000000000691 <_Z3foov+0x1>

DWARF section [25] '.debug_info' at offset 0x1084:
 [Offset]
 Compilation unit at offset 0:
 Version: 4, Abbreviation section offset: 0, Address size: 8, Offset size: 4
 [     b]  compile_unit
           producer             (strp) "GNU C++ 4.8.3 20140911 (Red Hat 4.8.3-7) -mtune=generic -march=x86-64 -g -O3 -fPIC -fvisibility=hidden"
           language             (data1) C_plus_plus (4)
           name                 (strp) "x.cpp"
           comp_dir             (strp) "/tmp"
           low_pc               (addr) +0x0000000000000680 <_Z6foobarv>
           high_pc              (data8) 18 (+0x0000000000000692)
           stmt_list            (sec_offset) 0
 [    2d]    subprogram
             external             (flag_present) Yes
             name                 (strp) "foobar"
             decl_file            (data1) 1
             decl_line            (data1) 3
             inline               (data1) inlined (1)
 [    35]    subprogram
             abstract_origin      (ref4) [    2d]
             linkage_name         (strp) "_Z6foobarv"
             low_pc               (addr) +0x0000000000000680 <_Z6foobarv>
             high_pc              (data8) 2 (+0x0000000000000682)
             frame_base           (exprloc) 
              [   0] call_frame_cfa
             GNU_all_call_sites   (flag_present) Yes
 [    50]    subprogram
             external             (flag_present) Yes
             name                 (string) "foo"
             decl_file            (data1) 1
             decl_line            (data1) 8
             linkage_name         (strp) "_Z3foov"
             low_pc               (addr) +0x0000000000000690 <_Z3foov>
             high_pc              (data8) 2 (+0x0000000000000692)
             frame_base           (exprloc) 
              [   0] call_frame_cfa
             GNU_all_call_sites   (flag_present) Yes
             sibling              (ref4) [    b1]
 [    71]      lexical_block
               low_pc               (addr) +0x0000000000000690 <_Z3foov>
               high_pc              (data8) 2 (+0x0000000000000692)
 [    82]        variable
                 name                 (string) "bar"
                 decl_file            (data1) 1
                 decl_line            (data1) 11
                 type                 (ref4) [    b2]
                 location             (exprloc) 
                  [   0] addr +0x680 <_Z6foobarv>
                  [   9] stack_value
 [    98]        inlined_subroutine
                 abstract_origin      (ref4) [    2d]
                 low_pc               (addr) +0x0000000000000690 <_Z3foov>
                 high_pc              (data8) 2 (+0x0000000000000692)
                 call_file            (data1) 1
                 call_line            (data1) 12
 [    b1]    subroutine_type
 [    b2]    pointer_type
             byte_size            (data1) 8
             type                 (ref4) [    b1]

DWARF section [26] '.debug_abbrev' at offset 0x113d:
 [ Code]

Abbreviation section at offset 0:
 [    1] offset: 0, children: yes, tag: compile_unit
          attr: producer, form: strp, offset: 0
          attr: language, form: data1, offset: 0x2
          attr: name, form: strp, offset: 0x4
          attr: comp_dir, form: strp, offset: 0x6
          attr: low_pc, form: addr, offset: 0x8
          attr: high_pc, form: data8, offset: 0xa
          attr: stmt_list, form: sec_offset, offset: 0xc
 [    2] offset: 19, children: no, tag: subprogram
          attr: external, form: flag_present, offset: 0x13
          attr: name, form: strp, offset: 0x15
          attr: decl_file, form: data1, offset: 0x17
          attr: decl_line, form: data1, offset: 0x19
          attr: inline, form: data1, offset: 0x1b
 [    3] offset: 34, children: no, tag: subprogram
          attr: abstract_origin, form: ref4, offset: 0x22
          attr: linkage_name, form: strp, offset: 0x24
          attr: low_pc, form: addr, offset: 0x26
          attr: high_pc, form: data8, offset: 0x28
          attr: frame_base, form: exprloc, offset: 0x2a
          attr: GNU_all_call_sites, form: flag_present, offset: 0x2c
 [    4] offset: 52, children: yes, tag: subprogram
          attr: external, form: flag_present, offset: 0x34
          attr: name, form: string, offset: 0x36
          attr: decl_file, form: data1, offset: 0x38
          attr: decl_line, form: data1, offset: 0x3a
          attr: linkage_name, form: strp, offset: 0x3c
          attr: low_pc, form: addr, offset: 0x3e
          attr: high_pc, form: data8, offset: 0x40
          attr: frame_base, form: exprloc, offset: 0x42
          attr: GNU_all_call_sites, form: flag_present, offset: 0x44
          attr: sibling, form: ref4, offset: 0x47
 [    5] offset: 78, children: yes, tag: lexical_block
          attr: low_pc, form: addr, offset: 0x4e
          attr: high_pc, form: data8, offset: 0x50
 [    6] offset: 87, children: no, tag: variable
          attr: name, form: string, offset: 0x57
          attr: decl_file, form: data1, offset: 0x59
          attr: decl_line, form: data1, offset: 0x5b
          attr: type, form: ref4, offset: 0x5d
          attr: location, form: exprloc, offset: 0x5f
 [    7] offset: 102, children: no, tag: inlined_subroutine
          attr: abstract_origin, form: ref4, offset: 0x66
          attr: low_pc, form: addr, offset: 0x68
          attr: high_pc, form: data8, offset: 0x6a
          attr: call_file, form: data1, offset: 0x6c
          attr: call_line, form: data1, offset: 0x6e
 [    8] offset: 117, children: no, tag: subroutine_type
 [    9] offset: 122, children: no, tag: pointer_type
          attr: byte_size, form: data1, offset: 0x7a
          attr: type, form: ref4, offset: 0x7c

DWARF section [27] '.debug_line' at offset 0x11c1:

Table at offset 0:

 Length:                     54
 DWARF version:              2
 Prologue length:            28
 Minimum instruction length: 1
 Maximum operations per instruction: 1
 Initial value if 'is_stmt': 1
 Line base:                  -5
 Line range:                 14
 Opcode base:                13

Opcodes:
  [ 1]  0 arguments
  [ 2]  1 argument
  [ 3]  1 argument
  [ 4]  1 argument
  [ 5]  1 argument
  [ 6]  0 arguments
  [ 7]  0 arguments
  [ 8]  0 arguments
  [ 9]  1 argument
  [10]  0 arguments
  [11]  0 arguments
  [12]  1 argument

Directory table:

File name table:
 Entry Dir   Time      Size      Name
 1     0     0         0         x.cpp

Line number statements:
 [    26] extended opcode 2:  set address to +0x680 <_Z6foobarv>
 [    31] special opcode 21: address+0 = +0x680 <_Z6foobarv>, line+3 = 4
 [    32] special opcode 19: address+0 = +0x680 <_Z6foobarv>, line+1 = 5
 [    33] special opcode 246: address+16 = +0x690 <_Z3foov>, line+4 = 9
 [    34] special opcode 14: address+0 = +0x690 <_Z3foov>, line-4 = 5
 [    35] advance address by 2 to +0x692
 [    37] extended opcode 1:  end of sequence

DWARF section [28] '.debug_str' at offset 0x11fb:
 Offset  String
 [   0]  "foobar"
 [   7]  "GNU C++ 4.8.3 20140911 (Red Hat 4.8.3-7) -mtune=generic -march=x86-64 -g -O3 -fPIC -fvisibility=hidden"
 [  6e]  "/tmp"
 [  73]  "_Z6foobarv"
 [  7e]  "_Z3foov"
 [  86]  "x.cpp"

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] fixing addr2line inline info
@ 2014-11-24  9:08 Mark Wielaard
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2014-11-24  9:08 UTC (permalink / raw)
  To: elfutils-devel

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

On Fri, 2014-11-21 at 14:31 -0800, Josh Stone wrote:
> >> 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. :)

That does make sense. If you want to set a breakpoint on either line and
only have the line info then you would want to set it at the same
address.

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

The glibc manual documents some tricks to turn qsort into a stable sort:
http://www.gnu.org/software/libc/manual/html_node/Array-Sort-Function.html
But it might be better to switch to some kind of merge sort to get a
stable sort function?

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

Note that there is already a hidden/unsupported -D option to eu-readelf.
It is just missing the hook to add the actual demangling.

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

Yes. Now I see. I wouldn't call it complete redundant though. If fixed
it would be a different representation of inlined backtraces. But given
that you cannot really select one without the other, it effectively is
redundant indeed. Maybe -f -i should show one and "plain" -i should show
the other. Or is that too confusing? Maybe it doesn't really add
anything.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] fixing addr2line inline info
@ 2014-11-21 22:31 Josh Stone
  0 siblings, 0 replies; 4+ messages in thread
From: Josh Stone @ 2014-11-21 22:31 UTC (permalink / raw)
  To: elfutils-devel

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

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] fixing addr2line inline info
@ 2014-11-21 15:52 Mark Wielaard
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2014-11-21 15:52 UTC (permalink / raw)
  To: elfutils-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-11-24  9:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-19  2:04 [RFC] fixing addr2line inline info Josh Stone
2014-11-21 15:52 Mark Wielaard
2014-11-21 22:31 Josh Stone
2014-11-24  9:08 Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).