public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target
@ 2014-02-12  6:07 Yao Qi
  2014-02-12  6:08 ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Yao Qi
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:07 UTC (permalink / raw)
  To: gdb-patches

In previous patch series, to_xfer_partial interface returns xfer_status and
set *xfered_len when necessary, but no implementation uses it.  In this
series, we start teach to_xfer_partial in target ctf and tfile to use it.

When I modify tfile_xfer_partial and ctf_xfer_partial, I find some of code
is duplicated, and I keep adding new duplicated code into them.  I decide
to refactor them first.  This kind of refactor should be done when I added
ctf target, but I didn't do.

Patch #1 and #2 move code on trace file out of tracepoint.c, which included
a lot of stuffs.  Patch #3 and #5 share some code between ctf and tfile.
Patch #6 really matters to adjust tfile_xfer_partial and ctf_xfer_partial
to set *xfered_len and return TARGET_XFER_E_UNAVAILABLE if data is
unavailable.  Patch #7 simplify read_value_memory.

Regression tested on x86_64-linux, also run gdb.trace on x86-linux 
with babeltrace installed.

*** BLURB HERE ***

Yao Qi (8):
  Move trace file writer out of tracepoint.c
  Move tfile target to tracefile-tfile.c
  Share some code between ctf and tfile target.
  Let tracefile has_memory and has_all_memory.
  Share code on to_xfer_partial for tfile and ctf target
  Use new to_xfer_partial interface in ctf and tfile target
  Adjust read_value_memory to use to_xfer_partial
  Call target_traceframe_info when traceframe is selected.

 gdb/Makefile.in       |    4 +-
 gdb/ctf.c             |   94 +---
 gdb/exec.c            |  108 ++++-
 gdb/exec.h            |   24 +-
 gdb/remote.c          |    5 -
 gdb/target.c          |    2 +-
 gdb/target.h          |   15 +-
 gdb/tracefile-tfile.c | 1073 +++++++++++++++++++++++++++++++++++
 gdb/tracefile.c       |  462 +++++++++++++++
 gdb/tracefile.h       |  116 ++++
 gdb/tracepoint.c      | 1499 +------------------------------------------------
 gdb/tracepoint.h      |  110 +----
 gdb/valops.c          |   90 +---
 13 files changed, 1822 insertions(+), 1780 deletions(-)
 create mode 100644 gdb/tracefile-tfile.c
 create mode 100644 gdb/tracefile.c
 create mode 100644 gdb/tracefile.h

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 8/8] Call target_traceframe_info when traceframe is selected.
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
                   ` (3 preceding siblings ...)
  2014-02-12  6:08 ` [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:30   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 1/8] Move trace file writer out of tracepoint.c Yao Qi
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

As we migrate to the new to_xfer_partial interface, some of previous
tweaks become unnecessary, first, we can check whether traceframe is
selected before call target_traceframe_info.  Then, we don't have to
check traceframe is selected in each target implementation, so this
patch below is reverted.

  [PATCH] Send qXfer:traceframe-info:read when traceframe is selected.
  https://sourceware.org/ml/gdb-patches/2013-10/msg00752.html

Third, to_traceframe_info is only called when traceframe is selected,
that means it is only called when target is remote, tfile or ctf, so
this patch can be partially reverted,

  https://sourceware.org/ml/gdb-patches/2013-04/msg00000.html

gdb:

2014-02-12  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c (get_traceframe_info): Call
	target_traceframe_info if traceframe is selected.

	Revert two patches:

	2013-10-25  Yao Qi  <yao@codesourcery.com>

	* remote.c (remote_traceframe_info): Return early if
	traceframe is not selected.

	2013-07-19  Yao Qi  <yao@codesourcery.com>

	* target.c (update_current_target): Change the default action
	of 'to_traceframe_info' from tcomplain to return_zero.
	* target.h (struct target_ops) <to_traceframe_info>: Add more
	comments.
---
 gdb/remote.c     |    5 -----
 gdb/target.c     |    2 +-
 gdb/target.h     |   15 +++------------
 gdb/tracepoint.c |    2 +-
 4 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index eff4c44..4504512 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11241,11 +11241,6 @@ remote_traceframe_info (void)
 {
   char *text;
 
-  /* If current traceframe is not selected, don't bother the remote
-     stub.  */
-  if (get_traceframe_number () < 0)
-    return NULL;
-
   text = target_read_stralloc (&current_target,
 			       TARGET_OBJECT_TRACEFRAME_INFO, NULL);
   if (text != NULL)
diff --git a/gdb/target.c b/gdb/target.c
index f08dad0..0c19c5a 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -917,7 +917,7 @@ update_current_target (void)
 	    tcomplain);
   de_fault (to_traceframe_info,
 	    (struct traceframe_info * (*) (void))
-	    return_null);
+	    tcomplain);
   de_fault (to_supports_evaluation_of_breakpoint_conditions,
 	    (int (*) (void))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 9fa56f2..5d1955b 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -815,18 +815,9 @@ struct target_ops
       (const char *id);
 
     /* Return a traceframe info object describing the current
-       traceframe's contents.  If the target doesn't support
-       traceframe info, return NULL.  If the current traceframe is not
-       selected (the current traceframe number is -1), the target can
-       choose to return either NULL or an empty traceframe info.  If
-       NULL is returned, for example in remote target, GDB will read
-       from the live inferior.  If an empty traceframe info is
-       returned, for example in tfile target, which means the
-       traceframe info is available, but the requested memory is not
-       available in it.  GDB will try to see if the requested memory
-       is available in the read-only sections.  This method should not
-       cache data; higher layers take care of caching, invalidating,
-       and re-fetching when necessary.  */
+       traceframe's contents.  This method should not cache data;
+       higher layers take care of caching, invalidating, and
+       re-fetching when necessary.  */
     struct traceframe_info *(*to_traceframe_info) (void);
 
     /* Ask the target to use or not to use agent according to USE.  Return 1
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c1dcb1e..1132167 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -4290,7 +4290,7 @@ parse_traceframe_info (const char *tframe_info)
 struct traceframe_info *
 get_traceframe_info (void)
 {
-  if (traceframe_info == NULL)
+  if (traceframe_info == NULL && get_traceframe_number () >= 0)
     traceframe_info = target_traceframe_info ();
 
   return traceframe_info;
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/8] Move tfile target to tracefile-tfile.c
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
  2014-02-12  6:08 ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:14   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 4/8] Let tracefile has_memory and has_all_memory Yao Qi
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

This patch moves tfile target related code from tracepoint.c to
tracefile-tfile.c.

gdb:

2014-02-12  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c (TFILE_PID): Move it to tracefile-tfile.c.
	(tfile_ops): Likewise.
	(TRACE_HEADER_SIZE): Likewise.
	(trace_fd, trace_frames_offset, cur_offset): Likewise.
	(cur_data_size): Likewise.
	(tfile_read, tfile_open, tfile_interp_line): Likewise.
	(tfile_close, tfile_files_info): Likewise.
	(tfile_get_trace_status): Likewise.
	(tfile_get_tracepoint_status): Likewise.
	(tfile_get_traceframe_address): Likewise.
	(tfile_trace_find, match_blocktype): Likewise.
	(traceframe_walk_blocks, traceframe_find_block_type): Likewise.
	(tfile_fetch_registers, tfile_xfer_partial): Likewise.
	(tfile_get_trace_state_variable_value): Likewise.
	(tfile_has_all_memory, tfile_has_memory): Likewise.
	(tfile_has_stack, tfile_has_registers): Likewise.
	(tfile_thread_alive, build_traceframe_info): Likewise.
	(tfile_traceframe_info, init_tfile_ops): Likewise.
	(_initialize_tracepoint): Don't call init_tfile_ops
	and add_target_with_completer.
	* tracefile-tfile.c: Include regcache.h, inferior.h, gdbthread.h,
	exec.h, completer.h and filenames.h.
	(_initialize_tracefile_tfile): New function.
---
 gdb/tracefile-tfile.c |  821 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/tracepoint.c      |  809 ------------------------------------------------
 2 files changed, 821 insertions(+), 809 deletions(-)

diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 97e93fc..1310151 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -22,6 +22,12 @@
 #include "readline/tilde.h"
 #include "filestuff.h"
 #include "remote.h" /* bin2hex */
+#include "regcache.h"
+#include "inferior.h"
+#include "gdbthread.h"
+#include "exec.h" /* exec_bfd */
+#include "completer.h"
+#include "filenames.h"
 
 /* TFILE trace writer.  */
 
@@ -325,3 +331,818 @@ tfile_trace_file_writer_new (void)
 
   return (struct trace_file_writer *) writer;
 }
+
+/* target tfile command */
+
+static struct target_ops tfile_ops;
+
+/* Fill in tfile_ops with its defined operations and properties.  */
+
+#define TRACE_HEADER_SIZE 8
+
+#define TFILE_PID (1)
+
+static char *trace_filename;
+static int trace_fd = -1;
+static off_t trace_frames_offset;
+static off_t cur_offset;
+static int cur_data_size;
+int trace_regblock_size;
+
+static void tfile_interp_line (char *line,
+			       struct uploaded_tp **utpp,
+			       struct uploaded_tsv **utsvp);
+
+/* Read SIZE bytes into READBUF from the trace frame, starting at
+   TRACE_FD's current position.  Note that this call `read'
+   underneath, hence it advances the file's seek position.  Throws an
+   error if the `read' syscall fails, or less than SIZE bytes are
+   read.  */
+
+static void
+tfile_read (gdb_byte *readbuf, int size)
+{
+  int gotten;
+
+  gotten = read (trace_fd, readbuf, size);
+  if (gotten < 0)
+    perror_with_name (trace_filename);
+  else if (gotten < size)
+    error (_("Premature end of file while reading trace file"));
+}
+
+static void
+tfile_open (char *filename, int from_tty)
+{
+  volatile struct gdb_exception ex;
+  char *temp;
+  struct cleanup *old_chain;
+  int flags;
+  int scratch_chan;
+  char header[TRACE_HEADER_SIZE];
+  char linebuf[1000]; /* Should be max remote packet size or so.  */
+  gdb_byte byte;
+  int bytes, i;
+  struct trace_status *ts;
+  struct uploaded_tp *uploaded_tps = NULL;
+  struct uploaded_tsv *uploaded_tsvs = NULL;
+
+  target_preopen (from_tty);
+  if (!filename)
+    error (_("No trace file specified."));
+
+  filename = tilde_expand (filename);
+  if (!IS_ABSOLUTE_PATH(filename))
+    {
+      temp = concat (current_directory, "/", filename, (char *) NULL);
+      xfree (filename);
+      filename = temp;
+    }
+
+  old_chain = make_cleanup (xfree, filename);
+
+  flags = O_BINARY | O_LARGEFILE;
+  flags |= O_RDONLY;
+  scratch_chan = gdb_open_cloexec (filename, flags, 0);
+  if (scratch_chan < 0)
+    perror_with_name (filename);
+
+  /* Looks semi-reasonable.  Toss the old trace file and work on the new.  */
+
+  discard_cleanups (old_chain);	/* Don't free filename any more.  */
+  unpush_target (&tfile_ops);
+
+  trace_filename = xstrdup (filename);
+  trace_fd = scratch_chan;
+
+  bytes = 0;
+  /* Read the file header and test for validity.  */
+  tfile_read ((gdb_byte *) &header, TRACE_HEADER_SIZE);
+
+  bytes += TRACE_HEADER_SIZE;
+  if (!(header[0] == 0x7f
+	&& (strncmp (header + 1, "TRACE0\n", 7) == 0)))
+    error (_("File is not a valid trace file."));
+
+  push_target (&tfile_ops);
+
+  trace_regblock_size = 0;
+  ts = current_trace_status ();
+  /* We know we're working with a file.  Record its name.  */
+  ts->filename = trace_filename;
+  /* Set defaults in case there is no status line.  */
+  ts->running_known = 0;
+  ts->stop_reason = trace_stop_reason_unknown;
+  ts->traceframe_count = -1;
+  ts->buffer_free = 0;
+  ts->disconnected_tracing = 0;
+  ts->circular_buffer = 0;
+
+  TRY_CATCH (ex, RETURN_MASK_ALL)
+    {
+      /* Read through a section of newline-terminated lines that
+	 define things like tracepoints.  */
+      i = 0;
+      while (1)
+	{
+	  tfile_read (&byte, 1);
+
+	  ++bytes;
+	  if (byte == '\n')
+	    {
+	      /* Empty line marks end of the definition section.  */
+	      if (i == 0)
+		break;
+	      linebuf[i] = '\0';
+	      i = 0;
+	      tfile_interp_line (linebuf, &uploaded_tps, &uploaded_tsvs);
+	    }
+	  else
+	    linebuf[i++] = byte;
+	  if (i >= 1000)
+	    error (_("Excessively long lines in trace file"));
+	}
+
+      /* Record the starting offset of the binary trace data.  */
+      trace_frames_offset = bytes;
+
+      /* If we don't have a blocksize, we can't interpret the
+	 traceframes.  */
+      if (trace_regblock_size == 0)
+	error (_("No register block size recorded in trace file"));
+    }
+  if (ex.reason < 0)
+    {
+      /* Remove the partially set up target.  */
+      unpush_target (&tfile_ops);
+      throw_exception (ex);
+    }
+
+  inferior_appeared (current_inferior (), TFILE_PID);
+  inferior_ptid = pid_to_ptid (TFILE_PID);
+  add_thread_silent (inferior_ptid);
+
+  if (ts->traceframe_count <= 0)
+    warning (_("No traceframes present in this file."));
+
+  /* Add the file's tracepoints and variables into the current mix.  */
+
+  /* Get trace state variables first, they may be checked when parsing
+     uploaded commands.  */
+  merge_uploaded_trace_state_variables (&uploaded_tsvs);
+
+  merge_uploaded_tracepoints (&uploaded_tps);
+
+  post_create_inferior (&tfile_ops, from_tty);
+}
+
+/* Interpret the given line from the definitions part of the trace
+   file.  */
+
+static void
+tfile_interp_line (char *line, struct uploaded_tp **utpp,
+		   struct uploaded_tsv **utsvp)
+{
+  char *p = line;
+
+  if (strncmp (p, "R ", strlen ("R ")) == 0)
+    {
+      p += strlen ("R ");
+      trace_regblock_size = strtol (p, &p, 16);
+    }
+  else if (strncmp (p, "status ", strlen ("status ")) == 0)
+    {
+      p += strlen ("status ");
+      parse_trace_status (p, current_trace_status ());
+    }
+  else if (strncmp (p, "tp ", strlen ("tp ")) == 0)
+    {
+      p += strlen ("tp ");
+      parse_tracepoint_definition (p, utpp);
+    }
+  else if (strncmp (p, "tsv ", strlen ("tsv ")) == 0)
+    {
+      p += strlen ("tsv ");
+      parse_tsv_definition (p, utsvp);
+    }
+  else
+    warning (_("Ignoring trace file definition \"%s\""), line);
+}
+
+/* Close the trace file and generally clean up.  */
+
+static void
+tfile_close (void)
+{
+  int pid;
+
+  if (trace_fd < 0)
+    return;
+
+  pid = ptid_get_pid (inferior_ptid);
+  inferior_ptid = null_ptid;	/* Avoid confusion from thread stuff.  */
+  exit_inferior_silent (pid);
+
+  close (trace_fd);
+  trace_fd = -1;
+  xfree (trace_filename);
+  trace_filename = NULL;
+
+  trace_reset_local_state ();
+}
+
+static void
+tfile_files_info (struct target_ops *t)
+{
+  printf_filtered ("\t`%s'\n", trace_filename);
+}
+
+/* The trace status for a file is that tracing can never be run.  */
+
+static int
+tfile_get_trace_status (struct trace_status *ts)
+{
+  /* Other bits of trace status were collected as part of opening the
+     trace files, so nothing to do here.  */
+
+  return -1;
+}
+
+static void
+tfile_get_tracepoint_status (struct breakpoint *tp, struct uploaded_tp *utp)
+{
+  /* Other bits of trace status were collected as part of opening the
+     trace files, so nothing to do here.  */
+}
+
+/* Given the position of a traceframe in the file, figure out what
+   address the frame was collected at.  This would normally be the
+   value of a collected PC register, but if not available, we
+   improvise.  */
+
+static CORE_ADDR
+tfile_get_traceframe_address (off_t tframe_offset)
+{
+  CORE_ADDR addr = 0;
+  short tpnum;
+  struct tracepoint *tp;
+  off_t saved_offset = cur_offset;
+
+  /* FIXME dig pc out of collected registers.  */
+
+  /* Fall back to using tracepoint address.  */
+  lseek (trace_fd, tframe_offset, SEEK_SET);
+  tfile_read ((gdb_byte *) &tpnum, 2);
+  tpnum = (short) extract_signed_integer ((gdb_byte *) &tpnum, 2,
+					  gdbarch_byte_order
+					      (target_gdbarch ()));
+
+  tp = get_tracepoint_by_number_on_target (tpnum);
+  /* FIXME this is a poor heuristic if multiple locations.  */
+  if (tp && tp->base.loc)
+    addr = tp->base.loc->address;
+
+  /* Restore our seek position.  */
+  cur_offset = saved_offset;
+  lseek (trace_fd, cur_offset, SEEK_SET);
+  return addr;
+}
+
+/* Given a type of search and some parameters, scan the collection of
+   traceframes in the file looking for a match.  When found, return
+   both the traceframe and tracepoint number, otherwise -1 for
+   each.  */
+
+static int
+tfile_trace_find (enum trace_find_type type, int num,
+		  CORE_ADDR addr1, CORE_ADDR addr2, int *tpp)
+{
+  short tpnum;
+  int tfnum = 0, found = 0;
+  unsigned int data_size;
+  struct tracepoint *tp;
+  off_t offset, tframe_offset;
+  CORE_ADDR tfaddr;
+
+  if (num == -1)
+    {
+      if (tpp)
+        *tpp = -1;
+      return -1;
+    }
+
+  lseek (trace_fd, trace_frames_offset, SEEK_SET);
+  offset = trace_frames_offset;
+  while (1)
+    {
+      tframe_offset = offset;
+      tfile_read ((gdb_byte *) &tpnum, 2);
+      tpnum = (short) extract_signed_integer ((gdb_byte *) &tpnum, 2,
+					      gdbarch_byte_order
+						  (target_gdbarch ()));
+      offset += 2;
+      if (tpnum == 0)
+	break;
+      tfile_read ((gdb_byte *) &data_size, 4);
+      data_size = (unsigned int) extract_unsigned_integer
+                                     ((gdb_byte *) &data_size, 4,
+				      gdbarch_byte_order (target_gdbarch ()));
+      offset += 4;
+
+      if (type == tfind_number)
+	{
+	  /* Looking for a specific trace frame.  */
+	  if (tfnum == num)
+	    found = 1;
+	}
+      else
+	{
+	  /* Start from the _next_ trace frame.  */
+	  if (tfnum > get_traceframe_number ())
+	    {
+	      switch (type)
+		{
+		case tfind_pc:
+		  tfaddr = tfile_get_traceframe_address (tframe_offset);
+		  if (tfaddr == addr1)
+		    found = 1;
+		  break;
+		case tfind_tp:
+		  tp = get_tracepoint (num);
+		  if (tp && tpnum == tp->number_on_target)
+		    found = 1;
+		  break;
+		case tfind_range:
+		  tfaddr = tfile_get_traceframe_address (tframe_offset);
+		  if (addr1 <= tfaddr && tfaddr <= addr2)
+		    found = 1;
+		  break;
+		case tfind_outside:
+		  tfaddr = tfile_get_traceframe_address (tframe_offset);
+		  if (!(addr1 <= tfaddr && tfaddr <= addr2))
+		    found = 1;
+		  break;
+		default:
+		  internal_error (__FILE__, __LINE__, _("unknown tfind type"));
+		}
+	    }
+	}
+
+      if (found)
+	{
+	  if (tpp)
+	    *tpp = tpnum;
+	  cur_offset = offset;
+	  cur_data_size = data_size;
+
+	  return tfnum;
+	}
+      /* Skip past the traceframe's data.  */
+      lseek (trace_fd, data_size, SEEK_CUR);
+      offset += data_size;
+      /* Update our own count of traceframes.  */
+      ++tfnum;
+    }
+  /* Did not find what we were looking for.  */
+  if (tpp)
+    *tpp = -1;
+  return -1;
+}
+
+/* Prototype of the callback passed to tframe_walk_blocks.  */
+typedef int (*walk_blocks_callback_func) (char blocktype, void *data);
+
+/* Callback for traceframe_walk_blocks, used to find a given block
+   type in a traceframe.  */
+
+static int
+match_blocktype (char blocktype, void *data)
+{
+  char *wantedp = data;
+
+  if (*wantedp == blocktype)
+    return 1;
+
+  return 0;
+}
+
+/* Walk over all traceframe block starting at POS offset from
+   CUR_OFFSET, and call CALLBACK for each block found, passing in DATA
+   unmodified.  If CALLBACK returns true, this returns the position in
+   the traceframe where the block is found, relative to the start of
+   the traceframe (cur_offset).  Returns -1 if no callback call
+   returned true, indicating that all blocks have been walked.  */
+
+static int
+traceframe_walk_blocks (walk_blocks_callback_func callback,
+			int pos, void *data)
+{
+  /* Iterate through a traceframe's blocks, looking for a block of the
+     requested type.  */
+
+  lseek (trace_fd, cur_offset + pos, SEEK_SET);
+  while (pos < cur_data_size)
+    {
+      unsigned short mlen;
+      char block_type;
+
+      tfile_read ((gdb_byte *) &block_type, 1);
+
+      ++pos;
+
+      if ((*callback) (block_type, data))
+	return pos;
+
+      switch (block_type)
+	{
+	case 'R':
+	  lseek (trace_fd, cur_offset + pos + trace_regblock_size, SEEK_SET);
+	  pos += trace_regblock_size;
+	  break;
+	case 'M':
+	  lseek (trace_fd, cur_offset + pos + 8, SEEK_SET);
+	  tfile_read ((gdb_byte *) &mlen, 2);
+          mlen = (unsigned short)
+                extract_unsigned_integer ((gdb_byte *) &mlen, 2,
+                                          gdbarch_byte_order
+                                              (target_gdbarch ()));
+	  lseek (trace_fd, mlen, SEEK_CUR);
+	  pos += (8 + 2 + mlen);
+	  break;
+	case 'V':
+	  lseek (trace_fd, cur_offset + pos + 4 + 8, SEEK_SET);
+	  pos += (4 + 8);
+	  break;
+	default:
+	  error (_("Unknown block type '%c' (0x%x) in trace frame"),
+		 block_type, block_type);
+	  break;
+	}
+    }
+
+  return -1;
+}
+
+/* Convenience wrapper around traceframe_walk_blocks.  Looks for the
+   position offset of a block of type TYPE_WANTED in the current trace
+   frame, starting at POS.  Returns -1 if no such block was found.  */
+
+static int
+traceframe_find_block_type (char type_wanted, int pos)
+{
+  return traceframe_walk_blocks (match_blocktype, pos, &type_wanted);
+}
+
+/* Look for a block of saved registers in the traceframe, and get the
+   requested register from it.  */
+
+static void
+tfile_fetch_registers (struct target_ops *ops,
+		       struct regcache *regcache, int regno)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  int offset, regn, regsize, pc_regno;
+  gdb_byte *regs;
+
+  /* An uninitialized reg size says we're not going to be
+     successful at getting register blocks.  */
+  if (!trace_regblock_size)
+    return;
+
+  regs = alloca (trace_regblock_size);
+
+  if (traceframe_find_block_type ('R', 0) >= 0)
+    {
+      tfile_read (regs, trace_regblock_size);
+
+      /* Assume the block is laid out in GDB register number order,
+	 each register with the size that it has in GDB.  */
+      offset = 0;
+      for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
+	{
+	  regsize = register_size (gdbarch, regn);
+	  /* Make sure we stay within block bounds.  */
+	  if (offset + regsize >= trace_regblock_size)
+	    break;
+	  if (regcache_register_status (regcache, regn) == REG_UNKNOWN)
+	    {
+	      if (regno == regn)
+		{
+		  regcache_raw_supply (regcache, regno, regs + offset);
+		  break;
+		}
+	      else if (regno == -1)
+		{
+		  regcache_raw_supply (regcache, regn, regs + offset);
+		}
+	    }
+	  offset += regsize;
+	}
+      return;
+    }
+
+  /* We get here if no register data has been found.  Mark registers
+     as unavailable.  */
+  for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
+    regcache_raw_supply (regcache, regn, NULL);
+
+  /* We can often usefully guess that the PC is going to be the same
+     as the address of the tracepoint.  */
+  pc_regno = gdbarch_pc_regnum (gdbarch);
+
+  /* XXX This guessing code below only works if the PC register isn't
+     a pseudo-register.  The value of a pseudo-register isn't stored
+     in the (non-readonly) regcache -- instead it's recomputed
+     (probably from some other cached raw register) whenever the
+     register is read.  This guesswork should probably move to some
+     higher layer.  */
+  if (pc_regno < 0 || pc_regno >= gdbarch_num_regs (gdbarch))
+    return;
+
+  if (regno == -1 || regno == pc_regno)
+    {
+      struct tracepoint *tp = get_tracepoint (get_tracepoint_number ());
+
+      if (tp && tp->base.loc)
+	{
+	  /* But don't try to guess if tracepoint is multi-location...  */
+	  if (tp->base.loc->next)
+	    {
+	      warning (_("Tracepoint %d has multiple "
+			 "locations, cannot infer $pc"),
+		       tp->base.number);
+	      return;
+	    }
+	  /* ... or does while-stepping.  */
+	  if (tp->step_count > 0)
+	    {
+	      warning (_("Tracepoint %d does while-stepping, "
+			 "cannot infer $pc"),
+		       tp->base.number);
+	      return;
+	    }
+
+	  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
+				  gdbarch_byte_order (gdbarch),
+				  tp->base.loc->address);
+	  regcache_raw_supply (regcache, pc_regno, regs);
+	}
+    }
+}
+
+static enum target_xfer_status
+tfile_xfer_partial (struct target_ops *ops, enum target_object object,
+		    const char *annex, gdb_byte *readbuf,
+		    const gdb_byte *writebuf, ULONGEST offset, ULONGEST len,
+		    ULONGEST *xfered_len)
+{
+  /* We're only doing regular memory for now.  */
+  if (object != TARGET_OBJECT_MEMORY)
+    return TARGET_XFER_E_IO;
+
+  if (readbuf == NULL)
+    error (_("tfile_xfer_partial: trace file is read-only"));
+
+  if (get_traceframe_number () != -1)
+    {
+      int pos = 0;
+
+      /* Iterate through the traceframe's blocks, looking for
+	 memory.  */
+      while ((pos = traceframe_find_block_type ('M', pos)) >= 0)
+	{
+	  ULONGEST maddr, amt;
+	  unsigned short mlen;
+	  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+
+	  tfile_read ((gdb_byte *) &maddr, 8);
+	  maddr = extract_unsigned_integer ((gdb_byte *) &maddr, 8,
+					    byte_order);
+	  tfile_read ((gdb_byte *) &mlen, 2);
+	  mlen = (unsigned short)
+	    extract_unsigned_integer ((gdb_byte *) &mlen, 2, byte_order);
+
+	  /* If the block includes the first part of the desired
+	     range, return as much it has; GDB will re-request the
+	     remainder, which might be in a different block of this
+	     trace frame.  */
+	  if (maddr <= offset && offset < (maddr + mlen))
+	    {
+	      amt = (maddr + mlen) - offset;
+	      if (amt > len)
+		amt = len;
+
+	      if (maddr != offset)
+	        lseek (trace_fd, offset - maddr, SEEK_CUR);
+	      tfile_read (readbuf, amt);
+	      *xfered_len = amt;
+	      return TARGET_XFER_OK;
+	    }
+
+	  /* Skip over this block.  */
+	  pos += (8 + 2 + mlen);
+	}
+    }
+
+  /* It's unduly pedantic to refuse to look at the executable for
+     read-only pieces; so do the equivalent of readonly regions aka
+     QTro packet.  */
+  /* FIXME account for relocation at some point.  */
+  if (exec_bfd)
+    {
+      asection *s;
+      bfd_size_type size;
+      bfd_vma vma;
+
+      for (s = exec_bfd->sections; s; s = s->next)
+	{
+	  if ((s->flags & SEC_LOAD) == 0
+	      || (s->flags & SEC_READONLY) == 0)
+	    continue;
+
+	  vma = s->vma;
+	  size = bfd_get_section_size (s);
+	  if (vma <= offset && offset < (vma + size))
+	    {
+	      ULONGEST amt;
+
+	      amt = (vma + size) - offset;
+	      if (amt > len)
+		amt = len;
+
+	      *xfered_len = bfd_get_section_contents (exec_bfd, s,
+						      readbuf, offset - vma, amt);
+	      return TARGET_XFER_OK;
+	    }
+	}
+    }
+
+  /* Indicate failure to find the requested memory block.  */
+  return TARGET_XFER_E_IO;
+}
+
+/* Iterate through the blocks of a trace frame, looking for a 'V'
+   block with a matching tsv number.  */
+
+static int
+tfile_get_trace_state_variable_value (int tsvnum, LONGEST *val)
+{
+  int pos;
+  int found = 0;
+
+  /* Iterate over blocks in current frame and find the last 'V'
+     block in which tsv number is TSVNUM.  In one trace frame, there
+     may be multiple 'V' blocks created for a given trace variable,
+     and the last matched 'V' block contains the updated value.  */
+  pos = 0;
+  while ((pos = traceframe_find_block_type ('V', pos)) >= 0)
+    {
+      int vnum;
+
+      tfile_read ((gdb_byte *) &vnum, 4);
+      vnum = (int) extract_signed_integer ((gdb_byte *) &vnum, 4,
+					   gdbarch_byte_order
+					   (target_gdbarch ()));
+      if (tsvnum == vnum)
+	{
+	  tfile_read ((gdb_byte *) val, 8);
+	  *val = extract_signed_integer ((gdb_byte *) val, 8,
+					 gdbarch_byte_order
+					 (target_gdbarch ()));
+	  found = 1;
+	}
+      pos += (4 + 8);
+    }
+
+  return found;
+}
+
+static int
+tfile_has_all_memory (struct target_ops *ops)
+{
+  return 1;
+}
+
+static int
+tfile_has_memory (struct target_ops *ops)
+{
+  return 1;
+}
+
+static int
+tfile_has_stack (struct target_ops *ops)
+{
+  return get_traceframe_number () != -1;
+}
+
+static int
+tfile_has_registers (struct target_ops *ops)
+{
+  return get_traceframe_number () != -1;
+}
+
+static int
+tfile_thread_alive (struct target_ops *ops, ptid_t ptid)
+{
+  return 1;
+}
+
+/* Callback for traceframe_walk_blocks.  Builds a traceframe_info
+   object for the tfile target's current traceframe.  */
+
+static int
+build_traceframe_info (char blocktype, void *data)
+{
+  struct traceframe_info *info = data;
+
+  switch (blocktype)
+    {
+    case 'M':
+      {
+	struct mem_range *r;
+	ULONGEST maddr;
+	unsigned short mlen;
+
+	tfile_read ((gdb_byte *) &maddr, 8);
+	maddr = extract_unsigned_integer ((gdb_byte *) &maddr, 8,
+					  gdbarch_byte_order
+					  (target_gdbarch ()));
+	tfile_read ((gdb_byte *) &mlen, 2);
+	mlen = (unsigned short)
+		extract_unsigned_integer ((gdb_byte *) &mlen,
+					  2, gdbarch_byte_order
+					  (target_gdbarch ()));
+
+	r = VEC_safe_push (mem_range_s, info->memory, NULL);
+
+	r->start = maddr;
+	r->length = mlen;
+	break;
+      }
+    case 'V':
+      {
+	int vnum;
+
+	tfile_read ((gdb_byte *) &vnum, 4);
+	VEC_safe_push (int, info->tvars, vnum);
+      }
+    case 'R':
+    case 'S':
+      {
+	break;
+      }
+    default:
+      warning (_("Unhandled trace block type (%d) '%c ' "
+		 "while building trace frame info."),
+	       blocktype, blocktype);
+      break;
+    }
+
+  return 0;
+}
+
+static struct traceframe_info *
+tfile_traceframe_info (void)
+{
+  struct traceframe_info *info = XCNEW (struct traceframe_info);
+
+  traceframe_walk_blocks (build_traceframe_info, 0, info);
+  return info;
+}
+
+static void
+init_tfile_ops (void)
+{
+  tfile_ops.to_shortname = "tfile";
+  tfile_ops.to_longname = "Local trace dump file";
+  tfile_ops.to_doc
+    = "Use a trace file as a target.  Specify the filename of the trace file.";
+  tfile_ops.to_open = tfile_open;
+  tfile_ops.to_close = tfile_close;
+  tfile_ops.to_fetch_registers = tfile_fetch_registers;
+  tfile_ops.to_xfer_partial = tfile_xfer_partial;
+  tfile_ops.to_files_info = tfile_files_info;
+  tfile_ops.to_get_trace_status = tfile_get_trace_status;
+  tfile_ops.to_get_tracepoint_status = tfile_get_tracepoint_status;
+  tfile_ops.to_trace_find = tfile_trace_find;
+  tfile_ops.to_get_trace_state_variable_value
+    = tfile_get_trace_state_variable_value;
+  tfile_ops.to_stratum = process_stratum;
+  tfile_ops.to_has_all_memory = tfile_has_all_memory;
+  tfile_ops.to_has_memory = tfile_has_memory;
+  tfile_ops.to_has_stack = tfile_has_stack;
+  tfile_ops.to_has_registers = tfile_has_registers;
+  tfile_ops.to_traceframe_info = tfile_traceframe_info;
+  tfile_ops.to_thread_alive = tfile_thread_alive;
+  tfile_ops.to_magic = OPS_MAGIC;
+}
+
+extern initialize_file_ftype _initialize_tracefile_tfile;
+
+void
+_initialize_tracefile_tfile (void)
+{
+  init_tfile_ops ();
+
+  add_target_with_completer (&tfile_ops, filename_completer);
+}
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 45a7fd4..c1dcb1e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -80,8 +80,6 @@
    large.  (400 - 31)/2 == 184 */
 #define MAX_AGENT_EXPR_LEN	184
 
