Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events

[Luis Machado <lgustavo@codesourcery.com>]

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 <don_breazeal@mentor.com>
>
> [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  <donb@codesourcery.com>
>
> 	* 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  <donb@codesourcery.com>
>
> 	* 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