gdb/infrun: handle already-exited threads when attempting to stop

author Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>

Thu, 14 May 2020 11:59:54 +0000 (13:59 +0200)

committer Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>

Thu, 14 May 2020 11:59:54 +0000 (13:59 +0200)
author Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Thu, 14 May 2020 11:59:54 +0000 (13:59 +0200)
committer Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Thu, 14 May 2020 11:59:54 +0000 (13:59 +0200)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog

index 6d56c04bc37d12c663a5abb534a26d85a12ff2ff..c4da2a92f9ac8fc52ae3e855361a0f03db73cf5b 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,20 @@
+2020-05-14  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+           Tom de Vries  <tdevries@suse.de>
+           Pedro Alves  <palves@redhat.com>
+
+       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.
+       (handle_no_resumed): Remove code handling a live inferior with no
+       threads.
+       * remote.c (has_single_non_exited_thread): New.
+       (remote_target::update_thread_list): Do not delete a thread if is
+       the last thread of the process.
+       * thread.c (thread_select): Call delete_exited_threads instead of
+       prune_threads.
+
  2020-05-14  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
  
         * infrun.c (stop_all_threads): Enable/disable thread events of all
diff --git a/gdb/infrun.c b/gdb/infrun.c

index 2a8e73c1111ced78f9dd74a80d83af45a0ad05b6..c3e23a28bdf163cf12d8b4292d94f52e1a998d31 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4804,7 +4804,11 @@ stop_all_threads (void)
         {
           int need_wait = 0;
  
-         update_thread_list ();
+         for (auto *target : all_non_exited_process_targets ())
+           {
+             switch_to_target_no_thread (target);
+             update_thread_list ();
+           }
  
           /* Go through all threads looking for threads that we need
              to tell the target to stop.  */
@@ -4879,13 +4883,63 @@ 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.  */
+           }
+         else if (event.ws.kind == TARGET_WAITKIND_THREAD_EXITED
+                  || event.ws.kind == TARGET_WAITKIND_EXITED
+                  || event.ws.kind == TARGET_WAITKIND_SIGNALLED)
             {
-             /* All resumed threads exited
-                or one thread/process exited/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;
+                   }
+
+                 /* If there is no available thread, the event would
+                    have to be appended to a per-inferior event list,
+                    which does not exist (and if it did, we'd have
+                    to adjust run control command to be able to
+                    resume such an inferior).  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.  */
+                 switch_to_thread_no_regs (t);
+                 mark_non_executing_threads (event.target, event.ptid,
+                                             event.ws);
+                 save_waitstatus (t, &event.ws);
+                 t->stop_requested = false;
+               }
             }
           else
             {
@@ -5063,24 +5117,6 @@ handle_no_resumed (struct execution_control_state *ecs)
         }
      }
  
-  /* Note however that we may find no resumed thread because the whole
-     process exited meanwhile (thus updating the thread list results
-     in an empty thread list).  In this case we know we'll be getting
-     a process exit event shortly.  */
-  for (inferior *inf : all_non_exited_inferiors (ecs->target))
-    {
-      thread_info *thread = any_live_thread_of_inferior (inf);
-      if (thread == NULL)
-       {
-         if (debug_infrun)
-           fprintf_unfiltered (gdb_stdlog,
-                               "infrun: TARGET_WAITKIND_NO_RESUMED "
-                               "(expect process exit)\n");
-         prepare_to_wait (ecs);
-         return 1;
-       }
-    }
-
    /* Go ahead and report the event.  */
    return 0;
  }
diff --git a/gdb/remote.c b/gdb/remote.c

index 5db406e045c171abda255a38a654c864aeb21ca2..5b1fa848536aa5fd07c690551f35e8d5826ee2de 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3785,6 +3785,18 @@ remote_target::remote_get_threads_with_qthreadinfo (threads_listing_context *con
    return 0;
  }
  
+/* Return true if INF only has one non-exited thread.  */
+
+static bool
+has_single_non_exited_thread (inferior *inf)
+{
+  int count = 0;
+  for (thread_info *tp ATTRIBUTE_UNUSED : inf->non_exited_threads ())
+    if (++count > 1)
+      break;
+  return count == 1;
+}
+
  /* Implement the to_update_thread_list function for the remote
     targets.  */
  
