Re: Why enforcing sw_breakpoint_from_kind() implementation in GDBserver targets

Simon Marchi
2020-06-11T03:05:39Z

On 2020-06-10 1:47 p.m., Shahab Vahedi via Gdb wrote:
> 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 [].

I don't understand your "This makes sense because we have a setup that looks
like below", because that looks like a standard GDB/GDBserver setup used for
other architectures.

When a breakpoint is inserted, what's the remote protocol packet used? Is it
Z0, or is it a memory write operation that writes the breakpoint's opcode? Z0
is the "modern" way that provides more features (like target-side condition
evaluation) and a memory write is the legacy fallback.

sw_breakpoint_from_kind would be used if the Z0 packet was used, to translate
the "kind" into an opcode. Since you claim that sw_breakpoint_from_kind is
not used, I guess that the breakpoint is inserted with a memory write operation.
I'd look into why that is the case. GDB tries Z0 first and falls back to the
memory write if Z0 is not supported, so your GDBserver must not support it for
some reason.
> 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".

As mentioned above, the input for sw_breakpoint_from_kind can come from
the Z0 packet, so it may make sense to have sw_breakpoint_from_kind without
the others. I am not sure off-hand when the others are used.

Simon