[v2,5/8] Use tdesc types in gdbserver tdesc

Message ID 771B81D3-D1C8-4D94-9713-3AD466DBA5F8@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Jan. 24, 2018, 9:29 a.m. UTC
  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 simpliest
way of getting gdbserver to retain the target description
information.

Stubs of the make_gdb_type functions are added to gdbserver
(they should never get called).

With this patch, gdbserver will now fully parse a target
description.

Alan.

2018-01-24  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* arch/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.
	* arch/tdesc.h (tdesc_type_builtin): Likewise.
	(tdesc_type_vector): Likewise.
	(tdesc_type_field): Likewise.
	(tdesc_type_with_fields): Likewise.
	* gdb/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.

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.
	(type *tdesc_type_builtin::make_gdb_type): Add stub.
	(type *tdesc_type_vector::make_gdb_type): Likewise.
	(type *tdesc_type_with_fields::make_gdb_type): Likewise.
  

Comments

Philipp Rudo Jan. 25, 2018, 1:12 p.m. UTC | #1
On Wed, 24 Jan 2018 09:29:20 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

[...]

> Stubs of the make_gdb_type functions are added to gdbserver
> (they should never get called).

See comment in patch #6.

[...]

> diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
> index 603ccd79434e1dccd17e69ef193193e673835dd0..9518571d03d394ee7cbf78b31974818201c889cd 100644
> --- a/gdb/arch/tdesc.c
> +++ b/gdb/arch/tdesc.c

[...]

> @@ -84,6 +106,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 arch/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 arch/tdesc.h.  */
> 
>  void
> @@ -96,3 +148,144 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
> 
>    feature->registers.emplace_back (reg);
>  }
> +
> +/* See arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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);
> +}
> \ No newline at end of file

With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
so many functions shared many of them could be made class methods. However
that exceeds the scope of this patch set, but should be kept in mind for the
future.

Philipp


> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index bc94d83ae58b9e79d8b71d0ce21c073f11765dd4..f0bd266a54601484df74ee1c5f8dce6fe04661c4 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -197,70 +197,17 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
>    return new_feature;
>  }
> 
> -/* See arch/tdesc.h.  */
> -
> -tdesc_type_with_fields *
> -tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> -		    int size)
> -{
> -  return NULL;
> -}
> -
> -/* See arch/tdesc.h.  */
> -
> -void
> -tdesc_add_flag (tdesc_type_with_fields *type, int start,
> -		const char *flag_name)
> -{}
> -
> -/* See arch/tdesc.h.  */
> -
> -struct tdesc_type *
> -tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> -{
> -  return NULL;
> -}
> -
> -/* See arch/tdesc.h.  */
> -
> -tdesc_type_with_fields *
> -tdesc_create_union (struct tdesc_feature *feature, const char *id)
> +type *tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
>  {
> -  return NULL;
> +  error (_("Cannot create gdbtypes."));
>  }
> 
> -/* See arch/tdesc.h.  */
> -
> -tdesc_type_with_fields *
> -tdesc_create_struct (struct tdesc_feature *feature, const char *id)
> -{
> -  return NULL;
> -}
> -
> -/* See arch/tdesc.h.  */
> -
> -struct tdesc_type *
> -tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> -		     struct tdesc_type *field_type, int count)
> +type *tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
>  {
> -  return NULL;
> +  error (_("Cannot create gdbtypes."));
>  }
> 
> -void
> -tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
> -		    int start, int end)
> -{}
> -
> -/* See arch/tdesc.h.  */
> -
> -void
> -tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> -		 struct tdesc_type *field_type)
> -{}
> -
> -/* See arch/tdesc.h.  */
> -
> -void
> -tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> +type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
>  {
> +  error (_("Cannot create gdbtypes."));
>  }
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 8ead19efa24b3cb154c895c484e791b92ff61387..4f11120dce4092f15de99680a7c4868c4a2f4493 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -50,80 +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);
> -  }
> -
> -  type *make_gdb_type (struct gdbarch *gdbarch) const override;
> -};
> -
> -/* 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);
> -  }
> -
> -  type *make_gdb_type (struct gdbarch *gdbarch) const override;
> -
> -  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);
> -  }
> -
> -  type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
> -  type *make_gdb_type_union (struct gdbarch *gdbarch) const;
> -  type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
> -  type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
> -  type *make_gdb_type (struct gdbarch *gdbarch) const override;
> -
> -  std::vector<tdesc_type_field> fields;
> -  int size;
> -};
> 
>  type *
>  tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> @@ -733,58 +659,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 arch/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 *
> @@ -1219,147 +1093,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
> 
>  /* See arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/tdesc.h.  */
> -
>  struct tdesc_feature *
>  tdesc_create_feature (struct target_desc *tdesc, const char *name,
>  		      const char *xml)
>
  
