[PATCH 0/6] Add a new field 'installed' when reporting tracepoint

[PATCH 0/6] Add a new field 'installed' when reporting tracepoint

[Yao Qi]

Hello,
During the review to 'tracepoint download' notification
<http://sourceware.org/ml/gdb-patches/2012-11/msg00871.html>, we find it is
better to treat 'installed on target' as an attribute of a tracepoint,
and breakpoint related notification can report it, instead of a
separate notification.

In this patch series, we add a field 'installed' in
print_one_breakpoint_location, which is a boolean to show "y" or "n" the
tracepoint is installed on target.  The field reflects the value of
'inserted' field of each location.

Originally, my proposal is for MI notification, but I think it also
useful to display the state of 'installed' in the output of CLI.  With
this series applied, we can see the 'installed' state in both CLI and
MI.

The 'installed' field is not shown for non-tracepoint breakpoint,
because in most of cases, breakpoints are inserted when to resume
inferior, and removed when inferior stops.

1/6  Add a field 'installed' for each location of tracepoint.
2/6  Iterate over ALL_TRACEPOINTS first.
3/6  Notify breakpoint-modified when tracepoints are downloaded.
4/6  Notify breakpoint-modified when uploaded tracepoints are merged.
5/6  Test tracepoints are installed or not
6/6  Update test cases for 'installed' field.

Patch 1/6 adds the new field 'installed' in
print_one_breakpoint_location, and patch 2/6 - 4/6 call
breakpoint-modified observer when the 'inserted' flag is changed.  Patch
5/6 is to test this feature, and patch 6/6 is update existing tests for
the new output of 'info breakpoint' or 'info tracepoint'.

The whole series are tested on x86_linux both native and gdbserver.  Is
it OK?

 gdb/breakpoint.c                                  |   48 ++++++++++++++-------
 gdb/testsuite/gdb.trace/actions.exp               |   25 +++++++++--
 gdb/testsuite/gdb.trace/change-loc.exp            |    6 ++-
 gdb/testsuite/gdb.trace/deltrace.exp              |   22 +++++++--
 gdb/testsuite/gdb.trace/infotrace.exp             |   10 +++-
 gdb/testsuite/gdb.trace/mi-traceframe-changed.exp |    3 +-
 gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   20 +++++++-
 gdb/testsuite/gdb.trace/passcount.exp             |   28 +++++++++---
 gdb/testsuite/gdb.trace/tracecmd.exp              |   13 ++++--
 gdb/testsuite/gdb.trace/while-stepping.exp        |    3 +-
 gdb/tracepoint.c                                  |   33 ++++++++++++++
 11 files changed, 163 insertions(+), 48 deletions(-)

-- 
1.7.7.6

[PATCH 3/6] Notify breakpoint-modified when tracepoints are downloaded.

[Yao Qi]

When any location of a tracepoint is downloaded, notify
'breakpoint-modified' observer.

gdb:

2012-12-03  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (download_tracepoint_locations): Notify
	'breakpoint-modified' observer if any tracepoint location is
	downloaded.
	* tracepoint.c (start_tracing): Likewise.
---
 gdb/breakpoint.c |    4 ++++
 gdb/tracepoint.c |    5 +++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3f2f0c9..56eeb35 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12090,6 +12090,7 @@ download_tracepoint_locations (void)
     {
       struct bp_location *bl;
       struct tracepoint *t;
+      int bp_location_downloaded = 0;
 
       if ((b->type == bp_fast_tracepoint
 	   ? !may_insert_fast_tracepoints
@@ -12109,9 +12110,12 @@ download_tracepoint_locations (void)
 	  target_download_tracepoint (bl);
 
 	  bl->inserted = 1;
+	  bp_location_downloaded = 1;
 	}
       t = (struct tracepoint *) b;
       t->number_on_target = b->number;
+      if (bp_location_downloaded)
+	observer_notify_breakpoint_modified (b);
     }
 
   do_cleanups (old_chain);
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 912341a..c467b9c 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1767,6 +1767,7 @@ start_tracing (char *notes)
     {
       struct tracepoint *t = (struct tracepoint *) b;
       struct bp_location *loc;
+      int bp_location_downloaded = 0;
 
       /* Clear `inserted' flag.  */
       for (loc = b->loc; loc; loc = loc->next)
@@ -1788,6 +1789,7 @@ start_tracing (char *notes)
 	  target_download_tracepoint (loc);
 
 	  loc->inserted = 1;
+	  bp_location_downloaded = 1;
 	}
 
       t->number_on_target = b->number;
@@ -1795,6 +1797,9 @@ start_tracing (char *notes)
       for (loc = b->loc; loc; loc = loc->next)
 	if (loc->probe != NULL)
 	  loc->probe->pops->set_semaphore (loc->probe, loc->gdbarch);
+
+      if (bp_location_downloaded)
+	observer_notify_breakpoint_modified (b);
     }
   VEC_free (breakpoint_p, tp_vec);
 
-- 
1.7.7.6

Re: [PATCH 3/6] Notify breakpoint-modified when tracepoints are downloaded.

[Pedro Alves]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> When any location of a tracepoint is downloaded, notify
> 'breakpoint-modified' observer.

Looks good.  But please make sure that the resulting MI change is
documented.

-- 
Pedro Alves

[PATCH 4/6] Notify breakpoint-modified when uploaded tracepoints are merged.

[Yao Qi]

When any location of a tracepoint is marked as installed/inserted,
notify 'breakpoint-modified' observer.

gdb:

2012-12-03  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c (merge_uploaded_tracepoints): Record all modified
	tracepoints and notify 'breakpoint-modified' observer for them.
---
 gdb/tracepoint.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c467b9c..0c023bd 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3475,6 +3475,10 @@ void
 merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
 {
   struct uploaded_tp *utp;
+  /* A set of tracepoints which are modified.  */
+  VEC(breakpoint_p) *modified_tp = NULL;
+  int ix;
+  struct breakpoint *b;
 
   /* Look for GDB tracepoints that match up with our uploaded versions.  */
   for (utp = *uploaded_tps; utp; utp = utp->next)
@@ -3485,6 +3489,8 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
       loc = find_matching_tracepoint_location (utp);
       if (loc)
 	{
+	  int found = 0;
+
 	  /* Mark this location as already inserted.  */
 	  loc->inserted = 1;
 	  t = (struct tracepoint *) loc->owner;
@@ -3492,6 +3498,22 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
 			     "as target's tracepoint %d at %s.\n"),
 			   loc->owner->number, utp->number,
 			   paddress (loc->gdbarch, utp->addr));
+
+	  /* One location LOC of tracepoint is modified ('inserted' is
+	     set to 1), save LOC->owner in MODIFIED_TP if LOC->owner
+	     is not added to MODIFIED_TP before.  The observer
+	     'breakpoint-modified' will be notified later with every
+	     tracepoint saved in MODIFIED_TP.  */
+	  for (ix = 0;
+	       VEC_iterate (breakpoint_p, modified_tp, ix, b);
+	       ix++)
+	    if (b == loc->owner)
+	      {
+		found = 1;
+		break;
+	      }
+	  if (!found)
+	    VEC_safe_push (breakpoint_p, modified_tp, loc->owner);
 	}
       else
 	{
@@ -3514,6 +3536,12 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
 	t->number_on_target = utp->number;
     }
 
+  /* Notify 'breakpoint-modified' observer that at least one of B's
+     locations is changed.  */
+  for (ix = 0; VEC_iterate (breakpoint_p, modified_tp, ix, b); ix++)
+    observer_notify_breakpoint_modified (b);
+
+  VEC_free (breakpoint_p, modified_tp);
   free_uploaded_tps (uploaded_tps);
 }
 
-- 
1.7.7.6

Re: [PATCH 4/6] Notify breakpoint-modified when uploaded tracepoints are merged.

[Pedro Alves]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> When any location of a tracepoint is marked as installed/inserted,
> notify 'breakpoint-modified' observer.
> 
> gdb:
> 
> 2012-12-03  Yao Qi  <yao@codesourcery.com>
> 
> 	* tracepoint.c (merge_uploaded_tracepoints): Record all modified
> 	tracepoints and notify 'breakpoint-modified' observer for them.

This looks good to me.  Some comment suggestions below.

I ran out of time for now, so I'll review the rest of the series later or tomorrow.

> ---
>  gdb/tracepoint.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index c467b9c..0c023bd 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -3475,6 +3475,10 @@ void
>  merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
>  {
>    struct uploaded_tp *utp;
> +  /* A set of tracepoints which are modified.  */
> +  VEC(breakpoint_p) *modified_tp = NULL;
> +  int ix;
> +  struct breakpoint *b;
>  
>    /* Look for GDB tracepoints that match up with our uploaded versions.  */
>    for (utp = *uploaded_tps; utp; utp = utp->next)
> @@ -3485,6 +3489,8 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
>        loc = find_matching_tracepoint_location (utp);
>        if (loc)
>  	{
> +	  int found = 0;
> +
>  	  /* Mark this location as already inserted.  */
>  	  loc->inserted = 1;
>  	  t = (struct tracepoint *) loc->owner;
> @@ -3492,6 +3498,22 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
>  			     "as target's tracepoint %d at %s.\n"),
>  			   loc->owner->number, utp->number,
>  			   paddress (loc->gdbarch, utp->addr));
> +
> +	  /* One location LOC of tracepoint is modified ('inserted' is
> +	     set to 1), save LOC->owner in MODIFIED_TP if LOC->owner
> +	     is not added to MODIFIED_TP before.  The observer
> +	     'breakpoint-modified' will be notified later with every
> +	     tracepoint saved in MODIFIED_TP.  */

I suggest:

	  /* The tracepoint was modified (a location was marked as inserted
	     in the target).  Save it in MODIFIED_TP if not there yet.  The
	     'breakpoint-modified' observers will be notified later once for each
	     tracepoint saved in MODIFIED_TP.  */


