public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 8/8] Remove xml files from gdbserver
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (6 preceding siblings ...)
  2018-04-10 14:34 ` [PATCH v5 1/8] Commonise tdesc_reg Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-18  2:49 ` [PATCH v5 0/8] Remove gdbserver dependency on xml files Simon Marchi
  8 siblings, 0 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

For ports which use new target descriptions, this patch removes
the xml files from being built into gdbserver,

This patch is identical to the V4 version.

Alan.

2018-04-10 Alan Hayward  <alan.hayward@arm.com>

gdbserver/
	* configure.srv (aarch64*-*-linux*): Don't include xml.
	(i[34567]86-*-cygwin*): Likewise.
	(i[34567]86-*-linux*): Likewise.
	(i[34567]86-*-lynxos*): Likewise.
	(i[34567]86-*-mingw32ce*): Likewise.
	(i[34567]86-*-mingw*): Likewise.
	(i[34567]86-*-nto*): Likewise.
	(tic6x-*-uclinux): Likewise.
	(x86_64-*-linux*): Likewise.
	(x86_64-*-mingw*): Likewise.
	(x86_64-*-cygwin*): Likewise.
---
 gdb/gdbserver/configure.srv | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 087fd31426..ffeefb9b92 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -34,14 +34,6 @@ fi
 
 ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-cell32l-ipa.o powerpc-vsx32l-ipa.o powerpc-isa205-32l-ipa.o powerpc-isa205-altivec32l-ipa.o powerpc-isa205-vsx32l-ipa.o powerpc-e500l-ipa.o powerpc-64l-ipa.o powerpc-altivec64l-ipa.o powerpc-cell64l-ipa.o powerpc-vsx64l-ipa.o powerpc-isa205-64l-ipa.o powerpc-isa205-altivec64l-ipa.o powerpc-isa205-vsx64l-ipa.o"
 
-srv_i386_32bit_xmlfiles="i386/32bit-core.xml i386/32bit-sse.xml i386/32bit-avx.xml i386/32bit-avx512.xml i386/32bit-mpx.xml i386/32bit-pkeys.xml"
-srv_i386_64bit_xmlfiles="i386/64bit-core.xml i386/64bit-segments.xml i386/64bit-sse.xml i386/64bit-avx.xml i386/64bit-avx512.xml i386/x32-core.xml i386/64bit-mpx.xml i386/64bit-pkeys.xml"
-srv_i386_xmlfiles="i386/i386.xml $srv_i386_32bit_xmlfiles"
-srv_amd64_xmlfiles="i386/amd64.xml $srv_i386_64bit_xmlfiles"
-srv_i386_linux_xmlfiles="i386/32bit-linux.xml $srv_i386_32bit_xmlfiles"
-srv_amd64_linux_xmlfiles="i386/64bit-linux.xml $srv_i386_64bit_xmlfiles"
-
-
 # Linux object files.  This is so we don't have to repeat
 # these files over and over again.
 srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o linux-namespaces.o fork-child.o fork-inferior.o"
@@ -63,11 +55,6 @@ case "${target}" in
 			srv_tgtobj="$srv_tgtobj arch/aarch64.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
 			srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
-			srv_xmlfiles="aarch64.xml"
-			srv_xmlfiles="${srv_xmlfiles} aarch64-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} aarch64-fpu.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-core.xml arm/arm-vfpv3.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-with-neon.xml"
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
 			ipa_obj="linux-aarch64-ipa.o"
@@ -121,13 +108,10 @@ case "${target}" in
   i[34567]86-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
   i[34567]86-*-linux*)	srv_regobj="$srv_i386_linux_regobj"
-			srv_xmlfiles="$srv_i386_linux_xmlfiles"
 			if test "$gdb_cv_i386_is_x86_64" = yes ; then
 			    srv_regobj="$srv_regobj $srv_amd64_linux_regobj"
-			    srv_xmlfiles="${srv_xmlfiles} $srv_amd64_linux_xmlfiles"
 			    srv_tgtobj="amd64-linux-siginfo.o"
 			fi
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
@@ -145,9 +129,6 @@ case "${target}" in
   i[34567]86-*-lynxos*)	srv_regobj=""
 			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o fork-inferior.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_xmlfiles="i386/i386.xml"
-			srv_xmlfiles="${srv_xmlfiles} i386/32bit-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} i386/32bit-sse.xml"
 			srv_lynxos=yes
 			;;
   i[34567]86-*-mingw32ce*)
@@ -155,7 +136,6 @@ case "${target}" in
 			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			# hostio_last_error implementation is in win32-low.c
 			srv_hostio_err_objs=""
 			srv_mingw=yes
@@ -164,12 +144,10 @@ case "${target}" in
   i[34567]86-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_mingw=yes
 			;;
   i[34567]86-*-nto*)	srv_regobj=""
 			srv_tgtobj="nto-low.o nto-x86-low.o arch/i386.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_qnx="yes"
 			;;
   ia64-*-linux*)	srv_regobj=reg-ia64.o
@@ -370,9 +348,6 @@ case "${target}" in
                         else
 			  srv_regobj=""
                         fi
-			srv_xmlfiles="${srv_xmlfiles} tic6x-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} tic6x-gp.xml"
-			srv_xmlfiles="${srv_xmlfiles} tic6x-c6xp.xml"
 			srv_tgtobj="$srv_linux_obj linux-tic6x-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/tic6x.o"
 			srv_linux_regsets=yes
@@ -386,7 +361,6 @@ case "${target}" in
 			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
 			srv_tgtobj="${srv_tgtobj} x86-linux-dregs.o"
 			srv_tgtobj="${srv_tgtobj} amd64-linux-siginfo.o"
-			srv_xmlfiles="$srv_i386_linux_xmlfiles $srv_amd64_linux_xmlfiles"
 			srv_linux_usrregs=yes # This is for i386 progs.
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -397,13 +371,11 @@ case "${target}" in
   x86_64-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
-			srv_xmlfiles="$srv_i386_xmlfiles $srv_amd64_xmlfiles"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
 
   xtensa*-*-linux*)	srv_regobj=reg-xtensa.o
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 2/8] Commonise tdesc_feature
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 6/8] Create xml from target descriptions Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 4/8] Add tdesc osabi and architecture functions Alan Hayward
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

This patch commonises tdesc_feature and makes use of it in gdbserver tdesc.

Previously in gdbserver, target_desc was interchangeable with tdesc_feature.
Now gdbserver target_desc contains a vector of tdesc_feature.

I am now able to commonise tdesc_create_reg (wanted to do this in the
previous patch).

I also had to commonise tdesc_type. This is ok, because I will need them
in the next patch.

Identical to V4 except for removal of "struct type;" at the top of
common/tdesc.h

Alan.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* common/tdesc.c (tdesc_feature::accept): Move to here.
	(tdesc_feature::operator==): Likewise.
	(tdesc_create_reg): Likewise.
	* common/tdesc.h (tdesc_type_kind): Likewise.
	(struct tdesc_type): Likewise.
	(struct tdesc_feature): Likewise.
	* regformats/regdat.sh: Create a feature.
	* target-descriptions.c (tdesc_type_kind): Move from here.
	(tdesc_type): Likewise.
	(tdesc_type_up): Likewise.
	(tdesc_feature): Likewise.
	(tdesc_create_reg): Likewise.

gdbserver/
	* tdesc.c (~target_desc): Remove implictly deleted items.
	(init_target_desc): Iterate all features.
	(tdesc_get_features_xml): Use vector.
	(tdesc_create_feature): Create feature.
	* tdesc.h (tdesc_feature) Remove
	(target_desc): Add features.
---
 gdb/common/tdesc.c        |  58 ++++++++++++++++++
 gdb/common/tdesc.h        |  93 +++++++++++++++++++++++++++++
 gdb/gdbserver/tdesc.c     |  55 ++++++-----------
 gdb/gdbserver/tdesc.h     |  14 ++---
 gdb/regformats/regdat.sh  |   4 +-
 gdb/target-descriptions.c | 148 ----------------------------------------------
 6 files changed, 175 insertions(+), 197 deletions(-)

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index 8a3d620b04..eefb2016f6 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -33,3 +33,61 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
      have easy access to the containing feature when we want it later.  */
   tdesc_type = tdesc_named_type (feature, type.c_str ());
 }
+
+void tdesc_feature::accept (tdesc_element_visitor &v) const
+{
+  v.visit_pre (this);
+
+  for (const tdesc_type_up &type : types)
+    type->accept (v);
+
+  for (const tdesc_reg_up &reg : registers)
+    reg->accept (v);
+
+  v.visit_post (this);
+}
+
+bool tdesc_feature::operator== (const tdesc_feature &other) const
+{
+  if (name != other.name)
+    return false;
+
+  if (registers.size () != other.registers.size ())
+    return false;
+
+  for (int ix = 0; ix < registers.size (); ix++)
+    {
+      const tdesc_reg_up &reg1 = registers[ix];
+      const tdesc_reg_up &reg2 = other.registers[ix];
+
+      if (reg1 != reg2 && *reg1 != *reg2)
+	return false;
+      }
+
+  if (types.size () != other.types.size ())
+    return false;
+
+  for (int ix = 0; ix < types.size (); ix++)
+    {
+      const tdesc_type_up &type1 = types[ix];
+      const tdesc_type_up &type2 = other.types[ix];
+
+      if (type1 != type2 && *type1 != *type2)
+	return false;
+    }
+
+  return true;
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_create_reg (struct tdesc_feature *feature, const char *name,
+		  int regnum, int save_restore, const char *group,
+		  int bitsize, const char *type)
+{
+  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+				  group, bitsize, type);
+
+  feature->registers.emplace_back (reg);
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 0ec45af3b4..b501dfa995 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -131,6 +131,99 @@ struct tdesc_reg : tdesc_element
 
 typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
 
+enum tdesc_type_kind
+{
+  /* Predefined types.  */
+  TDESC_TYPE_BOOL,
+  TDESC_TYPE_INT8,
+  TDESC_TYPE_INT16,
+  TDESC_TYPE_INT32,
+  TDESC_TYPE_INT64,
+  TDESC_TYPE_INT128,
+  TDESC_TYPE_UINT8,
+  TDESC_TYPE_UINT16,
+  TDESC_TYPE_UINT32,
+  TDESC_TYPE_UINT64,
+  TDESC_TYPE_UINT128,
+  TDESC_TYPE_CODE_PTR,
+  TDESC_TYPE_DATA_PTR,
+  TDESC_TYPE_IEEE_SINGLE,
+  TDESC_TYPE_IEEE_DOUBLE,
+  TDESC_TYPE_ARM_FPA_EXT,
+  TDESC_TYPE_I387_EXT,
+
+  /* Types defined by a target feature.  */
+  TDESC_TYPE_VECTOR,
+  TDESC_TYPE_STRUCT,
+  TDESC_TYPE_UNION,
+  TDESC_TYPE_FLAGS,
+  TDESC_TYPE_ENUM
+};
+
+struct tdesc_type : tdesc_element
+{
+  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
+    : name (name_), kind (kind_)
+  {}
+
+  virtual ~tdesc_type () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_type);
+
+  /* The name of this type.  */
+  std::string name;
+
+  /* Identify the kind of this type.  */
+  enum tdesc_type_kind kind;
+
+  bool operator== (const tdesc_type &other) const
+  {
+    return name == other.name && kind == other.kind;
+  }
+
+  bool operator!= (const tdesc_type &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+
+/* A feature from a target description.  Each feature is a collection
+   of other elements, e.g. registers and types.  */
+
+struct tdesc_feature : tdesc_element
+{
+  tdesc_feature (const std::string &name_)
+    : name (name_)
+  {}
+
+  virtual ~tdesc_feature () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_feature);
+
+  /* The name of this feature.  It may be recognized by the architecture
+     support code.  */
+  std::string name;
+
+  /* The registers associated with this feature.  */
+  std::vector<tdesc_reg_up> registers;
+
+  /* The types associated with this feature.  */
+  std::vector<tdesc_type_up> types;
+
+  void accept (tdesc_element_visitor &v) const override;
+
+  bool operator== (const tdesc_feature &other) const;
+
+  bool operator!= (const tdesc_feature &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
+
 /* Allocate a new target_desc.  */
 target_desc *allocate_target_description (void);
 
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index afe0187aeb..1047949e6d 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -23,8 +23,6 @@
 
 target_desc::~target_desc ()
 {
-  int i;
-
   xfree ((char *) arch);
   xfree ((char *) osabi);
 }
@@ -55,19 +53,21 @@ init_target_desc (struct target_desc *tdesc)
   int offset = 0;
 
   /* Go through all the features and populate reg_defs.  */
-  for (const tdesc_reg_up &treg : tdesc->registers)
-    {
-      int regnum = treg->target_regnum;
+  for (const tdesc_feature_up &feature : tdesc->features)
+    for (const tdesc_reg_up &treg : feature->registers)
+      {
+	int regnum = treg->target_regnum;
 
-      /* Register number will increase (possibly with gaps) or be zero.  */
-      gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());
+	/* Register number will increase (possibly with gaps) or be zero.  */
+	gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());
 
-      if (regnum != 0)
-	tdesc->reg_defs.resize (regnum, reg (offset));
+	if (regnum != 0)
+	  tdesc->reg_defs.resize (regnum, reg (offset));
 
-      tdesc->reg_defs.emplace_back (treg->name.c_str (), offset, treg->bitsize);
-      offset += treg->bitsize;
-    }
+	tdesc->reg_defs.emplace_back (treg->name.c_str (), offset,
+				      treg->bitsize);
+	offset += treg->bitsize;
+      }
 
   tdesc->registers_size = offset / 8;
 
@@ -150,11 +150,10 @@ tdesc_get_features_xml (target_desc *tdesc)
 	  buffer += "</osabi>";
 	}
 
-
-      for (const std::string &xml : tdesc->features)
+      for (const tdesc_feature_up &feature : tdesc->features)
 	{
 	  buffer += "<xi:include href=\"";
-	  buffer += xml;
+	  buffer += feature->name;
 	  buffer += "\"/>";
 	}
 
@@ -167,19 +166,16 @@ tdesc_get_features_xml (target_desc *tdesc)
 }
 #endif
 
-struct tdesc_type
-{};
-
 /* See common/tdesc.h.  */
 
 struct tdesc_feature *
 tdesc_create_feature (struct target_desc *tdesc, const char *name,
 		      const char *xml)
 {
-#ifndef IN_PROCESS_AGENT
-  tdesc->features.emplace_back (xml);
-#endif
-  return tdesc;
+  struct tdesc_feature *new_feature = new tdesc_feature
+    (xml != nullptr ? xml : name);
+  tdesc->features.emplace_back (new_feature);
+  return new_feature;
 }
 
 /* See common/tdesc.h.  */
@@ -224,21 +220,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id)
 
 /* See common/tdesc.h.  */
 
-void
-tdesc_create_reg (struct tdesc_feature *feature, const char *name,
-		  int regnum, int save_restore, const char *group,
-		  int bitsize, const char *type)
-{
-  struct target_desc *tdesc = (struct target_desc *) feature;
-
-  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
-				  group, bitsize, type);
-
-  tdesc->registers.emplace_back (reg);
-}
-
-/* See common/tdesc.h.  */
-
 struct tdesc_type *
 tdesc_create_vector (struct tdesc_feature *feature, const char *name,
 		     struct tdesc_type *field_type, int count)
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 8eb88eedce..197fb59127 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -24,16 +24,10 @@
 #include "regdef.h"
 #include <vector>
 
-struct tdesc_feature
-{
-  /* The registers associated with this feature.  */
-  std::vector<tdesc_reg_up> registers;
-};
-
 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */
 
-struct target_desc : tdesc_feature
+struct target_desc
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
@@ -42,6 +36,9 @@ struct target_desc : tdesc_feature
   /* The register cache size, in bytes.  */
   int registers_size;
 
+  /* XML features in this target description.  */
+  std::vector<tdesc_feature_up> features;
+
 #ifndef IN_PROCESS_AGENT
   /* An array of register names.  These are the "expedite" registers:
      registers whose values are sent along with stop replies.  */
@@ -56,9 +53,6 @@ struct target_desc : tdesc_feature
      fields features, arch, and osabi in tdesc_get_features_xml.  */
   const char *xmltarget = NULL;
 
-  /* XML features in this target description.  */
-  std::vector<std::string> features;
-
   /* The value of <architecture> element in the XML, replying GDB.  */
   const char *arch = NULL;
 
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index ce1627120d..8c6e191596 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -131,7 +131,7 @@ do
     echo "{"
     echo "  static struct target_desc tdesc_${name}_s;"
     echo "  struct target_desc *result = &tdesc_${name}_s;"
-
+    echo "  struct tdesc_feature *feature = tdesc_create_feature (result, \"${name}\");"
     continue
   elif test "${type}" = "xmltarget"; then
     xmltarget="${entry}"
@@ -149,7 +149,7 @@ do
     echo "$0: $1 does not specify \`\`name''." 1>&2
     exit 1
   else
-    echo "  tdesc_create_reg ((struct tdesc_feature *) result, \"${entry}\","
+    echo "  tdesc_create_reg (feature, \"${entry}\","
     echo "  0, 0, NULL, ${type}, NULL);"
 
     offset=`expr ${offset} + ${type}`
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 3186bf886f..a453eddec0 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -67,64 +67,6 @@ struct tdesc_type_field
   int start, end;
 };
 
-enum tdesc_type_kind
-{
-  /* Predefined types.  */
-  TDESC_TYPE_BOOL,
-  TDESC_TYPE_INT8,
-  TDESC_TYPE_INT16,
-  TDESC_TYPE_INT32,
-  TDESC_TYPE_INT64,
-  TDESC_TYPE_INT128,
-  TDESC_TYPE_UINT8,
-  TDESC_TYPE_UINT16,
-  TDESC_TYPE_UINT32,
-  TDESC_TYPE_UINT64,
-  TDESC_TYPE_UINT128,
-  TDESC_TYPE_CODE_PTR,
-  TDESC_TYPE_DATA_PTR,
-  TDESC_TYPE_IEEE_SINGLE,
-  TDESC_TYPE_IEEE_DOUBLE,
-  TDESC_TYPE_ARM_FPA_EXT,
-  TDESC_TYPE_I387_EXT,
-
-  /* Types defined by a target feature.  */
-  TDESC_TYPE_VECTOR,
-  TDESC_TYPE_STRUCT,
-  TDESC_TYPE_UNION,
-  TDESC_TYPE_FLAGS,
-  TDESC_TYPE_ENUM
-};
-
-struct tdesc_type : tdesc_element
-{
-  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
-    : name (name_), kind (kind_)
-  {}
-
-  virtual ~tdesc_type () = default;
-
-  DISABLE_COPY_AND_ASSIGN (tdesc_type);
-
-  /* The name of this type.   */
-  std::string name;
-
-  /* Identify the kind of this type.  */
-  enum tdesc_type_kind kind;
-
-  bool operator== (const tdesc_type &other) const
-  {
-    return name == other.name && kind == other.kind;
-  }
-
-  bool operator!= (const tdesc_type &other) const
-  {
-    return !(*this == other);
-  }
-};
-
-typedef std::unique_ptr<tdesc_type> tdesc_type_up;
-
 struct tdesc_type_builtin : tdesc_type
 {
   tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
@@ -428,82 +370,6 @@ make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype)
   return gdb_type.get_type ();
 }
 
