RISC-V: decr_pc_after_break causing problems
Andrew Burgess
andrew.burgess@embecosm.com
Wed Jul 11 14:52:00 GMT 2018
* John Baldwin <jhb@FreeBSD.org> [2018-07-05 15:54:32 -0700]:
> On 7/3/18 5:35 PM, Palmer Dabbelt wrote:
> > On Tue, 03 Jul 2018 17:17:04 PDT (-0700), Jim Wilson wrote:
> >> On Mon, Jun 25, 2018 at 7:54 PM, Jim Wilson <jimw@sifive.com> wrote:
> >>> The RISC-V port in the riscv-tdep.c file has
> >>> set_gdbarch_decr_pc_after_break (gdbarch, (has_compressed_isa ? 2 : 4));
> >>
> >> I'm still hoping to get a response to this. I need to make
> >> coordinated fixes to both gdb and the linux kernel to get breakpoints
> >> working correctly.
> >
> > Andrew: I think this materialized itself when you submitted the GDB patches,
> > probably because we have this in our Linux code:
> >
> > asmlinkage void do_trap_break(struct pt_regs *regs)
> > {
> > #ifdef CONFIG_GENERIC_BUG
> > if (!user_mode(regs)) {
> > enum bug_trap_type type;
> >
> > type = report_bug(regs->sepc, regs);
> > switch (type) {
> > case BUG_TRAP_TYPE_NONE:
> > break;
> > case BUG_TRAP_TYPE_WARN:
> > regs->sepc += sizeof(bug_insn_t);
> > return;
> > case BUG_TRAP_TYPE_BUG:
> > die(regs, "Kernel BUG");
> > }
> > }
> > #endif /* CONFIG_GENERIC_BUG */
> >
> > force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)(regs->sepc), current);
> > regs->sepc += 0x4;
> > }
> >
> > There's at least one bug in the Linux port here: we can enter a breakpoint trap
> > via either ebreak (a 4-byte instruction) or c.ebreak (a 2-byte instruction).
> > c.ebreak is necessary for a sane debugger so we need to support it. Our
> > options are:
> >
> > * Handle c.ebreak in Linux and leave this as it stands.
> > * Remove both the Linux PC adjustment and the GDB PC adjustment.
> >
> > I'm inclined to take the second option as it's less code. I suppose
> > technically it's an ABI break, but since it's broken anyway then I'm happy with
> > taking it.
> >
> > Is there something I'm missing? If not Jim will submit a Linux patch and then
> > we'll pull the trigger on this one.
>
> FWIW, my preference would be for the decr_after_pc_break match the hardware
> which from my understanding of the thread means it should always be
> zero.
I agree this solution is the correct approach and GDB should be
changed.
Thanks,
Andrew
More information about the Gdb
mailing list