Pavel Chromy: performance tweak of gdb_put_packet_inner() removing malloc and avoidin...
[openocd.git] / src / server / gdb_server.c
index 57ba2806276fa3780bbc43a3b5931e8e28ff3a30..1fac46972a9e4f6c9c3bae36313f396012272421 100644 (file)
@@ -43,6 +43,7 @@
 #define _DEBUG_GDB_IO_
 #endif
 
+extern int gdb_error(connection_t *connection, int retval);
 static unsigned short gdb_port;
 static const char *DIGITS = "0123456789abcdef";
 
@@ -84,14 +85,58 @@ int gdb_last_signal(target_t *target)
                case DBG_REASON_NOTHALTED:
                        return 0x0; /* no signal... shouldn't happen */
                default:
-                       ERROR("BUG: undefined debug reason");
-                       exit(-1);
+                       USER("undefined debug reason %d - target needs reset", target->debug_reason);
+                       return 0x0;
+       }
+}
+
+#ifndef _WIN32
+int check_pending(connection_t *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
+        */
+       struct timeval tv;
+       fd_set read_fds;
+       gdb_connection_t *gdb_con = connection->priv;
+       int t;
+       if (got_data==NULL)
+               got_data=&t;
+       *got_data=0;
+
+       if (gdb_con->buf_cnt>0)
+       {
+               *got_data = 1;
+               return ERROR_OK;
+       }
+       
+       FD_ZERO(&read_fds);
+       FD_SET(connection->fd, &read_fds);
+       
+       tv.tv_sec = timeout_s;
+       tv.tv_usec = 0;
+       if (select(connection->fd + 1, &read_fds, NULL, NULL, &tv) == 0)
+       {
+               /* This can typically be because a "monitor" command took too long
+                * before printing any progress messages
+                */
+               if (timeout_s>0)
+               {
+                       return ERROR_GDB_TIMEOUT;
+               } else
+               {
+                       return ERROR_OK;
+               }
        }
+       *got_data=FD_ISSET(connection->fd, &read_fds)!=0;
+       return ERROR_OK;
 }
+#endif
 
 int gdb_get_char(connection_t *connection, int* next_char)
 {
        gdb_connection_t *gdb_con = connection->priv;
+       int retval=ERROR_OK;
 
 #ifdef _DEBUG_GDB_IO_
        char *debug_buffer;
@@ -115,24 +160,9 @@ int gdb_get_char(connection_t *connection, int* next_char)
        for (;;)
        {
 #ifndef _WIN32
-               /* 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
-                */
-               struct timeval tv;
-               fd_set read_fds;
-
-               FD_ZERO(&read_fds);
-               FD_SET(connection->fd, &read_fds);
-
-               tv.tv_sec = 1;
-               tv.tv_usec = 0;
-               if (select(connection->fd + 1, &read_fds, NULL, NULL, &tv) == 0)
-               {
-                       /* This can typically be because a "monitor" command took too long
-                        * before printing any progress messages
-                        */
-                       return ERROR_GDB_TIMEOUT;
-               }
+               retval=check_pending(connection, 1, NULL);
+               if (retval!=ERROR_OK)
+                       return retval;
 #endif
                gdb_con->buf_cnt = read_socket(connection->fd, gdb_con->buffer, GDB_BUFFER_SIZE);
                if (gdb_con->buf_cnt > 0)
@@ -154,8 +184,10 @@ int gdb_get_char(connection_t *connection, int* next_char)
                                usleep(1000);
                                break;
                        case WSAECONNABORTED:
+                               gdb_con->closed = 1;
                                return ERROR_SERVER_REMOTE_CLOSED;
                        case WSAECONNRESET:
+                               gdb_con->closed = 1;
                                return ERROR_SERVER_REMOTE_CLOSED;
                        default:
                                ERROR("read: %d", errno);
@@ -168,11 +200,14 @@ int gdb_get_char(connection_t *connection, int* next_char)
                                usleep(1000);
                                break;
                        case ECONNABORTED:
+                               gdb_con->closed = 1;
                                return ERROR_SERVER_REMOTE_CLOSED;
                        case ECONNRESET:
+                               gdb_con->closed = 1;
                                return ERROR_SERVER_REMOTE_CLOSED;
                        default:
                                ERROR("read: %s", strerror(errno));
+                               gdb_con->closed = 1;
                                return ERROR_SERVER_REMOTE_CLOSED;
                }
 #endif
@@ -197,7 +232,7 @@ int gdb_get_char(connection_t *connection, int* next_char)
        DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char);
 #endif
 
