PR gdb/17472: With annotations, input while executing in the foreground crashes readline/GDB

Jan caught an intermittent GDB crash with the annota1.exp test:

 Starting program: .../gdb/testsuite/gdb.base/annota1 ^M
 [...]
 FAIL: gdb.base/annota1.exp: run until main breakpoint (timeout)
 [...]
 readline: readline_callback_read_char() called with no handler!^M
 ERROR: Process no longer exists

All we need to is to continue the inferior in the foreground, and type
a command while the inferior is running.  E.g.:

 (gdb) set annotate 2

 ▒▒pre-prompt
 (gdb)
 ▒▒prompt
 c

 ▒▒post-prompt
 Continuing.

 ▒▒starting

 ▒▒frames-invalid

 *inferior is running now*

 p 1<ret>

 readline: readline_callback_read_char() called with no handler!
 Aborted (core dumped)
 $


When we run a foreground execution command we call
target_terminal_inferior to stop GDB from processing input, and to put
the inferior's terminal settings in effect.  Then we tell readline to
hide the prompt with display_gdb_prompt, which clears readline's input
callback too.  When the target stops, we call target_terminal_ours,
which re-installs stdin in the event loop, and then we redisplay the
prompt, reinstalling the readline callbacks.

However, when annotations are in effect, the "frames-invalid"
annotation code calls target_terminal_ours after 'resume' had already
called target_terminal_inferior:

 (top-gdb) bt
 #0  0x000000000056b82f in annotate_frames_invalid () at gdb/annotate.c:219
 #1  0x000000000072e6cc in reinit_frame_cache () at gdb/frame.c:1705
 #2  0x0000000000594bb9 in registers_changed_ptid (ptid=...) at gdb/regcache.c:612
 #3  0x000000000064cca1 in target_resume (ptid=..., step=1, signal=GDB_SIGNAL_0) at gdb/target.c:2136
 #4  0x00000000005f57af in resume (step=1, sig=GDB_SIGNAL_0) at gdb/infrun.c:2263
 #5  0x00000000005f6051 in proceed (addr=18446744073709551615, siggnal=GDB_SIGNAL_DEFAULT, step=1) at gdb/infrun.c:2613

And then once we hide the prompt and remove readline's input handler
callback, we're in a bad state.  We end up with the target running
supposedly in the foreground, but with stdin still installed on the
event loop.  Any input then calls into readline, which aborts because
no rl_linefunc callback handler is installed:

 Program received signal SIGABRT, Aborted.
 0x0000003b36a35877 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 56        return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);

 (top-gdb) bt
 #0  0x0000003b36a35877 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 #1  0x0000003b36a36f68 in __GI_abort () at abort.c:89
 During symbol reading, debug info gives source 9 included from file at zero line 0.
 During symbol reading, debug info gives command-line macro definition with non-zero line 19: _STDC_PREDEF_H 1.
 #2  0x0000000000784a25 in rl_callback_read_char () at src/readline/callback.c:116
 #3  0x0000000000619111 in rl_callback_read_char_wrapper (client_data=0x0) at src/gdb/event-top.c:167
 #4  0x00000000006194e7 in stdin_event_handler (error=0, client_data=0x0) at src/gdb/event-top.c:373
 #5  0x00000000006180da in handle_file_event (data=...) at src/gdb/event-loop.c:763
 #6  0x00000000006175c1 in process_event () at src/gdb/event-loop.c:340
 #7  0x0000000000617688 in gdb_do_one_event () at src/gdb/event-loop.c:404
 #8  0x00000000006176d8 in start_event_loop () at src/gdb/event-loop.c:429
 #9  0x0000000000619143 in cli_command_loop (data=0x0) at src/gdb/event-top.c:182
 #10 0x000000000060f4c8 in current_interp_command_loop () at src/gdb/interps.c:318
 #11 0x0000000000610691 in captured_command_loop (data=0x0) at src/gdb/main.c:323
 #12 0x000000000060c385 in catch_errors (func=0x610676 <captured_command_loop>, func_args=0x0, errstring=0x900241 "", mask=RETURN_MASK_ALL)
     at src/gdb/exceptions.c:237
 #13 0x0000000000611b8f in captured_main (data=0x7fffffffd7b0) at src/gdb/main.c:1151
 #14 0x000000000060c385 in catch_errors (func=0x610a8e <captured_main>, func_args=0x7fffffffd7b0, errstring=0x900241 "", mask=RETURN_MASK_ALL)
     at src/gdb/exceptions.c:237
 #15 0x0000000000611bb8 in gdb_main (args=0x7fffffffd7b0) at src/gdb/main.c:1159
 #16 0x000000000045ef57 in main (argc=3, argv=0x7fffffffd8b8) at src/gdb/gdb.c:32

