This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFA [PATCH v3] Implement 'catch syscall' for gdbserver
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 27 Sep 2013 17:55:34 -0300
- Subject: Re: RFA [PATCH v3] Implement 'catch syscall' for gdbserver
- Authentication-results: sourceware.org; auth=none
- References: <1379796907 dot 5980 dot 20 dot camel at soleil>
On Saturday, September 21 2013, Philippe Waroquiers wrote:
> This is version 3 of the patch implementing catch syscall for gdbserver.
> This integrates the review comments given by Eli Zaretskii
> and Sergio Durigan Junior.
Thanks!
> Difference with version 2 is doc and code cleanup, renamed various
> variables and functions, added some clarification comments,
> avoid use of catched. No functional change.
Still have some formatting issues, as pointed by Tom on IRC.
> No regression on linux/x86-64, native and gdbserver mode.
> Manually tested with a (patched) Valgrind gdbserver.
>
> Ok to commit ?
>
> Thanks
>
> Philippe
>
> 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.
> * common/linux-ptrace.c (linux_check_ptrace_features):
> Detect PTRACE_O_TRACESYSGOOD for gdbserver.
> (ptrace_supports_feature): Initializes ptrace features if needed.
>
> doc/ChangeLog
> 2013-xx-yy Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * gdb.texinfo (General Query Packets): Document new QCatchSyscalls
> packet.
> (Stop Reply Packets): Document new stop reasons syscall_entry and
> syscall_return.
> (Async Records): Fixed syscall-return item name.
>
> gdbserver/ChangeLog
> 2013-xx-yy Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * target.h (struct target_ops): Add supports_catch_syscall operation.
> * linux-low.h (struct linux_target_ops): Add get_syscall_trapinfo
> operation.
> * linux-low.c (linux_wait_1): Enable, detect and handle
> SYSCALL_SIGTRAP.
> (gdb_catch_this_syscall_p): New function.
> (get_syscall_trapinfo): New function.
> (linux_supports_catch_syscall): New function.
> (struct target_ops linux_target_ops): Set linux_supports_catch_syscall.
> * linux-x86-low.c (x86_get_syscall_trapinfo): New function.
> (struct linux_target_ops the_low_target): Set x86_get_syscall_trapinfo.
> * remote-utils.c (prepare_resume_reply): Handle status kinds
> TARGET_WAITKIND_SYSCALL_ENTRY and TARGET_WAITKIND_SYSCALL_RETURN.
> * server.h: Declare catching_syscalls_p, syscalls_to_catch_size and
> syscalls_to_catch.
> * server.c: Define catching_syscalls_p, syscalls_to_catch_size and
> syscalls_to_catch.
> (handle_general_set): Handle QCatchSyscalls packet.
> (handle_query): Reports if low target supports QCatchSyscalls.
> * win32-low.c (struct target_ops win32_target_op): Sets NULL
> for supports_catch_syscall.
>
> testsuite/ChangeLog
> 2013-xx-yy Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * gdb.base/catch-syscall.exp: Enables test for x86 and amd64
> gdbserver.
>
>
>
> Index: NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.614
> diff -u -p -r1.614 NEWS
> --- NEWS 16 Sep 2013 17:47:29 -0000 1.614
> +++ NEWS 21 Sep 2013 20:38:09 -0000
> @@ -139,11 +139,20 @@ qXfer:libraries-svr4:read's annex
> necessary for library list updating, resulting in significant
> speedup.
>
> +QCatchSyscalls:1 [;SYSNO]...
> +QCatchSyscalls:0
> + Enable ("QCatchSyscalls:1") or disable ("QCatchSyscalls:0")
> + catching syscalls from the inferior process.
> +
> +
> * New features in the GDB remote stub, GDBserver
>
> ** GDBserver now supports target-assisted range stepping. Currently
> enabled on x86/x86_64 GNU/Linux targets.
>
> + ** GDBserver now supports catch syscall. Currently enabled
> + on x86/x86_64 GNU/Linux targets, and in the Valgrind gdbserver.
> +
> ** GDBserver now adds element 'tvar' in the XML in the reply to
> 'qXfer:traceframe-info:read'. It has the id of the collected
> trace state variables.
> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.577
> diff -u -p -r1.577 remote.c
> --- remote.c 23 Aug 2013 13:12:17 -0000 1.577
> +++ remote.c 21 Sep 2013 20:38:10 -0000
> @@ -1340,6 +1340,7 @@ enum {
> PACKET_qSupported,
> PACKET_qTStatus,
> PACKET_QPassSignals,
> + PACKET_QCatchSyscalls,
> PACKET_QProgramSignals,
> PACKET_qSearch_memory,
> PACKET_vAttach,
> @@ -1728,6 +1729,96 @@ remote_pass_signals (int numsigs, unsign
> }
> }
>
> +/* If 'QCatchSyscalls' is supported, tell the remote stub
> + to report syscalls to GDB. */
> +
> +static int
> +remote_set_syscall_catchpoint (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)
> + return 1; /* not supported */
> +
There is excessive whitespaces in the line after "return 1". Also, we
don't put comments in the same line. It would be better to write:
if (condition)
{
/* 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. */
This comment could be expanded to use more characters in each line.
> + const char *q1 = "QCatchSyscalls:1";
> + int i;
> + const int maxpktsz = strlen (q1) + n_sysno * 9 + 1;
> +
> + catch_packet = xmalloc (maxpktsz);
> + strcpy (catch_packet, q1);
> + if (!any_count)
> + {
> + char *p;
> +
> + p = catch_packet;
> + p += strlen(p);
Space between function name and paren.
> +
> + /* 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,
Likewise.
> + ";%x", i);
> + p += strlen(p);
Likewise.
> + }
> + }
> + }
> + if (strlen(catch_packet) > get_remote_packet_size())
Likewise.
> + {
> + /* catch_packet too big. Fallback to less efficient
> + non selective mode, with GDB doing the filtering. */
> + catch_packet[strlen (q1)] = 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. */
>
> @@ -4016,6 +4107,8 @@ static const struct protocol_feature rem
> 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,
> @@ -5284,6 +5377,11 @@ typedef struct stop_reply
> int stopped_by_watchpoint_p;
> CORE_ADDR watch_data_address;
>
> + /* Set to 1 if the stop_reply is for a syscall entry or
> + return. The field ws.value.syscall_number then identifies
> + the syscall. */
> + int syscall;
> +
This one can be removed now with Pedro's patch.
> int solibs_changed;
> int replay_event;
>
> @@ -5546,6 +5644,7 @@ remote_parse_stop_reply (char *buf, stru
> event->ptid = null_ptid;
> event->ws.kind = TARGET_WAITKIND_IGNORE;
> event->ws.value.integer = 0;
> + event->syscall = 0;
Likewise.
> event->solibs_changed = 0;
> event->replay_event = 0;
> event->stopped_by_watchpoint_p = 0;
> @@ -5596,6 +5695,24 @@ Packet: '%s'\n"),
> p, buf);
> if (strncmp (p, "thread", p1 - p) == 0)
> event->ptid = read_ptid (++p1, &p);
> + else if (strncmp (p, "syscall_entry", p1 - p) == 0)
> + {
> + ULONGEST sysno;
> +
> + event->ws.kind = TARGET_WAITKIND_SYSCALL_ENTRY;
> + p = unpack_varlen_hex (++p1, &sysno);
> + event->syscall = 1;
> + event->ws.value.syscall_number = (int) sysno;
> + }
> + else if (strncmp (p, "syscall_return", p1 - p) == 0)
> + {
> + ULONGEST sysno;
> +
> + event->ws.kind = TARGET_WAITKIND_SYSCALL_RETURN;
> + p = unpack_varlen_hex (++p1, &sysno);
> + event->syscall = 1;
> + event->ws.value.syscall_number = (int) sysno;
> + }
> else if ((strncmp (p, "watch", p1 - p) == 0)
> || (strncmp (p, "rwatch", p1 - p) == 0)
> || (strncmp (p, "awatch", p1 - p) == 0))
> @@ -5681,6 +5798,9 @@ Packet: '%s'\n"),
> event->ws.kind = TARGET_WAITKIND_LOADED;
> else if (event->replay_event)
> event->ws.kind = TARGET_WAITKIND_NO_HISTORY;
> + else if (event->syscall)
> + gdb_assert (event->ws.kind == TARGET_WAITKIND_SYSCALL_ENTRY
> + || event->ws.kind == TARGET_WAITKIND_SYSCALL_RETURN);
> else
> {
> event->ws.kind = TARGET_WAITKIND_STOPPED;
> @@ -11452,6 +11572,7 @@ Specify the serial device it is connecte
> remote_ops.to_load = generic_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_find_new_threads = remote_threads_info;
> @@ -11946,6 +12067,9 @@ Show the maximum size of the address (in
> 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);
>
> Index: common/linux-ptrace.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/common/linux-ptrace.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 linux-ptrace.c
> --- common/linux-ptrace.c 28 Aug 2013 14:09:31 -0000 1.12
> +++ common/linux-ptrace.c 21 Sep 2013 20:38:10 -0000
> @@ -361,16 +361,18 @@ linux_check_ptrace_features (void)
> return;
> }
>
> -#ifdef GDBSERVER
> - /* gdbserver does not support PTRACE_O_TRACESYSGOOD or
> - PTRACE_O_TRACEVFORKDONE yet. */
> -#else
> - /* Check if the target supports PTRACE_O_TRACESYSGOOD. */
> + /* Check if the target supports PTRACE_O_TRACESYSGOOD.
> + We keep PTRACE_O_TRACEFORK option activated as a fork
> + event notification is expected after my_waitpid below. */
> ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> - (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> + (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
> + | PTRACE_O_TRACESYSGOOD));
> if (ret == 0)
> current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
>
> +#ifdef GDBSERVER
> + /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet. */
> +#else
> /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
> ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
> @@ -474,6 +476,11 @@ linux_enable_event_reporting (pid_t pid)
> static int
> ptrace_supports_feature (int ptrace_options)
> {
> + /* Check if we have initialized the ptrace features for this
> + target. If not, do it now. */
> + if (current_ptrace_options == -1)
> + linux_check_ptrace_features ();
> +
> gdb_assert (current_ptrace_options >= 0);
>
> return ((current_ptrace_options & ptrace_options) == ptrace_options);
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.1112
> diff -u -p -r1.1112 gdb.texinfo
> --- doc/gdb.texinfo 16 Sep 2013 18:00:34 -0000 1.1112
> +++ doc/gdb.texinfo 21 Sep 2013 20:38:14 -0000
> @@ -18752,6 +18752,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}}
> @@ -38147,6 +38151,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.
> @@ -38517,6 +38526,44 @@ by supplying an appropriate @samp{qSuppo
> 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
> @@ -38880,6 +38927,11 @@ These are the currently defined stub fea
> @tab @samp{-}
> @tab Yes
>
> +@item @samp{QCatchSyscalls}
> +@tab No
> +@tab @samp{-}
> +@tab Yes
> +
> @item @samp{QPassSignals}
> @tab No
> @tab @samp{-}
> @@ -39040,6 +39092,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}).
> Index: gdbserver/linux-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
> retrieving revision 1.248
> diff -u -p -r1.248 linux-low.c
> --- gdbserver/linux-low.c 5 Sep 2013 20:45:39 -0000 1.248
> +++ gdbserver/linux-low.c 21 Sep 2013 20:38:15 -0000
> @@ -74,6 +74,11 @@
> #define W_STOPCODE(sig) ((sig) << 8 | 0x7f)
> #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 the kernel's hard limit. Not to be confused with
> SIGRTMIN. */
> #ifndef __SIGRTMIN
> @@ -481,6 +486,36 @@ get_pc (struct lwp_info *lwp)
> return pc;
> }
>
> +/* This function should only be called if LWP got a SIGTRAP_SYSCALL.
> + 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_inferior;
> + struct regcache *regcache;
> +
> + if (the_low_target.get_syscall_trapinfo == NULL)
> + {
> + *sysno = 0;
> + *sysret = 0;
> + return;
> + }
Just a reminder that you were going to check if -1 is a better value to
represent this case. And if it is, then it should be documented, of course.
> +
> + saved_inferior = current_inferior;
> + current_inferior = get_lwp_thread (lwp);
> +
> + regcache = get_thread_regcache (current_inferior, 1);
> + (*the_low_target.get_syscall_trapinfo) (regcache, sysno, sysret);
> +
> + if (debug_threads)
> + fprintf (stderr, "get_syscall_trapinfo sysno %d sysret %d\n",
> + *sysno, *sysret);
> +
> + current_inferior = saved_inferior;
> +}
> +
> /* This function should only be called if LWP got a SIGTRAP.
> The SIGTRAP could mean several things.
>
> @@ -2248,6 +2283,29 @@ linux_stabilize_threads (void)
> }
> }
>
> +/* Returns 1 if GDB is interested in the event_child syscall.
> + Only to be called when stopped reason is SIGTRAP_SYSCALL. */
> +
> +static int
> +gdb_catch_this_syscall_p (struct lwp_info *event_child)
> +{
> + int i;
> + int sysno, sysret;
> +
> + if (!catching_syscalls_p)
> + return 0;
> +
> + if (syscalls_to_catch_size == 0)
> + return 1;
> +
> + get_syscall_trapinfo (event_child, &sysno, &sysret);
> + for (i = 0; i < syscalls_to_catch_size; i++)
> + if (syscalls_to_catch[i] == sysno)
> + return 1;
> +
> + return 0;
> +}
> +
> /* Wait for process, returns status. */
>
> static ptid_t
> @@ -2529,6 +2587,19 @@ Check if we're already there.\n",
>
> /* 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)
> + fprintf (stderr, "Ignored syscall for LWP %ld.\n",
> + lwpid_of (event_child));
> + linux_resume_one_lwp (event_child, event_child->stepping,
> + 0, NULL);
> + goto retry;
> + }
> +
> /* 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
> @@ -2707,7 +2778,24 @@ Check if we're already there.\n",
>
> ourstatus->kind = TARGET_WAITKIND_STOPPED;
>
> - if (current_inferior->last_resume_kind == resume_stop
> + if (WSTOPSIG (w) == SYSCALL_SIGTRAP)
> + {
> + int sysret;
> +
> + get_syscall_trapinfo (event_child,
> + &ourstatus->value.syscall_number, &sysret);
> + /* Differentiate entry from return using the return code
> + -ENOSYS. This is not ideal as a syscall return from a not
> + implemented syscall will be seen as an entry. However, this
> + resists well to enabling/disabling catch syscall at various
> + moment. A better way to differentiate entry/return in GDB
> + and GDBSERVER would be nice. */
> + if (sysret == -ENOSYS)
> + ourstatus->kind = TARGET_WAITKIND_SYSCALL_ENTRY;
> + else
> + ourstatus->kind = TARGET_WAITKIND_SYSCALL_RETURN;
> + }
> + else if (current_inferior->last_resume_kind == resume_stop
> && WSTOPSIG (w) == SIGSTOP)
> {
> /* A thread that has been requested to stop by GDB with vCont;t,
> @@ -3269,7 +3357,10 @@ lwp %ld wants to get out of fast tracepo
> lwp->stopped = 0;
> lwp->stopped_by_watchpoint = 0;
> lwp->stepping = step;
> - ptrace (step ? PTRACE_SINGLESTEP : PTRACE_CONT, lwpid_of (lwp),
> + ptrace (step ? PTRACE_SINGLESTEP
> + : catching_syscalls_p ? PTRACE_SYSCALL
> + : PTRACE_CONT,
> + lwpid_of (lwp),
Just out of curiosity, have you thought about what I said in the other
patch, i.e., rewriting this piece to avoid the "double-ternary"?
> (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. */
> @@ -5108,6 +5199,13 @@ linux_process_qsupported (const char *qu
> }
>
> 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)
> @@ -5798,6 +5896,7 @@ static struct target_ops linux_target_op
> linux_common_core_of_thread,
> linux_read_loadmap,
> linux_process_qsupported,
> + linux_supports_catch_syscall,
> linux_supports_tracepoints,
> linux_read_pc,
> linux_write_pc,
> Index: gdbserver/linux-low.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/linux-low.h,v
> retrieving revision 1.65
> diff -u -p -r1.65 linux-low.h
> --- gdbserver/linux-low.h 22 Aug 2013 23:46:29 -0000 1.65
> +++ gdbserver/linux-low.h 21 Sep 2013 20:38:15 -0000
> @@ -187,6 +187,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);
>
> Index: gdbserver/linux-x86-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/linux-x86-low.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 linux-x86-low.c
> --- gdbserver/linux-x86-low.c 5 Sep 2013 20:40:58 -0000 1.51
> +++ gdbserver/linux-x86-low.c 21 Sep 2013 20:38:15 -0000
> @@ -1474,6 +1474,33 @@ x86_arch_setup (void)
> current_process ()->tdesc = x86_linux_read_description ();
> }
>
> +static void
> +x86_get_syscall_trapinfo (struct regcache *regcache, int *sysno, int *sysret)
Oh, I forgot in the other review, but this function needs a comment.
> +{
> + 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;
> + }
> +}
> +
> static int
> x86_supports_tracepoints (void)
> {
> @@ -3323,6 +3350,7 @@ struct linux_target_ops the_low_target =
> x86_linux_new_thread,
> 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,
> Index: gdbserver/remote-utils.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 remote-utils.c
> --- gdbserver/remote-utils.c 5 Sep 2013 20:41:55 -0000 1.99
> +++ gdbserver/remote-utils.c 21 Sep 2013 20:38:15 -0000
> @@ -1321,12 +1321,17 @@ prepare_resume_reply (char *buf, ptid_t
> switch (status->kind)
> {
> case TARGET_WAITKIND_STOPPED:
> + case TARGET_WAITKIND_SYSCALL_ENTRY:
> + case TARGET_WAITKIND_SYSCALL_RETURN:
> {
> struct thread_info *saved_inferior;
> const char **regp;
> struct regcache *regcache;
>
> - sprintf (buf, "T%02x", status->value.sig);
> + if (status->kind == TARGET_WAITKIND_STOPPED)
> + sprintf (buf, "T%02x", status->value.sig);
> + else
> + sprintf (buf, "T%02x", SIGTRAP);
> buf += strlen (buf);
>
> saved_inferior = current_inferior;
> @@ -1337,6 +1342,16 @@ prepare_resume_reply (char *buf, ptid_t
>
> regcache = get_thread_regcache (current_inferior, 1);
>
> + if (status->kind == TARGET_WAITKIND_SYSCALL_ENTRY
> + || status->kind == TARGET_WAITKIND_SYSCALL_RETURN)
> + {
> + sprintf (buf, "%s:%x;",
> + status->kind == TARGET_WAITKIND_SYSCALL_ENTRY
> + ? "syscall_entry" : "syscall_return",
> + status->value.syscall_number);
> + buf += strlen (buf);
> + }
> +
> if (the_target->stopped_by_watchpoint != NULL
> && (*the_target->stopped_by_watchpoint) ())
> {
> Index: gdbserver/server.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
> retrieving revision 1.201
> diff -u -p -r1.201 server.c
> --- gdbserver/server.c 18 Sep 2013 01:59:59 -0000 1.201
> +++ gdbserver/server.c 21 Sep 2013 20:38:15 -0000
> @@ -74,6 +74,9 @@ int debug_threads;
> int debug_hw_points;
>
> int pass_signals[GDB_SIGNAL_LAST];
> +int catching_syscalls_p;
> +int syscalls_to_catch_size;
> +int *syscalls_to_catch;
> int program_signals[GDB_SIGNAL_LAST];
> int program_signals_p;
>
> @@ -511,6 +514,46 @@ handle_general_set (char *own_buf)
> return;
> }
>
> + if (strncmp ("QCatchSyscalls:1", own_buf, strlen ("QCatchSyscalls:1")) == 0)
> + {
> + int i;
> + const char *p;
> + CORE_ADDR sysno;
> +
> + catching_syscalls_p = 1;
> + if (syscalls_to_catch != NULL)
> + {
> + xfree (syscalls_to_catch);
> + syscalls_to_catch = NULL;
> + }
> + syscalls_to_catch_size = 0;
> + p = own_buf + strlen("QCatchSyscalls:1");
> + while (*p)
> + {
> + if (*p++ == ';')
> + syscalls_to_catch_size++;
> + }
> + if (syscalls_to_catch_size > 0)
> + {
> + syscalls_to_catch = xmalloc (syscalls_to_catch_size * sizeof (int));
> + p = strchr(own_buf, ';') + 1;
> + for (i = 0; i < syscalls_to_catch_size; i++)
> + {
> + p = decode_address_to_semicolon (&sysno, p);
> + syscalls_to_catch[i] = (int) sysno;
> + }
> + }
> + strcpy (own_buf, "OK");
> + return;
> + }
> +
> + if (strcmp ("QCatchSyscalls:0", own_buf) == 0)
> + {
> + catching_syscalls_p = 0;
> + strcpy (own_buf, "OK");
> + return;
> + }
> +
> if (strncmp ("QProgramSignals:", own_buf, strlen ("QProgramSignals:")) == 0)
> {
> int numsigs = (int) GDB_SIGNAL_LAST, i;
> @@ -1744,6 +1787,9 @@ handle_query (char *own_buf, int packet_
> "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+");
> Index: gdbserver/server.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
> retrieving revision 1.118
> diff -u -p -r1.118 server.h
> --- gdbserver/server.h 5 Sep 2013 20:45:39 -0000 1.118
> +++ gdbserver/server.h 21 Sep 2013 20:38:15 -0000
> @@ -115,6 +115,15 @@ extern int server_waiting;
> extern int debug_threads;
> extern int debug_hw_points;
> extern int pass_signals[];
> +
> +/* Set to 1 if GDB is catching some (or all) syscalls, zero otherwise. */
> +extern int catching_syscalls_p;
> +/* syscalls_to_catch is the list of syscalls to report to GDB.
> + If catching_syscalls_p and syscalls_to_catch == NULL, it means
> + all syscalls must be reported. */
> +extern int syscalls_to_catch_size;
> +extern int *syscalls_to_catch;
> +
> extern int program_signals[];
> extern int program_signals_p;
>
> Index: gdbserver/target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
> retrieving revision 1.71
> diff -u -p -r1.71 target.h
> --- gdbserver/target.h 5 Sep 2013 20:41:22 -0000 1.71
> +++ gdbserver/target.h 21 Sep 2013 20:38:15 -0000
> @@ -268,6 +268,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);
> @@ -414,6 +418,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)
> Index: gdbserver/win32-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 win32-low.c
> --- gdbserver/win32-low.c 5 Sep 2013 20:45:39 -0000 1.69
> +++ gdbserver/win32-low.c 21 Sep 2013 20:38:16 -0000
> @@ -1824,6 +1824,7 @@ static struct target_ops win32_target_op
> NULL, /* core_of_thread */
> NULL, /* read_loadmap */
> NULL, /* process_qsupported */
> + NULL, /* supports_catch_syscall */
> NULL, /* supports_tracepoints */
> NULL, /* read_pc */
> NULL, /* write_pc */
> Index: testsuite/gdb.base/catch-syscall.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/catch-syscall.exp,v
> retrieving revision 1.19
> diff -u -p -r1.19 catch-syscall.exp
> --- testsuite/gdb.base/catch-syscall.exp 22 Aug 2013 20:32:54 -0000 1.19
> +++ testsuite/gdb.base/catch-syscall.exp 21 Sep 2013 20:38:16 -0000
> @@ -19,7 +19,7 @@
> # 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
> }
>
> @@ -28,6 +28,14 @@ if {![istarget "hppa*-hp-hpux*"] && ![is
> 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
> +}
> +
> # This shall be updated whenever 'catch syscall' is implemented
> # on some architecture.
> #if { ![istarget "i\[34567\]86-*-linux*"]
--
Sergio