Fix PR remote/21852: Remote run without specifying a local binary crashes GDB
Commit Message
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 Alvex <palves@redhat.com>
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 <sergiodj@redhat.com>
PR remote/21852
* gdb.server/normal.c: New file, copied from gdb.base.
* gdb.server/run-without-local-binary.exp: New file.
---
gdb/remote.c | 18 +++----
gdb/testsuite/gdb.server/normal.c | 24 +++++++++
.../gdb.server/run-without-local-binary.exp | 60 ++++++++++++++++++++++
3 files changed, 93 insertions(+), 9 deletions(-)
create mode 100644 gdb/testsuite/gdb.server/normal.c
create mode 100644 gdb/testsuite/gdb.server/run-without-local-binary.exp
Comments
On 08/23/2017 06:36 PM, Sergio Durigan Junior wrote:
> A testcase is provided. Regtested on buildbot.
Looks good to me. A question below.
> gdb/ChangeLog:
> 2017-08-23 Pedro Alvex <palves@redhat.com>
Love my new name, thanks. :-)
> + # Let's start gdbserver in extended-remote mode now.
> + 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
I guess this can't use gdbserver_start_extended because that
somehow assumes you're starting with a binary? Can you add
a comment?
Thanks,
Pedro Alves
On Wednesday, August 23 2017, Pedro Alves wrote:
> On 08/23/2017 06:36 PM, Sergio Durigan Junior wrote:
>
>> A testcase is provided. Regtested on buildbot.
>
> Looks good to me. A question below.
>
>> gdb/ChangeLog:
>> 2017-08-23 Pedro Alvex <palves@redhat.com>
>
> Love my new name, thanks. :-)
Sigh. Sorry, fixed.
>> + # Let's start gdbserver in extended-remote mode now.
>> + 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
>
> I guess this can't use gdbserver_start_extended because that
> somehow assumes you're starting with a binary? Can you add
> a comment?
Exactly. And sure, I added a comment.
Pushed:
87215ad1651ca3094d813eae06233fd7259b37e5
Thanks,
@@ -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
new file mode 100644
@@ -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 <http://www.gnu.org/licenses/>. */
+
+/* This test is just a normal return 0. */
+
+int
+main (int argc, char *argv[])
+{
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,60 @@
+# 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 <http://www.gnu.org/licenses/>. */
+
+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.
+ 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"
+}