The fix is to make the annotation code call target_terminal_inferior
again after printing, if the inferior's settings were in effect.

While at it, when we're doing output only, instead of
target_terminal_ours, we should call target_terminal_ours_for_output.
The latter doesn't actually remove stdin from the event loop, and also
leaves SIGINT forwarded to the target.

New test included.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-17  Pedro Alves  <palves@redhat.com>

	PR gdb/17472
	* annotate.c (annotate_breakpoints_invalid): Use
	target_terminal_our_for_output instead of target_terminal_ours.
	Give back the terminal to the target.
	(annotate_frames_invalid): Likewise.

gdb/testsuite/
2014-10-17  Pedro Alves  <palves@redhat.com>

	PR gdb/17472
	* gdb.base/annota-input-while-running.c: New file.
	* gdb.base/annota-input-while-running.exp: New file.

7 files changed, 202 insertions, 2 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ad465b08d9f..acad8ee9fac 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2014-10-17  Pedro Alves  <palves@redhat.com>
 
+	PR gdb/17472
+	* annotate.c (annotate_breakpoints_invalid): Use
+	target_terminal_our_for_output instead of target_terminal_ours.
+	Give back the terminal to the target.
+	(annotate_frames_invalid): Likewise.
+
+2014-10-17  Pedro Alves  <palves@redhat.com>
+
 	* target.c (enum terminal_state): New enum.
 	(terminal_state): New global.
 	(target_terminal_init): New function.
diff --git a/gdb/annotate.c b/gdb/annotate.c
index 6cce693385d..97e2b2ba8ac 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -72,8 +72,17 @@ annotate_breakpoints_invalid (void)
       && (!breakpoints_invalid_emitted
 	  || async_background_execution_p ()))
     {
-      target_terminal_ours ();
+      /* If the inferior owns the terminal (e.g., we're resuming),
+	 make sure to leave with the inferior still owning it.  */
+      int was_inferior = target_terminal_is_inferior ();
+
+      target_terminal_ours_for_output ();
+
       printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
+
+      if (was_inferior)
+	target_terminal_inferior ();
+
       breakpoints_invalid_emitted = 1;
     }
 }
@@ -210,8 +219,17 @@ annotate_frames_invalid (void)
       && (!frames_invalid_emitted
 	  || async_background_execution_p ()))
     {
-      target_terminal_ours ();
+      /* If the inferior owns the terminal (e.g., we're resuming),
+	 make sure to leave with the inferior still owning it.  */
+      int was_inferior = target_terminal_is_inferior ();
+
+      target_terminal_ours_for_output ();
+
       printf_unfiltered (("\n\032\032frames-invalid\n"));
+
+      if (was_inferior)
+	target_terminal_inferior ();
+
       frames_invalid_emitted = 1;
     }
 }
diff --git a/gdb/target.c b/gdb/target.c
index d692664f116..e219f9a29b9 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -511,6 +511,14 @@ target_terminal_init (void)
 
 /* See target.h.  */
 