-/* A feature from a target description.  Each feature is a collection
-   of other elements, e.g. registers and types.  */
-
-struct tdesc_feature : tdesc_element
-{
-  tdesc_feature (const std::string &name_)
-    : name (name_)
-  {}
-
-  virtual ~tdesc_feature () = default;
-
-  DISABLE_COPY_AND_ASSIGN (tdesc_feature);
-
-  /* The name of this feature.  It may be recognized by the architecture
-     support code.  */
-  std::string name;
-
-  /* The registers associated with this feature.  */
-  std::vector<tdesc_reg_up> registers;
-
-  /* The types associated with this feature.  */
-  std::vector<tdesc_type_up> types;
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit_pre (this);
-
-    for (const tdesc_type_up &type : types)
-      type->accept (v);
-
-    for (const tdesc_reg_up &reg : registers)
-      reg->accept (v);
-
-    v.visit_post (this);
-  }
-
-  bool operator== (const tdesc_feature &other) const
-  {
-    if (name != other.name)
-      return false;
-
-    if (registers.size () != other.registers.size ())
-      return false;
-
-    for (int ix = 0; ix < registers.size (); ix++)
-      {
-	const tdesc_reg_up &reg1 = registers[ix];
-	const tdesc_reg_up &reg2 = other.registers[ix];
-
-	if (reg1 != reg2 && *reg1 != *reg2)
-	  return false;
-      }
-
-    if (types.size () != other.types.size ())
-      return false;
-
-    for (int ix = 0; ix < types.size (); ix++)
-      {
-	const tdesc_type_up &type1 = types[ix];
-	const tdesc_type_up &type2 = other.types[ix];
-
-	if (type1 != type2 && *type1 != *type2)
-	  return false;
-      }
-
-    return true;
-  }
-
-  bool operator!= (const tdesc_feature &other) const
-  {
-    return !(*this == other);
-  }
-};
-
-typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
-
 /* A target description.  */
 
 struct target_desc : tdesc_element
@@ -1358,20 +1224,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 				      tdesc_remote_register_number);
   set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p);
 }
-\f
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_create_reg (struct tdesc_feature *feature, const char *name,
-		  int regnum, int save_restore, const char *group,
-		  int bitsize, const char *type)
-{
-  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
-				  group, bitsize, type);
-
-  feature->registers.emplace_back (reg);
-}
 
 /* See common/tdesc.h.  */
 
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 6/8] Create xml from target descriptions
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-18  2:43   ` Simon Marchi
  2018-04-10 14:34 ` [PATCH v5 2/8] Commonise tdesc_feature Alan Hayward
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

This patch adds a print_xml_feature visitor class which turns a C
target description into xml. Both gdb and gdbserver can do this.

An accept function is added to gdbserver tdesc to allow it to use
vistor classes.

Tests are added to maintenance_check_xml_descriptions which takes
each pair of tested descriptions, turns them both into xml, then back
again, and confirms the descriptions still match.

Differences from V4 are:

print_xml_feature now uses a ptr.
tdesc xmltarget is now mutable.
Added maintenance_check_tdesc_xml_convert test function.
Improved string_read_description_xml.

This patch requires the next patch in the series, otherwise target
description will be rejected by gdb when using gdbserver. I'll make sure
the two patches get committed in a single commit.

Alan.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* common/tdesc.c (print_xml_feature::visit_pre): Add xml parsing.
	(print_xml_feature::visit_post): Likewise.
	(print_xml_feature::visit): Likewise.
	* common/tdesc.h (tdesc_get_features_xml): Use const tdesc.
	(print_xml_feature): Add new class.
	* regformats/regdat.sh: Null xmltarget on feature targets.
	* target-descriptions.c (struct target_desc): Add xmltarget.
	(maintenance_check_tdesc_xml_convert): Add unittest function.
	(tdesc_get_features_xml): Add function to get xml.
	(maintenance_check_xml_descriptions): Test xml generation.
	* xml-tdesc.c (string_read_description_xml): Add function.
	* xml-tdesc.h (string_read_description_xml): Add declaration.

gdbserver/
	* gdb/gdbserver/server.c (get_features_xml): Remove cast.
	* tdesc.c (void target_desc::accept): Fill in function.
	(tdesc_get_features_xml): Remove old xml creation.
	(print_xml_feature::visit_pre): Add xml vistor.
	* tdesc.h (struct target_desc): Make xmltarget mutable.
	(tdesc_get_features_xml): Remove declaration.
---
 gdb/common/tdesc.c        | 108 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/tdesc.h        |  29 +++++++++++++
 gdb/gdbserver/server.c    |   2 +-
 gdb/gdbserver/tdesc.c     |  45 ++++++++-----------
 gdb/gdbserver/tdesc.h     |  10 ++---
 gdb/regformats/regdat.sh  |   4 +-
 gdb/target-descriptions.c |  56 ++++++++++++++++++++++++
 gdb/xml-tdesc.c           |  11 +++++
 gdb/xml-tdesc.h           |   5 +++
 9 files changed, 234 insertions(+), 36 deletions(-)

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index b9e9ddb3fa..b4bd7bfb00 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -290,3 +290,111 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
 			     tdesc_predefined_type (TDESC_TYPE_INT32),
 			     value, -1);
 }
+
+void print_xml_feature::visit_pre (const tdesc_feature *e)
+{
+  string_appendf (*m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
+}
+
+void print_xml_feature::visit_post (const tdesc_feature *e)
+{
+  string_appendf (*m_buffer, "</feature>\n");
+}
+
+void print_xml_feature::visit (const tdesc_type_builtin *t)
+{
+  error (_("xml output is not supported type \"%s\"."), t->name.c_str ());
+}
+
+void print_xml_feature::visit (const tdesc_type_vector *t)
+{
+  string_appendf (*m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
+		  t->name.c_str (), t->element_type->name.c_str (), t->count);
+}
+
+void print_xml_feature::visit (const tdesc_type_with_fields *t)
+{
+  struct tdesc_type_field *f;
+  const static char *types[] = { "struct", "union", "flags", "enum" };
+
+  gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
+
+  string_appendf (*m_buffer,
+		  "<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
+		  t->name.c_str ());
+
+  switch (t->kind)
+    {
+    case TDESC_TYPE_STRUCT:
+    case TDESC_TYPE_FLAGS:
+      if (t->size > 0)
+	string_appendf (*m_buffer, " size=\"%d\"", t->size);
+      string_appendf (*m_buffer, ">\n");
+
+      for (const tdesc_type_field &f : t->fields)
+	{
+	  string_appendf (*m_buffer, "  <field name=\"%s\" ", f.name.c_str ());
+	  if (f.start == -1)
+	    string_appendf (*m_buffer, "type=\"%s\"/>\n",
+			    f.type->name.c_str ());
+	  else
+	    string_appendf (*m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
+			    f.end);
+	}
+      break;
+
+    case TDESC_TYPE_ENUM:
+      string_appendf (*m_buffer, ">\n");
+      for (const tdesc_type_field &f : t->fields)
+	string_appendf (*m_buffer, "  <field name=\"%s\" start=\"%d\"/>\n",
+			f.name.c_str (), f.start);
+      break;
+
+    case TDESC_TYPE_UNION:
+      string_appendf (*m_buffer, ">\n");
+      for (const tdesc_type_field &f : t->fields)
+	string_appendf (*m_buffer, "  <field name=\"%s\" type=\"%s\"/>\n",
+			f.name.c_str (), f.type->name.c_str ());
+      break;
+
+    default:
+      error (_("xml output is not supported type \"%s\"."), t->name.c_str ());
+    }
+
+  string_appendf (*m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
+}
+
+void print_xml_feature::visit (const tdesc_reg *r)
+{
+  string_appendf (*m_buffer,
+		  "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
+		  r->name.c_str (), r->bitsize, r->type.c_str (),
+		  r->target_regnum);
+
+  if (r->group.length () > 0)
+    string_appendf (*m_buffer, " group=\"%s\"", r->group.c_str ());
+
+  if (r->save_restore == 0)
+    string_appendf (*m_buffer, " save-restore=\"no\"");
+
+  string_appendf (*m_buffer, "/>\n");
+}
+
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+#ifndef IN_PROCESS_AGENT
+  string_appendf (*m_buffer, "<?xml version=\"1.0\"?>\n");
+  string_appendf (*m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
+  string_appendf (*m_buffer, "<target>\n<architecture>%s</architecture>\n",
+		  tdesc_architecture_name (e));
+
+  const char *osabi = tdesc_osabi_name (e);
+  if (osabi != nullptr)
+    string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
+#endif
+}
+
+void print_xml_feature::visit_post (const target_desc *e)
+{
+  string_appendf (*m_buffer, "</target>\n");
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 311341da0d..00eaada072 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -381,4 +381,33 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		       int regnum, int save_restore, const char *group,
 		       int bitsize, const char *type);
 
+/* Return the tdesc in string XML format.  */
+
+const char *tdesc_get_features_xml (const target_desc *tdesc);
+
+/* Print target description as xml.  */
+
+class print_xml_feature : public tdesc_element_visitor
+{
+public:
+  print_xml_feature (std::string *buffer_)
+    : m_buffer (buffer_)
+  {}
+
+  ~print_xml_feature ()
+  {}
+
+  void visit_pre (const target_desc *e) override;
+  void visit_post (const target_desc *e) override;
+  void visit_pre (const tdesc_feature *e) override;
+  void visit_post (const tdesc_feature *e) override;
+  void visit (const tdesc_type_builtin *type) override;
+  void visit (const tdesc_type_vector *type) override;
+  void visit (const tdesc_type_with_fields *type) override;
+  void visit (const tdesc_reg *reg) override;
+
+private:
+  std::string *m_buffer;
+};
+
 #endif /* ARCH_TDESC_H */
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 64c72bdd58..5027df5e10 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -940,7 +940,7 @@ get_features_xml (const char *annex)
 
   if (strcmp (annex, "target.xml") == 0)
     {
-      const char *ret = tdesc_get_features_xml ((target_desc*) desc);
+      const char *ret = tdesc_get_features_xml (desc);
 
       if (*ret == '@')
 	return ret + 1;
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 7603a90a59..126589f3e3 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -47,6 +47,18 @@ bool target_desc::operator== (const target_desc &other) const
 
 #endif
 
+void target_desc::accept (tdesc_element_visitor &v) const
+{
+#ifndef IN_PROCESS_AGENT
+  v.visit_pre (this);
+
+  for (const tdesc_feature_up &feature : features)
+    feature->accept (v);
+
+  v.visit_post (this);
+#endif
+}
+
 void
 init_target_desc (struct target_desc *tdesc)
 {
@@ -138,11 +150,10 @@ set_tdesc_osabi (struct target_desc *target_desc, const char *name)
   target_desc->osabi = xstrdup (name);
 }
 
-/* Return a string which is of XML format, including XML target
-   description to be sent to GDB.  */
+/* See common/tdesc.h.  */
 
 const char *
-tdesc_get_features_xml (target_desc *tdesc)
+tdesc_get_features_xml (const target_desc *tdesc)
 {
   /* Either .xmltarget or .features is not NULL.  */
   gdb_assert (tdesc->xmltarget != NULL
@@ -151,31 +162,9 @@ tdesc_get_features_xml (target_desc *tdesc)
 
   if (tdesc->xmltarget == NULL)
     {
-      std::string buffer ("@<?xml version=\"1.0\"?>");
-
-      buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
-      buffer += "<target>";
-      buffer += "<architecture>";
-      buffer += tdesc_architecture_name (tdesc);
-      buffer += "</architecture>";
-
-      const char *osabi = tdesc_osabi_name (tdesc);
-      if (osabi != nullptr)
-	{
-	  buffer += "<osabi>";
-	  buffer += osabi;
-	  buffer += "</osabi>";
-	}
-
-      for (const tdesc_feature_up &feature : tdesc->features)
-	{
-	  buffer += "<xi:include href=\"";
-	  buffer += feature->name;
-	  buffer += "\"/>";
-	}
-
-      buffer += "</target>";
-
+      std::string buffer ("@");
+      print_xml_feature v (&buffer);
+      tdesc->accept (v);
       tdesc->xmltarget = xstrdup (buffer.c_str ());
     }
 
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 197fb59127..2152b17947 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -27,7 +27,7 @@
 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */
 
-struct target_desc
+struct target_desc : tdesc_element
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
@@ -51,7 +51,7 @@ struct target_desc
 
      It can be NULL, then, its content is got from the following three
      fields features, arch, and osabi in tdesc_get_features_xml.  */
-  const char *xmltarget = NULL;
+  mutable const char *xmltarget = NULL;
 
   /* The value of <architecture> element in the XML, replying GDB.  */
   const char *arch = NULL;
@@ -73,6 +73,8 @@ public:
     return !(*this == other);
   }
 #endif
+
+  void accept (tdesc_element_visitor &v) const;
 };
 
 /* Copy target description SRC to DEST.  */
@@ -89,8 +91,4 @@ void init_target_desc (struct target_desc *tdesc);
 
 const struct target_desc *current_target_desc (void);
 
-#ifndef IN_PROCESS_AGENT
-const char *tdesc_get_features_xml (struct target_desc *tdesc);
-#endif
-
 #endif /* TDESC_H */
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 18108d77eb..8f546fe276 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -163,7 +163,9 @@ done
 
 echo
 echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
-if test "${xmltarget}" = x; then
+if test "${feature}" != x; then
+  echo "static const char *xmltarget_${name} = 0;"
+elif test "${xmltarget}" = x; then
   if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
     echo "static const char *xmltarget_${name} = 0;"
   else
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index da2c1ce345..b96ef961a4 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -333,6 +333,8 @@ struct target_desc : tdesc_element
   /* The features associated with this target.  */
   std::vector<tdesc_feature_up> features;
 
+  mutable char *xmltarget = nullptr;
+
   void accept (tdesc_element_visitor &v) const override
   {
     v.visit_pre (this);
@@ -1667,6 +1669,21 @@ private:
   int m_next_regnum = 0;
 };
 
+/* See common/tdesc.h.  */
+
+const char *
+tdesc_get_features_xml (const target_desc *tdesc)
+{
+  if (tdesc->xmltarget == nullptr)
+    {
+      std::string buffer ("@");
+      print_xml_feature v (&buffer);
+      tdesc->accept (v);
+      tdesc->xmltarget = xstrdup (buffer.c_str ());
+    }
+  return tdesc->xmltarget;
+}
+
 static void
 maint_print_c_tdesc_cmd (const char *args, int from_tty)
 {
@@ -1739,6 +1756,39 @@ record_xml_tdesc (const char *xml_file, const struct target_desc *tdesc)
 
 }
 
+/* Test the convesion process of a target description to/from xml: Take a target
+   description TDESC, convert to xml, back to a description, and confirm the new
+   tdesc is identical to the original.  */
+static bool
+maintenance_check_tdesc_xml_convert (const target_desc *tdesc, const char *name)
+{
+  const char *xml = tdesc_get_features_xml (tdesc);
+
+  if (xml == nullptr && *xml != '@')
+    {
+      printf_filtered (_("Could not convert description for %s to xml.\n"),
+		       name);
+      return false;
+    }
+
+  const target_desc *tdesc_trans = string_read_description_xml (xml + 1);
+
+  if (tdesc_trans == nullptr)
+    {
+      printf_filtered (_("Could not convert description for %s from xml.\n"),
+		       name);
+      return false;
+    }
+  else if (*tdesc != *tdesc_trans)
+    {
+      printf_filtered (_("Converted description for %s does not match.\n"),
+		       name);
+      return false;
+    }
+  return true;
+}
+
+
 /* Check that the target descriptions created dynamically by
    architecture-specific code equal the descriptions created from XML files
    found in the specified directory DIR.  */
@@ -1760,6 +1810,12 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
 	= file_read_description_xml (tdesc_xml.data ());
 
       if (tdesc == NULL || *tdesc != *e.second)
+	{
+	  printf_filtered ( _("Descriptions for %s do not match.\n"), e.first);
+	  failed++;
+	}
+      else if (!maintenance_check_tdesc_xml_convert (tdesc, e.first)
+	       || !maintenance_check_tdesc_xml_convert (e.second, e.first))
 	failed++;
     }
   printf_filtered (_("Tested %lu XML files, %d failed\n"),
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 9190d5f3c6..5c6ee4c8cd 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -752,3 +752,14 @@ target_fetch_description_xml (struct target_ops *ops)
   return output;
 #endif
 }
+
+/* See xml-tdesc.h.  */
+
+const struct target_desc *
+string_read_description_xml (const char *xml_str)
+{
+  return tdesc_parse_xml (xml_str, [] (const char *href, void *baton) {
+     error (_("xincludes are unsupported with this method"));
+     return gdb::unique_xmalloc_ptr<char> ();
+     }, nullptr);
+}
\ No newline at end of file
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index 8f0679707a..e39d5580bb 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
    otherwise.  */
 gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
 
+/* Take an xml string, parse it, and return the parsed description.  Does not
+   handle a string containing includes.  */
+
+const struct target_desc *string_read_description_xml (const char *);
+
 #endif /* XML_TDESC_H */
 
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 5/8] Add feature reference in .dat files
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (3 preceding siblings ...)
  2018-04-10 14:34 ` [PATCH v5 7/8] Remove xml file references from target descriptions Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 3/8] Commonise tdesc types Alan Hayward
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

For all targets which use the newer style target descriptions, add a
"feature" marker in the dat files.
Update regdat.sh to parse feature, but do not use it (yet).

In the xml printer patch we want to ensure that only targets which
use the newer style descriptions dynamically generate xml. Other targets
should continue to return the name of the xml file.
The "feature" marker will enable this.

Identical to V4 version.

Alan.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

	* features/Makefile: Add feature marker to targets with new style
	target descriptions.
	* regformats/aarch64.dat: Regenerate.
	* regformats/i386/amd64-avx-avx512-linux.dat: Likewise.
	* regformats/i386/amd64-avx-linux.dat: Likewise.
	* regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat: Likewise.
	* regformats/i386/amd64-avx-mpx-linux.dat: Likewise.
	* regformats/i386/amd64-linux.dat: Likewise.
	* regformats/i386/amd64-mpx-linux.dat: Likewise.
	* regformats/i386/amd64.dat: Likewise.
	* regformats/i386/i386-avx-avx512-linux.dat: Likewise.
	* regformats/i386/i386-avx-linux.dat: Likewise.
	* regformats/i386/i386-avx-mpx-avx512-pku-linux.dat: Likewise.
	* regformats/i386/i386-avx-mpx-linux.dat: Likewise.
	* regformats/i386/i386-linux.dat: Likewise.
	* regformats/i386/i386-mmx-linux.dat: Likewise.
	* regformats/i386/i386-mpx-linux.dat: Likewise.
	* regformats/i386/i386.dat: Likewise.
	* regformats/i386/x32-avx-avx512-linux.dat: Likewise.
	* regformats/i386/x32-avx-linux.dat: Likewise.
	* regformats/i386/x32-linux.dat: Likewise.
	* regformats/tic6x-c62x-linux.dat: Likewise.
	* regformats/tic6x-c64x-linux.dat: Likewise.
	* regformats/tic6x-c64xp-linux.dat: Likewise.
	* regformats/regdat.sh: Parse feature marker.
---
 gdb/features/Makefile                                  | 6 ++++++
 gdb/regformats/aarch64.dat                             | 1 +
 gdb/regformats/i386/amd64-avx-avx512-linux.dat         | 1 +
 gdb/regformats/i386/amd64-avx-linux.dat                | 1 +
 gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat | 1 +
 gdb/regformats/i386/amd64-avx-mpx-linux.dat            | 1 +
 gdb/regformats/i386/amd64-linux.dat                    | 1 +
 gdb/regformats/i386/amd64-mpx-linux.dat                | 1 +
 gdb/regformats/i386/amd64.dat                          | 1 +
 gdb/regformats/i386/i386-avx-avx512-linux.dat          | 1 +
 gdb/regformats/i386/i386-avx-linux.dat                 | 1 +
 gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat  | 1 +
 gdb/regformats/i386/i386-avx-mpx-linux.dat             | 1 +
 gdb/regformats/i386/i386-linux.dat                     | 1 +
 gdb/regformats/i386/i386-mmx-linux.dat                 | 1 +
 gdb/regformats/i386/i386-mpx-linux.dat                 | 1 +
 gdb/regformats/i386/i386.dat                           | 1 +
 gdb/regformats/i386/x32-avx-avx512-linux.dat           | 1 +
 gdb/regformats/i386/x32-avx-linux.dat                  | 1 +
 gdb/regformats/i386/x32-linux.dat                      | 1 +
 gdb/regformats/regdat.sh                               | 4 ++++
 gdb/regformats/tic6x-c62x-linux.dat                    | 1 +
 gdb/regformats/tic6x-c64x-linux.dat                    | 1 +
 gdb/regformats/tic6x-c64xp-linux.dat                   | 1 +
 24 files changed, 32 insertions(+)

diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 82609f5862..168c46e003 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -193,12 +193,18 @@ XMLTOC = \
 TDESC_CFILES = $(patsubst %.xml,%.c,$(XMLTOC))
 GDB = false
 
+#Targets which use feature based target descriptions.
+aarch64-feature = 1
+i386-feature = 1
+tic6x-feature = 1
+
 all: $(OUTPUTS)
 
 $(outdir)/%.dat: %.xml number-regs.xsl sort-regs.xsl gdbserver-regs.xsl
 	echo "# THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:" > $(outdir)/$*.tmp
 	echo "# Generated from: $<" >> $(outdir)/$*.tmp
 	echo "name:`echo $(notdir $*) | sed 's/-/_/g'`" >> $(outdir)/$*.tmp
+	$(if $($(firstword $(subst /, ,$(subst -, ,$*)))-feature), echo "feature:1") >> $(outdir)/$*.tmp
 	echo "xmltarget:$(<F)" >> $(outdir)/$*.tmp
 	echo "expedite:$(if $($*-expedite),$($*-expedite),$($(firstword $(subst -, ,$(notdir $*)))-expedite))" \
 	  >> $(outdir)/$*.tmp
diff --git a/gdb/regformats/aarch64.dat b/gdb/regformats/aarch64.dat
index d4cea04358..75ba89a92e 100644
--- a/gdb/regformats/aarch64.dat
+++ b/gdb/regformats/aarch64.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: aarch64.xml
 name:aarch64
+feature:1
 xmltarget:aarch64.xml
 expedite:x29,sp,pc
 64:x0
diff --git a/gdb/regformats/i386/amd64-avx-avx512-linux.dat b/gdb/regformats/i386/amd64-avx-avx512-linux.dat
index 0743693886..6cb97964b3 100644
--- a/gdb/regformats/i386/amd64-avx-avx512-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-avx512-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64-avx-avx512-linux.xml
 name:amd64_avx_avx512_linux
+feature:1
 xmltarget:amd64-avx-avx512-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/amd64-avx-linux.dat b/gdb/regformats/i386/amd64-avx-linux.dat
index 7780b3b6a2..54dad1d0a7 100644
--- a/gdb/regformats/i386/amd64-avx-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64-avx-linux.xml
 name:amd64_avx_linux
+feature:1
 xmltarget:amd64-avx-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat b/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
index 9cd0fae820..8f326389f1 100644
--- a/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64-avx-mpx-avx512-pku-linux.xml
 name:amd64_avx_mpx_avx512_pku_linux
+feature:1
 xmltarget:amd64-avx-mpx-avx512-pku-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/amd64-avx-mpx-linux.dat b/gdb/regformats/i386/amd64-avx-mpx-linux.dat
index 7c2f928070..92ffa0694f 100644
--- a/gdb/regformats/i386/amd64-avx-mpx-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-mpx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64-avx-mpx-linux.xml
 name:amd64_avx_mpx_linux
+feature:1
 xmltarget:amd64-avx-mpx-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/amd64-linux.dat b/gdb/regformats/i386/amd64-linux.dat
index cd16a15442..81e6aef361 100644
--- a/gdb/regformats/i386/amd64-linux.dat
+++ b/gdb/regformats/i386/amd64-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64-linux.xml
 name:amd64_linux
+feature:1
 xmltarget:amd64-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/amd64-mpx-linux.dat b/gdb/regformats/i386/amd64-mpx-linux.dat
index 10487f6987..c90b1b5b8a 100644
--- a/gdb/regformats/i386/amd64-mpx-linux.dat
+++ b/gdb/regformats/i386/amd64-mpx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64-mpx-linux.xml
 name:amd64_mpx_linux
+feature:1
 xmltarget:amd64-mpx-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/amd64.dat b/gdb/regformats/i386/amd64.dat
index 66f26ad094..fde8c1dbe2 100644
--- a/gdb/regformats/i386/amd64.dat
+++ b/gdb/regformats/i386/amd64.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/amd64.xml
 name:amd64
+feature:1
 xmltarget:amd64.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/i386-avx-avx512-linux.dat b/gdb/regformats/i386/i386-avx-avx512-linux.dat
index 4477133997..57530978df 100644
--- a/gdb/regformats/i386/i386-avx-avx512-linux.dat
+++ b/gdb/regformats/i386/i386-avx-avx512-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-avx-avx512-linux.xml
 name:i386_avx_avx512_linux
+feature:1
 xmltarget:i386-avx-avx512-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386-avx-linux.dat b/gdb/regformats/i386/i386-avx-linux.dat
index 1c3fcfd763..6a81302766 100644
--- a/gdb/regformats/i386/i386-avx-linux.dat
+++ b/gdb/regformats/i386/i386-avx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-avx-linux.xml
 name:i386_avx_linux
+feature:1
 xmltarget:i386-avx-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat b/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
index 515ee10525..ef6216be92 100644
--- a/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
+++ b/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-avx-mpx-avx512-pku-linux.xml
 name:i386_avx_mpx_avx512_pku_linux
+feature:1
 xmltarget:i386-avx-mpx-avx512-pku-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386-avx-mpx-linux.dat b/gdb/regformats/i386/i386-avx-mpx-linux.dat
index 831c476fef..f15bd1a89d 100644
--- a/gdb/regformats/i386/i386-avx-mpx-linux.dat
+++ b/gdb/regformats/i386/i386-avx-mpx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-avx-mpx-linux.xml
 name:i386_avx_mpx_linux
+feature:1
 xmltarget:i386-avx-mpx-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386-linux.dat b/gdb/regformats/i386/i386-linux.dat
index 0e414e6490..5203cd4ef0 100644
--- a/gdb/regformats/i386/i386-linux.dat
+++ b/gdb/regformats/i386/i386-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-linux.xml
 name:i386_linux
+feature:1
 xmltarget:i386-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386-mmx-linux.dat b/gdb/regformats/i386/i386-mmx-linux.dat
index aa2a564ac7..736874273e 100644
--- a/gdb/regformats/i386/i386-mmx-linux.dat
+++ b/gdb/regformats/i386/i386-mmx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-mmx-linux.xml
 name:i386_mmx_linux
+feature:1
 xmltarget:i386-mmx-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386-mpx-linux.dat b/gdb/regformats/i386/i386-mpx-linux.dat
index 1dcdce98cf..856ef4b1c9 100644
--- a/gdb/regformats/i386/i386-mpx-linux.dat
+++ b/gdb/regformats/i386/i386-mpx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386-mpx-linux.xml
 name:i386_mpx_linux
+feature:1
 xmltarget:i386-mpx-linux.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/i386.dat b/gdb/regformats/i386/i386.dat
index 13abb485e4..e707ed2790 100644
--- a/gdb/regformats/i386/i386.dat
+++ b/gdb/regformats/i386/i386.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/i386.xml
 name:i386
+feature:1
 xmltarget:i386.xml
 expedite:ebp,esp,eip
 32:eax
diff --git a/gdb/regformats/i386/x32-avx-avx512-linux.dat b/gdb/regformats/i386/x32-avx-avx512-linux.dat
index 00786172fb..a9bd1d4bb3 100644
--- a/gdb/regformats/i386/x32-avx-avx512-linux.dat
+++ b/gdb/regformats/i386/x32-avx-avx512-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/x32-avx-avx512-linux.xml
 name:x32_avx_avx512_linux
+feature:1
 xmltarget:x32-avx-avx512-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/x32-avx-linux.dat b/gdb/regformats/i386/x32-avx-linux.dat
index eb0e395366..9c43fa48f5 100644
--- a/gdb/regformats/i386/x32-avx-linux.dat
+++ b/gdb/regformats/i386/x32-avx-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/x32-avx-linux.xml
 name:x32_avx_linux
+feature:1
 xmltarget:x32-avx-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/i386/x32-linux.dat b/gdb/regformats/i386/x32-linux.dat
index eee378fd84..fda1a89de0 100644
--- a/gdb/regformats/i386/x32-linux.dat
+++ b/gdb/regformats/i386/x32-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: i386/x32-linux.xml
 name:x32_linux
+feature:1
 xmltarget:x32-linux.xml
 expedite:rbp,rsp,rip
 64:rax
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 8c6e191596..18108d77eb 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -118,6 +118,7 @@ xmltarget=x
 xmlarch=x
 xmlosabi=x
 expedite=x
+feature=x
 exec < $1
 while do_read
 do
@@ -145,6 +146,9 @@ do
   elif test "${type}" = "expedite"; then
     expedite="${entry}"
     continue
+  elif test "${type}" = "feature"; then
+    feature="${entry}"
+    continue
   elif test "${name}" = x; then
     echo "$0: $1 does not specify \`\`name''." 1>&2
     exit 1
diff --git a/gdb/regformats/tic6x-c62x-linux.dat b/gdb/regformats/tic6x-c62x-linux.dat
index 82f2a0a0bf..98ca39e110 100644
--- a/gdb/regformats/tic6x-c62x-linux.dat
+++ b/gdb/regformats/tic6x-c62x-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: tic6x-c62x-linux.xml
 name:tic6x_c62x_linux
+feature:1
 xmltarget:tic6x-c62x-linux.xml
 expedite:A15,PC
 32:A0
diff --git a/gdb/regformats/tic6x-c64x-linux.dat b/gdb/regformats/tic6x-c64x-linux.dat
index 542826ad1d..91e84baebc 100644
--- a/gdb/regformats/tic6x-c64x-linux.dat
+++ b/gdb/regformats/tic6x-c64x-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: tic6x-c64x-linux.xml
 name:tic6x_c64x_linux
+feature:1
 xmltarget:tic6x-c64x-linux.xml
 expedite:A15,PC
 32:A0
diff --git a/gdb/regformats/tic6x-c64xp-linux.dat b/gdb/regformats/tic6x-c64xp-linux.dat
index 229b3c26c2..a3fa3b7cd4 100644
--- a/gdb/regformats/tic6x-c64xp-linux.dat
+++ b/gdb/regformats/tic6x-c64xp-linux.dat
@@ -1,6 +1,7 @@
 # THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi :set ro:
 # Generated from: tic6x-c64xp-linux.xml
 name:tic6x_c64xp_linux
+feature:1
 xmltarget:tic6x-c64xp-linux.xml
 expedite:A15,PC
 32:A0
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 7/8] Remove xml file references from target descriptions.
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (2 preceding siblings ...)
  2018-04-10 14:34 ` [PATCH v5 4/8] Add tdesc osabi and architecture functions Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 5/8] Add feature reference in .dat files Alan Hayward
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

