[PATCH 3/5] arm64: implement printing backtraces

[Casey Connolly]

On 03/07/2025 07:19, Heinrich Schuchardt wrote:
> Add a function to print frame pointers depending on symbols
> CONFIG_(SPL)_FRAMEPOINTER.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  arch/arm/Makefile            |  9 ++++++-
>  arch/arm/lib/interrupts_64.c | 46 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 5ecadb2ef1b..41e75b5bcb1 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -33,6 +33,13 @@ endif
>  # Evaluate arch cc-option calls now
>  arch-y := $(arch-y)
>  
> +ifeq ($(CONFIG_$(PHASE_)FRAMEPOINTER),y)
> +	ARCH_FLAGS += -fno-omit-frame-pointer

from my testing, current arm64 builds of U-Boot will make use of a frame
pointer, it seems to be the default.

So if CONFIG_FRAMEPOINTER is disabled we should explicitly add the
-fomit-frame-pointer flag I think? Or we ignore CONFIG_FRAMEPOINTER for
arm64 and accept that we always have a framepointer (I'm not sure how
significant the space saving really is here).

> +endif
> +
> +PLATFORM_CPPFLAGS	+= $(ARCH_FLAGS)
> +CFLAGS_EFI		+= $(ARCH_FLAGS)
> +
>  # This selects how we optimise for the processor.
>  tune-$(CONFIG_CPU_ARM720T)	=-mtune=arm7tdmi
>  tune-$(CONFIG_CPU_ARM920T)	=
> @@ -47,7 +54,7 @@ tune-$(CONFIG_ARM64)		=
>  # Evaluate tune cc-option calls now
>  tune-y := $(tune-y)
>  
> -PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)
> +PLATFORM_CPPFLAGS += $(ARCH_FLAGS) $(arch-y) $(tune-y)
>  
>  # Machine directory name.  This list is sorted alphanumerically
>  # by CONFIG_* macro name.
> diff --git a/arch/arm/lib/interrupts_64.c b/arch/arm/lib/interrupts_64.c
> index b3024ba514e..970f8b6831f 100644
> --- a/arch/arm/lib/interrupts_64.c
> +++ b/arch/arm/lib/interrupts_64.c
> @@ -98,6 +98,35 @@ void show_regs(struct pt_regs *regs)
>  	dump_instr(regs);
>  }
>  
> +/**
> + * show_backtrace() - show backtrace using frame pointers
> + *
> + * @regs - registers
> + */
> +static void show_backtrace(struct pt_regs *regs)
> +{
> +	void **fp = (void **)regs->regs[29];
> +	unsigned int count = 0;
> +	void *lr;
> +
> +	printf("\nbacktrace:\n");

This should be a capital 'B' as with the other headers.

In my series the output looks like:

Backtrace:
	<0x000001ffdfb344> do_mem_mw+0xbc
	<0x000001ffe0f07c> cmd_process+0x130
	<0x000001ffe06440> run_list_real+0x6d0
	<0x000001ffe065cc> parse_stream_outer+0x14c
	<0x000001ffe06b10> parse_file_outer+0x34
	<0x000001ffe0e3a4> cli_loop+0x18
	<0x000001ffe03b9c> main_loop+0x74
	<0x000001ffe07984> board_init_r+0x3c8

The aim was to match how the kernel backtraces look. I think it makes
sense to do that here too.

> +
> +	while (fp) {
> +		lr = fp[1];
> +		printf("%3d: fp: %p lr: %p",
> +		       count, fp, lr);

The address of the framepointer isn't so interesting, neither is the
index imo. I think the useful info here is the address of the function
and the offset from that to lr.

Printing the lr pre and post relocation does maybe make sense though, to
be useful for interactive debugging or analyzing an objdump.

Kind regards,

> +
> +		if (gd && gd->flags & GD_FLG_RELOC)
> +			printf(" - %p (reloc)\n", lr - gd->reloc_off);
> +		else
> +			printf("\n");
> +
> +		fp = fp[0];
> +		++count;
> +	}
> +	printf("\n");
> +}
> +
>  /*
>   * Try to "emulate" a semihosting call in the event that we don't have a
>   * debugger attached.
> @@ -149,6 +179,8 @@ void do_bad_sync(struct pt_regs *pt_regs)
>  	printf("Bad mode in \"Synchronous Abort\" handler, esr 0x%08lx\n",
>  	       pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -161,6 +193,8 @@ void do_bad_irq(struct pt_regs *pt_regs)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Irq\" handler, esr 0x%08lx\n", pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -173,6 +207,8 @@ void do_bad_fiq(struct pt_regs *pt_regs)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Fiq\" handler, esr 0x%08lx\n", pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -185,6 +221,8 @@ void do_bad_error(struct pt_regs *pt_regs)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Error\" handler, esr 0x%08lx\n", pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -202,6 +240,8 @@ void do_sync(struct pt_regs *pt_regs)
>  	dump_far(pt_regs->esr);
>  	printf("\n");
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -214,6 +254,8 @@ void do_irq(struct pt_regs *pt_regs)
>  	efi_restore_gd();
>  	printf("\"Irq\" handler, esr 0x%08lx\n", pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -226,6 +268,8 @@ void do_fiq(struct pt_regs *pt_regs)
>  	efi_restore_gd();
>  	printf("\"Fiq\" handler, esr 0x%08lx\n", pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> @@ -241,6 +285,8 @@ void __weak do_error(struct pt_regs *pt_regs)
>  	efi_restore_gd();
>  	printf("\"Error\" handler, esr 0x%08lx\n", pt_regs->esr);
>  	show_regs(pt_regs);
> +	if (CONFIG_IS_ENABLED(FRAMEPOINTER))
> +		show_backtrace(pt_regs);
>  	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
-- 
// Casey (she/her)