diff mbox

[1/4,REPOST] Remote Linux ptrace exit events

Message ID 1398885482-8449-2-git-send-email-donb@codesourcery.com
State New
Headers show

Commit Message

Don Breazeal April 30, 2014, 7:17 p.m. UTC
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(-)

Comments

Luis Machado May 7, 2014, 7:39 p.m. UTC | #1
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
Yao Qi May 8, 2014, 5:20 a.m. UTC | #2
Don,
I am not the right person to review this patch series and approve them,
but I can do a primary review.  Hope it helps to relieve feeling of the
long time review :)

On 05/01/2014 03:17 AM, Don Breazeal wrote:
> 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.

Can you elaborate a little please?  why existing wait mechanism is not
reliable?

>  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
>  }
>  

The code looks clear to me, but it is strange to put them in function
linux_test_for_tracefork.  IWBN to move them to a new function.

>  /* 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 */

Period and one space is missing at the end of the comment.

> +
> +static int
> +is_extended_waitstatus (int wstat)
> +{
> +    return wstat >> 16 != 0;
   ^^^^
Two spaces instead of four.

> +}
> +
> +/* Check wait status for extended event */

Likewise.

> +
> +static int
> +is_extended_exit (int wstat)
> +{
> +    return wstat >> 16 == PTRACE_EVENT_EXIT;
   ^^^^
Likewise.

> +}
> +
>  /* Handle a GNU/Linux extended wait response.  If we see a clone
>     event, we need to add the new LWP to our list (and not report the

I know it is not about your change, but better to replace LWP with
EVENT_CHILD.  We also need to update it for the exit event.

> -   trap to higher layers).  */
> +   trap to higher layers).  This function returns non-zero if the
> +   event should be ignored and we should wait again.  The wait status
> +   in WSTATP may be modified if an exit event occurred.  */
>  
> -static void
> -handle_extended_wait (struct lwp_info *event_child, int wstat)
> +static int
> +handle_extended_wait (struct lwp_info *event_child, int *wstatp)
>  {
> +  int wstat = *wstatp;
>    int event = wstat >> 16;
>    struct thread_info *event_thr = get_lwp_thread (event_child);
>    struct lwp_info *new_lwp;
> @@ -448,11 +468,54 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
>  	    linux_resume_one_lwp (new_lwp, 0, WSTOPSIG (status), NULL);
>  	}
>  
> +      /* Enable extended events for the new thread.  */
> +      linux_enable_event_reporting (new_pid);
> +

This change isn't mentioned in ChangeLog entry.  I also wonder why is it
needed here.

>        /* Always resume the current thread.  If we are stopping
>  	 threads, it will have a pending SIGSTOP; we may as well
>  	 collect it now.  */
>        linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
> +
> +      /* Don't report the event.  */
> +      return 1;
> +     }
> +  else if (event == PTRACE_EVENT_EXIT)
> +    {
> +      unsigned long exit_status;
> +      unsigned long lwpid = lwpid_of (event_thr);
> +      int ret;
> +
> +      if (debug_threads)
> +        debug_printf ("LHEW: Got exit event from LWP %ld\n",

s/LHEW/HEW/

> +                      lwpid_of (event_thr));
> +
> +      ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
> +	      &exit_status);
> +
> +      if (num_lwps (pid_of (event_thr)) > 1)
> +        {
> +	  /* If there is at least one more LWP, then the program still
> +	     exists and the exit should not be reported to GDB.  */
> +          delete_lwp (event_child);
> +          ret = 1;
> +        }
> +      else
> +        {
> +          /* Set the exit status to the actual exit status, so normal
> +             WIFEXITED/WIFSIGNALLED processing and reporting for the
> +             last lwp in the process can proceed from here.  */
> +          *wstatp = exit_status;
> +          ret = 0;
> +        }
> +
> +      /* Resume the thread so that it actually exits.  Subsequent exit
> +         events for LWPs that were deleted above will be ignored.  */
> +      ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
> +              (PTRACE_TYPE_ARG4) 0);
> +      return ret;
>      }
> +  internal_error (__FILE__, __LINE__,
> +	          _("unknown ptrace event %d"), event);

This should be mentioned in ChangeLog too.
Don Breazeal May 9, 2014, 9:03 p.m. UTC | #3
Hi Yao,
Thanks for the review.

On 5/7/2014 10:20 PM, Yao Qi wrote:
> Don,
> I am not the right person to review this patch series and approve them,
> but I can do a primary review.  Hope it helps to relieve feeling of the
> long time review :)
>
> On 05/01/2014 03:17 AM, Don Breazeal wrote:
>> 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.
>
> Can you elaborate a little please?  why existing wait mechanism is not
> reliable?

I'm working on a write-up that I will include in the next version of the 
patch.  The short version is that there is a race condition in the 
current implementation that can leave a dangling entry in the lwp list 
(an entry that doesn't have a corresponding actual lwp).  In this case 
gdbserver will hang waiting for the non-existent lwp to stop.  Using the 
exit events eliminates this race condition.

>
>>   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
>>   }
>>
>
> The code looks clear to me, but it is strange to put them in function
> linux_test_for_tracefork.  IWBN to move them to a new function.

I had gone back and forth on this issue.  I will make a new function for 
this.

>
>>   /* 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 */
>
> Period and one space is missing at the end of the comment.

I will clean up everything you mentioned from here to the bottom.
thanks
--Don

>
>> +
>> +static int
>> +is_extended_waitstatus (int wstat)
>> +{
>> +    return wstat >> 16 != 0;
>     ^^^^
> Two spaces instead of four.
>
>> +}
>> +
>> +/* Check wait status for extended event */
>
> Likewise.
>
>> +
>> +static int
>> +is_extended_exit (int wstat)
>> +{
>> +    return wstat >> 16 == PTRACE_EVENT_EXIT;
>     ^^^^
> Likewise.
>
>> +}
>> +
>>   /* Handle a GNU/Linux extended wait response.  If we see a clone
>>      event, we need to add the new LWP to our list (and not report the
>
> I know it is not about your change, but better to replace LWP with
> EVENT_CHILD.  We also need to update it for the exit event.
>
>> -   trap to higher layers).  */
>> +   trap to higher layers).  This function returns non-zero if the
>> +   event should be ignored and we should wait again.  The wait status
>> +   in WSTATP may be modified if an exit event occurred.  */
>>
>> -static void
>> -handle_extended_wait (struct lwp_info *event_child, int wstat)
>> +static int
>> +handle_extended_wait (struct lwp_info *event_child, int *wstatp)
>>   {
>> +  int wstat = *wstatp;
>>     int event = wstat >> 16;
>>     struct thread_info *event_thr = get_lwp_thread (event_child);
>>     struct lwp_info *new_lwp;
>> @@ -448,11 +468,54 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
>>   	    linux_resume_one_lwp (new_lwp, 0, WSTOPSIG (status), NULL);
>>   	}
>>
>> +      /* Enable extended events for the new thread.  */
>> +      linux_enable_event_reporting (new_pid);
>> +
>
> This change isn't mentioned in ChangeLog entry.  I also wonder why is it
> needed here.
>
>>         /* Always resume the current thread.  If we are stopping
>>   	 threads, it will have a pending SIGSTOP; we may as well
>>   	 collect it now.  */
>>         linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
>> +
>> +      /* Don't report the event.  */
>> +      return 1;
>> +     }
>> +  else if (event == PTRACE_EVENT_EXIT)
>> +    {
>> +      unsigned long exit_status;
>> +      unsigned long lwpid = lwpid_of (event_thr);
>> +      int ret;
>> +
>> +      if (debug_threads)
>> +        debug_printf ("LHEW: Got exit event from LWP %ld\n",
>
> s/LHEW/HEW/
>
>> +                      lwpid_of (event_thr));
>> +
>> +      ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
>> +	      &exit_status);
>> +
>> +      if (num_lwps (pid_of (event_thr)) > 1)
>> +        {
>> +	  /* If there is at least one more LWP, then the program still
>> +	     exists and the exit should not be reported to GDB.  */
>> +          delete_lwp (event_child);
>> +          ret = 1;
>> +        }
>> +      else
>> +        {
>> +          /* Set the exit status to the actual exit status, so normal
>> +             WIFEXITED/WIFSIGNALLED processing and reporting for the
>> +             last lwp in the process can proceed from here.  */
>> +          *wstatp = exit_status;
>> +          ret = 0;
>> +        }
>> +
>> +      /* Resume the thread so that it actually exits.  Subsequent exit
>> +         events for LWPs that were deleted above will be ignored.  */
>> +      ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
>> +              (PTRACE_TYPE_ARG4) 0);
>> +      return ret;
>>       }
>> +  internal_error (__FILE__, __LINE__,
>> +	          _("unknown ptrace event %d"), event);
>
> This should be mentioned in ChangeLog too.
>
diff mbox

Patch

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;
+}
+
 /* Handle a GNU/Linux extended wait response.  If we see a clone
    event, we need to add the new LWP to our list (and not report the
-   trap to higher layers).  */
+   trap to higher layers).  This function returns non-zero if the
+   event should be ignored and we should wait again.  The wait status
+   in WSTATP may be modified if an exit event occurred.  */
 
-static void
-handle_extended_wait (struct lwp_info *event_child, int wstat)
+static int
+handle_extended_wait (struct lwp_info *event_child, int *wstatp)
 {
+  int wstat = *wstatp;
   int event = wstat >> 16;
   struct thread_info *event_thr = get_lwp_thread (event_child);
   struct lwp_info *new_lwp;
@@ -448,11 +468,54 @@  handle_extended_wait (struct lwp_info *event_child, int wstat)
 	    linux_resume_one_lwp (new_lwp, 0, WSTOPSIG (status), NULL);
 	}
 
