Fix a bunch of typos.
[openocd.git] / src / server / gdb_server.c
index 84ad76c9333adc3f5459532ed213bf2f7320c448..77142dfadec8acfd5c2ebf41560cd491ded3ca38 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(struct connection *connection, int retval);
-static unsigned short gdb_port = 3333;
-static const char *DIGITS = "0123456789abcdef";
+static int gdb_error(struct connection *connection, int retval);
+static const char *gdb_port;
+static const char *gdb_port_next;
+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(struct target *target)
+static int gdb_last_signal(struct target *target)
 {
        switch (target->debug_reason)
        {
@@ -87,7 +124,8 @@ int gdb_last_signal(struct target *target)
        }
 }
 
-int check_pending(struct connection *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(struct connection *connection, int timeout_s, int *got_data)
        return ERROR_OK;
 }
 
-int gdb_get_char(struct connection *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,25 +174,9 @@ int gdb_get_char(struct connection *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)
+               if (connection->service->type != CONNECTION_TCP)
                {
                        gdb_con->buf_cnt = read(connection->fd, gdb_con->buffer, GDB_BUFFER_SIZE);
                }
@@ -236,7 +258,51 @@ int gdb_get_char(struct connection *connection, int* next_char)
        return retval;
 }
 
-int gdb_putback_char(struct connection *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,32 +322,22 @@ int gdb_putback_char(struct connection *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(struct connection *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)
                return ERROR_SERVER_REMOTE_CLOSED;
 
-       if (connection->service->type == CONNECTION_PIPE)
-       {
-               /* write to stdout */
-               if (write(STDOUT_FILENO, data, len) == len)
-               {
-                       return ERROR_OK;
-               }
-       }
-       else
+       if (connection_write(connection, data, len) == len)
        {
-               if (write_socket(connection->fd, data, len) == len)
-               {
-                       return ERROR_OK;
-               }
+               return ERROR_OK;
        }
        gdb_con->closed = 1;
        return ERROR_SERVER_REMOTE_CLOSED;
 }
 
-int gdb_put_packet_inner(struct connection *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 +360,8 @@ int gdb_put_packet_inner(struct connection *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 +479,7 @@ int gdb_put_packet_inner(struct connection *connection, char *buffer, int len)
        return ERROR_OK;
 }
 
-int gdb_put_packet(struct connection *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;
@@ -440,27 +497,33 @@ static __inline__ int fetch_packet(struct connection *connection, int *checksum_
        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 +555,21 @@ static __inline__ int fetch_packet(struct connection *connection, int *checksum_
                                        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 +579,11 @@ static __inline__ int fetch_packet(struct connection *connection, int *checksum_
                        /* 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 +594,12 @@ static __inline__ int fetch_packet(struct connection *connection, int *checksum_
                }
        }
 
+       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 +618,8 @@ static __inline__ int fetch_packet(struct connection *connection, int *checksum_
        return ERROR_OK;
 }
 
-int gdb_get_packet_inner(struct connection *connection, char *buffer, int *len)
+static int gdb_get_packet_inner(struct connection *connection,
+               char *buffer, int *len)
 {
        int character;
        int retval;
@@ -567,7 +642,7 @@ int gdb_get_packet_inner(struct connection *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 +694,7 @@ int gdb_get_packet_inner(struct connection *connection, char *buffer, int *len)
        return ERROR_OK;
 }
 
-int gdb_get_packet(struct connection *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 +703,7 @@ int gdb_get_packet(struct connection *connection, char *buffer, int *len)
        return retval;
 }
 
-int gdb_output_con(struct connection *connection, const char* line)
+static int gdb_output_con(struct connection *connection, const char* line)
 {
        char *hex_buffer;
        int i, bin_size;
@@ -650,7 +725,7 @@ int gdb_output_con(struct connection *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);
@@ -674,28 +749,29 @@ static void gdb_frontend_halted(struct target *target, struct connection *connec
        if (gdb_connection->frontend_state == TARGET_RUNNING)
        {
                char sig_reply[4];
-               int signal;
+               int signal_var;
 
                /* stop forwarding log packets! */
                log_remove_callback(gdb_log_callback, connection);
 
                if (gdb_connection->ctrl_c)
                {
-                       signal = 0x2;
+                       signal_var = 0x2;
                        gdb_connection->ctrl_c = 0;
                }
                else
                {
-                       signal = gdb_last_signal(target);
+                       signal_var = gdb_last_signal(target);
                }
 
-               snprintf(sig_reply, 4, "T%2.2x", signal);
+               snprintf(sig_reply, 4, "T%2.2x", signal_var);
                gdb_put_packet(connection, sig_reply, 3);
                gdb_connection->frontend_state = TARGET_HALTED;
        }
 }
 
-int gdb_target_callback_event_handler(struct target *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;
        struct connection *connection = priv;
@@ -723,7 +799,7 @@ int gdb_target_callback_event_handler(struct target *target, enum target_event e
        return ERROR_OK;
 }
 
-int gdb_new_connection(struct connection *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 +818,7 @@ int gdb_new_connection(struct connection *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);
@@ -756,9 +833,6 @@ int gdb_new_connection(struct connection *connection)
        breakpoint_clear_target(gdb_service->target);
        watchpoint_clear_target(gdb_service->target);
 
-       /* register callback to be informed about target events */
-       target_register_event_callback(gdb_target_callback_event_handler, connection);
-
        /* remove the initial ACK from the incoming buffer */
        if ((retval = gdb_get_char(connection, &initial_ack)) != ERROR_OK)
                return retval;
@@ -770,16 +844,43 @@ int gdb_new_connection(struct connection *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, or use 'gdb_memory_map disable'.");
+                               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));
 
+       /* DANGER! If we fail subsequently, we must remove this handler,
+        * otherwise we occasionally see crashes as the timer can invoke the
+        * callback fn.
+        *
+        * register callback to be informed about target events */
+       target_register_event_callback(gdb_target_callback_event_handler, connection);
+
        return ERROR_OK;
 }
 
-int gdb_connection_closed(struct connection *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 +892,7 @@ int gdb_connection_closed(struct connection *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,21 +927,22 @@ int gdb_connection_closed(struct connection *connection)
        return ERROR_OK;
 }
 
-void gdb_send_error(struct connection *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(struct connection *connection, struct target *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;
+       int signal_var;
 
-       signal = gdb_last_signal(target);
+       signal_var = gdb_last_signal(target);
 
-       snprintf(sig_reply, 4, "S%2.2x", signal);
+       snprintf(sig_reply, 4, "S%2.2x", signal_var);
        gdb_put_packet(connection, sig_reply, 3);
 
        return ERROR_OK;
@@ -858,19 +960,20 @@ static int gdb_reg_pos(struct target *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(struct target *target, char *tstr, struct reg *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 +983,7 @@ void gdb_str_to_target(struct target *target, char *tstr, struct reg *reg)
        }
 }
 
-static int hextoint(char c)
+static int hextoint(int c)
 {
        if (c>='0'&&c<='9')
        {
@@ -896,7 +999,8 @@ static int hextoint(char c)
 }
 
 /* copy over in register buffer */
-void gdb_target_to_reg(struct target *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,7 +1019,8 @@ void gdb_target_to_reg(struct target *target, char *tstr, int str_len, uint8_t *
        }
 }
 
-int gdb_get_registers_packet(struct connection *connection, struct target *target, char* packet, int packet_size)
+static int gdb_get_registers_packet(struct connection *connection,
+               struct target *target, char* packet, int packet_size)
 {
        struct reg **reg_list;
        int reg_list_size;
@@ -939,25 +1044,25 @@ int gdb_get_registers_packet(struct connection *connection, struct target *targe
                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,7 +1070,8 @@ int gdb_get_registers_packet(struct connection *connection, struct target *targe
        return ERROR_OK;
 }
 
-int gdb_set_registers_packet(struct connection *connection, struct target *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;
        struct reg **reg_list;
@@ -996,21 +1102,17 @@ int gdb_set_registers_packet(struct connection *connection, struct target *targe
        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");
                }
 
-               struct reg_arch_type *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;
@@ -1027,7 +1129,8 @@ int gdb_set_registers_packet(struct connection *connection, struct target *targe
        return ERROR_OK;
 }
 
-int gdb_get_register_packet(struct connection *connection, struct target *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);
@@ -1050,11 +1153,11 @@ int gdb_get_register_packet(struct connection *connection, struct target *target
                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,7 +1165,8 @@ int gdb_get_register_packet(struct connection *connection, struct target *target
        return ERROR_OK;
 }
 
-int gdb_set_register_packet(struct connection *connection, struct target *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;
@@ -1070,7 +1174,6 @@ int gdb_set_register_packet(struct connection *connection, struct target *target
        struct reg **reg_list;
        int reg_list_size;
        int retval;
-       struct reg_arch_type *arch_type;
 
        LOG_DEBUG("-");
 
@@ -1092,16 +1195,14 @@ int gdb_set_register_packet(struct connection *connection, struct target *target
        }
 
        /* 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,29 +1212,14 @@ int gdb_set_register_packet(struct connection *connection, struct target *target
        return ERROR_OK;
 }
 
-int gdb_error(struct connection *connection, int retval)
+/* No attempt is made to translate the "retval" to
+ * GDB speak. This has to be done at the calling
+ * site as no mapping really exists.
+ */
+static int gdb_error(struct connection *connection, int retval)
 {
-       switch (retval)
-       {
-               case ERROR_TARGET_DATA_ABORT:
-                       gdb_send_error(connection, EIO);
-                       break;
-               case ERROR_TARGET_TRANSLATION_FAULT:
-                       gdb_send_error(connection, EFAULT);
-                       break;
-               case ERROR_TARGET_UNALIGNED_ACCESS:
-                       gdb_send_error(connection, EFAULT);
-                       break;
-               case ERROR_TARGET_NOT_HALTED:
-                       gdb_send_error(connection, EFAULT);
-                       break;
-               default:
-                       /* This could be that the target reset itself. */
-                       LOG_ERROR("unexpected error %i", retval);
-                       gdb_send_error(connection, EFAULT);
-                       break;
-       }
-
+       LOG_DEBUG("Reporting %i to GDB as generic error", retval);
+       gdb_send_error(connection, EFAULT);
        return ERROR_OK;
 }
 
@@ -1142,7 +1228,8 @@ int gdb_error(struct connection *connection, int retval)
  *
  * 8191 bytes by the looks of it. Why 8191 bytes instead of 8192?????
  */
-int gdb_read_memory_packet(struct connection *connection, struct target *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 +1303,8 @@ int gdb_read_memory_packet(struct connection *connection, struct target *target,
        return retval;
 }
 
-int gdb_write_memory_packet(struct connection *connection, struct target *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 +1361,14 @@ int gdb_write_memory_packet(struct connection *connection, struct target *target
        return retval;
 }
 
-int gdb_write_memory_binary_packet(struct connection *connection, struct target *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 +1389,18 @@ int gdb_write_memory_binary_packet(struct connection *connection, struct target
                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 +1411,22 @@ int gdb_write_memory_binary_packet(struct connection *connection, struct target
                        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(struct connection *connection, struct target *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 +1459,12 @@ int gdb_step_continue_packet(struct connection *connection, struct target *targe
        return retval;
 }
 
-int gdb_breakpoint_watchpoint_packet(struct connection *connection, struct target *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 +1484,12 @@ int gdb_breakpoint_watchpoint_packet(struct connection *connection, struct targe
                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 +1540,7 @@ int gdb_breakpoint_watchpoint_packet(struct connection *connection, struct targe
                {
                        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 +1564,11 @@ int gdb_breakpoint_watchpoint_packet(struct connection *connection, struct targe
        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 +1635,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 +1653,155 @@ static int compare_bank (const void * a, const void * b)
        }
 }
 
-int gdb_query_packet(struct connection *connection, struct target *target, char *packet, int packet_size)
+static int gdb_memory_map(struct connection *connection,
+               struct target *target, char *packet, int packet_size)
+{
+       /* 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;
+       int target_flash_banks = 0;
+
+       /* 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_error(connection, retval);
+                       return retval;
+               }
+               if(p->target == target)
+                       banks[target_flash_banks++] = p;
+       }
+
+       qsort(banks, target_flash_banks, 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_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)
 {
-       command_context_t *cmd_ctx = connection->cmd_ctx;
+       struct command_context *cmd_ctx = connection->cmd_ctx;
        struct gdb_connection *gdb_connection = connection->priv;
 
        if (strstr(packet, "qRcmd,"))
@@ -1653,112 +1898,9 @@ int gdb_query_packet(struct connection *connection, struct target *target, char
 
                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;
@@ -1790,7 +1932,7 @@ int gdb_query_packet(struct connection *connection, struct target *target, char
 
                if (retval != ERROR_OK)
                {
-                       gdb_send_error(connection, retval);
+                       gdb_error(connection, retval);
                        return retval;
                }
 
@@ -1810,7 +1952,8 @@ int gdb_query_packet(struct connection *connection, struct target *target, char
        return ERROR_OK;
 }
 
-int gdb_v_packet(struct connection *connection, struct target *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 +2000,19 @@ int gdb_v_packet(struct connection *connection, struct target *target, char *pac
                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)
@@ -1947,11 +2100,12 @@ int gdb_v_packet(struct connection *connection, struct target *target, char *pac
        return ERROR_OK;
 }
 
-int gdb_detach(struct connection *connection, struct target *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);
 }
@@ -1971,9 +2125,6 @@ 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(struct connection *connection)
 {
        char sig_reply[4];
@@ -1982,8 +2133,11 @@ static void gdb_sig_halted(struct connection *connection)
 
 }
 
-int gdb_input_inner(struct connection *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;
        struct target *target = gdb_service->target;
        char *packet = gdb_packet_buffer;
@@ -1992,14 +2146,23 @@ int gdb_input_inner(struct connection *connection)
        struct gdb_connection *gdb_con = connection->priv;
        static int extended_protocol = 0;
 
-       /* drain input buffer */
+       /* drain input buffer. If one of the packets fail, then an error
+        * packet is replied, if applicable.
+        *
+        * This loop will terminate and the error code is returned.
+        *
+        * The calling fn will check if this error is something that
+        * can be recovered from, or if the connection must be closed.
+        *
+        * If the error is recoverable, this fn is called again to
+        * drain the rest of the buffer.
+        */
        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,48 +2194,70 @@ int gdb_input_inner(struct connection *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':
                                        {
-                                               int retval = ERROR_OK;
-
-                                               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 +2273,7 @@ int gdb_input_inner(struct connection *connection)
                                                }
                                                gdb_con->sync = false;
 
-                                               if ((retval!=ERROR_OK) || nostep)
+                                               if (!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 +2293,35 @@ int gdb_input_inner(struct connection *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);
+                                                               /* Here we don't want packet processing to stop even if this fails,
+                                                                * so we use a local variable instead of retval. */
+                                                               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 +2340,10 @@ int gdb_input_inner(struct connection *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 +2376,7 @@ int gdb_input_inner(struct connection *connection)
        return ERROR_OK;
 }
 
-int gdb_input(struct connection *connection)
+static int gdb_input(struct connection *connection)
 {
        int retval = gdb_input_inner(connection);
        struct gdb_connection *gdb_con = connection->priv;
@@ -2195,55 +2391,57 @@ int gdb_input(struct connection *connection)
        return ERROR_OK;
 }
 
-int gdb_init(void)
+static int gdb_target_start(struct target *target, const char *port)
 {
-       struct gdb_service *gdb_service;
-       struct target *target = all_targets;
+       struct gdb_service *gdb_service = malloc(sizeof(struct gdb_service));
+       if (NULL == gdb_service)
+               return -ENOMEM;
 
-       if (!target)
+       gdb_service->target = target;
+
+       return add_service("gdb",
+                       port, 1, &gdb_new_connection, &gdb_input,
+                       &gdb_connection_closed, gdb_service);
+}
+
+static int gdb_target_add_one(struct target *target)
+{
+       int retval = gdb_target_start(target, gdb_port_next);
+       if (retval == ERROR_OK)
        {
-               LOG_WARNING("no gdb ports allocated as no target has been specified");
-               return ERROR_OK;
+               long portnumber;
+               /* If we can parse the port number
+                * then we increment the port number for the next target.
+                */
+               char *end;
+               portnumber = strtol(gdb_port_next, &end, 0);
+               if (!*end)
+               {
+                       if (parse_long(gdb_port_next, &portnumber) == ERROR_OK)
+                       {
+                               free((void *)gdb_port_next);
+                               gdb_port_next = alloc_printf("%d", portnumber+1);
+                       }
+               }
        }
+       return retval;
+}
 
-       if (gdb_port == 0 && server_use_pipes == 0)
+int gdb_target_add_all(struct target *target)
+{
+       if (NULL == target)
        {
-               LOG_INFO("gdb port disabled");
+               LOG_WARNING("gdb services need one or more targets defined");
                return ERROR_OK;
        }
 
-       if (server_use_pipes)
-       {
-               /* only a single gdb connection when using a pipe */
-
-               gdb_service = malloc(sizeof(struct gdb_service));
-               gdb_service->target = target;
-
-               add_service("gdb", CONNECTION_PIPE, 0, 1, gdb_new_connection, gdb_input, gdb_connection_closed, gdb_service);
-
-               LOG_DEBUG("gdb service for target %s using pipes",
-                               target_get_name(target));
-       }
-       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 +2449,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 +2469,57 @@ 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_pipe_command, &gdb_port);
+       if (ERROR_OK == retval) {
+               free((void*)gdb_port_next);
+               gdb_port_next = strdup(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 +2532,69 @@ 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 = "Normally gdb listens to a TCP/IP port. Each subsequent GDB "
+                               "server listens for the next port number after the "
+                               "base port number specified. "
+                               "No arguments reports GDB port. \"pipe\" means listen to stdin "
+                               "output to stdout, an integer is base port number, \"disable\" disables "
+                               "port. Any other string is are interpreted as named pipe to listen to. "
+                               "Output pipe is the same name as input pipe, but with 'o' appended.",
+               .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;
+       gdb_port = strdup("3333");
+       gdb_port_next = strdup("3333");
+       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)