public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: elfutils-devel@sourceware.org
Cc: Ulf Hermann <ulf.hermann@qt.io>
Subject: Re: [PATCH 5/5] Add frame pointer unwinding for aarch64
Date: Tue, 25 Apr 2017 22:13:00 -0000	[thread overview]
Message-ID: <1493125881.31726.44.camel@klomp.org> (raw)
In-Reply-To: <1493124579-21017-5-git-send-email-mark@klomp.org>

On Tue, 2017-04-25 at 14:49 +0200, Mark Wielaard wrote:
> +bool
> +EBLHOOK(unwind) (Ebl *ebl __attribute__ ((unused)), Dwarf_Addr pc __attribute__ ((unused)),
> +                 ebl_tid_registers_t *setfunc, ebl_tid_registers_get_t *getfunc,
> +                 ebl_pid_memory_read_t *readfunc, void *arg,
> +                 bool *signal_framep __attribute__ ((unused)))
> +{
> +  Dwarf_Word fp, lr, sp;
> +
> +  if (!getfunc(LR_REG, 1, &lr, arg))
> +    return false;
> +
> +  if (!getfunc(FP_REG, 1, &fp, arg))
> +    fp = 0;
> +
> +  if (!getfunc(SP_REG, 1, &sp, arg))
> +    sp = 0;
> +
> +  Dwarf_Word newPc, newLr, newFp, newSp;
> +
> +  // The initial frame is special. We are expected to return lr directly in this case, and we'll
> +  // come back to the same frame again in the next round.
> +  if ((pc & 0x1) == 0)
> +    {
> +      newLr = lr;
> +      newFp = fp;
> +      newSp = sp;
> +    }
> +  else
> +    {
> +      if (!readfunc(fp + LR_OFFSET, &newLr, arg))
> +        newLr = 0;
> +
> +      if (!readfunc(fp + FP_OFFSET, &newFp, arg))
> +        newFp = 0;
> +
> +      newSp = fp + SP_OFFSET;
> +    }
> +
> +  newPc = newLr & (~0x1);
> +  if (!setfunc(-1, 1, &newPc, arg))
> +    return false;

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.

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

Lastly could you instead of returning the frame itself with just the pc
adjusted do that directly?

  if ((pc & 0x1) == 0)
    lr = lr & (~0x1);

And then use the code in the else clause always?

All the above might simply be me not understanding the significance of
the initial frame.

Cheers,

Mark

  parent reply	other threads:[~2017-04-25 13:11 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 4/5] Add i386 frame pointer unwinder Mark Wielaard
2017-04-25 13:38                     ` Ulf Hermann
2017-04-25 12:56                   ` [PATCH 1/5] Revert "Optionally allow unknown symbols in the backtrace tests" 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 [this message]
2017-04-25 22:23                       ` Ulf Hermann
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=1493125881.31726.44.camel@klomp.org \
    --to=mark@klomp.org \
    --cc=elfutils-devel@sourceware.org \
    --cc=ulf.hermann@qt.io \
    /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).