[RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Hui Zhu]

Hi,

I found that there is not introduce for -1 in QTFrame:@var{n}.So I add one.

For example:
(gdb) tfind none
Sending packet: $QTFrame:ffffffff#fa...Packet received: OK
No longer looking at any trace frame
Sending packet: $g#67...Packet received:
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c05e33c9ff7f000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000000000000000f02ad7a8537f000000020000330000002b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000ffff0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f00003b00000000000000
Sending packet: $m7f53a8d72af0,1#2c...Packet received: 48
Sending packet: $m7f53a8d72af0,1#2c...Packet received: 48
Sending packet: $m7f53a8d72af0,9#34...Packet received: 4889e7e82806000049
#0  0x00007f53a8d72af0 in ?? () from /lib64/ld-linux-x86-64.so.2

BTW, I found that tracepoint gdbrsp use a lot of  "int", I think it
will be not very friendly to cross compile debug.  What do you think
about it?

Thanks,
Hui

2010-08-15  Hui Zhu  <teawater@gmail.com>

	* gdb.texinfo (Tracepoint Packets): add introduce for -1.


---
 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 the
 currently selected frame whose PC is @var{addr};

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Michael Snyder]

Hui Zhu wrote:

> 2010-08-15  Hui Zhu  <teawater@gmail.com>
> 
> 	* gdb.texinfo (Tracepoint Packets): add introduce for -1.
> 
> 
> ---
>  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,
                            s/that stop/to stop/          s/,/ and/

> +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 the
>  currently selected frame whose PC is @var{addr};

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Sun, 15 Aug 2010 15:16:34 +0800
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> 2010-08-15  Hui Zhu  <teawater@gmail.com>
> 
> 	* gdb.texinfo (Tracepoint Packets): add introduce for -1.

	* gdb.texinfo (Tracepoint Packets): Document the meaning of -1
          as the frame number N.

> +If @var{n} is -1, it mean that stop debugging trace snapshots,
> +resume live debugging.

 If @var{n} is -1, it means stop debugging trace snapshots and resume
 live debugging.

> +Replies:

 Reply:

Okay with these changes.

Thanks.

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Pedro Alves]

On Sunday 15 August 2010 08:16:34, Hui Zhu wrote:
> +@item E @var{NN}
> +A badly formed request was detected, 

(...)

> (...) or an error was encountered while relocating the instruction.

This last bit looks to be a copy&paste.  It doesn't make sense
here.

-- 
Pedro Alves

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Pedro Alves]

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.  (not sure it that was what you were
referring to).  IMO, negatives should have an explicit '-' encoding; in
this case, "$QTFrame:-1".  Note sure if the RSP docs mention something
about this.  We are careful in some cases (passing thread id's,
I think, is one case), though clearly not everywhere.

-- 
Pedro Alves

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Michael Snyder]

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.  (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.

That might also be iffy, but less so....


  IMO, negatives should have an explicit '-' encoding; in
> this case, "$QTFrame:-1".  Note sure if the RSP docs mention something
> about this.  We are careful in some cases (passing thread id's,
> I think, is one case), though clearly not everywhere.
> 

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Pedro Alves]

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.  (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.  If 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)
{
  trace_find_command ("-1", from_tty);
                       ^^
}

through here:

/* Worker function for the various flavors of the tfind command.  */
void
tfind_1 (enum trace_find_type type, int num,
                                    ^^^^^^^
	 ULONGEST addr1, ULONGEST addr2,
	 int from_tty)
{
  int target_frameno = -1, target_tracept = -1;
  struct frame_id old_frame_id = null_frame_id;
  struct breakpoint *tp;

  /* Only try to get the current stack frame if we have a chance of
     succeeding.  In particular, if we're trying to get a first trace
     frame while all threads are running, it's not going to succeed,
     so leave it with a default value and let the frame comparison
     below (correctly) decide to print out the source location of the
     trace frame.  */
  if (!(type == tfind_number && num == -1)
      && (has_stack_frames () || traceframe_number >= 0))
    old_frame_id = get_frame_id (get_current_frame ());

  target_frameno = target_trace_find (type, num, addr1, addr2,
				      &target_tracept);
  
  if (type == tfind_number
      && num == -1
         ^^^^^^^^^
      && target_frameno == -1)
    {
      /* We told the target to get out of tfind mode, and it did.  */
    }
  else if (target_frameno == -1)
    {
      /* A request for a non-existent trace frame has failed.
	 Our response will be different, depending on FROM_TTY:

	 If FROM_TTY is true, meaning that this command was 
	 typed interactively by the user, then give an error
	 and DO NOT change the state of traceframe_number etc.

	 However if FROM_TTY is false, meaning that we're either
	 in a script, a loop, or a user-defined command, then 
	 DON'T give an error, but DO change the state of
	 traceframe_number etc. to invalid.

	 The rationalle is that if you typed the command, you
	 might just have committed a typo or something, and you'd
	 like to NOT lose your current debugging state.  However
	 if you're in a user-defined command or especially in a
	 loop, then you need a way to detect that the command
	 failed WITHOUT aborting.  This allows you to write
	 scripts that search thru the trace buffer until the end,
	 and then continue on to do something else.  */
  
      if (from_tty)
	error (_("Target failed to find requested trace frame."));


Notice how you get to the "else if" branch.  Choosing any other
"high enough" random frame id that doesn't exist errors
out.  Only 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,
		   ULONGEST addr1, ULONGEST addr2,
		   int *tpp)
{
  struct remote_state *rs = get_remote_state ();
  char *p, *reply;
  int target_frameno = -1, target_tracept = -1;

  p = rs->buf;
  strcpy (p, "QTFrame:");
  p = strchr (p, '\0');
  switch (type)
    {
    case tfind_number:
      sprintf (p, "%x", num);
                  ^^^^^^^^^
      break;

... and here, NUM, is an int, and, is printed with %x.  Assuming
that prints 0xffffffff is actually host dependent (e.g., consider
ILP64).  Granted, likely not a concern on all hosts we care
about.  Still a needless assumption.

The gdbserver side does:

...
      unpack_varlen_hex (packet, &frame);
      tfnum = (int) frame;
      if (tfnum == -1)
	{
	  trace_debug ("Want to stop looking at traceframes");
	  current_traceframe = -1;
	  write_ok (own_buf);
	  return;
	}

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.  Fixing 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.  Not sure what to do.  I guess
I'll just pretend I didn't spot this.  ;-)

>   IMO, negatives should have an explicit '-' encoding; in
> > this case, "$QTFrame:-1".  Note sure if the RSP docs mention something
> > about this.  We are careful in some cases (passing thread id's,
> > I think, is one case), though clearly not everywhere.

-- 
Pedro Alves

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Hui Zhu]

On Mon, Aug 16, 2010 at 07:09, Pedro Alves <pedro@codesourcery.com> 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.  (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.  If 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)
> {
>  trace_find_command ("-1", from_tty);
>                       ^^
> }
>
> through here:
>
> /* Worker function for the various flavors of the tfind command.  */
> void
> tfind_1 (enum trace_find_type type, int num,
>                                    ^^^^^^^
>         ULONGEST addr1, ULONGEST addr2,
>         int from_tty)
> {
>  int target_frameno = -1, target_tracept = -1;
>  struct frame_id old_frame_id = null_frame_id;
>  struct breakpoint *tp;
>
>  /* Only try to get the current stack frame if we have a chance of
>     succeeding.  In particular, if we're trying to get a first trace
>     frame while all threads are running, it's not going to succeed,
>     so leave it with a default value and let the frame comparison
>     below (correctly) decide to print out the source location of the
>     trace frame.  */
>  if (!(type == tfind_number && num == -1)
>      && (has_stack_frames () || traceframe_number >= 0))
>    old_frame_id = get_frame_id (get_current_frame ());
>
>  target_frameno = target_trace_find (type, num, addr1, addr2,
>                                      &target_tracept);
>
>  if (type == tfind_number
>      && num == -1
>         ^^^^^^^^^
>      && target_frameno == -1)
>    {
>      /* We told the target to get out of tfind mode, and it did.  */
>    }
>  else if (target_frameno == -1)
>    {
>      /* A request for a non-existent trace frame has failed.
>         Our response will be different, depending on FROM_TTY:
>
>         If FROM_TTY is true, meaning that this command was
>         typed interactively by the user, then give an error
>         and DO NOT change the state of traceframe_number etc.
>
>         However if FROM_TTY is false, meaning that we're either
>         in a script, a loop, or a user-defined command, then
>         DON'T give an error, but DO change the state of
>         traceframe_number etc. to invalid.
>
>         The rationalle is that if you typed the command, you
>         might just have committed a typo or something, and you'd
>         like to NOT lose your current debugging state.  However
>         if you're in a user-defined command or especially in a
>         loop, then you need a way to detect that the command
>         failed WITHOUT aborting.  This allows you to write
>         scripts that search thru the trace buffer until the end,
>         and then continue on to do something else.  */
>
>      if (from_tty)
>        error (_("Target failed to find requested trace frame."));
>
>
> Notice how you get to the "else if" branch.  Choosing any other
> "high enough" random frame id that doesn't exist errors
> out.  Only 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,
>                   ULONGEST addr1, ULONGEST addr2,
>                   int *tpp)
> {
>  struct remote_state *rs = get_remote_state ();
>  char *p, *reply;
>  int target_frameno = -1, target_tracept = -1;
>
>  p = rs->buf;
>  strcpy (p, "QTFrame:");
>  p = strchr (p, '\0');
>  switch (type)
>    {
>    case tfind_number:
>      sprintf (p, "%x", num);
>                  ^^^^^^^^^
>      break;
>
> ... and here, NUM, is an int, and, is printed with %x.  Assuming
> that prints 0xffffffff is actually host dependent (e.g., consider
> ILP64).  Granted, likely not a concern on all hosts we care
> about.  Still a needless assumption.
>
> The gdbserver side does:
>
> ...
>      unpack_varlen_hex (packet, &frame);
>      tfnum = (int) frame;
>      if (tfnum == -1)
>        {
>          trace_debug ("Want to stop looking at traceframes");
>          current_traceframe = -1;
>          write_ok (own_buf);
>          return;
>        }
>
> 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.  Fixing 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.  Not sure what to do.  I guess
> I'll just pretend I didn't spot this.  ;-)
>
>>   IMO, negatives should have an explicit '-' encoding; in
>> > this case, "$QTFrame:-1".  Note sure if the RSP docs mention something
>> > about this.  We 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  <teawater@gmail.com>

	* remote.c(remote_trace_find): Handle the negative.

2010-08-19  Hui Zhu  <teawater@gmail.com>

	* tracepoint.c(cmd_qtframe): Handle the negative.

2010-08-19  Hui Zhu  <teawater@gmail.com>

	* 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 = 0;
+
+      if (*packet == '-')
+        {
+          is_negative = 1;
+          ++packet;
+        }
       unpack_varlen_hex (packet, &frame);
       tfnum = (int) frame;
+      if (is_negative)
+        tfnum = -tfnum;
       if (tfnum == -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 the
 currently selected frame whose PC is @var{addr};

Re: [RFA/RFC] tracepoint gdbrsp: add -1 introduce for QTFrame:@var{n}

[Hui Zhu]

On Thu, Aug 19, 2010 at 15:43, Hui Zhu <teawater@gmail.com> wrote:
> On Mon, Aug 16, 2010 at 07:09, Pedro Alves <pedro@codesourcery.com> 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.  (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.  If 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)
>> {
>>  trace_find_command ("-1", from_tty);
>>                       ^^
>> }
>>
>> through here:
>>
>> /* Worker function for the various flavors of the tfind command.  */
>> void
>> tfind_1 (enum trace_find_type type, int num,
>>                                    ^^^^^^^
>>         ULONGEST addr1, ULONGEST addr2,
>>         int from_tty)
>> {
>>  int target_frameno = -1, target_tracept = -1;
>>  struct frame_id old_frame_id = null_frame_id;
>>  struct breakpoint *tp;
>>
>>  /* Only try to get the current stack frame if we have a chance of
>>     succeeding.  In particular, if we're trying to get a first trace
>>     frame while all threads are running, it's not going to succeed,
>>     so leave it with a default value and let the frame comparison
>>     below (correctly) decide to print out the source location of the
>>     trace frame.  */
>>  if (!(type == tfind_number && num == -1)
>>      && (has_stack_frames () || traceframe_number >= 0))
>>    old_frame_id = get_frame_id (get_current_frame ());
>>
>>  target_frameno = target_trace_find (type, num, addr1, addr2,
>>                                      &target_tracept);
>>
>>  if (type == tfind_number
>>      && num == -1
>>         ^^^^^^^^^
>>      && target_frameno == -1)
>>    {
>>      /* We told the target to get out of tfind mode, and it did.  */
>>    }
>>  else if (target_frameno == -1)
>>    {
>>      /* A request for a non-existent trace frame has failed.
>>         Our response will be different, depending on FROM_TTY:
>>
>>         If FROM_TTY is true, meaning that this command was
>>         typed interactively by the user, then give an error
>>         and DO NOT change the state of traceframe_number etc.
>>
>>         However if FROM_TTY is false, meaning that we're either
>>         in a script, a loop, or a user-defined command, then
>>         DON'T give an error, but DO change the state of
>>         traceframe_number etc. to invalid.
>>
>>         The rationalle is that if you typed the command, you
>>         might just have committed a typo or something, and you'd
>>         like to NOT lose your current debugging state.  However
>>         if you're in a user-defined command or especially in a
>>         loop, then you need a way to detect that the command
>>         failed WITHOUT aborting.  This allows you to write
>>         scripts that search thru the trace buffer until the end,
>>         and then continue on to do something else.  */
>>
>>      if (from_tty)
>>        error (_("Target failed to find requested trace frame."));
>>
>>
>> Notice how you get to the "else if" branch.  Choosing any other
>> "high enough" random frame id that doesn't exist errors
>> out.  Only 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,
>>                   ULONGEST addr1, ULONGEST addr2,
>>                   int *tpp)
>> {
>>  struct remote_state *rs = get_remote_state ();
>>  char *p, *reply;
>>  int target_frameno = -1, target_tracept = -1;
>>
>>  p = rs->buf;
>>  strcpy (p, "QTFrame:");
>>  p = strchr (p, '\0');
>>  switch (type)
>>    {
>>    case tfind_number:
>>      sprintf (p, "%x", num);
>>                  ^^^^^^^^^
>>      break;
>>
>> ... and here, NUM, is an int, and, is printed with %x.  Assuming
>> that prints 0xffffffff is actually host dependent (e.g., consider
>> ILP64).  Granted, likely not a concern on all hosts we care
>> about.  Still a needless assumption.
>>
>> The gdbserver side does:
>>
>> ...
>>      unpack_varlen_hex (packet, &frame);
>>      tfnum = (int) frame;
>>      if (tfnum == -1)
>>        {
>>          trace_debug ("Want to stop looking at traceframes");
>>          current_traceframe = -1;
>>          write_ok (own_buf);
>>          return;
>>        }
>>
>> 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.  Fixing 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.  Not sure what to do.  I guess
>> I'll just pretend I didn't spot this.  ;-)
>>
>>>   IMO, negatives should have an explicit '-' encoding; in
>>> > this case, "$QTFrame:-1".  Note sure if the RSP docs mention something
>>> > about this.  We 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  <teawater@gmail.com>
>
>        * remote.c(remote_trace_find): Handle the negative.
>
> 2010-08-19  Hui Zhu  <teawater@gmail.com>
>
>        * tracepoint.c(cmd_qtframe): Handle the negative.
>
> 2010-08-19  Hui Zhu  <teawater@gmail.com>
>
>        * 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 = 0;
> +
> +      if (*packet == '-')
> +        {
> +          is_negative = 1;
> +          ++packet;
> +        }
>       unpack_varlen_hex (packet, &frame);
>       tfnum = (int) frame;
> +      if (is_negative)
> +        tfnum = -tfnum;
>       if (tfnum == -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 the
>  currently selected frame whose PC is @var{addr};
>

Sorry the doc patch is not the right version. following is the current version.

Thanks,
Hui

2010-08-19  Hui Zhu  <teawater@gmail.com>

	* gdb.texinfo (Tracepoint Packets): add introduce for -1.


---
 doc/gdb.texinfo |    9 +++++++++
 1 file changed, 9 insertions(+)

--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -33155,6 +33155,15 @@ 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.
+@end table
+
 @item QTFrame:pc:@var{addr}
 Like @samp{QTFrame:@var{n}}, but select the first tracepoint frame after the
 currently selected frame whose PC is @var{addr};