]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
gdb/riscv: fix failure in gdb.base/completion.exp
authorAndrew Burgess <aburgess@redhat.com>
Sun, 28 Aug 2022 19:40:28 +0000 (20:40 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Sun, 2 Oct 2022 13:21:24 +0000 (14:21 +0100)
commitbd93abe8d5513ceecc10a91452a85c803cac895b
tree3903867354415f3e23b68d487041a083dbeba6a0
parente7b1ba07bc9dc28819c9a2523f5e5cdb43e13ca2
gdb/riscv: fix failure in gdb.base/completion.exp

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