tracing broken if target doesn't do disconnected tracing

tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

The gdbserver tracepoints patch I just posted doesn't
include disconnected tracing yet.  Note what
happens then:

 (gdb) tar remote :9999
 Remote debugging using :9999
 Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib/ld-2.10.1.so...done.
 done.
 Loaded symbols for /lib64/ld-linux-x86-64.so.2
 0x00007f62e8519af0 in _start () from /lib64/ld-linux-x86-64.so.2
 (gdb) trace main
 Tracepoint 1 at 0x400640: file threads.c, line 35.
 (gdb) tstart
 Target does not support this command.

Taking a closer look, here's the problem:

 (gdb) set debug remote 1
 (gdb) tstart
 Sending packet: $QTinit#59...Packet received: OK
 Sending packet: $QTDP:1:0000000000400640:E:0:0#3f...Packet received: OK
 Sending packet: $QTDPsrc:1:400640:at:0:4:6d61696e#80...Packet received: OK
 Sending packet: $QTro:0000000000400200,000000000040021c:000000000040021c,000000000040023c:0000000000400240,0000000000400278:0000000000400278,0000000000400294:0000000000400298,0000000000400370:0000000000400370,00000000004003fc:00000000004003fc,000000000040040e:0000000000400410,0000000000400450:0000000000400450,0000000000400468:0000000000400468,00000000004004f8:00000000004004f8,0000000000400510:0000000000400510,0000000000400580:0000000000400580,0000000000400868:0000000000400868,0000000000400876:0000000000400878,000000000040087c:000000000040087c,00000000004008b0:00000000004008b0,00000000004009a4#5c...Packet received: OK
 Sending packet: $QTDisconnected:0#e2...Packet received:
 Target does not support this command.
 (gdb)

Below's the patch I'm using to get around this.

You'll still see GDB offering to leave the
trace running after disconnecting:

 ...
 (gdb) trace main
 Tracepoint 1 at 0x400640: file threads.c, line 35.
 (gdb) tstart
 (gdb) detach
 Trace is running.  Continue tracing after detach? (y or n) y
 warning: Target does not support disconnected tracing.
 Ending remote debugging.
 (gdb)

This is because common code outside remote.c has no clue
whether the target supports or not disconnected tracing, and
just assumes it does.  Exposing that property seems to
conflict a bit with the desire to allow
"set disconnected-tracing on" before actually being connected,
so, I've avoided doing it so far.  Maybe we could get away with
a tristate "yes|no|not-known-yet-maybe".

WDYT?  Shall I proceed with this as is?  Would you like to
address this?

-- 
Pedro Alves

2010-04-05  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* remote.c (remote_set_disconnected_tracing): Only set
	QTDisconnected if the remote end supports disconnected tracing.
	Warn otherwise, if trying to enable disconnected tracing.

---
 gdb/remote.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2010-04-04 23:55:38.000000000 +0100
+++ src/gdb/remote.c	2010-04-05 00:40:57.000000000 +0100
@@ -9776,11 +9776,16 @@ remote_set_disconnected_tracing (int val
 {
   struct remote_state *rs = get_remote_state ();
 
-  sprintf (rs->buf, "QTDisconnected:%x", val);
-  putpkt (rs->buf);
-  remote_get_noisy_reply (&target_buf, &target_buf_size);
-  if (strcmp (target_buf, "OK"))
-    error (_("Target does not support this command."));
+  if (rs->disconnected_tracing)
+    {
+      sprintf (rs->buf, "QTDisconnected:%x", val);
+      putpkt (rs->buf);
+      remote_get_noisy_reply (&target_buf, &target_buf_size);
+      if (strcmp (target_buf, "OK"))
+	error (_("Target does not support this command."));
+    }
+  else if (val)
+    warning (_("Target does not support disconnected tracing."));
 }
 
 static int

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

Pedro Alves wrote:
> The gdbserver tracepoints patch I just posted doesn't
> include disconnected tracing yet.  Note what
> happens then: [...]
>   
>  
>
> This is because common code outside remote.c has no clue
> whether the target supports or not disconnected tracing, and
> just assumes it does.  Exposing that property seems to
> conflict a bit with the desire to allow
> "set disconnected-tracing on" before actually being connected,
> so, I've avoided doing it so far.  Maybe we could get away with
> a tristate "yes|no|not-known-yet-maybe".
>
> WDYT?  Shall I proceed with this as is?  Would you like to
> address this?
>   

This is pretty close to what we want, I think; but generic code does 
need to know about this, so that disconnect_or_stop_tracing can say the 
right things.  For instance, a target that cannot do disconnected 
tracing should warn that the user that the detach is about to bring an 
ongoing trace experiment to a halt, and certainly shouldn't tease the 
user with a query() that can't possibly succeed.

I'm thinking that this is in some ways like the circular trace buffer 
flag, and could be reasonably made accessible via the trace_status 
structure.  Unless you're just dying to do this yourself :-) , I'll take 
care of the two together.

(I can see where this is going longterm - struct trace_status is 
effectively a host-side copy of the agent's state, echoed back in 
response to any instructions that GDB sends down.)

Stan

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Monday 05 April 2010 02:07:56, Stan Shebs wrote:
> I'm thinking that this is in some ways like the circular trace buffer 
> flag, and could be reasonably made accessible via the trace_status 
> structure.  Unless you're just dying to do this yourself :-) , I'll take 
> care of the two together.

Please, be my guest.  :-)

> (I can see where this is going longterm - struct trace_status is 
> effectively a host-side copy of the agent's state, echoed back in 
> response to any instructions that GDB sends down.)

I'd only suggest you keep in mind what will happen when we define
how tracing works with multi-process, as we'll be working on that
soonish.  That is, whether with multi-process, there will be a trace
status object per process, while supporting disconnected tracing
or not sounds like a target (debug interface) wide attribute.

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

Pedro Alves wrote:
> On Monday 05 April 2010 02:07:56, Stan Shebs wrote:
>   
>> I'm thinking that this is in some ways like the circular trace buffer 
>> flag, and could be reasonably made accessible via the trace_status 
>> structure.  Unless you're just dying to do this yourself :-) , I'll take 
>> care of the two together.
>>     
>
> Please, be my guest.  :-)
>   

Here's my suggested changes, and of course it implies some target-side 
changes as well.  Turns out that I generated some confusion for myself 
by auto-generating tstop commands, which I realized when I was unable to 
write a code comment that justified it. :-)

So the new behavior is that GDB always simply drops the connection.  If 
the target can do disconnected tracing, it just keeps going.  If it 
can't, or the query answer instructs it to not keep tracing, it stops, 
and records the stop reason as "tdisconnection"; you'll then see that 
when you reconnect to the target and do a tstatus.  Since tstatus 
reports the target's state consistently now, you'll also see that in a 
trace file.  Ditto for circularity of trace buffer.

