[PATCH 2/4] gdb.trace: Read XML target description from tfile.

[PATCH 2/4] gdb.trace: Read XML target description from tfile.

[Marcin Kościelnicki]

gdb/ChangeLog:

	* tracefile-tfile.c (trace_tdesc): New static variable.
	(trace_tdesc_alloc): New static variable.
	(trace_tdesc_len): New static variable.
	(tfile_open): Clear trace_tdesc, call target_find_description.
	(tfile_interp_line): Recognize tdesc lines.
	(tfile_close): Clear trace_tdesc.
	(tfile_xfer_partial_features): New function.
	(tfile_xfer_partial): Call tfile_xfer_partial_features.
	(tfile_append_tdesc_line): New function.
---
 gdb/ChangeLog         | 12 ++++++++
 gdb/tracefile-tfile.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 29e1291..f5cd0c7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* tracefile-tfile.c (trace_tdesc): New static variable.
+	(trace_tdesc_alloc): New static variable.
+	(trace_tdesc_len): New static variable.
+	(tfile_open): Clear trace_tdesc, call target_find_description.
+	(tfile_interp_line): Recognize tdesc lines.
+	(tfile_close): Clear trace_tdesc.
+	(tfile_xfer_partial_features): New function.
+	(tfile_xfer_partial): Call tfile_xfer_partial_features.
+	(tfile_append_tdesc_line): New function.
+
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* ctf.c (ctf_write_tdesc): New function.
 	(ctf_write_ops): Wire in ctf_write_tdesc.
 	* tracefile-tfile.c (tfile_write_tdesc): New function.
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index f148758..71ac437 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -29,6 +29,7 @@
 #include "completer.h"
 #include "filenames.h"
 #include "xml-tdesc.h"
+#include "target-descriptions.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -390,7 +391,11 @@ static off_t trace_frames_offset;
 static off_t cur_offset;
 static int cur_data_size;
 int trace_regblock_size;
+static char *trace_tdesc;
+static int trace_tdesc_alloc;
+static int trace_tdesc_len;
 
+static void tfile_append_tdesc_line (const char *line);
 static void tfile_interp_line (char *line,
 			       struct uploaded_tp **utpp,
 			       struct uploaded_tsv **utsvp);
@@ -457,6 +462,12 @@ tfile_open (const char *arg, int from_tty)
   trace_filename = xstrdup (filename);
   trace_fd = scratch_chan;
 
+  /* Make sure this is clear.  */
+  xfree (trace_tdesc);
+  trace_tdesc = NULL;
+  trace_tdesc_alloc = 0;
+  trace_tdesc_len = 0;
+
   bytes = 0;
   /* Read the file header and test for validity.  */
   tfile_read ((gdb_byte *) &header, TRACE_HEADER_SIZE);
@@ -505,6 +516,9 @@ tfile_open (const char *arg, int from_tty)
 	    error (_("Excessively long lines in trace file"));
 	}
 
+      /* We should have fetched tdesc by now.  */
+      target_find_description ();
+
       /* Record the starting offset of the binary trace data.  */
       trace_frames_offset = bytes;
 
@@ -568,6 +582,11 @@ tfile_interp_line (char *line, struct uploaded_tp **utpp,
       p += strlen ("tsv ");
       parse_tsv_definition (p, utsvp);
     }
+  else if (startswith (p, "tdesc "))
+    {
+      p += strlen ("tdesc ");
+      tfile_append_tdesc_line (p);
+    }
   else
     warning (_("Ignoring trace file definition \"%s\""), line);
 }
@@ -590,6 +609,10 @@ tfile_close (struct target_ops *self)
   trace_fd = -1;
   xfree (trace_filename);
   trace_filename = NULL;
+  xfree (trace_tdesc);
+  trace_tdesc = NULL;
+  trace_tdesc_alloc = 0;
+  trace_tdesc_len = 0;
 
   trace_reset_local_state ();
 }
@@ -876,12 +899,42 @@ tfile_fetch_registers (struct target_ops *ops,
 }
 
 static enum target_xfer_status
+tfile_xfer_partial_features (struct target_ops *ops, const char *annex,
+			     gdb_byte *readbuf, const gdb_byte *writebuf,
+			     ULONGEST offset, ULONGEST len,
+			     ULONGEST *xfered_len)
+{
+  if (strcmp (annex, "target.xml"))
+    return TARGET_XFER_E_IO;
+
+  if (readbuf == NULL)
+    error (_("tfile_xfer_partial: tdesc is read-only"));
+
+  if (!trace_tdesc)
+    return TARGET_XFER_E_IO;
+
+  if (offset >= trace_tdesc_len)
+    return TARGET_XFER_EOF;
+
+  if (len > trace_tdesc_len - offset)
+    len = trace_tdesc_len - offset;
+
+  memcpy (readbuf, trace_tdesc + offset, len);
+  *xfered_len = len;
+
+  return TARGET_XFER_OK;
+}
+
+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.  */
+  /* We're only doing regular memory and tdesc for now.  */
+  if (object == TARGET_OBJECT_AVAILABLE_FEATURES)
+    return tfile_xfer_partial_features (ops, annex, readbuf, writebuf,
+					offset, len, xfered_len);
   if (object != TARGET_OBJECT_MEMORY)
     return TARGET_XFER_E_IO;
 
@@ -1061,6 +1114,31 @@ tfile_traceframe_info (struct target_ops *self)
   return info;
 }
 
+/* Handles tdesc lines from tfile by appending the payload to
+   a global trace_tdesc variable.  */
+
+static void
+tfile_append_tdesc_line (const char *line)
+{
+  int llen = strlen (line);
+
+  /* 2 chars for "\n\0".  */
+  while (trace_tdesc_len + llen + 2 > trace_tdesc_alloc)
+    {
+      /* Grow the buffer.  */
+      if (!trace_tdesc_alloc)
+	trace_tdesc_alloc = 4096;
+      else
+	trace_tdesc_alloc *= 2;
+      trace_tdesc = xrealloc(trace_tdesc, trace_tdesc_alloc);
+    }
+
+  strcpy (trace_tdesc + trace_tdesc_len, line);
+  trace_tdesc_len += llen;
+  strcpy (trace_tdesc + trace_tdesc_len, "\n");
+  trace_tdesc_len++;
+}
+
 static void
 init_tfile_ops (void)
 {
-- 
2.7.0

[PATCH 0/4] Save target description in tfile.

[Marcin Kościelnicki]

As discussed in the s390 trace thread, this adds target description
in XML format to tfile format.  The idea is simple:

1. target.xml is read from the target.
2. Includes are processed, resulting in a single in-memory XML file
   containing all the data.
3. The resulting file is stored in tfile header by prefixing every line
   with "tdesc ".  We may insert a spurious newline at the end of file
   with this encoding, but that won't matter for XML.
4. When tfile is read, the XML is stored in an allocated buffer, and
   xfer for TARGET_OBJECT_AVAILABLE_FEATURES is implemented, reading
   from it.
5. target_find_description is called to force reading it.

Patch #1 makes tsave write the XML data into tfile.  Patch #2 makes
tfile reader fetch and parse this data.  Patch #3 fixes wrong register
ordering in tfile_fetch_registers (noticed on x86_64 with AVX -
plain x86_64 was apparently fine).  Patch #4 fixes an off-by-one
in the same function that prevented fetching the last register
(orig_rax for plain x86_64, $ymm15h for x86_64 with AVX).

[PATCH 1/4] gdb.trace: Save XML target description in tfile.

[Marcin Kościelnicki]

gdb/ChangeLog:

	* ctf.c (ctf_write_tdesc): New function.
	(ctf_write_ops): Wire in ctf_write_tdesc.
	* tracefile-tfile.c (tfile_write_tdesc): New function.
	(tfile_write_ops): Wire in tfile_write_tdesc.
	* tracefile.c (trace_save): Call write_tdesc method.
	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
	* xml-tdesc.c (target_fetch_description_xml): New function.
	* xml-tdesc.h: Add target_fetch_description_xml prototype.
---
 gdb/ChangeLog         | 11 +++++++++++
 gdb/ctf.c             | 10 ++++++++++
 gdb/tracefile-tfile.c | 38 ++++++++++++++++++++++++++++++++++++++
 gdb/tracefile.c       |  3 +++
 gdb/tracefile.h       |  3 +++
 gdb/xml-tdesc.c       | 26 ++++++++++++++++++++++++++
 gdb/xml-tdesc.h       |  6 ++++++
 7 files changed, 97 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a2b0d39..29e1291 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* ctf.c (ctf_write_tdesc): New function.
+	(ctf_write_ops): Wire in ctf_write_tdesc.
+	* tracefile-tfile.c (tfile_write_tdesc): New function.
+	(tfile_write_ops): Wire in tfile_write_tdesc.
+	* tracefile.c (trace_save): Call write_tdesc method.
+	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
+	* xml-tdesc.c (target_fetch_description_xml): New function.
+	* xml-tdesc.h: Add target_fetch_description_xml prototype.
+
 2016-02-04  Yao Qi  <yao.qi@linaro.org>
 
 	* remote.c (remote_wait_as): Set rs->waiting_for_stop_reply to
diff --git a/gdb/ctf.c b/gdb/ctf.c
index 9d496e3..25a4c79 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -617,6 +617,15 @@ ctf_write_uploaded_tp (struct trace_file_writer *self,
 }
 
 /* This is the implementation of trace_file_write_ops method
+   write_tdesc.  */
+
+static void
+ctf_write_tdesc (struct trace_file_writer *self)
+{
+  /* Nothing so far. */
+}
+
+/* This is the implementation of trace_file_write_ops method
    write_definition_end.  */
 
 static void
@@ -799,6 +808,7 @@ static const struct trace_file_write_ops ctf_write_ops =
   ctf_write_status,
   ctf_write_uploaded_tsv,
   ctf_write_uploaded_tp,
+  ctf_write_tdesc,
   ctf_write_definition_end,
   NULL,
   &ctf_write_frame_ops,
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index b761894..f148758 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -28,6 +28,7 @@
 #include "exec.h" /* exec_bfd */
 #include "completer.h"
 #include "filenames.h"
+#include "xml-tdesc.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -263,6 +264,42 @@ tfile_write_uploaded_tp (struct trace_file_writer *self,
 }
 
 /* This is the implementation of trace_file_write_ops method
+   write_tdesc.  */
+
+static void
+tfile_write_tdesc (struct trace_file_writer *self)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+  char *tdesc = target_fetch_description_xml (&current_target);
+  char *ptr = tdesc;
+  char *next;
+
+  if (!tdesc)
+    return;
+
+  /* Write tdesc line by line, prefixing each line with "tdesc ".  */
+  while (ptr)
+    {
+      next = strchr (ptr, '\n');
+      if (next)
+	{
+	  fprintf (writer->fp, "tdesc %.*s\n", (int) (next-ptr), ptr);
+	  /* Skip the \n.  */
+	  next++;
+	}
+      else if (*ptr)
+	{
+	  /* Last line, doesn't have a newline.  */
+	  fprintf (writer->fp, "tdesc %s\n", ptr);
+	}
+      ptr = next;
+    }
+
+  xfree (tdesc);
+}
+
+/* This is the implementation of trace_file_write_ops method
    write_definition_end.  */
 
 static void
@@ -315,6 +352,7 @@ static const struct trace_file_write_ops tfile_write_ops =
   tfile_write_status,
   tfile_write_uploaded_tsv,
   tfile_write_uploaded_tp,
+  tfile_write_tdesc,
   tfile_write_definition_end,
   tfile_write_raw_data,
   NULL,
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index fef4ed9..de42165 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -90,6 +90,9 @@ trace_save (const char *filename, struct trace_file_writer *writer,
   /* Write out the size of a register block.  */
   writer->ops->write_regblock_type (writer, trace_regblock_size);
 
+  /* Write out the target description info.  */
+  writer->ops->write_tdesc (writer);
+
   /* Write out status of the tracing run (aka "tstatus" info).  */
   writer->ops->write_status (writer, ts);
 
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
index 8b711a1..e6d4460 100644
--- a/gdb/tracefile.h
+++ b/gdb/tracefile.h
@@ -84,6 +84,9 @@ struct trace_file_write_ops
   void (*write_uploaded_tp) (struct trace_file_writer *self,
 			     struct uploaded_tp *tp);
 
+  /* Write target description.  */
+  void (*write_tdesc) (struct trace_file_writer *self);
+
   /* Write to mark the end of the definition part.  */
   void (*write_definition_end) (struct trace_file_writer *self);
 
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 5eeda86..202de86 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -630,3 +630,29 @@ target_read_description_xml (struct target_ops *ops)
 
   return tdesc;
 }
+
+char *
+target_fetch_description_xml (struct target_ops *ops)
+{
+  struct target_desc *tdesc;
+  char *tdesc_str;
+  char *expanded_text;
+  struct cleanup *back_to;
+
+  tdesc_str = fetch_available_features_from_target ("target.xml", ops);
+  if (tdesc_str == NULL)
+    return NULL;
+
+  back_to = make_cleanup (xfree, tdesc_str);
+  expanded_text = xml_process_xincludes (_("target description"),
+					 tdesc_str,
+					 fetch_available_features_from_target, ops, 0);
+  if (expanded_text == NULL)
+    {
+      warning (_("Could not load XML target description; ignoring"));
+      return NULL;
+    }
+  do_cleanups (back_to);
+
+  return expanded_text;
+}
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index a0c38d7..38bb99e 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -31,3 +31,9 @@ const struct target_desc *file_read_description_xml (const char *filename);
    parsed description.  */
 
 const struct target_desc *target_read_description_xml (struct target_ops *);
+
+/* Fetches an XML target description using OPS,  processing
+   includes, but not parsing it.  Used to dump whole tdesc
+   as a single XML file.  */
+
+char * target_fetch_description_xml (struct target_ops *ops);
-- 
2.7.0

[PATCH 3/4] gdb.trace: Use g packet order in tfile_fetch_registers.

[Marcin Kościelnicki]

tfile_fetch_registers currently wrongly fetches registers using
gdb order instead of g packet order.  On x86_64 with AVX, this causes
problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
g packet has orig_rax first.

gdb/ChangeLog:

	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
	instead of gdb order.
---
 gdb/ChangeLog         |  5 +++++
 gdb/tracefile-tfile.c | 11 ++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5cd0c7..5afdb60 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
+	instead of gdb order.
+
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* tracefile-tfile.c (trace_tdesc): New static variable.
 	(trace_tdesc_alloc): New static variable.
 	(trace_tdesc_len): New static variable.
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 71ac437..23d78c3 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -30,6 +30,7 @@
 #include "filenames.h"
 #include "xml-tdesc.h"
 #include "target-descriptions.h"
+#include "remote.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -857,7 +858,7 @@ tfile_fetch_registers (struct target_ops *ops,
 		       struct regcache *regcache, int regno)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  int offset, regn, regsize;
+  int offset, regn, regsize, dummy;
 
   /* An uninitialized reg size says we're not going to be
      successful at getting register blocks.  */
@@ -870,11 +871,12 @@ tfile_fetch_registers (struct target_ops *ops,
 
       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++)
 	{
+	  if (!remote_register_number_and_offset (get_regcache_arch (regcache),
+						  regn, &dummy, &offset))
+	    continue;
+
 	  regsize = register_size (gdbarch, regn);
 	  /* Make sure we stay within block bounds.  */
 	  if (offset + regsize >= trace_regblock_size)
@@ -891,7 +893,6 @@ tfile_fetch_registers (struct target_ops *ops,
 		  regcache_raw_supply (regcache, regn, regs + offset);
 		}
 	    }
