[3/3] Avoid GDB SIGTTOU on catch exec + set follow-exec-mode new (PR 23368)
Commit Message
On 2018-10-22 4:56 p.m., Pedro Alves wrote:
> On 10/20/2018 04:38 PM, Simon Marchi wrote:
>> On 2018-10-20 11:13 a.m., Simon Marchi wrote:
>>> On 2018-10-17 1:04 p.m., Pedro Alves wrote:
>>>> I think the "for both inferiors" part is what's dubious here.
>>>>
>>>> The process lives on in the new inferior, but we lost its
>>>> terminal settings! Seems to me that they should be migrated
>>>> from the old inferior to the new one. And then the problem
>>>> sorts itself out, because then the new inferior will have
>>>> target_terminal_state::is_inferior state.
>>>
>>> Yeah that makes sense. This crossed my mind when I look at the issue,
>>> but for some reason I didn't went that route. But now that you say it,
>>> it appears obvious that this should be the fix.
>>>
>>> I just tried copying the inferior::terminal_state value from the old
>>> inferior to the new inferior, and it fixes the problem. But
>>> actually, we should also be transferring all of the struct terminal_info
>>> associated to the inferior, is that right?
>>
>> This would be the updated patch (testing on the buildbot at the moment).
>
> Yes. Looks good. I think you could avoid the
> copying-with-garanteed-deleting by adding a "swap_terminal_info" function
> that swaps the terminal_info and terminal_state between the inferiors.
Like this? (commit message and ChangeLog to be updated)
From a43edf2aada8d5312370429a4d7fad7cfb1ec85c Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sat, 20 Oct 2018 10:57:33 -0400
Subject: [PATCH] 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:
* infrun.c (follow_exec): In the follow_exec_mode_new case,
transfer terminal state from old new new inferior.
---
gdb/inflow.c | 16 ++++++++++++++++
gdb/infrun.c | 9 +++++----
gdb/terminal.h | 3 +++
3 files changed, 24 insertions(+), 4 deletions(-)
--
2.19.1
Comments
On 10/23/2018 10:48 AM, Simon Marchi wrote:
> On 2018-10-22 4:56 p.m., Pedro Alves wrote:
>> Yes. Looks good. I think you could avoid the
>> copying-with-garanteed-deleting by adding a "swap_terminal_info" function
>> that swaps the terminal_info and terminal_state between the inferiors.
>
> Like this? (commit message and ChangeLog to be updated)
>
Yes, like that.
Thanks,
Pedro Alves
That's still broken:
$ ./gdb gdb
GNU gdb (GDB) 8.2.50.20181024-git
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "ia64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from gdb...
Setting up the environment for debugging gdb.
During symbol reading: unsupported tag: 'DW_TAG_unspecified_type'
Breakpoint 1 at 0x400000000018fc00: file ../../gdb/common/errors.c, line 51.
During symbol reading: Member function "~_Sp_counted_base" (offset 0x517e90) is virtual but the vtable offset is not specified
During symbol reading: cannot get low and high bounds for subprogram DIE at 0x5201ee
During symbol reading: missing name for subprogram DIE at 0x5246c8
During symbol reading: Child DIE 0x52f12a and its abstract origin 0x52cafc have different parents
During symbol reading: Multiple children of DIE 0x53138e refer to DIE 0x52be3c as their abstract origin
During symbol reading: DW_AT_call_target target DIE has invalid low pc, for referencing DIE 0x53a7e0 [in module /usr/local/gcc/gdb/Build/gdb/gdb]
Breakpoint 2 at 0x4000000000150ac0: file ../../gdb/cli/cli-cmds.c, line 197.
(top-gdb) r
Starting program: /usr/local/gcc/gdb/Build/gdb/gdb
During symbol reading: .debug_line section has line program sequence without an end
[1]+ Stopped ./gdb gdb
$ fg
./gdb gdb
Failed to read a valid object file image from memory.
During symbol reading: .debug_line section has line program sequence without an end
During symbol reading: .debug_line section has line program sequence without an end
[Detaching after vfork from child process 11772]
GNU gdb (GDB) 8.2.50.20181024-git
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "ia64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Andreas.
On 2018-10-24 05:41, Andreas Schwab wrote:
> That's still broken:
>
> $ ./gdb gdb
> GNU gdb (GDB) 8.2.50.20181024-git
> Copyright (C) 2018 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "ia64-unknown-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
> <http://www.gnu.org/software/gdb/documentation/>.
>
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from gdb...
> Setting up the environment for debugging gdb.
> During symbol reading: unsupported tag: 'DW_TAG_unspecified_type'
> Breakpoint 1 at 0x400000000018fc00: file ../../gdb/common/errors.c,
> line 51.
> During symbol reading: Member function "~_Sp_counted_base" (offset
> 0x517e90) is virtual but the vtable offset is not specified
> During symbol reading: cannot get low and high bounds for subprogram
> DIE at 0x5201ee
> During symbol reading: missing name for subprogram DIE at 0x5246c8
> During symbol reading: Child DIE 0x52f12a and its abstract origin
> 0x52cafc have different parents
> During symbol reading: Multiple children of DIE 0x53138e refer to DIE
> 0x52be3c as their abstract origin
> During symbol reading: DW_AT_call_target target DIE has invalid low
> pc, for referencing DIE 0x53a7e0 [in module
> /usr/local/gcc/gdb/Build/gdb/gdb]
> Breakpoint 2 at 0x4000000000150ac0: file ../../gdb/cli/cli-cmds.c, line
> 197.
> (top-gdb) r
> Starting program: /usr/local/gcc/gdb/Build/gdb/gdb
> During symbol reading: .debug_line section has line program sequence
> without an end
>
> [1]+ Stopped ./gdb gdb
> $ fg
> ./gdb gdb
> Failed to read a valid object file image from memory.
> During symbol reading: .debug_line section has line program sequence
> without an end
> During symbol reading: .debug_line section has line program sequence
> without an end
> [Detaching after vfork from child process 11772]
> GNU gdb (GDB) 8.2.50.20181024-git
> Copyright (C) 2018 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "ia64-unknown-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
> <http://www.gnu.org/software/gdb/documentation/>.
>
> For help, type "help".
> Type "apropos word" to search for commands related to "word".
>
> Andreas.
That looks like a different case of SIGTTOU, same symptom but different
root cause. I didn't claim I fixed all of them :)
Simon
@@ -700,6 +700,22 @@ copy_terminal_info (struct inferior *to, struct inferior *from)
to->terminal_state = from->terminal_state;
}
+/* See terminal.h. */
+
+void
+swap_terminal_info (inferior *a, inferior *b)
+{
+ terminal_info *info_a
+ = (struct terminal_info *) inferior_data (a, inflow_inferior_data);
+ terminal_info *info_b
+ = (struct terminal_info *) inferior_data (a, inflow_inferior_data);
+
+ set_inferior_data (a, inflow_inferior_data, info_b);
+ set_inferior_data (b, inflow_inferior_data, info_a);
+
+ std::swap (a->terminal_state, b->terminal_state);
+}
+
void
info_terminal_command (const char *arg, int from_tty)
{
@@ -1189,12 +1189,13 @@ follow_exec (ptid_t ptid, char *exec_file_target)
/* The user wants to keep the old inferior and program spaces
around. Create a new fresh one, and switch to it. */
- /* Do exit processing for the original inferior before adding
- the new inferior so we don't have two active inferiors with
- the same ptid, which can confuse find_inferior_ptid. */
+ /* Do exit processing for the original inferior before setting the new
+ inferior's pid. Having two inferiors with the same pid would confuse
+ find_inferior_p(t)id. */
+ inf = add_inferior_with_spaces ();
+ swap_terminal_info (inf, current_inferior ());
exit_inferior_silent (current_inferior ());
- inf = add_inferior_with_spaces ();
inf->pid = pid;
target_follow_exec (inf, exec_file_target);
@@ -29,6 +29,9 @@ extern void new_tty_postfork (void);
extern void copy_terminal_info (struct inferior *to, struct inferior *from);
+/* Exchange the terminal info and state between inferiors A and B. */
+extern void swap_terminal_info (inferior *a, inferior *b);
+
extern pid_t create_tty_session (void);
/* Set up a serial structure describing standard input. In inflow.c. */