public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Ulf Hermann <ulf.hermann@qt.io>
To: Mark Wielaard <mark@klomp.org>, <elfutils-devel@sourceware.org>
Subject: Re: [PATCH 5/5] Add frame pointer unwinding for aarch64
Date: Tue, 25 Apr 2017 22:23:00 -0000	[thread overview]
Message-ID: <3b0d6718-cf17-9ae1-b5f7-8c6413b8d3d2@qt.io> (raw)
In-Reply-To: <1493125881.31726.44.camel@klomp.org>

> My question is about this "initial frame". In our testcase we don't have
> this case since the backtrace starts in a function that has some CFI.
> But I assume you have some tests that rely on this behavior.

Actually the test I provided does exercise this code. The initial __libc_do_syscall() frame does not have CFI. Only raise() has. You can check that by dropping the code for pc & 0x1.

> The first question is how/why the (pc & 0x1) == 0 check works?
> Why is that the correct check?
> 
> Secondly, if it really is the initial (or signal frame) we are after,
> should we pass in into bool *signal_framep argument. Currently we don't

We have this piece of code in __libdwfl_frame_unwind, in frame_unwind.c:

  if (! state->initial_frame && ! state->signal_frame)
      pc--;

AArch64 has a fixed instruction width of 32bit. So, normally the pc is aligned to 4 bytes. Except if we decrement it, then we are guaranteed to have an odd number, which we can then test to see if the frame in question is the initial or a signal frame. Of course it would be nicer to pass this information directly, but the signal_frame parameter is supposed to be an output parameter. After all we do the following after calling ebl_unwind():

  state->unwound->signal_frame = signal_frame;

> Lastly could you instead of returning the frame itself with just the pc
> adjusted do that directly?
> 
>   if ((pc & 0x1) == 0)
>     lr = lr & (~0x1);

If I dig up the first frame after the initial one from the stack, then we drop whatever we initially had in LR. Apparently, on aarch64 PC is always one frame "ahead" of the other registers. To establish that, we have to set PC to the value of LR on the initial frame, without actually unwinding.

br,
Ulf

  reply	other threads:[~2017-04-25 13:38 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-15 23:34 frame unwinding patches Mark Wielaard
2017-02-15 23:34 ` [PATCH 3/3] Add frame pointer unwinding for aarch64 Mark Wielaard
2017-02-15 23:34 ` [PATCH 2/3] Add frame pointer unwinding as fallback on arm Mark Wielaard
2017-02-15 23:34 ` [PATCH 1/3] Add frame pointer unwinding as fallback on x86_64 Mark Wielaard
2017-02-16  9:13 ` frame unwinding patches Ulf Hermann
2017-04-03  9:00 ` Milian Wolff
2017-04-03  9:03   ` Ulf Hermann
2017-04-03 21:14     ` Mark Wielaard
2017-04-07 10:27       ` Mark Wielaard
2017-04-11 10:16         ` Ulf Hermann
2017-04-19 19:48           ` Mark Wielaard
2017-04-20  9:26             ` Ulf Hermann
2017-04-25 12:50               ` Mark Wielaard
2017-04-25 12:54                 ` [PATCH 1/5] Revert "Optionally allow unknown symbols in the backtrace tests" Mark Wielaard
2017-04-25 12:50                   ` [PATCH 2/5] tests: Add core backtracegen chec and regenerate ppc32 backtrace test files Mark Wielaard
2017-04-25 13:04                     ` Ulf Hermann
2017-04-25 12:55                   ` [PATCH 3/5] Add frame pointer unwinding as fallback on x86_64 Mark Wielaard
2017-04-25 13:05                     ` Ulf Hermann
2017-04-25 12:56                   ` [PATCH 1/5] Revert "Optionally allow unknown symbols in the backtrace tests" Ulf Hermann
2017-04-25 12:56                   ` [PATCH 4/5] Add i386 frame pointer unwinder Mark Wielaard
2017-04-25 13:38                     ` Ulf Hermann
2017-04-25 13:11                   ` [PATCH 5/5] Add frame pointer unwinding for aarch64 Mark Wielaard
2017-04-25 21:55                     ` Ulf Hermann
2017-04-25 22:13                     ` Mark Wielaard
2017-04-25 22:23                       ` Ulf Hermann [this message]
2017-04-26 15:24                         ` Mark Wielaard
2017-04-27 14:02                           ` Ulf Hermann
2017-04-27 14:29                             ` Mark Wielaard
2017-04-27 14:35                               ` Ulf Hermann
2017-04-27 15:09                                 ` Mark Wielaard
2017-04-27 15:42                                   ` Ulf Hermann
2017-05-03  8:46                                 ` Mark Wielaard
2017-04-26 15:20                 ` frame unwinding patches Ulf Hermann
2017-04-03 21:23   ` Jan Kratochvil
2017-04-04  7:40     ` Milian Wolff
2017-04-04  7:55       ` Jan Kratochvil
2017-04-04  8:25         ` Ulf Hermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b0d6718-cf17-9ae1-b5f7-8c6413b8d3d2@qt.io \
    --to=ulf.hermann@qt.io \
    --cc=elfutils-devel@sourceware.org \
    --cc=mark@klomp.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).