[RFA,1/3] Fix GDB being suspended SIGTTOU when running gdb.multi/multi-arch-exec.exp

  When running under valgrind, multi-arch-exec.exp blocks forever.
Some (painful) investigation shows this is due to valgrind slowing
down GDB, and GDB has to output some messages at a different time,
when GDB does not have the terminal for output.

To reproduce the problem, you need to slow down GDB.
It can be reproduced by:
cd gdb/testsuite/outputs/gdb.multi/multi-arch-exec/
../../../../gdb -ex 'set debug lin-lwp 1' -ex 'break all_started' -ex 'run' ./2-multi-arch-exec

The above stops at a breakpoint.  Do continue.
GDB is then suspended because of SIGTTOU.
The stacktrace that leads to the hanging GDB is:
(top-gdb) bt
    at ../../binutils-gdb/gdb/exceptions.c:130
....

Alternatively, the same happens when doing
strace -o s.out ../../../../gdb  -ex 'break all_started' -ex 'run' ./2-multi-arch-exec

And of course, valgrind is also sufficiently slowing down GDB to
reproduce this :).

Fix this by calling target_terminal::ours_for_output ();
at the beginning of follow_exec.

Note that all this terminal handling is not very clear to me:
  * Some code takes the terminal, and then takes care to give it back to the inferior
    if the terminal was belonging to the inferior.
    (e.g. annotate_breakpoints_invalid).
  * some code takes the terminal, but does not give it back
    (e.g. update_inserted_breakpoint_locations).
  * some code takes it, and unconditionally gives it back
    (e.g. handle_jit_event)
  * here and there, we also find
    gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
    before a (sometimes optional) call to ours_for_output.
    And such calls to ours_for_output is sometimes protected by:
       if (target_supports_terminal_ours ())
    (e.g. exceptions.c: print_flush).
    but most of the code calls it without checking if the target supports it.
  * some code is outputting some errors, but only takes the terminal
    after. E.g. infcmd.c: prepare_one_step

gdb/ChangeLog
2019-03-23  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* infrun.c (follow_exec): Call target_terminal::ours_for_output.
---
 gdb/infrun.c | 4 ++++
 1 file changed, 4 insertions(+)
  

On Sat, 23 Mar 2019 21:00:20 +0100
Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:

> When running under valgrind, multi-arch-exec.exp blocks forever.
> Some (painful) investigation shows this is due to valgrind slowing
> down GDB, and GDB has to output some messages at a different time,
> when GDB does not have the terminal for output.
> 
> To reproduce the problem, you need to slow down GDB.
> It can be reproduced by:
> cd gdb/testsuite/outputs/gdb.multi/multi-arch-exec/
> ../../../../gdb -ex 'set debug lin-lwp 1' -ex 'break all_started' -ex 'run' ./2-multi-arch-exec
> 
> The above stops at a breakpoint.  Do continue.
> GDB is then suspended because of SIGTTOU.
> The stacktrace that leads to the hanging GDB is:
> (top-gdb) bt
>     at ../../binutils-gdb/gdb/exceptions.c:130
> ....
> 
> Alternatively, the same happens when doing
> strace -o s.out ../../../../gdb  -ex 'break all_started' -ex 'run' ./2-multi-arch-exec
> 
> And of course, valgrind is also sufficiently slowing down GDB to
> reproduce this :).
> 
> Fix this by calling target_terminal::ours_for_output ();
> at the beginning of follow_exec.
> 
> Note that all this terminal handling is not very clear to me:
>   * Some code takes the terminal, and then takes care to give it back to the inferior
>     if the terminal was belonging to the inferior.
>     (e.g. annotate_breakpoints_invalid).
>   * some code takes the terminal, but does not give it back
>     (e.g. update_inserted_breakpoint_locations).
>   * some code takes it, and unconditionally gives it back
>     (e.g. handle_jit_event)
>   * here and there, we also find
>     gdb::optional<target_terminal::scoped_restore_terminal_state> term_state;
>     before a (sometimes optional) call to ours_for_output.
>     And such calls to ours_for_output is sometimes protected by:
>        if (target_supports_terminal_ours ())
>     (e.g. exceptions.c: print_flush).
>     but most of the code calls it without checking if the target supports it.
>   * some code is outputting some errors, but only takes the terminal
>     after. E.g. infcmd.c: prepare_one_step
> 
> gdb/ChangeLog
> 2019-03-23  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* infrun.c (follow_exec): Call target_terminal::ours_for_output.

This sounds reasonable to me...

But wait a few days until pushing it to give time for feedback
from other maintainers.

Kevin
  

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Note that all this terminal handling is not very clear to me:
[...]

Me too.  For example I wonder when we would ever not want to acquire the
terminal for output -- i.e., maybe the lowest-level fputs-to-terminal
call should just ensure that gdb has the terminal for output.

The patch looks fine to me though.

Tom
  

On Sun, 2019-03-24 at 15:56 -0700, Kevin Buettner wrote:
> > gdb/ChangeLog
> > 2019-03-23  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> > 	* infrun.c (follow_exec): Call target_terminal::ours_for_output.
> 
> This sounds reasonable to me...
> 
> But wait a few days until pushing it to give time for feedback
> from other maintainers.
Kevin, Tom,
Thanks for the review.
I have pushed the series.

Philippe