[PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol

Pedro Alves
2015-10-14T15:36:52Z

Testing with "maint set target-non-stop on" causes regressions in
tests that rely on TARGET_WAITKIND_NO_RESUMED, which isn't modelled on
the RSP. In real all-stop, gdbserver detects the situation and
reporst error to GDB, and so the tests (e.g.,
gdb.threads/no-unwaited-for-left.exp) at fail quickly. But with
"maint set target-non-stop on", GDB instead hangs forever waiting for
a stop reply that never comes, and so the tests take longer to time
out.

This adds a new "N" stop reply packet that maps 1-1 to
TARGET_WAITKIND_NO_RESUMED.

gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>

PR 14618
* NEWS (New remote packets): Mention the N stop reply.
* remote.c (remote_protocol_features): Add "no-resumed" entry.
(remote_query_supported): Report no-resumed+ support.
(remote_parse_stop_reply): Handle 'N'.
(process_stop_reply): Handle TARGET_WAITKIND_NO_RESUMED.
(remote_wait_as): Handle 'N' / TARGET_WAITKIND_NO_RESUMED.
(_initialize_remote): Register "set/show remote
no-resumed-stop-reply" commands.

gdb/doc/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>

PR 14618
* gdb.texinfo (Stop Reply Packets): Document the N stop reply.
(Remote Configuration): Add the "set/show remote
no-resumed-stop-reply" to the available settings table.
(General Query Packets): Document the "no-resumed" qSupported
feature.

gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>

PR 14618
* linux-low.c (linux_wait_1): If the last resumed thread is gone,
report TARGET_WAITKIND_NO_RESUMED.
* remote-utils.c (prepare_resume_reply): Handle
TARGET_WAITKIND_NO_RESUMED.
* server.c (report_no_resumed): New global.
(handle_query) <qSupported>: Handle "no-resumed+". Report
"no-resumed+" support.
(resume): When the target reports TARGET_WAITKIND_NO_RESUMED, only
return error if the client doesn't support no-resumed events.
(push_stop_notification): New function.
(handle_target_event): Use it. Report TARGET_WAITKIND_NO_RESUMED
events if the client supports them.
---
gdb/NEWS | 6 ++++++
gdb/doc/gdb.texinfo | 27 ++++++++++++++++++++++++++
gdb/gdbserver/linux-low.c | 15 ++++++++++++++-
gdb/gdbserver/remote-utils.c | 3 +++
gdb/gdbserver/server.c | 45 +++++++++++++++++++++++++++++++-------------
gdb/remote.c | 23 +++++++++++++++++++---
6 files changed, 102 insertions(+), 17 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index cb3b2fc..1337650 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -97,6 +97,12 @@ QThreadEvents
would hang forever not knowing that it should no longer expect a
stop for that same thread.

+N stop reply
+
+ Indicates that there are no resumed threads left in the target (all
+ threads are stopped). The remote stub reports support for this stop
+ reply to GDB's qSupported query.
+
* Extended-remote exec events

** GDB now has support for exec events on extended-remote Linux targets.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1da9e4a..1d7ee8b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20236,6 +20236,10 @@ are:
@tab @code{QThreadEvents}
@tab Tracking thread lifetime.

+@item @code{no-resumed-stop-reply}
+@tab @code{no resumed thread left stop reply}
+@tab Tracking thread lifetime.
+
@end multitable

@node Remote Stub
@@ -35585,6 +35589,21 @@ 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.

+@item N
+There are no resumed threads left in the target. In other words, even
+though the process is alive, the last resumed thread has exited. For
+example, say the target process has two threads: thread 1 and thread
+2. The client leaves thread 1 stopped, and resumes thread 2, which
+subsequently exits. At this point, even though the process is still
+alive, and thus no @samp{W} stop reply is sent, no thread is actually
+executing either. The @samp{N} stop reply thus informs the client
+that it can stop waiting for stop replies. This packet should not be
+sent by default; older @value{GDBN} versions did not support it.
+@value{GDBN} requests it, by supplying an appropriate
+@samp{qSupported} feature (@pxref{qSupported}). The remote stub must
+also supply the appropriate @samp{qSupported} feature indicating
+support.
+
@item O @var{XX}@dots{}
@samp{@var{XX}@dots{}} is hex encoding of @sc{ascii} data, to be
written as the program's console output. This can happen at any time
@@ -36484,6 +36503,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No

+@item @samp{no-resumed}
+@tab No
+@tab @samp{-}
+@tab No
+
@end multitable

These are the currently defined stub features, in more detail:
@@ -36699,6 +36723,9 @@ The remote stub reports the supported actions in the reply to
@item QThreadEvents
The remote stub understands the @samp{QThreadEvents} packet.

+@item no-resumed
+The remote stub reports the @samp{N} stop reply.
+
@end table

@item qSymbol::
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index af9ca22..73abe5c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2971,6 +2971,7 @@ linux_wait_1 (ptid_t ptid,
int report_to_gdb;
int trace_event;
int in_step_range;
+ int any_resumed;

if (debug_threads)
{
@@ -2988,6 +2989,18 @@ linux_wait_1 (ptid_t ptid,
in_step_range = 0;
ourstatus->kind = TARGET_WAITKIND_IGNORE;

+ /* Find a resumed LWP, if any. */
+ if (find_inferior (&all_threads,
+ status_pending_p_callback,
+ &minus_one_ptid) != NULL)
+ any_resumed = 1;
+ else if ((find_inferior (&all_threads,
+ not_stopped_callback,
+ &minus_one_ptid) != NULL))
+ any_resumed = 1;
+ else
+ any_resumed = 0;
+
if (ptid_equal (step_over_bkpt, null_ptid))
pid = linux_wait_for_event (ptid, &w, options);
else
@@ -2998,7 +3011,7 @@ linux_wait_1 (ptid_t ptid,
pid = linux_wait_for_event (step_over_bkpt, &w, options & ~WNOHANG);
}

- if (pid == 0)
+ if (pid == 0 || (pid == -1 && !any_resumed))
{
gdb_assert (target_options & TARGET_WNOHANG);

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 0d9cde3..77280f7 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1289,6 +1289,9 @@ prepare_resume_reply (char *buf, ptid_t ptid,
buf += strlen (buf);
buf = write_ptid (buf, ptid);
break;
+ case TARGET_WAITKIND_NO_RESUMED:
+ sprintf (buf, "N");
+ break;
default:
error ("unhandled waitkind");
break;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 2ffc5d0..d83d608 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -61,6 +61,10 @@ int report_fork_events;
int report_vfork_events;
int report_exec_events;
int report_thread_events;
+
+/* Whether to report TARGET_WAITKING_NO_RESUMED events. */
+static int report_no_resumed;
+
int non_stop;
int swbreak_feature;
int hwbreak_feature;
@@ -2182,6 +2186,12 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
}
else if (strcmp (p, "vContSupported+") == 0)
vCont_supported = 1;
+ else if (strcmp (p, "no-resumed+") == 0)
+ {
+ /* GDB supports and wants TARGET_WAITKIND_NO_RESUMED
+ events. */
+ report_no_resumed = 1;
+ }
else
target_process_qsupported (p);

@@ -2295,6 +2305,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)

strcat (own_buf, ";QThreadEvents+");

+ strcat (own_buf, ";no-resumed+");
+
/* Reinitialize components as needed for the new connection. */
hostio_handle_new_gdb_connection ();
target_handle_new_gdb_connection ();
@@ -2702,10 +2714,11 @@ resume (struct thread_resume *actions, size_t num_actions)
{
last_ptid = mywait (minus_one_ptid, &last_status, 0, 1);

- if (last_status.kind == TARGET_WAITKIND_NO_RESUMED)
+ if (last_status.kind == TARGET_WAITKIND_NO_RESUMED
+ && !report_no_resumed)
{
- /* No proper RSP support for this yet. At least return
- error. */
+ /* The client does not support this stop reply. At least
+ return error. */
sprintf (own_buf, "E.No unwaited-for children left.");
disable_async_io ();
return;
@@ -4318,6 +4331,19 @@ handle_serial_event (int err, gdb_client_data client_data)
return 0;
}

+/* Push a stop notification on the notification queue. */
+
+static void
+push_stop_notification (ptid_t ptid, struct target_waitstatus *status)
+{
+ struct vstop_notif *vstop_notif = XNEW (struct vstop_notif);
+
+ vstop_notif->status = *status;
+ vstop_notif->ptid = ptid;
+ /* Push Stop notification. */
+ notif_push (&notif_stop, (struct notif_event *) vstop_notif);
+}
+
/* Event-loop callback for target events. */

int
@@ -4331,7 +4357,8 @@ handle_target_event (int err, gdb_client_data client_data)

if (last_status.kind == TARGET_WAITKIND_NO_RESUMED)
{
- /* No RSP support for this yet. */
+ if (gdb_connected () && report_no_resumed)
+ push_stop_notification (null_ptid, &last_status);
}
else if (last_status.kind != TARGET_WAITKIND_IGNORE)
{
@@ -4386,15 +4413,7 @@ handle_target_event (int err, gdb_client_data client_data)
target_pid_to_str (last_ptid));
}
else
- {
- struct vstop_notif *vstop_notif = XNEW (struct vstop_notif);
-
- vstop_notif->status = last_status;
- vstop_notif->ptid = last_ptid;
- /* Push Stop notification. */
- notif_push (&notif_stop,
- (struct notif_event *) vstop_notif);
- }
+ push_stop_notification (last_ptid, &last_status);
}

/* Be sure to not change the selected thread behind GDB's back.
diff --git a/gdb/remote.c b/gdb/remote.c
index f4714bc..c7a01d2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1486,6 +1486,9 @@ enum {
/* Support remote CTRL-C. */
PACKET_vCtrlC,

+ /* Support TARGET_WAITKIND_NO_RESUMED. */
+ PACKET_no_resumed,
+
PACKET_MAX
};

@@ -4501,6 +4504,7 @@ static const struct protocol_feature remote_protocol_features[] = {
PACKET_Qbtrace_conf_pt_size },
{ "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 },
};

static char *remote_support_xml;
@@ -4599,6 +4603,9 @@ remote_query_supported (void)
if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
q = remote_query_supported_append (q, "QThreadEvents+");

+ if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
+ q = remote_query_supported_append (q, "no-resumed+");
+
q = reconcat (q, "qSupported:", q, (char *) NULL);
putpkt (q);

@@ -6567,6 +6574,10 @@ Packet: '%s'\n"),
event->ptid = pid_to_ptid (pid);
}
break;
+ case 'N':
+ event->ws.kind = TARGET_WAITKIND_NO_RESUMED;
+ event->ptid = minus_one_ptid;
+ break;
}

