From 2d3469e057cc4996fa8eb8c6e616a7b74fd2fb79 Mon Sep 17 00:00:00 2001 From: MX <10697207+xMasterX@users.noreply.github.com> Date: Sat, 29 Oct 2022 19:15:46 +0300 Subject: [PATCH 1/4] downgrade check.c/.h - see desc (Furi: smaller crash routine) causes crashes when furi_halt is called, why? I donT knoW This is temporary solution until real bug will be found --- firmware/targets/f7/api_symbols.csv | 3 +- furi/core/check.c | 58 +++++++++++++---------------- furi/core/check.h | 34 ++++++----------- 3 files changed, 39 insertions(+), 56 deletions(-) diff --git a/firmware/targets/f7/api_symbols.csv b/firmware/targets/f7/api_symbols.csv index a05319b9d..34a578317 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,+,8.2,, +Version,+,8.21,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/cli/cli.h,, Header,+,applications/services/cli/cli_vcp.h,, @@ -4254,6 +4254,7 @@ Variable,-,ITM_RxBuffer,volatile int32_t, 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 00c20575a..e33d475a0 100644 --- a/furi/core/check.c +++ b/furi/core/check.c @@ -14,28 +14,6 @@ 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); @@ -77,11 +55,27 @@ static void __furi_print_name(bool isr) { } } -FURI_NORETURN void __furi_crash() { - __disable_irq(); - GET_MESSAGE_AND_STORE_REGISTERS(); +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(); +} - bool isr = FURI_IS_IRQ_MODE(); +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(); if(__furi_check_message == NULL) { __furi_check_message = "Fatal Error"; @@ -99,7 +93,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"); - RESTORE_REGISTERS_AND_HALT_MCU(); + __furi_halt_mcu(); #else furi_hal_rtc_set_fault_data((uint32_t)__furi_check_message); furi_hal_console_puts("\r\nRebooting system.\r\n"); @@ -110,10 +104,11 @@ FURI_NORETURN void __furi_crash() { } FURI_NORETURN void __furi_halt() { - __disable_irq(); - GET_MESSAGE_AND_STORE_REGISTERS(); + register const void* r12 asm("r12") = (void*)__furi_check_registers; + asm volatile("stm r12, {r0-r11} \n" : : "r"(r12) : "memory"); - bool isr = FURI_IS_IRQ_MODE(); + bool isr = FURI_IS_ISR(); + __disable_irq(); if(__furi_check_message == NULL) { __furi_check_message = "System halt requested."; @@ -124,6 +119,5 @@ 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"); - RESTORE_REGISTERS_AND_HALT_MCU(); - __builtin_unreachable(); + __furi_halt_mcu(); } diff --git a/furi/core/check.h b/furi/core/check.h index 78efc1451..fe2d07fb4 100644 --- a/furi/core/check.h +++ b/furi/core/check.h @@ -1,16 +1,3 @@ -/** - * @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 @@ -21,6 +8,9 @@ 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(); @@ -28,19 +18,17 @@ FURI_NORETURN void __furi_crash(); FURI_NORETURN void __furi_halt(); /** Crash system with message. Show message after reboot. */ -#define furi_crash(message) \ - do { \ - register const void* r12 asm("r12") = (void*)message; \ - asm volatile("sukima%=:" : : "r"(r12)); \ - __furi_crash(); \ +#define furi_crash(message) \ + do { \ + __furi_check_message = message; \ + __furi_crash(); \ } while(0) /** Halt system with message. */ -#define furi_halt(message) \ - do { \ - register const void* r12 asm("r12") = (void*)message; \ - asm volatile("sukima%=:" : : "r"(r12)); \ - __furi_halt(); \ +#define furi_halt(message) \ + do { \ + __furi_check_message = message; \ + __furi_halt(); \ } while(0) /** Check condition and crash if check failed */ From ddc6265920ee6b5924630f4040fee8d315b205b3 Mon Sep 17 00:00:00 2001 From: MX <10697207+xMasterX@users.noreply.github.com> Date: Sat, 29 Oct 2022 19:15:46 +0300 Subject: [PATCH 2/4] Revert "downgrade check.c/.h - see desc" This reverts commit 2d3469e057cc4996fa8eb8c6e616a7b74fd2fb79. --- firmware/targets/f7/api_symbols.csv | 3 +- furi/core/check.c | 56 ++++++++++++++++------------- furi/core/check.h | 34 ++++++++++++------ 3 files changed, 55 insertions(+), 38 deletions(-) diff --git a/firmware/targets/f7/api_symbols.csv b/firmware/targets/f7/api_symbols.csv index 34a578317..a05319b9d 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,+,8.21,, +Version,+,8.2,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/cli/cli.h,, Header,+,applications/services/cli/cli_vcp.h,, @@ -4254,7 +4254,6 @@ Variable,-,ITM_RxBuffer,volatile int32_t, 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 e33d475a0..00c20575a 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,27 +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"; @@ -93,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"); @@ -104,11 +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."; @@ -119,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 fe2d07fb4..78efc1451 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,17 +28,19 @@ 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 */ From 296bdfd433992b87a3728ee48d2da28a8a9c02df Mon Sep 17 00:00:00 2001 From: MX <10697207+xMasterX@users.noreply.github.com> Date: Sat, 29 Oct 2022 19:36:40 +0300 Subject: [PATCH 3/4] actual proper fix of furi_halt --- furi/core/check.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/furi/core/check.c b/furi/core/check.c index 00c20575a..107eb4d1a 100644 --- a/furi/core/check.c +++ b/furi/core/check.c @@ -24,8 +24,8 @@ PLACE_IN_SECTION("MB_MEM2") uint32_t __furi_check_registers[12] = {0}; : \ : "memory"); -// Restore registers and halt MCU -#define RESTORE_REGISTERS_AND_HALT_MCU() \ +// Restore registers and halt MCU with bkpt debug state +#define RESTORE_REGISTERS_AND_HALT_MCU_DEBUG() \ asm volatile("ldr r12, =__furi_check_registers \n" \ "ldm r12, {r0-r11} \n" \ "loop%=: \n" \ @@ -36,6 +36,17 @@ PLACE_IN_SECTION("MB_MEM2") uint32_t __furi_check_registers[12] = {0}; : \ : "memory"); +// Restore registers and halt MCU without bkpt debug mode +#define RESTORE_REGISTERS_AND_HALT_MCU_RELEASE() \ + asm volatile("ldr r12, =__furi_check_registers \n" \ + "ldm r12, {r0-r11} \n" \ + "loop%=: \n" \ + "wfi \n" \ + "b loop%= \n" \ + : \ + : \ + : "memory"); + extern size_t xPortGetTotalHeapSize(void); extern size_t xPortGetFreeHeapSize(void); extern size_t xPortGetMinimumEverFreeHeapSize(void); @@ -99,7 +110,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"); - RESTORE_REGISTERS_AND_HALT_MCU(); + RESTORE_REGISTERS_AND_HALT_MCU_DEBUG(); #else furi_hal_rtc_set_fault_data((uint32_t)__furi_check_message); furi_hal_console_puts("\r\nRebooting system.\r\n"); @@ -124,6 +135,10 @@ 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"); - RESTORE_REGISTERS_AND_HALT_MCU(); +#ifdef FURI_DEBUG + RESTORE_REGISTERS_AND_HALT_MCU_DEBUG(); +#else + RESTORE_REGISTERS_AND_HALT_MCU_RELEASE(); +#endif __builtin_unreachable(); } From 28d9d5194fadecfcbfe5f6a9477e3366131f70d9 Mon Sep 17 00:00:00 2001 From: MX <10697207+xMasterX@users.noreply.github.com> Date: Sat, 29 Oct 2022 20:13:07 +0300 Subject: [PATCH 4/4] simpler version as @DerSkythe suggested --- furi/core/check.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/furi/core/check.c b/furi/core/check.c index 107eb4d1a..cf1b790e8 100644 --- a/furi/core/check.c +++ b/furi/core/check.c @@ -24,8 +24,9 @@ PLACE_IN_SECTION("MB_MEM2") uint32_t __furi_check_registers[12] = {0}; : \ : "memory"); -// Restore registers and halt MCU with bkpt debug state -#define RESTORE_REGISTERS_AND_HALT_MCU_DEBUG() \ +#ifdef FURI_DEBUG +// Restore registers and halt MCU with bkpt mcu debug state +#define RESTORE_REGISTERS_AND_HALT_MCU() \ asm volatile("ldr r12, =__furi_check_registers \n" \ "ldm r12, {r0-r11} \n" \ "loop%=: \n" \ @@ -35,9 +36,9 @@ PLACE_IN_SECTION("MB_MEM2") uint32_t __furi_check_registers[12] = {0}; : \ : \ : "memory"); - -// Restore registers and halt MCU without bkpt debug mode -#define RESTORE_REGISTERS_AND_HALT_MCU_RELEASE() \ +#else +// Restore registers and halt MCU for release builds without bkpt instruction +#define RESTORE_REGISTERS_AND_HALT_MCU() \ asm volatile("ldr r12, =__furi_check_registers \n" \ "ldm r12, {r0-r11} \n" \ "loop%=: \n" \ @@ -46,6 +47,7 @@ PLACE_IN_SECTION("MB_MEM2") uint32_t __furi_check_registers[12] = {0}; : \ : \ : "memory"); +#endif extern size_t xPortGetTotalHeapSize(void); extern size_t xPortGetFreeHeapSize(void); @@ -110,7 +112,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"); - RESTORE_REGISTERS_AND_HALT_MCU_DEBUG(); + 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"); @@ -135,10 +137,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"); -#ifdef FURI_DEBUG - RESTORE_REGISTERS_AND_HALT_MCU_DEBUG(); -#else - RESTORE_REGISTERS_AND_HALT_MCU_RELEASE(); -#endif + RESTORE_REGISTERS_AND_HALT_MCU(); __builtin_unreachable(); }