From f2faf941ae49653ff6e1485adfee299313d47c91 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 30 Nov 2015 16:05:25 +0000 Subject: [PATCH] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol 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-11-30 Pedro Alves 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-11-30 Pedro Alves 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-11-30 Pedro Alves 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) : 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/testsuite/ChangeLog: 2015-11-30 Pedro Alves * gdb.threads/no-unwaited-for-left.exp: Remove setup_kfail calls. --- gdb/ChangeLog | 12 +++++ gdb/NEWS | 6 +++ gdb/doc/ChangeLog | 9 ++++ gdb/doc/gdb.texinfo | 27 +++++++++++ gdb/gdbserver/ChangeLog | 16 +++++++ gdb/gdbserver/linux-low.c | 15 ++++++- gdb/gdbserver/remote-utils.c | 3 ++ gdb/gdbserver/server.c | 45 +++++++++++++------ gdb/remote.c | 23 ++++++++-- gdb/testsuite/ChangeLog | 4 ++ .../gdb.threads/no-unwaited-for-left.exp | 6 --- 11 files changed, 143 insertions(+), 23 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0f188e6c48a..0d2d207b443 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2015-11-30 Pedro Alves + + 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. + 2015-11-30 Pedro Alves PR 14618 diff --git a/gdb/NEWS b/gdb/NEWS index a236161c7b3..a222dfb491f 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -106,6 +106,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/ChangeLog b/gdb/doc/ChangeLog index b5eddc9f51e..2c58ffe8f4d 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,12 @@ +2015-11-30 Pedro Alves + + 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. + 2015-11-30 Pedro Alves * gdb.texinfo (Remote Configuration): List "set/show remote diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index d2ec3549520..b82f3c67a8a 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20239,6 +20239,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 @@ -35588,6 +35592,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 @@ -36487,6 +36506,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: @@ -36702,6 +36726,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/ChangeLog b/gdb/gdbserver/ChangeLog index bc09844a666..dbe7dc42871 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,19 @@ +2015-11-30 Pedro Alves + + 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) : 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. + 2015-11-30 Pedro Alves * linux-low.c (thread_still_has_status_pending_p): Don't check diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 07cb83120d3..28abe418da4 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -2963,6 +2963,7 @@ linux_wait_1 (ptid_t ptid, int report_to_gdb; int trace_event; int in_step_range; + int any_resumed; if (debug_threads) { @@ -2980,6 +2981,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 @@ -2990,7 +3003,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 0d9cde38921..77280f7c11e 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 0762f6aba44..18e9bf12850 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; @@ -2187,6 +2191,12 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) vCont_supported = 1; else if (strcmp (p, "QThreadEvents+") == 0) ; + else if (strcmp (p, "no-resumed+") == 0) + { + /* GDB supports and wants TARGET_WAITKIND_NO_RESUMED + events. */ + report_no_resumed = 1; + } else { /* Move the unknown features all together. */ @@ -2309,6 +2319,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 (); @@ -2716,10 +2728,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; @@ -4332,6 +4345,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 (¬if_stop, (struct notif_event *) vstop_notif); +} + /* Event-loop callback for target events. */ int @@ -4345,7 +4371,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) { @@ -4405,15 +4432,7 @@ handle_target_event (int err, gdb_client_data client_data) } } 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 (¬if_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 4daaf08c4b4..9d44ce1d7be 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1488,6 +1488,9 @@ enum { /* Support remote CTRL-C. */ PACKET_vCtrlC, + /* Support TARGET_WAITKIND_NO_RESUMED. */ + PACKET_no_resumed, + PACKET_MAX }; @@ -4527,6 +4530,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; @@ -4622,6 +4626,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+"); + /* Keep this one last to work around a gdbserver <= 7.10 bug in the qSupported:xmlRegisters=i386 handling. */ if (remote_support_xml != NULL) @@ -6596,6 +6603,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)) @@ -6697,7 +6708,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; @@ -6875,7 +6887,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; @@ -6914,7 +6926,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. */ @@ -13755,6 +13769,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. */ { diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 08b5b6fbc75..197b2268887 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2015-11-30 Pedro Alves + + * gdb.threads/no-unwaited-for-left.exp: Remove setup_kfail calls. + 2015-11-30 Pedro Alves * lib/range-stepping-support.exp (exec_cmd_expect_vCont_count): diff --git a/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp b/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp index 3a43d58b45e..518301e7bcc 100644 --- a/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp +++ b/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp @@ -37,9 +37,6 @@ gdb_test_no_output "set scheduler-locking on" \ "enable scheduler-locking, for thread 2" # Continue. Thread 2 exits, and the main thread was already stopped. -if [is_remote target] { - setup_kfail "gdb/14618" "*-*-*" -} gdb_test "continue" \ "No unwaited-for children left." \ "continue stops when thread 2 exits" @@ -61,9 +58,6 @@ gdb_continue_to_breakpoint "break-here-2" ".* break-here-2 .*" gdb_test_no_output "set scheduler-locking on" \ "enable scheduler-locking, for main thread" # The main thread exits, and thread 3 was already stopped. -if [is_remote target] { - setup_kfail "gdb/14618" "*-*-*" -} gdb_test "continue" \ "No unwaited-for children left." \ "continue stops when the main thread exits" -- 2.30.2