[PATCH] Multiple locations for tracepoints

[Stan Shebs <stan at codesourcery dot com>]

If a tracepoint has multiple locations, it works to do the fairly 
obvious thing of iterating over the locations and downloading each to 
the target.  It turns out not to be necessary to invent new numbers for 
tracepoint locations; the target agent simply needs to use both 
tracepoint number and tracepoint address to identify a tracepoint 
location uniquely.

Committed to the trunk.  I'm not sure if this should go to 7.1 also; it 
has no effect outside tracepoint-land, but don't want to churn the 
sources if it's close to release.

Stan

2010-03-03 Stan Shebs  <stan@codesourcery.com>

   * remote.c (remote_download_tracepoint): Iterate over locations.
   * tracepoint.c (validate_actionline): Ditto.
   (encode_actions): Add location argument.
   (trace_dump_command): Check all locations to see if stepping
   frame.


Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.387
diff -p -r1.387 remote.c
*** remote.c	24 Feb 2010 17:01:58 -0000	1.387
--- remote.c	4 Mar 2010 00:28:43 -0000
***************
*** 72,79 ****
  static char *target_buf;
  static long target_buf_size;
  /*static*/ void
! encode_actions (struct breakpoint *t, char ***tdp_actions,
! 		char ***stepping_actions);
  
  /* The size to align memory write packets, when practical.  The protocol
     does not guarantee any alignment, and gdb will generate short
--- 72,79 ----
  static char *target_buf;
  static long target_buf_size;
  /*static*/ void
