public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
  2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi
@ 2017-05-11 15:55 ` Yao Qi
  2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches

Target description initialization are called in -tdep.c, instead of -nat.c
I've posted it separately
https://sourceware.org/ml/gdb-patches/2017-05/msg00204.html, include
it here to give more context.

gdb:

2017-05-09  Yao Qi  <yao.qi@linaro.org>

	* mips-linux-nat.c: Move include features/mips*-linux.c to
	mips-linux-tdep.c.
	(_initialize_mips_linux_nat): Move initialize_tdesc_mips* calls
	to mips-linux-tdep.c.
	* mips-linux-tdep.c: Include features/mips*-linux.c
	(_initialize_mips_linux_tdep): Call initialize_tdesc_mips*
	functions.
	* mips-linux-tdep.h (tdesc_mips_linux): Declare.
	(tdesc_mips_dsp_linux, tdesc_mips64_linux): Declare.
	(tdesc_mips64_dsp_linux): Declare.
---
 gdb/mips-linux-nat.c  | 11 -----------
 gdb/mips-linux-tdep.c | 11 +++++++++++
 gdb/mips-linux-tdep.h |  6 ++++++
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
index 8041d84..1fd3365 100644
--- a/gdb/mips-linux-nat.c
+++ b/gdb/mips-linux-nat.c
@@ -38,11 +38,6 @@
 
 #include "nat/mips-linux-watch.h"
 
-#include "features/mips-linux.c"
-#include "features/mips-dsp-linux.c"
-#include "features/mips64-linux.c"
-#include "features/mips64-dsp-linux.c"
-
 #ifndef PTRACE_GET_THREAD_AREA
 #define PTRACE_GET_THREAD_AREA 25
 #endif
@@ -803,10 +798,4 @@ triggers a breakpoint or watchpoint."),
 
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, mips_linux_new_thread);
-
-  /* Initialize the standard target descriptions.  */
-  initialize_tdesc_mips_linux ();
-  initialize_tdesc_mips_dsp_linux ();
-  initialize_tdesc_mips64_linux ();
-  initialize_tdesc_mips64_dsp_linux ();
 }
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index 48a582a..0f22a92 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -40,6 +40,11 @@
 #include "xml-syscall.h"
 #include "gdb_signals.h"
 
+#include "features/mips-linux.c"
+#include "features/mips-dsp-linux.c"
+#include "features/mips64-linux.c"
+#include "features/mips64-dsp-linux.c"
+
 static struct target_so_ops mips_svr4_so_ops;
 
 /* This enum represents the signals' numbers on the MIPS
@@ -1764,4 +1769,10 @@ _initialize_mips_linux_tdep (void)
 			      GDB_OSABI_LINUX,
 			      mips_linux_init_abi);
     }
+
+  /* Initialize the standard target descriptions.  */
+  initialize_tdesc_mips_linux ();
+  initialize_tdesc_mips_dsp_linux ();
+  initialize_tdesc_mips64_linux ();
+  initialize_tdesc_mips64_dsp_linux ();
 }
diff --git a/gdb/mips-linux-tdep.h b/gdb/mips-linux-tdep.h
index 407b577..cca4798 100644
--- a/gdb/mips-linux-tdep.h
+++ b/gdb/mips-linux-tdep.h
@@ -105,3 +105,9 @@ enum {
 /* Return 1 if MIPS_RESTART_REGNUM is usable.  */
 
 int mips_linux_restart_reg_p (struct gdbarch *gdbarch);
+
+/* Target descriptions.  */
+extern struct target_desc *tdesc_mips_linux;
+extern struct target_desc *tdesc_mips64_linux;
+extern struct target_desc *tdesc_mips_dsp_linux;
+extern struct target_desc *tdesc_mips64_dsp_linux;
-- 
1.9.1

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

* [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
                   ` (2 preceding siblings ...)
  2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi
@ 2017-05-11 15:55 ` Yao Qi
  2017-05-11 15:55 ` [RFC 5/7] Centralize i386 linux target descriptions Yao Qi
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches

Exchange the order of 32bit-linux.xml and 32bit-sse.xml in
i386/i386-linux.xml, to align with other i386 linux .xml files.

gdb:

2017-04-27  Yao Qi  <yao.qi@linaro.org>

	* features/i386/i386-linux.xml: Exchange the order of including
	32bit-linux.xml and 32bit-sse.xml.
	* features/i386/i386-linux.c: Regenerated.
---
 gdb/features/i386/i386-linux.c   | 6 +++---
 gdb/features/i386/i386-linux.xml | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/features/i386/i386-linux.c b/gdb/features/i386/i386-linux.c
index 39328af..649dcd6 100644
--- a/gdb/features/i386/i386-linux.c
+++ b/gdb/features/i386/i386-linux.c
@@ -71,9 +71,6 @@ initialize_tdesc_i386_linux (void)
   tdesc_create_reg (feature, "fooff", 30, 1, "float", 32, "int");
   tdesc_create_reg (feature, "fop", 31, 1, "float", 32, "int");
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
-  tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int");
-
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
   field_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", field_type, 4);
@@ -135,6 +132,9 @@ initialize_tdesc_i386_linux (void)
   tdesc_create_reg (feature, "xmm7", 39, 1, NULL, 128, "vec128");
   tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, "i386_mxcsr");
 
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
+  tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int");
+
   tdesc_i386_linux = result;
 #if GDB_SELF_TEST
   selftests::record_xml_tdesc ("i386/i386-linux.xml", tdesc_i386_linux);
diff --git a/gdb/features/i386/i386-linux.xml b/gdb/features/i386/i386-linux.xml
index f9aa311..17f9a1a 100644
--- a/gdb/features/i386/i386-linux.xml
+++ b/gdb/features/i386/i386-linux.xml
@@ -12,6 +12,6 @@
   <architecture>i386</architecture>
   <osabi>GNU/Linux</osabi>
   <xi:include href="32bit-core.xml"/>
-  <xi:include href="32bit-linux.xml"/>
   <xi:include href="32bit-sse.xml"/>
+  <xi:include href="32bit-linux.xml"/>
 </target>
-- 
1.9.1

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

* [RFC 5/7] Centralize i386 linux target descriptions
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
                   ` (3 preceding siblings ...)
  2017-05-11 15:55 ` [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Yao Qi
@ 2017-05-11 15:55 ` Yao Qi
  2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux " Yao Qi
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches

This patch moves all the tdesc_i386*_linux target descriptions to a
function i386_linux_read_description, which returns the right target
description according to xcr0.

gdb:

2017-04-27  Yao Qi  <yao.qi@linaro.org>

	* i386-linux-tdep.c (i386_linux_read_description): New function.
	(i386_linux_core_read_description): Call
	i386_linux_read_description.
	* i386-linux-tdep.h (i386_linux_read_description): Declare.
	(tdesc_i386_linux, tdesc_i386_mmx_linux): Remove declarations.
	(tdesc_i386_avx_linux, tdesc_i386_mpx_linux): Likewise
	(tdesc_i386_avx_mpx_linux, tdesc_i386_avx_avx512_linux): Likewise.
	(tdesc_i386_avx_mpx_avx512_pku_linux): Likewise.
	* x86-linux-nat.c (x86_linux_read_description): Call
	i386_linux_read_description.
---
 gdb/i386-linux-tdep.c | 34 ++++++++++++++++++++++------------
 gdb/i386-linux-tdep.h | 10 ++--------
 gdb/x86-linux-nat.c   | 24 ++++++++----------------
 3 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 1909d61..1bc1a6f 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -678,16 +678,9 @@ i386_linux_core_read_xcr0 (bfd *abfd)
   return xcr0;
 }
 
-/* Get Linux/x86 target description from core dump.  */
-
-static const struct target_desc *
-i386_linux_core_read_description (struct gdbarch *gdbarch,
-				  struct target_ops *target,
-				  bfd *abfd)
+const struct target_desc *
+i386_linux_read_description (uint64_t xcr0)
 {
-  /* Linux/i386.  */
-  uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
-
   switch ((xcr0 & X86_XSTATE_ALL_MASK))
     {
     case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
@@ -708,10 +701,27 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
       break;
     }
 
+  return NULL;
+}
+
+/* Get Linux/x86 target description from core dump.  */
+
+static const struct target_desc *
+i386_linux_core_read_description (struct gdbarch *gdbarch,
+				  struct target_ops *target,
+				  bfd *abfd)
+{
+  /* Linux/i386.  */
+  uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
+  const struct target_desc * tdesc = i386_linux_read_description (xcr0);
+
+  if (tdesc != NULL)
+    return tdesc;
+
   if (bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
-    return tdesc_i386_linux;
+    return i386_linux_read_description (X86_XSTATE_SSE_MASK);
   else
-    return tdesc_i386_mmx_linux;
+    return i386_linux_read_description (X86_XSTATE_X87_MASK);
 }
 
 /* Similar to i386_supply_fpregset, but use XSAVE extended state.  */
@@ -835,7 +845,7 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_num_regs (gdbarch, I386_LINUX_NUM_REGS);
 
   if (! tdesc_has_registers (tdesc))
-    tdesc = tdesc_i386_linux;
+    tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK);
   tdep->tdesc = tdesc;
 
   feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index 8d4c7ca..1ca8d6b 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -42,14 +42,8 @@ extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
 extern void i386_linux_handle_segmentation_fault (struct gdbarch *gdbarch,
 						  struct ui_out *uiout);
 
-/* Linux target description.  */
-extern struct target_desc *tdesc_i386_linux;
-extern struct target_desc *tdesc_i386_mmx_linux;
-extern struct target_desc *tdesc_i386_avx_linux;
-extern struct target_desc *tdesc_i386_mpx_linux;
-extern struct target_desc *tdesc_i386_avx_mpx_linux;
-extern struct target_desc *tdesc_i386_avx_avx512_linux;
-extern struct target_desc *tdesc_i386_avx_mpx_avx512_pku_linux;
+/* Return the target description according to XCR0.  */
+extern const struct target_desc *i386_linux_read_description (uint64_t xcr0);
 
 /* Format of XSAVE extended state is:
  	struct
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 40a1b62..4eafce6 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -163,7 +163,7 @@ x86_linux_read_description (struct target_ops *ops)
 	{
 	  have_ptrace_getfpxregs = 0;
 	  have_ptrace_getregset = TRIBOOL_FALSE;
-	  return tdesc_i386_mmx_linux;
+	  return i386_linux_read_description (X86_XSTATE_X87_MASK);
 	}
     }
 #endif
@@ -240,21 +240,13 @@ x86_linux_read_description (struct target_ops *ops)
     }
   else
     {
-      switch (xcr0_features_bits)
-	{
-	case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
-	  return tdesc_i386_avx_mpx_avx512_pku_linux;
-	case X86_XSTATE_AVX_AVX512_MASK:
-	  return tdesc_i386_avx_avx512_linux;
-	case X86_XSTATE_MPX_MASK:
-	  return tdesc_i386_mpx_linux;
-	case X86_XSTATE_AVX_MPX_MASK:
-	  return tdesc_i386_avx_mpx_linux;
-	case X86_XSTATE_AVX_MASK:
-	  return tdesc_i386_avx_linux;
-	default:
-	  return tdesc_i386_linux;
-	}
+      const struct target_desc * tdesc
+	= i386_linux_read_description (xcr0_features_bits);
+
+      if (tdesc == NULL)
+	tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK);
+
+      return tdesc;
     }
 
   gdb_assert_not_reached ("failed to return tdesc");
-- 
1.9.1

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

* [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
                   ` (4 preceding siblings ...)
  2017-05-11 15:55 ` [RFC 5/7] Centralize i386 linux target descriptions Yao Qi
@ 2017-05-11 15:55 ` Yao Qi
  2017-05-11 18:14   ` John Baldwin
  2017-05-17 15:43   ` Pedro Alves
  2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii
  2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi
  7 siblings, 2 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches

Instead of using pre-generated target descriptions, this patch
changes GDB to lazily and dynamically create target descriptions
according to the target hardware capability (xcr0 in i386).
This support any combination of target features.

This patch also adds a unit test to make sure dynamically generated
tdesc are identical to these generated from xml files.

gdb:

2017-04-27  Yao Qi  <yao.qi@linaro.org>

	* i386-linux-tdep.c (i386_linux_read_description): Generate
	target description if it doesn't exist.
        [GDB_SELF_TEST] (i386_linux_read_description_test): New unit test.
        (_initialize_i386_linux_tdep) [GDB_SELF_TEST]: Register unit test.
---
 gdb/i386-linux-tdep.c | 85 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 67 insertions(+), 18 deletions(-)

diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 1bc1a6f..615cca5 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -681,27 +681,50 @@ i386_linux_core_read_xcr0 (bfd *abfd)
 const struct target_desc *
 i386_linux_read_description (uint64_t xcr0)
 {
-  switch ((xcr0 & X86_XSTATE_ALL_MASK))
+  if (xcr0 == 0)
+    return NULL;
+
+  static struct target_desc *i386_linux_tdescs \
+    [2/*X87*/][2/*SSE*/][2/*AVX*/][2/*MPX*/][2/*AVX512*/][2/*PKRU*/] = {};
+  struct target_desc **tdesc;
+
+  tdesc = &i386_linux_tdescs[(xcr0 & X86_XSTATE_X87) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_SSE) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_AVX) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_MPX) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_AVX512) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0];
+
+  if (*tdesc == NULL)
     {
-    case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
-      return tdesc_i386_avx_mpx_avx512_pku_linux;
-    case X86_XSTATE_AVX_AVX512_MASK:
-      return tdesc_i386_avx_avx512_linux;
-    case X86_XSTATE_MPX_MASK:
-      return tdesc_i386_mpx_linux;
-    case X86_XSTATE_AVX_MPX_MASK:
-      return tdesc_i386_avx_mpx_linux;
-    case X86_XSTATE_AVX_MASK:
-      return tdesc_i386_avx_linux;
-    case X86_XSTATE_SSE_MASK:
-      return tdesc_i386_linux;
-    case X86_XSTATE_X87_MASK:
-      return tdesc_i386_mmx_linux;
-    default:
-      break;
+      *tdesc = allocate_target_description ();
+      set_tdesc_architecture (*tdesc, bfd_scan_arch ("i386"));
+      set_tdesc_osabi (*tdesc, osabi_from_tdesc_string ("GNU/Linux"));
+
+      long regnum = 0;
+
+      if (xcr0 & X86_XSTATE_X87)
+	regnum = create_feature_org_gnu_gdb_i386_core_i386 (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_SSE)
+	regnum = create_feature_org_gnu_gdb_i386_sse (*tdesc, regnum);
+
+      regnum = create_feature_org_gnu_gdb_i386_linux (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_AVX)
+	regnum = create_feature_org_gnu_gdb_i386_avx (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_MPX)
+	regnum = create_feature_org_gnu_gdb_i386_mpx (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_AVX512)
+	regnum = create_feature_org_gnu_gdb_i386_avx512 (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_PKRU)
+	regnum = create_feature_org_gnu_gdb_i386_pkeys (*tdesc, regnum);
     }
 
-  return NULL;
+  return *tdesc;
 }
 
 /* Get Linux/x86 target description from core dump.  */
@@ -1101,4 +1124,30 @@ _initialize_i386_linux_tdep (void)
   initialize_tdesc_i386_avx_mpx_linux ();
   initialize_tdesc_i386_avx_avx512_linux ();
   initialize_tdesc_i386_avx_mpx_avx512_pku_linux ();
+
+#if GDB_SELF_TEST
+  struct xml_and_mask
+  {
+    const char *xml_file_name;
+    uint64_t mask;
+  };
+
+  struct xml_and_mask array[] = {
+    { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK },
+    { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK },
+    { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK },
+    { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK },
+    { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK },
+    { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK },
+    { "i386/i386-avx-mpx-avx512-pku-linux.xml",
+      X86_XSTATE_AVX_MPX_AVX512_PKU_MASK },
+  };
+
+  for (auto &a : array)
+    {
+      auto tdesc = i386_linux_read_description (a.mask);
+
+      selftests::record_xml_tdesc (a.xml_file_name, tdesc);
+    }
+#endif /* GDB_SELF_TEST */
 }
-- 
1.9.1

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

* [RFC 0/7] Make GDB builtin target descriptions more flexible
@ 2017-05-11 15:55 Yao Qi
  2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi
                   ` (7 more replies)
  0 siblings, 8 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: alan.hayward

This patch series is to change GDB builtin target descriptions more
flexible, by removing pre-generated ones.  Instead, these builtin
target descriptions can be got lazily and dynamically.  GDB builtin
target descriptions are created from initialize_tdesc_* functions in
features/*.c files.  This patch series demonstrate what does target
descriptions look like by only touching i386-linux target descriptions.

There are some shortcoming in GDB target description,

  1) All builtin target descriptions are pre-defined.  Since all GDB
  target descriptions are pre-defined, it is not flexible to compose
  features for different target descriptions.  Suppose, some architecture
  has three hardware features (like avx or mpx in x86), A, B, and C.  B
  and C can be optional.  During to the current GDB target description
  limitation, we need to define four target descriptions A, A-B, A-C,
  A-B-C.  If we need to add a new optional feature D, we need to double
  target descriptions.

  2) Target feature is not parameterized.  Registers in the same target
  feature may have different register sizes in different target descriptions.
  For example, the register size in "org.gnu.gdb.power.core" and
  "org.gnu.gdb.mips.cpu" varies between 32-bit variant and 64-bit variant.
  As a result, there are two xml files for the same feature respectively.

Only 1) is addressed in this patch series for i386-linux target.  If
people like what this patch series does, I'll gradually change other
target descriptions to the new style.  That is why I post this RFC.
GDBserver target description needs change as well, to make it more
flexible too, but GDBserver changes can be independent with GDB
changes, as long as the basic xml format is not changed.

Patch 1 is to move mips target descriptions from -nat.c to -tdep.c,
so that I can test them on x86_64-linux.  I've posted it separately
https://sourceware.org/ml/gdb-patches/2017-05/msg00204.html, include
it here to give more context.

Patch 2 adds a good unit test to verify we can get the same target
description from both xml files and c files.  It makes sure my following
changes don't break anything on target descriptions, but it, as a
unit test case, can go in independently.

Patch 4 is the major part of this series, and the following patches
changes i386-linux target descriptions, which become more flexible,
so that we can compose these target features in a free way.

Regression tested on x86_64-linux{-m32,-m64} and ppc64-linux.

*** BLURB HERE ***

Yao Qi (7):
  Move initialize_tdesc_mips* calls from mips-linux-nat.c to
    mips-linux-tdep.c
  Add unit test to builtin tdesc generated by xml
  Adjust the order of 32bit-linux.xml and 32bit-sse.xml in
    i386/i386-linux.xml
  Share code in initialize_tdesc_ functions
  Centralize i386 linux target descriptions
  Lazily and dynamically create i386-linux target descriptions
  Remove builtin tdesc_i386_*_linux

 gdb/features/aarch64.c                             | 184 ++++---
 gdb/features/arc-arcompact.c                       | 119 +++--
 gdb/features/arc-v2.c                              | 119 +++--
 gdb/features/arm/arm-with-iwmmxt.c                 | 126 +++--
 gdb/features/arm/arm-with-m-fpa-layout.c           |  76 ++-
 gdb/features/arm/arm-with-m-vfp-d16.c              | 106 ++--
 gdb/features/arm/arm-with-m.c                      |  57 ++-
 gdb/features/arm/arm-with-neon.c                   | 153 ++++--
 gdb/features/arm/arm-with-vfpv2.c                  | 106 ++--
 gdb/features/arm/arm-with-vfpv3.c                  | 138 +++--
 gdb/features/i386/amd64-avx-avx512-linux.c         | 403 +++++++++------
 gdb/features/i386/amd64-avx-avx512.c               | 366 +++++++------
 gdb/features/i386/amd64-avx-linux.c                | 244 ++++++---
 gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c | 445 ++++++++++------
 gdb/features/i386/amd64-avx-mpx-avx512-pku.c       | 408 +++++++++------
 gdb/features/i386/amd64-avx-mpx-linux.c            | 271 ++++++----
 gdb/features/i386/amd64-avx-mpx.c                  | 234 +++++----
 gdb/features/i386/amd64-avx.c                      | 207 +++++---
 gdb/features/i386/amd64-linux.c                    | 199 ++++---
 gdb/features/i386/amd64-mpx-linux.c                | 226 +++++---
 gdb/features/i386/amd64-mpx.c                      | 189 ++++---
 gdb/features/i386/amd64.c                          | 162 +++---
 gdb/features/i386/i386-avx-avx512-linux.c          | 213 +++++---
 gdb/features/i386/i386-avx-avx512.c                | 205 +++++---
 gdb/features/i386/i386-avx-linux.c                 | 168 +++---
 gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c  | 251 ++++++---
 gdb/features/i386/i386-avx-mpx-avx512-pku.c        | 247 ++++++---
 gdb/features/i386/i386-avx-mpx-linux.c             | 192 ++++---
 gdb/features/i386/i386-avx-mpx.c                   | 186 ++++---
 gdb/features/i386/i386-avx.c                       | 159 ++++--
 gdb/features/i386/i386-linux.c                     | 142 +++--
 gdb/features/i386/i386-linux.xml                   |   2 +-
 gdb/features/i386/i386-mmx-linux.c                 | 105 ++--
 gdb/features/i386/i386-mmx.c                       |  96 ++--
 gdb/features/i386/i386-mpx-linux.c                 | 164 +++---
 gdb/features/i386/i386-mpx.c                       | 157 ++++--
 gdb/features/i386/i386.c                           | 130 +++--
 gdb/features/i386/x32-avx-avx512-linux.c           | 403 +++++++++------
 gdb/features/i386/x32-avx-avx512.c                 | 366 +++++++------
 gdb/features/i386/x32-avx-linux.c                  | 244 ++++++---
 gdb/features/i386/x32-avx.c                        | 207 +++++---
 gdb/features/i386/x32-linux.c                      | 199 ++++---
 gdb/features/i386/x32.c                            | 162 +++---
 gdb/features/microblaze-with-stack-protect.c       | 155 +++---
 gdb/features/microblaze.c                          | 136 ++---
 gdb/features/mips-dsp-linux.c                      | 258 ++++++----
 gdb/features/mips-linux.c                          | 224 +++++---
 gdb/features/mips64-dsp-linux.c                    | 256 +++++----
 gdb/features/mips64-linux.c                        | 222 +++++---
 gdb/features/nds32.c                               | 190 ++++---
 gdb/features/nios2-linux.c                         | 120 +++--
 gdb/features/nios2.c                               | 120 +++--
 gdb/features/rs6000/powerpc-32.c                   | 182 ++++---
 gdb/features/rs6000/powerpc-32l.c                  | 204 +++++---
 gdb/features/rs6000/powerpc-403.c                  | 355 +++++++------
 gdb/features/rs6000/powerpc-403gc.c                | 367 +++++++------
 gdb/features/rs6000/powerpc-405.c                  | 290 ++++++-----
 gdb/features/rs6000/powerpc-505.c                  | 313 ++++++-----
 gdb/features/rs6000/powerpc-601.c                  | 324 +++++++-----
 gdb/features/rs6000/powerpc-602.c                  | 328 +++++++-----
 gdb/features/rs6000/powerpc-603.c                  | 328 +++++++-----
 gdb/features/rs6000/powerpc-604.c                  | 327 +++++++-----
 gdb/features/rs6000/powerpc-64.c                   | 182 ++++---
 gdb/features/rs6000/powerpc-64l.c                  | 204 +++++---
 gdb/features/rs6000/powerpc-7400.c                 | 379 ++++++++------
 gdb/features/rs6000/powerpc-750.c                  | 354 +++++++------
 gdb/features/rs6000/powerpc-860.c                  | 401 +++++++++------
 gdb/features/rs6000/powerpc-altivec32.c            | 273 ++++++----
 gdb/features/rs6000/powerpc-altivec32l.c           | 291 ++++++-----
 gdb/features/rs6000/powerpc-altivec64.c            | 273 ++++++----
 gdb/features/rs6000/powerpc-altivec64l.c           | 291 ++++++-----
 gdb/features/rs6000/powerpc-cell32l.c              | 293 ++++++-----
 gdb/features/rs6000/powerpc-cell64l.c              | 293 ++++++-----
 gdb/features/rs6000/powerpc-e500.c                 | 184 ++++---
 gdb/features/rs6000/powerpc-e500l.c                | 206 +++++---
 gdb/features/rs6000/powerpc-isa205-32l.c           | 204 +++++---
 gdb/features/rs6000/powerpc-isa205-64l.c           | 204 +++++---
 gdb/features/rs6000/powerpc-isa205-altivec32l.c    | 291 ++++++-----
 gdb/features/rs6000/powerpc-isa205-altivec64l.c    | 291 ++++++-----
 gdb/features/rs6000/powerpc-isa205-vsx32l.c        | 368 +++++++------
 gdb/features/rs6000/powerpc-isa205-vsx64l.c        | 368 +++++++------
 gdb/features/rs6000/powerpc-vsx32.c                | 350 +++++++------
 gdb/features/rs6000/powerpc-vsx32l.c               | 368 +++++++------
 gdb/features/rs6000/powerpc-vsx64.c                | 350 +++++++------
 gdb/features/rs6000/powerpc-vsx64l.c               | 368 +++++++------
 gdb/features/rs6000/rs6000.c                       | 184 ++++---
 gdb/features/s390-linux32.c                        | 173 ++++---
 gdb/features/s390-linux32v1.c                      | 175 ++++---
 gdb/features/s390-linux32v2.c                      | 177 ++++---
 gdb/features/s390-linux64.c                        | 205 +++++---
 gdb/features/s390-linux64v1.c                      | 207 +++++---
 gdb/features/s390-linux64v2.c                      | 209 +++++---
 gdb/features/s390-te-linux64.c                     | 262 ++++++----
 gdb/features/s390-tevx-linux64.c                   | 343 ++++++++-----
 gdb/features/s390-vx-linux64.c                     | 290 +++++++----
 gdb/features/s390x-linux64.c                       | 173 ++++---
 gdb/features/s390x-linux64v1.c                     | 175 ++++---
 gdb/features/s390x-linux64v2.c                     | 177 ++++---
 gdb/features/s390x-te-linux64.c                    | 230 ++++++---
 gdb/features/s390x-tevx-linux64.c                  | 311 ++++++-----
 gdb/features/s390x-vx-linux64.c                    | 258 ++++++----
 gdb/features/tic6x-c62x-linux.c                    |  90 ++--
 gdb/features/tic6x-c62x.c                          |  90 ++--
 gdb/features/tic6x-c64x-linux.c                    | 169 +++---
 gdb/features/tic6x-c64x.c                          | 169 +++---
 gdb/features/tic6x-c64xp-linux.c                   | 192 ++++---
 gdb/features/tic6x-c64xp.c                         | 192 ++++---
 gdb/i386-linux-tdep.c                              | 103 +++-
 gdb/i386-linux-tdep.h                              |  10 +-
 gdb/mips-linux-nat.c                               |  11 -
 gdb/mips-linux-tdep.c                              |  11 +
 gdb/mips-linux-tdep.h                              |   6 +
 gdb/target-descriptions.c                          | 570 ++++++++++++++++++---
 gdb/target-descriptions.h                          |  20 +
 gdb/testsuite/gdb.xml/maint_print_struct.exp       |   4 +-
 gdb/x86-linux-nat.c                                |  24 +-
 gdb/xml-tdesc.c                                    |   7 +-
 117 files changed, 15741 insertions(+), 9497 deletions(-)

-- 
1.9.1

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

* [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
@ 2017-05-11 15:55 ` Yao Qi
  2017-05-16 12:00   ` Philipp Rudo
  2017-05-17 15:41   ` Pedro Alves
  2017-05-11 15:55 ` [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Yao Qi
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches

I need to modify how GDB generate C files under features/ directory
from xml files.  I realize that we don't have a test to verify that
target description got from xml files equals to the target description
created by C files generated from the same xml files.

GDB have two "groups" of target descriptions, one is builtin target
descriptions, and the other is got by parsing xml files (xml files
can be got from remote target or via "set tdesc filename XXX").  The
builtin target descriptions are created from feature/*.c files, which
are generated from xml files as well,

  GDB <---------------- XML files
   ^                       |
   |                       |
   `-----   C files <------,

