Patchwork [v3,6/8] Create xml from target descriptions

login
register
mail settings
Submitter Alan Hayward
Date March 1, 2018, 11:41 a.m.
Message ID <A3E98665-3DBD-42F4-B453-B365ED1AE7AB@arm.com>
Download mbox | patch
Permalink /patch/26133/
State New
Headers show

Comments

Alan Hayward - March 1, 2018, 11:41 a.m.
This patch adds a print_xml_feature visitor class which turns a C
target description into xml. Both gdb and gdbserver can do this.

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

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

Alan.

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

gdb/
	* common/tdesc.c (print_xml_feature::visit_post): Add xml parsing.
	(print_xml_feature::visit_pre): Likewise.
	(print_xml_feature::visit_post): Likewise.
	(print_xml_feature::visit): Likewise.
	(print_xml_feature::visit): Likewise.
	(print_xml_feature::visit): Likewise.
	(print_xml_feature::visit): Likewise.
	* common/tdesc.h (print_xml_feature): Add new class.
	* regformats/regdat.sh: obtain xml.
	* target-descriptions.c (struct target_desc): Add xmltarget.
	(print_xml_feature::visit_pre): Add xml vistor.
	(tdesc_get_features_xml): Add function to get xml.
	(maintenance_check_xml_descriptions): Test xml generation.
	* xml-tdesc.c (target_read_description_xml_string): Add function.
	* xml-tdesc.h (target_read_description_xml_string): Add declaration.

gdbserver/
	* tdesc.c (void target_desc::accept): Fill in function.
	(tdesc_get_features_xml): Remove old xml creation.
	(print_xml_feature::visit_pre): Add xml vistor.


index c0d2a10b0f7ba4e7b836e3b163d229f1608b02d8..1c5ddc572cf76cb24176fd638dde417687d202b0 100644
Philipp Rudo - March 12, 2018, 5:20 p.m.
On Thu, 1 Mar 2018 11:41:04 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

[...]

> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
> index 115e7cd77942b86dedced946773d1d71950e2bb3..ce5b89045c48c65ac4c6c48f6646f8d5c6ce9d8b 100644
> --- a/gdb/common/tdesc.c
> +++ b/gdb/common/tdesc.c
> @@ -288,4 +288,158 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
>    type->fields.emplace_back (name,
>  			     tdesc_predefined_type (TDESC_TYPE_INT32),
>  			     value, -1);
> -}
> \ No newline at end of file

This looks like an artefact to me.  It also appears in the commit on your
branch but not in the code.  Very strange ...

> +}
> +
> +void print_xml_feature::visit_post (const target_desc *e)
> +{
> +  *m_buffer += "</target>\n";
> +}
> +
> +void print_xml_feature::visit_pre (const tdesc_feature *e)
> +{
> +  *m_buffer += "<feature name=\"";
> +  *m_buffer += e->name;
> +  *m_buffer += "\">\n";
> +}

Have you considered using string_printf from common-utils? I think this could
make the code a lot better readable.  Maybe its even worth adding a
string_sprintf function.

Thanks
Philipp

