remote_target::update_thread_list: Don't delete stepping threads

Without this patch, the
gdb.threads/step-over-thread-exit-while-stop-all-threads.exp testcase
added later in the series would randomly fail against gdbserver.  It
would hit this assertion failure:

  src/gdb/displaced-stepping.c:54: internal-error: displaced_step_prepare_status displaced_step_buffers::prepare(thread_info*, CORE_ADDR&): Assertion `buf.current_thread != thread' failed.

That assertion was a bit of a head scratcher.  Turns out that is was
caused by a 'buf.current_thread' being a stale pointer to a thread
that had already been deleted.  I confirmed that this is what was
happening with the following hack:

  --- c/gdb/displaced-stepping.c
  +++ w/gdb/displaced-stepping.c
  @@ -145,6 +145,7 @@ displaced_step_buffers::prepare (thread_info *thread, CORE_ADDR &displaced_pc)

    /* This marks the buffer as being in use.  */
    buffer->current_thread = thread;
  +  buffer->current_thread_id = thread->global_num;

When the assertion triggers, I had for example:

 (top-gdb) p buf.current_thread_id
 $1 = 205
 (top-gdb) p thread->global_num
 $2 = 308

So, same pointer, but different threads.

Here's how it happens:

While GDB is stopping threads with stop_all_threads, it updates the
thread list, which reaches remote_target::update_thread_list.  Any
thread that is no longer found in the remote end is removed from GDB's
thread list by that function.  If the thread that is gone was one that
was stepping over a breakpoint set on top of a thread exit syscall,
then update_thread_list deletes the thread straight away, which leaves
the displaced stepping buffer with a stale pointer to the thread.

Later, when a new thread_info object is created (the testcase
continuously spawns threads), the heap may well return the same memory
address for the new object as the displaced stepping buffer's stale
pointer still points to.  That is what happened in the assertion
triggered above -- in reality the buffer's 'buf.current_thread'
conceptually pointed to a different thread not 'thread', but in
practice, the pointers had the same value.

What we really want to do is release the displaced stepping buffer, so
some other thread can reuse it, and we want to let another thread have
its turn at stepping over a breakpoint.  The simplest way is to make
update_thread_list not delete threads that are stepping, as we know
that we will get a 'w' (TARGET_WAITKIND_THREAD_EXITED) stop reply for
them shortly, so that infrun takes care of all that.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <[email protected]>

	PR gdb/27338
	* remote.c (remote_target::update_thread_list): Don't delete
	missing threads that were stepping.

Change-Id: I4a8687b48da8bf1d9cc23981c427722dd63fecb6

gdb/remote.c

@@ -3943,6 +3943,8 @@ remote_target::update_thread_list ()
 
 	  if (!context.contains_thread (tp->ptid))
 	    {
+	      /* Not found.  */
+
 	      /* Do not remove the thread if it is the last thread in
 		 the inferior.  This situation happens when we have a
 		 pending exit process status to process.  Otherwise we
@@ -3951,7 +3953,22 @@ remote_target::update_thread_list ()
 	      if (has_single_non_exited_thread (tp->inf))
 		continue;
 
-	      /* Not found.  */
+	      /* If the thread was stepping, then don't delete it yet,
+		 let infrun process the thread exit event which should
+		 come shortly.  If this thread was stepping over a
+		 breakpoint, we want infrun to see the exit so it can
+		 free the displaced stepping buffer (if used), and
+		 maybe start a new step-over in another thread (if any
+		 is waiting).  */
+	      remote_thread_info *remote_thr = get_remote_thread_info (tp);
+	      if (packet_support (PACKET_stepped_thread_exited) == PACKET_ENABLE
+		  && remote_thr->get_resume_state () != resume_state::NOT_RESUMED
+		  && remote_thr->resume_info ().step)
+		continue;
+
+	      /* The core doesn't particularly care about the thread,
+		 and a THREAD_EXITED event is likely not coming.  Go
+		 ahead and delete it immediately.  */
 	      delete_thread (tp);
 	    }
 	}