[PATCH] Implement 'catch syscall' for gdbserver

Josh Stone jistone@redhat.com
Wed Nov 25 02:37:00 GMT 2015


Thanks for the review!

I'll go ahead and correct the style issues without further comment.
Other replies are inline.

I'll try to rebase and send updates tomorrow.

On 11/22/2015 08:18 PM, Doug Evans wrote:
> Josh Stone <jistone@redhat.com> writes:
>> This adds a new QCatchSyscalls packet to enable 'catch syscall', and new
>> stop reasons "syscall_entry" and "syscall_return" for those events.  It
>> is currently only supported on Linux x86 and x86_64.
>>
>> Based on work from Philippe Waroquiers <philippe.waroquiers@skynet.be>.
>>
>> Beyond simple rebasing, I've also updated it to store the syscall catch
>> lists uniquely to each target process, and updated entry/return logic
>> from checking ENOSYS to now matching gdb's current Linux logic.
>>
>> gdb/ChangeLog:
>>
>> 2015-10-29  Josh Stone  <jistone@redhat.com>
>>
>> 	* NEWS (Changes since GDB 7.10): Mention QCatchSyscalls and new
>> 	GDBserver support for catch syscall.
>> 	* remote.c (PACKET_QCatchSyscalls): New enum.
>> 	(remote_set_syscall_catchpoint): New function.
>> 	(remote_protocol_features): New element for QCatchSyscalls.
>> 	(remote_parse_stop_reply): Parse syscall_entry/return stops.
>> 	(init_remote_ops): Install remote_set_syscall_catchpoint.
>> 	(_initialize_remote): Config QCatchSyscalls.
>>
>> gdb/doc/ChangeLog:
>>
>> 2015-10-29  Josh Stone  <jistone@redhat.com>
>>
>> 	* gdb.texinfo (Remote Configuration): List the QCatchSyscalls packet.
>> 	(Stop Reply Packets): List the syscall entry and return stop reasons.
>> 	(General Query Packets): Describe QCatchSyscalls, and add it to the
>> 	table and detailed list of stub features.
>>
>> gdb/gdbserver/ChangeLog:
>>
>> 2015-10-29  Josh Stone  <jistone@redhat.com>
>>
>> 	* inferiors.h: Include "gdb_vecs.h".
>> 	(struct process_info): Add syscalls_to_catch.
>> 	* inferiors.c (remove_process): Free syscalls_to_catch.
>> 	* remote-utils.c (prepare_resume_reply): Report syscall_entry and
>> 	syscall_resume stops.
>> 	* server.h (UNKNOWN_SYSCALL, ANY_SYSCALL): Define.
>> 	* server.c (handle_general_set): Handle QCatchSyscalls.
>> 	(handle_query): Report support for QCatchSyscalls.
>> 	* target.h (struct target_ops): Add supports_catch_syscall.
>> 	(target_supports_catch_syscall): New macro.
>> 	* linux-low.h (struct linux_target_ops): Add get_syscall_trapinfo.
>> 	(struct lwp_info): Add syscall_state.
>> 	* linux-low.c (SYSCALL_SIGTRAP): Define.
>> 	(handle_extended_wait): Mark syscall_state like an entry.
>> 	(get_syscall_trapinfo): New function, proxy to the_low_target.
>> 	(linux_low_ptrace_options): Enable PTRACE_O_TRACESYSGOOD.
>> 	(linux_low_filter_event): Set ptrace options even before arch-specific
>> 	setup.  Either toggle syscall_state entry/return or set ignored.
>> 	(gdb_catching_syscalls_p): New function.
>> 	(gdb_catch_this_syscall_p): New function.
>> 	(linux_wait_1): Handle SYSCALL_SIGTRAP.
>> 	(linux_resume_one_lwp_throw): Add PTRACE_SYSCALL possibility.
>> 	(linux_supports_catch_syscall): New function.
>> 	(linux_target_ops): Install it.
>> 	* linux-x86-low.c (x86_get_syscall_trapinfo): New function.
>> 	(the_low_target): Install it.
>> 	* nto-low.c (nto_target_ops): Install NULL supports_catch_syscall.
>> 	* spu-low.c (spu_target_ops): Likewise.
>> 	* win32-low.c (win32_target_ops): Likewise.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2015-10-29  Josh Stone  <jistone@redhat.com>
>>
>> 	* gdb.base/catch-syscall.exp: Enable testing for x86 and x86_64 linux
>> 	remote targets.
>> 	(do_syscall_tests): Only test mid-vfork on local or extended-remote.
>> ---
>>  gdb/NEWS                                 |  10 ++
>>  gdb/doc/gdb.texinfo                      |  56 +++++++++++
>>  gdb/gdbserver/inferiors.c                |   1 +
>>  gdb/gdbserver/inferiors.h                |   5 +
>>  gdb/gdbserver/linux-low.c                | 158 +++++++++++++++++++++++++++++--
>>  gdb/gdbserver/linux-low.h                |  13 +++
>>  gdb/gdbserver/linux-x86-low.c            |  31 ++++++
>>  gdb/gdbserver/nto-low.c                  |   1 +
>>  gdb/gdbserver/remote-utils.c             |  12 +++
>>  gdb/gdbserver/server.c                   |  49 ++++++++++
>>  gdb/gdbserver/server.h                   |   6 ++
>>  gdb/gdbserver/spu-low.c                  |   1 +
>>  gdb/gdbserver/target.h                   |   8 ++
>>  gdb/gdbserver/win32-low.c                |   1 +
>>  gdb/remote.c                             | 116 +++++++++++++++++++++++
>>  gdb/testsuite/gdb.base/catch-syscall.exp |  15 ++-
>>  16 files changed, 472 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index b2b1e99d58c6..9c9b4b04a146 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -77,6 +77,11 @@ exec-events feature in qSupported
>>    response can contain the corresponding 'stubfeature'.  Set and
>>    show commands can be used to display whether these features are enabled.
>>  
>> +QCatchSyscalls:1 [;SYSNO]...
>> +QCatchSyscalls:0
>> +  Enable ("QCatchSyscalls:1") or disable ("QCatchSyscalls:0")
>> +  catching syscalls from the inferior process.
>> +
>>  * Extended-remote exec events
>>  
>>    ** GDB now has support for exec events on extended-remote Linux targets.
>> @@ -87,6 +92,11 @@ set remote exec-event-feature-packet
>>  show remote exec-event-feature-packet
>>    Set/show the use of the remote exec event feature.
>>  
>> +* New features in the GDB remote stub, GDBserver
>> +
>> +  ** GDBserver now supports catch syscall.  Currently enabled
>> +     on x86/x86_64 GNU/Linux targets.
>> +
>>  *** Changes in GDB 7.10
>>  
>>  * Support for process record-replay and reverse debugging on aarch64*-linux*
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 3c1f7857393c..d8ed630da3fc 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -20121,6 +20121,10 @@ are:
>>  @tab @code{qSupported}
>>  @tab Remote communications parameters
>>  
>> +@item @code{catch-syscalls}
>> +@tab @code{QCatchSyscalls}
>> +@tab @code{catch syscall}
>> +
>>  @item @code{pass-signals}
>>  @tab @code{QPassSignals}
>>  @tab @code{handle @var{signal}}
>> @@ -35418,6 +35422,11 @@ The currently defined stop reasons are:
>>  The packet indicates a watchpoint hit, and @var{r} is the data address, in
>>  hex.
>>  
>> +@item syscall_entry
>> +@itemx syscall_return
>> +The packet indicates a syscall entry or return, and @var{r} is the
>> +syscall number, in hex.
>> +
>>  @cindex shared library events, remote reply
>>  @item library
>>  The packet indicates that the loaded libraries have changed.
>> @@ -35878,6 +35887,44 @@ by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
>>  Use of this packet is controlled by the @code{set non-stop} command; 
>>  @pxref{Non-Stop Mode}.
>>  
>> +@item QCatchSyscalls:1 @r{[};@var{sysno}@r{]}@dots{}
>> +@itemx QCatchSyscalls:0
>> +@cindex catch syscalls from inferior, remote request
>> +@cindex @samp{QCatchSyscalls} packet
>> +@anchor{QCatchSyscalls}
>> +Enable (@samp{QCatchSyscalls:1}) or disable (@samp{QCatchSyscalls:0})
>> +catching syscalls from the inferior process.
>> +
>> +For @samp{QCatchSyscalls:1}, each listed syscall @var{sysno} (encoded
>> +in hex) should be reported to @value{GDBN}.  If no syscall @var{sysno}
>> +is listed, every system call should be reported.
>> +
>> +Note that if a syscall not member of the list is reported, @value{GDBN}
>> +will filter it if this syscall is not caught.  It is however more efficient
>> +to only report the needed syscalls.
>> +
>> +Multiple @samp{QCatchSyscalls:1} packets do not
>> +combine; any earlier @samp{QCatchSyscalls:1} list is completely replaced by the
>> +new list.
>> +
>> +Reply:
>> +@table @samp
>> +@item OK
>> +The request succeeded.
>> +
>> +@item E @var{nn}
>> +An error occurred.  @var{nn} are hex digits.
>> +
>> +@item @w{}
>> +An empty reply indicates that @samp{QCatchSyscalls} is not supported by
>> +the stub.
>> +@end table
>> +
>> +Use of this packet is controlled by the @code{set remote catch-syscalls}
>> +command (@pxref{Remote Configuration, set remote catch-syscalls}).
>> +This packet is not probed by default; the remote stub must request it,
>> +by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
>> +
>>  @item QPassSignals: @var{signal} @r{[};@var{signal}@r{]}@dots{}
>>  @cindex pass signals to inferior, remote request
>>  @cindex @samp{QPassSignals} packet
>> @@ -36296,6 +36343,11 @@ These are the currently defined stub features and their properties:
>>  @tab @samp{-}
>>  @tab Yes
>>  
>> +@item @samp{QCatchSyscalls}
>> +@tab No
>> +@tab @samp{-}
>> +@tab Yes
>> +
>>  @item @samp{QPassSignals}
>>  @tab No
>>  @tab @samp{-}
>> @@ -36489,6 +36541,10 @@ packet (@pxref{qXfer fdpic loadmap read}).
>>  The remote stub understands the @samp{QNonStop} packet
>>  (@pxref{QNonStop}).
>>  
>> +@item QCatchSyscalls
>> +The remote stub understands the @samp{QCatchSyscalls} packet
>> +(@pxref{QCatchSyscalls}).
>> +
>>  @item QPassSignals
>>  The remote stub understands the @samp{QPassSignals} packet
>>  (@pxref{QPassSignals}).
>> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
>> index 72a3ef1b1b61..a8263656a4af 100644
>> --- a/gdb/gdbserver/inferiors.c
>> +++ b/gdb/gdbserver/inferiors.c
>> @@ -316,6 +316,7 @@ remove_process (struct process_info *process)
>>    free_all_breakpoints (process);
>>    gdb_assert (find_thread_process (process) == NULL);
>>    remove_inferior (&all_processes, &process->entry);
>> +  VEC_free (int, process->syscalls_to_catch);
>>    free (process);
>>  }
>>  
>> diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h
>> index d7226163c0e8..43fc869f6612 100644
>> --- a/gdb/gdbserver/inferiors.h
>> +++ b/gdb/gdbserver/inferiors.h
>> @@ -19,6 +19,8 @@
>>  #ifndef INFERIORS_H
>>  #define INFERIORS_H
>>  
>> +#include "gdb_vecs.h"
>> +
>>  /* Generic information for tracking a list of ``inferiors'' - threads,
>>     processes, etc.  */
>>  struct inferior_list
>> @@ -67,6 +69,9 @@ struct process_info
>>    /* The list of installed fast tracepoints.  */
>>    struct fast_tracepoint_jump *fast_tracepoint_jumps;
>>  
>> +  /* The list of syscalls to report, or just ANY_SYSCALL.  */
>> +  VEC (int) *syscalls_to_catch;
>> +
>>    const struct target_desc *tdesc;
>>  
>>    /* Private target data.  */
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 41ab510fa4ac..c2c513130997 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -70,6 +70,11 @@
>>  #define O_LARGEFILE 0
>>  #endif
>>  
>> +/* Unlike other extended result codes, WSTOPSIG (status) on
>> +   PTRACE_O_TRACESYSGOOD syscall events doesn't return SIGTRAP, but
>> +   instead SIGTRAP with bit 7 set.  */
>> +#define SYSCALL_SIGTRAP (SIGTRAP | 0x80)
>> +
> 
> This is already defined in nat/linux-nat.h.

