diff options
| -rw-r--r-- | gdb/ChangeLog | 9 | ||||
| -rw-r--r-- | gdb/remote.c | 163 | ||||
| -rw-r--r-- | gdb/testsuite/ChangeLog | 6 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c | 77 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp | 136 |
5 files changed, 336 insertions, 55 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bd039a7..7d20a69 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2021-01-13 Andrew Burgess <andrew.burgess@embecosm.com> + + PR gdb/26819 + * remote.c + (remote_target::select_thread_for_ambiguous_stop_reply): New + member function. + (remote_target::process_stop_reply): Call + select_thread_for_ambiguous_stop_reply. + 2021-01-13 Simon Marchi <simon.marchi@efficios.com> * record-btrace.c (class record_btrace_target): Remove. diff --git a/gdb/remote.c b/gdb/remote.c index a657902..74ebbf9 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -747,6 +747,9 @@ public: /* Remote specific methods. */ ptid_t process_stop_reply (struct stop_reply *stop_reply, target_waitstatus *status); + ptid_t select_thread_for_ambiguous_stop_reply + (const struct target_waitstatus *status); + void remote_notice_new_inferior (ptid_t currthread, int executing); void process_initial_stop_replies (int from_tty); @@ -7753,75 +7756,125 @@ remote_notif_get_pending_events (remote_target *remote, notif_client *nc) remote->remote_notif_get_pending_events (nc); } -/* Called when it is decided that STOP_REPLY holds the info of the - event that is to be returned to the core. This function always - destroys STOP_REPLY. */ +/* Called from process_stop_reply when the stop packet we are responding + to didn't include a process-id or thread-id. STATUS is the stop event + we are responding to. + + It is the task of this function to select a suitable thread (or process) + and return its ptid, this is the thread (or process) we will assume the + stop event came from. + + In some cases there isn't really any choice about which thread (or + process) is selected, a basic remote with a single process containing a + single thread might choose not to send any process-id or thread-id in + its stop packets, this function will select and return the one and only + thread. + + However, if a target supports multiple threads (or processes) and still + doesn't include a thread-id (or process-id) in its stop packet then + first, this is a badly behaving target, and second, we're going to have + to select a thread (or process) at random and use that. This function + will print a warning to the user if it detects that there is the + possibility that GDB is guessing which thread (or process) to + report. + + Note that this is called before GDB fetches the updated thread list from the + target. So it's possible for the stop reply to be ambiguous and for GDB to + not realize it. For example, if there's initially one thread, the target + spawns a second thread, and then sends a stop reply without an id that + concerns the first thread. GDB will assume the stop reply is about the + first thread - the only thread it knows about - without printing a warning. + Anyway, if the remote meant for the stop reply to be about the second thread, + then it would be really broken, because GDB doesn't know about that thread + yet. */ ptid_t -remote_target::process_stop_reply (struct stop_reply *stop_reply, - struct target_waitstatus *status) +remote_target::select_thread_for_ambiguous_stop_reply + (const struct target_waitstatus *status) { - ptid_t ptid; + /* Some stop events apply to all threads in an inferior, while others + only apply to a single thread. */ + bool process_wide_stop + = (status->kind == TARGET_WAITKIND_EXITED + || status->kind == TARGET_WAITKIND_SIGNALLED); - *status = stop_reply->ws; - ptid = stop_reply->ptid; + thread_info *first_resumed_thread = nullptr; + bool ambiguous = false; - /* If no thread/process was reported by the stub then use the first - non-exited thread in the current target. */ - if (ptid == null_ptid) + /* Consider all non-exited threads of the target, find the first resumed + one. */ + for (thread_info *thr : all_non_exited_threads (this)) { - /* Some stop events apply to all threads in an inferior, while others - only apply to a single thread. */ - bool is_stop_for_all_threads - = (status->kind == TARGET_WAITKIND_EXITED - || status->kind == TARGET_WAITKIND_SIGNALLED); + remote_thread_info *remote_thr = get_remote_thread_info (thr); - for (thread_info *thr : all_non_exited_threads (this)) + if (remote_thr->resume_state () != resume_state::RESUMED) + continue; + + if (first_resumed_thread == nullptr) + first_resumed_thread = thr; + else if (!process_wide_stop + || first_resumed_thread->ptid.pid () != thr->ptid.pid ()) + ambiguous = true; + } + + gdb_assert (first_resumed_thread != nullptr); + + /* Warn if the remote target is sending ambiguous stop replies. */ + if (ambiguous) + { + static bool warned = false; + + if (!warned) { - if (ptid != null_ptid - && (!is_stop_for_all_threads - || ptid.pid () != thr->ptid.pid ())) - { - static bool warned = false; + /* If you are seeing this warning then the remote target has + stopped without specifying a thread-id, but the target + does have multiple threads (or inferiors), and so GDB is + having to guess which thread stopped. - if (!warned) - { - /* If you are seeing this warning then the remote target - has stopped without specifying a thread-id, but the - target does have multiple threads (or inferiors), and - so GDB is having to guess which thread stopped. - - Examples of what might cause this are the target - sending and 'S' stop packet, or a 'T' stop packet and - not including a thread-id. - - Additionally, the target might send a 'W' or 'X - packet without including a process-id, when the target - has multiple running inferiors. */ - if (is_stop_for_all_threads) - warning (_("multi-inferior target stopped without " - "sending a process-id, using first " - "non-exited inferior")); - else - warning (_("multi-threaded target stopped without " - "sending a thread-id, using first " - "non-exited thread")); - warned = true; - } - break; - } + Examples of what might cause this are the target sending + and 'S' stop packet, or a 'T' stop packet and not + including a thread-id. - /* If this is a stop for all threads then don't use a particular - threads ptid, instead create a new ptid where only the pid - field is set. */ - if (is_stop_for_all_threads) - ptid = ptid_t (thr->ptid.pid ()); + Additionally, the target might send a 'W' or 'X packet + without including a process-id, when the target has + multiple running inferiors. */ + if (process_wide_stop) + warning (_("multi-inferior target stopped without " + "sending a process-id, using first " + "non-exited inferior")); else - ptid = thr->ptid; + warning (_("multi-threaded target stopped without " + "sending a thread-id, using first " + "non-exited thread")); + warned = true; } - gdb_assert (ptid != null_ptid); } + /* If this is a stop for all threads then don't use a particular threads + ptid, instead create a new ptid where only the pid field is set. */ + if (process_wide_stop) + return ptid_t (first_resumed_thread->ptid.pid ()); + else + return first_resumed_thread->ptid; +} + +/* Called when it is decided that STOP_REPLY holds the info of the + event that is to be returned to the core. This function always + destroys STOP_REPLY. */ + +ptid_t +remote_target::process_stop_reply (struct stop_reply *stop_reply, + struct target_waitstatus *status) +{ + *status = stop_reply->ws; + ptid_t ptid = stop_reply->ptid; + + /* If no thread/process was reported by the stub then select a suitable + thread/process. */ + if (ptid == null_ptid) + ptid = select_thread_for_ambiguous_stop_reply (status); + gdb_assert (ptid != null_ptid); + if (status->kind != TARGET_WAITKIND_EXITED && status->kind != TARGET_WAITKIND_SIGNALLED && status->kind != TARGET_WAITKIND_NO_RESUMED) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 8e54f8c..cfc49db 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2021-01-13 Andrew Burgess <andrew.burgess@embecosm.com> + + PR gdb/26819 + * gdb.server/stop-reply-no-thread-multi.c: New file. + * gdb.server/stop-reply-no-thread-multi.exp: New file. + 2021-01-12 Tom de Vries <tdevries@suse.de> * gdb.arch/i386-mpx-call.c (have_mpx): Remove. diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c new file mode 100644 index 0000000..40cc71a --- /dev/null +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c @@ -0,0 +1,77 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2021 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <stdlib.h> +#include <pthread.h> +#include <unistd.h> + +volatile int worker_blocked = 1; +volatile int main_blocked = 1; + +void +unlock_worker (void) +{ + worker_blocked = 0; +} + +void +unlock_main (void) +{ + main_blocked = 0; +} + +void +breakpt (void) +{ + /* Nothing. */ +} + +static void * +worker (void *data) +{ + unlock_main (); + + while (worker_blocked) + ; + + breakpt (); + + return NULL; +} + +int +main (void) +{ + pthread_t thr; + void *retval; + + /* Ensure the test doesn't run forever. */ + alarm (99); + + if (pthread_create (&thr, NULL, worker, NULL) != 0) + abort (); + + while (main_blocked) + ; + + unlock_worker (); + + if (pthread_join (thr, &retval) != 0) + abort (); + + return 0; +} diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp new file mode 100644 index 0000000..6350f57 --- /dev/null +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp @@ -0,0 +1,136 @@ +# This testcase is part of GDB, the GNU debugger. +# +# Copyright 2021 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Test how GDB handles the case where a target either doesn't use 'T' +# packets at all or doesn't include a thread-id in a 'T' packet, AND, +# where the test program contains multiple threads. +# +# In general if multiple threads are executing and the target doesn't +# include a thread-id in its stop response then GDB will not be able +# to correctly figure out which thread the stop applies to. +# +# However, this test covers a very specific case, there are multiple +# threads but only a single thread is actually executing. So, when +# the stop comes from the target, without a thread-id, GDB should be +# able to correctly figure out which thread has stopped. + +load_lib gdbserver-support.exp + +if { [skip_gdbserver_tests] } { + verbose "skipping gdbserver tests" + return -1 +} + +standard_testfile +if { [build_executable "failed to prepare" $testfile $srcfile {debug pthreads}] == -1 } { + return -1 +} + +# Run the tests with different features of GDBserver disabled. +proc run_test { disable_feature } { + global binfile gdb_prompt decimal hex + + clean_restart ${binfile} + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" + + set packet_arg "" + if { $disable_feature != "" } { + set packet_arg "--disable-packet=${disable_feature}" + } + set res [gdbserver_start $packet_arg $binfile] + set gdbserver_protocol [lindex $res 0] + set gdbserver_gdbport [lindex $res 1] + + # Disable XML-based thread listing, and multi-process extensions. + gdb_test_no_output "set remote threads-packet off" + gdb_test_no_output "set remote multiprocess-feature-packet off" + + set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] + if ![gdb_assert {$res == 0} "connect"] { + return + } + + # There should be only one thread listed at this point. + gdb_test_multiple "info threads" "" { + -re "2 Thread.*$gdb_prompt $" { + fail $gdb_test_name + } + -re "has terminated.*$gdb_prompt $" { + fail $gdb_test_name + } + -re "\\\* 1\[\t \]*Thread\[^\r\n\]*\r\n$gdb_prompt $" { + pass $gdb_test_name + } + } + + gdb_breakpoint "unlock_worker" + gdb_continue_to_breakpoint "run to unlock_worker" + + # There should be two threads at this point with thread 1 selected. + gdb_test "info threads" \ + "\\\* 1\[\t \]*Thread\[^\r\n\]*\r\n 2\[\t \]*Thread\[^\r\n\]*" \ + "second thread should now exist" + + # Switch threads. + gdb_test "thread 2" ".*" "switch to second thread" + + # Now turn on scheduler-locking so that when we step thread 2 only + # that one thread will be set running. + gdb_test_no_output "set scheduler-locking on" + + # Single step thread 2. Only the one thread will step. When the + # thread stops, if the stop packet doesn't include a thread-id + # then GDB should still understand which thread stopped. + gdb_test_multiple "stepi" "" { + -re -wrap "Thread 1 received signal SIGTRAP.*" { + fail $gdb_test_name + } + -re -wrap "$hex.*$decimal.*while \\(worker_blocked\\).*" { + pass $gdb_test_name + } + } + + # Check that thread 2 is still selected. + gdb_test "info threads" \ + " 1\[\t \]*Thread\[^\r\n\]*\r\n\\\* 2\[\t \]*Thread\[^\r\n\]*" \ + "second thread should still be selected after stepi" + + # Turn scheduler locking off again so that when we continue all + # threads will be set running. + gdb_test_no_output "set scheduler-locking off" + + # Continue until exit. The server sends a 'W' with no PID. + # Bad GDB gave an error like below when target is nonstop: + # (gdb) c + # Continuing. + # No process or thread specified in stop reply: W00 + gdb_continue_to_end "" continue 1 +} + +# Disable different features within gdbserver: +# +# Tthread: Start GDBserver, with ";thread:NNN" in T stop replies disabled, +# emulating old gdbservers when debugging single-threaded programs. +# +# T: Start GDBserver with the entire 'T' stop reply packet disabled, +# GDBserver will instead send the 'S' stop reply. +foreach_with_prefix to_disable { "" Tthread T } { + run_test $to_disable +} |