the reason that we transform xml files to c files is that GDB is still
able to have some target description without xml parsing support.

What this test does is to compare two target descriptions, one is from
xml files, and the other is from c files (which is generated from the
same xml files).  They are must be identical, otherwise, there is
something wrong in c files generation from xml files.

gdb:

2017-05-07  Yao Qi  <yao.qi@linaro.org>

	* target-descriptions.c (tdesc_reg_equals): New function.
	(tdesc_type_equals): New function.
	(tdesc_feature_equals): New function.
	(tdesc_equals): New function.
	(maint_print_c_tdesc_cmd): Print C code for unit test.
	[GDB_SELF_TEST] (lists): New variable.
	[GDB_SELF_TEST] (selftests::record_xml_tdesc): New function.
	[GDB_SELF_TEST] (xml_builtin_tdesc_test): New function.
	(_initialize_target_descriptions) [GDB_SELF_TEST]: Call
	register_self_test.
	* target-descriptions.h (tdesc_equals): Declare.
	(selftests::record_xml_tdesc): Declare.

	* features/aarch64.c: Regenerated.
	* features/arc-arcompact.c: Regenerated.
	* features/arc-v2.c: Regenerated.
	* features/arm/arm-with-iwmmxt.c: Regenerated.
	* features/arm/arm-with-m-fpa-layout.c: Regenerated.
	* features/arm/arm-with-m-vfp-d16.c: Regenerated.
	* features/arm/arm-with-m.c: Regenerated.
	* features/arm/arm-with-neon.c: Regenerated.
	* features/arm/arm-with-vfpv2.c: Regenerated.
	* features/arm/arm-with-vfpv3.c: Regenerated.
	* features/i386/amd64-avx-avx512-linux.c: Regenerated.
	* features/i386/amd64-avx-avx512.c: Regenerated.
	* features/i386/amd64-avx-linux.c: Regenerated.
	* features/i386/amd64-avx-mpx-avx512-pku-linux.c: Regenerated.
	* features/i386/amd64-avx-mpx-avx512-pku.c: Regenerated.
	* features/i386/amd64-avx-mpx-linux.c: Regenerated.
	* features/i386/amd64-avx-mpx.c: Regenerated.
	* features/i386/amd64-avx.c: Regenerated.
	* features/i386/amd64-linux.c: Regenerated.
	* features/i386/amd64-mpx-linux.c: Regenerated.
	* features/i386/amd64-mpx.c: Regenerated.
	* features/i386/amd64.c: Regenerated.
	* features/i386/i386-avx-avx512-linux.c: Regenerated.
	* features/i386/i386-avx-avx512.c: Regenerated.
	* features/i386/i386-avx-linux.c: Regenerated.
	* features/i386/i386-avx-mpx-avx512-pku-linux.c: Regenerated.
	* features/i386/i386-avx-mpx-avx512-pku.c: Regenerated.
	* features/i386/i386-avx-mpx-linux.c: Regenerated.
	* features/i386/i386-avx-mpx.c: Regenerated.
	* features/i386/i386-avx.c: Regenerated.
	* features/i386/i386-linux.c: Regenerated.
	* features/i386/i386-mmx-linux.c: Regenerated.
	* features/i386/i386-mmx.c: Regenerated.
	* features/i386/i386-mpx-linux.c: Regenerated.
	* features/i386/i386-mpx.c: Regenerated.
	* features/i386/i386.c: Regenerated.
	* features/i386/x32-avx-avx512-linux.c: Regenerated.
	* features/i386/x32-avx-avx512.c: Regenerated.
	* features/i386/x32-avx-linux.c: Regenerated.
	* features/i386/x32-avx.c: Regenerated.
	* features/i386/x32-linux.c: Regenerated.
	* features/i386/x32.c: Regenerated.
	* features/microblaze-with-stack-protect.c: Regenerated.
	* features/microblaze.c: Regenerated.
	* features/mips-dsp-linux.c: Regenerated.
	* features/mips-linux.c: Regenerated.
	* features/mips64-dsp-linux.c: Regenerated.
	* features/mips64-linux.c: Regenerated.
	* features/nds32.c: Regenerated.
	* features/nios2-linux.c: Regenerated.
	* features/nios2.c: Regenerated.
	* features/rs6000/powerpc-32.c: Regenerated.
	* features/rs6000/powerpc-32l.c: Regenerated.
	* features/rs6000/powerpc-403.c: Regenerated.
	* features/rs6000/powerpc-403gc.c: Regenerated.
	* features/rs6000/powerpc-405.c: Regenerated.
	* features/rs6000/powerpc-505.c: Regenerated.
	* features/rs6000/powerpc-601.c: Regenerated.
	* features/rs6000/powerpc-602.c: Regenerated.
	* features/rs6000/powerpc-603.c: Regenerated.
	* features/rs6000/powerpc-604.c: Regenerated.
	* features/rs6000/powerpc-64.c: Regenerated.
	* features/rs6000/powerpc-64l.c: Regenerated.
	* features/rs6000/powerpc-7400.c: Regenerated.
	* features/rs6000/powerpc-750.c: Regenerated.
	* features/rs6000/powerpc-860.c: Regenerated.
	* features/rs6000/powerpc-altivec32.c: Regenerated.
	* features/rs6000/powerpc-altivec32l.c: Regenerated.
	* features/rs6000/powerpc-altivec64.c: Regenerated.
	* features/rs6000/powerpc-altivec64l.c: Regenerated.
	* features/rs6000/powerpc-cell32l.c: Regenerated.
	* features/rs6000/powerpc-cell64l.c: Regenerated.
	* features/rs6000/powerpc-e500.c: Regenerated.
	* features/rs6000/powerpc-e500l.c: Regenerated.
	* features/rs6000/powerpc-isa205-32l.c: Regenerated.
	* features/rs6000/powerpc-isa205-64l.c: Regenerated.
	* features/rs6000/powerpc-isa205-altivec32l.c: Regenerated.
	* features/rs6000/powerpc-isa205-altivec64l.c: Regenerated.
	* features/rs6000/powerpc-isa205-vsx32l.c: Regenerated.
	* features/rs6000/powerpc-isa205-vsx64l.c: Regenerated.
	* features/rs6000/powerpc-vsx32.c: Regenerated.
	* features/rs6000/powerpc-vsx32l.c: Regenerated.
	* features/rs6000/powerpc-vsx64.c: Regenerated.
	* features/rs6000/powerpc-vsx64l.c: Regenerated.
	* features/rs6000/rs6000.c: Regenerated.
	* features/s390-linux32.c: Regenerated.
	* features/s390-linux32v1.c: Regenerated.
	* features/s390-linux32v2.c: Regenerated.
	* features/s390-linux64.c: Regenerated.
	* features/s390-linux64v1.c: Regenerated.
	* features/s390-linux64v2.c: Regenerated.
	* features/s390-te-linux64.c: Regenerated.
	* features/s390-tevx-linux64.c: Regenerated.
	* features/s390-vx-linux64.c: Regenerated.
	* features/s390x-linux64.c: Regenerated.
	* features/s390x-linux64v1.c: Regenerated.
	* features/s390x-linux64v2.c: Regenerated.
	* features/s390x-te-linux64.c: Regenerated.
	* features/s390x-tevx-linux64.c: Regenerated.
	* features/s390x-vx-linux64.c: Regenerated.
	* features/tic6x-c62x-linux.c: Regenerated.
	* features/tic6x-c62x.c: Regenerated.
	* features/tic6x-c64x-linux.c: Regenerated.
	* features/tic6x-c64x.c: Regenerated.
	* features/tic6x-c64xp-linux.c: Regenerated.
	* features/tic6x-c64xp.c: Regenerated.
---
 gdb/features/aarch64.c                             |   3 +
 gdb/features/arc-arcompact.c                       |   3 +
 gdb/features/arc-v2.c                              |   3 +
 gdb/features/arm/arm-with-iwmmxt.c                 |   3 +
 gdb/features/arm/arm-with-m-fpa-layout.c           |   3 +
 gdb/features/arm/arm-with-m-vfp-d16.c              |   3 +
 gdb/features/arm/arm-with-m.c                      |   3 +
 gdb/features/arm/arm-with-neon.c                   |   3 +
 gdb/features/arm/arm-with-vfpv2.c                  |   3 +
 gdb/features/arm/arm-with-vfpv3.c                  |   3 +
 gdb/features/i386/amd64-avx-avx512-linux.c         |   5 +-
 gdb/features/i386/amd64-avx-avx512.c               |   3 +
 gdb/features/i386/amd64-avx-linux.c                |   3 +
 gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c |   5 +-
 gdb/features/i386/amd64-avx-mpx-avx512-pku.c       |   3 +
 gdb/features/i386/amd64-avx-mpx-linux.c            |   3 +
 gdb/features/i386/amd64-avx-mpx.c                  |   3 +
 gdb/features/i386/amd64-avx.c                      |   3 +
 gdb/features/i386/amd64-linux.c                    |   3 +
 gdb/features/i386/amd64-mpx-linux.c                |   3 +
 gdb/features/i386/amd64-mpx.c                      |   3 +
 gdb/features/i386/amd64.c                          |   3 +
 gdb/features/i386/i386-avx-avx512-linux.c          |   3 +
 gdb/features/i386/i386-avx-avx512.c                |   3 +
 gdb/features/i386/i386-avx-linux.c                 |   3 +
 gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c  |   3 +
 gdb/features/i386/i386-avx-mpx-avx512-pku.c        |   3 +
 gdb/features/i386/i386-avx-mpx-linux.c             |   3 +
 gdb/features/i386/i386-avx-mpx.c                   |   3 +
 gdb/features/i386/i386-avx.c                       |   3 +
 gdb/features/i386/i386-linux.c                     |   3 +
 gdb/features/i386/i386-mmx-linux.c                 |   3 +
 gdb/features/i386/i386-mmx.c                       |   3 +
 gdb/features/i386/i386-mpx-linux.c                 |   3 +
 gdb/features/i386/i386-mpx.c                       |   3 +
 gdb/features/i386/i386.c                           |   3 +
 gdb/features/i386/x32-avx-avx512-linux.c           |   3 +
 gdb/features/i386/x32-avx-avx512.c                 |   3 +
 gdb/features/i386/x32-avx-linux.c                  |   3 +
 gdb/features/i386/x32-avx.c                        |   3 +
 gdb/features/i386/x32-linux.c                      |   3 +
 gdb/features/i386/x32.c                            |   3 +
 gdb/features/microblaze-with-stack-protect.c       |   3 +
 gdb/features/microblaze.c                          |   3 +
 gdb/features/mips-dsp-linux.c                      |   3 +
 gdb/features/mips-linux.c                          |   3 +
 gdb/features/mips64-dsp-linux.c                    |   3 +
 gdb/features/mips64-linux.c                        |   3 +
 gdb/features/nds32.c                               |   3 +
 gdb/features/nios2-linux.c                         |   3 +
 gdb/features/nios2.c                               |   3 +
 gdb/features/rs6000/powerpc-32.c                   |   3 +
 gdb/features/rs6000/powerpc-32l.c                  |   3 +
 gdb/features/rs6000/powerpc-403.c                  |   3 +
 gdb/features/rs6000/powerpc-403gc.c                |   3 +
 gdb/features/rs6000/powerpc-405.c                  |   3 +
 gdb/features/rs6000/powerpc-505.c                  |   3 +
 gdb/features/rs6000/powerpc-601.c                  |   3 +
 gdb/features/rs6000/powerpc-602.c                  |   3 +
 gdb/features/rs6000/powerpc-603.c                  |   3 +
 gdb/features/rs6000/powerpc-604.c                  |   3 +
 gdb/features/rs6000/powerpc-64.c                   |   3 +
 gdb/features/rs6000/powerpc-64l.c                  |   3 +
 gdb/features/rs6000/powerpc-7400.c                 |   3 +
 gdb/features/rs6000/powerpc-750.c                  |   3 +
 gdb/features/rs6000/powerpc-860.c                  |   3 +
 gdb/features/rs6000/powerpc-altivec32.c            |   3 +
 gdb/features/rs6000/powerpc-altivec32l.c           |   3 +
 gdb/features/rs6000/powerpc-altivec64.c            |   3 +
 gdb/features/rs6000/powerpc-altivec64l.c           |   3 +
 gdb/features/rs6000/powerpc-cell32l.c              |   3 +
 gdb/features/rs6000/powerpc-cell64l.c              |   3 +
 gdb/features/rs6000/powerpc-e500.c                 |   3 +
 gdb/features/rs6000/powerpc-e500l.c                |   3 +
 gdb/features/rs6000/powerpc-isa205-32l.c           |   3 +
 gdb/features/rs6000/powerpc-isa205-64l.c           |   3 +
 gdb/features/rs6000/powerpc-isa205-altivec32l.c    |   3 +
 gdb/features/rs6000/powerpc-isa205-altivec64l.c    |   3 +
 gdb/features/rs6000/powerpc-isa205-vsx32l.c        |   3 +
 gdb/features/rs6000/powerpc-isa205-vsx64l.c        |   3 +
 gdb/features/rs6000/powerpc-vsx32.c                |   3 +
 gdb/features/rs6000/powerpc-vsx32l.c               |   3 +
 gdb/features/rs6000/powerpc-vsx64.c                |   3 +
 gdb/features/rs6000/powerpc-vsx64l.c               |   3 +
 gdb/features/rs6000/rs6000.c                       |   3 +
 gdb/features/s390-linux32.c                        |   3 +
 gdb/features/s390-linux32v1.c                      |   3 +
 gdb/features/s390-linux32v2.c                      |   3 +
 gdb/features/s390-linux64.c                        |   3 +
 gdb/features/s390-linux64v1.c                      |   3 +
 gdb/features/s390-linux64v2.c                      |   3 +
 gdb/features/s390-te-linux64.c                     |   3 +
 gdb/features/s390-tevx-linux64.c                   |   3 +
 gdb/features/s390-vx-linux64.c                     |   3 +
 gdb/features/s390x-linux64.c                       |   3 +
 gdb/features/s390x-linux64v1.c                     |   3 +
 gdb/features/s390x-linux64v2.c                     |   3 +
 gdb/features/s390x-te-linux64.c                    |   3 +
 gdb/features/s390x-tevx-linux64.c                  |   3 +
 gdb/features/s390x-vx-linux64.c                    |   3 +
 gdb/features/tic6x-c62x-linux.c                    |   3 +
 gdb/features/tic6x-c62x.c                          |   3 +
 gdb/features/tic6x-c64x-linux.c                    |   3 +
 gdb/features/tic6x-c64x.c                          |   3 +
 gdb/features/tic6x-c64xp-linux.c                   |   3 +
 gdb/features/tic6x-c64xp.c                         |   3 +
 gdb/target-descriptions.c                          | 160 +++++++++++++++++++++
 gdb/target-descriptions.h                          |  16 +++
 108 files changed, 496 insertions(+), 2 deletions(-)

diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c
index e9eaed8..0aa0c54 100644
--- a/gdb/features/aarch64.c
+++ b/gdb/features/aarch64.c
@@ -188,4 +188,7 @@ initialize_tdesc_aarch64 (void)
   tdesc_create_reg (feature, "fpcr", 67, 1, NULL, 32, "int");
 
   tdesc_aarch64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("aarch64.xml", tdesc_aarch64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arc-arcompact.c b/gdb/features/arc-arcompact.c
index a527cc2..2624007 100644
--- a/gdb/features/arc-arcompact.c
+++ b/gdb/features/arc-arcompact.c
@@ -72,4 +72,7 @@ initialize_tdesc_arc_arcompact (void)
   tdesc_create_reg (feature, "status32", 35, 1, NULL, 32, "status32_type");
 
   tdesc_arc_arcompact = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arc-arcompact.xml", tdesc_arc_arcompact);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arc-v2.c b/gdb/features/arc-v2.c
index b2bdfb5..7533c1b 100644
--- a/gdb/features/arc-v2.c
+++ b/gdb/features/arc-v2.c
@@ -76,4 +76,7 @@ initialize_tdesc_arc_v2 (void)
   tdesc_create_reg (feature, "status32", 35, 1, NULL, 32, "status32_type");
 
   tdesc_arc_v2 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arc-v2.xml", tdesc_arc_v2);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-iwmmxt.c b/gdb/features/arm/arm-with-iwmmxt.c
index 1770e03..70e0dbb 100644
--- a/gdb/features/arm/arm-with-iwmmxt.c
+++ b/gdb/features/arm/arm-with-iwmmxt.c
@@ -79,4 +79,7 @@ initialize_tdesc_arm_with_iwmmxt (void)
   tdesc_create_reg (feature, "wCGR3", 47, 1, "vector", 32, "int");
 
   tdesc_arm_with_iwmmxt = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-iwmmxt.xml", tdesc_arm_with_iwmmxt);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-m-fpa-layout.c b/gdb/features/arm/arm-with-m-fpa-layout.c
index f720614..0a74ae9 100644
--- a/gdb/features/arm/arm-with-m-fpa-layout.c
+++ b/gdb/features/arm/arm-with-m-fpa-layout.c
@@ -43,4 +43,7 @@ initialize_tdesc_arm_with_m_fpa_layout (void)
   tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
 
   tdesc_arm_with_m_fpa_layout = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-m-fpa-layout.xml", tdesc_arm_with_m_fpa_layout);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-m-vfp-d16.c b/gdb/features/arm/arm-with-m-vfp-d16.c
index 069baac..2586fab 100644
--- a/gdb/features/arm/arm-with-m-vfp-d16.c
+++ b/gdb/features/arm/arm-with-m-vfp-d16.c
@@ -53,4 +53,7 @@ initialize_tdesc_arm_with_m_vfp_d16 (void)
   tdesc_create_reg (feature, "fpscr", 42, 1, "float", 32, "int");
 
   tdesc_arm_with_m_vfp_d16 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-m-vfp-d16.xml", tdesc_arm_with_m_vfp_d16);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-m.c b/gdb/features/arm/arm-with-m.c
index 64d31bb..92c47c5 100644
--- a/gdb/features/arm/arm-with-m.c
+++ b/gdb/features/arm/arm-with-m.c
@@ -34,4 +34,7 @@ initialize_tdesc_arm_with_m (void)
   tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
 
   tdesc_arm_with_m = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-m.xml", tdesc_arm_with_m);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-neon.c b/gdb/features/arm/arm-with-neon.c
index d365c0f..b236458 100644
--- a/gdb/features/arm/arm-with-neon.c
+++ b/gdb/features/arm/arm-with-neon.c
@@ -71,4 +71,7 @@ initialize_tdesc_arm_with_neon (void)
   feature = tdesc_create_feature (result, "org.gnu.gdb.arm.neon");
 
   tdesc_arm_with_neon = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-neon.xml", tdesc_arm_with_neon);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-vfpv2.c b/gdb/features/arm/arm-with-vfpv2.c
index 0ebbfef..e045b01 100644
--- a/gdb/features/arm/arm-with-vfpv2.c
+++ b/gdb/features/arm/arm-with-vfpv2.c
@@ -53,4 +53,7 @@ initialize_tdesc_arm_with_vfpv2 (void)
   tdesc_create_reg (feature, "fpscr", 42, 1, "float", 32, "int");
 
   tdesc_arm_with_vfpv2 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-vfpv2.xml", tdesc_arm_with_vfpv2);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/arm/arm-with-vfpv3.c b/gdb/features/arm/arm-with-vfpv3.c
index e235dfa..e8bfea6 100644
--- a/gdb/features/arm/arm-with-vfpv3.c
+++ b/gdb/features/arm/arm-with-vfpv3.c
@@ -69,4 +69,7 @@ initialize_tdesc_arm_with_vfpv3 (void)
   tdesc_create_reg (feature, "fpscr", 58, 1, "float", 32, "int");
 
   tdesc_arm_with_vfpv3 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("arm/arm-with-vfpv3.xml", tdesc_arm_with_vfpv3);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-avx512-linux.c b/gdb/features/i386/amd64-avx-avx512-linux.c
index 6129ab1..78f3277 100644
--- a/gdb/features/i386/amd64-avx-avx512-linux.c
+++ b/gdb/features/i386/amd64-avx-avx512-linux.c
@@ -282,7 +282,10 @@ initialize_tdesc_amd64_avx_avx512_linux (void)
   tdesc_create_reg (feature, "zmm28h", 144, 1, NULL, 256, "v2ui128");
   tdesc_create_reg (feature, "zmm29h", 145, 1, NULL, 256, "v2ui128");
   tdesc_create_reg (feature, "zmm30h", 146, 1, NULL, 256, "v2ui128");
-  tdesc_create_reg (feature, "zmm31h", 146, 1, NULL, 256, "v2ui128");
+  tdesc_create_reg (feature, "zmm31h", 147, 1, NULL, 256, "v2ui128");
 
   tdesc_amd64_avx_avx512_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-avx512-linux.xml", tdesc_amd64_avx_avx512_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-avx512.c b/gdb/features/i386/amd64-avx-avx512.c
index 8a185c1..b0d8f53 100644
--- a/gdb/features/i386/amd64-avx-avx512.c
+++ b/gdb/features/i386/amd64-avx-avx512.c
@@ -276,4 +276,7 @@ initialize_tdesc_amd64_avx_avx512 (void)
   tdesc_create_reg (feature, "zmm31h", 144, 1, NULL, 256, "v2ui128");
 
   tdesc_amd64_avx_avx512 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-avx512.xml", tdesc_amd64_avx_avx512);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-linux.c b/gdb/features/i386/amd64-avx-linux.c
index 1d56dbf..3da5273 100644
--- a/gdb/features/i386/amd64-avx-linux.c
+++ b/gdb/features/i386/amd64-avx-linux.c
@@ -174,4 +174,7 @@ initialize_tdesc_amd64_avx_linux (void)
   tdesc_create_reg (feature, "ymm15h", 75, 1, NULL, 128, "uint128");
 
   tdesc_amd64_avx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-linux.xml", tdesc_amd64_avx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c b/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c
index 248eff7..3d63c21 100644
--- a/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c
+++ b/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c
@@ -323,7 +323,10 @@ initialize_tdesc_amd64_avx_mpx_avx512_pku_linux (void)
   tdesc_create_reg (feature, "zmm31h", 153, 1, NULL, 256, "v2ui128");
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys");
-  tdesc_create_reg (feature, "pkru", 152, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "pkru", 154, 1, NULL, 32, "uint32");
 
   tdesc_amd64_avx_mpx_avx512_pku_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-mpx-avx512-pku-linux.xml", tdesc_amd64_avx_mpx_avx512_pku_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-mpx-avx512-pku.c b/gdb/features/i386/amd64-avx-mpx-avx512-pku.c
index dfe7d77..8c7d40d 100644
--- a/gdb/features/i386/amd64-avx-mpx-avx512-pku.c
+++ b/gdb/features/i386/amd64-avx-mpx-avx512-pku.c
@@ -317,4 +317,7 @@ initialize_tdesc_amd64_avx_mpx_avx512_pku (void)
   tdesc_create_reg (feature, "pkru", 151, 1, NULL, 32, "uint32");
 
   tdesc_amd64_avx_mpx_avx512_pku = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-mpx-avx512-pku.xml", tdesc_amd64_avx_mpx_avx512_pku);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-mpx-linux.c b/gdb/features/i386/amd64-avx-mpx-linux.c
index 26c1339..59d6f98 100644
--- a/gdb/features/i386/amd64-avx-mpx-linux.c
+++ b/gdb/features/i386/amd64-avx-mpx-linux.c
@@ -212,4 +212,7 @@ initialize_tdesc_amd64_avx_mpx_linux (void)
   tdesc_create_reg (feature, "bndstatus", 81, 1, NULL, 64, "status");
 
   tdesc_amd64_avx_mpx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-mpx-linux.xml", tdesc_amd64_avx_mpx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx-mpx.c b/gdb/features/i386/amd64-avx-mpx.c
index ab56f42..f744408 100644
--- a/gdb/features/i386/amd64-avx-mpx.c
+++ b/gdb/features/i386/amd64-avx-mpx.c
@@ -203,4 +203,7 @@ initialize_tdesc_amd64_avx_mpx (void)
   tdesc_create_reg (feature, "bndstatus", 78, 1, NULL, 64, "status");
 
   tdesc_amd64_avx_mpx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx-mpx.xml", tdesc_amd64_avx_mpx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-avx.c b/gdb/features/i386/amd64-avx.c
index 42bd69a..403b802 100644
--- a/gdb/features/i386/amd64-avx.c
+++ b/gdb/features/i386/amd64-avx.c
@@ -165,4 +165,7 @@ initialize_tdesc_amd64_avx (void)
   tdesc_create_reg (feature, "ymm15h", 72, 1, NULL, 128, "uint128");
 
   tdesc_amd64_avx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-avx.xml", tdesc_amd64_avx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-linux.c b/gdb/features/i386/amd64-linux.c
index 0e921ba9..8585838 100644
--- a/gdb/features/i386/amd64-linux.c
+++ b/gdb/features/i386/amd64-linux.c
@@ -156,4 +156,7 @@ initialize_tdesc_amd64_linux (void)
   tdesc_create_reg (feature, "gs_base", 59, 1, NULL, 64, "int");
 
   tdesc_amd64_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-linux.xml", tdesc_amd64_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-mpx-linux.c b/gdb/features/i386/amd64-mpx-linux.c
index e26a74a..8fb5150 100644
--- a/gdb/features/i386/amd64-mpx-linux.c
+++ b/gdb/features/i386/amd64-mpx-linux.c
@@ -194,4 +194,7 @@ initialize_tdesc_amd64_mpx_linux (void)
   tdesc_create_reg (feature, "bndstatus", 65, 1, NULL, 64, "status");
 
   tdesc_amd64_mpx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-mpx-linux.xml", tdesc_amd64_mpx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64-mpx.c b/gdb/features/i386/amd64-mpx.c
index 41f0e78..ddce80e 100644
--- a/gdb/features/i386/amd64-mpx.c
+++ b/gdb/features/i386/amd64-mpx.c
@@ -185,4 +185,7 @@ initialize_tdesc_amd64_mpx (void)
   tdesc_create_reg (feature, "bndstatus", 62, 1, NULL, 64, "status");
 
   tdesc_amd64_mpx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64-mpx.xml", tdesc_amd64_mpx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/amd64.c b/gdb/features/i386/amd64.c
index b875a9b..6def0df 100644
--- a/gdb/features/i386/amd64.c
+++ b/gdb/features/i386/amd64.c
@@ -147,4 +147,7 @@ initialize_tdesc_amd64 (void)
   tdesc_create_reg (feature, "mxcsr", 56, 1, "vector", 32, "i386_mxcsr");
 
   tdesc_amd64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/amd64.xml", tdesc_amd64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-avx512-linux.c b/gdb/features/i386/i386-avx-avx512-linux.c
index 81149d5..3ebe591 100644
--- a/gdb/features/i386/i386-avx-avx512-linux.c
+++ b/gdb/features/i386/i386-avx-avx512-linux.c
@@ -167,4 +167,7 @@ initialize_tdesc_i386_avx_avx512_linux (void)
   tdesc_create_reg (feature, "zmm7h", 65, 1, NULL, 256, "v2ui128");
 
   tdesc_i386_avx_avx512_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-avx512-linux.xml", tdesc_i386_avx_avx512_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-avx512.c b/gdb/features/i386/i386-avx-avx512.c
index 1075ca0..0932772 100644
--- a/gdb/features/i386/i386-avx-avx512.c
+++ b/gdb/features/i386/i386-avx-avx512.c
@@ -162,4 +162,7 @@ initialize_tdesc_i386_avx_avx512 (void)
   tdesc_create_reg (feature, "zmm7h", 64, 1, NULL, 256, "v2ui128");
 
   tdesc_i386_avx_avx512 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-avx512.xml", tdesc_i386_avx_avx512);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-linux.c b/gdb/features/i386/i386-avx-linux.c
index 4a8c6b5..4bcd36b 100644
--- a/gdb/features/i386/i386-avx-linux.c
+++ b/gdb/features/i386/i386-avx-linux.c
@@ -146,4 +146,7 @@ initialize_tdesc_i386_avx_linux (void)
   tdesc_create_reg (feature, "ymm7h", 49, 1, NULL, 128, "uint128");
 
   tdesc_i386_avx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-linux.xml", tdesc_i386_avx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
index f90c834..b083c2b 100644
--- a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
+++ b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
@@ -208,4 +208,7 @@ initialize_tdesc_i386_avx_mpx_avx512_pku_linux (void)
   tdesc_create_reg (feature, "pkru", 72, 1, NULL, 32, "uint32");
 
   tdesc_i386_avx_mpx_avx512_pku_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-mpx-avx512-pku-linux.xml", tdesc_i386_avx_mpx_avx512_pku_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-mpx-avx512-pku.c b/gdb/features/i386/i386-avx-mpx-avx512-pku.c
index 08d9b4b..790dd6d 100644
--- a/gdb/features/i386/i386-avx-mpx-avx512-pku.c
+++ b/gdb/features/i386/i386-avx-mpx-avx512-pku.c
@@ -203,4 +203,7 @@ initialize_tdesc_i386_avx_mpx_avx512_pku (void)
   tdesc_create_reg (feature, "pkru", 71, 1, NULL, 32, "uint32");
 
   tdesc_i386_avx_mpx_avx512_pku = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-mpx-avx512-pku.xml", tdesc_i386_avx_mpx_avx512_pku);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-mpx-linux.c b/gdb/features/i386/i386-avx-mpx-linux.c
index 4b27bfc..997e200 100644
--- a/gdb/features/i386/i386-avx-mpx-linux.c
+++ b/gdb/features/i386/i386-avx-mpx-linux.c
@@ -184,4 +184,7 @@ initialize_tdesc_i386_avx_mpx_linux (void)
   tdesc_create_reg (feature, "bndstatus", 55, 1, NULL, 64, "status");
 
   tdesc_i386_avx_mpx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-mpx-linux.xml", tdesc_i386_avx_mpx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-mpx.c b/gdb/features/i386/i386-avx-mpx.c
index b27b40a..86e7697 100644
--- a/gdb/features/i386/i386-avx-mpx.c
+++ b/gdb/features/i386/i386-avx-mpx.c
@@ -179,4 +179,7 @@ initialize_tdesc_i386_avx_mpx (void)
   tdesc_create_reg (feature, "bndstatus", 54, 1, NULL, 64, "status");
 
   tdesc_i386_avx_mpx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx-mpx.xml", tdesc_i386_avx_mpx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx.c b/gdb/features/i386/i386-avx.c
index 1cb0f9e..4dc74cf 100644
--- a/gdb/features/i386/i386-avx.c
+++ b/gdb/features/i386/i386-avx.c
@@ -141,4 +141,7 @@ initialize_tdesc_i386_avx (void)
   tdesc_create_reg (feature, "ymm7h", 48, 1, NULL, 128, "uint128");
 
   tdesc_i386_avx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-avx.xml", tdesc_i386_avx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-linux.c b/gdb/features/i386/i386-linux.c
index 42c406b..39328af 100644
--- a/gdb/features/i386/i386-linux.c
+++ b/gdb/features/i386/i386-linux.c
@@ -136,4 +136,7 @@ initialize_tdesc_i386_linux (void)
   tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, "i386_mxcsr");
 
   tdesc_i386_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-linux.xml", tdesc_i386_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-mmx-linux.c b/gdb/features/i386/i386-mmx-linux.c