Omair Javaid Jan. 29, 2018, 7:27 a.m. UTC | #2
On 25 January 2018 at 18:12, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:

> On Wed, 24 Jan 2018 09:29:20 +0000
> Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> [...]
>
> > Stubs of the make_gdb_type functions are added to gdbserver
> > (they should never get called).
>
> See comment in patch #6.
>
> [...]
>
> > diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
> > index 603ccd79434e1dccd17e69ef193193e673835dd0..
> 9518571d03d394ee7cbf78b31974818201c889cd 100644
> > --- a/gdb/arch/tdesc.c
> > +++ b/gdb/arch/tdesc.c
>
> [...]
>
> > @@ -84,6 +106,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 arch/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 arch/tdesc.h.  */
> >
> >  void
> > @@ -96,3 +148,144 @@ tdesc_create_reg (struct tdesc_feature *feature,
> const char *name,
> >
> >    feature->registers.emplace_back (reg);
> >  }
> > +
> > +/* See arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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);
> > +}
> > \ No newline at end of file
>
> With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
> so many functions shared many of them could be made class methods. However
> that exceeds the scope of this patch set, but should be kept in mind for
> the
> future.
>

Apparently, it may seem better to move the whole target description code to
a common location and share it between gdb and gdbserver.

Target descriptions are architecture specific but at the same time target
description class code is architecture independent. So as highlighted by
Phillip this code should be moved out of arc/ folder and placed elsewhere.

It may seem a clean approach to create the division now and also refactor
code now than doing it in a later patch. Only gotcha here is that this code
is used by multiple targets so need to be tested aggressively.


>
> Philipp
>
>
> > diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> > index bc94d83ae58b9e79d8b71d0ce21c073f11765dd4..
> f0bd266a54601484df74ee1c5f8dce6fe04661c4 100644
> > --- a/gdb/gdbserver/tdesc.c
> > +++ b/gdb/gdbserver/tdesc.c
> > @@ -197,70 +197,17 @@ tdesc_create_feature (struct target_desc *tdesc,
> const char *name,
> >    return new_feature;
> >  }
> >
> > -/* See arch/tdesc.h.  */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> > -                 int size)
> > -{
> > -  return NULL;
> > -}
> > -
> > -/* See arch/tdesc.h.  */
> > -
> > -void
> > -tdesc_add_flag (tdesc_type_with_fields *type, int start,
> > -             const char *flag_name)
> > -{}
> > -
> > -/* See arch/tdesc.h.  */
> > -
> > -struct tdesc_type *
> > -tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> > -{
> > -  return NULL;
> > -}
> > -
> > -/* See arch/tdesc.h.  */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_union (struct tdesc_feature *feature, const char *id)
> > +type *tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> >  {
> > -  return NULL;
> > +  error (_("Cannot create gdbtypes."));
> >  }
> >
> > -/* See arch/tdesc.h.  */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_struct (struct tdesc_feature *feature, const char *id)
> > -{
> > -  return NULL;
> > -}
> > -
> > -/* See arch/tdesc.h.  */
> > -
> > -struct tdesc_type *
> > -tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> > -                  struct tdesc_type *field_type, int count)
> > +type *tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
> >  {
> > -  return NULL;
> > +  error (_("Cannot create gdbtypes."));
> >  }
> >
> > -void
> > -tdesc_add_bitfield (tdesc_type_with_fields *type, const char
> *field_name,
> > -                 int start, int end)
> > -{}
> > -
> > -/* See arch/tdesc.h.  */
> > -
> > -void
> > -tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> > -              struct tdesc_type *field_type)
> > -{}
> > -
> > -/* See arch/tdesc.h.  */
> > -
> > -void
> > -tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> > +type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch)
> const
> >  {
> > +  error (_("Cannot create gdbtypes."));
> >  }
> > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> > index 8ead19efa24b3cb154c895c484e791b92ff61387..
> 4f11120dce4092f15de99680a7c4868c4a2f4493 100644
> > --- a/gdb/target-descriptions.c
> > +++ b/gdb/target-descriptions.c
> > @@ -50,80 +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);
> > -  }
> > -
> > -  type *make_gdb_type (struct gdbarch *gdbarch) const override;
> > -};
> > -
> > -/* 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);
> > -  }
> > -
> > -  type *make_gdb_type (struct gdbarch *gdbarch) const override;
> > -
> > -  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);
> > -  }
> > -
> > -  type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
> > -  type *make_gdb_type_union (struct gdbarch *gdbarch) const;
> > -  type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
> > -  type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
> > -  type *make_gdb_type (struct gdbarch *gdbarch) const override;
> > -
> > -  std::vector<tdesc_type_field> fields;
> > -  int size;
> > -};
> >
> >  type *
> >  tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> > @@ -733,58 +659,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 arch/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 *
> > @@ -1219,147 +1093,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
> >
> >  /* See arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/tdesc.h.  */
> > -
> >  struct tdesc_feature *
> >  tdesc_create_feature (struct target_desc *tdesc, const char *name,
> >                     const char *xml)
> >
>
>
  
