gdb_server: fix double free
[openocd.git] / src / server / gdb_server.c
index 3a39f8a252dba661d87ac9bea1fb662d8b12504b..f8a1aac83ac2a9af4bfd35c4040b0a551e5a1de8 100644 (file)
@@ -227,6 +227,7 @@ static int gdb_get_char_inner(struct connection *connection, int *next_char)
                if (gdb_con->buf_cnt > 0)
                        break;
                if (gdb_con->buf_cnt == 0) {
+                       LOG_DEBUG("GDB connection closed by the remote client");
                        gdb_con->closed = true;
                        return ERROR_SERVER_REMOTE_CLOSED;
                }
@@ -348,11 +349,15 @@ static int gdb_putback_char(struct connection *connection, int last_char)
 static int gdb_write(struct connection *connection, void *data, int len)
 {
        struct gdb_connection *gdb_con = connection->priv;
-       if (gdb_con->closed)
+       if (gdb_con->closed) {
+               LOG_DEBUG("GDB socket marked as closed, cannot write to it.");
                return ERROR_SERVER_REMOTE_CLOSED;
+       }
 
        if (connection_write(connection, data, len) == len)
                return ERROR_OK;
+
+       LOG_WARNING("Error writing to GDB socket. Dropping the connection.");
        gdb_con->closed = true;
        return ERROR_SERVER_REMOTE_CLOSED;
 }
@@ -1001,16 +1006,19 @@ static int gdb_new_connection(struct connection *connection)
        breakpoint_clear_target(target);
        watchpoint_clear_target(target);
 
-       /* remove the initial ACK from the incoming buffer */
+       /* Since version 3.95 (gdb-19990504), with the exclusion of 6.5~6.8, GDB
+        * sends an ACK at connection with the following comment in its source code:
+        * "Ack any packet which the remote side has already sent."
+        * LLDB does the same since the first gdb-remote implementation.
+        * Remove the initial ACK from the incoming buffer.
+        */
        retval = gdb_get_char(connection, &initial_ack);
        if (retval != ERROR_OK)
                return retval;
 
-       /* FIX!!!??? would we actually ever receive a + here???
-        * Not observed.
-        */
        if (initial_ack != '+')
                gdb_putback_char(connection, initial_ack);
