Re: [Libunwind-devel] Backtrace performance on x86-64 based on Dwarf info

[Milian Wolff]

On Thursday 19 June 2014 23:50:25 Lassi Tuura wrote:
> Hey,

Hey Lassi, Arun.

I want to revive this thread after my long absence in the hope to get the 
situation improved upstream. Locally, I've been running with the patch of the 
previous mail for some time now and it works very well in my case.

> Thanks for testing that. Note though that as the comment immediately above
> says, x86_64 ABI says the last frame should have null RBP (IIRC, that's
> 3.4.1 Initial Stack and Register State). Can you possibly find out why that
> doesn't hold on your platform?

Do you have any suggestions on how to do that? I.e. what should I look at? 
Specific compile options for libc? You can find the PKGBUILD that is used on 
ArchLinux here:

https://projects.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/glibc

That contains the configure invocation with all its arguments and the patches 
that are applied previously.

> The code I was thinking about is apply_reg_state in src/dwarf/Gparser.c
> where the DWARF_IS_NULL_LOC (c->loc[c->ret_addr_column]) conditions should
> have kicked in to set RIP for the next step to 0, which I vaguely recall
> caused the unwind to terminate at that step. But maybe there was a change
> to allow unwinding to continue with RIP == 0. (Arun do you recall details
> on that? For example
> http://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=commitdiff;h=d04dc94cc
> 2b0141f06ed9de1665ab89a3f549e0b - I vaguely recall we talked about it.)

Any news on that Arun? I'll also try to figure out whats going on and why this 
is not kicking in.

> At any rate it seems there's now some inconsistency about when exactly we
> stop unwinding - some of the code like apply_reg_state is setting RIP == 0
> to indicate we should stop, but on x86_64 we actually depend on
> (ABI-specified) RBP == 0, and some combination with "ret_addr_column is
> nil". We should probably have a more consistent indication of "this is the
> deepest stack frame" :-)

Yeah it seems to be inconsistent. Do you think its a bad idea to get the one-
liner patch in for now? Do you fear it could potentially lead to undesired 
side-effects? Note that I cannot get the tests to run (even without my patch) 
on my machine here:

make check
...
test-static-link-gen.c:57: error: undefined reference to '_Ux86_64_get_reg'
test-static-link-gen.c:65: error: undefined reference to '_Ux86_64_init_local'
test-static-link-gen.c:66: error: undefined reference to 
'_Ux86_64_local_addr_space'
test-static-link-gen.c:66: error: undefined reference to 
'_Ux86_64_init_remote'
test-static-link-gen.c:68: error: undefined reference to '_Ux86_64_step'
test-static-link-loc.c:80: error: undefined reference to '_ULx86_64_get_reg'
test-static-link-loc.c:83: error: undefined reference to 
'_ULx86_64_init_local'
test-static-link-loc.c:84: error: undefined reference to 
'_ULx86_64_local_addr_space'
test-static-link-loc.c:84: error: undefined reference to 
'_ULx86_64_init_remote'
test-static-link-loc.c:85: error: undefined reference to '_ULx86_64_step'
collect2: error: ld returned 1 exit status
...

running some tests manually in the tests folder yields:

./Gtest-dyn1
FAILURE: expected 13, not 1 frames below signal frame
./Gtest-exc
Segmentation fault
./Ltest-dyn1
FAILURE: expected 13, not 1 frames below signal frame
./Ltest-exc
Segmentation fault

Bye

> On Sun, Jun 15, 2014 at 1:05 AM, Milian Wolff <address@hidden> wrote:
> > On Friday 13 June 2014 14:05:45 Lassi Tuura wrote:
> > > Sorry for slow reply - unfortunately I haven't had much chance to look
> > 
> > into
> > 
> > > this in detail.
> > > 
> > > The things that ideally should happen:
> > > 
> > > - Dwarf unwinder should detect the "last" frame in the stack and stop
> > > there. IIRC this was supposed to be based on null return address column
> > 
> > in
> > 
> > > the dwarf info, and there should be an "if" in the generic dwarf parsing
> > 
> > to
> > 
> > > detect this. I don't recall how this was (if at all) recorded through
> > 
> > frame
> > 
> > > stashing and fast trace though, and didn't have time to look into it in
> > > detail.
> > 
> > Aha, that does help indeed. What do you say to this patch:
> > 
> > diff --git a/src/x86_64/Gstep.c b/src/x86_64/Gstep.c
> > index 809d60b..a6dde6a 100644
> > --- a/src/x86_64/Gstep.c
> > +++ b/src/x86_64/Gstep.c
> > @@ -84,7 +84,7 @@ unw_step (unw_cursor_t *cursor)
> > 
> >      {
> >      
> >        /* x86_64 ABI specifies that end of call-chain is marked with a
> >        
> >          NULL RBP.  */
> > 
> > -      if (DWARF_IS_NULL_LOC (c->dwarf.loc[RBP]))
> > +      if (DWARF_IS_NULL_LOC (c->dwarf.loc[RBP]) || DWARF_IS_NULL_LOC(c-
> > 
> > >dwarf.loc[c->dwarf.ret_addr_column]))
> > >
> >         {
> >         
> >           c->dwarf.ip = 0;
> >           ret = 0;
> > 
> > It works very well for me, as far as I can see! What do you say? Can we
> > merge
> > this upstream?
> > 
> > > - The frame stash should a) record the frame, b) remember somehow or
> > > another it's the last frame.
> > 
> > The patch above does that as then unw_step returns 0 which in turn is
> > cached
> > by Gtrace.c.
> > 
> > > - The fast trace should stop when at the end of the frame chain. I am
> > > not
> > > sure but I don't think Arun's suggested check on RBP would be the right
> > > thing to do, but I didn't fully trace how its value be would be tracked
> > > through the multi-condition "if". Maybe it's the right thing, just not
> > 
> > sure.
> > 
> > > The main thing I would look at, using full libunwind debug levels, is
> > > how
> > > the very first pass through the last frame is parsed and handled. First
> > > make sure it is correctly parsed and detected as the last frame in the
> > > chain, and if that's not the case, maybe look into why either the dwarf
> > > frame info is incorrect, or why the heuristics don't correctly detect
> > > the
> > > case. If and only if that detection is correct, figure out why the fast
> > > trace gets it wrong, and falls off the fast path.
> > 
> > I've send this debug information in my previous emails. But I've attached
> > the
> > log again before and after applying the patch listed above.
> > 
> > > We did run into several common enough cases where fast trace wouldn't
> > > detect the last frame correctly, and fell off to the slow trace, which
> > > would just produce the same result - slower. That was really annoying so
> > > you have my full sympathy :-) I tried to fix all the deficiencies we
> > 
> > found,
> > 
> > > but certainly there can be more of them. I was hoping linux system libc
> > > would by now correctly annotate everything with dwarf, maybe it's just a
> > > matter of suitable configuration, compilation or linking flags
> > > somewhere?
> > 
> > I'm running against the libc that is provided by the ArchLinux
> > repositories. I
> > do _not_ want to build this on my own. I hope you understand that. The
> > version
> > is glibc 2.19-5.
> > 
> > Thanks for your feedback!
> > 
> > --
> > Milian Wolff
> > address@hidden
> > http://milianw.de

-- 
Milian Wolff
address@hidden
http://milianw.de