public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH V3 0/8] Remove gdbserver dependency on xml files
@ 2018-03-01 11:38 Alan Hayward
  2018-03-01 11:39 ` [PATCH v3 2/8] Commonise tdesc_reg Alan Hayward
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

V3 builds on previous review comments, and the additional patches I've
already pushed. Complete patch series pushed to branch users/ahayward/xml3.

Summary:

For those targets that use new style target descriptions, this set of patches
removes the dependency on xml files. Namely:
* Removes inclusion of xml files within gdbserver.
* Removes the requirement for the .c files in features/ to be generated from
cached xml files.
This is made possible by changing xml descriptions generated by gdbserver, so
that instead of including xml file names, gdbserver now generate a complete
xml description.

The second point will be required for aarch64 SVE support, where the register
size are variable. Creating SVE xml files for every possible vector length
would not be feasible. Instead the plan for aarch64 SVE is to hand write the
features/ .c code that would normally be generated from xml.


XML Generation:

In existing code, gdbserver uses C code auto generated from xml files to
create target descriptions. When sending an xml description to GDB, the
function tdesc_get_features_xml () creates an xml containing the name of the
original xml file(s). For example:

<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
  <architecture>i386</architecture>
  <osabi>GNU/Linux</osabi>
  <xi:include href="32bit-core.xml"/>
  <xi:include href="32bit-sse.xml"/>
  <xi:include href="32bit-linux.xml"/>
  <xi:include href="32bit-avx.xml"/>
</target>

Upon receipt, GDB then makes requests to gdbserver for the contents of the
xml files. Gdbserver keeps full copies all the xml files inside the binary.

This patch series adds common code that allows gdbserver (and gdb) to turn
a C target description structure into xml.
Now when asked fort an xml description to gdb, gdbserver turns the entire
target description structure back into xml, without using any cached files.
Producing, for example:

<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
  <architecture>i386</architecture>
  <osabi>GNU/Linux</osabi>
  <feature name="org.gnu.gdb.i386.core">
    <flags id="i386_eflags" size="4">
      <field name="CF" start="0" end="0"/>
      <field name="" start="1" end="1"/>
      <field name="PF" start="2" end="2"/>
      <field name="AF" start="4" end="4"/>
...etc...


Patch Contents:

Patches 2-4 commonise the various target descriptor functionality, allowing
gdbserver to parse target descriptions in the same way as gdb. This series
does not commonise target_desc, but this is hopefully a long term goal.

The sixth patch adds the xml printer, which iterates through the parsing
generated in the previous patches.

The other patches are clean up patches.



Patches have been tested on a make check on x86 targets=all build with
target board unix native-gdbserver. Also tested aarch64. Built for power
(because it does not use new target descriptions), but am unable to test.
In addition, patch six adds new test cases to unit test.

Alan.

 gdb/Makefile.in                    |   2 +
 gdb/common/tdesc.c                 | 445 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/features/aarch64-core.c        |   2 +-
 gdb/features/aarch64-fpu.c         |   2 +-
 gdb/features/i386/32bit-avx.c      |   2 +-
 gdb/features/i386/32bit-avx512.c   |   2 +-
 gdb/features/i386/32bit-core.c     |   2 +-
 gdb/features/i386/32bit-linux.c    |   2 +-
 gdb/features/i386/32bit-mpx.c      |   2 +-
 gdb/features/i386/32bit-pkeys.c    |   2 +-
 gdb/features/i386/32bit-sse.c      |   2 +-
 gdb/features/i386/64bit-avx.c      |   2 +-
 gdb/features/i386/64bit-avx512.c   |   2 +-
 gdb/features/i386/64bit-core.c     |   2 +-
 gdb/features/i386/64bit-linux.c    |   2 +-
 gdb/features/i386/64bit-mpx.c      |   2 +-
 gdb/features/i386/64bit-pkeys.c    |   2 +-
 gdb/features/i386/64bit-segments.c |   2 +-
 gdb/features/i386/64bit-sse.c      |   2 +-
 gdb/features/i386/x32-core.c       |   2 +-
 gdb/features/tic6x-c6xp.c          |   2 +-
 gdb/features/tic6x-core.c          |   2 +-
 gdb/features/tic6x-gp.c            |   2 +-
 gdb/gdbserver/Makefile.in          |   3 +
 gdb/gdbserver/configure.srv        |  36 -------
 gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------------------
 gdb/gdbserver/tdesc.h              |  57 ++--------
 gdb/regformats/regdat.sh           |   5 +-
 gdb/target-descriptions.c          | 596 ++++++++++++--------------------------------------------------------------------------------------------
 gdb/xml-tdesc.c                    |   9 ++
 gdb/xml-tdesc.h                    |   5 +
 32 files changed, 974 insertions(+), 779 deletions(-)

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

* [PATCH v3 2/8] Commonise tdesc_reg
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
@ 2018-03-01 11:39 ` Alan Hayward
  2018-03-12 17:20   ` Philipp Rudo
  2018-03-01 11:39 ` [PATCH V3 1/8] Move tdesc header funcs to c file Alan Hayward
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.

gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.

However, other modules in gdbserver directly access the reg_defs structure.
To work around this, init_target_desc fills in reg_defs by parsing the
tdesc_reg vector.
The long term goal is to remove reg_defs, replacing with accessor funcs.

I wanted to make tdesc_create_reg common, but I cannot do that until
the next patch.

I also had to commonise tdesc_element_visitor and tdesc_element.


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

gdb/
	* Makefile.in: Add arch/tdesc.c
	* common/tdesc.c: New file.
	* common/tdesc.h (tdesc_element_visitor): Move to here.
	(tdesc_element): Likewise.
	(tdesc_reg): Likewise.
	(tdesc_reg_up): Likewise.	
	* target-descriptions.c: (tdesc_element_visitor): Move from here. 
	(tdesc_element): Likewise.
	(tdesc_reg): Likewise.
	(tdesc_reg_up): Likewise.	

gdbserver/
	* Makefile.in: Add common/tdesc.c
	* tdesc.c (init_target_desc): init all reg_defs from register vector.
	(tdesc_create_reg): Create tdesc_reg.
	* tdesc.h (tdesc_feature): Add register vector.


diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 1c58b9270d63b3bc30ebd61ae725149b2e5e760a..59d47858bbf4b2a3dc34499dc126b18af717c368 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -961,6 +961,7 @@ COMMON_SFILES = \
 	common/run-time-clock.c \
 	common/signals.c \
 	common/signals-state-save-restore.c \
+	common/tdesc.c \
 	common/vec.c \
 	common/xml-utils.c \
 	complaints.c \
@@ -1436,6 +1437,7 @@ HFILES_NO_SRCDIR = \
 	common/run-time-clock.h \
 	common/signals-state-save-restore.h \
 	common/symbol.h \
+	common/tdesc.h \
 	common/vec.h \
 	common/version.h \
 	common/x86-xstate.h \
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index cc11651dcaa7abe81598b69f509ef34ff0d94dbf..ac6dfdf7bb4ec2c3bbbed1b091f04308dd741f21 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -18,6 +18,12 @@
 #ifndef ARCH_TDESC_H
 #define ARCH_TDESC_H 1

+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
 struct tdesc_feature;
 struct tdesc_type;
 struct tdesc_type_builtin;
@@ -26,6 +32,111 @@ struct tdesc_type_with_fields;
 struct tdesc_reg;
 struct target_desc;

+/* The interface to visit different elements of target description.  */
+
+class tdesc_element_visitor
+{
+public:
+  virtual void visit_pre (const target_desc *e)
+  {}
+
+  virtual void visit_post (const target_desc *e)
+  {}
+
+  virtual void visit_pre (const tdesc_feature *e)
+  {}
+
+  virtual void visit_post (const tdesc_feature *e)
+  {}
+
+  virtual void visit (const tdesc_type_builtin *e)
+  {}
+
+  virtual void visit (const tdesc_type_vector *e)
+  {}
+
+  virtual void visit (const tdesc_type_with_fields *e)
+  {}
+
+  virtual void visit (const tdesc_reg *e)
+  {}
+};
+
+class tdesc_element
+{
+public:
+  virtual void accept (tdesc_element_visitor &v) const = 0;
+};
+
+/* An individual register from a target description.  */
+
+struct tdesc_reg : tdesc_element
+{
+  tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
+	     int regnum, int save_restore_, const char *group_,
+	     int bitsize_, const char *type_);
+
+  virtual ~tdesc_reg () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_reg);
+
+  /* The name of this register.  In standard features, it may be
+     recognized by the architecture support code, or it may be purely
+     for the user.  */
+  std::string name;
+
+  /* The register number used by this target to refer to this
+     register.  This is used for remote p/P packets and to determine
+     the ordering of registers in the remote g/G packets.  */
+  long target_regnum;
+
+  /* If this flag is set, GDB should save and restore this register
+     around calls to an inferior function.  */
+  int save_restore;
+
+  /* The name of the register group containing this register, or empty
+     if the group should be automatically determined from the
+     register's type.  If this is "general", "float", or "vector", the
+     corresponding "info" command should display this register's
+     value.  It can be an arbitrary string, but should be limited to
+     alphanumeric characters and internal hyphens.  Currently other
+     strings are ignored (treated as empty).  */
+  std::string group;
+
+  /* The size of the register, in bits.  */
+  int bitsize;
+
+  /* The type of the register.  This string corresponds to either
+     a named type from the target description or a predefined
+     type from GDB.  */
+  std::string type;
+
+  /* The target-described type corresponding to TYPE, if found.  */
+  struct tdesc_type *tdesc_type;
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+
+  bool operator== (const tdesc_reg &other) const
+  {
+    return (name == other.name
+       && target_regnum == other.target_regnum
+       && save_restore == other.save_restore
+       && bitsize == other.bitsize
+       && group == other.group
+       && type == other.type);
+  }
+
+  bool operator!= (const tdesc_reg &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
+
 /* Allocate a new target_desc.  */
 target_desc *allocate_target_description (void);

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
new file mode 100644
index 0000000000000000000000000000000000000000..a5f2de4b5ba2adb61dd098d8fc569a6339164586
--- /dev/null
+++ b/gdb/common/tdesc.c
@@ -0,0 +1,40 @@
+/* Target description support for GDB.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include "common/tdesc.h"
+
+tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
+		      int regnum, int save_restore_, const char *group_,
+		      int bitsize_, const char *type_)
+  : name (name_), target_regnum (regnum),
+    save_restore (save_restore_),
+    group (group_ != NULL ? group_ : ""),
+    bitsize (bitsize_),
+    type (type_ != NULL ? type_ : "<unknown>")
+{
+  /* If the register's type is target-defined, look it up now.  We may not
+     have easy access to the containing feature when we want it later.  */
+  tdesc_type = tdesc_named_type (feature, type.c_str ());
+}
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 2dbf9ae63df15502d038bcd26af6963dfa87e739..ce09b492646a810a554485372648c928bbae3cdc 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -212,6 +212,7 @@ SFILES = \
 	$(srcdir)/common/print-utils.c \
 	$(srcdir)/common/ptid.c \
 	$(srcdir)/common/rsp-low.c \
+	$(srcdir)/common/tdesc.c \
 	$(srcdir)/common/vec.c \
 	$(srcdir)/common/xml-utils.c \
 	$(srcdir)/nat/linux-btrace.c \
@@ -254,6 +255,7 @@ OBS = \
 	common/rsp-low.o \
 	common/signals.o \
 	common/signals-state-save-restore.o \
+	common/tdesc.o \
 	common/vec.o \
 	common/xml-utils.o \
 	debug.o \
@@ -397,6 +399,7 @@ IPA_OBJS = \
 	common/format-ipa.o \
 	common/print-utils-ipa.o \
 	common/rsp-low-ipa.o \
+	common/tdesc-ipa.o \
 	common/vec-ipa.o \
 	regcache-ipa.o \
 	remote-utils-ipa.o \
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 4513ea74232a456cc86eb9a655904012ff117373..b0b8fa3f9e63590398d0de52359ae8ee0271557b 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -25,7 +25,10 @@
 #include <vector>

 struct tdesc_feature
-{};
+{
+  /* The registers associated with this feature.  */
+  std::vector<tdesc_reg_up> registers;
+};

 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e50a848e2f9f280a84ab139cfce4d1f17bd05884..806e87da185623da29d39d9fd74c8722f795e086 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -72,9 +72,27 @@ init_target_desc (struct target_desc *tdesc)
 {
   int offset = 0;

-  for (reg *reg : tdesc->reg_defs)
+  /* Go through all the features and populate reg_defs.  */
+  for (const tdesc_reg_up &treg : tdesc->registers)
     {
+      /* Fill in any blank spaces.  */
+      while (tdesc->reg_defs.size () < treg->target_regnum)
+	{
+	  struct reg *reg = XCNEW (struct reg);
+	  reg->name = "";
+	  reg->size = 0;
+	  reg->offset = offset;
+	  tdesc->reg_defs.push_back (reg);
+	}
+
+      gdb_assert (treg->target_regnum == 0
+		  || treg->target_regnum == tdesc->reg_defs.size ());
+
+      struct reg *reg = XCNEW (struct reg);
+      reg->name = treg->name.c_str ();
+      reg->size = treg->bitsize;
       reg->offset = offset;
+      tdesc->reg_defs.push_back (reg);
       offset += reg->size;
     }

@@ -241,23 +259,10 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 {
   struct target_desc *tdesc = (struct target_desc *) feature;

-  while (tdesc->reg_defs.size () < regnum)
-    {
-      struct reg *reg = XCNEW (struct reg);
-
-      reg->name = "";
-      reg->size = 0;
-      tdesc->reg_defs.push_back (reg);
-    }
-
-  gdb_assert (regnum == 0
-	      || regnum == tdesc->reg_defs.size ());
-
-  struct reg *reg = XCNEW (struct reg);
+  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+				  group, bitsize, type);

-  reg->name = name;
-  reg->size = bitsize;
-  tdesc->reg_defs.push_back (reg);
+  tdesc->registers.emplace_back (reg);
 }

 /* See common/tdesc.h.  */
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 5d34e29a867f8d39cd0451eb48cbf39614c9acba..3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -38,44 +38,6 @@
 #include "completer.h"
 #include "readline/tilde.h" /* tilde_expand */

-static type *make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype);
-
-/* The interface to visit different elements of target description.  */
-
-class tdesc_element_visitor
-{
-public:
-  virtual void visit_pre (const target_desc *e)
-  {}
-
-  virtual void visit_post (const target_desc *e)
-  {}
-
-  virtual void visit_pre (const tdesc_feature *e)
-  {}
-
-  virtual void visit_post (const tdesc_feature *e)
-  {}
-
-  virtual void visit (const tdesc_type_builtin *e)
-  {}
-
-  virtual void visit (const tdesc_type_vector *e)
-  {}
-
-  virtual void visit (const tdesc_type_with_fields *e)
-  {}
-
-  virtual void visit (const tdesc_reg *e)
-  {}
-};
-
-class tdesc_element
-{
-public:
-  virtual void accept (tdesc_element_visitor &v) const = 0;
-};
-
 /* Types.  */

 struct property
@@ -88,84 +50,6 @@ struct property
   std::string value;
 };

-/* An individual register from a target description.  */
-
-struct tdesc_reg : tdesc_element
-{
-  tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
-	     int regnum, int save_restore_, const char *group_,
-	     int bitsize_, const char *type_)
-    : name (name_), target_regnum (regnum),
-      save_restore (save_restore_),
-      group (group_ != NULL ? group_ : ""),
-      bitsize (bitsize_),
-      type (type_ != NULL ? type_ : "<unknown>")
-  {
-    /* If the register's type is target-defined, look it up now.  We may not
-       have easy access to the containing feature when we want it later.  */
-    tdesc_type = tdesc_named_type (feature, type.c_str ());
-  }
-
-  virtual ~tdesc_reg () = default;
-
-  DISABLE_COPY_AND_ASSIGN (tdesc_reg);
-
-  /* The name of this register.  In standard features, it may be
-     recognized by the architecture support code, or it may be purely
-     for the user.  */
-  std::string name;
-
-  /* The register number used by this target to refer to this
-     register.  This is used for remote p/P packets and to determine
-     the ordering of registers in the remote g/G packets.  */
-  long target_regnum;
-
-  /* If this flag is set, GDB should save and restore this register
-     around calls to an inferior function.  */
-  int save_restore;
-
-  /* The name of the register group containing this register, or empty
-     if the group should be automatically determined from the register's
-     type.  This is traditionally "general", "float", "vector" but can
-     also be an arbitrary string.  If defined the corresponding "info"
-     command should display this register's value.  The string should be
-     limited to alphanumeric characters and internal hyphens.  */
-  std::string group;
-
-  /* The size of the register, in bits.  */
-  int bitsize;
-
-  /* The type of the register.  This string corresponds to either
-     a named type from the target description or a predefined
-     type from GDB.  */
-  std::string type;
-
-  /* The target-described type corresponding to TYPE, if found.  */
-  struct tdesc_type *tdesc_type;
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-
-  bool operator== (const tdesc_reg &other) const
-  {
-    return (name == other.name
-	    && target_regnum == other.target_regnum
-	    && save_restore == other.save_restore
-	    && bitsize == other.bitsize
-	    && group == other.group
-	    && type == other.type);
-  }
-
-  bool operator!= (const tdesc_reg &other) const
-  {
-    return !(*this == other);
-  }
-};
-
-typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
-
 /* A named type from a target description.  */

 struct tdesc_type_field

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