! encode_actions (struct breakpoint *t, struct bp_location *tloc,
! 		char ***tdp_actions, char ***stepping_actions);
  
  /* The size to align memory write packets, when practical.  The protocol
     does not guarantee any alignment, and gdb will generate short
*************** free_actions_list (char **actions_list)
*** 9253,9258 ****
--- 9253,9259 ----
  static void
  remote_download_tracepoint (struct breakpoint *t)
  {
+   struct bp_location *loc;
    CORE_ADDR tpaddr;
    char tmp[40];
    char buf[2048];
*************** remote_download_tracepoint (struct break
*** 9264,9374 ****
    struct cleanup *aexpr_chain = NULL;
    char *pkt;
  
!   encode_actions (t, &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 = t->loc->address;
!   sprintf_vma (tmp, (t->loc ? tpaddr : 0));
!   sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", t->number, 
! 	   tmp, /* address */
! 	   (t->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 (t->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 ())
! 	{
! 	  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");
! 	}
!       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"), t->number);
!     }
!   /* If the tracepoint has a conditional, make it into an agent
!      expression and append to the definition.  */
!   if (t->loc->cond)
!     {
!       /* 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 (t->loc->address, t->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"), t->number);
-     }
  
!   if (t->actions || *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."));
  
!   if (!t->actions && !*default_collect)
!     return;
  
!   /* do_single_steps (t); */
!   if (tdp_actions)
!     {
!       for (ndx = 0; tdp_actions[ndx]; ndx++)
  	{
! 	  QUIT;	/* allow user to bail out with ^C */
! 	  sprintf (buf, "QTDP:-%x:%s:%s%c",
! 		   t->number, tmp, /* 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)
!     {
!       for (ndx = 0; stepping_actions[ndx]; ndx++)
! 	{
! 	  QUIT;	/* allow user to bail out with ^C */
! 	  sprintf (buf, "QTDP:-%x:%s:%s%s%s",
! 		   t->number, tmp, /* 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."));
  	}
      }
-   do_cleanups (old_chain);
-   return;
  }
  
  static void
--- 9265,9380 ----
    struct cleanup *aexpr_chain = NULL;
    char *pkt;
  
!   /* 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 = t->loc; loc; loc = loc->next)
!     {
!       encode_actions (t, 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 (tmp, (loc ? tpaddr : 0));
!       sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", t->number, 
! 	       tmp, /* address */
! 	       (t->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 (t->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 ())
! 	    {
! 	      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");
! 	    }
  	  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"), t->number);
! 	}
!       /* 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 ())
! 	    {
! 	      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"), t->number);
  	}
  
!       if (t->actions || *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."));
  
!       if (!t->actions && !*default_collect)
! 	continue;
  
!       /* do_single_steps (t); */
!       if (tdp_actions)
  	{
! 	  for (ndx = 0; tdp_actions[ndx]; ndx++)
! 	    {
! 	      QUIT;	/* allow user to bail out with ^C */
! 	      sprintf (buf, "QTDP:-%x:%s:%s%c",
! 		       t->number, tmp, /* 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)
+ 	{
+ 	  for (ndx = 0; stepping_actions[ndx]; ndx++)
+ 	    {
+ 	      QUIT;	/* allow user to bail out with ^C */
+ 	      sprintf (buf, "QTDP:-%x:%s:%s%s%s",
+ 		       t->number, tmp, /* 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."));
+ 	    }
+ 	}
+       do_cleanups (old_chain);
      }
  }
  
  static void
Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.144
diff -p -r1.144 tracepoint.c
*** tracepoint.c	24 Feb 2010 01:06:27 -0000	1.144
--- tracepoint.c	4 Mar 2010 00:28:43 -0000
*************** validate_actionline (char **line, struct
*** 625,631 ****
    struct cmd_list_element *c;
    struct expression *exp = NULL;
    struct cleanup *old_chain = NULL;
!   char *p;
  
    /* if EOF is typed, *line is NULL */
    if (*line == NULL)
--- 625,632 ----
    struct cmd_list_element *c;
    struct expression *exp = NULL;
    struct cleanup *old_chain = NULL;
!   char *p, *tmp_p;
!   struct bp_location *loc;
  
    /* if EOF is typed, *line is NULL */
    if (*line == NULL)
*************** validate_actionline (char **line, struct
*** 671,718 ****
  		}
  	      /* else fall thru, treat p as an expression and parse it!  */
  	    }
! 	  exp = parse_exp_1 (&p, block_for_pc (t->loc->address), 1);
! 	  old_chain = make_cleanup (free_current_contents, &exp);
! 
! 	  if (exp->elts[0].opcode == OP_VAR_VALUE)
  	    {
! 	      if (SYMBOL_CLASS (exp->elts[2].symbol) == LOC_CONST)
! 		{
! 		  warning (_("constant %s (value %ld) will not be collected."),
! 			   SYMBOL_PRINT_NAME (exp->elts[2].symbol),
! 			   SYMBOL_VALUE (exp->elts[2].symbol));
! 		  return BADLINE;
! 		}
! 	      else if (SYMBOL_CLASS (exp->elts[2].symbol) == LOC_OPTIMIZED_OUT)
  		{
! 		  warning (_("%s is optimized away and cannot be collected."),
! 			   SYMBOL_PRINT_NAME (exp->elts[2].symbol));
! 		  return BADLINE;
  		}
- 	    }
  
! 	  /* We have something to collect, make sure that the expr to
! 	     bytecode translator can handle it and that it's not too
! 	     long.  */
! 	  aexpr = gen_trace_for_expr (t->loc->address, exp);
! 	  make_cleanup_free_agent_expr (aexpr);
  
! 	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
! 	    error (_("expression too complicated, try simplifying"));
  
! 	  ax_reqs (aexpr, &areqs);
! 	  (void) make_cleanup (xfree, areqs.reg_mask);
  
! 	  if (areqs.flaw != agent_flaw_none)
! 	    error (_("malformed expression"));
  
! 	  if (areqs.min_height < 0)
! 	    error (_("gdb: Internal error: expression has min height < 0"));
  
! 	  if (areqs.max_height > 20)
! 	    error (_("expression too complicated, try simplifying"));
  
! 	  do_cleanups (old_chain);
  	}
        while (p && *p++ == ',');
        return GENERIC;
--- 672,724 ----
  		}
  	      /* else fall thru, treat p as an expression and parse it!  */
  	    }
! 	  tmp_p = p;
! 	  for (loc = t->loc; loc; loc = loc->next)
  	    {
! 	      p = tmp_p;
! 	      exp = parse_exp_1 (&p, block_for_pc (loc->address), 1);
! 	      old_chain = make_cleanup (free_current_contents, &exp);
! 
! 	      if (exp->elts[0].opcode == OP_VAR_VALUE)
  		{
! 		  if (SYMBOL_CLASS (exp->elts[2].symbol) == LOC_CONST)
! 		    {
! 		      warning (_("constant %s (value %ld) will not be collected."),
! 			       SYMBOL_PRINT_NAME (exp->elts[2].symbol),
! 			       SYMBOL_VALUE (exp->elts[2].symbol));
! 		      return BADLINE;
! 		    }
! 		  else if (SYMBOL_CLASS (exp->elts[2].symbol) == LOC_OPTIMIZED_OUT)
! 		    {
! 		      warning (_("%s is optimized away and cannot be collected."),
! 			       SYMBOL_PRINT_NAME (exp->elts[2].symbol));
! 		      return BADLINE;
! 		    }
  		}
  
! 	      /* We have something to collect, make sure that the expr to
! 		 bytecode translator can handle it and that it's not too
! 		 long.  */
! 	      aexpr = gen_trace_for_expr (loc->address, exp);
! 	      make_cleanup_free_agent_expr (aexpr);
  
! 	      if (aexpr->len > MAX_AGENT_EXPR_LEN)
! 		error (_("expression too complicated, try simplifying"));
  
! 	      ax_reqs (aexpr, &areqs);
! 	      (void) make_cleanup (xfree, areqs.reg_mask);
  
! 	      if (areqs.flaw != agent_flaw_none)
! 		error (_("malformed expression"));
  
! 	      if (areqs.min_height < 0)
! 		error (_("gdb: Internal error: expression has min height < 0"));
  
! 	      if (areqs.max_height > 20)
! 		error (_("expression too complicated, try simplifying"));
  
! 	      do_cleanups (old_chain);
! 	    }
  	}
        while (p && *p++ == ',');
        return GENERIC;
*************** validate_actionline (char **line, struct
*** 727,746 ****
  	  while (isspace ((int) *p))
  	    p++;
  
! 	  /* Only expressions are allowed for this action.  */
! 	  exp = parse_exp_1 (&p, block_for_pc (t->loc->address), 1);
! 	  old_chain = make_cleanup (free_current_contents, &exp);
! 
! 	  /* We have something to evaluate, make sure that the expr to
! 	     bytecode translator can handle it and that it's not too
! 	     long.  */
! 	  aexpr = gen_eval_for_expr (t->loc->address, exp);
! 	  make_cleanup_free_agent_expr (aexpr);
  
! 	  if (aexpr->len > MAX_AGENT_EXPR_LEN)
! 	    error (_("expression too complicated, try simplifying"));
  
! 	  do_cleanups (old_chain);
  	}
        while (p && *p++ == ',');
        return GENERIC;
--- 733,757 ----
  	  while (isspace ((int) *p))
  	    p++;
  
! 	  tmp_p = p;
! 	  for (loc = t->loc; loc; loc = loc->next)
! 	    {
! 	      p = tmp_p;
! 	      /* Only expressions are allowed for this action.  */
! 	      exp = parse_exp_1 (&p, block_for_pc (loc->address), 1);
! 	      old_chain = make_cleanup (free_current_contents, &exp);
! 
! 	      /* We have something to evaluate, make sure that the expr to
! 		 bytecode translator can handle it and that it's not too
! 		 long.  */
! 	      aexpr = gen_eval_for_expr (loc->address, exp);
! 	      make_cleanup_free_agent_expr (aexpr);
  
! 	      if (aexpr->len > MAX_AGENT_EXPR_LEN)
! 		error (_("expression too complicated, try simplifying"));
  
! 	      do_cleanups (old_chain);
! 	    }
  	}
        while (p && *p++ == ',');
        return GENERIC;
*************** stringify_collection_list (struct collec
*** 1238,1245 ****
  
  /* Render all actions into gdb protocol.  */
  /*static*/ void
! encode_actions (struct breakpoint *t, char ***tdp_actions,
! 		char ***stepping_actions)
  {
    static char tdp_buff[2048], step_buff[2048];
    char *action_exp;
--- 1249,1256 ----
  
  /* Render all actions into gdb protocol.  */
  /*static*/ void
! encode_actions (struct breakpoint *t, struct bp_location *tloc,
! 		char ***tdp_actions, char ***stepping_actions)
  {
    static char tdp_buff[2048], step_buff[2048];
    char *action_exp;
*************** encode_actions (struct breakpoint *t, ch
*** 1263,1269 ****
    *stepping_actions = NULL;
  
    gdbarch_virtual_frame_pointer (t->gdbarch,
! 				 t->loc->address, &frame_reg, &frame_offset);
  
    action = t->actions;
  
--- 1274,1280 ----
    *stepping_actions = NULL;
  
    gdbarch_virtual_frame_pointer (t->gdbarch,
! 				 tloc->address, &frame_reg, &frame_offset);
  
    action = t->actions;
  
*************** encode_actions (struct breakpoint *t, ch
*** 1322,1328 ****
  		{
  		  add_local_symbols (collect,
  				     t->gdbarch,
! 				     t->loc->address,
  				     frame_reg,
  				     frame_offset,
  				     'A');
--- 1333,1339 ----
  		{
  		  add_local_symbols (collect,
  				     t->gdbarch,
! 				     tloc->address,
  				     frame_reg,
  				     frame_offset,
  				     'A');
*************** encode_actions (struct breakpoint *t, ch
*** 1332,1338 ****
  		{
  		  add_local_symbols (collect,
  				     t->gdbarch,
! 				     t->loc->address,
  				     frame_reg,
  				     frame_offset,
  				     'L');
--- 1343,1349 ----
  		{
  		  add_local_symbols (collect,
  				     t->gdbarch,
! 				     tloc->address,
  				     frame_reg,
  				     frame_offset,
  				     'L');
*************** encode_actions (struct breakpoint *t, ch
*** 1346,1352 ****
  		  struct agent_reqs areqs;
  
  		  exp = parse_exp_1 (&action_exp, 
! 				     block_for_pc (t->loc->address), 1);
  		  old_chain = make_cleanup (free_current_contents, &exp);
  
  		  switch (exp->elts[0].opcode)
--- 1357,1363 ----
  		  struct agent_reqs areqs;
  
  		  exp = parse_exp_1 (&action_exp, 
! 				     block_for_pc (tloc->address), 1);
  		  old_chain = make_cleanup (free_current_contents, &exp);
  
  		  switch (exp->elts[0].opcode)
*************** encode_actions (struct breakpoint *t, ch
*** 1381,1391 ****
  				      t->gdbarch,
  				      frame_reg,
  				      frame_offset,
! 				      t->loc->address);
  		      break;
  
  		    default:	/* full-fledged expression */
! 		      aexpr = gen_trace_for_expr (t->loc->address, exp);
  
  		      old_chain1 = make_cleanup_free_agent_expr (aexpr);
  
--- 1392,1402 ----
  				      t->gdbarch,
  				      frame_reg,
  				      frame_offset,
! 				      tloc->address);
  		      break;
  
  		    default:	/* full-fledged expression */
! 		      aexpr = gen_trace_for_expr (tloc->address, exp);
  
  		      old_chain1 = make_cleanup_free_agent_expr (aexpr);
  
*************** encode_actions (struct breakpoint *t, ch
*** 1443,1452 ****
  		  struct agent_reqs areqs;
  
  		  exp = parse_exp_1 (&action_exp, 
! 				     block_for_pc (t->loc->address), 1);
  		  old_chain = make_cleanup (free_current_contents, &exp);
  
! 		  aexpr = gen_eval_for_expr (t->loc->address, exp);
  		  old_chain1 = make_cleanup_free_agent_expr (aexpr);
  
  		  ax_reqs (aexpr, &areqs);
--- 1454,1463 ----
  		  struct agent_reqs areqs;
  
  		  exp = parse_exp_1 (&action_exp, 
! 				     block_for_pc (tloc->address), 1);
  		  old_chain = make_cleanup (free_current_contents, &exp);
  
! 		  aexpr = gen_eval_for_expr (tloc->address, exp);
  		  old_chain1 = make_cleanup_free_agent_expr (aexpr);
  
  		  ax_reqs (aexpr, &areqs);
*************** trace_dump_command (char *args, int from
*** 2224,2229 ****
--- 2235,2241 ----
    struct cleanup *old_cleanups;
    int stepping_actions = 0;
    int stepping_frame = 0;
+   struct bp_location *loc;
  
    if (tracepoint_number == -1)
      {
*************** trace_dump_command (char *args, int from
*** 2249,2255 ****
    regcache = get_current_regcache ();
    gdbarch = get_regcache_arch (regcache);
  
!   stepping_frame = (t->loc->address != (regcache_read_pc (regcache)));
  
    for (action = t->actions; action; action = action->next)
      {
--- 2261,2274 ----
    regcache = get_current_regcache ();
    gdbarch = get_regcache_arch (regcache);
  
!   /* If the traceframe's address matches any of the tracepoint's
!      locations, assume it is a direct hit rather than a while-stepping
!      frame.  (FIXME this is not reliable, should record each frame's
!      type.)  */
!   stepping_frame = 1;
!   for (loc = t->loc; loc; loc = loc->next)
!     if (loc->address == regcache_read_pc (regcache))
!       stepping_frame = 0;
  
    for (action = t->actions; action; action = action->next)
      {
*************** tfile_get_traceframe_address (off_t tfra
*** 3239,3244 ****
--- 3258,3264 ----
      error (_("Premature end of file while reading trace file"));
  
    tp = get_tracepoint_by_number_on_target (tpnum);
+   /* FIXME this is a poor heuristic if multiple locations */
    if (tp && tp->loc)
      addr = tp->loc->address;