gdbserver: gdb cmds returning failure on success
[openocd.git] / src / server / gdb_server.c
index fe916c2677c2373a9906e5243b030a5a421fe3fb..a84c618aff084177becbf0c85e9918e0a880c383 100644 (file)
@@ -2,7 +2,7 @@
  *   Copyright (C) 2005 by Dominic Rath                                    *
  *   Dominic.Rath@gmx.de                                                   *
  *                                                                         *
- *   Copyright (C) 2007,2008 Øyvind Harboe                                 *
+ *   Copyright (C) 2007-2010 Øyvind Harboe                                 *
  *   oyvind.harboe@zylin.com                                               *
  *                                                                         *
  *   Copyright (C) 2008 by Spencer Oliver                                  *
 #include "config.h"
 #endif
 
-#include "gdb_server.h"
-#include "target_request.h"
-#include "register.h"
+#include <target/breakpoints.h>
+#include <target/target_request.h>
+#include <target/register.h>
 #include "server.h"
-#include "flash.h"
-#include "image.h"
-#include "jtag.h"
+#include <flash/nor/core.h>
+#include "gdb_server.h"
+#include <target/image.h>
+#include <jtag/jtag.h>
+
+
+/**
+ * @file
+ * GDB server implementation.
+ *
+ * This implements the GDB Remote Serial Protocol, over TCP connections,
+ * giving GDB access to the JTAG or other hardware debugging facilities
+ * found in most modern embedded processors.
+ */
+
+/* private connection data for GDB */
+struct gdb_connection
+{
+       char buffer[GDB_BUFFER_SIZE];
+       char *buf_p;
+       int buf_cnt;
+       int ctrl_c;
+       enum target_state frontend_state;
+       struct image *vflash_image;
+       int closed;
+       int busy;
+       int noack_mode;
+       bool sync;      /* set flag to true if you want the next stepi to return immediately.
+                      allowing GDB to pick up a fresh set of register values from the target
+                      without modifying the target state. */
+       /* We delay reporting memory write errors until next step/continue or memory
+        * write. This improves performance of gdb load significantly as the GDB packet
+        * can be replied immediately and a new GDB packet will be ready without delay
+        * (ca. 10% or so...).
+        */
+       bool mem_write_error;
+};
 
 
 #if 0
@@ -45,29 +79,32 @@ static struct gdb_connection *current_gdb_connection;
 static int gdb_breakpoint_override;
 static enum breakpoint_type gdb_breakpoint_override_type;
 
-extern int gdb_error(connection_t *connection, int retval);
+static int gdb_error(struct connection *connection, int retval);
 static unsigned short gdb_port = 3333;
-static const char *DIGITS = "0123456789abcdef";
+static unsigned short gdb_port_next = 0;
+static const char DIGITS[16] = "0123456789abcdef";
 
 static void gdb_log_callback(void *priv, const char *file, unsigned line,
                const char *function, const char *string);
 
-/* number of gdb connections, mainly to supress gdb related debugging spam
+/* number of gdb connections, mainly to suppress gdb related debugging spam
  * in helper/log.c when no gdb connections are actually active */
 int gdb_actual_connections;
 
 /* set if we are sending a memory map to gdb
  * via qXfer:memory-map:read packet */
 /* enabled by default*/
-int gdb_use_memory_map = 1;
+static int gdb_use_memory_map = 1;
 /* enabled by default*/
-int gdb_flash_program = 1;
+static int gdb_flash_program = 1;
 
 /* if set, data aborts cause an error to be reported in memory read packets
- * see the code in gdb_read_memory_packet() for further explanations */
-int gdb_report_data_abort = 0;
+ * see the code in gdb_read_memory_packet() for further explanations.
+ * Disabled by default.
+ */
+static int gdb_report_data_abort;
 
-int gdb_last_signal(target_t *target)
+static int gdb_last_signal(struct target *target)
 {
        switch (target->debug_reason)
        {
@@ -87,7 +124,8 @@ int gdb_last_signal(target_t *target)
        }
 }
 
