On 7/3/25 14:08, Heinrich Schuchardt wrote:
On 03.07.25 13:39, Casey Connolly wrote: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.schucha...@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-pointerfrom my testing, current arm64 builds of U-Boot will make use of a frame pointer, it seems to be the default.The built-in defaults may depend on the distro providing the compiler and may be different on LLVM and GCC.
I think you missed the second half of my comment vv>
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.okIn 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+0x3c8Please, provide a link to your series.
Already sent in a different mail but just for completeness sake https://lore.kernel.org/u-boot/20240808-arm64-backtrace-v2-0-29665a226...@linaro.org/
Showing function names would be great. Best regards HeinrichThe 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/they)
