[patch 1/8] Download tracepoint on location level

[Yao Qi <yao at codesourcery dot com>]

Existing gdb target hook to_download_tracepoint is about downloading
tracepoint on tracepoint level, rather than location level.  However,
locations of one tracepoint may change during inferior run, so we need a
fine-granularity target hook on tracepoint locations, instead of tracepoint.

This patch is to refactor to_download_tracepoint to
to_download_tracepoint_loc.  Beside this refactor, a return value is
added in new hook to reflect whether tracepoint location is
downloaded/installed successfully.  Functionality of gdb is not changed.

-- 
Yao (éå)

        * target.h (struct target): Rename field to_download_tracepoint
        to to_download_tracepoint_loc.  Change type of parameter from
        tracepoint bp_location.
        * target.c (update_current_target): Update.
        * tracepoint.c (start_tracing): Update.
        * remote.c: Move remote_download_tracepoint. to
        remote_download_tracepoint_loc.  Remove loop for each location
        of a tracepoint.
---
 gdb/remote.c     |  284 +++++++++++++++++++++++++++---------------------------
 gdb/target.c     |    6 +-
 gdb/target.h     |   12 ++-
 gdb/tracepoint.c |    6 +-
 4 files changed, 157 insertions(+), 151 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 5182ef1..1c43f39 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9834,10 +9834,10 @@ remote_download_command_source (int num, ULONGEST addr,
     }
 }
 
