diff mbox series

[RFC,3/3] arm64: unwind stack on exception

Message ID 20240710-arm64-backtrace-v1-3-5a2ba50485dd@linaro.org
State New
Headers show
Series ARM64: add symbol name lookup and print a backtrace on exception | expand

Commit Message

Caleb Connolly July 10, 2024, 4:26 p.m. UTC
We already build arm64 images with frame pointers. Let's finally make
use of them in tandem with the new symbol lookup support by unwinding
the stack when an exception occurs, producing a backtrace similar to
those emitted by Linux.

In addition, introduce a dedicated unwind_stack() function which can be
called from anywhere to print a backtrace.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 arch/arm/include/asm/ptrace.h |  4 +++
 arch/arm/lib/interrupts_64.c  | 76 +++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig                   |  6 ++--
 3 files changed, 84 insertions(+), 2 deletions(-)

Comments

Tom Rini July 12, 2024, 4:48 p.m. UTC | #1
On Wed, Jul 10, 2024 at 06:26:20PM +0200, Caleb Connolly wrote:

> We already build arm64 images with frame pointers. Let's finally make
> use of them in tandem with the new symbol lookup support by unwinding
> the stack when an exception occurs, producing a backtrace similar to
> those emitted by Linux.
> 
> In addition, introduce a dedicated unwind_stack() function which can be
> called from anywhere to print a backtrace.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  arch/arm/include/asm/ptrace.h |  4 +++
>  arch/arm/lib/interrupts_64.c  | 76 +++++++++++++++++++++++++++++++++++++++++++
>  lib/Kconfig                   |  6 ++--
>  3 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
> index a836f6cc60db..f42cfb33e918 100644
> --- a/arch/arm/include/asm/ptrace.h
> +++ b/arch/arm/include/asm/ptrace.h
> @@ -22,8 +22,12 @@
>  
>  #ifdef __KERNEL__
>  extern void show_regs(struct pt_regs *);
>  
> +#if CONFIG_IS_ENABLED(SYMBOL_LOOKUP)
> +void unwind_stack(void);
> +#endif

No guarding prototypes.

> diff --git a/lib/Kconfig b/lib/Kconfig
> index 06a78f83b7d6..092b780aeeb8 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -979,11 +979,13 @@ config VPL_OF_LIBFDT_ASSUME_MASK
>  	  unsafe execution. See FDT_ASSUME_PERFECT, etc. in libfdt_internal.h
>  
>  config SYMBOL_LOOKUP
>  	bool "Enable symbol lookup"
> +	default n

This is already the default.

>  	help
> -	  This enables support for looking up symbol names from addresses. The
> -	  primary usecase for this is improved debugging support.
> +	  This enables support for looking up symbol names from addresses. When
> +	  enabled U-Boot will print stack traces with function names when an
> +	  exception occurs.

Please just word this as you want in the previous patch.
diff mbox series

Patch

diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
index a836f6cc60db..f42cfb33e918 100644
--- a/arch/arm/include/asm/ptrace.h
+++ b/arch/arm/include/asm/ptrace.h
@@ -22,8 +22,12 @@ 
 
 #ifdef __KERNEL__
 extern void show_regs(struct pt_regs *);
 
+#if CONFIG_IS_ENABLED(SYMBOL_LOOKUP)
+void unwind_stack(void);
+#endif
+
 #define predicate(x)	(x & 0xf0000000)
 #define PREDICATE_ALWAYS	0xe0000000
 
 #endif
diff --git a/arch/arm/lib/interrupts_64.c b/arch/arm/lib/interrupts_64.c
index b3024ba514ec..24edeb0de51e 100644
--- a/arch/arm/lib/interrupts_64.c
+++ b/arch/arm/lib/interrupts_64.c
@@ -3,15 +3,17 @@ 
  * (C) Copyright 2013
  * David Feng <fenghua@phytium.com.cn>
  */
 
+#include <asm-generic/sections.h>
 #include <asm/esr.h>
 #include <asm/global_data.h>
 #include <asm/ptrace.h>
 #include <irq_func.h>
 #include <linux/compiler.h>
 #include <efi_loader.h>
 #include <semihosting.h>
