From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 9BF603886C58 for ; Mon, 7 Jun 2021 17:15:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9BF603886C58 X-ASG-Debug-ID: 1623086107-0c856e67e214040a0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id fc3gBLdoOFhOypd3 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 07 Jun 2021 13:15:07 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id B1342441D95; Mon, 7 Jun 2021 13:15:07 -0400 (EDT) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Subject: [PATCH] gdb: enable target_async around stop_all_threads call in process_initial_stop_replies Date: Mon, 7 Jun 2021 13:15:05 -0400 X-ASG-Orig-Subj: [PATCH] gdb: enable target_async around stop_all_threads call in process_initial_stop_replies Message-Id: <20210607171505.874208-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1623086107 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-BRTS-Status: 1 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 8165 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.90490 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-17.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Jun 2021 17:15:10 -0000 The following scenario hangs: - maint set target-non-stop on - `gdbserver --attach` - a multi-threaded program For example: Terminal 1: $ gnome-calculator& [1] 495731 $ ../gdbserver/gdbserver --once --attach :1234 495731 Attached; pid = 495731 Listening on port 1234 Terminal 2: $ ./gdb -nx -q --data-directory=data-directory /usr/bin/gnome-calculator -ex "maint set target-non-stop on" -ex "tar rem :1234" Reading symbols from /usr/bin/gnome-calculator... (No debugging symbols found in /usr/bin/gnome-calculator) Remote debugging using :1234 * hangs * What happens is: - The protocol between gdb and gdbserver is in non-stop mode, but the user-visible behavior is all-stop - On connect, gdbserver sends one stop reply for one thread that is stops, the others stay running - In process_initial_stop_replies, gdb calls stop_all_threads to stop these other threads, because we are using the all-stop user-visible mode - stop_all_threads sends a stop request for all the running threads and then waits for resulting events - At this point, the remote target is in target_async(0) mode, which makes stop_all_threads not consider it for events - stop_all_threads loops indefinitely (it does not event block indefinitely, it is in an infinite loop) because there are no event sources. wait_one_event returns a TARGET_WAITKIND_NO_RESUMED wait status. Fix that by making the remote target async around the stop_all_threads call. I haven't implemented it because I'm not sure how to do it, but I think it would be a good idea to have, in stop_all_threads / wait_one / handle_one, an assert to check that if we are expecting one or more event, then there are some targets that are in a state where they can supply some events. Otherwise, we'll necessarily be stuck in this infinite loop, and it's probably due to a bug in GDB. I'm not too sure where to put this or how to express it though. Perhaps in stop_all_threads, here: for (int i = 0; i < waits_needed; i++) { wait_one_event event = wait_one (); *here* if (handle_one (event)) break; } If at that point, the returned event is TARGET_WAITKIND_NO_RESUMED, there's a problem. We expect some event, because we've asked some threads to stop, but all targets are answering that they won't have any events for us. That's a contradiction, and a sign that something has gone wrong. It could perhaps event be: gdb_assert (event.ws.kind != TARGET_WAITKIND_NO_RESUMED); in handle_one, as the idea is the same in prepare_for_detach. A bit more sophisticated would be: we know which targets we are expecting waits from, since we know which threads we have asked to stop. So if any of these targets returns TARGET_WAITKIND_NO_RESUMED, something is fishy. Add a test that tests attaching with gdbserver's --attach flag to a multi-threaded program, and then connecting to it. Without the fix, the test reproduces the hang. gdb/ChangeLog: * remote.c (remote_target::process_initial_stop_replies): Enable target_async around stop_all_threads call. gdb/testsuite/ChangeLog: * gdb.server/attach-flag.c: New test. * gdb.server/attach-flag.exp: New test. Change-Id: If6f6690a4887ca66693ef1af64791dda4c65f24f --- gdb/remote.c | 10 ++- gdb/testsuite/gdb.server/attach-flag.c | 29 +++++++++ gdb/testsuite/gdb.server/attach-flag.exp | 79 ++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.server/attach-flag.c create mode 100644 gdb/testsuite/gdb.server/attach-flag.exp diff --git a/gdb/remote.c b/gdb/remote.c index de04aab43dc9..cd4b0e1c5a5b 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -4591,7 +4591,15 @@ remote_target::process_initial_stop_replies (int from_tty) the inferiors. */ if (!non_stop) { - stop_all_threads (); + { + /* At this point, the remote target is not async. It needs to be for + the poll in stop_all_threads to consider events from it, so enable + it temporarily. */ + gdb_assert (!this->is_async_p ()); + SCOPE_EXIT { target_async (0); }; + target_async (1); + stop_all_threads (); + } /* If all threads of an inferior were already stopped, we haven't setup the inferior yet. */ diff --git a/gdb/testsuite/gdb.server/attach-flag.c b/gdb/testsuite/gdb.server/attach-flag.c new file mode 100644 index 000000000000..f6ed6180eef9 --- /dev/null +++ b/gdb/testsuite/gdb.server/attach-flag.c @@ -0,0 +1,29 @@ +#include +#include + +static const int NTHREADS = 10; +static pthread_barrier_t barrier; + +static void * +thread_func (void *p) +{ + pthread_barrier_wait (&barrier); + return NULL; +} + +int +main (void) +{ + alarm (60); + + pthread_t threads[NTHREADS]; + pthread_barrier_init (&barrier, NULL, NTHREADS + 2); + + for (int i = 0; i < NTHREADS; i++) + pthread_create (&threads[i], NULL, thread_func, NULL); + + pthread_barrier_wait (&barrier); + + for (int i = 0; i < NTHREADS; i++) + pthread_join (threads[i], NULL); +} diff --git a/gdb/testsuite/gdb.server/attach-flag.exp b/gdb/testsuite/gdb.server/attach-flag.exp new file mode 100644 index 000000000000..dc949386e0e2 --- /dev/null +++ b/gdb/testsuite/gdb.server/attach-flag.exp @@ -0,0 +1,79 @@ +# 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 . + +# Test attaching to a multi-threaded process using gdbserver's --attach flag. + +load_lib gdbserver-support.exp + +standard_testfile + +if { [skip_gdbserver_tests] } { + return 0 +} + +if {![can_spawn_for_attach]} { + return 0 +} + +# Start the test program, attach to it using gdbserver's --attach flag, connect +# to it with GDB, check that what we see makes sense. + +proc run_one_test { non-stop target-non-stop } { + save_vars { ::GDBFLAGS } { + # If GDB and GDBserver are both running locally, set the sysroot to avoid + # reading files via the remote protocol. + if { ![is_remote host] && ![is_remote target] } { + set ::GDBFLAGS "$::GDBFLAGS -ex \"set sysroot\"" + } + + if { [prepare_for_testing "failed to prepare" $::testfile $::srcfile \ + {debug pthreads}] } { + return -1 + } + } + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" + + set target_exec [gdbserver_download_current_prog] + set test_spawn_id [spawn_wait_for_attach $::binfile] + set testpid [spawn_id_get_pid $test_spawn_id] + + lassign [gdbserver_start "" "--attach $testpid"] unused gdbserver_address + + gdb_test_no_output "set non-stop ${non-stop}" + gdb_test_no_output "maint set target-non-stop ${target-non-stop}" + gdb_target_cmd "remote" $gdbserver_address + + # There should be 11 threads. + gdb_test "thread 11" "Switching to thread 11.*" + + kill_wait_spawned_process $test_spawn_id + gdbserver_exit 0 +} + +foreach_with_prefix non-stop {0 1} { + foreach_with_prefix target-non-stop {0 1} { + # This combination does not make sense. + if { ${non-stop} == 1 && ${target-non-stop} == 0} { + continue + } + + run_one_test ${non-stop} ${target-non-stop} + } +} -- 2.31.1