Re: [PATCH] driver: ignore SIGINT while waiting on subprocesses to finish

[Richard Biener <richard dot guenther at gmail dot com>]

On Sun, Nov 16, 2014 at 2:01 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> Currently the top-level driver handles SIGINT by immediately killing
> itself even when the driver has subprocesses (e.g. cc1, as) running.  I
> don't think this is a good idea because
>
>   1. if the top-level driver is waiting on a hanging subprocess,
>   pressing ^C will kill the driver but it may not necessarily kill the
>   subprocess; an unresponsive, perhaps busy-looping subprocess may be
>   running in the background yet the compiler will seem to have to
>   terminated successfully.
>
>   2. when debugging gcc with "gcc -wrapper gdb,--args" we are unable to
>   use ^C from within the GDB subprocess because pressing ^C immediately
>   kills the driver and we lose our terminal.  This makes debugging more
>   inconvenient.
>
> This patch fixes these two issues by having the driver ignore SIGINT
> while a subprocess is running.  The driver instead will have to wait for
> the subprocess to exit before it terminates, like usual.
>
> I tested this change by running "gcc -wrapper gdb", "gcc -wrapper
> valgrind" and plain old "gcc" in various ways (-pipe, -flto, -c, etc)
> and pressing ^C during compilation.  I noticed no differences in
> behavior or promptness other than finally being able to use ^C inside
> GDB.
>
> Does this change look OK for trunk after a successful bootstrap +
> regtest on x86_64-unknown-linux-gnu?

This means I can no longer interrupt a compile that is running too long?
That sounds wrong.

You should instead debug the actual compiler, not the driver.

Richard.

> ---
>  gcc/gcc.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 653ca8d..0802f41 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -2653,7 +2653,11 @@ add_sysrooted_prefix (struct path_prefix *pprefix, const char *prefix,
>    add_prefix (pprefix, prefix, component, priority,
>               require_machine_suffix, os_multilib);
>  }
> -
> +
> +/* True if the SIGINT signal should be ignored by the driver's
> +   signal handler.  */
> +static bool ignore_sigint_p;
> +
>  /* Execute the command specified by the arguments on the current line of spec.
>     When using pipes, this includes several piped-together commands
>     with `|' between them.
> @@ -2839,6 +2843,10 @@ execute (void)
>    if (pex == NULL)
>      fatal_error ("pex_init failed: %m");
>
> +  /* A SIGINT raised while subprocesses are running should not kill the
> +     driver.  */
> +  ignore_sigint_p = true;
> +
>    for (i = 0; i < n_commands; i++)
>      {
>        const char *errmsg;
> @@ -2878,6 +2886,8 @@ execute (void)
>      if (!pex_get_status (pex, n_commands, statuses))
>        fatal_error ("failed to get exit status: %m");
>
> +    ignore_sigint_p = false;
> +
>      if (report_times || report_times_to_file)
>        {
>         times = (struct pex_time *) alloca (n_commands * sizeof (struct pex_time));
> @@ -2893,6 +2903,9 @@ execute (void)
>
>         if (WIFSIGNALED (status))
>           {
> +           if (WTERMSIG (status) == SIGINT)
> +             raise (SIGINT);
> +           else
>  #ifdef SIGPIPE
>             /* SIGPIPE is a special case.  It happens in -pipe mode
>                when the compiler dies before the preprocessor is done,
> @@ -6710,6 +6723,12 @@ set_input (const char *filename)
>  static void
>  fatal_signal (int signum)
>  {
> +  if (signum == SIGINT && ignore_sigint_p)
> +    {
> +      signal (signum, fatal_signal);
> +      return;
> +    }
> +
>    signal (signum, SIG_DFL);
>    delete_failure_queue ();
>    delete_temp_files ();
> --
> 2.2.0.rc1.23.gf570943
>