> +
> +void print_xml_feature::visit_post (const tdesc_feature *e)
> +{
> +  *m_buffer += "</feature>\n";
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_builtin *type)
> +{
> +  error (_("xml output is not supported type \"%s\"."), type->name.c_str ());
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_vector *type)
> +{
> +  *m_buffer += "<vector id=\"";
> +  *m_buffer += type->name;
> +  *m_buffer += "\" type=\"";
> +  *m_buffer += type->element_type->name;
> +  *m_buffer += "\" count=\"";
> +  *m_buffer += std::to_string (type->count);
> +  *m_buffer += "\"/>\n";
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_with_fields *type)
> +{
> +  struct tdesc_type_field *f;
> +  const static char *types[] = { "struct", "union", "flags", "enum" };
> +
> +  gdb_assert (type->kind >= TDESC_TYPE_STRUCT && type->kind <= TDESC_TYPE_ENUM);
> +  *m_buffer += "<";
> +  *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
> +
> +  switch (type->kind)
> +    {
> +    case TDESC_TYPE_STRUCT:
> +    case TDESC_TYPE_FLAGS:
> +      *m_buffer += " id=\"";
> +      *m_buffer += type->name;
> +      if (type->size > 0)
> +	{
> +	  *m_buffer += "\" size=\"";
> +	  *m_buffer += std::to_string (type->size);
> +	}
> +	*m_buffer += "\">\n";
> +
> +      for (const tdesc_type_field &f : type->fields)
> +	{
> +	  *m_buffer += "  <field name=\"";
> +	  *m_buffer += f.name;
> +	  if (f.start == -1)
> +	  {
> +	    *m_buffer += "\" type=\"";
> +	    *m_buffer += f.type->name;
> +	  }
> +	  else
> +	  {
> +	    *m_buffer += "\" start=\"";
> +	    *m_buffer += std::to_string (f.start);
> +	    *m_buffer += "\" end=\"";
> +	    *m_buffer += std::to_string (f.end);
> +	  }
> +
> +	  *m_buffer += "\"/>\n";
> +	}
> +    break;
> +
> +    case TDESC_TYPE_ENUM:
> +      *m_buffer += " id=\"";
> +      *m_buffer += type->name;
> +      *m_buffer += "\">\n";
> +
> +      for (const tdesc_type_field &f : type->fields)
> +	{
> +	  *m_buffer += "  <field name=\"";
> +	  *m_buffer += f.name;
> +	  *m_buffer += "\" start=\"";
> +	  *m_buffer += std::to_string (f.start);
> +	  *m_buffer += "\"/>\n";
> +	}
> +      break;
> +
> +    case TDESC_TYPE_UNION:
> +      *m_buffer += " id=\"";
> +      *m_buffer += type->name;
> +      *m_buffer += "\">\n";
> +
> +      for (const tdesc_type_field &f : type->fields)
> +	{
> +	  *m_buffer += "  <field name=\"";
> +	  *m_buffer += f.name;
> +	  *m_buffer += "\" type=\"";
> +	  *m_buffer += f.type->name;
> +	  *m_buffer += "\"/>\n";
> +	}
> +      break;
> +
> +    default:
> +      error (_("xml output is not supported type \"%s\"."),
> +	     type->name.c_str ());
> +    }
> +
> +  *m_buffer += "</";
> +  *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
> +  *m_buffer += ">\n";
> +}
> +
> +void print_xml_feature::visit (const tdesc_reg *reg)
> +{
> +  *m_buffer += "<reg name=\"";
> +  *m_buffer += reg->name;
> +  *m_buffer += "\" bitsize=\"";
> +  *m_buffer += std::to_string (reg->bitsize);
> +  *m_buffer += "\" type=\"";
> +  *m_buffer += reg->type;
> +  *m_buffer += "\" regnum=\"";
> +  *m_buffer += std::to_string (reg->target_regnum);
> +  if (reg->group.length () > 0)
> +    {
> +      *m_buffer += "\" group=\"";
> +      *m_buffer += reg->group;
> +    }
> +  *m_buffer += "\"/>\n";
> +}
> +
> +void print_xml_feature::visit_pre (const target_desc *e)
> +{
> +#ifndef IN_PROCESS_AGENT
> +  *m_buffer += "<?xml version=\"1.0\"?>\n";
> +  *m_buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n";
> +  *m_buffer += "<target>\n";
> +  *m_buffer += "<architecture>";
> +  *m_buffer += tdesc_architecture_name (e);
> +  *m_buffer += "</architecture>\n";
> +
> +  const char *osabi = tdesc_osabi_name (e);
> +  if (osabi != nullptr)
> +    {
> +      *m_buffer += "<osabi>";
> +      *m_buffer += osabi;
> +      *m_buffer += "</osabi>\n";
> +    }
> +#endif
> +}
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 8534069d616b97dc82df04939a4ac935aa4e1cad..1c0c4b15f3cf2a7c4afd190963c1ca1ac77e1b59 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -27,7 +27,7 @@
>  /* A target description.  Inherit from tdesc_feature so that target_desc
>     can be used as tdesc_feature.  */
> 
> -struct target_desc
> +struct target_desc : tdesc_element
>  {
>    /* A vector of elements of register definitions that
>       describe the inferior's register set.  */
> @@ -73,6 +73,8 @@ public:
>      return !(*this == other);
>    }
>  #endif
> +
> +  void accept (tdesc_element_visitor &v) const;
>  };
> 
>  /* Copy target description SRC to DEST.  */
> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index e11344762a3a4114ed9aa459d7d739bd96a90ae5..0d9609bd4c3446b7a15df569923d22fc10105b74 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -59,6 +59,18 @@ bool target_desc::operator== (const target_desc &other) const
> 
>  #endif
> 
> +void target_desc::accept (tdesc_element_visitor &v) const
> +{
> +#ifndef IN_PROCESS_AGENT
> +  v.visit_pre (this);
> +
> +  for (const tdesc_feature_up &feature : features)
> +    feature->accept (v);
> +
> +  v.visit_post (this);
> +#endif
> +}
> +
>  void
>  init_target_desc (struct target_desc *tdesc)
>  {
> @@ -158,8 +170,7 @@ set_tdesc_osabi (struct target_desc *target_desc, const char *name)
>    target_desc->osabi = xstrdup (name);
>  }
> 
> -/* Return a string which is of XML format, including XML target
> -   description to be sent to GDB.  */
> +/* See common/tdesc.h.  */
> 
>  const char *
>  tdesc_get_features_xml (target_desc *tdesc)
> @@ -171,31 +182,9 @@ tdesc_get_features_xml (target_desc *tdesc)
> 
>    if (tdesc->xmltarget == NULL)
>      {
> -      std::string buffer ("@<?xml version=\"1.0\"?>");
> -
> -      buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
> -      buffer += "<target>";
> -      buffer += "<architecture>";
> -      buffer += tdesc_architecture_name (tdesc);
> -      buffer += "</architecture>";
> -
> -      const char *osabi = tdesc_osabi_name (tdesc);
> -      if (osabi != nullptr)
> -	{
> -	  buffer += "<osabi>";
> -	  buffer += osabi;
> -	  buffer += "</osabi>";
> -	}
> -
> -      for (const tdesc_feature_up &feature : tdesc->features)
> -	{
> -	  buffer += "<xi:include href=\"";
> -	  buffer += feature->name;
> -	  buffer += "\"/>";
> -	}
> -
> -      buffer += "</target>";
> -
> +      std::string buffer ("@");
> +      print_xml_feature v (&buffer);
> +      tdesc->accept (v);
>        tdesc->xmltarget = xstrdup (buffer.c_str ());
>      }
> 
> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -180,7 +180,6 @@ echo
>  cat <<EOF
>  #ifndef IN_PROCESS_AGENT
>    result->expedite_regs = expedite_regs_${name};
> -  result->xmltarget = xmltarget_${name};
>  #endif
> 
>    init_target_desc (result);
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index da2c1ce34531c1b23281c42f2dacbc85444ef544..93e73571177c980b4cd1975256c89e8ffb9fcdd6 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -333,6 +333,8 @@ struct target_desc : tdesc_element
>    /* The features associated with this target.  */
>    std::vector<tdesc_feature_up> features;
> 
> +  char *xmltarget = nullptr;
> +
>    void accept (tdesc_element_visitor &v) const override
>    {
>      v.visit_pre (this);
> @@ -1667,6 +1669,21 @@ private:
>    int m_next_regnum = 0;
>  };
> 
> +/* See common/tdesc.h.  */
> +
> +const char *
> +tdesc_get_features_xml (target_desc *tdesc)
> +{
> +  if (tdesc->xmltarget == nullptr)
> +    {
> +      std::string buffer ("@");
> +      print_xml_feature v (&buffer);
> +      tdesc->accept (v);
> +      tdesc->xmltarget = xstrdup (buffer.c_str ());
> +    }
> +  return tdesc->xmltarget;
> +}
> +
>  static void
>  maint_print_c_tdesc_cmd (const char *args, int from_tty)
>  {
> @@ -1760,7 +1777,36 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
>  	= file_read_description_xml (tdesc_xml.data ());
> 
>        if (tdesc == NULL || *tdesc != *e.second)
> -	failed++;
> +	{
> +	  printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
> +	  failed++;
> +	  continue;
> +	}
> +
> +      /* Convert both descriptions to xml, and then back again.  Confirm all
> +	 descriptions are identical.  */
> +
> +      const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
> +      const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
> +      gdb_assert (*xml == '@');
> +      gdb_assert (*xml2 == '@');
> +      const target_desc *t_trans = target_read_description_xml_string (xml+1);
> +      const target_desc *t_trans2 = target_read_description_xml_string (xml2+1);
> +
> +      if (t_trans == NULL || t_trans2 == NULL)
> +	{
> +	  printf_filtered (
> +	    _("Could not convert descriptions for %s back to xml (%p %p)\n"),
> +	    e.first, t_trans, t_trans2);
> +	  failed++;
> +	}
> +      else if (*tdesc != *t_trans || *tdesc != *t_trans2)
> +	{
> +	  printf_filtered
> +	    (_("Translated descriptions for %s do not match (%d %d)\n"),
> +	    e.first, *tdesc == *t_trans, *tdesc == *t_trans2);
> +	  failed++;
> +	}
>      }
>    printf_filtered (_("Tested %lu XML files, %d failed\n"),
>  		   (long) selftests::xml_tdesc.size (), failed);
> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
> index 8f0679707ad0f1e04d803f955f7fb98b4cc0c8c8..fee60e86dd10e1543b935c3cebce505d4dc828e2 100644
> --- a/gdb/xml-tdesc.h
> +++ b/gdb/xml-tdesc.h
> @@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
>     otherwise.  */
>  gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
> 
> +/* Take an xml string, parse it, and return the parsed description.  Does not
> +   handle a string containing includes.  */
> +
> +const struct target_desc *target_read_description_xml_string (const char *);
> +
>  #endif /* XML_TDESC_H */
> 
> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 9190d5f3c64ffdc6d7987d651527f597d695c5a6..f793f07c96847a3d61188fff1c5d63952cc37565 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -752,3 +752,12 @@ target_fetch_description_xml (struct target_ops *ops)
>    return output;
>  #endif
>  }
> +
> +/* Take an xml string, parse it, and return the parsed description.  Does not
> +   handle a string containing includes.  */
> +
> +const struct target_desc *
> +target_read_description_xml_string (const char *xml_str)
> +{
> +  return tdesc_parse_xml (xml_str, nullptr, nullptr);
> +}
>
Philipp Rudo - March 13, 2018, 6:05 p.m.
Hi Alan,

