public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 6/9] Background execution + pagination aborts readline/gdb
Date: Thu, 03 Jul 2014 15:19:00 -0000	[thread overview]
Message-ID: <1404400736-17307-7-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1404400736-17307-1-git-send-email-palves@redhat.com>

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

  parent reply	other threads:[~2014-07-03 15:19 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 15:19 [RESEND/PROPER PATCH 0/9] pagination/readline/async issues Pedro Alves
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: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 ` Pedro Alves [this message]
2014-09-13  0:05   ` [PATCH 6/9] Background execution + pagination aborts readline/gdb Sergio Durigan Junior
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1404400736-17307-7-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).