-	  offset += regsize;
 	}
     }
   else
-- 
2.7.0

[PATCH 4/4] gdb.trace: Fix off-by-one in tfile_fetch_registers.

[Marcin Kościelnicki]

This resulted in the last register being considered unavailable.
On plain x86_64 (without AVX), this happened to be orig_rax.

gdb/ChangeLog:

	* tracefile-tfile.c (tfile_fetch_registers): Fix off-by-one in bounds
	check.
---
 gdb/ChangeLog         | 5 +++++
 gdb/tracefile-tfile.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5afdb60..339f2d0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* tracefile-tfile.c (tfile_fetch_registers): Fix off-by-one in bounds
+	check.
+
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
 	instead of gdb order.
 
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 23d78c3..a6c3c9c 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -879,7 +879,7 @@ tfile_fetch_registers (struct target_ops *ops,
 
 	  regsize = register_size (gdbarch, regn);
 	  /* Make sure we stay within block bounds.  */
-	  if (offset + regsize >= trace_regblock_size)
+	  if (offset + regsize > trace_regblock_size)
 	    break;
 	  if (regcache_register_status (regcache, regn) == REG_UNKNOWN)
 	    {
-- 
2.7.0

[PATCH 6/6] gdb.trace: Add a testcase for tdesc in tfile.

[Marcin Kościelnicki]

This tests whether $ymm15 can be correctly collected and printed from
tfile.  It covers:

- storing tdesc in tfile (without that, $ymm15 doesn't exist)
- ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
  collected)
- register order in tfile_fetch_registers (without that, $ymm15h is
  fetched from wrong position)
- off-by-one in tfile_fetch_registers (without that, $ymm15h is
  incorrectly considered to be out of bounds)
- using proper tdesc in encoding tracepoint actions (without that,
  internal error happens due to $ymm15h being considered unavailable)

gdb/testsuite/ChangeLog:

	* gdb.trace/tfile-avx.c: New test.
	* gdb.trace/tfile-avx.exp: New test.
---
This test requires https://sourceware.org/ml/gdb-patches/2016-01/msg00115.html
and all the previous 5 patches in this series.

 gdb/testsuite/ChangeLog               |  5 +++
 gdb/testsuite/gdb.trace/tfile-avx.c   | 48 ++++++++++++++++++++
 gdb/testsuite/gdb.trace/tfile-avx.exp | 82 +++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.c
 create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.exp

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 046f112..c9a596c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdb.trace/tfile-avx.c: New test.
+	* gdb.trace/tfile-avx.exp: New test.
+
 2016-02-04  Yao Qi  <yao.qi@linaro.org>
 
 	* gdb.base/foll-exec-mode.c: Include limits.h.
diff --git a/gdb/testsuite/gdb.trace/tfile-avx.c b/gdb/testsuite/gdb.trace/tfile-avx.c
new file mode 100644
index 0000000..71107b8
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/tfile-avx.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   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/>.  */
+
+/*
+ * Test program for reading target description from tfile: collects AVX
+ * registers on x86_64.
+ */
+
+#include <immintrin.h>
+
+void
+dummy (void)
+{}
+
+static void
+end (void)
+{}
+
+int main (void)
+{
+  register __v8si a asm("ymm15") = {
+    0x12340001,
+    0x12340002,
+    0x12340003,
+    0x12340004,
+    0x12340005,
+    0x12340006,
+    0x12340007,
+    0x12340008,
+  };
+  asm volatile ("traceme: call dummy" : : "x" (a));
+  end ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/tfile-avx.exp b/gdb/testsuite/gdb.trace/tfile-avx.exp
new file mode 100644
index 0000000..1acf4cb
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/tfile-avx.exp
@@ -0,0 +1,82 @@
+# Copyright 2016 Free Software Foundation, Inc.
+# 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/>.
+
+if { ! [istarget "x86_64*-*-*linux*"] } {
+    verbose "Skipping tfile AVX test (target is not x86_64 linux)."
+    return
+}
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile \
+     [list debug nowarnings additional_flags=-mavx]]} {
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1
+}
+
+send_gdb "print \$ymm15\n"
+gdb_expect {
+    -re "\\$\[0-9\]+ = void.*$gdb_prompt \$" {
+	verbose "Skipping tfile AVX test (target doesn't support AVX)."
+	return
+    }
+    -re "\\$\[0-9\]+ = \\{.*}.*$gdb_prompt \$" {
+	# All is well.
+    }
+}
+
+gdb_test "trace traceme" ".*"
+
+gdb_trace_setactions "set actions for tracepoint" "" \
+	"collect \$ymm15" "^$"
+
+gdb_breakpoint "end"
+
+gdb_test_no_output "tstart"
+
+gdb_test "continue" ".*Breakpoint \[0-9\]+, end .*"
+
+set tracefile [standard_output_file ${testfile}]
+
+# Save trace frames to tfile.
+gdb_test "tsave ${tracefile}.tf" \
+    "Trace data saved to file '${tracefile}.tf'.*" \
+    "save tfile trace file"
+
+# Change target to tfile.
+set test "change to tfile target"
+gdb_test_multiple "target tfile ${tracefile}.tf" "$test" {
+    -re "A program is being debugged already.  Kill it. .y or n. " {
+	send_gdb "y\n"
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	pass "$test"
+    }
+}
+
+gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
+
+gdb_test "print/x \$ymm15.v8_int32" "\\$\[0-9\]+ = \\{0x12340001, .*, 0x12340008}.*"
-- 
2.7.0

[PATCH 5/6] gdb/x86: Implement ax_pseudo_register_collect hook.

[Marcin Kościelnicki]

Makes "collect $ymm15" action work.

gdb/ChangeLog:

	* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
	(amd64_init_abi): Fill ax_pseudo_register_collect hook.
	* i386-tdep.c (i386_ax_pseudo_register_collect): New function.
	(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
	* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
---
 gdb/ChangeLog    |  8 ++++++
 gdb/amd64-tdep.c | 26 ++++++++++++++++++
 gdb/i386-tdep.c  | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/i386-tdep.h  |  4 +++
 4 files changed, 119 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 339f2d0..b3fda06 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
+	(amd64_init_abi): Fill ax_pseudo_register_collect hook.
+	* i386-tdep.c (i386_ax_pseudo_register_collect): New function.
+	(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
+	* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
+
+2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* tracefile-tfile.c (tfile_fetch_registers): Fix off-by-one in bounds
 	check.
 
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index fae92b2..d959ac2 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -455,6 +455,30 @@ amd64_pseudo_register_write (struct gdbarch *gdbarch,
     i386_pseudo_register_write (gdbarch, regcache, regnum, buf);
 }
 
+static int
+amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+				  struct agent_expr *ax, int regnum)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  if (i386_byte_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->al_regnum;
+      if (gpnum >= AMD64_NUM_LOWER_BYTE_REGS)
+	ax_reg_mask (ax, gpnum - AMD64_NUM_LOWER_BYTE_REGS);
+      else
+	ax_reg_mask (ax, gpnum);
+      return 0;
+    }
+  else if (i386_dword_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->eax_regnum;
+      ax_reg_mask (ax, gpnum);
+      return 0;
+    }
+  else
+    return i386_ax_pseudo_register_collect (gdbarch, ax, regnum);
+}
+
 \f
 
 /* Register classes as defined in the psABI.  */
@@ -2997,6 +3021,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 					  amd64_pseudo_register_read_value);
   set_gdbarch_pseudo_register_write (gdbarch,
 				     amd64_pseudo_register_write);
+  set_gdbarch_ax_pseudo_register_collect (gdbarch,
+					  amd64_ax_pseudo_register_collect);
 
   set_tdesc_pseudo_register_name (gdbarch, amd64_pseudo_register_name);
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b706463..934c3ab 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3603,6 +3603,85 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	internal_error (__FILE__, __LINE__, _("invalid regnum"));
     }
 }
+
+int
+i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+				 struct agent_expr *ax, int regnum)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  if (i386_mmx_regnum_p (gdbarch, regnum))
+    {
+      /* MMX to FPU register mapping depends on current TOS.  Let's just
+	 not care and collect everything...  */
+      int i;
+      ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
+      for (i = 0; i < 8; i++)
+	ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
+      return 0;
+    }
+  else if (i386_bnd_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->bnd0_regnum;
+      ax_reg_mask (ax, I387_BND0R_REGNUM (tdep) + regnum);
+      return 0;
+    }
+  else if (i386_k_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->k0_regnum;
+      ax_reg_mask (ax, tdep->k0_regnum + regnum);
+      return 0;
+    }
+  else if (i386_zmm_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->zmm0_regnum;
+      if (regnum < num_lower_zmm_regs)
+	{
+	  ax_reg_mask (ax, I387_XMM0_REGNUM (tdep) + regnum);
+	  ax_reg_mask (ax, tdep->ymm0h_regnum + regnum);
+	}
+      else
+	{
+	  ax_reg_mask (ax, I387_XMM16_REGNUM (tdep) + regnum
+			   - num_lower_zmm_regs);
+	  ax_reg_mask (ax, I387_YMM16H_REGNUM (tdep) + regnum
+			   - num_lower_zmm_regs);
+	}
+      ax_reg_mask (ax, tdep->zmm0h_regnum + regnum);
+      return 0;
+    }
+  else if (i386_ymm_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->ymm0_regnum;
+      ax_reg_mask (ax, I387_XMM0_REGNUM (tdep) + regnum);
+      ax_reg_mask (ax, tdep->ymm0h_regnum + regnum);
+      return 0;
+    }
+  else if (i386_ymm_avx512_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->ymm16_regnum;
+      ax_reg_mask (ax, I387_XMM16_REGNUM (tdep) + regnum);
+      ax_reg_mask (ax, tdep->ymm16h_regnum + regnum);
+      return 0;
+    }
+  else if (i386_word_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->ax_regnum;
+      ax_reg_mask (ax, gpnum);
+      return 0;
+    }
+  else if (i386_byte_regnum_p (gdbarch, regnum))
+    {
+      /* Check byte pseudo registers last since this function will
+	 be called from amd64_ax_pseudo_register_collect, which handles
+	 byte pseudo registers differently.  */
+      int gpnum = regnum - tdep->al_regnum;
+      ax_reg_mask (ax, gpnum % 4);
+      return 0;
+    }
+  else
+    internal_error (__FILE__, __LINE__, _("invalid regnum"));
+  return 1;
+}
 \f
 
 /* Return the register number of the register allocated by GCC after
@@ -8423,6 +8502,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_pseudo_register_read_value (gdbarch,
 					  i386_pseudo_register_read_value);
   set_gdbarch_pseudo_register_write (gdbarch, i386_pseudo_register_write);
+  set_gdbarch_ax_pseudo_register_collect (gdbarch,
+					  i386_ax_pseudo_register_collect);
 
   set_tdesc_pseudo_register_type (gdbarch, i386_pseudo_register_type);
   set_tdesc_pseudo_register_name (gdbarch, i386_pseudo_register_name);
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 10d2772..770f59d 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -360,6 +360,10 @@ extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
 					struct regcache *regcache,
 					int regnum, const gdb_byte *buf);
 
+extern int i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+					    struct agent_expr *ax,
+					    int regnum);
+
 /* Segment selectors.  */
 #define I386_SEL_RPL	0x0003  /* Requester's Privilege Level mask.  */
 #define I386_SEL_UPL	0x0003	/* User Privilige Level.  */
-- 
2.7.0

Re: [PATCH 2/4] gdb.trace: Read XML target description from tfile.

[Pedro Alves]

On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:

>  
> +static void tfile_append_tdesc_line (const char *line);
>  static void tfile_interp_line (char *line,
>  			       struct uploaded_tp **utpp,
>  			       struct uploaded_tsv **utsvp);
> @@ -457,6 +462,12 @@ tfile_open (const char *arg, int from_tty)
>    trace_filename = xstrdup (filename);
>    trace_fd = scratch_chan;
>  
> +  /* Make sure this is clear.  */
> +  xfree (trace_tdesc);
> +  trace_tdesc = NULL;
> +  trace_tdesc_alloc = 0;
> +  trace_tdesc_len = 0;
> +
>    bytes = 0;
>    /* Read the file header and test for validity.  */
>    tfile_read ((gdb_byte *) &header, TRACE_HEADER_SIZE);
> @@ -505,6 +516,9 @@ tfile_open (const char *arg, int from_tty)
>  	    error (_("Excessively long lines in trace file"));
>  	}
>  
> +      /* We should have fetched tdesc by now.  */
> +      target_find_description ();

The comment makes it sounds like the call isn't needed.  ITYM
that the loop above should have read in the tdesc, if there was one,
right?  Could you reword the comment, please?