-#define TFILE_PID (1)
-
 /* A hook used to notify the UI of tracepoint operations.  */
 
 void (*deprecated_trace_find_hook) (char *arg, int from_tty);
@@ -3570,201 +3568,6 @@ merge_uploaded_trace_state_variables (struct uploaded_tsv **uploaded_tsvs)
   free_uploaded_tsvs (uploaded_tsvs);
 }
 
-/* target tfile command */
-
-static struct target_ops tfile_ops;
-
-/* Fill in tfile_ops with its defined operations and properties.  */
-
-#define TRACE_HEADER_SIZE 8
-
-static char *trace_filename;
-static int trace_fd = -1;
-static off_t trace_frames_offset;
-static off_t cur_offset;
-static int cur_data_size;
-int trace_regblock_size;
-
-static void tfile_interp_line (char *line,
-			       struct uploaded_tp **utpp,
-			       struct uploaded_tsv **utsvp);
-
-/* Read SIZE bytes into READBUF from the trace frame, starting at
-   TRACE_FD's current position.  Note that this call `read'
-   underneath, hence it advances the file's seek position.  Throws an
-   error if the `read' syscall fails, or less than SIZE bytes are
-   read.  */
-
-static void
-tfile_read (gdb_byte *readbuf, int size)
-{
-  int gotten;
-
-  gotten = read (trace_fd, readbuf, size);
-  if (gotten < 0)
-    perror_with_name (trace_filename);
-  else if (gotten < size)
-    error (_("Premature end of file while reading trace file"));
-}
-
-static void
-tfile_open (char *filename, int from_tty)
-{
-  volatile struct gdb_exception ex;
-  char *temp;
-  struct cleanup *old_chain;
-  int flags;
-  int scratch_chan;
-  char header[TRACE_HEADER_SIZE];
-  char linebuf[1000]; /* Should be max remote packet size or so.  */
-  gdb_byte byte;
-  int bytes, i;
-  struct trace_status *ts;
-  struct uploaded_tp *uploaded_tps = NULL;
-  struct uploaded_tsv *uploaded_tsvs = NULL;
-
-  target_preopen (from_tty);
-  if (!filename)
-    error (_("No trace file specified."));
-
-  filename = tilde_expand (filename);
-  if (!IS_ABSOLUTE_PATH(filename))
-    {
-      temp = concat (current_directory, "/", filename, (char *) NULL);
-      xfree (filename);
-      filename = temp;
-    }
-
-  old_chain = make_cleanup (xfree, filename);
-
-  flags = O_BINARY | O_LARGEFILE;
-  flags |= O_RDONLY;
-  scratch_chan = gdb_open_cloexec (filename, flags, 0);
-  if (scratch_chan < 0)
-    perror_with_name (filename);
-
-  /* Looks semi-reasonable.  Toss the old trace file and work on the new.  */
-
-  discard_cleanups (old_chain);	/* Don't free filename any more.  */
-  unpush_target (&tfile_ops);
-
-  trace_filename = xstrdup (filename);
-  trace_fd = scratch_chan;
-
-  bytes = 0;
-  /* Read the file header and test for validity.  */
-  tfile_read ((gdb_byte *) &header, TRACE_HEADER_SIZE);
-
-  bytes += TRACE_HEADER_SIZE;
-  if (!(header[0] == 0x7f
-	&& (strncmp (header + 1, "TRACE0\n", 7) == 0)))
-    error (_("File is not a valid trace file."));
-
-  push_target (&tfile_ops);
-
-  trace_regblock_size = 0;
-  ts = current_trace_status ();
-  /* We know we're working with a file.  Record its name.  */
-  ts->filename = trace_filename;
-  /* Set defaults in case there is no status line.  */
-  ts->running_known = 0;
-  ts->stop_reason = trace_stop_reason_unknown;
-  ts->traceframe_count = -1;
-  ts->buffer_free = 0;
-  ts->disconnected_tracing = 0;
-  ts->circular_buffer = 0;
-
-  TRY_CATCH (ex, RETURN_MASK_ALL)
-    {
-      /* Read through a section of newline-terminated lines that
-	 define things like tracepoints.  */
-      i = 0;
-      while (1)
-	{
-	  tfile_read (&byte, 1);
-
-	  ++bytes;
-	  if (byte == '\n')
-	    {
-	      /* Empty line marks end of the definition section.  */
-	      if (i == 0)
-		break;
-	      linebuf[i] = '\0';
-	      i = 0;
-	      tfile_interp_line (linebuf, &uploaded_tps, &uploaded_tsvs);
-	    }
-	  else
-	    linebuf[i++] = byte;
-	  if (i >= 1000)
-	    error (_("Excessively long lines in trace file"));
-	}
-
-      /* Record the starting offset of the binary trace data.  */
-      trace_frames_offset = bytes;
-
-      /* If we don't have a blocksize, we can't interpret the
-	 traceframes.  */
-      if (trace_regblock_size == 0)
-	error (_("No register block size recorded in trace file"));
-    }
-  if (ex.reason < 0)
-    {
-      /* Remove the partially set up target.  */
-      unpush_target (&tfile_ops);
-      throw_exception (ex);
-    }
-
-  inferior_appeared (current_inferior (), TFILE_PID);
-  inferior_ptid = pid_to_ptid (TFILE_PID);
-  add_thread_silent (inferior_ptid);
-
-  if (ts->traceframe_count <= 0)
-    warning (_("No traceframes present in this file."));
-
-  /* Add the file's tracepoints and variables into the current mix.  */
-
-  /* Get trace state variables first, they may be checked when parsing
-     uploaded commands.  */
-  merge_uploaded_trace_state_variables (&uploaded_tsvs);
-
-  merge_uploaded_tracepoints (&uploaded_tps);
-
-  post_create_inferior (&tfile_ops, from_tty);
-}
-
-/* Interpret the given line from the definitions part of the trace
-   file.  */
-
-static void
-tfile_interp_line (char *line, struct uploaded_tp **utpp,
-		   struct uploaded_tsv **utsvp)
-{
-  char *p = line;
-
-  if (strncmp (p, "R ", strlen ("R ")) == 0)
-    {
-      p += strlen ("R ");
-      trace_regblock_size = strtol (p, &p, 16);
-    }
-  else if (strncmp (p, "status ", strlen ("status ")) == 0)
-    {
-      p += strlen ("status ");
-      parse_trace_status (p, current_trace_status ());
-    }
-  else if (strncmp (p, "tp ", strlen ("tp ")) == 0)
-    {
-      p += strlen ("tp ");
-      parse_tracepoint_definition (p, utpp);
-    }
-  else if (strncmp (p, "tsv ", strlen ("tsv ")) == 0)
-    {
-      p += strlen ("tsv ");
-      parse_tsv_definition (p, utsvp);
-    }
-  else
-    warning (_("Ignoring trace file definition \"%s\""), line);
-}
-
 /* Parse the part of trace status syntax that is shared between
    the remote protocol and the trace file reader.  */
 
@@ -4092,614 +3895,6 @@ parse_tsv_definition (char *line, struct uploaded_tsv **utsvp)
   utsv->name = xstrdup (buf);
 }
 