> +	  for (ix = 0;
> +	       VEC_iterate (breakpoint_p, modified_tp, ix, b);
> +	       ix++)
> +	    if (b == loc->owner)
> +	      {
> +		found = 1;
> +		break;
> +	      }
> +	  if (!found)
> +	    VEC_safe_push (breakpoint_p, modified_tp, loc->owner);
>  	}
>        else
>  	{
> @@ -3514,6 +3536,12 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
>  	t->number_on_target = utp->number;
>      }
>  
> +  /* Notify 'breakpoint-modified' observer that at least one of B's
> +     locations is changed.  */

  /* Notify 'breakpoint-modified' observers that at least one of B's
     locations was changed.  */

> +  for (ix = 0; VEC_iterate (breakpoint_p, modified_tp, ix, b); ix++)
> +    observer_notify_breakpoint_modified (b);
> +
> +  VEC_free (breakpoint_p, modified_tp);
>    free_uploaded_tps (uploaded_tps);
>  }


-- 
Pedro Alves

[PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Yao Qi]

Hi, this patch adds a field 'installed' indicating whether a
location of tracepoint is installed on target.  In CLI, the output of
'info tracepoint' will be like this,

info trace
Num     Type           Disp Enb Address    What^M
2       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
        installed y
4       tracepoint     keep y   <MULTIPLE>
        collect $eip
4.1                         y     0x0804859c in func4 at gdb.trace/change-loc.h:35
        installed y
4.2                         y     0xb7ffc480 in func4 at gdb.trace/change-loc.h:35
        installed y

Of course, the breakpoint related MI notifications are changed as
well.

gdb:

2012-12-03  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (print_one_breakpoint_location): Add field
	'installed' for each location of a tracepoint.
---
 gdb/breakpoint.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 53944a6..40d2edd 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6123,6 +6123,17 @@ print_one_breakpoint_location (struct breakpoint *b,
 	  ui_out_field_int (uiout, "pass", t->pass_count);
 	  ui_out_text (uiout, " \n");
 	}
+
+      if (!header_of_multiple)
+	{
+	  annotate_field (11);
+	  ui_out_text (uiout, "\tinstalled ");
+	  /* LOC is NULL means the tracepoint is still pending, set
+	     field 'installed' 'n'.  */
+	  ui_out_field_string (uiout, "installed",
+			       loc ? (loc->inserted ? "y" : "n") : "n");
+	  ui_out_text (uiout, " \n");
+	}
     }
 
   if (ui_out_is_mi_like_p (uiout) && !part_of_multiple)
-- 
1.7.7.6

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Pedro Alves]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> Hi, this patch adds a field 'installed' indicating whether a
> location of tracepoint is installed on target.  In CLI, the output of
> 'info tracepoint' will be like this,
> 
> info trace
> Num     Type           Disp Enb Address    What^M
> 2       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
>         installed y

I'd prefer:

2       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
        installed on target
3       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
        not installed on target

As the other similar cases of more breakpoint info more textual as well:

4       breakpoint     keep y   0x0804c8a5 in main at ../../src/gdb/gdb.c:29
        breakpoint already hit 1 time

This should be documented in the manual and NEWS, both the CLI changes, and
the MI change (this patch adds the "installed" field to various MI things).

> Of course, the breakpoint related MI notifications are changed as
> well.
> 
> gdb:
> 
> 2012-12-03  Yao Qi  <yao@codesourcery.com>
> 
> 	* breakpoint.c (print_one_breakpoint_location): Add field
> 	'installed' for each location of a tracepoint.
> ---
>  gdb/breakpoint.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 53944a6..40d2edd 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6123,6 +6123,17 @@ print_one_breakpoint_location (struct breakpoint *b,
>  	  ui_out_field_int (uiout, "pass", t->pass_count);
>  	  ui_out_text (uiout, " \n");
>  	}
> +
> +      if (!header_of_multiple)
> +	{
> +	  annotate_field (11);
> +	  ui_out_text (uiout, "\tinstalled ");
> +	  /* LOC is NULL means the tracepoint is still pending, set
> +	     field 'installed' 'n'.  */
> +	  ui_out_field_string (uiout, "installed",
> +			       loc ? (loc->inserted ? "y" : "n") : "n");
> +	  ui_out_text (uiout, " \n");

Not sure about the pending case.  Can you show how does that look like?
Why not just display nothing in that case?  How does the MI output look
like in the pending case?

> +	}
>      }
>  
>    if (ui_out_is_mi_like_p (uiout) && !part_of_multiple)
> 


-- 
Pedro Alves

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Yao Qi]

On 12/07/2012 04:56 AM, Pedro Alves wrote:
> 2       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
>          installed on target
> 3       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
>          not installed on target
>

That is good to me.

> This should be documented in the manual and NEWS, both the CLI changes, and
> the MI change (this patch adds the "installed" field to various MI things).
>

Will do.

>> >+
>> >+      if (!header_of_multiple)
>> >+	{
>> >+	  annotate_field (11);
>> >+	  ui_out_text (uiout, "\tinstalled ");
>> >+	  /* LOC is NULL means the tracepoint is still pending, set
>> >+	     field 'installed' 'n'.  */
>> >+	  ui_out_field_string (uiout, "installed",
>> >+			       loc ? (loc->inserted ? "y" : "n") : "n");
>> >+	  ui_out_text (uiout, " \n");
> Not sure about the pending case.  Can you show how does that look like?

It shows pending tracepoint like this,

Num     Type           Disp Enb Address    What
1       tracepoint     keep y   <PENDING>  foo
         installed n

> Why not just display nothing in that case?  How does the MI output look
> like in the pending case?
>

Right, it makes few sense to display the 'installed' field for pending 
tracepoint, as it doesn't have any locations at all.  I'll update the 
patch to display 'installed' field for non-pending tracepoints.

-- 
Yao (齐尧)

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Yao Qi]

On 12/07/2012 04:56 AM, Pedro Alves wrote:
> This should be documented in the manual and NEWS, both the CLI changes, and
> the MI change (this patch adds the "installed" field to various MI things).
> 

The new version includes changes to manual and NEWS.

>> >Of course, the breakpoint related MI notifications are changed as
>> >well.
>> >
>> >gdb:
>> >
>> >2012-12-03  Yao Qi<yao@codesourcery.com>
>> >
>> >	* breakpoint.c (print_one_breakpoint_location): Add field
>> >	'installed' for each location of a tracepoint.
>> >---
>> >  gdb/breakpoint.c |   11 +++++++++++
>> >  1 files changed, 11 insertions(+), 0 deletions(-)
>> >
>> >diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> >index 53944a6..40d2edd 100644
>> >--- a/gdb/breakpoint.c
>> >+++ b/gdb/breakpoint.c
>> >@@ -6123,6 +6123,17 @@ print_one_breakpoint_location (struct breakpoint *b,
>> >  	  ui_out_field_int (uiout, "pass", t->pass_count);
>> >  	  ui_out_text (uiout, " \n");
>> >  	}
>> >+
>> >+      if (!header_of_multiple)
>> >+	{
>> >+	  annotate_field (11);
>> >+	  ui_out_text (uiout, "\tinstalled ");
>> >+	  /* LOC is NULL means the tracepoint is still pending, set
>> >+	     field 'installed' 'n'.  */
>> >+	  ui_out_field_string (uiout, "installed",
>> >+			       loc ? (loc->inserted ? "y" : "n") : "n");
>> >+	  ui_out_text (uiout, " \n");
> Not sure about the pending case.  Can you show how does that look like?
> Why not just display nothing in that case?  How does the MI output look
> like in the pending case?
> 

In the new version, we don't display it for neither pending tracepoint
nor pending locations.  In CLI, the output of 'info tracepoint' will be
like this,

info trace
Num     Type           Disp Enb Address    What^M
2       tracepoint     keep y   0x080485b1 in main at gdb.trace/change-loc.c:29
        installed on target
4       tracepoint     keep y   <MULTIPLE>
        collect $eip
4.1                         y     0x0804859c in func4 at gdb.trace/change-loc.h:35
        installed on target
4.2                         y     0xb7ffc480 in func4 at gdb.trace/change-loc.h:35
        installed on target
4.3                         y     <PENDING>  set_tracepoint

I'll post patch 6/6 once the output here is OK to us.

-- 
Yao (齐尧)

gdb:

2012-12-09  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (print_one_breakpoint_location): Display the
	state of 'installed' of each non-pending location of a tracepoint
	in both CLI and MI.
	* NEWS: Mention the change for CLI and MI.

gdb/doc:

2012-12-09  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Listing Tracepoints): Move the example into the
	item about 'passcount'.
	New item and example about 'installed on target' output.
	(GDB/MI Breakpoint Commands): Doc about 'installed field.
---
 gdb/NEWS            |    6 ++++++
 gdb/breakpoint.c    |   19 +++++++++++++++++++
 gdb/doc/gdb.texinfo |   29 +++++++++++++++++++++++------
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 3b09e5f..dd8b1f4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -38,6 +38,9 @@
 
 * The command 'forward-search' can now be abbreviated as 'fo'.
 
+* The command 'info tracepoints' can now display 'installed on target'
+  or 'not installed on target' for each non-pending location of tracepoint.
+
 * New configure options
 
 --enable-libmcheck/--disable-libmcheck
@@ -99,6 +102,9 @@ show print type typedefs
      has been requested.
   ** New optional parameter COUNT added to the "-data-write-memory-bytes" 
      command, to allow pattern filling of memory areas.
+  ** The response to breakpoint commands and breakpoint async records will
+     include an "installed" field containing a boolean state about each
+     non-pending tracepoint location is whether installed on target or not.
 
 * GDB now supports the "mini debuginfo" section, .gnu_debugdata.
   You must have the LZMA library available when configuring GDB for this
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 94874ec..5d5033f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6123,6 +6123,25 @@ print_one_breakpoint_location (struct breakpoint *b,
 	  ui_out_field_int (uiout, "pass", t->pass_count);
 	  ui_out_text (uiout, " \n");
 	}
