[PATCH 0/2] Fix gdbserver crash on all non-GNU/Linux targets

[PATCH 0/2] Fix gdbserver crash on all non-GNU/Linux targets

[Pedro Alves]

Documentation review needed on patch #1.

This series fixes the crash Ulrich reported here:

  https://sourceware.org/ml/gdb-patches/2015-08/msg00675.html

In order to add a test that exposes the issue on GNU/Linux, I need to
add the missing "set remote ...-packet" knob to disable the
multiprocess extensions.

I've pushed this to the
users/palves/gdbserver-crash-without-multiprocess branch for testing
convenience.

Pedro Alves (2):
  Add "set remote multiprocess-extensions-packet" command
  gdbserver crashes when multiprocess extensions aren't supported

 gdb/NEWS                                           |  4 ++
 gdb/doc/gdb.texinfo                                |  4 ++
 gdb/gdbserver/inferiors.c                          |  8 +++
 gdb/gdbserver/inferiors.h                          |  3 +
 gdb/gdbserver/remote-utils.c                       |  7 ++-
 gdb/remote.c                                       |  7 ++-
 .../gdb.server/connect-without-multi-process.c     | 22 +++++++
 .../gdb.server/connect-without-multi-process.exp   | 68 ++++++++++++++++++++++
 8 files changed, 118 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/connect-without-multi-process.c
 create mode 100644 gdb/testsuite/gdb.server/connect-without-multi-process.exp

-- 
1.9.3

[PATCH 1/2] Add "set remote multiprocess-extensions-packet" command

[Pedro Alves]

Being able to force-disable the RSP multiprocess extensions is useful
for testing.

gdb/ChangeLog:
2015-08-24  Pedro Alves  <palves@redhat.com>

	* NEWS (New commands): Mention set/show remote
	multiprocess-extensions-packet.
	* remote.c (remote_query_supported): Only tell the server to use
	the multiprocess extensions if the user hasn't force-disabled them
	with "set remote multiprocess-extensions-packet off".

gdb/doc/ChangeLog:
2015-08-24  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Remote Configuration): Document the "set/show
	remote multiprocess-extensions-packet" commands.
---
 gdb/NEWS            | 4 ++++
 gdb/doc/gdb.texinfo | 4 ++++
 gdb/remote.c        | 7 +++++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 9edfd5e..cc1866f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -28,6 +28,10 @@ set debug bfd-cache
 show debug bfd-cache
   Control display of debugging info regarding bfd caching.
 
+set remote multiprocess-extensions-packet
+show remote multiprocess-extensions--packet
+  Set/show the use of the remote protocol multiprocess extensions.
+
 * The "disassemble" command accepts a new modifier: /s.
   It prints mixed source+disassembly like /m with two differences:
   - disassembled instructions are now printed in program order, and
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5dfb14b..cd0abad 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20205,6 +20205,10 @@ are:
 @tab @code{Z0 and Z1}
 @tab @code{Support for target-side breakpoint condition evaluation}
 
+@item @code{multiprocess-extensions}
+@tab @code{multiprocess extensions}
+@tab Debug multiple processes and remote process PID awareness
+
 @item @code{swbreak-feature}
 @tab @code{swbreak stop reason}
 @tab @code{break}
diff --git a/gdb/remote.c b/gdb/remote.c
index 12294bc..a06037f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4352,7 +4352,8 @@ remote_query_supported (void)
       char *q = NULL;
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
-      q = remote_query_supported_append (q, "multiprocess+");
+      if (packet_set_cmd_state (PACKET_multiprocess_feature) != AUTO_BOOLEAN_FALSE)
+	q = remote_query_supported_append (q, "multiprocess+");
 
       if (packet_set_cmd_state (PACKET_swbreak_feature) != AUTO_BOOLEAN_FALSE)
 	q = remote_query_supported_append (q, "swbreak+");
@@ -13231,6 +13232,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace_conf_bts_size],
        "Qbtrace-conf:bts:size", "btrace-conf-bts-size", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_multiprocess_feature],
+       "multiprocess-feature", "multiprocess-feature", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_swbreak_feature],
                          "swbreak-feature", "swbreak-feature", 0);
 