* [PATCH V3 1/8] Move tdesc header funcs to c file
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
  2018-03-01 11:39 ` [PATCH v3 2/8] Commonise tdesc_reg Alan Hayward
@ 2018-03-01 11:39 ` Alan Hayward
  2018-03-01 11:40 ` [PATCH v3 4/8] Commonise tdesc types Alan Hayward
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

Move the destructor and equals operator for gdbserver tdesc from the .h
to the .c file. Both functions are too long to be inlined and make the
header look messy. Patch does not change any functionality.


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

gdbserver/
	* tdesc.c (target_desc::~target_desc): Move to here.
	(target_desc::operator==): Likewise.
	* tdesc.h (target_desc::~target_desc): Move from here.
	(target_desc::operator==): Likewise.


diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index d21574ca73e5042d560adad0e3839e89ee2c67ab..4513ea74232a456cc86eb9a655904012ff117373 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -67,49 +67,9 @@ public:
     : registers_size (0)
   {}

-  ~target_desc ()
-  {
-    int i;
-
-    for (reg *reg : reg_defs)
-      xfree (reg);
-
-    xfree ((char *) arch);
-    xfree ((char *) osabi);
-
-    char *f;
+  ~target_desc ();

-    for (i = 0; VEC_iterate (char_ptr, features, i, f); i++)
-      xfree (f);
-    VEC_free (char_ptr, features);
-  }
-
-  bool operator== (const target_desc &other) const
-  {
-    if (reg_defs.size () != other.reg_defs.size ())
-      return false;
-
-    for (int i = 0; i < reg_defs.size (); ++i)
-      {
-	struct reg *reg = reg_defs[i];
-	struct reg *reg2 = other.reg_defs[i];
-
-	if (reg != reg2 && *reg != *reg2)
-	  return false;
-      }
-
-    /* Compare expedite_regs.  */
-    int i = 0;
-    for (; expedite_regs[i] != NULL; i++)
-      {
-	if (strcmp (expedite_regs[i], other.expedite_regs[i]) != 0)
-	  return false;
-      }
-    if (other.expedite_regs[i] != NULL)
-      return false;
-
-    return true;
-  }
+  bool operator== (const target_desc &other) const;

   bool operator!= (const target_desc &other) const
   {
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 00a5e8dc4d6fe3c0809194c04902f96056622c82..e50a848e2f9f280a84ab139cfce4d1f17bd05884 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -19,6 +19,54 @@
 #include "tdesc.h"
 #include "regdef.h"

+#ifndef IN_PROCESS_AGENT
+
+target_desc::~target_desc ()
+{
+  int i;
+
+  for (reg *reg : reg_defs)
+    xfree (reg);
+
+  xfree ((char *) arch);
+  xfree ((char *) osabi);
+
+  char *f;
+
+  for (i = 0; VEC_iterate (char_ptr, features, i, f); i++)
+    xfree (f);
+  VEC_free (char_ptr, features);
+}
+
+bool target_desc::operator== (const target_desc &other) const
+{
+  if (reg_defs.size () != other.reg_defs.size ())
+    return false;
+
+  for (int i = 0; i < reg_defs.size (); ++i)
+    {
+      struct reg *reg = reg_defs[i];
+      struct reg *reg2 = other.reg_defs[i];
+
+      if (reg != reg2 && *reg != *reg2)
+	return false;
+    }
+
+  /* Compare expedite_regs.  */
+  int i = 0;
+  for (; expedite_regs[i] != NULL; i++)
+    {
+      if (strcmp (expedite_regs[i], other.expedite_regs[i]) != 0)
+	return false;
+    }
+  if (other.expedite_regs[i] != NULL)
+    return false;
+
+  return true;
+}
+
+#endif
+
 void
 init_target_desc (struct target_desc *tdesc)
 {

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

* [PATCH v3 5/8] Add tdesc osabi and architecture functions
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (3 preceding siblings ...)
  2018-03-01 11:40 ` [PATCH v3 3/8] Commonise tdesc_feature Alan Hayward
@ 2018-03-01 11:40 ` Alan Hayward
  2018-03-01 11:41 ` [PATCH v3 8/8] Remove xml files from gdbserver Alan Hayward
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

Add functions to access to printable names for osabi and architecture in
target_desc.

I wanted to add these as member functions of target_desc, but cannot until
target_desc is moved into the header files.

Alan.

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

gdb/
	* common/tdesc.h (tdesc_architecture_name): Add new declaration.
	(tdesc_osabi_name): Likewise.
	* target-descriptions.c (tdesc_architecture_name): Add new function.
	(tdesc_osabi_name): Likewise.

gdb/gdbserver/
	* tdesc.c (tdesc_architecture_name): Add new function.
	(tdesc_osabi_name): Likewise.
	(tdesc_get_features_xml): Use new functions.


diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 14f7bc3a8f248adff04089438ad61c16e83385d0..c0d2a10b0f7ba4e7b836e3b163d229f1608b02d8 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -304,9 +304,18 @@ target_desc *allocate_target_description (void);
 void set_tdesc_architecture (target_desc *target_desc,
 			     const char *name);

+/* Return the architecture associated with this target description as a string,
+   or NULL if no architecture was specified.  */
+const char *tdesc_architecture_name (const struct target_desc *target_desc);
+
 /* Set TARGET_DESC's osabi by NAME.  */
 void set_tdesc_osabi (target_desc *target_desc, const char *name);

+/* Return the osabi associated with this target description as a string,
+   or NULL if no osabi was specified.  */
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc);
+
 /* Return the type associated with ID in the context of FEATURE, or
    NULL if none.  */
 struct tdesc_type *tdesc_named_type (const struct tdesc_feature *feature,
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 1d9aeed217da37fee7220845ff96085dde877876..e11344762a3a4114ed9aa459d7d739bd96a90ae5 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -127,6 +127,14 @@ current_target_desc (void)

 /* See common/tdesc.h.  */

+const char *
+tdesc_architecture_name (const struct target_desc *target_desc)
+{
+  return target_desc->arch;
+}
+
+/* See common/tdesc.h.  */
+
 void
 set_tdesc_architecture (struct target_desc *target_desc,
 			const char *name)
@@ -136,6 +144,14 @@ set_tdesc_architecture (struct target_desc *target_desc,

 /* See common/tdesc.h.  */

+const char *
+tdesc_osabi_name (const struct target_desc *target_desc)
+{
+  return target_desc->osabi;
+}
+
+/* See common/tdesc.h.  */
+
 void
 set_tdesc_osabi (struct target_desc *target_desc, const char *name)
 {
@@ -160,13 +176,14 @@ tdesc_get_features_xml (target_desc *tdesc)
       buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
       buffer += "<target>";
       buffer += "<architecture>";
-      buffer += tdesc->arch;
+      buffer += tdesc_architecture_name (tdesc);
       buffer += "</architecture>";

-      if (tdesc->osabi != nullptr)
+      const char *osabi = tdesc_osabi_name (tdesc);
+      if (osabi != nullptr)
 	{
 	  buffer += "<osabi>";
-	  buffer += tdesc->osabi;
+	  buffer += osabi;
 	  buffer += "</osabi>";
 	}

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 2782ffaab9355e5a74da45e326ad468ff6bed796..da2c1ce34531c1b23281c42f2dacbc85444ef544 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -628,6 +628,14 @@ tdesc_architecture (const struct target_desc *target_desc)
   return target_desc->arch;
 }

+/* See common/tdesc.h.  */
+
+const char *
+tdesc_architecture_name (const struct target_desc *target_desc)
+{
+  return target_desc->arch->printable_name;
+}
+
 /* Return the OSABI associated with this target description, or
    GDB_OSABI_UNKNOWN if no osabi was specified.  */

@@ -637,7 +645,16 @@ tdesc_osabi (const struct target_desc *target_desc)
   return target_desc->osabi;
 }

-

+/* See common/tdesc.h.  */
+
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc)
+{
+  enum gdb_osabi osabi = tdesc_osabi (target_desc);
+  if (osabi > GDB_OSABI_UNKNOWN && osabi < GDB_OSABI_INVALID)
+    return gdbarch_osabi_name (osabi);
+  return nullptr;
+}

 /* Return 1 if this target description includes any registers.  */

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

* [PATCH v3 4/8] Commonise tdesc types
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
  2018-03-01 11:39 ` [PATCH v3 2/8] Commonise tdesc_reg Alan Hayward
  2018-03-01 11:39 ` [PATCH V3 1/8] Move tdesc header funcs to c file Alan Hayward
@ 2018-03-01 11:40 ` Alan Hayward
  2018-03-01 11:40 ` [PATCH v3 3/8] Commonise tdesc_feature Alan Hayward
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

This patch moves all the various tdesc types to common code.

It also commonises all the tdesc_ functions that are stubbed
out in gdbserver.

This is a lot of code to move across, but it is the simplest
way of getting gdbserver to retain the target description
information.

With this patch, gdb and gdbserver will now parse a target
description in the same way.

Alan.

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

gdb/
	* common/tdesc.c (tdesc_predefined_type): Move to here.
	(tdesc_named_type): Likewise.
	(tdesc_create_vector): Likewise.
	(tdesc_create_struct): Likewise.
	(tdesc_set_struct_size): Likewise.
	(tdesc_create_union): Likewise.
	(tdesc_create_flags): Likewise.
	(tdesc_create_enum): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_flag): Likewise.
	(tdesc_add_enum_value): Likewise.
	* common/tdesc.h (tdesc_type_builtin): Likewise.
	(tdesc_type_vector): Likewise.
	(tdesc_type_field): Likewise.
	(tdesc_type_with_fields): Likewise.
	* target-descriptions.c (tdesc_type_field): Move from here.
	(tdesc_type_builtin): Likewise.
	(tdesc_type_vector): Likewise.
	(tdesc_type_with_fields): Likewise.
	(tdesc_predefined_types): Likewise.
	(tdesc_named_type): Likewise.
	(tdesc_create_vector): Likewise.
	(tdesc_create_struct): Likewise.
	(tdesc_set_struct_size): Likewise.
	(tdesc_create_union): Likewise.
	(tdesc_create_flags): Likewise.
	(tdesc_create_enum): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_add_typed_bitfield): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_flag): Likewise.
	(tdesc_add_enum_value): Likewise.

gdbserver/
	* tdesc.c (tdesc_create_flags): Remove.
	(tdesc_add_flag): Likewise.
	(tdesc_named_type): Likewise.
	(tdesc_create_union): Likewise.
	(tdesc_create_struct): Likewise.
	(tdesc_create_vector): Likewise.
	(tdesc_add_bitfield): Likewise.
	(tdesc_add_field): Likewise.
	(tdesc_set_struct_size): Likewise.


diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 07083e3d1d41c4f86f85ae032b45c1d6f6a105ff..14f7bc3a8f248adff04089438ad61c16e83385d0 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -196,6 +196,72 @@ struct tdesc_type : tdesc_element

 typedef std::unique_ptr<tdesc_type> tdesc_type_up;

+struct tdesc_type_builtin : tdesc_type
+{
+  tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
+  : tdesc_type (name, kind)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+};
+
+/* tdesc_type for vector types.  */
+
+struct tdesc_type_vector : tdesc_type
+{
+  tdesc_type_vector (const std::string &name, tdesc_type *element_type_,
+		     int count_)
+  : tdesc_type (name, TDESC_TYPE_VECTOR),
+    element_type (element_type_), count (count_)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+
+  struct tdesc_type *element_type;
+  int count;
+};
+
+/* A named type from a target description.  */
+
+struct tdesc_type_field
+{
+  tdesc_type_field (const std::string &name_, tdesc_type *type_,
+		    int start_, int end_)
+  : name (name_), type (type_), start (start_), end (end_)
+  {}
+
+  std::string name;
+  struct tdesc_type *type;
+  /* For non-enum-values, either both are -1 (non-bitfield), or both are
+     not -1 (bitfield).  For enum values, start is the value (which could be
+     -1), end is -1.  */
+  int start, end;
+};
+
+/* tdesc_type for struct, union, flags, and enum types.  */
+
+struct tdesc_type_with_fields : tdesc_type
+{
+  tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
+			  int size_ = 0)
+  : tdesc_type (name, kind), size (size_)
+  {}
+
+  void accept (tdesc_element_visitor &v) const override
+  {
+    v.visit (this);
+  }
+
+  std::vector<tdesc_type_field> fields;
+  int size;
+};
+
 /* A feature from a target description.  Each feature is a collection
    of other elements, e.g. registers and types.  */

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index ef6ed944df20d4613dd0dd338710a4c1ad67bbf2..115e7cd77942b86dedced946773d1d71950e2bb3 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -39,6 +39,28 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
   tdesc_type = tdesc_named_type (feature, type.c_str ());
 }

+/* Predefined types.  */
+static tdesc_type_builtin tdesc_predefined_types[] =
+{
+  { "bool", TDESC_TYPE_BOOL },
+  { "int8", TDESC_TYPE_INT8 },
+  { "int16", TDESC_TYPE_INT16 },
+  { "int32", TDESC_TYPE_INT32 },
+  { "int64", TDESC_TYPE_INT64 },
+  { "int128", TDESC_TYPE_INT128 },
+  { "uint8", TDESC_TYPE_UINT8 },
+  { "uint16", TDESC_TYPE_UINT16 },
+  { "uint32", TDESC_TYPE_UINT32 },
+  { "uint64", TDESC_TYPE_UINT64 },
+  { "uint128", TDESC_TYPE_UINT128 },
+  { "code_ptr", TDESC_TYPE_CODE_PTR },
+  { "data_ptr", TDESC_TYPE_DATA_PTR },
+  { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
+  { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
+  { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
+  { "i387_ext", TDESC_TYPE_I387_EXT }
+};
+
 void tdesc_feature::accept (tdesc_element_visitor &v) const
 {
   v.visit_pre (this);
@@ -84,6 +106,36 @@ bool tdesc_feature::operator== (const tdesc_feature &other) const
   return true;
 }

+/* Lookup a predefined type.  */
+
+static struct tdesc_type *
+tdesc_predefined_type (enum tdesc_type_kind kind)
+{
+  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+    if (tdesc_predefined_types[ix].kind == kind)
+      return &tdesc_predefined_types[ix];
+
+  gdb_assert_not_reached ("bad predefined tdesc type");
+}
+
+/* See common/tdesc.h.  */
+
+struct tdesc_type *
+tdesc_named_type (const struct tdesc_feature *feature, const char *id)
+{
+  /* First try target-defined types.  */
+  for (const tdesc_type_up &type : feature->types)
+    if (type->name == id)
+      return type.get ();
+
+  /* Next try the predefined types.  */
+  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+    if (tdesc_predefined_types[ix].name == id)
+      return &tdesc_predefined_types[ix];
+
+  return NULL;
+}
+
 /* See common/tdesc.h.  */

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

   feature->registers.emplace_back (reg);
 }
+
+/* See common/tdesc.h.  */
+
+struct tdesc_type *
+tdesc_create_vector (struct tdesc_feature *feature, const char *name,
+		     struct tdesc_type *field_type, int count)
+{
+  tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_struct (struct tdesc_feature *feature, const char *name)
+{
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
+{
+  gdb_assert (type->kind == TDESC_TYPE_STRUCT);
+  gdb_assert (size > 0);
+  type->size = size;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_union (struct tdesc_feature *feature, const char *name)
+{
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+tdesc_type_with_fields *
+tdesc_create_flags (struct tdesc_feature *feature, const char *name,
+		    int size)
+{
+  gdb_assert (size > 0);
+
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+tdesc_type_with_fields *
+tdesc_create_enum (struct tdesc_feature *feature, const char *name,
+		   int size)
+{
+  gdb_assert (size > 0);
+
+  tdesc_type_with_fields *type
+    = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
+  feature->types.emplace_back (type);
+
+  return type;
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
+		 struct tdesc_type *field_type)
+{
+  gdb_assert (type->kind == TDESC_TYPE_UNION
+	      || type->kind == TDESC_TYPE_STRUCT);
+
+  /* Initialize start and end so we know this is not a bit-field
+     when we print-c-tdesc.  */
+  type->fields.emplace_back (field_name, field_type, -1, -1);
+}
+
+void
+tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
+			  int start, int end, struct tdesc_type *field_type)
+{
+  gdb_assert (type->kind == TDESC_TYPE_STRUCT
+	      || type->kind == TDESC_TYPE_FLAGS);
+  gdb_assert (start >= 0 && end >= start);
+
+  type->fields.emplace_back (field_name, field_type, start, end);
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
+		    int start, int end)
+{
+  struct tdesc_type *field_type;
+
+  gdb_assert (start >= 0 && end >= start);
+
+  if (type->size > 4)
+    field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
+  else
+    field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
+
+  tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_add_flag (tdesc_type_with_fields *type, int start,
+		const char *flag_name)
+{
+  gdb_assert (type->kind == TDESC_TYPE_FLAGS
+	      || type->kind == TDESC_TYPE_STRUCT);
+
+  type->fields.emplace_back (flag_name,
+			     tdesc_predefined_type (TDESC_TYPE_BOOL),
+			     start, start);
+}
+
+void
+tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
+		      const char *name)
+{
+  gdb_assert (type->kind == TDESC_TYPE_ENUM);
+  type->fields.emplace_back (name,
+			     tdesc_predefined_type (TDESC_TYPE_INT32),
+			     value, -1);
+}
\ No newline at end of file
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 72769ff00fd073d93807a4aaa59ec19ec6292641..1d9aeed217da37fee7220845ff96085dde877876 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -197,71 +197,3 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
   tdesc->features.emplace_back (new_feature);
   return new_feature;
 }
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
-		    int size)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
-		const char *flag_name)
-{}
-
-/* See common/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *id)
-{
-  return NULL;
-}
-
-/* See common/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
-		     struct tdesc_type *field_type, int count)
-{
-  return NULL;
-}
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
-		    int start, int end)
-{}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
-		 struct tdesc_type *field_type)
-{}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
-}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index a453eddec0cd97b692c4c9ace3f093c3fbc67422..2782ffaab9355e5a74da45e326ad468ff6bed796 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -50,71 +50,6 @@ struct property
   std::string value;
 };

-/* A named type from a target description.  */
-
-struct tdesc_type_field
-{
-  tdesc_type_field (const std::string &name_, tdesc_type *type_,
-		    int start_, int end_)
-  : name (name_), type (type_), start (start_), end (end_)
-  {}
-
-  std::string name;
-  struct tdesc_type *type;
-  /* For non-enum-values, either both are -1 (non-bitfield), or both are
-     not -1 (bitfield).  For enum values, start is the value (which could be
-     -1), end is -1.  */
-  int start, end;
-};
-
-struct tdesc_type_builtin : tdesc_type
-{
-  tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
-  : tdesc_type (name, kind)
-  {}
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-};
-
-/* tdesc_type for vector types.  */
-
-struct tdesc_type_vector : tdesc_type
-{
-  tdesc_type_vector (const std::string &name, tdesc_type *element_type_, int count_)
-  : tdesc_type (name, TDESC_TYPE_VECTOR),
-    element_type (element_type_), count (count_)
-  {}
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-
-  struct tdesc_type *element_type;
-  int count;
-};
-
-/* tdesc_type for struct, union, flags, and enum types.  */
-
-struct tdesc_type_with_fields : tdesc_type
-{
-  tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
-			  int size_ = 0)
-  : tdesc_type (name, kind), size (size_)
-  {}
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit (this);
-  }
-
-  std::vector<tdesc_type_field> fields;
-  int size;
-};
-
 /* Convert a tdesc_type to a gdb type.  */

 static type *
@@ -741,58 +676,6 @@ tdesc_feature_name (const struct tdesc_feature *feature)
   return feature->name.c_str ();
 }

