From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7999 invoked by alias); 7 May 2014 19:40:14 -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 7983 invoked by uid 89); 7 May 2014 19:40:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 May 2014 19:40:12 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1Wi7hI-0006RZ-Gu from Luis_Gustavo@mentor.com ; Wed, 07 May 2014 12:40:08 -0700 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 7 May 2014 12:40:08 -0700 Received: from [172.30.1.162] ([172.30.1.162]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 7 May 2014 12:40:07 -0700 Message-ID: <536A8C0E.20205@codesourcery.com> Date: Wed, 07 May 2014 19:40:00 -0000 From: Luis Machado Reply-To: lgustavo@codesourcery.com User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Don Breazeal , gdb-patches@sourceware.org CC: Don Breazeal Subject: Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events References: <1398885482-8449-1-git-send-email-donb@codesourcery.com> <1398885482-8449-2-git-send-email-donb@codesourcery.com> In-Reply-To: <1398885482-8449-2-git-send-email-donb@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-05/txt/msg00073.txt.bz2 Hi Don, Thanks for moving this work forward. The patch looks good to me. I only have a small comment below. As for the use of the EXIT events, i think it is good that we get to finally start using it. Maybe it will be useful for other things besides this specific feature too. On 04/30/2014 04:17 PM, Don Breazeal wrote: > From: Don Breazeal > > [Reposting to eliminate unexpected attachment type.] > > This patch implements support for the extended ptrace event > PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event > support. > > The use of this event is entirely internal to gdbserver; these events are > not reported to GDB or the user. If an existing thread is not the last > thread in a process, its lwp entry is simply deleted, since this is what > happens in the absence of exit events. If it is the last thread of a > process, the wait status is set to the actual wait status of the thread, > instead of the status that indicates the extended event, and the existing > mechanisms for handling thread exit proceed as usual. > > The only purpose in using the exit events instead of the existing wait > mechanisms is to ensure that the exit of a thread group leader is detected > reliably when a non-leader thread calls exec. > > gdb/ > 2014-04-02 Don Breazeal > > * common/linux-ptrace.c (linux_test_for_tracefork) > [GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS > supports it. > > gdbserver/ > 2014-04-02 Don Breazeal > > * linux-low.c (is_extended_waitstatus): New function. > (is_extended_exit): New function. > (handle_extended_wait): Change type from void to int, change > wait status arg to pointer, add support for PTRACE_EVENT_EXIT. > (get_stop_pc): Use is_extended_waitstatus instead of hardcoded > shift operation. > (get_detach_signal): Use is_extended_waitstatus instead of > hardcoded shift operation. > (linux_low_filter_event): Handle new return value from > handle_extended_wait. Check for extended events as needed. > Allow status arg to be modified. > (linux_wait_for_event_filtered): Allow status arg to be > modified. > > --- > gdb/common/linux-ptrace.c | 42 ++++++++++++++++++- > gdb/gdbserver/linux-low.c | 99 +++++++++++++++++++++++++++++++++++++++------ > 2 files changed, 125 insertions(+), 16 deletions(-) > > diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c > index 7c1b78a..e3fc705 100644 > --- a/gdb/common/linux-ptrace.c > +++ b/gdb/common/linux-ptrace.c > @@ -414,7 +414,7 @@ linux_test_for_tracefork (int child_pid) > ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0, > (PTRACE_TYPE_ARG4) 0); > if (ret != 0) > - warning (_("linux_test_for_tracefork: failed to resume child")); > + warning (_("linux_test_for_tracefork: failed to resume child (a)")); > > ret = my_waitpid (child_pid, &status, 0); > > @@ -455,10 +455,46 @@ linux_test_for_tracefork (int child_pid) > "failed to kill second child")); > my_waitpid (second_pid, &status, 0); > } > + else > + { > + /* Fork events are not supported. */ > + return; > + } > } > else > - warning (_("linux_test_for_tracefork: unexpected result from waitpid " > - "(%d, status 0x%x)"), ret, status); > + { > + warning (_("linux_test_for_tracefork: unexpected result from waitpid " > + "(%d, status 0x%x)"), ret, status); > + return; > + } > + > +#ifdef GDBSERVER > + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT. > + First try to set the 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_TRACEEXIT); > + if (ret != 0) > + return; > + > + /* We don't know for sure that the feature is available; old > + versions of PTRACE_SETOPTIONS ignored unknown options. So > + see if the process exit will generate a PTRACE_EVENT_EXIT. */ > + ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0, > + (PTRACE_TYPE_ARG4) 0); > + if (ret != 0) > + warning (_("linux_test_for_tracefork: failed to resume child (b)")); > + > + ret = my_waitpid (child_pid, &status, 0); > + > + /* Check if we received an exit event notification. */ > + if (ret == child_pid && WIFSTOPPED (status) > + && status >> 16 == PTRACE_EVENT_EXIT) > + { > + /* PTRACE_O_TRACEEXIT is supported. */ > + current_ptrace_options |= PTRACE_O_TRACEEXIT; > + } > +#endif > } > > /* Enable reporting of all currently supported ptrace events. */ > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 2d8d5f5..90e7b15 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -224,6 +224,7 @@ static void proceed_all_lwps (void); > static int finish_step_over (struct lwp_info *lwp); > static CORE_ADDR get_stop_pc (struct lwp_info *lwp); > static int kill_lwp (unsigned long lwpid, int signo); > +static int num_lwps (int pid); > > /* True if the low target can hardware single-step. Such targets > don't need a BREAKPOINT_REINSERT_ADDR callback. */ > @@ -367,13 +368,32 @@ linux_add_process (int pid, int attached) > return proc; > } > > +/* Check wait status for extended event */ > + > +static int > +is_extended_waitstatus (int wstat) > +{ > + return wstat >> 16 != 0; > +} > + > +/* Check wait status for extended event */ > + > +static int > +is_extended_exit (int wstat) > +{ > + return wstat >> 16 == PTRACE_EVENT_EXIT; > +} > + The function above needs to have its comment updated to reflect the actual function body. Luis