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 <[email protected]> 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
> [email protected]
> http://milianw.de
>
_______________________________________________
Libunwind-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/libunwind-devel

Reply via email to