[PATCH v7 5/5] gdb/infrun: handle already-exited threads when attempting to stop

Tankut Baris Aktemur
2020-04-22T15:01:24Z

In stop_all_threads, GDB sends signals to other threads in an attempt
to stop them. While in a typical scenario the expected wait status is
TARGET_WAITKIND_STOPPED, it is possible that the thread GDB attempted
to stop has already terminated. If so, a waitstatus other than
TARGET_WAITKIND_STOPPED would be received. Handle this case
appropriately.

If a wait status that denotes thread termination is ignored, GDB goes
into an infinite loop in stop_all_threads.
E.g.:

$ gdb ./a.out
(gdb) start
...
(gdb) add-inferior -exec ./a.out
...
(gdb) inferior 2
...
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) set debug infrun 2
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 10449)
infrun: clear_proceed_status_thread (process 10453)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 10449
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 10449] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 10453
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 10453] at 0x55555555514e
infrun: prepare_to_wait
infrun: Found 2 inferiors, starting at #0
infrun: target_wait (-1.0.0, status) =
infrun: 10449.10449.0 [process 10449],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 1 (process 10449) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 10453 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 10453.10453.0 [process 10453],
infrun: status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 10453
infrun: process 10453 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: infrun_async(0)
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 10453 executing, already stopping
...

And this polling goes on forever. This patch prevents the infinite
looping behavior. For the same scenario above, we obtain the
following behavior:

...
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 31229)
infrun: clear_proceed_status_thread (process 31233)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 31229
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 31229] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 31233
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 31233] at 0x55555555514e
infrun: prepare_to_wait
infrun: Found 2 inferiors, starting at #0
infrun: target_wait (-1.0.0, status) =
infrun: 31229.31229.0 [process 31229],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 1 (process 31229) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 31233 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 31233.31233.0 [process 31233],
infrun: status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 31233
infrun: saving status status->kind = exited, status = 0 for 31233.31233.0
infrun: process 31233 not executing
infrun: stop_all_threads, pass=1, iterations=1
infrun: process 31233 not executing
infrun: stop_all_threads done
(gdb)

The exit event from Inferior 1 is received and shown to the user.
The exit event from Inferior 2 is not displayed, but kept pending.

(gdb) info inferiors
Num Description Connection Executable
* 1 <null> a.out
2 process 31233 1 (native) a.out
(gdb) inferior 2
[Switching to inferior 2 [process 31233] (a.out)]
[Switching to thread 2.1 (process 31233)]
Couldn't get registers: No such process.
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 31233)
infrun: clear_proceed_status_thread: thread process 31233 has pending wait status status->kind = exited, status = 0 (currently_stepping=0).
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 31233
infrun: resume: thread process 31233 has pending wait status status->kind = exited, status = 0 (currently_stepping=0).
infrun: prepare_to_wait
infrun: Using pending wait status status->kind = exited, status = 0 for process 31233.
infrun: target_wait (-1.0.0, status) =
infrun: 31233.31233.0 [process 31233],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 31233) exited normally]
infrun: stop_waiting
(gdb) info inferiors
Num Description Connection Executable
1 <null> a.out
* 2 <null> a.out
(gdb)

Regression-tested on X86_64 Linux.

gdb/ChangeLog:
2020-02-05 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Tom de Vries <tdevries@suse.de>

PR threads/25478
* infrun.c (stop_all_threads): Do NOT ignore
TARGET_WAITKIND_NO_RESUMED, TARGET_WAITKIND_THREAD_EXITED,
TARGET_WAITKIND_EXITED, TARGET_WAITKIND_SIGNALLED wait statuses
received from threads we attempt to stop.

gdb/testsuite/ChangeLog:
2019-11-04 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>