@@ -3824,6 +3836,14 @@ remote_target::update_thread_list ()
  
           if (!context.contains_thread (tp->ptid))
             {
+             /* Do not remove the thread if it is the last thread in
+                the inferior.  This situation happens when we have a
+                pending exit process status to process.  Otherwise we
+                may end up with a seemingly live inferior (i.e.  pid
+                != 0) that has no threads.  */
+             if (has_single_non_exited_thread (tp->inf))
+               continue;
+
               /* Not found.  */
               delete_thread (tp);
             }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog

index 3ce59bc1c56c790e0617e8c5882621ba9f6d29e7..c987c829c45d92d1cf16872ff23b0b3df17325e9 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-14  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+           Pedro Alves  <palves@redhat.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.
+
  2020-05-14  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
  
         * gdb.base/annota1.exp: Update the expected output.
diff --git a/gdb/testsuite/gdb.multi/multi-exit.c b/gdb/testsuite/gdb.multi/multi-exit.c

new file mode 100644 (file)

index 0000000..f4825c8
--- /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 (file)

index 0000000..393093b
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-exit.exp
@@ -0,0 +1,134 @@
+# 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}
+}
+
+# Start inferior NUM.
+
+proc start_inferior {num} {
+    with_test_prefix "start_inferior $num" {
+       global srcfile binfile
+
+       if {$num != 1} {
+           gdb_test "add-inferior" "Added inferior $num.*" \
+               "add empty inferior"
+           gdb_test "inferior $num" "Switching to inferior $num.*" \
+               "switch to inferior"
+       }
+
+       gdb_load $binfile
+
+       if {[gdb_start_cmd] < 0} {
+           fail "could not start"
+           return -1
+       }
+       gdb_test "" ".*reakpoint .*, main .*${srcfile}.*" "start"
+    }
+
+    return 0
+}
+
+# Sufficient inferiors to make sure that at least some other inferior
+# exits while we're handling a process exit event.
+set NUM_INFS 10
+
+for {set i 1} {$i <= $NUM_INFS} {incr i} {
+    if {[start_inferior $i] < 0} {
+       return -1
+    }
+}
+
+# We want to continue all processes.
+gdb_test_no_output "set schedule-multiple on"
+
+# Check that "continue" continues to the end of an inferior, as many
+# times as we have inferiors.
+
+for {set i 1} {$i <= $NUM_INFS} {incr i} {
+    with_test_prefix "inf $i" {
+       set live_inferior ""
+
+       # Pick any live inferior.
+       gdb_test_multiple "info inferiors" "" {
+           -re "($decimal) *process.*$gdb_prompt $" {
+               set live_inferior $expect_out(1,string)
+           }
+       }
+
+       if {$live_inferior == ""} {
+           return -1
+       }
+
+       gdb_test "inferior $live_inferior" \
+           ".*Switching to inferior $live_inferior.*" \
+           "switch to another inferior"
+
+       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" "continue" {
+           -re "Inferior ($decimal) \[^\n\r\]+ exited normally.*$gdb_prompt $" {
+               set exited_inferior $expect_out(1,string)
+               pass $gdb_test_name
+           }
+       }
+
+       if {$exited_inferior == ""} {
+           return -1
+       }
+    }
+}
+
+# Finally, check that we can re-run all inferiors.  Note that if any
+# inferior was still alive this would catch it, as "run" would query
+# "Start it from the beginning?".
+
+delete_breakpoints
+
+for {set i 1} {$i <= $NUM_INFS} {incr i} {
+    with_test_prefix "inf $i" {
+       gdb_test "inferior $i" \
+           ".*Switching to inferior $i.*" \
+           "switch to inferior for re-run"
+
+       gdb_test "run" "$inferior_exited_re normally\]" \
+           "re-run inferior"
+    }
+}
diff --git a/gdb/testsuite/gdb.multi/multi-kill.c b/gdb/testsuite/gdb.multi/multi-kill.c

new file mode 100644 (file)

index 0000000..66642bb
--- /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 (file)