-static void
-remote_download_tracepoint (struct breakpoint *b)
+static int
+remote_download_tracepoint_loc (struct bp_location *loc)
 {
-  struct bp_location *loc;
+
   CORE_ADDR tpaddr;
   char addrbuf[40];
   char buf[2048];
@@ -9848,169 +9848,167 @@ remote_download_tracepoint (struct breakpoint *b)
   struct agent_expr *aexpr;
   struct cleanup *aexpr_chain = NULL;
   char *pkt;
+  struct breakpoint *b = loc->owner;
   struct tracepoint *t = (struct tracepoint *) b;
 
-  /* Iterate over all the tracepoint locations.  It's up to the target to
-     notice multiple tracepoint packets with the same number but different
-     addresses, and treat them as multiple locations.  */
-  for (loc = b->loc; loc; loc = loc->next)
-    {
-      encode_actions (b, loc, &tdp_actions, &stepping_actions);
-      old_chain = make_cleanup (free_actions_list_cleanup_wrapper,
-				tdp_actions);
-      (void) make_cleanup (free_actions_list_cleanup_wrapper,
-			   stepping_actions);
-
-      tpaddr = loc->address;
-      sprintf_vma (addrbuf, tpaddr);
-      sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", b->number,
-	       addrbuf, /* address */
-	       (b->enable_state == bp_enabled ? 'E' : 'D'),
-	       t->step_count, t->pass_count);
-      /* Fast tracepoints are mostly handled by the target, but we can
-	 tell the target how big of an instruction block should be moved
-	 around.  */
-      if (b->type == bp_fast_tracepoint)
+  encode_actions (loc->owner, loc, &tdp_actions, &stepping_actions);
+  old_chain = make_cleanup (free_actions_list_cleanup_wrapper,
+			    tdp_actions);
+  (void) make_cleanup (free_actions_list_cleanup_wrapper,
+		       stepping_actions);
+
+  tpaddr = loc->address;
+  sprintf_vma (addrbuf, tpaddr);
+  sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", b->number,
+	   addrbuf, /* address */
+	   (b->enable_state == bp_enabled ? 'E' : 'D'),
+	   t->step_count, t->pass_count);
+  /* Fast tracepoints are mostly handled by the target, but we can
+     tell the target how big of an instruction block should be moved
+     around.  */
+  if (b->type == bp_fast_tracepoint)
+    {
+      /* Only test for support at download time; we may not know
+	 target capabilities at definition time.  */
+      if (remote_supports_fast_tracepoints ())
 	{
-	  /* Only test for support at download time; we may not know
-	     target capabilities at definition time.  */
-	  if (remote_supports_fast_tracepoints ())
-	    {
-	      int isize;
+	  int isize;
 
-	      if (gdbarch_fast_tracepoint_valid_at (target_gdbarch,
-						    tpaddr, &isize, NULL))
-		sprintf (buf + strlen (buf), ":F%x", isize);
-	      else
-		/* If it passed validation at definition but fails now,
-		   something is very wrong.  */
-		internal_error (__FILE__, __LINE__,
-				_("Fast tracepoint not "
-				  "valid during download"));
-	    }
+	  if (gdbarch_fast_tracepoint_valid_at (target_gdbarch,
+						tpaddr, &isize, NULL))
+	    sprintf (buf + strlen (buf), ":F%x", isize);
 	  else
-	    /* Fast tracepoints are functionally identical to regular
-	       tracepoints, so don't take lack of support as a reason to
-	       give up on the trace run.  */
-	    warning (_("Target does not support fast tracepoints, "
-		       "downloading %d as regular tracepoint"), b->number);
+	    /* If it passed validation at definition but fails now,
+	       something is very wrong.  */
+	    internal_error (__FILE__, __LINE__,
+			    _("Fast tracepoint not "
+			      "valid during download"));
 	}
-      else if (b->type == bp_static_tracepoint)
+      else
+	/* Fast tracepoints are functionally identical to regular
+	   tracepoints, so don't take lack of support as a reason to
+	   give up on the trace run.  */
+	warning (_("Target does not support fast tracepoints, "
+		   "downloading %d as regular tracepoint"), b->number);
+    }
+  else if (b->type == bp_static_tracepoint)
+    {
+      /* Only test for support at download time; we may not know
+	 target capabilities at definition time.  */
+      if (remote_supports_static_tracepoints ())
 	{
-	  /* Only test for support at download time; we may not know
-	     target capabilities at definition time.  */
-	  if (remote_supports_static_tracepoints ())
-	    {
-	      struct static_tracepoint_marker marker;
+	  struct static_tracepoint_marker marker;
 
-	      if (target_static_tracepoint_marker_at (tpaddr, &marker))
-		strcat (buf, ":S");
-	      else
-		error (_("Static tracepoint not valid during download"));
-	    }
+	  if (target_static_tracepoint_marker_at (tpaddr, &marker))
+	    strcat (buf, ":S");
 	  else
-	    /* Fast tracepoints are functionally identical to regular
-	       tracepoints, so don't take lack of support as a reason
-	       to give up on the trace run.  */
-	    error (_("Target does not support static tracepoints"));
+	    error (_("Static tracepoint not valid during download"));
 	}
-      /* If the tracepoint has a conditional, make it into an agent
-	 expression and append to the definition.  */
-      if (loc->cond)
+      else
+	/* Fast tracepoints are functionally identical to regular
+	   tracepoints, so don't take lack of support as a reason
+	   to give up on the trace run.  */
+	error (_("Target does not support static tracepoints"));
+    }
+  /* If the tracepoint has a conditional, make it into an agent
+     expression and append to the definition.  */
+  if (loc->cond)
+    {
+      /* Only test support at download time, we may not know target
+	 capabilities at definition time.  */
+      if (remote_supports_cond_tracepoints ())
 	{
-	  /* Only test support at download time, we may not know target
-	     capabilities at definition time.  */
-	  if (remote_supports_cond_tracepoints ())
-	    {
-	      aexpr = gen_eval_for_expr (tpaddr, loc->cond);
-	      aexpr_chain = make_cleanup_free_agent_expr (aexpr);
-	      sprintf (buf + strlen (buf), ":X%x,", aexpr->len);
-	      pkt = buf + strlen (buf);
-	      for (ndx = 0; ndx < aexpr->len; ++ndx)
-		pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
-	      *pkt = '\0';
-	      do_cleanups (aexpr_chain);
-	    }
-	  else
-	    warning (_("Target does not support conditional tracepoints, "
-		       "ignoring tp %d cond"), b->number);
+	  aexpr = gen_eval_for_expr (tpaddr, loc->cond);
+	  aexpr_chain = make_cleanup_free_agent_expr (aexpr);
+	  sprintf (buf + strlen (buf), ":X%x,", aexpr->len);
+	  pkt = buf + strlen (buf);
+	  for (ndx = 0; ndx < aexpr->len; ++ndx)
+	    pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
+	  *pkt = '\0';
+	  do_cleanups (aexpr_chain);
 	}
+      else
+	warning (_("Target does not support conditional tracepoints, "
+		   "ignoring tp %d cond"), b->number);
+    }
 
   if (b->commands || *default_collect)
-	strcat (buf, "-");
-      putpkt (buf);
-      remote_get_noisy_reply (&target_buf, &target_buf_size);
-      if (strcmp (target_buf, "OK"))
-	error (_("Target does not support tracepoints."));
+    strcat (buf, "-");
+  putpkt (buf);
+  remote_get_noisy_reply (&target_buf, &target_buf_size);
+  if (strcmp (target_buf, "OK"))
+    error (_("Target does not support tracepoints."));
 
