From 2415a52afbf350f33915225b4c87cb89ed6c95ac Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Wed, 16 Mar 2022 16:56:41 +0000
Subject: [PATCH 1/7] Add support for avr-gdb's "info io_registers" command and
 also "monitor ior <base> <count>" to select only some registers for output.
 Selective cherry-pick of 6923a0ac705c773e663358c7231e36a10e453d19 and
 05b6803d390ac87c1b4deb9f296f7e39a20a6633.

---
 simavr/sim/sim_avr.h  |   2 +
 simavr/sim/sim_core.c |   4 +-
 simavr/sim/sim_gdb.c  | 157 ++++++++++++++++++++++++++++++++++++------
 3 files changed, 140 insertions(+), 23 deletions(-)

diff --git a/simavr/sim/sim_avr.h b/simavr/sim/sim_avr.h
index 710e12b1b..e9a97cde2 100644
--- a/simavr/sim/sim_avr.h
+++ b/simavr/sim/sim_avr.h
@@ -152,6 +152,8 @@ typedef void (*avr_run_t)(
 #define AVR_FUSE_HIGH	1
 #define AVR_FUSE_EXT	2
 
+#define REG_NAME_COUNT (256 + 32)       // Size of reg_names table.
+
 /*
  * Main AVR instance. Some of these fields are set by the AVR "Core" definition files
  * the rest is runtime data (as little as possible)
diff --git a/simavr/sim/sim_core.c b/simavr/sim/sim_core.c
index 4bb254334..4dae91ed5 100644
--- a/simavr/sim/sim_core.c
+++ b/simavr/sim/sim_core.c
@@ -321,7 +321,7 @@ avr_flashaddr_t _avr_pop_addr(avr_t * avr)
 /*
  * "Pretty" register names
  */
-const char * reg_names[255] = {
+const char * reg_names[REG_NAME_COUNT] = {
 		[R_XH] = "XH", [R_XL] = "XL",
 		[R_YH] = "YH", [R_YL] = "YL",
 		[R_ZH] = "ZH", [R_ZL] = "ZL",
@@ -330,7 +330,7 @@ const char * reg_names[255] = {
 };
 
 
-const char * avr_regname(uint8_t reg)
+const char * avr_regname(unsigned int reg)
 {
 	if (!reg_names[reg]) {
 		char tt[16];
diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index 5ea74a126..144c6ffa6 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -48,11 +48,16 @@ typedef struct {
 
 typedef struct avr_gdb_t {
 	avr_t * avr;
-	int		listen;	// listen socket
-	int		s;		// current gdb connection
+	int	listen;	// listen socket
+	int	s;	// current gdb connection
 
 	avr_gdb_watchpoints_t breakpoints;
 	avr_gdb_watchpoints_t watchpoints;
+
+	// These are used by gdb's "info io_registers" command.
+
+	uint16_t ior_base;
+	uint8_t  ior_count, mad;
 } avr_gdb_t;
 
 
@@ -218,7 +223,7 @@ gdb_change_breakpoint(
 		uint32_t addr,
 		uint32_t size )
 {
-	DBG(printf("set %d kind %d addr %08x len %d\n", set, kind, addr, len);)
+	DBG(printf("set %d kind %d addr %08x len %d\n", set, kind, addr, size);)
 
 	if (set) {
 		return gdb_watch_add_or_update(w, kind, addr, size);
@@ -281,6 +286,123 @@ gdb_read_register(
 	return strlen(rep);
 }
 
+
+static uint8_t
+handle_monitor(avr_t * avr, avr_gdb_t * g, char * cmd)
+{
+	char         *ip, *op;
+	unsigned int  c1, c2;
+	char          dehex[64];
+
+	if (*cmd++ != ',')
+		return 1;		// Bad format
+	for (op = dehex; op < dehex + (sizeof dehex - 1); ++op) {
+		if (!*cmd)
+			break;
+		if (sscanf(cmd, "%1x%1x", &c1, &c2) != 2)
+			return 2;	// Bad format
+		*op = (c1 << 4) + c2;
+		cmd += 2;
+	}
+	*op = '\0';
+	if (*cmd)
+		return 3;		// Too long
+	ip = dehex;
+	while (*ip) {
+		while (*ip == ' ' || *ip == '\t')
+			++ip;
+
+		if (strncmp(ip, "reset", 5) == 0) {
+			avr->state = cpu_StepDone;
+			avr_reset(avr);
+			ip += 5;
+		} else if (strncmp(ip, "halt", 4) == 0) {
+			avr->state = cpu_Stopped;
+			ip += 4;
+		} else if (strncmp(ip, "ior", 3) == 0) {
+			unsigned int base;
+			int          n, m, count;
+
+			// Format is "ior <base> <count>
+			// or just "ior" to reset.
+
+			ip += 3;
+			m = sscanf(ip, "%x %i%n", &base, &count, &n);
+			if (m <= 0) {
+				// Reset values.
+
+				g->ior_base = g->ior_count = 0;
+				n = 0;
+			} else if (m != 2) {
+				return 1;
+			} else {
+				if (count <= 0 || base + count + 32 > REG_NAME_COUNT ||
+					base + count + 32 > avr->ioend) {
+					return 4;	// bad value
+				}
+				g->ior_base = base;
+				g->ior_count = count;
+			}
+			ip += n;
+		} else {
+			return 5;
+		}
+	}
+	return 0;
+}
+
+static void
+handle_io_registers(avr_t * avr, avr_gdb_t * g, char * cmd)
+{
+	extern const char *avr_regname(unsigned int); // sim_core.c
+	char *       params;
+	char *       reply;
+	unsigned int addr, count;
+	char         buff[1024];
+
+	if (g->mad) {
+		/* For this command, gdb employs a streaming protocol,
+		 * with the command being repeated until the stub sends
+		 * an empy packet as terminator.  That makes no sense,
+		 * as the requests are sized to ensure the reply will
+		 * fit in a single packet.
+		 */
+
+		reply = "";
+		g->mad = 0;
+	} else {
+		params = cmd + 11;
+		if (sscanf(params, ":%x,%x", &addr, &count) == 2) {
+			int i;
+
+			// Send names and values.
+			addr += 32;
+			if (addr + count > avr->ioend)
+				count = avr->ioend + 1 - addr;
+			reply = buff;
+			for (i = 0; i < count; ++i) {
+				const char *name;
+
+				name = avr_regname(addr + i);
+				reply += sprintf(reply, "%s,%x;",
+						 name, avr->data[addr + i]);
+				if (reply > buff + sizeof buff - 20)
+					break;
+			}
+		} else {
+			// Send register count.
+
+			count = g->ior_count ? g->ior_count :
+						avr->ioend > REG_NAME_COUNT ?
+							REG_NAME_COUNT - 32 : avr->ioend - 32;
+			sprintf(buff, "%x", count);
+		}
+		reply = buff;
+		g->mad = 1;
+	}
+	gdb_send_reply(g, reply);
+}
+
 static void
 gdb_handle_command(
 		avr_gdb_t * g,
@@ -349,25 +471,18 @@ gdb_handle_command(
 					// all available registers.
 				}
 			} else if (strncmp(cmd, "Rcmd", 4) == 0) { // monitor command
-				char * args = strchr(cmd, ',');
-				if (args != NULL) {
-					args++;
-					while (args != 0x00) {
-						printf("%s",args);
-						if (strncmp(args, "7265736574", 10) == 0) { // reset matched
-							avr->state = cpu_StepDone;
-							avr_reset(avr);
-							args += 10;
-						} else if (strncmp(args, "68616c74", 8) == 0) { // halt matched
-							avr->state = cpu_Stopped;
-							args += 8;
-						} else if (strncmp(args, "20", 2) == 0) { // space matched
-							args += 2;
-						} else // no match - end
-							break;
-					}
+				uint8_t err = handle_monitor(avr, g, cmd + 4);
+				if (err) {
+					snprintf(rep, sizeof rep,
+						 "E%02x", err);
+					gdb_send_reply(g, rep);
+				} else {
+					gdb_send_reply(g, "OK");
 				}
-				gdb_send_reply(g, "OK");
+				break;
+			} else if (strncmp(cmd, "Ravr.io_reg", 11) == 0) {
+				handle_io_registers(avr, g, cmd);
+				break;
 			}
 			gdb_send_reply(g, "");
 			break;

From 25b69b25a0050a0f83d1ff8e39b6696be0827028 Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Wed, 16 Mar 2022 18:56:13 +0000
Subject: [PATCH 2/7] Support for gdb's "load" command, based on
 https://github.com/msquirogac/simavr/tree/fix-371 This implements the
 vFlashxxxx remote protocol commands and should fix issue #371 - "GDB Error on
 OS X: Remote target does not support flash erase".

---
 simavr/sim/sim_gdb.c | 102 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 94 insertions(+), 8 deletions(-)

diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index 144c6ffa6..702a2cd1e 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -33,6 +33,7 @@
 #include "avr_eeprom.h"
 #include "sim_gdb.h"
 
+// For debug printfs: "#define DBG(w) w"
 #define DBG(w)
 
 #define WATCH_LIMIT (32)
@@ -403,10 +404,84 @@ handle_io_registers(avr_t * avr, avr_gdb_t * g, char * cmd)
 	gdb_send_reply(g, reply);
 }
 
+static void
+handle_v(avr_t * avr, avr_gdb_t * g, char * cmd, int length)
+{
+	uint32_t  addr;
+	uint8_t  *src = NULL;
+	int       len, err = -1;
+
+	if (strncmp(cmd, "FlashErase", 10) == 0) {
+
+		sscanf(cmd, "%*[^:]:%x,%x", &addr, &len);
+		if (addr < avr->flashend) {
+			src = avr->flash + addr;
+			if (addr + len > avr->flashend)
+				len = avr->flashend - addr;
+			memset(src, 0xff, len);
+			DBG(printf("FlashErase: %x,%x\n", addr, len);) //Remove
+		} else {
+			err = 1;
+		}
+	} else if (strncmp(cmd, "FlashWrite", 10) == 0) {
+		if (sscanf(cmd, "%*[^:]:%x:%n", &addr, &len) != 1) {
+			err = 2;
+		} else {
+			DBG(printf("FlashWrite %x\n", addr);) //Remove
+			if  (len >= length) {
+				err = 99;
+			} else if (addr < avr->flashend) {
+				char    *end;
+				uint8_t *limit;
+
+				end = cmd + length;
+				cmd += len;
+				src = avr->flash + addr;
+				limit = avr->flash + avr->flashend;
+				for (; cmd < end && src < limit; ++cmd) {
+					int escaped = 0;
+
+					if (escaped) {
+						*src++ = *cmd ^ 0x20;
+						escaped = 0;
+					} else if (*cmd == '}') {
+						escaped = 1;
+					} else {
+						*src++ = *cmd;
+					}
+				}
+				addr = src - avr->flash; // Address of end.
+				if (addr > avr->codeend)
+					avr->codeend = addr;
+				if (cmd != end) {
+					DBG(printf("FlashWrite %ld bytes left!\n", end - cmd));
+				}
+			} else {
+				err = 1;
+			}
+		}
+	} else if (strncmp(cmd, "FlashDone", 9) == 0) {
+		DBG(printf("FlashDone\n");) //Remove
+	} else {
+		gdb_send_reply(g, "");
+		return;
+	}
+
+	if (err < 0) {
+		gdb_send_reply(g, "OK");
+	} else {
+		char b[32];
+
+		sprintf(b, "E %.2d", err);
+		gdb_send_reply(g, b);
+	}
+}
+
 static void
 gdb_handle_command(
 		avr_gdb_t * g,
-		char * cmd )
+		char      * cmd,
+		int         length)
 {
 	avr_t * avr = g->avr;
 	char rep[1024];
@@ -636,6 +711,9 @@ gdb_handle_command(
 			avr->state = cpu_Done;
 			gdb_send_reply(g, "OK");
 		}	break;
+		case 'v':
+			handle_v(avr, g, cmd, length);
+			break;
 		default:
 			gdb_send_reply(g, "");
 			break;
@@ -675,7 +753,7 @@ gdb_network_handler(
 		int i = 1;
 		setsockopt (g->s, IPPROTO_TCP, TCP_NODELAY, &i, sizeof (i));
 		g->avr->state = cpu_Stopped;
-		printf("%s connection opened\n", __FUNCTION__);
+		DBG(printf("%s connection opened\n", __FUNCTION__);)
 	}
 
 	if (g->s != -1 && FD_ISSET(g->s, &read_set)) {
@@ -684,7 +762,7 @@ gdb_network_handler(
 		ssize_t r = recv(g->s, buffer, sizeof(buffer)-1, 0);
 
 		if (r == 0) {
-			printf("%s connection closed\n", __FUNCTION__);
+			DBG(printf("%s connection closed\n", __FUNCTION__);)
 			close(g->s);
 			gdb_watch_clear(&g->breakpoints);
 			gdb_watch_clear(&g->watchpoints);
@@ -698,7 +776,14 @@ gdb_network_handler(
 			return 1;
 		}
 		buffer[r] = 0;
-	//	printf("%s: received %d bytes\n'%s'\n", __FUNCTION__, r, buffer);
+		DBG(
+			if (!strncmp("+$vFlashWrite", (char *)buffer, 13)) {
+				printf("%s: received Flashwrite command %ld bytes\n",
+					   __FUNCTION__, r);
+			} else {
+				printf("%s: received command %ld bytes\n'%s'\n",
+					   __FUNCTION__, r, buffer);
+			})
 	//	hdump("gdb", buffer, r);
 
 		uint8_t * src = buffer;
@@ -717,11 +802,12 @@ gdb_network_handler(
 				*end-- = 0;
 			*end = 0;
 			src++;
-			DBG(printf("GDB command = '%s'\n", src);)
-
+			DBG(
+				if (strncmp("vFlashWrite", (char *)src, 11))
+					printf("GDB command = '%s'\n", src);)
 			send(g->s, "+", 1, 0);
-
-			gdb_handle_command(g, (char*)src);
+			if (end > src)
+				gdb_handle_command(g, (char*)src, end - src);
 		}
 	}
 	return 1;

From 4eb0e42896fde15b3f6e6f34a2d3dce3c025217a Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Wed, 16 Mar 2022 19:20:43 +0000
Subject: [PATCH 3/7] Fix three problems with gdb support: 1. Do not report a
 TRAP signal on every stop, as it confuses gdb.    Initial symptom was failure
 to step into an ISR. 2. AVR was running after reset command. 3. Excessive
 processor activity (buzzing) while AVR stopped.

---
 simavr/sim/sim_avr.c |  3 +--
 simavr/sim/sim_gdb.c | 14 ++++++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/simavr/sim/sim_avr.c b/simavr/sim/sim_avr.c
index 6e43bf91f..872904031 100644
--- a/simavr/sim/sim_avr.c
+++ b/simavr/sim/sim_avr.c
@@ -286,7 +286,7 @@ void
 avr_callback_run_gdb(
 		avr_t * avr)
 {
-	avr_gdb_processor(avr, avr->state == cpu_Stopped);
+	avr_gdb_processor(avr, avr->state == cpu_Stopped ? 50000 : 0);
 
 	if (avr->state == cpu_Stopped)
 		return ;
@@ -331,7 +331,6 @@ avr_callback_run_gdb(
 	// if we were stepping, use this state to inform remote gdb
 	if (step)
 		avr->state = cpu_StepDone;
-
 }
 
 /*
diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index 702a2cd1e..d68aea246 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -210,9 +210,10 @@ gdb_send_quick_status(
 	READ_SREG_INTO(g->avr, sreg);
 
 	sprintf(cmd, "T%02x20:%02x;21:%02x%02x;22:%02x%02x%02x00;",
-		signal ? signal : 5, sreg,
+		signal, sreg,
 		g->avr->data[R_SPL], g->avr->data[R_SPH],
-		g->avr->pc & 0xff, (g->avr->pc>>8)&0xff, (g->avr->pc>>16)&0xff);
+		g->avr->pc & 0xff, (g->avr->pc >> 8) & 0xff,
+		(g->avr->pc >> 16) & 0xff);
 	gdb_send_reply(g, cmd);
 }
 
@@ -314,8 +315,8 @@ handle_monitor(avr_t * avr, avr_gdb_t * g, char * cmd)
 			++ip;
 
 		if (strncmp(ip, "reset", 5) == 0) {
-			avr->state = cpu_StepDone;
 			avr_reset(avr);
+			avr->state = cpu_Stopped;
 			ip += 5;
 		} else if (strncmp(ip, "halt", 4) == 0) {
 			avr->state = cpu_Stopped;
@@ -668,8 +669,8 @@ gdb_handle_command(
 			avr->state = cpu_Step;
 		}	break;
 		case 'r': {	// deprecated, suggested for AVRStudio compatibility
-			avr->state = cpu_StepDone;
 			avr_reset(avr);
+			avr->state = cpu_Stopped;
 		}	break;
 		case 'Z': 	// set clear break/watchpoint
 		case 'z': {
@@ -792,7 +793,8 @@ gdb_network_handler(
 		// control C -- lets send the guy a nice status packet
 		if (*src == 3) {
 			src++;
-			g->avr->state = cpu_StepDone;
+			gdb_send_quick_status(g, 2); // SIGINT
+			g->avr->state = cpu_Stopped;
 			printf("GDB hit control-c\n");
 		}
 		if (*src  == '$') {
@@ -862,7 +864,7 @@ avr_gdb_processor(
 	if (avr->state == cpu_Running &&
 			gdb_watch_find(&g->breakpoints, avr->pc) != -1) {
 		DBG(printf("avr_gdb_processor hit breakpoint at %08x\n", avr->pc);)
-		gdb_send_quick_status(g, 0);
+		gdb_send_quick_status(g, 5);
 		avr->state = cpu_Stopped;
 	} else if (avr->state == cpu_StepDone) {
 		gdb_send_quick_status(g, 0);

From 3723282bab35432adb1824339c719ac0fbcb346d Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Wed, 16 Mar 2022 21:42:34 +0000
Subject: [PATCH 4/7] Fix access (read/write) watchpoints and incorrect
 reporting of watchpoint stops (always reported as "awatch").  Fix the kill
 command and allow the simulator to continue running on detach. Add "monitor
 say" for debugging.

---
 simavr/sim/sim_gdb.c | 27 +++++++++++++++++++++------
 simavr/sim/sim_gdb.h |  2 +-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index d68aea246..40ed524d0 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -77,7 +77,6 @@ gdb_watch_find(
 			return i;
 		}
 	}
-
 	return -1;
 }
 
@@ -93,11 +92,11 @@ gdb_watch_find_range(
 	for (int i = 0; i < w->len; i++) {
 		if (w->points[i].addr > addr) {
 			return -1;
-		} else if (w->points[i].addr <= addr && addr < w->points[i].addr + w->points[i].size) {
+		} else if (w->points[i].addr <= addr &&
+				   addr < w->points[i].addr + w->points[i].size) {
 			return i;
 		}
 	}
-
 	return -1;
 }
 
@@ -111,6 +110,9 @@ gdb_watch_add_or_update(
 		uint32_t addr,
 		uint32_t size )
 {
+	if (kind == AVR_GDB_WATCH_ACCESS)
+		kind |= AVR_GDB_WATCH_WRITE | AVR_GDB_WATCH_READ;
+
 	/* If the watchpoint exists, update it. */
 	int i = gdb_watch_find(w, addr);
 	if (i != -1) {
@@ -346,6 +348,12 @@ handle_monitor(avr_t * avr, avr_gdb_t * g, char * cmd)
 				g->ior_count = count;
 			}
 			ip += n;
+	DBG(
+		} else if (strncmp(ip, "say ", 4) == 0) {
+			// Put a message in the debug output.
+			printf("Say: %s\n", ip + 4);
+			ip += strlen(ip);
+		)
 		} else {
 			return 5;
 		}
@@ -707,11 +715,16 @@ gdb_handle_command(
 					break;
 			}
 		}	break;
-		case 'K': 	// kill
-		case 'D': {	// detach
+		case 'k': 	// kill
 			avr->state = cpu_Done;
 			gdb_send_reply(g, "OK");
-		}	break;
+			break;
+		case 'D': 	// detach
+			avr->state = cpu_Running;
+			gdb_send_reply(g, "OK");
+			close(g->s);
+			g->s = -1;
+			break;
 		case 'v':
 			handle_v(avr, g, cmd, length);
 			break;
@@ -833,6 +846,8 @@ avr_gdb_handle_watchpoints(
 	}
 
 	int kind = g->watchpoints.points[i].kind;
+	DBG(printf("Addr %04x found watchpoint %d size %d type %x wanted %x\n",
+			   addr, i, g->watchpoints.points[i].size, kind, type);)
 	if (kind & type) {
 		/* Send gdb reply (see GDB user manual appendix E.3). */
 		char cmd[78];
diff --git a/simavr/sim/sim_gdb.h b/simavr/sim/sim_gdb.h
index 225291bb0..4ff643061 100644
--- a/simavr/sim/sim_gdb.h
+++ b/simavr/sim/sim_gdb.h
@@ -34,7 +34,7 @@ enum avr_gdb_watch_type {
 
 	AVR_GDB_WATCH_WRITE  = 1 << 2,
 	AVR_GDB_WATCH_READ   = 1 << 3,
-	AVR_GDB_WATCH_ACCESS = AVR_GDB_WATCH_WRITE | AVR_GDB_WATCH_READ,
+	AVR_GDB_WATCH_ACCESS = 1 << 4
 };
 
 int avr_gdb_init(avr_t * avr);

From ee7434d5f74210dda9615629dcb6f40ffc943783 Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Wed, 16 Mar 2022 22:05:49 +0000
Subject: [PATCH 5/7] Make the break instruction useful.  The previous
 implementation could never move past it, as gdb knows it is in ROM and will
 not replace it.  Distinguish hard and soft breaks.

---
 simavr/sim/sim_core.c | 12 +++---
 simavr/sim/sim_gdb.c  | 91 +++++++++++++++++++++++++++----------------
 simavr/sim/sim_gdb.h  |  1 +
 3 files changed, 64 insertions(+), 40 deletions(-)

diff --git a/simavr/sim/sim_core.c b/simavr/sim/sim_core.c
index 4dae91ed5..a1a9ac926 100644
--- a/simavr/sim/sim_core.c
+++ b/simavr/sim/sim_core.c
@@ -164,7 +164,8 @@ uint8_t avr_core_watch_read(avr_t *avr, uint16_t addr)
 				"CORE: *** Wrapping read address "
 				"PC=%04x SP=%04x O=%04x Address %04x %% %04x --> %04x\n"
 				FONT_DEFAULT,
-				avr->pc, _avr_sp_get(avr), _avr_flash_read16le(avr, avr->pc), addr, (avr->ramend + 1), addr % (avr->ramend + 1));
+				avr->pc, _avr_sp_get(avr), _avr_flash_read16le(avr, avr->pc),
+				addr, (avr->ramend + 1), addr % (avr->ramend + 1));
 		addr = addr % (avr->ramend + 1);
 	}
 
@@ -938,12 +939,9 @@ avr_flashaddr_t avr_run_one(avr_t * avr)
 				case 0x9598: { // BREAK -- 1001 0101 1001 1000
 					STATE("break\n");
 					if (avr->gdb) {
-						// if gdb is on, we break here as in here
-						// and we do so until gdb restores the instruction
-						// that was here before
-						avr->state = cpu_StepDone;
-						new_pc = avr->pc;
-						cycle = 0;
+						// if gdb is on, break here.
+						avr->state = cpu_Stopped;
+						avr_gdb_handle_break(avr);
 					}
 				}	break;
 				case 0x95a8: { // WDR -- Watchdog Reset -- 1001 0101 1010 1000
diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index 40ed524d0..6d6ef9752 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -201,22 +201,41 @@ gdb_send_reply(
 	send(g->s, reply, dst - reply + 3, 0);
 }
 
+static void
+gdb_send_stop_status(
+		avr_gdb_t  * g,
+		uint8_t     signal,
+		const char * reason,
+		uint32_t   * pp )
+{
+	avr_t   * avr;
+	uint8_t   sreg;
+	int       n;
+	char      cmd[64];
+
+	avr = g->avr;
+	READ_SREG_INTO(avr, sreg);
+
+	n = sprintf(cmd, "T%02x20:%02x;21:%02x%02x;22:%02x%02x%02x00;",
+				signal, sreg,
+				avr->data[R_SPL], avr->data[R_SPH],
+				avr->pc & 0xff, (avr->pc >> 8) & 0xff,
+				(avr->pc >> 16) & 0xff);
+	if (reason) {
+		if (pp)
+			sprintf(cmd + n, "%s:%x;", reason, *pp);
+		else
+			sprintf(cmd + n, "%s:;", reason);
+	}
+	gdb_send_reply(g, cmd);
+}
+
 static void
 gdb_send_quick_status(
 		avr_gdb_t * g,
 		uint8_t signal )
 {
-	char cmd[64];
-	uint8_t sreg;
-
-	READ_SREG_INTO(g->avr, sreg);
-
-	sprintf(cmd, "T%02x20:%02x;21:%02x%02x;22:%02x%02x%02x00;",
-		signal, sreg,
-		g->avr->data[R_SPL], g->avr->data[R_SPH],
-		g->avr->pc & 0xff, (g->avr->pc >> 8) & 0xff,
-		(g->avr->pc >> 16) & 0xff);
-	gdb_send_reply(g, cmd);
+	gdb_send_stop_status(g, signal, NULL, NULL);
 }
 
 static int
@@ -227,14 +246,14 @@ gdb_change_breakpoint(
 		uint32_t addr,
 		uint32_t size )
 {
-	DBG(printf("set %d kind %d addr %08x len %d\n", set, kind, addr, size);)
+	DBG(printf("%s kind %d addr %08x len %d\n", set ? "Set" : "Clear",
+			   kind, addr, size);)
 
 	if (set) {
 		return gdb_watch_add_or_update(w, kind, addr, size);
 	} else {
 		return gdb_watch_rm(w, kind, addr);
 	}
-
 	return -1;
 }
 
@@ -500,9 +519,9 @@ gdb_handle_command(
 			if (strncmp(cmd, "Supported", 9) == 0) {
 				/* If GDB asked what features we support, report back
 				 * the features we support, which is just memory layout
-				 * information for now.
+				 * information and stop reasons for now.
 				 */
-				gdb_send_reply(g, "qXfer:memory-map:read+");
+				gdb_send_reply(g, "qXfer:memory-map:read+;swbreak+;hwbreak+");
 				break;
 			} else if (strncmp(cmd, "Attached", 8) == 0) {
 				/* Respond that we are attached to an existing process..
@@ -715,16 +734,19 @@ gdb_handle_command(
 					break;
 			}
 		}	break;
-		case 'k': 	// kill
-			avr->state = cpu_Done;
-			gdb_send_reply(g, "OK");
-			break;
 		case 'D': 	// detach
-			avr->state = cpu_Running;
+#ifdef DETACHABLE
+			if (avr->state = cpu_Stopped)
+				avr->state = cpu_Running;
 			gdb_send_reply(g, "OK");
 			close(g->s);
 			g->s = -1;
 			break;
+#endif
+		case 'k': 	// kill
+			avr->state = cpu_Done;
+			gdb_send_reply(g, "OK");
+			break;
 		case 'v':
 			handle_v(avr, g, cmd, length);
 			break;
@@ -828,6 +850,14 @@ gdb_network_handler(
 	return 1;
 }
 
+/* Called on a hardware break instruction. */
+void avr_gdb_handle_break(avr_t *avr)
+{
+	avr_gdb_t *g = avr->gdb;
+
+	gdb_send_stop_status(g, 5, "hwbreak", NULL);
+}
+
 /**
  * If an applicable watchpoint exists for addr, stop the cpu and send a status report.
  * type is one of AVR_GDB_WATCH_READ, AVR_GDB_WATCH_WRITE depending on the type of access.
@@ -839,6 +869,7 @@ avr_gdb_handle_watchpoints(
 		enum avr_gdb_watch_type type )
 {
 	avr_gdb_t *g = avr->gdb;
+    uint32_t   false_addr;
 
 	int i = gdb_watch_find_range(&g->watchpoints, addr);
 	if (i == -1) {
@@ -850,19 +881,13 @@ avr_gdb_handle_watchpoints(
 			   addr, i, g->watchpoints.points[i].size, kind, type);)
 	if (kind & type) {
 		/* Send gdb reply (see GDB user manual appendix E.3). */
-		char cmd[78];
-		uint8_t sreg;
-
-		READ_SREG_INTO(g->avr, sreg);
-		sprintf(cmd, "T%02x20:%02x;21:%02x%02x;22:%02x%02x%02x00;%s:%06x;",
-				5, sreg,
-				g->avr->data[R_SPL], g->avr->data[R_SPH],
-				g->avr->pc & 0xff, (g->avr->pc>>8)&0xff, (g->avr->pc>>16)&0xff,
-				kind & AVR_GDB_WATCH_ACCESS ? "awatch" :
-					kind & AVR_GDB_WATCH_WRITE ? "watch" : "rwatch",
-				addr | 0x800000);
-		gdb_send_reply(g, cmd);
 
+		const char * what;
+
+		what = (kind & AVR_GDB_WATCH_ACCESS) ? "awatch" :
+			(kind & AVR_GDB_WATCH_WRITE) ? "watch" : "rwatch";
+		false_addr = addr + 0x800000;
+		gdb_send_stop_status(g, 5, what, &false_addr);
 		avr->state = cpu_Stopped;
 	}
 }
@@ -879,7 +904,7 @@ avr_gdb_processor(
 	if (avr->state == cpu_Running &&
 			gdb_watch_find(&g->breakpoints, avr->pc) != -1) {
 		DBG(printf("avr_gdb_processor hit breakpoint at %08x\n", avr->pc);)
-		gdb_send_quick_status(g, 5);
+		gdb_send_stop_status(g, 5, "hwbreak", NULL);
 		avr->state = cpu_Stopped;
 	} else if (avr->state == cpu_StepDone) {
 		gdb_send_quick_status(g, 0);
diff --git a/simavr/sim/sim_gdb.h b/simavr/sim/sim_gdb.h
index 4ff643061..2936df79e 100644
--- a/simavr/sim/sim_gdb.h
+++ b/simavr/sim/sim_gdb.h
@@ -46,6 +46,7 @@ int avr_gdb_processor(avr_t * avr, int sleep);
 
 // Called from sim_core.c
 void avr_gdb_handle_watchpoints(avr_t * g, uint16_t addr, enum avr_gdb_watch_type type);
+void avr_gdb_handle_break(avr_t *);
 
 #ifdef __cplusplus
 };

From 0fe7429065be9d583a89fc880da55e80eb9c65b8 Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Thu, 17 Mar 2022 08:02:22 +0000
Subject: [PATCH 6/7] Fix bugs in handling vFlashWrite in sim_gdb.c and reset
 the cycle counter on reset so that there is no crash on restart.

---
 simavr/sim/sim_avr.c |  1 +
 simavr/sim/sim_gdb.c | 27 +++++++++++++--------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/simavr/sim/sim_avr.c b/simavr/sim/sim_avr.c
index 872904031..4c2602ca9 100644
--- a/simavr/sim/sim_avr.c
+++ b/simavr/sim/sim_avr.c
@@ -179,6 +179,7 @@ avr_reset(
 			port->reset(port);
 		port = port->next;
 	}
+	avr->cycle = 0; // Prevent crash
 }
 
 void
diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index 6d6ef9752..b8c48b81f 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -455,20 +455,18 @@ handle_v(avr_t * avr, avr_gdb_t * g, char * cmd, int length)
 		if (sscanf(cmd, "%*[^:]:%x:%n", &addr, &len) != 1) {
 			err = 2;
 		} else {
-			DBG(printf("FlashWrite %x\n", addr);) //Remove
 			if  (len >= length) {
 				err = 99;
 			} else if (addr < avr->flashend) {
+				int      escaped;
 				char    *end;
 				uint8_t *limit;
 
-				end = cmd + length;
+				end = cmd + length - 1; // Ignore final '#'.
 				cmd += len;
 				src = avr->flash + addr;
 				limit = avr->flash + avr->flashend;
-				for (; cmd < end && src < limit; ++cmd) {
-					int escaped = 0;
-
+				for (escaped = 0; cmd < end && src < limit; ++cmd) {
 					if (escaped) {
 						*src++ = *cmd ^ 0x20;
 						escaped = 0;
@@ -478,8 +476,10 @@ handle_v(avr_t * avr, avr_gdb_t * g, char * cmd, int length)
 						*src++ = *cmd;
 					}
 				}
+				DBG(printf("FlashWrite %x, %ld bytes\n", addr,
+						   (src - avr->flash) - addr);)
 				addr = src - avr->flash; // Address of end.
-				if (addr > avr->codeend)
+				if (addr > avr->codeend) // Checked by sim_core.c
 					avr->codeend = addr;
 				if (cmd != end) {
 					DBG(printf("FlashWrite %ld bytes left!\n", end - cmd));
@@ -812,27 +812,26 @@ gdb_network_handler(
 			return 1;
 		}
 		buffer[r] = 0;
+
+		uint8_t * src = buffer;
+		while (*src == '+' || *src == '-')
+			src++;
 		DBG(
-			if (!strncmp("+$vFlashWrite", (char *)buffer, 13)) {
+			if (!strncmp("$vFlashWrite", (char *)src, 12)) {
 				printf("%s: received Flashwrite command %ld bytes\n",
 					   __FUNCTION__, r);
 			} else {
 				printf("%s: received command %ld bytes\n'%s'\n",
 					   __FUNCTION__, r, buffer);
 			})
-	//	hdump("gdb", buffer, r);
-
-		uint8_t * src = buffer;
-		while (*src == '+' || *src == '-')
-			src++;
+		// hdump("gdb", buffer, r);
 		// control C -- lets send the guy a nice status packet
 		if (*src == 3) {
 			src++;
 			gdb_send_quick_status(g, 2); // SIGINT
 			g->avr->state = cpu_Stopped;
 			printf("GDB hit control-c\n");
-		}
-		if (*src  == '$') {
+		} else if (*src == '$') {
 			// strip checksum
 			uint8_t * end = buffer + r - 1;
 			while (end > src && *end != '#')

From a5634af63899454b5c7a85aae7b5c547a06f43d8 Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
Date: Wed, 16 Mar 2022 15:49:39 +0000
Subject: [PATCH 7/7] Really fix handling of 'break' (testing blunder) and
 include a helpful message when it is executed.  Also add help message
 response for gdb's "monitor" command.

---
 simavr/sim/sim_gdb.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/simavr/sim/sim_gdb.c b/simavr/sim/sim_gdb.c
index b8c48b81f..ec950e698 100644
--- a/simavr/sim/sim_gdb.c
+++ b/simavr/sim/sim_gdb.c
@@ -309,13 +309,34 @@ gdb_read_register(
 	return strlen(rep);
 }
 
+static int tohex(const char *in, char *out, unsigned int len)
+{
+	int n = 0;
+
+	while (*in && n + 2 < len)
+		n += sprintf(out + n, "%02x", (uint8_t)*in++);
+	return n;
+}
+
+/* Send a message to the user. Gdb must be expecting a reply, otherwise this
+ * is ignored.
+ */
+
+static void message(avr_gdb_t * g, const char *m)
+{
+	char buff[256];
+
+	buff[0] = 'O';
+	tohex(m, buff + 1, sizeof buff - 1);
+	gdb_send_reply(g, buff);
+}
 
-static uint8_t
+static int
 handle_monitor(avr_t * avr, avr_gdb_t * g, char * cmd)
 {
 	char         *ip, *op;
 	unsigned int  c1, c2;
-	char          dehex[64];
+	char          dehex[128];
 
 	if (*cmd++ != ',')
 		return 1;		// Bad format
@@ -374,7 +395,10 @@ handle_monitor(avr_t * avr, avr_gdb_t * g, char * cmd)
 			ip += strlen(ip);
 		)
 		} else {
-			return 5;
+			tohex("Monitor subcommands are: ior halt reset" DBG(" say") "\n",
+				  dehex, sizeof dehex);
+			gdb_send_reply(g, dehex);
+			return -1;
 		}
 	}
 	return 0;
@@ -574,12 +598,12 @@ gdb_handle_command(
 					// all available registers.
 				}
 			} else if (strncmp(cmd, "Rcmd", 4) == 0) { // monitor command
-				uint8_t err = handle_monitor(avr, g, cmd + 4);
-				if (err) {
+				int err = handle_monitor(avr, g, cmd + 4);
+				if (err > 0) {
 					snprintf(rep, sizeof rep,
 						 "E%02x", err);
 					gdb_send_reply(g, rep);
-				} else {
+				} else if (err == 0) {
 					gdb_send_reply(g, "OK");
 				}
 				break;
@@ -854,7 +878,9 @@ void avr_gdb_handle_break(avr_t *avr)
 {
 	avr_gdb_t *g = avr->gdb;
 
-	gdb_send_stop_status(g, 5, "hwbreak", NULL);
+	message(g, "Simavr executed 'break' instruction.\n");
+	//gdb_send_stop_status(g, 5, "swbreak", NULL);  Correct but ignored!
+	gdb_send_quick_status(g, 5);
 }
 
 /**