gdb: Fix assert for extended-remote target (PR gdb/18050)
authorAndrew Burgess <andrew.burgess@embecosm.com>
Fri, 22 Jun 2018 19:39:26 +0000 (20:39 +0100)
committerAndrew Burgess <andrew.burgess@embecosm.com>
Wed, 8 Aug 2018 12:14:45 +0000 (13:14 +0100)
Consider the following GDB session:

   (gdb) target extended-remote :2347
   (gdb) file /path/to/exe
   (gdb) set remote exec-file /path/to/exe
   (gdb) set detach-on-fork off
   (gdb) break breakpt
   (gdb) run
   # ... hits breakpoint
   (gdb) info inferiors
     Num  Description       Executable
   * 1    process 17001     /path/to/exe
     2    process 17002     /path/to/exe
   (gdb) kill
   (gdb) info inferiors
     Num  Description       Executable
   * 1    <null>            /path/to/exe
     2    process 17002     /path/to/exe
   (gdb) target extended-remote :2348
   ../../src/gdb/thread.c:660: internal-error: thread_info* any_thread_of_process(int): Assertion `pid != 0' failed.
   A problem internal to GDB has been detected,
   further debugging may prove unreliable.

Or, from bug PR gdb/18050:

   (gdb) start
   (gdb) add-inferior -exec /path/to/exe
   (gdb) target extended-remote :2347
   ../../src/gdb/thread.c:660: internal-error: thread_info* any_thread_of_process(int): Assertion `pid != 0' failed.
   A problem internal to GDB has been detected,
   further debugging may prove unreliable.

The issue is calling target.c:dispose_inferior with a killed inferior in
the inferior list.  This assertion is fixed in this commit.

The new test for this issue only runs on platforms that support
'detach-on-fork', and when using
'--target_board=native-extended-gdbserver'.

gdb/ChangeLog:

PR gdb/18050:
* target.c (dispose_inferior): Don't dispose of inferiors that are
already killed.

gdb/testsuite/ChangeLog:

PR gdb/18050:
* gdb.server/extended-remote-restart.c: New file.
* gdb.server/extended-remote-restart.exp: New file.

gdb/ChangeLog
gdb/target.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.server/extended-remote-restart.c [new file with mode: 0644]
gdb/testsuite/gdb.server/extended-remote-restart.exp [new file with mode: 0644]

index 2f5d1ef8b7b51620c215e688a4c65a595089cab2..682520fc4005875e12d38186a6dc1b4a497a7282 100644 (file)
@@ -1,3 +1,9 @@
+2018-08-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       PR gdb/18050:
+       * target.c (dispose_inferior): Don't dispose of inferiors that are
+       already killed.
+
 2018-08-08  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
        * remote.c (remote_target::download_tracepoint): Change char* to
