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.

Author: skotopes

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);

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);

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,20 +66,20 @@ 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
         return True
 
     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")

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);
 }

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,17 +507,21 @@ 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();
     }
 }
 
 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;
 }