* [PATCH 0/2] Additional maintenance command for dumping target descriptions
@ 2020-06-11 10:41 Andrew Burgess
2020-06-11 10:41 ` [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Andrew Burgess @ 2020-06-11 10:41 UTC (permalink / raw)
To: gdb-patches
After some fun and games debugging some target description issues I
finally got fed up enough to implement something that I've wanted for
a while. A mechanism to ask GDB what target description was sent from
the target.
Patch #1 was a quick hack to `maint print c-tdesc', which was easy,
and did give me the information I needed, but the output format isn't
great for what I wanted, so
Patch #2 allows GDB developers to see in XML form the description that
was sent from the target.
Feedback welcome.
Thanks,
Andrew
---
Andrew Burgess (2):
gdb: Allow target description to be dumped even when it is remote
gdb: New maintenance command to print XML target description
gdb/ChangeLog | 17 +++
gdb/NEWS | 6 +
gdb/doc/ChangeLog | 5 +
gdb/doc/gdb.texinfo | 9 ++
gdb/target-descriptions.c | 102 +++++++++++++++-
gdb/testsuite/ChangeLog | 6 +
gdb/testsuite/gdb.xml/maint-xml-dump-01.xml | 10 ++
gdb/testsuite/gdb.xml/maint-xml-dump-02.xml | 27 +++++
gdb/testsuite/gdb.xml/maint-xml-dump.exp | 124 ++++++++++++++++++++
gdbsupport/ChangeLog | 13 ++
gdbsupport/tdesc.cc | 99 +++++++++++-----
gdbsupport/tdesc.h | 15 +++
12 files changed, 399 insertions(+), 34 deletions(-)
create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump.exp
--
2.25.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote
2020-06-11 10:41 [PATCH 0/2] Additional maintenance command for dumping target descriptions Andrew Burgess
@ 2020-06-11 10:41 ` Andrew Burgess
2020-06-11 11:33 ` Pedro Alves
2020-06-11 10:41 ` [PATCH 2/2] gdb: New maintenance command to print XML target description Andrew Burgess
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2020-06-11 10:41 UTC (permalink / raw)
To: gdb-patches
The maintenance command 'maintenance print c-tdesc' can only print the
target description if it was loaded from a local file, or if the local
filename is passed to the maintenance command as an argument.
Sometimes it would be nice to know what target description GDB was
given by the remote, however, if I connect to a remote target and try
this command I see this:
(gdb) maintenance print c-tdesc
The current target description did not come from an XML file.
(gdb)
Which is not very helpful.
This commit changes things so that if the description came from the
remote end then GDB will use the fake filename 'target.xml' as the
filename for the description, GDB will then create the C description
of the target as if it was in a file 'target.xml'.
I originally added this functionality so I could inspect the
description passed to GDB by the remote target. After using this for
a while I realised that actually having GDB recreate the XML would be
even better, so a later commit will add that functionality too.
Still, given how small this patch is I thought it might be nice to
include this in GDB anyway.
While I was working on this anyway I've added filename command
completion to this command.
gdb/ChangeLog:
* target-descriptions.c (maint_print_c_tdesc_cmd): Use fake
filename for target descriptions that came from the target.
(_initialize_target_descriptions): Add filename command completion
for 'maint print c-tdesc'.
---
gdb/ChangeLog | 7 +++++++
gdb/target-descriptions.c | 14 ++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 20a3a640f4f..55ea416d69a 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1680,7 +1680,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
error (_("There is no target description to print."));
if (filename == NULL)
- error (_("The current target description did not come from an XML file."));
+ {
+ printf_unfiltered (_("The current target description was fetched "
+ "from the target, using\n'target.xml' as a fake "
+ "filename.\n\n"));
+ filename = "target.xml";
+ }
std::string filename_after_features (filename);
auto loc = filename_after_features.rfind ("/features/");
@@ -1811,6 +1816,8 @@ void _initialize_target_descriptions ();
void
_initialize_target_descriptions ()
{
+ cmd_list_element *cmd;
+
tdesc_data = gdbarch_data_register_pre_init (tdesc_data_init);
add_basic_prefix_cmd ("tdesc", class_maintenance, _("\
@@ -1842,11 +1849,10 @@ Unset the file to read for an XML target description.\n\
When unset, GDB will read the description from the target."),
&tdesc_unset_cmdlist);
- add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
+ cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
Print the current target description as a C source file."),
&maintenanceprintlist);
-
- cmd_list_element *cmd;
+ set_cmd_completer (cmd, filename_completer);
cmd = add_cmd ("xml-descriptions", class_maintenance,
maintenance_check_xml_descriptions, _("\
--
2.25.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] gdb: New maintenance command to print XML target description
2020-06-11 10:41 [PATCH 0/2] Additional maintenance command for dumping target descriptions Andrew Burgess
2020-06-11 10:41 ` [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
@ 2020-06-11 10:41 ` Andrew Burgess
2020-06-11 13:10 ` Pedro Alves
2020-06-11 13:27 ` Eli Zaretskii
2020-06-11 19:50 ` [PATCH 0/2] Additional maintenance command for dumping target descriptions Christian Biesinger
2020-06-16 20:51 ` Andrew Burgess
3 siblings, 2 replies; 10+ messages in thread
From: Andrew Burgess @ 2020-06-11 10:41 UTC (permalink / raw)
To: gdb-patches
This commit adds a new maintenance command that dumps the current
target description as an XML document. This is a maintenance command
as I currently only see this being useful for GDB developers, or for
people debugging a new remote target.
By default the command will print whatever the current target
description is, whether this was delivered by the remote, loaded by
the user from a file, or if it is a built in target within GDB.
The command can also take an optional filename argument. In this case
GDB loads a target description from the file, and then reprints it.
This could be useful for testing GDB's parsing of target descriptions,
or to check that GDB can successfully parse a particular XML
description.
It is worth noting that the XML description printed will not be an
exact copy of the document fed into GDB. For example this minimal
input file:
<target>
<feature name="abc">
<reg name="r1" bitsize="32"/>
</feature>
</target>
Will produce this output:
(gdb) maint print xml-tdesc path/to/file.xml
<?xml version="1.0"?>
<!DOCTYPE target SYSTEM "gdb-target.dtd">
<target>
<feature name="abc">
<reg name="r1" bitsize="32" type="int" regnum="0"/>
</feature>
</target>
Notice that GDB filled in both the 'type' and 'regnum' fields of the
<reg>. I think this is actually a positive as it means we get to
really understand how GDB processed the document, if GDB made some
assumptions that differ to those the user expected then hopefully this
will bring those issues to the users attention.
As for implementation I initially started writing a new tdesc visitor
class that produced XML, but after remembering to check gdbsupport/ I
found such a class already existed.
However, that class produced XML which seems designed for a machine to
read, with only minimal indentation. With a bit of tweaking I was
able to restructure things so I could inherit from the existing XML
producer and add indentation.
During this tweaking I did make two changes to the original XML
producing class, these are:
1. The <architecture>...</architecture> tags are now not produced if
the architecture name is NULL.
2. The <osabi>...</osabi> tags get a newline at the end.
gdbsupport/ChangeLog:
* tdesc.cc (print_xml_feature::visit_pre): Use add_line to add
output content, and call indent as needed in all overloaded
variants.
(print_xml_feature::visit_post): Likewise.
(print_xml_feature::visit): Likewise.
(print_xml_feature::add_line): Two new overloaded functions.
* tdesc.h (print_xml_feature::indent): Declare new member
function.
(print_xml_feature::add_line): Two new overloaded member
functions.
gdb/ChangeLog:
* target-descriptions.c (tdesc_architecture_name): Protect against
NULL pointer dereference.
(class gdb_print_xml_feature): New class.
(maint_print_xml_tdesc_cmd): New function.
(_initialize_target_descriptions): Register new 'maint print
xml-tdesc' command and give it the filename completer.
* NEWS: Mention new 'maint print xml-tdesc' command.
gdb/testsuite/ChangeLog:
* gdb.xml/maint-xml-dump-01.xml: New file.
* gdb.xml/maint-xml-dump-02.xml: New file.
* gdb.xml/maint-xml-dump.exp: New file.
gdb/doc/ChangeLog:
* gdb.texinfo (Maintenance Commands): Document new 'maint print
xml-desc' command.
---
gdb/ChangeLog | 10 ++
gdb/NEWS | 6 +
gdb/doc/ChangeLog | 5 +
gdb/doc/gdb.texinfo | 9 ++
gdb/target-descriptions.c | 90 +++++++++++++-
gdb/testsuite/ChangeLog | 6 +
gdb/testsuite/gdb.xml/maint-xml-dump-01.xml | 10 ++
gdb/testsuite/gdb.xml/maint-xml-dump-02.xml | 27 +++++
gdb/testsuite/gdb.xml/maint-xml-dump.exp | 124 ++++++++++++++++++++
gdbsupport/ChangeLog | 13 ++
gdbsupport/tdesc.cc | 99 +++++++++++-----
gdbsupport/tdesc.h | 15 +++
12 files changed, 383 insertions(+), 31 deletions(-)
create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index 9ef85ab3ca7..491209b2772 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -66,6 +66,12 @@ tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
Define a new TUI layout, specifying its name and the windows that
will be displayed.
+maintenance print xml-tdesc [FILE]
+ Prints the current target description as an XML document. If the
+ optional FILE is provided (which is an XML target description) then
+ the target description is read from FILE into GDB, and then
+ reprinted.
+
* New targets
GNU/Linux/RISC-V (gdbserver) riscv*-*-linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 64181628495..add14856f45 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38397,6 +38397,15 @@
built again. This command is used by developers after they add or
modify XML target descriptions.
+@kindex maint print xml-tdesc @r{[}@var{file}@r{]}
+@item maint print xml-tdesc
+Print the target description (@pxref{Target Descriptions}) as an XML
+file. By default print the target description for the current target,
+but if the optional argument @var{file} is provided, then that file is
+read in by GDB and then used to produce the description. The
+@var{file} should be an XML document, of the form described in
+@ref{Target Description Format}.
+
@kindex maint check xml-descriptions
@item maint check xml-descriptions @var{dir}
Check that the target descriptions dynamically created by @value{GDBN}
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 55ea416d69a..f86ffdeb518 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -639,7 +639,9 @@ tdesc_architecture (const struct target_desc *target_desc)
const char *
tdesc_architecture_name (const struct target_desc *target_desc)
{
- return target_desc->arch->printable_name;
+ if (target_desc->arch != NULL)
+ return target_desc->arch->printable_name;
+ return NULL;
}
/* Return the OSABI associated with this target description, or
@@ -1717,6 +1719,87 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
}
}
+/* Class to format the target description XML with nesting. */
+
+class gdb_print_xml_feature : public print_xml_feature
+{
+public:
+ /* Constructor. */
+
+ gdb_print_xml_feature (std::string *arg)
+ :print_xml_feature (arg)
+ { /* Nothing. */ }
+
+ /* Override this from the parent so we can add the compatibility
+ information. */
+
+ void visit_pre (const target_desc *e) override
+ {
+ print_xml_feature::visit_pre (e);
+ for (const bfd_arch_info_type *compatible : e->compatible)
+ add_line ("<compatible>%s</compatible>", compatible->printable_name);
+ };
+
+protected:
+
+ /* Pull in all copies of add_line. */
+
+ using print_xml_feature::add_line;
+
+ /* Override this version of add_line to add white space for indentation
+ at the start of the line. */
+
+ void add_line (const std::string &str) override
+ {
+ std::string tmp = string_printf ("%*s", m_depth, "");
+ tmp += str;
+ print_xml_feature::add_line (tmp);
+ }
+
+ /* Increase our nesting by ADJUST. We use two spaces for every level of
+ indentation. */
+
+ void indent (int adjust) override
+ {
+ m_depth += (adjust * 2);
+ }
+
+private:
+
+ /* The current level of indentation. */
+ int m_depth = 0;
+};
+
+/* Implement the maintenance print xml-tdesc command. */
+
+static void
+maint_print_xml_tdesc_cmd (const char *args, int from_tty)
+{
+ const struct target_desc *tdesc;
+
+ if (args == NULL)
+ {
+ /* Use the global target-supplied description, not the current
+ architecture's. This lets a GDB for one architecture generate C
+ for another architecture's description, even though the gdbarch
+ initialization code will reject the new description. */
+ tdesc = current_target_desc;
+ }
+ else
+ {
+ /* Use the target description from the XML file. */
+ tdesc = file_read_description_xml (args);
+ }
+
+ if (tdesc == NULL)
+ error (_("There is no target description to print."));
+
+ std::string buf;
+ gdb_print_xml_feature v (&buf);
+ tdesc->accept (v);
+ puts_unfiltered (buf.c_str ());
+}
+
namespace selftests {
/* A reference target description, used for testing (see record_xml_tdesc). */
@@ -1854,6 +1937,11 @@ Print the current target description as a C source file."),
&maintenanceprintlist);
set_cmd_completer (cmd, filename_completer);
+ cmd = add_cmd ("xml-tdesc", class_maintenance, maint_print_xml_tdesc_cmd, _("\
+Print the current target description as an XML file."),
+ &maintenanceprintlist);
+ set_cmd_completer (cmd, filename_completer);
+
cmd = add_cmd ("xml-descriptions", class_maintenance,
maintenance_check_xml_descriptions, _("\
Check equality of GDB target descriptions and XML created descriptions.\n\
diff --git a/gdb/testsuite/gdb.xml/maint-xml-dump-01.xml b/gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
new file mode 100644
index 00000000000..dd4f9251676
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<!-- This is a comment before DOCTYPE -->
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<!-- This is a comment after DOCTYPE -->
+<target>
+ <feature name="abc">
+ <!-- The following is a register. -->
+ <reg name="r1" bitsize="32"/> <!-- <reg name="r1" bitsize="32" type="int" regnum="0"/> -->
+ </feature>
+</target>
diff --git a/gdb/testsuite/gdb.xml/maint-xml-dump-02.xml b/gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
new file mode 100644
index 00000000000..c192294ca10
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
@@ -0,0 +1,27 @@
+<target>
+ <osabi>Solaris</osabi>
+ <feature name="abc">
+ <vector id="foo" type="int32" count="4"/>
+ <reg name="foo" bitsize="16" /> <!-- <reg name="foo" bitsize="16" type="int" regnum="0"/> -->
+ </feature>
+ <feature name="def.xyz">
+ <struct id="my_struct">
+ <field name="field1" type="int8"/>
+ <field name="field2" type="int16"/>
+ <field name="field3" type="int8"/>
+ </struct>
+ <struct id="bit_field" size="8">
+ <field name="bits1" start="0" end="3" type="int8"/>
+ <field name="bits2" start="4" end="6" type="int8"/>
+ <field name="bits3" start="7" end="7"/> <!-- <field name="bits3" start="7" end="7" type="bool"/> -->
+ </struct>
+ <flags id="my_flags" size="8">
+ <field name="flg1" start="0" end="0"/> <!-- <field name="flg1" start="0" end="0" type="bool"/> -->
+ <field name="flg2" start="1" end="1"/> <!-- <field name="flg2" start="1" end="1" type="bool"/> -->
+ <field name="flg3" start="2" end="6"/> <!-- <field name="flg3" start="2" end="6" type="uint64"/> -->
+ <field name="flg4" start="7" end="7"/> <!-- <field name="flg4" start="7" end="7" type="bool"/> -->
+ </flags>
+ <reg name="r1" bitsize="8" type="my_flags"/> <!-- <reg name="r1" bitsize="8" type="my_flags" regnum="1"/> -->
+ <reg name="r2" bitsize="8" type="bit_field"/> <!-- <reg name="r2" bitsize="8" type="bit_field" regnum="2"/> -->
+ </feature>
+</target>
diff --git a/gdb/testsuite/gdb.xml/maint-xml-dump.exp b/gdb/testsuite/gdb.xml/maint-xml-dump.exp
new file mode 100644
index 00000000000..163996450eb
--- /dev/null
+++ b/gdb/testsuite/gdb.xml/maint-xml-dump.exp
@@ -0,0 +1,124 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test the 'maint print xml-tdesc' command. This file picks up every
+# XML file matching the pattern maint-xml-dump-*.xml (in the same
+# directory as this script) and passes each in turn to the command
+# 'maint print xml-tdesc'.
+#
+# The expected output is generated by parsing the input XML file. The
+# rules for changing an XML file into the expected output are:
+#
+# 1. Blank lines, and lines starting with a comment are stripped from
+# the expected output.
+#
+# 2. The <?xml ... ?> and <!DOCTYPE ...> entities are optional,
+# suitable defaults will be added if these lines are missing from
+# the input file.
+#
+# 3. A trailing comment on a line will replace the expected output for
+# that line but with the indentation of the line preserved. So
+# this (The '|' marks the start of the line):
+# | <reg name="r1" bitsize="32"/> <!-- <reg name="r1" bitsize="32" type="int" regnum="0"/> -->
+# Will actually look for the following output:
+# | <reg name="r1" bitsize="32" type="int" regnum="0"/>
+#
+# 4. Indentation of lines will be preserved so your input file needs
+# to follow the expected indentation.
+if {[gdb_skip_xml_test]} {
+ unsupported "maint-xml-dump.exp"
+ return -1
+}
+
+gdb_start
+
+# Read the XML file FILENAME and produce an output pattern that should
+# match what GDB produces with the 'maint print xml-desc' command.
+proc build_pattern { filename } {
+ set pattern {}
+
+ set xml_version_line {<?xml version="1.0"?>}
+ set doc_type_line {<!DOCTYPE target SYSTEM "gdb-target.dtd">}
+
+ set linenum 0
+ set ifd [open "$filename" r]
+ while {[gets $ifd line] >= 0} {
+ incr linenum
+
+ # The <?xml .... ?> tag can only appear as the first line in
+ # the file. If it is not present then add one to the expected
+ # output now.
+ if {$linenum == 1} {
+ if {![regexp {^<\?xml} $line]} {
+ set pattern [string_to_regexp $xml_version_line]
+ set xml_version_line ""
+ }
+ }
+
+ # If we have not yet seen a DOCTYPE line, then maybe we should
+ # be adding one? If we find <target> then add a default
+ # DOCTYPE line, otherwise, if the XML file includes a DOCTYPE
+ # line, use that.
+ if {$doc_type_line != "" } {
+ if {[regexp {^[ \t]*<target>} $line]} {
+ set pattern [multi_line $pattern \
+ [string_to_regexp $doc_type_line]]
+ set doc_type_line ""
+ } elseif {[regexp {^[ \t]*<!DOCTYPE } $line]} {
+ set doc_type_line ""
+ }
+ }
+
+ if {[regexp {^[ \t]*<!--} $line]} {
+ # Comment line, ignore it.
+ } elseif {[regexp {^[ \t]+$} $line]} {
+ # Blank line, ignore it.
+ } elseif {[regexp {^([ \t]*).*<!-- (.*) -->$} $line \
+ matches grp1 grp2]} {
+ set pattern [multi_line \
+ $pattern \
+ [string_to_regexp "$grp1$grp2"]]
+ } else {
+ set pattern [multi_line \
+ $pattern \
+ [string_to_regexp $line]]
+ }
+ }
+ close $ifd
+
+ # Due handling the <?xml ...?> tags we can end up with a stray
+ # '\r\n' at the start of the output pattern. Remove it here.
+ if {[string range $pattern 0 1] == "\r\n"} {
+ set pattern [string range $pattern 2 end]
+ }
+
+ return $pattern
+}
+
+# Run over every test XML file and check the output.
+foreach filename [lsort [glob $srcdir/$subdir/maint-xml-dump-*.xml]] {
+ set pattern [build_pattern $filename]
+
+ if {[is_remote host]} {
+ set test_path [remote_download host $filename]
+ } else {
+ set test_path $filename
+ }
+
+ verbose -log "Looking for:\n$pattern"
+
+ gdb_test "maint print xml-tdesc $test_path" \
+ "$pattern" "check [file tail $filename]"
+}
diff --git a/gdbsupport/tdesc.cc b/gdbsupport/tdesc.cc
index aaea8e0d8a8..cc11d3013c3 100644
--- a/gdbsupport/tdesc.cc
+++ b/gdbsupport/tdesc.cc
@@ -294,12 +294,14 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value,
void print_xml_feature::visit_pre (const tdesc_feature *e)
{
- string_appendf (*m_buffer, "<feature name=\"%s\">\n", e->name.c_str ());
+ add_line ("<feature name=\"%s\">", e->name.c_str ());
+ indent (1);
}
void print_xml_feature::visit_post (const tdesc_feature *e)
{
- string_appendf (*m_buffer, "</feature>\n");
+ indent (-1);
+ add_line ("</feature>");
}
void print_xml_feature::visit (const tdesc_type_builtin *t)
@@ -309,8 +311,8 @@ void print_xml_feature::visit (const tdesc_type_builtin *t)
void print_xml_feature::visit (const tdesc_type_vector *t)
{
- string_appendf (*m_buffer, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>\n",
- t->name.c_str (), t->element_type->name.c_str (), t->count);
+ add_line ("<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
+ t->name.c_str (), t->element_type->name.c_str (), t->count);
}
void print_xml_feature::visit (const tdesc_type_with_fields *t)
@@ -319,7 +321,9 @@ void print_xml_feature::visit (const tdesc_type_with_fields *t)
gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
- string_appendf (*m_buffer,
+ std::string tmp;
+
+ string_appendf (tmp,
"<%s id=\"%s\"", types[t->kind - TDESC_TYPE_STRUCT],
t->name.c_str ());
@@ -328,33 +332,37 @@ void print_xml_feature::visit (const tdesc_type_with_fields *t)
case TDESC_TYPE_STRUCT:
case TDESC_TYPE_FLAGS:
if (t->size > 0)
- string_appendf (*m_buffer, " size=\"%d\"", t->size);
- string_appendf (*m_buffer, ">\n");
+ string_appendf (tmp, " size=\"%d\"", t->size);
+ string_appendf (tmp, ">");
+ add_line (tmp);
for (const tdesc_type_field &f : t->fields)
{
- string_appendf (*m_buffer, " <field name=\"%s\" ", f.name.c_str ());
- if (f.start == -1)
- string_appendf (*m_buffer, "type=\"%s\"/>\n",
- f.type->name.c_str ());
- else
- string_appendf (*m_buffer, "start=\"%d\" end=\"%d\"/>\n", f.start,
+ tmp.clear ();
+ string_appendf (tmp, " <field name=\"%s\"", f.name.c_str ());
+ if (f.start != -1)
+ string_appendf (tmp, " start=\"%d\" end=\"%d\"", f.start,
f.end);
+ string_appendf (tmp, " type=\"%s\"/>",
+ f.type->name.c_str ());
+ add_line (tmp);
}
break;
case TDESC_TYPE_ENUM:
- string_appendf (*m_buffer, ">\n");
+ string_appendf (tmp, ">");
+ add_line (tmp);
for (const tdesc_type_field &f : t->fields)
- string_appendf (*m_buffer, " <field name=\"%s\" start=\"%d\"/>\n",
- f.name.c_str (), f.start);
+ add_line (" <field name=\"%s\" start=\"%d\"/>",
+ f.name.c_str (), f.start);
break;
case TDESC_TYPE_UNION:
- string_appendf (*m_buffer, ">\n");
+ string_appendf (tmp, ">");
+ add_line (tmp);
for (const tdesc_type_field &f : t->fields)
- string_appendf (*m_buffer, " <field name=\"%s\" type=\"%s\"/>\n",
- f.name.c_str (), f.type->name.c_str ());
+ add_line (" <field name=\"%s\" type=\"%s\"/>",
+ f.name.c_str (), f.type->name.c_str ());
break;
default:
@@ -362,40 +370,71 @@ void print_xml_feature::visit (const tdesc_type_with_fields *t)
t->name.c_str ());
}
- string_appendf (*m_buffer, "</%s>\n", types[t->kind - TDESC_TYPE_STRUCT]);
+ add_line ("</%s>", types[t->kind - TDESC_TYPE_STRUCT]);
}
void print_xml_feature::visit (const tdesc_reg *r)
{
- string_appendf (*m_buffer,
+ std::string tmp;
+
+ string_appendf (tmp,
"<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" regnum=\"%ld\"",
r->name.c_str (), r->bitsize, r->type.c_str (),
r->target_regnum);
if (r->group.length () > 0)
- string_appendf (*m_buffer, " group=\"%s\"", r->group.c_str ());
+ string_appendf (tmp, " group=\"%s\"", r->group.c_str ());
if (r->save_restore == 0)
- string_appendf (*m_buffer, " save-restore=\"no\"");
+ string_appendf (tmp, " save-restore=\"no\"");
- string_appendf (*m_buffer, "/>\n");
+ string_appendf (tmp, "/>");
+
+ add_line (tmp);
}
void print_xml_feature::visit_pre (const target_desc *e)
{
#ifndef IN_PROCESS_AGENT
- string_appendf (*m_buffer, "<?xml version=\"1.0\"?>\n");
- string_appendf (*m_buffer, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">\n");
- string_appendf (*m_buffer, "<target>\n<architecture>%s</architecture>\n",
- tdesc_architecture_name (e));
+ add_line ("<?xml version=\"1.0\"?>");
+ add_line ("<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+ add_line ("<target>");
+ indent (1);
+ if (tdesc_architecture_name (e))
+ add_line ("<architecture>%s</architecture>",
+ tdesc_architecture_name (e));
const char *osabi = tdesc_osabi_name (e);
if (osabi != nullptr)
- string_appendf (*m_buffer, "<osabi>%s</osabi>", osabi);
+ add_line ("<osabi>%s</osabi>", osabi);
#endif
}
void print_xml_feature::visit_post (const target_desc *e)
{
- string_appendf (*m_buffer, "</target>\n");
+ indent (-1);
+ add_line ("</target>");
+}
+
+/* See gdbsupport/tdesc.h. */
+
+void
+print_xml_feature::add_line (const std::string &str)
+{
+ string_appendf (*m_buffer, "%s", str.c_str ());
+ string_appendf (*m_buffer, "\n");
+}
+
+/* See gdbsupport/tdesc.h. */
+
+void
+print_xml_feature::add_line (const char *fmt, ...)
+{
+ std::string tmp;
+
+ va_list ap;
+ va_start (ap, fmt);
+ string_vappendf (tmp, fmt, ap);
+ va_end (ap);
+ add_line (tmp);
}
diff --git a/gdbsupport/tdesc.h b/gdbsupport/tdesc.h
index 3b1f1f57ff8..009710e736b 100644
--- a/gdbsupport/tdesc.h
+++ b/gdbsupport/tdesc.h
@@ -401,6 +401,21 @@ class print_xml_feature : public tdesc_element_visitor
void visit (const tdesc_type_with_fields *type) override;
void visit (const tdesc_reg *reg) override;
+protected:
+
+ /* Called with a positive value of ADJUST we move inside an element, for
+ example inside <target>, and with a negative value when we leave the
+ element. In this class this function does nothing, but a sub-class
+ can override this to track the current level of nesting. */
+ virtual void indent (int adjust)
+ { /* Nothing. */ }
+
+ /* Functions to add lines to the output buffer M_BUFFER. Each of these
+ functions appends a newline, so don't include one the strings being
+ sent. */
+ virtual void add_line (const std::string &str);
+ virtual void add_line (const char *fmt, ...);
+
private:
std::string *m_buffer;
};
--
2.25.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote
2020-06-11 10:41 ` [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
@ 2020-06-11 11:33 ` Pedro Alves
2020-06-11 14:05 ` Andrew Burgess
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2020-06-11 11:33 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 6/11/20 11:41 AM, Andrew Burgess wrote:
> The maintenance command 'maintenance print c-tdesc' can only print the
> target description if it was loaded from a local file, or if the local
> filename is passed to the maintenance command as an argument.
>
> Sometimes it would be nice to know what target description GDB was
> given by the remote, however, if I connect to a remote target and try
> this command I see this:
>
> (gdb) maintenance print c-tdesc
> The current target description did not come from an XML file.
> (gdb)
>
> Which is not very helpful.
>
> This commit changes things so that if the description came from the
> remote end then GDB will use the fake filename 'target.xml' as the
> filename for the description, GDB will then create the C description
> of the target as if it was in a file 'target.xml'.
>
> I originally added this functionality so I could inspect the
> description passed to GDB by the remote target. After using this for
> a while I realised that actually having GDB recreate the XML would be
> even better, so a later commit will add that functionality too.
>
> Still, given how small this patch is I thought it might be nice to
> include this in GDB anyway.
>
> While I was working on this anyway I've added filename command
> completion to this command.
>
> gdb/ChangeLog:
>
> * target-descriptions.c (maint_print_c_tdesc_cmd): Use fake
> filename for target descriptions that came from the target.
> (_initialize_target_descriptions): Add filename command completion
> for 'maint print c-tdesc'.
> ---
> gdb/ChangeLog | 7 +++++++
> gdb/target-descriptions.c | 14 ++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 20a3a640f4f..55ea416d69a 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -1680,7 +1680,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
> error (_("There is no target description to print."));
>
> if (filename == NULL)
> - error (_("The current target description did not come from an XML file."));
> + {
> + printf_unfiltered (_("The current target description was fetched "
> + "from the target, using\n'target.xml' as a fake "
> + "filename.\n\n"));
That "," makes it read a little bit ambiguously. Try reading the sentence
without the "," to see what I mean:
The current target description was fetched from the target using
'target.xml' as a fake filename.
This can be read as GDB having read the remote fake 'target.xml' filename,
like:
fetch_available_features_from_target ("target.xml", ops);
which is what it always does anyway...
I'd suggest a hard period (and line break after the period) instead:
The current target description was fetched from the target.
Using 'target.xml' as a fake filename.
But, why do we need to provide a fake name at all? Isn't the only use of
that filename to print it in the comment, here:
/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
Original: target.xml */
How about just doing this:
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1680,7 +1680,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
error (_("There is no target description to print."));
if (filename == NULL)
- error (_("The current target description did not come from an XML file."));
+ filename = "fetched from current target";
with no printf_unfiltered call, and then we get:
(gdb) maint print c-tdesc
/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
Original: fetched from current target */
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: New maintenance command to print XML target description
2020-06-11 10:41 ` [PATCH 2/2] gdb: New maintenance command to print XML target description Andrew Burgess
@ 2020-06-11 13:10 ` Pedro Alves
2020-06-11 13:27 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2020-06-11 13:10 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 6/11/20 11:41 AM, Andrew Burgess wrote:
> However, that class produced XML which seems designed for a machine to
> read, with only minimal indentation. With a bit of tweaking I was
> able to restructure things so I could inherit from the existing XML
> producer and add indentation.
>
Cool.
An interesting testcase this could add would be, run a program
to main, and then:
(gdb) pipe maint print xml-tdesc | cat > target.xml (*)
(gdb) set tdesc filename target.xml
and then step to fetch registers with the new tesc.
Also another test that this could do is make make sure
a roundtrip through GDB of a GDB-generated XML produces the
exact same XML. Like:
(gdb) pipe maint print xml-tdesc input.xml | cat > out-1.xml
(gdb) pipe maint print xml-tdesc out-1.xml | cat > out-2.xml
After this, input.xml and out-*.xml may be different, but
out-1.xml and out-2.xml should be exactly the same, right?
(*) - or something like
maint print xml-tdesc -o target.xml
> +/* Class to format the target description XML with nesting. */
> +
> +class gdb_print_xml_feature : public print_xml_feature
> +{
> +public:
> + /* Constructor. */
> +
> + gdb_print_xml_feature (std::string *arg)
explicit.
> + :print_xml_feature (arg)
Space after ":".
> + { /* Nothing. */ }
> +
> + /* Override this from the parent so we can add the compatibility
> + information. */
Curious. Do you know why this isn't done in the parent?
> +
> + void visit_pre (const target_desc *e) override
> + {
> + print_xml_feature::visit_pre (e);
> + for (const bfd_arch_info_type *compatible : e->compatible)
> + add_line ("<compatible>%s</compatible>", compatible->printable_name);
> + };
> +
> +protected:
> +
> + /* Pull in all copies of add_line. */
> +
> + using print_xml_feature::add_line;
> +
> + /* Override this version of add_line to add white space for indentation
> + at the start of the line. */
> +
> + void add_line (const std::string &str) override
> + {
> + std::string tmp = string_printf ("%*s", m_depth, "");
> + tmp += str;
> + print_xml_feature::add_line (tmp);
> + }
> +
> + /* Increase our nesting by ADJUST. We use two spaces for every level of
> + indentation. */
> +
> + void indent (int adjust) override
> + {
> + m_depth += (adjust * 2);
> + }
> +
> +private:
> +
> + /* The current level of indentation. */
> + int m_depth = 0;
> +};
> +
> +/* Implement the maintenance print xml-tdesc command. */
> +
> +static void
> +maint_print_xml_tdesc_cmd (const char *args, int from_tty)
> +{
> + const struct target_desc *tdesc;
> +
> + if (args == NULL)
> + {
> + /* Use the global target-supplied description, not the current
> + architecture's. This lets a GDB for one architecture generate C
> + for another architecture's description, even though the gdbarch
The "generate C" bit (copied from "maint print c-tdesc") should be
tweaked to say XML instead.
> + initialization code will reject the new description. */
> + tdesc = current_target_desc;
> + }
> + else
> + {
> + /* Use the target description from the XML file. */
> + tdesc = file_read_description_xml (args);
> + }
> +
> + if (tdesc == NULL)
> + error (_("There is no target description to print."));
> +
> + std::string buf;
> + gdb_print_xml_feature v (&buf);
> + tdesc->accept (v);
> + puts_unfiltered (buf.c_str ());
> +}
> +
> +#
> +# 4. Indentation of lines will be preserved so your input file needs
> +# to follow the expected indentation.
> +if {[gdb_skip_xml_test]} {
> + unsupported "maint-xml-dump.exp"
This just results in:
UNSUPPORTED: gdb.xml/maint-xml-dump.exp: maint-xml-dump.exp
See:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls
> + return -1
> +}
> +
> +gdb_start
> + close $ifd
> +
> + # Due handling the <?xml ...?> tags we can end up with a stray
This reads a bit strange: is there a word missing in "Due handling"?
> + # '\r\n' at the start of the output pattern. Remove it here.
> + if {[string range $pattern 0 1] == "\r\n"} {
> + set pattern [string range $pattern 2 end]
> + }
> +
> + return $pattern
> +}
> --- a/gdbsupport/tdesc.h
> +++ b/gdbsupport/tdesc.h
> @@ -401,6 +401,21 @@ class print_xml_feature : public tdesc_element_visitor
> void visit (const tdesc_type_with_fields *type) override;
> void visit (const tdesc_reg *reg) override;
>
> +protected:
> +
> + /* Called with a positive value of ADJUST we move inside an element, for
Spurious double space. Maybe missing "when" as in "ADJUST when we" ?
> + example inside <target>, and with a negative value when we leave the
> + element. In this class this function does nothing, but a sub-class
> + can override this to track the current level of nesting. */
> + virtual void indent (int adjust)
> + { /* Nothing. */ }
> +
> + /* Functions to add lines to the output buffer M_BUFFER. Each of these
> + functions appends a newline, so don't include one the strings being
> + sent. */
Did you mean "don't include one IN the" ?
> + virtual void add_line (const std::string &str);
> + virtual void add_line (const char *fmt, ...);
> +
> private:
> std::string *m_buffer;
> };
>
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: New maintenance command to print XML target description
2020-06-11 10:41 ` [PATCH 2/2] gdb: New maintenance command to print XML target description Andrew Burgess
2020-06-11 13:10 ` Pedro Alves
@ 2020-06-11 13:27 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2020-06-11 13:27 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Thu, 11 Jun 2020 11:41:31 +0100
>
> gdbsupport/ChangeLog:
>
> * tdesc.cc (print_xml_feature::visit_pre): Use add_line to add
> output content, and call indent as needed in all overloaded
> variants.
> (print_xml_feature::visit_post): Likewise.
> (print_xml_feature::visit): Likewise.
> (print_xml_feature::add_line): Two new overloaded functions.
> * tdesc.h (print_xml_feature::indent): Declare new member
> function.
> (print_xml_feature::add_line): Two new overloaded member
> functions.
>
> gdb/ChangeLog:
>
> * target-descriptions.c (tdesc_architecture_name): Protect against
> NULL pointer dereference.
> (class gdb_print_xml_feature): New class.
> (maint_print_xml_tdesc_cmd): New function.
> (_initialize_target_descriptions): Register new 'maint print
> xml-tdesc' command and give it the filename completer.
> * NEWS: Mention new 'maint print xml-tdesc' command.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.xml/maint-xml-dump-01.xml: New file.
> * gdb.xml/maint-xml-dump-02.xml: New file.
> * gdb.xml/maint-xml-dump.exp: New file.
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (Maintenance Commands): Document new 'maint print
> xml-desc' command.
OK for the documentation changes.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote
2020-06-11 11:33 ` Pedro Alves
@ 2020-06-11 14:05 ` Andrew Burgess
2020-06-11 14:24 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2020-06-11 14:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
* Pedro Alves <palves@redhat.com> [2020-06-11 12:33:08 +0100]:
> On 6/11/20 11:41 AM, Andrew Burgess wrote:
> > The maintenance command 'maintenance print c-tdesc' can only print the
> > target description if it was loaded from a local file, or if the local
> > filename is passed to the maintenance command as an argument.
> >
> > Sometimes it would be nice to know what target description GDB was
> > given by the remote, however, if I connect to a remote target and try
> > this command I see this:
> >
> > (gdb) maintenance print c-tdesc
> > The current target description did not come from an XML file.
> > (gdb)
> >
> > Which is not very helpful.
> >
> > This commit changes things so that if the description came from the
> > remote end then GDB will use the fake filename 'target.xml' as the
> > filename for the description, GDB will then create the C description
> > of the target as if it was in a file 'target.xml'.
> >
> > I originally added this functionality so I could inspect the
> > description passed to GDB by the remote target. After using this for
> > a while I realised that actually having GDB recreate the XML would be
> > even better, so a later commit will add that functionality too.
> >
> > Still, given how small this patch is I thought it might be nice to
> > include this in GDB anyway.
> >
> > While I was working on this anyway I've added filename command
> > completion to this command.
> >
> > gdb/ChangeLog:
> >
> > * target-descriptions.c (maint_print_c_tdesc_cmd): Use fake
> > filename for target descriptions that came from the target.
> > (_initialize_target_descriptions): Add filename command completion
> > for 'maint print c-tdesc'.
> > ---
> > gdb/ChangeLog | 7 +++++++
> > gdb/target-descriptions.c | 14 ++++++++++----
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> > index 20a3a640f4f..55ea416d69a 100644
> > --- a/gdb/target-descriptions.c
> > +++ b/gdb/target-descriptions.c
> > @@ -1680,7 +1680,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
> > error (_("There is no target description to print."));
> >
> > if (filename == NULL)
> > - error (_("The current target description did not come from an XML file."));
> > + {
> > + printf_unfiltered (_("The current target description was fetched "
> > + "from the target, using\n'target.xml' as a fake "
> > + "filename.\n\n"));
>
> That "," makes it read a little bit ambiguously. Try reading the sentence
> without the "," to see what I mean:
>
> The current target description was fetched from the target using
> 'target.xml' as a fake filename.
>
> This can be read as GDB having read the remote fake 'target.xml' filename,
> like:
> fetch_available_features_from_target ("target.xml", ops);
> which is what it always does anyway...
>
> I'd suggest a hard period (and line break after the period) instead:
>
> The current target description was fetched from the target.
> Using 'target.xml' as a fake filename.
>
> But, why do we need to provide a fake name at all? Isn't the only use of
> that filename to print it in the comment, here:
>
> /* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
> Original: target.xml */
>
> How about just doing this:
No, the filename is used in a couple of other places too, object names
are created based on the filename as well. Still I took your idea and
went with it.
The patch below uses 'fetched from target' as the fake filename
instead. I needed one small extra change so that 'fetched from
target' could become 'fetched_from_target' when creating C++ variable
names, but otherwise this seem fine to me.
How's this?
Thanks,
Andrew
---
commit 7426b6d447cc764cfe5a23486ad8af95ec46d2fc
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Tue Jun 9 19:00:55 2020 +0100
gdb: Allow target description to be dumped even when it is remote
The maintenance command 'maintenance print c-tdesc' can only print the
target description if it was loaded from a local file, or if the local
filename is passed to the maintenance command as an argument.
Sometimes it would be nice to know what target description GDB was
given by the remote, however, if I connect to a remote target and try
this command I see this:
(gdb) maintenance print c-tdesc
The current target description did not come from an XML file.
(gdb)
Which is not very helpful.
This commit changes things so that if the description came from the
remote end then GDB will use a fake filename 'fetched from target' as
the filename for the description, GDB will then create the C
description of the target as though it came from this file. Example
output would look like this (I snipped the feature creation from the
middle as that hasn't changed):
(gdb) maintenance print c-tdesc
/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
Original: fetched from target */
#include "defs.h"
#include "osabi.h"
#include "target-descriptions.h"
struct target_desc *tdesc_fetched_from_target;
static void
initialize_tdesc_fetched_from_target (void)
{
struct target_desc *result = allocate_target_description ();
struct tdesc_feature *feature;
/* ... features created here ... */
tdesc_fetched_from_target = result;
}
(gdb)
I originally added this functionality so I could inspect the
description passed to GDB by the remote target. After using this for
a while I realised that actually having GDB recreate the XML would be
even better, so a later commit will add that functionality too.
Still, given how small this patch is I thought it might be nice to
include this in GDB anyway.
While I was working on this anyway I've added filename command
completion to this command.
gdb/ChangeLog:
* target-descriptions.c (print_c_tdesc::print_c_tdesc): Change
whitespace to underscore.
(maint_print_c_tdesc_cmd): Use fake filename for target
descriptions that came from the target.
(_initialize_target_descriptions): Add filename command completion
for 'maint print c-tdesc'.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 26310c429b3..d358ff4ace0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-06-10 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * target-descriptions.c (print_c_tdesc::print_c_tdesc): Change
+ whitespace to underscore.
+ (maint_print_c_tdesc_cmd): Use fake filename for target
+ descriptions that came from the target.
+ (_initialize_target_descriptions): Add filename command completion
+ for 'maint print c-tdesc'.
+
2020-06-08 Simon Marchi <simon.marchi@efficios.com>
* gdbtypes.h (TYPE_FIELD_TYPE): Remove. Change all call sites
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 20a3a640f4f..da6cb76404c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1270,6 +1270,8 @@ class print_c_tdesc : public tdesc_element_visitor
break;
else if (*inp == '-')
*outp++ = '_';
+ else if (*inp == ' ')
+ *outp++ = '_';
else
*outp++ = *inp;
*outp = '\0';
@@ -1680,7 +1682,7 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
error (_("There is no target description to print."));
if (filename == NULL)
- error (_("The current target description did not come from an XML file."));
+ filename = "fetched from target";
std::string filename_after_features (filename);
auto loc = filename_after_features.rfind ("/features/");
@@ -1811,6 +1813,8 @@ void _initialize_target_descriptions ();
void
_initialize_target_descriptions ()
{
+ cmd_list_element *cmd;
+
tdesc_data = gdbarch_data_register_pre_init (tdesc_data_init);
add_basic_prefix_cmd ("tdesc", class_maintenance, _("\
@@ -1842,11 +1846,10 @@ Unset the file to read for an XML target description.\n\
When unset, GDB will read the description from the target."),
&tdesc_unset_cmdlist);
- add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
+ cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
Print the current target description as a C source file."),
&maintenanceprintlist);
-
- cmd_list_element *cmd;
+ set_cmd_completer (cmd, filename_completer);
cmd = add_cmd ("xml-descriptions", class_maintenance,
maintenance_check_xml_descriptions, _("\
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote
2020-06-11 14:05 ` Andrew Burgess
@ 2020-06-11 14:24 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2020-06-11 14:24 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 6/11/20 3:05 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2020-06-11 12:33:08 +0100]:
>
>> On 6/11/20 11:41 AM, Andrew Burgess wrote:
>>> The maintenance command 'maintenance print c-tdesc' can only print the
>>> target description if it was loaded from a local file, or if the local
>>> filename is passed to the maintenance command as an argument.
>>>
>>> Sometimes it would be nice to know what target description GDB was
>>> given by the remote, however, if I connect to a remote target and try
>>> this command I see this:
>>>
>>> (gdb) maintenance print c-tdesc
>>> The current target description did not come from an XML file.
>>> (gdb)
>>>
>>> Which is not very helpful.
>>>
>>> This commit changes things so that if the description came from the
>>> remote end then GDB will use the fake filename 'target.xml' as the
>>> filename for the description, GDB will then create the C description
>>> of the target as if it was in a file 'target.xml'.
>>>
>>> I originally added this functionality so I could inspect the
>>> description passed to GDB by the remote target. After using this for
>>> a while I realised that actually having GDB recreate the XML would be
>>> even better, so a later commit will add that functionality too.
>>>
>>> Still, given how small this patch is I thought it might be nice to
>>> include this in GDB anyway.
>>>
>>> While I was working on this anyway I've added filename command
>>> completion to this command.
>>>
>>> gdb/ChangeLog:
>>>
>>> * target-descriptions.c (maint_print_c_tdesc_cmd): Use fake
>>> filename for target descriptions that came from the target.
>>> (_initialize_target_descriptions): Add filename command completion
>>> for 'maint print c-tdesc'.
>>> ---
>>> gdb/ChangeLog | 7 +++++++
>>> gdb/target-descriptions.c | 14 ++++++++++----
>>> 2 files changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
>>> index 20a3a640f4f..55ea416d69a 100644
>>> --- a/gdb/target-descriptions.c
>>> +++ b/gdb/target-descriptions.c
>>> @@ -1680,7 +1680,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
>>> error (_("There is no target description to print."));
>>>
>>> if (filename == NULL)
>>> - error (_("The current target description did not come from an XML file."));
>>> + {
>>> + printf_unfiltered (_("The current target description was fetched "
>>> + "from the target, using\n'target.xml' as a fake "
>>> + "filename.\n\n"));
>>
>> That "," makes it read a little bit ambiguously. Try reading the sentence
>> without the "," to see what I mean:
>>
>> The current target description was fetched from the target using
>> 'target.xml' as a fake filename.
>>
>> This can be read as GDB having read the remote fake 'target.xml' filename,
>> like:
>> fetch_available_features_from_target ("target.xml", ops);
>> which is what it always does anyway...
>>
>> I'd suggest a hard period (and line break after the period) instead:
>>
>> The current target description was fetched from the target.
>> Using 'target.xml' as a fake filename.
>>
>> But, why do we need to provide a fake name at all? Isn't the only use of
>> that filename to print it in the comment, here:
>>
>> /* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
>> Original: target.xml */
>>
>> How about just doing this:
>
> No, the filename is used in a couple of other places too, object names
> are created based on the filename as well. Still I took your idea and
> went with it.
Ah, missed that.
>
> The patch below uses 'fetched from target' as the fake filename
> instead. I needed one small extra change so that 'fetched from
> target' could become 'fetched_from_target' when creating C++ variable
> names, but otherwise this seem fine to me.
That fixes the case of a filename with spaces, regardless,
like "foo bar.xml", as in:
(gdb) maint print c-tdesc foo bar.xml
Maybe mention that in the commit log.
>
> How's this?
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Additional maintenance command for dumping target descriptions
2020-06-11 10:41 [PATCH 0/2] Additional maintenance command for dumping target descriptions Andrew Burgess
2020-06-11 10:41 ` [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
2020-06-11 10:41 ` [PATCH 2/2] gdb: New maintenance command to print XML target description Andrew Burgess
@ 2020-06-11 19:50 ` Christian Biesinger
2020-06-16 20:51 ` Andrew Burgess
3 siblings, 0 replies; 10+ messages in thread
From: Christian Biesinger @ 2020-06-11 19:50 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On Thu, Jun 11, 2020 at 5:41 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> Patch #2 allows GDB developers to see in XML form the description that
> was sent from the target.
I haven't looked at the code but I've wanted that as well! Really
happy that you're working on this.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Additional maintenance command for dumping target descriptions
2020-06-11 10:41 [PATCH 0/2] Additional maintenance command for dumping target descriptions Andrew Burgess
` (2 preceding siblings ...)
2020-06-11 19:50 ` [PATCH 0/2] Additional maintenance command for dumping target descriptions Christian Biesinger
@ 2020-06-16 20:51 ` Andrew Burgess
3 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2020-06-16 20:51 UTC (permalink / raw)
To: gdb-patches
I messed up the in-reply-to header while posting a second iteration of
this patch series. The replacement patches ended up here:
https://sourceware.org/pipermail/gdb-patches/2020-June/169419.html
Thanks,
Andrew
* Andrew Burgess <andrew.burgess@embecosm.com> [2020-06-11 11:41:29 +0100]:
> After some fun and games debugging some target description issues I
> finally got fed up enough to implement something that I've wanted for
> a while. A mechanism to ask GDB what target description was sent from
> the target.
>
> Patch #1 was a quick hack to `maint print c-tdesc', which was easy,
> and did give me the information I needed, but the output format isn't
> great for what I wanted, so
>
> Patch #2 allows GDB developers to see in XML form the description that
> was sent from the target.
>
> Feedback welcome.
>
> Thanks,
> Andrew
>
>
> ---
>
> Andrew Burgess (2):
> gdb: Allow target description to be dumped even when it is remote
> gdb: New maintenance command to print XML target description
>
> gdb/ChangeLog | 17 +++
> gdb/NEWS | 6 +
> gdb/doc/ChangeLog | 5 +
> gdb/doc/gdb.texinfo | 9 ++
> gdb/target-descriptions.c | 102 +++++++++++++++-
> gdb/testsuite/ChangeLog | 6 +
> gdb/testsuite/gdb.xml/maint-xml-dump-01.xml | 10 ++
> gdb/testsuite/gdb.xml/maint-xml-dump-02.xml | 27 +++++
> gdb/testsuite/gdb.xml/maint-xml-dump.exp | 124 ++++++++++++++++++++
> gdbsupport/ChangeLog | 13 ++
> gdbsupport/tdesc.cc | 99 +++++++++++-----
> gdbsupport/tdesc.h | 15 +++
> 12 files changed, 399 insertions(+), 34 deletions(-)
> create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-01.xml
> create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump-02.xml
> create mode 100644 gdb/testsuite/gdb.xml/maint-xml-dump.exp
>
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-06-16 20:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 10:41 [PATCH 0/2] Additional maintenance command for dumping target descriptions Andrew Burgess
2020-06-11 10:41 ` [PATCH 1/2] gdb: Allow target description to be dumped even when it is remote Andrew Burgess
2020-06-11 11:33 ` Pedro Alves
2020-06-11 14:05 ` Andrew Burgess
2020-06-11 14:24 ` Pedro Alves
2020-06-11 10:41 ` [PATCH 2/2] gdb: New maintenance command to print XML target description Andrew Burgess
2020-06-11 13:10 ` Pedro Alves
2020-06-11 13:27 ` Eli Zaretskii
2020-06-11 19:50 ` [PATCH 0/2] Additional maintenance command for dumping target descriptions Christian Biesinger
2020-06-16 20:51 ` Andrew Burgess
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).