[Bug breakpoints/23368] New: gdb goes to into background when hitting exec catchpoint

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

Bug ID: 23368
Summary: gdb goes to into background when hitting exec
catchpoint
Product: gdb
Version: HEAD
Status: NEW
Severity: normal
Priority: P2
Component: breakpoints
Assignee: unassigned at sourceware dot org
Reporter: vries at gcc dot gnu.org
Target Milestone: ---

Consider test executable t from PR 23366.

When hitting an exec catchpoint with follow-exec-mode new mode, gdb goes to the
background:
...
$ ./install/bin/gdb -q t -ex "catch exec" -ex "set follow-exec-mode new" -ex
"run"
Reading symbols from t...done.
Catchpoint 1 (exec)
Starting program: /data/gdb_versions/devel/t
process 27010 is executing new program: /usr/bin/ls
[New inferior 2 (process 0)]
[New process 27010]

Thread 2.1 "ls" hit Catchpoint 1 (exec'd /usr/bin/ls), 0x00007ffff7dd7ea0 in
_start ()
from /lib64/ld-linux-x86-64.so.2

[3]+ Stopped ./install/bin/gdb -q t -ex "catch exec" -ex "set
follow-exec-mode new" -ex "run"
$ fg
./install/bin/gdb -q t -ex "catch exec" -ex "set follow-exec-mode new" -ex
"run"
(gdb) q
A debugging session is active.

Inferior 2 [process 27010] will be killed.

Quit anyway? (y or n) y
$
...

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

Tom de Vries <vries at gcc dot gnu.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|gdb goes to into background |gdb goes to into background
|when hitting exec |when hitting exec
|catchpoint |catchpoint with
| |follow-exec-mode new

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

Tom de Vries <vries at gcc dot gnu.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |DUPLICATE

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Regression introduced by same commit as mentioned in PR23020 - "GDB gets
stopped by SIGTTOU when breakpoint insertion in fork child fails".

Marking duplicate.

*** This bug has been marked as a duplicate of bug 23020 ***

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

Simon Marchi <simon.marchi at ericsson dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
CC| |simon.marchi at ericsson dot com
Resolution|DUPLICATE |---

--- Comment #2 from Simon Marchi <simon.marchi at ericsson dot com> ---
As stated in

https://sourceware.org/bugzilla/show_bug.cgi?id=23020#c2

I'm not sure they are really duplicate, I suspect the root cause is different.

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

--- Comment #3 from Simon Marchi <simon.marchi at ericsson dot com> ---
Here's what I think is happening:

When the exec happen, follow_exec calls exit_inferior_silent to clean up the
first inferior (the execution will continue as inferior 2). The inferior_exit
observer calls inflow_inferior_exit. This observer sets the inferior's
terminal_state to is_ours:

inf->terminal_state = target_terminal_state::is_ours;

At this point, inferior1's terminal_state is is_ours, while the "reality",
tracked by gdb_tty_state, is is_inferior.

Later, when we call target_terminal::ours to make sure we own the terminal, we
don't actually go to the target backend to change it, because the core thinks
that inferior doesn't own the terminal (inf->terminal_state in
target_terminal_is_ours_kind). When something in readline tries to mess with
the terminal settings, it generates a SIGTTOU.

This patch makes it work for me, but I don't know if it's right. It calls
target_terminal::ours() instead of just overwriting the terminal state value in
the inferior.


From e9e867e11861e632b1517d06af9263f4499d4ba9 Mon Sep 17 00:00:00 2001
From: Simon Marchi <***@ericsson.com>
Date: Mon, 23 Jul 2018 15:55:34 -0400
Subject: [PATCH] patch

---
gdb/inflow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/inflow.c b/gdb/inflow.c
index caff646..fb2e490 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -666,7 +666,7 @@ inflow_inferior_exit (struct inferior *inf)
{
struct terminal_info *info;

- inf->terminal_state = target_terminal_state::is_ours;
+ target_terminal::ours ();

info = (struct terminal_info *) inferior_data (inf, inflow_inferior_data);
if (info != NULL)

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

--- Comment #4 from Simon Marchi <simon.marchi at ericsson dot com> ---
Note that this fix is probably not good in non-stop/multi-inferior scenario.
It's not because a single inferior exits that we want to change everything for
ours... more thinking is needed.

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Simon Marchi <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=35ed81d4f45855b98ea0c517b396662c3ae2a8c5

commit 35ed81d4f45855b98ea0c517b396662c3ae2a8c5
Author: Simon Marchi <***@polymtl.ca>
Date: Tue Oct 23 17:00:33 2018 -0400

Avoid GDB SIGTTOU on catch exec + set follow-exec-mode new (PR 23368)

Here's a summary of PR 23368:

#include <unistd.h>
int main (void)
{
char *exec_args[] = { "/bin/ls", NULL };
execve (exec_args[0], exec_args, NULL);
}

$ gdb -nx t -ex "catch exec" -ex "set follow-exec-mode new" -ex run
...
[1] + 13146 suspended (tty output) gdb -q -nx t -ex "catch exec" -ex "set
follow-exec-mode new" -ex run
$

Here's what happens: when the inferior execs with "follow-exec-mode
new", we first "mourn" it before creating the new one. This ends up
calling inflow_inferior_exit, which sets the per-inferior terminal state
to "is_ours":

inf->terminal_state = target_terminal_state::is_ours;

At this point, the inferior's terminal_state is is_ours, while the
"reality", tracked by gdb_tty_state, is is_inferior (GDB doesn't own the
terminal).

Later, we continue processing the exec inferior event and decide we want
to stop (because of the "catch exec") and call target_terminal::ours to
make sure we own the terminal. However, we don't actually go to the
target backend to change the settings, because the core thinks that no
inferior owns the terminal (inf->terminal_state is
target_terminal_state::is_ours, as checked in
target_terminal_is_ours_kind, for both inferiors). When something in
readline tries to mess with the terminal settings, it generates a
SIGTTOU.

This patch fixes this by tranferring the state of the terminal from the
old inferior to the new inferior.

gdb/ChangeLog:

PR gdb/23368
* infrun.c (follow_exec): In the follow_exec_mode_new case,
transfer terminal state from old new new inferior.
* terminal.h (swap_terminal_info): New function.
* inflow.c (swap_terminal_info): New function.

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

--- Comment #6 from Simon Marchi <simon.marchi at ericsson dot com> ---
This should be fixed by commit mentioned above.

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23368

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a76dc3b7705c5c85efc00521ebfac49e80c3fedf

commit a76dc3b7705c5c85efc00521ebfac49e80c3fedf
Author: Tom de Vries <***@suse.de>
Date: Tue Oct 23 23:54:21 2018 +0200

[gdb/testsuite] Rewrite catch-follow-exec.exp using gdb_test

The testcase catch-follow-exec.exp is written use gdb -batch in order to
avoid
a GDB SIGTTOU. After the commit of "Avoid GDB SIGTTOU on catch exec + set
follow-exec-mode new (PR 23368)", that no longer is necessary.

Rewrite the test using regular gdb_test commands.

Tested with x86_64-linux.

2018-10-24 Tom de Vries <***@suse.de>

* gdb.base/catch-follow-exec.exp: Rewrite using gdb_test.