-      /* do_single_steps (t); */
-      if (tdp_actions)
+  /* do_single_steps (t); */
+  if (tdp_actions)
+    {
+      for (ndx = 0; tdp_actions[ndx]; ndx++)
 	{
-	  for (ndx = 0; tdp_actions[ndx]; ndx++)
-	    {
-	      QUIT;	/* Allow user to bail out with ^C.  */
-	      sprintf (buf, "QTDP:-%x:%s:%s%c",
-		       b->number, addrbuf, /* address */
-		       tdp_actions[ndx],
-		       ((tdp_actions[ndx + 1] || stepping_actions)
-			? '-' : 0));
-	      putpkt (buf);
-	      remote_get_noisy_reply (&target_buf,
-				      &target_buf_size);
-	      if (strcmp (target_buf, "OK"))
-		error (_("Error on target while setting tracepoints."));
-	    }
+	  QUIT;	/* Allow user to bail out with ^C.  */
+	  sprintf (buf, "QTDP:-%x:%s:%s%c",
+		   b->number, addrbuf, /* address */
+		   tdp_actions[ndx],
+		   ((tdp_actions[ndx + 1] || stepping_actions)
+		    ? '-' : 0));
+	  putpkt (buf);
+	  remote_get_noisy_reply (&target_buf,
+				  &target_buf_size);
+	  if (strcmp (target_buf, "OK"))
+	    error (_("Error on target while setting tracepoints."));
 	}
-      if (stepping_actions)
+    }
+  if (stepping_actions)
+    {
+      for (ndx = 0; stepping_actions[ndx]; ndx++)
 	{
-	  for (ndx = 0; stepping_actions[ndx]; ndx++)
-	    {
-	      QUIT;	/* Allow user to bail out with ^C.  */
-	      sprintf (buf, "QTDP:-%x:%s:%s%s%s",
-		       b->number, addrbuf, /* address */
-		       ((ndx == 0) ? "S" : ""),
-		       stepping_actions[ndx],
-		       (stepping_actions[ndx + 1] ? "-" : ""));
-	      putpkt (buf);
-	      remote_get_noisy_reply (&target_buf,
-				      &target_buf_size);
-	      if (strcmp (target_buf, "OK"))
-		error (_("Error on target while setting tracepoints."));
-	    }
+	  QUIT;	/* Allow user to bail out with ^C.  */
+	  sprintf (buf, "QTDP:-%x:%s:%s%s%s",
+		   b->number, addrbuf, /* address */
+		   ((ndx == 0) ? "S" : ""),
+		   stepping_actions[ndx],
+		   (stepping_actions[ndx + 1] ? "-" : ""));
+	  putpkt (buf);
+	  remote_get_noisy_reply (&target_buf,
+				  &target_buf_size);
+	  if (strcmp (target_buf, "OK"))
+	    error (_("Error on target while setting tracepoints."));
 	}
+    }
 
-      if (remote_protocol_packets[PACKET_TracepointSource].support
-	  == PACKET_ENABLE)
+  if (remote_protocol_packets[PACKET_TracepointSource].support
+      == PACKET_ENABLE)
+    {
+      if (b->addr_string)
 	{
-	  if (b->addr_string)
-	    {
-	      strcpy (buf, "QTDPsrc:");
-	      encode_source_string (b->number, loc->address,
-				    "at", b->addr_string, buf + strlen (buf),
-				    2048 - strlen (buf));
+	  strcpy (buf, "QTDPsrc:");
+	  encode_source_string (b->number, loc->address,
+				"at", b->addr_string, buf + strlen (buf),
+				2048 - strlen (buf));
 
-	      putpkt (buf);
-	      remote_get_noisy_reply (&target_buf, &target_buf_size);
-	      if (strcmp (target_buf, "OK"))
-		warning (_("Target does not support source download."));
-	    }
-	  if (b->cond_string)
-	    {
-	      strcpy (buf, "QTDPsrc:");
-	      encode_source_string (b->number, loc->address,
-				    "cond", b->cond_string, buf + strlen (buf),
-				    2048 - strlen (buf));
-	      putpkt (buf);
-	      remote_get_noisy_reply (&target_buf, &target_buf_size);
-	      if (strcmp (target_buf, "OK"))
-		warning (_("Target does not support source download."));
-	    }
-	  remote_download_command_source (b->number, loc->address,
-					  breakpoint_commands (b));
+	  putpkt (buf);
+	  remote_get_noisy_reply (&target_buf, &target_buf_size);
+	  if (strcmp (target_buf, "OK"))
+	    warning (_("Target does not support source download."));
 	}
-
-      do_cleanups (old_chain);
+      if (b->cond_string)
+	{
+	  strcpy (buf, "QTDPsrc:");
+	  encode_source_string (b->number, loc->address,
+				"cond", b->cond_string, buf + strlen (buf),
+				2048 - strlen (buf));
+	  putpkt (buf);
+	  remote_get_noisy_reply (&target_buf, &target_buf_size);
+	  if (strcmp (target_buf, "OK"))
+	    warning (_("Target does not support source download."));
+	}
+      remote_download_command_source (b->number, loc->address,
+				      breakpoint_commands (b));
     }
