Re: RISC-V: decr_pc_after_break causing problems

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Jim Wilson <jimw@sifive.com> [2018-06-25 19:54:14 -0700]:

> 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.

Maybe I'm going to look completely silly here, but....

....aren't epc and pc different registers?

The epc is really many register, right? one for each privilege level,
uepc, sepc, and mepc.

The spec is clear that when an ebreak or c.ebreak is hit the contents
of *epc are the contents of the breaking instruction, but nothing is
said about the contents of pc.

> 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.

Assuming that my above observations are correct, then when we hit a
breakpoint *epc is defined, but pc is not, an implementation could do
anything, right?  The two obvious choices are not increment pc, or
increment pc.

Right now my understanding is that GDB doesn't have special support
for fetching the program counter from different places depending on
the current inferior state (stopped at breakpoint, not stopped at
breakpoint).  So, somehow the value in *epc needs to be passed back
into the pc register (I guess).

I'm not exactly sure at which point in the process this is supposed to
happen.... I'll have another look around and see if any other target
does something similar.

Thanks,
Andrew