-/* Predefined types.  */
-static tdesc_type_builtin tdesc_predefined_types[] =
-{
-  { "bool", TDESC_TYPE_BOOL },
-  { "int8", TDESC_TYPE_INT8 },
-  { "int16", TDESC_TYPE_INT16 },
-  { "int32", TDESC_TYPE_INT32 },
-  { "int64", TDESC_TYPE_INT64 },
-  { "int128", TDESC_TYPE_INT128 },
-  { "uint8", TDESC_TYPE_UINT8 },
-  { "uint16", TDESC_TYPE_UINT16 },
-  { "uint32", TDESC_TYPE_UINT32 },
-  { "uint64", TDESC_TYPE_UINT64 },
-  { "uint128", TDESC_TYPE_UINT128 },
-  { "code_ptr", TDESC_TYPE_CODE_PTR },
-  { "data_ptr", TDESC_TYPE_DATA_PTR },
-  { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
-  { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
-  { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
-  { "i387_ext", TDESC_TYPE_I387_EXT }
-};
-
-/* Lookup a predefined type.  */
-
-static struct tdesc_type *
-tdesc_predefined_type (enum tdesc_type_kind kind)
-{
-  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
-    if (tdesc_predefined_types[ix].kind == kind)
-      return &tdesc_predefined_types[ix];
-
-  gdb_assert_not_reached ("bad predefined tdesc type");
-}
-
-/* See common/tdesc.h.  */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
-  /* First try target-defined types.  */
-  for (const tdesc_type_up &type : feature->types)
-    if (type->name == id)
-      return type.get ();
-
-  /* Next try the predefined types.  */
-  for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
-    if (tdesc_predefined_types[ix].name == id)
-      return &tdesc_predefined_types[ix];
-
-  return NULL;
-}
-
 /* Lookup type associated with ID.  */

 struct type *
@@ -1227,147 +1110,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,

 /* See common/tdesc.h.  */

-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
-		     struct tdesc_type *field_type, int count)
-{
-  tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *name)
-{
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
-  gdb_assert (type->kind == TDESC_TYPE_STRUCT);
-  gdb_assert (size > 0);
-  type->size = size;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *name)
-{
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
-		    int size)
-{
-  gdb_assert (size > 0);
-
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-tdesc_type_with_fields *
-tdesc_create_enum (struct tdesc_feature *feature, const char *name,
-		   int size)
-{
-  gdb_assert (size > 0);
-
-  tdesc_type_with_fields *type
-    = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
-  feature->types.emplace_back (type);
-
-  return type;
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
-		 struct tdesc_type *field_type)
-{
-  gdb_assert (type->kind == TDESC_TYPE_UNION
-	      || type->kind == TDESC_TYPE_STRUCT);
-
-  /* Initialize start and end so we know this is not a bit-field
-     when we print-c-tdesc.  */
-  type->fields.emplace_back (field_name, field_type, -1, -1);
-}
-
-void
-tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
-			  int start, int end, struct tdesc_type *field_type)
-{
-  gdb_assert (type->kind == TDESC_TYPE_STRUCT
-	      || type->kind == TDESC_TYPE_FLAGS);
-  gdb_assert (start >= 0 && end >= start);
-
-  type->fields.emplace_back (field_name, field_type, start, end);
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
-		    int start, int end)
-{
-  struct tdesc_type *field_type;
-
-  gdb_assert (start >= 0 && end >= start);
-
-  if (type->size > 4)
-    field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
-  else
-    field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
-
-  tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
-}
-
-/* See common/tdesc.h.  */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
-		const char *flag_name)
-{
-  gdb_assert (type->kind == TDESC_TYPE_FLAGS
-	      || type->kind == TDESC_TYPE_STRUCT);
-
-  type->fields.emplace_back (flag_name,
-			     tdesc_predefined_type (TDESC_TYPE_BOOL),
-			     start, start);
-}
-
-void
-tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
-		      const char *name)
-{
-  gdb_assert (type->kind == TDESC_TYPE_ENUM);
-  type->fields.emplace_back (name,
-			     tdesc_predefined_type (TDESC_TYPE_INT32),
-			     value, -1);
-}
-
-/* See common/tdesc.h.  */
-
 struct tdesc_feature *
 tdesc_create_feature (struct target_desc *tdesc, const char *name,
 		      const char *xml)


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

* [PATCH v3 3/8] Commonise tdesc_feature
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (2 preceding siblings ...)
  2018-03-01 11:40 ` [PATCH v3 4/8] Commonise tdesc types Alan Hayward
@ 2018-03-01 11:40 ` Alan Hayward
  2018-03-12 17:20   ` Philipp Rudo
  2018-03-01 11:40 ` [PATCH v3 5/8] Add tdesc osabi and architecture functions Alan Hayward
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

This patch commonises tdesc_feature and makes use of it in gdbserver tdesc.

Previously in gdbserver, target_desc was interchangeable with tdesc_feature.
Now gdbserver target_desc contains a vector of tdesc_feature.

I am now able to commonise tdesc_create_reg (wanted to do this in the
previous patch).

I also had to commonise tdesc_type. This is ok, because I will need them
in the next patch.

Alan.

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

gdb/
	* common/tdesc.c (tdesc_feature::accept): Move to here.
	(tdesc_feature::operator==): Likewise.
	(tdesc_create_reg): Likewise.
	* common/tdesc.h (tdesc_type_kind): Likewise.
	(struct tdesc_type): Likewise.
	(struct tdesc_feature): Likewise.
	* regformats/regdat.sh: Create a feature.
	* target-descriptions.c (tdesc_type_kind): Move from here.
	(tdesc_type): Likewise.
	(tdesc_type_up): Likewise.
	(tdesc_feature): Likewise.
	(tdesc_create_reg): Likewise.


gdbserver/
	* tdesc.c (~target_desc): Remove implictly deleted items.
	(init_target_desc): Iterate all features.
	(tdesc_get_features_xml): Use vector.
	(tdesc_create_feature): Create feature.
	* tdesc.h (tdesc_feature) Remove
	(target_desc): Add features.


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

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

@@ -137,6 +138,99 @@ struct tdesc_reg : tdesc_element

 typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;

+enum tdesc_type_kind
+{
+  /* Predefined types.  */
+  TDESC_TYPE_BOOL,
+  TDESC_TYPE_INT8,
+  TDESC_TYPE_INT16,
+  TDESC_TYPE_INT32,
+  TDESC_TYPE_INT64,
+  TDESC_TYPE_INT128,
+  TDESC_TYPE_UINT8,
+  TDESC_TYPE_UINT16,
+  TDESC_TYPE_UINT32,
+  TDESC_TYPE_UINT64,
+  TDESC_TYPE_UINT128,
+  TDESC_TYPE_CODE_PTR,
+  TDESC_TYPE_DATA_PTR,
+  TDESC_TYPE_IEEE_SINGLE,
+  TDESC_TYPE_IEEE_DOUBLE,
+  TDESC_TYPE_ARM_FPA_EXT,
+  TDESC_TYPE_I387_EXT,
+
+  /* Types defined by a target feature.  */
+  TDESC_TYPE_VECTOR,
+  TDESC_TYPE_STRUCT,
+  TDESC_TYPE_UNION,
+  TDESC_TYPE_FLAGS,
+  TDESC_TYPE_ENUM
+};
+
+struct tdesc_type : tdesc_element
+{
+  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
+    : name (name_), kind (kind_)
+  {}
+
+  virtual ~tdesc_type () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_type);
+
+  /* The name of this type.  */
+  std::string name;
+
+  /* Identify the kind of this type.  */
+  enum tdesc_type_kind kind;
+
+  bool operator== (const tdesc_type &other) const
+  {
+    return name == other.name && kind == other.kind;
+  }
+
+  bool operator!= (const tdesc_type &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+
+/* A feature from a target description.  Each feature is a collection
+   of other elements, e.g. registers and types.  */
+
+struct tdesc_feature : tdesc_element
+{
+  tdesc_feature (const std::string &name_)
+    : name (name_)
+  {}
+
+  virtual ~tdesc_feature () = default;
+
+  DISABLE_COPY_AND_ASSIGN (tdesc_feature);
+
+  /* The name of this feature.  It may be recognized by the architecture
+     support code.  */
+  std::string name;
+
+  /* The registers associated with this feature.  */
+  std::vector<tdesc_reg_up> registers;
+
+  /* The types associated with this feature.  */
+  std::vector<tdesc_type_up> types;
+
+  void accept (tdesc_element_visitor &v) const override;
+
+  bool operator== (const tdesc_feature &other) const;
+
+  bool operator!= (const tdesc_feature &other) const
+  {
+    return !(*this == other);
+  }
+};
+
+typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
+
 /* Allocate a new target_desc.  */
 target_desc *allocate_target_description (void);

diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index a5f2de4b5ba2adb61dd098d8fc569a6339164586..ef6ed944df20d4613dd0dd338710a4c1ad67bbf2 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -38,3 +38,61 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
      have easy access to the containing feature when we want it later.  */
   tdesc_type = tdesc_named_type (feature, type.c_str ());
 }
+
+void tdesc_feature::accept (tdesc_element_visitor &v) const
+{
+  v.visit_pre (this);
+
+  for (const tdesc_type_up &type : types)
+    type->accept (v);
+
+  for (const tdesc_reg_up &reg : registers)
+    reg->accept (v);
+
+  v.visit_post (this);
+}
+
+bool tdesc_feature::operator== (const tdesc_feature &other) const
+{
+  if (name != other.name)
+    return false;
+
+  if (registers.size () != other.registers.size ())
+    return false;
+
+  for (int ix = 0; ix < registers.size (); ix++)
+    {
+      const tdesc_reg_up &reg1 = registers[ix];
+      const tdesc_reg_up &reg2 = other.registers[ix];
+
+      if (reg1 != reg2 && *reg1 != *reg2)
+	return false;
+      }
+
+  if (types.size () != other.types.size ())
+    return false;
+
+  for (int ix = 0; ix < types.size (); ix++)
+    {
+      const tdesc_type_up &type1 = types[ix];
+      const tdesc_type_up &type2 = other.types[ix];
+
+      if (type1 != type2 && *type1 != *type2)
+	return false;
+    }
+
+  return true;
+}
+
+/* See common/tdesc.h.  */
+
+void
+tdesc_create_reg (struct tdesc_feature *feature, const char *name,
+		  int regnum, int save_restore, const char *group,
+		  int bitsize, const char *type)
+{
+  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+				  group, bitsize, type);
+
+  feature->registers.emplace_back (reg);
+}
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index b0b8fa3f9e63590398d0de52359ae8ee0271557b..8534069d616b97dc82df04939a4ac935aa4e1cad 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -24,16 +24,10 @@
 #include "regdef.h"
 #include <vector>

-struct tdesc_feature
-{
-  /* The registers associated with this feature.  */
-  std::vector<tdesc_reg_up> registers;
-};
-
 /* A target description.  Inherit from tdesc_feature so that target_desc
    can be used as tdesc_feature.  */

-struct target_desc : tdesc_feature
+struct target_desc
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
@@ -42,6 +36,9 @@ struct target_desc : tdesc_feature
   /* The register cache size, in bytes.  */
   int registers_size;

+  /* XML features in this target description.  */
+  std::vector<tdesc_feature_up> features;
+
 #ifndef IN_PROCESS_AGENT
   /* An array of register names.  These are the "expedite" registers:
      registers whose values are sent along with stop replies.  */
@@ -56,9 +53,6 @@ struct target_desc : tdesc_feature
      fields features, arch, and osabi in tdesc_get_features_xml.  */
   const char *xmltarget = NULL;

-  /* XML features in this target description.  */
-  VEC (char_ptr) *features = NULL;
-
   /* The value of <architecture> element in the XML, replying GDB.  */
   const char *arch = NULL;

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

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

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

 bool target_desc::operator== (const target_desc &other) const
@@ -73,28 +65,29 @@ init_target_desc (struct target_desc *tdesc)
   int offset = 0;

   /* Go through all the features and populate reg_defs.  */
-  for (const tdesc_reg_up &treg : tdesc->registers)
-    {
-      /* Fill in any blank spaces.  */
-      while (tdesc->reg_defs.size () < treg->target_regnum)
-	{
-	  struct reg *reg = XCNEW (struct reg);
-	  reg->name = "";
-	  reg->size = 0;
-	  reg->offset = offset;
-	  tdesc->reg_defs.push_back (reg);
-	}
-
-      gdb_assert (treg->target_regnum == 0
-		  || treg->target_regnum == tdesc->reg_defs.size ());
-
-      struct reg *reg = XCNEW (struct reg);
-      reg->name = treg->name.c_str ();
-      reg->size = treg->bitsize;
-      reg->offset = offset;
-      tdesc->reg_defs.push_back (reg);
-      offset += reg->size;
-    }
+  for (const tdesc_feature_up &feature : tdesc->features)
+    for (const tdesc_reg_up &treg : feature->registers)
+      {
+	/* Fill in any blank spaces.  */
+	while (tdesc->reg_defs.size () < treg->target_regnum)
+	  {
+	    struct reg *reg = XCNEW (struct reg);
+	    reg->name = "";
+	    reg->size = 0;
+	    reg->offset = offset;
+	    tdesc->reg_defs.push_back (reg);
+	  }
+
+	gdb_assert (treg->target_regnum == 0
+		    || treg->target_regnum == tdesc->reg_defs.size ());
+
+	struct reg *reg = XCNEW (struct reg);
+	reg->name = treg->name.c_str ();
+	reg->size = treg->bitsize;
+	reg->offset = offset;
+	tdesc->reg_defs.push_back (reg);
+	offset += reg->size;
+      }

   tdesc->registers_size = offset / 8;

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

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

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

@@ -195,19 +186,16 @@ tdesc_get_features_xml (target_desc *tdesc)
 }
 #endif

-struct tdesc_type
-{};
-
 /* See common/tdesc.h.  */

 struct tdesc_feature *
 tdesc_create_feature (struct target_desc *tdesc, const char *name,
 		      const char *xml)
 {
-#ifndef IN_PROCESS_AGENT
-  VEC_safe_push (char_ptr, tdesc->features, xstrdup (xml));
-#endif
-  return tdesc;
+  struct tdesc_feature *new_feature = new tdesc_feature
+    (xml != nullptr ? xml : name);
+  tdesc->features.emplace_back (new_feature);
+  return new_feature;
 }

 /* See common/tdesc.h.  */
@@ -252,21 +240,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id)

 /* See common/tdesc.h.  */

-void
-tdesc_create_reg (struct tdesc_feature *feature, const char *name,
-		  int regnum, int save_restore, const char *group,
-		  int bitsize, const char *type)
-{
-  struct target_desc *tdesc = (struct target_desc *) feature;
-
-  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
-				  group, bitsize, type);
-
-  tdesc->registers.emplace_back (reg);
-}
-
-/* See common/tdesc.h.  */
-
 struct tdesc_type *
 tdesc_create_vector (struct tdesc_feature *feature, const char *name,
 		     struct tdesc_type *field_type, int count)
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index ce1627120d9439082709c82c5804724f39477eb1..8c6e191596350fb4e983f8736985d9832f41e2d3 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -131,7 +131,7 @@ do
     echo "{"
     echo "  static struct target_desc tdesc_${name}_s;"
     echo "  struct target_desc *result = &tdesc_${name}_s;"
-
+    echo "  struct tdesc_feature *feature = tdesc_create_feature (result, \"${name}\");"
     continue
   elif test "${type}" = "xmltarget"; then
     xmltarget="${entry}"
@@ -149,7 +149,7 @@ do
     echo "$0: $1 does not specify \`\`name''." 1>&2
     exit 1
   else
-    echo "  tdesc_create_reg ((struct tdesc_feature *) result, \"${entry}\","
+    echo "  tdesc_create_reg (feature, \"${entry}\","
     echo "  0, 0, NULL, ${type}, NULL);"

     offset=`expr ${offset} + ${type}`
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250..a453eddec0cd97b692c4c9ace3f093c3fbc67422 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -67,64 +67,6 @@ struct tdesc_type_field
   int start, end;
 };

