This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Implement 'catch syscall' for gdbserver
- From: Doug Evans <xdje42 at gmail dot com>
- To: Josh Stone <jistone at redhat dot com>
- Cc: gdb-patches at sourceware dot org, sergiodj at redhat dot com, palves at redhat dot com, philippe dot waroquiers at skynet dot be
- Date: Sun, 22 Nov 2015 20:18:51 -0800
- Subject: Re: [PATCH] Implement 'catch syscall' for gdbserver
- Authentication-results: sourceware.org; auth=none
- References: <1446169946-28117-1-git-send-email-jistone at redhat dot com>
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.
> /* 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.
> + {
> + 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.
> 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.
How about changing the parameters to sysno_ptr, sysret_ptr,
and change l_sysno,l_sysret to sysno,sysret?
Or some such.
> + }
> +}
> +
> 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).