diff --git a/components/esp_system/panic.c b/components/esp_system/panic.c index 8622a640ce..b263cf120c 100644 --- a/components/esp_system/panic.c +++ b/components/esp_system/panic.c @@ -257,8 +257,8 @@ void esp_panic_handler(panic_info_t *info) * * ---------------------------------------------------------------------------------------- * core - core where exception was triggered - * exception - what kind of exception occured - * description - a short description regarding the exception that occured + * exception - what kind of exception occurred + * description - a short description regarding the exception that occurred * details - more details about the exception * state - processor state like register contents, and backtrace * elf_info - details about the image currently running @@ -323,6 +323,14 @@ void esp_panic_handler(panic_info_t *info) PANIC_INFO_DUMP(info, state); panic_print_str("\r\n"); + /* No matter if we come here from abort or an exception, this variable must be reset. + * Else, any exception/error occurring during the current panic handler would considered + * an abort. Do this after PANIC_INFO_DUMP(info, state) as it also checks this variable. + * For example, if coredump triggers a stack overflow and this variable is not reset, + * the second panic would be still be marked as the result of an abort, even the previous + * message reason would be kept. */ + g_panic_abort = false; + #ifdef WITH_ELF_SHA256 panic_print_str("\r\nELF file SHA256: "); char sha256_buf[65]; @@ -359,10 +367,6 @@ void esp_panic_handler(panic_info_t *info) } else { disable_all_wdts(); s_dumping_core = true; - /* No matter if we come here from abort or an exception, this variable must be reset. - * Else, any exception/error occuring during the current panic handler would considered - * an abort. */ - g_panic_abort = false; #if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH esp_core_dump_to_flash(info); #endif diff --git a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c index 96a7765e76..cb4a8bc8bd 100644 --- a/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c +++ b/components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/port.c @@ -536,30 +536,6 @@ void vApplicationGetTimerTaskMemory(StaticTask_t **ppxTimerTaskTCBBuffer, // ------------------------ Stack -------------------------- -__attribute__((noreturn)) static void _prvTaskExitError(void) -{ - /* A function that implements a task must not exit or attempt to return to - its caller as there is nothing to return to. If a task wants to exit it - should instead call vTaskDelete( NULL ). - - Artificially force an assert() to be triggered if configASSERT() is - defined, then stop here so application writers can catch the error. */ - portDISABLE_INTERRUPTS(); - abort(); -} - -__attribute__((naked)) static void prvTaskExitError(void) -{ - asm volatile(".option push\n" \ - ".option norvc\n" \ - "nop\n" \ - ".option pop"); - /* Task entry's RA will point here. Shifting RA into prvTaskExitError is necessary - to make GDB backtrace ending inside that function. - Otherwise backtrace will end in the function laying just before prvTaskExitError in address space. */ - _prvTaskExitError(); -} - /** * @brief Align stack pointer in a downward growing stack * @@ -641,6 +617,29 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackTLS(UBaseType_t uxStackPointer, u return uxStackPointer; } +#if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER +/** + * Wrapper to allow task functions to return. Force the optimization option -O1 on that function to make sure there + * is no tail-call. Indeed, we need the compiler to keep the return address to this function when calling `panic_abort`. + * + * Thanks to `naked` attribute, the compiler won't generate a prologue and epilogue for the function, which saves time + * and stack space. + */ +static void __attribute__((optimize("O1"), naked)) vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters) +{ + asm volatile(".cfi_undefined ra\n"); + extern void __attribute__((noreturn)) panic_abort(const char *details); + static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0"; + pxCode(pvParameters); + //FreeRTOS tasks should not return. Log the task name and abort. + char *pcTaskName = pcTaskGetName(NULL); + /* We cannot use s(n)printf because it is in flash */ + strcat(msg, pcTaskName); + strcat(msg, "\" should not return, Aborting now!"); + panic_abort(msg); +} +#endif // CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER + /** * @brief Initialize the task's starting interrupt stack frame * @@ -669,16 +668,16 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackFrame(UBaseType_t uxStackPointer, RvExcFrame *frame = (RvExcFrame *)uxStackPointer; memset(frame, 0, sizeof(RvExcFrame)); - /* - Initialize the stack frame. - - Note: Shifting RA into prvTaskExitError is necessary to make the GDB backtrace terminate inside that function. - Otherwise, the backtrace will end in the function located just before prvTaskExitError in the address space. - */ + /* Initialize the stack frame. */ extern uint32_t __global_pointer$; - frame->ra = (UBaseType_t)prvTaskExitError + 4; // size of the nop instruction at the beginning of prvTaskExitError - frame->mepc = (UBaseType_t)pxCode; - frame->a0 = (UBaseType_t)pvParameters; + #if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER + frame->mepc = (UBaseType_t)vPortTaskWrapper; + frame->a0 = (UBaseType_t)pxCode; + frame->a1 = (UBaseType_t)pvParameters; + #else + frame->mepc = (UBaseType_t)pxCode; + frame->a0 = (UBaseType_t)pvParameters; + #endif // CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER frame->gp = (UBaseType_t)&__global_pointer$; frame->tp = (UBaseType_t)threadptr_reg_init; diff --git a/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c b/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c index f5b3d08545..13b8cc6cce 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c +++ b/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c @@ -114,31 +114,6 @@ void vPortEndScheduler(void) // ------------------------ Stack -------------------------- -__attribute__((noreturn)) static void _prvTaskExitError(void) -{ - /* A function that implements a task must not exit or attempt to return to - its caller as there is nothing to return to. If a task wants to exit it - should instead call vTaskDelete( NULL ). - - Artificially force an assert() to be triggered if configASSERT() is - defined, then stop here so application writers can catch the error. */ - configASSERT(uxCriticalNesting == ~0UL); - portDISABLE_INTERRUPTS(); - abort(); -} - -__attribute__((naked)) static void prvTaskExitError(void) -{ - asm volatile(".option push\n" \ - ".option norvc\n" \ - "nop\n" \ - ".option pop"); - /* Task entry's RA will point here. Shifting RA into prvTaskExitError is necessary - to make GDB backtrace ending inside that function. - Otherwise backtrace will end in the function laying just before prvTaskExitError in address space. */ - _prvTaskExitError(); -} - /** * @brief Align stack pointer in a downward growing stack * @@ -220,6 +195,29 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackTLS(UBaseType_t uxStackPointer, u return uxStackPointer; } +#if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER +/** + * Wrapper to allow task functions to return. Force the optimization option -O1 on that function to make sure there + * is no tail-call. Indeed, we need the compiler to keep the return address to this function when calling `panic_abort`. + * + * Thanks to `naked` attribute, the compiler won't generate a prologue and epilogue for the function, which saves time + * and stack space. + */ +static void __attribute__((optimize("O1"), naked)) vPortTaskWrapper(TaskFunction_t pxCode, void *pvParameters) +{ + asm volatile(".cfi_undefined ra\n"); + extern void __attribute__((noreturn)) panic_abort(const char *details); + static char DRAM_ATTR msg[80] = "FreeRTOS: FreeRTOS Task \"\0"; + pxCode(pvParameters); + //FreeRTOS tasks should not return. Log the task name and abort. + char *pcTaskName = pcTaskGetName(NULL); + /* We cannot use s(n)printf because it is in flash */ + strcat(msg, pcTaskName); + strcat(msg, "\" should not return, Aborting now!"); + panic_abort(msg); +} +#endif // CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER + /** * @brief Initialize the task's starting interrupt stack frame * @@ -248,16 +246,16 @@ FORCE_INLINE_ATTR UBaseType_t uxInitialiseStackFrame(UBaseType_t uxStackPointer, RvExcFrame *frame = (RvExcFrame *)uxStackPointer; memset(frame, 0, sizeof(RvExcFrame)); - /* - Initialize the stack frame. - - Note: Shifting RA into prvTaskExitError is necessary to make the GDB backtrace terminate inside that function. - Otherwise, the backtrace will end in the function located just before prvTaskExitError in the address space. - */ + /* Initialize the stack frame. */ extern uint32_t __global_pointer$; - frame->ra = (UBaseType_t)prvTaskExitError + 4; // size of the nop instruction at the beginning of prvTaskExitError - frame->mepc = (UBaseType_t)pxCode; - frame->a0 = (UBaseType_t)pvParameters; + #if CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER + frame->mepc = (UBaseType_t)vPortTaskWrapper; + frame->a0 = (UBaseType_t)pxCode; + frame->a1 = (UBaseType_t)pvParameters; + #else + frame->mepc = (UBaseType_t)pxCode; + frame->a0 = (UBaseType_t)pvParameters; + #endif // CONFIG_FREERTOS_TASK_FUNCTION_WRAPPER frame->gp = (UBaseType_t)&__global_pointer$; frame->tp = (UBaseType_t)threadptr_reg_init;