[review v3] infrun: handle already-exited threads when attempting to stop

["Tankut Baris Aktemur (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................

infrun: handle already-exited threads when attempting to stop

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.

If a wait status that denotes thread termination is ignored, GDB goes
into an infinite loop in stop_all_threads.
E.g.:

~~~
$ gdb ./a.out
(gdb) start
...
(gdb) add-inferior -exec ./a.out
...
(gdb) inferior 2
...
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) set debug infrun 2
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 23419)
infrun: clear_proceed_status_thread (process 23703)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 23419
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23419] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 23703
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23703] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   23703.23703.0 [process 23703],
infrun:   status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 23703) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun:   process 23419 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun:   23419.23419.0 [process 23419],
infrun:   status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 23419
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
~~~

And this polling goes on forever.  This patch prevents the infinite
looping behavior.  For the same scenario above, we obtain the
following behavior:

~~~
...
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 8564)
infrun: clear_proceed_status_thread (process 8652)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 8564
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 8564] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 8652
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 8652] at 0x55555555514e
infrun: prepare_to_wait
infrun: Found 2 inferiors, starting at #0
infrun: target_wait (-1.0.0, status) =
infrun:   8564.8564.0 [process 8564],
infrun:   status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 1 (process 8564) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun:   process 8652 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun:   8652.8652.0 [process 8652],
infrun:   status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 8652
infrun: saving status status->kind = exited, status = 0 for 8652.8652.0
infrun:   process 8652 not executing
infrun: stop_all_threads, pass=1, iterations=1
infrun:   process 8652 not executing
infrun: stop_all_threads done
(gdb) infrun: Using pending wait status status->kind = exited, status = 0 for process 8652.
infrun: target_wait (-1.0.0, status) =
infrun:   8652.8652.0 [process 8652],
infrun:   status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 8652) exited normally]
infrun: stop_waiting
info inferiors
  Num  Description       Connection           Executable
  1    <null>                                 a.out
* 2    <null>                                 a.out
(gdb) info threads
No threads.
(gdb)
~~~

gdb/ChangeLog:
2020-02-05  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
	    Tom de Vries  <tdevries@suse.de>

	PR threads/25478
	* infrun.c (stop_all_threads): Do NOT ignore
	TARGET_WAITKIND_NO_RESUMED, TARGET_WAITKIND_THREAD_EXITED,
	TARGET_WAITKIND_EXITED, TARGET_WAITKIND_SIGNALLED wait statuses
	received from threads we attempt to stop.

Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
M gdb/infrun.c
1 file changed, 43 insertions(+), 5 deletions(-)



diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9224376..77c64da 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4799,9 +4799,14 @@
 					"infrun:   %s not executing\n",
 					target_pid_to_str (t->ptid).c_str ());
 
-		  /* The thread may be not executing, but still be
-		     resumed with a pending status to process.  */
-		  t->resumed = false;
+		  if ((t->suspend.waitstatus.kind == TARGET_WAITKIND_SIGNALLED
+		       || t->suspend.waitstatus.kind == TARGET_WAITKIND_EXITED)
+		      && t->suspend.waitstatus_pending_p)
+		    ;
+		  else
+		    /* The thread may be not executing, but still be
+		       resumed with a pending status to process.  */
+		    t->resumed = false;
 		}
 	    }
 
@@ -4829,8 +4834,41 @@
 	      || event.ws.kind == TARGET_WAITKIND_EXITED
 	      || event.ws.kind == TARGET_WAITKIND_SIGNALLED)
 	    {
-	      /* All resumed threads exited
-		 or one thread/process exited/signalled.  */
+	      /* Eventing target is null if there were no waitable
+		 targets.  */
+	      if (event.target != nullptr)
+		{
+		  thread_info *t = find_thread_ptid (event.target,
+						     event.ptid);
+		  /* Check if this is the first time we see this
+		     thread.  Don't bother adding if it exited.  */
+		  if (t == nullptr
+		      && event.ws.kind != TARGET_WAITKIND_THREAD_EXITED)
+		    t = add_thread (event.target, event.ptid);
+
+		  if (event.ws.kind != TARGET_WAITKIND_THREAD_EXITED)
+		    {
+		      /* Set the threads as non-executing to avoid
+			 another stop attempt on them.  */
+		      mark_non_executing_threads (event.target, event.ptid,
+						  event.ws);
+
+		      if (event.ws.kind == TARGET_WAITKIND_NO_RESUMED)
+			{
+			  /* Do nothing.  Already marked the threads.  */
+			}
+		      else
+			{
+			  /* TARGET_WAITKIND_EXITED or
+			     TARGET_WAITKIND_SIGNALLED.  */
+			  save_waitstatus (t, &event.ws);
+			  /* This was cleared in mark_non_executing_threads.
+			     Set it so this pending event is considered by
+			     do_target_wait.  */
+			  t->resumed = true;
+			}
+		    }
+		}
 	    }
 	  else
 	    {

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: newpatchset