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

	* tracepoint.h (struct uploaded_string): New struct.
	(struct uploaded_tp): New fields for source strings.
	* breakpoint.c (this_utp, next_cmd): New globals.
	(read_uploaded_action): New function.
	(create_tracepoint_from_upload): Fill in more parts
	of a tracepoint.
	* tracepoint.c (encode_source_string): New function.
	(trace_save): Write out source strings, fix error checks.
	(parse_tracepoint_definition): Add source string parsing.
	* remote.c (PACKET_TracepointSource): New packet type.
	(remote_download_command_source): New function.
	(remote_download_tracepoint): Download source pieces also.
	(_initialize_remote): Add packet config command.

	* gdb.texinfo (Tracepoint Packets): Describe QTDPsrc.
	(General Query Packets): Describe TracepointSource.

7 files changed, 315 insertions, 38 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a75db4d..b7055b7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2010-03-29  Stan Shebs  <stan@codesourcery.com>
 
+	* tracepoint.h (struct uploaded_string): New struct.
+	(struct uploaded_tp): New fields for source strings.
+	* breakpoint.c (this_utp, next_cmd): New globals.
+	(read_uploaded_action): New function.
+	(create_tracepoint_from_upload): Fill in more parts
+	of a tracepoint.
+	* tracepoint.c (encode_source_string): New function.
+	(trace_save): Write out source strings, fix error checks.
+	(parse_tracepoint_definition): Add source string parsing.
+	* remote.c (PACKET_TracepointSource): New packet type.
+	(remote_download_command_source): New function.
+	(remote_download_tracepoint): Download source pieces also.
+	(_initialize_remote): Add packet config command.
+
 	* tracepoint.c (collect_symbol): Send LOC_UNRESOLVED symbols to
 	expression handler.
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e9fb71e..2bded96 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -10305,6 +10305,26 @@ ftrace_command (char *arg, int from_tty)
     set_tracepoint_count (breakpoint_count);
 }
 
+/* Set up a fake reader function that gets command lines from a linked
+   list that was acquired during tracepoint uploading.  */
+
+static struct uploaded_tp *this_utp;
+static struct uploaded_string *next_cmd;
+
+static char *
+read_uploaded_action (void)
+{
+  char *rslt;
+
+  if (!next_cmd)
+    return NULL;
+
+  rslt = next_cmd->str;
+  next_cmd = next_cmd->next;
+
+  return rslt;
+}
+
 /* Given information about a tracepoint as recorded on a target (which
    can be either a live system or a trace file), attempt to create an
    equivalent GDB tracepoint.  This is not a reliable process, since
@@ -10314,15 +10334,31 @@ ftrace_command (char *arg, int from_tty)
 struct breakpoint *
 create_tracepoint_from_upload (struct uploaded_tp *utp)
 {
-  char buf[100];
+  char *addr_str, small_buf[100];
   struct breakpoint *tp;
 
-  /* In the absence of a source location, fall back to raw address.  */
-  sprintf (buf, "*%s", paddress (get_current_arch(), utp->addr));
+  if (utp->at_string)
+    addr_str = utp->at_string;
+  else
+    {
+      /* In the absence of a source location, fall back to raw
+	 address.  Since there is no way to confirm that the address
+	 means the same thing as when the trace was started, warn the
+	 user.  */
+      warning (_("Uploaded tracepoint %d has no source location, using raw address"),
+	       utp->number);
+      sprintf (small_buf, "*%s", hex_string (utp->addr));
+      addr_str = small_buf;
+    }
+
+  /* There's not much we can do with a sequence of bytecodes.  */
+  if (utp->cond && !utp->cond_string)
+    warning (_("Uploaded tracepoint %d condition has no source form, ignoring it"),
+	     utp->number);
 
   if (!create_breakpoint (get_current_arch (),
-			  buf,
-			  NULL, 0, 1 /* parse arg */,
+			  addr_str,
+			  utp->cond_string, -1, 0 /* parse cond/thread */,
 			  0 /* tempflag */,
 			  (utp->type == bp_fast_tracepoint) /* hardwareflag */,
 			  1 /* traceflag */,
@@ -10335,30 +10371,35 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 
   set_tracepoint_count (breakpoint_count);
   
+  /* Get the tracepoint we just created.  */
   tp = get_tracepoint (tracepoint_count);
   gdb_assert (tp != NULL);
 
   if (utp->pass > 0)
     {
-      sprintf (buf, "%d %d", utp->pass, tp->number);
+      sprintf (small_buf, "%d %d", utp->pass, tp->number);
 
-      trace_pass_command (buf, 0);
+      trace_pass_command (small_buf, 0);
     }
 
-  if (utp->cond)
+  /* If we have uploaded versions of the original commands, set up a
+     special-purpose "reader" function and call the usual command line
+     reader, then pass the result to the breakpoint command-setting
+     function.  */
+  if (utp->cmd_strings)
     {
-      printf_filtered ("Want to restore a condition\n");
-    }
+      struct command_line *cmd_list;
 
-  if (utp->numactions > 0)
-    {
-      printf_filtered ("Want to restore action list\n");
-    }
+      this_utp = utp;
+      next_cmd = utp->cmd_strings;
 
-  if (utp->num_step_actions > 0)
-    {
-      printf_filtered ("Want to restore action list\n");
+      cmd_list = read_command_lines_1 (read_uploaded_action, 1, NULL, NULL);
+
+      breakpoint_set_commands (tp, cmd_list);
     }
+  else if (utp->numactions > 0 || utp->num_step_actions > 0)
+    warning (_("Uploaded tracepoint %d actions have no source form, ignoring them"),
+	     utp->number);
 
   return tp;
   }
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index f57033c..76d2c88 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2010-03-29  Stan Shebs  <stan@codesourcery.com>
+	
+	* gdb.texinfo (Tracepoint Packets): Describe QTDPsrc.
+	(General Query Packets): Describe TracepointSource.
+
 2010-03-27  Matt Rice  <ratmice@gmail.com>
 
         * gdb.texinfo (ARM): Document arguments to "target sim".
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 56dbe5d..57e4f03 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -30732,6 +30732,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{TracepointSource}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -30825,6 +30830,10 @@ The remote stub accepts and implements the reverse continue packet
 The remote stub accepts and implements the reverse step packet
 (@pxref{bs}).
 
+@item TracepointSource
+The remote stub understands the @samp{QTDPsrc} packet that supplies
+the source form of tracepoint definitions.
+
 @end table
 
 @item qSymbol::
@@ -30868,6 +30877,7 @@ encoded).  @value{GDBN} will continue to supply the values of symbols
 @item QTBuffer
 @item QTDisconnected
 @itemx QTDP
