Fix: noackmode zombie state handling

src/gdb_main.c

@@ -335,7 +335,8 @@ int32_t gdb_main_loop(target_controller_s *const tc, const gdb_packet_s *const p
 		}
 		if (packet->data[0] == 'D')
 			gdb_put_packet_ok();
-		gdb_set_noackmode(false);
+		else /* packet->data[0] == '\x04' */
+			gdb_set_noackmode(false);
 		break;
 
 	case 'k': /* Kill the target */
@@ -462,10 +463,15 @@ static void exec_q_supported(const char *packet, const size_t length)
 	(void)length;
 
 	/*
-	 * This is the first packet sent by GDB, so we can reset the NoAckMode flag here in case
-	 * the previous session was terminated abruptly with NoAckMode enabled
+	 * This might be the first packet of a GDB connection, If NoAckMode is enabled it might
+	 * be because the previous session was terminated abruptly, and we need to acknowledge
+	 * the first packet of the new session.
+	 * 
+	 * If NoAckMode was intentionally enabled before (e.g. LLDB enables NoAckMode first),
+	 * the acknowledgment should be safely ignored.
 	 */
-	gdb_set_noackmode(false);
+	if (gdb_noackmode())
+		gdb_packet_ack(true);
 
 	/*
 	 * The Remote Protocol documentation is not clear on what format the PacketSize feature should be in,
@@ -475,6 +481,16 @@ static void exec_q_supported(const char *packet, const size_t length)
 	gdb_putpacket_str_f("PacketSize=%x;qXfer:memory-map:read+;qXfer:features:read+;"
 						"vContSupported+" GDB_QSUPPORTED_NOACKMODE,
 		GDB_PACKET_BUFFER_SIZE);
+
+	/*
+	 * If an acknowledgement was received in response while in NoAckMode, then NoAckMode is probably
+	 * not meant to be enabled and is likely a result of the previous session being terminated
+	 * abruptly. Disable NoAckMode to prevent any further issues.
+	 */
+	if (gdb_noackmode() && gdb_packet_get_ack(100U)) {
+		DEBUG_GDB("Received acknowledgment in NoAckMode, likely result of a session being terminated abruptly\n");
+		gdb_set_noackmode(false);
+	}
 }
 
 static void exec_q_memory_map(const char *packet, const size_t length)
@@ -570,6 +586,13 @@ static void exec_q_noackmode(const char *packet, const size_t length)
 {
 	(void)packet;
 	(void)length;
+	/*
+	 * This might be the first packet of a LLDB connection, If NoAckMode is enabled it might
+	 * be because the previous session was terminated abruptly, and we need to acknowledge
+	 * the first packet of the new session.
+	 */
+	if (gdb_noackmode())
+		gdb_packet_ack(true);
 	gdb_set_noackmode(true);
 	gdb_put_packet_ok();
 }

src/gdb_packet.c

@@ -60,27 +60,29 @@ char *gdb_packet_buffer(void)
 #endif /* EXTERNAL_PACKET_BUFFER */
 
 /* https://sourceware.org/gdb/onlinedocs/gdb/Packet-Acknowledgment.html */
-void gdb_set_noackmode(bool enable)
+void gdb_set_noackmode(const bool enable)
 {
-	/*
-	 * If we were asked to disable NoAckMode, and it was previously enabled,
-	 * it might mean we got a packet we determined to be the first of a new
-	 * GDB session, and as such it was not acknowledged (before GDB enabled NoAckMode),
-	 * better late than never.
-	 *
-	 * If we were asked after the connection was terminated, sending the ack will have no effect.
-	 */
-	if (!enable && noackmode)
-		gdb_if_putchar(GDB_PACKET_ACK, true);
-
 	/* Log only changes */
 	if (noackmode != enable)
 		DEBUG_GDB("%s NoAckMode\n", enable ? "Enabling" : "Disabling");
 
 	noackmode = enable;
 }
 
+bool gdb_noackmode(void)
+{
+	return noackmode;
+}
+
 #ifndef DEBUG_GDB_IS_NOOP
+/*
+ * To debug packets from the perspective of GDB we can use the following command:
+ *     set debug remote on
+ * This will print packets sent and received by the GDB client
+ * 
+ * This is not directly related to BMD, but as it's hard to find this information
+ * and it is extremely useful for debugging, we included it here for reference.
+ */
 static void gdb_packet_debug(const char *const func, const gdb_packet_s *const packet)
 {
 	/* Log packet for debugging */
@@ -256,7 +258,7 @@ gdb_packet_s *gdb_packet_receive(void)
 
 				/* (N)Acknowledge packet */
 				const bool checksum_ok = gdb_packet_checksum(packet) == rx_checksum;
-				gdb_if_putchar(checksum_ok ? GDB_PACKET_ACK : GDB_PACKET_NACK, true);
+				gdb_packet_ack(checksum_ok);
 				if (!checksum_ok) {
 					/* Checksum error, restart packet capture */
 					state = PACKET_IDLE;
@@ -287,14 +289,19 @@ gdb_packet_s *gdb_packet_receive(void)
 	}
 }
 
-static inline bool gdb_get_ack(const uint32_t timeout)
+void gdb_packet_ack(const bool ack)
 {
-	/* Return true early if NoAckMode is enabled */
-	if (noackmode)
-		return true;
+	/* Send ACK/NACK */
+	DEBUG_GDB("%s: %s\n", __func__, ack ? "ACK" : "NACK");
+	gdb_if_putchar(ack ? GDB_PACKET_ACK : GDB_PACKET_NACK, true);
+}
 
+bool gdb_packet_get_ack(const uint32_t timeout)
+{
 	/* Wait for ACK/NACK */
-	return gdb_if_getchar_to(timeout) == GDB_PACKET_ACK;
+	const bool ack = gdb_if_getchar_to(timeout) == GDB_PACKET_ACK;
+	DEBUG_GDB("%s: %s\n", __func__, ack ? "ACK" : "NACK");
+	return ack;
 }
 
 static inline void gdb_if_putchar_escaped(const char value)
@@ -335,7 +342,7 @@ void gdb_packet_send(const gdb_packet_s *const packet)
 #endif
 
 		/* Wait for ACK/NACK on standard packets */
-		if (packet->notification || gdb_get_ack(2000U))
+		if (packet->notification || noackmode || gdb_packet_get_ack(2000U))
 			break;
 	}
 }

src/include/gdb_packet.h

@@ -70,11 +70,15 @@ typedef struct gdb_packet {
 
 /* GDB packet transmission configuration */
 void gdb_set_noackmode(bool enable);
+bool gdb_noackmode(void);
 
 /* Raw GDB packet transmission */
 gdb_packet_s *gdb_packet_receive(void);
 void gdb_packet_send(const gdb_packet_s *packet);
 
+void gdb_packet_ack(bool ack);
+bool gdb_packet_get_ack(uint32_t timeout);
+
 char *gdb_packet_buffer(void);
 
 /* Convenience wrappers */