@@ -13260,7 +13264,6 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
 	switch (i)
 	  {
 	  case PACKET_QNonStop:
-	  case PACKET_multiprocess_feature:
 	  case PACKET_EnableDisableTracepoints_feature:
 	  case PACKET_tracenz_feature:
 	  case PACKET_DisconnectedTracing_feature:
-- 
1.9.3

[PATCH 2/2] gdbserver crashes when multiprocess extensions aren't supported

[Pedro Alves]

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.
---
 gdb/gdbserver/inferiors.c                          |  8 +++
 gdb/gdbserver/inferiors.h                          |  3 +
 gdb/gdbserver/remote-utils.c                       |  7 ++-
 .../gdb.server/connect-without-multi-process.c     | 22 +++++++
 .../gdb.server/connect-without-multi-process.exp   | 68 ++++++++++++++++++++++
 5 files changed, 105 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/connect-without-multi-process.c
 create mode 100644 gdb/testsuite/gdb.server/connect-without-multi-process.exp

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 05563be..10cb664 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/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..9889479
--- /dev/null
+++ b/gdb/testsuite/gdb.server/connect-without-multi-process.exp
@@ -0,0 +1,68 @@
+# 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
+set executable ${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
+    global gdb_prompt
+    global hex
+
+    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 .*"
+
+    set test "target $gdbserver_protocol"
+    gdb_test_multiple "target $gdbserver_protocol $gdbserver_gdbport" $test {
+	-re "Remote debugging using .*\r\n$gdb_prompt " {
+	    # Do not anchor end, there may be more output in non-stop mode.
+	    pass $test
+	}
+    }
+
+    gdb_test "continue" "main .*" "continue to main"
+}
+
+foreach multiprocess { "off" "auto" } {
+    with_test_prefix "multiprocess=$multiprocess" {
+	do_test $multiprocess
+    }
+}
-- 
1.9.3

Re: [PATCH 1/2] Add "set remote multiprocess-extensions-packet" command

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 24 Aug 2015 18:10:36 +0100
> 
> +set remote multiprocess-extensions-packet
> +show remote multiprocess-extensions--packet
                                      ^^
Extra hyphen.

> gdb/doc/ChangeLog:
> 2015-08-24  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.texinfo (Remote Configuration): Document the "set/show
> 	remote multiprocess-extensions-packet" commands.

What the ChangeLog entry promises the text doesn't keep:

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 5dfb14b..cd0abad 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -20205,6 +20205,10 @@ are:
>  @tab @code{Z0 and Z1}
>  @tab @code{Support for target-side breakpoint condition evaluation}
>  
> +@item @code{multiprocess-extensions}
> +@tab @code{multiprocess extensions}
> +@tab Debug multiple processes and remote process PID awareness
> +
>  @item @code{swbreak-feature}
>  @tab @code{swbreak stop reason}
>  @tab @code{break}

Thanks.

Re: [PATCH 1/2] Add "set remote multiprocess-extensions-packet" command

[Pedro Alves]

On 08/24/2015 06:28 PM, Eli Zaretskii wrote:

>> gdb/doc/ChangeLog:
>> 2015-08-24  Pedro Alves  <palves@redhat.com>
>>
>> 	* gdb.texinfo (Remote Configuration): Document the "set/show
>> 	remote multiprocess-extensions-packet" commands.
> 
> What the ChangeLog entry promises the text doesn't keep:

AFAIK, that's really all there is to document this kind of
set remote foo-packet commands.  Above that table we have:

For each packet @var{name}, the command to enable or disable the
packet is @code{set remote @var{name}-packet}.  The available settings
are:

@multitable @columnfractions 0.28 0.32 0.25
@item Command Name
@tab Remote Packet
@tab Related Features

Thanks,
Pedro Alves

> 
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 5dfb14b..cd0abad 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -20205,6 +20205,10 @@ are:
>>  @tab @code{Z0 and Z1}
>>  @tab @code{Support for target-side breakpoint condition evaluation}
>>  
>> +@item @code{multiprocess-extensions}
>> +@tab @code{multiprocess extensions}
>> +@tab Debug multiple processes and remote process PID awareness
>> +
>>  @item @code{swbreak-feature}
>>  @tab @code{swbreak stop reason}
>>  @tab @code{break}
> 
> Thanks.
> 

Re: [PATCH 1/2] Add "set remote multiprocess-extensions-packet" command

[Eli Zaretskii]

> Date: Mon, 24 Aug 2015 18:31:08 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> On 08/24/2015 06:28 PM, Eli Zaretskii wrote:
> 
> >> gdb/doc/ChangeLog:
> >> 2015-08-24  Pedro Alves  <palves@redhat.com>
> >>
> >> 	* gdb.texinfo (Remote Configuration): Document the "set/show
> >> 	remote multiprocess-extensions-packet" commands.
> > 
> > What the ChangeLog entry promises the text doesn't keep:
> 
> AFAIK, that's really all there is to document this kind of
> set remote foo-packet commands.  Above that table we have:
> 
> For each packet @var{name}, the command to enable or disable the
> packet is @code{set remote @var{name}-packet}.

Strange habit, but who am I to object?

Thanks.