From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 5/9] gdb: check for duplicate register names in selftest
Date: Thu, 1 Sep 2022 22:31:13 +0100 [thread overview]
Message-ID: <7c3f810e976d03e516956492a16a8ae2948e3023.1662067442.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1662067442.git.aburgess@redhat.com>
Building on the previous commit, this commit extends the register_name
selftest to check for duplicate register names.
If two registers in the cooked register set (real + pseudo registers)
have the same name, then this will show up as duplicate registers in
the 'info all-registers' output, but the user will only be able to
interact with one copy of the register.
In this commit I extend the selftest that I added in the previous
commit to check for duplicate register names, I didn't include this
functionality in the previous commit because one architecture needed
fixing, and I wanted to keep those fixes separate from the fixes in
the previous commit.
The problematic architecture(s) are powerpc:750 and powerpc:604. In
both of these cases the 'dabr' register appears twice, there's a
definition of dabr in power-oea.xml which is included into both
powerpc-604.xml and powerpc-750.xml. Both of these later two xml
files also define the dabr register.
I'm hopeful that this change shouldn't break anything, but I don't
have the ability to actually test this change, however:
On the gdbserver side, neither powerpc-604.xml nor powerpc-750.xml are
mentioned in gdbserver/configure.srv, which I think means that
gdbserver will never use these descriptions, and,
Within GDB the problematic descriptions are held in the variables
tdesc_powerpc_604 and tdesc_powerpc_750, which are only mentioned in
the variants array in rs6000-tdep.c, this is used when looking up a
description based on the architecture.
For a native Linux target however, this will not be used as
ppc_linux_nat_target::read_description exists, which calls
ppc_linux_match_description, which I don't believe can return either
of the problematic descriptions.
This leaves the other native targets, FreeBSD, AIX, etc. These don't
appear to override the ::read_description method, so will potentially
return the problematic descriptions, but, in each case I think the
::fetch_registers and ::store_registers methods will ignore the dabr
register, which will leave the register as <unavailable>.
So, my proposed solution is to just remove the duplicate register from
each of powerpc-604.xml and powerpc-750.xml, then regenerate the
corresponding C++ source file. With this change made, the selftest
now passes for all architectures.
---
gdb/features/rs6000/powerpc-604.c | 13 ++++++-------
gdb/features/rs6000/powerpc-604.xml | 1 -
gdb/features/rs6000/powerpc-750.c | 1 -
gdb/features/rs6000/powerpc-750.xml | 1 -
gdb/gdbarch-selftests.c | 21 ++++++++++++++++++++-
5 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/gdb/features/rs6000/powerpc-604.c b/gdb/features/rs6000/powerpc-604.c
index fe0fe2fc7e4..474b29fed55 100644
--- a/gdb/features/rs6000/powerpc-604.c
+++ b/gdb/features/rs6000/powerpc-604.c
@@ -141,13 +141,12 @@ initialize_tdesc_powerpc_604 (void)
tdesc_create_reg (feature, "hid0", 119, 1, NULL, 32, "int");
tdesc_create_reg (feature, "hid1", 120, 1, NULL, 32, "int");
tdesc_create_reg (feature, "iabr", 121, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "dabr", 122, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "pir", 123, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "mmcr0", 124, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "pmc1", 125, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "pmc2", 126, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "sia", 127, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "sda", 128, 1, NULL, 32, "int");
+ tdesc_create_reg (feature, "pir", 122, 1, NULL, 32, "int");
+ tdesc_create_reg (feature, "mmcr0", 123, 1, NULL, 32, "int");
+ tdesc_create_reg (feature, "pmc1", 124, 1, NULL, 32, "int");
+ tdesc_create_reg (feature, "pmc2", 125, 1, NULL, 32, "int");
+ tdesc_create_reg (feature, "sia", 126, 1, NULL, 32, "int");
+ tdesc_create_reg (feature, "sda", 127, 1, NULL, 32, "int");
tdesc_powerpc_604 = result.release ();
}
diff --git a/gdb/features/rs6000/powerpc-604.xml b/gdb/features/rs6000/powerpc-604.xml
index fbc9e946d22..052cfbb7e76 100644
--- a/gdb/features/rs6000/powerpc-604.xml
+++ b/gdb/features/rs6000/powerpc-604.xml
@@ -17,7 +17,6 @@
<reg name="hid0" bitsize="32"/>
<reg name="hid1" bitsize="32"/>
<reg name="iabr" bitsize="32"/>
- <reg name="dabr" bitsize="32"/>
<reg name="pir" bitsize="32"/>
<reg name="mmcr0" bitsize="32"/>
<reg name="pmc1" bitsize="32"/>
diff --git a/gdb/features/rs6000/powerpc-750.c b/gdb/features/rs6000/powerpc-750.c
index 396ec456651..3533c42d6cb 100644
--- a/gdb/features/rs6000/powerpc-750.c
+++ b/gdb/features/rs6000/powerpc-750.c
@@ -141,7 +141,6 @@ initialize_tdesc_powerpc_750 (void)
tdesc_create_reg (feature, "hid0", 119, 1, NULL, 32, "int");
tdesc_create_reg (feature, "hid1", 120, 1, NULL, 32, "int");
tdesc_create_reg (feature, "iabr", 121, 1, NULL, 32, "int");
- tdesc_create_reg (feature, "dabr", 122, 1, NULL, 32, "int");
tdesc_create_reg (feature, "ummcr0", 124, 1, NULL, 32, "int");
tdesc_create_reg (feature, "upmc1", 125, 1, NULL, 32, "int");
tdesc_create_reg (feature, "upmc2", 126, 1, NULL, 32, "int");
diff --git a/gdb/features/rs6000/powerpc-750.xml b/gdb/features/rs6000/powerpc-750.xml
index 908ecbdfd96..dfc5a87aaa0 100644
--- a/gdb/features/rs6000/powerpc-750.xml
+++ b/gdb/features/rs6000/powerpc-750.xml
@@ -17,7 +17,6 @@
<reg name="hid0" bitsize="32"/>
<reg name="hid1" bitsize="32"/>
<reg name="iabr" bitsize="32"/>
- <reg name="dabr" bitsize="32"/>
<reg name="ummcr0" bitsize="32" regnum="124"/>
<reg name="upmc1" bitsize="32"/>
<reg name="upmc2" bitsize="32"/>
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 75a3109143f..9bb00c93ff0 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -27,6 +27,8 @@
#include "gdbarch.h"
#include "scoped-mock-context.h"
+#include <map>
+
namespace selftests {
/* Test gdbarch methods register_to_value and value_to_register. */
@@ -129,6 +131,9 @@ register_name_test (struct gdbarch *gdbarch)
{
scoped_mock_context<test_target_ops> mockctx (gdbarch);
+ /* Track the number of times each register name appears. */
+ std::map<const std::string, int> name_counts;
+
const int num_regs = gdbarch_num_cooked_regs (gdbarch);
for (auto regnum = 0; regnum < num_regs; regnum++)
{
@@ -141,8 +146,22 @@ register_name_test (struct gdbarch *gdbarch)
debug_printf ("arch: %s, register: %d returned nullptr\n",
gdbarch_bfd_arch_info (gdbarch)->printable_name,
regnum);
-
SELF_CHECK (name != nullptr);
+
+ /* Every register name, that is not the empty string, should be
+ unique. If this is not the case then the user will see duplicate
+ copies of the register in e.g. 'info registers' output, but will
+ only be able to interact with one of the copies. */
+ if (*name != '\0')
+ {
+ std::string s (name);
+ name_counts[s]++;
+ if (run_verbose() && name_counts[s] > 1)
+ debug_printf ("arch: %s, register: %d (%s) is a duplicate\n",
+ gdbarch_bfd_arch_info (gdbarch)->printable_name,
+ regnum, name);
+ SELF_CHECK (name_counts[s] == 1);
+ }
}
}
--
2.25.4
next prev parent reply other threads:[~2022-09-01 21:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
2022-09-01 21:31 ` [PATCH 1/9] gdb/testsuite: rewrite capture_command_output proc Andrew Burgess
2022-09-01 21:31 ` [PATCH 2/9] gdb/riscv: fix failure in gdb.base/completion.exp Andrew Burgess
2022-09-01 21:31 ` [PATCH 3/9] gdb/gdbarch: add a comment to gdbarch_register_name Andrew Burgess
2022-09-01 21:31 ` [PATCH 4/9] gdb: add a gdbarch_register_name self test, and fix some architectures Andrew Burgess
2022-09-01 21:31 ` Andrew Burgess [this message]
2022-09-01 21:31 ` [PATCH 6/9] gdb: add asserts to gdbarch_register_name Andrew Burgess
2022-09-21 18:04 ` Tom Tromey
2022-09-01 21:31 ` [PATCH 7/9] gdb/csky: remove nullptr return from csky_pseudo_register_name Andrew Burgess
2022-09-01 21:31 ` [PATCH 8/9] gdb: final cleanup of various gdbarch_register_name methods Andrew Burgess
2022-09-01 21:31 ` [PATCH 9/9] gdb: update now gdbarch_register_name doesn't return nullptr Andrew Burgess
2022-09-21 18:07 ` [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Tom Tromey
2022-10-02 16:28 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7c3f810e976d03e516956492a16a8ae2948e3023.1662067442.git.aburgess@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).