gdb_server: support qXfer:threads:read packet

This patch adds support for the qXfer:threads:read packet. In addition
to providing a more efficient method of updating thread state, recent
versions of GDB (7.11.1 and up) can also report remote thread names.
While thread names are not enabled in this patch due to its limited
applicability at the moment, it can be enabled at a later date with
little effort.

As a part of revamping how threads are presented to GDB, extra info
strings for each of the supported RTOSes were updated to match
conventions present in the GDB source code. For more information, see
remote_threads_extra_info() in remote.c. This results in a much smoother
experience when interacting with GDB.

It is also worth mentioning that use of qXfer:threads:read works around
a number of regressions in older versions of GDB regarding remote thread
display. Trust me, it's great.

Change-Id: I97dd6a93c342ceb9b9d0023b6359db0e5604c6e6
Signed-off-by: Steven Stallion <stallion@squareup.com>
Reviewed-on: http://openocd.zylin.com/3559
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-by: Paul Fertser <fercerpav@gmail.com>

diff --git a/src/rtos/ChibiOS.c b/src/rtos/ChibiOS.c
index 00e9df7839740fdc41c45537924132d74fa8eddc..1bc1af8fcd5d15441da1eb2ac1d26d0fed6c9488 100644 (file)
--- a/src/rtos/ChibiOS.c
+++ b/src/rtos/ChibiOS.c
@@ -440,11 +440,11 @@ static int ChibiOS_update_threads(struct rtos *rtos)
                if (threadState < CHIBIOS_NUM_STATES)
                        state_desc = ChibiOS_thread_states[threadState];
                else
-                       state_desc = "Unknown state";
+                       state_desc = "Unknown";
 
                curr_thrd_details->extra_info_str = malloc(strlen(
-                                       state_desc)+1);
-               strcpy(curr_thrd_details->extra_info_str, state_desc);
+                                       state_desc)+8);
+               sprintf(curr_thrd_details->extra_info_str, "State: %s", state_desc);
 
                curr_thrd_details->exists = true;
 

diff --git a/src/rtos/FreeRTOS.c b/src/rtos/FreeRTOS.c
index 52cebadc7ca7423d789cabc5b0ce593d0332b7b7..83961eb9573cedb5683858999d678fba19de94e0 100644 (file)
--- a/src/rtos/FreeRTOS.c
+++ b/src/rtos/FreeRTOS.c
@@ -362,7 +362,7 @@ static int FreeRTOS_update_threads(struct rtos *rtos)
                        rtos->thread_details[tasks_found].exists = true;
 
                        if (rtos->thread_details[tasks_found].threadid == rtos->current_thread) {
-                               char running_str[] = "Running";
+                               char running_str[] = "State: Running";
                                rtos->thread_details[tasks_found].extra_info_str = malloc(
                                                sizeof(running_str));
                                strcpy(rtos->thread_details[tasks_found].extra_info_str,

diff --git a/src/rtos/ThreadX.c b/src/rtos/ThreadX.c
index 8267b9f6adfd5cf41ec848886de2e789a17f6ee5..ab8a66e93b262627a6e75e78fa4a67c4dc0a9013 100644 (file)
--- a/src/rtos/ThreadX.c
+++ b/src/rtos/ThreadX.c
@@ -408,8 +408,8 @@ static int ThreadX_update_threads(struct rtos *rtos)
                        state_desc = "Unknown state";
 
                rtos->thread_details[tasks_found].extra_info_str = malloc(strlen(
-                                       state_desc)+1);
-               strcpy(rtos->thread_details[tasks_found].extra_info_str, state_desc);
+                                       state_desc)+8);
+               sprintf(rtos->thread_details[tasks_found].extra_info_str, "State: %s", state_desc);
 
                rtos->thread_details[tasks_found].exists = true;
 

