* [PATCH v4 00/10] Remove gdbserver dependency on xml files
@ 2018-03-22 8:44 alan.hayward
2018-03-22 8:45 ` [PATCH v4 01/10] Move tdesc header funcs to c file alan.hayward
` (10 more replies)
0 siblings, 11 replies; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:44 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
V4 addresses Philipps review comments. I'm fairly confident the issues he
found on s390 I reproduced on arm32, and have now fixed up - the code now
ensures xml is not generated for targets using older style descriptions.
Using git sendmail for the first time, which should sort out the formatting
issues I've had previously. However, just to the safe I've also pushed to
my patches to the remote branch users/ahayward/xml4.
This set adds two new patches to handle when to generate xml (fixing s390
issues) and the reg_defs vector change.
Summary:
For those targets that use new style target descriptions, this set of patches
removes the dependency on xml files. Namely:
* Removes inclusion of xml files within gdbserver.
* Removes the requirement for the .c files in features/ to be generated from
cached xml files.
This is made possible by changing xml descriptions generated by gdbserver, so
that instead of including xml file names, gdbserver now generate a complete
xml description.
The second point will be required for aarch64 SVE support, where the register
size are variable. Creating SVE xml files for every possible vector length
would not be feasible. Instead the plan for aarch64 SVE is to hand write the
features/ .c code that would normally be generated from xml.
Targets which use the older style target descriptions have not been changed.
XML Generation:
In existing code, gdbserver uses C code auto generated from xml files to
create target descriptions. When sending an xml description to GDB, the
function tdesc_get_features_xml () creates an xml containing the name of the
original xml file(s). For example:
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
<architecture>i386</architecture>
<osabi>GNU/Linux</osabi>
<xi:include href="32bit-core.xml"/>
<xi:include href="32bit-sse.xml"/>
<xi:include href="32bit-linux.xml"/>
<xi:include href="32bit-avx.xml"/>
</target>
Upon receipt, GDB then makes requests to gdbserver for the contents of the
xml files. Gdbserver keeps full copies all the xml files inside the binary.
This patch series adds common code that allows gdbserver (and gdb) to turn
a C target description structure into xml.
Now when asked fort an xml description to gdb, gdbserver turns the entire
target description structure back into xml, without using any cached files.
Producing, for example:
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
<architecture>i386</architecture>
<osabi>GNU/Linux</osabi>
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
<field name="CF" start="0" end="0"/>
<field name="" start="1" end="1"/>
<field name="PF" start="2" end="2"/>
<field name="AF" start="4" end="4"/>
...etc...
Patch Contents:
Patches 3-5 commonise the various target descriptor functionality, allowing
gdbserver to parse target descriptions in the same way as gdb. This series
does not commonise target_desc, but this is hopefully a long term goal.
The eighth patch adds the xml printer, which iterates through the parsing
generated in the previous patches.
The other patches are clean up patches.
Patches have been tested on a make check on x86 targets=all build with
target boards unix and native-gdbserver. Also built and tested aarch64 and
Arm32 (which uses old style descriptions)
In addition, patch six adds new test cases to unit test.
Alan.
Alan Hayward (10):
Move tdesc header funcs to c file
Make gdbserver reg_defs a vector of objects
Commonise tdesc_reg
Commonise tdesc_feature
Commonise tdesc types
Add tdesc osabi and architecture functions
Add feature reference in .dat files
Create xml from target descriptions
Remove xml file references from target descriptions.
Remove xml files from gdbserver
gdb/Makefile.in | 2 +
gdb/common/tdesc.c | 397 ++++++++++++++
gdb/common/tdesc.h | 312 ++++++++++-
gdb/features/Makefile | 6 +
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 | 28 -
gdb/gdbserver/regcache.c | 27 +-
gdb/gdbserver/regcache.h | 4 -
gdb/gdbserver/tdesc.c | 244 ++++-----
gdb/gdbserver/tdesc.h | 59 +-
gdb/regformats/aarch64.dat | 1 +
gdb/regformats/i386/amd64-avx-avx512-linux.dat | 1 +
gdb/regformats/i386/amd64-avx-linux.dat | 1 +
.../i386/amd64-avx-mpx-avx512-pku-linux.dat | 1 +
gdb/regformats/i386/amd64-avx-mpx-linux.dat | 1 +
gdb/regformats/i386/amd64-linux.dat | 1 +
gdb/regformats/i386/amd64-mpx-linux.dat | 1 +
gdb/regformats/i386/amd64.dat | 1 +
gdb/regformats/i386/i386-avx-avx512-linux.dat | 1 +
gdb/regformats/i386/i386-avx-linux.dat | 1 +
.../i386/i386-avx-mpx-avx512-pku-linux.dat | 1 +
gdb/regformats/i386/i386-avx-mpx-linux.dat | 1 +
gdb/regformats/i386/i386-linux.dat | 1 +
gdb/regformats/i386/i386-mmx-linux.dat | 1 +
gdb/regformats/i386/i386-mpx-linux.dat | 1 +
gdb/regformats/i386/i386.dat | 1 +
gdb/regformats/i386/x32-avx-avx512-linux.dat | 1 +
gdb/regformats/i386/x32-avx-linux.dat | 1 +
gdb/regformats/i386/x32-linux.dat | 1 +
gdb/regformats/regdat.sh | 12 +-
gdb/regformats/tic6x-c62x-linux.dat | 1 +
gdb/regformats/tic6x-c64x-linux.dat | 1 +
gdb/regformats/tic6x-c64xp-linux.dat | 1 +
gdb/target-descriptions.c | 596 +++------------------
gdb/xml-tdesc.c | 9 +
gdb/xml-tdesc.h | 5 +
57 files changed, 976 insertions(+), 792 deletions(-)
create mode 100644 gdb/common/tdesc.c
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 01/10] Move tdesc header funcs to c file
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-22 20:43 ` Simon Marchi
2018-03-22 8:45 ` [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects alan.hayward
` (9 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
Move the destructor and equals operator for gdbserver tdesc from the .h
to the .c file. Both functions are too long to be inlined and make the
header look messy. Patch does not change any functionality.
Patch identical to V3 version.
2018-03-21 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.
---
gdb/gdbserver/tdesc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/tdesc.h | 44 ++------------------------------------------
2 files changed, 50 insertions(+), 42 deletions(-)
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 00a5e8dc4d..e50a848e2f 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)
{
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index d21574ca73..4513ea7423 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
{
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
2018-03-22 8:45 ` [PATCH v4 01/10] Move tdesc header funcs to c file alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-22 21:33 ` Simon Marchi
2018-03-22 8:45 ` [PATCH v4 04/10] Commonise tdesc_feature alan.hayward
` (8 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
Following a suggestion from Philipp, this patch changes reg_defs in
gdbserver tdesc so that it is a vector of objects instead of a vector
of pointers. Updated uses of reg_defs to use references.
This change reduces the number of small hunks allocated when building
reg_defs.
find_register_by_number() is only used by regcache, therefore I made
it a static function and moved it earlier in the file so that find_regno()
could use it. The eventual plan is to replace this with accessor functions
in a common tdesc class.
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/gdbserver/
* regcache.c (find_register_by_number): Make static and return a ref.
(find_regno): Use find_register_by_number
(register_size): Use references.
(register_data): Likewise.
* regcache.h (struct reg): Remove declaration.
* tdesc.c (target_desc::~target_desc): Remove free calls.
(target_desc::operator==): Use references.
(init_target_desc): Likewise.
(tdesc_create_reg): Use references and resize in a single call.
* tdesc.h (struct target_desc): Replace pointer with object.
---
gdb/gdbserver/regcache.c | 27 +++++++++++----------------
gdb/gdbserver/regcache.h | 4 ----
gdb/gdbserver/tdesc.c | 30 ++++++++++++++----------------
gdb/gdbserver/tdesc.h | 2 +-
4 files changed, 26 insertions(+), 37 deletions(-)
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 1bb15900dd..df5b9e9dd2 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -196,6 +196,13 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
dst->registers_valid = src->registers_valid;
}
+/* Return a reference to the description of register ``n''. */
+
+static const struct reg &
+find_register_by_number (const struct target_desc *tdesc, int n)
+{
+ return tdesc->reg_defs[n];
+}
#ifndef IN_PROCESS_AGENT
@@ -243,25 +250,13 @@ int
find_regno (const struct target_desc *tdesc, const char *name)
{
for (int i = 0; i < tdesc->reg_defs.size (); ++i)
- {
- struct reg *reg = tdesc->reg_defs[i];
+ if (strcmp (name, find_register_by_number (tdesc, i).name) == 0)
+ return i;
- if (strcmp (name, reg->name) == 0)
- return i;
- }
internal_error (__FILE__, __LINE__, "Unknown register %s requested",
name);
}
-#endif
-
-struct reg *
-find_register_by_number (const struct target_desc *tdesc, int n)
-{
- return tdesc->reg_defs[n];
-}
-
-#ifndef IN_PROCESS_AGENT
static void
free_register_cache_thread (struct thread_info *thread)
{
@@ -292,7 +287,7 @@ register_cache_size (const struct target_desc *tdesc)
int
register_size (const struct target_desc *tdesc, int n)
{
- return find_register_by_number (tdesc, n)->size / 8;
+ return find_register_by_number (tdesc, n).size / 8;
}
/* See common/common-regcache.h. */
@@ -307,7 +302,7 @@ static unsigned char *
register_data (struct regcache *regcache, int n, int fetch)
{
return (regcache->registers
- + find_register_by_number (regcache->tdesc, n)->offset / 8);
+ + find_register_by_number (regcache->tdesc, n).offset / 8);
}
/* Supply register N, whose contents are stored in BUF, to REGCACHE.
diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
index 3a75ce3fe1..6ff13084b0 100644
--- a/gdb/gdbserver/regcache.h
+++ b/gdb/gdbserver/regcache.h
@@ -94,10 +94,6 @@ void registers_from_string (struct regcache *regcache, char *buf);
void regcache_write_pc (struct regcache *regcache, CORE_ADDR pc);
-/* Return a pointer to the description of register ``n''. */
-
-struct reg *find_register_by_number (const struct target_desc *tdesc, int n);
-
int register_cache_size (const struct target_desc *tdesc);
int register_size (const struct target_desc *tdesc, int n);
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e50a848e2f..8e68a27c7c 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -25,9 +25,6 @@ target_desc::~target_desc ()
{
int i;
- for (reg *reg : reg_defs)
- xfree (reg);
-
xfree ((char *) arch);
xfree ((char *) osabi);
@@ -45,10 +42,10 @@ bool target_desc::operator== (const target_desc &other) const
for (int i = 0; i < reg_defs.size (); ++i)
{
- struct reg *reg = reg_defs[i];
- struct reg *reg2 = other.reg_defs[i];
+ struct reg reg = reg_defs[i];
+ struct reg reg2 = other.reg_defs[i];
- if (reg != reg2 && *reg != *reg2)
+ if (® != ®2 && reg != reg2)
return false;
}
@@ -72,10 +69,10 @@ init_target_desc (struct target_desc *tdesc)
{
int offset = 0;
- for (reg *reg : tdesc->reg_defs)
+ for (reg ® : tdesc->reg_defs)
{
- reg->offset = offset;
- offset += reg->size;
+ reg.offset = offset;
+ offset += reg.size;
}
tdesc->registers_size = offset / 8;
@@ -240,24 +237,25 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
int bitsize, const char *type)
{
struct target_desc *tdesc = (struct target_desc *) feature;
+ int current_size = tdesc->reg_defs.size ();
- while (tdesc->reg_defs.size () < regnum)
- {
- struct reg *reg = XCNEW (struct reg);
+ tdesc->reg_defs.resize (regnum != 0 ? regnum + 1 : current_size + 1);
+ while (current_size < regnum)
+ {
+ struct reg *reg = &tdesc->reg_defs[current_size];
reg->name = "";
reg->size = 0;
- tdesc->reg_defs.push_back (reg);
+ current_size++;
}
gdb_assert (regnum == 0
- || regnum == tdesc->reg_defs.size ());
+ || regnum + 1 == tdesc->reg_defs.size ());
- struct reg *reg = XCNEW (struct reg);
+ struct reg *reg = &tdesc->reg_defs.back ();
reg->name = name;
reg->size = bitsize;
- tdesc->reg_defs.push_back (reg);
}
/* See common/tdesc.h. */
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 4513ea7423..a62544341c 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -34,7 +34,7 @@ struct target_desc : tdesc_feature
{
/* A vector of elements of register definitions that
describe the inferior's register set. */
- std::vector<struct reg *> reg_defs;
+ std::vector<struct reg> reg_defs;
/* The register cache size, in bytes. */
int registers_size;
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 08/10] Create xml from target descriptions
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (3 preceding siblings ...)
2018-03-22 8:45 ` [PATCH v4 05/10] Commonise tdesc types alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-23 21:24 ` Simon Marchi
2018-03-22 8:45 ` [PATCH v4 06/10] Add tdesc osabi and architecture functions alan.hayward
` (5 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
This patch adds a print_xml_feature visitor class which turns a C
target description into xml. Both gdb and gdbserver can do this.
An accept function is added to gdbserver tdesc to allow it to use
vistor classes.
Tests are added to maintenance_check_xml_descriptions which takes
each pair of tested descriptions, turns them both into xml, then back
again, and confirms the descriptions still match.
Differences from V3 are:
I now use string_appendf to print to the buffer.
regdat.sh ensures tdesc->xmltarget is null for all targets with new
style target descriptions. tdesc_get_features_xml() will only generate
xml if xmltarget is null. This ensures older targets continue to send
just the name of the xml file.
Added a save_restore check when parsing registers.
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/
* common/tdesc.c (print_xml_feature::visit_post): Add xml parsing.
(print_xml_feature::visit_pre): Likewise.
(print_xml_feature::visit_post): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::visit): Likewise.
* common/tdesc.h (print_xml_feature): Add new class.
* regformats/regdat.sh: Null xmltarget on feature targets.
* 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.
---
gdb/common/tdesc.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++
gdb/common/tdesc.h | 29 +++++++++++++
gdb/gdbserver/tdesc.c | 43 +++++++------------
gdb/gdbserver/tdesc.h | 4 +-
gdb/regformats/regdat.sh | 4 +-
gdb/target-descriptions.c | 48 ++++++++++++++++++++-
gdb/xml-tdesc.c | 9 ++++
gdb/xml-tdesc.h | 5 +++
8 files changed, 218 insertions(+), 30 deletions(-)
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index a0ed7c5115..04f8bc6912 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -289,3 +289,109 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
tdesc_predefined_type (TDESC_TYPE_INT32),
value, -1);
}
+
+void print_xml_feature::visit_pre (const tdesc_feature *e)
+{
+ string_appendf (m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
+}
+
+void print_xml_feature::visit_post (const tdesc_feature *e)
+{
+ string_appendf (m_buffer, "</feature>\n");
+}
+
+void print_xml_feature::visit (const tdesc_type_builtin *t)
+{
+ error (_("xml output is not supported type \"%s\"."), t->name.c_str ());
+}
+
+void print_xml_feature::visit (const tdesc_type_vector *t)
+{
+ string_appendf (m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
+ t->name.c_str (), t->element_type->name.c_str (), t->count);
+}
+
+void print_xml_feature::visit (const tdesc_type_with_fields *t)
+{
+ struct tdesc_type_field *f;
+ const static char *types[] = { "struct", "union", "flags", "enum" };
+
+ gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
+
+ string_appendf (m_buffer, "<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
+ t->name.c_str ());
+
+ switch (t->kind)
+ {
+ case TDESC_TYPE_STRUCT:
+ case TDESC_TYPE_FLAGS:
+ if (t->size > 0)
+ string_appendf (m_buffer, " size=\"%d\"", t->size);
+ string_appendf (m_buffer, ">\n");
+
+ for (const tdesc_type_field &f : t->fields)
+ {
+ string_appendf (m_buffer, " <field name=\"%s\" ", f.name.c_str ());
+ if (f.start == -1)
+ string_appendf (m_buffer, "type=\"%s\"/>\n", f.type->name.c_str ());
+ else
+ string_appendf (m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
+ f.end);
+ }
+ break;
+
+ case TDESC_TYPE_ENUM:
+ string_appendf (m_buffer, ">\n");
+ for (const tdesc_type_field &f : t->fields)
+ string_appendf (m_buffer, " <field name=\"%s\" start=\"%d\"/>\n",
+ f.name.c_str (), f.start);
+ break;
+
+ case TDESC_TYPE_UNION:
+ string_appendf (m_buffer, ">\n");
+ for (const tdesc_type_field &f : t->fields)
+ string_appendf (m_buffer, " <field name=\"%s\" type=\"%s\"/>\n",
+ f.name.c_str (), f.type->name.c_str ());
+ break;
+
+ default:
+ error (_("xml output is not supported type \"%s\"."), t->name.c_str ());
+ }
+
+ string_appendf (m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
+}
+
+void print_xml_feature::visit (const tdesc_reg *r)
+{
+ string_appendf (m_buffer,
+ "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
+ r->name.c_str (), r->bitsize, r->type.c_str (),
+ r->target_regnum);
+
+ if (r->group.length () > 0)
+ string_appendf (m_buffer, " group=\"%s\"", r->group.c_str ());
+
+ if (r->save_restore == 0)
+ string_appendf (m_buffer, " save-restore=\"no\"");
+
+ string_appendf (m_buffer, "/>\n");
+}
+
+void print_xml_feature::visit_pre (const target_desc *e)
+{
+#ifndef IN_PROCESS_AGENT
+ string_appendf (m_buffer, "<?xml version=\"1.0\"?>\n");
+ string_appendf (m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
+ string_appendf (m_buffer, "<target>\n<architecture>%s</architecture>\n",
+ tdesc_architecture_name (e));
+
+ const char *osabi = tdesc_osabi_name (e);
+ if (osabi != nullptr)
+ string_appendf (m_buffer, "<osabi>%s</osabi>", osabi);
+#endif
+}
+
+void print_xml_feature::visit_post (const target_desc *e)
+{
+ string_appendf (m_buffer, "</target>\n");
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 8ab77e365f..45eb24ea2b 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -371,4 +371,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/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 69bde2ae42..5ec73d9117 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -56,6 +56,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)
{
@@ -162,8 +174,7 @@ set_tdesc_osabi (struct target_desc *target_desc, const char *name)
target_desc->osabi = xstrdup (name);
}
-/* Return a string which is of XML format, including XML target
- description to be sent to GDB. */
+/* See common/tdesc.h. */
const char *
tdesc_get_features_xml (target_desc *tdesc)
@@ -175,31 +186,9 @@ tdesc_get_features_xml (target_desc *tdesc)
if (tdesc->xmltarget == NULL)
{
- std::string buffer ("@<?xml version=\"1.0\"?>");
-
- buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
- buffer += "<target>";
- buffer += "<architecture>";
- buffer += tdesc_architecture_name (tdesc);
- buffer += "</architecture>";
-
- const char *osabi = tdesc_osabi_name (tdesc);
- if (osabi != nullptr)
- {
- buffer += "<osabi>";
- buffer += osabi;
- buffer += "</osabi>";
- }
-
- for (const tdesc_feature_up &feature : tdesc->features)
- {
- buffer += "<xi:include href=\"";
- buffer += feature->name;
- buffer += "\"/>";
- }
-
- buffer += "</target>";
-
+ std::string buffer ("@");
+ print_xml_feature v (buffer);
+ tdesc->accept (v);
tdesc->xmltarget = xstrdup (buffer.c_str ());
}
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 197fb59127..c00c252f70 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -27,7 +27,7 @@
/* A target description. Inherit from tdesc_feature so that target_desc
can be used as tdesc_feature. */
-struct target_desc
+struct target_desc : tdesc_element
{
/* A vector of elements of register definitions that
describe the inferior's register set. */
@@ -73,6 +73,8 @@ public:
return !(*this == other);
}
#endif
+
+ void accept (tdesc_element_visitor &v) const;
};
/* Copy target description SRC to DEST. */
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 18108d77eb..8f546fe276 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -163,7 +163,9 @@ done
echo
echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
-if test "${xmltarget}" = x; then
+if test "${feature}" != x; then
+ echo "static const char *xmltarget_${name} = 0;"
+elif test "${xmltarget}" = x; then
if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
echo "static const char *xmltarget_${name} = 0;"
else
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index da2c1ce345..467ae94e0c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -333,6 +333,8 @@ struct target_desc : tdesc_element
/* The features associated with this target. */
std::vector<tdesc_feature_up> features;
+ char *xmltarget = nullptr;
+
void accept (tdesc_element_visitor &v) const override
{
v.visit_pre (this);
@@ -1667,6 +1669,21 @@ private:
int m_next_regnum = 0;
};
+/* See common/tdesc.h. */
+
+const char *
+tdesc_get_features_xml (target_desc *tdesc)
+{
+ if (tdesc->xmltarget == nullptr)
+ {
+ std::string buffer ("@");
+ print_xml_feature v (buffer);
+ tdesc->accept (v);
+ tdesc->xmltarget = xstrdup (buffer.c_str ());
+ }
+ return tdesc->xmltarget;
+}
+
static void
maint_print_c_tdesc_cmd (const char *args, int from_tty)
{
@@ -1760,7 +1777,36 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
= file_read_description_xml (tdesc_xml.data ());
if (tdesc == NULL || *tdesc != *e.second)
- failed++;
+ {
+ printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
+ failed++;
+ continue;
+ }
+
+ /* Convert both descriptions to xml, and then back again. Confirm all
+ descriptions are identical. */
+
+ const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
+ const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
+ gdb_assert (*xml == '@');
+ gdb_assert (*xml2 == '@');
+ const target_desc *t_trans = target_read_description_xml_string (xml+1);
+ const target_desc *t_trans2 = target_read_description_xml_string (xml2+1);
+
+ if (t_trans == NULL || t_trans2 == NULL)
+ {
+ printf_filtered (
+ _("Could not convert descriptions for %s back to xml (%p %p)\n"),
+ e.first, t_trans, t_trans2);
+ failed++;
+ }
+ else if (*tdesc != *t_trans || *tdesc != *t_trans2)
+ {
+ printf_filtered
+ (_("Translated descriptions for %s do not match (%d %d)\n"),
+ e.first, *tdesc == *t_trans, *tdesc == *t_trans2);
+ failed++;
+ }
}
printf_filtered (_("Tested %lu XML files, %d failed\n"),
(long) selftests::xml_tdesc.size (), failed);
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 9190d5f3c6..f793f07c96 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);
+}
diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h
index 8f0679707a..fee60e86dd 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 */
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 04/10] Commonise tdesc_feature
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
2018-03-22 8:45 ` [PATCH v4 01/10] Move tdesc header funcs to c file alan.hayward
2018-03-22 8:45 ` [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-22 8:45 ` [PATCH v4 05/10] Commonise tdesc types alan.hayward
` (7 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
This patch commonises tdesc_feature and makes use of it in gdbserver tdesc.
Previously in gdbserver, target_desc was interchangeable with tdesc_feature.
Now gdbserver target_desc contains a vector of tdesc_feature.
I am now able to commonise tdesc_create_reg (wanted to do this in the
previous patch).
I also had to commonise tdesc_type. This is ok, because I will need them
in the next patch.
Identical to V3 except for removal of "struct type;" at the top of
common/tdesc.h
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/
* common/tdesc.c (tdesc_feature::accept): Move to here.
(tdesc_feature::operator==): Likewise.
(tdesc_create_reg): Likewise.
* common/tdesc.h (tdesc_type_kind): Likewise.
(struct tdesc_type): Likewise.
(struct tdesc_feature): Likewise.
* regformats/regdat.sh: Create a feature.
* target-descriptions.c (tdesc_type_kind): Move from here.
(tdesc_type): Likewise.
(tdesc_type_up): Likewise.
(tdesc_feature): Likewise.
(tdesc_create_reg): Likewise.
gdbserver/
* tdesc.c (~target_desc): Remove implictly deleted items.
(init_target_desc): Iterate all features.
(tdesc_get_features_xml): Use vector.
(tdesc_create_feature): Create feature.
* tdesc.h (tdesc_feature) Remove
(target_desc): Add features.
---
gdb/common/tdesc.c | 58 ++++++++++++++++++
gdb/common/tdesc.h | 93 +++++++++++++++++++++++++++++
gdb/gdbserver/tdesc.c | 89 ++++++++++------------------
gdb/gdbserver/tdesc.h | 14 ++---
gdb/regformats/regdat.sh | 4 +-
gdb/target-descriptions.c | 148 ----------------------------------------------
6 files changed, 189 insertions(+), 217 deletions(-)
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index a5f2de4b5b..ef6ed944df 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -38,3 +38,61 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
have easy access to the containing feature when we want it later. */
tdesc_type = tdesc_named_type (feature, type.c_str ());
}
+
+void tdesc_feature::accept (tdesc_element_visitor &v) const
+{
+ v.visit_pre (this);
+
+ for (const tdesc_type_up &type : types)
+ type->accept (v);
+
+ for (const tdesc_reg_up ® : 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 common/tdesc.h. */
+
+void
+tdesc_create_reg (struct tdesc_feature *feature, const char *name,
+ int regnum, int save_restore, const char *group,
+ int bitsize, const char *type)
+{
+ tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+ group, bitsize, type);
+
+ feature->registers.emplace_back (reg);
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index ac6dfdf7bb..58d7126c7b 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -137,6 +137,99 @@ struct tdesc_reg : tdesc_element
typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
+enum tdesc_type_kind
+{
+ /* Predefined types. */
+ TDESC_TYPE_BOOL,
+ TDESC_TYPE_INT8,
+ TDESC_TYPE_INT16,
+ TDESC_TYPE_INT32,
+ TDESC_TYPE_INT64,
+ TDESC_TYPE_INT128,
+ TDESC_TYPE_UINT8,
+ TDESC_TYPE_UINT16,
+ TDESC_TYPE_UINT32,
+ TDESC_TYPE_UINT64,
+ TDESC_TYPE_UINT128,
+ TDESC_TYPE_CODE_PTR,
+ TDESC_TYPE_DATA_PTR,
+ TDESC_TYPE_IEEE_SINGLE,
+ TDESC_TYPE_IEEE_DOUBLE,
+ TDESC_TYPE_ARM_FPA_EXT,
+ TDESC_TYPE_I387_EXT,
+
+ /* Types defined by a target feature. */
+ TDESC_TYPE_VECTOR,
+ TDESC_TYPE_STRUCT,
+ TDESC_TYPE_UNION,
+ TDESC_TYPE_FLAGS,
+ TDESC_TYPE_ENUM
+};
+
+struct tdesc_type : tdesc_element
+{
+ tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
+ : name (name_), kind (kind_)
+ {}
+
+ virtual ~tdesc_type () = default;
+
+ DISABLE_COPY_AND_ASSIGN (tdesc_type);
+
+ /* The name of this type. */
+ std::string name;
+
+ /* Identify the kind of this type. */
+ enum tdesc_type_kind kind;
+
+ bool operator== (const tdesc_type &other) const
+ {
+ return name == other.name && kind == other.kind;
+ }
+
+ bool operator!= (const tdesc_type &other) const
+ {
+ return !(*this == other);
+ }
+};
+
+typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+
+/* A feature from a target description. Each feature is a collection
+ of other elements, e.g. registers and types. */
+
+struct tdesc_feature : tdesc_element
+{
+ tdesc_feature (const std::string &name_)
+ : name (name_)
+ {}
+
+ virtual ~tdesc_feature () = default;
+
+ DISABLE_COPY_AND_ASSIGN (tdesc_feature);
+
+ /* The name of this feature. It may be recognized by the architecture
+ support code. */
+ std::string name;
+
+ /* The registers associated with this feature. */
+ std::vector<tdesc_reg_up> registers;
+
+ /* The types associated with this feature. */
+ std::vector<tdesc_type_up> types;
+
+ void accept (tdesc_element_visitor &v) const override;
+
+ bool operator== (const tdesc_feature &other) const;
+
+ bool operator!= (const tdesc_feature &other) const
+ {
+ return !(*this == other);
+ }
+};
+
+typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
+
/* Allocate a new target_desc. */
target_desc *allocate_target_description (void);
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index dbf07b2c8d..da1dbdf651 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -23,16 +23,8 @@
target_desc::~target_desc ()
{
- int i;
-
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
@@ -70,35 +62,36 @@ 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)
- {
- int current_size = tdesc->reg_defs.size ();
+ for (const tdesc_feature_up &feature : tdesc->features)
+ for (const tdesc_reg_up &treg : feature->registers)
+ {
+ int current_size = tdesc->reg_defs.size ();
- /* Register number will either increase (possibly with gaps) or be
- zero. */
- gdb_assert (treg->target_regnum == 0
- || treg->target_regnum >= current_size);
+ /* Register number will either increase (possibly with gaps) or be
+ zero. */
+ gdb_assert (treg->target_regnum == 0
+ || treg->target_regnum >= current_size);
- tdesc->reg_defs.resize (treg->target_regnum != 0
+ tdesc->reg_defs.resize (treg->target_regnum != 0
? treg->target_regnum + 1
: current_size + 1);
- /* Fill in any blank spaces. */
- while (current_size < treg->target_regnum)
- {
- struct reg *reg = &tdesc->reg_defs[current_size];
- reg->name = "";
- reg->size = 0;
- reg->offset = offset;
- current_size++;
- }
-
- struct reg *reg = &tdesc->reg_defs.back ();
- reg->name = treg->name.c_str ();
- reg->size = treg->bitsize;
- reg->offset = offset;
- offset += reg->size;
- }
+ /* Fill in any blank spaces. */
+ while (current_size < treg->target_regnum)
+ {
+ struct reg *reg = &tdesc->reg_defs[current_size];
+ reg->name = "";
+ reg->size = 0;
+ reg->offset = offset;
+ current_size++;
+ }
+
+ struct reg *reg = &tdesc->reg_defs.back ();
+ reg->name = treg->name.c_str ();
+ reg->size = treg->bitsize;
+ reg->offset = offset;
+ offset += reg->size;
+ }
tdesc->registers_size = offset / 8;
@@ -161,7 +154,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)
@@ -181,12 +174,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 += "\"/>";
}
@@ -199,19 +190,16 @@ tdesc_get_features_xml (target_desc *tdesc)
}
#endif
-struct tdesc_type
-{};
-
/* See common/tdesc.h. */
struct tdesc_feature *
tdesc_create_feature (struct target_desc *tdesc, const char *name,
const char *xml)
{
-#ifndef IN_PROCESS_AGENT
- VEC_safe_push (char_ptr, tdesc->features, xstrdup (xml));
-#endif
- return tdesc;
+ struct tdesc_feature *new_feature = new tdesc_feature
+ (xml != nullptr ? xml : name);
+ tdesc->features.emplace_back (new_feature);
+ return new_feature;
}
/* See common/tdesc.h. */
@@ -256,19 +244,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id)
/* See common/tdesc.h. */
-void
-tdesc_create_reg (struct tdesc_feature *feature, const char *name,
- int regnum, int save_restore, const char *group,
- int bitsize, const char *type)
-{
- tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
- group, bitsize, type);
-
- tdesc->registers.emplace_back (reg);
-}
-
-/* See common/tdesc.h. */
-
struct tdesc_type *
tdesc_create_vector (struct tdesc_feature *feature, const char *name,
struct tdesc_type *field_type, int count)
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index c64ce13b25..197fb59127 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/regformats/regdat.sh b/gdb/regformats/regdat.sh
index ce1627120d..8c6e191596 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 3186bf886f..a453eddec0 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -67,64 +67,6 @@ struct tdesc_type_field
int start, end;
};
-enum tdesc_type_kind
-{
- /* Predefined types. */
- TDESC_TYPE_BOOL,
- TDESC_TYPE_INT8,
- TDESC_TYPE_INT16,
- TDESC_TYPE_INT32,
- TDESC_TYPE_INT64,
- TDESC_TYPE_INT128,
- TDESC_TYPE_UINT8,
- TDESC_TYPE_UINT16,
- TDESC_TYPE_UINT32,
- TDESC_TYPE_UINT64,
- TDESC_TYPE_UINT128,
- TDESC_TYPE_CODE_PTR,
- TDESC_TYPE_DATA_PTR,
- TDESC_TYPE_IEEE_SINGLE,
- TDESC_TYPE_IEEE_DOUBLE,
- TDESC_TYPE_ARM_FPA_EXT,
- TDESC_TYPE_I387_EXT,
-
- /* Types defined by a target feature. */
- TDESC_TYPE_VECTOR,
- TDESC_TYPE_STRUCT,
- TDESC_TYPE_UNION,
- TDESC_TYPE_FLAGS,
- TDESC_TYPE_ENUM
-};
-
-struct tdesc_type : tdesc_element
-{
- tdesc_type (const std::string &name_, enum tdesc_type_kind kind_)
- : name (name_), kind (kind_)
- {}
-
- virtual ~tdesc_type () = default;
-
- DISABLE_COPY_AND_ASSIGN (tdesc_type);
-
- /* The name of this type. */
- std::string name;
-
- /* Identify the kind of this type. */
- enum tdesc_type_kind kind;
-
- bool operator== (const tdesc_type &other) const
- {
- return name == other.name && kind == other.kind;
- }
-
- bool operator!= (const tdesc_type &other) const
- {
- return !(*this == other);
- }
-};
-
-typedef std::unique_ptr<tdesc_type> tdesc_type_up;
-
struct tdesc_type_builtin : tdesc_type
{
tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
@@ -428,82 +370,6 @@ make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype)
return gdb_type.get_type ();
}
-/* A feature from a target description. Each feature is a collection
- of other elements, e.g. registers and types. */
-
-struct tdesc_feature : tdesc_element
-{
- tdesc_feature (const std::string &name_)
- : name (name_)
- {}
-
- virtual ~tdesc_feature () = default;
-
- DISABLE_COPY_AND_ASSIGN (tdesc_feature);
-
- /* The name of this feature. It may be recognized by the architecture
- support code. */
- std::string name;
-
- /* The registers associated with this feature. */
- std::vector<tdesc_reg_up> registers;
-
- /* The types associated with this feature. */
- std::vector<tdesc_type_up> types;
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit_pre (this);
-
- for (const tdesc_type_up &type : types)
- type->accept (v);
-
- for (const tdesc_reg_up ® : 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
@@ -1358,20 +1224,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
tdesc_remote_register_number);
set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p);
}
-\f
-
-/* See common/tdesc.h. */
-
-void
-tdesc_create_reg (struct tdesc_feature *feature, const char *name,
- int regnum, int save_restore, const char *group,
- int bitsize, const char *type)
-{
- tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
- group, bitsize, type);
-
- feature->registers.emplace_back (reg);
-}
/* See common/tdesc.h. */
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 03/10] Commonise tdesc_reg
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (6 preceding siblings ...)
2018-03-22 8:45 ` [PATCH v4 07/10] Add feature reference in .dat files alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-23 19:50 ` Simon Marchi
2018-03-22 8:46 ` [PATCH v4 09/10] Remove xml file references from target descriptions alan.hayward
` (2 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.
However, other modules in gdbserver directly access the reg_defs structure.
To work around this, init_target_desc fills in reg_defs by parsing the
tdesc_reg vector.
The long term goal is to remove reg_defs, replacing with accessor funcs.
I wanted to make tdesc_create_reg common, but I cannot do that until
the next patch.
I also had to commonise tdesc_element_visitor and tdesc_element.
This patch only differs from the V3 version in init_target_desc(),
where I now use reg_def references as introduced in the previous patch.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/
* Makefile.in: Add arch/tdesc.c
* common/tdesc.c: New file.
* common/tdesc.h (tdesc_element_visitor): Move to here.
(tdesc_element): Likewise.
(tdesc_reg): Likewise.
(tdesc_reg_up): Likewise.
* target-descriptions.c: (tdesc_element_visitor): Move from here.
(tdesc_element): Likewise.
(tdesc_reg): Likewise.
(tdesc_reg_up): Likewise.
gdbserver/
* Makefile.in: Add common/tdesc.c
* tdesc.c (init_target_desc): init all reg_defs from register vector.
(tdesc_create_reg): Create tdesc_reg.
* tdesc.h (tdesc_feature): Add register vector.
---
gdb/Makefile.in | 2 +
gdb/common/tdesc.c | 40 ++++++++++++++++
gdb/common/tdesc.h | 111 ++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/Makefile.in | 3 ++
gdb/gdbserver/tdesc.c | 53 ++++++++++++---------
gdb/gdbserver/tdesc.h | 5 +-
gdb/target-descriptions.c | 116 ----------------------------------------------
7 files changed, 191 insertions(+), 139 deletions(-)
create mode 100644 gdb/common/tdesc.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 44db1937b6..f1d2f0193a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -966,6 +966,7 @@ COMMON_SFILES = \
common/run-time-clock.c \
common/signals.c \
common/signals-state-save-restore.c \
+ common/tdesc.c \
common/vec.c \
common/xml-utils.c \
complaints.c \
@@ -1443,6 +1444,7 @@ HFILES_NO_SRCDIR = \
common/run-time-clock.h \
common/signals-state-save-restore.h \
common/symbol.h \
+ common/tdesc.h \
common/vec.h \
common/version.h \
common/x86-xstate.h \
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
new file mode 100644
index 0000000000..a5f2de4b5b
--- /dev/null
+++ b/gdb/common/tdesc.c
@@ -0,0 +1,40 @@
+/* Target description support for GDB.
+
+ Copyright (C) 2017 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include "common/tdesc.h"
+
+tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
+ int regnum, int save_restore_, const char *group_,
+ int bitsize_, const char *type_)
+ : name (name_), target_regnum (regnum),
+ save_restore (save_restore_),
+ group (group_ != NULL ? group_ : ""),
+ bitsize (bitsize_),
+ type (type_ != NULL ? type_ : "<unknown>")
+{
+ /* If the register's type is target-defined, look it up now. We may not
+ have easy access to the containing feature when we want it later. */
+ tdesc_type = tdesc_named_type (feature, type.c_str ());
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index cc11651dca..ac6dfdf7bb 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -18,6 +18,12 @@
#ifndef ARCH_TDESC_H
#define ARCH_TDESC_H 1
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
struct tdesc_feature;
struct tdesc_type;
struct tdesc_type_builtin;
@@ -26,6 +32,111 @@ struct tdesc_type_with_fields;
struct tdesc_reg;
struct target_desc;
+/* The interface to visit different elements of target description. */
+
+class tdesc_element_visitor
+{
+public:
+ virtual void visit_pre (const target_desc *e)
+ {}
+
+ virtual void visit_post (const target_desc *e)
+ {}
+
+ virtual void visit_pre (const tdesc_feature *e)
+ {}
+
+ virtual void visit_post (const tdesc_feature *e)
+ {}
+
+ virtual void visit (const tdesc_type_builtin *e)
+ {}
+
+ virtual void visit (const tdesc_type_vector *e)
+ {}
+
+ virtual void visit (const tdesc_type_with_fields *e)
+ {}
+
+ virtual void visit (const tdesc_reg *e)
+ {}
+};
+
+class tdesc_element
+{
+public:
+ virtual void accept (tdesc_element_visitor &v) const = 0;
+};
+
+/* An individual register from a target description. */
+
+struct tdesc_reg : tdesc_element
+{
+ tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
+ int regnum, int save_restore_, const char *group_,
+ int bitsize_, const char *type_);
+
+ virtual ~tdesc_reg () = default;
+
+ DISABLE_COPY_AND_ASSIGN (tdesc_reg);
+
+ /* The name of this register. In standard features, it may be
+ recognized by the architecture support code, or it may be purely
+ for the user. */
+ std::string name;
+
+ /* The register number used by this target to refer to this
+ register. This is used for remote p/P packets and to determine
+ the ordering of registers in the remote g/G packets. */
+ long target_regnum;
+
+ /* If this flag is set, GDB should save and restore this register
+ around calls to an inferior function. */
+ int save_restore;
+
+ /* The name of the register group containing this register, or empty
+ if the group should be automatically determined from the
+ register's type. If this is "general", "float", or "vector", the
+ corresponding "info" command should display this register's
+ value. It can be an arbitrary string, but should be limited to
+ alphanumeric characters and internal hyphens. Currently other
+ strings are ignored (treated as empty). */
+ std::string group;
+
+ /* The size of the register, in bits. */
+ int bitsize;
+
+ /* The type of the register. This string corresponds to either
+ a named type from the target description or a predefined
+ type from GDB. */
+ std::string type;
+
+ /* The target-described type corresponding to TYPE, if found. */
+ struct tdesc_type *tdesc_type;
+
+ void accept (tdesc_element_visitor &v) const override
+ {
+ v.visit (this);
+ }
+
+ bool operator== (const tdesc_reg &other) const
+ {
+ return (name == other.name
+ && target_regnum == other.target_regnum
+ && save_restore == other.save_restore
+ && bitsize == other.bitsize
+ && group == other.group
+ && type == other.type);
+ }
+
+ bool operator!= (const tdesc_reg &other) const
+ {
+ return !(*this == other);
+ }
+};
+
+typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
+
/* Allocate a new target_desc. */
target_desc *allocate_target_description (void);
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 75fbf7ec75..4a54235dae 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -215,6 +215,7 @@ SFILES = \
$(srcdir)/common/print-utils.c \
$(srcdir)/common/ptid.c \
$(srcdir)/common/rsp-low.c \
+ $(srcdir)/common/tdesc.c \
$(srcdir)/common/vec.c \
$(srcdir)/common/xml-utils.c \
$(srcdir)/nat/linux-btrace.c \
@@ -258,6 +259,7 @@ OBS = \
common/rsp-low.o \
common/signals.o \
common/signals-state-save-restore.o \
+ common/tdesc.o \
common/vec.o \
common/xml-utils.o \
debug.o \
@@ -403,6 +405,7 @@ IPA_OBJS = \
common/format-ipa.o \
common/print-utils-ipa.o \
common/rsp-low-ipa.o \
+ common/tdesc-ipa.o \
common/vec-ipa.o \
regcache-ipa.o \
remote-utils-ipa.o \
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 8e68a27c7c..dbf07b2c8d 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -69,10 +69,35 @@ init_target_desc (struct target_desc *tdesc)
{
int offset = 0;
- for (reg ® : tdesc->reg_defs)
+ /* Go through all the features and populate reg_defs. */
+ for (const tdesc_reg_up &treg : tdesc->registers)
{
- reg.offset = offset;
- offset += reg.size;
+ int current_size = tdesc->reg_defs.size ();
+
+ /* Register number will either increase (possibly with gaps) or be
+ zero. */
+ gdb_assert (treg->target_regnum == 0
+ || treg->target_regnum >= current_size);
+
+ tdesc->reg_defs.resize (treg->target_regnum != 0
+ ? treg->target_regnum + 1
+ : current_size + 1);
+
+ /* Fill in any blank spaces. */
+ while (current_size < treg->target_regnum)
+ {
+ struct reg *reg = &tdesc->reg_defs[current_size];
+ reg->name = "";
+ reg->size = 0;
+ reg->offset = offset;
+ current_size++;
+ }
+
+ struct reg *reg = &tdesc->reg_defs.back ();
+ reg->name = treg->name.c_str ();
+ reg->size = treg->bitsize;
+ reg->offset = offset;
+ offset += reg->size;
}
tdesc->registers_size = offset / 8;
@@ -236,26 +261,10 @@ 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;
- int current_size = tdesc->reg_defs.size ();
-
- tdesc->reg_defs.resize (regnum != 0 ? regnum + 1 : current_size + 1);
-
- while (current_size < regnum)
- {
- struct reg *reg = &tdesc->reg_defs[current_size];
- reg->name = "";
- reg->size = 0;
- current_size++;
- }
-
- gdb_assert (regnum == 0
- || regnum + 1 == tdesc->reg_defs.size ());
-
- struct reg *reg = &tdesc->reg_defs.back ();
+ tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+ group, bitsize, type);
- reg->name = name;
- reg->size = bitsize;
+ tdesc->registers.emplace_back (reg);
}
/* See common/tdesc.h. */
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index a62544341c..c64ce13b25 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/target-descriptions.c b/gdb/target-descriptions.c
index 5d34e29a86..3186bf886f 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -38,44 +38,6 @@
#include "completer.h"
#include "readline/tilde.h" /* tilde_expand */
-static type *make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype);
-
-/* The interface to visit different elements of target description. */
-
-class tdesc_element_visitor
-{
-public:
- virtual void visit_pre (const target_desc *e)
- {}
-
- virtual void visit_post (const target_desc *e)
- {}
-
- virtual void visit_pre (const tdesc_feature *e)
- {}
-
- virtual void visit_post (const tdesc_feature *e)
- {}
-
- virtual void visit (const tdesc_type_builtin *e)
- {}
-
- virtual void visit (const tdesc_type_vector *e)
- {}
-
- virtual void visit (const tdesc_type_with_fields *e)
- {}
-
- virtual void visit (const tdesc_reg *e)
- {}
-};
-
-class tdesc_element
-{
-public:
- virtual void accept (tdesc_element_visitor &v) const = 0;
-};
-
/* Types. */
struct property
@@ -88,84 +50,6 @@ struct property
std::string value;
};
-/* An individual register from a target description. */
-
-struct tdesc_reg : tdesc_element
-{
- tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
- int regnum, int save_restore_, const char *group_,
- int bitsize_, const char *type_)
- : name (name_), target_regnum (regnum),
- save_restore (save_restore_),
- group (group_ != NULL ? group_ : ""),
- bitsize (bitsize_),
- type (type_ != NULL ? type_ : "<unknown>")
- {
- /* If the register's type is target-defined, look it up now. We may not
- have easy access to the containing feature when we want it later. */
- tdesc_type = tdesc_named_type (feature, type.c_str ());
- }
-
- virtual ~tdesc_reg () = default;
-
- DISABLE_COPY_AND_ASSIGN (tdesc_reg);
-
- /* The name of this register. In standard features, it may be
- recognized by the architecture support code, or it may be purely
- for the user. */
- std::string name;
-
- /* The register number used by this target to refer to this
- register. This is used for remote p/P packets and to determine
- the ordering of registers in the remote g/G packets. */
- long target_regnum;
-
- /* If this flag is set, GDB should save and restore this register
- around calls to an inferior function. */
- int save_restore;
-
- /* The name of the register group containing this register, or empty
- if the group should be automatically determined from the register's
- type. This is traditionally "general", "float", "vector" but can
- also be an arbitrary string. If defined the corresponding "info"
- command should display this register's value. The string should be
- limited to alphanumeric characters and internal hyphens. */
- std::string group;
-
- /* The size of the register, in bits. */
- int bitsize;
-
- /* The type of the register. This string corresponds to either
- a named type from the target description or a predefined
- type from GDB. */
- std::string type;
-
- /* The target-described type corresponding to TYPE, if found. */
- struct tdesc_type *tdesc_type;
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-
- bool operator== (const tdesc_reg &other) const
- {
- return (name == other.name
- && target_regnum == other.target_regnum
- && save_restore == other.save_restore
- && bitsize == other.bitsize
- && group == other.group
- && type == other.type);
- }
-
- bool operator!= (const tdesc_reg &other) const
- {
- return !(*this == other);
- }
-};
-
-typedef std::unique_ptr<tdesc_reg> tdesc_reg_up;
-
/* A named type from a target description. */
struct tdesc_type_field
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 05/10] Commonise tdesc types
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (2 preceding siblings ...)
2018-03-22 8:45 ` [PATCH v4 04/10] Commonise tdesc_feature alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-23 20:12 ` Simon Marchi
2018-03-22 8:45 ` [PATCH v4 08/10] Create xml from target descriptions alan.hayward
` (6 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
This patch moves all the various tdesc types to common code.
It also commonises all the tdesc_ functions that are stubbed
out in gdbserver.
This is a lot of code to move across, but it is the simplest
way of getting gdbserver to retain the target description
information.
With this patch, gdb and gdbserver will now parse a target
description in the same way.
Identical to V3 version.
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/
* common/tdesc.c (tdesc_predefined_type): Move to here.
(tdesc_named_type): Likewise.
(tdesc_create_vector): Likewise.
(tdesc_create_struct): Likewise.
(tdesc_set_struct_size): Likewise.
(tdesc_create_union): Likewise.
(tdesc_create_flags): Likewise.
(tdesc_create_enum): Likewise.
(tdesc_add_field): Likewise.
(tdesc_add_typed_bitfield): Likewise.
(tdesc_add_bitfield): Likewise.
(tdesc_add_flag): Likewise.
(tdesc_add_enum_value): Likewise.
* common/tdesc.h (tdesc_type_builtin): Likewise.
(tdesc_type_vector): Likewise.
(tdesc_type_field): Likewise.
(tdesc_type_with_fields): Likewise.
* target-descriptions.c (tdesc_type_field): Move from here.
(tdesc_type_builtin): Likewise.
(tdesc_type_vector): Likewise.
(tdesc_type_with_fields): Likewise.
(tdesc_predefined_types): Likewise.
(tdesc_named_type): Likewise.
(tdesc_create_vector): Likewise.
(tdesc_create_struct): Likewise.
(tdesc_set_struct_size): Likewise.
(tdesc_create_union): Likewise.
(tdesc_create_flags): Likewise.
(tdesc_create_enum): Likewise.
(tdesc_add_field): Likewise.
(tdesc_add_typed_bitfield): Likewise.
(tdesc_add_bitfield): Likewise.
(tdesc_add_flag): Likewise.
(tdesc_add_enum_value): Likewise.
gdbserver/
* tdesc.c (tdesc_create_flags): Remove.
(tdesc_add_flag): Likewise.
(tdesc_named_type): Likewise.
(tdesc_create_union): Likewise.
(tdesc_create_struct): Likewise.
(tdesc_create_vector): Likewise.
(tdesc_add_bitfield): Likewise.
(tdesc_add_field): Likewise.
(tdesc_set_struct_size): Likewise.
---
gdb/common/tdesc.c | 193 ++++++++++++++++++++++++++++++++++
gdb/common/tdesc.h | 66 ++++++++++++
gdb/gdbserver/tdesc.c | 68 ------------
gdb/target-descriptions.c | 258 ----------------------------------------------
4 files changed, 259 insertions(+), 326 deletions(-)
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index ef6ed944df..a0ed7c5115 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -39,6 +39,28 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_,
tdesc_type = tdesc_named_type (feature, type.c_str ());
}
+/* Predefined types. */
+static tdesc_type_builtin tdesc_predefined_types[] =
+{
+ { "bool", TDESC_TYPE_BOOL },
+ { "int8", TDESC_TYPE_INT8 },
+ { "int16", TDESC_TYPE_INT16 },
+ { "int32", TDESC_TYPE_INT32 },
+ { "int64", TDESC_TYPE_INT64 },
+ { "int128", TDESC_TYPE_INT128 },
+ { "uint8", TDESC_TYPE_UINT8 },
+ { "uint16", TDESC_TYPE_UINT16 },
+ { "uint32", TDESC_TYPE_UINT32 },
+ { "uint64", TDESC_TYPE_UINT64 },
+ { "uint128", TDESC_TYPE_UINT128 },
+ { "code_ptr", TDESC_TYPE_CODE_PTR },
+ { "data_ptr", TDESC_TYPE_DATA_PTR },
+ { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
+ { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
+ { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
+ { "i387_ext", TDESC_TYPE_I387_EXT }
+};
+
void tdesc_feature::accept (tdesc_element_visitor &v) const
{
v.visit_pre (this);
@@ -84,6 +106,36 @@ bool tdesc_feature::operator== (const tdesc_feature &other) const
return true;
}
+/* Lookup a predefined type. */
+
+static struct tdesc_type *
+tdesc_predefined_type (enum tdesc_type_kind kind)
+{
+ for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+ if (tdesc_predefined_types[ix].kind == kind)
+ return &tdesc_predefined_types[ix];
+
+ gdb_assert_not_reached ("bad predefined tdesc type");
+}
+
+/* See common/tdesc.h. */
+
+struct tdesc_type *
+tdesc_named_type (const struct tdesc_feature *feature, const char *id)
+{
+ /* First try target-defined types. */
+ for (const tdesc_type_up &type : feature->types)
+ if (type->name == id)
+ return type.get ();
+
+ /* Next try the predefined types. */
+ for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
+ if (tdesc_predefined_types[ix].name == id)
+ return &tdesc_predefined_types[ix];
+
+ return NULL;
+}
+
/* See common/tdesc.h. */
void
@@ -96,3 +148,144 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
feature->registers.emplace_back (reg);
}
+
+/* See common/tdesc.h. */
+
+struct tdesc_type *
+tdesc_create_vector (struct tdesc_feature *feature, const char *name,
+ struct tdesc_type *field_type, int count)
+{
+ tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See common/tdesc.h. */
+
+tdesc_type_with_fields *
+tdesc_create_struct (struct tdesc_feature *feature, const char *name)
+{
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See common/tdesc.h. */
+
+void
+tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
+{
+ gdb_assert (type->kind == TDESC_TYPE_STRUCT);
+ gdb_assert (size > 0);
+ type->size = size;
+}
+
+/* See common/tdesc.h. */
+
+tdesc_type_with_fields *
+tdesc_create_union (struct tdesc_feature *feature, const char *name)
+{
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See common/tdesc.h. */
+
+tdesc_type_with_fields *
+tdesc_create_flags (struct tdesc_feature *feature, const char *name,
+ int size)
+{
+ gdb_assert (size > 0);
+
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+tdesc_type_with_fields *
+tdesc_create_enum (struct tdesc_feature *feature, const char *name,
+ int size)
+{
+ gdb_assert (size > 0);
+
+ tdesc_type_with_fields *type
+ = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
+ feature->types.emplace_back (type);
+
+ return type;
+}
+
+/* See common/tdesc.h. */
+
+void
+tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
+ struct tdesc_type *field_type)
+{
+ gdb_assert (type->kind == TDESC_TYPE_UNION
+ || type->kind == TDESC_TYPE_STRUCT);
+
+ /* Initialize start and end so we know this is not a bit-field
+ when we print-c-tdesc. */
+ type->fields.emplace_back (field_name, field_type, -1, -1);
+}
+
+void
+tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
+ int start, int end, struct tdesc_type *field_type)
+{
+ gdb_assert (type->kind == TDESC_TYPE_STRUCT
+ || type->kind == TDESC_TYPE_FLAGS);
+ gdb_assert (start >= 0 && end >= start);
+
+ type->fields.emplace_back (field_name, field_type, start, end);
+}
+
+/* See common/tdesc.h. */
+
+void
+tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
+ int start, int end)
+{
+ struct tdesc_type *field_type;
+
+ gdb_assert (start >= 0 && end >= start);
+
+ if (type->size > 4)
+ field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
+ else
+ field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
+
+ tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
+}
+
+/* See common/tdesc.h. */
+
+void
+tdesc_add_flag (tdesc_type_with_fields *type, int start,
+ const char *flag_name)
+{
+ gdb_assert (type->kind == TDESC_TYPE_FLAGS
+ || type->kind == TDESC_TYPE_STRUCT);
+
+ type->fields.emplace_back (flag_name,
+ tdesc_predefined_type (TDESC_TYPE_BOOL),
+ start, start);
+}
+
+void
+tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
+ const char *name)
+{
+ gdb_assert (type->kind == TDESC_TYPE_ENUM);
+ type->fields.emplace_back (name,
+ tdesc_predefined_type (TDESC_TYPE_INT32),
+ value, -1);
+}
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 58d7126c7b..986625e685 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -195,6 +195,72 @@ struct tdesc_type : tdesc_element
typedef std::unique_ptr<tdesc_type> tdesc_type_up;
+struct tdesc_type_builtin : tdesc_type
+{
+ tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
+ : tdesc_type (name, kind)
+ {}
+
+ void accept (tdesc_element_visitor &v) const override
+ {
+ v.visit (this);
+ }
+};
+
+/* tdesc_type for vector types. */
+
+struct tdesc_type_vector : tdesc_type
+{
+ tdesc_type_vector (const std::string &name, tdesc_type *element_type_,
+ int count_)
+ : tdesc_type (name, TDESC_TYPE_VECTOR),
+ element_type (element_type_), count (count_)
+ {}
+
+ void accept (tdesc_element_visitor &v) const override
+ {
+ v.visit (this);
+ }
+
+ struct tdesc_type *element_type;
+ int count;
+};
+
+/* A named type from a target description. */
+
+struct tdesc_type_field
+{
+ tdesc_type_field (const std::string &name_, tdesc_type *type_,
+ int start_, int end_)
+ : name (name_), type (type_), start (start_), end (end_)
+ {}
+
+ std::string name;
+ struct tdesc_type *type;
+ /* For non-enum-values, either both are -1 (non-bitfield), or both are
+ not -1 (bitfield). For enum values, start is the value (which could be
+ -1), end is -1. */
+ int start, end;
+};
+
+/* tdesc_type for struct, union, flags, and enum types. */
+
+struct tdesc_type_with_fields : tdesc_type
+{
+ tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
+ int size_ = 0)
+ : tdesc_type (name, kind), size (size_)
+ {}
+
+ void accept (tdesc_element_visitor &v) const override
+ {
+ v.visit (this);
+ }
+
+ std::vector<tdesc_type_field> fields;
+ int size;
+};
+
/* A feature from a target description. Each feature is a collection
of other elements, e.g. registers and types. */
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index da1dbdf651..1f30a32431 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -201,71 +201,3 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name,
tdesc->features.emplace_back (new_feature);
return new_feature;
}
-
-/* See common/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
- int size)
-{
- return NULL;
-}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
- const char *flag_name)
-{}
-
-/* See common/tdesc.h. */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
- return NULL;
-}
-
-/* See common/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *id)
-{
- return NULL;
-}
-
-/* See common/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *id)
-{
- return NULL;
-}
-
-/* See common/tdesc.h. */
-
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
- struct tdesc_type *field_type, int count)
-{
- return NULL;
-}
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
- int start, int end)
-{}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
- struct tdesc_type *field_type)
-{}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
-}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index a453eddec0..2782ffaab9 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -50,71 +50,6 @@ struct property
std::string value;
};
-/* A named type from a target description. */
-
-struct tdesc_type_field
-{
- tdesc_type_field (const std::string &name_, tdesc_type *type_,
- int start_, int end_)
- : name (name_), type (type_), start (start_), end (end_)
- {}
-
- std::string name;
- struct tdesc_type *type;
- /* For non-enum-values, either both are -1 (non-bitfield), or both are
- not -1 (bitfield). For enum values, start is the value (which could be
- -1), end is -1. */
- int start, end;
-};
-
-struct tdesc_type_builtin : tdesc_type
-{
- tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind)
- : tdesc_type (name, kind)
- {}
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-};
-
-/* tdesc_type for vector types. */
-
-struct tdesc_type_vector : tdesc_type
-{
- tdesc_type_vector (const std::string &name, tdesc_type *element_type_, int count_)
- : tdesc_type (name, TDESC_TYPE_VECTOR),
- element_type (element_type_), count (count_)
- {}
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-
- struct tdesc_type *element_type;
- int count;
-};
-
-/* tdesc_type for struct, union, flags, and enum types. */
-
-struct tdesc_type_with_fields : tdesc_type
-{
- tdesc_type_with_fields (const std::string &name, tdesc_type_kind kind,
- int size_ = 0)
- : tdesc_type (name, kind), size (size_)
- {}
-
- void accept (tdesc_element_visitor &v) const override
- {
- v.visit (this);
- }
-
- std::vector<tdesc_type_field> fields;
- int size;
-};
-
/* Convert a tdesc_type to a gdb type. */
static type *
@@ -741,58 +676,6 @@ tdesc_feature_name (const struct tdesc_feature *feature)
return feature->name.c_str ();
}
-/* Predefined types. */
-static tdesc_type_builtin tdesc_predefined_types[] =
-{
- { "bool", TDESC_TYPE_BOOL },
- { "int8", TDESC_TYPE_INT8 },
- { "int16", TDESC_TYPE_INT16 },
- { "int32", TDESC_TYPE_INT32 },
- { "int64", TDESC_TYPE_INT64 },
- { "int128", TDESC_TYPE_INT128 },
- { "uint8", TDESC_TYPE_UINT8 },
- { "uint16", TDESC_TYPE_UINT16 },
- { "uint32", TDESC_TYPE_UINT32 },
- { "uint64", TDESC_TYPE_UINT64 },
- { "uint128", TDESC_TYPE_UINT128 },
- { "code_ptr", TDESC_TYPE_CODE_PTR },
- { "data_ptr", TDESC_TYPE_DATA_PTR },
- { "ieee_single", TDESC_TYPE_IEEE_SINGLE },
- { "ieee_double", TDESC_TYPE_IEEE_DOUBLE },
- { "arm_fpa_ext", TDESC_TYPE_ARM_FPA_EXT },
- { "i387_ext", TDESC_TYPE_I387_EXT }
-};
-
-/* Lookup a predefined type. */
-
-static struct tdesc_type *
-tdesc_predefined_type (enum tdesc_type_kind kind)
-{
- for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
- if (tdesc_predefined_types[ix].kind == kind)
- return &tdesc_predefined_types[ix];
-
- gdb_assert_not_reached ("bad predefined tdesc type");
-}
-
-/* See common/tdesc.h. */
-
-struct tdesc_type *
-tdesc_named_type (const struct tdesc_feature *feature, const char *id)
-{
- /* First try target-defined types. */
- for (const tdesc_type_up &type : feature->types)
- if (type->name == id)
- return type.get ();
-
- /* Next try the predefined types. */
- for (int ix = 0; ix < ARRAY_SIZE (tdesc_predefined_types); ix++)
- if (tdesc_predefined_types[ix].name == id)
- return &tdesc_predefined_types[ix];
-
- return NULL;
-}
-
/* Lookup type associated with ID. */
struct type *
@@ -1227,147 +1110,6 @@ tdesc_use_registers (struct gdbarch *gdbarch,
/* See common/tdesc.h. */
-struct tdesc_type *
-tdesc_create_vector (struct tdesc_feature *feature, const char *name,
- struct tdesc_type *field_type, int count)
-{
- tdesc_type_vector *type = new tdesc_type_vector (name, field_type, count);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See common/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_struct (struct tdesc_feature *feature, const char *name)
-{
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_STRUCT);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_set_struct_size (tdesc_type_with_fields *type, int size)
-{
- gdb_assert (type->kind == TDESC_TYPE_STRUCT);
- gdb_assert (size > 0);
- type->size = size;
-}
-
-/* See common/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_union (struct tdesc_feature *feature, const char *name)
-{
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_UNION);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See common/tdesc.h. */
-
-tdesc_type_with_fields *
-tdesc_create_flags (struct tdesc_feature *feature, const char *name,
- int size)
-{
- gdb_assert (size > 0);
-
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_FLAGS, size);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-tdesc_type_with_fields *
-tdesc_create_enum (struct tdesc_feature *feature, const char *name,
- int size)
-{
- gdb_assert (size > 0);
-
- tdesc_type_with_fields *type
- = new tdesc_type_with_fields (name, TDESC_TYPE_ENUM, size);
- feature->types.emplace_back (type);
-
- return type;
-}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_add_field (tdesc_type_with_fields *type, const char *field_name,
- struct tdesc_type *field_type)
-{
- gdb_assert (type->kind == TDESC_TYPE_UNION
- || type->kind == TDESC_TYPE_STRUCT);
-
- /* Initialize start and end so we know this is not a bit-field
- when we print-c-tdesc. */
- type->fields.emplace_back (field_name, field_type, -1, -1);
-}
-
-void
-tdesc_add_typed_bitfield (tdesc_type_with_fields *type, const char *field_name,
- int start, int end, struct tdesc_type *field_type)
-{
- gdb_assert (type->kind == TDESC_TYPE_STRUCT
- || type->kind == TDESC_TYPE_FLAGS);
- gdb_assert (start >= 0 && end >= start);
-
- type->fields.emplace_back (field_name, field_type, start, end);
-}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_add_bitfield (tdesc_type_with_fields *type, const char *field_name,
- int start, int end)
-{
- struct tdesc_type *field_type;
-
- gdb_assert (start >= 0 && end >= start);
-
- if (type->size > 4)
- field_type = tdesc_predefined_type (TDESC_TYPE_UINT64);
- else
- field_type = tdesc_predefined_type (TDESC_TYPE_UINT32);
-
- tdesc_add_typed_bitfield (type, field_name, start, end, field_type);
-}
-
-/* See common/tdesc.h. */
-
-void
-tdesc_add_flag (tdesc_type_with_fields *type, int start,
- const char *flag_name)
-{
- gdb_assert (type->kind == TDESC_TYPE_FLAGS
- || type->kind == TDESC_TYPE_STRUCT);
-
- type->fields.emplace_back (flag_name,
- tdesc_predefined_type (TDESC_TYPE_BOOL),
- start, start);
-}
-
-void
-tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
- const char *name)
-{
- gdb_assert (type->kind == TDESC_TYPE_ENUM);
- type->fields.emplace_back (name,
- tdesc_predefined_type (TDESC_TYPE_INT32),
- value, -1);
-}
-
-/* See common/tdesc.h. */
-
struct tdesc_feature *
tdesc_create_feature (struct target_desc *tdesc, const char *name,
const char *xml)
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 06/10] Add tdesc osabi and architecture functions
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (4 preceding siblings ...)
2018-03-22 8:45 ` [PATCH v4 08/10] Create xml from target descriptions alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-22 8:45 ` [PATCH v4 07/10] Add feature reference in .dat files alan.hayward
` (4 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
Add functions to access to printable names for osabi and architecture in
target_desc.
I wanted to add these as member functions of target_desc, but cannot until
target_desc is moved into the header files.
Identical to V3 version.
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/
* common/tdesc.h (tdesc_architecture_name): Add new declaration.
(tdesc_osabi_name): Likewise.
* target-descriptions.c (tdesc_architecture_name): Add new function.
(tdesc_osabi_name): Likewise.
gdb/gdbserver/
* tdesc.c (tdesc_architecture_name): Add new function.
(tdesc_osabi_name): Likewise.
(tdesc_get_features_xml): Use new functions.
---
gdb/common/tdesc.h | 9 +++++++++
gdb/gdbserver/tdesc.c | 23 ++++++++++++++++++++---
gdb/target-descriptions.c | 19 ++++++++++++++++++-
3 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 986625e685..8ab77e365f 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -303,9 +303,18 @@ target_desc *allocate_target_description (void);
void set_tdesc_architecture (target_desc *target_desc,
const char *name);
+/* Return the architecture associated with this target description as a string,
+ or NULL if no architecture was specified. */
+const char *tdesc_architecture_name (const struct target_desc *target_desc);
+
/* Set TARGET_DESC's osabi by NAME. */
void set_tdesc_osabi (target_desc *target_desc, const char *name);
+/* Return the osabi associated with this target description as a string,
+ or NULL if no osabi was specified. */
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc);
+
/* Return the type associated with ID in the context of FEATURE, or
NULL if none. */
struct tdesc_type *tdesc_named_type (const struct tdesc_feature *feature,
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 1f30a32431..69bde2ae42 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -131,6 +131,14 @@ current_target_desc (void)
/* See common/tdesc.h. */
+const char *
+tdesc_architecture_name (const struct target_desc *target_desc)
+{
+ return target_desc->arch;
+}
+
+/* See common/tdesc.h. */
+
void
set_tdesc_architecture (struct target_desc *target_desc,
const char *name)
@@ -140,6 +148,14 @@ set_tdesc_architecture (struct target_desc *target_desc,
/* See common/tdesc.h. */
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc)
+{
+ return target_desc->osabi;
+}
+
+/* See common/tdesc.h. */
+
void
set_tdesc_osabi (struct target_desc *target_desc, const char *name)
{
@@ -164,13 +180,14 @@ tdesc_get_features_xml (target_desc *tdesc)
buffer += "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">";
buffer += "<target>";
buffer += "<architecture>";
- buffer += tdesc->arch;
+ buffer += tdesc_architecture_name (tdesc);
buffer += "</architecture>";
- if (tdesc->osabi != nullptr)
+ const char *osabi = tdesc_osabi_name (tdesc);
+ if (osabi != nullptr)
{
buffer += "<osabi>";
- buffer += tdesc->osabi;
+ buffer += osabi;
buffer += "</osabi>";
}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 2782ffaab9..da2c1ce345 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -628,6 +628,14 @@ tdesc_architecture (const struct target_desc *target_desc)
return target_desc->arch;
}
+/* See common/tdesc.h. */
+
+const char *
+tdesc_architecture_name (const struct target_desc *target_desc)
+{
+ return target_desc->arch->printable_name;
+}
+
/* Return the OSABI associated with this target description, or
GDB_OSABI_UNKNOWN if no osabi was specified. */
@@ -637,7 +645,16 @@ tdesc_osabi (const struct target_desc *target_desc)
return target_desc->osabi;
}
-\f
+/* See common/tdesc.h. */
+
+const char *
+tdesc_osabi_name (const struct target_desc *target_desc)
+{
+ enum gdb_osabi osabi = tdesc_osabi (target_desc);
+ if (osabi > GDB_OSABI_UNKNOWN && osabi < GDB_OSABI_INVALID)
+ return gdbarch_osabi_name (osabi);
+ return nullptr;
+}
/* Return 1 if this target description includes any registers. */
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 07/10] Add feature reference in .dat files
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (5 preceding siblings ...)
2018-03-22 8:45 ` [PATCH v4 06/10] Add tdesc osabi and architecture functions alan.hayward
@ 2018-03-22 8:45 ` alan.hayward
2018-03-22 8:45 ` [PATCH v4 03/10] Commonise tdesc_reg alan.hayward
` (3 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:45 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
For all targets which use the newer style target descriptions, add a
"feature" marker in the dat files.
Update regdat.sh to parse feature, but do not use it (yet).
In the xml printer patch we want to ensure that only targets which
use the newer style descriptions dynamically generate xml. Other targets
should continue to return the name of the xml file.
The "feature" marker will enable this.
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
* features/Makefile: Add feature marker to targets with new style
target descriptions.
* regformats/aarch64.dat: Regenerate.
* regformats/i386/amd64-avx-avx512-linux.dat: Likewise.
* regformats/i386/amd64-avx-linux.dat: Likewise.
* regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat: Likewise.
* regformats/i386/amd64-avx-mpx-linux.dat: Likewise.
* regformats/i386/amd64-linux.dat: Likewise.
* regformats/i386/amd64-mpx-linux.dat: Likewise.
* regformats/i386/amd64.dat: Likewise.
* regformats/i386/i386-avx-avx512-linux.dat: Likewise.
* regformats/i386/i386-avx-linux.dat: Likewise.
* regformats/i386/i386-avx-mpx-avx512-pku-linux.dat: Likewise.
* regformats/i386/i386-avx-mpx-linux.dat: Likewise.
* regformats/i386/i386-linux.dat: Likewise.
* regformats/i386/i386-mmx-linux.dat: Likewise.
* regformats/i386/i386-mpx-linux.dat: Likewise.
* regformats/i386/i386.dat: Likewise.
* regformats/i386/x32-avx-avx512-linux.dat: Likewise.
* regformats/i386/x32-avx-linux.dat: Likewise.
* regformats/i386/x32-linux.dat: Likewise.
* regformats/tic6x-c62x-linux.dat: Likewise.
* regformats/tic6x-c64x-linux.dat: Likewise.
* regformats/tic6x-c64xp-linux.dat: Likewise.
* regformats/regdat.sh: Parse feature marker.
---
gdb/features/Makefile | 6 ++++++
gdb/regformats/aarch64.dat | 1 +
gdb/regformats/i386/amd64-avx-avx512-linux.dat | 1 +
gdb/regformats/i386/amd64-avx-linux.dat | 1 +
gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat | 1 +
gdb/regformats/i386/amd64-avx-mpx-linux.dat | 1 +
gdb/regformats/i386/amd64-linux.dat | 1 +
gdb/regformats/i386/amd64-mpx-linux.dat | 1 +
gdb/regformats/i386/amd64.dat | 1 +
gdb/regformats/i386/i386-avx-avx512-linux.dat | 1 +
gdb/regformats/i386/i386-avx-linux.dat | 1 +
gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat | 1 +
gdb/regformats/i386/i386-avx-mpx-linux.dat | 1 +
gdb/regformats/i386/i386-linux.dat | 1 +
gdb/regformats/i386/i386-mmx-linux.dat | 1 +
gdb/regformats/i386/i386-mpx-linux.dat | 1 +
gdb/regformats/i386/i386.dat | 1 +
gdb/regformats/i386/x32-avx-avx512-linux.dat | 1 +
gdb/regformats/i386/x32-avx-linux.dat | 1 +
gdb/regformats/i386/x32-linux.dat | 1 +
gdb/regformats/regdat.sh | 4 ++++
gdb/regformats/tic6x-c62x-linux.dat | 1 +
gdb/regformats/tic6x-c64x-linux.dat | 1 +
gdb/regformats/tic6x-c64xp-linux.dat | 1 +
24 files changed, 32 insertions(+)
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 82609f5862..168c46e003 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -193,12 +193,18 @@ XMLTOC = \
TDESC_CFILES = $(patsubst %.xml,%.c,$(XMLTOC))
GDB = false
+#Targets which use feature based target descriptions.
+aarch64-feature = 1
+i386-feature = 1
+tic6x-feature = 1
+
all: $(OUTPUTS)
$(outdir)/%.dat: %.xml number-regs.xsl sort-regs.xsl gdbserver-regs.xsl
echo "# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:" > $(outdir)/$*.tmp
echo "# Generated from: $<" >> $(outdir)/$*.tmp
echo "name:`echo $(notdir $*) | sed 's/-/_/g'`" >> $(outdir)/$*.tmp
+ $(if $($(firstword $(subst /, ,$(subst -, ,$*)))-feature), echo "feature:1") >> $(outdir)/$*.tmp
echo "xmltarget:$(<F)" >> $(outdir)/$*.tmp
echo "expedite:$(if $($*-expedite),$($*-expedite),$($(firstword $(subst -, ,$(notdir $*)))-expedite))" \
>> $(outdir)/$*.tmp
diff --git a/gdb/regformats/aarch64.dat b/gdb/regformats/aarch64.dat
index d4cea04358..75ba89a92e 100644
--- a/gdb/regformats/aarch64.dat
+++ b/gdb/regformats/aarch64.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: aarch64.xml
name:aarch64
+feature:1
xmltarget:aarch64.xml
expedite:x29,sp,pc
64:x0
diff --git a/gdb/regformats/i386/amd64-avx-avx512-linux.dat b/gdb/regformats/i386/amd64-avx-avx512-linux.dat
index 0743693886..6cb97964b3 100644
--- a/gdb/regformats/i386/amd64-avx-avx512-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-avx512-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64-avx-avx512-linux.xml
name:amd64_avx_avx512_linux
+feature:1
xmltarget:amd64-avx-avx512-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/amd64-avx-linux.dat b/gdb/regformats/i386/amd64-avx-linux.dat
index 7780b3b6a2..54dad1d0a7 100644
--- a/gdb/regformats/i386/amd64-avx-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64-avx-linux.xml
name:amd64_avx_linux
+feature:1
xmltarget:amd64-avx-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat b/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
index 9cd0fae820..8f326389f1 100644
--- a/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-mpx-avx512-pku-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64-avx-mpx-avx512-pku-linux.xml
name:amd64_avx_mpx_avx512_pku_linux
+feature:1
xmltarget:amd64-avx-mpx-avx512-pku-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/amd64-avx-mpx-linux.dat b/gdb/regformats/i386/amd64-avx-mpx-linux.dat
index 7c2f928070..92ffa0694f 100644
--- a/gdb/regformats/i386/amd64-avx-mpx-linux.dat
+++ b/gdb/regformats/i386/amd64-avx-mpx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64-avx-mpx-linux.xml
name:amd64_avx_mpx_linux
+feature:1
xmltarget:amd64-avx-mpx-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/amd64-linux.dat b/gdb/regformats/i386/amd64-linux.dat
index cd16a15442..81e6aef361 100644
--- a/gdb/regformats/i386/amd64-linux.dat
+++ b/gdb/regformats/i386/amd64-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64-linux.xml
name:amd64_linux
+feature:1
xmltarget:amd64-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/amd64-mpx-linux.dat b/gdb/regformats/i386/amd64-mpx-linux.dat
index 10487f6987..c90b1b5b8a 100644
--- a/gdb/regformats/i386/amd64-mpx-linux.dat
+++ b/gdb/regformats/i386/amd64-mpx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64-mpx-linux.xml
name:amd64_mpx_linux
+feature:1
xmltarget:amd64-mpx-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/amd64.dat b/gdb/regformats/i386/amd64.dat
index 66f26ad094..fde8c1dbe2 100644
--- a/gdb/regformats/i386/amd64.dat
+++ b/gdb/regformats/i386/amd64.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/amd64.xml
name:amd64
+feature:1
xmltarget:amd64.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/i386-avx-avx512-linux.dat b/gdb/regformats/i386/i386-avx-avx512-linux.dat
index 4477133997..57530978df 100644
--- a/gdb/regformats/i386/i386-avx-avx512-linux.dat
+++ b/gdb/regformats/i386/i386-avx-avx512-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-avx-avx512-linux.xml
name:i386_avx_avx512_linux
+feature:1
xmltarget:i386-avx-avx512-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386-avx-linux.dat b/gdb/regformats/i386/i386-avx-linux.dat
index 1c3fcfd763..6a81302766 100644
--- a/gdb/regformats/i386/i386-avx-linux.dat
+++ b/gdb/regformats/i386/i386-avx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-avx-linux.xml
name:i386_avx_linux
+feature:1
xmltarget:i386-avx-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat b/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
index 515ee10525..ef6216be92 100644
--- a/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
+++ b/gdb/regformats/i386/i386-avx-mpx-avx512-pku-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-avx-mpx-avx512-pku-linux.xml
name:i386_avx_mpx_avx512_pku_linux
+feature:1
xmltarget:i386-avx-mpx-avx512-pku-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386-avx-mpx-linux.dat b/gdb/regformats/i386/i386-avx-mpx-linux.dat
index 831c476fef..f15bd1a89d 100644
--- a/gdb/regformats/i386/i386-avx-mpx-linux.dat
+++ b/gdb/regformats/i386/i386-avx-mpx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-avx-mpx-linux.xml
name:i386_avx_mpx_linux
+feature:1
xmltarget:i386-avx-mpx-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386-linux.dat b/gdb/regformats/i386/i386-linux.dat
index 0e414e6490..5203cd4ef0 100644
--- a/gdb/regformats/i386/i386-linux.dat
+++ b/gdb/regformats/i386/i386-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-linux.xml
name:i386_linux
+feature:1
xmltarget:i386-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386-mmx-linux.dat b/gdb/regformats/i386/i386-mmx-linux.dat
index aa2a564ac7..736874273e 100644
--- a/gdb/regformats/i386/i386-mmx-linux.dat
+++ b/gdb/regformats/i386/i386-mmx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-mmx-linux.xml
name:i386_mmx_linux
+feature:1
xmltarget:i386-mmx-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386-mpx-linux.dat b/gdb/regformats/i386/i386-mpx-linux.dat
index 1dcdce98cf..856ef4b1c9 100644
--- a/gdb/regformats/i386/i386-mpx-linux.dat
+++ b/gdb/regformats/i386/i386-mpx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386-mpx-linux.xml
name:i386_mpx_linux
+feature:1
xmltarget:i386-mpx-linux.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/i386.dat b/gdb/regformats/i386/i386.dat
index 13abb485e4..e707ed2790 100644
--- a/gdb/regformats/i386/i386.dat
+++ b/gdb/regformats/i386/i386.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/i386.xml
name:i386
+feature:1
xmltarget:i386.xml
expedite:ebp,esp,eip
32:eax
diff --git a/gdb/regformats/i386/x32-avx-avx512-linux.dat b/gdb/regformats/i386/x32-avx-avx512-linux.dat
index 00786172fb..a9bd1d4bb3 100644
--- a/gdb/regformats/i386/x32-avx-avx512-linux.dat
+++ b/gdb/regformats/i386/x32-avx-avx512-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/x32-avx-avx512-linux.xml
name:x32_avx_avx512_linux
+feature:1
xmltarget:x32-avx-avx512-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/x32-avx-linux.dat b/gdb/regformats/i386/x32-avx-linux.dat
index eb0e395366..9c43fa48f5 100644
--- a/gdb/regformats/i386/x32-avx-linux.dat
+++ b/gdb/regformats/i386/x32-avx-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/x32-avx-linux.xml
name:x32_avx_linux
+feature:1
xmltarget:x32-avx-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/i386/x32-linux.dat b/gdb/regformats/i386/x32-linux.dat
index eee378fd84..fda1a89de0 100644
--- a/gdb/regformats/i386/x32-linux.dat
+++ b/gdb/regformats/i386/x32-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: i386/x32-linux.xml
name:x32_linux
+feature:1
xmltarget:x32-linux.xml
expedite:rbp,rsp,rip
64:rax
diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 8c6e191596..18108d77eb 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -118,6 +118,7 @@ xmltarget=x
xmlarch=x
xmlosabi=x
expedite=x
+feature=x
exec < $1
while do_read
do
@@ -145,6 +146,9 @@ do
elif test "${type}" = "expedite"; then
expedite="${entry}"
continue
+ elif test "${type}" = "feature"; then
+ feature="${entry}"
+ continue
elif test "${name}" = x; then
echo "$0: $1 does not specify \`\`name''." 1>&2
exit 1
diff --git a/gdb/regformats/tic6x-c62x-linux.dat b/gdb/regformats/tic6x-c62x-linux.dat
index 82f2a0a0bf..98ca39e110 100644
--- a/gdb/regformats/tic6x-c62x-linux.dat
+++ b/gdb/regformats/tic6x-c62x-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: tic6x-c62x-linux.xml
name:tic6x_c62x_linux
+feature:1
xmltarget:tic6x-c62x-linux.xml
expedite:A15,PC
32:A0
diff --git a/gdb/regformats/tic6x-c64x-linux.dat b/gdb/regformats/tic6x-c64x-linux.dat
index 542826ad1d..91e84baebc 100644
--- a/gdb/regformats/tic6x-c64x-linux.dat
+++ b/gdb/regformats/tic6x-c64x-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: tic6x-c64x-linux.xml
name:tic6x_c64x_linux
+feature:1
xmltarget:tic6x-c64x-linux.xml
expedite:A15,PC
32:A0
diff --git a/gdb/regformats/tic6x-c64xp-linux.dat b/gdb/regformats/tic6x-c64xp-linux.dat
index 229b3c26c2..a3fa3b7cd4 100644
--- a/gdb/regformats/tic6x-c64xp-linux.dat
+++ b/gdb/regformats/tic6x-c64xp-linux.dat
@@ -1,6 +1,7 @@
# THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi :set ro:
# Generated from: tic6x-c64xp-linux.xml
name:tic6x_c64xp_linux
+feature:1
xmltarget:tic6x-c64xp-linux.xml
expedite:A15,PC
32:A0
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 10/10] Remove xml files from gdbserver
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (8 preceding siblings ...)
2018-03-22 8:46 ` [PATCH v4 09/10] Remove xml file references from target descriptions alan.hayward
@ 2018-03-22 8:46 ` alan.hayward
2018-03-24 2:06 ` [PATCH v4 00/10] Remove gdbserver dependency on xml files Simon Marchi
10 siblings, 0 replies; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:46 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
For ports which use new target descriptions, this patch removes
the xml files from being built into gdbserver,
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdbserver/
* configure.srv (aarch64*-*-linux*): Don't include xml.
(i[34567]86-*-cygwin*): Likewise.
(i[34567]86-*-linux*): Likewise.
(i[34567]86-*-lynxos*): Likewise.
(i[34567]86-*-mingw32ce*): Likewise.
(i[34567]86-*-mingw*): Likewise.
(i[34567]86-*-nto*): Likewise.
(tic6x-*-uclinux): Likewise.
(x86_64-*-linux*): Likewise.
(x86_64-*-mingw*): Likewise.
(x86_64-*-cygwin*): Likewise.
---
gdb/gdbserver/configure.srv | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 087fd31426..ffeefb9b92 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"
@@ -121,13 +108,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 +129,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 +136,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 +144,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 +348,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 +361,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 +371,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
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 09/10] Remove xml file references from target descriptions.
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (7 preceding siblings ...)
2018-03-22 8:45 ` [PATCH v4 03/10] Commonise tdesc_reg alan.hayward
@ 2018-03-22 8:46 ` alan.hayward
2018-03-22 8:46 ` [PATCH v4 10/10] Remove xml files from gdbserver alan.hayward
2018-03-24 2:06 ` [PATCH v4 00/10] Remove gdbserver dependency on xml files Simon Marchi
10 siblings, 0 replies; 25+ messages in thread
From: alan.hayward @ 2018-03-22 8:46 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
From: Alan Hayward <alan.hayward@arm.com>
We no longer need to know the name of the xml file for targets using the
new target descriptions. This patch removes the references and regenerates
the C files.
This patch is identical to the V3 version.
Alan.
2018-03-21 Alan Hayward <alan.hayward@arm.com>
gdb/
* common/tdesc.h (tdesc_create_feature): Remove xml filename parameter.
* features/aarch64-core.c (create_feature_aarch64_core): Regenerate.
* features/aarch64-fpu.c (create_feature_aarch64_fpu): Likewise.
* features/i386/32bit-avx.c (create_feature_i386_32bit_avx): Likewise.
* features/i386/32bit-avx512.c (create_feature_i386_32bit_avx512):
Likewise.
* features/i386/32bit-core.c (create_feature_i386_32bit_core):
Likewise.
* features/i386/32bit-linux.c (create_feature_i386_32bit_linux):
Likewise.
* features/i386/32bit-mpx.c (create_feature_i386_32bit_mpx): Likewise.
* features/i386/32bit-pkeys.c (create_feature_i386_32bit_pkeys):
Likewise.
* features/i386/32bit-sse.c (create_feature_i386_32bit_sse): Likewise.
* features/i386/64bit-avx.c (create_feature_i386_64bit_avx): Likewise.
* features/i386/64bit-avx512.c (create_feature_i386_64bit_avx512):
Likewise.
* features/i386/64bit-core.c (create_feature_i386_64bit_core):
Likewise.
* features/i386/64bit-linux.c (create_feature_i386_64bit_linux):
Likewise.
* features/i386/64bit-mpx.c (create_feature_i386_64bit_mpx): Likewise.
* features/i386/64bit-pkeys.c (create_feature_i386_64bit_pkeys):
Likewise.
* features/i386/64bit-segments.c (create_feature_i386_64bit_segments):
Likewise.
* features/i386/64bit-sse.c (create_feature_i386_64bit_sse): Likewise.
* features/i386/x32-core.c (create_feature_i386_x32_core): Likewise.
* features/tic6x-c6xp.c (create_feature_tic6x_c6xp): Likewise.
* features/tic6x-core.c (create_feature_tic6x_core): Likewise.
* features/tic6x-gp.c (create_feature_tic6x_gp): Likewise.
* target-descriptions.c: In generated code, don't pass xml filename.
gdbserver/
* gdbserver/tdesc.c: Remove xml parameter.
---
gdb/common/tdesc.h | 4 +---
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/tdesc.c | 6 ++----
gdb/target-descriptions.c | 7 +++----
24 files changed, 27 insertions(+), 32 deletions(-)
diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
index 45eb24ea2b..3d23258acf 100644
--- a/gdb/common/tdesc.h
+++ b/gdb/common/tdesc.h
@@ -322,9 +322,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 db10c4aa06..cd3de02cbc 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 0c40c4cef2..47962d1f24 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 6e2cfdbdcb..b9feac377c 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 863c119ac4..51a403259d 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 f6de737074..098c2ca31c 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 af476da194..3bc593739c 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 84101746b5..8288e9d758 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 272751f578..a2a72f40a3 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 78fc02b863..68fbab06a4 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 5f11035af3..7e45c980db 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 733910025b..acc07a3628 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 efc7016380..617425da21 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 c9c56f193c..68de9d27e8 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 d923813d69..ce4d611d9c 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 f1cbce0ce7..70b88cf74d 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 900471fc48..b06d40fbda 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 294360197f..e083093741 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 9b0bcc5a7e..dd6e1a2ed6 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 261a562d90..bfd69d7fd5 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 65f73ec97d..0415209275 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 b624588379..4a0734c04a 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 5ec73d9117..13d7225f49 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -199,11 +199,9 @@ tdesc_get_features_xml (target_desc *tdesc)
/* See common/tdesc.h. */
struct tdesc_feature *
-tdesc_create_feature (struct target_desc *tdesc, const char *name,
- const char *xml)
+tdesc_create_feature (struct target_desc *tdesc, const char *name)
{
- struct tdesc_feature *new_feature = new tdesc_feature
- (xml != nullptr ? xml : name);
+ struct tdesc_feature *new_feature = new tdesc_feature (name);
tdesc->features.emplace_back (new_feature);
return new_feature;
}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 467ae94e0c..e3e5f60b27 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1130,8 +1130,7 @@ tdesc_use_registers (struct gdbarch *gdbarch,
/* See common/tdesc.h. */
struct tdesc_feature *
-tdesc_create_feature (struct target_desc *tdesc, const char *name,
- const char *xml)
+tdesc_create_feature (struct target_desc *tdesc, const char *name)
{
struct tdesc_feature *new_feature = new tdesc_feature (name);
@@ -1599,8 +1598,8 @@ public:
printf_unfiltered (" struct tdesc_feature *feature;\n");
printf_unfiltered
- ("\n feature = tdesc_create_feature (result, \"%s\", \"%s\");\n",
- e->name.c_str (), lbasename (m_filename_after_features.c_str ()));
+ ("\n feature = tdesc_create_feature (result, \"%s\");\n",
+ e->name.c_str ());
}
void visit_post (const tdesc_feature *e) override
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 01/10] Move tdesc header funcs to c file
2018-03-22 8:45 ` [PATCH v4 01/10] Move tdesc header funcs to c file alan.hayward
@ 2018-03-22 20:43 ` Simon Marchi
0 siblings, 0 replies; 25+ messages in thread
From: Simon Marchi @ 2018-03-22 20:43 UTC (permalink / raw)
To: alan.hayward, gdb-patches; +Cc: nd
On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
> From: Alan Hayward <alan.hayward@arm.com>
>
> Move the destructor and equals operator for gdbserver tdesc from the .h
> to the .c file. Both functions are too long to be inlined and make the
> header look messy. Patch does not change any functionality.
> Patch identical to V3 version.
>
> 2018-03-21 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.
This patch LGTM, feel free to push it by itself, it will be one
less patch to worry about.
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-22 8:45 ` [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects alan.hayward
@ 2018-03-22 21:33 ` Simon Marchi
2018-03-23 14:54 ` Alan Hayward
0 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2018-03-22 21:33 UTC (permalink / raw)
To: alan.hayward, gdb-patches; +Cc: nd
On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
> From: Alan Hayward <alan.hayward@arm.com>
>
> Following a suggestion from Philipp, this patch changes reg_defs in
> gdbserver tdesc so that it is a vector of objects instead of a vector
> of pointers. Updated uses of reg_defs to use references.
>
> This change reduces the number of small hunks allocated when building
> reg_defs.
>
> find_register_by_number() is only used by regcache, therefore I made
> it a static function and moved it earlier in the file so that find_regno()
> could use it. The eventual plan is to replace this with accessor functions
> in a common tdesc class.
>
> Alan.
>
> 2018-03-21 Alan Hayward <alan.hayward@arm.com>
>
> gdb/gdbserver/
> * regcache.c (find_register_by_number): Make static and return a ref.
> (find_regno): Use find_register_by_number
> (register_size): Use references.
> (register_data): Likewise.
> * regcache.h (struct reg): Remove declaration.
> * tdesc.c (target_desc::~target_desc): Remove free calls.
> (target_desc::operator==): Use references.
> (init_target_desc): Likewise.
> (tdesc_create_reg): Use references and resize in a single call.
> * tdesc.h (struct target_desc): Replace pointer with object.
> ---
> gdb/gdbserver/regcache.c | 27 +++++++++++----------------
> gdb/gdbserver/regcache.h | 4 ----
> gdb/gdbserver/tdesc.c | 30 ++++++++++++++----------------
> gdb/gdbserver/tdesc.h | 2 +-
> 4 files changed, 26 insertions(+), 37 deletions(-)
>
> diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
> index 1bb15900dd..df5b9e9dd2 100644
> --- a/gdb/gdbserver/regcache.c
> +++ b/gdb/gdbserver/regcache.c
> @@ -196,6 +196,13 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
> dst->registers_valid = src->registers_valid;
> }
>
> +/* Return a reference to the description of register ``n''. */
> +
> +static const struct reg &
> +find_register_by_number (const struct target_desc *tdesc, int n)
> +{
> + return tdesc->reg_defs[n];
> +}
>
> #ifndef IN_PROCESS_AGENT
>
> @@ -243,25 +250,13 @@ int
> find_regno (const struct target_desc *tdesc, const char *name)
> {
> for (int i = 0; i < tdesc->reg_defs.size (); ++i)
> - {
> - struct reg *reg = tdesc->reg_defs[i];
> + if (strcmp (name, find_register_by_number (tdesc, i).name) == 0)
> + return i;
>
> - if (strcmp (name, reg->name) == 0)
> - return i;
> - }
> internal_error (__FILE__, __LINE__, "Unknown register %s requested",
> name);
Please keep the curly braces for the for, as shown here:
https://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.html#index-multiple-variables-in-a-line
Can you put the moving of this function/making it static/removing the declaration
in its own patch? It's pre-approved, with that fixed.
> }
>
> -#endif
> -
> -struct reg *
> -find_register_by_number (const struct target_desc *tdesc, int n)
> -{
> - return tdesc->reg_defs[n];
> -}
> -
> -#ifndef IN_PROCESS_AGENT
> static void
> free_register_cache_thread (struct thread_info *thread)
> {
> @@ -292,7 +287,7 @@ register_cache_size (const struct target_desc *tdesc)
> int
> register_size (const struct target_desc *tdesc, int n)
> {
> - return find_register_by_number (tdesc, n)->size / 8;
> + return find_register_by_number (tdesc, n).size / 8;
> }
>
> /* See common/common-regcache.h. */
> @@ -307,7 +302,7 @@ static unsigned char *
> register_data (struct regcache *regcache, int n, int fetch)
> {
> return (regcache->registers
> - + find_register_by_number (regcache->tdesc, n)->offset / 8);
> + + find_register_by_number (regcache->tdesc, n).offset / 8);
> }
>
> /* Supply register N, whose contents are stored in BUF, to REGCACHE.
> diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
> index 3a75ce3fe1..6ff13084b0 100644
> --- a/gdb/gdbserver/regcache.h
> +++ b/gdb/gdbserver/regcache.h
> @@ -94,10 +94,6 @@ void registers_from_string (struct regcache *regcache, char *buf);
>
> void regcache_write_pc (struct regcache *regcache, CORE_ADDR pc);
>
> -/* Return a pointer to the description of register ``n''. */
> -
> -struct reg *find_register_by_number (const struct target_desc *tdesc, int n);
> -
> int register_cache_size (const struct target_desc *tdesc);
>
> int register_size (const struct target_desc *tdesc, int n);
> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
> index e50a848e2f..8e68a27c7c 100644
> --- a/gdb/gdbserver/tdesc.c
> +++ b/gdb/gdbserver/tdesc.c
> @@ -25,9 +25,6 @@ target_desc::~target_desc ()
> {
> int i;
>
> - for (reg *reg : reg_defs)
> - xfree (reg);
> -
> xfree ((char *) arch);
> xfree ((char *) osabi);
>
> @@ -45,10 +42,10 @@ bool target_desc::operator== (const target_desc &other) const
>
> for (int i = 0; i < reg_defs.size (); ++i)
> {
> - struct reg *reg = reg_defs[i];
> - struct reg *reg2 = other.reg_defs[i];
> + struct reg reg = reg_defs[i];
> + struct reg reg2 = other.reg_defs[i];
>
> - if (reg != reg2 && *reg != *reg2)
> + if (® != ®2 && reg != reg2)
I don't think the first part, "® != ®", makes sense. It's comparing
the addresses of the local variables. Also, that comparison only made
sense as a shortcut when the vector held pointers to dynamically allocated
memory. So I think it should just be removed.
Actually, now that the vector elements are the objects directly, I think that
this whole for loop could be replaced with:
if (reg_defs != other.reg_defs)
return false;
The if that compares the vector sizes above that can also be removed, since
it's also done by std::vector::operator==:
template<typename _Tp, typename _Alloc>
inline bool
operator==(const vector<_Tp, _Alloc>& __x, const vector<_Tp, _Alloc>& __y)
{ return (__x.size() == __y.size()
&& std::equal(__x.begin(), __x.end(), __y.begin())); }
> return false;
> }
>
> @@ -72,10 +69,10 @@ init_target_desc (struct target_desc *tdesc)
> {
> int offset = 0;
>
> - for (reg *reg : tdesc->reg_defs)
> + for (reg ® : tdesc->reg_defs)
> {
> - reg->offset = offset;
> - offset += reg->size;
> + reg.offset = offset;
> + offset += reg.size;
> }
>
> tdesc->registers_size = offset / 8;
> @@ -240,24 +237,25 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
> int bitsize, const char *type)
> {
> struct target_desc *tdesc = (struct target_desc *) feature;
> + int current_size = tdesc->reg_defs.size ();
>
> - while (tdesc->reg_defs.size () < regnum)
> - {
> - struct reg *reg = XCNEW (struct reg);
> + tdesc->reg_defs.resize (regnum != 0 ? regnum + 1 : current_size + 1);
>
> + while (current_size < regnum)
> + {
> + struct reg *reg = &tdesc->reg_defs[current_size];
> reg->name = "";
> reg->size = 0;
> - tdesc->reg_defs.push_back (reg);
> + current_size++;
> }
>
> gdb_assert (regnum == 0
> - || regnum == tdesc->reg_defs.size ());
> + || regnum + 1 == tdesc->reg_defs.size ());
>
> - struct reg *reg = XCNEW (struct reg);
> + struct reg *reg = &tdesc->reg_defs.back ();
>
> reg->name = name;
> reg->size = bitsize;
> - tdesc->reg_defs.push_back (reg);
> }
To simplify this further, I would suggest adding a default constructor to reg
that initializes everything to nullptr/0, and adding a constructor that takes
parameters. The .resize() will only be necessary if regnum != 0 and will
automatically initialize all the unused register slots properly, so the for
loop will be unnecessary:
if (regnum != 0)
tdesc->reg_defs.resize (regnum);
The register can then be added with
tdesc->reg_defs.emplace_back (name, bitsize);
One behavior that might change compared to the previous code is when trying to
add a register with a regnum smaller than the current size of the vector. I
think that the old code would have asserted, but the new code would just
accept it and resize the vector to a smaller size. Could you verify? In that
case, maybe we should move the gdb_assert to the beginning of the function:
gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ())
something like that. It can also be moved under the "if (regnum != 0)" shown
above.
>
> /* See common/tdesc.h. */
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 4513ea7423..a62544341c 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -34,7 +34,7 @@ struct target_desc : tdesc_feature
> {
> /* A vector of elements of register definitions that
> describe the inferior's register set. */
> - std::vector<struct reg *> reg_defs;
> + std::vector<struct reg> reg_defs;
>
> /* The register cache size, in bytes. */
> int registers_size;
>
Thanks,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-22 21:33 ` Simon Marchi
@ 2018-03-23 14:54 ` Alan Hayward
2018-03-23 15:36 ` Simon Marchi
0 siblings, 1 reply; 25+ messages in thread
From: Alan Hayward @ 2018-03-23 14:54 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, nd
> On 22 Mar 2018, at 21:33, Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
>> From: Alan Hayward <alan.hayward@arm.com>
>>
>> Following a suggestion from Philipp, this patch changes reg_defs in
>> gdbserver tdesc so that it is a vector of objects instead of a vector
>> of pointers. Updated uses of reg_defs to use references.
>>
>> This change reduces the number of small hunks allocated when building
>> reg_defs.
>>
>> find_register_by_number() is only used by regcache, therefore I made
>> it a static function and moved it earlier in the file so that find_regno()
>> could use it. The eventual plan is to replace this with accessor functions
>> in a common tdesc class.
>>
>> Alan.
>>
>> 2018-03-21 Alan Hayward <alan.hayward@arm.com>
>>
>> gdb/gdbserver/
>> * regcache.c (find_register_by_number): Make static and return a ref.
>> (find_regno): Use find_register_by_number
>> (register_size): Use references.
>> (register_data): Likewise.
>> * regcache.h (struct reg): Remove declaration.
>> * tdesc.c (target_desc::~target_desc): Remove free calls.
>> (target_desc::operator==): Use references.
>> (init_target_desc): Likewise.
>> (tdesc_create_reg): Use references and resize in a single call.
>> * tdesc.h (struct target_desc): Replace pointer with object.
>> ---
>> gdb/gdbserver/regcache.c | 27 +++++++++++----------------
>> gdb/gdbserver/regcache.h | 4 ----
>> gdb/gdbserver/tdesc.c | 30 ++++++++++++++----------------
>> gdb/gdbserver/tdesc.h | 2 +-
>> 4 files changed, 26 insertions(+), 37 deletions(-)
>>
>> diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
>> index 1bb15900dd..df5b9e9dd2 100644
>> --- a/gdb/gdbserver/regcache.c
>> +++ b/gdb/gdbserver/regcache.c
>> @@ -196,6 +196,13 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
>> dst->registers_valid = src->registers_valid;
>> }
>>
>> +/* Return a reference to the description of register ``n''. */
>> +
>> +static const struct reg &
>> +find_register_by_number (const struct target_desc *tdesc, int n)
>> +{
>> + return tdesc->reg_defs[n];
>> +}
>>
>> #ifndef IN_PROCESS_AGENT
>>
>> @@ -243,25 +250,13 @@ int
>> find_regno (const struct target_desc *tdesc, const char *name)
>> {
>> for (int i = 0; i < tdesc->reg_defs.size (); ++i)
>> - {
>> - struct reg *reg = tdesc->reg_defs[i];
>> + if (strcmp (name, find_register_by_number (tdesc, i).name) == 0)
>> + return i;
>>
>> - if (strcmp (name, reg->name) == 0)
>> - return i;
>> - }
>> internal_error (__FILE__, __LINE__, "Unknown register %s requested",
>> name);
>
> Please keep the curly braces for the for, as shown here:
>
> https://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.html#index-multiple-variables-in-a-line
>
> Can you put the moving of this function/making it static/removing the declaration
> in its own patch? It's pre-approved, with that fixed.
>
Ok, I’ve pushed this bit as requested:
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 1bb15900dd..d6511fda65 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -196,6 +196,13 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
dst->registers_valid = src->registers_valid;
}
+/* Return a pointer to the description of register N. */
+
+static const struct reg *
+find_register_by_number (const struct target_desc *tdesc, int n)
+{
+ return tdesc->reg_defs[n];
+}
#ifndef IN_PROCESS_AGENT
@@ -244,24 +251,13 @@ find_regno (const struct target_desc *tdesc, const char *name)
{
for (int i = 0; i < tdesc->reg_defs.size (); ++i)
{
- struct reg *reg = tdesc->reg_defs[i];
-
- if (strcmp (name, reg->name) == 0)
+ if (strcmp (name, find_register_by_number (tdesc, i)->name) == 0)
return i;
}
internal_error (__FILE__, __LINE__, "Unknown register %s requested",
name);
}
-#endif
-
-struct reg *
-find_register_by_number (const struct target_desc *tdesc, int n)
-{
- return tdesc->reg_defs[n];
-}
-
-#ifndef IN_PROCESS_AGENT
static void
free_register_cache_thread (struct thread_info *thread)
{
diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
index 3a75ce3fe1..6ff13084b0 100644
--- a/gdb/gdbserver/regcache.h
+++ b/gdb/gdbserver/regcache.h
@@ -94,10 +94,6 @@ void registers_from_string (struct regcache *regcache, char *buf);
void regcache_write_pc (struct regcache *regcache, CORE_ADDR pc);
-/* Return a pointer to the description of register ``n''. */
-
-struct reg *find_register_by_number (const struct target_desc *tdesc, int n);
-
int register_cache_size (const struct target_desc *tdesc);
int register_size (const struct target_desc *tdesc, int n);
>> }
>>
>> -#endif
>> -
>> -struct reg *
>> -find_register_by_number (const struct target_desc *tdesc, int n)
>> -{
>> - return tdesc->reg_defs[n];
>> -}
>> -
>> -#ifndef IN_PROCESS_AGENT
>> static void
>> free_register_cache_thread (struct thread_info *thread)
>> {
>> @@ -292,7 +287,7 @@ register_cache_size (const struct target_desc *tdesc)
>> int
>> register_size (const struct target_desc *tdesc, int n)
>> {
>> - return find_register_by_number (tdesc, n)->size / 8;
>> + return find_register_by_number (tdesc, n).size / 8;
>> }
>>
>> /* See common/common-regcache.h. */
>> @@ -307,7 +302,7 @@ static unsigned char *
>> register_data (struct regcache *regcache, int n, int fetch)
>> {
>> return (regcache->registers
>> - + find_register_by_number (regcache->tdesc, n)->offset / 8);
>> + + find_register_by_number (regcache->tdesc, n).offset / 8);
>> }
>>
>> /* Supply register N, whose contents are stored in BUF, to REGCACHE.
>> diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
>> index 3a75ce3fe1..6ff13084b0 100644
>> --- a/gdb/gdbserver/regcache.h
>> +++ b/gdb/gdbserver/regcache.h
>> @@ -94,10 +94,6 @@ void registers_from_string (struct regcache *regcache, char *buf);
>>
>> void regcache_write_pc (struct regcache *regcache, CORE_ADDR pc);
>>
>> -/* Return a pointer to the description of register ``n''. */
>> -
>> -struct reg *find_register_by_number (const struct target_desc *tdesc, int n);
>> -
>> int register_cache_size (const struct target_desc *tdesc);
>>
>> int register_size (const struct target_desc *tdesc, int n);
>> diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
>> index e50a848e2f..8e68a27c7c 100644
>> --- a/gdb/gdbserver/tdesc.c
>> +++ b/gdb/gdbserver/tdesc.c
>> @@ -25,9 +25,6 @@ target_desc::~target_desc ()
>> {
>> int i;
>>
>> - for (reg *reg : reg_defs)
>> - xfree (reg);
>> -
>> xfree ((char *) arch);
>> xfree ((char *) osabi);
>>
>> @@ -45,10 +42,10 @@ bool target_desc::operator== (const target_desc &other) const
>>
>> for (int i = 0; i < reg_defs.size (); ++i)
>> {
>> - struct reg *reg = reg_defs[i];
>> - struct reg *reg2 = other.reg_defs[i];
>> + struct reg reg = reg_defs[i];
>> + struct reg reg2 = other.reg_defs[i];
>>
>> - if (reg != reg2 && *reg != *reg2)
>> + if (® != ®2 && reg != reg2)
>
> I don't think the first part, "® != ®", makes sense. It's comparing
> the addresses of the local variables. Also, that comparison only made
> sense as a shortcut when the vector held pointers to dynamically allocated
> memory. So I think it should just be removed.
>
> Actually, now that the vector elements are the objects directly, I think that
> this whole for loop could be replaced with:
>
> if (reg_defs != other.reg_defs)
> return false;
>
> The if that compares the vector sizes above that can also be removed, since
> it's also done by std::vector::operator==:
>
> template<typename _Tp, typename _Alloc>
> inline bool
> operator==(const vector<_Tp, _Alloc>& __x, const vector<_Tp, _Alloc>& __y)
> { return (__x.size() == __y.size()
> && std::equal(__x.begin(), __x.end(), __y.begin())); }
>
Didn’t realise that std:vector worked like this. Agreed with the change.
>> return false;
>> }
>>
>> @@ -72,10 +69,10 @@ init_target_desc (struct target_desc *tdesc)
>> {
>> int offset = 0;
>>
>> - for (reg *reg : tdesc->reg_defs)
>> + for (reg ® : tdesc->reg_defs)
>> {
>> - reg->offset = offset;
>> - offset += reg->size;
>> + reg.offset = offset;
>> + offset += reg.size;
>> }
>>
>> tdesc->registers_size = offset / 8;
>> @@ -240,24 +237,25 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
>> int bitsize, const char *type)
>> {
>> struct target_desc *tdesc = (struct target_desc *) feature;
>> + int current_size = tdesc->reg_defs.size ();
>>
>> - while (tdesc->reg_defs.size () < regnum)
>> - {
>> - struct reg *reg = XCNEW (struct reg);
>> + tdesc->reg_defs.resize (regnum != 0 ? regnum + 1 : current_size + 1);
>>
>> + while (current_size < regnum)
>> + {
>> + struct reg *reg = &tdesc->reg_defs[current_size];
>> reg->name = "";
>> reg->size = 0;
>> - tdesc->reg_defs.push_back (reg);
>> + current_size++;
>> }
>>
>> gdb_assert (regnum == 0
>> - || regnum == tdesc->reg_defs.size ());
>> + || regnum + 1 == tdesc->reg_defs.size ());
>>
>> - struct reg *reg = XCNEW (struct reg);
>> + struct reg *reg = &tdesc->reg_defs.back ();
>>
>> reg->name = name;
>> reg->size = bitsize;
>> - tdesc->reg_defs.push_back (reg);
>> }
>
> To simplify this further, I would suggest adding a default constructor to reg
> that initializes everything to nullptr/0, and adding a constructor that takes
> parameters. The .resize() will only be necessary if regnum != 0 and will
> automatically initialize all the unused register slots properly, so the for
> loop will be unnecessary:
>
> if (regnum != 0)
> tdesc->reg_defs.resize (regnum);
>
> The register can then be added with
>
> tdesc->reg_defs.emplace_back (name, bitsize);
I need to remember to think in C++ and not C. This way is much nicer/simpler.
>
> One behavior that might change compared to the previous code is when trying to
> add a register with a regnum smaller than the current size of the vector. I
> think that the old code would have asserted, but the new code would just
> accept it and resize the vector to a smaller size. Could you verify? In that
> case, maybe we should move the gdb_assert to the beginning of the function:
>
> gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ())
>
> something like that. It can also be moved under the "if (regnum != 0)" shown
> above.
Yes, in the old code, if you give it a smaller regnum, vector wouldn’t have been resized,
and then the assert would have fired.
I’ll move the assert to the top of the function to ensure it doesn’t resize smaller.
New version updated with all of the above.
Checked this on X86 with make check on target board gdbserver.
(Patch 3/10, and possibly others, will need updating too, but should be obvious).
Thanks for the review.
Alan.
gdb/
* regformats/regdef.h (reg): Add constructors.
gdb/gdbserver/
* regcache.c (find_register_by_number): Return a ref.
(find_regno): Use references.
(register_size): Likewise.
(register_data): Likewise.
* tdesc.c (target_desc::~target_desc): Remove free calls.
(target_desc::operator==): Use std::vector compare.
(init_target_desc): Use reference.
(tdesc_create_reg): Use reg constructors.
* tdesc.h (struct target_desc): Replace pointer with object.
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index d6511fda650ca52688cd4d7db1acd94e822a3c0d..cbdf766df2c2b95f605198c617ffead9f9ac3775 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -196,9 +196,9 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
dst->registers_valid = src->registers_valid;
}
-/* Return a pointer to the description of register N. */
+/* Return a reference to the description of register N. */
-static const struct reg *
+static const struct reg &
find_register_by_number (const struct target_desc *tdesc, int n)
{
return tdesc->reg_defs[n];
@@ -251,7 +251,7 @@ find_regno (const struct target_desc *tdesc, const char *name)
{
for (int i = 0; i < tdesc->reg_defs.size (); ++i)
{
- if (strcmp (name, find_register_by_number (tdesc, i)->name) == 0)
+ if (strcmp (name, find_register_by_number (tdesc, i).name) == 0)
return i;
}
internal_error (__FILE__, __LINE__, "Unknown register %s requested",
@@ -288,7 +288,7 @@ register_cache_size (const struct target_desc *tdesc)
int
register_size (const struct target_desc *tdesc, int n)
{
- return find_register_by_number (tdesc, n)->size / 8;
+ return find_register_by_number (tdesc, n).size / 8;
}
/* See common/common-regcache.h. */
@@ -303,7 +303,7 @@ static unsigned char *
register_data (struct regcache *regcache, int n, int fetch)
{
return (regcache->registers
- + find_register_by_number (regcache->tdesc, n)->offset / 8);
+ + find_register_by_number (regcache->tdesc, n).offset / 8);
}
/* Supply register N, whose contents are stored in BUF, to REGCACHE.
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 4513ea74232a456cc86eb9a655904012ff117373..a62544341cd23a9a8ec6833e1eae73616a315d2d 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -34,7 +34,7 @@ struct target_desc : tdesc_feature
{
/* A vector of elements of register definitions that
describe the inferior's register set. */
- std::vector<struct reg *> reg_defs;
+ std::vector<struct reg> reg_defs;
/* The register cache size, in bytes. */
int registers_size;
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e50a848e2f9f280a84ab139cfce4d1f17bd05884..877dbdaecac43f42c031dffafe2bd9f01b577038 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -25,9 +25,6 @@ target_desc::~target_desc ()
{
int i;
- for (reg *reg : reg_defs)
- xfree (reg);
-
xfree ((char *) arch);
xfree ((char *) osabi);
@@ -40,18 +37,9 @@ target_desc::~target_desc ()
bool target_desc::operator== (const target_desc &other) const
{
- if (reg_defs.size () != other.reg_defs.size ())
+ if (reg_defs != other.reg_defs)
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++)
@@ -72,10 +60,10 @@ init_target_desc (struct target_desc *tdesc)
{
int offset = 0;
- for (reg *reg : tdesc->reg_defs)
+ for (reg ® : tdesc->reg_defs)
{
- reg->offset = offset;
- offset += reg->size;
+ reg.offset = offset;
+ offset += reg.size;
}
tdesc->registers_size = offset / 8;
@@ -241,23 +229,12 @@ 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 ());
+ gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());
- struct reg *reg = XCNEW (struct reg);
+ if (regnum != 0)
+ tdesc->reg_defs.resize (regnum);
- reg->name = name;
- reg->size = bitsize;
- tdesc->reg_defs.push_back (reg);
+ tdesc->reg_defs.emplace_back (name, 0, bitsize);
}
/* See common/tdesc.h. */
diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
index 262d03c0785f48e83b784b6177c52e2d253a6067..1f7861fd98dd9085c3fe9d7ee4b8396999060265 100644
--- a/gdb/regformats/regdef.h
+++ b/gdb/regformats/regdef.h
@@ -21,6 +21,18 @@
struct reg
{
+ reg ()
+ : name (""),
+ offset (0),
+ size (0)
+ {}
+
+ reg (const char *_name, int _offset, int _size)
+ : name (_name),
+ offset (_offset),
+ size (_size)
+ {}
+
/* The name of this register - NULL for pad entries. */
const char *name;
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-23 14:54 ` Alan Hayward
@ 2018-03-23 15:36 ` Simon Marchi
2018-03-23 16:52 ` Alan Hayward
0 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2018-03-23 15:36 UTC (permalink / raw)
To: Alan Hayward; +Cc: Simon Marchi, gdb-patches, nd
On 2018-03-23 10:54, Alan Hayward wrote:
>> Please keep the curly braces for the for, as shown here:
>>
>> https://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.html#index-multiple-variables-in-a-line
>>
>> Can you put the moving of this function/making it static/removing the
>> declaration
>> in its own patch? It's pre-approved, with that fixed.
>>
>
> Ok, I’ve pushed this bit as requested:
Thanks!
> New version updated with all of the above.
> Checked this on X86 with make check on target board gdbserver.
> (Patch 3/10, and possibly others, will need updating too, but should
> be obvious).
>
> Thanks for the review.
>
> Alan.
I had some problems applying your patch, the tabs were replaced with
spaces. You can either fix the settings of your email client, or you
can also use git-send-email when sending individual patch updates, like
this:
git send-email HEAD^ --subject-prefix="PATCH v4.1 02/10" --to <to>
--in-reply-to <message-id>
where message-id can be found in the headers of the message you want to
reply to (header Message-ID). This allows the message to be correctly
threaded. It means the patch will be in a separate email than your
response to the reviewer's comments, but I think that's fine.
Anyway, this particular one wasn't too difficult to fix up by hand. It
LGTM with one nit fixed:
> diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
> index
> 262d03c0785f48e83b784b6177c52e2d253a6067..1f7861fd98dd9085c3fe9d7ee4b8396999060265
> 100644
> --- a/gdb/regformats/regdef.h
> +++ b/gdb/regformats/regdef.h
> @@ -21,6 +21,18 @@
>
> struct reg
> {
> + reg ()
> + : name (""),
> + offset (0),
> + size (0)
> + {}
> +
> + reg (const char *_name, int _offset, int _size)
The offset value is always 0 initially, so you can remove it and
initialize it to 0.
I think that this patch can also be pushed on its own, it's a good
improvement regardless of the rest of the series.
Thanks,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-23 15:36 ` Simon Marchi
@ 2018-03-23 16:52 ` Alan Hayward
2018-03-23 17:04 ` Simon Marchi
0 siblings, 1 reply; 25+ messages in thread
From: Alan Hayward @ 2018-03-23 16:52 UTC (permalink / raw)
To: Simon Marchi; +Cc: Simon Marchi, gdb-patches, nd
> On 23 Mar 2018, at 15:35, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2018-03-23 10:54, Alan Hayward wrote:
>>> Please keep the curly braces for the for, as shown here:
>>> https://www.gnu.org/prep/standards/html_node/Syntactic-Conventions.html#index-multiple-variables-in-a-line
>>> Can you put the moving of this function/making it static/removing the declaration
>>> in its own patch? It's pre-approved, with that fixed.
>> Ok, I’ve pushed this bit as requested:
>
> Thanks!
>
>> New version updated with all of the above.
>> Checked this on X86 with make check on target board gdbserver.
>> (Patch 3/10, and possibly others, will need updating too, but should
>> be obvious).
>> Thanks for the review.
>> Alan.
>
> I had some problems applying your patch, the tabs were replaced with spaces. You can either fix the settings of your email client, or you can also use git-send-email when sending individual patch updates, like this:
>
> git send-email HEAD^ --subject-prefix="PATCH v4.1 02/10" --to <to> --in-reply-to <message-id>
>
> where message-id can be found in the headers of the message you want to reply to (header Message-ID). This allows the message to be correctly threaded. It means the patch will be in a separate email than your response to the reviewer's comments, but I think that's fine.
>
> Anyway, this particular one wasn't too difficult to fix up by hand. It LGTM with one nit fixed:
>
Thanks for the tip
>> diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
>> index
>> 262d03c0785f48e83b784b6177c52e2d253a6067..1f7861fd98dd9085c3fe9d7ee4b8396999060265
>> 100644
>> --- a/gdb/regformats/regdef.h
>> +++ b/gdb/regformats/regdef.h
>> @@ -21,6 +21,18 @@
>> struct reg
>> {
>> + reg ()
>> + : name (""),
>> + offset (0),
>> + size (0)
>> + {}
>> +
>> + reg (const char *_name, int _offset, int _size)
>
> The offset value is always 0 initially, so you can remove it and initialize it to 0.
>
Reason I added offset here was that in "Commonise tdesc_reg” this code will get merged into init_target_desc().
At that point I’ll be creating a reg and setting and offset at the same time.
This was just so that I didn’t need to touch regdef.h again.
I can still remove offset from this patch if you want - given that I’m not using it yet?
> I think that this patch can also be pushed on its own, it's a good improvement regardless of the rest of the series.
>
> Thanks,
>
> Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-23 16:52 ` Alan Hayward
@ 2018-03-23 17:04 ` Simon Marchi
2018-03-26 10:50 ` Alan Hayward
0 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2018-03-23 17:04 UTC (permalink / raw)
To: Alan Hayward; +Cc: Simon Marchi, gdb-patches, nd
On 2018-03-23 12:52, Alan Hayward wrote:
>> The offset value is always 0 initially, so you can remove it and
>> initialize it to 0.
>>
>
> Reason I added offset here was that in "Commonise tdesc_reg†this code
> will get merged into init_target_desc().
> At that point I’ll be creating a reg and setting and offset at the same
> time.
> This was just so that I didn’t need to touch regdef.h again.
>
> I can still remove offset from this patch if you want - given that I’m
> not using it yet?
This it not terribly important, but I think it's better to only add it
once you really need it. The proposed patches could change, and the
parameter could end up unused (not saying that's what will happen here,
it's just an example).
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 03/10] Commonise tdesc_reg
2018-03-22 8:45 ` [PATCH v4 03/10] Commonise tdesc_reg alan.hayward
@ 2018-03-23 19:50 ` Simon Marchi
2018-03-26 10:50 ` Alan Hayward
0 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2018-03-23 19:50 UTC (permalink / raw)
To: alan.hayward, gdb-patches; +Cc: nd
On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
> From: Alan Hayward <alan.hayward@arm.com>
>
> This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
>
> gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
> a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.
>
> However, other modules in gdbserver directly access the reg_defs structure.
> To work around this, init_target_desc fills in reg_defs by parsing the
> tdesc_reg vector.
> The long term goal is to remove reg_defs, replacing with accessor funcs.
>
> I wanted to make tdesc_create_reg common, but I cannot do that until
> the next patch.
>
> I also had to commonise tdesc_element_visitor and tdesc_element.
>
> This patch only differs from the V3 version in init_target_desc(),
> where I now use reg_def references as introduced in the previous patch.
Hi Alan,
I have just a few superficial comments because I don't have the big picture
yet, I'll continue with the next patches and might come back.
> 2018-03-21 Alan Hayward <alan.hayward@arm.com>
>
> gdb/
> * Makefile.in: Add arch/tdesc.c
> * common/tdesc.c: New file.
> * common/tdesc.h (tdesc_element_visitor): Move to here.
> (tdesc_element): Likewise.
> (tdesc_reg): Likewise.
> (tdesc_reg_up): Likewise.
> * target-descriptions.c: (tdesc_element_visitor): Move from here.
Spurious colon after the filename.
> (tdesc_element): Likewise.
> (tdesc_reg): Likewise.
> (tdesc_reg_up): Likewise.
>
> gdbserver/
> * Makefile.in: Add common/tdesc.c
> * tdesc.c (init_target_desc): init all reg_defs from register vector.
> (tdesc_create_reg): Create tdesc_reg.
> * tdesc.h (tdesc_feature): Add register vector.
> ---
> gdb/Makefile.in | 2 +
> gdb/common/tdesc.c | 40 ++++++++++++++++
> gdb/common/tdesc.h | 111 ++++++++++++++++++++++++++++++++++++++++++++
> gdb/gdbserver/Makefile.in | 3 ++
> gdb/gdbserver/tdesc.c | 53 ++++++++++++---------
> gdb/gdbserver/tdesc.h | 5 +-
> gdb/target-descriptions.c | 116 ----------------------------------------------
> 7 files changed, 191 insertions(+), 139 deletions(-)
> create mode 100644 gdb/common/tdesc.c
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 44db1937b6..f1d2f0193a 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -966,6 +966,7 @@ COMMON_SFILES = \
> common/run-time-clock.c \
> common/signals.c \
> common/signals-state-save-restore.c \
> + common/tdesc.c \
> common/vec.c \
> common/xml-utils.c \
> complaints.c \
> @@ -1443,6 +1444,7 @@ HFILES_NO_SRCDIR = \
> common/run-time-clock.h \
> common/signals-state-save-restore.h \
> common/symbol.h \
> + common/tdesc.h \
> common/vec.h \
> common/version.h \
> common/x86-xstate.h \
> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
> new file mode 100644
> index 0000000000..a5f2de4b5b
> --- /dev/null
> +++ b/gdb/common/tdesc.c
> @@ -0,0 +1,40 @@
> +/* Target description support for GDB.
> +
> + Copyright (C) 2017 Free Software Foundation, Inc.
2018
> +
> + 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
We are aiming to remove all uses of #ifdef GDBSERVER in the code base, so we shouldn't
add any. You might want to read this if you haven't seen it already:
https://sourceware.org/gdb/wiki/Common
Replace this with #include "common-defs.h". You can also remove the same snippet in
common/tdesc.h. Header files assume that defs.h/server.h/common-defs.h has been
included already, so they don't need to do it.
> @@ -236,26 +261,10 @@ 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;
> - int current_size = tdesc->reg_defs.size ();
> -
> - tdesc->reg_defs.resize (regnum != 0 ? regnum + 1 : current_size + 1);
> -
> - while (current_size < regnum)
> - {
> - struct reg *reg = &tdesc->reg_defs[current_size];
> - reg->name = "";
> - reg->size = 0;
> - current_size++;
> - }
> -
> - gdb_assert (regnum == 0
> - || regnum + 1 == tdesc->reg_defs.size ());
> -
> - struct reg *reg = &tdesc->reg_defs.back ();
> + tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
> + group, bitsize, type);
>
> - reg->name = name;
> - reg->size = bitsize;
> + tdesc->registers.emplace_back (reg);
This doesn't build, I guess it should be feature->...
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 05/10] Commonise tdesc types
2018-03-22 8:45 ` [PATCH v4 05/10] Commonise tdesc types alan.hayward
@ 2018-03-23 20:12 ` Simon Marchi
0 siblings, 0 replies; 25+ messages in thread
From: Simon Marchi @ 2018-03-23 20:12 UTC (permalink / raw)
To: alan.hayward, gdb-patches; +Cc: nd
On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
> From: Alan Hayward <alan.hayward@arm.com>
>
> This patch moves all the various tdesc types to common code.
>
> It also commonises all the tdesc_ functions that are stubbed
> out in gdbserver.
>
> This is a lot of code to move across, but it is the simplest
> way of getting gdbserver to retain the target description
> information.
>
> With this patch, gdb and gdbserver will now parse a target
> description in the same way.
>
> Identical to V3 version.
>
> Alan.
Just a nit, some functions in common/tdesc.c have the
/* See common/tdesc.h. */
comment, and some don't.
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 08/10] Create xml from target descriptions
2018-03-22 8:45 ` [PATCH v4 08/10] Create xml from target descriptions alan.hayward
@ 2018-03-23 21:24 ` Simon Marchi
2018-03-26 10:52 ` Alan Hayward
0 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2018-03-23 21:24 UTC (permalink / raw)
To: alan.hayward, gdb-patches; +Cc: nd
On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
> index 8ab77e365f..45eb24ea2b 100644
> --- a/gdb/common/tdesc.h
> +++ b/gdb/common/tdesc.h
> @@ -371,4 +371,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_)
I made a suggestion earlier that we don't use non-const references parameters
(I did not get any feedback yet though):
https://sourceware.org/ml/gdb-patches/2018-03/msg00145.html
Would you mind changing this parameter for a pointer?
> static void
> maint_print_c_tdesc_cmd (const char *args, int from_tty)
> {
> @@ -1760,7 +1777,36 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
> = file_read_description_xml (tdesc_xml.data ());
>
> if (tdesc == NULL || *tdesc != *e.second)
> - failed++;
> + {
> + printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
> + failed++;
> + continue;
> + }
> +
> + /* Convert both descriptions to xml, and then back again. Confirm all
> + descriptions are identical. */
> +
> + const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
> + const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
Instead of casting away the const, tdesc_get_features_xml should probably take
a const target_desc *. Because it's just used as some kind of cache/memoization,
the xmltarget field of target_desc can be made mutable (changing its value doesn't
really change the value of the target_desc).
> + gdb_assert (*xml == '@');
> + gdb_assert (*xml2 == '@');
> + const target_desc *t_trans = target_read_description_xml_string (xml+1);
> + const target_desc *t_trans2 = target_read_description_xml_string (xml2+1);
Spaces around the +.
Can you try to find better names than xml and xml2?
Instead of doing everything twice, maybe it would be clearer to have a separate
function that takes a single target_desc and converts it to xml and back, and
verifies that the resulting target_desc is equal to the initial one. You can
then call this function twice.
But stepping back a little bit, is it relevant to do this test on both target_desc,
even after we check that they are equal? Maybe it is, I'm just asking :)
> +
> + 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.c b/gdb/xml-tdesc.c
> index 9190d5f3c6..f793f07c96 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. */
Should be /* See xml-tdesc.h. */
> +
> +const struct target_desc *
> +target_read_description_xml_string (const char *xml_str)
I think this function is misnamed. If you look at the other similar functions,
the prefix (target_, file_) refers to the source of the file. So to follow the
same convention, this function could be named string_read_description_xml.
> +{
> + return tdesc_parse_xml (xml_str, nullptr, nullptr);
Instead of passing nullptr for fetcher, could you pass a dummy function that
just errors out? If it ever happens, it will be more graceful than a segfault.
You can use a lambda like this:
const struct target_desc *
target_read_description_xml_string (const char *xml_str)
{
return tdesc_parse_xml (xml_str, [] (const char *href, void *baton) {
error (_("xincludes are unsupported with this method"));
return gdb::unique_xmalloc_ptr<char> ();
}, nullptr);
}
Thanks,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 00/10] Remove gdbserver dependency on xml files
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
` (9 preceding siblings ...)
2018-03-22 8:46 ` [PATCH v4 10/10] Remove xml files from gdbserver alan.hayward
@ 2018-03-24 2:06 ` Simon Marchi
2018-03-26 10:55 ` Alan Hayward
10 siblings, 1 reply; 25+ messages in thread
From: Simon Marchi @ 2018-03-24 2:06 UTC (permalink / raw)
To: alan.hayward, gdb-patches; +Cc: nd
On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
> From: Alan Hayward <alan.hayward@arm.com>
>
> V4 addresses Philipps review comments. I'm fairly confident the issues he
> found on s390 I reproduced on arm32, and have now fixed up - the code now
> ensures xml is not generated for targets using older style descriptions.
>
> Using git sendmail for the first time, which should sort out the formatting
> issues I've had previously. However, just to the safe I've also pushed to
> my patches to the remote branch users/ahayward/xml4.
>
> This set adds two new patches to handle when to generate xml (fixing s390
> issues) and the reg_defs vector change.
>
> Summary:
>
> For those targets that use new style target descriptions, this set of patches
> removes the dependency on xml files. Namely:
> * Removes inclusion of xml files within gdbserver.
> * Removes the requirement for the .c files in features/ to be generated from
> cached xml files.
> This is made possible by changing xml descriptions generated by gdbserver, so
> that instead of including xml file names, gdbserver now generate a complete
> xml description.
>
> The second point will be required for aarch64 SVE support, where the register
> size are variable. Creating SVE xml files for every possible vector length
> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
> features/ .c code that would normally be generated from xml.
>
> Targets which use the older style target descriptions have not been changed.
>
>
> XML Generation:
>
> In existing code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, the
> function tdesc_get_features_xml () creates an xml containing the name of the
> original xml file(s). For example:
>
> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> <target>
> <architecture>i386</architecture>
> <osabi>GNU/Linux</osabi>
> <xi:include href="32bit-core.xml"/>
> <xi:include href="32bit-sse.xml"/>
> <xi:include href="32bit-linux.xml"/>
> <xi:include href="32bit-avx.xml"/>
> </target>
>
> Upon receipt, GDB then makes requests to gdbserver for the contents of the
> xml files. Gdbserver keeps full copies all the xml files inside the binary.
>
> This patch series adds common code that allows gdbserver (and gdb) to turn
> a C target description structure into xml.
> Now when asked fort an xml description to gdb, gdbserver turns the entire
> target description structure back into xml, without using any cached files.
> Producing, for example:
>
> <!DOCTYPE target SYSTEM "gdb-target.dtd">
> <target>
> <architecture>i386</architecture>
> <osabi>GNU/Linux</osabi>
> <feature name="org.gnu.gdb.i386.core">
> <flags id="i386_eflags" size="4">
> <field name="CF" start="0" end="0"/>
> <field name="" start="1" end="1"/>
> <field name="PF" start="2" end="2"/>
> <field name="AF" start="4" end="4"/>
> ...etc...
>
>
> Patch Contents:
>
> Patches 3-5 commonise the various target descriptor functionality, allowing
> gdbserver to parse target descriptions in the same way as gdb. This series
> does not commonise target_desc, but this is hopefully a long term goal.
>
> The eighth patch adds the xml printer, which iterates through the parsing
> generated in the previous patches.
>
> The other patches are clean up patches.
>
>
>
> Patches have been tested on a make check on x86 targets=all build with
> target boards unix and native-gdbserver. Also built and tested aarch64 and
> Arm32 (which uses old style descriptions)
> In addition, patch six adds new test cases to unit test.
Hi Alan,
I went through the whole series, so I am a bit more familiar with the problem now.
I often see the terms "old" and "new" style target descriptions, but I am not
really familiar with the differences. Reading this
https://sourceware.org/gdb/wiki/TargetDescription
this is what I understand:
- old: An entire target description is pre-generated (as C code) for each possible
configuration, possibly leading to a combinatorial explosion if there are many
optional features.
- new: Each feature is independently generated (as C code) and a hand-written function
manually assembles the final target description at runtime, adding the necessary
features based on the CPU features.
Is that right, and is there anything more to it?
Also, more long term-ish question, I never really quite understood the need for the
regformats/*.dat step. Couldn't we directly go from XML to the generated C files when
building gdb/gdbserver?
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 03/10] Commonise tdesc_reg
2018-03-23 19:50 ` Simon Marchi
@ 2018-03-26 10:50 ` Alan Hayward
0 siblings, 0 replies; 25+ messages in thread
From: Alan Hayward @ 2018-03-26 10:50 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, nd
> On 23 Mar 2018, at 19:50, Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
>> From: Alan Hayward <alan.hayward@arm.com>
>>
>> This patch commonises tdesc_reg and makes use of it in gdbserver tdesc.
>>
>> gdbserver tdesc_create_reg is changed to create a tdesc_reg instead of
>> a reg_defs entry. The vector of tdesc_reg is held inside tdesc_feature.
>>
>> However, other modules in gdbserver directly access the reg_defs structure.
>> To work around this, init_target_desc fills in reg_defs by parsing the
>> tdesc_reg vector.
>> The long term goal is to remove reg_defs, replacing with accessor funcs.
>>
>> I wanted to make tdesc_create_reg common, but I cannot do that until
>> the next patch.
>>
>> I also had to commonise tdesc_element_visitor and tdesc_element.
>>
>> This patch only differs from the V3 version in init_target_desc(),
>> where I now use reg_def references as introduced in the previous patch.
>
> Hi Alan,
>
> I have just a few superficial comments because I don't have the big picture
> yet, I'll continue with the next patches and might come back.
>
>> 2018-03-21 Alan Hayward <alan.hayward@arm.com>
>>
>> gdb/
>> * Makefile.in: Add arch/tdesc.c
>> * common/tdesc.c: New file.
>> * common/tdesc.h (tdesc_element_visitor): Move to here.
>> (tdesc_element): Likewise.
>> (tdesc_reg): Likewise.
>> (tdesc_reg_up): Likewise.
>> * target-descriptions.c: (tdesc_element_visitor): Move from here.
>
> Spurious colon after the filename.
>
>> (tdesc_element): Likewise.
>> (tdesc_reg): Likewise.
>> (tdesc_reg_up): Likewise.
>>
>> gdbserver/
>> * Makefile.in: Add common/tdesc.c
>> * tdesc.c (init_target_desc): init all reg_defs from register vector.
>> (tdesc_create_reg): Create tdesc_reg.
>> * tdesc.h (tdesc_feature): Add register vector.
>> ---
>> gdb/Makefile.in | 2 +
>> gdb/common/tdesc.c | 40 ++++++++++++++++
>> gdb/common/tdesc.h | 111 ++++++++++++++++++++++++++++++++++++++++++++
>> gdb/gdbserver/Makefile.in | 3 ++
>> gdb/gdbserver/tdesc.c | 53 ++++++++++++---------
>> gdb/gdbserver/tdesc.h | 5 +-
>> gdb/target-descriptions.c | 116 ----------------------------------------------
>> 7 files changed, 191 insertions(+), 139 deletions(-)
>> create mode 100644 gdb/common/tdesc.c
>>
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index 44db1937b6..f1d2f0193a 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -966,6 +966,7 @@ COMMON_SFILES = \
>> common/run-time-clock.c \
>> common/signals.c \
>> common/signals-state-save-restore.c \
>> + common/tdesc.c \
>> common/vec.c \
>> common/xml-utils.c \
>> complaints.c \
>> @@ -1443,6 +1444,7 @@ HFILES_NO_SRCDIR = \
>> common/run-time-clock.h \
>> common/signals-state-save-restore.h \
>> common/symbol.h \
>> + common/tdesc.h \
>> common/vec.h \
>> common/version.h \
>> common/x86-xstate.h \
>> diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
>> new file mode 100644
>> index 0000000000..a5f2de4b5b
>> --- /dev/null
>> +++ b/gdb/common/tdesc.c
>> @@ -0,0 +1,40 @@
>> +/* Target description support for GDB.
>> +
>> + Copyright (C) 2017 Free Software Foundation, Inc.
>
> 2018
>
>> +
>> + 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
>
> We are aiming to remove all uses of #ifdef GDBSERVER in the code base, so we shouldn't
> add any. You might want to read this if you haven't seen it already:
>
> https://sourceware.org/gdb/wiki/Common
>
> Replace this with #include "common-defs.h". You can also remove the same snippet in
> common/tdesc.h. Header files assume that defs.h/server.h/common-defs.h has been
> included already, so they don't need to do it.
Aha! I was trying to find a better way of doing those includes. That was the best I could reduce
it down to. Will update, and read through the links on that wiki page.
>
>> @@ -236,26 +261,10 @@ 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;
>> - int current_size = tdesc->reg_defs.size ();
>> -
>> - tdesc->reg_defs.resize (regnum != 0 ? regnum + 1 : current_size + 1);
>> -
>> - while (current_size < regnum)
>> - {
>> - struct reg *reg = &tdesc->reg_defs[current_size];
>> - reg->name = "";
>> - reg->size = 0;
>> - current_size++;
>> - }
>> -
>> - gdb_assert (regnum == 0
>> - || regnum + 1 == tdesc->reg_defs.size ());
>> -
>> - struct reg *reg = &tdesc->reg_defs.back ();
>> + tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
>> + group, bitsize, type);
>>
>> - reg->name = name;
>> - reg->size = bitsize;
>> + tdesc->registers.emplace_back (reg);
>
> This doesn't build, I guess it should be feature->…
>
Yeah, after the 02 changes this patch needs a little bit of a rewrite.
I’m going to be away from a pc for two weeks now. Will update this (and part 4 too),
when I get back.
Alan.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects
2018-03-23 17:04 ` Simon Marchi
@ 2018-03-26 10:50 ` Alan Hayward
0 siblings, 0 replies; 25+ messages in thread
From: Alan Hayward @ 2018-03-26 10:50 UTC (permalink / raw)
To: Simon Marchi; +Cc: Simon Marchi, gdb-patches, nd
> On 23 Mar 2018, at 17:04, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
> On 2018-03-23 12:52, Alan Hayward wrote:
>>> The offset value is always 0 initially, so you can remove it and initialize it to 0.
>> Reason I added offset here was that in "Commonise tdesc_reg” this code
>> will get merged into init_target_desc().
>> At that point I’ll be creating a reg and setting and offset at the same time.
>> This was just so that I didn’t need to touch regdef.h again.
>> I can still remove offset from this patch if you want - given that I’m
>> not using it yet?
>
> This it not terribly important, but I think it's better to only add it once you really need it. The proposed patches could change, and the parameter could end up unused (not saying that's what will happen here, it's just an example).
>
> Simon
Ok, pushed as suggested with offset initialised to 0.
Diff for reference:
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index d6511fda650ca52688cd4d7db1acd94e822a3c0d..cbdf766df2c2b95f605198c617ffead9f9ac3775 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -196,9 +196,9 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
dst->registers_valid = src->registers_valid;
}
-/* Return a pointer to the description of register N. */
+/* Return a reference to the description of register N. */
-static const struct reg *
+static const struct reg &
find_register_by_number (const struct target_desc *tdesc, int n)
{
return tdesc->reg_defs[n];
@@ -251,7 +251,7 @@ find_regno (const struct target_desc *tdesc, const char *name)
{
for (int i = 0; i < tdesc->reg_defs.size (); ++i)
{
- if (strcmp (name, find_register_by_number (tdesc, i)->name) == 0)
+ if (strcmp (name, find_register_by_number (tdesc, i).name) == 0)
return i;
}
internal_error (__FILE__, __LINE__, "Unknown register %s requested",
@@ -288,7 +288,7 @@ register_cache_size (const struct target_desc *tdesc)
int
register_size (const struct target_desc *tdesc, int n)
{
- return find_register_by_number (tdesc, n)->size / 8;
+ return find_register_by_number (tdesc, n).size / 8;
}
/* See common/common-regcache.h. */
@@ -303,7 +303,7 @@ static unsigned char *
register_data (struct regcache *regcache, int n, int fetch)
{
return (regcache->registers
- + find_register_by_number (regcache->tdesc, n)->offset / 8);
+ + find_register_by_number (regcache->tdesc, n).offset / 8);
}
/* Supply register N, whose contents are stored in BUF, to REGCACHE.
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 4513ea74232a456cc86eb9a655904012ff117373..a62544341cd23a9a8ec6833e1eae73616a315d2d 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -34,7 +34,7 @@ struct target_desc : tdesc_feature
{
/* A vector of elements of register definitions that
describe the inferior's register set. */
- std::vector<struct reg *> reg_defs;
+ std::vector<struct reg> reg_defs;
/* The register cache size, in bytes. */
int registers_size;
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e50a848e2f9f280a84ab139cfce4d1f17bd05884..cec7a66f9748f7295462c76fdae3d3e9029ee421 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -25,9 +25,6 @@ target_desc::~target_desc ()
{
int i;
- for (reg *reg : reg_defs)
- xfree (reg);
-
xfree ((char *) arch);
xfree ((char *) osabi);
@@ -40,18 +37,9 @@ target_desc::~target_desc ()
bool target_desc::operator== (const target_desc &other) const
{
- if (reg_defs.size () != other.reg_defs.size ())
+ if (reg_defs != other.reg_defs)
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++)
@@ -72,10 +60,10 @@ init_target_desc (struct target_desc *tdesc)
{
int offset = 0;
- for (reg *reg : tdesc->reg_defs)
+ for (reg ® : tdesc->reg_defs)
{
- reg->offset = offset;
- offset += reg->size;
+ reg.offset = offset;
+ offset += reg.size;
}
tdesc->registers_size = offset / 8;
@@ -241,23 +229,12 @@ 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 ());
+ gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());
- struct reg *reg = XCNEW (struct reg);
+ if (regnum != 0)
+ tdesc->reg_defs.resize (regnum);
- reg->name = name;
- reg->size = bitsize;
- tdesc->reg_defs.push_back (reg);
+ tdesc->reg_defs.emplace_back (name, bitsize);
}
/* See common/tdesc.h. */
diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
index 262d03c0785f48e83b784b6177c52e2d253a6067..4775e863e9acc516c0d0ab90baf428604eb967c4 100644
--- a/gdb/regformats/regdef.h
+++ b/gdb/regformats/regdef.h
@@ -21,6 +21,18 @@
struct reg
{
+ reg ()
+ : name (""),
+ offset (0),
+ size (0)
+ {}
+
+ reg (const char *_name, int _size)
+ : name (_name),
+ offset (0),
+ size (_size)
+ {}
+
/* The name of this register - NULL for pad entries. */
const char *name;
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 08/10] Create xml from target descriptions
2018-03-23 21:24 ` Simon Marchi
@ 2018-03-26 10:52 ` Alan Hayward
0 siblings, 0 replies; 25+ messages in thread
From: Alan Hayward @ 2018-03-26 10:52 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, nd
> On 23 Mar 2018, at 21:24, Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
>> diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h
>> index 8ab77e365f..45eb24ea2b 100644
>> --- a/gdb/common/tdesc.h
>> +++ b/gdb/common/tdesc.h
>> @@ -371,4 +371,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_)
>
> I made a suggestion earlier that we don't use non-const references parameters
> (I did not get any feedback yet though):
>
> https://sourceware.org/ml/gdb-patches/2018-03/msg00145.html
>
> Would you mind changing this parameter for a pointer?
Sorry, missed that. Will do.
>
>> static void
>> maint_print_c_tdesc_cmd (const char *args, int from_tty)
>> {
>> @@ -1760,7 +1777,36 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty)
>> = file_read_description_xml (tdesc_xml.data ());
>>
>> if (tdesc == NULL || *tdesc != *e.second)
>> - failed++;
>> + {
>> + printf_filtered ( _("Descriptions for %s do not match\n"), e.first);
>> + failed++;
>> + continue;
>> + }
>> +
>> + /* Convert both descriptions to xml, and then back again. Confirm all
>> + descriptions are identical. */
>> +
>> + const char *xml = tdesc_get_features_xml ((target_desc *) tdesc);
>> + const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second);
>
> Instead of casting away the const, tdesc_get_features_xml should probably take
> a const target_desc *. Because it's just used as some kind of cache/memoization,
> the xmltarget field of target_desc can be made mutable (changing its value doesn't
> really change the value of the target_desc).
The mutable keyword is exactly what I was looking for here!
>
>> + gdb_assert (*xml == '@');
>> + gdb_assert (*xml2 == '@');
>> + const target_desc *t_trans = target_read_description_xml_string (xml+1);
>> + const target_desc *t_trans2 = target_read_description_xml_string (xml2+1);
>
> Spaces around the +.
>
> Can you try to find better names than xml and xml2?
>
> Instead of doing everything twice, maybe it would be clearer to have a separate
> function that takes a single target_desc and converts it to xml and back, and
> verifies that the resulting target_desc is equal to the initial one. You can
> then call this function twice.
>
Will do.
> But stepping back a little bit, is it relevant to do this test on both target_desc,
> even after we check that they are equal? Maybe it is, I'm just asking :)
>
If this code was executed as part of a general run of gdb, then I’d agree and
move one of them out. However, given this is part of test the unit tests and it’s not
slowing anything down then I’m more inclined to leave it in. Maybe if someone
added new tdesc fields and didn’t include them correctly in the equals operator
and/or xml generation. (Although I see the argument for removing it to simply
the code).
>> +
>> + 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.c b/gdb/xml-tdesc.c
>> index 9190d5f3c6..f793f07c96 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. */
>
> Should be /* See xml-tdesc.h. */
Ok.
>
>> +
>> +const struct target_desc *
>> +target_read_description_xml_string (const char *xml_str)
>
> I think this function is misnamed. If you look at the other similar functions,
> the prefix (target_, file_) refers to the source of the file. So to follow the
> same convention, this function could be named string_read_description_xml.
Ok.
>
>> +{
>> + return tdesc_parse_xml (xml_str, nullptr, nullptr);
>
> Instead of passing nullptr for fetcher, could you pass a dummy function that
> just errors out? If it ever happens, it will be more graceful than a segfault.
> You can use a lambda like this:
>
> const struct target_desc *
> target_read_description_xml_string (const char *xml_str)
> {
> return tdesc_parse_xml (xml_str, [] (const char *href, void *baton) {
> error (_("xincludes are unsupported with this method"));
> return gdb::unique_xmalloc_ptr<char> ();
> }, nullptr);
> }
>
Will do.
Will update when I get back in two weeks.
Alan.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 00/10] Remove gdbserver dependency on xml files
2018-03-24 2:06 ` [PATCH v4 00/10] Remove gdbserver dependency on xml files Simon Marchi
@ 2018-03-26 10:55 ` Alan Hayward
0 siblings, 0 replies; 25+ messages in thread
From: Alan Hayward @ 2018-03-26 10:55 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, nd
> On 24 Mar 2018, at 02:06, Simon Marchi <simark@simark.ca> wrote:
>
> On 2018-03-22 04:44 AM, alan.hayward@arm.com wrote:
>> From: Alan Hayward <alan.hayward@arm.com>
>>
>> V4 addresses Philipps review comments. I'm fairly confident the issues he
>> found on s390 I reproduced on arm32, and have now fixed up - the code now
>> ensures xml is not generated for targets using older style descriptions.
>>
>> Using git sendmail for the first time, which should sort out the formatting
>> issues I've had previously. However, just to the safe I've also pushed to
>> my patches to the remote branch users/ahayward/xml4.
>>
>> This set adds two new patches to handle when to generate xml (fixing s390
>> issues) and the reg_defs vector change.
>>
>> Summary:
>>
>> For those targets that use new style target descriptions, this set of patches
>> removes the dependency on xml files. Namely:
>> * Removes inclusion of xml files within gdbserver.
>> * Removes the requirement for the .c files in features/ to be generated from
>> cached xml files.
>> This is made possible by changing xml descriptions generated by gdbserver, so
>> that instead of including xml file names, gdbserver now generate a complete
>> xml description.
>>
>> The second point will be required for aarch64 SVE support, where the register
>> size are variable. Creating SVE xml files for every possible vector length
>> would not be feasible. Instead the plan for aarch64 SVE is to hand write the
>> features/ .c code that would normally be generated from xml.
>>
>> Targets which use the older style target descriptions have not been changed.
>>
>>
>> XML Generation:
>>
>> In existing code, gdbserver uses C code auto generated from xml files to
>> create target descriptions. When sending an xml description to GDB, the
>> function tdesc_get_features_xml () creates an xml containing the name of the
>> original xml file(s). For example:
>>
>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
>> <target>
>> <architecture>i386</architecture>
>> <osabi>GNU/Linux</osabi>
>> <xi:include href="32bit-core.xml"/>
>> <xi:include href="32bit-sse.xml"/>
>> <xi:include href="32bit-linux.xml"/>
>> <xi:include href="32bit-avx.xml"/>
>> </target>
>>
>> Upon receipt, GDB then makes requests to gdbserver for the contents of the
>> xml files. Gdbserver keeps full copies all the xml files inside the binary.
>>
>> This patch series adds common code that allows gdbserver (and gdb) to turn
>> a C target description structure into xml.
>> Now when asked fort an xml description to gdb, gdbserver turns the entire
>> target description structure back into xml, without using any cached files.
>> Producing, for example:
>>
>> <!DOCTYPE target SYSTEM "gdb-target.dtd">
>> <target>
>> <architecture>i386</architecture>
>> <osabi>GNU/Linux</osabi>
>> <feature name="org.gnu.gdb.i386.core">
>> <flags id="i386_eflags" size="4">
>> <field name="CF" start="0" end="0"/>
>> <field name="" start="1" end="1"/>
>> <field name="PF" start="2" end="2"/>
>> <field name="AF" start="4" end="4"/>
>> ...etc...
>>
>>
>> Patch Contents:
>>
>> Patches 3-5 commonise the various target descriptor functionality, allowing
>> gdbserver to parse target descriptions in the same way as gdb. This series
>> does not commonise target_desc, but this is hopefully a long term goal.
>>
>> The eighth patch adds the xml printer, which iterates through the parsing
>> generated in the previous patches.
>>
>> The other patches are clean up patches.
>>
>>
>>
>> Patches have been tested on a make check on x86 targets=all build with
>> target boards unix and native-gdbserver. Also built and tested aarch64 and
>> Arm32 (which uses old style descriptions)
>> In addition, patch six adds new test cases to unit test.
>
> Hi Alan,
>
> I went through the whole series, so I am a bit more familiar with the problem now.
> I often see the terms "old" and "new" style target descriptions, but I am not
> really familiar with the differences. Reading this
>
> https://sourceware.org/gdb/wiki/TargetDescription
>
> this is what I understand:
>
> - old: An entire target description is pre-generated (as C code) for each possible
> configuration, possibly leading to a combinatorial explosion if there are many
> optional features.
> - new: Each feature is independently generated (as C code) and a hand-written function
> manually assembles the final target description at runtime, adding the necessary
> features based on the CPU features.
>
> Is that right, and is there anything more to it?
>
Yeah, that’s the gist of it. I should probably have stuck to Yao’s term “flexible” instead
of “new”.
> Also, more long term-ish question, I never really quite understood the need for the
> regformats/*.dat step. Couldn't we directly go from XML to the generated C files when
> building gdb/gdbserver?
>
I think you could….not sure if you’d want to.
Today features Makefile is using a bunch of xsl files to turn the xml into a simple .dat format,
and adds in the expedite registers (hardcoded in the makefile).
regdat.sh turns the simple .dat format into generated C code.
Going straight from XML to generated C would be trickier. I suspect that doing it all in xsl files
would be horrible to debug every time you wanted to change the generated C code. There are
probably many other ways than using xsl. I guess it could be added into the xml parser already
inside gdb, but then that’s adding extra code into the production gdb. I’m not keen on the two
step process, but the .dat format is very simple to work with.
Alan.
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-03-26 10:55 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 8:44 [PATCH v4 00/10] Remove gdbserver dependency on xml files alan.hayward
2018-03-22 8:45 ` [PATCH v4 01/10] Move tdesc header funcs to c file alan.hayward
2018-03-22 20:43 ` Simon Marchi
2018-03-22 8:45 ` [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects alan.hayward
2018-03-22 21:33 ` Simon Marchi
2018-03-23 14:54 ` Alan Hayward
2018-03-23 15:36 ` Simon Marchi
2018-03-23 16:52 ` Alan Hayward
2018-03-23 17:04 ` Simon Marchi
2018-03-26 10:50 ` Alan Hayward
2018-03-22 8:45 ` [PATCH v4 04/10] Commonise tdesc_feature alan.hayward
2018-03-22 8:45 ` [PATCH v4 05/10] Commonise tdesc types alan.hayward
2018-03-23 20:12 ` Simon Marchi
2018-03-22 8:45 ` [PATCH v4 08/10] Create xml from target descriptions alan.hayward
2018-03-23 21:24 ` Simon Marchi
2018-03-26 10:52 ` Alan Hayward
2018-03-22 8:45 ` [PATCH v4 06/10] Add tdesc osabi and architecture functions alan.hayward
2018-03-22 8:45 ` [PATCH v4 07/10] Add feature reference in .dat files alan.hayward
2018-03-22 8:45 ` [PATCH v4 03/10] Commonise tdesc_reg alan.hayward
2018-03-23 19:50 ` Simon Marchi
2018-03-26 10:50 ` Alan Hayward
2018-03-22 8:46 ` [PATCH v4 09/10] Remove xml file references from target descriptions alan.hayward
2018-03-22 8:46 ` [PATCH v4 10/10] Remove xml files from gdbserver alan.hayward
2018-03-24 2:06 ` [PATCH v4 00/10] Remove gdbserver dependency on xml files Simon Marchi
2018-03-26 10:55 ` 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).