[2/2,GDBserver] Block and unblock SIGIO

  Nowadays, GDBserver disables async io (by ignoring SIGIO) when process
a serial event, and enables async io (by installing signal handler) when
resume the inferior and wait.  GDBserver may miss SIGIO (by interrupt)
and doesn't process SIGIO in time, which is shown by
gdb.base/interrupt-noterm.exp.  In the test, GDB sends "continue &" and
then "interrupt".  if '\003' arrives at a period between GDBserver
receives vCont;c and enables async io, SIGIO is ignored because signal
handler isn't installed.  GDBserver waits for the inferior and can not
notice '\003' until it returns from wait.

This patch changes the code to install SIGIO handler early, but block
and unblock SIGIO as needed.  In this way, we don't remove SIGIO
handler, so SIGIO can't be ignored.  However, GDBserver needs to
remove the signal handler when connection is closed.

gdb/gdbserver:

2016-01-26  Yao Qi  <yao.qi@linaro.org>

	* remote-utils.c (remote_close) [!USE_WIN32API]: Ignore SIGIO.
	(unblock_async_io): Remove.
	(enable_async_io): Don't install SIGIO handler.  Unblock it
	instead.
	(disable_async_io): Don't ignore SIGIO.  Block it instead.
	(initialize_async_io): Install SIGIO handler.  Don't call
	unblock_async_io.
---
 gdb/gdbserver/remote-utils.c | 51 ++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 21 deletions(-)
  

On 01/26/2016 09:58 AM, Yao Qi wrote:

> -static void
> -unblock_async_io (void)
> -{
> -#ifndef USE_WIN32API
> -  sigset_t sigio_set;
> -
> -  sigemptyset (&sigio_set);
> -  sigaddset (&sigio_set, SIGIO);
> -  sigprocmask (SIG_UNBLOCK, &sigio_set, NULL);
> -#endif
> -}
> +/* Asynchronous I/O support.  SIGIO must be unblocked when waiting,
> +   in order to accept Control-C from the client, and must be blocked
> +   when talking to the client.  */
>  
>  #ifdef __QNX__
>  static void
> @@ -820,12 +813,19 @@ static int async_io_enabled;
>  void
>  enable_async_io (void)
>  {
> +#ifndef USE_WIN32API
> +  sigset_t sigio_set;
> +#endif
> +
>    if (async_io_enabled)
>      return;
>  
>  #ifndef USE_WIN32API
> -  signal (SIGIO, input_interrupt);
> +  sigemptyset (&sigio_set);
> +  sigaddset (&sigio_set, SIGIO);
> +  sigprocmask (SIG_UNBLOCK, &sigio_set, NULL);
>  #endif
> +
>    async_io_enabled = 1;
>  #ifdef __QNX__
>    nto_comctrl (1);
> @@ -836,12 +836,19 @@ enable_async_io (void)
>  void
>  disable_async_io (void)
>  {
> +#ifndef USE_WIN32API
> +  sigset_t sigio_set;
> +#endif
> +
>    if (!async_io_enabled)
>      return;
>  
>  #ifndef USE_WIN32API
> -  signal (SIGIO, SIG_IGN);
> +  sigemptyset (&sigio_set);
> +  sigaddset (&sigio_set, SIGIO);
> +  sigprocmask (SIG_BLOCK, &sigio_set, NULL);
>  #endif
> +

I'd suggest factoring out this duplicate sigprocmask handling,
like, say, rename unblock_async_io and add a parameter:

/* Block or unblock SIGIO.  */

static void
block_unblock_async_io (int block)
{
#ifndef USE_WIN32API
  sigset_t sigio_set;

  sigemptyset (&sigio_set);
  sigaddset (&sigio_set, SIGIO);
  sigprocmask (block ? SIG_BLOCK : SIG_UNBLOCK, &sigio_set, NULL);
#endif
}

>    async_io_enabled = 0;
>  #ifdef __QNX__
>    nto_comctrl (0);
> @@ -852,12 +859,14 @@ disable_async_io (void)
>  void
>  initialize_async_io (void)
>  {
> -  /* Make sure that async I/O starts disabled.  */
> +  /* Install the signal handler.  */
> +#ifndef USE_WIN32API
> +  signal (SIGIO, input_interrupt);
> +#endif
> +
> +  /* Make sure that async I/O starts blocked.  */
>    async_io_enabled = 1;
>    disable_async_io ();

I think it's safer practice to block the signal before
installing the handler.

Otherwise LGTM.

> -
> -  /* Make sure the signal is unblocked.  */
> -  unblock_async_io ();
>  }
>  
>  /* Internal buffer used by readchar.
> 

Thanks,
Pedro Alves