Re: [PATCH] gdb: Improve syscall entry/return tracking on Linux

[Pedro Alves <palves@redhat.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