Re: [PATCH v2 00/16] Interrupting programs that block/ignore SIGINT

[Andrew Burgess <andrew.burgess@embecosm.com>]

Pedro,

This change looks awesome!  I've wanted this for a long time now, so
glad it might finally arrive.

I have a question for a slightly related topic.  I don't think this
needs resolving prior to this series landing, but it might be worth
having a discussion about what the solution might be within this new
setup.

I was recently looking at signal handling in general, and started
looking at how SIGTSTP is handled, for example, as might be delivered
by Ctrl-Z from a terminal.

My interest in this started by looking at async_sigtstp_handler in
event-top.c, I noticed at the end of this function there is code to
reprint the prompt, which should be run after GDB is resumed, after a
stop.

However, right now, this is not the case.  When the terminal is
claimed by GDB then, when GDB is stopped and resumed, no prompt is
printed.

On digging into this, the problem is that the SIGTSTP handler is only
installed for the duration of command_line_input, which is the
non-async command line parser.

So, my initial plan was to removed the SIGTSTP handling from
command_line_input, and instead have the code which switches terminal
ownership install/remove the SIGTSTP handler...

...but then I realised, that if GDB doesn't own the terminal the
SIGTSTP should be sent directly to the inferior anyway, right?  So,
then my plan became, lets just install the SIGTSTP handler right from
the start, and this seems to work fine before your changes (there are
still some bugs to work out relating to printing the correct prompt in
all cases, and reprinting any partial read line input that might have
been in place, but these issues can be ignored for this discussion).

The problem then is what should happen after your work is applied?

The patch below applies on top of your work, and demonstrates the
problem, when GDB owns the terminal stop/resume works fine:

  (gdb)
  [1]+  Stopped                 ./gdb/gdb --data-directory ./gdb/data-directory/ ~/tmp/loop.x
  $ fg
  ./gdb/gdb --data-directory ./gdb/data-directory/ ~/tmp/loop.x
  (gdb)

But as the inferior never really takes over the terminal any more,
it's not clear to me what we should do?

One idea I've had is to just make SIGTSTP interrupt the inferior just
like SIGINT does.  This would drop the user back to a prompt, at which
point the user could send SIGTSTP again if they then wanted GDB itself
to stop.

Look forward to hearing your thoughts,

Thanks,
Andrew

---

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 9741b23576a..d609d73a016 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -962,6 +962,7 @@ async_init_signals (void)
 #ifdef SIGTSTP
   sigtstp_token =
     create_async_signal_handler (async_sigtstp_handler, NULL, "sigtstp");
+  signal (SIGTSTP, handle_sigtstp);
 #endif
 
   install_handle_sigsegv ();
@@ -1182,27 +1183,34 @@ handle_sigtstp (int sig)
 static void
 async_sigtstp_handler (gdb_client_data arg)
 {
-  char *prompt = get_prompt ();
+  if (!target_terminal::is_ours ())
+    {
+      fprintf (stderr, "TODO: Not really sure what we should do here.\n");
+    }
+  else
+    {
+      char *prompt = get_prompt ();
 
-  signal (SIGTSTP, SIG_DFL);
+      signal (SIGTSTP, SIG_DFL);
 #if HAVE_SIGPROCMASK
-  {
-    sigset_t zero;
+      {
+	sigset_t zero;
 
-    sigemptyset (&zero);
-    gdb_sigmask (SIG_SETMASK, &zero, 0);
-  }
+	sigemptyset (&zero);
+	gdb_sigmask (SIG_SETMASK, &zero, 0);
+      }
 #elif HAVE_SIGSETMASK
-  sigsetmask (0);
+      sigsetmask (0);
 #endif
-  raise (SIGTSTP);
-  signal (SIGTSTP, handle_sigtstp);
-  printf_unfiltered ("%s", prompt);
-  gdb_flush (gdb_stdout);
+      raise (SIGTSTP);
+      signal (SIGTSTP, handle_sigtstp);
+      fprintf_unfiltered (gdb_stdout, "%s%s", prompt, rl_line_buffer);
+      gdb_flush (gdb_stdout);
 
-  /* Forget about any previous command -- null line now will do
-     nothing.  */
-  dont_repeat ();
+      /* Forget about any previous command -- null line now will do
+	 nothing.  */
+      dont_repeat ();
+    }
 }
 #endif /* SIGTSTP */
 
diff --git a/gdb/top.c b/gdb/top.c
index 6e0f43d2fd9..876546c023f 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1325,11 +1325,6 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix)
   /* Starting a new command line.  */
   cmd_line_buffer.used_size = 0;
 
-#ifdef SIGTSTP
-  if (job_control)
-    signal (SIGTSTP, handle_sigtstp);
-#endif
-
   while (1)
     {
       gdb::unique_xmalloc_ptr<char> rl;
@@ -1385,11 +1380,6 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix)
       prompt = NULL;
     }
 
-#ifdef SIGTSTP
-  if (job_control)
-    signal (SIGTSTP, SIG_DFL);
-#endif
-
   return cmd;
 }
 \f