+
+      /* Don't display it when tracepoint or tracepoint location is
+	 pending.   */
+      if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
+	{
+	  annotate_field (11);
+
+	  if (ui_out_is_mi_like_p (uiout))
+	    ui_out_field_string (uiout, "installed",
+				 loc->inserted ? "y" : "n");
+	  else
+	    {
+	      if (loc->inserted)
+		ui_out_text (uiout, "\t");
+	      else
+		ui_out_text (uiout, "\tnot ");
+	      ui_out_text (uiout, "installed on target\n");
+	    }
+	}
     }
 
   if (ui_out_is_mi_like_p (uiout) && !part_of_multiple)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9ffdb77..cd2f76a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11476,8 +11476,6 @@ tracing:
 @itemize @bullet
 @item
 its passcount as given by the @code{passcount @var{n}} command
-@end itemize
-
 @smallexample
 (@value{GDBP}) @b{info trace}
 Num     Type           Disp Enb Address    What
@@ -11491,6 +11489,23 @@ Num     Type           Disp Enb Address    What
 (@value{GDBP})
 @end smallexample
 
+@item
+the state about installed on target of each location
+@smallexample
+(@value{GDBP}) @b{info trace}
+Num     Type           Disp Enb Address    What
+4       tracepoint     keep y   <MULTIPLE>
+        collect $eip
+4.1                         y     0x0804859c in func4 at change-loc.h:35
+        installed on target
+4.2                         y     0xb7ffc480 in func4 at change-loc.h:35
+        installed on target
+4.3                         y     <PENDING>  set_tracepoint
+5       tracepoint     keep y   0x080485b1 in foo at change-loc.c:29
+        not installed on target
+@end smallexample
+@end itemize
+
 @noindent
 This command can be abbreviated @code{info tp}.
 @end table
@@ -28441,17 +28456,19 @@ The result is in the form:
 ^done,bkpt=@{number="@var{number}",type="@var{type}",disp="del"|"keep",
 enabled="y"|"n",addr="@var{hex}",func="@var{funcname}",file="@var{filename}",
 fullname="@var{full_filename}",line="@var{lineno}",[thread="@var{threadno},]
-times="@var{times}"@}
+times="@var{times}"[,installed="@var{installed}"]@}
 @end smallexample
 
 @noindent
 where @var{number} is the @value{GDBN} number for this breakpoint,
 @var{funcname} is the name of the function where the breakpoint was
 inserted, @var{filename} is the name of the source file which contains
-this function, @var{lineno} is the source line number within that file
-and @var{times} the number of times that the breakpoint has been hit
+this function, @var{lineno} is the source line number within that file,
+@var{times} the number of times that the breakpoint has been hit
 (always 0 for -break-insert but may be greater for -break-info or -break-list
-which use the same output).
+which use the same output), and @var{installed}, which is an optional
+boolean, is about the state of each non-pending tracepoint location
+installed on target or not.
 
 Note: this format is open to change.
 @c An out-of-band breakpoint instead of part of the result?
-- 
1.7.7.6

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Eli Zaretskii]

> Date: Sun, 9 Dec 2012 20:49:20 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 12/07/2012 04:56 AM, Pedro Alves wrote:
> > This should be documented in the manual and NEWS, both the CLI changes, and
> > the MI change (this patch adds the "installed" field to various MI things).
> > 
> 
> The new version includes changes to manual and NEWS.

Thanks.

> +  ** The response to breakpoint commands and breakpoint async records will
> +     include an "installed" field containing a boolean state about each

"includes", not "will include".  NEWS describes features that are
already present, not future features.

> +     non-pending tracepoint location is whether installed on target or not.
>  
>  * GDB now supports the "mini debuginfo" section, .gnu_debugdata.
>    You must have the LZMA library available when configuring GDB for this
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 94874ec..5d5033f 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6123,6 +6123,25 @@ print_one_breakpoint_location (struct breakpoint *b,
>  	  ui_out_field_int (uiout, "pass", t->pass_count);
>  	  ui_out_text (uiout, " \n");
>  	}
> +
> +      /* Don't display it when tracepoint or tracepoint location is
> +	 pending.   */
> +      if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
> +	{
> +	  annotate_field (11);
> +
> +	  if (ui_out_is_mi_like_p (uiout))
> +	    ui_out_field_string (uiout, "installed",
> +				 loc->inserted ? "y" : "n");
> +	  else
> +	    {
> +	      if (loc->inserted)
> +		ui_out_text (uiout, "\t");
> +	      else
> +		ui_out_text (uiout, "\tnot ");
> +	      ui_out_text (uiout, "installed on target\n");
> +	    }
> +	}
>      }
>  
>    if (ui_out_is_mi_like_p (uiout) && !part_of_multiple)
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9ffdb77..cd2f76a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -11476,8 +11476,6 @@ tracing:
>  @itemize @bullet
>  @item
>  its passcount as given by the @code{passcount @var{n}} command
> -@end itemize
> -
>  @smallexample
>  (@value{GDBP}) @b{info trace}
>  Num     Type           Disp Enb Address    What
> @@ -11491,6 +11489,23 @@ Num     Type           Disp Enb Address    What
>  (@value{GDBP})
>  @end smallexample
>  
> +@item
> +the state about installed on target of each location
> +@smallexample
> +(@value{GDBP}) @b{info trace}
> +Num     Type           Disp Enb Address    What
> +4       tracepoint     keep y   <MULTIPLE>
> +        collect $eip
> +4.1                         y     0x0804859c in func4 at change-loc.h:35
> +        installed on target
> +4.2                         y     0xb7ffc480 in func4 at change-loc.h:35
> +        installed on target
> +4.3                         y     <PENDING>  set_tracepoint
> +5       tracepoint     keep y   0x080485b1 in foo at change-loc.c:29
> +        not installed on target
> +@end smallexample
> +@end itemize

A @smallexample inside @itemize is not necessarily a good idea.  And
why have 2 examples, when one will do?  I suggest to have both @items
together, then "@end itemize", and then a single example that shows
both features.

OK with those changes.

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Pedro Alves]

On 12/09/2012 12:49 PM, Yao Qi wrote:
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 94874ec..5d5033f 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6123,6 +6123,25 @@ print_one_breakpoint_location (struct breakpoint *b,
>  	  ui_out_field_int (uiout, "pass", t->pass_count);
>  	  ui_out_text (uiout, " \n");
>  	}
> +
> +      /* Don't display it when tracepoint or tracepoint location is
> +	 pending.   */

I'm not a native speaker, but using a pronoun ("it") as a forward reference
to refer to something that only appears afterwards parses awkwardly to me.
I suggest:

      /* For tracepoints, display the installed state, but not if the tracepoint
         or tracepoint location is pending.   */

> +      if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
> +	{
> +	  annotate_field (11);
> +
> +	  if (ui_out_is_mi_like_p (uiout))
> +	    ui_out_field_string (uiout, "installed",
> +				 loc->inserted ? "y" : "n");
> +	  else
> +	    {
> +	      if (loc->inserted)
> +		ui_out_text (uiout, "\t");
> +	      else
> +		ui_out_text (uiout, "\tnot ");
> +	      ui_out_text (uiout, "installed on target\n");
> +	    }
> +	}
>      }

Wait, what is making sure this is only printed for tracepoints?

-- 
Pedro Alves

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Yao Qi]

On 12/12/2012 01:25 AM, Pedro Alves wrote:
>> +      if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
>> >+	{
>> >+	  annotate_field (11);
>> >+
>> >+	  if (ui_out_is_mi_like_p (uiout))
>> >+	    ui_out_field_string (uiout, "installed",
>> >+				 loc->inserted ? "y" : "n");
>> >+	  else
>> >+	    {
>> >+	      if (loc->inserted)
>> >+		ui_out_text (uiout, "\t");
>> >+	      else
>> >+		ui_out_text (uiout, "\tnot ");
>> >+	      ui_out_text (uiout, "installed on target\n");
>> >+	    }
>> >+	}
>> >      }
> Wait, what is making sure this is only printed for tracepoints?

This part of code 'shares' the same condition check
'if (is_tracepoint (b))' with the code printing 'pass count'.  However, 
it is invisible in the patch itself.

-- 
Yao (齐尧)

Re: [PATCH 1/6] Add a field 'installed' for each location of tracepoint.

[Pedro Alves]

On 12/12/2012 01:54 AM, Yao Qi wrote:
> On 12/12/2012 01:25 AM, Pedro Alves wrote:
>>> +      if (!header_of_multiple && loc != NULL && !loc->shlib_disabled)
>>> >+    {
>>> >+      annotate_field (11);
>>> >+
>>> >+      if (ui_out_is_mi_like_p (uiout))
>>> >+        ui_out_field_string (uiout, "installed",
>>> >+                 loc->inserted ? "y" : "n");
>>> >+      else
>>> >+        {
>>> >+          if (loc->inserted)
>>> >+        ui_out_text (uiout, "\t");
>>> >+          else
>>> >+        ui_out_text (uiout, "\tnot ");
>>> >+          ui_out_text (uiout, "installed on target\n");
>>> >+        }
>>> >+    }
>>> >      }
>> Wait, what is making sure this is only printed for tracepoints?
> 
> This part of code 'shares' the same condition check
> 'if (is_tracepoint (b))' with the code printing 'pass count'.  However, it is invisible in the patch itself.

I see, thanks.

Patch is OK.

-- 
Pedro Alves

[PATCH 5/6] Test tracepoints are installed or not

[Yao Qi]

Test the state of field 'installed'.  This patch should be applied on
top of this one,

  [PATCH] Test on =breakpoint-created when reconnect
  http://sourceware.org/ml/gdb-patches/2012-12/msg00019.html

gdb/testsuite:

2012-12-03  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
	'installed' field in '=breakpoint-modified'.
	(test_reconnect): Check 'installed' field in
	'=breakpoint-modified' and '=breakpoint-created'.
---
 gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