-/* Close the trace file and generally clean up.  */
-
-static void
-tfile_close (void)
-{
-  int pid;
-
-  if (trace_fd < 0)
-    return;
-
-  pid = ptid_get_pid (inferior_ptid);
-  inferior_ptid = null_ptid;	/* Avoid confusion from thread stuff.  */
-  exit_inferior_silent (pid);
-
-  close (trace_fd);
-  trace_fd = -1;
-  xfree (trace_filename);
-  trace_filename = NULL;
-
-  trace_reset_local_state ();
-}
-
-static void
-tfile_files_info (struct target_ops *t)
-{
-  printf_filtered ("\t`%s'\n", trace_filename);
-}
-
-/* The trace status for a file is that tracing can never be run.  */
-
-static int
-tfile_get_trace_status (struct trace_status *ts)
-{
-  /* Other bits of trace status were collected as part of opening the
-     trace files, so nothing to do here.  */
-
-  return -1;
-}
-
-static void
-tfile_get_tracepoint_status (struct breakpoint *tp, struct uploaded_tp *utp)
-{
-  /* Other bits of trace status were collected as part of opening the
-     trace files, so nothing to do here.  */
-}
-
-/* Given the position of a traceframe in the file, figure out what
-   address the frame was collected at.  This would normally be the
-   value of a collected PC register, but if not available, we
-   improvise.  */
-
-static CORE_ADDR
-tfile_get_traceframe_address (off_t tframe_offset)
-{
-  CORE_ADDR addr = 0;
-  short tpnum;
-  struct tracepoint *tp;
-  off_t saved_offset = cur_offset;
-
-  /* FIXME dig pc out of collected registers.  */
-
-  /* Fall back to using tracepoint address.  */
-  lseek (trace_fd, tframe_offset, SEEK_SET);
-  tfile_read ((gdb_byte *) &tpnum, 2);
-  tpnum = (short) extract_signed_integer ((gdb_byte *) &tpnum, 2,
-					  gdbarch_byte_order
-					      (target_gdbarch ()));
-
-  tp = get_tracepoint_by_number_on_target (tpnum);
-  /* FIXME this is a poor heuristic if multiple locations.  */
-  if (tp && tp->base.loc)
-    addr = tp->base.loc->address;
-
-  /* Restore our seek position.  */
-  cur_offset = saved_offset;
-  lseek (trace_fd, cur_offset, SEEK_SET);
-  return addr;
-}
-
-/* Given a type of search and some parameters, scan the collection of
-   traceframes in the file looking for a match.  When found, return
-   both the traceframe and tracepoint number, otherwise -1 for
-   each.  */
-
-static int
-tfile_trace_find (enum trace_find_type type, int num,
-		  CORE_ADDR addr1, CORE_ADDR addr2, int *tpp)
-{
-  short tpnum;
-  int tfnum = 0, found = 0;
-  unsigned int data_size;
-  struct tracepoint *tp;
-  off_t offset, tframe_offset;
-  CORE_ADDR tfaddr;
-
-  if (num == -1)
-    {
-      if (tpp)
-        *tpp = -1;
-      return -1;
-    }
-
-  lseek (trace_fd, trace_frames_offset, SEEK_SET);
-  offset = trace_frames_offset;
-  while (1)
-    {
-      tframe_offset = offset;
-      tfile_read ((gdb_byte *) &tpnum, 2);
-      tpnum = (short) extract_signed_integer ((gdb_byte *) &tpnum, 2,
-					      gdbarch_byte_order
-						  (target_gdbarch ()));
-      offset += 2;
-      if (tpnum == 0)
-	break;
-      tfile_read ((gdb_byte *) &data_size, 4);
-      data_size = (unsigned int) extract_unsigned_integer
-                                     ((gdb_byte *) &data_size, 4,
-				      gdbarch_byte_order (target_gdbarch ()));
-      offset += 4;
-
-      if (type == tfind_number)
-	{
-	  /* Looking for a specific trace frame.  */
-	  if (tfnum == num)
-	    found = 1;
-	}
-      else
-	{
-	  /* Start from the _next_ trace frame.  */
-	  if (tfnum > traceframe_number)
-	    {
-	      switch (type)
-		{
-		case tfind_pc:
-		  tfaddr = tfile_get_traceframe_address (tframe_offset);
-		  if (tfaddr == addr1)
-		    found = 1;
-		  break;
-		case tfind_tp:
-		  tp = get_tracepoint (num);
-		  if (tp && tpnum == tp->number_on_target)
-		    found = 1;
-		  break;
-		case tfind_range:
-		  tfaddr = tfile_get_traceframe_address (tframe_offset);
-		  if (addr1 <= tfaddr && tfaddr <= addr2)
-		    found = 1;
-		  break;
-		case tfind_outside:
-		  tfaddr = tfile_get_traceframe_address (tframe_offset);
-		  if (!(addr1 <= tfaddr && tfaddr <= addr2))
-		    found = 1;
-		  break;
-		default:
-		  internal_error (__FILE__, __LINE__, _("unknown tfind type"));
-		}
-	    }
-	}
-
-      if (found)
-	{
-	  if (tpp)
-	    *tpp = tpnum;
-	  cur_offset = offset;
-	  cur_data_size = data_size;
-
-	  return tfnum;
-	}
-      /* Skip past the traceframe's data.  */
-      lseek (trace_fd, data_size, SEEK_CUR);
-      offset += data_size;
-      /* Update our own count of traceframes.  */
-      ++tfnum;
-    }
-  /* Did not find what we were looking for.  */
-  if (tpp)
-    *tpp = -1;
-  return -1;
-}
-
-/* Prototype of the callback passed to tframe_walk_blocks.  */
-typedef int (*walk_blocks_callback_func) (char blocktype, void *data);
-
-/* Callback for traceframe_walk_blocks, used to find a given block
-   type in a traceframe.  */
-
-static int
-match_blocktype (char blocktype, void *data)
-{
-  char *wantedp = data;
-
-  if (*wantedp == blocktype)
-    return 1;
-
-  return 0;
-}
-
-/* Walk over all traceframe block starting at POS offset from
-   CUR_OFFSET, and call CALLBACK for each block found, passing in DATA
-   unmodified.  If CALLBACK returns true, this returns the position in
-   the traceframe where the block is found, relative to the start of
-   the traceframe (cur_offset).  Returns -1 if no callback call
-   returned true, indicating that all blocks have been walked.  */
-
-static int
-traceframe_walk_blocks (walk_blocks_callback_func callback,
-			int pos, void *data)
-{
-  /* Iterate through a traceframe's blocks, looking for a block of the
-     requested type.  */
-
-  lseek (trace_fd, cur_offset + pos, SEEK_SET);
-  while (pos < cur_data_size)
-    {
-      unsigned short mlen;
-      char block_type;
-
-      tfile_read ((gdb_byte *) &block_type, 1);
-
-      ++pos;
-
-      if ((*callback) (block_type, data))
-	return pos;
-
-      switch (block_type)
-	{
-	case 'R':
-	  lseek (trace_fd, cur_offset + pos + trace_regblock_size, SEEK_SET);
-	  pos += trace_regblock_size;
-	  break;
-	case 'M':
-	  lseek (trace_fd, cur_offset + pos + 8, SEEK_SET);
-	  tfile_read ((gdb_byte *) &mlen, 2);
-          mlen = (unsigned short)
-                extract_unsigned_integer ((gdb_byte *) &mlen, 2,
-                                          gdbarch_byte_order
-                                              (target_gdbarch ()));
-	  lseek (trace_fd, mlen, SEEK_CUR);
-	  pos += (8 + 2 + mlen);
-	  break;
-	case 'V':
-	  lseek (trace_fd, cur_offset + pos + 4 + 8, SEEK_SET);
-	  pos += (4 + 8);
-	  break;
-	default:
-	  error (_("Unknown block type '%c' (0x%x) in trace frame"),
-		 block_type, block_type);
-	  break;
-	}
-    }
-
-  return -1;
-}
-
-/* Convenience wrapper around traceframe_walk_blocks.  Looks for the
-   position offset of a block of type TYPE_WANTED in the current trace
-   frame, starting at POS.  Returns -1 if no such block was found.  */
-
-static int
-traceframe_find_block_type (char type_wanted, int pos)
-{
-  return traceframe_walk_blocks (match_blocktype, pos, &type_wanted);
-}
-
-/* Look for a block of saved registers in the traceframe, and get the
-   requested register from it.  */
-
-static void
-tfile_fetch_registers (struct target_ops *ops,
-		       struct regcache *regcache, int regno)
-{
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  int offset, regn, regsize, pc_regno;
-  gdb_byte *regs;
-
-  /* An uninitialized reg size says we're not going to be
-     successful at getting register blocks.  */
-  if (!trace_regblock_size)
-    return;
-
-  regs = alloca (trace_regblock_size);
-
-  if (traceframe_find_block_type ('R', 0) >= 0)
-    {
-      tfile_read (regs, trace_regblock_size);
-
-      /* Assume the block is laid out in GDB register number order,
-	 each register with the size that it has in GDB.  */
-      offset = 0;
-      for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
-	{
-	  regsize = register_size (gdbarch, regn);
-	  /* Make sure we stay within block bounds.  */
-	  if (offset + regsize >= trace_regblock_size)
-	    break;
-	  if (regcache_register_status (regcache, regn) == REG_UNKNOWN)
-	    {
-	      if (regno == regn)
-		{
-		  regcache_raw_supply (regcache, regno, regs + offset);
-		  break;
-		}
-	      else if (regno == -1)
-		{
-		  regcache_raw_supply (regcache, regn, regs + offset);
-		}
-	    }
-	  offset += regsize;
-	}
-      return;
-    }
-
-  /* We get here if no register data has been found.  Mark registers
-     as unavailable.  */
-  for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
-    regcache_raw_supply (regcache, regn, NULL);
-
-  /* We can often usefully guess that the PC is going to be the same
-     as the address of the tracepoint.  */
-  pc_regno = gdbarch_pc_regnum (gdbarch);
-
-  /* XXX This guessing code below only works if the PC register isn't
-     a pseudo-register.  The value of a pseudo-register isn't stored
-     in the (non-readonly) regcache -- instead it's recomputed
-     (probably from some other cached raw register) whenever the
-     register is read.  This guesswork should probably move to some
-     higher layer.  */
-  if (pc_regno < 0 || pc_regno >= gdbarch_num_regs (gdbarch))
-    return;
-
-  if (regno == -1 || regno == pc_regno)
-    {
-      struct tracepoint *tp = get_tracepoint (tracepoint_number);
-
-      if (tp && tp->base.loc)
-	{
-	  /* But don't try to guess if tracepoint is multi-location...  */
-	  if (tp->base.loc->next)
-	    {
-	      warning (_("Tracepoint %d has multiple "
-			 "locations, cannot infer $pc"),
-		       tp->base.number);
-	      return;
-	    }
-	  /* ... or does while-stepping.  */
-	  if (tp->step_count > 0)
-	    {
-	      warning (_("Tracepoint %d does while-stepping, "
-			 "cannot infer $pc"),
-		       tp->base.number);
-	      return;
-	    }
-
-	  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
-				  gdbarch_byte_order (gdbarch),
-				  tp->base.loc->address);
-	  regcache_raw_supply (regcache, pc_regno, regs);
-	}
-    }
-}
-
-static enum target_xfer_status
-tfile_xfer_partial (struct target_ops *ops, enum target_object object,
-		    const char *annex, gdb_byte *readbuf,
-		    const gdb_byte *writebuf, ULONGEST offset, ULONGEST len,
-		    ULONGEST *xfered_len)
-{
-  /* We're only doing regular memory for now.  */
-  if (object != TARGET_OBJECT_MEMORY)
-    return TARGET_XFER_E_IO;
-
-  if (readbuf == NULL)
-    error (_("tfile_xfer_partial: trace file is read-only"));
-
- if (traceframe_number != -1)
-    {
-      int pos = 0;
-
-      /* Iterate through the traceframe's blocks, looking for
-	 memory.  */
-      while ((pos = traceframe_find_block_type ('M', pos)) >= 0)
-	{
-	  ULONGEST maddr, amt;
-	  unsigned short mlen;
-	  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-
-	  tfile_read ((gdb_byte *) &maddr, 8);
-	  maddr = extract_unsigned_integer ((gdb_byte *) &maddr, 8,
-					    byte_order);
-	  tfile_read ((gdb_byte *) &mlen, 2);
-	  mlen = (unsigned short)
-	    extract_unsigned_integer ((gdb_byte *) &mlen, 2, byte_order);
-
-	  /* If the block includes the first part of the desired
-	     range, return as much it has; GDB will re-request the
-	     remainder, which might be in a different block of this
-	     trace frame.  */
-	  if (maddr <= offset && offset < (maddr + mlen))
-	    {
-	      amt = (maddr + mlen) - offset;
-	      if (amt > len)
-		amt = len;
-
-	      if (maddr != offset)
-	        lseek (trace_fd, offset - maddr, SEEK_CUR);
-	      tfile_read (readbuf, amt);
-	      *xfered_len = amt;
-	      return TARGET_XFER_OK;
-	    }
-
-	  /* Skip over this block.  */
-	  pos += (8 + 2 + mlen);
-	}
-    }
-
-  /* It's unduly pedantic to refuse to look at the executable for
-     read-only pieces; so do the equivalent of readonly regions aka
-     QTro packet.  */
-  /* FIXME account for relocation at some point.  */
-  if (exec_bfd)
-    {
-      asection *s;
-      bfd_size_type size;
-      bfd_vma vma;
-
-      for (s = exec_bfd->sections; s; s = s->next)
-	{
-	  if ((s->flags & SEC_LOAD) == 0
-	      || (s->flags & SEC_READONLY) == 0)
-	    continue;
-
-	  vma = s->vma;
-	  size = bfd_get_section_size (s);
-	  if (vma <= offset && offset < (vma + size))
-	    {
-	      ULONGEST amt;
-
-	      amt = (vma + size) - offset;
-	      if (amt > len)
-		amt = len;
-
-	      *xfered_len = bfd_get_section_contents (exec_bfd, s,
-						      readbuf, offset - vma, amt);
-	      return TARGET_XFER_OK;
-	    }
-	}
-    }
-
-  /* Indicate failure to find the requested memory block.  */
-  return TARGET_XFER_E_IO;
-}
-
-/* Iterate through the blocks of a trace frame, looking for a 'V'
-   block with a matching tsv number.  */
-
-static int
-tfile_get_trace_state_variable_value (int tsvnum, LONGEST *val)
-{
-  int pos;
-  int found = 0;
-
-  /* Iterate over blocks in current frame and find the last 'V'
-     block in which tsv number is TSVNUM.  In one trace frame, there
-     may be multiple 'V' blocks created for a given trace variable,
-     and the last matched 'V' block contains the updated value.  */
-  pos = 0;
-  while ((pos = traceframe_find_block_type ('V', pos)) >= 0)
-    {
-      int vnum;
-
-      tfile_read ((gdb_byte *) &vnum, 4);
-      vnum = (int) extract_signed_integer ((gdb_byte *) &vnum, 4,
-					   gdbarch_byte_order
-					   (target_gdbarch ()));
-      if (tsvnum == vnum)
-	{
-	  tfile_read ((gdb_byte *) val, 8);
-	  *val = extract_signed_integer ((gdb_byte *) val, 8,
-					 gdbarch_byte_order
-					 (target_gdbarch ()));
-	  found = 1;
-	}
-      pos += (4 + 8);
-    }
-
-  return found;
-}
-
-static int
-tfile_has_all_memory (struct target_ops *ops)
-{
-  return 1;
-}
-
-static int
-tfile_has_memory (struct target_ops *ops)
-{
-  return 1;
-}
-
-static int
-tfile_has_stack (struct target_ops *ops)
-{
-  return traceframe_number != -1;
-}
-
-static int
-tfile_has_registers (struct target_ops *ops)
-{
-  return traceframe_number != -1;
-}
-
-static int
-tfile_thread_alive (struct target_ops *ops, ptid_t ptid)
-{
-  return 1;
-}
-
-/* Callback for traceframe_walk_blocks.  Builds a traceframe_info
-   object for the tfile target's current traceframe.  */
-
-static int
-build_traceframe_info (char blocktype, void *data)
-{
-  struct traceframe_info *info = data;
-
-  switch (blocktype)
-    {
-    case 'M':
-      {
-	struct mem_range *r;
-	ULONGEST maddr;
-	unsigned short mlen;
-
-	tfile_read ((gdb_byte *) &maddr, 8);
-	maddr = extract_unsigned_integer ((gdb_byte *) &maddr, 8,
-					  gdbarch_byte_order
-					  (target_gdbarch ()));
-	tfile_read ((gdb_byte *) &mlen, 2);
-	mlen = (unsigned short)
-		extract_unsigned_integer ((gdb_byte *) &mlen,
-					  2, gdbarch_byte_order
-					  (target_gdbarch ()));
-
-	r = VEC_safe_push (mem_range_s, info->memory, NULL);
-
-	r->start = maddr;
-	r->length = mlen;
-	break;
-      }
-    case 'V':
-      {
-	int vnum;
-
-	tfile_read ((gdb_byte *) &vnum, 4);
-	VEC_safe_push (int, info->tvars, vnum);
-      }
-    case 'R':
-    case 'S':
-      {
-	break;
-      }
-    default:
-      warning (_("Unhandled trace block type (%d) '%c ' "
-		 "while building trace frame info."),
-	       blocktype, blocktype);
-      break;
-    }
-
-  return 0;
-}
-
-static struct traceframe_info *
-tfile_traceframe_info (void)
-{
-  struct traceframe_info *info = XCNEW (struct traceframe_info);
-
-  traceframe_walk_blocks (build_traceframe_info, 0, info);
-  return info;
-}
-
-static void
-init_tfile_ops (void)
-{
-  tfile_ops.to_shortname = "tfile";
-  tfile_ops.to_longname = "Local trace dump file";
-  tfile_ops.to_doc
-    = "Use a trace file as a target.  Specify the filename of the trace file.";
-  tfile_ops.to_open = tfile_open;
-  tfile_ops.to_close = tfile_close;
-  tfile_ops.to_fetch_registers = tfile_fetch_registers;
-  tfile_ops.to_xfer_partial = tfile_xfer_partial;
-  tfile_ops.to_files_info = tfile_files_info;
-  tfile_ops.to_get_trace_status = tfile_get_trace_status;
-  tfile_ops.to_get_tracepoint_status = tfile_get_tracepoint_status;
-  tfile_ops.to_trace_find = tfile_trace_find;
-  tfile_ops.to_get_trace_state_variable_value
-    = tfile_get_trace_state_variable_value;
-  tfile_ops.to_stratum = process_stratum;
-  tfile_ops.to_has_all_memory = tfile_has_all_memory;
-  tfile_ops.to_has_memory = tfile_has_memory;
-  tfile_ops.to_has_stack = tfile_has_stack;
-  tfile_ops.to_has_registers = tfile_has_registers;
-  tfile_ops.to_traceframe_info = tfile_traceframe_info;
-  tfile_ops.to_thread_alive = tfile_thread_alive;
-  tfile_ops.to_magic = OPS_MAGIC;
-}
-
 void
 free_current_marker (void *arg)
 {
@@ -5361,8 +4556,4 @@ Set notes string to use for future tstop commands"), _("\
 Show the notes string to use for future tstop commands"), NULL,
 			  set_trace_stop_notes, NULL,
 			  &setlist, &showlist);
-
-  init_tfile_ops ();
-
-  add_target_with_completer (&tfile_ops, filename_completer);
 }
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 14:06   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 2/8] Move tfile target to tracefile-tfile.c Yao Qi
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

As the new to_xfer_partial implementations are done in ctf and tfile
targets, read_value_memory can be simplified a lot.  Call
target_xfer_partial in a loop, check return value, and set bytes
unavailable when necessary.

gdb:

2014-02-11  Yao Qi  <yao@codesourcery.com>

	* valops.c (read_value_memory): Rewrite it.  Call
	target_xfer_partial in a loop.
---
 gdb/valops.c |   90 ++++++++++++---------------------------------------------
 1 files changed, 19 insertions(+), 71 deletions(-)

diff --git a/gdb/valops.c b/gdb/valops.c
index 898401d..ec87409 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -949,81 +949,29 @@ read_value_memory (struct value *val, int embedded_offset,
 		   int stack, CORE_ADDR memaddr,
 		   gdb_byte *buffer, size_t length)
 {
-  if (length)
-    {
-      VEC(mem_range_s) *available_memory;
-
-      if (!traceframe_available_memory (&available_memory, memaddr, length))
-	{
-	  if (stack)
-	    read_stack (memaddr, buffer, length);
-	  else
-	    read_memory (memaddr, buffer, length);
-	}
-      else
-	{
-	  struct target_section_table *table;
-	  struct cleanup *old_chain;
-	  CORE_ADDR unavail;
-	  mem_range_s *r;
-	  int i;
-
-	  /* Fallback to reading from read-only sections.  */
-	  table = target_get_section_table (&exec_ops);
-	  available_memory =
-	    section_table_available_memory (available_memory,
-					    memaddr, length,
-					    table->sections,
-					    table->sections_end);
-
-	  old_chain = make_cleanup (VEC_cleanup(mem_range_s),
-				    &available_memory);
+  ULONGEST xfered = 0;
 
-	  normalize_mem_ranges (available_memory);
-
-	  /* Mark which bytes are unavailable, and read those which
-	     are available.  */
-
-	  unavail = memaddr;
+  while (xfered < length)
+    {
+      enum target_xfer_status status;
+      ULONGEST xfered_len;
 
-	  for (i = 0;
-	       VEC_iterate (mem_range_s, available_memory, i, r);
-	       i++)
-	    {
-	      if (mem_ranges_overlap (r->start, r->length,
-				      memaddr, length))
-		{
-		  CORE_ADDR lo1, hi1, lo2, hi2;
-		  CORE_ADDR start, end;
-
-		  /* Get the intersection window.  */
-		  lo1 = memaddr;
-		  hi1 = memaddr + length;
-		  lo2 = r->start;
-		  hi2 = r->start + r->length;
-		  start = max (lo1, lo2);
-		  end = min (hi1, hi2);
-
-		  gdb_assert (end - memaddr <= length);
-
-		  if (start > unavail)
-		    mark_value_bytes_unavailable (val,
-						  (embedded_offset
-						   + unavail - memaddr),
-						  start - unavail);
-		  unavail = end;
-
-		  read_memory (start, buffer + start - memaddr, end - start);
-		}
-	    }
+      status = target_xfer_partial (current_target.beneath,
+				    TARGET_OBJECT_MEMORY, NULL,
+				    buffer + xfered, NULL,
+				    memaddr + xfered, length - xfered,
+				    &xfered_len);
 
-	  if (unavail != memaddr + length)
-	    mark_value_bytes_unavailable (val,
-					  embedded_offset + unavail - memaddr,
-					  (memaddr + length) - unavail);
+      if (status == TARGET_XFER_EOF)
+	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
+      else if (status == TARGET_XFER_E_UNAVAILABLE)
+	mark_value_bytes_unavailable (val, embedded_offset + xfered,
+				      xfered_len);
+      else if (TARGET_XFER_STATUS_ERROR_P (status))
+	memory_error (status, memaddr + xfered);
 
-	  do_cleanups (old_chain);
-	}
+      xfered += xfered_len;
+      QUIT;
     }
 }
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 4/8] Let tracefile has_memory and has_all_memory.
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
  2014-02-12  6:08 ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Yao Qi
  2014-02-12  6:08 ` [PATCH 2/8] Move tfile target to tracefile-tfile.c Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:15   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target Yao Qi
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

At present, tfile target thinks it has memory but ctf doesn't.
This is an oversight when I added ctf target support.  This patch
moves the implementations of to_has_all_memory and to_has_memory to
upper layer.  After this change, both tfile and ctf target think
they have memory.

gdb:

2014-02-122  Yao Qi  <yao@codesourcery.com>

	* tracefile-tfile.c (tfile_has_all_memory): Remove.
	(tfile_has_memory): Remove.
	(init_tfile_ops): Don't set fields to_has_all_memory and
	to_has_memory of tfile_ops.
	* tracefile.c (tracefile_has_all_memory): New function.
	(tracefile_has_memory): New function.
	(init_tracefile_ops): Initialize fields to_has_all_memory and
	to_has_memory of 'ops'.
---
 gdb/tracefile-tfile.c |   14 --------------
 gdb/tracefile.c       |   18 ++++++++++++++++++
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 205494e..80df960 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -1006,18 +1006,6 @@ tfile_get_trace_state_variable_value (int tsvnum, LONGEST *val)
   return found;
 }
 
-static int
-tfile_has_all_memory (struct target_ops *ops)
-{
-  return 1;
-}
-
-static int
-tfile_has_memory (struct target_ops *ops)
-{
-  return 1;
-}
-
 /* Callback for traceframe_walk_blocks.  Builds a traceframe_info
    object for the tfile target's current traceframe.  */
 
@@ -1099,8 +1087,6 @@ init_tfile_ops (void)
   tfile_ops.to_trace_find = tfile_trace_find;
   tfile_ops.to_get_trace_state_variable_value
     = tfile_get_trace_state_variable_value;
-  tfile_ops.to_has_all_memory = tfile_has_all_memory;
-  tfile_ops.to_has_memory = tfile_has_memory;
   tfile_ops.to_traceframe_info = tfile_traceframe_info;
 }
 
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index 0f8ffc5..a2dc9fd 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -376,6 +376,22 @@ trace_save_ctf (const char *dirname, int target_does_save)
   do_cleanups (back_to);
 }
 
+/* This is the implementation of target_ops method to_has_all_memory.  */
+
+static int
+tracefile_has_all_memory (struct target_ops *ops)
+{
+  return 1;
+}
+
+/* This is the implementation of target_ops method to_has_memory.  */
+
+static int
+tracefile_has_memory (struct target_ops *ops)
+{
+  return 1;
+}
+
 /* This is the implementation of target_ops method to_has_stack.
    The target has a stack when GDB has already selected one trace
    frame.  */
@@ -424,6 +440,8 @@ init_tracefile_ops (struct target_ops *ops)
 {
   ops->to_stratum = process_stratum;
   ops->to_get_trace_status = tracefile_get_trace_status;
+  ops->to_has_all_memory = tracefile_has_all_memory;
+  ops->to_has_memory = tracefile_has_memory;
   ops->to_has_stack = tracefile_has_stack;
   ops->to_has_registers = tracefile_has_registers;
   ops->to_thread_alive = tracefile_thread_alive;
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/8] Move trace file writer out of tracepoint.c
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
                   ` (4 preceding siblings ...)
  2014-02-12  6:08 ` [PATCH 8/8] Call target_traceframe_info when traceframe is selected Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:14   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
  2014-02-12  6:08 ` [PATCH 3/8] Share some code between " Yao Qi
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

This patch is a refactor which moves trace file writer related code
out of tracepoint.c, which has 6k LOC.  It moves general trace file
writer to a new file tracefile.c and moves tfile specific writer to
tracefile-tfile.c.

gdb:

2014-02-12  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (REMOTE_OBS): Append tracefile.o and
	tracefile-tfile.o.
	(HFILES_NO_SRCDIR): Add tracefile.h.
	* ctf.c: Include "tracefile.h".
	* tracefile.h: New file.
	* tracefile.c: New file
	* tracefile-tfile.c: New file.
	* tracepoint.c: Include "tracefile.h".
	(free_uploaded_tps, free_uploaded_tsvs): Remove declarations.
	(stop_reason_names): Add const.
	(trace_file_writer_xfree): Move it to tracefile.c.
	(trace_save, trace_save_command, trace_save_tfile): Likewise.
	(trace_save_ctf): Likewise.
	(struct tfile_trace_file_writer): Move it to tracefile-tfile.c.
	(tfile_target_save, tfile_dtor, tfile_start): Likewise.
	(tfile_write_header, tfile_write_regblock_type): Likewise.
	(tfile_write_status, tfile_write_uploaded_tsv): Likewise.
	(tfile_write_uploaded_tp, tfile_write_definition_end): Likewise.
	(tfile_write_raw_data, tfile_end): Likewise.
	(tfile_trace_file_writer_new): Likewise.
	(free_uploaded_tp): Make it extern.
	(free_uploaded_tsv): Make it extern.
	(_initialize_tracepoint): Move code to register command 'tsave'
	to tracefile.c.
	* tracepoint.h (stop_reason_names): Declare.
	(struct trace_frame_write_ops): Move it to tracefile.h.
	(struct trace_file_write_ops): Likewise.
	(struct trace_file_writer): Likewise.
	(free_uploaded_tsvs, free_uploaded_tps): Declare.
