Why enforcing sw_breakpoint_from_kind() implementation in GDBserver targets

[Shahab Vahedi]

Hi Baris,

Sorry for my late reply and thank you again for your thorough response.

On Tue, Jun 16, 2020 at 01:15:29PM +0000, Aktemur, Tankut Baris wrote:
> 1. In GDB 9 (i.e. before C++'ification of gdbserver's target definition),
> the base linux target implemented the "sw_breakpoint_from_kind" target op
> to directly delegate to "sw_breakpoint_from_kind" of the linux low target.
> It asserted that the low target's "sw_breakpoint_from_kind" function pointer
> is non-null.

And this is how the ARC port at the time looked like:
gdb/gdbserver/linux-arc-low.c [1]:
...
struct linux_target_ops the_low_target =
{
  arc_arch_setup,
  arc_regs_info,
  arc_cannot_fetch_register,
  arc_cannot_store_register,
  NULL,                      /* fetch_register  */
  linux_get_pc_32bit,
  linux_set_pc_32bit,
  NULL,                      /* breakpoint_kind_from_pc  */
  NULL,                      /* sw_breakpoint_from_kind  */
  NULL,                      /* get_next_pcs  */
  0,                         /* decr_pc_after_break  */
  arc_breakpoint_at,
};
...
This did not hit the assert in base linux implementation, since
"linux_sw_breakpoint_from_kind" was never executed. It's because
the GDB client was writing the breakpoint opcodes to the address
through gdbserver while bypassing sw_breakpoint_from_kind().

> As far as I understand, there is a certain control flow and a combination
> of (un)supported features in the ARC port with which the "sw_breakpoint_from_kind"
> target op is not invoked.  I'm not sure what this flow or feature combination is.

I guess what I explained above should be the case. I do not know
the exact control flow that led/leads to this though.
> Just out of curiosity, is there a specific reason why you want to avoid implementing
> the "sw_breakpoint_from_kind" target op?

No, there is no reason that I might be against it. I brought this whole thing
up because I noticed ARC did not have sw_breakpoint_from_kind implemented and
it was working fine. Even after I implemented the pure virtual method [2], it
does not get executed (I put an assert in a temporary implementation as an
experiment). So it seemed like an unnecessary constraint for compilation.
If we want to deprecate how ARC inserts breakpoints (GDB client writing
the opcode through GDBserver) then that is fine [3]. If not, then implementing
sw_breakpoint_from_kind() shouldn't be mandatory. 

I just wanted to make sure that this message is conveyed. That's all :)

For the record, I plan to change ARC's behavior to really use
sw_breakpoint_from_kind(), but that is not the purpose of this email.


Shahab

[1]
https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb/blob/arc-2019.09-gdb/gdb/gdbserver/linux-arc-low.c#L300

[2]
https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb/blob/arc-2020.09/gdbserver/linux-arc-low.cc#L350

[3]
Please keep in mind that if deprecation is wanted, yet still nothing is
stopping ARC (and the likes) from bypassing sw_breakpoint_from_kind().