* [PATCH] gdb: add an option flag to 'maint print c-tdesc'
@ 2020-10-21 8:22 Andrew Burgess
2020-11-11 22:42 ` Andrew Burgess
2020-11-12 2:26 ` Simon Marchi
0 siblings, 2 replies; 3+ messages in thread
From: Andrew Burgess @ 2020-10-21 8:22 UTC (permalink / raw)
To: gdb-patches
GDB has two approaches to generating the target descriptions found in
gdb/features/, the whole description approach, where the XML file
contains a complete target description which is then used to generate
a single C file that builds that target description. Or, the split
feature approach, where the XML files contain a single target feature,
each feature results in a single C file to create that one feature,
and then a manually written C file is used to build a complete target
description from individual features.
There's a Makefile, gdb/features/Makefile, which is responsible for
managing the regeneration of the C files from the XML files.
However, some of the logic the selects between the whole description
approach, or the split feature approach is actually hard-coded into
GDB, inside target-descriptions.c:maint_print_c_tdesc_cmd we check the
path to the incoming XML file and use this to choose which type of C
file we should generate.
This commit removes this hard coding from GDB, and makes the Makefile
entirely responsible for choosing the approach. This makes sense as
the Makefile already has the XML files partitioned based on which
approach they should use.
In order to allow this change the 'maint print c-tdesc' command is
given a new command option '-single-feature', which tells GDB which
type of C file should be created. The makefile now supplies this flag
to GDB.
This did reveal a bug in features/Makefile, the rx.xml file was in the
wrong list, this didn't matter previously as the actual choice of
which approach to use was done in GDB. Now the Makefile decides, so
placing each XML file in the correct list is critical.
Tested this by doing 'make GDB=/path/to/gdb clean-cfiles cfiles' to
regenerate all the C files from their XML source. There are no
changes after this commit.
gdb/ChangeLog:
* features/Makefile (XMLTOC): Add rx.xml.
(FEATURE_XMLFILES): Remove rx.xml.
(FEATURE_CFILES rule): Pass '-single-feature' flag.
* features/rx.c: Regenerate.
* features/rx.xml: Wrap in `target` tags, and reindent.
* target-descriptions.c (struct maint_print_c_tdesc_options): New
structure.
(maint_print_c_tdesc_opt_def): New typedef.
(maint_print_c_tdesc_opt_defs): New static global.
(make_maint_print_c_tdesc_options_def_group): New function.
(maint_print_c_tdesc_cmd): Make use of command line flags, only
print single feature C file for target descriptions containing a
single feature.
(maint_print_c_tdesc_cmd_completer): New function.
(_initialize_target_descriptions): Update call to register command
completer, and include command line flag in help text.
gdb/doc/ChangeLog:
* gdb.texinfo (Maintenance Commands): Update description of 'maint
print c-tdesc'.
---
gdb/ChangeLog | 19 ++++++
gdb/doc/ChangeLog | 5 ++
gdb/doc/gdb.texinfo | 9 ++-
gdb/features/Makefile | 4 +-
gdb/features/rx.c | 2 +-
gdb/features/rx.xml | 130 +++++++++++++++++++-------------------
gdb/target-descriptions.c | 85 +++++++++++++++++++++----
7 files changed, 173 insertions(+), 81 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2636b6f9903..ad0d587b366 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38594,8 +38594,8 @@
Print the entire architecture configuration. The optional argument
@var{file} names the file where the output goes.
-@kindex maint print c-tdesc @r{[}@var{file}@r{]}
-@item maint print c-tdesc
+@kindex maint print c-tdesc
+@item maint print c-tdesc @r{[}-single-feature@r{]} @r{[}@var{file}@r{]}
Print the target description (@pxref{Target Descriptions}) as
a C source file. By default, the target description is for the current
target, but if the optional argument @var{file} is provided, that file
@@ -38605,6 +38605,11 @@
built again. This command is used by developers after they add or
modify XML target descriptions.
+When the optional flag @samp{-single-feature} is provided then the
+target description being processed (either the default, or from
+@var{file}) must only contain a single feature. The source file
+produced is different in this case.
+
@kindex maint print xml-tdesc
@item maint print xml-tdesc @r{[}@var{file}@r{]}
Print the target description (@pxref{Target Descriptions}) as an XML
diff --git a/gdb/features/Makefile b/gdb/features/Makefile
index 689603847a0..d5b3236242d 100644
--- a/gdb/features/Makefile
+++ b/gdb/features/Makefile
@@ -153,6 +153,7 @@ XMLTOC = \
rs6000/powerpc-vsx64.xml \
rs6000/powerpc-vsx64l.xml \
rs6000/rs6000.xml \
+ rx.xml \
s390-linux32.xml \
s390-linux32v1.xml \
s390-linux32v2.xml \
@@ -232,7 +233,6 @@ FEATURE_XMLFILES = aarch64-core.xml \
riscv/32bit-fpu.xml \
riscv/64bit-cpu.xml \
riscv/64bit-fpu.xml \
- rx.xml \
tic6x-c6xp.xml \
tic6x-core.xml \
tic6x-gp.xml
@@ -247,7 +247,7 @@ $(TDESC_CFILES): %.c: %.xml
$(FEATURE_CFILES): %.c: %.xml.tmp
$(GDB) -nx -q -batch \
- -ex 'maint print c-tdesc $<' > $@.tmp
+ -ex 'maint print c-tdesc -single-feature $<' > $@.tmp
sh ../../move-if-change $@.tmp $@
rm $<
diff --git a/gdb/features/rx.c b/gdb/features/rx.c
index ebbc504ec35..dd765c2c63a 100644
--- a/gdb/features/rx.c
+++ b/gdb/features/rx.c
@@ -1,5 +1,5 @@
/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
- Original: rx.xml.tmp */
+ Original: rx.xml */
#include "defs.h"
#include "osabi.h"
diff --git a/gdb/features/rx.xml b/gdb/features/rx.xml
index d19b53676a3..35348e8b3e5 100644
--- a/gdb/features/rx.xml
+++ b/gdb/features/rx.xml
@@ -5,70 +5,72 @@
are permitted in any medium without royalty provided the copyright
notice and this notice are preserved. -->
-<!DOCTYPE feature SYSTEM "gdb-target.dtd">
-<feature name="org.gnu.gdb.rx.core">
- <reg name="r0" bitsize="32" type="data_ptr"/>
- <reg name="r1" bitsize="32" type="uint32"/>
- <reg name="r2" bitsize="32" type="uint32"/>
- <reg name="r3" bitsize="32" type="uint32"/>
- <reg name="r4" bitsize="32" type="uint32"/>
- <reg name="r5" bitsize="32" type="uint32"/>
- <reg name="r6" bitsize="32" type="uint32"/>
- <reg name="r7" bitsize="32" type="uint32"/>
- <reg name="r8" bitsize="32" type="uint32"/>
- <reg name="r9" bitsize="32" type="uint32"/>
- <reg name="r10" bitsize="32" type="uint32"/>
- <reg name="r11" bitsize="32" type="uint32"/>
- <reg name="r12" bitsize="32" type="uint32"/>
- <reg name="r13" bitsize="32" type="uint32"/>
- <reg name="r14" bitsize="32" type="uint32"/>
- <reg name="r15" bitsize="32" type="uint32"/>
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+ <feature name="org.gnu.gdb.rx.core">
+ <reg name="r0" bitsize="32" type="data_ptr"/>
+ <reg name="r1" bitsize="32" type="uint32"/>
+ <reg name="r2" bitsize="32" type="uint32"/>
+ <reg name="r3" bitsize="32" type="uint32"/>
+ <reg name="r4" bitsize="32" type="uint32"/>
+ <reg name="r5" bitsize="32" type="uint32"/>
+ <reg name="r6" bitsize="32" type="uint32"/>
+ <reg name="r7" bitsize="32" type="uint32"/>
+ <reg name="r8" bitsize="32" type="uint32"/>
+ <reg name="r9" bitsize="32" type="uint32"/>
+ <reg name="r10" bitsize="32" type="uint32"/>
+ <reg name="r11" bitsize="32" type="uint32"/>
+ <reg name="r12" bitsize="32" type="uint32"/>
+ <reg name="r13" bitsize="32" type="uint32"/>
+ <reg name="r14" bitsize="32" type="uint32"/>
+ <reg name="r15" bitsize="32" type="uint32"/>
- <flags id="psw_flags" size="4">
- <field name="C" start="0" end="0"/>
- <field name="Z" start="1" end="1"/>
- <field name="S" start="2" end="2"/>
- <field name="O" start="3" end="3"/>
- <field name="I" start="16" end="16"/>
- <field name="U" start="17" end="17"/>
- <field name="PM" start="20" end="20"/>
- <field name="IPL0" start="24" end="24"/>
- <field name="IPL1" start="25" end="25"/>
- <field name="IPL2" start="26" end="26"/>
- <field name="IPL3" start="27" end="27"/>
- </flags>
+ <flags id="psw_flags" size="4">
+ <field name="C" start="0" end="0"/>
+ <field name="Z" start="1" end="1"/>
+ <field name="S" start="2" end="2"/>
+ <field name="O" start="3" end="3"/>
+ <field name="I" start="16" end="16"/>
+ <field name="U" start="17" end="17"/>
+ <field name="PM" start="20" end="20"/>
+ <field name="IPL0" start="24" end="24"/>
+ <field name="IPL1" start="25" end="25"/>
+ <field name="IPL2" start="26" end="26"/>
+ <field name="IPL3" start="27" end="27"/>
+ </flags>
- <flags id="fpsw_flags" size="4">
- <field name="RM0" start="0" end="0"/>
- <field name="RM1" start="1" end="1"/>
- <field name="CV" start="2" end="2"/>
- <field name="CO" start="3" end="3"/>
- <field name="CZ" start="4" end="4"/>
- <field name="CU" start="5" end="5"/>
- <field name="CX" start="6" end="6"/>
- <field name="CE" start="7" end="7"/>
- <field name="DN" start="8" end="8"/>
- <field name="EV" start="10" end="10"/>
- <field name="EO" start="11" end="11"/>
- <field name="EZ" start="12" end="12"/>
- <field name="EU" start="13" end="13"/>
- <field name="EX" start="14" end="14"/>
- <field name="FV" start="26" end="26"/>
- <field name="FO" start="27" end="27"/>
- <field name="FZ" start="28" end="28"/>
- <field name="FU" start="29" end="29"/>
- <field name="FX" start="30" end="30"/>
- <field name="FS" start="31" end="31"/>
- </flags>
+ <flags id="fpsw_flags" size="4">
+ <field name="RM0" start="0" end="0"/>
+ <field name="RM1" start="1" end="1"/>
+ <field name="CV" start="2" end="2"/>
+ <field name="CO" start="3" end="3"/>
+ <field name="CZ" start="4" end="4"/>
+ <field name="CU" start="5" end="5"/>
+ <field name="CX" start="6" end="6"/>
+ <field name="CE" start="7" end="7"/>
+ <field name="DN" start="8" end="8"/>
+ <field name="EV" start="10" end="10"/>
+ <field name="EO" start="11" end="11"/>
+ <field name="EZ" start="12" end="12"/>
+ <field name="EU" start="13" end="13"/>
+ <field name="EX" start="14" end="14"/>
+ <field name="FV" start="26" end="26"/>
+ <field name="FO" start="27" end="27"/>
+ <field name="FZ" start="28" end="28"/>
+ <field name="FU" start="29" end="29"/>
+ <field name="FX" start="30" end="30"/>
+ <field name="FS" start="31" end="31"/>
+ </flags>
- <reg name="usp" bitsize="32" type="data_ptr"/>
- <reg name="isp" bitsize="32" type="data_ptr"/>
- <reg name="psw" bitsize="32" type="psw_flags"/>
- <reg name="pc" bitsize="32" type="code_ptr"/>
- <reg name="intb" bitsize="32" type="data_ptr"/>
- <reg name="bpsw" bitsize="32" type="psw_flags"/>
- <reg name="bpc" bitsize="32" type="code_ptr"/>
- <reg name="fintv" bitsize="32" type="code_ptr"/>
- <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
- <reg name="acc" bitsize="64" type="uint64"/>
-</feature>
+ <reg name="usp" bitsize="32" type="data_ptr"/>
+ <reg name="isp" bitsize="32" type="data_ptr"/>
+ <reg name="psw" bitsize="32" type="psw_flags"/>
+ <reg name="pc" bitsize="32" type="code_ptr"/>
+ <reg name="intb" bitsize="32" type="data_ptr"/>
+ <reg name="bpsw" bitsize="32" type="psw_flags"/>
+ <reg name="bpc" bitsize="32" type="code_ptr"/>
+ <reg name="fintv" bitsize="32" type="code_ptr"/>
+ <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
+ <reg name="acc" bitsize="64" type="uint64"/>
+ </feature>
+</target>
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 6d0905b90b8..d21f6fdb63c 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1731,12 +1731,45 @@ tdesc_get_features_xml (const target_desc *tdesc)
return tdesc->xmltarget;
}
+/* Data structures and functions to setup the option flags for 'maintenance
+ print c-tdesc command. */
+
+struct maint_print_c_tdesc_options
+{
+ /* True when the '-single-feature' flag was passed. */
+ bool single_feature = false;
+};
+
+using maint_print_c_tdesc_opt_def
+ = gdb::option::flag_option_def<maint_print_c_tdesc_options>;
+
+static const gdb::option::option_def maint_print_c_tdesc_opt_defs[] = {
+ maint_print_c_tdesc_opt_def {
+ "single-feature",
+ [] (maint_print_c_tdesc_options *opt) { return &opt->single_feature; },
+ N_("Print C description of just a single feature.")
+ },
+};
+
+static inline gdb::option::option_def_group
+make_maint_print_c_tdesc_options_def_group (maint_print_c_tdesc_options *opts)
+{
+ return {{maint_print_c_tdesc_opt_defs}, opts};
+}
+
+/* Implement 'maintenance print c-tdesc' command. */
+
static void
maint_print_c_tdesc_cmd (const char *args, int from_tty)
{
const struct target_desc *tdesc;
const char *filename;
+ maint_print_c_tdesc_options opts;
+ auto grp = make_maint_print_c_tdesc_options_def_group (&opts);
+ gdb::option::process_options
+ (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp);
+
if (args == NULL)
{
/* Use the global target-supplied description, not the current
@@ -1768,15 +1801,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
/* Print c files for target features instead of target descriptions,
because c files got from target features are more flexible than the
counterparts. */
- if (startswith (filename_after_features.c_str (), "i386/32bit-")
- || startswith (filename_after_features.c_str (), "i386/64bit-")
- || startswith (filename_after_features.c_str (), "i386/x32-core.xml")
- || startswith (filename_after_features.c_str (), "riscv/")
- || startswith (filename_after_features.c_str (), "tic6x-")
- || startswith (filename_after_features.c_str (), "aarch64")
- || startswith (filename_after_features.c_str (), "arm/")
- || startswith (filename_after_features.c_str (), "arc/"))
+ if (opts.single_feature)
{
+ if (tdesc->features.size () != 1)
+ error (_("only target descriptions with 1 feature can be used "
+ "with -single-feature option"));
+
print_c_feature v (filename_after_features);
tdesc->accept (v);
@@ -1789,6 +1819,22 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
}
}
+/* Completer for the "backtrace" command. */
+
+static void
+maint_print_c_tdesc_cmd_completer (struct cmd_list_element *ignore,
+ completion_tracker &tracker,
+ const char *text, const char *word)
+{
+ auto grp = make_maint_print_c_tdesc_options_def_group (nullptr);
+ if (gdb::option::complete_options
+ (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp))
+ return;
+
+ word = advance_to_filename_complete_word_point (tracker, text);
+ filename_completer (ignore, tracker, text, word);
+}
+
/* Implement the maintenance print xml-tdesc command. */
static void
@@ -1951,10 +1997,25 @@ Unset the file to read for an XML target description.\n\
When unset, GDB will read the description from the target."),
&tdesc_unset_cmdlist);
- cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
-Print the current target description as a C source file."),
- &maintenanceprintlist);
- set_cmd_completer (cmd, filename_completer);
+ auto grp = make_maint_print_c_tdesc_options_def_group (nullptr);
+ static std::string help_text
+ = gdb::option::build_help (_("\
+Print the current target description as a C source file.\n\
+Usage: maintenance print c-tdesc [OPTION] [FILENAME]\n\
+\n\
+Options:\n\
+%OPTIONS%\n\
+\n\
+When FILENAME is not provided then print the current target\n\
+description, otherwise an XML target description is read from\n\
+FILENAME and printed as a C function.\n\
+\n\
+When '-single-feature' is used then the target description should\n\
+contain a single feature and the generated C code will only create\n\
+that feature within an already existing target_desc object."), grp);
+ cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd,
+ help_text.c_str (), &maintenanceprintlist);
+ set_cmd_completer_handle_brkchars (cmd, maint_print_c_tdesc_cmd_completer);
cmd = add_cmd ("xml-tdesc", class_maintenance, maint_print_xml_tdesc_cmd, _("\
Print the current target description as an XML file."),
--
2.25.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: add an option flag to 'maint print c-tdesc'
2020-10-21 8:22 [PATCH] gdb: add an option flag to 'maint print c-tdesc' Andrew Burgess
@ 2020-11-11 22:42 ` Andrew Burgess
2020-11-12 2:26 ` Simon Marchi
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Burgess @ 2020-11-11 22:42 UTC (permalink / raw)
To: gdb-patches
Ping!
I plan to push this soon-ish. It's a reasonably self contained fix
that removes the need for some hard coded paths in GDB. If anyone
spots any nits post commit I'm always happy to resolve them later.
Thanks,
Andrew
* Andrew Burgess <andrew.burgess@embecosm.com> [2020-10-21 09:22:35 +0100]:
> GDB has two approaches to generating the target descriptions found in
> gdb/features/, the whole description approach, where the XML file
> contains a complete target description which is then used to generate
> a single C file that builds that target description. Or, the split
> feature approach, where the XML files contain a single target feature,
> each feature results in a single C file to create that one feature,
> and then a manually written C file is used to build a complete target
> description from individual features.
>
> There's a Makefile, gdb/features/Makefile, which is responsible for
> managing the regeneration of the C files from the XML files.
>
> However, some of the logic the selects between the whole description
> approach, or the split feature approach is actually hard-coded into
> GDB, inside target-descriptions.c:maint_print_c_tdesc_cmd we check the
> path to the incoming XML file and use this to choose which type of C
> file we should generate.
>
> This commit removes this hard coding from GDB, and makes the Makefile
> entirely responsible for choosing the approach. This makes sense as
> the Makefile already has the XML files partitioned based on which
> approach they should use.
>
> In order to allow this change the 'maint print c-tdesc' command is
> given a new command option '-single-feature', which tells GDB which
> type of C file should be created. The makefile now supplies this flag
> to GDB.
>
> This did reveal a bug in features/Makefile, the rx.xml file was in the
> wrong list, this didn't matter previously as the actual choice of
> which approach to use was done in GDB. Now the Makefile decides, so
> placing each XML file in the correct list is critical.
>
> Tested this by doing 'make GDB=/path/to/gdb clean-cfiles cfiles' to
> regenerate all the C files from their XML source. There are no
> changes after this commit.
>
> gdb/ChangeLog:
>
> * features/Makefile (XMLTOC): Add rx.xml.
> (FEATURE_XMLFILES): Remove rx.xml.
> (FEATURE_CFILES rule): Pass '-single-feature' flag.
> * features/rx.c: Regenerate.
> * features/rx.xml: Wrap in `target` tags, and reindent.
> * target-descriptions.c (struct maint_print_c_tdesc_options): New
> structure.
> (maint_print_c_tdesc_opt_def): New typedef.
> (maint_print_c_tdesc_opt_defs): New static global.
> (make_maint_print_c_tdesc_options_def_group): New function.
> (maint_print_c_tdesc_cmd): Make use of command line flags, only
> print single feature C file for target descriptions containing a
> single feature.
> (maint_print_c_tdesc_cmd_completer): New function.
> (_initialize_target_descriptions): Update call to register command
> completer, and include command line flag in help text.
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (Maintenance Commands): Update description of 'maint
> print c-tdesc'.
> ---
> gdb/ChangeLog | 19 ++++++
> gdb/doc/ChangeLog | 5 ++
> gdb/doc/gdb.texinfo | 9 ++-
> gdb/features/Makefile | 4 +-
> gdb/features/rx.c | 2 +-
> gdb/features/rx.xml | 130 +++++++++++++++++++-------------------
> gdb/target-descriptions.c | 85 +++++++++++++++++++++----
> 7 files changed, 173 insertions(+), 81 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 2636b6f9903..ad0d587b366 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -38594,8 +38594,8 @@
> Print the entire architecture configuration. The optional argument
> @var{file} names the file where the output goes.
>
> -@kindex maint print c-tdesc @r{[}@var{file}@r{]}
> -@item maint print c-tdesc
> +@kindex maint print c-tdesc
> +@item maint print c-tdesc @r{[}-single-feature@r{]} @r{[}@var{file}@r{]}
> Print the target description (@pxref{Target Descriptions}) as
> a C source file. By default, the target description is for the current
> target, but if the optional argument @var{file} is provided, that file
> @@ -38605,6 +38605,11 @@
> built again. This command is used by developers after they add or
> modify XML target descriptions.
>
> +When the optional flag @samp{-single-feature} is provided then the
> +target description being processed (either the default, or from
> +@var{file}) must only contain a single feature. The source file
> +produced is different in this case.
> +
> @kindex maint print xml-tdesc
> @item maint print xml-tdesc @r{[}@var{file}@r{]}
> Print the target description (@pxref{Target Descriptions}) as an XML
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 689603847a0..d5b3236242d 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -153,6 +153,7 @@ XMLTOC = \
> rs6000/powerpc-vsx64.xml \
> rs6000/powerpc-vsx64l.xml \
> rs6000/rs6000.xml \
> + rx.xml \
> s390-linux32.xml \
> s390-linux32v1.xml \
> s390-linux32v2.xml \
> @@ -232,7 +233,6 @@ FEATURE_XMLFILES = aarch64-core.xml \
> riscv/32bit-fpu.xml \
> riscv/64bit-cpu.xml \
> riscv/64bit-fpu.xml \
> - rx.xml \
> tic6x-c6xp.xml \
> tic6x-core.xml \
> tic6x-gp.xml
> @@ -247,7 +247,7 @@ $(TDESC_CFILES): %.c: %.xml
>
> $(FEATURE_CFILES): %.c: %.xml.tmp
> $(GDB) -nx -q -batch \
> - -ex 'maint print c-tdesc $<' > $@.tmp
> + -ex 'maint print c-tdesc -single-feature $<' > $@.tmp
> sh ../../move-if-change $@.tmp $@
> rm $<
>
> diff --git a/gdb/features/rx.c b/gdb/features/rx.c
> index ebbc504ec35..dd765c2c63a 100644
> --- a/gdb/features/rx.c
> +++ b/gdb/features/rx.c
> @@ -1,5 +1,5 @@
> /* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro:
> - Original: rx.xml.tmp */
> + Original: rx.xml */
>
> #include "defs.h"
> #include "osabi.h"
> diff --git a/gdb/features/rx.xml b/gdb/features/rx.xml
> index d19b53676a3..35348e8b3e5 100644
> --- a/gdb/features/rx.xml
> +++ b/gdb/features/rx.xml
> @@ -5,70 +5,72 @@
> are permitted in any medium without royalty provided the copyright
> notice and this notice are preserved. -->
>
> -<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> -<feature name="org.gnu.gdb.rx.core">
> - <reg name="r0" bitsize="32" type="data_ptr"/>
> - <reg name="r1" bitsize="32" type="uint32"/>
> - <reg name="r2" bitsize="32" type="uint32"/>
> - <reg name="r3" bitsize="32" type="uint32"/>
> - <reg name="r4" bitsize="32" type="uint32"/>
> - <reg name="r5" bitsize="32" type="uint32"/>
> - <reg name="r6" bitsize="32" type="uint32"/>
> - <reg name="r7" bitsize="32" type="uint32"/>
> - <reg name="r8" bitsize="32" type="uint32"/>
> - <reg name="r9" bitsize="32" type="uint32"/>
> - <reg name="r10" bitsize="32" type="uint32"/>
> - <reg name="r11" bitsize="32" type="uint32"/>
> - <reg name="r12" bitsize="32" type="uint32"/>
> - <reg name="r13" bitsize="32" type="uint32"/>
> - <reg name="r14" bitsize="32" type="uint32"/>
> - <reg name="r15" bitsize="32" type="uint32"/>
> +<!DOCTYPE target SYSTEM "gdb-target.dtd">
> +<target>
> + <feature name="org.gnu.gdb.rx.core">
> + <reg name="r0" bitsize="32" type="data_ptr"/>
> + <reg name="r1" bitsize="32" type="uint32"/>
> + <reg name="r2" bitsize="32" type="uint32"/>
> + <reg name="r3" bitsize="32" type="uint32"/>
> + <reg name="r4" bitsize="32" type="uint32"/>
> + <reg name="r5" bitsize="32" type="uint32"/>
> + <reg name="r6" bitsize="32" type="uint32"/>
> + <reg name="r7" bitsize="32" type="uint32"/>
> + <reg name="r8" bitsize="32" type="uint32"/>
> + <reg name="r9" bitsize="32" type="uint32"/>
> + <reg name="r10" bitsize="32" type="uint32"/>
> + <reg name="r11" bitsize="32" type="uint32"/>
> + <reg name="r12" bitsize="32" type="uint32"/>
> + <reg name="r13" bitsize="32" type="uint32"/>
> + <reg name="r14" bitsize="32" type="uint32"/>
> + <reg name="r15" bitsize="32" type="uint32"/>
>
> - <flags id="psw_flags" size="4">
> - <field name="C" start="0" end="0"/>
> - <field name="Z" start="1" end="1"/>
> - <field name="S" start="2" end="2"/>
> - <field name="O" start="3" end="3"/>
> - <field name="I" start="16" end="16"/>
> - <field name="U" start="17" end="17"/>
> - <field name="PM" start="20" end="20"/>
> - <field name="IPL0" start="24" end="24"/>
> - <field name="IPL1" start="25" end="25"/>
> - <field name="IPL2" start="26" end="26"/>
> - <field name="IPL3" start="27" end="27"/>
> - </flags>
> + <flags id="psw_flags" size="4">
> + <field name="C" start="0" end="0"/>
> + <field name="Z" start="1" end="1"/>
> + <field name="S" start="2" end="2"/>
> + <field name="O" start="3" end="3"/>
> + <field name="I" start="16" end="16"/>
> + <field name="U" start="17" end="17"/>
> + <field name="PM" start="20" end="20"/>
> + <field name="IPL0" start="24" end="24"/>
> + <field name="IPL1" start="25" end="25"/>
> + <field name="IPL2" start="26" end="26"/>
> + <field name="IPL3" start="27" end="27"/>
> + </flags>
>
> - <flags id="fpsw_flags" size="4">
> - <field name="RM0" start="0" end="0"/>
> - <field name="RM1" start="1" end="1"/>
> - <field name="CV" start="2" end="2"/>
> - <field name="CO" start="3" end="3"/>
> - <field name="CZ" start="4" end="4"/>
> - <field name="CU" start="5" end="5"/>
> - <field name="CX" start="6" end="6"/>
> - <field name="CE" start="7" end="7"/>
> - <field name="DN" start="8" end="8"/>
> - <field name="EV" start="10" end="10"/>
> - <field name="EO" start="11" end="11"/>
> - <field name="EZ" start="12" end="12"/>
> - <field name="EU" start="13" end="13"/>
> - <field name="EX" start="14" end="14"/>
> - <field name="FV" start="26" end="26"/>
> - <field name="FO" start="27" end="27"/>
> - <field name="FZ" start="28" end="28"/>
> - <field name="FU" start="29" end="29"/>
> - <field name="FX" start="30" end="30"/>
> - <field name="FS" start="31" end="31"/>
> - </flags>
> + <flags id="fpsw_flags" size="4">
> + <field name="RM0" start="0" end="0"/>
> + <field name="RM1" start="1" end="1"/>
> + <field name="CV" start="2" end="2"/>
> + <field name="CO" start="3" end="3"/>
> + <field name="CZ" start="4" end="4"/>
> + <field name="CU" start="5" end="5"/>
> + <field name="CX" start="6" end="6"/>
> + <field name="CE" start="7" end="7"/>
> + <field name="DN" start="8" end="8"/>
> + <field name="EV" start="10" end="10"/>
> + <field name="EO" start="11" end="11"/>
> + <field name="EZ" start="12" end="12"/>
> + <field name="EU" start="13" end="13"/>
> + <field name="EX" start="14" end="14"/>
> + <field name="FV" start="26" end="26"/>
> + <field name="FO" start="27" end="27"/>
> + <field name="FZ" start="28" end="28"/>
> + <field name="FU" start="29" end="29"/>
> + <field name="FX" start="30" end="30"/>
> + <field name="FS" start="31" end="31"/>
> + </flags>
>
> - <reg name="usp" bitsize="32" type="data_ptr"/>
> - <reg name="isp" bitsize="32" type="data_ptr"/>
> - <reg name="psw" bitsize="32" type="psw_flags"/>
> - <reg name="pc" bitsize="32" type="code_ptr"/>
> - <reg name="intb" bitsize="32" type="data_ptr"/>
> - <reg name="bpsw" bitsize="32" type="psw_flags"/>
> - <reg name="bpc" bitsize="32" type="code_ptr"/>
> - <reg name="fintv" bitsize="32" type="code_ptr"/>
> - <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
> - <reg name="acc" bitsize="64" type="uint64"/>
> -</feature>
> + <reg name="usp" bitsize="32" type="data_ptr"/>
> + <reg name="isp" bitsize="32" type="data_ptr"/>
> + <reg name="psw" bitsize="32" type="psw_flags"/>
> + <reg name="pc" bitsize="32" type="code_ptr"/>
> + <reg name="intb" bitsize="32" type="data_ptr"/>
> + <reg name="bpsw" bitsize="32" type="psw_flags"/>
> + <reg name="bpc" bitsize="32" type="code_ptr"/>
> + <reg name="fintv" bitsize="32" type="code_ptr"/>
> + <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
> + <reg name="acc" bitsize="64" type="uint64"/>
> + </feature>
> +</target>
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 6d0905b90b8..d21f6fdb63c 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -1731,12 +1731,45 @@ tdesc_get_features_xml (const target_desc *tdesc)
> return tdesc->xmltarget;
> }
>
> +/* Data structures and functions to setup the option flags for 'maintenance
> + print c-tdesc command. */
> +
> +struct maint_print_c_tdesc_options
> +{
> + /* True when the '-single-feature' flag was passed. */
> + bool single_feature = false;
> +};
> +
> +using maint_print_c_tdesc_opt_def
> + = gdb::option::flag_option_def<maint_print_c_tdesc_options>;
> +
> +static const gdb::option::option_def maint_print_c_tdesc_opt_defs[] = {
> + maint_print_c_tdesc_opt_def {
> + "single-feature",
> + [] (maint_print_c_tdesc_options *opt) { return &opt->single_feature; },
> + N_("Print C description of just a single feature.")
> + },
> +};
> +
> +static inline gdb::option::option_def_group
> +make_maint_print_c_tdesc_options_def_group (maint_print_c_tdesc_options *opts)
> +{
> + return {{maint_print_c_tdesc_opt_defs}, opts};
> +}
> +
> +/* Implement 'maintenance print c-tdesc' command. */
> +
> static void
> maint_print_c_tdesc_cmd (const char *args, int from_tty)
> {
> const struct target_desc *tdesc;
> const char *filename;
>
> + maint_print_c_tdesc_options opts;
> + auto grp = make_maint_print_c_tdesc_options_def_group (&opts);
> + gdb::option::process_options
> + (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp);
> +
> if (args == NULL)
> {
> /* Use the global target-supplied description, not the current
> @@ -1768,15 +1801,12 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
> /* Print c files for target features instead of target descriptions,
> because c files got from target features are more flexible than the
> counterparts. */
> - if (startswith (filename_after_features.c_str (), "i386/32bit-")
> - || startswith (filename_after_features.c_str (), "i386/64bit-")
> - || startswith (filename_after_features.c_str (), "i386/x32-core.xml")
> - || startswith (filename_after_features.c_str (), "riscv/")
> - || startswith (filename_after_features.c_str (), "tic6x-")
> - || startswith (filename_after_features.c_str (), "aarch64")
> - || startswith (filename_after_features.c_str (), "arm/")
> - || startswith (filename_after_features.c_str (), "arc/"))
> + if (opts.single_feature)
> {
> + if (tdesc->features.size () != 1)
> + error (_("only target descriptions with 1 feature can be used "
> + "with -single-feature option"));
> +
> print_c_feature v (filename_after_features);
>
> tdesc->accept (v);
> @@ -1789,6 +1819,22 @@ maint_print_c_tdesc_cmd (const char *args, int from_tty)
> }
> }
>
> +/* Completer for the "backtrace" command. */
> +
> +static void
> +maint_print_c_tdesc_cmd_completer (struct cmd_list_element *ignore,
> + completion_tracker &tracker,
> + const char *text, const char *word)
> +{
> + auto grp = make_maint_print_c_tdesc_options_def_group (nullptr);
> + if (gdb::option::complete_options
> + (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp))
> + return;
> +
> + word = advance_to_filename_complete_word_point (tracker, text);
> + filename_completer (ignore, tracker, text, word);
> +}
> +
> /* Implement the maintenance print xml-tdesc command. */
>
> static void
> @@ -1951,10 +1997,25 @@ Unset the file to read for an XML target description.\n\
> When unset, GDB will read the description from the target."),
> &tdesc_unset_cmdlist);
>
> - cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\
> -Print the current target description as a C source file."),
> - &maintenanceprintlist);
> - set_cmd_completer (cmd, filename_completer);
> + auto grp = make_maint_print_c_tdesc_options_def_group (nullptr);
> + static std::string help_text
> + = gdb::option::build_help (_("\
> +Print the current target description as a C source file.\n\
> +Usage: maintenance print c-tdesc [OPTION] [FILENAME]\n\
> +\n\
> +Options:\n\
> +%OPTIONS%\n\
> +\n\
> +When FILENAME is not provided then print the current target\n\
> +description, otherwise an XML target description is read from\n\
> +FILENAME and printed as a C function.\n\
> +\n\
> +When '-single-feature' is used then the target description should\n\
> +contain a single feature and the generated C code will only create\n\
> +that feature within an already existing target_desc object."), grp);
> + cmd = add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd,
> + help_text.c_str (), &maintenanceprintlist);
> + set_cmd_completer_handle_brkchars (cmd, maint_print_c_tdesc_cmd_completer);
>
> cmd = add_cmd ("xml-tdesc", class_maintenance, maint_print_xml_tdesc_cmd, _("\
> Print the current target description as an XML file."),
> --
> 2.25.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: add an option flag to 'maint print c-tdesc'
2020-10-21 8:22 [PATCH] gdb: add an option flag to 'maint print c-tdesc' Andrew Burgess
2020-11-11 22:42 ` Andrew Burgess
@ 2020-11-12 2:26 ` Simon Marchi
1 sibling, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2020-11-12 2:26 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 2020-10-21 4:22 a.m., Andrew Burgess wrote:
> GDB has two approaches to generating the target descriptions found in
> gdb/features/, the whole description approach, where the XML file
> contains a complete target description which is then used to generate
> a single C file that builds that target description. Or, the split
> feature approach, where the XML files contain a single target feature,
> each feature results in a single C file to create that one feature,
> and then a manually written C file is used to build a complete target
> description from individual features.
>
> There's a Makefile, gdb/features/Makefile, which is responsible for
> managing the regeneration of the C files from the XML files.
>
> However, some of the logic the selects between the whole description
second "the" -> "that"?
> approach, or the split feature approach is actually hard-coded into
> GDB, inside target-descriptions.c:maint_print_c_tdesc_cmd we check the
> path to the incoming XML file and use this to choose which type of C
> file we should generate.
>
> This commit removes this hard coding from GDB, and makes the Makefile
> entirely responsible for choosing the approach. This makes sense as
> the Makefile already has the XML files partitioned based on which
> approach they should use.
>
> In order to allow this change the 'maint print c-tdesc' command is
> given a new command option '-single-feature', which tells GDB which
> type of C file should be created. The makefile now supplies this flag
> to GDB.
>
> This did reveal a bug in features/Makefile, the rx.xml file was in the
> wrong list, this didn't matter previously as the actual choice of
> which approach to use was done in GDB. Now the Makefile decides, so
> placing each XML file in the correct list is critical.
>
> Tested this by doing 'make GDB=/path/to/gdb clean-cfiles cfiles' to
> regenerate all the C files from their XML source. There are no
> changes after this commit.
That all make sense to me, I think it's a good idea.
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-12 2:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 8:22 [PATCH] gdb: add an option flag to 'maint print c-tdesc' Andrew Burgess
2020-11-11 22:42 ` Andrew Burgess
2020-11-12 2:26 ` Simon Marchi
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).