From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12594 invoked by alias); 19 Aug 2010 07:44:28 -0000 Received: (qmail 12579 invoked by uid 22791); 19 Aug 2010 07:44:25 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,TW_CP,TW_QT,TW_XF X-Spam-Check-By: sourceware.org Received: from mail-ww0-f43.google.com (HELO mail-ww0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 19 Aug 2010 07:44:18 +0000 Received: by wwe15 with SMTP id 15so781425wwe.12 for ; Thu, 19 Aug 2010 00:44:15 -0700 (PDT) Received: by 10.216.1.6 with SMTP id 6mr8117173wec.24.1282203855659; Thu, 19 Aug 2010 00:44:15 -0700 (PDT) MIME-Version: 1.0 Received: by 10.216.186.205 with HTTP; Thu, 19 Aug 2010 00:43:49 -0700 (PDT) In-Reply-To: <201008160009.57924.pedro@codesourcery.com> References: <201008151946.55330.pedro@codesourcery.com> <4C683A6C.8040501@vmware.com> <201008160009.57924.pedro@codesourcery.com> From: Hui Zhu Date: Thu, 19 Aug 2010 07:44:00 -0000 Message-ID: Subject: Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n} To: Pedro Alves Cc: Michael Snyder , "gdb-patches@sourceware.org" , Eli Zaretskii Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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: 2010-08/txt/msg00329.txt.bz2 On Mon, Aug 16, 2010 at 07:09, Pedro Alves wrote: > On Sunday 15 August 2010 20:05:16, Michael Snyder wrote: >> Pedro Alves wrote: >> > On Sunday 15 August 2010 08:16:34, Hui Zhu wrote: >> >> Sending packet: $QTFrame:ffffffff#fa...Packet received: OK >> > >> > I think it is a bug that this is assuming 32-bit, two's complement >> > on both client/server sides. =A0(not sure it that was what you were >> > referring to). >> >> It's not really assuming that -- it's just assuming that no >> legitimate frame ID will ever be as high as ffffffff. > > I don't know where you're getting that from. =A0If you believe that > to be true, then you also have to believe Hui's patch is wrong, > as it documents "-1", not the magic hard limit of "0xffffffff". > > Start here: > > /* tfind end */ > static void > trace_find_end_command (char *args, int from_tty) > { > =A0trace_find_command ("-1", from_tty); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ^^ > } > > through here: > > /* Worker function for the various flavors of the tfind command. =A0*/ > void > tfind_1 (enum trace_find_type type, int num, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0^^= ^^^^^ > =A0 =A0 =A0 =A0 ULONGEST addr1, ULONGEST addr2, > =A0 =A0 =A0 =A0 int from_tty) > { > =A0int target_frameno =3D -1, target_tracept =3D -1; > =A0struct frame_id old_frame_id =3D null_frame_id; > =A0struct breakpoint *tp; > > =A0/* Only try to get the current stack frame if we have a chance of > =A0 =A0 succeeding. =A0In particular, if we're trying to get a first trace > =A0 =A0 frame while all threads are running, it's not going to succeed, > =A0 =A0 so leave it with a default value and let the frame comparison > =A0 =A0 below (correctly) decide to print out the source location of the > =A0 =A0 trace frame. =A0*/ > =A0if (!(type =3D=3D tfind_number && num =3D=3D -1) > =A0 =A0 =A0&& (has_stack_frames () || traceframe_number >=3D 0)) > =A0 =A0old_frame_id =3D get_frame_id (get_current_frame ()); > > =A0target_frameno =3D target_trace_find (type, num, addr1, addr2, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0&target_tracept); > > =A0if (type =3D=3D tfind_number > =A0 =A0 =A0&& num =3D=3D -1 > =A0 =A0 =A0 =A0 ^^^^^^^^^ > =A0 =A0 =A0&& target_frameno =3D=3D -1) > =A0 =A0{ > =A0 =A0 =A0/* We told the target to get out of tfind mode, and it did. = =A0*/ > =A0 =A0} > =A0else if (target_frameno =3D=3D -1) > =A0 =A0{ > =A0 =A0 =A0/* A request for a non-existent trace frame has failed. > =A0 =A0 =A0 =A0 Our response will be different, depending on FROM_TTY: > > =A0 =A0 =A0 =A0 If FROM_TTY is true, meaning that this command was > =A0 =A0 =A0 =A0 typed interactively by the user, then give an error > =A0 =A0 =A0 =A0 and DO NOT change the state of traceframe_number etc. > > =A0 =A0 =A0 =A0 However if FROM_TTY is false, meaning that we're either > =A0 =A0 =A0 =A0 in a script, a loop, or a user-defined command, then > =A0 =A0 =A0 =A0 DON'T give an error, but DO change the state of > =A0 =A0 =A0 =A0 traceframe_number etc. to invalid. > > =A0 =A0 =A0 =A0 The rationalle is that if you typed the command, you > =A0 =A0 =A0 =A0 might just have committed a typo or something, and you'd > =A0 =A0 =A0 =A0 like to NOT lose your current debugging state. =A0However > =A0 =A0 =A0 =A0 if you're in a user-defined command or especially in a > =A0 =A0 =A0 =A0 loop, then you need a way to detect that the command > =A0 =A0 =A0 =A0 failed WITHOUT aborting. =A0This allows you to write > =A0 =A0 =A0 =A0 scripts that search thru the trace buffer until the end, > =A0 =A0 =A0 =A0 and then continue on to do something else. =A0*/ > > =A0 =A0 =A0if (from_tty) > =A0 =A0 =A0 =A0error (_("Target failed to find requested trace frame.")); > > > Notice how you get to the "else if" branch. =A0Choosing any other > "high enough" random frame id that doesn't exist errors > out. =A0Only the special case of 0xffffffff being parsed as -1 on > both target and host actually gets out of tfind mode. > > Continuing, the target_trace_find call ends up here: > > static int > remote_trace_find (enum trace_find_type type, int num, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ULONGEST addr1, ULONGEST addr2, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *tpp) > { > =A0struct remote_state *rs =3D get_remote_state (); > =A0char *p, *reply; > =A0int target_frameno =3D -1, target_tracept =3D -1; > > =A0p =3D rs->buf; > =A0strcpy (p, "QTFrame:"); > =A0p =3D strchr (p, '\0'); > =A0switch (type) > =A0 =A0{ > =A0 =A0case tfind_number: > =A0 =A0 =A0sprintf (p, "%x", num); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0^^^^^^^^^ > =A0 =A0 =A0break; > > ... and here, NUM, is an int, and, is printed with %x. =A0Assuming > that prints 0xffffffff is actually host dependent (e.g., consider > ILP64). =A0Granted, likely not a concern on all hosts we care > about. =A0Still a needless assumption. > > The gdbserver side does: > > ... > =A0 =A0 =A0unpack_varlen_hex (packet, &frame); > =A0 =A0 =A0tfnum =3D (int) frame; > =A0 =A0 =A0if (tfnum =3D=3D -1) > =A0 =A0 =A0 =A0{ > =A0 =A0 =A0 =A0 =A0trace_debug ("Want to stop looking at traceframes"); > =A0 =A0 =A0 =A0 =A0current_traceframe =3D -1; > =A0 =A0 =A0 =A0 =A0write_ok (own_buf); > =A0 =A0 =A0 =A0 =A0return; > =A0 =A0 =A0 =A0} > > Again would break if the target is ILP64, and host is not. > > And below that bit of code you'll see that trying to select any > other high enough frame number that does not exist > does not get out of tfind mode, but instead returns an error. > >> That might also be iffy, but less so.... > > I should have mentioned that I consider this a _design_ bug we > get to live with. =A0Fixing it like I suggested would be incompatible > with current targets, so, best leave this alone until it actually > becomes a problem. > > Though, this raises the point that Hui's docs don't > actually match what GDB sends. =A0Not sure what to do. =A0I guess > I'll just pretend I didn't spot this. =A0;-) > >> =A0 IMO, negatives should have an explicit '-' encoding; in >> > this case, "$QTFrame:-1". =A0Note sure if the RSP docs mention somethi= ng >> > about this. =A0We are careful in some cases (passing thread id's, >> > I think, is one case), though clearly not everywhere. > > -- > Pedro Alves > Hi, I am not sure we need keep this bug in the GDB. Make gdb and gdbserver support the old version is not a very hard thing. I make 3 patch for gdb, gdbserver and doc. For the gdbserver, it can support the old gdb that use 0xffffffff and new gdb that use -1. For the gdb, if you think we need, I can add a switch or something to make it send 0xffffffff. What do you think about it? Thanks, Hui 2010-08-19 Hui Zhu * remote.c(remote_trace_find): Handle the negative. 2010-08-19 Hui Zhu * tracepoint.c(cmd_qtframe): Handle the negative. 2010-08-19 Hui Zhu * gdb.texinfo (Tracepoint Packets): add introduce for -1. --- remote.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) --- a/remote.c +++ b/remote.c @@ -9979,7 +9979,10 @@ remote_trace_find (enum trace_find_type switch (type) { case tfind_number: - sprintf (p, "%x", num); + if (num < 0) + sprintf (p, "-%x", -num); + else + sprintf (p, "%x", num); break; case tfind_pc: sprintf (p, "pc:%s", phex_nz (addr1, 0)); --- gdbserver/tracepoint.c | 9 +++++++++ 1 file changed, 9 insertions(+) --- a/gdbserver/tracepoint.c +++ b/gdbserver/tracepoint.c @@ -3157,8 +3157,17 @@ cmd_qtframe (char *own_buf) } else { + int is_negative =3D 0; + + if (*packet =3D=3D '-') + { + is_negative =3D 1; + ++packet; + } unpack_varlen_hex (packet, &frame); tfnum =3D (int) frame; + if (is_negative) + tfnum =3D -tfnum; if (tfnum =3D=3D -1) { trace_debug ("Want to stop looking at traceframes"); --- doc/gdb.texinfo | 12 ++++++++++++ 1 file changed, 12 insertions(+) --- a/doc/gdb.texinfo +++ b/doc/gdb.texinfo @@ -33133,6 +33133,18 @@ The selected trace frame records a hit o @end table +If @var{n} is -1, it mean that stop debugging trace snapshots, +resume live debugging. + +Replies: +@table @samp +@item OK +The packet was understood and carried out. +@item E @var{NN} +A badly formed request was detected, or an error was encountered while +relocating the instruction. +@end table + @item QTFrame:pc:@var{addr} Like @samp{QTFrame:@var{n}}, but select the first tracepoint frame after t= he currently selected frame whose PC is @var{addr};