On Thu, 1 Mar 2018 11:41:04 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

[...]

> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
> index 115e7cd77942b86dedced946773d1d71950e2bb3..ce5b89045c48c65ac4c6c48f6646f8d5c6ce9d8b 100644
> --- a/gdb/common/tdesc.c
> +++ b/gdb/common/tdesc.c
> @@ -288,4 +288,158 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,

[...]

> +void print_xml_feature::visit (const tdesc_reg *reg)
> +{
> +  *m_buffer += "<reg name=\"";
> +  *m_buffer += reg->name;
> +  *m_buffer += "\" bitsize=\"";
> +  *m_buffer += std::to_string (reg->bitsize);
> +  *m_buffer += "\" type=\"";
> +  *m_buffer += reg->type;
> +  *m_buffer += "\" regnum=\"";
> +  *m_buffer += std::to_string (reg->target_regnum);
> +  if (reg->group.length () > 0)
> +    {
> +      *m_buffer += "\" group=\"";
> +      *m_buffer += reg->group;
> +    }
> +  *m_buffer += "\"/>\n";
> +}

in the xml you can also set if an register is gets save_resore, i.e 

if (!reg->save_restore)
  *m_buffer += "\" save-restore=\"no";

[...]

> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -180,7 +180,6 @@ echo
>  cat <<EOF
>  #ifndef IN_PROCESS_AGENT
>    result->expedite_regs = expedite_regs_${name};
> -  result->xmltarget = xmltarget_${name};
>  #endif
> 
>    init_target_desc (result);

