Re: RISC-V: decr_pc_after_break causing problems

As another point of reference, the gdb we use with OpenOCD does not have
the set_gdbarch_decr_pc_after_break line at all.

Tim

On Mon, Jun 25, 2018 at 7:54 PM, Jim Wilson <[email protected]> wrote:

> The RISC-V port in the riscv-tdep.c file has
>   set_gdbarch_decr_pc_after_break (gdbarch, (has_compressed_isa ? 2 : 4));
>
> The privileged architecture spec v1.10 states in section 3.2.1 that
> the ebreak instruction causes the receiving privilege mode's epc
> register to be set to the address of the ebreak instruction, not the
> address of the following instruction.  So gdb should not be
> decrementing from the pc after a breakpoint is hit.
>
> It isn't clear why this code is even here, as it isn't present in the
> original gdb port in the github riscv/riscv-binutils-gdb tree.
>
> Curiously, there is a corresponding bug in the riscv linux kernel
> sources, where it is adding 4 to the sepc in the breakpoint trap
> handling code for no apparent reason.  This might be OK if this was a
> 4-byte breakpoint instruction, but is not OK if this is a 2-byte
> breakpoint instruction.
>
> In order to get compressed breakpoints working on a SiFive HiFive
> Unleashed board running linux, I need both the gdb and the linux
> kernel bugs fixed.  The 4-byte breakpoint instruction works OK now,
> but is not safe to use in code compiled with compressed instructions.
> A good example is in the shared library support where _dl_debug_state
> is a 2-byte function located 2-bytes before _dl_debug_initialize, so
> placing a 4-byte breakpoint at _dl_debug_state overwrites the first
> two bytes of the first instruction of _dl_debug_initialize causing it
> to segfault.
>
> I can submit patches for gdb and the linux kernel, but it would be
> useful to know why gdb is trying to subtract from the pc after a
> break.  Maybe someone has a part that doesn't conform to the v1.10
> privilege architecture spec?  I noticed that this epc == breakpoint
> address is not stated in earlier versions of the spec, which makes
> earlier spec versions potentially ambiguous.  If we need to support
> parts that don't conform to v1.10 priv spec then that makes the fix
> more complicated.  It isn't clear how gdb is supposed to detect
> whether a part conforms or not.  Maybe we can add an option to turn
> this decrementing on
> or off?  Maybe a configure option to select whether it is on/off by
> default?
>
> There is another problem here incidentally that there is an option to
> turn on/off compressed breakpoints, but it doesn't affect the amount
> we subtract from the pc, which means this option can't work as
> currently written.  This problem goes away if we stop decrementing the
> pc in gdb.  If we have to keep the code that decrements the pc for
> some targets, then maybe we should just eliminate the option.  It
> isn't safe to use 4-byte breakpoints in code with compressed
> instructions anyways.  And there is no point in using 2-byte
> breakpoints in code with no compressed instructions.
>
> Jim
>