* [PATCH v2 0/8] Remove XML files from gdbserver
@ 2018-01-24 9:26 Alan Hayward
2018-01-24 9:26 ` [PATCH v2 1/8] Move tdesc header funcs to c file Alan Hayward
` (10 more replies)
0 siblings, 11 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:26 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
Following review comments from Yao, this patch series is mostly a
reordering of the code from the previous patch series.
This set of patches removes the need for gdbserver to ship the xml files
in the binary for those targets that use new style target descriptions.
In exisiting code, gdbserver uses C code auto generated from xml files to
create target descriptions. When sending an xml description to GDB, it
creates an xml containing just the name of the original xml file.
Upon receipt, GDB reads and parses a local copy of xml file.
With this new patch, we add common code that allows gdbserver and gdb
to turn a C target description structure into xml. To do this we must
first ensure that when gdbserver parses a target description it retains
enough information to turn it back into xml.
The second patch commonises tdesc_reg, the third patch commonises
tdesc_feature and the fifth patch commonises the tdesc type structures.
This enables gdbserver to store all required information about a target
description.
The sixth patch adds the xml printer.
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
(becuase it does not use new target descriptions), but am unable to test.
In addition, patch four adds new test cases to unit test.
Alan.
gdb/Makefile.in | 1 -
gdb/arch/tdesc.c | 425 ------------------------
gdb/arch/tdesc.h | 307 +-----------------
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 | 229 +++++++------
gdb/gdbserver/tdesc.h | 58 +++-
gdb/regformats/regdat.sh | 5 +-
gdb/target-descriptions.c | 924 +++++++++++++++++++++++++++++++++++++++--------------
gdb/xml-tdesc.c | 9 -
gdb/xml-tdesc.h | 5 -
32 files changed, 920 insertions(+), 1124 deletions(-)
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/8] Move tdesc header funcs to c file
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
@ 2018-01-24 9:26 ` Alan Hayward
2018-01-24 9:27 ` [PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc Alan Hayward
` (9 subsequent siblings)
10 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:26 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 makes the
header look messy. Patch does not change any functionality.
Alan.
2018-01-24 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 783500598b5599e51f8e768a876fdfe499cdf0ef..705b968b98af9405682507acbcfb41356da62857 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 c39b5e7d27e879c5d3fb49b6e04d1eb6128f8eef..359ab97361c07ff2cbc2c8d5ff392c6233c329ff 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] 32+ messages in thread
* [PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
2018-01-24 9:26 ` [PATCH v2 1/8] Move tdesc header funcs to c file Alan Hayward
@ 2018-01-24 9:27 ` Alan Hayward
2018-01-25 13:12 ` Philipp Rudo
2018-01-24 9:28 ` [PATCH v2 3/8] Use tdesc_feature in gdbserver tdesc Alan Hayward
` (8 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:27 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
tdesc_create_reg is changed to create a tdesc_reg and place it in
tdesc_feature.
After registers have been parsed, init_target_desc is called. I
have expanded this function so that it now populates the reg_defs
directly from the tdesc_reg registers.
The long term goal is to change the other files within gdbserver to
access member functions instead of reg_defs directly. reg_defs can
then be removed. This is out of the scope of this patch series.
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.
The gdbserver makefile change is a little strange. I had to add using
"..". I think there is a rule that needs changing, but I have no idea
where it is. Note that this is the first file in arch which is common
for all builds. I'm also wondering if tdesc.c/h should be in arch/ ?
Maybe it should be moved to common?
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* Makefile.in: Add arch/tdesc.c
* arch/tdesc.c: New file.
* arch/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 arch/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 364ea7afc66d00d0be44c4369f3ddc924de7b6aa..05ecf64451de4df3a5593d158404d526a71c0b29 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -911,6 +911,7 @@ COMMON_SFILES = \
agent.c \
annotate.c \
arch-utils.c \
+ arch/tdesc.c \
auto-load.c \
auxv.c \
ax-gdb.c \
diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
index cc11651dcaa7abe81598b69f509ef34ff0d94dbf..e957822b348182b8cae9809cd21f72974ffe05c5 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/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,99 @@ 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) = 0;
+ virtual void visit_post (const target_desc *e) = 0;
+
+ virtual void visit_pre (const tdesc_feature *e) = 0;
+ virtual void visit_post (const tdesc_feature *e) = 0;
+
+ virtual void visit (const tdesc_type_builtin *e) = 0;
+ virtual void visit (const tdesc_type_vector *e) = 0;
+ virtual void visit (const tdesc_type_with_fields *e) = 0;
+
+ virtual void visit (const tdesc_reg *e) = 0;
+};
+
+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/arch/tdesc.c b/gdb/arch/tdesc.c
new file mode 100644
index 0000000000000000000000000000000000000000..3d84ad8d3fa2e017f4c6f7d137a3be2b58427370
--- /dev/null
+++ b/gdb/arch/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 "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 3ce086d70f23df445b174c49c489ec8415d7614a..8ae124875dd08f31e8424b18281cb1454ed40edb 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -194,6 +194,7 @@ SFILES = \
$(srcdir)/arch/arm.c \
$(srcdir)/arch/arm-get-next-pcs.c \
$(srcdir)/arch/arm-linux.c \
+ $(srcdir)/../arch/tdesc.c \
$(srcdir)/common/btrace-common.c \
$(srcdir)/common/buffer.c \
$(srcdir)/common/cleanups.c \
@@ -232,6 +233,7 @@ TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
OBS = \
agent.o \
+ arch/tdesc.o \
ax.o \
btrace-common.o \
buffer.o \
@@ -391,6 +393,7 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
$(XM_CLIBS) $(LIBGNU) $(LIBIBERTY)
IPA_OBJS = \
+ arch/tdesc-ipa.o \
ax-ipa.o \
common-utils-ipa.o \
errors-ipa.o \
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 705b968b98af9405682507acbcfb41356da62857..cfc1a194def019ecf899e9184385be94de39d08c 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 359ab97361c07ff2cbc2c8d5ff392c6233c329ff..5dec636baa6b899490e8c700eea21d363fb79fa3 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 arch/tdesc.h. */
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 1b20a12d769718e591dea6df8183c2e9ecfac990..881ef78d2e3adde409e8585deb67c96e2f0594ed 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -38,30 +38,6 @@
#include "completer.h"
#include "readline/tilde.h" /* tilde_expand */
-/* The interface to visit different elements of target description. */
-
-class tdesc_element_visitor
-{
-public:
- virtual void visit_pre (const target_desc *e) = 0;
- virtual void visit_post (const target_desc *e) = 0;
-
- virtual void visit_pre (const tdesc_feature *e) = 0;
- virtual void visit_post (const tdesc_feature *e) = 0;
-
- virtual void visit (const tdesc_type_builtin *e) = 0;
- virtual void visit (const tdesc_type_vector *e) = 0;
- virtual void visit (const tdesc_type_with_fields *e) = 0;
-
- virtual void visit (const tdesc_reg *e) = 0;
-};
-
-class tdesc_element
-{
-public:
- virtual void accept (tdesc_element_visitor &v) const = 0;
-};
-
/* Types. */
struct property
@@ -74,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] 32+ messages in thread
* [PATCH v2 4/8] Move make_gdb_type functions within file
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (2 preceding siblings ...)
2018-01-24 9:28 ` [PATCH v2 3/8] Use tdesc_feature in gdbserver tdesc Alan Hayward
@ 2018-01-24 9:28 ` Alan Hayward
2018-01-24 9:29 ` [PATCH v2 5/8] Use tdesc types in gdbserver tdesc Alan Hayward
` (6 subsequent siblings)
10 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:28 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
This patch moves the function bodies for the various make_gdb_type
functions outside of the class declaration.
It doesn't not change any functionality and produces the same binary.
In the next patch I will move the class declarations to a different
file. This patch makes the diff of the next patch easier to read.
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* target-descriptions.c (tdesc_type_builtin::make_gdb_type): Move
location in file.
(tdesc_type_vector::make_gdb_type): Likewise.
(tdesc_type_with_fields::make_gdb_type_struct): Likewise.
(tdesc_type_with_fields::make_gdb_type_union): Likewise.
(tdesc_type_with_fields::make_gdb_type_flags): Likewise.
(tdesc_type_with_fields::make_gdb_type_enum): Likewise.
(tdesc_type_with_fields::make_gdb_type): Likewise.
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 6df29521eb2319919c7e20df14ca5aebc1d25321..8ead19efa24b3cb154c895c484e791b92ff61387 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -78,80 +78,7 @@ struct tdesc_type_builtin : tdesc_type
v.visit (this);
}
- type *make_gdb_type (struct gdbarch *gdbarch) const override
- {
- switch (this->kind)
- {
- /* Predefined types. */
- case TDESC_TYPE_BOOL:
- return builtin_type (gdbarch)->builtin_bool;
-
- case TDESC_TYPE_INT8:
- return builtin_type (gdbarch)->builtin_int8;
-
- case TDESC_TYPE_INT16:
- return builtin_type (gdbarch)->builtin_int16;
-
- case TDESC_TYPE_INT32:
- return builtin_type (gdbarch)->builtin_int32;
-
- case TDESC_TYPE_INT64:
- return builtin_type (gdbarch)->builtin_int64;
-
- case TDESC_TYPE_INT128:
- return builtin_type (gdbarch)->builtin_int128;
-
- case TDESC_TYPE_UINT8:
- return builtin_type (gdbarch)->builtin_uint8;
-
- case TDESC_TYPE_UINT16:
- return builtin_type (gdbarch)->builtin_uint16;
-
- case TDESC_TYPE_UINT32:
- return builtin_type (gdbarch)->builtin_uint32;
-
- case TDESC_TYPE_UINT64:
- return builtin_type (gdbarch)->builtin_uint64;
-
- case TDESC_TYPE_UINT128:
- return builtin_type (gdbarch)->builtin_uint128;
-
- case TDESC_TYPE_CODE_PTR:
- return builtin_type (gdbarch)->builtin_func_ptr;
-
- case TDESC_TYPE_DATA_PTR:
- return builtin_type (gdbarch)->builtin_data_ptr;
- }
-
- type *gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
- if (gdb_type != NULL)
- return gdb_type;
-
- switch (this->kind)
- {
- case TDESC_TYPE_IEEE_SINGLE:
- return arch_float_type (gdbarch, -1, "builtin_type_ieee_single",
- floatformats_ieee_single);
-
- case TDESC_TYPE_IEEE_DOUBLE:
- return arch_float_type (gdbarch, -1, "builtin_type_ieee_double",
- floatformats_ieee_double);
-
- case TDESC_TYPE_ARM_FPA_EXT:
- return arch_float_type (gdbarch, -1, "builtin_type_arm_ext",
- floatformats_arm_ext);
-
- case TDESC_TYPE_I387_EXT:
- return arch_float_type (gdbarch, -1, "builtin_type_i387_ext",
- floatformats_i387_ext);
- }
-
- internal_error (__FILE__, __LINE__,
- "Type \"%s\" has an unknown kind %d",
- this->name.c_str (), this->kind);
-
- return NULL;
- }
+ type *make_gdb_type (struct gdbarch *gdbarch) const override;
};
/* tdesc_type for vector types. */
@@ -168,18 +95,7 @@ struct tdesc_type_vector : tdesc_type
v.visit (this);
}
- type *make_gdb_type (struct gdbarch *gdbarch) const override
- {
- type *vector_gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
- if (vector_gdb_type != NULL)
- return vector_gdb_type;
-
- type *element_gdb_type = this->element_type->make_gdb_type (gdbarch);
- vector_gdb_type = init_vector_type (element_gdb_type, this->count);
- TYPE_NAME (vector_gdb_type) = xstrdup (this->name.c_str ());
-
- return vector_gdb_type;
- }
+ type *make_gdb_type (struct gdbarch *gdbarch) const override;
struct tdesc_type *element_type;
int count;
@@ -199,151 +115,252 @@ struct tdesc_type_with_fields : tdesc_type
v.visit (this);
}
- type *make_gdb_type_struct (struct gdbarch *gdbarch) const
- {
- type *struct_gdb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
- TYPE_NAME (struct_gdb_type) = xstrdup (this->name.c_str ());
- TYPE_TAG_NAME (struct_gdb_type) = TYPE_NAME (struct_gdb_type);
+ type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
+ type *make_gdb_type_union (struct gdbarch *gdbarch) const;
+ type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
+ type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
+ type *make_gdb_type (struct gdbarch *gdbarch) const override;
- for (const tdesc_type_field &f : this->fields)
- {
- if (f.start != -1 && f.end != -1)
- {
- /* Bitfield. */
- struct field *fld;
- struct type *field_gdb_type;
- int bitsize, total_size;
-
- /* This invariant should be preserved while creating types. */
- gdb_assert (this->size != 0);
- if (f.type != NULL)
- field_gdb_type = f.type->make_gdb_type (gdbarch);
- else if (this->size > 4)
- field_gdb_type = builtin_type (gdbarch)->builtin_uint64;
- else
- field_gdb_type = builtin_type (gdbarch)->builtin_uint32;
-
- fld = append_composite_type_field_raw
- (struct_gdb_type, xstrdup (f.name.c_str ()), field_gdb_type);
-
- /* For little-endian, BITPOS counts from the LSB of
- the structure and marks the LSB of the field. For
- big-endian, BITPOS counts from the MSB of the
- structure and marks the MSB of the field. Either
- way, it is the number of bits to the "left" of the
- field. To calculate this in big-endian, we need
- the total size of the structure. */
- bitsize = f.end - f.start + 1;
- total_size = this->size * TARGET_CHAR_BIT;
- if (gdbarch_bits_big_endian (gdbarch))
- SET_FIELD_BITPOS (fld[0], total_size - f.start - bitsize);
- else
- SET_FIELD_BITPOS (fld[0], f.start);
- FIELD_BITSIZE (fld[0]) = bitsize;
- }
- else
- {
- gdb_assert (f.start == -1 && f.end == -1);
- type *field_gdb_type = f.type->make_gdb_type (gdbarch);
- append_composite_type_field (struct_gdb_type,
- xstrdup (f.name.c_str ()),
- field_gdb_type);
- }
- }
+ std::vector<tdesc_type_field> fields;
+ int size;
+};
+
+type *
+tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
+{
+ switch (this->kind)
+ {
+ /* Predefined types. */
+ case TDESC_TYPE_BOOL:
+ return builtin_type (gdbarch)->builtin_bool;
- if (this->size != 0)
- TYPE_LENGTH (struct_gdb_type) = this->size;
+ case TDESC_TYPE_INT8:
+ return builtin_type (gdbarch)->builtin_int8;
- return struct_gdb_type;
- }
+ case TDESC_TYPE_INT16:
+ return builtin_type (gdbarch)->builtin_int16;
- type *make_gdb_type_union (struct gdbarch *gdbarch) const
- {
- type *union_gdb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
- TYPE_NAME (union_gdb_type) = xstrdup (this->name.c_str ());
+ case TDESC_TYPE_INT32:
+ return builtin_type (gdbarch)->builtin_int32;
- for (const tdesc_type_field &f : this->fields)
- {
- type* field_gdb_type = f.type->make_gdb_type (gdbarch);
- append_composite_type_field (union_gdb_type, xstrdup (f.name.c_str ()),
+ case TDESC_TYPE_INT64:
+ return builtin_type (gdbarch)->builtin_int64;
+
+ case TDESC_TYPE_INT128:
+ return builtin_type (gdbarch)->builtin_int128;
+
+ case TDESC_TYPE_UINT8:
+ return builtin_type (gdbarch)->builtin_uint8;
+
+ case TDESC_TYPE_UINT16:
+ return builtin_type (gdbarch)->builtin_uint16;
+
+ case TDESC_TYPE_UINT32:
+ return builtin_type (gdbarch)->builtin_uint32;
+
+ case TDESC_TYPE_UINT64:
+ return builtin_type (gdbarch)->builtin_uint64;
+
+ case TDESC_TYPE_UINT128:
+ return builtin_type (gdbarch)->builtin_uint128;
+
+ case TDESC_TYPE_CODE_PTR:
+ return builtin_type (gdbarch)->builtin_func_ptr;
+
+ case TDESC_TYPE_DATA_PTR:
+ return builtin_type (gdbarch)->builtin_data_ptr;
+ }
+
+ type *gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
+ if (gdb_type != NULL)
+ return gdb_type;
+
+ switch (this->kind)
+ {
+ case TDESC_TYPE_IEEE_SINGLE:
+ return arch_float_type (gdbarch, -1, "builtin_type_ieee_single",
+ floatformats_ieee_single);
+
+ case TDESC_TYPE_IEEE_DOUBLE:
+ return arch_float_type (gdbarch, -1, "builtin_type_ieee_double",
+ floatformats_ieee_double);
+
+ case TDESC_TYPE_ARM_FPA_EXT:
+ return arch_float_type (gdbarch, -1, "builtin_type_arm_ext",
+ floatformats_arm_ext);
+
+ case TDESC_TYPE_I387_EXT:
+ return arch_float_type (gdbarch, -1, "builtin_type_i387_ext",
+ floatformats_i387_ext);
+ }
+
+ internal_error (__FILE__, __LINE__,
+ "Type \"%s\" has an unknown kind %d",
+ this->name.c_str (), this->kind);
+
+ return NULL;
+}
+
+type *
+tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
+{
+ type *vector_gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
+ if (vector_gdb_type != NULL)
+ return vector_gdb_type;
+
+ type *element_gdb_type = this->element_type->make_gdb_type (gdbarch);
+ vector_gdb_type = init_vector_type (element_gdb_type, this->count);
+ TYPE_NAME (vector_gdb_type) = xstrdup (this->name.c_str ());
+
+ return vector_gdb_type;
+}
+
+type *
+tdesc_type_with_fields::make_gdb_type_struct (struct gdbarch *gdbarch) const
+{
+ type *struct_gdb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
+ TYPE_NAME (struct_gdb_type) = xstrdup (this->name.c_str ());
+ TYPE_TAG_NAME (struct_gdb_type) = TYPE_NAME (struct_gdb_type);
+
+ for (const tdesc_type_field &f : this->fields)
+ {
+ if (f.start != -1 && f.end != -1)
+ {
+ /* Bitfield. */
+ struct field *fld;
+ struct type *field_gdb_type;
+ int bitsize, total_size;
+
+ /* This invariant should be preserved while creating types. */
+ gdb_assert (this->size != 0);
+ if (f.type != NULL)
+ field_gdb_type = f.type->make_gdb_type (gdbarch);
+ else if (this->size > 4)
+ field_gdb_type = builtin_type (gdbarch)->builtin_uint64;
+ else
+ field_gdb_type = builtin_type (gdbarch)->builtin_uint32;
+
+ fld = append_composite_type_field_raw
+ (struct_gdb_type, xstrdup (f.name.c_str ()), field_gdb_type);
+
+ /* For little-endian, BITPOS counts from the LSB of
+ the structure and marks the LSB of the field. For
+ big-endian, BITPOS counts from the MSB of the
+ structure and marks the MSB of the field. Either
+ way, it is the number of bits to the "left" of the
+ field. To calculate this in big-endian, we need
+ the total size of the structure. */
+ bitsize = f.end - f.start + 1;
+ total_size = this->size * TARGET_CHAR_BIT;
+ if (gdbarch_bits_big_endian (gdbarch))
+ SET_FIELD_BITPOS (fld[0], total_size - f.start - bitsize);
+ else
+ SET_FIELD_BITPOS (fld[0], f.start);
+ FIELD_BITSIZE (fld[0]) = bitsize;
+ }
+ else
+ {
+ gdb_assert (f.start == -1 && f.end == -1);
+ type *field_gdb_type = f.type->make_gdb_type (gdbarch);
+ append_composite_type_field (struct_gdb_type,
+ xstrdup (f.name.c_str ()),
+ field_gdb_type);
+ }
+ }
+
+ if (this->size != 0)
+ TYPE_LENGTH (struct_gdb_type) = this->size;
+
+ return struct_gdb_type;
+}
+
+type *
+tdesc_type_with_fields::make_gdb_type_union (struct gdbarch *gdbarch) const
+{
+ type *union_gdb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
+ TYPE_NAME (union_gdb_type) = xstrdup (this->name.c_str ());
+
+ for (const tdesc_type_field &f : this->fields)
+ {
+ type* field_gdb_type = f.type->make_gdb_type (gdbarch);
+ append_composite_type_field (union_gdb_type, xstrdup (f.name.c_str ()),
field_gdb_type);
- /* If any of the children of a union are vectors, flag the
- union as a vector also. This allows e.g. a union of two
- vector types to show up automatically in "info vector". */
- if (TYPE_VECTOR (field_gdb_type))
- TYPE_VECTOR (union_gdb_type) = 1;
- }
+ /* If any of the children of a union are vectors, flag the
+ union as a vector also. This allows e.g. a union of two
+ vector types to show up automatically in "info vector". */
+ if (TYPE_VECTOR (field_gdb_type))
+ TYPE_VECTOR (union_gdb_type) = 1;
+ }
- return union_gdb_type;
- }
+ return union_gdb_type;
+}
- type *make_gdb_type_flags (struct gdbarch *gdbarch) const
- {
- type *flags_gdb_type = arch_flags_type (gdbarch, this->name.c_str (),
+type *
+tdesc_type_with_fields::make_gdb_type_flags (struct gdbarch *gdbarch) const
+{
+ type *flags_gdb_type = arch_flags_type (gdbarch, this->name.c_str (),
this->size * TARGET_CHAR_BIT);
- for (const tdesc_type_field &f : this->fields)
- {
+ for (const tdesc_type_field &f : this->fields)
+ {
int bitsize = f.end - f.start + 1;
gdb_assert (f.type != NULL);
type *field_gdb_type = f.type->make_gdb_type (gdbarch);
append_flags_type_field (flags_gdb_type, f.start, bitsize,
field_gdb_type, f.name.c_str ());
- }
+ }
- return flags_gdb_type;
- }
+ return flags_gdb_type;
+}
- type *make_gdb_type_enum (struct gdbarch *gdbarch) const
- {
- type *enum_gdb_type = arch_type (gdbarch, TYPE_CODE_ENUM,
+type *
+tdesc_type_with_fields::make_gdb_type_enum (struct gdbarch *gdbarch) const
+{
+ type *enum_gdb_type = arch_type (gdbarch, TYPE_CODE_ENUM,
this->size * TARGET_CHAR_BIT,
this->name.c_str ());
- TYPE_UNSIGNED (enum_gdb_type) = 1;
- for (const tdesc_type_field &f : this->fields)
- {
+ TYPE_UNSIGNED (enum_gdb_type) = 1;
+ for (const tdesc_type_field &f : this->fields)
+ {
struct field *fld
= append_composite_type_field_raw (enum_gdb_type,
xstrdup (f.name.c_str ()),
NULL);
SET_FIELD_BITPOS (fld[0], f.start);
- }
+ }
- return enum_gdb_type;
- }
+ return enum_gdb_type;
+}
- type *make_gdb_type (struct gdbarch *gdbarch) const override
- {
- type *gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
- if (gdb_type != NULL)
- return gdb_type;
+type *
+tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
+{
+ type *gdb_type = tdesc_find_type (gdbarch, this->name.c_str ());
+ if (gdb_type != NULL)
+ return gdb_type;
- switch (this->kind)
+ switch (this->kind)
{
- case TDESC_TYPE_STRUCT:
- return make_gdb_type_struct (gdbarch);
- case TDESC_TYPE_UNION:
- return make_gdb_type_union (gdbarch);
- case TDESC_TYPE_FLAGS:
- return make_gdb_type_flags (gdbarch);
- case TDESC_TYPE_ENUM:
- return make_gdb_type_enum (gdbarch);
+ case TDESC_TYPE_STRUCT:
+ return make_gdb_type_struct (gdbarch);
+ case TDESC_TYPE_UNION:
+ return make_gdb_type_union (gdbarch);
+ case TDESC_TYPE_FLAGS:
+ return make_gdb_type_flags (gdbarch);
+ case TDESC_TYPE_ENUM:
+ return make_gdb_type_enum (gdbarch);
}
- internal_error (__FILE__, __LINE__,
- "Type \"%s\" has an unknown kind %d",
- this->name.c_str (), this->kind);
-
- return NULL;
- }
+ internal_error (__FILE__, __LINE__,
+ "Type \"%s\" has an unknown kind %d",
+ this->name.c_str (), this->kind);
- std::vector<tdesc_type_field> fields;
- int size;
-};
+ return NULL;
+}
/* A target description. */
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 3/8] Use tdesc_feature in gdbserver tdesc
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
2018-01-24 9:26 ` [PATCH v2 1/8] Move tdesc header funcs to c file Alan Hayward
2018-01-24 9:27 ` [PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc Alan Hayward
@ 2018-01-24 9:28 ` Alan Hayward
2018-01-25 13:12 ` Philipp Rudo
2018-01-24 9:28 ` [PATCH v2 4/8] Move make_gdb_type functions within file Alan Hayward
` (7 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:28 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
This patch commonises tdesc_feature and makes use of it in gdbserver tdesc.
Updated tdesc_create_feature so that it now creates a vector of features.
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 a later patch.
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* arch/tdesc.c (tdesc_feature::accept): Move to here.
(tdesc_feature::operator==): Likewise.
(tdesc_create_reg): Likewise.
* arch/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/arch/tdesc.h b/gdb/arch/tdesc.h
index e957822b348182b8cae9809cd21f72974ffe05c5..480475fff873a19710befdf39e2bf3053be3efb3 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/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. */
@@ -125,6 +126,104 @@ 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);
+ }
+
+ /* Construct, if necessary, and return the GDB type implementing this
+ target type for architecture GDBARCH. */
+
+ virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
+};
+
+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/arch/tdesc.c b/gdb/arch/tdesc.c
index 3d84ad8d3fa2e017f4c6f7d137a3be2b58427370..603ccd79434e1dccd17e69ef193193e673835dd0 100644
--- a/gdb/arch/tdesc.c
+++ b/gdb/arch/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 ® : 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 ®1 = registers[ix];
+ const tdesc_reg_up ®2 = 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 arch/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 cfc1a194def019ecf899e9184385be94de39d08c..8ae6cddc1896af99d86206d159fb952a0f666043 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 5dec636baa6b899490e8c700eea21d363fb79fa3..bc94d83ae58b9e79d8b71d0ce21c073f11765dd4 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,15 @@ tdesc_get_features_xml (target_desc *tdesc)
}
#endif
-struct tdesc_type
-{};
-
/* See arch/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 (name);
+ tdesc->features.emplace_back (new_feature);
+ return new_feature;
}
/* See arch/tdesc.h. */
@@ -252,21 +239,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id)
/* See arch/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 arch/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 881ef78d2e3adde409e8585deb67c96e2f0594ed..6df29521eb2319919c7e20df14ca5aebc1d25321 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -67,69 +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);
- }
-
- /* Construct, if necessary, and return the GDB type implementing this
- target type for architecture GDBARCH. */
-
- virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
-};
-
-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)
@@ -408,82 +345,6 @@ struct tdesc_type_with_fields : tdesc_type
int size;
};
-/* 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 ® : 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 ®1 = registers[ix];
- const tdesc_reg_up ®2 = 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
@@ -1338,20 +1199,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
tdesc_remote_register_number);
set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p);
}
-
-
-/* See arch/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 arch/tdesc.h. */
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 5/8] Use tdesc types in gdbserver tdesc
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (3 preceding siblings ...)
2018-01-24 9:28 ` [PATCH v2 4/8] Move make_gdb_type functions within file Alan Hayward
@ 2018-01-24 9:29 ` Alan Hayward
2018-01-25 13:13 ` Philipp Rudo
2018-01-24 9:30 ` [PATCH v2 6/8] Create xml from target descriptions Alan Hayward
` (5 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:29 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 simpliest
way of getting gdbserver to retain the target description
information.
Stubs of the make_gdb_type functions are added to gdbserver
(they should never get called).
With this patch, gdbserver will now fully parse a target
description.
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* arch/tdesc.c (tdesc_predefined_type): Move to here.
(tdesc_named_type): Likewise.
(tdesc_create_vector): Likewise.
(tdesc_create_struct): Likewise.
(tdesc_set_struct_size): Likewise.
(tdesc_create_union): Likewise.
(tdesc_create_flags): Likewise.
(tdesc_create_enum): Likewise.
(tdesc_add_field): Likewise.
(tdesc_add_typed_bitfield): Likewise.
(tdesc_add_bitfield): Likewise.
(tdesc_add_flag): Likewise.
(tdesc_add_enum_value): Likewise.
* arch/tdesc.h (tdesc_type_builtin): Likewise.
(tdesc_type_vector): Likewise.
(tdesc_type_field): Likewise.
(tdesc_type_with_fields): Likewise.
* gdb/target-descriptions.c (tdesc_type_field): Move from here.
(tdesc_type_builtin): Likewise.
(tdesc_type_vector): Likewise.
(tdesc_type_with_fields): Likewise.
(tdesc_predefined_types): Likewise.
(tdesc_named_type): Likewise.
(tdesc_create_vector): Likewise.
(tdesc_create_struct): Likewise.
(tdesc_set_struct_size): Likewise.
(tdesc_create_union): Likewise.
(tdesc_create_flags): Likewise.
(tdesc_create_enum): Likewise.
(tdesc_add_field): Likewise.
(tdesc_add_typed_bitfield): Likewise.
(tdesc_add_bitfield): Likewise.
(tdesc_add_flag): Likewise.
(tdesc_add_enum_value): Likewise.
gdbserver/
* tdesc.c (tdesc_create_flags): Remove.
(tdesc_add_flag): Likewise.
(tdesc_named_type): Likewise.
(tdesc_create_union): Likewise.
(tdesc_create_struct): Likewise.
(tdesc_create_vector): Likewise.
(tdesc_add_bitfield): Likewise.
(tdesc_add_field): Likewise.
(tdesc_set_struct_size): Likewise.
(type *tdesc_type_builtin::make_gdb_type): Add stub.
(type *tdesc_type_vector::make_gdb_type): Likewise.
(type *tdesc_type_with_fields::make_gdb_type): Likewise.
diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
index 480475fff873a19710befdf39e2bf3053be3efb3..9a5bf6f11b670e04e2b51f8334bc0adaf0b43962 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/tdesc.h
@@ -189,6 +189,82 @@ struct tdesc_type : tdesc_element
typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+struct tdesc_type_builtin : tdesc_type
+{
+ tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
+ : tdesc_type (name, kind)
+ {}
+
+ void accept (tdesc_element_visitor &v) const override;
+ {
+ v.visit (this);
+ }
+
+ type *make_gdb_type (struct gdbarch *gdbarch) const override;
+};
+
+/* tdesc_type for vector types. */
+
+struct tdesc_type_vector : tdesc_type
+{
+ tdesc_type_vector (const std::string &name, tdesc_type *element_type_,
+ int count_)
+ : tdesc_type (name, TDESC_TYPE_VECTOR),
+ element_type (element_type_), count (count_)
+ {}
+
+ void accept (tdesc_element_visitor &v) const override
+ {
+ v.visit (this);
+ }
+
+ type *make_gdb_type (struct gdbarch *gdbarch) const override;
+
+ struct tdesc_type *element_type;
+ int count;
+};
+
+/* A named type from a target description. */
+
+struct tdesc_type_field
+{
+ tdesc_type_field (const std::string &name_, tdesc_type *type_,
+ int start_, int end_)
+ : name (name_), type (type_), start (start_), end (end_)
+ {}
+
+ std::string name;
+ struct tdesc_type *type;
+ /* For non-enum-values, either both are -1 (non-bitfield), or both are
+ not -1 (bitfield). For enum values, start is the value (which could be
+ -1), end is -1. */
+ int start, end;
+};
+
+/* tdesc_type for struct, union, flags, and enum types. */
+
+struct tdesc_type_with_fields : tdesc_type
+{
+ tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
+ int size_ = 0)
+ : tdesc_type (name, kind), size (size_)
+ {}
+
+ void accept (tdesc_element_visitor &v) const override
+ {
+ v.visit (this);
+ }
+
+ type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
+ type *make_gdb_type_union (struct gdbarch *gdbarch) const;
+ type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
+ type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
+ type *make_gdb_type (struct gdbarch *gdbarch) const override;
+
+ std::vector<tdesc_type_field> fields;
+ int size;
+};
+
/* A feature from a target description. Each feature is a collection
of other elements, e.g. registers and types. */
diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
index 603ccd79434e1dccd17e69ef193193e673835dd0..9518571d03d394ee7cbf78b31974818201c889cd 100644
--- a/gdb/arch/tdesc.c
+++ b/gdb/arch/tdesc.c
@@ -39,6 +39,28 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
tdesc_type = tdesc_named_type (feature, type.c_str ());
}
+/* Predefined types. */
+static tdesc_type_builtin tdesc_predefined_types[] =
+{
+ { "bool", TDESC_TYPE_BOOL },
+ { "int8", TDESC_TYPE_INT8 },
+ { "int16", TDESC_TYPE_INT16 },
+ { "int32", TDESC_TYPE_INT32 },
+ { "int64", TDESC_TYPE_INT64 },
+ { "int128", TDESC_TYPE_INT128 },
+ { "uint8", TDESC_TYPE_UINT8 },
+ { "uint16", TDESC_TYPE_UINT16 },
+ { "uint32", TDESC_TYPE_UINT32 },
+ { "uint64", TDESC_TYPE_UINT64 },
+ { "uint128", TDESC_TYPE_UINT128 },
+ { "code_ptr", TDESC_TYPE_CODE_PTR },
+ { "data_ptr", TDESC_TYPE_DATA_PTR },
+ { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
+ { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
+ { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
+ { "i387_ext", TDESC_TYPE_I387_EXT }
+};
+
void tdesc_feature::accept (tdesc_element_visitor &v) const
{
v.visit_pre (this);
@@ -84,6 +106,36 @@ bool tdesc_feature::operator== (const tdesc_feature &other) const
return true;
}
+/* Lookup a predefined type. */
+
+static struct tdesc_type *
+tdesc_predefined_type (enum tdesc_type_kind kind)
+{
+ for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+ if (tdesc_predefined_types[ix].kind == kind)
+ return &tdesc_predefined_types[ix];
+
+ gdb_assert_not_reached ("bad predefined tdesc type");
+}
+
+/* See arch/tdesc.h. */
+
+struct tdesc_type *
+tdesc_named_type (const struct tdesc_feature *feature, const char *id)
+{
+ /* First try target-defined types. */
+ for (const tdesc_type_up &type : feature->types)
+ if (type->name == id)
+ return type.get ();
+
+ /* Next try the predefined types. */
+ for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+ if (tdesc_predefined_types[ix].name == id)
+ return &tdesc_predefined_types[ix];
+
+ return NULL;
+}
+
/* See arch/tdesc.h. */
void
@@ -96,3 +148,144 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
feature->registers.emplace_back (reg);
}
+
+/* See arch/tdesc.h. */
+
+struct tdesc_type *
+tdesc_create_vector (struct tdesc_feature *feature, const char *name,
+ struct tdesc_type *field_type, int count)
+{
+ tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See arch/tdesc.h. */
+
+tdesc_type_with_fields *
+tdesc_create_struct (struct tdesc_feature *feature, const char *name)
+{
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See arch/tdesc.h. */
+
+void
+tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
+{
+ gdb_assert (type->kind == TDESC_TYPE_STRUCT);
+ gdb_assert (size > 0);
+ type->size = size;
+}
+
+/* See arch/tdesc.h. */
+
+tdesc_type_with_fields *
+tdesc_create_union (struct tdesc_feature *feature, const char *name)
+{
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See arch/tdesc.h. */
+
+tdesc_type_with_fields *
+tdesc_create_flags (struct tdesc_feature *feature, const char *name,
+ int size)
+{
+ gdb_assert (size > 0);
+
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+tdesc_type_with_fields *
+tdesc_create_enum (struct tdesc_feature *feature, const char *name,
+ int size)
+{
+ gdb_assert (size > 0);
+
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See arch/tdesc.h. */
+
+void
+tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
+ struct tdesc_type *field_type)
+{
+ gdb_assert (type->kind == TDESC_TYPE_UNION
+ || type->kind == TDESC_TYPE_STRUCT);
+
+ /* Initialize start and end so we know this is not a bit-field
+ when we print-c-tdesc. */
+ type->fields.emplace_back (field_name, field_type, -1, -1);
+}
+
+void
+tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
+ int start, int end, struct tdesc_type *field_type)
+{
+ gdb_assert (type->kind == TDESC_TYPE_STRUCT
+ || type->kind == TDESC_TYPE_FLAGS);
+ gdb_assert (start >= 0 && end >= start);
+
+ type->fields.emplace_back (field_name, field_type, start, end);
+}
+
+/* See arch/tdesc.h. */
+
+void
+tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
+ int start, int end)
+{
+ struct tdesc_type *field_type;
+
+ gdb_assert (start >= 0 && end >= start);
+
+ if (type->size > 4)
+ field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
+ else
+ field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
+
+ tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
+}
+
+/* See arch/tdesc.h. */
+
+void
+tdesc_add_flag (tdesc_type_with_fields *type, int start,
+ const char *flag_name)
+{
+ gdb_assert (type->kind == TDESC_TYPE_FLAGS
+ || type->kind == TDESC_TYPE_STRUCT);
+
+ type->fields.emplace_back (flag_name,
+ tdesc_predefined_type (TDESC_TYPE_BOOL),
+ start, start);
+}
+
+void
+tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
+ const char *name)
+{
+ gdb_assert (type->kind == TDESC_TYPE_ENUM);
+ type->fields.emplace_back (name,
+ tdesc_predefined_type (TDESC_TYPE_INT32),
+ value, -1);
+}
\ No newline at end of file
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index bc94d83ae58b9e79d8b71d0ce21c073f11765dd4..f0bd266a54601484df74ee1c5f8dce6fe04661c4 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -197,70 +197,17 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
return new_feature;
}
-/* See arch/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
- int size)
-{
- return NULL;
-}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
- const char *flag_name)
-{}
-
-/* See arch/tdesc.h. */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
- return NULL;
-}
-
-/* See arch/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *id)
+type *tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
{
- return NULL;
+ error (_("Cannot create gdbtypes."));
}
-/* See arch/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *id)
-{
- return NULL;
-}
-
-/* See arch/tdesc.h. */
-
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
- struct tdesc_type *field_type, int count)
+type *tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
{
- return NULL;
+ error (_("Cannot create gdbtypes."));
}
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
- int start, int end)
-{}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
- struct tdesc_type *field_type)
-{}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
+type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
{
+ error (_("Cannot create gdbtypes."));
}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 8ead19efa24b3cb154c895c484e791b92ff61387..4f11120dce4092f15de99680a7c4868c4a2f4493 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -50,80 +50,6 @@ struct property
std::string value;
};
-/* A named type from a target description. */
-
-struct tdesc_type_field
-{
- tdesc_type_field (const std::string &name_, tdesc_type *type_,
- int start_, int end_)
- : name (name_), type (type_), start (start_), end (end_)
- {}
-
- std::string name;
- struct tdesc_type *type;
- /* For non-enum-values, either both are -1 (non-bitfield), or both are
- not -1 (bitfield). For enum values, start is the value (which could be
- -1), end is -1. */
- int start, end;
-};
-
-struct tdesc_type_builtin : tdesc_type
-{
- tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
- : tdesc_type (name, kind)
- {}
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-
- type *make_gdb_type (struct gdbarch *gdbarch) const override;
-};
-
-/* tdesc_type for vector types. */
-
-struct tdesc_type_vector : tdesc_type
-{
- tdesc_type_vector (const std::string &name, tdesc_type *element_type_, int count_)
- : tdesc_type (name, TDESC_TYPE_VECTOR),
- element_type (element_type_), count (count_)
- {}
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-
- type *make_gdb_type (struct gdbarch *gdbarch) const override;
-
- struct tdesc_type *element_type;
- int count;
-};
-
-/* tdesc_type for struct, union, flags, and enum types. */
-
-struct tdesc_type_with_fields : tdesc_type
-{
- tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
- int size_ = 0)
- : tdesc_type (name, kind), size (size_)
- {}
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-
- type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
- type *make_gdb_type_union (struct gdbarch *gdbarch) const;
- type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
- type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
- type *make_gdb_type (struct gdbarch *gdbarch) const override;
-
- std::vector<tdesc_type_field> fields;
- int size;
-};
type *
tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
@@ -733,58 +659,6 @@ tdesc_feature_name (const struct tdesc_feature *feature)
return feature->name.c_str ();
}
-/* Predefined types. */
-static tdesc_type_builtin tdesc_predefined_types[] =
-{
- { "bool", TDESC_TYPE_BOOL },
- { "int8", TDESC_TYPE_INT8 },
- { "int16", TDESC_TYPE_INT16 },
- { "int32", TDESC_TYPE_INT32 },
- { "int64", TDESC_TYPE_INT64 },
- { "int128", TDESC_TYPE_INT128 },
- { "uint8", TDESC_TYPE_UINT8 },
- { "uint16", TDESC_TYPE_UINT16 },
- { "uint32", TDESC_TYPE_UINT32 },
- { "uint64", TDESC_TYPE_UINT64 },
- { "uint128", TDESC_TYPE_UINT128 },
- { "code_ptr", TDESC_TYPE_CODE_PTR },
- { "data_ptr", TDESC_TYPE_DATA_PTR },
- { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
- { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
- { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
- { "i387_ext", TDESC_TYPE_I387_EXT }
-};
-
-/* Lookup a predefined type. */
-
-static struct tdesc_type *
-tdesc_predefined_type (enum tdesc_type_kind kind)
-{
- for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
- if (tdesc_predefined_types[ix].kind == kind)
- return &tdesc_predefined_types[ix];
-
- gdb_assert_not_reached ("bad predefined tdesc type");
-}
-
-/* See arch/tdesc.h. */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
- /* First try target-defined types. */
- for (const tdesc_type_up &type : feature->types)
- if (type->name == id)
- return type.get ();
-
- /* Next try the predefined types. */
- for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
- if (tdesc_predefined_types[ix].name == id)
- return &tdesc_predefined_types[ix];
-
- return NULL;
-}
-
/* Lookup type associated with ID. */
struct type *
@@ -1219,147 +1093,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
/* See arch/tdesc.h. */
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
- struct tdesc_type *field_type, int count)
-{
- tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See arch/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *name)
-{
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
- gdb_assert (type->kind == TDESC_TYPE_STRUCT);
- gdb_assert (size > 0);
- type->size = size;
-}
-
-/* See arch/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *name)
-{
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See arch/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
- int size)
-{
- gdb_assert (size > 0);
-
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-tdesc_type_with_fields *
-tdesc_create_enum (struct tdesc_feature *feature, const char *name,
- int size)
-{
- gdb_assert (size > 0);
-
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
- struct tdesc_type *field_type)
-{
- gdb_assert (type->kind == TDESC_TYPE_UNION
- || type->kind == TDESC_TYPE_STRUCT);
-
- /* Initialize start and end so we know this is not a bit-field
- when we print-c-tdesc. */
- type->fields.emplace_back (field_name, field_type, -1, -1);
-}
-
-void
-tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
- int start, int end, struct tdesc_type *field_type)
-{
- gdb_assert (type->kind == TDESC_TYPE_STRUCT
- || type->kind == TDESC_TYPE_FLAGS);
- gdb_assert (start >= 0 && end >= start);
-
- type->fields.emplace_back (field_name, field_type, start, end);
-}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
- int start, int end)
-{
- struct tdesc_type *field_type;
-
- gdb_assert (start >= 0 && end >= start);
-
- if (type->size > 4)
- field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
- else
- field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
-
- tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
-}
-
-/* See arch/tdesc.h. */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
- const char *flag_name)
-{
- gdb_assert (type->kind == TDESC_TYPE_FLAGS
- || type->kind == TDESC_TYPE_STRUCT);
-
- type->fields.emplace_back (flag_name,
- tdesc_predefined_type (TDESC_TYPE_BOOL),
- start, start);
-}
-
-void
-tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
- const char *name)
-{
- gdb_assert (type->kind == TDESC_TYPE_ENUM);
- type->fields.emplace_back (name,
- tdesc_predefined_type (TDESC_TYPE_INT32),
- value, -1);
-}
-
-/* See arch/tdesc.h. */
-
struct tdesc_feature *
tdesc_create_feature (struct target_desc *tdesc, const char *name,
const char *xml)
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 6/8] Create xml from target descriptions
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (4 preceding siblings ...)
2018-01-24 9:29 ` [PATCH v2 5/8] Use tdesc types in gdbserver tdesc Alan Hayward
@ 2018-01-24 9:30 ` Alan Hayward
2018-01-25 13:14 ` Philipp Rudo
2018-01-24 9:31 ` [PATCH v2 7/8]: Remove xml file references " Alan Hayward
` (4 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:30 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.
An accept function is added to gdbserver tdesc to allow it to use
vistor classes.
Tests are added to maintenance_check_xml_descriptions which takes
each pair of tested descriptions, turns them both into xml, then back
again, and confirms the descriptions still match.
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* arch/tdesc.c (print_xml_feature::visit_post): Add xml parsing.
(print_xml_feature::visit_pre): Likewise.
(print_xml_feature::visit_post): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::visit): Likewise.
* arch/tdesc.h (print_xml_feature): Add new class.
* regformats/regdat.sh: obtain xml.
* target-descriptions.c (struct target_desc): Add xmltarget.
(print_xml_feature::visit_pre): Add xml vistor.
(tdesc_get_features_xml): Add function to get xml.
(maintenance_check_xml_descriptions): Test xml generation.
* xml-tdesc.c (target_read_description_xml_string): Add function.
* xml-tdesc.h (target_read_description_xml_string): Add declaration.
gdbserver/
* tdesc.c (void target_desc::accept): Fill in function.
(tdesc_get_features_xml): Remove old xml creation.
(print_xml_feature::visit_pre): Add xml vistor.
diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
index 9a5bf6f11b670e04e2b51f8334bc0adaf0b43962..b2e99c5ce1f7da0e6a194ca721eea96082e1fa68 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/tdesc.h
@@ -195,7 +195,7 @@ struct tdesc_type_builtin : tdesc_type
: tdesc_type (name, kind)
{}
- void accept (tdesc_element_visitor &v) const override;
+ void accept (tdesc_element_visitor &v) const override
{
v.visit (this);
}
@@ -366,4 +366,33 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
int regnum, int save_restore, const char *group,
int bitsize, const char *type);
+/* Return the tdesc in string XML format. */
+
+const char *tdesc_get_features_xml (target_desc *tdesc);
+
+/* Print target description as xml. */
+
+class print_xml_feature : public tdesc_element_visitor
+{
+public:
+ print_xml_feature (std::string *buffer_)
+ : m_buffer (buffer_)
+ {}
+
+ ~print_xml_feature ()
+ {}
+
+ void visit_pre (const target_desc *e) override;
+ void visit_post (const target_desc *e) override;
+ void visit_pre (const tdesc_feature *e) override;
+ void visit_post (const tdesc_feature *e) override;
+ void visit (const tdesc_type_builtin *type) override;
+ void visit (const tdesc_type_vector *type) override;
+ void visit (const tdesc_type_with_fields *type) override;
+ void visit (const tdesc_reg *reg) override;
+
+private:
+ std::string *m_buffer;
+};
+
#endif /* ARCH_TDESC_H */
diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
index 9518571d03d394ee7cbf78b31974818201c889cd..388c6af1b93d40cf7cb5fb97c55912b91601814b 100644
--- a/gdb/arch/tdesc.c
+++ b/gdb/arch/tdesc.c
@@ -288,4 +288,138 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
type->fields.emplace_back (name,
tdesc_predefined_type (TDESC_TYPE_INT32),
value, -1);
-}
\ No newline at end of file
+}
+
+void print_xml_feature::visit_post (const target_desc *e)
+{
+ *m_buffer += "</target>\n";
+}
+
+void print_xml_feature::visit_pre (const tdesc_feature *e)
+{
+ *m_buffer += "<feature name=\"";
+ *m_buffer += e->name;
+ *m_buffer += "\">\n";
+}
+
+void print_xml_feature::visit_post (const tdesc_feature *e)
+{
+ *m_buffer += "</feature>\n";
+}
+
+void print_xml_feature::visit (const tdesc_type_builtin *type)
+{
+ error (_("xml output is not supported type \"%s\"."), type->name.c_str ());
+}
+
+void print_xml_feature::visit (const tdesc_type_vector *type)
+{
+ *m_buffer += "<vector id=\"";
+ *m_buffer += type->name;
+ *m_buffer += "\" type=\"";
+ *m_buffer += type->element_type->name;
+ *m_buffer += "\" count=\"";
+ *m_buffer += std::to_string (type->count);
+ *m_buffer += "\"/>\n";
+}
+
+void print_xml_feature::visit (const tdesc_type_with_fields *type)
+{
+ struct tdesc_type_field *f;
+ const static char *types[] = { "struct", "union", "flags", "enum" };
+
+ gdb_assert (type->kind >= TDESC_TYPE_STRUCT && type->kind <= TDESC_TYPE_ENUM);
+ *m_buffer += "<";
+ *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
+
+ switch (type->kind)
+ {
+ case TDESC_TYPE_STRUCT:
+ case TDESC_TYPE_FLAGS:
+ *m_buffer += " id=\"";
+ *m_buffer += type->name;
+ if (type->size > 0)
+ {
+ *m_buffer += "\" size=\"";
+ *m_buffer += std::to_string (type->size);
+ }
+ *m_buffer += "\">\n";
+
+ for (const tdesc_type_field &f : type->fields)
+ {
+ *m_buffer += " <field name=\"";
+ *m_buffer += f.name;
+ if (f.start == -1)
+ {
+ *m_buffer += "\" type=\"";
+ *m_buffer += f.type->name;
+ }
+ else
+ {
+ *m_buffer += "\" start=\"";
+ *m_buffer += std::to_string (f.start);
+ *m_buffer += "\" end=\"";
+ *m_buffer += std::to_string (f.end);
+ }
+
+ *m_buffer += "\"/>\n";
+ }
+ break;
+
+ case TDESC_TYPE_ENUM:
+ *m_buffer += " id=\"";
+ *m_buffer += type->name;
+ *m_buffer += "\">\n";
+
+ for (const tdesc_type_field &f : type->fields)
+ {
+ *m_buffer += " <field name=\"";
+ *m_buffer += f.name;
+ *m_buffer += "\" start=\"";
+ *m_buffer += std::to_string (f.start);
+ *m_buffer += "\"/>\n";
+ }
+ break;
+
+ case TDESC_TYPE_UNION:
+ *m_buffer += " id=\"";
+ *m_buffer += type->name;
+ *m_buffer += "\">\n";
+
+ for (const tdesc_type_field &f : type->fields)
+ {
+ *m_buffer += " <field name=\"";
+ *m_buffer += f.name;
+ *m_buffer += "\" type=\"";
+ *m_buffer += f.type->name;
+ *m_buffer += "\"/>\n";
+ }
+ break;
+
+ default:
+ error (_("xml output is not supported type \"%s\"."),
+ type->name.c_str ());
+ }
+
+ *m_buffer += "</";
+ *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
+ *m_buffer += ">\n";
+}
+
+void print_xml_feature::visit (const tdesc_reg *reg)
+{
+ *m_buffer += "<reg name=\"";
+ *m_buffer += reg->name;
+ *m_buffer += "\" bitsize=\"";
+ *m_buffer += std::to_string (reg->bitsize);
+ *m_buffer += "\" type=\"";
+ *m_buffer += reg->type;
+ *m_buffer += "\" regnum=\"";
+ *m_buffer += std::to_string (reg->target_regnum);
+ if (reg->group.length () > 0)
+ {
+ *m_buffer += "\" group=\"";
+ *m_buffer += reg->group;
+ }
+ *m_buffer += "\"/>\n";
+}
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 8ae6cddc1896af99d86206d159fb952a0f666043..6a86f66cf2aaf9aa2b20203cad1d272d70026822 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -27,7 +27,7 @@
/* A target description. Inherit from tdesc_feature so that target_desc
can be used as tdesc_feature. */
-struct target_desc
+struct target_desc : tdesc_element
{
/* A vector of elements of register definitions that
describe the inferior's register set. */
@@ -72,7 +72,10 @@ public:
{
return !(*this == other);
}
+
#endif
+
+ void accept (tdesc_element_visitor &v) const;
};
/* Copy target description SRC to DEST. */
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index f0bd266a54601484df74ee1c5f8dce6fe04661c4..42a657f6827ed06c9d98275c7566d0c02890e7b3 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -59,6 +59,18 @@ bool target_desc::operator== (const target_desc &other) const
#endif
+void target_desc::accept (tdesc_element_visitor &v) const
+{
+#ifndef IN_PROCESS_AGENT
+ v.visit_pre (this);
+
+ for (const tdesc_feature_up &feature : features)
+ feature->accept (v);
+
+ v.visit_post (this);
+#endif
+}
+
void
init_target_desc (struct target_desc *tdesc)
{
@@ -155,30 +167,9 @@ tdesc_get_features_xml (target_desc *tdesc)
if (tdesc->xmltarget == NULL)
{
- std::string buffer ("@<?xml version=\"1.0\"?>");
-
- buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
- buffer += "<target>";
- buffer += "<architecture>";
- buffer += tdesc->arch;
- buffer += "</architecture>";
-
- if (tdesc->osabi != nullptr)
- {
- buffer += "<osabi>";
- buffer += tdesc->osabi;
- buffer += "</osabi>";
- }
-
- for (const tdesc_feature_up &feature : tdesc->features)
- {
- buffer += "<xi:include href=\"";
- buffer += feature->name;
- buffer += "\"/>";
- }
-
- buffer += "</target>";
-
+ std::string buffer ("");
+ print_xml_feature v (&buffer);
+ tdesc->accept (v);
tdesc->xmltarget = xstrdup (buffer.c_str ());
}
@@ -211,3 +202,22 @@ type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
{
error (_("Cannot create gdbtypes."));
}
+
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+#ifndef IN_PROCESS_AGENT
+ *m_buffer += "@<?xml version=\"1.0\"?>\n";
+ *m_buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n";
+ *m_buffer += "<target>\n";
+ *m_buffer += "<architecture>";
+ *m_buffer += e->arch;
+ *m_buffer += "</architecture>\n";
+
+ if (e->osabi != nullptr)
+ {
+ *m_buffer += "<osabi>";
+ *m_buffer += e->osabi;
+ *m_buffer += "</osabi>\n";
+ }
+#endif
+ }
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -180,7 +180,6 @@ echo
cat <<EOF
#ifndef IN_PROCESS_AGENT
result->expedite_regs = expedite_regs_${name};
- result->xmltarget = xmltarget_${name};
#endif
init_target_desc (result);
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 4f11120dce4092f15de99680a7c4868c4a2f4493..68a85758f7f63e6b862afa29697adee77e776d89 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -316,6 +316,8 @@ struct target_desc : tdesc_element
/* The features associated with this target. */
std::vector<tdesc_feature_up> features;
+ char *xmltarget = nullptr;
+
void accept (tdesc_element_visitor &v) const override
{
v.visit_pre (this);
@@ -1633,6 +1635,39 @@ private:
int m_next_regnum = 0;
};
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+ *m_buffer += "<?xml version=\"1.0\"?>\n";
+ *m_buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n";
+ *m_buffer += "<target>\n";
+ *m_buffer += "<architecture>";
+ *m_buffer += tdesc_architecture (e)->printable_name;
+ *m_buffer += "</architecture>\n";
+
+ if (e->osabi != GDB_OSABI_UNKNOWN)
+ {
+ *m_buffer += "<osabi>";
+ *m_buffer += gdbarch_osabi_name (tdesc_osabi (e));
+ *m_buffer += "</osabi>\n";
+ }
+}
+
+/* Return a string which is of XML format, including XML target
+ description to be sent to GDB. */
+
+const char *
+tdesc_get_features_xml (target_desc *tdesc)
+{
+ if (tdesc->xmltarget == nullptr)
+ {
+ std::string buffer ("");
+ print_xml_feature v (&buffer);
+ tdesc->accept (v);
+ tdesc->xmltarget = xstrdup (buffer.c_str ());
+ }
+ return tdesc->xmltarget;
+}
+
static void
maint_print_c_tdesc_cmd (const char *args, int from_tty)
{
@@ -1726,7 +1761,34 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
= file_read_description_xml (tdesc_xml.data ());
if (tdesc == NULL || *tdesc != *e.second)
- failed++;
+ {
+ printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
+ failed++;
+ continue;
+ }
+
+ /* Convert both descriptions to xml, and then back again. Confirm all
+ descriptions are identical. */
+
+ const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
+ const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
+ const target_desc *t_trans = target_read_description_xml_string (xml);
+ const target_desc *t_trans2 = target_read_description_xml_string (xml2);
+
+ if (t_trans == NULL || t_trans2 == NULL)
+ {
+ printf_filtered (
+ _("Could not convert descriptions for %s back to xml (%p %p)\n"),
+ e.first, t_trans, t_trans2);
+ failed++;
+ }
+ else if (*tdesc != *t_trans || *tdesc != *t_trans2)
+ {
+ printf_filtered
+ (_("Translated descriptions for %s do not match (%d %d)\n"),
+ e.first, *tdesc == *t_trans, *tdesc == *t_trans2);
+ failed++;
+ }
}
printf_filtered (_("Tested %lu XML files, %d failed\n"),
(long) selftests::xml_tdesc.size (), failed);
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index 8f0679707ad0f1e04d803f955f7fb98b4cc0c8c8..fee60e86dd10e1543b935c3cebce505d4dc828e2 100644
--- a/gdb/xml-tdesc.h
+++ b/gdb/xml-tdesc.h
@@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
otherwise. */
gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
+/* Take an xml string, parse it, and return the parsed description. Does not
+ handle a string containing includes. */
+
+const struct target_desc *target_read_description_xml_string (const char *);
+
#endif /* XML_TDESC_H */
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 9190d5f3c64ffdc6d7987d651527f597d695c5a6..f793f07c96847a3d61188fff1c5d63952cc37565 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -752,3 +752,12 @@ target_fetch_description_xml (struct target_ops *ops)
return output;
#endif
}
+
+/* Take an xml string, parse it, and return the parsed description. Does not
+ handle a string containing includes. */
+
+const struct target_desc *
+target_read_description_xml_string (const char *xml_str)
+{
+ return tdesc_parse_xml (xml_str, nullptr, nullptr);
+}
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 7/8]: Remove xml file references from target descriptions.
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (5 preceding siblings ...)
2018-01-24 9:30 ` [PATCH v2 6/8] Create xml from target descriptions Alan Hayward
@ 2018-01-24 9:31 ` Alan Hayward
2018-01-24 9:32 ` [PATCH v2 8/8] Remove xml files from gdbserver Alan Hayward
` (3 subsequent siblings)
10 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:31 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
With the previous patch we no longer need to know the name of the
xml file. This patch removes the references and regenerates the
C files.
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
gdb/
* arch/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/arch/tdesc.h b/gdb/arch/tdesc.h
index b2e99c5ce1f7da0e6a194ca721eea96082e1fa68..ebf0f1b0ef4d1c1decdd0d7d0fb486a1b536d71b 100644
--- a/gdb/arch/tdesc.h
+++ b/gdb/arch/tdesc.h
@@ -317,9 +317,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 3707b7e05560507328077684215218ccbad63fee..a9ec3941a19a75ef27f35dd6bbf1fea1135e88f9 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 cac3981f7dd918e051826272eb69da8e7ac5269a..32ff9e46a095e327f6af13f6b68333575440a2d8 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 8a0c35655d9a9efa19ea04053b05b0e6ba0035e8..3a98936a0c7a9715cb2d83feacde0cc6f675d8ac 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 39c7e9771a94d06f05ebb72cb108ef1e569cbaa7..064e541de983bcb9309f22e2ff80900d98f58923 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 294e86d81eecbd488adb45e29644290ce5fd313e..215c0b5ae2b7a0f3b0e6c06953257111f295f86e 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 136e3d71b41a6e5af7f115fb94937a659a48185a..1ba932d795af393ebfa2860b29acdd069177ca19 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 8f1be3a60f03243bd5decde1fcb4ba3a94773159..4ec42466e7a77785fd249ec55c8d13eca196c103 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 4ad7649915f21a0f62c9162a2fc425907c692ea4..89f1a5b3e0612ac5ad17cac7d78cd626d5ea9f3d 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 cf48960353881519789986fd7f1c46b8802d9a11..4913c9aec8cb04981bdef3747dd596589eabcad4 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 d8e391423bc2f52fe491972d20aabea4ea26f21a..146b6a57b7e1afb5ef1b21251b0613df919de05b 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 e103e43464c7337f6556fb6035d953a3881f3e98..61108c2e5816062eb8bfd1de239edd58544ef304 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 9e39ee42d9a31d68efbf838c47ae43b12a797e3f..ed540e091bcea98c3fa5548f16fd8d7aa6297280 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 570910b9cc0f6ebf88f83dd4e73e54a38cf11636..5222ff22fb8dbb6281c0dea9deadbdd490b1d4a8 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 725e76a0dd99cc79e1ac10dd62cd4bcca3cf682d..2b88dd5db994f6a29de78adb3d872b584d295022 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 9d974c3772d93b44d5ae92ccc0752d6be9c21f4d..5bdeecd8460e02ba96a27fe9248fa30184d25d84 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 7a1fbf53f35eb0106c2c3c4c32b1f7f0aa276b6a..9c7b0fe1ac15b60e66807e95bec94613a39575b7 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 2859217f45860d52c76650fa5ad17eb9611ab8c8..edb70e9de29400642c11a384dfe2e5e41edc6356 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 c268e11bea9e81563fe586299ccea4bde9c7497f..aad5d687f7a4774bfca6790c049f968846422338 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 5b0f566ee4c5687c1a8de4e08ab2828ba5242505..470ba21f4ad1d83c7a9398b62dcc974b518d0866 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 823d4c1da107786b5a983afb04a2fa88b69d3684..eb4891821296dd3a09db39667bdcbf25c3e1a954 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 df0d0e37fc2c085e8bddedff2ab00cf26ddb2c73..2efb8390ade1ad2ef1af64e30306868ab3096420 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 42a657f6827ed06c9d98275c7566d0c02890e7b3..e553323e435f4cd284158b92b4583b890df153fc 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -180,8 +180,7 @@ tdesc_get_features_xml (target_desc *tdesc)
/* See arch/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);
tdesc->features.emplace_back (new_feature);
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 68a85758f7f63e6b862afa29697adee77e776d89..c6ba9761cc803281ad1096ce3b28c3fd16a0b318 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1096,8 +1096,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
/* See arch/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);
@@ -1565,8 +1564,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] 32+ messages in thread
* [PATCH v2 8/8] Remove xml files from gdbserver
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (6 preceding siblings ...)
2018-01-24 9:31 ` [PATCH v2 7/8]: Remove xml file references " Alan Hayward
@ 2018-01-24 9:32 ` Alan Hayward
2018-01-24 10:57 ` [PATCH v2 0/8] Remove XML " Omair Javaid
` (2 subsequent siblings)
10 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 9:32 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
This patch removes the xml files from being built into gdbserver,
only for ports which use new target descriptions.
Alan.
2018-01-24 Alan Hayward <alan.hayward@arm.com>
* gdb/gdbserver/configure.srv: Don't include xml.
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] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (7 preceding siblings ...)
2018-01-24 9:32 ` [PATCH v2 8/8] Remove xml files from gdbserver Alan Hayward
@ 2018-01-24 10:57 ` Omair Javaid
2018-01-24 12:29 ` Alan Hayward
2018-01-25 13:11 ` Philipp Rudo
2018-01-29 18:18 ` Pedro Alves
10 siblings, 1 reply; 32+ messages in thread
From: Omair Javaid @ 2018-01-24 10:57 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
On 24 January 2018 at 14:26, Alan Hayward <Alan.Hayward@arm.com> wrote:
> Following review comments from Yao, this patch series is mostly a
> reordering of the code from the previous patch series.
>
> This set of patches removes the need for gdbserver to ship the xml files
> in the binary for those targets that use new style target descriptions.
>
> In exisiting code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, it
> creates an xml containing just the name of the original xml file.
> Upon receipt, GDB reads and parses a local copy of xml file.
>
> With this new patch, we add common code that allows gdbserver and gdb
> to turn a C target description structure into xml. To do this we must
> first ensure that when gdbserver parses a target description it retains
> enough information to turn it back into xml.
>
> The second patch commonises tdesc_reg, the third patch commonises
> tdesc_feature and the fifth patch commonises the tdesc type structures.
> This enables gdbserver to store all required information about a target
> description.
>
> The sixth patch adds the xml printer.
> 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
> (becuase it does not use new target descriptions), but am unable to test.
> In addition, patch four adds new test cases to unit test.
>
Hi Alan,
I am failing to apply your patch series to the latest gdb sources. Apparently,
your patch series needs a re-base over most recent gdb sources as there has
not been a commit since these patches have been posted.
Generally its a good idea to generate patches using git format-patch and
send patches using git send-email. Patches need to be based off the most
recent gdb commit that is available at the time of sending them for review.
If you use git format-patch to generate patches from your local commits it
would become very easy for a reviewer to just apply those patches using
"git apply" in their local branches.
Kindly post a patchset that works with a known version of gdb sources in
order for your patches to be tested/reviewed upstream.
Thanks!
>
> Alan.
>
>
> gdb/Makefile.in | 1 -
> gdb/arch/tdesc.c | 425 ------------------------
> gdb/arch/tdesc.h | 307 +-----------------
> 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 | 229 +++++++------
> gdb/gdbserver/tdesc.h | 58 +++-
> gdb/regformats/regdat.sh | 5 +-
> gdb/target-descriptions.c | 924 ++++++++++++++++++++++++++++++
> +++++++++--------------
> gdb/xml-tdesc.c | 9 -
> gdb/xml-tdesc.h | 5 -
> 32 files changed, 920 insertions(+), 1124 deletions(-)
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-24 10:57 ` [PATCH v2 0/8] Remove XML " Omair Javaid
@ 2018-01-24 12:29 ` Alan Hayward
2018-01-24 14:44 ` Omair Javaid
0 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 12:29 UTC (permalink / raw)
To: Omair Javaid; +Cc: gdb-patches, nd
> On 24 Jan 2018, at 10:57, Omair Javaid <omair.javaid@linaro.org> wrote:
>
> Hi Alan,
>
> I am failing to apply your patch series to the latest gdb sources. Apparently, your patch series needs a re-base over most recent gdb sources as there has not been a commit since these patches have been posted.
>
> Generally its a good idea to generate patches using git format-patch and send patches using git send-email. Patches need to be based off the most recent gdb commit that is available at the time of sending them for review. If you use git format-patch to generate patches from your local commits it would become very easy for a reviewer to just apply those patches using "git apply" in their local branches.
>
> Kindly post a patchset that works with a known version of gdb sources in order for your patches to be tested/reviewed upstream.
>
That’s odd.
My patches were based off yesterday’s head (d820e164e4baccacd8d4e7e3761ec547402ae828).
I’ve just rebased my git to the latest head, and I got no conflicts or merge warnings.
Looking at the diff from then until now, I can’t see any files that my patch touches.
Happy to repost my rebased patches in the same way, but I suspect that’s not the issue.
I’ve probably got something missing/wrong in the upper parts of of the email.
Were you having trouble with a particular patch or was it all of them?
Also, how are you getting the patch email from your email client to git?
I’m sending mails by opening my mail client, writing my comments, then copy/pasting the contents
of a diff downloaded from a local reviewboard. That diff looks identical to a git format-patch or git diff.
I’ve tried applying the first patch of the series to the latest head using both my patch file and contents
of the email, and both work for me.
I’ve just tested git send-mail, and it looks like I need to do some messing around to get that working.
I can look at getting that working.
In the meantime, I’ve pushed this branch to gdb with all my commits:
remotes/origin/users/ahayward/xml
Thanks,
Alan.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-24 12:29 ` Alan Hayward
@ 2018-01-24 14:44 ` Omair Javaid
2018-01-24 18:53 ` Alan Hayward
0 siblings, 1 reply; 32+ messages in thread
From: Omair Javaid @ 2018-01-24 14:44 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
On 24 January 2018 at 17:29, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> > On 24 Jan 2018, at 10:57, Omair Javaid <omair.javaid@linaro.org> wrote:
> >
> > Hi Alan,
> >
> > I am failing to apply your patch series to the latest gdb sources.
> Apparently, your patch series needs a re-base over most recent gdb sources
> as there has not been a commit since these patches have been posted.
> >
> > Generally its a good idea to generate patches using git format-patch and
> send patches using git send-email. Patches need to be based off the most
> recent gdb commit that is available at the time of sending them for review.
> If you use git format-patch to generate patches from your local commits it
> would become very easy for a reviewer to just apply those patches using
> "git apply" in their local branches.
> >
> > Kindly post a patchset that works with a known version of gdb sources in
> order for your patches to be tested/reviewed upstream.
> >
>
> That’s odd.
> My patches were based off yesterday’s head (d820e164e4baccacd8d4e7e3761ec
> 547402ae828).
> I’ve just rebased my git to the latest head, and I got no conflicts or
> merge warnings.
> Looking at the diff from then until now, I can’t see any files that my
> patch touches.
> Happy to repost my rebased patches in the same way, but I suspect that’s
> not the issue.
>
> I’ve probably got something missing/wrong in the upper parts of of the
> email.
>
> Were you having trouble with a particular patch or was it all of them?
> Also, how are you getting the patch email from your email client to git?
>
I downloaded eml files. Also tried with untouched orignal email text. First
2 patches are applicable using git am. Third patch and onwards fail.
With git apply or patch command even first patch fails.
>
> I’m sending mails by opening my mail client, writing my comments, then
> copy/pasting the contents
> of a diff downloaded from a local reviewboard. That diff looks identical
> to a git format-patch or git diff.
> I’ve tried applying the first patch of the series to the latest head using
> both my patch file and contents
> of the email, and both work for me.
>
I am not sure what exactly could be wrong but it could be email client
mangling patch email or wrapping text or converting tabs/spaces.
I think you are using a windows based email client which is using CR+LF as
end line character which is incompatible with unix LF.
>
> I’ve just tested git send-mail, and it looks like I need to do some
> messing around to get that working.
> I can look at getting that working.
>
> In the meantime, I’ve pushed this branch to gdb with all my commits:
> remotes/origin/users/ahayward/xml
>
> Thanks,
> Alan.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-24 14:44 ` Omair Javaid
@ 2018-01-24 18:53 ` Alan Hayward
0 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-24 18:53 UTC (permalink / raw)
To: Omair Javaid; +Cc: gdb-patches, nd
> On 24 Jan 2018, at 14:43, Omair Javaid <omair.javaid@linaro.org> wrote:
>
> Were you having trouble with a particular patch or was it all of them?
> Also, how are you getting the patch email from your email client to git?
>
> I downloaded eml files. Also tried with untouched orignal email text. First 2 patches are applicable using git am. Third patch and onwards fail.
>
> With git apply or patch command even first patch fails.
Testing this, I also get the same error with patch 3, when copying from the sent email.
>
>
> I’m sending mails by opening my mail client, writing my comments, then copy/pasting the contents
> of a diff downloaded from a local reviewboard. That diff looks identical to a git format-patch or git diff.
> I’ve tried applying the first patch of the series to the latest head using both my patch file and contents
> of the email, and both work for me.
>
> I am not sure what exactly could be wrong but it could be email client mangling patch email or wrapping text or converting tabs/spaces.
>
> I think you are using a windows based email client which is using CR+LF as end line character which is incompatible with unix LF.
>
Looking at the patch, my mail client is stripping out the FF character!
Fixing this causes the patch to apply cleanly.
I’m using Mac mail. I stopped using Outlook for Mac because of similar issues.
I will investigate getting git send-email working and/or move to a new client - but that might take time.
I hope the existing emails with the remotes/origin/users/ahayward/xml branch provide everything you
need for a review.
Alan.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (8 preceding siblings ...)
2018-01-24 10:57 ` [PATCH v2 0/8] Remove XML " Omair Javaid
@ 2018-01-25 13:11 ` Philipp Rudo
2018-01-26 22:31 ` Omair Javaid
2018-01-29 18:18 ` Pedro Alves
10 siblings, 1 reply; 32+ messages in thread
From: Philipp Rudo @ 2018-01-25 13:11 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
Hi Alan,
in general the series looks fine for me. Just some nits/suggestions.
Philipp
On Wed, 24 Jan 2018 09:26:10 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> Following review comments from Yao, this patch series is mostly a
> reordering of the code from the previous patch series.
>
> This set of patches removes the need for gdbserver to ship the xml files
> in the binary for those targets that use new style target descriptions.
>
> In exisiting code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, it
> creates an xml containing just the name of the original xml file.
> Upon receipt, GDB reads and parses a local copy of xml file.
>
> With this new patch, we add common code that allows gdbserver and gdb
> to turn a C target description structure into xml. To do this we must
> first ensure that when gdbserver parses a target description it retains
> enough information to turn it back into xml.
>
> The second patch commonises tdesc_reg, the third patch commonises
> tdesc_feature and the fifth patch commonises the tdesc type structures.
> This enables gdbserver to store all required information about a target
> description.
>
> The sixth patch adds the xml printer.
> 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
> (becuase it does not use new target descriptions), but am unable to test.
> In addition, patch four adds new test cases to unit test.
>
> Alan.
>
>
> gdb/Makefile.in | 1 -
> gdb/arch/tdesc.c | 425 ------------------------
> gdb/arch/tdesc.h | 307 +-----------------
> 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 | 229 +++++++------
> gdb/gdbserver/tdesc.h | 58 +++-
> gdb/regformats/regdat.sh | 5 +-
> gdb/target-descriptions.c | 924 +++++++++++++++++++++++++++++++++++++++--------------
> gdb/xml-tdesc.c | 9 -
> gdb/xml-tdesc.h | 5 -
> 32 files changed, 920 insertions(+), 1124 deletions(-)
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] Use tdesc_feature in gdbserver tdesc
2018-01-24 9:28 ` [PATCH v2 3/8] Use tdesc_feature in gdbserver tdesc Alan Hayward
@ 2018-01-25 13:12 ` Philipp Rudo
0 siblings, 0 replies; 32+ messages in thread
From: Philipp Rudo @ 2018-01-25 13:12 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
On Wed, 24 Jan 2018 09:28:01 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
[...]
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index cfc1a194def019ecf899e9184385be94de39d08c..8ae6cddc1896af99d86206d159fb952a0f666043 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
With your change the comment above is no longer true.
Philipp
> {
> /* 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 5dec636baa6b899490e8c700eea21d363fb79fa3..bc94d83ae58b9e79d8b71d0ce21c073f11765dd4 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,15 @@ tdesc_get_features_xml (target_desc *tdesc)
> }
> #endif
>
> -struct tdesc_type
> -{};
> -
> /* See arch/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 (name);
> + tdesc->features.emplace_back (new_feature);
> + return new_feature;
> }
>
> /* See arch/tdesc.h. */
> @@ -252,21 +239,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id)
>
> /* See arch/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 arch/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 881ef78d2e3adde409e8585deb67c96e2f0594ed..6df29521eb2319919c7e20df14ca5aebc1d25321 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -67,69 +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);
> - }
> -
> - /* Construct, if necessary, and return the GDB type implementing this
> - target type for architecture GDBARCH. */
> -
> - virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
> -};
> -
> -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)
> @@ -408,82 +345,6 @@ struct tdesc_type_with_fields : tdesc_type
> int size;
> };
>
> -/* 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 ® : 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 ®1 = registers[ix];
> - const tdesc_reg_up ®2 = 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
> @@ -1338,20 +1199,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
> tdesc_remote_register_number);
> set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p);
> }
> -
>
> -
> -/* See arch/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 arch/tdesc.h. */
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc
2018-01-24 9:27 ` [PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc Alan Hayward
@ 2018-01-25 13:12 ` Philipp Rudo
0 siblings, 0 replies; 32+ messages in thread
From: Philipp Rudo @ 2018-01-25 13:12 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
On Wed, 24 Jan 2018 09:27:21 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
>
> tdesc_create_reg is changed to create a tdesc_reg and place it in
> tdesc_feature.
>
> After registers have been parsed, init_target_desc is called. I
> have expanded this function so that it now populates the reg_defs
> directly from the tdesc_reg registers.
>
> The long term goal is to change the other files within gdbserver to
> access member functions instead of reg_defs directly. reg_defs can
> then be removed. This is out of the scope of this patch series.
>
> 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.
>
> The gdbserver makefile change is a little strange. I had to add using
> "..". I think there is a rule that needs changing, but I have no idea
> where it is. Note that this is the first file in arch which is common
> for all builds. I'm also wondering if tdesc.c/h should be in arch/ ?
> Maybe it should be moved to common?
In my opinion arch/ should only hold architecture specific code. So i would
prefer when you move tdesc.c/h to common/.
Philipp
>
> Alan.
>
> 2018-01-24 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * Makefile.in: Add arch/tdesc.c
> * arch/tdesc.c: New file.
> * arch/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 arch/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 364ea7afc66d00d0be44c4369f3ddc924de7b6aa..05ecf64451de4df3a5593d158404d526a71c0b29 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -911,6 +911,7 @@ COMMON_SFILES = \
> agent.c \
> annotate.c \
> arch-utils.c \
> + arch/tdesc.c \
> auto-load.c \
> auxv.c \
> ax-gdb.c \
> diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
> index cc11651dcaa7abe81598b69f509ef34ff0d94dbf..e957822b348182b8cae9809cd21f72974ffe05c5 100644
> --- a/gdb/arch/tdesc.h
> +++ b/gdb/arch/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,99 @@ 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) = 0;
> + virtual void visit_post (const target_desc *e) = 0;
> +
> + virtual void visit_pre (const tdesc_feature *e) = 0;
> + virtual void visit_post (const tdesc_feature *e) = 0;
> +
> + virtual void visit (const tdesc_type_builtin *e) = 0;
> + virtual void visit (const tdesc_type_vector *e) = 0;
> + virtual void visit (const tdesc_type_with_fields *e) = 0;
> +
> + virtual void visit (const tdesc_reg *e) = 0;
> +};
> +
> +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/arch/tdesc.c b/gdb/arch/tdesc.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..3d84ad8d3fa2e017f4c6f7d137a3be2b58427370
> --- /dev/null
> +++ b/gdb/arch/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 "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 3ce086d70f23df445b174c49c489ec8415d7614a..8ae124875dd08f31e8424b18281cb1454ed40edb 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -194,6 +194,7 @@ SFILES = \
> $(srcdir)/arch/arm.c \
> $(srcdir)/arch/arm-get-next-pcs.c \
> $(srcdir)/arch/arm-linux.c \
> + $(srcdir)/../arch/tdesc.c \
> $(srcdir)/common/btrace-common.c \
> $(srcdir)/common/buffer.c \
> $(srcdir)/common/cleanups.c \
> @@ -232,6 +233,7 @@ TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
>
> OBS = \
> agent.o \
> + arch/tdesc.o \
> ax.o \
> btrace-common.o \
> buffer.o \
> @@ -391,6 +393,7 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
> $(XM_CLIBS) $(LIBGNU) $(LIBIBERTY)
>
> IPA_OBJS = \
> + arch/tdesc-ipa.o \
> ax-ipa.o \
> common-utils-ipa.o \
> errors-ipa.o \
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 705b968b98af9405682507acbcfb41356da62857..cfc1a194def019ecf899e9184385be94de39d08c 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 359ab97361c07ff2cbc2c8d5ff392c6233c329ff..5dec636baa6b899490e8c700eea21d363fb79fa3 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 arch/tdesc.h. */
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 1b20a12d769718e591dea6df8183c2e9ecfac990..881ef78d2e3adde409e8585deb67c96e2f0594ed 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -38,30 +38,6 @@
> #include "completer.h"
> #include "readline/tilde.h" /* tilde_expand */
>
> -/* The interface to visit different elements of target description. */
> -
> -class tdesc_element_visitor
> -{
> -public:
> - virtual void visit_pre (const target_desc *e) = 0;
> - virtual void visit_post (const target_desc *e) = 0;
> -
> - virtual void visit_pre (const tdesc_feature *e) = 0;
> - virtual void visit_post (const tdesc_feature *e) = 0;
> -
> - virtual void visit (const tdesc_type_builtin *e) = 0;
> - virtual void visit (const tdesc_type_vector *e) = 0;
> - virtual void visit (const tdesc_type_with_fields *e) = 0;
> -
> - virtual void visit (const tdesc_reg *e) = 0;
> -};
> -
> -class tdesc_element
> -{
> -public:
> - virtual void accept (tdesc_element_visitor &v) const = 0;
> -};
> -
> /* Types. */
>
> struct property
> @@ -74,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] 32+ messages in thread
* Re: [PATCH v2 5/8] Use tdesc types in gdbserver tdesc
2018-01-24 9:29 ` [PATCH v2 5/8] Use tdesc types in gdbserver tdesc Alan Hayward
@ 2018-01-25 13:13 ` Philipp Rudo
2018-01-29 7:28 ` Omair Javaid
0 siblings, 1 reply; 32+ messages in thread
From: Philipp Rudo @ 2018-01-25 13:13 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
On Wed, 24 Jan 2018 09:29:20 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
[...]
> Stubs of the make_gdb_type functions are added to gdbserver
> (they should never get called).
See comment in patch #6.
[...]
> diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
> index 603ccd79434e1dccd17e69ef193193e673835dd0..9518571d03d394ee7cbf78b31974818201c889cd 100644
> --- a/gdb/arch/tdesc.c
> +++ b/gdb/arch/tdesc.c
[...]
> @@ -84,6 +106,36 @@ bool tdesc_feature::operator== (const tdesc_feature &other) const
> return true;
> }
>
> +/* Lookup a predefined type. */
> +
> +static struct tdesc_type *
> +tdesc_predefined_type (enum tdesc_type_kind kind)
> +{
> + for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> + if (tdesc_predefined_types[ix].kind == kind)
> + return &tdesc_predefined_types[ix];
> +
> + gdb_assert_not_reached ("bad predefined tdesc type");
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +struct tdesc_type *
> +tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> +{
> + /* First try target-defined types. */
> + for (const tdesc_type_up &type : feature->types)
> + if (type->name == id)
> + return type.get ();
> +
> + /* Next try the predefined types. */
> + for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> + if (tdesc_predefined_types[ix].name == id)
> + return &tdesc_predefined_types[ix];
> +
> + return NULL;
> +}
> +
> /* See arch/tdesc.h. */
>
> void
> @@ -96,3 +148,144 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
>
> feature->registers.emplace_back (reg);
> }
> +
> +/* See arch/tdesc.h. */
> +
> +struct tdesc_type *
> +tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> + struct tdesc_type *field_type, int count)
> +{
> + tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
> + feature->types.emplace_back (type);
> +
> + return type;
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +tdesc_type_with_fields *
> +tdesc_create_struct (struct tdesc_feature *feature, const char *name)
> +{
> + tdesc_type_with_fields *type
> + = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
> + feature->types.emplace_back (type);
> +
> + return type;
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +void
> +tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> +{
> + gdb_assert (type->kind == TDESC_TYPE_STRUCT);
> + gdb_assert (size > 0);
> + type->size = size;
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +tdesc_type_with_fields *
> +tdesc_create_union (struct tdesc_feature *feature, const char *name)
> +{
> + tdesc_type_with_fields *type
> + = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
> + feature->types.emplace_back (type);
> +
> + return type;
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +tdesc_type_with_fields *
> +tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> + int size)
> +{
> + gdb_assert (size > 0);
> +
> + tdesc_type_with_fields *type
> + = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
> + feature->types.emplace_back (type);
> +
> + return type;
> +}
> +
> +tdesc_type_with_fields *
> +tdesc_create_enum (struct tdesc_feature *feature, const char *name,
> + int size)
> +{
> + gdb_assert (size > 0);
> +
> + tdesc_type_with_fields *type
> + = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
> + feature->types.emplace_back (type);
> +
> + return type;
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +void
> +tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> + struct tdesc_type *field_type)
> +{
> + gdb_assert (type->kind == TDESC_TYPE_UNION
> + || type->kind == TDESC_TYPE_STRUCT);
> +
> + /* Initialize start and end so we know this is not a bit-field
> + when we print-c-tdesc. */
> + type->fields.emplace_back (field_name, field_type, -1, -1);
> +}
> +
> +void
> +tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
> + int start, int end, struct tdesc_type *field_type)
> +{
> + gdb_assert (type->kind == TDESC_TYPE_STRUCT
> + || type->kind == TDESC_TYPE_FLAGS);
> + gdb_assert (start >= 0 && end >= start);
> +
> + type->fields.emplace_back (field_name, field_type, start, end);
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +void
> +tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
> + int start, int end)
> +{
> + struct tdesc_type *field_type;
> +
> + gdb_assert (start >= 0 && end >= start);
> +
> + if (type->size > 4)
> + field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
> + else
> + field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
> +
> + tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
> +}
> +
> +/* See arch/tdesc.h. */
> +
> +void
> +tdesc_add_flag (tdesc_type_with_fields *type, int start,
> + const char *flag_name)
> +{
> + gdb_assert (type->kind == TDESC_TYPE_FLAGS
> + || type->kind == TDESC_TYPE_STRUCT);
> +
> + type->fields.emplace_back (flag_name,
> + tdesc_predefined_type (TDESC_TYPE_BOOL),
> + start, start);
> +}
> +
> +void
> +tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
> + const char *name)
> +{
> + gdb_assert (type->kind == TDESC_TYPE_ENUM);
> + type->fields.emplace_back (name,
> + tdesc_predefined_type (TDESC_TYPE_INT32),
> + value, -1);
> +}
> \ No newline at end of file
With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
so many functions shared many of them could be made class methods. However
that exceeds the scope of this patch set, but should be kept in mind for the
future.
Philipp
> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index bc94d83ae58b9e79d8b71d0ce21c073f11765dd4..f0bd266a54601484df74ee1c5f8dce6fe04661c4 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -197,70 +197,17 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
> return new_feature;
> }
>
> -/* See arch/tdesc.h. */
> -
> -tdesc_type_with_fields *
> -tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> - int size)
> -{
> - return NULL;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_add_flag (tdesc_type_with_fields *type, int start,
> - const char *flag_name)
> -{}
> -
> -/* See arch/tdesc.h. */
> -
> -struct tdesc_type *
> -tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> -{
> - return NULL;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -tdesc_type_with_fields *
> -tdesc_create_union (struct tdesc_feature *feature, const char *id)
> +type *tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> {
> - return NULL;
> + error (_("Cannot create gdbtypes."));
> }
>
> -/* See arch/tdesc.h. */
> -
> -tdesc_type_with_fields *
> -tdesc_create_struct (struct tdesc_feature *feature, const char *id)
> -{
> - return NULL;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -struct tdesc_type *
> -tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> - struct tdesc_type *field_type, int count)
> +type *tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
> {
> - return NULL;
> + error (_("Cannot create gdbtypes."));
> }
>
> -void
> -tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
> - int start, int end)
> -{}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> - struct tdesc_type *field_type)
> -{}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> +type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
> {
> + error (_("Cannot create gdbtypes."));
> }
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 8ead19efa24b3cb154c895c484e791b92ff61387..4f11120dce4092f15de99680a7c4868c4a2f4493 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -50,80 +50,6 @@ struct property
> std::string value;
> };
>
> -/* A named type from a target description. */
> -
> -struct tdesc_type_field
> -{
> - tdesc_type_field (const std::string &name_, tdesc_type *type_,
> - int start_, int end_)
> - : name (name_), type (type_), start (start_), end (end_)
> - {}
> -
> - std::string name;
> - struct tdesc_type *type;
> - /* For non-enum-values, either both are -1 (non-bitfield), or both are
> - not -1 (bitfield). For enum values, start is the value (which could be
> - -1), end is -1. */
> - int start, end;
> -};
> -
> -struct tdesc_type_builtin : tdesc_type
> -{
> - tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
> - : tdesc_type (name, kind)
> - {}
> -
> - void accept (tdesc_element_visitor &v) const override
> - {
> - v.visit (this);
> - }
> -
> - type *make_gdb_type (struct gdbarch *gdbarch) const override;
> -};
> -
> -/* tdesc_type for vector types. */
> -
> -struct tdesc_type_vector : tdesc_type
> -{
> - tdesc_type_vector (const std::string &name, tdesc_type *element_type_, int count_)
> - : tdesc_type (name, TDESC_TYPE_VECTOR),
> - element_type (element_type_), count (count_)
> - {}
> -
> - void accept (tdesc_element_visitor &v) const override
> - {
> - v.visit (this);
> - }
> -
> - type *make_gdb_type (struct gdbarch *gdbarch) const override;
> -
> - struct tdesc_type *element_type;
> - int count;
> -};
> -
> -/* tdesc_type for struct, union, flags, and enum types. */
> -
> -struct tdesc_type_with_fields : tdesc_type
> -{
> - tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
> - int size_ = 0)
> - : tdesc_type (name, kind), size (size_)
> - {}
> -
> - void accept (tdesc_element_visitor &v) const override
> - {
> - v.visit (this);
> - }
> -
> - type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
> - type *make_gdb_type_union (struct gdbarch *gdbarch) const;
> - type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
> - type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
> - type *make_gdb_type (struct gdbarch *gdbarch) const override;
> -
> - std::vector<tdesc_type_field> fields;
> - int size;
> -};
>
> type *
> tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> @@ -733,58 +659,6 @@ tdesc_feature_name (const struct tdesc_feature *feature)
> return feature->name.c_str ();
> }
>
> -/* Predefined types. */
> -static tdesc_type_builtin tdesc_predefined_types[] =
> -{
> - { "bool", TDESC_TYPE_BOOL },
> - { "int8", TDESC_TYPE_INT8 },
> - { "int16", TDESC_TYPE_INT16 },
> - { "int32", TDESC_TYPE_INT32 },
> - { "int64", TDESC_TYPE_INT64 },
> - { "int128", TDESC_TYPE_INT128 },
> - { "uint8", TDESC_TYPE_UINT8 },
> - { "uint16", TDESC_TYPE_UINT16 },
> - { "uint32", TDESC_TYPE_UINT32 },
> - { "uint64", TDESC_TYPE_UINT64 },
> - { "uint128", TDESC_TYPE_UINT128 },
> - { "code_ptr", TDESC_TYPE_CODE_PTR },
> - { "data_ptr", TDESC_TYPE_DATA_PTR },
> - { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
> - { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
> - { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
> - { "i387_ext", TDESC_TYPE_I387_EXT }
> -};
> -
> -/* Lookup a predefined type. */
> -
> -static struct tdesc_type *
> -tdesc_predefined_type (enum tdesc_type_kind kind)
> -{
> - for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> - if (tdesc_predefined_types[ix].kind == kind)
> - return &tdesc_predefined_types[ix];
> -
> - gdb_assert_not_reached ("bad predefined tdesc type");
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -struct tdesc_type *
> -tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> -{
> - /* First try target-defined types. */
> - for (const tdesc_type_up &type : feature->types)
> - if (type->name == id)
> - return type.get ();
> -
> - /* Next try the predefined types. */
> - for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> - if (tdesc_predefined_types[ix].name == id)
> - return &tdesc_predefined_types[ix];
> -
> - return NULL;
> -}
> -
> /* Lookup type associated with ID. */
>
> struct type *
> @@ -1219,147 +1093,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
>
> /* See arch/tdesc.h. */
>
> -struct tdesc_type *
> -tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> - struct tdesc_type *field_type, int count)
> -{
> - tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
> - feature->types.emplace_back (type);
> -
> - return type;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -tdesc_type_with_fields *
> -tdesc_create_struct (struct tdesc_feature *feature, const char *name)
> -{
> - tdesc_type_with_fields *type
> - = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
> - feature->types.emplace_back (type);
> -
> - return type;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> -{
> - gdb_assert (type->kind == TDESC_TYPE_STRUCT);
> - gdb_assert (size > 0);
> - type->size = size;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -tdesc_type_with_fields *
> -tdesc_create_union (struct tdesc_feature *feature, const char *name)
> -{
> - tdesc_type_with_fields *type
> - = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
> - feature->types.emplace_back (type);
> -
> - return type;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -tdesc_type_with_fields *
> -tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> - int size)
> -{
> - gdb_assert (size > 0);
> -
> - tdesc_type_with_fields *type
> - = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
> - feature->types.emplace_back (type);
> -
> - return type;
> -}
> -
> -tdesc_type_with_fields *
> -tdesc_create_enum (struct tdesc_feature *feature, const char *name,
> - int size)
> -{
> - gdb_assert (size > 0);
> -
> - tdesc_type_with_fields *type
> - = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
> - feature->types.emplace_back (type);
> -
> - return type;
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> - struct tdesc_type *field_type)
> -{
> - gdb_assert (type->kind == TDESC_TYPE_UNION
> - || type->kind == TDESC_TYPE_STRUCT);
> -
> - /* Initialize start and end so we know this is not a bit-field
> - when we print-c-tdesc. */
> - type->fields.emplace_back (field_name, field_type, -1, -1);
> -}
> -
> -void
> -tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
> - int start, int end, struct tdesc_type *field_type)
> -{
> - gdb_assert (type->kind == TDESC_TYPE_STRUCT
> - || type->kind == TDESC_TYPE_FLAGS);
> - gdb_assert (start >= 0 && end >= start);
> -
> - type->fields.emplace_back (field_name, field_type, start, end);
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
> - int start, int end)
> -{
> - struct tdesc_type *field_type;
> -
> - gdb_assert (start >= 0 && end >= start);
> -
> - if (type->size > 4)
> - field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
> - else
> - field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
> -
> - tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
> -}
> -
> -/* See arch/tdesc.h. */
> -
> -void
> -tdesc_add_flag (tdesc_type_with_fields *type, int start,
> - const char *flag_name)
> -{
> - gdb_assert (type->kind == TDESC_TYPE_FLAGS
> - || type->kind == TDESC_TYPE_STRUCT);
> -
> - type->fields.emplace_back (flag_name,
> - tdesc_predefined_type (TDESC_TYPE_BOOL),
> - start, start);
> -}
> -
> -void
> -tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
> - const char *name)
> -{
> - gdb_assert (type->kind == TDESC_TYPE_ENUM);
> - type->fields.emplace_back (name,
> - tdesc_predefined_type (TDESC_TYPE_INT32),
> - value, -1);
> -}
> -
> -/* See arch/tdesc.h. */
> -
> struct tdesc_feature *
> tdesc_create_feature (struct target_desc *tdesc, const char *name,
> const char *xml)
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 6/8] Create xml from target descriptions
2018-01-24 9:30 ` [PATCH v2 6/8] Create xml from target descriptions Alan Hayward
@ 2018-01-25 13:14 ` Philipp Rudo
2018-01-25 15:45 ` Yao Qi
0 siblings, 1 reply; 32+ messages in thread
From: Philipp Rudo @ 2018-01-25 13:14 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
with different implementations in target-descriptions.c and gdbserver/tdesc.c
while all other visit_* implementations are in arch/tdesc.c. I would prefere
visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
even declare the method for GDBserver, i.e.
struct tdesc_type
{
[...]
#ifndef GDBSERVER
virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
#endif
};
The problem i see with implementing stubs calling error is that you cannot find
out you made a mistake until you call the function during run-time. This gives
room to nasty bugs which could easily be prevented when there is a compile bug.
Philipp
On Wed, 24 Jan 2018 09:29:50 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> This patch adds a print_xml_feature visitor class which turns a C
> target description into xml.
>
> An accept function is added to gdbserver tdesc to allow it to use
> vistor classes.
>
> Tests are added to maintenance_check_xml_descriptions which takes
> each pair of tested descriptions, turns them both into xml, then back
> again, and confirms the descriptions still match.
>
> Alan.
>
> 2018-01-24 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * arch/tdesc.c (print_xml_feature::visit_post): Add xml parsing.
> (print_xml_feature::visit_pre): Likewise.
> (print_xml_feature::visit_post): Likewise.
> (print_xml_feature::visit): Likewise.
> (print_xml_feature::visit): Likewise.
> (print_xml_feature::visit): Likewise.
> (print_xml_feature::visit): Likewise.
> * arch/tdesc.h (print_xml_feature): Add new class.
> * regformats/regdat.sh: obtain xml.
> * target-descriptions.c (struct target_desc): Add xmltarget.
> (print_xml_feature::visit_pre): Add xml vistor.
> (tdesc_get_features_xml): Add function to get xml.
> (maintenance_check_xml_descriptions): Test xml generation.
> * xml-tdesc.c (target_read_description_xml_string): Add function.
> * xml-tdesc.h (target_read_description_xml_string): Add declaration.
>
> gdbserver/
> * tdesc.c (void target_desc::accept): Fill in function.
> (tdesc_get_features_xml): Remove old xml creation.
> (print_xml_feature::visit_pre): Add xml vistor.
>
>
> diff --git a/gdb/arch/tdesc.h b/gdb/arch/tdesc.h
> index 9a5bf6f11b670e04e2b51f8334bc0adaf0b43962..b2e99c5ce1f7da0e6a194ca721eea96082e1fa68 100644
> --- a/gdb/arch/tdesc.h
> +++ b/gdb/arch/tdesc.h
> @@ -195,7 +195,7 @@ struct tdesc_type_builtin : tdesc_type
> : tdesc_type (name, kind)
> {}
>
> - void accept (tdesc_element_visitor &v) const override;
> + void accept (tdesc_element_visitor &v) const override
> {
> v.visit (this);
> }
> @@ -366,4 +366,33 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
> int regnum, int save_restore, const char *group,
> int bitsize, const char *type);
>
> +/* Return the tdesc in string XML format. */
> +
> +const char *tdesc_get_features_xml (target_desc *tdesc);
> +
> +/* Print target description as xml. */
> +
> +class print_xml_feature : public tdesc_element_visitor
> +{
> +public:
> + print_xml_feature (std::string *buffer_)
> + : m_buffer (buffer_)
> + {}
> +
> + ~print_xml_feature ()
> + {}
> +
> + void visit_pre (const target_desc *e) override;
> + void visit_post (const target_desc *e) override;
> + void visit_pre (const tdesc_feature *e) override;
> + void visit_post (const tdesc_feature *e) override;
> + void visit (const tdesc_type_builtin *type) override;
> + void visit (const tdesc_type_vector *type) override;
> + void visit (const tdesc_type_with_fields *type) override;
> + void visit (const tdesc_reg *reg) override;
> +
> +private:
> + std::string *m_buffer;
> +};
> +
> #endif /* ARCH_TDESC_H */
> diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
> index 9518571d03d394ee7cbf78b31974818201c889cd..388c6af1b93d40cf7cb5fb97c55912b91601814b 100644
> --- a/gdb/arch/tdesc.c
> +++ b/gdb/arch/tdesc.c
> @@ -288,4 +288,138 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
> type->fields.emplace_back (name,
> tdesc_predefined_type (TDESC_TYPE_INT32),
> value, -1);
> -}
> \ No newline at end of file
> +}
> +
> +void print_xml_feature::visit_post (const target_desc *e)
> +{
> + *m_buffer += "</target>\n";
> +}
> +
> +void print_xml_feature::visit_pre (const tdesc_feature *e)
> +{
> + *m_buffer += "<feature name=\"";
> + *m_buffer += e->name;
> + *m_buffer += "\">\n";
> +}
> +
> +void print_xml_feature::visit_post (const tdesc_feature *e)
> +{
> + *m_buffer += "</feature>\n";
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_builtin *type)
> +{
> + error (_("xml output is not supported type \"%s\"."), type->name.c_str ());
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_vector *type)
> +{
> + *m_buffer += "<vector id=\"";
> + *m_buffer += type->name;
> + *m_buffer += "\" type=\"";
> + *m_buffer += type->element_type->name;
> + *m_buffer += "\" count=\"";
> + *m_buffer += std::to_string (type->count);
> + *m_buffer += "\"/>\n";
> +}
> +
> +void print_xml_feature::visit (const tdesc_type_with_fields *type)
> +{
> + struct tdesc_type_field *f;
> + const static char *types[] = { "struct", "union", "flags", "enum" };
> +
> + gdb_assert (type->kind >= TDESC_TYPE_STRUCT && type->kind <= TDESC_TYPE_ENUM);
> + *m_buffer += "<";
> + *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
> +
> + switch (type->kind)
> + {
> + case TDESC_TYPE_STRUCT:
> + case TDESC_TYPE_FLAGS:
> + *m_buffer += " id=\"";
> + *m_buffer += type->name;
> + if (type->size > 0)
> + {
> + *m_buffer += "\" size=\"";
> + *m_buffer += std::to_string (type->size);
> + }
> + *m_buffer += "\">\n";
> +
> + for (const tdesc_type_field &f : type->fields)
> + {
> + *m_buffer += " <field name=\"";
> + *m_buffer += f.name;
> + if (f.start == -1)
> + {
> + *m_buffer += "\" type=\"";
> + *m_buffer += f.type->name;
> + }
> + else
> + {
> + *m_buffer += "\" start=\"";
> + *m_buffer += std::to_string (f.start);
> + *m_buffer += "\" end=\"";
> + *m_buffer += std::to_string (f.end);
> + }
> +
> + *m_buffer += "\"/>\n";
> + }
> + break;
> +
> + case TDESC_TYPE_ENUM:
> + *m_buffer += " id=\"";
> + *m_buffer += type->name;
> + *m_buffer += "\">\n";
> +
> + for (const tdesc_type_field &f : type->fields)
> + {
> + *m_buffer += " <field name=\"";
> + *m_buffer += f.name;
> + *m_buffer += "\" start=\"";
> + *m_buffer += std::to_string (f.start);
> + *m_buffer += "\"/>\n";
> + }
> + break;
> +
> + case TDESC_TYPE_UNION:
> + *m_buffer += " id=\"";
> + *m_buffer += type->name;
> + *m_buffer += "\">\n";
> +
> + for (const tdesc_type_field &f : type->fields)
> + {
> + *m_buffer += " <field name=\"";
> + *m_buffer += f.name;
> + *m_buffer += "\" type=\"";
> + *m_buffer += f.type->name;
> + *m_buffer += "\"/>\n";
> + }
> + break;
> +
> + default:
> + error (_("xml output is not supported type \"%s\"."),
> + type->name.c_str ());
> + }
> +
> + *m_buffer += "</";
> + *m_buffer += types[type->kind - TDESC_TYPE_STRUCT];
> + *m_buffer += ">\n";
> +}
> +
> +void print_xml_feature::visit (const tdesc_reg *reg)
> +{
> + *m_buffer += "<reg name=\"";
> + *m_buffer += reg->name;
> + *m_buffer += "\" bitsize=\"";
> + *m_buffer += std::to_string (reg->bitsize);
> + *m_buffer += "\" type=\"";
> + *m_buffer += reg->type;
> + *m_buffer += "\" regnum=\"";
> + *m_buffer += std::to_string (reg->target_regnum);
> + if (reg->group.length () > 0)
> + {
> + *m_buffer += "\" group=\"";
> + *m_buffer += reg->group;
> + }
> + *m_buffer += "\"/>\n";
> +}
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 8ae6cddc1896af99d86206d159fb952a0f666043..6a86f66cf2aaf9aa2b20203cad1d272d70026822 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -27,7 +27,7 @@
> /* A target description. Inherit from tdesc_feature so that target_desc
> can be used as tdesc_feature. */
>
> -struct target_desc
> +struct target_desc : tdesc_element
> {
> /* A vector of elements of register definitions that
> describe the inferior's register set. */
> @@ -72,7 +72,10 @@ public:
> {
> return !(*this == other);
> }
> +
> #endif
> +
> + void accept (tdesc_element_visitor &v) const;
> };
>
> /* Copy target description SRC to DEST. */
> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index f0bd266a54601484df74ee1c5f8dce6fe04661c4..42a657f6827ed06c9d98275c7566d0c02890e7b3 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -59,6 +59,18 @@ bool target_desc::operator== (const target_desc &other) const
>
> #endif
>
> +void target_desc::accept (tdesc_element_visitor &v) const
> +{
> +#ifndef IN_PROCESS_AGENT
> + v.visit_pre (this);
> +
> + for (const tdesc_feature_up &feature : features)
> + feature->accept (v);
> +
> + v.visit_post (this);
> +#endif
> +}
> +
> void
> init_target_desc (struct target_desc *tdesc)
> {
> @@ -155,30 +167,9 @@ tdesc_get_features_xml (target_desc *tdesc)
>
> if (tdesc->xmltarget == NULL)
> {
> - std::string buffer ("@<?xml version=\"1.0\"?>");
> -
> - buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
> - buffer += "<target>";
> - buffer += "<architecture>";
> - buffer += tdesc->arch;
> - buffer += "</architecture>";
> -
> - if (tdesc->osabi != nullptr)
> - {
> - buffer += "<osabi>";
> - buffer += tdesc->osabi;
> - buffer += "</osabi>";
> - }
> -
> - for (const tdesc_feature_up &feature : tdesc->features)
> - {
> - buffer += "<xi:include href=\"";
> - buffer += feature->name;
> - buffer += "\"/>";
> - }
> -
> - buffer += "</target>";
> -
> + std::string buffer ("");
> + print_xml_feature v (&buffer);
> + tdesc->accept (v);
> tdesc->xmltarget = xstrdup (buffer.c_str ());
> }
>
> @@ -211,3 +202,22 @@ type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch) const
> {
> error (_("Cannot create gdbtypes."));
> }
> +
> +void print_xml_feature::visit_pre (const target_desc *e)
> +{
> +#ifndef IN_PROCESS_AGENT
> + *m_buffer += "@<?xml version=\"1.0\"?>\n";
> + *m_buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n";
> + *m_buffer += "<target>\n";
> + *m_buffer += "<architecture>";
> + *m_buffer += e->arch;
> + *m_buffer += "</architecture>\n";
> +
> + if (e->osabi != nullptr)
> + {
> + *m_buffer += "<osabi>";
> + *m_buffer += e->osabi;
> + *m_buffer += "</osabi>\n";
> + }
> +#endif
> + }
> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -180,7 +180,6 @@ echo
> cat <<EOF
> #ifndef IN_PROCESS_AGENT
> result->expedite_regs = expedite_regs_${name};
> - result->xmltarget = xmltarget_${name};
> #endif
>
> init_target_desc (result);
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 4f11120dce4092f15de99680a7c4868c4a2f4493..68a85758f7f63e6b862afa29697adee77e776d89 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -316,6 +316,8 @@ struct target_desc : tdesc_element
> /* The features associated with this target. */
> std::vector<tdesc_feature_up> features;
>
> + char *xmltarget = nullptr;
> +
> void accept (tdesc_element_visitor &v) const override
> {
> v.visit_pre (this);
> @@ -1633,6 +1635,39 @@ private:
> int m_next_regnum = 0;
> };
>
> +void print_xml_feature::visit_pre (const target_desc *e)
> +{
> + *m_buffer += "<?xml version=\"1.0\"?>\n";
> + *m_buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n";
> + *m_buffer += "<target>\n";
> + *m_buffer += "<architecture>";
> + *m_buffer += tdesc_architecture (e)->printable_name;
> + *m_buffer += "</architecture>\n";
> +
> + if (e->osabi != GDB_OSABI_UNKNOWN)
> + {
> + *m_buffer += "<osabi>";
> + *m_buffer += gdbarch_osabi_name (tdesc_osabi (e));
> + *m_buffer += "</osabi>\n";
> + }
> +}
> +
> +/* Return a string which is of XML format, including XML target
> + description to be sent to GDB. */
> +
> +const char *
> +tdesc_get_features_xml (target_desc *tdesc)
> +{
> + if (tdesc->xmltarget == nullptr)
> + {
> + std::string buffer ("");
> + print_xml_feature v (&buffer);
> + tdesc->accept (v);
> + tdesc->xmltarget = xstrdup (buffer.c_str ());
> + }
> + return tdesc->xmltarget;
> +}
> +
> static void
> maint_print_c_tdesc_cmd (const char *args, int from_tty)
> {
> @@ -1726,7 +1761,34 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
> = file_read_description_xml (tdesc_xml.data ());
>
> if (tdesc == NULL || *tdesc != *e.second)
> - failed++;
> + {
> + printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
> + failed++;
> + continue;
> + }
> +
> + /* Convert both descriptions to xml, and then back again. Confirm all
> + descriptions are identical. */
> +
> + const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
> + const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
> + const target_desc *t_trans = target_read_description_xml_string (xml);
> + const target_desc *t_trans2 = target_read_description_xml_string (xml2);
> +
> + if (t_trans == NULL || t_trans2 == NULL)
> + {
> + printf_filtered (
> + _("Could not convert descriptions for %s back to xml (%p %p)\n"),
> + e.first, t_trans, t_trans2);
> + failed++;
> + }
> + else if (*tdesc != *t_trans || *tdesc != *t_trans2)
> + {
> + printf_filtered
> + (_("Translated descriptions for %s do not match (%d %d)\n"),
> + e.first, *tdesc == *t_trans, *tdesc == *t_trans2);
> + failed++;
> + }
> }
> printf_filtered (_("Tested %lu XML files, %d failed\n"),
> (long) selftests::xml_tdesc.size (), failed);
> diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
> index 8f0679707ad0f1e04d803f955f7fb98b4cc0c8c8..fee60e86dd10e1543b935c3cebce505d4dc828e2 100644
> --- a/gdb/xml-tdesc.h
> +++ b/gdb/xml-tdesc.h
> @@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *);
> otherwise. */
> gdb::optional<std::string> target_fetch_description_xml (target_ops *ops);
>
> +/* Take an xml string, parse it, and return the parsed description. Does not
> + handle a string containing includes. */
> +
> +const struct target_desc *target_read_description_xml_string (const char *);
> +
> #endif /* XML_TDESC_H */
>
> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 9190d5f3c64ffdc6d7987d651527f597d695c5a6..f793f07c96847a3d61188fff1c5d63952cc37565 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -752,3 +752,12 @@ target_fetch_description_xml (struct target_ops *ops)
> return output;
> #endif
> }
> +
> +/* Take an xml string, parse it, and return the parsed description. Does not
> + handle a string containing includes. */
> +
> +const struct target_desc *
> +target_read_description_xml_string (const char *xml_str)
> +{
> + return tdesc_parse_xml (xml_str, nullptr, nullptr);
> +}
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 6/8] Create xml from target descriptions
2018-01-25 13:14 ` Philipp Rudo
@ 2018-01-25 15:45 ` Yao Qi
2018-01-25 16:13 ` Alan Hayward
0 siblings, 1 reply; 32+ messages in thread
From: Yao Qi @ 2018-01-25 15:45 UTC (permalink / raw)
To: Philipp Rudo; +Cc: Alan Hayward, gdb-patches, nd
Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
> even declare the method for GDBserver, i.e.
>
> struct tdesc_type
> {
>
> [...]
>
> #ifndef GDBSERVER
> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
> #endif
> };
>
> The problem i see with implementing stubs calling error is that you cannot find
> out you made a mistake until you call the function during run-time. This gives
> room to nasty bugs which could easily be prevented when there is a compile bug.
make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
possible. You can create an sub-class of tdesc_element_visitor in gdb
side, and create the gdb type by visiting these elements, like this,
class gdb_type_creator : public tdesc_element_visitor
{
public:
gdb_type_createor (struct gdbarch *gdbarch)
: m_gdbarch (gdbarch)
{}
void visit (const tdesc_type_builtin *e) override
{
switch (e->kind)
{
case TDESC_TYPE_BOOL:
m_type = builtin_type (m_gdbarch)->builtin_bool;
break;
....
};
}
void visit (const tdesc_type_vector *e) override
{
// do what tdesc_type_vector::make_gdb_type does.
}
void visit (const tdesc_type_with_fields *e) override
{
// likewise.
}
private:
// input
struct * const m_gdbarch;
// output
type *m_type;
};
gdb_type_creator gdb_type (gdbarch);
tdesc_type->accept (gdb_type);
gdb_type.m_type is the type we need.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 6/8] Create xml from target descriptions
2018-01-25 15:45 ` Yao Qi
@ 2018-01-25 16:13 ` Alan Hayward
2018-01-25 16:56 ` Philipp Rudo
0 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-25 16:13 UTC (permalink / raw)
To: Yao Qi; +Cc: Philipp Rudo, gdb-patches, nd
> On 25 Jan 2018, at 13:14, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
>
>
> i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
> with different implementations in target-descriptions.c and gdbserver/tdesc.c
> while all other visit_* implementations are in arch/tdesc.c. I would prefere
> visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
> some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
Thanks for the review!
A common print_xml_feature would require 4 ifdefs, which would be too messy.
I’ll take a look see if I can communise the tdesc_architecture and tdesc_osabi
interfaces - it should be fairly simple to do.
Once done, I should be able to add a common print_xml_feature without any
ifdefs. Hopefully.
> On 25 Jan 2018, at 15:44, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
>
>> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
>> even declare the method for GDBserver, i.e.
>>
>> struct tdesc_type
>> {
>>
>> [...]
>>
>> #ifndef GDBSERVER
>> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
>> #endif
>> };
>>
>> The problem i see with implementing stubs calling error is that you cannot find
>> out you made a mistake until you call the function during run-time. This gives
>> room to nasty bugs which could easily be prevented when there is a compile bug.
>
>
> make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
> possible. You can create an sub-class of tdesc_element_visitor in gdb
> side, and create the gdb type by visiting these elements, like this,
>
> class gdb_type_creator : public tdesc_element_visitor
….
Nice idea. I can add an extra patch into the series to do this.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 6/8] Create xml from target descriptions
2018-01-25 16:13 ` Alan Hayward
@ 2018-01-25 16:56 ` Philipp Rudo
0 siblings, 0 replies; 32+ messages in thread
From: Philipp Rudo @ 2018-01-25 16:56 UTC (permalink / raw)
To: Alan Hayward; +Cc: Yao Qi, gdb-patches, nd
On Thu, 25 Jan 2018 16:13:16 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> > On 25 Jan 2018, at 13:14, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> >
> >
> > i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
> > with different implementations in target-descriptions.c and gdbserver/tdesc.c
> > while all other visit_* implementations are in arch/tdesc.c. I would prefere
> > visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
> > some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
>
> Thanks for the review!
>
> A common print_xml_feature would require 4 ifdefs, which would be too messy.
> I’ll take a look see if I can communise the tdesc_architecture and tdesc_osabi
> interfaces - it should be fairly simple to do.
> Once done, I should be able to add a common print_xml_feature without any
> ifdefs. Hopefully.
>
Having a common interface would be even better :)
>
> > On 25 Jan 2018, at 15:44, Yao Qi <qiyaoltc@gmail.com> wrote:
> >
> > Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> >
> >> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
> >> even declare the method for GDBserver, i.e.
> >>
> >> struct tdesc_type
> >> {
> >>
> >> [...]
> >>
> >> #ifndef GDBSERVER
> >> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
> >> #endif
> >> };
> >>
> >> The problem i see with implementing stubs calling error is that you cannot find
> >> out you made a mistake until you call the function during run-time. This gives
> >> room to nasty bugs which could easily be prevented when there is a compile bug.
> >
> >
> > make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
> > possible. You can create an sub-class of tdesc_element_visitor in gdb
> > side, and create the gdb type by visiting these elements, like this,
> >
> > class gdb_type_creator : public tdesc_element_visitor
> ….
>
> Nice idea. I can add an extra patch into the series to do this.
Yeah, Yao's solution is better than mine.
Oh i forgot to tell you, great series.
Philipp
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-25 13:11 ` Philipp Rudo
@ 2018-01-26 22:31 ` Omair Javaid
2018-01-29 16:28 ` Yao Qi
0 siblings, 1 reply; 32+ messages in thread
From: Omair Javaid @ 2018-01-26 22:31 UTC (permalink / raw)
To: Philipp Rudo; +Cc: Alan Hayward, gdb-patches, nd
Hi Alan,
I just ran gdb testsuite with native-gdbserver configuration on your
patches and found a couple of regressions. At least first three seems to be
coming out of your patch series as they seem persistent over multiple runs
of testsuite.
1c1
< Test Run By omair on Sat Jan 27 02:51:24 2018
---
> Test Run By omair on Sat Jan 27 01:58:06 2018
1257,1258c1257,1258
< FAIL: gdb.arch/i386-mpx.exp: bndcfgu formating
< FAIL: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
---
> PASS: gdb.arch/i386-mpx.exp: bndcfgu formating
> PASS: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
6706c6706
< FAIL: gdb.base/gcore.exp: corefile restored all registers
---
> PASS: gdb.base/gcore.exp: corefile restored all registers
50790c50790
< PASS: gdb.threads/thread-unwindonsignal.exp: continue until exit
---
> FAIL: gdb.threads/thread-unwindonsignal.exp: continue until exit (timeout)
55145,55146c55145,55146
< # of expected passes 51385
< # of unexpected failures 1249
---
> # of expected passes 51387
> # of unexpected failures 1247
On 25 January 2018 at 18:11, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> Hi Alan,
>
> in general the series looks fine for me. Just some nits/suggestions.
>
> Philipp
>
> On Wed, 24 Jan 2018 09:26:10 +0000
> Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> > Following review comments from Yao, this patch series is mostly a
> > reordering of the code from the previous patch series.
> >
> > This set of patches removes the need for gdbserver to ship the xml files
> > in the binary for those targets that use new style target descriptions.
> >
> > In exisiting code, gdbserver uses C code auto generated from xml files to
> > create target descriptions. When sending an xml description to GDB, it
> > creates an xml containing just the name of the original xml file.
> > Upon receipt, GDB reads and parses a local copy of xml file.
> >
> > With this new patch, we add common code that allows gdbserver and gdb
> > to turn a C target description structure into xml. To do this we must
> > first ensure that when gdbserver parses a target description it retains
> > enough information to turn it back into xml.
> >
> > The second patch commonises tdesc_reg, the third patch commonises
> > tdesc_feature and the fifth patch commonises the tdesc type structures.
> > This enables gdbserver to store all required information about a target
> > description.
> >
> > The sixth patch adds the xml printer.
> > 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
> > (becuase it does not use new target descriptions), but am unable to test.
> > In addition, patch four adds new test cases to unit test.
> >
> > Alan.
> >
> >
> > gdb/Makefile.in | 1 -
> > gdb/arch/tdesc.c | 425 ------------------------
> > gdb/arch/tdesc.h | 307 +-----------------
> > 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 | 229 +++++++------
> > gdb/gdbserver/tdesc.h | 58 +++-
> > gdb/regformats/regdat.sh | 5 +-
> > gdb/target-descriptions.c | 924 ++++++++++++++++++++++++++++++
> +++++++++--------------
> > gdb/xml-tdesc.c | 9 -
> > gdb/xml-tdesc.h | 5 -
> > 32 files changed, 920 insertions(+), 1124 deletions(-)
> >
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/8] Use tdesc types in gdbserver tdesc
2018-01-25 13:13 ` Philipp Rudo
@ 2018-01-29 7:28 ` Omair Javaid
2018-01-29 11:01 ` Alan Hayward
0 siblings, 1 reply; 32+ messages in thread
From: Omair Javaid @ 2018-01-29 7:28 UTC (permalink / raw)
To: Philipp Rudo; +Cc: Alan Hayward, gdb-patches, nd
On 25 January 2018 at 18:12, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
> On Wed, 24 Jan 2018 09:29:20 +0000
> Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> [...]
>
> > Stubs of the make_gdb_type functions are added to gdbserver
> > (they should never get called).
>
> See comment in patch #6.
>
> [...]
>
> > diff --git a/gdb/arch/tdesc.c b/gdb/arch/tdesc.c
> > index 603ccd79434e1dccd17e69ef193193e673835dd0..
> 9518571d03d394ee7cbf78b31974818201c889cd 100644
> > --- a/gdb/arch/tdesc.c
> > +++ b/gdb/arch/tdesc.c
>
> [...]
>
> > @@ -84,6 +106,36 @@ bool tdesc_feature::operator== (const tdesc_feature
> &other) const
> > return true;
> > }
> >
> > +/* Lookup a predefined type. */
> > +
> > +static struct tdesc_type *
> > +tdesc_predefined_type (enum tdesc_type_kind kind)
> > +{
> > + for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> > + if (tdesc_predefined_types[ix].kind == kind)
> > + return &tdesc_predefined_types[ix];
> > +
> > + gdb_assert_not_reached ("bad predefined tdesc type");
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +struct tdesc_type *
> > +tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> > +{
> > + /* First try target-defined types. */
> > + for (const tdesc_type_up &type : feature->types)
> > + if (type->name == id)
> > + return type.get ();
> > +
> > + /* Next try the predefined types. */
> > + for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> > + if (tdesc_predefined_types[ix].name == id)
> > + return &tdesc_predefined_types[ix];
> > +
> > + return NULL;
> > +}
> > +
> > /* See arch/tdesc.h. */
> >
> > void
> > @@ -96,3 +148,144 @@ tdesc_create_reg (struct tdesc_feature *feature,
> const char *name,
> >
> > feature->registers.emplace_back (reg);
> > }
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +struct tdesc_type *
> > +tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> > + struct tdesc_type *field_type, int count)
> > +{
> > + tdesc_type_vector *type = new tdesc_type_vector (name, field_type,
> count);
> > + feature->types.emplace_back (type);
> > +
> > + return type;
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +tdesc_type_with_fields *
> > +tdesc_create_struct (struct tdesc_feature *feature, const char *name)
> > +{
> > + tdesc_type_with_fields *type
> > + = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
> > + feature->types.emplace_back (type);
> > +
> > + return type;
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +void
> > +tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> > +{
> > + gdb_assert (type->kind == TDESC_TYPE_STRUCT);
> > + gdb_assert (size > 0);
> > + type->size = size;
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +tdesc_type_with_fields *
> > +tdesc_create_union (struct tdesc_feature *feature, const char *name)
> > +{
> > + tdesc_type_with_fields *type
> > + = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
> > + feature->types.emplace_back (type);
> > +
> > + return type;
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +tdesc_type_with_fields *
> > +tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> > + int size)
> > +{
> > + gdb_assert (size > 0);
> > +
> > + tdesc_type_with_fields *type
> > + = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
> > + feature->types.emplace_back (type);
> > +
> > + return type;
> > +}
> > +
> > +tdesc_type_with_fields *
> > +tdesc_create_enum (struct tdesc_feature *feature, const char *name,
> > + int size)
> > +{
> > + gdb_assert (size > 0);
> > +
> > + tdesc_type_with_fields *type
> > + = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
> > + feature->types.emplace_back (type);
> > +
> > + return type;
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +void
> > +tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> > + struct tdesc_type *field_type)
> > +{
> > + gdb_assert (type->kind == TDESC_TYPE_UNION
> > + || type->kind == TDESC_TYPE_STRUCT);
> > +
> > + /* Initialize start and end so we know this is not a bit-field
> > + when we print-c-tdesc. */
> > + type->fields.emplace_back (field_name, field_type, -1, -1);
> > +}
> > +
> > +void
> > +tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char
> *field_name,
> > + int start, int end, struct tdesc_type
> *field_type)
> > +{
> > + gdb_assert (type->kind == TDESC_TYPE_STRUCT
> > + || type->kind == TDESC_TYPE_FLAGS);
> > + gdb_assert (start >= 0 && end >= start);
> > +
> > + type->fields.emplace_back (field_name, field_type, start, end);
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +void
> > +tdesc_add_bitfield (tdesc_type_with_fields *type, const char
> *field_name,
> > + int start, int end)
> > +{
> > + struct tdesc_type *field_type;
> > +
> > + gdb_assert (start >= 0 && end >= start);
> > +
> > + if (type->size > 4)
> > + field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
> > + else
> > + field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
> > +
> > + tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
> > +}
> > +
> > +/* See arch/tdesc.h. */
> > +
> > +void
> > +tdesc_add_flag (tdesc_type_with_fields *type, int start,
> > + const char *flag_name)
> > +{
> > + gdb_assert (type->kind == TDESC_TYPE_FLAGS
> > + || type->kind == TDESC_TYPE_STRUCT);
> > +
> > + type->fields.emplace_back (flag_name,
> > + tdesc_predefined_type (TDESC_TYPE_BOOL),
> > + start, start);
> > +}
> > +
> > +void
> > +tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
> > + const char *name)
> > +{
> > + gdb_assert (type->kind == TDESC_TYPE_ENUM);
> > + type->fields.emplace_back (name,
> > + tdesc_predefined_type (TDESC_TYPE_INT32),
> > + value, -1);
> > +}
> > \ No newline at end of file
>
> With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
> so many functions shared many of them could be made class methods. However
> that exceeds the scope of this patch set, but should be kept in mind for
> the
> future.
>
Apparently, it may seem better to move the whole target description code to
a common location and share it between gdb and gdbserver.
Target descriptions are architecture specific but at the same time target
description class code is architecture independent. So as highlighted by
Phillip this code should be moved out of arc/ folder and placed elsewhere.
It may seem a clean approach to create the division now and also refactor
code now than doing it in a later patch. Only gotcha here is that this code
is used by multiple targets so need to be tested aggressively.
>
> Philipp
>
>
> > diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> > index bc94d83ae58b9e79d8b71d0ce21c073f11765dd4..
> f0bd266a54601484df74ee1c5f8dce6fe04661c4 100644
> > --- a/gdb/gdbserver/tdesc.c
> > +++ b/gdb/gdbserver/tdesc.c
> > @@ -197,70 +197,17 @@ tdesc_create_feature (struct target_desc *tdesc,
> const char *name,
> > return new_feature;
> > }
> >
> > -/* See arch/tdesc.h. */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> > - int size)
> > -{
> > - return NULL;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_add_flag (tdesc_type_with_fields *type, int start,
> > - const char *flag_name)
> > -{}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -struct tdesc_type *
> > -tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> > -{
> > - return NULL;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_union (struct tdesc_feature *feature, const char *id)
> > +type *tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> > {
> > - return NULL;
> > + error (_("Cannot create gdbtypes."));
> > }
> >
> > -/* See arch/tdesc.h. */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_struct (struct tdesc_feature *feature, const char *id)
> > -{
> > - return NULL;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -struct tdesc_type *
> > -tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> > - struct tdesc_type *field_type, int count)
> > +type *tdesc_type_vector::make_gdb_type (struct gdbarch *gdbarch) const
> > {
> > - return NULL;
> > + error (_("Cannot create gdbtypes."));
> > }
> >
> > -void
> > -tdesc_add_bitfield (tdesc_type_with_fields *type, const char
> *field_name,
> > - int start, int end)
> > -{}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> > - struct tdesc_type *field_type)
> > -{}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> > +type *tdesc_type_with_fields::make_gdb_type (struct gdbarch *gdbarch)
> const
> > {
> > + error (_("Cannot create gdbtypes."));
> > }
> > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> > index 8ead19efa24b3cb154c895c484e791b92ff61387..
> 4f11120dce4092f15de99680a7c4868c4a2f4493 100644
> > --- a/gdb/target-descriptions.c
> > +++ b/gdb/target-descriptions.c
> > @@ -50,80 +50,6 @@ struct property
> > std::string value;
> > };
> >
> > -/* A named type from a target description. */
> > -
> > -struct tdesc_type_field
> > -{
> > - tdesc_type_field (const std::string &name_, tdesc_type *type_,
> > - int start_, int end_)
> > - : name (name_), type (type_), start (start_), end (end_)
> > - {}
> > -
> > - std::string name;
> > - struct tdesc_type *type;
> > - /* For non-enum-values, either both are -1 (non-bitfield), or both are
> > - not -1 (bitfield). For enum values, start is the value (which
> could be
> > - -1), end is -1. */
> > - int start, end;
> > -};
> > -
> > -struct tdesc_type_builtin : tdesc_type
> > -{
> > - tdesc_type_builtin (const std::string &name, enum tdesc_type_kind
> kind)
> > - : tdesc_type (name, kind)
> > - {}
> > -
> > - void accept (tdesc_element_visitor &v) const override
> > - {
> > - v.visit (this);
> > - }
> > -
> > - type *make_gdb_type (struct gdbarch *gdbarch) const override;
> > -};
> > -
> > -/* tdesc_type for vector types. */
> > -
> > -struct tdesc_type_vector : tdesc_type
> > -{
> > - tdesc_type_vector (const std::string &name, tdesc_type
> *element_type_, int count_)
> > - : tdesc_type (name, TDESC_TYPE_VECTOR),
> > - element_type (element_type_), count (count_)
> > - {}
> > -
> > - void accept (tdesc_element_visitor &v) const override
> > - {
> > - v.visit (this);
> > - }
> > -
> > - type *make_gdb_type (struct gdbarch *gdbarch) const override;
> > -
> > - struct tdesc_type *element_type;
> > - int count;
> > -};
> > -
> > -/* tdesc_type for struct, union, flags, and enum types. */
> > -
> > -struct tdesc_type_with_fields : tdesc_type
> > -{
> > - tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
> > - int size_ = 0)
> > - : tdesc_type (name, kind), size (size_)
> > - {}
> > -
> > - void accept (tdesc_element_visitor &v) const override
> > - {
> > - v.visit (this);
> > - }
> > -
> > - type *make_gdb_type_struct (struct gdbarch *gdbarch) const;
> > - type *make_gdb_type_union (struct gdbarch *gdbarch) const;
> > - type *make_gdb_type_flags (struct gdbarch *gdbarch) const;
> > - type *make_gdb_type_enum (struct gdbarch *gdbarch) const;
> > - type *make_gdb_type (struct gdbarch *gdbarch) const override;
> > -
> > - std::vector<tdesc_type_field> fields;
> > - int size;
> > -};
> >
> > type *
> > tdesc_type_builtin::make_gdb_type (struct gdbarch *gdbarch) const
> > @@ -733,58 +659,6 @@ tdesc_feature_name (const struct tdesc_feature
> *feature)
> > return feature->name.c_str ();
> > }
> >
> > -/* Predefined types. */
> > -static tdesc_type_builtin tdesc_predefined_types[] =
> > -{
> > - { "bool", TDESC_TYPE_BOOL },
> > - { "int8", TDESC_TYPE_INT8 },
> > - { "int16", TDESC_TYPE_INT16 },
> > - { "int32", TDESC_TYPE_INT32 },
> > - { "int64", TDESC_TYPE_INT64 },
> > - { "int128", TDESC_TYPE_INT128 },
> > - { "uint8", TDESC_TYPE_UINT8 },
> > - { "uint16", TDESC_TYPE_UINT16 },
> > - { "uint32", TDESC_TYPE_UINT32 },
> > - { "uint64", TDESC_TYPE_UINT64 },
> > - { "uint128", TDESC_TYPE_UINT128 },
> > - { "code_ptr", TDESC_TYPE_CODE_PTR },
> > - { "data_ptr", TDESC_TYPE_DATA_PTR },
> > - { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
> > - { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
> > - { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
> > - { "i387_ext", TDESC_TYPE_I387_EXT }
> > -};
> > -
> > -/* Lookup a predefined type. */
> > -
> > -static struct tdesc_type *
> > -tdesc_predefined_type (enum tdesc_type_kind kind)
> > -{
> > - for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> > - if (tdesc_predefined_types[ix].kind == kind)
> > - return &tdesc_predefined_types[ix];
> > -
> > - gdb_assert_not_reached ("bad predefined tdesc type");
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -struct tdesc_type *
> > -tdesc_named_type (const struct tdesc_feature *feature, const char *id)
> > -{
> > - /* First try target-defined types. */
> > - for (const tdesc_type_up &type : feature->types)
> > - if (type->name == id)
> > - return type.get ();
> > -
> > - /* Next try the predefined types. */
> > - for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
> > - if (tdesc_predefined_types[ix].name == id)
> > - return &tdesc_predefined_types[ix];
> > -
> > - return NULL;
> > -}
> > -
> > /* Lookup type associated with ID. */
> >
> > struct type *
> > @@ -1219,147 +1093,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
> >
> > /* See arch/tdesc.h. */
> >
> > -struct tdesc_type *
> > -tdesc_create_vector (struct tdesc_feature *feature, const char *name,
> > - struct tdesc_type *field_type, int count)
> > -{
> > - tdesc_type_vector *type = new tdesc_type_vector (name, field_type,
> count);
> > - feature->types.emplace_back (type);
> > -
> > - return type;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_struct (struct tdesc_feature *feature, const char *name)
> > -{
> > - tdesc_type_with_fields *type
> > - = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
> > - feature->types.emplace_back (type);
> > -
> > - return type;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
> > -{
> > - gdb_assert (type->kind == TDESC_TYPE_STRUCT);
> > - gdb_assert (size > 0);
> > - type->size = size;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_union (struct tdesc_feature *feature, const char *name)
> > -{
> > - tdesc_type_with_fields *type
> > - = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
> > - feature->types.emplace_back (type);
> > -
> > - return type;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_flags (struct tdesc_feature *feature, const char *name,
> > - int size)
> > -{
> > - gdb_assert (size > 0);
> > -
> > - tdesc_type_with_fields *type
> > - = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
> > - feature->types.emplace_back (type);
> > -
> > - return type;
> > -}
> > -
> > -tdesc_type_with_fields *
> > -tdesc_create_enum (struct tdesc_feature *feature, const char *name,
> > - int size)
> > -{
> > - gdb_assert (size > 0);
> > -
> > - tdesc_type_with_fields *type
> > - = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
> > - feature->types.emplace_back (type);
> > -
> > - return type;
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
> > - struct tdesc_type *field_type)
> > -{
> > - gdb_assert (type->kind == TDESC_TYPE_UNION
> > - || type->kind == TDESC_TYPE_STRUCT);
> > -
> > - /* Initialize start and end so we know this is not a bit-field
> > - when we print-c-tdesc. */
> > - type->fields.emplace_back (field_name, field_type, -1, -1);
> > -}
> > -
> > -void
> > -tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char
> *field_name,
> > - int start, int end, struct tdesc_type
> *field_type)
> > -{
> > - gdb_assert (type->kind == TDESC_TYPE_STRUCT
> > - || type->kind == TDESC_TYPE_FLAGS);
> > - gdb_assert (start >= 0 && end >= start);
> > -
> > - type->fields.emplace_back (field_name, field_type, start, end);
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_add_bitfield (tdesc_type_with_fields *type, const char
> *field_name,
> > - int start, int end)
> > -{
> > - struct tdesc_type *field_type;
> > -
> > - gdb_assert (start >= 0 && end >= start);
> > -
> > - if (type->size > 4)
> > - field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
> > - else
> > - field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
> > -
> > - tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > -void
> > -tdesc_add_flag (tdesc_type_with_fields *type, int start,
> > - const char *flag_name)
> > -{
> > - gdb_assert (type->kind == TDESC_TYPE_FLAGS
> > - || type->kind == TDESC_TYPE_STRUCT);
> > -
> > - type->fields.emplace_back (flag_name,
> > - tdesc_predefined_type (TDESC_TYPE_BOOL),
> > - start, start);
> > -}
> > -
> > -void
> > -tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
> > - const char *name)
> > -{
> > - gdb_assert (type->kind == TDESC_TYPE_ENUM);
> > - type->fields.emplace_back (name,
> > - tdesc_predefined_type (TDESC_TYPE_INT32),
> > - value, -1);
> > -}
> > -
> > -/* See arch/tdesc.h. */
> > -
> > struct tdesc_feature *
> > tdesc_create_feature (struct target_desc *tdesc, const char *name,
> > const char *xml)
> >
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/8] Use tdesc types in gdbserver tdesc
2018-01-29 7:28 ` Omair Javaid
@ 2018-01-29 11:01 ` Alan Hayward
2018-01-29 11:31 ` Philipp Rudo
0 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-29 11:01 UTC (permalink / raw)
To: Omair Javaid; +Cc: Philipp Rudo, gdb-patches, nd
>
> With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
> so many functions shared many of them could be made class methods. However
> that exceeds the scope of this patch set, but should be kept in mind for the
> future.
The "[PATCH] Use visitors for make_gdb_type” patch should take care of a large set of them.
>
> Apparently, it may seem better to move the whole target description code to a common location and share it between gdb and gdbserver.
>
> Target descriptions are architecture specific but at the same time target description class code is architecture independent. So as highlighted by Phillip this code should be moved out of arc/ folder and placed elsewhere.
>
Agreed.
Is common/ the best place? I can’t see any other directory that looks right.
> It may seem a clean approach to create the division now and also refactor code now than doing it in a later patch. Only gotcha here is that this code is used by multiple targets so need to be tested aggressively.
That’s why I didn’t want to refactor too much in one set.
Thanks,
Alan.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/8] Use tdesc types in gdbserver tdesc
2018-01-29 11:01 ` Alan Hayward
@ 2018-01-29 11:31 ` Philipp Rudo
2018-01-29 15:52 ` Alan Hayward
0 siblings, 1 reply; 32+ messages in thread
From: Philipp Rudo @ 2018-01-29 11:31 UTC (permalink / raw)
To: Alan Hayward; +Cc: Omair Javaid, gdb-patches, nd
Hi Alan,
On Mon, 29 Jan 2018 11:01:25 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:
> >
> > With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
> > so many functions shared many of them could be made class methods. However
> > that exceeds the scope of this patch set, but should be kept in mind for the
> > future.
>
> The "[PATCH] Use visitors for make_gdb_type” patch should take care of a large set of them.
I was more thinking of functions like tdesc_add_* and tdesc_create_* (including
tdesc_create_reg). They are not covered by your make_gdb_type patch. The way
I see it the changes needed to be made are quite small. Unfortunately they
require to regenerate the cfiles making the resulting patches large and hard
to read.
Philipp
> >
> > Apparently, it may seem better to move the whole target description code to a common location and share it between gdb and gdbserver.
> >
> > Target descriptions are architecture specific but at the same time target description class code is architecture independent. So as highlighted by Phillip this code should be moved out of arc/ folder and placed elsewhere.
> >
>
> Agreed.
> Is common/ the best place? I can’t see any other directory that looks right.
>
> > It may seem a clean approach to create the division now and also refactor code now than doing it in a later patch. Only gotcha here is that this code is used by multiple targets so need to be tested aggressively.
>
> That’s why I didn’t want to refactor too much in one set.
>
>
> Thanks,
> Alan.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/8] Use tdesc types in gdbserver tdesc
2018-01-29 11:31 ` Philipp Rudo
@ 2018-01-29 15:52 ` Alan Hayward
0 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-29 15:52 UTC (permalink / raw)
To: Philipp Rudo; +Cc: Omair Javaid, gdb-patches, nd
> On 29 Jan 2018, at 11:30, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
>
> Hi Alan,
>
>
> On Mon, 29 Jan 2018 11:01:25 +0000
> Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>>>
>>> With tdesc_features, tdesc_type_with_fields etc. being no longer opaque and
>>> so many functions shared many of them could be made class methods. However
>>> that exceeds the scope of this patch set, but should be kept in mind for the
>>> future.
>>
>> The "[PATCH] Use visitors for make_gdb_type” patch should take care of a large set of them.
>
> I was more thinking of functions like tdesc_add_* and tdesc_create_* (including
> tdesc_create_reg). They are not covered by your make_gdb_type patch. The way
> I see it the changes needed to be made are quite small. Unfortunately they
> require to regenerate the cfiles making the resulting patches large and hard
> to read.
>
Ok, happy to look at these in a follow on patch. Should be simple enough to write the patch,
just a little messy to review.
Alan.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-26 22:31 ` Omair Javaid
@ 2018-01-29 16:28 ` Yao Qi
2018-01-29 17:13 ` Alan Hayward
0 siblings, 1 reply; 32+ messages in thread
From: Yao Qi @ 2018-01-29 16:28 UTC (permalink / raw)
To: Omair Javaid; +Cc: Philipp Rudo, Alan Hayward, gdb-patches, nd
Hi Omair,
Thanks for testing these patches...
On Fri, Jan 26, 2018 at 10:31 PM, Omair Javaid <omair.javaid@linaro.org> wrote:
> Hi Alan,
>
> I just ran gdb testsuite with native-gdbserver configuration on your
> patches and found a couple of regressions. At least first three seems to be
> coming out of your patch series as they seem persistent over multiple runs
> of testsuite.
>
> 1c1
> < Test Run By omair on Sat Jan 27 02:51:24 2018
> ---
>> Test Run By omair on Sat Jan 27 01:58:06 2018
> 1257,1258c1257,1258
> < FAIL: gdb.arch/i386-mpx.exp: bndcfgu formating
> < FAIL: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
> ---
>> PASS: gdb.arch/i386-mpx.exp: bndcfgu formating
>> PASS: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
> 6706c6706
> < FAIL: gdb.base/gcore.exp: corefile restored all registers
> ---
>> PASS: gdb.base/gcore.exp: corefile restored all registers
> 50790c50790
If I read the diff correctly, there are regressions in
gdb.arch/i386-mpx.exp and gdb.base/gcore.exp.
> < PASS: gdb.threads/thread-unwindonsignal.exp: continue until exit
> ---
>> FAIL: gdb.threads/thread-unwindonsignal.exp: continue until exit (timeout)
> 55145,55146c55145,55146
This test case isn't stable for me, I remember I saw the fail several times
before.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-29 16:28 ` Yao Qi
@ 2018-01-29 17:13 ` Alan Hayward
2018-01-31 11:28 ` Alan Hayward
0 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-29 17:13 UTC (permalink / raw)
To: Yao Qi; +Cc: Omair Javaid, Philipp Rudo, gdb-patches, nd
Only just spotted this thread.
> On 29 Jan 2018, at 16:28, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Hi Omair,
> Thanks for testing these patches...
>
> On Fri, Jan 26, 2018 at 10:31 PM, Omair Javaid <omair.javaid@linaro.org> wrote:
>> Hi Alan,
>>
>> I just ran gdb testsuite with native-gdbserver configuration on your
>> patches and found a couple of regressions. At least first three seems to be
>> coming out of your patch series as they seem persistent over multiple runs
>> of testsuite.
>>
>> 1c1
>> < Test Run By omair on Sat Jan 27 02:51:24 2018
>> ---
>>> Test Run By omair on Sat Jan 27 01:58:06 2018
>> 1257,1258c1257,1258
>> < FAIL: gdb.arch/i386-mpx.exp: bndcfgu formating
>> < FAIL: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
>> ---
>>> PASS: gdb.arch/i386-mpx.exp: bndcfgu formating
>>> PASS: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
>> 6706c6706
>> < FAIL: gdb.base/gcore.exp: corefile restored all registers
>> ---
>>> PASS: gdb.base/gcore.exp: corefile restored all registers
>> 50790c50790
>
> If I read the diff correctly, there are regressions in
> gdb.arch/i386-mpx.exp and gdb.base/gcore.exp.
Looking into it, I always get UNTESTED: gdb.arch/i386-mpx.exp: failed to prepare
MPX was a Skylake addition. I’ve been using a Sandy Bridge processor.
Is there any emulator setup or suchlike to enable testing?
I’ll see if I can find a suitable box to use.
>
>> < PASS: gdb.threads/thread-unwindonsignal.exp: continue until exit
>> ---
>>> FAIL: gdb.threads/thread-unwindonsignal.exp: continue until exit (timeout)
>> 55145,55146c55145,55146
>
> This test case isn't stable for me, I remember I saw the fail several times
> before.
>
> --
> Yao (齐尧)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
` (9 preceding siblings ...)
2018-01-25 13:11 ` Philipp Rudo
@ 2018-01-29 18:18 ` Pedro Alves
2018-01-30 12:16 ` Alan Hayward
10 siblings, 1 reply; 32+ messages in thread
From: Pedro Alves @ 2018-01-29 18:18 UTC (permalink / raw)
To: Alan Hayward, gdb-patches; +Cc: nd
On 01/24/2018 09:26 AM, Alan Hayward wrote:
> In exisiting code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, it
> creates an xml containing just the name of the original xml file.
> Upon receipt, GDB reads and parses a local copy of xml file.
Huh? What do you mean reads and parses a _local copy_?
That's false AFAICS.
GDB fetches named file from the server, not from the local filesystem.
E.g., on x86-64:
Sending packet: $qXfer:features:read:target.xml:0,fff#7d...Packet received: l<?xml version="1.0"?><!DOCTYPE target SYSTEM "gdb-target.dtd"><target><architecture>i386:x86-64</architecture><osabi>GNU/Linux</osabi><xi:include href="64bit-core.xml"/><xi:include href="64bit-sse.xml"/><xi:include href="64bit-linux.xml"/><xi:include href="64bit-segments.xml"/><xi:include href="64bit-avx.xml"/></target>
Sending packet: $qXfer:features:read:64bit-core.xml:0,fff#75...Packet received: l<?xml version="1.0"?>\n<!-- Copyright (C) 2010-2018 Free Software Foundation, Inc.\n\n Copying and distribution of this file, with or without modification,\n are permitted in any medium without royalty provided the copyright\n notice and this notice are preserved. -->\n\n<!DOCTYPE feature SYSTEM "gdb-target.dtd">\n<feature name="org.gnu.gdb.i386.core">\n <flags id="i386_eflags" size="4">\n <field name="CF" start="0" end="0"/>\n <field name="" start="1" end="1"/>\n <field name="PF" start="2" end=[12 bytes omitted]
Sending packet: $qXfer:features:read:64bit-sse.xml:0,fff#17...Packet received: l<?xml version="1.0"?>\n<!-- Copyright (C) 2010-2018 Free Software Foundation, Inc.\n\n Copying and distribution of this file, with or without modification,\n are permitted in any medium without royalty provided the copyright\n notice and this notice are preserved. -->\n\n<!DOCTYPE feature SYSTEM "gdb-target.dtd">\n<feature name="org.gnu.gdb.i386.sse">\n <vector id="v4f" type="ieee_single" count="4"/>\n <vector id="v2d" type="ieee_double" count="2"/>\n <vector id="v16i8" type="int8" count="16"/>\n <vec[12 bytes omitted]
Sending packet: $qXfer:features:read:64bit-linux.xml:0,fff#fc...Packet received: l<?xml version="1.0"?>\n<!-- Copyright (C) 2010-2018 Free Software Foundation, Inc.\n\n Copying and distribution of this file, with or without modification,\n are permitted in any medium without royalty provided the copyright\n notice and this notice are preserved. -->\n\n<!DOCTYPE feature SYSTEM "gdb-target.dtd">\n<feature name="org.gnu.gdb.i386.linux">\n <reg name="orig_rax" bitsize="64" type="int" regnum="57"/>\n</feature>\n
Sending packet: $qXfer:features:read:64bit-segments.xml:0,fff#32...Packet received: l<?xml version="1.0"?>\n<!-- Copyright (C) 2016-2018 Free Software Foundation, Inc.\n\n Copying and distribution of this file, with or without modification,\n are permitted in any medium without royalty provided the copyright\n notice and this notice are preserved. -->\n\n<!DOCTYPE feature SYSTEM "gdb-target.dtd">\n<feature name="org.gnu.gdb.i386.segments">\n <reg name="fs_base" bitsize="64" type="int"/>\n <reg name="gs_base" bitsize="64" type="int"/>\n</feature>\n
Sending packet: $qXfer:features:read:64bit-avx.xml:0,fff#1b...Packet received: l<?xml version="1.0"?>\n<!-- Copyright (C) 2010-2018 Free Software Foundation, Inc.\n\n Copying and distribution of this file, with or without modification,\n are permitted in any medium without royalty provided the copyright\n notice and this notice are preserved. -->\n\n<!DOCTYPE feature SYSTEM "gdb-target.dtd">\n<feature name="org.gnu.gdb.i386.avx">\n <reg name="ymm0h" bitsize="128" type="uint128"/>\n <reg name="ymm1h" bitsize="128" type="uint128"/>\n <reg name="ymm2h" bitsize="128" type="uint128"/>[11 bytes omitted]
Above we see GDB processing the xi:includes by subsequently fetching the
xi:included files from the server.
There is already some support in gdbserver for baking in the XML
code directly in gdbserver instead of reading it from (gdbserver's)
filesystem:
/* `desc->xmltarget' defines what to return when looking for the
"target.xml" file. Its contents can either be verbatim XML code
(prefixed with a '@') or else the name of the actual XML file to
be used in place of "target.xml".
This variable is set up from the auto-generated
init_registers_... routine for the current target. */
if (strcmp (annex, "target.xml") == 0)
{
const char *ret = tdesc_get_features_xml ((target_desc*) desc);
if (*ret == '@')
return ret + 1;
else
annex = ret;
}
Can you please clarify the rationale for the series?
The current justification doesn't look very convincing to me. This
is going to be probably something around dynamic generation of XML
descriptions based on CPU optional features, instead of having
to have gdbserver carry a bunch of different XML files for each possible
combination of optional features. (I.e., a continuation of Yao's
earlier work).
I haven't read the patches, but it's likely that you should need
to tweak the individual patches' rationales too.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-29 18:18 ` Pedro Alves
@ 2018-01-30 12:16 ` Alan Hayward
0 siblings, 0 replies; 32+ messages in thread
From: Alan Hayward @ 2018-01-30 12:16 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, nd
> On 29 Jan 2018, at 18:18, Pedro Alves <palves@redhat.com> wrote:
>
> On 01/24/2018 09:26 AM, Alan Hayward wrote:
>> In exisiting code, gdbserver uses C code auto generated from xml files to
>> create target descriptions. When sending an xml description to GDB, it
>> creates an xml containing just the name of the original xml file.
>> Upon receipt, GDB reads and parses a local copy of xml file.
>
> Huh? What do you mean reads and parses a _local copy_?
> That's false AFAICS.
>
> GDB fetches named file from the server, not from the local filesystem.
> E.g., on x86-64:
>
> Sending packet: $qXfer:features:read:target.xml:0,fff#7d...Packet received: l<?xml version="1.0"?><!DOCTYPE target SYSTEM "gdb-target.dtd"><target><architecture>i386:x86-64</architecture><osabi>GNU/Linux</osabi><xi:include href="64bit-core.xml"/><xi:include href="64bit-sse.xml"/><xi:include href="64bit-linux.xml"/><xi:include href="64bit-segments.xml"/><xi:include href="64bit-avx.xml"/></target>
> Sending packet: $qXfer:features:read:64bit-core.xml:0,fff#75...Packet received: l<?xml version="1.0"?>\n<!-- Copyright (C) 2010-2018 Free Software Foundation, Inc.\n\n Copying and distribution of this file, with or without modification,\n are permitted in any medium without royalty provided the copyright\n notice and this notice are preserved. -->\n\n<!DOCTYPE feature SYSTEM "gdb-target.dtd">\n<feature name="org.gnu.gdb.i386.core">\n <flags id="i386_eflags" size="4">\n <field name="CF" start="0" end="0"/>\n <field name="" start="1" end="1"/>\n <field name="PF" start="2" end=[12 bytes omitted]
Yes, I misunderstood what was happening here.
(The review of the final patch of v1 cleared this up for me, but I never updated
this part of the abstract for v2).
Instead I should have said something like:
In exisiting code, gdbserver uses C code auto generated from xml files to
create target descriptions. When sending an xml description to GDB, it
creates an xml containing the name of the original xml file(s).
GDB parses the xml and then requests from gdbserver a copy of each of
the included files.
With this new patch, we add common code that allows gdbserver (and gdb)
to turn a C target description structure into xml. Now when sending an xml
description to gdb, gdbserver creates a single xml structure containing the
entire target description, without any includes. GDB no longer needs to ask
for additional xml packets from gdbserver.
>
> Above we see GDB processing the xi:includes by subsequently fetching the
> xi:included files from the server.
>
> There is already some support in gdbserver for baking in the XML
> code directly in gdbserver instead of reading it from (gdbserver's)
> filesystem:
>
> /* `desc->xmltarget' defines what to return when looking for the
> "target.xml" file. Its contents can either be verbatim XML code
> (prefixed with a '@') or else the name of the actual XML file to
> be used in place of "target.xml".
>
> This variable is set up from the auto-generated
> init_registers_... routine for the current target. */
>
> if (strcmp (annex, "target.xml") == 0)
> {
> const char *ret = tdesc_get_features_xml ((target_desc*) desc);
>
> if (*ret == '@')
> return ret + 1;
> else
> annex = ret;
> }
Essentially, tdesc_get_features_xml() is the function I am rewriting with this series.
In the existing code it will produce something like:
<!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>
My patch removes the include sections and instead dumps the entire xml:
<!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
>
> Can you please clarify the rationale for the series?
>
> The current justification doesn't look very convincing to me. This
> is going to be probably something around dynamic generation of XML
> descriptions based on CPU optional features, instead of having
> to have gdbserver carry a bunch of different XML files for each possible
> combination of optional features. (I.e., a continuation of Yao's
> earlier work).
The reason for me writing the patch series is eventually to support aarch64 sve.
With sve I want to avoid creating an xml file for every sve size possibility.
With this patch in place, I can avoid creating xml files for sve and instead write
the .c code (that would normally be generated from the xml)
Maybe I should have mentioned that in the abstract, but I wanted to avoid binding
this patch to future work.
The rational for this patch then is that is removes the need for gdbserver to include
xml files and it means a given target can have just a .c file in features/ instead of
an xml and generated .c file.
>
> I haven't read the patches, but it's likely that you should need
> to tweak the individual patches' rationales too.
I’ll look through them.
Thanks for the review!
>
> Thanks,
> Pedro Alves
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-29 17:13 ` Alan Hayward
@ 2018-01-31 11:28 ` Alan Hayward
2018-01-31 11:43 ` Omair Javaid
0 siblings, 1 reply; 32+ messages in thread
From: Alan Hayward @ 2018-01-31 11:28 UTC (permalink / raw)
To: Yao Qi, Omair Javaid; +Cc: Philipp Rudo, gdb-patches, nd
> On 29 Jan 2018, at 17:13, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> Only just spotted this thread.
>
>> On 29 Jan 2018, at 16:28, Yao Qi <qiyaoltc@gmail.com> wrote:
>>
>> Hi Omair,
>> Thanks for testing these patches...
>>
>> On Fri, Jan 26, 2018 at 10:31 PM, Omair Javaid <omair.javaid@linaro.org> wrote:
>>> Hi Alan,
>>>
>>> I just ran gdb testsuite with native-gdbserver configuration on your
>>> patches and found a couple of regressions. At least first three seems to be
>>> coming out of your patch series as they seem persistent over multiple runs
>>> of testsuite.
>>>
>>> 1c1
>>> < Test Run By omair on Sat Jan 27 02:51:24 2018
>>> ---
>>>> Test Run By omair on Sat Jan 27 01:58:06 2018
>>> 1257,1258c1257,1258
>>> < FAIL: gdb.arch/i386-mpx.exp: bndcfgu formating
>>> < FAIL: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
>>> ---
>>>> PASS: gdb.arch/i386-mpx.exp: bndcfgu formating
>>>> PASS: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
>>> 6706c6706
>>> < FAIL: gdb.base/gcore.exp: corefile restored all registers
>>> ---
>>>> PASS: gdb.base/gcore.exp: corefile restored all registers
>>> 50790c50790
>>
>> If I read the diff correctly, there are regressions in
>> gdb.arch/i386-mpx.exp and gdb.base/gcore.exp.
>
> Looking into it, I always get UNTESTED: gdb.arch/i386-mpx.exp: failed to prepare
>
> MPX was a Skylake addition. I’ve been using a Sandy Bridge processor.
>
> Is there any emulator setup or suchlike to enable testing?
>
> I’ll see if I can find a suitable box to use.
Was finally able to run the mpx tests.
(My MacBook is a Skylake, but is running macOS. Using ubuntu in virtualbox disables
mpx, but using ubuntu in vmware fusion is fine! Thankfully didn’t have to resort to a
live usb stick)
On the latest head, with or without my patches I always get the same results:
Mpx tests pass with unix / no target board, but fail with native-gdbserver
"gcore.exp: corefile restored all registers” fails with unix / no target board, and passes
with native-gdbserver.
I can’t see any obvious reason why my patches would break mpx or the gcore tests,
but never say never.
Is it worth me digging more into these tests?
>
>
>>
>>> < PASS: gdb.threads/thread-unwindonsignal.exp: continue until exit
>>> ---
>>>> FAIL: gdb.threads/thread-unwindonsignal.exp: continue until exit (timeout)
>>> 55145,55146c55145,55146
>>
>> This test case isn't stable for me, I remember I saw the fail several times
>> before.
>>
>> --
>> Yao (齐尧)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/8] Remove XML files from gdbserver
2018-01-31 11:28 ` Alan Hayward
@ 2018-01-31 11:43 ` Omair Javaid
0 siblings, 0 replies; 32+ messages in thread
From: Omair Javaid @ 2018-01-31 11:43 UTC (permalink / raw)
To: Alan Hayward; +Cc: Yao Qi, Philipp Rudo, gdb-patches, nd
On 31 January 2018 at 16:28, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>
> > On 29 Jan 2018, at 17:13, Alan Hayward <Alan.Hayward@arm.com> wrote:
> >
> > Only just spotted this thread.
> >
> >> On 29 Jan 2018, at 16:28, Yao Qi <qiyaoltc@gmail.com> wrote:
> >>
> >> Hi Omair,
> >> Thanks for testing these patches...
> >>
> >> On Fri, Jan 26, 2018 at 10:31 PM, Omair Javaid <omair.javaid@linaro.org>
> wrote:
> >>> Hi Alan,
> >>>
> >>> I just ran gdb testsuite with native-gdbserver configuration on your
> >>> patches and found a couple of regressions. At least first three seems
> to be
> >>> coming out of your patch series as they seem persistent over multiple
> runs
> >>> of testsuite.
> >>>
> >>> 1c1
> >>> < Test Run By omair on Sat Jan 27 02:51:24 2018
> >>> ---
> >>>> Test Run By omair on Sat Jan 27 01:58:06 2018
> >>> 1257,1258c1257,1258
> >>> < FAIL: gdb.arch/i386-mpx.exp: bndcfgu formating
> >>> < FAIL: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
> >>> ---
> >>>> PASS: gdb.arch/i386-mpx.exp: bndcfgu formating
> >>>> PASS: gdb.arch/i386-mpx.exp: test if bndstatus is enabled
> >>> 6706c6706
> >>> < FAIL: gdb.base/gcore.exp: corefile restored all registers
> >>> ---
> >>>> PASS: gdb.base/gcore.exp: corefile restored all registers
> >>> 50790c50790
> >>
> >> If I read the diff correctly, there are regressions in
> >> gdb.arch/i386-mpx.exp and gdb.base/gcore.exp.
> >
> > Looking into it, I always get UNTESTED: gdb.arch/i386-mpx.exp: failed to
> prepare
> >
> > MPX was a Skylake addition. I’ve been using a Sandy Bridge processor.
> >
> > Is there any emulator setup or suchlike to enable testing?
> >
> > I’ll see if I can find a suitable box to use.
>
> Was finally able to run the mpx tests.
> (My MacBook is a Skylake, but is running macOS. Using ubuntu in virtualbox
> disables
> mpx, but using ubuntu in vmware fusion is fine! Thankfully didn’t have to
> resort to a
> live usb stick)
>
> On the latest head, with or without my patches I always get the same
> results:
>
> Mpx tests pass with unix / no target board, but fail with native-gdbserver
>
> "gcore.exp: corefile restored all registers” fails with unix / no target
> board, and passes
> with native-gdbserver.
>
> I can’t see any obvious reason why my patches would break mpx or the gcore
> tests,
> but never say never.
>
> Is it worth me digging more into these tests?
>
>
Thanks Alan for looking into these failures.
As Yao suggested these tests have been failing previously so there could be
chance of a false positive here.
If these failures persists with only your patch series then it would be
worthwhile looking into the reasons behind those failures.
Probably, at this stage its better to fix other review comments and then
see if your patch series is breaking anything or not.
> >
> >
> >>
> >>> < PASS: gdb.threads/thread-unwindonsignal.exp: continue until exit
> >>> ---
> >>>> FAIL: gdb.threads/thread-unwindonsignal.exp: continue until exit
> (timeout)
> >>> 55145,55146c55145,55146
> >>
> >> This test case isn't stable for me, I remember I saw the fail several
> times
> >> before.
> >>
> >> --
> >> Yao (齐尧)
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2018-01-31 11:43 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 9:26 [PATCH v2 0/8] Remove XML files from gdbserver Alan Hayward
2018-01-24 9:26 ` [PATCH v2 1/8] Move tdesc header funcs to c file Alan Hayward
2018-01-24 9:27 ` [PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc Alan Hayward
2018-01-25 13:12 ` Philipp Rudo
2018-01-24 9:28 ` [PATCH v2 3/8] Use tdesc_feature in gdbserver tdesc Alan Hayward
2018-01-25 13:12 ` Philipp Rudo
2018-01-24 9:28 ` [PATCH v2 4/8] Move make_gdb_type functions within file Alan Hayward
2018-01-24 9:29 ` [PATCH v2 5/8] Use tdesc types in gdbserver tdesc Alan Hayward
2018-01-25 13:13 ` Philipp Rudo
2018-01-29 7:28 ` Omair Javaid
2018-01-29 11:01 ` Alan Hayward
2018-01-29 11:31 ` Philipp Rudo
2018-01-29 15:52 ` Alan Hayward
2018-01-24 9:30 ` [PATCH v2 6/8] Create xml from target descriptions Alan Hayward
2018-01-25 13:14 ` Philipp Rudo
2018-01-25 15:45 ` Yao Qi
2018-01-25 16:13 ` Alan Hayward
2018-01-25 16:56 ` Philipp Rudo
2018-01-24 9:31 ` [PATCH v2 7/8]: Remove xml file references " Alan Hayward
2018-01-24 9:32 ` [PATCH v2 8/8] Remove xml files from gdbserver Alan Hayward
2018-01-24 10:57 ` [PATCH v2 0/8] Remove XML " Omair Javaid
2018-01-24 12:29 ` Alan Hayward
2018-01-24 14:44 ` Omair Javaid
2018-01-24 18:53 ` Alan Hayward
2018-01-25 13:11 ` Philipp Rudo
2018-01-26 22:31 ` Omair Javaid
2018-01-29 16:28 ` Yao Qi
2018-01-29 17:13 ` Alan Hayward
2018-01-31 11:28 ` Alan Hayward
2018-01-31 11:43 ` Omair Javaid
2018-01-29 18:18 ` Pedro Alves
2018-01-30 12:16 ` Alan Hayward
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).