This hunk caused all the test cases in gdb.server to fail.  Removing it 'fixed'
it for me.  Although i cannot tell you what went wrong.

Thanks
Philipp


> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index da2c1ce34531c1b23281c42f2dacbc85444ef544..93e73571177c980b4cd1975256c89e8ffb9fcdd6 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -333,6 +333,8 @@ struct target_desc : tdesc_element
>    /* The features associated with this target.  */
>    std::vector<tdesc_feature_up> features;
> 
> +  char *xmltarget = nullptr;
> +
>    void accept (tdesc_element_visitor &v) const override
>    {
>      v.visit_pre (this);
> @@ -1667,6 +1669,21 @@ private:
>    int m_next_regnum = 0;
>  };
> 
> +/* See common/tdesc.h.  */
> +
> +const char *
> +tdesc_get_features_xml (target_desc *tdesc)
> +{
> +  if (tdesc->xmltarget == nullptr)
> +    {
> +      std::string buffer ("@");
> +      print_xml_feature v (&buffer);
> +      tdesc->accept (v);
> +      tdesc->xmltarget = xstrdup (buffer.c_str ());
> +    }
> +  return tdesc->xmltarget;
> +}
> +
>  static void
>  maint_print_c_tdesc_cmd (const char *args, int from_tty)
>  {
> @@ -1760,7 +1777,36 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
>  	= file_read_description_xml (tdesc_xml.data ());
> 
>        if (tdesc == NULL || *tdesc != *e.second)
> -	failed++;
> +	{
> +	  printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
> +	  failed++;
> +	  continue;
> +	}
> +
> +      /* Convert both descriptions to xml, and then back again.  Confirm all
> +	 descriptions are identical.  */
> +
> +      const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
> +      const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
> +      gdb_assert (*xml == '@');
> +      gdb_assert (*xml2 == '@');
> +      const target_desc *t_trans = target_read_description_xml_string (xml+1);
> +      const target_desc *t_trans2 = target_read_description_xml_string (xml2+1);
> +
> +      if (t_trans == NULL || t_trans2 == NULL)
> +	{
> +	  printf_filtered (
> +	    _("Could not convert descriptions for %s back to xml (%p %p)\n"),
> +	    e.first, t_trans, t_trans2);
> +	  failed++;
> +	}
> +      else if (*tdesc != *t_trans || *tdesc != *t_trans2)
> +	{
> +	  printf_filtered
> +	    (_("Translated descriptions for %s do not match (%d %d)\n"),
> +	    e.first, *tdesc == *t_trans, *tdesc == *t_trans2);
> +	  failed++;
> +	}
>      }
>    printf_filtered (_("Tested %lu XML files, %d failed\n"),
>  		   (long) selftests::xml_tdesc.size (), failed);
> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
> index 8f0679707ad0f1e04d803f955f7fb98b4cc0c8c8..fee60e86dd10e1543b935c3cebce505d4dc828e2 100644
> --- a/gdb/xml-tdesc.h
> +++ b/gdb/xml-tdesc.h
> @@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
>     otherwise.  */
>  gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
> 
> +/* Take an xml string, parse it, and return the parsed description.  Does not
> +   handle a string containing includes.  */
> +
> +const struct target_desc *target_read_description_xml_string (const char *);
> +
>  #endif /* XML_TDESC_H */
> 
> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 9190d5f3c64ffdc6d7987d651527f597d695c5a6..f793f07c96847a3d61188fff1c5d63952cc37565 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -752,3 +752,12 @@ target_fetch_description_xml (struct target_ops *ops)
>    return output;
>  #endif
>  }
> +
> +/* Take an xml string, parse it, and return the parsed description.  Does not
> +   handle a string containing includes.  */
> +
> +const struct target_desc *
> +target_read_description_xml_string (const char *xml_str)
> +{
> +  return tdesc_parse_xml (xml_str, nullptr, nullptr);
> +}
>
Alan Hayward - March 14, 2018, 10:09 a.m.
> On 13 Mar 2018, at 18:05, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:

> 

> Hi Alan,

> 

> On Thu, 1 Mar 2018 11:41:04 +0000

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

> 

> [...]

> 

>> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c

>> index 115e7cd77942b86dedced946773d1d71950e2bb3..ce5b89045c48c65ac4c6c48f6646f8d5c6ce9d8b 100644

>> --- a/gdb/common/tdesc.c

>> +++ b/gdb/common/tdesc.c

>> @@ -288,4 +288,158 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,

> 

> [...]

> 

>> +void print_xml_feature::visit (const tdesc_reg *reg)

>> +{

>> +  *m_buffer += "<reg name=\"";

>> +  *m_buffer += reg->name;

>> +  *m_buffer += "\" bitsize=\"";

>> +  *m_buffer += std::to_string (reg->bitsize);

>> +  *m_buffer += "\" type=\"";

>> +  *m_buffer += reg->type;

>> +  *m_buffer += "\" regnum=\"";

>> +  *m_buffer += std::to_string (reg->target_regnum);

>> +  if (reg->group.length () > 0)

>> +    {

>> +      *m_buffer += "\" group=\"";

>> +      *m_buffer += reg->group;

>> +    }

>> +  *m_buffer += "\"/>\n";

>> +}

> 

> in the xml you can also set if an register is gets save_resore, i.e 

