From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100015 invoked by alias); 9 Oct 2015 17:57:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 100000 invoked by uid 89); 9 Oct 2015 17:57:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 09 Oct 2015 17:57:14 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id F3293A85 for ; Fri, 9 Oct 2015 17:57:12 +0000 (UTC) Received: from [10.3.113.151] (ovpn-113-151.phx2.redhat.com [10.3.113.151]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t99HvCUo017970; Fri, 9 Oct 2015 13:57:12 -0400 Subject: Re: [PATCH] gdb: Improve syscall entry/return tracking on Linux To: Pedro Alves , gdb-patches@sourceware.org References: <1444353736-14451-1-git-send-email-jistone@redhat.com> <56179B68.80200@redhat.com> Cc: sergiodj@redhat.com From: Josh Stone Message-ID: <5617FFF8.50905@redhat.com> Date: Fri, 09 Oct 2015 17:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <56179B68.80200@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-10/txt/msg00107.txt.bz2 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.