[PATCH] store trace default-collect to target [1/6] target, trace and remote
Hui Zhu
teawater@gmail.com
Tue Apr 16 15:22:00 GMT 2013
Hi Yao and Abid,
Thanks for your review.
On Thu, Apr 11, 2013 at 6:48 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 04/11/2013 02:16 PM, Hui Zhu wrote:
>>
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -215,6 +215,8 @@ static int remote_get_trace_status (stru
>> static int remote_upload_tracepoints (struct uploaded_tp **utpp);
>>
>> static int remote_upload_trace_state_variables (struct uploaded_tsv
>> **utsvp);
>> +
>> +static void remote_upload_trace_default_collest (char **collectp);
>
> ^^^^^^^ typo "collect"
Oops, all fixed.
>
>>
>> static void remote_query_supported (void);
>>
>> @@ -3567,6 +3569,7 @@ remote_start_remote (int from_tty, struc
>> {
>> struct uploaded_tp *uploaded_tps = NULL;
>> struct uploaded_tsv *uploaded_tsvs = NULL;
>> + char *uploaded_default_collest = NULL;
>
> ^^^^^^^ typo.
>
>>
>> if (current_trace_status ()->running)
>> printf_filtered (_("Trace is already running on the target.\n"));
>> @@ -3581,6 +3584,10 @@ remote_start_remote (int from_tty, struc
>> remote_upload_tracepoints (&uploaded_tps);
>>
>> merge_uploaded_tracepoints (&uploaded_tps);
>> +
>> + remote_upload_trace_default_collest (&uploaded_default_collest);
>> +
>> + trace_set_default_collect (uploaded_default_collest);
>
>
> Likewise.
>
>> }
>>
>> /* The thread and inferior lists are now synchronized with the
>> @@ -10567,6 +10574,23 @@ remote_download_tracepoint (struct bp_lo
>> do_cleanups (old_chain);
>> }
>>
>> +static void
>> +remote_download_tracepoint_default_collect (char *collect)
>
>
> Add a comment to this function.
Fixed.
/* Download COLLECT that is default collect string to target. */
>
>>
>> +static void
>> +remote_upload_trace_default_collest (char **collectp)
>
>
> Comments to this function are needed as well.
Fixed.
/* Download COLLECT that is default collect string to target. */
static void
remote_download_tracepoint_default_collect (char *collect)
>
>> +/* Not overwrite default collect If COLLECT is NULL or its size is 0. */
>> +
>> +void
>> +trace_set_default_collect (char *collect)
>> +{
>> + if (collect != NULL && strlen (collect) != 0)
>> + {
>> + xfree (default_collect);
>> + default_collect = collect;
>> + }
>> +}
>
>
> It looks incorrect to me. When we change target to a tfile, in which the
> default-collect is "", the variable default_collect won't be updated.
>
> Supposing we have two tfile actions.tf and actions1.tf, the default-collect
> is "$regs" and "" respectively,
>
> (gdb) target tfile testsuite/gdb.trace/actions.tf
> (gdb) show default-collect
> The list of expressions to collect by default is "$regs".
> (gdb) target tfile testsuite/gdb.trace/actions1.tf
> (gdb) show default-collect
> The list of expressions to collect by default is "$regs".
>
> I don't have other comments.
Agree with you. Updated patch for it.
>
> --
> Yao (齐尧)
On Thu, Apr 11, 2013 at 10:08 PM, Abid, Hafiz <hafiz_abid@mentor.com> wrote:
> Hi Hui,
>>
>> + putpkt (buf);
>> + remote_get_noisy_reply (&target_buf, &target_buf_size);
>> + if (strcmp (target_buf, "OK"))
>> + warning (_("\
>
>
> Just a small comment on style. I think it is better to be explicit in such
> case and write != 0. Also I was advised by Pedro recently that new packets
> should be using packet_ok which will eliminate the need for strcmp here
> anyway.
>
Update there part use packet_ok.
>> + putpkt ("qTDC");
>> + getpkt (&rs->buf, &rs->buf_size, 0);
>> + if (strncmp (rs->buf, "DC", 2))
>> + {
>
> Why not remote_get_noisy_reply here.
Changed. And I also add "PACKET_qTDC" for this part. But I am not
sure set "remote_protocol_packets[PACKET_qTDC].support" inside the
function is a good choice or not.
>
>
> Regards,
> Abid
Post a new version for that. Please help me review it.
Best,
Hui
2013-04-16 Hui Zhu <hui@codesourcery.com>
* remote.c (remote_upload_trace_default_collect): New static.
(PACKET_QTDDCsrc, PACKET_qTDC): New.
(remote_start_remote): Call remote_upload_trace_default_collect
and trace_set_default_collect.
(remote_download_tracepoint_default_collect): New.
(remote_can_download_tracepoint): Set
to_download_tracepoint_default_collect.
(remote_upload_trace_default_collect): New.
(_initialize_remote): Add QTDDCsrc and qTDC.
* target.c (update_current_target): Add
to_download_tracepoint_default_collect.
* target.h (target_ops): to_download_tracepoint_default_collect.
(target_download_tracepoint_default_collect): New.
* tracepoint.c (start_tracing): Call
target_download_tracepoint_default_collect.
(trace_set_default_collect): New.
* tracepoint.h (trace_set_default_collect): New extern.
-------------- next part --------------
--- a/remote.c
+++ b/remote.c
@@ -215,6 +215,8 @@ static int remote_get_trace_status (stru
static int remote_upload_tracepoints (struct uploaded_tp **utpp);
static int remote_upload_trace_state_variables (struct uploaded_tsv **utsvp);
+
+static void remote_upload_trace_default_collect (char **collectp);
static void remote_query_supported (void);
@@ -1286,6 +1288,8 @@ enum {
PACKET_Qbtrace_off,
PACKET_Qbtrace_bts,
PACKET_qXfer_btrace,
+ PACKET_QTDDCsrc,
+ PACKET_qTDC,
PACKET_MAX
};
@@ -3567,6 +3571,7 @@ remote_start_remote (int from_tty, struc
{
struct uploaded_tp *uploaded_tps = NULL;
struct uploaded_tsv *uploaded_tsvs = NULL;
+ char *uploaded_default_collect = NULL;
if (current_trace_status ()->running)
printf_filtered (_("Trace is already running on the target.\n"));
@@ -3581,6 +3586,10 @@ remote_start_remote (int from_tty, struc
remote_upload_tracepoints (&uploaded_tps);
merge_uploaded_tracepoints (&uploaded_tps);
+
+ remote_upload_trace_default_collect (&uploaded_default_collect);
+
+ trace_set_default_collect (uploaded_default_collect);
}
/* The thread and inferior lists are now synchronized with the
@@ -10567,6 +10576,33 @@ remote_download_tracepoint (struct bp_lo
do_cleanups (old_chain);
}
+/* Download COLLECT that is default collect string to target. */
+
+static void
+remote_download_tracepoint_default_collect (char *collect)
+{
+ char buf[BUF_SIZE];
+ struct remote_state *rs = get_remote_state ();
+ enum packet_result ret;
+
+ if (remote_protocol_packets[PACKET_QTDDCsrc].support == PACKET_DISABLE)
+ return;
+
+ strcpy (buf, "QTDDCsrc:");
+ if (strlen (buf) + strlen (collect) * 2 >= BUF_SIZE)
+ error (_("Source string too long for buffer"));
+ bin2hex (collect, buf + strlen (buf), 0);
+
+ putpkt (buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+ ret = packet_ok (rs->buf, &remote_protocol_packets[PACKET_QTDDCsrc]);
+ if (ret == PACKET_UNKNOWN)
+ warning (_("\
+Target does not support tracepoint default collect source download."));
+ else if (ret == PACKET_ERROR)
+ warning (_("Remote failure reply: %s"), rs->buf);
+}
+
static int
remote_can_download_tracepoint (void)
{
@@ -11422,6 +11458,8 @@ Specify the serial device it is connecte
remote_ops.to_can_run_breakpoint_commands = remote_can_run_breakpoint_commands;
remote_ops.to_trace_init = remote_trace_init;
remote_ops.to_download_tracepoint = remote_download_tracepoint;
+ remote_ops.to_download_tracepoint_default_collect
+ = remote_download_tracepoint_default_collect;
remote_ops.to_can_download_tracepoint = remote_can_download_tracepoint;
remote_ops.to_download_trace_state_variable
= remote_download_trace_state_variable;
@@ -11644,6 +11682,36 @@ remote_upload_trace_state_variables (str
return 0;
}
+/* Request target upload default collect, alloc buf for it and put it
+ to COLLECTP. */
+
+static void
+remote_upload_trace_default_collect (char **collectp)
+{
+ struct remote_state *rs = get_remote_state ();
+ int collect_size;
+
+ if (remote_protocol_packets[PACKET_qTDC].support == PACKET_DISABLE)
+ return;
+
+ putpkt ("qTDC");
+ remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+ if (strncmp (rs->buf, "DC", 2))
+ {
+ remote_protocol_packets[PACKET_qTDC].support = PACKET_DISABLE;
+ *collectp = NULL;
+ warning (_("\
+Target does not support tracepoint default collect upload."));
+ return;
+ }
+
+ remote_protocol_packets[PACKET_qTDC].support = PACKET_ENABLE;
+ collect_size = (strlen (rs->buf) - 2) / 2;
+ *collectp = xmalloc (collect_size + 1);
+ hex2bin (rs->buf + 2, *collectp, collect_size);
+ (*collectp)[collect_size] = '\0';
+}
+
void
_initialize_remote (void)
{
@@ -12000,6 +12068,12 @@ Show the maximum size of the address (in
add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_btrace],
"qXfer:btrace", "read-btrace", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QTDDCsrc],
+ "QTDDCsrc", "set-default-collect", 1);
+
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_qTDC],
+ "qTDC", "get-default-collect", 1);
+
/* Keep the old ``set remote Z-packet ...'' working. Each individual
Z sub-packet has its own set and show commands, but users may
have sets to this variable in their .gdbinit files (or in their
--- a/target.c
+++ b/target.c
@@ -706,6 +706,7 @@ update_current_target (void)
INHERIT (to_supports_string_tracing, t);
INHERIT (to_trace_init, t);
INHERIT (to_download_tracepoint, t);
+ INHERIT (to_download_tracepoint_default_collect, t);
INHERIT (to_can_download_tracepoint, t);
INHERIT (to_download_trace_state_variable, t);
INHERIT (to_enable_tracepoint, t);
@@ -890,6 +891,9 @@ update_current_target (void)
de_fault (to_download_tracepoint,
(void (*) (struct bp_location *))
tcomplain);
+ de_fault (to_download_tracepoint_default_collect,
+ (void (*) (char *))
+ tcomplain);
de_fault (to_can_download_tracepoint,
(int (*) (void))
return_zero);
--- a/target.h
+++ b/target.h
@@ -748,6 +748,10 @@ struct target_ops
/* Send full details of a tracepoint location to the target. */
void (*to_download_tracepoint) (struct bp_location *location);
+ /* Send full details of the list of expressions to collect by default
+ to the target. */
+ void (*to_download_tracepoint_default_collect) (char *collect);
+
/* Is the target able to download tracepoint locations in current
state? */
int (*to_can_download_tracepoint) (void);
@@ -1725,6 +1729,9 @@ extern char *target_fileio_read_stralloc
#define target_download_tracepoint(t) \
(*current_target.to_download_tracepoint) (t)
+#define target_download_tracepoint_default_collect(collect) \
+ (*current_target.to_download_tracepoint_default_collect) (collect)
+
#define target_can_download_tracepoint() \
(*current_target.to_can_download_tracepoint) ()
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -1796,6 +1796,8 @@ start_tracing (char *notes)
t->number_on_target = 0;
+ target_download_tracepoint_default_collect (default_collect);
+
for (loc = b->loc; loc; loc = loc->next)
{
/* Since tracepoint locations are never duplicated, `inserted'
@@ -5660,6 +5662,18 @@ traceframe_available_memory (VEC(mem_ran
return 0;
}
+/* Not overwrite default collect If COLLECT is NULL or its size is 0. */
+
+void
+trace_set_default_collect (char *collect)
+{
+ if (collect != NULL)
+ {
+ xfree (default_collect);
+ default_collect = collect;
+ }
+}
+
/* Implementation of `sdata' variable. */
static const struct internalvar_funcs sdata_funcs =
--- a/tracepoint.h
+++ b/tracepoint.h
@@ -412,4 +412,6 @@ extern struct traceframe_info *parse_tra
extern int traceframe_available_memory (VEC(mem_range_s) **result,
CORE_ADDR memaddr, ULONGEST len);
+extern void trace_set_default_collect (char *collect);
+
#endif /* TRACEPOINT_H */
More information about the Gdb-patches
mailing list