Why enforcing sw_breakpoint_from_kind() implementation in GDBserver targets

Shahab Vahedi
2020-06-10T17:47:05Z

Hello,

I believe that the "sw_breakpoint_from_kind()" method declared in
gdbserver/target.h should not be a pure virtual method. Please read on
for further details.

Recently, I have rebased our code base of ARC GDB [1] onto the master of
binutils-gdb [2]. During this process, I noticed that "linux target ops"
have become methods [3].

I noticed that the proto type of "sw_breakpoint_from_kind()" has become
[4]:

-------------------------[ gdbserver/target.h ]-------------------------
...
class process_stratum_target
{
public:
...
virtual const gdb_byte *
sw_breakpoint_from_kind (int kind, int *size) = 0;
...
};
...
------------------------------------------------------------------------

This means "sw_breakpoint_from_kind()" is a pure virtual method now.
Any class inheriting "process_stratum_target" or its derivatives
("linux_process_target" in particular) MUST implement this method.
Else, the compilation will fail.

In ARC's case, the GDBserver code did not have this method implemented
[5]:

---------------------[ gdbserver/linux-arc-low.cc ]---------------------
...
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,
};
...
------------------------------------------------------------------------

Now, it has to provide the implementation for "sw_breakpoint_from_kind".
That is not the whole story. I also noticed that this piece of newly
implemented code never gets executed. This makes sense because we have
a setup that looks like below (both entities are running inside
GNU/Linux):

,------------------------------------------------.
| ARC GDB client on x86 machine (cross debugger) |
`------------------------------------------------'
/\
||
remote debugging
||
\/
,------------------------------------------------.
| ARC GDBserver on ARC board (native server) |
`------------------------------------------------'

It is the "ARC GDB client" that inserts the breakpoint. That has always
been the case. Else, it would be impossible to break while debugging
with GDBserver in older implementation (before rebase). It is worth
mentioning the "ARC GDB client" does have the "sw_breakpoint_from_kind"
implemented [].

Last but not least, one nitpick: Even though I have added the
implementation of "sw_breakpoint_from_kind", I have never done so for
"breakpoint_kind_from_pc" or "breakpoint_kind_from_current_state".
These last two are supposed to provide the "kind" that will be the
input parameter for "sw_breakpoint_from_kind". Therefore, even if the
new piece of "sw_breakpoint_from_kind" would be executed, that would be
problematic. I'm not sure what can be done about this but I think _if_
"sw_breakpoint_from_kind" should be mandatory, so are
"breakpoint_kind_from_pc" and "breakpoint_kind_from_current_state".

Cheers,
Shahab

[1] ARC GDB repository
https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb

[2] Head of the master at the time of rebase
commit: cebd6b8ac1c5a2a847a50e3efe932ff2d0867b3e
IFUNC: Update IFUNC resolver check with DT_TEXTREL

[3] Commits for converting "linux target ops" to methods
0dd7b52ede3 gdbserver/linux-low: delete 'linux_target_ops' and 'the_low_target'
fc5ecdb6304 gdbserver/linux-low: turn 'get_ipa_tdesc_idx' into a method
9eedd27d42c gdbserver/linux-low: turn 'get_syscall_trapinfo' into a method
b31cdfa69f4 gdbserver/linux-low: turn 'supports_hardware_single_step' into a method
9cfd8715514 gdbserver/linux-low: turn 'supports_range_stepping' into a method
ab64c99982e gdbserver/linux-low: turn 'emit_ops' into a method
809a0c354b9 gdbserver/linux-low: turn fast tracepoint ops into methods
13e567af27e gdbserver/linux-low: turn 'get_thread_area' into a method
47f70aa7685 gdbserver/linux-low: turn 'supports_tracepoints' into a method
a5b5da9258d gdbserver/linux-low: turn 'process_qsupported' into a method
d7599cc0826 gdbserver/linux-low: turn 'prepare_to_resume' into a method
fd000fb3dfd gdbserver/linux-low: turn process/thread addition/deletion ops into methods
cb63de7ca80 gdbserver/linux-low: turn 'siginfo_fixup' into a method
b35db73327c gdbserver/linux-low: turn '{collect, supply}_ptrace_register' into methods
ac1bbaca106 gdbserver/linux-low: turn watchpoint ops into methods
9db9aa232ac gdbserver/linux-low: turn 'insert_point' and 'remove_point' into methods
007c9b975dc gdbserver/linux-low: turn 'supports_z_point_type' into a method
d7146cda56c gdbserver/linux-low: turn 'breakpoint_at' into a method
d4807ea231e gdbserver/linux-low: turn the 'decr_pc_after_break' field into a method
7582c77c1d2 gdbserver/linux-low: turn 'supports_software_single_step' and 'get_next_pcs' into methods
3ca4edb6617 gdbserver/linux-low: turn 'sw_breakpoint_from_kind' into a method
06250e4e67c gdbserver/linux-low: turn 'breakpoint_kind_from_{pc, current_state}' into methods
bf9ae9d8c37 gdbserver/linux-low: turn 'get_pc' and 'set_pc' into methods
df95181f00d gdbserver/linux-low: turn some more static functions into private methods
bd70b1f240b gdbserver/linux-low: turn 'fetch_register' into a method
daca57a7de5 gdbserver/linux-low: turn 'cannot_{fetch/store}_register' into methods
aa8d21c9bb4 gdbserver/linux-low: turn 'regs_info' into a method
797bcff595c gdbserver/linux-low: turn 'arch_setup' into a method
ef0478f6112 gdbserver/linux-low: start turning linux target ops into methods

[4] gdbserver: turn breakpoint kind-related target ops into methods
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=d367006fb7cf

[5] ARC's GDB server code before rebase; not providing sw_breakpoint_from_kind()
https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb/blob/arc-2020.03/gdbserver/linux-arc-low.cc#L311

[6] ARC's GDB client having the "sw_breakpoint_from_kind" method