From 984d89c6d072a9c4c1955299b46f793ddf69cdd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=82=E3=81=8F?= Date: Mon, 24 Oct 2022 19:50:34 +0900 Subject: [PATCH] Furi: smaller crash routine (#1912) * Furi: smaller crash routine * Furi: small fixes * Furi: cleanup check routines, more assembly code, force inline of __furi_halt_mcu * SubGhz: cleanup residual line mess * Documentation * Dap-link: fix help * Furi: replace __furi_halt_mcu with HALT_MCU macros * Furi: disable IRQ earlier in crash handler * Furi: properly handle masked mode when detecting ISR * Ble: allow 0 length feed in rpc_session_feed * Format sources * Furi: better crash logic explanation. * Furi: some grammar in check.h Co-authored-by: SG --- .../dap_link/gui/scenes/dap_scene_help.c | 8 +-- applications/services/rpc/rpc.c | 3 +- firmware/targets/f7/api_symbols.csv | 3 +- furi/core/check.c | 65 +++++++++---------- furi/core/check.h | 60 ++++++++++------- 5 files changed, 74 insertions(+), 65 deletions(-) diff --git a/applications/plugins/dap_link/gui/scenes/dap_scene_help.c b/applications/plugins/dap_link/gui/scenes/dap_scene_help.c index 7193f4f4..d8d70e7f 100644 --- a/applications/plugins/dap_link/gui/scenes/dap_scene_help.c +++ b/applications/plugins/dap_link/gui/scenes/dap_scene_help.c @@ -72,13 +72,13 @@ void dap_scene_help_on_enter(void* context) { if(config->uart_swap == DapUartTXRXNormal) { furi_string_cat( string, - " TX: 15 [С1]\r\n" - " RX: 16 [С0]\r\n"); + " TX: 15 [C1]\r\n" + " RX: 16 [C0]\r\n"); } else { furi_string_cat( string, - " RX: 15 [С1]\r\n" - " TX: 16 [С0]\r\n"); + " RX: 15 [C1]\r\n" + " TX: 16 [C0]\r\n"); } break; default: diff --git a/applications/services/rpc/rpc.c b/applications/services/rpc/rpc.c index 06c05173..73eaadfb 100644 --- a/applications/services/rpc/rpc.c +++ b/applications/services/rpc/rpc.c @@ -148,7 +148,8 @@ size_t rpc_session_feed(RpcSession* session, uint8_t* encoded_bytes, size_t size, TickType_t timeout) { furi_assert(session); furi_assert(encoded_bytes); - furi_assert(size > 0); + + if(!size) return 0; size_t bytes_sent = furi_stream_buffer_send(session->stream, encoded_bytes, size, timeout); diff --git a/firmware/targets/f7/api_symbols.csv b/firmware/targets/f7/api_symbols.csv index 188cd748..4c48d9ab 100644 --- a/firmware/targets/f7/api_symbols.csv +++ b/firmware/targets/f7/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,4.0,, +Version,+,5.0,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/cli/cli.h,, Header,+,applications/services/cli/cli_vcp.h,, @@ -2843,7 +2843,6 @@ Variable,+,I_update_10px,const Icon, Variable,-,MSIRangeTable,const uint32_t[16], Variable,-,SmpsPrescalerTable,const uint32_t[4][6], Variable,+,SystemCoreClock,uint32_t, -Variable,+,__furi_check_message,const char*, Variable,+,_ctype_,const char[], Variable,+,_global_impure_ptr,_reent*, Variable,+,_impure_ptr,_reent*, diff --git a/furi/core/check.c b/furi/core/check.c index ed38038a..00c20575 100644 --- a/furi/core/check.c +++ b/furi/core/check.c @@ -14,6 +14,28 @@ PLACE_IN_SECTION("MB_MEM2") const char* __furi_check_message = NULL; PLACE_IN_SECTION("MB_MEM2") uint32_t __furi_check_registers[12] = {0}; +/** Load r12 value to __furi_check_message and store registers to __furi_check_registers */ +#define GET_MESSAGE_AND_STORE_REGISTERS() \ + asm volatile("ldr r11, =__furi_check_message \n" \ + "str r12, [r11] \n" \ + "ldr r12, =__furi_check_registers \n" \ + "stm r12, {r0-r11} \n" \ + : \ + : \ + : "memory"); + +// Restore registers and halt MCU +#define RESTORE_REGISTERS_AND_HALT_MCU() \ + asm volatile("ldr r12, =__furi_check_registers \n" \ + "ldm r12, {r0-r11} \n" \ + "loop%=: \n" \ + "bkpt 0x00 \n" \ + "wfi \n" \ + "b loop%= \n" \ + : \ + : \ + : "memory"); + extern size_t xPortGetTotalHeapSize(void); extern size_t xPortGetFreeHeapSize(void); extern size_t xPortGetMinimumEverFreeHeapSize(void); @@ -55,32 +77,11 @@ static void __furi_print_name(bool isr) { } } -static FURI_NORETURN void __furi_halt_mcu() { - register const void* r12 asm ("r12") = (void*)__furi_check_registers; - asm volatile( - "ldm r12, {r0-r11} \n" -#ifdef FURI_DEBUG - "bkpt 0x00 \n" -#endif - "loop%=: \n" - "wfi \n" - "b loop%= \n" - : - : "r" (r12) - : "memory"); - __builtin_unreachable(); -} - FURI_NORETURN void __furi_crash() { - register const void* r12 asm ("r12") = (void*)__furi_check_registers; - asm volatile( - "stm r12, {r0-r11} \n" - : - : "r" (r12) - : "memory"); - - bool isr = FURI_IS_ISR(); __disable_irq(); + GET_MESSAGE_AND_STORE_REGISTERS(); + + bool isr = FURI_IS_IRQ_MODE(); if(__furi_check_message == NULL) { __furi_check_message = "Fatal Error"; @@ -98,7 +99,7 @@ FURI_NORETURN void __furi_crash() { #ifdef FURI_DEBUG furi_hal_console_puts("\r\nSystem halted. Connect debugger for more info\r\n"); furi_hal_console_puts("\033[0m\r\n"); - __furi_halt_mcu(); + RESTORE_REGISTERS_AND_HALT_MCU(); #else furi_hal_rtc_set_fault_data((uint32_t)__furi_check_message); furi_hal_console_puts("\r\nRebooting system.\r\n"); @@ -109,15 +110,10 @@ FURI_NORETURN void __furi_crash() { } FURI_NORETURN void __furi_halt() { - register const void* r12 asm ("r12") = (void*)__furi_check_registers; - asm volatile( - "stm r12, {r0-r11} \n" - : - : "r" (r12) - : "memory"); - - bool isr = FURI_IS_ISR(); __disable_irq(); + GET_MESSAGE_AND_STORE_REGISTERS(); + + bool isr = FURI_IS_IRQ_MODE(); if(__furi_check_message == NULL) { __furi_check_message = "System halt requested."; @@ -128,5 +124,6 @@ FURI_NORETURN void __furi_halt() { furi_hal_console_puts(__furi_check_message); furi_hal_console_puts("\r\nSystem halted. Bye-bye!\r\n"); furi_hal_console_puts("\033[0m\r\n"); - __furi_halt_mcu(); + RESTORE_REGISTERS_AND_HALT_MCU(); + __builtin_unreachable(); } diff --git a/furi/core/check.h b/furi/core/check.h index e77891f7..78efc145 100644 --- a/furi/core/check.h +++ b/furi/core/check.h @@ -1,3 +1,16 @@ +/** + * @file check.h + * + * Furi crash and assert functions. + * + * The main problem with crashing is that you can't do anything without disturbing registers, + * and if you disturb registers, you won't be able to see the correct register values in the debugger. + * + * Current solution works around it by passing the message through r12 and doing some magic with registers in crash function. + * r0-r10 are stored in the ram2 on crash routine start and restored at the end. + * The only register that is going to be lost is r11. + * + */ #pragma once #ifdef __cplusplus @@ -8,9 +21,6 @@ extern "C" { #define FURI_NORETURN noreturn #endif -/** Pointer to pass message to __furi_crash and __furi_halt */ -extern const char* __furi_check_message; - /** Crash system */ FURI_NORETURN void __furi_crash(); @@ -18,39 +28,41 @@ FURI_NORETURN void __furi_crash(); FURI_NORETURN void __furi_halt(); /** Crash system with message. Show message after reboot. */ -#define furi_crash(message) \ - do { \ - __furi_check_message = message; \ - __furi_crash(); \ +#define furi_crash(message) \ + do { \ + register const void* r12 asm("r12") = (void*)message; \ + asm volatile("sukima%=:" : : "r"(r12)); \ + __furi_crash(); \ } while(0) /** Halt system with message. */ -#define furi_halt(message) \ - do { \ - __furi_check_message = message; \ - __furi_halt(); \ +#define furi_halt(message) \ + do { \ + register const void* r12 asm("r12") = (void*)message; \ + asm volatile("sukima%=:" : : "r"(r12)); \ + __furi_halt(); \ } while(0) /** Check condition and crash if check failed */ -#define furi_check(__e) \ - do { \ - if ((__e) == 0) { \ - furi_crash("furi_check failed\r\n"); \ - } \ +#define furi_check(__e) \ + do { \ + if((__e) == 0) { \ + furi_crash("furi_check failed\r\n"); \ + } \ } while(0) /** Only in debug build: Assert condition and crash if assert failed */ #ifdef FURI_DEBUG -#define furi_assert(__e) \ - do { \ - if ((__e) == 0) { \ - furi_crash("furi_assert failed\r\n"); \ - } \ +#define furi_assert(__e) \ + do { \ + if((__e) == 0) { \ + furi_crash("furi_assert failed\r\n"); \ + } \ } while(0) #else -#define furi_assert(__e) \ - do { \ - ((void)(__e)); \ +#define furi_assert(__e) \ + do { \ + ((void)(__e)); \ } while(0) #endif