We no longer need to know the name of the xml file for targets using the
new target descriptions. This patch removes the references and regenerates
the C files.

This patch is identical to the V3/4 versions.

Alan.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* common/tdesc.h (tdesc_create_feature): Remove xml filename parameter.
	* features/aarch64-core.c (create_feature_aarch64_core): Regenerate.
	* features/aarch64-fpu.c (create_feature_aarch64_fpu): Likewise.
	* features/i386/32bit-avx.c (create_feature_i386_32bit_avx): Likewise.
	* features/i386/32bit-avx512.c (create_feature_i386_32bit_avx512):
	Likewise.
	* features/i386/32bit-core.c (create_feature_i386_32bit_core):
	Likewise.
	* features/i386/32bit-linux.c (create_feature_i386_32bit_linux):
	Likewise.
	* features/i386/32bit-mpx.c (create_feature_i386_32bit_mpx): Likewise.
	* features/i386/32bit-pkeys.c (create_feature_i386_32bit_pkeys):
	Likewise.
	* features/i386/32bit-sse.c (create_feature_i386_32bit_sse): Likewise.
	* features/i386/64bit-avx.c (create_feature_i386_64bit_avx): Likewise.
	* features/i386/64bit-avx512.c (create_feature_i386_64bit_avx512):
	Likewise.
	* features/i386/64bit-core.c (create_feature_i386_64bit_core):
	Likewise.
	* features/i386/64bit-linux.c (create_feature_i386_64bit_linux):
	Likewise.
	* features/i386/64bit-mpx.c (create_feature_i386_64bit_mpx): Likewise.
	* features/i386/64bit-pkeys.c (create_feature_i386_64bit_pkeys):
	Likewise.
	* features/i386/64bit-segments.c (create_feature_i386_64bit_segments):
	Likewise.
	* features/i386/64bit-sse.c (create_feature_i386_64bit_sse): Likewise.
	* features/i386/x32-core.c (create_feature_i386_x32_core): Likewise.
	* features/tic6x-c6xp.c (create_feature_tic6x_c6xp): Likewise.
	* features/tic6x-core.c (create_feature_tic6x_core): Likewise.
	* features/tic6x-gp.c (create_feature_tic6x_gp): Likewise.
	* target-descriptions.c: In generated code, don't pass xml filename.

gdbserver/
	* gdbserver/tdesc.c: Remove xml parameter.
---
 gdb/common/tdesc.h                 | 4 +---
 gdb/features/aarch64-core.c        | 2 +-
 gdb/features/aarch64-fpu.c         | 2 +-
 gdb/features/i386/32bit-avx.c      | 2 +-
 gdb/features/i386/32bit-avx512.c   | 2 +-
 gdb/features/i386/32bit-core.c     | 2 +-
 gdb/features/i386/32bit-linux.c    | 2 +-
 gdb/features/i386/32bit-mpx.c      | 2 +-
 gdb/features/i386/32bit-pkeys.c    | 2 +-
 gdb/features/i386/32bit-sse.c      | 2 +-
 gdb/features/i386/64bit-avx.c      | 2 +-
 gdb/features/i386/64bit-avx512.c   | 2 +-
 gdb/features/i386/64bit-core.c     | 2 +-
 gdb/features/i386/64bit-linux.c    | 2 +-
 gdb/features/i386/64bit-mpx.c      | 2 +-
 gdb/features/i386/64bit-pkeys.c    | 2 +-
 gdb/features/i386/64bit-segments.c | 2 +-
 gdb/features/i386/64bit-sse.c      | 2 +-
 gdb/features/i386/x32-core.c       | 2 +-
 gdb/features/tic6x-c6xp.c          | 2 +-
 gdb/features/tic6x-core.c          | 2 +-
 gdb/features/tic6x-gp.c            | 2 +-
 gdb/gdbserver/tdesc.c              | 6 ++----
 gdb/target-descriptions.c          | 7 +++----
 24 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 00eaada072..2c173074de 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -316,9 +316,7 @@ struct tdesc_type *tdesc_named_type (const struct tdesc_feature *feature,
 
 /* Return the created feature named NAME in target description TDESC.  */
 struct tdesc_feature *tdesc_create_feature (struct target_desc *tdesc,
-					    const char *name,
-					    const char *xml = nullptr);
-
+					    const char *name);
 
 /* Return the created vector tdesc_type named NAME in FEATURE.  */
 struct tdesc_type *tdesc_create_vector (struct tdesc_feature *feature,
diff --git a/gdb/features/aarch64-core.c b/gdb/features/aarch64-core.c
index db10c4aa06..cd3de02cbc 100644
--- a/gdb/features/aarch64-core.c
+++ b/gdb/features/aarch64-core.c
@@ -8,7 +8,7 @@ create_feature_aarch64_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core", "aarch64-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "cpsr_flags", 4);
   tdesc_add_flag (type_with_fields, 0, "SP");
