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: Josh Stone <jistone at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>, gdb-patches at sourceware dot org
- Cc: sergiodj at redhat dot com
- Date: Fri, 9 Oct 2015 10:57:12 -0700
- 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> <56179B68 dot 80200 at redhat dot 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.