[patch v9 23/23] btrace, remote: drop qbtrace packet

Markus Metzger
2013-03-04T17:09:14Z

Use the qXfer:btrace:read packet's annex to encode a trace read method:

all ... read all trace
new ... read all trace if the trace has changed

This obsoletes the need for the qbtrace packet.

CC: Pedro Alves <palves@redhat.com>
CC: Jan Kratochvil <jan.kratochvil@redhat.com>
CC: Eli Zaretskii <eliz@gnu.org>

2013-03-04 Markus Metzger <markus.t.metzger@intel.com>

* target.h (target_ops) <to_read_btrace>: Add type parameter.
(target_ops) <to_btrace_has_changed>: Remove.
(target_btrace_has_changed): Remove.
(target_read_btrace): Add type parameter.
* target.c (target_read_btrace): Add type parameter.
(target_btrace_has_changed): Remove.
* remote.c (PACKET_qbtrace): Remove.
(remote_protocol_features): Remove qbtrace.
(remote_supports_btrace): Remove check for qbtrace.
(remote_btrace_has_changed): Remove.
(remote_read_btrace): Consider read type.
(init_remote_ops): Remove to_disable_btrace.
(_initialize_remote): Remove qbtrace.
* i386-linux-nat.c (_initialize_i386_linux_nat): Remove
initialization of to_btrace_has_changed.
* common/linux-btrace.h (linux_read_btrace): Add type parameter.
(linux_btrace_has_changed): Removed.
* common/linux-btrace.c (linux_btrace_has_changed): Moved.
Changed to static.
(linux_read_btrace): Consider read type.
* common/btrace-common.h (btrace_read_type): New enum.
* btrace.c (btrace_fetch): Only update branch trace if it
changed.
* amd64-linux-nat.c (_initialize_amd64_linux_nat): Remove
initialization of to_btrace_has_changed.

gdbserver/

* target.h (target_ops): Remove btrace_has_changed. Add type
parameter to read_btrace. Update target_ macros.
* server.c: Include btrace-common.h.
(handle_qxfer_btrace): Consider read type in annex.
(handle_btrace_query): Removed.
(handle_query): Remove qbtrace from qSupported. Remove call to
handle_btrace_query.
* linux-low.c (linux_low_read_btrace): Consider read type.
(linux_target_ops): Remove linux_btrace_has_changed.

doc/

* gdb.texinfo (Remote Configuration): Remove qbtrace.
Describe annex of Qbtrace.

---
gdb/amd64-linux-nat.c | 1 -
gdb/btrace.c | 6 ++-
gdb/common/btrace-common.h | 11 +++++++
gdb/common/linux-btrace.c | 37 ++++++++++------------
gdb/common/linux-btrace.h | 6 +--
gdb/doc/gdb.texinfo | 32 ++++++--------------
gdb/gdbserver/linux-low.c | 6 ++--
gdb/gdbserver/server.c | 56 +++++++++--------------------------
gdb/gdbserver/target.h | 14 +++-----
gdb/i386-linux-nat.c | 1 -
gdb/remote.c | 70 ++++++++++---------------------------------
gdb/target.c | 19 ++----------
gdb/target.h | 12 ++-----
13 files changed, 90 insertions(+), 181 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index c82edda..6f13fca 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -1196,7 +1196,6 @@ _initialize_amd64_linux_nat (void)
t->to_supports_btrace = linux_supports_btrace;
t->to_enable_btrace = amd64_linux_enable_btrace;
t->to_disable_btrace = amd64_linux_disable_btrace;
- t->to_btrace_has_changed = linux_btrace_has_changed;
t->to_read_btrace = linux_read_btrace;

