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 8/9] Fix double prompt
Date: Thu, 03 Jul 2014 15:19:00 -0000	[thread overview]
Message-ID: <1404400736-17307-9-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1404400736-17307-1-git-send-email-palves@redhat.com>

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

  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 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 1/9] Put the inferior's terminal settings in effect while running (fg) infcalls Pedro Alves
2014-07-03 15:19 ` Pedro Alves [this message]
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 2/9] Eliminate exceptions.c:print_any_exception Pedro Alves
2014-07-03 20:38   ` Tom Tromey
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:32 ` [PATCH 8/9] Fix double prompt 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-9-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).