-enum tdesc_type_kind
-{
-  /* Predefined types.  */
-  TDESC_TYPE_BOOL,
-  TDESC_TYPE_INT8,
-  TDESC_TYPE_INT16,
-  TDESC_TYPE_INT32,
-  TDESC_TYPE_INT64,
-  TDESC_TYPE_INT128,
-  TDESC_TYPE_UINT8,
-  TDESC_TYPE_UINT16,
-  TDESC_TYPE_UINT32,
-  TDESC_TYPE_UINT64,
-  TDESC_TYPE_UINT128,
-  TDESC_TYPE_CODE_PTR,
-  TDESC_TYPE_DATA_PTR,
-  TDESC_TYPE_IEEE_SINGLE,
-  TDESC_TYPE_IEEE_DOUBLE,
-  TDESC_TYPE_ARM_FPA_EXT,
-  TDESC_TYPE_I387_EXT,
-
-  /* Types defined by a target feature.  */
-  TDESC_TYPE_VECTOR,
-  TDESC_TYPE_STRUCT,
-  TDESC_TYPE_UNION,
-  TDESC_TYPE_FLAGS,
-  TDESC_TYPE_ENUM
-};
-
-struct tdesc_type : tdesc_element
-{
-  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
-    : name (name_), kind (kind_)
-  {}
-
-  virtual ~tdesc_type () = default;
-
-  DISABLE_COPY_AND_ASSIGN (tdesc_type);
-
-  /* The name of this type.   */
-  std::string name;
-
-  /* Identify the kind of this type.  */
-  enum tdesc_type_kind kind;
-
-  bool operator== (const tdesc_type &other) const
-  {
-    return name == other.name && kind == other.kind;
-  }
-
-  bool operator!= (const tdesc_type &other) const
-  {
-    return !(*this == other);
-  }
-};
-
-typedef std::unique_ptr<tdesc_type> tdesc_type_up;
-
 struct tdesc_type_builtin : tdesc_type
 {
   tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
@@ -428,82 +370,6 @@ make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype)
   return gdb_type.get_type ();
 }

-/* A feature from a target description.  Each feature is a collection
-   of other elements, e.g. registers and types.  */
-
-struct tdesc_feature : tdesc_element
-{
-  tdesc_feature (const std::string &name_)
-    : name (name_)
-  {}
-
-  virtual ~tdesc_feature () = default;
-
-  DISABLE_COPY_AND_ASSIGN (tdesc_feature);
-
-  /* The name of this feature.  It may be recognized by the architecture
-     support code.  */
-  std::string name;
-
-  /* The registers associated with this feature.  */
-  std::vector<tdesc_reg_up> registers;
-
-  /* The types associated with this feature.  */
-  std::vector<tdesc_type_up> types;
-
-  void accept (tdesc_element_visitor &v) const override
-  {
-    v.visit_pre (this);
-
-    for (const tdesc_type_up &type : types)
-      type->accept (v);
-
-    for (const tdesc_reg_up &reg : registers)
-      reg->accept (v);
-
-    v.visit_post (this);
-  }
-
-  bool operator== (const tdesc_feature &other) const
-  {
-    if (name != other.name)
-      return false;
-
-    if (registers.size () != other.registers.size ())
-      return false;
-
-    for (int ix = 0; ix < registers.size (); ix++)
-      {
-	const tdesc_reg_up &reg1 = registers[ix];
-	const tdesc_reg_up &reg2 = other.registers[ix];
-
-	if (reg1 != reg2 && *reg1 != *reg2)
-	  return false;
-      }
-
-    if (types.size () != other.types.size ())
-      return false;
-
-    for (int ix = 0; ix < types.size (); ix++)
-      {
-	const tdesc_type_up &type1 = types[ix];
-	const tdesc_type_up &type2 = other.types[ix];
-
-	if (type1 != type2 && *type1 != *type2)
-	  return false;
-      }
-
-    return true;
-  }
-
-  bool operator!= (const tdesc_feature &other) const
-  {
-    return !(*this == other);
-  }
-};
-
-typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
-
 /* A target description.  */

 struct target_desc : tdesc_element
@@ -1358,20 +1224,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 				      tdesc_remote_register_number);
   set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p);
 }
-

-
-/* See common/tdesc.h.  */
-
-void
-tdesc_create_reg (struct tdesc_feature *feature, const char *name,
-		  int regnum, int save_restore, const char *group,
-		  int bitsize, const char *type)
-{
-  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
-				  group, bitsize, type);
-
-  feature->registers.emplace_back (reg);
-}

 /* See common/tdesc.h.  */

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

* [PATCH v3 6/8] Create xml from target descriptions
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (6 preceding siblings ...)
  2018-03-01 11:41 ` [PATCH v3 7/8]: Remove xml file references from target descriptions Alan Hayward
@ 2018-03-01 11:41 ` Alan Hayward
  2018-03-12 17:20   ` Philipp Rudo
  2018-03-13 18:05   ` Philipp Rudo
  2018-03-09  8:21 ` [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
  8 siblings, 2 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

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
--- 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);
+}

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

* [PATCH v3 7/8]: Remove xml file references from target descriptions.
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (5 preceding siblings ...)
  2018-03-01 11:41 ` [PATCH v3 8/8] Remove xml files from gdbserver Alan Hayward
@ 2018-03-01 11:41 ` Alan Hayward
  2018-03-01 11:41 ` [PATCH v3 6/8] Create xml " Alan Hayward
  2018-03-09  8:21 ` [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
  8 siblings, 0 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

We no longer need to know the name of the xml file. This patch removes the
references and regenerates the C files.

Alan.


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

gdb/
	* common/tdesc.h (tdesc_create_feature): Remove xml filename parameter.
	* features/aarch64-core.c (create_feature_aarch64_core): Regenerate.
	* features/aarch64-fpu.c (create_feature_aarch64_fpu): Likewise.
	* features/i386/32bit-avx.c (create_feature_i386_32bit_avx): Likewise.
	* features/i386/32bit-avx512.c (create_feature_i386_32bit_avx512):
	Likewise.
	* features/i386/32bit-core.c (create_feature_i386_32bit_core):
	Likewise.
	* features/i386/32bit-linux.c (create_feature_i386_32bit_linux):
	Likewise.
	* features/i386/32bit-mpx.c (create_feature_i386_32bit_mpx): Likewise.
	* features/i386/32bit-pkeys.c (create_feature_i386_32bit_pkeys):
	Likewise.
	* features/i386/32bit-sse.c (create_feature_i386_32bit_sse): Likewise.
	* features/i386/64bit-avx.c (create_feature_i386_64bit_avx): Likewise.
	* features/i386/64bit-avx512.c (create_feature_i386_64bit_avx512):
	Likewise.
	* features/i386/64bit-core.c (create_feature_i386_64bit_core):
	Likewise.
	* features/i386/64bit-linux.c (create_feature_i386_64bit_linux):
	Likewise.
	* features/i386/64bit-mpx.c (create_feature_i386_64bit_mpx): Likewise.
	* features/i386/64bit-pkeys.c (create_feature_i386_64bit_pkeys):
	Likewise.
	* features/i386/64bit-segments.c (create_feature_i386_64bit_segments):
	Likewise.
	* features/i386/64bit-sse.c (create_feature_i386_64bit_sse): Likewise.
	* features/i386/x32-core.c (create_feature_i386_x32_core): Likewise.
	* features/tic6x-c6xp.c (create_feature_tic6x_c6xp): Likewise.
	* features/tic6x-core.c (create_feature_tic6x_core): Likewise.
	* features/tic6x-gp.c (create_feature_tic6x_gp): Likewise.
	* target-descriptions.c: In generated code, don't pass xml filename.

gdbserver/
	* gdbserver/tdesc.c: Remove xml parameter.


diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 1c5ddc572cf76cb24176fd638dde417687d202b0..cf288b096a6b553ce8fc1a155f3488402e63dac3 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -323,9 +323,7 @@ struct tdesc_type *tdesc_named_type (const struct tdesc_feature *feature,

 /* Return the created feature named NAME in target description TDESC.  */
 struct tdesc_feature *tdesc_create_feature (struct target_desc *tdesc,
-					    const char *name,
-					    const char *xml = nullptr);
-
+					    const char *name);

 /* Return the created vector tdesc_type named NAME in FEATURE.  */
 struct tdesc_type *tdesc_create_vector (struct tdesc_feature *feature,
diff --git a/gdb/features/aarch64-core.c b/gdb/features/aarch64-core.c
index db10c4aa068d9fa1fb089f15dfc0fc3827a3a286..cd3de02cbc122d0be4a2b30a806b259222b17519 100644
--- a/gdb/features/aarch64-core.c
+++ b/gdb/features/aarch64-core.c
@@ -8,7 +8,7 @@ create_feature_aarch64_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core", "aarch64-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "cpsr_flags", 4);
   tdesc_add_flag (type_with_fields, 0, "SP");
diff --git a/gdb/features/aarch64-fpu.c b/gdb/features/aarch64-fpu.c
index 0c40c4cef22e42be5ee27bf205ee60f5dbbdbf0f..47962d1f242751f68d1a9a41aedba4ad662a2ecd 100644
--- a/gdb/features/aarch64-fpu.c
+++ b/gdb/features/aarch64-fpu.c
@@ -8,7 +8,7 @@ create_feature_aarch64_fpu (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu", "aarch64-fpu.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpu");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_double");
   tdesc_create_vector (feature, "v2d", element_type, 2);
diff --git a/gdb/features/i386/32bit-avx.c b/gdb/features/i386/32bit-avx.c
index 6e2cfdbdcbcc024fc81db522842e8377cb2d50c5..b9feac377cf696efbabc417895788081ac3bfb32 100644
--- a/gdb/features/i386/32bit-avx.c
+++ b/gdb/features/i386/32bit-avx.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_avx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx", "32bit-avx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
   tdesc_create_reg (feature, "ymm0h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm1h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm2h", regnum++, 1, NULL, 128, "uint128");
diff --git a/gdb/features/i386/32bit-avx512.c b/gdb/features/i386/32bit-avx512.c
index 863c119ac40d25270729ae849b4bbb53977b4e15..51a403259db2dbae7524df606f6c03bd0f03f5cb 100644
--- a/gdb/features/i386/32bit-avx512.c
+++ b/gdb/features/i386/32bit-avx512.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_avx512 (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512", "32bit-avx512.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "uint128");
   tdesc_create_vector (feature, "v2ui128", element_type, 2);
diff --git a/gdb/features/i386/32bit-core.c b/gdb/features/i386/32bit-core.c
index f6de737074202f37e57861ca3f1609d1e9ecc86a..098c2ca31c6bb63d51c77aa75bc46061966403be 100644
--- a/gdb/features/i386/32bit-core.c
+++ b/gdb/features/i386/32bit-core.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "32bit-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type_with_fields, 0, "CF");
diff --git a/gdb/features/i386/32bit-linux.c b/gdb/features/i386/32bit-linux.c
index af476da194e2cb0c4a88030f6480917130323de0..3bc593739c9bcbda7e7a8b0a9ca830ed2cc51e39 100644
--- a/gdb/features/i386/32bit-linux.c
+++ b/gdb/features/i386/32bit-linux.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_linux (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux", "32bit-linux.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
   regnum = 41;
   tdesc_create_reg (feature, "orig_eax", regnum++, 1, NULL, 32, "int");
   return regnum;
diff --git a/gdb/features/i386/32bit-mpx.c b/gdb/features/i386/32bit-mpx.c
index 84101746b50e3a55ea6e76a9c1b6549a75de402f..8288e9d758f2718465621ed55b3514145cf1dd3b 100644
--- a/gdb/features/i386/32bit-mpx.c
+++ b/gdb/features/i386/32bit-mpx.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_mpx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx", "32bit-mpx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_struct (feature, "br128");
   tdesc_type *field_type;
diff --git a/gdb/features/i386/32bit-pkeys.c b/gdb/features/i386/32bit-pkeys.c
index 272751f5781e81373e090d18e8bd133529d3570e..a2a72f40a31005153b2bd7ba35625ca39fa99b87 100644
--- a/gdb/features/i386/32bit-pkeys.c
+++ b/gdb/features/i386/32bit-pkeys.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_pkeys (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys", "32bit-pkeys.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys");
   tdesc_create_reg (feature, "pkru", regnum++, 1, NULL, 32, "uint32");
   return regnum;
 }
diff --git a/gdb/features/i386/32bit-sse.c b/gdb/features/i386/32bit-sse.c
index 78fc02b863c7d1107d8360c4a4707a8df2ea4642..68fbab06a4003f0df9040a880968350d5b84f1dd 100644
--- a/gdb/features/i386/32bit-sse.c
+++ b/gdb/features/i386/32bit-sse.c
@@ -8,7 +8,7 @@ create_feature_i386_32bit_sse (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse", "32bit-sse.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", element_type, 4);
diff --git a/gdb/features/i386/64bit-avx.c b/gdb/features/i386/64bit-avx.c
index 5f11035af3f208b0a0bdfee36802e4cc7b29ccf9..7e45c980dbc1ed25e22c116119b0c784b551a962 100644
--- a/gdb/features/i386/64bit-avx.c
+++ b/gdb/features/i386/64bit-avx.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_avx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx", "64bit-avx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx");
   tdesc_create_reg (feature, "ymm0h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm1h", regnum++, 1, NULL, 128, "uint128");
   tdesc_create_reg (feature, "ymm2h", regnum++, 1, NULL, 128, "uint128");
diff --git a/gdb/features/i386/64bit-avx512.c b/gdb/features/i386/64bit-avx512.c
index 733910025bab29345124e4c006136ffa19e98339..acc07a3628b2fb7e01b839eb063eabbf99ad42a7 100644
--- a/gdb/features/i386/64bit-avx512.c
+++ b/gdb/features/i386/64bit-avx512.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_avx512 (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512", "64bit-avx512.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx512");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", element_type, 4);
diff --git a/gdb/features/i386/64bit-core.c b/gdb/features/i386/64bit-core.c
index efc7016380851ab21686cd7920b57666313a1bab..617425da217da0823dd1c49440312d86d54a778f 100644
--- a/gdb/features/i386/64bit-core.c
+++ b/gdb/features/i386/64bit-core.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "64bit-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type_with_fields, 0, "CF");
diff --git a/gdb/features/i386/64bit-linux.c b/gdb/features/i386/64bit-linux.c
index c9c56f193cf7bee14b3c1b2b4733cb037196c5c6..68de9d27e8bb495b5cdb0a9e681171ed0f811284 100644
--- a/gdb/features/i386/64bit-linux.c
+++ b/gdb/features/i386/64bit-linux.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_linux (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux", "64bit-linux.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
   regnum = 57;
   tdesc_create_reg (feature, "orig_rax", regnum++, 1, NULL, 64, "int");
   return regnum;
diff --git a/gdb/features/i386/64bit-mpx.c b/gdb/features/i386/64bit-mpx.c
index d923813d699d331c8669e1f4493706bb0851b4ba..ce4d611d9c11aa5e63a33e304f99b66d9e63fb87 100644
--- a/gdb/features/i386/64bit-mpx.c
+++ b/gdb/features/i386/64bit-mpx.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_mpx (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx", "64bit-mpx.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.mpx");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_struct (feature, "br128");
   tdesc_type *field_type;
diff --git a/gdb/features/i386/64bit-pkeys.c b/gdb/features/i386/64bit-pkeys.c
index f1cbce0ce7fbf4af2a285dae9cbb1db2c569c3a5..70b88cf74db40f341a9c7195d897604d96b58d8b 100644
--- a/gdb/features/i386/64bit-pkeys.c
+++ b/gdb/features/i386/64bit-pkeys.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_pkeys (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys", "64bit-pkeys.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys");
   tdesc_create_reg (feature, "pkru", regnum++, 1, NULL, 32, "uint32");
   return regnum;
 }
diff --git a/gdb/features/i386/64bit-segments.c b/gdb/features/i386/64bit-segments.c
index 900471fc48c9361d6c453c92c1bc67a93409941d..b06d40fbda3b9a5aa0bcea01831e23399cad27d6 100644
--- a/gdb/features/i386/64bit-segments.c
+++ b/gdb/features/i386/64bit-segments.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_segments (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.segments", "64bit-segments.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.segments");
   tdesc_create_reg (feature, "fs_base", regnum++, 1, NULL, 64, "int");
   tdesc_create_reg (feature, "gs_base", regnum++, 1, NULL, 64, "int");
   return regnum;
diff --git a/gdb/features/i386/64bit-sse.c b/gdb/features/i386/64bit-sse.c
index 294360197f588ddfc595e18d6386a53f108b60e6..e08309374115d16ee19c6b5bba63abbbf65ae821 100644
--- a/gdb/features/i386/64bit-sse.c
+++ b/gdb/features/i386/64bit-sse.c
@@ -8,7 +8,7 @@ create_feature_i386_64bit_sse (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse", "64bit-sse.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
   tdesc_type *element_type;
   element_type = tdesc_named_type (feature, "ieee_single");
   tdesc_create_vector (feature, "v4f", element_type, 4);
diff --git a/gdb/features/i386/x32-core.c b/gdb/features/i386/x32-core.c
index 9b0bcc5a7ef0df5835a1a95dbd9a50688cf8cd79..dd6e1a2ed6a9234aec1c8472e2f811dfa6efc13b 100644
--- a/gdb/features/i386/x32-core.c
+++ b/gdb/features/i386/x32-core.c
@@ -8,7 +8,7 @@ create_feature_i386_x32_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core", "x32-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
   tdesc_type_with_fields *type_with_fields;
   type_with_fields = tdesc_create_flags (feature, "i386_eflags", 4);
   tdesc_add_flag (type_with_fields, 0, "CF");
diff --git a/gdb/features/tic6x-c6xp.c b/gdb/features/tic6x-c6xp.c
index 261a562d903b7c1a203cd9a66f2f5fca8ac8bec7..bfd69d7fd52cf028c623bc32b0ea5e7377335411 100644
--- a/gdb/features/tic6x-c6xp.c
+++ b/gdb/features/tic6x-c6xp.c
@@ -8,7 +8,7 @@ create_feature_tic6x_c6xp (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.c6xp", "tic6x-c6xp.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.c6xp");
   tdesc_create_reg (feature, "TSR", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "ILC", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "RILC", regnum++, 1, NULL, 32, "uint32");
diff --git a/gdb/features/tic6x-core.c b/gdb/features/tic6x-core.c
index 65f73ec97df8fef36a265e0a0c2891dc6aedb2aa..0415209275a792f0ad287e35ab40674b164eb786 100644
--- a/gdb/features/tic6x-core.c
+++ b/gdb/features/tic6x-core.c
@@ -8,7 +8,7 @@ create_feature_tic6x_core (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.core", "tic6x-core.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.core");
   tdesc_create_reg (feature, "A0", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A1", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A2", regnum++, 1, NULL, 32, "uint32");
diff --git a/gdb/features/tic6x-gp.c b/gdb/features/tic6x-gp.c
index b62458837947247405564d055925acd424c6be03..4a0734c04a477e38b21b4b59a7055eade4db515a 100644
--- a/gdb/features/tic6x-gp.c
+++ b/gdb/features/tic6x-gp.c
@@ -8,7 +8,7 @@ create_feature_tic6x_gp (struct target_desc *result, long regnum)
 {
   struct tdesc_feature *feature;

-  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.gp", "tic6x-gp.xml");
+  feature = tdesc_create_feature (result, "org.gnu.gdb.tic6x.gp");
   tdesc_create_reg (feature, "A16", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A17", regnum++, 1, NULL, 32, "uint32");
   tdesc_create_reg (feature, "A18", regnum++, 1, NULL, 32, "uint32");
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 0d9609bd4c3446b7a15df569923d22fc10105b74..b5cd8fa87576f415d0eede991148ef2e535b2097 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -195,11 +195,9 @@ tdesc_get_features_xml (target_desc *tdesc)
 /* See common/tdesc.h.  */

 struct tdesc_feature *
-tdesc_create_feature (struct target_desc *tdesc, const char *name,
-		      const char *xml)
+tdesc_create_feature (struct target_desc *tdesc, const char *name)
 {
-  struct tdesc_feature *new_feature = new tdesc_feature
-    (xml != nullptr ? xml : name);
+  struct tdesc_feature *new_feature = new tdesc_feature (name);
   tdesc->features.emplace_back (new_feature);
   return new_feature;
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 93e73571177c980b4cd1975256c89e8ffb9fcdd6..fa8ea66a6f57fdd99710e3447efa60d7387dad58 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1130,8 +1130,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 /* See common/tdesc.h.  */

 struct tdesc_feature *
-tdesc_create_feature (struct target_desc *tdesc, const char *name,
-		      const char *xml)
+tdesc_create_feature (struct target_desc *tdesc, const char *name)
 {
   struct tdesc_feature *new_feature = new tdesc_feature (name);

@@ -1599,8 +1598,8 @@ public:
     printf_unfiltered ("  struct tdesc_feature *feature;\n");

     printf_unfiltered
-      ("\n  feature = tdesc_create_feature (result, \"%s\", \"%s\");\n",
-       e->name.c_str (), lbasename (m_filename_after_features.c_str ()));
+      ("\n  feature = tdesc_create_feature (result, \"%s\");\n",
+       e->name.c_str ());
   }

   void visit_post (const tdesc_feature *e) override

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

* [PATCH v3 8/8] Remove xml files from gdbserver
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (4 preceding siblings ...)
  2018-03-01 11:40 ` [PATCH v3 5/8] Add tdesc osabi and architecture functions Alan Hayward