/* Register the target. */
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 406508d..2fe6bf5 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -413,6 +413,7 @@ void
btrace_fetch (struct thread_info *tp)
{
struct btrace_thread_info *btinfo;
+ VEC (btrace_block_s) *btrace;

DEBUG ("fetch thread %d (%s)", tp->num, target_pid_to_str (tp->ptid));

@@ -420,12 +421,13 @@ btrace_fetch (struct thread_info *tp)
if (btinfo->target == NULL)
return;

- if (!target_btrace_has_changed (btinfo->target))
+ btrace = target_read_btrace (btinfo->target, btrace_read_new);
+ if (VEC_empty (btrace_block_s, btrace))
return;

btrace_clear (tp);

- btinfo->btrace = target_read_btrace (btinfo->target);
+ btinfo->btrace = btrace;
btinfo->itrace = compute_itrace (btinfo->btrace);
btinfo->ftrace = compute_ftrace (btinfo->itrace);

diff --git a/gdb/common/btrace-common.h b/gdb/common/btrace-common.h
index 90372ba..eab6c74 100644
--- a/gdb/common/btrace-common.h
+++ b/gdb/common/btrace-common.h
@@ -59,4 +59,15 @@ DEF_VEC_O (btrace_block_s);
/* Target specific branch trace information. */
struct btrace_target_info;

+/* Enumeration of btrace read types. */
+
+enum btrace_read_type
+{
+ /* Send all available trace. */
+ btrace_read_all,
+
+ /* Send all available trace, if it changed. */
+ btrace_read_new
+};
+
#endif /* BTRACE_COMMON_H */
diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
index 238d7ba..08a34dd 100644
--- a/gdb/common/linux-btrace.c
+++ b/gdb/common/linux-btrace.c
@@ -448,16 +448,6 @@ linux_supports_btrace (void)

/* See linux-btrace.h. */

-int
-linux_btrace_has_changed (struct btrace_target_info *tinfo)
-{
- volatile struct perf_event_mmap_page *header = perf_event_header (tinfo);
-
- return header->data_head != tinfo->data_head;
-}
-
-/* See linux-btrace.h. */
-
struct btrace_target_info *
linux_enable_btrace (ptid_t ptid)
{
@@ -526,10 +516,21 @@ linux_disable_btrace (struct btrace_target_info *tinfo)
return 0;
}

+/* Check whether the branch trace has changed. */
+
+static int
+linux_btrace_has_changed (struct btrace_target_info *tinfo)
+{
+ volatile struct perf_event_mmap_page *header = perf_event_header (tinfo);
+
+ return header->data_head != tinfo->data_head;
+}
+
/* See linux-btrace.h. */

VEC (btrace_block_s) *
-linux_read_btrace (struct btrace_target_info *tinfo)
+linux_read_btrace (struct btrace_target_info *tinfo,
+ enum btrace_read_type type)
{
VEC (btrace_block_s) *btrace = NULL;
volatile struct perf_event_mmap_page *header;
@@ -537,6 +538,9 @@ linux_read_btrace (struct btrace_target_info *tinfo)
unsigned long data_head, retries = 5;
size_t buffer_size;

+ if (type == btrace_read_new && !linux_btrace_has_changed (tinfo))
+ return NULL;
+
header = perf_event_header (tinfo);
buffer_size = perf_event_buffer_size (tinfo);

@@ -589,14 +593,6 @@ linux_supports_btrace (void)

/* See linux-btrace.h. */

-int
-linux_btrace_has_changed (struct btrace_target_info *tinfo)
-{
- return 0;
-}
-
-/* See linux-btrace.h. */
-
struct btrace_target_info *
linux_enable_btrace (ptid_t ptid)
{
@@ -614,7 +610,8 @@ linux_disable_btrace (struct btrace_target_info *tinfo)
/* See linux-btrace.h. */

VEC (btrace_block_s) *
-linux_read_btrace (struct btrace_target_info *tinfo)
+linux_read_btrace (struct btrace_target_info *tinfo,
+ enum btrace_read_type type)
{
return NULL;
}
diff --git a/gdb/common/linux-btrace.h b/gdb/common/linux-btrace.h
index b382e49..df4a4ae 100644
--- a/gdb/common/linux-btrace.h
+++ b/gdb/common/linux-btrace.h
@@ -70,10 +70,8 @@ extern struct btrace_target_info *linux_enable_btrace (ptid_t ptid);
/* Disable branch tracing and deallocate @tinfo. */
extern int linux_disable_btrace (struct btrace_target_info *tinfo);

-/* Check whether there is new trace data available. */
-extern int linux_btrace_has_changed (struct btrace_target_info *);
-
/* Read branch trace data. */
-extern VEC (btrace_block_s) *linux_read_btrace (struct btrace_target_info *);
+extern VEC (btrace_block_s) *linux_read_btrace (struct btrace_target_info *,
+ enum btrace_read_type);

#endif /* LINUX_BTRACE_H */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a3ebcd2..e8c2b34 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37393,11 +37393,6 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No

-@item @samp{qbtrace}
-@tab No
-@tab @samp{-}
-@tab Yes
-
@item @samp{qXfer:auxv:read}
@tab No
@tab @samp{-}
@@ -37711,9 +37706,6 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
The remote stub supports running a breakpoint's command list itself,
rather than reporting the hit to @value{GDBN}.

-@item qbtrace
-The remote stub understands the @samp{qbtrace} packet.
-
@item Qbtrace:off
The remote stub understands the @samp{Qbtrace:off} packet.

@@ -37843,7 +37835,16 @@ by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).

Return a description of the current branch trace.
@xref{Branch Trace Format}. The annex part of the generic @samp{qXfer}
-packet is empty.
+packet may have one of the following values:
+
+@table @code
+@item all
+Returns all available branch trace.
+
+@item new
+Returns all available branch trace if the branch trace changed since
+the last read request.
+@end table

This packet is not probed by default; the remote stub must request it
by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
@@ -38084,19 +38085,6 @@ The remote server created a new process.
A badly formed request or an error was encountered.
@end table

-@item qbtrace
-Return whether new branch trace data is available for the current thread.
-
-Reply:
-@table @samp
-@item yes
-New branch trace data is available.
-@item no
-No new branch trace data is available.
-@item E.errtext
-A badly formed request or an error was encountered.
-@end table
-
@item Qbtrace:bts
Enable branch tracing for the current thread using bts tracing.

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index c4507c8..0131de6 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5838,13 +5838,14 @@ linux_low_enable_btrace (ptid_t ptid)
/* Read branch trace data as btrace xml document. */

static void
-linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer)
+linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
+ int type)
{
VEC (btrace_block_s) *btrace;
struct btrace_block *block;
int i;

- btrace = linux_read_btrace (tinfo);
+ btrace = linux_read_btrace (tinfo, type);

buffer_grow_str (buffer, " \n");
buffer_grow_str (buffer, "<btrace version=\"1.0\">\n");
@@ -5927,7 +5928,6 @@ static struct target_ops linux_target_ops = {
linux_supports_btrace,
linux_low_enable_btrace,
linux_disable_btrace,
- linux_btrace_has_changed,
linux_low_read_btrace,
#else
NULL,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 00cac8e..6c8bff4 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -28,6 +28,7 @@
#include <signal.h>
#endif
#include "gdb_wait.h"
+#include "btrace-common.h"

/* The thread set with an `Hc' packet. `Hc' is deprecated in favor of
`vCont'. Note the multi-process extensions made `vCont' a
@@ -1345,11 +1346,12 @@ handle_qxfer_btrace (const char *annex,
{
static struct buffer cache;
struct thread_info *thread;
+ int type;

if (the_target->read_btrace == NULL || writebuf != NULL)
return -2;

- if (!target_running () || annex[0] != '\0')
+ if (!target_running ())
return -1;

if (ptid_equal (general_thread, null_ptid)
@@ -1372,11 +1374,21 @@ handle_qxfer_btrace (const char *annex,
return -1;
}

+ if (strcmp (annex, "all") == 0)
+ type = btrace_read_all;
+ else if (strcmp (annex, "new") == 0)
+ type = btrace_read_new;
+ else
+ {
+ strcpy (own_buf, "E.Bad annex.");
+ return -1;
+ }
+
if (offset == 0)
{
buffer_free (&cache);

- target_read_btrace (thread->btrace, &cache);
+ target_read_btrace (thread->btrace, &cache, type);
}
else if (offset > cache.used_size)
{
@@ -1560,42 +1572,6 @@ crc32 (CORE_ADDR base, int len, unsigned int crc)
return (unsigned long long) crc;
}

-/* Handle the "qbtrace" packet. */
-
-static int
-handle_btrace_query (char *own_buf)
-{
- if (strncmp ("qbtrace", own_buf, strlen ("qbtrace")) == 0)
- {
- struct thread_info *thread;
-
- if (ptid_equal (general_thread, null_ptid)
- || ptid_equal (general_thread, minus_one_ptid))
- {
- strcpy (own_buf, "E.Must select a single thread.");
- return -1;
- }
-
- thread = find_thread_ptid (general_thread);
- if (thread == NULL)
- {
- strcpy (own_buf, "E.No such thread.");
- return -1;
- }
-
- if (thread->btrace == NULL)
- {
- strcpy (own_buf, "E.Btrace not enabled.");
- return -1;
- }
-
- strcpy (own_buf,
- (target_btrace_has_changed (thread->btrace) ? "yes" : "no"));
- return 1;
- }
- return 0;
-}
-
/* Handle all of the extended 'q' packets. */

void
@@ -1827,7 +1803,6 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)

if (target_supports_btrace ())
{
- strcat (own_buf, ";qbtrace+");
strcat (own_buf, ";Qbtrace:bts+");
strcat (own_buf, ";Qbtrace:off+");
strcat (own_buf, ";qXfer:btrace:read+");
@@ -2023,9 +1998,6 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_tracepoints () && handle_tracepoint_query (own_buf))
return;

- if (handle_btrace_query (own_buf))
- return;
-
/* Otherwise we didn't know what packet it was. Say we didn't
understand it. */
own_buf[0] = 0;
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index a597911..f257459 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -410,11 +410,10 @@ struct target_ops
/* Disable branch tracing. */
int (*disable_btrace) (struct btrace_target_info *tinfo);

- /* Check whether branch trace changed on the target. */
- int (*btrace_has_changed) (struct btrace_target_info *);
+ /* Read branch trace data into buffer. We use an int to specify the type
+ to break a cyclic dependency. */
+ void (*read_btrace) (struct btrace_target_info *, struct buffer *, int type);

- /* Read branch trace data into buffer. */
- void (*read_btrace) (struct btrace_target_info *, struct buffer *);
};