if (target_is_non_stop_p () && ptid_equal (event->ptid, null_ptid))
@@ -6668,7 +6679,8 @@ process_stop_reply (struct stop_reply *stop_reply,
ptid = inferior_ptid;

if (status->kind != TARGET_WAITKIND_EXITED
- && status->kind != TARGET_WAITKIND_SIGNALLED)
+ && status->kind != TARGET_WAITKIND_SIGNALLED
+ && status->kind != TARGET_WAITKIND_NO_RESUMED)
{
struct remote_state *rs = get_remote_state ();
struct private_thread_info *remote_thr;
@@ -6847,7 +6859,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
remote_fileio_request (buf, rs->ctrlc_pending_p);
rs->ctrlc_pending_p = 0;
break;
- case 'T': case 'S': case 'X': case 'W':
+ case 'N': case 'T': case 'S': case 'X': case 'W':
{
struct stop_reply *stop_reply
= (struct stop_reply *) remote_notif_parse (&notif_client_stop,
@@ -6891,7 +6903,9 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
break;
}

- if (status->kind == TARGET_WAITKIND_IGNORE)
+ if (status->kind == TARGET_WAITKIND_NO_RESUMED)
+ return minus_one_ptid;
+ else if (status->kind == TARGET_WAITKIND_IGNORE)
{
/* Nothing interesting happened. If we're doing a non-blocking
poll, we're done. Otherwise, go back to waiting. */
@@ -13723,6 +13737,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QThreadEvents],
"QThreadEvents", "thread-events", 0);

+ add_packet_config_cmd (&remote_protocol_packets[PACKET_no_resumed],
+ "N stop reply", "no-resumed-stop-reply", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
--
1.9.3