From e45020422076c1014579ae8763fd253990ddf58e Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Tue, 24 Nov 2020 18:08:25 +0000 Subject: [PATCH] gdb/riscv: place unknown csrs into the correct register groups Unknown riscv CSRs should not be in the 'general' group, but should be in the system and csr register groups. To see this in action connect to QEMU, this target advertises two registers dscratch and mucounteren which are unknown to GDB (these are legacy CSRs). Before this commit these registers would show up in the output of: (gdb) info registers .... dscratch Could not fetch register "dscratch"; remote failure reply 'E14' mucounteren Could not fetch register "mucounteren"; remote failure reply 'E14' Ignore the errors, this is just a QEMU annoyance, it advertises these CSRs, but doesn't actually let GDB read them. These registers don't show up in the output of either: (gdb) info registers csr (gdb) info registers system After this commit this situation is reveresed, which makes more sense to me. gdb/ChangeLog: * riscv-tdep.c (riscv_is_unknown_csr): New function, implementation moved from riscv_register_reggroup_p. (riscv_register_reggroup_p): Update group handling for unknown CSRs. gdb/testsuite/ChangeLog: * gdb.arch/riscv-tdesc-regs.exp (get_expected_result): New proc, update test to use this. --- gdb/ChangeLog | 7 ++++ gdb/riscv-tdep.c | 34 ++++++++++++++---- gdb/testsuite/ChangeLog | 5 +++ gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp | 39 ++++++++++++++++----- 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4946a522df8..1d5bbcfe696 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2020-12-02 Andrew Burgess + + * riscv-tdep.c (riscv_is_unknown_csr): New function, + implementation moved from riscv_register_reggroup_p. + (riscv_register_reggroup_p): Update group handling for unknown + CSRs. + 2020-12-01 Sergio Durigan Junior * dwarf2/read.c (dwz_search_other_debugdirs): New function. diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 4e255056863..437e8f847ae 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -969,6 +969,18 @@ riscv_is_regnum_a_named_csr (int regnum) } } +/* Return true if REGNUM is an unknown CSR identified in + riscv_tdesc_unknown_reg for GDBARCH. */ + +static bool +riscv_is_unknown_csr (struct gdbarch *gdbarch, int regnum) +{ + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); + return (regnum >= tdep->unknown_csrs_first_regnum + && regnum < (tdep->unknown_csrs_first_regnum + + tdep->unknown_csrs_count)); +} + /* Implement the register_reggroup_p gdbarch method. Is REGNUM a member of REGGROUP? */ @@ -986,13 +998,21 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, { /* Any extra registers from the CSR tdesc_feature (identified in riscv_tdesc_unknown_reg) are removed from the save/restore groups - as some targets (QEMU) report CSRs which then can't be read. */ - struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); - if ((reggroup == restore_reggroup || reggroup == save_reggroup) - && regnum >= tdep->unknown_csrs_first_regnum - && regnum < (tdep->unknown_csrs_first_regnum - + tdep->unknown_csrs_count)) - return 0; + as some targets (QEMU) report CSRs which then can't be read and + having unreadable registers in the save/restore group breaks + things like inferior calls. + + The unknown CSRs are also removed from the general group, and + added into both the csr and system group. This is inline with the + known CSRs (see below). */ + if (riscv_is_unknown_csr (gdbarch, regnum)) + { + if (reggroup == restore_reggroup || reggroup == save_reggroup + || reggroup == general_reggroup) + return 0; + else if (reggroup == system_reggroup || reggroup == csr_reggroup) + return 1; + } /* This is some other unknown register from the target description. In this case we trust whatever the target description says about diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index f35a2dc72ac..f0938453932 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-12-02 Andrew Burgess + + * gdb.arch/riscv-tdesc-regs.exp (get_expected_result): New proc, + update test to use this. + 2020-12-01 Simon Marchi * gdb.threads/non-ldr-exc-1.exp: Fix indentation. diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp index 1be32e0e8a1..e35d6181b7f 100644 --- a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp +++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp @@ -80,7 +80,32 @@ gdb_test "info registers \$csr0" "Invalid register `csr0'" gdb_test "info registers \$dscratch0" "dscratch0\[ \t\]+.*" gdb_test "info registers \$dscratch" "dscratch\[ \t\]+.*" -foreach rgroup {x_all all save restore} { +# Return the number of times REGISTER should appear in GROUP, this +# will either be 0 or 1. +proc get_expected_result { register group } { + + # Everything should appear once in the 'all' group. + if { $group == "all" || $group == "x_all" } { + return 1 + } + + if { $group == "save" || $group == "restore" } { + # Everything is in the save/restore groups except these two. + if { $register == "unknown_csr" || $register == "dscratch" } { + return 0 + } + return 1 + } + + if { $group == "system" || $group == "csr" } { + # All the registers we check should be in these groups. + return 1 + } + + return 0 +} + +foreach rgroup {x_all all save restore general system csr} { # Now use 'info registers all' to see how many times the floating # point status registers show up in the output. array set reg_counts {} @@ -110,14 +135,10 @@ foreach rgroup {x_all all save restore} { } else { set count 0 } - if {($reg == "unknown_csr" || $reg == "dscratch") \ - && $rgroup != "all" && $rgroup != "x_all"} { - gdb_assert {$count == 0} \ - "register $reg not seen in reggroup $rgroup" - } else { - gdb_assert {$count == 1} \ - "register $reg seen once in reggroup $rgroup" - } + + set expected_count [ get_expected_result $reg $rgroup ] + gdb_assert {$count == $expected_count} \ + "register $reg seen in reggroup $rgroup $expected_count times" } array unset reg_counts } -- 2.30.2