[v3,2/2] Implement 'catch syscall' for gdbserver
Commit Message
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.
gdb/ChangeLog:
2015-12-03 Josh Stone <jistone@redhat.com>
Philippe Waroquiers <philippe.waroquiers@skynet.be>
* 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.
* linux-nat.h (struct lwp_info) <syscall_state>: Comment typo.
gdb/doc/ChangeLog:
2015-12-03 Josh Stone <jistone@redhat.com>
Philippe Waroquiers <philippe.waroquiers@skynet.be>
* 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-12-03 Josh Stone <jistone@redhat.com>
Philippe Waroquiers <philippe.waroquiers@skynet.be>
* 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_return 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 (handle_extended_wait): Mark syscall_state as an entry.
(get_syscall_trapinfo): New function, proxy to the_low_target.
(linux_low_ptrace_options): Enable PTRACE_O_TRACESYSGOOD.
(linux_low_filter_event): Toggle syscall_state entry/return for
syscall traps, and set it ignored for all others.
(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-12-03 Josh Stone <jistone@redhat.com>
Philippe Waroquiers <philippe.waroquiers@skynet.be>
* 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 | 140 ++++++++++++++++++++++++++++++-
gdb/gdbserver/linux-low.h | 13 +++
gdb/gdbserver/linux-x86-low.c | 26 ++++++
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/linux-nat.h | 2 +-
gdb/remote.c | 116 +++++++++++++++++++++++++
gdb/testsuite/gdb.base/catch-syscall.exp | 15 +++-
17 files changed, 456 insertions(+), 6 deletions(-)
Comments
> From: Josh Stone <jistone@redhat.com>
> Cc: philippe.waroquiers@skynet.be, sergiodj@redhat.com, palves@redhat.com, eliz@gnu.org, xdje42@gmail.com, Josh Stone <jistone@redhat.com>
> Date: Thu, 3 Dec 2015 18:26:46 -0800
>
> 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.
>
> gdb/ChangeLog:
>
> 2015-12-03 Josh Stone <jistone@redhat.com>
> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * 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.
> * linux-nat.h (struct lwp_info) <syscall_state>: Comment typo.
>
> gdb/doc/ChangeLog:
>
> 2015-12-03 Josh Stone <jistone@redhat.com>
> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * 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.
Any changes since v2 in the documentation parts that require a fresh
review?
Thanks.
Quick question: What is supposed to happen to the QCatchSyscalls
when the process execs? I'm thinking of 64-bit inferior execing
32-bit inferior, etc. The syscall numbers aren't necessarily shared
between the different architectures. This implementation deletes discards
the previous QCatchSyscalls on exec, and I think that's what makes sense,
but, I think that this should be explicit in the packet's description.
I'm not sure gdb clears the inferior's "syscalls to the caught" VEC
on exec events, but it probably does (if it doesn't, I think it should.)
On 12/04/2015 02:26 AM, Josh Stone wrote:
> 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.
>
> gdb/ChangeLog:
>
> 2015-12-03 Josh Stone <jistone@redhat.com>
> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * 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.
> * linux-nat.h (struct lwp_info) <syscall_state>: Comment typo.
>
> gdb/doc/ChangeLog:
>
> 2015-12-03 Josh Stone <jistone@redhat.com>
> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * 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-12-03 Josh Stone <jistone@redhat.com>
> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * 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_return 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 (handle_extended_wait): Mark syscall_state as an entry.
> (get_syscall_trapinfo): New function, proxy to the_low_target.
> (linux_low_ptrace_options): Enable PTRACE_O_TRACESYSGOOD.
> (linux_low_filter_event): Toggle syscall_state entry/return for
> syscall traps, and set it ignored for all others.
> (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-12-03 Josh Stone <jistone@redhat.com>
> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> * 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 | 140 ++++++++++++++++++++++++++++++-
> gdb/gdbserver/linux-low.h | 13 +++
> gdb/gdbserver/linux-x86-low.c | 26 ++++++
> 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/linux-nat.h | 2 +-
> gdb/remote.c | 116 +++++++++++++++++++++++++
> gdb/testsuite/gdb.base/catch-syscall.exp | 15 +++-
> 17 files changed, 456 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a222dfb491f0..1917523d8249 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -112,6 +112,11 @@ N stop reply
> threads are stopped). The remote stub reports support for this stop
> reply to GDB's qSupported query.
>
> +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.
> @@ -127,6 +132,11 @@ show remote exec-event-feature-packet
> The reply to qXfer:threads:read may now include a name attribute for each
> thread.
>
> +* 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
>
Please mention the new set/show commands as well.
> +@item QCatchSyscalls
> +The remote stub understands the @samp{QCatchSyscalls} packet
> +(@pxref{QCatchSyscalls}).
> +
> @item QPassSignals
> The remote stub understands the @samp{QPassSignals} packet
> (@pxref{QPassSignals}).
You also need to list the new commands in the table below:
For each packet @var{name}, the command to enable or disable the
packet is @code{set remote @var{name}-packet}. The available settings
are:
> /* 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;
"just" here means that it's a list with a single element that
happens to be ANY_SYSCALL; I think it's helpful for the reader to
be explicit and expand the "or just ANY_SYSCALL" comment little bit.
> +
> const struct target_desc *tdesc;
>
> /* Private target data. */
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 5e2dc5857a8d..cceede6fe408 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) (char **, int count);
>
> + /* 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);
> +
As you're not putting this new callback at the end of the struct,
you need to install a NULL callback in all linux-*-low.c files (all
archs) that install linux_target_ops methods after process_qsupported.
Otherwise you'll break their builds.
> /* Returns true if the low target supports tracepoints. */
> int (*supports_tracepoints) (void);
>
> @@ -277,6 +283,13 @@ struct lwp_info
> event already received in a wait()). */
> int stopped;
>
> + /* Signal whether 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 18adf5ee7d17..1caf71edf485 100644
> @@ -1172,6 +1174,16 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>
> sprintf (buf, "T%02xcreate:;", signal);
> }
> + else if ((status->kind == TARGET_WAITKIND_SYSCALL_ENTRY)
> + || (status->kind == TARGET_WAITKIND_SYSCALL_RETURN))
Unnecessary parens.
> + {
> + 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 6d151ee35d39..e63656b9c0f6 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -597,6 +597,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;
> + }
I think you should merge this with
> + if (strcmp ("QCatchSyscalls:0", own_buf) == 0)
> + {
> + if (!target_running () || !target_supports_catch_syscall ())
> + {
> + write_enn (own_buf);
> + return;
> + }
into a single
if (startswith (own_buf, "QCatchSyscalls:")
{
...
if (!target_running () || !target_supports_catch_syscall ())
{
write_enn (own_buf);
return;
}
See "QNonStop:" handling. The end result is that "QCatchSyscalls:2"
is treated as an error instead of returning the empty packet.
> +
> + process = current_process ();
> +
> + VEC_truncate (int, process->syscalls_to_catch, 0);
> +
> + p = own_buf + strlen("QCatchSyscalls:1");
> + if (*p == ';')
> + {
> + p += 1;
> + while (*p)
while (*p != '\0)
> + {
> + 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;
> @@ -2219,6 +2265,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 dc0361fdcd14..bc2abd5f50c8 100644
> + 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])
if (table[i] != 0)
just like the comment. :-)
> + 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. */
"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])
Likewise.
> + {
> + xsnprintf (p, catch_packet + maxpktsz - p, ";%x", i);
> + p += strlen (p);
This looks like:
len = xsnprintf (p, catch_packet + maxpktsz - p, ";%x", i);
p += len;
> + }
> + }
> + }
> + 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;
catch_packet[sizeof ("QCatchSyscalls:1") - 1] = 0;
> + }
> + }
> + else
> + {
> + catch_packet = xmalloc (strlen ("QCatchSyscalls:0") + 1);
> + strcpy (catch_packet, "QCatchSyscalls:0");
catch_packet = xstrdup ("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]);
getpkt takes a pointer to pointer because it may xrealloc 'rs->buf'.
So this 'buf' reference may be dangling. Just drop the 'buf' local.
> + xfree (catch_packet);
putpkt/getpkt may throw. This should be freed with a cleanup instead.
> + 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. */
>
> 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
> }
This check won't reach the "continue" when testing
with --target_board=native-extended-gdbserver on some arch that doesn't
implement QCatchSyscall yet. That board is not "is_remote".
I'd actually favor dropping the arch check altogether. Otherwise it's very
likely that arch maintainers won't ever notice there's a new method they
can wire in.
>
> @@ -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" } {
I think IWBN to add a supports_catch_fork proc to lib/gdb.exp. These
checks are getting duplicated (and not sure they are consistent) in
several places. E.g., here, your now catch-fork-static.exp test, the
test that was based on, etc...
> + 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).
Thanks,
Pedro Alves
On 12/04/2015 12:44 AM, Eli Zaretskii wrote:
>> From: Josh Stone <jistone@redhat.com>
>> Cc: philippe.waroquiers@skynet.be, sergiodj@redhat.com, palves@redhat.com, eliz@gnu.org, xdje42@gmail.com, Josh Stone <jistone@redhat.com>
>> Date: Thu, 3 Dec 2015 18:26:46 -0800
>>
>> 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.
>>
>> gdb/ChangeLog:
>>
>> 2015-12-03 Josh Stone <jistone@redhat.com>
>> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * 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.
>> * linux-nat.h (struct lwp_info) <syscall_state>: Comment typo.
>>
>> gdb/doc/ChangeLog:
>>
>> 2015-12-03 Josh Stone <jistone@redhat.com>
>> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * 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.
>
> Any changes since v2 in the documentation parts that require a fresh
> review?
I made the changes you requested in v2 -- v3 just has some
merge-resolution in NEWS. But Pedro has noted some more things to
document for the next round.
On 12/04/2015 05:18 AM, Pedro Alves wrote:
> Quick question: What is supposed to happen to the QCatchSyscalls
> when the process execs? I'm thinking of 64-bit inferior execing
> 32-bit inferior, etc. The syscall numbers aren't necessarily shared
> between the different architectures. This implementation deletes discards
> the previous QCatchSyscalls on exec, and I think that's what makes sense,
> but, I think that this should be explicit in the packet's description.
Yes, I think it should be cleared to avoid any assumption about the
architecture. I'll add a note in the description codifying this.
> I'm not sure gdb clears the inferior's "syscalls to the caught" VEC
> on exec events, but it probably does (if it doesn't, I think it should.)
I'll see if I can find out.
Something else I just realized - my syscall_state accounting gets lost
when an exec creates a new lwp_info. GDB is sending another
QCatchSyscalls for the "new" process, and then the exec return is
thought to be a syscall_entry. That's easy enough to correct, but I
should write a new test for this too. (and see what gdb alone does...)
On a total tangent, is it ever possible that GDB/GDBserver might try to
read and modify registers from a PTRACE_EVENT stop? If so, you should
beware that registers may actually be in flux. I ran into this with
Dyninst, which I fixed here[1] though I can't find the discussion now.
The gist was that in a PTRACE_EVENT, the kernel may not have written the
return register yet. Dyninst wanted to save registers, resume in a bit
of instrumentation code, then restore registers and resume the normal
program. So the saved registers got an intermediate RAX, and when it
resumed into instrumentation the kernel finally wrote the good RAX
return value to complete the syscall (which the instrumentation
ignored). Then when dyninst restored registers the bad RAX was written
back, and thus the normal program code didn't get the correct value for
its fork return. My solution was to step out of the event with
PTRACE_SYSCALL before doing anything else.
[1]
http://git.dyninst.org/?p=dyninst.git;a=commit;h=b89ea1d19677fa0dd9c605ef492c5f6dabf15752
> On 12/04/2015 02:26 AM, Josh Stone wrote:
>> 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.
>>
>> gdb/ChangeLog:
>>
>> 2015-12-03 Josh Stone <jistone@redhat.com>
>> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * 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.
>> * linux-nat.h (struct lwp_info) <syscall_state>: Comment typo.
>>
>> gdb/doc/ChangeLog:
>>
>> 2015-12-03 Josh Stone <jistone@redhat.com>
>> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * 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-12-03 Josh Stone <jistone@redhat.com>
>> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * 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_return 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 (handle_extended_wait): Mark syscall_state as an entry.
>> (get_syscall_trapinfo): New function, proxy to the_low_target.
>> (linux_low_ptrace_options): Enable PTRACE_O_TRACESYSGOOD.
>> (linux_low_filter_event): Toggle syscall_state entry/return for
>> syscall traps, and set it ignored for all others.
>> (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-12-03 Josh Stone <jistone@redhat.com>
>> Philippe Waroquiers <philippe.waroquiers@skynet.be>
>>
>> * 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 | 140 ++++++++++++++++++++++++++++++-
>> gdb/gdbserver/linux-low.h | 13 +++
>> gdb/gdbserver/linux-x86-low.c | 26 ++++++
>> 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/linux-nat.h | 2 +-
>> gdb/remote.c | 116 +++++++++++++++++++++++++
>> gdb/testsuite/gdb.base/catch-syscall.exp | 15 +++-
>> 17 files changed, 456 insertions(+), 6 deletions(-)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index a222dfb491f0..1917523d8249 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -112,6 +112,11 @@ N stop reply
>> threads are stopped). The remote stub reports support for this stop
>> reply to GDB's qSupported query.
>>
>> +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.
>> @@ -127,6 +132,11 @@ show remote exec-event-feature-packet
>> The reply to qXfer:threads:read may now include a name attribute for each
>> thread.
>>
>> +* 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
>>
>
> Please mention the new set/show commands as well.
OK. Looking at "exec" for a model, I guess I should also mention the
syscall_entry/return stop reasons and the qSupported addition.
>> +@item QCatchSyscalls
>> +The remote stub understands the @samp{QCatchSyscalls} packet
>> +(@pxref{QCatchSyscalls}).
>> +
>> @item QPassSignals
>> The remote stub understands the @samp{QPassSignals} packet
>> (@pxref{QPassSignals}).
>
> You also need to list the new commands in the table below:
>
> For each packet @var{name}, the command to enable or disable the
> packet is @code{set remote @var{name}-packet}. The available settings
> are:
It's already added in the first patch hunk of this file, no?
Related, I just noticed the code was checking only for remote support,
not the local enabled state. I'll fix that to use packet_support().
>> /* 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;
>
> "just" here means that it's a list with a single element that
> happens to be ANY_SYSCALL; I think it's helpful for the reader to
> be explicit and expand the "or just ANY_SYSCALL" comment little bit.
OK, I'll expand this.
>> +
>> const struct target_desc *tdesc;
>>
>> /* Private target data. */
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 5e2dc5857a8d..cceede6fe408 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) (char **, int count);
>>
>> + /* 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);
>> +
>
> As you're not putting this new callback at the end of the struct,
> you need to install a NULL callback in all linux-*-low.c files (all
> archs) that install linux_target_ops methods after process_qsupported.
> Otherwise you'll break their builds.
Ah, indeed. It seems more convenient to put the new field at the end,
but right now it roughly mirrors the placement of the new target_ops
field. Maybe that should be at the end too, then I can touch fewer
files overall. (i.e. no change in targets I'm not implementing.)
>> /* Returns true if the low target supports tracepoints. */
>> int (*supports_tracepoints) (void);
>>
>> @@ -277,6 +283,13 @@ struct lwp_info
>> event already received in a wait()). */
>> int stopped;
>>
>> + /* Signal whether 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 18adf5ee7d17..1caf71edf485 100644
>
>
>
>> @@ -1172,6 +1174,16 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>>
>> sprintf (buf, "T%02xcreate:;", signal);
>> }
>> + else if ((status->kind == TARGET_WAITKIND_SYSCALL_ENTRY)
>> + || (status->kind == TARGET_WAITKIND_SYSCALL_RETURN))
>
> Unnecessary parens.
OK, removed.
>> + {
>> + 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 6d151ee35d39..e63656b9c0f6 100644
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -597,6 +597,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;
>> + }
>
> I think you should merge this with
>
>> + if (strcmp ("QCatchSyscalls:0", own_buf) == 0)
>> + {
>> + if (!target_running () || !target_supports_catch_syscall ())
>> + {
>> + write_enn (own_buf);
>> + return;
>> + }
>
> into a single
>
> if (startswith (own_buf, "QCatchSyscalls:")
> {
> ...
> if (!target_running () || !target_supports_catch_syscall ())
> {
> write_enn (own_buf);
> return;
> }
>
> See "QNonStop:" handling. The end result is that "QCatchSyscalls:2"
> is treated as an error instead of returning the empty packet.
OK, I will merge these and make it parse more strictly.
>> +
>> + process = current_process ();
>> +
>> + VEC_truncate (int, process->syscalls_to_catch, 0);
>> +
>> + p = own_buf + strlen("QCatchSyscalls:1");
>> + if (*p == ';')
>> + {
>> + p += 1;
>> + while (*p)
>
> while (*p != '\0)
OK
>> + {
>> + 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;
>> @@ -2219,6 +2265,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 dc0361fdcd14..bc2abd5f50c8 100644
>
>
>
>> + 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])
>
> if (table[i] != 0)
>
> just like the comment. :-)
OK
>> + 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. */
>
> "non-selective packet"
OK
>> + 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])
>
> Likewise.
OK
>> + {
>> + xsnprintf (p, catch_packet + maxpktsz - p, ";%x", i);
>> + p += strlen (p);
>
> This looks like:
>
> len = xsnprintf (p, catch_packet + maxpktsz - p, ";%x", i);
> p += len;
OK, I see, since xsnprintf asserts that the return count did fit, it can
be used directly. I guess "p += xsnprintf (...)" should do fine.
>> + }
>> + }
>> + }
>> + 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;
>
> catch_packet[sizeof ("QCatchSyscalls:1") - 1] = 0;
OK
>> + }
>> + }
>> + else
>> + {
>> + catch_packet = xmalloc (strlen ("QCatchSyscalls:0") + 1);
>> + strcpy (catch_packet, "QCatchSyscalls:0");
>
> catch_packet = xstrdup ("QCatchSyscalls:0");
OK
>> + }
>> +
>> + {
>> + 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]);
>
> getpkt takes a pointer to pointer because it may xrealloc 'rs->buf'.
> So this 'buf' reference may be dangling. Just drop the 'buf' local.
OK
>> + xfree (catch_packet);
>
> putpkt/getpkt may throw. This should be freed with a cleanup instead.
OK
>> + 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. */
>>
>
>
>
>> 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
>> }
>
> This check won't reach the "continue" when testing
> with --target_board=native-extended-gdbserver on some arch that doesn't
> implement QCatchSyscall yet. That board is not "is_remote".
>
> I'd actually favor dropping the arch check altogether. Otherwise it's very
> likely that arch maintainers won't ever notice there's a new method they
> can wire in.
OK, I'll just remove it then.
>>
>> @@ -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" } {
>
> I think IWBN to add a supports_catch_fork proc to lib/gdb.exp. These
> checks are getting duplicated (and not sure they are consistent) in
> several places. E.g., here, your now catch-fork-static.exp test, the
> test that was based on, etc...
Yeah, well this one is "vfork", but it still conceptually overlaps with
what foll-vfork.exp checks.
For the moment, I guess I can just remove the latter clause, since you
pointed out that native-extended-gdbserver is already not "is_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).
>
> Thanks,
> Pedro Alves
>
> Cc: gdb-patches@sourceware.org, philippe.waroquiers@skynet.be,
> sergiodj@redhat.com, palves@redhat.com, xdje42@gmail.com
> From: Josh Stone <jistone@redhat.com>
> Date: Fri, 4 Dec 2015 18:14:51 -0800
>
> On 12/04/2015 12:44 AM, Eli Zaretskii wrote:
> >> From: Josh Stone <jistone@redhat.com>
> >> Cc: philippe.waroquiers@skynet.be, sergiodj@redhat.com, palves@redhat.com, eliz@gnu.org, xdje42@gmail.com, Josh Stone <jistone@redhat.com>
> >> Date: Thu, 3 Dec 2015 18:26:46 -0800
> >>
> >> 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.
> >>
> >> gdb/ChangeLog:
> >>
> >> 2015-12-03 Josh Stone <jistone@redhat.com>
> >> Philippe Waroquiers <philippe.waroquiers@skynet.be>
> >>
> >> * 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.
> >> * linux-nat.h (struct lwp_info) <syscall_state>: Comment typo.
> >>
> >> gdb/doc/ChangeLog:
> >>
> >> 2015-12-03 Josh Stone <jistone@redhat.com>
> >> Philippe Waroquiers <philippe.waroquiers@skynet.be>
> >>
> >> * 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.
> >
> > Any changes since v2 in the documentation parts that require a fresh
> > review?
>
> I made the changes you requested in v2 -- v3 just has some
> merge-resolution in NEWS. But Pedro has noted some more things to
> document for the next round.
Then I guess it's better if I review the next round, is that right?
Or do you mean this v3 _is_ that next round?
Thanks.
On 12/05/2015 12:00 AM, Eli Zaretskii wrote:
>>> Any changes since v2 in the documentation parts that require a fresh
>>> review?
>>
>> I made the changes you requested in v2 -- v3 just has some
>> merge-resolution in NEWS. But Pedro has noted some more things to
>> document for the next round.
>
> Then I guess it's better if I review the next round, is that right?
> Or do you mean this v3 _is_ that next round?
The former -- I mean v4 will have a few additions.
Sorry if I confused you by explicitly CCing. I just included everyone
who had commented at all on prior versions, but maybe that's too greedy.
> Cc: gdb-patches@sourceware.org, philippe.waroquiers@skynet.be,
> sergiodj@redhat.com, palves@redhat.com, xdje42@gmail.com
> From: Josh Stone <jistone@redhat.com>
> Date: Mon, 7 Dec 2015 08:50:42 -0800
>
> On 12/05/2015 12:00 AM, Eli Zaretskii wrote:
> >>> Any changes since v2 in the documentation parts that require a fresh
> >>> review?
> >>
> >> I made the changes you requested in v2 -- v3 just has some
> >> merge-resolution in NEWS. But Pedro has noted some more things to
> >> document for the next round.
> >
> > Then I guess it's better if I review the next round, is that right?
> > Or do you mean this v3 _is_ that next round?
>
> The former -- I mean v4 will have a few additions.
OK, will wait then.
> Sorry if I confused you by explicitly CCing. I just included everyone
> who had commented at all on prior versions, but maybe that's too greedy.
No need to be sorry, nothing bad happened.
Thanks.
On 12/05/2015 02:16 AM, Josh Stone wrote:
> On a total tangent, is it ever possible that GDB/GDBserver might try to
> read and modify registers from a PTRACE_EVENT stop?
Do "catch fork", and you'll be given a prompt right inside a
PTRACE_EVENT_FORK, where you can try to poke at registers at will.
> If so, you should
> beware that registers may actually be in flux. I ran into this with
> Dyninst, which I fixed here[1] though I can't find the discussion now.
>
Ouch.
> The gist was that in a PTRACE_EVENT, the kernel may not have written the
> return register yet. Dyninst wanted to save registers, resume in a bit
> of instrumentation code, then restore registers and resume the normal
> program. So the saved registers got an intermediate RAX, and when it
> resumed into instrumentation the kernel finally wrote the good RAX
> return value to complete the syscall (which the instrumentation
> ignored). Then when dyninst restored registers the bad RAX was written
> back, and thus the normal program code didn't get the correct value for
> its fork return. My solution was to step out of the event with
> PTRACE_SYSCALL before doing anything else.
>
> [1]
> http://git.dyninst.org/?p=dyninst.git;a=commit;h=b89ea1d19677fa0dd9c605ef492c5f6dabf15752
Just to be clear, doesn't $orig_rax help here? Are you saving/restoring that?
Otherwise, it sounds like trying to run an inferior function
call [(gdb) p foo_func()] when the program is stopped for "catch fork"
may misbehave too.
Thanks,
Pedro Alves
On 12/05/2015 02:16 AM, Josh Stone wrote:
>> You also need to list the new commands in the table below:
>>
>> For each packet @var{name}, the command to enable or disable the
>> packet is @code{set remote @var{name}-packet}. The available settings
>> are:
>
> It's already added in the first patch hunk of this file, no?
Indeed. Sorry, got confused.
Thanks,
Pedro Alves
On 12/08/2015 05:31 AM, Pedro Alves wrote:
> On 12/05/2015 02:16 AM, Josh Stone wrote:
>> On a total tangent, is it ever possible that GDB/GDBserver might try to
>> read and modify registers from a PTRACE_EVENT stop?
>
> Do "catch fork", and you'll be given a prompt right inside a
> PTRACE_EVENT_FORK, where you can try to poke at registers at will.
>
>> If so, you should
>> beware that registers may actually be in flux. I ran into this with
>> Dyninst, which I fixed here[1] though I can't find the discussion now.
>>
>
> Ouch.
>
>> The gist was that in a PTRACE_EVENT, the kernel may not have written the
>> return register yet. Dyninst wanted to save registers, resume in a bit
>> of instrumentation code, then restore registers and resume the normal
>> program. So the saved registers got an intermediate RAX, and when it
>> resumed into instrumentation the kernel finally wrote the good RAX
>> return value to complete the syscall (which the instrumentation
>> ignored). Then when dyninst restored registers the bad RAX was written
>> back, and thus the normal program code didn't get the correct value for
>> its fork return. My solution was to step out of the event with
>> PTRACE_SYSCALL before doing anything else.
>>
>> [1]
>> http://git.dyninst.org/?p=dyninst.git;a=commit;h=b89ea1d19677fa0dd9c605ef492c5f6dabf15752
>
> Just to be clear, doesn't $orig_rax help here? Are you saving/restoring that?
$orig_rax only keeps the value $rax had upon entry, which will be the
syscall number. The problem is the return value, which will eventually
go into $rax, but at the moment of a PTRACE_EVENT stop this value only
exists in kernel state. It's not visible in any user state at all.
Just now I had the thought that perhaps you could kludge the return $rax
by using the pid from PTRACE_GETEVENTMSG. (In the kernel, that comes
from the task_struct: child->ptrace_message.) However, this is subject
to pid namespace translation. So if gdb and the child are different,
say the latter is in a container, then PTRACE_GETEVENTMSG's pid will be
different than the return pid the child should see.
That said, the PTRACE_SYSCALL step isn't foolproof either. In theory
this should be fine, but arm and aarch64 kernels had bugs that they
wouldn't report the syscall return if you hadn't already caught the
entry. They would get on the syscall fast path and not check again for
tracing when they returned. Fixed in 4.1 for arm, 4.2 for aarch64.
PTRACE_SINGLESTEP may be a safer option to get out of the event syscall.
In a brief experiment with stepi on x86_64, this case doesn't even
advance the user $rip, but I can't be sure for all architectures. At
least we should expect it won't run away like a broken return
PTRACE_SYSCALL might.
> Otherwise, it sounds like trying to run an inferior function
> call [(gdb) p foo_func()] when the program is stopped for "catch fork"
> may misbehave too.
OK, I'll play with it this way to see what happens... but I ought to
finish QCatchSyscalls before I tackle new problems. :)
On 12/04/2015 06:16 PM, Josh Stone wrote:
> On 12/04/2015 05:18 AM, Pedro Alves wrote:
>> Quick question: What is supposed to happen to the QCatchSyscalls
>> when the process execs? I'm thinking of 64-bit inferior execing
>> 32-bit inferior, etc. The syscall numbers aren't necessarily shared
>> between the different architectures. This implementation deletes discards
>> the previous QCatchSyscalls on exec, and I think that's what makes sense,
>> but, I think that this should be explicit in the packet's description.
>
> Yes, I think it should be cleared to avoid any assumption about the
> architecture. I'll add a note in the description codifying this.
After exploration, I'm having second thoughts about this point. Yes,
the current implementation clears it, but only when PTRACE_O_TRACEEXEC
is enabled to actually get that event. That's only if the client sent
"qSupports:exec-events+". Otherwise, the server doesn't even know an
exec happened, so it can't really promise to reset the syscall table.
Since the server doesn't promise to always catch execs, I think we
should actually go the other way to stay consistent. Let the syscall
list be carried over, and document that clients should probably send a
new list after execs in case the architecture changed. Some clients may
just choose to live with the assumption that the arch is consistent in
their environment.
Thoughts?
>> I'm not sure gdb clears the inferior's "syscalls to the caught" VEC
>> on exec events, but it probably does (if it doesn't, I think it should.)
>
> I'll see if I can find out.
AFAICT the only time anything is removed from syscall_catchpoint's VEC
syscalls_to_be_caught is in breakpoint_ops->remove_location and ->dtor,
respectively remove_catch_syscall and dtor_catch_syscall. And since
this list isn't stored in the lwp structure itself, the exec doesn't
really affect anything. Right?
Hi Josh.
On 12/11/2015 09:19 PM, Josh Stone wrote:
> On 12/04/2015 06:16 PM, Josh Stone wrote:
>> On 12/04/2015 05:18 AM, Pedro Alves wrote:
>>> Quick question: What is supposed to happen to the QCatchSyscalls
>>> when the process execs? I'm thinking of 64-bit inferior execing
>>> 32-bit inferior, etc. The syscall numbers aren't necessarily shared
>>> between the different architectures. This implementation deletes discards
>>> the previous QCatchSyscalls on exec, and I think that's what makes sense,
>>> but, I think that this should be explicit in the packet's description.
>>
>> Yes, I think it should be cleared to avoid any assumption about the
>> architecture. I'll add a note in the description codifying this.
>
> After exploration, I'm having second thoughts about this point. Yes,
> the current implementation clears it, but only when PTRACE_O_TRACEEXEC
> is enabled to actually get that event. That's only if the client sent
> "qSupports:exec-events+". Otherwise, the server doesn't even know an
> exec happened, so it can't really promise to reset the syscall table.
Hmm. It sounds reasonable.
However, I think that gdb will always want to listen to exec events.
It has to, in order to install breakpoints in the new image, and make sure
that it doesn't incorrectly remove old breakpoints in the new image either
the next time the program stops. That older gdb's weren't aware of execs
is plain and simply a bug.
I understand you may be thinking of strace instead of gdb though,
which mainly only cares about syscalls.
I think even strace always need to handle exec events too, to handle
the case that the exec event is reported to the thread group
leader, even if it was some other thread that execed.
But I see the point that this may avoid extra stops and traffic
in some scenarios, so I'm fine with specifying it that way.
>
> Since the server doesn't promise to always catch execs, I think we
> should actually go the other way to stay consistent.
If the server doesn't catch execs, then gdb won't know about them either,
and so from gdb's perpective, the syscalls-to-catch-list doesn't change
either. It's as-if the exec didn't happen, so the server would naturally
catch syscalls across the exec. Doesn't seem to me that there'd be an
issue here.
> Let the syscall
> list be carried over, and document that clients should probably send a
> new list after execs in case the architecture changed. Some clients may
> just choose to live with the assumption that the arch is consistent in
> their environment.
We really should say "probably" in the docs. Stub authors won't know
what to do then. We should be definitive.
On the gdb side, I think that determining whether the arch changed would
be somewhat a complication -- the place we discover the new arch isn't
the same place we install new breakpoints. We could perhaps have the core
always install the catchpoints and then we could handle that
by making remote.c remember the last arch the syscalls list was set
to for the given inferior, and compare that arch with the inferior's
current arch, somehow generically.
BTW, am I right that with your current patch, if the user sets
10 syscall catchpoints, remote_set_syscall_catchpoint will
be called 10 times, and thus gdbserver gets 10 QCatchSyscalls
packets?
>
>>> I'm not sure gdb clears the inferior's "syscalls to the caught" VEC
>>> on exec events, but it probably does (if it doesn't, I think it should.)
>>
>> I'll see if I can find out.
>
> AFAICT the only time anything is removed from syscall_catchpoint's VEC
> syscalls_to_be_caught is in breakpoint_ops->remove_location and ->dtor,
> respectively remove_catch_syscall and dtor_catch_syscall. And since
> this list isn't stored in the lwp structure itself, the exec doesn't
> really affect anything. Right?
>
Hmm. Then it sounds like we still mishandle e.g., "catch syscall open"
across execs that change archs, as described at:
https://sourceware.org/bugzilla/show_bug.cgi?id=10737#c5
Thanks,
Pedro Alves
@@ -112,6 +112,11 @@ N stop reply
threads are stopped). The remote stub reports support for this stop
reply to GDB's qSupported query.
+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.
@@ -127,6 +132,11 @@ show remote exec-event-feature-packet
The reply to qXfer:threads:read may now include a name attribute for each
thread.
+* 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*
@@ -20135,6 +20135,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}}
@@ -35458,6 +35462,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.
@@ -35950,6 +35959,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 in the list is reported, @value{GDBN} will
+still filter the event according to its own list from all corresponding
+@code{catch syscall} commands. However, it is more efficient to only
+report the requested 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
@@ -36401,6 +36448,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{-}
@@ -36604,6 +36656,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}).
@@ -339,6 +339,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);
}
@@ -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. */
@@ -461,6 +461,11 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
gdb_assert (event_lwp->waitstatus.kind == TARGET_WAITKIND_IGNORE);
+ /* 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))
{
@@ -682,6 +687,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.
@@ -2241,6 +2280,8 @@ linux_low_ptrace_options (int attached)
if (report_exec_events)
options |= PTRACE_O_TRACEEXEC;
+ options |= PTRACE_O_TRACESYSGOOD;
+
return options;
}
@@ -2369,6 +2410,21 @@ linux_low_filter_event (int lwpid, int wstat)
child->must_set_ptrace_flags = 0;
}
+ /* Always update syscall_state, even if it will be filtered later. */
+ if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SYSCALL_SIGTRAP)
+ {
+ 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
check_stopped_by_breakpoint is called. */
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
@@ -2978,6 +3034,44 @@ filter_exit_event (struct lwp_info *event_child,
return 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
@@ -3312,6 +3406,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
@@ -3566,8 +3676,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
@@ -3987,6 +4105,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
@@ -4174,7 +4293,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;
+ else
+ ptrace_request = PTRACE_CONT;
+ 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. */
@@ -6270,6 +6396,13 @@ linux_process_qsupported (char **features, int count)
}
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)
@@ -7157,6 +7290,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,
@@ -201,6 +201,12 @@ struct linux_target_ops
/* Hook to support target specific qSupported. */
void (*process_qsupported) (char **, int count);
+ /* 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);
@@ -277,6 +283,13 @@ struct lwp_info
event already received in a wait()). */
int stopped;
+ /* Signal whether 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;
@@ -1438,6 +1438,31 @@ 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
+ {
+ collect_register_by_name (regcache, "orig_eax", sysno);
+ collect_register_by_name (regcache, "eax", sysret);
+ }
+}
+
static int
x86_supports_tracepoints (void)
{
@@ -3307,6 +3332,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,
@@ -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 */
@@ -1123,6 +1123,8 @@ prepare_resume_reply (char *buf, ptid_t ptid,
case TARGET_WAITKIND_VFORK_DONE:
case TARGET_WAITKIND_EXECD:
case TARGET_WAITKIND_THREAD_CREATED:
+ case TARGET_WAITKIND_SYSCALL_ENTRY:
+ case TARGET_WAITKIND_SYSCALL_RETURN:
{
struct thread_info *saved_thread;
const char **regp;
@@ -1172,6 +1174,16 @@ prepare_resume_reply (char *buf, ptid_t ptid,
sprintf (buf, "T%02xcreate:;", signal);
}
+ 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);
@@ -597,6 +597,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;
@@ -2219,6 +2265,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+");
@@ -134,4 +134,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 */
@@ -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 */
@@ -312,6 +312,10 @@ struct target_ops
features with COUNT elements. */
void (*process_qsupported) (char **features, int count);
+ /* 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);
@@ -542,6 +546,10 @@ int kill_inferior (int);
the_target->process_qsupported (features, count); \
} 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)
@@ -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 */
@@ -93,7 +93,7 @@ struct lwp_info
or to a local variable in lin_lwp_wait. */
struct target_waitstatus waitstatus;
- /* Signal wether we are in a SYSCALL_ENTRY or
+ /* Signal whether we are in a SYSCALL_ENTRY or
in a SYSCALL_RETURN event.
Values:
- TARGET_WAITKIND_SYSCALL_ENTRY
@@ -1392,6 +1392,7 @@ enum {
PACKET_qSupported,
PACKET_qTStatus,
PACKET_QPassSignals,
+ PACKET_QCatchSyscalls,
PACKET_QProgramSignals,
PACKET_qCRC,
PACKET_qSearch_memory,
@@ -1986,6 +1987,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. */
@@ -4466,6 +4560,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,
@@ -6371,6 +6467,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"))
@@ -12975,6 +13087,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_thread_name = remote_thread_name;
@@ -13549,6 +13662,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);
@@ -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).