[PATCH 03/10] remote+gdbserver: stepped-thread-exited feature

Pedro Alves pedro@palves.net
Fri Jul 2 13:01:47 GMT 2021


This patch makes GDB and GDBserver each tell the other that they
support reporting 'w' (thread exit) stop replies for stepping threads
that exit, even without QThreadEvents active.

Having GDBserver tell GDB that it sends thread exited events for
stepping threads that exit is important to fix one race, in a
following patch.

This also avoids breaking backward compatibility.  If we make
gdbserver send 'w' stop replies for stepping threads that exit, we'd
get:

  Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/gdb/testsuite/lib/my-syscalls.S:68
  68              syscall
  (gdb) s
  No unwaited-for children left.

After that same change (and without this fix), we get:

  Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/gdb/testsuite/lib/my-syscalls.S:68
  68              syscall
  (gdb) s
  warning: Invalid remote reply: w0;p27c295.27cb15

At this point, the target is stopped, but GDB is still waiting.  All
you can do is stop debugging:

  ^C^CThe target is not responding to interrupt requests.
  Stop debugging it? (y or n) Quit
  (gdb)

With an old gdb vs new gdbserver, we'll instead get:

  ...
  Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/gdb/testsuite/lib/my-syscalls.S:68
  68              syscall
  (gdb) s
  warning: Remote failure reply: E.Thread exited.
  Could not read registers; remote failure reply 'E01'
  Could not read registers; remote failure reply 'E01'
  (gdb) info threads
    Id   Target Id                                Frame
    1    Thread 2486981.2486981 "step-over-threa" __pthread_clockjoin_ex (....) at pthread_join_common.c:145

  The current thread <Thread ID 2> has terminated.  See `help thread'.
  (gdb)

This may look like a regression against before the change to make
gdbserver send 'w' stop replies for stepping threads that exit, but
keep in mind that old GDB does not release the displaced stepping
buffer on no-resumed, leading to hangs on subsequent resumes.

No GDBserver backend enables the feature yet.  That will happen in a
following patch.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <pedro@palves.net>
	    Simon Marchi  <simon.marchi@efficios.com>

	PR gdb/27338
	* NEWS: Mention stepped-thread-exited feature in qSupported.
	* remote.c (PACKET_stepped_thread_exited): New.
	(remote_protocol_features): Add "stepped-thread-exited" entry.
	(remote_target::remote_query_supported): Report
	stepped-thread-exited+.
	(remote_target::wait_as): Handle 'w' (aka thread exit) stop reply.
	(_initialize_remote): Register "set/show remote
	stepped-thread-exited-stop-reply".

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

	PR gdb/27338
	* server.cc (handle_query): Handle and report
	stepped-thread-exited+.
	(resume): Handle TARGET_WAITKIND_THREAD_EXITED in all-stop mode.

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

	PR gdb/27338
	* gdb.texinfo (Remote Configuration): Document
	stepped-thread-exited-stop-reply.
	(Stop Reply Packets) <w stop reply>: Mention
	stepped-threads-exited feature.
	(General Query Packets): Document stepped-thread-exited.

Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I299f7e73c5702738dfaf5d18bc9e957da473055d
---
 gdb/NEWS            |  5 +++++
 gdb/doc/gdb.texinfo | 28 +++++++++++++++++++++++++++-
 gdb/remote.c        | 20 +++++++++++++++++++-
 gdbserver/server.cc | 28 +++++++++++++++++++++++++++-
 gdbserver/target.cc |  6 ++++++
 gdbserver/target.h  |  7 +++++++
 6 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 7f3ed4f02f0..967886c2958 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -239,6 +239,11 @@ QMemTags
   Request the remote to store the specified allocation tags to the requested
   memory range.
 
+stepped-thread-exited feature in qSupported
+  Indicates that GDB is expecting thread exit (w) stop replies for
+  threads that were single-stepping, even when QThreadEvents is not in
+  effect.
+
 * Guile API
 
   ** Improved support for rvalue reference values:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f1c3e7ba847..e175ad14fa5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23463,6 +23463,10 @@ are:
 @tab @code{no resumed thread left stop reply}
 @tab Tracking thread lifetime.
 
+@item @code{stepped-thread-exited-stop-reply}
+@tab @code{stepped thread exited stop reply}
+@tab Tracking thread lifetime.
+
 @end multitable
 
 @node Remote Stub
@@ -40953,7 +40957,11 @@ hex strings.
 The thread exited, and @var{AA} is the exit status.  This response
 should not be sent by default; @value{GDBN} requests it with the
 @ref{QThreadEvents} packet.  See also @ref{thread create event} above.
-@var{AA} is formatted as a big-endian hex string.
+If @value{GDBN} reported support for the @samp{stepped-thread-exited}
+@samp{qSupported} feature (@pxref{qSupported}), the remote stub should
+send this response for stepping threads that exit, even if thread
+create/exit events are disabled with @samp{QThreadEvents:0}.  @var{AA}
+is formatted as a big-endian hex string.
 
 @item N
 There are no resumed threads left in the target.  In other words, even
@@ -41870,6 +41878,16 @@ including @samp{exec-events+} in its @samp{qSupported} reply.
 @item vContSupported
 This feature indicates whether @value{GDBN} wants to know the
 supported actions in the reply to @samp{vCont?} packet.
+
+@item stepped-thread-exited
+This feature indicates whether @value{GDBN} expects thread exited
+events for stepping threads, even if @samp{QThreadEvents:1} was not
+requested.  This can also be used as indication that @value{GDBN}
+understands the @samp{w} stop reply (@pxref{thread exit event}) in
+all-stop mode.  Prior versions of @value{GDBN} considered such a
+response an error.  @value{GDBN} does not expect such events unless
+the stub also reports that it reports them by including
+@samp{stepped-thread-exited+} in its @samp{qSupported} reply.
 @end table
 
 Stubs should ignore any unknown values for
@@ -42143,6 +42161,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{stepped-thread-exited}
+@tab No
+@tab @samp{-}
+@tab No
+
 @item @samp{memory-tagging}
 @tab No
 @tab @samp{-}
@@ -42362,6 +42385,9 @@ The remote stub understands the @samp{QThreadEvents} packet.
 @item no-resumed
 The remote stub reports the @samp{N} stop reply.
 
+@item stepped-thread-exited
+The remote stub reports the @samp{w} stop reply for single-stepped
+threads that exit.
 
 @item memory-tagging
 The remote stub supports and implements the required memory tagging
diff --git a/gdb/remote.c b/gdb/remote.c
index adc53e324d0..e1162fc3664 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2180,6 +2180,11 @@ enum {
   /* Support TARGET_WAITKIND_NO_RESUMED.  */
   PACKET_no_resumed,
 
+  /* Support TARGET_WAITKIND_THREAD_EXITED / 'w' stop replies when
+     single-stepped threads exit.  Also indicates that GDB understands
+     'w' stop replies in the all-stop protocol.  */
+  PACKET_stepped_thread_exited,
+
   /* Support for memory tagging, allocation tag fetch/store
      packets and the tag violation stop replies.  */
   PACKET_memory_tagging_feature,
@@ -5331,6 +5336,8 @@ static const struct protocol_feature remote_protocol_features[] = {
   { "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported },
   { "QThreadEvents", PACKET_DISABLE, remote_supported_packet, PACKET_QThreadEvents },
   { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
+  { "stepped-thread-exited", PACKET_DISABLE, remote_supported_packet,
+    PACKET_stepped_thread_exited },
   { "memory-tagging", PACKET_DISABLE, remote_supported_packet,
     PACKET_memory_tagging_feature },
 };
@@ -5427,6 +5434,9 @@ remote_target::remote_query_supported ()
       if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "no-resumed+");
 
+      if (packet_set_cmd_state (PACKET_stepped_thread_exited) != AUTO_BOOLEAN_FALSE)
+	remote_query_supported_append (&q, "stepped-thread-exited+");
+
       if (packet_set_cmd_state (PACKET_memory_tagging_feature)
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "memory-tagging+");
@@ -8207,6 +8217,7 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
       status->kind = TARGET_WAITKIND_STOPPED;
       status->value.sig = GDB_SIGNAL_0;
       break;
+
     case 'F':		/* File-I/O request.  */
       /* GDB may access the inferior memory while handling the File-I/O
 	 request, but we don't want GDB accessing memory while waiting
@@ -8219,7 +8230,8 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
 	 again.  Keep waiting for events.  */
       rs->waiting_for_stop_reply = 1;
       break;
-    case 'N': case 'T': case 'S': case 'X': case 'W':
+
+    case 'N': case 'T': case 'S': case 'X': case 'W': case 'w':
       {
 	/* There is a stop reply to handle.  */
 	rs->waiting_for_stop_reply = 0;
@@ -8232,9 +8244,11 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
 	event_ptid = process_stop_reply (stop_reply, status);
 	break;
       }
+
     case 'O':		/* Console output.  */
       remote_console_output (buf + 1);
       break;
+
     case '\0':
       if (rs->last_sent_signal != GDB_SIGNAL_0)
 	{
@@ -15245,6 +15259,10 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_no_resumed],
 			 "N stop reply", "no-resumed-stop-reply", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_stepped_thread_exited],
+			 "w stop reply (after step)",
+			 "stepped-thread-exited-stop-reply", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_memory_tagging_feature],
 			 "memory-tagging-feature", "memory-tagging-feature", 0);
 
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 32dcc05924e..eb4cd89dbad 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -85,6 +85,10 @@ bool run_once;
 /* Whether to report TARGET_WAITKIND_NO_RESUMED events.  */
 static bool report_no_resumed;
 
+/* Whether to report TARGET_WAITKIND_THREAD_EXITED events when a
+   stepping thread exits.  */
+static bool report_stepped_thread_exited;
+
 /* The event loop checks this to decide whether to continue accepting
    events.  */
 static bool keep_processing_events = true;
@@ -2398,6 +2402,13 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		     events.  */
 		  report_no_resumed = true;
 		}
+	      else if (feature == "stepped-thread-exited+")
+		{
+		  /* GDB understands & expects
+		     TARGET_WAITKIND_THREAD_EXITED for stepping
+		     threads that exit.  */
+		  report_stepped_thread_exited = true;
+		}
 	      else if (feature == "memory-tagging+")
 		{
 		  /* GDB supports memory tagging features.  */
@@ -2521,6 +2532,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 
       strcat (own_buf, ";no-resumed+");
 
+      if (target_supports_stepped_thread_exited ())
+	strcat (own_buf, ";stepped-thread-exited+");
+
       if (target_supports_memory_tagging ())
 	strcat (own_buf, ";memory-tagging+");
 
@@ -2954,9 +2968,21 @@ resume (struct thread_resume *actions, size_t num_actions)
 	  return;
 	}
 
+      if (cs.last_status.kind == TARGET_WAITKIND_THREAD_EXITED
+	  && !report_stepped_thread_exited)
+	{
+	  /* report_stepped_thread_exited also indicates whether the
+	     client supports the 'w' stop reply in all-stop mode.  At
+	     least return error.  */
+	  sprintf (cs.own_buf, "E.Thread exited.");
+	  disable_async_io ();
+	  return;
+	}
+
       if (cs.last_status.kind != TARGET_WAITKIND_EXITED
 	  && cs.last_status.kind != TARGET_WAITKIND_SIGNALLED
-	  && cs.last_status.kind != TARGET_WAITKIND_NO_RESUMED)
+	  && cs.last_status.kind != TARGET_WAITKIND_NO_RESUMED
+	  && cs.last_status.kind != TARGET_WAITKIND_THREAD_EXITED)
 	current_thread->last_status = cs.last_status;
 
       /* From the client's perspective, all-stop mode always stops all
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 6660cc04fd5..2d15a840711 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -470,6 +470,12 @@ process_stratum_target::supports_memory_tagging ()
   return false;
 }
 
+bool
+process_stratum_target::supports_stepped_thread_exited ()
+{
+  return false;
+}
+
 bool
 process_stratum_target::fetch_memtags (CORE_ADDR address, size_t len,
 				       gdb::byte_vector &tags, int type)
diff --git a/gdbserver/target.h b/gdbserver/target.h
index 2c4393ec8c6..862300f72f5 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -500,6 +500,10 @@ class process_stratum_target
   /* Returns true if the target supports memory tagging facilities.  */
   virtual bool supports_memory_tagging ();
 
+  /* Returns true if the target supports returning
+     TARGET_WAITKIND_THREAD_EXITED for stepping threads.  */
+  virtual bool supports_stepped_thread_exited();
+
   /* Return the allocated memory tags of type TYPE associated with
      [ADDRESS, ADDRESS + LEN) in TAGS.
 
@@ -542,6 +546,9 @@ int kill_inferior (process_info *proc);
 #define target_supports_memory_tagging() \
   the_target->supports_memory_tagging ()
 
+#define target_supports_stepped_thread_exited() \
+  the_target->supports_stepped_thread_exited ()
+
 #define target_handle_new_gdb_connection()		 \
   the_target->handle_new_gdb_connection ()
 
-- 
2.26.2



More information about the Gdb-patches mailing list