This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdb: Improve syscall entry/return tracking on Linux
- From: Pedro Alves <palves at redhat dot com>
- To: Josh Stone <jistone at redhat dot com>, gdb-patches at sourceware dot org
- Cc: sergiodj at redhat dot com
- Date: Fri, 09 Oct 2015 11:48:08 +0100
- Subject: Re: [PATCH] gdb: Improve syscall entry/return tracking on Linux
- Authentication-results: sourceware.org; auth=none
- References: <1444353736-14451-1-git-send-email-jistone at redhat dot com>
Hi Josh,
This looks generally good to me. A couple comments below.
On 10/09/2015 02:22 AM, Josh Stone wrote:
> @@ -2324,6 +2329,10 @@ wait_lwp (struct lwp_info *lp)
> if (linux_handle_syscall_trap (lp, 1))
> return wait_lwp (lp);
> }
> + else
> + /* Almost all other ptrace-stops are known to be outside of system
> + calls, with further exceptions in linux_handle_extended_wait. */
> + lp->syscall_state = TARGET_WAITKIND_IGNORE;
Our coding conventions state that this should be wrapped in braces:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
(look for "braces")
>
> /* Handle GNU/Linux's extended waitstatus for trace events. */
> if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
> @@ -3126,6 +3135,10 @@ linux_nat_filter_event (int lwpid, int status)
> if (linux_handle_syscall_trap (lp, 0))
> return NULL;
> }
> + else
> + /* Almost all other ptrace-stops are known to be outside of system
> + calls, with further exceptions in linux_handle_extended_wait. */
> + lp->syscall_state = TARGET_WAITKIND_IGNORE;
Ditto.
>
> /* Handle GNU/Linux's extended waitstatus for trace events. */
> if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
> diff --git a/gdb/testsuite/gdb.base/catch-syscall.c b/gdb/testsuite/gdb.base/catch-syscall.c
> index 4d0131c0d733..35955fe4a078 100644
> --- a/gdb/testsuite/gdb.base/catch-syscall.c
> +++ b/gdb/testsuite/gdb.base/catch-syscall.c
> @@ -27,6 +27,8 @@ int pipe_syscall = SYS_pipe;
> int pipe2_syscall = SYS_pipe2;
> #endif
> int write_syscall = SYS_write;
> +int fork_syscall = SYS_fork;
no-mmu / uclinux systems don't have fork. I'm not sure whether
fork returns ENOSYS or SYS_fork isn't even defined there.
Maybe just switch to vfork so we can keep catch syscall
coverage on those systems?
> +int unknown_syscall = 123456789;
> int exit_group_syscall = SYS_exit_group;
>
> int
> @@ -47,6 +49,13 @@ main (void)
> write (fd[1], buf1, sizeof (buf1));
> read (fd[0], buf2, sizeof (buf2));
>
> + /* Test fork-event interactions. Child exits immediately.
> + NB: glibc actually uses clone(), so force a fork. */
> + if (syscall (fork_syscall) == 0) _exit (0);
We've recently agreed that tests should follow the coding conventions too,
unless there's a good reason otherwise. Can you put the _exit on
its own line?
Thanks,
Pedro Alves