+int
+target_terminal_is_inferior (void)
+{
+  return (terminal_state == terminal_is_inferior);
+}
+
+/* See target.h.  */
+
 void
 target_terminal_inferior (void)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 71dce2420ff..c88da4987eb 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1356,6 +1356,11 @@ extern int target_insert_breakpoint (struct gdbarch *gdbarch,
 extern int target_remove_breakpoint (struct gdbarch *gdbarch,
 				     struct bp_target_info *bp_tgt);
 
+/* Returns true if the terminal settings of the inferior are in
+   effect.  */
+
+extern int target_terminal_is_inferior (void);
+
 /* Initialize the terminal settings we record for the inferior,
    before we actually run the inferior.  */
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3d24531cd19..a0f0b93d66e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-17  Pedro Alves  <palves@redhat.com>
+
+	PR gdb/17472
+	* gdb.base/annota-input-while-running.c: New file.
+	* gdb.base/annota-input-while-running.exp: New file.
+
 2014-09-11  Pedro Alves  <palves@redhat.com>
 
 	PR gdb/17347
diff --git a/gdb/testsuite/gdb.base/annota-input-while-running.c b/gdb/testsuite/gdb.base/annota-input-while-running.c
new file mode 100644
index 00000000000..3db1399d3e0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/annota-input-while-running.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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 <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+int
+main (void)
+{
+  sleep (5);
+  return 0; /* set break here */
+}
diff --git a/gdb/testsuite/gdb.base/annota-input-while-running.exp b/gdb/testsuite/gdb.base/annota-input-while-running.exp
new file mode 100644
index 00000000000..137571871ca
--- /dev/null
+++ b/gdb/testsuite/gdb.base/annota-input-while-running.exp
@@ -0,0 +1,130 @@
+# Copyright 1999-2014 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 <http://www.gnu.org/licenses/>.
+
+# Test that annotations support doesn't leave GDB's terminal settings
+# into effect when we run a foreground command.
+
+if [is_remote target] then {
+    # We cannot use runto_main because of the different prompt we get
+    # when using annotation level 2.
+    return 0
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Break at main
+
+gdb_test "break main" \
+    "Breakpoint.*at.* file .*$srcfile.*\\." \
+    "breakpoint main"
+
+# NOTE: this prompt is OK only when the annotation level is > 1
+# NOTE: When this prompt is in use the gdb_test procedure cannot be
+# used because it assumes that the last char after the gdb_prompt is a
+# white space.  This is not true with this annotated prompt.  So we
+# must use the gdb_annota_test replacement below, or
+# gdb_test_multiple.
+
+set old_gdb_prompt $gdb_prompt
+set gdb_prompt "\r\n\032\032pre-prompt\r\n$gdb_prompt \r\n\032\032prompt\r\n"
+
+# Like gdb_test, but cope with the annotation prompt.
+proc gdb_annota_test {command pattern message} {
+    global gdb_prompt
+
+    gdb_test_multiple $command $message {
+	-re "$pattern$gdb_prompt$" {
+	    pass "$message"
+	}
+	-re "$gdb_prompt$" {
+	    fail "$message"
+	}
+    }
+}
+
+# Set the annotation level to 2.
+
+set test "annotation set at level 2"
+gdb_annota_test "set annotate 2" ".*" "annotation set at level 2"
+
+# Run to main.
+
+gdb_annota_test "run" \
+    "\r\n\032\032post-prompt.*\r\n\r\n\032\032stopped.*" \
+    "run until main breakpoint"
+
+set test "delete breakpoints"
+gdb_test_multiple "delete" $test {
+    -re "Delete all breakpoints. .y or n." {
+	send_gdb "y\n"
+	exp_continue
+    }
+    -re "$gdb_prompt$" {
+	pass $test
+    }
+}
+
+# Set the target running, and then type something.  GDB used to have a
+# bug where it'd be accepting input even though the target was
+# supposedly resumed in the foreground.  This ultimately resulted in
+# readline aborting.
+
+set linenum [gdb_get_line_number "set break here"]
+
+gdb_annota_test "break $linenum" \
+    "Breakpoint .*$srcfile, line .*" \
+    "break after sleep"
+
+# Continue, and wait a bit to make sure the inferior really starts
+# running.  Wait less than much the program sleeps, which is 5
+# seconds, though.
+set saw_continuing 0
+set test "continue"
+gdb_test_multiple $test $test {
+    -timeout 2
+    -re "Continuing\\." {
+	set saw_continuing 1
+	exp_continue
+    }
+    timeout {
+	gdb_assert $saw_continuing $test
+    }
+}
+
+# Type something.
+send_gdb "print 1\n"
+
+# Poor buggy GDB would crash before the breakpoint was hit.
+set test "breakpoint hit"
+gdb_test_multiple "" $test {
+    -re "stopped\r\n$gdb_prompt" {
+	pass $test
+    }
+}
+
+set test "print command result"
+gdb_test_multiple "" $test {
+    -re "\r\n1\r\n\r\n\032\032value-history-end\r\n$gdb_prompt" {
+	pass $test
+    }
+}
+
+# Restore the original prompt for the rest of the testsuite.
+
+set gdb_prompt $old_gdb_prompt