From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13108 invoked by alias); 2 Oct 2013 19:41:17 -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 13098 invoked by uid 89); 2 Oct 2013 19:41:17 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Oct 2013 19:41:16 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com 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 (8.14.4/8.14.4) with ESMTP id r92JfBhh010561 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 2 Oct 2013 15:41:11 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r92Jf8qE032511; Wed, 2 Oct 2013 15:41:09 -0400 Message-ID: <524C76D4.1010301@redhat.com> Date: Wed, 02 Oct 2013 19:41:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Philippe Waroquiers CC: Sergio Durigan Junior , gdb-patches@sourceware.org Subject: Always run the PTRACE_O_TRACESYSGOOD tests even if PTRACE_O_TRACEFORK is not supported. (was: Re: RFA [PATCH v4] Implement 'catch syscall' for gdbserver) References: <1379796907.5980.20.camel@soleil> <1380467062.3567.52.camel@soleil> In-Reply-To: <1380467062.3567.52.camel@soleil> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-10/txt/msg00105.txt.bz2 On 09/29/2013 04:04 PM, Philippe Waroquiers wrote: > Index: common/linux-ptrace.c > =================================================================== > RCS file: /cvs/src/src/gdb/common/linux-ptrace.c,v > retrieving revision 1.12 > diff -u -p -r1.12 linux-ptrace.c > --- common/linux-ptrace.c 28 Aug 2013 14:09:31 -0000 1.12 > +++ common/linux-ptrace.c 29 Sep 2013 14:20:29 -0000 > @@ -361,16 +361,18 @@ linux_check_ptrace_features (void) > return; > } > > -#ifdef GDBSERVER > - /* gdbserver does not support PTRACE_O_TRACESYSGOOD or > - PTRACE_O_TRACEVFORKDONE yet. */ > -#else > - /* Check if the target supports PTRACE_O_TRACESYSGOOD. */ > + /* Check if the target supports PTRACE_O_TRACESYSGOOD. > + We keep PTRACE_O_TRACEFORK option activated as a fork > + event notification is expected after my_waitpid below. */ > ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, > - (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD); > + (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK > + | PTRACE_O_TRACESYSGOOD)); This coupling looks unfortunate. Actually why wasn't this a problem for the native target, even without GDBserver in the picture? It just looks like a bug fix? In any case, there's another related bug here. Above that we have: /* First, set the PTRACE_O_TRACEFORK option. If this fails, we know for sure that it is not supported. */ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) PTRACE_O_TRACEFORK); if (ret != 0) { ret = ptrace (PTRACE_KILL, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0); if (ret != 0) { warning (_("linux_check_ptrace_features: failed to kill child")); return; } ret = my_waitpid (child_pid, &status, 0); if (ret != child_pid) warning (_("linux_check_ptrace_features: failed " "to wait for killed child")); else if (!WIFSIGNALED (status)) warning (_("linux_check_ptrace_features: unexpected " "wait status 0x%x from killed child"), status); return; <<<<<<<<<<<<<<<<< } Note that early return. If PTRACE_O_TRACEFORK isn't supported, we're not checking PTRACE_O_TRACESYSGOOD. This didn't use to be a problem before the unification of this whole detection business in linux-ptrace.c. Before, the sysgood detection was completely separate: static void linux_test_for_tracesysgood (int original_pid) { int ret; sigset_t prev_mask; /* We don't want those ptrace calls to be interrupted. */ block_child_signals (&prev_mask); linux_supports_tracesysgood_flag = 0; ret = ptrace (PTRACE_SETOPTIONS, original_pid, 0, PTRACE_O_TRACESYSGOOD); if (ret != 0) goto out; linux_supports_tracesysgood_flag = 1; out: restore_child_signals_mask (&prev_mask); } Here's a patch that fixes these issues. ---- Subject: Always run the PTRACE_O_TRACESYSGOOD tests even if PTRACE_O_TRACEFORK is not supported. If enabling PTRACE_O_TRACEFORK fails, we never test for PTRACE_O_TRACESYSGOOD support. Before PTRACE_O_TRACESYSGOOD is checked, we have: /* First, set the PTRACE_O_TRACEFORK option. If this fails, we know for sure that it is not supported. */ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) PTRACE_O_TRACEFORK); if (ret != 0) { ret = ptrace (PTRACE_KILL, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0); if (ret != 0) { warning (_("linux_check_ptrace_features: failed to kill child")); return; } ret = my_waitpid (child_pid, &status, 0); if (ret != child_pid) warning (_("linux_check_ptrace_features: failed " "to wait for killed child")); else if (!WIFSIGNALED (status)) warning (_("linux_check_ptrace_features: unexpected " "wait status 0x%x from killed child"), status); return; <<<<<<<<<<<<<<<<< } Note that early return. If PTRACE_O_TRACEFORK isn't supported, we're not checking PTRACE_O_TRACESYSGOOD. This didn't use to be a problem before the unification of this whole detection business in linux-ptrace.c. Before, the sysgood detection was completely separate: static void linux_test_for_tracesysgood (int original_pid) { int ret; sigset_t prev_mask; /* We don't want those ptrace calls to be interrupted. */ block_child_signals (&prev_mask); linux_supports_tracesysgood_flag = 0; ret = ptrace (PTRACE_SETOPTIONS, original_pid, 0, PTRACE_O_TRACESYSGOOD); if (ret != 0) goto out; linux_supports_tracesysgood_flag = 1; out: restore_child_signals_mask (&prev_mask); } So we need to get back the decoupling somehow. I think it's cleaner to split the seperate feature detections to separate functions. This patch does that. The new functions are named for their counterparts that existed before this code was moved to linux-ptrace.c. Note I've used forward declarations for the new functions to make the patch clearer, as otherwise the patch would look like I'd be adding a bunch of new code. A reorder can be done in a follow up patch. Tested on x86_64 Fedora 17. gdb/ 2013-10-02 Pedro Alves * common/linux-ptrace.c (linux_check_ptrace_features): Factor out the PTRACE_O_TRACESYSGOOD and PTRACE_O_TRACEFORK to separate functions. Always test for PTRACE_O_TRACESYSGOOD even if PTRACE_O_TRACEFORK is not supported. (linux_test_for_tracesysgood): New function. (linux_test_for_tracefork): New function, factored out from linux_check_ptrace_features, and also don't kill child_pid here. --- gdb/common/linux-ptrace.c | 75 +++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c index 3a8e25e..3ea2d6d 100644 --- a/gdb/common/linux-ptrace.c +++ b/gdb/common/linux-ptrace.c @@ -308,13 +308,15 @@ linux_child_function (gdb_byte *child_stack) _exit (0); } +static void linux_test_for_tracesysgood (int child_pid); +static void linux_test_for_tracefork (int child_pid); + /* Determine ptrace features available on this target. */ static void linux_check_ptrace_features (void) { int child_pid, ret, status; - long second_pid; /* Initialize the options. */ current_ptrace_options = 0; @@ -335,42 +337,60 @@ linux_check_ptrace_features (void) error (_("linux_check_ptrace_features: waitpid: unexpected status %d."), status); - /* First, set the PTRACE_O_TRACEFORK option. If this fails, we - know for sure that it is not supported. */ - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, - (PTRACE_TYPE_ARG4) PTRACE_O_TRACEFORK); + linux_test_for_tracesysgood (child_pid); - if (ret != 0) + linux_test_for_tracefork (child_pid); + + /* Clean things up and kill any pending children. */ + do { ret = ptrace (PTRACE_KILL, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0); if (ret != 0) - { - warning (_("linux_check_ptrace_features: failed to kill child")); - return; - } - - ret = my_waitpid (child_pid, &status, 0); - if (ret != child_pid) - warning (_("linux_check_ptrace_features: failed " - "to wait for killed child")); - else if (!WIFSIGNALED (status)) - warning (_("linux_check_ptrace_features: unexpected " - "wait status 0x%x from killed child"), status); - - return; + warning (_("linux_check_ptrace_features: failed to kill child")); + my_waitpid (child_pid, &status, 0); } + while (WIFSTOPPED (status)); +} +/* Determine if PTRACE_O_TRACESYSGOOD can be used to catch + syscalls. */ + +static void +linux_test_for_tracesysgood (int child_pid) +{ #ifdef GDBSERVER - /* gdbserver does not support PTRACE_O_TRACESYSGOOD or - PTRACE_O_TRACEVFORKDONE yet. */ + /* gdbserver does not support PTRACE_O_TRACESYSGOOD. */ #else + int ret; /* Check if the target supports PTRACE_O_TRACESYSGOOD. */ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD); if (ret == 0) current_ptrace_options |= PTRACE_O_TRACESYSGOOD; +#endif +} + +/* Determine if PTRACE_O_TRACEFORK can be used to follow fork + events. */ +static void +linux_test_for_tracefork (int child_pid) +{ + int ret, status; + long second_pid; + + /* First, set the PTRACE_O_TRACEFORK option. If this fails, we + know for sure that it is not supported. */ + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEFORK); + + if (ret != 0) + return; + +#ifdef GDBSERVER + /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet. */ +#else /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK @@ -439,17 +459,6 @@ linux_check_ptrace_features (void) else warning (_("linux_check_ptrace_features: unexpected result from waitpid " "(%d, status 0x%x)"), ret, status); - - /* Clean things up and kill any pending children. */ - do - { - ret = ptrace (PTRACE_KILL, child_pid, (PTRACE_TYPE_ARG3) 0, - (PTRACE_TYPE_ARG4) 0); - if (ret != 0) - warning (_("linux_check_ptrace_features: failed to kill child")); - my_waitpid (child_pid, &status, 0); - } - while (WIFSTOPPED (status)); } /* Enable reporting of all currently supported ptrace events. */