index e53b55f..415e7cc 100644
--- a/gdb/features/i386/i386-mmx-linux.c
+++ b/gdb/features/i386/i386-mmx-linux.c
@@ -75,4 +75,7 @@ initialize_tdesc_i386_mmx_linux (void)
   tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int");
 
   tdesc_i386_mmx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-mmx-linux.xml", tdesc_i386_mmx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-mmx.c b/gdb/features/i386/i386-mmx.c
index 74f67ed..604e114 100644
--- a/gdb/features/i386/i386-mmx.c
+++ b/gdb/features/i386/i386-mmx.c
@@ -70,4 +70,7 @@ initialize_tdesc_i386_mmx (void)
   tdesc_create_reg (feature, "fop", 31, 1, "float", 32, "int");
 
   tdesc_i386_mmx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-mmx.xml", tdesc_i386_mmx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-mpx-linux.c b/gdb/features/i386/i386-mpx-linux.c
index 43ea192..778568e 100644
--- a/gdb/features/i386/i386-mpx-linux.c
+++ b/gdb/features/i386/i386-mpx-linux.c
@@ -174,4 +174,7 @@ initialize_tdesc_i386_mpx_linux (void)
   tdesc_create_reg (feature, "bndstatus", 47, 1, NULL, 64, "status");
 
   tdesc_i386_mpx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml", tdesc_i386_mpx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-mpx.c b/gdb/features/i386/i386-mpx.c
index e832d2e..969422e 100644
--- a/gdb/features/i386/i386-mpx.c
+++ b/gdb/features/i386/i386-mpx.c
@@ -169,4 +169,7 @@ initialize_tdesc_i386_mpx (void)
   tdesc_create_reg (feature, "bndstatus", 46, 1, NULL, 64, "status");
 
   tdesc_i386_mpx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386-mpx.xml", tdesc_i386_mpx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386.c b/gdb/features/i386/i386.c
index ede73fc..31fc3b6 100644
--- a/gdb/features/i386/i386.c
+++ b/gdb/features/i386/i386.c
@@ -131,4 +131,7 @@ initialize_tdesc_i386 (void)
   tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, "i386_mxcsr");
 
   tdesc_i386 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/i386.xml", tdesc_i386);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/x32-avx-avx512-linux.c b/gdb/features/i386/x32-avx-avx512-linux.c
index 0467d87..579566f 100644
--- a/gdb/features/i386/x32-avx-avx512-linux.c
+++ b/gdb/features/i386/x32-avx-avx512-linux.c
@@ -285,4 +285,7 @@ initialize_tdesc_x32_avx_avx512_linux (void)
   tdesc_create_reg (feature, "zmm31h", 147, 1, NULL, 256, "v2ui128");
 
   tdesc_x32_avx_avx512_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/x32-avx-avx512-linux.xml", tdesc_x32_avx_avx512_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/x32-avx-avx512.c b/gdb/features/i386/x32-avx-avx512.c
index a7a2d52..18ede9b 100644
--- a/gdb/features/i386/x32-avx-avx512.c
+++ b/gdb/features/i386/x32-avx-avx512.c
@@ -276,4 +276,7 @@ initialize_tdesc_x32_avx_avx512 (void)
   tdesc_create_reg (feature, "zmm31h", 144, 1, NULL, 256, "v2ui128");
 
   tdesc_x32_avx_avx512 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/x32-avx-avx512.xml", tdesc_x32_avx_avx512);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/x32-avx-linux.c b/gdb/features/i386/x32-avx-linux.c
index 8406815..d5ca2f1 100644
--- a/gdb/features/i386/x32-avx-linux.c
+++ b/gdb/features/i386/x32-avx-linux.c
@@ -174,4 +174,7 @@ initialize_tdesc_x32_avx_linux (void)
   tdesc_create_reg (feature, "ymm15h", 75, 1, NULL, 128, "uint128");
 
   tdesc_x32_avx_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/x32-avx-linux.xml", tdesc_x32_avx_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/x32-avx.c b/gdb/features/i386/x32-avx.c
index 7f62e8f..5800f11 100644
--- a/gdb/features/i386/x32-avx.c
+++ b/gdb/features/i386/x32-avx.c
@@ -165,4 +165,7 @@ initialize_tdesc_x32_avx (void)
   tdesc_create_reg (feature, "ymm15h", 72, 1, NULL, 128, "uint128");
 
   tdesc_x32_avx = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/x32-avx.xml", tdesc_x32_avx);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/x32-linux.c b/gdb/features/i386/x32-linux.c
index ae49549..6485e0c 100644
--- a/gdb/features/i386/x32-linux.c
+++ b/gdb/features/i386/x32-linux.c
@@ -156,4 +156,7 @@ initialize_tdesc_x32_linux (void)
   tdesc_create_reg (feature, "gs_base", 59, 1, NULL, 64, "int");
 
   tdesc_x32_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/x32-linux.xml", tdesc_x32_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/x32.c b/gdb/features/i386/x32.c
index 6005d99..18d4a1f 100644
--- a/gdb/features/i386/x32.c
+++ b/gdb/features/i386/x32.c
@@ -147,4 +147,7 @@ initialize_tdesc_x32 (void)
   tdesc_create_reg (feature, "mxcsr", 56, 1, "vector", 32, "i386_mxcsr");
 
   tdesc_x32 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("i386/x32.xml", tdesc_x32);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/microblaze-with-stack-protect.c b/gdb/features/microblaze-with-stack-protect.c
index b39aa19..67588f5 100644
--- a/gdb/features/microblaze-with-stack-protect.c
+++ b/gdb/features/microblaze-with-stack-protect.c
@@ -76,4 +76,7 @@ initialize_tdesc_microblaze_with_stack_protect (void)
   tdesc_create_reg (feature, "rshr", 58, 1, NULL, 32, "int");
 
   tdesc_microblaze_with_stack_protect = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("microblaze-with-stack-protect.xml", tdesc_microblaze_with_stack_protect);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/microblaze.c b/gdb/features/microblaze.c
index 6c86fc0..53a1e14 100644
--- a/gdb/features/microblaze.c
+++ b/gdb/features/microblaze.c
@@ -72,4 +72,7 @@ initialize_tdesc_microblaze (void)
   tdesc_create_reg (feature, "rtlbhi", 56, 1, NULL, 32, "int");
 
   tdesc_microblaze = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("microblaze.xml", tdesc_microblaze);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/mips-dsp-linux.c b/gdb/features/mips-dsp-linux.c
