* fixed malloc corruption in target->debug_reason
authoroharboe <oharboe@b42882b7-edfa-0310-969c-e2dbd0fdcd60>
Wed, 5 Mar 2008 10:28:32 +0000 (10:28 +0000)
committeroharboe <oharboe@b42882b7-edfa-0310-969c-e2dbd0fdcd60>
Wed, 5 Mar 2008 10:28:32 +0000 (10:28 +0000)
* GDB remote server will now remain online even if the target
  is in a funny state, e.g. if it requires a reset, it is
  running while GDB is not in the continue or step packet,
  e.g. via monitor resume/halt commands in GDB script.
* Added some _DEBUG_GDB_IO_ debug tools
* Fixed a couple of GDB server lockups, e.g. when O packets
  detect a severed connection
* added ACK upon connection (send +).
* added keep-alive messages to reset so GDB protocol remains happy.
* fixed crash when timing out connection to GDB

git-svn-id: svn://svn.berlios.de/openocd/trunk@445 b42882b7-edfa-0310-969c-e2dbd0fdcd60

src/helper/command.c
src/server/gdb_server.c
src/server/server.c
src/target/target.c
src/target/target.h

index aa71f0ba18c1b2af47436423181638143ab0dd20..5df12901566b85b545802b9ef27094a08a15070a 100644 (file)
@@ -447,7 +447,6 @@ void command_print_help_line(command_context_t* context, struct command_s *comma
        char indent_text[indent + 2];
        char *help = "no help available";
        char name_buf[64];
-       int i;
        
        if (indent)
        {
@@ -478,7 +477,6 @@ void command_print_help_line(command_context_t* context, struct command_s *comma
 int command_print_help_match(command_context_t* context, command_t* c_first, char* name, char** args, int argc)
 {
        command_t * c;
-       int i;
 
        for (c = c_first; c; c = c->next)
        {
index a8b6023131e9fb967c8a8996c32d786704375b37..c7200d20cea377802dab5bd48e78bf13b17a9921 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,17 +313,7 @@ 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;
@@ -294,7 +340,7 @@ int gdb_put_packet_inner(connection_t *connection, char *buffer, int len)
                {
                        free(allocated);
                }
-#endif
+               
                if ((retval = gdb_get_char(connection, &reply)) != ERROR_OK)
                        return retval;
 
@@ -322,12 +368,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;
                }
        }
@@ -624,6 +672,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 +682,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 +835,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 +893,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 +949,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 +985,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 +1026,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 +1039,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 +1123,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 +1180,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 +1230,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 +1266,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 +1315,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 +1337,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
@@ -1502,7 +1505,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;
                        }
 
@@ -1939,8 +1942,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;
 }
index 4e8ddc692e6b861f204d43d7680487880e157607..ae13b129d036669554578c6a197b7c98dddbab98 100644 (file)
@@ -71,6 +71,7 @@ int add_connection(service_t *service, command_context_t *cmd_ctx)
                close_socket(c->fd);
                INFO("attempted '%s' connection rejected", service->name);
                free(c);
+               return retval;
        }
        
        /* add to the end of linked list */
index 76f7449bde62083f91383613ab4fbd00ec3977f0..06abb37c232011a64ca6bb470def4cca713d9292 100644 (file)
@@ -122,7 +122,7 @@ char *target_debug_reason_strings[] =
 {
        "debug request", "breakpoint", "watchpoint",
        "watchpoint and breakpoint", "single step",
-       "target not halted"
+       "target not halted", "undefined"
 };
 
 char *target_endianess_strings[] =
@@ -362,7 +362,11 @@ int target_process_reset(struct command_context_s *cmd_ctx)
                                                command_print(cmd_ctx, "Timed out waiting for reset");
                                                goto done;
                                        }
-                                       usleep(100*1000); /* Do not eat all cpu */
+                                       /* this will send alive messages on e.g. GDB remote protocol.
+                                        * GDB warns me that I'm sending a zero length formatting message,
+                                        * which is strange, but in fact what is intended here. */
+                                       usleep(500*1000); 
+                                       USER_N(""); 
                                        goto again;
                                }
                        }
@@ -1178,6 +1182,7 @@ int handle_target_command(struct command_context_s *cmd_ctx, char *cmd, char **a
                                (*last_target_p)->backup_working_area = 0;
                                
                                (*last_target_p)->state = TARGET_UNKNOWN;
+                               (*last_target_p)->debug_reason = DBG_REASON_UNDEFINED;
                                (*last_target_p)->reg_cache = NULL;
                                (*last_target_p)->breakpoints = NULL;
                                (*last_target_p)->watchpoints = NULL;
index e6d6f2947d62754deaf1971019b56817625dbc18..dae5f19e717ae7084787e764f14107239c6b7d3c 100644 (file)
@@ -78,7 +78,8 @@ enum target_debug_reason
        DBG_REASON_WATCHPOINT = 2,
        DBG_REASON_WPTANDBKPT = 3,
        DBG_REASON_SINGLESTEP = 4,
-       DBG_REASON_NOTHALTED = 5
+       DBG_REASON_NOTHALTED = 5,
+       DBG_REASON_UNDEFINED = 6
 };
 
 extern char *target_debug_reason_strings[];

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)