@ 2018-03-01 11:41 ` Alan Hayward
  2018-03-01 11:41 ` [PATCH v3 7/8]: Remove xml file references from target descriptions Alan Hayward
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-01 11:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

For ports which use new target descriptions, this patch removes
the xml files from being built into gdbserver,

Alan.

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

gdbserver/
	* configure.srv (aarch64*-*-linux*): Don't include xml.
	(arm*-*-linux*): Likewise.
	(i[34567]86-*-cygwin*): Likewise.
	(i[34567]86-*-linux*): Likewise.
	(i[34567]86-*-lynxos*): Likewise.
	(i[34567]86-*-mingw32ce*): Likewise.
	(i[34567]86-*-mingw*): Likewise.
	(i[34567]86-*-nto*): Likewise.
	(tic6x-*-uclinux): Likewise.
	(x86_64-*-linux*): Likewise.
	(x86_64-*-mingw*): Likewise.
	(x86_64-*-cygwin*): Likewise.


diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 087fd31426bc738fdae91dde4bd4d575d8b26224..1f681ebd200720d7d286980d00f88fac593e512a 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -34,14 +34,6 @@ fi

 ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-cell32l-ipa.o powerpc-vsx32l-ipa.o powerpc-isa205-32l-ipa.o powerpc-isa205-altivec32l-ipa.o powerpc-isa205-vsx32l-ipa.o powerpc-e500l-ipa.o powerpc-64l-ipa.o powerpc-altivec64l-ipa.o powerpc-cell64l-ipa.o powerpc-vsx64l-ipa.o powerpc-isa205-64l-ipa.o powerpc-isa205-altivec64l-ipa.o powerpc-isa205-vsx64l-ipa.o"

-srv_i386_32bit_xmlfiles="i386/32bit-core.xml i386/32bit-sse.xml i386/32bit-avx.xml i386/32bit-avx512.xml i386/32bit-mpx.xml i386/32bit-pkeys.xml"
-srv_i386_64bit_xmlfiles="i386/64bit-core.xml i386/64bit-segments.xml i386/64bit-sse.xml i386/64bit-avx.xml i386/64bit-avx512.xml i386/x32-core.xml i386/64bit-mpx.xml i386/64bit-pkeys.xml"
-srv_i386_xmlfiles="i386/i386.xml $srv_i386_32bit_xmlfiles"
-srv_amd64_xmlfiles="i386/amd64.xml $srv_i386_64bit_xmlfiles"
-srv_i386_linux_xmlfiles="i386/32bit-linux.xml $srv_i386_32bit_xmlfiles"
-srv_amd64_linux_xmlfiles="i386/64bit-linux.xml $srv_i386_64bit_xmlfiles"
-
-
 # Linux object files.  This is so we don't have to repeat
 # these files over and over again.
 srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o linux-namespaces.o fork-child.o fork-inferior.o"
@@ -63,11 +55,6 @@ case "${target}" in
 			srv_tgtobj="$srv_tgtobj arch/aarch64.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
 			srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
-			srv_xmlfiles="aarch64.xml"
-			srv_xmlfiles="${srv_xmlfiles} aarch64-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} aarch64-fpu.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-core.xml arm/arm-vfpv3.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-with-neon.xml"
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
 			ipa_obj="linux-aarch64-ipa.o"
@@ -83,14 +70,6 @@ case "${target}" in
 			srv_tgtobj="${srv_tgtobj} arch/arm.o"
 			srv_tgtobj="${srv_tgtobj} arch/arm-linux.o"
 			srv_tgtobj="${srv_tgtobj} arch/arm-get-next-pcs.o"
-			srv_xmlfiles="arm/arm-with-iwmmxt.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-with-vfpv2.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-with-vfpv3.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-with-neon.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/xscale-iwmmxt.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-vfpv2.xml"
-			srv_xmlfiles="${srv_xmlfiles} arm/arm-vfpv3.xml"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -121,13 +100,10 @@ case "${target}" in
   i[34567]86-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
   i[34567]86-*-linux*)	srv_regobj="$srv_i386_linux_regobj"
-			srv_xmlfiles="$srv_i386_linux_xmlfiles"
 			if test "$gdb_cv_i386_is_x86_64" = yes ; then
 			    srv_regobj="$srv_regobj $srv_amd64_linux_regobj"
-			    srv_xmlfiles="${srv_xmlfiles} $srv_amd64_linux_xmlfiles"
 			    srv_tgtobj="amd64-linux-siginfo.o"
 			fi
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
@@ -145,9 +121,6 @@ case "${target}" in
   i[34567]86-*-lynxos*)	srv_regobj=""
 			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o fork-inferior.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_xmlfiles="i386/i386.xml"
-			srv_xmlfiles="${srv_xmlfiles} i386/32bit-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} i386/32bit-sse.xml"
 			srv_lynxos=yes
 			;;
   i[34567]86-*-mingw32ce*)
@@ -155,7 +128,6 @@ case "${target}" in
 			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			# hostio_last_error implementation is in win32-low.c
 			srv_hostio_err_objs=""
 			srv_mingw=yes
@@ -164,12 +136,10 @@ case "${target}" in
   i[34567]86-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_mingw=yes
 			;;
   i[34567]86-*-nto*)	srv_regobj=""
 			srv_tgtobj="nto-low.o nto-x86-low.o arch/i386.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_qnx="yes"
 			;;
   ia64-*-linux*)	srv_regobj=reg-ia64.o
@@ -370,9 +340,6 @@ case "${target}" in
                         else
 			  srv_regobj=""
                         fi
-			srv_xmlfiles="${srv_xmlfiles} tic6x-core.xml"
-			srv_xmlfiles="${srv_xmlfiles} tic6x-gp.xml"
-			srv_xmlfiles="${srv_xmlfiles} tic6x-c6xp.xml"
 			srv_tgtobj="$srv_linux_obj linux-tic6x-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/tic6x.o"
 			srv_linux_regsets=yes
@@ -386,7 +353,6 @@ case "${target}" in
 			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
 			srv_tgtobj="${srv_tgtobj} x86-linux-dregs.o"
 			srv_tgtobj="${srv_tgtobj} amd64-linux-siginfo.o"
-			srv_xmlfiles="$srv_i386_linux_xmlfiles $srv_amd64_linux_xmlfiles"
 			srv_linux_usrregs=yes # This is for i386 progs.
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -397,13 +363,11 @@ case "${target}" in
   x86_64-*-mingw*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
-			srv_xmlfiles="$srv_i386_xmlfiles $srv_amd64_xmlfiles"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj=""
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
-			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;

   xtensa*-*-linux*)	srv_regobj=reg-xtensa.o

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

* Re: [PATCH V3 0/8] Remove gdbserver dependency on xml files
  2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
                   ` (7 preceding siblings ...)
  2018-03-01 11:41 ` [PATCH v3 6/8] Create xml " Alan Hayward