> 

> if (!reg->save_restore)

>  *m_buffer += "\" save-restore=\"no”;


A special 390 flag :)
I’ll double check make sure I haven’t missed any other flags too.


> 

> [...]

> 

>> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh

>> index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755

>> --- a/gdb/regformats/regdat.sh

>> +++ b/gdb/regformats/regdat.sh

>> @@ -180,7 +180,6 @@ echo

>> cat <<EOF

>> #ifndef IN_PROCESS_AGENT

>>   result->expedite_regs = expedite_regs_${name};

>> -  result->xmltarget = xmltarget_${name};

>> #endif

>> 

>>   init_target_desc (result);

> 

> This hunk caused all the test cases in gdb.server to fail.  Removing it 'fixed'

> it for me.  Although i cannot tell you what went wrong.

> 


I suspect this is an old style vs new style target descriptions issue.
I’ve found myself an Arm 32 box and potentially a PPC box too - hoping I can recreate the issue on one of them.

Thanks for narrowing it down to this.


Alan.

Patch

--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -372,4 +372,33 @@  void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		       int regnum, int save_restore, const char *group,
 		       int bitsize, const char *type);

+/* Return the tdesc in string XML format.  */
+
+const char *tdesc_get_features_xml (target_desc *tdesc);
+
+/* Print target description as xml.  */
+
+class print_xml_feature : public tdesc_element_visitor
+{
+public:
+  print_xml_feature (std::string *buffer_)
+    : m_buffer (buffer_)
+  {}
+
+  ~print_xml_feature ()
+  {}
+
+  void visit_pre (const target_desc *e) override;
+  void visit_post (const target_desc *e) override;
+  void visit_pre (const tdesc_feature *e) override;
+  void visit_post (const tdesc_feature *e) override;
+  void visit (const tdesc_type_builtin *type) override;
+  void visit (const tdesc_type_vector *type) override;
+  void visit (const tdesc_type_with_fields *type) override;
+  void visit (const tdesc_reg *reg) override;
+
+private:
+  std::string *m_buffer;
+};
+
 #endif /* ARCH_TDESC_H */
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index 115e7cd77942b86dedced946773d1d71950e2bb3..ce5b89045c48c65ac4c6c48f6646f8d5c6ce9d8b 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -288,4 +288,158 @@  tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
   type->fields.emplace_back (name,
 			     tdesc_predefined_type (TDESC_TYPE_INT32),
 			     value, -1);
