public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCHv2 0/3] Additional maintenance command for dumping target descriptions
@ 2020-06-11 23:22 Andrew Burgess
  2020-06-11 23:22 ` [PATCHv2 1/3] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Andrew Burgess @ 2020-06-11 23:22 UTC (permalink / raw)
  To: gdb-patches

In the first patch of this series the only change is I addressed
Pedro's feedback.

The second patch is new, this moves the printing of <compatible>
entities into the gdbsupport/ directory, inside the print_xml_feature
class.

The third patch is the old second patch.  The documentation has not
changed.  The code is now simpler thanks to the new #2 patch.  I
addressed Pedro's feedback including adding some extra tests.

Feedback welcome.

Thanks,
Andrew


---

Andrew Burgess (3):
  gdb: Allow target description to be dumped even when it is remote
  gdb: Print compatible information within print_xml_feature
  gdb: New maintenance command to print XML target description

 gdb/ChangeLog                               |  30 +++++
 gdb/NEWS                                    |   6 +
 gdb/doc/ChangeLog                           |   5 +
 gdb/doc/gdb.texinfo                         |   9 ++
 gdb/target-descriptions.c                   | 109 ++++++++++++++---
 gdb/testsuite/ChangeLog                     |   8 ++
 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml |  10 ++
 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml |  27 +++++
 gdb/testsuite/gdb.xml/maint-xml-dump.exp    | 124 ++++++++++++++++++++
 gdb/testsuite/gdb.xml/tdesc-reload.c        |  22 ++++
 gdb/testsuite/gdb.xml/tdesc-reload.exp      |  83 +++++++++++++
 gdbserver/ChangeLog                         |   6 +
 gdbserver/tdesc.cc                          |  21 ++++
 gdbsupport/ChangeLog                        |  22 ++++
 gdbsupport/tdesc.cc                         | 106 ++++++++++++-----
 gdbsupport/tdesc.h                          |  44 ++++++-
 16 files changed, 587 insertions(+), 45 deletions(-)
 create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
 create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
 create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump.exp
 create mode 100644 gdb/testsuite/gdb.xml/tdesc-reload.c
 create mode 100644 gdb/testsuite/gdb.xml/tdesc-reload.exp

-- 
2.25.4


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

* [PATCHv2 1/3] gdb: Allow target description to be dumped even when it is remote
  2020-06-11 23:22 [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
@ 2020-06-11 23:22 ` Andrew Burgess
  2020-06-11 23:22 ` [PATCHv2 2/3] gdb: Print compatible information within print_xml_feature Andrew Burgess
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2020-06-11 23:22 UTC (permalink / raw)
  To: gdb-patches

The maintenance command 'maintenance print c-tdesc' can only print the
target description if it was loaded from a local file, or if the local
filename is passed to the maintenance command as an argument.

Sometimes it would be nice to know what target description GDB was
given by the remote, however, if I connect to a remote target and try
this command I see this:

  (gdb) maintenance print c-tdesc
  The current target description did not come from an XML file.
  (gdb)

Which is not very helpful.

This commit changes things so that if the description came from the
remote end then GDB will use a fake filename 'fetched from target' as
the filename for the description, GDB will then create the C
description of the target as though it came from this file.  Example
output would look like this (I snipped the feature creation from the
middle as that hasn't changed):

  (gdb) maintenance print c-tdesc
  /* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
    Original: fetched from target */

  #include "defs.h"
  #include "osabi.h"
  #include "target-descriptions.h"

  struct target_desc *tdesc_fetched_from_target;
  static void
  initialize_tdesc_fetched_from_target (void)
  {
    struct target_desc *result = allocate_target_description ();
    struct tdesc_feature *feature;

    /* ... features created here ... */

    tdesc_fetched_from_target = result;
  }
  (gdb)

In order to support using 'fetched from target' I had to update the
print_c_tdesc code to handle filenames that include a space.  This has
the benefit that we can now print out real files with spaces in the
name, for example the file 'with space.xml':

  (gdb) maint print c-tdesc with space.xml

I originally added this functionality so I could inspect the
description passed to GDB by the remote target.  After using this for
a while I realised that actually having GDB recreate the XML would be
even better, so a later commit will add that functionality too.

Still, given how small this patch is I thought it might be nice to
include this in GDB anyway.

While I was working on this anyway I've added filename command
completion to this command.

gdb/ChangeLog:

	* target-descriptions.c (print_c_tdesc::print_c_tdesc): Change
	whitespace to underscore.
	(maint_print_c_tdesc_cmd): Use fake filename for target
	descriptions that came from the target.
	(_initialize_target_descriptions): Add filename command completion
	for 'maint print c-tdesc'.
---
 gdb/ChangeLog             |  9 +++++++++
 gdb/target-descriptions.c | 11 +++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 20a3a640f4f..da6cb76404c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1270,6 +1270,8 @@ class print_c_tdesc : public tdesc_element_visitor
 	break;
       else if (*inp == '-')
 	*outp++ = '_';
+      else if (*inp == ' ')
+	*outp++ = '_';
       else
 	*outp++ = *inp;
     *outp = '\0';
@@ -1680,7 +1682,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
     error (_("There is no target description to print."));
 
   if (filename == NULL)
-    error (_("The current target description did not come from an XML file."));
+    filename = "fetched from target";
 
   std::string filename_after_features (filename);
   auto loc = filename_after_features.rfind ("/features/");
@@ -1811,6 +1813,8 @@ void _initialize_target_descriptions ();
 void
 _initialize_target_descriptions ()
 {
+  cmd_list_element *cmd;
+
   tdesc_data = gdbarch_data_register_pre_init (tdesc_data_init);
 
   add_basic_prefix_cmd ("tdesc", class_maintenance, _("\
@@ -1842,11 +1846,10 @@ Unset the file to read for an XML target description.\n\
 When unset, GDB will read the description from the target."),
 	   &tdesc_unset_cmdlist);
 
-  add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
+  cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
 Print the current target description as a C source file."),
 	   &maintenanceprintlist);
-
-  cmd_list_element *cmd;
+  set_cmd_completer (cmd, filename_completer);
 
   cmd = add_cmd ("xml-descriptions", class_maintenance,
 		 maintenance_check_xml_descriptions, _("\
-- 
2.25.4


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

* [PATCHv2 2/3] gdb: Print compatible information within print_xml_feature
  2020-06-11 23:22 [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
  2020-06-11 23:22 ` [PATCHv2 1/3] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
