This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[commit] remote async: don't start any query or command while waiting for a stop reply


Hi,

I've checked in this patch that fixes issues with target
remote in all-stop + async mode similar to this:

 (gdb) set target-async 1
 (gdb) tar remote :9999
 Remote debugging using :9999
 0xf7f15810 in ?? () from /lib/ld-linux.so.2
 (gdb) c&
 Continuing.
 (gdb) info threads
 Ignoring packet error, continuing...

The problem is that we've issued a background execution command, which
returns us to the event loop immediately, allowing other commands to be
issued, e.g., `help', `info breakpoints', `info threads', etc., while
the target is running.

What can and can't be issued while the target is running is
target-dependant.  With the remote target in all-stop, we can't
start another remote query if we're waiting for a stop
reply from the stub, so we've got a limitation at the protocol level.
What we *can* do is detect that situation, and error out instead
of hanging and destroying the debug session.

Here's the same as above, showing the remote log:

 (gdb)c&
 (...)
 Sending packet: $vCont;c#a8...
 (gdb) info threads
 Sending packet: $T71da#81...Timed out.
 Timed out.
 Timed out.
 Ignoring packet error, continuing...
 Sending packet: $qfThreadInfo#bb...Timed out.
 Timed out.
 Timed out.
 Ignoring packet error, continuing...
 (gdb)

The attached patch makes us throw an error if we're waiting
for a stop reply and try to send something to the
remote server.

Tested without regressions against a native gdbserver on 
x86-64-unknown-linux-gnu, in all-stop, sync and async modes.

-- 
Pedro Alves
2008-10-08  Pedro Alves  <pedro@codesourcery.com>

	* remote.c (struct remote_state) <waiting_for_stop_reply>: New
	field.
	(remote_open_1): Clear waiting_for_stop_reply.
	(remote_resume): Set waiting_for_stop_reply.
	(remote_wait): Clear or set waiting_for_stop_reply accordingly.
	(putpkt_binary): If we're in async mode and waiting for a stop
	reply, bail out with an error.
	(extended_remote_mourn_1): Clear waiting_for_stop_reply.

---
 gdb/remote.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-10-08 18:51:49.000000000 +0100
+++ src/gdb/remote.c	2008-10-08 20:11:59.000000000 +0100
@@ -263,6 +263,14 @@ struct remote_state
   /* True if the stub reported support for multi-process
      extensions.  */
   int multi_process_aware;
+
+  /* True if we resumed the target and we're waiting for the target to
+     stop.  In the mean time, we can't start another command/query.
+     The remote server wouldn't be ready to process it, so we'd
+     timeout waiting for a reply that would never come and eventually
+     we'd close the connection.  This can happen in asynchronous mode
+     because we allow GDB commands while the target is running.  */
+  int waiting_for_stop_reply;
 };
 
 /* Returns true if the multi-process extensions are in effect.  */
@@ -2866,6 +2874,7 @@ remote_open_1 (char *name, int from_tty,
   rs->noack_mode = 0;
   rs->multi_process_aware = 0;
   rs->extended = extended_p;
+  rs->waiting_for_stop_reply = 0;
 
   general_thread = not_sent_ptid;
   continue_thread = not_sent_ptid;
@@ -3411,6 +3420,12 @@ remote_resume (ptid_t ptid, int step, en
      NOT asynchronously.  */
   if (target_can_async_p ())
     target_async (inferior_event_handler, 0);
+
+  /* We've just told the target to resume.  The remote server will
+     wait for the inferior to stop, and then send a stop reply.  In
+     the mean time, we can't start another command/query ourselves
+     because the stub wouldn't be ready to process it.  */
+  rs->waiting_for_stop_reply = 1;
 }
 
 
@@ -3649,6 +3664,9 @@ remote_wait (ptid_t ptid, struct target_
 
       remote_stopped_by_watchpoint_p = 0;
 
+      /* We got something.  */
+      rs->waiting_for_stop_reply = 0;
+
       switch (buf[0])
 	{
 	case 'E':		/* Error of some sort.  */
@@ -3660,6 +3678,10 @@ remote_wait (ptid_t ptid, struct target_
 	  goto got_status;
 	case 'F':		/* File-I/O request.  */
 	  remote_fileio_request (buf);
+
+	  /* This stop reply is special.  We reply back to the stub,
+	     and keep waiting for the target to stop.  */
+	  rs->waiting_for_stop_reply = 1;
 	  continue;
 	case 'T':		/* Status with PC, SP, FP, ...  */
 	  {
@@ -3828,6 +3850,10 @@ Packet: '%s'\n"),
 	  }
 	case 'O':		/* Console output.  */
 	  remote_console_output (buf + 1);
+
+	  /* The target didn't really stop; keep waiting.  */
+	  rs->waiting_for_stop_reply = 1;
+
 	  if (target_can_async_p ())
 	    {
 	      /* Return immediately to the event loop. The event loop
@@ -3851,11 +3877,17 @@ Packet: '%s'\n"),
 
 	      strcpy ((char *) buf, last_sent_step ? "s" : "c");
 	      putpkt ((char *) buf);
+
+	      /* We just told the target to resume, so a stop reply is
+		 in order.  */
+	      rs->waiting_for_stop_reply = 1;
 	      continue;
 	    }
 	  /* else fallthrough */
 	default:
 	  warning (_("Invalid remote reply: %s"), buf);
+	  /* Keep waiting.  */
+	  rs->waiting_for_stop_reply = 1;
 	  continue;
 	}
     }
@@ -4968,6 +5000,15 @@ putpkt_binary (char *buf, int cnt)
   int tcount = 0;
   char *p;
 
+  /* Catch cases like trying to read memory or listing threads while
+     we're waiting for a stop reply.  The remote server wouldn't be
+     ready to handle this request, so we'd hang and timeout.  We don't
+     have to worry about this in synchronous mode, because in that
+     case it's not possible to issue a command while the target is
+     running.  */
+  if (target_can_async_p () && rs->waiting_for_stop_reply)
+    error (_("Cannot execute this command while the target is running."));
+
   /* We're sending out a new packet.  Make sure we don't look at a
      stale cached response.  */
   rs->cached_wait_status = 0;
@@ -5480,6 +5521,10 @@ extended_remote_mourn_1 (struct target_o
 {
   struct remote_state *rs = get_remote_state ();
 
+  /* In case we got here due to an error, but we're going to stay
+     connected.  */
+  rs->waiting_for_stop_reply = 0;
+
   /* Unlike "target remote", we do not want to unpush the target; then
      the next time the user says "run", we won't be connected.  */
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]