-int check_pending(connection_t *connection, int timeout_s, int *got_data)
+static int check_pending(struct connection *connection,
+               int timeout_s, int *got_data)
 {
        /* a non-blocking socket will block if there is 0 bytes available on the socket,
         * but return with as many bytes as are available immediately
@@ -128,7 +166,7 @@ int check_pending(connection_t *connection, int timeout_s, int *got_data)
        return ERROR_OK;
 }
 
-int gdb_get_char(connection_t *connection, int* next_char)
+static int gdb_get_char_inner(struct connection *connection, int* next_char)
 {
        struct gdb_connection *gdb_con = connection->priv;
        int retval = ERROR_OK;
@@ -136,22 +174,6 @@ int gdb_get_char(connection_t *connection, int* next_char)
 #ifdef _DEBUG_GDB_IO_
        char *debug_buffer;
 #endif
-
-       if (gdb_con->buf_cnt-- > 0)
-       {
-               *next_char = *(gdb_con->buf_p++);
-               if (gdb_con->buf_cnt > 0)
-                       connection->input_pending = 1;
-               else
-                       connection->input_pending = 0;
-
-#ifdef _DEBUG_GDB_IO_
-               LOG_DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char);
-#endif
-
-               return ERROR_OK;
-       }
-
        for (;;)
        {
                if (connection->service->type == CONNECTION_PIPE)
@@ -236,7 +258,51 @@ int gdb_get_char(connection_t *connection, int* next_char)
        return retval;
 }
 
-int gdb_putback_char(connection_t *connection, int last_char)
+/**
+ * The cool thing about this fn is that it allows buf_p and buf_cnt to be
+ * held in registers in the inner loop.
+ *
+ * For small caches and embedded systems this is important!
+ */
+static inline int gdb_get_char_fast(struct connection *connection, int* next_char, char **buf_p, int *buf_cnt)
+{
+       int retval = ERROR_OK;
+
+       if ((*buf_cnt)-- > 0)
+       {
+               *next_char = **buf_p;
+               (*buf_p)++;
+               if (*buf_cnt > 0)
+                       connection->input_pending = 1;
+               else
+                       connection->input_pending = 0;
+
+#ifdef _DEBUG_GDB_IO_
+               LOG_DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char);
+#endif
+
+               return ERROR_OK;
+       }
+
+       struct gdb_connection *gdb_con = connection->priv;
+       gdb_con->buf_p = *buf_p;
+       gdb_con->buf_cnt = *buf_cnt;
+       retval = gdb_get_char_inner(connection, next_char);
+       *buf_p = gdb_con->buf_p;
+       *buf_cnt = gdb_con->buf_cnt;
+
+       return retval;
+}
+
+
+static int gdb_get_char(struct connection *connection, int* next_char)
+{
+       struct gdb_connection *gdb_con = connection->priv;
+       return gdb_get_char_fast(connection, next_char, &gdb_con->buf_p, &gdb_con->buf_cnt);
+}
+
+
+static int gdb_putback_char(struct connection *connection, int last_char)
 {
        struct gdb_connection *gdb_con = connection->priv;
 
@@ -256,7 +322,7 @@ int gdb_putback_char(connection_t *connection, int last_char)
 /* The only way we can detect that the socket is closed is the first time
  * we write to it, we will fail. Subsequent write operations will
  * succeed. Shudder! */
-int gdb_write(connection_t *connection, void *data, int len)
+static int gdb_write(struct connection *connection, void *data, int len)
 {
        struct gdb_connection *gdb_con = connection->priv;
        if (gdb_con->closed)
@@ -281,7 +347,8 @@ int gdb_write(connection_t *connection, void *data, int len)
        return ERROR_SERVER_REMOTE_CLOSED;
 }
 
-int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
+static int gdb_put_packet_inner(struct connection *connection,
+               char *buffer, int len)
 {
        int i;
        unsigned char my_checksum = 0;
@@ -304,7 +371,8 @@ int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
        int gotdata;
        for (;;)
        {
-               if ((retval = check_pending(connection, 0, &gotdata)) != ERROR_OK)
+               retval = check_pending(connection, 0, &gotdata);
+               if (retval != ERROR_OK)
                        return retval;
                if (!gotdata)
                        break;
@@ -422,7 +490,7 @@ int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
        return ERROR_OK;
 }
 
-int gdb_put_packet(connection_t *connection, char *buffer, int len)
+static int gdb_put_packet(struct connection *connection, char *buffer, int len)
 {
        struct gdb_connection *gdb_con = connection->priv;
        gdb_con->busy = 1;
@@ -435,32 +503,38 @@ int gdb_put_packet(connection_t *connection, char *buffer, int len)
        return retval;
 }
 
-static __inline__ int fetch_packet(connection_t *connection, int *checksum_ok, int noack, int *len, char *buffer)
+static __inline__ int fetch_packet(struct connection *connection, int *checksum_ok, int noack, int *len, char *buffer)
 {
        unsigned char my_checksum = 0;
        char checksum[3];
        int character;
-       int retval;
+       int retval = ERROR_OK;
 
        struct gdb_connection *gdb_con = connection->priv;
        my_checksum = 0;
        int count = 0;
        count = 0;
+
+       /* move this over into local variables to use registers and give the
+        * more freedom to optimize */
+       char *buf_p = gdb_con->buf_p;
+       int buf_cnt = gdb_con->buf_cnt;
+
        for (;;)
        {
                /* The common case is that we have an entire packet with no escape chars.
                 * We need to leave at least 2 bytes in the buffer to have
                 * gdb_get_char() update various bits and bobs correctly.
                 */
-               if ((gdb_con->buf_cnt > 2) && ((gdb_con->buf_cnt + count) < *len))
+               if ((buf_cnt > 2) && ((buf_cnt + count) < *len))
                {
                        /* The compiler will struggle a bit with constant propagation and
                         * aliasing, so we help it by showing that these values do not
                         * change inside the loop
                         */
                        int i;
-                       char *buf = gdb_con->buf_p;
-                       int run = gdb_con->buf_cnt - 2;
+                       char *buf = buf_p;
+                       int run = buf_cnt - 2;
                        i = 0;
                        int done = 0;
                        while (i < run)
@@ -492,19 +566,21 @@ static __inline__ int fetch_packet(connection_t *connection, int *checksum_ok, i
                                        buffer[count++] = character & 0xff;
                                }
                        }
-                       gdb_con->buf_p += i;
-                       gdb_con->buf_cnt -= i;
+                       buf_p += i;
+                       buf_cnt -= i;
                        if (done)
                                break;
                }
                if (count > *len)
                {
                        LOG_ERROR("packet buffer too small");
-                       return ERROR_GDB_BUFFER_TOO_SMALL;
+                       retval = ERROR_GDB_BUFFER_TOO_SMALL;
+                       break;
                }
 
-               if ((retval = gdb_get_char(connection, &character)) != ERROR_OK)
-                       return retval;
+               retval = gdb_get_char_fast(connection, &character, &buf_p, &buf_cnt);
+               if (retval != ERROR_OK)
+                       break;
 
                if (character == '#')
                        break;
@@ -514,8 +590,11 @@ static __inline__ int fetch_packet(connection_t *connection, int *checksum_ok, i
                        /* data transmitted in binary mode (X packet)
                         * uses 0x7d as escape character */
                        my_checksum += character & 0xff;
-                       if ((retval = gdb_get_char(connection, &character)) != ERROR_OK)
-                               return retval;
+
+                       retval = gdb_get_char_fast(connection, &character, &buf_p, &buf_cnt);
+                       if (retval != ERROR_OK)
+                               break;
+
                        my_checksum += character & 0xff;
                        buffer[count++] = (character ^ 0x20) & 0xff;
                }
@@ -526,6 +605,12 @@ static __inline__ int fetch_packet(connection_t *connection, int *checksum_ok, i
                }
        }
 
+       gdb_con->buf_p = buf_p;
+       gdb_con->buf_cnt = buf_cnt;
+
+       if (retval != ERROR_OK)
+               return retval;
+
        *len = count;
 
        if ((retval = gdb_get_char(connection, &character)) != ERROR_OK)
@@ -544,7 +629,8 @@ static __inline__ int fetch_packet(connection_t *connection, int *checksum_ok, i
        return ERROR_OK;
 }
 
-int gdb_get_packet_inner(connection_t *connection, char *buffer, int *len)
+static int gdb_get_packet_inner(struct connection *connection,
+               char *buffer, int *len)
 {
        int character;
        int retval;
@@ -567,7 +653,7 @@ int gdb_get_packet_inner(connection_t *connection, char *buffer, int *len)
                                        break;
                                case '+':
                                        /* gdb sends a dummy ack '+' at every remote connect - see remote_start_remote (remote.c)
-                                        * incase anyone tries to debug why they receive this warning every time */
+                                        * in case anyone tries to debug why they receive this warning every time */
                                        LOG_WARNING("acknowledgment received, but no packet pending");
                                        break;
                                case '-':
@@ -619,7 +705,7 @@ int gdb_get_packet_inner(connection_t *connection, char *buffer, int *len)
        return ERROR_OK;
 }
 
-int gdb_get_packet(connection_t *connection, char *buffer, int *len)
+static int gdb_get_packet(struct connection *connection, char *buffer, int *len)
 {
        struct gdb_connection *gdb_con = connection->priv;
        gdb_con->busy = 1;
@@ -628,7 +714,7 @@ int gdb_get_packet(connection_t *connection, char *buffer, int *len)
        return retval;
 }
 
-int gdb_output_con(connection_t *connection, const char* line)
+static int gdb_output_con(struct connection *connection, const char* line)
 {
        char *hex_buffer;
        int i, bin_size;
@@ -650,7 +736,7 @@ int gdb_output_con(connection_t *connection, const char* line)
        return retval;
 }
 
-int gdb_output(struct command_context_s *context, const char* line)
+static int gdb_output(struct command_context *context, const char* line)
 {
        /* this will be dumped to the log and also sent as an O packet if possible */
        LOG_USER_N("%s", line);
@@ -658,7 +744,7 @@ int gdb_output(struct command_context_s *context, const char* line)
 }
 
 
-static void gdb_frontend_halted(struct target_s *target, connection_t *connection)
+static void gdb_frontend_halted(struct target *target, struct connection *connection)
 {
        struct gdb_connection *gdb_connection = connection->priv;
 
@@ -695,10 +781,11 @@ static void gdb_frontend_halted(struct target_s *target, connection_t *connectio
        }
 }
 
-int gdb_target_callback_event_handler(struct target_s *target, enum target_event event, void *priv)
+static int gdb_target_callback_event_handler(struct target *target,
+               enum target_event event, void *priv)
 {
        int retval;
-       connection_t *connection = priv;
+       struct connection *connection = priv;
 
        target_handle_event(target, event);
        switch (event)
@@ -723,7 +810,7 @@ int gdb_target_callback_event_handler(struct target_s *target, enum target_event
        return ERROR_OK;
 }
 
-int gdb_new_connection(connection_t *connection)
+static int gdb_new_connection(struct connection *connection)
 {
        struct gdb_connection *gdb_connection = malloc(sizeof(struct gdb_connection));
        struct gdb_service *gdb_service = connection->service->priv;
@@ -742,6 +829,7 @@ int gdb_new_connection(connection_t *connection)
        gdb_connection->busy = 0;
        gdb_connection->noack_mode = 0;
        gdb_connection->sync = true;
+       gdb_connection->mem_write_error = false;
 
        /* send ACK to GDB for debug request */
        gdb_write(connection, "+", 1);
@@ -770,16 +858,36 @@ int gdb_new_connection(connection_t *connection)
                gdb_putback_char(connection, initial_ack);
        target_call_event_callbacks(gdb_service->target, TARGET_EVENT_GDB_ATTACH);
 
+       if (gdb_use_memory_map)
+       {
+               /* Connect must fail if the memory map can't be set up correctly.
+                *
+                * This will cause an auto_probe to be invoked, which is either
+                * a no-op or it will fail when the target isn't ready(e.g. not halted).
+                */
+               int i;
+               for (i = 0; i < flash_get_bank_count(); i++)
+               {
+                       struct flash_bank *p;
+                       retval = get_flash_bank_by_num(i, &p);
+                       if (retval != ERROR_OK)
+                       {
+                               LOG_ERROR("Connect failed. Consider setting up a gdb-attach event for the target to prepare target for GDB connect.");
+                               return retval;
+                       }
+               }
+       }
+
        gdb_actual_connections++;
        LOG_DEBUG("New GDB Connection: %d, Target %s, state: %s",
                  gdb_actual_connections,
-                 gdb_service->target->cmd_name,
+                 target_name(gdb_service->target),
                  target_state_name(gdb_service->target));
 
        return ERROR_OK;
 }
 
-int gdb_connection_closed(connection_t *connection)
+static int gdb_connection_closed(struct connection *connection)
 {
        struct gdb_service *gdb_service = connection->service->priv;
        struct gdb_connection *gdb_connection = connection->priv;
@@ -791,7 +899,7 @@ int gdb_connection_closed(connection_t *connection)
 
        gdb_actual_connections--;
        LOG_DEBUG("GDB Close, Target: %s, state: %s, gdb_actual_connections=%d",
-                 gdb_service->target->cmd_name,
+                 target_name(gdb_service->target),
                  target_state_name(gdb_service->target),
                  gdb_actual_connections);
 
@@ -826,14 +934,15 @@ int gdb_connection_closed(connection_t *connection)
        return ERROR_OK;
 }
 
-void gdb_send_error(connection_t *connection, uint8_t the_error)
+static void gdb_send_error(struct connection *connection, uint8_t the_error)
 {
        char err[4];
        snprintf(err, 4, "E%2.2X", the_error);
        gdb_put_packet(connection, err, 3);
 }
 
-int gdb_last_signal_packet(connection_t *connection, target_t *target, char* packet, int packet_size)
+static int gdb_last_signal_packet(struct connection *connection,
+               struct target *target, char* packet, int packet_size)
 {
        char sig_reply[4];
        int signal;
@@ -846,7 +955,7 @@ int gdb_last_signal_packet(connection_t *connection, target_t *target, char* pac
        return ERROR_OK;
 }
 
-static int gdb_reg_pos(target_t *target, int pos, int len)
+static int gdb_reg_pos(struct target *target, int pos, int len)
 {
        if (target->endianness == TARGET_LITTLE_ENDIAN)
                return pos;
@@ -858,19 +967,20 @@ static int gdb_reg_pos(target_t *target, int pos, int len)
  * register might be non-divisible by 8(a byte), in which
  * case an entire byte is shown.
  *
- * NB! the format on the wire is the target endianess
+ * NB! the format on the wire is the target endianness
  *
  * The format of reg->value is little endian
  *
  */
-void gdb_str_to_target(target_t *target, char *tstr, reg_t *reg)
+static void gdb_str_to_target(struct target *target,
+               char *tstr, struct reg *reg)
 {
        int i;
 
        uint8_t *buf;
        int buf_len;
        buf = reg->value;
-       buf_len = CEIL(reg->size, 8);
+       buf_len = DIV_ROUND_UP(reg->size, 8);
 
        for (i = 0; i < buf_len; i++)
        {
@@ -880,7 +990,7 @@ void gdb_str_to_target(target_t *target, char *tstr, reg_t *reg)
        }
 }
 
-static int hextoint(char c)
+static int hextoint(int c)
 {
        if (c>='0'&&c<='9')
        {
@@ -896,7 +1006,8 @@ static int hextoint(char c)
 }
 
 /* copy over in register buffer */
-void gdb_target_to_reg(target_t *target, char *tstr, int str_len, uint8_t *bin)
+static void gdb_target_to_reg(struct target *target,
+               char *tstr, int str_len, uint8_t *bin)
 {
        if (str_len % 2)
        {
@@ -915,9 +1026,10 @@ void gdb_target_to_reg(target_t *target, char *tstr, int str_len, uint8_t *bin)
        }
 }
 
-int gdb_get_registers_packet(connection_t *connection, target_t *target, char* packet, int packet_size)
+static int gdb_get_registers_packet(struct connection *connection,
+               struct target *target, char* packet, int packet_size)
 {
-       reg_t **reg_list;
+       struct reg **reg_list;
        int reg_list_size;
        int retval;
        int reg_packet_size = 0;
@@ -939,25 +1051,25 @@ int gdb_get_registers_packet(connection_t *connection, target_t *target, char* p
                reg_packet_size += reg_list[i]->size;
        }
 
-       reg_packet = malloc(CEIL(reg_packet_size, 8) * 2);
+       reg_packet = malloc(DIV_ROUND_UP(reg_packet_size, 8) * 2);
        reg_packet_p = reg_packet;
 
        for (i = 0; i < reg_list_size; i++)
        {
                gdb_str_to_target(target, reg_packet_p, reg_list[i]);
-               reg_packet_p += CEIL(reg_list[i]->size, 8) * 2;
+               reg_packet_p += DIV_ROUND_UP(reg_list[i]->size, 8) * 2;
        }
 
 #ifdef _DEBUG_GDB_IO_
        {
                char *reg_packet_p;
-               reg_packet_p = strndup(reg_packet, CEIL(reg_packet_size, 8) * 2);
+               reg_packet_p = strndup(reg_packet, DIV_ROUND_UP(reg_packet_size, 8) * 2);
                LOG_DEBUG("reg_packet: %s", reg_packet_p);
                free(reg_packet_p);
        }
 #endif
 
-       gdb_put_packet(connection, reg_packet, CEIL(reg_packet_size, 8) * 2);
+       gdb_put_packet(connection, reg_packet, DIV_ROUND_UP(reg_packet_size, 8) * 2);
        free(reg_packet);
 
        free(reg_list);
@@ -965,10 +1077,11 @@ int gdb_get_registers_packet(connection_t *connection, target_t *target, char* p
        return ERROR_OK;
 }
 
-int gdb_set_registers_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_set_registers_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        int i;
-       reg_t **reg_list;
+       struct reg **reg_list;
        int reg_list_size;
        int retval;
        char *packet_p;
@@ -996,21 +1109,17 @@ int gdb_set_registers_packet(connection_t *connection, target_t *target, char *p
        for (i = 0; i < reg_list_size; i++)
        {
                uint8_t *bin_buf;
-               int chars = (CEIL(reg_list[i]->size, 8) * 2);
+               int chars = (DIV_ROUND_UP(reg_list[i]->size, 8) * 2);
 
                if (packet_p + chars > packet + packet_size)
                {
                        LOG_ERROR("BUG: register packet is too small for registers");
                }
 
-               reg_arch_type_t *arch_type;
-               bin_buf = malloc(CEIL(reg_list[i]->size, 8));
+               bin_buf = malloc(DIV_ROUND_UP(reg_list[i]->size, 8));
                gdb_target_to_reg(target, packet_p, chars, bin_buf);
 
-               /* get register arch_type, and call set method */
-               arch_type = register_get_arch_type(reg_list[i]->arch_type);
-
-               arch_type->set(reg_list[i], bin_buf);
+               reg_list[i]->type->set(reg_list[i], bin_buf);
 
                /* advance packet pointer */
                packet_p += chars;
@@ -1019,7 +1128,7 @@ int gdb_set_registers_packet(connection_t *connection, target_t *target, char *p
                free(bin_buf);
        }
 
-       /* free reg_t *reg_list[] array allocated by get_gdb_reg_list */
+       /* free struct reg *reg_list[] array allocated by get_gdb_reg_list */
        free(reg_list);
 
        gdb_put_packet(connection, "OK", 2);
@@ -1027,11 +1136,12 @@ int gdb_set_registers_packet(connection_t *connection, target_t *target, char *p
        return ERROR_OK;
 }
 
-int gdb_get_register_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_get_register_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        char *reg_packet;
        int reg_num = strtoul(packet + 1, NULL, 16);
-       reg_t **reg_list;
+       struct reg **reg_list;
        int reg_list_size;
        int retval;
 
@@ -1050,11 +1160,11 @@ int gdb_get_register_packet(connection_t *connection, target_t *target, char *pa
                exit(-1);
        }
 
-       reg_packet = malloc(CEIL(reg_list[reg_num]->size, 8) * 2);
+       reg_packet = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2);
 
        gdb_str_to_target(target, reg_packet, reg_list[reg_num]);
 
-       gdb_put_packet(connection, reg_packet, CEIL(reg_list[reg_num]->size, 8) * 2);
+       gdb_put_packet(connection, reg_packet, DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2);
 
        free(reg_list);
        free(reg_packet);
@@ -1062,15 +1172,15 @@ int gdb_get_register_packet(connection_t *connection, target_t *target, char *pa
        return ERROR_OK;
 }
 
-int gdb_set_register_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_set_register_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        char *separator;
        uint8_t *bin_buf;
        int reg_num = strtoul(packet + 1, &separator, 16);
-       reg_t **reg_list;
+       struct reg **reg_list;
        int reg_list_size;
        int retval;
-       reg_arch_type_t *arch_type;
 
        LOG_DEBUG("-");
 
@@ -1092,16 +1202,14 @@ int gdb_set_register_packet(connection_t *connection, target_t *target, char *pa
        }
 
        /* convert from GDB-string (target-endian) to hex-string (big-endian) */
-       bin_buf = malloc(CEIL(reg_list[reg_num]->size, 8));
-       int chars = (CEIL(reg_list[reg_num]->size, 8) * 2);
+       bin_buf = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8));
+       int chars = (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2);
 
        /* fix!!! add some sanity checks on packet size here */
 
        gdb_target_to_reg(target, separator + 1, chars, bin_buf);
 
-               /* get register arch_type, and call set method */
-       arch_type = register_get_arch_type(reg_list[reg_num]->arch_type);
-       arch_type->set(reg_list[reg_num], bin_buf);
+       reg_list[reg_num]->type->set(reg_list[reg_num], bin_buf);
 
        gdb_put_packet(connection, "OK", 2);
 
@@ -1111,7 +1219,7 @@ int gdb_set_register_packet(connection_t *connection, target_t *target, char *pa
        return ERROR_OK;
 }
 
-int gdb_error(connection_t *connection, int retval)
+static int gdb_error(struct connection *connection, int retval)
 {
        switch (retval)
        {
@@ -1142,7 +1250,8 @@ int gdb_error(connection_t *connection, int retval)
  *
  * 8191 bytes by the looks of it. Why 8191 bytes instead of 8192?????
  */
-int gdb_read_memory_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_read_memory_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        char *separator;
        uint32_t addr = 0;
@@ -1216,7 +1325,8 @@ int gdb_read_memory_packet(connection_t *connection, target_t *target, char *pac
        return retval;
 }
 
-int gdb_write_memory_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_write_memory_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        char *separator;
        uint32_t addr = 0;
@@ -1273,13 +1383,14 @@ int gdb_write_memory_packet(connection_t *connection, target_t *target, char *pa
        return retval;
 }
 
-int gdb_write_memory_binary_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_write_memory_binary_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        char *separator;
        uint32_t addr = 0;
        uint32_t len = 0;
 
-       int retval;
+       int retval = ERROR_OK;
 
        /* skip command character */
        packet++;
@@ -1300,14 +1411,18 @@ int gdb_write_memory_binary_packet(connection_t *connection, target_t *target, c
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
-       retval = ERROR_OK;
-       if (len)
-       {
-               LOG_DEBUG("addr: 0x%8.8" PRIx32 ", len: 0x%8.8" PRIx32 "", addr, len);
+       struct gdb_connection *gdb_connection = connection->priv;
 
-               retval = target_write_buffer(target, addr, len, (uint8_t*)separator);
+       if (gdb_connection->mem_write_error)
+       {
+               retval = ERROR_FAIL;
+               /* now that we have reported the memory write error, we can clear the condition */
+               gdb_connection->mem_write_error = false;
        }
 
+       /* By replying the packet *immediately* GDB will send us a new packet
+        * while we write the last one to the target.
+        */
        if (retval == ERROR_OK)
        {
                gdb_put_packet(connection, "OK", 2);
@@ -1318,10 +1433,22 @@ int gdb_write_memory_binary_packet(connection_t *connection, target_t *target, c
                        return retval;
        }
 
+       if (len)
+       {
+               LOG_DEBUG("addr: 0x%8.8" PRIx32 ", len: 0x%8.8" PRIx32 "", addr, len);
+
+               retval = target_write_buffer(target, addr, len, (uint8_t*)separator);
+               if (retval != ERROR_OK)
+               {
+                       gdb_connection->mem_write_error = true;
+               }
+       }
+
        return ERROR_OK;
 }
 
-int gdb_step_continue_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_step_continue_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        int current = 0;
        uint32_t address = 0x0;
@@ -1354,11 +1481,12 @@ int gdb_step_continue_packet(connection_t *connection, target_t *target, char *p
        return retval;
 }
 
-int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_breakpoint_watchpoint_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        int type;
        enum breakpoint_type bp_type = BKPT_SOFT /* dummy init to avoid warning */;
-       enum watchpoint_rw wp_type;
+       enum watchpoint_rw wp_type = WPT_READ /* dummy init to avoid warning */;
        uint32_t address;
        uint32_t size;
        char *separator;
@@ -1378,6 +1506,12 @@ int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target,
                wp_type = WPT_READ;
        else if (type == 4) /* access watchpoint */
                wp_type = WPT_ACCESS;
+       else
+       {
+               LOG_ERROR("invalid gdb watch/breakpoint type(%d), dropping connection", type);
+               return ERROR_SERVER_REMOTE_CLOSED;
+       }
+
 
        if (gdb_breakpoint_override && ((bp_type == BKPT_SOFT)||(bp_type == BKPT_HARD)))
        {
@@ -1428,7 +1562,7 @@ int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target,
                {
                        if (packet[0] == 'Z')
                        {
-                               if ((retval = watchpoint_add(target, address, size, type-2, 0, 0xffffffffu)) != ERROR_OK)
+                               if ((retval = watchpoint_add(target, address, size, wp_type, 0, 0xffffffffu)) != ERROR_OK)
                                {
                                        if ((retval = gdb_error(connection, retval)) != ERROR_OK)
                                                return retval;
@@ -1452,8 +1586,11 @@ int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target,
        return ERROR_OK;
 }
 
-/* print out a string and allocate more space as needed, mainly used for XML at this point */
-void xml_printf(int *retval, char **xml, int *pos, int *size, const char *fmt, ...)
+/* print out a string and allocate more space as needed,
+ * mainly used for XML at this point
+ */
+static void xml_printf(int *retval, char **xml, int *pos, int *size,
+               const char *fmt, ...)
 {
        if (*retval != ERROR_OK)
        {
@@ -1520,27 +1657,11 @@ static int decode_xfer_read(char *buf, char **annex, int *ofs, unsigned int *len
        return 0;
 }
 
-int gdb_calc_blocksize(flash_bank_t *bank)
-{
-       uint32_t i;
-       uint32_t block_size = 0xffffffff;
-
-       /* loop through all sectors and return smallest sector size */
-
-       for (i = 0; i < (uint32_t)bank->num_sectors; i++)
-       {
-               if (bank->sectors[i].size < block_size)
-                       block_size = bank->sectors[i].size;
-       }
-
-       return block_size;
-}
-
 static int compare_bank (const void * a, const void * b)
 {
-       flash_bank_t *b1, *b2;
-       b1=*((flash_bank_t **)a);
-       b2=*((flash_bank_t **)b);
+       struct flash_bank *b1, *b2;
+       b1=*((struct flash_bank **)a);
+       b2=*((struct flash_bank **)b);
 
        if (b1->base == b2->base)
        {
@@ -1554,9 +1675,153 @@ static int compare_bank (const void * a, const void * b)
        }
 }
 
-int gdb_query_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_memory_map(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
-       command_context_t *cmd_ctx = connection->cmd_ctx;
+       /* We get away with only specifying flash here. Regions that are not
+        * specified are treated as if we provided no memory map(if not we
+        * could detect the holes and mark them as RAM).
+        * Normally we only execute this code once, but no big deal if we
+        * have to regenerate it a couple of times.
+        */
+
+       struct flash_bank *p;
+       char *xml = NULL;
+       int size = 0;
+       int pos = 0;
+       int retval = ERROR_OK;
+       struct flash_bank **banks;
+       int offset;
+       int length;
+       char *separator;
+       uint32_t ram_start = 0;
+       int i;
+
+       /* skip command character */
+       packet += 23;
+
+       offset = strtoul(packet, &separator, 16);
+       length = strtoul(separator + 1, &separator, 16);
+
+       xml_printf(&retval, &xml, &pos, &size, "<memory-map>\n");
+
+       /* Sort banks in ascending order.  We need to report non-flash
+        * memory as ram (or rather read/write) by default for GDB, since
+        * it has no concept of non-cacheable read/write memory (i/o etc).
+        *
+        * FIXME Most non-flash addresses are *NOT* RAM!  Don't lie.
+        * Current versions of GDB assume unlisted addresses are RAM...
+        */
+       banks = malloc(sizeof(struct flash_bank *)*flash_get_bank_count());
+
+       for (i = 0; i < flash_get_bank_count(); i++) {
+               retval = get_flash_bank_by_num(i, &p);
+               if (retval != ERROR_OK)
+               {
+                       free(banks);
+                       gdb_send_error(connection, retval);
+                       return retval;
+               }
+               banks[i] = p;
+       }
+
+       qsort(banks, flash_get_bank_count(), sizeof(struct flash_bank *),
+                       compare_bank);
+
+       for (i = 0; i < flash_get_bank_count(); i++) {
+               int j;
+               unsigned sector_size = 0;
+               uint32_t start, end;
+
+               p = banks[i];
+               start = p->base;
+               end = p->base + p->size;
+
+               if (ram_start < p->base)
+                       xml_printf(&retval, &xml, &pos, &size,
+                               "<memory type=\"ram\" start=\"0x%x\" "
+                                       "length=\"0x%x\"/>\n",
+                               ram_start, p->base - ram_start);
+
+               /* Report adjacent groups of same-size sectors.  So for
+                * example top boot CFI flash will list an initial region
+                * with several large sectors (maybe 128KB) and several
+                * smaller ones at the end (maybe 32KB).  STR7 will have
+                * regions with 8KB, 32KB, and 64KB sectors; etc.
+                */
+               for (j = 0; j < p->num_sectors; j++) {
+                       unsigned group_len;
+
+                       /* Maybe start a new group of sectors. */
+                       if (sector_size == 0) {
+                               start = p->base + p->sectors[j].offset;
+                               xml_printf(&retval, &xml, &pos, &size,
+                                       "<memory type=\"flash\" "
+                                               "start=\"0x%x\" ",
+                                       start);
+                               sector_size = p->sectors[j].size;
+                       }
+
+                       /* Does this finish a group of sectors?
+                        * If not, continue an already-started group.
+                        */
+                       if (j == p->num_sectors -1)
+                               group_len = (p->base + p->size) - start;
+                       else if (p->sectors[j + 1].size != sector_size)
+                               group_len = p->base + p->sectors[j + 1].offset
+                                               - start;
+                       else
+                               continue;
+
+                       xml_printf(&retval, &xml, &pos, &size,
+                               "length=\"0x%x\">\n"
+                               "<property name=\"blocksize\">"
+                                       "0x%x</property>\n"
+                               "</memory>\n",
+                               group_len,
+                               sector_size);
+                       sector_size = 0;
+               }
+
+               ram_start = p->base + p->size;
+       }
+
+       if (ram_start != 0)
+               xml_printf(&retval, &xml, &pos, &size,
+                       "<memory type=\"ram\" start=\"0x%x\" "
+                               "length=\"0x%x\"/>\n",
+                       ram_start, 0-ram_start);
+       /* ELSE a flash chip could be at the very end of the 32 bit address
+        * space, in which case ram_start will be precisely 0
+        */
+
+       free(banks);
+       banks = NULL;
+
+       xml_printf(&retval, &xml, &pos, &size, "</memory-map>\n");
+
+       if (retval != ERROR_OK) {
+               gdb_send_error(connection, retval);
+               return retval;
+       }
+
+       if (offset + length > pos)
+               length = pos - offset;
+
+       char *t = malloc(length + 1);
+       t[0] = 'l';
+       memcpy(t + 1, xml + offset, length);
+       gdb_put_packet(connection, t, length + 1);
+
+       free(t);
+       free(xml);
+       return ERROR_OK;
+}
+
+static int gdb_query_packet(struct connection *connection,
+       struct target *target, char *packet, int packet_size)
+{
+       struct command_context *cmd_ctx = connection->cmd_ctx;
        struct gdb_connection *gdb_connection = connection->priv;
 
        if (strstr(packet, "qRcmd,"))
@@ -1653,112 +1918,9 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i
 
                return ERROR_OK;
        }
-       else if (strstr(packet, "qXfer:memory-map:read::") && (flash_get_bank_count() > 0))
-       {
-               /* We get away with only specifying flash here. Regions that are not
-                * specified are treated as if we provided no memory map(if not we
-                * could detect the holes and mark them as RAM).
-                * Normally we only execute this code once, but no big deal if we
-                * have to regenerate it a couple of times. */
-
-               flash_bank_t *p;
-               char *xml = NULL;
-               int size = 0;
-               int pos = 0;
-               int retval = ERROR_OK;
-
-               int offset;
-               int length;
-               char *separator;
-               int blocksize;
-
-               /* skip command character */
-               packet += 23;
-
-               offset = strtoul(packet, &separator, 16);
-               length = strtoul(separator + 1, &separator, 16);
-
-               xml_printf(&retval, &xml, &pos, &size, "<memory-map>\n");
-
-               /*
-               sort banks in ascending order, we need to make non-flash memory be ram(or rather
-               read/write) by default for GDB.
-               GDB does not have a concept of non-cacheable read/write memory.
-                */
-               flash_bank_t **banks = malloc(sizeof(flash_bank_t *)*flash_get_bank_count());
-               int i;
-
-               for (i = 0; i < flash_get_bank_count(); i++)
-               {
-                       p = get_flash_bank_by_num(i);
-                       if (p == NULL)
-                       {
-                               free(banks);
-                               retval = ERROR_FAIL;
-                               gdb_send_error(connection, retval);
-                               return retval;
-                       }
-                       banks[i]=p;
-               }
-
-               qsort(banks, flash_get_bank_count(), sizeof(flash_bank_t *), compare_bank);
-
-               uint32_t ram_start = 0;
-               for (i = 0; i < flash_get_bank_count(); i++)
-               {
-                       p = banks[i];
-
-                       if (ram_start < p->base)
-                       {
-                               xml_printf(&retval, &xml, &pos, &size, "<memory type=\"ram\" start=\"0x%x\" length=\"0x%x\"/>\n",
-                                       ram_start, p->base-ram_start);
-                       }
-
-                       /* if device has uneven sector sizes, eg. str7, lpc
-                        * we pass the smallest sector size to gdb memory map */
-                       blocksize = gdb_calc_blocksize(p);
-
-                       xml_printf(&retval, &xml, &pos, &size, "<memory type=\"flash\" start=\"0x%x\" length=\"0x%x\">\n" \
-                               "<property name=\"blocksize\">0x%x</property>\n" \
-                               "</memory>\n", \
-                               p->base, p->size, blocksize);
-                       ram_start = p->base + p->size;
-               }
-               if (ram_start != 0)
-               {
-                       xml_printf(&retval, &xml, &pos, &size, "<memory type=\"ram\" start=\"0x%x\" length=\"0x%x\"/>\n",
-                               ram_start, 0-ram_start);
-               } else
-               {
-                       /* a flash chip could be at the very end of the 32 bit address space, in which case
-                       ram_start will be precisely 0 */
-               }
-
-               free(banks);
-               banks = NULL;
-
-               xml_printf(&retval, &xml, &pos, &size, "</memory-map>\n");
-
-               if (retval != ERROR_OK)
-               {
-                       gdb_send_error(connection, retval);
-                       return retval;
-               }
-
-               if (offset + length > pos)
-               {
-                       length = pos - offset;
-               }
-
-               char *t = malloc(length + 1);
-               t[0] = 'l';
-               memcpy(t + 1, xml + offset, length);
-               gdb_put_packet(connection, t, length + 1);
-
-               free(t);
-               free(xml);
-               return ERROR_OK;
-       }
+       else if (strstr(packet, "qXfer:memory-map:read::")
+                       && (flash_get_bank_count() > 0))
+               return gdb_memory_map(connection, target, packet, packet_size);
        else if (strstr(packet, "qXfer:features:read:"))
        {
                char *xml = NULL;
@@ -1810,7 +1972,8 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i
        return ERROR_OK;
 }
 
-int gdb_v_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
+static int gdb_v_packet(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
 {
        struct gdb_connection *gdb_connection = connection->priv;
        struct gdb_service *gdb_service = connection->service->priv;
@@ -1857,9 +2020,19 @@ int gdb_v_packet(connection_t *connection, target_t *target, char *packet, int p
                flash_set_dirty();
 
                /* perform any target specific operations before the erase */
-               target_call_event_callbacks(gdb_service->target, TARGET_EVENT_GDB_FLASH_ERASE_START);
-               result = flash_erase_address_range(gdb_service->target, addr, length);
-               target_call_event_callbacks(gdb_service->target, TARGET_EVENT_GDB_FLASH_ERASE_END);
+               target_call_event_callbacks(gdb_service->target,
+                               TARGET_EVENT_GDB_FLASH_ERASE_START);
+
+               /* vFlashErase:addr,length messages require region start and
+                * end to be "block" aligned ... if padding is ever needed,
+                * GDB will have become dangerously confused.
+                */
+               result = flash_erase_address_range(gdb_service->target,
+                               false, addr, length);
+
+               /* perform any target specific operations after the erase */
+               target_call_event_callbacks(gdb_service->target,
+                               TARGET_EVENT_GDB_FLASH_ERASE_END);
 
                /* perform erase */
                if (result != ERROR_OK)
@@ -1899,7 +2072,7 @@ int gdb_v_packet(connection_t *connection, target_t *target, char *packet, int p
                /* create a new image if there isn't already one */
                if (gdb_connection->vflash_image == NULL)
                {
-                       gdb_connection->vflash_image = malloc(sizeof(image_t));
+                       gdb_connection->vflash_image = malloc(sizeof(struct image));
                        image_open(gdb_connection->vflash_image, "", "build");
                }
 
@@ -1947,11 +2120,12 @@ int gdb_v_packet(connection_t *connection, target_t *target, char *packet, int p
        return ERROR_OK;
 }
 
-int gdb_detach(connection_t *connection, target_t *target)
+static int gdb_detach(struct connection *connection, struct target *target)
 {
        struct gdb_service *gdb_service = connection->service->priv;
 
-       target_call_event_callbacks(gdb_service->target, TARGET_EVENT_GDB_DETACH);
+       target_call_event_callbacks(gdb_service->target,
+                       TARGET_EVENT_GDB_DETACH);
 
        return gdb_put_packet(connection, "OK", 2);
 }
@@ -1959,7 +2133,7 @@ int gdb_detach(connection_t *connection, target_t *target)
 static void gdb_log_callback(void *priv, const char *file, unsigned line,
                const char *function, const char *string)
 {
-       connection_t *connection = priv;
+       struct connection *connection = priv;
        struct gdb_connection *gdb_con = connection->priv;
 
        if (gdb_con->busy)
@@ -1971,10 +2145,7 @@ static void gdb_log_callback(void *priv, const char *file, unsigned line,
        gdb_output_con(connection, string);
 }
 
-/* Do not allocate this on the stack */
-char gdb_packet_buffer[GDB_BUFFER_SIZE];
-
-static void gdb_sig_halted(connection_t *connection)
+static void gdb_sig_halted(struct connection *connection)
 {
        char sig_reply[4];
        snprintf(sig_reply, 4, "T%2.2x", 2);
@@ -1982,10 +2153,13 @@ static void gdb_sig_halted(connection_t *connection)
 
 }
 
-int gdb_input_inner(connection_t *connection)
+static int gdb_input_inner(struct connection *connection)
 {
+       /* Do not allocate this on the stack */
+       static char gdb_packet_buffer[GDB_BUFFER_SIZE];
+
        struct gdb_service *gdb_service = connection->service->priv;
-       target_t *target = gdb_service->target;
+       struct target *target = gdb_service->target;
        char *packet = gdb_packet_buffer;
        int packet_size;
        int retval;
@@ -1996,10 +2170,9 @@ int gdb_input_inner(connection_t *connection)
        do
        {
                packet_size = GDB_BUFFER_SIZE-1;
-               if ((retval = gdb_get_packet(connection, packet, &packet_size)) != ERROR_OK)
-               {
+               retval = gdb_get_packet(connection, packet, &packet_size);
+               if (retval != ERROR_OK)
                        return retval;
-               }
 
                /* terminate with zero */
                packet[packet_size] = 0;
@@ -2031,32 +2204,48 @@ int gdb_input_inner(connection_t *connection)
                                        break;
                                case 'q':
                                case 'Q':
-                                       retval = gdb_query_packet(connection, target, packet, packet_size);
+                                       retval = gdb_query_packet(connection,
+                                                       target, packet,
+                                                       packet_size);
                                        break;
                                case 'g':
-                                       retval = gdb_get_registers_packet(connection, target, packet, packet_size);
+                                       retval = gdb_get_registers_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'G':
-                                       retval = gdb_set_registers_packet(connection, target, packet, packet_size);
+                                       retval = gdb_set_registers_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'p':
-                                       retval = gdb_get_register_packet(connection, target, packet, packet_size);
+                                       retval = gdb_get_register_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'P':
-                                       retval = gdb_set_register_packet(connection, target, packet, packet_size);
+                                       retval = gdb_set_register_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'm':
-                                       retval = gdb_read_memory_packet(connection, target, packet, packet_size);
+                                       retval = gdb_read_memory_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'M':
-                                       retval = gdb_write_memory_packet(connection, target, packet, packet_size);
+                                       retval = gdb_write_memory_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'z':
                                case 'Z':
                                        retval = gdb_breakpoint_watchpoint_packet(connection, target, packet, packet_size);
                                        break;
                                case '?':
-                                       gdb_last_signal_packet(connection, target, packet, packet_size);
+                                       gdb_last_signal_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'c':
                                case 's':
@@ -2066,13 +2255,22 @@ int gdb_input_inner(connection_t *connection)
                                                struct gdb_connection *gdb_con = connection->priv;
                                                log_add_callback(gdb_log_callback, connection);
 
+                                               if (gdb_con->mem_write_error)
+                                               {
+                                                       LOG_ERROR("Memory write failure!");
+
+                                                       /* now that we have reported the memory write error, we can clear the condition */
+                                                       gdb_con->mem_write_error = false;
+                                               }
+
                                                bool nostep = false;
+                                               bool already_running = false;
                                                if (target->state == TARGET_RUNNING)
                                                {
-                                                       LOG_WARNING("The target is already running. Halt target before stepi/continue.");
-                                                       retval = target_halt(target);
-                                                       if (retval == ERROR_OK)
-                                                               retval = target_wait_state(target, TARGET_HALTED, 100);
+                                                       LOG_WARNING("WARNING! The target is already running. "
+                                                                       "All changes GDB did to registers will be discarded! "
+                                                                       "Waiting for target to halt.");
+                                                       already_running = true;
                                                } else if (target->state != TARGET_HALTED)
                                                {
                                                        LOG_WARNING("The target is not in the halted nor running stated, stepi/continue ignored.");
@@ -2088,7 +2286,7 @@ int gdb_input_inner(connection_t *connection)
                                                }
                                                gdb_con->sync = false;
 
-                                               if ((retval!=ERROR_OK) || nostep)
+                                               if ((retval!=ERROR_OK) || (!already_running && nostep))
                                                {
                                                        /* Either the target isn't in the halted state, then we can't
                                                         * step/continue. This might be early setup, etc.
@@ -2108,24 +2306,33 @@ int gdb_input_inner(connection_t *connection)
                                                         */
                                                        gdb_con->frontend_state = TARGET_RUNNING;
                                                        target_call_event_callbacks(target, TARGET_EVENT_GDB_START);
-                                                       int retval = gdb_step_continue_packet(connection, target, packet, packet_size);
-                                                       if (retval != ERROR_OK)
+
+                                                       if (!already_running)
                                                        {
-                                                               /* we'll never receive a halted condition... issue a false one.. */
-                                                               gdb_frontend_halted(target, connection);
+                                                               int retval = gdb_step_continue_packet(connection, target, packet, packet_size);
+                                                               if (retval != ERROR_OK)
+                                                               {
+                                                                       /* we'll never receive a halted condition... issue a false one.. */
+                                                                       gdb_frontend_halted(target, connection);
+                                                               }
                                                        }
                                                }
                                        }
                                        break;
                                case 'v':
-                                       retval = gdb_v_packet(connection, target, packet, packet_size);
+                                       retval = gdb_v_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
                                        break;
                                case 'D':
                                        retval = gdb_detach(connection, target);
                                        extended_protocol = 0;
                                        break;
                                case 'X':
-                                       if ((retval = gdb_write_memory_binary_packet(connection, target, packet, packet_size)) != ERROR_OK)
+                                       retval = gdb_write_memory_binary_packet(
+                                                       connection, target,
+                                                       packet, packet_size);
+                                       if (retval != ERROR_OK)
                                                return retval;
                                        break;
                                case 'k':
@@ -2144,10 +2351,10 @@ int gdb_input_inner(connection_t *connection)
                                        watchpoint_clear_target(gdb_service->target);
                                        command_run_linef(connection->cmd_ctx,
                                                        "ocd_gdb_restart %s",
-                                                       target->cmd_name);
+                                                       target_name(target));
                                        break;
                                default:
-                                       /* ignore unkown packets */
+                                       /* ignore unknown packets */
                                        LOG_DEBUG("ignoring 0x%2.2x packet", packet[0]);
                                        gdb_put_packet(connection, NULL, 0);
                                        break;
@@ -2180,7 +2387,7 @@ int gdb_input_inner(connection_t *connection)
        return ERROR_OK;
 }
 
-int gdb_input(connection_t *connection)
+static int gdb_input(struct connection *connection)
 {
        int retval = gdb_input_inner(connection);
        struct gdb_connection *gdb_con = connection->priv;
@@ -2195,55 +2402,72 @@ int gdb_input(connection_t *connection)
        return ERROR_OK;
 }
 
-int gdb_init(void)
+static int gdb_target_start(struct target *target, uint16_t port)
 {
-       struct gdb_service *gdb_service;
-       target_t *target = all_targets;
+       bool use_pipes = 0 == port;
+       struct gdb_service *gdb_service = malloc(sizeof(struct gdb_service));
+       if (NULL == gdb_service)
+               return -ENOMEM;
 
-       if (!target)
-       {
-               LOG_WARNING("no gdb ports allocated as no target has been specified");
-               return ERROR_OK;
-       }
+       gdb_service->target = target;
+
+       add_service("gdb", use_pipes ? CONNECTION_PIPE : CONNECTION_TCP,
+                       port, 1, &gdb_new_connection, &gdb_input,
+                       &gdb_connection_closed, gdb_service);
 
+       const char *name = target_name(target);
+       if (use_pipes)
+               LOG_DEBUG("gdb service for target '%s' using pipes", name);
+       else
+               LOG_DEBUG("gdb service for target '%s' on TCP port %u", name, port);
+       return ERROR_OK;
+}
+
+static int gdb_target_add_one(struct target *target)
+{
        if (gdb_port == 0 && server_use_pipes == 0)
        {
                LOG_INFO("gdb port disabled");
                return ERROR_OK;
        }
+       if (0 == gdb_port_next)
+               gdb_port_next = gdb_port;
 
-       if (server_use_pipes)
+       bool use_pipes = server_use_pipes;
+       static bool server_started_with_pipes = false;
+       if (server_started_with_pipes)
        {
-               /* only a single gdb connection when using a pipe */
+               LOG_WARNING("gdb service permits one target when using pipes");
+               if (0 == gdb_port)
+                       return ERROR_OK;
 
-               gdb_service = malloc(sizeof(struct gdb_service));
-               gdb_service->target = target;
+               use_pipes = false;
+       }
 
-               add_service("gdb", CONNECTION_PIPE, 0, 1, gdb_new_connection, gdb_input, gdb_connection_closed, gdb_service);
+       int e = gdb_target_start(target, use_pipes ? 0 : gdb_port_next);
+       if (ERROR_OK == e)
+       {
+               server_started_with_pipes |= use_pipes;
+               gdb_port_next++;
+       }
+       return e;
+}
 
-               LOG_DEBUG("gdb service for target %s using pipes",
-                               target_get_name(target));
+int gdb_target_add_all(struct target *target)
+{
+       if (NULL == target)
+       {
+               LOG_WARNING("gdb services need one or more targets defined");
+               return ERROR_OK;
        }
-       else
+
+       while (NULL != target)
        {
-               unsigned short port = gdb_port;
+               int retval = gdb_target_add_one(target);
+               if (ERROR_OK != retval)
+                       return retval;
 
-               while (target)
-               {
-                       gdb_service = malloc(sizeof(struct gdb_service));
-                       gdb_service->target = target;
-
-                       add_service("gdb", CONNECTION_TCP,
-                                       port, 1,
-                                       gdb_new_connection, gdb_input,
-                                       gdb_connection_closed, gdb_service);
-
-                       LOG_DEBUG("gdb service for target %s at TCP port %i",
-                                       target_get_name(target),
-                                       port);
-                       target = target->next;
-                       port++;
-               }
+               target = target->next;
        }
 
        return ERROR_OK;
@@ -2251,14 +2475,14 @@ int gdb_init(void)
 
 COMMAND_HANDLER(handle_gdb_sync_command)
 {
-       if (argc != 0)
+       if (CMD_ARGC != 0)
        {
                return ERROR_COMMAND_SYNTAX_ERROR;
        }
 
        if (current_gdb_connection == NULL)
        {
-               command_print(cmd_ctx,
+               command_print(CMD_CTX,
                                "gdb_sync command can only be run from within gdb using \"monitor gdb_sync\"");
                return ERROR_FAIL;
        }
@@ -2271,88 +2495,55 @@ COMMAND_HANDLER(handle_gdb_sync_command)
 /* daemon configuration command gdb_port */
 COMMAND_HANDLER(handle_gdb_port_command)
 {
-       return CALL_COMMAND_HANDLER(server_port_command, &gdb_port);
+       int retval = CALL_COMMAND_HANDLER(server_port_command, &gdb_port);
+       if (ERROR_OK == retval)
+               gdb_port_next = gdb_port;
+       return retval;
 }
 
 COMMAND_HANDLER(handle_gdb_memory_map_command)
 {
-       if (argc == 1)
-       {
-               if (strcmp(args[0], "enable") == 0)
-               {
-                       gdb_use_memory_map = 1;
-                       return ERROR_OK;
-               }
-               else if (strcmp(args[0], "disable") == 0)
-               {
-                       gdb_use_memory_map = 0;
-                       return ERROR_OK;
-               }
-               else
-                       LOG_WARNING("invalid gdb_memory_map configuration directive %s", args[0]);
-       }
+       if (CMD_ARGC != 1)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       return ERROR_COMMAND_SYNTAX_ERROR;
+       COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_use_memory_map);
+       return ERROR_OK;
 }
 
 COMMAND_HANDLER(handle_gdb_flash_program_command)
 {
-       if (argc == 1)
-       {
-               if (strcmp(args[0], "enable") == 0)
-               {
-                       gdb_flash_program = 1;
-                       return ERROR_OK;
-               }
-               else if (strcmp(args[0], "disable") == 0)
-               {
-                       gdb_flash_program = 0;
-                       return ERROR_OK;
-               }
-               else
-                       LOG_WARNING("invalid gdb_flash_program configuration directive: %s", args[0]);
-       }
+       if (CMD_ARGC != 1)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       return ERROR_COMMAND_SYNTAX_ERROR;
+       COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_flash_program);
+       return ERROR_OK;
 }
 
 COMMAND_HANDLER(handle_gdb_report_data_abort_command)
 {
-       if (argc == 1)
-       {
-               if (strcmp(args[0], "enable") == 0)
-               {
-                       gdb_report_data_abort = 1;
-                       return ERROR_OK;
-               }
-               else if (strcmp(args[0], "disable") == 0)
-               {
-                       gdb_report_data_abort = 0;
-                       return ERROR_OK;
-               }
-               else
-                       LOG_WARNING("invalid gdb_report_data_abort configuration directive: %s", args[0]);
-       }
+       if (CMD_ARGC != 1)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       return ERROR_COMMAND_SYNTAX_ERROR;
+       COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_report_data_abort);
+       return ERROR_OK;
 }
 
 /* gdb_breakpoint_override */
 COMMAND_HANDLER(handle_gdb_breakpoint_override_command)
 {
-       if (argc == 0)
+       if (CMD_ARGC == 0)
        {
 
-       } else if (argc == 1)
+       } else if (CMD_ARGC == 1)
        {
                gdb_breakpoint_override = 1;
-               if (strcmp(args[0], "hard") == 0)
+               if (strcmp(CMD_ARGV[0], "hard") == 0)
                {
                        gdb_breakpoint_override_type = BKPT_HARD;
-               } else if (strcmp(args[0], "soft") == 0)
+               } else if (strcmp(CMD_ARGV[0], "soft") == 0)
                {
                        gdb_breakpoint_override_type = BKPT_SOFT;
-               } else if (strcmp(args[0], "disable") == 0)
+               } else if (strcmp(CMD_ARGV[0], "disable") == 0)
                {
                        gdb_breakpoint_override = 0;
                }
@@ -2365,33 +2556,63 @@ COMMAND_HANDLER(handle_gdb_breakpoint_override_command)
                LOG_USER("force %s breakpoints", (gdb_breakpoint_override_type == BKPT_HARD)?"hard":"soft");
        } else
        {
-               LOG_USER("breakpoint type is not overriden");
+               LOG_USER("breakpoint type is not overridden");
        }
 
        return ERROR_OK;
 }
 
-int gdb_register_commands(command_context_t *command_context)
+static const struct command_registration gdb_command_handlers[] = {
+       {
+               .name = "gdb_sync",
+               .handler = handle_gdb_sync_command,
+               .mode = COMMAND_ANY,
+               .help = "next stepi will return immediately allowing "
+                       "GDB to fetch register state without affecting "
+                       "target state",
+       },
+       {
+               .name = "gdb_port",
+               .handler = handle_gdb_port_command,
+               .mode = COMMAND_ANY,
+               .help = "Display or specify base port on which to listen "
+                       "for incoming GDB connections.  "
+                       "No arguments reports GDB port; zero disables.",
+               .usage = "[port_num]",
+       },
+       {
+               .name = "gdb_memory_map",
+               .handler = handle_gdb_memory_map_command,
+               .mode = COMMAND_CONFIG,
+               .help = "enable or disable memory map",
+               .usage = "('enable'|'disable')"
+       },
+       {
+               .name = "gdb_flash_program",
+               .handler = handle_gdb_flash_program_command,
+               .mode = COMMAND_CONFIG,
+               .help = "enable or disable flash program",
+               .usage = "('enable'|'disable')"
+       },
+       {
+               .name = "gdb_report_data_abort",
+               .handler = handle_gdb_report_data_abort_command,
+               .mode = COMMAND_CONFIG,
+               .help = "enable or disable reporting data aborts",
+               .usage = "('enable'|'disable')"
+       },
+       {
+               .name = "gdb_breakpoint_override",
+               .handler = handle_gdb_breakpoint_override_command,
+               .mode = COMMAND_ANY,
+               .help = "Display or specify type of breakpoint "
+                       "to be used by gdb 'break' commands.",
+               .usage = "('hard'|'soft'|'disable')"
+       },
+       COMMAND_REGISTRATION_DONE
+};
+
+int gdb_register_commands(struct command_context *cmd_ctx)
 {
-       register_command(command_context, NULL, "gdb_sync",
-                       handle_gdb_sync_command, COMMAND_ANY,
-                       "next stepi will return immediately allowing GDB to "
-                       "fetch register state without affecting target state");
-       register_command(command_context, NULL, "gdb_port",
-                       handle_gdb_port_command, COMMAND_ANY,
-                       "daemon configuration command gdb_port");
-       register_command(command_context, NULL, "gdb_memory_map",
-                       handle_gdb_memory_map_command, COMMAND_CONFIG,
-                       "enable or disable memory map");
-       register_command(command_context, NULL, "gdb_flash_program",
-                       handle_gdb_flash_program_command, COMMAND_CONFIG,
-                       "enable or disable flash program");
-       register_command(command_context, NULL, "gdb_report_data_abort",
-                       handle_gdb_report_data_abort_command, COMMAND_CONFIG,
-                       "enable or disable reporting data aborts");
-       register_command(command_context, NULL, "gdb_breakpoint_override",
-                       handle_gdb_breakpoint_override_command, COMMAND_EXEC,
-                       "hard/soft/disable - force type of breakpoint "
-                       "used by gdb 'break' commands.");
-       return ERROR_OK;
+       return register_commands(cmd_ctx, NULL, gdb_command_handlers);
 }

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)