@ 2020-06-11 23:22 ` Andrew Burgess
  2020-06-11 23:22 ` [PATCHv2 3/3] gdb: New maintenance command to print XML target description Andrew Burgess
  2020-06-22 18:13 ` [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
  3 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2020-06-11 23:22 UTC (permalink / raw)
  To: gdb-patches

The gdbsupport directory contains a helper class print_xml_feature
that is shared between gdb and gdbserver.  This class is used for
printing an XML representation of a target_desc object.

Currently this class doesn't have the ability to print the
<compatible> entities that can appear within a target description, I
guess no targets have needed that functionality yet.

The print_xml_feature classes API is based around operating on the
target_desc class, however, the sharing between gdb and gdbserver is
purely textural, we rely on their being a class called target_desc in
both gdb and gdbserver, but there is no shared implementation.  We
then have a set of functions declared that operate on an object of
type target_desc, and again these functions have completely separate
implementations.

Currently then the gdb version of target_desc contains a vector of
bfd_arch_info pointers which represents the compatible entries from a
target description.  The gdbserver version of target_desc has no such
information.  Further, the gdbserver code doesn't seem to include the
bfd headers, and so doesn't know about the bfd types.

I was reluctant to include the bfd headers into gdbserver just so I
can reference the compatible information, which isn't (currently) even
needed in gdbserver.

So, the approach I take in this patch is to wrap the compatible
information into a new helper class.  This class is declared in the
gdbsupport library, but implemented separately in both gdb and
gdbserver.

In gdbserver the class is empty.  The compatible information within
the gdbserver is an empty list, of empty classes.

In gdb the class contains a pointer to the bfd_arch_info object.

With this in place we can now add support to print_xml_feature for
printing the compatible information if it is present.  In the
gdbserver code this will never happen, as the gdbserver never has any
compatible information.  But in gdb, this code will trigger when
appropriate.

gdb/ChangeLog:

	* target-descriptions.c (class tdesc_compatible_info): New class.
	(struct target_desc): Change type of compatible vector.
	(tdesc_compatible_p): Update for change in type of
	target_desc::compatible.
	(tdesc_compatible_info_list): New function.
	(tdesc_compatible_info_arch_name): New function.
	(tdesc_add_compatible): Update for change in type of
	target_desc::compatible.
	(print_c_tdesc::visit_pre): Likewise.

gdbserver/ChangeLog:

	* tdesc.cc (struct tdesc_compatible_info): New struct.
	(tdesc_compatible_info_list): New function.
	(tdesc_compatible_info_arch_name): New function.

gdbsupport/ChangeLog:

	* tdesc.cc (print_xml_feature::visit_pre): Print compatible
	information.
	* tdesc.h (struct tdesc_compatible_info): Declare new struct.
	(tdesc_compatible_info_up): New typedef.
	(tdesc_compatible_info_list): Declare new function.
	(tdesc_compatible_info_arch_name): Declare new function.
---
 gdb/ChangeLog             | 12 ++++++++
 gdb/target-descriptions.c | 61 ++++++++++++++++++++++++++++++++-------
 gdbserver/ChangeLog       |  6 ++++
 gdbserver/tdesc.cc        | 21 ++++++++++++++
 gdbsupport/ChangeLog      |  9 ++++++
 gdbsupport/tdesc.cc       |  6 ++++
 gdbsupport/tdesc.h        | 21 ++++++++++++++
 7 files changed, 126 insertions(+), 10 deletions(-)

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index da6cb76404c..1937e7ca4a7 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -308,6 +308,29 @@ make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype)
   return gdb_type.get_type ();
 }
 
+/* Wrapper around bfd_arch_info_type.  A class with this name is used in
+   the API that is shared between gdb and gdbserver code, but gdbserver
+   doesn't use compatibility information, so its version of this class is
+   empty.  */
+
+class tdesc_compatible_info
+{
+public:
+  /* Constructor.  */
+  explicit tdesc_compatible_info (const bfd_arch_info_type *arch)
+    : m_arch (arch)
+  { /* Nothing.  */ }
+
+  /* Access the contained pointer.  */
+  const bfd_arch_info_type *arch () const
+  { return m_arch; }
+
+private:
+  /* Architecture information looked up from the <compatible> entity within
+     a target description.  */
+  const bfd_arch_info_type *m_arch;
+};
+
 /* A target description.  */
 
 struct target_desc : tdesc_element
@@ -328,7 +351,7 @@ struct target_desc : tdesc_element
   enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
 
   /* The list of compatible architectures reported by the target.  */
-  std::vector<const bfd_arch_info *> compatible;
+  std::vector<tdesc_compatible_info_up> compatible;
 
   /* Any architecture-specific properties specified by the target.  */
   std::vector<property> properties;
@@ -598,11 +621,11 @@ int
 tdesc_compatible_p (const struct target_desc *target_desc,
 		    const struct bfd_arch_info *arch)
 {
-  for (const bfd_arch_info *compat : target_desc->compatible)
+  for (const tdesc_compatible_info_up &compat : target_desc->compatible)
     {
-      if (compat == arch
-	  || arch->compatible (arch, compat)
-	  || compat->compatible (compat, arch))
+      if (compat->arch () == arch
+	  || arch->compatible (arch, compat->arch ())
+	  || compat->arch ()->compatible (compat->arch (), arch))
 	return 1;
     }
 
@@ -642,6 +665,22 @@ tdesc_architecture_name (const struct target_desc *target_desc)
   return target_desc->arch->printable_name;
 }
 
+/* See gdbsupport/tdesc.h.  */
+
+const std::vector<tdesc_compatible_info_up> &
+tdesc_compatible_info_list (const target_desc *target_desc)
+{
+  return target_desc->compatible;
+}
+
+/* See gdbsupport/tdesc.h.  */
+
+const char *
+tdesc_compatible_info_arch_name (const tdesc_compatible_info_up &compatible)
+{
+  return compatible->arch ()->printable_name;
+}
+
 /* Return the OSABI associated with this target description, or
    GDB_OSABI_UNKNOWN if no osabi was specified.  */
 
@@ -1158,14 +1197,16 @@ tdesc_add_compatible (struct target_desc *target_desc,
   if (compatible == NULL)
     return;
 
-  for (const bfd_arch_info *compat : target_desc->compatible)
-    if (compat == compatible)
+  for (const tdesc_compatible_info_up &compat : target_desc->compatible)
+    if (compat->arch () == compatible)
       internal_error (__FILE__, __LINE__,
 		      _("Attempted to add duplicate "
 			"compatible architecture \"%s\""),
 		      compatible->printable_name);
 
-  target_desc->compatible.push_back (compatible);
+  target_desc->compatible.push_back
+    (std::unique_ptr<tdesc_compatible_info>
+     (new tdesc_compatible_info (compatible)));
 }
 
 void
@@ -1320,10 +1361,10 @@ class print_c_tdesc : public tdesc_element_visitor
 	printf_unfiltered ("\n");
       }
 
-    for (const bfd_arch_info_type *compatible : e->compatible)
+    for (const tdesc_compatible_info_up &compatible : e->compatible)
       printf_unfiltered
 	("  tdesc_add_compatible (result, bfd_scan_arch (\"%s\"));\n",
-	 compatible->printable_name);
+	 compatible->arch ()->printable_name);
 
     if (!e->compatible.empty ())
       printf_unfiltered ("\n");
diff --git a/gdbserver/tdesc.cc b/gdbserver/tdesc.cc
index 8d97defb9b5..d21688b932b 100644
--- a/gdbserver/tdesc.cc
+++ b/gdbserver/tdesc.cc
@@ -122,6 +122,27 @@ current_target_desc (void)
   return current_process ()->tdesc;
 }
 
+/* An empty structure.  */
+
+struct tdesc_compatible_info { };
+
+/* See gdbsupport/tdesc.h.  */
+
+const std::vector<tdesc_compatible_info_up> &
+tdesc_compatible_info_list (const target_desc *target_desc)
+{
+  static std::vector<tdesc_compatible_info_up> empty;
+  return empty;
+}
+
+/* See gdbsupport/tdesc.h.  */
+
+const char *
+tdesc_compatible_info_arch_name (const tdesc_compatible_info_up &c_info)
+{
+  return nullptr;
+}
+
 /* See gdbsupport/tdesc.h.  */
 
 const char *
diff --git a/gdbsupport/tdesc.cc b/gdbsupport/tdesc.cc
index aaea8e0d8a8..63f41cbf677 100644
--- a/gdbsupport/tdesc.cc
+++ b/gdbsupport/tdesc.cc
@@ -392,6 +392,12 @@ void print_xml_feature::visit_pre (const target_desc *e)
   const char *osabi = tdesc_osabi_name (e);
   if (osabi != nullptr)
     string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
+
+  const std::vector<tdesc_compatible_info_up> &compatible_list
+    = tdesc_compatible_info_list (e);
+  for (const auto &c : compatible_list)
+    string_appendf (*m_buffer, "<compatible>%s</compatible>\n",
+		    tdesc_compatible_info_arch_name (c));
 #endif
 }
 
diff --git a/gdbsupport/tdesc.h b/gdbsupport/tdesc.h
index 3b1f1f57ff8..0cdcf56346c 100644
--- a/gdbsupport/tdesc.h
+++ b/gdbsupport/tdesc.h
@@ -131,6 +131,27 @@ struct tdesc_reg : tdesc_element
 
 typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
 
+/* Declaration of a structure that holds information about one
+   "compatibility" entry within a target description.  */
+
+struct tdesc_compatible_info;
+
+/* A pointer to a single piece of compatibility information.  */
+
+typedef std::unique_ptr<tdesc_compatible_info> tdesc_compatible_info_up;
+
+/* Return a vector of compatibility information pointers from the target
+   description TARGET_DESC.  */
+
+const std::vector<tdesc_compatible_info_up> &tdesc_compatible_info_list
+	(const target_desc *target_desc);
+
+/* Return the architecture name from a compatibility information
+   COMPATIBLE.  */
+
+const char *tdesc_compatible_info_arch_name
+	(const tdesc_compatible_info_up &compatible);
+
 enum tdesc_type_kind
 {
   /* Predefined types.  */
-- 
2.25.4


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

* [PATCHv2 3/3] gdb: New maintenance command to print XML target description
  2020-06-11 23:22 [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
  2020-06-11 23:22 ` [PATCHv2 1/3] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
  2020-06-11 23:22 ` [PATCHv2 2/3] gdb: Print compatible information within print_xml_feature Andrew Burgess
@ 2020-06-11 23:22 ` Andrew Burgess
  2020-06-23 13:48   ` Pedro Alves
  2020-06-22 18:13 ` [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
  3 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2020-06-11 23:22 UTC (permalink / raw)
  To: gdb-patches

This commit adds a new maintenance command that dumps the current
target description as an XML document.  This is a maintenance command
as I currently only see this being useful for GDB developers, or for
people debugging a new remote target.

By default the command will print whatever the current target
description is, whether this was delivered by the remote, loaded by
the user from a file, or if it is a built in target within GDB.

The command can also take an optional filename argument.  In this case
GDB loads a target description from the file, and then reprints it.
This could be useful for testing GDB's parsing of target descriptions,
or to check that GDB can successfully parse a particular XML
description.

It is worth noting that the XML description printed will not be an
exact copy of the document fed into GDB.  For example this minimal
input file:

  <target>
    <feature name="abc">
      <reg name="r1" bitsize="32"/>
    </feature>
  </target>

Will produce this output:

  (gdb) maint print xml-tdesc path/to/file.xml
  <?xml version="1.0"?>
  <!DOCTYPE target SYSTEM "gdb-target.dtd">
  <target>
    <feature name="abc">
      <reg name="r1" bitsize="32" type="int" regnum="0"/>
    </feature>
  </target>

Notice that GDB filled in both the 'type' and 'regnum' fields of the
<reg>.  I think this is actually a positive as it means we get to
really understand how GDB processed the document, if GDB made some
assumptions that differ to those the user expected then hopefully this
will bring those issues to the users attention.

To implement this I have tweaked the output produced by the
print_xml_feature which is defined within the gdbsupport/ directory.
The changes I have made to this class are:

  1. The <architecture>...</architecture> tags are now not produced if
  the architecture name is NULL.

  2. The <osabi>...</osabi> tags get a newline at the end.

  3. And, the whole XML document is indented using white space in a
  nested fashion (as in the example output above).

I think that these changes should be fine, the print_xml_feature class
is used:

  1. In gdbserver to generate an XML document to send as the target
  description to GDB.

  2. In GDB as part of a self-check function, a target_desc is
  converted to XML then parsed back into a target_desc.  We then check
  the before and after target_desc objects are the same.

  3. In the new 'maint print xml-tdesc' command.

In non of these use cases adding the extra white space should be fine.

gdbsupport/ChangeLog:

	* tdesc.cc (print_xml_feature::visit_pre): Use add_line to add
	output content, and call indent as needed in all overloaded
	variants.
	(print_xml_feature::visit_post): Likewise.
	(print_xml_feature::visit): Likewise.
	(print_xml_feature::add_line): Two new overloaded functions.
	* tdesc.h (print_xml_feature::indent): New member function.
	(print_xml_feature::add_line): Two new overloaded member
	functions.
	(print_xml_feature::m_depth): New member variable.

gdb/ChangeLog:

	* target-descriptions.c (tdesc_architecture_name): Protect against
	NULL pointer dereference.
	(maint_print_xml_tdesc_cmd): New function.
	(_initialize_target_descriptions): Register new 'maint print
	xml-tdesc' command and give it the filename completer.
	* NEWS: Mention new 'maint print xml-tdesc' command.

gdb/testsuite/ChangeLog:

	* gdb.xml/tdesc-reload.c: New file.
	* gdb.xml/tdesc-reload.exp: New file.
	* gdb.xml/maint-xml-dump-01.xml: New file.
	* gdb.xml/maint-xml-dump-02.xml: New file.
	* gdb.xml/maint-xml-dump.exp: New file.

gdb/doc/ChangeLog:

	* gdb.texinfo (Maintenance Commands): Document new 'maint print
	xml-desc' command.
---
 gdb/ChangeLog                               |   9 ++
 gdb/NEWS                                    |   6 +
 gdb/doc/ChangeLog                           |   5 +
 gdb/doc/gdb.texinfo                         |   9 ++
 gdb/target-descriptions.c                   |  39 +++++-
 gdb/testsuite/ChangeLog                     |   8 ++
 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml |  10 ++
 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml |  27 +++++
 gdb/testsuite/gdb.xml/maint-xml-dump.exp    | 124 ++++++++++++++++++++
 gdb/testsuite/gdb.xml/tdesc-reload.c        |  22 ++++
 gdb/testsuite/gdb.xml/tdesc-reload.exp      |  83 +++++++++++++
 gdbsupport/ChangeLog                        |  13 ++
 gdbsupport/tdesc.cc                         | 104 +++++++++++-----
 gdbsupport/tdesc.h                          |  23 +++-
 14 files changed, 448 insertions(+), 34 deletions(-)
 create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
 create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
 create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump.exp
 create mode 100644 gdb/testsuite/gdb.xml/tdesc-reload.c
 create mode 100644 gdb/testsuite/gdb.xml/tdesc-reload.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 9ef85ab3ca7..491209b2772 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -66,6 +66,12 @@ tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
 
+maintenance print xml-tdesc [FILE]
+  Prints the current target description as an XML document.  If the
+  optional FILE is provided (which is an XML target description) then
+  the target description is read from FILE into GDB, and then
+  reprinted.
+
 * New targets
 
 GNU/Linux/RISC-V (gdbserver)	riscv*-*-linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 64181628495..add14856f45 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38397,6 +38397,15 @@
 built again.  This command is used by developers after they add or
 modify XML target descriptions.
 
+@kindex maint print xml-tdesc @r{[}@var{file}@r{]}
+@item maint print xml-tdesc
+Print the target description (@pxref{Target Descriptions}) as an XML
+file.  By default print the target description for the current target,
+but if the optional argument @var{file} is provided, then that file is
+read in by GDB and then used to produce the description.  The
+@var{file} should be an XML document, of the form described in
+@ref{Target Description Format}.
+
 @kindex maint check xml-descriptions
 @item maint check xml-descriptions @var{dir}
 Check that the target descriptions dynamically created by @value{GDBN}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 1937e7ca4a7..1abdf51ec72 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -662,7 +662,9 @@ tdesc_architecture (const struct target_desc *target_desc)
 const char *
 tdesc_architecture_name (const struct target_desc *target_desc)
 {
-  return target_desc->arch->printable_name;
+  if (target_desc->arch != NULL)
+    return target_desc->arch->printable_name;
+  return NULL;
 }
 
 /* See gdbsupport/tdesc.h.  */
@@ -1755,6 +1757,36 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
     }
 }
 
