[RFA] Fix gdbserver queued packet handling

[dje@google.com (Doug Evans)]

Hi.

gdbserver can read more than it needs to handle the current packet,
and then go to sleep in select waiting for the next packet.  Oops.

gdb handles this by using the timer support in the event loop to
queue more callbacks to finish processing buffered data.

e.g. ser-base.c:

	      next_state = create_timer (0, push_event, scb);

This isn't really a timer event, the delta is zero.
[btw, AFAICT, this is the only use of the timer support in the event loop]

This patch solves the problem in a similar way but doesn't bring over
the timer support from gdb's event-loop.c.
Timer support is a good thing to have in an event loop, but we don't need it,
at least not yet.

For reference sake, here's a patch to trigger the bug consistently:

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.402
diff -u -p -r1.402 remote.c
--- remote.c	16 Apr 2010 07:49:35 -0000	1.402
+++ remote.c	30 Apr 2010 16:37:59 -0000
@@ -6490,7 +6490,9 @@ putpkt_binary (char *buf, int cnt)
   int i;
   unsigned char csum = 0;
   char *buf2 = alloca (cnt + 6);
-
+  int is_noack_switch = strncmp (buf, "QStartNoAckMode",
+				 sizeof ("QStartNoAckMode")) == 0;
+  int timeout_test = ! rs->noack_mode && ! is_noack_switch;
   int ch;
   int tcount = 0;
   char *p;
@@ -6554,7 +6556,15 @@ putpkt_binary (char *buf, int cnt)
 	 Handle any notification that arrives in the mean time.  */
       while (1)
 	{
-	  ch = readchar (remote_timeout);
+	  if (timeout_test)
+	    {
+	      ch = SERIAL_TIMEOUT;
+	      timeout_test = 0;
+	    }
+	  else
+	    {
+	      ch = readchar (remote_timeout);
+	    }
 
 	  if (remote_debug)
 	    {


2010-04-30  Doug Evans  <dje@google.com>

	* server.h (queue_file_read_event): Declare.
	(reschedule_remote): Declare.
	* event-loop.c (queue_file_read_event): New function.
	* remote-utils.c (reschedule_remote): New function.
	(readchar_buf, readchar_bufcnt, readchar_bufp): New static globals,
	moved out of readchar.
	* server.c (handle_serial_event): Call reschedule_remote.

Index: event-loop.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/event-loop.c,v
retrieving revision 1.5
diff -u -p -r1.5 event-loop.c
--- event-loop.c	11 Apr 2010 16:33:55 -0000	1.5
+++ event-loop.c	30 Apr 2010 16:45:37 -0000
@@ -407,6 +407,37 @@ create_file_event (int fd)
   return file_event_ptr;
 }
 
+/* Queue a read event for file descriptor FD.
+   A handler must already be registered for FD.  */
+
+void
+queue_file_read_event (int fd)
+{
+  file_handler *file_ptr;
+  gdb_event *file_event_ptr;
+  int found = 0;
+
+  for (file_ptr = gdb_notifier.first_file_handler;
+       file_ptr != NULL;
+       file_ptr = file_ptr->next_file)
+    {
+      if (file_ptr->fd == fd)
+	{
+	  /* Enqueue an event only if there is no existing event for FD.  */
+	  if (file_ptr->ready_mask == 0)
+	    {
+	      file_event_ptr = create_file_event (file_ptr->fd);
+	      async_queue_event (file_event_ptr);
+	    }
+	  file_ptr->ready_mask |= GDB_READABLE;
+	  found = 1;
+	  break;
+	}
+    }
+
+  gdb_assert (found);
+}
+
 /* Called by do_one_event to wait for new events on the monitored file
    descriptors.  Queue file events as they are detected by the poll.
    If there are no events, this function will block in the call to
Index: remote-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.74
diff -u -p -r1.74 remote-utils.c
--- remote-utils.c	26 Apr 2010 17:38:07 -0000	1.74
+++ remote-utils.c	30 Apr 2010 16:45:37 -0000
@@ -926,23 +926,27 @@ initialize_async_io (void)
   unblock_async_io ();
 }
 
+/* Internal buffer used by readchar.
+   These are global to readchar because reschedule_remote needs to be
+   able to tell whether the buffer is empty.  */
+
+static unsigned char readchar_buf[BUFSIZ];
+static int readchar_bufcnt = 0;
+static unsigned char *readchar_bufp;
+
 /* Returns next char from remote GDB.  -1 if error.  */
 
 static int
 readchar (void)
 {
-  static unsigned char buf[BUFSIZ];
-  static int bufcnt = 0;
-  static unsigned char *bufp;
-
-  if (bufcnt-- > 0)
-    return *bufp++;
+  if (readchar_bufcnt-- > 0)
+    return *readchar_bufp++;
 
-  bufcnt = read (remote_desc, buf, sizeof (buf));
+  readchar_bufcnt = read (remote_desc, readchar_buf, sizeof (readchar_buf));
 
-  if (bufcnt <= 0)
+  if (readchar_bufcnt <= 0)
     {
-      if (bufcnt == 0)
+      if (readchar_bufcnt == 0)
 	fprintf (stderr, "readchar: Got EOF\n");
       else
 	perror ("readchar");
@@ -950,9 +954,19 @@ readchar (void)
       return -1;
     }
 
-  bufp = buf;
-  bufcnt--;
-  return *bufp++;
+  readchar_bufp = readchar_buf;
+  readchar_bufcnt--;
+  return *readchar_bufp++;
+}
+
+/* If there is still data in the buffer, queue another event to process it,
+   we can't sleep in select yet.  */
+
+void
+reschedule_remote (void)
+{
+  if (readchar_bufcnt > 0)
+    queue_file_read_event (remote_desc);
 }
 
 /* Read a packet from the remote machine, with error checking,
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.118
diff -u -p -r1.118 server.c
--- server.c	26 Apr 2010 22:02:33 -0000	1.118
+++ server.c	30 Apr 2010 16:45:37 -0000
@@ -3009,6 +3009,10 @@ handle_serial_event (int err, gdb_client
      Important in the non-stop mode asynchronous protocol.  */
   set_desired_inferior (1);
 
+  /* Give the packet reader a chance to schedule more work before
+     we go to sleep.  */
+  reschedule_remote ();
+
   return 0;
 }
 
Index: server.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
retrieving revision 1.66
diff -u -p -r1.66 server.h
--- server.h	12 Apr 2010 13:51:22 -0000	1.66
+++ server.h	30 Apr 2010 16:45:37 -0000
@@ -341,6 +341,7 @@ typedef int (handler_func) (int, gdb_cli
 extern void delete_file_handler (int fd);
 extern void add_file_handler (int fd, handler_func *proc,
 			      gdb_client_data client_data);
+extern void queue_file_read_event (int fd);
 
 extern void start_event_loop (void);
 
@@ -372,6 +373,7 @@ int putpkt (char *buf);
 int putpkt_binary (char *buf, int len);
 int putpkt_notif (char *buf);
 int getpkt (char *buf);
+void reschedule_remote (void);
 void remote_open (char *name);
 void remote_close (void);
 void write_ok (char *buf);