diff --git a/gdb/features/aarch64-fpu.c b/gdb/features/aarch64-fpu.c
index 0c40c4cef2..47962d1f24 100644
--- a/gdb/features/aarch64-fpu.c
+++ b/gdb/features/aarch64-fpu.c
@@ -8,7 +8,7 @@ create_feature_aarch64_fpu (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu", "aarch64-fpu.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_double");
   tdesc_create_vector (feature, "v2d", element_type, 2);
diff --git a/gdb/features/i386/32bit-avx.c b/gdb/features/i386/32bit-avx.c
index 6e2cfdbdcb..b9feac377c 100644
--- a/gdb/features/i386/32bit-avx.c
+++ b/gdb/features/i386/32bit-avx.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_avx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx", "32bit-avx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
   tdesc_create_reg (feature, "ymm0h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm1h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm2h", regnum++, 1, NULL, 128, "uint128");
diff --git a/gdb/features/i386/32bit-avx512.c b/gdb/features/i386/32bit-avx512.c
index 863c119ac4..51a403259d 100644
--- a/gdb/features/i386/32bit-avx512.c
+++ b/gdb/features/i386/32bit-avx512.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_avx512 (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512", "32bit-avx512.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "uint128");
   tdesc_create_vector (feature, "v2ui128", element_type, 2);
diff --git a/gdb/features/i386/32bit-core.c b/gdb/features/i386/32bit-core.c
index f6de737074..098c2ca31c 100644
--- a/gdb/features/i386/32bit-core.c
+++ b/gdb/features/i386/32bit-core.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "32bit-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type_with_fields, 0, "CF");
diff --git a/gdb/features/i386/32bit-linux.c b/gdb/features/i386/32bit-linux.c
index af476da194..3bc593739c 100644
--- a/gdb/features/i386/32bit-linux.c
+++ b/gdb/features/i386/32bit-linux.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_linux (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux", "32bit-linux.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
   regnum = 41;
   tdesc_create_reg (feature, "orig_eax", regnum++, 1, NULL, 32, "int");
   return regnum;
diff --git a/gdb/features/i386/32bit-mpx.c b/gdb/features/i386/32bit-mpx.c
index 84101746b5..8288e9d758 100644
--- a/gdb/features/i386/32bit-mpx.c
+++ b/gdb/features/i386/32bit-mpx.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_mpx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx", "32bit-mpx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_struct (feature, "br128");
   tdesc_type *field_type;
diff --git a/gdb/features/i386/32bit-pkeys.c b/gdb/features/i386/32bit-pkeys.c
index 272751f578..a2a72f40a3 100644
--- a/gdb/features/i386/32bit-pkeys.c
+++ b/gdb/features/i386/32bit-pkeys.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_pkeys (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys", "32bit-pkeys.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys");
   tdesc_create_reg (feature, "pkru", regnum++, 1, NULL, 32, "uint32");
   return regnum;
 }
diff --git a/gdb/features/i386/32bit-sse.c b/gdb/features/i386/32bit-sse.c
index 78fc02b863..68fbab06a4 100644
--- a/gdb/features/i386/32bit-sse.c
+++ b/gdb/features/i386/32bit-sse.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_sse (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse", "32bit-sse.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", element_type, 4);
diff --git a/gdb/features/i386/64bit-avx.c b/gdb/features/i386/64bit-avx.c
index 5f11035af3..7e45c980db 100644
--- a/gdb/features/i386/64bit-avx.c
+++ b/gdb/features/i386/64bit-avx.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_avx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx", "64bit-avx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
   tdesc_create_reg (feature, "ymm0h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm1h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm2h", regnum++, 1, NULL, 128, "uint128");
diff --git a/gdb/features/i386/64bit-avx512.c b/gdb/features/i386/64bit-avx512.c
index 733910025b..acc07a3628 100644
--- a/gdb/features/i386/64bit-avx512.c
+++ b/gdb/features/i386/64bit-avx512.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_avx512 (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512", "64bit-avx512.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", element_type, 4);
diff --git a/gdb/features/i386/64bit-core.c b/gdb/features/i386/64bit-core.c
index efc7016380..617425da21 100644
--- a/gdb/features/i386/64bit-core.c
+++ b/gdb/features/i386/64bit-core.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "64bit-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type_with_fields, 0, "CF");
diff --git a/gdb/features/i386/64bit-linux.c b/gdb/features/i386/64bit-linux.c
index c9c56f193c..68de9d27e8 100644
--- a/gdb/features/i386/64bit-linux.c
+++ b/gdb/features/i386/64bit-linux.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_linux (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux", "64bit-linux.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
   regnum = 57;
   tdesc_create_reg (feature, "orig_rax", regnum++, 1, NULL, 64, "int");
   return regnum;
diff --git a/gdb/features/i386/64bit-mpx.c b/gdb/features/i386/64bit-mpx.c
index d923813d69..ce4d611d9c 100644
--- a/gdb/features/i386/64bit-mpx.c
+++ b/gdb/features/i386/64bit-mpx.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_mpx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx", "64bit-mpx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_struct (feature, "br128");
   tdesc_type *field_type;
diff --git a/gdb/features/i386/64bit-pkeys.c b/gdb/features/i386/64bit-pkeys.c
index f1cbce0ce7..70b88cf74d 100644
--- a/gdb/features/i386/64bit-pkeys.c
+++ b/gdb/features/i386/64bit-pkeys.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_pkeys (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys", "64bit-pkeys.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys");
   tdesc_create_reg (feature, "pkru", regnum++, 1, NULL, 32, "uint32");
   return regnum;
 }
diff --git a/gdb/features/i386/64bit-segments.c b/gdb/features/i386/64bit-segments.c
index 900471fc48..b06d40fbda 100644
--- a/gdb/features/i386/64bit-segments.c
+++ b/gdb/features/i386/64bit-segments.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_segments (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.segments", "64bit-segments.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.segments");
   tdesc_create_reg (feature, "fs_base", regnum++, 1, NULL, 64, "int");
   tdesc_create_reg (feature, "gs_base", regnum++, 1, NULL, 64, "int");
   return regnum;
diff --git a/gdb/features/i386/64bit-sse.c b/gdb/features/i386/64bit-sse.c
index 294360197f..e083093741 100644
--- a/gdb/features/i386/64bit-sse.c
+++ b/gdb/features/i386/64bit-sse.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_sse (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse", "64bit-sse.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", element_type, 4);
diff --git a/gdb/features/i386/x32-core.c b/gdb/features/i386/x32-core.c
index 9b0bcc5a7e..dd6e1a2ed6 100644
--- a/gdb/features/i386/x32-core.c
+++ b/gdb/features/i386/x32-core.c
@@ -8,7 +8,7 @@ create_feature_i386_x32_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "x32-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type_with_fields, 0, "CF");
diff --git a/gdb/features/tic6x-c6xp.c b/gdb/features/tic6x-c6xp.c
index 261a562d90..bfd69d7fd5 100644
--- a/gdb/features/tic6x-c6xp.c
+++ b/gdb/features/tic6x-c6xp.c
@@ -8,7 +8,7 @@ create_feature_tic6x_c6xp (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.c6xp", "tic6x-c6xp.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.c6xp");
   tdesc_create_reg (feature, "TSR", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "ILC", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "RILC", regnum++, 1, NULL, 32, "uint32");
diff --git a/gdb/features/tic6x-core.c b/gdb/features/tic6x-core.c
index 65f73ec97d..0415209275 100644
--- a/gdb/features/tic6x-core.c
+++ b/gdb/features/tic6x-core.c
@@ -8,7 +8,7 @@ create_feature_tic6x_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.core", "tic6x-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.core");
   tdesc_create_reg (feature, "A0", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A1", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A2", regnum++, 1, NULL, 32, "uint32");
diff --git a/gdb/features/tic6x-gp.c b/gdb/features/tic6x-gp.c
index b624588379..4a0734c04a 100644
--- a/gdb/features/tic6x-gp.c
+++ b/gdb/features/tic6x-gp.c
@@ -8,7 +8,7 @@ create_feature_tic6x_gp (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;
 
-  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.gp", "tic6x-gp.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.gp");
   tdesc_create_reg (feature, "A16", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A17", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A18", regnum++, 1, NULL, 32, "uint32");
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 126589f3e3..92524b3dec 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -175,11 +175,9 @@ tdesc_get_features_xml (const target_desc *tdesc)
 /* See common/tdesc.h.  */
 
 struct tdesc_feature *
-tdesc_create_feature (struct target_desc *tdesc, const char *name,
-		      const char *xml)
+tdesc_create_feature (struct target_desc *tdesc, const char *name)
 {
-  struct tdesc_feature *new_feature = new tdesc_feature
-    (xml != nullptr ? xml : name);
+  struct tdesc_feature *new_feature = new tdesc_feature (name);
   tdesc->features.emplace_back (new_feature);
   return new_feature;
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index b96ef961a4..0c5e51ee5c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1130,8 +1130,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 /* See common/tdesc.h.  */
 
 struct tdesc_feature *
-tdesc_create_feature (struct target_desc *tdesc, const char *name,
-		      const char *xml)
+tdesc_create_feature (struct target_desc *tdesc, const char *name)
 {
   struct tdesc_feature *new_feature = new tdesc_feature (name);
 
@@ -1599,8 +1598,8 @@ public:
     printf_unfiltered ("  struct tdesc_feature *feature;\n");
 
     printf_unfiltered
-      ("\n  feature = tdesc_create_feature (result, \"%s\", \"%s\");\n",
-       e->name.c_str (), lbasename (m_filename_after_features.c_str ()));
+      ("\n  feature = tdesc_create_feature (result, \"%s\");\n",
+       e->name.c_str ());
   }
 
   void visit_post (const tdesc_feature *e) override
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 0/8] Remove gdbserver dependency on xml files
@ 2018-04-10 14:34 Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 6/8] Create xml from target descriptions Alan Hayward
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

V5 addresses Simon's review comments, mostly in the first two patches and the
"Create xml..." patch. Added changes since previous verion in the patch
headers. Series is slightly shorted as some early patches have been committed.

Summary:

For those targets that use new style target descriptions (also known as
flexible target descriptions), this set of patches removes the dependency on
xml files. Namely:
* Removes inclusion of xml files within gdbserver.
* Removes the requirement for the .c files in features/ to be generated from
cached xml files.
This is made possible by changing xml descriptions generated by gdbserver, so
that instead of including xml file names, gdbserver now generate a complete
xml description.

The second point will be required for aarch64 SVE support, where the register
size are variable. Creating SVE xml files for every possible vector length
would not be feasible. Instead the plan for aarch64 SVE is to hand write the
features/ .c code that would normally be generated from xml.

Targets which use the older style target descriptions have not been changed.


XML Generation:

In existing code, gdbserver uses C code auto generated from xml files to
create target descriptions. When sending an xml description to GDB, the
function tdesc_get_features_xml () creates an xml containing the name of the
original xml file(s). For example:

<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
  <architecture>i386</architecture>
  <osabi>GNU/Linux</osabi>
  <xi:include href="32bit-core.xml"/>
  <xi:include href="32bit-sse.xml"/>
  <xi:include href="32bit-linux.xml"/>
  <xi:include href="32bit-avx.xml"/>
</target>

Upon receipt, GDB then makes requests to gdbserver for the contents of the
xml files. Gdbserver keeps full copies all the xml files inside the binary.

This patch series adds common code that allows gdbserver (and gdb) to turn
a C target description structure into xml.
Now when asked fort an xml description to gdb, gdbserver turns the entire
target description structure back into xml, without using any cached files.
Producing, for example:

<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
  <architecture>i386</architecture>
  <osabi>GNU/Linux</osabi>
  <feature name="org.gnu.gdb.i386.core">
    <flags id="i386_eflags" size="4">
      <field name="CF" start="0" end="0"/>
      <field name="" start="1" end="1"/>
      <field name="PF" start="2" end="2"/>
      <field name="AF" start="4" end="4"/>
...etc...


Patch Contents:

Patches 1-3 commonise the various target descriptor functionality, allowing
gdbserver to parse target descriptions in the same way as gdb. This series
does not commonise target_desc, but this is hopefully a long term goal.

The sixth patch adds the xml printer, which iterates through the parsing
generated in the previous patches.

The other patches are clean up patches.

Patches have been tested on a make check on x86 targets=all build with
target boards unix and native-gdbserver. Also built and tested aarch64 and
Arm32 (which uses old style descriptions)
In addition, new test cases are added to the unit tests.

Alan Hayward (8):
  Commonise tdesc_reg
  Commonise tdesc_feature
  Commonise tdesc types
  Add tdesc osabi and architecture functions
  Add feature reference in .dat files
  Create xml from target descriptions
  Remove xml file references from target descriptions.
  Remove xml files from gdbserver

 gdb/Makefile.in                                    |   2 +
 gdb/common/tdesc.c                                 | 400 ++++++++++++++
 gdb/common/tdesc.h                                 | 322 ++++++++++-
 gdb/features/Makefile                              |   6 +
 gdb/features/aarch64-core.c                        |   2 +-
 gdb/features/aarch64-fpu.c                         |   2 +-
 gdb/features/i386/32bit-avx.c                      |   2 +-
 gdb/features/i386/32bit-avx512.c                   |   2 +-
 gdb/features/i386/32bit-core.c                     |   2 +-
 gdb/features/i386/32bit-linux.c                    |   2 +-
 gdb/features/i386/32bit-mpx.c                      |   2 +-
 gdb/features/i386/32bit-pkeys.c                    |   2 +-
 gdb/features/i386/32bit-sse.c                      |   2 +-
 gdb/features/i386/64bit-avx.c                      |   2 +-
 gdb/features/i386/64bit-avx512.c                   |   2 +-
 gdb/features/i386/64bit-core.c                     |   2 +-
 gdb/features/i386/64bit-linux.c                    |   2 +-
 gdb/features/i386/64bit-mpx.c                      |   2 +-
 gdb/features/i386/64bit-pkeys.c                    |   2 +-
 gdb/features/i386/64bit-segments.c                 |   2 +-
 gdb/features/i386/64bit-sse.c                      |   2 +-
 gdb/features/i386/x32-core.c                       |   2 +-
 gdb/features/tic6x-c6xp.c                          |   2 +-
 gdb/features/tic6x-core.c                          |   2 +-
 gdb/features/tic6x-gp.c                            |   2 +-
 gdb/gdbserver/Makefile.in                          |   3 +
 gdb/gdbserver/configure.srv                        |  28 -
 gdb/gdbserver/server.c                             |   2 +-
 gdb/gdbserver/tdesc.c                              | 182 ++-----
 gdb/gdbserver/tdesc.h                              |  19 +-
 gdb/regformats/aarch64.dat                         |   1 +
 gdb/regformats/i386/amd64-avx-avx512-linux.dat     |   1 +
 gdb/regformats/i386/amd64-avx-linux.dat            |   1 +
 .../i386/amd64-avx-mpx-avx512-pku-linux.dat        |   1 +
 gdb/regformats/i386/amd64-avx-mpx-linux.dat        |   1 +
 gdb/regformats/i386/amd64-linux.dat                |   1 +
 gdb/regformats/i386/amd64-mpx-linux.dat            |   1 +
 gdb/regformats/i386/amd64.dat                      |   1 +
 gdb/regformats/i386/i386-avx-avx512-linux.dat      |   1 +
 gdb/regformats/i386/i386-avx-linux.dat             |   1 +
 .../i386/i386-avx-mpx-avx512-pku-linux.dat         |   1 +
 gdb/regformats/i386/i386-avx-mpx-linux.dat         |   1 +
 gdb/regformats/i386/i386-linux.dat                 |   1 +
 gdb/regformats/i386/i386-mmx-linux.dat             |   1 +
 gdb/regformats/i386/i386-mpx-linux.dat             |   1 +
 gdb/regformats/i386/i386.dat                       |   1 +
 gdb/regformats/i386/x32-avx-avx512-linux.dat       |   1 +
 gdb/regformats/i386/x32-avx-linux.dat              |   1 +
 gdb/regformats/i386/x32-linux.dat                  |   1 +
 gdb/regformats/regdat.sh                           |  12 +-
 gdb/regformats/regdef.h                            |   8 +-
 gdb/regformats/tic6x-c62x-linux.dat                |   1 +
 gdb/regformats/tic6x-c64x-linux.dat                |   1 +
 gdb/regformats/tic6x-c64xp-linux.dat               |   1 +
 gdb/target-descriptions.c                          | 604 +++------------------
 gdb/target-descriptions.h                          |   8 -
 gdb/xml-tdesc.c                                    |  11 +
 gdb/xml-tdesc.h                                    |   5 +
 58 files changed, 940 insertions(+), 736 deletions(-)
 create mode 100644 gdb/common/tdesc.c

-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 1/8] Commonise tdesc_reg
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (5 preceding siblings ...)
  2018-04-10 14:34 ` [PATCH v5 3/8] Commonise tdesc types Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-18  1:57   ` Simon Marchi
  2018-04-10 14:34 ` [PATCH v5 8/8] Remove xml files from gdbserver Alan Hayward
  2018-04-18  2:49 ` [PATCH v5 0/8] Remove gdbserver dependency on xml files Simon Marchi
  8 siblings, 1 reply; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.

gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.

However, other modules in gdbserver directly access the reg_defs structure.
To work around this, init_target_desc fills in reg_defs by parsing the
tdesc_reg vector.
The long term goal is to remove reg_defs, replacing with accessor funcs.

I wanted to make tdesc_create_reg common, but I cannot do that until
the next patch.

I also had to commonise tdesc_element_visitor and tdesc_element.

This patch only differs from the V4 version in init_target_desc() and
the changing of constructors for regdef.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* Makefile.in: Add arch/tdesc.c
	* common/tdesc.c: New file.
	* common/tdesc.h (tdesc_element_visitor): Move to here.
	(tdesc_element): Likewise.
	(tdesc_reg): Likewise.
	(tdesc_reg_up): Likewise.
	* regformats/regdef.h (reg): Add offset to constructors.
	* target-descriptions.c (tdesc_element_visitor): Move from here.
	(tdesc_element): Likewise.
	(tdesc_reg): Likewise.
	(tdesc_reg_up): Likewise.

gdbserver/
	* Makefile.in: Add common/tdesc.c
	* tdesc.c (init_target_desc): init all reg_defs from register vector.
	(tdesc_create_reg): Create tdesc_reg.
	* tdesc.h (tdesc_feature): Add register vector.
---
 gdb/Makefile.in           |   2 +
 gdb/common/tdesc.c        |  35 ++++++++++++++
 gdb/common/tdesc.h        | 105 +++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/Makefile.in |   3 ++
 gdb/gdbserver/tdesc.c     |  23 +++++----
 gdb/gdbserver/tdesc.h     |   5 +-
 gdb/regformats/regdef.h   |   8 ++--
 gdb/target-descriptions.c | 116 ----------------------------------------------
 8 files changed, 168 insertions(+), 129 deletions(-)
 create mode 100644 gdb/common/tdesc.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0a07cabb43..4aa27e7c4a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -968,6 +968,7 @@ COMMON_SFILES = \
 	common/run-time-clock.c \
 	common/signals.c \
 	common/signals-state-save-restore.c \
+	common/tdesc.c \
 	common/vec.c \
 	common/xml-utils.c \
 	complaints.c \
@@ -1449,6 +1450,7 @@ HFILES_NO_SRCDIR = \
 	common/run-time-clock.h \
 	common/signals-state-save-restore.h \
 	common/symbol.h \
+	common/tdesc.h \
 	common/vec.h \
 	common/version.h \
 	common/x86-xstate.h \
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
new file mode 100644
index 0000000000..8a3d620b04
--- /dev/null
+++ b/gdb/common/tdesc.c
@@ -0,0 +1,35 @@
+/* Target description support for GDB.
+
+   Copyright (C) 2018 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 "common-defs.h"
+#include "common/tdesc.h"
+
+tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
+		      int regnum, int save_restore_, const char *group_,
+		      int bitsize_, const char *type_)
+  : name (name_), target_regnum (regnum),
+    save_restore (save_restore_),
+    group (group_ != NULL ? group_ : ""),
+    bitsize (bitsize_),
+    type (type_ != NULL ? type_ : "<unknown>")
+{
+  /* If the register's type is target-defined, look it up now.  We may not
+     have easy access to the containing feature when we want it later.  */
+  tdesc_type = tdesc_named_type (feature, type.c_str ());
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index cc11651dca..0ec45af3b4 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -26,6 +26,111 @@ struct tdesc_type_with_fields;
 struct tdesc_reg;
 struct target_desc;
 
+/* The interface to visit different elements of target description.  */
+
+class tdesc_element_visitor
+{
+public:
+  virtual void visit_pre (const target_desc *e)
+  {}
+
+  virtual void visit_post (const target_desc *e)
+  {}
+
+  virtual void visit_pre (const tdesc_feature *e)
+  {}
+
+  virtual void visit_post (const tdesc_feature *e)
+  {}
+
+  virtual void visit (const tdesc_type_builtin *e)
+  {}
+
+  virtual void visit (const tdesc_type_vector *e)
+  {}
+
+  virtual void visit (const tdesc_type_with_fields *e)
+  {}
+
+  virtual void visit (const tdesc_reg *e)
+  {}
+};
+
+class tdesc_element
+{
+public:
+  virtual void accept (tdesc_element_visitor &v) const = 0;
+};
+
+/* An individual register from a target description.  */
+
+struct tdesc_reg : tdesc_element
+{
+  tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
+	     int regnum, int save_restore_, const char *group_,
+	     int bitsize_, const char *type_);
+
+  virtual ~tdesc_reg () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_reg);
+
+  /* The name of this register.  In standard features, it may be
+     recognized by the architecture support code, or it may be purely
+     for the user.  */
+  std::string name;
+
+  /* The register number used by this target to refer to this
+     register.  This is used for remote p/P packets and to determine
+     the ordering of registers in the remote g/G packets.  */
+  long target_regnum;
+
+  /* If this flag is set, GDB should save and restore this register
+     around calls to an inferior function.  */
+  int save_restore;
+
+  /* The name of the register group containing this register, or empty
+     if the group should be automatically determined from the
+     register's type.  If this is "general", "float", or "vector", the
+     corresponding "info" command should display this register's
+     value.  It can be an arbitrary string, but should be limited to
+     alphanumeric characters and internal hyphens.  Currently other
+     strings are ignored (treated as empty).  */
+  std::string group;
+
+  /* The size of the register, in bits.  */
+  int bitsize;
+
+  /* The type of the register.  This string corresponds to either
+     a named type from the target description or a predefined
+     type from GDB.  */
+  std::string type;
+
+  /* The target-described type corresponding to TYPE, if found.  */
+  struct tdesc_type *tdesc_type;
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+
+  bool operator== (const tdesc_reg &other) const
+  {
+    return (name == other.name
+       && target_regnum == other.target_regnum
+       && save_restore == other.save_restore
+       && bitsize == other.bitsize
+       && group == other.group
+       && type == other.type);
+  }
+
+  bool operator!= (const tdesc_reg &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
+
 /* Allocate a new target_desc.  */
 target_desc *allocate_target_description (void);
 
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 75fbf7ec75..4a54235dae 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -215,6 +215,7 @@ SFILES = \
 	$(srcdir)/common/print-utils.c \
 	$(srcdir)/common/ptid.c \
 	$(srcdir)/common/rsp-low.c \
+	$(srcdir)/common/tdesc.c \
 	$(srcdir)/common/vec.c \
 	$(srcdir)/common/xml-utils.c \
 	$(srcdir)/nat/linux-btrace.c \
@@ -258,6 +259,7 @@ OBS = \
 	common/rsp-low.o \
 	common/signals.o \
 	common/signals-state-save-restore.o \
+	common/tdesc.o \
 	common/vec.o \
 	common/xml-utils.o \
 	debug.o \
@@ -403,6 +405,7 @@ IPA_OBJS = \
 	common/format-ipa.o \
 	common/print-utils-ipa.o \
 	common/rsp-low-ipa.o \
+	common/tdesc-ipa.o \
 	common/vec-ipa.o \
 	regcache-ipa.o \
 	remote-utils-ipa.o \
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 9f00ecf938..afe0187aeb 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -54,10 +54,19 @@ init_target_desc (struct target_desc *tdesc)
 {
   int offset = 0;
 
-  for (reg &reg : tdesc->reg_defs)
+  /* Go through all the features and populate reg_defs.  */
+  for (const tdesc_reg_up &treg : tdesc->registers)
     {
-      reg.offset = offset;
-      offset += reg.size;
+      int regnum = treg->target_regnum;
+
+      /* Register number will increase (possibly with gaps) or be zero.  */
+      gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());
+
+      if (regnum != 0)
+	tdesc->reg_defs.resize (regnum, reg (offset));
+
+      tdesc->reg_defs.emplace_back (treg->name.c_str (), offset, treg->bitsize);
+      offset += treg->bitsize;
     }
 
   tdesc->registers_size = offset / 8;
@@ -222,12 +231,10 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 {
   struct target_desc *tdesc = (struct target_desc *) feature;
 
-  gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());
-
-  if (regnum != 0)
-    tdesc->reg_defs.resize (regnum);
+  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+				  group, bitsize, type);
 
-  tdesc->reg_defs.emplace_back (name, bitsize);
+  tdesc->registers.emplace_back (reg);
 }
 
 /* See common/tdesc.h.  */
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 85139d948c..8eb88eedce 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -25,7 +25,10 @@
 #include <vector>
 
 struct tdesc_feature
-{};
+{
+  /* The registers associated with this feature.  */
+  std::vector<tdesc_reg_up> registers;
+};
 
 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */
diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
index 4775e863e9..7c80d45d48 100644
--- a/gdb/regformats/regdef.h
+++ b/gdb/regformats/regdef.h
@@ -21,15 +21,15 @@
 
 struct reg
 {
-  reg ()
+  reg (int _offset)
     : name (""),
-      offset (0),
+      offset (_offset),
       size (0)
   {}
 
-  reg (const char *_name, int _size)
+  reg (const char *_name, int _offset, int _size)
     : name (_name),
-      offset (0),
+      offset (_offset),
       size (_size)
   {}
 
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 5d34e29a86..3186bf886f 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -38,44 +38,6 @@
 #include "completer.h"
 #include "readline/tilde.h" /* tilde_expand */
 
-static type *make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype);
-
-/* The interface to visit different elements of target description.  */
-
-class tdesc_element_visitor
-{
-public:
-  virtual void visit_pre (const target_desc *e)
-  {}
-
-  virtual void visit_post (const target_desc *e)
-  {}
-
-  virtual void visit_pre (const tdesc_feature *e)
-  {}
-
-  virtual void visit_post (const tdesc_feature *e)
-  {}
-
-  virtual void visit (const tdesc_type_builtin *e)
-  {}
-
-  virtual void visit (const tdesc_type_vector *e)
-  {}
-
-  virtual void visit (const tdesc_type_with_fields *e)
-  {}
-
-  virtual void visit (const tdesc_reg *e)
-  {}
-};
-
-class tdesc_element
-{
-public:
-  virtual void accept (tdesc_element_visitor &v) const = 0;
-};
-
 /* Types.  */
 
 struct property
@@ -88,84 +50,6 @@ struct property
   std::string value;
 };
 
-/* An individual register from a target description.  */
-
-struct tdesc_reg : tdesc_element
-{
-  tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
-	     int regnum, int save_restore_, const char *group_,
-	     int bitsize_, const char *type_)
-    : name (name_), target_regnum (regnum),
-      save_restore (save_restore_),
-      group (group_ != NULL ? group_ : ""),
-      bitsize (bitsize_),
-      type (type_ != NULL ? type_ : "<unknown>")
-  {
-    /* If the register's type is target-defined, look it up now.  We may not
-       have easy access to the containing feature when we want it later.  */
-    tdesc_type = tdesc_named_type (feature, type.c_str ());
-  }
-
-  virtual ~tdesc_reg () = default;
-
-  DISABLE_COPY_AND_ASSIGN (tdesc_reg);
-
-  /* The name of this register.  In standard features, it may be
-     recognized by the architecture support code, or it may be purely
-     for the user.  */
-  std::string name;
-
-  /* The register number used by this target to refer to this
-     register.  This is used for remote p/P packets and to determine
-     the ordering of registers in the remote g/G packets.  */
-  long target_regnum;
-
-  /* If this flag is set, GDB should save and restore this register
-     around calls to an inferior function.  */
-  int save_restore;
-
-  /* The name of the register group containing this register, or empty
-     if the group should be automatically determined from the register's
-     type.  This is traditionally "general", "float", "vector" but can
-     also be an arbitrary string.  If defined the corresponding "info"
-     command should display this register's value.  The string should be
-     limited to alphanumeric characters and internal hyphens.  */
-  std::string group;
-
-  /* The size of the register, in bits.  */
-  int bitsize;
-
-  /* The type of the register.  This string corresponds to either
-     a named type from the target description or a predefined
-     type from GDB.  */
-  std::string type;
-
-  /* The target-described type corresponding to TYPE, if found.  */
-  struct tdesc_type *tdesc_type;
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-
-  bool operator== (const tdesc_reg &other) const
-  {
-    return (name == other.name
-	    && target_regnum == other.target_regnum
-	    && save_restore == other.save_restore
-	    && bitsize == other.bitsize
-	    && group == other.group
-	    && type == other.type);
-  }
-
-  bool operator!= (const tdesc_reg &other) const
-  {
-    return !(*this == other);
-  }
-};
-
-typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
-
 /* A named type from a target description.  */
 
 struct tdesc_type_field
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 3/8] Commonise tdesc types
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (4 preceding siblings ...)
  2018-04-10 14:34 ` [PATCH v5 5/8] Add feature reference in .dat files Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 1/8] Commonise tdesc_reg Alan Hayward
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

This patch moves all the various tdesc types to common code.

It also commonises all the tdesc_ functions that are stubbed
out in gdbserver.

This is a lot of code to move across, but it is the simplest
way of getting gdbserver to retain the target description
information.

With this patch, gdb and gdbserver will now parse a target
description in the same way.

Identical to V4 version, except for additional declarations
moved from common/tdesc.h.

Alan.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* common/tdesc.c (tdesc_predefined_type): Move to here.
	(tdesc_named_type): Likewise.
	(tdesc_create_vector): Likewise.
	(tdesc_create_struct): Likewise.
	(tdesc_set_struct_size): Likewise.
	(tdesc_create_union): Likewise.
	(tdesc_create_flags): Likewise.
	(tdesc_create_enum): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_flag): Likewise.
	(tdesc_add_enum_value): Likewise.
	* common/tdesc.h (struct tdesc_type_builtin): Likewise.
	(struct tdesc_type_vector): Likewise.
	(struct tdesc_type_field): Likewise.
	(struct tdesc_type_with_fields): Likewise.
	(tdesc_create_enum): Add declaration.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_enum_value): Likewise.
	* target-descriptions.c (tdesc_type_field): Move from here.
	(tdesc_type_builtin): Likewise.
	(tdesc_type_vector): Likewise.
	(tdesc_type_with_fields): Likewise.
	(tdesc_predefined_types): Likewise.
	(tdesc_named_type): Likewise.
	(tdesc_create_vector): Likewise.
	(tdesc_create_struct): Likewise.
	(tdesc_set_struct_size): Likewise.
	(tdesc_create_union): Likewise.
	(tdesc_create_flags): Likewise.
	(tdesc_create_enum): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_flag): Likewise.
	(tdesc_add_enum_value): Likewise.
	* gdb/target-descriptions.h (tdesc_create_enum): Likewise.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_enum_value): Likewise.

gdbserver/
	* tdesc.c (tdesc_create_flags): Remove.
	(tdesc_add_flag): Likewise.
	(tdesc_named_type): Likewise.
	(tdesc_create_union): Likewise.
	(tdesc_create_struct): Likewise.
	(tdesc_create_vector): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_set_struct_size): Likewise.
---
 gdb/common/tdesc.c        | 199 +++++++++++++++++++++++++++++++++++
 gdb/common/tdesc.h        |  82 +++++++++++++++
 gdb/gdbserver/tdesc.c     |  68 ------------
 gdb/target-descriptions.c | 258 ----------------------------------------------
 gdb/target-descriptions.h |   8 --
 5 files changed, 281 insertions(+), 334 deletions(-)

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index eefb2016f6..b9e9ddb3fa 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -34,6 +34,28 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
   tdesc_type = tdesc_named_type (feature, type.c_str ());
 }
 
+/* Predefined types.  */
+static tdesc_type_builtin tdesc_predefined_types[] =
+{
+  { "bool", TDESC_TYPE_BOOL },
+  { "int8", TDESC_TYPE_INT8 },
+  { "int16", TDESC_TYPE_INT16 },
+  { "int32", TDESC_TYPE_INT32 },
+  { "int64", TDESC_TYPE_INT64 },
+  { "int128", TDESC_TYPE_INT128 },
+  { "uint8", TDESC_TYPE_UINT8 },
+  { "uint16", TDESC_TYPE_UINT16 },
+  { "uint32", TDESC_TYPE_UINT32 },
+  { "uint64", TDESC_TYPE_UINT64 },
+  { "uint128", TDESC_TYPE_UINT128 },
+  { "code_ptr", TDESC_TYPE_CODE_PTR },
+  { "data_ptr", TDESC_TYPE_DATA_PTR },
+  { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
+  { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
+  { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
+  { "i387_ext", TDESC_TYPE_I387_EXT }
+};
+
 void tdesc_feature::accept (tdesc_element_visitor &v) const
 {
   v.visit_pre (this);
@@ -79,6 +101,36 @@ bool tdesc_feature::operator== (const tdesc_feature &other) const
   return true;
 }
 
+/* Lookup a predefined type.  */
+
+static struct tdesc_type *
+tdesc_predefined_type (enum tdesc_type_kind kind)
+{
+  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+    if (tdesc_predefined_types[ix].kind == kind)
+      return &tdesc_predefined_types[ix];
+
+  gdb_assert_not_reached ("bad predefined tdesc type");
+}
+
+/* See common/tdesc.h.  */
+
+struct tdesc_type *
+tdesc_named_type (const struct tdesc_feature *feature, const char *id)
+{
+  /* First try target-defined types.  */
+  for (const tdesc_type_up &type : feature->types)
+    if (type->name == id)
+      return type.get ();
+
+  /* Next try the predefined types.  */
+  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+    if (tdesc_predefined_types[ix].name == id)
+      return &tdesc_predefined_types[ix];
+
+  return NULL;
+}
+
 /* See common/tdesc.h.  */
 
 void
@@ -91,3 +143,150 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 
   feature->registers.emplace_back (reg);
 }
+
+/* See common/tdesc.h.  */
+
+struct tdesc_type *
+tdesc_create_vector (struct tdesc_feature *feature, const char *name,
+		     struct tdesc_type *field_type, int count)
+{
+  tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_struct (struct tdesc_feature *feature, const char *name)
+{
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
+{
+  gdb_assert (type->kind == TDESC_TYPE_STRUCT);
+  gdb_assert (size > 0);
+  type->size = size;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_union (struct tdesc_feature *feature, const char *name)
+{
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_flags (struct tdesc_feature *feature, const char *name,
+		    int size)
+{
+  gdb_assert (size > 0);
+
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_enum (struct tdesc_feature *feature, const char *name,
+		   int size)
+{
+  gdb_assert (size > 0);
+
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
+		 struct tdesc_type *field_type)
+{
+  gdb_assert (type->kind == TDESC_TYPE_UNION
+	      || type->kind == TDESC_TYPE_STRUCT);
+
+  /* Initialize start and end so we know this is not a bit-field
+     when we print-c-tdesc.  */
+  type->fields.emplace_back (field_name, field_type, -1, -1);
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
+			  int start, int end, struct tdesc_type *field_type)
+{
+  gdb_assert (type->kind == TDESC_TYPE_STRUCT
+	      || type->kind == TDESC_TYPE_FLAGS);
+  gdb_assert (start >= 0 && end >= start);
+
+  type->fields.emplace_back (field_name, field_type, start, end);
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
+		    int start, int end)
+{
+  struct tdesc_type *field_type;
+
+  gdb_assert (start >= 0 && end >= start);
+
+  if (type->size > 4)
+    field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
+  else
+    field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
+
+  tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_flag (tdesc_type_with_fields *type, int start,
+		const char *flag_name)
+{
+  gdb_assert (type->kind == TDESC_TYPE_FLAGS
+	      || type->kind == TDESC_TYPE_STRUCT);
+
+  type->fields.emplace_back (flag_name,
+			     tdesc_predefined_type (TDESC_TYPE_BOOL),
+			     start, start);
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
+		      const char *name)
+{
+  gdb_assert (type->kind == TDESC_TYPE_ENUM);
+  type->fields.emplace_back (name,
+			     tdesc_predefined_type (TDESC_TYPE_INT32),
+			     value, -1);
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index b501dfa995..7f4222b653 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -189,6 +189,72 @@ struct tdesc_type : tdesc_element
 
 typedef std::unique_ptr<tdesc_type> tdesc_type_up;
 
+struct tdesc_type_builtin : tdesc_type
+{
+  tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
+  : tdesc_type (name, kind)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+};
+
+/* tdesc_type for vector types.  */
+
+struct tdesc_type_vector : tdesc_type
+{
+  tdesc_type_vector (const std::string &name, tdesc_type *element_type_,
+		     int count_)
+  : tdesc_type (name, TDESC_TYPE_VECTOR),
+    element_type (element_type_), count (count_)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+
+  struct tdesc_type *element_type;
+  int count;
+};
+
+/* A named type from a target description.  */
+
+struct tdesc_type_field
+{
+  tdesc_type_field (const std::string &name_, tdesc_type *type_,
+		    int start_, int end_)
+  : name (name_), type (type_), start (start_), end (end_)
+  {}
+
+  std::string name;
+  struct tdesc_type *type;
+  /* For non-enum-values, either both are -1 (non-bitfield), or both are
+     not -1 (bitfield).  For enum values, start is the value (which could be
+     -1), end is -1.  */
+  int start, end;
+};
+
+/* tdesc_type for struct, union, flags, and enum types.  */
+
+struct tdesc_type_with_fields : tdesc_type
+{
+  tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
+			  int size_ = 0)
+  : tdesc_type (name, kind), size (size_)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+
+  std::vector<tdesc_type_field> fields;
+  int size;
+};
+
 /* A feature from a target description.  Each feature is a collection
    of other elements, e.g. registers and types.  */
 
@@ -264,11 +330,23 @@ tdesc_type_with_fields *tdesc_create_flags (struct tdesc_feature *feature,
 					    const char *name,
 					    int size);
 
+/* Return the created enum tdesc_type named NAME in FEATURE.  */
+tdesc_type_with_fields *tdesc_create_enum (struct tdesc_feature *feature,
+					   const char *name,
+					   int size);
+
 /* Add a new field to TYPE.  FIELD_NAME is its name, and FIELD_TYPE is
    its type.  */
 void tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
 		      struct tdesc_type *field_type);
 
+/* Add a new bitfield to TYPE, with range START to END.  FIELD_NAME is its name,
+   and FIELD_TYPE is its type.  */
+void tdesc_add_typed_bitfield (tdesc_type_with_fields *type,
+			       const char *field_name,
+			       int start, int end,
+			       struct tdesc_type *field_type);
+
 /* Set the total length of TYPE.  Structs which contain bitfields may
    omit the reserved bits, so the end of the last field may not
    suffice.  */
@@ -285,6 +363,10 @@ void tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
 void tdesc_add_flag (tdesc_type_with_fields *type, int start,
 		     const char *flag_name);
 
+/* Add field with VALUE and NAME to the enum TYPE.  */
+void tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
+			   const char *name);
+
 /* Create a register in feature FEATURE.  */
 void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		       int regnum, int save_restore, const char *group,
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 1047949e6d..aca27ea3b0 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -177,71 +177,3 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
   tdesc->features.emplace_back (new_feature);
   return new_feature;
 }
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
-		    int size)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
-		const char *flag_name)
-{}
-
-/* See common/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
-		     struct tdesc_type *field_type, int count)
-{
-  return NULL;
-}
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
-		    int start, int end)
-{}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
-		 struct tdesc_type *field_type)
-{}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
-}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index a453eddec0..2782ffaab9 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -50,71 +50,6 @@ struct property
   std::string value;
 };
 
-/* A named type from a target description.  */
-
-struct tdesc_type_field
-{
-  tdesc_type_field (const std::string &name_, tdesc_type *type_,
-		    int start_, int end_)
-  : name (name_), type (type_), start (start_), end (end_)
-  {}
-
-  std::string name;
-  struct tdesc_type *type;
-  /* For non-enum-values, either both are -1 (non-bitfield), or both are
-     not -1 (bitfield).  For enum values, start is the value (which could be
-     -1), end is -1.  */
-  int start, end;
-};
-
-struct tdesc_type_builtin : tdesc_type
-{
-  tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
-  : tdesc_type (name, kind)
-  {}
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-};
-
-/* tdesc_type for vector types.  */
-
-struct tdesc_type_vector : tdesc_type
-{
-  tdesc_type_vector (const std::string &name, tdesc_type *element_type_, int count_)
-  : tdesc_type (name, TDESC_TYPE_VECTOR),
-    element_type (element_type_), count (count_)
-  {}
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-
-  struct tdesc_type *element_type;
-  int count;
-};
-
-/* tdesc_type for struct, union, flags, and enum types.  */
-
-struct tdesc_type_with_fields : tdesc_type
-{
-  tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
-			  int size_ = 0)
-  : tdesc_type (name, kind), size (size_)
-  {}
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-
-  std::vector<tdesc_type_field> fields;
-  int size;
-};
-
 /* Convert a tdesc_type to a gdb type.  */
 
 static type *
@@ -741,58 +676,6 @@ tdesc_feature_name (const struct tdesc_feature *feature)
   return feature->name.c_str ();
 }
 
-/* Predefined types.  */
-static tdesc_type_builtin tdesc_predefined_types[] =
-{
-  { "bool", TDESC_TYPE_BOOL },
-  { "int8", TDESC_TYPE_INT8 },
-  { "int16", TDESC_TYPE_INT16 },
-  { "int32", TDESC_TYPE_INT32 },
-  { "int64", TDESC_TYPE_INT64 },
-  { "int128", TDESC_TYPE_INT128 },
-  { "uint8", TDESC_TYPE_UINT8 },
-  { "uint16", TDESC_TYPE_UINT16 },
-  { "uint32", TDESC_TYPE_UINT32 },
-  { "uint64", TDESC_TYPE_UINT64 },
-  { "uint128", TDESC_TYPE_UINT128 },
-  { "code_ptr", TDESC_TYPE_CODE_PTR },
-  { "data_ptr", TDESC_TYPE_DATA_PTR },
-  { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
-  { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
-  { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
-  { "i387_ext", TDESC_TYPE_I387_EXT }
-};
-
-/* Lookup a predefined type.  */
-
-static struct tdesc_type *
-tdesc_predefined_type (enum tdesc_type_kind kind)
-{
-  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
-    if (tdesc_predefined_types[ix].kind == kind)
-      return &tdesc_predefined_types[ix];
-
-  gdb_assert_not_reached ("bad predefined tdesc type");
-}
-
-/* See common/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
-  /* First try target-defined types.  */
-  for (const tdesc_type_up &type : feature->types)
-    if (type->name == id)
-      return type.get ();
-
-  /* Next try the predefined types.  */
-  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
-    if (tdesc_predefined_types[ix].name == id)
-      return &tdesc_predefined_types[ix];
-
-  return NULL;
-}
-
 /* Lookup type associated with ID.  */
 
 struct type *
@@ -1227,147 +1110,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 
 /* See common/tdesc.h.  */
 
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
-		     struct tdesc_type *field_type, int count)
-{
-  tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *name)
-{
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
-  gdb_assert (type->kind == TDESC_TYPE_STRUCT);
-  gdb_assert (size > 0);
-  type->size = size;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *name)
-{
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
-		    int size)
-{
-  gdb_assert (size > 0);
-
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-tdesc_type_with_fields *
-tdesc_create_enum (struct tdesc_feature *feature, const char *name,
-		   int size)
-{
-  gdb_assert (size > 0);
-
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
-		 struct tdesc_type *field_type)
-{
-  gdb_assert (type->kind == TDESC_TYPE_UNION
-	      || type->kind == TDESC_TYPE_STRUCT);
-
-  /* Initialize start and end so we know this is not a bit-field
-     when we print-c-tdesc.  */
-  type->fields.emplace_back (field_name, field_type, -1, -1);
-}
-
-void
-tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
-			  int start, int end, struct tdesc_type *field_type)
-{
-  gdb_assert (type->kind == TDESC_TYPE_STRUCT
-	      || type->kind == TDESC_TYPE_FLAGS);
-  gdb_assert (start >= 0 && end >= start);
-
-  type->fields.emplace_back (field_name, field_type, start, end);
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
-		    int start, int end)
-{
-  struct tdesc_type *field_type;
-
-  gdb_assert (start >= 0 && end >= start);
-
-  if (type->size > 4)
-    field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
-  else
-    field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
-
-  tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
-		const char *flag_name)
-{
-  gdb_assert (type->kind == TDESC_TYPE_FLAGS
-	      || type->kind == TDESC_TYPE_STRUCT);
-
-  type->fields.emplace_back (flag_name,
-			     tdesc_predefined_type (TDESC_TYPE_BOOL),
-			     start, start);
-}
-
-void
-tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
-		      const char *name)
-{
-  gdb_assert (type->kind == TDESC_TYPE_ENUM);
-  type->fields.emplace_back (name,
-			     tdesc_predefined_type (TDESC_TYPE_INT32),
-			     value, -1);
-}
-
-/* See common/tdesc.h.  */
-
 struct tdesc_feature *
 tdesc_create_feature (struct target_desc *tdesc, const char *name,
 		      const char *xml)
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index fa554fddf6..3ba71b1add 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -209,14 +209,6 @@ void set_tdesc_property (struct target_desc *,
 			 const char *key, const char *value);
 void tdesc_add_compatible (struct target_desc *,
 			   const struct bfd_arch_info *);
-tdesc_type_with_fields *tdesc_create_enum (struct tdesc_feature *feature,
-					   const char *name,
-					   int size);
-void tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
-			       int start, int end,
-			       struct tdesc_type *field_type);
-void tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
-			   const char *name);
 
 #if GDB_SELF_TEST
 namespace selftests {
-- 
2.14.3 (Apple Git-98)

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

* [PATCH v5 4/8] Add tdesc osabi and architecture functions
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 6/8] Create xml from target descriptions Alan Hayward
  2018-04-10 14:34 ` [PATCH v5 2/8] Commonise tdesc_feature Alan Hayward
@ 2018-04-10 14:34 ` Alan Hayward
  2018-04-18  2:10   ` Simon Marchi
  2018-04-10 14:34 ` [PATCH v5 7/8] Remove xml file references from target descriptions Alan Hayward
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Alan Hayward @ 2018-04-10 14:34 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

Add functions to access to printable names for osabi and architecture in
target_desc.

I wanted to add these as member functions of target_desc, but cannot until
target_desc is moved into the header files.

Identical to V3/4 versions.

Alan.

2018-04-10  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* common/tdesc.h (tdesc_architecture_name): Add new declaration.
	(tdesc_osabi_name): Likewise.
	* target-descriptions.c (tdesc_architecture_name): Add new function.
	(tdesc_osabi_name): Likewise.

gdb/gdbserver/
	* tdesc.c (tdesc_architecture_name): Add new function.
	(tdesc_osabi_name): Likewise.
	(tdesc_get_features_xml): Use new functions.
---
 gdb/common/tdesc.h        |  9 +++++++++
 gdb/gdbserver/tdesc.c     | 23 ++++++++++++++++++++---
 gdb/target-descriptions.c | 19 ++++++++++++++++++-
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 7f4222b653..311341da0d 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -297,9 +297,18 @@ target_desc *allocate_target_description (void);
 void set_tdesc_architecture (target_desc *target_desc,
 			     const char *name);
 
+/* Return the architecture associated with this target description as a string,
+   or NULL if no architecture was specified.  */
+const char *tdesc_architecture_name (const struct target_desc *target_desc);
+
 /* Set TARGET_DESC's osabi by NAME.  */
 void set_tdesc_osabi (target_desc *target_desc, const char *name);
 
+/* Return the osabi associated with this target description as a string,
+   or NULL if no osabi was specified.  */
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc);
+
 /* Return the type associated with ID in the context of FEATURE, or
    NULL if none.  */
 struct tdesc_type *tdesc_named_type (const struct tdesc_feature *feature,
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index aca27ea3b0..7603a90a59 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -107,6 +107,14 @@ current_target_desc (void)
 
 /* See common/tdesc.h.  */
 
+const char *
+tdesc_architecture_name (const struct target_desc *target_desc)
+{
+  return target_desc->arch;
+}
+
+/* See common/tdesc.h.  */
+
 void
 set_tdesc_architecture (struct target_desc *target_desc,
 			const char *name)
@@ -116,6 +124,14 @@ set_tdesc_architecture (struct target_desc *target_desc,
 
 /* See common/tdesc.h.  */
 
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc)
+{
+  return target_desc->osabi;
+}
+
+/* See common/tdesc.h.  */
+
 void
 set_tdesc_osabi (struct target_desc *target_desc, const char *name)
 {
@@ -140,13 +156,14 @@ tdesc_get_features_xml (target_desc *tdesc)
       buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
       buffer += "<target>";
       buffer += "<architecture>";
-      buffer += tdesc->arch;
+      buffer += tdesc_architecture_name (tdesc);
       buffer += "</architecture>";
 
-      if (tdesc->osabi != nullptr)
+      const char *osabi = tdesc_osabi_name (tdesc);
+      if (osabi != nullptr)
 	{
 	  buffer += "<osabi>";
-	  buffer += tdesc->osabi;
+	  buffer += osabi;
 	  buffer += "</osabi>";
 	}
 
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 2782ffaab9..da2c1ce345 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -628,6 +628,14 @@ tdesc_architecture (const struct target_desc *target_desc)
   return target_desc->arch;
 }
 
+/* See common/tdesc.h.  */
+
+const char *
+tdesc_architecture_name (const struct target_desc *target_desc)
+{
+  return target_desc->arch->printable_name;
+}
+
 /* Return the OSABI associated with this target description, or
    GDB_OSABI_UNKNOWN if no osabi was specified.  */
 
@@ -637,7 +645,16 @@ tdesc_osabi (const struct target_desc *target_desc)
   return target_desc->osabi;
 }
 
-\f
+/* See common/tdesc.h.  */
+
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc)
+{
+  enum gdb_osabi osabi = tdesc_osabi (target_desc);
+  if (osabi > GDB_OSABI_UNKNOWN && osabi < GDB_OSABI_INVALID)
+    return gdbarch_osabi_name (osabi);
+  return nullptr;
+}
 
 /* Return 1 if this target description includes any registers.  */
 
-- 
2.14.3 (Apple Git-98)

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

* Re: [PATCH v5 1/8] Commonise tdesc_reg
  2018-04-10 14:34 ` [PATCH v5 1/8] Commonise tdesc_reg Alan Hayward
@ 2018-04-18  1:57   ` Simon Marchi
  2018-04-18  9:03     ` Alan Hayward
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Marchi @ 2018-04-18  1:57 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 2018-04-10 10:33 AM, Alan Hayward wrote:
> This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
> 
> gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
> a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.
> 
> However, other modules in gdbserver directly access the reg_defs structure.
> To work around this, init_target_desc fills in reg_defs by parsing the
> tdesc_reg vector.
> The long term goal is to remove reg_defs, replacing with accessor funcs.
> 
> I wanted to make tdesc_create_reg common, but I cannot do that until
> the next patch.
> 
> I also had to commonise tdesc_element_visitor and tdesc_element.
> 
> This patch only differs from the V4 version in init_target_desc() and
> the changing of constructors for regdef.

Hi Alan,

Just two small comment, but the patch LGTM with those answered or addressed.

> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 85139d948c..8eb88eedce 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -25,7 +25,10 @@
>  #include <vector>
>  
>  struct tdesc_feature
> -{};
> +{
> +  /* The registers associated with this feature.  */
> +  std::vector<tdesc_reg_up> registers;
> +};
>  
>  /* A target description.  Inherit from tdesc_feature so that target_desc
>     can be used as tdesc_feature.  */
> diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
> index 4775e863e9..7c80d45d48 100644
> --- a/gdb/regformats/regdef.h
> +++ b/gdb/regformats/regdef.h
> @@ -21,15 +21,15 @@
>  
>  struct reg
>  {
> -  reg ()
> +  reg (int _offset)
>      : name (""),
> -      offset (0),
> +      offset (_offset),
>        size (0)
>    {}

If this constructor is only used for padding entries, shouldn't name be
NULL, as the documentation for the field states?

Also, did you notice something failing if the padding entries don't have
the offset field to the "current" offset at the time they are created?
If we could leave them at 0, I think it would keep things simpler.  I
stopped for a few seconds, wondering why init_target_desc did:

  tdesc->reg_defs.resize (regnum, reg (offset));

and not just:

  tdesc->reg_defs.resize (regnum);

Simon

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

* Re: [PATCH v5 4/8] Add tdesc osabi and architecture functions
  2018-04-10 14:34 ` [PATCH v5 4/8] Add tdesc osabi and architecture functions Alan Hayward
@ 2018-04-18  2:10   ` Simon Marchi
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Marchi @ 2018-04-18  2:10 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

Hi Alan,

Just a nit:

On 2018-04-10 10:33 AM, Alan Hayward wrote:
> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
> index 7f4222b653..311341da0d 100644
> --- a/gdb/common/tdesc.h
> +++ b/gdb/common/tdesc.h
> @@ -297,9 +297,18 @@ target_desc *allocate_target_description (void);
>  void set_tdesc_architecture (target_desc *target_desc,
>  			     const char *name);
>  
> +/* Return the architecture associated with this target description as a string,
> +   or NULL if no architecture was specified.  */
> +const char *tdesc_architecture_name (const struct target_desc *target_desc);
> +
>  /* Set TARGET_DESC's osabi by NAME.  */
>  void set_tdesc_osabi (target_desc *target_desc, const char *name);
>  
> +/* Return the osabi associated with this target description as a string,
> +   or NULL if no osabi was specified.  */
> +const char *
> +tdesc_osabi_name (const struct target_desc *target_desc);

Remove the \n before the function name.

Simon

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

* Re: [PATCH v5 6/8] Create xml from target descriptions
  2018-04-10 14:34 ` [PATCH v5 6/8] Create xml from target descriptions Alan Hayward
@ 2018-04-18  2:43   ` Simon Marchi
  2018-04-18 21:26     ` Alan Hayward
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Marchi @ 2018-04-18  2:43 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 2018-04-10 10:33 AM, Alan Hayward wrote:
> This patch adds a print_xml_feature visitor class which turns a C
> target description into xml. Both gdb and gdbserver can do this.
> 
> An accept function is added to gdbserver tdesc to allow it to use
> vistor classes.
> 
> Tests are added to maintenance_check_xml_descriptions which takes
> each pair of tested descriptions, turns them both into xml, then back
> again, and confirms the descriptions still match.
> 
> Differences from V4 are:
> 
> print_xml_feature now uses a ptr.
> tdesc xmltarget is now mutable.
> Added maintenance_check_tdesc_xml_convert test function.
> Improved string_read_description_xml.
> 
> This patch requires the next patch in the series, otherwise target
> description will be rejected by gdb when using gdbserver. I'll make sure
> the two patches get committed in a single commit.

Hi Alan,

The patch looks good to me.  I noted a few formatting/minor comments.

> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
> index b9e9ddb3fa..b4bd7bfb00 100644
> --- a/gdb/common/tdesc.c
> +++ b/gdb/common/tdesc.c
> @@ -290,3 +290,111 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
>  			     tdesc_predefined_type (TDESC_TYPE_INT32),
>  			     value, -1);
>  }
> +
> +void print_xml_feature::visit_pre (const tdesc_feature *e)
> +{
> +  string_appendf (*m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
> +}
> +
> +void print_xml_feature::visit_post (const tdesc_feature *e)
> +{
> +  string_appendf (*m_buffer, "</feature>\n");
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_builtin *t)
> +{
> +  error (_("xml output is not supported type \"%s\"."), t->name.c_str ());

Just a nit, but it seems to be missing something like a "for" between "supported" and "type".

> +}
> +
> +void print_xml_feature::visit (const tdesc_type_vector *t)
> +{
> +  string_appendf (*m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
> +		  t->name.c_str (), t->element_type->name.c_str (), t->count);
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_with_fields *t)
> +{
> +  struct tdesc_type_field *f;
> +  const static char *types[] = { "struct", "union", "flags", "enum" };
> +
> +  gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
> +
> +  string_appendf (*m_buffer,
> +		  "<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
> +		  t->name.c_str ());
> +
> +  switch (t->kind)
> +    {
> +    case TDESC_TYPE_STRUCT:
> +    case TDESC_TYPE_FLAGS:
> +      if (t->size > 0)
> +	string_appendf (*m_buffer, " size=\"%d\"", t->size);
> +      string_appendf (*m_buffer, ">\n");
> +
> +      for (const tdesc_type_field &f : t->fields)
> +	{
> +	  string_appendf (*m_buffer, "  <field name=\"%s\" ", f.name.c_str ());
> +	  if (f.start == -1)
> +	    string_appendf (*m_buffer, "type=\"%s\"/>\n",
> +			    f.type->name.c_str ());
> +	  else
> +	    string_appendf (*m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
> +			    f.end);
> +	}
> +      break;
> +
> +    case TDESC_TYPE_ENUM:
> +      string_appendf (*m_buffer, ">\n");
> +      for (const tdesc_type_field &f : t->fields)
> +	string_appendf (*m_buffer, "  <field name=\"%s\" start=\"%d\"/>\n",
> +			f.name.c_str (), f.start);
> +      break;
> +
> +    case TDESC_TYPE_UNION:
> +      string_appendf (*m_buffer, ">\n");
> +      for (const tdesc_type_field &f : t->fields)
> +	string_appendf (*m_buffer, "  <field name=\"%s\" type=\"%s\"/>\n",
> +			f.name.c_str (), f.type->name.c_str ());
> +      break;
> +
> +    default:
> +      error (_("xml output is not supported type \"%s\"."), t->name.c_str ());

"supported for type" here too.

> +    }
> +
> +  string_appendf (*m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
> +}
> +
> +void print_xml_feature::visit (const tdesc_reg *r)
> +{
> +  string_appendf (*m_buffer,
> +		  "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
> +		  r->name.c_str (), r->bitsize, r->type.c_str (),
> +		  r->target_regnum);
> +
> +  if (r->group.length () > 0)
> +    string_appendf (*m_buffer, " group=\"%s\"", r->group.c_str ());
> +
> +  if (r->save_restore == 0)
> +    string_appendf (*m_buffer, " save-restore=\"no\"");
> +
> +  string_appendf (*m_buffer, "/>\n");
> +}
> +
> +void print_xml_feature::visit_pre (const target_desc *e)
> +{
> +#ifndef IN_PROCESS_AGENT
> +  string_appendf (*m_buffer, "<?xml version=\"1.0\"?>\n");
> +  string_appendf (*m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
> +  string_appendf (*m_buffer, "<target>\n<architecture>%s</architecture>\n",
> +		  tdesc_architecture_name (e));
> +
> +  const char *osabi = tdesc_osabi_name (e);
> +  if (osabi != nullptr)
> +    string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
> +#endif
> +}
> +
> +void print_xml_feature::visit_post (const target_desc *e)
> +{
> +  string_appendf (*m_buffer, "</target>\n");
> +}
> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
> index 311341da0d..00eaada072 100644
> --- a/gdb/common/tdesc.h
> +++ b/gdb/common/tdesc.h
> @@ -381,4 +381,33 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
>  		       int regnum, int save_restore, const char *group,
>  		       int bitsize, const char *type);
>  
> +/* Return the tdesc in string XML format.  */
> +
> +const char *tdesc_get_features_xml (const target_desc *tdesc);
> +
> +/* Print target description as xml.  */
> +
> +class print_xml_feature : public tdesc_element_visitor
> +{
> +public:
> +  print_xml_feature (std::string *buffer_)
> +    : m_buffer (buffer_)
> +  {}
> +
> +  ~print_xml_feature ()
> +  {}

You can remove the explicit destructor.

> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 197fb59127..2152b17947 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -27,7 +27,7 @@
>  /* A target description.  Inherit from tdesc_feature so that target_desc
>     can be used as tdesc_feature.  */
>  
> -struct target_desc
> +struct target_desc : tdesc_element
>  {
>    /* A vector of elements of register definitions that
>       describe the inferior's register set.  */
> @@ -51,7 +51,7 @@ struct target_desc
>  
>       It can be NULL, then, its content is got from the following three
>       fields features, arch, and osabi in tdesc_get_features_xml.  */
> -  const char *xmltarget = NULL;
> +  mutable const char *xmltarget = NULL;

I think the comment above might need a bit of updating.

>  
>    /* The value of <architecture> element in the XML, replying GDB.  */
>    const char *arch = NULL;
> @@ -73,6 +73,8 @@ public:
>      return !(*this == other);
>    }
>  #endif
> +
> +  void accept (tdesc_element_visitor &v) const;

override

>  };
>  
>  /* Copy target description SRC to DEST.  */
> @@ -89,8 +91,4 @@ void init_target_desc (struct target_desc *tdesc);
>  
>  const struct target_desc *current_target_desc (void);
>  
> -#ifndef IN_PROCESS_AGENT
> -const char *tdesc_get_features_xml (struct target_desc *tdesc);
> -#endif
> -
>  #endif /* TDESC_H */
> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index 18108d77eb..8f546fe276 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -163,7 +163,9 @@ done
>  
>  echo
>  echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
> -if test "${xmltarget}" = x; then
> +if test "${feature}" != x; then
> +  echo "static const char *xmltarget_${name} = 0;"
> +elif test "${xmltarget}" = x; then
>    if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
>      echo "static const char *xmltarget_${name} = 0;"
>    else
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index da2c1ce345..b96ef961a4 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -333,6 +333,8 @@ struct target_desc : tdesc_element
>    /* The features associated with this target.  */
>    std::vector<tdesc_feature_up> features;
>  
> +  mutable char *xmltarget = nullptr;

Can you add a comment for this member?  Just to mention that this
is used for caching/memoization of the generated xml.

> +
>    void accept (tdesc_element_visitor &v) const override
>    {
>      v.visit_pre (this);
> @@ -1667,6 +1669,21 @@ private:
>    int m_next_regnum = 0;
>  };
>  
> +/* See common/tdesc.h.  */
> +
> +const char *
> +tdesc_get_features_xml (const target_desc *tdesc)
> +{
> +  if (tdesc->xmltarget == nullptr)
> +    {
> +      std::string buffer ("@");
> +      print_xml_feature v (&buffer);
> +      tdesc->accept (v);
> +      tdesc->xmltarget = xstrdup (buffer.c_str ());
> +    }
> +  return tdesc->xmltarget;
> +}
> +
>  static void
>  maint_print_c_tdesc_cmd (const char *args, int from_tty)
>  {
> @@ -1739,6 +1756,39 @@ record_xml_tdesc (const char *xml_file, const struct target_desc *tdesc)
>  
>  }
>  
> +/* Test the convesion process of a target description to/from xml: Take a target
> +   description TDESC, convert to xml, back to a description, and confirm the new
> +   tdesc is identical to the original.  */
> +static bool
> +maintenance_check_tdesc_xml_convert (const target_desc *tdesc, const char *name)
> +{
> +  const char *xml = tdesc_get_features_xml (tdesc);
> +
> +  if (xml == nullptr && *xml != '@')

This condition is wrong.  You probably meant

  if (xml == nullptr || *xml != '@')

?

> +    {
> +      printf_filtered (_("Could not convert description for %s to xml.\n"),
> +		       name);
> +      return false;
> +    }
> +
> +  const target_desc *tdesc_trans = string_read_description_xml (xml + 1);
> +
> +  if (tdesc_trans == nullptr)
> +    {
> +      printf_filtered (_("Could not convert description for %s from xml.\n"),
> +		       name);
> +      return false;
> +    }
> +  else if (*tdesc != *tdesc_trans)
> +    {
> +      printf_filtered (_("Converted description for %s does not match.\n"),
> +		       name);
> +      return false;
> +    }
> +  return true;
> +}
> +
> +
>  /* Check that the target descriptions created dynamically by
>     architecture-specific code equal the descriptions created from XML files
>     found in the specified directory DIR.  */
> @@ -1760,6 +1810,12 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
>  	= file_read_description_xml (tdesc_xml.data ());
>  
>        if (tdesc == NULL || *tdesc != *e.second)
> +	{
> +	  printf_filtered ( _("Descriptions for %s do not match.\n"), e.first);
> +	  failed++;
> +	}
> +      else if (!maintenance_check_tdesc_xml_convert (tdesc, e.first)
> +	       || !maintenance_check_tdesc_xml_convert (e.second, e.first))
>  	failed++;
>      }
>    printf_filtered (_("Tested %lu XML files, %d failed\n"),
> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 9190d5f3c6..5c6ee4c8cd 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -752,3 +752,14 @@ target_fetch_description_xml (struct target_ops *ops)
>    return output;
>  #endif
>  }
> +
> +/* See xml-tdesc.h.  */
> +
> +const struct target_desc *
> +string_read_description_xml (const char *xml_str)
> +{
> +  return tdesc_parse_xml (xml_str, [] (const char *href, void *baton) {
> +     error (_("xincludes are unsupported with this method"));
> +     return gdb::unique_xmalloc_ptr<char> ();
> +     }, nullptr);

You'll need to change this to

  return gdb::optional<gdb::char_vector> ();

because of a recent change upstream.  Also, this indentation would follow a bit
more what we do:

  return tdesc_parse_xml (xml_str, [] (const char *href, void *baton)
    {
      error (_("xincludes are unsupported with this method"));
      return gdb::optional<gdb::char_vector> ();
    }, nullptr);

The idea is to format the lambda the same way we would format an if or for body.

> +}
> \ No newline at end of file
> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
> index 8f0679707a..e39d5580bb 100644
> --- a/gdb/xml-tdesc.h
> +++ b/gdb/xml-tdesc.h
> @@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
>     otherwise.  */
>  gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
>  
> +/* Take an xml string, parse it, and return the parsed description.  Does not
> +   handle a string containing includes.  */
> +
> +const struct target_desc *string_read_description_xml (const char *);

Can you give a name to the parameter?  "xml" would be fine.

Thanks,

Simon

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

* Re: [PATCH v5 0/8] Remove gdbserver dependency on xml files
  2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (7 preceding siblings ...)
  2018-04-10 14:34 ` [PATCH v5 8/8] Remove xml files from gdbserver Alan Hayward
@ 2018-04-18  2:49 ` Simon Marchi
  8 siblings, 0 replies; 16+ messages in thread
From: Simon Marchi @ 2018-04-18  2:49 UTC (permalink / raw)
  To: Alan Hayward, gdb-patches; +Cc: nd

On 2018-04-10 10:33 AM, Alan Hayward wrote:
> V5 addresses Simon's review comments, mostly in the first two patches and the
> "Create xml..." patch. Added changes since previous verion in the patch
> headers. Series is slightly shorted as some early patches have been committed.
> 
> Summary:
> 
> For those targets that use new style target descriptions (also known as
> flexible target descriptions), this set of patches removes the dependency on
> xml files. Namely:
> * Removes inclusion of xml files within gdbserver.
> * Removes the requirement for the .c files in features/ to be generated from
> cached xml files.
> This is made possible by changing xml descriptions generated by gdbserver, so
> that instead of including xml file names, gdbserver now generate a complete
> xml description.
> 
> The second point will be required for aarch64 SVE support, where the register
> size are variable. Creating SVE xml files for every possible vector length
> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
> features/ .c code that would normally be generated from xml.
> 
> Targets which use the older style target descriptions have not been changed.
> 
> 
> XML Generation:
> 
> In existing code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, the
> function tdesc_get_features_xml () creates an xml containing the name of the
> original xml file(s). For example:
> 
> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> <target>
>   <architecture>i386</architecture>
>   <osabi>GNU/Linux</osabi>
>   <xi:include href="32bit-core.xml"/>
>   <xi:include href="32bit-sse.xml"/>
>   <xi:include href="32bit-linux.xml"/>
>   <xi:include href="32bit-avx.xml"/>
> </target>
> 
> Upon receipt, GDB then makes requests to gdbserver for the contents of the
> xml files. Gdbserver keeps full copies all the xml files inside the binary.
> 
> This patch series adds common code that allows gdbserver (and gdb) to turn
> a C target description structure into xml.
> Now when asked fort an xml description to gdb, gdbserver turns the entire
> target description structure back into xml, without using any cached files.
> Producing, for example:
> 
> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> <target>
>   <architecture>i386</architecture>
>   <osabi>GNU/Linux</osabi>
>   <feature name="org.gnu.gdb.i386.core">
>     <flags id="i386_eflags" size="4">
>       <field name="CF" start="0" end="0"/>
>       <field name="" start="1" end="1"/>
>       <field name="PF" start="2" end="2"/>
>       <field name="AF" start="4" end="4"/>
> ...etc...
> 
> 
> Patch Contents:
> 
> Patches 1-3 commonise the various target descriptor functionality, allowing
> gdbserver to parse target descriptions in the same way as gdb. This series
> does not commonise target_desc, but this is hopefully a long term goal.
> 
> The sixth patch adds the xml printer, which iterates through the parsing
> generated in the previous patches.
> 
> The other patches are clean up patches.
> 
> Patches have been tested on a make check on x86 targets=all build with
> target boards unix and native-gdbserver. Also built and tested aarch64 and
> Arm32 (which uses old style descriptions)
> In addition, new test cases are added to the unit tests.

Hi Alan,

Thanks a lot for this writeup, it was very useful to get back in the subject,
since a few weeks is enough for me to forget everything I learned last time :)
Seriously, it helps a lot, and I'm sure it will be a good reference in the
future.

I replied to some patches, those I have not replied to LGTM.  I think that with
those little fixes, this series is ready to go in (as far as my target description
knowledge goes).

Simon

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

* Re: [PATCH v5 1/8] Commonise tdesc_reg
  2018-04-18  1:57   ` Simon Marchi
@ 2018-04-18  9:03     ` Alan Hayward
  2018-04-18 13:54       ` Simon Marchi
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Hayward @ 2018-04-18  9:03 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, nd



> On 18 Apr 2018, at 02:57, Simon Marchi <simark@simark.ca> wrote:
> 
> On 2018-04-10 10:33 AM, Alan Hayward wrote:
>> This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
>> 
>> gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
>> a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.
>> 
>> However, other modules in gdbserver directly access the reg_defs structure.
>> To work around this, init_target_desc fills in reg_defs by parsing the
>> tdesc_reg vector.
>> The long term goal is to remove reg_defs, replacing with accessor funcs.
>> 
>> I wanted to make tdesc_create_reg common, but I cannot do that until
>> the next patch.
>> 
>> I also had to commonise tdesc_element_visitor and tdesc_element.
>> 
>> This patch only differs from the V4 version in init_target_desc() and
>> the changing of constructors for regdef.
> 
> Hi Alan,
> 
> Just two small comment, but the patch LGTM with those answered or addressed.
> 
>> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
>> index 85139d948c..8eb88eedce 100644
>> --- a/gdb/gdbserver/tdesc.h
>> +++ b/gdb/gdbserver/tdesc.h
>> @@ -25,7 +25,10 @@
>> #include <vector>
>> 
>> struct tdesc_feature
>> -{};
>> +{
>> +  /* The registers associated with this feature.  */
>> +  std::vector<tdesc_reg_up> registers;
>> +};
>> 
>> /* A target description.  Inherit from tdesc_feature so that target_desc
>>    can be used as tdesc_feature.  */
>> diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
>> index 4775e863e9..7c80d45d48 100644
>> --- a/gdb/regformats/regdef.h
>> +++ b/gdb/regformats/regdef.h
>> @@ -21,15 +21,15 @@
>> 
>> struct reg
>> {
>> -  reg ()
>> +  reg (int _offset)
>>     : name (""),
>> -      offset (0),
>> +      offset (_offset),
>>       size (0)
>>   {}
> 
> If this constructor is only used for padding entries, shouldn't name be
> NULL, as the documentation for the field states?
> 

That creates two issues:

The reg:operator== segfaults in the strcmp due to the null.
Easily fixable, but a little ugly.

With that patched, the gdbserver unit tests will fail.
That is because the creation of target descriptions in the  -generated.c files
in the build dir create the gaps using:
tdesc_create_reg (feature, “", 0, 0, NULL, 0, NULL);

Would fixing that cause more issues? Not sure.

Having “” rather than 0 does make sense when you format these to xml files.
We are explicitly setting to a blank string. Whereas null is more for uninitialised.

I was going to suggest fixing in a follow on patch, but the more I think about it,
The more I’m thinking “” is correct.

> Also, did you notice something failing if the padding entries don't have
> the offset field to the "current" offset at the time they are created?
> If we could leave them at 0, I think it would keep things simpler.  I
> stopped for a few seconds, wondering why init_target_desc did:
> 
>  tdesc->reg_defs.resize (regnum, reg (offset));
> 
> and not just:
> 
>  tdesc->reg_defs.resize (regnum);

Again, this creates unit test failures due to the same tdesc_create_reg above.
Here offset is a little more explicit, and setting to 0 says it’s at the start of the
description, which isn’t right.

Thanks again for the reviews.

Alan.


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

* Re: [PATCH v5 1/8] Commonise tdesc_reg
  2018-04-18  9:03     ` Alan Hayward
@ 2018-04-18 13:54       ` Simon Marchi
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Marchi @ 2018-04-18 13:54 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

On 2018-04-18 05:03, Alan Hayward wrote:
>> On 18 Apr 2018, at 02:57, Simon Marchi <simark@simark.ca> wrote:
>> 
>> On 2018-04-10 10:33 AM, Alan Hayward wrote:
>>> This patch commonises tdesc_reg and makes use of it in gdbserver 
>>> tdesc.
>>> 
>>> gdbserver tdesc_create_reg is changed to create a tdesc_reg instead 
>>> of
>>> a reg_defs entry. The vector of tdesc_reg is held inside 
>>> tdesc_feature.
>>> 
>>> However, other modules in gdbserver directly access the reg_defs 
>>> structure.
>>> To work around this, init_target_desc fills in reg_defs by parsing 
>>> the
>>> tdesc_reg vector.
>>> The long term goal is to remove reg_defs, replacing with accessor 
>>> funcs.
>>> 
>>> I wanted to make tdesc_create_reg common, but I cannot do that until
>>> the next patch.
>>> 
>>> I also had to commonise tdesc_element_visitor and tdesc_element.
>>> 
>>> This patch only differs from the V4 version in init_target_desc() and
>>> the changing of constructors for regdef.
>> 
>> Hi Alan,
>> 
>> Just two small comment, but the patch LGTM with those answered or 
>> addressed.
>> 
>>> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
>>> index 85139d948c..8eb88eedce 100644
>>> --- a/gdb/gdbserver/tdesc.h
>>> +++ b/gdb/gdbserver/tdesc.h
>>> @@ -25,7 +25,10 @@
>>> #include <vector>
>>> 
>>> struct tdesc_feature
>>> -{};
>>> +{
>>> +  /* The registers associated with this feature.  */
>>> +  std::vector<tdesc_reg_up> registers;
>>> +};
>>> 
>>> /* A target description.  Inherit from tdesc_feature so that 
>>> target_desc
>>>    can be used as tdesc_feature.  */
>>> diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
>>> index 4775e863e9..7c80d45d48 100644
>>> --- a/gdb/regformats/regdef.h
>>> +++ b/gdb/regformats/regdef.h
>>> @@ -21,15 +21,15 @@
>>> 
>>> struct reg
>>> {
>>> -  reg ()
>>> +  reg (int _offset)
>>>     : name (""),
>>> -      offset (0),
>>> +      offset (_offset),
>>>       size (0)
>>>   {}
>> 
>> If this constructor is only used for padding entries, shouldn't name 
>> be
>> NULL, as the documentation for the field states?
>> 
> 
> That creates two issues:
> 
> The reg:operator== segfaults in the strcmp due to the null.
> Easily fixable, but a little ugly.
> 
> With that patched, the gdbserver unit tests will fail.
> That is because the creation of target descriptions in the  
> -generated.c files
> in the build dir create the gaps using:
> tdesc_create_reg (feature, “", 0, 0, NULL, 0, NULL);
> 
> Would fixing that cause more issues? Not sure.
> 
> Having “” rather than 0 does make sense when you format these to xml 
> files.
> We are explicitly setting to a blank string. Whereas null is more for
> uninitialised.
> 
> I was going to suggest fixing in a follow on patch, but the more I
> think about it,
> The more I’m thinking “” is correct.

The important thing is that documentation is accurate, so could you 
adjust the doc of the name field accordingly?

>> Also, did you notice something failing if the padding entries don't 
>> have
>> the offset field to the "current" offset at the time they are created?
>> If we could leave them at 0, I think it would keep things simpler.  I
>> stopped for a few seconds, wondering why init_target_desc did:
>> 
>>  tdesc->reg_defs.resize (regnum, reg (offset));
>> 
>> and not just:
>> 
>>  tdesc->reg_defs.resize (regnum);
> 
> Again, this creates unit test failures due to the same tdesc_create_reg 
> above.
> Here offset is a little more explicit, and setting to 0 says it’s at
> the start of the
> description, which isn’t right.

Ok.

Thanks,

Simon

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

* Re: [PATCH v5 6/8] Create xml from target descriptions
  2018-04-18  2:43   ` Simon Marchi
@ 2018-04-18 21:26     ` Alan Hayward
  0 siblings, 0 replies; 16+ messages in thread
From: Alan Hayward @ 2018-04-18 21:26 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, nd



> On 18 Apr 2018, at 03:43, Simon Marchi <simark@simark.ca> wrote:
> 
> Hi Alan,
> 
> The patch looks good to me.  I noted a few formatting/minor comments.

Thanks!
Updated as requested and pushed. New version included at the bottom for reference.


> 
>> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
>> index b9e9ddb3fa..b4bd7bfb00 100644
>> --- a/gdb/common/tdesc.c
>> +++ b/gdb/common/tdesc.c
>> @@ -290,3 +290,111 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
>> 			     tdesc_predefined_type (TDESC_TYPE_INT32),
>> 			     value, -1);
>> }
>> +
>> +void print_xml_feature::visit_pre (const tdesc_feature *e)
>> +{
>> +  string_appendf (*m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
>> +}
>> +
>> +void print_xml_feature::visit_post (const tdesc_feature *e)
>> +{
>> +  string_appendf (*m_buffer, "</feature>\n");
>> +}
>> +
>> +void print_xml_feature::visit (const tdesc_type_builtin *t)
>> +{
>> +  error (_("xml output is not supported type \"%s\"."), t->name.c_str ());
> 
> Just a nit, but it seems to be missing something like a "for" between "supported" and "type".
> 
>> +}
>> +
>> +void print_xml_feature::visit (const tdesc_type_vector *t)
>> +{
>> +  string_appendf (*m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
>> +		  t->name.c_str (), t->element_type->name.c_str (), t->count);
>> +}
>> +
>> +void print_xml_feature::visit (const tdesc_type_with_fields *t)
>> +{
>> +  struct tdesc_type_field *f;
>> +  const static char *types[] = { "struct", "union", "flags", "enum" };
>> +
>> +  gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
>> +
>> +  string_appendf (*m_buffer,
>> +		  "<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
>> +		  t->name.c_str ());
>> +
>> +  switch (t->kind)
>> +    {
>> +    case TDESC_TYPE_STRUCT:
>> +    case TDESC_TYPE_FLAGS:
>> +      if (t->size > 0)
>> +	string_appendf (*m_buffer, " size=\"%d\"", t->size);
>> +      string_appendf (*m_buffer, ">\n");
>> +
>> +      for (const tdesc_type_field &f : t->fields)
>> +	{
>> +	  string_appendf (*m_buffer, "  <field name=\"%s\" ", f.name.c_str ());
>> +	  if (f.start == -1)
>> +	    string_appendf (*m_buffer, "type=\"%s\"/>\n",
>> +			    f.type->name.c_str ());
>> +	  else
>> +	    string_appendf (*m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
>> +			    f.end);
>> +	}
>> +      break;
>> +
>> +    case TDESC_TYPE_ENUM:
>> +      string_appendf (*m_buffer, ">\n");
>> +      for (const tdesc_type_field &f : t->fields)
>> +	string_appendf (*m_buffer, "  <field name=\"%s\" start=\"%d\"/>\n",
>> +			f.name.c_str (), f.start);
>> +      break;
>> +
>> +    case TDESC_TYPE_UNION:
>> +      string_appendf (*m_buffer, ">\n");
>> +      for (const tdesc_type_field &f : t->fields)
>> +	string_appendf (*m_buffer, "  <field name=\"%s\" type=\"%s\"/>\n",
>> +			f.name.c_str (), f.type->name.c_str ());
>> +      break;
>> +
>> +    default:
>> +      error (_("xml output is not supported type \"%s\"."), t->name.c_str ());
> 
> "supported for type" here too.
> 
>> +    }
>> +
>> +  string_appendf (*m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
>> +}
>> +
>> +void print_xml_feature::visit (const tdesc_reg *r)
>> +{
>> +  string_appendf (*m_buffer,
>> +		  "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
>> +		  r->name.c_str (), r->bitsize, r->type.c_str (),
>> +		  r->target_regnum);
>> +
>> +  if (r->group.length () > 0)
>> +    string_appendf (*m_buffer, " group=\"%s\"", r->group.c_str ());
>> +
>> +  if (r->save_restore == 0)
>> +    string_appendf (*m_buffer, " save-restore=\"no\"");
>> +
>> +  string_appendf (*m_buffer, "/>\n");
>> +}
>> +
>> +void print_xml_feature::visit_pre (const target_desc *e)
>> +{
>> +#ifndef IN_PROCESS_AGENT
>> +  string_appendf (*m_buffer, "<?xml version=\"1.0\"?>\n");
>> +  string_appendf (*m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
>> +  string_appendf (*m_buffer, "<target>\n<architecture>%s</architecture>\n",
>> +		  tdesc_architecture_name (e));
>> +
>> +  const char *osabi = tdesc_osabi_name (e);
>> +  if (osabi != nullptr)
>> +    string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
>> +#endif
>> +}
>> +
>> +void print_xml_feature::visit_post (const target_desc *e)
>> +{
>> +  string_appendf (*m_buffer, "</target>\n");
>> +}
>> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
>> index 311341da0d..00eaada072 100644
>> --- a/gdb/common/tdesc.h
>> +++ b/gdb/common/tdesc.h
>> @@ -381,4 +381,33 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
>> 		       int regnum, int save_restore, const char *group,
>> 		       int bitsize, const char *type);
>> 
>> +/* Return the tdesc in string XML format.  */
>> +
>> +const char *tdesc_get_features_xml (const target_desc *tdesc);
>> +
>> +/* Print target description as xml.  */
>> +
>> +class print_xml_feature : public tdesc_element_visitor
>> +{
>> +public:
>> +  print_xml_feature (std::string *buffer_)
>> +    : m_buffer (buffer_)
>> +  {}
>> +
>> +  ~print_xml_feature ()
>> +  {}
> 
> You can remove the explicit destructor.
> 
>> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
>> index 197fb59127..2152b17947 100644
>> --- a/gdb/gdbserver/tdesc.h
>> +++ b/gdb/gdbserver/tdesc.h
>> @@ -27,7 +27,7 @@
>> /* A target description.  Inherit from tdesc_feature so that target_desc
>>    can be used as tdesc_feature.  */
>> 
>> -struct target_desc
>> +struct target_desc : tdesc_element
>> {
>>   /* A vector of elements of register definitions that
>>      describe the inferior's register set.  */
>> @@ -51,7 +51,7 @@ struct target_desc
>> 
>>      It can be NULL, then, its content is got from the following three
>>      fields features, arch, and osabi in tdesc_get_features_xml.  */
>> -  const char *xmltarget = NULL;
>> +  mutable const char *xmltarget = NULL;
> 
> I think the comment above might need a bit of updating.
> 
>> 
>>   /* The value of <architecture> element in the XML, replying GDB.  */
>>   const char *arch = NULL;
>> @@ -73,6 +73,8 @@ public:
>>     return !(*this == other);
>>   }
>> #endif
>> +
>> +  void accept (tdesc_element_visitor &v) const;
> 
> override
> 
>> };
>> 
>> /* Copy target description SRC to DEST.  */
>> @@ -89,8 +91,4 @@ void init_target_desc (struct target_desc *tdesc);
>> 
>> const struct target_desc *current_target_desc (void);
>> 
>> -#ifndef IN_PROCESS_AGENT
>> -const char *tdesc_get_features_xml (struct target_desc *tdesc);
>> -#endif
>> -
>> #endif /* TDESC_H */
>> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
>> index 18108d77eb..8f546fe276 100755
>> --- a/gdb/regformats/regdat.sh
>> +++ b/gdb/regformats/regdat.sh
>> @@ -163,7 +163,9 @@ done
>> 
>> echo
>> echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
>> -if test "${xmltarget}" = x; then
>> +if test "${feature}" != x; then
>> +  echo "static const char *xmltarget_${name} = 0;"
>> +elif test "${xmltarget}" = x; then
>>   if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
>>     echo "static const char *xmltarget_${name} = 0;"
>>   else
>> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
>> index da2c1ce345..b96ef961a4 100644
>> --- a/gdb/target-descriptions.c
>> +++ b/gdb/target-descriptions.c
>> @@ -333,6 +333,8 @@ struct target_desc : tdesc_element
>>   /* The features associated with this target.  */
>>   std::vector<tdesc_feature_up> features;
>> 
>> +  mutable char *xmltarget = nullptr;
> 
> Can you add a comment for this member?  Just to mention that this
> is used for caching/memoization of the generated xml.
> 
>> +
>>   void accept (tdesc_element_visitor &v) const override
>>   {
>>     v.visit_pre (this);
>> @@ -1667,6 +1669,21 @@ private:
>>   int m_next_regnum = 0;
>> };
>> 
>> +/* See common/tdesc.h.  */
>> +
>> +const char *
>> +tdesc_get_features_xml (const target_desc *tdesc)
>> +{
>> +  if (tdesc->xmltarget == nullptr)
>> +    {
>> +      std::string buffer ("@");
>> +      print_xml_feature v (&buffer);
>> +      tdesc->accept (v);
>> +      tdesc->xmltarget = xstrdup (buffer.c_str ());
>> +    }
>> +  return tdesc->xmltarget;
>> +}
>> +
>> static void
>> maint_print_c_tdesc_cmd (const char *args, int from_tty)
>> {
>> @@ -1739,6 +1756,39 @@ record_xml_tdesc (const char *xml_file, const struct target_desc *tdesc)
>> 
>> }
>> 
>> +/* Test the convesion process of a target description to/from xml: Take a target
>> +   description TDESC, convert to xml, back to a description, and confirm the new
>> +   tdesc is identical to the original.  */
>> +static bool
>> +maintenance_check_tdesc_xml_convert (const target_desc *tdesc, const char *name)
>> +{
>> +  const char *xml = tdesc_get_features_xml (tdesc);
>> +
>> +  if (xml == nullptr && *xml != '@')
> 
> This condition is wrong.  You probably meant
> 
>  if (xml == nullptr || *xml != '@')
> 
> ?
> 
>> +    {
>> +      printf_filtered (_("Could not convert description for %s to xml.\n"),
>> +		       name);
>> +      return false;
>> +    }
>> +
>> +  const target_desc *tdesc_trans = string_read_description_xml (xml + 1);
>> +
>> +  if (tdesc_trans == nullptr)
>> +    {
>> +      printf_filtered (_("Could not convert description for %s from xml.\n"),
>> +		       name);
>> +      return false;
>> +    }
>> +  else if (*tdesc != *tdesc_trans)
>> +    {
>> +      printf_filtered (_("Converted description for %s does not match.\n"),
>> +		       name);
>> +      return false;
>> +    }
>> +  return true;
>> +}
>> +
>> +
>> /* Check that the target descriptions created dynamically by
>>    architecture-specific code equal the descriptions created from XML files
>>    found in the specified directory DIR.  */
>> @@ -1760,6 +1810,12 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
>> 	= file_read_description_xml (tdesc_xml.data ());
>> 
>>       if (tdesc == NULL || *tdesc != *e.second)
>> +	{
>> +	  printf_filtered ( _("Descriptions for %s do not match.\n"), e.first);
>> +	  failed++;
>> +	}
>> +      else if (!maintenance_check_tdesc_xml_convert (tdesc, e.first)
>> +	       || !maintenance_check_tdesc_xml_convert (e.second, e.first))
>> 	failed++;
>>     }
>>   printf_filtered (_("Tested %lu XML files, %d failed\n"),
>> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
>> index 9190d5f3c6..5c6ee4c8cd 100644
>> --- a/gdb/xml-tdesc.c
>> +++ b/gdb/xml-tdesc.c
>> @@ -752,3 +752,14 @@ target_fetch_description_xml (struct target_ops *ops)
>>   return output;
>> #endif
>> }
>> +
>> +/* See xml-tdesc.h.  */
>> +
>> +const struct target_desc *
>> +string_read_description_xml (const char *xml_str)
>> +{
>> +  return tdesc_parse_xml (xml_str, [] (const char *href, void *baton) {
>> +     error (_("xincludes are unsupported with this method"));
>> +     return gdb::unique_xmalloc_ptr<char> ();
>> +     }, nullptr);
> 
> You'll need to change this to
> 
>  return gdb::optional<gdb::char_vector> ();
> 
> because of a recent change upstream.  Also, this indentation would follow a bit
> more what we do:
> 
>  return tdesc_parse_xml (xml_str, [] (const char *href, void *baton)
>    {
>      error (_("xincludes are unsupported with this method"));
>      return gdb::optional<gdb::char_vector> ();
>    }, nullptr);
> 
> The idea is to format the lambda the same way we would format an if or for body.
> 
>> +}
>> \ No newline at end of file
>> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
>> index 8f0679707a..e39d5580bb 100644
>> --- a/gdb/xml-tdesc.h
>> +++ b/gdb/xml-tdesc.h
>> @@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
>>    otherwise.  */
>> gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
>> 
>> +/* Take an xml string, parse it, and return the parsed description.  Does not
>> +   handle a string containing includes.  */
>> +
>> +const struct target_desc *string_read_description_xml (const char *);
> 
> Can you give a name to the parameter?  "xml" would be fine.
> 
> Thanks,
> 
> Simon

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index b9e9ddb3fa..68584a7f94 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -290,3 +290,112 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
 			     tdesc_predefined_type (TDESC_TYPE_INT32),
 			     value, -1);
 }
+
+void print_xml_feature::visit_pre (const tdesc_feature *e)
+{
+  string_appendf (*m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
+}
+
+void print_xml_feature::visit_post (const tdesc_feature *e)
+{
+  string_appendf (*m_buffer, "</feature>\n");
+}
+
+void print_xml_feature::visit (const tdesc_type_builtin *t)
+{
+  error (_("xml output is not supported for type \"%s\"."), t->name.c_str ());
+}
+
+void print_xml_feature::visit (const tdesc_type_vector *t)
+{
+  string_appendf (*m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
+		  t->name.c_str (), t->element_type->name.c_str (), t->count);
+}
+
+void print_xml_feature::visit (const tdesc_type_with_fields *t)
+{
+  struct tdesc_type_field *f;
+  const static char *types[] = { "struct", "union", "flags", "enum" };
+
+  gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
+
+  string_appendf (*m_buffer,
+		  "<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
+		  t->name.c_str ());
+
+  switch (t->kind)
+    {
+    case TDESC_TYPE_STRUCT:
+    case TDESC_TYPE_FLAGS:
+      if (t->size > 0)
+	string_appendf (*m_buffer, " size=\"%d\"", t->size);
+      string_appendf (*m_buffer, ">\n");
+
+      for (const tdesc_type_field &f : t->fields)
+	{
+	  string_appendf (*m_buffer, "  <field name=\"%s\" ", f.name.c_str ());
+	  if (f.start == -1)
+	    string_appendf (*m_buffer, "type=\"%s\"/>\n",
+			    f.type->name.c_str ());
+	  else
+	    string_appendf (*m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
+			    f.end);
+	}
+      break;
+
+    case TDESC_TYPE_ENUM:
+      string_appendf (*m_buffer, ">\n");
+      for (const tdesc_type_field &f : t->fields)
+	string_appendf (*m_buffer, "  <field name=\"%s\" start=\"%d\"/>\n",
+			f.name.c_str (), f.start);
+      break;
+
+    case TDESC_TYPE_UNION:
+      string_appendf (*m_buffer, ">\n");
+      for (const tdesc_type_field &f : t->fields)
+	string_appendf (*m_buffer, "  <field name=\"%s\" type=\"%s\"/>\n",
+			f.name.c_str (), f.type->name.c_str ());
+      break;
+
+    default:
+      error (_("xml output is not supported for type \"%s\"."),
+	     t->name.c_str ());
+    }
+
+  string_appendf (*m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
+}
+
+void print_xml_feature::visit (const tdesc_reg *r)
+{
+  string_appendf (*m_buffer,
+		  "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
+		  r->name.c_str (), r->bitsize, r->type.c_str (),
+		  r->target_regnum);
+
+  if (r->group.length () > 0)
+    string_appendf (*m_buffer, " group=\"%s\"", r->group.c_str ());
+
+  if (r->save_restore == 0)
+    string_appendf (*m_buffer, " save-restore=\"no\"");
+
+  string_appendf (*m_buffer, "/>\n");
+}
+
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+#ifndef IN_PROCESS_AGENT
+  string_appendf (*m_buffer, "<?xml version=\"1.0\"?>\n");
+  string_appendf (*m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
+  string_appendf (*m_buffer, "<target>\n<architecture>%s</architecture>\n",
+		  tdesc_architecture_name (e));
+
+  const char *osabi = tdesc_osabi_name (e);
+  if (osabi != nullptr)
+    string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
+#endif
+}
+
+void print_xml_feature::visit_post (const target_desc *e)
+{
+  string_appendf (*m_buffer, "</target>\n");
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 8b826ec436..6868bf47d9 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -380,4 +380,30 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		       int regnum, int save_restore, const char *group,
 		       int bitsize, const char *type);

+/* Return the tdesc in string XML format.  */
+
+const char *tdesc_get_features_xml (const target_desc *tdesc);
+
+/* Print target description as xml.  */
+
+class print_xml_feature : public tdesc_element_visitor
+{
+public:
+  print_xml_feature (std::string *buffer_)
+    : m_buffer (buffer_)
+  {}
+
+  void visit_pre (const target_desc *e) override;
+  void visit_post (const target_desc *e) override;
+  void visit_pre (const tdesc_feature *e) override;
+  void visit_post (const tdesc_feature *e) override;
+  void visit (const tdesc_type_builtin *type) override;
+  void visit (const tdesc_type_vector *type) override;
+  void visit (const tdesc_type_with_fields *type) override;
+  void visit (const tdesc_reg *reg) override;
+
+private:
+  std::string *m_buffer;
+};
+
 #endif /* ARCH_TDESC_H */
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 64c72bdd58..5027df5e10 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -940,7 +940,7 @@ get_features_xml (const char *annex)

   if (strcmp (annex, "target.xml") == 0)
     {
-      const char *ret = tdesc_get_features_xml ((target_desc*) desc);
+      const char *ret = tdesc_get_features_xml (desc);

       if (*ret == '@')
 	return ret + 1;
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 7603a90a59..126589f3e3 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -47,6 +47,18 @@ bool target_desc::operator== (const target_desc &other) const

 #endif

+void target_desc::accept (tdesc_element_visitor &v) const
+{
+#ifndef IN_PROCESS_AGENT
+  v.visit_pre (this);
+
+  for (const tdesc_feature_up &feature : features)
+    feature->accept (v);
+
+  v.visit_post (this);
+#endif
+}
+
 void
 init_target_desc (struct target_desc *tdesc)
 {
@@ -138,11 +150,10 @@ set_tdesc_osabi (struct target_desc *target_desc, const char *name)
   target_desc->osabi = xstrdup (name);
 }

-/* Return a string which is of XML format, including XML target
-   description to be sent to GDB.  */
+/* See common/tdesc.h.  */

 const char *
-tdesc_get_features_xml (target_desc *tdesc)
+tdesc_get_features_xml (const target_desc *tdesc)
 {
   /* Either .xmltarget or .features is not NULL.  */
   gdb_assert (tdesc->xmltarget != NULL
@@ -151,31 +162,9 @@ tdesc_get_features_xml (target_desc *tdesc)

   if (tdesc->xmltarget == NULL)
     {
-      std::string buffer ("@<?xml version=\"1.0\"?>");
-
-      buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
-      buffer += "<target>";
-      buffer += "<architecture>";
-      buffer += tdesc_architecture_name (tdesc);
-      buffer += "</architecture>";
-
-      const char *osabi = tdesc_osabi_name (tdesc);
-      if (osabi != nullptr)
-	{
-	  buffer += "<osabi>";
-	  buffer += osabi;
-	  buffer += "</osabi>";
-	}
-
-      for (const tdesc_feature_up &feature : tdesc->features)
-	{
-	  buffer += "<xi:include href=\"";
-	  buffer += feature->name;
-	  buffer += "\"/>";
-	}
-
-      buffer += "</target>";
-
+      std::string buffer ("@");
+      print_xml_feature v (&buffer);
+      tdesc->accept (v);
       tdesc->xmltarget = xstrdup (buffer.c_str ());
     }

diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 197fb59127..61a3e4ecdd 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -27,7 +27,7 @@
 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */

-struct target_desc
+struct target_desc : tdesc_element
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
@@ -49,9 +49,9 @@ struct target_desc
      verbatim XML code (prefixed with a '@') or else the name of the
      actual XML file to be used in place of "target.xml".

-     It can be NULL, then, its content is got from the following three
-     fields features, arch, and osabi in tdesc_get_features_xml.  */
-  const char *xmltarget = NULL;
+     If NULL then its content will be generated by parsing the target
+     description into xml.  */
+  mutable const char *xmltarget = NULL;

   /* The value of <architecture> element in the XML, replying GDB.  */
   const char *arch = NULL;
@@ -73,6 +73,8 @@ public:
     return !(*this == other);
   }
 #endif
+
+  void accept (tdesc_element_visitor &v) const override;
 };

 /* Copy target description SRC to DEST.  */
@@ -89,8 +91,4 @@ void init_target_desc (struct target_desc *tdesc);

 const struct target_desc *current_target_desc (void);

-#ifndef IN_PROCESS_AGENT
-const char *tdesc_get_features_xml (struct target_desc *tdesc);
-#endif
-
 #endif /* TDESC_H */
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 18108d77eb..8f546fe276 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -163,7 +163,9 @@ done

 echo
 echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
-if test "${xmltarget}" = x; then
+if test "${feature}" != x; then
+  echo "static const char *xmltarget_${name} = 0;"
+elif test "${xmltarget}" = x; then
   if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
     echo "static const char *xmltarget_${name} = 0;"
   else
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index da2c1ce345..c0f85112b1 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -333,6 +333,9 @@ struct target_desc : tdesc_element
   /* The features associated with this target.  */
   std::vector<tdesc_feature_up> features;

+  /* Used to cache the generated xml version of the target description.  */
+  mutable char *xmltarget = nullptr;
+
   void accept (tdesc_element_visitor &v) const override
   {
     v.visit_pre (this);
@@ -1667,6 +1670,21 @@ private:
   int m_next_regnum = 0;
 };

+/* See common/tdesc.h.  */
+
+const char *
+tdesc_get_features_xml (const target_desc *tdesc)
+{
+  if (tdesc->xmltarget == nullptr)
+    {
+      std::string buffer ("@");
+      print_xml_feature v (&buffer);
+      tdesc->accept (v);
+      tdesc->xmltarget = xstrdup (buffer.c_str ());
+    }
+  return tdesc->xmltarget;
+}
+
 static void
 maint_print_c_tdesc_cmd (const char *args, int from_tty)
 {
@@ -1739,6 +1757,39 @@ record_xml_tdesc (const char *xml_file, const struct target_desc *tdesc)

 }

+/* Test the convesion process of a target description to/from xml: Take a target
+   description TDESC, convert to xml, back to a description, and confirm the new
+   tdesc is identical to the original.  */
+static bool
+maintenance_check_tdesc_xml_convert (const target_desc *tdesc, const char *name)
+{
+  const char *xml = tdesc_get_features_xml (tdesc);
+
+  if (xml == nullptr || *xml != '@')
+    {
+      printf_filtered (_("Could not convert description for %s to xml.\n"),
+		       name);
+      return false;
+    }
+
+  const target_desc *tdesc_trans = string_read_description_xml (xml + 1);
+
+  if (tdesc_trans == nullptr)
+    {
+      printf_filtered (_("Could not convert description for %s from xml.\n"),
+		       name);
+      return false;
+    }
+  else if (*tdesc != *tdesc_trans)
+    {
+      printf_filtered (_("Converted description for %s does not match.\n"),
+		       name);
+      return false;
+    }
+  return true;
+}
+
+
 /* Check that the target descriptions created dynamically by
    architecture-specific code equal the descriptions created from XML files
    found in the specified directory DIR.  */
@@ -1760,6 +1811,12 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
 	= file_read_description_xml (tdesc_xml.data ());

       if (tdesc == NULL || *tdesc != *e.second)
+	{
+	  printf_filtered ( _("Descriptions for %s do not match.\n"), e.first);
+	  failed++;
+	}
+      else if (!maintenance_check_tdesc_xml_convert (tdesc, e.first)
+	       || !maintenance_check_tdesc_xml_convert (e.second, e.first))
 	failed++;
     }
   printf_filtered (_("Tested %lu XML files, %d failed\n"),
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 1c3409d53b..87c6be428f 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -752,3 +752,15 @@ target_fetch_description_xml (struct target_ops *ops)
   return output;
 #endif
 }
+
+/* See xml-tdesc.h.  */
+
+const struct target_desc *
+string_read_description_xml (const char *xml)
+{
+  return tdesc_parse_xml (xml, [] (const char *href, void *baton)
+    {
+      error (_("xincludes are unsupported with this method"));
+      return gdb::optional<gdb::char_vector> ();
+    }, nullptr);
+}
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index 8f0679707a..1e3caba745 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
    otherwise.  */
 gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);

+/* Take an xml string, parse it, and return the parsed description.  Does not
+   handle a string containing includes.  */
+
+const struct target_desc *string_read_description_xml (const char *xml);
+
 #endif /* XML_TDESC_H */

--
2.14.3 (Apple Git-98)


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

end of thread, other threads:[~2018-04-18 21:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-10 14:34 [PATCH v5 0/8] Remove gdbserver dependency on xml files Alan Hayward
2018-04-10 14:34 ` [PATCH v5 6/8] Create xml from target descriptions Alan Hayward
2018-04-18  2:43   ` Simon Marchi
2018-04-18 21:26     ` Alan Hayward
2018-04-10 14:34 ` [PATCH v5 2/8] Commonise tdesc_feature Alan Hayward
2018-04-10 14:34 ` [PATCH v5 4/8] Add tdesc osabi and architecture functions Alan Hayward
2018-04-18  2:10   ` Simon Marchi
2018-04-10 14:34 ` [PATCH v5 7/8] Remove xml file references from target descriptions Alan Hayward
2018-04-10 14:34 ` [PATCH v5 5/8] Add feature reference in .dat files Alan Hayward
2018-04-10 14:34 ` [PATCH v5 3/8] Commonise tdesc types Alan Hayward
2018-04-10 14:34 ` [PATCH v5 1/8] Commonise tdesc_reg Alan Hayward
2018-04-18  1:57   ` Simon Marchi
2018-04-18  9:03     ` Alan Hayward
2018-04-18 13:54       ` Simon Marchi
2018-04-10 14:34 ` [PATCH v5 8/8] Remove xml files from gdbserver Alan Hayward
2018-04-18  2:49 ` [PATCH v5 0/8] Remove gdbserver dependency on xml files Simon Marchi

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).