RE: [PATCH v7 5/5] gdb/infrun: handle already-exited threads when attempting to stop

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

On Wednesday, April 22, 2020 5:01 PM, Tankut Baris Aktemur wrote:
> In stop_all_threads, GDB sends signals to other threads in an attempt
> to stop them.  While in a typical scenario the expected wait status is
> TARGET_WAITKIND_STOPPED, it is possible that the thread GDB attempted
> to stop has already terminated.  If so, a waitstatus other than
> TARGET_WAITKIND_STOPPED would be received.  Handle this case
> appropriately.

I noticed that at the beginning of stop_all_threads, we have this:

  scoped_restore_current_thread restore_thread;

  target_thread_events (1);
  SCOPE_EXIT { target_thread_events (0); };

This is not quite right, because the current top target at the scope exit
time is not necessarily the same as the current top target at the function
start.  The 'scoped_restore_current_thread' should have come after 'SCOPE_EXIT
 { target_thread_events (0); };'.  Yet, this re-ordering of the statements
would still not be sufficient, because that would enable/disable the thread
events for only the current top target.  I think this has to be done for all
targets.

So, I've just emailed a two-patch series where the first one defines a convenience
method, 'all_non_exited_process_targets', to get a collection of targets:

  https://sourceware.org/pipermail/gdb-patches/2020-May/168172.html
  https://sourceware.org/pipermail/gdb-patches/2020-May/168173.html 

During the review period of the multi-target patches, Tom Tromey had commented
that such a method to allow for-each iteration over the targets would be useful.
For instance, a place where 'all_non_exited_process_targets' could be used is...

> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 167d50ff3ab..93169269553 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4781,7 +4781,47 @@ stop_all_threads (void)
>  	{
>  	  int need_wait = 0;
> 
> -	  update_thread_list ();
> +	  for (inferior *inf : all_non_exited_inferiors ())
> +	    {
> +	      update_thread_list ();

... this iteration, to avoid calling 'update_thread_list' multiple times on
a particular target.  I can submit a revision, if you think that's appropriate.

Regards.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928