index 80ceb22..6d6ab7b 100644
--- a/gdb/features/mips-dsp-linux.c
+++ b/gdb/features/mips-dsp-linux.c
@@ -107,4 +107,7 @@ initialize_tdesc_mips_dsp_linux (void)
   tdesc_create_reg (feature, "restart", 79, 1, "system", 32, "int");
 
   tdesc_mips_dsp_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("mips-dsp-linux.xml", tdesc_mips_dsp_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/mips-linux.c b/gdb/features/mips-linux.c
index c990119..31e63ec 100644
--- a/gdb/features/mips-linux.c
+++ b/gdb/features/mips-linux.c
@@ -98,4 +98,7 @@ initialize_tdesc_mips_linux (void)
   tdesc_create_reg (feature, "restart", 72, 1, "system", 32, "int");
 
   tdesc_mips_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("mips-linux.xml", tdesc_mips_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/mips64-dsp-linux.c b/gdb/features/mips64-dsp-linux.c
index bc09078..1c3310d 100644
--- a/gdb/features/mips64-dsp-linux.c
+++ b/gdb/features/mips64-dsp-linux.c
@@ -105,4 +105,7 @@ initialize_tdesc_mips64_dsp_linux (void)
   tdesc_create_reg (feature, "restart", 79, 1, "system", 64, "int");
 
   tdesc_mips64_dsp_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("mips64-dsp-linux.xml", tdesc_mips64_dsp_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/mips64-linux.c b/gdb/features/mips64-linux.c
index 2ecda9b..bd897ee 100644
--- a/gdb/features/mips64-linux.c
+++ b/gdb/features/mips64-linux.c
@@ -96,4 +96,7 @@ initialize_tdesc_mips64_linux (void)
   tdesc_create_reg (feature, "restart", 72, 1, "system", 64, "int");
 
   tdesc_mips64_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("mips64-linux.xml", tdesc_mips64_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/nds32.c b/gdb/features/nds32.c
index 21f63f5..9fecfc7 100644
--- a/gdb/features/nds32.c
+++ b/gdb/features/nds32.c
@@ -89,4 +89,7 @@ initialize_tdesc_nds32 (void)
   tdesc_create_reg (feature, "ifc_lp", 67, 1, NULL, 32, "int");
 
   tdesc_nds32 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("nds32.xml", tdesc_nds32);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/nios2-linux.c b/gdb/features/nios2-linux.c
index 3288f79..b585b4e 100644
--- a/gdb/features/nios2-linux.c
+++ b/gdb/features/nios2-linux.c
@@ -68,4 +68,7 @@ initialize_tdesc_nios2_linux (void)
   tdesc_create_reg (feature, "mpuacc", 48, 1, NULL, 32, "uint32");
 
   tdesc_nios2_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("nios2-linux.xml", tdesc_nios2_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/nios2.c b/gdb/features/nios2.c
index 0cedc12..9536850 100644
--- a/gdb/features/nios2.c
+++ b/gdb/features/nios2.c
@@ -66,4 +66,7 @@ initialize_tdesc_nios2 (void)
   tdesc_create_reg (feature, "mpuacc", 48, 1, NULL, 32, "uint32");
 
   tdesc_nios2 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("nios2.xml", tdesc_nios2);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-32.c b/gdb/features/rs6000/powerpc-32.c
index 5ee5d9c..ae6e946 100644
--- a/gdb/features/rs6000/powerpc-32.c
+++ b/gdb/features/rs6000/powerpc-32.c
@@ -90,4 +90,7 @@ initialize_tdesc_powerpc_32 (void)
   tdesc_create_reg (feature, "fpscr", 70, 1, "float", 32, "int");
 
   tdesc_powerpc_32 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-32.xml", tdesc_powerpc_32);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-32l.c b/gdb/features/rs6000/powerpc-32l.c
index 971fd4b..f7478a4 100644
--- a/gdb/features/rs6000/powerpc-32l.c
+++ b/gdb/features/rs6000/powerpc-32l.c
@@ -94,4 +94,7 @@ initialize_tdesc_powerpc_32l (void)
   tdesc_create_reg (feature, "trap", 72, 1, NULL, 32, "int");
 
   tdesc_powerpc_32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-32l.xml", tdesc_powerpc_32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-403.c b/gdb/features/rs6000/powerpc-403.c
index a9106f5..50df451 100644
--- a/gdb/features/rs6000/powerpc-403.c
+++ b/gdb/features/rs6000/powerpc-403.c
@@ -164,4 +164,7 @@ initialize_tdesc_powerpc_403 (void)
   tdesc_create_reg (feature, "pbu2", 142, 1, NULL, 32, "int");
 
   tdesc_powerpc_403 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-403.xml", tdesc_powerpc_403);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-403gc.c b/gdb/features/rs6000/powerpc-403gc.c
index 402b747..b63da57 100644
--- a/gdb/features/rs6000/powerpc-403gc.c
+++ b/gdb/features/rs6000/powerpc-403gc.c
@@ -170,4 +170,7 @@ initialize_tdesc_powerpc_403gc (void)
   tdesc_create_reg (feature, "tblu", 148, 1, NULL, 32, "int");
 
   tdesc_powerpc_403gc = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-403gc.xml", tdesc_powerpc_403gc);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-405.c b/gdb/features/rs6000/powerpc-405.c
index bcfa144..d647b3a 100644
--- a/gdb/features/rs6000/powerpc-405.c
+++ b/gdb/features/rs6000/powerpc-405.c
@@ -133,4 +133,7 @@ initialize_tdesc_powerpc_405 (void)
   tdesc_create_reg (feature, "usprg0", 161, 1, NULL, 32, "int");
 
   tdesc_powerpc_405 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-405.xml", tdesc_powerpc_405);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-505.c b/gdb/features/rs6000/powerpc-505.c
index 09b0c7a..c85bca2 100644
--- a/gdb/features/rs6000/powerpc-505.c
+++ b/gdb/features/rs6000/powerpc-505.c
@@ -143,4 +143,7 @@ initialize_tdesc_powerpc_505 (void)
   tdesc_create_reg (feature, "nri", 121, 1, NULL, 32, "int");
 
   tdesc_powerpc_505 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-505.xml", tdesc_powerpc_505);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-601.c b/gdb/features/rs6000/powerpc-601.c
index f30f5e6..77a294b 100644
--- a/gdb/features/rs6000/powerpc-601.c
+++ b/gdb/features/rs6000/powerpc-601.c
@@ -147,4 +147,7 @@ initialize_tdesc_powerpc_601 (void)
   tdesc_create_reg (feature, "rtcl", 126, 1, NULL, 32, "int");
 
   tdesc_powerpc_601 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-601.xml", tdesc_powerpc_601);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-602.c b/gdb/features/rs6000/powerpc-602.c
index 7696717..73c3529 100644
--- a/gdb/features/rs6000/powerpc-602.c
+++ b/gdb/features/rs6000/powerpc-602.c
@@ -150,4 +150,7 @@ initialize_tdesc_powerpc_602 (void)
   tdesc_create_reg (feature, "lt", 130, 1, NULL, 32, "int");
 
   tdesc_powerpc_602 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-602.xml", tdesc_powerpc_602);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-603.c b/gdb/features/rs6000/powerpc-603.c
index d5dae39..ce28f9e 100644
--- a/gdb/features/rs6000/powerpc-603.c
+++ b/gdb/features/rs6000/powerpc-603.c
@@ -150,4 +150,7 @@ initialize_tdesc_powerpc_603 (void)
   tdesc_create_reg (feature, "rpa", 130, 1, NULL, 32, "int");
 
   tdesc_powerpc_603 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-603.xml", tdesc_powerpc_603);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-604.c b/gdb/features/rs6000/powerpc-604.c
index 44dc8ca..36a77bc 100644
--- a/gdb/features/rs6000/powerpc-604.c
+++ b/gdb/features/rs6000/powerpc-604.c
@@ -150,4 +150,7 @@ initialize_tdesc_powerpc_604 (void)
   tdesc_create_reg (feature, "sda", 128, 1, NULL, 32, "int");
 
   tdesc_powerpc_604 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-604.xml", tdesc_powerpc_604);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-64.c b/gdb/features/rs6000/powerpc-64.c
index 160d122..68016c7 100644
--- a/gdb/features/rs6000/powerpc-64.c
+++ b/gdb/features/rs6000/powerpc-64.c
@@ -90,4 +90,7 @@ initialize_tdesc_powerpc_64 (void)
   tdesc_create_reg (feature, "fpscr", 70, 1, "float", 32, "int");
 
   tdesc_powerpc_64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-64.xml", tdesc_powerpc_64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-64l.c b/gdb/features/rs6000/powerpc-64l.c
index 16a766e..8f911af 100644
--- a/gdb/features/rs6000/powerpc-64l.c
+++ b/gdb/features/rs6000/powerpc-64l.c
@@ -94,4 +94,7 @@ initialize_tdesc_powerpc_64l (void)
   tdesc_create_reg (feature, "trap", 72, 1, NULL, 64, "int");
 
   tdesc_powerpc_64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-64l.xml", tdesc_powerpc_64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-7400.c b/gdb/features/rs6000/powerpc-7400.c
index 69d20c4..17a670a 100644
--- a/gdb/features/rs6000/powerpc-7400.c
+++ b/gdb/features/rs6000/powerpc-7400.c
@@ -200,4 +200,7 @@ initialize_tdesc_powerpc_7400 (void)
   tdesc_create_reg (feature, "vrsave", 152, 1, "vector", 32, "int");
 
   tdesc_powerpc_7400 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-7400.xml", tdesc_powerpc_7400);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-750.c b/gdb/features/rs6000/powerpc-750.c
index 099a478..7c04e14 100644
--- a/gdb/features/rs6000/powerpc-750.c
+++ b/gdb/features/rs6000/powerpc-750.c
@@ -163,4 +163,7 @@ initialize_tdesc_powerpc_750 (void)
   tdesc_create_reg (feature, "thrm3", 142, 1, NULL, 32, "int");
 
   tdesc_powerpc_750 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-750.xml", tdesc_powerpc_750);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-860.c b/gdb/features/rs6000/powerpc-860.c
index 0692feb..68b72d4 100644
--- a/gdb/features/rs6000/powerpc-860.c
+++ b/gdb/features/rs6000/powerpc-860.c
@@ -187,4 +187,7 @@ initialize_tdesc_powerpc_860 (void)
   tdesc_create_reg (feature, "md_dbram1", 165, 1, NULL, 32, "int");
 
   tdesc_powerpc_860 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-860.xml", tdesc_powerpc_860);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-altivec32.c b/gdb/features/rs6000/powerpc-altivec32.c
index 285e87d..aa302ea 100644
--- a/gdb/features/rs6000/powerpc-altivec32.c
+++ b/gdb/features/rs6000/powerpc-altivec32.c
@@ -152,4 +152,7 @@ initialize_tdesc_powerpc_altivec32 (void)
   tdesc_create_reg (feature, "vrsave", 104, 1, "vector", 32, "int");
 
   tdesc_powerpc_altivec32 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-altivec32.xml", tdesc_powerpc_altivec32);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-altivec32l.c b/gdb/features/rs6000/powerpc-altivec32l.c
index 447ed47..1e7ed1b 100644
--- a/gdb/features/rs6000/powerpc-altivec32l.c
+++ b/gdb/features/rs6000/powerpc-altivec32l.c
@@ -156,4 +156,7 @@ initialize_tdesc_powerpc_altivec32l (void)
   tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int");
 
   tdesc_powerpc_altivec32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-altivec32l.xml", tdesc_powerpc_altivec32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-altivec64.c b/gdb/features/rs6000/powerpc-altivec64.c
index 1e9a61d..2e61480 100644
--- a/gdb/features/rs6000/powerpc-altivec64.c
+++ b/gdb/features/rs6000/powerpc-altivec64.c
@@ -152,4 +152,7 @@ initialize_tdesc_powerpc_altivec64 (void)
   tdesc_create_reg (feature, "vrsave", 104, 1, "vector", 32, "int");
 
   tdesc_powerpc_altivec64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-altivec64.xml", tdesc_powerpc_altivec64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-altivec64l.c b/gdb/features/rs6000/powerpc-altivec64l.c
index 10ecd8a..bfd9b57 100644
--- a/gdb/features/rs6000/powerpc-altivec64l.c
+++ b/gdb/features/rs6000/powerpc-altivec64l.c
@@ -156,4 +156,7 @@ initialize_tdesc_powerpc_altivec64l (void)
   tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int");
 
   tdesc_powerpc_altivec64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-altivec64l.xml", tdesc_powerpc_altivec64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-cell32l.c b/gdb/features/rs6000/powerpc-cell32l.c
index 7d33dc2..dd483b0 100644
--- a/gdb/features/rs6000/powerpc-cell32l.c
+++ b/gdb/features/rs6000/powerpc-cell32l.c
@@ -158,4 +158,7 @@ initialize_tdesc_powerpc_cell32l (void)
   tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int");
 
   tdesc_powerpc_cell32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-cell32l.xml", tdesc_powerpc_cell32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-cell64l.c b/gdb/features/rs6000/powerpc-cell64l.c
index 6054c26..6e763f2 100644
--- a/gdb/features/rs6000/powerpc-cell64l.c
+++ b/gdb/features/rs6000/powerpc-cell64l.c
@@ -158,4 +158,7 @@ initialize_tdesc_powerpc_cell64l (void)
   tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int");
 
   tdesc_powerpc_cell64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-cell64l.xml", tdesc_powerpc_cell64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-e500.c b/gdb/features/rs6000/powerpc-e500.c
index aaca3a7..27570bb 100644
--- a/gdb/features/rs6000/powerpc-e500.c
+++ b/gdb/features/rs6000/powerpc-e500.c
@@ -91,4 +91,7 @@ initialize_tdesc_powerpc_e500 (void)
   tdesc_create_reg (feature, "spefscr", 74, 1, NULL, 32, "int");
 
   tdesc_powerpc_e500 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-e500.xml", tdesc_powerpc_e500);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-e500l.c b/gdb/features/rs6000/powerpc-e500l.c
index de03862..3f753c8 100644
--- a/gdb/features/rs6000/powerpc-e500l.c
+++ b/gdb/features/rs6000/powerpc-e500l.c
@@ -95,4 +95,7 @@ initialize_tdesc_powerpc_e500l (void)
   tdesc_create_reg (feature, "trap", 72, 1, NULL, 32, "int");
 
   tdesc_powerpc_e500l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-e500l.xml", tdesc_powerpc_e500l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-isa205-32l.c b/gdb/features/rs6000/powerpc-isa205-32l.c
index 1b5bd6d..37e1179 100644
--- a/gdb/features/rs6000/powerpc-isa205-32l.c
+++ b/gdb/features/rs6000/powerpc-isa205-32l.c
@@ -94,4 +94,7 @@ initialize_tdesc_powerpc_isa205_32l (void)
   tdesc_create_reg (feature, "trap", 72, 1, NULL, 32, "int");
 
   tdesc_powerpc_isa205_32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-isa205-32l.xml", tdesc_powerpc_isa205_32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-isa205-64l.c b/gdb/features/rs6000/powerpc-isa205-64l.c
index 31bfc87..b0bbd3d 100644
--- a/gdb/features/rs6000/powerpc-isa205-64l.c
+++ b/gdb/features/rs6000/powerpc-isa205-64l.c
@@ -94,4 +94,7 @@ initialize_tdesc_powerpc_isa205_64l (void)
   tdesc_create_reg (feature, "trap", 72, 1, NULL, 64, "int");
 
   tdesc_powerpc_isa205_64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-isa205-64l.xml", tdesc_powerpc_isa205_64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-isa205-altivec32l.c b/gdb/features/rs6000/powerpc-isa205-altivec32l.c
index 6c216ce..e162982 100644
--- a/gdb/features/rs6000/powerpc-isa205-altivec32l.c
+++ b/gdb/features/rs6000/powerpc-isa205-altivec32l.c
@@ -156,4 +156,7 @@ initialize_tdesc_powerpc_isa205_altivec32l (void)
   tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int");
 
   tdesc_powerpc_isa205_altivec32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-isa205-altivec32l.xml", tdesc_powerpc_isa205_altivec32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-isa205-altivec64l.c b/gdb/features/rs6000/powerpc-isa205-altivec64l.c
index 2c206aa..93850fb 100644
--- a/gdb/features/rs6000/powerpc-isa205-altivec64l.c
+++ b/gdb/features/rs6000/powerpc-isa205-altivec64l.c
@@ -156,4 +156,7 @@ initialize_tdesc_powerpc_isa205_altivec64l (void)
   tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int");
 
   tdesc_powerpc_isa205_altivec64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-isa205-altivec64l.xml", tdesc_powerpc_isa205_altivec64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-isa205-vsx32l.c b/gdb/features/rs6000/powerpc-isa205-vsx32l.c
index 4659ce1..05ef8c2 100644
--- a/gdb/features/rs6000/powerpc-isa205-vsx32l.c
+++ b/gdb/features/rs6000/powerpc-isa205-vsx32l.c
@@ -190,4 +190,7 @@ initialize_tdesc_powerpc_isa205_vsx32l (void)
   tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64");
 
   tdesc_powerpc_isa205_vsx32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-isa205-vsx32l.xml", tdesc_powerpc_isa205_vsx32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-isa205-vsx64l.c b/gdb/features/rs6000/powerpc-isa205-vsx64l.c
index 64b12b9..f6a94c4 100644
--- a/gdb/features/rs6000/powerpc-isa205-vsx64l.c
+++ b/gdb/features/rs6000/powerpc-isa205-vsx64l.c
@@ -190,4 +190,7 @@ initialize_tdesc_powerpc_isa205_vsx64l (void)
   tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64");
 
   tdesc_powerpc_isa205_vsx64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-isa205-vsx64l.xml", tdesc_powerpc_isa205_vsx64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-vsx32.c b/gdb/features/rs6000/powerpc-vsx32.c
index ba1fcb6..1a17e04 100644
--- a/gdb/features/rs6000/powerpc-vsx32.c
+++ b/gdb/features/rs6000/powerpc-vsx32.c
@@ -186,4 +186,7 @@ initialize_tdesc_powerpc_vsx32 (void)
   tdesc_create_reg (feature, "vs31h", 136, 1, NULL, 64, "uint64");
 
   tdesc_powerpc_vsx32 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-vsx32.xml", tdesc_powerpc_vsx32);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-vsx32l.c b/gdb/features/rs6000/powerpc-vsx32l.c
index 013e392..27b47ad 100644
--- a/gdb/features/rs6000/powerpc-vsx32l.c
+++ b/gdb/features/rs6000/powerpc-vsx32l.c
@@ -190,4 +190,7 @@ initialize_tdesc_powerpc_vsx32l (void)
   tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64");
 
   tdesc_powerpc_vsx32l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-vsx32l.xml", tdesc_powerpc_vsx32l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-vsx64.c b/gdb/features/rs6000/powerpc-vsx64.c
index ca02323..9bbe42e 100644
--- a/gdb/features/rs6000/powerpc-vsx64.c
+++ b/gdb/features/rs6000/powerpc-vsx64.c
@@ -186,4 +186,7 @@ initialize_tdesc_powerpc_vsx64 (void)
   tdesc_create_reg (feature, "vs31h", 136, 1, NULL, 64, "uint64");
 
   tdesc_powerpc_vsx64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-vsx64.xml", tdesc_powerpc_vsx64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/powerpc-vsx64l.c b/gdb/features/rs6000/powerpc-vsx64l.c
index 31bb224..9df1bdb 100644
--- a/gdb/features/rs6000/powerpc-vsx64l.c
+++ b/gdb/features/rs6000/powerpc-vsx64l.c
@@ -190,4 +190,7 @@ initialize_tdesc_powerpc_vsx64l (void)
   tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64");
 
   tdesc_powerpc_vsx64l = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/powerpc-vsx64l.xml", tdesc_powerpc_vsx64l);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/rs6000/rs6000.c b/gdb/features/rs6000/rs6000.c
index d4e93a5..0bde256 100644
--- a/gdb/features/rs6000/rs6000.c
+++ b/gdb/features/rs6000/rs6000.c
@@ -91,4 +91,7 @@ initialize_tdesc_rs6000 (void)
   tdesc_create_reg (feature, "fpscr", 71, 1, "float", 32, "int");
 
   tdesc_rs6000 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("rs6000/rs6000.xml", tdesc_rs6000);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-linux32.c b/gdb/features/s390-linux32.c
index 6d13094..c7be0ce 100644
--- a/gdb/features/s390-linux32.c
+++ b/gdb/features/s390-linux32.c
@@ -75,4 +75,7 @@ initialize_tdesc_s390_linux32 (void)
   tdesc_create_reg (feature, "orig_r2", 51, 1, "system", 32, "uint32");
 
   tdesc_s390_linux32 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-linux32.xml", tdesc_s390_linux32);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-linux32v1.c b/gdb/features/s390-linux32v1.c
index f773fc1..f9bc149 100644
--- a/gdb/features/s390-linux32v1.c
+++ b/gdb/features/s390-linux32v1.c
@@ -76,4 +76,7 @@ initialize_tdesc_s390_linux32v1 (void)
   tdesc_create_reg (feature, "last_break", 52, 0, "system", 32, "code_ptr");
 
   tdesc_s390_linux32v1 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-linux32v1.xml", tdesc_s390_linux32v1);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-linux32v2.c b/gdb/features/s390-linux32v2.c
index 2317752..cf5a94a 100644
--- a/gdb/features/s390-linux32v2.c
+++ b/gdb/features/s390-linux32v2.c
@@ -77,4 +77,7 @@ initialize_tdesc_s390_linux32v2 (void)
   tdesc_create_reg (feature, "system_call", 53, 1, "system", 32, "uint32");
 
   tdesc_s390_linux32v2 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-linux32v2.xml", tdesc_s390_linux32v2);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-linux64.c b/gdb/features/s390-linux64.c
index 3c7145b..663e6db 100644
--- a/gdb/features/s390-linux64.c
+++ b/gdb/features/s390-linux64.c
@@ -91,4 +91,7 @@ initialize_tdesc_s390_linux64 (void)
   tdesc_create_reg (feature, "orig_r2", 67, 1, "system", 32, "uint32");
 
   tdesc_s390_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-linux64.xml", tdesc_s390_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-linux64v1.c b/gdb/features/s390-linux64v1.c
index 72bd894..26bac20 100644
--- a/gdb/features/s390-linux64v1.c
+++ b/gdb/features/s390-linux64v1.c
@@ -92,4 +92,7 @@ initialize_tdesc_s390_linux64v1 (void)
   tdesc_create_reg (feature, "last_break", 68, 0, "system", 32, "code_ptr");
 
   tdesc_s390_linux64v1 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-linux64v1.xml", tdesc_s390_linux64v1);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-linux64v2.c b/gdb/features/s390-linux64v2.c
index a1757da..c375644 100644
--- a/gdb/features/s390-linux64v2.c
+++ b/gdb/features/s390-linux64v2.c
@@ -93,4 +93,7 @@ initialize_tdesc_s390_linux64v2 (void)
   tdesc_create_reg (feature, "system_call", 69, 1, "system", 32, "uint32");
 
   tdesc_s390_linux64v2 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-linux64v2.xml", tdesc_s390_linux64v2);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-te-linux64.c b/gdb/features/s390-te-linux64.c
index 0a3aedf..c5e6e53 100644
--- a/gdb/features/s390-te-linux64.c
+++ b/gdb/features/s390-te-linux64.c
@@ -115,4 +115,7 @@ initialize_tdesc_s390_te_linux64 (void)
   tdesc_create_reg (feature, "tr15", 89, 1, "tdb", 64, "uint64");
 
   tdesc_s390_te_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-te-linux64.xml", tdesc_s390_te_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-tevx-linux64.c b/gdb/features/s390-tevx-linux64.c
index 5bc3eec..7d8fc5f 100644
--- a/gdb/features/s390-tevx-linux64.c
+++ b/gdb/features/s390-tevx-linux64.c
@@ -185,4 +185,7 @@ initialize_tdesc_s390_tevx_linux64 (void)
   tdesc_create_reg (feature, "v31", 121, 1, NULL, 128, "vec128");
 
   tdesc_s390_tevx_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-tevx-linux64.xml", tdesc_s390_tevx_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390-vx-linux64.c b/gdb/features/s390-vx-linux64.c
index c3ffa16..c1d64aa 100644
--- a/gdb/features/s390-vx-linux64.c
+++ b/gdb/features/s390-vx-linux64.c
@@ -163,4 +163,7 @@ initialize_tdesc_s390_vx_linux64 (void)
   tdesc_create_reg (feature, "v31", 101, 1, NULL, 128, "vec128");
 
   tdesc_s390_vx_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390-vx-linux64.xml", tdesc_s390_vx_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390x-linux64.c b/gdb/features/s390x-linux64.c
index 04502c6..4bede5e 100644
--- a/gdb/features/s390x-linux64.c
+++ b/gdb/features/s390x-linux64.c
@@ -75,4 +75,7 @@ initialize_tdesc_s390x_linux64 (void)
   tdesc_create_reg (feature, "orig_r2", 51, 1, "system", 64, "uint64");
 
   tdesc_s390x_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390x-linux64.xml", tdesc_s390x_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390x-linux64v1.c b/gdb/features/s390x-linux64v1.c
index 05bfd53..353655f 100644
--- a/gdb/features/s390x-linux64v1.c
+++ b/gdb/features/s390x-linux64v1.c
@@ -76,4 +76,7 @@ initialize_tdesc_s390x_linux64v1 (void)
   tdesc_create_reg (feature, "last_break", 52, 0, "system", 64, "code_ptr");
 
   tdesc_s390x_linux64v1 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390x-linux64v1.xml", tdesc_s390x_linux64v1);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390x-linux64v2.c b/gdb/features/s390x-linux64v2.c
index 4108cc0..e1edb51 100644
--- a/gdb/features/s390x-linux64v2.c
+++ b/gdb/features/s390x-linux64v2.c
@@ -77,4 +77,7 @@ initialize_tdesc_s390x_linux64v2 (void)
   tdesc_create_reg (feature, "system_call", 53, 1, "system", 32, "uint32");
 
   tdesc_s390x_linux64v2 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390x-linux64v2.xml", tdesc_s390x_linux64v2);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390x-te-linux64.c b/gdb/features/s390x-te-linux64.c
index f75d900..3dee7c2 100644
--- a/gdb/features/s390x-te-linux64.c
+++ b/gdb/features/s390x-te-linux64.c
@@ -99,4 +99,7 @@ initialize_tdesc_s390x_te_linux64 (void)
   tdesc_create_reg (feature, "tr15", 73, 1, "tdb", 64, "uint64");
 
   tdesc_s390x_te_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390x-te-linux64.xml", tdesc_s390x_te_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390x-tevx-linux64.c b/gdb/features/s390x-tevx-linux64.c
index 327cd23..d841528 100644
--- a/gdb/features/s390x-tevx-linux64.c
+++ b/gdb/features/s390x-tevx-linux64.c
@@ -169,4 +169,7 @@ initialize_tdesc_s390x_tevx_linux64 (void)
   tdesc_create_reg (feature, "v31", 105, 1, NULL, 128, "vec128");
 
   tdesc_s390x_tevx_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390x-tevx-linux64.xml", tdesc_s390x_tevx_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/s390x-vx-linux64.c b/gdb/features/s390x-vx-linux64.c
index e66da70..9bac54b 100644
--- a/gdb/features/s390x-vx-linux64.c
+++ b/gdb/features/s390x-vx-linux64.c
@@ -147,4 +147,7 @@ initialize_tdesc_s390x_vx_linux64 (void)
   tdesc_create_reg (feature, "v31", 85, 1, NULL, 128, "vec128");
 
   tdesc_s390x_vx_linux64 = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("s390x-vx-linux64.xml", tdesc_s390x_vx_linux64);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/tic6x-c62x-linux.c b/gdb/features/tic6x-c62x-linux.c
index 8dd426d..46cbce6 100644
--- a/gdb/features/tic6x-c62x-linux.c
+++ b/gdb/features/tic6x-c62x-linux.c
@@ -53,4 +53,7 @@ initialize_tdesc_tic6x_c62x_linux (void)
   tdesc_create_reg (feature, "PC", 33, 1, NULL, 32, "code_ptr");
 
   tdesc_tic6x_c62x_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("tic6x-c62x-linux.xml", tdesc_tic6x_c62x_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/tic6x-c62x.c b/gdb/features/tic6x-c62x.c
index 2089aaf..5c7c898 100644
--- a/gdb/features/tic6x-c62x.c
+++ b/gdb/features/tic6x-c62x.c
@@ -51,4 +51,7 @@ initialize_tdesc_tic6x_c62x (void)
   tdesc_create_reg (feature, "PC", 33, 1, NULL, 32, "code_ptr");
 
   tdesc_tic6x_c62x = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("tic6x-c62x.xml", tdesc_tic6x_c62x);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/tic6x-c64x-linux.c b/gdb/features/tic6x-c64x-linux.c
index 2752358..310fe5a 100644
--- a/gdb/features/tic6x-c64x-linux.c
+++ b/gdb/features/tic6x-c64x-linux.c
@@ -87,4 +87,7 @@ initialize_tdesc_tic6x_c64x_linux (void)
   tdesc_create_reg (feature, "B31", 65, 1, NULL, 32, "uint32");
 
   tdesc_tic6x_c64x_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("tic6x-c64x-linux.xml", tdesc_tic6x_c64x_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/tic6x-c64x.c b/gdb/features/tic6x-c64x.c
index 0feda24..c4bbb3a 100644
--- a/gdb/features/tic6x-c64x.c
+++ b/gdb/features/tic6x-c64x.c
@@ -85,4 +85,7 @@ initialize_tdesc_tic6x_c64x (void)
   tdesc_create_reg (feature, "B31", 65, 1, NULL, 32, "uint32");
 
   tdesc_tic6x_c64x = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("tic6x-c64x.xml", tdesc_tic6x_c64x);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/tic6x-c64xp-linux.c b/gdb/features/tic6x-c64xp-linux.c
index c1bee4c..df89570 100644
--- a/gdb/features/tic6x-c64xp-linux.c
+++ b/gdb/features/tic6x-c64xp-linux.c
@@ -92,4 +92,7 @@ initialize_tdesc_tic6x_c64xp_linux (void)
   tdesc_create_reg (feature, "RILC", 68, 1, NULL, 32, "uint32");
 
   tdesc_tic6x_c64xp_linux = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("tic6x-c64xp-linux.xml", tdesc_tic6x_c64xp_linux);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/tic6x-c64xp.c b/gdb/features/tic6x-c64xp.c
index 160b854..6cc6430 100644
--- a/gdb/features/tic6x-c64xp.c
+++ b/gdb/features/tic6x-c64xp.c
@@ -90,4 +90,7 @@ initialize_tdesc_tic6x_c64xp (void)
   tdesc_create_reg (feature, "RILC", 68, 1, NULL, 32, "uint32");
 
   tdesc_tic6x_c64xp = result;
+#if GDB_SELF_TEST
+  selftests::record_xml_tdesc ("tic6x-c64xp.xml", tdesc_tic6x_c64xp);
+#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 9a7e2dd..754f15b 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -468,6 +468,101 @@ tdesc_osabi (const struct target_desc *target_desc)
   return target_desc->osabi;
 }
 
+/* Return true if REG1 and REG2 are identical.  */
+
+static bool
+tdesc_reg_equals (const struct tdesc_reg *reg1,
+		  const struct tdesc_reg *reg2)
+{
+  return (strcmp (reg1->name, reg2->name) == 0
+	  && reg1->target_regnum == reg2->target_regnum
+	  && reg1->save_restore == reg2->save_restore
+	  && reg1->bitsize == reg2->bitsize
+	  && (reg1->group == reg2->group
+	      || strcmp (reg1->group, reg2->group) == 0)
+	  && strcmp (reg1->type, reg2->type) == 0);
+}
+
+/* Return true if TYPE1 and TYPE2 are identical.  */
+
+static bool
+tdesc_type_equals (const struct tdesc_type *type1,
+		   const struct tdesc_type *type2)
+{
+  return (strcmp (type1->name, type2->name) == 0
+	  && type1->kind == type2->kind);
+}
+
+/* Return true if FEATURE1 and FEATURE2 are identical.  */
+
+static bool
+tdesc_feature_equals (const struct tdesc_feature *feature1,
+		      const struct tdesc_feature *feature2)
+{
+  if (feature1 == feature2)
+    return true;
+
+  if (strcmp (feature1->name, feature2->name) != 0)
+    return false;
+
+  struct tdesc_reg *reg;
+
+  for (int ix = 0;
+       VEC_iterate (tdesc_reg_p, feature1->registers, ix, reg);
+       ix++)
+    {
+      struct tdesc_reg *reg2
+	= VEC_index (tdesc_reg_p, feature2->registers, ix);
+
+      if (!tdesc_reg_equals (reg, reg2))
+	return false;
+    }
+
+  struct tdesc_type *type;
+
+  for (int ix = 0;
+       VEC_iterate (tdesc_type_p, feature1->types, ix, type);
+       ix++)
+    {
+      struct tdesc_type *type2
+	= VEC_index (tdesc_type_p, feature2->types, ix);
+
+      if (!tdesc_type_equals (type, type2))
+	return false;
+    }
+
+  return true;
+}
+
+bool
+tdesc_equals (const struct target_desc *tdesc1,
+	      const struct target_desc *tdesc2)
+{
+  if (tdesc1->arch != tdesc2->arch)
+    return false;
+
+  if (tdesc1->osabi != tdesc2->osabi)
+    return false;
+
+  if (VEC_length (tdesc_feature_p, tdesc1->features)
+      != VEC_length (tdesc_feature_p, tdesc2->features))
+      return false;
+
+  struct tdesc_feature *feature;
+
+  for (int ix = 0;
+       VEC_iterate (tdesc_feature_p, tdesc1->features, ix, feature);
+       ix++)
+    {
+      struct tdesc_feature *feature2
+	= VEC_index (tdesc_feature_p, tdesc2->features, ix);
+
+      if (!tdesc_feature_equals (feature, feature2))
+	return false;
+    }
+  return true;
+}
+
 \f
 
 /* Return 1 if this target description includes any registers.  */
@@ -1734,6 +1829,12 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
     error (_("The current target description did not come from an XML file."));
 
   filename = lbasename (target_description_filename);
+  std::string filename_after_features (target_description_filename);
+  auto loc = filename_after_features.rfind ("/features/");
+
+  if (loc != std::string::npos)
+    filename_after_features = filename_after_features.substr (loc + 10);
+
   function = (char *) alloca (strlen (filename) + 1);
   for (inp = filename, outp = function; *inp != '\0'; inp++)
     if (*inp == '.')
@@ -1979,9 +2080,64 @@ feature = tdesc_create_feature (result, \"%s\");\n",
     }
 
   printf_unfiltered ("  tdesc_%s = result;\n", function);
+
+  printf_unfiltered ("#if GDB_SELF_TEST\n");
+  printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n",
+		     filename_after_features.data (), function);
+  printf_unfiltered ("#endif /* GDB_SELF_TEST */\n");
   printf_unfiltered ("}\n");
 }
 
+#if GDB_SELF_TEST
+#include "selftest.h"
+
+namespace selftests {
+
+static std::vector<std::pair<std::string, const struct target_desc *>> lists;
+
+void
+record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc)
+{
+  lists.emplace_back (xml_file, tdesc);
+}
+
+/* Test these GDB builtin target descriptions are identical to these which
+   are generated by the corresponding xml files.  */
+
+static void
+xml_builtin_tdesc_test (void)
+{
+  std::string feature_dir (ldirname (__FILE__));
+  struct stat st;
+
+  /* Look for the features directory.  If the directory of __FILE__ can't
+     be found, __FILE__ is a file name with relative path.  Guess that
+     GDB is executed in testsuite directory like ../gdb, because I don't
+     expect that GDB is invoked somewhere else and run self tests.  */
+  if (stat (feature_dir.data (), &st) < 0)
+    {
+      feature_dir.insert (0, SLASH_STRING);
+      feature_dir.insert (0, "..");
+
+      /* If still can't find the path, something is wrong.  */
+      SELF_CHECK (stat (feature_dir.data (), &st) == 0);
+    }
+
+  feature_dir = feature_dir + SLASH_STRING + "features";
+
+  for (auto const &e : lists)
+    {
+      std::string tdesc_xml = (feature_dir + SLASH_STRING + e.first);
+      const struct target_desc *tdesc
+	= file_read_description_xml (tdesc_xml.data ());
+
+      SELF_CHECK (tdesc != NULL);
+      SELF_CHECK (tdesc_equals (tdesc, e.second));
+    }
+}
+}
+#endif /* GDB_SELF_TEST */
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_target_descriptions;
 
@@ -2022,4 +2178,8 @@ GDB will read the description from the target."),
   add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
 Print the current target description as a C source file."),
 	   &maintenanceprintlist);
+
+#if GDB_SELF_TEST
+  register_self_test (selftests::xml_builtin_tdesc_test);
+#endif
 }
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 361ac97..78416ae 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *);
 int tdesc_compatible_p (const struct target_desc *,
 			const struct bfd_arch_info *);
 
+/* Compare target descriptions TDESC1 and TDESC2, return true if they
+   are identical.  */
+
+bool tdesc_equals (const struct target_desc *tdesc1,
+		   const struct target_desc *tdesc2);
+
 /* Return the string value of a property named KEY, or NULL if the
    property was not specified.  */
 
@@ -253,4 +259,14 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		       int regnum, int save_restore, const char *group,
 		       int bitsize, const char *type);
 
+#if GDB_SELF_TEST
+namespace selftests {
+
+/* Record the target description TDESC generated by XML_FILE.  */
+
+void record_xml_tdesc (std::string xml_file,
+		       const struct target_desc *tdesc);
+}
+#endif
+
 #endif /* TARGET_DESCRIPTIONS_H */
-- 
1.9.1

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

* [RFC 7/7] Remove builtin tdesc_i386_*_linux
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
  2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi
  2017-05-11 15:55 ` [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Yao Qi
@ 2017-05-11 15:55 ` Yao Qi
  2017-05-16 12:02   ` Philipp Rudo
  2017-05-17 15:46   ` Pedro Alves
  2017-05-11 15:55 ` [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Yao Qi
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw)
  To: gdb-patches

With the previous patch, i386-linux target descriptions are dynamically
generated, so don't need all these i386-linux builtin target descriptions.
This patch removes the code in initialize_tdesc_i386_*_linux to create
these tdesc_i386_*_linux.  These initialize_tdesc_i386_*_linux functions
are still kept to call selftests::record_xml to record the builtin
xml file target descriptions are transformed to c files and compiled
into GDB, so that we can verify that target descriptions from these
xml files are tested (see changes in xml_builtin_tdesc_test).

Function tdesc_print_intiailize_tdesc_p is added to control the scope
of this removal.  In order to highlight the change, I don't re-indent
the code.

gdb:

2017-05-09  Yao Qi  <yao.qi@linaro.org>

	* target-descriptions.c (tdesc_print_intiailize_tdesc_p): New
	function.
	(maint_print_c_tdesc_cmd): Call tdesc_print_intiailize_tdesc_p
	and print code differently.
	(xml_files): New.
	(selftests::record_xml): New function.
	(selftests::xml_builtin_tdesc_test): Add test.
	* target-descriptions.h (selftests::record_xml): Declare.
	* features/i386/i386-avx-avx512-linux.c: Regenerated.
	* features/i386/i386-avx-linux.c: Regenerated.
	* features/i386/i386-avx-mpx-avx512-pku-linux.c: Regenerated.
	* features/i386/i386-avx-mpx-linux.c: Regenerated.
	* features/i386/i386-linux.c: Regenerated.
	* features/i386/i386-mmx-linux.c: Regenerated.
	* features/i386/i386-mpx-linux.c: Regenerated.
---
 gdb/features/i386/i386-avx-avx512-linux.c         | 18 +-------
 gdb/features/i386/i386-avx-linux.c                | 17 +-------
 gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c | 20 +--------
 gdb/features/i386/i386-avx-mpx-linux.c            | 18 +-------
 gdb/features/i386/i386-linux.c                    | 16 +-------
 gdb/features/i386/i386-mmx-linux.c                | 15 +------
 gdb/features/i386/i386-mpx-linux.c                | 17 +-------
 gdb/target-descriptions.c                         | 50 +++++++++++++++++++++--
 gdb/target-descriptions.h                         |  4 ++
 9 files changed, 58 insertions(+), 117 deletions(-)

diff --git a/gdb/features/i386/i386-avx-avx512-linux.c b/gdb/features/i386/i386-avx-avx512-linux.c
index e1aec3c..2f47a40 100644
--- a/gdb/features/i386/i386-avx-avx512-linux.c
+++ b/gdb/features/i386/i386-avx-avx512-linux.c
@@ -220,26 +220,10 @@ create_feature_org_gnu_gdb_i386_avx512 (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_AVX512 */
 
-struct target_desc *tdesc_i386_avx_avx512_linux;
 static void
 initialize_tdesc_i386_avx_avx512_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_avx512 (result, regnum);
-
-  tdesc_i386_avx_avx512_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-avx-avx512-linux.xml", tdesc_i386_avx_avx512_linux);
+  selftests::record_xml ("i386/i386-avx-avx512-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-linux.c b/gdb/features/i386/i386-avx-linux.c
index 5645dbf..2e85e0d 100644
--- a/gdb/features/i386/i386-avx-linux.c
+++ b/gdb/features/i386/i386-avx-linux.c
@@ -186,25 +186,10 @@ create_feature_org_gnu_gdb_i386_avx (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_AVX */
 
-struct target_desc *tdesc_i386_avx_linux;
 static void
 initialize_tdesc_i386_avx_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum);
-
-  tdesc_i386_avx_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-avx-linux.xml", tdesc_i386_avx_linux);
+  selftests::record_xml ("i386/i386-avx-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
index c65b515..433f596 100644
--- a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
+++ b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c
@@ -287,28 +287,10 @@ create_feature_org_gnu_gdb_i386_pkeys (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_PKEYS */
 
-struct target_desc *tdesc_i386_avx_mpx_avx512_pku_linux;
 static void
 initialize_tdesc_i386_avx_mpx_avx512_pku_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_avx512 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_pkeys (result, regnum);
-
-  tdesc_i386_avx_mpx_avx512_pku_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-avx-mpx-avx512-pku-linux.xml", tdesc_i386_avx_mpx_avx512_pku_linux);
+  selftests::record_xml ("i386/i386-avx-mpx-avx512-pku-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-avx-mpx-linux.c b/gdb/features/i386/i386-avx-mpx-linux.c
index 49d25f2..9ff8f25 100644
--- a/gdb/features/i386/i386-avx-mpx-linux.c
+++ b/gdb/features/i386/i386-avx-mpx-linux.c
@@ -238,26 +238,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_MPX */
 
-struct target_desc *tdesc_i386_avx_mpx_linux;
 static void
 initialize_tdesc_i386_avx_mpx_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum);
-
-  tdesc_i386_avx_mpx_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-avx-mpx-linux.xml", tdesc_i386_avx_mpx_linux);
+  selftests::record_xml ("i386/i386-avx-mpx-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-linux.c b/gdb/features/i386/i386-linux.c
index 51719c5..50ffbda 100644
--- a/gdb/features/i386/i386-linux.c
+++ b/gdb/features/i386/i386-linux.c
@@ -164,24 +164,10 @@ create_feature_org_gnu_gdb_i386_linux (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_LINUX */
 
-struct target_desc *tdesc_i386_linux;
 static void
 initialize_tdesc_i386_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-
-  tdesc_i386_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-linux.xml", tdesc_i386_linux);
+  selftests::record_xml ("i386/i386-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-mmx-linux.c b/gdb/features/i386/i386-mmx-linux.c
index 5141692..fe308ba 100644
--- a/gdb/features/i386/i386-mmx-linux.c
+++ b/gdb/features/i386/i386-mmx-linux.c
@@ -88,23 +88,10 @@ create_feature_org_gnu_gdb_i386_linux (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_LINUX */
 
-struct target_desc *tdesc_i386_mmx_linux;
 static void
 initialize_tdesc_i386_mmx_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-
-  tdesc_i386_mmx_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-mmx-linux.xml", tdesc_i386_mmx_linux);
+  selftests::record_xml ("i386/i386-mmx-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/features/i386/i386-mpx-linux.c b/gdb/features/i386/i386-mpx-linux.c
index 9c722cf..15a1ebe 100644
--- a/gdb/features/i386/i386-mpx-linux.c
+++ b/gdb/features/i386/i386-mpx-linux.c
@@ -216,25 +216,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc *result, long regnum)
 }
 #endif /* _ORG_GNU_GDB_I386_MPX */
 
-struct target_desc *tdesc_i386_mpx_linux;
 static void
 initialize_tdesc_i386_mpx_linux (void)
 {
-  struct target_desc *result = allocate_target_description ();
-
-  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
-
-  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
-
-  long regnum = 0;
-
-  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
-  regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum);
-
-  tdesc_i386_mpx_linux = result;
 #if GDB_SELF_TEST
-  selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml", tdesc_i386_mpx_linux);
+  selftests::record_xml ("i386/i386-mpx-linux.xml");
 #endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index a81ae0d..da44e37 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -2005,6 +2005,15 @@ tdesc_feature_unique_name (const struct tdesc_feature* feature)
   return name;
 }
 
+/* Whether print code initializing tdesc_FUNCTION or not.  */
+
+static bool
+tdesc_print_intiailize_tdesc_p (const char *function)
+{
+  return !(strncmp (function, "i386", 4) == 0
+	   && strncmp (&function[strlen (function) - 6], "_linux", 6) == 0);
+}
+
 static void
 maint_print_c_tdesc_cmd (char *args, int from_tty)
 {
@@ -2262,10 +2271,15 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
       printf_unfiltered ("\n");
     }
 
-  printf_unfiltered ("struct target_desc *tdesc_%s;\n", function);
+  if (tdesc_print_intiailize_tdesc_p (function))
+    printf_unfiltered ("struct target_desc *tdesc_%s;\n", function);
+
   printf_unfiltered ("static void\n");
   printf_unfiltered ("initialize_tdesc_%s (void)\n", function);
   printf_unfiltered ("{\n");
+
+  if (tdesc_print_intiailize_tdesc_p (function))
+    {
   printf_unfiltered
     ("  struct target_desc *result = allocate_target_description ();\n");
 
@@ -2320,10 +2334,20 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
 
   printf_unfiltered ("\n");
   printf_unfiltered ("  tdesc_%s = result;\n", function);
+    }
 
   printf_unfiltered ("#if GDB_SELF_TEST\n");
-  printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n",
-		     filename_after_features.data (), function);
+
+  if (tdesc_print_intiailize_tdesc_p (function))
+    {
+      printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n",
+			 filename_after_features.data (), function);
+    }
+  else
+    {
+      printf_unfiltered ("  selftests::record_xml (\"%s\");\n",
+			 filename_after_features.data ());
+    }
   printf_unfiltered ("#endif /* GDB_SELF_TEST */\n");
   printf_unfiltered ("}\n");
 }
@@ -2341,12 +2365,32 @@ record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc)
   lists.emplace_back (xml_file, tdesc);
 }
 
+/* XML files, which generate C files and compiled into GDB.  */
+
+static std::vector<std::string> xml_files;
+
+void
+record_xml (const char *xml_file)
+{
+  xml_files.emplace_back (xml_file);
+}
+
 /* Test these GDB builtin target descriptions are identical to these which
    are generated by the corresponding xml files.  */
 
 static void
 xml_builtin_tdesc_test (void)
 {
+  /* Make sure we have a test for every xml target description.  */
+  for (auto const &xml : xml_files)
+    {
+      auto r = std::find_if (std::begin (lists), std::end (lists),
+			     [&xml](decltype (lists)::const_reference e)
+			     -> bool {return e.first == xml;});
+
+      SELF_CHECK (r != std::end (lists));
+    }
+
   std::string feature_dir (ldirname (__FILE__));
   struct stat st;
 
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 78416ae..2667bcd 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -266,6 +266,10 @@ namespace selftests {
 
 void record_xml_tdesc (std::string xml_file,
 		       const struct target_desc *tdesc);
+
+/* Record c file generated by XML_FILE is compiled into GDB.  */
+
+void record_xml (const char *xml_file);
 }
 #endif
 
-- 
1.9.1

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

* Re: [RFC 0/7] Make GDB builtin target descriptions more flexible
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
                   ` (5 preceding siblings ...)
  2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux " Yao Qi
