Re: [RFC] Fork and Exec events with target remote

[Pedro Alves <palves at redhat dot com>]

On 10/13/2015 11:34 AM, Pedro Alves wrote:

>>  * What is the long-term plan for each of them?  Will they eventually
>> merge into one?
> 
> I'd very much like that to happen.  It always felt to me that
> "extended" was first added as a separate target to avoid changing
> existing stubs, with the idea that things would be merged later on.
> That was all before my time, so I'm just guessing.  It'll take someone
> to evaluate the current use cases (and try things against a few different
> stubs; e.g., qemu, gdbserver, valgrind, some deep embedded stub) and come up
> with a better model.  It may be that e.g., we can make gdb always
> try enabling extended remote, and leave disconnecting or not to
> the server side.  Or we add a "set remote disconnect-after-kill/detach/exit on/off"
> knob to split out that decision from the initial connection.  Or maybe
> something else.  It could also be that we could simplify things on the gdbserver
> side and get rid of the gdbserver --multi option.  I never gave it a
> deep enough think.

Funny enough, I stumbled on something quite related to this.
I've been working on "maint set target-non-stop on" for remote targets
(that is, always run the remote backend in non-stop mode), and noticed
that when testing with "remote" instead of "extended-remote", I got this
regression:

Running /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.threads/continue-pending-after-query.exp ...
FAIL: gdb.threads/continue-pending-after-query.exp: iter 4: continue until exit
FAIL: gdb.threads/continue-pending-after-query.exp: iter 6: continue until exit
FAIL: gdb.threads/continue-pending-after-query.exp: iter 10: continue until exit

                === gdb Summary ===

# of expected passes            28
# of unexpected failures        3


gdb.log shows:

continue
Continuing.
Remote communication error.  Target disconnected.: Connection reset by peer.
(gdb) FAIL: gdb.threads/continue-pending-after-query.exp: iter 4: continue until exit


Enabling gdb + gdbserver debug logs I see:

<<<< exiting linux_wait_1
Packet received: OK
infrun: prepare_to_wait
Sending packet: $vStopped#55...Packet received: W0;process:2b51
Sending packet: $vStopped#55...handling possible serial event
Writing resume reply for LWP 11089.11089:0
handling possible serial event
GDBserver exiting
Packet received: OK
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [Thread 0],
infrun:   status->kind = no-resumed
Sending packet: $Hgp2b51.2b51#41...Remote connection closed
(gdb) FAIL: gdb.threads/continue-pending-after-query.exp: iter 1: continue until exit
testcase /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.threads/continue-pending-after-query.exp completed in 0 seconds

Notice the "Packet received: W0;process:2b51" notification
was successfully sent to GDB (the "Packet received: OK"
is the response to the last "vStopped").

That makes gdbserver exit, in server.c:process_serial_event:

  if (!extended_protocol && have_ran && !target_running ())
    {
      /* In non-stop, defer exiting until GDB had a chance to query
	 the whole vStopped list (until it gets an OK).  */
      if (QUEUE_is_empty (notif_event_p, notif_stop.queue))
	{
	  /* Be transparent when GDB is connected through stdio -- no
	     need to spam GDB's console.  */
	  if (!remote_connection_is_stdio ())
	    fprintf (stderr, "GDBserver exiting\n");
	  remote_close ();
	  exit (0);
	}
    }

However, GDB is still busy processing an earlier event, and
sends a "Hg" packet, which errors out with "Remote connection closed".
IOW, it's not enough to wait for GDB to query the whole vStopped list,
gdbserver needs to wait until the exit event is really processed...

I'm not seeing a sane way to fix this only on the gdb side.  I'm
thinking the best is to make gdbserver not disconnect until gdb
does, like the patch below...

>From 93f47f8dd0fcbd51f52cc68c2246265cef51353e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 14 Oct 2015 11:56:36 +0100
Subject: [PATCH] gdbserver don't exit immediately

---
 gdb/gdbserver/remote-utils.c |  5 ++++-
 gdb/gdbserver/server.c       | 40 ++++++++++++++--------------------------
 2 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index e366091..9e63d52 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -882,7 +882,10 @@ readchar (void)
       if (readchar_bufcnt <= 0)
 	{
 	  if (readchar_bufcnt == 0)
-	    fprintf (stderr, "readchar: Got EOF\n");
+	    {
+	      if (remote_debug)
+		fprintf (stderr, "readchar: Got EOF\n");
+	    }
 	  else
 	    perror ("readchar");
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index e25b7c7..1ccc023 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3598,11 +3598,21 @@ captured_main (int argc, char *argv[])
 	  start_event_loop ();
 
 	  /* If an exit was requested (using the "monitor exit"
-	     command), terminate now.  The only other way to get
-	     here is for getpkt to fail; close the connection
-	     and reopen it at the top of the loop.  */
+	     command), terminate now.  */
+	  if (exit_requested)
+	    throw_quit ("Quit");
+
+	  /* The only other way to get here is for getpkt to fail:
+
+	      - If --once was specified, we're done.
 
-	  if (exit_requested || run_once)
+	      - If not in extended-remote mode, and we're no longer
+	        debugging anything, simply exit: GDB has disconnected
+	        after processing the last process exit.
+
+	      - Otherwise, close the connection and reopen it at the
+	        top of the loop.  */
+	  if (run_once || (!extended_protocol && !target_running ()))
 	    throw_quit ("Quit");
 
 	  fprintf (stderr,
@@ -3767,13 +3777,6 @@ process_serial_event (void)
   int packet_len;
   int new_packet_len = -1;
 
-  /* Used to decide when gdbserver should exit in
-     multi-mode/remote.  */
-  static int have_ran = 0;
-
-  if (!have_ran)
-    have_ran = target_running ();
-
   disable_async_io ();
 
   response_needed = 0;
@@ -4212,21 +4215,6 @@ process_serial_event (void)
 
   response_needed = 0;
 
-  if (!extended_protocol && have_ran && !target_running ())
-    {
-      /* In non-stop, defer exiting until GDB had a chance to query
-	 the whole vStopped list (until it gets an OK).  */
-      if (QUEUE_is_empty (notif_event_p, notif_stop.queue))
-	{
-	  /* Be transparent when GDB is connected through stdio -- no
-	     need to spam GDB's console.  */
-	  if (!remote_connection_is_stdio ())
-	    fprintf (stderr, "GDBserver exiting\n");
-	  remote_close ();
-	  exit (0);
-	}
-    }
-
   if (exit_requested)
     return -1;
 
-- 
1.9.3