From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19923 invoked by alias); 12 Oct 2012 08:44:16 -0000 Received: (qmail 19904 invoked by uid 22791); 12 Oct 2012 08:44:13 -0000 X-SWARE-Spam-Status: No, hits=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_XS X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 12 Oct 2012 08:44:06 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1TMaqh-0004gH-Pn from Dmitry_Kozlov@mentor.com ; Fri, 12 Oct 2012 01:44:03 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Fri, 12 Oct 2012 01:44:03 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Fri, 12 Oct 2012 09:44:00 +0100 Message-ID: <5077D84D.4010308@mentor.com> Date: Fri, 12 Oct 2012 08:44:00 -0000 From: Dmitry Kozlov User-Agent: Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120912 Thunderbird/15.0.1 MIME-Version: 1.0 To: Pedro Alves CC: Yao Qi , , "'Stan_Shebs@mentor.com'" , Vladimir Prus Subject: Re: [PATCH] Extend tsave: save start time, stop time, user and notes References: <506C4719.3030900@mentor.com> <5072CB78.8030908@codesourcery.com> <5076EB66.6080307@mentor.com> <5076FD5F.4060009@redhat.com> In-Reply-To: <5076FD5F.4060009@redhat.com> Content-Type: multipart/mixed; boundary="------------030609030001010105090301" Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-10/txt/msg00192.txt.bz2 --------------030609030001010105090301 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3752 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 >> + >> + * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes. >> + >> +2012-10-04 Dmitry Kozlov >> + >> + * 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 >> >> 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); >> + } --------------030609030001010105090301 Content-Type: text/x-patch; name="gdb_dates_v4.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gdb_dates_v4.diff" Content-length: 3646 commit b256b0abcf8e45a615c4ce763d694d371e988f83 Author: Dmitry Kozlov 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 + + * tracepoint.c (trace_status_command): Fix type of printf arg to + prevent improper type conversion. + (trace_status_mi): Likewise. + 2012-10-11 Andrew Burgess * 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 + + * 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 * 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); } } --------------030609030001010105090301 Content-Type: text/x-patch; name="gdb-stop-notes_v2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gdb-stop-notes_v2.diff" Content-length: 3417 commit d9cc6813eccfc2afa35be6b201a222a44c657b55 Author: Dmitry Kozlov Date: Wed Oct 10 14:56:52 2012 +0400 Modify trace-status output to always output stop-notes. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 9196bc2..f34f04e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,8 +1,19 @@ +2012-10-10 Dmitry Kozlov + + * tracepoint.c (trace_status_mi): Modify trace status MI command output to + always output stop-notes. + (trace_save): Save stop-notes if any by tsave. + (parse_trace_status): Add parsing stop-notes. + 2012-10-04 Dmitry Kozlov * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes. -2012-10-04 Dmitry Kozlov +2012-10-04 Dmitry Kozlov + + * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes. + +2012-10-04 Dmitry Kozlov * tracepoint.c (trace_status_command): Fix type of printf arg. (trace_status_mi): Likewise. diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 8bfbe9c..a23bb6d 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,8 @@ +2012-10-11 Dmitry Kozlov + + * gdbserver/tracepoint.c (cmd_qtstatus): Modify trace-status output to + always output stop-notes. + 2012-10-01 Andrew Burgess * server.c (handle_search_memory_1): Include access length in diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c index 201a25b..60ba775 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, @@ -3665,7 +3665,7 @@ cmd_qtstatus (char *packet) circular_trace_buffer, disconnected_tracing, plongest (tracing_start_time), plongest (tracing_stop_time), - buf1, buf2); + buf1, buf2, buf3); } static void diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index 959ede5..507ca41 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]; @@ -3065,6 +3067,14 @@ trace_save (const char *filename, int target_does_save) fprintf (fp, ";username:%s", buf); xfree (buf); } + if (ts->stop_desc) + { + char *buf = (char *) 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"); @@ -3994,6 +4004,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. */ --------------030609030001010105090301 Content-Type: text/x-patch; name="gdb_tsave_v4.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gdb_tsave_v4.diff" Content-length: 2347 commit 4ce80946bb684913900b5e4db3c627b2acd91399 Author: Dmitry Kozlov 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 + * tracepoint.c (trace_save): Add saving starttime, stoptime, user and notes. + +2012-10-12 Dmitry Kozlov + * 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 --------------030609030001010105090301--