From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by sourceware.org (Postfix) with ESMTPS id 682C6386F477 for ; Tue, 9 Jun 2020 17:30:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 682C6386F477 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-x434.google.com with SMTP id x13so22253784wrv.4 for ; Tue, 09 Jun 2020 10:30:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Kub31Pj96eDQJPVxScbLOtEs3/206jYTW66YKbdyPLI=; b=MXQEFRtNhLKb61D2rj+nf5C/J+iWz/iHuCeBZCXomrU0r2XI7VXcXqbGPydxalPmpv 2wMgcEqbNmhmyMT8he0v+cr04g/0kBdt4w2+oJl3QNjGh7sL4zrqn7oiVO3GRUf8+Nh0 t6nYS8uUxqveUt/roSg8F1ivzGAdGLIcFetDez7Wtfns3nnr54DgKNk8mD/PLH/eINUt d6GwqepAHlxnuoSVEyF1DE5yyCyjy3faTlZvERO1UPNhPzE79v5AHt4GwjdodElZalH1 xYUZ35RO61aOJyw0ibb2HDzy6RhlvXowSNzZRw1jQktSQTNcYJpLISCAAmygXW2J5SJV Ad5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Kub31Pj96eDQJPVxScbLOtEs3/206jYTW66YKbdyPLI=; b=X2+tGLng8dqMpv++RQQ/zgTzaCg7e1VKDrO26tAJQo/EUgTdWhvREBsTAoEWauGkzn UXXhJim3T+Jc2oZJslRkZTMPoNZ6GlcKCrIf7R2nx3AFxP4Ih4rh35gsFRBI2Zb/z6+Q NMKUkMDrPWp2k+43q8FwRiuIYkGevyxtuXcu2KJwiaJJo2BxBq50ILrjy1sM/xOvahqn fNjtMBfp+20wvYJV7eJSyNwjBU7usZEkqgnwjtnbLwv7tcAi+qtFLH5pkF+i5dGOSHaG +wBYhoI149l6sj6+fvu+oG3FsJ5KEPofJTPm9jCFju6aLNHI/lrXuKz50VNIYSNX9bHP j1Xw== X-Gm-Message-State: AOAM532n8bqsytekQbqRQ7UiA4gzwCIqMUiF7uhBZcM2hVtnR9ZxT9zi ljRN0kk4huI1NMdir+3XrWp1rA== X-Google-Smtp-Source: ABdhPJxY8FeUVaJXG1zwDRYvVeOILXdd2tFnDZ5BNNDxcmC+4F0vqWdDOaSk6HFP119jSLPkakVODQ== X-Received: by 2002:a5d:5483:: with SMTP id h3mr5858267wrv.10.1591723843277; Tue, 09 Jun 2020 10:30:43 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id x205sm3736103wmx.21.2020.06.09.10.30.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jun 2020 10:30:42 -0700 (PDT) Date: Tue, 9 Jun 2020 18:30:40 +0100 From: Andrew Burgess To: Tom Tromey Cc: Nelson Chu , gdb-patches@sourceware.org Subject: [RFC] gdb/riscv: Improved register alias name creation Message-ID: <20200609173040.GE2737@embecosm.com> References: <1584007257-14466-1-git-send-email-nelson.chu@sifive.com> <87r1upefg8.fsf@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87r1upefg8.fsf@tromey.com> X-Operating-System: Linux/5.6.15-200.fc31.x86_64 (x86_64) X-Uptime: 18:27:30 up 1 day, 7:34, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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, 09 Jun 2020 17:30:47 -0000 The patch below has had only minimal testing so far, but I'm sharing it here so it can be discussed while I also get it tested. This changes how the 'csr%d' aliases are created, and adds the dscratch to dscratch0 alias. Feedback and comments welcome. Thanks, Andrew --- 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/ChangeLog | 12 ++++++ gdb/riscv-tdep.c | 97 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 0842dcbcb23..d1c8949290a 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,17 @@ 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. Add any aliases for this register onto the list + of pending aliases. */ + for (const char *alias : reg.names) + { + if (alias != name) + aliases->emplace_back (alias, (void *)®.regnum); + } + break; + } } if (!found && reg.required_p) @@ -2993,24 +3045,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 +3148,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 +3173,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 +3210,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 +3354,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); -- 2.25.4