Alan Hayward Jan. 29, 2018, 11:01 a.m. UTC | #3
> 

> With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and

> so many functions shared many of them could be made class methods. However

> that exceeds the scope of this patch set, but should be kept in mind for the

> future.


The "[PATCH] Use visitors for make_gdb_type” patch should take care of a large set of them.

> 

> Apparently, it may seem better to move the whole target description code to a common location and share it between gdb and gdbserver. 

> 

> Target descriptions are architecture specific but at the same time target description class code is architecture independent. So as highlighted by Phillip this code should be moved out of arc/ folder and placed elsewhere.

> 


Agreed.
Is common/ the best place? I can’t see any other directory that looks right. 

> It may seem a clean approach to create the division now and also refactor code now than doing it in a later patch. Only gotcha here is that this code is used by multiple targets so need to be tested aggressively.


That’s why I didn’t want to refactor too much in one set.


Thanks,
Alan.
  
Philipp Rudo Jan. 29, 2018, 11:30 a.m. UTC | #4
Hi Alan,


On Mon, 29 Jan 2018 11:01:25 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

> > 
> > With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
> > so many functions shared many of them could be made class methods. However
> > that exceeds the scope of this patch set, but should be kept in mind for the
> > future.  
> 
> The "[PATCH] Use visitors for make_gdb_type” patch should take care of a large set of them.

I was more thinking of functions like tdesc_add_* and tdesc_create_* (including
tdesc_create_reg).  They are not covered by your make_gdb_type patch.  The way
I see it the changes needed to be made are quite small.  Unfortunately they
require to regenerate the cfiles making the resulting patches large and hard
to read.

Philipp

 
> > 
> > Apparently, it may seem better to move the whole target description code to a common location and share it between gdb and gdbserver. 
> > 
> > Target descriptions are architecture specific but at the same time target description class code is architecture independent. So as highlighted by Phillip this code should be moved out of arc/ folder and placed elsewhere.
> >   
> 
> Agreed.
> Is common/ the best place? I can’t see any other directory that looks right. 
> 
> > It may seem a clean approach to create the division now and also refactor code now than doing it in a later patch. Only gotcha here is that this code is used by multiple targets so need to be tested aggressively.  
> 
> That’s why I didn’t want to refactor too much in one set.
> 
> 
> Thanks,
> Alan.
  
Alan Hayward Jan. 29, 2018, 3:52 p.m. UTC | #5
> On 29 Jan 2018, at 11:30, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:

> 

> Hi Alan,

> 

> 

> On Mon, 29 Jan 2018 11:01:25 +0000

