From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id 180BB3858298; Sun, 2 Oct 2022 16:27:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 180BB3858298 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1664728076; bh=amPmEiLV032BIJ+GP/IXwX6wTzmy/dTd3Hnq+GZYE1M=; h=From:To:Subject:Date:From; b=adkC1nf+JMy1kWEL/4hscba/OMzc/esmotNB1ZzMX9juET/jubaOPpMX6mFmAxdf9 h1M1VW8N0n9F6lQ5uEirg3uv02ygvwFRisJjDCweV3LOqVoEo5zi3gsQotQ5nuwnRR PpOQ5HZvhezKBwaWVW7W15EVWnN2CQYQjRUNPKoo= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb: check for duplicate register names in selftest X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/master X-Git-Oldrev: 9a972b54040c6791a163b65a27d24d1acad7c397 X-Git-Newrev: 9a103324fee16bf4d0bc4fa7b19f5d937d47b881 Message-Id: <20221002162756.180BB3858298@sourceware.org> Date: Sun, 2 Oct 2022 16:27:56 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D9a103324fee1= 6bf4d0bc4fa7b19f5d937d47b881 commit 9a103324fee16bf4d0bc4fa7b19f5d937d47b881 Author: Andrew Burgess Date: Wed Aug 31 11:40:16 2022 +0100 gdb: check for duplicate register names in selftest =20 Building on the previous commit, this commit extends the register_name selftest to check for duplicate register names. =20 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. =20 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. =20 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. =20 I'm hopeful that this change shouldn't break anything, but I don't have the ability to actually test this change, however: =20 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, =20 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. =20 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. =20 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 . =20 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. Diff: --- 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/powerp= c-604.c index 09cbb50129f..16e6c12dae9 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"); =20 tdesc_powerpc_604 =3D result.release (); } diff --git a/gdb/features/rs6000/powerpc-604.xml b/gdb/features/rs6000/powe= rpc-604.xml index fbc9e946d22..052cfbb7e76 100644 --- a/gdb/features/rs6000/powerpc-604.xml +++ b/gdb/features/rs6000/powerpc-604.xml @@ -17,7 +17,6 @@ - diff --git a/gdb/features/rs6000/powerpc-750.c b/gdb/features/rs6000/powerp= c-750.c index 60a7b412070..3c9e7e1d23a 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/powe= rpc-750.xml index 908ecbdfd96..dfc5a87aaa0 100644 --- a/gdb/features/rs6000/powerpc-750.xml +++ b/gdb/features/rs6000/powerpc-750.xml @@ -17,7 +17,6 @@ - diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c index 68254412ed6..fa3f4d180cf 100644 --- a/gdb/gdbarch-selftests.c +++ b/gdb/gdbarch-selftests.c @@ -27,6 +27,8 @@ #include "gdbarch.h" #include "scoped-mock-context.h" =20 +#include + namespace selftests { =20 /* Test gdbarch methods register_to_value and value_to_register. */ @@ -129,6 +131,9 @@ register_name_test (struct gdbarch *gdbarch) { scoped_mock_context mockctx (gdbarch); =20 + /* Track the number of times each register name appears. */ + std::map name_counts; + const int num_regs =3D gdbarch_num_cooked_regs (gdbarch); for (auto regnum =3D 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 !=3D 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 !=3D '\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] =3D=3D 1); + } } }