public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RESEND/PROPER PATCH 0/9] pagination/readline/async issues
@ 2014-07-03 15:19 Pedro Alves
  2014-07-03 15:19 ` [PATCH 1/9] Put the inferior's terminal settings in effect while running (fg) infcalls Pedro Alves
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

This series fixes PR gdb/17072 (a 7.8 blocker) and other related
issues I stumbled on while fixing it and writing tests.

Some of these fixes are conservative as this is intended for the 7.8
branch as well.

Tested on x86_64 Fedora 20, native and gdbserver.

Comments?

Pedro Alves (9):
  Put the inferior's terminal settings in effect while running (fg)
    infcalls
  Eliminate exceptions.c:print_any_exception.
  Move catch_command_errors and catch_command_errors_const to main.c
  testsuite: Introduce gdb_assert
  Canceling pagination caused by execution command from command line
    aborts readline/gdb
  Background execution + pagination aborts readline/gdb
  Remove the target from the event loop while in secondary prompts
  Fix double prompt
  Put GDB's terminal settings into effect when paginating.

 gdb/exceptions.c                                   |  54 +------
 gdb/exceptions.h                                   |  13 --
 gdb/inf-loop.c                                     |  36 ++---
 gdb/infcall.c                                      |   7 +
 gdb/main.c                                         |  58 +++++++
 .../gdb.base/double-prompt-target-event-error.c    |  23 +++
 .../gdb.base/double-prompt-target-event-error.exp  | 108 +++++++++++++
 gdb/testsuite/gdb.base/execution-termios.c         |  35 +++++
 gdb/testsuite/gdb.base/execution-termios.exp       |  60 +++++++
 .../gdb.base/paginate-after-ctrl-c-running.c       |  23 +++
 .../gdb.base/paginate-after-ctrl-c-running.exp     |  80 ++++++++++
 gdb/testsuite/gdb.base/paginate-bg-execution.c     |  30 ++++
 gdb/testsuite/gdb.base/paginate-bg-execution.exp   | 121 ++++++++++++++
 .../gdb.base/paginate-execution-startup.c          |  30 ++++
 .../gdb.base/paginate-execution-startup.exp        | 175 +++++++++++++++++++++
 gdb/testsuite/gdb.base/paginate-inferior-exit.c    |  30 ++++
 gdb/testsuite/gdb.base/paginate-inferior-exit.exp  |  82 ++++++++++
 gdb/testsuite/gdb.trace/backtrace.exp              |   7 +-
 gdb/testsuite/lib/gdb.exp                          |  82 ++++++++--
 gdb/top.c                                          |  21 ++-
 gdb/utils.c                                        |   4 +
 21 files changed, 977 insertions(+), 102 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/double-prompt-target-event-error.c
 create mode 100644 gdb/testsuite/gdb.base/double-prompt-target-event-error.exp
 create mode 100644 gdb/testsuite/gdb.base/execution-termios.c
 create mode 100644 gdb/testsuite/gdb.base/execution-termios.exp
 create mode 100644 gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp
 create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.exp
 create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.exp
 create mode 100644 gdb/testsuite/gdb.base/paginate-inferior-exit.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-inferior-exit.exp

-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (5 preceding siblings ...)
  2014-07-03 15:19 ` [PATCH 8/9] Fix double prompt Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-07-04  6:11   ` Yao Qi
  2014-07-03 15:27 ` [PATCH 4/9] testsuite: Introduce gdb_assert Pedro Alves
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

This fixes:

 $ ./gdb program -ex "set height 2" -ex "start"
 ...
 Reading symbols from /home/pedro/gdb/tests/threads...done.
 ---Type <return> to continue, or q <return> to quit---^CQuit  << ctrl-c triggers a Quit

 *type something*
 readline: readline_callback_read_char() called with no handler!
 Aborted
 $

Usually, if an error propagates all the way to the top level, we'll
re-enable stdin, in case the command that was running was a
synchronous command.  That's done in the event loop's actual loop
(event-loop.c:start_event_loop).  However, if a foreground execution
command is run before the event loop starts and throws, nothing is
presently reenabling stdin, which leaves sync_execution set.

When we do start the event loop, because sync_execution is still
(mistakenly) set, display_gdb_prompt removes the readline input
callback, even though stdin is registered in the event loop.  Any
input from here on results in readline aborting.

Such commands are run through catch_command_errors,
catch_command_errors_const, so add the tweak there.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* main.c: Include event-top.h.
	(handle_command_errors): New function.
	(catch_command_errors, catch_command_errors_const): Use it.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* gdb.base/paginate-execution-startup.c: New file.
	* gdb.base/paginate-execution-startup.exp: New file.
	* lib/gdb.exp (pagination_prompt): New global.
	(default_gdb_spawn): New procedure, factored out from
	default_gdb_spawn.
	(default_gdb_start): Adjust to call default_gdb_spawn.
	(gdb_spawn): New procedure.
---
 gdb/main.c                                         |  30 +++-
 .../gdb.base/paginate-execution-startup.c          |  30 ++++
 .../gdb.base/paginate-execution-startup.exp        | 175 +++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                          |  61 +++++--
 4 files changed, 276 insertions(+), 20 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.exp

diff --git a/gdb/main.c b/gdb/main.c
index 1d77bd3..b51ff89 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -46,6 +46,7 @@
 #include "filenames.h"
 #include "filestuff.h"
 #include <signal.h>
+#include "event-top.h"
 
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
@@ -337,6 +338,25 @@ captured_command_loop (void *data)
   return 1;
 }
 
+/* Handle command errors thrown from within
+   catch_command_errors/catch_command_errors_const.  */
+
+static int
+handle_command_errors (volatile struct gdb_exception e)
+{
+  if (e.reason < 0)
+    {
+      exception_print (gdb_stderr, e);
+
+      /* If any exception escaped to here, we better enable stdin.
+	 Otherwise, any command that calls async_disable_stdin, and
+	 then throws, will leave stdin inoperable.  */
+      async_enable_stdin ();
+      return 0;
+    }
+  return 1;
+}
+
 /* Type of the command callback passed to catch_command_errors.  */
 
 typedef void (catch_command_errors_ftype) (char *, int);
@@ -353,10 +373,7 @@ catch_command_errors (catch_command_errors_ftype *command,
     {
       command (arg, from_tty);
     }
-  exception_print (gdb_stderr, e);
-  if (e.reason < 0)
-    return 0;
-  return 1;
+  return handle_command_errors (e);
 }
 
 /* Type of the command callback passed to catch_command_errors_const.  */
@@ -375,10 +392,7 @@ catch_command_errors_const (catch_command_errors_const_ftype *command,
     {
       command (arg, from_tty);
     }
-  exception_print (gdb_stderr, e);
-  if (e.reason < 0)
-    return 0;
-  return 1;
+  return handle_command_errors (e);
 }
 
 /* Arguments of --command option and its counterpart.  */
diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.c b/gdb/testsuite/gdb.base/paginate-execution-startup.c
new file mode 100644
index 0000000..7457b18
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-execution-startup.c
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+static void
+after_sleep (void)
+{
+  return; /* after sleep */
+}
+
+int
+main (void)
+{
+  sleep (3);
+  after_sleep ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.exp b/gdb/testsuite/gdb.base/paginate-execution-startup.exp
new file mode 100644
index 0000000..77822fb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-execution-startup.exp
@@ -0,0 +1,175 @@
+# Copyright (C) 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/>.
+
+# A collection of tests related to pagination resulting from running
+# execution commands directly from the command line, with "-ex".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+
+# Returns true if the board can 'gdb -ex "start"', false otherwise.
+
+proc probe_can_run_cmdline  {} {
+    global srcfile binfile
+    global saved_gdbflags GDBFLAGS
+    global gdb_prompt
+
+    set GDBFLAGS $saved_gdbflags
+    append GDBFLAGS " -ex \"set height 0\""
+    append GDBFLAGS " -ex \"start\""
+    append GDBFLAGS " --args \"$binfile\""
+
+    with_test_prefix "probe support" {
+	set test "run to main"
+
+	gdb_exit
+	set res [gdb_spawn]
+	if { $res != 0} {
+	    fail $test
+	    return -1
+	}
+
+	set res -1
+	gdb_test_multiple "" $test {
+	    -re "main .* at .*$srcfile.*$gdb_prompt $" {
+		set res 1
+		pass $test
+	    }
+	    -re "Don't know how to run.*$gdb_prompt $" {
+		set res 0
+		unsupported $test
+	    }
+	}
+	return $res
+    }
+}
+
+# Check that we handle pagination correctly when it triggers due to an
+# execution command entered directly on the command line.
+
+proc test_fg_execution_pagination_return {} {
+    global binfile
+    global saved_gdbflags GDBFLAGS
+    global gdb_prompt pagination_prompt
+
+    set GDBFLAGS $saved_gdbflags
+    append GDBFLAGS " -ex \"set height 2\""
+    append GDBFLAGS " -ex \"start\""
+    append GDBFLAGS " --args \"$binfile\""
+
+    with_test_prefix "return" {
+	set test "run to pagination"
+
+	gdb_exit
+	set res [gdb_spawn]
+	if { $res != 0} {
+	    fail $test
+	    return $res
+	}
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		pass $test
+	    }
+	    -re "$gdb_prompt $" {
+		fail $test
+	    }
+	}
+
+	send_gdb "\n"
+
+	set saw_pagination_prompt 0
+	set test "send \\n to GDB"
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		set saw_pagination_prompt 1
+		send_gdb "\n"
+		exp_continue
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	    -re "$gdb_prompt $" {
+		gdb_assert $saw_pagination_prompt $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+# Check that we handle canceling pagination correctly when it triggers
+# due to an execution command entered directly on the command line.
+
+proc test_fg_execution_pagination_cancel { how } {
+    global binfile
+    global saved_gdbflags GDBFLAGS
+    global gdb_prompt pagination_prompt
+
+    set GDBFLAGS $saved_gdbflags
+
+    append GDBFLAGS " -ex \"set height 2\""
+    append GDBFLAGS " -ex \"start\""
+    append GDBFLAGS " --args \"$binfile\""
+
+    with_test_prefix "ctrl-c" {
+	set test "run to pagination"
+
+	gdb_exit
+	set res [gdb_spawn]
+	if { $res != 0} {
+	    fail $test
+	    return $res
+	}
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		pass $test
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	set test "cancel pagination"
+	if { $how == "ctrl-c" } {
+	    send_gdb "\003"
+	} else {
+	    send_gdb "q\n"
+
+	}
+	gdb_test_multiple "" $test {
+	    -re "Quit\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+if {[probe_can_run_cmdline] > 0} {
+    test_fg_execution_pagination_return
+    test_fg_execution_pagination_cancel "ctrl-c"
+    test_fg_execution_pagination_cancel "quit"
+}
+
+set GDBFLAGS $saved_gdbflags
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 36cbf05..3533ee3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -69,6 +69,9 @@ if ![info exists gdb_prompt] then {
     set gdb_prompt "\[(\]gdb\[)\]"
 }
 
+# A regexp that matches the pagination prompt.
+set pagination_prompt "---Type <return> to continue, or q <return> to quit---"
+
 # The variable fullname_syntax_POSIX is a regexp which matches a POSIX 
 # absolute path ie. /foo/ 
 set fullname_syntax_POSIX {/[^\n]*/}
@@ -1425,19 +1428,12 @@ proc gdb_file_cmd { arg } {
     }
 }
 
-#
-# start gdb -- start gdb running, default procedure
-#
-# When running over NFS, particularly if running many simultaneous
-# tests on different hosts all using the same server, things can
-# get really slow.  Give gdb at least 3 minutes to start up.
-#
-proc default_gdb_start { } {
-    global verbose use_gdb_stub
+# Default gdb_spawn procedure.
+
+proc default_gdb_spawn { } {
+    global use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
-    global gdb_prompt
-    global timeout
     global gdb_spawn_id
 
     gdb_stop_suppressing_tests
@@ -1468,21 +1464,45 @@ proc default_gdb_start { } {
 	perror "Spawning $GDB failed."
 	return 1
     }
+    set gdb_spawn_id -1
+    return 0
+}
+
+# Default gdb_start procedure.
+
+proc default_gdb_start { } {
+    global gdb_prompt
+    global gdb_spawn_id
+
+    if [info exists gdb_spawn_id] {
+	return 0
+    }
+
+    set res [gdb_spawn]
+    if { $res != 0} {
+	return $res
+    }
+
+    # When running over NFS, particularly if running many simultaneous
+    # tests on different hosts all using the same server, things can
+    # get really slow.  Give gdb at least 3 minutes to start up.
     gdb_expect 360 {
 	-re "\[\r\n\]$gdb_prompt $" {
 	    verbose "GDB initialized."
 	}
 	-re "$gdb_prompt $"	{
 	    perror "GDB never initialized."
+	    unset gdb_spawn_id
 	    return -1
 	}
 	timeout	{
 	    perror "(timeout) GDB never initialized after 10 seconds."
 	    remote_close host
+	    unset gdb_spawn_id
 	    return -1
 	}
     }
-    set gdb_spawn_id -1
+
     # force the height to "unlimited", so no pagers get used
 
     send_gdb "set height 0\n"
@@ -3285,6 +3305,23 @@ proc gdb_clear_suppressed { } {
     set suppress_flag 0
 }
 
+# Spawn the gdb process.
+#
+# This doesn't expect any output or do any other initialization,
+# leaving those to the caller.
+#
+# Overridable function -- you can override this function in your
+# baseboard file.
+
+proc gdb_spawn { } {
+    default_gdb_spawn
+}
+
+# Start gdb running, wait for prompt, and disable the pagers.
+
+# Overridable function -- you can override this function in your
+# baseboard file.
+
 proc gdb_start { } {
     default_gdb_start
 }
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/9] Eliminate exceptions.c:print_any_exception.
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (3 preceding siblings ...)
  2014-07-03 15:19 ` [PATCH 6/9] Background execution + pagination aborts readline/gdb Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-07-03 20:38   ` Tom Tromey
  2014-07-03 15:19 ` [PATCH 8/9] Fix double prompt Pedro Alves
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

exception_print and exception_fprintf call print_flush, which does all the
same flushing and annotation things that print_any_exception does, and more.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* exceptions.c (print_any_exception): Delete.
	(catch_exceptions_with_msg): Use exception_print instead of
	print_any_exception.
	(catch_errors): Use exception_fprintf instead of
	print_any_exception.
	(catch_command_errors, catch_command_errors_const): Use
	exception_print instead of print_any_exception.
---
 gdb/exceptions.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index ca80893..8ee428d 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -331,24 +331,6 @@ exception_fprintf (struct ui_file *file, struct gdb_exception e,
     }
 }
 
-static void
-print_any_exception (struct ui_file *file, const char *prefix,
-		     struct gdb_exception e)
-{
-  if (e.reason < 0 && e.message != NULL)
-    {
-      target_terminal_ours ();
-      wrap_here ("");		/* Force out any buffered output.  */
-      gdb_flush (gdb_stdout);
-      annotate_error_begin ();
-
-      /* Print the prefix.  */
-      if (prefix != NULL && prefix[0] != '\0')
-	fputs_filtered (prefix, file);
-      print_exception (file, e);
-    }
-}
-
 /* A stack of exception messages.
    This is needed to handle nested calls to throw_it: we don't want to
    xfree space for a message before it's used.
@@ -486,7 +468,7 @@ catch_exceptions_with_msg (struct ui_out *func_uiout,
       throw_exception (exception);
     }
 
-  print_any_exception (gdb_stderr, NULL, exception);
+  exception_print (gdb_stderr, exception);
   gdb_assert (val >= 0);
   gdb_assert (exception.reason <= 0);
   if (exception.reason < 0)
@@ -534,7 +516,7 @@ catch_errors (catch_errors_ftype *func, void *func_args, char *errstring,
       throw_exception (exception);
     }
 
-  print_any_exception (gdb_stderr, errstring, exception);
+  exception_fprintf (gdb_stderr, exception, "%s", errstring);
   if (exception.reason != 0)
     return 0;
   return val;
@@ -550,7 +532,7 @@ catch_command_errors (catch_command_errors_ftype *command,
     {
       command (arg, from_tty);
     }
-  print_any_exception (gdb_stderr, NULL, e);
+  exception_print (gdb_stderr, e);
   if (e.reason < 0)
     return 0;
   return 1;
@@ -566,7 +548,7 @@ catch_command_errors_const (catch_command_errors_const_ftype *command,
     {
       command (arg, from_tty);
     }
-  print_any_exception (gdb_stderr, NULL, e);
+  exception_print (gdb_stderr, e);
   if (e.reason < 0)
     return 0;
   return 1;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] Put the inferior's terminal settings in effect while running (fg) infcalls
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-07-03 15:19 ` [PATCH 3/9] Move catch_command_errors and catch_command_errors_const to main.c Pedro Alves
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

The "call" and "print" commands presently always run synchronously, in
the foreground, but GDB currently forgets to put the inferior's
terminal settings into effect while running them, on async-capable
targets, resulting in:

 (gdb) print func ()
 hello world

 Program received signal SIGTTOU, Stopped (tty output).
 0x000000373bceb8d0 in __libc_tcdrain (fd=1) at ../sysdeps/unix/sysv/linux/tcdrain.c:29
 29          return INLINE_SYSCALL (ioctl, 3, fd, TCSBRK, 1);
 The program being debugged was signaled while in a function called from GDB.
 GDB remains in the frame where the signal was received.
 To change this behavior use "set unwindonsignal on".
 Evaluation of the expression containing the function
 (func) will be abandoned.
 When the function is done executing, GDB will silently stop.
 (gdb)

That's because target_terminal_inferior skips actually doing anything
if running in the background, and, nothing is setting sync_execution
while running infcalls:

 void
 target_terminal_inferior (void)
 {
   /* A background resume (``run&'') should leave GDB in control of the
      terminal.  Use target_can_async_p, not target_is_async_p, since at
      this point the target is not async yet.  However, if sync_execution
      is not set, we know it will become async prior to resume.  */
   if (target_can_async_p () && !sync_execution)
     return;

This would best be all cleaned up by making GDB not even call
target_terminal_inferior and try to pass the terminal to the inferior
if running in the background, but that's a more invasive fix that is
better done post-7.8.

This was originally caught by a patch later in this series that makes
catch_command_errors use exception_print instead of
print_any_exception.  Note that print_flush calls serial_drain_output
while print_any_exception doesnt't have that bit.  And,
gdb.gdb/python-selftest.exp does:

 gdb_test "call catch_command_errors(execute_command, \"python print 5\", 0, RETURN_MASK_ALL)" \
   "Python not initialized.* = 0"

which without this fix results in SIGTTOU...

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* infcall.c (run_inferior_call): Set 'sync_execution' while
	running the inferior call.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* gdb.base/execution-termios.c: New file.
	* gdb.base/execution-termios.exp: New file.
---
 gdb/infcall.c                                |  7 ++++
 gdb/testsuite/gdb.base/execution-termios.c   | 35 ++++++++++++++++
 gdb/testsuite/gdb.base/execution-termios.exp | 60 ++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/execution-termios.c
 create mode 100644 gdb/testsuite/gdb.base/execution-termios.exp

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 52f0612..a9b1ceb 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -388,6 +388,11 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
   volatile struct gdb_exception e;
   int saved_in_infcall = call_thread->control.in_infcall;
   ptid_t call_thread_ptid = call_thread->ptid;
+  int saved_sync_execution = sync_execution;
+
+  /* Infcalls run synchronously, in the foreground.  */
+  if (target_can_async_p ())
+    sync_execution = 1;
 
   call_thread->control.in_infcall = 1;
 
@@ -441,6 +446,8 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
   if (call_thread != NULL)
     call_thread->control.in_infcall = saved_in_infcall;
 
+  sync_execution = saved_sync_execution;
+
   return e;
 }
 
diff --git a/gdb/testsuite/gdb.base/execution-termios.c b/gdb/testsuite/gdb.base/execution-termios.c
new file mode 100644
index 0000000..adeb1f1
--- /dev/null
+++ b/gdb/testsuite/gdb.base/execution-termios.c
@@ -0,0 +1,35 @@
+/* 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 <stdio.h>
+#include <termios.h>
+#include <unistd.h>
+
+int
+func (void)
+{
+  puts ("hello world\n");
+  tcdrain (fileno (stdout));
+  return 1;
+}
+
+int
+main (void)
+{
+  func ();
+  return 0; /* set break here */
+}
diff --git a/gdb/testsuite/gdb.base/execution-termios.exp b/gdb/testsuite/gdb.base/execution-termios.exp
new file mode 100644
index 0000000..cbdcce6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/execution-termios.exp
@@ -0,0 +1,60 @@
+# 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/>.  */
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Run to main, and execute BODY in the caller's context, with PREFIX
+# set as test message prefix.
+
+proc test { prefix body } {
+    with_test_prefix $prefix {
+	if ![runto_main] {
+	    fail "Can't run to main"
+	    return 0
+	}
+	uplevel 1 $body
+    }
+}
+
+# If GDB forgets to put the inferior's terminal settings into effect
+# while running any of these commands, the program will get a SIGTTOU.
+
+test "next" {
+    gdb_test "next" "set break here.*" "termios ok"
+}
+
+test "infcall" {
+    if ![target_info exists gdb,cannot_call_functions] {
+	gdb_test "print func ()" " = 1"  "termios ok"
+    } else {
+	unsupported "cannot call functions"
+    }
+}
+
+test "continue" {
+    set lineno [gdb_get_line_number "set break here"]
+    gdb_test "break $lineno"
+    gdb_test "continue" ".*set break here.*" "termios ok"
+}
+
+test "finish" {
+    gdb_test "break func" "func.*"
+    gdb_test "continue" "func .*"
+    gdb_test "finish" " = 1" "termios ok"
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 8/9] Fix double prompt
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (4 preceding siblings ...)
  2014-07-03 15:19 ` [PATCH 2/9] Eliminate exceptions.c:print_any_exception Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-07-03 15:19 ` [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb Pedro Alves
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

If an error is thrown while handling a target event (within
fetch_inferior_event), and, the interpreter is not async (but the
target is), then GDB prints the prompt twice.

One way to see that in action is throw a QUIT while in a pagination
prompt issued from within fetch_inferior_event (or one of its
callees).  E.g. from the test:

 ---Type <return> to continue, or q <return> to quit---
 ^CQuit
 (gdb) (gdb) p 1
 ^^^^^^^^^^^
 $1 = 1
 (gdb)

The issue is that inferior_event_handler swallows errors and notifies
the observers (the interpreters) about the command error, even if the
interpreter is forced sync while we're handling a nested event loop
(for execute_command).  The observers print a prompt, and then when we
get back to the top event loop, we print another (in
start_event_loop).

I see no reason the error should be swallowed here.  Just cancel the
execution related bits and let the error propagate to the top level
(start_event_loop), which re-enables stdin and notifies observers.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* inf-loop.c (inferior_event_handler): Use TRY_CATCH instead of
	catch_errors.  Don't re-enable stdin or notify observers where,
	and rethrow error.
	(fetch_inferior_event_wrapper): Delete.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* gdb.base/double-prompt-target-event-error.c: New file.
	* gdb.base/double-prompt-target-event-error.exp: New file.
---
 gdb/inf-loop.c                                     |  36 ++++---
 .../gdb.base/double-prompt-target-event-error.c    |  23 +++++
 .../gdb.base/double-prompt-target-event-error.exp  | 108 +++++++++++++++++++++
 3 files changed, 148 insertions(+), 19 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/double-prompt-target-event-error.c
 create mode 100644 gdb/testsuite/gdb.base/double-prompt-target-event-error.exp

diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index 247e9d6..d4f9a35 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -33,8 +33,6 @@
 #include "top.h"
 #include "observer.h"
 
-static int fetch_inferior_event_wrapper (gdb_client_data client_data);
-
 /* General function to handle events in the inferior.  So far it just
    takes care of detecting errors reported by select() or poll(),
    otherwise it assumes that all is OK, and goes on reading data from
@@ -48,19 +46,26 @@ inferior_event_handler (enum inferior_event_type event_type,
   switch (event_type)
     {
     case INF_REG_EVENT:
-      /* Use catch errors for now, until the inner layers of
+      /* Catch errors for now, until the inner layers of
 	 fetch_inferior_event (i.e. readchar) can return meaningful
 	 error status.  If an error occurs while getting an event from
 	 the target, just cancel the current command.  */
-      if (!catch_errors (fetch_inferior_event_wrapper, 
-			 client_data, "", RETURN_MASK_ALL))
-	{
-	  bpstat_clear_actions ();
-	  do_all_intermediate_continuations (1);
-	  do_all_continuations (1);
-	  async_enable_stdin ();
-	  observer_notify_command_error ();
-	}
+      {
+	volatile struct gdb_exception ex;
+
+	TRY_CATCH (ex, RETURN_MASK_ALL)
+	  {
+	    fetch_inferior_event (client_data);
+	  }
+	if (ex.reason < 0)
+	  {
+	    bpstat_clear_actions ();
+	    do_all_intermediate_continuations (1);
+	    do_all_continuations (1);
+
+	    throw_exception (ex);
+	  }
+      }
       break;
 
     case INF_EXEC_COMPLETE:
@@ -140,10 +145,3 @@ inferior_event_handler (enum inferior_event_type event_type,
 
   discard_cleanups (cleanup_if_error);
 }
-
-static int 
-fetch_inferior_event_wrapper (gdb_client_data client_data)
-{
-  fetch_inferior_event (client_data);
-  return 1;
-}
diff --git a/gdb/testsuite/gdb.base/double-prompt-target-event-error.c b/gdb/testsuite/gdb.base/double-prompt-target-event-error.c
new file mode 100644
index 0000000..89e92fb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/double-prompt-target-event-error.c
@@ -0,0 +1,23 @@
+/* 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/>.  */
+
+int
+main (void)
+{
+  sleep (3);
+  return 0; /* after sleep */
+}
diff --git a/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp b/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp
new file mode 100644
index 0000000..bb5096e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/double-prompt-target-event-error.exp
@@ -0,0 +1,108 @@
+# Copyright (C) 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/>.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Test throwing an error while GDB is handling a target event.  We use
+# a ctrl-c/quit in a pagination prompt to emulate an error.  COMMAND
+# is either "continue" or "wrapcont".  The latter is a continue issued
+# from a user-defined command.  That exercises the case of the
+# interpreter forced sync, which was the case that originally had a
+# bug.
+
+proc cancel_pagination_in_target_event { command } {
+    global binfile srcfile
+    global gdb_prompt pagination_prompt
+
+    set testline [gdb_get_line_number "after sleep"]
+
+    with_test_prefix "ctrlc target event: $command" {
+	clean_restart $binfile
+
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	gdb_test "b $srcfile:$testline" \
+	    "Breakpoint .*$srcfile, line $testline.*" \
+	    "set breakpoint"
+
+	gdb_test_no_output "set height 2"
+
+	if { $command == "wrapcont" } {
+	    gdb_test_multiple "define wrapcont" "define user command: wrapcont" {
+		-re "Type commands for definition of \"wrapcont\".\r\nEnd with a line saying just \"end\".\r\n>$" {
+		    # Note that "Continuing." is ommitted when
+		    # "continue" is issued from a user-defined
+		    # command.  Issue it ourselves.
+		    gdb_test "echo Continuing\.\ncontinue\nend" "" \
+			"define user command: wrapcont"
+		}
+	    }
+	}
+
+	# Wait for pagination prompt after the "Continuing" line,
+	# indicating the program was running and then stopped.
+	set saw_continuing 0
+	set test "continue to pagination"
+	gdb_test_multiple "$command" $test {
+	    -re "$pagination_prompt$" {
+		if {$saw_continuing} {
+		    pass $test
+		} else {
+		    send_gdb "\n"
+		    exp_continue
+		}
+	    }
+	    -re "Continuing" {
+		set saw_continuing 1
+		exp_continue
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	# We're now stopped in a pagination query while handling a
+	# target event (printing where the program stopped).  Quitting
+	# the pagination should result in only one prompt being
+	# output.
+	send_gdb "\003p 1\n"
+
+	set test "no double prompt"
+	gdb_test_multiple "" $test {
+	    -re "$gdb_prompt.*$gdb_prompt.*$gdb_prompt $" {
+		fail $test
+	    }
+	    -re "$gdb_prompt .* = 1\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+    }
+}
+
+foreach variant { "continue" "wrapcont" } {
+    cancel_pagination_in_target_event $variant
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 6/9] Background execution + pagination aborts readline/gdb
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (2 preceding siblings ...)
  2014-07-03 15:19 ` [PATCH 9/9] Put GDB's terminal settings into effect when paginating Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-09-13  0:05   ` Sergio Durigan Junior
  2014-07-03 15:19 ` [PATCH 2/9] Eliminate exceptions.c:print_any_exception Pedro Alves
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

If pagination occurs as result of output sent as response to a target
event while the target is executing in the background, subsequent
input aborts readline/gdb:

 $ gdb program
 ...
 (gdb) continue&
 Continuing.
 (gdb)
 ---Type <return> to continue, or q <return> to quit---
 *return*
 ---Type <return> to continue, or q <return> to quit---
 Breakpoint 2, after_sleep () at paginate-bg-execution.c:21
 ---Type <return> to continue, or q <return> to quit---
 21        return; /* after sleep */
 p 1
 readline: readline_callback_read_char() called with no handler!
 *abort/SIGABRT*
 $

gdb_readline_wrapper_line removes the handler after a line is
processed.  Usually, we'll end up re-displaying the prompt, and that
reinstalls the handler.  But if the output is coming out of handling
a stop event, we don't re-display the prompt, and nothing restores the
handler.  So the next input wakes up the event loop and calls into
readline, which aborts.

We should do better with the prompt handling while the target is
running (I think we should coordinate with readline, and
hide/redisplay it around output), but that's a more invasive change
better done post 7.8, so this patch is conservative and just
reinstalls the handler as soon as we're out of the readline line
callback.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* top.c (gdb_readline_wrapper_line): Tweak comment.
	(gdb_readline_wrapper_cleanup): If readline is enabled, reinstall
	the input handler callback.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* gdb.base/paginate-bg-execution.c: New file.
 	* gdb.base/paginate-bg-execution.exp: New file.
---
 gdb/testsuite/gdb.base/paginate-bg-execution.c   |  30 ++++++
 gdb/testsuite/gdb.base/paginate-bg-execution.exp | 121 +++++++++++++++++++++++
 gdb/top.c                                        |   9 +-
 3 files changed, 159 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.exp

diff --git a/gdb/testsuite/gdb.base/paginate-bg-execution.c b/gdb/testsuite/gdb.base/paginate-bg-execution.c
new file mode 100644
index 0000000..7457b18
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-bg-execution.c
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+static void
+after_sleep (void)
+{
+  return; /* after sleep */
+}
+
+int
+main (void)
+{
+  sleep (3);
+  after_sleep ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/paginate-bg-execution.exp b/gdb/testsuite/gdb.base/paginate-bg-execution.exp
new file mode 100644
index 0000000..ac62727
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-bg-execution.exp
@@ -0,0 +1,121 @@
+# Copyright (C) 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/>.
+
+# A collection of tests related to running execution commands directly
+# from the command line, with "-ex".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Check that we handle pagination correctly when it triggers due to an
+# background execution command entered directly on the command line.
+
+proc test_bg_execution_pagination_return {} {
+    global binfile
+    global pagination_prompt
+
+    with_test_prefix "paginate" {
+	clean_restart $binfile
+
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	gdb_test "b after_sleep"
+
+	gdb_test_no_output "set height 2"
+
+	gdb_test "continue&" "Continuing\."
+
+	set test "pagination handled, breakpoint hit"
+	set saw_pagination_prompt 0
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		set saw_pagination_prompt 1
+		send_gdb "\n"
+		exp_continue
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an
+		# error.
+		exp_continue
+	    }
+	    -re "after sleep\[^\r\n\]+\r\n$" {
+		gdb_assert $saw_pagination_prompt $test
+	    }
+	}
+
+	# GDB used to crash here.
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+# Check that we handle canceling pagination correctly when it triggers
+# due to a background execution command entered directly on the
+# command line.
+
+proc test_bg_execution_pagination_cancel { how } {
+    global binfile
+    global gdb_prompt pagination_prompt
+
+    with_test_prefix "cancel with $how" {
+	clean_restart $binfile
+
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	gdb_test "b after_sleep"
+
+	gdb_test_no_output "set height 2"
+
+	gdb_test "continue&" "Continuing\."
+
+	set test "continue& paginates"
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		pass $test
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	set test "cancel pagination"
+	if { $how == "ctrl-c" } {
+	    send_gdb "\003"
+	} else {
+	    send_gdb "q\n"
+
+	}
+	gdb_test_multiple "" $test {
+	    -re "Quit\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+test_bg_execution_pagination_return
+test_bg_execution_pagination_cancel "ctrl-c"
+test_bg_execution_pagination_cancel "quit"
diff --git a/gdb/top.c b/gdb/top.c
index 722eb55..93a4a16 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -756,7 +756,8 @@ gdb_readline_wrapper_line (char *line)
   after_char_processing_hook = NULL;
 
   /* Prevent parts of the prompt from being redisplayed if annotations
-     are enabled, and readline's state getting out of sync.  */
+     are enabled, and readline's state getting out of sync.  We'll
+     restore it in gdb_readline_wrapper_cleanup.  */
   if (async_command_editing_p)
     rl_callback_handler_remove ();
 }
@@ -776,6 +777,12 @@ gdb_readline_wrapper_cleanup (void *arg)
 
   gdb_assert (input_handler == gdb_readline_wrapper_line);
   input_handler = cleanup->handler_orig;
+
+  /* Reinstall INPUT_HANDLER in readline, without displaying a
+     prompt.  */
+  if (async_command_editing_p)
+    rl_callback_handler_install (NULL, input_handler);
+
   gdb_readline_wrapper_result = NULL;
   gdb_readline_wrapper_done = 0;
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 9/9] Put GDB's terminal settings into effect when paginating.
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
  2014-07-03 15:19 ` [PATCH 1/9] Put the inferior's terminal settings in effect while running (fg) infcalls Pedro Alves
  2014-07-03 15:19 ` [PATCH 3/9] Move catch_command_errors and catch_command_errors_const to main.c Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-07-03 15:19 ` [PATCH 6/9] Background execution + pagination aborts readline/gdb Pedro Alves
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

When the target is resumed in the foreground, we put the inferior's
terminal settings into effect, and remove stdin from the event loop.
When the target stops, we put GDB's terminal settings into effect
again, and re-register stdin in the event loop, ready for user input.
The former is done by target_terminal_inferior, and the latter by
target_terminal_ours.

There's an intermediate -- target_terminal_ours_for_output -- that is
called when printing output related to target events, and we don't
know yet whether we'll stop the program.  That puts our terminal
settings into effect, enough to get proper results from our output,
but leaves input wired into the inferior.

If such output paginates, then we need the full target_terminal_ours
in order for the user to be able to provide input to answer the
pagination query.

The test in this commit hangs in async-capable targets without the fix
(as the user/test can't answer the pagination query).  It doesn't hang
on sync targets because on those we don't unregister stdin from the
event loop while the target is running (because we block in
target_wait instead of in the event loop in that case).

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* utils.c (prompt_for_continue): Call target_terminal_ours.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* gdb.base/paginate-after-ctrl-c-running.c: New file.
 	* gdb.base/paginate-after-ctrl-c-running.exp: New file.
---
 .../gdb.base/paginate-after-ctrl-c-running.c       | 23 +++++++
 .../gdb.base/paginate-after-ctrl-c-running.exp     | 80 ++++++++++++++++++++++
 gdb/utils.c                                        |  4 ++
 3 files changed, 107 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp

diff --git a/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.c b/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.c
new file mode 100644
index 0000000..6d0a3f2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.c
@@ -0,0 +1,23 @@
+/* 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/>.  */
+
+int
+main (void)
+{
+  sleep (10);
+  return 0; /* after sleep */
+}
diff --git a/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp b/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp
new file mode 100644
index 0000000..d9cae67
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp
@@ -0,0 +1,80 @@
+# Copyright (C) 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/>.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Send a ctrl-c while the target is running and the next output causes
+# a pagination prompt.
+
+proc test_ctrlc_while_target_running_paginates {} {
+    global binfile srcfile
+    global gdb_prompt pagination_prompt
+
+    set testline [gdb_get_line_number "after sleep"]
+
+    with_test_prefix "ctrlc target running" {
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	gdb_test "b $srcfile:$testline" \
+	    "Breakpoint .*$srcfile, line $testline.*" \
+	    "set breakpoint"
+
+	gdb_test_no_output "set height 2"
+
+	# Wait for the "Continuing" line, indicating the program is
+	# running.
+	set test "continue"
+	gdb_test_multiple $test $test {
+	    -re "Continuing" {
+		pass $test
+	    }
+	}
+
+	# The program sleeps 10 seconds.  Wait just a bit and send a
+	# ctrl-c.
+	sleep 2
+	send_gdb "\003"
+
+	# GDB now intercepts the SIGINT and tries to let the user know
+	# about the spurious signal, but that paginates.  Make sure
+	# the user can respond to the pagination query.
+	set test "got prompt"
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		send_gdb "\n"
+		exp_continue
+	    }
+	    -re "$gdb_prompt $" {
+		pass $test
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	# Confirm GDB can still process input.
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+test_ctrlc_while_target_running_paginates
diff --git a/gdb/utils.c b/gdb/utils.c
index 6f47cb0..d324227 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1877,6 +1877,10 @@ prompt_for_continue (void)
 
   immediate_quit++;
   QUIT;
+
+  /* We'll need to handle input.  */
+  target_terminal_ours ();
+
   /* On a real operating system, the user can quit with SIGINT.
      But not on GO32.
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/9] Move catch_command_errors and catch_command_errors_const to main.c
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
  2014-07-03 15:19 ` [PATCH 1/9] Put the inferior's terminal settings in effect while running (fg) infcalls Pedro Alves
@ 2014-07-03 15:19 ` Pedro Alves
  2014-07-03 15:19 ` [PATCH 9/9] Put GDB's terminal settings into effect when paginating Pedro Alves
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:19 UTC (permalink / raw)
  To: gdb-patches

We'll need to add error handling code to commands run before the event
loop starts (commands in .gdbinit, -ex commands, etc.).  Turns out
those are run through catch_command_errors, and, catch_command_errors
is used nowhere else.  Move it (and the _const variant) to main.c, so
that we can further specialize it freely.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* exceptions.c (catch_command_errors, catch_command_errors_const):
	Moved to main.c.
	* exceptions.h (catch_command_errors_ftype)
	(catch_command_errors_const_ftype): Moved to main.c.
	(catch_command_errors, catch_command_errors_const): Delete
	declarations.
	* main.c (catch_command_errors_ftype)
	(catch_command_errors_const_ftype): Moved here from exceptions.h.
	(catch_command_errors, catch_command_errors_const)): Moved here
	from exceptions.c and make static.
---
 gdb/exceptions.c | 32 --------------------------------
 gdb/exceptions.h | 13 -------------
 gdb/main.c       | 44 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 8ee428d..def1f41 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -521,35 +521,3 @@ catch_errors (catch_errors_ftype *func, void *func_args, char *errstring,
     return 0;
   return val;
 }
-
-int
-catch_command_errors (catch_command_errors_ftype *command,
-		      char *arg, int from_tty, return_mask mask)
-{
-  volatile struct gdb_exception e;
-
-  TRY_CATCH (e, mask)
-    {
-      command (arg, from_tty);
-    }
-  exception_print (gdb_stderr, e);
-  if (e.reason < 0)
-    return 0;
-  return 1;
-}
-
-int
-catch_command_errors_const (catch_command_errors_const_ftype *command,
-			    const char *arg, int from_tty, return_mask mask)
-{
-  volatile struct gdb_exception e;
-
-  TRY_CATCH (e, mask)
-    {
-      command (arg, from_tty);
-    }
-  exception_print (gdb_stderr, e);
-  if (e.reason < 0)
-    return 0;
-  return 1;
-}
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index b8dadc7..e5e1a49 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -255,17 +255,4 @@ extern int catch_exceptions_with_msg (struct ui_out *uiout,
 typedef int (catch_errors_ftype) (void *);
 extern int catch_errors (catch_errors_ftype *, void *, char *, return_mask);
 
-/* Template to catch_errors() that wraps calls to command
-   functions.  */
-
-typedef void (catch_command_errors_ftype) (char *, int);
-extern int catch_command_errors (catch_command_errors_ftype *func,
-				 char *arg, int from_tty, return_mask);
-
-/* Like catch_command_errors, but works with const command and args.  */
-
-typedef void (catch_command_errors_const_ftype) (const char *, int);
-extern int catch_command_errors_const (catch_command_errors_const_ftype *func,
-				       const char *arg, int from_tty, return_mask);
-
 #endif
diff --git a/gdb/main.c b/gdb/main.c
index 0d4d512..1d77bd3 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -337,6 +337,50 @@ captured_command_loop (void *data)
   return 1;
 }
 
+/* Type of the command callback passed to catch_command_errors.  */
+
+typedef void (catch_command_errors_ftype) (char *, int);
+
+/* Wrap calls to commands run before the event loop is started.  */
+
+static int
+catch_command_errors (catch_command_errors_ftype *command,
+		      char *arg, int from_tty, return_mask mask)
+{
+  volatile struct gdb_exception e;
+
+  TRY_CATCH (e, mask)
+    {
+      command (arg, from_tty);
+    }
+  exception_print (gdb_stderr, e);
+  if (e.reason < 0)
+    return 0;
+  return 1;
+}
+
+/* Type of the command callback passed to catch_command_errors_const.  */
+
+typedef void (catch_command_errors_const_ftype) (const char *, int);
+
+/* Like catch_command_errors, but works with const command and args.  */
+
+static int
+catch_command_errors_const (catch_command_errors_const_ftype *command,
+			    const char *arg, int from_tty, return_mask mask)
+{
+  volatile struct gdb_exception e;
+
+  TRY_CATCH (e, mask)
+    {
+      command (arg, from_tty);
+    }
+  exception_print (gdb_stderr, e);
+  if (e.reason < 0)
+    return 0;
+  return 1;
+}
+
 /* Arguments of --command option and its counterpart.  */
 typedef struct cmdarg {
   /* Type of this option.  */
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 4/9] testsuite: Introduce gdb_assert
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (6 preceding siblings ...)
  2014-07-03 15:19 ` [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb Pedro Alves
@ 2014-07-03 15:27 ` Pedro Alves
  2014-07-03 20:41   ` Tom Tromey
  2014-07-03 15:31 ` [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
  2014-07-03 15:40 ` [PATCH 7/9] Remove the target from the event loop while in secondary prompts Pedro Alves
  9 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:27 UTC (permalink / raw)
  To: gdb-patches

Often we'll do something like:

    if {$ok} {
	fail "whatever"
    } else {
	pass "whatever"
    }

This adds a helper procedure for that, and converts one random place
to use it, as an example.

2014-07-02  Pedro Alves  <palves@redhat.com>

	* lib/gdb.exp (gdb_assert): New procedure.
	* gdb.trace/backtrace.exp (gdb_backtrace_tdp_4): Use it.
---
 gdb/testsuite/gdb.trace/backtrace.exp |  7 ++-----
 gdb/testsuite/lib/gdb.exp             | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/backtrace.exp b/gdb/testsuite/gdb.trace/backtrace.exp
index a74fc3f..cb50f06 100644
--- a/gdb/testsuite/gdb.trace/backtrace.exp
+++ b/gdb/testsuite/gdb.trace/backtrace.exp
@@ -256,11 +256,8 @@ proc gdb_backtrace_tdp_4 { msg depth traceframe } {
 
 	# Output of 'tdump' on frame 0 and frame 1 should be
 	# identical.
-	if ![string compare $output_string0 $output_string1]  {
-	    pass "tdump output"
-	} else {
-	    fail "tdump output"
-	}
+	gdb_assert ![string compare $output_string0 $output_string1] \
+	    "tdump output"
     }
 }
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 30463a9..36cbf05 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1204,6 +1204,27 @@ proc gdb_test_list_exact { cmd name elm_find_regexp elm_extract_regexp result_ma
     }
 }
 \f
+
+# Issue a PASS and return true if evaluating CONDITION in the caller's
+# frame returns true, and issue a FAIL and return false otherwise.
+# MESSAGE is the pass/fail message to be printed.  If MESSAGE is
+# omitted or is empty, then the pass/fail messages use the condition
+# string as the message.
+
+proc gdb_assert { condition {message ""} } {
+    if { $message == ""} {
+	set message $condition
+    }
+
+    set res [uplevel 1 expr $condition]
+    if {!$res} {
+	fail $message
+    } else {
+	pass $message
+    }
+    return $res
+}
+
 proc gdb_reinitialize_dir { subdir } {
     global gdb_prompt
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND/PROPER PATCH 0/9] pagination/readline/async issues
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (7 preceding siblings ...)
  2014-07-03 15:27 ` [PATCH 4/9] testsuite: Introduce gdb_assert Pedro Alves
@ 2014-07-03 15:31 ` Pedro Alves
  2014-07-14 22:38   ` Pedro Alves
  2014-07-03 15:40 ` [PATCH 7/9] Remove the target from the event loop while in secondary prompts Pedro Alves
  9 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:31 UTC (permalink / raw)
  To: gdb-patches

I've now pushed this to:

 git@github.com:palves/gdb.git palves/pr17072_pagination_async

-- 
Pedro Alves

On 07/03/2014 04:18 PM, Pedro Alves wrote:
> This series fixes PR gdb/17072 (a 7.8 blocker) and other related
> issues I stumbled on while fixing it and writing tests.
> 
> Some of these fixes are conservative as this is intended for the 7.8
> branch as well.
> 
> Tested on x86_64 Fedora 20, native and gdbserver.
> 
> Comments?
> 
> Pedro Alves (9):
>   Put the inferior's terminal settings in effect while running (fg)
>     infcalls
>   Eliminate exceptions.c:print_any_exception.
>   Move catch_command_errors and catch_command_errors_const to main.c
>   testsuite: Introduce gdb_assert
>   Canceling pagination caused by execution command from command line
>     aborts readline/gdb
>   Background execution + pagination aborts readline/gdb
>   Remove the target from the event loop while in secondary prompts
>   Fix double prompt
>   Put GDB's terminal settings into effect when paginating.
> 
>  gdb/exceptions.c                                   |  54 +------
>  gdb/exceptions.h                                   |  13 --
>  gdb/inf-loop.c                                     |  36 ++---
>  gdb/infcall.c                                      |   7 +
>  gdb/main.c                                         |  58 +++++++
>  .../gdb.base/double-prompt-target-event-error.c    |  23 +++
>  .../gdb.base/double-prompt-target-event-error.exp  | 108 +++++++++++++
>  gdb/testsuite/gdb.base/execution-termios.c         |  35 +++++
>  gdb/testsuite/gdb.base/execution-termios.exp       |  60 +++++++
>  .../gdb.base/paginate-after-ctrl-c-running.c       |  23 +++
>  .../gdb.base/paginate-after-ctrl-c-running.exp     |  80 ++++++++++
>  gdb/testsuite/gdb.base/paginate-bg-execution.c     |  30 ++++
>  gdb/testsuite/gdb.base/paginate-bg-execution.exp   | 121 ++++++++++++++
>  .../gdb.base/paginate-execution-startup.c          |  30 ++++
>  .../gdb.base/paginate-execution-startup.exp        | 175 +++++++++++++++++++++
>  gdb/testsuite/gdb.base/paginate-inferior-exit.c    |  30 ++++
>  gdb/testsuite/gdb.base/paginate-inferior-exit.exp  |  82 ++++++++++
>  gdb/testsuite/gdb.trace/backtrace.exp              |   7 +-
>  gdb/testsuite/lib/gdb.exp                          |  82 ++++++++--
>  gdb/top.c                                          |  21 ++-
>  gdb/utils.c                                        |   4 +
>  21 files changed, 977 insertions(+), 102 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/double-prompt-target-event-error.c
>  create mode 100644 gdb/testsuite/gdb.base/double-prompt-target-event-error.exp
>  create mode 100644 gdb/testsuite/gdb.base/execution-termios.c
>  create mode 100644 gdb/testsuite/gdb.base/execution-termios.exp
>  create mode 100644 gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.c
>  create mode 100644 gdb/testsuite/gdb.base/paginate-after-ctrl-c-running.exp
>  create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.c
>  create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.exp
>  create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.c
>  create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.exp
>  create mode 100644 gdb/testsuite/gdb.base/paginate-inferior-exit.c
>  create mode 100644 gdb/testsuite/gdb.base/paginate-inferior-exit.exp
> 


-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 7/9] Remove the target from the event loop while in secondary prompts
  2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
                   ` (8 preceding siblings ...)
  2014-07-03 15:31 ` [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
@ 2014-07-03 15:40 ` Pedro Alves
  9 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:40 UTC (permalink / raw)
  To: gdb-patches

If a pagination prompt triggers while the target is running, and the
target exits before the user responded to the pagination query, this
happens:

  Starting program: foo
  ---Type <return> to continue, or q <return> to quit---No unwaited-for children left.
  Couldn't get registers: No such process.
  Couldn't get registers: No such process.
  Couldn't get registers: No such process.
  (gdb) Couldn't get registers: No such process.
  (gdb)

To reiterate, the user hasn't replied to the pagination prompt above.

A pagination query nests an event loop (in gdb_readline_wrapper).  In
async mode, in addition to stdin and signal handlers, we'll have the
target also installed in the event loop still.  So if the target
reports an event, that wakes up the nested event loop, which calls
into fetch_inferior_event etc. to handle the event which generates
further output, all while we should be waiting for pagination
confirmation...

(TBC, any target event that generates output ends up spuriously waking
up the pagination, though exits seem to be the worse kind.)

I've played with a couple different approaches to fixing this, while
at the same time trying to avoid being invasive.  Both revolve around
not listening to target events while in a pagination prompt (doing
anything else I think would be a much bigger change).

The approach taken just removes the target from the event loop while
within gdb_readline_wrapper.  The other approach used gdb_select
directly, with only input_fd installed, but that had the issue that it
didn't handle the async signal handlers, and turned out to be a bit
more code than the first version.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* top.c: Include "inf-loop.h".
	(struct gdb_readline_wrapper_cleanup) <target_is_async_orig>: New
	field.
	(gdb_readline_wrapper_cleanup): Make the target async again, if it
	was async before.
	(gdb_readline_wrapper): Store whether the target is async, and
	make it sync.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* gdb.base/paginate-inferior-exit.c: New file.
	* gdb.base/paginate-inferior-exit.exp: New file.
---
 gdb/testsuite/gdb.base/paginate-inferior-exit.c   | 30 +++++++++
 gdb/testsuite/gdb.base/paginate-inferior-exit.exp | 82 +++++++++++++++++++++++
 gdb/top.c                                         | 12 ++++
 3 files changed, 124 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/paginate-inferior-exit.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-inferior-exit.exp

diff --git a/gdb/testsuite/gdb.base/paginate-inferior-exit.c b/gdb/testsuite/gdb.base/paginate-inferior-exit.c
new file mode 100644
index 0000000..7457b18
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-inferior-exit.c
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+static void
+after_sleep (void)
+{
+  return; /* after sleep */
+}
+
+int
+main (void)
+{
+  sleep (3);
+  after_sleep ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/paginate-inferior-exit.exp b/gdb/testsuite/gdb.base/paginate-inferior-exit.exp
new file mode 100644
index 0000000..3418de8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-inferior-exit.exp
@@ -0,0 +1,82 @@
+# Copyright (C) 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/>.
+
+# A collection of tests related to running execution commands directly
+# from the command line, with "-ex".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Test paginating while printing about the inferior having exited.
+#
+proc test_paginate_inferior_exited {} {
+    global binfile
+    global gdb_prompt pagination_prompt
+    global inferior_exited_re
+
+    with_test_prefix "paginate" {
+	clean_restart $binfile
+
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	# Force pagination.
+	gdb_test_no_output "set height 2"
+
+	set test "continue to pagination"
+
+	# Wait for the "Starting program" line, indicating the program
+	# is running.
+	set saw_starting 0
+	gdb_test_multiple "continue" $test {
+	    -re "$pagination_prompt" {
+		if {$saw_starting} {
+		    pass $test
+		} else {
+		    send_gdb "\n"
+		    exp_continue
+		}
+	    }
+	    -re "Continuing" {
+		set saw_starting 1
+		exp_continue
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	# We're now stopped in a pagination output while handling a
+	# target event, trying to print about the program exiting.
+	set test "inferior exits normally"
+
+	send_gdb "\n"
+	gdb_test_multiple "" $test {
+	    -re "$inferior_exited_re normally.*$gdb_prompt $" {
+		pass $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+test_paginate_inferior_exited
diff --git a/gdb/top.c b/gdb/top.c
index 93a4a16..19c88f9 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -68,6 +68,7 @@
 #include "ui-out.h"
 #include "cli-out.h"
 #include "tracepoint.h"
+#include "inf-loop.h"
 
 extern void initialize_all_files (void);
 
@@ -766,6 +767,9 @@ struct gdb_readline_wrapper_cleanup
   {
     void (*handler_orig) (char *);
     int already_prompted_orig;
+
+    /* Whether the target was async.  */
+    int target_is_async_orig;
   };
 
 static void
@@ -789,6 +793,9 @@ gdb_readline_wrapper_cleanup (void *arg)
   after_char_processing_hook = saved_after_char_processing_hook;
   saved_after_char_processing_hook = NULL;
 
+  if (cleanup->target_is_async_orig)
+    target_async (inferior_event_handler, 0);
+
   xfree (cleanup);
 }
 
@@ -805,8 +812,13 @@ gdb_readline_wrapper (char *prompt)
 
   cleanup->already_prompted_orig = rl_already_prompted;
 
+  cleanup->target_is_async_orig = target_is_async_p ();
+
   back_to = make_cleanup (gdb_readline_wrapper_cleanup, cleanup);
 
+  if (cleanup->target_is_async_orig)
+    target_async (NULL, NULL);
+
   /* Display our prompt and prevent double prompt display.  */
   display_gdb_prompt (prompt);
   rl_already_prompted = 1;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/9] Eliminate exceptions.c:print_any_exception.
  2014-07-03 15:19 ` [PATCH 2/9] Eliminate exceptions.c:print_any_exception Pedro Alves
@ 2014-07-03 20:38   ` Tom Tromey
  0 siblings, 0 replies; 19+ messages in thread
From: Tom Tromey @ 2014-07-03 20:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> exception_print and exception_fprintf call print_flush, which
Pedro> does all the same flushing and annotation things that
Pedro> print_any_exception does, and more.

Looks like a nice cleanup.

Tom

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] testsuite: Introduce gdb_assert
  2014-07-03 15:27 ` [PATCH 4/9] testsuite: Introduce gdb_assert Pedro Alves
@ 2014-07-03 20:41   ` Tom Tromey
  0 siblings, 0 replies; 19+ messages in thread
From: Tom Tromey @ 2014-07-03 20:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> 2014-07-02  Pedro Alves  <palves@redhat.com>
Pedro> 	* lib/gdb.exp (gdb_assert): New procedure.
Pedro> 	* gdb.trace/backtrace.exp (gdb_backtrace_tdp_4): Use it.

Looks nice, I've wanted this from time to time.

Tom

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb
  2014-07-03 15:19 ` [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb Pedro Alves
@ 2014-07-04  6:11   ` Yao Qi
  2014-07-09 16:45     ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2014-07-04  6:11 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

Hi, Pedro,
the fix looks reasonable to me, some comments on test case.

On 07/03/2014 11:18 PM, Pedro Alves wrote:
> diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.c b/gdb/testsuite/gdb.base/paginate-execution-startup.c
> new file mode 100644
> index 0000000..7457b18
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/paginate-execution-startup.c
> @@ -0,0 +1,30 @@
> +/* 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/>.  */
> +
> +static void
> +after_sleep (void)
> +{
> +  return; /* after sleep */
> +}
> +
> +int
> +main (void)
> +{
> +  sleep (3);

We need to include <unistd.h>.  With some old mingw32 gcc (4.7.3), I
get the error: undefined reference to `sleep' (note that
i686-w64-mingw32-gcc 4.8.2 doesn't complain on this).  I fixed it by

#ifdef _WIN32
#include <windows.h>
#define sleep(x) Sleep(1000 * x)
#else
#include <unistd.h>
#endif

> +  after_sleep ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.exp b/gdb/testsuite/gdb.base/paginate-execution-startup.exp
> new file mode 100644
> index 0000000..77822fb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/paginate-execution-startup.exp
> @@ -0,0 +1,175 @@
> +# Copyright (C) 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/>.
> +
> +# A collection of tests related to pagination resulting from running
> +# execution commands directly from the command line, with "-ex".
> +
> +standard_testfile
> +
> +if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
> +    return -1
> +}
> +
> +global GDBFLAGS
> +set saved_gdbflags $GDBFLAGS
> +
> +# Returns true if the board can 'gdb -ex "start"', false otherwise.
> +
> +proc probe_can_run_cmdline  {} {
> +    global srcfile binfile
> +    global saved_gdbflags GDBFLAGS
> +    global gdb_prompt
> +
> +    set GDBFLAGS $saved_gdbflags
> +    append GDBFLAGS " -ex \"set height 0\""
> +    append GDBFLAGS " -ex \"start\""
> +    append GDBFLAGS " --args \"$binfile\""

This doesn't work on remote host, as we haven't copy binfile from build
to host yet.  We can do this after build_executable, and pass $file_arg
to each proc instead of using $binfile in each proc.

set file_arg $binfile
if [is_remote host] {
  set file_arg [remote_download host $file_arg]
}

> +
> +    with_test_prefix "probe support" {
> +	set test "run to main"
> +
> +	gdb_exit
> +	set res [gdb_spawn]
> +	if { $res != 0} {
> +	    fail $test
> +	    return -1
> +	}
> +
> +	set res -1
> +	gdb_test_multiple "" $test {
> +	    -re "main .* at .*$srcfile.*$gdb_prompt $" {
> +		set res 1
> +		pass $test
> +	    }
> +	    -re "Don't know how to run.*$gdb_prompt $" {
> +		set res 0
> +		unsupported $test
> +	    }
> +	}
> +	return $res
> +    }
> +}
> +
> +# Check that we handle pagination correctly when it triggers due to an
> +# execution command entered directly on the command line.
> +
> +proc test_fg_execution_pagination_return {} {
> +    global binfile
> +    global saved_gdbflags GDBFLAGS
> +    global gdb_prompt pagination_prompt
> +
> +    set GDBFLAGS $saved_gdbflags
> +    append GDBFLAGS " -ex \"set height 2\""
> +    append GDBFLAGS " -ex \"start\""
> +    append GDBFLAGS " --args \"$binfile\""
> +
> +    with_test_prefix "return" {
> +	set test "run to pagination"
> +
> +	gdb_exit
> +	set res [gdb_spawn]
> +	if { $res != 0} {
> +	    fail $test
> +	    return $res
> +	}
> +	gdb_test_multiple "" $test {
> +	    -re "$pagination_prompt$" {
> +		pass $test
> +	    }
> +	    -re "$gdb_prompt $" {
> +		fail $test
> +	    }
> +	}
> +
> +	send_gdb "\n"
> +
> +	set saw_pagination_prompt 0
> +	set test "send \\n to GDB"
> +	gdb_test_multiple "" $test {
> +	    -re "$pagination_prompt$" {
> +		set saw_pagination_prompt 1
> +		send_gdb "\n"
> +		exp_continue
> +	    }
> +	    -notransfer -re "<return>" {
> +		# Otherwise gdb_test_multiple considers this an error.
> +		exp_continue
> +	    }
> +	    -re "$gdb_prompt $" {
> +		gdb_assert $saw_pagination_prompt $test
> +	    }
> +	}
> +
> +	gdb_test "p 1" " = 1" "GDB accepts further input"
> +    }
> +}
> +
> +# Check that we handle canceling pagination correctly when it triggers
> +# due to an execution command entered directly on the command line.
> +
> +proc test_fg_execution_pagination_cancel { how } {
> +    global binfile
> +    global saved_gdbflags GDBFLAGS
> +    global gdb_prompt pagination_prompt
> +
> +    set GDBFLAGS $saved_gdbflags
> +
> +    append GDBFLAGS " -ex \"set height 2\""
> +    append GDBFLAGS " -ex \"start\""
> +    append GDBFLAGS " --args \"$binfile\""
> +
> +    with_test_prefix "ctrl-c" {
> +	set test "run to pagination"
> +
> +	gdb_exit
> +	set res [gdb_spawn]
> +	if { $res != 0} {
> +	    fail $test
> +	    return $res
> +	}
> +	gdb_test_multiple "" $test {
> +	    -re "$pagination_prompt$" {
> +		pass $test
> +	    }
> +	    -notransfer -re "<return>" {
> +		# Otherwise gdb_test_multiple considers this an error.
> +		exp_continue
> +	    }
> +	}
> +
> +	set test "cancel pagination"
> +	if { $how == "ctrl-c" } {
> +	    send_gdb "\003"
> +	} else {
> +	    send_gdb "q\n"
> +
> +	}
> +	gdb_test_multiple "" $test {
> +	    -re "Quit\r\n$gdb_prompt $" {
> +		pass $test
> +	    }
> +	}
> +
> +	gdb_test "p 1" " = 1" "GDB accepts further input"
> +    }
> +}
> +
> +if {[probe_can_run_cmdline] > 0} {
> +    test_fg_execution_pagination_return
> +    test_fg_execution_pagination_cancel "ctrl-c"

Skip it if [target_info exists gdb,nointerrupts] is true?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb
  2014-07-04  6:11   ` Yao Qi
@ 2014-07-09 16:45     ` Pedro Alves
  2014-07-10  9:26       ` Yao Qi
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2014-07-09 16:45 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 07/04/2014 07:10 AM, Yao Qi wrote:
> Hi, Pedro,
> the fix looks reasonable to me, some comments on test case.

Thanks Yao.

>> +static void
>> +after_sleep (void)
>> +{
>> +  return; /* after sleep */
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  sleep (3);
> 
> We need to include <unistd.h>.

Indeed.  I've added it to all new tests that needed it.

> With some old mingw32 gcc (4.7.3), I
> get the error: undefined reference to `sleep' (note that
> i686-w64-mingw32-gcc 4.8.2 doesn't complain on this).  I fixed it by
> 
> #ifdef _WIN32
> #include <windows.h>
> #define sleep(x) Sleep(1000 * x)
> #else
> #include <unistd.h>
> #endif

Looks like we already have a ton of tests using sleep/usleep,
so such a fix would best be done across the board.  Perhaps
even by adding a sleep.h header to testsuite/lib/ or some such ?
Or if not needed with newer toolchains, just ignore it.  :-)

>> +proc probe_can_run_cmdline  {} {
>> +    global srcfile binfile
>> +    global saved_gdbflags GDBFLAGS
>> +    global gdb_prompt
>> +
>> +    set GDBFLAGS $saved_gdbflags
>> +    append GDBFLAGS " -ex \"set height 0\""
>> +    append GDBFLAGS " -ex \"start\""
>> +    append GDBFLAGS " --args \"$binfile\""
> 
> This doesn't work on remote host, as we haven't copy binfile from build
> to host yet.  We can do this after build_executable, and pass $file_arg
> to each proc instead of using $binfile in each proc.

Thanks.  See the new version below.

Running the new tests with --host_board=local-remote-host
I also noticed they were taking longer than a non
remote-host testing run.  That turned out to be caused by
this gdb_expect timing out:

#
# gdb_exit -- exit the GDB, killing the target program if necessary
#
proc default_gdb_exit {} {
...
    if { [is_remote host] && [board_info host exists fileid] } {
	send_gdb "quit\n"
	gdb_expect 10 {
	    -re "y or n" {
		send_gdb "y\n"
		exp_continue
	    }
...
	}
    }

and that was because that "quit" paginates:

 (gdb) 
 quit
 A debugging session is active.
 ---Type <return> to continue, or q <return> to quit---

WDYT ?  I've rebased and force-pushed the updated series to:

 git@github.com:palves/gdb.git palves/pr17072_pagination_async

8<-----------
From bdbd2c630e01918cb6c6d07f52d6912af4fb3bf9 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Jul 2014 15:59:04 +0100
Subject: [PATCH] Canceling pagination caused by execution command from command
 line aborts readline/gdb

This fixes:

 $ ./gdb program -ex "set height 2" -ex "start"
 ...
 Reading symbols from /home/pedro/gdb/tests/threads...done.
 ---Type <return> to continue, or q <return> to quit---^CQuit  << ctrl-c triggers a Quit

 *type something*
 readline: readline_callback_read_char() called with no handler!
 Aborted
 $

Usually, if an error propagates all the way to the top level, we'll
re-enable stdin, in case the command that was running was a
synchronous command.  That's done in the event loop's actual loop
(event-loop.c:start_event_loop).  However, if a foreground execution
command is run before the event loop starts and throws, nothing is
presently reenabling stdin, which leaves sync_execution set.

When we do start the event loop, because sync_execution is still
(mistakenly) set, display_gdb_prompt removes the readline input
callback, even though stdin is registered in the event loop.  Any
input from here on results in readline aborting.

Such commands are run through catch_command_errors,
catch_command_errors_const, so add the tweak there.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* main.c: Include event-top.h.
	(handle_command_errors): New function.
	(catch_command_errors, catch_command_errors_const): Use it.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	* gdb.base/paginate-execution-startup.c: New file.
	* gdb.base/paginate-execution-startup.exp: New file.
	* lib/gdb.exp (pagination_prompt): New global.
	(default_gdb_spawn): New procedure, factored out from
	default_gdb_spawn.
	(default_gdb_start): Adjust to call default_gdb_spawn.
	(gdb_spawn): New procedure.
---
 gdb/main.c                                         |  30 +++-
 .../gdb.base/paginate-execution-startup.c          |  32 ++++
 .../gdb.base/paginate-execution-startup.exp        | 189 +++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                          |  61 +++++--
 4 files changed, 292 insertions(+), 20 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-execution-startup.exp

diff --git a/gdb/main.c b/gdb/main.c
index 1d77bd3..b51ff89 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -46,6 +46,7 @@
 #include "filenames.h"
 #include "filestuff.h"
 #include <signal.h>
+#include "event-top.h"
 
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
@@ -337,6 +338,25 @@ captured_command_loop (void *data)
   return 1;
 }
 
+/* Handle command errors thrown from within
+   catch_command_errors/catch_command_errors_const.  */
+
+static int
+handle_command_errors (volatile struct gdb_exception e)
+{
+  if (e.reason < 0)
+    {
+      exception_print (gdb_stderr, e);
+
+      /* If any exception escaped to here, we better enable stdin.
+	 Otherwise, any command that calls async_disable_stdin, and
+	 then throws, will leave stdin inoperable.  */
+      async_enable_stdin ();
+      return 0;
+    }
+  return 1;
+}
+
 /* Type of the command callback passed to catch_command_errors.  */
 
 typedef void (catch_command_errors_ftype) (char *, int);
@@ -353,10 +373,7 @@ catch_command_errors (catch_command_errors_ftype *command,
     {
       command (arg, from_tty);
     }
-  exception_print (gdb_stderr, e);
-  if (e.reason < 0)
-    return 0;
-  return 1;
+  return handle_command_errors (e);
 }
 
 /* Type of the command callback passed to catch_command_errors_const.  */
@@ -375,10 +392,7 @@ catch_command_errors_const (catch_command_errors_const_ftype *command,
     {
       command (arg, from_tty);
     }
-  exception_print (gdb_stderr, e);
-  if (e.reason < 0)
-    return 0;
-  return 1;
+  return handle_command_errors (e);
 }
 
 /* Arguments of --command option and its counterpart.  */
diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.c b/gdb/testsuite/gdb.base/paginate-execution-startup.c
new file mode 100644
index 0000000..e741785
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-execution-startup.c
@@ -0,0 +1,32 @@
+/* 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>
+
+static void
+after_sleep (void)
+{
+  return; /* after sleep */
+}
+
+int
+main (void)
+{
+  sleep (3);
+  after_sleep ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/paginate-execution-startup.exp b/gdb/testsuite/gdb.base/paginate-execution-startup.exp
new file mode 100644
index 0000000..dc713ec
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-execution-startup.exp
@@ -0,0 +1,189 @@
+# Copyright (C) 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/>.
+
+# A collection of tests related to pagination resulting from running
+# execution commands directly from the command line, with "-ex".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+set file_arg $binfile
+if [is_remote host] {
+  set file_arg [remote_download host $file_arg]
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+
+# Returns true if the board can 'gdb -ex "start"', false otherwise.
+
+proc probe_can_run_cmdline  {} {
+    global srcfile file_arg
+    global saved_gdbflags GDBFLAGS
+    global gdb_prompt
+
+    set GDBFLAGS $saved_gdbflags
+    append GDBFLAGS " -ex \"set height 0\""
+    append GDBFLAGS " -ex \"start\""
+    append GDBFLAGS " --args \"$file_arg\""
+
+    with_test_prefix "probe support" {
+	set test "run to main"
+
+	gdb_exit
+	set res [gdb_spawn]
+	if { $res != 0} {
+	    fail $test
+	    return -1
+	}
+
+	set res -1
+	gdb_test_multiple "" $test {
+	    -re "main .* at .*$srcfile.*$gdb_prompt $" {
+		set res 1
+		pass $test
+	    }
+	    -re "Don't know how to run.*$gdb_prompt $" {
+		set res 0
+		unsupported $test
+	    }
+	}
+
+	# In case the board file wants to send further commands.
+	gdb_test_no_output "set height unlimited"
+	return $res
+    }
+}
+
+# Check that we handle pagination correctly when it triggers due to an
+# execution command entered directly on the command line.
+
+proc test_fg_execution_pagination_return {} {
+    global file_arg
+    global saved_gdbflags GDBFLAGS
+    global gdb_prompt pagination_prompt
+
+    set GDBFLAGS $saved_gdbflags
+    append GDBFLAGS " -ex \"set height 2\""
+    append GDBFLAGS " -ex \"start\""
+    append GDBFLAGS " --args \"$file_arg\""
+
+    with_test_prefix "return" {
+	set test "run to pagination"
+
+	gdb_exit
+	set res [gdb_spawn]
+	if { $res != 0} {
+	    fail $test
+	    return $res
+	}
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		pass $test
+	    }
+	    -re "$gdb_prompt $" {
+		fail $test
+	    }
+	}
+
+	send_gdb "\n"
+
+	set saw_pagination_prompt 0
+	set test "send \\n to GDB"
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		set saw_pagination_prompt 1
+		send_gdb "\n"
+		exp_continue
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	    -re "$gdb_prompt $" {
+		gdb_assert $saw_pagination_prompt $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+
+	# In case the board file wants to send further commands.
+	gdb_test_no_output "set height unlimited"
+    }
+}
+
+# Check that we handle canceling pagination correctly when it triggers
+# due to an execution command entered directly on the command line.
+
+proc test_fg_execution_pagination_cancel { how } {
+    global file_arg
+    global saved_gdbflags GDBFLAGS
+    global gdb_prompt pagination_prompt
+
+    set GDBFLAGS $saved_gdbflags
+
+    append GDBFLAGS " -ex \"set height 2\""
+    append GDBFLAGS " -ex \"start\""
+    append GDBFLAGS " --args \"$file_arg\""
+
+    with_test_prefix "cancel with $how" {
+	set test "run to pagination"
+
+	gdb_exit
+	set res [gdb_spawn]
+	if { $res != 0} {
+	    fail $test
+	    return $res
+	}
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		pass $test
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	set test "cancel pagination"
+	if { $how == "ctrl-c" } {
+	    send_gdb "\003"
+	} else {
+	    send_gdb "q\n"
+
+	}
+	gdb_test_multiple "" $test {
+	    -re "Quit\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+
+	# In case the board file wants to send further commands.
+	gdb_test_no_output "set height unlimited"
+    }
+}
+
+if {[probe_can_run_cmdline] > 0} {
+    test_fg_execution_pagination_return
+    test_fg_execution_pagination_cancel "ctrl-c"
+    test_fg_execution_pagination_cancel "quit"
+}
+
+set GDBFLAGS $saved_gdbflags
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 36cbf05..3533ee3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -69,6 +69,9 @@ if ![info exists gdb_prompt] then {
     set gdb_prompt "\[(\]gdb\[)\]"
 }
 
+# A regexp that matches the pagination prompt.
+set pagination_prompt "---Type <return> to continue, or q <return> to quit---"
+
 # The variable fullname_syntax_POSIX is a regexp which matches a POSIX 
 # absolute path ie. /foo/ 
 set fullname_syntax_POSIX {/[^\n]*/}
@@ -1425,19 +1428,12 @@ proc gdb_file_cmd { arg } {
     }
 }
 
-#
-# start gdb -- start gdb running, default procedure
-#
-# When running over NFS, particularly if running many simultaneous
-# tests on different hosts all using the same server, things can
-# get really slow.  Give gdb at least 3 minutes to start up.
-#
-proc default_gdb_start { } {
-    global verbose use_gdb_stub
+# Default gdb_spawn procedure.
+
+proc default_gdb_spawn { } {
+    global use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
-    global gdb_prompt
-    global timeout
     global gdb_spawn_id
 
     gdb_stop_suppressing_tests
@@ -1468,21 +1464,45 @@ proc default_gdb_start { } {
 	perror "Spawning $GDB failed."
 	return 1
     }
+    set gdb_spawn_id -1
+    return 0
+}
+
+# Default gdb_start procedure.
+
+proc default_gdb_start { } {
+    global gdb_prompt
+    global gdb_spawn_id
+
+    if [info exists gdb_spawn_id] {
+	return 0
+    }
+
+    set res [gdb_spawn]
+    if { $res != 0} {
+	return $res
+    }
+
+    # When running over NFS, particularly if running many simultaneous
+    # tests on different hosts all using the same server, things can
+    # get really slow.  Give gdb at least 3 minutes to start up.
     gdb_expect 360 {
 	-re "\[\r\n\]$gdb_prompt $" {
 	    verbose "GDB initialized."
 	}
 	-re "$gdb_prompt $"	{
 	    perror "GDB never initialized."
+	    unset gdb_spawn_id
 	    return -1
 	}
 	timeout	{
 	    perror "(timeout) GDB never initialized after 10 seconds."
 	    remote_close host
+	    unset gdb_spawn_id
 	    return -1
 	}
     }
-    set gdb_spawn_id -1
+
     # force the height to "unlimited", so no pagers get used
 
     send_gdb "set height 0\n"
@@ -3285,6 +3305,23 @@ proc gdb_clear_suppressed { } {
     set suppress_flag 0
 }
 
+# Spawn the gdb process.
+#
+# This doesn't expect any output or do any other initialization,
+# leaving those to the caller.
+#
+# Overridable function -- you can override this function in your
+# baseboard file.
+
+proc gdb_spawn { } {
+    default_gdb_spawn
+}
+
+# Start gdb running, wait for prompt, and disable the pagers.
+
+# Overridable function -- you can override this function in your
+# baseboard file.
+
 proc gdb_start { } {
     default_gdb_start
 }
-- 
1.9.3


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb
  2014-07-09 16:45     ` Pedro Alves
@ 2014-07-10  9:26       ` Yao Qi
  0 siblings, 0 replies; 19+ messages in thread
From: Yao Qi @ 2014-07-10  9:26 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 07/10/2014 12:45 AM, Pedro Alves wrote:
> Looks like we already have a ton of tests using sleep/usleep,
> so such a fix would best be done across the board.  Perhaps
> even by adding a sleep.h header to testsuite/lib/ or some such ?
> Or if not needed with newer toolchains, just ignore it.  :-)

Yes, sleep has been widely used in the testsuite, but some of these
tests are not really compiled nor executed on mingw.  I'll take a
look if it makes trouble, but I am fine to ignore it at current stage.

> 
>>> >> +proc probe_can_run_cmdline  {} {
>>> >> +    global srcfile binfile
>>> >> +    global saved_gdbflags GDBFLAGS
>>> >> +    global gdb_prompt
>>> >> +
>>> >> +    set GDBFLAGS $saved_gdbflags
>>> >> +    append GDBFLAGS " -ex \"set height 0\""
>>> >> +    append GDBFLAGS " -ex \"start\""
>>> >> +    append GDBFLAGS " --args \"$binfile\""
>> > 
>> > This doesn't work on remote host, as we haven't copy binfile from build
>> > to host yet.  We can do this after build_executable, and pass $file_arg
>> > to each proc instead of using $binfile in each proc.
> Thanks.  See the new version below.

It looks good to me.  I don't have other comments.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND/PROPER PATCH 0/9] pagination/readline/async issues
  2014-07-03 15:31 ` [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
@ 2014-07-14 22:38   ` Pedro Alves
  0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-14 22:38 UTC (permalink / raw)
  To: gdb-patches

On 07/03/2014 04:31 PM, Pedro Alves wrote:
> I've now pushed this to:
> 
>  git@github.com:palves/gdb.git palves/pr17072_pagination_async
> 

... and pushed it to both master and the 7.8 branch now.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/9] Background execution + pagination aborts readline/gdb
  2014-07-03 15:19 ` [PATCH 6/9] Background execution + pagination aborts readline/gdb Pedro Alves
@ 2014-09-13  0:05   ` Sergio Durigan Junior
  0 siblings, 0 replies; 19+ messages in thread
From: Sergio Durigan Junior @ 2014-09-13  0:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Thursday, July 03 2014, Pedro Alves wrote:

> If pagination occurs as result of output sent as response to a target
> event while the target is executing in the background, subsequent
> input aborts readline/gdb:
>
>  $ gdb program
>  ...
>  (gdb) continue&
>  Continuing.
>  (gdb)
>  ---Type <return> to continue, or q <return> to quit---
>  *return*
>  ---Type <return> to continue, or q <return> to quit---
>  Breakpoint 2, after_sleep () at paginate-bg-execution.c:21
>  ---Type <return> to continue, or q <return> to quit---
>  21        return; /* after sleep */
>  p 1
>  readline: readline_callback_read_char() called with no handler!
>  *abort/SIGABRT*
>  $

Hi Pedro,

FWIW, this patch caused
<https://sourceware.org/bugzilla/show_bug.cgi?id=17372>.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 6/9] Background execution + pagination aborts readline/gdb
  2014-07-03 15:13 [PATCH 0/9] pagination/readline/async issues Pedro Alves
@ 2014-07-03 15:14 ` Pedro Alves
  0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2014-07-03 15:14 UTC (permalink / raw)
  To: gdb-patches

If pagination occurs as result of output sent as response to a target
event while the target is executing in the background, subsequent
input aborts readline/gdb:

 $ gdb program
 ...
 (gdb) continue&
 Continuing.
 (gdb)
 ---Type <return> to continue, or q <return> to quit---
 *return*
 ---Type <return> to continue, or q <return> to quit---
 Breakpoint 2, after_sleep () at paginate-bg-execution.c:21
 ---Type <return> to continue, or q <return> to quit---
 21        return; /* after sleep */
 p 1
 readline: readline_callback_read_char() called with no handler!
 *abort/SIGABRT*
 $

gdb_readline_wrapper_line removes the handler after a line is
processed.  Usually, we'll end up re-displaying the prompt, and that
reinstalls the handler.  But if the output is comming out of handling
a stop event, we don't re-display the prompt, and nothing restores the
handler.  So the next input wakes up the event loop and calls into
readline, which aborts.

We should do better with the prompt handling while the target is
running (I think we should coordinate with readline, and
hide/redisplay it around output), but that's a more invasive change
better done post 7.8, so this patch is conservative and just
reinstalls the handler as soon as we're out of the readline line
callback.

gdb/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* top.c (gdb_readline_wrapper_line): Tweak comment.
	(gdb_readline_wrapper_cleanup): If readline is enabled, reinstall
	the input handler callback.

gdb/testsuite/
2014-07-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17072
	* gdb.base/paginate-bg-execution.c: New file.
 	* gdb.base/paginate-bg-execution.exp: New file.
---
 gdb/testsuite/gdb.base/paginate-bg-execution.c   |  30 ++++++
 gdb/testsuite/gdb.base/paginate-bg-execution.exp | 121 +++++++++++++++++++++++
 gdb/top.c                                        |   9 +-
 3 files changed, 159 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.c
 create mode 100644 gdb/testsuite/gdb.base/paginate-bg-execution.exp

diff --git a/gdb/testsuite/gdb.base/paginate-bg-execution.c b/gdb/testsuite/gdb.base/paginate-bg-execution.c
new file mode 100644
index 0000000..7457b18
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-bg-execution.c
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+static void
+after_sleep (void)
+{
+  return; /* after sleep */
+}
+
+int
+main (void)
+{
+  sleep (3);
+  after_sleep ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/paginate-bg-execution.exp b/gdb/testsuite/gdb.base/paginate-bg-execution.exp
new file mode 100644
index 0000000..ac62727
--- /dev/null
+++ b/gdb/testsuite/gdb.base/paginate-bg-execution.exp
@@ -0,0 +1,121 @@
+# Copyright (C) 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/>.
+
+# A collection of tests related to running execution commands directly
+# from the command line, with "-ex".
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Check that we handle pagination correctly when it triggers due to an
+# background execution command entered directly on the command line.
+
+proc test_bg_execution_pagination_return {} {
+    global binfile
+    global pagination_prompt
+
+    with_test_prefix "paginate" {
+	clean_restart $binfile
+
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	gdb_test "b after_sleep"
+
+	gdb_test_no_output "set height 2"
+
+	gdb_test "continue&" "Continuing\."
+
+	set test "pagination handled, breakpoint hit"
+	set saw_pagination_prompt 0
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		set saw_pagination_prompt 1
+		send_gdb "\n"
+		exp_continue
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an
+		# error.
+		exp_continue
+	    }
+	    -re "after sleep\[^\r\n\]+\r\n$" {
+		gdb_assert $saw_pagination_prompt $test
+	    }
+	}
+
+	# GDB used to crash here.
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+# Check that we handle canceling pagination correctly when it triggers
+# due to a background execution command entered directly on the
+# command line.
+
+proc test_bg_execution_pagination_cancel { how } {
+    global binfile
+    global gdb_prompt pagination_prompt
+
+    with_test_prefix "cancel with $how" {
+	clean_restart $binfile
+
+	if ![runto_main] then {
+	    fail "Can't run to main"
+	    return 0
+	}
+
+	gdb_test "b after_sleep"
+
+	gdb_test_no_output "set height 2"
+
+	gdb_test "continue&" "Continuing\."
+
+	set test "continue& paginates"
+	gdb_test_multiple "" $test {
+	    -re "$pagination_prompt$" {
+		pass $test
+	    }
+	    -notransfer -re "<return>" {
+		# Otherwise gdb_test_multiple considers this an error.
+		exp_continue
+	    }
+	}
+
+	set test "cancel pagination"
+	if { $how == "ctrl-c" } {
+	    send_gdb "\003"
+	} else {
+	    send_gdb "q\n"
+
+	}
+	gdb_test_multiple "" $test {
+	    -re "Quit\r\n$gdb_prompt $" {
+		pass $test
+	    }
+	}
+
+	gdb_test "p 1" " = 1" "GDB accepts further input"
+    }
+}
+
+test_bg_execution_pagination_return
+test_bg_execution_pagination_cancel "ctrl-c"
+test_bg_execution_pagination_cancel "quit"
diff --git a/gdb/top.c b/gdb/top.c
index 722eb55..93a4a16 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -756,7 +756,8 @@ gdb_readline_wrapper_line (char *line)
   after_char_processing_hook = NULL;
 
   /* Prevent parts of the prompt from being redisplayed if annotations
-     are enabled, and readline's state getting out of sync.  */
+     are enabled, and readline's state getting out of sync.  We'll
+     restore it in gdb_readline_wrapper_cleanup.  */
   if (async_command_editing_p)
     rl_callback_handler_remove ();
 }
@@ -776,6 +777,12 @@ gdb_readline_wrapper_cleanup (void *arg)
 
   gdb_assert (input_handler == gdb_readline_wrapper_line);
   input_handler = cleanup->handler_orig;
+
+  /* Reinstall INPUT_HANDLER in readline, without displaying a
+     prompt.  */
+  if (async_command_editing_p)
+    rl_callback_handler_install (NULL, input_handler);
+
   gdb_readline_wrapper_result = NULL;
   gdb_readline_wrapper_done = 0;
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-09-13  0:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
2014-07-03 15:19 ` [PATCH 1/9] Put the inferior's terminal settings in effect while running (fg) infcalls Pedro Alves
2014-07-03 15:19 ` [PATCH 3/9] Move catch_command_errors and catch_command_errors_const to main.c Pedro Alves
2014-07-03 15:19 ` [PATCH 9/9] Put GDB's terminal settings into effect when paginating Pedro Alves
2014-07-03 15:19 ` [PATCH 6/9] Background execution + pagination aborts readline/gdb Pedro Alves
2014-09-13  0:05   ` Sergio Durigan Junior
2014-07-03 15:19 ` [PATCH 2/9] Eliminate exceptions.c:print_any_exception Pedro Alves
2014-07-03 20:38   ` Tom Tromey
2014-07-03 15:19 ` [PATCH 8/9] Fix double prompt Pedro Alves
2014-07-03 15:19 ` [PATCH 5/9] Canceling pagination caused by execution command from command line aborts readline/gdb Pedro Alves
2014-07-04  6:11   ` Yao Qi
2014-07-09 16:45     ` Pedro Alves
2014-07-10  9:26       ` Yao Qi
2014-07-03 15:27 ` [PATCH 4/9] testsuite: Introduce gdb_assert Pedro Alves
2014-07-03 20:41   ` Tom Tromey
2014-07-03 15:31 ` [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
2014-07-14 22:38   ` Pedro Alves
2014-07-03 15:40 ` [PATCH 7/9] Remove the target from the event loop while in secondary prompts Pedro Alves
  -- strict thread matches above, loose matches on Subject: below --
2014-07-03 15:13 [PATCH 0/9] pagination/readline/async issues Pedro Alves
2014-07-03 15:14 ` [PATCH 6/9] Background execution + pagination aborts readline/gdb Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).