Fix PICO_PANIC_FUNCTION= imelementations

- stack should remain properly aligned
- risc-v was actually completely broken for user defined handlers because GCC was auto-pushing vaargs for naked function even with no stack frame
- new implementation supports full call stacks in gdb for Arm or RISC-V when hitting breakpoint in panic function
- new config defines make it explicit that gdb full stgack trace comes at the cost of corrupting stack based vaargs 4th is corrupted on Arm, later on RiSC-V (7 maybe?)

Author: kilograham

src/rp2_common/pico_platform_panic/BUILD.bazel

@@ -20,7 +20,7 @@ cc_library(
 
 cc_library(
     name = "pico_platform_panic",
-    srcs = ["panic.c"],
+    srcs = ["panic.c", "custom_panic_function.S"],
     hdrs = ["include/pico/platform/panic.h"],
     includes = ["include"],
     target_compatible_with = compatible_with_rp2(),

src/rp2_common/pico_platform_panic/CMakeLists.txt

@@ -3,6 +3,7 @@ if (NOT TARGET pico_platform_panic)
 
     target_sources(pico_platform_panic INTERFACE
             ${CMAKE_CURRENT_LIST_DIR}/panic.c
+            ${CMAKE_CURRENT_LIST_DIR}/custom_panic_function.S
     )
 
     target_include_directories(pico_platform_panic_headers SYSTEM INTERFACE ${CMAKE_CURRENT_LIST_DIR}/include)

src/rp2_common/pico_platform_panic/custom_panic_function.S

@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2026 Raspberry Pi (Trading) Ltd.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include "pico/asm_helper.S"
+
+#ifdef PICO_PANIC_FUNCTION
+#define	__CONCAT1(x,y)	x ## y
+#define	__CONCAT(x,y)	__CONCAT1(x,y)
+
+#define PICO_PANIC_FUNCTION_IS_EMPTY (__CONCAT(PICO_PANIC_FUNCTION, 1))
+.text
+// Use a forwarding method here as it is a little simpler than renaming the symbol as it is used from assembler
+.weak panic // also allow override
+.type panic,%function
+panic:
+#ifdef __riscv
+    // we seem to need to help gdb out with call frame on RISC-V
+    .cfi_sections .debug_frame
+    .cfi_startproc
+    #if PICO_PANIC_FUNCTION_IS_EMPTY
+        1:  ebreak
+            j 1b
+    #elif PICO_PANIC_FUNCTION_DOES_NOT_RETURN
+        // if you get an undefined reference here, you didn't define your PICO_PANIC_FUNCTION!
+        jal zero, PICO_PANIC_FUNCTION
+    #else
+        #if PICO_PANIC_FUNCTION_WITH_CALL_STACK
+            #if PICO_PANIC_FUNCTION_WITH_ALL_VAARGS
+            #error PICO_PANIC_FUNCTION_WITH_CALL_STACK & PICO_PANIC_FUNCTION_WITH_ALL_VAARGS are incompatible when PICO_PANIC_FUNCTION_DOES_NOT_RETURN is 0
+            #endif
+            // .insn 0xb842      // cm.push {ra}, -16: Save ultimate return address
+            // use explicit for better compatibility with gdb stack unwinding
+            addi sp,sp,-16
+            .cfi_def_cfa_offset 16
+            sw ra,12(sp)
+            .cfi_offset ra, -4
+        #endif
+        // if you get an undefined reference here, you didn't define your PICO_PANIC_FUNCTION!
+        jal PICO_PANIC_FUNCTION
+        #if !PICO_PANIC_FUNCTION_DOES_NOT_RETURN
+        1:  ebreak
+            j 1b
+       #endif
+   #endif
+    .cfi_endproc
+#else
+    #if PICO_PANIC_FUNCTION_IS_EMPTY
+        1:  bkpt #0
+            b 1b
+    #elif PICO_PANIC_FUNCTION_DOES_NOT_RETURN
+        // if you get an undefined reference here, you didn't define your PICO_PANIC_FUNCTION!
+        ldr r4, =PICO_PANIC_FUNCTION
+        bx r4
+    #else
+        #if PICO_PANIC_FUNCTION_WITH_CALL_STACK
+            #if PICO_PANIC_FUNCTION_WITH_ALL_VAARGS
+            #error PICO_PANIC_FUNCTION_WITH_CALL_STACK & PICO_PANIC_FUNCTION_WITH_ALL_VAARGS are incompatible when PICO_PANIC_FUNCTION_DOES_NOT_RETURN is 0
+            #endif
+            push {r4, lr} // must keep 8 byte alignment for call to user function
+        #endif
+        // if you get an undefined reference here, you didn't define your PICO_PANIC_FUNCTION!
+        bl PICO_PANIC_FUNCTION
+        #if !PICO_PANIC_FUNCTION_DOES_NOT_RETURN
+        1:  bkpt #0
+            b 1b
+        #endif
+    #endif
+#endif
+
+#endif // PICO_PANIC_FUNCTION
+
+

src/rp2_common/pico_platform_panic/include/pico/platform/panic.h

@@ -11,8 +11,20 @@
 extern "C" {
 #endif
 
-#ifndef __ASSEMBLER__
+// PICO_CONFIG: PICO_PANIC_FUNCTION, Name of a function to use in place of the stock panic function or empty string to simply breakpoint on panic, group=pico_runtime
+
+// PICO_CONFIG: PICO_PANIC_FUNCTION_WITH_CALL_STACK, Ensure the call stack is available when using a custom panic function via PICO_PANIC_FUNCTION=. When set to 1 it conflicts with PICO_PANIC_FUNCTION_WITH_ALL_VAARGS=1 as a stack fram is pushed onto the stack corrupting later vaargs. Note this defaults to 1 as most custom panic functions don't actually use the arguments and the call stack seems more useful in general, default=1group=pico_runtime
+#ifndef PICO_PANIC_FUNCTION_WITH_CALL_STACK
+#define PICO_PANIC_FUNCTION_WITH_CALL_STACK 1
+#endif
 
+// PICO_CONFIG: PICO_PANIC_FUNCTION_WITH_ALL_VAARGS, Ensures that all vaargs are faithfully passed to the custom panic function via PICO_PANIC_FUNCTION=. When set to 1 it conflicts with PICO_PANIC_FUNCTION_WITH_CALL_STACK=1 because both the later vaargs and the stack frame would be expected at the top of the stack. Note this defaults to 0 as most custom panic functions don't actually use the arguments and the call stack seems more useful in general, default=1group=pico_runtime
+#ifndef PICO_PANIC_FUNCTION_WITH_ALL_VAARGS
+#define PICO_PANIC_FUNCTION_WITH_ALL_VAARGS 0
+#endif
+
+
+#ifndef __ASSEMBLER__
 /*! \brief Panics with the message "Unsupported"
  *  \ingroup pico_platform
  *  \see panic

src/rp2_common/pico_platform_panic/panic.c

@@ -21,41 +21,8 @@ void __attribute__((noreturn)) panic_unsupported(void) {
     panic("not supported");
 }
 
-// PICO_CONFIG: PICO_PANIC_FUNCTION, Name of a function to use in place of the stock panic function or empty string to simply breakpoint on panic, group=pico_runtime
 // note the default is not "panic" it is undefined
-#ifdef PICO_PANIC_FUNCTION
-#define PICO_PANIC_FUNCTION_EMPTY (__CONCAT(PICO_PANIC_FUNCTION, 1) == 1)
-#if !PICO_PANIC_FUNCTION_EMPTY
-extern void __attribute__((noreturn)) __printflike(1, 0) PICO_PANIC_FUNCTION(__unused const char *fmt, ...);
-#endif
-// Use a forwarding method here as it is a little simpler than renaming the symbol as it is used from assembler
-void __attribute__((naked, noreturn)) __printflike(1, 0) panic(__unused const char *fmt, ...) {
-    // if you get an undefined reference here, you didn't define your PICO_PANIC_FUNCTION!
-    pico_default_asm (
-#ifdef __riscv
-
-#if !PICO_PANIC_FUNCTION_EMPTY
-            "jal " __XSTRING(PICO_PANIC_FUNCTION) "\n"
-#endif
-            "1: ebreak\n"
-            "j 1b\n"
-
-#else
-
-            "push {lr}\n"
-#if !PICO_PANIC_FUNCTION_EMPTY
-            "bl " __XSTRING(PICO_PANIC_FUNCTION) "\n"
-#endif
-            "1: bkpt #0\n"
-            "b 1b\n" // loop for ever as we are no return
-
-#endif
-        :
-        :
-        :
-    );
-}
-#else
+#ifndef PICO_PANIC_FUNCTION
 // todo consider making this try harder to output if we panic early
 //  right now, print mutex may be uninitialised (in which case it deadlocks - although after printing "PANIC")
 //  more importantly there may be no stdout/UART initialized yet
@@ -75,7 +42,7 @@ void __attribute__((noreturn)) __printflike(1, 0) panic(const char *fmt, ...) {
         weak_raw_vprintf(fmt, args);
 #endif
         va_end(args);
-        puts("\n");
+        puts("");
 #endif
     }
 

test/CMakeLists.txt

@@ -4,6 +4,7 @@ add_subdirectory(pico_stdlib_test)
 add_subdirectory(pico_stdio_test)
 add_subdirectory(pico_time_test)
 add_subdirectory(pico_divider_test)
+add_subdirectory(panic_function_test)
 if (PICO_ON_DEVICE)
     add_subdirectory(pico_float_test)
     add_subdirectory(kitchen_sink)

test/panic_function_test/CMakeLists.txt

@@ -0,0 +1,18 @@
+add_executable(panic_function_test_default ${CMAKE_CURRENT_LIST_DIR}/panic_function_test.c)
+target_link_libraries(panic_function_test_default PRIVATE pico_stdlib)
+pico_add_extra_outputs(panic_function_test_default)
+
+add_executable(panic_function_test_bkpt ${CMAKE_CURRENT_LIST_DIR}/panic_function_test.c)
+target_compile_definitions(panic_function_test_bkpt PRIVATE PICO_PANIC_FUNCTION=) # should just bkpt
+target_link_libraries(panic_function_test_bkpt PRIVATE pico_stdlib)
+pico_add_extra_outputs(panic_function_test_bkpt)
+
+add_executable(panic_function_test_user ${CMAKE_CURRENT_LIST_DIR}/panic_function_test.c)
+target_compile_definitions(panic_function_test_user PRIVATE PICO_PANIC_FUNCTION=handle_panic)
+target_link_libraries(panic_function_test_user PRIVATE pico_stdlib)
+pico_add_extra_outputs(panic_function_test_user)
+
+add_executable(panic_function_test_user_all_vaargs ${CMAKE_CURRENT_LIST_DIR}/panic_function_test.c)
+target_compile_definitions(panic_function_test_user_all_vaargs PRIVATE PICO_PANIC_FUNCTION=handle_panic PICO_PANIC_FUNCTION_WITH_ALL_VAARGS=1 PICO_PANIC_FUNCTION_WITH_CALL_STACK=0)
+target_link_libraries(panic_function_test_user_all_vaargs PRIVATE pico_stdlib)
+pico_add_extra_outputs(panic_function_test_user_all_vaargs)

test/panic_function_test/panic_function_test.c

@@ -0,0 +1,78 @@
+#include <stdio.h>
+#include <stdarg.h>
+
+#include "pico/stdlib.h"
+
+#define MAGIC1 ((const char *)0x87654321)
+#define MAGIC2 0x97654321
+#define MAGIC3 3.14159265358979323846
+#define MAGIC4 123
+#define MAGIC5 456
+#define MAGIC6 789
+#define MAGIC7 0xabc
+
+#define PICO_PANIC_FUNCTION_IS_EMPTY (__CONCAT(PICO_PANIC_FUNCTION, 1))
+
+void __printflike(1, 0) handle_panic(const char *magic1, ...)
+{
+    printf("checking first arg...\n");
+    if (magic1 != MAGIC1) {
+        printf("magic1 (%08x) != 0x%08x\n", magic1, MAGIC1);
+        return;
+    }
+    va_list args;
+    va_start(args, magic1);
+    printf("checking early vaargs...\n");
+    int magic2 = va_arg(args, int);
+    if (magic2 != MAGIC2) {
+        printf("magic2 (%08x) != 0x%08x\n", magic2, MAGIC2);
+        return;
+    }
+    double magic3 = va_arg(args, double);
+    if (magic3 != MAGIC3) {
+        printf("magic3 (%f) != 0x%f\n", magic3, MAGIC3);
+        return;
+    }
+#if PICO_PANIC_FUNCTION_WITH_ALL_VAARGS
+    printf("checking remaining vaargs...\n");
+    int magic4 = va_arg(args, int);
+    if (magic4 != MAGIC4) {
+        printf("magic4 (%08x) != 0x%08x\n", magic4, MAGIC4);
+        return;
+    }
+    int magic5 = va_arg(args, int);
+    if (magic5 != MAGIC5) {
+        printf("magic5 (%08x) != 0x%08x\n", magic5, MAGIC5);
+        return;
+    }
+    int magic6 = va_arg(args, int);
+    if (magic6 != MAGIC6) {
+        printf("magic6 (%08x) != 0x%08x\n", magic6, MAGIC6);
+        return;
+    }
+    int magic7 = va_arg(args, int);
+    if (magic7 != MAGIC7) {
+        printf("magic7 (%08x) != 0x%08x\n", magic7, MAGIC7);
+        return;
+    }
+#endif
+    va_end(args);
+    puts("PASSED");
+}
+
+void main() {
+    stdio_init_all();
+#ifndef PICO_PANIC_FUNCTION
+    printf("Using default panic function...\n");
+    panic("PASSED"); // should be printed
+#else
+    #if PICO_PANIC_FUNCTION_IS_EMPTY
+    printf("Using empty (bkpt only) panic function...\n");
+    printf("PASSED\n"); // we should breakpoint next
+    panic(MAGIC1, MAGIC2, MAGIC3, MAGIC4, MAGIC5, MAGIC6, MAGIC7);
+    #endif
+    printf("Using custom panic function '" __XSTRING(PICO_PANIC_FUNCTION) "'...\n");
+    panic(MAGIC1, MAGIC2, MAGIC3, MAGIC4, MAGIC5, MAGIC6, MAGIC7);
+#endif
+    printf("FAILED: expected panic not to return");
+}