Ah, indeed, and that's already included too.  I'll nuke this.

>>  /* Some targets did not define these ptrace constants from the start,
>>     so gdbserver defines them locally here.  In the future, these may
>>     be removed after they are added to asm/ptrace.h.  */
>> @@ -447,6 +452,11 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
>>    struct thread_info *event_thr = get_lwp_thread (event_lwp);
>>    struct lwp_info *new_lwp;
>>  
>> +  /* All extended events we currently use are mid-syscall.  Only
>> +     PTRACE_EVENT_STOP is delivered more like a signal-stop, but
>> +     you have to be using PTRACE_SEIZE to get that.  */
>> +  event_lwp->syscall_state = TARGET_WAITKIND_SYSCALL_ENTRY;
>> +
>>    if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_VFORK)
>>        || (event == PTRACE_EVENT_CLONE))
>>      {
>> @@ -662,6 +672,40 @@ get_pc (struct lwp_info *lwp)
>>    return pc;
>>  }
>>  
>> +/* This function should only be called if LWP got a SYSCALL_SIGTRAP.
>> +   Fill *SYSNO with the syscall nr trapped.  Fill *SYSRET with the
>> +   return code.  */
>> +
>> +static void
>> +get_syscall_trapinfo (struct lwp_info *lwp, int *sysno, int *sysret)
>> +{
>> +  struct thread_info *saved_thread;
>> +  struct regcache *regcache;
>> +
>> +  if (the_low_target.get_syscall_trapinfo == NULL)
>> +    {
>> +      /* If we cannot get the syscall trapinfo, report an unknown
>> +	 system call number and -ENOSYS return value.  */
>> +      *sysno = UNKNOWN_SYSCALL;
>> +      *sysret = -ENOSYS;
>> +      return;
>> +    }
>> +
>> +  saved_thread = current_thread;
>> +  current_thread = get_lwp_thread (lwp);
>> +
>> +  regcache = get_thread_regcache (current_thread, 1);
>> +  (*the_low_target.get_syscall_trapinfo) (regcache, sysno, sysret);
>> +
>> +  if (debug_threads)
>> +    {
>> +      debug_printf ("get_syscall_trapinfo sysno %d sysret %d\n",
>> +		    *sysno, *sysret);
>> +    }
>> +
>> +  current_thread = saved_thread;
>> +}
>> +
>>  /* This function should only be called if LWP got a SIGTRAP.
>>     The SIGTRAP could mean several things.
>>  
>> @@ -2154,6 +2198,8 @@ linux_low_ptrace_options (int attached)
>>    if (report_exec_events)
>>      options |= PTRACE_O_TRACEEXEC;
>>  
>> +  options |= PTRACE_O_TRACESYSGOOD;
>> +
>>    return options;
>>  }
>>  
>> @@ -2249,6 +2295,16 @@ linux_low_filter_event (int lwpid, int wstat)
>>  
>>    gdb_assert (WIFSTOPPED (wstat));
>>  
>> +  /* Set ptrace flags ASAP, so no events can be missed.  */
>> +  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
> 
> It's a bit weird to check WIFSTOPPED after we just asserted it's true,
> but I realize all the subsequent "if"s checks WIFSTOPPED too.