+@itemx QTDPsrc
 @itemx QTDV
 @itemx qTfP
 @itemx qTfV
@@ -31269,6 +31279,40 @@ The packet was understood and carried out.
 The packet was not recognized.
 @end table
 
+@item QTDPsrc:@var{n}:@var{addr}:@var{type}:@var{start}:@var{slen}:@var{bytes}
+@cindex @samp{QTDPsrc} packet
+Specify a source string of tracepoint @var{n} at address @var{addr}.
+This is useful to get accurate reproduction of the tracepoints
+originally downloaded at the beginning of the trace run.  @var{type}
+is the name of the tracepoint part, such as @samp{cond} for the
+tracepoint's conditional expression (see below for a list of types), while
+@var{bytes} is the string, encoded in hexadecimal.
+
+@var{start} is the offset of the @var{bytes} within the overall source
+string, while @var{slen} is the total length of the source string.
+This is intended for handling source strings that are longer than will
+fit in a single packet.
+@c Add detailed example when this info is moved into a dedicated
+@c tracepoint descriptions section.
+
+The available string types are @samp{at} for the location,
+@samp{cond} for the conditional, and @samp{cmd} for an action command.
+@value{GDBN} sends a separate packet for each command in the action
+list, in the same order in which the commands are stored in the list.
+
+The target does not need to do anything with source strings except
+report them back as part of the replies to the @samp{qTfP}/@samp{qTsP}
+query packets.
+
+Although this packet is optional, and @value{GDBN} will only send it
+if the target replies with @samp{TracepointSource} @xref{General
+Query Packets}, it makes both disconnected tracing and trace files
+much easier to use.  Otherwise the user must be careful that the
+tracepoints in effect while looking at trace frames are identical to
+the ones in effect during the trace run; even a small discrepancy
+could cause @samp{tdump} not to work, or a particular trace frame not
+be found.
+
 @item QTDV:@var{n}:@var{value}
 @cindex define trace state variable, remote request
 @cindex @samp{QTDV} packet
diff --git a/gdb/remote.c b/gdb/remote.c
index 0c791aa..dcae72c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1154,6 +1154,7 @@ enum {
   PACKET_FastTracepoints,
   PACKET_bc,
   PACKET_bs,
+  PACKET_TracepointSource,
   PACKET_MAX
 };
 
