From 3a42bf812d391999d045d998eef036f1093690f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=82=E3=81=8F?= Date: Tue, 18 Feb 2025 03:16:14 +0900 Subject: [PATCH] Furi, USB, BLE, Debug: various bug fixes and improvements (#4114) * Furi, USB, BLE: extra stack space for some threads, small code cleanup. * Furi: thread watermark check on exit, explicitly crash if built with LIB_DEBUG=1 * Debug: color logging in apps/furi gdb helper, check and show crash message in gdb console. --- applications/main/gpio/usb_uart_bridge.c | 7 ++- furi/core/thread.c | 14 +++++ scripts/debug/flipperapps.py | 72 +++++++++++++++++++----- targets/f7/ble_glue/ble_event_thread.c | 2 +- targets/f7/furi_hal/furi_hal_usb_cdc.c | 15 +++-- 5 files changed, 87 insertions(+), 23 deletions(-) diff --git a/applications/main/gpio/usb_uart_bridge.c b/applications/main/gpio/usb_uart_bridge.c index f6e68b10966..e6b71cb34f8 100644 --- a/applications/main/gpio/usb_uart_bridge.c +++ b/applications/main/gpio/usb_uart_bridge.c @@ -183,7 +183,7 @@ static int32_t usb_uart_worker(void* context) { usb_uart->usb_mutex = furi_mutex_alloc(FuriMutexTypeNormal); usb_uart->tx_thread = - furi_thread_alloc_ex("UsbUartTxWorker", 512, usb_uart_tx_thread, usb_uart); + furi_thread_alloc_ex("UsbUartTxWorker", 768, usb_uart_tx_thread, usb_uart); usb_uart_vcp_init(usb_uart, usb_uart->cfg.vcp_ch); usb_uart_serial_init(usb_uart, usb_uart->cfg.uart_ch); @@ -288,8 +288,6 @@ static int32_t usb_uart_worker(void* context) { usb_uart_update_ctrl_lines(usb_uart); } } - usb_uart_vcp_deinit(usb_uart, usb_uart->cfg.vcp_ch); - usb_uart_serial_deinit(usb_uart); furi_hal_gpio_init(USB_USART_DE_RE_PIN, GpioModeAnalog, GpioPullNo, GpioSpeedLow); @@ -302,6 +300,9 @@ static int32_t usb_uart_worker(void* context) { furi_thread_join(usb_uart->tx_thread); furi_thread_free(usb_uart->tx_thread); + usb_uart_vcp_deinit(usb_uart, usb_uart->cfg.vcp_ch); + usb_uart_serial_deinit(usb_uart); + furi_stream_buffer_free(usb_uart->rx_stream); furi_mutex_free(usb_uart->usb_mutex); furi_semaphore_free(usb_uart->tx_sem); diff --git a/furi/core/thread.c b/furi/core/thread.c index 6e515795775..f6cfa1d363a 100644 --- a/furi/core/thread.c +++ b/furi/core/thread.c @@ -23,6 +23,8 @@ #define THREAD_MAX_STACK_SIZE (UINT16_MAX * sizeof(StackType_t)) +#define THREAD_STACK_WATERMARK_MIN (256u) + typedef struct { FuriThreadStdoutWriteCallback write_callback; FuriString* buffer; @@ -115,6 +117,18 @@ static void furi_thread_body(void* context) { furi_check(!thread->is_service, "Service threads MUST NOT return"); + size_t stack_watermark = furi_thread_get_stack_space(thread); + if(stack_watermark < THREAD_STACK_WATERMARK_MIN) { +#ifdef FURI_DEBUG + furi_crash("Stack watermark is dangerously low"); +#endif + FURI_LOG_E( //-V779 + thread->name ? thread->name : "Thread", + "Stack watermark is too low %zu < " STRINGIFY( + THREAD_STACK_WATERMARK_MIN) ". Increase stack size.", + stack_watermark); + } + if(thread->heap_trace_enabled == true) { furi_delay_ms(33); thread->heap_size = memmgr_heap_get_thread_memory((FuriThreadId)thread); diff --git a/scripts/debug/flipperapps.py b/scripts/debug/flipperapps.py index 81aa43c34cf..6d2d4c0a9d4 100644 --- a/scripts/debug/flipperapps.py +++ b/scripts/debug/flipperapps.py @@ -7,6 +7,33 @@ import gdb +class bcolors: + HEADER = "\033[95m" + OKBLUE = "\033[94m" + OKCYAN = "\033[96m" + OKGREEN = "\033[92m" + WARNING = "\033[93m" + FAIL = "\033[91m" + ENDC = "\033[0m" + BOLD = "\033[1m" + UNDERLINE = "\033[4m" + + +LOG_PREFIX = "[FURI]" + + +def error(line): + print(f"{bcolors.FAIL}{LOG_PREFIX} {line}{bcolors.ENDC}") + + +def warning(line): + print(f"{bcolors.WARNING}{LOG_PREFIX} {line}{bcolors.ENDC}") + + +def info(line): + print(f"{bcolors.OKGREEN}{LOG_PREFIX} {line}{bcolors.ENDC}") + + def get_file_crc32(filename): with open(filename, "rb") as f: return zlib.crc32(f.read()) @@ -39,12 +66,12 @@ def get_original_elf_path(self) -> str: def is_debug_available(self) -> bool: have_debug_info = bool(self.debug_link_elf and self.debug_link_crc) if not have_debug_info: - print("No debug info available for this app") + warning("No debug info available for this app") return False debug_elf_path = self.get_original_elf_path() debug_elf_crc32 = get_file_crc32(debug_elf_path) if self.debug_link_crc != debug_elf_crc32: - print( + warning( f"Debug info ({debug_elf_path}) CRC mismatch: {self.debug_link_crc:08x} != {debug_elf_crc32:08x}, rebuild app" ) return False @@ -52,7 +79,7 @@ def is_debug_available(self) -> bool: def get_gdb_load_command(self) -> str: load_path = self.get_original_elf_path() - print(f"Loading debug information from {load_path}") + info(f"Loading debug information from {load_path}") load_command = ( f"add-symbol-file -readnow {load_path} 0x{self.text_address:08x} " ) @@ -121,12 +148,12 @@ def invoke(self, arg, from_tty): AppState.DEBUG_ELF_ROOT = arg try: global helper - print(f"Set '{arg}' as debug info lookup path for Flipper external apps") + info(f"Set '{arg}' as debug info lookup path for Flipper external apps") helper.attach_to_fw() gdb.events.stop.connect(helper.handle_stop) gdb.events.gdb_exiting.connect(helper.handle_exit) except gdb.error as e: - print(f"Support for Flipper external apps debug is not available: {e}") + error(f"Support for Flipper external apps debug is not available: {e}") class FlipperAppStateHelper: @@ -148,13 +175,29 @@ def _exec_gdb_command(self, command: str) -> bool: gdb.execute(command) return True except gdb.error as e: - print(f"Failed to execute GDB command '{command}': {e}") + error(f"Failed to execute GDB command '{command}': {e}") return False + def _get_crash_message(self): + message = self.app_check_message.value() + if message == 1: + return "furi_assert failed" + elif message == 2: + return "furi_check failed" + else: + return message + def _sync_apps(self) -> None: + crash_message = self._get_crash_message() + if crash_message: + crash_message = f"! System crashed: {crash_message} !" + error("!" * len(crash_message)) + error(crash_message) + error("!" * len(crash_message)) + self.set_debug_mode(True) if not (app_list := self.app_list_ptr.value()): - print("Reset app loader state") + info("Reset app loader state") for app in self._current_apps: self._exec_gdb_command(app.get_gdb_unload_command()) self._current_apps = [] @@ -167,22 +210,23 @@ def _sync_apps(self) -> None: for app in self._current_apps.copy(): if app.entry_address not in loaded_apps: - print(f"Application {app.name} is no longer loaded") + warning(f"Application {app.name} is no longer loaded") if not self._exec_gdb_command(app.get_gdb_unload_command()): - print(f"Failed to unload debug info for {app.name}") + error(f"Failed to unload debug info for {app.name}") self._current_apps.remove(app) for entry_point, app in loaded_apps.items(): if entry_point not in set(app.entry_address for app in self._current_apps): new_app_state = AppState.from_gdb(app) - print(f"New application loaded. Adding debug info") + warning(f"New application loaded. Adding debug info") if self._exec_gdb_command(new_app_state.get_gdb_load_command()): self._current_apps.append(new_app_state) else: - print(f"Failed to load debug info for {new_app_state}") + error(f"Failed to load debug info for {new_app_state}") def attach_to_fw(self) -> None: - print("Attaching to Flipper firmware") + info("Attaching to Flipper firmware") + self.app_check_message = gdb.lookup_global_symbol("__furi_check_message") self.app_list_ptr = gdb.lookup_global_symbol( "flipper_application_loaded_app_list" ) @@ -200,10 +244,10 @@ def set_debug_mode(self, mode: bool) -> None: try: gdb.execute(f"set variable furi_hal_debug_gdb_session_active = {int(mode)}") except gdb.error as e: - print(f"Failed to set debug mode: {e}") + error(f"Failed to set debug mode: {e}") # Init additional 'fap-set-debug-elf-root' command and set up hooks SetFapDebugElfRoot() helper = FlipperAppStateHelper() -print("Support for Flipper external apps debug is loaded") +info("Support for Flipper external apps debug is loaded") diff --git a/targets/f7/ble_glue/ble_event_thread.c b/targets/f7/ble_glue/ble_event_thread.c index c6bc5684613..3d1fdd1962a 100644 --- a/targets/f7/ble_glue/ble_event_thread.c +++ b/targets/f7/ble_glue/ble_event_thread.c @@ -90,7 +90,7 @@ void ble_event_thread_stop(void) { void ble_event_thread_start(void) { furi_check(event_thread == NULL); - event_thread = furi_thread_alloc_ex("BleEventWorker", 1024, ble_event_thread, NULL); + event_thread = furi_thread_alloc_ex("BleEventWorker", 1280, ble_event_thread, NULL); furi_thread_set_priority(event_thread, FuriThreadPriorityHigh); furi_thread_start(event_thread); } diff --git a/targets/f7/furi_hal/furi_hal_usb_cdc.c b/targets/f7/furi_hal/furi_hal_usb_cdc.c index cfedb5e76bf..f9c1d3a426a 100644 --- a/targets/f7/furi_hal/furi_hal_usb_cdc.c +++ b/targets/f7/furi_hal/furi_hal_usb_cdc.c @@ -392,10 +392,11 @@ static void cdc_on_suspend(usbd_device* dev); static usbd_respond cdc_ep_config(usbd_device* dev, uint8_t cfg); static usbd_respond cdc_control(usbd_device* dev, usbd_ctlreq* req, usbd_rqc_callback* callback); + static usbd_device* usb_dev; -static FuriHalUsbInterface* cdc_if_cur = NULL; -static bool connected = false; -static CdcCallbacks* callbacks[IF_NUM_MAX] = {NULL}; +static volatile FuriHalUsbInterface* cdc_if_cur = NULL; +static volatile bool connected = false; +static volatile CdcCallbacks* callbacks[IF_NUM_MAX] = {NULL}; static void* cb_ctx[IF_NUM_MAX]; FuriHalUsbInterface usb_cdc_single = { @@ -506,8 +507,10 @@ uint8_t furi_hal_cdc_get_ctrl_line_state(uint8_t if_num) { void furi_hal_cdc_send(uint8_t if_num, uint8_t* buf, uint16_t len) { if(if_num == 0) { usbd_ep_write(usb_dev, CDC0_TXD_EP, buf, len); - } else { + } else if(if_num == 1) { usbd_ep_write(usb_dev, CDC1_TXD_EP, buf, len); + } else { + furi_crash(); } } @@ -515,8 +518,10 @@ int32_t furi_hal_cdc_receive(uint8_t if_num, uint8_t* buf, uint16_t max_len) { int32_t len = 0; if(if_num == 0) { len = usbd_ep_read(usb_dev, CDC0_RXD_EP, buf, max_len); - } else { + } else if(if_num == 1) { len = usbd_ep_read(usb_dev, CDC1_RXD_EP, buf, max_len); + } else { + furi_crash(); } return (len < 0) ? 0 : len; }