extern struct target_ops *the_target;
@@ -547,11 +546,8 @@ int kill_inferior (int);
#define target_disable_btrace(tinfo) \
(*the_target->disable_btrace) (tinfo)

-#define target_btrace_has_changed(tinfo) \
- (*the_target->btrace_has_changed) (tinfo)
-
-#define target_read_btrace(tinfo, buffer) \
- (*the_target->read_btrace) (tinfo, buffer)
+#define target_read_btrace(tinfo, buffer, type) \
+ (*the_target->read_btrace) (tinfo, buffer, type)

/* Start non-stop mode, returns 0 on success, -1 on failure. */

diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index d439133..715c6d4 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -1118,7 +1118,6 @@ _initialize_i386_linux_nat (void)
t->to_supports_btrace = linux_supports_btrace;
t->to_enable_btrace = i386_linux_enable_btrace;
t->to_disable_btrace = i386_linux_disable_btrace;
- t->to_btrace_has_changed = linux_btrace_has_changed;
t->to_read_btrace = linux_read_btrace;

/* Register the target. */
diff --git a/gdb/remote.c b/gdb/remote.c
index 2365713..f76846a 100755
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1285,7 +1285,6 @@ enum {
PACKET_qXfer_fdpic,
PACKET_QDisableRandomization,
PACKET_QAgent,
- PACKET_qbtrace,
PACKET_Qbtrace_off,
PACKET_Qbtrace_bts,
PACKET_qXfer_btrace,
@@ -4000,7 +3999,6 @@ static struct protocol_feature remote_protocol_features[] = {
{ "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
{ "tracenz", PACKET_DISABLE,
remote_string_tracing_feature, -1 },
- { "qbtrace", PACKET_DISABLE, remote_supported_packet, PACKET_qbtrace },
{ "Qbtrace:off", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_off },
{ "Qbtrace:bts", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_bts },
{ "qXfer:btrace:read", PACKET_DISABLE, remote_supported_packet,
@@ -11140,8 +11138,6 @@ struct btrace_target_info
static int
remote_supports_btrace (void)
{
- if (remote_protocol_packets[PACKET_qbtrace].support != PACKET_ENABLE)
- return 0;
if (remote_protocol_packets[PACKET_Qbtrace_off].support != PACKET_ENABLE)
return 0;
if (remote_protocol_packets[PACKET_Qbtrace_bts].support != PACKET_ENABLE)
@@ -11208,57 +11204,16 @@ remote_disable_btrace (struct btrace_target_info *tinfo)
xfree (tinfo);
}

-/* Check whether branch trace data has changed. */
-
-static int
-remote_btrace_has_changed (struct btrace_target_info *tinfo)
-{
- struct packet_config *packet = &remote_protocol_packets[PACKET_qbtrace];
- struct remote_state *rs = get_remote_state ();
- char *buf = rs->buf;
- char *endbuf = rs->buf + get_remote_packet_size ();
-
- if (packet->support != PACKET_ENABLE)
- error (_("Target does not support branch tracing."));
-
- set_general_thread (tinfo->ptid);
-
- buf += xsnprintf (buf, endbuf - buf, "%s", packet->name);
- putpkt (rs->buf);
- getpkt (&rs->buf, &rs->buf_size, 0);
-
- switch (packet_ok (rs->buf, packet))
- {
- case PACKET_OK:
- break;
-
- case PACKET_UNKNOWN:
- return 0;
-
- case PACKET_ERROR:
- error (_("Failed to check for branch trace data for %s: %s."),
- target_pid_to_str (tinfo->ptid), rs->buf);
- }
-
- if (strcmp (rs->buf, "yes") == 0)
- return 1;
-
- if (strcmp (rs->buf, "no") == 0)
- return 0;
-
- error (_("Bad remote reply: %s."), rs->buf);
-
- return 0;
-}
-
/* Read the branch trace. */

static VEC (btrace_block_s) *
-remote_read_btrace (struct btrace_target_info *tinfo)
+remote_read_btrace (struct btrace_target_info *tinfo,
+ enum btrace_read_type type)
{
struct packet_config *packet = &remote_protocol_packets[PACKET_qXfer_btrace];
struct remote_state *rs = get_remote_state ();
VEC (btrace_block_s) *btrace = NULL;
+ const char *annex;
char *xml;

if (packet->support != PACKET_ENABLE)
@@ -11268,8 +11223,21 @@ remote_read_btrace (struct btrace_target_info *tinfo)
error (_("Cannot process branch tracing result. XML parsing not supported."));
#endif

+ switch (type)
+ {
+ case btrace_read_all:
+ annex = "all";
+ break;
+ case btrace_read_new:
+ annex = "new";
+ break;
+ default:
+ annex = NULL;
+ break;
+ }
+
xml = target_read_stralloc (&current_target,
- TARGET_OBJECT_BTRACE, NULL);
+ TARGET_OBJECT_BTRACE, annex);
if (xml != NULL)
{
struct cleanup *cleanup = make_cleanup (xfree, xml);
@@ -11400,7 +11368,6 @@ Specify the serial device it is connected to\n\
remote_ops.to_supports_btrace = remote_supports_btrace;
remote_ops.to_enable_btrace = remote_enable_btrace;
remote_ops.to_disable_btrace = remote_disable_btrace;
- remote_ops.to_btrace_has_changed = remote_btrace_has_changed;
remote_ops.to_read_btrace = remote_read_btrace;
}

@@ -11926,9 +11893,6 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
"QAgent", "agent", 0);

- add_packet_config_cmd (&remote_protocol_packets[PACKET_qbtrace],
- "qbtrace", "query-btrace", 0);
-
add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace_off],
"Qbtrace:off", "disable-btrace", 0);

diff --git a/gdb/target.c b/gdb/target.c
index a788a67..778b6b9 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4203,28 +4203,15 @@ target_disable_btrace (struct btrace_target_info *btinfo)
}

