This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add support for catching system calls to native FreeBSD targets.
- From: Pedro Alves <palves at redhat dot com>
- To: John Baldwin <jhb at FreeBSD dot org>, gdb-patches at sourceware dot org
- Date: Mon, 20 Jun 2016 23:56:40 +0100
- Subject: Re: [PATCH] Add support for catching system calls to native FreeBSD targets.
- Authentication-results: sourceware.org; auth=none
- References: <20160614205751 dot 11566-1-jhb at FreeBSD dot org>
Hi John,
This looks good to me. Just some minor nits below.
On 06/14/2016 09:57 PM, John Baldwin wrote:
> All platforms on FreeBSD use a shared system call table, so use a
> single XML file to describe the system calls available on each FreeBSD
> platform.
>
> xRecent
typo.
> versions of FreeBSD include the identifier of the current
> system call when reporting a system call entry or exit event in the
> ptrace_lwpinfo structure obtained via PT_LWPINFO in fbsd_wait. As
> such, FreeBSD native targets do not use the gdbarch method to fetch
> the system call code. In addition, FreeBSD register sets fetched via
> ptrace do not include an equivalent of 'orig_rax' (on amd64 for
> example), so the system call code cannot be extracted from the
> available registers during a system call exit. However, GDB assumes
> that system call catch points are not supported if the gdbarch method
> is not present. As a workaround, FreeBSD ABIs install a dummy gdbarch
> method that throws an internal_error if it is ever invoked.
>
We should probably get rid of this gdbarch method, by making linux-nat.c
(the only caller) call an arch-specific target_ops override instead of
a gdbarch method, like gdbserver's equivalent code does.
To replace the break-catch-syscall.c error, I think that it'd be reasonable
to remove it altogether, and for Linux targets that don't implement
the gdbarch hook yet, instead just always intercept all syscalls, reporting
an <unknown> syscall number.
But what you did seems like a reasonable thing to do as long as do
have the gdbarch hook.
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 6a72f72..4ed706a 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1526,6 +1526,11 @@ fi
> AC_CHECK_MEMBERS([struct ptrace_lwpinfo.pl_tdname], [], [],
> [#include <sys/ptrace.h>])
>
> +# See if <sys/ptrace.h> supports syscall fields on FreeBSD
Missing period.
> +# Older FreeBSD versions don't have the pl_syscall_code member of
> +# `struct ptrace_lwpinfo'.
Can you replace "Older" with a non-relative version reference?
> +AC_CHECK_MEMBERS([struct ptrace_lwpinfo.pl_syscall_code], [], [],
> + [#include <sys/ptrace.h>])
>
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index b582abe..741a96d 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -707,6 +707,40 @@ fbsd_wait (struct target_ops *ops,
> return wptid;
> }
> #endif
> +
> + /* Note that PL_FLAG_SCE is set for any event reported while
> + a thread is executing a system call in the kernel. In
> + particular, signals that interrupt a sleep in a system
> + call will report this flag as part of their event. Stops
> + explicitly for system call entry and exit always use
> + SIGTRAP, so only treat SIGTRAP events as system call
> + entriy/exit events. */
Typo "entriy".
> + if (pl.pl_flags & (PL_FLAG_SCE | PL_FLAG_SCX)
> + && ourstatus->value.sig == SIGTRAP)
> + {
> +#ifdef HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE
> + if (catch_syscall_enabled ())
> + {
> + if (catching_syscall_number (pl.pl_syscall_code))
> + {
> + if (pl.pl_flags & PL_FLAG_SCE)
> + ourstatus->kind = TARGET_WAITKIND_SYSCALL_ENTRY;
> + else
> + ourstatus->kind = TARGET_WAITKIND_SYSCALL_RETURN;
> + ourstatus->value.syscall_number = pl.pl_syscall_code;
> + return wptid;
> + }
> + }
> +#endif
> + /* If the core isn't interested in this event, just
> + continue the process explicitly and wait for another
> + event. Note that PT_SYSCALL is "sticky" on FreeBSD
> + and once system call stops are enabled on a process
> + it stops for all system call entries and exits. */
> + if (ptrace (PT_CONTINUE, pid, (caddr_t) 1, 0) == -1)
> + perror_with_name (("ptrace"));
> + continue;
> + }
> }
> return wptid;
> }
> #include "elf-bfd.h"
> #include "fbsd-tdep.h"
> @@ -283,6 +284,20 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
> return note_data;
> }
>
> +static LONGEST
> +fbsd_get_syscall_number (struct gdbarch *gdbarch,
> + ptid_t ptid)
Add the usual "implement foo gdbarch method" or some such comment.
> +{
Thanks,
Pedro Alves