-}
\ No newline at end of file
+}
+
+void print_xml_feature::visit_post (const target_desc *e)
+{
+  *m_buffer += "</target>\n";
+}
+
+void print_xml_feature::visit_pre (const tdesc_feature *e)
+{
+  *m_buffer += "<feature name=\"";
+  *m_buffer += e->name;
+  *m_buffer += "\">\n";
+}
+
+void print_xml_feature::visit_post (const tdesc_feature *e)
+{
+  *m_buffer += "</feature>\n";
+}
+
+void print_xml_feature::visit (const tdesc_type_builtin *type)
+{
+  error (_("xml output is not supported type \"%s\"."), type->name.c_str ());
+}
+
+void print_xml_feature::visit (const tdesc_type_vector *type)
+{
+  *m_buffer += "<vector id=\"";
+  *m_buffer += type->name;
+  *m_buffer += "\" type=\"";
+  *m_buffer += type->element_type->name;
+  *m_buffer += "\" count=\"";
+  *m_buffer += std::to_string (type->count);
+  *m_buffer += "\"/>\n";
+}
+
+void print_xml_feature::visit (const tdesc_type_with_fields *type)
+{
+  struct tdesc_type_field *f;
+  const static char *types[] = { "struct", "union", "flags", "enum" };
+
+  gdb_assert (type->kind >= TDESC_TYPE_STRUCT && type->kind <= TDESC_TYPE_ENUM);
+  *m_buffer += "<";
+  *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
+
+  switch (type->kind)
+    {
+    case TDESC_TYPE_STRUCT:
+    case TDESC_TYPE_FLAGS:
+      *m_buffer += " id=\"";
+      *m_buffer += type->name;
+      if (type->size > 0)
+	{
+	  *m_buffer += "\" size=\"";
+	  *m_buffer += std::to_string (type->size);
+	}
+	*m_buffer += "\">\n";
+
+      for (const tdesc_type_field &f : type->fields)
+	{
+	  *m_buffer += "  <field name=\"";
+	  *m_buffer += f.name;
+	  if (f.start == -1)
+	  {
+	    *m_buffer += "\" type=\"";
+	    *m_buffer += f.type->name;
+	  }
+	  else
+	  {
+	    *m_buffer += "\" start=\"";
+	    *m_buffer += std::to_string (f.start);
+	    *m_buffer += "\" end=\"";
+	    *m_buffer += std::to_string (f.end);
+	  }
+
+	  *m_buffer += "\"/>\n";
+	}
+    break;
+
+    case TDESC_TYPE_ENUM:
+      *m_buffer += " id=\"";
+      *m_buffer += type->name;
+      *m_buffer += "\">\n";
+
+      for (const tdesc_type_field &f : type->fields)
+	{
+	  *m_buffer += "  <field name=\"";
+	  *m_buffer += f.name;
+	  *m_buffer += "\" start=\"";
+	  *m_buffer += std::to_string (f.start);
+	  *m_buffer += "\"/>\n";
+	}
+      break;
+
+    case TDESC_TYPE_UNION:
+      *m_buffer += " id=\"";
+      *m_buffer += type->name;
+      *m_buffer += "\">\n";
+
+      for (const tdesc_type_field &f : type->fields)
+	{
+	  *m_buffer += "  <field name=\"";
+	  *m_buffer += f.name;
+	  *m_buffer += "\" type=\"";
+	  *m_buffer += f.type->name;
+	  *m_buffer += "\"/>\n";
+	}
+      break;
+
+    default:
+      error (_("xml output is not supported type \"%s\"."),
+	     type->name.c_str ());
+    }
+
+  *m_buffer += "</";
+  *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
+  *m_buffer += ">\n";
+}
+
+void print_xml_feature::visit (const tdesc_reg *reg)
+{
+  *m_buffer += "<reg name=\"";
+  *m_buffer += reg->name;
+  *m_buffer += "\" bitsize=\"";
+  *m_buffer += std::to_string (reg->bitsize);
+  *m_buffer += "\" type=\"";
+  *m_buffer += reg->type;
+  *m_buffer += "\" regnum=\"";
+  *m_buffer += std::to_string (reg->target_regnum);
+  if (reg->group.length () > 0)
+    {
+      *m_buffer += "\" group=\"";
+      *m_buffer += reg->group;
+    }
+  *m_buffer += "\"/>\n";
+}
+
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+#ifndef IN_PROCESS_AGENT
+  *m_buffer += "<?xml version=\"1.0\"?>\n";
+  *m_buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n";
+  *m_buffer += "<target>\n";
+  *m_buffer += "<architecture>";
+  *m_buffer += tdesc_architecture_name (e);
+  *m_buffer += "</architecture>\n";
+
+  const char *osabi = tdesc_osabi_name (e);
+  if (osabi != nullptr)
+    {
+      *m_buffer += "<osabi>";
+      *m_buffer += osabi;
+      *m_buffer += "</osabi>\n";
+    }
+#endif
+}
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 8534069d616b97dc82df04939a4ac935aa4e1cad..1c0c4b15f3cf2a7c4afd190963c1ca1ac77e1b59 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -27,7 +27,7 @@ 
 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */

-struct target_desc
+struct target_desc : tdesc_element
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
@@ -73,6 +73,8 @@  public:
     return !(*this == other);
   }
 #endif
+
+  void accept (tdesc_element_visitor &v) const;
 };

 /* Copy target description SRC to DEST.  */
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e11344762a3a4114ed9aa459d7d739bd96a90ae5..0d9609bd4c3446b7a15df569923d22fc10105b74 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -59,6 +59,18 @@  bool target_desc::operator== (const target_desc &other) const

 #endif

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

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

 const char *
 tdesc_get_features_xml (target_desc *tdesc)
@@ -171,31 +182,9 @@  tdesc_get_features_xml (target_desc *tdesc)

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

diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -180,7 +180,6 @@  echo
 cat <<EOF
 #ifndef IN_PROCESS_AGENT
   result->expedite_regs = expedite_regs_${name};
-  result->xmltarget = xmltarget_${name};
 #endif

   init_target_desc (result);
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index da2c1ce34531c1b23281c42f2dacbc85444ef544..93e73571177c980b4cd1975256c89e8ffb9fcdd6 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -333,6 +333,8 @@  struct target_desc : tdesc_element
   /* The features associated with this target.  */
   std::vector<tdesc_feature_up> features;

+  char *xmltarget = nullptr;
+
   void accept (tdesc_element_visitor &v) const override
   {
     v.visit_pre (this);
@@ -1667,6 +1669,21 @@  private:
   int m_next_regnum = 0;
 };

+/* See common/tdesc.h.  */
+
+const char *
+tdesc_get_features_xml (target_desc *tdesc)
+{
+  if (tdesc->xmltarget == nullptr)
+    {
+      std::string buffer ("@");
+      print_xml_feature v (&buffer);
+      tdesc->accept (v);
+      tdesc->xmltarget = xstrdup (buffer.c_str ());
+    }
+  return tdesc->xmltarget;
+}
+
 static void
 maint_print_c_tdesc_cmd (const char *args, int from_tty)
 {
@@ -1760,7 +1777,36 @@  maintenance_check_xml_descriptions (const char *dir, int from_tty)
 	= file_read_description_xml (tdesc_xml.data ());

       if (tdesc == NULL || *tdesc != *e.second)
-	failed++;
+	{
+	  printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
+	  failed++;
+	  continue;
+	}
+
+      /* Convert both descriptions to xml, and then back again.  Confirm all
+	 descriptions are identical.  */
+
+      const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
+      const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
+      gdb_assert (*xml == '@');
+      gdb_assert (*xml2 == '@');
+      const target_desc *t_trans = target_read_description_xml_string (xml+1);
+      const target_desc *t_trans2 = target_read_description_xml_string (xml2+1);
+
+      if (t_trans == NULL || t_trans2 == NULL)
+	{
+	  printf_filtered (
+	    _("Could not convert descriptions for %s back to xml (%p %p)\n"),
+	    e.first, t_trans, t_trans2);
+	  failed++;
+	}
+      else if (*tdesc != *t_trans || *tdesc != *t_trans2)
+	{
+	  printf_filtered
+	    (_("Translated descriptions for %s do not match (%d %d)\n"),
+	    e.first, *tdesc == *t_trans, *tdesc == *t_trans2);
+	  failed++;
+	}
     }
   printf_filtered (_("Tested %lu XML files, %d failed\n"),
 		   (long) selftests::xml_tdesc.size (), failed);
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index 8f0679707ad0f1e04d803f955f7fb98b4cc0c8c8..fee60e86dd10e1543b935c3cebce505d4dc828e2 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -44,5 +44,10 @@  const struct target_desc *target_read_description_xml (struct target_ops *);
    otherwise.  */
 gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);

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

diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 9190d5f3c64ffdc6d7987d651527f597d695c5a6..f793f07c96847a3d61188fff1c5d63952cc37565 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -752,3 +752,12 @@  target_fetch_description_xml (struct target_ops *ops)
   return output;
 #endif
 }
+
+/* Take an xml string, parse it, and return the parsed description.  Does not
+   handle a string containing includes.  */
+
+const struct target_desc *
+target_read_description_xml_string (const char *xml_str)
+{
+  return tdesc_parse_xml (xml_str, nullptr, nullptr);
+}