---
 gdb/Makefile.in       |    4 +-
 gdb/ctf.c             |    1 +
 gdb/tracefile-tfile.c |  327 ++++++++++++++++++++++++
 gdb/tracefile.c       |  389 ++++++++++++++++++++++++++++
 gdb/tracefile.h       |  114 +++++++++
 gdb/tracepoint.c      |  674 +------------------------------------------------
 gdb/tracepoint.h      |  110 +--------
 7 files changed, 842 insertions(+), 777 deletions(-)
 create mode 100644 gdb/tracefile-tfile.c
 create mode 100644 gdb/tracefile.c
 create mode 100644 gdb/tracefile.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 6c8db6f..61d0bab 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -584,7 +584,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o ctf.o
+	remote-notif.o ctf.o tracefile.o tracefile-tfile.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -894,7 +894,7 @@ doublest.h regset.h hppa-tdep.h ppc-linux-tdep.h ppc64-tdep.h \
 rs6000-tdep.h rs6000-aix-tdep.h \
 common/gdb_locale.h arch-utils.h trad-frame.h gnu-nat.h \
 language.h nbsd-tdep.h solib-svr4.h \
-macroexp.h ui-file.h regcache.h tracepoint.h i386-tdep.h \
+macroexp.h ui-file.h regcache.h tracepoint.h tracefile.h i386-tdep.h \
 inf-child.h p-lang.h event-top.h gdbtypes.h user-regs.h \
 regformats/regdef.h config/alpha/nm-osf3.h  config/i386/nm-i386gnu.h \
 config/i386/nm-fbsd.h \
diff --git a/gdb/ctf.c b/gdb/ctf.c
index 71453df..8ad363e 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -28,6 +28,7 @@
 #include "completer.h"
 #include "inferior.h"
 #include "gdbthread.h"
+#include "tracefile.h"
 
 #include <ctype.h>
 
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
new file mode 100644
index 0000000..97e93fc
--- /dev/null
+++ b/gdb/tracefile-tfile.c
@@ -0,0 +1,327 @@
+/* Trace file TFILE format support in GDB.
+
+   Copyright (C) 1997-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "tracefile.h"
+#include "readline/tilde.h"
+#include "filestuff.h"
+#include "remote.h" /* bin2hex */
+
+/* TFILE trace writer.  */
+
+struct tfile_trace_file_writer
+{
+  struct trace_file_writer base;
+
+  /* File pointer to tfile trace file.  */
+  FILE *fp;
+  /* Path name of the tfile trace file.  */
+  char *pathname;
+};
+
+/* This is the implementation of trace_file_write_ops method
+   target_save.  We just call the generic target
+   target_save_trace_data to do target-side saving.  */
+
+static int
+tfile_target_save (struct trace_file_writer *self,
+		   const char *filename)
+{
+  int err = target_save_trace_data (filename);
+
+  return (err >= 0);
+}
+
+/* This is the implementation of trace_file_write_ops method
+   dtor.  */
+
+static void
+tfile_dtor (struct trace_file_writer *self)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  xfree (writer->pathname);
+
+  if (writer->fp != NULL)
+    fclose (writer->fp);
+}
+
+/* This is the implementation of trace_file_write_ops method
+   start.  It creates the trace file FILENAME and registers some
+   cleanups.  */
+
+static void
+tfile_start (struct trace_file_writer *self, const char *filename)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  writer->pathname = tilde_expand (filename);
+  writer->fp = gdb_fopen_cloexec (writer->pathname, "wb");
+  if (writer->fp == NULL)
+    error (_("Unable to open file '%s' for saving trace data (%s)"),
+	   writer->pathname, safe_strerror (errno));
+}
+
+/* This is the implementation of trace_file_write_ops method
+   write_header.  Write the TFILE header.  */
+
+static void
+tfile_write_header (struct trace_file_writer *self)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+  int written;
+
+  /* Write a file header, with a high-bit-set char to indicate a
+     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, writer->fp);
+  if (written < 1)
+    perror_with_name (writer->pathname);
+}
+
+/* This is the implementation of trace_file_write_ops method
+   write_regblock_type.  Write the size of register block.  */
+
+static void
+tfile_write_regblock_type (struct trace_file_writer *self, int size)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  fprintf (writer->fp, "R %x\n", size);
+}
+
+/* This is the implementation of trace_file_write_ops method
+   write_status.  */
+
+static void
+tfile_write_status (struct trace_file_writer *self,
+		    struct trace_status *ts)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  fprintf (writer->fp, "status %c;%s",
+	   (ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
+  if (ts->stop_reason == tracepoint_error
+      || ts->stop_reason == tstop_command)
+    {
+      char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);
+
+      bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
+      fprintf (writer->fp, ":%s", buf);
+    }
+  fprintf (writer->fp, ":%x", ts->stopping_tracepoint);
+  if (ts->traceframe_count >= 0)
+    fprintf (writer->fp, ";tframes:%x", ts->traceframe_count);
+  if (ts->traceframes_created >= 0)
+    fprintf (writer->fp, ";tcreated:%x", ts->traceframes_created);
+  if (ts->buffer_free >= 0)
+    fprintf (writer->fp, ";tfree:%x", ts->buffer_free);
+  if (ts->buffer_size >= 0)
+    fprintf (writer->fp, ";tsize:%x", ts->buffer_size);
+  if (ts->disconnected_tracing)
+    fprintf (writer->fp, ";disconn:%x", ts->disconnected_tracing);
+  if (ts->circular_buffer)
+    fprintf (writer->fp, ";circular:%x", ts->circular_buffer);
+  if (ts->start_time)
+    {
+      fprintf (writer->fp, ";starttime:%s",
+      phex_nz (ts->start_time, sizeof (ts->start_time)));
+    }
+  if (ts->stop_time)
+    {
+      fprintf (writer->fp, ";stoptime:%s",
+      phex_nz (ts->stop_time, sizeof (ts->stop_time)));
+    }
+  if (ts->notes != NULL)
+    {
+      char *buf = (char *) alloca (strlen (ts->notes) * 2 + 1);
+
+      bin2hex ((gdb_byte *) ts->notes, buf, 0);
+      fprintf (writer->fp, ";notes:%s", buf);
+    }
+  if (ts->user_name != NULL)
+    {
+      char *buf = (char *) alloca (strlen (ts->user_name) * 2 + 1);
+
+      bin2hex ((gdb_byte *) ts->user_name, buf, 0);
+      fprintf (writer->fp, ";username:%s", buf);
+    }
+  fprintf (writer->fp, "\n");
+}
+
+/* This is the implementation of trace_file_write_ops method
+   write_uploaded_tsv.  */
+
+static void
+tfile_write_uploaded_tsv (struct trace_file_writer *self,
+			  struct uploaded_tsv *utsv)
+{
+  char *buf = "";
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  if (utsv->name)
+    {
+      buf = (char *) xmalloc (strlen (utsv->name) * 2 + 1);
+      bin2hex ((gdb_byte *) (utsv->name), buf, 0);
+    }
+
+  fprintf (writer->fp, "tsv %x:%s:%x:%s\n",
+	   utsv->number, phex_nz (utsv->initial_value, 8),
+	   utsv->builtin, buf);
+
+  if (utsv->name)
+    xfree (buf);
+}
+
+#define MAX_TRACE_UPLOAD 2000
+
+/* This is the implementation of trace_file_write_ops method
+   write_uploaded_tp.  */
+
+static void
+tfile_write_uploaded_tp (struct trace_file_writer *self,
+			 struct uploaded_tp *utp)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+  int a;
+  char *act;
+  char buf[MAX_TRACE_UPLOAD];
+
+  fprintf (writer->fp, "tp T%x:%s:%c:%x:%x",
+	   utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
+	   (utp->enabled ? 'E' : 'D'), utp->step, utp->pass);
+  if (utp->type == bp_fast_tracepoint)
+    fprintf (writer->fp, ":F%x", utp->orig_size);
+  if (utp->cond)
+    fprintf (writer->fp,
+	     ":X%x,%s", (unsigned int) strlen (utp->cond) / 2,
+	     utp->cond);
+  fprintf (writer->fp, "\n");
+  for (a = 0; VEC_iterate (char_ptr, utp->actions, a, act); ++a)
+    fprintf (writer->fp, "tp A%x:%s:%s\n",
+	     utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
+  for (a = 0; VEC_iterate (char_ptr, utp->step_actions, a, act); ++a)
+    fprintf (writer->fp, "tp S%x:%s:%s\n",
+	     utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
+  if (utp->at_string)
+    {
+      encode_source_string (utp->number, utp->addr,
+			    "at", utp->at_string, buf, MAX_TRACE_UPLOAD);
+      fprintf (writer->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 (writer->fp, "tp Z%s\n", buf);
+    }
+  for (a = 0; VEC_iterate (char_ptr, utp->cmd_strings, a, act); ++a)
+    {
+      encode_source_string (utp->number, utp->addr, "cmd", act,
+			    buf, MAX_TRACE_UPLOAD);
+      fprintf (writer->fp, "tp Z%s\n", buf);
+    }
+  fprintf (writer->fp, "tp V%x:%s:%x:%s\n",
+	   utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
+	   utp->hit_count,
+	   phex_nz (utp->traceframe_usage,
+		    sizeof (utp->traceframe_usage)));
+}
+
+/* This is the implementation of trace_file_write_ops method
+   write_definition_end.  */
+
+static void
+tfile_write_definition_end (struct trace_file_writer *self)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  fprintf (writer->fp, "\n");
+}
+
+/* This is the implementation of trace_file_write_ops method
+   write_raw_data.  */
+
+static void
+tfile_write_raw_data (struct trace_file_writer *self, gdb_byte *buf,
+		      LONGEST len)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+
+  if (fwrite (buf, len, 1, writer->fp) < 1)
+    perror_with_name (writer->pathname);
+}
+
+/* This is the implementation of trace_file_write_ops method
+   end.  */
+
+static void
+tfile_end (struct trace_file_writer *self)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+  uint32_t gotten = 0;
+
+  /* Mark the end of trace data.  */
+  if (fwrite (&gotten, 4, 1, writer->fp) < 1)
+    perror_with_name (writer->pathname);
+}
+
+/* Operations to write trace buffers into TFILE format.  */
+
+static const struct trace_file_write_ops tfile_write_ops =
+{
+  tfile_dtor,
+  tfile_target_save,
+  tfile_start,
+  tfile_write_header,
+  tfile_write_regblock_type,
+  tfile_write_status,
+  tfile_write_uploaded_tsv,
+  tfile_write_uploaded_tp,
+  tfile_write_definition_end,
+  tfile_write_raw_data,
+  NULL,
+  tfile_end,
+};
+
+/* Return a trace writer for TFILE format.  */
+
+struct trace_file_writer *
+tfile_trace_file_writer_new (void)
+{
+  struct tfile_trace_file_writer *writer
+    = xmalloc (sizeof (struct tfile_trace_file_writer));
+
+  writer->base.ops = &tfile_write_ops;
+  writer->fp = NULL;
+  writer->pathname = NULL;
+
+  return (struct trace_file_writer *) writer;
+}
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
new file mode 100644
index 0000000..9945ae5
--- /dev/null
+++ b/gdb/tracefile.c
@@ -0,0 +1,389 @@
+/* Trace file support in GDB.
+
+   Copyright (C) 1997-2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "tracefile.h"
+#include "ctf.h"
+
+/* Helper macros.  */
+
+#define TRACE_WRITE_R_BLOCK(writer, buf, size)	\
+  writer->ops->frame_ops->write_r_block ((writer), (buf), (size))
+#define TRACE_WRITE_M_BLOCK_HEADER(writer, addr, size)		  \
+  writer->ops->frame_ops->write_m_block_header ((writer), (addr), \
+						(size))
+#define TRACE_WRITE_M_BLOCK_MEMORY(writer, buf, size)	  \
+  writer->ops->frame_ops->write_m_block_memory ((writer), (buf), \
+						(size))
+#define TRACE_WRITE_V_BLOCK(writer, num, val)	\
+  writer->ops->frame_ops->write_v_block ((writer), (num), (val))
+
+/* Free trace file writer.  */
+
+static void
+trace_file_writer_xfree (void *arg)
+{
+  struct trace_file_writer *writer = arg;
+
+  writer->ops->dtor (writer);
+  xfree (writer);
+}
+
+/* Save tracepoint data to file named FILENAME through WRITER.  WRITER
+   determines the trace file format.  If TARGET_DOES_SAVE is non-zero,
+   the save is performed on the target, otherwise GDB obtains all trace
+   data and saves it locally.  */
+
+static void
+trace_save (const char *filename, struct trace_file_writer *writer,
+	    int target_does_save)
+{
+  struct trace_status *ts = current_trace_status ();
+  int status;
+  struct uploaded_tp *uploaded_tps = NULL, *utp;
+  struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
+
+  ULONGEST offset = 0;
+#define MAX_TRACE_UPLOAD 2000
+  gdb_byte buf[MAX_TRACE_UPLOAD];
+  int written;
+  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+
+  /* If the target is to save the data to a file on its own, then just
+     send the command and be done with it.  */
+  if (target_does_save)
+    {
+      if (!writer->ops->target_save (writer, filename))
+	error (_("Target failed to save trace data to '%s'."),
+	       filename);
+      return;
+    }
+
+  /* Get the trace status first before opening the file, so if the
+     target is losing, we can get out without touching files.  */
+  status = target_get_trace_status (ts);
+
+  writer->ops->start (writer, filename);
+
+  writer->ops->write_header (writer);
+
+  /* Write descriptive info.  */
+
+  /* Write out the size of a register block.  */
+  writer->ops->write_regblock_type (writer, trace_regblock_size);
+
+  /* Write out status of the tracing run (aka "tstatus" info).  */
+  writer->ops->write_status (writer, ts);
+
+  /* Note that we want to upload tracepoints and save those, rather
+     than simply writing out the local ones, because the user may have
+     changed tracepoints in GDB in preparation for a future tracing
+     run, or maybe just mass-deleted all types of breakpoints as part
+     of cleaning up.  So as not to contaminate the session, leave the
+     data in its uploaded form, don't make into real tracepoints.  */
+
+  /* Get trace state variables first, they may be checked when parsing
+     uploaded commands.  */
+
+  target_upload_trace_state_variables (&uploaded_tsvs);
+
+  for (utsv = uploaded_tsvs; utsv; utsv = utsv->next)
+    writer->ops->write_uploaded_tsv (writer, utsv);
+
+  free_uploaded_tsvs (&uploaded_tsvs);
+
+  target_upload_tracepoints (&uploaded_tps);
+
+  for (utp = uploaded_tps; utp; utp = utp->next)
+    target_get_tracepoint_status (NULL, utp);
+
+  for (utp = uploaded_tps; utp; utp = utp->next)
+    writer->ops->write_uploaded_tp (writer, utp);
+
+  free_uploaded_tps (&uploaded_tps);
+
+  /* Mark the end of the definition section.  */
+  writer->ops->write_definition_end (writer);
+
+  /* Get and write the trace data proper.  */
+  while (1)
+    {
+      LONGEST gotten = 0;
+
+      /* The writer supports writing the contents of trace buffer
+	  directly to trace file.  Don't parse the contents of trace
+	  buffer.  */
+      if (writer->ops->write_trace_buffer != NULL)
+	{
+	  /* We ask for big blocks, in the hopes of efficiency, but
+	     will take less if the target has packet size limitations
+	     or some such.  */
+	  gotten = target_get_raw_trace_data (buf, offset,
+					      MAX_TRACE_UPLOAD);
+	  if (gotten < 0)
+	    error (_("Failure to get requested trace buffer data"));
+	  /* No more data is forthcoming, we're done.  */
+	  if (gotten == 0)
+	    break;
+
+	  writer->ops->write_trace_buffer (writer, buf, gotten);
+
+	  offset += gotten;
+	}
+      else
+	{
+	  uint16_t tp_num;
+	  uint32_t tf_size;
+	  /* Parse the trace buffers according to how data are stored
+	     in trace buffer in GDBserver.  */
+
+	  gotten = target_get_raw_trace_data (buf, offset, 6);
+
+	  if (gotten == 0)
+	    break;
+
+	  /* Read the first six bytes in, which is the tracepoint
+	     number and trace frame size.  */
+	  tp_num = (uint16_t)
+	    extract_unsigned_integer (&buf[0], 2, byte_order);
+
+	  tf_size = (uint32_t)
+	    extract_unsigned_integer (&buf[2], 4, byte_order);
+
+	  writer->ops->frame_ops->start (writer, tp_num);
+	  gotten = 6;
+
+	  if (tf_size > 0)
+	    {
+	      unsigned int block;
+
+	      offset += 6;
+
+	      for (block = 0; block < tf_size; )
+		{
+		  gdb_byte block_type;
+
+		  /* We'll fetch one block each time, in order to
+		     handle the extremely large 'M' block.  We first
+		     fetch one byte to get the type of the block.  */
+		  gotten = target_get_raw_trace_data (buf, offset, 1);
+		  if (gotten < 1)
+		    error (_("Failure to get requested trace buffer data"));
+
+		  gotten = 1;
+		  block += 1;
+		  offset += 1;
+
+		  block_type = buf[0];
+		  switch (block_type)
+		    {
+		    case 'R':
+		      gotten
+			= target_get_raw_trace_data (buf, offset,
+						     trace_regblock_size);
+		      if (gotten < trace_regblock_size)
+			error (_("Failure to get requested trace"
+				 " buffer data"));
+
+		      TRACE_WRITE_R_BLOCK (writer, buf,
+					   trace_regblock_size);
+		      break;
+		    case 'M':
+		      {
+			unsigned short mlen;
+			ULONGEST addr;
+			LONGEST t;
+			int j;
+
+			t = target_get_raw_trace_data (buf,offset, 10);
+			if (t < 10)
+			  error (_("Failure to get requested trace"
+				   " buffer data"));
+
+			offset += 10;
+			block += 10;
+
+			gotten = 0;
+			addr = (ULONGEST)
+			  extract_unsigned_integer (buf, 8,
+						    byte_order);
+			mlen = (unsigned short)
+			  extract_unsigned_integer (&buf[8], 2,
+						    byte_order);
+
+			TRACE_WRITE_M_BLOCK_HEADER (writer, addr,
+						    mlen);
+
+			/* The memory contents in 'M' block may be
+			   very large.  Fetch the data from the target
+			   and write them into file one by one.  */
+			for (j = 0; j < mlen; )
+			  {
+			    unsigned int read_length;
+
+			    if (mlen - j > MAX_TRACE_UPLOAD)
+			      read_length = MAX_TRACE_UPLOAD;
+			    else
+			      read_length = mlen - j;
+
+			    t = target_get_raw_trace_data (buf,
+							   offset + j,
+							   read_length);
+			    if (t < read_length)
+			      error (_("Failure to get requested"
+				       " trace buffer data"));
+
+			    TRACE_WRITE_M_BLOCK_MEMORY (writer, buf,
+							read_length);
+
+			    j += read_length;
+			    gotten += read_length;
+			  }
+
+			break;
+		      }
+		    case 'V':
+		      {
+			int vnum;
+			LONGEST val;
+
+			gotten
+			  = target_get_raw_trace_data (buf, offset,
+						       12);
+			if (gotten < 12)
+			  error (_("Failure to get requested"
+				   " trace buffer data"));
+
+			vnum  = (int) extract_signed_integer (buf,
+							      4,
+							      byte_order);
+			val
+			  = extract_signed_integer (&buf[4], 8,
+						    byte_order);
+
+			TRACE_WRITE_V_BLOCK (writer, vnum, val);
+		      }
+		      break;
+		    default:
+		      error (_("Unknown block type '%c' (0x%x) in"
+			       " trace frame"),
+			     block_type, block_type);
+		    }
+
+		  block += gotten;
+		  offset += gotten;
+		}
+	    }
+	  else
+	    offset += gotten;
+
+	  writer->ops->frame_ops->end (writer);
+	}
+    }
+
+  writer->ops->end (writer);
+}
+
+static void
+trace_save_command (char *args, int from_tty)
+{
+  int target_does_save = 0;
+  char **argv;
+  char *filename = NULL;
+  struct cleanup *back_to;
+  int generate_ctf = 0;
+  struct trace_file_writer *writer = NULL;
+
+  if (args == NULL)
+    error_no_arg (_("file in which to save trace data"));
+
+  argv = gdb_buildargv (args);
+  back_to = make_cleanup_freeargv (argv);
+
+  for (; *argv; ++argv)
+    {
+      if (strcmp (*argv, "-r") == 0)
+	target_does_save = 1;
+      if (strcmp (*argv, "-ctf") == 0)
+	generate_ctf = 1;
+      else if (**argv == '-')
+	error (_("unknown option `%s'"), *argv);
+      else
+	filename = *argv;
+    }
+
+  if (!filename)
+    error_no_arg (_("file in which to save trace data"));
+
+  if (generate_ctf)
+    writer = ctf_trace_file_writer_new ();
+  else
+    writer = tfile_trace_file_writer_new ();
+
+  make_cleanup (trace_file_writer_xfree, writer);
+
+  trace_save (filename, writer, target_does_save);
+
+  if (from_tty)
+    printf_filtered (_("Trace data saved to %s '%s'.\n"),
+		     generate_ctf ? "directory" : "file", filename);
+
+  do_cleanups (back_to);
+}
+
+/* Save the trace data to file FILENAME of tfile format.  */
+
+void
+trace_save_tfile (const char *filename, int target_does_save)
+{
+  struct trace_file_writer *writer;
+  struct cleanup *back_to;
+
+  writer = tfile_trace_file_writer_new ();
+  back_to = make_cleanup (trace_file_writer_xfree, writer);
+  trace_save (filename, writer, target_does_save);
+  do_cleanups (back_to);
+}
+
+/* Save the trace data to dir DIRNAME of ctf format.  */
+
+void
+trace_save_ctf (const char *dirname, int target_does_save)
+{
+  struct trace_file_writer *writer;
+  struct cleanup *back_to;
+
+  writer = ctf_trace_file_writer_new ();
+  back_to = make_cleanup (trace_file_writer_xfree, writer);
+
+  trace_save (dirname, writer, target_does_save);
+  do_cleanups (back_to);
+}
+
+extern initialize_file_ftype _initialize_tracefile;
+
+void
+_initialize_tracefile (void)
+{
+  add_com ("tsave", class_trace, trace_save_command, _("\
+Save the trace data to a file.\n\
+Use the '-ctf' option to save the data to CTF format.\n\
+Use the '-r' option to direct the target to save directly to the file,\n\
+using its own filesystem."));
+}
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
new file mode 100644
index 0000000..833de5c
--- /dev/null
+++ b/gdb/tracefile.h
@@ -0,0 +1,114 @@
+#ifndef TRACEFILE_H
+#define TRACEFILE_H 1
+
+#include "defs.h"
+#include "tracepoint.h"
+
+struct trace_file_writer;
+
+/* Operations to write trace frames to a specific trace format.  */
+
+struct trace_frame_write_ops
+{
+  /* Write a new trace frame.  The tracepoint number of this trace
+     frame is TPNUM.  */
+  void (*start) (struct trace_file_writer *self, uint16_t tpnum);
+
+  /* Write an 'R' block.  Buffer BUF contains its contents and SIZE is
+     its size.  */
+  void (*write_r_block) (struct trace_file_writer *self,
+			 gdb_byte *buf, int32_t size);
+
+  /* Write an 'M' block, the header and memory contents respectively.
+     The header of 'M' block is composed of the start address and the
+     length of memory collection, and the memory contents contain
+     the collected memory contents in tracing.
+     For extremely large M block, GDB is unable to get its contents
+     and write them into trace file in one go, due to the limitation
+     of the remote target or the size of internal buffer, we split
+     the operation to 'M' block to two operations.  */
+  /* Write the head of 'M' block.  ADDR is the start address of
+     collected memory and LENGTH is the length of memory contents.  */
+  void (*write_m_block_header) (struct trace_file_writer *self,
+				uint64_t addr, uint16_t length);
+  /* Write the memory contents of 'M' block.  Buffer BUF contains
+     its contents and LENGTH is its length.  This method can be called
+     multiple times to write large memory contents of a single 'M'
+     block.  */
+  void (*write_m_block_memory) (struct trace_file_writer *self,
+				gdb_byte *buf, uint16_t length);
+
+  /* Write a 'V' block.  NUM is the trace variable number and VAL is
+     the value of the trace variable.  */
+  void (*write_v_block) (struct trace_file_writer *self, int32_t num,
+			 uint64_t val);
+
+  /* The end of the trace frame.  */
+  void (*end) (struct trace_file_writer *self);
+};
+
+/* Operations to write trace buffers to a specific trace format.  */
+
+struct trace_file_write_ops
+{
+  /* Destructor.  Releases everything from SELF (but not SELF
+     itself).  */
+  void (*dtor) (struct trace_file_writer *self);
+
+  /*  Save the data to file or directory NAME of desired format in
+      target side.  Return true for success, otherwise return
+      false.  */
+  int (*target_save) (struct trace_file_writer *self,
+		      const char *name);
+
+  /* Write the trace buffers to file or directory NAME.  */
+  void (*start) (struct trace_file_writer *self,
+		 const char *name);
+
+  /* Write the trace header.  */
+  void (*write_header) (struct trace_file_writer *self);
+
+  /* Write the type of block about registers.  SIZE is the size of
+     all registers on the target.  */
+  void (*write_regblock_type) (struct trace_file_writer *self,
+			       int size);
+
+  /* Write trace status TS.  */
+  void (*write_status) (struct trace_file_writer *self,
+			struct trace_status *ts);
+
+  /* Write the uploaded TSV.  */
+  void (*write_uploaded_tsv) (struct trace_file_writer *self,
+			      struct uploaded_tsv *tsv);
+
+  /* Write the uploaded tracepoint TP.  */
+  void (*write_uploaded_tp) (struct trace_file_writer *self,
+			     struct uploaded_tp *tp);
+
+  /* Write to mark the end of the definition part.  */
+  void (*write_definition_end) (struct trace_file_writer *self);
+
+  /* Write the data of trace buffer without parsing.  The content is
+     in BUF and length is LEN.  */
+  void (*write_trace_buffer) (struct trace_file_writer *self,
+			      gdb_byte *buf, LONGEST len);
+
+  /* Operations to write trace frames.  The user of this field is
+     responsible to parse the data of trace buffer.  Either field
+     'write_trace_buffer' or field ' frame_ops' is NULL.  */
+  const struct trace_frame_write_ops *frame_ops;
+
+  /* The end of writing trace buffers.  */
+  void (*end) (struct trace_file_writer *self);
+};
+
+/* Trace file writer for a given format.  */
+
+struct trace_file_writer
+{
+  const struct trace_file_write_ops *ops;
+};
+
+extern struct trace_file_writer *tfile_trace_file_writer_new (void);
+
+#endif /* TRACEFILE_H */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 2cb7442..45a7fd4 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -55,6 +55,7 @@
 #include "probe.h"
 #include "ctf.h"
 #include "filestuff.h"