>  
>  static enum target_xfer_status
> +tfile_xfer_partial_features (struct target_ops *ops, const char *annex,
> +			     gdb_byte *readbuf, const gdb_byte *writebuf,
> +			     ULONGEST offset, ULONGEST len,
> +			     ULONGEST *xfered_len)
> +{
> +  if (strcmp (annex, "target.xml"))
> +    return TARGET_XFER_E_IO;
> +
> +  if (readbuf == NULL)
> +    error (_("tfile_xfer_partial: tdesc is read-only"));
> +
> +  if (!trace_tdesc)
> +    return TARGET_XFER_E_IO;

  if (trace_tdesc == NULL)
    return TARGET_XFER_E_IO;

>  
> +/* Handles tdesc lines from tfile by appending the payload to
> +   a global trace_tdesc variable.  */
> +
> +static void
> +tfile_append_tdesc_line (const char *line)
> +{
> +  int llen = strlen (line);
> +
> +  /* 2 chars for "\n\0".  */
> +  while (trace_tdesc_len + llen + 2 > trace_tdesc_alloc)
> +    {
> +      /* Grow the buffer.  */
> +      if (!trace_tdesc_alloc)
> +	trace_tdesc_alloc = 4096;
> +      else
> +	trace_tdesc_alloc *= 2;
> +      trace_tdesc = xrealloc(trace_tdesc, trace_tdesc_alloc);
> +    }
> +
> +  strcpy (trace_tdesc + trace_tdesc_len, line);
> +  trace_tdesc_len += llen;
> +  strcpy (trace_tdesc + trace_tdesc_len, "\n");
> +  trace_tdesc_len++;
> +}

Please use "struct buffer" instead of the manual xreallocing.

See similar examples here:
https://sourceware.org/ml/gdb-patches/2016-02/msg00070.html

Thanks,
Pedro Alves

Re: [PATCH 1/4] gdb.trace: Save XML target description in tfile.

[Pedro Alves]

On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:

> diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
> index b761894..f148758 100644
> --- a/gdb/tracefile-tfile.c
> +++ b/gdb/tracefile-tfile.c
> @@ -28,6 +28,7 @@
>  #include "exec.h" /* exec_bfd */
>  #include "completer.h"
>  #include "filenames.h"
> +#include "xml-tdesc.h"
>  
>  #ifndef O_LARGEFILE
>  #define O_LARGEFILE 0
> @@ -263,6 +264,42 @@ tfile_write_uploaded_tp (struct trace_file_writer *self,
>  }
>  
>  /* This is the implementation of trace_file_write_ops method
> +   write_tdesc.  */
> +
> +static void
> +tfile_write_tdesc (struct trace_file_writer *self)
> +{
> +  struct tfile_trace_file_writer *writer
> +    = (struct tfile_trace_file_writer *) self;
> +  char *tdesc = target_fetch_description_xml (&current_target);
> +  char *ptr = tdesc;
> +  char *next;
> +
> +  if (!tdesc)
> +    return;

  if (tdesc == NULL)
    return;


> +
> +  /* Write tdesc line by line, prefixing each line with "tdesc ".  */
> +  while (ptr)

+  while (ptr != NULL)


> +    {
> +      next = strchr (ptr, '\n');
> +      if (next)
> +	{
> +	  fprintf (writer->fp, "tdesc %.*s\n", (int) (next-ptr), ptr);

Spaces around the '-'.

> +	  /* Skip the \n.  */
> +	  next++;
> +	}
> +      else if (*ptr)

      else if (*ptr != '\0')


> +	{
> +	  /* Last line, doesn't have a newline.  */
> +	  fprintf (writer->fp, "tdesc %s\n", ptr);
> +	}
> +      ptr = next;
> +    }
> +
> +  xfree (tdesc);
> +}
> +
> +/* This is the implementation of trace_file_write_ops method
>     write_definition_end.  */
>  
>  static void
> @@ -315,6 +352,7 @@ static const struct trace_file_write_ops tfile_write_ops =
>    tfile_write_status,
>    tfile_write_uploaded_tsv,
>    tfile_write_uploaded_tp,
> +  tfile_write_tdesc,
>    tfile_write_definition_end,
>    tfile_write_raw_data,
>    NULL,
> diff --git a/gdb/tracefile.c b/gdb/tracefile.c
> index fef4ed9..de42165 100644
> --- a/gdb/tracefile.c
> +++ b/gdb/tracefile.c
> @@ -90,6 +90,9 @@ trace_save (const char *filename, struct trace_file_writer *writer,
>    /* Write out the size of a register block.  */
>    writer->ops->write_regblock_type (writer, trace_regblock_size);
>  
> +  /* Write out the target description info.  */
> +  writer->ops->write_tdesc (writer);
> +
>    /* Write out status of the tracing run (aka "tstatus" info).  */
>    writer->ops->write_status (writer, ts);
>  
> diff --git a/gdb/tracefile.h b/gdb/tracefile.h
> index 8b711a1..e6d4460 100644
> --- a/gdb/tracefile.h
> +++ b/gdb/tracefile.h
> @@ -84,6 +84,9 @@ struct trace_file_write_ops
>    void (*write_uploaded_tp) (struct trace_file_writer *self,
>  			     struct uploaded_tp *tp);
>  
> +  /* Write target description.  */
> +  void (*write_tdesc) (struct trace_file_writer *self);
> +
>    /* Write to mark the end of the definition part.  */
>    void (*write_definition_end) (struct trace_file_writer *self);
>  
> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 5eeda86..202de86 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -630,3 +630,29 @@ target_read_description_xml (struct target_ops *ops)
>  
>    return tdesc;
>  }

/* See whatever.h.  */

> +
> +char *
> +target_fetch_description_xml (struct target_ops *ops)
> +{
> +  struct target_desc *tdesc;
> +  char *tdesc_str;
> +  char *expanded_text;
> +  struct cleanup *back_to;
> +
> +  tdesc_str = fetch_available_features_from_target ("target.xml", ops);
> +  if (tdesc_str == NULL)
> +    return NULL;
> +
> +  back_to = make_cleanup (xfree, tdesc_str);
> +  expanded_text = xml_process_xincludes (_("target description"),
> +					 tdesc_str,
> +					 fetch_available_features_from_target, ops, 0);
> +  if (expanded_text == NULL)
> +    {
> +      warning (_("Could not load XML target description; ignoring"));
> +      return NULL;

This path is missing do_cleanups.

> +    }
> +  do_cleanups (back_to);
> +
> +  return expanded_text;
> +}
> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
> index a0c38d7..38bb99e 100644
> --- a/gdb/xml-tdesc.h
> +++ b/gdb/xml-tdesc.h
> @@ -31,3 +31,9 @@ const struct target_desc *file_read_description_xml (const char *filename);
>     parsed description.  */
>  
>  const struct target_desc *target_read_description_xml (struct target_ops *);
> +
> +/* Fetches an XML target description using OPS,  processing
> +   includes, but not parsing it.  Used to dump whole tdesc
> +   as a single XML file.  */
> +
> +char * target_fetch_description_xml (struct target_ops *ops);
> 

No space after "char *" :

char *target_fetch_description_xml (struct target_ops *ops);

Thanks,
Pedro Alves

Re: [PATCH 3/4] gdb.trace: Use g packet order in tfile_fetch_registers.

[Pedro Alves]

On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
> tfile_fetch_registers currently wrongly fetches registers using
> gdb order instead of g packet order.  On x86_64 with AVX, this causes
> problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
> g packet has orig_rax first.
> 
> gdb/ChangeLog:
> 
> 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
> 	instead of gdb order.

OK.

The docs already explicitly say that we use g packet order, though the
bit about GDB register order seems odd:

 @table @code
 @item R @var{bytes}
 Register block.  The number and ordering of bytes matches that of a
                                 ^^^^^^^^          ^^^^^^^

 @code{g} packet in the remote protocol.  Note that these are the
 ^^^^^^^^^^^^^^^
 actual bytes, in target order and @value{GDBN} register order, not a
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^

                                            ????

 hexadecimal encoding.


I can't make sense of that.  I think we should
s/and @value{GDBN} register order,// .

WDYT?

Thanks,
Pedro Alves

Re: [PATCH 3/4] gdb.trace: Use g packet order in tfile_fetch_registers.

[Marcin Kościelnicki]

On 10/02/16 14:20, Pedro Alves wrote:
> On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
>> tfile_fetch_registers currently wrongly fetches registers using
>> gdb order instead of g packet order.  On x86_64 with AVX, this causes
>> problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
>> g packet has orig_rax first.
>>
>> gdb/ChangeLog:
>>
>> 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
>> 	instead of gdb order.
>
> OK.
>
> The docs already explicitly say that we use g packet order, though the
> bit about GDB register order seems odd:
>
>   @table @code
>   @item R @var{bytes}
>   Register block.  The number and ordering of bytes matches that of a
>                                   ^^^^^^^^          ^^^^^^^
>
>   @code{g} packet in the remote protocol.  Note that these are the
>   ^^^^^^^^^^^^^^^
>   actual bytes, in target order and @value{GDBN} register order, not a
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>                                              ????
>
>   hexadecimal encoding.
>
>
> I can't make sense of that.  I think we should
> s/and @value{GDBN} register order,// .
>
> WDYT?
>
> Thanks,
> Pedro Alves
>

Yeah, I think so.  Should I add it to the patch?

Re: [PATCH 4/4] gdb.trace: Fix off-by-one in tfile_fetch_registers.

[Pedro Alves]

On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
> This resulted in the last register being considered unavailable.
> On plain x86_64 (without AVX), this happened to be orig_rax.
> 
> gdb/ChangeLog:
> 
> 	* tracefile-tfile.c (tfile_fetch_registers): Fix off-by-one in bounds
> 	check.

OK.

Thanks,
Pedro Alves

Re: [PATCH 5/6] gdb/x86: Implement ax_pseudo_register_collect hook.

[Pedro Alves]

Great stuff.  Minor comments follow...

Patch is missing intro comments to new functions:

  /* Implementation of foo. / See foo.h. / etc.  */

On 02/06/2016 08:54 PM, Marcin Kościelnicki wrote:

> +static int
> +amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
> +				  struct agent_expr *ax, int regnum)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

Empty line after decl here.


> +int
> +i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
> +				 struct agent_expr *ax, int regnum)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

Ditto.

> +  if (i386_mmx_regnum_p (gdbarch, regnum))
> +    {
> +      /* MMX to FPU register mapping depends on current TOS.  Let's just
> +	 not care and collect everything...  */
> +      int i;

Ditto.  (several instances more in the function)

> +      ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
> +      for (i = 0; i < 8; i++)
> +	ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
> +      return 0;
> +    }


> +  else if (i386_byte_regnum_p (gdbarch, regnum))
> +    {
> +      /* Check byte pseudo registers last since this function will
> +	 be called from amd64_ax_pseudo_register_collect, which handles
> +	 byte pseudo registers differently.  */

I don't understand this comment.  amd64_ax_pseudo_register_collect
checks i386_byte_regnum_p before ever reaching here, afaics.

> +      int gpnum = regnum - tdep->al_regnum;
> +      ax_reg_mask (ax, gpnum % 4);
> +      return 0;
> +    }
> +  else
> +    internal_error (__FILE__, __LINE__, _("invalid regnum"));
> +  return 1;
> +}

Thanks,
Pedro Alves

Re: [PATCH 5/6] gdb/x86: Implement ax_pseudo_register_collect hook.

[Marcin Kościelnicki]

On 10/02/16 14:31, Pedro Alves wrote:
> Great stuff.  Minor comments follow...
>
> Patch is missing intro comments to new functions:
>
>    /* Implementation of foo. / See foo.h. / etc.  */

OK.
>
> On 02/06/2016 08:54 PM, Marcin Kościelnicki wrote:
>
>> +static int
>> +amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
>> +				  struct agent_expr *ax, int regnum)
>> +{
>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> Empty line after decl here.

OK.
>
>
>> +int
>> +i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
>> +				 struct agent_expr *ax, int regnum)
>> +{
>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
> Ditto.
>
>> +  if (i386_mmx_regnum_p (gdbarch, regnum))
>> +    {
>> +      /* MMX to FPU register mapping depends on current TOS.  Let's just
>> +	 not care and collect everything...  */
>> +      int i;
>
> Ditto.  (several instances more in the function)
>
>> +      ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
>> +      for (i = 0; i < 8; i++)
>> +	ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
>> +      return 0;
>> +    }
>
>
>> +  else if (i386_byte_regnum_p (gdbarch, regnum))
>> +    {
>> +      /* Check byte pseudo registers last since this function will
>> +	 be called from amd64_ax_pseudo_register_collect, which handles
>> +	 byte pseudo registers differently.  */
>
> I don't understand this comment.  amd64_ax_pseudo_register_collect
> checks i386_byte_regnum_p before ever reaching here, afaics.

I've copied it from i386_pseudo_register_read_into_value - didn't make 
that much sense to me either.  Let's remove it from both places?
>
>> +      int gpnum = regnum - tdep->al_regnum;
>> +      ax_reg_mask (ax, gpnum % 4);
>> +      return 0;
>> +    }
>> +  else
>> +    internal_error (__FILE__, __LINE__, _("invalid regnum"));
>> +  return 1;
>> +}
>
> Thanks,
> Pedro Alves
>

Re: [PATCH 6/6] gdb.trace: Add a testcase for tdesc in tfile.

[Pedro Alves]

On 02/06/2016 08:54 PM, Marcin Kościelnicki wrote:
> This tests whether $ymm15 can be correctly collected and printed from
> tfile.  It covers:
> 
...
> - using proper tdesc in encoding tracepoint actions (without that,
>   internal error happens due to $ymm15h being considered unavailable)

> This test requires https://sourceware.org/ml/gdb-patches/2016-01/msg00115.html

I'll take a look at that one soon.

> 
>  gdb/testsuite/ChangeLog               |  5 +++
>  gdb/testsuite/gdb.trace/tfile-avx.c   | 48 ++++++++++++++++++++
>  gdb/testsuite/gdb.trace/tfile-avx.exp | 82 +++++++++++++++++++++++++++++++++++

Looks mostly OK.  Thanks much for doing all of this.  Some comments below.

>  3 files changed, 135 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.c
>  create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.exp
> 
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 046f112..c9a596c 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-02-06  Marcin Kościelnicki  <koriakin@0x04.net>
> +
> +	* gdb.trace/tfile-avx.c: New test.
> +	* gdb.trace/tfile-avx.exp: New test.
> +
>  2016-02-04  Yao Qi  <yao.qi@linaro.org>
>  
>  	* gdb.base/foll-exec-mode.c: Include limits.h.
> diff --git a/gdb/testsuite/gdb.trace/tfile-avx.c b/gdb/testsuite/gdb.trace/tfile-avx.c
> new file mode 100644
> index 0000000..71107b8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/tfile-avx.c
> @@ -0,0 +1,48 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2016 Free Software Foundation, Inc.
> +
> +   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/>.  */
> +
> +/*
> + * Test program for reading target description from tfile: collects AVX
> + * registers on x86_64.
> + */
> +
> +#include <immintrin.h>
> +
> +void
> +dummy (void)
> +{}
> +
> +static void
> +end (void)
> +{}
> +
> +int main (void)

Tests should follow GNU/GDB coding conventions too, unless
required for the functionality of the test.

So write:

void
dummy (void)
{
}

int
main (void)

...


> +{
> +  register __v8si a asm("ymm15") = {
> +    0x12340001,
> +    0x12340002,
> +    0x12340003,
> +    0x12340004,
> +    0x12340005,
> +    0x12340006,
> +    0x12340007,
> +    0x12340008,
> +  };
> +  asm volatile ("traceme: call dummy" : : "x" (a));
> +  end ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.trace/tfile-avx.exp b/gdb/testsuite/gdb.trace/tfile-avx.exp
> new file mode 100644
> index 0000000..1acf4cb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/tfile-avx.exp
> @@ -0,0 +1,82 @@
> +# Copyright 2016 Free Software Foundation, Inc.
> +# 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/>.
> +
> +if { ! [istarget "x86_64*-*-*linux*"] } {
> +    verbose "Skipping tfile AVX test (target is not x86_64 linux)."
> +    return
> +}

Check is_amd64_regs_target instead, to cover i386-linux/-m64


> +
> +load_lib "trace-support.exp"
> +
> +standard_testfile
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile \
> +     [list debug nowarnings additional_flags=-mavx]]} {
> +    return -1

Why nowarnings?

> +}
> +
> +if ![runto_main] {
> +    fail "Can't run to main to check for trace support"
> +    return -1
> +}
> +
> +if ![gdb_target_supports_trace] {
> +    unsupported "target does not support trace"
> +    return -1
> +}
> +
> +send_gdb "print \$ymm15\n"
> +gdb_expect {
> +    -re "\\$\[0-9\]+ = void.*$gdb_prompt \$" {
> +	verbose "Skipping tfile AVX test (target doesn't support AVX)."
> +	return
> +    }
> +    -re "\\$\[0-9\]+ = \\{.*}.*$gdb_prompt \$" {
> +	# All is well.
> +    }
> +}

Please use gdb_test_multiple instead of raw send_gdb/gdb_expect.

No backslash in "$gdb_prompt \$".

Use $decimal, or just drop that "\\$\[0-9\]+" part.

> +
> +gdb_test "trace traceme" ".*"
> +
> +gdb_trace_setactions "set actions for tracepoint" "" \
> +	"collect \$ymm15" "^$"
> +
> +gdb_breakpoint "end"
> +
> +gdb_test_no_output "tstart"
> +
> +gdb_test "continue" ".*Breakpoint \[0-9\]+, end .*"

$decimal

