From d9414bb6f20b227c14b2e9f3b4e763364cb1cd45 Mon Sep 17 00:00:00 2001 From: Lephe Date: Sat, 30 Mar 2024 19:43:29 +0100 Subject: [PATCH] gdb: allow gdb stub to automatically start when a crash occurs The user can still gdb_start() + gdb_main(NULL) manually, which allows e.g. early setup of breakpoints. The start_on_except mechanism is the lazier method where we just run the add-in normally and start fxsdk gdb on the PC *after* a crash occurs. --- include/gint/gdb.h | 33 ++++++++++--------- include/gint/usb.h | 3 ++ src/gdb/gdb.c | 80 ++++++++++++++++++++++++++++++--------------- src/usb/configure.c | 10 +++--- src/usb/usb.c | 13 ++++++++ 5 files changed, 93 insertions(+), 46 deletions(-) diff --git a/include/gint/gdb.h b/include/gint/gdb.h index 1f4c958..093a242 100644 --- a/include/gint/gdb.h +++ b/include/gint/gdb.h @@ -11,19 +11,12 @@ extern "C" { #include -/* Error codes for GDB functions */ -enum { - GDB_NO_INTERFACE = -1, - GDB_ALREADY_STARTED = -2, - GDB_USB_ERROR = -3, -}; - /* gdb_cpu_state_t: State of the CPU when breaking This struct keep the same register indices as those declared by GDB to allow easy R/W without needing a "translation" table. See : https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/sh-tdep.c; h=c402961b80a0b4589243023ea5362d43f644a9ec;hb=4f3e26ac6ee31f7bc4b04abd - 8bdb944e7f1fc5d2#l327 + 8bdb944e7f1fc5d2#l361 */ // TODO : Should we expose r*b*, ssr, spc ? are they double-saved when breaking // inside an interrupt handler ? @@ -60,17 +53,27 @@ typedef struct { /* gdb_start(): Start the GDB remote serial protocol server - This function will start the GDB remote serial protocol implementation and - block until the program is resumed from the connected debugger. - It currently only supports USB communication and will fail if USB is already - in use.*/ + This function starts the GDB stub (program that communicates with GDB). It + takes over USB, and returns once communication is established with a GDB + instance on the computer. Normally this is called by the panic handler after + a crash. It can also be called manually, in which case it should be followed + by a call to gdb_main() with the current state (NULL if you don't have any). + + If the GDB stub is already started, this is a no-op. Returns 0 on success + and a negative value if the USB setup fails. */ int gdb_start(void); /* gdb_main(): Main GDB loop + Main loop for when the program is in a paused state. Communicates with GDB + over USB and mutates the cpu_state struct, memory and the UBC configuration + accordingly. Returns once the program should resume. */ +void gdb_main(gdb_cpu_state_t *cpu_state); - This function handles GDB messages sent over USB and will mutate the cpu_state - struct, memory and the UBC configuration accordingly. */ -void gdb_main(gdb_cpu_state_t* cpu_state); +/* gdb_start_on_exception(): Set the GDB stub to autostart on crashes + This function sets a crash handler that starts the GDB stub whenever a + System ERROR occurs. */ +// TODO: Get some visible signal on-screen when that happens +void gdb_start_on_exception(void); #ifdef __cplusplus } diff --git a/include/gint/usb.h b/include/gint/usb.h index e727bfe..268b146 100644 --- a/include/gint/usb.h +++ b/include/gint/usb.h @@ -90,6 +90,9 @@ void usb_open_wait(void); /* usb_is_open(): Check whether the USB link is active */ bool usb_is_open(void); +/* usb_is_open_interface(): Check whether a particular interface is open */ +bool usb_is_open_interface(usb_interface_t const *interface); + /* usb_close(): Close the USB link This function closes the link opened by usb_open(), and notifies the host of diff --git a/src/gdb/gdb.c b/src/gdb/gdb.c index 54931f2..38af765 100644 --- a/src/gdb/gdb.c +++ b/src/gdb/gdb.c @@ -38,11 +38,7 @@ static uint32_t gdb_unhexlify(const char* input_string) return gdb_unhexlify_sized(input_string, strlen(input_string)); } -static enum { - GDB_STATE_STOPPED, - GDB_STATE_FIRST_BREAK, - GDB_STATE_STARTED, -} GPACKEDENUM gdb_state = GDB_STATE_STOPPED; +static bool gdb_started = false; static void gdb_send(const char *data, size_t size) { @@ -55,6 +51,16 @@ static void gdb_send(const char *data, size_t size) usb_commit_sync(pipe); } +static void gdb_send_start(void) +{ + usb_fxlink_header_t header; + usb_fxlink_fill_header(&header, "gdb", "start", 0); + + int pipe = usb_ff_bulk_output(); + usb_write_sync(pipe, &header, sizeof(header), false); + usb_commit_sync(pipe); +} + static char *gdb_recv_buffer = NULL; static const size_t gdb_recv_buffer_capacity = 256; static size_t gdb_recv_buffer_size = 0; @@ -488,6 +494,13 @@ static void gdb_handle_single_step(uint32_t pc, ubc_break_mode_t break_mode) void gdb_main(gdb_cpu_state_t* cpu_state) { + if (!gdb_started && gdb_start()) { + // TODO: Visual signal "GDB stub error" + return; + } + + // TODO: Visual signal "GDB stub idle" + if (gdb_single_step_backup.single_stepped) { if (gdb_single_step_backup.channel0_used) { ubc_set_breakpoint(0, gdb_single_step_backup.channel0_addr, UBC_BREAK_BEFORE); @@ -508,6 +521,8 @@ void gdb_main(gdb_cpu_state_t* cpu_state) } while (1) { + // TODO: Visual signal "GDB stub communicating" + char packet_buffer[256]; ssize_t packet_size = gdb_recv_packet(packet_buffer, sizeof(packet_buffer)); if (packet_size <= 0) { @@ -515,6 +530,8 @@ void gdb_main(gdb_cpu_state_t* cpu_state) continue; } + // TODO: Visual signal "GDB stub working" + switch (packet_buffer[0]) { case '?': // Halt reason gdb_send_stop_reply(); @@ -544,7 +561,6 @@ void gdb_main(gdb_cpu_state_t* cpu_state) case 'k': // Kill request abort(); - return; case 'Z': gdb_handle_insert_hardware_breakpoint(packet_buffer); @@ -555,21 +571,27 @@ void gdb_main(gdb_cpu_state_t* cpu_state) case 's': gdb_handle_single_step(cpu_state->reg.pc, UBC_BREAK_AFTER); - return; + goto ret; case 'c': // Continue - return; + goto ret; default: // Unsupported packet gdb_send_packet(NULL, 0); break; } + + // TODO: Visual signal "GDB stub idle" } + +ret: + // We're started after the first round of exchanges + gdb_started = true; } static void gdb_notifier_function(void) { // We ignore fxlink notifications when we're already inside GDB code. - if (ubc_dbh_lock || gdb_state != GDB_STATE_STARTED) + if (ubc_dbh_lock || !gdb_started) return; // We make sure we are called during a USB interrupt. @@ -585,7 +607,7 @@ static void gdb_notifier_function(void) static int gdb_panic_handler(uint32_t code) { - // If we are currently expecting to handle TLB misses + // Catch memory access errors from GDB trying to read/print stuff if (gdb_tlbh_enable) { // We only handle TLB miss reads (0x040) and writes (0x060) if (code != 0x040 && code != 0x060) @@ -598,11 +620,12 @@ static int gdb_panic_handler(uint32_t code) return 0; } // If we are in user code, let's break - else if (!ubc_dbh_lock && gdb_state == GDB_STATE_STARTED) { + else if (!ubc_dbh_lock) { // We make sure an other step break is not already set up if (gdb_single_step_backup.single_stepped) return 1; + // TODO: This only works for re-execution type exceptions uint32_t spc; __asm__("stc spc, %0" : "=r"(spc)); gdb_handle_single_step(spc, UBC_BREAK_BEFORE); @@ -613,32 +636,35 @@ static int gdb_panic_handler(uint32_t code) int gdb_start(void) { - if (gdb_state != GDB_STATE_STOPPED) { - return GDB_ALREADY_STARTED; + if (gdb_started) + return 0; + + if(usb_is_open() && !usb_is_open_interface(&usb_ff_bulk)) + usb_close(); + + if(!usb_is_open()) { + usb_interface_t const *interfaces[] = { &usb_ff_bulk, NULL }; + if(usb_open(interfaces, GINT_CALL_NULL) < 0) + return -1; + usb_open_wait(); } - if (usb_is_open()) { - return GDB_NO_INTERFACE; - } + usb_fxlink_set_notifier(gdb_notifier_function); + gdb_send_start(); if (!gdb_recv_buffer) { gdb_recv_buffer = malloc(gdb_recv_buffer_capacity); } - usb_interface_t const *interfaces[] = { &usb_ff_bulk, NULL }; - if (usb_open(interfaces, GINT_CALL_NULL) < 0) { - return GDB_USB_ERROR; - } - usb_open_wait(); - usb_fxlink_set_notifier(gdb_notifier_function); - // TODO : Should we detect if other panic or debug handlers are setup ? gint_exc_catch(gdb_panic_handler); - ubc_set_debug_handler(gdb_main); - gdb_state = GDB_STATE_FIRST_BREAK; - gdb_main(NULL); - gdb_state = GDB_STATE_STARTED; return 0; } + +void gdb_start_on_exception(void) +{ + gint_exc_catch(gdb_panic_handler); + ubc_set_debug_handler(gdb_main); +} diff --git a/src/usb/configure.c b/src/usb/configure.c index 324b582..4381c85 100644 --- a/src/usb/configure.c +++ b/src/usb/configure.c @@ -8,7 +8,8 @@ //--- /* Current configuration: list of interfaces and endpoint assignments */ -static usb_interface_t const *conf_if[16]; +#define MAX_INTERFACES 16 /* actually 15 */ +static usb_interface_t const *conf_if[MAX_INTERFACES]; static endpoint_t *conf_ep; GCONSTRUCTOR static void usb_alloc(void) @@ -86,7 +87,7 @@ static int find_pipe(int type) int usb_configure_solve(usb_interface_t const **interfaces) { /* Reset the previous configuration */ - memset(conf_if, 0, 16 * sizeof *conf_if); + memset(conf_if, 0, MAX_INTERFACES * sizeof *conf_if); memset(conf_ep, 0, 32 * sizeof *conf_ep); /* Next interface number to assign */ @@ -98,7 +99,8 @@ int usb_configure_solve(usb_interface_t const **interfaces) for(int i = 0; interfaces[i]; i++) { - if(i == 16) return USB_OPEN_TOO_MANY_INTERFACES; + if(i == MAX_INTERFACES - 1) + return USB_OPEN_TOO_MANY_INTERFACES; usb_interface_t const *intf = interfaces[i]; conf_if[next_interface++] = intf; @@ -180,7 +182,7 @@ void usb_configure_log(void) { #ifdef GINT_USB_DEBUG /* Log the final configuration */ - for(int i = 0; i < 16 && conf_if[i]; i++) + for(int i = 0; conf_if[i]; i++) USB_LOG("Interface #%d: %p\n", i, conf_if[i]); for(int i = 0; i < 32; i++) diff --git a/src/usb/usb.c b/src/usb/usb.c index 3d065b0..bfda448 100644 --- a/src/usb/usb.c +++ b/src/usb/usb.c @@ -229,6 +229,19 @@ bool usb_is_open(void) return usb_open_status; } +bool usb_is_open_interface(usb_interface_t const *interface) +{ + if(!usb_is_open()) + return false; + + usb_interface_t const * const *open = usb_configure_interfaces(); + for(int i = 0; open[i]; i++) { + if(open[i] == interface) + return true; + } + return false; +} + void usb_open_wait(void) { while(!usb_open_status) sleep();