On 29 June 2016 at 04:48, Dave Anderson <anderson@redhat.com> wrote:
>
> > On arm64, the link register (LR) holds a return address, which is the one
> > just after a branch instruction. So using a saved lr as PC for backtracing
> > might cause some confusion.
> > For example, in kernel/entry.S,
> > work_resched:
> > ...
> > bl schedule
> >
> > ret_to_user:
> > ...
> >
> > The current code shows "ret_o_user", instead of "work_resched",
> > as a caller of schedule().
>
> I see...
FYI, this can potentially be seen in the kernel's ftrace-based stack tracer or
panic dump.
In the past, PC calculation was modified to be "LR-4", but this patch
was reverted. See
commit 9702970c7bd3(Revert "ARM64: unwind: Fix PC calculation")
So I said that adding this patch was a discussion topic.
> >
> > This patch corrects a PC by decrementing it by 4.
> > But please note that this change may also make people a bit confused
> > because a value of LR in the stack dump of "bt -f" doesn't match with
> > an address in one-line summary.
> >
> > #2 [ffffcc7511407eb0] schedule at ffff0000d628aee0
> > ffffcc7511407eb0: ffffcc6d22f23080 ffff0000d5b44d6c <= LR
> > ffffcc7511407ec0: ffffcc7511407ed0 0000000000000000
> > #3 [ffffcc7511407ed0] work_resched at ffff0000d5b44d68 <= correcrted PC
>
> That's probably OK -- and at least "bt -F" would clarify it somewhat.
and "bt -Fs"
-Takahiro AKASHI
> Thanks,
> Dave
>
>
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > arm64.c | 23 ++++++++++++++---------
> > 1 file changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/arm64.c b/arm64.c
> > index b74d2c4..bad36aa 100644
> > --- a/arm64.c
> > +++ b/arm64.c
> > @@ -1504,35 +1504,40 @@ static int
> > arm64_print_stackframe_entry(struct bt_info *bt, int level, struct
> > arm64_stackframe *frame, FILE *ofp)
> > {
> > char *name, *name_plus_offset;
> > - ulong symbol_offset;
> > + ulong pc, symbol_offset;
> > struct syment *sp;
> > struct load_module *lm;
> > char buf[BUFSIZE];
> >
> > - name = closest_symbol(frame->pc);
> > + /*
> > + * if pc comes from a saved lr, it actually points to an instruction
> > + * after branch. To avoid any confusion, decrement pc by 4.
> > + * See, for example, "bl schedule" before ret_to_user().
> > + */
> > + pc = frame->pc - 0x4;
> > + name = closest_symbol(pc);
> > name_plus_offset = NULL;
> >
> > if (bt->flags & BT_SYMBOL_OFFSET) {
> > - sp = value_search(frame->pc, &symbol_offset);
> > + sp = value_search(pc, &symbol_offset);
> > if (sp && symbol_offset)
> > - name_plus_offset =
> > - value_to_symstr(frame->pc, buf, bt->radix);
> > + name_plus_offset = value_to_symstr(pc, buf, bt->radix);
> > }
> >
> > fprintf(ofp, "%s#%d [%8lx] %s at %lx", level < 10 ? " " : "", level,
> > frame->fp ? frame->fp : bt->stacktop - USER_EFRAME_OFFSET,
> > - name_plus_offset ? name_plus_offset : name, frame->pc);
> > + name_plus_offset ? name_plus_offset : name, pc);
> >
> > if (BT_REFERENCE_CHECK(bt))
> > - arm64_do_bt_reference_check(bt, frame->pc, name);
> > + arm64_do_bt_reference_check(bt, pc, name);
> >
> > - if (module_symbol(frame->pc, NULL, &lm, NULL, 0))
> > + if (module_symbol(pc, NULL, &lm, NULL, 0))
> > fprintf(ofp, " [%s]", lm->mod_name);
> >
> > fprintf(ofp, "\n");
> >
> > if (bt->flags & BT_LINE_NUMBERS) {
> > - get_line_number(frame->pc, buf, FALSE);
> > + get_line_number(pc, buf, FALSE);
> > if (strlen(buf))
> > fprintf(ofp, " %s\n", buf);
> > }
> > --
> > 2.9.0
> >
> > --
> > Crash-utility mailing list
> > Crash-utility@redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> >
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility