Re: [PATCH] Supress SIGTTOU when handling errors

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

* Andreas Schwab <schwab@linux-m68k.org> [2019-05-18 15:41:56 +0200]:

> On Mai 16 2019, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> > [I've seen this on and off over many months on AArch64 and Arm, and am
> > assuming it isn't the intended behaviour. Not sure if this should be at
> > tcdrain or it should be done at a higher level - eg in the terminal
> > handling code]
> >
> > Calls to error () can cause SIGTTOU to send gdb to the background.
> >
> > For example, on an Arm build:
> >   (gdb) b main
> >   Breakpoint 1 at 0x10774: file /build/gdb/testsuite/../../../src/binutils-gdb/gdb/testsuite/gdb.base/watchpoint.c, line 174.
> >   (gdb) r
> >   Starting program: /build/gdb/testsuite/outputs/gdb.base/watchpoint/watchpoint
> >
> >   [1]+  Stopped                 ../gdb ./outputs/gdb.base/watchpoint/watchpoint
> 
> e671cd59d74cec9f53e110ce887128d1eeadb7f2 is the first bad commit
> commit e671cd59d74cec9f53e110ce887128d1eeadb7f2
> Author: Pedro Alves <palves@redhat.com>
> Date:   Tue Jan 30 14:23:51 2018 +0000
> 
>     Per-inferior target_terminal state, fix PR gdb/13211, more
> 
> Andreas.

Andreas,

Thanks for tracking this down.

It appears that the change in this patch that seems to be responsible
would correspond to Alan's patch #2 option.

I wonder if we should just apply something like the below to revert
part of Pedro's patch?  This will fix this problems we're seeing (as
Alan already pointed out) as this effectively makes 'ours_for_output
()' the same as 'ours ()' again.

My concern would be whether there's going to be some place in GDB that
calls 'ours_for_output ()' and assumes Ctrl-C / Ctrl-Z will be
automatically passed to the inferior.  This change means they are now
passed to GDB instead, will GDB always forward these to the inferior
correctly?

Thanks,
Andrew



--

diff --git a/gdb/inflow.c b/gdb/inflow.c
index 339b55c0bc6..6ed22c14b6b 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -506,10 +506,11 @@ child_terminal_ours_1 (target_terminal_state desired_state)
       /* Set tty state to our_ttystate.  */
       serial_set_tty_state (stdin_serial, our_terminal_info.ttystate);
 
-      /* If we only want output, then leave the inferior's pgrp in the
-	 foreground, so that Ctrl-C/Ctrl-Z reach the inferior
-	 directly.  */
-      if (job_control && desired_state == target_terminal_state::is_ours)
+      /* If might be tempting to think that we can leave the inferior's
+	 pgrp in the foreground if we only want ours_for_output, however,
+	 calls to tcdrain within GDB will result in SIGTTOU unless GDB's
+	 process group is in the foreground.  */
+      if (job_control)
 	{
 #ifdef HAVE_TERMIOS_H
 	  result = tcsetpgrp (0, our_terminal_info.process_group);
@@ -526,7 +527,7 @@ child_terminal_ours_1 (target_terminal_state desired_state)
 #endif /* termios */
 	}
 
-      if (!job_control && desired_state == target_terminal_state::is_ours)
+      if (!job_control)
 	{
 	  signal (SIGINT, sigint_ours);
 #ifdef SIGQUIT