[binutils-gdb] Fix PR remote/21852: Remote run without specifying a local binary crashes GDB

[Sergio Durigan Junior <sergiodj at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=87215ad1651ca3094d813eae06233fd7259b37e5

commit 87215ad1651ca3094d813eae06233fd7259b37e5
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Tue Aug 22 08:52:44 2017 -0400

    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  <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.

Diff:
---
 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(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7eeab62..437a9cb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-08-23  Pedro Alves  <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.
+
 2017-08-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* NEWS (Changes since GDB 8.0): Add set compile-gcc and show
diff --git a/gdb/remote.c b/gdb/remote.c
index ff59a0f..2249533 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 5581e5a..56ff191 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+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.
+
 2017-08-07  Weimin Pan  <weimin.pan@oracle.com>
 
 	* 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 0000000..f0eb154
--- /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 <http://www.gnu.org/licenses/>.  */
+
+/* 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 0000000..07bb828
--- /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 <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.  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"
+}