From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6984 invoked by alias); 25 Nov 2015 02:37:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 6958 invoked by uid 89); 25 Nov 2015 02:37:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 25 Nov 2015 02:36:56 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 9EEA0C0AA554; Wed, 25 Nov 2015 02:36:54 +0000 (UTC) Received: from [10.3.113.18] ([10.3.113.18]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAP2arHJ024037; Tue, 24 Nov 2015 21:36:53 -0500 Subject: Re: [PATCH] Implement 'catch syscall' for gdbserver To: Doug Evans References: <1446169946-28117-1-git-send-email-jistone@redhat.com> Cc: gdb-patches@sourceware.org, sergiodj@redhat.com, palves@redhat.com, philippe.waroquiers@skynet.be From: Josh Stone Message-ID: <56551EC4.1020905@redhat.com> Date: Wed, 25 Nov 2015 02:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-11/txt/msg00515.txt.bz2 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 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 . >> >> 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 >> >> * 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 >> >> * 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 >> >> * 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 >> >> * 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 >> # 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).