> +
> +set tracefile [standard_output_file ${testfile}]
> +
> +# Save trace frames to tfile.
> +gdb_test "tsave ${tracefile}.tf" \
> +    "Trace data saved to file '${tracefile}.tf'.*" \
> +    "save tfile trace file"
> +
> +# Change target to tfile.
> +set test "change to tfile target"
> +gdb_test_multiple "target tfile ${tracefile}.tf" "$test" {
> +    -re "A program is being debugged already.  Kill it. .y or n. " {
> +	send_gdb "y\n"
> +	exp_continue
> +    }
> +    -re "$gdb_prompt $" {
> +	pass "$test"
> +    }
> +}

gdb_test handles queries:

gdb_test \
  "target tfile ${tracefile}.tf" "" \
  "change to tfile target" \
  "A program is being debugged already.  Kill it. .y or n. $" \
  "y"

> +
> +gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
> +
> +gdb_test "print/x \$ymm15.v8_int32" "\\$\[0-9\]+ = \\{0x12340001, .*, 0x12340008}.*"
> 

$decimal, or just drop the value number match.  Drop .* at the end to tighten
the regex (gdb_test expects \r\n before the prompt).

  gdb_test "print/x \$ymm15.v8_int32" " = \\{0x12340001, .*, 0x12340008}"

Thanks,
Pedro Alves

Re: [PATCH 5/6] gdb/x86: Implement ax_pseudo_register_collect hook.

[Pedro Alves]

On 02/10/2016 01:34 PM, Marcin Kościelnicki wrote:

>> I don't understand this comment.  amd64_ax_pseudo_register_collect
>> checks i386_byte_regnum_p before ever reaching here, afaics.
> 
> I've copied it from i386_pseudo_register_read_into_value - didn't make 
> that much sense to me either.  Let's remove it from both places?

Yes please.

Thanks,
Pedro Alves

Re: [PATCH 4/4] gdb.trace: Fix off-by-one in tfile_fetch_registers.

[Marcin Kościelnicki]

On 10/02/16 14:21, Pedro Alves wrote:
> On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
>> This resulted in the last register being considered unavailable.
>> On plain x86_64 (without AVX), this happened to be orig_rax.
>>
>> gdb/ChangeLog:
>>
>> 	* tracefile-tfile.c (tfile_fetch_registers): Fix off-by-one in bounds
>> 	check.
>
> OK.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

Re: [PATCH 3/4] gdb.trace: Use g packet order in tfile_fetch_registers.

[Pedro Alves]

On 02/10/2016 01:21 PM, Marcin Kościelnicki wrote:

>> I can't make sense of that.  I think we should
>> s/and @value{GDBN} register order,// .
>>
>> WDYT?
>>

> Yeah, I think so.  Should I add it to the patch?

Yes please.

Thanks,
Pedro Alves

[PATCH] gdb.trace: Use g packet order in tfile_fetch_registers.

[Marcin Kościelnicki]

tfile_fetch_registers currently wrongly fetches registers using
gdb order instead of g packet order.  On x86_64 with AVX, this causes
problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
g packet has orig_rax first.

gdb/ChangeLog:

	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
	instead of gdb order.

gdb/doc/ChangeLog:

	* gdb.texinfo (Trace File Format): Remove misleading information
	about register block ordering.
---
Removed misleading documentation bit.  OK to push now?

 gdb/ChangeLog         |  5 +++++
 gdb/doc/ChangeLog     |  5 +++++
 gdb/doc/gdb.texinfo   |  3 +--
 gdb/tracefile-tfile.c | 11 ++++++-----
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0454696..fd7dd9d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
+	instead of gdb order.
+
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* tracefile-tfile.c (trace_tdesc): New static variable.
 	(trace_tdesc_alloc): New static variable.
 	(trace_tdesc_len): New static variable.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 05d2694..75b24ef 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdb.texinfo (Trace File Format): Remove misleading information
+	about register block ordering.
+
 2016-02-01  Doug Evans  <dje@google.com>
 
 	* gdb.texinfo (Value Sizes): Fix typo.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2d09d13..9db234e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -41048,8 +41048,7 @@ endianness.
 @item R @var{bytes}
 Register block.  The number and ordering of bytes matches that of a
 @code{g} packet in the remote protocol.  Note that these are the
-actual bytes, in target order and @value{GDBN} register order, not a
-hexadecimal encoding.
+actual bytes, in target order, not a hexadecimal encoding.
 
 @item M @var{address} @var{length} @var{bytes}...
 Memory block.  This is a contiguous block of memory, at the 8-byte
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 139d90a..a6c3c9c 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -30,6 +30,7 @@
 #include "filenames.h"
 #include "xml-tdesc.h"
 #include "target-descriptions.h"
+#include "remote.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -857,7 +858,7 @@ tfile_fetch_registers (struct target_ops *ops,
 		       struct regcache *regcache, int regno)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  int offset, regn, regsize;
+  int offset, regn, regsize, dummy;
 
   /* An uninitialized reg size says we're not going to be
      successful at getting register blocks.  */
@@ -870,11 +871,12 @@ tfile_fetch_registers (struct target_ops *ops,
 
       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++)
 	{
+	  if (!remote_register_number_and_offset (get_regcache_arch (regcache),
+						  regn, &dummy, &offset))
+	    continue;
+
 	  regsize = register_size (gdbarch, regn);
 	  /* Make sure we stay within block bounds.  */
 	  if (offset + regsize > trace_regblock_size)
@@ -891,7 +893,6 @@ tfile_fetch_registers (struct target_ops *ops,
 		  regcache_raw_supply (regcache, regn, regs + offset);
 		}
 	    }
-	  offset += regsize;
 	}
     }
   else
-- 
2.7.0

Re: [PATCH] gdb.trace: Use g packet order in tfile_fetch_registers.

[Pedro Alves]

On 02/10/2016 02:12 PM, Marcin Kościelnicki wrote:
> tfile_fetch_registers currently wrongly fetches registers using
> gdb order instead of g packet order.  On x86_64 with AVX, this causes
> problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
> g packet has orig_rax first.
> 
> gdb/ChangeLog:
> 
> 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
> 	instead of gdb order.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Trace File Format): Remove misleading information
> 	about register block ordering.
> ---
> Removed misleading documentation bit.  OK to push now?

OK.

Thanks,
Pedro Alves

Re: [PATCH 0/4] Save target description in tfile.

[Pedro Alves]

On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
> As discussed in the s390 trace thread, this adds target description
> in XML format to tfile format.  The idea is simple:
> 
> 1. target.xml is read from the target.
> 2. Includes are processed, resulting in a single in-memory XML file
>    containing all the data.
> 3. The resulting file is stored in tfile header by prefixing every line
>    with "tdesc ".  We may insert a spurious newline at the end of file
>    with this encoding, but that won't matter for XML.
> 4. When tfile is read, the XML is stored in an allocated buffer, and
>    xfer for TARGET_OBJECT_AVAILABLE_FEATURES is implemented, reading
>    from it.
> 5. target_find_description is called to force reading it.
> 
> Patch #1 makes tsave write the XML data into tfile.  Patch #2 makes
> tfile reader fetch and parse this data.  Patch #3 fixes wrong register
> ordering in tfile_fetch_registers (noticed on x86_64 with AVX -
> plain x86_64 was apparently fine).  Patch #4 fixes an off-by-one
> in the same function that prevented fetching the last register
> (orig_rax for plain x86_64, $ymm15h for x86_64 with AVX).
> 

I'd be good to also document the tdesc header lines in
the "Trace File Format" node in the manual.

Thanks,
Pedro Alves

Re: [PATCH 0/4] Save target description in tfile.

[Marcin Kościelnicki]

On 10/02/16 15:24, Pedro Alves wrote:
> On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:
>> As discussed in the s390 trace thread, this adds target description
>> in XML format to tfile format.  The idea is simple:
>>
>> 1. target.xml is read from the target.
>> 2. Includes are processed, resulting in a single in-memory XML file
>>     containing all the data.
>> 3. The resulting file is stored in tfile header by prefixing every line
>>     with "tdesc ".  We may insert a spurious newline at the end of file
>>     with this encoding, but that won't matter for XML.
>> 4. When tfile is read, the XML is stored in an allocated buffer, and
>>     xfer for TARGET_OBJECT_AVAILABLE_FEATURES is implemented, reading
>>     from it.
>> 5. target_find_description is called to force reading it.
>>
>> Patch #1 makes tsave write the XML data into tfile.  Patch #2 makes
>> tfile reader fetch and parse this data.  Patch #3 fixes wrong register
>> ordering in tfile_fetch_registers (noticed on x86_64 with AVX -
>> plain x86_64 was apparently fine).  Patch #4 fixes an off-by-one
>> in the same function that prevented fetching the last register
>> (orig_rax for plain x86_64, $ymm15h for x86_64 with AVX).
>>
>
> I'd be good to also document the tdesc header lines in
> the "Trace File Format" node in the manual.

Alright, will do that in another commit.
>
> Thanks,
> Pedro Alves
>

[PATCH] gdb/x86: Implement ax_pseudo_register_collect hook.

[Marcin Kościelnicki]

Makes "collect $ymm15" action work.

gdb/ChangeLog:

	* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
	(amd64_init_abi): Fill ax_pseudo_register_collect hook.
	* gdb/i386-tdep.c (i386_pseudo_register_read_into_value): Remove
	misleading comment.
	(i386_pseudo_register_write): Ditto.
	(i386_ax_pseudo_register_collect): New function.
	(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
	* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
---
Fixed, misleading comments removed.  OK to push now?

 gdb/ChangeLog    | 11 +++++++
 gdb/amd64-tdep.c | 31 +++++++++++++++++++
 gdb/i386-tdep.c  | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 gdb/i386-tdep.h  |  4 +++
 4 files changed, 130 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fd7dd9d..c260834 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
 2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
+	(amd64_init_abi): Fill ax_pseudo_register_collect hook.
+	* gdb/i386-tdep.c (i386_pseudo_register_read_into_value): Remove
+	misleading comment.
+	(i386_pseudo_register_write): Ditto.
+	(i386_ax_pseudo_register_collect): New function.
+	(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
+	* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
+
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
 	instead of gdb order.
 
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index fae92b2..a62efde 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -455,6 +455,35 @@ amd64_pseudo_register_write (struct gdbarch *gdbarch,
     i386_pseudo_register_write (gdbarch, regcache, regnum, buf);
 }
 
+/* Implement the 'ax_pseudo_register_collect' gdbarch method.  */
+
+static int
+amd64_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+				  struct agent_expr *ax, int regnum)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (i386_byte_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->al_regnum;
+
+      if (gpnum >= AMD64_NUM_LOWER_BYTE_REGS)
+	ax_reg_mask (ax, gpnum - AMD64_NUM_LOWER_BYTE_REGS);
+      else
+	ax_reg_mask (ax, gpnum);
+      return 0;
+    }
+  else if (i386_dword_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->eax_regnum;
+
+      ax_reg_mask (ax, gpnum);
+      return 0;
+    }
+  else
+    return i386_ax_pseudo_register_collect (gdbarch, ax, regnum);
+}
+
 \f
 
 /* Register classes as defined in the psABI.  */
@@ -2997,6 +3026,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 					  amd64_pseudo_register_read_value);
   set_gdbarch_pseudo_register_write (gdbarch,
 				     amd64_pseudo_register_write);
+  set_gdbarch_ax_pseudo_register_collect (gdbarch,
+					  amd64_ax_pseudo_register_collect);
 
   set_tdesc_pseudo_register_name (gdbarch, amd64_pseudo_register_name);
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b706463..db4cd0a 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3419,9 +3419,6 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 	}
       else if (i386_byte_regnum_p (gdbarch, regnum))
 	{
-	  /* Check byte pseudo registers last since this function will
-	     be called from amd64_pseudo_register_read, which handles
-	     byte pseudo registers differently.  */
 	  int gpnum = regnum - tdep->al_regnum;
 
 	  /* Extract (always little endian).  We read both lower and
@@ -3584,9 +3581,6 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	}
       else if (i386_byte_regnum_p (gdbarch, regnum))
 	{
-	  /* Check byte pseudo registers last since this function will
-	     be called from amd64_pseudo_register_read, which handles
-	     byte pseudo registers differently.  */
 	  int gpnum = regnum - tdep->al_regnum;
 
 	  /* Read ...  We read both lower and upper registers.  */
@@ -3603,6 +3597,88 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 	internal_error (__FILE__, __LINE__, _("invalid regnum"));
     }
 }
+
+/* Implement the 'ax_pseudo_register_collect' gdbarch method.  */
+
+int
+i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+				 struct agent_expr *ax, int regnum)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (i386_mmx_regnum_p (gdbarch, regnum))
+    {
+      /* MMX to FPU register mapping depends on current TOS.  Let's just
+	 not care and collect everything...  */
+      int i;
+
+      ax_reg_mask (ax, I387_FSTAT_REGNUM (tdep));
+      for (i = 0; i < 8; i++)
+	ax_reg_mask (ax, I387_ST0_REGNUM (tdep) + i);
+      return 0;
+    }
+  else if (i386_bnd_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->bnd0_regnum;
+      ax_reg_mask (ax, I387_BND0R_REGNUM (tdep) + regnum);
+      return 0;
+    }
+  else if (i386_k_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->k0_regnum;
+      ax_reg_mask (ax, tdep->k0_regnum + regnum);
+      return 0;
+    }
+  else if (i386_zmm_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->zmm0_regnum;
+      if (regnum < num_lower_zmm_regs)
+	{
+	  ax_reg_mask (ax, I387_XMM0_REGNUM (tdep) + regnum);
+	  ax_reg_mask (ax, tdep->ymm0h_regnum + regnum);
+	}
+      else
+	{
+	  ax_reg_mask (ax, I387_XMM16_REGNUM (tdep) + regnum
+			   - num_lower_zmm_regs);
+	  ax_reg_mask (ax, I387_YMM16H_REGNUM (tdep) + regnum
+			   - num_lower_zmm_regs);
+	}
+      ax_reg_mask (ax, tdep->zmm0h_regnum + regnum);
+      return 0;
+    }
+  else if (i386_ymm_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->ymm0_regnum;
+      ax_reg_mask (ax, I387_XMM0_REGNUM (tdep) + regnum);
+      ax_reg_mask (ax, tdep->ymm0h_regnum + regnum);
+      return 0;
+    }
+  else if (i386_ymm_avx512_regnum_p (gdbarch, regnum))
+    {
+      regnum -= tdep->ymm16_regnum;
+      ax_reg_mask (ax, I387_XMM16_REGNUM (tdep) + regnum);
+      ax_reg_mask (ax, tdep->ymm16h_regnum + regnum);
+      return 0;
+    }
+  else if (i386_word_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->ax_regnum;
+
+      ax_reg_mask (ax, gpnum);
+      return 0;
+    }
+  else if (i386_byte_regnum_p (gdbarch, regnum))
+    {
+      int gpnum = regnum - tdep->al_regnum;
+
+      ax_reg_mask (ax, gpnum % 4);
+      return 0;
+    }
+  else
+    internal_error (__FILE__, __LINE__, _("invalid regnum"));
+  return 1;
+}
 \f
 
 /* Return the register number of the register allocated by GCC after
@@ -8423,6 +8499,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_pseudo_register_read_value (gdbarch,
 					  i386_pseudo_register_read_value);
   set_gdbarch_pseudo_register_write (gdbarch, i386_pseudo_register_write);
+  set_gdbarch_ax_pseudo_register_collect (gdbarch,
+					  i386_ax_pseudo_register_collect);
 
   set_tdesc_pseudo_register_type (gdbarch, i386_pseudo_register_type);
   set_tdesc_pseudo_register_name (gdbarch, i386_pseudo_register_name);
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 10d2772..770f59d 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -360,6 +360,10 @@ extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
 					struct regcache *regcache,
 					int regnum, const gdb_byte *buf);
 
+extern int i386_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+					    struct agent_expr *ax,
+					    int regnum);
+
 /* Segment selectors.  */
 #define I386_SEL_RPL	0x0003  /* Requester's Privilege Level mask.  */
 #define I386_SEL_UPL	0x0003	/* User Privilige Level.  */
