[PATCH] Avoid inferior_ptid reference in gdb/remote.c (Re: [PATCH 00/23] Multi-target support)

[Pedro Alves <palves@redhat.com>]

Pedro Alves wrote:
On 9/8/19 9:50 PM, Philippe Waroquiers wrote:
>> Then when stopping this gdb (which automatically continues the valgrind executables
>> till valgrind reports the next error), launching a new gdb, and only connecting to the 32
>> bits valgrind gdbserver, here is what I see.
>>
>> (gdb) tar rem|lvgdb --pid=16727
>> Remote debugging using |lvgdb
>> ...
>> 0x04001092 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
>> (gdb) bt
>> #0  0x04001092 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
>> #1  0x0495fd27 in syscall () at ../sysdeps/unix/sysv/linux/i386/syscall.S:29
>> #2  0x0010922c in main () at scalar_exit_group.c:14
>> (gdb) c
>> Continuing.
>> ../../multi-target-v1/gdb/inferior.c:285: internal-error: inferior*
>> find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.
>> A problem internal to GDB has been detected,
>> further debugging may prove unreliable.
>> Quit this debugging session? (y or n) 
>>
>> So, that looks to be a regression with the valgrind gdbserver.
> Thanks.  I'll have to try reproducing this.

Thanks for spotting this.  It was a regression against any remote target
that does not support the remote protocol multi-process extensions.

This patch fixes it.

From cd7e386563fcdb43d647ce499c5e05b85d44ba68 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 15 Oct 2019 20:01:55 +0100
Subject: [PATCH] Avoid inferior_ptid reference in gdb/remote.c

The multi-target patch makes inferior_ptid point to null_ptid before
calling into target_wait, which catches bad uses of inferior_ptid,
since the current selected thread in gdb shouldn't have much relation
to the thread that reports an event.

One such bad use is found in remote_target::remote_parse_stop_reply,
where we handle the 'W' or 'X' packets (process exit), and the remote
target does not support the multi-process extensions, i.e., it does
not report the PID of the process that exited.

With the multi-target patch, that would result in a failed assertion,
trying to find the inferior for process pid 0.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* remote.c (remote_target::remote_parse_stop_reply) <W/X packets>:
	If no process is specified, assume remote's current process
	instead of inferior_ptid.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.server/no-multi-process.exp: New file.
---
 gdb/remote.c                                  |  6 ++--
 gdb/testsuite/gdb.server/no-multi-process.exp | 40 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/no-multi-process.exp

diff --git a/gdb/remote.c b/gdb/remote.c
index b6d7bb0a6d..05c8b336f2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7511,7 +7511,6 @@ Packet: '%s'\n"),
     case 'W':		/* Target exited.  */
     case 'X':
       {
-	int pid;
 	ULONGEST value;
 
 	/* GDB used to accept only 2 hex chars here.  Stubs should
@@ -7535,8 +7534,9 @@ Packet: '%s'\n"),
 	      event->ws.value.sig = GDB_SIGNAL_UNKNOWN;
 	  }
 
-	/* If no process is specified, assume inferior_ptid.  */
-	pid = inferior_ptid.pid ();
+	/* If no process is specified, assume the remote's current
+	   process.  */
+	int pid = get_remote_state ()->general_thread.pid ();
 	if (*p == '\0')
 	  ;
 	else if (*p == ';')
diff --git a/gdb/testsuite/gdb.server/no-multi-process.exp b/gdb/testsuite/gdb.server/no-multi-process.exp
new file mode 100644
index 0000000000..0e946dba6f
--- /dev/null
+++ b/gdb/testsuite/gdb.server/no-multi-process.exp
@@ -0,0 +1,40 @@
+# Copyright (C) 2019 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/>.
+
+# Smoke tests for debugging against a remote target with no
+# multi-process extensions support.
+
+load_lib gdbserver-support.exp
+
+if {[skip_gdbserver_tests]} {
+    return
+}
+
+standard_testfile server.c
+if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] {
+    return -1
+}
+
+# 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-packet off"
+gdbserver_run ""
+
+# The W/X packets do not include the PID of the exiting process
+# without the multi-process extensions.  Check that we handle process
+# exit correctly in that case.
+gdb_continue_to_end "" continue 1
-- 
2.14.5