index 0000000..ce60750
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/multi-kill.exp
@@ -0,0 +1,127 @@
+# 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}
+}
+
+# Start inferior NUM and record its PID in the TESTPID array.
+
+proc start_inferior {num} {
+    with_test_prefix "start_inferior $num" {
+       global testpid binfile srcfile
+
+       if {$num != 1} {
+           gdb_test "add-inferior" "Added inferior .*" \
+               "add empty inferior"
+           gdb_test "inferior $num" "Switching to inferior .*" \
+               "switch to inferior"
+       }
+
+       gdb_load $binfile
+
+       gdb_breakpoint "initialized" {temporary}
+       gdb_run_cmd
+       gdb_test "" ".*reakpoint .*, initialized .*${srcfile}.*" "run"
+
+       set testpid($num) [get_integer_valueof "pid" -1]
+       if {$testpid($num) == -1} {
+           return -1
+       }
+
+       return 0
+    }
+}
+
+# Sufficient inferiors to make sure that at least some other inferior
+# is killed while we're handling a killed event.
+set NUM_INFS 10
+
+for {set i 1} {$i <= $NUM_INFS} {incr i} {
+    if {[start_inferior $i] < 0} {
+       return -1
+    }
+}
+
+# We want to continue all processes.
+gdb_test_no_output "set schedule-multiple on"
+
+# Resume, but then kill all from outside.
+gdb_test_multiple "continue" "continue processes" {
+    -re "Continuing.\[\r\n\]+" {
+       # Kill all processes at once.
+
+       set kill_cmd "kill -9"
+       for {set i 1} {$i <= $NUM_INFS} {incr i} {
+           append kill_cmd " $testpid($i)"
+       }
+
+       remote_exec target $kill_cmd
+       exp_continue
+    }
+    -re "Program terminated with signal.*$gdb_prompt $" {
+       pass $gdb_test_name
+    }
+}
+
+# Check that "continue" collects the process kill event, as many times
+# as we have inferiors left.
+
+for {set i 2} {$i <= $NUM_INFS} {incr i} {
+    with_test_prefix "inf $i" {
+       set live_inferior ""
+
+       # Pick any live inferior.
+       gdb_test_multiple "info inferiors" "" {
+           -re "($decimal) *process.*$gdb_prompt $" {
+               set live_inferior $expect_out(1,string)
+           }
+       }
+
+       if {$live_inferior == ""} {
+           return -1
+       }
+
+       gdb_test "inferior $live_inferior" \
+           ".*Switching to inferior $live_inferior.*" \
+           "switch to inferior"
+
+       gdb_test "continue" \
+           "Program terminated with signal SIGKILL, .*" \
+           "continue to SIGKILL"
+    }
+}
diff --git a/gdb/thread.c b/gdb/thread.c

index 03805bd25659d6f672596cef91178f70c278a293..02672f01fcf13aed1a5d4571d3e6d15002233e7d 100644 (file)
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -2043,7 +2043,7 @@ thread_select (const char *tidstr, thread_info *tp)
  
    /* Since the current thread may have changed, see if there is any
       exited thread we can now delete.  */
-  prune_threads ();
+  delete_exited_threads ();
  }
  
  /* Print thread and frame switch command response.  */
author	Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
	Thu, 14 May 2020 11:59:54 +0000 (13:59 +0200)
committer	Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
	Thu, 14 May 2020 11:59:54 +0000 (13:59 +0200)
gdb/ChangeLog		patch \| blob \| blame \| history
gdb/infrun.c		patch \| blob \| blame \| history
gdb/remote.c		patch \| blob \| blame \| history
gdb/testsuite/ChangeLog		patch \| blob \| blame \| history
gdb/testsuite/gdb.multi/multi-exit.c	[new file with mode: 0644]	patch \| blob
gdb/testsuite/gdb.multi/multi-exit.exp	[new file with mode: 0644]	patch \| blob
gdb/testsuite/gdb.multi/multi-kill.c	[new file with mode: 0644]	patch \| blob
gdb/testsuite/gdb.multi/multi-kill.exp	[new file with mode: 0644]	patch \| blob
gdb/thread.c		patch \| blob \| blame \| history