From patchwork Tue Oct 16 21:49:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 29772 Received: (qmail 85337 invoked by alias); 16 Oct 2018 21:50:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 85321 invoked by uid 89); 16 Oct 2018 21:50:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=x26, HX-Received:sk:p10-v6m, compressed, sk:riscv_f X-HELO: mail-wm1-f65.google.com Received: from mail-wm1-f65.google.com (HELO mail-wm1-f65.google.com) (209.85.128.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 16 Oct 2018 21:50:09 +0000 Received: by mail-wm1-f65.google.com with SMTP id i8-v6so16954wmg.0 for ; Tue, 16 Oct 2018 14:50:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id; bh=yZZWie1hUEHFHoFJbXCjjhO3B4TcvCX7Ra5n0l7az2w=; b=FdaH0vY93HT5GJ5bIS6V4iFfuLX7Sehc2o81k3BXIjjlz/Llr15WHY9EaocNQwUmcj Xzax33ELGJKOvI6vmMDTaRGoIOxAygB6OkEKS8/SwQ5MSUjJ9edkoDuCToeIUbIKk/Jt rlIh2b4A2ixlEoBysfxBxNVgAsB1DwInfUyWv75R5ikTEXzf9dHRrwgL6hkdrYYGxRJO c7uFQubyi4uiuICR6O9D8S7OgNSFQBeSlm5sZJ6NtWp1aOI+QIKfOTC8kHt/h7E4N8GG GEgc1willk2LLbOFvi4dbzMqMYLcI72U+AyxCZvO6Sofv6dI77Cj2/RUjcKtnjv8N5ir fICw== Return-Path: Received: from localhost (host86-164-85-193.range86-164.btcentralplus.com. [86.164.85.193]) by smtp.gmail.com with ESMTPSA id c184-v6sm10494493wma.15.2018.10.16.14.50.05 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Oct 2018 14:50:05 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: jimw@sifive.com, palmer@sifive.com, carsten.gosvig@sifive.com, Andrew Burgess Subject: [PATCH] gdb/riscv: Fix register access for register aliases Date: Tue, 16 Oct 2018 22:49:57 +0100 Message-Id: <20181016214957.23680-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes Some confusion over how the register names and aliases are setup in riscv means that we currently can't access registers through their architectural name. This commit fixes this issue, and moves some of the csr register handling out of the alias handling code and deals with it separately. This has the benefit that we can now directly access some arrays rather than having to iterate over them. A new test is added to ensure that register aliases now work correctly. gdb/ChangeLog: * riscv-tdep.c (riscv_gdb_reg_names): Update comment, and all register names. (struct register_alias): Rename to... (struct riscv_register_alias): ...this, and update comment. (riscv_register_aliases): Update type, and alias names. Remove CSR names from this list. (riscv_register_name): Use riscv_gdb_reg_names for int and float register names. Add an extra assertion. (riscv_is_regnum_a_named_csr): New function. (riscv_register_reggroup_p): Use riscv_is_regnum_a_named_csr. gdb/testsuite/ChangeLog: * gdb.arch/riscv-reg-aliases.c: New file. * gdb.arch/riscv-reg-aliases.exp: New file. --- gdb/ChangeLog | 13 ++ gdb/riscv-tdep.c | 219 ++++++++++++++------------- gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.arch/riscv-reg-aliases.c | 22 +++ gdb/testsuite/gdb.arch/riscv-reg-aliases.exp | 130 ++++++++++++++++ 5 files changed, 285 insertions(+), 104 deletions(-) create mode 100644 gdb/testsuite/gdb.arch/riscv-reg-aliases.c create mode 100644 gdb/testsuite/gdb.arch/riscv-reg-aliases.exp diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 3402241b972..c4916e24c4c 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -95,24 +95,25 @@ struct riscv_unwind_cache CORE_ADDR frame_base; }; -/* Architectural name for core registers. */ +/* The preferred register names for all the general purpose and floating + point registers. These are what GDB will use when referencing a + register. */ static const char * const riscv_gdb_reg_names[RISCV_LAST_FP_REGNUM + 1] = { - "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", - "x8", "x9", "x10", "x11", "x12", "x13", "x14", "x15", - "x16", "x17", "x18", "x19", "x20", "x21", "x22", "x23", - "x24", "x25", "x26", "x27", "x28", "x29", "x30", "x31", - "pc", - "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", - "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15", - "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23", - "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", + "zero", "ra", "sp", "gp", "tp", "t0", "t1", "t2", "fp", "s1", + "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "s2", "s3", "s4", + "s5", "s6", "s7", "s8", "s9", "s10", "s11", "t3", "t4", "t5", "t6", + "pc", + "ft0", "ft1", "ft2", "ft3", "ft4", "ft5", "ft6", "ft7", "fs0", "fs1", + "fa0", "fa1", "fa2", "fa3", "fa4", "fa5", "fa6", "fa7", "fs2", "fs3", + "fs4", "fs5", "fs6", "fs7", "fs8", "fs9", "fs10", "fs11", "ft8", "ft9", + "ft10", "ft11", }; -/* Maps "pretty" register names onto their GDB register number. */ +/* Map alternative register names onto their GDB register number. */ -struct register_alias +struct riscv_register_alias { /* The register alias. Usually more descriptive than the architectural name of the register. */ @@ -124,77 +125,78 @@ struct register_alias /* Table of register aliases. */ -static const struct register_alias riscv_register_aliases[] = +static const struct riscv_register_alias riscv_register_aliases[] = { - { "zero", 0 }, - { "ra", 1 }, - { "sp", 2 }, - { "gp", 3 }, - { "tp", 4 }, - { "t0", 5 }, - { "t1", 6 }, - { "t2", 7 }, - { "s0", 8 }, - { "fp", 8 }, - { "s1", 9 }, - { "a0", 10 }, - { "a1", 11 }, - { "a2", 12 }, - { "a3", 13 }, - { "a4", 14 }, - { "a5", 15 }, - { "a6", 16 }, - { "a7", 17 }, - { "s2", 18 }, - { "s3", 19 }, - { "s4", 20 }, - { "s5", 21 }, - { "s6", 22 }, - { "s7", 23 }, - { "s8", 24 }, - { "s9", 25 }, - { "s10", 26 }, - { "s11", 27 }, - { "t3", 28 }, - { "t4", 29 }, - { "t5", 30 }, - { "t6", 31 }, - /* pc is 32. */ - { "ft0", 33 }, - { "ft1", 34 }, - { "ft2", 35 }, - { "ft3", 36 }, - { "ft4", 37 }, - { "ft5", 38 }, - { "ft6", 39 }, - { "ft7", 40 }, - { "fs0", 41 }, - { "fs1", 42 }, - { "fa0", 43 }, - { "fa1", 44 }, - { "fa2", 45 }, - { "fa3", 46 }, - { "fa4", 47 }, - { "fa5", 48 }, - { "fa6", 49 }, - { "fa7", 50 }, - { "fs2", 51 }, - { "fs3", 52 }, - { "fs4", 53 }, - { "fs5", 54 }, - { "fs6", 55 }, - { "fs7", 56 }, - { "fs8", 57 }, - { "fs9", 58 }, - { "fs10", 59 }, - { "fs11", 60 }, - { "ft8", 61 }, - { "ft9", 62 }, - { "ft10", 63 }, - { "ft11", 64 }, -#define DECLARE_CSR(name, num) { #name, (num) + 65 }, -#include "opcode/riscv-opc.h" -#undef DECLARE_CSR + /* Aliases for general purpose registers. These are the architectural + names, as GDB uses the more user friendly names by default. */ + { "x0", (RISCV_ZERO_REGNUM + 0) }, + { "x1", (RISCV_ZERO_REGNUM + 1) }, + { "x2", (RISCV_ZERO_REGNUM + 2) }, + { "x3", (RISCV_ZERO_REGNUM + 3) }, + { "x4", (RISCV_ZERO_REGNUM + 4) }, + { "x5", (RISCV_ZERO_REGNUM + 5) }, + { "x6", (RISCV_ZERO_REGNUM + 6) }, + { "x7", (RISCV_ZERO_REGNUM + 7) }, + { "x8", (RISCV_ZERO_REGNUM + 8) }, + { "s0", (RISCV_ZERO_REGNUM + 8) }, /* fp, s0, and x8 are all aliases. */ + { "x9", (RISCV_ZERO_REGNUM + 9) }, + { "x10", (RISCV_ZERO_REGNUM + 10) }, + { "x11", (RISCV_ZERO_REGNUM + 11) }, + { "x12", (RISCV_ZERO_REGNUM + 12) }, + { "x13", (RISCV_ZERO_REGNUM + 13) }, + { "x14", (RISCV_ZERO_REGNUM + 14) }, + { "x15", (RISCV_ZERO_REGNUM + 15) }, + { "x16", (RISCV_ZERO_REGNUM + 16) }, + { "x17", (RISCV_ZERO_REGNUM + 17) }, + { "x18", (RISCV_ZERO_REGNUM + 18) }, + { "x19", (RISCV_ZERO_REGNUM + 19) }, + { "x20", (RISCV_ZERO_REGNUM + 20) }, + { "x21", (RISCV_ZERO_REGNUM + 21) }, + { "x22", (RISCV_ZERO_REGNUM + 22) }, + { "x23", (RISCV_ZERO_REGNUM + 23) }, + { "x24", (RISCV_ZERO_REGNUM + 24) }, + { "x25", (RISCV_ZERO_REGNUM + 25) }, + { "x26", (RISCV_ZERO_REGNUM + 26) }, + { "x27", (RISCV_ZERO_REGNUM + 27) }, + { "x28", (RISCV_ZERO_REGNUM + 28) }, + { "x29", (RISCV_ZERO_REGNUM + 29) }, + { "x30", (RISCV_ZERO_REGNUM + 30) }, + { "x31", (RISCV_ZERO_REGNUM + 31) }, + + /* Aliases for the integer registers. These are the architectural names + as GDB uses the more user friendly names by default. */ + { "f0", (RISCV_FIRST_FP_REGNUM + 0) }, + { "f1", (RISCV_FIRST_FP_REGNUM + 1) }, + { "f2", (RISCV_FIRST_FP_REGNUM + 2) }, + { "f3", (RISCV_FIRST_FP_REGNUM + 3) }, + { "f4", (RISCV_FIRST_FP_REGNUM + 4) }, + { "f5", (RISCV_FIRST_FP_REGNUM + 5) }, + { "f6", (RISCV_FIRST_FP_REGNUM + 6) }, + { "f7", (RISCV_FIRST_FP_REGNUM + 7) }, + { "f8", (RISCV_FIRST_FP_REGNUM + 8) }, + { "f9", (RISCV_FIRST_FP_REGNUM + 9) }, + { "f10", (RISCV_FIRST_FP_REGNUM + 10) }, + { "f11", (RISCV_FIRST_FP_REGNUM + 11) }, + { "f12", (RISCV_FIRST_FP_REGNUM + 12) }, + { "f13", (RISCV_FIRST_FP_REGNUM + 13) }, + { "f14", (RISCV_FIRST_FP_REGNUM + 14) }, + { "f15", (RISCV_FIRST_FP_REGNUM + 15) }, + { "f16", (RISCV_FIRST_FP_REGNUM + 16) }, + { "f17", (RISCV_FIRST_FP_REGNUM + 17) }, + { "f18", (RISCV_FIRST_FP_REGNUM + 18) }, + { "f19", (RISCV_FIRST_FP_REGNUM + 19) }, + { "f20", (RISCV_FIRST_FP_REGNUM + 20) }, + { "f21", (RISCV_FIRST_FP_REGNUM + 21) }, + { "f22", (RISCV_FIRST_FP_REGNUM + 22) }, + { "f23", (RISCV_FIRST_FP_REGNUM + 23) }, + { "f24", (RISCV_FIRST_FP_REGNUM + 24) }, + { "f25", (RISCV_FIRST_FP_REGNUM + 25) }, + { "f26", (RISCV_FIRST_FP_REGNUM + 26) }, + { "f27", (RISCV_FIRST_FP_REGNUM + 27) }, + { "f28", (RISCV_FIRST_FP_REGNUM + 28) }, + { "f29", (RISCV_FIRST_FP_REGNUM + 29) }, + { "f30", (RISCV_FIRST_FP_REGNUM + 30) }, + { "f31", (RISCV_FIRST_FP_REGNUM + 31) }, }; /* Controls whether we place compressed breakpoints or not. When in auto @@ -468,17 +470,15 @@ static const char * riscv_register_name (struct gdbarch *gdbarch, int regnum) { /* Prefer to use the alias. */ - if (regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_LAST_REGNUM) + if (regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_LAST_FP_REGNUM) { - int i; - - for (i = 0; i < ARRAY_SIZE (riscv_register_aliases); ++i) - if (regnum == riscv_register_aliases[i].regnum) - return riscv_register_aliases[i].name; + gdb_assert (regnum < ARRAY_SIZE (riscv_gdb_reg_names)); + return riscv_gdb_reg_names[regnum]; } - if (regnum >= RISCV_ZERO_REGNUM && regnum <= RISCV_LAST_FP_REGNUM) - return riscv_gdb_reg_names[regnum]; + /* Check that there's no gap between the set of registers handled above, + and the set of registers handled next. */ + gdb_assert ((RISCV_LAST_FP_REGNUM + 1) == RISCV_FIRST_CSR_REGNUM); if (regnum >= RISCV_FIRST_CSR_REGNUM && regnum <= RISCV_LAST_CSR_REGNUM) { @@ -759,6 +759,27 @@ riscv_print_one_register_info (struct gdbarch *gdbarch, fprintf_filtered (file, "\n"); } +/* Return true if REGNUM is a valid CSR register. The CSR register space + is sparsely populated, so not every number is a named CSR. */ + +static bool +riscv_is_regnum_a_named_csr (int regnum) +{ + gdb_assert (regnum >= RISCV_FIRST_CSR_REGNUM + && regnum <= RISCV_LAST_CSR_REGNUM); + + switch (regnum) + { +#define DECLARE_CSR(name, num) case RISCV_ ## num ## _REGNUM: +#include "opcode/riscv-opc.h" +#undef DECLARE_CSR + return true; + + default: + return false; + } +} + /* Implement the register_reggroup_p gdbarch method. Is REGNUM a member of REGGROUP? */ @@ -766,8 +787,6 @@ static int riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, struct reggroup *reggroup) { - unsigned int i; - /* Used by 'info registers' and 'info registers '. */ if (gdbarch_register_name (gdbarch, regnum) == NULL @@ -778,12 +797,8 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, { if (regnum < RISCV_FIRST_CSR_REGNUM || regnum == RISCV_PRIV_REGNUM) return 1; - /* Only include CSRs that have aliases. */ - for (i = 0; i < ARRAY_SIZE (riscv_register_aliases); ++i) - { - if (regnum == riscv_register_aliases[i].regnum) - return 1; - } + if (riscv_is_regnum_a_named_csr (regnum)) + return 1; return 0; } else if (reggroup == float_reggroup) @@ -806,12 +821,8 @@ riscv_register_reggroup_p (struct gdbarch *gdbarch, int regnum, return 1; if (regnum < RISCV_FIRST_CSR_REGNUM || regnum > RISCV_LAST_CSR_REGNUM) return 0; - /* Only include CSRs that have aliases. */ - for (i = 0; i < ARRAY_SIZE (riscv_register_aliases); ++i) - { - if (regnum == riscv_register_aliases[i].regnum) - return 1; - } + if (riscv_is_regnum_a_named_csr (regnum)) + return 1; return 0; } else if (reggroup == vector_reggroup) diff --git a/gdb/testsuite/gdb.arch/riscv-reg-aliases.c b/gdb/testsuite/gdb.arch/riscv-reg-aliases.c new file mode 100644 index 00000000000..f6341e30a1e --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-reg-aliases.c @@ -0,0 +1,22 @@ +/* This file is part of GDB, the GNU debugger. + + Copyright 2018 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +int +main () +{ + return 0; +} diff --git a/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp b/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp new file mode 100644 index 00000000000..0b54723d8bc --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-reg-aliases.exp @@ -0,0 +1,130 @@ +# Copyright 2018 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +if {![istarget "riscv*-*-*"]} { + verbose "Skipping ${gdb_test_file_name}." + return +} + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +if ![runto_main] then { + fail "can't run to main" + return 0 +} + +# A list, each entry is itself a list, the first item being the +# primary name of a register (the name GDB uses by default), and the +# second entry being a list of register aliases. +set register_names \ +{ { ra {x1} } { sp {x2} } { gp {x3} } { tp {x4} } { t0 {x5} } \ + { t1 {x6} } { t2 {x7} } { fp {x8 s0} } { s1 {x9} } { a0 {x10} } \ + { a1 {x11} } { a2 {x12} } { a3 {x13} } { a4 {x14} } { a5 {x15} } \ + { a6 {x16} } { a7 {x17} } { s2 {x18} } { s3 {x19} } { s4 {x20} } \ + { s5 {x21} } { s6 {x22} } { s7 {x23} } { s8 {x24} } { s9 {x25} } \ + { s10 {x26} } { s11 {x27} } { t3 {x28} } { t4 {x29} } { t5 {x30} } \ + { t6 {x31} } { ft0 {f0} } { ft1 {f1} } { ft2 {f2} } { ft3 {f3} } \ + { ft4 {f4} } { ft5 {f5} } { ft6 {f6} } { ft7 {f7} } { fs0 {f8} } \ + { fs1 {f9} } { fa0 {f10} } { fa1 {f11} } { fa2 {f12} } { fa3 {f13} } \ + { fa4 {f14} } { fa5 {f15} } { fa6 {f16} } { fa7 {f17} } { fs2 {f18} } \ + { fs3 {f19} } { fs4 {f20} } { fs5 {f21} } { fs6 {f22} } { fs7 {f23} } \ + { fs8 {f24} } { fs9 {f25} } { fs10 {f26} } { fs11 {f27} } { ft8 {f28} } \ + { ft9 {f29} } { ft10 {f30} } { ft11 {f31} } } + +# Check that the zero register (and its x0 alias) both contain the +# value 0. + +proc check_zero_register_value {testname} { + gdb_test "p/d \$zero" " = 0" "check \$zero: ${testname}" + gdb_test "p/d \$x0" " = 0" "check \$x0: ${testname}" +} + +# First, some testing of the zero register. This register should +# always read as zero, and should swallow any attempt to write a +# non-zero value to the register. + +check_zero_register_value "before any writes" + +gdb_test_no_output "set \$zero = 123" \ + "write to the \$zero register" + +check_zero_register_value "after write to \$zero" + +gdb_test_no_output "set \$x0 = 123" \ + "write to the \$x0 register" + +check_zero_register_value "after write to \$x0" + +# Set all of the general registers to zero. Confirm that the value of +# zero can be read back from the primary name, and from all of the +# alias names. + +foreach reg_desc ${register_names} { + set primary_name [lindex ${reg_desc} 0] + set alias_names [lindex ${reg_desc} 1] + + gdb_test_no_output "set \$${primary_name} = 0" \ + "set register ${primary_name} to an initial value of zero" + gdb_test "p/d \$${primary_name}" " = 0" \ + "check the initial value of ${primary_name} is now zero" + + foreach reg_alias ${alias_names} { + gdb_test "p/d \$${reg_alias}" " = 0" \ + "check the initial value of ${reg_alias} is now zero" + } +} + +# Set each register in turn to a new value, and confirm that the new +# value can be read back from the primary name, and from all of the +# alias names. + +set reg_value 100 +foreach reg_desc ${register_names} { + set primary_name [lindex ${reg_desc} 0] + set alias_names [lindex ${reg_desc} 1] + + # Set value through the primary register name, and check that all + # the aliases see the same value. + set reg_value [incr reg_value] + gdb_test_no_output "set \$${primary_name} = $reg_value" \ + "write non-zero value to ${primary_name}" + gdb_test "p/d \$${primary_name}" " = $reg_value" \ + "read ${primary_name} after non-zero write to ${primary_name}" + foreach reg_alias ${alias_names} { + gdb_test "p/d \$${reg_alias}" " = $reg_value" \ + "read ${reg_alias} after non-zero write to ${primary_name}" + } + + # For each alias, set a new value, and check that the primary + # register name, and all the other aliases, see the new value. + foreach reg_alias ${alias_names} { + set reg_value [incr reg_value] + + gdb_test_no_output "set \$${reg_alias} = $reg_value" \ + "write non-zero value to ${reg_alias}" + + gdb_test "p/d \$${primary_name}" " = $reg_value" \ + "read ${primary_name} after non-zero write to ${reg_alias}" + + foreach other_reg_alias ${alias_names} { + gdb_test "p/d \$${other_reg_alias}" " = $reg_value" \ + "read ${other_reg_alias} after non-zero write to ${reg_alias}" + } + } +}