[1/3] Use ui-out tables in some "maint print" commands
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
This changes various "maint print" register commands to use ui-out
tables rather than the current printf approach.
---
gdb/regcache-dump.c | 210 ++++++++++++++++++--------------
gdb/regcache.c | 132 +++++++++-----------
gdb/regcache.h | 11 +-
gdb/testsuite/gdb.base/maint.exp | 4 +-
gdb/testsuite/gdb.server/server-run.exp | 2 +-
5 files changed, 191 insertions(+), 168 deletions(-)
Comments
Hi Tom,
Thank you for working on this. Please see my feedback below.
> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Saturday, October 12, 2024 1:25 AM
> To: gdb-patches@sourceware.org
> Subject: [PATCH 1/3] Use ui-out tables in some "maint print" commands
>
> This changes various "maint print" register commands to use ui-out tables
> rather than the current printf approach.
> ---
> gdb/regcache-dump.c | 210 ++++++++++++++++++--------------
> gdb/regcache.c | 132 +++++++++-----------
> gdb/regcache.h | 11 +-
> gdb/testsuite/gdb.base/maint.exp | 4 +-
> gdb/testsuite/gdb.server/server-run.exp | 2 +-
> 5 files changed, 191 insertions(+), 168 deletions(-)
>
> diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c index
> 6b711bf6c2a01e21b89b952e9926a7acfb27540b..f71f05bbdc77c6956bfb09f1
> cc62b57245ee59f0 100644
> --- a/gdb/regcache-dump.c
> +++ b/gdb/regcache-dump.c
> @@ -38,43 +38,45 @@ class register_dump_regcache : public register_dump
> }
>
> protected:
> - void dump_reg (ui_file *file, int regnum) override
> +
> + int num_additional_headers () override { return 1; }
> +
> + void additional_headers (ui_out *out) override
> {
> - if (regnum < 0)
> + out->table_header (0, ui_left, "value",
> + m_dump_pseudo ? "Cooked value" : "Raw value");
> + }
> +
> + void dump_reg (ui_out *out, int regnum) override {
> + if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
> {
> - if (m_dump_pseudo)
> - gdb_printf (file, "Cooked value");
> + auto size = register_size (m_gdbarch, regnum);
I know that "auto" has been used in the code before, too. But I still wonder if we
should use it here, as register_size simply returns an int, which is not really harder
to write out.
> +
> + if (size == 0)
> + return;
> +
> + gdb::byte_vector buf (size);
> + auto status = m_regcache->cooked_read (regnum, buf.data ());
Similar commit for "auto" here. Should we really use it for "status"?
Also, I noticed that for some other dump_reg functions you touch below, there is
an assert for the register status:
gdb_assert (status != REG_VALID);
Would it make sense to add it here, too?
> + if (status == REG_UNKNOWN)
> + out->field_string ("value", "<invalid>");
> + else if (status == REG_UNAVAILABLE)
> + out->field_string ("value", "<unavailable>");
> else
> - gdb_printf (file, "Raw value");
> + {
> + string_file str;
> + print_hex_chars (&str, buf.data (), size,
> + gdbarch_byte_order (m_gdbarch), true);
> + out->field_stream ("value", str);
> + }
> }
> else
> {
> - if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
> - {
> - auto size = register_size (m_gdbarch, regnum);
> -
> - if (size == 0)
> - return;
> -
> - gdb::byte_vector buf (size);
> - auto status = m_regcache->cooked_read (regnum, buf.data ());
Same comment as before on "auto" used for status and size here.
Also for the assert.
> -
> - if (status == REG_UNKNOWN)
> - gdb_printf (file, "<invalid>");
> - else if (status == REG_UNAVAILABLE)
> - gdb_printf (file, "<unavailable>");
> - else
> - {
> - print_hex_chars (file, buf.data (), size,
> - gdbarch_byte_order (m_gdbarch), true);
> - }
> - }
> - else
> - {
> - /* Just print "<cooked>" for pseudo register when
> - regcache_dump_raw. */
> - gdb_printf (file, "<cooked>");
> - }
> + /* Just print "<cooked>" for pseudo register when
> + regcache_dump_raw. */
> + out->field_string ("value", "<cooked>");
> }
> }
>
> @@ -97,39 +99,39 @@ class register_dump_reg_buffer : public
> register_dump, reg_buffer
> }
>
> protected:
> - void dump_reg (ui_file *file, int regnum) override
> +
> + int num_additional_headers () override { return 1; }
> +
> + void additional_headers (ui_out *out) override
> {
> - if (regnum < 0)
> - {
> - if (m_has_pseudo)
> - gdb_printf (file, "Cooked value");
> - else
> - gdb_printf (file, "Raw value");
> - }
> - else
> + out->table_header (0, ui_left, "value",
> + m_has_pseudo ? "Cooked value" : "Raw value");
> + }
> +
> + void dump_reg (ui_out *out, int regnum) override {
> + if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
> {
> - if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
> - {
> - auto size = register_size (m_gdbarch, regnum);
> + auto size = register_size (m_gdbarch, regnum);
>
> - if (size == 0)
> - return;
> + if (size == 0)
> + return;
>
> - auto status = get_register_status (regnum);
> + auto status = get_register_status (regnum);
Again, same comment for "auto" and "status".
> - gdb_assert (status != REG_VALID);
> + gdb_assert (status != REG_VALID);
>
Here we have the assert I talked about before.
> - if (status == REG_UNKNOWN)
> - gdb_printf (file, "<invalid>");
> - else
> - gdb_printf (file, "<unavailable>");
> - }
> + if (status == REG_UNKNOWN)
> + out->field_string ("value", "<invalid>");
> else
> - {
> - /* Just print "<cooked>" for pseudo register when
> - regcache_dump_raw. */
> - gdb_printf (file, "<cooked>");
> - }
> + out->field_string ("value", "<unavailable>");
> + }
> + else
> + {
> + /* Just print "<cooked>" for pseudo register when
> + regcache_dump_raw. */
> + out->field_string ("value", "<cooked>");
> }
> }
> };
> @@ -144,7 +146,14 @@ class register_dump_none : public register_dump
> {}
>
> protected:
> - void dump_reg (ui_file *file, int regnum) override
> +
> + int num_additional_headers () override { return 0; }
> +
> + void additional_headers (ui_out *out) override { }
> +
> + void dump_reg (ui_out *out, int regnum) override
> {}
> };
>
> @@ -158,24 +167,38 @@ class register_dump_remote : public
> register_dump
> {}
>
> protected:
> - void dump_reg (ui_file *file, int regnum) override
> +
> + int num_additional_headers () override { return 3; }
> +
> + void additional_headers (ui_out *out) override {
> + out->table_header (7, ui_left, "remnum", "Rmt Nr");
> + out->table_header (11, ui_left, "goffset", "g/G Offset");
> + out->table_header (3, ui_left, "expedited", "Expedited"); }
> +
> + void dump_reg (ui_out *out, int regnum) override
> {
> - if (regnum < 0)
> + int pnum, poffset;
> +
> + if (regnum < gdbarch_num_regs (m_gdbarch)
> + && remote_register_number_and_offset (m_gdbarch, regnum,
> + &pnum, &poffset))
> {
> - gdb_printf (file, "Rmt Nr g/G Offset Expedited");
> + out->field_signed ("remnum", pnum);
> + out->field_signed ("goffset", poffset);
> +
> + if (remote_register_is_expedited (regnum))
> + out->field_string ("expedited", "yes");
> + else
> + out->field_skip ("expedited");
> }
> - else if (regnum < gdbarch_num_regs (m_gdbarch))
> + else
> {
> - int pnum, poffset;
> -
> - if (remote_register_number_and_offset (m_gdbarch, regnum,
> - &pnum, &poffset))
> - {
> - if (remote_register_is_expedited (regnum))
> - gdb_printf (file, "%7d %11d yes", pnum, poffset);
> - else
> - gdb_printf (file, "%7d %11d", pnum, poffset);
> - }
> + out->field_skip ("remnum");
> + out->field_skip ("goffset");
> + out->field_skip ("expedited");
> }
> }
> };
> @@ -190,22 +213,28 @@ class register_dump_groups : public register_dump
> {}
>
> protected:
> - void dump_reg (ui_file *file, int regnum) override
> +
> + int num_additional_headers () override { return 1; }
> +
> + void additional_headers (ui_out *out) override
> {
> - if (regnum < 0)
> - gdb_printf (file, "Groups");
> - else
> + out->table_header (0, ui_left, "groups", "Groups"); }
> +
> + void dump_reg (ui_out *out, int regnum) override {
> + string_file file;
> + const char *sep = "";
> + for (const struct reggroup *group : gdbarch_reggroups (m_gdbarch))
I think we can omit the "struct" from " const struct reggroup *group"
> {
> - const char *sep = "";
> - for (const struct reggroup *group : gdbarch_reggroups (m_gdbarch))
> + if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
> {
> - if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
> - {
> - gdb_printf (file, "%s%s", sep, group->name ());
> - sep = ",";
> - }
> + gdb_printf (&file, "%s%s", sep, group->name ());
> + sep = ",";
> }
> }
> + out->field_stream ("groups", file);
> }
> };
>
> @@ -221,15 +250,13 @@ regcache_print (const char *args, enum
> regcache_dump_what what_to_dump) {
> /* Where to send output. */
> stdio_file file;
> - ui_file *out;
> + std::optional<ui_out_redirect_pop> redirect;
>
> - if (args == NULL)
> - out = gdb_stdout;
> - else
> + if (args != nullptr)
> {
> if (!file.open (args, "w"))
> perror_with_name (_("maintenance print architecture"));
> - out = &file;
> + redirect.emplace (current_uiout, &file);
> }
>
> std::unique_ptr<register_dump> dump;
> @@ -241,20 +268,25 @@ regcache_print (const char *args, enum
> regcache_dump_what what_to_dump)
I think there is an unused unique pointer in this function:
" std::unique_ptr<regcache> regs;"
I know you did not touch that line, but I wonder if we can remove it,
while you're at it.
> else
> gdbarch = current_inferior ()->arch ();
>
> + const char *name;
> switch (what_to_dump)
> {
> case regcache_dump_none:
> dump.reset (new register_dump_none (gdbarch));
> + name = "Registers";
> break;
> case regcache_dump_remote:
> dump.reset (new register_dump_remote (gdbarch));
> + name = "RegisterRemote";
> break;
> case regcache_dump_groups:
> dump.reset (new register_dump_groups (gdbarch));
> + name = "RegisterGroups";
> break;
> case regcache_dump_raw:
> case regcache_dump_cooked:
> {
> + name = "RegisterDump";
> auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
>
> if (target_has_registers ())
> @@ -272,7 +304,7 @@ regcache_print (const char *args, enum
> regcache_dump_what what_to_dump)
> break;
> }
>
> - dump->dump (out);
> + dump->dump (current_uiout, name);
> }
>
> static void
> diff --git a/gdb/regcache.c b/gdb/regcache.c index
> f04354d822f9c78db671e9cd41f15811b0770bd8..c0433121d2fd6312bbafaca9
> 1fe8c3a536ffe7ec 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1509,7 +1509,7 @@ reg_flush_command (const char *command, int
> from_tty) }
>
> void
> -register_dump::dump (ui_file *file)
> +register_dump::dump (ui_out *out, const char *name)
> {
> auto descr = regcache_descr (m_gdbarch);
> int regnum;
> @@ -1521,107 +1521,91 @@ register_dump::dump (ui_file *file)
> gdb_assert (descr->nr_cooked_registers
> == gdbarch_num_cooked_regs (m_gdbarch));
>
> - for (regnum = -1; regnum < descr->nr_cooked_registers; regnum++)
> + ui_out_emit_table table (out, 6 + num_additional_headers (), -1,
> + name); out->table_header (10, ui_left, "name", "Name");
> + out->table_header (4, ui_left, "num", "Nr"); out->table_header (4,
> + ui_left, "relnum", "Rel"); out->table_header (8, ui_left, "offset",
> + "Offset"); out->table_header (5, ui_left, "size", "Size");
> + out->table_header (15, ui_left, "type", "Type"); additional_headers
> + (out); out->table_body ();
> +
> + for (regnum = 0; regnum < descr->nr_cooked_registers; regnum++)
> {
> - /* Name. */
> - if (regnum < 0)
> - gdb_printf (file, " %-10s", "Name");
> - else
> - {
> - const char *p = gdbarch_register_name (m_gdbarch, regnum);
> + ui_out_emit_tuple tuple_emitter (out, nullptr);
>
> - if (p[0] == '\0')
> - p = "''";
> - gdb_printf (file, " %-10s", p);
> - }
> + /* Name. */
> + const char *p = gdbarch_register_name (m_gdbarch, regnum);
> + if (p[0] == '\0')
> + p = "''";
> + out->field_string ("name", p);
>
> /* Number. */
> - if (regnum < 0)
> - gdb_printf (file, " %4s", "Nr");
> - else
> - gdb_printf (file, " %4d", regnum);
> + out->field_signed ("num", regnum);
>
> /* Relative number. */
> - if (regnum < 0)
> - gdb_printf (file, " %4s", "Rel");
> - else if (regnum < gdbarch_num_regs (m_gdbarch))
> - gdb_printf (file, " %4d", regnum);
> + if (regnum < gdbarch_num_regs (m_gdbarch))
> + out->field_signed ("relnum", regnum);
> else
> - gdb_printf (file, " %4d",
> - (regnum - gdbarch_num_regs (m_gdbarch)));
> + out->field_signed ("relnum", (regnum - gdbarch_num_regs
> (m_gdbarch)));
>
> /* Offset. */
> - if (regnum < 0)
> - gdb_printf (file, " %6s ", "Offset");
> - else
> + if (register_offset != descr->register_offset[regnum]
> + || (regnum > 0
> + && (descr->register_offset[regnum]
> + != (descr->register_offset[regnum - 1]
> + + descr->sizeof_register[regnum - 1]))))
> {
> - gdb_printf (file, " %6ld",
> - descr->register_offset[regnum]);
> - if (register_offset != descr->register_offset[regnum]
> - || (regnum > 0
> - && (descr->register_offset[regnum]
> - != (descr->register_offset[regnum - 1]
> - + descr->sizeof_register[regnum - 1])))
> - )
> - {
> - if (!footnote_register_offset)
> - footnote_register_offset = ++footnote_nr;
> - gdb_printf (file, "*%d", footnote_register_offset);
> - }
> - else
> - gdb_printf (file, " ");
> - register_offset = (descr->register_offset[regnum]
> - + descr->sizeof_register[regnum]);
> + if (!footnote_register_offset)
> + footnote_register_offset = ++footnote_nr;
> + std::string val = string_printf ("%ld*%d",
> + descr->register_offset[regnum],
> + footnote_register_offset);
> + out->field_string ("offset", val);
> }
> + else
> + out->field_signed ("offset", descr->register_offset[regnum]);
> + register_offset = (descr->register_offset[regnum]
> + + descr->sizeof_register[regnum]);
>
> /* Size. */
> - if (regnum < 0)
> - gdb_printf (file, " %5s ", "Size");
> - else
> - gdb_printf (file, " %5ld", descr->sizeof_register[regnum]);
> + out->field_signed ("size", descr->sizeof_register[regnum]);
>
> /* Type. */
> {
> const char *t;
> std::string name_holder;
>
> - if (regnum < 0)
> - t = "Type";
> - else
> + static const char blt[] = "builtin_type";
> +
> + t = register_type (m_gdbarch, regnum)->name ();
> + if (t == NULL)
Nit: better use nullptr
> {
> - static const char blt[] = "builtin_type";
> -
> - t = register_type (m_gdbarch, regnum)->name ();
> - if (t == NULL)
> - {
> - if (!footnote_register_type_name_null)
> - footnote_register_type_name_null = ++footnote_nr;
> - name_holder = string_printf ("*%d",
> -
> footnote_register_type_name_null);
> - t = name_holder.c_str ();
> - }
> - /* Chop a leading builtin_type. */
> - if (startswith (t, blt))
> - t += strlen (blt);
> + if (!footnote_register_type_name_null)
> + footnote_register_type_name_null = ++footnote_nr;
> + name_holder = string_printf ("*%d",
> + footnote_register_type_name_null);
> + t = name_holder.c_str ();
> }
> - gdb_printf (file, " %-15s", t);
> - }
> + /* Chop a leading builtin_type. */
> + if (startswith (t, blt))
> + t += strlen (blt);
>
> - /* Leading space always present. */
> - gdb_printf (file, " ");
> + out->field_string ("type", t);
> + }
>
> - dump_reg (file, regnum);
> + dump_reg (out, regnum);
>
> - gdb_printf (file, "\n");
> + out->text ("\n");
> }
>
> if (footnote_register_offset)
> - gdb_printf (file, "*%d: Inconsistent register offsets.\n",
> - footnote_register_offset);
> + out->message ("*%d: Inconsistent register offsets.\n",
> + footnote_register_offset);
> if (footnote_register_type_name_null)
> - gdb_printf (file,
> - "*%d: Register type's name NULL.\n",
> - footnote_register_type_name_null);
> + out->message ("*%d: Register type's name NULL.\n",
> + footnote_register_type_name_null);
Nit: As the if blocks cover more than one line, should we use brackets here?
> }
>
> #if GDB_SELF_TEST
> diff --git a/gdb/regcache.h b/gdb/regcache.h index
> 2f4b7d94c69344bbe9d2f5b716874fc47c6720c4..916044e3a793b20886b7460
> 2c3aefcd89742e487 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -31,6 +31,7 @@ struct gdbarch;
> class thread_info;
> struct process_stratum_target;
> struct inferior;
> +class ui_out;
>
> extern struct regcache *get_thread_regcache (process_stratum_target
> *target,
> ptid_t ptid);
> @@ -533,7 +534,7 @@ extern void registers_changed_thread (thread_info
> *thread); class register_dump {
> public:
> - void dump (ui_file *file);
> + void dump (ui_out *out, const char *name);
> virtual ~register_dump () = default;
>
> protected:
> @@ -541,9 +542,15 @@ class register_dump
> : m_gdbarch (arch)
> {}
>
> + /* Number of additional table headers. */ virtual int
> + num_additional_headers () = 0;
> +
> + /* Add the additional headers to OUT. */ virtual void
> + additional_headers (ui_out *out) = 0;
> +
> /* Dump the register REGNUM contents. If REGNUM is -1, print the
> header. */
> - virtual void dump_reg (ui_file *file, int regnum) = 0;
> + virtual void dump_reg (ui_out *out, int regnum) = 0;
"If REGNUM is -1, print the ... " Does this part of the comment still apply ?
BR,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
>>>>> ">" == Schimpe, Christina <christina.schimpe@intel.com> writes:
>> + auto size = register_size (m_gdbarch, regnum);
>> I know that "auto" has been used in the code before, too. But I still wonder if we
>> should use it here, as register_size simply returns an int, which is not really harder
>> to write out.
I think most of the comments you made here are on pre-existing code. A
lot of this patch is just re-indenting some code because I removed a
layer of braces.
Would you mind re-reading the patch with that in mind?
If you really want those changes made, I will see if I can write a
separate cleanup and then try again.
Tom
@@ -38,43 +38,45 @@ class register_dump_regcache : public register_dump
}
protected:
- void dump_reg (ui_file *file, int regnum) override
+
+ int num_additional_headers () override
+ { return 1; }
+
+ void additional_headers (ui_out *out) override
{
- if (regnum < 0)
+ out->table_header (0, ui_left, "value",
+ m_dump_pseudo ? "Cooked value" : "Raw value");
+ }
+
+ void dump_reg (ui_out *out, int regnum) override
+ {
+ if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
{
- if (m_dump_pseudo)
- gdb_printf (file, "Cooked value");
+ auto size = register_size (m_gdbarch, regnum);
+
+ if (size == 0)
+ return;
+
+ gdb::byte_vector buf (size);
+ auto status = m_regcache->cooked_read (regnum, buf.data ());
+
+ if (status == REG_UNKNOWN)
+ out->field_string ("value", "<invalid>");
+ else if (status == REG_UNAVAILABLE)
+ out->field_string ("value", "<unavailable>");
else
- gdb_printf (file, "Raw value");
+ {
+ string_file str;
+ print_hex_chars (&str, buf.data (), size,
+ gdbarch_byte_order (m_gdbarch), true);
+ out->field_stream ("value", str);
+ }
}
else
{
- if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
- {
- auto size = register_size (m_gdbarch, regnum);
-
- if (size == 0)
- return;
-
- gdb::byte_vector buf (size);
- auto status = m_regcache->cooked_read (regnum, buf.data ());
-
- if (status == REG_UNKNOWN)
- gdb_printf (file, "<invalid>");
- else if (status == REG_UNAVAILABLE)
- gdb_printf (file, "<unavailable>");
- else
- {
- print_hex_chars (file, buf.data (), size,
- gdbarch_byte_order (m_gdbarch), true);
- }
- }
- else
- {
- /* Just print "<cooked>" for pseudo register when
- regcache_dump_raw. */
- gdb_printf (file, "<cooked>");
- }
+ /* Just print "<cooked>" for pseudo register when
+ regcache_dump_raw. */
+ out->field_string ("value", "<cooked>");
}
}
@@ -97,39 +99,39 @@ class register_dump_reg_buffer : public register_dump, reg_buffer
}
protected:
- void dump_reg (ui_file *file, int regnum) override
+
+ int num_additional_headers () override
+ { return 1; }
+
+ void additional_headers (ui_out *out) override
{
- if (regnum < 0)
- {
- if (m_has_pseudo)
- gdb_printf (file, "Cooked value");
- else
- gdb_printf (file, "Raw value");
- }
- else
+ out->table_header (0, ui_left, "value",
+ m_has_pseudo ? "Cooked value" : "Raw value");
+ }
+
+ void dump_reg (ui_out *out, int regnum) override
+ {
+ if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
{
- if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
- {
- auto size = register_size (m_gdbarch, regnum);
+ auto size = register_size (m_gdbarch, regnum);
- if (size == 0)
- return;
+ if (size == 0)
+ return;
- auto status = get_register_status (regnum);
+ auto status = get_register_status (regnum);
- gdb_assert (status != REG_VALID);
+ gdb_assert (status != REG_VALID);
- if (status == REG_UNKNOWN)
- gdb_printf (file, "<invalid>");
- else
- gdb_printf (file, "<unavailable>");
- }
+ if (status == REG_UNKNOWN)
+ out->field_string ("value", "<invalid>");
else
- {
- /* Just print "<cooked>" for pseudo register when
- regcache_dump_raw. */
- gdb_printf (file, "<cooked>");
- }
+ out->field_string ("value", "<unavailable>");
+ }
+ else
+ {
+ /* Just print "<cooked>" for pseudo register when
+ regcache_dump_raw. */
+ out->field_string ("value", "<cooked>");
}
}
};
@@ -144,7 +146,14 @@ class register_dump_none : public register_dump
{}
protected:
- void dump_reg (ui_file *file, int regnum) override
+
+ int num_additional_headers () override
+ { return 0; }
+
+ void additional_headers (ui_out *out) override
+ { }
+
+ void dump_reg (ui_out *out, int regnum) override
{}
};
@@ -158,24 +167,38 @@ class register_dump_remote : public register_dump
{}
protected:
- void dump_reg (ui_file *file, int regnum) override
+
+ int num_additional_headers () override
+ { return 3; }
+
+ void additional_headers (ui_out *out) override
+ {
+ out->table_header (7, ui_left, "remnum", "Rmt Nr");
+ out->table_header (11, ui_left, "goffset", "g/G Offset");
+ out->table_header (3, ui_left, "expedited", "Expedited");
+ }
+
+ void dump_reg (ui_out *out, int regnum) override
{
- if (regnum < 0)
+ int pnum, poffset;
+
+ if (regnum < gdbarch_num_regs (m_gdbarch)
+ && remote_register_number_and_offset (m_gdbarch, regnum,
+ &pnum, &poffset))
{
- gdb_printf (file, "Rmt Nr g/G Offset Expedited");
+ out->field_signed ("remnum", pnum);
+ out->field_signed ("goffset", poffset);
+
+ if (remote_register_is_expedited (regnum))
+ out->field_string ("expedited", "yes");
+ else
+ out->field_skip ("expedited");
}
- else if (regnum < gdbarch_num_regs (m_gdbarch))
+ else
{
- int pnum, poffset;
-
- if (remote_register_number_and_offset (m_gdbarch, regnum,
- &pnum, &poffset))
- {
- if (remote_register_is_expedited (regnum))
- gdb_printf (file, "%7d %11d yes", pnum, poffset);
- else
- gdb_printf (file, "%7d %11d", pnum, poffset);
- }
+ out->field_skip ("remnum");
+ out->field_skip ("goffset");
+ out->field_skip ("expedited");
}
}
};
@@ -190,22 +213,28 @@ class register_dump_groups : public register_dump
{}
protected:
- void dump_reg (ui_file *file, int regnum) override
+
+ int num_additional_headers () override
+ { return 1; }
+
+ void additional_headers (ui_out *out) override
{
- if (regnum < 0)
- gdb_printf (file, "Groups");
- else
+ out->table_header (0, ui_left, "groups", "Groups");
+ }
+
+ void dump_reg (ui_out *out, int regnum) override
+ {
+ string_file file;
+ const char *sep = "";
+ for (const struct reggroup *group : gdbarch_reggroups (m_gdbarch))
{
- const char *sep = "";
- for (const struct reggroup *group : gdbarch_reggroups (m_gdbarch))
+ if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
{
- if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
- {
- gdb_printf (file, "%s%s", sep, group->name ());
- sep = ",";
- }
+ gdb_printf (&file, "%s%s", sep, group->name ());
+ sep = ",";
}
}
+ out->field_stream ("groups", file);
}
};
@@ -221,15 +250,13 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
{
/* Where to send output. */
stdio_file file;
- ui_file *out;
+ std::optional<ui_out_redirect_pop> redirect;
- if (args == NULL)
- out = gdb_stdout;
- else
+ if (args != nullptr)
{
if (!file.open (args, "w"))
perror_with_name (_("maintenance print architecture"));
- out = &file;
+ redirect.emplace (current_uiout, &file);
}
std::unique_ptr<register_dump> dump;
@@ -241,20 +268,25 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
else
gdbarch = current_inferior ()->arch ();
+ const char *name;
switch (what_to_dump)
{
case regcache_dump_none:
dump.reset (new register_dump_none (gdbarch));
+ name = "Registers";
break;
case regcache_dump_remote:
dump.reset (new register_dump_remote (gdbarch));
+ name = "RegisterRemote";
break;
case regcache_dump_groups:
dump.reset (new register_dump_groups (gdbarch));
+ name = "RegisterGroups";
break;
case regcache_dump_raw:
case regcache_dump_cooked:
{
+ name = "RegisterDump";
auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
if (target_has_registers ())
@@ -272,7 +304,7 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
break;
}
- dump->dump (out);
+ dump->dump (current_uiout, name);
}
static void
@@ -1509,7 +1509,7 @@ reg_flush_command (const char *command, int from_tty)
}
void
-register_dump::dump (ui_file *file)
+register_dump::dump (ui_out *out, const char *name)
{
auto descr = regcache_descr (m_gdbarch);
int regnum;
@@ -1521,107 +1521,91 @@ register_dump::dump (ui_file *file)
gdb_assert (descr->nr_cooked_registers
== gdbarch_num_cooked_regs (m_gdbarch));
- for (regnum = -1; regnum < descr->nr_cooked_registers; regnum++)
+ ui_out_emit_table table (out, 6 + num_additional_headers (), -1, name);
+ out->table_header (10, ui_left, "name", "Name");
+ out->table_header (4, ui_left, "num", "Nr");
+ out->table_header (4, ui_left, "relnum", "Rel");
+ out->table_header (8, ui_left, "offset", "Offset");
+ out->table_header (5, ui_left, "size", "Size");
+ out->table_header (15, ui_left, "type", "Type");
+ additional_headers (out);
+ out->table_body ();
+
+ for (regnum = 0; regnum < descr->nr_cooked_registers; regnum++)
{
- /* Name. */
- if (regnum < 0)
- gdb_printf (file, " %-10s", "Name");
- else
- {
- const char *p = gdbarch_register_name (m_gdbarch, regnum);
+ ui_out_emit_tuple tuple_emitter (out, nullptr);
- if (p[0] == '\0')
- p = "''";
- gdb_printf (file, " %-10s", p);
- }
+ /* Name. */
+ const char *p = gdbarch_register_name (m_gdbarch, regnum);
+ if (p[0] == '\0')
+ p = "''";
+ out->field_string ("name", p);
/* Number. */
- if (regnum < 0)
- gdb_printf (file, " %4s", "Nr");
- else
- gdb_printf (file, " %4d", regnum);
+ out->field_signed ("num", regnum);
/* Relative number. */
- if (regnum < 0)
- gdb_printf (file, " %4s", "Rel");
- else if (regnum < gdbarch_num_regs (m_gdbarch))
- gdb_printf (file, " %4d", regnum);
+ if (regnum < gdbarch_num_regs (m_gdbarch))
+ out->field_signed ("relnum", regnum);
else
- gdb_printf (file, " %4d",
- (regnum - gdbarch_num_regs (m_gdbarch)));
+ out->field_signed ("relnum", (regnum - gdbarch_num_regs (m_gdbarch)));
/* Offset. */
- if (regnum < 0)
- gdb_printf (file, " %6s ", "Offset");
- else
+ if (register_offset != descr->register_offset[regnum]
+ || (regnum > 0
+ && (descr->register_offset[regnum]
+ != (descr->register_offset[regnum - 1]
+ + descr->sizeof_register[regnum - 1]))))
{
- gdb_printf (file, " %6ld",
- descr->register_offset[regnum]);
- if (register_offset != descr->register_offset[regnum]
- || (regnum > 0
- && (descr->register_offset[regnum]
- != (descr->register_offset[regnum - 1]
- + descr->sizeof_register[regnum - 1])))
- )
- {
- if (!footnote_register_offset)
- footnote_register_offset = ++footnote_nr;
- gdb_printf (file, "*%d", footnote_register_offset);
- }
- else
- gdb_printf (file, " ");
- register_offset = (descr->register_offset[regnum]
- + descr->sizeof_register[regnum]);
+ if (!footnote_register_offset)
+ footnote_register_offset = ++footnote_nr;
+ std::string val = string_printf ("%ld*%d",
+ descr->register_offset[regnum],
+ footnote_register_offset);
+ out->field_string ("offset", val);
}
+ else
+ out->field_signed ("offset", descr->register_offset[regnum]);
+ register_offset = (descr->register_offset[regnum]
+ + descr->sizeof_register[regnum]);
/* Size. */
- if (regnum < 0)
- gdb_printf (file, " %5s ", "Size");
- else
- gdb_printf (file, " %5ld", descr->sizeof_register[regnum]);
+ out->field_signed ("size", descr->sizeof_register[regnum]);
/* Type. */
{
const char *t;
std::string name_holder;
- if (regnum < 0)
- t = "Type";
- else
+ static const char blt[] = "builtin_type";
+
+ t = register_type (m_gdbarch, regnum)->name ();
+ if (t == NULL)
{
- static const char blt[] = "builtin_type";
-
- t = register_type (m_gdbarch, regnum)->name ();
- if (t == NULL)
- {
- if (!footnote_register_type_name_null)
- footnote_register_type_name_null = ++footnote_nr;
- name_holder = string_printf ("*%d",
- footnote_register_type_name_null);
- t = name_holder.c_str ();
- }
- /* Chop a leading builtin_type. */
- if (startswith (t, blt))
- t += strlen (blt);
+ if (!footnote_register_type_name_null)
+ footnote_register_type_name_null = ++footnote_nr;
+ name_holder = string_printf ("*%d",
+ footnote_register_type_name_null);
+ t = name_holder.c_str ();
}
- gdb_printf (file, " %-15s", t);
- }
+ /* Chop a leading builtin_type. */
+ if (startswith (t, blt))
+ t += strlen (blt);
- /* Leading space always present. */
- gdb_printf (file, " ");
+ out->field_string ("type", t);
+ }
- dump_reg (file, regnum);
+ dump_reg (out, regnum);
- gdb_printf (file, "\n");
+ out->text ("\n");
}
if (footnote_register_offset)
- gdb_printf (file, "*%d: Inconsistent register offsets.\n",
- footnote_register_offset);
+ out->message ("*%d: Inconsistent register offsets.\n",
+ footnote_register_offset);
if (footnote_register_type_name_null)
- gdb_printf (file,
- "*%d: Register type's name NULL.\n",
- footnote_register_type_name_null);
+ out->message ("*%d: Register type's name NULL.\n",
+ footnote_register_type_name_null);
}
#if GDB_SELF_TEST
@@ -31,6 +31,7 @@ struct gdbarch;
class thread_info;
struct process_stratum_target;
struct inferior;
+class ui_out;
extern struct regcache *get_thread_regcache (process_stratum_target *target,
ptid_t ptid);
@@ -533,7 +534,7 @@ extern void registers_changed_thread (thread_info *thread);
class register_dump
{
public:
- void dump (ui_file *file);
+ void dump (ui_out *out, const char *name);
virtual ~register_dump () = default;
protected:
@@ -541,9 +542,15 @@ class register_dump
: m_gdbarch (arch)
{}
+ /* Number of additional table headers. */
+ virtual int num_additional_headers () = 0;
+
+ /* Add the additional headers to OUT. */
+ virtual void additional_headers (ui_out *out) = 0;
+
/* Dump the register REGNUM contents. If REGNUM is -1, print the
header. */
- virtual void dump_reg (ui_file *file, int regnum) = 0;
+ virtual void dump_reg (ui_out *out, int regnum) = 0;
gdbarch *m_gdbarch;
};
@@ -70,11 +70,11 @@ set saw_registers 0
set saw_headers 0
set test "maint print registers"
gdb_test_multiple $test $test {
- -re "\[^\r\n\]+Name\[^\r\n\]+Nr\[^\r\n\]+Rel\[^\r\n\]+Offset\[^\r\n\]+Size\[^\r\n\]+Type\[^\r\n\]+\r\n" {
+ -re "Name\[^\r\n\]+Nr\[^\r\n\]+Rel\[^\r\n\]+Offset\[^\r\n\]+Size\[^\r\n\]+Type\[^\r\n\]+\r\n" {
set saw_headers 1
exp_continue
}
- -re "^\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\r\n" {
+ -re "\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\[0-9\]+\[^\r\n\]+\r\n" {
set saw_registers 1
exp_continue
}
@@ -74,7 +74,7 @@ if { [info exists pc_regname] } {
set seen_line false
gdb_test_multiple "maintenance print remote-registers" \
$expedited_pc_test_name -lbl {
- -re " ${pc_regname}\[\[:space:\]\]+${decimal}.*${decimal} yes" {
+ -re "${pc_regname}\[\[:space:\]\]+${decimal}.*${decimal}\[\[:space:\]\]+yes" {
set seen_line true
exp_continue
}