[binutils-gdb/gdb-7.10-branch] remote: allow aborting long operations (e.g., file transfers)

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=026ca0231ae6dcc107ec496ed677bd1b00474a2f

commit 026ca0231ae6dcc107ec496ed677bd1b00474a2f
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Aug 25 16:19:56 2015 +0100

    remote: allow aborting long operations (e.g., file transfers)
    
    Currently, when remote debugging, if you type Ctrl-C just while the
    target stopped for an internal event, and GDB is busy doing something
    that takes a while (e.g., fetching chunks of a shared library off of
    the target, with vFile, to process ELF headers and debug info), the
    Ctrl-C is lost.
    
    The patch hooks up the QUIT macro to a new target method that lets the
    target react to the double-Ctrl-C before the event loop is reached,
    which allows reacting to a double-Ctrl-C even when GDB is busy doing
    some long operation and not waiting for a stop reply.  That end result
    is:
    
     (gdb) c
     Continuing.
     ^C
     ^C
     Interrupted while waiting for the program.
     Give up waiting? (y or n) y
     Quit
     (gdb) info threads
       Id   Target Id         Frame
     * 1    Thread 11673      0x00007ffff7deb240 in _dl_debug_state () from target:/lib64/ld-linux-x86-64.so.2
     (gdb)
    
    If, however, GDB is waiting for a stop reply (because the target has
    been resumed, with e.g., vCont;c), but the target isn't responding, we
    now get:
    
     (gdb) c
     Continuing.
     ^C
     ^C
     The target is not responding to interrupt requests.
     Stop debugging it? (y or n) y
     Disconnected from target.
     (gdb) info threads
     No threads.
    
    This offers to disconnect, because when we're waiting for a stop
    reply, there's nothing else we can send the target other than an
    interrupt request.  And if that doesn't work, there's nothing else we
    can do.
    
    The Ctrl-C is presently lost because until we get to a user-visible
    stop, the SIGINT handler that is installed is the one that forwards
    the interrupt to the remote side, with the \003 "packet" [1].  But,
    gdbserver ignores an interrupt request if the program is stopped.
    Still, even if it didn't, the server can only report back a
    stop-because-of-SIGINT when the program is next resumed.  And it may
    take a while to actually re-resume the target.
    
    [1] - In the old sync days, the remote target would react to a
    double-Ctrl-C by asking users whether they wanted to give up waiting
    and disconnect.  The code is still there, but it it isn't reacheable
    on most hosts, which support serial connections in async mode
    (probably only DJGPP doesn't).  Even then, in sync mode, remote.c's
    SIGINT handler is only installed while the target is resumed, and is
    removed as soon as the target sends back a stop reply.  That means
    that a Ctrl-C just while GDB is processing an internal event can end
    up with an odd "Quit" at the prompt instead of "Program stopped by
    SIGINT".  In contrast, in async mode, remote.c's SIGINT handler is set
    up as long as target_terminal_inferior or
    target_terminal_ours_for_output are in effect (IOW, until we get a
    user-visible stop and call target_terminal_ours), so the user
    shouldn't get back a spurious Quit.  However, it's still desirable to
    be able to interrupt a long-running GDB operation, if GDB takes a
    while to re-resume the target or get back to the event loop.
    
    Tested on x86_64 Fedora 20.
    
    gdb/ChangeLog:
    2015-08-24  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/18804
    	* defs.h (maybe_quit): Declare.
    	(QUIT): Now calls maybe_quit.
    	* event-loop.c (clear_async_signal_handler)
    	(async_signal_handler_is_marked): New functions.
    	* event-loop.h (async_signal_handler_is_marked)
    	(clear_async_signal_handler): New declarations.
    	* remote.c (remote_check_pending_interrupt): New function.
    	(interrupt_query): Use make_cleanup_restore_target_terminal.  No
    	longer check whether the target is async.  If waiting for a stop
    	reply, and a Ctrl-C as been sent to the target, offer to
    	disconnect, and throw TARGET_CLOSE_ERROR instead of a quit.
    	Otherwise do not disconnect and throw a quit.
    	(_initialize_remote): Install remote_check_pending_interrupt as
    	to_check_pending_interrupt.
    	* target.c (target_check_pending_interrupt): New function.
    	* target.h (struct target_ops) <to_check_pending_interrupt>: New
    	field.
    	(target_check_pending_interrupt): New declaration.
    	* utils.c (maybe_quit): New function.
    	* target-delegates.c: Regenerate.

Diff:
---
 gdb/ChangeLog          | 24 ++++++++++++++++++++++++
 gdb/defs.h             | 20 +++++++++-----------
 gdb/event-loop.c       | 16 ++++++++++++++++
 gdb/event-loop.h       |  9 +++++++++
 gdb/remote.c           | 40 ++++++++++++++++++++++++++++++----------
 gdb/target-delegates.c | 26 ++++++++++++++++++++++++++
 gdb/target.c           |  8 ++++++++
 gdb/target.h           | 10 ++++++++++
 gdb/utils.c            | 12 ++++++++++++
 9 files changed, 144 insertions(+), 21 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4813c04..71664a0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,27 @@
+2015-08-24  Pedro Alves  <palves@redhat.com>
+
+	PR gdb/18804
+	* defs.h (maybe_quit): Declare.
+	(QUIT): Now calls maybe_quit.
+	* event-loop.c (clear_async_signal_handler)
+	(async_signal_handler_is_marked): New functions.
+	* event-loop.h (async_signal_handler_is_marked)
+	(clear_async_signal_handler): New declarations.
+	* remote.c (remote_check_pending_interrupt): New function.
+	(interrupt_query): Use make_cleanup_restore_target_terminal.  No
+	longer check whether the target is async.  If waiting for a stop
+	reply, and a Ctrl-C as been sent to the target, offer to
+	disconnect, and throw TARGET_CLOSE_ERROR instead of a quit.
+	Otherwise do not disconnect and throw a quit.
+	(_initialize_remote): Install remote_check_pending_interrupt as
+	to_check_pending_interrupt.
+	* target.c (target_check_pending_interrupt): New function.
+	* target.h (struct target_ops) <to_check_pending_interrupt>: New
+	field.
+	(target_check_pending_interrupt): New declaration.
+	* utils.c (maybe_quit): New function.
+	* target-delegates.c: Regenerate.
+
 2015-08-21  Gary Benson  <gbenson@redhat.com>
 
 	* gdb_bfd.c (gdb_bfd_iovec_fileio_pread): Add QUIT call.
diff --git a/gdb/defs.h b/gdb/defs.h
index 32b08bb..ccdab18 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -149,17 +149,15 @@ extern int immediate_quit;
 
 extern void quit (void);
 
-/* FIXME: cagney/2000-03-13: It has been suggested that the peformance
-   benefits of having a ``QUIT'' macro rather than a function are
-   marginal.  If the overhead of a QUIT function call is proving
-   significant then its calling frequency should probably be reduced
-   [kingdon].  A profile analyzing the current situtation is
-   needed.  */
-
-#define QUIT { \
-  if (check_quit_flag () || sync_quit_force_run) quit (); \
-  if (deprecated_interactive_hook) deprecated_interactive_hook (); \
-}
+/* Helper for the QUIT macro.  */
+
+extern void maybe_quit (void);
+
+/* Check whether a Ctrl-C was typed, and if so, call quit.  The target
+   is given a chance to process the Ctrl-C.  E.g., it may detect that
+   repeated Ctrl-C requests were issued, and choose to close the
+   connection.  */
+#define QUIT maybe_quit ()
 
 /* * Languages represented in the symbol table and elsewhere.
    This should probably be in language.h, but since enum's can't
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 9ac4908..9e54c94 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -908,6 +908,22 @@ mark_async_signal_handler (async_signal_handler * async_handler_ptr)
   async_handler_ptr->ready = 1;
 }
 
+/* See event-loop.h.  */
+
+void
+clear_async_signal_handler (async_signal_handler *async_handler_ptr)
+{
+  async_handler_ptr->ready = 0;
+}
+
+/* See event-loop.h.  */
+
+int
+async_signal_handler_is_marked (async_signal_handler *async_handler_ptr)
+{
+  return async_handler_ptr->ready;
+}
+
 /* Call all the handlers that are ready.  Returns true if any was
    indeed ready.  */
 static int
diff --git a/gdb/event-loop.h b/gdb/event-loop.h
index 6ae4013..598d0da 100644
--- a/gdb/event-loop.h
+++ b/gdb/event-loop.h
@@ -102,6 +102,15 @@ void call_async_signal_handler (struct async_signal_handler *handler);
    below, with IMMEDIATE_P == 0.  */
 void mark_async_signal_handler (struct async_signal_handler *handler);
 
+/* Returns true if HANDLER is marked ready.  */
+
+extern int
+  async_signal_handler_is_marked (struct async_signal_handler *handler);
+
+/* Mark HANDLER as NOT ready.  */
+
+extern void clear_async_signal_handler (struct async_signal_handler *handler);
+
 /* Wrapper for the body of signal handlers.  Call this function from
    any SIGINT handler which needs to access GDB data structures or
    escape via longjmp.  If IMMEDIATE_P is set, this triggers either
diff --git a/gdb/remote.c b/gdb/remote.c
index 8fc9002..610da1e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5191,6 +5191,20 @@ async_handle_remote_sigint_twice (int sig)
   gdb_call_async_signal_handler (async_sigint_remote_twice_token, 0);
 }
 
+/* Implementation of to_check_pending_interrupt.  */
+
+static void
+remote_check_pending_interrupt (struct target_ops *self)
+{
+  struct async_signal_handler *token = async_sigint_remote_twice_token;
+
+  if (async_signal_handler_is_marked (token))
+    {
+      clear_async_signal_handler (token);
+      call_async_signal_handler (token);
+    }
+}
+
 /* Perform the real interruption of the target execution, in response
    to a ^C.  */
 static void
@@ -5342,24 +5356,29 @@ remote_stop (struct target_ops *self, ptid_t ptid)
 static void
 interrupt_query (void)
 {
+  struct remote_state *rs = get_remote_state ();
+  struct cleanup *old_chain;
+
+  old_chain = make_cleanup_restore_target_terminal ();
   target_terminal_ours ();
 
-  if (target_is_async_p ())
-    {
-      signal (SIGINT, handle_sigint);
-      quit ();
-    }
-  else
+  if (rs->waiting_for_stop_reply && rs->ctrlc_pending_p)
     {
-      if (query (_("Interrupted while waiting for the program.\n\
-Give up (and stop debugging it)? ")))
+      if (query (_("The target is not responding to interrupt requests.\n"
+		   "Stop debugging it? ")))
 	{
 	  remote_unpush_target ();
-	  quit ();
+	  throw_error (TARGET_CLOSE_ERROR, _("Disconnected from target."));
 	}
     }
+  else
+    {
+      if (query (_("Interrupted while waiting for the program.\n"
+		   "Give up waiting? ")))
+	quit ();
+    }
 
-  target_terminal_inferior ();
+  do_cleanups (old_chain);
 }
 
 /* Enable/disable target terminal ownership.  Most targets can use
@@ -12404,6 +12423,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_get_ada_task_ptid = remote_get_ada_task_ptid;
   remote_ops.to_stop = remote_stop;
+  remote_ops.to_check_pending_interrupt = remote_check_pending_interrupt;
   remote_ops.to_xfer_partial = remote_xfer_partial;
   remote_ops.to_rcmd = remote_rcmd;
   remote_ops.to_pid_to_exec_file = remote_pid_to_exec_file;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 36eacbf..a2a3e48 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1537,6 +1537,28 @@ debug_stop (struct target_ops *self, ptid_t arg1)
 }
 
 static void
+delegate_check_pending_interrupt (struct target_ops *self)
+{
+  self = self->beneath;
+  self->to_check_pending_interrupt (self);
+}
+
+static void
+tdefault_check_pending_interrupt (struct target_ops *self)
+{
+}
+
+static void
+debug_check_pending_interrupt (struct target_ops *self)
+{
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_check_pending_interrupt (...)\n", debug_target.to_shortname);
+  debug_target.to_check_pending_interrupt (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_check_pending_interrupt (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (")\n", gdb_stdlog);
+}
+
+static void
 delegate_rcmd (struct target_ops *self, const char *arg1, struct ui_file *arg2)
 {
   self = self->beneath;
@@ -3989,6 +4011,8 @@ install_delegators (struct target_ops *ops)
     ops->to_thread_name = delegate_thread_name;
   if (ops->to_stop == NULL)
     ops->to_stop = delegate_stop;
+  if (ops->to_check_pending_interrupt == NULL)
+    ops->to_check_pending_interrupt = delegate_check_pending_interrupt;
   if (ops->to_rcmd == NULL)
     ops->to_rcmd = delegate_rcmd;
   if (ops->to_pid_to_exec_file == NULL)
@@ -4224,6 +4248,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_extra_thread_info = tdefault_extra_thread_info;
   ops->to_thread_name = tdefault_thread_name;
   ops->to_stop = tdefault_stop;
+  ops->to_check_pending_interrupt = tdefault_check_pending_interrupt;
   ops->to_rcmd = default_rcmd;
   ops->to_pid_to_exec_file = tdefault_pid_to_exec_file;
   ops->to_log_command = tdefault_log_command;
@@ -4372,6 +4397,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_extra_thread_info = debug_extra_thread_info;
   ops->to_thread_name = debug_thread_name;
   ops->to_stop = debug_stop;
+  ops->to_check_pending_interrupt = debug_check_pending_interrupt;
   ops->to_rcmd = debug_rcmd;
   ops->to_pid_to_exec_file = debug_pid_to_exec_file;
   ops->to_log_command = debug_log_command;
diff --git a/gdb/target.c b/gdb/target.c
index 65fe7f8..4dd991a 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3297,6 +3297,14 @@ target_stop (ptid_t ptid)
   (*current_target.to_stop) (&current_target, ptid);
 }
 
+/* See target.h.  */
+
+void
+target_check_pending_interrupt (void)
+{
+  (*current_target.to_check_pending_interrupt) (&current_target);
+}
+
 /* See target/target.h.  */
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index b9dca55..46d52d1 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -636,6 +636,8 @@ struct target_ops
       TARGET_DEFAULT_RETURN (NULL);
     void (*to_stop) (struct target_ops *, ptid_t)
       TARGET_DEFAULT_IGNORE ();
+    void (*to_check_pending_interrupt) (struct target_ops *)
+      TARGET_DEFAULT_IGNORE ();
     void (*to_rcmd) (struct target_ops *,
 		     const char *command, struct ui_file *output)
       TARGET_DEFAULT_FUNC (default_rcmd);
@@ -1668,6 +1670,14 @@ extern void target_update_thread_list (void);
 
 extern void target_stop (ptid_t ptid);
 
+/* Some targets install their own SIGINT handler while the target is
+   running.  This method is called from the QUIT macro to give such
+   targets a chance to process a Ctrl-C.  The target may e.g., choose
+   to interrupt the (potentially) long running operation, or give up
+   waiting and disconnect.  */
+
+extern void target_check_pending_interrupt (void);
+
 /* Send the specified COMMAND to the target's monitor
    (shell,interpreter) for execution.  The result of the query is
    placed in OUTBUF.  */
diff --git a/gdb/utils.c b/gdb/utils.c
index acb4c7d..ffac4ee 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1041,6 +1041,18 @@ quit (void)
 #endif
 }
 
+/* See defs.h.  */
+
+void
+maybe_quit (void)
+{
+  if (check_quit_flag () || sync_quit_force_run)
+    quit ();
+  if (deprecated_interactive_hook)
+    deprecated_interactive_hook ();
+  target_check_pending_interrupt ();
+}
+
 
 /* Called when a memory allocation fails, with the number of bytes of
    memory requested in SIZE.  */