-       return ERROR_OK;
+       return retval;
 }
 
 int gdb_putback_char(connection_t *connection, int last_char)
@@ -248,6 +283,27 @@ int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
        for (i = 0; i < len; i++)
                my_checksum += buffer[i];
 
+#ifdef _DEBUG_GDB_IO_
+       /* 
+        * At this point we should have nothing in the input queue from GDB,
+        * however sometimes '-' is sent even though we've already received
+        * an ACK (+) for everything we've sent off.
+        */
+#ifndef _WIN32
+       int gotdata;
+       for (;;)
+       {
+               if ((retval=check_pending(connection, 0, &gotdata))!=ERROR_OK)
+                       return retval;
+               if (!gotdata)
+                       break;
+               if ((retval = gdb_get_char(connection, &reply)) != ERROR_OK)
+                       return retval;
+               WARNING("Discard unexpected char %c", reply);
+       }
+#endif
+#endif
+
        while (1)
        {
 #ifdef _DEBUG_GDB_IO_
@@ -257,44 +313,30 @@ int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
                DEBUG("sending packet '$%s#%2.2x'", debug_buffer, my_checksum);
                free(debug_buffer);
 #endif
-#if 0
-               char checksum[3];
-               gdb_write(connection, "$", 1);
-               if (len > 0)
-                       gdb_write(connection, buffer, len);
-               gdb_write(connection, "#", 1);
 
-               snprintf(checksum, 3, "%2.2x", my_checksum);
-
-               gdb_write(connection, checksum, 2);
-#else
-               void *allocated = NULL;
-               char stackAlloc[1024];
-               char *t = stackAlloc;
-               int totalLen = 1 + len + 1 + 2;
-               if (totalLen > sizeof(stackAlloc))
+               char local_buffer[1024];
+               local_buffer[0] = '$';
+               if (len+4 <= sizeof(local_buffer))
                {
-                       allocated = malloc(totalLen);
-                       t = allocated;
-                       if (allocated == NULL)
-                       {
-                               ERROR("Ran out of memory trying to reply packet %d\n", totalLen);
-                               exit(-1);
-                       }
+                       /* performance gain on smaller packets by only a single call to gdb_write() */
+                       memcpy(local_buffer+1, buffer, len++);
+                       local_buffer[len++] = '#';
+                       local_buffer[len++] = DIGITS[(my_checksum >> 4) & 0xf];
+                       local_buffer[len++] = DIGITS[my_checksum & 0xf];
+                       gdb_write(connection, local_buffer, len);
                }
-               t[0] = '$';
-               memcpy(t + 1, buffer, len);
-               t[1 + len] = '#';
-               t[1 + len + 1] = DIGITS[(my_checksum >> 4) & 0xf];
-               t[1 + len + 2] = DIGITS[my_checksum & 0xf];
-
-               gdb_write(connection, t, totalLen);
-
-               if (allocated)
+               else
                {
-                       free(allocated);
+                       /* larger packets are transmitted directly from caller supplied buffer
+                          by several calls to gdb_write() to avoid dynamic allocation */
+                       local_buffer[1] = '#';
+                       local_buffer[2] = DIGITS[(my_checksum >> 4) & 0xf];
+                       local_buffer[3] = DIGITS[my_checksum & 0xf];
+                       gdb_write(connection, local_buffer, 1);
+                       gdb_write(connection, buffer, len);
+                       gdb_write(connection, local_buffer+1, 3);
                }
-#endif
+
                if ((retval = gdb_get_char(connection, &reply)) != ERROR_OK)
                        return retval;
 
@@ -322,12 +364,14 @@ int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
                        else
                        {
                                ERROR("unknown character 0x%2.2x in reply, dropping connection", reply);
+                               gdb_con->closed=1;
                                return ERROR_SERVER_REMOTE_CLOSED;
                        }
                }
                else
                {
                        ERROR("unknown character 0x%2.2x in reply, dropping connection", reply);
+                       gdb_con->closed=1;
                        return ERROR_SERVER_REMOTE_CLOSED;
                }
        }
