From: Dmitry Kozlov <dmitry_kozlov@mentor.com>
To: Pedro Alves <palves@redhat.com>
Cc: Yao Qi <yao@codesourcery.com>, <gdb-patches@sourceware.org>,
"'Stan_Shebs@mentor.com'" <Stan_Shebs@mentor.com>,
Vladimir Prus <vladimir@codesourcery.com>
Subject: Re: [PATCH] Extend tsave: save start time, stop time, user and notes
Date: Fri, 12 Oct 2012 08:46:00 -0000 [thread overview]
Message-ID: <5077D8DB.2070908@mentor.com> (raw)
In-Reply-To: <5076FD5F.4060009@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3847 bytes --]
This is duplicate because I made wrong attachment in previous email. All
patches are v4.
Hi Pedro,
would you be so kind to review/approve whole 3 tracepoints patches
together. They are interdependent, so it is much easier to look at them
together. I did my best to satisfy all your and Yao's comments.
Stan, I will add tests after these 3 patches will be approved because I
don't want to rewrite tests all time patches change.
Thank you,
Dmitry
On 10/11/2012 09:09 PM, Pedro Alves wrote:
> On 10/11/2012 04:53 PM, Dmitry Kozlov wrote:
>
>>> Logically, this patch depends on your previous patch
>>>
>>> PATCH fix start-time and stop-time in trace-status
>>> http://sourceware.org/ml/gdb-patches/2012-09/msg00621.html
>>>
>>> otherwise, we generate start-time and stop-time in trace file in decimal format, while gdb still interprets it as hex.
> Since gdbserver and gdb have always been in disagreement (IOW, this
> never worked correctly with gdbserver, so we're free to change it), and
> the RSP uses hex everywhere (*), let's fix gdbserver instead.
>
> @cindex remote protocol, field separator
> Fields within the packet should be separated using @samp{,} @samp{;} or
> @samp{:}. Except where otherwise noted all numbers are represented in
> @sc{hex} with leading zeros suppressed.
>
>> +2012-10-04 Dmitry Kozlov <ddk@codesourcery.com>
>> +
>> + * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes.
>> +
>> +2012-10-04 Dmitry Kozlov <ddk@mentor.com>
>> +
>> + * tracepoint.c (trace_status_command): Fix type of printf arg.
>> + (trace_status_mi): Likewise.
> Looks like something stale (two entries).
>
>> +
>> 2012-10-03 Doug Evans <dje@google.com>
>>
>> PR symtab/14601
>> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
>> index 0f94150..959ede5 100644
>> --- a/gdb/tracepoint.c
>> +++ b/gdb/tracepoint.c
>> @@ -3018,10 +3018,11 @@ trace_save (const char *filename, int target_does_save)
>> (ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
>> if (ts->stop_reason == tracepoint_error)
>> {
>> - char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);
>> + char *buf = (char *) xmalloc (strlen (ts->stop_desc) * 2 + 1);
>>
>> bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
>> fprintf (fp, ":%s", buf);
>> + xfree (buf);
>> }
>> fprintf (fp, ":%x", ts->stopping_tracepoint);
>> if (ts->traceframe_count >= 0)
>> @@ -3036,6 +3037,35 @@ trace_save (const char *filename, int target_does_save)
>> fprintf (fp, ";disconn:%x", ts->disconnected_tracing);
>> if (ts->circular_buffer)
>> fprintf (fp, ";circular:%x", ts->circular_buffer);
>> + if (ts->start_time)
>> + {
>> + fprintf (fp, ";starttime:%ld%06ld",
>> + (long int) (ts->start_time / 1000000),
>> + (long int) (ts->start_time % 1000000));
> See above. Numbers in the remote protocol are represented in hex.
>
>> + }
>> + if (ts->stop_time)
>> + {
>> + fprintf (fp, ";stoptime:%ld%06ld",
>> + (long int) (ts->stop_time / 1000000),
>> + (long int) (ts->stop_time % 1000000));
>> + }
>> + if (ts->notes && ts->notes[0] != 0 )
> Spurious space after 0.
>
>> + {
>> + char *buf = (char *) xmalloc (strlen (ts->notes) * 2 + 1);
> Unnecessary cast.
>
>> +
>> + bin2hex ((gdb_byte *) ts->notes, buf, 0);
>> + fprintf (fp, ";notes:%s", buf);
>> + xfree (buf);
>
> Indentation is off (compared to ts->stop_time branch above).
> Check tabs vs spaces.
>
>> + }
>> + if (ts->user_name)
> As Yao pointed out, shouldn't this get the same check for empty string?
>
>> + {
>> + char *buf = (char *) xmalloc (strlen (ts->user_name) * 2 + 1);
>> +
>> + bin2hex ((gdb_byte *) ts->user_name, buf, 0);
>> + fprintf (fp, ";username:%s", buf);
>> + xfree (buf);
>> + }
[-- Attachment #2: gdb_dates_v4.diff --]
[-- Type: text/x-patch, Size: 3646 bytes --]
commit b256b0abcf8e45a615c4ce763d694d371e988f83
Author: Dmitry Kozlov <dmitry_kozlov@mentor.com>
Date: Fri Oct 12 11:20:50 2012 +0400
Fix trace-status to output proper start-time and stop-time.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e6867c6..27ad6ab 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+
+ * tracepoint.c (trace_status_command): Fix type of printf arg to
+ prevent improper type conversion.
+ (trace_status_mi): Likewise.
+
2012-10-11 Andrew Burgess <aburgess@broadcom.com>
* remote-sim.c (gdbsim_create_inferior): Call init_thread_list to
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 8bfbe9c..bb4ace8 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+
+ * gdbserver/tracepoint.c (cmd_qtstatus): Modify trace-status output to
+ output start time and stop time in hex as gdb expects.
+
2012-10-01 Andrew Burgess <aburgess@broadcom.com>
* server.c (handle_search_memory_1): Include access length in
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 201a25b..5b56f81 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3664,7 +3664,8 @@ cmd_qtstatus (char *packet)
free_space (), phex_nz (trace_buffer_hi - trace_buffer_lo, 0),
circular_trace_buffer,
disconnected_tracing,
- plongest (tracing_start_time), plongest (tracing_stop_time),
+ phex_nz (tracing_start_time, sizeof (tracing_start_time)),
+ phex_nz (tracing_stop_time, sizeof (tracing_stop_time)),
buf1, buf2);
}
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index cce8d00..1577ad3 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2041,20 +2041,20 @@ trace_status_command (char *args, int from_tty)
/* Reporting a run time is more readable than two long numbers. */
printf_filtered (_("Trace started at %ld.%06ld secs, stopped %ld.%06ld secs later.\n"),
- (long int) ts->start_time / 1000000,
- (long int) ts->start_time % 1000000,
- (long int) run_time / 1000000,
- (long int) run_time % 1000000);
+ (long int) (ts->start_time / 1000000),
+ (long int) (ts->start_time % 1000000),
+ (long int) (run_time / 1000000),
+ (long int) (run_time % 1000000));
}
else
printf_filtered (_("Trace started at %ld.%06ld secs.\n"),
- (long int) ts->start_time / 1000000,
- (long int) ts->start_time % 1000000);
+ (long int) (ts->start_time / 1000000),
+ (long int) (ts->start_time % 1000000));
}
else if (ts->stop_time)
printf_filtered (_("Trace stopped at %ld.%06ld secs.\n"),
- (long int) ts->stop_time / 1000000,
- (long int) ts->stop_time % 1000000);
+ (long int) (ts->stop_time / 1000000),
+ (long int) (ts->stop_time % 1000000));
/* Now report any per-tracepoint status available. */
tp_vec = all_tracepoints ();
@@ -2169,12 +2169,12 @@ trace_status_mi (int on_stop)
char buf[100];
xsnprintf (buf, sizeof buf, "%ld.%06ld",
- (long int) ts->start_time / 1000000,
- (long int) ts->start_time % 1000000);
+ (long int) (ts->start_time / 1000000),
+ (long int) (ts->start_time % 1000000));
ui_out_field_string (uiout, "start-time", buf);
xsnprintf (buf, sizeof buf, "%ld.%06ld",
- (long int) ts->stop_time / 1000000,
- (long int) ts->stop_time % 1000000);
+ (long int) (ts->stop_time / 1000000),
+ (long int) (ts->stop_time % 1000000));
ui_out_field_string (uiout, "stop-time", buf);
}
}
[-- Attachment #3: gdb_stop_notes_v4.diff --]
[-- Type: text/x-patch, Size: 3258 bytes --]
commit 570ec36c0dfd2d19cde2f779ee3f146fc526e44e
Author: Dmitry Kozlov <dmitry_kozlov@mentor.com>
Date: Fri Oct 12 11:37:31 2012 +0400
Modify trace-status output to always output stop-notes.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3e2a818..dd9edde 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+ * tracepoint.c (trace_status_mi): Modify trace status MI command to
+ always output stop-notes.
+ (trace_save): Save stop-notes if any by tsave.
+ (parse_trace_status): Add parsing stop-notes.
+
+2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+
* tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes.
2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index bb4ace8..58a8827 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,10 @@
2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+ * gdbserver/tracepoint.c (cmd_qtstatus): Modify trace-status
+ to always output stop-notes.
+
+2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+
* gdbserver/tracepoint.c (cmd_qtstatus): Modify trace-status output to
output start time and stop time in hex as gdb expects.
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 5b56f81..72fca51 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3657,7 +3657,7 @@ cmd_qtstatus (char *packet)
"circular:%d;"
"disconn:%d;"
"starttime:%s;stoptime:%s;"
- "username:%s:;notes:%s:",
+ "username:%s;notes:%s;stop-notes:%s",
tracing ? 1 : 0,
stop_reason_rsp, tracing_stop_tpnum,
traceframe_count, traceframes_created,
@@ -3666,7 +3666,7 @@ cmd_qtstatus (char *packet)
disconnected_tracing,
phex_nz (tracing_start_time, sizeof (tracing_start_time)),
phex_nz (tracing_stop_time, sizeof (tracing_stop_time)),
- buf1, buf2);
+ buf1, buf2, buf3);
}
static void
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 8f4f18c..0905e89 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2164,6 +2164,8 @@ trace_status_mi (int on_stop)
ui_out_field_string (uiout, "user-name", ts->user_name);
ui_out_field_string (uiout, "notes", ts->notes);
+ // Always output stop-notes to MI to unify IDE processing of start and stop notes
+ ui_out_field_string (uiout, "stop-notes", ts->stop_desc);
{
char buf[100];
@@ -3063,6 +3065,14 @@ trace_save (const char *filename, int target_does_save)
fprintf (fp, ";username:%s", buf);
xfree (buf);
}
+ if (ts->stop_desc && ts->stop_desc[0] != 0)
+ {
+ char *buf = xmalloc (strlen (ts->stop_desc) * 2 + 1);
+
+ bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
+ fprintf (fp, ";stop-notes:%s", buf);
+ xfree (buf);
+ }
fprintf (fp, "\n");
@@ -3992,6 +4002,14 @@ Status line: '%s'\n"), p, line);
ts->notes[end] = '\0';
p = p3;
}
+ else if (strncmp (p, "stop-notes", p1 - p) == 0)
+ {
+ ++p1;
+ ts->stop_desc = xmalloc (strlen (p) / 2);
+ end = hex2bin (p1, ts->stop_desc, (p3 - p1) / 2);
+ ts->stop_desc[end] = '\0';
+ p = p3;
+ }
else
{
/* Silently skip unknown optional info. */
[-- Attachment #4: gdb_tsave_v4.diff --]
[-- Type: text/x-patch, Size: 2347 bytes --]
commit 4ce80946bb684913900b5e4db3c627b2acd91399
Author: Dmitry Kozlov <dmitry_kozlov@mentor.com>
Date: Fri Oct 12 11:32:25 2012 +0400
Extend tsave to save trace start time, stop time and notes.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 27ad6ab..3e2a818 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+ * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes.
+
+2012-10-12 Dmitry Kozlov <ddk@codesourcery.com>
+
* tracepoint.c (trace_status_command): Fix type of printf arg to
prevent improper type conversion.
(trace_status_mi): Likewise.
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 1577ad3..8f4f18c 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3018,10 +3018,11 @@ trace_save (const char *filename, int target_does_save)
(ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
if (ts->stop_reason == tracepoint_error)
{
- char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);
+ char *buf = xmalloc (strlen (ts->stop_desc) * 2 + 1);
bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
fprintf (fp, ":%s", buf);
+ xfree (buf);
}
fprintf (fp, ":%x", ts->stopping_tracepoint);
if (ts->traceframe_count >= 0)
@@ -3036,6 +3037,33 @@ trace_save (const char *filename, int target_does_save)
fprintf (fp, ";disconn:%x", ts->disconnected_tracing);
if (ts->circular_buffer)
fprintf (fp, ";circular:%x", ts->circular_buffer);
+ if (ts->start_time)
+ {
+ fprintf (fp, ";starttime:%s",
+ phex_nz (ts->start_time, sizeof (ts->start_time)));
+ }
+ if (ts->stop_time)
+ {
+ fprintf (fp, ";stoptime:%s",
+ phex_nz (ts->stop_time, sizeof (ts->stop_time)));
+ }
+ if (ts->notes && ts->notes[0] != 0)
+ {
+ char *buf = xmalloc (strlen (ts->notes) * 2 + 1);
+
+ bin2hex ((gdb_byte *) ts->notes, buf, 0);
+ fprintf (fp, ";notes:%s", buf);
+ xfree (buf);
+ }
+ if (ts->user_name && ts->user_name[0] != 0)
+ {
+ char *buf = xmalloc (strlen (ts->user_name) * 2 + 1);
+
+ bin2hex ((gdb_byte *) ts->user_name, buf, 0);
+ fprintf (fp, ";username:%s", buf);
+ xfree (buf);
+ }
+
fprintf (fp, "\n");
/* Note that we want to upload tracepoints and save those, rather
next prev parent reply other threads:[~2012-10-12 8:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-03 14:09 Dmitry Kozlov
2012-10-08 12:48 ` Yao Qi
2012-10-08 19:14 ` Dmitry Kozlov
2012-10-11 15:53 ` Dmitry Kozlov
2012-10-11 17:10 ` Pedro Alves
2012-10-12 8:44 ` Dmitry Kozlov
2012-10-12 8:46 ` Dmitry Kozlov [this message]
2012-10-16 16:36 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5077D8DB.2070908@mentor.com \
--to=dmitry_kozlov@mentor.com \
--cc=Stan_Shebs@mentor.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
--cc=vladimir@codesourcery.com \
--cc=yao@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox