Remove the target from the event loop while in secondary prompts
authorPedro Alves <palves@redhat.com>
Mon, 14 Jul 2014 18:55:32 +0000 (19:55 +0100)
committerPedro Alves <palves@redhat.com>
Mon, 14 Jul 2014 19:33:16 +0000 (20:33 +0100)
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-14  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-14  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/ChangeLog
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/paginate-inferior-exit.c [new file with mode: 0644]
gdb/testsuite/gdb.base/paginate-inferior-exit.exp [new file with mode: 0644]
gdb/top.c

index cd13f70db268f7dea0b25fcd17283cf6c73594ed..96d507163a06233b638a9af1bf0e3320eb4aec41 100644 (file)
@@ -1,3 +1,14 @@
+2014-07-14  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.
+
 2014-07-14  Pedro Alves  <palves@redhat.com>
 
        PR gdb/17072
index f6c963df1957e3c321c65cf38e6cd285df9eee11..6d3fa569b982e2218d40947395a2051a1ce5fb0d 100644 (file)
@@ -1,3 +1,9 @@
+2014-07-14  Pedro Alves  <palves@redhat.com>
+
+       PR gdb/17072
+       * gdb.base/paginate-inferior-exit.c: New file.
+       * gdb.base/paginate-inferior-exit.exp: New file.
+
 2014-07-14  Pedro Alves  <palves@redhat.com>
 
        PR gdb/17072
diff --git a/gdb/testsuite/gdb.base/paginate-inferior-exit.c b/gdb/testsuite/gdb.base/paginate-inferior-exit.c
new file mode 100644 (file)
index 0000000..e741785
--- /dev/null
@@ -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-inferior-exit.exp b/gdb/testsuite/gdb.base/paginate-inferior-exit.exp
new file mode 100644 (file)
index 0000000..0e37be9
--- /dev/null
@@ -0,0 +1,85 @@
+# 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"
+
+       # In case the board file wants to send further commands.
+       gdb_test_no_output "set height unlimited"
+    }
+}
+
+test_paginate_inferior_exited
index 93a4a1694cf76989da9e1083d9f4be239f239fab..19c88f9ffddb67d3849b3e44d6beeb7d883e6b5f 100644 (file)
--- 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;