+      /* Enable extended events for the new thread.  */
+      linux_enable_event_reporting (new_pid);
+
       /* Always resume the current thread.  If we are stopping
 	 threads, it will have a pending SIGSTOP; we may as well
 	 collect it now.  */
       linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
+
+      /* Don't report the event.  */
+      return 1;
+     }
+  else if (event == PTRACE_EVENT_EXIT)
+    {
+      unsigned long exit_status;
+      unsigned long lwpid = lwpid_of (event_thr);
+      int ret;
+
+      if (debug_threads)
+        debug_printf ("LHEW: Got exit event from LWP %ld\n",
+                      lwpid_of (event_thr));
+
+      ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
+	      &exit_status);
+
+      if (num_lwps (pid_of (event_thr)) > 1)
+        {
+	  /* If there is at least one more LWP, then the program still
+	     exists and the exit should not be reported to GDB.  */
+          delete_lwp (event_child);
+          ret = 1;
+        }
+      else
+        {
+          /* Set the exit status to the actual exit status, so normal
+             WIFEXITED/WIFSIGNALLED processing and reporting for the
+             last lwp in the process can proceed from here.  */
+          *wstatp = exit_status;
+          ret = 0;
+        }
+
+      /* Resume the thread so that it actually exits.  Subsequent exit
+         events for LWPs that were deleted above will be ignored.  */
+      ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
+              (PTRACE_TYPE_ARG4) 0);
+      return ret;
     }
+  internal_error (__FILE__, __LINE__,
+	          _("unknown ptrace event %d"), event);
 }
 
 /* Return the PC as read from the regcache of LWP, without any
@@ -516,7 +579,7 @@  get_stop_pc (struct lwp_info *lwp)
   if (WSTOPSIG (lwp->last_status) == SIGTRAP
       && !lwp->stepping
       && !lwp->stopped_by_watchpoint
-      && lwp->last_status >> 16 == 0)
+      && !is_extended_waitstatus (lwp->last_status))
     stop_pc -= the_low_target.decr_pc_after_break;
 
   if (debug_threads)
@@ -1031,7 +1094,7 @@  get_detach_signal (struct thread_info *thread)
     }
 
   /* Extended wait statuses aren't real SIGTRAPs.  */
-  if (WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+  if (WSTOPSIG (status) == SIGTRAP && is_extended_waitstatus (status))
     {
       if (debug_threads)
 	debug_printf ("GPS: lwp %s had stopped with extended "
@@ -1716,11 +1779,13 @@  cancel_breakpoint (struct lwp_info *lwp)
 
 /* Do low-level handling of the event, and check if we should go on
    and pass it to caller code.  Return the affected lwp if we are, or
-   NULL otherwise.  */
+   NULL otherwise.  The wait status in WSTATP may be modified if an
+   exit event occurred.  */
 
 static struct lwp_info *
-linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
+linux_low_filter_event (ptid_t filter_ptid, int lwpid, int *wstatp)
 {
+  int wstat = *wstatp;
   struct lwp_info *child;
   struct thread_info *thread;
 
@@ -1772,7 +1837,7 @@  linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
      architecture being defined already (so that CHILD has a valid
      regcache), and on LAST_STATUS being set (to check for SIGTRAP or
      not).  */
-  if (WIFSTOPPED (wstat))
+  if (WIFSTOPPED (wstat) && !is_extended_exit (wstat))
     {
       if (debug_threads
 	  && the_low_target.get_pc != NULL)
@@ -1808,7 +1873,8 @@  linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
      changes the debug registers meanwhile, we have the cached data we
      can rely on.  */
 
-  if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP)
+  if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
+      && !is_extended_waitstatus (wstat))
     {
       if (the_low_target.stopped_by_watchpoint == NULL)
 	{
@@ -1844,10 +1910,17 @@  linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
     }
 
   if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
-      && wstat >> 16 != 0)
+      && is_extended_waitstatus (wstat))
     {
-      handle_extended_wait (child, wstat);
-      return NULL;
+      if (handle_extended_wait (child, &wstat))
+        return NULL;
+      else
+        {
+          /* Update caller's wstat in case handle_extended_wait fixed
+             it up to reflect the actual status.  */
+          *wstatp = wstat;
+          return child;
+        }
     }
 
   if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGSTOP
@@ -2067,7 +2140,7 @@  linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
 	    }
 
 	  event_child = linux_low_filter_event (filter_ptid,
-						ret, *wstatp);
+						ret, wstatp);
 	  if (event_child != NULL)
 	    {
 	      /* We got an event to report to the core.  */