-- 
2.7.0

Re: [PATCH] gdb/x86: Implement ax_pseudo_register_collect hook.

[Pedro Alves]

On 02/10/2016 02:27 PM, Marcin Kościelnicki wrote:
> Makes "collect $ymm15" action work.
> 
> gdb/ChangeLog:
> 
> 	* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
> 	(amd64_init_abi): Fill ax_pseudo_register_collect hook.
> 	* gdb/i386-tdep.c (i386_pseudo_register_read_into_value): Remove
> 	misleading comment.
> 	(i386_pseudo_register_write): Ditto.
> 	(i386_ax_pseudo_register_collect): New function.
> 	(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
> 	* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
> ---
> Fixed, misleading comments removed.  OK to push now?

OK.

Thanks,
Pedro Alves

Re: [PATCH] gdb/x86: Implement ax_pseudo_register_collect hook.

[Marcin Kościelnicki]

On 10/02/16 15:28, Pedro Alves wrote:
> On 02/10/2016 02:27 PM, Marcin Kościelnicki wrote:
>> Makes "collect $ymm15" action work.
>>
>> gdb/ChangeLog:
>>
>> 	* amd64-tdep.c (amd64_ax_pseudo_register_collect): New function.
>> 	(amd64_init_abi): Fill ax_pseudo_register_collect hook.
>> 	* gdb/i386-tdep.c (i386_pseudo_register_read_into_value): Remove
>> 	misleading comment.
>> 	(i386_pseudo_register_write): Ditto.
>> 	(i386_ax_pseudo_register_collect): New function.
>> 	(i386_gdbarch_init): Fill ax_pseudo_register_collect hook.
>> 	* i386-tdep.h: Add i386_ax_pseudo_register_collect prototype.
>> ---
>> Fixed, misleading comments removed.  OK to push now?
>
> OK.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

Re: [PATCH] gdb.trace: Use g packet order in tfile_fetch_registers.

[Marcin Kościelnicki]

On 10/02/16 15:21, Pedro Alves wrote:
> On 02/10/2016 02:12 PM, Marcin Kościelnicki wrote:
>> tfile_fetch_registers currently wrongly fetches registers using
>> gdb order instead of g packet order.  On x86_64 with AVX, this causes
>> problems with ymm*h and orig_rax registers: gdb has ymm*h first, while
>> g packet has orig_rax first.
>>
>> gdb/ChangeLog:
>>
>> 	* tracefile-tfile.c (tfile_fetch_registers): Use g packet order
>> 	instead of gdb order.
>>
>> gdb/doc/ChangeLog:
>>
>> 	* gdb.texinfo (Trace File Format): Remove misleading information
>> 	about register block ordering.
>> ---
>> Removed misleading documentation bit.  OK to push now?
>
> OK.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

[PATCH 1/3] gdb.trace: Save XML target description in tfile.

[Marcin Kościelnicki]

gdb/ChangeLog:

	* ctf.c (ctf_write_tdesc): New function.
	(ctf_write_ops): Wire in ctf_write_tdesc.
	* tracefile-tfile.c (tfile_write_tdesc): New function.
	(tfile_write_ops): Wire in tfile_write_tdesc.
	* tracefile.c (trace_save): Call write_tdesc method.
	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
	* xml-tdesc.c (target_fetch_description_xml): New function.
	* xml-tdesc.h: Add target_fetch_description_xml prototype.
---
Fixes applied.

 gdb/ChangeLog         | 11 +++++++++++
 gdb/ctf.c             | 10 ++++++++++
 gdb/tracefile-tfile.c | 38 ++++++++++++++++++++++++++++++++++++++
 gdb/tracefile.c       |  3 +++
 gdb/tracefile.h       |  3 +++
 gdb/xml-tdesc.c       | 30 ++++++++++++++++++++++++++++++
 gdb/xml-tdesc.h       |  6 ++++++
 7 files changed, 101 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 07411d8..77412ca 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* ctf.c (ctf_write_tdesc): New function.
+	(ctf_write_ops): Wire in ctf_write_tdesc.
+	* tracefile-tfile.c (tfile_write_tdesc): New function.
+	(tfile_write_ops): Wire in tfile_write_tdesc.
+	* tracefile.c (trace_save): Call write_tdesc method.
+	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
+	* xml-tdesc.c (target_fetch_description_xml): New function.
+	* xml-tdesc.h: Add target_fetch_description_xml prototype.
+
 2016-02-10  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* arm-tdep.c (arm_copy_extra_ld_st): Fix "unpriveleged" typo.
diff --git a/gdb/ctf.c b/gdb/ctf.c
index 9d496e3..25a4c79 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -617,6 +617,15 @@ ctf_write_uploaded_tp (struct trace_file_writer *self,
 }
 
 /* This is the implementation of trace_file_write_ops method
+   write_tdesc.  */
+
+static void
+ctf_write_tdesc (struct trace_file_writer *self)
+{
+  /* Nothing so far. */
+}
+
+/* This is the implementation of trace_file_write_ops method
    write_definition_end.  */
 
 static void
@@ -799,6 +808,7 @@ static const struct trace_file_write_ops ctf_write_ops =
   ctf_write_status,
   ctf_write_uploaded_tsv,
   ctf_write_uploaded_tp,
+  ctf_write_tdesc,
   ctf_write_definition_end,
   NULL,
   &ctf_write_frame_ops,
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 8b42aba..c87f61d 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -29,6 +29,7 @@
 #include "completer.h"
 #include "filenames.h"
 #include "remote.h"
+#include "xml-tdesc.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -264,6 +265,42 @@ tfile_write_uploaded_tp (struct trace_file_writer *self,
 }
 
 /* This is the implementation of trace_file_write_ops method
+   write_tdesc.  */
+
+static void
+tfile_write_tdesc (struct trace_file_writer *self)
+{
+  struct tfile_trace_file_writer *writer
+    = (struct tfile_trace_file_writer *) self;
+  char *tdesc = target_fetch_description_xml (&current_target);
+  char *ptr = tdesc;
+  char *next;
+
+  if (tdesc == NULL)
+    return;
+
+  /* Write tdesc line by line, prefixing each line with "tdesc ".  */
+  while (ptr != NULL)
+    {
+      next = strchr (ptr, '\n');
+      if (next != NULL)
+	{
+	  fprintf (writer->fp, "tdesc %.*s\n", (int) (next - ptr), ptr);
+	  /* Skip the \n.  */
+	  next++;
+	}
+      else if (*ptr != '\0')
+	{
+	  /* Last line, doesn't have a newline.  */
+	  fprintf (writer->fp, "tdesc %s\n", ptr);
+	}
+      ptr = next;
+    }
+
+  xfree (tdesc);
+}
+
+/* This is the implementation of trace_file_write_ops method
    write_definition_end.  */
 
 static void
@@ -316,6 +353,7 @@ static const struct trace_file_write_ops tfile_write_ops =
   tfile_write_status,
   tfile_write_uploaded_tsv,
   tfile_write_uploaded_tp,
+  tfile_write_tdesc,
   tfile_write_definition_end,
   tfile_write_raw_data,
   NULL,
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index fef4ed9..de42165 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -90,6 +90,9 @@ trace_save (const char *filename, struct trace_file_writer *writer,
   /* Write out the size of a register block.  */
   writer->ops->write_regblock_type (writer, trace_regblock_size);
 
+  /* Write out the target description info.  */
+  writer->ops->write_tdesc (writer);
+
   /* Write out status of the tracing run (aka "tstatus" info).  */
   writer->ops->write_status (writer, ts);
 
diff --git a/gdb/tracefile.h b/gdb/tracefile.h
index 8b711a1..e6d4460 100644
--- a/gdb/tracefile.h
+++ b/gdb/tracefile.h
@@ -84,6 +84,9 @@ struct trace_file_write_ops
   void (*write_uploaded_tp) (struct trace_file_writer *self,
 			     struct uploaded_tp *tp);
 
+  /* Write target description.  */
+  void (*write_tdesc) (struct trace_file_writer *self);
+
   /* Write to mark the end of the definition part.  */
   void (*write_definition_end) (struct trace_file_writer *self);
 
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 5eeda86..4625b60 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -630,3 +630,33 @@ target_read_description_xml (struct target_ops *ops)
 
   return tdesc;
 }
+
+/* Fetches an XML target description using OPS,  processing
+   includes, but not parsing it.  Used to dump whole tdesc
+   as a single XML file.  */
+
+char *
+target_fetch_description_xml (struct target_ops *ops)
+{
+  struct target_desc *tdesc;
+  char *tdesc_str;
+  char *expanded_text;
+  struct cleanup *back_to;
+
+  tdesc_str = fetch_available_features_from_target ("target.xml", ops);
+  if (tdesc_str == NULL)
+    return NULL;
+
+  back_to = make_cleanup (xfree, tdesc_str);
+  expanded_text = xml_process_xincludes (_("target description"),
+					 tdesc_str,
+					 fetch_available_features_from_target, ops, 0);
+  do_cleanups (back_to);
+  if (expanded_text == NULL)
+    {
+      warning (_("Could not load XML target description; ignoring"));
+      return NULL;
+    }
+
+  return expanded_text;
+}
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index a0c38d7..70a1ebb 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -31,3 +31,9 @@ const struct target_desc *file_read_description_xml (const char *filename);
    parsed description.  */
 
 const struct target_desc *target_read_description_xml (struct target_ops *);
+
+/* Fetches an XML target description using OPS,  processing
+   includes, but not parsing it.  Used to dump whole tdesc
+   as a single XML file.  */
+
+char *target_fetch_description_xml (struct target_ops *ops);
-- 
2.7.0

[PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Marcin Kościelnicki]

gdb/doc/ChangeLog:

	* gdb.texinfo (Trace File Format): Add documentation for description
	section lines.
---
Here's the promised documentation for tdesc lines and then some.

 gdb/doc/ChangeLog   |  5 +++++
 gdb/doc/gdb.texinfo | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 75b24ef..12ace48 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,10 @@
 2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* gdb.texinfo (Trace File Format): Add documentation for description
+	section lines.
+
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* gdb.texinfo (Trace File Format): Remove misleading information
 	about register block ordering.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9db234e..0028eda 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -41031,7 +41031,38 @@ as tracepoint definitions or register set size.  @value{GDBN} will
 ignore any line that it does not recognize.  An empty line marks the end
 of this section.
 
-@c FIXME add some specific types of data
+@table @code
+@item R @var{size}
+Specifies the size of a register block in bytes.  This is equal to the
+size of a @code{g} packet payload in the remote protocol.  @var{size}
+is an ascii decimal number.  There should be only one such line in
+a single trace file.
+
+@item status @var{status}
+Trace status.  @var{status} has the same format as a @code{qTStatus}
+remote packet reply.  There should be only one such line in a single trace
+file.
+
+@item tp @var{payload}
+Tracepoint definition.  The @var{payload} has the same format as
+@code{qTfP}/@code{qTsP} remote packet reply payload.  A single tracepoint
+may take multiple lines of definition, corresponding to the multiple
+reply packets.
+
+@item tsv @var{payload}
+Trace state variable definition.  The @var{payload} has the same format as
+@code{qTfV}/@code{qTsV} remote packet reply payload.  A single variable
+may take multiple lines of definition, corresponding to the multiple
+reply packets.
+
+@item tdesc @var{payload}
+Target description in XML format.  The @var{payload} is a single line of
+the XML file.  All such lines should be concatenated together to get
+the original XML file.  This file is in the same format as @code{qXfer}
+@code{features} payload, and corresponds to the main @code{target.xml}
+file.  Includes are not allowed.
+
+@end table
 
 The trace frame section consists of a number of consecutive frames.
 Each frame begins with a two-byte tracepoint number, followed by a
-- 
2.7.0

[PATCH 2/3] gdb.trace: Read XML target description from tfile.

[Marcin Kościelnicki]

gdb/ChangeLog:

	* tracefile-tfile.c (trace_tdesc): New static variable.
	(tfile_open): Clear trace_tdesc, call target_find_description.
	(tfile_interp_line): Recognize tdesc lines.
	(tfile_close): Clear trace_tdesc.
	(tfile_xfer_partial_features): New function.
	(tfile_xfer_partial): Call tfile_xfer_partial_features.
	(tfile_append_tdesc_line): New function.
---
Fixes applied, buffer used.

 gdb/ChangeLog         | 10 +++++++++
 gdb/tracefile-tfile.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 77412ca..0ed35fa 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* tracefile-tfile.c (trace_tdesc): New static variable.
+	(tfile_open): Clear trace_tdesc, call target_find_description.
+	(tfile_interp_line): Recognize tdesc lines.
+	(tfile_close): Clear trace_tdesc.
+	(tfile_xfer_partial_features): New function.
+	(tfile_xfer_partial): Call tfile_xfer_partial_features.
+	(tfile_append_tdesc_line): New function.
+
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* ctf.c (ctf_write_tdesc): New function.
 	(ctf_write_ops): Wire in ctf_write_tdesc.
 	* tracefile-tfile.c (tfile_write_tdesc): New function.
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index c87f61d..d5d5ed1 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -30,6 +30,8 @@
 #include "filenames.h"
 #include "remote.h"
 #include "xml-tdesc.h"
+#include "target-descriptions.h"
+#include "buffer.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -391,7 +393,9 @@ static off_t trace_frames_offset;
 static off_t cur_offset;
 static int cur_data_size;
 int trace_regblock_size;
+static struct buffer trace_tdesc;
 
+static void tfile_append_tdesc_line (const char *line);
 static void tfile_interp_line (char *line,
 			       struct uploaded_tp **utpp,
 			       struct uploaded_tsv **utsvp);
@@ -458,6 +462,9 @@ tfile_open (const char *arg, int from_tty)
   trace_filename = xstrdup (filename);
   trace_fd = scratch_chan;
 
+  /* Make sure this is clear.  */
+  buffer_free (&trace_tdesc);
+
   bytes = 0;
   /* Read the file header and test for validity.  */
   tfile_read ((gdb_byte *) &header, TRACE_HEADER_SIZE);
@@ -506,6 +513,9 @@ tfile_open (const char *arg, int from_tty)
 	    error (_("Excessively long lines in trace file"));
 	}
 
+      /* By now, tdesc lines have been read from tfile - let's parse them.  */
+      target_find_description ();
+
       /* Record the starting offset of the binary trace data.  */
       trace_frames_offset = bytes;
 
@@ -569,6 +579,11 @@ tfile_interp_line (char *line, struct uploaded_tp **utpp,
       p += strlen ("tsv ");
       parse_tsv_definition (p, utsvp);
     }
+  else if (startswith (p, "tdesc "))
+    {
+      p += strlen ("tdesc ");
+      tfile_append_tdesc_line (p);
+    }
   else
     warning (_("Ignoring trace file definition \"%s\""), line);
 }
