From 02aae27f008dec343962f5e97a307abbf625d9d7 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Mon, 28 Jun 2021 14:05:54 +0100
Subject: [PATCH] stop_all_threads: (re-)enable async before waiting for stops

Running the gdb.threads/interrupt-while-step-over.exp testcase added
later in the series against gdbserver, after the
TARGET_WAITKIND_NO_RESUMED fix from the following patch would run into
an infinite loop in stop_all_threads.  I added a hacky gdb_assert to
catch the loop, and it looked like this:

   [infrun] stop_all_threads:   Thread 3492141.3492141 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492716 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492717 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492718 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492719 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492720 executing, need stop
   [remote] stop: enter
     [remote] remote_stop_ns: Enqueueing phony stop reply for thread pending vCont-resume (3492141, 3492720, 0)
   [remote] stop: exit
   [infrun] stop_all_threads:   Thread 3492141.3492721 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492722 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492723 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492724 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492725 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492726 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492727 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492728 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492729 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492730 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492731 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492732 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492733 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492734 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492735 not executing
   [remote] wait: enter
   [remote] wait: exit
   [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
   [infrun] print_target_wait_results:   -1.0.0 [process -1],
   [infrun] print_target_wait_results:   status->kind = no-resumed
   [infrun] infrun_async: enable=0
   [infrun] handle_one: status->kind = no-resumed process -1
   [remote] Sending packet: $qXfer:threads:read::0,1000#92
   [remote] Packet received: l<threads>\n<thread id="p35492d.35492d" core="16" name="interrupt-while" handle="40d7d8f7ff7f0000"/>\n<thread id="p35492d.354b6c" core="12" name="interrupt-while" handle="00c7d8f7ff7f0000"/>\n<thread id="p35492d.354b6d" core="5" name="interrupt-while" handle="00b758f7ff7f0000"/>\n<thread id="p35492d.354b6e" core="19" name="interrupt-while" handle="00a7d8f6ff7f0000"/>\n<thread id="p35492d.354b6f" core="15" name="interrupt-while" handle="009758f6ff7f0000"/>\n<thread id="p35492d.354b70" core="6" name="interrupt-whil [1370 bytes omitted]
   [infrun] stop_all_threads:   Thread 3492141.3492141 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492716 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492717 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492718 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492719 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492720 executing, already stopping
   [infrun] stop_all_threads:   Thread 3492141.3492721 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492722 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492723 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492724 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492725 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492726 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492727 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492728 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492729 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492730 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492731 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492732 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492733 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492734 not executing
   [infrun] stop_all_threads:   Thread 3492141.3492735 not executing
 ../../src/gdb/infrun.c:5066: internal-error: void stop_all_threads(): Assertion `0' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n)

What happened above was that gdbserver had sent a
TARGET_WAITKIND_NO_RESUMED stop reply (N) to GDB already, and that
event was still in transit, held in remote.c's stop reply queue, not
yet seen by infrun.  In stop_all_threads, GDB told remote.c to stop
thread 3492141.3492720.  That thread had a pending vCont resume, so
remote.c queued a phony stop.  This event is queued _after_ the
TARGET_WAITKIND_NO_RESUMED stop reply that was already there.
stop_all_threads calls wait_one to collect one event and gets back
that TARGET_WAITKIND_NO_RESUMED.  Seeing TARGET_WAITKIND_NO_RESUMED,
wait_one disables target_async for that target to stop listing to
events from that target.  No other unresumed target has async active,
so wait_one returns TARGET_WAITKIND_NO_RESUMED.  stop_all_threads
still hasn't seen the stop for 3492141.3492720, so we go for another
iteration.  But since the target is not async, wait_one returns
TARGET_WAITKIND_NO_RESUMED without calling target_wait.  The queued
event for 3492141.3492720 remains pending forever, and
stop_all_threads keeps iterating forever.

The comment in the patch describes a different scenario that does not
involve the phony stop:

	/* wait_one waits for events until it sees a
	TARGET_WAITKIND_NO_RESUMED.  When it sees one, it disables
	target_async for the target and no longer waits for events from
	that target.  TARGET_WAITKIND_NO_RESUMED can be delayed though,
	consider:

     #1 - threads 2-5 are stopped, thread 1 is running

     #2 - target reports breakpoint hit for thread 1, event
	  is queued.

     #3 - target reports no-resumed left, event is queued

     #4 - user resumes all threads

     #5 - gdb decides to stop all threads, stops threads 1-5

     #6 - wait_one returns the queued breakpoint hit for
	  thread 1

     #7 - wait_one returns the queued no-resumed event,
	  wait_one stops waiting for events from target.

     #8 - we still haven't seen the stops for threads 2-5, so
	  we do another pass.

     #9 - if the target is not async wait_one doesn't wait on
	  the target, so it won't see the stops.

     #a - we'd loop forever with WAITS_NEEDED > 0.

Fix this by explicitly enabling target async on each iteration, before
starting the wait_one loop.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>

	PR gdb/27338
	* infrun.c (reenable_target_async): New.
	(stop_all_threads): Enable/re-enable async on targets that can
	async before waiting for stops.

Change-Id: Ie3ffb0df89635585a6631aa842689cecc989e33f
---
 gdb/infrun.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 53331f27a0a..a2d63c4fe89 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4961,6 +4961,55 @@ handle_one (const wait_one_event &event)
   return false;
 }
 
+/* Helper for stop_all_threads.  wait_one waits for events until it
+   sees a TARGET_WAITKIND_NO_RESUMED.  When it sees one, it disables
+   target_async for the target and no longer waits for events from
+   that target.  TARGET_WAITKIND_NO_RESUMED can be delayed though,
+   consider:
+
+    #1 - threads 2-5 are stopped, thread 1 is running
+
+    #2 - target reports breakpoint hit for thread 1, event is queued
+
+    #3 - target reports no-resumed left, event is queued
+
+    #4 - user resumes all threads
+
+    #5 - gdb decides to stop all threads, stops threads 1-5
+
+    #6 - wait_one returns the queued breakpoint hit for thread 1
+
+    #7 - wait_one returns the queued no-resumed event, wait_one stops
+	 waiting for events from target
+
+    #8 - we still haven't seen the stops for threads 2-5, so we do
+	 another pass
+
+    #9 - if the target is not async wait_one doesn't wait on the
+	 target, so it won't see the stops.
+
+    #a - we'd loop forever with WAITS_NEEDED > 0.
+
+   To handle this, we need to explicitly (re-)enable target async on
+   all targets that can async.  */
+
+static void
+reenable_target_async ()
+{
+  for (inferior *inf : all_inferiors ())
+    {
+      process_stratum_target *target = inf->process_target ();
+      if (target != nullptr
+	  && target->threads_executing
+	  && target->can_async_p ()
+	  && !target->is_async_p ())
+	{
+	  switch_to_inferior_no_thread (inf);
+	  target_async (1);
+	}
+    }
+}
+
 /* See infrun.h.  */
 
 void
@@ -5071,6 +5120,8 @@ stop_all_threads (void)
 	  if (pass > 0)
 	    pass = -1;
 
+	  reenable_target_async ();
+
 	  for (int i = 0; i < waits_needed; i++)
 	    {
 	      wait_one_event event = wait_one ();