@@ -3462,6 +3463,8 @@ static struct protocol_feature remote_protocol_features[] = {
     PACKET_bc },
   { "ReverseStep", PACKET_DISABLE, remote_supported_packet,
     PACKET_bs },
+  { "TracepointSource", PACKET_DISABLE, remote_supported_packet,
+    PACKET_TracepointSource },
 };
 
 static void
@@ -9267,12 +9270,52 @@ free_actions_list (char **actions_list)
   xfree (actions_list);
 }
 
+/* Recursive routine to walk through command list including loops, and
+   download packets for each command.  */
+
+static void
+remote_download_command_source (int num, ULONGEST addr,
+				struct command_line *cmds)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct command_line *cmd;
+
+  for (cmd = cmds; cmd; cmd = cmd->next)
+    {
+      QUIT;	/* allow user to bail out with ^C */
+      strcpy (rs->buf, "QTDPsrc:");
+      encode_source_string (num, addr, "cmd", cmd->line,
+			    rs->buf + strlen (rs->buf),
+			    rs->buf_size - strlen (rs->buf));
+      putpkt (rs->buf);
+      remote_get_noisy_reply (&target_buf, &target_buf_size);
+      if (strcmp (target_buf, "OK"))
+	warning (_("Target does not support source download."));
+
+      if (cmd->control_type == while_control
+	  || cmd->control_type == while_stepping_control)
+	{
+	  remote_download_command_source (num, addr, *cmd->body_list);
+
+	  QUIT;	/* allow user to bail out with ^C */
+	  strcpy (rs->buf, "QTDPsrc:");
+	  encode_source_string (num, addr, "cmd", "end",
+				rs->buf + strlen (rs->buf),
+				rs->buf_size - strlen (rs->buf));
+	  putpkt (rs->buf);
+	  remote_get_noisy_reply (&target_buf, &target_buf_size);
+	  if (strcmp (target_buf, "OK"))
+	    warning (_("Target does not support source download."));
+	}
+    }
+}
+
 static void
 remote_download_tracepoint (struct breakpoint *t)
 {
   struct bp_location *loc;
   CORE_ADDR tpaddr;
-  char tmp[40];
+  char addrbuf[40];
   char buf[2048];
   char **tdp_actions;
   char **stepping_actions;
@@ -9293,9 +9336,9 @@ remote_download_tracepoint (struct breakpoint *t)
       (void) make_cleanup (free_actions_list_cleanup_wrapper, stepping_actions);
 
       tpaddr = loc->address;
-      sprintf_vma (tmp, (loc ? tpaddr : 0));
+      sprintf_vma (addrbuf, tpaddr);
       sprintf (buf, "QTDP:%x:%s:%c:%lx:%x", t->number, 
-	       tmp, /* address */
+	       addrbuf, /* 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
@@ -9352,9 +9395,6 @@ remote_download_tracepoint (struct breakpoint *t)
       if (strcmp (target_buf, "OK"))
 	error (_("Target does not support tracepoints."));
 
-  if (!t->commands && !*default_collect)
-	continue;
-
       /* do_single_steps (t); */
       if (tdp_actions)
 	{
@@ -9362,7 +9402,7 @@ remote_download_tracepoint (struct breakpoint *t)
 	    {
 	      QUIT;	/* allow user to bail out with ^C */
 	      sprintf (buf, "QTDP:-%x:%s:%s%c",
-		       t->number, tmp, /* address */
+		       t->number, addrbuf, /* address */
 		       tdp_actions[ndx],
 		       ((tdp_actions[ndx + 1] || stepping_actions)
 			? '-' : 0));
@@ -9379,7 +9419,7 @@ remote_download_tracepoint (struct breakpoint *t)
 	    {
 	      QUIT;	/* allow user to bail out with ^C */
 	      sprintf (buf, "QTDP:-%x:%s:%s%s%s",
-		       t->number, tmp, /* address */
+		       t->number, addrbuf, /* address */
 		       ((ndx == 0) ? "S" : ""),
 		       stepping_actions[ndx],
 		       (stepping_actions[ndx + 1] ? "-" : ""));
@@ -9390,6 +9430,36 @@ remote_download_tracepoint (struct breakpoint *t)
 		error (_("Error on target while setting tracepoints."));
 	    }
 	}
+
+      if (remote_protocol_packets[PACKET_TracepointSource].support == PACKET_ENABLE)
+	{
+	  if (t->addr_string)
+	    {
+	      strcpy (buf, "QTDPsrc:");
+	      encode_source_string (t->number, loc->address,
+				    "at", t->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 (t->cond_string)
+	    {
+	      strcpy (buf, "QTDPsrc:");
+	      encode_source_string (t->number, loc->address,
+				    "cond", t->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 (t->number, loc->address,
+					  t->commands->commands);
+	}
+
       do_cleanups (old_chain);
     }
 }
@@ -10231,6 +10301,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_FastTracepoints],
 			 "FastTracepoints", "fast-tracepoints", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_TracepointSource],
+			 "TracepointSource", "TracepointSource", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 44b8c2b..07e8541 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2446,6 +2446,25 @@ trace_dump_command (char *args, int from_tty)
   discard_cleanups (old_cleanups);
 }
 
+/* Encode a piece of a tracepoint's source-level definition in a form
+   that is suitable for both protocol and saving in files.  */
+/* This version does not do multiple encodes for long strings; it should
+   return an offset to the next piece to encode.  FIXME  */
+
+extern int
+encode_source_string (int tpnum, ULONGEST addr,
+		      char *srctype, char *src, char *buf, int buf_size)
+{
+  if (80 + strlen (srctype) > buf_size)
+    error (_("Buffer too small for source encoding"));
+  sprintf (buf, "%x:%s:%s:%x:%x:",
+	   tpnum, phex_nz (addr, sizeof (addr)), srctype, 0, (int) strlen (src));
+  if (strlen (buf) + strlen (src) * 2 >= buf_size)
+    error (_("Source string too long for buffer"));
+  bin2hex (src, buf + strlen (buf), 0);
+  return -1;
+}
+
 extern int trace_regblock_size;
 
 /* Save tracepoint data to file named FILENAME.  If TARGET_DOES_SAVE is
@@ -2463,6 +2482,7 @@ trace_save (const char *filename, int target_does_save)
   struct uploaded_tp *uploaded_tps = NULL, *utp;
   struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
   int a;
+  struct uploaded_string *cmd;
   LONGEST gotten = 0;
   ULONGEST offset = 0;
 #define MAX_TRACE_UPLOAD 2000
@@ -2497,7 +2517,7 @@ trace_save (const char *filename, int target_does_save)
      binary file, plus a hint as what this file is, and a version
      number in case of future needs.  */
   written = fwrite ("\x7fTRACE0\n", 8, 1, fp);
-  if (written < 8)
+  if (written < 1)
     perror_with_name (pathname);
 
   /* Write descriptive info.  */
@@ -2578,6 +2598,24 @@ trace_save (const char *filename, int target_does_save)
 	fprintf (fp, "tp S%x:%s:%s\n",
 		 utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
 		 utp->step_actions[a]);
+      if (utp->at_string)
+	{
+	  encode_source_string (utp->number, utp->addr,
+				"at", utp->at_string, buf, MAX_TRACE_UPLOAD);
+	  fprintf (fp, "tp Z%s\n", buf);
+	}
+      if (utp->cond_string)
+	{
+	  encode_source_string (utp->number, utp->addr,
+				"cond", utp->cond_string, buf, MAX_TRACE_UPLOAD);
+	  fprintf (fp, "tp Z%s\n", buf);
+	}
+      for (cmd = utp->cmd_strings; cmd; cmd = cmd->next)
+	{
+	  encode_source_string (utp->number, utp->addr, "cmd", cmd->str,
+				buf, MAX_TRACE_UPLOAD);
+	  fprintf (fp, "tp Z%s\n", buf);
+	}
     }
 
   free_uploaded_tps (&uploaded_tps);
@@ -2597,14 +2635,14 @@ trace_save (const char *filename, int target_does_save)
       if (gotten == 0)
 	break;
       written = fwrite (buf, gotten, 1, fp);
-      if (written < gotten)
+      if (written < 1)
 	perror_with_name (pathname);
       offset += gotten;
     }
 
