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

[Josh Stone <jistone@redhat.com>]

On 10/09/2015 03:48 AM, Pedro Alves wrote:
> Hi Josh,
> 
> This looks generally good to me.  A couple comments below.

Thanks for reviewing!

> 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")

Ok, I will update these.

>>  
>>    /* 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?

In kernel/fork.c I see that lacking CONFIG_MMU returns EINVAL.

But it appears a few archs don't implement fork/vfork syscalls at all,
only clone.  Maybe I should use CLONE_VFORK for broadest coverage?

>> +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?

No problem.