[PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Kwok Cheung Yeung]

Tracepoints currently cannot be enabled or disabled while a trace experiment is
running (or rather, you can, but it does not take effect until the next tstart
command, which would discard all the data collected so far). This ability could
be useful, for example, for long running experiments where a tracepoint is
discovered to be unneeded after beginning the experiment and can be disabled to
avoid wasting trace buffer space.

This patch adds support for enabling or disabling tracepoints while an
experiment is running, by sending a new remote command to the gdbserver whenever
this happens. The gdbserver will then set the enabled flag on the tracepoint
accordingly.

Kwok Cheung Yeung

	gdb/
	* breakpoint.c (disable_breakpoint): Disable all locations
  	associated with a tracepoint on target if a trace experiment is
    	running.
 	(disable_command): Disable a specific tracepoint location on target if
 	a trace	experiment is running.
 	(do_enable_breakpoint): Enable all locations associated with a
 	tracepoint on target if a trace experiment is running.
 	(enable_command) Enable a specific tracepoint location on target if a
 	trace experiment is running.
 	* target.c (update_current_target): Add INHERIT and de_fault clauses for
 	to_enable_tracepoint and to_disable_tracepoint.
 	* target.h: Add declaration of struct bp_location.
 	(struct target_ops): Add new function to_enable_tracepoint and
 	to_disable_tracepoint to target operations.
 	(target_enable_tracepoint): New macro.
 	(target_disable_tracepoint): New macro.
 	* remote.c (remote_enable_tracepoint): New.
 	(remote_disable_tracepoint): New.
 	(init_remote_ops): Add remote_enable_tracepoint and
 	remote_disable_tracepoint to remote operations.
 	* tracepoint.c (start_tracing): Allow tracing to start without any
 	tracepoints enabled with just a warning, since they can now be
 	re-enabled.
 	* NEWS: Add news item for the new behaviour of the enable and disable
 	GDB commands when applied to tracepoints.
 	Add news items for the new remote packets QTEnable and QTDisable.
 	* gdbserver/tracepoint.c (clear_installed_tracepoints): Uninstall
 	disabled tracepoints.
 	(cmd_qtenable_disable): New.
 	(cmd_qtstart): Install tracepoints even if disabled.
 	(handle_tracepoint_general_set): Add call to cmd_qtenable_disable on
 	receiving a QTEnable or QTDisable packet.
 	(gdb_collect): Skip data collection if fast tracepoint is disabled.
 	(ust_marker_to_static_tracepoint): Do not ignore disabled static
 	tracepoints.
 	(gdb_probe): Skip data collection if static tracepoint is disabled.

	gdb/doc/
	* gdb.texinfo: Document change in the behaviour of the enable and
	disable GDB commands when applied to tracepoints.
	Document QTEnable and QTDisable in the list of tracepoint packets.

Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.436
diff -u -p -r1.436 NEWS
--- gdb/NEWS	24 Apr 2011 08:02:18 -0000	1.436
+++ gdb/NEWS	5 May 2011 17:09:54 -0000
@@ -42,6 +42,22 @@
   Initial support for the OpenCL C language (http://www.khronos.org/opencl)
   has been integrated into GDB.

+* Tracepoints can now be enabled and disabled at any time after a trace
+  experiment has been started using the standard "enable" and "disable"
+  commands.  If a trace experiment is started with no enabled
+  tracepoints, then a warning will be printed but the experiment will
+  proceed anyway.
+
+* New remote packets
+
+QTEnable
+
+  Dynamically enable a tracepoint in a started trace experiment.
+
+QTDisable
+
+  Dynamically disable a tracepoint in a started trace experiment.
+
 * Python scripting

   ** The function gdb.Write now accepts an optional keyword 'stream'.
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.573
diff -u -p -r1.573 breakpoint.c
--- gdb/breakpoint.c	3 May 2011 07:29:17 -0000	1.573
+++ gdb/breakpoint.c	5 May 2011 17:09:57 -0000
@@ -11412,6 +11412,14 @@ disable_breakpoint (struct breakpoint *b

   bpt->enable_state = bp_disabled;

+  if (current_trace_status ()->running && is_tracepoint (bpt))
+    {
+      struct bp_location *location;
+
+      for (location = bpt->loc; location; location = location->next)
+	target_disable_tracepoint (location);
+    }
+
   update_global_location_list (0);

   observer_notify_breakpoint_modified (bpt);
@@ -11441,7 +11449,12 @@ disable_command (char *args, int from_tt
     {
       struct bp_location *loc = find_location_by_number (args);
       if (loc)
-	loc->enabled = 0;
+	{
+	  loc->enabled = 0;
+	  if (current_trace_status ()->running && loc->owner
+	      && is_tracepoint (loc->owner))
+	    target_disable_tracepoint (loc);
+	}
       update_global_location_list (0);
     }
   else
@@ -11489,6 +11502,15 @@ do_enable_breakpoint (struct breakpoint

   if (bpt->enable_state != bp_permanent)
     bpt->enable_state = bp_enabled;
+
+  if (current_trace_status ()->running && is_tracepoint (bpt))
+    {
+      struct bp_location *location;
+
+      for (location = bpt->loc; location; location = location->next)
+	target_enable_tracepoint (location);
+    }
+
   bpt->disposition = disposition;
   update_global_location_list (1);
   breakpoints_changed ();
@@ -11531,7 +11553,12 @@ enable_command (char *args, int from_tty
     {
       struct bp_location *loc = find_location_by_number (args);
       if (loc)
-	loc->enabled = 1;
+	{
+	  loc->enabled = 1;
+	  if (current_trace_status ()->running && loc->owner
+	      && is_tracepoint (loc->owner))
+	    target_enable_tracepoint (loc);
+	}
       update_global_location_list (1);
     }
   else
Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.445
diff -u -p -r1.445 remote.c
--- gdb/remote.c	27 Apr 2011 13:29:14 -0000	1.445
+++ gdb/remote.c	5 May 2011 17:10:00 -0000
@@ -9919,6 +9919,38 @@ remote_download_trace_state_variable (st
 }

 static void
+remote_enable_tracepoint (struct bp_location *location)
+{
+  struct remote_state *rs = get_remote_state ();
+  char addr_buf[40];
+
+  sprintf_vma (addr_buf, location->address);
+  sprintf (rs->buf, "QTEnable:%x:%s", location->owner->number, addr_buf);
+  putpkt (rs->buf);
+  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+  if (*rs->buf == '\0')
+    error (_("Target does not support enabling tracepoints while a trace run is
ongoing."));
+  if (strcmp (rs->buf, "OK") != 0)
+    error (_("Error on target while enabling tracepoint."));
+}
+
+static void
+remote_disable_tracepoint (struct bp_location *location)
+{
+  struct remote_state *rs = get_remote_state ();
+  char addr_buf[40];
+
+  sprintf_vma (addr_buf, location->address);
+  sprintf (rs->buf, "QTDisable:%x:%s", location->owner->number, addr_buf);
+  putpkt (rs->buf);
+  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+  if (*rs->buf == '\0')
+    error (_("Target does not support disabling tracepoints while a trace run
is ongoing."));
+  if (strcmp (rs->buf, "OK") != 0)
+    error (_("Error on target while disabling tracepoint."));
+}
+
+static void
 remote_trace_set_readonly_regions (void)
 {
   asection *s;
@@ -10312,6 +10344,8 @@ Specify the serial device it is connecte
   remote_ops.to_download_tracepoint = remote_download_tracepoint;
   remote_ops.to_download_trace_state_variable
     = remote_download_trace_state_variable;
+  remote_ops.to_enable_tracepoint = remote_enable_tracepoint;
+  remote_ops.to_disable_tracepoint = remote_disable_tracepoint;
   remote_ops.to_trace_set_readonly_regions = remote_trace_set_readonly_regions;
   remote_ops.to_trace_start = remote_trace_start;
   remote_ops.to_get_trace_status = remote_get_trace_status;
Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.281
diff -u -p -r1.281 target.c
--- gdb/target.c	27 Apr 2011 13:29:15 -0000	1.281
+++ gdb/target.c	5 May 2011 17:10:00 -0000
@@ -659,6 +659,8 @@ update_current_target (void)
       INHERIT (to_trace_init, t);
       INHERIT (to_download_tracepoint, t);
       INHERIT (to_download_trace_state_variable, t);
+      INHERIT (to_enable_tracepoint, t);
+      INHERIT (to_disable_tracepoint, t);
       INHERIT (to_trace_set_readonly_regions, t);
       INHERIT (to_trace_start, t);
       INHERIT (to_get_trace_status, t);
@@ -831,6 +833,12 @@ update_current_target (void)
   de_fault (to_download_trace_state_variable,
 	    (void (*) (struct trace_state_variable *))
 	    tcomplain);
+  de_fault (to_enable_tracepoint,
+	    (void (*) (struct bp_location *))
+	    tcomplain);
+  de_fault (to_disable_tracepoint,
+	    (void (*) (struct bp_location *))
+	    tcomplain);
   de_fault (to_trace_set_readonly_regions,
 	    (void (*) (void))
 	    tcomplain);
Index: gdb/target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.204
diff -u -p -r1.204 target.h
--- gdb/target.h	27 Apr 2011 13:29:15 -0000	1.204
+++ gdb/target.h	5 May 2011 17:10:01 -0000
@@ -28,6 +28,7 @@ struct objfile;
 struct ui_file;
 struct mem_attrib;
 struct target_ops;
+struct bp_location;
 struct bp_target_info;
 struct regcache;
 struct target_section_table;
@@ -668,6 +669,12 @@ struct target_ops
     /* Send full details of a trace state variable to the target.  */
     void (*to_download_trace_state_variable) (struct trace_state_variable *tsv);

+    /* Enable a tracepoint on the target.  */
+    void (*to_enable_tracepoint) (struct bp_location *location);
+
+    /* Disable a tracepoint on the target.  */
+    void (*to_disable_tracepoint) (struct bp_location *location);
+
     /* Inform the target info of memory regions that are readonly
        (such as text sections), and so it should return data from
        those rather than look in the trace buffer.  */
@@ -1431,6 +1438,12 @@ extern int target_search_memory (CORE_AD
 #define target_download_trace_state_variable(tsv) \
   (*current_target.to_download_trace_state_variable) (tsv)

+#define target_enable_tracepoint(loc) \
+  (*current_target.to_enable_tracepoint) (loc)
+
+#define target_disable_tracepoint(loc) \
+  (*current_target.to_disable_tracepoint) (loc)
+
 #define target_trace_start() \
   (*current_target.to_trace_start) ()

Index: gdb/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.222
diff -u -p -r1.222 tracepoint.c
--- gdb/tracepoint.c	19 Apr 2011 18:04:07 -0000	1.222
+++ gdb/tracepoint.c	5 May 2011 17:10:02 -0000
@@ -1583,12 +1583,9 @@ start_tracing (void)
 		 (t->type == bp_fast_tracepoint ? "fast " : ""), t->number);
     }

-  /* No point in tracing with only disabled tracepoints.  */
+  /* Warn if tracing with only disabled tracepoints.  */
   if (!any_enabled)
-    {
-      VEC_free (breakpoint_p, tp_vec);
-      error (_("No tracepoints enabled, not starting trace"));
-    }
+    warning (_("No tracepoints enabled"));

   if (num_to_download <= 0)
     {
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.829
diff -u -p -r1.829 gdb.texinfo
--- gdb/doc/gdb.texinfo	3 May 2011 15:18:10 -0000	1.829
+++ gdb/doc/gdb.texinfo	5 May 2011 17:10:13 -0000
@@ -10008,15 +10008,14 @@ These commands are deprecated; they are
 @kindex disable tracepoint
 @item disable tracepoint @r{[}@var{num}@r{]}
 Disable tracepoint @var{num}, or all tracepoints if no argument
-@var{num} is given.  A disabled tracepoint will have no effect during
-the next trace experiment, but it is not forgotten.  You can re-enable
-a disabled tracepoint using the @code{enable tracepoint} command.
+@var{num} is given.  The change is effective immediately.  You
+can re-enable a disabled tracepoint using the @code{enable tracepoint}
+command.

 @kindex enable tracepoint
 @item enable tracepoint @r{[}@var{num}@r{]}
 Enable tracepoint @var{num}, or all tracepoints.  The enabled
-tracepoints will become effective the next time a trace experiment is
-run.
+tracepoints will become effective immediately.
 @end table

 @node Tracepoint Passcounts
@@ -33794,6 +33793,8 @@ packets.)
 @item qTsV
 @itemx QTStart
 @itemx QTStop
+@itemx QTEnable
+@itemx QTDisable
 @itemx QTinit
 @itemx QTro
 @itemx qTStatus
@@ -34283,6 +34284,16 @@ instruction reply packet}).
 @item QTStop
 End the tracepoint experiment.  Stop collecting trace frames.

+@item QTEnable:@var{n}:@var{addr}
+Enable tracepoint @var{n} at address @var{addr} in a started tracepoint
+experiment.  If the tracepoint was previously disabled, then collection
+of data from it will resume.
+
+@item QTDisable:@var{n}:@var{addr}
+Disable tracepoint @var{n} at address @var{addr} in a started tracepoint
+experiment.  No more data will be collected from the tracepoint unless
+@samp{QTEnable:@var{n}:@var{addr}} is subsequently issued.
+
 @item QTinit
 Clear the table of tracepoints, and empty the trace frame buffer.

Index: gdb/gdbserver/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/tracepoint.c,v
retrieving revision 1.24
diff -u -p -r1.24 tracepoint.c
--- gdb/gdbserver/tracepoint.c	10 Mar 2011 20:21:14 -0000	1.24
+++ gdb/gdbserver/tracepoint.c	5 May 2011 17:10:14 -0000
@@ -2214,9 +2214,6 @@ clear_installed_tracepoints (void)
   /* Restore any bytes overwritten by tracepoints.  */
   for (tpoint = tracepoints; tpoint; tpoint = tpoint->next)
     {
-      if (!tpoint->enabled)
-	continue;
-
       /* Catch the case where we might try to remove a tracepoint that
 	 was never actually installed.  */
       if (tpoint->handle == NULL)
@@ -2455,6 +2452,73 @@ cmd_qtdv (char *own_buf)
 }

 static void
+cmd_qtenable_disable (char *own_buf, int enable)
+{
+  char *packet = own_buf;
+  ULONGEST num, addr;
+  struct tracepoint *tp;
+
+  packet += strlen (enable ? "QTEnable:" : "QTDisable:");
+  packet = unpack_varlen_hex (packet, &num);
+  ++packet; /* skip a colon */
+  packet = unpack_varlen_hex (packet, &addr);
+
+  tp = find_tracepoint (num, addr);
+
+  if (tp)
+    {
+      if ((enable && tp->enabled) || (!enable && !tp->enabled))
+	{
+	  trace_debug ("Tracepoint %d at 0x%s is already %s",
+		       (int) num, paddress (addr),
+		       enable ? "enabled" : "disabled");
+	  write_ok (own_buf);
+	  return;
+	}
+
+      trace_debug ("%s tracepoint %d at 0x%s",
+		   enable ? "Enabling" : "Disabling",
+		   (int) num, paddress (addr));
+
+      tp->enabled = enable;
+
+      if (tp->type == fast_tracepoint || tp->type == static_tracepoint)
+	{
+	  int ret;
+	  int offset = offsetof (struct tracepoint, enabled);
+	  CORE_ADDR obj_addr = tp->obj_addr_on_target + offset;
+
+	  ret = prepare_to_access_memory ();
+	  if (ret)
+	    {
+	      trace_debug ("Failed to temporarily stop inferior threads");
+	      write_enn (own_buf);
+	      return;
+	    }
+	
+	  ret = write_inferior_integer (obj_addr, enable);
+	  done_accessing_memory ();
+	
+	  if (ret)
+	    {
+	      trace_debug ("Cannot write enabled flag into "
+			   "inferior process memory");
+	      write_enn (own_buf);
+	      return;
+	    }
+	}
+
+      write_ok (own_buf);
+    }
+  else
+    {
+      trace_debug ("Tracepoint %d at 0x%s not found",
+		   (int) num, paddress (addr));
+      write_enn (own_buf);
+    }
+}
+
+static void
 cmd_qtv (char *own_buf)
 {
   ULONGEST num;
@@ -2719,9 +2783,6 @@ cmd_qtstart (char *packet)
       /* Ensure all the hit counts start at zero.  */
       tpoint->hit_count = 0;

-      if (!tpoint->enabled)
-	continue;
-
       if (tpoint->type == trap_tracepoint)
 	{
 	  ++slow_tracepoint_count;
@@ -3461,6 +3522,16 @@ handle_tracepoint_general_set (char *pac
       cmd_qtdpsrc (packet);
       return 1;
     }
+  else if (strncmp ("QTEnable:", packet, strlen ("QTEnable:")) == 0)
+    {
+      cmd_qtenable_disable (packet, 1);
+      return 1;
+    }
+  else if (strncmp ("QTDisable:", packet, strlen ("QTDisable:")) == 0)
+    {
+      cmd_qtenable_disable (packet, 0);
+      return 1;
+    }
   else if (strncmp ("QTDV:", packet, strlen ("QTDV:")) == 0)
     {
       cmd_qtdv (packet);
@@ -5340,6 +5411,9 @@ gdb_collect (struct tracepoint *tpoint,
   if (!tracing)
     return;

+  if (!tpoint->enabled)
+    return;
+
   ctx.base.type = fast_tracepoint;
   ctx.regs = regs;
   ctx.regcache_initted = 0;
@@ -6598,7 +6672,7 @@ ust_marker_to_static_tracepoint (const s

   for (tpoint = tracepoints; tpoint; tpoint = tpoint->next)
     {
-      if (!tpoint->enabled || tpoint->type != static_tracepoint)
+      if (tpoint->type != static_tracepoint)
 	continue;

       if (tpoint->address == (uintptr_t) mdata->location)
@@ -6653,6 +6727,12 @@ gdb_probe (const struct marker *mdata, v
       return;
     }

+  if (!tpoint->enabled)
+    {
+      trace_debug ("gdb_probe: tracepoint disabled");
+      return;
+    }
+
   ctx.tpoint = tpoint;

   trace_debug ("gdb_probe: collecting marker: "

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Pedro Alves]

On Thursday 05 May 2011 18:27:11, Kwok Cheung Yeung wrote:

>  	* gdbserver/tracepoint.c (clear_installed_tracepoints): Uninstall
>  	disabled tracepoints.

gdbserver has its own gdbserver/ChangeLog file:

  	* tracepoint.c (clear_installed_tracepoints): Uninstall
  	disabled tracepoints.


>  	* NEWS: Add news item for the new behaviour of the enable and disable
>  	GDB commands when applied to tracepoints.
>  	Add news items for the new remote packets QTEnable and QTDisable.

> 
> 	gdb/doc/
> 	* gdb.texinfo: Document change in the behaviour of the enable and
> 	disable GDB commands when applied to tracepoints.
> 	Document QTEnable and QTDisable in the list of tracepoint packets.

Eli is our documentation maintainer and will review these bits.

The code looks okay to me.

>  static void
> +cmd_qtenable_disable (char *own_buf, int enable)
> +{
> +  char *packet = own_buf;
> +  ULONGEST num, addr;
> +  struct tracepoint *tp;
> +
> +  packet += strlen (enable ? "QTEnable:" : "QTDisable:");
> +  packet = unpack_varlen_hex (packet, &num);
> +  ++packet; /* skip a colon */
> +  packet = unpack_varlen_hex (packet, &addr);
> +
> +  tp = find_tracepoint (num, addr);
> +
> +  if (tp)
> +    {
> +      if ((enable && tp->enabled) || (!enable && !tp->enabled))

but isn't this just:

 if (enable != tp->enabled)

?

> +	{
> +	  trace_debug ("Tracepoint %d at 0x%s is already %s",
> +		       (int) num, paddress (addr),
> +		       enable ? "enabled" : "disabled");
> +	  write_ok (own_buf);
> +	  return;
> +	}
> +

-- 
Pedro Alves

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Kwok Cheung Yeung]

On 05/05/2011 18:48, Pedro Alves wrote:
>> +      if ((enable && tp->enabled) || (!enable && !tp->enabled))
> 
> but isn't this just:
> 
>  if (enable != tp->enabled)
> 
> ?
> 

In this case it is, since enable and tp->enabled can only take the values 0 and
1. However, 'true' in C is 'non-zero' - 1 and ~0 are both non-zero and therefore
'true', but they aren't equal. The first version would get the boolean
comparison correct if there were multiple values of 'true' being used, whereas
the second would not.

Kwok

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Eli Zaretskii]

> Date: Thu, 05 May 2011 18:27:11 +0100
> From: Kwok Cheung Yeung <kcy@codesourcery.com>
> 
> +                     If a trace experiment is started with no enabled
> +  tracepoints, then a warning will be printed but the experiment will
> +  proceed anyway.

You mean, if the experiment started with no enabled tracepoints, and
the user says "disable", right?  Otherwise, why the warning?

> +* New remote packets
> +
> +QTEnable
> +
> +  Dynamically enable a tracepoint in a started trace experiment.
> +
> +QTDisable
> +
> +  Dynamically disable a tracepoint in a started trace experiment.

Can't the existing packets do the job?

>  Disable tracepoint @var{num}, or all tracepoints if no argument
> -@var{num} is given.  A disabled tracepoint will have no effect during
> -the next trace experiment, but it is not forgotten.  You can re-enable
> -a disabled tracepoint using the @code{enable tracepoint} command.
> +@var{num} is given.  The change is effective immediately.

Err... that's inaccurate, isn't it?  The code patch clearly shows that
sometimes it won't work:

> +  sprintf_vma (addr_buf, location->address);
> +  sprintf (rs->buf, "QTEnable:%x:%s", location->owner->number, addr_buf);
> +  putpkt (rs->buf);
> +  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
> +  if (*rs->buf == '\0')
> +    error (_("Target does not support enabling tracepoints while a trace run is ongoing."));
> +  if (strcmp (rs->buf, "OK") != 0)
> +    error (_("Error on target while enabling tracepoint."));

>  Enable tracepoint @var{num}, or all tracepoints.  The enabled
> -tracepoints will become effective the next time a trace experiment is
> -run.
> +tracepoints will become effective immediately.

Same here.

OK with these issues taken care of.

Thanks.

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Kwok Cheung Yeung]

On 05/05/2011 19:51, Eli Zaretskii wrote:
>> Date: Thu, 05 May 2011 18:27:11 +0100
>> From: Kwok Cheung Yeung <kcy@codesourcery.com>
>>
>> +                     If a trace experiment is started with no enabled
>> +  tracepoints, then a warning will be printed but the experiment will
>> +  proceed anyway.
> 
> You mean, if the experiment started with no enabled tracepoints, and
> the user says "disable", right?  Otherwise, why the warning?
>

Originally, it was an error to try to start an experiment with no enabled
tracepoints because the resulting experiment would never do anything. However,
with the patch, tracepoints can be re-enabled after the experiment starts, so I
downgraded the error to a warning. I suppose we could do away with it
altogether, but then a user who inadvertently disabled all tracepoints might sit
and wonder why nothing seems to be happening.

>> +* New remote packets
>> +
>> +QTEnable
>> +
>> +  Dynamically enable a tracepoint in a started trace experiment.
>> +
>> +QTDisable
>> +
>> +  Dynamically disable a tracepoint in a started trace experiment.
> 
> Can't the existing packets do the job?
> 

As far as I can see, there are no packets that allow a tracepoint that is
already created on the target to be modified. It might be possible to extend
QTDP: to do so, but that would seem to needlessly complicate its definition
(which is to create new tracepoints, not modify existing ones).

>>  Disable tracepoint @var{num}, or all tracepoints if no argument
>> -@var{num} is given.  A disabled tracepoint will have no effect during
>> -the next trace experiment, but it is not forgotten.  You can re-enable
>> -a disabled tracepoint using the @code{enable tracepoint} command.
>> +@var{num} is given.  The change is effective immediately.
> 
> Err... that's inaccurate, isn't it?  The code patch clearly shows that
> sometimes it won't work:
> 
>> +  sprintf_vma (addr_buf, location->address);
>> +  sprintf (rs->buf, "QTEnable:%x:%s", location->owner->number, addr_buf);
>> +  putpkt (rs->buf);
>> +  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
>> +  if (*rs->buf == '\0')
>> +    error (_("Target does not support enabling tracepoints while a trace run is ongoing."));
>> +  if (strcmp (rs->buf, "OK") != 0)
>> +    error (_("Error on target while enabling tracepoint."));
>

It won't work if the target does not have support for it (i.e. the target was
not compiled with this patch) or if an unexpected error occurs (in which case
all bets are off). Would something like this be better?

 Disable tracepoint @var{num}, or all tracepoints if no argument
 @var{num} is given.  A disabled tracepoint will have no effect during
-the next trace experiment, but it is not forgotten.  You can re-enable
+a trace experiment, but it is not forgotten.  You can re-enable
 a disabled tracepoint using the @code{enable tracepoint} command.
+If the command is issued during a trace experiment and the debug target
+has support for disabling tracepoints during a trace experiment, then the
+change will be effective immediately.  Otherwise, it will be applied to the
+next trace experiment.

-Enable tracepoint @var{num}, or all tracepoints.  The enabled
-tracepoints will become effective the next time a trace experiment is
-run.
+Enable tracepoint @var{num}, or all tracepoints.  If this command is
+issued during a trace experiment and the debug target supports enabling
+tracepoints during a trace experiment, then the enabled tracepoints will
+become effective immediately.  Otherwise, they will become effective the
+next time a trace experiment is run.

Kwok

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Stan Shebs]

On 5/5/11 4:13 PM, Kwok Cheung Yeung wrote:
> On 05/05/2011 19:51, Eli Zaretskii wrote:
>>> Date: Thu, 05 May 2011 18:27:11 +0100
>>> From: Kwok Cheung Yeung<kcy@codesourcery.com>
>>>
>>> +                     If a trace experiment is started with no enabled
>>> +  tracepoints, then a warning will be printed but the experiment will
>>> +  proceed anyway.
>> You mean, if the experiment started with no enabled tracepoints, and
>> the user says "disable", right?  Otherwise, why the warning?
>>
> Originally, it was an error to try to start an experiment with no enabled
> tracepoints because the resulting experiment would never do anything. However,
> with the patch, tracepoints can be re-enabled after the experiment starts, so I
> downgraded the error to a warning. I suppose we could do away with it
> altogether, but then a user who inadvertently disabled all tracepoints might sit
> and wonder why nothing seems to be happening.

Definitely need at least a warning - all too easy to disable everything, 
including all the tracepoints, with a plain "dis", then be well into the 
run before realizing that none of the tracepoints are getting hits recorded.

Of course, that does go against the Unix tradition of putting the 
snakepit in the middle of the living room floor... :-)

"Eh?"

Stan
stan@codesourcery.com

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Eli Zaretskii]

> Date: Fri, 06 May 2011 00:13:37 +0100
> From: Kwok Cheung Yeung <kcy@codesourcery.com>
> CC: gdb-patches@sourceware.org
> 
> On 05/05/2011 19:51, Eli Zaretskii wrote:
> >> Date: Thu, 05 May 2011 18:27:11 +0100
> >> From: Kwok Cheung Yeung <kcy@codesourcery.com>
> >>
> >> +                     If a trace experiment is started with no enabled
> >> +  tracepoints, then a warning will be printed but the experiment will
> >> +  proceed anyway.
> > 
> > You mean, if the experiment started with no enabled tracepoints, and
> > the user says "disable", right?  Otherwise, why the warning?
> >
> 
> Originally, it was an error to try to start an experiment with no enabled
> tracepoints because the resulting experiment would never do anything. However,
> with the patch, tracepoints can be re-enabled after the experiment starts, so I
> downgraded the error to a warning. I suppose we could do away with it
> altogether, but then a user who inadvertently disabled all tracepoints might sit
> and wonder why nothing seems to be happening.

In that case, the emphasis in this entry should be shifted slightly.
How about this variant:

  It is now possible to start a trace experiment with no enabled
  tracepoints; GDB will display a warning, but will allow the
  experiment to begin, assuming that tracepoints will be enabled as
  needed while the trace is running.

Btw, what if the target does not support this new feature?  Do we
still disallow to start a trace experiment with no enabled
tracepoints?  I think we should.

>  Disable tracepoint @var{num}, or all tracepoints if no argument
>  @var{num} is given.  A disabled tracepoint will have no effect during
> -the next trace experiment, but it is not forgotten.  You can re-enable
> +a trace experiment, but it is not forgotten.  You can re-enable
>  a disabled tracepoint using the @code{enable tracepoint} command.
> +If the command is issued during a trace experiment and the debug target
> +has support for disabling tracepoints during a trace experiment, then the
> +change will be effective immediately.  Otherwise, it will be applied to the
> +next trace experiment.
> 
> -Enable tracepoint @var{num}, or all tracepoints.  The enabled
> -tracepoints will become effective the next time a trace experiment is
> -run.
> +Enable tracepoint @var{num}, or all tracepoints.  If this command is
> +issued during a trace experiment and the debug target supports enabling
> +tracepoints during a trace experiment, then the enabled tracepoints will
> +become effective immediately.  Otherwise, they will become effective the
> +next time a trace experiment is run.

Yes, this is fine.  Thanks.

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Tom Tromey]

>>>>> ">" == Kwok Cheung Yeung <kcy@codesourcery.com> writes:

>> This patch adds support for enabling or disabling tracepoints while
>> an experiment is running, by sending a new remote command to the
>> gdbserver whenever this happens.

Just one nit...

>> Index: gdb/NEWS
[...]
>>    Initial support for the OpenCL C language (http://www.khronos.org/opencl)
>>    has been integrated into GDB.

>> +* Tracepoints can now be enabled and disabled at any time after a trace
>> +  experiment has been started using the standard "enable" and "disable"
>> +  commands.  If a trace experiment is started with no enabled
>> +  tracepoints, then a warning will be printed but the experiment will
>> +  proceed anyway.

I think these additions are in the "in 7.3" section but they should be
in the "since 7.3" section.

Tom

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Kwok Cheung Yeung]

On 06/05/2011 11:32, Eli Zaretskii wrote:
> Btw, what if the target does not support this new feature?  Do we
> still disallow to start a trace experiment with no enabled
> tracepoints?  I think we should.
> 

I've now added 'EnableDisableTracepoints' to the list of features returned by
the qSupported packet from a patched gdbserver. GDB now checks whether this is
supported before trying to enable/disable a tracepoint on the target. If it is
not, then it silently reverts to the original behaviour.

Kwok

	gdb/
	* breakpoint.c (disable_breakpoint): Disable all locations
  	associated with a tracepoint on target if a trace experiment is
	running.
 	(disable_command): Disable a specific tracepoint location on target if
 	a trace	experiment is running.
 	(do_enable_breakpoint): Enable all locations associated with a
 	tracepoint on target if a trace experiment is running.
 	(enable_command) Enable a specific tracepoint location on target if a
 	trace experiment is running.
 	* target.c (update_current_target): Add INHERIT and de_fault clauses for
 	to_supports_enable_disable_tracepoint, to_enable_tracepoint and
	to_disable_tracepoint.
 	* target.h: Add declaration of struct bp_location.
 	(struct target_ops): Add new functions
	to_supports_enable_disable_tracepoint, to_enable_tracepoint and
 	to_disable_tracepoint to target operations.
	(target_supports_enable_disable_tracepoint): New macro.
 	(target_enable_tracepoint): New macro.
 	(target_disable_tracepoint): New macro.
 	* remote.c (struct remote_state): Add new field.
	(remote_enable_disable_tracepoint_feature): New.
	(remote_protocol_features): Add new entry.
	(remote_supports_enable_disable_tracepoint): New.
	(remote_enable_tracepoint): New.
 	(remote_disable_tracepoint): New.
 	(init_remote_ops): Add remote_enable_tracepoint,
	remote_disable_tracepoint and remote_supports_enable_disable_tracepoint
	to remote operations.
 	* tracepoint.c (start_tracing): Allow tracing to start without any
 	tracepoints enabled with just a warning if they can be re-enabled
	later.
 	* NEWS: Add news item for the new behaviour of the enable and disable
 	GDB commands when applied to tracepoints.
 	Add news items for the new remote packets QTEnable and QTDisable.

	gdb/doc/
	* gdb.texinfo: Document change in the behaviour of the enable and
	disable GDB commands when applied to tracepoints.
	Document the EnableDisableTracepoints remote stub feature.
	Document QTEnable and QTDisable in the list of tracepoint packets.

	gdb/gdbserver/
	* server.c (handle_query): Add EnableDisableTracepoints to the list
	of supported features.
	* tracepoint.c (clear_installed_tracepoints): Uninstall	disabled
	tracepoints.
 	(cmd_qtenable_disable): New.
 	(cmd_qtstart): Install tracepoints even if disabled.
 	(handle_tracepoint_general_set): Add call to cmd_qtenable_disable on
 	receiving a QTEnable or QTDisable packet.
 	(gdb_collect): Skip data collection if fast tracepoint is disabled.
 	(ust_marker_to_static_tracepoint): Do not ignore disabled static
 	tracepoints.
 	(gdb_probe): Skip data collection if static tracepoint is disabled.

Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.437
diff -u -p -r1.437 NEWS
--- gdb/NEWS	6 May 2011 18:46:31 -0000	1.437
+++ gdb/NEWS	9 May 2011 19:17:10 -0000
@@ -21,6 +21,23 @@ watch EXPRESSION mask MASK_VALUE
   The watch command now supports the mask argument which allows creation
   of masked watchpoints, if the current architecture supports this feature.

+* Tracepoints can now be enabled and disabled at any time after a trace
+  experiment has been started using the standard "enable" and "disable"
+  commands.  It is now possible to start a trace experiment with no enabled
+  tracepoints; GDB will display a warning, but will allow the experiment to
+  begin, assuming that tracepoints will be enabled as needed while the trace
+  is running.
+
+* New remote packets
+
+QTEnable
+
+  Dynamically enable a tracepoint in a started trace experiment.
+
+QTDisable
+
+  Dynamically disable a tracepoint in a started trace experiment.
+
 *** Changes in GDB 7.3

 * GDB has a new command: "thread find [REGEXP]".
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.575
diff -u -p -r1.575 breakpoint.c
--- gdb/breakpoint.c	6 May 2011 18:46:31 -0000	1.575
+++ gdb/breakpoint.c	9 May 2011 19:17:11 -0000
@@ -11685,6 +11685,15 @@ disable_breakpoint (struct breakpoint *b

   bpt->enable_state = bp_disabled;

+  if (target_supports_enable_disable_tracepoint ()
+      && current_trace_status ()->running && is_tracepoint (bpt))
+    {
+      struct bp_location *location;
+
+      for (location = bpt->loc; location; location = location->next)
+	target_disable_tracepoint (location);
+    }
+
   update_global_location_list (0);

   observer_notify_breakpoint_modified (bpt);
@@ -11714,7 +11723,13 @@ disable_command (char *args, int from_tt
     {
       struct bp_location *loc = find_location_by_number (args);
       if (loc)
-	loc->enabled = 0;
+	{
+	  loc->enabled = 0;
+	  if (target_supports_enable_disable_tracepoint ()
+	      && current_trace_status ()->running && loc->owner
+	      && is_tracepoint (loc->owner))
+	    target_disable_tracepoint (loc);
+	}
       update_global_location_list (0);
     }
   else
@@ -11762,6 +11777,16 @@ do_enable_breakpoint (struct breakpoint

   if (bpt->enable_state != bp_permanent)
     bpt->enable_state = bp_enabled;
+
+  if (target_supports_enable_disable_tracepoint ()
+      && current_trace_status ()->running && is_tracepoint (bpt))
+    {
+      struct bp_location *location;
+
+      for (location = bpt->loc; location; location = location->next)
+	target_enable_tracepoint (location);
+    }
+
   bpt->disposition = disposition;
   update_global_location_list (1);
   breakpoints_changed ();
@@ -11804,7 +11829,13 @@ enable_command (char *args, int from_tty
     {
       struct bp_location *loc = find_location_by_number (args);
       if (loc)
-	loc->enabled = 1;
+	{
+	  loc->enabled = 1;
+	  if (target_supports_enable_disable_tracepoint ()
+	      && current_trace_status ()->running && loc->owner
+	      && is_tracepoint (loc->owner))
+	    target_enable_tracepoint (loc);
+	}
       update_global_location_list (1);
     }
   else
Index: gdb/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.445
diff -u -p -r1.445 remote.c
--- gdb/remote.c	27 Apr 2011 13:29:14 -0000	1.445
+++ gdb/remote.c	9 May 2011 19:17:12 -0000
@@ -329,6 +329,10 @@ struct remote_state
      disconnected.  */
   int disconnected_tracing;

+  /* True if the stub reports support for enabling and disabling
+     tracepoints while a trace experiment is running.  */
+  int enable_disable_tracepoints;
+
   /* Nonzero if the user has pressed Ctrl-C, but the target hasn't
      responded to that.  */
   int ctrlc_pending_p;
@@ -3689,6 +3693,16 @@ remote_disconnected_tracing_feature (con
   rs->disconnected_tracing = (support == PACKET_ENABLE);
 }

+static void
+remote_enable_disable_tracepoint_feature (const struct protocol_feature *feature,
+					  enum packet_support support,
+					  const char *value)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  rs->enable_disable_tracepoints = (support == PACKET_ENABLE);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -3735,6 +3749,8 @@ static struct protocol_feature remote_pr
     PACKET_TracepointSource },
   { "QAllow", PACKET_DISABLE, remote_supported_packet,
     PACKET_QAllow },
+  { "EnableDisableTracepoints", PACKET_DISABLE,
+    remote_enable_disable_tracepoint_feature, -1 },
 };

 static char *remote_support_xml;
@@ -9648,6 +9664,14 @@ remote_supports_static_tracepoints (void
   return rs->static_tracepoints;
 }

+static int
+remote_supports_enable_disable_tracepoint (void)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  return rs->enable_disable_tracepoints;
+}
+
 static void
 remote_trace_init (void)
 {
@@ -9919,6 +9943,38 @@ remote_download_trace_state_variable (st
 }

 static void
+remote_enable_tracepoint (struct bp_location *location)
+{
+  struct remote_state *rs = get_remote_state ();
+  char addr_buf[40];
+
+  sprintf_vma (addr_buf, location->address);
+  sprintf (rs->buf, "QTEnable:%x:%s", location->owner->number, addr_buf);
+  putpkt (rs->buf);
+  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+  if (*rs->buf == '\0')
+    error (_("Target does not support enabling tracepoints while a trace run is
ongoing."));
+  if (strcmp (rs->buf, "OK") != 0)
+    error (_("Error on target while enabling tracepoint."));
+}
+
+static void
+remote_disable_tracepoint (struct bp_location *location)
+{
+  struct remote_state *rs = get_remote_state ();
+  char addr_buf[40];
+
+  sprintf_vma (addr_buf, location->address);
+  sprintf (rs->buf, "QTDisable:%x:%s", location->owner->number, addr_buf);
+  putpkt (rs->buf);
+  remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+  if (*rs->buf == '\0')
+    error (_("Target does not support disabling tracepoints while a trace run
is ongoing."));
+  if (strcmp (rs->buf, "OK") != 0)
+    error (_("Error on target while disabling tracepoint."));
+}
+
+static void
 remote_trace_set_readonly_regions (void)
 {
   asection *s;
@@ -10308,10 +10364,13 @@ Specify the serial device it is connecte
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_supports_enable_disable_tracepoint =
remote_supports_enable_disable_tracepoint;
   remote_ops.to_trace_init = remote_trace_init;
   remote_ops.to_download_tracepoint = remote_download_tracepoint;
   remote_ops.to_download_trace_state_variable
     = remote_download_trace_state_variable;
+  remote_ops.to_enable_tracepoint = remote_enable_tracepoint;
+  remote_ops.to_disable_tracepoint = remote_disable_tracepoint;
   remote_ops.to_trace_set_readonly_regions = remote_trace_set_readonly_regions;
   remote_ops.to_trace_start = remote_trace_start;
   remote_ops.to_get_trace_status = remote_get_trace_status;
Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.282
diff -u -p -r1.282 target.c
--- gdb/target.c	6 May 2011 18:46:31 -0000	1.282
+++ gdb/target.c	9 May 2011 19:17:12 -0000
@@ -659,9 +659,12 @@ update_current_target (void)
       INHERIT (to_get_ada_task_ptid, t);
       /* Do not inherit to_search_memory.  */
       INHERIT (to_supports_multi_process, t);
+      INHERIT (to_supports_enable_disable_tracepoint, t);
       INHERIT (to_trace_init, t);
       INHERIT (to_download_tracepoint, t);
       INHERIT (to_download_trace_state_variable, t);
+      INHERIT (to_enable_tracepoint, t);
+      INHERIT (to_disable_tracepoint, t);
       INHERIT (to_trace_set_readonly_regions, t);
       INHERIT (to_trace_start, t);
       INHERIT (to_get_trace_status, t);
@@ -825,6 +828,9 @@ update_current_target (void)
   de_fault (to_supports_multi_process,
 	    (int (*) (void))
 	    return_zero);
+  de_fault (to_supports_enable_disable_tracepoint,
+	    (int (*) (void))
+	    return_zero);
   de_fault (to_trace_init,
 	    (void (*) (void))
 	    tcomplain);
@@ -834,6 +840,12 @@ update_current_target (void)
   de_fault (to_download_trace_state_variable,
 	    (void (*) (struct trace_state_variable *))
 	    tcomplain);
+  de_fault (to_enable_tracepoint,
+	    (void (*) (struct bp_location *))
+	    tcomplain);
+  de_fault (to_disable_tracepoint,
+	    (void (*) (struct bp_location *))
+	    tcomplain);
   de_fault (to_trace_set_readonly_regions,
 	    (void (*) (void))
 	    tcomplain);
Index: gdb/target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.205
diff -u -p -r1.205 target.h
--- gdb/target.h	6 May 2011 18:46:32 -0000	1.205
+++ gdb/target.h	9 May 2011 19:17:12 -0000
@@ -28,6 +28,7 @@ struct objfile;
 struct ui_file;
 struct mem_attrib;
 struct target_ops;
+struct bp_location;
 struct bp_target_info;
 struct regcache;
 struct target_section_table;
@@ -644,6 +645,10 @@ struct target_ops
        simultaneously?  */
     int (*to_supports_multi_process) (void);

+    /* Does this target support enabling and disabling tracepoints while a trace
+       experiment is running?  */
+    int (*to_supports_enable_disable_tracepoint) (void);
+
     /* Determine current architecture of thread PTID.

        The target is supposed to determine the architecture of the code where
@@ -674,6 +679,12 @@ struct target_ops
     /* Send full details of a trace state variable to the target.  */
     void (*to_download_trace_state_variable) (struct trace_state_variable *tsv);

+    /* Enable a tracepoint on the target.  */
+    void (*to_enable_tracepoint) (struct bp_location *location);
+
+    /* Disable a tracepoint on the target.  */
+    void (*to_disable_tracepoint) (struct bp_location *location);
+
     /* Inform the target info of memory regions that are readonly
        (such as text sections), and so it should return data from
        those rather than look in the trace buffer.  */
@@ -873,6 +884,12 @@ struct address_space *target_thread_addr
 #define	target_supports_multi_process()	\
      (*current_target.to_supports_multi_process) ()

+/* Returns true if this target can enable and disable tracepoints
+   while a trace experiment is running.  */
+
+#define target_supports_enable_disable_tracepoint() \
+  (*current_target.to_supports_enable_disable_tracepoint) ()
+
 /* Invalidate all target dcaches.  */
 extern void target_dcache_invalidate (void);

@@ -1457,6 +1474,12 @@ extern int target_search_memory (CORE_AD
 #define target_download_trace_state_variable(tsv) \
   (*current_target.to_download_trace_state_variable) (tsv)

+#define target_enable_tracepoint(loc) \
+  (*current_target.to_enable_tracepoint) (loc)
+
+#define target_disable_tracepoint(loc) \
+  (*current_target.to_disable_tracepoint) (loc)
+
 #define target_trace_start() \
   (*current_target.to_trace_start) ()

Index: gdb/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.222
diff -u -p -r1.222 tracepoint.c
--- gdb/tracepoint.c	19 Apr 2011 18:04:07 -0000	1.222
+++ gdb/tracepoint.c	9 May 2011 19:17:13 -0000
@@ -1583,11 +1583,17 @@ start_tracing (void)
 		 (t->type == bp_fast_tracepoint ? "fast " : ""), t->number);
     }

-  /* No point in tracing with only disabled tracepoints.  */
   if (!any_enabled)
     {
-      VEC_free (breakpoint_p, tp_vec);
-      error (_("No tracepoints enabled, not starting trace"));
+      if (target_supports_enable_disable_tracepoint ())
+	warning (_("No tracepoints enabled"));
+      else
+	{
+	  /* No point in tracing with only disabled tracepoints that
+	     cannot be re-enabled.  */
+	  VEC_free (breakpoint_p, tp_vec);
+	  error (_("No tracepoints enabled, not starting trace"));
+	}
     }

   if (num_to_download <= 0)
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.830
diff -u -p -r1.830 gdb.texinfo
--- gdb/doc/gdb.texinfo	6 May 2011 18:46:33 -0000	1.830
+++ gdb/doc/gdb.texinfo	9 May 2011 19:17:16 -0000
@@ -10026,14 +10026,20 @@ These commands are deprecated; they are
 @item disable tracepoint @r{[}@var{num}@r{]}
 Disable tracepoint @var{num}, or all tracepoints if no argument
 @var{num} is given.  A disabled tracepoint will have no effect during
-the next trace experiment, but it is not forgotten.  You can re-enable
+a trace experiment, but it is not forgotten.  You can re-enable
 a disabled tracepoint using the @code{enable tracepoint} command.
+If the command is issued during a trace experiment and the debug target
+has support for disabling tracepoints during a trace experiment, then the
+change will be effective immediately.  Otherwise, it will be applied to the
+next trace experiment.

 @kindex enable tracepoint
 @item enable tracepoint @r{[}@var{num}@r{]}
-Enable tracepoint @var{num}, or all tracepoints.  The enabled
-tracepoints will become effective the next time a trace experiment is
-run.
+Enable tracepoint @var{num}, or all tracepoints.  If this command is
+issued during a trace experiment and the debug target supports enabling
+tracepoints during a trace experiment, then the enabled tracepoints will
+become effective immediately.  Otherwise, they will become effective the
+next time a trace experiment is run.
 @end table

 @node Tracepoint Passcounts
@@ -33622,6 +33628,11 @@ These are the currently defined stub fea
 @tab @samp{-}
 @tab No

+@item @samp{EnableDisableTracepoints}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable

 These are the currently defined stub features, in more detail:
@@ -33734,6 +33745,11 @@ The remote stub understands the @samp{QA
 @cindex static tracepoints, in remote protocol
 The remote stub supports static tracepoints.

+@item EnableDisableTracepoints
+The remote stub supports the @samp{QTEnable} (@pxref{QTEnable}) and
+@samp{QTDisable} (@pxref{QTDisable}) packets that allow tracepoints
+to be enabled and disabled while a trace experiment is running.
+
 @end table

 @item qSymbol::
@@ -33814,6 +33830,8 @@ packets.)
 @item qTsV
 @itemx QTStart
 @itemx QTStop
+@itemx QTEnable
+@itemx QTDisable
 @itemx QTinit
 @itemx QTro
 @itemx qTStatus
@@ -34303,6 +34321,18 @@ instruction reply packet}).
 @item QTStop
 End the tracepoint experiment.  Stop collecting trace frames.

+@item QTEnable:@var{n}:@var{addr}
+@anchor{QTEnable}
+Enable tracepoint @var{n} at address @var{addr} in a started tracepoint
+experiment.  If the tracepoint was previously disabled, then collection
+of data from it will resume.
+
+@item QTDisable:@var{n}:@var{addr}
+@anchor{QTDisable}
+Disable tracepoint @var{n} at address @var{addr} in a started tracepoint
+experiment.  No more data will be collected from the tracepoint unless
+@samp{QTEnable:@var{n}:@var{addr}} is subsequently issued.
+
 @item QTinit
 Clear the table of tracepoints, and empty the trace frame buffer.

Index: gdb/gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.144
diff -u -p -r1.144 server.c
--- gdb/gdbserver/server.c	4 May 2011 20:20:12 -0000	1.144
+++ gdb/gdbserver/server.c	9 May 2011 19:17:17 -0000
@@ -1540,6 +1540,7 @@ handle_query (char *own_buf, int packet_
 	  strcat (own_buf, ";StaticTracepoints+");
 	  strcat (own_buf, ";qXfer:statictrace:read+");
 	  strcat (own_buf, ";qXfer:traceframe-info:read+");
+	  strcat (own_buf, ";EnableDisableTracepoints+");
 	}

       return;
Index: gdb/gdbserver/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/tracepoint.c,v
retrieving revision 1.24
diff -u -p -r1.24 tracepoint.c
--- gdb/gdbserver/tracepoint.c	10 Mar 2011 20:21:14 -0000	1.24
+++ gdb/gdbserver/tracepoint.c	9 May 2011 19:17:17 -0000
@@ -2214,9 +2214,6 @@ clear_installed_tracepoints (void)
   /* Restore any bytes overwritten by tracepoints.  */
   for (tpoint = tracepoints; tpoint; tpoint = tpoint->next)
     {
-      if (!tpoint->enabled)
-	continue;
-
       /* Catch the case where we might try to remove a tracepoint that
 	 was never actually installed.  */
       if (tpoint->handle == NULL)
@@ -2455,6 +2452,73 @@ cmd_qtdv (char *own_buf)
 }

 static void
+cmd_qtenable_disable (char *own_buf, int enable)
+{
+  char *packet = own_buf;
+  ULONGEST num, addr;
+  struct tracepoint *tp;
+
+  packet += strlen (enable ? "QTEnable:" : "QTDisable:");
+  packet = unpack_varlen_hex (packet, &num);
+  ++packet; /* skip a colon */
+  packet = unpack_varlen_hex (packet, &addr);
+
+  tp = find_tracepoint (num, addr);
+
+  if (tp)
+    {
+      if ((enable && tp->enabled) || (!enable && !tp->enabled))
+	{
+	  trace_debug ("Tracepoint %d at 0x%s is already %s",
+		       (int) num, paddress (addr),
+		       enable ? "enabled" : "disabled");
+	  write_ok (own_buf);
+	  return;
+	}
+
+      trace_debug ("%s tracepoint %d at 0x%s",
+		   enable ? "Enabling" : "Disabling",
+		   (int) num, paddress (addr));
+
+      tp->enabled = enable;
+
+      if (tp->type == fast_tracepoint || tp->type == static_tracepoint)
+	{
+	  int ret;
+	  int offset = offsetof (struct tracepoint, enabled);
+	  CORE_ADDR obj_addr = tp->obj_addr_on_target + offset;
+
+	  ret = prepare_to_access_memory ();
+	  if (ret)
+	    {
+	      trace_debug ("Failed to temporarily stop inferior threads");
+	      write_enn (own_buf);
+	      return;
+	    }
+	
+	  ret = write_inferior_integer (obj_addr, enable);
+	  done_accessing_memory ();
+	
+	  if (ret)
+	    {
+	      trace_debug ("Cannot write enabled flag into "
+			   "inferior process memory");
+	      write_enn (own_buf);
+	      return;
+	    }
+	}
+
+      write_ok (own_buf);
+    }
+  else
+    {
+      trace_debug ("Tracepoint %d at 0x%s not found",
+		   (int) num, paddress (addr));
+      write_enn (own_buf);
+    }
+}
+
+static void
 cmd_qtv (char *own_buf)
 {
   ULONGEST num;
@@ -2719,9 +2783,6 @@ cmd_qtstart (char *packet)
       /* Ensure all the hit counts start at zero.  */
       tpoint->hit_count = 0;

-      if (!tpoint->enabled)
-	continue;
-
       if (tpoint->type == trap_tracepoint)
 	{
 	  ++slow_tracepoint_count;
@@ -3461,6 +3522,16 @@ handle_tracepoint_general_set (char *pac
       cmd_qtdpsrc (packet);
       return 1;
     }
+  else if (strncmp ("QTEnable:", packet, strlen ("QTEnable:")) == 0)
+    {
+      cmd_qtenable_disable (packet, 1);
+      return 1;
+    }
+  else if (strncmp ("QTDisable:", packet, strlen ("QTDisable:")) == 0)
+    {
+      cmd_qtenable_disable (packet, 0);
+      return 1;
+    }
   else if (strncmp ("QTDV:", packet, strlen ("QTDV:")) == 0)
     {
       cmd_qtdv (packet);
@@ -5340,6 +5411,9 @@ gdb_collect (struct tracepoint *tpoint,
   if (!tracing)
     return;

+  if (!tpoint->enabled)
+    return;
+
   ctx.base.type = fast_tracepoint;
   ctx.regs = regs;
   ctx.regcache_initted = 0;
@@ -6598,7 +6672,7 @@ ust_marker_to_static_tracepoint (const s

   for (tpoint = tracepoints; tpoint; tpoint = tpoint->next)
     {
-      if (!tpoint->enabled || tpoint->type != static_tracepoint)
+      if (tpoint->type != static_tracepoint)
 	continue;

       if (tpoint->address == (uintptr_t) mdata->location)
@@ -6653,6 +6727,12 @@ gdb_probe (const struct marker *mdata, v
       return;
     }

+  if (!tpoint->enabled)
+    {
+      trace_debug ("gdb_probe: tracepoint disabled");
+      return;
+    }
+
   ctx.tpoint = tpoint;

   trace_debug ("gdb_probe: collecting marker: "

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Eli Zaretskii]

> Date: Mon, 09 May 2011 20:31:10 +0100
> From: Kwok Cheung Yeung <kcy@codesourcery.com>
> CC: gdb-patches@sourceware.org
> 
> On 06/05/2011 11:32, Eli Zaretskii wrote:
> > Btw, what if the target does not support this new feature?  Do we
> > still disallow to start a trace experiment with no enabled
> > tracepoints?  I think we should.
> > 
> 
> I've now added 'EnableDisableTracepoints' to the list of features returned by
> the qSupported packet from a patched gdbserver. GDB now checks whether this is
> supported before trying to enable/disable a tracepoint on the target. If it is
> not, then it silently reverts to the original behaviour.

Thanks.

This version of the docs is fine with me.

Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Pedro Alves]

On Monday 09 May 2011 20:31:10, Kwok Cheung Yeung wrote:
>         gdb/
>         * breakpoint.c (disable_breakpoint): Disable all locations
>         associated with a tracepoint on target if a trace experiment is
>         running.
>         (disable_command): Disable a specific tracepoint location on target if
>         a trace experiment is running.
>         (do_enable_breakpoint): Enable all locations associated with a
>         tracepoint on target if a trace experiment is running.
>         (enable_command) Enable a specific tracepoint location on target if a
>         trace experiment is running.
>         * target.c (update_current_target): Add INHERIT and de_fault clauses for
>         to_supports_enable_disable_tracepoint, to_enable_tracepoint and
>         to_disable_tracepoint.
>         * target.h: Add declaration of struct bp_location.
>         (struct target_ops): Add new functions
>         to_supports_enable_disable_tracepoint, to_enable_tracepoint and
>         to_disable_tracepoint to target operations.
>         (target_supports_enable_disable_tracepoint): New macro.
>         (target_enable_tracepoint): New macro.
>         (target_disable_tracepoint): New macro.
>         * remote.c (struct remote_state): Add new field.
>         (remote_enable_disable_tracepoint_feature): New.
>         (remote_protocol_features): Add new entry.
>         (remote_supports_enable_disable_tracepoint): New.
>         (remote_enable_tracepoint): New.
>         (remote_disable_tracepoint): New.
>         (init_remote_ops): Add remote_enable_tracepoint,
>         remote_disable_tracepoint and remote_supports_enable_disable_tracepoint
>         to remote operations.
>         * tracepoint.c (start_tracing): Allow tracing to start without any
>         tracepoints enabled with just a warning if they can be re-enabled
>         later.

Okay.

> 
>         gdb/gdbserver/
>         * server.c (handle_query): Add EnableDisableTracepoints to the list
>         of supported features.
>         * tracepoint.c (clear_installed_tracepoints): Uninstall disabled
>         tracepoints.
>         (cmd_qtenable_disable): New.
>         (cmd_qtstart): Install tracepoints even if disabled.
>         (handle_tracepoint_general_set): Add call to cmd_qtenable_disable on
>         receiving a QTEnable or QTDisable packet.
>         (gdb_collect): Skip data collection if fast tracepoint is disabled.
>         (ust_marker_to_static_tracepoint): Do not ignore disabled static
>         tracepoints.
>         (gdb_probe): Skip data collection if static tracepoint is disabled.

Okay.

-- 
Pedro Alves