gdb_server: fix HW thread status at gdb attach
[fw/openocd] / src / server / gdb_server.c
index a9c2a64062f3d4971cf80b8d8f45ea71371a6f15..9e44287fd768be72b1c6a710bd120af75ea9b255 100644 (file)
@@ -43,6 +43,7 @@
 #include <target/register.h>
 #include <target/target.h>
 #include <target/target_type.h>
+#include <target/semihosting_common.h>
 #include "server.h"
 #include <flash/nor/core.h>
 #include "gdb_server.h"
@@ -67,10 +68,10 @@ struct target_desc_format {
 
 /* private connection data for GDB */
 struct gdb_connection {
-       char buffer[GDB_BUFFER_SIZE];
+       char buffer[GDB_BUFFER_SIZE + 1]; /* Extra byte for null-termination */
        char *buf_p;
        int buf_cnt;
-       int ctrl_c;
+       bool ctrl_c;
        enum target_state frontend_state;
        struct image *vflash_image;
        bool closed;
@@ -90,6 +91,8 @@ struct gdb_connection {
         * normally we reply with a S reply via gdb_last_signal_packet.
         * as a side note this behaviour only effects gdb > 6.8 */
        bool attached;
+       /* set when extended protocol is used */
+       bool extended_protocol;
        /* temporarily used for target description support */
        struct target_desc_format target_desc;
        /* temporarily used for thread list support */
@@ -153,6 +156,8 @@ static int gdb_last_signal(struct target *target)
                        return 0x05;    /* SIGTRAP */
                case DBG_REASON_SINGLESTEP:
                        return 0x05;    /* SIGTRAP */
+               case DBG_REASON_EXC_CATCH:
+                       return 0x05;
                case DBG_REASON_NOTHALTED:
                        return 0x0;             /* no signal... shouldn't happen */
                default:
@@ -272,9 +277,9 @@ static int gdb_get_char_inner(struct connection *connection, int *next_char)
        gdb_con->buf_cnt--;
        *next_char = *(gdb_con->buf_p++);
        if (gdb_con->buf_cnt > 0)
-               connection->input_pending = 1;
+               connection->input_pending = true;
        else
-               connection->input_pending = 0;
+               connection->input_pending = false;
 #ifdef _DEBUG_GDB_IO_
        LOG_DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char);
 #endif
@@ -297,9 +302,9 @@ static inline int gdb_get_char_fast(struct connection *connection,
                *next_char = **buf_p;
                (*buf_p)++;
                if (*buf_cnt > 0)
-                       connection->input_pending = 1;
+                       connection->input_pending = true;
                else
-                       connection->input_pending = 0;
+                       connection->input_pending = false;
 
 #ifdef _DEBUG_GDB_IO_
                LOG_DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char);
@@ -439,7 +444,7 @@ static int gdb_put_packet_inner(struct connection *connection,
                        log_remove_callback(gdb_log_callback, connection);
                        LOG_WARNING("negative reply, retrying");
                } else if (reply == 0x3) {
-                       gdb_con->ctrl_c = 1;
+                       gdb_con->ctrl_c = true;
                        retval = gdb_get_char(connection, &reply);
                        if (retval != ERROR_OK)
                                return retval;
@@ -644,7 +649,7 @@ static int gdb_get_packet_inner(struct connection *connection,
                                        LOG_WARNING("negative acknowledgment, but no packet pending");
                                        break;
                                case 0x3:
-                                       gdb_con->ctrl_c = 1;
+                                       gdb_con->ctrl_c = true;
                                        *len = 0;
                                        return ERROR_OK;
                                default:
@@ -722,7 +727,7 @@ static int gdb_output(struct command_context *context, const char *line)
 static void gdb_signal_reply(struct target *target, struct connection *connection)
 {
        struct gdb_connection *gdb_connection = connection->priv;
-       char sig_reply[45];
+       char sig_reply[65];
        char stop_reason[20];
        char current_thread[25];
        int sig_reply_len;
@@ -733,17 +738,25 @@ static void gdb_signal_reply(struct target *target, struct connection *connectio
        if (target->debug_reason == DBG_REASON_EXIT) {
                sig_reply_len = snprintf(sig_reply, sizeof(sig_reply), "W00");
        } else {
+               struct target *ct;
+               if (target->rtos != NULL) {
+                       target->rtos->current_threadid = target->rtos->current_thread;
+                       target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &ct);
+               } else {
+                       ct = target;
+               }
+
                if (gdb_connection->ctrl_c) {
                        signal_var = 0x2;
                } else
-                       signal_var = gdb_last_signal(target);
+                       signal_var = gdb_last_signal(ct);
 
                stop_reason[0] = '\0';
-               if (target->debug_reason == DBG_REASON_WATCHPOINT) {
+               if (ct->debug_reason == DBG_REASON_WATCHPOINT) {
                        enum watchpoint_rw hit_wp_type;
                        target_addr_t hit_wp_address;
 
-                       if (watchpoint_hit(target, &hit_wp_type, &hit_wp_address) == ERROR_OK) {
+                       if (watchpoint_hit(ct, &hit_wp_type, &hit_wp_address) == ERROR_OK) {
 
                                switch (hit_wp_type) {
                                        case WPT_WRITE:
@@ -765,20 +778,14 @@ static void gdb_signal_reply(struct target *target, struct connection *connectio
                }
 
                current_thread[0] = '\0';
-               if (target->rtos != NULL) {
-                       struct target *ct;
-                       snprintf(current_thread, sizeof(current_thread), "thread:%016" PRIx64 ";",
+               if (target->rtos != NULL)
+                       snprintf(current_thread, sizeof(current_thread), "thread:%" PRIx64 ";",
                                        target->rtos->current_thread);
-                       target->rtos->current_threadid = target->rtos->current_thread;
-                       target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &ct);
-                       if (!gdb_connection->ctrl_c)
-                               signal_var = gdb_last_signal(ct);
-               }
 
                sig_reply_len = snprintf(sig_reply, sizeof(sig_reply), "T%2.2x%s%s",
                                signal_var, stop_reason, current_thread);
 
-               gdb_connection->ctrl_c = 0;
+               gdb_connection->ctrl_c = false;
        }
 
        gdb_put_packet(connection, sig_reply, sig_reply_len);
@@ -795,7 +802,7 @@ static void gdb_fileio_reply(struct target *target, struct connection *connectio
        if (strcmp(target->fileio_info->identifier, "open") == 0)
                sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64 ",%" PRIx64 ",%" PRIx64, target->fileio_info->identifier,
                                target->fileio_info->param_1,
-                               target->fileio_info->param_2,
+                               target->fileio_info->param_2 + 1,       /* len + trailing zero */
                                target->fileio_info->param_3,
                                target->fileio_info->param_4);
        else if (strcmp(target->fileio_info->identifier, "close") == 0)
@@ -819,13 +826,13 @@ static void gdb_fileio_reply(struct target *target, struct connection *connectio
        else if (strcmp(target->fileio_info->identifier, "rename") == 0)
                sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64 ",%" PRIx64 "/%" PRIx64, target->fileio_info->identifier,
                                target->fileio_info->param_1,
-                               target->fileio_info->param_2,
+                               target->fileio_info->param_2 + 1,       /* len + trailing zero */
                                target->fileio_info->param_3,
-                               target->fileio_info->param_4);
+                               target->fileio_info->param_4 + 1);      /* len + trailing zero */
        else if (strcmp(target->fileio_info->identifier, "unlink") == 0)
                sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64, target->fileio_info->identifier,
                                target->fileio_info->param_1,
-                               target->fileio_info->param_2);
+                               target->fileio_info->param_2 + 1);      /* len + trailing zero */
        else if (strcmp(target->fileio_info->identifier, "stat") == 0)
                sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64 ",%" PRIx64, target->fileio_info->identifier,
                                target->fileio_info->param_1,
@@ -845,7 +852,7 @@ static void gdb_fileio_reply(struct target *target, struct connection *connectio
        else if (strcmp(target->fileio_info->identifier, "system") == 0)
                sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64, target->fileio_info->identifier,
                                target->fileio_info->param_1,
-                               target->fileio_info->param_2);
+                               target->fileio_info->param_2 + 1);      /* len + trailing zero */
        else if (strcmp(target->fileio_info->identifier, "exit") == 0) {
                /* If target hits exit syscall, report to GDB the program is terminated.
                 * In addition, let target run its own exit syscall handler. */
@@ -901,7 +908,6 @@ static void gdb_frontend_halted(struct target *target, struct connection *connec
 static int gdb_target_callback_event_handler(struct target *target,
                enum target_event event, void *priv)
 {
-       int retval;
        struct connection *connection = priv;
        struct gdb_service *gdb_service = connection->service->priv;
 
@@ -915,11 +921,6 @@ static int gdb_target_callback_event_handler(struct target *target,
                case TARGET_EVENT_HALTED:
                        target_call_event_callbacks(target, TARGET_EVENT_GDB_END);
                        break;
-               case TARGET_EVENT_GDB_FLASH_ERASE_START:
-                       retval = jtag_execute_queue();
-                       if (retval != ERROR_OK)
-                               return retval;
-                       break;
                default:
                        break;
        }
@@ -936,11 +937,12 @@ static int gdb_new_connection(struct connection *connection)
 
        target = get_target_from_connection(connection);
        connection->priv = gdb_connection;
+       connection->cmd_ctx->current_target = target;
 
        /* initialize gdb connection information */
        gdb_connection->buf_p = gdb_connection->buffer;
        gdb_connection->buf_cnt = 0;
-       gdb_connection->ctrl_c = 0;
+       gdb_connection->ctrl_c = false;
        gdb_connection->frontend_state = TARGET_HALTED;
        gdb_connection->vflash_image = NULL;
        gdb_connection->closed = false;
@@ -949,6 +951,7 @@ static int gdb_new_connection(struct connection *connection)
        gdb_connection->sync = false;
        gdb_connection->mem_write_error = false;
        gdb_connection->attached = true;
+       gdb_connection->extended_protocol = false;
        gdb_connection->target_desc.tdesc = NULL;
        gdb_connection->target_desc.tdesc_length = 0;
        gdb_connection->thread_list = NULL;
@@ -966,15 +969,6 @@ static int gdb_new_connection(struct connection *connection)
        breakpoint_clear_target(target);
        watchpoint_clear_target(target);
 
-       if (target->rtos) {
-               /* clean previous rtos session if supported*/
-               if (target->rtos->type->clean)
-                       target->rtos->type->clean(target);
-
-               /* update threads */
-               rtos_update_threads(target);
-       }
-
        /* remove the initial ACK from the incoming buffer */
        retval = gdb_get_char(connection, &initial_ack);
        if (retval != ERROR_OK)
@@ -987,21 +981,29 @@ static int gdb_new_connection(struct connection *connection)
                gdb_putback_char(connection, initial_ack);
        target_call_event_callbacks(target, TARGET_EVENT_GDB_ATTACH);
 
+       if (target->rtos) {
+               /* clean previous rtos session if supported*/
+               if (target->rtos->type->clean)
+                       target->rtos->type->clean(target);
+
+               /* update threads */
+               rtos_update_threads(target);
+       }
+
        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++) {
+               for (unsigned int i = 0; i < flash_get_bank_count(); i++) {
                        struct flash_bank *p;
                        p = get_flash_bank_by_num_noprobe(i);
                        if (p->target != target)
                                continue;
                        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 " \
+                               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;
                        }
@@ -1016,6 +1018,17 @@ static int gdb_new_connection(struct connection *connection)
                        target_name(target),
                        target_state_name(target));
 
+       if (!target_was_examined(target)) {
+               LOG_ERROR("Target %s not examined yet, refuse gdb connection %d!",
+                                 target_name(target), gdb_actual_connections);
+               gdb_actual_connections--;
+               return ERROR_TARGET_NOT_EXAMINED;
+       }
+
+       if (target->state != TARGET_HALTED)
+               LOG_WARNING("GDB connection %d on target %s not halted",
+                                       gdb_actual_connections, target_name(target));
+
        /* DANGER! If we fail subsequently, we must remove this handler,
         * otherwise we occasionally see crashes as the timer can invoke the
         * callback fn.
@@ -1054,11 +1067,8 @@ static int gdb_connection_closed(struct connection *connection)
        /* if this connection registered a debug-message receiver delete it */
        delete_debug_msg_receiver(connection->cmd_ctx, target);
 
-       if (connection->priv) {
-               free(connection->priv);
-               connection->priv = NULL;
-       } else
-               LOG_ERROR("BUG: connection->priv == NULL");
+       free(connection->priv);
+       connection->priv = NULL;
 
        target_unregister_event_callback(gdb_target_callback_event_handler, connection);
 
@@ -1178,8 +1188,11 @@ static int gdb_get_registers_packet(struct connection *connection,
        if (retval != ERROR_OK)
                return gdb_error(connection, retval);
 
-       for (i = 0; i < reg_list_size; i++)
+       for (i = 0; i < reg_list_size; i++) {
+               if (reg_list[i] == NULL || reg_list[i]->exist == false || reg_list[i]->hidden)
+                       continue;
                reg_packet_size += DIV_ROUND_UP(reg_list[i]->size, 8) * 2;
+       }
 
        assert(reg_packet_size > 0);
 
@@ -1190,6 +1203,8 @@ static int gdb_get_registers_packet(struct connection *connection,
        reg_packet_p = reg_packet;
 
        for (i = 0; i < reg_list_size; i++) {
+               if (reg_list[i] == NULL || reg_list[i]->exist == false || reg_list[i]->hidden)
+                       continue;
                if (!reg_list[i]->valid) {
                        retval = reg_list[i]->type->get(reg_list[i]);
                        if (retval != ERROR_OK && gdb_report_register_access_error) {
@@ -1295,7 +1310,10 @@ static int gdb_get_register_packet(struct connection *connection,
        LOG_DEBUG("-");
 #endif
 
-       retval = target_get_gdb_reg_list(target, &reg_list, &reg_list_size,
+       if ((target->rtos != NULL) && (ERROR_OK == rtos_get_gdb_reg(connection, reg_num)))
+               return ERROR_OK;
+
+       retval = target_get_gdb_reg_list_noread(target, &reg_list, &reg_list_size,
                        REG_CLASS_ALL);
        if (retval != ERROR_OK)
                return gdb_error(connection, retval);
@@ -1314,7 +1332,7 @@ static int gdb_get_register_packet(struct connection *connection,
                }
        }
 
-       reg_packet = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2 + 1); /* plus one for string termination null */
+       reg_packet = calloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2 + 1, 1); /* plus one for string termination null */
 
        gdb_str_to_target(target, reg_packet, reg_list[reg_num]);
 
@@ -1331,36 +1349,49 @@ static int gdb_set_register_packet(struct connection *connection,
 {
        struct target *target = get_target_from_connection(connection);
        char *separator;
-       uint8_t *bin_buf;
        int reg_num = strtoul(packet + 1, &separator, 16);
        struct reg **reg_list;
        int reg_list_size;
        int retval;
 
+#ifdef _DEBUG_GDB_IO_
        LOG_DEBUG("-");
+#endif
 
-       retval = target_get_gdb_reg_list(target, &reg_list, &reg_list_size,
+       if (*separator != '=') {
+               LOG_ERROR("GDB 'set register packet', but no '=' following the register number");
+               return ERROR_SERVER_REMOTE_CLOSED;
+       }
+       size_t chars = strlen(separator + 1);
+       uint8_t *bin_buf = malloc(chars / 2);
+       gdb_target_to_reg(target, separator + 1, chars, bin_buf);
+
+       if ((target->rtos != NULL) &&
+                       (ERROR_OK == rtos_set_reg(connection, reg_num, bin_buf))) {
+               free(bin_buf);
+               gdb_put_packet(connection, "OK", 2);
+               return ERROR_OK;
+       }
+
+       retval = target_get_gdb_reg_list_noread(target, &reg_list, &reg_list_size,
                        REG_CLASS_ALL);
-       if (retval != ERROR_OK)
+       if (retval != ERROR_OK) {
+               free(bin_buf);
                return gdb_error(connection, retval);
+       }
 
        if (reg_list_size <= reg_num) {
                LOG_ERROR("gdb requested a non-existing register");
+               free(bin_buf);
+               free(reg_list);
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
-       if (*separator != '=') {
-               LOG_ERROR("GDB 'set register packet', but no '=' following the register number");
-               return ERROR_SERVER_REMOTE_CLOSED;
-       }
-
-       /* convert from GDB-string (target-endian) to hex-string (big-endian) */
-       bin_buf = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8));
-       int chars = (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2);
-
-       if ((unsigned int)chars != strlen(separator + 1)) {
-               LOG_ERROR("gdb sent a packet with wrong register size");
+       if (chars != (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2)) {
+               LOG_ERROR("gdb sent %zu bits for a %" PRIu32 "-bit register (%s)",
+                               chars * 4, reg_list[reg_num]->size, reg_list[reg_num]->name);
                free(bin_buf);
+               free(reg_list);
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
@@ -1395,8 +1426,6 @@ static int gdb_error(struct connection *connection, int retval)
 
 /* We don't have to worry about the default 2 second timeout for GDB packets,
  * because GDB breaks up large memory reads into smaller reads.
- *
- * 8191 bytes by the looks of it. Why 8191 bytes instead of 8192?????
  */
 static int gdb_read_memory_packet(struct connection *connection,
                char const *packet, int packet_size)
@@ -1425,7 +1454,7 @@ static int gdb_read_memory_packet(struct connection *connection,
 
        if (!len) {
                LOG_WARNING("invalid read memory packet received (len == 0)");
-               gdb_put_packet(connection, NULL, 0);
+               gdb_put_packet(connection, "", 0);
                return ERROR_OK;
        }
 
@@ -1630,7 +1659,7 @@ static int gdb_breakpoint_watchpoint_packet(struct connection *connection,
        char *separator;
        int retval;
 
-       LOG_DEBUG("-");
+       LOG_DEBUG("[%s]", target_name(target));
 
        type = strtoul(packet + 1, &separator, 16);
 
@@ -1710,8 +1739,8 @@ static int gdb_breakpoint_watchpoint_packet(struct connection *connection,
 /* 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, ...)
+static __attribute__ ((format (PRINTF_ATTRIBUTE_FORMAT, 5, 6))) void xml_printf(int *retval,
+               char **xml, int *pos, int *size, const char *fmt, ...)
 {
        if (*retval != ERROR_OK)
                return;
@@ -1726,8 +1755,7 @@ static void xml_printf(int *retval, char **xml, int *pos, int *size,
                        char *t = *xml;
                        *xml = realloc(*xml, *size);
                        if (*xml == NULL) {
-                               if (t)
-                                       free(t);
+                               free(t);
                                *retval = ERROR_SERVER_REMOTE_CLOSED;
                                return;
                        }
@@ -1809,8 +1837,7 @@ static int gdb_memory_map(struct connection *connection,
        int length;
        char *separator;
        target_addr_t ram_start = 0;
-       int i;
-       int target_flash_banks = 0;
+       unsigned int target_flash_banks = 0;
 
        /* skip command character */
        packet += 23;
@@ -1826,7 +1853,7 @@ static int gdb_memory_map(struct connection *connection,
         */
        banks = malloc(sizeof(struct flash_bank *)*flash_get_bank_count());
 
-       for (i = 0; i < flash_get_bank_count(); i++) {
+       for (unsigned int i = 0; i < flash_get_bank_count(); i++) {
                p = get_flash_bank_by_num_noprobe(i);
                if (p->target != target)
                        continue;
@@ -1842,8 +1869,7 @@ static int gdb_memory_map(struct connection *connection,
        qsort(banks, target_flash_banks, sizeof(struct flash_bank *),
                compare_bank);
 
-       for (i = 0; i < target_flash_banks; i++) {
-               int j;
+       for (unsigned int i = 0; i < target_flash_banks; i++) {
                unsigned sector_size = 0;
                unsigned group_len = 0;
 
@@ -1852,7 +1878,7 @@ static int gdb_memory_map(struct connection *connection,
                if (ram_start < p->base)
                        xml_printf(&retval, &xml, &pos, &size,
                                "<memory type=\"ram\" start=\"" TARGET_ADDR_FMT "\" "
-                               "length=\"0x%x\"/>\n",
+                               "length=\"" TARGET_ADDR_FMT "\"/>\n",
                                ram_start, p->base - ram_start);
 
                /* Report adjacent groups of same-size sectors.  So for
@@ -1861,7 +1887,7 @@ static int gdb_memory_map(struct connection *connection,
                 * 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++) {
+               for (unsigned int j = 0; j < p->num_sectors; j++) {
 
                        /* Maybe start a new group of sectors. */
                        if (sector_size == 0) {
@@ -1909,11 +1935,10 @@ static int gdb_memory_map(struct connection *connection,
        if (ram_start != 0)
                xml_printf(&retval, &xml, &pos, &size,
                        "<memory type=\"ram\" start=\"" TARGET_ADDR_FMT "\" "
-                       "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
-        */
+                       "length=\"" TARGET_ADDR_FMT "\"/>\n",
+                       ram_start, target_address_max(target) - ram_start + 1);
+       /* ELSE a flash chip could be at the very end of the address space, in
+        * which case ram_start will be precisely 0 */
 
        free(banks);
 
@@ -2010,7 +2035,7 @@ static int lookup_add_arch_defined_types(char const **arch_defined_types_list[],
 
 static int gdb_generate_reg_type_description(struct target *target,
                char **tdesc, int *pos, int *size, struct reg_data_type *type,
-               char const **arch_defined_types_list[], int * num_arch_defined_types)
+               char const **arch_defined_types_list[], int *num_arch_defined_types)
 {
        int retval = ERROR_OK;
 
@@ -2025,7 +2050,7 @@ static int gdb_generate_reg_type_description(struct target *target,
                }
                /* <vector id="id" type="type" count="count"/> */
                xml_printf(&retval, tdesc, pos, size,
-                               "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
+                               "<vector id=\"%s\" type=\"%s\" count=\"%" PRIu32 "\"/>\n",
                                type->id, type->reg_type_vector->type->id,
                                type->reg_type_vector->count);
 
@@ -2072,11 +2097,11 @@ static int gdb_generate_reg_type_description(struct target *target,
                         *  <field name="name" start="start" end="end"/> ...
                         * </struct> */
                        xml_printf(&retval, tdesc, pos, size,
-                                       "<struct id=\"%s\" size=\"%d\">\n",
+                                       "<struct id=\"%s\" size=\"%" PRIu32 "\">\n",
                                        type->id, type->reg_type_struct->size);
                        while (field != NULL) {
                                xml_printf(&retval, tdesc, pos, size,
-                                               "<field name=\"%s\" start=\"%d\" end=\"%d\" type=\"%s\" />\n",
+                                               "<field name=\"%s\" start=\"%" PRIu32 "\" end=\"%" PRIu32 "\" type=\"%s\" />\n",
                                                field->name, field->bitfield->start, field->bitfield->end,
                                                gdb_get_reg_type_name(field->bitfield->type));
 
@@ -2117,14 +2142,14 @@ static int gdb_generate_reg_type_description(struct target *target,
                 *  <field name="name" start="start" end="end"/> ...
                 * </flags> */
                xml_printf(&retval, tdesc, pos, size,
-                               "<flags id=\"%s\" size=\"%d\">\n",
+                               "<flags id=\"%s\" size=\"%" PRIu32 "\">\n",
                                type->id, type->reg_type_flags->size);
 
                struct reg_data_type_flags_field *field;
                field = type->reg_type_flags->fields;
                while (field != NULL) {
                        xml_printf(&retval, tdesc, pos, size,
-                                       "<field name=\"%s\" start=\"%d\" end=\"%d\" type=\"%s\" />\n",
+                                       "<field name=\"%s\" start=\"%" PRIu32 "\" end=\"%" PRIu32 "\" type=\"%s\" />\n",
                                        field->name, field->bitfield->start, field->bitfield->end,
                                        gdb_get_reg_type_name(field->bitfield->type));
 
@@ -2151,7 +2176,7 @@ static int get_reg_features_list(struct target *target, char const **feature_lis
        *feature_list = calloc(1, sizeof(char *));
 
        for (int i = 0; i < reg_list_size; i++) {
-               if (reg_list[i]->exist == false)
+               if (reg_list[i]->exist == false || reg_list[i]->hidden)
                        continue;
 
                if (reg_list[i]->feature != NULL
@@ -2186,17 +2211,15 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
        int retval = ERROR_OK;
        struct reg **reg_list = NULL;
        int reg_list_size;
+       char const *architecture;
        char const **features = NULL;
-       char const **arch_defined_types = NULL;
        int feature_list_size = 0;
-       int num_arch_defined_types = 0;
        char *tdesc = NULL;
        int pos = 0;
        int size = 0;
 
-       arch_defined_types = calloc(1, sizeof(char *));
 
-       retval = target_get_gdb_reg_list(target, &reg_list,
+       retval = target_get_gdb_reg_list_noread(target, &reg_list,
                        &reg_list_size, REG_CLASS_ALL);
 
        if (retval != ERROR_OK) {
@@ -2227,10 +2250,19 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
                        "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n"
                        "<target version=\"1.0\">\n");
 
+       /* generate architecture element if supported by target */
+       architecture = target_get_gdb_arch(target);
+       if (architecture != NULL)
+               xml_printf(&retval, &tdesc, &pos, &size,
+                               "<architecture>%s</architecture>\n", architecture);
+
        /* generate target description according to register list */
        if (features != NULL) {
                while (features[current_feature]) {
+                       char const **arch_defined_types = NULL;
+                       int num_arch_defined_types = 0;
 
+                       arch_defined_types = calloc(1, sizeof(char *));
                        xml_printf(&retval, &tdesc, &pos, &size,
                                        "<feature name=\"%s\">\n",
                                        features[current_feature]);
@@ -2238,7 +2270,7 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
                        int i;
                        for (i = 0; i < reg_list_size; i++) {
 
-                               if (reg_list[i]->exist == false)
+                               if (reg_list[i]->exist == false || reg_list[i]->hidden)
                                        continue;
 
                                if (strcmp(reg_list[i]->feature->name, features[current_feature]))
@@ -2270,9 +2302,9 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
                                xml_printf(&retval, &tdesc, &pos, &size,
                                                "<reg name=\"%s\"", reg_list[i]->name);
                                xml_printf(&retval, &tdesc, &pos, &size,
-                                               " bitsize=\"%d\"", reg_list[i]->size);
+                                               " bitsize=\"%" PRIu32 "\"", reg_list[i]->size);
                                xml_printf(&retval, &tdesc, &pos, &size,
-                                               " regnum=\"%d\"", reg_list[i]->number);
+                                               " regnum=\"%" PRIu32 "\"", reg_list[i]->number);
                                if (reg_list[i]->caller_save)
                                        xml_printf(&retval, &tdesc, &pos, &size,
                                                        " save-restore=\"yes\"");
@@ -2295,6 +2327,7 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
                                        "</feature>\n");
 
                        current_feature++;
+                       free(arch_defined_types);
                }
        }
 
@@ -2304,7 +2337,6 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
 error:
        free(features);
        free(reg_list);
-       free(arch_defined_types);
 
        if (retval == ERROR_OK)
                *tdesc_out = tdesc;
@@ -2376,7 +2408,9 @@ static int gdb_target_description_supported(struct target *target, int *supporte
        char const **features = NULL;
        int feature_list_size = 0;
 
-       retval = target_get_gdb_reg_list(target, &reg_list,
+       char const *architecture = target_get_gdb_arch(target);
+
+       retval = target_get_gdb_reg_list_noread(target, &reg_list,
                        &reg_list_size, REG_CLASS_ALL);
        if (retval != ERROR_OK) {
                LOG_ERROR("get register list failed");
@@ -2397,7 +2431,7 @@ static int gdb_target_description_supported(struct target *target, int *supporte
        }
 
        if (supported) {
-               if (feature_list_size)
+               if (architecture || feature_list_size)
                        *supported = 1;
                else
                        *supported = 0;
@@ -2442,7 +2476,7 @@ static int gdb_generate_thread_list(struct target *target, char **thread_list_ou
                                        xml_printf(&retval, &thread_list, &pos, &size,
                                                   ", ");
                                xml_printf(&retval, &thread_list, &pos, &size,
-                                          thread_detail->extra_info_str);
+                                          "%s", thread_detail->extra_info_str);
                        }
 
                        xml_printf(&retval, &thread_list, &pos, &size,
@@ -2482,7 +2516,7 @@ static int gdb_get_thread_list_chunk(struct target *target, char **thread_list,
                transfer_type = 'l';
 
        *chunk = malloc(length + 2 + 3);
-    /* Allocating extra 3 bytes prevents false positive valgrind report
+       /* Allocating extra 3 bytes prevents false positive valgrind report
         * of strlen(chunk) word access:
         * Invalid read of size 4
         * Address 0x4479934 is 44 bytes inside a block of size 45 alloc'd */
@@ -2594,7 +2628,7 @@ static int gdb_query_packet(struct connection *connection,
                        &pos,
                        &size,
                        "PacketSize=%x;qXfer:memory-map:read%c;qXfer:features:read%c;qXfer:threads:read+;QStartNoAckMode+;vContSupported+",
-                       (GDB_BUFFER_SIZE - 1),
+                       GDB_BUFFER_SIZE,
                        ((gdb_use_memory_map == 1) && (flash_get_bank_count() > 0)) ? '+' : '-',
                        (gdb_target_desc_supported == 1) ? '+' : '-');
 
@@ -2771,13 +2805,11 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
 
                                if (parse[0] == 'c') {
                                        parse += 1;
-                                       packet_size -= 1;
 
                                        /* check if thread-id follows */
                                        if (parse[0] == ':') {
                                                int64_t tid;
                                                parse += 1;
-                                               packet_size -= 1;
 
                                                tid = strtoll(parse, &endp, 16);
                                                if (tid == thread_id) {
@@ -2829,7 +2861,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
                        if (gdb_connection->sync) {
                                gdb_connection->sync = false;
                                if (ct->state == TARGET_HALTED) {
-                                       LOG_WARNING("stepi ignored. GDB will now fetch the register state " \
+                                       LOG_DEBUG("stepi ignored. GDB will now fetch the register state "
                                                                        "from the target.");
                                        gdb_sig_halted(connection);
                                        log_remove_callback(gdb_log_callback, connection);
@@ -2863,14 +2895,103 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p
        return false;
 }
 
+static char *next_hex_encoded_field(const char **str, char sep)
+{
+       size_t hexlen;
+       const char *hex = *str;
+       if (hex[0] == '\0')
+               return NULL;
+
+       const char *end = strchr(hex, sep);
+       if (end == NULL)
+               hexlen = strlen(hex);
+       else
+               hexlen = end - hex;
+       *str = hex + hexlen + 1;
+
+       if (hexlen % 2 != 0) {
+               /* Malformed hex data */
+               return NULL;
+       }
+
+       size_t count = hexlen / 2;
+       char *decoded = malloc(count + 1);
+       if (decoded == NULL)
+               return NULL;
+
+       size_t converted = unhexify((void *)decoded, hex, count);
+       if (converted != count) {
+               free(decoded);
+               return NULL;
+       }
+
+       decoded[count] = '\0';
+       return decoded;
+}
+
+/* handle extended restart packet */
+static void gdb_restart_inferior(struct connection *connection, const char *packet, int packet_size)
+{
+       struct gdb_connection *gdb_con = connection->priv;
+       struct target *target = get_target_from_connection(connection);
+
+       breakpoint_clear_target(target);
+       watchpoint_clear_target(target);
+       command_run_linef(connection->cmd_ctx, "ocd_gdb_restart %s",
+                       target_name(target));
+       /* set connection as attached after reset */
+       gdb_con->attached = true;
+       /*  info rtos parts */
+       gdb_thread_packet(connection, packet, packet_size);
+}
+
+static bool gdb_handle_vrun_packet(struct connection *connection, const char *packet, int packet_size)
+{
+       struct target *target = get_target_from_connection(connection);
+       const char *parse = packet;
+
+       /* Skip "vRun" */
+       parse += 4;
+
+       if (parse[0] != ';')
+               return false;
+       parse++;
+
+       /* Skip first field "filename"; don't know what to do with it. */
+       free(next_hex_encoded_field(&parse, ';'));
+
+       char *cmdline = next_hex_encoded_field(&parse, ';');
+       char *arg;
+       while (cmdline != NULL && (arg = next_hex_encoded_field(&parse, ';')) != NULL) {
+               char *new_cmdline = alloc_printf("%s %s", cmdline, arg);
+               free(cmdline);
+               free(arg);
+               cmdline = new_cmdline;
+       }
+
+       if (cmdline != NULL) {
+               if (target->semihosting != NULL) {
+                       LOG_INFO("GDB set inferior command line to '%s'", cmdline);
+                       free(target->semihosting->cmdline);
+                       target->semihosting->cmdline = cmdline;
+               } else {
+                       LOG_INFO("GDB set inferior command line to '%s' but semihosting is unavailable", cmdline);
+                       free(cmdline);
+               }
+       }
+
+       gdb_restart_inferior(connection, packet, packet_size);
+       gdb_put_packet(connection, "S00", 3);
+       return true;
+}
+
 static int gdb_v_packet(struct connection *connection,
                char const *packet, int packet_size)
 {
        struct gdb_connection *gdb_connection = connection->priv;
-       struct target *target;
        int result;
 
-       target = get_target_from_connection(connection);
+       struct target *target = get_target_from_connection(connection);
 
        if (strncmp(packet, "vCont", 5) == 0) {
                bool handled;
@@ -2885,6 +3006,16 @@ static int gdb_v_packet(struct connection *connection,
                return ERROR_OK;
        }
 
+       if (strncmp(packet, "vRun", 4) == 0) {
+               bool handled;
+
+               handled = gdb_handle_vrun_packet(connection, packet, packet_size);
+               if (!handled)
+                       gdb_put_packet(connection, "", 0);
+
+               return ERROR_OK;
+       }
+
        /* if flash programming disabled - send a empty reply */
 
        if (gdb_flash_program == 0) {
@@ -2990,7 +3121,7 @@ static int gdb_v_packet(struct connection *connection,
                target_call_event_callbacks(target,
                                TARGET_EVENT_GDB_FLASH_WRITE_START);
                result = flash_write(target, gdb_connection->vflash_image,
-                       &written, 0);
+                       &written, false);
                target_call_event_callbacks(target,
                        TARGET_EVENT_GDB_FLASH_WRITE_END);
                if (result != ERROR_OK) {
@@ -3016,9 +3147,12 @@ static int gdb_v_packet(struct connection *connection,
 
 static int gdb_detach(struct connection *connection)
 {
-       target_call_event_callbacks(get_target_from_connection(connection),
-               TARGET_EVENT_GDB_DETACH);
-
+       /*
+        * Only reply "OK" to GDB
+        * it will close the connection and this will trigger a call to
+        * gdb_connection_closed() that will in turn trigger the event
+        * TARGET_EVENT_GDB_DETACH
+        */
        return gdb_put_packet(connection, "OK", 2);
 }
 
@@ -3094,14 +3228,14 @@ static void gdb_sig_halted(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];
+       static char gdb_packet_buffer[GDB_BUFFER_SIZE + 1]; /* Extra byte for null-termination */
 
        struct target *target;
        char const *packet = gdb_packet_buffer;
        int packet_size;
        int retval;
        struct gdb_connection *gdb_con = connection->priv;
-       static int extended_protocol;
+       static bool warn_use_ext;
 
        target = get_target_from_connection(connection);
 
@@ -3117,7 +3251,7 @@ static int gdb_input_inner(struct connection *connection)
         * drain the rest of the buffer.
         */
        do {
-               packet_size = GDB_BUFFER_SIZE-1;
+               packet_size = GDB_BUFFER_SIZE;
                retval = gdb_get_packet(connection, gdb_packet_buffer, &packet_size);
                if (retval != ERROR_OK)
                        return retval;
@@ -3178,6 +3312,12 @@ static int gdb_input_inner(struct connection *connection)
                                        break;
                                case '?':
                                        gdb_last_signal_packet(connection, packet, packet_size);
+                                       /* '?' is sent after the eventual '!' */
+                                       if (!warn_use_ext && !gdb_con->extended_protocol) {
+                                               warn_use_ext = true;
+                                               LOG_WARNING("Prefer GDB command \"target extended-remote %s\" instead of \"target remote %s\"",
+                                                                       connection->service->port, connection->service->port);
+                                       }
                                        break;
                                case 'c':
                                case 's':
@@ -3201,7 +3341,7 @@ static int gdb_input_inner(struct connection *connection)
                                                                "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, " \
+                                               LOG_WARNING("The target is not in the halted nor running stated, "
                                                                "stepi/continue ignored.");
                                                nostep = true;
                                        } else if ((packet[0] == 's') && gdb_con->sync) {
@@ -3210,7 +3350,7 @@ static int gdb_input_inner(struct connection *connection)
                                                 * make only the single stepping have the sync feature...
                                                 */
                                                nostep = true;
-                                               LOG_WARNING("stepi ignored. GDB will now fetch the register state " \
+                                               LOG_DEBUG("stepi ignored. GDB will now fetch the register state "
                                                                "from the target.");
                                        }
                                        gdb_con->sync = false;
@@ -3253,7 +3393,6 @@ static int gdb_input_inner(struct connection *connection)
                                        break;
                                case 'D':
                                        retval = gdb_detach(connection);
-                                       extended_protocol = 0;
                                        break;
                                case 'X':
                                        retval = gdb_write_memory_binary_packet(connection, packet, packet_size);
@@ -3261,7 +3400,7 @@ static int gdb_input_inner(struct connection *connection)
                                                return retval;
                                        break;
                                case 'k':
-                                       if (extended_protocol != 0) {
+                                       if (gdb_con->extended_protocol) {
                                                gdb_con->attached = false;
                                                break;
                                        }
@@ -3269,19 +3408,12 @@ static int gdb_input_inner(struct connection *connection)
                                        return ERROR_SERVER_REMOTE_CLOSED;
                                case '!':
                                        /* handle extended remote protocol */
-                                       extended_protocol = 1;
+                                       gdb_con->extended_protocol = true;
                                        gdb_put_packet(connection, "OK", 2);
                                        break;
                                case 'R':
                                        /* handle extended restart packet */
-                                       breakpoint_clear_target(target);
-                                       watchpoint_clear_target(target);
-                                       command_run_linef(connection->cmd_ctx, "ocd_gdb_restart %s",
-                                                       target_name(target));
-                                       /* set connection as attached after reset */
-                                       gdb_con->attached = true;
-                                       /*  info rtos parts */
-                                       gdb_thread_packet(connection, packet, packet_size);
+                                       gdb_restart_inferior(connection, packet, packet_size);
                                        break;
 
                                case 'j':
@@ -3313,7 +3445,7 @@ static int gdb_input_inner(struct connection *connection)
                                default:
                                        /* ignore unknown packets */
                                        LOG_DEBUG("ignoring 0x%2.2x packet", packet[0]);
-                                       gdb_put_packet(connection, NULL, 0);
+                                       gdb_put_packet(connection, "", 0);
                                        break;
                        }
 
@@ -3332,7 +3464,7 @@ static int gdb_input_inner(struct connection *connection)
                                        retval = target_poll(t);
                                if (retval != ERROR_OK)
                                        target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT);
-                               gdb_con->ctrl_c = 0;
+                               gdb_con->ctrl_c = false;
                        } else {
                                LOG_INFO("The target is not running when halt was requested, stopping GDB.");
                                target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT);
@@ -3368,15 +3500,17 @@ static int gdb_target_start(struct target *target, const char *port)
        if (NULL == gdb_service)
                return -ENOMEM;
 
+       LOG_INFO("starting gdb server for %s on %s", target_name(target), port);
+
        gdb_service->target = target;
        gdb_service->core[0] = -1;
        gdb_service->core[1] = -1;
        target->gdb_service = gdb_service;
 
        ret = add_service("gdb",
-                       port, 1, &gdb_new_connection, &gdb_input,
-                       &gdb_connection_closed, gdb_service);
-       /* initialialize all targets gdb service with the same pointer */
+                       port, target->gdb_max_connections, &gdb_new_connection, &gdb_input,
+                       &gdb_connection_closed, gdb_service, NULL);
+       /* initialize all targets gdb service with the same pointer */
        {
                struct target_list *head;
                struct target *curr;
@@ -3393,16 +3527,36 @@ static int gdb_target_start(struct target *target, const char *port)
 
 static int gdb_target_add_one(struct target *target)
 {
+       /*  one gdb instance per smp list */
+       if ((target->smp) && (target->gdb_service))
+               return ERROR_OK;
+
+       /* skip targets that cannot handle a gdb connections (e.g. mem_ap) */
+       if (!target_supports_gdb_connection(target)) {
+               LOG_DEBUG("skip gdb server for target %s", target_name(target));
+               return ERROR_OK;
+       }
+
+       if (target->gdb_port_override) {
+               if (strcmp(target->gdb_port_override, "disabled") == 0) {
+                       LOG_INFO("gdb port disabled");
+                       return ERROR_OK;
+               }
+               return gdb_target_start(target, target->gdb_port_override);
+       }
+
        if (strcmp(gdb_port, "disabled") == 0) {
                LOG_INFO("gdb port disabled");
                return ERROR_OK;
        }
 
-       /*  one gdb instance per smp list */
-       if ((target->smp) && (target->gdb_service))
-               return ERROR_OK;
        int retval = gdb_target_start(target, gdb_port_next);
        if (retval == ERROR_OK) {
+               /* save the port number so can be queried with
+                * $target_name cget -gdb-port
+                */
+               target->gdb_port_override = strdup(gdb_port_next);
+
                long portnumber;
                /* If we can parse the port number
                 * then we increment the port number for the next target.
@@ -3413,7 +3567,7 @@ static int gdb_target_add_one(struct target *target)
                        if (parse_long(gdb_port_next, &portnumber) == ERROR_OK) {
                                free(gdb_port_next);
                                if (portnumber) {
-                                       gdb_port_next = alloc_printf("%d", portnumber+1);
+                                       gdb_port_next = alloc_printf("%ld", portnumber+1);
                                } else {
                                        /* Don't increment if gdb_port is 0, since we're just
                                         * trying to allocate an unused port. */
@@ -3427,11 +3581,6 @@ static int gdb_target_add_one(struct target *target)
 
 int gdb_target_add_all(struct target *target)
 {
-       if (strcmp(gdb_port, "disabled") == 0) {
-               LOG_INFO("gdb server disabled");
-               return ERROR_OK;
-       }
-
        if (NULL == target) {
                LOG_WARNING("gdb services need one or more targets defined");
                return ERROR_OK;
@@ -3454,7 +3603,7 @@ COMMAND_HANDLER(handle_gdb_sync_command)
                return ERROR_COMMAND_SYNTAX_ERROR;
 
        if (current_gdb_connection == NULL) {
-               command_print(CMD_CTX,
+               command_print(CMD,
                        "gdb_sync command can only be run from within gdb using \"monitor gdb_sync\"");
                return ERROR_FAIL;
        }
@@ -3601,7 +3750,7 @@ static const struct command_registration gdb_command_handlers[] = {
        {
                .name = "gdb_port",
                .handler = handle_gdb_port_command,
-               .mode = COMMAND_ANY,
+               .mode = COMMAND_CONFIG,
                .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. "
@@ -3659,6 +3808,7 @@ static const struct command_registration gdb_command_handlers[] = {
                .handler = handle_gdb_save_tdesc_command,
                .mode = COMMAND_EXEC,
                .help = "Save the target description file",
+               .usage = "",
        },
        COMMAND_REGISTRATION_DONE
 };