From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26291 invoked by alias); 1 Nov 2018 22:05:15 -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 26276 invoked by uid 89); 1 Nov 2018 22:05:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=HContent-Transfer-Encoding:8bit X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Nov 2018 22:05:12 +0000 Received: from John-Baldwins-MacBook-Pro-2.local (ralph.baldwin.cx [66.234.199.215]) by mail.baldwin.cx (Postfix) with ESMTPSA id C368F10AFCD; Thu, 1 Nov 2018 18:05:09 -0400 (EDT) Subject: Re: [PATCH] RISC-V: Don't allow unaligned breakpoints. To: Jim Wilson , gdb-patches@sourceware.org References: <20181101214139.27196-1-jimw@sifive.com> From: John Baldwin Openpgp: preference=signencrypt Message-ID: Date: Thu, 01 Nov 2018 22:05:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181101214139.27196-1-jimw@sifive.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00011.txt.bz2 On 11/1/18 2:41 PM, Jim Wilson wrote: > Some hardware doesn't support unaligned accesses, and a bare metal target > may not have an unaligned access trap handler. So if the PC is 2-byte > aligned, then use a 2-byte breakpoint to avoid unaligned accesses. > > Tested on native RV64GC Linux with gdb testsuite and cross on spike > simulator and openocd with riscv-tests/debug. > > gdb/ > * riscv-tdep.c (riscv_breakpoint_kind_from_pc): New local unaligned_p. > Set if pcptr if unaligned. Return 2 if unaligned_p true. Update > debugging messages. > --- > gdb/riscv-tdep.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 4b5f38a877..9c6872d021 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -415,18 +415,33 @@ riscv_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr) > { > if (use_compressed_breakpoints == AUTO_BOOLEAN_AUTO) > { > + bool unaligned_p = false; > gdb_byte buf[1]; > > - /* Read the opcode byte to determine the instruction length. */ > - read_code (*pcptr, buf, 1); > + /* Some targets don't support unaligned reads. If the instruction > + address is unaligned, use a compressed breakpoint. */ > + if (*pcptr & 0x2) > + unaligned_p = true; > + else > + { > + /* Read the opcode byte to determine the instruction length. */ > + read_code (*pcptr, buf, 1); > + } > > if (riscv_debug_breakpoints) > - fprintf_unfiltered > - (gdb_stdlog, > - "Using %s for breakpoint at %s (instruction length %d)\n", > - riscv_insn_length (buf[0]) == 2 ? "C.EBREAK" : "EBREAK", > - paddress (gdbarch, *pcptr), riscv_insn_length (buf[0])); > - if (riscv_insn_length (buf[0]) == 2) > + { > + const char *bp = (unaligned_p || riscv_insn_length (buf[0]) == 2 > + ? "C.EBREAK" : "EBREAK"); > + > + fprintf_unfiltered (gdb_stdlog, "Using %s for breakpoint at %s ", > + bp, paddress (gdbarch, *pcptr)); > + if (unaligned_p) > + fprintf_unfiltered (gdb_stdlog, "(unaligned address)\n"); > + else > + fprintf_unfiltered (gdb_stdlog, "(instruction length %d)\n", > + riscv_insn_length (buf[0])); > + } > + if (unaligned_p || riscv_insn_length (buf[0]) == 2) > return 2; > else > return 4; This looks good to me. Not sure if you want to explicitly add something to the comment that an unaligned PC is only valid if C is supported, so we know C.EBREAK is safe to use with an unaligned PC? -- John Baldwin                                                                            Â