Re: [PATCH 02/15] Remove some GDBSERVER checks from linux-ptrace

["Breazeal, Don" <donb@codesourcery.com>]

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  <tromey@redhat.com>
> 	    Gary Benson  <gbenson@redhat.com>
> 
> 	* 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);
>  }
>  \f
>  
> 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 */
>