Re: [RFC] reviving 'catch syscall' for gdbserver

[Josh Stone <jistone at redhat dot com>]

On 09/15/2015 03:01 PM, Philippe Waroquiers wrote:
> On Fri, 2015-09-11 at 18:44 -0700, Josh Stone wrote:
>> Hi all,
>>
>> I've been using Philippe's QCatchSyscalls patch[1] for gdbserver, and
>> I'd like to reopen the discussion about merging this patch.  Philippe
>> told me would be fine with me taking this up, as he doesn't have time to
>> work on it now.
> Thanks for reviving this.
> 
>> I haven't included formalities like ChangeLogs here, because I would
>> like to get basic consensus on the approach first.  I also don't know
>> the right way to approach authorship here, between what Philippe
>> originally wrote and my changes since, for what's ultimately committed.
> For authorship : there is no problem to re-use what you want from the
> old patch (this old patch is covered by the copyright assignment I
> signed).
> So, feel free to re-use and commit what you want.
> If you think you better keep my name around that, I will be more
> than happy with something like:
> '(starting from some work done by Philippe)'

Copyright assignment covers the legal requirement, I guess, but there's
still plain courtesy. :)  I'll try to credit you somewhere.

>> I tested catch-syscall.exp on Fedora 22 x86_64, for targets unix,
>> native-gdbserver, and native-extended-gdbserver, and all passed.
>>
>> Philippe pointed me to his last summary of issues, which I'd like to
>> start the discussion with:
>> https://sourceware.org/ml/gdb-patches/2013-10/msg00321.html
>>
>>> * QCatchSyscalls contains target specific numbers (this is the
>>>   above comment)
>>>   => have gdbserver handling QCatchSyscalls packet per inferior
>>
>> Does this still need to be per-inferior?  I do understand that syscall
>> numbers may differ, e.g. from i686 to x86_64 on the same target.  Are
>> there any other examples of such things that are dealt with separately?
> 
> I am not sure to understand how GDB handles catch syscall 
> when it has multiple inferiors of different archs
> (in a 'non-remote' setup).
> When looking at catch_syscall_command_1, I do not understand
> how a single catch syscall command would be translated into
> different numbers depending on the arch: catch syscall seems
> to just work for the current arch, translating a name to 
> a nr, and then working/storing this number (or VEC of numbers).
> So, not sure about the behaviour in a 'non-remote' setup.
> (unless a 'catch syscall' is associated to an inferior.
> Then the 'info breakpoint' does not show that the catch syscall
> is for a specific inferior:
> 6       catchpoint     keep y                      syscall "mprotect" 
> 7       catchpoint     keep y                      syscall "unlink" 
> 8       breakpoint     keep y   0x08048457         in main at xadd.c:35 inf 2
> As you can see, breakpoint 8 is marked as being for 'inf 2';
> while the catchpoint are not associated to an inferior.

I just noticed in PR10737#c5, Pedro said, "syscall catchpoints is
presently inferior-specific".  But it's not clear to me if he meant the
entire existence of the catchpoint, or just name->NR mapping.

> In any case, assuming that GDB handles properly catch syscall for
> different archs, then I think that the change needed is to update
> the catchsyscall packet, so as to include the pid in the packet.
> Then at gdbserver side, the list of syscall to catch has to be stored
> per inferior (in the  struct process_info of gdbserver/inferiors.h)
> instead of being a global variable.

What about relying on the current target, and use Hg switching?  Or is
it more common for new commands to have an explicit pid?  It doesn't
look like many q packets have a pid, even though many have an action
specific to one target.

>>> * extend the stop reply packet to allow to return a
>>>     "syscall" stop reason that does not specify if this is a syscall
>>>     entry or exit.
>>>   I suggest to do this even if a correct flip/flop logic can be
>>>   found during the previous investigation.
>>>   This 3rd syscall stop reason allow stubs to report a syscall
>>>   without necessarily having the logic to differentiate entry
>>>   from return.
>>
>> I'm not keen on this, personally.  If a stub can't keep proper track of
>> this, it seems unlikely that the remote gdb can do it better.
> The idea was to allow a (simple) stub to just report it has
> encountered a syscall, when the stub cannot differentiate an entry
> from an exit. But I guess we can always add this new stop
> reason in the future, when we see this would be needed (and such
> a stub is encountered).

Yeah, I think that should be left for a demonstrated need.  Even a
simple stub that just toggled entry/return can probably track this
better than gdb could from afar.