Re: RFA [PATCH v4] Implement 'catch syscall' for gdbserver (was Re: RFA [PATCH v3] Implement 'catch syscall' for gdbserver)

Pedro Alves
2013-10-04T17:40:26Z

On 10/04/2013 05:22 AM, Luis Machado wrote:
> On 09/29/2013 12:04 PM, Philippe Waroquiers wrote:
>> ChangeLog
>> 2013-xx-yy Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * NEWS: Document new QcatchSyscalls packet and its use
>> in x86/amd64 linux gdbserver and Valgrind gdbserver.
>> * remote.c (PACKET_QCatchSyscalls): New.
>> (remote_protocol_features): Add QcatchSyscalls.
>> (remote_set_syscall_catchpoint): New function.
>> (remote_parse_stop_reply): New stop reasons syscall_entry
>> and syscall_return.
>> (init_remote_ops): Registers remote_set_syscall_catchpoint
>> and the config commands for PACKET_QCatchSyscalls.
>
> I'm late to the party, but i've always wondered why we have all these
> different "insert_<foo>_catchpoint" and "remove_<foo>_catchpoint"
> functions to accomplish tasks that seem to be very similar in nature.
>
> Not saying we should go this route for this patch, but we may want to
> consider a more generic RSP packet for catchpoints. Something like the
> following:
>
> QInsertCatchpoint:[syscall|fork|exec|vfork|unload|...]
> QRemoveCatchpoint:[syscall|fork|exec|vfork|unload|...]
>
> ... or even communicate catchpoints through Z/z packets, though that
> would be a more radical approach.

You know, you actually have a very good point. It actually looks
unfortunate to come up with new packets that don't incorporate
all the nice new features we've added to the Z/z packets recently,
such as target side conditions and commands.

The issue I see is that syscall (and other catchpoints) have
arguments. What would you pass to QRemoveCatchpoint to remove
a previous catchpoint? Sounds like QInsertCatchpoint would need
to return a unique target-side identifier, that QRemoveCatchpoint
would then use? There's also the issue with the fact that
for Z packets, the RSP specifies that a second Z packet seen for
the same address replaces the previous packet, because it might
have happened that GDB and the server lost sync for a bit, and the
second packet was actually a retransmission. Making QInsertCatchpoint
return a reference conflicts with that. Unless perhaps we make GDB
send a unique id along as well... I think the RSP used to always send
a sequence number with each packet, and that has been removed a long
time ago. I wish I know why it was removed. It would solve these
issues. Maybe we should add it back.

> Anyway, just throwing a few ideas since i've been dealing with some of
> the issues with catchpoints, forking and gdbserver as well.

Yeah.

--
Pedro Alves