Re: [RFC] fixing addr2line inline info

[Mark Wielaard <mjw@redhat.com>]

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