From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id 8CC8A395383D for ; Thu, 11 Jun 2020 13:16:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8CC8A395383D 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-x42e.google.com with SMTP id r7so6153873wro.1 for ; Thu, 11 Jun 2020 06:16:22 -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=QzAUx9UrB75AkIgoJQSb5m0NuplJk+80LHBIvXgDDSM=; b=fgzRjolozUva3CPNnDqOPRsd+WZI7CaufaMnRQr2NobcyTXuqzbJIV2blgkw4Ctc6J f6tzNWbTQHHxmvIlhIKP4ObjAH98acToqpQF4MK0E7gqMVbuciCiIi8Vdxd/yFmrPDoN Sqw4+/fGJhNw+CSLlbh3u4t4s2OOkrS9acww3+fjnf3E5zoaT2TVGCGNUpk3PddthrxR uvbQHMYg9ChuCVy7k4zPM1WOu/yc/K6AILAX985e1jccGcXM8mEOQyHljUxbs33P4Ue3 MrKZN7LpN18gEalbsqlibx3wzD2/U5qf4ftPFdTdXuNTTdQKjCVAtha+mLsifHGlJvT1 sdWg== 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=QzAUx9UrB75AkIgoJQSb5m0NuplJk+80LHBIvXgDDSM=; b=uUnUpYBtQdsP8qSwayooFSTTgC9f3Dj8aCyV5mOM11FXmClSsFsNWFJ+fVXl3iJepn WylUOYd5SbikEnIggZe3HR8e6m0Vma2maHcjfam/oQBt9HzjdC0IFhsuSSUA6iZPhGot 7F13++eHOM8XQraCvWDBY/5HeFbnJ6KAxHEq6HaVFUb2YTAo03ENP2HvcrQg7OI4aqSw tu0io5TRRKoLf8ik1MYCgIgDAMvwQSb6WvLGN0xndwZkpIlLrBW3kZD8TKX8nmS/HNDo OKdaj5WaidmsjbPWpArq0owRB5DnUElzzGEyXlFIVh/dNgCriN4V2uDJO17t0xRn6Q/Y Azlg== X-Gm-Message-State: AOAM532MRTS+/dvDa6+W12jDso2tHCmsL/b7afpKyLOJmQ6Z99ecti2F Hje1wmjkQbcUS7iCUDs7wRk2bLbfI/M= X-Google-Smtp-Source: ABdhPJw3Ui480xbVajv3Rxl0GEVO6c1vhCrozJY+vA89mxvXKNwpK+RnDQzf2BcdBTNl+IzczmbpLQ== X-Received: by 2002:adf:db09:: with SMTP id s9mr9327961wri.256.1591881381183; Thu, 11 Jun 2020 06:16:21 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id e10sm4979943wrn.11.2020.06.11.06.16.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jun 2020 06:16:20 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCH 1/2] gdb/riscv: Improved register alias name creation Date: Thu, 11 Jun 2020 14:16:14 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: <87d0673rmx.fsf@tromey.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.4 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: Thu, 11 Jun 2020 13:16:24 -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/ChangeLog | 12 ++++++ gdb/riscv-tdep.c | 95 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 78 insertions(+), 29 deletions(-) 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); -- 2.25.4