* [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures)
@ 2022-09-01 21:31 Andrew Burgess
2022-09-01 21:31 ` [PATCH 1/9] gdb/testsuite: rewrite capture_command_output proc Andrew Burgess
` (9 more replies)
0 siblings, 10 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
This started with a single RISC-V test failure in
gdb.base/completion.exp.
It ended with me changing gdbarch_register_name for almost every
architecture.
The problem was the precise API for gdbarch_register_name isn't really
written down anywhere, and different bits of GDB seem to expect
slightly different things.
I used a new selftest to figure out what the most common API was, and
then brought all the other architecture into line with.
Unsurprisingly, this is basically to do what the more common / most
used / most tested architectures do (e.g. copy i386, arm, aarch64,
ppc, etc)
I've tested on x86-64, ppc, and sparc, but I've not tested most of the
architectures that I change here.
Thoughts?
Thanks,
Andrew
---
Andrew Burgess (9):
gdb/testsuite: rewrite capture_command_output proc
gdb/riscv: fix failure in gdb.base/completion.exp
gdb/gdbarch: add a comment to gdbarch_register_name
gdb: add a gdbarch_register_name self test, and fix some architectures
gdb: check for duplicate register names in selftest
gdb: add asserts to gdbarch_register_name
gdb/csky: remove nullptr return from csky_pseudo_register_name
gdb: final cleanup of various gdbarch_register_name methods
gdb: update now gdbarch_register_name doesn't return nullptr
gdb/alpha-tdep.c | 8 +--
gdb/arch-utils.c | 3 +-
gdb/avr-tdep.c | 6 +-
gdb/bpf-tdep.c | 5 +-
gdb/cris-tdep.c | 7 +-
gdb/csky-linux-tdep.c | 14 ++--
gdb/csky-tdep.c | 20 ++----
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/frv-tdep.c | 6 --
gdb/ft32-tdep.c | 5 +-
gdb/gdbarch-components.py | 23 +++++++
gdb/gdbarch-gen.h | 6 ++
gdb/gdbarch-selftests.c | 46 +++++++++++++
gdb/gdbarch.c | 6 +-
gdb/gdbarch.py | 19 ++++-
gdb/h8300-tdep.c | 11 +--
gdb/hppa-tdep.c | 12 ++--
gdb/infcmd.c | 3 +-
gdb/iq2000-tdep.c | 3 +-
gdb/lm32-tdep.c | 6 +-
gdb/m32c-tdep.c | 8 +--
gdb/m32r-tdep.c | 5 +-
gdb/m68hc11-tdep.c | 9 +--
gdb/m68k-tdep.c | 9 +--
gdb/mep-tdep.c | 2 +-
gdb/mi/mi-main.c | 20 ++----
gdb/microblaze-tdep.c | 6 +-
gdb/mips-tdep.c | 8 +--
gdb/mn10300-tdep.c | 14 ++--
gdb/moxie-tdep.c | 5 +-
gdb/msp430-tdep.c | 2 +
gdb/nds32-tdep.c | 10 +--
gdb/nios2-tdep.c | 2 +-
gdb/regcache.c | 5 +-
gdb/reggroups.c | 3 +-
gdb/riscv-tdep.c | 34 ++++-----
gdb/sh-tdep.c | 103 +++++++++-------------------
gdb/sparc-tdep.c | 8 +--
gdb/sparc64-tdep.c | 8 +--
gdb/testsuite/lib/gdb.exp | 35 +++++++++-
gdb/tic6x-tdep.c | 3 -
gdb/tilegx-tdep.c | 8 +--
gdb/tui/tui-regs.c | 4 +-
gdb/user-regs.c | 10 +--
gdb/v850-tdep.c | 14 ++--
gdb/valops.c | 2 +-
gdb/vax-tdep.c | 6 +-
gdb/xstormy16-tdep.c | 9 +--
gdb/xtensa-tdep.c | 6 +-
gdb/z80-tdep.c | 4 +-
53 files changed, 280 insertions(+), 307 deletions(-)
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/9] gdb/testsuite: rewrite capture_command_output proc
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
@ 2022-09-01 21:31 ` Andrew Burgess
2022-09-01 21:31 ` [PATCH 2/9] gdb/riscv: fix failure in gdb.base/completion.exp Andrew Burgess
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
I noticed a test failure in gdb.base/completion.exp for RISC-V on a
native Linux target. Upon investigation I discovered a couple of
reasons for the failure, this commit addresses one of them. A later
commit will address the other issue.
The completion.exp test makes use of the capture_command_output proc
to collect the output of the 'maint print registers' command. For
RISC-V this command produces a lot of output.
Currently the capture_command_output proc tries to collect the
complete command output in a single expect buffer, and what I see is
an error caused by the expect buffer becoming full.
This commit rewrites capture_command_output to make use of
gdb_test_multiple to collect the command output line at a time, in
this way we avoid overflowing the expect buffer.
The capture_command_output proc has some logic for skipping a prefix
pattern, which is passed in to the proc as an argument. In order to
handle this correctly (only matching the prefix at the start of the
command output), I use two gdb_test_multiple calls, the first spots
and discards the echoed command and the (optional) prefix pattern, the
second gdb_test_multiple call then collects the rest of the command
output line at a time until a prompt is seen.
There is one slight oddity with the current implementation, which I
have changed in my rewrite, this does, slightly, change the behaviour
of the proc.
The current implementation uses this pattern:
-re "[string_to_regexp ${command}]\[\r\n\]+${prefix}(.*)\[\r\n\]+$gdb_prompt $"
Now a typical command output will look like this:
output here\r\n
(gdb)
As the TCL regexp matching is greedy, TCL will try to match as much as
possible in one part of the pattern before moving on to the next.
Thus, when this matches against (.*)[\r\n]+, the (.*) will end up
matching against 'output here\r' and the [\r\n]+ will match '\n' only.
In short the previous implementation would leave the '\r' on the end
of the returned text, but not the final trailing '\n'.
Now clearly I could make a new version of capture_command_output that
maintained this behaviour, but I couldn't see any reason to do this.
So, my new implementation drops the final '\r\n' completely, in our
example above we now return 'output here' with no '\r'.
This change doesn't seem to affect any of the existing tests, but I
thought it was worth mentioning.
---
gdb/testsuite/lib/gdb.exp | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 17523f82996..65318f1cd55 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7862,12 +7862,41 @@ proc capture_command_output { command prefix } {
global gdb_prompt
global expect_out
+ set code {
+ -re "^[string_to_regexp ${command}]\r\n" {
+ if { $prefix != "" } {
+ exp_continue
+ }
+ }
+ }
+
+ if { $prefix != "" } {
+ append code {
+ -re "^${prefix}" {
+ # Nothing, we just move onto the next gdb_test_multiple
+ # call, which actually collects the command output.
+ }
+ }
+ }
+
+ gdb_test_multiple "$command" "capture_command_output for $command" $code
+
set output_string ""
- gdb_test_multiple "$command" "capture_command_output for $command" {
- -re "[string_to_regexp ${command}]\[\r\n\]+${prefix}(.*)\[\r\n\]+$gdb_prompt $" {
- set output_string $expect_out(1,string)
+ gdb_test_multiple "" "" {
+ -re "^(\[^\r\n\]+\r\n)" {
+ if { ![string equal $output_string ""] } {
+ set output_string [join [list $output_string $expect_out(1,string)] ""]
+ } else {
+ set output_string $expect_out(1,string)
+ }
+ exp_continue
+ }
+
+ -re "^$gdb_prompt $" {
}
}
+
+ set output_string [regsub "\r\n$" $output_string ""]
return $output_string
}
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/9] gdb/riscv: fix failure in gdb.base/completion.exp
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 ` Andrew Burgess
2022-09-01 21:31 ` [PATCH 3/9] gdb/gdbarch: add a comment to gdbarch_register_name Andrew Burgess
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
I noticed a test failure in gdb.base/completion.exp for RISC-V on
a native Linux target, this is the failure:
(gdb) FAIL: gdb.base/completion.exp: complete 'info registers '
The problem is caused by a mismatch in the output of 'maint print
registers' and the completion list for 'info registers'. The 'info
registers' completion list contains less registers than
expected. Additionally, the list of registers extracted from the
'maint print registers' list was wrong too, in some cases the test was
grabbing the register number, rather than a register name,
Both of these problems have the same root cause, riscv_register_name
returns nullptr for some registers when it should return an empty
string.
The gdbarch_register_name API is not clearly documented anywhere, and
at first glance it would appear that the function can return either
nullptr, or an empty string to indicate that a register is not
available on the current target. Indeed, there are plenty of places
in GDB where we compare the output of gdbarch_register_name to both
nullptr and '\0' in order to see if a register is supported or not,
and there are plenty of targets that return empty string in some
cases, and nullptr in others.
However, the 'info registers' completion code (reg_or_group_completer)
clearly depends on user_reg_map_regnum_to_name only returning nullptr
when the passed in regnum is greater than the maximum possible
register number (i.e. after all physical registers, pseudo-registers,
and user-registers), this means that gdbarch_register_name should not
be returning nullptr.
I did consider "fixing" user_reg_map_regnum_to_name, if
gdbarch_register_name returns nullptr, I could convert to an empty
string at this point, but that felt like a real hack, so I discarded
that plan.
The next possibility I considered was "fixing" reg_or_group_completer
to not rely on nullptr to indicate the end marker. Or rather, I could
have reg_or_group_completer use gdbarch_num_cooked_regs, we know that
we should check at least that many register numbers. Then, once we're
passed that limit, we keep checking until we hit a nullptr. This
would absolutely work, and didn't actually feel that bad, but, it
still felt a little weird that gdbarch_register_name could return
nullptr OR the empty string to mean the same thing, so I wondered if
the "right" solution was to have gdbarch_register_name not return
nullptr. With this in mind I tried an experiment:
I added a self-test that, for each architecture, calls
gdbarch_register_name for every register number up to the
gdbarch_num_cooked_regs limit, and checks that the name is not
nullptr.
Only a handful of architectures failed this test, RISC-V being one of
them.
This seems to suggest that most architectures agree that the correct
API for gdbarch_register_name is to return an empty string for
registers that are not supported on the current target, and that
returning nullptr is really a mistake.
In this commit I will update the RISC-V target so that GDB no longer
returns nullptr from riscv_register_name, instead we return the empty
string.
In subsequent commits I will add the selftest that I mention above,
and will fix the targets that fail the selftest.
With this change the gdb.base/completion.exp test now passes.
---
gdb/riscv-tdep.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 9ec430d8a10..088cf4ac55e 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -882,8 +882,9 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
then this is an unknown register. If we do get a name back then we
look up the registers preferred name below. */
const char *name = tdesc_register_name (gdbarch, regnum);
- if (name == NULL || name[0] == '\0')
- return NULL;
+ gdb_assert (name != nullptr);
+ if (name[0] == '\0')
+ return name;
/* We want GDB to use the ABI names for registers even if the target
gives us a target description with the architectural name. For
@@ -893,13 +894,13 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
return riscv_xreg_feature.register_name (regnum);
/* Like with the x-regs we prefer the abi names for the floating point
- registers. */
+ registers. If the target doesn't have floating point registers then
+ the tdesc_register_name call above should have returned an empty
+ string. */
if (regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
{
- if (riscv_has_fp_regs (gdbarch))
- return riscv_freg_feature.register_name (regnum);
- else
- return NULL;
+ gdb_assert (riscv_has_fp_regs (gdbarch));
+ return riscv_freg_feature.register_name (regnum);
}
/* Some targets (QEMU) are reporting these three registers twice, once
@@ -911,12 +912,10 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
duplicate copies of these registers (in riscv_tdesc_unknown_reg) and
then hide the registers here by giving them no name. */
riscv_gdbarch_tdep *tdep = gdbarch_tdep<riscv_gdbarch_tdep> (gdbarch);
- if (tdep->duplicate_fflags_regnum == regnum)
- return NULL;
- if (tdep->duplicate_frm_regnum == regnum)
- return NULL;
- if (tdep->duplicate_fcsr_regnum == regnum)
- return NULL;
+ if (tdep->duplicate_fflags_regnum == regnum
+ || tdep->duplicate_frm_regnum == regnum
+ || tdep->duplicate_fcsr_regnum == regnum)
+ return "";
/* The remaining registers are different. For all other registers on the
machine we prefer to see the names that the target description
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/9] gdb/gdbarch: add a comment to gdbarch_register_name
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 ` Andrew Burgess
2022-09-01 21:31 ` [PATCH 4/9] gdb: add a gdbarch_register_name self test, and fix some architectures Andrew Burgess
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
After the previous commit, this commit sets out to formalise the API
for gdbarch_register_name. Not every architecture is actually in
compliance with the API I set out here, but I believe that most are.
I think architectures that don't comply with the API laid out here
will fail the gdb.base/completion.exp test.
The claims in the comment are I feel, best demonstrated with the
asserts in this code:
const char *
gdbarch_register_name (struct gdbarch *gdbarch, int regnr)
{
gdb_assert (regnr >= 0);
gdb_assert (regnr < gdbarch_num_cooked_regs (gdbarch));
const char *name = gdbarch->register_name (gdbarch, regnr);
gdb_assert (name != nullptr);
return name;
}
Like I said, I don't believe every architecture follows these rules
right now, which is why I'm not actually adding any asserts. Instead,
this commit adds a comment to gdbarch_register_name, this comment is
where I'd like to get to, rather than where we are right now.
Subsequent commits will fix all targets to be in compliance with this
comment, and will even add the asserts shown above to
gdbarch_register_name.
---
gdb/gdbarch-components.py | 7 +++++++
gdb/gdbarch-gen.h | 6 ++++++
2 files changed, 13 insertions(+)
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index 71aa5991fbe..fcef090c2eb 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -558,6 +558,13 @@ Return -1 for bad REGNUM. Note: Several targets get this wrong.
)
Method(
+ comment="""
+Return the name of register REGNR for the specified architecture.
+REGNR can be any value greater than, or equal to zero, and less than
+'gdbarch_num_cooked_regs (GDBARCH)'. If REGNR is not supported for
+GDBARCH, then this function will return an empty string, this function
+should never return nullptr.
+""",
type="const char *",
name="register_name",
params=[("int", "regnr")],
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 0504962e50d..a3df62536c6 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -288,6 +288,12 @@ typedef int (gdbarch_dwarf2_reg_to_regnum_ftype) (struct gdbarch *gdbarch, int d
extern int gdbarch_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int dwarf2_regnr);
extern void set_gdbarch_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, gdbarch_dwarf2_reg_to_regnum_ftype *dwarf2_reg_to_regnum);
+/* Return the name of register REGNR for the specified architecture.
+ REGNR can be any value greater than, or equal to zero, and less than
+ 'gdbarch_num_cooked_regs (GDBARCH)'. If REGNR is not supported for
+ GDBARCH, then this function will return an empty string, this function
+ should never return nullptr. */
+
typedef const char * (gdbarch_register_name_ftype) (struct gdbarch *gdbarch, int regnr);
extern const char * gdbarch_register_name (struct gdbarch *gdbarch, int regnr);
extern void set_gdbarch_register_name (struct gdbarch *gdbarch, gdbarch_register_name_ftype *register_name);
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/9] gdb: add a gdbarch_register_name self test, and fix some architectures
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (2 preceding siblings ...)
2022-09-01 21:31 ` [PATCH 3/9] gdb/gdbarch: add a comment to gdbarch_register_name Andrew Burgess
@ 2022-09-01 21:31 ` Andrew Burgess
2022-09-01 21:31 ` [PATCH 5/9] gdb: check for duplicate register names in selftest Andrew Burgess
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
This commit adds a self-test that checks that gdbarch_register_name
never returns nullptr for any valid register number.
Most architectures already met this requirement, there were just 6
that failed the new selftest, and are updated in this commit.
Beyond the self-tests I don't have any facilities to test that the
architectures I've adjusted still work correctly.
If you review all the various gdbarch_register_name implementations
then you will see that there are far more architectures that seem like
they might return nullptr in some situations, e.g. alpha, avr, bpf,
etc. This commit doesn't attempt to address these cases as non of
them are hit during the selftest. Many of these cases can never be
hit, for example, in alpha_register_name GDB checks for a register
number less than zero, this case can't happen and could be changed
into an assert.
A later commit in this series will have a general cleanup of all the
various register_name methods, and remove all references to NULL from
their code, however, as that commit will be mostly adjusting code that
is never hit, I want to keep those changes separate.
The selftest has been tested on x86-64, but I don't have access to
suitable systems to fully test any of the *-tdep.c code I've changed
in this commit.
---
gdb/cris-tdep.c | 4 +-
gdb/gdbarch-selftests.c | 27 +++++++++++++
gdb/m32c-tdep.c | 8 ++--
gdb/m68hc11-tdep.c | 2 +-
gdb/mep-tdep.c | 2 +-
gdb/sh-tdep.c | 89 ++++++++++++++---------------------------
gdb/v850-tdep.c | 8 ++--
7 files changed, 69 insertions(+), 71 deletions(-)
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index 73110d97f2f..a25490a3fa1 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -1654,7 +1654,7 @@ cris_special_register_name (struct gdbarch *gdbarch, int regno)
return cris_spec_regs[i].name;
}
/* Special register not applicable to this CRIS version. */
- return NULL;
+ return "";
}
static const char *
@@ -1678,7 +1678,7 @@ cris_register_name (struct gdbarch *gdbarch, int regno)
else
{
/* Invalid register. */
- return NULL;
+ return "";
}
}
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 198346bddec..75a3109143f 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -122,6 +122,30 @@ register_to_value_test (struct gdbarch *gdbarch)
}
}
+/* Test function gdbarch_register_name. */
+
+static void
+register_name_test (struct gdbarch *gdbarch)
+{
+ scoped_mock_context<test_target_ops> mockctx (gdbarch);
+
+ const int num_regs = gdbarch_num_cooked_regs (gdbarch);
+ for (auto regnum = 0; regnum < num_regs; regnum++)
+ {
+ /* If a register is to be hidden from the user then we should get
+ back an empty string, not nullptr. Every other register should
+ return a non-empty string. */
+ const char *name = gdbarch_register_name (gdbarch, regnum);
+
+ if (run_verbose() && name == nullptr)
+ debug_printf ("arch: %s, register: %d returned nullptr\n",
+ gdbarch_bfd_arch_info (gdbarch)->printable_name,
+ regnum);
+
+ SELF_CHECK (name != nullptr);
+ }
+}
+
} // namespace selftests
void _initialize_gdbarch_selftests ();
@@ -130,4 +154,7 @@ _initialize_gdbarch_selftests ()
{
selftests::register_test_foreach_arch ("register_to_value",
selftests::register_to_value_test);
+
+ selftests::register_test_foreach_arch ("register_name",
+ selftests::register_name_test);
}
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 83b7432f491..20460535ea9 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -757,14 +757,14 @@ mark_save_restore (struct m32c_reg *reg)
/* A raw banked general-purpose data register named NAME.
NAME should be an identifier, not a string. */
#define RBD(name) \
- (R(NULL, tdep->int16, SIM (name ## _bank0)), \
- R(NULL, tdep->int16, SIM (name ## _bank1)) - 1)
+ (R("", tdep->int16, SIM (name ## _bank0)), \
+ R("", tdep->int16, SIM (name ## _bank1)) - 1)
/* A raw banked data address register named NAME.
NAME should be an identifier, not a string. */
#define RBA(name) \
- (R(NULL, tdep->data_addr_reg_type, SIM (name ## _bank0)), \
- R(NULL, tdep->data_addr_reg_type, SIM (name ## _bank1)) - 1)
+ (R("", tdep->data_addr_reg_type, SIM (name ## _bank0)), \
+ R("", tdep->data_addr_reg_type, SIM (name ## _bank1)) - 1)
/* A cooked register named NAME referring to a raw banked register
from the bank selected by the current value of FLG. RAW_PAIR
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 53b7752f073..f860ff81f2e 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -400,7 +400,7 @@ m68hc11_register_name (struct gdbarch *gdbarch, int reg_nr)
/* If we don't know the address of a soft register, pretend it
does not exist. */
if (reg_nr > M68HC11_LAST_HARD_REG && soft_regs[reg_nr].name == 0)
- return NULL;
+ return "";
return m68hc11_register_names[reg_nr];
}
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index 66b7656b3d5..e3655de6a73 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -1016,7 +1016,7 @@ mep_register_name (struct gdbarch *gdbarch, int regnr)
would affect the output of 'info all-registers', which would
disturb the test suites. So we leave it invisible. */
else
- return NULL;
+ return "";
}
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index 062df860c81..64b7ffa43da 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -112,19 +112,12 @@ sh_sh_register_name (struct gdbarch *gdbarch, int reg_nr)
static const char *register_names[] = {
"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
- "pc", "pr", "gbr", "vbr", "mach", "macl", "sr",
- "", "",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
- "", "",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
+ "pc", "pr", "gbr", "vbr", "mach", "macl", "sr"
};
- if (reg_nr < 0)
- return NULL;
+
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -140,13 +133,12 @@ sh_sh3_register_name (struct gdbarch *gdbarch, int reg_nr)
"", "", "", "", "", "", "", "",
"ssr", "spc",
"r0b0", "r1b0", "r2b0", "r3b0", "r4b0", "r5b0", "r6b0", "r7b0",
- "r0b1", "r1b1", "r2b1", "r3b1", "r4b1", "r5b1", "r6b1", "r7b1"
- "", "", "", "", "", "", "", "",
+ "r0b1", "r1b1", "r2b1", "r3b1", "r4b1", "r5b1", "r6b1", "r7b1",
};
- if (reg_nr < 0)
- return NULL;
+
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -163,12 +155,10 @@ sh_sh3e_register_name (struct gdbarch *gdbarch, int reg_nr)
"ssr", "spc",
"r0b0", "r1b0", "r2b0", "r3b0", "r4b0", "r5b0", "r6b0", "r7b0",
"r0b1", "r1b1", "r2b1", "r3b1", "r4b1", "r5b1", "r6b1", "r7b1",
- "", "", "", "", "", "", "", "",
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -182,15 +172,10 @@ sh_sh2e_register_name (struct gdbarch *gdbarch, int reg_nr)
"fpul", "fpscr",
"fr0", "fr1", "fr2", "fr3", "fr4", "fr5", "fr6", "fr7",
"fr8", "fr9", "fr10", "fr11", "fr12", "fr13", "fr14", "fr15",
- "", "",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -227,10 +212,9 @@ sh_sh2a_register_name (struct gdbarch *gdbarch, int reg_nr)
/* double precision (pseudo) 68 - 75 */
"dr0", "dr2", "dr4", "dr6", "dr8", "dr10", "dr12", "dr14",
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -264,13 +248,11 @@ sh_sh2a_nofpu_register_name (struct gdbarch *gdbarch, int reg_nr)
"ibcr", "ibnr", "tbr",
/* 67: register bank number, the user visible pseudo register. */
"bank",
- /* double precision (pseudo) 68 - 75 */
- "", "", "", "", "", "", "", "",
+ /* double precision (pseudo) 68 - 75: report blank, see below. */
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -285,14 +267,11 @@ sh_sh_dsp_register_name (struct gdbarch *gdbarch, int reg_nr)
"a0g", "a0", "a1g", "a1", "m0", "m1", "x0", "x1",
"y0", "y1", "", "", "", "", "", "mod",
"", "",
- "rs", "re", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
+ "rs", "re",
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -309,13 +288,10 @@ sh_sh3_dsp_register_name (struct gdbarch *gdbarch, int reg_nr)
"ssr", "spc",
"rs", "re", "", "", "", "", "", "",
"r0b", "r1b", "r2b", "r3b", "r4b", "r5b", "r6b", "r7b",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -350,10 +326,9 @@ sh_sh4_register_name (struct gdbarch *gdbarch, int reg_nr)
/* FIXME: missing XF */
/* FIXME: missing XD */
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -383,13 +358,12 @@ sh_sh4_nofpu_register_name (struct gdbarch *gdbarch, int reg_nr)
"",
/* double precision (pseudo) 68 - 75 -- not for nofpu target */
"", "", "", "", "", "", "", "",
- /* vectors (pseudo) 76 - 79 -- not for nofpu target */
- "", "", "", "",
+ /* vectors (pseudo) 76 - 79 -- not for nofpu target: report blank
+ below. */
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
@@ -406,13 +380,10 @@ sh_sh4al_dsp_register_name (struct gdbarch *gdbarch, int reg_nr)
"ssr", "spc",
"rs", "re", "", "", "", "", "", "",
"r0b", "r1b", "r2b", "r3b", "r4b", "r5b", "r6b", "r7b",
- "", "", "", "", "", "", "", "",
- "", "", "", "", "", "", "", "",
};
- if (reg_nr < 0)
- return NULL;
+ gdb_assert (reg_nr >= 0);
if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ return "";
return register_names[reg_nr];
}
diff --git a/gdb/v850-tdep.c b/gdb/v850-tdep.c
index 232b92d0a08..a067bf74f16 100644
--- a/gdb/v850-tdep.c
+++ b/gdb/v850-tdep.c
@@ -313,7 +313,7 @@ v850_register_name (struct gdbarch *gdbarch, int regnum)
"pc", "fp"
};
if (regnum < 0 || regnum > E_NUM_OF_V850_REGS)
- return NULL;
+ return "";
return v850_reg_names[regnum];
}
@@ -333,7 +333,7 @@ v850e_register_name (struct gdbarch *gdbarch, int regnum)
"pc", "fp"
};
if (regnum < 0 || regnum > E_NUM_OF_V850E_REGS)
- return NULL;
+ return "";
return v850e_reg_names[regnum];
}
@@ -377,7 +377,7 @@ v850e2_register_name (struct gdbarch *gdbarch, int regnum)
"", "", "", "fpspc"
};
if (regnum < 0 || regnum >= E_NUM_OF_V850E2_REGS)
- return NULL;
+ return "";
return v850e2_reg_names[regnum];
}
@@ -480,7 +480,7 @@ v850e3v5_register_name (struct gdbarch *gdbarch, int regnum)
};
if (regnum < 0 || regnum >= E_NUM_OF_V850E3V5_REGS)
- return NULL;
+ return "";
return v850e3v5_reg_names[regnum];
}
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/9] gdb: check for duplicate register names in selftest
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (3 preceding siblings ...)
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
2022-09-01 21:31 ` [PATCH 6/9] gdb: add asserts to gdbarch_register_name Andrew Burgess
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/9] gdb: add asserts to gdbarch_register_name
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (4 preceding siblings ...)
2022-09-01 21:31 ` [PATCH 5/9] gdb: check for duplicate register names in selftest Andrew Burgess
@ 2022-09-01 21:31 ` 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
` (3 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
This commit adds asserts to gdbarch_register_name that validate the
parameters, and the return value.
The interesting thing here is that gdbarch_register_name is generated
by gdbarch.py, and so, to add these asserts, I need to update the
generation script.
I've added two new arguments for Functions and Methods (as declared in
gdbarch-components.py), these arguments are 'param_checks' and
'result_checks'. Each of these new arguments can be used to list some
expressions that are then used within gdb_assert calls in the
generated code.
The asserts that validate the API as described in the comment I added
to gdbarch_register_name a few commits back; the register number
passed in needs to be a valid cooked register number, and the result
being returned should not be nullptr.
---
gdb/gdbarch-components.py | 16 ++++++++++++++++
gdb/gdbarch.c | 6 +++++-
gdb/gdbarch.py | 19 ++++++++++++++++++-
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index fcef090c2eb..b8dc1dd36f5 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -99,6 +99,20 @@
# argument, and NAME is the name. Note that while the names could be
# auto-generated, this approach lets the "comment" field refer to
# arguments in a nicer way. It is also just nicer for users.
+#
+# * "param_checks" - optional, a list of strings. Each string is an
+# expression that is placed within a gdb_assert before the call is
+# made to the Function/Method implementation. Each expression is
+# something that should be true, and it is expected that the
+# expression will make use of the parameters named in 'params' (though
+# this is not required).
+#
+# * "result_checks" - optional, a list of strings. Each string is an
+# expression that is placed within a gdb_assert after the call to the
+# Function/Method implementation. Within each expression the variable
+# 'result' can be used to reference the result of the function/method
+# implementation. The 'result_checks' can only be used if the 'type'
+# of this Function/Method is not 'void'.
Info(
type="const struct bfd_arch_info *",
@@ -568,6 +582,8 @@ should never return nullptr.
type="const char *",
name="register_name",
params=[("int", "regnr")],
+ param_checks=["regnr >= 0", "regnr < gdbarch_num_cooked_regs (gdbarch)"],
+ result_checks=["result != nullptr"],
predefault="0",
invalid=True,
)
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 0edae7f6f0a..81e26362954 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2229,9 +2229,13 @@ gdbarch_register_name (struct gdbarch *gdbarch, int regnr)
{
gdb_assert (gdbarch != NULL);
gdb_assert (gdbarch->register_name != NULL);
+ gdb_assert (regnr >= 0);
+ gdb_assert (regnr < gdbarch_num_cooked_regs (gdbarch));
if (gdbarch_debug >= 2)
gdb_printf (gdb_stdlog, "gdbarch_register_name called\n");
- return gdbarch->register_name (gdbarch, regnr);
+ auto result = gdbarch->register_name (gdbarch, regnr);
+ gdb_assert (result != nullptr);
+ return result;
}
void
diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index 696b3028e6f..699ed4f69d2 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -112,6 +112,8 @@ class Function(_Component):
postdefault=None,
invalid=None,
printer=None,
+ param_checks=None,
+ result_checks=None,
):
super().__init__(
comment=comment,
@@ -124,6 +126,11 @@ class Function(_Component):
printer=printer,
params=params,
)
+ self.param_checks = param_checks
+ self.result_checks = result_checks
+
+ if self.type == "void" and self.result_checks:
+ raise Exception("can't have result checks with a void return type")
def ftype(self):
"Return the name of the function typedef to use."
@@ -444,6 +451,9 @@ with open("gdbarch.c", "w") as f:
f" /* Do not check predicate: {c.get_predicate()}, allow call. */",
file=f,
)
+ if c.param_checks:
+ for rule in c.param_checks:
+ print(f" gdb_assert ({rule});", file=f)
print(" if (gdbarch_debug >= 2)", file=f)
print(
f""" gdb_printf (gdb_stdlog, "gdbarch_{c.name} called\\n");""",
@@ -451,8 +461,15 @@ with open("gdbarch.c", "w") as f:
)
print(" ", file=f, end="")
if c.type != "void":
- print("return ", file=f, end="")
+ if c.result_checks:
+ print("auto result = ", file=f, end="")
+ else:
+ print("return ", file=f, end="")
print(f"gdbarch->{c.name} ({c.actuals()});", file=f)
+ if c.type != "void" and c.result_checks:
+ for rule in c.result_checks:
+ print(f" gdb_assert ({rule});", file=f)
+ print(" return result;", file=f)
print("}", file=f)
print(file=f)
print("void", file=f)
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 7/9] gdb/csky: remove nullptr return from csky_pseudo_register_name
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (5 preceding siblings ...)
2022-09-01 21:31 ` [PATCH 6/9] gdb: add asserts to gdbarch_register_name Andrew Burgess
@ 2022-09-01 21:31 ` Andrew Burgess
2022-09-01 21:31 ` [PATCH 8/9] gdb: final cleanup of various gdbarch_register_name methods Andrew Burgess
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
Building on the previous commits, in this commit I remove two
instances of 'return NULL' from csky_pseudo_register_name, and replace
them with a return of the empty string.
These two are particularly interesting, and worth pulling into their
own commit, because these returns of NULL appear to be depended on
within other parts of the csky code.
In csky-linux-tdep.c in the register collect/supply code, GDB checks
for the register name being nullptr in order to decide if a target
supports a particular feature or not. I've updated the code to check
for the empty string.
I have no way of testing this change.
---
gdb/csky-linux-tdep.c | 14 +++++++-------
gdb/csky-tdep.c | 17 ++++-------------
2 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/gdb/csky-linux-tdep.c b/gdb/csky-linux-tdep.c
index 7bcc1712180..ea306cded09 100644
--- a/gdb/csky-linux-tdep.c
+++ b/gdb/csky-linux-tdep.c
@@ -155,7 +155,7 @@ csky_supply_fregset (const struct regset *regset,
/* Supply vr0~vr15. */
for (i = 0; i < 16; i ++)
{
- if (gdbarch_register_name (gdbarch, (CSKY_VR0_REGNUM + i)))
+ if (*gdbarch_register_name (gdbarch, (CSKY_VR0_REGNUM + i)) != '\0')
{
offset = 16 * i;
regcache->raw_supply (CSKY_VR0_REGNUM + i, fregs + offset);
@@ -164,7 +164,7 @@ csky_supply_fregset (const struct regset *regset,
/* Supply fr0~fr15. */
for (i = 0; i < 16; i ++)
{
- if (gdbarch_register_name (gdbarch, (CSKY_FR0_REGNUM + i)))
+ if (*gdbarch_register_name (gdbarch, (CSKY_FR0_REGNUM + i)) != '\0')
{
offset = 16 * i;
regcache->raw_supply (CSKY_FR0_REGNUM + i, fregs + offset);
@@ -173,7 +173,7 @@ csky_supply_fregset (const struct regset *regset,
/* Supply fr16~fr31. */
for (i = 0; i < 16; i ++)
{
- if (gdbarch_register_name (gdbarch, (CSKY_FR16_REGNUM + i)))
+ if (*gdbarch_register_name (gdbarch, (CSKY_FR16_REGNUM + i)) != '\0')
{
offset = (16 * 16) + (8 * i);
regcache->raw_supply (CSKY_FR16_REGNUM + i, fregs + offset);
@@ -182,7 +182,7 @@ csky_supply_fregset (const struct regset *regset,
/* Supply fcr, fesr, fid. */
for (i = 0; i < 3; i ++)
{
- if (gdbarch_register_name (gdbarch, fcr_regno[i]))
+ if (*gdbarch_register_name (gdbarch, fcr_regno[i]) != '\0')
{
offset = (16 * 16) + (16 * 8) + (4 * i);
regcache->raw_supply (fcr_regno[i], fregs + offset);
@@ -245,7 +245,7 @@ csky_collect_fregset (const struct regset *regset,
/* Supply vr0~vr15. */
for (i = 0; i < 16; i ++)
{
- if (gdbarch_register_name (gdbarch, (CSKY_VR0_REGNUM + i)))
+ if (*gdbarch_register_name (gdbarch, (CSKY_VR0_REGNUM + i)) != '\0')
{
offset = 16 * i;
regcache ->raw_collect (CSKY_VR0_REGNUM + i, fregs + offset);
@@ -254,7 +254,7 @@ csky_collect_fregset (const struct regset *regset,
/* Supply fr16~fr31. */
for (i = 0; i < 16; i ++)
{
- if (gdbarch_register_name (gdbarch, (CSKY_FR16_REGNUM + i)))
+ if (*gdbarch_register_name (gdbarch, (CSKY_FR16_REGNUM + i)) != '\0')
{
offset = (16 * 16) + (8 * i);
regcache ->raw_collect (CSKY_FR16_REGNUM + i, fregs + offset);
@@ -263,7 +263,7 @@ csky_collect_fregset (const struct regset *regset,
/* Supply fcr, fesr, fid. */
for (i = 0; i < 3; i ++)
{
- if (gdbarch_register_name (gdbarch, fcr_regno[i]))
+ if (*gdbarch_register_name (gdbarch, fcr_regno[i]) != '\0')
{
offset = (16 * 16) + (16 * 8) + (4 * i);
regcache ->raw_collect (fcr_regno[i], fregs + offset);
diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index ba53c1b10ca..51336148b6d 100644
--- a/gdb/csky-tdep.c
+++ b/gdb/csky-tdep.c
@@ -660,21 +660,12 @@ static const char * const csky_register_names[] =
static const char *
csky_register_name (struct gdbarch *gdbarch, int reg_nr)
{
- int num_regs = gdbarch_num_regs (gdbarch);
- int num_pseudo_regs = gdbarch_num_pseudo_regs (gdbarch);
-
- if ((reg_nr >= num_regs) && (reg_nr < (num_regs + num_pseudo_regs)))
+ if (reg_nr >= gdbarch_num_regs (gdbarch))
return csky_pseudo_register_name (gdbarch, reg_nr);
if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
return tdesc_register_name (gdbarch, reg_nr);
- if (reg_nr < 0)
- return NULL;
-
- if (reg_nr >= gdbarch_num_regs (gdbarch))
- return NULL;
-
return csky_register_names[reg_nr];
}
@@ -2712,15 +2703,15 @@ csky_pseudo_register_name (struct gdbarch *gdbarch, int regno)
if (regno < tdep->fv_pseudo_registers_count)
{
if ((regno < 64) && ((regno % 4) >= 2) && !tdep->has_vr0)
- return NULL;
+ return "";
else if ((regno >= 64) && ((regno % 4) >= 2))
- return NULL;
+ return "";
else
return fv_pseudo_names[regno];
}
}
- return NULL;
+ return "";
}
/* Read for csky pseudo regs. */
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 8/9] gdb: final cleanup of various gdbarch_register_name methods
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (6 preceding siblings ...)
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 ` 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
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
Building on the previous commits, this commit goes through the various
gdbarch_register_name methods and removes all the remaining 'return
NULL' cases, I claim that these either couldn't be hit, or should be
returning the empty string.
In all cases the return of NULL was used if the register number being
passed to gdbarch_register_name was "invalid", i.e. negative, or
greater than the total number of declared registers. I don't believe
either of these cases can occur, and the previous commit asserts that
this is the case. As a result we can simplify the code by removing
these checks. In many cases, where the register names are held in an
array, I was able to add a static assert that the array contains the
correct number of strings, after that, a direct access into the array
is fine.
I don't have any means of testing these changes.
---
gdb/alpha-tdep.c | 5 +----
gdb/avr-tdep.c | 6 ++----
gdb/bpf-tdep.c | 5 ++---
gdb/cris-tdep.c | 3 ++-
gdb/frv-tdep.c | 6 ------
gdb/ft32-tdep.c | 5 +----
gdb/h8300-tdep.c | 9 ++-------
gdb/hppa-tdep.c | 12 ++++--------
gdb/iq2000-tdep.c | 3 +--
gdb/lm32-tdep.c | 6 ++----
gdb/m32r-tdep.c | 5 +----
gdb/m68hc11-tdep.c | 5 +----
gdb/m68k-tdep.c | 9 +++------
gdb/microblaze-tdep.c | 6 +++---
gdb/mn10300-tdep.c | 14 ++++++--------
gdb/moxie-tdep.c | 5 +----
gdb/msp430-tdep.c | 2 ++
gdb/nds32-tdep.c | 7 ++-----
gdb/nios2-tdep.c | 2 +-
gdb/sh-tdep.c | 33 +++++++++++----------------------
gdb/sparc-tdep.c | 8 ++------
gdb/sparc64-tdep.c | 8 ++------
gdb/tic6x-tdep.c | 3 ---
gdb/tilegx-tdep.c | 8 ++------
gdb/v850-tdep.c | 12 +++++-------
gdb/vax-tdep.c | 6 ++----
gdb/xstormy16-tdep.c | 9 ++-------
gdb/xtensa-tdep.c | 6 +-----
gdb/z80-tdep.c | 4 ++--
29 files changed, 66 insertions(+), 146 deletions(-)
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 44efc8e4bbb..8a5697939ec 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -99,10 +99,7 @@ alpha_register_name (struct gdbarch *gdbarch, int regno)
"pc", "", "unique"
};
- if (regno < 0)
- return NULL;
- if (regno >= ARRAY_SIZE(register_names))
- return NULL;
+ gdb_static_assert (ALPHA_NUM_REGS == ARRAY_SIZE (register_names));
return register_names[regno];
}
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 693d563b911..f57dfac5e64 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -216,10 +216,8 @@ avr_register_name (struct gdbarch *gdbarch, int regnum)
"SREG", "SP", "PC2",
"pc"
};
- if (regnum < 0)
- return NULL;
- if (regnum >= (sizeof (register_names) / sizeof (*register_names)))
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (register_names)
+ == (AVR_NUM_REGS + AVR_NUM_PSEUDO_REGS));
return register_names[regnum];
}
diff --git a/gdb/bpf-tdep.c b/gdb/bpf-tdep.c
index 4c085093632..b52fa5d5efb 100644
--- a/gdb/bpf-tdep.c
+++ b/gdb/bpf-tdep.c
@@ -93,9 +93,8 @@ static const char *bpf_register_names[] =
static const char *
bpf_register_name (struct gdbarch *gdbarch, int reg)
{
- if (reg >= 0 && reg < BPF_NUM_REGS)
- return bpf_register_names[reg];
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (bpf_register_names) == BPF_NUM_REGS);
+ return bpf_register_names[reg];
}
/* Return the GDB type of register REGNUM. */
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index a25490a3fa1..d1d85a557fb 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -1666,9 +1666,10 @@ cris_register_name (struct gdbarch *gdbarch, int regno)
"r8", "r9", "r10", "r11", \
"r12", "r13", "sp", "pc" };
- if (regno >= 0 && regno < NUM_GENREGS)
+ if (regno < NUM_GENREGS)
{
/* General register. */
+ gdb_static_assert (ARRAY_SIZE (cris_genreg_names) == NUM_GENREGS);
return cris_genreg_names[regno];
}
else if (regno >= NUM_GENREGS && regno < gdbarch_num_regs (gdbarch))
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index 8029080c043..866d5883876 100644
--- a/gdb/frv-tdep.c
+++ b/gdb/frv-tdep.c
@@ -269,12 +269,6 @@ set_variant_scratch_registers (frv_gdbarch_tdep *var)
static const char *
frv_register_name (struct gdbarch *gdbarch, int reg)
{
- if (reg < 0)
- return "?toosmall?";
-
- if (reg >= frv_num_regs + frv_num_pseudo_regs)
- return "?toolarge?";
-
frv_gdbarch_tdep *tdep = gdbarch_tdep<frv_gdbarch_tdep> (gdbarch);
return tdep->register_names[reg];
}
diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 500c691aa34..251baaf28ff 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -96,10 +96,7 @@ static const char *const ft32_register_names[] =
static const char *
ft32_register_name (struct gdbarch *gdbarch, int reg_nr)
{
- if (reg_nr < 0)
- return NULL;
- if (reg_nr >= FT32_NUM_REGS)
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (ft32_register_names) == FT32_NUM_REGS);
return ft32_register_names[reg_nr];
}
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index d453fce3319..0d1ab463ef3 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -934,13 +934,8 @@ static const char *
h8300_register_name_common (const char *regnames[], int numregs,
struct gdbarch *gdbarch, int regno)
{
- if (regno < 0
- || regno >= numregs)
- internal_error (__FILE__, __LINE__,
- _("h8300_register_name_common: illegal register number %d"),
- regno);
- else
- return regnames[regno];
+ gdb_assert (numregs == gdbarch_num_cooked_regs (gdbarch));
+ return regnames[regno];
}
static const char *
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 91f9cecdcbc..7d5871c0e0f 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -632,10 +632,8 @@ hppa32_register_name (struct gdbarch *gdbarch, int i)
"fr28", "fr28R", "fr29", "fr29R",
"fr30", "fr30R", "fr31", "fr31R"
};
- if (i < 0 || i >= (sizeof (names) / sizeof (*names)))
- return NULL;
- else
- return names[i];
+ gdb_static_assert (ARRAY_SIZE (names) == hppa32_num_regs);
+ return names[i];
}
static const char *
@@ -667,10 +665,8 @@ hppa64_register_name (struct gdbarch *gdbarch, int i)
"fr24", "fr25", "fr26", "fr27",
"fr28", "fr29", "fr30", "fr31"
};
- if (i < 0 || i >= (sizeof (names) / sizeof (*names)))
- return NULL;
- else
- return names[i];
+ gdb_static_assert (ARRAY_SIZE (names) == hppa64_num_regs);
+ return names[i];
}
/* Map dwarf DBX register numbers to GDB register numbers. */
diff --git a/gdb/iq2000-tdep.c b/gdb/iq2000-tdep.c
index e5fe1c62466..57498e879a1 100644
--- a/gdb/iq2000-tdep.c
+++ b/gdb/iq2000-tdep.c
@@ -135,8 +135,7 @@ iq2000_register_name (struct gdbarch *gdbarch, int regnum)
"r30", "r31",
"pc"
};
- if (regnum < 0 || regnum >= E_NUM_REGS)
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (names) == E_NUM_REGS);
return names[regnum];
}
diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
index af322511cfa..0394e1503ed 100644
--- a/gdb/lm32-tdep.c
+++ b/gdb/lm32-tdep.c
@@ -87,10 +87,8 @@ lm32_register_name (struct gdbarch *gdbarch, int reg_nr)
"PC", "EID", "EBA", "DEBA", "IE", "IM", "IP"
};
- if ((reg_nr < 0) || (reg_nr >= ARRAY_SIZE (register_names)))
- return NULL;
- else
- return register_names[reg_nr];
+ gdb_static_assert (ARRAY_SIZE (register_names) == SIM_LM32_NUM_REGS);
+ return register_names[reg_nr];
}
/* Return type of register. */
diff --git a/gdb/m32r-tdep.c b/gdb/m32r-tdep.c
index 7edbd500bc5..297e5bf47b8 100644
--- a/gdb/m32r-tdep.c
+++ b/gdb/m32r-tdep.c
@@ -212,10 +212,7 @@ static const char * const m32r_register_names[] = {
static const char *
m32r_register_name (struct gdbarch *gdbarch, int reg_nr)
{
- if (reg_nr < 0)
- return NULL;
- if (reg_nr >= M32R_NUM_REGS)
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (m32r_register_names) == M32R_NUM_REGS);
return m32r_register_names[reg_nr];
}
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index f860ff81f2e..6277572adee 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -388,12 +388,9 @@ m68hc11_register_name (struct gdbarch *gdbarch, int reg_nr)
if (reg_nr == HARD_PC_REGNUM && use_page_register (gdbarch))
return "ppc";
-
- if (reg_nr < 0)
- return NULL;
if (reg_nr >= M68HC11_ALL_REGS)
- return NULL;
+ return "";
m68hc11_initialize_register_info ();
diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index 9e59f5904c3..855fa1dd9a9 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -173,12 +173,9 @@ m68k_register_name (struct gdbarch *gdbarch, int regnum)
{
m68k_gdbarch_tdep *tdep = gdbarch_tdep<m68k_gdbarch_tdep> (gdbarch);
- if (regnum < 0 || regnum >= ARRAY_SIZE (m68k_register_names))
- internal_error (__FILE__, __LINE__,
- _("m68k_register_name: illegal register number %d"),
- regnum);
- else if (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FPI_REGNUM
- && tdep->fpregs_present == 0)
+ gdb_static_assert (ARRAY_SIZE (m68k_register_names) == M68K_NUM_REGS);
+ if (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FPI_REGNUM
+ && tdep->fpregs_present == 0)
return "";
else
return m68k_register_names[regnum];
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 3525453deeb..55258ca32ae 100644
--- a/gdb/microblaze-tdep.c
+++ b/gdb/microblaze-tdep.c
@@ -92,9 +92,9 @@ static unsigned int microblaze_debug_flag = 0;
static const char *
microblaze_register_name (struct gdbarch *gdbarch, int regnum)
{
- if (regnum >= 0 && regnum < MICROBLAZE_NUM_REGS)
- return microblaze_register_names[regnum];
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (microblaze_register_names)
+ == MICROBLAZE_NUM_REGS);
+ return microblaze_register_names[regnum];
}
static struct type *
diff --git a/gdb/mn10300-tdep.c b/gdb/mn10300-tdep.c
index 22511d894d4..974d07dd998 100644
--- a/gdb/mn10300-tdep.c
+++ b/gdb/mn10300-tdep.c
@@ -250,12 +250,10 @@ mn10300_return_value (struct gdbarch *gdbarch, struct value *function,
}
static const char *
-register_name (int reg, const char **regs, long sizeof_regs)
+register_name (int reg, const char **regs, long num_regs)
{
- if (reg < 0 || reg >= sizeof_regs / sizeof (regs[0]))
- return NULL;
- else
- return regs[reg];
+ gdb_assert (reg < num_regs);
+ return regs[reg];
}
static const char *
@@ -267,7 +265,7 @@ mn10300_generic_register_name (struct gdbarch *gdbarch, int reg)
"", "", "", "", "", "", "", "",
"", "", "", "", "", "", "", "fp"
};
- return register_name (reg, regs, sizeof regs);
+ return register_name (reg, regs, ARRAY_SIZE (regs));
}
@@ -280,7 +278,7 @@ am33_register_name (struct gdbarch *gdbarch, int reg)
"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
"ssp", "msp", "usp", "mcrh", "mcrl", "mcvf", "", "", ""
};
- return register_name (reg, regs, sizeof regs);
+ return register_name (reg, regs, ARRAY_SIZE (regs));
}
static const char *
@@ -297,7 +295,7 @@ am33_2_register_name (struct gdbarch *gdbarch, int reg)
"fs16", "fs17", "fs18", "fs19", "fs20", "fs21", "fs22", "fs23",
"fs24", "fs25", "fs26", "fs27", "fs28", "fs29", "fs30", "fs31"
};
- return register_name (reg, regs, sizeof regs);
+ return register_name (reg, regs, ARRAY_SIZE (regs));
}
static struct type *
diff --git a/gdb/moxie-tdep.c b/gdb/moxie-tdep.c
index 77e1c33ecb6..86cfa6110d1 100644
--- a/gdb/moxie-tdep.c
+++ b/gdb/moxie-tdep.c
@@ -81,10 +81,7 @@ static const char * const moxie_register_names[] = {
static const char *
moxie_register_name (struct gdbarch *gdbarch, int reg_nr)
{
- if (reg_nr < 0)
- return NULL;
- if (reg_nr >= MOXIE_NUM_REGS)
- return NULL;
+ gdb_static_assert (ARRAY_SIZE (moxie_register_names) == MOXIE_NUM_REGS);
return moxie_register_names[reg_nr];
}
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 35bea093076..8f98965e8d0 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -196,6 +196,8 @@ msp430_register_name (struct gdbarch *gdbarch, int regnr)
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15"
};
+ gdb_static_assert (ARRAY_SIZE (reg_names) == (MSP430_NUM_REGS
+ + MSP430_NUM_PSEUDO_REGS));
return reg_names[regnr];
}
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index 5b555236d46..43d18c46d66 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -418,11 +418,8 @@ nds32_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
regnum -= gdbarch_num_regs (gdbarch);
/* Currently, only FSRs could be defined as pseudo registers. */
- if (regnum < gdbarch_num_pseudo_regs (gdbarch))
- return nds32_fsr_register_names[regnum];
-
- warning (_("Unknown nds32 pseudo register %d."), regnum);
- return NULL;
+ gdb_assert (regnum < gdbarch_num_pseudo_regs (gdbarch));
+ return nds32_fsr_register_names[regnum];
}
/* Implement the "pseudo_register_read" gdbarch method. */
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 0bad229b44a..14f593aba03 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -174,7 +174,7 @@ static const char *
nios2_register_name (struct gdbarch *gdbarch, int regno)
{
/* Use mnemonic aliases for GPRs. */
- if (regno >= 0 && regno < NIOS2_NUM_REGS)
+ if (regno < NIOS2_NUM_REGS)
return nios2_reg_names[regno];
else
return tdesc_register_name (gdbarch, regno);
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index 64b7ffa43da..739e4c9a9ba 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -115,8 +115,7 @@ sh_sh_register_name (struct gdbarch *gdbarch, int reg_nr)
"pc", "pr", "gbr", "vbr", "mach", "macl", "sr"
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -136,8 +135,7 @@ sh_sh3_register_name (struct gdbarch *gdbarch, int reg_nr)
"r0b1", "r1b1", "r2b1", "r3b1", "r4b1", "r5b1", "r6b1", "r7b1",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -156,8 +154,7 @@ sh_sh3e_register_name (struct gdbarch *gdbarch, int reg_nr)
"r0b0", "r1b0", "r2b0", "r3b0", "r4b0", "r5b0", "r6b0", "r7b0",
"r0b1", "r1b1", "r2b1", "r3b1", "r4b1", "r5b1", "r6b1", "r7b1",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -173,8 +170,7 @@ sh_sh2e_register_name (struct gdbarch *gdbarch, int reg_nr)
"fr0", "fr1", "fr2", "fr3", "fr4", "fr5", "fr6", "fr7",
"fr8", "fr9", "fr10", "fr11", "fr12", "fr13", "fr14", "fr15",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -212,8 +208,7 @@ sh_sh2a_register_name (struct gdbarch *gdbarch, int reg_nr)
/* double precision (pseudo) 68 - 75 */
"dr0", "dr2", "dr4", "dr6", "dr8", "dr10", "dr12", "dr14",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -250,8 +245,7 @@ sh_sh2a_nofpu_register_name (struct gdbarch *gdbarch, int reg_nr)
"bank",
/* double precision (pseudo) 68 - 75: report blank, see below. */
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -269,8 +263,7 @@ sh_sh_dsp_register_name (struct gdbarch *gdbarch, int reg_nr)
"", "",
"rs", "re",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -289,8 +282,7 @@ sh_sh3_dsp_register_name (struct gdbarch *gdbarch, int reg_nr)
"rs", "re", "", "", "", "", "", "",
"r0b", "r1b", "r2b", "r3b", "r4b", "r5b", "r6b", "r7b",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -326,8 +318,7 @@ sh_sh4_register_name (struct gdbarch *gdbarch, int reg_nr)
/* FIXME: missing XF */
/* FIXME: missing XD */
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -361,8 +352,7 @@ sh_sh4_nofpu_register_name (struct gdbarch *gdbarch, int reg_nr)
/* vectors (pseudo) 76 - 79 -- not for nofpu target: report blank
below. */
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
@@ -381,8 +371,7 @@ sh_sh4al_dsp_register_name (struct gdbarch *gdbarch, int reg_nr)
"rs", "re", "", "", "", "", "", "",
"r0b", "r1b", "r2b", "r3b", "r4b", "r5b", "r6b", "r7b",
};
- gdb_assert (reg_nr >= 0);
- if (reg_nr >= (sizeof (register_names) / sizeof (*register_names)))
+ if (reg_nr >= ARRAY_SIZE (register_names))
return "";
return register_names[reg_nr];
}
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 0f5da2c393a..cce67babd13 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -398,12 +398,8 @@ sparc32_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
{
regnum -= gdbarch_num_regs (gdbarch);
- if (regnum < SPARC32_NUM_PSEUDO_REGS)
- return sparc32_pseudo_register_names[regnum];
-
- internal_error (__FILE__, __LINE__,
- _("sparc32_pseudo_register_name: bad register number %d"),
- regnum);
+ gdb_assert (regnum < SPARC32_NUM_PSEUDO_REGS);
+ return sparc32_pseudo_register_names[regnum];
}
/* Return the name of register REGNUM. */
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 5ca5f2dca8c..c42ae4ff94f 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -810,12 +810,8 @@ sparc64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
{
regnum -= gdbarch_num_regs (gdbarch);
- if (regnum < SPARC64_NUM_PSEUDO_REGS)
- return sparc64_pseudo_register_names[regnum];
-
- internal_error (__FILE__, __LINE__,
- _("sparc64_pseudo_register_name: bad register number %d"),
- regnum);
+ gdb_assert (regnum < SPARC64_NUM_PSEUDO_REGS);
+ return sparc64_pseudo_register_names[regnum];
}
/* Return the name of register REGNUM. */
diff --git a/gdb/tic6x-tdep.c b/gdb/tic6x-tdep.c
index 77576834733..38ea045f3ab 100644
--- a/gdb/tic6x-tdep.c
+++ b/gdb/tic6x-tdep.c
@@ -102,9 +102,6 @@ static const int arg_regs[] = { 4, 20, 6, 22, 8, 24, 10, 26, 12, 28 };
static const char *
tic6x_register_name (struct gdbarch *gdbarch, int regno)
{
- if (regno < 0)
- return NULL;
-
if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
return tdesc_register_name (gdbarch, regno);
else if (regno >= ARRAY_SIZE (tic6x_register_names))
diff --git a/gdb/tilegx-tdep.c b/gdb/tilegx-tdep.c
index 744aca96438..fac917c01a1 100644
--- a/gdb/tilegx-tdep.c
+++ b/gdb/tilegx-tdep.c
@@ -142,7 +142,7 @@ template_reverse_regs[TILEGX_NUM_PHYS_REGS] =
static const char *
tilegx_register_name (struct gdbarch *gdbarch, int regnum)
{
- static const char *const register_names[TILEGX_NUM_REGS] =
+ static const char *const register_names[] =
{
"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
@@ -155,11 +155,7 @@ tilegx_register_name (struct gdbarch *gdbarch, int regnum)
"pc", "faultnum",
};
- if (regnum < 0 || regnum >= TILEGX_NUM_REGS)
- internal_error (__FILE__, __LINE__,
- "tilegx_register_name: invalid register number %d",
- regnum);
-
+ gdb_static_assert (TILEGX_NUM_REGS == ARRAY_SIZE (register_names));
return register_names[regnum];
}
diff --git a/gdb/v850-tdep.c b/gdb/v850-tdep.c
index a067bf74f16..6fc73cdc22e 100644
--- a/gdb/v850-tdep.c
+++ b/gdb/v850-tdep.c
@@ -312,8 +312,7 @@ v850_register_name (struct gdbarch *gdbarch, int regnum)
"sr24", "sr25", "sr26", "sr27", "sr28", "sr29", "sr30", "sr31",
"pc", "fp"
};
- if (regnum < 0 || regnum > E_NUM_OF_V850_REGS)
- return "";
+ gdb_static_assert (E_NUM_OF_V850_REGS == ARRAY_SIZE (v850_reg_names));
return v850_reg_names[regnum];
}
@@ -332,8 +331,7 @@ v850e_register_name (struct gdbarch *gdbarch, int regnum)
"sr24", "sr25", "sr26", "sr27", "sr28", "sr29", "sr30", "sr31",
"pc", "fp"
};
- if (regnum < 0 || regnum > E_NUM_OF_V850E_REGS)
- return "";
+ gdb_static_assert (E_NUM_OF_V850E_REGS == ARRAY_SIZE (v850e_reg_names));
return v850e_reg_names[regnum];
}
@@ -376,7 +374,7 @@ v850e2_register_name (struct gdbarch *gdbarch, int regnum)
"", "", "", "", "", "", "", "",
"", "", "", "fpspc"
};
- if (regnum < 0 || regnum >= E_NUM_OF_V850E2_REGS)
+ if (regnum >= E_NUM_OF_V850E2_REGS)
return "";
return v850e2_reg_names[regnum];
}
@@ -479,8 +477,8 @@ v850e3v5_register_name (struct gdbarch *gdbarch, int regnum)
"vr24", "vr25", "vr26", "vr27", "vr28", "vr29", "vr30", "vr31",
};
- if (regnum < 0 || regnum >= E_NUM_OF_V850E3V5_REGS)
- return "";
+ gdb_static_assert (E_NUM_OF_V850E3V5_REGS
+ == ARRAY_SIZE (v850e3v5_reg_names));
return v850e3v5_reg_names[regnum];
}
diff --git a/gdb/vax-tdep.c b/gdb/vax-tdep.c
index 6ad0dda021e..8de62124398 100644
--- a/gdb/vax-tdep.c
+++ b/gdb/vax-tdep.c
@@ -45,10 +45,8 @@ vax_register_name (struct gdbarch *gdbarch, int regnum)
"ps",
};
- if (regnum >= 0 && regnum < ARRAY_SIZE (register_names))
- return register_names[regnum];
-
- return NULL;
+ gdb_static_assert (VAX_NUM_REGS == ARRAY_SIZE (register_names));
+ return register_names[regnum];
}
/* Return the GDB type object for the "standard" data type of data in
diff --git a/gdb/xstormy16-tdep.c b/gdb/xstormy16-tdep.c
index 10c0d3e6b1c..a5192f0fcf4 100644
--- a/gdb/xstormy16-tdep.c
+++ b/gdb/xstormy16-tdep.c
@@ -108,13 +108,8 @@ xstormy16_register_name (struct gdbarch *gdbarch, int regnum)
"psw", "sp", "pc"
};
- if (regnum < 0 || regnum >= E_NUM_REGS)
- internal_error (__FILE__, __LINE__,
- _("xstormy16_register_name: illegal register number %d"),
- regnum);
- else
- return register_names[regnum];
-
+ gdb_static_assert (ARRAY_SIZE (register_names) == E_NUM_REGS);
+ return register_names[regnum];
}
static struct type *
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index f881870e244..8f8195d647a 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -242,11 +242,7 @@ xtensa_register_name (struct gdbarch *gdbarch, int regnum)
xtensa_gdbarch_tdep *tdep = gdbarch_tdep<xtensa_gdbarch_tdep> (gdbarch);
/* Return the name stored in the register map. */
- if (regnum >= 0 && regnum < gdbarch_num_cooked_regs (gdbarch))
- return tdep->regmap[regnum].name;
-
- internal_error (__FILE__, __LINE__, _("invalid register %d"), regnum);
- return 0;
+ return tdep->regmap[regnum].name;
}
/* Return the type of a register. Create a new type, if necessary. */
diff --git a/gdb/z80-tdep.c b/gdb/z80-tdep.c
index 684f08303c4..fe5cf993c01 100644
--- a/gdb/z80-tdep.c
+++ b/gdb/z80-tdep.c
@@ -167,10 +167,10 @@ static const char *z80_reg_names[] =
static const char *
z80_register_name (struct gdbarch *gdbarch, int regnum)
{
- if (regnum >= 0 && regnum < ARRAY_SIZE (z80_reg_names))
+ if (regnum < ARRAY_SIZE (z80_reg_names))
return z80_reg_names[regnum];
- return NULL;
+ return "";
}
/* Return the type of a register specified by the architecture. Only
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 9/9] gdb: update now gdbarch_register_name doesn't return nullptr
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (7 preceding siblings ...)
2022-09-01 21:31 ` [PATCH 8/9] gdb: final cleanup of various gdbarch_register_name methods Andrew Burgess
@ 2022-09-01 21:31 ` Andrew Burgess
2022-09-21 18:07 ` [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Tom Tromey
9 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-09-01 21:31 UTC (permalink / raw)
To: gdb-patches
After the previous few commit, gdbarch_register_name no longer returns
nullptr. This commit audits all the calls to gdbarch_register_name
and removes any code that checks the result against nullptr.
There should be no visible change after this commit.
---
gdb/alpha-tdep.c | 3 +--
gdb/arch-utils.c | 3 +--
gdb/csky-tdep.c | 3 +--
gdb/h8300-tdep.c | 2 +-
gdb/infcmd.c | 3 +--
gdb/m68hc11-tdep.c | 2 +-
gdb/mi/mi-main.c | 20 ++++++--------------
gdb/mips-tdep.c | 8 ++------
gdb/nds32-tdep.c | 3 +--
gdb/regcache.c | 5 +----
gdb/reggroups.c | 3 +--
gdb/riscv-tdep.c | 9 +++------
gdb/sh-tdep.c | 3 +--
gdb/tui/tui-regs.c | 4 ++--
gdb/user-regs.c | 10 +++-------
gdb/valops.c | 2 +-
16 files changed, 27 insertions(+), 56 deletions(-)
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 8a5697939ec..fdabf94692a 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -140,8 +140,7 @@ alpha_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
{
/* Filter out any registers eliminated, but whose regnum is
reserved for backward compatibility, e.g. the vfp. */
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *gdbarch_register_name (gdbarch, regnum) == '\0')
+ if (*gdbarch_register_name (gdbarch, regnum) == '\0')
return 0;
if (group == all_reggroup)
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 9bd4f0ddae6..f0edaf4381e 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -75,8 +75,7 @@ legacy_register_sim_regno (struct gdbarch *gdbarch, int regnum)
suspected that some GDB/SIM combinations may rely on this
behaviour. The default should be one2one_register_sim_regno
(below). */
- if (gdbarch_register_name (gdbarch, regnum) != NULL
- && gdbarch_register_name (gdbarch, regnum)[0] != '\0')
+ if (gdbarch_register_name (gdbarch, regnum)[0] != '\0')
return regnum;
else
return LEGACY_SIM_REGNO_IGNORE;
diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index 51336148b6d..224661d2d5e 100644
--- a/gdb/csky-tdep.c
+++ b/gdb/csky-tdep.c
@@ -2498,8 +2498,7 @@ csky_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
{
int raw_p;
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || gdbarch_register_name (gdbarch, regnum)[0] == '\0')
+ if (gdbarch_register_name (gdbarch, regnum)[0] == '\0')
return 0;
if (reggroup == all_reggroup)
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 0d1ab463ef3..636d8c93a3e 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -997,7 +997,7 @@ h8300_print_register (struct gdbarch *gdbarch, struct ui_file *file,
LONGEST rval;
const char *name = gdbarch_register_name (gdbarch, regno);
- if (!name || !*name)
+ if (*name == '\0')
return;
rval = get_frame_register_signed (frame, regno);
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 69a7896b560..357bde6c481 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2224,8 +2224,7 @@ default_print_registers_info (struct gdbarch *gdbarch,
/* If the register name is empty, it is undefined for this
processor, so don't display anything. */
- if (gdbarch_register_name (gdbarch, i) == NULL
- || *(gdbarch_register_name (gdbarch, i)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, i)) == '\0')
continue;
default_print_one_register_info (file,
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 6277572adee..08fa2e0bf04 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -1090,7 +1090,7 @@ m68hc11_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file,
{
const char *name = gdbarch_register_name (gdbarch, regno);
- if (!name || !*name)
+ if (*name == '\0')
return;
gdb_printf (file, "%-10s ", name);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index b758f398e2a..80b5cd9bf90 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -880,8 +880,7 @@ mi_cmd_data_list_register_names (const char *command, char **argv, int argc)
regnum < numregs;
regnum++)
{
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
uiout->field_string (NULL, "");
else
uiout->field_string (NULL, gdbarch_register_name (gdbarch, regnum));
@@ -895,8 +894,7 @@ mi_cmd_data_list_register_names (const char *command, char **argv, int argc)
if (regnum < 0 || regnum >= numregs)
error (_("bad register number"));
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
uiout->field_string (NULL, "");
else
uiout->field_string (NULL, gdbarch_register_name (gdbarch, regnum));
@@ -940,8 +938,7 @@ mi_cmd_data_list_changed_registers (const char *command, char **argv, int argc)
regnum < numregs;
regnum++)
{
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
continue;
if (register_changed_p (regnum, prev_regs.get (),
@@ -957,7 +954,6 @@ mi_cmd_data_list_changed_registers (const char *command, char **argv, int argc)
if (regnum >= 0
&& regnum < numregs
- && gdbarch_register_name (gdbarch, regnum) != NULL
&& *gdbarch_register_name (gdbarch, regnum) != '\000')
{
if (register_changed_p (regnum, prev_regs.get (),
@@ -1067,8 +1063,7 @@ mi_cmd_data_list_register_values (const char *command, char **argv, int argc)
regnum < numregs;
regnum++)
{
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
continue;
output_register (frame, regnum, format, skip_unavailable);
@@ -1082,7 +1077,6 @@ mi_cmd_data_list_register_values (const char *command, char **argv, int argc)
if (regnum >= 0
&& regnum < numregs
- && gdbarch_register_name (gdbarch, regnum) != NULL
&& *gdbarch_register_name (gdbarch, regnum) != '\000')
output_register (frame, regnum, format, skip_unavailable);
else
@@ -1163,8 +1157,7 @@ mi_cmd_data_write_register_values (const char *command, char **argv, int argc)
int regnum = atoi (argv[i]);
if (regnum >= 0 && regnum < numregs
- && gdbarch_register_name (gdbarch, regnum)
- && *gdbarch_register_name (gdbarch, regnum))
+ && *gdbarch_register_name (gdbarch, regnum) != '\0')
{
LONGEST value;
@@ -2632,8 +2625,7 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
for (regnum = 0; regnum < numregs; regnum++)
{
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
continue;
output_register (frame, regnum, registers_format, 1);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 98270268584..30c97356500 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -712,8 +712,7 @@ mips_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
/* FIXME: cagney/2003-04-13: Can't yet use gdbarch_num_regs
(gdbarch), as not all architectures are multi-arch. */
raw_p = rawnum < gdbarch_num_regs (gdbarch);
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || gdbarch_register_name (gdbarch, regnum)[0] == '\0')
+ if (gdbarch_register_name (gdbarch, regnum)[0] == '\0')
return 0;
if (reggroup == float_reggroup)
return float_p && pseudo;
@@ -7950,10 +7949,7 @@ mips_register_sim_regno (struct gdbarch *gdbarch, int regnum)
decide if it is valid. Should instead define a standard sim/gdb
register numbering scheme. */
if (gdbarch_register_name (gdbarch,
- gdbarch_num_regs (gdbarch) + regnum) != NULL
- && gdbarch_register_name (gdbarch,
- gdbarch_num_regs (gdbarch)
- + regnum)[0] != '\0')
+ gdbarch_num_regs (gdbarch) + regnum)[0] != '\0')
return regnum;
else
return LEGACY_SIM_REGNO_IGNORE;
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index 43d18c46d66..1dc59d6e104 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -2030,8 +2030,7 @@ nds32_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
{
const char *regname = gdbarch_register_name (gdbarch, j);
- if (regname != NULL
- && strcmp (regname, nds32_register_aliases[i].name) == 0)
+ if (strcmp (regname, nds32_register_aliases[i].name) == 0)
{
regnum = j;
break;
diff --git a/gdb/regcache.c b/gdb/regcache.c
index a22a839d902..1caf851dca9 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1392,7 +1392,6 @@ regcache::debug_print_register (const char *func, int regno)
gdb_printf (gdb_stdlog, "%s ", func);
if (regno >= 0 && regno < gdbarch_num_regs (gdbarch)
- && gdbarch_register_name (gdbarch, regno) != NULL
&& gdbarch_register_name (gdbarch, regno)[0] != '\0')
gdb_printf (gdb_stdlog, "(%s)",
gdbarch_register_name (gdbarch, regno));
@@ -1453,9 +1452,7 @@ register_dump::dump (ui_file *file)
{
const char *p = gdbarch_register_name (m_gdbarch, regnum);
- if (p == NULL)
- p = "";
- else if (p[0] == '\0')
+ if (p[0] == '\0')
p = "''";
gdb_printf (file, " %-10s", p);
}
diff --git a/gdb/reggroups.c b/gdb/reggroups.c
index b81031b42ac..8e4af303c54 100644
--- a/gdb/reggroups.c
+++ b/gdb/reggroups.c
@@ -146,8 +146,7 @@ default_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
int float_p;
int raw_p;
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *gdbarch_register_name (gdbarch, regnum) == '\0')
+ if (*gdbarch_register_name (gdbarch, regnum) == '\0')
return 0;
if (group == all_reggroup)
return 1;
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 088cf4ac55e..d3050a46b00 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1283,8 +1283,7 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
{
/* Used by 'info registers' and 'info registers <groupname>'. */
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || gdbarch_register_name (gdbarch, regnum)[0] == '\0')
+ if (gdbarch_register_name (gdbarch, regnum)[0] == '\0')
return 0;
if (regnum > RISCV_LAST_REGNUM)
@@ -1370,8 +1369,7 @@ riscv_print_registers_info (struct gdbarch *gdbarch,
if (regnum != -1)
{
/* Print one specified register. */
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
error (_("Not a valid register for the current processor type"));
riscv_print_one_register_info (gdbarch, file, frame, regnum);
}
@@ -1391,8 +1389,7 @@ riscv_print_registers_info (struct gdbarch *gdbarch,
continue;
/* Registers with no name are not valid on this ISA. */
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
+ if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
continue;
/* Is the register in the group we're interested in? */
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index 739e4c9a9ba..4a8822fe2d8 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -1452,8 +1452,7 @@ static int
sh_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
const struct reggroup *reggroup)
{
- if (gdbarch_register_name (gdbarch, regnum) == NULL
- || *gdbarch_register_name (gdbarch, regnum) == '\0')
+ if (*gdbarch_register_name (gdbarch, regnum) == '\0')
return 0;
if (reggroup == float_reggroup
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 5106a3b9670..830df5427c7 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -231,7 +231,7 @@ tui_data_window::show_register_group (const reggroup *group,
/* If the register name is empty, it is undefined for this
processor, so don't display anything. */
name = gdbarch_register_name (gdbarch, regnum);
- if (name == 0 || *name == '\0')
+ if (*name == '\0')
continue;
nr_regs++;
@@ -253,7 +253,7 @@ tui_data_window::show_register_group (const reggroup *group,
/* If the register name is empty, it is undefined for this
processor, so don't display anything. */
name = gdbarch_register_name (gdbarch, regnum);
- if (name == 0 || *name == '\0')
+ if (*name == '\0')
continue;
data_item_win = &m_regs_content[pos];
diff --git a/gdb/user-regs.c b/gdb/user-regs.c
index 05bb04ef2ed..3030d952cf1 100644
--- a/gdb/user-regs.c
+++ b/gdb/user-regs.c
@@ -139,18 +139,14 @@ user_reg_map_name_to_regnum (struct gdbarch *gdbarch, const char *name,
/* Search register name space first - always let an architecture
specific register override the user registers. */
{
- int i;
int maxregs = gdbarch_num_cooked_regs (gdbarch);
- for (i = 0; i < maxregs; i++)
+ for (int i = 0; i < maxregs; i++)
{
const char *regname = gdbarch_register_name (gdbarch, i);
- if (regname != NULL && len == strlen (regname)
- && strncmp (regname, name, len) == 0)
- {
- return i;
- }
+ if (len == strlen (regname) && strncmp (regname, name, len) == 0)
+ return i;
}
}
diff --git a/gdb/valops.c b/gdb/valops.c
index 27e84d9f6b3..0d183e1e512 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1417,7 +1417,7 @@ address_of_variable (struct symbol *var, const struct block *b)
regname = gdbarch_register_name (get_frame_arch (frame),
VALUE_REGNUM (val));
- gdb_assert (regname && *regname);
+ gdb_assert (regname != nullptr && *regname != '\0');
error (_("Address requested for identifier "
"\"%s\" which is in register $%s"),
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/9] gdb: add asserts to gdbarch_register_name
2022-09-01 21:31 ` [PATCH 6/9] gdb: add asserts to gdbarch_register_name Andrew Burgess
@ 2022-09-21 18:04 ` Tom Tromey
0 siblings, 0 replies; 13+ messages in thread
From: Tom Tromey @ 2022-09-21 18:04 UTC (permalink / raw)
To: Andrew Burgess via Gdb-patches
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
Andrew> index 696b3028e6f..699ed4f69d2 100755
Andrew> --- a/gdb/gdbarch.py
Andrew> +++ b/gdb/gdbarch.py
Andrew> @@ -112,6 +112,8 @@ class Function(_Component):
Andrew> postdefault=None,
Andrew> invalid=None,
Andrew> printer=None,
Andrew> + param_checks=None,
Andrew> + result_checks=None,
Andrew> ):
Andrew> super().__init__(
Andrew> comment=comment,
Andrew> @@ -124,6 +126,11 @@ class Function(_Component):
Andrew> printer=printer,
Andrew> params=params,
Andrew> )
Andrew> + self.param_checks = param_checks
Andrew> + self.result_checks = result_checks
It's fine to just pass these to the superclass constructor.
That will ensure they end up as attributes of 'self', without needing
separate assignments.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures)
2022-09-01 21:31 [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures) Andrew Burgess
` (8 preceding siblings ...)
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 ` Tom Tromey
2022-10-02 16:28 ` Andrew Burgess
9 siblings, 1 reply; 13+ messages in thread
From: Tom Tromey @ 2022-09-21 18:07 UTC (permalink / raw)
To: Andrew Burgess via Gdb-patches
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> This started with a single RISC-V test failure in
Andrew> gdb.base/completion.exp.
Andrew> It ended with me changing gdbarch_register_name for almost every
Andrew> architecture.
Thanks for doing this. I read through the series and had one small
comment. The rest looks good to me.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/9] Lots of changes to gdbarch_register_name (many architectures)
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
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-10-02 16:28 UTC (permalink / raw)
To: Tom Tromey, Andrew Burgess via Gdb-patches
Tom Tromey <tom@tromey.com> writes:
>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> This started with a single RISC-V test failure in
> Andrew> gdb.base/completion.exp.
>
> Andrew> It ended with me changing gdbarch_register_name for almost every
> Andrew> architecture.
>
> Thanks for doing this. I read through the series and had one small
> comment. The rest looks good to me.
Thanks, I updated patch #6 as you suggested, and pushed this series.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-10-02 16:28 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 5/9] gdb: check for duplicate register names in selftest Andrew Burgess
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
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).