/* See target.h. */
-int
-target_btrace_has_changed (struct btrace_target_info *btinfo)
-{
- struct target_ops *t;
-
- for (t = current_target.beneath; t != NULL; t = t->beneath)
- if (t->to_btrace_has_changed != NULL)
- return t->to_btrace_has_changed (btinfo);
-
- tcomplain ();
- return 0;
-}
-
-/* See target.h. */
VEC (btrace_block_s) *
-target_read_btrace (struct btrace_target_info *btinfo)
+target_read_btrace (struct btrace_target_info *btinfo,
+ enum btrace_read_type type)
{
struct target_ops *t;

for (t = current_target.beneath; t != NULL; t = t->beneath)
if (t->to_read_btrace != NULL)
- return t->to_read_btrace (btinfo);
+ return t->to_read_btrace (btinfo, type);

tcomplain ();
return NULL;
diff --git a/gdb/target.h b/gdb/target.h
index a3fd892..88c13cc 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -870,11 +870,9 @@ struct target_ops
/* Disable branch tracing and deallocate @tinfo. */
void (*to_disable_btrace) (struct btrace_target_info *tinfo);

- /* Check whether branch trace changed on the target. */
- int (*to_btrace_has_changed) (struct btrace_target_info *);
-
/* Read branch trace data. */
- VEC (btrace_block_s) *(*to_read_btrace) (struct btrace_target_info *);
+ VEC (btrace_block_s) *(*to_read_btrace) (struct btrace_target_info *,
+ enum btrace_read_type);

/* Print information about the recording. */
void (*to_info_record) (void);
@@ -1973,12 +1971,10 @@ extern struct btrace_target_info *target_enable_btrace (ptid_t ptid);
/* Disable branch tracing. Deallocates @btinfo. */
extern void target_disable_btrace (struct btrace_target_info *btinfo);

-/* Check whether there is no branch tracing data available. */
-extern int target_btrace_has_changed (struct btrace_target_info *btinfo);
-
/* Read branch tracing data.
Returns a vector of branch trace blocks with the latest entry at index 0. */
-extern VEC (btrace_block_s) *target_read_btrace (struct btrace_target_info *);
+extern VEC (btrace_block_s) *target_read_btrace (struct btrace_target_info *,
+ enum btrace_read_type);

/* See to_info_record in struct target_ops. */
extern void target_info_record (void);
--
1.7.1