+#include "tracefile.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -190,9 +191,6 @@ static char *mem2hex (gdb_byte *, char *, int);
 static void add_register (struct collection_list *collection,
 			  unsigned int regno);
 
-static void free_uploaded_tps (struct uploaded_tp **utpp);
-static void free_uploaded_tsvs (struct uploaded_tsv **utsvp);
-
 static struct command_line *
   all_tracepoint_actions_and_cleanup (struct breakpoint *t);
 
@@ -200,7 +198,7 @@ extern void _initialize_tracepoint (void);
 
 static struct trace_status trace_status;
 
-char *stop_reason_names[] = {
+const char *stop_reason_names[] = {
   "tunknown",
   "tnotrun",
   "tstop",
@@ -3095,664 +3093,6 @@ encode_source_string (int tpnum, ULONGEST addr,
   return -1;
 }
 
-/* Free trace file writer.  */
-
-static void
-trace_file_writer_xfree (void *arg)
-{
-  struct trace_file_writer *writer = arg;
-
-  writer->ops->dtor (writer);
-  xfree (writer);
-}
-
-/* TFILE trace writer.  */
-
-struct tfile_trace_file_writer
-{
-  struct trace_file_writer base;
-
-  /* File pointer to tfile trace file.  */
-  FILE *fp;
-  /* Path name of the tfile trace file.  */
-  char *pathname;
-};
-
-/* This is the implementation of trace_file_write_ops method
-   target_save.  We just call the generic target
-   target_save_trace_data to do target-side saving.  */
-
-static int
-tfile_target_save (struct trace_file_writer *self,
-		   const char *filename)
-{
-  int err = target_save_trace_data (filename);
-
-  return (err >= 0);
-}
-
-/* This is the implementation of trace_file_write_ops method
-   dtor.  */
-
-static void
-tfile_dtor (struct trace_file_writer *self)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  xfree (writer->pathname);
-
-  if (writer->fp != NULL)
-    fclose (writer->fp);
-}
-
-/* This is the implementation of trace_file_write_ops method
-   start.  It creates the trace file FILENAME and registers some
-   cleanups.  */
-
-static void
-tfile_start (struct trace_file_writer *self, const char *filename)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  writer->pathname = tilde_expand (filename);
-  writer->fp = gdb_fopen_cloexec (writer->pathname, "wb");
-  if (writer->fp == NULL)
-    error (_("Unable to open file '%s' for saving trace data (%s)"),
-	   writer->pathname, safe_strerror (errno));
-}
-
-/* This is the implementation of trace_file_write_ops method
-   write_header.  Write the TFILE header.  */
-
-static void
-tfile_write_header (struct trace_file_writer *self)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-  int written;
-
-  /* Write a file header, with a high-bit-set char to indicate a
-     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, writer->fp);
-  if (written < 1)
-    perror_with_name (writer->pathname);
-}
-
-/* This is the implementation of trace_file_write_ops method
-   write_regblock_type.  Write the size of register block.  */
-
-static void
-tfile_write_regblock_type (struct trace_file_writer *self, int size)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  fprintf (writer->fp, "R %x\n", size);
-}
-
-/* This is the implementation of trace_file_write_ops method
-   write_status.  */
-
-static void
-tfile_write_status (struct trace_file_writer *self,
-		    struct trace_status *ts)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  fprintf (writer->fp, "status %c;%s",
-	   (ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
-  if (ts->stop_reason == tracepoint_error
-      || ts->stop_reason == tstop_command)
-    {
-      char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);
-
-      bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
-      fprintf (writer->fp, ":%s", buf);
-    }
-  fprintf (writer->fp, ":%x", ts->stopping_tracepoint);
-  if (ts->traceframe_count >= 0)
-    fprintf (writer->fp, ";tframes:%x", ts->traceframe_count);
-  if (ts->traceframes_created >= 0)
-    fprintf (writer->fp, ";tcreated:%x", ts->traceframes_created);
-  if (ts->buffer_free >= 0)
-    fprintf (writer->fp, ";tfree:%x", ts->buffer_free);
-  if (ts->buffer_size >= 0)
-    fprintf (writer->fp, ";tsize:%x", ts->buffer_size);
-  if (ts->disconnected_tracing)
-    fprintf (writer->fp, ";disconn:%x", ts->disconnected_tracing);
-  if (ts->circular_buffer)
-    fprintf (writer->fp, ";circular:%x", ts->circular_buffer);
-  if (ts->start_time)
-    {
-      fprintf (writer->fp, ";starttime:%s",
-      phex_nz (ts->start_time, sizeof (ts->start_time)));
-    }
-  if (ts->stop_time)
-    {
-      fprintf (writer->fp, ";stoptime:%s",
-      phex_nz (ts->stop_time, sizeof (ts->stop_time)));
-    }
-  if (ts->notes != NULL)
-    {
-      char *buf = (char *) alloca (strlen (ts->notes) * 2 + 1);
-
-      bin2hex ((gdb_byte *) ts->notes, buf, 0);
-      fprintf (writer->fp, ";notes:%s", buf);
-    }
-  if (ts->user_name != NULL)
-    {
-      char *buf = (char *) alloca (strlen (ts->user_name) * 2 + 1);
-
-      bin2hex ((gdb_byte *) ts->user_name, buf, 0);
-      fprintf (writer->fp, ";username:%s", buf);
-    }
-  fprintf (writer->fp, "\n");
-}
-
-/* This is the implementation of trace_file_write_ops method
-   write_uploaded_tsv.  */
-
-static void
-tfile_write_uploaded_tsv (struct trace_file_writer *self,
-			  struct uploaded_tsv *utsv)
-{
-  char *buf = "";
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  if (utsv->name)
-    {
-      buf = (char *) xmalloc (strlen (utsv->name) * 2 + 1);
-      bin2hex ((gdb_byte *) (utsv->name), buf, 0);
-    }
-
-  fprintf (writer->fp, "tsv %x:%s:%x:%s\n",
-	   utsv->number, phex_nz (utsv->initial_value, 8),
-	   utsv->builtin, buf);
-
-  if (utsv->name)
-    xfree (buf);
-}
-
-#define MAX_TRACE_UPLOAD 2000
-
-/* This is the implementation of trace_file_write_ops method
-   write_uploaded_tp.  */
-
-static void
-tfile_write_uploaded_tp (struct trace_file_writer *self,
-			 struct uploaded_tp *utp)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-  int a;
-  char *act;
-  char buf[MAX_TRACE_UPLOAD];
-
-  fprintf (writer->fp, "tp T%x:%s:%c:%x:%x",
-	   utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
-	   (utp->enabled ? 'E' : 'D'), utp->step, utp->pass);
-  if (utp->type == bp_fast_tracepoint)
-    fprintf (writer->fp, ":F%x", utp->orig_size);
-  if (utp->cond)
-    fprintf (writer->fp,
-	     ":X%x,%s", (unsigned int) strlen (utp->cond) / 2,
-	     utp->cond);
-  fprintf (writer->fp, "\n");
-  for (a = 0; VEC_iterate (char_ptr, utp->actions, a, act); ++a)
-    fprintf (writer->fp, "tp A%x:%s:%s\n",
-	     utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
-  for (a = 0; VEC_iterate (char_ptr, utp->step_actions, a, act); ++a)
-    fprintf (writer->fp, "tp S%x:%s:%s\n",
-	     utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
-  if (utp->at_string)
-    {
-      encode_source_string (utp->number, utp->addr,
-			    "at", utp->at_string, buf, MAX_TRACE_UPLOAD);
-      fprintf (writer->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 (writer->fp, "tp Z%s\n", buf);
-    }
-  for (a = 0; VEC_iterate (char_ptr, utp->cmd_strings, a, act); ++a)
-    {
-      encode_source_string (utp->number, utp->addr, "cmd", act,
-			    buf, MAX_TRACE_UPLOAD);
-      fprintf (writer->fp, "tp Z%s\n", buf);
-    }
-  fprintf (writer->fp, "tp V%x:%s:%x:%s\n",
-	   utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
-	   utp->hit_count,
-	   phex_nz (utp->traceframe_usage,
-		    sizeof (utp->traceframe_usage)));
-}
-
-/* This is the implementation of trace_file_write_ops method
-   write_definition_end.  */
-
-static void
-tfile_write_definition_end (struct trace_file_writer *self)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  fprintf (writer->fp, "\n");
-}
-
-/* This is the implementation of trace_file_write_ops method
-   write_raw_data.  */
-
-static void
-tfile_write_raw_data (struct trace_file_writer *self, gdb_byte *buf,
-		      LONGEST len)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-
-  if (fwrite (buf, len, 1, writer->fp) < 1)
-    perror_with_name (writer->pathname);
-}
-
-/* This is the implementation of trace_file_write_ops method
-   end.  */
-
-static void
-tfile_end (struct trace_file_writer *self)
-{
-  struct tfile_trace_file_writer *writer
-    = (struct tfile_trace_file_writer *) self;
-  uint32_t gotten = 0;
-
-  /* Mark the end of trace data.  */
-  if (fwrite (&gotten, 4, 1, writer->fp) < 1)
-    perror_with_name (writer->pathname);
-}
-
-/* Operations to write trace buffers into TFILE format.  */
-
-static const struct trace_file_write_ops tfile_write_ops =
-{
-  tfile_dtor,
-  tfile_target_save,
-  tfile_start,
-  tfile_write_header,
-  tfile_write_regblock_type,
-  tfile_write_status,
-  tfile_write_uploaded_tsv,
-  tfile_write_uploaded_tp,
-  tfile_write_definition_end,
-  tfile_write_raw_data,
-  NULL,
-  tfile_end,
-};
-
-/* Helper macros.  */
-
-#define TRACE_WRITE_R_BLOCK(writer, buf, size)	\
-  writer->ops->frame_ops->write_r_block ((writer), (buf), (size))
-#define TRACE_WRITE_M_BLOCK_HEADER(writer, addr, size)		  \
-  writer->ops->frame_ops->write_m_block_header ((writer), (addr), \
-						(size))
-#define TRACE_WRITE_M_BLOCK_MEMORY(writer, buf, size)	  \
-  writer->ops->frame_ops->write_m_block_memory ((writer), (buf), \
-						(size))
-#define TRACE_WRITE_V_BLOCK(writer, num, val)	\
-  writer->ops->frame_ops->write_v_block ((writer), (num), (val))
-
-/* Save tracepoint data to file named FILENAME through WRITER.  WRITER
-   determines the trace file format.  If TARGET_DOES_SAVE is non-zero,
-   the save is performed on the target, otherwise GDB obtains all trace
-   data and saves it locally.  */
-
-static void
-trace_save (const char *filename, struct trace_file_writer *writer,
-	    int target_does_save)
-{
-  struct trace_status *ts = current_trace_status ();
-  int status;
-  struct uploaded_tp *uploaded_tps = NULL, *utp;
-  struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
-
-  ULONGEST offset = 0;
-  gdb_byte buf[MAX_TRACE_UPLOAD];
-#define MAX_TRACE_UPLOAD 2000
-  int written;
-  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-
-  /* If the target is to save the data to a file on its own, then just
-     send the command and be done with it.  */
-  if (target_does_save)
-    {
-      if (!writer->ops->target_save (writer, filename))
-	error (_("Target failed to save trace data to '%s'."),
-	       filename);
-      return;
-    }
-
-  /* Get the trace status first before opening the file, so if the
-     target is losing, we can get out without touching files.  */
-  status = target_get_trace_status (ts);
-
-  writer->ops->start (writer, filename);
-
-  writer->ops->write_header (writer);
-
-  /* Write descriptive info.  */
-
-  /* Write out the size of a register block.  */
-  writer->ops->write_regblock_type (writer, trace_regblock_size);
-
-  /* Write out status of the tracing run (aka "tstatus" info).  */
-  writer->ops->write_status (writer, ts);
-
-  /* Note that we want to upload tracepoints and save those, rather
-     than simply writing out the local ones, because the user may have
-     changed tracepoints in GDB in preparation for a future tracing
-     run, or maybe just mass-deleted all types of breakpoints as part
-     of cleaning up.  So as not to contaminate the session, leave the
-     data in its uploaded form, don't make into real tracepoints.  */
-
-  /* Get trace state variables first, they may be checked when parsing
-     uploaded commands.  */
-
-  target_upload_trace_state_variables (&uploaded_tsvs);
-
-  for (utsv = uploaded_tsvs; utsv; utsv = utsv->next)
-    writer->ops->write_uploaded_tsv (writer, utsv);
-
-  free_uploaded_tsvs (&uploaded_tsvs);
-
-  target_upload_tracepoints (&uploaded_tps);
-
-  for (utp = uploaded_tps; utp; utp = utp->next)
-    target_get_tracepoint_status (NULL, utp);
-
-  for (utp = uploaded_tps; utp; utp = utp->next)
-    writer->ops->write_uploaded_tp (writer, utp);
-
-  free_uploaded_tps (&uploaded_tps);
-
-  /* Mark the end of the definition section.  */
-  writer->ops->write_definition_end (writer);
-
-  /* Get and write the trace data proper.  */
-  while (1)
-    {
-      LONGEST gotten = 0;
-
-      /* The writer supports writing the contents of trace buffer
-	  directly to trace file.  Don't parse the contents of trace
-	  buffer.  */
-      if (writer->ops->write_trace_buffer != NULL)
-	{
-	  /* We ask for big blocks, in the hopes of efficiency, but
-	     will take less if the target has packet size limitations
-	     or some such.  */
-	  gotten = target_get_raw_trace_data (buf, offset,
-					      MAX_TRACE_UPLOAD);
-	  if (gotten < 0)
-	    error (_("Failure to get requested trace buffer data"));
-	  /* No more data is forthcoming, we're done.  */
-	  if (gotten == 0)
-	    break;
-
-	  writer->ops->write_trace_buffer (writer, buf, gotten);
-
-	  offset += gotten;
-	}
-      else
-	{
-	  uint16_t tp_num;
-	  uint32_t tf_size;
-	  /* Parse the trace buffers according to how data are stored
-	     in trace buffer in GDBserver.  */
-
-	  gotten = target_get_raw_trace_data (buf, offset, 6);
-
-	  if (gotten == 0)
-	    break;
-
-	  /* Read the first six bytes in, which is the tracepoint
-	     number and trace frame size.  */
-	  tp_num = (uint16_t)
-	    extract_unsigned_integer (&buf[0], 2, byte_order);
-
-	  tf_size = (uint32_t)
-	    extract_unsigned_integer (&buf[2], 4, byte_order);
-
-	  writer->ops->frame_ops->start (writer, tp_num);
-	  gotten = 6;
-
-	  if (tf_size > 0)
-	    {
-	      unsigned int block;
-
-	      offset += 6;
-
-	      for (block = 0; block < tf_size; )
-		{
-		  gdb_byte block_type;
-
-		  /* We'll fetch one block each time, in order to
-		     handle the extremely large 'M' block.  We first
-		     fetch one byte to get the type of the block.  */
-		  gotten = target_get_raw_trace_data (buf, offset, 1);
-		  if (gotten < 1)
-		    error (_("Failure to get requested trace buffer data"));
-
-		  gotten = 1;
-		  block += 1;
-		  offset += 1;
-
-		  block_type = buf[0];
-		  switch (block_type)
-		    {
-		    case 'R':
-		      gotten
-			= target_get_raw_trace_data (buf, offset,
-						     trace_regblock_size);
-		      if (gotten < trace_regblock_size)
-			error (_("Failure to get requested trace"
-				 " buffer data"));
-
-		      TRACE_WRITE_R_BLOCK (writer, buf,
-					   trace_regblock_size);
-		      break;
-		    case 'M':
-		      {
-			unsigned short mlen;
-			ULONGEST addr;
-			LONGEST t;
-			int j;
-
-			t = target_get_raw_trace_data (buf,offset, 10);
-			if (t < 10)
-			  error (_("Failure to get requested trace"
-				   " buffer data"));
-
-			offset += 10;
-			block += 10;
-
-			gotten = 0;
-			addr = (ULONGEST)
-			  extract_unsigned_integer (buf, 8,
-						    byte_order);
-			mlen = (unsigned short)
-			  extract_unsigned_integer (&buf[8], 2,
-						    byte_order);
-
-			TRACE_WRITE_M_BLOCK_HEADER (writer, addr,
-						    mlen);
-
-			/* The memory contents in 'M' block may be
-			   very large.  Fetch the data from the target
-			   and write them into file one by one.  */
-			for (j = 0; j < mlen; )
-			  {
-			    unsigned int read_length;
-
-			    if (mlen - j > MAX_TRACE_UPLOAD)
-			      read_length = MAX_TRACE_UPLOAD;
-			    else
-			      read_length = mlen - j;
-
-			    t = target_get_raw_trace_data (buf,
-							   offset + j,
-							   read_length);
-			    if (t < read_length)
-			      error (_("Failure to get requested"
-				       " trace buffer data"));
-
-			    TRACE_WRITE_M_BLOCK_MEMORY (writer, buf,
-							read_length);
-
-			    j += read_length;
-			    gotten += read_length;
-			  }
-
-			break;
-		      }
-		    case 'V':
-		      {
-			int vnum;
-			LONGEST val;
-
-			gotten
-			  = target_get_raw_trace_data (buf, offset,
-						       12);
-			if (gotten < 12)
-			  error (_("Failure to get requested"
-				   " trace buffer data"));
-
-			vnum  = (int) extract_signed_integer (buf,
-							      4,
-							      byte_order);
-			val
-			  = extract_signed_integer (&buf[4], 8,
-						    byte_order);
-
-			TRACE_WRITE_V_BLOCK (writer, vnum, val);
-		      }
-		      break;
-		    default:
-		      error (_("Unknown block type '%c' (0x%x) in"
-			       " trace frame"),
-			     block_type, block_type);
-		    }
-
-		  block += gotten;
-		  offset += gotten;
-		}
-	    }
-	  else
-	    offset += gotten;
-
-	  writer->ops->frame_ops->end (writer);
-	}
-    }
-
-  writer->ops->end (writer);
-}
-
-/* Return a trace writer for TFILE format.  */
-
-static struct trace_file_writer *
-tfile_trace_file_writer_new (void)
-{
-  struct tfile_trace_file_writer *writer
-    = xmalloc (sizeof (struct tfile_trace_file_writer));
-
-  writer->base.ops = &tfile_write_ops;
-  writer->fp = NULL;
-  writer->pathname = NULL;
-
-  return (struct trace_file_writer *) writer;
-}
-
-static void
-trace_save_command (char *args, int from_tty)
-{
-  int target_does_save = 0;
-  char **argv;
-  char *filename = NULL;
-  struct cleanup *back_to;
-  int generate_ctf = 0;
-  struct trace_file_writer *writer = NULL;
-
-  if (args == NULL)
-    error_no_arg (_("file in which to save trace data"));
-
-  argv = gdb_buildargv (args);
-  back_to = make_cleanup_freeargv (argv);
-
-  for (; *argv; ++argv)
-    {
-      if (strcmp (*argv, "-r") == 0)
-	target_does_save = 1;
-      if (strcmp (*argv, "-ctf") == 0)
-	generate_ctf = 1;
-      else if (**argv == '-')
-	error (_("unknown option `%s'"), *argv);
-      else
-	filename = *argv;
-    }
-
-  if (!filename)
-    error_no_arg (_("file in which to save trace data"));
-
-  if (generate_ctf)
-    writer = ctf_trace_file_writer_new ();
-  else
-    writer = tfile_trace_file_writer_new ();
-
-  make_cleanup (trace_file_writer_xfree, writer);
-
-  trace_save (filename, writer, target_does_save);
-
-  if (from_tty)
-    printf_filtered (_("Trace data saved to %s '%s'.\n"),
-		     generate_ctf ? "directory" : "file", filename);
-
-  do_cleanups (back_to);
-}
-
-/* Save the trace data to file FILENAME of tfile format.  */
-
-void
-trace_save_tfile (const char *filename, int target_does_save)
-{
-  struct trace_file_writer *writer;
-  struct cleanup *back_to;
-
-  writer = tfile_trace_file_writer_new ();
-  back_to = make_cleanup (trace_file_writer_xfree, writer);
-  trace_save (filename, writer, target_does_save);
-  do_cleanups (back_to);
-}
-
-/* Save the trace data to dir DIRNAME of ctf format.  */
-
-void
-trace_save_ctf (const char *dirname, int target_does_save)
-{
-  struct trace_file_writer *writer;
-  struct cleanup *back_to;
-
-  writer = ctf_trace_file_writer_new ();
-  back_to = make_cleanup (trace_file_writer_xfree, writer);
-
-  trace_save (dirname, writer, target_does_save);
-  do_cleanups (back_to);
-}
-
 /* Tell the target what to do with an ongoing tracing run if GDB
    disconnects for some reason.  */
 