I agree it's weird, but as you say it's the local pattern.

>> +    {
>> +      struct process_info *proc = find_process_pid (pid_of (thread));
>> +      int options = linux_low_ptrace_options (proc->attached);
>> +
>> +      linux_enable_event_reporting (lwpid, options);
>> +      child->must_set_ptrace_flags = 0;
>> +    }
>> +
> 
> Is moving the must_set_ptrace_flags check up to here good in general,
> or is it only necessary for this patch?
> I see that gdb/linux-nat.c does the must_set_ptrace_flags check early.
> [ref: gdb/linux.c line 2312: if (lp->must_set_ptrace_flags)]
> If this part of patch can be separated out, I think that'd be helpful.

Before I moved this, I was getting my first syscall trap as a plain
SIGTRAP, indicating PTRACE_O_TRACESYSGOOD wasn't set as desired.  I
finally figured out that the following block with child->status_pending
was returning early, so must_set_ptrace_flags wasn't even checked.

It might even by my own peculiar setup though, as I was also testing
this with my own strace-over-gdbserver hack.  I'm sure there are ways
that doesn't behave like a normal gdb client.

Anyway, I think it's probably a good change in general, especially to
mirror gdb's own behavior.  In practice it may only matter for syscalls
or the weird way I was using it, but that's ok.  It'll be easy to make
this piece a patch preceding the rest.