If this behavior seems reasonable, I'll add appropriate bits to the 
manual and repost/commit.

As for multi-process, I can see reasons to make these kinds of flags 
either per-process or global to the target, so didn't try to guess at 
the final decision here. :-)

2010-04-06  Stan Shebs  <stan@codesourcery.com>
        Pedro Alves  <pedro@codesourcery.com>

    * tracepoint.h (struct trace_status): New fields disconnected_tracing
    and circular_buffer.
    * tracepoint.c (trace_status_command): Display target's status for
    disconnected tracing and circular buffer.
    (disconnect_or_stop_tracing): Add query for non-disconnected-tracing
    case, remove the stop_tracing call.
    (tfile_open): Clear disconnected and circular buffer status.
    (trace_save): Save disconnected and circular buffer status.
    (parse_trace_status): Parse disconnected and circular buffer status,
    also recognize disconnected as a stop reason.
    * remote.c (remote_set_disconnected_tracing): Only set
    QTDisconnected if the remote end supports disconnected tracing.
    Warn otherwise, if trying to enable disconnected tracing.




[-- Attachment #2: disconn-patch-1 --]
[-- Type: text/plain, Size: 7482 bytes --]

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.399
diff -p -r1.399 remote.c
*** remote.c	2 Apr 2010 01:18:34 -0000	1.399
--- remote.c	7 Apr 2010 01:16:33 -0000
*************** remote_set_disconnected_tracing (int val
*** 9776,9786 ****
  {
    struct remote_state *rs = get_remote_state ();
  
!   sprintf (rs->buf, "QTDisconnected:%x", val);
!   putpkt (rs->buf);
!   remote_get_noisy_reply (&target_buf, &target_buf_size);
!   if (strcmp (target_buf, "OK"))
!     error (_("Target does not support this command."));
  }
  
  static int
--- 9776,9791 ----
  {
    struct remote_state *rs = get_remote_state ();
  
!   if (rs->disconnected_tracing)
!     {
!       sprintf (rs->buf, "QTDisconnected:%x", val);
!       putpkt (rs->buf);
!       remote_get_noisy_reply (&target_buf, &target_buf_size);
!       if (strcmp (target_buf, "OK"))
! 	error (_("Target does not support this command."));
!     }
!   else if (val)
!     warning (_("Target does not support disconnected tracing."));
  }
  
  static int
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.173
diff -p -r1.173 tracepoint.c
*** tracepoint.c	6 Apr 2010 17:47:40 -0000	1.173
--- tracepoint.c	7 Apr 2010 01:16:33 -0000
*************** trace_status_command (char *args, int fr
*** 1626,1635 ****
    else if (ts->running)
      {
        printf_filtered (_("Trace is running on the target.\n"));
-       if (disconnected_tracing)
- 	printf_filtered (_("Trace will continue if GDB disconnects.\n"));
-       else
- 	printf_filtered (_("Trace will stop if GDB disconnects.\n"));
      }
    else
      {
--- 1626,1631 ----
*************** trace_status_command (char *args, int fr
*** 1699,1704 ****
--- 1695,1708 ----
  			 ts->buffer_free);
      }
  
+   if (ts->disconnected_tracing)
+     printf_filtered (_("Trace will continue if GDB disconnects.\n"));
+   else
+     printf_filtered (_("Trace will stop if GDB disconnects.\n"));
+ 
+   if (ts->circular_buffer)
+     printf_filtered (_("Trace buffer is circular.\n"));
+ 
    /* Now report on what we're doing with tfind.  */
    if (traceframe_number >= 0)
      printf_filtered (_("Looking at trace frame %d, tracepoint %d.\n"),
*************** trace_status_mi (int on_stop)
*** 1801,1806 ****
--- 1805,1813 ----
      ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
  }
  
+ /* This function handles the details of what to do about an ongoing
+    tracing run if the user has asked to detach or otherwise disconnect
+    from the target.  */
  
  void
  disconnect_or_stop_tracing (int from_tty)
*************** disconnect_or_stop_tracing (int from_tty
*** 1812,1829 ****
    if (target_get_trace_status (current_trace_status ()) < 0)
      current_trace_status ()->running = 0;
  
    if (current_trace_status ()->running && from_tty)
      {
!       int cont = query (_("Trace is running.  Continue tracing after detach? "));
!       /* Note that we send the query result without affecting the
! 	 user's setting of disconnected_tracing, so that the answer is
! 	 a one-time-only.  */
!       send_disconnected_tracing_value (cont);
! 
!       /* Also ensure that we do the equivalent of a tstop command if
! 	 tracing is not to continue after the detach.  */
!       if (!cont)
! 	stop_tracing ();
      }
  
    /* Also we want to be out of tfind mode, otherwise things can get
--- 1819,1844 ----
    if (target_get_trace_status (current_trace_status ()) < 0)
      current_trace_status ()->running = 0;
  
+   /* If running interactively, offer the user options for what to do
+      with the run.  Scripts are just going to disconnect and let the
+      target deal with it, according to how it's been instructed
+      previously via disconnected-tracing.  */
    if (current_trace_status ()->running && from_tty)
      {
!       if (current_trace_status ()->disconnected_tracing)
! 	{
! 	  int cont = query (_("Trace is running.  Continue tracing after detach? "));
! 
! 	  /* Note that we send the query result without affecting the
! 	     user's setting of disconnected_tracing, so that the answer is
! 	     a one-time-only.  */
! 	  send_disconnected_tracing_value (cont);
! 	}
!       else
! 	{
! 	  if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
! 	    error (_("Not confirmed."));
! 	}
      }
  
    /* Also we want to be out of tfind mode, otherwise things can get
*************** trace_save (const char *filename, int ta
*** 2599,2604 ****
--- 2614,2623 ----
      fprintf (fp, ";tfree:%x", ts->buffer_free);
    if (ts->buffer_size >= 0)
      fprintf (fp, ";tsize:%x", ts->buffer_size);
+   if (ts->disconnected_tracing)
+     fprintf (fp, ";disconn:%x", ts->disconnected_tracing);
+   if (ts->circular_buffer)
+     fprintf (fp, ";circular:%x", ts->circular_buffer);
    fprintf (fp, "\n");
  
    /* Note that we want to upload tracepoints and save those, rather
*************** tfile_open (char *filename, int from_tty
*** 3167,3172 ****
--- 3186,3193 ----
    ts->stop_reason = trace_stop_reason_unknown;
    ts->traceframe_count = -1;
    ts->buffer_free = 0;
+   ts->disconnected_tracing = 0;
+   ts->circular_buffer = 0;
  
    /* Read through a section of newline-terminated lines that
       define things like tracepoints.  */
*************** parse_trace_status (char *line, struct t
*** 3282,3287 ****
--- 3303,3310 ----
    ts->traceframes_created = -1;
    ts->buffer_free = -1;
    ts->buffer_size = -1;
+   ts->disconnected_tracing = 0;
+   ts->circular_buffer = 0;
  
    while (*p++)
      {
*************** Status line: '%s'\n"), p, line);
*** 3310,3315 ****
--- 3333,3343 ----
  	  p = unpack_varlen_hex (++p1, &val);
  	  ts->stop_reason = tstop_command;
  	}
+       else if (strncmp (p, stop_reason_names[trace_disconnected], p1 - p) == 0)
+ 	{
+ 	  p = unpack_varlen_hex (++p1, &val);
+ 	  ts->stop_reason = trace_disconnected;
+ 	}
        else if (strncmp (p, stop_reason_names[tracepoint_error], p1 - p) == 0)
  	{
  	  p2 = strchr (++p1, ':');
*************** Status line: '%s'\n"), p, line);
*** 3348,3353 ****
--- 3376,3391 ----
  	  p = unpack_varlen_hex (++p1, &val);
  	  ts->buffer_size = val;
  	}
+       else if (strncmp (p, "disconn", p1 - p) == 0)
+ 	{
+ 	  p = unpack_varlen_hex (++p1, &val);
+ 	  ts->disconnected_tracing = val;
+ 	}
+       else if (strncmp (p, "circular", p1 - p) == 0)
+ 	{
+ 	  p = unpack_varlen_hex (++p1, &val);
+ 	  ts->circular_buffer = val;
+ 	}
        else
  	{
  	  /* Silently skip unknown optional info.  */
Index: tracepoint.h
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.h,v
retrieving revision 1.34
diff -p -r1.34 tracepoint.h
*** tracepoint.h	6 Apr 2010 17:47:40 -0000	1.34
--- tracepoint.h	7 Apr 2010 01:16:33 -0000
*************** struct trace_status
*** 106,111 ****
--- 106,121 ----
    /* Unused bytes left in the target's trace buffer.  */
  
    int buffer_free;
+ 
+   /* 1 if the target will continue tracing after disconnection, else
+      0.  If the target does not report a value, assume 0.  */
+ 
+   int disconnected_tracing;
+ 
+   /* 1 if the target is using a circular trace buffer, else 0.  If the
+      target does not report a value, assume 0.  */
+ 
+   int circular_buffer;
  };
  
  struct trace_status *current_trace_status (void);

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Wednesday 07 April 2010 02:34:12, Stan Shebs wrote:
> Here's my suggested changes, and of course it implies some target-side 
> changes as well.  Turns out that I generated some confusion for myself 
> by auto-generating tstop commands, which I realized when I was unable to 
> write a code comment that justified it. :-)
> 
> So the new behavior is that GDB always simply drops the connection.  If 
> the target can do disconnected tracing, it just keeps going.  If it 
> can't, or the query answer instructs it to not keep tracing, it stops, 
> and records the stop reason as "tdisconnection"; you'll then see that 
> when you reconnect to the target and do a tstatus.  Since tstatus 
> reports the target's state consistently now, you'll also see that in a 
> trace file.  Ditto for circularity of trace buffer.
> 
> If this behavior seems reasonable, I'll add appropriate bits to the 
> manual and repost/commit.

Thanks!  This is a good improvement, IMO.

> As for multi-process, I can see reasons to make these kinds of flags 
> either per-process or global to the target, so didn't try to guess at 
> the final decision here. :-)

The support for the feature is reported by qSupported, hence it's
certainly target-wide noawadays.  It may or not be desirable to
be able to select which processes keep tracing on disconnect, so
a per-status state flag for that also sounds acceptable --- it
could represent whether tracing will continue for a given process
after disconnection.  The flag (trace_status->disconnected_tracing)
being 0 doesn't mean the target doesn't support disconnected
tracing, so there's still no way for the common code to know it.

With the current patch, against the gdbserver that doesn't
support disconnected tracing, I still see this:

 (gdb) set disconnected-tracing on
 warning: Target does not support disconnected tracing.
 (gdb) show disconnected-tracing
 Whether tracing continues after GDB disconnects is on.

(yes, I wrote the warning, but it was a hack needed before
so that quitting wouldn't get stuck.)

With a user hat on, the "Whether tracing continues after
GDB disconnects is on." string as above looks confusing
to me.  Same, or worse for the circular-trace-buffer
setting, as that one says "target's use of", which isn't
true, it's solely GDB's intention.

Is there a way to make this pattern unconfusing?  We're
going to grow more similar options in the future, and
this will proliferate if unattended.

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

Pedro Alves wrote:
> The support for the feature is reported by qSupported, hence it's
> certainly target-wide noawadays.  It may or not be desirable to
> be able to select which processes keep tracing on disconnect, so
> a per-status state flag for that also sounds acceptable --- it
> could represent whether tracing will continue for a given process
> after disconnection.  The flag (trace_status->disconnected_tracing)
> being 0 doesn't mean the target doesn't support disconnected
> tracing, so there's still no way for the common code to know it.
>   

In a way, what the user wants to know is what qSupported reports, 
dressed up in a reasonable fashion.  Our traditional expectation has 
been the user knows already, because, well, it's the user's program and 
the user's hardware.  But as the target gets more elaborate - and 
tracepoint support is certainly a quantum jump in that direction :-) - 
that assumption breaks down.  In that vein, you may recall that one of 
our upcoming enhancements is to attach an arbitrary text string to a 
trace run, with the intended purpose of including things like a name and 
phone number, so that someone else connecting to the target can have a 
way to find out about the trace run, and know whether it's OK to mess 
with it.
> With the current patch, against the gdbserver that doesn't
> support disconnected tracing, I still see this:
>
>  (gdb) set disconnected-tracing on
>  warning: Target does not support disconnected tracing.
>  (gdb) show disconnected-tracing
>  Whether tracing continues after GDB disconnects is on.
>
> (yes, I wrote the warning, but it was a hack needed before
> so that quitting wouldn't get stuck.)
>
> With a user hat on, the "Whether tracing continues after
> GDB disconnects is on." string as above looks confusing
> to me.  Same, or worse for the circular-trace-buffer
> setting, as that one says "target's use of", which isn't
> true, it's solely GDB's intention.
>
> Is there a way to make this pattern unconfusing?  We're
> going to grow more similar options in the future, and
> this will proliferate if unattended.
>
>   

Yeah, it's been troubling me too.  User-settable variables are GDB's 
traditional way of instructing GDB about user preferences, but the 
canned method of phrase construction is too lame to express what is 
really going on, which is "I prefer that targets to continue tracing 
after disconnect, whether or not the current target can actually do so". 
We could use something other than set/show, or invent a better method to 
produce output - there are other set/shows for which the verbiage is 
rather contorted.

Stan

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Wednesday 07 April 2010 14:32:49, Stan Shebs wrote:
> Pedro Alves wrote:
> > The support for the feature is reported by qSupported, hence it's
> > certainly target-wide noawadays.  It may or not be desirable to
> > be able to select which processes keep tracing on disconnect, so
> > a per-status state flag for that also sounds acceptable --- it
> > could represent whether tracing will continue for a given process
> > after disconnection.  The flag (trace_status->disconnected_tracing)
> > being 0 doesn't mean the target doesn't support disconnected
> > tracing, so there's still no way for the common code to know it.
> >   
> 
> In a way, what the user wants to know is what qSupported reports, 
> dressed up in a reasonable fashion.  Our traditional expectation has 
> been the user knows already, because, well, it's the user's program and 
> the user's hardware.  But as the target gets more elaborate - and 
> tracepoint support is certainly a quantum jump in that direction :-) - 
> that assumption breaks down.  In that vein, you may recall that one of 
> our upcoming enhancements is to attach an arbitrary text string to a 
> trace run, with the intended purpose of including things like a name and 
> phone number, so that someone else connecting to the target can have a 
> way to find out about the trace run, and know whether it's OK to mess 
> with it.

I missed your point here.  My point was that supporting disconnect
tracing or not seems to want to be target-wide feature, and that hence,
it should probably me exposed to common code as a target method (e.g.,
target_supports_disconnected_tracing).  This, independent of a given
process/trace status saying that tracing will continue on detach
for a given run.  So, common code could do things like:

  if (current_trace_status ()->running
      && !current_trace_status ()->disconnected_tracing 
      && target_supports_disconnected_tracing ())
     {
        int cont = query (_("Trace is running.  Continue tracing after foo?")));
        send_disconnected_tracing_value (cont);
     }

and also in common code:

 static void
 set_disconnected_tracing (char *args, int from_tty,
                           struct cmd_list_element *c)
 {
   if (target_supports_disconnected_tracing ()
       && disconnected_tracing)
     send_disconnected_tracing_value (disconnected_tracing);
   else if (disconnected_tracing)
    {
      error/warn ("Target doesn't do disconnected tracing");
    }
 }

Instead of hacking it in remote.c, way late.

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Wednesday 07 April 2010 14:32:49, Stan Shebs wrote:
> Yeah, it's been troubling me too.  User-settable variables are GDB's 
> traditional way of instructing GDB about user preferences, but the 
> canned method of phrase construction is too lame to express what is 
> really going on, which is "I prefer that targets to continue tracing 
> after disconnect, whether or not the current target can actually do so". 
> We could use something other than set/show, or invent a better method to 
> produce output - there are other set/shows for which the verbiage is 
> rather contorted.

Hmm, can you expand on what lameness you're referring to exactly?
Is it a technical limitation?

These commands seem to fall in a close category:

(gdb) apropos willingness
set can-use-hw-watchpoints -- Set debugger's willingness to use watchpoint hardware
set displaced-stepping -- Set debugger's willingness to use displaced stepping
show can-use-hw-watchpoints -- Show debugger's willingness to use watchpoint hardware
show displaced-stepping -- Show debugger's willingness to use displaced stepping

Maybe we could follow suit similarly, or instead say something
like: "Show whether GDB prefers to/that/whether ..."

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

Pedro Alves wrote:
> On Wednesday 07 April 2010 14:32:49, Stan Shebs wrote:
>   
>> Yeah, it's been troubling me too.  User-settable variables are GDB's 
>> traditional way of instructing GDB about user preferences, but the 
>> canned method of phrase construction is too lame to express what is 
>> really going on, which is "I prefer that targets to continue tracing 
>> after disconnect, whether or not the current target can actually do so". 
>> We could use something other than set/show, or invent a better method to 
>> produce output - there are other set/shows for which the verbiage is 
>> rather contorted.
>>     
>
> Hmm, can you expand on what lameness you're referring to exactly?
> Is it a technical limitation?
>   

It's the requirement to describe the variable with a noun phrase like 
"foo bar of baz", so that the output can be "foo bar of baz is on" etc.  
For simple settings the algorithm is reasonable, but as the concept gets 
more complicated, the phrasing gets more tortured.

> These commands seem to fall in a close category:
>
> (gdb) apropos willingness
> set can-use-hw-watchpoints -- Set debugger's willingness to use watchpoint hardware
> set displaced-stepping -- Set debugger's willingness to use displaced stepping
> show can-use-hw-watchpoints -- Show debugger's willingness to use watchpoint hardware
> show displaced-stepping -- Show debugger's willingness to use displaced stepping
>
> Maybe we could follow suit similarly, or instead say something
> like: "Show whether GDB prefers to/that/whether ..."
>   

These are perfect examples of the same problem - "willingness" is not 
the right word.  What these flags really mean is "use if possible", but 
that's a verb in an imperative form plus a conditional, and it doesn't 
have a direct equivalent as a noun phrase.  Ideally, the "use if 
possible" would be augmented with "and it's possible" or "but it's not 
possible for reason X" so that the user can see both that the setting is 
as desired, and whether the debugger is able to respect it.

Not a difficult extension to the command definition machinery, just have 
to spend some time on it.

Stan

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Wednesday 07 April 2010 21:20:55, Stan Shebs wrote:
> >
> > Hmm, can you expand on what lameness you're referring to exactly?
> > Is it a technical limitation?
> >   
> 
> It's the requirement to describe the variable with a noun phrase like 
> "foo bar of baz", so that the output can be "foo bar of baz is on" etc.  
> For simple settings the algorithm is reasonable, but as the concept gets 
> more complicated, the phrasing gets more tortured.

I think you can already have what you want.  See how
the "show displaced-stepping", "show breakpoint always-inserted",
"show language" are implemented.  E.g.,

static void
show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
				 struct cmd_list_element *c,
				 const char *value)
{
  if (can_use_displaced_stepping == can_use_displaced_stepping_auto)
    fprintf_filtered (file, _("\
Debugger's willingness to use displaced stepping to step over \
breakpoints is %s (currently %s).\n"),
		      value, non_stop ? "on" : "off");
  else
    fprintf_filtered (file, _("\
Debugger's willingness to use displaced stepping to step over \
breakpoints is %s.\n"), value);
}

So, here:

>  add_setshow_boolean_cmd ("disconnected-tracing", no_class,
>			   &disconnected_tracing, _("\
>Set whether tracing continues after GDB disconnects."), _("\
>Show whether tracing continues after GDB disconnects."), _("\
>Use this to continue a tracing run even if GDB disconnects\n\
>or detaches from the target.  You can reconnect later and look at\n\
>trace data collected in the meantime."),
>			   set_disconnected_tracing,
>			   NULL,
                                       ^^^^

this callback is NULL.  YOu can set to a callback that computes
the show string in response to "show disconnected-tracing".
In there you can use something like "target_can_use_disconnected_tracing()",
or "current_trace_status ()->circular" to do that "(currently foo)"/"(but
target is lame and doesn't support)" thing.

Does that help?

Actually, we should not be allowing new commands with that
callback as NULL.  Take a look at `deprecated_show_value_hack',
which is the default callback --- it's busted with i18n:

  /* Print doc minus "show" at start.  */
  print_doc_line (gdb_stdout, c->doc + 5);

This is PR8413 <http://sourceware.org/bugzilla/show_bug.cgi?id=8413>.

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Wednesday 07 April 2010 12:40:36, Pedro Alves wrote:
> On Wednesday 07 April 2010 02:34:12, Stan Shebs wrote:
> > Here's my suggested changes, and of course it implies some target-side 
> > changes as well.  Turns out that I generated some confusion for myself 
> > by auto-generating tstop commands, which I realized when I was unable to 
> > write a code comment that justified it. :-)
> > 
> > So the new behavior is that GDB always simply drops the connection.  If 
> > the target can do disconnected tracing, it just keeps going.  If it 
> > can't, or the query answer instructs it to not keep tracing, it stops, 
> > and records the stop reason as "tdisconnection"; you'll then see that 
> > when you reconnect to the target and do a tstatus.  Since tstatus 
> > reports the target's state consistently now, you'll also see that in a 
> > trace file.  Ditto for circularity of trace buffer.
> > 
> > If this behavior seems reasonable, I'll add appropriate bits to the 
> > manual and repost/commit.
> 
> Thanks!  This is a good improvement, IMO.

Actually, I have one further comment, before you go write documentation
bits:

> +   /* If running interactively, offer the user options for what to do
> +      with the run.  Scripts are just going to disconnect and let the
> +      target deal with it, according to how it's been instructed
> +      previously via disconnected-tracing.  */
>     if (current_trace_status ()->running && from_tty)
>       {
> !       if (current_trace_status ()->disconnected_tracing)
> !       {
> !         int cont = query (_("Trace is running.  Continue tracing after detach? "));
> ! 
> !         /* Note that we send the query result without affecting the
> !            user's setting of disconnected_tracing, so that the answer is
> !            a one-time-only.  */
> !         send_disconnected_tracing_value (cont);
> !       }
> !       else
> !       {
> !         if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
> !           error (_("Not confirmed."));
> !       }
>       }

We've had people reporting these queries were confusing
before.

The user has asked to detach: in one branch (the else branch), gdb
would warn trace was running and gave you
the option of canceling the detach.  In the other branch, it
gave you no such option, it always detaches.  No simple way of
going "ooops, right, I have a trace tracing, didn't really
mean to detach yet."  I've been bitten by this before.  Yes, you can
reconnect to the target, but, it's just annoying, and I don't think
people will often want to chose to say "no" to the
"Trace is running.  Continue tracing after detach? " query.
I think they'll want to say "don't detach yet then" often instead.

I suggest we change that to simply:

  /* If running interactively, warn the user a trace run is ongoing.
     She may want to cancel detaching instead.  */
    if (current_trace_status ()->running && from_tty)
      {
        if (current_trace_status ()->disconnected_tracing)
          {
            if (!query (_("Trace is running and will continue after detach; detach anyway? ")))
              error (_("Not confirmed."));
          }
        else
          {
            if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
             error (_("Not confirmed."));
          }
      }

  - simpler, more coherent, less explaining, less confusing.

> 
> > As for multi-process, I can see reasons to make these kinds of flags 
> > either per-process or global to the target, so didn't try to guess at 
> > the final decision here. :-)
> 
> The support for the feature is reported by qSupported, hence it's
> certainly target-wide noawadays.  It may or not be desirable to
> be able to select which processes keep tracing on disconnect, so
> a per-status state flag for that also sounds acceptable --- it
> could represent whether tracing will continue for a given process
> after disconnection.  The flag (trace_status->disconnected_tracing)
> being 0 doesn't mean the target doesn't support disconnected
> tracing, so there's still no way for the common code to know it.
> 
> With the current patch, against the gdbserver that doesn't
> support disconnected tracing, I still see this:
> 
>  (gdb) set disconnected-tracing on
>  warning: Target does not support disconnected tracing.
>  (gdb) show disconnected-tracing
>  Whether tracing continues after GDB disconnects is on.
> 
> (yes, I wrote the warning, but it was a hack needed before
> so that quitting wouldn't get stuck.)
> 
> With a user hat on, the "Whether tracing continues after
> GDB disconnects is on." string as above looks confusing
> to me.  Same, or worse for the circular-trace-buffer
> setting, as that one says "target's use of", which isn't
> true, it's solely GDB's intention.
> 
> Is there a way to make this pattern unconfusing?  We're
> going to grow more similar options in the future, and
> this will proliferate if unattended.
> 
> 


-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

Pedro Alves wrote:

> I suggest we change that to simply:
>
>   /* If running interactively, warn the user a trace run is ongoing.
>      She may want to cancel detaching instead.  */
>     if (current_trace_status ()->running && from_tty)
>       {
>         if (current_trace_status ()->disconnected_tracing)
>           {
>             if (!query (_("Trace is running and will continue after detach; detach anyway? ")))
>               error (_("Not confirmed."));
>           }
>         else
>           {
>             if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
>              error (_("Not confirmed."));
>           }
>       }
>
>   - simpler, more coherent, less explaining, less confusing.
>   

The downside of this design is that if you did want to shut tracing 
down, you have to cancel the detach, do a tstop, then redo the detach.  
It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
the power to choose whether to keep the trace running, but not to 
exercise it, and to insist that you have cancel and type the command 
yourself.  Perhaps the crux of the confusion is that this is really a 
three-way choice - trace/detach, tstop/detach, cancel - and a pair of 
yes/no questions is not a good way to model it.

Stan

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Wednesday 07 April 2010 23:04:38, Stan Shebs wrote:
> Pedro Alves wrote:
> 
> > I suggest we change that to simply:
> >
> >   /* If running interactively, warn the user a trace run is ongoing.
> >      She may want to cancel detaching instead.  */
> >     if (current_trace_status ()->running && from_tty)
> >       {
> >         if (current_trace_status ()->disconnected_tracing)
> >           {
> >             if (!query (_("Trace is running and will continue after detach; detach anyway? ")))
> >               error (_("Not confirmed."));
> >           }
> >         else
> >           {
> >             if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
> >              error (_("Not confirmed."));
> >           }
> >       }
> >
> >   - simpler, more coherent, less explaining, less confusing.
> >   
> 
> The downside of this design is that if you did want to shut tracing 
> down, you have to cancel the detach, do a tstop, then redo the detach.  
> It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
> the power to choose whether to keep the trace running, but not to 
> exercise it, and to insist that you have cancel and type the command 
> yourself.  Perhaps the crux of the confusion is that this is really a 
> three-way choice - trace/detach, tstop/detach, cancel - and a pair of 
> yes/no questions is not a good way to model it.

That's just begging for:

The downside of your current design is that if you did want to detach
and leave tracing running, you have to cancel the detach, do a "set
disconnected-tracing on", then redo the detach.
It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
the power to choose whether to keep the trace running, but not to 
exercise it, and to insist that you have cancel and type the command 
yourself.

:-)

I'd say that this use case would be much more common, and if
gdb gave you the option of turning disconnected tracing _on_
would be more helpful than the other way around (having trace
running and wanting it to stop on detach).  After having used
tracing for a while (granted, not really in the field), I believe
that this "do you want to stop tracing" query will get old soon.  But,
I can see that I can't manage to convince you of anything, so
please, let's not get stuck on this issue, and let's move forward
with whatever you think is right.

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

Pedro Alves wrote:
> On Wednesday 07 April 2010 23:04:38, Stan Shebs wrote:
>   
>> Pedro Alves wrote:
>>
>>     
>>> I suggest we change that to simply:
>>>
>>>   /* If running interactively, warn the user a trace run is ongoing.
>>>      She may want to cancel detaching instead.  */
>>>     if (current_trace_status ()->running && from_tty)
>>>       {
>>>         if (current_trace_status ()->disconnected_tracing)
>>>           {
>>>             if (!query (_("Trace is running and will continue after detach; detach anyway? ")))
>>>               error (_("Not confirmed."));
>>>           }
>>>         else
>>>           {
>>>             if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
>>>              error (_("Not confirmed."));
>>>           }
>>>       }
>>>
>>>   - simpler, more coherent, less explaining, less confusing.
>>>   
>>>       
>> The downside of this design is that if you did want to shut tracing 
>> down, you have to cancel the detach, do a tstop, then redo the detach.  
>> It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
>> the power to choose whether to keep the trace running, but not to 
>> exercise it, and to insist that you have cancel and type the command 
>> yourself.  Perhaps the crux of the confusion is that this is really a 
>> three-way choice - trace/detach, tstop/detach, cancel - and a pair of 
>> yes/no questions is not a good way to model it.
>>     
>
> That's just begging for:
>
> The downside of your current design is that if you did want to detach
> and leave tracing running, you have to cancel the detach, do a "set
> disconnected-tracing on", then redo the detach.
> It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
> the power to choose whether to keep the trace running, but not to 
> exercise it, and to insist that you have cancel and type the command 
> yourself.
>
> :-)
>   

Um, I must be missing the joke, it looks like you cut-n-pasted verbatim?

> I'd say that this use case would be much more common, and if
> gdb gave you the option of turning disconnected tracing _on_
> would be more helpful than the other way around (having trace
> running and wanting it to stop on detach).  After having used
> tracing for a while (granted, not really in the field), I believe
> that this "do you want to stop tracing" query will get old soon.  But,
> I can see that I can't manage to convince you of anything, so
> please, let's not get stuck on this issue, and let's move forward
> with whatever you think is right.
>   

My mental model is that users will tend to have disconnected tracing on 
by default if possible, because it ensures that the tracing run isn't 
prematurely terminated by GDB crash or loss of network connection.  But 
yeah, without much user feedback yet, we are just speculating on what 
users will prefer.  So I'll check in my version, and if people complain 
about it, then you'll get that delicious feeling of vindication. :-)

Stan

Re: tracing broken if target doesn't do disconnected tracing

[Pedro Alves]

On Thursday 08 April 2010 19:18:40, Stan Shebs wrote:
> >> The downside of this design is that if you did want to shut tracing 
> >> down, you have to cancel the detach, do a tstop, then redo the detach.  
> >> It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
> >> the power to choose whether to keep the trace running, but not to 
> >> exercise it, and to insist that you have cancel and type the command 
> >> yourself.  Perhaps the crux of the confusion is that this is really a 
> >> three-way choice - trace/detach, tstop/detach, cancel - and a pair of 
> >> yes/no questions is not a good way to model it.
> >>     
> >
> > That's just begging for:
> >
> > The downside of your current design is that if you did want to detach
> > and leave tracing running, you have to cancel the detach, do a "set
> > disconnected-tracing on", then redo the detach.
> > It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
> > the power to choose whether to keep the trace running, but not to 
> > exercise it, and to insist that you have cancel and type the command 
> > yourself.
> >
> > :-)
> >   
> 
> Um, I must be missing the joke, it looks like you cut-n-pasted verbatim?

Your patch only asks the user whether she wants to leave tracing on or
stop it when "set disconnected-tracing" was on.  If it was "off" (the
user forgot to set it on, as it is off by default), you still have to
cancel the detach and issue a "set disconnected-tracing on" command.
The joke was that your words only justify one of the branches in your code,
and can be used to justify my point in the other branch.  Compare the
first sentences.  As is, your patch doesn't expose GDB's power to
turn disconnected-tracing _on_, only _off_.  Following your mental model,
a user would be much more thankful if gdb offered the ability to turn
it on, on the spot.

-- 
Pedro Alves

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

Pedro Alves wrote:
> On Thursday 08 April 2010 19:18:40, Stan Shebs wrote:
>   
>>>> The downside of this design is that if you did want to shut tracing 
>>>> down, you have to cancel the detach, do a tstop, then redo the detach.  
>>>> It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
>>>> the power to choose whether to keep the trace running, but not to 
>>>> exercise it, and to insist that you have cancel and type the command 
>>>> yourself.  Perhaps the crux of the confusion is that this is really a 
>>>> three-way choice - trace/detach, tstop/detach, cancel - and a pair of 
>>>> yes/no questions is not a good way to model it.
>>>>     
>>>>         
>>> That's just begging for:
>>>
>>> The downside of your current design is that if you did want to detach
>>> and leave tracing running, you have to cancel the detach, do a "set
>>> disconnected-tracing on", then redo the detach.
>>> It's not crucial perhaps, but it seems a bit pedantic for GDB to have 
>>> the power to choose whether to keep the trace running, but not to 
>>> exercise it, and to insist that you have cancel and type the command 
>>> yourself.
>>>
>>> :-)
>>>   
>>>       
>> Um, I must be missing the joke, it looks like you cut-n-pasted verbatim?
>>     
>
> Your patch only asks the user whether she wants to leave tracing on or
> stop it when "set disconnected-tracing" was on.  If it was "off" (the
> user forgot to set it on, as it is off by default), you still have to
> cancel the detach and issue a "set disconnected-tracing on" command.
> The joke was that your words only justify one of the branches in your code,
> and can be used to justify my point in the other branch.  Compare the
> first sentences.  As is, your patch doesn't expose GDB's power to
> turn disconnected-tracing _on_, only _off_.  Following your mental model,
> a user would be much more thankful if gdb offered the ability to turn
> it on, on the spot.
>
>   

Ok, I'm with you now.  And you're right, they shouldn't be asymmetrical.

Stan

Re: tracing broken if target doesn't do disconnected tracing

[Stan Shebs]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

Ironically, after all that, I ended up going with Pedro's pair of cancel 
queries - there was no way to do nested query() calls so that answering 
two questions would be the exception rather than the rule, and a 
three-answer query seemed like it was going to be a mouthful to 
explain.  So here's what I ended up committing.

Stan

2010-04-08  Stan Shebs  <stan@codesourcery.com>
        Pedro Alves  <pedro@codesourcery.com>

    * tracepoint.h (struct trace_status): New fields disconnected_tracing
    and circular_buffer.
    (disconnect_tracing): Rename from disconnect_or_stop_tracing.
    * tracepoint.c (trace_status_command): Display target's status for
    disconnected tracing and circular buffer.
    (disconnect_tracing): Rename from disconnect_or_stop_tracing, add
    query for non-disconnected-tracing case, remove the stop_tracing
    call.
    (tfile_open): Clear disconnected and circular buffer status.
    (trace_save): Save disconnected and circular buffer status.
    (parse_trace_status): Parse disconnected and circular buffer status,
    also recognize disconnected as a stop reason.
    * remote.c (remote_set_disconnected_tracing): Only set
    QTDisconnected if the remote end supports disconnected tracing.
    Warn otherwise, if trying to enable disconnected tracing.
    * infcmd.c (detach_command): Update disconnect_tracing call.
    * cli/cli-cmds.c (quit_command): Ditto.

    * gdb.texinfo (Tracepoint Packets): Describe disconn and circular
    trace status fields.




[-- Attachment #2: disconn-patch-2 --]
[-- Type: text/plain, Size: 11814 bytes --]

Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.264
diff -p -r1.264 infcmd.c
*** infcmd.c	4 Apr 2010 22:12:04 -0000	1.264
--- infcmd.c	9 Apr 2010 02:59:41 -0000
*************** detach_command (char *args, int from_tty
*** 2547,2553 ****
    if (ptid_equal (inferior_ptid, null_ptid))
      error (_("The program is not being run."));
  
!   disconnect_or_stop_tracing (from_tty);
  
    target_detach (args, from_tty);
  
--- 2547,2553 ----
    if (ptid_equal (inferior_ptid, null_ptid))
      error (_("The program is not being run."));
  
!   disconnect_tracing (from_tty);
  
    target_detach (args, from_tty);
  
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.399
diff -p -r1.399 remote.c
*** remote.c	2 Apr 2010 01:18:34 -0000	1.399
--- remote.c	9 Apr 2010 02:59:41 -0000
*************** remote_set_disconnected_tracing (int val
*** 9776,9786 ****
  {
    struct remote_state *rs = get_remote_state ();
  
!   sprintf (rs->buf, "QTDisconnected:%x", val);
!   putpkt (rs->buf);
!   remote_get_noisy_reply (&target_buf, &target_buf_size);
!   if (strcmp (target_buf, "OK"))
!     error (_("Target does not support this command."));
  }
  
  static int
--- 9776,9791 ----
  {
    struct remote_state *rs = get_remote_state ();
  
!   if (rs->disconnected_tracing)
!     {
!       sprintf (rs->buf, "QTDisconnected:%x", val);
!       putpkt (rs->buf);
!       remote_get_noisy_reply (&target_buf, &target_buf_size);
!       if (strcmp (target_buf, "OK"))
! 	error (_("Target does not support this command."));
!     }
!   else if (val)
!     warning (_("Target does not support disconnected tracing."));
  }
  
  static int
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.173
diff -p -r1.173 tracepoint.c
*** tracepoint.c	6 Apr 2010 17:47:40 -0000	1.173
--- tracepoint.c	9 Apr 2010 02:59:41 -0000
*************** trace_status_command (char *args, int fr
*** 1626,1635 ****
    else if (ts->running)
      {
        printf_filtered (_("Trace is running on the target.\n"));
-       if (disconnected_tracing)
- 	printf_filtered (_("Trace will continue if GDB disconnects.\n"));
-       else
- 	printf_filtered (_("Trace will stop if GDB disconnects.\n"));
      }
    else
      {
--- 1626,1631 ----
*************** trace_status_command (char *args, int fr
*** 1699,1704 ****
--- 1695,1708 ----
  			 ts->buffer_free);
      }
  
+   if (ts->disconnected_tracing)
+     printf_filtered (_("Trace will continue if GDB disconnects.\n"));
+   else
+     printf_filtered (_("Trace will stop if GDB disconnects.\n"));
+ 
+   if (ts->circular_buffer)
+     printf_filtered (_("Trace buffer is circular.\n"));
+ 
    /* Now report on what we're doing with tfind.  */
    if (traceframe_number >= 0)
      printf_filtered (_("Looking at trace frame %d, tracepoint %d.\n"),
*************** trace_status_mi (int on_stop)
*** 1801,1809 ****
      ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
  }
  
! 
  void
! disconnect_or_stop_tracing (int from_tty)
  {
    /* It can happen that the target that was tracing went away on its
       own, and we didn't notice.  Get a status update, and if the
--- 1805,1815 ----
      ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
  }
  
! /* This function handles the details of what to do about an ongoing
!    tracing run if the user has asked to detach or otherwise disconnect
!    from the target.  */
  void
! disconnect_tracing (int from_tty)
  {
    /* It can happen that the target that was tracing went away on its
       own, and we didn't notice.  Get a status update, and if the
*************** disconnect_or_stop_tracing (int from_tty
*** 1812,1829 ****
    if (target_get_trace_status (current_trace_status ()) < 0)
      current_trace_status ()->running = 0;
  
    if (current_trace_status ()->running && from_tty)
      {
!       int cont = query (_("Trace is running.  Continue tracing after detach? "));
!       /* Note that we send the query result without affecting the
! 	 user's setting of disconnected_tracing, so that the answer is
! 	 a one-time-only.  */
!       send_disconnected_tracing_value (cont);
! 
!       /* Also ensure that we do the equivalent of a tstop command if
! 	 tracing is not to continue after the detach.  */
!       if (!cont)
! 	stop_tracing ();
      }
  
    /* Also we want to be out of tfind mode, otherwise things can get
--- 1818,1840 ----
    if (target_get_trace_status (current_trace_status ()) < 0)
      current_trace_status ()->running = 0;
  
+   /* If running interactively, give the user the option to cancel and
+      then decide what to do differently with the run.  Scripts are
+      just going to disconnect and let the target deal with it,
+      according to how it's been instructed previously via
+      disconnected-tracing.  */
    if (current_trace_status ()->running && from_tty)
      {
!       if (current_trace_status ()->disconnected_tracing)
! 	{
! 	  if (!query (_("Trace is running and will continue after detach; detach anyway? ")))
! 	    error (_("Not confirmed."));
! 	}
!       else
! 	{
! 	  if (!query (_("Trace is running but will stop on detach; detach anyway? ")))
! 	    error (_("Not confirmed."));
! 	}
      }
  
    /* Also we want to be out of tfind mode, otherwise things can get
*************** trace_save (const char *filename, int ta
*** 2599,2604 ****
--- 2610,2619 ----
      fprintf (fp, ";tfree:%x", ts->buffer_free);
    if (ts->buffer_size >= 0)
      fprintf (fp, ";tsize:%x", ts->buffer_size);
+   if (ts->disconnected_tracing)
+     fprintf (fp, ";disconn:%x", ts->disconnected_tracing);
+   if (ts->circular_buffer)
+     fprintf (fp, ";circular:%x", ts->circular_buffer);
    fprintf (fp, "\n");
  
    /* Note that we want to upload tracepoints and save those, rather
*************** tfile_open (char *filename, int from_tty
*** 3167,3172 ****
--- 3182,3189 ----
    ts->stop_reason = trace_stop_reason_unknown;
    ts->traceframe_count = -1;
    ts->buffer_free = 0;
+   ts->disconnected_tracing = 0;
+   ts->circular_buffer = 0;
  
    /* Read through a section of newline-terminated lines that
       define things like tracepoints.  */
*************** parse_trace_status (char *line, struct t
*** 3282,3287 ****
--- 3299,3306 ----
    ts->traceframes_created = -1;
    ts->buffer_free = -1;
    ts->buffer_size = -1;
+   ts->disconnected_tracing = 0;
+   ts->circular_buffer = 0;
  
    while (*p++)
      {
*************** Status line: '%s'\n"), p, line);
*** 3310,3315 ****
--- 3329,3339 ----
  	  p = unpack_varlen_hex (++p1, &val);
  	  ts->stop_reason = tstop_command;
  	}
+       else if (strncmp (p, stop_reason_names[trace_disconnected], p1 - p) == 0)
+ 	{
+ 	  p = unpack_varlen_hex (++p1, &val);
+ 	  ts->stop_reason = trace_disconnected;
+ 	}
        else if (strncmp (p, stop_reason_names[tracepoint_error], p1 - p) == 0)
  	{
  	  p2 = strchr (++p1, ':');
*************** Status line: '%s'\n"), p, line);
*** 3348,3353 ****
--- 3372,3387 ----
  	  p = unpack_varlen_hex (++p1, &val);
  	  ts->buffer_size = val;
  	}
+       else if (strncmp (p, "disconn", p1 - p) == 0)
+ 	{
+ 	  p = unpack_varlen_hex (++p1, &val);
+ 	  ts->disconnected_tracing = val;
+ 	}
+       else if (strncmp (p, "circular", p1 - p) == 0)
+ 	{
+ 	  p = unpack_varlen_hex (++p1, &val);
+ 	  ts->circular_buffer = val;
+ 	}
        else
  	{
  	  /* Silently skip unknown optional info.  */
Index: tracepoint.h
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.h,v
retrieving revision 1.34
diff -p -r1.34 tracepoint.h
*** tracepoint.h	6 Apr 2010 17:47:40 -0000	1.34
--- tracepoint.h	9 Apr 2010 02:59:41 -0000
*************** struct trace_status
*** 106,111 ****
--- 106,121 ----
    /* Unused bytes left in the target's trace buffer.  */
  
    int buffer_free;
+ 
+   /* 1 if the target will continue tracing after disconnection, else
+      0.  If the target does not report a value, assume 0.  */
+ 
+   int disconnected_tracing;
+ 
+   /* 1 if the target is using a circular trace buffer, else 0.  If the
+      target does not report a value, assume 0.  */
+ 
+   int circular_buffer;
  };
  
  struct trace_status *current_trace_status (void);
*************** extern struct breakpoint *create_tracepo
*** 189,195 ****
  extern void merge_uploaded_tracepoints (struct uploaded_tp **utpp);
  extern void merge_uploaded_trace_state_variables (struct uploaded_tsv **utsvp);
  
! extern void disconnect_or_stop_tracing (int from_tty);
  
  extern void start_tracing (void);
  extern void stop_tracing (void);
--- 199,205 ----
  extern void merge_uploaded_tracepoints (struct uploaded_tp **utpp);
  extern void merge_uploaded_trace_state_variables (struct uploaded_tsv **utsvp);
  
! extern void disconnect_tracing (int from_tty);
  
  extern void start_tracing (void);
  extern void stop_tracing (void);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.698
diff -p -r1.698 gdb.texinfo
*** doc/gdb.texinfo	8 Apr 2010 22:32:37 -0000	1.698
--- doc/gdb.texinfo	9 Apr 2010 02:59:42 -0000
*************** The trace stopped for some other reason.
*** 31508,31517 ****
  
  @end table
  
! Additional optional fields supply statistical information.  Although
! not required, they are extremely useful for users monitoring the
! progress of a trace run.  If a trace has stopped, and these numbers
! are reported, they must reflect the state of the just-stopped trace.
  
  @table @samp
  
--- 31508,31518 ----
  
  @end table
  
! Additional optional fields supply statistical and other information.
! Although not required, they are extremely useful for users monitoring
! the progress of a trace run.  If a trace has stopped, and these
! numbers are reported, they must reflect the state of the just-stopped
! trace.
  
  @table @samp
  
*************** The total size of the trace buffer, in b
*** 31528,31533 ****
--- 31529,31545 ----
  @item tfree:@var{n}
  The number of bytes still unused in the buffer.
  
+ @item circular:@var{n}
+ The value of the circular trace buffer flag.  @code{1} means that the
+ trace buffer is circular and old trace frames will be discarded if
+ necessary to make room, @code{0} means that the trace buffer is linear
+ and may fill up.
+ 
+ @item disconn:@var{n}
+ The value of the disconnected tracing flag.  @code{1} means that
+ tracing will continue after @value{GDBN} disconnects, @code{0} means
+ that the trace run will stop.
+ 
  @end table
  
  @item qTV:@var{var}
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.99
diff -p -r1.99 cli-cmds.c
*** cli/cli-cmds.c	7 Apr 2010 03:42:55 -0000	1.99
--- cli/cli-cmds.c	9 Apr 2010 02:59:42 -0000
***************
*** 38,44 ****
  #include "objfiles.h"
  #include "source.h"
  #include "disasm.h"
! extern void disconnect_or_stop_tracing (int from_tty);
  
  #include "ui-out.h"
  
--- 38,44 ----
  #include "objfiles.h"
  #include "source.h"
  #include "disasm.h"
! #include "tracepoint.h"
  
  #include "ui-out.h"
  
*************** quit_command (char *args, int from_tty)
*** 337,343 ****
    if (!quit_confirm ())
      error (_("Not confirmed."));
  
!   disconnect_or_stop_tracing (from_tty);
  
    quit_force (args, from_tty);
  }
--- 337,343 ----
    if (!quit_confirm ())
      error (_("Not confirmed."));
  
!   disconnect_tracing (from_tty);
  
    quit_force (args, from_tty);
  }