From 87215ad1651ca3094d813eae06233fd7259b37e5 Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Tue, 22 Aug 2017 08:52:44 -0400 Subject: [PATCH] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB There is an assertion that is triggering when we start GDB and instruct it to debug a remote inferior, but don't provide a local binary, like: ./gdb -nx -q --data-directory=data-directory -ex "tar ext :1234" \ -ex "set remote exec-file /bin/ls" -ex r In this case, when calling exec_file_locate_attach to locate the inferior, GDB is incorrectly resetting the breakpoints without a thread/inferior even running, which causes an assertion to be triggered: binutils-gdb/gdb/thread.c:1609: internal-error: scoped_restore_current_thread::scoped_restore_current_thread(): Assertion `tp != NULL' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) This happens because add_current_inferior_and_thread (on remote.c) is breaking an invariant: making inferior_ptid point to a non-existing thread and then calling common code, which in this case is breakpoint_re_set. The fix is to make sure that inferior_ptid points to null_ptid if there is no thread present. A testcase is provided. Regtested on buildbot. gdb/ChangeLog: 2017-08-23 Pedro Alves PR remote/21852 * remote.c (add_current_inferior_and_thread): Set inferior_ptid to null_ptid and switch to thread without reading the registers after adding the inferior. gdb/testsuite/ChangeLog: 2017-08-23 Sergio Durigan Junior PR remote/21852 * gdb.server/normal.c: New file, copied from gdb.base. * gdb.server/run-without-local-binary.exp: New file. --- gdb/ChangeLog | 7 +++ gdb/remote.c | 18 +++--- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.server/normal.c | 24 +++++++ .../gdb.server/run-without-local-binary.exp | 62 +++++++++++++++++++ 5 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.server/normal.c create mode 100644 gdb/testsuite/gdb.server/run-without-local-binary.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7eeab62ae11..437a9cb0721 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2017-08-23 Pedro Alves + + PR remote/21852 + * remote.c (add_current_inferior_and_thread): Set inferior_ptid + to null_ptid and switch to thread without reading the registers + after adding the inferior. + 2017-08-23 Jan Kratochvil * NEWS (Changes since GDB 8.0): Add set compile-gcc and show diff --git a/gdb/remote.c b/gdb/remote.c index ff59a0f81f4..2249533258e 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3831,19 +3831,16 @@ add_current_inferior_and_thread (char *wait_status) { struct remote_state *rs = get_remote_state (); int fake_pid_p = 0; - ptid_t ptid; inferior_ptid = null_ptid; /* Now, if we have thread information, update inferior_ptid. */ - ptid = get_current_thread (wait_status); + ptid_t curr_ptid = get_current_thread (wait_status); - if (!ptid_equal (ptid, null_ptid)) + if (curr_ptid != null_ptid) { if (!remote_multi_process_p (rs)) fake_pid_p = 1; - - inferior_ptid = ptid; } else { @@ -3851,14 +3848,17 @@ add_current_inferior_and_thread (char *wait_status) (such as kill) won't work. This variable serves (at least) double duty as both the pid of the target process (if it has such), and as a flag indicating that a target is active. */ - inferior_ptid = magic_null_ptid; + curr_ptid = magic_null_ptid; fake_pid_p = 1; } - remote_add_inferior (fake_pid_p, ptid_get_pid (inferior_ptid), -1, 1); + remote_add_inferior (fake_pid_p, ptid_get_pid (curr_ptid), -1, 1); - /* Add the main thread. */ - add_thread_silent (inferior_ptid); + /* Add the main thread and switch to it. Don't try reading + registers yet, since we haven't fetched the target description + yet. */ + thread_info *tp = add_thread_silent (curr_ptid); + switch_to_thread_no_regs (tp); } /* Print info about a thread that was found already stopped on diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 5581e5a69d0..56ff191dcae 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2017-08-23 Sergio Durigan Junior + + PR remote/21852 + * gdb.server/normal.c: New file, copied from gdb.base. + * gdb.server/run-without-local-binary.exp: New file. + 2017-08-07 Weimin Pan * gdb.arch/sparc64-adi.exp: New file. diff --git a/gdb/testsuite/gdb.server/normal.c b/gdb/testsuite/gdb.server/normal.c new file mode 100644 index 00000000000..f0eb1541fd3 --- /dev/null +++ b/gdb/testsuite/gdb.server/normal.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013-2017 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 just a normal return 0. */ + +int +main (int argc, char *argv[]) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.server/run-without-local-binary.exp b/gdb/testsuite/gdb.server/run-without-local-binary.exp new file mode 100644 index 00000000000..07bb8281190 --- /dev/null +++ b/gdb/testsuite/gdb.server/run-without-local-binary.exp @@ -0,0 +1,62 @@ +# Copyright 2017 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 . */ + +load_lib gdbserver-support.exp + +if {[skip_gdbserver_tests]} { + return +} + +standard_testfile normal.c + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +# Test running GDB without providing a local binary for it. In order +# to do that, we unset GDBFLAGS before running GDB. We also start +# gdbserver "by hand". For more details about this test, see PR +# remote/21852. +save_vars { GDBFLAGS } { + set GDBFLAGS "" + gdb_exit + gdb_start + + gdb_test_no_output "set remote exec-file $binfile" \ + "set remote exec-file" + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" "disconnect" + + # Let's start gdbserver in extended-remote mode now. We cannot + # use gdbserver_start_extended here because it starts gdbserver, + # then GDB *with* a local binary, and the connect both. + set res [gdbserver_start "--multi" ""] + set gdbserver_protocol [lindex $res 0] + if { [string first "extended-" $gdbserver_protocol] != 0} { + set gdbserver_protocol "extended-$gdbserver_protocol" + } + set gdbserver_gdbport [lindex $res 1] + set use_gdb_stub 0 + + gdb_test "target ${gdbserver_protocol} ${gdbserver_gdbport}" \ + "Remote debugging using $gdbserver_gdbport" \ + "connect to gdbserver" + + gdb_test "run" \ + "\\\[Inferior $decimal \\\(process $decimal\\\) exited normally\\\]" \ + "run test program until the end" +} -- 2.30.2