index a4cef0a..f20ea74 100644
--- a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
@@ -130,10 +130,20 @@ proc test_reconnect { } { with_test_prefix "reconnect" {
 	    fail "$test: 2"
 	    exp_continue
 	}
-	-re ".*=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*${mi_gdb_prompt}" {
+	-re ".*=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*installed=\"y\"" {
 	    # Tracepoint on main is created because it is not defined
 	    # before and it is from remote stub.
 	    pass $test
+	    exp_continue
+	}
+	-re ".*=breakpoint-modified,bkpt=\{number=\"2\".*,func=\"marker\".*installed=\"y\"" {
+	    # Tracepoint on marker is defined.  After the sync, we
+	    # know that the tracepoint is in remote stub.  Mark it
+	    # 'installed'.
+	    pass "tracepoint on marker is installed"
+	    exp_continue
+	}
+	-re ".*${mi_gdb_prompt}" {
 	}
 	timeout {
 	    fail $test
@@ -187,8 +197,12 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
 
     set test "tracepoint on pendfunc2 resolved"
     gdb_expect {
-	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
+	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
 	    pass "$test"
+	    exp_continue
+	}
+	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
+	    pass "tracepoint on pendfunc2 installed"
 	}
 	-re ".*${mi_gdb_prompt}$" {
 	    fail $test
@@ -210,7 +224,7 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
     mi_send_resuming_command "exec-continue" "continuing to exit"
     set test "tracepoint on pendfunc2 becomes pending again"
     gdb_expect {
-	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\"" {
+	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\".*installed=\"n\"" {
 	    pass "$test"
 	}
 	-re ".*${mi_gdb_prompt}$" {
-- 
1.7.7.6

Re: [PATCH 5/6] Test tracepoints are installed or not

[Pedro Alves]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> Test the state of field 'installed'.  This patch should be applied on
> top of this one,
> 
>   [PATCH] Test on =breakpoint-created when reconnect
>   http://sourceware.org/ml/gdb-patches/2012-12/msg00019.html
> 
> gdb/testsuite:
> 
> 2012-12-03  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
> 	'installed' field in '=breakpoint-modified'.
> 	(test_reconnect): Check 'installed' field in
> 	'=breakpoint-modified' and '=breakpoint-created'.
> ---
>  gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
> index a4cef0a..f20ea74 100644
> --- a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
> +++ b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
> @@ -130,10 +130,20 @@ proc test_reconnect { } { with_test_prefix "reconnect" {
>  	    fail "$test: 2"
>  	    exp_continue
>  	}
> -	-re ".*=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*${mi_gdb_prompt}" {
> +	-re ".*=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*installed=\"y\"" {
>  	    # Tracepoint on main is created because it is not defined
>  	    # before and it is from remote stub.
>  	    pass $test
> +	    exp_continue
> +	}
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"2\".*,func=\"marker\".*installed=\"y\"" {
> +	    # Tracepoint on marker is defined.  After the sync, we
> +	    # know that the tracepoint is in remote stub.  Mark it
> +	    # 'installed'.
> +	    pass "tracepoint on marker is installed"
> +	    exp_continue
> +	}
> +	-re ".*${mi_gdb_prompt}" {
>  	}
>  	timeout {
>  	    fail $test
> @@ -187,8 +197,12 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
>  
>      set test "tracepoint on pendfunc2 resolved"
>      gdb_expect {
> -	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
>  	    pass "$test"
> +	    exp_continue
> +	}
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
> +	    pass "tracepoint on pendfunc2 installed"
>  	}

I don't understand this part.  Why do we get two notifications?  Different calls to the
breakpoint_modified observers?  This also relates to the "installed" field being present
or not for pending breakpoints issue I pointed out in a previous patch.  It'd be good to
see (and test perhaps) this field is output correctly in the case of a tracepoint with multiple
locations.

>  	-re ".*${mi_gdb_prompt}$" {
>  	    fail $test
> @@ -210,7 +224,7 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
>      mi_send_resuming_command "exec-continue" "continuing to exit"
>      set test "tracepoint on pendfunc2 becomes pending again"
>      gdb_expect {
> -	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\"" {
> +	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\".*installed=\"n\"" {
>  	    pass "$test"
>  	}

Also related.  It very much feels to me that installed or not is a location property, not a
breakpoint property.  So if the breakpoint is pending, it doesn't have any location at all,
and then it feels strange to me to include an "installed" attribute.  WDYT?

>  	-re ".*${mi_gdb_prompt}$" {
> 

-- 
Pedro Alves

Re: [PATCH 5/6] Test tracepoints are installed or not

[Yao Qi]

On 12/07/2012 08:39 PM, Pedro Alves wrote:
> I don't understand this part.  Why do we get two notifications?  Different calls to the
> breakpoint_modified observers?  This also relates to the "installed" field being present
> or not for pending breakpoints issue I pointed out in a previous patch.  It'd be good to

When pending tracepoint is resolved, breakpoint_modified observer is 
notified, and then, when tracepoint is downloaded, the 
breakpoint_modified observer is notified again.

> see (and test perhaps) this field is output correctly in the case of a tracepoint with multiple
> locations.
>

Sure, but it is covered by other test cases, such as 
gdb.trace/change-loc.exp.

>> >  	-re ".*${mi_gdb_prompt}$" {
>> >  	    fail $test
>> >@@ -210,7 +224,7 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
>> >      mi_send_resuming_command "exec-continue" "continuing to exit"
>> >      set test "tracepoint on pendfunc2 becomes pending again"
>> >      gdb_expect {
>> >-	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\"" {
>> >+	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*addr=\"<PENDING>\",.*times=\"0\".*installed=\"n\"" {
>> >  	    pass "$test"
>> >  	}
> Also related.  It very much feels to me that installed or not is a location property, not a
> breakpoint property.  So if the breakpoint is pending, it doesn't have any location at all,
> and then it feels strange to me to include an "installed" attribute.  WDYT?
>

Agreed.  I'll remove it for pending tracepoint.

-- 
Yao (齐尧)

Re: [PATCH 5/6] Test tracepoints are installed or not

[Pedro Alves]

On 12/07/2012 01:55 PM, Yao Qi wrote:
> On 12/07/2012 08:39 PM, Pedro Alves wrote:
>> I don't understand this part.  Why do we get two notifications?  Different calls to the
>> breakpoint_modified observers?  This also relates to the "installed" field being present
>> or not for pending breakpoints issue I pointed out in a previous patch.  It'd be good to
> 
> When pending tracepoint is resolved, breakpoint_modified observer is notified, and then, when tracepoint is downloaded, the breakpoint_modified observer is notified again.

Okay.  Could you add a comment mentioning this in the test?

Thanks,
-- 
Pedro Alves

Re: [PATCH 5/6] Test tracepoints are installed or not

[Yao Qi]

On 12/07/2012 10:20 PM, Pedro Alves wrote:
>> When pending tracepoint is resolved, breakpoint_modified observer is notified, and then, when tracepoint is downloaded, the breakpoint_modified observer is notified again.
> Okay.  Could you add a comment mentioning this in the test?

Sure.  Comments are added in the new version.

-- 
Yao (齐尧)

gdb/testsuite:

2012-12-09  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
	'installed' field in '=breakpoint-modified'.
	(test_reconnect): Check 'installed' field in
	'=breakpoint-modified' and '=breakpoint-created'.
---
 gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
index 2913b12..5955b58 100644
--- a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
@@ -133,10 +133,20 @@ proc test_reconnect { } { with_test_prefix "reconnect" {
 	    fail "$test: 2"
 	    exp_continue
 	}
-	-re "=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*${mi_gdb_prompt}" {
+	-re "=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*installed=\"y\"" {
 	    # A tracepoint on main was defined in the stub, not in GDB,
 	    # so we should see a =breakpoint-created notification.
 	    pass $test
+	    exp_continue
+	}
+	-re "=breakpoint-modified,bkpt=\{number=\"2\".*,func=\"marker\".*installed=\"y\"" {
+	    # Tracepoint on marker is defined.  After the sync, we
+	    # know that the tracepoint is in remote stub.  Mark it
+	    # 'installed'.
+	    pass "tracepoint on marker is installed"
+	    exp_continue
+	}
+	-re ".*${mi_gdb_prompt}" {
 	}
 	timeout {
 	    fail $test
@@ -189,9 +199,16 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
     mi_send_resuming_command "exec-continue" "continuing execution to marker 1"
 
     set test "tracepoint on pendfunc2 resolved"
+    # It is expected to get two "=breakpoint-modified" notifications.
     gdb_expect {
-	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
+	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
+	    # Pending tracepoint is resolved.
 	    pass "$test"
+	    exp_continue
+	}
+	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
+	    # Resolved tracepoint is installed.
+	    pass "tracepoint on pendfunc2 installed"
 	}
 	-re ".*${mi_gdb_prompt}$" {
 	    fail $test
-- 
1.7.7.6

Re: [PATCH 5/6] Test tracepoints are installed or not

[Pedro Alves]

On 12/09/2012 12:53 PM, Yao Qi wrote:
> On 12/07/2012 10:20 PM, Pedro Alves wrote:
>>> >> When pending tracepoint is resolved, breakpoint_modified observer is notified, and then, when tracepoint is downloaded, the breakpoint_modified observer is notified again.
>> > Okay.  Could you add a comment mentioning this in the test?
> Sure.  Comments are added in the new version.

Thanks.

On 12/09/2012 12:53 PM, Yao Qi wrote:
>      set test "tracepoint on pendfunc2 resolved"
> +    # It is expected to get two "=breakpoint-modified" notifications.
>      gdb_expect {
> -	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
> +	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
> +	    # Pending tracepoint is resolved.
>  	    pass "$test"
> +	    exp_continue
> +	}
> +	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
> +	    # Resolved tracepoint is installed.
> +	    pass "tracepoint on pendfunc2 installed"
>  	}
>  	-re ".*${mi_gdb_prompt}$" {
>  	    fail $test

Actually, sorry for not thinking of this before, but, it seems to me
this use of pass+exp_continue is fragile.  Say a GDB bug is
introduced, and the second =breakpoint-modified fails to be output.  In
that case, this will result in:

     PASS: tracepoint on pendfunc2 resolved
     FAIL: tracepoint on pendfunc2 resolved

instead of
     PASS: tracepoint on pendfunc2 resolved
     FAIL: tracepoint on pendfunc2 installed

You could fix that by doing:

 +	    # Pending tracepoint is resolved.
  	    pass "$test"
 +	    set test "tracepoint on pendfunc2 installed"
 +	    exp_continue

But then, if the order of the notifications changes (IOW, due to a bug,
we end up with the tracepoint not installed), this won't catch it.
It seems best to me to only use exp_continue in cases we won't to
consume/skip output, and in the case of this patch, split the two
tests into two consecutive gdb_expects.

-- 
Pedro Alves

Re: [PATCH 5/6] Test tracepoints are installed or not

[Yao Qi]

On 12/12/2012 01:53 AM, Pedro Alves wrote:
> Actually, sorry for not thinking of this before, but, it seems to me

Never mind, your comments are always welcome :)

> this use of pass+exp_continue is fragile.  Say a GDB bug is
> introduced, and the second =breakpoint-modified fails to be output.  In
> that case, this will result in:
> 
>       PASS: tracepoint on pendfunc2 resolved
>       FAIL: tracepoint on pendfunc2 resolved
> 
> instead of
>       PASS: tracepoint on pendfunc2 resolved
>       FAIL: tracepoint on pendfunc2 installed
> 

My intention of using exp_coutine here is to handle the situation that
notifications may arrive in different orders.  The fail message is not
clear, but not a big deal, to me.  It is important to catch a fail and
then we can check gdb.log and test case to fix it.

> You could fix that by doing:
> 
>   +	    # Pending tracepoint is resolved.
>    	    pass "$test"
>   +	    set test "tracepoint on pendfunc2 installed"
>   +	    exp_continue
> 
> But then, if the order of the notifications changes (IOW, due to a bug,
> we end up with the tracepoint not installed), this won't catch it.

Looks the fix is worse than the original one, at least no fail is
missing in the original one.

> It seems best to me to only use exp_continue in cases we won't to
> consume/skip output, and in the case of this patch, split the two
> tests into two consecutive gdb_expects.

If this way, we can't handle that two notifications arrive in a
reversed order (which is also correct).  If we don't have to worry
about the order (after all, the order of MI notifications are still
deterministic nowadays), I am OK with your suggestion.

-- 
Yao (齐尧)

gdb/testsuite:

2012-12-12  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
	'installed' field in '=breakpoint-modified'.
	(test_reconnect): Check 'installed' field in
	'=breakpoint-modified' and '=breakpoint-created'.
---
 gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   27 ++++++++++++++-------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
index 2913b12..0073d4b 100644
--- a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
@@ -133,13 +133,18 @@ proc test_reconnect { } { with_test_prefix "reconnect" {
 	    fail "$test: 2"
 	    exp_continue
 	}
-	-re "=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*${mi_gdb_prompt}" {
+	-re "=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*installed=\"y\"" {
 	    # A tracepoint on main was defined in the stub, not in GDB,
 	    # so we should see a =breakpoint-created notification.
 	    pass $test
 	}
-	timeout {
-	    fail $test
+    }
+    # Tracepoint on marker is defined.  After the sync, we know that
+    # the tracepoint is in remote stub.  Mark it 'installed'.
+    set test "tracepoint on marker is installed"
+    gdb_expect {
+	-re "=breakpoint-modified,bkpt=\{number=\"2\".*,func=\"marker\".*installed=\"y\".*${mi_gdb_prompt}$" {
+	    pass "$test"
 	}
     }
     # Check that tracepoint 1 is still pending.
@@ -188,18 +193,22 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
 
     mi_send_resuming_command "exec-continue" "continuing execution to marker 1"
 
+    # It is expected to get two "=breakpoint-modified" notifications.
+    # Pending tracepoint is resolved.
     set test "tracepoint on pendfunc2 resolved"
     gdb_expect {
-	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
+	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
 	    pass "$test"
 	}
-	-re ".*${mi_gdb_prompt}$" {
-	    fail $test
-	}
-	timeout {
-	    fail "$test (timeout)"
+    }
+    # Resolved tracepoint is installed.
+    set test "tracepoint on pendfunc2 installed"
+    gdb_expect {
+	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
+	    pass "$test"
 	}
     }
+
     mi_expect_stop "breakpoint-hit" "marker" ".*" ".*" ".*" \
 	{"" "disp=\"keep\""} "continue to marker"
 
-- 
1.7.7.6

Re: [PATCH 5/6] Test tracepoints are installed or not

[Pedro Alves]

On 12/12/2012 02:59 AM, Yao Qi wrote:
> On 12/12/2012 01:53 AM, Pedro Alves wrote:
>>
>> this use of pass+exp_continue is fragile.  Say a GDB bug is
>> introduced, and the second =breakpoint-modified fails to be output.  In
>> that case, this will result in:
>>
>>       PASS: tracepoint on pendfunc2 resolved
>>       FAIL: tracepoint on pendfunc2 resolved
>>
>> instead of
>>       PASS: tracepoint on pendfunc2 resolved
>>       FAIL: tracepoint on pendfunc2 installed
>>
> 
> My intention of using exp_coutine here is to handle the situation that
> notifications may arrive in different orders.  The fail message is not
> clear, but not a big deal, to me.  It is important to catch a fail and
> then we can check gdb.log and test case to fix it.

The text of the fail message isn't that important, I agree, but
ending up with the _same_ text for both the PASS and the FAIL (the whole
duplicate messages issue, PR13443) is something we're actively
trying to avoid.

> 
>> You could fix that by doing:
>>
>>   +	    # Pending tracepoint is resolved.
>>    	    pass "$test"
>>   +	    set test "tracepoint on pendfunc2 installed"
>>   +	    exp_continue
>>
>> But then, if the order of the notifications changes (IOW, due to a bug,
>> we end up with the tracepoint not installed), this won't catch it.
> 
> Looks the fix is worse than the original one, at least no fail is
> missing in the original one.

Not sure what you mean.  The only change compared to the original one
would be the new

	    set test "tracepoint on pendfunc2 installed"

line.  Sorry if that wasn't clear.  (you'd probably change the other pass
to be 'pass "$test"' too then.)

> 
>> It seems best to me to only use exp_continue in cases we won't to
>> consume/skip output, and in the case of this patch, split the two
>> tests into two consecutive gdb_expects.
> 
> If this way, we can't handle that two notifications arrive in a
> reversed order (which is also correct).  

This seems to be crux of the issue here.  I don't understand how
reverse order would be correct.  This:

 =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="n"
 =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="y"

means the frontend ends up thinking the tracepoint is installed, while this:

 =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="y"
 =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="n"

means the frontend ends up thinking the tracepoint is not installed, which
I'd think is not what we want in this test.

> If we don't have to worry about the order 

Confused, because I'm suggesting to worry about the order.  :-)

> (after all, the order of MI notifications are still
> deterministic nowadays), I am OK with your suggestion.

> 2012-12-12  Yao Qi  <yao@codesourcery.com>
>
> 	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
> 	'installed' field in '=breakpoint-modified'.
> 	(test_reconnect): Check 'installed' field in
> 	'=breakpoint-modified' and '=breakpoint-created'.

This version looks good to me.  Thanks,

-- 
Pedro Alves

Re: [PATCH 5/6] Test tracepoints are installed or not

[Yao Qi]

On 12/12/2012 08:23 PM, Pedro Alves wrote:
> Not sure what you mean.  The only change compared to the original one
> would be the new
>
> 	    set test "tracepoint on pendfunc2 installed"
>
> line.  Sorry if that wasn't clear.  (you'd probably change the other pass
> to be 'pass "$test"' too then.)
>

OK, I get your points now.

>> >
>>> >>It seems best to me to only use exp_continue in cases we won't to
>>> >>consume/skip output, and in the case of this patch, split the two
>>> >>tests into two consecutive gdb_expects.
>> >
>> >If this way, we can't handle that two notifications arrive in a
>> >reversed order (which is also correct).
> This seems to be crux of the issue here.  I don't understand how
> reverse order would be correct.  This:
>
>   =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="n"
>   =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="y"
>
> means the frontend ends up thinking the tracepoint is installed, while this:
>
>   =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="y"
>   =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="n"
>
> means the frontend ends up thinking the tracepoint is not installed, which
> I'd think is not what we want in this test.
>

I linked this with other issue I am thinking about recently.  It is 
about 'gdb may send notifications on breakpoint 1 and breakpoint 2 in 
different orders', and how to write test to handle.  Sorry for the 
confusion and messing up the discussion.

Again, thanks for your patient review, Pedro.

-- 
Yao (齐尧)

[PATCH 6/6] Update test cases for 'installed' field.

[Yao Qi]

With the new field 'installed', tests depends on the output of 'info
trace' should be updated.

gdb/testsuite:

2012-12-03  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/actions.exp: Update test for 'installed' field.
	* gdb.trace/change-loc.exp (tracepoint_change_loc_1):
	(tracepoint_change_loc_2): Likewise.
	* gdb.trace/deltrace.exp: Likewise.
	* gdb.trace/infotrace.exp: Likewise.
	* gdb.trace/mi-traceframe-changed.exp (test_tfind_remote):
	Likewise.
	* gdb.trace/passcount.exp: Likewise.
	* gdb.trace/tracecmd.exp: Likewise.
	* gdb.trace/while-stepping.exp: Likewise.
---
 gdb/testsuite/gdb.trace/actions.exp               |   25 +++++++++++++++---
 gdb/testsuite/gdb.trace/change-loc.exp            |    6 +++-
 gdb/testsuite/gdb.trace/deltrace.exp              |   22 ++++++++++++---
 gdb/testsuite/gdb.trace/infotrace.exp             |   10 +++++--
 gdb/testsuite/gdb.trace/mi-traceframe-changed.exp |    3 +-
 gdb/testsuite/gdb.trace/passcount.exp             |   28 +++++++++++++++------
 gdb/testsuite/gdb.trace/tracecmd.exp              |   13 ++++++---
 gdb/testsuite/gdb.trace/while-stepping.exp        |    3 +-
 8 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/actions.exp b/gdb/testsuite/gdb.trace/actions.exp
index 5e2cb12..0345f0d 100644
--- a/gdb/testsuite/gdb.trace/actions.exp
+++ b/gdb/testsuite/gdb.trace/actions.exp
@@ -80,8 +80,11 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_char_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 		"5.1c: verify actions set for first tracepoint"
 
 gdb_trace_setactions "5.1d: set actions for second tracepoint" \
@@ -92,9 +95,12 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_char_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_short_test.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 		"5.1e: verify actions set for second tracepoint"
 
 gdb_trace_setactions "5.2a: set actions for last (default) tracepoint" \
@@ -105,10 +111,13 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_char_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_short_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+collect gdb_long_test." \
+\[\t \]+collect gdb_long_test.
+\[\t \]+installed n ." \
 		"5.2b: verify actions set for second tracepoint"
 
 # 5.3 replace actions set earlier
@@ -121,10 +130,13 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_struct1_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_short_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+collect gdb_long_test." \
+\[\t \]+collect gdb_long_test.
+\[\t \]+installed n ." \
 		"5.3b: verify actions set for first tracepoint"
 
 #
@@ -214,9 +226,12 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+teval gdb_char_test.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+teval \\\$tsv \\\+= 1.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+collect gdb_long_test." \
+\[\t \]+collect gdb_long_test.
+\[\t \]+installed n ." \
 		"5.10a: verify teval actions set for two tracepoints"
 
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index 0064b6f..fa8d09b 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -154,7 +154,8 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
     # shlib is unloaded, there are still three locations, but one is pending.
     gdb_test "info trace" \
         "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*4\.1.* in func4.*(4\.2.* in func4.*4\.3.* \<PENDING\>\[\t \]+set_tracepoint|4\.2.* \<PENDING\>\[\t \]+set_tracepoint\r\n4\.3.* in func4).*" \
+\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*
+4\.1.* in func4.*installed y \r\n(4\.2.* in func4.*installed y \r\n4\.3.* \<PENDING\>\[\t \]+set_tracepoint.*installed n |4\.2.* \<PENDING\>\[\t \]+set_tracepoint.*installed n \r\n4\.3.* in func4.*installed y).*" \
         "tracepoint with two locations (unload)"
 
     gdb_test_no_output "tstop"
@@ -258,7 +259,8 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
     # shlib is unloaded, there are still three locations, but one is pending.
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*1\.1.* in func4.*(1\.2.* in func4.*1\.3.* \<PENDING\>\[\t \]+set_tracepoint|1\.2.* \<PENDING\>\[\t \]+set_tracepoint\r\n1\.3.* in func4).*" \
+\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*
+1\.1.* in func4.*installed y \r\n(1\.2.* in func4.*installed y \r\n1\.3.* \<PENDING\>\[\t \]+set_tracepoint\r\n\[\t \]+installed n|1\.2.* \<PENDING\>\[\t \]+set_tracepoint\r\n\[\t \]+installed n \r\n1\.3.* in func4.*installed y ).*" \
 	"tracepoint with two locations (unload)"
 
     gdb_test_no_output "tstop"
diff --git a/gdb/testsuite/gdb.trace/deltrace.exp b/gdb/testsuite/gdb.trace/deltrace.exp
index 2423de2..e7143a0 100644
--- a/gdb/testsuite/gdb.trace/deltrace.exp
+++ b/gdb/testsuite/gdb.trace/deltrace.exp
@@ -56,8 +56,11 @@ gdb_test "trace $testline1"   "Tracepoint \[0-9\]+ at .*" "set tracepoint 3"
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 			"3.1a: set three tracepoints"
 
 gdb_test "delete tracepoints" \
@@ -80,8 +83,11 @@ if { $trcpt1 <= 0 || $trcpt2 <= 0 || $trcpt3 <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 			"3.2a: set three tracepoints"
 
 #gdb_test_no_output "delete tracepoint $trcpt1" ""
@@ -100,7 +106,9 @@ gdb_test_multiple "delete tracepoint $trcpt1" "3.2b: delete first tracepoint" {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 			"3.2c: verify delete first tracepoint"
 
 #gdb_test_no_output "delete tracepoint $trcpt2" ""
@@ -118,7 +126,8 @@ gdb_test_multiple "delete tracepoint $trcpt2" "3.2d: delete second tracepoint" {
 
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 			"3.2e: verify delete second tracepoint"
 
 #gdb_test_no_output "delete tracepoint $trcpt3" ""
@@ -152,8 +161,11 @@ if { $trcpt1 <= 0 || $trcpt2 <= 0 || $trcpt3 <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 			"3.3a: set three tracepoints"
 
 #gdb_test_no_output "delete tracepoint $trcpt1 $trcpt2 $trcpt3" ""
diff --git a/gdb/testsuite/gdb.trace/infotrace.exp b/gdb/testsuite/gdb.trace/infotrace.exp
index bdc3830..4dac614 100644
--- a/gdb/testsuite/gdb.trace/infotrace.exp
+++ b/gdb/testsuite/gdb.trace/infotrace.exp
@@ -50,18 +50,22 @@ if { $c_test_num <= 0 || $asm_test_num <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+." \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
     "2.1: info tracepoints (all)"
 
 # 2.2 info tracepoint (specific)
 gdb_test "info tracepoint $c_test_num" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+." \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
     "2.2a: info tracepoint $c_test_num (gdb_c_test)"
 
 gdb_test "info tracepoint $asm_test_num" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+." \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
     "2.2b: info tracepoint $asm_test_num (gdb_asm_test)"
 
 # 2.3 info tracepoint (invalid tracepoint number)
diff --git a/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp b/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
index b587d10..60e7076 100644
--- a/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
@@ -109,7 +109,8 @@ proc test_tfind_remote { } { with_test_prefix "remote" {
 
     mi_gdb_test "-break-insert end" "\\^done.*" "break end"
     mi_gdb_test "-break-insert -a func2" "\\^done.*" "break func2"
-    mi_gdb_test "-trace-start" "\\^done.*" "trace start"
+    mi_gdb_test "-trace-start" "=breakpoint-modified,bkpt={.*installed=\"y\".*}.*\\^done.*" \
+	"trace start"
 
     mi_execute_to "exec-continue" "breakpoint-hit" end "" ".*" ".*" \
 	{ "" "disp=\"keep\"" } \
diff --git a/gdb/testsuite/gdb.trace/passcount.exp b/gdb/testsuite/gdb.trace/passcount.exp
index 42f767a..9f27131 100644
--- a/gdb/testsuite/gdb.trace/passcount.exp
+++ b/gdb/testsuite/gdb.trace/passcount.exp
@@ -61,8 +61,11 @@ if { $trcpt1 <= 0 || $trcpt2 <= 0 || $trcpt3 <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 			"4.1a: set three tracepoints, passcounts all zero"
 
 gdb_test "passcount 2 $trcpt1" \
@@ -72,9 +75,12 @@ gdb_test "passcount 2 $trcpt1" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 2 .*
+\[\t \]+pass count 2 .
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 	"4.1c: verify 1st tracepoint's passcount set to two"
 
 gdb_test "passcount 4 $trcpt2" \
@@ -84,10 +90,13 @@ gdb_test "passcount 4 $trcpt2" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 2 .*
+\[\t \]+pass count 2 .
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 4 .*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+pass count 4 .
+\[\t \]+installed n .
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 	"4.1c: verify 2nd tracepoint's passcount set to four"
 
 # 4.2 passcount of last (default) tracepoint
@@ -181,10 +190,13 @@ gdb_test "passcount 0 $trcpt1" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 4 .*
+\[\t \]+pass count 4 .
+\[\t \]+installed n .
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 4 .*"  \
+\[\t \]+pass count 4 .
+\[\t \]+installed n ."  \
 	"4.6: set passcount to zero"
 
 # 4.7 (test a very large passcount)
diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
index b4fc3e2..4b768b7 100644
--- a/gdb/testsuite/gdb.trace/tracecmd.exp
+++ b/gdb/testsuite/gdb.trace/tracecmd.exp
@@ -62,7 +62,8 @@ gdb_delete_tracepoints
 gdb_test "trace $srcfile:$testline2" \
 	"Tracepoint $decimal at $hex: file.*$srcfile, line $testline2." \
 	"1.1a: set tracepoint at sourceline"
-gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2" \
+gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
+\[\t \]+installed n ." \
 	"1.1b: trace sourcefile:line"
 
 # 1.2 trace invalid source line
@@ -85,7 +86,8 @@ gdb_delete_tracepoints
 gdb_test "trace gdb_recursion_test" \
 	"Tracepoint $decimal at $hex: file.*$srcfile, line $testline1." \
 	"1.4a: trace function by name"
-gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1" \
+gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1.
+\[\t \]+installed n ." \
 	"1.4b: trace function by name"
 
 # 1.5 trace non-existant function
@@ -122,7 +124,8 @@ gdb_delete_tracepoints
 gdb_test "trace \*gdb_recursion_test" \
 	"Tracepoint $decimal at .*$c_test_addr.*" \
 	"1.7a: trace at function label (before prologue)"
-gdb_test "info trace" "$c_test_addr.*in gdb_recursion_test.*:$baseline" \
+gdb_test "info trace" "$c_test_addr.*in gdb_recursion_test.*:$baseline.
+\[\t \]+installed n ." \
 	"1.7b: verify trace at specific address"
 
 # 1.8 trace at invalid address
@@ -139,7 +142,9 @@ gdb_delete_tracepoints
 gdb_test "trace gdb_recursion_test if q1 > 0" \
 	"Tracepoint $decimal at $hex: file.*$srcfile, line $testline1." \
 	"1.11a: conditional tracepoint"
-gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1.*trace only if q1 > 0" \
+gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1.
+\[\t \]+trace only if q1 > 0.
+\[\t \]+installed n ." \
 	"1.11b: verify conditional tracepoint"
 
 # 1.12 set tracepoint in prologue
diff --git a/gdb/testsuite/gdb.trace/while-stepping.exp b/gdb/testsuite/gdb.trace/while-stepping.exp
index 5d8e066..c8a603c 100644
--- a/gdb/testsuite/gdb.trace/while-stepping.exp
+++ b/gdb/testsuite/gdb.trace/while-stepping.exp
@@ -49,7 +49,8 @@ if { $trcpt1 <= 0 } then {
 
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+" \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+installed n ." \
 	"5.12: set a tracepoint, stepcount is zero"
 
 set stepcount 12
-- 
1.7.7.6

Re: [PATCH 6/6] Update test cases for 'installed' field.

[Pedro Alves]

I'll wait for resolution in the other patches before looking at this one.

-- 
Pedro Alves

Re: [PATCH 6/6] Update test cases for 'installed' field.

[Yao Qi]

On 12/07/2012 08:42 PM, Pedro Alves wrote:
> I'll wait for resolution in the other patches before looking at this one.
> 

Here is the updated tests to reflect the changes of GDB output.  Is it OK?

-- 
Yao (齐尧)

gdb/testsuite:

2012-12-13  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/actions.exp: Update test for 'installed' field.
	* gdb.trace/change-loc.exp (tracepoint_change_loc_1):
	(tracepoint_change_loc_2): Likewise.
	Check 'info tracepoint' display nothing else.
	* gdb.trace/deltrace.exp: Likewise.
	* gdb.trace/infotrace.exp: Likewise.
	* gdb.trace/mi-traceframe-changed.exp (test_tfind_remote):
	Likewise.
	* gdb.trace/passcount.exp: Likewise.
	* gdb.trace/tracecmd.exp: Likewise.
	* gdb.trace/while-stepping.exp: Likewise.
---
 gdb/testsuite/gdb.trace/actions.exp               |   25 +++++++++++++++---
 gdb/testsuite/gdb.trace/change-loc.exp            |   11 +++++---
 gdb/testsuite/gdb.trace/deltrace.exp              |   22 ++++++++++++---
 gdb/testsuite/gdb.trace/infotrace.exp             |   10 +++++--
 gdb/testsuite/gdb.trace/mi-traceframe-changed.exp |    3 +-
 gdb/testsuite/gdb.trace/passcount.exp             |   28 +++++++++++++++------
 gdb/testsuite/gdb.trace/tracecmd.exp              |   13 ++++++---
 gdb/testsuite/gdb.trace/while-stepping.exp        |    3 +-
 8 files changed, 84 insertions(+), 31 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/actions.exp b/gdb/testsuite/gdb.trace/actions.exp
index 5e2cb12..c05ee2f 100644
--- a/gdb/testsuite/gdb.trace/actions.exp
+++ b/gdb/testsuite/gdb.trace/actions.exp
@@ -80,8 +80,11 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_char_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 		"5.1c: verify actions set for first tracepoint"
 
 gdb_trace_setactions "5.1d: set actions for second tracepoint" \
@@ -92,9 +95,12 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_char_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_short_test.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 		"5.1e: verify actions set for second tracepoint"
 
 gdb_trace_setactions "5.2a: set actions for last (default) tracepoint" \
@@ -105,10 +111,13 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_char_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_short_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+collect gdb_long_test." \
+\[\t \]+collect gdb_long_test.
+\[\t \]+not installed on target." \
 		"5.2b: verify actions set for second tracepoint"
 
 # 5.3 replace actions set earlier
@@ -121,10 +130,13 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_struct1_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+collect gdb_short_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+collect gdb_long_test." \
+\[\t \]+collect gdb_long_test.
+\[\t \]+not installed on target." \
 		"5.3b: verify actions set for first tracepoint"
 
 #
@@ -214,9 +226,12 @@ gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
 \[\t \]+teval gdb_char_test.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
 \[\t \]+teval \\\$tsv \\\+= 1.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+collect gdb_long_test." \
+\[\t \]+collect gdb_long_test.
+\[\t \]+not installed on target." \
 		"5.10a: verify teval actions set for two tracepoints"
 
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index 0064b6f..a17c54c 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -154,7 +154,8 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
     # shlib is unloaded, there are still three locations, but one is pending.
     gdb_test "info trace" \
         "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*4\.1.* in func4.*(4\.2.* in func4.*4\.3.* \<PENDING\>\[\t \]+set_tracepoint|4\.2.* \<PENDING\>\[\t \]+set_tracepoint\r\n4\.3.* in func4).*" \
+\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*
+4\.1.* in func4.*\tinstalled on target\r\n(4\.2.* in func4.*\tinstalled on target\r\n4\.3.* \<PENDING\>\[\t \]+set_tracepoint|4\.2.* \<PENDING\>\[\t \]+set_tracepoint.*4\.3.* in func4.*\tinstalled on target).*" \
         "tracepoint with two locations (unload)"
 
     gdb_test_no_output "tstop"
@@ -186,10 +187,11 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
     gdb_trace_setactions "set action for tracepoint" "" \
 	"collect \$$pcreg" "^$"
 
-    # tracepoint has no location information now.
+    # tracepoint has no location information now.  Make sure nothing
+    # else is displayed.
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*PENDING.*set_tracepoint.*" \
+\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*PENDING.*set_tracepoint\r\n\[\t \]+collect \\$$pcreg\r" \
 	"single pending tracepoint info (without symbols)"
 
     gdb_load ${binfile}
@@ -258,7 +260,8 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
     # shlib is unloaded, there are still three locations, but one is pending.
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*1\.1.* in func4.*(1\.2.* in func4.*1\.3.* \<PENDING\>\[\t \]+set_tracepoint|1\.2.* \<PENDING\>\[\t \]+set_tracepoint\r\n1\.3.* in func4).*" \
+\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*
+1\.1.* in func4.*\tinstalled on target\r\n(1\.2.* in func4.*\tinstalled on target\r\n1\.3.* \<PENDING\>\[\t \]+set_tracepoint|1\.2.* \<PENDING\>\[\t \]+set_tracepoint\r\n1\.3.* in func4.*\tinstalled on target).*" \
 	"tracepoint with two locations (unload)"
 
     gdb_test_no_output "tstop"
diff --git a/gdb/testsuite/gdb.trace/deltrace.exp b/gdb/testsuite/gdb.trace/deltrace.exp
index 2423de2..53bbc7f 100644
--- a/gdb/testsuite/gdb.trace/deltrace.exp
+++ b/gdb/testsuite/gdb.trace/deltrace.exp
@@ -56,8 +56,11 @@ gdb_test "trace $testline1"   "Tracepoint \[0-9\]+ at .*" "set tracepoint 3"
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 			"3.1a: set three tracepoints"
 
 gdb_test "delete tracepoints" \
@@ -80,8 +83,11 @@ if { $trcpt1 <= 0 || $trcpt2 <= 0 || $trcpt3 <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 			"3.2a: set three tracepoints"
 
 #gdb_test_no_output "delete tracepoint $trcpt1" ""
@@ -100,7 +106,9 @@ gdb_test_multiple "delete tracepoint $trcpt1" "3.2b: delete first tracepoint" {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 			"3.2c: verify delete first tracepoint"
 
 #gdb_test_no_output "delete tracepoint $trcpt2" ""
@@ -118,7 +126,8 @@ gdb_test_multiple "delete tracepoint $trcpt2" "3.2d: delete second tracepoint" {
 
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 			"3.2e: verify delete second tracepoint"
 
 #gdb_test_no_output "delete tracepoint $trcpt3" ""
@@ -152,8 +161,11 @@ if { $trcpt1 <= 0 || $trcpt2 <= 0 || $trcpt3 <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 			"3.3a: set three tracepoints"
 
 #gdb_test_no_output "delete tracepoint $trcpt1 $trcpt2 $trcpt3" ""
diff --git a/gdb/testsuite/gdb.trace/infotrace.exp b/gdb/testsuite/gdb.trace/infotrace.exp
index bdc3830..abb1dc8 100644
--- a/gdb/testsuite/gdb.trace/infotrace.exp
+++ b/gdb/testsuite/gdb.trace/infotrace.exp
@@ -50,18 +50,22 @@ if { $c_test_num <= 0 || $asm_test_num <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+." \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
     "2.1: info tracepoints (all)"
 
 # 2.2 info tracepoint (specific)
 gdb_test "info tracepoint $c_test_num" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+." \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
     "2.2a: info tracepoint $c_test_num (gdb_c_test)"
 
 gdb_test "info tracepoint $asm_test_num" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+." \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
     "2.2b: info tracepoint $asm_test_num (gdb_asm_test)"
 
 # 2.3 info tracepoint (invalid tracepoint number)
diff --git a/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp b/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
index b587d10..60e7076 100644
--- a/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-traceframe-changed.exp
@@ -109,7 +109,8 @@ proc test_tfind_remote { } { with_test_prefix "remote" {
 
     mi_gdb_test "-break-insert end" "\\^done.*" "break end"
     mi_gdb_test "-break-insert -a func2" "\\^done.*" "break func2"
-    mi_gdb_test "-trace-start" "\\^done.*" "trace start"
+    mi_gdb_test "-trace-start" "=breakpoint-modified,bkpt={.*installed=\"y\".*}.*\\^done.*" \
+	"trace start"
 
     mi_execute_to "exec-continue" "breakpoint-hit" end "" ".*" ".*" \
 	{ "" "disp=\"keep\"" } \
diff --git a/gdb/testsuite/gdb.trace/passcount.exp b/gdb/testsuite/gdb.trace/passcount.exp
index 42f767a..fffeda2 100644
--- a/gdb/testsuite/gdb.trace/passcount.exp
+++ b/gdb/testsuite/gdb.trace/passcount.exp
@@ -61,8 +61,11 @@ if { $trcpt1 <= 0 || $trcpt2 <= 0 || $trcpt3 <= 0 } then {
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 			"4.1a: set three tracepoints, passcounts all zero"
 
 gdb_test "passcount 2 $trcpt1" \
@@ -72,9 +75,12 @@ gdb_test "passcount 2 $trcpt1" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 2 .*
+\[\t \]+pass count 2 .
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 	"4.1c: verify 1st tracepoint's passcount set to two"
 
 gdb_test "passcount 4 $trcpt2" \
@@ -84,10 +90,13 @@ gdb_test "passcount 4 $trcpt2" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 2 .*
+\[\t \]+pass count 2 .
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 4 .*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+" \
+\[\t \]+pass count 4 .
+\[\t \]+not installed on target.
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 	"4.1c: verify 2nd tracepoint's passcount set to four"
 
 # 4.2 passcount of last (default) tracepoint
@@ -181,10 +190,13 @@ gdb_test "passcount 0 $trcpt1" \
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_asm_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 4 .*
+\[\t \]+pass count 4 .
+\[\t \]+not installed on target.
 \[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_recursion_test at .*$srcfile:\[0-9\]+.
-\[\t \]+pass count 4 .*"  \
+\[\t \]+pass count 4 .
+\[\t \]+not installed on target."  \
 	"4.6: set passcount to zero"
 
 # 4.7 (test a very large passcount)
diff --git a/gdb/testsuite/gdb.trace/tracecmd.exp b/gdb/testsuite/gdb.trace/tracecmd.exp
index b4fc3e2..2b0a96a 100644
--- a/gdb/testsuite/gdb.trace/tracecmd.exp
+++ b/gdb/testsuite/gdb.trace/tracecmd.exp
@@ -62,7 +62,8 @@ gdb_delete_tracepoints
 gdb_test "trace $srcfile:$testline2" \
 	"Tracepoint $decimal at $hex: file.*$srcfile, line $testline2." \
 	"1.1a: set tracepoint at sourceline"
-gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2" \
+gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline2.
+\[\t \]+not installed on target." \
 	"1.1b: trace sourcefile:line"
 
 # 1.2 trace invalid source line
@@ -85,7 +86,8 @@ gdb_delete_tracepoints
 gdb_test "trace gdb_recursion_test" \
 	"Tracepoint $decimal at $hex: file.*$srcfile, line $testline1." \
 	"1.4a: trace function by name"
-gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1" \
+gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1.
+\[\t \]+not installed on target." \
 	"1.4b: trace function by name"
 
 # 1.5 trace non-existant function
@@ -122,7 +124,8 @@ gdb_delete_tracepoints
 gdb_test "trace \*gdb_recursion_test" \
 	"Tracepoint $decimal at .*$c_test_addr.*" \
 	"1.7a: trace at function label (before prologue)"
-gdb_test "info trace" "$c_test_addr.*in gdb_recursion_test.*:$baseline" \
+gdb_test "info trace" "$c_test_addr.*in gdb_recursion_test.*:$baseline.
+\[\t \]+not installed on target." \
 	"1.7b: verify trace at specific address"
 
 # 1.8 trace at invalid address
@@ -139,7 +142,9 @@ gdb_delete_tracepoints
 gdb_test "trace gdb_recursion_test if q1 > 0" \
 	"Tracepoint $decimal at $hex: file.*$srcfile, line $testline1." \
 	"1.11a: conditional tracepoint"
-gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1.*trace only if q1 > 0" \
+gdb_test "info trace" "in gdb_recursion_test.*$srcfile:$testline1.
+\[\t \]+trace only if q1 > 0.
+\[\t \]+not installed on target." \
 	"1.11b: verify conditional tracepoint"
 
 # 1.12 set tracepoint in prologue
diff --git a/gdb/testsuite/gdb.trace/while-stepping.exp b/gdb/testsuite/gdb.trace/while-stepping.exp
index 5d8e066..f926d75 100644
--- a/gdb/testsuite/gdb.trace/while-stepping.exp
+++ b/gdb/testsuite/gdb.trace/while-stepping.exp
@@ -49,7 +49,8 @@ if { $trcpt1 <= 0 } then {
 
 gdb_test "info tracepoints" \
     "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+" \
+\[0-9\]+\[\t \]+tracepoint     keep y.* in gdb_c_test at .*$srcfile:\[0-9\]+.
+\[\t \]+not installed on target." \
 	"5.12: set a tracepoint, stepcount is zero"
 
 set stepcount 12
-- 
1.7.7.6

Re: [PATCH 6/6] Update test cases for 'installed' field.

[Pedro Alves]

On 12/13/2012 12:06 PM, Yao Qi wrote:
> 2012-12-13  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.trace/actions.exp: Update test for 'installed' field.
> 	* gdb.trace/change-loc.exp (tracepoint_change_loc_1):
> 	(tracepoint_change_loc_2): Likewise.
> 	Check 'info tracepoint' display nothing else.
> 	* gdb.trace/deltrace.exp: Likewise.
> 	* gdb.trace/infotrace.exp: Likewise.
> 	* gdb.trace/mi-traceframe-changed.exp (test_tfind_remote):
> 	Likewise.
> 	* gdb.trace/passcount.exp: Likewise.
> 	* gdb.trace/tracecmd.exp: Likewise.
> 	* gdb.trace/while-stepping.exp: Likewise.

Looks fine to me.  Thanks.

-- 
Pedro Alves

[PATCH 2/6] Iterate over ALL_TRACEPOINTS first.

[Yao Qi]

Hi,
The 'breakpoint-modified' observer is notified on the breakpoint
level, in order to notify observer for a given tracepoint only once,
we change to iterate over tracepoint first, and then iterate over
locations of each tracepoint.  In the inner loop, when we find a
tracepoint location is downloaded, we mark a flag, and notify the
observer in outer loop (on tracepoints) if flag is true.  In short, we
change the iteration from:

  ALL_BP_LOCATIONS (bl, blp_tmp)
   {
     if (!is_tracepoint (bl->owner))
       continue;
   }

to:

  ALL_TRACEPOINTS (b)
   {
      for (bl = b->loc; bl; bl = bl->next)
        {}
   }

download_tracepoint_locations is called after breakpoint re-setting,
so I think this change has no functional affect.

gdb:

2012-12-03  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (download_tracepoint_locations): Iterate over
	ALL_TRACEPOINTS first and then iterate over locations of
	each tracepoint.
---
 gdb/breakpoint.c |   33 +++++++++++++++++----------------
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 40d2edd..3f2f0c9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12078,7 +12078,7 @@ bp_location_target_extensions_update (void)
 static void
 download_tracepoint_locations (void)
 {
-  struct bp_location *bl, **blp_tmp;
+  struct breakpoint *b;
   struct cleanup *old_chain;
 
   if (!target_can_download_tracepoint ())
@@ -12086,31 +12086,32 @@ download_tracepoint_locations (void)
 
   old_chain = save_current_space_and_thread ();
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  ALL_TRACEPOINTS (b)
     {
+      struct bp_location *bl;
       struct tracepoint *t;
 
-      if (!is_tracepoint (bl->owner))
-	continue;
-
-      if ((bl->owner->type == bp_fast_tracepoint
+      if ((b->type == bp_fast_tracepoint
 	   ? !may_insert_fast_tracepoints
 	   : !may_insert_tracepoints))
 	continue;
 
-      /* In tracepoint, locations are _never_ duplicated, so
-	 should_be_inserted is equivalent to
-	 unduplicated_should_be_inserted.  */
-      if (!should_be_inserted (bl) || bl->inserted)
-	continue;
+      for (bl = b->loc; bl; bl = bl->next)
+	{
+	  /* In tracepoint, locations are _never_ duplicated, so
+	     should_be_inserted is equivalent to
+	     unduplicated_should_be_inserted.  */
+	  if (!should_be_inserted (bl) || bl->inserted)
+	    continue;
 
-      switch_to_program_space_and_thread (bl->pspace);
+	  switch_to_program_space_and_thread (bl->pspace);
 
-      target_download_tracepoint (bl);
+	  target_download_tracepoint (bl);
 
-      bl->inserted = 1;
-      t = (struct tracepoint *) bl->owner;
-      t->number_on_target = bl->owner->number;
+	  bl->inserted = 1;
+	}
+      t = (struct tracepoint *) b;
+      t->number_on_target = b->number;
     }
 
   do_cleanups (old_chain);
-- 
1.7.7.6

Re: [PATCH 2/6] Iterate over ALL_TRACEPOINTS first.

[Pedro Alves]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> Hi,
> The 'breakpoint-modified' observer is notified on the breakpoint
> level, in order to notify observer for a given tracepoint only once,
> we change to iterate over tracepoint first, and then iterate over
> locations of each tracepoint.  In the inner loop, when we find a
> tracepoint location is downloaded, we mark a flag, and notify the
> observer in outer loop (on tracepoints) if flag is true.  In short, we
> change the iteration from:
> 
>   ALL_BP_LOCATIONS (bl, blp_tmp)
>    {
>      if (!is_tracepoint (bl->owner))
>        continue;
>    }
> 
> to:
> 
>   ALL_TRACEPOINTS (b)
>    {
>       for (bl = b->loc; bl; bl = bl->next)
>         {}
>    }
> 
> download_tracepoint_locations is called after breakpoint re-setting,
> so I think this change has no functional affect.

Yes, I think so too, because tracepoint locations are never duplicates
(otherwise this would be a problem).

OK.

-- 
Pedro Alves

Re: [PATCH 0/6] Add a new field 'installed' when reporting tracepoint

[Pedro Alves]

On 12/04/2012 04:44 AM, Yao Qi wrote:
> Hello,
> During the review to 'tracepoint download' notification
> <http://sourceware.org/ml/gdb-patches/2012-11/msg00871.html>, we find it is
> better to treat 'installed on target' as an attribute of a tracepoint,
> and breakpoint related notification can report it, instead of a
> separate notification.
> 
> In this patch series, we add a field 'installed' in
> print_one_breakpoint_location, which is a boolean to show "y" or "n" the
> tracepoint is installed on target.  The field reflects the value of
> 'inserted' field of each location.

Thanks.

> Originally, my proposal is for MI notification, but I think it also
> useful to display the state of 'installed' in the output of CLI.  With
> this series applied, we can see the 'installed' state in both CLI and
> MI.

Absolutely.

> 
> The 'installed' field is not shown for non-tracepoint breakpoint,
> because in most of cases, breakpoints are inserted when to resume
> inferior, and removed when inferior stops.

Yeah.

-- 
Pedro Alves