Message ID | 1457038116-3448-2-git-send-email-pfeiner@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Mar 03, 2016 at 12:48:33PM -0800, Peter Feiner wrote: > Functions to walk stack and print backtrace. The stack's unadorned as > > STACK: [@]addr addr addr ... > > where the optional @ indicates that addr isn't a return address. > > A follow-up patch post-processes the output to pretty-print the stack. > > Frame stack walker is just a stub on arm and ppc. > > Signed-off-by: Peter Feiner <pfeiner@google.com> > --- > Makefile | 3 ++- > arm/Makefile.common | 1 + > lib/arm/asm/stack.h | 1 + > lib/arm64/asm/stack.h | 1 + > lib/asm-generic/stack.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ > lib/asm-generic/stack.h | 14 +++++++++++++ > lib/libcflat.h | 3 +++ > lib/powerpc/asm/stack.h | 1 + > lib/ppc64/asm/stack.h | 1 + > lib/printf.c | 42 +++++++++++++++++++++++++++++++++++++++ > lib/x86/asm/stack.c | 25 +++++++++++++++++++++++ > lib/x86/asm/stack.h | 12 +++++++++++ > powerpc/Makefile.common | 1 + > x86/Makefile.common | 4 ++++ > 14 files changed, 161 insertions(+), 1 deletion(-) > create mode 100644 lib/arm/asm/stack.h > create mode 100644 lib/arm64/asm/stack.h > create mode 100644 lib/asm-generic/stack.c > create mode 100644 lib/asm-generic/stack.h > create mode 100644 lib/powerpc/asm/stack.h > create mode 100644 lib/ppc64/asm/stack.h > create mode 100644 lib/x86/asm/stack.c > create mode 100644 lib/x86/asm/stack.h It's not usual to put C files in the asm directories, at least Linux doesn't do that. So I'd prefer lib/stack.c: the common backtrace() lib/<ARCH>/stack.c: arch specific backtrace* functions I'm tempted to suggest that print_stack() and the dump*stack functions also go in lib/stack.c, instead of lib/printf.c, but then we need some #ifndef backtrace #define backtrace backtrace int backtrace(...) { } #endif ugliness to allow the common backtrace() to be overridden. Although, it might still be better to move those functions out of printf, as I don't feel they really fit there. > > diff --git a/Makefile b/Makefile > index 72e6711..536f2e6 100644 > --- a/Makefile > +++ b/Makefile > @@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > > CFLAGS += -g > CFLAGS += $(autodepend-flags) -Wall > -CFLAGS += $(call cc-option, -fomit-frame-pointer, "") > +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer > +CFLAGS += $(call cc-option, $(frame-pointer-flag), "") > CFLAGS += $(call cc-option, -fno-stack-protector, "") > CFLAGS += $(call cc-option, -fno-stack-protector-all, "") > > diff --git a/arm/Makefile.common b/arm/Makefile.common > index dd3a0ca..ede8851 100644 > --- a/arm/Makefile.common > +++ b/arm/Makefile.common > @@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o > cflatobjs += lib/arm/bitops.o > cflatobjs += lib/arm/psci.o > cflatobjs += lib/arm/smp.o > +cflatobjs += lib/asm-generic/stack.o > > libeabi = lib/arm/libeabi.a > eabiobjs = lib/arm/eabi_compat.o > diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h > new file mode 100644 > index 0000000..f101d03 > --- /dev/null > +++ b/lib/arm/asm/stack.h > @@ -0,0 +1 @@ > +#include <asm-generic/stack.h> > diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h > new file mode 100644 > index 0000000..f101d03 > --- /dev/null > +++ b/lib/arm64/asm/stack.h > @@ -0,0 +1 @@ > +#include <asm-generic/stack.h> > diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c > new file mode 100644 > index 0000000..93219cb > --- /dev/null > +++ b/lib/asm-generic/stack.c > @@ -0,0 +1,53 @@ > +#include <libcflat.h> > +#include <asm-generic/stack.h> nit: including asm-generic/stack.h doesn't appear to be necessary. Actually, if there was something needed from stack.h, then we should probably get it from asm/stack.h, in case the arch wanted to override it. > + > +int backtrace(const void **return_addrs, int max_depth) > +{ > + static int walking; > + int depth = 0; > + void *addr; > + > + if (walking) { > + printf("RECURSIVE STACK WALK!!!\n"); > + return 0; > + } > + walking = 1; > + > + /* __builtin_return_address requires a compile-time constant argument */ > +#define GET_RETURN_ADDRESS(i) \ > + if (max_depth == i) \ > + goto done; \ > + addr = __builtin_return_address(i); \ > + if (!addr) \ > + goto done; \ > + return_addrs[i] = __builtin_extract_return_addr(addr); \ > + depth = i + 1; \ > + > + GET_RETURN_ADDRESS(0) > + GET_RETURN_ADDRESS(1) > + GET_RETURN_ADDRESS(2) > + GET_RETURN_ADDRESS(3) > + GET_RETURN_ADDRESS(4) > + GET_RETURN_ADDRESS(5) > + GET_RETURN_ADDRESS(6) > + GET_RETURN_ADDRESS(7) > + GET_RETURN_ADDRESS(8) > + GET_RETURN_ADDRESS(9) > + GET_RETURN_ADDRESS(10) > + GET_RETURN_ADDRESS(11) > + GET_RETURN_ADDRESS(12) > + GET_RETURN_ADDRESS(13) > + GET_RETURN_ADDRESS(14) > + GET_RETURN_ADDRESS(15) > + GET_RETURN_ADDRESS(16) > + GET_RETURN_ADDRESS(17) > + GET_RETURN_ADDRESS(18) > + GET_RETURN_ADDRESS(19) > + GET_RETURN_ADDRESS(20) > + > +#undef GET_RETURN_ADDRESS > + > +done: > + walking = 0; > + return depth; > +} > diff --git a/lib/asm-generic/stack.h b/lib/asm-generic/stack.h > new file mode 100644 > index 0000000..381fce4 > --- /dev/null > +++ b/lib/asm-generic/stack.h > @@ -0,0 +1,14 @@ > +#ifndef _ASMGENERIC_STACK_H_ > +#define _ASMGENERIC_STACK_H_ > + > +static inline int > +backtrace_frame(const void *frame __attribute__((unused)), > + const void **return_addrs __attribute__((unused)), > + int max_depth __attribute__((unused))) > +{ > + return 0; > +} > + > +int backtrace(const void **return_addrs, int max_depth); > + > +#endif > diff --git a/lib/libcflat.h b/lib/libcflat.h > index b58a8a1..55bddca 100644 > --- a/lib/libcflat.h > +++ b/lib/libcflat.h > @@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...); > extern void report_abort(const char *msg_fmt, ...); > extern int report_summary(void); > > +extern void dump_stack(void); > +extern void dump_frame_stack(const void *instruction, const void *frame); > + > #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0])) > > #define container_of(ptr, type, member) ({ \ > diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h > new file mode 100644 > index 0000000..f101d03 > --- /dev/null > +++ b/lib/powerpc/asm/stack.h > @@ -0,0 +1 @@ > +#include <asm-generic/stack.h> > diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h > new file mode 100644 > index 0000000..f101d03 > --- /dev/null > +++ b/lib/ppc64/asm/stack.h > @@ -0,0 +1 @@ > +#include <asm-generic/stack.h> > diff --git a/lib/printf.c b/lib/printf.c > index 2aec59a..096e175 100644 > --- a/lib/printf.c > +++ b/lib/printf.c > @@ -1,4 +1,5 @@ > #include "libcflat.h" > +#include "asm/stack.h" > > #define BUFSZ 2000 > > @@ -259,3 +260,44 @@ int printf(const char *fmt, ...) > puts(buf); > return r; > } > + > +static void print_stack(const void **return_addrs, int depth, > + bool top_is_return_address) > +{ > + int i = 0; > + > + printf("\tSTACK:"); > + > + /* @addr indicates a non-return address, as expected by the stack > + * pretty printer script. */ > + if (depth > 0 && !top_is_return_address) { > + printf(" @%lx", (unsigned long) return_addrs[0]); > + i++; > + } > + > + for (; i < depth; i++) { > + printf(" %lx", (unsigned long) return_addrs[i]); > + } > + printf("\n"); > +} > + > +#define MAX_DEPTH 10 How about 20, like the common backtrace() supports? > + > +void dump_stack(void) > +{ > + const void *return_addrs[MAX_DEPTH]; > + int depth; > + > + depth = backtrace(return_addrs, MAX_DEPTH); > + print_stack(return_addrs, depth, true); > +} > + > +void dump_frame_stack(const void *instruction, const void *frame) > +{ > + const void *return_addrs[MAX_DEPTH]; > + int depth; > + > + return_addrs[0] = instruction; > + depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1); > + print_stack(return_addrs, depth + 1, false); > +} > diff --git a/lib/x86/asm/stack.c b/lib/x86/asm/stack.c > new file mode 100644 > index 0000000..b30b3b1 > --- /dev/null > +++ b/lib/x86/asm/stack.c > @@ -0,0 +1,25 @@ > +#include <libcflat.h> > +#include <asm/stack.h> > + > +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth) > +{ > + static int walking; > + int depth = 0; > + const unsigned long *bp = (unsigned long *) frame; > + > + if (walking) { > + printf("RECURSIVE STACK WALK!!!\n"); > + return 0; > + } > + walking = 1; > + > + for (depth = 0; depth < max_depth; depth++) { > + return_addrs[depth] = (void *) bp[1]; > + if (return_addrs[depth] == 0) > + break; > + bp = (unsigned long *) bp[0]; > + } > + > + walking = 0; > + return depth; > +} > diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h > new file mode 100644 > index 0000000..c43bb96 > --- /dev/null > +++ b/lib/x86/asm/stack.h > @@ -0,0 +1,12 @@ > +#ifndef _X86ASM_STACK_H_ > +#define _X86ASM_STACK_H_ > + > +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth); > + > +static inline int backtrace(const void **return_addrs, int max_depth) > +{ > + return backtrace_frame(__builtin_frame_address(0), return_addrs, > + max_depth); > +} > + > +#endif > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common > index 424983e..ea8f669 100644 > --- a/powerpc/Makefile.common > +++ b/powerpc/Makefile.common > @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o > cflatobjs += lib/powerpc/hcall.o > cflatobjs += lib/powerpc/setup.o > cflatobjs += lib/powerpc/rtas.o > +cflatobjs += lib/asm-generic/stack.o > > FLATLIBS = $(libcflat) $(LIBFDT_archive) > %.elf: CFLAGS += $(arch_CFLAGS) > diff --git a/x86/Makefile.common b/x86/Makefile.common > index 3a14fea..c4750f0 100644 > --- a/x86/Makefile.common > +++ b/x86/Makefile.common > @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o > cflatobjs += lib/x86/desc.o > cflatobjs += lib/x86/isr.o > cflatobjs += lib/x86/acpi.o > +cflatobjs += lib/x86/asm/stack.o > > $(libcflat): LDFLAGS += -nostdlib > $(libcflat): CFLAGS += -ffreestanding -I lib > @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib > CFLAGS += -m$(bits) > CFLAGS += -O1 > > +# dump_stack.o relies on frame pointers. > +KEEP_FRAME_POINTER := y > + > libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name) > > FLATLIBS = lib/libcflat.a $(libgcc) > -- > 2.7.0.rc3.207.g0ac5344 Other than my desire to keep C out of asm directories, then this looks good to me. Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Makefile b/Makefile index 72e6711..536f2e6 100644 --- a/Makefile +++ b/Makefile @@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ CFLAGS += -g CFLAGS += $(autodepend-flags) -Wall -CFLAGS += $(call cc-option, -fomit-frame-pointer, "") +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer +CFLAGS += $(call cc-option, $(frame-pointer-flag), "") CFLAGS += $(call cc-option, -fno-stack-protector, "") CFLAGS += $(call cc-option, -fno-stack-protector-all, "") diff --git a/arm/Makefile.common b/arm/Makefile.common index dd3a0ca..ede8851 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o cflatobjs += lib/arm/bitops.o cflatobjs += lib/arm/psci.o cflatobjs += lib/arm/smp.o +cflatobjs += lib/asm-generic/stack.o libeabi = lib/arm/libeabi.a eabiobjs = lib/arm/eabi_compat.o diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h new file mode 100644 index 0000000..f101d03 --- /dev/null +++ b/lib/arm/asm/stack.h @@ -0,0 +1 @@ +#include <asm-generic/stack.h> diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h new file mode 100644 index 0000000..f101d03 --- /dev/null +++ b/lib/arm64/asm/stack.h @@ -0,0 +1 @@ +#include <asm-generic/stack.h> diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c new file mode 100644 index 0000000..93219cb --- /dev/null +++ b/lib/asm-generic/stack.c @@ -0,0 +1,53 @@ +#include <libcflat.h> +#include <asm-generic/stack.h> + +int backtrace(const void **return_addrs, int max_depth) +{ + static int walking; + int depth = 0; + void *addr; + + if (walking) { + printf("RECURSIVE STACK WALK!!!\n"); + return 0; + } + walking = 1; + + /* __builtin_return_address requires a compile-time constant argument */ +#define GET_RETURN_ADDRESS(i) \ + if (max_depth == i) \ + goto done; \ + addr = __builtin_return_address(i); \ + if (!addr) \ + goto done; \ + return_addrs[i] = __builtin_extract_return_addr(addr); \ + depth = i + 1; \ + + GET_RETURN_ADDRESS(0) + GET_RETURN_ADDRESS(1) + GET_RETURN_ADDRESS(2) + GET_RETURN_ADDRESS(3) + GET_RETURN_ADDRESS(4) + GET_RETURN_ADDRESS(5) + GET_RETURN_ADDRESS(6) + GET_RETURN_ADDRESS(7) + GET_RETURN_ADDRESS(8) + GET_RETURN_ADDRESS(9) + GET_RETURN_ADDRESS(10) + GET_RETURN_ADDRESS(11) + GET_RETURN_ADDRESS(12) + GET_RETURN_ADDRESS(13) + GET_RETURN_ADDRESS(14) + GET_RETURN_ADDRESS(15) + GET_RETURN_ADDRESS(16) + GET_RETURN_ADDRESS(17) + GET_RETURN_ADDRESS(18) + GET_RETURN_ADDRESS(19) + GET_RETURN_ADDRESS(20) + +#undef GET_RETURN_ADDRESS + +done: + walking = 0; + return depth; +} diff --git a/lib/asm-generic/stack.h b/lib/asm-generic/stack.h new file mode 100644 index 0000000..381fce4 --- /dev/null +++ b/lib/asm-generic/stack.h @@ -0,0 +1,14 @@ +#ifndef _ASMGENERIC_STACK_H_ +#define _ASMGENERIC_STACK_H_ + +static inline int +backtrace_frame(const void *frame __attribute__((unused)), + const void **return_addrs __attribute__((unused)), + int max_depth __attribute__((unused))) +{ + return 0; +} + +int backtrace(const void **return_addrs, int max_depth); + +#endif diff --git a/lib/libcflat.h b/lib/libcflat.h index b58a8a1..55bddca 100644 --- a/lib/libcflat.h +++ b/lib/libcflat.h @@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...); extern void report_abort(const char *msg_fmt, ...); extern int report_summary(void); +extern void dump_stack(void); +extern void dump_frame_stack(const void *instruction, const void *frame); + #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0])) #define container_of(ptr, type, member) ({ \ diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h new file mode 100644 index 0000000..f101d03 --- /dev/null +++ b/lib/powerpc/asm/stack.h @@ -0,0 +1 @@ +#include <asm-generic/stack.h> diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h new file mode 100644 index 0000000..f101d03 --- /dev/null +++ b/lib/ppc64/asm/stack.h @@ -0,0 +1 @@ +#include <asm-generic/stack.h> diff --git a/lib/printf.c b/lib/printf.c index 2aec59a..096e175 100644 --- a/lib/printf.c +++ b/lib/printf.c @@ -1,4 +1,5 @@ #include "libcflat.h" +#include "asm/stack.h" #define BUFSZ 2000 @@ -259,3 +260,44 @@ int printf(const char *fmt, ...) puts(buf); return r; } + +static void print_stack(const void **return_addrs, int depth, + bool top_is_return_address) +{ + int i = 0; + + printf("\tSTACK:"); + + /* @addr indicates a non-return address, as expected by the stack + * pretty printer script. */ + if (depth > 0 && !top_is_return_address) { + printf(" @%lx", (unsigned long) return_addrs[0]); + i++; + } + + for (; i < depth; i++) { + printf(" %lx", (unsigned long) return_addrs[i]); + } + printf("\n"); +} + +#define MAX_DEPTH 10 + +void dump_stack(void) +{ + const void *return_addrs[MAX_DEPTH]; + int depth; + + depth = backtrace(return_addrs, MAX_DEPTH); + print_stack(return_addrs, depth, true); +} + +void dump_frame_stack(const void *instruction, const void *frame) +{ + const void *return_addrs[MAX_DEPTH]; + int depth; + + return_addrs[0] = instruction; + depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1); + print_stack(return_addrs, depth + 1, false); +} diff --git a/lib/x86/asm/stack.c b/lib/x86/asm/stack.c new file mode 100644 index 0000000..b30b3b1 --- /dev/null +++ b/lib/x86/asm/stack.c @@ -0,0 +1,25 @@ +#include <libcflat.h> +#include <asm/stack.h> + +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth) +{ + static int walking; + int depth = 0; + const unsigned long *bp = (unsigned long *) frame; + + if (walking) { + printf("RECURSIVE STACK WALK!!!\n"); + return 0; + } + walking = 1; + + for (depth = 0; depth < max_depth; depth++) { + return_addrs[depth] = (void *) bp[1]; + if (return_addrs[depth] == 0) + break; + bp = (unsigned long *) bp[0]; + } + + walking = 0; + return depth; +} diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h new file mode 100644 index 0000000..c43bb96 --- /dev/null +++ b/lib/x86/asm/stack.h @@ -0,0 +1,12 @@ +#ifndef _X86ASM_STACK_H_ +#define _X86ASM_STACK_H_ + +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth); + +static inline int backtrace(const void **return_addrs, int max_depth) +{ + return backtrace_frame(__builtin_frame_address(0), return_addrs, + max_depth); +} + +#endif diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common index 424983e..ea8f669 100644 --- a/powerpc/Makefile.common +++ b/powerpc/Makefile.common @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o cflatobjs += lib/powerpc/hcall.o cflatobjs += lib/powerpc/setup.o cflatobjs += lib/powerpc/rtas.o +cflatobjs += lib/asm-generic/stack.o FLATLIBS = $(libcflat) $(LIBFDT_archive) %.elf: CFLAGS += $(arch_CFLAGS) diff --git a/x86/Makefile.common b/x86/Makefile.common index 3a14fea..c4750f0 100644 --- a/x86/Makefile.common +++ b/x86/Makefile.common @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o cflatobjs += lib/x86/desc.o cflatobjs += lib/x86/isr.o cflatobjs += lib/x86/acpi.o +cflatobjs += lib/x86/asm/stack.o $(libcflat): LDFLAGS += -nostdlib $(libcflat): CFLAGS += -ffreestanding -I lib @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib CFLAGS += -m$(bits) CFLAGS += -O1 +# dump_stack.o relies on frame pointers. +KEEP_FRAME_POINTER := y + libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name) FLATLIBS = lib/libcflat.a $(libgcc)
Functions to walk stack and print backtrace. The stack's unadorned as STACK: [@]addr addr addr ... where the optional @ indicates that addr isn't a return address. A follow-up patch post-processes the output to pretty-print the stack. Frame stack walker is just a stub on arm and ppc. Signed-off-by: Peter Feiner <pfeiner@google.com> --- Makefile | 3 ++- arm/Makefile.common | 1 + lib/arm/asm/stack.h | 1 + lib/arm64/asm/stack.h | 1 + lib/asm-generic/stack.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/asm-generic/stack.h | 14 +++++++++++++ lib/libcflat.h | 3 +++ lib/powerpc/asm/stack.h | 1 + lib/ppc64/asm/stack.h | 1 + lib/printf.c | 42 +++++++++++++++++++++++++++++++++++++++ lib/x86/asm/stack.c | 25 +++++++++++++++++++++++ lib/x86/asm/stack.h | 12 +++++++++++ powerpc/Makefile.common | 1 + x86/Makefile.common | 4 ++++ 14 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 lib/arm/asm/stack.h create mode 100644 lib/arm64/asm/stack.h create mode 100644 lib/asm-generic/stack.c create mode 100644 lib/asm-generic/stack.h create mode 100644 lib/powerpc/asm/stack.h create mode 100644 lib/ppc64/asm/stack.h create mode 100644 lib/x86/asm/stack.c create mode 100644 lib/x86/asm/stack.h