+
        target_call_event_callbacks(target, TARGET_EVENT_GDB_ATTACH);
 
        if (target->rtos) {
@@ -1212,7 +1220,7 @@ static int gdb_get_registers_packet(struct connection *connection,
        LOG_DEBUG("-");
 #endif
 
-       if ((target->rtos) && (ERROR_OK == rtos_get_gdb_reg_list(connection)))
+       if ((target->rtos) && (rtos_get_gdb_reg_list(connection) == ERROR_OK))
                return ERROR_OK;
 
        retval = target_get_gdb_reg_list(target, &reg_list, &reg_list_size,
@@ -1342,7 +1350,7 @@ static int gdb_get_register_packet(struct connection *connection,
        LOG_DEBUG("-");
 #endif
 
-       if ((target->rtos) && (ERROR_OK == rtos_get_gdb_reg(connection, reg_num)))
+       if ((target->rtos) && (rtos_get_gdb_reg(connection, reg_num) == ERROR_OK))
                return ERROR_OK;
 
        retval = target_get_gdb_reg_list_noread(target, &reg_list, &reg_list_size,
@@ -1351,7 +1359,7 @@ static int gdb_get_register_packet(struct connection *connection,
                return gdb_error(connection, retval);
 
        if (reg_list_size <= reg_num) {
-               LOG_ERROR("gdb requested a non-existing register");
+               LOG_ERROR("gdb requested a non-existing register (reg_num=%d)", reg_num);
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
@@ -1399,7 +1407,7 @@ static int gdb_set_register_packet(struct connection *connection,
        gdb_target_to_reg(target, separator + 1, chars, bin_buf);
 
        if ((target->rtos) &&
-                       (ERROR_OK == rtos_set_reg(connection, reg_num, bin_buf))) {
+                       (rtos_set_reg(connection, reg_num, bin_buf) == ERROR_OK)) {
                free(bin_buf);
                gdb_put_packet(connection, "OK", 2);
                return ERROR_OK;
@@ -1413,7 +1421,7 @@ static int gdb_set_register_packet(struct connection *connection,
        }
 
        if (reg_list_size <= reg_num) {
-               LOG_ERROR("gdb requested a non-existing register");
+               LOG_ERROR("gdb requested a non-existing register (reg_num=%d)", reg_num);
                free(bin_buf);
                free(reg_list);
                return ERROR_SERVER_REMOTE_CLOSED;
@@ -2251,6 +2259,110 @@ static int get_reg_features_list(struct target *target, char const **feature_lis
        return ERROR_OK;
 }
 
+/* Create a register list that's the union of all the registers of the SMP
+ * group this target is in. If the target is not part of an SMP group, this
+ * returns the same as target_get_gdb_reg_list_noread().
+ */
+static int smp_reg_list_noread(struct target *target,
+               struct reg **combined_list[], int *combined_list_size,
+               enum target_register_class reg_class)
+{
+       if (!target->smp)
+               return target_get_gdb_reg_list_noread(target, combined_list,
+                               combined_list_size, REG_CLASS_ALL);
+
+       unsigned int combined_allocated = 256;
+       struct reg **local_list = malloc(combined_allocated * sizeof(struct reg *));
+       if (!local_list) {
+               LOG_ERROR("malloc(%zu) failed", combined_allocated * sizeof(struct reg *));
+               return ERROR_FAIL;
+       }
+       unsigned int local_list_size = 0;
+
+       struct target_list *head;
+       foreach_smp_target(head, target->smp_targets) {
+               struct reg **reg_list = NULL;
+               int reg_list_size;
+               int result = target_get_gdb_reg_list_noread(head->target, &reg_list,
+                               &reg_list_size, reg_class);
+               if (result != ERROR_OK) {
+                       free(local_list);
+                       return result;
+               }
+               for (int i = 0; i < reg_list_size; i++) {
+                       bool found = false;
+                       struct reg *a = reg_list[i];
+                       if (a->exist) {
+                               /* Nested loop makes this O(n^2), but this entire function with
+                                * 5 RISC-V targets takes just 2ms on my computer. Fast enough
+                                * for me. */
+                               for (unsigned int j = 0; j < local_list_size; j++) {
+                                       struct reg *b = local_list[j];
+                                       if (!strcmp(a->name, b->name)) {
+                                               found = true;
+                                               if (a->size != b->size) {
+                                                       LOG_ERROR("SMP register %s is %d bits on one "
+                                                                       "target, but %d bits on another target.",
+                                                                       a->name, a->size, b->size);
+                                                       free(reg_list);
+                                                       free(local_list);
+                                                       return ERROR_FAIL;
+                                               }
+                                               break;
+                                       }
+                               }
+                               if (!found) {
+                                       LOG_DEBUG("[%s] %s not found in combined list", target_name(target), a->name);
+                                       if (local_list_size >= combined_allocated) {
+                                               combined_allocated *= 2;
+                                               local_list = realloc(local_list, combined_allocated * sizeof(struct reg *));
+                                               if (!local_list) {
+                                                       LOG_ERROR("realloc(%zu) failed", combined_allocated * sizeof(struct reg *));
+                                                       return ERROR_FAIL;
+                                               }
+                                       }
+                                       local_list[local_list_size] = a;
+                                       local_list_size++;
+                               }
+                       }
+               }
+               free(reg_list);
+       }
+
+       /* Now warn the user about any registers that weren't found in every target. */
+       foreach_smp_target(head, target->smp_targets) {
+               struct reg **reg_list = NULL;
+               int reg_list_size;
+               int result = target_get_gdb_reg_list_noread(head->target, &reg_list,
+                               &reg_list_size, reg_class);
+               if (result != ERROR_OK) {
+                       free(local_list);
+                       return result;
+               }
+               for (unsigned int i = 0; i < local_list_size; i++) {
+                       bool found = false;
+                       struct reg *a = local_list[i];
+                       for (int j = 0; j < reg_list_size; j++) {
+                               struct reg *b = reg_list[j];
+                               if (b->exist && !strcmp(a->name, b->name)) {
+                                       found = true;
+                                       break;
+                               }
+                       }
+                       if (!found) {
+                               LOG_WARNING("Register %s does not exist in %s, which is part of an SMP group where "
+                                           "this register does exist.",
+                                           a->name, target_name(head->target));
+                       }
+               }
+               free(reg_list);
+       }
+
+       *combined_list = local_list;
+       *combined_list_size = local_list_size;
+       return ERROR_OK;
+}
+
 static int gdb_generate_target_description(struct target *target, char **tdesc_out)
 {
        int retval = ERROR_OK;
@@ -2264,8 +2376,8 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o
        int size = 0;
 
 
-       retval = target_get_gdb_reg_list_noread(target, &reg_list,
-                       &reg_list_size, REG_CLASS_ALL);
+       retval = smp_reg_list_noread(target, &reg_list, &reg_list_size,
+                       REG_CLASS_ALL);
 
        if (retval != ERROR_OK) {
                LOG_ERROR("get register list failed");
@@ -2509,8 +2621,14 @@ static int gdb_generate_thread_list(struct target *target, char **thread_list_ou
                        if (!thread_detail->exists)
                                continue;
 
-                       xml_printf(&retval, &thread_list, &pos, &size,
-                                  "<thread id=\"%" PRIx64 "\">", thread_detail->threadid);
+                       if (thread_detail->thread_name_str)
+                               xml_printf(&retval, &thread_list, &pos, &size,
+                                          "<thread id=\"%" PRIx64 "\" name=\"%s\">",
+                                          thread_detail->threadid,
+                                          thread_detail->thread_name_str);
+                       else
+                               xml_printf(&retval, &thread_list, &pos, &size,
+                                          "<thread id=\"%" PRIx64 "\">", thread_detail->threadid);
 
                        if (thread_detail->thread_name_str)
                                xml_printf(&retval, &thread_list, &pos, &size,
@@ -3006,8 +3124,10 @@ static bool gdb_handle_vrun_packet(struct connection *connection, const char *pa
        free(next_hex_encoded_field(&parse, ';'));
 
        char *cmdline = next_hex_encoded_field(&parse, ';');
-       char *arg;
-       while (cmdline && (arg = next_hex_encoded_field(&parse, ';')) != NULL) {
+       while (cmdline) {
+               char *arg = next_hex_encoded_field(&parse, ';');
+               if (!arg)
+                       break;
                char *new_cmdline = alloc_printf("%s %s", cmdline, arg);
                free(cmdline);
                free(arg);
@@ -3547,13 +3667,10 @@ static int gdb_target_start(struct target *target, const char *port)
        /* initialize all targets gdb service with the same pointer */
        {
                struct target_list *head;
-               struct target *curr;
-               head = target->head;
-               while (head != (struct target_list *)NULL) {
-                       curr = head->target;
+               foreach_smp_target(head, target->smp_targets) {
+                       struct target *curr = head->target;
                        if (curr != target)
                                curr->gdb_service = gdb_service;
-                       head = head->next;
                }
        }
        return ret;

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)