@@ -3952,7 +3292,7 @@ get_uploaded_tp (int num, ULONGEST addr, struct uploaded_tp **utpp)
   return utp;
 }
 
-static void
+void
 free_uploaded_tps (struct uploaded_tp **utpp)
 {
   struct uploaded_tp *next_one;
@@ -3984,7 +3324,7 @@ get_uploaded_tsv (int num, struct uploaded_tsv **utsvp)
   return utsv;
 }
 
-static void
+void
 free_uploaded_tsvs (struct uploaded_tsv **utsvp)
 {
   struct uploaded_tsv *next_one;
@@ -5839,12 +5179,6 @@ _initialize_tracepoint (void)
   add_com ("tdump", class_trace, trace_dump_command,
 	   _("Print everything collected at the current tracepoint."));
 
-  add_com ("tsave", class_trace, trace_save_command, _("\
-Save the trace data to a file.\n\
-Use the '-ctf' option to save the data to CTF format.\n\
-Use the '-r' option to direct the target to save directly to the file,\n\
-using its own filesystem."));
-
   c = add_com ("tvariable", class_trace, trace_variable_command,_("\
 Define a trace state variable.\n\
 Argument is a $-prefixed name, optionally followed\n\
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index dd9b48b..6b00b30 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -155,6 +155,8 @@ extern char *default_collect;
 
 extern int trace_regblock_size;
 
+extern const char *stop_reason_names[];
+
 /* Struct to collect random info about tracepoints on the target.  */
 
 struct uploaded_tp
@@ -218,111 +220,6 @@ struct static_tracepoint_marker
   char *extra;
 };
 
-struct trace_file_writer;
-
-/* Operations to write trace frames to a specific trace format.  */
-
-struct trace_frame_write_ops
-{
-  /* Write a new trace frame.  The tracepoint number of this trace
-     frame is TPNUM.  */
-  void (*start) (struct trace_file_writer *self, uint16_t tpnum);
-
-  /* Write an 'R' block.  Buffer BUF contains its contents and SIZE is
-     its size.  */
-  void (*write_r_block) (struct trace_file_writer *self,
-			 gdb_byte *buf, int32_t size);
-
-  /* Write an 'M' block, the header and memory contents respectively.
-     The header of 'M' block is composed of the start address and the
-     length of memory collection, and the memory contents contain
-     the collected memory contents in tracing.
-     For extremely large M block, GDB is unable to get its contents
-     and write them into trace file in one go, due to the limitation
-     of the remote target or the size of internal buffer, we split
-     the operation to 'M' block to two operations.  */
-  /* Write the head of 'M' block.  ADDR is the start address of
-     collected memory and LENGTH is the length of memory contents.  */
-  void (*write_m_block_header) (struct trace_file_writer *self,
-				uint64_t addr, uint16_t length);
-  /* Write the memory contents of 'M' block.  Buffer BUF contains
-     its contents and LENGTH is its length.  This method can be called
-     multiple times to write large memory contents of a single 'M'
-     block.  */
-  void (*write_m_block_memory) (struct trace_file_writer *self,
-				gdb_byte *buf, uint16_t length);
-
-  /* Write a 'V' block.  NUM is the trace variable number and VAL is
-     the value of the trace variable.  */
-  void (*write_v_block) (struct trace_file_writer *self, int32_t num,
-			 uint64_t val);
-
-  /* The end of the trace frame.  */
-  void (*end) (struct trace_file_writer *self);
-};
-
-/* Operations to write trace buffers to a specific trace format.  */
-
-struct trace_file_write_ops
-{
-  /* Destructor.  Releases everything from SELF (but not SELF
-     itself).  */
-  void (*dtor) (struct trace_file_writer *self);
-
-  /*  Save the data to file or directory NAME of desired format in
-      target side.  Return true for success, otherwise return
-      false.  */
-  int (*target_save) (struct trace_file_writer *self,
-		      const char *name);
-
-  /* Write the trace buffers to file or directory NAME.  */
-  void (*start) (struct trace_file_writer *self,
-		 const char *name);
-
-  /* Write the trace header.  */
-  void (*write_header) (struct trace_file_writer *self);
-
-  /* Write the type of block about registers.  SIZE is the size of
-     all registers on the target.  */
-  void (*write_regblock_type) (struct trace_file_writer *self,
-			       int size);
-
-  /* Write trace status TS.  */
-  void (*write_status) (struct trace_file_writer *self,
-			struct trace_status *ts);
-
-  /* Write the uploaded TSV.  */
-  void (*write_uploaded_tsv) (struct trace_file_writer *self,
-			      struct uploaded_tsv *tsv);
-
-  /* Write the uploaded tracepoint TP.  */
-  void (*write_uploaded_tp) (struct trace_file_writer *self,
-			     struct uploaded_tp *tp);
-
-  /* Write to mark the end of the definition part.  */
-  void (*write_definition_end) (struct trace_file_writer *self);
-
-  /* Write the data of trace buffer without parsing.  The content is
-     in BUF and length is LEN.  */
-  void (*write_trace_buffer) (struct trace_file_writer *self,
-			      gdb_byte *buf, LONGEST len);
-
-  /* Operations to write trace frames.  The user of this field is
-     responsible to parse the data of trace buffer.  Either field
-     'write_trace_buffer' or field ' frame_ops' is NULL.  */
-  const struct trace_frame_write_ops *frame_ops;
-
-  /* The end of writing trace buffers.  */
-  void (*end) (struct trace_file_writer *self);
-};
-
-/* Trace file writer for a given format.  */
-
-struct trace_file_writer
-{
-  const struct trace_file_write_ops *ops;
-};
-
 struct memrange
 {
   /* memrange_absolute for absolute memory range, else basereg
@@ -424,8 +321,11 @@ extern void parse_tsv_definition (char *line, struct uploaded_tsv **utsvp);
 
 extern struct uploaded_tp *get_uploaded_tp (int num, ULONGEST addr,
 					    struct uploaded_tp **utpp);
+extern void free_uploaded_tps (struct uploaded_tp **utpp);
+
 extern struct uploaded_tsv *get_uploaded_tsv (int num,
 					      struct uploaded_tsv **utsvp);
+extern void free_uploaded_tsvs (struct uploaded_tsv **utsvp);
 extern struct tracepoint *create_tracepoint_from_upload (struct uploaded_tp *utp);
 extern void merge_uploaded_tracepoints (struct uploaded_tp **utpp);
 extern void merge_uploaded_trace_state_variables (struct uploaded_tsv **utsvp);
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
                   ` (2 preceding siblings ...)
  2014-02-12  6:08 ` [PATCH 4/8] Let tracefile has_memory and has_all_memory Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:17   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 8/8] Call target_traceframe_info when traceframe is selected Yao Qi
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

In the to_xfer_partial implementations of ctf and tfile, the code on
reading from read-only sections is duplicated.  This patch moves it to
a separate function exec_read_only_xfer_partial.

gdb:

2014-02-12  Yao Qi  <yao@codesourcery.com>

	* ctf.c (ctf_xfer_partial): Move code to ...
	* exec.c (exec_read_only_xfer_partial): ... it.  New function.
	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
	* tracefile.c: Include "exec.h".
	* exec.h (exec_read_only_xfer_partial): Declare.
---
 gdb/ctf.c             |   42 +-----------------------------------------
 gdb/exec.c            |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/exec.h            |    9 +++++++++
 gdb/tracefile-tfile.c |   36 +-----------------------------------
 gdb/tracefile.c       |    1 +
 5 files changed, 59 insertions(+), 76 deletions(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 8c60545..96ea966 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1467,47 +1467,7 @@ ctf_xfer_partial (struct target_ops *ops, enum target_object object,
       bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
     }
 
-  /* It's unduly pedantic to refuse to look at the executable for
-     read-only pieces; so do the equivalent of readonly regions aka
-     QTro packet.  */
-  if (exec_bfd != NULL)
-    {
-      asection *s;
-      bfd_size_type size;
-      bfd_vma vma;
-
-      for (s = exec_bfd->sections; s; s = s->next)
-	{
-	  if ((s->flags & SEC_LOAD) == 0
-	      || (s->flags & SEC_READONLY) == 0)
-	    continue;
-
-	  vma = s->vma;
-	  size = bfd_get_section_size (s);
-	  if (vma <= offset && offset < (vma + size))
-	    {
-	      ULONGEST amt;
-
-	      amt = (vma + size) - offset;
-	      if (amt > len)
-		amt = len;
-
-	      amt = bfd_get_section_contents (exec_bfd, s,
-					      readbuf, offset - vma, amt);
-
-	      if (amt == 0)
-		return TARGET_XFER_EOF;
-	      else
-		{
-		  *xfered_len = amt;
-		  return TARGET_XFER_OK;
-		}
-	    }
-	}
-    }
-
-  /* Indicate failure to find the requested memory block.  */
-  return TARGET_XFER_E_IO;
+  return exec_read_only_xfer_partial (readbuf, offset, len, xfered_len);
 }
 
 /* This is the implementation of target_ops method
diff --git a/gdb/exec.c b/gdb/exec.c
index 23d6187..bf422a7 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -529,6 +529,53 @@ remove_target_sections (void *owner)
 
 \f
 
+enum target_xfer_status
+exec_read_only_xfer_partial (gdb_byte *readbuf, ULONGEST offset,
+			     ULONGEST len, ULONGEST *xfered_len)
+{
+  /* It's unduly pedantic to refuse to look at the executable for
+     read-only pieces; so do the equivalent of readonly regions aka
+     QTro packet.  */
+  if (exec_bfd != NULL)
+    {
+      asection *s;
+      bfd_size_type size;
+      bfd_vma vma;
+
+      for (s = exec_bfd->sections; s; s = s->next)
+	{
+	  if ((s->flags & SEC_LOAD) == 0
+	      || (s->flags & SEC_READONLY) == 0)
+	    continue;
+
+	  vma = s->vma;
+	  size = bfd_get_section_size (s);
+	  if (vma <= offset && offset < (vma + size))
+	    {
+	      ULONGEST amt;
+
+	      amt = (vma + size) - offset;
+	      if (amt > len)
+		amt = len;
+
+	      amt = bfd_get_section_contents (exec_bfd, s,
+					      readbuf, offset - vma, amt);
+
+	      if (amt == 0)
+		return TARGET_XFER_EOF;
+	      else
+		{
+		  *xfered_len = amt;
+		  return TARGET_XFER_OK;
+		}
+	    }
+	}
+    }
+
+  /* Indicate failure to find the requested memory block.  */
+  return TARGET_XFER_E_IO;
+}
+
 VEC(mem_range_s) *
 section_table_available_memory (VEC(mem_range_s) *memory,
 				CORE_ADDR memaddr, ULONGEST len,
diff --git a/gdb/exec.h b/gdb/exec.h
index 4725f1b..a0c59c2 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -46,6 +46,15 @@ extern int build_section_table (struct bfd *, struct target_section **,
 
 extern int resize_section_table (struct target_section_table *, int);
 
+/* Read from mappable read-only sections of BFD executable files.
+   Return TARGET_XFER_OK, if read is successful.  Return
+   TARGET_XFER_EOF if read is done.  Return TARGET_XFER_E_IO
+   otherwise.  */
+
+extern enum target_xfer_status
+  exec_read_only_xfer_partial (gdb_byte *readbuf, ULONGEST offset,
+			       ULONGEST len, ULONGEST *xfered_len);
+
 /* Appends all read-only memory ranges found in the target section
    table defined by SECTIONS and SECTIONS_END, starting at (and
    intersected with) MEMADDR for LEN bytes.  Returns the augmented
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 80df960..dcd46e4 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -933,41 +933,7 @@ tfile_xfer_partial (struct target_ops *ops, enum target_object object,
 	}
     }
 
-  /* It's unduly pedantic to refuse to look at the executable for
-     read-only pieces; so do the equivalent of readonly regions aka
-     QTro packet.  */
-  /* FIXME account for relocation at some point.  */
-  if (exec_bfd)
-    {
-      asection *s;
-      bfd_size_type size;
-      bfd_vma vma;
-
-      for (s = exec_bfd->sections; s; s = s->next)
-	{
-	  if ((s->flags & SEC_LOAD) == 0
-	      || (s->flags & SEC_READONLY) == 0)
-	    continue;
-
-	  vma = s->vma;
-	  size = bfd_get_section_size (s);
-	  if (vma <= offset && offset < (vma + size))
-	    {
-	      ULONGEST amt;
-
-	      amt = (vma + size) - offset;
-	      if (amt > len)
-		amt = len;
-
-	      *xfered_len = bfd_get_section_contents (exec_bfd, s,
-						      readbuf, offset - vma, amt);
-	      return TARGET_XFER_OK;
-	    }
-	}
-    }
-
-  /* Indicate failure to find the requested memory block.  */
-  return TARGET_XFER_E_IO;
+  return exec_read_only_xfer_partial (readbuf, offset, len, xfered_len);
 }
 
 /* Iterate through the blocks of a trace frame, looking for a 'V'
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index a2dc9fd..5e263c5 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -20,6 +20,7 @@
 #include "defs.h"
 #include "tracefile.h"
 #include "ctf.h"
+#include "exec.h"
 
 /* Helper macros.  */
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
                   ` (5 preceding siblings ...)
  2014-02-12  6:08 ` [PATCH 1/8] Move trace file writer out of tracepoint.c Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:18   ` Pedro Alves
  2014-02-12  6:08 ` [PATCH 3/8] Share some code between " Yao Qi
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

This patch adjust both ctf and tfile target implementation of to_xfer_partial,
to return TARGET_XFER_E_UNAVAILABLE and set *XFERED_LEN if data is
unavailable.

gdb:

2014-02-12  Yao Qi  <yao@codesourcery.com>

	* exec.c (section_table_available_memory): Make it static.  Move
	comments from exec.h.
	(section_table_xfer_available_memory): New function.
	* exec.h (section_table_available_memory): Remove declaration.
	(section_table_xfer_available_memory): Declare.
	* ctf.c (ctf_xfer_partial): Call
	section_table_xfer_available_memory.
	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
---
 gdb/ctf.c             |    9 +++++-
 gdb/exec.c            |   61 ++++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/exec.h            |   19 ++++++---------
 gdb/tracefile-tfile.c |   10 ++++++-
 4 files changed, 83 insertions(+), 16 deletions(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 96ea966..62152d3 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1465,9 +1465,14 @@ ctf_xfer_partial (struct target_ops *ops, enum target_object object,
 
       /* Restore the position.  */
       bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
-    }
 
-  return exec_read_only_xfer_partial (readbuf, offset, len, xfered_len);
+      return exec_read_only_xfer_partial (readbuf, offset, len, xfered_len);
+    }
+  else
+    {
+      /* Fallback to reading from read-only sections.  */
+      return section_table_xfer_available_memory (readbuf, offset, len, xfered_len);
+    }
 }
 
 /* This is the implementation of target_ops method
diff --git a/gdb/exec.c b/gdb/exec.c
index bf422a7..e649bf0 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -576,7 +576,12 @@ exec_read_only_xfer_partial (gdb_byte *readbuf, ULONGEST offset,
   return TARGET_XFER_E_IO;
 }
 
-VEC(mem_range_s) *
+/* Appends all read-only memory ranges found in the target section
+   table defined by SECTIONS and SECTIONS_END, starting at (and
+   intersected with) MEMADDR for LEN bytes.  Returns the augmented
+   VEC.  */
+
+static VEC(mem_range_s) *
 section_table_available_memory (VEC(mem_range_s) *memory,
 				CORE_ADDR memaddr, ULONGEST len,
 				struct target_section *sections,
@@ -614,6 +619,60 @@ section_table_available_memory (VEC(mem_range_s) *memory,
 }
 
 enum target_xfer_status
+section_table_xfer_available_memory (gdb_byte *readbuf, ULONGEST offset,
+				     ULONGEST len, ULONGEST *xfered_len)
+{
+  VEC(mem_range_s) *available_memory = NULL;
+  struct target_section_table *table;
+  struct cleanup *old_chain;
+  mem_range_s *r;
+  int i;
+
+  table = target_get_section_table (&exec_ops);
+  available_memory = section_table_available_memory (available_memory,
+						     offset, len,
+						     table->sections,
+						     table->sections_end);
+
+  old_chain = make_cleanup (VEC_cleanup(mem_range_s),
+			    &available_memory);
+
+  normalize_mem_ranges (available_memory);
+
+  for (i = 0;
+       VEC_iterate (mem_range_s, available_memory, i, r);
+       i++)
+    {
+      if (mem_ranges_overlap (r->start, r->length, offset, len))
+	{
+	  CORE_ADDR end;
+	  enum target_xfer_status status;
+
+	  /* Get the intersection window.  */
+	  end = min (offset + len, r->start + r->length);
+
+	  gdb_assert (end - offset <= len);
+
+	  if (offset >= r->start)
+	    status = exec_read_only_xfer_partial (readbuf, offset,
+						  end - offset,
+						  xfered_len);
+	  else
+	    {
+	      *xfered_len = r->start - offset;
+	      status = TARGET_XFER_E_UNAVAILABLE;
+	    }
+	  do_cleanups (old_chain);
+	  return status;
+	}
+    }
+  do_cleanups (old_chain);
+
+  *xfered_len = len;
+  return TARGET_XFER_E_UNAVAILABLE;
+}
+
+enum target_xfer_status
 section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf,
 				   ULONGEST offset, ULONGEST len,
 				   ULONGEST *xfered_len,
diff --git a/gdb/exec.h b/gdb/exec.h
index a0c59c2..f6ccf07 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -55,17 +55,6 @@ extern enum target_xfer_status
   exec_read_only_xfer_partial (gdb_byte *readbuf, ULONGEST offset,
 			       ULONGEST len, ULONGEST *xfered_len);
 
-/* Appends all read-only memory ranges found in the target section
-   table defined by SECTIONS and SECTIONS_END, starting at (and
-   intersected with) MEMADDR for LEN bytes.  Returns the augmented
-   VEC.  */
-
-extern VEC(mem_range_s) *
-  section_table_available_memory (VEC(mem_range_s) *ranges,
-				  CORE_ADDR memaddr, ULONGEST len,
-				  struct target_section *sections,
-				  struct target_section *sections_end);
-
 /* Read or write from mappable sections of BFD executable files.
 
    Request to transfer up to LEN 8-bit bytes of the target sections
@@ -91,6 +80,14 @@ extern enum target_xfer_status
 				     struct target_section *,
 				     const char *);
 
+/* Read from mappable read-only sections of BFD executable files.
+   Similar to exec_read_only_xfer_partial, but return
+   TARGET_XFER_E_UNAVAILABLE is data is unavailable.  */
+
+extern enum target_xfer_status
+  section_table_xfer_available_memory (gdb_byte *readbuf, ULONGEST offset,
+				       ULONGEST len, ULONGEST *xfered_len);
+
 /* Set the loaded address of a section.  */
 extern void exec_set_section_address (const char *, int, CORE_ADDR);
 
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index dcd46e4..c3cc6a4 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -931,9 +931,15 @@ tfile_xfer_partial (struct target_ops *ops, enum target_object object,
 	  /* Skip over this block.  */
 	  pos += (8 + 2 + mlen);
 	}
-    }
 
-  return exec_read_only_xfer_partial (readbuf, offset, len, xfered_len);
+      return exec_read_only_xfer_partial (readbuf, offset, len, xfered_len);
+    }
+  else
+    {
+      /* Fallback to reading from read-only sections.  */
+      return section_table_xfer_available_memory (readbuf, offset, len,
+						  xfered_len);
+    }
 }
 
 /* Iterate through the blocks of a trace frame, looking for a 'V'
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/8] Share some code between ctf and tfile target.
  2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
                   ` (6 preceding siblings ...)
  2014-02-12  6:08 ` [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
@ 2014-02-12  6:08 ` Yao Qi
  2014-02-20 13:15   ` Pedro Alves
  7 siblings, 1 reply; 23+ messages in thread
From: Yao Qi @ 2014-02-12  6:08 UTC (permalink / raw)
  To: gdb-patches

This patch move the duplicated code between tfile and ctf
targets into file tracefile.c.  The common part of target_ops
fields are set in init_tracefile_ops.

gdb:

2014-02-12  Yao Qi  <yao@codesourcery.com>

	* ctf.c (ctf_has_stack, ctf_has_registers): Remove.
	(ctf_thread_alive, ctf_get_trace_status): Remove.
	(init_ctf_ops): Don't set some fields of ctf_ops.  Call
	init_tracefile_ops.
	* tracefile-tfile.c (tfile_get_trace_status): Remove.
	(tfile_has_stack, tfile_has_registers): Remove.
	(tfile_thread_alive): Remove.
	(init_tfile_ops): Dont' set some fields of tfile_ops.  Call
	init_tracefile_ops.
	* tracefile.c (tracefile_has_stack): New function.
	(tracefile_has_registers): New function.
	(tracefile_thread_alive): New function.
	(tracefile_get_trace_status): New function.
	(init_tracefile_ops): New function.
	* tracefile.h (init_tracefile_ops): Declare.
---
 gdb/ctf.c             |   48 +------------------------------------------
 gdb/tracefile-tfile.c |   37 +-------------------------------
 gdb/tracefile.c       |   54 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/tracefile.h       |    2 +
 4 files changed, 59 insertions(+), 82 deletions(-)

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 8ad363e..8c60545 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1733,35 +1733,6 @@ ctf_trace_find (enum trace_find_type type, int num,
   return -1;
 }
 
-/* This is the implementation of target_ops method to_has_stack.
-   The target has a stack when GDB has already selected one trace
-   frame.  */
-
-static int
-ctf_has_stack (struct target_ops *ops)
-{
-  return get_traceframe_number () != -1;
-}
-
-/* This is the implementation of target_ops method to_has_registers.
-   The target has registers when GDB has already selected one trace
-   frame.  */
-
-static int
-ctf_has_registers (struct target_ops *ops)
-{
-  return get_traceframe_number () != -1;
-}
-
-/* This is the implementation of target_ops method to_thread_alive.
-   CTF trace data has one thread faked by GDB.  */
-
-static int
-ctf_thread_alive (struct target_ops *ops, ptid_t ptid)
-{
-  return 1;
-}
-
 /* This is the implementation of target_ops method to_traceframe_info.
    Iterate the events whose name is "memory", in current
    frame, extract memory range information, and return them in
@@ -1834,23 +1805,12 @@ ctf_traceframe_info (void)
   return info;
 }
 
-/* This is the implementation of target_ops method to_get_trace_status.
-   The trace status for a file is that tracing can never be run.  */
-
-static int
-ctf_get_trace_status (struct trace_status *ts)
-{
-  /* Other bits of trace status were collected as part of opening the
-     trace files, so nothing to do here.  */
-
-  return -1;
-}
-
 static void
 init_ctf_ops (void)
 {
   memset (&ctf_ops, 0, sizeof (ctf_ops));
 
+  init_tracefile_ops (&ctf_ops);
   ctf_ops.to_shortname = "ctf";
   ctf_ops.to_longname = "CTF file";
   ctf_ops.to_doc = "Use a CTF directory as a target.\n\
@@ -1860,16 +1820,10 @@ Specify the filename of the CTF directory.";
   ctf_ops.to_fetch_registers = ctf_fetch_registers;
   ctf_ops.to_xfer_partial = ctf_xfer_partial;
   ctf_ops.to_files_info = ctf_files_info;
-  ctf_ops.to_get_trace_status = ctf_get_trace_status;
   ctf_ops.to_trace_find = ctf_trace_find;
   ctf_ops.to_get_trace_state_variable_value
     = ctf_get_trace_state_variable_value;
-  ctf_ops.to_stratum = process_stratum;
-  ctf_ops.to_has_stack = ctf_has_stack;
-  ctf_ops.to_has_registers = ctf_has_registers;
   ctf_ops.to_traceframe_info = ctf_traceframe_info;
-  ctf_ops.to_thread_alive = ctf_thread_alive;
-  ctf_ops.to_magic = OPS_MAGIC;
 }
 
 #endif
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 1310151..205494e 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -557,17 +557,6 @@ tfile_files_info (struct target_ops *t)
   printf_filtered ("\t`%s'\n", trace_filename);
 }
 