+
+  do_cleanups (old_chain);
+
+  return 0;
+
 }
 
 static void
@@ -10484,7 +10482,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_supports_enable_disable_tracepoint = remote_supports_enable_disable_tracepoint;
   remote_ops.to_supports_string_tracing = remote_supports_string_tracing;
   remote_ops.to_trace_init = remote_trace_init;
-  remote_ops.to_download_tracepoint = remote_download_tracepoint;
+  remote_ops.to_download_tracepoint_loc = remote_download_tracepoint_loc;
   remote_ops.to_download_trace_state_variable
     = remote_download_trace_state_variable;
   remote_ops.to_enable_tracepoint = remote_enable_tracepoint;
diff --git a/gdb/target.c b/gdb/target.c
index 41ff6cf..48cd888 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -674,7 +674,7 @@ update_current_target (void)
       INHERIT (to_supports_enable_disable_tracepoint, t);
       INHERIT (to_supports_string_tracing, t);
       INHERIT (to_trace_init, t);
-      INHERIT (to_download_tracepoint, t);
+      INHERIT (to_download_tracepoint_loc, t);
       INHERIT (to_download_trace_state_variable, t);
       INHERIT (to_enable_tracepoint, t);
       INHERIT (to_disable_tracepoint, t);
@@ -847,8 +847,8 @@ update_current_target (void)
   de_fault (to_trace_init,
 	    (void (*) (void))
 	    tcomplain);
-  de_fault (to_download_tracepoint,
-	    (void (*) (struct breakpoint *))
+  de_fault (to_download_tracepoint_loc,
+	    (int (*) (struct bp_location *))
 	    tcomplain);
   de_fault (to_download_trace_state_variable,
 	    (void (*) (struct trace_state_variable *))
diff --git a/gdb/target.h b/gdb/target.h
index 352f2df..74e1d8a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -686,8 +686,12 @@ struct target_ops
     /* Prepare the target for a tracing run.  */
     void (*to_trace_init) (void);
 
-    /* Send full details of a tracepoint to the target.  */
-    void (*to_download_tracepoint) (struct breakpoint *t);
+    /* Send full details of a tracepoint location to the target.  Target may
+       install or not install tracepoint locations to inferior, which is
+       determined by target's factors, such tracing state.  If the location
+       of tracepoint is downloaded and installed successfully, return 0,
+       otherwise return 1.  */
+    int (*to_download_tracepoint_loc) (struct bp_location *location);
 
     /* Send full details of a trace state variable to the target.  */
     void (*to_download_trace_state_variable) (struct trace_state_variable *tsv);
@@ -1474,8 +1478,8 @@ extern int target_search_memory (CORE_ADDR start_addr,
 #define target_trace_init() \
   (*current_target.to_trace_init) ()
 
-#define target_download_tracepoint(t) \
-  (*current_target.to_download_tracepoint) (t)
+#define target_download_tracepoint_loc(t) \
+  (*current_target.to_download_tracepoint_loc) (t)
 
 #define target_download_trace_state_variable(tsv) \
   (*current_target.to_download_trace_state_variable) (tsv)
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 4ca4ec2..493d324 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1701,6 +1701,7 @@ start_tracing (void)
   for (ix = 0; VEC_iterate (breakpoint_p, tp_vec, ix, b); ix++)
     {
       struct tracepoint *t = (struct tracepoint *) b;
+      struct bp_location *loc;
 
       if ((b->type == bp_fast_tracepoint
 	   ? !may_insert_fast_tracepoints
@@ -1708,7 +1709,10 @@ start_tracing (void)
 	continue;
 
       t->number_on_target = 0;
-      target_download_tracepoint (b);
+
+      for (loc = b->loc; loc; loc = loc->next)
+	target_download_tracepoint_loc (loc);
+
       t->number_on_target = b->number;
     }
   VEC_free (breakpoint_p, tp_vec);
-- 
1.7.0.4