>>    if (WIFSTOPPED (wstat))
>>      {
>>        struct process_info *proc;
>> @@ -2276,13 +2332,19 @@ linux_low_filter_event (int lwpid, int wstat)
>>  	}
>>      }
>>  
>> -  if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
>> +  /* Always update syscall_state, even if it will be filtered later.  */
>> +  if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SYSCALL_SIGTRAP)
>>      {
>> -      struct process_info *proc = find_process_pid (pid_of (thread));
>> -      int options = linux_low_ptrace_options (proc->attached);
>> -
>> -      linux_enable_event_reporting (lwpid, options);
>> -      child->must_set_ptrace_flags = 0;
>> +      child->syscall_state =
>> +	child->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
>> +	? TARGET_WAITKIND_SYSCALL_RETURN
>> +	: TARGET_WAITKIND_SYSCALL_ENTRY;
> 
> Our convention when wrapping on "=" is to put the "=" on the next line.
> Also, I'd put the expression in parens like this:
> 
>       child->syscall_state
> 	= (child->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY
> 	   ? TARGET_WAITKIND_SYSCALL_RETURN
> 	   : TARGET_WAITKIND_SYSCALL_ENTRY);
> 
>> +    }
>> +  else
>> +    {
>> +      /* Almost all other ptrace-stops are known to be outside of system
>> +	 calls, with further exceptions in handle_extended_wait.  */
>> +      child->syscall_state = TARGET_WAITKIND_IGNORE;
>>      }
>>  
>>    /* Be careful to not overwrite stop_pc until
>> @@ -2885,6 +2947,44 @@ ignore_event (struct target_waitstatus *ourstatus)
>>    return null_ptid;
>>  }
>>  
>> +/* Returns 1 if GDB is interested in any event_child syscalls.  */
>> +
>> +static int
>> +gdb_catching_syscalls_p (struct lwp_info *event_child)
>> +{
>> +  struct thread_info *thread = get_lwp_thread (event_child);
>> +  struct process_info *proc = get_thread_process (thread);
>> +
>> +  return !VEC_empty (int, proc->syscalls_to_catch);
>> +}
>> +
>> +/* Returns 1 if GDB is interested in the event_child syscall.
>> +   Only to be called when stopped reason is SYSCALL_SIGTRAP.  */
>> +
>> +static int
>> +gdb_catch_this_syscall_p (struct lwp_info *event_child)
>> +{
>> +  int i, iter;
>> +  int sysno, sysret;
>> +  struct thread_info *thread = get_lwp_thread (event_child);
>> +  struct process_info *proc = get_thread_process (thread);
>> +
>> +  if (VEC_empty (int, proc->syscalls_to_catch))
>> +    return 0;
>> +
>> +  if (VEC_index (int, proc->syscalls_to_catch, 0) == ANY_SYSCALL)
>> +    return 1;
>> +
>> +  get_syscall_trapinfo (event_child, &sysno, &sysret);
>> +  for (i = 0;
>> +       VEC_iterate (int, proc->syscalls_to_catch, i, iter);
>> +       i++)
>> +    if (iter == sysno)
>> +      return 1;
>> +
>> +  return 0;
>> +}
>> +
>>  /* Wait for process, returns status.  */
>>  
>>  static ptid_t
>> @@ -3195,6 +3295,22 @@ linux_wait_1 (ptid_t ptid,
>>  
>>    /* Check whether GDB would be interested in this event.  */
>>  
>> +  /* Check if GDB is interested in this syscall.  */
>> +  if (WIFSTOPPED (w)
>> +      && WSTOPSIG (w) == SYSCALL_SIGTRAP
>> +      && !gdb_catch_this_syscall_p (event_child))
>> +    {
>> +      if (debug_threads)
>> +	{
>> +	  debug_printf ("Ignored syscall for LWP %ld.\n",
>> +			lwpid_of (current_thread));
>> +	}
>> +
>> +      linux_resume_one_lwp (event_child, event_child->stepping,
>> +			    0, NULL);
>> +      return ignore_event (ourstatus);
>> +    }
>> +
>>    /* If GDB is not interested in this signal, don't stop other
>>       threads, and don't report it to GDB.  Just resume the inferior
>>       right away.  We do this for threading-related signals as well as
>> @@ -3449,8 +3565,16 @@ linux_wait_1 (ptid_t ptid,
>>  	}
>>      }
>>  
>> -  if (current_thread->last_resume_kind == resume_stop
>> -      && WSTOPSIG (w) == SIGSTOP)
>> +  if (WSTOPSIG (w) == SYSCALL_SIGTRAP)
>> +    {
>> +      int sysret;
>> +
>> +      get_syscall_trapinfo (event_child,
>> +			    &ourstatus->value.syscall_number, &sysret);
>> +      ourstatus->kind = event_child->syscall_state;
>> +    }
>> +  else if (current_thread->last_resume_kind == resume_stop
>> +	   && WSTOPSIG (w) == SIGSTOP)
>>      {
>>        /* A thread that has been requested to stop by GDB with vCont;t,
>>  	 and it stopped cleanly, so report as SIG0.  The use of
>> @@ -3867,6 +3991,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
>>    struct thread_info *thread = get_lwp_thread (lwp);
>>    struct thread_info *saved_thread;
>>    int fast_tp_collecting;
>> +  int ptrace_request;
>>    struct process_info *proc = get_thread_process (thread);
>>  
>>    /* Note that target description may not be initialised
>> @@ -4052,7 +4177,14 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
>>    regcache_invalidate_thread (thread);
>>    errno = 0;
>>    lwp->stepping = step;
>> -  ptrace (step ? PTRACE_SINGLESTEP : PTRACE_CONT, lwpid_of (thread),
>> +  if (step)
>> +    ptrace_request = PTRACE_SINGLESTEP;
>> +  else if (gdb_catching_syscalls_p (lwp))
>> +    ptrace_request =  PTRACE_SYSCALL;
> 
> Unnecessary extra space after =.
> 
>> +  else
>> +    ptrace_request =  PTRACE_CONT;
> 
> Ditto.
> 
>> +  ptrace (ptrace_request,
>> +	  lwpid_of (thread),
>>  	  (PTRACE_TYPE_ARG3) 0,
>>  	  /* Coerce to a uintptr_t first to avoid potential gcc warning
>>  	     of coercing an 8 byte integer to a 4 byte pointer.  */
>> @@ -6135,6 +6267,13 @@ linux_process_qsupported (const char *query)
>>  }
>>  
>>  static int
>> +linux_supports_catch_syscall (void)
>> +{
>> +  return (the_low_target.get_syscall_trapinfo != NULL
>> +	  && linux_supports_tracesysgood());
>> +}
>> +
>> +static int
>>  linux_supports_tracepoints (void)
>>  {
>>    if (*the_low_target.supports_tracepoints == NULL)
>> @@ -7010,6 +7149,7 @@ static struct target_ops linux_target_ops = {
>>    linux_common_core_of_thread,
>>    linux_read_loadmap,
>>    linux_process_qsupported,
>> +  linux_supports_catch_syscall,
>>    linux_supports_tracepoints,
>>    linux_read_pc,
>>    linux_write_pc,
>> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
>> index ccf4c9421d87..71d72f48ea3a 100644
>> --- a/gdb/gdbserver/linux-low.h
>> +++ b/gdb/gdbserver/linux-low.h
>> @@ -201,6 +201,12 @@ struct linux_target_ops
>>    /* Hook to support target specific qSupported.  */
>>    void (*process_qsupported) (const char *);
>>  
>> +  /* Fill *SYSNO with the syscall nr trapped.  Fill *SYSRET with the
>> +     return code.  Only to be called when inferior is stopped
>> +     due to SYSCALL_SIGTRAP.  */
>> +  void (*get_syscall_trapinfo) (struct regcache *regcache,
>> +				int *sysno, int *sysret);
>> +
>>    /* Returns true if the low target supports tracepoints.  */
>>    int (*supports_tracepoints) (void);
>>  
>> @@ -271,6 +277,13 @@ struct lwp_info
>>       event already received in a wait()).  */
>>    int stopped;
>>  
>> +  /* Signal wether we are in a SYSCALL_ENTRY or
>> +     in a SYSCALL_RETURN event.
>> +     Values:
>> +     - TARGET_WAITKIND_SYSCALL_ENTRY
>> +     - TARGET_WAITKIND_SYSCALL_RETURN */
>> +  enum target_waitkind syscall_state;
>> +
>>    /* When stopped is set, the last wait status recorded for this lwp.  */
>>    int last_status;
>>  
>> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
>> index 89ec4e54b3b8..6e3c589de8fb 100644
>> --- a/gdb/gdbserver/linux-x86-low.c
>> +++ b/gdb/gdbserver/linux-x86-low.c
>> @@ -1432,6 +1432,36 @@ x86_arch_setup (void)
>>    current_process ()->tdesc = x86_linux_read_description ();
>>  }
>>  
>> +/* Fill *SYSNO and *SYSRET with the syscall nr trapped and the syscall return
>> +   code.  This should only be called if LWP got a SYSCALL_SIGTRAP.  */
>> +
>> +static void
>> +x86_get_syscall_trapinfo (struct regcache *regcache, int *sysno, int *sysret)
>> +{
>> +  int use_64bit = register_size (regcache->tdesc, 0) == 8;
>> +
>> +  if (use_64bit)
>> +    {
>> +      long l_sysno;
>> +      long l_sysret;
>> +
>> +      collect_register_by_name (regcache, "orig_rax", &l_sysno);
>> +      collect_register_by_name (regcache, "rax", &l_sysret);
>> +      *sysno = (int) l_sysno;
>> +      *sysret = (int) l_sysret;
>> +    }
>> +  else
>> +    {
>> +      int l_sysno;
>> +      int l_sysret;
>> +
>> +      collect_register_by_name (regcache, "orig_eax", &l_sysno);
>> +      collect_register_by_name (regcache, "eax", &l_sysret);
>> +      *sysno = (int) l_sysno;
>> +      *sysret = (int) l_sysret;
> 
> These casts are confusing, especially when coupled with the l_ prefix
> ("l" for "long" is the first thing that comes to mind, but these are ints).
> I can appreciate them when casting from long to int, but here we
> already have ints.

Ah, yeah, I think this is simply duped from the 64bit block.

> How about changing the parameters to sysno_ptr, sysret_ptr,
> and change l_sysno,l_sysret to sysno,sysret?
> Or some such.

I think we can just drop the locals entirely for this block and just
collect the registers directly into the sysno/sysret parameters.

>> +    }
>> +}
>> +
>>  static int
>>  x86_supports_tracepoints (void)
>>  {
>> @@ -3292,6 +3322,7 @@ struct linux_target_ops the_low_target =
>>    x86_linux_new_fork,
>>    x86_linux_prepare_to_resume,
>>    x86_linux_process_qsupported,
>> +  x86_get_syscall_trapinfo,
>>    x86_supports_tracepoints,
>>    x86_get_thread_area,
>>    x86_install_fast_tracepoint_jump_pad,
>> diff --git a/gdb/gdbserver/nto-low.c b/gdb/gdbserver/nto-low.c
>> index d72c46515d13..1301acea49b7 100644
>> --- a/gdb/gdbserver/nto-low.c
>> +++ b/gdb/gdbserver/nto-low.c
>> @@ -979,6 +979,7 @@ static struct target_ops nto_target_ops = {
>>    NULL, /* core_of_thread */
>>    NULL, /* read_loadmap */
>>    NULL, /* process_qsupported */
>> +  NULL, /* supports_catch_syscall */
>>    NULL, /* supports_tracepoints */
>>    NULL, /* read_pc */
>>    NULL, /* write_pc */
>> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
>> index e36609176afa..19eed77d4874 100644
>> --- a/gdb/gdbserver/remote-utils.c
>> +++ b/gdb/gdbserver/remote-utils.c
>> @@ -1119,6 +1119,8 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>>      case TARGET_WAITKIND_VFORKED:
>>      case TARGET_WAITKIND_VFORK_DONE:
>>      case TARGET_WAITKIND_EXECD:
>> +    case TARGET_WAITKIND_SYSCALL_ENTRY:
>> +    case TARGET_WAITKIND_SYSCALL_RETURN:
>>        {
>>  	struct thread_info *saved_thread;
>>  	const char **regp;
>> @@ -1161,6 +1163,16 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>>  	    status->value.execd_pathname = NULL;
>>  	    buf += strlen (buf);
>>  	  }
>> +	else if ((status->kind == TARGET_WAITKIND_SYSCALL_ENTRY)
>> +		 || (status->kind == TARGET_WAITKIND_SYSCALL_RETURN))
>> +	  {
>> +	    enum gdb_signal signal = GDB_SIGNAL_TRAP;
>> +	    const char *event = (status->kind == TARGET_WAITKIND_SYSCALL_ENTRY
>> +				 ? "syscall_entry" : "syscall_return");
>> +
>> +	    sprintf (buf, "T%02x%s:%x;", signal, event,
>> +		     status->value.syscall_number);
>> +	  }
>>  	else
>>  	  sprintf (buf, "T%02x", status->value.sig);
>>  
>> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
>> index fd2804beaa4e..72621f261dfa 100644
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -592,6 +592,52 @@ handle_general_set (char *own_buf)
>>        return;
>>      }
>>  
>> +  if (startswith (own_buf, "QCatchSyscalls:1"))
>> +    {
>> +      const char *p;
>> +      CORE_ADDR sysno;
>> +      struct process_info *process;
>> +
>> +      if (!target_running () || !target_supports_catch_syscall ())
>> +	{
>> +	  write_enn (own_buf);
>> +	  return;
>> +	}
>> +
>> +      process = current_process ();
>> +
>> +      VEC_truncate (int, process->syscalls_to_catch, 0);
>> +
>> +      p = own_buf + strlen("QCatchSyscalls:1");
>> +      if (*p == ';')
>> +	{
>> +	  p += 1;
>> +	  while (*p)
>> +	    {
>> +	      p = decode_address_to_semicolon (&sysno, p);
>> +	      VEC_safe_push (int, process->syscalls_to_catch, (int) sysno);
>> +	    }
>> +	}
>> +      else
>> +	VEC_safe_push (int, process->syscalls_to_catch, ANY_SYSCALL);
>> +
>> +      write_ok (own_buf);
>> +      return;
>> +    }
>> +
>> +  if (strcmp ("QCatchSyscalls:0", own_buf) == 0)
>> +    {
>> +      if (!target_running () || !target_supports_catch_syscall ())
>> +	{
>> +	  write_enn (own_buf);
>> +	  return;
>> +	}
>> +
>> +      VEC_free (int, current_process ()->syscalls_to_catch);
>> +      write_ok (own_buf);
>> +      return;
>> +    }
>> +
>>    if (startswith (own_buf, "QProgramSignals:"))
>>      {
>>        int numsigs = (int) GDB_SIGNAL_LAST, i;
>> @@ -2140,6 +2186,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>>  	       "PacketSize=%x;QPassSignals+;QProgramSignals+",
>>  	       PBUFSIZ - 1);
>>  
>> +      if (target_supports_catch_syscall ())
>> +	strcat (own_buf, ";QCatchSyscalls+");
>> +
>>        if (the_target->qxfer_libraries_svr4 != NULL)
>>  	strcat (own_buf, ";qXfer:libraries-svr4:read+"
>>  		";augmented-libraries-svr4-read+");
>> diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
>> index 96ad4fa58b7c..a8780ebfbc65 100644
>> --- a/gdb/gdbserver/server.h
>> +++ b/gdb/gdbserver/server.h
>> @@ -133,4 +133,10 @@ extern void discard_queued_stop_replies (ptid_t ptid);
>>     as large as the largest register set supported by gdbserver.  */
>>  #define PBUFSIZ 16384
>>  
>> +/* Definition for an unknown syscall, used basically in error-cases.  */
>> +#define UNKNOWN_SYSCALL (-1)
>> +
>> +/* Definition for any syscall, used for unfiltered syscall reporting.  */
>> +#define ANY_SYSCALL (-2)
>> +
>>  #endif /* SERVER_H */
>> diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c
>> index 89bed7a02dd1..55ec0caef296 100644
>> --- a/gdb/gdbserver/spu-low.c
>> +++ b/gdb/gdbserver/spu-low.c
>> @@ -699,6 +699,7 @@ static struct target_ops spu_target_ops = {
>>    NULL, /* core_of_thread */
>>    NULL, /* read_loadmap */
>>    NULL, /* process_qsupported */
>> +  NULL, /* supports_catch_syscall */
>>    NULL, /* supports_tracepoints */
>>    NULL, /* read_pc */
>>    NULL, /* write_pc */
>> diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
>> index 769c876ede49..ef8a7680ccca 100644
>> --- a/gdb/gdbserver/target.h
>> +++ b/gdb/gdbserver/target.h
>> @@ -309,6 +309,10 @@ struct target_ops
>>    /* Target specific qSupported support.  */
>>    void (*process_qsupported) (const char *);
>>  
>> +  /* Return 1 if the target supports catch syscall, 0 (or leave the
>> +     callback NULL) otherwise.  */
>> +  int (*supports_catch_syscall) (void);
>> +
>>    /* Return 1 if the target supports tracepoints, 0 (or leave the
>>       callback NULL) otherwise.  */
>>    int (*supports_tracepoints) (void);
>> @@ -526,6 +530,10 @@ int kill_inferior (int);
>>  	the_target->process_qsupported (query);		\
>>      } while (0)
>>  
>> +#define target_supports_catch_syscall()              	\
>> +  (the_target->supports_catch_syscall ?			\
>> +   (*the_target->supports_catch_syscall) () : 0)
>> +
>>  #define target_supports_tracepoints()			\
>>    (the_target->supports_tracepoints			\
>>     ? (*the_target->supports_tracepoints) () : 0)
>> diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
>> index 6e33509c0b50..4314513876fa 100644
>> --- a/gdb/gdbserver/win32-low.c
>> +++ b/gdb/gdbserver/win32-low.c
>> @@ -1844,6 +1844,7 @@ static struct target_ops win32_target_ops = {
>>    NULL, /* core_of_thread */
>>    NULL, /* read_loadmap */
>>    NULL, /* process_qsupported */
>> +  NULL, /* supports_catch_syscall */
>>    NULL, /* supports_tracepoints */
>>    NULL, /* read_pc */
>>    NULL, /* write_pc */
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index fed397affeab..41751e9d93b1 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -1388,6 +1388,7 @@ enum {
>>    PACKET_qSupported,
>>    PACKET_qTStatus,
>>    PACKET_QPassSignals,
>> +  PACKET_QCatchSyscalls,
>>    PACKET_QProgramSignals,
>>    PACKET_qCRC,
>>    PACKET_qSearch_memory,
>> @@ -1973,6 +1974,99 @@ remote_pass_signals (struct target_ops *self,
>>      }
>>  }
>>  
>> +/* If 'QCatchSyscalls' is supported, tell the remote stub
>> +   to report syscalls to GDB.  */
>> +
>> +static int
>> +remote_set_syscall_catchpoint (struct target_ops *self,
>> +			       int pid, int needed, int any_count,
>> +			       int table_size, int *table)
>> +{
>> +  char *catch_packet, *p;
>> +  enum packet_result result;
>> +  int n_sysno = 0;
>> +
>> +  if (remote_protocol_packets[PACKET_QCatchSyscalls].support == PACKET_DISABLE)
>> +    {
>> +      /* Not supported.  */
>> +      return 1;
>> +    }
>> +
>> +  if (needed && !any_count)
>> +    {
>> +      int i;
>> +
>> +      /* Count how many syscalls are to be caught (table[sysno] != 0).  */
>> +      for (i = 0; i < table_size; i++)
>> +	{
>> +	  if (table[i])
>> +	    n_sysno++;
>> +	}
>> +    }
>> +
>> +  if (remote_debug)
>> +    {
>> +      fprintf_unfiltered (gdb_stdlog,
>> +			  "remote_set_syscall_catchpoint "
>> +			  "pid %d needed %d any_count %d n_sysno %d\n",
>> +			  pid, needed, any_count, n_sysno);
>> +    }
>> +
>> +  if (needed)
>> +    {
>> +      /* Prepare a packet with the sysno list, assuming max 8+1
>> +	 characters for a sysno.  If the resulting packet size is too
>> +	 big, fallback on the non selective packet.  */
>> +      const int maxpktsz = strlen ("QCatchSyscalls:1") + n_sysno * 9 + 1;
>> +
>> +      catch_packet = xmalloc (maxpktsz);
>> +      strcpy (catch_packet, "QCatchSyscalls:1");
>> +      if (!any_count)
>> +	{
>> +	  int i;
>> +	  char *p;
>> +
>> +	  p = catch_packet;
>> +	  p += strlen (p);
>> +
>> +	  /* Add in catch_packet each syscall to be caught (table[i] != 0).  */
>> +	  for (i = 0; i < table_size; i++)
>> +	    {
>> +	      if (table[i])
>> +		{
>> +		  xsnprintf (p, catch_packet + maxpktsz - p, ";%x", i);
>> +		  p += strlen (p);
>> +		}
>> +	    }
>> +	}
>> +      if (strlen (catch_packet) > get_remote_packet_size ())
>> +	{
>> +	  /* catch_packet too big.  Fallback to less efficient
>> +	     non selective mode, with GDB doing the filtering.  */
>> +	  catch_packet[strlen ("QCatchSyscalls:1")] = 0;
>> +	}
>> +    }
>> +  else
>> +    {
>> +      catch_packet = xmalloc (strlen ("QCatchSyscalls:0") + 1);
>> +      strcpy (catch_packet, "QCatchSyscalls:0");
>> +    }
>> +
>> +  {
>> +    struct remote_state *rs = get_remote_state ();
>> +    char *buf = rs->buf;
>> +
>> +    putpkt (catch_packet);
>> +    getpkt (&rs->buf, &rs->buf_size, 0);
>> +    result = packet_ok (buf, &remote_protocol_packets[PACKET_QCatchSyscalls]);
>> +    xfree (catch_packet);
>> +    if (result == PACKET_OK)
>> +      return 0;
>> +    else
>> +      return -1;
>> +  }
>> +}
>> +
>>  /* If 'QProgramSignals' is supported, tell the remote stub what
>>     signals it should pass through to the inferior when detaching.  */
>>  
>> @@ -4328,6 +4422,8 @@ static const struct protocol_feature remote_protocol_features[] = {
>>      PACKET_qXfer_traceframe_info },
>>    { "QPassSignals", PACKET_DISABLE, remote_supported_packet,
>>      PACKET_QPassSignals },
>> +  { "QCatchSyscalls", PACKET_DISABLE, remote_supported_packet,
>> +    PACKET_QCatchSyscalls },
>>    { "QProgramSignals", PACKET_DISABLE, remote_supported_packet,
>>      PACKET_QProgramSignals },
>>    { "QStartNoAckMode", PACKET_DISABLE, remote_supported_packet,
>> @@ -6159,6 +6255,22 @@ Packet: '%s'\n"),
>>  
>>  	  if (strprefix (p, p1, "thread"))
>>  	    event->ptid = read_ptid (++p1, &p);
>> +	  else if (strprefix (p, p1, "syscall_entry"))
>> +	    {
>> +	      ULONGEST sysno;
>> +
>> +	      event->ws.kind = TARGET_WAITKIND_SYSCALL_ENTRY;
>> +	      p = unpack_varlen_hex (++p1, &sysno);
>> +	      event->ws.value.syscall_number = (int) sysno;
>> +	    }
>> +	  else if (strprefix (p, p1, "syscall_return"))
>> +	    {
>> +	      ULONGEST sysno;
>> +
>> +	      event->ws.kind = TARGET_WAITKIND_SYSCALL_RETURN;
>> +	      p = unpack_varlen_hex (++p1, &sysno);
>> +	      event->ws.value.syscall_number = (int) sysno;
>> +	    }
>>  	  else if (strprefix (p, p1, "watch")
>>  		   || strprefix (p, p1, "rwatch")
>>  		   || strprefix (p, p1, "awatch"))
>> @@ -12734,6 +12846,7 @@ Specify the serial device it is connected to\n\
>>    remote_ops.to_load = remote_load;
>>    remote_ops.to_mourn_inferior = remote_mourn;
>>    remote_ops.to_pass_signals = remote_pass_signals;
>> +  remote_ops.to_set_syscall_catchpoint = remote_set_syscall_catchpoint;
>>    remote_ops.to_program_signals = remote_program_signals;
>>    remote_ops.to_thread_alive = remote_thread_alive;
>>    remote_ops.to_update_thread_list = remote_update_thread_list;
>> @@ -13263,6 +13376,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
>>    add_packet_config_cmd (&remote_protocol_packets[PACKET_QPassSignals],
>>  			 "QPassSignals", "pass-signals", 0);
>>  
>> +  add_packet_config_cmd (&remote_protocol_packets[PACKET_QCatchSyscalls],
>> +			 "QCatchSyscalls", "catch-syscalls", 0);
>> +
>>    add_packet_config_cmd (&remote_protocol_packets[PACKET_QProgramSignals],
>>  			 "QProgramSignals", "program-signals", 0);
>>  
>> diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp b/gdb/testsuite/gdb.base/catch-syscall.exp
>> index c1cfe23cdddb..0ba078db22ac 100644
>> --- a/gdb/testsuite/gdb.base/catch-syscall.exp
>> +++ b/gdb/testsuite/gdb.base/catch-syscall.exp
>> @@ -19,7 +19,15 @@
>>  # It was written by Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>
>>  # on September/2008.
>>  
>> -if { [is_remote target] || ![isnative] } then {
>> +if { ![isnative] } then {
>> +    continue
>> +}
>> +
>> +# This shall be updated whenever QCatchSyscalls packet support is implemented
>> +# on some gdbserver architecture.
>> +if { [is_remote target]
>> +     && ![istarget "x86_64-*-linux*"]
>> +     && ![istarget "i\[34567\]86-*-linux*"] } {
>>      continue
>>  }
>>  
>> @@ -390,7 +398,10 @@ proc do_syscall_tests {} {
>>      if [runto_main] then { test_catch_syscall_skipping_return }
>>  
>>      # Testing the 'catch syscall' command starting mid-vfork.
>> -    if [runto_main] then { test_catch_syscall_mid_vfork }
>> +    # (Only local or extended-remote can use "catch vfork".)
>> +    if { ![is_remote target] || [target_info gdb_protocol] == "extended-remote" } {
>> +	if [runto_main] then { test_catch_syscall_mid_vfork }
>> +    }
>>  
>>      # Testing if the 'catch syscall' command works when switching to
>>      # different architectures on-the-fly (PR gdb/10737).



More information about the Gdb-patches mailing list