@ 2017-05-11 16:06 ` Eli Zaretskii
  2017-05-11 20:56   ` Yao Qi
  2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi
  7 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2017-05-11 16:06 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, alan.hayward

> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: alan.hayward@arm.com
> Date: Thu, 11 May 2017 16:54:58 +0100
> 
> Patch 4 is the major part of this series

But the series you've sent doesn't seem to include patch 4...

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

* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux " Yao Qi
@ 2017-05-11 18:14   ` John Baldwin
  2017-05-11 21:03     ` Yao Qi
  2017-05-17 15:43   ` Pedro Alves
  1 sibling, 1 reply; 32+ messages in thread
From: John Baldwin @ 2017-05-11 18:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Yao Qi

On Thursday, May 11, 2017 04:55:04 PM Yao Qi wrote:
> Instead of using pre-generated target descriptions, this patch
> changes GDB to lazily and dynamically create target descriptions
> according to the target hardware capability (xcr0 in i386).
> This support any combination of target features.
> 
> This patch also adds a unit test to make sure dynamically generated
> tdesc are identical to these generated from xml files.

I definitely like this approach of composing the tdesc.  For the
non-Linux case there are already amd64_target_description() and
i386_target_description() functions in the respective -tdep.c files
that could use the same treatment.  Not sure if you wanted to fix all
of x86 in the same patch series or do it as a separate followup?

-- 
John Baldwin

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

* [RFC 4/7] Share code in initialize_tdesc_ functions
  2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
                   ` (6 preceding siblings ...)
  2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii
@ 2017-05-11 20:55 ` Yao Qi
  2017-05-16 12:02   ` Philipp Rudo
  7 siblings, 1 reply; 32+ messages in thread
From: Yao Qi @ 2017-05-11 20:55 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 8436 bytes --]

[Resend it as the message is too large, and mail list doesn't accept
it.  Compress the patch and put it in attachment.]

We define target description features in .xml files, and compose
target descriptions of different features in .xml too.  They are
used to generated .c files and these .c files are included into
GDB.  Each feature may exist in multiple target descriptions, for
example, feature avx exists in 4 different i386-linux target
descriptions, so that each .c file generated from these 4 target
descriptions has a copy of code for this feature, like this,

  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
  tdesc_create_reg (feature, "ymm0h", 42, 1, NULL, 128, "uint128");
  tdesc_create_reg (feature, "ymm1h", 43, 1, NULL, 128, "uint128");

that is the code duplication.

This patch moves the code for each feature into a function, and
each target description just call the function for that feature.
it becomes,

  #ifndef _ORG_GNU_GDB_I386_AVX
  #define _ORG_GNU_GDB_I386_AVX
  static void
  create_feature_org_gnu_gdb_i386_avx (struct target_desc *result)
  {
    struct tdesc_feature *feature;

    feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
    tdesc_create_reg (feature, "ymm0h", 42, 1, NULL, 128, "uint128");

  static void
  initialize_tdesc_i386_avx_mpx_avx512_pku_linux (void)
  {
    ....
    create_feature_org_gnu_gdb_i386_avx (result);
  }

So far, everything looks simple, however, two things make it more
complicated,

 1) different target features have the same feature name, like
    "org.gnu.gdb.power.core" and "org.gnu.gdb.mips.cpu", so we can't
    define only one function for "org.gnu.gdb.power.core",
 2) the register number of target feature (tdesc_reg::target_regnum)
    varies in different target descriptions, because the register
    number is allocated sequentially feature by feature within a
    target description, so we can't use the c function for a feature
    generated in one target description for some other target
    description.

In order to address 1), function tdesc_feature_unique_name is added
in this patch, which returns the unique name for each feature, even
these feature name are the same.  To address 2), this patch removes
the constant number in generated c functions, instead, use variable
"regnum", and get it passed from the c functions.

The benefits of doing this are two-folded,

 - remove the duplication, there is only one copy of code for each
   feature.  With all targets enabled, gdb size is smaller,

$ size ./gdb
   text    data     bss     dec     hex filename
23823773        2395640  364776 26584189        195a47d ./gdb

with the patch applied,

   text    data     bss     dec     hex filename
23552417        2395648  364808 26312873        19180a9 ./gdb

 - the composition of features can be more flexible, which will be
   demonstrated in the following patch,

gdb:

2017-05-09  Yao Qi  <yao.qi@linaro.org>

	* target-descriptions.c: Include string and algorithm.
	(struct tdesc_reg) <target_regnum>: Add comments.
	(tdesc_reg_equals): Update.
	(tdesc_remote_register_number): Call abs.
	(tdesc_feature_unique_name): New function.
	(maint_print_c_tdesc_cmd): Print function for each feature.
	* xml-tdesc.c (tdesc_start_reg): Set regnum negative if the
	value is from "regnum" attribute.
	(tdesc_start_reg): Call abs.

	* features/aarch64.c: Regenerated.
	* features/arc-arcompact.c: Regenerated.
	* features/arc-v2.c: Regenerated.
	* features/arm/arm-with-iwmmxt.c: Regenerated.
	* features/arm/arm-with-m-fpa-layout.c: Regenerated.
	* features/arm/arm-with-m-vfp-d16.c: Regenerated.
	* features/arm/arm-with-m.c: Regenerated.
	* features/arm/arm-with-neon.c: Regenerated.
	* features/arm/arm-with-vfpv2.c: Regenerated.
	* features/arm/arm-with-vfpv3.c: Regenerated.
	* features/i386/amd64-avx-avx512-linux.c: Regenerated.
	* features/i386/amd64-avx-avx512.c: Regenerated.
	* features/i386/amd64-avx-linux.c: Regenerated.
	* features/i386/amd64-avx-mpx-avx512-pku-linux.c: Regenerated.
	* features/i386/amd64-avx-mpx-avx512-pku.c: Regenerated.
	* features/i386/amd64-avx-mpx-linux.c: Regenerated.
	* features/i386/amd64-avx-mpx.c: Regenerated.
	* features/i386/amd64-avx.c: Regenerated.
	* features/i386/amd64-linux.c: Regenerated.
	* features/i386/amd64-mpx-linux.c: Regenerated.
	* features/i386/amd64-mpx.c: Regenerated.
	* features/i386/amd64.c: Regenerated.
	* features/i386/i386-avx-avx512-linux.c: Regenerated.
	* features/i386/i386-avx-avx512.c: Regenerated.
	* features/i386/i386-avx-linux.c: Regenerated.
	* features/i386/i386-avx-mpx-avx512-pku-linux.c: Regenerated.
	* features/i386/i386-avx-mpx-avx512-pku.c: Regenerated.
	* features/i386/i386-avx-mpx-linux.c: Regenerated.
	* features/i386/i386-avx-mpx.c: Regenerated.
	* features/i386/i386-avx.c: Regenerated.
	* features/i386/i386-linux.c: Regenerated.
	* features/i386/i386-mmx-linux.c: Regenerated.
	* features/i386/i386-mmx.c: Regenerated.
	* features/i386/i386-mpx-linux.c: Regenerated.
	* features/i386/i386-mpx.c: Regenerated.
	* features/i386/i386.c: Regenerated.
	* features/i386/x32-avx-avx512-linux.c: Regenerated.
	* features/i386/x32-avx-avx512.c: Regenerated.
	* features/i386/x32-avx-linux.c: Regenerated.
	* features/i386/x32-avx.c: Regenerated.
	* features/i386/x32-linux.c: Regenerated.
	* features/i386/x32.c: Regenerated.
	* features/microblaze-with-stack-protect.c: Regenerated.
	* features/microblaze.c: Regenerated.
	* features/mips-dsp-linux.c: Regenerated.
	* features/mips-linux.c: Regenerated.
	* features/mips64-dsp-linux.c: Regenerated.
	* features/mips64-linux.c: Regenerated.
	* features/nds32.c: Regenerated.
	* features/nios2-linux.c: Regenerated.
	* features/nios2.c: Regenerated.
	* features/rs6000/powerpc-32.c: Regenerated.
	* features/rs6000/powerpc-32l.c: Regenerated.
	* features/rs6000/powerpc-403.c: Regenerated.
	* features/rs6000/powerpc-403gc.c: Regenerated.
	* features/rs6000/powerpc-405.c: Regenerated.
	* features/rs6000/powerpc-505.c: Regenerated.
	* features/rs6000/powerpc-601.c: Regenerated.
	* features/rs6000/powerpc-602.c: Regenerated.
	* features/rs6000/powerpc-603.c: Regenerated.
	* features/rs6000/powerpc-604.c: Regenerated.
	* features/rs6000/powerpc-64.c: Regenerated.
	* features/rs6000/powerpc-64l.c: Regenerated.
	* features/rs6000/powerpc-7400.c: Regenerated.
	* features/rs6000/powerpc-750.c: Regenerated.
	* features/rs6000/powerpc-860.c: Regenerated.
	* features/rs6000/powerpc-altivec32.c: Regenerated.
	* features/rs6000/powerpc-altivec32l.c: Regenerated.
	* features/rs6000/powerpc-altivec64.c: Regenerated.
	* features/rs6000/powerpc-altivec64l.c: Regenerated.
	* features/rs6000/powerpc-cell32l.c: Regenerated.
	* features/rs6000/powerpc-cell64l.c: Regenerated.
	* features/rs6000/powerpc-e500.c: Regenerated.
	* features/rs6000/powerpc-e500l.c: Regenerated.
	* features/rs6000/powerpc-isa205-32l.c: Regenerated.
	* features/rs6000/powerpc-isa205-64l.c: Regenerated.
	* features/rs6000/powerpc-isa205-altivec32l.c: Regenerated.
	* features/rs6000/powerpc-isa205-altivec64l.c: Regenerated.
	* features/rs6000/powerpc-isa205-vsx32l.c: Regenerated.
	* features/rs6000/powerpc-isa205-vsx64l.c: Regenerated.
	* features/rs6000/powerpc-vsx32.c: Regenerated.
	* features/rs6000/powerpc-vsx32l.c: Regenerated.
	* features/rs6000/powerpc-vsx64.c: Regenerated.
	* features/rs6000/powerpc-vsx64l.c: Regenerated.
	* features/rs6000/rs6000.c: Regenerated.
	* features/s390-linux32.c: Regenerated.
	* features/s390-linux32v1.c: Regenerated.
	* features/s390-linux32v2.c: Regenerated.
	* features/s390-linux64.c: Regenerated.
	* features/s390-linux64v1.c: Regenerated.
	* features/s390-linux64v2.c: Regenerated.
	* features/s390-te-linux64.c: Regenerated.
	* features/s390-tevx-linux64.c: Regenerated.
	* features/s390-vx-linux64.c: Regenerated.
	* features/s390x-linux64.c: Regenerated.
	* features/s390x-linux64v1.c: Regenerated.
	* features/s390x-linux64v2.c: Regenerated.
	* features/s390x-te-linux64.c: Regenerated.
	* features/s390x-tevx-linux64.c: Regenerated.
	* features/s390x-vx-linux64.c: Regenerated.
	* features/tic6x-c62x-linux.c: Regenerated.
	* features/tic6x-c62x.c: Regenerated.
	* features/tic6x-c64x-linux.c: Regenerated.
	* features/tic6x-c64x.c: Regenerated.
	* features/tic6x-c64xp-linux.c: Regenerated.
	* features/tic6x-c64xp.c: Regenerated.

gdb/testsuite:

2017-05-10  Yao Qi  <yao.qi@linaro.org>

	* gdb.xml/maint_print_struct.exp: Update.
-- 
Yao 