index a5245abb281168f2996170387e00e9b7b1fdaa0c..115e9ae49483f4ee592ca8deca71f728c595e16a 100644 (file)
@@ -2022,6 +2022,12 @@ target_pre_inferior (int from_tty)
 static int
 dispose_inferior (struct inferior *inf, void *args)
 {
+  /* Not all killed inferiors can, or will ever be, removed from the
+     inferior list.  Killed inferiors clearly don't need to be killed
+     again, so, we're done.  */
+  if (inf->pid == 0)
+    return 0;
+
   thread_info *thread = any_thread_of_inferior (inf);
   if (thread != NULL)
     {
index c79e1e8f20cd3f794b8faffbb652284e50b627dc..d3e6a4a0fc4d5df2aab8c3c39cbeb26ae63bab3a 100644 (file)
@@ -1,3 +1,9 @@
+2018-08-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       PR gdb/18050:
+       * gdb.server/extended-remote-restart.c: New file.
+       * gdb.server/extended-remote-restart.exp: New file.
+
 2018-08-07  Simon Marchi  <simon.marchi@polymtl.ca>
 
        * gdb.fortran/nested-funcs.exp: Replace "set index = 42" with
diff --git a/gdb/testsuite/gdb.server/extended-remote-restart.c b/gdb/testsuite/gdb.server/extended-remote-restart.c
new file mode 100644 (file)
index 0000000..e195f48
--- /dev/null
@@ -0,0 +1,60 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 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 <unistd.h>
+#include <stdlib.h>
+
+static void
+breakpt ()
+{
+  asm ("" ::: "memory");
+}
+
+static void
+go_child ()
+{
+  breakpt ();
+
+  while (1)
+    sleep (1);
+}
+
+static void
+go_parent ()
+{
+  breakpt ();
+
+  while (1)
+    sleep (1);
+}
+
+int
+main ()
+{
+  pid_t pid;
+
+  pid = fork ();
+  if (pid == -1)
+    abort ();
+
+  if (pid == 0)
+    go_child ();
+  else
+    go_parent ();
+
+  exit (EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.server/extended-remote-restart.exp b/gdb/testsuite/gdb.server/extended-remote-restart.exp
new file mode 100644 (file)
index 0000000..39fcb9e
--- /dev/null
@@ -0,0 +1,132 @@
+# Copyright 2018 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/>.
+
+# This test is about restarting execution of a forked application when
+# using gdb extended remote target.
+#
+# There are two issues that the test tries to expose in GDB:
+#
+# 1. GDB would throw an assertion upon reconnecting to a remote target
+# if there was more than one inferior already active in GDB, and
+#
+# 2. GDB would not prune transient inferiors from the inferior list
+# when reconnecting to a remote target.  So, for example, an inferior
+# created by GDB to track the child of a fork would usually be removed
+# from the inferior list once the child exited.  However, reconnecting
+# to a remote target would result in the child inferior remaining in
+# the inferior list.
+
+# This test is only for extended remote targets.
+if {[target_info gdb_protocol] != "extended-remote"} {
+    continue
+}
+
+# This test also makes use of 'detach-on-fork' which is not supported
+# on all platforms.
+if { ![istarget "*-*-linux*"] && ![istarget "*-*-openbsd*"] } then {
+    continue
+}
+
+# And we need to be able to reconnect to gdbserver.
+set gdbserver_reconnect_p 1
+if { [info proc gdb_reconnect] == "" } {
+    return 0
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Core of the test.  DO_KILL_P controls whether we kill one of the
+# inferiors before reconnecting.  And FOLLOW_CHILD_P controls whether
+# we follow the child or the parent at the fork.
+proc test_reload { do_kill_p follow_child_p } {
+    global decimal
+    global binfile
+
+    clean_restart ${binfile}
+
+    if ![runto_main] then {
+       fail "can't run to main"
+       return 0
+    }
+
+    # Set detach-on-fork off
+    gdb_test_no_output "set detach-on-fork off"
+
+    set live_inf_ptn "process $decimal"
+    set dead_inf_ptn "<null>"
+
+    if ${follow_child_p} {
+       gdb_test_no_output "set follow-fork child"
+       set parent_prefix " "
+       set child_prefix "\\*"
+       set parent_inf_after_kill_ptn ${live_inf_ptn}
+       set child_inf_after_kill_ptn ${dead_inf_ptn}
+    } else {
+       gdb_test_no_output "set follow-fork parent"
+       set parent_prefix "\\*"
+       set child_prefix " "
+       set parent_inf_after_kill_ptn ${dead_inf_ptn}
+       set child_inf_after_kill_ptn ${live_inf_ptn}
+    }
+
+    gdb_breakpoint "breakpt"
+    gdb_continue_to_breakpoint "breakpt"
+
+    # Check we have the expected inferiors.
+    gdb_test "info inferiors" \
+       [multi_line \
+            "  Num  Description       Executable.*" \
+            "${parent_prefix} 1 +${live_inf_ptn} \[^\r\n\]+" \
+            "${child_prefix} 2 +${live_inf_ptn} \[^\r\n\]+" ] \
+       "Check inferiors at breakpoint"
+
+    if { $do_kill_p } {
+       # (Optional) Kill one of the inferiors.
+       gdb_test "kill" \
+           "" \
+           "Kill inferior" \
+           "Kill the program being debugged.*y or n. $" \
+           "y"
+
+       # Check the first inferior really did die.
+       gdb_test "info inferiors" \
+           [multi_line \
+                "  Num  Description       Executable.*" \
+                "${parent_prefix} 1 +${parent_inf_after_kill_ptn} \[^\r\n\]+" \
+                "${child_prefix} 2 +${child_inf_after_kill_ptn} \[^\r\n\]+" ] \
+           "Check inferior was killed"
+    }
+
+    # Reconnect to the target.
+    if { [gdb_reconnect] == 0 } {
+       pass "reconnect after fork"
+    } else {
+       fail "reconnect after fork"
+    }
+}
+
+# Run all combinations of the test.
+foreach do_kill_p [list 1 0] {
+    foreach follow_child_p [list 1 0] {
+       with_test_prefix \
+           "kill: ${do_kill_p}, follow-child ${follow_child_p}" {
+               test_reload ${do_kill_p} ${follow_child_p}
+           }
+    }
+}