> Alan Hayward <Alan.Hayward@arm.com> wrote:

> 

>>> 

>>> With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and

>>> so many functions shared many of them could be made class methods. However

>>> that exceeds the scope of this patch set, but should be kept in mind for the

>>> future.  

>> 

>> The "[PATCH] Use visitors for make_gdb_type” patch should take care of a large set of them.

> 

> I was more thinking of functions like tdesc_add_* and tdesc_create_* (including

> tdesc_create_reg).  They are not covered by your make_gdb_type patch.  The way

> I see it the changes needed to be made are quite small.  Unfortunately they

> require to regenerate the cfiles making the resulting patches large and hard

> to read.

> 


Ok, happy to look at these in a follow on patch. Should be simple enough to write the patch,
just a little messy to review.

Alan.
  

Patch

diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
index 480475fff873a19710befdf39e2bf3053be3efb3..9a5bf6f11b670e04e2b51f8334bc0adaf0b43962 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/tdesc.h
@@ -189,6 +189,82 @@  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);
+  }
+
+  type *make_gdb_type (struct gdbarch *gdbarch) const override;
+};
+
+/* 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);
+  }
+
+  type *make_gdb_type (struct gdbarch *gdbarch) const override;
+
+  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);
+  }
+
+  type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
+  type *make_gdb_type_union (struct gdbarch *gdbarch) const;
+  type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
+  type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
+  type *make_gdb_type (struct gdbarch *gdbarch) const override;
+
+  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.  */

diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
index 603ccd79434e1dccd17e69ef193193e673835dd0..9518571d03d394ee7cbf78b31974818201c889cd 100644
--- a/gdb/arch/tdesc.c
+++ b/gdb/arch/tdesc.c
@@ -39,6 +39,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);
@@ -84,6 +106,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 arch/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 arch/tdesc.h.  */

 void
@@ -96,3 +148,144 @@  tdesc_create_reg (struct tdesc_feature *feature, const char *name,

   feature->registers.emplace_back (reg);
 }
+
+/* See arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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);
+}
\ No newline at end of file
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index bc94d83ae58b9e79d8b71d0ce21c073f11765dd4..f0bd266a54601484df74ee1c5f8dce6fe04661c4 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -197,70 +197,17 @@  tdesc_create_feature (struct target_desc *tdesc, const char *name,
   return new_feature;
 }

-/* See arch/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
-		    int size)
-{
-  return NULL;
-}
-
-/* See arch/tdesc.h.  */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
-		const char *flag_name)
-{}
-
-/* See arch/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See arch/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *id)
+type *tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
 {
-  return NULL;
+  error (_("Cannot create gdbtypes."));
 }

-/* See arch/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See arch/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
-		     struct tdesc_type *field_type, int count)
+type *tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
 {
-  return NULL;
+  error (_("Cannot create gdbtypes."));
 }

-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
-		    int start, int end)
-{}
-
-/* See arch/tdesc.h.  */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
-		 struct tdesc_type *field_type)
-{}
-
-/* See arch/tdesc.h.  */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
+type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
 {
+  error (_("Cannot create gdbtypes."));
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 8ead19efa24b3cb154c895c484e791b92ff61387..4f11120dce4092f15de99680a7c4868c4a2f4493 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -50,80 +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);
-  }
-
-  type *make_gdb_type (struct gdbarch *gdbarch) const override;
-};
-
-/* 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);
-  }
-
-  type *make_gdb_type (struct gdbarch *gdbarch) const override;
-
-  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);
-  }
-
-  type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
-  type *make_gdb_type_union (struct gdbarch *gdbarch) const;
-  type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
-  type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
-  type *make_gdb_type (struct gdbarch *gdbarch) const override;
-
-  std::vector<tdesc_type_field> fields;
-  int size;
-};

 type *
 tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
@@ -733,58 +659,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 arch/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 *
@@ -1219,147 +1093,6 @@  tdesc_use_registers (struct gdbarch *gdbarch,

 /* See arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/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 arch/tdesc.h.  */
-
 struct tdesc_feature *
 tdesc_create_feature (struct target_desc *tdesc, const char *name,
 		      const char *xml)