-  /* Mark the end of trace data.  */
+  /* Mark the end of trace data.  (We know that gotten is 0 at this point.)  */
   written = fwrite (&gotten, 4, 1, fp);
-  if (written < 4)
+  if (written < 1)
     perror_with_name (pathname);
 
   do_cleanups (cleanup);
@@ -3266,18 +3304,18 @@ Status line: '%s'\n"), p, line);
     }
 }
 
-/* Given a line of text defining a tracepoint or tracepoint action, parse
-   it into an "uploaded tracepoint".  */
+/* Given a line of text defining a part of a tracepoint, parse it into
+   an "uploaded tracepoint".  */
 
 void
 parse_tracepoint_definition (char *line, struct uploaded_tp **utpp)
 {
   char *p;
   char piece;
-  ULONGEST num, addr, step, pass, orig_size, xlen;
-  int enabled, i;
+  ULONGEST num, addr, step, pass, orig_size, xlen, start;
+  int enabled, i, end;
   enum bptype type;
-  char *cond;
+  char *cond, *srctype, *src, *buf;
   struct uploaded_tp *utp = NULL;
 
   p = line;
@@ -3318,7 +3356,7 @@ parse_tracepoint_definition (char *line, struct uploaded_tp **utpp)
 	      p += 2 * xlen;
 	    }
 	  else
