From 6d95014674415e3b9ea9d46d5148d3410f96bbfd Mon Sep 17 00:00:00 2001
From: oharboe <oharboe@b42882b7-edfa-0310-969c-e2dbd0fdcd60>
Date: Wed, 5 Mar 2008 10:28:32 +0000
Subject: * fixed malloc corruption in target->debug_reason * 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/server/gdb_server.c | 231 +++++++++++++++++++++++++-----------------------
src/server/server.c | 1 +
2 files changed, 121 insertions(+), 111 deletions(-)
(limited to 'src/server')
diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index a8b60231..c7200d20 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -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, ®_list, ®_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, ®_list, ®_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, ®_list, ®_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, ®_list, ®_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;
}
diff --git a/src/server/server.c b/src/server/server.c
index 4e8ddc69..ae13b129 100644
--- a/src/server/server.c
+++ b/src/server/server.c
@@ -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 */
--
cgit v1.2.3