Patchwork [v3,3/8] Commonise tdesc_feature

login
register
mail settings
Submitter Alan Hayward
Date March 1, 2018, 11:40 a.m.
Message ID <CC765700-BA04-443C-A278-60D4C20BB6C7@arm.com>
Download mbox | patch
Permalink /patch/26130/
State New
Headers show

Comments

Alan Hayward - March 1, 2018, 11:40 a.m.
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.

Alan.

2018-03-01  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.
Philipp Rudo - March 12, 2018, 5:20 p.m.
On Thu, 1 Mar 2018 11:40:00 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

> 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.
> 
> Alan.
> 
> 2018-03-01  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.
> 
> 
> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
> index ac6dfdf7bb4ec2c3bbbed1b091f04308dd741f21..07083e3d1d41c4f86f85ae032b45c1d6f6a105ff 100644
> --- a/gdb/common/tdesc.h
> +++ b/gdb/common/tdesc.h
> @@ -31,6 +31,7 @@ struct tdesc_type_vector;
>  struct tdesc_type_with_fields;
>  struct tdesc_reg;
>  struct target_desc;
> +struct type;

This forward declaration looks unnecessary to me.

Thanks
Philipp


> 
>  /* The interface to visit different elements of target description.  */
> 
> @@ -137,6 +138,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/common/tdesc.c b/gdb/common/tdesc.c
> index a5f2de4b5ba2adb61dd098d8fc569a6339164586..ef6ed944df20d4613dd0dd338710a4c1ad67bbf2 100644
> --- a/gdb/common/tdesc.c
> +++ b/gdb/common/tdesc.c
> @@ -38,3 +38,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/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index b0b8fa3f9e63590398d0de52359ae8ee0271557b..8534069d616b97dc82df04939a4ac935aa4e1cad 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.  */
> -  VEC (char_ptr) *features = NULL;
> -
>    /* The value of <architecture> element in the XML, replying GDB.  */
>    const char *arch = NULL;
> 
> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index 806e87da185623da29d39d9fd74c8722f795e086..72769ff00fd073d93807a4aaa59ec19ec6292641 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -23,19 +23,11 @@
> 
>  target_desc::~target_desc ()
>  {
> -  int i;
> -
>    for (reg *reg : reg_defs)
>      xfree (reg);
> 
>    xfree ((char *) arch);
>    xfree ((char *) osabi);
> -
> -  char *f;
> -
> -  for (i = 0; VEC_iterate (char_ptr, features, i, f); i++)
> -    xfree (f);
> -  VEC_free (char_ptr, features);
>  }
> 
>  bool target_desc::operator== (const target_desc &other) const
> @@ -73,28 +65,29 @@ 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)
> -    {
> -      /* Fill in any blank spaces.  */
> -      while (tdesc->reg_defs.size () < treg->target_regnum)
> -	{
> -	  struct reg *reg = XCNEW (struct reg);
> -	  reg->name = "";
> -	  reg->size = 0;
> -	  reg->offset = offset;
> -	  tdesc->reg_defs.push_back (reg);
> -	}
> -
> -      gdb_assert (treg->target_regnum == 0
> -		  || treg->target_regnum == tdesc->reg_defs.size ());
> -
> -      struct reg *reg = XCNEW (struct reg);
> -      reg->name = treg->name.c_str ();
> -      reg->size = treg->bitsize;
> -      reg->offset = offset;
> -      tdesc->reg_defs.push_back (reg);
> -      offset += reg->size;
> -    }
> +  for (const tdesc_feature_up &feature : tdesc->features)
> +    for (const tdesc_reg_up &treg : feature->registers)
> +      {
> +	/* Fill in any blank spaces.  */
> +	while (tdesc->reg_defs.size () < treg->target_regnum)
> +	  {
> +	    struct reg *reg = XCNEW (struct reg);
> +	    reg->name = "";
> +	    reg->size = 0;
> +	    reg->offset = offset;
> +	    tdesc->reg_defs.push_back (reg);
> +	  }
> +
> +	gdb_assert (treg->target_regnum == 0
> +		    || treg->target_regnum == tdesc->reg_defs.size ());
> +
> +	struct reg *reg = XCNEW (struct reg);
> +	reg->name = treg->name.c_str ();
> +	reg->size = treg->bitsize;
> +	reg->offset = offset;
> +	tdesc->reg_defs.push_back (reg);
> +	offset += reg->size;
> +      }
> 
>    tdesc->registers_size = offset / 8;
> 
> @@ -157,7 +150,7 @@ tdesc_get_features_xml (target_desc *tdesc)
>  {
>    /* Either .xmltarget or .features is not NULL.  */
>    gdb_assert (tdesc->xmltarget != NULL
> -	      || (tdesc->features != NULL
> +	      || (!tdesc->features.empty ()
>  		  && tdesc->arch != NULL));
> 
>    if (tdesc->xmltarget == NULL)
> @@ -177,12 +170,10 @@ tdesc_get_features_xml (target_desc *tdesc)
>  	  buffer += "</osabi>";
>  	}
> 
> -      char *xml;
> -
> -      for (int i = 0; VEC_iterate (char_ptr, tdesc->features, i, xml); i++)
> +      for (const tdesc_feature_up &feature : tdesc->features)
>  	{
>  	  buffer += "<xi:include href=\"";
> -	  buffer += xml;
> +	  buffer += feature->name;
>  	  buffer += "\"/>";
>  	}
> 
> @@ -195,19 +186,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
> -  VEC_safe_push (char_ptr, tdesc->features, xstrdup (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.  */
> @@ -252,21 +240,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/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index ce1627120d9439082709c82c5804724f39477eb1..8c6e191596350fb4e983f8736985d9832f41e2d3 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 3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250..a453eddec0cd97b692c4c9ace3f093c3fbc67422 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);
>  }
> -
> 
> -
> -/* 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.  */
>

Patch

diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index ac6dfdf7bb4ec2c3bbbed1b091f04308dd741f21..07083e3d1d41c4f86f85ae032b45c1d6f6a105ff 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -31,6 +31,7 @@  struct tdesc_type_vector;
 struct tdesc_type_with_fields;
 struct tdesc_reg;
 struct target_desc;
+struct type;

 /* The interface to visit different elements of target description.  */

@@ -137,6 +138,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/common/tdesc.c b/gdb/common/tdesc.c
index a5f2de4b5ba2adb61dd098d8fc569a6339164586..ef6ed944df20d4613dd0dd338710a4c1ad67bbf2 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -38,3 +38,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/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index b0b8fa3f9e63590398d0de52359ae8ee0271557b..8534069d616b97dc82df04939a4ac935aa4e1cad 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.  */
-  VEC (char_ptr) *features = NULL;
-
   /* The value of <architecture> element in the XML, replying GDB.  */
   const char *arch = NULL;

diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 806e87da185623da29d39d9fd74c8722f795e086..72769ff00fd073d93807a4aaa59ec19ec6292641 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -23,19 +23,11 @@ 

 target_desc::~target_desc ()
 {
-  int i;
-
   for (reg *reg : reg_defs)
     xfree (reg);

   xfree ((char *) arch);
   xfree ((char *) osabi);
-
-  char *f;
-
-  for (i = 0; VEC_iterate (char_ptr, features, i, f); i++)
-    xfree (f);
-  VEC_free (char_ptr, features);
 }

 bool target_desc::operator== (const target_desc &other) const
@@ -73,28 +65,29 @@  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)
-    {
-      /* Fill in any blank spaces.  */
-      while (tdesc->reg_defs.size () < treg->target_regnum)
-	{
-	  struct reg *reg = XCNEW (struct reg);
-	  reg->name = "";
-	  reg->size = 0;
-	  reg->offset = offset;
-	  tdesc->reg_defs.push_back (reg);
-	}
-
-      gdb_assert (treg->target_regnum == 0
-		  || treg->target_regnum == tdesc->reg_defs.size ());
-
-      struct reg *reg = XCNEW (struct reg);
-      reg->name = treg->name.c_str ();
-      reg->size = treg->bitsize;
-      reg->offset = offset;
-      tdesc->reg_defs.push_back (reg);
-      offset += reg->size;
-    }
+  for (const tdesc_feature_up &feature : tdesc->features)
+    for (const tdesc_reg_up &treg : feature->registers)
+      {
+	/* Fill in any blank spaces.  */
+	while (tdesc->reg_defs.size () < treg->target_regnum)
+	  {
+	    struct reg *reg = XCNEW (struct reg);
+	    reg->name = "";
+	    reg->size = 0;
+	    reg->offset = offset;
+	    tdesc->reg_defs.push_back (reg);
+	  }
+
+	gdb_assert (treg->target_regnum == 0
+		    || treg->target_regnum == tdesc->reg_defs.size ());
+
+	struct reg *reg = XCNEW (struct reg);
+	reg->name = treg->name.c_str ();
+	reg->size = treg->bitsize;
+	reg->offset = offset;
+	tdesc->reg_defs.push_back (reg);
+	offset += reg->size;
+      }

   tdesc->registers_size = offset / 8;