+/* Implement the maintenance print xml-tdesc command.  */
+
+static void
+maint_print_xml_tdesc_cmd (const char *args, int from_tty)
+{
+  const struct target_desc *tdesc;
+
+  if (args == NULL)
+    {
+      /* Use the global target-supplied description, not the current
+	 architecture's.  This lets a GDB for one architecture generate XML
+	 for another architecture's description, even though the gdbarch
+	 initialization code will reject the new description.  */
+      tdesc = current_target_desc;
+    }
+  else
+    {
+      /* Use the target description from the XML file.  */
+      tdesc = file_read_description_xml (args);
+    }
+
+  if (tdesc == NULL)
+    error (_("There is no target description to print."));
+
+  std::string buf;
+  print_xml_feature v (&buf);
+  tdesc->accept (v);
+  puts_unfiltered (buf.c_str ());
+}
+
 namespace selftests {
 
 /* A reference target description, used for testing (see record_xml_tdesc).  */
@@ -1892,6 +1924,11 @@ Print the current target description as a C source file."),
 	   &maintenanceprintlist);
   set_cmd_completer (cmd, filename_completer);
 
+  cmd = add_cmd ("xml-tdesc", class_maintenance, maint_print_xml_tdesc_cmd, _("\
+Print the current target description as an XML file."),
+		 &maintenanceprintlist);
+  set_cmd_completer (cmd, filename_completer);
+
   cmd = add_cmd ("xml-descriptions", class_maintenance,
 		 maintenance_check_xml_descriptions, _("\
 Check equality of GDB target descriptions and XML created descriptions.\n\
diff --git a/gdb/testsuite/gdb.xml/maint-xml-dump-01.xml b/gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
new file mode 100644
index 00000000000..dd4f9251676
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<!-- This is a comment before DOCTYPE -->
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<!-- This is a comment after DOCTYPE -->
+<target>
+  <feature name="abc">
+    <!-- The following is a register. -->
+    <reg name="r1" bitsize="32"/>	<!-- <reg name="r1" bitsize="32" type="int" regnum="0"/> -->
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.xml/maint-xml-dump-02.xml b/gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
new file mode 100644
index 00000000000..c192294ca10
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
@@ -0,0 +1,27 @@
+<target>
+  <osabi>Solaris</osabi>
+  <feature name="abc">
+    <vector id="foo" type="int32" count="4"/>
+    <reg name="foo" bitsize="16" />	<!-- <reg name="foo" bitsize="16" type="int" regnum="0"/> -->
+  </feature>
+  <feature name="def.xyz">
+    <struct id="my_struct">
+      <field name="field1" type="int8"/>
+      <field name="field2" type="int16"/>
+      <field name="field3" type="int8"/>
+    </struct>
+    <struct id="bit_field" size="8">
+      <field name="bits1" start="0" end="3" type="int8"/>
+      <field name="bits2" start="4" end="6" type="int8"/>
+      <field name="bits3" start="7" end="7"/>	<!-- <field name="bits3" start="7" end="7" type="bool"/> -->
+    </struct>
+    <flags id="my_flags" size="8">
+      <field name="flg1" start="0" end="0"/>	<!-- <field name="flg1" start="0" end="0" type="bool"/> -->
+      <field name="flg2" start="1" end="1"/>	<!-- <field name="flg2" start="1" end="1" type="bool"/> -->
+      <field name="flg3" start="2" end="6"/>	<!-- <field name="flg3" start="2" end="6" type="uint64"/> -->
+      <field name="flg4" start="7" end="7"/>	<!-- <field name="flg4" start="7" end="7" type="bool"/> -->
+    </flags>
+    <reg name="r1" bitsize="8" type="my_flags"/>	<!-- <reg name="r1" bitsize="8" type="my_flags" regnum="1"/> -->
+    <reg name="r2" bitsize="8" type="bit_field"/>	<!-- <reg name="r2" bitsize="8" type="bit_field" regnum="2"/> -->
+  </feature>
+</target>
diff --git a/gdb/testsuite/gdb.xml/maint-xml-dump.exp b/gdb/testsuite/gdb.xml/maint-xml-dump.exp
new file mode 100644
index 00000000000..8ccfbb5e687
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint-xml-dump.exp
@@ -0,0 +1,124 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test the 'maint print xml-tdesc' command.  This file picks up every
+# XML file matching the pattern maint-xml-dump-*.xml (in the same
+# directory as this script) and passes each in turn to the command
+# 'maint print xml-tdesc'.
+#
+# The expected output is generated by parsing the input XML file.  The
+# rules for changing an XML file into the expected output are:
+#
+# 1. Blank lines, and lines starting with a comment are stripped from
+#    the expected output.
+#
+# 2. The <?xml ... ?> and <!DOCTYPE ...> entities are optional,
+#    suitable defaults will be added if these lines are missing from
+#    the input file.
+#
+# 3. A trailing comment on a line will replace the expected output for
+#    that line but with the indentation of the line preserved.  So
+#    this (The '|' marks the start of the line):
+#    |    <reg name="r1" bitsize="32"/>  <!-- <reg name="r1" bitsize="32" type="int" regnum="0"/> -->
+#    Will actually look for the following output:
+#    |    <reg name="r1" bitsize="32" type="int" regnum="0"/>
+#
+# 4. Indentation of lines will be preserved so your input file needs
+#    to follow the expected indentation.
+if {[gdb_skip_xml_test]} {
+    unsupported "xml tests not being run"
+    return -1
+}
+
+gdb_start
+
+# Read the XML file FILENAME and produce an output pattern that should
+# match what GDB produces with the 'maint print xml-desc' command.
+proc build_pattern { filename } {
+    set pattern {}
+
+    set xml_version_line {<?xml version="1.0"?>}
+    set doc_type_line {<!DOCTYPE target SYSTEM "gdb-target.dtd">}
+
+    set linenum 0
+    set ifd [open "$filename" r]
+    while {[gets $ifd line] >= 0} {
+	incr linenum
+
+	# The <?xml .... ?> tag can only appear as the first line in
+	# the file.  If it is not present then add one to the expected
+	# output now.
+	if {$linenum == 1} {
+	    if {![regexp {^<\?xml} $line]} {
+		set pattern [string_to_regexp $xml_version_line]
+		set xml_version_line ""
+	    }
+	}
+
+	# If we have not yet seen a DOCTYPE line, then maybe we should
+	# be adding one?  If we find <target> then add a default
+	# DOCTYPE line, otherwise, if the XML file includes a DOCTYPE
+	# line, use that.
+	if {$doc_type_line != "" } {
+	    if {[regexp {^[ \t]*<target>} $line]} {
+		set pattern [multi_line $pattern \
+				 [string_to_regexp $doc_type_line]]
+		set doc_type_line ""
+	    } elseif {[regexp {^[ \t]*<!DOCTYPE } $line]} {
+		set doc_type_line ""
+	    }
+	}
+
+	if {[regexp {^[ \t]*<!--} $line]} {
+	    # Comment line, ignore it.
+	} elseif {[regexp {^[ \t]+$} $line]} {
+	    # Blank line, ignore it.
+	} elseif {[regexp {^([ \t]*).*<!-- (.*) -->$} $line \
+		       matches grp1 grp2]} {
+	    set pattern [multi_line \
+			     $pattern \
+			     [string_to_regexp "$grp1$grp2"]]
+	} else {
+	    set pattern [multi_line \
+			     $pattern \
+			     [string_to_regexp $line]]
+	}
+    }
+    close $ifd
+
+    # Due to handling the <?xml ...?> tags we can end up with a stray
+    # '\r\n' at the start of the output pattern.  Remove it here.
+    if {[string range $pattern 0 1] == "\r\n"} {
+	set pattern [string range $pattern 2 end]
+    }
+
+    return $pattern
+}
+
+# Run over every test XML file and check the output.
+foreach filename [lsort [glob $srcdir/$subdir/maint-xml-dump-*.xml]] {
+    set pattern [build_pattern $filename]
+
+    if {[is_remote host]} {
+	set test_path [remote_download host $filename]
+    } else {
+	set test_path $filename
+    }
+
+    verbose -log "Looking for:\n$pattern"
+
+    gdb_test "maint print xml-tdesc $test_path" \
+	"$pattern" "check [file tail $filename]"
+}
diff --git a/gdb/testsuite/gdb.xml/tdesc-reload.c b/gdb/testsuite/gdb.xml/tdesc-reload.c
new file mode 100644
index 00000000000..f4825c8a7c1
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/tdesc-reload.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.xml/tdesc-reload.exp b/gdb/testsuite/gdb.xml/tdesc-reload.exp
new file mode 100644
index 00000000000..a671297f821
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/tdesc-reload.exp
@@ -0,0 +1,83 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Testing for 'maint print xml-tdesc'.  Check we can print out the
+# current target description and load it back in again.
+
+if {[gdb_skip_xml_test]} {
+    unsupported "xml tests not being run"
+    return -1
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+# Three files we're going to write out to.
+set xml_file_1 [standard_output_file outfile1.xml]
+set xml_file_2 [standard_output_file outfile2.xml]
+set xml_file_3 [standard_output_file outfile3.xml]
+
+# Write the current target description to a file.
+gdb_test_no_output "pipe maint print xml-tdesc | cat > $xml_file_1" \
+    "write current target description to file"
+
+# Read the target description back in to GDB, and the write it back
+# out to a file.
+gdb_test_no_output \
+    "pipe maint print xml-tdesc $xml_file_1 | cat > $xml_file_2" \
+    "read previous xml description, and write it out to a second file"
+
+# Check the two produced files are identical.
+gdb_test "shell diff -s $xml_file_1 $xml_file_2" \
+    "Files \[^\r\n\]* are identical" \
+    "first two produced xml files are identical"
+
+# Restart GDB.
+clean_restart
+
+# Change to use one of the target descriptions we wrote out earlier.
+gdb_test_no_output "set tdesc filename $xml_file_1" \
+    "set target description to use"
+
+# Load the executable.
+gdb_load ${binfile}
+
+# Run to `main' where we begin our tests.
+if ![runto_main] then {
+    untested "could not run to main"
+    return -1
+}
+
+# Run info registers just to check this appears to run fine with the
+# new target description.
+gdb_test "info all-registers" ".*" \
+    "Run info registers"
+
+# Write out the current target description.
+gdb_test_no_output "pipe maint print xml-tdesc | cat > $xml_file_3" \
+    "write third target description to file"
+
+# And check that it matches the original file we loaded.
+gdb_test "shell diff -s $xml_file_1 $xml_file_3" \
+    "Files \[^\r\n\]* are identical" \
+    "first and third produced xml files are identical"
diff --git a/gdbsupport/tdesc.cc b/gdbsupport/tdesc.cc
index 63f41cbf677..624588b6563 100644
--- a/gdbsupport/tdesc.cc
+++ b/gdbsupport/tdesc.cc
@@ -294,12 +294,14 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
 
 void print_xml_feature::visit_pre (const tdesc_feature *e)
 {
-  string_appendf (*m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
+  add_line ("<feature name=\"%s\">", e->name.c_str ());
+  indent (1);
 }
 
 void print_xml_feature::visit_post (const tdesc_feature *e)
 {
-  string_appendf (*m_buffer, "</feature>\n");
+  indent (-1);
+  add_line ("</feature>");
 }
 
 void print_xml_feature::visit (const tdesc_type_builtin *t)
@@ -309,8 +311,8 @@ void print_xml_feature::visit (const tdesc_type_builtin *t)
 
 void print_xml_feature::visit (const tdesc_type_vector *t)
 {
-  string_appendf (*m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
-		  t->name.c_str (), t->element_type->name.c_str (), t->count);
+  add_line ("<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
+	    t->name.c_str (), t->element_type->name.c_str (), t->count);
 }
 
 void print_xml_feature::visit (const tdesc_type_with_fields *t)
@@ -319,7 +321,9 @@ void print_xml_feature::visit (const tdesc_type_with_fields *t)
 
   gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
 
-  string_appendf (*m_buffer,
+  std::string tmp;
+
+  string_appendf (tmp,
 		  "<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
 		  t->name.c_str ());
 
@@ -328,33 +332,37 @@ void print_xml_feature::visit (const tdesc_type_with_fields *t)
     case TDESC_TYPE_STRUCT:
     case TDESC_TYPE_FLAGS:
       if (t->size > 0)
-	string_appendf (*m_buffer, " size=\"%d\"", t->size);
-      string_appendf (*m_buffer, ">\n");
+	string_appendf (tmp, " size=\"%d\"", t->size);
+      string_appendf (tmp, ">");
+      add_line (tmp);
 
       for (const tdesc_type_field &f : t->fields)
 	{
-	  string_appendf (*m_buffer, "  <field name=\"%s\" ", f.name.c_str ());
-	  if (f.start == -1)
-	    string_appendf (*m_buffer, "type=\"%s\"/>\n",
-			    f.type->name.c_str ());
-	  else
-	    string_appendf (*m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
+	  tmp.clear ();
+	  string_appendf (tmp, "  <field name=\"%s\"", f.name.c_str ());
+	  if (f.start != -1)
+	    string_appendf (tmp, " start=\"%d\" end=\"%d\"", f.start,
 			    f.end);
+	  string_appendf (tmp, " type=\"%s\"/>",
+			  f.type->name.c_str ());
+	  add_line (tmp);
 	}
       break;
 
     case TDESC_TYPE_ENUM:
-      string_appendf (*m_buffer, ">\n");
+      string_appendf (tmp, ">");
+      add_line (tmp);
       for (const tdesc_type_field &f : t->fields)
-	string_appendf (*m_buffer, "  <field name=\"%s\" start=\"%d\"/>\n",
-			f.name.c_str (), f.start);
+	add_line ("  <field name=\"%s\" start=\"%d\"/>",
+		  f.name.c_str (), f.start);
       break;
 
     case TDESC_TYPE_UNION:
-      string_appendf (*m_buffer, ">\n");
+      string_appendf (tmp, ">");
+      add_line (tmp);
       for (const tdesc_type_field &f : t->fields)
-	string_appendf (*m_buffer, "  <field name=\"%s\" type=\"%s\"/>\n",
-			f.name.c_str (), f.type->name.c_str ());
+	add_line ("  <field name=\"%s\" type=\"%s\"/>",
+		  f.name.c_str (), f.type->name.c_str ());
       break;
 
     default:
@@ -362,46 +370,78 @@ void print_xml_feature::visit (const tdesc_type_with_fields *t)
 	     t->name.c_str ());
     }
 
-  string_appendf (*m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
+  add_line ("</%s>", types[t->kind - TDESC_TYPE_STRUCT]);
 }
 
 void print_xml_feature::visit (const tdesc_reg *r)
 {
-  string_appendf (*m_buffer,
+  std::string tmp;
+
+  string_appendf (tmp,
 		  "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
 		  r->name.c_str (), r->bitsize, r->type.c_str (),
 		  r->target_regnum);
 
   if (r->group.length () > 0)
-    string_appendf (*m_buffer, " group=\"%s\"", r->group.c_str ());
+    string_appendf (tmp, " group=\"%s\"", r->group.c_str ());
 
   if (r->save_restore == 0)
-    string_appendf (*m_buffer, " save-restore=\"no\"");
+    string_appendf (tmp, " save-restore=\"no\"");
 
-  string_appendf (*m_buffer, "/>\n");
+  string_appendf (tmp, "/>");
+
+  add_line (tmp);
 }
 
 void print_xml_feature::visit_pre (const target_desc *e)
 {
 #ifndef IN_PROCESS_AGENT
-  string_appendf (*m_buffer, "<?xml version=\"1.0\"?>\n");
-  string_appendf (*m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
-  string_appendf (*m_buffer, "<target>\n<architecture>%s</architecture>\n",
-		  tdesc_architecture_name (e));
+  add_line ("<?xml version=\"1.0\"?>");
+  add_line ("<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+  add_line ("<target>");
+  indent (1);
+  if (tdesc_architecture_name (e))
+    add_line ("<architecture>%s</architecture>",
+	      tdesc_architecture_name (e));
 
   const char *osabi = tdesc_osabi_name (e);
   if (osabi != nullptr)
-    string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
+    add_line ("<osabi>%s</osabi>", osabi);
 
   const std::vector<tdesc_compatible_info_up> &compatible_list
     = tdesc_compatible_info_list (e);
   for (const auto &c : compatible_list)
-    string_appendf (*m_buffer, "<compatible>%s</compatible>\n",
-		    tdesc_compatible_info_arch_name (c));
+    add_line ("<compatible>%s</compatible>",
+	      tdesc_compatible_info_arch_name (c));
 #endif
 }
 
 void print_xml_feature::visit_post (const target_desc *e)
 {
-  string_appendf (*m_buffer, "</target>\n");
+  indent (-1);
+  add_line ("</target>");
+}
+
+/* See gdbsupport/tdesc.h.  */
+
+void
+print_xml_feature::add_line (const std::string &str)
+{
+  string_appendf (*m_buffer, "%*s", m_depth, "");
+  string_appendf (*m_buffer, "%s", str.c_str ());
+  string_appendf (*m_buffer, "\n");
+}
+
+/* See gdbsupport/tdesc.h.  */
+
+void
+print_xml_feature::add_line (const char *fmt, ...)
+{
+  std::string tmp;
+
+  va_list ap;
+  va_start (ap, fmt);
+  string_vappendf (tmp, fmt, ap);
+  va_end (ap);
+  add_line (tmp);
 }
diff --git a/gdbsupport/tdesc.h b/gdbsupport/tdesc.h
index 0cdcf56346c..73caf24536f 100644
--- a/gdbsupport/tdesc.h
+++ b/gdbsupport/tdesc.h
@@ -410,7 +410,8 @@ class print_xml_feature : public tdesc_element_visitor
 {
 public:
   print_xml_feature (std::string *buffer_)
-    : m_buffer (buffer_)
+    : m_buffer (buffer_),
+      m_depth (0)
   {}
 
   void visit_pre (const target_desc *e) override;
@@ -423,7 +424,27 @@ class print_xml_feature : public tdesc_element_visitor
   void visit (const tdesc_reg *reg) override;
 
 private:
+
+  /* Called with a positive value of ADJUST when we move inside an element,
+     for example inside <target>, and with a negative value when we leave
+     the element.  In this class this function does nothing, but a
+     sub-class can override this to track the current level of nesting.  */
+  void indent (int adjust)
+  {
+    m_depth += (adjust * 2);
+  }
+
+  /* Functions to add lines to the output buffer M_BUFFER.  Each of these
+     functions appends a newline, so don't include one in the strings being
+     passed.  */
+  void add_line (const std::string &str);
+  void add_line (const char *fmt, ...);
+
+  /* The buffer we are writing too.  */
   std::string *m_buffer;
+
+  /* The current indentation depth.  */
+  int m_depth;
 };
 
 #endif /* COMMON_TDESC_H */
-- 
2.25.4


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

* Re: [PATCHv2 0/3] Additional maintenance command for dumping target descriptions
  2020-06-11 23:22 [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
                   ` (2 preceding siblings ...)
  2020-06-11 23:22 ` [PATCHv2 3/3] gdb: New maintenance command to print XML target description Andrew Burgess
@ 2020-06-22 18:13 ` Andrew Burgess
  2020-06-23 13:58   ` Pedro Alves
  3 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2020-06-22 18:13 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves

Pedro,

I wonder if you have any feedback on the changes in this version?
Specifically with how I now handle the 'compatibility' tags in a way
that allows the code to be shared between gdbserver and gdb?

Thanks
Andrew

---


* Andrew Burgess <andrew.burgess@embecosm.com> [2020-06-12 00:22:45 +0100]:

> In the first patch of this series the only change is I addressed
> Pedro's feedback.
> 
> The second patch is new, this moves the printing of <compatible>
> entities into the gdbsupport/ directory, inside the print_xml_feature
> class.
> 
> The third patch is the old second patch.  The documentation has not
> changed.  The code is now simpler thanks to the new #2 patch.  I
> addressed Pedro's feedback including adding some extra tests.
> 
> Feedback welcome.
> 
> Thanks,
> Andrew
> 
> 
> ---
> 
> Andrew Burgess (3):
>   gdb: Allow target description to be dumped even when it is remote
>   gdb: Print compatible information within print_xml_feature
>   gdb: New maintenance command to print XML target description
> 
>  gdb/ChangeLog                               |  30 +++++
>  gdb/NEWS                                    |   6 +
>  gdb/doc/ChangeLog                           |   5 +
>  gdb/doc/gdb.texinfo                         |   9 ++
>  gdb/target-descriptions.c                   | 109 ++++++++++++++---
>  gdb/testsuite/ChangeLog                     |   8 ++
>  gdb/testsuite/gdb.xml/maint-xml-dump-01.xml |  10 ++
>  gdb/testsuite/gdb.xml/maint-xml-dump-02.xml |  27 +++++
>  gdb/testsuite/gdb.xml/maint-xml-dump.exp    | 124 ++++++++++++++++++++
>  gdb/testsuite/gdb.xml/tdesc-reload.c        |  22 ++++
>  gdb/testsuite/gdb.xml/tdesc-reload.exp      |  83 +++++++++++++
>  gdbserver/ChangeLog                         |   6 +
>  gdbserver/tdesc.cc                          |  21 ++++
>  gdbsupport/ChangeLog                        |  22 ++++
>  gdbsupport/tdesc.cc                         | 106 ++++++++++++-----
>  gdbsupport/tdesc.h                          |  44 ++++++-
>  16 files changed, 587 insertions(+), 45 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
>  create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
>  create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump.exp
>  create mode 100644 gdb/testsuite/gdb.xml/tdesc-reload.c
>  create mode 100644 gdb/testsuite/gdb.xml/tdesc-reload.exp
> 
> -- 
> 2.25.4
> 

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

* Re: [PATCHv2 3/3] gdb: New maintenance command to print XML target description
  2020-06-11 23:22 ` [PATCHv2 3/3] gdb: New maintenance command to print XML target description Andrew Burgess
@ 2020-06-23 13:48   ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2020-06-23 13:48 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 6/12/20 12:22 AM, Andrew Burgess wrote:

> I think that these changes should be fine, the print_xml_feature class
> is used:
> 
>   1. In gdbserver to generate an XML document to send as the target
>   description to GDB.
> 
>   2. In GDB as part of a self-check function, a target_desc is
>   converted to XML then parsed back into a target_desc.  We then check
>   the before and after target_desc objects are the same.
> 
>   3. In the new 'maint print xml-tdesc' command.
> 
> In non of these use cases adding the extra white space should be fine.

s/non/none ?  But it doesn't seem like the sentence would make sense then.

Did you mean

 "In all of these use cases",

or

 ".... white space should be a problem?"

instead?

LGTM otherwise.

Thanks,
Pedro Alves


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

* Re: [PATCHv2 0/3] Additional maintenance command for dumping target descriptions
  2020-06-22 18:13 ` [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
@ 2020-06-23 13:58   ` Pedro Alves
  0 siblings, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2020-06-23 13:58 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

Hi Andrew,

On 6/22/20 7:13 PM, Andrew Burgess wrote:
> Pedro,
> 
> I wonder if you have any feedback on the changes in this version?
> Specifically with how I now handle the 'compatibility' tags in a way
> that allows the code to be shared between gdbserver and gdb?
> 

I commented on a commit log typo on patch #3, but otherwise
this all LGTM.  Thanks for confirming.

Pedro Alves


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

end of thread, other threads:[~2020-06-23 13:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 23:22 [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
2020-06-11 23:22 ` [PATCHv2 1/3] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
2020-06-11 23:22 ` [PATCHv2 2/3] gdb: Print compatible information within print_xml_feature Andrew Burgess
2020-06-11 23:22 ` [PATCHv2 3/3] gdb: New maintenance command to print XML target description Andrew Burgess
2020-06-23 13:48   ` Pedro Alves
2020-06-22 18:13 ` [PATCHv2 0/3] Additional maintenance command for dumping target descriptions Andrew Burgess
2020-06-23 13:58   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).