* gdb.multi/multi-exit.c: New file.
* gdb.multi/multi-exit.exp: New file.
* gdb.multi/multi-kill.c: New file.
* gdb.multi/multi-kill.exp: New file.

Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
gdb/infrun.c | 101 ++++++++++++++++++++--
gdb/testsuite/gdb.multi/multi-exit.c | 22 +++++
gdb/testsuite/gdb.multi/multi-exit.exp | 111 +++++++++++++++++++++++++
gdb/testsuite/gdb.multi/multi-kill.c | 42 ++++++++++
gdb/testsuite/gdb.multi/multi-kill.exp | 110 ++++++++++++++++++++++++
5 files changed, 379 insertions(+), 7 deletions(-)
create mode 100644 gdb/testsuite/gdb.multi/multi-exit.c
create mode 100644 gdb/testsuite/gdb.multi/multi-exit.exp
create mode 100644 gdb/testsuite/gdb.multi/multi-kill.c
create mode 100644 gdb/testsuite/gdb.multi/multi-kill.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 167d50ff3ab..93169269553 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4781,7 +4781,47 @@ stop_all_threads (void)
{
int need_wait = 0;

- update_thread_list ();
+ for (inferior *inf : all_non_exited_inferiors ())
+ {
+ update_thread_list ();
+
+ /* After updating the thread list, it's possible to end
+ up with pid != 0 but no threads, if the inf's process
+ has exited but we have not processed that event yet.
+ The exit event must be waiting somewhere in the queue
+ to be processed. Silently add a thread so that we do
+ a wait_one() below to pick the pending event. */
+
+ bool has_threads = false;
+ for (thread_info *tp ATTRIBUTE_UNUSED
+ : inf->non_exited_threads ())
+ {
+ has_threads = true;
+ break;
+ }
+
+ if (has_threads)
+ continue;
+
+ ptid_t ptid (inf->pid, inf->pid, 0);
+
+ /* Re-surrect the thread, if not physically deleted.
+ Add a new one otherwise. */
+ thread_info *t = find_thread_ptid (inf->process_target (), ptid);
+ gdb_assert (t == nullptr || t->state == THREAD_EXITED);
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads,"
+ " inf (pid %d): %s a thread", inf->pid,
+ (t == nullptr)
+ ? "adding" : "resurrecting");
+
+ if (t == nullptr)
+ t = add_thread_silent (inf->process_target (), ptid);
+
+ set_executing (inf->process_target (), ptid, true);
+ t->state = THREAD_STOPPED;
+ }

/* Go through all threads looking for threads that we need
to tell the target to stop. */
@@ -4856,13 +4896,60 @@ stop_all_threads (void)
target_pid_to_str (event.ptid).c_str ());
}

