From patchwork Fri Oct 26 07:16: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: 29901 Received: (qmail 39606 invoked by alias); 26 Oct 2018 07:17:08 -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 39566 invoked by uid 89); 26 Oct 2018 07:17:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:209.85.128.67, Hx-spam-relays-external:209.85.128.67 X-HELO: mail-wm1-f67.google.com Received: from mail-wm1-f67.google.com (HELO mail-wm1-f67.google.com) (209.85.128.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 26 Oct 2018 07:17:03 +0000 Received: by mail-wm1-f67.google.com with SMTP id s10-v6so395440wmc.5 for ; Fri, 26 Oct 2018 00:17:02 -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:user-agent; bh=dhcBb3jPEOENOJUZj7JZ+4UMmDT6TocCds8fmOxsQEk=; b=btKaGR3LeIxWQ1V2GvFTQwks5I/73tpqCW9ARv3ACsB7EudJgf3T/X1hN3iKcYj9RM J4arTj8yxsP8ajoZ+btuLEm4b9MlHJ6QCqAHqMMkN2UPFFYIFBGyNiYbgiAPCTeR5+hn l2AZK0WuSWrfQ3I+U+zidBXoJNM4eKXTGgNkjjEOqMrTgkrM8VKk0b1DSSAfZi/+TonP rVR/cUURTlImgzex6xfJwxrysgBGtyktaeiwd+Uad3xrqTNcUiPjrINaXwCe0r8Bvu+r NLIuQWJ/zcEIxICyEoaR387gpMQabi6KbOiAyofej26bA9r0+HS2eTOvT/Wk+wVOObQa 5l+A== Return-Path: Received: from localhost (host81-148-252-35.range81-148.btcentralplus.com. [81.148.252.35]) by smtp.gmail.com with ESMTPSA id 4-v6sm2337222wmt.16.2018.10.26.00.16.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 26 Oct 2018 00:16:59 -0700 (PDT) Date: Fri, 26 Oct 2018 08:16:57 +0100 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: jimw@sifive.com, John Baldwin Subject: Re: [PATCH] gdb/riscv: Remove redundant code, and catch more errors when accessing MISA Message-ID: <20181026071657.GP2929@embecosm.com> References: <20181023112631.9722-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181023112631.9722-1-andrew.burgess@embecosm.com> X-Fortune: Someone else stole your IP address, call the Internet detectives! X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes Thanks for the reviews. I ended up pushing the patch version below. With some additional testing I realised that I was always reading both locations for the MISA register. The problem turned out to be that `break;` within a TRY { ... } block doesn't work (the TRY macro adds a do { ... } while (0) block). The new patch moves the early loop termination out of the TRY block. Thanks, Andrew --- [PATCH] gdb/riscv: Remove redundant code, and catch more errors when accessing MISA When reading the MISA register, the RISC-V specification says that, if MISA can't be found then a default value of 0 should be assumed. As such, this patch ensures that GDB ignores errors when accessing both the new and old locations for the MISA register. Additionally, this patch removes an unneeded flag parameter which didn't provide any additional functionality beyond checking the MISA for the default value of 0. gdb/ChangeLog: * riscv-tdep.c (riscv_read_misa_reg): Update comment, remove READ_P parameter, catch and ignore register access errors from either the old or new MISA location. (riscv_has_feature): Update call to riscv_read_misa_reg. --- gdb/ChangeLog | 7 +++++++ gdb/riscv-tdep.c | 51 +++++++++++++++++++++++++++++---------------------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index f02420dfe52..e4b35a01ffc 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -290,34 +290,44 @@ static unsigned int riscv_debug_infcall = 0; static unsigned int riscv_debug_unwinder = 0; -/* Read the MISA register from the target. The register will only be read - once, and the value read will be cached. If the register can't be read - from the target then a default value (0) will be returned. If the - pointer READ_P is not null, then the bool pointed to is updated to - indicate if the value returned was read from the target (true) or is the - default (false). */ +/* Read the MISA register from the target. There are a couple of locations + that the register might be found, these are all tried. If the MISA + register can't be found at all then the default value of 0 is returned, + this is inline with the RISC-V specification. */ static uint32_t -riscv_read_misa_reg (bool *read_p) +riscv_read_misa_reg () { uint32_t value = 0; if (target_has_registers) { + /* Old cores might have MISA located at a different offset. */ + static int misa_regs[] = + { RISCV_CSR_MISA_REGNUM, RISCV_CSR_LEGACY_MISA_REGNUM }; + struct frame_info *frame = get_current_frame (); - TRY - { - value = get_frame_register_unsigned (frame, - RISCV_CSR_MISA_REGNUM); - } - CATCH (ex, RETURN_MASK_ERROR) + for (int i = 0; i < ARRAY_SIZE (misa_regs); ++i) { - /* Old cores might have MISA located at a different offset. */ - value = get_frame_register_unsigned (frame, - RISCV_CSR_LEGACY_MISA_REGNUM); + bool success = false; + + TRY + { + value = get_frame_register_unsigned (frame, misa_regs[i]); + success = true; + } + CATCH (ex, RETURN_MASK_ERROR) + { + /* Ignore errors, it is acceptable for a target to not + provide a MISA register, in which case the default value + of 0 should be used. */ + } + END_CATCH + + if (success) + break; } - END_CATCH } return value; @@ -330,13 +340,10 @@ riscv_read_misa_reg (bool *read_p) static bool riscv_has_feature (struct gdbarch *gdbarch, char feature) { - bool have_read_misa = false; - uint32_t misa; - gdb_assert (feature >= 'A' && feature <= 'Z'); - misa = riscv_read_misa_reg (&have_read_misa); - if (!have_read_misa || misa == 0) + uint32_t misa = riscv_read_misa_reg (); + if (misa == 0) misa = gdbarch_tdep (gdbarch)->core_features; return (misa & (1 << (feature - 'A'))) != 0;