[binutils-gdb] [GDBserver] Check input interrupt after reading in a packet

[Yao Qi <qiyao at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=18879fef1741464e522624bcc529048400453e0d

commit 18879fef1741464e522624bcc529048400453e0d
Author: Yao Qi <yao.qi@linaro.org>
Date:   Tue Jan 26 13:50:22 2016 +0000

    [GDBserver] Check input interrupt after reading in a packet
    
    GDBserver may read some packet together with '\003' in one go.  We've
    already checked '\003' first when reading packet by my patch,
    
      Check input interrupt first when reading packet
      https://sourceware.org/ml/gdb-patches/2016-01/msg00057.html
    
    but if we don't check '\003' *after* each packet, the interrupt will
    be processed next time GDBserver reads from the buffer, so that the
    interrupt isn't processed in time.  For example, GDB sends vCont;c and
    interrupt (see gdb.base/interrupt-noterm.exp), we'll resume the
    inferior and wait once packet vCont;c is seen.  If we don't check the
    interrupt character after vCont;c packet, interrupt character will stay
    in the buffer unattended until GDBserver returns from the wait, which
    may take a while.  Note that since we've read '\003' from file
    descriptor, SIGIO signal handler input_interrupt doesn't help either.
    
    This issue can be exposed by hacking the end of getpkt like
    @@ -1041,6 +1050,9 @@ getpkt (char *buf)
            }
         }
    
    +  if (readchar_bufcnt > 0)
    +    gdb_assert (*readchar_bufp != '\003');
    +
       return bp - buf;
     }
    
    and this can trigger internal error,
    (gdb) PASS: gdb.base/interrupt-noterm.exp: interrupt
    Remote connection closed^M
    (gdb) FAIL: gdb.base/interrupt-noterm.exp: inferior received SIGINT
    Remote debugging from host 10.2.206.40^M
    /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/remote-utils.c:1054: A problem internal to GDBserver has been detected.^M
    getpkt: Assertion `*readchar_bufp != '\003'' failed.^M
    
    This patch is to peek the buffer, if it is '\003', consume it and call
    *the_target->request_interrupt.
    
    gdb/gdbserver:
    
    2016-01-26  Yao Qi  <yao.qi@linaro.org>
    
    	* remote-utils.c (getpkt): If the buffer isn't empty, and the
    	first character is '\003', call *the_target->request_interrupt.

Diff:
---
 gdb/gdbserver/ChangeLog      |  5 +++++
 gdb/gdbserver/remote-utils.c | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 2e94aa8..4aa7350 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2016-01-26  Yao Qi  <yao.qi@linaro.org>
+
+	* remote-utils.c (getpkt): If the buffer isn't empty, and the
+	first character is '\003', call *the_target->request_interrupt.
+
 2016-01-25  Yao Qi  <yao.qi@linaro.org>
 
 	* remote-utils.c (new_thread_notify): Remove.
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 292197a..ccc99f1 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1041,6 +1041,22 @@ getpkt (char *buf)
 	}
     }
 
+  /* The readchar above may have already read a '\003' out of the socket
+     and moved it to the local buffer.  For example, when GDB sends
+     vCont;c immediately followed by interrupt (see
+     gdb.base/interrupt-noterm.exp).  As soon as we see the vCont;c, we'll
+     resume the inferior and wait.  Since we've already moved the '\003'
+     to the local buffer, SIGIO won't help.  In that case, if we don't
+     check for interrupt after the vCont;c packet, the interrupt character
+     would stay in the buffer unattended until after the next (unrelated)
+     stop.  */
+  while (readchar_bufcnt > 0 && *readchar_bufp == '\003')
+    {
+      /* Consume the interrupt character in the buffer.  */
+      readchar ();
+      (*the_target->request_interrupt) ();
+    }
+
   return bp - buf;
 }