[v2,6/8] Create xml from target descriptions
Commit Message
This patch adds a print_xml_feature visitor class which turns a C
target description into xml.
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-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* arch/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.
* arch/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.
Comments
i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
with different implementations in target-descriptions.c and gdbserver/tdesc.c
while all other visit_* implementations are in arch/tdesc.c. I would prefere
visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
even declare the method for GDBserver, i.e.
struct tdesc_type
{
[...]
#ifndef GDBSERVER
virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
#endif
};
The problem i see with implementing stubs calling error is that you cannot find
out you made a mistake until you call the function during run-time. This gives
room to nasty bugs which could easily be prevented when there is a compile bug.
Philipp
On Wed, 24 Jan 2018 09:29:50 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> This patch adds a print_xml_feature visitor class which turns a C
> target description into xml.
>
> 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-01-24 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * arch/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.
> * arch/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.
>
>
> diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
> index 9a5bf6f11b670e04e2b51f8334bc0adaf0b43962..b2e99c5ce1f7da0e6a194ca721eea96082e1fa68 100644
> --- a/gdb/arch/tdesc.h
> +++ b/gdb/arch/tdesc.h
> @@ -195,7 +195,7 @@ struct tdesc_type_builtin : tdesc_type
> : tdesc_type (name, kind)
> {}
>
> - void accept (tdesc_element_visitor &v) const override;
> + void accept (tdesc_element_visitor &v) const override
> {
> v.visit (this);
> }
> @@ -366,4 +366,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/arch/tdesc.c b/gdb/arch/tdesc.c
> index 9518571d03d394ee7cbf78b31974818201c889cd..388c6af1b93d40cf7cb5fb97c55912b91601814b 100644
> --- a/gdb/arch/tdesc.c
> +++ b/gdb/arch/tdesc.c
> @@ -288,4 +288,138 @@ 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";
> +}
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 8ae6cddc1896af99d86206d159fb952a0f666043..6a86f66cf2aaf9aa2b20203cad1d272d70026822 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. */
> @@ -72,7 +72,10 @@ 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 f0bd266a54601484df74ee1c5f8dce6fe04661c4..42a657f6827ed06c9d98275c7566d0c02890e7b3 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)
> {
> @@ -155,30 +167,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->arch;
> - buffer += "</architecture>";
> -
> - if (tdesc->osabi != nullptr)
> - {
> - buffer += "<osabi>";
> - buffer += tdesc->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 ());
> }
>
> @@ -211,3 +202,22 @@ type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
> {
> error (_("Cannot create gdbtypes."));
> }
> +
> +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 += e->arch;
> + *m_buffer += "</architecture>\n";
> +
> + if (e->osabi != nullptr)
> + {
> + *m_buffer += "<osabi>";
> + *m_buffer += e->osabi;
> + *m_buffer += "</osabi>\n";
> + }
> +#endif
> + }
> 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 4f11120dce4092f15de99680a7c4868c4a2f4493..68a85758f7f63e6b862afa29697adee77e776d89 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -316,6 +316,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);
> @@ -1633,6 +1635,39 @@ private:
> int m_next_regnum = 0;
> };
>
> +void print_xml_feature::visit_pre (const target_desc *e)
> +{
> + *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 (e)->printable_name;
> + *m_buffer += "</architecture>\n";
> +
> + if (e->osabi != GDB_OSABI_UNKNOWN)
> + {
> + *m_buffer += "<osabi>";
> + *m_buffer += gdbarch_osabi_name (tdesc_osabi (e));
> + *m_buffer += "</osabi>\n";
> + }
> +}
> +
> +/* Return a string which is of XML format, including XML target
> + description to be sent to GDB. */
> +
> +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)
> {
> @@ -1726,7 +1761,34 @@ 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);
> + const target_desc *t_trans = target_read_description_xml_string (xml);
> + const target_desc *t_trans2 = target_read_description_xml_string (xml2);
> +
> + 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 <prudo@linux.vnet.ibm.com> writes:
> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
> even declare the method for GDBserver, i.e.
>
> struct tdesc_type
> {
>
> [...]
>
> #ifndef GDBSERVER
> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
> #endif
> };
>
> The problem i see with implementing stubs calling error is that you cannot find
> out you made a mistake until you call the function during run-time. This gives
> room to nasty bugs which could easily be prevented when there is a compile bug.
make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
possible. You can create an sub-class of tdesc_element_visitor in gdb
side, and create the gdb type by visiting these elements, like this,
class gdb_type_creator : public tdesc_element_visitor
{
public:
gdb_type_createor (struct gdbarch *gdbarch)
: m_gdbarch (gdbarch)
{}
void visit (const tdesc_type_builtin *e) override
{
switch (e->kind)
{
case TDESC_TYPE_BOOL:
m_type = builtin_type (m_gdbarch)->builtin_bool;
break;
....
};
}
void visit (const tdesc_type_vector *e) override
{
// do what tdesc_type_vector::make_gdb_type does.
}
void visit (const tdesc_type_with_fields *e) override
{
// likewise.
}
private:
// input
struct * const m_gdbarch;
// output
type *m_type;
};
gdb_type_creator gdb_type (gdbarch);
tdesc_type->accept (gdb_type);
gdb_type.m_type is the type we need.
> On 25 Jan 2018, at 13:14, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
>
>
> i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
> with different implementations in target-descriptions.c and gdbserver/tdesc.c
> while all other visit_* implementations are in arch/tdesc.c. I would prefere
> visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
> some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
Thanks for the review!
A common print_xml_feature would require 4 ifdefs, which would be too messy.
I’ll take a look see if I can communise the tdesc_architecture and tdesc_osabi
interfaces - it should be fairly simple to do.
Once done, I should be able to add a common print_xml_feature without any
ifdefs. Hopefully.
> On 25 Jan 2018, at 15:44, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
>
>> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
>> even declare the method for GDBserver, i.e.
>>
>> struct tdesc_type
>> {
>>
>> [...]
>>
>> #ifndef GDBSERVER
>> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
>> #endif
>> };
>>
>> The problem i see with implementing stubs calling error is that you cannot find
>> out you made a mistake until you call the function during run-time. This gives
>> room to nasty bugs which could easily be prevented when there is a compile bug.
>
>
> make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
> possible. You can create an sub-class of tdesc_element_visitor in gdb
> side, and create the gdb type by visiting these elements, like this,
>
> class gdb_type_creator : public tdesc_element_visitor
….
Nice idea. I can add an extra patch into the series to do this.
On Thu, 25 Jan 2018 16:13:16 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> > On 25 Jan 2018, at 13:14, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> >
> >
> > i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
> > with different implementations in target-descriptions.c and gdbserver/tdesc.c
> > while all other visit_* implementations are in arch/tdesc.c. I would prefere
> > visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
> > some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
>
> Thanks for the review!
>
> A common print_xml_feature would require 4 ifdefs, which would be too messy.
> I’ll take a look see if I can communise the tdesc_architecture and tdesc_osabi
> interfaces - it should be fairly simple to do.
> Once done, I should be able to add a common print_xml_feature without any
> ifdefs. Hopefully.
>
Having a common interface would be even better :)
>
> > On 25 Jan 2018, at 15:44, Yao Qi <qiyaoltc@gmail.com> wrote:
> >
> > Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> >
> >> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
> >> even declare the method for GDBserver, i.e.
> >>
> >> struct tdesc_type
> >> {
> >>
> >> [...]
> >>
> >> #ifndef GDBSERVER
> >> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
> >> #endif
> >> };
> >>
> >> The problem i see with implementing stubs calling error is that you cannot find
> >> out you made a mistake until you call the function during run-time. This gives
> >> room to nasty bugs which could easily be prevented when there is a compile bug.
> >
> >
> > make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
> > possible. You can create an sub-class of tdesc_element_visitor in gdb
> > side, and create the gdb type by visiting these elements, like this,
> >
> > class gdb_type_creator : public tdesc_element_visitor
> ….
>
> Nice idea. I can add an extra patch into the series to do this.
Yeah, Yao's solution is better than mine.
Oh i forgot to tell you, great series.
Philipp
@@ -195,7 +195,7 @@ struct tdesc_type_builtin : tdesc_type
: tdesc_type (name, kind)
{}
- void accept (tdesc_element_visitor &v) const override;
+ void accept (tdesc_element_visitor &v) const override
{
v.visit (this);
}
@@ -366,4 +366,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 */
@@ -288,4 +288,138 @@ 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";
+}
@@ -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. */
@@ -72,7 +72,10 @@ public:
{
return !(*this == other);
}
+
#endif
+
+ void accept (tdesc_element_visitor &v) const;
};
/* Copy target description SRC to DEST. */
@@ -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)
{
@@ -155,30 +167,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->arch;
- buffer += "</architecture>";
-
- if (tdesc->osabi != nullptr)
- {
- buffer += "<osabi>";
- buffer += tdesc->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 ());
}
@@ -211,3 +202,22 @@ type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
{
error (_("Cannot create gdbtypes."));
}
+
+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 += e->arch;
+ *m_buffer += "</architecture>\n";
+
+ if (e->osabi != nullptr)
+ {
+ *m_buffer += "<osabi>";
+ *m_buffer += e->osabi;
+ *m_buffer += "</osabi>\n";
+ }
+#endif
+ }
@@ -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);
@@ -316,6 +316,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);
@@ -1633,6 +1635,39 @@ private:
int m_next_regnum = 0;
};
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+ *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 (e)->printable_name;
+ *m_buffer += "</architecture>\n";
+
+ if (e->osabi != GDB_OSABI_UNKNOWN)
+ {
+ *m_buffer += "<osabi>";
+ *m_buffer += gdbarch_osabi_name (tdesc_osabi (e));
+ *m_buffer += "</osabi>\n";
+ }
+}
+
+/* Return a string which is of XML format, including XML target
+ description to be sent to GDB. */
+
+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)
{
@@ -1726,7 +1761,34 @@ 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);
+ const target_desc *t_trans = target_read_description_xml_string (xml);
+ const target_desc *t_trans2 = target_read_description_xml_string (xml2);
+
+ 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);
@@ -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 */
@@ -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);
+}