From 6f64ef53c0a77b34717741d4a5453760ff2a2cab Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 26 Jul 2013 11:15:45 +0000 Subject: [PATCH] GDB kills itself instead of interrupting inferior When GDB is run with IO redirected to a pipe, the 'interrupt' command causes it to kill its own process group instead of the inferior's. The problem manifests itself in async mode, native debugging: $ cat | gdb (gdb) set target-async on (gdb) run & (gdb) interrupt A debugging session is active. Inferior 1 [process 20584] will be killed. Quit anyway? (y or n) [answered Y; input not from terminal] In this case, GDB tells that its stdin isn't a tty and doesn't save the inferior's process group in inflow.c:terminal_init_inferior_with_pgrp. The 'interrupt' command tries to 'kill' the inferior's process group in `inf-ptrace.c:inf_ptrace_stop`, but since that wasn't saved in the first place, GDB kills process group 0, meaning, its own process group. When GDB is used from a frontend, that means killing its own process group including the frontend and possibly the X session. This was originally seen with SublimeGDB: https://github.com/quarnster/SublimeGDB/issues/29. The patch makes GDB save the inferior pgid regardless of having a terminal, as pgid is used not only to reset foreground process group, but also to interrupt the inferior process. It also adds a regression test. Luckily, we can emulate not having a terminal with "set interactive-mode off", avoiding the need of special magic to spawn gdb with a pipe. Tested on x86_64 Fedora 17. gdb/ 2013-07-26 Cyril Nikolaev * inflow.c (terminal_init_inferior_with_pgrp): Save inferior process group regardless of having tty on stdin. gdb/testsuite/ 2013-07-26 Pedro Alves * gdb.base/interrupt-noterm.c, gdb.base/interrupt-noterm.exp: New files. --- gdb/ChangeLog | 5 ++ gdb/inflow.c | 17 +++-- gdb/testsuite/ChangeLog | 5 ++ gdb/testsuite/gdb.base/interrupt-noterm.c | 25 +++++++ gdb/testsuite/gdb.base/interrupt-noterm.exp | 76 +++++++++++++++++++++ 5 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 gdb/testsuite/gdb.base/interrupt-noterm.c create mode 100644 gdb/testsuite/gdb.base/interrupt-noterm.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 6cfa771cad..671a11c48d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2013-07-26 Cyril Nikolaev + + * inflow.c (terminal_init_inferior_with_pgrp): Save inferior + process group regardless of having tty on stdin. + 2013-07-25 Doug Evans * linux-fork.h (detach_fork): Delete. diff --git a/gdb/inflow.c b/gdb/inflow.c index 79a99d1315..faf4888214 100644 --- a/gdb/inflow.c +++ b/gdb/inflow.c @@ -217,19 +217,22 @@ static void terminal_ours_1 (int); void terminal_init_inferior_with_pgrp (int pgrp) { + struct inferior *inf = current_inferior (); + struct terminal_info *tinfo = get_inflow_inferior_data (inf); + +#ifdef PROCESS_GROUP_TYPE + /* Store the process group even without a terminal as it is used not + only to reset the tty foreground process group, but also to + interrupt the inferior. */ + tinfo->process_group = pgrp; +#endif + if (gdb_has_a_terminal ()) { - struct inferior *inf = current_inferior (); - struct terminal_info *tinfo = get_inflow_inferior_data (inf); - xfree (tinfo->ttystate); tinfo->ttystate = serial_copy_tty_state (stdin_serial, our_terminal_info.ttystate); -#ifdef PROCESS_GROUP_TYPE - tinfo->process_group = pgrp; -#endif - /* Make sure that next time we call terminal_inferior (which will be before the program runs, as it needs to be), we install the new process group. */ diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index fdb6c3d015..ed1337aa0d 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2013-07-26 Pedro Alves + + * gdb.base/interrupt-noterm.c, gdb.base/interrupt-noterm.exp: New + files. + 2013-07-25 Andrew Burgess * gdb.mi/mi-reg-undefined.exp: New file. diff --git a/gdb/testsuite/gdb.base/interrupt-noterm.c b/gdb/testsuite/gdb.base/interrupt-noterm.c new file mode 100644 index 0000000000..264b0b0242 --- /dev/null +++ b/gdb/testsuite/gdb.base/interrupt-noterm.c @@ -0,0 +1,25 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2013 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +int +main () +{ + sleep (3); + return 0; +} diff --git a/gdb/testsuite/gdb.base/interrupt-noterm.exp b/gdb/testsuite/gdb.base/interrupt-noterm.exp new file mode 100644 index 0000000000..42d17b1361 --- /dev/null +++ b/gdb/testsuite/gdb.base/interrupt-noterm.exp @@ -0,0 +1,76 @@ +# Copyright (C) 2013 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +standard_testfile + +if [prepare_for_testing "failed to prepare for testing" \ + ${testfile} ${srcfile} {debug}] { + return -1 +} + +# Pretend there's no terminal. +gdb_test_no_output "set interactive-mode off" +gdb_test_no_output "set target-async on" + +if ![runto main] { + fail "Can't run to main" + return -1 +} + +# Delete breakpoints so that the next resume is a plain continue, +# instead of a step-over-breakpoint sequence just while GDB sends the +# interrupt request. If that's buggy on some targets (and it was on +# target remote for a while, where a ctrl-c at the wrong time will get +# lost), then it should get its own specific test. Disable +# confirmation, avoiding complications caused by the fact that we've +# disabled the terminal -- GDB would auto-answer "yes", confusing +# gdb_test_multiple. +gdb_test_no_output "set confirm off" +gdb_test_no_output "delete" +gdb_test_no_output "set confirm on" + +set async_supported -1 +set test "continue &" +gdb_test_multiple $test $test { + -re "Continuing\\.\r\n$gdb_prompt $" { + set async_supported 1 + pass $test + } + -re ".*Asynchronous execution not supported on this target..*" { + unsupported $test + } +} +if { $async_supported < 0 } { + return 1 +} + +# With native debugging, and no terminal (emulated by interactive-mode +# off, above), GDB had a bug where "interrupt" would send SIGINT to +# its own process group, instead of the inferior's. +set test "interrupt" +gdb_test_multiple $test $test { + -re "interrupt\r\n$gdb_prompt " { + pass $test + } +} + +set test "inferior received SIGINT" +gdb_test_multiple "" $test { + -re "\r\nProgram received signal SIGINT.*" { + # This appears after the prompt, which was already consumed + # above. + pass $test + } +}