This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Always sync TSVs
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Tue, 13 Apr 2010 17:17:57 +0100
- Subject: Always sync TSVs
We should always upload and merge TSVs even if the remote
doesn't claim disconnected tracing support. The target
may have builtin TSV's, like gdbserver's $trace_timestamp,
or may have created TSV's in a previous gdb session that
disconnected.
I've applied this patch below to fix it.
When this patch was originally written, against one of our
internal trees, remote_get_trace_status was using remote_get_noisy_reply
instead of getpkt, and that function presently complains loudly
if the remote returns "unsupported" a.k.a., empty reply. Later,
and in the FSF tree, remote_get_trace_status was changed to use getpkt
instead, so that it could gracefully detect trace support without throwing.
That change wouldn't have been needed as temporary measure if this patch
had already been submitted. :-) So, this patch makes it so that
remote_get_noisy_reply doesn't throw an error if the remote
sends an empty reply, and moves that check to its callers that don't
check it yet explicitly, so we can reinstate the remote_get_noisy_reply
call in remote_get_trace_status and get rid of the FIXME. This
is still good cleanup, IMO, so I'm still pushing this part as well.
Tested by issuing several tracepoint related commands against
an old gdbserver that doesn't support tracepoints, to confirm
the error messages still make sense, and regtested gdb.trace/ with
a tracepoints-capable gdbserver, without regressions.
--
Pedro Alves
2010-04-13 Pedro Alves <pedro@codesourcery.com>
gdb/
* remote.c (remote_get_noisy_reply): Don't error out on empty
replies.
(remote_start_remote): Update and merge tracepoints and trace
state variables as long as the target supports tracepoints.
(remote_trace_init): Fix prototype.
(remote_download_trace_state_variable): Validate reply.
(remote_trace_set_readonly_regions): Fix prototype.
(remote_trace_start): Fix prototype. Check for empty reply.
(remote_get_trace_status): Small cleanup.
(remote_trace_stop): Fix prototype. Check for empty reply.
(remote_trace_find): Check for empty reply.
(remote_save_trace_data): Validate reply.
(remote_set_disconnected_tracing): Check for empty reply, and
validate reply.
(remote_set_circular_trace_buffer): Ditto.
---
gdb/remote.c | 60 ++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 23 deletions(-)
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c 2010-04-13 16:53:47.000000000 +0100
+++ src/gdb/remote.c 2010-04-13 17:01:18.000000000 +0100
@@ -430,12 +430,9 @@ remote_get_noisy_reply (char **buf_p,
QUIT; /* allow user to bail out with ^C */
getpkt (buf_p, sizeof_buf, 0);
buf = *buf_p;
- if (buf[0] == 0)
- error (_("Target does not support this command."));
- else if (buf[0] == 'E')
+ if (buf[0] == 'E')
trace_error (buf);
- else if (buf[0] == 'O' &&
- buf[1] != 'K')
+ else if (buf[0] == 'O' && buf[1] != 'K')
remote_console_output (buf + 1); /* 'O' message from stub */
else
return buf; /* here's the actual reply */
@@ -3168,12 +3165,11 @@ remote_start_remote (struct ui_out *uiou
/* Possibly the target has been engaged in a trace run started
previously; find out where things are at. */
- if (rs->disconnected_tracing)
+ if (remote_get_trace_status (current_trace_status ()) != -1)
{
struct uploaded_tp *uploaded_tps = NULL;
struct uploaded_tsv *uploaded_tsvs = NULL;
- remote_get_trace_status (current_trace_status ());
if (current_trace_status ()->running)
printf_filtered (_("Trace is already running on the target.\n"));
@@ -9290,11 +9286,11 @@ remote_supports_fast_tracepoints (void)
}
static void
-remote_trace_init ()
+remote_trace_init (void)
{
putpkt ("QTinit");
remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
+ if (strcmp (target_buf, "OK") != 0)
error (_("Target does not support this command."));
}
@@ -9529,10 +9525,14 @@ remote_download_trace_state_variable (st
*p++ = '\0';
putpkt (rs->buf);
remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (*target_buf == '\0')
+ error (_("Target does not support this command."));
+ if (strcmp (target_buf, "OK") != 0)
+ error (_("Error on target while downloading trace state variable."));
}
static void
-remote_trace_set_readonly_regions ()
+remote_trace_set_readonly_regions (void)
{
asection *s;
bfd_size_type size;
@@ -9568,11 +9568,13 @@ remote_trace_set_readonly_regions ()
}
static void
-remote_trace_start ()
+remote_trace_start (void)
{
putpkt ("QTStart");
remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
+ if (*target_buf == '\0')
+ error (_("Target does not support this command."));
+ if (strcmp (target_buf, "OK") != 0)
error (_("Bogus reply from target: %s"), target_buf);
}
@@ -9586,10 +9588,7 @@ remote_get_trace_status (struct trace_st
trace_regblock_size = get_remote_arch_state ()->sizeof_g_packet;
putpkt ("qTStatus");
- getpkt (&target_buf, &target_buf_size, 0);
- /* FIXME should handle more variety of replies */
-
- p = target_buf;
+ p = remote_get_noisy_reply (&target_buf, &target_buf_size);
/* If the remote target doesn't do tracing, flag it. */
if (*p == '\0')
@@ -9613,11 +9612,13 @@ remote_get_trace_status (struct trace_st
}
static void
-remote_trace_stop ()
+remote_trace_stop (void)
{
putpkt ("QTStop");
remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
+ if (*target_buf == '\0')
+ error (_("Target does not support this command."));
+ if (strcmp (target_buf, "OK") != 0)
error (_("Bogus reply from target: %s"), target_buf);
}
@@ -9656,6 +9657,8 @@ remote_trace_find (enum trace_find_type
putpkt (rs->buf);
reply = remote_get_noisy_reply (&(rs->buf), &sizeof_pkt);
+ if (*reply == '\0')
+ error (_("Target does not support this command."));
while (reply && *reply)
switch (*reply)
@@ -9724,7 +9727,11 @@ remote_save_trace_data (const char *file
p += 2 * bin2hex ((gdb_byte *) filename, p, 0);
*p++ = '\0';
putpkt (rs->buf);
- remote_get_noisy_reply (&target_buf, &target_buf_size);
+ reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (*reply != '\0')
+ error (_("Target does not support this command."));
+ if (strcmp (reply, "OK") != 0)
+ error (_("Bogus reply from target: %s"), reply);
return 0;
}
@@ -9778,11 +9785,15 @@ remote_set_disconnected_tracing (int val
if (rs->disconnected_tracing)
{
+ char *reply;
+
sprintf (rs->buf, "QTDisconnected:%x", val);
putpkt (rs->buf);
- remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
+ reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (*reply == '\0')
error (_("Target does not support this command."));
+ if (strcmp (reply, "OK") != 0)
+ error (_("Bogus reply from target: %s"), reply);
}
else if (val)
warning (_("Target does not support disconnected tracing."));
@@ -9801,12 +9812,15 @@ static void
remote_set_circular_trace_buffer (int val)
{
struct remote_state *rs = get_remote_state ();
+ char *reply;
sprintf (rs->buf, "QTBuffer:circular:%x", val);
putpkt (rs->buf);
- remote_get_noisy_reply (&target_buf, &target_buf_size);
- if (strcmp (target_buf, "OK"))
+ reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
+ if (*reply == '\0')
error (_("Target does not support this command."));
+ if (strcmp (reply, "OK") != 0)
+ error (_("Bogus reply from target: %s"), reply);
}
static void