-/* The trace status for a file is that tracing can never be run.  */
-
-static int
-tfile_get_trace_status (struct trace_status *ts)
-{
-  /* Other bits of trace status were collected as part of opening the
-     trace files, so nothing to do here.  */
-
-  return -1;
-}
-
 static void
 tfile_get_tracepoint_status (struct breakpoint *tp, struct uploaded_tp *utp)
 {
@@ -1029,24 +1018,6 @@ tfile_has_memory (struct target_ops *ops)
   return 1;
 }
 
-static int
-tfile_has_stack (struct target_ops *ops)
-{
-  return get_traceframe_number () != -1;
-}
-
-static int
-tfile_has_registers (struct target_ops *ops)
-{
-  return get_traceframe_number () != -1;
-}
-
-static int
-tfile_thread_alive (struct target_ops *ops, ptid_t ptid)
-{
-  return 1;
-}
-
 /* Callback for traceframe_walk_blocks.  Builds a traceframe_info
    object for the tfile target's current traceframe.  */
 
@@ -1113,6 +1084,8 @@ tfile_traceframe_info (void)
 static void
 init_tfile_ops (void)
 {
+  init_tracefile_ops (&tfile_ops);
+
   tfile_ops.to_shortname = "tfile";
   tfile_ops.to_longname = "Local trace dump file";
   tfile_ops.to_doc
@@ -1122,19 +1095,13 @@ init_tfile_ops (void)
   tfile_ops.to_fetch_registers = tfile_fetch_registers;
   tfile_ops.to_xfer_partial = tfile_xfer_partial;
   tfile_ops.to_files_info = tfile_files_info;
-  tfile_ops.to_get_trace_status = tfile_get_trace_status;
   tfile_ops.to_get_tracepoint_status = tfile_get_tracepoint_status;
   tfile_ops.to_trace_find = tfile_trace_find;
   tfile_ops.to_get_trace_state_variable_value
     = tfile_get_trace_state_variable_value;
-  tfile_ops.to_stratum = process_stratum;
   tfile_ops.to_has_all_memory = tfile_has_all_memory;
   tfile_ops.to_has_memory = tfile_has_memory;
-  tfile_ops.to_has_stack = tfile_has_stack;
-  tfile_ops.to_has_registers = tfile_has_registers;
   tfile_ops.to_traceframe_info = tfile_traceframe_info;
-  tfile_ops.to_thread_alive = tfile_thread_alive;
-  tfile_ops.to_magic = OPS_MAGIC;
 }
 
 extern initialize_file_ftype _initialize_tracefile_tfile;
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index 9945ae5..0f8ffc5 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -376,6 +376,60 @@ trace_save_ctf (const char *dirname, int target_does_save)
   do_cleanups (back_to);
 }
 
+/* This is the implementation of target_ops method to_has_stack.
+   The target has a stack when GDB has already selected one trace
+   frame.  */
+
+static int
+tracefile_has_stack (struct target_ops *ops)
+{
+  return get_traceframe_number () != -1;
+}
+
+/* This is the implementation of target_ops method to_has_registers.
+   The target has registers when GDB has already selected one trace
+   frame.  */
+
+static int
+tracefile_has_registers (struct target_ops *ops)
+{
+  return get_traceframe_number () != -1;
+}
+
+/* This is the implementation of target_ops method to_thread_alive.
+   tracefile has one thread faked by GDB.  */
+
+static int
+tracefile_thread_alive (struct target_ops *ops, ptid_t ptid)
+{
+  return 1;
+}
+
+/* This is the implementation of target_ops method to_get_trace_status.
+   The trace status for a file is that tracing can never be run.  */
+
+static int
+tracefile_get_trace_status (struct trace_status *ts)
+{
+  /* Other bits of trace status were collected as part of opening the
+     trace files, so nothing to do here.  */
+
+  return -1;
+}
+
+/* Initialize OPS for tracefile related targets.  */
+
+void
+init_tracefile_ops (struct target_ops *ops)
+{
+  ops->to_stratum = process_stratum;
+  ops->to_get_trace_status = tracefile_get_trace_status;
+  ops->to_has_stack = tracefile_has_stack;
+  ops->to_has_registers = tracefile_has_registers;
+  ops->to_thread_alive = tracefile_thread_alive;
+  ops->to_magic = OPS_MAGIC;
+}
+
 extern initialize_file_ftype _initialize_tracefile;
 
 void
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
index 833de5c..db454e3 100644
--- a/gdb/tracefile.h
+++ b/gdb/tracefile.h
@@ -111,4 +111,6 @@ struct trace_file_writer
 
 extern struct trace_file_writer *tfile_trace_file_writer_new (void);
 
+extern void init_tracefile_ops (struct target_ops *ops);
+
 #endif /* TRACEFILE_H */
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/8] Move tfile target to tracefile-tfile.c
  2014-02-12  6:08 ` [PATCH 2/8] Move tfile target to tracefile-tfile.c Yao Qi
@ 2014-02-20 13:14   ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:14 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:
> This patch moves tfile target related code from tracepoint.c to
> tracefile-tfile.c.
> 

OK.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/8] Move trace file writer out of tracepoint.c
  2014-02-12  6:08 ` [PATCH 1/8] Move trace file writer out of tracepoint.c Yao Qi
@ 2014-02-20 13:14   ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:14 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:
> This patch is a refactor which moves trace file writer related code
> out of tracepoint.c, which has 6k LOC.  It moves general trace file
> writer to a new file tracefile.c and moves tfile specific writer to
> tracefile-tfile.c.

OK.

> 2014-02-12  Yao Qi  <yao@codesourcery.com>
> 
> 	* Makefile.in (REMOTE_OBS): Append tracefile.o and
> 	tracefile-tfile.o.

(Eh, I had never noticed REMOTE_OBS before.  Looks like
it's unnecessary nowadays, and could just be merged to
COMMON_OBS).

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/8] Let tracefile has_memory and has_all_memory.
  2014-02-12  6:08 ` [PATCH 4/8] Let tracefile has_memory and has_all_memory Yao Qi
@ 2014-02-20 13:15   ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:15 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:
> At present, tfile target thinks it has memory but ctf doesn't.
> This is an oversight when I added ctf target support.  This patch
> moves the implementations of to_has_all_memory and to_has_memory to
> upper layer.  After this change, both tfile and ctf target think
> they have memory.

OK.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/8] Share some code between ctf and tfile target.
  2014-02-12  6:08 ` [PATCH 3/8] Share some code between " Yao Qi
@ 2014-02-20 13:15   ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:15 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:
> 	(init_tfile_ops): Dont' set some fields of tfile_ops.  Call
> 	init_tracefile_ops.

typo: "Don't".

OK.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target
  2014-02-12  6:08 ` [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target Yao Qi
@ 2014-02-20 13:17   ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:17 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:

> +enum target_xfer_status
> +exec_read_only_xfer_partial (gdb_byte *readbuf, ULONGEST offset,
> +			     ULONGEST len, ULONGEST *xfered_len)

I think 'TARGET _ METHOD _ OVERLOAD_REASON' is clearer and
results in a better interface when we end up with multiple
overloads (thinking ahead).  E.g.:

 exec_xfer_partial_read_only
 exec_xfer_partial_loaded_only

vs:

 exec_read_only_xfer_partial
 exec_loaded_only_xfer_partial

etc.

Also nicer for discovery with tab completion, IMO.

Another nit is that so far, we've tended to reserve "xfer" for functions
that take both a read and write argument, using "read" and "write"
explicitly in the signature instead in functions that do only
one of the directions.  E.g., target_read_partial,
target_write_partial.  I think following that results in a
least surpristing API.  IOW, I think this function name would
be clearer:

enum target_xfer_status
exec_read_partial_read_only (gdb_byte *readbuf, ULONGEST offset,
			     ULONGEST len, ULONGEST *xfered_len)

Otherwise looks good to me.  Thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target
  2014-02-12  6:08 ` [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
@ 2014-02-20 13:18   ` Pedro Alves
  2014-02-23  3:55     ` Yao Qi
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:18 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:
> +/* Read from mappable read-only sections of BFD executable files.
> +   Similar to exec_read_only_xfer_partial, but return
> +   TARGET_XFER_E_UNAVAILABLE is data is unavailable.  */

Typo: s/is data/if data/

> +
> +extern enum target_xfer_status
> +  section_table_xfer_available_memory (gdb_byte *readbuf, ULONGEST offset,
> +				       ULONGEST len, ULONGEST *xfered_len);

The comment on function names I made on a previous patch applies here too.

This only reads, so instead:

extern enum target_xfer_status
  section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
			               ULONGEST len, ULONGEST *xfered_len);

or:

extern enum target_xfer_status
  section_table_read_partial_available_memory (gdb_byte *readbuf,
					       ULONGEST offset,
				               ULONGEST len,
					       ULONGEST *xfered_len);

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 8/8] Call target_traceframe_info when traceframe is selected.
  2014-02-12  6:08 ` [PATCH 8/8] Call target_traceframe_info when traceframe is selected Yao Qi
@ 2014-02-20 13:30   ` Pedro Alves
  2014-02-21  8:22     ` Yao Qi
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 13:30 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:

> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -4290,7 +4290,7 @@ parse_traceframe_info (const char *tframe_info)
>  struct traceframe_info *
>  get_traceframe_info (void)
>  {
> -  if (traceframe_info == NULL)
> +  if (traceframe_info == NULL && get_traceframe_number () >= 0)
>      traceframe_info = target_traceframe_info ();
>  
>    return traceframe_info;

Hmm, it looks cleaner to me to not have the check here.
It looked to me that after this patch, all callers of
get_traceframe_info or traceframe_available_memory are already
checking whether a traceframe is selected.  So why's this hunk
needed?  What did I miss?

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
  2014-02-12  6:08 ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Yao Qi
@ 2014-02-20 14:06   ` Pedro Alves
  2014-02-23  3:54     ` Yao Qi
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2014-02-20 14:06 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/12/2014 06:05 AM, Yao Qi wrote:
> As the new to_xfer_partial implementations are done in ctf and tfile
> targets

I think we'll want to move the traceframe_available_memory
code from memory_xfer_partial_1 down to the targets too
(at some point).

> -	  if (unavail != memaddr + length)
> -	    mark_value_bytes_unavailable (val,
> -					  embedded_offset + unavail - memaddr,
> -					  (memaddr + length) - unavail);
> +      if (status == TARGET_XFER_EOF)
> +	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
> +      else if (status == TARGET_XFER_E_UNAVAILABLE)
> +	mark_value_bytes_unavailable (val, embedded_offset + xfered,
> +				      xfered_len);
> +      else if (TARGET_XFER_STATUS_ERROR_P (status))
> +	memory_error (status, memaddr + xfered);

I maintain that using TARGET_XFER_STATUS_ERROR_P tends to
be unnecessary, and actually leads to fragile code.   It doesn't
really help here.  A newer status added in the future that is
not an error will pass by here unhandled.  Handle TARGET_XFER_OK
explicitly instead, like this:

      if (status == TARGET_XFER_EOF)
	/* nothing */;
      else if (status == TARGET_XFER_E_UNAVAILABLE)
	mark_value_bytes_unavailable (val, embedded_offset + xfered,
				      xfered_len);
      else if (status == TARGET_XFER_EOF)
	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
      else
	memory_error (status, memaddr + xfered);

      xfered += xfered_len;
      QUIT;
    }

and now all future unhandled statuses fall through to
memory_error.

(Hmm, the _E_ in TARGET_XFER_E_UNAVAILABLE is now starting
to look a little odd and confusing to me.  E.g., it's not
really an error in this case.  I'm pondering renaming it to
TARGET_XFER_UNAVAILABLE.)

Otherwise looks good, thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 8/8] Call target_traceframe_info when traceframe is selected.
  2014-02-20 13:30   ` Pedro Alves
@ 2014-02-21  8:22     ` Yao Qi
  0 siblings, 0 replies; 23+ messages in thread
From: Yao Qi @ 2014-02-21  8:22 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 02/20/2014 09:30 PM, Pedro Alves wrote:
> On 02/12/2014 06:05 AM, Yao Qi wrote:
> 
>> --- a/gdb/tracepoint.c
>> +++ b/gdb/tracepoint.c
>> @@ -4290,7 +4290,7 @@ parse_traceframe_info (const char *tframe_info)
>>  struct traceframe_info *
>>  get_traceframe_info (void)
>>  {
>> -  if (traceframe_info == NULL)
>> +  if (traceframe_info == NULL && get_traceframe_number () >= 0)
>>      traceframe_info = target_traceframe_info ();
>>  
>>    return traceframe_info;
> 
> Hmm, it looks cleaner to me to not have the check here.
> It looked to me that after this patch, all callers of
> get_traceframe_info or traceframe_available_memory are already
> checking whether a traceframe is selected.  So why's this hunk
> needed?  What did I miss?
> 

No, you are right that this check becomes unnecessary.  I added
this check in get_traceframe_info in order to guard
remote_traceframe_info, but didn't notice that "whether a traceframe
is selected" is checked in the callers of get_traceframe_info.

I'll remove this part from the patch.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
  2014-02-20 14:06   ` Pedro Alves
@ 2014-02-23  3:54     ` Yao Qi
  2014-02-24  3:38       ` [PUSHED] Remove TARGET_XFER_STATUS_ERROR_P (Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial) Yao Qi
                         ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Yao Qi @ 2014-02-23  3:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 02/20/2014 09:20 PM, Pedro Alves wrote:
> I think we'll want to move the traceframe_available_memory
> code from memory_xfer_partial_1 down to the targets too
> (at some point).
> 

Yes, that'll be a step forward after this patch,

  [unavailable values part 1, 05/17] move traceframe memory reading fallback to read-only sections to GDB side
  https://sourceware.org/ml/gdb-patches/2011-02/msg00136.html

and we'll move "traceframe memory reading fallback to read-only
sections" from GDB core to GDB target part, such as tfile, ctf and
remote.

AFAICS, targets tfile and ctf nowadays have such fallback logic,
but target remote doesn't.  We need to add such logic in remote
target.  I'll look into it.

>> > -	  if (unavail != memaddr + length)
>> > -	    mark_value_bytes_unavailable (val,
>> > -					  embedded_offset + unavail - memaddr,
>> > -					  (memaddr + length) - unavail);
>> > +      if (status == TARGET_XFER_EOF)
>> > +	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
>> > +      else if (status == TARGET_XFER_E_UNAVAILABLE)
>> > +	mark_value_bytes_unavailable (val, embedded_offset + xfered,
>> > +				      xfered_len);
>> > +      else if (TARGET_XFER_STATUS_ERROR_P (status))
>> > +	memory_error (status, memaddr + xfered);
> I maintain that using TARGET_XFER_STATUS_ERROR_P tends to
> be unnecessary, and actually leads to fragile code.   It doesn't
> really help here.  A newer status added in the future that is
> not an error will pass by here unhandled.  Handle TARGET_XFER_OK
> explicitly instead, like this:
> 
>       if (status == TARGET_XFER_EOF)
                      ^^^^^^^^^^^^^^^
It should be TARGET_XFER_OK
> 	/* nothing */;
>       else if (status == TARGET_XFER_E_UNAVAILABLE)
> 	mark_value_bytes_unavailable (val, embedded_offset + xfered,
> 				      xfered_len);
>       else if (status == TARGET_XFER_EOF)
> 	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
>       else
> 	memory_error (status, memaddr + xfered);
> 
>       xfered += xfered_len;
>       QUIT;
>     }
> 
> and now all future unhandled statuses fall through to
> memory_error.

OK, update patch as you suggested.  I'll get rid of
TARGET_XFER_STATUS_ERROR_P in a follow-up patch.

> 
> (Hmm, the _E_ in TARGET_XFER_E_UNAVAILABLE is now starting
> to look a little odd and confusing to me.  E.g., it's not
> really an error in this case.  I'm pondering renaming it to
> TARGET_XFER_UNAVAILABLE.)

It isn't an error.  Will rename it in a follow-up patch too.

Patch below is pushed in.  Note that I move hunk on
making section_table_available_memory static from patch #6
to patch #7, to fix a compilation error when patches #1~#6
are applied.

-- 
Yao (齐尧)

gdb:

2014-02-23  Yao Qi  <yao@codesourcery.com>

	* valops.c (read_value_memory): Rewrite it.  Call
	target_xfer_partial in a loop.
	* exec.h (section_table_available_memory): Remove declaration.
	Move comments to ...
	* exec.c (section_table_available_memory): ... here.  Make it static.
---
 gdb/ChangeLog |    9 +++++
 gdb/exec.c    |    7 +++-
 gdb/exec.h    |   11 ------
 gdb/valops.c  |   96 +++++++++++++-------------------------------------------
 4 files changed, 38 insertions(+), 85 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 55745af..326909f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2014-02-23  Yao Qi  <yao@codesourcery.com>
 
+	* valops.c (read_value_memory): Rewrite it.  Call
+	target_xfer_partial in a loop.
+	* exec.h (section_table_available_memory): Remove declaration.
+	Move comments to ...
+	* exec.c (section_table_available_memory): ... here.  Make it
+	static.
+
+2014-02-23  Yao Qi  <yao@codesourcery.com>
+
 	* exec.c (section_table_read_available_memory): New function.
 	* exec.h (section_table_read_available_memory): Declare.
 	* ctf.c (ctf_xfer_partial): Call
diff --git a/gdb/exec.c b/gdb/exec.c
index 607f5ac..758e382 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -577,7 +577,12 @@ exec_read_partial_read_only (gdb_byte *readbuf, ULONGEST offset,
   return TARGET_XFER_E_IO;
 }
 
-VEC(mem_range_s) *
+/* Appends all read-only memory ranges found in the target section
+   table defined by SECTIONS and SECTIONS_END, starting at (and
+   intersected with) MEMADDR for LEN bytes.  Returns the augmented
+   VEC.  */
+
+static VEC(mem_range_s) *
 section_table_available_memory (VEC(mem_range_s) *memory,
 				CORE_ADDR memaddr, ULONGEST len,
 				struct target_section *sections,
diff --git a/gdb/exec.h b/gdb/exec.h
index 84dc40f..4d9de90 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -55,17 +55,6 @@ extern enum target_xfer_status
   exec_read_partial_read_only (gdb_byte *readbuf, ULONGEST offset,
 			       ULONGEST len, ULONGEST *xfered_len);
 
-/* Appends all read-only memory ranges found in the target section
-   table defined by SECTIONS and SECTIONS_END, starting at (and
-   intersected with) MEMADDR for LEN bytes.  Returns the augmented
-   VEC.  */
-
-extern VEC(mem_range_s) *
-  section_table_available_memory (VEC(mem_range_s) *ranges,
-				  CORE_ADDR memaddr, ULONGEST len,
-				  struct target_section *sections,
-				  struct target_section *sections_end);
-
 /* Read or write from mappable sections of BFD executable files.
 
    Request to transfer up to LEN 8-bit bytes of the target sections
diff --git a/gdb/valops.c b/gdb/valops.c
index 898401d..0d726d0 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -949,81 +949,31 @@ read_value_memory (struct value *val, int embedded_offset,
 		   int stack, CORE_ADDR memaddr,
 		   gdb_byte *buffer, size_t length)
 {
-  if (length)
-    {
-      VEC(mem_range_s) *available_memory;
-
-      if (!traceframe_available_memory (&available_memory, memaddr, length))
-	{
-	  if (stack)
-	    read_stack (memaddr, buffer, length);
-	  else
-	    read_memory (memaddr, buffer, length);
-	}
+  ULONGEST xfered = 0;
+
+  while (xfered < length)
+    {
+      enum target_xfer_status status;
+      ULONGEST xfered_len;
+
+      status = target_xfer_partial (current_target.beneath,
+				    TARGET_OBJECT_MEMORY, NULL,
+				    buffer + xfered, NULL,
+				    memaddr + xfered, length - xfered,
+				    &xfered_len);
+
+      if (status == TARGET_XFER_OK)
+	/* nothing */;
+      else if (status == TARGET_XFER_E_UNAVAILABLE)
+	mark_value_bytes_unavailable (val, embedded_offset + xfered,
+				      xfered_len);
+      else if (status == TARGET_XFER_EOF)
+	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
       else
-	{
-	  struct target_section_table *table;
-	  struct cleanup *old_chain;
-	  CORE_ADDR unavail;
-	  mem_range_s *r;
-	  int i;
-
-	  /* Fallback to reading from read-only sections.  */
-	  table = target_get_section_table (&exec_ops);
-	  available_memory =
-	    section_table_available_memory (available_memory,
-					    memaddr, length,
-					    table->sections,
-					    table->sections_end);
-
-	  old_chain = make_cleanup (VEC_cleanup(mem_range_s),
-				    &available_memory);
-
-	  normalize_mem_ranges (available_memory);
+	memory_error (status, memaddr + xfered);
 
-	  /* Mark which bytes are unavailable, and read those which
-	     are available.  */
-
-	  unavail = memaddr;
-
-	  for (i = 0;
-	       VEC_iterate (mem_range_s, available_memory, i, r);
-	       i++)
-	    {
-	      if (mem_ranges_overlap (r->start, r->length,
-				      memaddr, length))
-		{
-		  CORE_ADDR lo1, hi1, lo2, hi2;
-		  CORE_ADDR start, end;
-
-		  /* Get the intersection window.  */
-		  lo1 = memaddr;
-		  hi1 = memaddr + length;
-		  lo2 = r->start;
-		  hi2 = r->start + r->length;
-		  start = max (lo1, lo2);
-		  end = min (hi1, hi2);
-
-		  gdb_assert (end - memaddr <= length);
-
-		  if (start > unavail)
-		    mark_value_bytes_unavailable (val,
-						  (embedded_offset
-						   + unavail - memaddr),
-						  start - unavail);
-		  unavail = end;
-
-		  read_memory (start, buffer + start - memaddr, end - start);
-		}
-	    }
-
-	  if (unavail != memaddr + length)
-	    mark_value_bytes_unavailable (val,
-					  embedded_offset + unavail - memaddr,
-					  (memaddr + length) - unavail);
-
-	  do_cleanups (old_chain);
-	}
+      xfered += xfered_len;
+      QUIT;
     }
 }
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target
  2014-02-20 13:18   ` Pedro Alves
@ 2014-02-23  3:55     ` Yao Qi
  0 siblings, 0 replies; 23+ messages in thread