@ 2018-03-09  8:21 ` Alan Hayward
  2018-03-12 14:05   ` Omair Javaid
  2018-03-12 17:19   ` Philipp Rudo
  8 siblings, 2 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-09  8:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd

Ping for this series please.

Alan.

> On 1 Mar 2018, at 11:38, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> V3 builds on previous review comments, and the additional patches I've
> already pushed. Complete patch series pushed to branch users/ahayward/xml3.
> 
> Summary:
> 
> For those targets that use new style target descriptions, this set of patches
> removes the dependency on xml files. Namely:
> * Removes inclusion of xml files within gdbserver.
> * Removes the requirement for the .c files in features/ to be generated from
> cached xml files.
> This is made possible by changing xml descriptions generated by gdbserver, so
> that instead of including xml file names, gdbserver now generate a complete
> xml description.
> 
> The second point will be required for aarch64 SVE support, where the register
> size are variable. Creating SVE xml files for every possible vector length
> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
> features/ .c code that would normally be generated from xml.
> 
> 
> XML Generation:
> 
> In existing code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, the
> function tdesc_get_features_xml () creates an xml containing the name of the
> original xml file(s). For example:
> 
> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> <target>
>  <architecture>i386</architecture>
>  <osabi>GNU/Linux</osabi>
>  <xi:include href="32bit-core.xml"/>
>  <xi:include href="32bit-sse.xml"/>
>  <xi:include href="32bit-linux.xml"/>
>  <xi:include href="32bit-avx.xml"/>
> </target>
> 
> Upon receipt, GDB then makes requests to gdbserver for the contents of the
> xml files. Gdbserver keeps full copies all the xml files inside the binary.
> 
> This patch series adds common code that allows gdbserver (and gdb) to turn
> a C target description structure into xml.
> Now when asked fort an xml description to gdb, gdbserver turns the entire
> target description structure back into xml, without using any cached files.
> Producing, for example:
> 
> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> <target>
>  <architecture>i386</architecture>
>  <osabi>GNU/Linux</osabi>
>  <feature name="org.gnu.gdb.i386.core">
>    <flags id="i386_eflags" size="4">
>      <field name="CF" start="0" end="0"/>
>      <field name="" start="1" end="1"/>
>      <field name="PF" start="2" end="2"/>
>      <field name="AF" start="4" end="4"/>
> ...etc...
> 
> 
> Patch Contents:
> 
> Patches 2-4 commonise the various target descriptor functionality, allowing
> gdbserver to parse target descriptions in the same way as gdb. This series
> does not commonise target_desc, but this is hopefully a long term goal.
> 
> The sixth patch adds the xml printer, which iterates through the parsing
> generated in the previous patches.
> 
> The other patches are clean up patches.
> 
> 
> 
> Patches have been tested on a make check on x86 targets=all build with
> target board unix native-gdbserver. Also tested aarch64. Built for power
> (because it does not use new target descriptions), but am unable to test.
> In addition, patch six adds new test cases to unit test.
> 
> Alan.
> 
> gdb/Makefile.in                    |   2 +
> gdb/common/tdesc.c                 | 445 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> gdb/features/aarch64-core.c        |   2 +-
> gdb/features/aarch64-fpu.c         |   2 +-
> gdb/features/i386/32bit-avx.c      |   2 +-
> gdb/features/i386/32bit-avx512.c   |   2 +-
> gdb/features/i386/32bit-core.c     |   2 +-
> gdb/features/i386/32bit-linux.c    |   2 +-
> gdb/features/i386/32bit-mpx.c      |   2 +-
> gdb/features/i386/32bit-pkeys.c    |   2 +-
> gdb/features/i386/32bit-sse.c      |   2 +-
> gdb/features/i386/64bit-avx.c      |   2 +-
> gdb/features/i386/64bit-avx512.c   |   2 +-
> gdb/features/i386/64bit-core.c     |   2 +-
> gdb/features/i386/64bit-linux.c    |   2 +-
> gdb/features/i386/64bit-mpx.c      |   2 +-
> gdb/features/i386/64bit-pkeys.c    |   2 +-
> gdb/features/i386/64bit-segments.c |   2 +-
> gdb/features/i386/64bit-sse.c      |   2 +-
> gdb/features/i386/x32-core.c       |   2 +-
> gdb/features/tic6x-c6xp.c          |   2 +-
> gdb/features/tic6x-core.c          |   2 +-
> gdb/features/tic6x-gp.c            |   2 +-
> gdb/gdbserver/Makefile.in          |   3 +
> gdb/gdbserver/configure.srv        |  36 -------
> gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------------------
> gdb/gdbserver/tdesc.h              |  57 ++--------
> gdb/regformats/regdat.sh           |   5 +-
> gdb/target-descriptions.c          | 596 ++++++++++++--------------------------------------------------------------------------------------------
> gdb/xml-tdesc.c                    |   9 ++
> gdb/xml-tdesc.h                    |   5 +
> 32 files changed, 974 insertions(+), 779 deletions(-)
> 

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

* Re: [PATCH V3 0/8] Remove gdbserver dependency on xml files
  2018-03-09  8:21 ` [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
@ 2018-03-12 14:05   ` Omair Javaid
  2018-03-12 17:19   ` Philipp Rudo
  1 sibling, 0 replies; 19+ messages in thread
From: Omair Javaid @ 2018-03-12 14:05 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

Hi Alan,

Your patch series look good overall. I wanted to have a look at the overall
code after applying this series but it still doesnt work.

First 2 patches apply just fine. Patch 3 fails.

Thanks!



On 9 March 2018 at 13:21, Alan Hayward <Alan.Hayward@arm.com> wrote:

> Ping for this series please.
>
> Alan.
>
> > On 1 Mar 2018, at 11:38, Alan Hayward <Alan.Hayward@arm.com> wrote:
> >
> > V3 builds on previous review comments, and the additional patches I've
> > already pushed. Complete patch series pushed to branch
> users/ahayward/xml3.
> >
> > Summary:
> >
> > For those targets that use new style target descriptions, this set of
> patches
> > removes the dependency on xml files. Namely:
> > * Removes inclusion of xml files within gdbserver.
> > * Removes the requirement for the .c files in features/ to be generated
> from
> > cached xml files.
> > This is made possible by changing xml descriptions generated by
> gdbserver, so
> > that instead of including xml file names, gdbserver now generate a
> complete
> > xml description.
> >
> > The second point will be required for aarch64 SVE support, where the
> register
> > size are variable. Creating SVE xml files for every possible vector
> length
> > would not be feasible. Instead the plan for aarch64 SVE is to hand write
> the
> > features/ .c code that would normally be generated from xml.
> >
> >
> > XML Generation:
> >
> > In existing code, gdbserver uses C code auto generated from xml files to
> > create target descriptions. When sending an xml description to GDB, the
> > function tdesc_get_features_xml () creates an xml containing the name of
> the
> > original xml file(s). For example:
> >
> > <!DOCTYPE target SYSTEM "gdb-target.dtd">
> > <target>
> >  <architecture>i386</architecture>
> >  <osabi>GNU/Linux</osabi>
> >  <xi:include href="32bit-core.xml"/>
> >  <xi:include href="32bit-sse.xml"/>
> >  <xi:include href="32bit-linux.xml"/>
> >  <xi:include href="32bit-avx.xml"/>
> > </target>
> >
> > Upon receipt, GDB then makes requests to gdbserver for the contents of
> the
> > xml files. Gdbserver keeps full copies all the xml files inside the
> binary.
> >
> > This patch series adds common code that allows gdbserver (and gdb) to
> turn
> > a C target description structure into xml.
> > Now when asked fort an xml description to gdb, gdbserver turns the entire
> > target description structure back into xml, without using any cached
> files.
> > Producing, for example:
> >
> > <!DOCTYPE target SYSTEM "gdb-target.dtd">
> > <target>
> >  <architecture>i386</architecture>
> >  <osabi>GNU/Linux</osabi>
> >  <feature name="org.gnu.gdb.i386.core">
> >    <flags id="i386_eflags" size="4">
> >      <field name="CF" start="0" end="0"/>
> >      <field name="" start="1" end="1"/>
> >      <field name="PF" start="2" end="2"/>
> >      <field name="AF" start="4" end="4"/>
> > ...etc...
> >
> >
> > Patch Contents:
> >
> > Patches 2-4 commonise the various target descriptor functionality,
> allowing
> > gdbserver to parse target descriptions in the same way as gdb. This
> series
> > does not commonise target_desc, but this is hopefully a long term goal.
> >
> > The sixth patch adds the xml printer, which iterates through the parsing
> > generated in the previous patches.
> >
> > The other patches are clean up patches.
> >
> >
> >
> > Patches have been tested on a make check on x86 targets=all build with
> > target board unix native-gdbserver. Also tested aarch64. Built for power
> > (because it does not use new target descriptions), but am unable to test.
> > In addition, patch six adds new test cases to unit test.
> >
> > Alan.
> >
> > gdb/Makefile.in                    |   2 +
> > gdb/common/tdesc.c                 | 445 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++
> > gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++-
> > gdb/features/aarch64-core.c        |   2 +-
> > gdb/features/aarch64-fpu.c         |   2 +-
> > gdb/features/i386/32bit-avx.c      |   2 +-
> > gdb/features/i386/32bit-avx512.c   |   2 +-
> > gdb/features/i386/32bit-core.c     |   2 +-
> > gdb/features/i386/32bit-linux.c    |   2 +-
> > gdb/features/i386/32bit-mpx.c      |   2 +-
> > gdb/features/i386/32bit-pkeys.c    |   2 +-
> > gdb/features/i386/32bit-sse.c      |   2 +-
> > gdb/features/i386/64bit-avx.c      |   2 +-
> > gdb/features/i386/64bit-avx512.c   |   2 +-
> > gdb/features/i386/64bit-core.c     |   2 +-
> > gdb/features/i386/64bit-linux.c    |   2 +-
> > gdb/features/i386/64bit-mpx.c      |   2 +-
> > gdb/features/i386/64bit-pkeys.c    |   2 +-
> > gdb/features/i386/64bit-segments.c |   2 +-
> > gdb/features/i386/64bit-sse.c      |   2 +-
> > gdb/features/i386/x32-core.c       |   2 +-
> > gdb/features/tic6x-c6xp.c          |   2 +-
> > gdb/features/tic6x-core.c          |   2 +-
> > gdb/features/tic6x-gp.c            |   2 +-
> > gdb/gdbserver/Makefile.in          |   3 +
> > gdb/gdbserver/configure.srv        |  36 -------
> > gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------
> ------------
> > gdb/gdbserver/tdesc.h              |  57 ++--------
> > gdb/regformats/regdat.sh           |   5 +-
> > gdb/target-descriptions.c          | 596 ++++++++++++------------------
> --------------------------------------------------------------------------
> > gdb/xml-tdesc.c                    |   9 ++
> > gdb/xml-tdesc.h                    |   5 +
> > 32 files changed, 974 insertions(+), 779 deletions(-)
> >
>
>

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

* Re: [PATCH V3 0/8] Remove gdbserver dependency on xml files
  2018-03-09  8:21 ` [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
  2018-03-12 14:05   ` Omair Javaid
@ 2018-03-12 17:19   ` Philipp Rudo
  2018-03-13 10:17     ` Alan Hayward
  1 sibling, 1 reply; 19+ messages in thread
From: Philipp Rudo @ 2018-03-12 17:19 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

Hi Alan,

sorry for the late response.  Here are my first findings.  I'm afraid there is
a bug somewhere in you patch set.  When running the testsuite there are quite a
lot of test from gdb.server failing (at least on s390). I have to take a closer
look at it tomorrow.

Nevertheless there are some comments i already have to your patches.

Thanks
Philipp


On Fri, 9 Mar 2018 08:21:36 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

> Ping for this series please.
> 
> Alan.
> 
> > On 1 Mar 2018, at 11:38, Alan Hayward <Alan.Hayward@arm.com> wrote:
> > 
> > V3 builds on previous review comments, and the additional patches I've
> > already pushed. Complete patch series pushed to branch users/ahayward/xml3.
> > 
> > Summary:
> > 
> > For those targets that use new style target descriptions, this set of patches
> > removes the dependency on xml files. Namely:
> > * Removes inclusion of xml files within gdbserver.
> > * Removes the requirement for the .c files in features/ to be generated from
> > cached xml files.
> > This is made possible by changing xml descriptions generated by gdbserver, so
> > that instead of including xml file names, gdbserver now generate a complete
> > xml description.
> > 
> > The second point will be required for aarch64 SVE support, where the register
> > size are variable. Creating SVE xml files for every possible vector length
> > would not be feasible. Instead the plan for aarch64 SVE is to hand write the
> > features/ .c code that would normally be generated from xml.
> > 
> > 
> > XML Generation:
> > 
> > In existing code, gdbserver uses C code auto generated from xml files to
> > create target descriptions. When sending an xml description to GDB, the
> > function tdesc_get_features_xml () creates an xml containing the name of the
> > original xml file(s). For example:
> > 
> > <!DOCTYPE target SYSTEM "gdb-target.dtd">
> > <target>
> >  <architecture>i386</architecture>
> >  <osabi>GNU/Linux</osabi>
> >  <xi:include href="32bit-core.xml"/>
> >  <xi:include href="32bit-sse.xml"/>
> >  <xi:include href="32bit-linux.xml"/>
> >  <xi:include href="32bit-avx.xml"/>
> > </target>
> > 
> > Upon receipt, GDB then makes requests to gdbserver for the contents of the
> > xml files. Gdbserver keeps full copies all the xml files inside the binary.
> > 
> > This patch series adds common code that allows gdbserver (and gdb) to turn
> > a C target description structure into xml.
> > Now when asked fort an xml description to gdb, gdbserver turns the entire
> > target description structure back into xml, without using any cached files.
> > Producing, for example:
> > 
> > <!DOCTYPE target SYSTEM "gdb-target.dtd">
> > <target>
> >  <architecture>i386</architecture>
> >  <osabi>GNU/Linux</osabi>
> >  <feature name="org.gnu.gdb.i386.core">
> >    <flags id="i386_eflags" size="4">
> >      <field name="CF" start="0" end="0"/>
> >      <field name="" start="1" end="1"/>
> >      <field name="PF" start="2" end="2"/>
> >      <field name="AF" start="4" end="4"/>
> > ...etc...
> > 
> > 
> > Patch Contents:
> > 
> > Patches 2-4 commonise the various target descriptor functionality, allowing
> > gdbserver to parse target descriptions in the same way as gdb. This series
> > does not commonise target_desc, but this is hopefully a long term goal.
> > 
> > The sixth patch adds the xml printer, which iterates through the parsing
> > generated in the previous patches.
> > 
> > The other patches are clean up patches.
> > 
> > 
> > 
> > Patches have been tested on a make check on x86 targets=all build with
> > target board unix native-gdbserver. Also tested aarch64. Built for power
> > (because it does not use new target descriptions), but am unable to test.
> > In addition, patch six adds new test cases to unit test.
> > 
> > Alan.
> > 
> > gdb/Makefile.in                    |   2 +
> > gdb/common/tdesc.c                 | 445 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > gdb/features/aarch64-core.c        |   2 +-
> > gdb/features/aarch64-fpu.c         |   2 +-
> > gdb/features/i386/32bit-avx.c      |   2 +-
> > gdb/features/i386/32bit-avx512.c   |   2 +-
> > gdb/features/i386/32bit-core.c     |   2 +-
> > gdb/features/i386/32bit-linux.c    |   2 +-
> > gdb/features/i386/32bit-mpx.c      |   2 +-
> > gdb/features/i386/32bit-pkeys.c    |   2 +-
> > gdb/features/i386/32bit-sse.c      |   2 +-
> > gdb/features/i386/64bit-avx.c      |   2 +-
> > gdb/features/i386/64bit-avx512.c   |   2 +-
> > gdb/features/i386/64bit-core.c     |   2 +-
> > gdb/features/i386/64bit-linux.c    |   2 +-
> > gdb/features/i386/64bit-mpx.c      |   2 +-
> > gdb/features/i386/64bit-pkeys.c    |   2 +-
> > gdb/features/i386/64bit-segments.c |   2 +-
> > gdb/features/i386/64bit-sse.c      |   2 +-
> > gdb/features/i386/x32-core.c       |   2 +-
> > gdb/features/tic6x-c6xp.c          |   2 +-
> > gdb/features/tic6x-core.c          |   2 +-
> > gdb/features/tic6x-gp.c            |   2 +-
> > gdb/gdbserver/Makefile.in          |   3 +
> > gdb/gdbserver/configure.srv        |  36 -------
> > gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------------------
> > gdb/gdbserver/tdesc.h              |  57 ++--------
> > gdb/regformats/regdat.sh           |   5 +-
> > gdb/target-descriptions.c          | 596 ++++++++++++--------------------------------------------------------------------------------------------
> > gdb/xml-tdesc.c                    |   9 ++
> > gdb/xml-tdesc.h                    |   5 +
> > 32 files changed, 974 insertions(+), 779 deletions(-)
> >   
> 

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

* Re: [PATCH v3 6/8] Create xml from target descriptions
  2018-03-01 11:41 ` [PATCH v3 6/8] Create xml " Alan Hayward
@ 2018-03-12 17:20   ` Philipp Rudo
  2018-03-13 18:05   ` Philipp Rudo
  1 sibling, 0 replies; 19+ messages in thread
From: Philipp Rudo @ 2018-03-12 17:20 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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);
> +}
> 

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

* Re: [PATCH v3 2/8] Commonise tdesc_reg
  2018-03-01 11:39 ` [PATCH v3 2/8] Commonise tdesc_reg Alan Hayward
@ 2018-03-12 17:20   ` Philipp Rudo
  0 siblings, 0 replies; 19+ messages in thread
From: Philipp Rudo @ 2018-03-12 17:20 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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

[...]

> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index e50a848e2f9f280a84ab139cfce4d1f17bd05884..806e87da185623da29d39d9fd74c8722f795e086 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -72,9 +72,27 @@ init_target_desc (struct target_desc *tdesc)
>  {
>    int offset = 0;
> 
> -  for (reg *reg : tdesc->reg_defs)
> +  /* Go through all the features and populate reg_defs.  */
> +  for (const tdesc_reg_up &treg : tdesc->registers)
>      {
> +      /* Fill in any blank spaces.  */
> +      while (tdesc->reg_defs.size () < treg->target_regnum)
> +	{
> +	  struct reg *reg = XCNEW (struct reg);
> +	  reg->name = "";
> +	  reg->size = 0;
> +	  reg->offset = offset;
> +	  tdesc->reg_defs.push_back (reg);
> +	}
> +
> +      gdb_assert (treg->target_regnum == 0
> +		  || treg->target_regnum == tdesc->reg_defs.size ());
> +
> +      struct reg *reg = XCNEW (struct reg);
> +      reg->name = treg->name.c_str ();
> +      reg->size = treg->bitsize;
>        reg->offset = offset;
> +      tdesc->reg_defs.push_back (reg);
>        offset += reg->size;
>      }

I think it would make sense here to also change the reg_defs vector to hold the
struct reg directly instead of a pointer to it.  Then the for-loop (should)
read like this

  gdb_assert (treg->target_regnum == 0
	      || treg->target_regnum >= tdesc->reg_defs.size ());
  tdesc->reg_defs.resize (treg->target_regnum);
  struct reg *reg = &tdesc->reg_defs.back ()
  reg->name = treg->name.c_str ();
  reg->size = treg->bitsize;
  reg->offset = offset;
  offset += reg->size;

With this you don't have to allocate so many small chunks. Furthermore in
combination with copy_target_description this construct smells like a memory
leak to me.

Thanks
Philipp

 
> @@ -241,23 +259,10 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
>  {
>    struct target_desc *tdesc = (struct target_desc *) feature;
> 
> -  while (tdesc->reg_defs.size () < regnum)
> -    {
> -      struct reg *reg = XCNEW (struct reg);
> -
> -      reg->name = "";
> -      reg->size = 0;
> -      tdesc->reg_defs.push_back (reg);
> -    }
> -
> -  gdb_assert (regnum == 0
> -	      || regnum == tdesc->reg_defs.size ());
> -
> -  struct reg *reg = XCNEW (struct reg);
> +  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
> +				  group, bitsize, type);
> 
> -  reg->name = name;
> -  reg->size = bitsize;
> -  tdesc->reg_defs.push_back (reg);
> +  tdesc->registers.emplace_back (reg);
>  }
> 
>  /* See common/tdesc.h.  */
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 5d34e29a867f8d39cd0451eb48cbf39614c9acba..3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -38,44 +38,6 @@
>  #include "completer.h"
>  #include "readline/tilde.h" /* tilde_expand */
> 
> -static type *make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype);
> -
> -/* The interface to visit different elements of target description.  */
> -
> -class tdesc_element_visitor
> -{
> -public:
> -  virtual void visit_pre (const target_desc *e)
> -  {}
> -
> -  virtual void visit_post (const target_desc *e)
> -  {}
> -
> -  virtual void visit_pre (const tdesc_feature *e)
> -  {}
> -
> -  virtual void visit_post (const tdesc_feature *e)
> -  {}
> -
> -  virtual void visit (const tdesc_type_builtin *e)
> -  {}
> -
> -  virtual void visit (const tdesc_type_vector *e)
> -  {}
> -
> -  virtual void visit (const tdesc_type_with_fields *e)
> -  {}
> -
> -  virtual void visit (const tdesc_reg *e)
> -  {}
> -};
> -
> -class tdesc_element
> -{
> -public:
> -  virtual void accept (tdesc_element_visitor &v) const = 0;
> -};
> -
>  /* Types.  */
> 
>  struct property
> @@ -88,84 +50,6 @@ struct property
>    std::string value;
>  };
> 
> -/* An individual register from a target description.  */
> -
> -struct tdesc_reg : tdesc_element
> -{
> -  tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
> -	     int regnum, int save_restore_, const char *group_,
> -	     int bitsize_, const char *type_)
> -    : name (name_), target_regnum (regnum),
> -      save_restore (save_restore_),
> -      group (group_ != NULL ? group_ : ""),
> -      bitsize (bitsize_),
> -      type (type_ != NULL ? type_ : "<unknown>")
> -  {
> -    /* If the register's type is target-defined, look it up now.  We may not
> -       have easy access to the containing feature when we want it later.  */
> -    tdesc_type = tdesc_named_type (feature, type.c_str ());
> -  }
> -
> -  virtual ~tdesc_reg () = default;
> -
> -  DISABLE_COPY_AND_ASSIGN (tdesc_reg);
> -
> -  /* The name of this register.  In standard features, it may be
> -     recognized by the architecture support code, or it may be purely
> -     for the user.  */
> -  std::string name;
> -
> -  /* The register number used by this target to refer to this
> -     register.  This is used for remote p/P packets and to determine
> -     the ordering of registers in the remote g/G packets.  */
> -  long target_regnum;
> -
> -  /* If this flag is set, GDB should save and restore this register
> -     around calls to an inferior function.  */
> -  int save_restore;
> -
> -  /* The name of the register group containing this register, or empty
> -     if the group should be automatically determined from the register's
> -     type.  This is traditionally "general", "float", "vector" but can
> -     also be an arbitrary string.  If defined the corresponding "info"
> -     command should display this register's value.  The string should be
> -     limited to alphanumeric characters and internal hyphens.  */
> -  std::string group;
> -
> -  /* The size of the register, in bits.  */
> -  int bitsize;
> -
> -  /* The type of the register.  This string corresponds to either
> -     a named type from the target description or a predefined
> -     type from GDB.  */
> -  std::string type;
> -
> -  /* The target-described type corresponding to TYPE, if found.  */
> -  struct tdesc_type *tdesc_type;
> -
> -  void accept (tdesc_element_visitor &v) const override
> -  {
> -    v.visit (this);
> -  }
> -
> -  bool operator== (const tdesc_reg &other) const
> -  {
> -    return (name == other.name
> -	    && target_regnum == other.target_regnum
> -	    && save_restore == other.save_restore
> -	    && bitsize == other.bitsize
> -	    && group == other.group
> -	    && type == other.type);
> -  }
> -
> -  bool operator!= (const tdesc_reg &other) const
> -  {
> -    return !(*this == other);
> -  }
> -};
> -
> -typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
> -
>  /* A named type from a target description.  */
> 
>  struct tdesc_type_field
> 

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

* Re: [PATCH v3 3/8] Commonise tdesc_feature
  2018-03-01 11:40 ` [PATCH v3 3/8] Commonise tdesc_feature Alan Hayward
@ 2018-03-12 17:20   ` Philipp Rudo
  0 siblings, 0 replies; 19+ messages in thread
From: Philipp Rudo @ 2018-03-12 17:20 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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

> This patch commonises tdesc_feature and makes use of it in gdbserver tdesc.
> 
> Previously in gdbserver, target_desc was interchangeable with tdesc_feature.
> Now gdbserver target_desc contains a vector of tdesc_feature.
> 
> I am now able to commonise tdesc_create_reg (wanted to do this in the
> previous patch).
> 
> I also had to commonise tdesc_type. This is ok, because I will need them
> in the next patch.
> 
> Alan.
> 
> 2018-03-01  Alan Hayward  <alan.hayward@arm.com>
> 
> gdb/
> 	* common/tdesc.c (tdesc_feature::accept): Move to here.
> 	(tdesc_feature::operator==): Likewise.
> 	(tdesc_create_reg): Likewise.
> 	* common/tdesc.h (tdesc_type_kind): Likewise.
> 	(struct tdesc_type): Likewise.
> 	(struct tdesc_feature): Likewise.
> 	* regformats/regdat.sh: Create a feature.
> 	* target-descriptions.c (tdesc_type_kind): Move from here.
> 	(tdesc_type): Likewise.
> 	(tdesc_type_up): Likewise.
> 	(tdesc_feature): Likewise.
> 	(tdesc_create_reg): Likewise.
> 
> 
> gdbserver/
> 	* tdesc.c (~target_desc): Remove implictly deleted items.
> 	(init_target_desc): Iterate all features.
> 	(tdesc_get_features_xml): Use vector.
> 	(tdesc_create_feature): Create feature.
> 	* tdesc.h (tdesc_feature) Remove
> 	(target_desc): Add features.
> 
> 
> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
> index ac6dfdf7bb4ec2c3bbbed1b091f04308dd741f21..07083e3d1d41c4f86f85ae032b45c1d6f6a105ff 100644
> --- a/gdb/common/tdesc.h
> +++ b/gdb/common/tdesc.h
> @@ -31,6 +31,7 @@ struct tdesc_type_vector;
>  struct tdesc_type_with_fields;
>  struct tdesc_reg;
>  struct target_desc;
> +struct type;

This forward declaration looks unnecessary to me.

Thanks
Philipp


> 
>  /* The interface to visit different elements of target description.  */
> 
> @@ -137,6 +138,99 @@ struct tdesc_reg : tdesc_element
> 
>  typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
> 
> +enum tdesc_type_kind
> +{
> +  /* Predefined types.  */
> +  TDESC_TYPE_BOOL,
> +  TDESC_TYPE_INT8,
> +  TDESC_TYPE_INT16,
> +  TDESC_TYPE_INT32,
> +  TDESC_TYPE_INT64,
> +  TDESC_TYPE_INT128,
> +  TDESC_TYPE_UINT8,
> +  TDESC_TYPE_UINT16,
> +  TDESC_TYPE_UINT32,
> +  TDESC_TYPE_UINT64,
> +  TDESC_TYPE_UINT128,
> +  TDESC_TYPE_CODE_PTR,
> +  TDESC_TYPE_DATA_PTR,
> +  TDESC_TYPE_IEEE_SINGLE,
> +  TDESC_TYPE_IEEE_DOUBLE,
> +  TDESC_TYPE_ARM_FPA_EXT,
> +  TDESC_TYPE_I387_EXT,
> +
> +  /* Types defined by a target feature.  */
> +  TDESC_TYPE_VECTOR,
> +  TDESC_TYPE_STRUCT,
> +  TDESC_TYPE_UNION,
> +  TDESC_TYPE_FLAGS,
> +  TDESC_TYPE_ENUM
> +};
> +
> +struct tdesc_type : tdesc_element
> +{
> +  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
> +    : name (name_), kind (kind_)
> +  {}
> +
> +  virtual ~tdesc_type () = default;
> +
> +  DISABLE_COPY_AND_ASSIGN (tdesc_type);
> +
> +  /* The name of this type.  */
> +  std::string name;
> +
> +  /* Identify the kind of this type.  */
> +  enum tdesc_type_kind kind;
> +
> +  bool operator== (const tdesc_type &other) const
> +  {
> +    return name == other.name && kind == other.kind;
> +  }
> +
> +  bool operator!= (const tdesc_type &other) const
> +  {
> +    return !(*this == other);
> +  }
> +};
> +
> +typedef std::unique_ptr<tdesc_type> tdesc_type_up;
> +
> +/* A feature from a target description.  Each feature is a collection
> +   of other elements, e.g. registers and types.  */
> +
> +struct tdesc_feature : tdesc_element
> +{
> +  tdesc_feature (const std::string &name_)
> +    : name (name_)
> +  {}
> +
> +  virtual ~tdesc_feature () = default;
> +
> +  DISABLE_COPY_AND_ASSIGN (tdesc_feature);
> +
> +  /* The name of this feature.  It may be recognized by the architecture
> +     support code.  */
> +  std::string name;
> +
> +  /* The registers associated with this feature.  */
> +  std::vector<tdesc_reg_up> registers;
> +
> +  /* The types associated with this feature.  */
> +  std::vector<tdesc_type_up> types;
> +
> +  void accept (tdesc_element_visitor &v) const override;
> +
> +  bool operator== (const tdesc_feature &other) const;
> +
> +  bool operator!= (const tdesc_feature &other) const
> +  {
> +    return !(*this == other);
> +  }
> +};
> +
> +typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
> +
>  /* Allocate a new target_desc.  */
>  target_desc *allocate_target_description (void);
> 
> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
> index a5f2de4b5ba2adb61dd098d8fc569a6339164586..ef6ed944df20d4613dd0dd338710a4c1ad67bbf2 100644
> --- a/gdb/common/tdesc.c
> +++ b/gdb/common/tdesc.c
> @@ -38,3 +38,61 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
>       have easy access to the containing feature when we want it later.  */
>    tdesc_type = tdesc_named_type (feature, type.c_str ());
>  }
> +
> +void tdesc_feature::accept (tdesc_element_visitor &v) const
> +{
> +  v.visit_pre (this);
> +
> +  for (const tdesc_type_up &type : types)
> +    type->accept (v);
> +
> +  for (const tdesc_reg_up &reg : registers)
> +    reg->accept (v);
> +
> +  v.visit_post (this);
> +}
> +
> +bool tdesc_feature::operator== (const tdesc_feature &other) const
> +{
> +  if (name != other.name)
> +    return false;
> +
> +  if (registers.size () != other.registers.size ())
> +    return false;
> +
> +  for (int ix = 0; ix < registers.size (); ix++)
> +    {
> +      const tdesc_reg_up &reg1 = registers[ix];
> +      const tdesc_reg_up &reg2 = other.registers[ix];
> +
> +      if (reg1 != reg2 && *reg1 != *reg2)
> +	return false;
> +      }
> +
> +  if (types.size () != other.types.size ())
> +    return false;
> +
> +  for (int ix = 0; ix < types.size (); ix++)
> +    {
> +      const tdesc_type_up &type1 = types[ix];
> +      const tdesc_type_up &type2 = other.types[ix];
> +
> +      if (type1 != type2 && *type1 != *type2)
> +	return false;
> +    }
> +
> +  return true;
> +}
> +
> +/* See common/tdesc.h.  */
> +
> +void
> +tdesc_create_reg (struct tdesc_feature *feature, const char *name,
> +		  int regnum, int save_restore, const char *group,
> +		  int bitsize, const char *type)
> +{
> +  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
> +				  group, bitsize, type);
> +
> +  feature->registers.emplace_back (reg);
> +}
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index b0b8fa3f9e63590398d0de52359ae8ee0271557b..8534069d616b97dc82df04939a4ac935aa4e1cad 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -24,16 +24,10 @@
>  #include "regdef.h"
>  #include <vector>
> 
> -struct tdesc_feature
> -{
> -  /* The registers associated with this feature.  */
> -  std::vector<tdesc_reg_up> registers;
> -};
> -
>  /* A target description.  Inherit from tdesc_feature so that target_desc
>     can be used as tdesc_feature.  */
> 
> -struct target_desc : tdesc_feature
> +struct target_desc
>  {
>    /* A vector of elements of register definitions that
>       describe the inferior's register set.  */
> @@ -42,6 +36,9 @@ struct target_desc : tdesc_feature
>    /* The register cache size, in bytes.  */
>    int registers_size;
> 
> +  /* XML features in this target description.  */
> +  std::vector<tdesc_feature_up> features;
> +
>  #ifndef IN_PROCESS_AGENT
>    /* An array of register names.  These are the "expedite" registers:
>       registers whose values are sent along with stop replies.  */
> @@ -56,9 +53,6 @@ struct target_desc : tdesc_feature
>       fields features, arch, and osabi in tdesc_get_features_xml.  */
>    const char *xmltarget = NULL;
> 
> -  /* XML features in this target description.  */
> -  VEC (char_ptr) *features = NULL;
> -
>    /* The value of <architecture> element in the XML, replying GDB.  */
>    const char *arch = NULL;
> 
> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index 806e87da185623da29d39d9fd74c8722f795e086..72769ff00fd073d93807a4aaa59ec19ec6292641 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -23,19 +23,11 @@
> 
>  target_desc::~target_desc ()
>  {
> -  int i;
> -
>    for (reg *reg : reg_defs)
>      xfree (reg);
> 
>    xfree ((char *) arch);
>    xfree ((char *) osabi);
> -
> -  char *f;
> -
> -  for (i = 0; VEC_iterate (char_ptr, features, i, f); i++)
> -    xfree (f);
> -  VEC_free (char_ptr, features);
>  }
> 
>  bool target_desc::operator== (const target_desc &other) const
> @@ -73,28 +65,29 @@ init_target_desc (struct target_desc *tdesc)
>    int offset = 0;
> 
>    /* Go through all the features and populate reg_defs.  */
> -  for (const tdesc_reg_up &treg : tdesc->registers)
> -    {
> -      /* Fill in any blank spaces.  */
> -      while (tdesc->reg_defs.size () < treg->target_regnum)
> -	{
> -	  struct reg *reg = XCNEW (struct reg);
> -	  reg->name = "";
> -	  reg->size = 0;
> -	  reg->offset = offset;
> -	  tdesc->reg_defs.push_back (reg);
> -	}
> -
> -      gdb_assert (treg->target_regnum == 0
> -		  || treg->target_regnum == tdesc->reg_defs.size ());
> -
> -      struct reg *reg = XCNEW (struct reg);
> -      reg->name = treg->name.c_str ();
> -      reg->size = treg->bitsize;
> -      reg->offset = offset;
> -      tdesc->reg_defs.push_back (reg);
> -      offset += reg->size;
> -    }
> +  for (const tdesc_feature_up &feature : tdesc->features)
> +    for (const tdesc_reg_up &treg : feature->registers)
> +      {
> +	/* Fill in any blank spaces.  */
> +	while (tdesc->reg_defs.size () < treg->target_regnum)
> +	  {
> +	    struct reg *reg = XCNEW (struct reg);
> +	    reg->name = "";
> +	    reg->size = 0;
> +	    reg->offset = offset;
> +	    tdesc->reg_defs.push_back (reg);
> +	  }
> +
> +	gdb_assert (treg->target_regnum == 0
> +		    || treg->target_regnum == tdesc->reg_defs.size ());
> +
> +	struct reg *reg = XCNEW (struct reg);
> +	reg->name = treg->name.c_str ();
> +	reg->size = treg->bitsize;
> +	reg->offset = offset;
> +	tdesc->reg_defs.push_back (reg);
> +	offset += reg->size;
> +      }
> 
>    tdesc->registers_size = offset / 8;
> 
> @@ -157,7 +150,7 @@ tdesc_get_features_xml (target_desc *tdesc)
>  {
>    /* Either .xmltarget or .features is not NULL.  */
>    gdb_assert (tdesc->xmltarget != NULL
> -	      || (tdesc->features != NULL
> +	      || (!tdesc->features.empty ()
>  		  && tdesc->arch != NULL));
> 
>    if (tdesc->xmltarget == NULL)
> @@ -177,12 +170,10 @@ tdesc_get_features_xml (target_desc *tdesc)
>  	  buffer += "</osabi>";
>  	}
> 
> -      char *xml;
> -
> -      for (int i = 0; VEC_iterate (char_ptr, tdesc->features, i, xml); i++)
> +      for (const tdesc_feature_up &feature : tdesc->features)
>  	{
>  	  buffer += "<xi:include href=\"";
> -	  buffer += xml;
> +	  buffer += feature->name;
>  	  buffer += "\"/>";
>  	}
> 
> @@ -195,19 +186,16 @@ tdesc_get_features_xml (target_desc *tdesc)
>  }
>  #endif
> 
> -struct tdesc_type
> -{};
> -
>  /* See common/tdesc.h.  */
> 
>  struct tdesc_feature *
>  tdesc_create_feature (struct target_desc *tdesc, const char *name,
>  		      const char *xml)
>  {
> -#ifndef IN_PROCESS_AGENT
> -  VEC_safe_push (char_ptr, tdesc->features, xstrdup (xml));
> -#endif
> -  return tdesc;
> +  struct tdesc_feature *new_feature = new tdesc_feature
> +    (xml != nullptr ? xml : name);
> +  tdesc->features.emplace_back (new_feature);
> +  return new_feature;
>  }
> 
>  /* See common/tdesc.h.  */
> @@ -252,21 +240,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id)
> 
>  /* See common/tdesc.h.  */
> 
> -void
> -tdesc_create_reg (struct tdesc_feature *feature, const char *name,
> -		  int regnum, int save_restore, const char *group,
> -		  int bitsize, const char *type)
> -{
> -  struct target_desc *tdesc = (struct target_desc *) feature;
> -
> -  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
> -				  group, bitsize, type);
> -
> -  tdesc->registers.emplace_back (reg);
> -}
> -
> -/* See common/tdesc.h.  */
> -
>  struct tdesc_type *
>  tdesc_create_vector (struct tdesc_feature *feature, const char *name,
>  		     struct tdesc_type *field_type, int count)
> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index ce1627120d9439082709c82c5804724f39477eb1..8c6e191596350fb4e983f8736985d9832f41e2d3 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -131,7 +131,7 @@ do
>      echo "{"
>      echo "  static struct target_desc tdesc_${name}_s;"
>      echo "  struct target_desc *result = &tdesc_${name}_s;"
> -
> +    echo "  struct tdesc_feature *feature = tdesc_create_feature (result, \"${name}\");"
>      continue
>    elif test "${type}" = "xmltarget"; then
>      xmltarget="${entry}"
> @@ -149,7 +149,7 @@ do
>      echo "$0: $1 does not specify \`\`name''." 1>&2
>      exit 1
>    else
> -    echo "  tdesc_create_reg ((struct tdesc_feature *) result, \"${entry}\","
> +    echo "  tdesc_create_reg (feature, \"${entry}\","
>      echo "  0, 0, NULL, ${type}, NULL);"
> 
>      offset=`expr ${offset} + ${type}`
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250..a453eddec0cd97b692c4c9ace3f093c3fbc67422 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -67,64 +67,6 @@ struct tdesc_type_field
>    int start, end;
>  };
> 
> -enum tdesc_type_kind
> -{
> -  /* Predefined types.  */
> -  TDESC_TYPE_BOOL,
> -  TDESC_TYPE_INT8,
> -  TDESC_TYPE_INT16,
> -  TDESC_TYPE_INT32,
> -  TDESC_TYPE_INT64,
> -  TDESC_TYPE_INT128,
> -  TDESC_TYPE_UINT8,
> -  TDESC_TYPE_UINT16,
> -  TDESC_TYPE_UINT32,
> -  TDESC_TYPE_UINT64,
> -  TDESC_TYPE_UINT128,
> -  TDESC_TYPE_CODE_PTR,
> -  TDESC_TYPE_DATA_PTR,
> -  TDESC_TYPE_IEEE_SINGLE,
> -  TDESC_TYPE_IEEE_DOUBLE,
> -  TDESC_TYPE_ARM_FPA_EXT,
> -  TDESC_TYPE_I387_EXT,
> -
> -  /* Types defined by a target feature.  */
> -  TDESC_TYPE_VECTOR,
> -  TDESC_TYPE_STRUCT,
> -  TDESC_TYPE_UNION,
> -  TDESC_TYPE_FLAGS,
> -  TDESC_TYPE_ENUM
> -};
> -
> -struct tdesc_type : tdesc_element
> -{
> -  tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
> -    : name (name_), kind (kind_)
> -  {}
> -
> -  virtual ~tdesc_type () = default;
> -
> -  DISABLE_COPY_AND_ASSIGN (tdesc_type);
> -
> -  /* The name of this type.   */
> -  std::string name;
> -
> -  /* Identify the kind of this type.  */
> -  enum tdesc_type_kind kind;
> -
> -  bool operator== (const tdesc_type &other) const
> -  {
> -    return name == other.name && kind == other.kind;
> -  }
> -
> -  bool operator!= (const tdesc_type &other) const
> -  {
> -    return !(*this == other);
> -  }
> -};
> -
> -typedef std::unique_ptr<tdesc_type> tdesc_type_up;
> -
>  struct tdesc_type_builtin : tdesc_type
>  {
>    tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
> @@ -428,82 +370,6 @@ make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype)
>    return gdb_type.get_type ();
>  }
> 
> -/* A feature from a target description.  Each feature is a collection
> -   of other elements, e.g. registers and types.  */
> -
> -struct tdesc_feature : tdesc_element
> -{
> -  tdesc_feature (const std::string &name_)
> -    : name (name_)
> -  {}
> -
> -  virtual ~tdesc_feature () = default;
> -
> -  DISABLE_COPY_AND_ASSIGN (tdesc_feature);
> -
> -  /* The name of this feature.  It may be recognized by the architecture
> -     support code.  */
> -  std::string name;
> -
> -  /* The registers associated with this feature.  */
> -  std::vector<tdesc_reg_up> registers;
> -
> -  /* The types associated with this feature.  */
> -  std::vector<tdesc_type_up> types;
> -
> -  void accept (tdesc_element_visitor &v) const override
> -  {
> -    v.visit_pre (this);
> -
> -    for (const tdesc_type_up &type : types)
> -      type->accept (v);
> -
> -    for (const tdesc_reg_up &reg : registers)
> -      reg->accept (v);
> -
> -    v.visit_post (this);
> -  }
> -
> -  bool operator== (const tdesc_feature &other) const
> -  {
> -    if (name != other.name)
> -      return false;
> -
> -    if (registers.size () != other.registers.size ())
> -      return false;
> -
> -    for (int ix = 0; ix < registers.size (); ix++)
> -      {
> -	const tdesc_reg_up &reg1 = registers[ix];
> -	const tdesc_reg_up &reg2 = other.registers[ix];
> -
> -	if (reg1 != reg2 && *reg1 != *reg2)
> -	  return false;
> -      }
> -
> -    if (types.size () != other.types.size ())
> -      return false;
> -
> -    for (int ix = 0; ix < types.size (); ix++)
> -      {
> -	const tdesc_type_up &type1 = types[ix];
> -	const tdesc_type_up &type2 = other.types[ix];
> -
> -	if (type1 != type2 && *type1 != *type2)
> -	  return false;
> -      }
> -
> -    return true;
> -  }
> -
> -  bool operator!= (const tdesc_feature &other) const
> -  {
> -    return !(*this == other);
> -  }
> -};
> -
> -typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
> -
>  /* A target description.  */
> 
>  struct target_desc : tdesc_element
> @@ -1358,20 +1224,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
>  				      tdesc_remote_register_number);
>    set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p);
>  }
> -
> 
> -
> -/* See common/tdesc.h.  */
> -
> -void
> -tdesc_create_reg (struct tdesc_feature *feature, const char *name,
> -		  int regnum, int save_restore, const char *group,
> -		  int bitsize, const char *type)
> -{
> -  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
> -				  group, bitsize, type);
> -
> -  feature->registers.emplace_back (reg);
> -}
> 
>  /* See common/tdesc.h.  */
> 

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

* Re: [PATCH V3 0/8] Remove gdbserver dependency on xml files
  2018-03-12 17:19   ` Philipp Rudo
@ 2018-03-13 10:17     ` Alan Hayward
  2018-03-13 17:58       ` Philipp Rudo
  0 siblings, 1 reply; 19+ messages in thread
From: Alan Hayward @ 2018-03-13 10:17 UTC (permalink / raw)
  To: Philipp Rudo; +Cc: gdb-patches, nd

Could you please try gdb.gdb/unittest.exp and gdb.server/unittest.exp
As that’ll test every xml file your build uses.
And, if there are any errors could you please send me the .log file contents - think I
should have enough info from there to debug (don’t have an s390 to try this myself).

Your review comments on the other patches make sense. Haven’t had a chance to
try the changes yet, but I’ll reply there if I have any issues with them.

Many thanks for the review!

Alan.

> On 12 Mar 2018, at 17:19, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> 
> Hi Alan,
> 
> sorry for the late response.  Here are my first findings.  I'm afraid there is
> a bug somewhere in you patch set.  When running the testsuite there are quite a
> lot of test from gdb.server failing (at least on s390). I have to take a closer
> look at it tomorrow.
> 
> Nevertheless there are some comments i already have to your patches.
> 
> Thanks
> Philipp
> 
> 
> On Fri, 9 Mar 2018 08:21:36 +0000
> Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
>> Ping for this series please.
>> 
>> Alan.
>> 
>>> On 1 Mar 2018, at 11:38, Alan Hayward <Alan.Hayward@arm.com> wrote:
>>> 
>>> V3 builds on previous review comments, and the additional patches I've
>>> already pushed. Complete patch series pushed to branch users/ahayward/xml3.
>>> 
>>> Summary:
>>> 
>>> For those targets that use new style target descriptions, this set of patches
>>> removes the dependency on xml files. Namely:
>>> * Removes inclusion of xml files within gdbserver.
>>> * Removes the requirement for the .c files in features/ to be generated from
>>> cached xml files.
>>> This is made possible by changing xml descriptions generated by gdbserver, so
>>> that instead of including xml file names, gdbserver now generate a complete
>>> xml description.
>>> 
>>> The second point will be required for aarch64 SVE support, where the register
>>> size are variable. Creating SVE xml files for every possible vector length
>>> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
>>> features/ .c code that would normally be generated from xml.
>>> 
>>> 
>>> XML Generation:
>>> 
>>> In existing code, gdbserver uses C code auto generated from xml files to
>>> create target descriptions. When sending an xml description to GDB, the
>>> function tdesc_get_features_xml () creates an xml containing the name of the
>>> original xml file(s). For example:
>>> 
>>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
>>> <target>
>>> <architecture>i386</architecture>
>>> <osabi>GNU/Linux</osabi>
>>> <xi:include href="32bit-core.xml"/>
>>> <xi:include href="32bit-sse.xml"/>
>>> <xi:include href="32bit-linux.xml"/>
>>> <xi:include href="32bit-avx.xml"/>
>>> </target>
>>> 
>>> Upon receipt, GDB then makes requests to gdbserver for the contents of the
>>> xml files. Gdbserver keeps full copies all the xml files inside the binary.
>>> 
>>> This patch series adds common code that allows gdbserver (and gdb) to turn
>>> a C target description structure into xml.
>>> Now when asked fort an xml description to gdb, gdbserver turns the entire
>>> target description structure back into xml, without using any cached files.
>>> Producing, for example:
>>> 
>>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
>>> <target>
>>> <architecture>i386</architecture>
>>> <osabi>GNU/Linux</osabi>
>>> <feature name="org.gnu.gdb.i386.core">
>>>   <flags id="i386_eflags" size="4">
>>>     <field name="CF" start="0" end="0"/>
>>>     <field name="" start="1" end="1"/>
>>>     <field name="PF" start="2" end="2"/>
>>>     <field name="AF" start="4" end="4"/>
>>> ...etc...
>>> 
>>> 
>>> Patch Contents:
>>> 
>>> Patches 2-4 commonise the various target descriptor functionality, allowing
>>> gdbserver to parse target descriptions in the same way as gdb. This series
>>> does not commonise target_desc, but this is hopefully a long term goal.
>>> 
>>> The sixth patch adds the xml printer, which iterates through the parsing
>>> generated in the previous patches.
>>> 
>>> The other patches are clean up patches.
>>> 
>>> 
>>> 
>>> Patches have been tested on a make check on x86 targets=all build with
>>> target board unix native-gdbserver. Also tested aarch64. Built for power
>>> (because it does not use new target descriptions), but am unable to test.
>>> In addition, patch six adds new test cases to unit test.
>>> 
>>> Alan.
>>> 
>>> gdb/Makefile.in                    |   2 +
>>> gdb/common/tdesc.c                 | 445 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> gdb/features/aarch64-core.c        |   2 +-
>>> gdb/features/aarch64-fpu.c         |   2 +-
>>> gdb/features/i386/32bit-avx.c      |   2 +-
>>> gdb/features/i386/32bit-avx512.c   |   2 +-
>>> gdb/features/i386/32bit-core.c     |   2 +-
>>> gdb/features/i386/32bit-linux.c    |   2 +-
>>> gdb/features/i386/32bit-mpx.c      |   2 +-
>>> gdb/features/i386/32bit-pkeys.c    |   2 +-
>>> gdb/features/i386/32bit-sse.c      |   2 +-
>>> gdb/features/i386/64bit-avx.c      |   2 +-
>>> gdb/features/i386/64bit-avx512.c   |   2 +-
>>> gdb/features/i386/64bit-core.c     |   2 +-
>>> gdb/features/i386/64bit-linux.c    |   2 +-
>>> gdb/features/i386/64bit-mpx.c      |   2 +-
>>> gdb/features/i386/64bit-pkeys.c    |   2 +-
>>> gdb/features/i386/64bit-segments.c |   2 +-
>>> gdb/features/i386/64bit-sse.c      |   2 +-
>>> gdb/features/i386/x32-core.c       |   2 +-
>>> gdb/features/tic6x-c6xp.c          |   2 +-
>>> gdb/features/tic6x-core.c          |   2 +-
>>> gdb/features/tic6x-gp.c            |   2 +-
>>> gdb/gdbserver/Makefile.in          |   3 +
>>> gdb/gdbserver/configure.srv        |  36 -------
>>> gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------------------
>>> gdb/gdbserver/tdesc.h              |  57 ++--------
>>> gdb/regformats/regdat.sh           |   5 +-
>>> gdb/target-descriptions.c          | 596 ++++++++++++--------------------------------------------------------------------------------------------
>>> gdb/xml-tdesc.c                    |   9 ++
>>> gdb/xml-tdesc.h                    |   5 +
>>> 32 files changed, 974 insertions(+), 779 deletions(-)
>>> 
>> 
> 


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

* Re: [PATCH V3 0/8] Remove gdbserver dependency on xml files
  2018-03-13 10:17     ` Alan Hayward
@ 2018-03-13 17:58       ` Philipp Rudo
  0 siblings, 0 replies; 19+ messages in thread
From: Philipp Rudo @ 2018-03-13 17:58 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

Hi Alan,

i managed to track the bug down (cannot say i understood what the problem is
though). It's in patch #6 please see the comments there.

> Could you please try gdb.gdb/unittest.exp and gdb.server/unittest.exp
> As that’ll test every xml file your build uses.

For the unittests you have to register the target description first. That's not
done for s390 currently.  However i hacked something together and it helped me
find an other bug in patch #6.

Thanks
Philipp

> And, if there are any errors could you please send me the .log file contents - think I
> should have enough info from there to debug (don’t have an s390 to try this myself).
> 
> Your review comments on the other patches make sense. Haven’t had a chance to
> try the changes yet, but I’ll reply there if I have any issues with them.
> 
> Many thanks for the review!
> 
> Alan.
> 
> > On 12 Mar 2018, at 17:19, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> > 
> > Hi Alan,
> > 
> > sorry for the late response.  Here are my first findings.  I'm afraid there is
> > a bug somewhere in you patch set.  When running the testsuite there are quite a
> > lot of test from gdb.server failing (at least on s390). I have to take a closer
> > look at it tomorrow.
> > 
> > Nevertheless there are some comments i already have to your patches.
> > 
> > Thanks
> > Philipp
> > 
> > 
> > On Fri, 9 Mar 2018 08:21:36 +0000
> > Alan Hayward <Alan.Hayward@arm.com> wrote:
> >   
> >> Ping for this series please.
> >> 
> >> Alan.
> >>   
> >>> On 1 Mar 2018, at 11:38, Alan Hayward <Alan.Hayward@arm.com> wrote:
> >>> 
> >>> V3 builds on previous review comments, and the additional patches I've
> >>> already pushed. Complete patch series pushed to branch users/ahayward/xml3.
> >>> 
> >>> Summary:
> >>> 
> >>> For those targets that use new style target descriptions, this set of patches
> >>> removes the dependency on xml files. Namely:
> >>> * Removes inclusion of xml files within gdbserver.
> >>> * Removes the requirement for the .c files in features/ to be generated from
> >>> cached xml files.
> >>> This is made possible by changing xml descriptions generated by gdbserver, so
> >>> that instead of including xml file names, gdbserver now generate a complete
> >>> xml description.
> >>> 
> >>> The second point will be required for aarch64 SVE support, where the register
> >>> size are variable. Creating SVE xml files for every possible vector length
> >>> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
> >>> features/ .c code that would normally be generated from xml.
> >>> 
> >>> 
> >>> XML Generation:
> >>> 
> >>> In existing code, gdbserver uses C code auto generated from xml files to
> >>> create target descriptions. When sending an xml description to GDB, the
> >>> function tdesc_get_features_xml () creates an xml containing the name of the
> >>> original xml file(s). For example:
> >>> 
> >>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> >>> <target>
> >>> <architecture>i386</architecture>
> >>> <osabi>GNU/Linux</osabi>
> >>> <xi:include href="32bit-core.xml"/>
> >>> <xi:include href="32bit-sse.xml"/>
> >>> <xi:include href="32bit-linux.xml"/>
> >>> <xi:include href="32bit-avx.xml"/>
> >>> </target>
> >>> 
> >>> Upon receipt, GDB then makes requests to gdbserver for the contents of the
> >>> xml files. Gdbserver keeps full copies all the xml files inside the binary.
> >>> 
> >>> This patch series adds common code that allows gdbserver (and gdb) to turn
> >>> a C target description structure into xml.
> >>> Now when asked fort an xml description to gdb, gdbserver turns the entire
> >>> target description structure back into xml, without using any cached files.
> >>> Producing, for example:
> >>> 
> >>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> >>> <target>
> >>> <architecture>i386</architecture>
> >>> <osabi>GNU/Linux</osabi>
> >>> <feature name="org.gnu.gdb.i386.core">
> >>>   <flags id="i386_eflags" size="4">
> >>>     <field name="CF" start="0" end="0"/>
> >>>     <field name="" start="1" end="1"/>
> >>>     <field name="PF" start="2" end="2"/>
> >>>     <field name="AF" start="4" end="4"/>
> >>> ...etc...
> >>> 
> >>> 
> >>> Patch Contents:
> >>> 
> >>> Patches 2-4 commonise the various target descriptor functionality, allowing
> >>> gdbserver to parse target descriptions in the same way as gdb. This series
> >>> does not commonise target_desc, but this is hopefully a long term goal.
> >>> 
> >>> The sixth patch adds the xml printer, which iterates through the parsing
> >>> generated in the previous patches.
> >>> 
> >>> The other patches are clean up patches.
> >>> 
> >>> 
> >>> 
> >>> Patches have been tested on a make check on x86 targets=all build with
> >>> target board unix native-gdbserver. Also tested aarch64. Built for power
> >>> (because it does not use new target descriptions), but am unable to test.
> >>> In addition, patch six adds new test cases to unit test.
> >>> 
> >>> Alan.
> >>> 
> >>> gdb/Makefile.in                    |   2 +
> >>> gdb/common/tdesc.c                 | 445 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>> gdb/common/tdesc.h                 | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>> gdb/features/aarch64-core.c        |   2 +-
> >>> gdb/features/aarch64-fpu.c         |   2 +-
> >>> gdb/features/i386/32bit-avx.c      |   2 +-
> >>> gdb/features/i386/32bit-avx512.c   |   2 +-
> >>> gdb/features/i386/32bit-core.c     |   2 +-
> >>> gdb/features/i386/32bit-linux.c    |   2 +-
> >>> gdb/features/i386/32bit-mpx.c      |   2 +-
> >>> gdb/features/i386/32bit-pkeys.c    |   2 +-
> >>> gdb/features/i386/32bit-sse.c      |   2 +-
> >>> gdb/features/i386/64bit-avx.c      |   2 +-
> >>> gdb/features/i386/64bit-avx512.c   |   2 +-
> >>> gdb/features/i386/64bit-core.c     |   2 +-
> >>> gdb/features/i386/64bit-linux.c    |   2 +-
> >>> gdb/features/i386/64bit-mpx.c      |   2 +-
> >>> gdb/features/i386/64bit-pkeys.c    |   2 +-
> >>> gdb/features/i386/64bit-segments.c |   2 +-
> >>> gdb/features/i386/64bit-sse.c      |   2 +-
> >>> gdb/features/i386/x32-core.c       |   2 +-
> >>> gdb/features/tic6x-c6xp.c          |   2 +-
> >>> gdb/features/tic6x-core.c          |   2 +-
> >>> gdb/features/tic6x-gp.c            |   2 +-
> >>> gdb/gdbserver/Makefile.in          |   3 +
> >>> gdb/gdbserver/configure.srv        |  36 -------
> >>> gdb/gdbserver/tdesc.c              | 240 ++++++++++++++++++------------------------
> >>> gdb/gdbserver/tdesc.h              |  57 ++--------
> >>> gdb/regformats/regdat.sh           |   5 +-
> >>> gdb/target-descriptions.c          | 596 ++++++++++++--------------------------------------------------------------------------------------------
> >>> gdb/xml-tdesc.c                    |   9 ++
> >>> gdb/xml-tdesc.h                    |   5 +
> >>> 32 files changed, 974 insertions(+), 779 deletions(-)
> >>>   
> >>   
> >   
> 

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

* Re: [PATCH v3 6/8] Create xml from target descriptions
  2018-03-01 11:41 ` [PATCH v3 6/8] Create xml " Alan Hayward
  2018-03-12 17:20   ` Philipp Rudo
@ 2018-03-13 18:05   ` Philipp Rudo
  2018-03-14 10:09     ` Alan Hayward
  1 sibling, 1 reply; 19+ messages in thread
From: Philipp Rudo @ 2018-03-13 18:05 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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);
> +}
> 

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

