[binutils-gdb] linux-nat: Eliminate custom target_terminal_{inferior, ours}, stop using set_sigint_trap

[Pedro Alves <palves at sourceware dot org>]

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

commit 9c3a5d9319648db16b30a91253ad02d41d242cef
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Jan 30 14:23:51 2018 +0000

    linux-nat: Eliminate custom target_terminal_{inferior,ours}, stop using set_sigint_trap
    
    This patch gets rid of linux-nat.c's custom
    target_terminal_inferior/target_terminal_ours implementations.
    
    The only remaining reason those overrides exist is to install
    clear_sigint_trap in order to pass Ctrl-C/SIGINT to the inferior
    process in case the inferior is not sharing GDB's terminal (and
    target_wait was called without TARGET_WNOHANG).
    
    However, I think that's better handled by QUIT / target_pass_ctrlc
    nowadays.  Going that route avoids the issue with set_sigint_trap only
    looking at the current inferior to know whether to override SIGINT or
    not, which doesn't really work correctly with multi-inferior in the
    picture.  Also centralizing on a single SIGINT handler as much as
    possible seems better considering a future multi-target world.
    
    Tested on x86-64 GNU/Linux.
    
    gdb/ChangeLog:
    2018-01-30  Pedro Alves  <palves@redhat.com>
    
    	* linux-nat.c (wait_for_signal): New function.
    	(wait_lwp, linux_nat_wait_1): Use it instead of calling sigsuspend
    	directly.
    	(async_terminal_is_ours)
    	(linux_nat_terminal_inferior, linux_nat_terminal_ours): Delete.
    	(linux_nat_add_target): Don't override
    	to_terminal_inferior/to_terminal_ours.

Diff:
---
 gdb/ChangeLog   | 10 ++++++++
 gdb/linux-nat.c | 78 +++++++++++++++++++--------------------------------------
 2 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 12c1874..e640902 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2018-01-30  Pedro Alves  <palves@redhat.com>
+
+	* linux-nat.c (wait_for_signal): New function.
+	(wait_lwp, linux_nat_wait_1): Use it instead of calling sigsuspend
+	directly.
+	(async_terminal_is_ours)
+	(linux_nat_terminal_inferior, linux_nat_terminal_ours): Delete.
+	(linux_nat_add_target): Don't override
+	to_terminal_inferior/to_terminal_ours.
+
 2018-01-29  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	* remote.c (remote_follow_fork): Don't call "detach_inferior".
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b83066c..cc930c6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2152,6 +2152,30 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
 		  _("unknown ptrace event %d"), event);
 }
 
+/* Suspend waiting for a signal.  We're mostly interested in
+   SIGCHLD/SIGINT.  */
+
+static void
+wait_for_signal ()
+{
+  if (debug_linux_nat)
+    fprintf_unfiltered (gdb_stdlog, "linux-nat: about to sigsuspend\n");
+  sigsuspend (&suspend_mask);
+
+  /* If the quit flag is set, it means that the user pressed Ctrl-C
+     and we're debugging a process that is running on a separate
+     terminal, so we must forward the Ctrl-C to the inferior.  (If the
+     inferior is sharing GDB's terminal, then the Ctrl-C reaches the
+     inferior directly.)  We must do this here because functions that
+     need to block waiting for a signal loop forever until there's an
+     event to report before returning back to the event loop.  */
+  if (!target_terminal::is_ours ())
+    {
+      if (check_quit_flag ())
+	target_pass_ctrlc ();
+    }
+}
+
 /* Wait for LP to stop.  Returns the wait status, or 0 if the LWP has
    exited.  */
 
@@ -2217,10 +2241,7 @@ wait_lwp (struct lwp_info *lp)
 	 linux_nat_wait_1 and there if we get called my_waitpid gets called
 	 again before it gets to sigsuspend so we can safely let the handlers
 	 get executed here.  */
-
-      if (debug_linux_nat)
-	fprintf_unfiltered (gdb_stdlog, "WL: about to sigsuspend\n");
-      sigsuspend (&suspend_mask);
+      wait_for_signal ();
     }
 
   restore_child_signals_mask (&prev_mask);
@@ -3404,9 +3425,7 @@ linux_nat_wait_1 (struct target_ops *ops,
       gdb_assert (lp == NULL);
 
       /* Block until we get an event reported with SIGCHLD.  */
-      if (debug_linux_nat)
-	fprintf_unfiltered (gdb_stdlog, "LNW: about to sigsuspend\n");
-      sigsuspend (&suspend_mask);
+      wait_for_signal ();
     }
 
   gdb_assert (lp);
@@ -4429,49 +4448,6 @@ linux_nat_supports_disable_randomization (struct target_ops *self)
 #endif
 }
 
-static int async_terminal_is_ours = 1;
-
-/* target_terminal_inferior implementation.
-
-   This is a wrapper around child_terminal_inferior to add async support.  */
-
-static void
-linux_nat_terminal_inferior (struct target_ops *self)
-{
-  child_terminal_inferior (self);
-
-  /* Calls to target_terminal_*() are meant to be idempotent.  */
-  if (!async_terminal_is_ours)
-    return;
-
-  async_terminal_is_ours = 0;
-  set_sigint_trap ();
-}
-
-/* target_terminal::ours implementation.
-
-   This is a wrapper around child_terminal_ours to add async support (and
-   implement the target_terminal::ours vs target_terminal::ours_for_output
-   distinction).  child_terminal_ours is currently no different than
-   child_terminal_ours_for_output.
-   We leave target_terminal::ours_for_output alone, leaving it to
-   child_terminal_ours_for_output.  */
-
-static void
-linux_nat_terminal_ours (struct target_ops *self)
-{
-  /* GDB should never give the terminal to the inferior if the
-     inferior is running in the background (run&, continue&, etc.),
-     but claiming it sure should.  */
-  child_terminal_ours (self);
-
-  if (async_terminal_is_ours)
-    return;
-
-  clear_sigint_trap ();
-  async_terminal_is_ours = 1;
-}
-
 /* SIGCHLD handler that serves two purposes: In non-stop/async mode,
    so we notice when any child changes state, and notify the
    event-loop; it allows us to use sigsuspend in linux_nat_wait_1
@@ -4820,8 +4796,6 @@ linux_nat_add_target (struct target_ops *t)
   t->to_supports_non_stop = linux_nat_supports_non_stop;
   t->to_always_non_stop_p = linux_nat_always_non_stop_p;
   t->to_async = linux_nat_async;
-  t->to_terminal_inferior = linux_nat_terminal_inferior;
-  t->to_terminal_ours = linux_nat_terminal_ours;
 
   super_close = t->to_close;
   t->to_close = linux_nat_close;