@@ -157,7 +150,7 @@  tdesc_get_features_xml (target_desc *tdesc)
 {
   /* Either .xmltarget or .features is not NULL.  */
   gdb_assert (tdesc->xmltarget != NULL
-	      || (tdesc->features != NULL
+	      || (!tdesc->features.empty ()
 		  && tdesc->arch != NULL));

   if (tdesc->xmltarget == NULL)
@@ -177,12 +170,10 @@  tdesc_get_features_xml (target_desc *tdesc)
 	  buffer += "</osabi>";
 	}

-      char *xml;
-
-      for (int i = 0; VEC_iterate (char_ptr, tdesc->features, i, xml); i++)
+      for (const tdesc_feature_up &feature : tdesc->features)
 	{
 	  buffer += "<xi:include href=\"";
-	  buffer += xml;
+	  buffer += feature->name;
 	  buffer += "\"/>";
 	}

@@ -195,19 +186,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
-  VEC_safe_push (char_ptr, tdesc->features, xstrdup (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.  */
@@ -252,21 +240,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/regformats/regdat.sh b/gdb/regformats/regdat.sh
index ce1627120d9439082709c82c5804724f39477eb1..8c6e191596350fb4e983f8736985d9832f41e2d3 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 3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250..a453eddec0cd97b692c4c9ace3f093c3fbc67422 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);
 }
-

-
-/* 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.  */