From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by sourceware.org (Postfix) with ESMTPS id 4F1FC388E825 for ; Tue, 16 Jun 2020 17:14:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4F1FC388E825 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x430.google.com with SMTP id h5so21610073wrc.7 for ; Tue, 16 Jun 2020 10:14:55 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=/VM4rRBFYlHvL/9OYlGG1JaDMCDFG4ayOkj88mAK3wM=; b=erZ7z+PEGnzD1Rk9XDvmQdqEwTmdXuRVa6Jjr3gqZsjE4e8sf9Yh+I3wT4VZ3OvkEj tAmIq1ECznSqf11lEhITFLhp0FsvueVDmaBSezE/6Bgke0N8bTQoAnVaNRL/Us4kjrtu zRSi0SbvVrOph9XAJvEYFNjugCm+HWzgK1ni0aMpu6R8ljMwGU8MJWyOBCUYF0ZMFLtE F9Zy5WZHwFtRKrAAAeLQnxW01G8rd9qzrTvmFRnzimxowFlC8FkCELHN1qyr44H9jDtQ OS5bWjkHQIfSXC1MfLh8YT7El6I+ZNAgvlno4EM0U7UZ96U72EVVY/gE74F0/LKKiQkW pWkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=/VM4rRBFYlHvL/9OYlGG1JaDMCDFG4ayOkj88mAK3wM=; b=k956YWG5Y18LZ8+ImEHQt4L2up9cQPWNEPwSqLXgAxryGnlGTAz+yHGEhNy9DEwDEA 3j4cvmdN1e5Mr5LSORldJx1Jducnb/p1heC8RcU2Xj2NjRc6hkgjucZk/I9wzaOWgXPw CIj5+Ne2bxRHqJUlClhM6CyQ2HIgtL/4C8CubIPv1EZZHtPyhoFomohyCMxkfHhr7Ev0 fTaMfjSKPcW2dltWAClwJ9z/Rkt/UlyL8iCfm7QhP9N8NyrODFdk3Hd2CJuQURkWcumg jfegc4uwKnxtE3PFb17a9MkC51C1DOvBtcGZJzaJFGRWMzq7FI3cgfyWPGi5e0emKE/n NANg== X-Gm-Message-State: AOAM531Mb6mZ1hUp1baLH0dzWkX4HYtjE5psqhJSOk4UyTB38PGOgkj5 LeTKMx1VNA7cwODQsY0z6EH2bc6+jOU= X-Google-Smtp-Source: ABdhPJyvm2dS4hAJ5+GSb9hFEY+/B76TsOnYVpfs7x+bGmeU/wQ27FXxPd7lyrtE2eiJprcCiNnbXQ== X-Received: by 2002:adf:dcc3:: with SMTP id x3mr3984445wrm.93.1592327693409; Tue, 16 Jun 2020 10:14:53 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id o20sm31455403wra.29.2020.06.16.10.14.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Jun 2020 10:14:52 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Nelson Chu , Jim Wilson , Tom Tromey , palmer@dabbelt.com, Andrew Burgess Subject: [PATCH 1/8] gdb/riscv: Improved register alias name creation Date: Tue, 16 Jun 2020 18:14:40 +0100 Message-Id: <5c39d5cd17980532f1394b53ac33611e971b5dd5.1592327296.git.andrew.burgess@embecosm.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jun 2020 17:14:58 -0000 This commit does two things: 1. Makes use of the DECLARE_CSR_ALIAS definitions in riscv-opc.h to add additional aliases for CSRs. 2. Only creates aliases for registers that are actually present on the target (as announced in the target XML description). This means that the 'csr%d' aliases that exist will only be created for those CSRs the target actually has, which is a nice improvement, as accessing one of the CSRs that didn't exist would cause GDB to crash with this error: valprint.c:1560: internal-error: bool maybe_negate_by_bytes(const gdb_byte*, unsigned int, bfd_endian, gdb::byte_vector*): Assertion `len > 0' failed. When we look at the DECLARE_CSR_ALIAS lines in riscv-opc.h, these can be split into three groups: DECLARE_CSR_ALIAS(misa, 0xf10, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P9P1) The 'misa' register used to exist of offset 0xf10, but was moved to its current offset (0x301) in with privilege spec 1.9.1. We don't want GDB to create an alias called 'misa' as we will already have a 'misa' register created by the DECLARE_CSR(misa ....) call earlier in riscv-opc.h DECLARE_CSR_ALIAS(ubadaddr, CSR_UTVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10) DECLARE_CSR_ALIAS(sbadaddr, CSR_STVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10) DECLARE_CSR_ALIAS(sptbr, CSR_SATP, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10) DECLARE_CSR_ALIAS(mbadaddr, CSR_MTVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10) DECLARE_CSR_ALIAS(mucounteren, CSR_MCOUNTINHIBIT, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10) These aliases are all CSRs that were removed in privilege spec 1.10, and whose addresses were reused by new CSRs. The names meaning of the old names is totally different to the new CSRs that have taken their place. I don't believe we should add these as aliases into GDB. If the new CSR exists in the target then that should be enough. DECLARE_CSR_ALIAS(dscratch, CSR_DSCRATCH0, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P11) In privilege spec 1.11 the 'dscratch' register was renamed to 'dscratch0', however the meaning of the register didn't change. Adding the 'dscratch' alias makes sense I think. Looking then at the final PRIV_SPEC_CLASS_* field for each alias then we can see that currently we only want to take the alias from PRIV_SPEC_CLASS_1P11. For now then this is what I'm using to filter the aliases within GDB. In the future there's no telling how DECLARE_CSR_ALIAS will be used. I've heard it said that future RISC-V privilege specs will not reuse CSR offsets again. But it could happen. We just don't know. If / when it does we may need to revisit how aliases are created for GDB, but for now this seems to be OK. gdb/ChangeLog: * riscv-tdep.c (riscv_create_csr_aliases): Handle csr aliases from riscv-opc.h. (class riscv_pending_register_alias): New class. (riscv_check_tdesc_feature): Take vector of pending aliases and populate it as appropriate. (riscv_setup_register_aliases): Delete. (riscv_gdbarch_init): Create vector of pending aliases and pass it to riscv_check_tdesc_feature in all cases. Use the vector to create the register aliases. gdb/testsuite/ChangeLog: * gdb.arch/riscv-tdesc-regs-32.xml: New file. * gdb.arch/riscv-tdesc-regs-64.xml: New file. * gdb.arch/riscv-tdesc-regs.c: New file. * gdb.arch/riscv-tdesc-regs.exp: New file. --- gdb/ChangeLog | 12 +++ gdb/riscv-tdep.c | 95 +++++++++++++------ gdb/testsuite/ChangeLog | 7 ++ .../gdb.arch/riscv-tdesc-regs-32.xml | 89 +++++++++++++++++ .../gdb.arch/riscv-tdesc-regs-64.xml | 93 ++++++++++++++++++ gdb/testsuite/gdb.arch/riscv-tdesc-regs.c | 22 +++++ gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp | 81 ++++++++++++++++ 7 files changed, 370 insertions(+), 29 deletions(-) create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs.c create mode 100644 gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index fa43c8d02f2..64ec9b4a257 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -258,6 +258,16 @@ riscv_create_csr_aliases () int csr_num = reg.regnum - RISCV_FIRST_CSR_REGNUM; const char *alias = xstrprintf ("csr%d", csr_num); reg.names.push_back (alias); + + /* Setup the other csr aliases. We don't use a switch table here in + case there are multiple aliases with the same value. Also filter + based on ABRT_VER in order to avoid a very old alias for misa that + duplicates the name "misa" but at a different CSR address. */ +#define DECLARE_CSR_ALIAS(NAME,VALUE,CLASS,DEF_VER,ABRT_VER) \ + if (csr_num == VALUE && ABRT_VER >= PRIV_SPEC_CLASS_1P11) \ + reg.names.push_back ( # NAME ); +#include "opcode/riscv-opc.h" +#undef DECLARE_CSR_ALIAS } } @@ -2945,6 +2955,37 @@ riscv_find_default_target_description (const struct gdbarch_info info) return riscv_lookup_target_description (features); } +/* Information about a register alias that needs to be set up for this + target. These are collected when the target's XML description is + analysed, and then processed later, once the gdbarch has been created. */ + +class riscv_pending_register_alias +{ +public: + /* Constructor. */ + + riscv_pending_register_alias (const char *name, const void *baton) + : m_name (name), + m_baton (baton) + { /* Nothing. */ } + + /* Convert this into a user register for GDBARCH. */ + + void create (struct gdbarch *gdbarch) const + { + user_reg_add (gdbarch, m_name, value_of_riscv_user_reg, m_baton); + } + +private: + /* The name for this alias. */ + const char *m_name; + + /* The baton value for passing to user_reg_add. This must point to some + data that will live for at least as long as the gdbarch object to + which the user register is attached. */ + const void *m_baton; +}; + /* All of the registers in REG_SET are checked for in FEATURE, TDESC_DATA is updated with the register numbers for each register as listed in REG_SET. If any register marked as required in REG_SET is not found in @@ -2953,7 +2994,8 @@ riscv_find_default_target_description (const struct gdbarch_info info) static bool riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data, const struct tdesc_feature *feature, - const struct riscv_register_feature *reg_set) + const struct riscv_register_feature *reg_set, + std::vector *aliases) { for (const auto ® : reg_set->registers) { @@ -2965,7 +3007,15 @@ riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data, tdesc_numbered_register (feature, tdesc_data, reg.regnum, name); if (found) - break; + { + /* We know that the target description mentions this + register. In RISCV_REGISTER_NAME we ensure that GDB + always uses the first name for each register, so here we + add aliases for all of the remaining names. */ + for (int i = 0; i < reg.names.size (); ++i) + aliases->emplace_back (reg.names[i], (void *)®.regnum); + break; + } } if (!found && reg.required_p) @@ -2993,24 +3043,6 @@ riscv_add_reggroups (struct gdbarch *gdbarch) reggroup_add (gdbarch, csr_reggroup); } -/* Create register aliases for all the alternative names that exist for - registers in REG_SET. */ - -static void -riscv_setup_register_aliases (struct gdbarch *gdbarch, - const struct riscv_register_feature *reg_set) -{ - for (auto ® : reg_set->registers) - { - /* The first item in the names list is the preferred name for the - register, this is what RISCV_REGISTER_NAME returns, and so we - don't need to create an alias with that name here. */ - for (int i = 1; i < reg.names.size (); ++i) - user_reg_add (gdbarch, reg.names[i], value_of_riscv_user_reg, - ®.regnum); - } -} - /* Implement the "dwarf2_reg_to_regnum" gdbarch method. */ static int @@ -3114,10 +3146,12 @@ riscv_gdbarch_init (struct gdbarch_info info, return NULL; struct tdesc_arch_data *tdesc_data = tdesc_data_alloc (); + std::vector pending_aliases; bool valid_p = riscv_check_tdesc_feature (tdesc_data, feature_cpu, - &riscv_xreg_feature); + &riscv_xreg_feature, + &pending_aliases); if (valid_p) { /* Check that all of the core cpu registers have the same bitsize. */ @@ -3137,7 +3171,8 @@ riscv_gdbarch_init (struct gdbarch_info info, if (feature_fpu != NULL) { valid_p &= riscv_check_tdesc_feature (tdesc_data, feature_fpu, - &riscv_freg_feature); + &riscv_freg_feature, + &pending_aliases); /* Search for the first floating point register (by any alias), to determine the bitsize. */ @@ -3173,11 +3208,13 @@ riscv_gdbarch_init (struct gdbarch_info info, if (feature_virtual) riscv_check_tdesc_feature (tdesc_data, feature_virtual, - &riscv_virtual_feature); + &riscv_virtual_feature, + &pending_aliases); if (feature_csr) riscv_check_tdesc_feature (tdesc_data, feature_csr, - &riscv_csr_feature); + &riscv_csr_feature, + &pending_aliases); if (!valid_p) { @@ -3315,11 +3352,11 @@ riscv_gdbarch_init (struct gdbarch_info info, want, ignoring what the target tells us. */ set_gdbarch_register_reggroup_p (gdbarch, riscv_register_reggroup_p); - /* Create register aliases for alternative register names. */ - riscv_setup_register_aliases (gdbarch, &riscv_xreg_feature); - if (riscv_has_fp_regs (gdbarch)) - riscv_setup_register_aliases (gdbarch, &riscv_freg_feature); - riscv_setup_register_aliases (gdbarch, &riscv_csr_feature); + /* Create register aliases for alternative register names. We only + create aliases for registers which were mentioned in the target + description. */ + for (const auto &alias : pending_aliases) + alias.create (gdbarch); /* Compile command hooks. */ set_gdbarch_gcc_target_options (gdbarch, riscv_gcc_target_options); diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml new file mode 100644 index 00000000000..90406a0c676 --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-32.xml @@ -0,0 +1,89 @@ + + + + riscv + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml new file mode 100644 index 00000000000..b2422aeb866 --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs-64.xml @@ -0,0 +1,93 @@ + + + + riscv + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/gdb/testsuite/gdb.arch/riscv-tdesc-regs.c b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.c new file mode 100644 index 00000000000..f4825c8a7c1 --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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-tdesc-regs.exp b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp new file mode 100644 index 00000000000..46e64d62a2b --- /dev/null +++ b/gdb/testsuite/gdb.arch/riscv-tdesc-regs.exp @@ -0,0 +1,81 @@ +# Copyright 2020 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 . + +# Various tests to check which register names are available after +# loading a new target description file. + +if {![istarget "riscv*-*-*"]} { + verbose "Skipping ${gdb_test_file_name}." + return +} + +standard_testfile + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ + {debug quiet}] } { + unsupported "failed to compile" + return -1 +} + +if { ![runto_main] } { + untested "failed to runto main" + return -1 +} + +# First, figure out if we are 32-bit or 64-bit. +set xlen [get_valueof "/d" "sizeof (\$a0)" 0] +set flen [get_valueof "/d" "sizeof (\$fa0)" 0] + +gdb_assert { $xlen != 0 && $flen != 0 } "read xlen and flen" + +# We only handle 32-bit or 64-bit x-registers. +if { $xlen != 4 && $xlen != 8 } { + unsupported "unknown x-register size" + return -1 +} + +# If FLEN is 1 then the target doesn't have floating point support +# (the register $fa0 was not recognised). Otherwise, we can only +# proceed if FLEN equals XLEN, otherwise we'd need more test XML +# files. +if { $flen != 1 && $flen != $xlen } { + unsupport "unknown xlen/flen combination" + return -1 +} + +if { $xlen == 4 } { + set xml_tdesc "riscv-tdesc-regs-32.xml" +} else { + set xml_tdesc "riscv-tdesc-regs-64.xml" +} +set xml_tdesc "${srcdir}/${subdir}/${xml_tdesc}" + +# Maybe copy the target over if we're remote testing. +if {[is_remote host]} { + set remote_file [remote_download host $xml_tdesc] +} else { + set remote_file $xml_tdesc +} + +gdb_test_no_output "set tdesc filename $remote_file" \ + "load the new target description" + +# Check that an alias for an unknown CSR will give a suitable error. +gdb_test "info registers \$csr0" "Invalid register `csr0'" + +# Check we can access the dscratch register using either of its names. +gdb_test "info registers \$dscratch0" "dscratch0\[ \t\]+.*" +gdb_test "info registers \$dscratch" "dscratch\[ \t\]+.*" + -- 2.25.4