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

[Lassi Tuura]

Hey,

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?

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=d04dc94cc2b0141f06ed9de1665ab89a3f549e0b - I vaguely recall we talked about it.)

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" :-)

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