On Tue, Apr 19, 2016 at 08:41:02AM -0700, Peter Feiner wrote:
> On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@xxxxxxxxxx> wrote:
> > This actually fixes the arm build too, as arm's __builtin_return_address
> > doesn't allow any input other than zero, and thus the common backtrace()
> > wasn't compiling.
> >
> > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> > ---
> > arm/Makefile.arm | 4 ++++
> > lib/arm/asm/stack.h | 3 +++
> > lib/arm/processor.c | 1 +
> > lib/arm/stack.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 49 insertions(+)
> > create mode 100644 lib/arm/stack.c
> >
> > diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> > index 946422872532d..92f375700c45e 100644
> > --- a/arm/Makefile.arm
> > +++ b/arm/Makefile.arm
> > @@ -8,12 +8,16 @@ ldarch = elf32-littlearm
> > kernel_offset = 0x10000
> > machine = -marm
> >
> > +# stack.o relies on frame pointers.
> > +KEEP_FRAME_POINTER := y
> > +
> > CFLAGS += $(machine)
> > CFLAGS += -mcpu=$(PROCESSOR)
> >
> > cstart.o = $(TEST_DIR)/cstart.o
> > cflatobjs += lib/arm/spinlock.o
> > cflatobjs += lib/arm/processor.o
> > +cflatobjs += lib/arm/stack.o
> >
> > # arm specific tests
> > tests =
> > diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
> > index fe97ac5d4b7d9..ebcedc2ef938c 100644
> > --- a/lib/arm/asm/stack.h
> > +++ b/lib/arm/asm/stack.h
> > @@ -5,4 +5,7 @@
> > #error Do not directly include <asm/stack.h>. Just use <stack.h>.
> > #endif
> >
> > +#define HAVE_ARCH_BACKTRACE_FRAME
> > +#define HAVE_ARCH_BACKTRACE
> > +
> > #endif
> > diff --git a/lib/arm/processor.c b/lib/arm/processor.c
> > index 1cef46ab28647..54fdb87ef0196 100644
> > --- a/lib/arm/processor.c
> > +++ b/lib/arm/processor.c
> > @@ -108,6 +108,7 @@ void do_handle_exception(enum vector v, struct pt_regs *regs)
> > asm volatile("mrc p15, 0, %0, c5, c0, 1": "=r" (fsr));
> > printf("IFAR: %08lx IFSR: %08lx\n", far, fsr);
> > }
> > + dump_frame_stack((void *)regs->ARM_pc, (void *)regs->ARM_fp);
> > abort();
> > }
> >
> > diff --git a/lib/arm/stack.c b/lib/arm/stack.c
> > new file mode 100644
> > index 0000000000000..f58b731f277bf
> > --- /dev/null
> > +++ b/lib/arm/stack.c
> > @@ -0,0 +1,41 @@
> > +/*
> > + * backtrace support (this is a modified lib/x86/stack.c)
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +#include <stack.h>
> > +
> > +int backtrace_frame(const void *frame, const void **return_addrs,
> > + int max_depth)
> > +{
> > + static int walking;
> > + int depth = 0;
>
> Nit: unnecessary initialization.
/me catches the nit and flings it over to lib/x86/stack.c :-)
I'll fix for v2.
>
> > + const unsigned long *fp = (unsigned long *)frame;
> > +
> > + if (walking) {
> > + printf("RECURSIVE STACK WALK!!!\n");
> > + return 0;
> > + }
> > + walking = 1;
> > +
> > + for (depth = 0; depth < max_depth; depth++) {
> > + if (!fp)
>
> Interesting difference in termination case here from x86. Instead of
> checking for a null return address, you check for a null frame
> address. Any particular reason why?
When I configured a unit test that had more than one vcpu, then
if I dereferenced a null frame, address zero, I actually got
non-zero junk. Then fp was getting updated to fffffffc, and when
I dereferenced that (an unmapped address) it led to an exception.
So I just threw this test in to make sure that can't happen. It'll
also keep me safe in case regs->ARM_fp is ever null in
do_handle_exception.
>
> > + break;
> > + return_addrs[depth] = (void *)fp[0];
> > + if (return_addrs[depth] == 0)
> > + break;
> > + fp = (unsigned long *)fp[-1];
> > + }
> > +
> > + walking = 0;
> > + return depth;
> > +}
> > +
> > +int backtrace(const void **return_addrs, int max_depth)
> > +{
> > + return backtrace_frame(__builtin_frame_address(0),
> > + return_addrs, max_depth);
> > +}
> > --
> > 2.4.11
> >
>
> Nice fix!
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