- if (event.ws.kind == TARGET_WAITKIND_NO_RESUMED
- || event.ws.kind == TARGET_WAITKIND_THREAD_EXITED
- || event.ws.kind == TARGET_WAITKIND_EXITED
- || event.ws.kind == TARGET_WAITKIND_SIGNALLED)
+ if (event.ws.kind == TARGET_WAITKIND_NO_RESUMED)
{
- /* All resumed threads exited
- or one thread/process exited/signalled. */
+ /* All resumed threads exited. */
+ }
+ else if (event.ws.kind == TARGET_WAITKIND_THREAD_EXITED
+ || event.ws.kind == TARGET_WAITKIND_EXITED
+ || event.ws.kind == TARGET_WAITKIND_SIGNALLED)
+ {
+ /* One thread/process exited/signalled. */
+
+ thread_info *t = nullptr;
+
+ /* The target may have reported just a pid. If so, try
+ the first non-exited thread. */
+ if (event.ptid.is_pid ())
+ {
+ int pid = event.ptid.pid ();
+ inferior *inf = find_inferior_pid (event.target, pid);
+ for (thread_info *tp : inf->non_exited_threads ())
+ {
+ t = tp;
+ break;
+ }
+
+ /* FIXME: If there is no available thread, the event
+ would have to be appended to a per-inferior event
+ list, which, unfortunately, does not exist yet. We
+ assert here instead of going into an infinite loop. */
+ gdb_assert (t != nullptr);
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: stop_all_threads, using %s\n",
+ target_pid_to_str (t->ptid).c_str ());
+ }
+ else
+ {
+ t = find_thread_ptid (event.target, event.ptid);
+ /* Check if this is the first time we see this thread.
+ Don't bother adding if it individually exited. */
+ if (t == nullptr
+ && event.ws.kind != TARGET_WAITKIND_THREAD_EXITED)
+ t = add_thread (event.target, event.ptid);
+ }
+
+ if (t != nullptr)
+ {
+ /* Set the threads as non-executing to avoid
+ another stop attempt on them. */
+ mark_non_executing_threads (event.target, event.ptid,
+ event.ws);
+ save_waitstatus (t, &event.ws);
+ t->stop_requested = false;
+ }
}
else
{
diff --git a/gdb/testsuite/gdb.multi/multi-exit.c b/gdb/testsuite/gdb.multi/multi-exit.c
new file mode 100644
index 00000000000..f4825c8a7c1
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-exit.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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/>. */
+
+int
+main ()
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/multi-exit.exp b/gdb/testsuite/gdb.multi/multi-exit.exp
new file mode 100644
index 00000000000..e8f188ca58a
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-exit.exp
@@ -0,0 +1,111 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2020 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 receiving TARGET_WAITKIND_EXITED events from multiple
+# inferiors. In all stop-mode, upon receiving the exit event from one
+# of the inferiors, GDB will try to stop the other inferior, too. So,
+# a stop request will be sent. Receiving a TARGET_WAITKIND_EXITED
+# status kind as a response to that stop request instead of a
+# TARGET_WAITKIND_STOPPED should be handled by GDB without problems.
+
+standard_testfile
+
+if {[use_gdb_stub]} {
+ return 0
+}
+
+if {[build_executable "failed to prepare" $testfile $srcfile]} {
+ return -1
+}
+
+# We are testing GDB's ability to stop all threads.
+# Hence, go with the all-stop-on-top-of-non-stop mode.
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"maint set target-non-stop on\""
+ clean_restart ${binfile}
+}
+
+with_test_prefix "inf 1" {
+ gdb_load $binfile
+
+ if {[gdb_start_cmd] < 0} {
+ fail "could not start"
+ return -1
+ }
+ gdb_test "" ".*reakpoint ., main .*${srcfile}.*" "start"
+}
+
+# Start another inferior.
+gdb_test "add-inferior" "Added inferior 2.*" \
+ "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" \
+ "switch to inferior 2"
+
+with_test_prefix "inf 2" {
+ gdb_load $binfile
+
+ if {[gdb_start_cmd] < 0} {
+ fail "could not start"
+ return -1
+ }
+ gdb_test "" ".*reakpoint ., main .*${srcfile}.*" "start"
+}
+
+# We want to continue both processes.
+gdb_test_no_output "set schedule-multiple on"
+
+set exited_inferior ""
+
+# We want GDB to complete the command and return the prompt
+# instead of going into an infinite loop.
+gdb_test_multiple "continue" "first continue" {
+ -re "Inferior ($decimal) \[^\n\r\]+ exited normally.*$gdb_prompt $" {
+ set exited_inferior $expect_out(1,string)
+ pass $gdb_test_name
+ }
+}
+
+if {$exited_inferior == ""} {
+ fail "first continue"
+ return -1
+}
+
+if {$exited_inferior == 1} {
+ set other_inferior 2
+} else {
+ set other_inferior 1
+}
+
+# Switch to the other inferior and check it, too, continues to the end.
+gdb_test "inferior $other_inferior" \
+ ".*Switching to inferior $other_inferior.*" \
+ "switch to the other inferior"
+
+gdb_continue_to_end
+
+# Finally, check if we can re-run both inferiors.
+delete_breakpoints
+
+gdb_test "run" "$inferior_exited_re normally\]" \
+ "re-run the other inferior"
+
+gdb_test "inferior $exited_inferior" \
+ ".*Switching to inferior $exited_inferior.*" \
+ "switch to the first exited inferior"
+
+gdb_test "run" "$inferior_exited_re normally\]" \
+ "re-run the first exited inferior"
diff --git a/gdb/testsuite/gdb.multi/multi-kill.c b/gdb/testsuite/gdb.multi/multi-kill.c
new file mode 100644
index 00000000000..66642bbb0e6
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-kill.c
@@ -0,0 +1,42 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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 <sys/types.h>
+#include <unistd.h>
+
+static pid_t pid;
+
+static void
+initialized ()
+{
+}
+
+int
+main ()
+{
+ pid = getpid ();
+ initialized ();
+
+ /* Don't run forever in case GDB crashes and DejaGNU fails to kill
+ this program. */
+ alarm (10);
+
+ while (1)
+ ;
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/multi-kill.exp b/gdb/testsuite/gdb.multi/multi-kill.exp
new file mode 100644
index 00000000000..706bbeb542c
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-kill.exp
@@ -0,0 +1,110 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2020 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 receiving TARGET_WAITKIND_SIGNALLED events from multiple
+# inferiors. In all stop-mode, upon receiving the exit event from one
+# of the inferiors, GDB will try to stop the other inferior, too. So,
+# a stop request will be sent. Receiving a TARGET_WAITKIND_SIGNALLED
+# status kind as a response to that stop request instead of a
+# TARGET_WAITKIND_STOPPED should be handled by GDB without problems.
+
+standard_testfile
+
+if {[use_gdb_stub]} {
+ return 0
+}
+
+if {[build_executable "failed to prepare" $testfile $srcfile {debug}]} {
+ return -1
+}
+
+# We are testing GDB's ability to stop all threads.
+# Hence, go with the all-stop-on-top-of-non-stop mode.
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"maint set target-non-stop on\""
+ clean_restart ${binfile}
+}
+
+# Initialize an inferior and return its pid.
+
+proc initialize_inferior {prefix} {
+ global binfile srcfile
+ with_test_prefix $prefix {
+ gdb_load $binfile
+
+ gdb_breakpoint "initialized" {temporary}
+ gdb_run_cmd
+ gdb_test "" ".*reakpoint ., initialized .*${srcfile}.*" "run"
+
+ return [get_integer_valueof "pid" -1]
+ }
+}
+
+set testpid1 [initialize_inferior "inf 1"]
+if {$testpid1 == -1} {
+ return -1
+}
+
+# Start another inferior.
+gdb_test "add-inferior" "Added inferior 2.*" \
+ "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" \
+ "switch to inferior 2"
+
+set testpid2 [initialize_inferior "inf 2"]
+if {$testpid2 == -1} {
+ return -1
+}
+
+# We want to continue both processes.
+gdb_test_no_output "set schedule-multiple on"
+
+# Resume, but then kill both from outside.
+gdb_test_multiple "continue" "continue processes" {
+ -re "Continuing.\[\r\n\]+" {
+ # Kill both processes at once.
+ remote_exec target "kill -9 ${testpid1} ${testpid2}"
+ exp_continue
+ }
+ -re "Program terminated with signal.*$gdb_prompt" {
+ pass $gdb_test_name
+ }
+}
+
+# Find the current inferior's id.
+set current_inferior 1
+gdb_test_multiple "info inferiors" "find the current inf id" {
+ -re "\\* 1 .*$gdb_prompt $" {
+ set current_inferior 1
+ set other_inferior 2
+ pass $gdb_test_name
+ }
+ -re "\\* 2 .*$gdb_prompt $" {
+ set current_inferior 2
+ set other_inferior 1
+ pass $gdb_test_name
+ }
+}
+
+# Switch to the other inferior and check it, too, continues to the end.
+gdb_test "inferior $other_inferior" \
+ ".*Switching to inferior $other_inferior.*" \
+ "switch to the other inferior"
+
+gdb_test "continue" \
+ "Program terminated with signal SIGKILL, .*" \
+ "continue the other inferior"
--
2.17.1