[binutils-gdb] gdbserver crashes when multiprocess extensions aren't supported

[Pedro Alves <palves at sourceware dot org>]

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

commit 3d40fbb53b4478d94b6fa5abad0796cec5d5db49
Author: Pedro Alves <palves@redhat.com>
Date:   Mon Aug 24 19:58:31 2015 +0100

    gdbserver crashes when multiprocess extensions aren't supported
    
    Ref: https://sourceware.org/ml/gdb-patches/2015-08/msg00675.html
    
    If multiprocess extensions are off (because specific gdbserver port
    doesn't support them), then when gdbserver doesn't have a thread
    selected yet, and GDB sends Hg packet to select one, gdbserver
    crashes.  That's because extracting the desired thread id out of the
    packet that GDB sent depends on the current thread to fill in the
    missing process id ...  Fix this by getting the process id from the
    first (and only) process in the processes list instead.
    
    The GNU/Linux port doesn't trip on this because it always runs with
    multiprocess extensions enabled.  To make it easier to catch such
    regressions going forward, this commit also adds a new smoke test that
    spawns gdbserver, connects to it and runs to main with the
    multiprocess extensions force-disabled.
    
    gdb/gdbserver/ChangeLog:
    2015-08-24  Pedro Alves  <palves@redhat.com>
    
    	* inferiors.c (get_first_process): New function.
    	* inferiors.h (get_first_process): New declaration.
    	* remote-utils.c (read_ptid): Default to the first process in the
    	list, instead of to the current thread's process.
    
    gdb/testsuite/ChangeLog:
    2015-08-24  Pedro Alves  <palves@redhat.com>
    
    	* gdb.server/connect-without-multi-process.c: New file.
    	* gdb.server/connect-without-multi-process.exp: New file.

Diff:
---
 gdb/gdbserver/ChangeLog                            |  7 +++
 gdb/gdbserver/inferiors.c                          |  8 +++
 gdb/gdbserver/inferiors.h                          |  3 ++
 gdb/gdbserver/remote-utils.c                       |  7 +--
 gdb/testsuite/ChangeLog                            |  5 ++
 .../gdb.server/connect-without-multi-process.c     | 22 ++++++++
 .../gdb.server/connect-without-multi-process.exp   | 61 ++++++++++++++++++++++
 7 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 017fd5d..d9675e7 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,12 @@
 2015-08-24  Pedro Alves  <palves@redhat.com>
 
+	* inferiors.c (get_first_process): New function.
+	* inferiors.h (get_first_process): New declaration.
+	* remote-utils.c (read_ptid): Default to the first process in the
+	list, instead of to the current thread's process.
+
+2015-08-24  Pedro Alves  <palves@redhat.com>
+
 	* debug.c: Include gdb_sys_time.h instead of sys/time.h.
 	* event-loop.c: Likewise.
 	* remote-utils.c: Likewise.
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 3659df1..ce87d8c 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -306,6 +306,14 @@ find_process_pid (int pid)
     find_inferior_id (&all_processes, pid_to_ptid (pid));
 }
 
+/* Wrapper around get_first_inferior to return a struct process_info *.  */
+
+struct process_info *
+get_first_process (void)
+{
+  return (struct process_info *) get_first_inferior (&all_processes);
+}
+
 /* Return non-zero if INF, a struct process_info, was started by us,
    i.e. not attached to.  */
 
diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h
index 88ebbe3..d722616 100644
--- a/gdb/gdbserver/inferiors.h
+++ b/gdb/gdbserver/inferiors.h
@@ -128,6 +128,9 @@ void remove_inferior (struct inferior_list *list,
 
 struct inferior_list_entry *get_first_inferior (struct inferior_list *list);
 
+/* Return the first process in the processes list.  */
+struct process_info *get_first_process (void);
+
 struct process_info *add_process (int pid, int attached);
 void remove_process (struct process_info *process);
 struct process_info *find_process_pid (int pid);
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 586ced8..c6c516c 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -573,9 +573,10 @@ read_ptid (char *buf, char **obuf)
   /* No multi-process.  Just a tid.  */
   tid = hex_or_minus_one (p, &pp);
 
-  /* Since the stub is not sending a process id, then default to
-     what's in the current inferior.  */
-  pid = ptid_get_pid (current_ptid);
+  /* Since GDB is not sending a process id (multi-process extensions
+     are off), then there's only one process.  Default to the first in
+     the list.  */
+  pid = pid_of (get_first_process ());
 
   if (obuf)
     *obuf = pp;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9c0d33b..bf7f1eb 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-08-24  Pedro Alves  <palves@redhat.com>
+
+	* gdb.server/connect-without-multi-process.c: New file.
+	* gdb.server/connect-without-multi-process.exp: New file.
+
 2015-08-24  Luis Machado  <lgustavo@codesourcery.com>
 
 	* gdb.opt/inline-markers.c: Make z int.
diff --git a/gdb/testsuite/gdb.server/connect-without-multi-process.c b/gdb/testsuite/gdb.server/connect-without-multi-process.c
new file mode 100644
index 0000000..7e362b2
--- /dev/null
+++ b/gdb/testsuite/gdb.server/connect-without-multi-process.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/connect-without-multi-process.exp b/gdb/testsuite/gdb.server/connect-without-multi-process.exp
new file mode 100644
index 0000000..c4214c8
--- /dev/null
+++ b/gdb/testsuite/gdb.server/connect-without-multi-process.exp
@@ -0,0 +1,61 @@
+# Copyright 2015 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/>.  */
+
+# Check that we can connect to GDBserver with the multiprocess
+# extensions disabled, and run to main.
+
+load_lib gdbserver-support.exp
+
+if {[skip_gdbserver_tests]} {
+    return
+}
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Test spawning gdbserver with a program, connect to it and run to
+# main, with both multiprocess extensions on and off.
+proc do_test {multiprocess} {
+    global binfile
+
+    clean_restart $binfile
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
+
+    gdb_test_no_output "set remote multiprocess-feature $multiprocess"
+
+    set res [gdbserver_spawn ${binfile}]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    gdb_test "break main" "Breakpoint .*"
+
+    gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \
+	"Remote debugging using .*" \
+	"target $gdbserver_protocol"
+
+    gdb_test "continue" "main .*" "continue to main"
+}
+
+foreach multiprocess { "off" "auto" } {
+    with_test_prefix "multiprocess=$multiprocess" {
+	do_test $multiprocess
+    }
+}