[1/2,GDBserver] Check input interrupt after reading in a packet

Message ID 1453802339-20401-2-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi Jan. 26, 2016, 9:58 a.m. UTC
  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.
---
 gdb/gdbserver/remote-utils.c | 9 +++++++++
 1 file changed, 9 insertions(+)
  

Comments

Pedro Alves Jan. 26, 2016, 11:42 a.m. UTC | #1
On 01/26/2016 09:58 AM, Yao Qi wrote:

> +  /* The '\003' may appear after some packet, and check it in the buffer,
> +     so that we can process the interrupt in time.  */

I can't parse seem to parse this correctly.

I think we should expand the explanation, like:

  /* 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.  */

> +  if (readchar_bufcnt > 0 && *readchar_bufp == '\003')

This should be a "while" instead of a single "if".

> +    {
> +      /* Consume the interrupt character in the buffer.  */
> +      readchar ();
> +      (*the_target->request_interrupt) ();
> +    }
> +
>    return bp - buf;
>  }

Otherwise LGTM.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 292197a..000457d 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1041,6 +1041,15 @@  getpkt (char *buf)
 	}
     }
 
+  /* The '\003' may appear after some packet, and check it in the buffer,
+     so that we can process the interrupt in time.  */
+  if (readchar_bufcnt > 0 && *readchar_bufp == '\003')
+    {
+      /* Consume the interrupt character in the buffer.  */
+      readchar ();
+      (*the_target->request_interrupt) ();
+    }
+
   return bp - buf;
 }