From 9d4a934ce604afea155c39f06834cdbc47e92a6e Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Fri, 22 Jun 2018 20:39:26 +0100 Subject: [PATCH] gdb: Fix assert for extended-remote target (PR gdb/18050) 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 /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 | 6 + gdb/target.c | 6 + gdb/testsuite/ChangeLog | 6 + .../gdb.server/extended-remote-restart.c | 60 ++++++++ .../gdb.server/extended-remote-restart.exp | 132 ++++++++++++++++++ 5 files changed, 210 insertions(+) create mode 100644 gdb/testsuite/gdb.server/extended-remote-restart.c create mode 100644 gdb/testsuite/gdb.server/extended-remote-restart.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2f5d1ef8b7b..682520fc400 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2018-08-08 Andrew Burgess + + PR gdb/18050: + * target.c (dispose_inferior): Don't dispose of inferiors that are + already killed. + 2018-08-08 Szabolcs Nagy * remote.c (remote_target::download_tracepoint): Change char* to diff --git a/gdb/target.c b/gdb/target.c index a5245abb281..115e9ae4948 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -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) { diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index c79e1e8f20c..d3e6a4a0fc4 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-08-08 Andrew Burgess + + PR gdb/18050: + * gdb.server/extended-remote-restart.c: New file. + * gdb.server/extended-remote-restart.exp: New file. + 2018-08-07 Simon Marchi * 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 index 00000000000..e195f48e080 --- /dev/null +++ b/gdb/testsuite/gdb.server/extended-remote-restart.c @@ -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 . */ + +#include +#include + +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 index 00000000000..39fcb9e2e58 --- /dev/null +++ b/gdb/testsuite/gdb.server/extended-remote-restart.exp @@ -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 . + +# 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 "" + + 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} + } + } +} -- 2.30.2