This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] RISC-V: Don't allow unaligned breakpoints.
- From: John Baldwin <jhb at FreeBSD dot org>
- To: Jim Wilson <jimw at sifive dot com>, gdb-patches at sourceware dot org
- Date: Thu, 1 Nov 2018 15:05:07 -0700
- Subject: Re: [PATCH] RISC-V: Don't allow unaligned breakpoints.
- References: <20181101214139.27196-1-jimw@sifive.com>
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