[PATCH v2] gdb/remote: do not check for null_ptid in stop reply (was: [PATCH] gdb: do not require ptid in stop reply if target does not support multi-process)

[PATCH v2] gdb/remote: do not check for null_ptid in stop reply (was: [PATCH] gdb: do not require ptid in stop reply if target does not support multi-process)

[Tankut Baris Aktemur]

A gdbserver does not report a ptid in a 'W' or 'X' packet if multi-process
extensions are not supported or turned off.  See

https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#multiprocess-extensions
https://sourceware.org/gdb/current/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets

GDB's remote packet parser checks for whether a stop-reply packet
contains a ptid if the target is non-stop, and issues an error if no
ptid is included:

  if (target_is_non_stop_p () && event->ptid == null_ptid)
    error (_("No process or thread specified in stop reply: %s"), buf);

This leads to the following error when the non-stop
mode is turned on but multi-process extensions are off:

  $ gdb
  (gdb) set non-stop on
  (gdb) set remote multiprocess-feature-packet off
  (gdb) target remote | gdbserver - ./foo
  Remote debugging using | gdbserver - ./foo
  stdin/stdout redirected
  Process ./foo created; pid = 3712
  ...
  (gdb) continue
  Continuing.
  ...
  No process or thread specified in stop reply: W2a
  (gdb)

Because the check is done for stop reply packets in general, a similar
situation occurs if the 'T' or 'Tthread' packet is disabled in
gdbserver (i.e.  via --disable-packet=T).  E.g:

  $ gdb
  (gdb) set non-stop on
  (gdb) target remote | gdbserver --disable-packet=Tthread - ./foo
  ...
  No process or thread specified in stop reply: T0506:0000000000000000;07:10e2ffffff7f0000;10:9060ddf7ff7f0000;

or

  $ gdb
  (gdb) set non-stop on
  (gdb) target remote | gdbserver --disable-packet=T - ./foo
  ...
  No process or thread specified in stop reply: S05

The commit

  commit cada5fc921e39a1945c422eea055c8b326d8d353
  Date:   Wed Mar 11 12:30:13 2020 +0000

      gdb: Handle W and X remote packets without giving a warning

and its predecessor

  commit 24ed6739b699f329c2c45aedee5f8c7d2f54e493
  Date:   Thu Jan 30 14:35:40 2020 +0000

      gdb/remote: Restore support for 'S' stop reply packet

added warnings for when GDB has to make a guess for a missing ptid in
case of multiple threads/inferiors.  These warnings should suffice.
So, the simple solution is to remove the check completely.

Regression-tested on X86_64 Linux.

gdb/ChangeLog:
2020-03-31  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* remote.c (remote_target::remote_parse_stop_reply): Remove the
	check for no ptid in the stop reply when the target is non-stop.

gdb/testsuite/ChangeLog:
2020-03-31  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.server/stop-reply-no-thread.exp: Enhance the test
	scenario to cover execution until the end and also the case
	when no packet is disabled when starting gdbserver.
---
 gdb/remote.c                                  |  3 ---
 .../gdb.server/stop-reply-no-thread.exp       | 24 +++++++++++++++----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 72f1728ecbc..bfbc0bc21d3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7561,9 +7561,6 @@ Packet: '%s'\n"),
       event->ptid = minus_one_ptid;
       break;
     }
-
-  if (target_is_non_stop_p () && event->ptid == null_ptid)
-    error (_("No process or thread specified in stop reply: %s"), buf);
 }
 
 /* When the stub wants to tell GDB about a new notification reply, it
diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread.exp
index ffc1c27dcb4..7d99b090575 100644
--- a/gdb/testsuite/gdb.server/stop-reply-no-thread.exp
+++ b/gdb/testsuite/gdb.server/stop-reply-no-thread.exp
@@ -33,7 +33,7 @@ if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
 }
 
 # Run the tests with different features of GDBserver disabled.
-proc run_test { disable_feature } {
+proc run_test { disable_feature target_nonstop } {
     global binfile gdb_prompt decimal
 
     clean_restart ${binfile}
@@ -42,7 +42,11 @@ proc run_test { disable_feature } {
     # extended-remote board, therefore already connected.
     gdb_test "disconnect" ".*"
 
-    set res [gdbserver_start "--disable-packet=${disable_feature}" $binfile]
+    set packet_arg ""
+    if { $disable_feature != "" } {
+	set packet_arg "--disable-packet=${disable_feature}"
+    }
+    set res [gdbserver_start $packet_arg $binfile]
     set gdbserver_protocol [lindex $res 0]
     set gdbserver_gdbport [lindex $res 1]
 
@@ -50,6 +54,9 @@ proc run_test { disable_feature } {
     gdb_test_no_output "set remote threads-packet off"
     gdb_test_no_output "set remote multiprocess-feature-packet off"
 
+    # Set target-nonstop mode.
+    gdb_test_no_output "maint set target-non-stop ${target_nonstop}"
+
     set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
     if ![gdb_assert {$res == 0} "connect"] {
 	return
@@ -76,6 +83,13 @@ proc run_test { disable_feature } {
     #  Cannot execute this command without a live selected thread.
     #  (gdb)
     gdb_test "c" "Breakpoint $decimal, main.*" "continue to main"
+
+    # Continue until exit.  The server sends a 'W' with no PID.
+    # Bad GDB gave an error like below when target is nonstop:
+    #  (gdb) c
+    #  Continuing.
+    #  No process or thread specified in stop reply: W00
+    gdb_continue_to_end "" continue 1
 }
 
 # Disable different features within gdbserver:
@@ -85,6 +99,8 @@ proc run_test { disable_feature } {
 #
 # T: Start GDBserver with the entire 'T' stop reply packet disabled,
 #    GDBserver will instead send the 'S' stop reply.
-foreach_with_prefix to_disable { Tthread T } {
-    run_test $to_disable
+foreach_with_prefix to_disable { "" Tthread T } {
+    foreach_with_prefix t_nonstop { off on } {
+	run_test $to_disable $t_nonstop
+    }
 }
-- 
2.17.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2] gdb/remote: do not check for null_ptid in stop reply (was: [PATCH] gdb: do not require ptid in stop reply if target does not support multi-process)

[Pedro Alves]

On 4/1/20 2:17 PM, Tankut Baris Aktemur wrote:

> gdb/ChangeLog:
> 2020-03-31  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* remote.c (remote_target::remote_parse_stop_reply): Remove the
> 	check for no ptid in the stop reply when the target is non-stop.
> 
> gdb/testsuite/ChangeLog:
> 2020-03-31  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	* gdb.server/stop-reply-no-thread.exp: Enhance the test
> 	scenario to cover execution until the end and also the case
> 	when no packet is disabled when starting gdbserver.

Perfect, thanks.

Pedro Alves