From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4992 invoked by alias); 9 Jul 2014 17:27:21 -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 4958 invoked by uid 89); 9 Jul 2014 17:27:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 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, 09 Jul 2014 17:27:14 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1X4veB-00007o-02 from donb@codesourcery.com ; Wed, 09 Jul 2014 10:27:11 -0700 Received: from [127.0.0.1] ([172.30.7.155]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 9 Jul 2014 10:27:10 -0700 Message-ID: <53BD7B6C.8030205@codesourcery.com> Date: Wed, 09 Jul 2014 17:27:00 -0000 From: "Breazeal, Don" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Gary Benson , gdb-patches@sourceware.org Subject: Re: [PATCH 02/15] Remove some GDBSERVER checks from linux-ptrace References: <1404902255-11101-1-git-send-email-gbenson@redhat.com> <1404902255-11101-3-git-send-email-gbenson@redhat.com> In-Reply-To: <1404902255-11101-3-git-send-email-gbenson@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg00194.txt.bz2 Hi, On 7/9/2014 3:37 AM, Gary Benson wrote: > This patch removes some GDBSERVER checks from nat/linux-ptrace.c. > Currently the code uses a compile-time check to decide whether some > flags should be used. This changes the code to instead let users of > the module specify an additional set of flags; and then changes gdb's > linux-nat.c to call this function. At some later date, when the back > ends are fully merged, we will be able to remove this function again. I just want to point out that I'm doing something similar as part of my work on follow-fork for extended remote, based on Tom's suggestion here: https://sourceware.org/ml/gdb-patches/2014-05/msg00486.html In my case it adds "base" options and the capability to set the ptrace options to the "additional" options at the caller's discretion, such as when gdbserver receives the enable-extended-mode packet. So, no objection, just wanted to let you know that I hope to submit somewhat different changes to the same code soon. --Don > > gdb/ > 2014-07-09 Tom Tromey > Gary Benson > > * nat/linux-ptrace.c (additional_flags): New global. > (linux_test_for_tracesysgood, linux_test_for_tracefork): Use > additional_flags; don't check GDBSERVER. > (linux_ptrace_set_additional_flags): New function. > * nat/linux-ptrace.h (linux_ptrace_set_additional_flags): > Declare. > * linux-nat.c (_initialize_linux_nat): Call > linux_ptrace_set_additional_flags. > --- > gdb/ChangeLog | 12 +++++++++ > gdb/linux-nat.c | 6 ++++ > gdb/nat/linux-ptrace.c | 64 ++++++++++++++++++++++++----------------------- > gdb/nat/linux-ptrace.h | 1 + > 4 files changed, 52 insertions(+), 31 deletions(-) > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index 0ab0362..5c7a21a 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -5025,6 +5025,12 @@ Enables printf debugging output."), > sigdelset (&suspend_mask, SIGCHLD); > > sigemptyset (&blocked_mask); > + > + linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD > + | PTRACE_O_TRACEVFORKDONE > + | PTRACE_O_TRACEVFORK > + | PTRACE_O_TRACEFORK > + | PTRACE_O_TRACEEXEC); > } > > > diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c > index 3ad2113..59c9bfa 100644 > --- a/gdb/nat/linux-ptrace.c > +++ b/gdb/nat/linux-ptrace.c > @@ -37,6 +37,10 @@ > there are no supported features. */ > static int current_ptrace_options = -1; > > +/* Additional flags to test. */ > + > +static int additional_flags; > + > /* Find all possible reasons we could fail to attach PID and append > these as strings to the already initialized BUFFER. '\0' > termination of BUFFER must be done by the caller. */ > @@ -359,16 +363,15 @@ linux_check_ptrace_features (void) > static void > linux_test_for_tracesysgood (int child_pid) > { > -#ifdef GDBSERVER > - /* gdbserver does not support PTRACE_O_TRACESYSGOOD. */ > -#else > - int ret; > + if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0) > + { > + int ret; > > - 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 > + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, > + (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD); > + if (ret != 0) > + additional_flags &= ~PTRACE_O_TRACESYSGOOD; > + } > } > > /* Determine if PTRACE_O_TRACEFORK can be used to follow fork > @@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid) > 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 > - | PTRACE_O_TRACEVFORKDONE)); > - if (ret == 0) > - current_ptrace_options |= PTRACE_O_TRACEVFORKDONE; > -#endif > + if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0) > + { > + /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */ > + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, > + (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK > + | PTRACE_O_TRACEVFORKDONE)); > + if (ret != 0) > + additional_flags &= ~PTRACE_O_TRACEVFORKDONE; > + } > > /* Setting PTRACE_O_TRACEFORK did not cause an error, however we > don't know for sure that the feature is available; old > @@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid) > > /* We got the PID from the grandchild, which means fork > tracing is supported. */ > -#ifdef GDBSERVER > - /* Do not enable all the options for now since gdbserver does not > - properly support them. This restriction will be lifted when > - gdbserver is augmented to support them. */ > - current_ptrace_options |= PTRACE_O_TRACECLONE; > -#else > - current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK > - | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC; > - > - /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to > - support read-only process state. */ > -#endif > + current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags; > > /* Do some cleanup and kill the grandchild. */ > my_waitpid (second_pid, &second_status, 0); > @@ -551,3 +542,14 @@ linux_ptrace_init_warnings (void) > > linux_ptrace_test_ret_to_nx (); > } > + > +/* Set additional ptrace flags to use. Some such flags may be checked > + by the implementation above. This function must be called before > + any other function in this file; otherwise the flags may not take > + effect appropriately. */ > + > +void > +linux_ptrace_set_additional_flags (int flags) > +{ > + additional_flags = flags; > +} > diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h > index cffb5ce..41b3198 100644 > --- a/gdb/nat/linux-ptrace.h > +++ b/gdb/nat/linux-ptrace.h > @@ -91,5 +91,6 @@ extern int linux_supports_tracefork (void); > extern int linux_supports_traceclone (void); > extern int linux_supports_tracevforkdone (void); > extern int linux_supports_tracesysgood (void); > +extern void linux_ptrace_set_additional_flags (int); > > #endif /* COMMON_LINUX_PTRACE_H */ >