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

[Pedro Alves <palves@redhat.com>]

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