[-- Attachment #2: 4.patch.tar.gz --]
[-- Type: application/gzip, Size: 64621 bytes --]

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

* Re: [RFC 0/7] Make GDB builtin target descriptions more flexible
  2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii
@ 2017-05-11 20:56   ` Yao Qi
  0 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 20:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, alan.hayward

On 17-05-11 19:06:21, Eli Zaretskii wrote:
> > From: Yao Qi <qiyaoltc@gmail.com>
> > Cc: alan.hayward@arm.com
> > Date: Thu, 11 May 2017 16:54:58 +0100
> > 
> > Patch 4 is the major part of this series
> 
> But the series you've sent doesn't seem to include patch 4...

The mail was rejected because it is too large.  Compress the patch and
send it again.

-- 
Yao 

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

* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-11 18:14   ` John Baldwin
@ 2017-05-11 21:03     ` Yao Qi
  0 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-11 21:03 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 17-05-11 10:04:39, John Baldwin wrote:
> On Thursday, May 11, 2017 04:55:04 PM Yao Qi wrote:
> > Instead of using pre-generated target descriptions, this patch
> > changes GDB to lazily and dynamically create target descriptions
> > according to the target hardware capability (xcr0 in i386).
> > This support any combination of target features.
> > 
> > This patch also adds a unit test to make sure dynamically generated
> > tdesc are identical to these generated from xml files.
> 
> I definitely like this approach of composing the tdesc.  For the
> non-Linux case there are already amd64_target_description() and
> i386_target_description() functions in the respective -tdep.c files
> that could use the same treatment.  Not sure if you wanted to fix all
> of x86 in the same patch series or do it as a separate followup?
>

The later, but I can fix all x86 in one patch if people think it is more
reasonable to do so.  I plan to change one arch each time, and gradually
change all archs to use the new style of target description.

-- 
Yao 

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi
@ 2017-05-16 12:00   ` Philipp Rudo
  2017-05-16 15:46     ` Yao Qi
  2017-05-17 16:06     ` Pedro Alves
  2017-05-17 15:41   ` Pedro Alves
  1 sibling, 2 replies; 32+ messages in thread
From: Philipp Rudo @ 2017-05-16 12:00 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

I really like the direction this series is going to.  Generating the target
description dynamically during runtime definitely makes the code much nicer and
reduces the number of XML files in the feature directory.

The only thing you could have done to make it better is to get rid of the
conversion to C altogether and create the target description directly by
parsing the XML file. I don't see a benefit in the conversion and removing it
would simplify the code even more.  But that is a long term goal and your patch
set definitely helps towards that goal.

See more comments below

On Thu, 11 May 2017 16:55:00 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

[...]

> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 9a7e2dd..754f15b 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -468,6 +468,101 @@ tdesc_osabi (const struct target_desc *target_desc)
>    return target_desc->osabi;
>  }
> 
> +/* Return true if REG1 and REG2 are identical.  */
> +
> +static bool
> +tdesc_reg_equals (const struct tdesc_reg *reg1,
> +		  const struct tdesc_reg *reg2)
> +{
> +  return (strcmp (reg1->name, reg2->name) == 0
> +	  && reg1->target_regnum == reg2->target_regnum
> +	  && reg1->save_restore == reg2->save_restore
> +	  && reg1->bitsize == reg2->bitsize
> +	  && (reg1->group == reg2->group
> +	      || strcmp (reg1->group, reg2->group) == 0)
> +	  && strcmp (reg1->type, reg2->type) == 0);
> +}

inline?
strcmp (...) == 0 -> utils.h:streq (...)? Makes it at least a little shorter.

> +/* Return true if TYPE1 and TYPE2 are identical.  */
> +
> +static bool
> +tdesc_type_equals (const struct tdesc_type *type1,
> +		   const struct tdesc_type *type2)
> +{
> +  return (strcmp (type1->name, type2->name) == 0
> +	  && type1->kind == type2->kind);
> +}

Same as above.

> +/* Return true if FEATURE1 and FEATURE2 are identical.  */
> +
> +static bool
> +tdesc_feature_equals (const struct tdesc_feature *feature1,
> +		      const struct tdesc_feature *feature2)
> +{
> +  if (feature1 == feature2)
> +    return true;
> +
> +  if (strcmp (feature1->name, feature2->name) != 0)
> +    return false;
> +
> +  struct tdesc_reg *reg;
> +
> +  for (int ix = 0;
> +       VEC_iterate (tdesc_reg_p, feature1->registers, ix, reg);
> +       ix++)
> +    {
> +      struct tdesc_reg *reg2
> +	= VEC_index (tdesc_reg_p, feature2->registers, ix);
> +
> +      if (!tdesc_reg_equals (reg, reg2))
> +	return false;
> +    }
> +
> +  struct tdesc_type *type;
> +
> +  for (int ix = 0;
> +       VEC_iterate (tdesc_type_p, feature1->types, ix, type);
> +       ix++)
> +    {
> +      struct tdesc_type *type2
> +	= VEC_index (tdesc_type_p, feature2->types, ix);
> +
> +      if (!tdesc_type_equals (type, type2))
> +	return false;
> +    }
> +
> +  return true;
> +}
> +
> +bool
> +tdesc_equals (const struct target_desc *tdesc1,
> +	      const struct target_desc *tdesc2)
> +{
> +  if (tdesc1->arch != tdesc2->arch)
> +    return false;
> +
> +  if (tdesc1->osabi != tdesc2->osabi)
> +    return false;
> +
> +  if (VEC_length (tdesc_feature_p, tdesc1->features)
> +      != VEC_length (tdesc_feature_p, tdesc2->features))
> +      return false;
> +
> +  struct tdesc_feature *feature;
> +
> +  for (int ix = 0;
> +       VEC_iterate (tdesc_feature_p, tdesc1->features, ix, feature);
> +       ix++)
> +    {
> +      struct tdesc_feature *feature2
> +	= VEC_index (tdesc_feature_p, tdesc2->features, ix);
> +
> +      if (!tdesc_feature_equals (feature, feature2))
> +	return false;
> +    }
> +  return true;
> +}
> +
>  \f
> 
>  /* Return 1 if this target description includes any registers.  */
> @@ -1734,6 +1829,12 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
>      error (_("The current target description did not come from an XML
> file."));
> 
>    filename = lbasename (target_description_filename);
> +  std::string filename_after_features (target_description_filename);
> +  auto loc = filename_after_features.rfind ("/features/");
> +
> +  if (loc != std::string::npos)
> +    filename_after_features = filename_after_features.substr (loc + 10);

This piece is hard to read.  This should do the same and (for my taste) is
better readable

std::string filename_with_subdir (target_description_filename);
std::string subdir (lbasename (ldirname (target_description_filename)));
if (subdir.campare ("features") != 0)
  filename_with_subdir = concat_path (subdir, filename);

If you prefer your way, please replace '10' by strlen ("/features/") (or
similar) in the last line.

>    function = (char *) alloca (strlen (filename) + 1);
>    for (inp = filename, outp = function; *inp != '\0'; inp++)
>      if (*inp == '.')
> @@ -1979,9 +2080,64 @@ feature = tdesc_create_feature (result, \"%s\");\n",
>      }
> 
>    printf_unfiltered ("  tdesc_%s = result;\n", function);
> +
> +  printf_unfiltered ("#if GDB_SELF_TEST\n");
> +  printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n",
> +		     filename_after_features.data (), function);
> +  printf_unfiltered ("#endif /* GDB_SELF_TEST */\n");
>    printf_unfiltered ("}\n");
>  }
> 
> +#if GDB_SELF_TEST
> +#include "selftest.h"
> +
> +namespace selftests {
> +
> +static std::vector<std::pair<std::string, const struct target_desc *>> lists;

Can you rename the vector? Just 'lists' is pretty vague.

> +void
> +record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc)
> +{
> +  lists.emplace_back (xml_file, tdesc);
> +}
> +
> +/* Test these GDB builtin target descriptions are identical to these which
> +   are generated by the corresponding xml files.  */
> +
> +static void
> +xml_builtin_tdesc_test (void)
> +{
> +  std::string feature_dir (ldirname (__FILE__));
> +  struct stat st;
> +
> +  /* Look for the features directory.  If the directory of __FILE__ can't
> +     be found, __FILE__ is a file name with relative path.  Guess that
> +     GDB is executed in testsuite directory like ../gdb, because I don't
> +     expect that GDB is invoked somewhere else and run self tests.  */
> +  if (stat (feature_dir.data (), &st) < 0)
> +    {
> +      feature_dir.insert (0, SLASH_STRING);
> +      feature_dir.insert (0, "..");

This would be a perfect spot for concat_path (patch 2 from my lk series
https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html).
Then it would read

feature_dir = concat_path ("..", feature_dir);

I actually want to bring some of my patches upstream (mostly the
s390-linux-tdep split up patch) to reduce maintenance cost of my
series.  This would be a good time for this patch, too.

> +
> +      /* If still can't find the path, something is wrong.  */
> +      SELF_CHECK (stat (feature_dir.data (), &st) == 0);
> +    }
> +
> +  feature_dir = feature_dir + SLASH_STRING + "features";

feature_dir = concat_path (feature_dir, "features"); ?

> +
> +  for (auto const &e : lists)
> +    {
> +      std::string tdesc_xml = (feature_dir + SLASH_STRING + e.first);

std::string tdesc_xml = concat_path (feature_dir, e.first); ?

Thanks
Philipp


> +      const struct target_desc *tdesc
> +	= file_read_description_xml (tdesc_xml.data ());
> +
> +      SELF_CHECK (tdesc != NULL);
> +      SELF_CHECK (tdesc_equals (tdesc, e.second));
> +    }
> +}
> +}
> +#endif /* GDB_SELF_TEST */
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_target_descriptions;
> 
> @@ -2022,4 +2178,8 @@ GDB will read the description from the target."),
>    add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
>  Print the current target description as a C source file."),
>  	   &maintenanceprintlist);
> +
> +#if GDB_SELF_TEST
> +  register_self_test (selftests::xml_builtin_tdesc_test);
> +#endif
>  }
> diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
> index 361ac97..78416ae 100644
> --- a/gdb/target-descriptions.h
> +++ b/gdb/target-descriptions.h
> @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *);
>  int tdesc_compatible_p (const struct target_desc *,
>  			const struct bfd_arch_info *);
> 
> +/* Compare target descriptions TDESC1 and TDESC2, return true if they
> +   are identical.  */
> +
> +bool tdesc_equals (const struct target_desc *tdesc1,
> +		   const struct target_desc *tdesc2);
> +
>  /* Return the string value of a property named KEY, or NULL if the
>     property was not specified.  */
> 
> @@ -253,4 +259,14 @@ void tdesc_create_reg (struct tdesc_feature *feature,
> const char *name, int regnum, int save_restore, const char *group,
>  		       int bitsize, const char *type);
> 
> +#if GDB_SELF_TEST
> +namespace selftests {
> +
> +/* Record the target description TDESC generated by XML_FILE.  */
> +
> +void record_xml_tdesc (std::string xml_file,
> +		       const struct target_desc *tdesc);
> +}
> +#endif
> +
>  #endif /* TARGET_DESCRIPTIONS_H */

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

* Re: [RFC 4/7] Share code in initialize_tdesc_ functions
  2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi
@ 2017-05-16 12:02   ` Philipp Rudo
  2017-05-17 15:43     ` Pedro Alves
  0 siblings, 1 reply; 32+ messages in thread
From: Philipp Rudo @ 2017-05-16 12:02 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

On Thu, 11 May 2017 21:55:04 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 754f15b..a81ae0d 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c

[...]

> @@ -54,7 +57,10 @@ typedef struct tdesc_reg
>    char *name;
>  
>    /* The register number used by this target to refer to this
> -     register.  This is used for remote p/P packets and to determine
> +     register.  Positive number means it is got by increasing
> +     the previous register number by one.  Negative number means
> +     it is got from "regnum" attribute in the xml file.
> +     This is used for remote p/P packets and to determine
>       the ordering of registers in the remote g/G packets.  */
>    long target_regnum;

I'm not really sure if I like your idea with positive/negative regnums.  It
could easily lead to hard to find bugs because someone forgot a '-' somewhere.
I would prefer an extra field like 

bool explicit_regnum;
or
bool automatic_regnum;

[...]

> +/* Return the unique name of FEATURE.  The uniqueness means different
> +   target description features have different values.  This is used
> +   to differentiate different features with the same name.  */
> +
> +static std::string
> +tdesc_feature_unique_name (const struct tdesc_feature* feature)
> +{
> +  std::string name = std::string (feature->name);
> +
> +  if (name.compare ("org.gnu.gdb.arm.m-profile") == 0)
> +    {
> +      struct tdesc_reg *reg;
> +
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	if (reg->type != NULL && strcmp (reg->type, "arm_fpa_ext") == 0)
> +	  {
> +	    name.append ("_fpa");
> +	    break;
> +	  }
> +    }
> +  else if (name.compare ("org.gnu.gdb.arm.vfp") == 0)
> +    {
> +      name.append ("_");
> +      name.append (std::to_string (VEC_length (tdesc_reg_p,
> +					       feature->registers) - 1));
> +    }
> +  else if (name.compare ("org.gnu.gdb.i386.core") == 0)
> +    {
> +      struct tdesc_reg *reg = NULL;
> +
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	{
> +	  if (strcmp (reg->name, "ebp") == 0
> +	      || strcmp (reg->name, "rbp") == 0)
> +	    break;
> +	}
> +
> +      name.append ("_");
> +      if (strcmp (reg->name, "ebp") == 0)
> +	name.append ("i386");
> +      else
> +	{
> +	  if (strcmp (reg->type, "data_ptr") == 0)
> +	    name.append ("amd64");
> +	  else
> +	    name.append ("x32");
> +	}
> +    }
> +  else if (name.compare ("org.gnu.gdb.power.core") == 0)
> +    {
> +      struct tdesc_reg *reg = NULL;
> +      struct tdesc_reg *reg_mq = NULL;
> +      struct tdesc_reg *reg_r0 = NULL;
> +
> +      /* Four variants.  */
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	{
> +	  if (strcmp (reg->name, "r0") == 0)
> +	    reg_r0 = reg;
> +	  if (strcmp (reg->name, "mq") == 0)
> +	    reg_mq = reg;
> +
> +	  if (reg_r0 != NULL && reg_mq != NULL)
> +	    break;
> +	}
> +
> +      name.append ("_");
> +      if (reg_mq == NULL)
> +	name.append (std::to_string (reg_r0->bitsize));
> +      else
> +	{
> +	  if (reg_mq->target_regnum == -124)
> +	    name.append ("601");
> +	  else
> +	    name.append ("rs6000");
> +	}
> +    }
> +  else if (name.compare ("org.gnu.gdb.power.fpu") == 0)
> +    {
> +      struct tdesc_reg *reg = NULL;
> +
> +      /* Three variants.  */
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	{
> +	  if (strcmp (reg->name, "fpscr") == 0)
> +	    break;
> +	}
> +
> +      name.append ("_");
> +
> +      if (reg->target_regnum == -71)
> +	name.append ("rs6000");
> +      else
> +	name.append (std::to_string (reg->bitsize));
> +    }
> +  else if (name.compare ("org.gnu.gdb.power.linux") == 0)
> +    {
> +      struct tdesc_reg *reg = NULL;
> +
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	{
> +	  if (strcmp (reg->name, "orig_r3") == 0)
> +	    break;
> +	}
> +
> +      name.append (std::to_string (reg->bitsize));
> +    }
> +  else if (name.compare ("org.gnu.gdb.s390.linux") == 0)
> +    {
> +      struct tdesc_reg *reg = NULL;
> +      struct tdesc_reg *reg_orig_r2 = NULL;
> +      struct tdesc_reg *reg_sc = NULL;
> +      struct tdesc_reg *reg_lb = NULL;
> +
> +      /* Six variants.  */
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	{
> +	  if (strcmp (reg->name, "orig_r2") == 0)
> +	    reg_orig_r2 = reg;
> +	  if (strcmp (reg->name, "last_break") == 0)
> +	    reg_lb = reg;
> +	  if (strcmp (reg->name, "system_call") == 0)
> +	    reg_sc = reg;
> +
> +	  if (reg_orig_r2 != NULL && reg_sc != NULL && reg_lb != NULL)
> +	    break;
> +	}
> +
> +      name.append ("_");
> +      name.append (std::to_string (reg_orig_r2->bitsize));
> +
> +      if (reg_lb != NULL)
> +	{
> +	  name.append ("_");
> +
> +	  if (reg_sc == NULL)
> +	    name.append ("v1");
> +	  else
> +	    name.append ("v2");
> +	}
> +    }
> +  else if (name.compare ("org.gnu.gdb.s390.core") == 0)
> +    {
> +      struct tdesc_reg *reg = NULL;
> +      struct tdesc_reg *reg_r0 = NULL;
> +      struct tdesc_reg *reg_r0l = NULL;
> +
> +      /* Six variants.  */
> +      for (int ix = 0;
> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
> +	   ix++)
> +	{
> +	  if (strcmp (reg->name, "r0") == 0)
> +	    reg_r0 = reg;
> +	  if (strcmp (reg->name, "r0l") == 0)
> +	    reg_r0l = reg;
> +
> +	  if (reg_r0 != NULL && reg_r0l != NULL)
> +	    break;
> +	}
> +
> +      name.append ("_");
> +      if (reg_r0l != NULL)
> +	name.append ("s64");
> +      else
> +	name.append (std::to_string (reg_r0->bitsize));
> +    }
> +  else if (name.compare ("org.gnu.gdb.mips.cpu") == 0
> +	   || name.compare ("org.gnu.gdb.mips.cp0") == 0
> +	   || name.compare ("org.gnu.gdb.mips.fpu") == 0
> +	   || name.compare ("org.gnu.gdb.mips.dsp") == 0
> +	   || name.compare ("org.gnu.gdb.mips.linux") == 0)
> +    {
> +      struct tdesc_reg *reg = VEC_index (tdesc_reg_p, feature->registers, 0);
> +
> +      name.append (std::to_string (reg->bitsize));
> +    }
> +
> +  std::replace (name.begin (), name.end (), '.', '_');
> +  std::replace (name.begin (), name.end (), '-', '_');
> +
> +  return name;
> +}
> +

This function is actually the part I like least of your implementation.  Its
way to long and barely readable.  The way I understand, it is needed to create
unique macro and function names.  So why don't you simply use the filename of
the XML file where the feature is defined?  It already is unique.  Or use an
gdbarch hook so every architecture can decide for itself how to name them?

Philipp

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

* Re: [RFC 7/7] Remove builtin tdesc_i386_*_linux
  2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi
@ 2017-05-16 12:02   ` Philipp Rudo
  2017-05-17 15:46   ` Pedro Alves
  1 sibling, 0 replies; 32+ messages in thread
From: Philipp Rudo @ 2017-05-16 12:02 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

On Thu, 11 May 2017 16:55:05 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

[...]

> diff --git a/gdb/features/i386/i386-mpx-linux.c
> b/gdb/features/i386/i386-mpx-linux.c index 9c722cf..15a1ebe 100644
> --- a/gdb/features/i386/i386-mpx-linux.c
> +++ b/gdb/features/i386/i386-mpx-linux.c
> @@ -216,25 +216,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc
> *result, long regnum) }
>  #endif /* _ORG_GNU_GDB_I386_MPX */
> 
> -struct target_desc *tdesc_i386_mpx_linux;
>  static void
>  initialize_tdesc_i386_mpx_linux (void)
>  {
> -  struct target_desc *result = allocate_target_description ();
> -
> -  set_tdesc_architecture (result, bfd_scan_arch ("i386"));
> -
> -  set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux"));
> -
> -  long regnum = 0;
> -
> -  regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum);
> -  regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum);
> -  regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum);
> -  regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum);
> -
> -  tdesc_i386_mpx_linux = result;
>  #if GDB_SELF_TEST
> -  selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml",
> tdesc_i386_mpx_linux);
> +  selftests::record_xml ("i386/i386-mpx-linux.xml");
>  #endif /* GDB_SELF_TEST */
>  }

Do you really still need the initialize_tdesc_* functions when you initialize
the target description dynamically? Can't you move the seftest somewhere else
and get rid of it in this case?  This would make the code slightly nicer.

> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index a81ae0d..da44e37 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -2005,6 +2005,15 @@ tdesc_feature_unique_name (const struct tdesc_feature*
> feature) return name;
>  }
> 
> +/* Whether print code initializing tdesc_FUNCTION or not.  */
> +
> +static bool
> +tdesc_print_intiailize_tdesc_p (const char *function)
> +{
> +  return !(strncmp (function, "i386", 4) == 0

common/common-utils.h:startswith (function, "i386") ?

> +	   && strncmp (&function[strlen (function) - 6], "_linux", 6) == 0);

common/common-utils.h:endswith (function, "_linux") ?
(From my concat_path patch
https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html)

Even nicer would be if you don't check the function name at all but check e.g.
a flag a target must set or use a feature method.  Otherwise you will get an
extremely long and unreadable expression once more targets use this method.

Philipp

> +}
> +
>  static void
>  maint_print_c_tdesc_cmd (char *args, int from_tty)
>  {
> @@ -2262,10 +2271,15 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
>        printf_unfiltered ("\n");
>      }
> 
> -  printf_unfiltered ("struct target_desc *tdesc_%s;\n", function);
> +  if (tdesc_print_intiailize_tdesc_p (function))
> +    printf_unfiltered ("struct target_desc *tdesc_%s;\n", function);
> +
>    printf_unfiltered ("static void\n");
>    printf_unfiltered ("initialize_tdesc_%s (void)\n", function);
>    printf_unfiltered ("{\n");
> +
> +  if (tdesc_print_intiailize_tdesc_p (function))
> +    {
>    printf_unfiltered
>      ("  struct target_desc *result = allocate_target_description ();\n");
> 
> @@ -2320,10 +2334,20 @@ maint_print_c_tdesc_cmd (char *args, int from_tty)
> 
>    printf_unfiltered ("\n");
>    printf_unfiltered ("  tdesc_%s = result;\n", function);
> +    }
> 
>    printf_unfiltered ("#if GDB_SELF_TEST\n");
> -  printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n",
> -		     filename_after_features.data (), function);
> +
> +  if (tdesc_print_intiailize_tdesc_p (function))
> +    {
> +      printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\",
> tdesc_%s);\n",
> +			 filename_after_features.data (), function);
> +    }
> +  else
> +    {
> +      printf_unfiltered ("  selftests::record_xml (\"%s\");\n",
> +			 filename_after_features.data ());
> +    }
>    printf_unfiltered ("#endif /* GDB_SELF_TEST */\n");
>    printf_unfiltered ("}\n");
>  }
> @@ -2341,12 +2365,32 @@ record_xml_tdesc (std::string xml_file, const struct
> target_desc *tdesc) lists.emplace_back (xml_file, tdesc);
>  }
> 
> +/* XML files, which generate C files and compiled into GDB.  */
> +
> +static std::vector<std::string> xml_files;
> +
> +void
> +record_xml (const char *xml_file)
> +{
> +  xml_files.emplace_back (xml_file);
> +}
> +
>  /* Test these GDB builtin target descriptions are identical to these which
>     are generated by the corresponding xml files.  */
> 
>  static void
>  xml_builtin_tdesc_test (void)
>  {
> +  /* Make sure we have a test for every xml target description.  */
> +  for (auto const &xml : xml_files)
> +    {
> +      auto r = std::find_if (std::begin (lists), std::end (lists),
> +			     [&xml](decltype (lists)::const_reference e)
> +			     -> bool {return e.first == xml;});
> +
> +      SELF_CHECK (r != std::end (lists));
> +    }
> +
>    std::string feature_dir (ldirname (__FILE__));
>    struct stat st;
> 
> diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
> index 78416ae..2667bcd 100644
> --- a/gdb/target-descriptions.h
> +++ b/gdb/target-descriptions.h
> @@ -266,6 +266,10 @@ namespace selftests {
> 
>  void record_xml_tdesc (std::string xml_file,
>  		       const struct target_desc *tdesc);
> +
> +/* Record c file generated by XML_FILE is compiled into GDB.  */
> +
> +void record_xml (const char *xml_file);
>  }
>  #endif
> 

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-16 12:00   ` Philipp Rudo
@ 2017-05-16 15:46     ` Yao Qi
  2017-05-17  9:09       ` Philipp Rudo
  2017-05-17 16:06     ` Pedro Alves
  1 sibling, 1 reply; 32+ messages in thread
From: Yao Qi @ 2017-05-16 15:46 UTC (permalink / raw)
  To: Philipp Rudo; +Cc: gdb-patches

Philipp Rudo <prudo@linux.vnet.ibm.com> writes:

Hi Philipp,
Thanks for your comments.

> The only thing you could have done to make it better is to get rid of the
> conversion to C altogether and create the target description directly by
> parsing the XML file. I don't see a benefit in the conversion and removing it
> would simplify the code even more.  But that is a long term goal and your patch
> set definitely helps towards that goal.

As I described in the commit log, "the reason that we transform xml files
to c files is that GDB is still able to have some target description
without xml parsing support." so that XML parsing is not a hard
requirement for GDB.  Nowadays, XML parsing is only used for remote
debugging.  In native debugging, GDB just uses these pre-generated
builtin target descriptions, without parsing any XML.

Suppose XML parsing is always there, at least on Linux host, GDB can
create a big buffer contains various target features in the runtime.  We
can do this on top of patch 6/7, like this, (take i386-linux as an
example),

/* Return a string having XML contents reflecting the right target
   description according to XCR0.  */

std::string
i386_linux_read_description (uint64_t xcr0)
{
   std::string buffer;

   buffer.append ("<?xml version="1.0"?>");
   buffer.append ("<!DOCTYPE target SYSTEM "gdb-target.dtd">");
   buffer.append ("<target>");
   buffer.append ("  <architecture>i386</architecture>");
   buffer.append ("  <osabi>GNU/Linux</osabi>");

   if (xcr0 & X86_XSTATE_X87)
     {
        /* Open 32bit-core.xml and copy its content here.  */
     }

   if (xcr0 & X86_XSTATE_SSE)
     {
       /* Open 32bit-sse.xml and copy its content here.  */
     }

    ....

   buffer.append ("</target>");

   return buffer;
}

Even further, this code can be shared between GDB and GDBserver, so that
GDBserver can compose the XML contents and send it to GDB.  Note that
GDBserver doesn't need to parse the XML.

-- 
Yao (齐尧)

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-16 15:46     ` Yao Qi
@ 2017-05-17  9:09       ` Philipp Rudo
  0 siblings, 0 replies; 32+ messages in thread
From: Philipp Rudo @ 2017-05-17  9:09 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

On Tue, 16 May 2017 16:46:11 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> 
> Hi Philipp,
> Thanks for your comments.
> 
> > The only thing you could have done to make it better is to get rid of the
> > conversion to C altogether and create the target description directly by
> > parsing the XML file. I don't see a benefit in the conversion and removing
> > it would simplify the code even more.  But that is a long term goal and
> > your patch set definitely helps towards that goal.
> 
> As I described in the commit log, "the reason that we transform xml files
> to c files is that GDB is still able to have some target description
> without xml parsing support." so that XML parsing is not a hard
> requirement for GDB.  Nowadays, XML parsing is only used for remote
> debugging.  In native debugging, GDB just uses these pre-generated
> builtin target descriptions, without parsing any XML.

Ok, I see my mistake now.  I thought XML parsing would always be present.
Then the target descriptions could be handled similar to syscalls.  But
that was clearly focused too much on GDB on Linux.  For other systems and
GDBserver this needn't necessarily work...

Sorry for the mistake.
 
> Suppose XML parsing is always there, at least on Linux host, GDB can
> create a big buffer contains various target features in the runtime.  We
> can do this on top of patch 6/7, like this, (take i386-linux as an
> example),
> 
> /* Return a string having XML contents reflecting the right target
>    description according to XCR0.  */
> 
> std::string
> i386_linux_read_description (uint64_t xcr0)
> {
>    std::string buffer;
> 
>    buffer.append ("<?xml version="1.0"?>");
>    buffer.append ("<!DOCTYPE target SYSTEM "gdb-target.dtd">");
>    buffer.append ("<target>");
>    buffer.append ("  <architecture>i386</architecture>");
>    buffer.append ("  <osabi>GNU/Linux</osabi>");
> 
>    if (xcr0 & X86_XSTATE_X87)
>      {
>         /* Open 32bit-core.xml and copy its content here.  */
>      }
> 
>    if (xcr0 & X86_XSTATE_SSE)
>      {
>        /* Open 32bit-sse.xml and copy its content here.  */
>      }
> 
>     ....
> 
>    buffer.append ("</target>");
> 
>    return buffer;
> }
> 
> Even further, this code can be shared between GDB and GDBserver, so that
> GDBserver can compose the XML contents and send it to GDB.  Note that
> GDBserver doesn't need to parse the XML.

That would be neat!

Thanks
Philipp

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi
  2017-05-16 12:00   ` Philipp Rudo
@ 2017-05-17 15:41   ` Pedro Alves
  2017-05-18  9:54     ` Yao Qi
  1 sibling, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2017-05-17 15:41 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 05/11/2017 04:55 PM, Yao Qi wrote:
>    filename = lbasename (target_description_filename);
> +  std::string filename_after_features (target_description_filename);
> +  auto loc = filename_after_features.rfind ("/features/");
> +
> +  if (loc != std::string::npos)
> +    filename_after_features = filename_after_features.substr (loc + 10);
> +
>    function = (char *) alloca (strlen (filename) + 1);
>    for (inp = filename, outp = function; *inp != '\0'; inp++)
>      if (*inp == '.')
> @@ -1979,9 +2080,64 @@ feature = tdesc_create_feature (result, \"%s\");\n",
>      }
>  
>    printf_unfiltered ("  tdesc_%s = result;\n", function);
> +
> +  printf_unfiltered ("#if GDB_SELF_TEST\n");
> +  printf_unfiltered ("  selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n",
> +		     filename_after_features.data (), function);

Since you're using the string as a C string -- c_str() is clearer
than data().

> +  printf_unfiltered ("#endif /* GDB_SELF_TEST */\n");
>    printf_unfiltered ("}\n");
>  }
>  
> +#if GDB_SELF_TEST
> +#include "selftest.h"
> +
> +namespace selftests {
> +
> +static std::vector<std::pair<std::string, const struct target_desc *>> lists;

I think this "lists" either needs a comment or a more descriptive name.

> +
> +void
> +record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc)
> +{
> +  lists.emplace_back (xml_file, tdesc);

Write:

 lists.emplace_back (std::move (xml_file), tdesc);

to avoid another copy.

Though, I can't tell why do we need to store a dynamically allocated
std::string, when seemingly a "const char *" would do?  All callers pass
in a string literal, and all you do with the strings in the list is
iterate over all list elements and build new strings from those.

> +}
> +
> +/* Test these GDB builtin target descriptions are identical to these which
> +   are generated by the corresponding xml files.  */
> +
> +static void
> +xml_builtin_tdesc_test (void)

(void) -> ()

> +{
> +  std::string feature_dir (ldirname (__FILE__));
> +  struct stat st;

Ugh.  Obviously this can't work if gdb is installed / copied elsewhere,
remote host testing, etc.

> +
> +  /* Look for the features directory.  If the directory of __FILE__ can't
> +     be found, __FILE__ is a file name with relative path.  Guess that
> +     GDB is executed in testsuite directory like ../gdb, because I don't
> +     expect that GDB is invoked somewhere else and run self tests.  */
> +  if (stat (feature_dir.data (), &st) < 0)
> +    {
> +      feature_dir.insert (0, SLASH_STRING);
> +      feature_dir.insert (0, "..");
> +
> +      /* If still can't find the path, something is wrong.  */
> +      SELF_CHECK (stat (feature_dir.data (), &st) == 0);

Do we want to flag this as an error / unit test failure?
Maybe it should be a warning instead?

> +    }
> +
> +  feature_dir = feature_dir + SLASH_STRING + "features";

  feature_dir += SLASH_STRING + "features";

> +
> +  for (auto const &e : lists)
> +    {
> +      std::string tdesc_xml = (feature_dir + SLASH_STRING + e.first);
> +      const struct target_desc *tdesc
> +	= file_read_description_xml (tdesc_xml.data ());
> +
> +      SELF_CHECK (tdesc != NULL);
> +      SELF_CHECK (tdesc_equals (tdesc, e.second));
> +    }
> +}
> +}
> +#endif /* GDB_SELF_TEST */
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_target_descriptions;
>  
> @@ -2022,4 +2178,8 @@ GDB will read the description from the target."),
>    add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
>  Print the current target description as a C source file."),
>  	   &maintenanceprintlist);
> +
> +#if GDB_SELF_TEST
> +  register_self_test (selftests::xml_builtin_tdesc_test);
> +#endif
>  }
> diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
> index 361ac97..78416ae 100644
> --- a/gdb/target-descriptions.h
> +++ b/gdb/target-descriptions.h
> @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *);
>  int tdesc_compatible_p (const struct target_desc *,
>  			const struct bfd_arch_info *);
>  
> +/* Compare target descriptions TDESC1 and TDESC2, return true if they
> +   are identical.  */
> +
> +bool tdesc_equals (const struct target_desc *tdesc1,
> +		   const struct target_desc *tdesc2);

Any reason this and the other equals functions aren't operator== implementations?
It's not obvious since the comments say "identical", which would maybe suggest that
there may be some property that is not being compared and thus this is not
strict value equality, but then function name says "equals".