@@ -591,6 +606,7 @@ tfile_close (struct target_ops *self)
   trace_fd = -1;
   xfree (trace_filename);
   trace_filename = NULL;
+  buffer_free (&trace_tdesc);
 
   trace_reset_local_state ();
 }
@@ -877,12 +893,42 @@ tfile_fetch_registers (struct target_ops *ops,
 }
 
 static enum target_xfer_status
+tfile_xfer_partial_features (struct target_ops *ops, const char *annex,
+			     gdb_byte *readbuf, const gdb_byte *writebuf,
+			     ULONGEST offset, ULONGEST len,
+			     ULONGEST *xfered_len)
+{
+  if (strcmp (annex, "target.xml"))
+    return TARGET_XFER_E_IO;
+
+  if (readbuf == NULL)
+    error (_("tfile_xfer_partial: tdesc is read-only"));
+
+  if (trace_tdesc.used_size == 0)
+    return TARGET_XFER_E_IO;
+
+  if (offset >= trace_tdesc.used_size)
+    return TARGET_XFER_EOF;
+
+  if (len > trace_tdesc.used_size - offset)
+    len = trace_tdesc.used_size - offset;
+
+  memcpy (readbuf, trace_tdesc.buffer + offset, len);
+  *xfered_len = len;
+
+  return TARGET_XFER_OK;
+}
+
+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.  */
+  /* We're only doing regular memory and tdesc for now.  */
+  if (object == TARGET_OBJECT_AVAILABLE_FEATURES)
+    return tfile_xfer_partial_features (ops, annex, readbuf, writebuf,
+					offset, len, xfered_len);
   if (object != TARGET_OBJECT_MEMORY)
     return TARGET_XFER_E_IO;
 
@@ -1062,6 +1108,18 @@ tfile_traceframe_info (struct target_ops *self)
   return info;
 }
 
+/* Handles tdesc lines from tfile by appending the payload to
+   a global trace_tdesc variable.  */
+
+static void
+tfile_append_tdesc_line (const char *line)
+{
+  int llen = strlen (line);
+
+  buffer_grow_str (&trace_tdesc, line);
+  buffer_grow_str (&trace_tdesc, "\n");
+}
+
 static void
 init_tfile_ops (void)
 {
-- 
2.7.0

Re: [PATCH 2/3] gdb.trace: Read XML target description from tfile.

[Pedro Alves]

On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
> gdb/ChangeLog:
> 
> 	* tracefile-tfile.c (trace_tdesc): New static variable.
> 	(tfile_open): Clear trace_tdesc, call target_find_description.
> 	(tfile_interp_line): Recognize tdesc lines.
> 	(tfile_close): Clear trace_tdesc.
> 	(tfile_xfer_partial_features): New function.
> 	(tfile_xfer_partial): Call tfile_xfer_partial_features.
> 	(tfile_append_tdesc_line): New function.
> ---
> Fixes applied, buffer used.

Thanks.  This is OK, with...

> +/* Handles tdesc lines from tfile by appending the payload to
> +   a global trace_tdesc variable.  */
> +
> +static void
> +tfile_append_tdesc_line (const char *line)
> +{
> +  int llen = strlen (line);
> +

... this unused var dropped.

> +  buffer_grow_str (&trace_tdesc, line);
> +  buffer_grow_str (&trace_tdesc, "\n");
> +}


Thanks,
Pedro Alves

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Pedro Alves]

On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Trace File Format): Add documentation for description
> 	section lines.
> ---
> Here's the promised documentation for tdesc lines and then some.

Excellent!  Thanks much.

Looks good to me content-wise, but let's get an ack from Eli as well.

-- 
Pedro Alves

Re: [PATCH 1/3] gdb.trace: Save XML target description in tfile.

[Pedro Alves]

On 02/10/2016 08:18 PM, Marcin Kościelnicki wrote:
> gdb/ChangeLog:
> 
> 	* ctf.c (ctf_write_tdesc): New function.
> 	(ctf_write_ops): Wire in ctf_write_tdesc.
> 	* tracefile-tfile.c (tfile_write_tdesc): New function.
> 	(tfile_write_ops): Wire in tfile_write_tdesc.
> 	* tracefile.c (trace_save): Call write_tdesc method.
> 	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
> 	* xml-tdesc.c (target_fetch_description_xml): New function.
> 	* xml-tdesc.h: Add target_fetch_description_xml prototype.

OK.

Thanks,
Pedro Alves

Re: [PATCH 2/3] gdb.trace: Read XML target description from tfile.

[Marcin Kościelnicki]

On 10/02/16 23:20, Pedro Alves wrote:
> On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
>> gdb/ChangeLog:
>>
>> 	* tracefile-tfile.c (trace_tdesc): New static variable.
>> 	(tfile_open): Clear trace_tdesc, call target_find_description.
>> 	(tfile_interp_line): Recognize tdesc lines.
>> 	(tfile_close): Clear trace_tdesc.
>> 	(tfile_xfer_partial_features): New function.
>> 	(tfile_xfer_partial): Call tfile_xfer_partial_features.
>> 	(tfile_append_tdesc_line): New function.
>> ---
>> Fixes applied, buffer used.
>
> Thanks.  This is OK, with...
>
>> +/* Handles tdesc lines from tfile by appending the payload to
>> +   a global trace_tdesc variable.  */
>> +
>> +static void
>> +tfile_append_tdesc_line (const char *line)
>> +{
>> +  int llen = strlen (line);
>> +
>
> ... this unused var dropped.
>
>> +  buffer_grow_str (&trace_tdesc, line);
>> +  buffer_grow_str (&trace_tdesc, "\n");
>> +}
>
>
> Thanks,
> Pedro Alves
>

Thanks, fixed and pushed.

Re: [PATCH 1/3] gdb.trace: Save XML target description in tfile.

[Marcin Kościelnicki]

On 10/02/16 23:20, Pedro Alves wrote:
> On 02/10/2016 08:18 PM, Marcin Kościelnicki wrote:
>> gdb/ChangeLog:
>>
>> 	* ctf.c (ctf_write_tdesc): New function.
>> 	(ctf_write_ops): Wire in ctf_write_tdesc.
>> 	* tracefile-tfile.c (tfile_write_tdesc): New function.
>> 	(tfile_write_ops): Wire in tfile_write_tdesc.
>> 	* tracefile.c (trace_save): Call write_tdesc method.
>> 	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
>> 	* xml-tdesc.c (target_fetch_description_xml): New function.
>> 	* xml-tdesc.h: Add target_fetch_description_xml prototype.
>
> OK.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

[PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Marcin Kościelnicki]

This tests whether $ymm15 can be correctly collected and printed from
tfile.  It covers:

- storing tdesc in tfile (without that, $ymm15 doesn't exist)
- ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
  collected)
- register order in tfile_fetch_registers (without that, $ymm15h is
  fetched from wrong position)
- off-by-one in tfile_fetch_registers (without that, $ymm15h is
  incorrectly considered to be out of bounds)
- using proper tdesc in encoding tracepoint actions (without that,
  internal error happens due to $ymm15h being considered unavailable)

gdb/testsuite/ChangeLog:

	* gdb.trace/tfile-avx.c: New test.
	* gdb.trace/tfile-avx.exp: New test.
---
Fixes applied.

 gdb/testsuite/ChangeLog               |  5 +++
 gdb/testsuite/gdb.trace/tfile-avx.c   | 51 ++++++++++++++++++++++++
 gdb/testsuite/gdb.trace/tfile-avx.exp | 73 +++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.c
 create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.exp

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c2e3466..dfded59 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-02-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdb.trace/tfile-avx.c: New test.
+	* gdb.trace/tfile-avx.exp: New test.
+
 2016-02-09  Keith Seitz  <keiths@redhat.com>
 
 	PR breakpoints/19546
diff --git a/gdb/testsuite/gdb.trace/tfile-avx.c b/gdb/testsuite/gdb.trace/tfile-avx.c
new file mode 100644
index 0000000..212c556
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/tfile-avx.c
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   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/>.  */
+
+/*
+ * Test program for reading target description from tfile: collects AVX
+ * registers on x86_64.
+ */
+
+#include <immintrin.h>
+
+void
+dummy (void)
+{
+}
+
+static void
+end (void)
+{
+}
+
+int
+main (void)
+{
+  register __v8si a asm("ymm15") = {
+    0x12340001,
+    0x12340002,
+    0x12340003,
+    0x12340004,
+    0x12340005,
+    0x12340006,
+    0x12340007,
+    0x12340008,
+  };
+  asm volatile ("traceme: call dummy" : : "x" (a));
+  end ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/tfile-avx.exp b/gdb/testsuite/gdb.trace/tfile-avx.exp
new file mode 100644
index 0000000..4c52c64
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/tfile-avx.exp
@@ -0,0 +1,73 @@
+# Copyright 2016 Free Software Foundation, Inc.
+# 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/>.
+
+if { ! [is_amd64_regs_target] } {
+    verbose "Skipping tfile AVX test (target is not x86_64)."
+    return
+}
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile \
+     [list debug additional_flags=-mavx]]} {
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1
+}
+
+gdb_test_multiple "print \$ymm15" "check for AVX support" {
+    -re " = void.*$gdb_prompt $" {
+	verbose "Skipping tfile AVX test (target doesn't support AVX)."
+	return
+    }
+    -re " = \\{.*}.*$gdb_prompt $" {
+	# All is well.
+    }
+}
+
+gdb_test "trace traceme" ".*"
+
+gdb_trace_setactions "set actions for tracepoint" "" \
+	"collect \$ymm15" "^$"
+
+gdb_breakpoint "end"
+
+gdb_test_no_output "tstart"
+
+gdb_test "continue" ".*Breakpoint $decimal, end .*"
+
+set tracefile [standard_output_file ${testfile}]
+
+# Save trace frames to tfile.
+gdb_test "tsave ${tracefile}.tf" \
+    "Trace data saved to file '${tracefile}.tf'.*" \
+    "save tfile trace file"
+
+# Change target to tfile.
+gdb_test "target tfile ${tracefile}.tf" "" "change to tfile target" \
+  "A program is being debugged already.  Kill it. .y or n. $" "y"
+
+gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
+
+gdb_test "print/x \$ymm15.v8_int32" " = \\{0x12340001, .*, 0x12340008}"
-- 
2.7.0

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Pedro Alves]

On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
> This tests whether $ymm15 can be correctly collected and printed from
> tfile.  It covers:
> 
> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>   collected)
> - register order in tfile_fetch_registers (without that, $ymm15h is
>   fetched from wrong position)
> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>   incorrectly considered to be out of bounds)
> - using proper tdesc in encoding tracepoint actions (without that,
>   internal error happens due to $ymm15h being

OK once prereqs are in.

Thanks,
Pedro Alves

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Marcin Kościelnicki]

On 11/02/16 14:00, Pedro Alves wrote:
> On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
>> This tests whether $ymm15 can be correctly collected and printed from
>> tfile.  It covers:
>>
>> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
>> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>>    collected)
>> - register order in tfile_fetch_registers (without that, $ymm15h is
>>    fetched from wrong position)
>> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>>    incorrectly considered to be out of bounds)
>> - using proper tdesc in encoding tracepoint actions (without that,
>>    internal error happens due to $ymm15h being
>
> OK once prereqs are in.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

Re: [PATCH 0/4] Save target description in tfile.

[Yao Qi]

Marcin Kościelnicki <koriakin@0x04.net> writes:

> 1. target.xml is read from the target.
> 2. Includes are processed, resulting in a single in-memory XML file
>    containing all the data.
> 3. The resulting file is stored in tfile header by prefixing every line
>    with "tdesc ".  We may insert a spurious newline at the end of file
>    with this encoding, but that won't matter for XML.
> 4. When tfile is read, the XML is stored in an allocated buffer, and
>    xfer for TARGET_OBJECT_AVAILABLE_FEATURES is implemented, reading
>    from it.
> 5. target_find_description is called to force reading it.

The target description is a global state in trace file, so it doesn't
work tracepoints have different target descriptions, for example in
multi-inferior case, one is x86 and the other one is x86_64 (and arm vs
aarch64 too), and we have tracepoints set in two processes.

I know tracepoint doesn't support multi-inferior/multi-process, but we
may support that in the future.  Since we are changing tracepoint file
format, we'd better think about this a little bit.  Can we save multiple
target descriptions in trace file, and associate the tracepoints to the
right target description in trace file?

-- 
Yao (齐尧)

Re: [PATCH 1/3] gdb.trace: Save XML target description in tfile.

[Simon Marchi]

On 16-02-10 05:35 PM, Marcin Kościelnicki wrote:
> On 10/02/16 23:20, Pedro Alves wrote:
>> On 02/10/2016 08:18 PM, Marcin Kościelnicki wrote:
>>> gdb/ChangeLog:
>>>
>>> 	* ctf.c (ctf_write_tdesc): New function.
>>> 	(ctf_write_ops): Wire in ctf_write_tdesc.
>>> 	* tracefile-tfile.c (tfile_write_tdesc): New function.
>>> 	(tfile_write_ops): Wire in tfile_write_tdesc.
>>> 	* tracefile.c (trace_save): Call write_tdesc method.
>>> 	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
>>> 	* xml-tdesc.c (target_fetch_description_xml): New function.
>>> 	* xml-tdesc.h: Add target_fetch_description_xml prototype.
>>
>> OK.
>>
>> Thanks,
>> Pedro Alves
>>
> 
> Thanks, pushed.
> 

Hi Marcin,

Did you get an email from Builbot about this patch?

An unrelated patch did fail because of this one:
http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/3021/steps/compile%20gdb/logs/stdio

It happens when you build without expat.  Maybe this particular slave doesn't have expat-devel
installed, but others do.  You can reproduce it locally by configuring with --with-expat=no or
--without-expat.

@Sergio: To ensure that we always test in the same conditions, perhaps we could add --with-expat,
--with-python, and maybe others?  Can it be done on a per-builder basis?

This way, if the slave for a builder is mis-configured (doesn't have development packages for
those), configure will fail right away.  In an ideal world, to catch problems like the current
one though, we should test gdb (at least see if it builds) with various combinations of
with/without python, expat, etc.

Re: [PATCH 1/3] gdb.trace: Save XML target description in tfile.

[Sergio Durigan Junior]

On Thursday, February 11 2016, Simon Marchi wrote:

> On 16-02-10 05:35 PM, Marcin Kościelnicki wrote:
>> On 10/02/16 23:20, Pedro Alves wrote:
>>> On 02/10/2016 08:18 PM, Marcin Kościelnicki wrote:
>>>> gdb/ChangeLog:
>>>>
>>>> 	* ctf.c (ctf_write_tdesc): New function.
>>>> 	(ctf_write_ops): Wire in ctf_write_tdesc.
>>>> 	* tracefile-tfile.c (tfile_write_tdesc): New function.
>>>> 	(tfile_write_ops): Wire in tfile_write_tdesc.
>>>> 	* tracefile.c (trace_save): Call write_tdesc method.
>>>> 	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
>>>> 	* xml-tdesc.c (target_fetch_description_xml): New function.
>>>> 	* xml-tdesc.h: Add target_fetch_description_xml prototype.
>>>
>>> OK.
>>>
>>> Thanks,
>>> Pedro Alves
>>>
>> 
>> Thanks, pushed.
>> 
>
> Hi Marcin,
>
> Did you get an email from Builbot about this patch?
>
> An unrelated patch did fail because of this one:
> http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/3021/steps/compile%20gdb/logs/stdio
>
> It happens when you build without expat.  Maybe this particular slave doesn't have expat-devel
> installed, but others do.  You can reproduce it locally by configuring with --with-expat=no or
> --without-expat.

This was my fault.  I run two Fedora buildslaves for x86, and one of
them did not have the i686 packages installed.  Everything should be
fine now.

BTW, I am really interested in knowing if Marcin received an e-mail from
BuildBot.  Simon, did you receive an e-mail as well?

> @Sergio: To ensure that we always test in the same conditions, perhaps we could add --with-expat,
> --with-python, and maybe others?  Can it be done on a per-builder basis?

This could be a good idea.  One of the problems I have is ensure that
all the buildslaves have a minimum set of packages installed for
testing.  I tried listing all the packages I could in the wiki:

  <https://sourceware.org/gdb/wiki/BuildBot#Buildslave_configuration>

But it's not easy to guarantee that all of them are following this.
Enforcing compliance through the use of some flags (--with-*) could help
us to bring everybody up to speed.  The only problem is that we'll see
some failures happening when a buildslave is not configured properly,
and that may seem like a failure introduced by a patch.  But I'm
guessing they'll be quick to fix.

> This way, if the slave for a builder is mis-configured (doesn't have development packages for
> those), configure will fail right away.  In an ideal world, to catch problems like the current
> one though, we should test gdb (at least see if it builds) with various combinations of
> with/without python, expat, etc.

The problem is that we don't have enough buildslaves now to handle
several combinations of flags.  For now, I think it's enough to test the
"normal" configuration.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 1/3] gdb.trace: Save XML target description in tfile.