diff --git a/src/rtos/eCos.c b/src/rtos/eCos.c
index e38e11f04437a6646b2c59ce56cd4d26632da1e0..edc3d8b51014d0630ae8c5c71b335b9ad65e0f4e 100644 (file)
--- a/src/rtos/eCos.c
+++ b/src/rtos/eCos.c
@@ -261,8 +261,8 @@ static int eCos_update_threads(struct rtos *rtos)
                        state_desc = "Unknown state";
 
                rtos->thread_details[tasks_found].extra_info_str = malloc(strlen(
-                                       state_desc)+1);
-               strcpy(rtos->thread_details[tasks_found].extra_info_str, state_desc);
+                                       state_desc)+8);
+               sprintf(rtos->thread_details[tasks_found].extra_info_str, "State: %s", state_desc);
 
                rtos->thread_details[tasks_found].exists = true;
 

diff --git a/src/rtos/embKernel.c b/src/rtos/embKernel.c
index ceb313f02979c5e14e33b56d3587408a789cc382..e515383ac935193938edb43fbc0fad48db0ca243 100644 (file)
--- a/src/rtos/embKernel.c
+++ b/src/rtos/embKernel.c
@@ -168,11 +168,11 @@ static int embKernel_get_tasks_details(struct rtos *rtos, int64_t iterable, cons
                return retval;
        details->extra_info_str = malloc(EMBKERNEL_MAX_THREAD_NAME_STR_SIZE);
        if (task == rtos->current_thread) {
-               snprintf(details->extra_info_str, EMBKERNEL_MAX_THREAD_NAME_STR_SIZE, "Pri=%u, Running",
+               snprintf(details->extra_info_str, EMBKERNEL_MAX_THREAD_NAME_STR_SIZE, "State: Running, Priority: %u",
                                (unsigned int) priority);
        } else {
-               snprintf(details->extra_info_str, EMBKERNEL_MAX_THREAD_NAME_STR_SIZE, "Pri=%u, %s", (unsigned int) priority,
-                               state_str);
+               snprintf(details->extra_info_str, EMBKERNEL_MAX_THREAD_NAME_STR_SIZE, "State: %s, Priority: %u",
+                               state_str, (unsigned int) priority);
        }
 
        LOG_OUTPUT("Getting task details: iterable=0x%08X, task=0x%08X, name=%s\n", (unsigned int)iterable,

diff --git a/src/rtos/linux.c b/src/rtos/linux.c
index 8c150af2e7f093c1b133ececab0625785e3ab9b8..31d661844cb32cb27741ee5c94b3a12ca1f7ce9c 100644 (file)
--- a/src/rtos/linux.c
+++ b/src/rtos/linux.c
@@ -1213,7 +1213,7 @@ int linux_thread_extra_info(struct target *target,
                if (temp->threadid == threadid) {
                        char *pid = " PID: ";
                        char *pid_current = "*PID: ";
-                       char *name = "NAME: ";
+                       char *name = "Name: ";
                        int str_size = strlen(pid) + strlen(name);
                        char *tmp_str = calloc(1, str_size + 50);
                        char *tmp_str_ptr = tmp_str;
@@ -1225,9 +1225,7 @@ int linux_thread_extra_info(struct target *target,
                        else
                                tmp_str_ptr += sprintf(tmp_str_ptr, "%s", pid);
 
-                       tmp_str_ptr +=
-                               sprintf(tmp_str_ptr, "%d", (int)temp->pid);
-                       tmp_str_ptr += sprintf(tmp_str_ptr, "%s", " | ");
+                       tmp_str_ptr += sprintf(tmp_str_ptr, "%d, ", (int)temp->pid);
                        sprintf(tmp_str_ptr, "%s", name);
                        sprintf(tmp_str_ptr, "%s", temp->name);
                        char *hex_str = calloc(1, strlen(tmp_str) * 2 + 1);

diff --git a/src/rtos/mqx.c b/src/rtos/mqx.c
index b8095a0cf00ba90c1796177d3361bc98073f61d0..63a48c54b3d6355d81bec7c98d798d98d089a5a6 100644 (file)
--- a/src/rtos/mqx.c
+++ b/src/rtos/mqx.c
@@ -353,7 +353,7 @@ static int mqx_update_threads(
                uint32_t task_name_addr = 0, task_id = 0, task_errno = 0;
                uint32_t state_index = 0, state_max = 0;
                uint32_t extra_info_length = 0;
-               char *state_name = "unknown state";
+               char *state_name = "Unknown";
 
                /* set current taskpool address */
                if (ERROR_OK != mqx_get_member(
@@ -435,13 +435,13 @@ static int mqx_update_threads(
                 * calculate length as:
                 * state length + address length + errno length + formatter length
                 */
-               extra_info_length += strlen((void *)state_name) + 8 + 8 + 8;
+               extra_info_length += strlen((void *)state_name) + 7 + 13 + 8 + 15 + 8;
                rtos->thread_details[i].extra_info_str = malloc(extra_info_length + 1);
                if (NULL == rtos->thread_details[i].extra_info_str)
                        return ERROR_FAIL;
-               snprintf(
-                       rtos->thread_details[i].extra_info_str, extra_info_length, "%s : 0x%"PRIx32 " : %" PRIu32,
-                       state_name, task_addr, task_errno
+               snprintf(rtos->thread_details[i].extra_info_str, extra_info_length,
+                        "State: %s, Address: 0x%" PRIx32 ",  Error Code: %" PRIu32,
+                        state_name, task_addr, task_errno
                );
                /* set active thread */
                if (active_td_addr == task_addr)

diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index 785fc6161a1f798da791b0e6193b73ec683dc714..6938336f4848fe6876d38e1d706d55de0d18c196 100644 (file)
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -306,14 +306,14 @@ int rtos_thread_packet(struct connection *connection, char const *packet, int pa
                        if (detail->extra_info_str != NULL)
                                str_size += strlen(detail->extra_info_str);
 
-                       char *tmp_str = calloc(str_size + 4, sizeof(char));
+                       char *tmp_str = calloc(str_size + 9, sizeof(char));
                        char *tmp_str_ptr = tmp_str;
 
                        if (detail->thread_name_str != NULL)
-                               tmp_str_ptr += sprintf(tmp_str_ptr, "%s", detail->thread_name_str);
+                               tmp_str_ptr += sprintf(tmp_str_ptr, "Name: %s", detail->thread_name_str);
                        if (detail->extra_info_str != NULL) {
                                if (tmp_str_ptr != tmp_str)
-                                       tmp_str_ptr += sprintf(tmp_str_ptr, " : ");
+                                       tmp_str_ptr += sprintf(tmp_str_ptr, ", ");
                                tmp_str_ptr += sprintf(tmp_str_ptr, "%s", detail->extra_info_str);
                        }
 

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 7fd579b4651170052c78dd53e0a6119e11167ab3..6f111e14a22b306fff4876b07eafc3f691e86bd9 100644 (file)
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -90,6 +90,8 @@ struct gdb_connection {
        bool attached;
        /* temporarily used for target description support */
        struct target_desc_format target_desc;
+       /* temporarily used for thread list support */
+       char *thread_list;
 };
 
 #if 0
@@ -934,6 +936,7 @@ static int gdb_new_connection(struct connection *connection)
        gdb_connection->attached = true;
        gdb_connection->target_desc.tdesc = NULL;
        gdb_connection->target_desc.tdesc_length = 0;
+       gdb_connection->thread_list = NULL;
 
        /* send ACK to GDB for debug request */
        gdb_write(connection, "+", 1);
@@ -2283,6 +2286,95 @@ error:
        return retval;
 }
 
+static int gdb_generate_thread_list(struct target *target, char **thread_list_out)
+{
+       struct rtos *rtos = target->rtos;
+       int retval = ERROR_OK;
+       char *thread_list = NULL;
+       int pos = 0;
+       int size = 0;
+
+       xml_printf(&retval, &thread_list, &pos, &size,
+                  "<?xml version=\"1.0\"?>\n"
+                  "<threads>\n");
+
+       if (rtos != NULL) {
+               for (int i = 0; i < rtos->thread_count; i++) {
+                       struct thread_detail *thread_detail = &rtos->thread_details[i];
+
+                       if (!thread_detail->exists)
+                               continue;
+
+                       xml_printf(&retval, &thread_list, &pos, &size,
+                                  "<thread id=\"%" PRIx64 "\">", thread_detail->threadid);
+
+                       if (thread_detail->thread_name_str != NULL)
+                               xml_printf(&retval, &thread_list, &pos, &size,
+                                          "Name: %s", thread_detail->thread_name_str);
+
+                       if (thread_detail->extra_info_str != NULL) {
+                               if (thread_detail->thread_name_str != NULL)
+                                       xml_printf(&retval, &thread_list, &pos, &size,
+                                                  ", ");
+                               xml_printf(&retval, &thread_list, &pos, &size,
+                                          thread_detail->extra_info_str);
+                       }
+
+                       xml_printf(&retval, &thread_list, &pos, &size,
+                                  "</thread>\n");
+               }
+       }
+
+       xml_printf(&retval, &thread_list, &pos, &size,
+                  "</threads>\n");
+
+       if (retval == ERROR_OK)
+               *thread_list_out = thread_list;
+       else
+               free(thread_list);
+
+       return retval;
+}
+
+static int gdb_get_thread_list_chunk(struct target *target, char **thread_list,
+               char **chunk, int32_t offset, uint32_t length)
+{
+       if (*thread_list == NULL) {
+               int retval = gdb_generate_thread_list(target, thread_list);
+               if (retval != ERROR_OK) {
+                       LOG_ERROR("Unable to Generate Thread List");
+                       return ERROR_FAIL;
+               }
+       }
+
+       size_t thread_list_length = strlen(*thread_list);
+       char transfer_type;
+
+       length = MIN(length, thread_list_length - offset);
+       if (length < (thread_list_length - offset))
+               transfer_type = 'm';
+       else
+               transfer_type = 'l';
+
+       *chunk = malloc(length + 2);
+       if (*chunk == NULL) {
+               LOG_ERROR("Unable to allocate memory");
+               return ERROR_FAIL;
+       }
+
+       (*chunk)[0] = transfer_type;
+       strncpy((*chunk) + 1, (*thread_list) + offset, length);
+       (*chunk)[1 + length] = '\0';
+
+       /* After gdb-server sends out last chunk, invalidate thread list. */
+       if (transfer_type == 'l') {
+               free(*thread_list);
+               *thread_list = NULL;
+       }
+
+       return ERROR_OK;
+}
+
 static int gdb_query_packet(struct connection *connection,
                char const *packet, int packet_size)
 {
@@ -2372,7 +2464,7 @@ static int gdb_query_packet(struct connection *connection,
                        &buffer,
                        &pos,
                        &size,
-                       "PacketSize=%x;qXfer:memory-map:read%c;qXfer:features:read%c;QStartNoAckMode+",
+                       "PacketSize=%x;qXfer:memory-map:read%c;qXfer:features:read%c;qXfer:threads:read+;QStartNoAckMode+",
                        (GDB_BUFFER_SIZE - 1),
                        ((gdb_use_memory_map == 1) && (flash_get_bank_count() > 0)) ? '+' : '-',
                        (gdb_target_desc_supported == 1) ? '+' : '-');
@@ -2418,6 +2510,37 @@ static int gdb_query_packet(struct connection *connection,
 
                gdb_put_packet(connection, xml, strlen(xml));
 
+               free(xml);
+               return ERROR_OK;
+       } else if (strncmp(packet, "qXfer:threads:read:", 19) == 0) {
+               char *xml = NULL;
+               int retval = ERROR_OK;
+
+               int offset;
+               unsigned int length;
+
+               /* skip command character */
+               packet += 19;
+
+               if (decode_xfer_read(packet, NULL, &offset, &length) < 0) {
+                       gdb_send_error(connection, 01);
+                       return ERROR_OK;
+               }
+
+               /* Target should prepare correct thread list for annex.
+                * The first character of returned xml is 'm' or 'l'. 'm' for
+                * there are *more* chunks to transfer. 'l' for it is the *last*
+                * chunk of target description.
+                */
+               retval = gdb_get_thread_list_chunk(target, &gdb_connection->thread_list,
+                                                  &xml, offset, length);
+               if (retval != ERROR_OK) {
+                       gdb_error(connection, retval);
+                       return retval;
+               }
+
+               gdb_put_packet(connection, xml, strlen(xml));
+
                free(xml);
                return ERROR_OK;
        } else if (strncmp(packet, "QStartNoAckMode", 15) == 0) {