+#include <symbols.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 int interrupt_init(void)
@@ -80,8 +82,77 @@  static void dump_instr(struct pt_regs *regs)
 		printf(i == 0 ? "(%08x) " : "%08x ", addr[i]);
 	printf("\n");
 }
 
+#if IS_ENABLED(CONFIG_SYMBOL_LOOKUP)
+
+static void print_sym(unsigned long symaddr, const char *symname, unsigned long offset)
+{
+	if (symaddr > (unsigned long)__bss_end)
+		printf("\t<%#016lx> ???\n", symaddr);
+	else
+		printf("\t<%#016lx> %s+%#lx\n", symaddr, symname, offset);
+}
+
+/* Stack frames automatically generated by compiler emitted code */
+struct stack_frame {
+	struct stack_frame *prev_ptr; /* Alays a bigger address on ARM64 */
+	unsigned long lr;
+	char data[];
+};
+
+static void __unwind_stack(unsigned long lr, unsigned long x29)
+{
+	char symname[KSYM_NAME_LEN+1] = { 0 };
+	unsigned long addr, offset;
+	struct stack_frame *fp, *last_fp;
+
+	/*
+	 * Either the place where unwind_stack() was called, or the
+	 * instruction that caused an exception
+	 */
+	symbols_lookup(lr, &addr, &offset, symname);
+	print_sym(addr, symname, offset);
+
+	fp = (struct stack_frame *)x29;
+	while (fp && fp->prev_ptr > fp) {
+		symbols_lookup(fp->lr, &addr, &offset, symname);
+		/*
+		 * _start is used over gd->relocaddr because it will be correct
+		 * if U-Boot was built as relocatable and loaded at an arbitrary
+		 * address (both pre and post-relocation).
+		 */
+		print_sym(addr + (unsigned long)_start, symname, offset);
+		last_fp = fp;
+		fp = (struct stack_frame *)(u64)fp->prev_ptr;
+	}
+}
+
+/**
+ * unwind_stack() - Unwind the callstack and print a backtrace.
+ *
+ * This function can be called for debugging purposes to walk backwards through
+ * stack frames, printing the function name and offset of the branch instruction.
+ *
+ * It reads out the link register (x30) which contains the return address of the
+ * caller, and x29 which contains the frame pointer of the caller.
+ */
+void unwind_stack(void)
+{
+	unsigned long x29, x30;
+
+	asm("mov %0, x29" : "=r" (x29));
+	asm("mov %0, x30" : "=r" (x30));
+
+	__unwind_stack(x30, x29);
+}
+
+#else
+
+#define __unwind_stack(lr, x29) do {} while (0)
+
+#endif
+
 void show_regs(struct pt_regs *regs)
 {
 	int i;
 
@@ -95,8 +166,13 @@  void show_regs(struct pt_regs *regs)
 		printf("x%-2d: %016lx x%-2d: %016lx\n",
 		       i, regs->regs[i], i+1, regs->regs[i+1]);
 	printf("\n");
 	dump_instr(regs);
+
+	if (IS_ENABLED(CONFIG_SYMBOL_LOOKUP)) {
+		printf("\nBacktrace:\n");
+		__unwind_stack(regs->elr, regs->regs[29]);
+	}
 }
 
 /*
  * Try to "emulate" a semihosting call in the event that we don't have a
diff --git a/lib/Kconfig b/lib/Kconfig
index 06a78f83b7d6..092b780aeeb8 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -979,11 +979,13 @@  config VPL_OF_LIBFDT_ASSUME_MASK
 	  unsafe execution. See FDT_ASSUME_PERFECT, etc. in libfdt_internal.h
 
 config SYMBOL_LOOKUP
 	bool "Enable symbol lookup"
+	default n
 	help
-	  This enables support for looking up symbol names from addresses. The
-	  primary usecase for this is improved debugging support.
+	  This enables support for looking up symbol names from addresses. When
+	  enabled U-Boot will print stack traces with function names when an
+	  exception occurs.
 
 menu "System tables"
 	depends on (!EFI && !SYS_COREBOOT) || (ARM && EFI_LOADER)