-	    warning ("Unrecognized char '%c' in tracepoint definition, skipping rest", *p);
+	    warning (_("Unrecognized char '%c' in tracepoint definition, skipping rest"), *p);
 	}
       utp = get_uploaded_tp (num, addr, utpp);
       utp->type = type;
@@ -3337,9 +3375,49 @@ parse_tracepoint_definition (char *line, struct uploaded_tp **utpp)
       utp = get_uploaded_tp (num, addr, utpp);
       utp->step_actions[utp->num_step_actions++] = xstrdup (p);
     }
+  else if (piece == 'Z')
+    {
+      /* Parse a chunk of source form definition.  */
+      utp = get_uploaded_tp (num, addr, utpp);
+      srctype = p;
+      p = strchr (p, ':');
+      p++;  /* skip a colon */
+      p = unpack_varlen_hex (p, &start);
+      p++;  /* skip a colon */
+      p = unpack_varlen_hex (p, &xlen);
+      p++;  /* skip a colon */
+
+      buf = alloca (strlen (line));
+
+      end = hex2bin (p, (gdb_byte *) buf, strlen (p) / 2);
+      buf[end] = '\0';
+
+      if (strncmp (srctype, "at:", strlen ("at:")) == 0)
+	utp->at_string = xstrdup (buf);
+      else if (strncmp (srctype, "cond:", strlen ("cond:")) == 0)
+	utp->cond_string = xstrdup (buf);
+      else if (strncmp (srctype, "cmd:", strlen ("cmd:")) == 0)
+	{
+	  /* FIXME consider using a vector? */
+	  struct uploaded_string *last, *newlast;
+	  newlast = (struct uploaded_string *) xmalloc (sizeof (struct uploaded_string));
+	  newlast->str = xstrdup (buf);
+	  newlast->next = NULL;
+	  if (utp->cmd_strings)
+	    {
+	      for (last = utp->cmd_strings; last->next; last = last->next)
+		;
+	      last->next = newlast;
+	    }
+	  else
+	    utp->cmd_strings = newlast;
+	}
+    }
   else
     {
-      error ("Invalid tracepoint piece");
+      /* Don't error out, the target might be sending us optional
+	 info that we don't care about.  */
+      warning (_("Unrecognized tracepoint piece '%c', ignoring"), piece);
     }
 }
 
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 1da7d26..578ae6b7 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -122,7 +122,14 @@ extern char *default_collect;
 
 /* Struct to collect random info about tracepoints on the target.  */
 
-struct uploaded_tp {
+struct uploaded_string
+{
+  char *str;
+  struct uploaded_string *next;
+};
+
+struct uploaded_tp
+{
   int number;
   enum bptype type;
   ULONGEST addr;
@@ -135,12 +142,23 @@ struct uploaded_tp {
   char *actions[100];
   int num_step_actions;
   char *step_actions[100];
+
+  /* The original string defining the location of the tracepoint.  */
+  char *at_string;
+
+  /* The original string defining the tracepoint's condition.  */
+  char *cond_string;
+
+  /* List of original strings defining the tracepoint's actions.  */
+  struct uploaded_string *cmd_strings;
+
   struct uploaded_tp *next;
 };
 
 /* Struct recording info about trace state variables on the target.  */
 
-struct uploaded_tsv {
+struct uploaded_tsv
+{
   const char *name;
   int number;
   LONGEST initial_value;
@@ -166,6 +184,10 @@ extern void while_stepping_pseudocommand (char *args, int from_tty);
 extern struct trace_state_variable *find_trace_state_variable (const char *name);
 extern struct trace_state_variable *create_trace_state_variable (const char *name);
 
+extern int encode_source_string (int num, ULONGEST addr,
+				 char *srctype, char *src,
+				 char *buf, int buf_size);
+
 extern void parse_trace_status (char *line, struct trace_status *ts);
 
 extern void parse_tracepoint_definition (char *line, struct uploaded_tp **utpp);