@@ -529,7 +573,7 @@ int gdb_output_con(connection_t *connection, const char* line)
 int gdb_output(struct command_context_s *context, char* line)
 {
        /* this will be dumped to the log and also sent as an O packet if possible */
-       USER_N(line);
+       USER_N("%s", line);
        return ERROR_OK;
 }
 
@@ -540,7 +584,7 @@ int gdb_program_handler(struct target_s *target, enum target_event event, void *
 
        if (target->gdb_program_script)
        {
-               script = open_file_from_path(cmd_ctx, target->gdb_program_script, "r");
+               script = open_file_from_path(target->gdb_program_script, "r");
                if (!script)
                {
                        ERROR("couldn't open script file %s", target->gdb_program_script);
@@ -624,6 +668,9 @@ int gdb_new_connection(connection_t *connection)
        gdb_connection->vflash_image = NULL;
        gdb_connection->closed = 0;
        gdb_connection->busy = 0;
+       
+       /* send ACK to GDB for debug request */
+       gdb_write(connection, "+", 1);
 
        /* output goes through gdb connection */
        command_set_output_handler(connection->cmd_ctx, gdb_output, connection);
@@ -631,21 +678,27 @@ int gdb_new_connection(connection_t *connection)
        /* register callback to be informed about target events */
        target_register_event_callback(gdb_target_callback_event_handler, connection);
 
-       /* a gdb session just attached, put the target in halt mode */
+       /* a gdb session just attached, try to put the target in halt mode
+        * or alterantively try to issue a reset.
+        *
+        * GDB connection will fail if e.g. register read packets fail,
+        * otherwise resetting/halting the target could have been left to GDB init
+        * scripts
+        */
        if (((retval = gdb_service->target->type->halt(gdb_service->target)) != ERROR_OK) &&
                        (retval != ERROR_TARGET_ALREADY_HALTED))
        {
-               ERROR("error(%d) when trying to halt target, falling back to \"reset halt\"", retval);
-               command_run_line(connection->cmd_ctx, "reset halt");
+               ERROR("error(%d) when trying to halt target, falling back to \"reset\"", retval);
+               command_run_line(connection->cmd_ctx, "reset");
        }
-
-       /* This will time out after 1 second */
-       command_run_line(connection->cmd_ctx, "wait_halt 1");
-
+       
        /* remove the initial ACK from the incoming buffer */
        if ((retval = gdb_get_char(connection, &initial_ack)) != ERROR_OK)
                return retval;
 
+       /* FIX!!!??? would we actually ever receive a + here??? 
+        * Not observed.
+        */
        if (initial_ack != '+')
                gdb_putback_char(connection, initial_ack);
 
@@ -778,16 +831,7 @@ int gdb_get_registers_packet(connection_t *connection, target_t *target, char* p
 
        if ((retval = target->type->get_gdb_reg_list(target, &reg_list, &reg_list_size)) != ERROR_OK)
        {
-               switch (retval)
-               {
-                       case ERROR_TARGET_NOT_HALTED:
-                               ERROR("gdb requested registers but we're not halted, dropping connection");
-                               return ERROR_SERVER_REMOTE_CLOSED;
-                       default:
-                               /* this is a bug condition - get_gdb_reg_list() may not return any other error */
-                               ERROR("BUG: unexpected error returned by get_gdb_reg_list()");
-                               exit(-1);
-               }
+               return gdb_error(connection, retval);
        }
 
        for (i = 0; i < reg_list_size; i++)
@@ -845,16 +889,7 @@ int gdb_set_registers_packet(connection_t *connection, target_t *target, char *p
 
        if ((retval = target->type->get_gdb_reg_list(target, &reg_list, &reg_list_size)) != ERROR_OK)
        {
-               switch (retval)
-               {
-                       case ERROR_TARGET_NOT_HALTED:
-                               ERROR("gdb tried to registers but we're not halted, dropping connection");
-                               return ERROR_SERVER_REMOTE_CLOSED;
-                       default:
-                               /* this is a bug condition - get_gdb_reg_list() may not return any other error */
-                               ERROR("BUG: unexpected error returned by get_gdb_reg_list()");
-                               exit(-1);
-               }
+               return gdb_error(connection, retval);
        }
 
        packet_p = packet;
@@ -910,16 +945,7 @@ int gdb_get_register_packet(connection_t *connection, target_t *target, char *pa
 
        if ((retval = target->type->get_gdb_reg_list(target, &reg_list, &reg_list_size)) != ERROR_OK)
        {
-               switch (retval)
-               {
-                       case ERROR_TARGET_NOT_HALTED:
-                               ERROR("gdb requested registers but we're not halted, dropping connection");
-                               return ERROR_SERVER_REMOTE_CLOSED;
-                       default:
-                               /* this is a bug condition - get_gdb_reg_list() may not return any other error */
-                               ERROR("BUG: unexpected error returned by get_gdb_reg_list()");
-                               exit(-1);
-               }
+               return gdb_error(connection, retval);
        }
 
        if (reg_list_size <= reg_num)
@@ -955,22 +981,13 @@ int gdb_set_register_packet(connection_t *connection, target_t *target, char *pa
 
        if ((retval = target->type->get_gdb_reg_list(target, &reg_list, &reg_list_size)) != ERROR_OK)
        {
-               switch (retval)
-               {
-                       case ERROR_TARGET_NOT_HALTED:
-                               ERROR("gdb tried to set a register but we're not halted, dropping connection");
-                               return ERROR_SERVER_REMOTE_CLOSED;
-                       default:
-                               /* this is a bug condition - get_gdb_reg_list() may not return any other error */
-                               ERROR("BUG: unexpected error returned by get_gdb_reg_list()");
-                               exit(-1);
-               }
+               return gdb_error(connection, retval);
        }
 
        if (reg_list_size < reg_num)
        {
                ERROR("gdb requested a non-existing register");
-               return ERROR_SERVER_REMOTE_CLOSED;
+               return ERROR_SERVER_REMOTE_CLOSED;      
        }
 
        if (*separator != '=')
@@ -1005,13 +1022,10 @@ int gdb_set_register_packet(connection_t *connection, target_t *target, char *pa
        return ERROR_OK;
 }
 
-int gdb_memory_packet_error(connection_t *connection, int retval)
+int gdb_error(connection_t *connection, int retval)
 {
        switch (retval)
        {
-               case ERROR_TARGET_NOT_HALTED:
-                       ERROR("gdb tried to read memory but we're not halted, dropping connection");
-                       return ERROR_SERVER_REMOTE_CLOSED;
                case ERROR_TARGET_DATA_ABORT:
                        gdb_send_error(connection, EIO);
                        break;
@@ -1021,10 +1035,14 @@ int gdb_memory_packet_error(connection_t *connection, int retval)
                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. */
-                       ERROR("unexpected error %i. Dropping connection.", retval);
-                       return ERROR_SERVER_REMOTE_CLOSED;
+                       ERROR("unexpected error %i", retval);
+                       gdb_send_error(connection, EFAULT);
+                       break;
        }
 
        return ERROR_OK;
@@ -1101,7 +1119,7 @@ int gdb_read_memory_packet(connection_t *connection, target_t *target, char *pac
        }
        else
        {
-               retval = gdb_memory_packet_error(connection, retval);
+               retval = gdb_error(connection, retval);
        }
 
        free(buffer);
@@ -1158,7 +1176,7 @@ int gdb_write_memory_packet(connection_t *connection, target_t *target, char *pa
        }
        else
        {
-               if ((retval = gdb_memory_packet_error(connection, retval)) != ERROR_OK)
+               if ((retval = gdb_error(connection, retval)) != ERROR_OK)
                        return retval;
        }
 
@@ -1208,7 +1226,7 @@ int gdb_write_memory_binary_packet(connection_t *connection, target_t *target, c
        }
        else
        {
-               if ((retval = gdb_memory_packet_error(connection, retval)) != ERROR_OK)
+               if ((retval = gdb_error(connection, retval)) != ERROR_OK)
                        return retval;
        }
 
@@ -1244,25 +1262,6 @@ void gdb_step_continue_packet(connection_t *connection, target_t *target, char *
        }
 }
 
-int gdb_bp_wp_packet_error(connection_t *connection, int retval)
-{
-       switch (retval)
-       {
-               case ERROR_TARGET_NOT_HALTED:
-                       ERROR("gdb tried to set a breakpoint but we're not halted, dropping connection");
-                       return ERROR_SERVER_REMOTE_CLOSED;
-                       break;
-               case ERROR_TARGET_RESOURCE_NOT_AVAILABLE:
-                       gdb_send_error(connection, EBUSY);
-                       break;
-               default:
-                       ERROR("BUG: unexpected error %i", retval);
-                       exit(-1);
-       }
-
-       return ERROR_OK;
-}
-
 int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target, char *packet, int packet_size)
 {
        int type;
@@ -1312,7 +1311,7 @@ int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target,
                        {
                                if ((retval = breakpoint_add(target, address, size, bp_type)) != ERROR_OK)
                                {
-                                       if ((retval = gdb_bp_wp_packet_error(connection, retval)) != ERROR_OK)
+                                       if ((retval = gdb_error(connection, retval)) != ERROR_OK)
                                                return retval;
                                }
                                else
@@ -1334,7 +1333,7 @@ int gdb_breakpoint_watchpoint_packet(connection_t *connection, target_t *target,
                        {
                                if ((retval = watchpoint_add(target, address, size, type-2, 0, 0xffffffffu)) != ERROR_OK)
                                {
-                                       if ((retval = gdb_bp_wp_packet_error(connection, retval)) != ERROR_OK)
+                                       if ((retval = gdb_error(connection, retval)) != ERROR_OK)
                                                return retval;
                                }
                                else
@@ -1384,18 +1383,18 @@ void xml_printf(int *retval, char **xml, int *pos, int *size, const char *fmt, .
                        }
                }
 
-           va_list ap;
-           int ret;
-           va_start(ap, fmt);
-           ret = vsnprintf(*xml + *pos, *size - *pos, fmt, ap);
-           va_end(ap);
-           if ((ret > 0) && ((ret + 1) < *size - *pos))
-           {
-               *pos += ret;
-               return;
-           }
-           /* there was just enough or not enough space, allocate more. */
-           first = 0;
+               va_list ap;
+               int ret;
+               va_start(ap, fmt);
+               ret = vsnprintf(*xml + *pos, *size - *pos, fmt, ap);
+               va_end(ap);
+               if ((ret > 0) && ((ret + 1) < *size - *pos))
+               {
+                       *pos += ret;
+                       return;
+               }
+               /* there was just enough or not enough space, allocate more. */
+               first = 0;
        }
 }
 
@@ -1463,6 +1462,7 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i
                        log_add_callback(gdb_log_callback, connection);
                        target_call_timer_callbacks();
                        command_run_line(cmd_ctx, cmd);
+                       log_remove_callback(gdb_log_callback, connection);
                        free(cmd);
                }
                gdb_put_packet(connection, "OK", 2);
@@ -1501,7 +1501,7 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i
                        }
                        else
                        {
-                               if ((retval = gdb_memory_packet_error(connection, retval)) != ERROR_OK)
+                               if ((retval = gdb_error(connection, retval)) != ERROR_OK)
                                        return retval;
                        }
 
@@ -1559,8 +1559,8 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i
 
                xml_printf(&retval, &xml, &pos, &size, "<memory-map>\n");
 
-               int i = 0;
-               for (;;)
+               int i;
+               for (i=0; i<flash_get_bank_count(); i++)
                {
                        p = get_flash_bank_by_num(i);
                        if (p == NULL)
@@ -1574,7 +1574,6 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i
                                "<property name=\"blocksize\">0x%x</property>\n" \
                                "</memory>\n", \
                                p->base, p->size, blocksize);
-                       i++;
                }
 
                xml_printf(&retval, &xml, &pos, &size, "</memory-map>\n");
@@ -1939,8 +1938,14 @@ int gdb_input_inner(connection_t *connection)
 int gdb_input(connection_t *connection)
 {
        int retval = gdb_input_inner(connection);
+       gdb_connection_t *gdb_con = connection->priv;
        if (retval == ERROR_SERVER_REMOTE_CLOSED)
                return retval;
+
+       /* logging does not propagate the error, yet can set th gdb_con->closed flag */
+       if (gdb_con->closed)
+               return ERROR_SERVER_REMOTE_CLOSED;
+       
        /* we'll recover from any other errors(e.g. temporary timeouts, etc.) */
        return ERROR_OK;
 }

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)