* Re: [PATCH v3 6/8] Create xml from target descriptions
  2018-03-13 18:05   ` Philipp Rudo
@ 2018-03-14 10:09     ` Alan Hayward
  0 siblings, 0 replies; 19+ messages in thread
From: Alan Hayward @ 2018-03-14 10:09 UTC (permalink / raw)
  To: Philipp Rudo; +Cc: gdb-patches, nd



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



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

end of thread, other threads:[~2018-03-14 10:09 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-01 11:38 [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
2018-03-01 11:39 ` [PATCH v3 2/8] Commonise tdesc_reg Alan Hayward
2018-03-12 17:20   ` Philipp Rudo
2018-03-01 11:39 ` [PATCH V3 1/8] Move tdesc header funcs to c file Alan Hayward
2018-03-01 11:40 ` [PATCH v3 4/8] Commonise tdesc types Alan Hayward
2018-03-01 11:40 ` [PATCH v3 3/8] Commonise tdesc_feature Alan Hayward
2018-03-12 17:20   ` Philipp Rudo
2018-03-01 11:40 ` [PATCH v3 5/8] Add tdesc osabi and architecture functions Alan Hayward
2018-03-01 11:41 ` [PATCH v3 8/8] Remove xml files from gdbserver Alan Hayward
2018-03-01 11:41 ` [PATCH v3 7/8]: Remove xml file references from target descriptions Alan Hayward
2018-03-01 11:41 ` [PATCH v3 6/8] Create xml " Alan Hayward
2018-03-12 17:20   ` Philipp Rudo
2018-03-13 18:05   ` Philipp Rudo
2018-03-14 10:09     ` Alan Hayward
2018-03-09  8:21 ` [PATCH V3 0/8] Remove gdbserver dependency on xml files Alan Hayward
2018-03-12 14:05   ` Omair Javaid
2018-03-12 17:19   ` Philipp Rudo
2018-03-13 10:17     ` Alan Hayward
2018-03-13 17:58       ` Philipp Rudo

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).