From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115722 invoked by alias); 17 Jul 2018 15:37:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 115626 invoked by uid 89); 17 Jul 2018 15:37:08 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=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=eclass, pc, Functions X-HELO: mail-wm0-f68.google.com Received: from mail-wm0-f68.google.com (HELO mail-wm0-f68.google.com) (74.125.82.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Jul 2018 15:37:06 +0000 Received: by mail-wm0-f68.google.com with SMTP id v25-v6so1977005wmc.0 for ; Tue, 17 Jul 2018 08:37:05 -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=G2USd47kNsVujN4ZgHUAbnNqQdNPodjzyDxNyw7tY+A=; b=D2bM0vRXmO9Oxuzkoe3KG+6VXhHRpo7tkTs0Aca+oM9xCxBXIkDMoILRDAIxsE+iC/ X/OQiZhCSpiIrlgC92RjKcuFjNwK+MgXImRAHxCNjnZaOkU+d7uNCK12MZiKsDuwlyvH ZvQ9Fp2lZvVKCNhzmJ9GuodtOy+wzmRU/i9evREUkRoIFRg6YzGiAOtF2VwmgkJa0iXx nte9PbgicveF9mVughgQ8AHpkDXToFegoyA5HC1/EJo8pzLxh0ZwoALInnI1Wxy5zlVd bD78WSgHhu5IVZxwaBlhvEGysCMHeqSRyBVW95yigkHotD7Yznjm3N7O6vUGe3kcJUMb SsaA== Return-Path: Received: from localhost (host86-162-243-217.range86-162.btcentralplus.com. [86.162.243.217]) by smtp.gmail.com with ESMTPSA id w204-v6sm1902114wmw.17.2018.07.17.08.37.03 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 17 Jul 2018 08:37:03 -0700 (PDT) Date: Tue, 17 Jul 2018 15:37:00 -0000 From: Andrew Burgess To: Jim Wilson Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] RISC-V: Don't decrement pc after break. Message-ID: <20180717153702.GG2888@embecosm.com> References: <20180717001241.25908-1-jimw@sifive.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180717001241.25908-1-jimw@sifive.com> X-Fortune: In order to get a loan you must first prove you don't need it. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-07/txt/msg00518.txt.bz2 * Jim Wilson [2018-07-16 17:12:41 -0700]: > This is the gdb patch for the RISC-V Linux kernel patch I just submitted. > https://patchwork.kernel.org/patch/10524925/ > This removes the code that decrements the pc after a break, now that we have > a patch to stop linux from pointlessly adding to the pc after a break. It > would be nice if this goes in now, to avoid unnecessary divergence between > gdb and the linux kernel. Palmer, the RISC-V linux kernel maintainer, has > already agreed to accept that patch. > > This will also be needed by the FreeBSD gdb port which may be started soon. > > This also fixes a bug in the code. The fact that we have two different > mechanisms to decide breakpoint size, used_compressed_breakpoints and > has_compressed_isa, means that it is possible for gdb to emit a 4 byte > breakpoint and then subtract 2 from the pc, and vice versa. Removing the > unnecessary pc decrement fixes that problem. > > OK? > > Jim > > gdb/ > * riscv-tdep.c (riscv_has_feature): Delete comment that refers to > set_gdbarch_decr_pc_after_break. Call riscv_read_misa_reg always. > (riscv_gdbarch_init): Delete local has_compressed_isa. Delete now > unecessary braces after EF_RISCV_RVC test. Delete call to > set_gdbarch_decr_pc_after_break. This looks good to me. Thanks, Andrew > --- > gdb/riscv-tdep.c | 25 ++----------------------- > 1 file changed, 2 insertions(+), 23 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 72dab0f897..f5d1af822c 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -335,23 +335,7 @@ riscv_has_feature (struct gdbarch *gdbarch, char feature) > > gdb_assert (feature >= 'A' && feature <= 'Z'); > > - /* It would be nice to always check with the real target where possible, > - however, for compressed instructions this is a bad idea. > - > - The call to `set_gdbarch_decr_pc_after_break' is made just once per > - GDBARCH and we decide at that point if we should decrement by 2 or 4 > - bytes based on whether the BFD has compressed instruction support or > - not. > - > - If the BFD was not compiled with compressed instruction support, but we > - are running on a target with compressed instructions then we might > - place a 4-byte breakpoint, then decrement the $pc by 2 bytes leading to > - confusion. > - > - It's safer if we just make decisions about compressed instruction > - support based on the BFD. */ > - if (feature != 'C') > - misa = riscv_read_misa_reg (&have_read_misa); > + misa = riscv_read_misa_reg (&have_read_misa); > if (!have_read_misa || misa == 0) > misa = gdbarch_tdep (gdbarch)->core_features; > > @@ -2440,7 +2424,6 @@ riscv_gdbarch_init (struct gdbarch_info info, > struct gdbarch *gdbarch; > struct gdbarch_tdep *tdep; > struct gdbarch_tdep tmp_tdep; > - bool has_compressed_isa = false; > int i; > > /* Ideally, we'd like to get as much information from the target for > @@ -2472,10 +2455,7 @@ riscv_gdbarch_init (struct gdbarch_info info, > _("unknown ELF header class %d"), eclass); > > if (e_flags & EF_RISCV_RVC) > - { > - has_compressed_isa = true; > - tmp_tdep.core_features |= (1 << ('C' - 'A')); > - } > + tmp_tdep.core_features |= (1 << ('C' - 'A')); > > if (e_flags & EF_RISCV_FLOAT_ABI_DOUBLE) > { > @@ -2545,7 +2525,6 @@ riscv_gdbarch_init (struct gdbarch_info info, > set_gdbarch_register_reggroup_p (gdbarch, riscv_register_reggroup_p); > > /* Functions to analyze frames. */ > - set_gdbarch_decr_pc_after_break (gdbarch, (has_compressed_isa ? 2 : 4)); > set_gdbarch_skip_prologue (gdbarch, riscv_skip_prologue); > set_gdbarch_inner_than (gdbarch, core_addr_lessthan); > set_gdbarch_frame_align (gdbarch, riscv_frame_align); > -- > 2.17.1 >