> +
>  /* Return the string value of a property named KEY, or NULL if the
>     property was not specified.  */
>  
> @@ -253,4 +259,14 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
>  		       int regnum, int save_restore, const char *group,
>  		       int bitsize, const char *type);
>  
> +#if GDB_SELF_TEST
> +namespace selftests {
> +
> +/* Record the target description TDESC generated by XML_FILE.  */
> +

> +void record_xml_tdesc (std::string xml_file,
> +		       const struct target_desc *tdesc);

Thanks,
Pedro Alves

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

* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux " Yao Qi
  2017-05-11 18:14   ` John Baldwin
@ 2017-05-17 15:43   ` Pedro Alves
  2017-05-18 15:12     ` Yao Qi
  1 sibling, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2017-05-17 15:43 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 05/11/2017 04:55 PM, Yao Qi wrote:

> +#if GDB_SELF_TEST
> +  struct xml_and_mask
> +  {
> +    const char *xml_file_name;
> +    uint64_t mask;
> +  };

Minor nit: I'd instinctively find it more natural to read the
list below as { input, output }  (left to right), while you
have have { output, input }.

> +
> +  struct xml_and_mask array[] = {
> +    { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK },
> +    { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK },
> +    { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK },
> +    { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK },
> +    { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK },
> +    { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK },
> +    { "i386/i386-avx-mpx-avx512-pku-linux.xml",
> +      X86_XSTATE_AVX_MPX_AVX512_PKU_MASK },

About these xml files.  Is your idea that:

 #1 - we remove these xml description files at some point, keeping only
      the description fragments which are currently xi:included by the xml
      files above?
 #2 - or, we'll still continue adding new xml files and grow this
      list here?
 #3 - or, we'll keep the existing xml files as representative / legacy,
      and use them in the unit tests going forward, just to make sure
      the machinery builds correct descriptions?

(I don't expect to answer to be #2, I just put it there for completeness.)

> +  };
> +
> +  for (auto &a : array)
> +    {
> +      auto tdesc = i386_linux_read_description (a.mask);
> +
> +      selftests::record_xml_tdesc (a.xml_file_name, tdesc);
> +    }
> +#endif /* GDB_SELF_TEST */
>  }
> 

Thanks,
Pedro Alves

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

* Re: [RFC 4/7] Share code in initialize_tdesc_ functions
  2017-05-16 12:02   ` Philipp Rudo
@ 2017-05-17 15:43     ` Pedro Alves
  2017-05-18 11:21       ` Yao Qi
  0 siblings, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2017-05-17 15:43 UTC (permalink / raw)
  To: Philipp Rudo, Yao Qi; +Cc: gdb-patches

On 05/16/2017 01:02 PM, Philipp Rudo wrote:
> Hi Yao,
> 
> On Thu, 11 May 2017 21:55:04 +0100
> Yao Qi <qiyaoltc@gmail.com> wrote:
> 
>> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
>> index 754f15b..a81ae0d 100644
>> --- a/gdb/target-descriptions.c
>> +++ b/gdb/target-descriptions.c
> 
> [...]
> 
>> @@ -54,7 +57,10 @@ typedef struct tdesc_reg
>>    char *name;
>>  
>>    /* The register number used by this target to refer to this
>> -     register.  This is used for remote p/P packets and to determine
>> +     register.  Positive number means it is got by increasing
>> +     the previous register number by one.  Negative number means
>> +     it is got from "regnum" attribute in the xml file.
>> +     This is used for remote p/P packets and to determine
>>       the ordering of registers in the remote g/G packets.  */
>>    long target_regnum;
> 
> I'm not really sure if I like your idea with positive/negative regnums.  It
> could easily lead to hard to find bugs because someone forgot a '-' somewhere.
> I would prefer an extra field like 
> 
> bool explicit_regnum;
> or
> bool automatic_regnum;
> 
> [...]
> 
>> +/* Return the unique name of FEATURE.  The uniqueness means different
>> +   target description features have different values.  This is used
>> +   to differentiate different features with the same name.  */
>> +
>> +static std::string
>> +tdesc_feature_unique_name (const struct tdesc_feature* feature)
>> +{
>> +  std::string name = std::string (feature->name);
>> +
>> +  if (name.compare ("org.gnu.gdb.arm.m-profile") == 0)
>> +    {
>> +      struct tdesc_reg *reg;
>> +
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	if (reg->type != NULL && strcmp (reg->type, "arm_fpa_ext") == 0)
>> +	  {
>> +	    name.append ("_fpa");
>> +	    break;
>> +	  }
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.arm.vfp") == 0)
>> +    {
>> +      name.append ("_");
>> +      name.append (std::to_string (VEC_length (tdesc_reg_p,
>> +					       feature->registers) - 1));
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.i386.core") == 0)
>> +    {
>> +      struct tdesc_reg *reg = NULL;
>> +
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	{
>> +	  if (strcmp (reg->name, "ebp") == 0
>> +	      || strcmp (reg->name, "rbp") == 0)
>> +	    break;
>> +	}
>> +
>> +      name.append ("_");
>> +      if (strcmp (reg->name, "ebp") == 0)
>> +	name.append ("i386");
>> +      else
>> +	{
>> +	  if (strcmp (reg->type, "data_ptr") == 0)
>> +	    name.append ("amd64");
>> +	  else
>> +	    name.append ("x32");
>> +	}
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.power.core") == 0)
>> +    {
>> +      struct tdesc_reg *reg = NULL;
>> +      struct tdesc_reg *reg_mq = NULL;
>> +      struct tdesc_reg *reg_r0 = NULL;
>> +
>> +      /* Four variants.  */
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	{
>> +	  if (strcmp (reg->name, "r0") == 0)
>> +	    reg_r0 = reg;
>> +	  if (strcmp (reg->name, "mq") == 0)
>> +	    reg_mq = reg;
>> +
>> +	  if (reg_r0 != NULL && reg_mq != NULL)
>> +	    break;
>> +	}
>> +
>> +      name.append ("_");
>> +      if (reg_mq == NULL)
>> +	name.append (std::to_string (reg_r0->bitsize));
>> +      else
>> +	{
>> +	  if (reg_mq->target_regnum == -124)
>> +	    name.append ("601");
>> +	  else
>> +	    name.append ("rs6000");
>> +	}
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.power.fpu") == 0)
>> +    {
>> +      struct tdesc_reg *reg = NULL;
>> +
>> +      /* Three variants.  */
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	{
>> +	  if (strcmp (reg->name, "fpscr") == 0)
>> +	    break;
>> +	}
>> +
>> +      name.append ("_");
>> +
>> +      if (reg->target_regnum == -71)
>> +	name.append ("rs6000");
>> +      else
>> +	name.append (std::to_string (reg->bitsize));
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.power.linux") == 0)
>> +    {
>> +      struct tdesc_reg *reg = NULL;
>> +
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	{
>> +	  if (strcmp (reg->name, "orig_r3") == 0)
>> +	    break;
>> +	}
>> +
>> +      name.append (std::to_string (reg->bitsize));
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.s390.linux") == 0)
>> +    {
>> +      struct tdesc_reg *reg = NULL;
>> +      struct tdesc_reg *reg_orig_r2 = NULL;
>> +      struct tdesc_reg *reg_sc = NULL;
>> +      struct tdesc_reg *reg_lb = NULL;
>> +
>> +      /* Six variants.  */
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	{
>> +	  if (strcmp (reg->name, "orig_r2") == 0)
>> +	    reg_orig_r2 = reg;
>> +	  if (strcmp (reg->name, "last_break") == 0)
>> +	    reg_lb = reg;
>> +	  if (strcmp (reg->name, "system_call") == 0)
>> +	    reg_sc = reg;
>> +
>> +	  if (reg_orig_r2 != NULL && reg_sc != NULL && reg_lb != NULL)
>> +	    break;
>> +	}
>> +
>> +      name.append ("_");
>> +      name.append (std::to_string (reg_orig_r2->bitsize));
>> +
>> +      if (reg_lb != NULL)
>> +	{
>> +	  name.append ("_");
>> +
>> +	  if (reg_sc == NULL)
>> +	    name.append ("v1");
>> +	  else
>> +	    name.append ("v2");
>> +	}
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.s390.core") == 0)
>> +    {
>> +      struct tdesc_reg *reg = NULL;
>> +      struct tdesc_reg *reg_r0 = NULL;
>> +      struct tdesc_reg *reg_r0l = NULL;
>> +
>> +      /* Six variants.  */
>> +      for (int ix = 0;
>> +	   VEC_iterate (tdesc_reg_p, feature->registers, ix, reg);
>> +	   ix++)
>> +	{
>> +	  if (strcmp (reg->name, "r0") == 0)
>> +	    reg_r0 = reg;
>> +	  if (strcmp (reg->name, "r0l") == 0)
>> +	    reg_r0l = reg;
>> +
>> +	  if (reg_r0 != NULL && reg_r0l != NULL)
>> +	    break;
>> +	}
>> +
>> +      name.append ("_");
>> +      if (reg_r0l != NULL)
>> +	name.append ("s64");
>> +      else
>> +	name.append (std::to_string (reg_r0->bitsize));
>> +    }
>> +  else if (name.compare ("org.gnu.gdb.mips.cpu") == 0
>> +	   || name.compare ("org.gnu.gdb.mips.cp0") == 0
>> +	   || name.compare ("org.gnu.gdb.mips.fpu") == 0
>> +	   || name.compare ("org.gnu.gdb.mips.dsp") == 0
>> +	   || name.compare ("org.gnu.gdb.mips.linux") == 0)
>> +    {
>> +      struct tdesc_reg *reg = VEC_index (tdesc_reg_p, feature->registers, 0);
>> +
>> +      name.append (std::to_string (reg->bitsize));
>> +    }
>> +
>> +  std::replace (name.begin (), name.end (), '.', '_');
>> +  std::replace (name.begin (), name.end (), '-', '_');
>> +
>> +  return name;
>> +}
>> +
> 
> This function is actually the part I like least of your implementation.  Its
> way to long and barely readable.  The way I understand, it is needed to create
> unique macro and function names.  So why don't you simply use the filename of
> the XML file where the feature is defined?  It already is unique.  Or use an
> gdbarch hook so every architecture can decide for itself how to name them?

Agreed.  I was reading the patch and thinking how there must be a better
way to handle this.

Thanks,
Pedro Alves

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

* Re: [RFC 7/7] Remove builtin tdesc_i386_*_linux
  2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi
  2017-05-16 12:02   ` Philipp Rudo
@ 2017-05-17 15:46   ` Pedro Alves
  1 sibling, 0 replies; 32+ messages in thread
From: Pedro Alves @ 2017-05-17 15:46 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 05/11/2017 04:55 PM, Yao Qi wrote:
> 
> Function tdesc_print_intiailize_tdesc_p is added to control the scope

typo: "intiailize"

> 	* target-descriptions.c (tdesc_print_intiailize_tdesc_p): New

Ditto.  (multiple places).

> +static bool
> +tdesc_print_intiailize_tdesc_p (const char *function)

In the code too.

Thanks,
Pedro Alves

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-16 12:00   ` Philipp Rudo
  2017-05-16 15:46     ` Yao Qi
@ 2017-05-17 16:06     ` Pedro Alves
  2017-05-30  8:00       ` Philipp Rudo
  1 sibling, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2017-05-17 16:06 UTC (permalink / raw)
  To: Philipp Rudo, Yao Qi; +Cc: gdb-patches

On 05/16/2017 01:00 PM, Philipp Rudo wrote:

>> +  /* Look for the features directory.  If the directory of __FILE__ can't
>> +     be found, __FILE__ is a file name with relative path.  Guess that
>> +     GDB is executed in testsuite directory like ../gdb, because I don't
>> +     expect that GDB is invoked somewhere else and run self tests.  */
>> +  if (stat (feature_dir.data (), &st) < 0)
>> +    {
>> +      feature_dir.insert (0, SLASH_STRING);
>> +      feature_dir.insert (0, "..");
> 
> This would be a perfect spot for concat_path (patch 2 from my lk series
> https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html).
> Then it would read
> 
> feature_dir = concat_path ("..", feature_dir);
> 
> I actually want to bring some of my patches upstream (mostly the
> s390-linux-tdep split up patch) to reduce maintenance cost of my
> series.  This would be a good time for this patch, too.

Could that patch be split out of the series, and justified on its
own grounds?  Is there some representative place in the codebase
that you could cleanup a bit using the new API, just to show it
working nicely?  Also, a unit test would be nice.

Also, and most importantly, seems to me that patch still has
a lot inefficiency related to std::string copying, e.g.:

 +static inline unsigned long
 +approx_path_length (std::initializer_list<std::string> args,
 +		   std::string dir_separator)

This is passing in the strings by value / copy.  That looks like
an aweful lot of malloc/free/strdup behind the scenes.

I still think that if we're adding some utility for building
paths from dir components, that it'd be preferred to model
the API on (a small subset of) std::experimental::filesystem::path,
since in a few years that's the API that everyone learning C++ will
be learning.

Thanks,
Pedro Alves

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-17 15:41   ` Pedro Alves
@ 2017-05-18  9:54     ` Yao Qi
  2017-05-18 11:34       ` Pedro Alves
  0 siblings, 1 reply; 32+ messages in thread
From: Yao Qi @ 2017-05-18  9:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

>> +{
>> +  std::string feature_dir (ldirname (__FILE__));
>> +  struct stat st;
>
> Ugh.  Obviously this can't work if gdb is installed / copied elsewhere,
> remote host testing, etc.
>

I thought about this, but I can't figure out one better than __FILE__.
What I want to do is to find srcdir, and open these xml files during
unit tests.  Since it is a unit test, I expect gdb is executed in either
builddir/gdb or builddir/gdb/testsuite.  I don't see a case that people
build gdb in one place, and run unit/self tests somewhere else.

>> +
>> +  /* Look for the features directory.  If the directory of __FILE__ can't
>> +     be found, __FILE__ is a file name with relative path.  Guess that
>> +     GDB is executed in testsuite directory like ../gdb, because I don't
>> +     expect that GDB is invoked somewhere else and run self tests.  */
>> +  if (stat (feature_dir.data (), &st) < 0)
>> +    {
>> +      feature_dir.insert (0, SLASH_STRING);
>> +      feature_dir.insert (0, "..");
>> +
>> +      /* If still can't find the path, something is wrong.  */
>> +      SELF_CHECK (stat (feature_dir.data (), &st) == 0);
>
> Do we want to flag this as an error / unit test failure?
> Maybe it should be a warning instead?
>

We can skip this test if it can't find "features" directory in source, but
something wrong can be easily ignored if we do so.

>> --- a/gdb/target-descriptions.h
>> +++ b/gdb/target-descriptions.h
>> @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *);
>>  int tdesc_compatible_p (const struct target_desc *,
>>  			const struct bfd_arch_info *);
>>  
>> +/* Compare target descriptions TDESC1 and TDESC2, return true if they
>> +   are identical.  */
>> +
>> +bool tdesc_equals (const struct target_desc *tdesc1,
>> +		   const struct target_desc *tdesc2);
>
> Any reason this and the other equals functions aren't operator==
> implementations?

tdesc_reg, tdesc_type, tdesc_feature and target_desc should be
class-fied first, including adding proper ctor, dtor, etc.  I thought it
must be a lot of work, so I don't do that.  I can do that if it doesn't
take me much time.

> It's not obvious since the comments say "identical", which would maybe
> suggest that
> there may be some property that is not being compared and thus this is not
> strict value equality, but then function name says "equals".

I think "identical" implies "strict value equality".  The information I
want to deliver is that this function compares all properties, return
true if they are exactly the same.  Is it this better,

/* Return true if target description TDESC1 and TDESC2 are equal.  */

-- 
Yao (齐尧)

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

* Re: [RFC 4/7] Share code in initialize_tdesc_ functions
  2017-05-17 15:43     ` Pedro Alves
@ 2017-05-18 11:21       ` Yao Qi
  0 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-18 11:21 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Philipp Rudo, gdb-patches

Pedro Alves <palves@redhat.com> writes:

Hi Philipp and Pedro,
Thanks for your comments.

>> This function is actually the part I like least of your implementation.  Its
>> way to long and barely readable.  The way I understand, it is needed to create
>> unique macro and function names.  So why don't you simply use the filename of
>> the XML file where the feature is defined?  It already is unique.  Or use an
>> gdbarch hook so every architecture can decide for itself how to name them?
>
> Agreed.  I was reading the patch and thinking how there must be a better
> way to handle this.

I dislike my implementation either.  I thought about using xml feature
file name to distinguish features with the same name.  However it
doesn't work because the file name is lost after processing xi:include.
GDB processes XML target descriptions in two steps,

 1) process xi:include, copy feature xml files into a buffer,
 2) parse the buffer with gdb-target.dtd then,

All tdesc_features are created in step 2, and GDB doesn't know what file
is this feature from.  For example, there is an XML target description,

<target>
  <xi:include href="32bit-core.xml"/>
</target>

after step 1, the buffer contains XML contents from 32bit-core.xml,

<target>
  <feature name="org.gnu.gdb.i386.core">
  ...
  </feature>
</target>

GDB starts parse it in buffer, but doesn't know where is this feature
from.  I'll think about gdbarch hook you suggested.

-- 
Yao (齐尧)

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-18  9:54     ` Yao Qi
@ 2017-05-18 11:34       ` Pedro Alves
  2017-05-19 15:47         ` Yao Qi
  0 siblings, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2017-05-18 11:34 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 05/18/2017 10:54 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>> +{
>>> +  std::string feature_dir (ldirname (__FILE__));
>>> +  struct stat st;
>>
>> Ugh.  Obviously this can't work if gdb is installed / copied elsewhere,
>> remote host testing, etc.
>>
> 
> I thought about this, but I can't figure out one better than __FILE__.
> What I want to do is to find srcdir, and open these xml files during
> unit tests.  Since it is a unit test, I expect gdb is executed in either
> builddir/gdb or builddir/gdb/testsuite.  I don't see a case that people
> build gdb in one place, and run unit/self tests somewhere else.

Not sure -- does it work in remote host testing scenarios?
Recall that unit testing is invoked by the normal testsuite, from
gdb.gdb/unittest.exp.

An alternative would be to add a specific command to run these tests,
which would take the path as argument, like
"maint check xml-descriptions /path/to/features/", and then run that
from gdb.gdb/unittest.exp.

> 
>>> +
>>> +  /* Look for the features directory.  If the directory of __FILE__ can't
>>> +     be found, __FILE__ is a file name with relative path.  Guess that
>>> +     GDB is executed in testsuite directory like ../gdb, because I don't
>>> +     expect that GDB is invoked somewhere else and run self tests.  */
>>> +  if (stat (feature_dir.data (), &st) < 0)
>>> +    {
>>> +      feature_dir.insert (0, SLASH_STRING);
>>> +      feature_dir.insert (0, "..");
>>> +
>>> +      /* If still can't find the path, something is wrong.  */
>>> +      SELF_CHECK (stat (feature_dir.data (), &st) == 0);
>>
>> Do we want to flag this as an error / unit test failure?
>> Maybe it should be a warning instead?
>>
> 
> We can skip this test if it can't find "features" directory in source, but
> something wrong can be easily ignored if we do so.
> 
>>> --- a/gdb/target-descriptions.h
>>> +++ b/gdb/target-descriptions.h
>>> @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *);
>>>  int tdesc_compatible_p (const struct target_desc *,
>>>  			const struct bfd_arch_info *);
>>>  
>>> +/* Compare target descriptions TDESC1 and TDESC2, return true if they
>>> +   are identical.  */
>>> +
>>> +bool tdesc_equals (const struct target_desc *tdesc1,
>>> +		   const struct target_desc *tdesc2);
>>
>> Any reason this and the other equals functions aren't operator==
>> implementations?
> 
> tdesc_reg, tdesc_type, tdesc_feature and target_desc should be
> class-fied first, including adding proper ctor, dtor, etc.

There's no such requirement in the language.

> I thought it
> must be a lot of work, so I don't do that.  I can do that if it doesn't
> take me much time.

But you don't need to do that to implement operators.

> 
>> It's not obvious since the comments say "identical", which would maybe
>> suggest that
>> there may be some property that is not being compared and thus this is not
>> strict value equality, but then function name says "equals".
> 
> I think "identical" implies "strict value equality".  

Yeah, I think I got it backwards (been a long time since I used a programming
language that has such a distinction), but the point still stands -- "identical"
and "equals" can mean slightly different things so I'd rather not mix them up
to avoid confusion.

> The information I
> want to deliver is that this function compares all properties, return
> true if they are exactly the same.  Is it this better,
> 
> /* Return true if target description TDESC1 and TDESC2 are equal.  */

Sure, that makes the comment matches the function name, so it's fine.

Say "target descriptions", plural, though.

Thanks,
Pedro Alves

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

* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-17 15:43   ` Pedro Alves
@ 2017-05-18 15:12     ` Yao Qi
  2017-05-19 10:15       ` Pedro Alves
  0 siblings, 1 reply; 32+ messages in thread
From: Yao Qi @ 2017-05-18 15:12 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

>> +
>> +  struct xml_and_mask array[] = {
>> +    { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK },
>> +    { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK },
>> +    { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK },
>> +    { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK },
>> +    { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK },
>> +    { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK },
>> +    { "i386/i386-avx-mpx-avx512-pku-linux.xml",
>> +      X86_XSTATE_AVX_MPX_AVX512_PKU_MASK },
>
> About these xml files.  Is your idea that:
>
>  #1 - we remove these xml description files at some point, keeping only
>       the description fragments which are currently xi:included by the xml
>       files above?
>  #2 - or, we'll still continue adding new xml files and grow this
>       list here?
>  #3 - or, we'll keep the existing xml files as representative / legacy,
>       and use them in the unit tests going forward, just to make sure
>       the machinery builds correct descriptions?
>
> (I don't expect to answer to be #2, I just put it there for completeness.)

In short, #2 and #3.

IIUC, you are not against "continue adding new xml files" in #2, you are
against that we have to maintain the list here as we add new xml files.
Even we finished the transition for all ports to dynamic tdesc creation,
we still need to add new xml files to features/ directory for new
hardware features, because GDBserver needs them (regformats/*.dat need
them).  We may add i386/i386-avx-mpx-avx512-linux.xml, GDB
doesn't need it, but GDBserver still needs it.  Given we need to add
such .xml file, it is better to grow the list so that we can do the
test.  So the answer is #2.  Suppose one day, we don't need these XML
files to generate regformats/*.dat for GDBserver, we can remove all
i386-*linux.xml files except i386-avx-mpx-avx512-pku-linux.xml, but I
still want to move them to somewhere in testsuite directory to keep them
as tests.  Then, it becomes #3.

The purpose of this test is to make sure these lazily/dynamically
created tdesc (using create_feature_org_gnu_gdb_XXX functions) equal to
the tdesc created from XML files.  The dynamic tdesc creation is still
new, needs more tests.

During the process that we change other targets (amd64, arm, mips, etc)
to the dynamic tdesc creation, the lists like this are expected to be
added for each target.

When we finish the transition to dynamic tdesc creation for all ports,
and dynamic tdesc creation is quite stable, we can remove all these
tests and lists, but I prefer to keeping the tests.

Before we remove these tests and lists, we still need to update the
lists when,

 1) add a new port with target description, a new list should be added
 in its -tdep.c file.
 2) add a new xml target description for the existing port,

Let me think about how to generate this list instead of manually
maintain this list.

-- 
Yao (齐尧)

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

* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-18 15:12     ` Yao Qi
@ 2017-05-19 10:15       ` Pedro Alves
  2017-05-19 14:27         ` Yao Qi
  0 siblings, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2017-05-19 10:15 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 05/18/2017 04:12 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>> +
>>> +  struct xml_and_mask array[] = {
>>> +    { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK },
>>> +    { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK },
>>> +    { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK },
>>> +    { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK },
>>> +    { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK },
>>> +    { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK },
>>> +    { "i386/i386-avx-mpx-avx512-pku-linux.xml",
>>> +      X86_XSTATE_AVX_MPX_AVX512_PKU_MASK },
>>
>> About these xml files.  Is your idea that:
>>
>>  #1 - we remove these xml description files at some point, keeping only
>>       the description fragments which are currently xi:included by the xml
>>       files above?
>>  #2 - or, we'll still continue adding new xml files and grow this
>>       list here?
>>  #3 - or, we'll keep the existing xml files as representative / legacy,
>>       and use them in the unit tests going forward, just to make sure
>>       the machinery builds correct descriptions?
>>
>> (I don't expect to answer to be #2, I just put it there for completeness.)
> 
> In short, #2 and #3.
> 
> IIUC, you are not against "continue adding new xml files" in #2, you are
> against that we have to maintain the list here as we add new xml files.

Well, I wasn't ever expecting we'd end up with #2.  I guess I don't see
the point of going through all of this if we end up with having to add
manually-written xml descriptions covering all feature combinations, anyway?

I could see keeping the xml files pushed in the tree, but make them
generated instead?  It could be gdb that generates them from that
xml_and_mask array.  We'd share the code with gdbserver, so that gdbserver
would generate xml files on the fly to send to gdb.

Or we could do the generation completely outside gdb, with some file
describing the potential combinations (in spirit similar to that xml_and_mask
array), much like we generate the syscall xml files.

> Even we finished the transition for all ports to dynamic tdesc creation,
> we still need to add new xml files to features/ directory for new
> hardware features, because GDBserver needs them (regformats/*.dat need
> them).  We may add i386/i386-avx-mpx-avx512-linux.xml, GDB
> doesn't need it, but GDBserver still needs it.  Given we need to add
> such .xml file, it is better to grow the list so that we can do the
> test.  So the answer is #2.  Suppose one day, we don't need these XML
> files to generate regformats/*.dat for GDBserver, we can remove all
> i386-*linux.xml files except i386-avx-mpx-avx512-pku-linux.xml, but I
> still want to move them to somewhere in testsuite directory to keep them
> as tests.  Then, it becomes #3.

OK.  But if there's no plan to do the gdbserver side of the work,
I kind of call into question the more-complicated state we're getting
into, for an indeterminately long time.

> The purpose of this test is to make sure these lazily/dynamically
> created tdesc (using create_feature_org_gnu_gdb_XXX functions) equal to
> the tdesc created from XML files.  The dynamic tdesc creation is still
> new, needs more tests.
> 
> During the process that we change other targets (amd64, arm, mips, etc)
> to the dynamic tdesc creation, the lists like this are expected to be
> added for each target.
> 
> When we finish the transition to dynamic tdesc creation for all ports,
> and dynamic tdesc creation is quite stable, we can remove all these
> tests and lists, but I prefer to keeping the tests.

I think most ports are not troublesome, WRT to explosion of
target description feature set combinations.  Maybe a better approach
would be to fully transition an architecture all the way to
dynamic generation, gdb and gdbserver?

> 
> Before we remove these tests and lists, we still need to update the
> lists when,
> 
>  1) add a new port with target description, a new list should be added
>  in its -tdep.c file.
>  2) add a new xml target description for the existing port,
> 
> Let me think about how to generate this list instead of manually
> maintain this list.

Thanks,
Pedro Alves

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

* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions
  2017-05-19 10:15       ` Pedro Alves
@ 2017-05-19 14:27         ` Yao Qi
  0 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-19 14:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

> Well, I wasn't ever expecting we'd end up with #2.  I guess I don't see
> the point of going through all of this if we end up with having to add
> manually-written xml descriptions covering all feature combinations, anyway?
>

These changes are about GDB, but we still need manually-written xml
files because GDBserver still needs it.  There are still some benefits,

 - GDB binary becomes smaller, see
   https://sourceware.org/ml/gdb-patches/2017-05/msg00299.html

 - We don't have to generate a lot of gdb/features/*.c files, so don't
   need to include them.  In i386-linux, we only need
   features/i386/i386-avx-mpx-avx512-pku-linux.c and all other
   i386*linux.c can be removed.

> I could see keeping the xml files pushed in the tree, but make them
> generated instead?  It could be gdb that generates them from that
> xml_and_mask array.  We'd share the code with gdbserver, so that gdbserver
> would generate xml files on the fly to send to gdb.
>
> Or we could do the generation completely outside gdb, with some file
> describing the potential combinations (in spirit similar to that xml_and_mask
> array), much like we generate the syscall xml files.
>

I try to stop using the approach we pre-generate them (both xml files
and c files).

>> Even we finished the transition for all ports to dynamic tdesc creation,
>> we still need to add new xml files to features/ directory for new
>> hardware features, because GDBserver needs them (regformats/*.dat need
>> them).  We may add i386/i386-avx-mpx-avx512-linux.xml, GDB
>> doesn't need it, but GDBserver still needs it.  Given we need to add
>> such .xml file, it is better to grow the list so that we can do the
>> test.  So the answer is #2.  Suppose one day, we don't need these XML
>> files to generate regformats/*.dat for GDBserver, we can remove all
>> i386-*linux.xml files except i386-avx-mpx-avx512-pku-linux.xml, but I
>> still want to move them to somewhere in testsuite directory to keep them
>> as tests.  Then, it becomes #3.
>
> OK.  But if there's no plan to do the gdbserver side of the work,
> I kind of call into question the more-complicated state we're getting
> into, for an indeterminately long time.

I do plan to do GDBserver side, but I don't have a clear picture on how
to do it yet.  I post this RFC, because this is only about GDB.  This
series can make GDB better and keep GDBserver unchanged, it is still a
progress to me.

>
>> The purpose of this test is to make sure these lazily/dynamically
>> created tdesc (using create_feature_org_gnu_gdb_XXX functions) equal to
>> the tdesc created from XML files.  The dynamic tdesc creation is still
>> new, needs more tests.
>> 
>> During the process that we change other targets (amd64, arm, mips, etc)
>> to the dynamic tdesc creation, the lists like this are expected to be
>> added for each target.
>> 
>> When we finish the transition to dynamic tdesc creation for all ports,
>> and dynamic tdesc creation is quite stable, we can remove all these
>> tests and lists, but I prefer to keeping the tests.
>
> I think most ports are not troublesome, WRT to explosion of
> target description feature set combinations.  Maybe a better approach
> would be to fully transition an architecture all the way to
> dynamic generation, gdb and gdbserver?

ARM, MIPS, I386 and S390 have this problem to different extents.  I can
take GDBserver into account at this stage.  I did it last month, but
gave up because it was too brain-damaging.  Let me try again.

-- 
Yao (齐尧)

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-18 11:34       ` Pedro Alves
@ 2017-05-19 15:47         ` Yao Qi
  2017-05-22  8:51           ` Yao Qi
  0 siblings, 1 reply; 32+ messages in thread
From: Yao Qi @ 2017-05-19 15:47 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

> Not sure -- does it work in remote host testing scenarios?

It doesn't work in remote host.

> Recall that unit testing is invoked by the normal testsuite, from
> gdb.gdb/unittest.exp.
>
> An alternative would be to add a specific command to run these tests,
> which would take the path as argument, like
> "maint check xml-descriptions /path/to/features/", and then run that
> from gdb.gdb/unittest.exp.

It is difficult to get it working for remote host either, because we
need to copy all features/*.xml to the remote host.

Can we just skip this unit test if it can't find "features" directory?

  std::string feature_dir (ldirname (__FILE__));
  struct stat st;

  /* Look for the features directory.  If the directory of __FILE__ can't
     be found, __FILE__ is a file name with relative path.  Guess that
     GDB is executed in testsuite directory like ../gdb, because I don't
     expect that GDB is invoked somewhere else and run self tests.  */
  if (stat (feature_dir.data (), &st) < 0)
    {
      feature_dir.insert (0, SLASH_STRING);
      feature_dir.insert (0, "..");

      /* If still can't find the path, skip it.  */
      return;
    }

-- 
Yao (齐尧)

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-19 15:47         ` Yao Qi
@ 2017-05-22  8:51           ` Yao Qi
  0 siblings, 0 replies; 32+ messages in thread
From: Yao Qi @ 2017-05-22  8:51 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Yao Qi <qiyaoltc@gmail.com> writes:

> It is difficult to get it working for remote host either, because we
> need to copy all features/*.xml to the remote host.
>
> Can we just skip this unit test if it can't find "features" directory?

We need to copy these .xml files to host sooner or later.  Let me sort
it out.

-- 
Yao (齐尧)

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-17 16:06     ` Pedro Alves
@ 2017-05-30  8:00       ` Philipp Rudo
  2017-06-01 17:53         ` Philipp Rudo
  0 siblings, 1 reply; 32+ messages in thread
From: Philipp Rudo @ 2017-05-30  8:00 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

Hi Pedro,

sorry for the late answer but I was sick the last 10 days and just returned
yesterday...

On Wed, 17 May 2017 17:06:13 +0100
Pedro Alves <palves@redhat.com> wrote:

> On 05/16/2017 01:00 PM, Philipp Rudo wrote:
> 
> >> +  /* Look for the features directory.  If the directory of __FILE__ can't
> >> +     be found, __FILE__ is a file name with relative path.  Guess that
> >> +     GDB is executed in testsuite directory like ../gdb, because I don't
> >> +     expect that GDB is invoked somewhere else and run self tests.  */
> >> +  if (stat (feature_dir.data (), &st) < 0)
> >> +    {
> >> +      feature_dir.insert (0, SLASH_STRING);
> >> +      feature_dir.insert (0, "..");  
> > 
> > This would be a perfect spot for concat_path (patch 2 from my lk series
> > https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html).
> > Then it would read
> > 
> > feature_dir = concat_path ("..", feature_dir);
> > 
> > I actually want to bring some of my patches upstream (mostly the
> > s390-linux-tdep split up patch) to reduce maintenance cost of my
> > series.  This would be a good time for this patch, too.  
> 
> Could that patch be split out of the series, and justified on its
> own grounds?  Is there some representative place in the codebase
> that you could cleanup a bit using the new API, just to show it
> working nicely?  Also, a unit test would be nice.

The patch can be split from my series without problem.  I already pulled it
right to the beginning of the series, as it is independent of all other changes
in the set.

There are quite some places you can simplify by using the function.  Just grep
for SLASH_STRING to find most of them.  I also had a patch once where I
eliminated all use of SLASH_STRING using the concat_path.  Unfortunately it had
some problems with the mixture of C and C++ strings causing memory leaks and
read after free's.  I never had the time to clean that up and thus didn't send
it to the mailing list ...
 
> Also, and most importantly, seems to me that patch still has
> a lot inefficiency related to std::string copying, e.g.:
> 
>  +static inline unsigned long
>  +approx_path_length (std::initializer_list<std::string> args,
>  +		   std::string dir_separator)
> 
> This is passing in the strings by value / copy.  That looks like
> an aweful lot of malloc/free/strdup behind the scenes.

True, this can be optimized.

> I still think that if we're adding some utility for building
> paths from dir components, that it'd be preferred to model
> the API on (a small subset of) std::experimental::filesystem::path,
> since in a few years that's the API that everyone learning C++ will
> be learning.

Also true, let me see if I can hack something today.  Currently I don't like to
do what I should do and there are many meeting today anyway.  So this looks like
a perfect task for today ;)

Philipp
 
> Thanks,
> Pedro Alves
> 

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

* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml
  2017-05-30  8:00       ` Philipp Rudo
@ 2017-06-01 17:53         ` Philipp Rudo
  0 siblings, 0 replies; 32+ messages in thread
From: Philipp Rudo @ 2017-06-01 17:53 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 11936 bytes --]

Hi Pedro,

On Tue, 30 May 2017 10:00:07 +0200
Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:

[...]
 
> > I still think that if we're adding some utility for building
> > paths from dir components, that it'd be preferred to model
> > the API on (a small subset of) std::experimental::filesystem::path,
> > since in a few years that's the API that everyone learning C++ will
> > be learning.  
> 
> Also true, let me see if I can hack something today.  Currently I don't like
> to do what I should do and there are many meeting today anyway.  So this
> looks like a perfect task for today ;)

I looked into it a "little" and it got a "little" out of hand...

I'm going on vacation, starting tomorrow until next Wednesday, and didn't have
the time to include it in GDB yet.  So you find the code attached in two files
as separate "programm".  My plan is once I'm back (and finished the work I
should have done this week (sorry Yao and Omair)) to move gdb_path.h to
common/gdb_path.h and expand gdb_path-selftest.c with some static_asserts.

While working on it I noticed one detail in std::filesystem::path which could
be problematic for GDB.  The dir separator it uses (preferred_separator) is
'static constexpr char'.  So for cross debugging, with different dir separators,
it doesn't allow us to distinguish between host and target paths.  That's why
my implementation uses a simple "char" such that the dir separator can be
changed anytime.

Otherwise the implementation should be compatible with std::filesystem::path
http://en.cppreference.com/w/cpp/filesystem/path

Any comment is welcome.

Philipp


----------
To be added as binutils-gdb/gdb/common/gdb_path.h

gdb_path.h |  542 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 542 insertions(+)
---------- 

/* Filesystem path handling for GDB.

   Copyright (C) 2017 Free Software Foundation, Inc.

   This file is part of GDB.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

#ifndef __COMMON_GDB_PATH__
#define __COMMON_GDB_PATH__

#include <iomanip>
#include <iostream>
#include <list>
#include <string>

namespace gdb {
namespace filesystem {

/* Stub implementation of C++17 std::filesystem::path.  */

class path
{
public:
  path () noexcept {}

  template<class T>
  path (const T &s);

  /* Replace this path by OTHER.  */
  template<class T>
  path &operator= (const T &other);

  /* Append OTHER to this path.  */
  template<class T>
  path &operator/= (const T &other);

  /* Same as operator/=.  */
  template<class T>
  path &append (const T &other)
    {
      return *this /= other;
    }

  /* Append RHS to LHS and return the resulting path.  */
  template<class T>
  friend path operator/ (path lhs, const T &rhs);

  /* Add OTHER to this path without adding a dir seperator.  */
  template<class T>
  path &operator+= (const T &other);

  /* Same as operator+=.  */
  template<class T>
  path &concat (const T &other)
    {
      return *this += other;
    }

  /* Send this path to an ostream.  */
  friend std::ostream &operator<< (std::ostream &os, const path &p);

  /* Directory seperator.  */
  char preferred_seperator = '/';

  /* Erase the content of this path.  */
  void clear ();

  /* Concatenate the path and return it as a std::string.  */
  std::string string () const;

  /* Same as .string but returns const char *.  */
  const char *
  c_str () { return string ().c_str (); }

  /* Return the root_name of path, e.g. on DOS-like systems the drive name.  */
  path root_name () const;

  /* Return the root directory if it exists.  */
  path root_directory () const;

  /* Return the root path, i.e. root_name + root_directory.  */
  path root_path () const;

  /* Return the relative path from root_path.  */
  path relative_path () const;

  /* Return the parent path of this path.  */
  path parent_path () const;

  /* Return the filename component of this path.  */
  path filename () const;

  /* Return the stem of filename component of this path, i.e. the filename
     without extension.  */
  path stem () const;

  /* Return the extension of filename component of this path.  */
  path extension () const;

  /* Check if this path is empty.  */
  bool empty () const noexcept;

protected:
  /* Root of the filesystem, e.g. "C:" on dos-like filesystems.  */
  std::string m_root_name = "";

  /* Is this path absolute?  I.e. do we need to add a dir_sep at the
     beginning?  */
  bool m_absolute = false;

  /* Components of the path relative to root_path.  */
  std::list <std::string> m_path = {};

  /* Helper function.  */
  /* Does the given string sart with a root_name?  Needed for constructor.  */
  bool has_root_name (const std::string &s) const;

  /* Is the substring of S starting at position POS a dir_seperator?  */
  bool is_dirsep (const std::string &s, const size_t pos) const;

  /* Calculate the size (i.e. number of characters) of the total path
     including dir_sep's.  */
  size_t path_size () const;

  /* Append path component COMP to m_path.  */
  void append_component (std::string &comp);
};

/* See declaration.  */

inline bool
path::has_root_name (const std::string &s) const
{
//#if defined (HAVE_DOS_BASED_FILESYSTEM)
  /* Assume 'C:'-like root_names.  */
  return s.size () >= 2 && (std::isalpha (s[0]) && s[1] == ':');
//#else
  return false;
//#endif /* HAVE_DOS_BASED_FILESYSTEM */
}

/* See declaration.  */

inline bool
path::is_dirsep (const std::string &s, const size_t pos) const
{
  return s[pos] == preferred_seperator;
}

/* See declaration.  */

size_t
path::path_size () const
{
  size_t size = 0;

  for (auto &p : m_path)
    size += p.size () + 1;

  return size;
}

/* See declaration.  */

void
path::append_component (std::string &comp)
{
  if (comp == ".")
    return;

  if (comp == ".." && !m_path.empty ())
  {
    m_path.pop_back ();
    return;
  }

  m_path.push_back (comp);
}

/* Constructors.  */

template<class T>
path::path (const T &s)
  : path (std::string (s))
{}

template<>
path::path (const std::string &s)
{
  size_t pos = 0;

  if (has_root_name (s))
    {
      /* Assume 'C:'-like root_names.  */
      m_root_name = s.substr (pos, 2);
      pos += 2;
    }

  if (is_dirsep (s, pos))
    {
      m_absolute = true;
      pos++;
    }

  do
    {
      /* Remove duplicate dir_seps.  */
      while (s[pos] == preferred_seperator)
	pos++;

      size_t last_pos = pos;

      pos = s.find (preferred_seperator, pos);
      std::string comp (s.substr (last_pos, pos - last_pos));

      append_component (comp);
    }
  while (pos != std::string::npos);
}

template<>
path::path (const path &other)
{
  *this = other;
}

/* See declaration.  */

std::ostream &
operator<< (std::ostream &os, const path &p)
{
  os << std::quoted (p.string ());
  return os;
}

/* See declaration.  */

template<class T>
path &
path::operator= (const T &other)
{
  return *this = path (other);
}

/* See declaration.  */

template<>
path &
path::operator= (const path &other)
{
  preferred_seperator = other.preferred_seperator;

  m_root_name = other.m_root_name;
  m_absolute = other.m_absolute;
  m_path = other.m_path;

  return *this;
}

/* See declaration.  */

template<>
path &
path::operator/= (const path &other)
{
  for (auto comp : other.m_path)
    append_component (comp);

  return *this;
}

/* See declaration.  */

template<class T>
path &
path::operator/= (const T &other)
{
  return *this /= path (other);
}

/* See declaration.  */

template<class T>
path
operator/ (path lhs, const T &rhs)
{
  return lhs /= rhs;
}

/* See declaration.  */

template<>
path &
path::operator+= (const path &other)
{
  /* Ignore a possible root_name in other.  */
  m_path.back () += other.m_path.front ();
  m_path.insert (m_path.end (), ++other.m_path.begin (), other.m_path.end ());

  return *this;
}

/* See declaration.  */

template<class T>
path &
path::operator+= (const T &other)
{
  return *this += path (other);
}

/* See declaration.  */

void
path::clear ()
{
  m_root_name.clear ();
  m_path.clear ();
  m_absolute = false;
}

/* See declaration.  */

std::string
path::string () const
{
  std::string ret;

  if (empty ())
    return "";

  ret.reserve (path_size ());

  ret += m_root_name;
  if (m_absolute)
    ret += preferred_seperator;

  for (auto p = m_path.begin (); p != m_path.end (); p++)
    ret += *p + preferred_seperator;

  /* Remove trailing dir_sep.  */
  ret.pop_back ();

  return ret;
}

/* See declaration.  */

path
path::root_name () const
{
  return empty () ? path () : path (m_root_name);
}

/* See declaration.  */

path
path::root_directory () const
{
  return m_absolute ? path (&preferred_seperator) : path ();
}

/* See declaration.  */

path
path::root_path () const
{
  return root_name () / root_directory ();
}

/* See declaration.  */

path
path::relative_path () const
{
  if (empty ())
    return path ();

  path ret (*this);

  ret.m_root_name = "";
  ret.m_absolute = false;

  return ret;
}

/* See declaration.  */

path
path::parent_path () const
{
  if (empty ())
    return path ();

  path ret (*this);
  ret.m_path.pop_back ();

  return ret;
}

/* See declaration.  */

path
path::filename () const
{
  if (empty ())
    return path ();

  return path (m_path.back ());
}

/* See declaration.  */

path
path::stem () const
{
  if (empty ())
    return path ();

  auto pos = m_path.back ().rfind ('.');
  if (pos == 0)
    return path (m_path.back ());

  return path (m_path.back ().substr (0, pos));
}

/* See declaration.  */

path
path::extension () const
{
  if (empty ())
    return path ();

  auto pos = m_path.back ().rfind ('.');
  if (pos == 0 || pos == std::string::npos)
    return path ();

  return path (m_path.back ().substr (pos));
}

/* See declaration.  */

bool
path::empty () const noexcept
{
  return m_path.empty () && m_root_name.empty () && !m_absolute;
}

} /* namespace filesystem */

enum path_type
{
  HOST_PATH,
  TARGET_PATH,
};

class path : public gdb::filesystem::path
{
public:
  path () noexcept {};

  template<class T>
  path (const T& s, enum path_type _type = HOST_PATH)
    : gdb::filesystem::path (s), type (_type)
  {}

  /* Overload .string method to prepend target_prefix.  */
  std::string string () const;

  /* Substitute all occurrences of FROM to TO in this path.  */
  path &substitute (const std::string &from, const std::string &to);

  /* Type of this path (host or target).  */
  enum path_type type;

private:
  /* Prefix to be prepended to target paths.  */
  const std::string m_target_prefix = "target:";
};

/* See declaration.  */

std::string
path::string () const
{
  std::string ret;

  if (type == TARGET_PATH)
    {
      ret.reserve (path_size () + m_target_prefix.size ());
      ret = m_target_prefix + gdb::filesystem::path::string ();
    }
  else
    {
      ret = gdb::filesystem::path::string ();
    }

  return ret;
}

/* See declaration.  */

path &
path::substitute (const std::string &from, const std::string &to)
{
  if (from.empty ())
    return *this;

  /* Use TO == "" to remove the component completely.  */
  if (to.empty ())
    {
      m_path.remove (from);
      return *this;
    }

  for (auto it = m_path.begin (); it != m_path.end (); ++it)
    {
      if (*it != from)
	continue;

      *it = to;
    }

  return *this;
}

} /* namespace gdb */

#endif /*__COMMON_GDB_PATH__ */

[-- Attachment #2: gdb_path.h --]
[-- Type: text/x-chdr, Size: 10141 bytes --]

/* Filesystem path handling for GDB.

   Copyright (C) 2017 Free Software Foundation, Inc.

   This file is part of GDB.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

#ifndef __COMMON_GDB_PATH__
#define __COMMON_GDB_PATH__

#include <iomanip>
#include <iostream>
#include <list>
#include <string>

namespace gdb {
namespace filesystem {

/* Stub implementation of C++17 std::filesystem::path.  */

class path
{
public:
  path () noexcept {}

  template<class T>
  path (const T &s);

  /* Replace this path by OTHER.  */
  template<class T>
  path &operator= (const T &other);

  /* Append OTHER to this path.  */
  template<class T>
  path &operator/= (const T &other);

  /* Same as operator/=.  */
  template<class T>
  path &append (const T &other)
    {
      return *this /= other;
    }

  /* Append RHS to LHS and return the resulting path.  */
  template<class T>
  friend path operator/ (path lhs, const T &rhs);

  /* Add OTHER to this path without adding a dir seperator.  */
  template<class T>
  path &operator+= (const T &other);

  /* Same as operator+=.  */
  template<class T>
  path &concat (const T &other)
    {
      return *this += other;
    }

  /* Send this path to an ostream.  */
  friend std::ostream &operator<< (std::ostream &os, const path &p);

  /* Directory seperator.  */
  char preferred_seperator = '/';

  /* Erase the content of this path.  */
  void clear ();

  /* Concatenate the path and return it as a std::string.  */
  std::string string () const;

  /* Same as .string but returns const char *.  */
  const char *
  c_str () { return string ().c_str (); }

  /* Return the root_name of path, e.g. on DOS-like systems the drive name.  */
  path root_name () const;

  /* Return the root directory if it exists.  */
  path root_directory () const;

  /* Return the root path, i.e. root_name + root_directory.  */
  path root_path () const;

  /* Return the relative path from root_path.  */
  path relative_path () const;

  /* Return the parent path of this path.  */
  path parent_path () const;

  /* Return the filename component of this path.  */
  path filename () const;

  /* Return the stem of filename component of this path, i.e. the filename
     without extension.  */
  path stem () const;

  /* Return the extension of filename component of this path.  */
  path extension () const;

  /* Check if this path is empty.  */
  bool empty () const noexcept;

protected:
  /* Root of the filesystem, e.g. "C:" on dos-like filesystems.  */
  std::string m_root_name = "";

  /* Is this path absolute?  I.e. do we need to add a dir_sep at the
     beginning?  */
  bool m_absolute = false;

  /* Components of the path relative to root_path.  */
  std::list <std::string> m_path = {};

  /* Helper function.  */
  /* Does the given string sart with a root_name?  Needed for constructor.  */
  bool has_root_name (const std::string &s) const;

  /* Is the substring of S starting at position POS a dir_seperator?  */
  bool is_dirsep (const std::string &s, const size_t pos) const;

  /* Calculate the size (i.e. number of characters) of the total path
     including dir_sep's.  */
  size_t path_size () const;

  /* Append path component COMP to m_path.  */
  void append_component (std::string &comp);
};

/* See declaration.  */

inline bool
path::has_root_name (const std::string &s) const
{
//#if defined (HAVE_DOS_BASED_FILESYSTEM)
  /* Assume 'C:'-like root_names.  */
  return s.size () >= 2 && (std::isalpha (s[0]) && s[1] == ':');
//#else
  return false;
//#endif /* HAVE_DOS_BASED_FILESYSTEM */
}

/* See declaration.  */

inline bool
path::is_dirsep (const std::string &s, const size_t pos) const
{
  return s[pos] == preferred_seperator;
}

/* See declaration.  */

size_t
path::path_size () const
{
  size_t size = 0;

  for (auto &p : m_path)
    size += p.size () + 1;

  return size;
}

/* See declaration.  */

void
path::append_component (std::string &comp)
{
  if (comp == ".")
    return;

  if (comp == ".." && !m_path.empty ())
  {
    m_path.pop_back ();
    return;
  }

  m_path.push_back (comp);
}

/* Constructors.  */

template<class T>
path::path (const T &s)
  : path (std::string (s))
{}

template<>
path::path (const std::string &s)
{
  size_t pos = 0;

  if (has_root_name (s))
    {
      /* Assume 'C:'-like root_names.  */
      m_root_name = s.substr (pos, 2);
      pos += 2;
    }

  if (is_dirsep (s, pos))
    {
      m_absolute = true;
      pos++;
    }

  do
    {
      /* Remove duplicate dir_seps.  */
      while (s[pos] == preferred_seperator)
	pos++;

      size_t last_pos = pos;

      pos = s.find (preferred_seperator, pos);
      std::string comp (s.substr (last_pos, pos - last_pos));

      append_component (comp);
    }
  while (pos != std::string::npos);
}

template<>
path::path (const path &other)
{
  *this = other;
}

/* See declaration.  */

std::ostream &
operator<< (std::ostream &os, const path &p)
{
  os << std::quoted (p.string ());
  return os;
}

/* See declaration.  */

template<class T>
path &
path::operator= (const T &other)
{
  return *this = path (other);
}

/* See declaration.  */

template<>
path &
path::operator= (const path &other)
{
  preferred_seperator = other.preferred_seperator;

  m_root_name = other.m_root_name;
  m_absolute = other.m_absolute;
  m_path = other.m_path;

  return *this;
}

/* See declaration.  */

template<>
path &
path::operator/= (const path &other)
{
  for (auto comp : other.m_path)
    append_component (comp);

  return *this;
}

/* See declaration.  */

template<class T>
path &
path::operator/= (const T &other)
{
  return *this /= path (other);
}

/* See declaration.  */

template<class T>
path
operator/ (path lhs, const T &rhs)
{
  return lhs /= rhs;
}

/* See declaration.  */

template<>
path &
path::operator+= (const path &other)
{
  /* Ignore a possible root_name in other.  */
  m_path.back () += other.m_path.front ();
  m_path.insert (m_path.end (), ++other.m_path.begin (), other.m_path.end ());

  return *this;
}

/* See declaration.  */

template<class T>
path &
path::operator+= (const T &other)
{
  return *this += path (other);
}

/* See declaration.  */

void
path::clear ()
{
  m_root_name.clear ();
  m_path.clear ();
  m_absolute = false;
}

/* See declaration.  */

std::string
path::string () const
{
  std::string ret;

  if (empty ())
    return "";

  ret.reserve (path_size ());

  ret += m_root_name;
  if (m_absolute)
    ret += preferred_seperator;

  for (auto p = m_path.begin (); p != m_path.end (); p++)
    ret += *p + preferred_seperator;

  /* Remove trailing dir_sep.  */
  ret.pop_back ();

  return ret;
}

/* See declaration.  */

path
path::root_name () const
{
  return empty () ? path () : path (m_root_name);
}

/* See declaration.  */

path
path::root_directory () const
{
  return m_absolute ? path (&preferred_seperator) : path ();
}

/* See declaration.  */

path
path::root_path () const
{
  return root_name () / root_directory ();
}

/* See declaration.  */

path
path::relative_path () const
{
  if (empty ())
    return path ();

  path ret (*this);

  ret.m_root_name = "";
  ret.m_absolute = false;

  return ret;
}

/* See declaration.  */

path
path::parent_path () const
{
  if (empty ())
    return path ();

  path ret (*this);
  ret.m_path.pop_back ();

  return ret;
}

/* See declaration.  */

path
path::filename () const
{
  if (empty ())
    return path ();

  return path (m_path.back ());
}

/* See declaration.  */

path
path::stem () const
{
  if (empty ())
    return path ();

  auto pos = m_path.back ().rfind ('.');
  if (pos == 0)
    return path (m_path.back ());

  return path (m_path.back ().substr (0, pos));
}

/* See declaration.  */

path
path::extension () const
{
  if (empty ())
    return path ();

  auto pos = m_path.back ().rfind ('.');
  if (pos == 0 || pos == std::string::npos)
    return path ();

  return path (m_path.back ().substr (pos));
}

/* See declaration.  */

bool
path::empty () const noexcept
{
  return m_path.empty () && m_root_name.empty () && !m_absolute;
}

} /* namespace filesystem */

enum path_type
{
  HOST_PATH,
  TARGET_PATH,
};

class path : public gdb::filesystem::path
{
public:
  path () noexcept {};

  template<class T>
  path (const T& s, enum path_type _type = HOST_PATH)
    : gdb::filesystem::path (s), type (_type)
  {}

  /* Overload .string method to prepend target_prefix.  */
  std::string string () const;

  /* Substitute all occurrences of FROM to TO in this path.  */
  path &substitute (const std::string &from, const std::string &to);

  /* Type of this path (host or target).  */
  enum path_type type;

private:
  /* Prefix to be prepended to target paths.  */
  const std::string m_target_prefix = "target:";
};

/* See declaration.  */

std::string
path::string () const
{
  std::string ret;

  if (type == TARGET_PATH)
    {
      ret.reserve (path_size () + m_target_prefix.size ());
      ret = m_target_prefix + gdb::filesystem::path::string ();
    }
  else
    {
      ret = gdb::filesystem::path::string ();
    }

  return ret;
}

/* See declaration.  */

path &
path::substitute (const std::string &from, const std::string &to)
{
  if (from.empty ())
    return *this;

  /* Use TO == "" to remove the component completely.  */
  if (to.empty ())
    {
      m_path.remove (from);
      return *this;
    }

  for (auto it = m_path.begin (); it != m_path.end (); ++it)
    {
      if (*it != from)
	continue;

      *it = to;
    }

  return *this;
}

} /* namespace gdb */

#endif /*__COMMON_GDB_PATH__ */

[-- Attachment #3: gdb_path-selftest.c --]
[-- Type: text/x-c++src, Size: 2388 bytes --]

/* Self tests for filesystem path handling for GDB.

   Copyright (C) 2017 Free Software Foundation, Inc.

   This file is part of GDB.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

#include <iostream>

#include "gdb_path.h"

//namespace selftest {

namespace fs = gdb::filesystem;
static int i = 0;

template<class T>
void
print_path (T &p)
{
  std::cout << i++ << ":\n";

  std::cout << "string =        " << p.string () << "\n";
  std::cout << "c_str =         " << p.c_str () << "\n";

  std::cout << "root_name =     " << p.root_name () << "\n";
  std::cout << "root_dir =      " << p.root_directory () << "\n";
  std::cout << "root_path =     " << p.root_path () << "\n";
  std::cout << "relative_path = " << p.relative_path () << "\n";
  std::cout << "parent_path =   " << p.parent_path () << "\n";
  std::cout << "filename =      " << p.filename () << "\n";
  std::cout << "stem =          " << p.stem () << "\n";
  std::cout << "extension =     " << p.extension () << "\n";

  std::cout << "\n";
}

int main ()
{
  fs::path p ("/");

  print_path (p);

  p.clear ();


  p = "C:1/2///";
  print_path (p);
  p.clear ();


  p = "C:/1/2///";
  print_path (p);
  p.clear ();


  p = fs::path ("/1/x/y/z/../../..///2/.");
  fs::path q  = "3";
  fs::path r  = "4";
  fs::path s;

  s = q / r;
  p /= s;
  p /= std::string ("5");
  p /= "6";
  p.append (".7");

  p /= ".";
  p /= "x";
  p /= "..";

  print_path (p);

  p += fs::path (".foo");

  print_path (p);

  p += ".bar";

  print_path (p);

  fs::path t;
  print_path (t);


  gdb::path gp ("/a/b/c/$x/$y/e/f");
  print_path (gp);

  gp.type = gdb::TARGET_PATH;
  gp.substitute ("$x", "d");
  gp.substitute ("$y", "");
  print_path (gp);

  gp.clear ();
  gp /= "../foo";
  print_path (gp);

}

//} /* namespace selftest */

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

end of thread, other threads:[~2017-06-01 17:53 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi
2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi
2017-05-16 12:00   ` Philipp Rudo
2017-05-16 15:46     ` Yao Qi
2017-05-17  9:09       ` Philipp Rudo
2017-05-17 16:06     ` Pedro Alves
2017-05-30  8:00       ` Philipp Rudo
2017-06-01 17:53         ` Philipp Rudo
2017-05-17 15:41   ` Pedro Alves
2017-05-18  9:54     ` Yao Qi
2017-05-18 11:34       ` Pedro Alves
2017-05-19 15:47         ` Yao Qi
2017-05-22  8:51           ` Yao Qi
2017-05-11 15:55 ` [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Yao Qi
2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi
2017-05-16 12:02   ` Philipp Rudo
2017-05-17 15:46   ` Pedro Alves
2017-05-11 15:55 ` [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Yao Qi
2017-05-11 15:55 ` [RFC 5/7] Centralize i386 linux target descriptions Yao Qi
2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux " Yao Qi
2017-05-11 18:14   ` John Baldwin
2017-05-11 21:03     ` Yao Qi
2017-05-17 15:43   ` Pedro Alves
2017-05-18 15:12     ` Yao Qi
2017-05-19 10:15       ` Pedro Alves
2017-05-19 14:27         ` Yao Qi
2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii
2017-05-11 20:56   ` Yao Qi
2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi
2017-05-16 12:02   ` Philipp Rudo
2017-05-17 15:43     ` Pedro Alves
2017-05-18 11:21       ` Yao Qi

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