[PATCH v2 2/8] Use tdesc_reg in gxdbserver tdesc

Alan Hayward
2018-01-24T09:27:46Z

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