[Marcin Kościelnicki]

On 11/02/16 20:02, Simon Marchi wrote:
> On 16-02-10 05:35 PM, Marcin Kościelnicki wrote:
>> On 10/02/16 23:20, Pedro Alves wrote:
>>> On 02/10/2016 08:18 PM, Marcin Kościelnicki wrote:
>>>> gdb/ChangeLog:
>>>>
>>>> 	* ctf.c (ctf_write_tdesc): New function.
>>>> 	(ctf_write_ops): Wire in ctf_write_tdesc.
>>>> 	* tracefile-tfile.c (tfile_write_tdesc): New function.
>>>> 	(tfile_write_ops): Wire in tfile_write_tdesc.
>>>> 	* tracefile.c (trace_save): Call write_tdesc method.
>>>> 	* tracefile.h (struct trace_file_write_ops): Add write_tdesc method.
>>>> 	* xml-tdesc.c (target_fetch_description_xml): New function.
>>>> 	* xml-tdesc.h: Add target_fetch_description_xml prototype.
>>>
>>> OK.
>>>
>>> Thanks,
>>> Pedro Alves
>>>
>>
>> Thanks, pushed.
>>
>
> Hi Marcin,
>
> Did you get an email from Builbot about this patch?
>
> An unrelated patch did fail because of this one:
> http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/3021/steps/compile%20gdb/logs/stdio
>
> It happens when you build without expat.  Maybe this particular slave doesn't have expat-devel
> installed, but others do.  You can reproduce it locally by configuring with --with-expat=no or
> --without-expat.
>
> @Sergio: To ensure that we always test in the same conditions, perhaps we could add --with-expat,
> --with-python, and maybe others?  Can it be done on a per-builder basis?
>
> This way, if the slave for a builder is mis-configured (doesn't have development packages for
> those), configure will fail right away.  In an ideal world, to catch problems like the current
> one though, we should test gdb (at least see if it builds) with various combinations of
> with/without python, expat, etc.
>

I didn't get any email about the breakage before this one.

Thanks for noticing that.  Sorry for the problem, I'll submit a patch soon.

[PATCH] gdb: Fix build failure in xml-tdesc.c without expat.

[Marcin Kościelnicki]

Introduced by 18d3cec54e1b4fce278dba436484846f8048d7d6.

gdb/ChangeLog:

	* xml-tdesc.c (target_fetch_description_xml): Return NULL when
	built without expat.
---
OK to push?  Or should we emit a warning in this case?

 gdb/ChangeLog   | 5 +++++
 gdb/xml-tdesc.c | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5edafdf..4856db5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-02-11  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* xml-tdesc.c (target_fetch_description_xml): Return NULL when
+	built without expat.
+
 2016-02-11  Pedro Alves  <palves@redhat.com>
 
 	* Makefile.in (check-parallel): New rule.
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 4625b60..22cf26b 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -638,6 +638,9 @@ target_read_description_xml (struct target_ops *ops)
 char *
 target_fetch_description_xml (struct target_ops *ops)
 {
+#if !defined(HAVE_LIBEXPAT)
+  return NULL;
+#else
   struct target_desc *tdesc;
   char *tdesc_str;
   char *expanded_text;
@@ -659,4 +662,5 @@ target_fetch_description_xml (struct target_ops *ops)
     }
 
   return expanded_text;
+#endif
 }
-- 
2.7.0

Re: [PATCH 0/4] Save target description in tfile.

[Marcin Kościelnicki]

On 11/02/16 18:36, Yao Qi wrote:
> Marcin Kościelnicki <koriakin@0x04.net> writes:
>
>> 1. target.xml is read from the target.
>> 2. Includes are processed, resulting in a single in-memory XML file
>>     containing all the data.
>> 3. The resulting file is stored in tfile header by prefixing every line
>>     with "tdesc ".  We may insert a spurious newline at the end of file
>>     with this encoding, but that won't matter for XML.
>> 4. When tfile is read, the XML is stored in an allocated buffer, and
>>     xfer for TARGET_OBJECT_AVAILABLE_FEATURES is implemented, reading
>>     from it.
>> 5. target_find_description is called to force reading it.
>
> The target description is a global state in trace file, so it doesn't
> work tracepoints have different target descriptions, for example in
> multi-inferior case, one is x86 and the other one is x86_64 (and arm vs
> aarch64 too), and we have tracepoints set in two processes.
>
> I know tracepoint doesn't support multi-inferior/multi-process, but we
> may support that in the future.  Since we are changing tracepoint file
> format, we'd better think about this a little bit.  Can we save multiple
> target descriptions in trace file, and associate the tracepoints to the
> right target description in trace file?
>
Well, that'd require rethinking the whole format - there's already the 
'R' line, which is also global and specifies register packet size, which 
is probably different for every target (it's effectively a very poor 
kind of tdesc).  There's also no way of assigning the tracepoints to 
inferiors, or for that matter to say what executable is running in what 
inferior (target tfile just picks the current one loaded in gdb).

In effect, we'd have to prefix every single line with the inferior id 
(or otherwise include it), and the result is probably no better than 
just bundling several tfile files eg. in a zip (I don't think we have 
support for merging trace buffers from several sources anyhow).

Re: [PATCH] gdb: Fix build failure in xml-tdesc.c without expat.

[Pedro Alves]

On 02/11/2016 10:56 PM, Marcin Kościelnicki wrote:
> Introduced by 18d3cec54e1b4fce278dba436484846f8048d7d6.
> 
> gdb/ChangeLog:
> 
> 	* xml-tdesc.c (target_fetch_description_xml): Return NULL when
> 	built without expat.
> ---
> OK to push?  Or should we emit a warning in this case?

Yes, I think we should emit a warning.

Thanks,
Pedro Alves

[PATCH] gdb: Fix build failure in xml-tdesc.c without expat.

[Marcin Kościelnicki]

Introduced by 18d3cec54e1b4fce278dba436484846f8048d7d6.

gdb/ChangeLog:

	* xml-tdesc.c (target_fetch_description_xml): Warn and return NULL
	when built without expat.
---
How about that?

 gdb/ChangeLog   |  5 +++++
 gdb/xml-tdesc.c | 13 +++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e06fc3c..7e59266 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-02-12  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* xml-tdesc.c (target_fetch_description_xml): Return NULL when
+	built without expat.
+
 2016-02-12  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* frame.h (skip_tailcall_frames): Update comment.
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 4625b60..b5439e5 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -638,6 +638,18 @@ target_read_description_xml (struct target_ops *ops)
 char *
 target_fetch_description_xml (struct target_ops *ops)
 {
+#if !defined(HAVE_LIBEXPAT)
+  static int have_warned;
+
+  if (!have_warned)
+    {
+      have_warned = 1;
+      warning (_("Can not fetch XML target description; XML support was "
+		 "disabled at compile time"));
+    }
+
+  return NULL;
+#else
   struct target_desc *tdesc;
   char *tdesc_str;
   char *expanded_text;
@@ -659,4 +671,5 @@ target_fetch_description_xml (struct target_ops *ops)
     }
 
   return expanded_text;
+#endif
 }
-- 
2.7.0

Re: [PATCH] gdb: Fix build failure in xml-tdesc.c without expat.

[Pedro Alves]

On 02/12/2016 10:09 AM, Marcin Kościelnicki wrote:
> Introduced by 18d3cec54e1b4fce278dba436484846f8048d7d6.
> 
> gdb/ChangeLog:
> 
> 	* xml-tdesc.c (target_fetch_description_xml): Warn and return NULL
> 	when built without expat.
> ---
> How about that?

There's a standard CL format for conditionally compiled code:

	* xml-tdesc.c (target_fetch_description_xml) [!HAVE_LIBEXPAT]: Warn
	and return NULL.

OK with that change.

Thanks,
Pedro Alves

Re: [PATCH] gdb: Fix build failure in xml-tdesc.c without expat.

[Marcin Kościelnicki]

On 12/02/16 11:19, Pedro Alves wrote:
> On 02/12/2016 10:09 AM, Marcin Kościelnicki wrote:
>> Introduced by 18d3cec54e1b4fce278dba436484846f8048d7d6.
>>
>> gdb/ChangeLog:
>>
>> 	* xml-tdesc.c (target_fetch_description_xml): Warn and return NULL
>> 	when built without expat.
>> ---
>> How about that?
>
> There's a standard CL format for conditionally compiled code:
>
> 	* xml-tdesc.c (target_fetch_description_xml) [!HAVE_LIBEXPAT]: Warn
> 	and return NULL.
>
> OK with that change.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

Re: [PATCH 0/4] Save target description in tfile.

[Yao Qi]

Marcin Kościelnicki <koriakin@0x04.net> writes:

> Well, that'd require rethinking the whole format - there's already the
> R' line, which is also global and specifies register packet size,
> which is probably different for every target (it's effectively a very
> poor kind of tdesc).  There's also no way of assigning the tracepoints
> to inferiors, or for that matter to say what executable is running in
> what inferior (target tfile just picks the current one loaded in gdb).

Right, target file just chooses the current inferior in gdb, so the
trace file can not be used in a multi-inferior case, although it can be
generated in a multi-inferior case potentially.

>
> In effect, we'd have to prefix every single line with the inferior id
> (or otherwise include it), and the result is probably no better than
> just bundling several tfile files eg. in a zip (I don't think we have
> support for merging trace buffers from several sources anyhow).

Yes, generate trace files for each inferior is feasible, and the tdesc
can be the global state of each trace file.

-- 
Yao (齐尧)

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Antoine Tremblay]

Marcin Kościelnicki writes:

> On 11/02/16 14:00, Pedro Alves wrote:
>> On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
>>> This tests whether $ymm15 can be correctly collected and printed from
>>> tfile.  It covers:
>>>
>>> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
>>> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>>>    collected)
>>> - register order in tfile_fetch_registers (without that, $ymm15h is
>>>    fetched from wrong position)
>>> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>>>    incorrectly considered to be out of bounds)
>>> - using proper tdesc in encoding tracepoint actions (without that,
>>>    internal error happens due to $ymm15h being
>>
>> OK once prereqs are in.
>>
>> Thanks,
>> Pedro Alves
>>
>
> Thanks, pushed.

Hi,
  I've been trying to run this test on x86 but I get the following error
  while compiling tfile-avx.c :

 binutils-gdb/build-x86/gdb/testsuite/../../../gdb/testsuite/gdb.trace/tfile-avx.c:38:19: error: invalid register name for 'a'
  register __v8si a asm("ymm15") = {
                   ^

  I've also noticed the same error on the buildbot results see:
  http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/2928/steps/test%20gdb/logs/stdio

  My cpu (Intel(R) Core(TM) i7-4600M ) supports avx, cat /proc/cpuinfo
  shows avx and a gdb print $ymm15 returns something...

  This is with gcc 4.8.4...

  Am I missing something?

Regards,
Antoine

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Marcin Kościelnicki]

On 12/02/16 19:31, Antoine Tremblay wrote:
>
> Marcin Kościelnicki writes:
>
>> On 11/02/16 14:00, Pedro Alves wrote:
>>> On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
>>>> This tests whether $ymm15 can be correctly collected and printed from
>>>> tfile.  It covers:
>>>>
>>>> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
>>>> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>>>>     collected)
>>>> - register order in tfile_fetch_registers (without that, $ymm15h is
>>>>     fetched from wrong position)
>>>> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>>>>     incorrectly considered to be out of bounds)
>>>> - using proper tdesc in encoding tracepoint actions (without that,
>>>>     internal error happens due to $ymm15h being
>>>
>>> OK once prereqs are in.
>>>
>>> Thanks,
>>> Pedro Alves
>>>
>>
>> Thanks, pushed.
>
> Hi,
>    I've been trying to run this test on x86 but I get the following error
>    while compiling tfile-avx.c :
>
>   binutils-gdb/build-x86/gdb/testsuite/../../../gdb/testsuite/gdb.trace/tfile-avx.c:38:19: error: invalid register name for 'a'
>    register __v8si a asm("ymm15") = {
>                     ^
>
>    I've also noticed the same error on the buildbot results see:
>    http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/2928/steps/test%20gdb/logs/stdio
>
>    My cpu (Intel(R) Core(TM) i7-4600M ) supports avx, cat /proc/cpuinfo
>    shows avx and a gdb print $ymm15 returns something...
>
>    This is with gcc 4.8.4...
>
>    Am I missing something?
>
> Regards,
> Antoine
>

Ugh.  It seems you need a newer gcc to recognize "ymm15" as a register 
name - 4.8.2 seems to want it called "xmm15" - sort of incorrect, but 
close enough.  gcc 5.3 still accepts that, so perhaps we should change 
it to xmm15 for the sake of older compilers, even if it harms readability?

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Antoine Tremblay]

Marcin Kościelnicki writes:

> On 12/02/16 19:31, Antoine Tremblay wrote:
>>
>> Marcin Kościelnicki writes:
>>
>>> On 11/02/16 14:00, Pedro Alves wrote:
>>>> On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
>>>>> This tests whether $ymm15 can be correctly collected and printed from
>>>>> tfile.  It covers:
>>>>>
>>>>> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
>>>>> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>>>>>     collected)
>>>>> - register order in tfile_fetch_registers (without that, $ymm15h is
>>>>>     fetched from wrong position)
>>>>> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>>>>>     incorrectly considered to be out of bounds)
>>>>> - using proper tdesc in encoding tracepoint actions (without that,
>>>>>     internal error happens due to $ymm15h being
>>>>
>>>> OK once prereqs are in.
>>>>
>>>> Thanks,
>>>> Pedro Alves
>>>>
>>>
>>> Thanks, pushed.
>>
>> Hi,
>>    I've been trying to run this test on x86 but I get the following error
>>    while compiling tfile-avx.c :
>>
>>   binutils-gdb/build-x86/gdb/testsuite/../../../gdb/testsuite/gdb.trace/tfile-avx.c:38:19: error: invalid register name for 'a'
>>    register __v8si a asm("ymm15") = {
>>                     ^
>>
>>    I've also noticed the same error on the buildbot results see:
>>    http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/2928/steps/test%20gdb/logs/stdio
>>
>>    My cpu (Intel(R) Core(TM) i7-4600M ) supports avx, cat /proc/cpuinfo
>>    shows avx and a gdb print $ymm15 returns something...
>>
>>    This is with gcc 4.8.4...
>>
>>    Am I missing something?
>>
>> Regards,
>> Antoine
>>
>
> Ugh.  It seems you need a newer gcc to recognize "ymm15" as a register 
> name - 4.8.2 seems to want it called "xmm15" - sort of incorrect, but 
> close enough.  gcc 5.3 still accepts that, so perhaps we should change 
> it to xmm15 for the sake of older compilers, even if it harms readability?

Would xmm15 still work on newer gccs ? If so I would guess it's a good
idea to change it given that our own buildbot test machines seem to test
with an older gcc...?

Maybe adding a note of it in the test... or if there's a way to check
for the gcc version ?

I actually can't find the gcc doc where those names are defined at the
moment would you have that handy by any chance?

Regards,
Antoine

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Marcin Kościelnicki]

On 12/02/16 19:49, Antoine Tremblay wrote:
>
> Marcin Kościelnicki writes:
>
>> On 12/02/16 19:31, Antoine Tremblay wrote:
>>>
>>> Marcin Kościelnicki writes:
>>>
>>>> On 11/02/16 14:00, Pedro Alves wrote:
>>>>> On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
>>>>>> This tests whether $ymm15 can be correctly collected and printed from
>>>>>> tfile.  It covers:
>>>>>>
>>>>>> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
>>>>>> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>>>>>>      collected)
>>>>>> - register order in tfile_fetch_registers (without that, $ymm15h is
>>>>>>      fetched from wrong position)
>>>>>> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>>>>>>      incorrectly considered to be out of bounds)
>>>>>> - using proper tdesc in encoding tracepoint actions (without that,
>>>>>>      internal error happens due to $ymm15h being
>>>>>
>>>>> OK once prereqs are in.
>>>>>
>>>>> Thanks,
>>>>> Pedro Alves
>>>>>
>>>>
>>>> Thanks, pushed.
>>>
>>> Hi,
>>>     I've been trying to run this test on x86 but I get the following error
>>>     while compiling tfile-avx.c :
>>>
>>>    binutils-gdb/build-x86/gdb/testsuite/../../../gdb/testsuite/gdb.trace/tfile-avx.c:38:19: error: invalid register name for 'a'
>>>     register __v8si a asm("ymm15") = {
>>>                      ^
>>>
>>>     I've also noticed the same error on the buildbot results see:
>>>     http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/2928/steps/test%20gdb/logs/stdio
>>>
>>>     My cpu (Intel(R) Core(TM) i7-4600M ) supports avx, cat /proc/cpuinfo
>>>     shows avx and a gdb print $ymm15 returns something...
>>>
>>>     This is with gcc 4.8.4...
>>>
>>>     Am I missing something?
>>>
>>> Regards,
>>> Antoine
>>>
>>
>> Ugh.  It seems you need a newer gcc to recognize "ymm15" as a register
>> name - 4.8.2 seems to want it called "xmm15" - sort of incorrect, but
>> close enough.  gcc 5.3 still accepts that, so perhaps we should change
>> it to xmm15 for the sake of older compilers, even if it harms readability?
>
> Would xmm15 still work on newer gccs ? If so I would guess it's a good
> idea to change it given that our own buildbot test machines seem to test
> with an older gcc...?

Both xmm15 and ymm15 work just fine on gcc 4.9.3 and 5.3.0, which are 
the only ones I have around right now.  So, let's do it.
>
> Maybe adding a note of it in the test... or if there's a way to check
> for the gcc version ?

I'll throw in a comment.
>
> I actually can't find the gcc doc where those names are defined at the
> moment would you have that handy by any chance?

gcc/config/i386/i386.h, ADDITIONAL_REGISTER_NAMES.
>
> Regards,
> Antoine
>

Re: [PATCH] gdb.trace: Add a testcase for tdesc in tfile.

[Antoine Tremblay]

Marcin Kościelnicki writes:

> On 12/02/16 19:49, Antoine Tremblay wrote:
>>
>> Marcin Kościelnicki writes:
>>
>>> On 12/02/16 19:31, Antoine Tremblay wrote:
>>>>
>>>> Marcin Kościelnicki writes:
>>>>
>>>>> On 11/02/16 14:00, Pedro Alves wrote:
>>>>>> On 02/11/2016 10:14 AM, Marcin Kościelnicki wrote:
>>>>>>> This tests whether $ymm15 can be correctly collected and printed from
>>>>>>> tfile.  It covers:
>>>>>>>
>>>>>>> - storing tdesc in tfile (without that, $ymm15 doesn't exist)
>>>>>>> - ax_pseudo_register_collect for x86 (without that, $ymm15 cannot be
>>>>>>>      collected)
>>>>>>> - register order in tfile_fetch_registers (without that, $ymm15h is
>>>>>>>      fetched from wrong position)
>>>>>>> - off-by-one in tfile_fetch_registers (without that, $ymm15h is
>>>>>>>      incorrectly considered to be out of bounds)
>>>>>>> - using proper tdesc in encoding tracepoint actions (without that,
>>>>>>>      internal error happens due to $ymm15h being
>>>>>>
>>>>>> OK once prereqs are in.
>>>>>>
>>>>>> Thanks,
>>>>>> Pedro Alves
>>>>>>
>>>>>
>>>>> Thanks, pushed.
>>>>
>>>> Hi,
>>>>     I've been trying to run this test on x86 but I get the following error
>>>>     while compiling tfile-avx.c :
>>>>
>>>>    binutils-gdb/build-x86/gdb/testsuite/../../../gdb/testsuite/gdb.trace/tfile-avx.c:38:19: error: invalid register name for 'a'
>>>>     register __v8si a asm("ymm15") = {
>>>>                      ^
>>>>
>>>>     I've also noticed the same error on the buildbot results see:
>>>>     http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/2928/steps/test%20gdb/logs/stdio
>>>>
>>>>     My cpu (Intel(R) Core(TM) i7-4600M ) supports avx, cat /proc/cpuinfo
>>>>     shows avx and a gdb print $ymm15 returns something...
>>>>
>>>>     This is with gcc 4.8.4...
>>>>
>>>>     Am I missing something?
>>>>
>>>> Regards,
>>>> Antoine
>>>>
>>>
>>> Ugh.  It seems you need a newer gcc to recognize "ymm15" as a register
>>> name - 4.8.2 seems to want it called "xmm15" - sort of incorrect, but
>>> close enough.  gcc 5.3 still accepts that, so perhaps we should change
>>> it to xmm15 for the sake of older compilers, even if it harms readability?
>>
>> Would xmm15 still work on newer gccs ? If so I would guess it's a good
>> idea to change it given that our own buildbot test machines seem to test
>> with an older gcc...?
>
> Both xmm15 and ymm15 work just fine on gcc 4.9.3 and 5.3.0, which are 
> the only ones I have around right now.  So, let's do it.

Great, sorry I had missed that in your original mail.

>>
>> Maybe adding a note of it in the test... or if there's a way to check
>> for the gcc version ?
>
> I'll throw in a comment.
>>
>> I actually can't find the gcc doc where those names are defined at the
>> moment would you have that handy by any chance?
>
> gcc/config/i386/i386.h, ADDITIONAL_REGISTER_NAMES.
>>

Thanks!
Antoine

[PATCH] gdb.trace/tfile-avx.c: Change ymm15 to xmm15 for old gcc.

[Marcin Kościelnicki]

gcc older than 4.9 doesn't understand ymm15 as a register name.  Use
xmm15 instead.

gdb/testsuite/ChangeLog:

	* gdb.trace/tfile-avx.c (main): Change ymm15 to xmm15.
---
OK to push?

 gdb/testsuite/ChangeLog             | 4 ++++
 gdb/testsuite/gdb.trace/tfile-avx.c | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 89aa2d7..6ef8932 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2016-02-12  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdb.trace/tfile-avx.c (main): Change ymm15 to xmm15.
+
 2016-02-12  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* i386-biarch-core.exp: Define corefile using
diff --git a/gdb/testsuite/gdb.trace/tfile-avx.c b/gdb/testsuite/gdb.trace/tfile-avx.c
index 212c556..3cc3ec0 100644
--- a/gdb/testsuite/gdb.trace/tfile-avx.c
+++ b/gdb/testsuite/gdb.trace/tfile-avx.c
@@ -35,7 +35,9 @@ end (void)
 int
 main (void)
 {
-  register __v8si a asm("ymm15") = {
+  /* Strictly speaking, it should be ymm15 (xmm15 is 128-bit), but gcc older
+     than 4.9 doesn't recognize "ymm15" as a valid register name.  */
+  register __v8si a asm("xmm15") = {
     0x12340001,
     0x12340002,
     0x12340003,
-- 
2.7.0

Re: [PATCH] gdb.trace/tfile-avx.c: Change ymm15 to xmm15 for old gcc.

[Pedro Alves]

On 02/12/2016 07:47 PM, Marcin Kościelnicki wrote:
> gcc older than 4.9 doesn't understand ymm15 as a register name.  Use
> xmm15 instead.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.trace/tfile-avx.c (main): Change ymm15 to xmm15.
> ---
> OK to push?
> 

OK.

Thanks,
Pedro Alves

Re: [PATCH] gdb.trace/tfile-avx.c: Change ymm15 to xmm15 for old gcc.

[Marcin Kościelnicki]

On 12/02/16 20:57, Pedro Alves wrote:
> On 02/12/2016 07:47 PM, Marcin Kościelnicki wrote:
>> gcc older than 4.9 doesn't understand ymm15 as a register name.  Use
>> xmm15 instead.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.trace/tfile-avx.c (main): Change ymm15 to xmm15.
>> ---
>> OK to push?
>>
>
> OK.
>
> Thanks,
> Pedro Alves
>

Thanks, pushed.

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Marcin Kościelnicki]

On 10/02/16 23:20, Pedro Alves wrote:
> On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
>> gdb/doc/ChangeLog:
>>
>> 	* gdb.texinfo (Trace File Format): Add documentation for description
>> 	section lines.
>> ---
>> Here's the promised documentation for tdesc lines and then some.
>
> Excellent!  Thanks much.
>
> Looks good to me content-wise, but let's get an ack from Eli as well.
>

Ping?

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Pedro Alves]

[+Eli]

Original at:
 https://sourceware.org/ml/gdb-patches/2016-02/msg00320.html

Thanks,
Pedro Alves

On 02/17/2016 09:50 AM, Marcin Kościelnicki wrote:
> On 10/02/16 23:20, Pedro Alves wrote:
>> On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
>>> gdb/doc/ChangeLog:
>>>
>>> 	* gdb.texinfo (Trace File Format): Add documentation for description
>>> 	section lines.
>>> ---
>>> Here's the promised documentation for tdesc lines and then some.
>>
>> Excellent!  Thanks much.
>>
>> Looks good to me content-wise, but let's get an ack from Eli as well.
>>
> 
> Ping?
> 

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Pedro Alves]

[Really +Eli ...]

Thanks,
Pedro Alves

On 02/17/2016 03:37 PM, Pedro Alves wrote:
> [+Eli]
> 
> Original at:
>  https://sourceware.org/ml/gdb-patches/2016-02/msg00320.html
> 
> Thanks,
> Pedro Alves
> 
> On 02/17/2016 09:50 AM, Marcin Kościelnicki wrote:
>> On 10/02/16 23:20, Pedro Alves wrote:
>>> On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
>>>> gdb/doc/ChangeLog:
>>>>
>>>> 	* gdb.texinfo (Trace File Format): Add documentation for description
>>>> 	section lines.
>>>> ---
>>>> Here's the promised documentation for tdesc lines and then some.
>>>
>>> Excellent!  Thanks much.
>>>
>>> Looks good to me content-wise, but let's get an ack from Eli as well.
>>>
>>
>> Ping?
>>
> 

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Marcin Kościelnicki <koriakin@0x04.net>
> Date: Wed, 17 Feb 2016 10:50:05 +0100
> 
> On 10/02/16 23:20, Pedro Alves wrote:
> > On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
> >> gdb/doc/ChangeLog:
> >>
> >> 	* gdb.texinfo (Trace File Format): Add documentation for description
> >> 	section lines.
> >> ---
> >> Here's the promised documentation for tdesc lines and then some.
> >
> > Excellent!  Thanks much.
> >
> > Looks good to me content-wise, but let's get an ack from Eli as well.
> >
> 
> Ping?

Sorry, missed that.

The patch is OK, but I wonder what about that FIXME there?

Thanks.

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Marcin Kościelnicki]

On 17/02/16 16:59, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Marcin Kościelnicki <koriakin@0x04.net>
>> Date: Wed, 17 Feb 2016 10:50:05 +0100
>>
>> On 10/02/16 23:20, Pedro Alves wrote:
>>> On 02/10/2016 08:19 PM, Marcin Kościelnicki wrote:
>>>> gdb/doc/ChangeLog:
>>>>
>>>> 	* gdb.texinfo (Trace File Format): Add documentation for description
>>>> 	section lines.
>>>> ---
>>>> Here's the promised documentation for tdesc lines and then some.
>>>
>>> Excellent!  Thanks much.
>>>
>>> Looks good to me content-wise, but let's get an ack from Eli as well.
>>>
>>
>> Ping?
>
> Sorry, missed that.
>
> The patch is OK, but I wonder what about that FIXME there?
>
> Thanks.
>

Yeah, the FIXME is worded a bit weirdly and I wondered about it too, but 
the only thing it could've referred to was the description section line 
types (the trace frame section is already documented), so I figured it'd 
be best to remove it.

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Pedro Alves]

On 02/17/2016 04:04 PM, Marcin Kościelnicki wrote:
> On 17/02/16 16:59, Eli Zaretskii wrote:

>> The patch is OK, but I wonder what about that FIXME there?
>>
>> Thanks.
>>
> 
> Yeah, the FIXME is worded a bit weirdly and I wondered about it too, but 
> the only thing it could've referred to was the description section line 
> types (the trace frame section is already documented), so I figured it'd 
> be best to remove it.
> 

I agree.

Thanks,
Pedro Alves

Re: [PATCH 3/3] gdb/doc: Add documentation for tfile description section lines.

[Marcin Kościelnicki]

On 17/02/16 17:05, Pedro Alves wrote:
> On 02/17/2016 04:04 PM, Marcin Kościelnicki wrote:
>> On 17/02/16 16:59, Eli Zaretskii wrote:
>
>>> The patch is OK, but I wonder what about that FIXME there?
>>>
>>> Thanks.
>>>
>>
>> Yeah, the FIXME is worded a bit weirdly and I wondered about it too, but
>> the only thing it could've referred to was the description section line
>> types (the trace frame section is already documented), so I figured it'd
>> be best to remove it.
>>
>
> I agree.
>
> Thanks,
> Pedro Alves
>

I've pushed the patch.