From: Yao Qi @ 2014-02-23  3:55 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 02/20/2014 09:18 PM, Pedro Alves wrote:
> Typo: s/is data/if data/
> 

Fixed.

>> > +
>> > +extern enum target_xfer_status
>> > +  section_table_xfer_available_memory (gdb_byte *readbuf, ULONGEST offset,
>> > +				       ULONGEST len, ULONGEST *xfered_len);
> The comment on function names I made on a previous patch applies here too.
> 
> This only reads, so instead:
> 
> extern enum target_xfer_status
>   section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
> 			               ULONGEST len, ULONGEST *xfered_len);
> 
> or:
> 
> extern enum target_xfer_status
>   section_table_read_partial_available_memory (gdb_byte *readbuf,
> 					       ULONGEST offset,
> 				               ULONGEST len,
> 					       ULONGEST *xfered_len);

I choose section_table_read_available_memory because it is shorter.
Patch below is pushed in.

-- 
Yao (齐尧)

gdb:

2014-02-23  Yao Qi  <yao@codesourcery.com>

	* exec.c (section_table_read_available_memory): New function.
	* exec.h (section_table_read_available_memory): Declare.
	* ctf.c (ctf_xfer_partial): Call
	section_table_read_available_memory.
	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
---
 gdb/ChangeLog         |    8 +++++++
 gdb/ctf.c             |    9 ++++++-
 gdb/exec.c            |   54 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/exec.h            |    8 +++++++
 gdb/tracefile-tfile.c |   10 +++++++-
 5 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 11b70b8..55745af 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2014-02-23  Yao Qi  <yao@codesourcery.com>
 
+	* exec.c (section_table_read_available_memory): New function.
+	* exec.h (section_table_read_available_memory): Declare.
+	* ctf.c (ctf_xfer_partial): Call
+	section_table_read_available_memory.
+	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
+
+2014-02-23  Yao Qi  <yao@codesourcery.com>
+
 	* ctf.c (ctf_xfer_partial): Move code to ...
 	* exec.c (exec_read_partial_read_only): ... it.  New function.
 	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
diff --git a/gdb/ctf.c b/gdb/ctf.c
index 9c8f4d7..95fd31f 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1465,9 +1465,14 @@ ctf_xfer_partial (struct target_ops *ops, enum target_object object,
 
       /* Restore the position.  */
       bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
-    }
 
-  return exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+      return exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+    }
+  else
+    {
+      /* Fallback to reading from read-only sections.  */
+      return section_table_read_available_memory (readbuf, offset, len, xfered_len);
+    }
 }
 
 /* This is the implementation of target_ops method
diff --git a/gdb/exec.c b/gdb/exec.c
index 74c61eb..607f5ac 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -615,6 +615,60 @@ section_table_available_memory (VEC(mem_range_s) *memory,
 }
 
 enum target_xfer_status
+section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
+				     ULONGEST len, ULONGEST *xfered_len)
+{
+  VEC(mem_range_s) *available_memory = NULL;
+  struct target_section_table *table;
+  struct cleanup *old_chain;
+  mem_range_s *r;
+  int i;
+
+  table = target_get_section_table (&exec_ops);
+  available_memory = section_table_available_memory (available_memory,
+						     offset, len,
+						     table->sections,
+						     table->sections_end);
+
+  old_chain = make_cleanup (VEC_cleanup(mem_range_s),
+			    &available_memory);
+
+  normalize_mem_ranges (available_memory);
+
+  for (i = 0;
+       VEC_iterate (mem_range_s, available_memory, i, r);
+       i++)
+    {
+      if (mem_ranges_overlap (r->start, r->length, offset, len))
+	{
+	  CORE_ADDR end;
+	  enum target_xfer_status status;
+
+	  /* Get the intersection window.  */
+	  end = min (offset + len, r->start + r->length);
+
+	  gdb_assert (end - offset <= len);
+
+	  if (offset >= r->start)
+	    status = exec_read_partial_read_only (readbuf, offset,
+						  end - offset,
+						  xfered_len);
+	  else
+	    {
+	      *xfered_len = r->start - offset;
+	      status = TARGET_XFER_E_UNAVAILABLE;
+	    }
+	  do_cleanups (old_chain);
+	  return status;
+	}
+    }
+  do_cleanups (old_chain);
+
+  *xfered_len = len;
+  return TARGET_XFER_E_UNAVAILABLE;
+}
+
+enum target_xfer_status
 section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf,
 				   ULONGEST offset, ULONGEST len,
 				   ULONGEST *xfered_len,
diff --git a/gdb/exec.h b/gdb/exec.h
index 960c585..84dc40f 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -91,6 +91,14 @@ extern enum target_xfer_status
 				     struct target_section *,
 				     const char *);
 
+/* Read from mappable read-only sections of BFD executable files.
+   Similar to exec_read_partial_read_only, but return
+   TARGET_XFER_E_UNAVAILABLE if data is unavailable.  */
+
+extern enum target_xfer_status
+  section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
+				       ULONGEST len, ULONGEST *xfered_len);
+
 /* Set the loaded address of a section.  */
 extern void exec_set_section_address (const char *, int, CORE_ADDR);
 
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 02122eb..cbf746d 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -936,9 +936,15 @@ tfile_xfer_partial (struct target_ops *ops, enum target_object object,
 	  /* Skip over this block.  */
 	  pos += (8 + 2 + mlen);
 	}
-    }
 
-  return exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+      return exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+    }
+  else
+    {
+      /* Fallback to reading from read-only sections.  */
+      return section_table_read_available_memory (readbuf, offset, len,
+						  xfered_len);
+    }
 }
 
 /* Iterate through the blocks of a trace frame, looking for a 'V'
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PUSHED] Remove TARGET_XFER_STATUS_ERROR_P (Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial)
  2014-02-23  3:54     ` Yao Qi
@ 2014-02-24  3:38       ` Yao Qi
  2014-02-24  6:40       ` Rename TARGET_XFER_E_UNAVAILABLE to TARGET_XFER_UNAVAILABLE " Yao Qi
  2014-02-24 11:16       ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Pedro Alves
  2 siblings, 0 replies; 23+ messages in thread
From: Yao Qi @ 2014-02-24  3:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 02/23/2014 11:52 AM, Yao Qi wrote:
>> I maintain that using TARGET_XFER_STATUS_ERROR_P tends to
>> > be unnecessary, and actually leads to fragile code.   It doesn't
>> > really help here.  A newer status added in the future that is
>> > not an error will pass by here unhandled.  Handle TARGET_XFER_OK
>> > explicitly instead, like this:
>> > 
>> >       if (status == TARGET_XFER_EOF)
>                       ^^^^^^^^^^^^^^^
> It should be TARGET_XFER_OK
>> > 	/* nothing */;
>> >       else if (status == TARGET_XFER_E_UNAVAILABLE)
>> > 	mark_value_bytes_unavailable (val, embedded_offset + xfered,
>> > 				      xfered_len);
>> >       else if (status == TARGET_XFER_EOF)
>> > 	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
>> >       else
>> > 	memory_error (status, memaddr + xfered);
>> > 
>> >       xfered += xfered_len;
>> >       QUIT;
>> >     }
>> > 
>> > and now all future unhandled statuses fall through to
>> > memory_error.
> OK, update patch as you suggested.  I'll get rid of
> TARGET_XFER_STATUS_ERROR_P in a follow-up patch.
> 

This patch removes macro TARGET_XFER_STATUS_ERROR_P, as Pedro pointed
out during patches review that TARGET_XFER_STATUS_ERROR_P tends to
be unnecessary.

Regression tested on x86_64-linux.

-- 
Yao (齐尧)

gdb:

2014-02-24  Yao Qi  <yao@codesourcery.com>

	* target.h (TARGET_XFER_STATUS_ERROR_P): Remove.
	* corefile.c (read_memory): Adjusted.
	* target.c (target_write_with_progress): Adjusted.
---
 gdb/ChangeLog  |    6 ++++++
 gdb/corefile.c |    9 +++------
 gdb/target.c   |    7 ++-----
 gdb/target.h   |    2 --
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3b199ac..01be0a3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-02-24  Yao Qi  <yao@codesourcery.com>
+
+	* target.h (TARGET_XFER_STATUS_ERROR_P): Remove.
+	* corefile.c (read_memory): Adjusted.
+	* target.c (target_write_with_progress): Adjusted.
+
 2014-02-23  Yao Qi  <yao@codesourcery.com>
 
 	Revert two patches:
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 048669b..815adaf 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -260,13 +260,10 @@ read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
 				    memaddr + xfered, len - xfered,
 				    &xfered_len);
 
-      if (status == TARGET_XFER_EOF)
-	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
+      if (status != TARGET_XFER_OK)
+	memory_error (status == TARGET_XFER_EOF ? TARGET_XFER_E_IO : status,
+		      memaddr + xfered);
 
-      if (TARGET_XFER_STATUS_ERROR_P (status))
-	memory_error (status, memaddr + xfered);
-
-      gdb_assert (status == TARGET_XFER_OK);
       xfered += xfered_len;
       QUIT;
     }
diff --git a/gdb/target.c b/gdb/target.c
index 0f3bd30..60a11dd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2002,12 +2002,9 @@ target_write_with_progress (struct target_ops *ops,
 				     offset + xfered, len - xfered,
 				     &xfered_len);
 
-      if (status == TARGET_XFER_EOF)
-	return xfered;
-      if (TARGET_XFER_STATUS_ERROR_P (status))
-	return -1;
+      if (status != TARGET_XFER_OK)
+	return status == TARGET_XFER_EOF ? xfered : -1;
 
-      gdb_assert (status == TARGET_XFER_OK);
       if (progress)
 	(*progress) (xfered_len, baton);
 
diff --git a/gdb/target.h b/gdb/target.h
index 6cc1337..4254609 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -225,8 +225,6 @@ enum target_xfer_status
   /* Keep list in sync with target_xfer_error_to_string.  */
 };
 
-#define TARGET_XFER_STATUS_ERROR_P(STATUS) ((STATUS) < TARGET_XFER_EOF)
-
 /* Return the string form of ERR.  */
 
 extern const char *target_xfer_status_to_string (enum target_xfer_status err);
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Rename TARGET_XFER_E_UNAVAILABLE to TARGET_XFER_UNAVAILABLE (Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial)
  2014-02-23  3:54     ` Yao Qi
  2014-02-24  3:38       ` [PUSHED] Remove TARGET_XFER_STATUS_ERROR_P (Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial) Yao Qi
@ 2014-02-24  6:40       ` Yao Qi
  2014-02-24 11:16       ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Pedro Alves
  2 siblings, 0 replies; 23+ messages in thread
From: Yao Qi @ 2014-02-24  6:40 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On 02/23/2014 11:52 AM, Yao Qi wrote:
>> (Hmm, the _E_ in TARGET_XFER_E_UNAVAILABLE is now starting
>> > to look a little odd and confusing to me.  E.g., it's not
>> > really an error in this case.  I'm pondering renaming it to
>> > TARGET_XFER_UNAVAILABLE.)
> It isn't an error.  Will rename it in a follow-up patch too.

Nowadays, TARGET_XFER_E_UNAVAILABLE isn't regarded as an error in
to_xfer_partial interface, so _E_ looks odd.  This patch is to
replace TARGET_XFER_E_UNAVAILABLE with TARGET_XFER_UNAVAILABLE,
and change its value from -2 to 2.  Since there is no comparison
on the value of 'enum target_xfer_status', so it should be safe.

Regression tested on x86_64-linux.  Patch is pushed in.

-- 
Yao (齐尧)

gdb:

2014-02-24  Yao Qi  <yao@codesourcery.com>

	* target.h (enum target_xfer_status)
	<TARGET_XFER_E_UNAVAILABLE>: Rename it to ...
	<TARGET_XFER_UNAVAILABLE>: ... it with setting value 2
	explicitly.  New.
	* corefile.c (memory_error_message): User updated.
	* exec.c (section_table_read_available_memory): Likewise.
	* record-btrace.c (record_btrace_xfer_partial): Likewise.
	* target.c (target_xfer_status_to_string): Likewise.
	(raw_memory_xfer_partial): Likewise.
	(memory_xfer_partial_1, target_xfer_partial): Likewise.
	* valops.c (read_value_memory): Likewise.
	* exec.h: Update comments.
---
 gdb/ChangeLog       |   15 +++++++++++++++
 gdb/corefile.c      |    4 ++--
 gdb/exec.c          |    4 ++--
 gdb/exec.h          |    2 +-
 gdb/record-btrace.c |    6 +++---
 gdb/target.c        |    8 ++++----
 gdb/target.h        |    9 ++++-----
 gdb/valops.c        |    2 +-
 8 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9778f7e..5fdbf4f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,20 @@
 2014-02-24  Yao Qi  <yao@codesourcery.com>
 
+	* target.h (enum target_xfer_status)
+	<TARGET_XFER_E_UNAVAILABLE>: Rename it to ...
+	<TARGET_XFER_UNAVAILABLE>: ... it with setting value 2
+	explicitly.  New.
+	* corefile.c (memory_error_message): User updated.
+	* exec.c (section_table_read_available_memory): Likewise.
+	* record-btrace.c (record_btrace_xfer_partial): Likewise.
+	* target.c (target_xfer_status_to_string): Likewise.
+	(raw_memory_xfer_partial): Likewise.
+	(memory_xfer_partial_1, target_xfer_partial): Likewise.
+	* valops.c (read_value_memory): Likewise.
+	* exec.h: Update comments.
+
+2014-02-24  Yao Qi  <yao@codesourcery.com>
+
 	* target.c (target_xfer_status_to_string): Rename argument err
 	to status.
 	* target.h (target_xfer_status_to_string): Update declaration.
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 815adaf..8a96d75 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -204,7 +204,7 @@ memory_error_message (enum target_xfer_status err,
 	 bounds.  */
       return xstrprintf (_("Cannot access memory at address %s"),
 			 paddress (gdbarch, memaddr));
-    case TARGET_XFER_E_UNAVAILABLE:
+    case TARGET_XFER_UNAVAILABLE:
       return xstrprintf (_("Memory at address %s unavailable."),
 			 paddress (gdbarch, memaddr));
     default:
@@ -233,7 +233,7 @@ memory_error (enum target_xfer_status err, CORE_ADDR memaddr)
     case TARGET_XFER_E_IO:
       exception = MEMORY_ERROR;
       break;
-    case TARGET_XFER_E_UNAVAILABLE:
+    case TARGET_XFER_UNAVAILABLE:
       exception = NOT_AVAILABLE_ERROR;
       break;
     }
diff --git a/gdb/exec.c b/gdb/exec.c
index 758e382..44dddc1 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -661,7 +661,7 @@ section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
 	  else
 	    {
 	      *xfered_len = r->start - offset;
-	      status = TARGET_XFER_E_UNAVAILABLE;
+	      status = TARGET_XFER_UNAVAILABLE;
 	    }
 	  do_cleanups (old_chain);
 	  return status;
@@ -670,7 +670,7 @@ section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
   do_cleanups (old_chain);
 
   *xfered_len = len;
-  return TARGET_XFER_E_UNAVAILABLE;
+  return TARGET_XFER_UNAVAILABLE;
 }
 
 enum target_xfer_status
diff --git a/gdb/exec.h b/gdb/exec.h
index 4d9de90..44f1367 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -82,7 +82,7 @@ extern enum target_xfer_status
 
 /* Read from mappable read-only sections of BFD executable files.
    Similar to exec_read_partial_read_only, but return
-   TARGET_XFER_E_UNAVAILABLE if data is unavailable.  */
+   TARGET_XFER_UNAVAILABLE if data is unavailable.  */
 
 extern enum target_xfer_status
   section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index c326c8d..05e7713 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -827,7 +827,7 @@ record_btrace_xfer_partial (struct target_ops *ops, enum target_object object,
 	    if (writebuf != NULL)
 	      {
 		*xfered_len = len;
-		return TARGET_XFER_E_UNAVAILABLE;
+		return TARGET_XFER_UNAVAILABLE;
 	      }
 
 	    /* We allow reading readonly memory.  */
@@ -846,7 +846,7 @@ record_btrace_xfer_partial (struct target_ops *ops, enum target_object object,
 	      }
 
 	    *xfered_len = len;
-	    return TARGET_XFER_E_UNAVAILABLE;
+	    return TARGET_XFER_UNAVAILABLE;
 	  }
 	}
     }
@@ -858,7 +858,7 @@ record_btrace_xfer_partial (struct target_ops *ops, enum target_object object,
 				   offset, len, xfered_len);
 
   *xfered_len = len;
-  return TARGET_XFER_E_UNAVAILABLE;
+  return TARGET_XFER_UNAVAILABLE;
 }
 
 /* The to_insert_breakpoint method of target record-btrace.  */
diff --git a/gdb/target.c b/gdb/target.c
index 25f1cf7..e4bf2e9 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -886,7 +886,7 @@ target_xfer_status_to_string (enum target_xfer_status status)
   switch (status)
     {
       CASE(TARGET_XFER_E_IO);
-      CASE(TARGET_XFER_E_UNAVAILABLE);
+      CASE(TARGET_XFER_UNAVAILABLE);
     default:
       return "<unknown>";
     }
@@ -1099,7 +1099,7 @@ raw_memory_xfer_partial (struct target_ops *ops, gdb_byte *readbuf,
 	break;
 
       /* Stop if the target reports that the memory is not available.  */
-      if (res == TARGET_XFER_E_UNAVAILABLE)
+      if (res == TARGET_XFER_UNAVAILABLE)
 	break;
 
       /* We want to continue past core files to executables, but not
@@ -1212,7 +1212,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 		  /* No use trying further, we know some memory starting
 		     at MEMADDR isn't available.  */
 		  *xfered_len = len;
-		  return TARGET_XFER_E_UNAVAILABLE;
+		  return TARGET_XFER_UNAVAILABLE;
 		}
 	    }
 
@@ -1479,7 +1479,7 @@ target_xfer_partial (struct target_ops *ops,
   /* Check implementations of to_xfer_partial update *XFERED_LEN
      properly.  Do assertion after printing debug messages, so that we
      can find more clues on assertion failure from debugging messages.  */
-  if (retval == TARGET_XFER_OK || retval == TARGET_XFER_E_UNAVAILABLE)
+  if (retval == TARGET_XFER_OK || retval == TARGET_XFER_UNAVAILABLE)
     gdb_assert (*xfered_len > 0);
 
   return retval;
diff --git a/gdb/target.h b/gdb/target.h
index 9143ee2..db248a8 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -213,15 +213,14 @@ enum target_xfer_status
   /* No further transfer is possible.  */
   TARGET_XFER_EOF = 0,
 
+  /* The piece of the object requested is unavailable.  */
+  TARGET_XFER_UNAVAILABLE = 2,
+
   /* Generic I/O error.  Note that it's important that this is '-1',
      as we still have target_xfer-related code returning hardcoded
      '-1' on error.  */
   TARGET_XFER_E_IO = -1,
 
-  /* Transfer failed because the piece of the object requested is
-     unavailable.  */
-  TARGET_XFER_E_UNAVAILABLE = -2,
-
   /* Keep list in sync with target_xfer_status_to_string.  */
 };
 
@@ -628,7 +627,7 @@ struct target_ops
        'enum target_xfer_status' value).  Save the number of bytes
        actually transferred in *XFERED_LEN if transfer is successful
        (TARGET_XFER_OK) or the number unavailable bytes if the requested
-       data is unavailable (TARGET_XFER_E_UNAVAILABLE).  *XFERED_LEN
+       data is unavailable (TARGET_XFER_UNAVAILABLE).  *XFERED_LEN
        smaller than LEN does not indicate the end of the object, only
        the end of the transfer; higher level code should continue
        transferring if desired.  This is handled in target.c.
diff --git a/gdb/valops.c b/gdb/valops.c
index 0d726d0..82417dac 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -964,7 +964,7 @@ read_value_memory (struct value *val, int embedded_offset,
 
       if (status == TARGET_XFER_OK)
 	/* nothing */;
-      else if (status == TARGET_XFER_E_UNAVAILABLE)
+      else if (status == TARGET_XFER_UNAVAILABLE)
 	mark_value_bytes_unavailable (val, embedded_offset + xfered,
 				      xfered_len);
       else if (status == TARGET_XFER_EOF)
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
  2014-02-23  3:54     ` Yao Qi
  2014-02-24  3:38       ` [PUSHED] Remove TARGET_XFER_STATUS_ERROR_P (Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial) Yao Qi
  2014-02-24  6:40       ` Rename TARGET_XFER_E_UNAVAILABLE to TARGET_XFER_UNAVAILABLE " Yao Qi
@ 2014-02-24 11:16       ` Pedro Alves
  2 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2014-02-24 11:16 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 02/23/2014 03:52 AM, Yao Qi wrote:

>> 	/* nothing */;
>>       else if (status == TARGET_XFER_E_UNAVAILABLE)
>> 	mark_value_bytes_unavailable (val, embedded_offset + xfered,
>> 				      xfered_len);
>>       else if (status == TARGET_XFER_EOF)
>> 	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
>>       else
>> 	memory_error (status, memaddr + xfered);
>>
>>       xfered += xfered_len;
>>       QUIT;
>>     }
>>
>> and now all future unhandled statuses fall through to
>> memory_error.
> 
> OK, update patch as you suggested.  I'll get rid of
> TARGET_XFER_STATUS_ERROR_P in a follow-up patch.

Great, thanks.

>> (Hmm, the _E_ in TARGET_XFER_E_UNAVAILABLE is now starting
>> to look a little odd and confusing to me.  E.g., it's not
>> really an error in this case.  I'm pondering renaming it to
>> TARGET_XFER_UNAVAILABLE.)
> 
> It isn't an error.  Will rename it in a follow-up patch too.

Thank you!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-02-24 11:16 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-12  6:07 [PATCH 0/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
2014-02-12  6:08 ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Yao Qi
2014-02-20 14:06   ` Pedro Alves
2014-02-23  3:54     ` Yao Qi
2014-02-24  3:38       ` [PUSHED] Remove TARGET_XFER_STATUS_ERROR_P (Re: [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial) Yao Qi
2014-02-24  6:40       ` Rename TARGET_XFER_E_UNAVAILABLE to TARGET_XFER_UNAVAILABLE " Yao Qi
2014-02-24 11:16       ` [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial Pedro Alves
2014-02-12  6:08 ` [PATCH 2/8] Move tfile target to tracefile-tfile.c Yao Qi
2014-02-20 13:14   ` Pedro Alves
2014-02-12  6:08 ` [PATCH 4/8] Let tracefile has_memory and has_all_memory Yao Qi
2014-02-20 13:15   ` Pedro Alves
2014-02-12  6:08 ` [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target Yao Qi
2014-02-20 13:17   ` Pedro Alves
2014-02-12  6:08 ` [PATCH 8/8] Call target_traceframe_info when traceframe is selected Yao Qi
2014-02-20 13:30   ` Pedro Alves
2014-02-21  8:22     ` Yao Qi
2014-02-12  6:08 ` [PATCH 1/8] Move trace file writer out of tracepoint.c Yao Qi
2014-02-20 13:14   ` Pedro Alves
2014-02-12  6:08 ` [PATCH 6/8] Use new to_xfer_partial interface in ctf and tfile target Yao Qi
2014-02-20 13:18   ` Pedro Alves
2014-02-23  3:55     ` Yao Qi
2014-02-12  6:08 ` [PATCH 3/8] Share some code between " Yao Qi
2014-02-20 13:15   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).