Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 25/30] Do target_terminal_ours in query & friends instead of in all callers
Date: Fri, 18 Mar 2016 19:24:00 -0000	[thread overview]
Message-ID: <1458328714-4938-26-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1458328714-4938-1-git-send-email-palves@redhat.com>

Any time a caller calls query & friends / prompt_for_continue without
ensuring that gdb owns the terminal for input is a bug.  So do that in
defaulted_query / prompt_for_continue directly instead.

An example of a case where we currently miss calling
target_terminal_ours is internal_error.  Ever since defaulted_query
was made to use gdb_readline_callback, there's no way to answer the
internal error query if the internal error happens while the target is
has the terminal:

  (gdb) c
  Continuing.
  .../src/gdb/linux-nat.c:1676: internal-error: linux_nat_resume: Assertion `dummy_counter < 10' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) _

Entering 'y' or 'n' does not work, GDB does not respond.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR gdb/19828
	* gnu-nat.c (inf_validate_task_sc): Don't call
	target_terminal_ours / target_terminal_inferior around query.
	* i386-tdep.c (i386_record_lea_modrm, i386_process_record): Don't
	call target_terminal_ours / target_terminal_inferior around
	yquery.
	* linux-record.c (record_linux_system_call): Don't call
	target_terminal_ours / target_terminal_inferior around yquery.
	* nto-procfs.c (interrupt_query): Don't call target_terminal_ours
	/ target_terminal_inferior around query.
	* record-full.c (record_full_check_insn_num): Remove
	'set_terminal' parameter.  Don't call target_terminal_ours /
	target_terminal_inferior around query.
	(record_full_message, record_full_registers_change)
	(record_full_xfer_partial): Adjust.
	* remote.c (interrupt_query): Don't call target_terminal_ours /
	target_terminal_inferior around query.
	* utils.c (defaulted_query): Install cleanup to restore target
	terminal.  Put target_terminal_ours_for_output in effect while
	defaulted producing, and target_terminal_ours in in effect while
	handling input.
	(prompt_for_continue): Install cleanup to restore target terminal.
	Put target_terminal_ours in in effect while handling input.
---
 gdb/gnu-nat.c      | 12 +++---------
 gdb/i386-tdep.c    | 49 ++++++++++++-------------------------------------
 gdb/linux-record.c | 54 ++++++++++++++----------------------------------------
 gdb/nto-procfs.c   |  4 ----
 gdb/record-full.c  | 22 +++++++---------------
 gdb/remote.c       |  6 ------
 gdb/utils.c        | 16 +++++++++++++---
 7 files changed, 49 insertions(+), 114 deletions(-)

diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index b0b870c..c268732 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -852,15 +852,9 @@ inf_validate_task_sc (struct inf *inf)
 
   if (inf->task->cur_sc < suspend_count)
     {
-      int abort;
-
-      target_terminal_ours ();	/* Allow I/O.  */
-      abort = !query (_("Pid %d has an additional task suspend count of %d;"
-		      " clear it? "), inf->pid,
-		      suspend_count - inf->task->cur_sc);
-      target_terminal_inferior ();	/* Give it back to the child.  */
-
-      if (abort)
+      if (!query (_("Pid %d has an additional task suspend count of %d;"
+		    " clear it? "), inf->pid,
+		  suspend_count - inf->task->cur_sc))
 	error (_("Additional task suspend count left untouched."));
 
       inf->task->cur_sc = suspend_count;
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 4c66edf..a328c18 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4914,17 +4914,12 @@ i386_record_lea_modrm (struct i386_record_s *irp)
     {
       if (record_full_memory_query)
         {
-	  int q;
-
-          target_terminal_ours ();
-          q = yquery (_("\
+          if (yquery (_("\
 Process record ignores the memory change of instruction at address %s\n\
 because it can't get the value of the segment register.\n\
 Do you want to stop the program?"),
-                      paddress (gdbarch, irp->orig_addr));
-            target_terminal_inferior ();
-            if (q)
-              return -1;
+                      paddress (gdbarch, irp->orig_addr)))
+	    return -1;
         }
 
       return 0;
@@ -5805,16 +5800,11 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
         {
           if (record_full_memory_query)
             {
-	      int q;
-
-              target_terminal_ours ();
-              q = yquery (_("\
+              if (yquery (_("\
 Process record ignores the memory change of instruction at address %s\n\
 because it can't get the value of the segment register.\n\
 Do you want to stop the program?"),
-                          paddress (gdbarch, ir.orig_addr));
-              target_terminal_inferior ();
-              if (q)
+                          paddress (gdbarch, ir.orig_addr)))
                 return -1;
             }
 	}
@@ -6479,16 +6469,11 @@ Do you want to stop the program?"),
               /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */
               if (record_full_memory_query)
                 {
-	          int q;
-
-                  target_terminal_ours ();
-                  q = yquery (_("\
+                  if (yquery (_("\
 Process record ignores the memory change of instruction at address %s\n\
 because it can't get the value of the segment register.\n\
 Do you want to stop the program?"),
-                              paddress (gdbarch, ir.orig_addr));
-                  target_terminal_inferior ();
-                  if (q)
+                              paddress (gdbarch, ir.orig_addr)))
                     return -1;
                 }
             }
@@ -7034,17 +7019,12 @@ Do you want to stop the program?"),
 	      {
                 if (record_full_memory_query)
                   {
-	            int q;
-
-                    target_terminal_ours ();
-                    q = yquery (_("\
+                    if (yquery (_("\
 Process record ignores the memory change of instruction at address %s\n\
 because it can't get the value of the segment register.\n\
 Do you want to stop the program?"),
-                                paddress (gdbarch, ir.orig_addr));
-                    target_terminal_inferior ();
-                    if (q)
-                      return -1;
+                                paddress (gdbarch, ir.orig_addr)))
+		      return -1;
                   }
 	      }
 	    else
@@ -7091,16 +7071,11 @@ Do you want to stop the program?"),
 		{
                   if (record_full_memory_query)
                     {
-	              int q;
-
-                      target_terminal_ours ();
-                      q = yquery (_("\
+                      if (yquery (_("\
 Process record ignores the memory change of instruction at address %s\n\
 because it can't get the value of the segment register.\n\
 Do you want to stop the program?"),
-                                  paddress (gdbarch, ir.orig_addr));
-                      target_terminal_inferior ();
-                      if (q)
+                                  paddress (gdbarch, ir.orig_addr)))
                         return -1;
                     }
 		}
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index bf20419..fda7ada 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -254,17 +254,10 @@ record_linux_system_call (enum gdb_syscall syscall,
       break;
 
     case gdb_sys_exit:
-      {
-	int q;
-
-	target_terminal_ours ();
-	q = yquery (_("The next instruction is syscall exit.  "
-		      "It will make the program exit.  "
-		      "Do you want to stop the program?"));
-	target_terminal_inferior ();
-	if (q)
-	  return 1;
-      }
+      if (yquery (_("The next instruction is syscall exit.  "
+		    "It will make the program exit.  "
+		    "Do you want to stop the program?")))
+	return 1;
       break;
 
     case gdb_sys_fork:
@@ -663,17 +656,10 @@ record_linux_system_call (enum gdb_syscall syscall,
       break;
 
     case gdb_sys_reboot:
-      {
-	int q;
-
-	target_terminal_ours ();
-	q = yquery (_("The next instruction is syscall reboot.  "
-		      "It will restart the computer.  "
-		      "Do you want to stop the program?"));
-	target_terminal_inferior ();
-	if (q)
-	  return 1;
-      }
+      if (yquery (_("The next instruction is syscall reboot.  "
+		    "It will restart the computer.  "
+		    "Do you want to stop the program?")))
+	return 1;
       break;
 
     case gdb_old_readdir:
@@ -693,17 +679,12 @@ record_linux_system_call (enum gdb_syscall syscall,
 	regcache_raw_read_unsigned (regcache, tdep->arg2, &len);
 	if (record_full_memory_query)
 	  {
-	    int q;
-
-	    target_terminal_ours ();
-	    q = yquery (_("\
+	    if (yquery (_("\
 The next instruction is syscall munmap.\n\
 It will free the memory addr = 0x%s len = %u.\n\
 It will make record target cannot record some memory change.\n\
 Do you want to stop the program?"),
-			OUTPUT_REG (tmpulongest, tdep->arg1), (int) len);
-	    target_terminal_inferior ();
-	    if (q)
+			OUTPUT_REG (tmpulongest, tdep->arg1), (int) len))
 	      return 1;
 	  }
       }
@@ -1764,17 +1745,10 @@ Do you want to stop the program?"),
       break;
 
     case gdb_sys_exit_group:
-      {
-	int q;
-
-	target_terminal_ours ();
-	q = yquery (_("The next instruction is syscall exit_group.  "
-		      "It will make the program exit.  "
-		      "Do you want to stop the program?"));
-	target_terminal_inferior ();
-	if (q)
-	  return 1;
-      }
+      if (yquery (_("The next instruction is syscall exit_group.  "
+		    "It will make the program exit.  "
+		    "Do you want to stop the program?")))
+	return 1;
       break;
 
     case gdb_sys_lookup_dcookie:
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 992de29..eb7dcfe 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -723,16 +723,12 @@ do_attach (ptid_t ptid)
 static void
 interrupt_query (void)
 {
-  target_terminal_ours ();
-
   if (query (_("Interrupted while waiting for the program.\n\
 Give up (and stop debugging it)? ")))
     {
       target_mourn_inferior ();
       quit ();
     }
-
-  target_terminal_inferior ();
 }
 
 /* The user typed ^C twice.  */
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f6023bf..aec41ad 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -535,26 +535,18 @@ record_full_arch_list_add_end (void)
 }
 
 static void
-record_full_check_insn_num (int set_terminal)
+record_full_check_insn_num (void)
 {
   if (record_full_insn_num == record_full_insn_max_num)
     {
       /* Ask user what to do.  */
       if (record_full_stop_at_limit)
 	{
-	  int q;
-
-	  if (set_terminal)
-	    target_terminal_ours ();
-	  q = yquery (_("Do you want to auto delete previous execution "
+	  if (!yquery (_("Do you want to auto delete previous execution "
 			"log entries when record/replay buffer becomes "
-			"full (record full stop-at-limit)?"));
-	  if (set_terminal)
-	    target_terminal_inferior ();
-	  if (q)
-	    record_full_stop_at_limit = 0;
-	  else
+			"full (record full stop-at-limit)?")))
 	    error (_("Process record: stopped by user."));
+	  record_full_stop_at_limit = 0;
 	}
     }
 }
@@ -583,7 +575,7 @@ record_full_message (struct regcache *regcache, enum gdb_signal signal)
   record_full_arch_list_tail = NULL;
 
   /* Check record_full_insn_num.  */
-  record_full_check_insn_num (1);
+  record_full_check_insn_num ();
 
   /* If gdb sends a signal value to target_resume,
      save it in the 'end' field of the previous instruction.
@@ -1420,7 +1412,7 @@ static void
 record_full_registers_change (struct regcache *regcache, int regnum)
 {
   /* Check record_full_insn_num.  */
-  record_full_check_insn_num (0);
+  record_full_check_insn_num ();
 
   record_full_arch_list_head = NULL;
   record_full_arch_list_tail = NULL;
@@ -1546,7 +1538,7 @@ record_full_xfer_partial (struct target_ops *ops, enum target_object object,
 	}
 
       /* Check record_full_insn_num */
-      record_full_check_insn_num (0);
+      record_full_check_insn_num ();
 
       /* Record registers change to list as an instruction.  */
       record_full_arch_list_head = NULL;
diff --git a/gdb/remote.c b/gdb/remote.c
index 3c8de40..2aa4e07 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5900,10 +5900,6 @@ 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 (rs->waiting_for_stop_reply && rs->ctrlc_pending_p)
     {
@@ -5920,8 +5916,6 @@ interrupt_query (void)
 		   "Give up waiting? ")))
 	quit ();
     }
-
-  do_cleanups (old_chain);
 }
 
 /* Enable/disable target terminal ownership.  Most targets can use
diff --git a/gdb/utils.c b/gdb/utils.c
index 46f61c1..46765c5 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1245,12 +1245,15 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
   if (!confirm || server_command)
     return def_value;
 
+  old_chain = make_cleanup_restore_target_terminal ();
+
   /* If input isn't coming from the user directly, just say what
      question we're asking, and then answer the default automatically.  This
      way, important error messages don't get lost when talking to GDB
      over a pipe.  */
   if (! input_from_terminal_p ())
     {
+      target_terminal_ours_for_output ();
       wrap_here ("");
       vfprintf_filtered (gdb_stdout, ctlstr, args);
 
@@ -1259,15 +1262,18 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
 		       y_string, n_string, def_answer);
       gdb_flush (gdb_stdout);
 
+      do_cleanups (old_chain);
       return def_value;
     }
 
   if (deprecated_query_hook)
     {
-      return deprecated_query_hook (ctlstr, args);
-    }
+      int res;
 
-  old_chain = make_cleanup (null_cleanup, NULL);
+      res = deprecated_query_hook (ctlstr, args);
+      do_cleanups (old_chain);
+      return res;
+    }
 
   /* Format the question outside of the loop, to avoid reusing args.  */
   question = xstrvprintf (ctlstr, args);
@@ -1281,6 +1287,9 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
   /* Used for calculating time spend waiting for user.  */
   gettimeofday (&prompt_started, NULL);
 
+  /* We'll need to handle input.  */
+  target_terminal_ours ();
+
   while (1)
     {
       char *response, answer;
@@ -1852,6 +1861,7 @@ prompt_for_continue (void)
   reinitialize_more_filter ();
 
   /* We'll need to handle input.  */
+  make_cleanup_restore_target_terminal ();
   target_terminal_ours ();
 
   /* Call gdb_readline_wrapper, not readline, in order to keep an
-- 
2.5.0


  parent reply	other threads:[~2016-03-18 19:24 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-18 19:19 [PATCH 00/30] Stop throwing exceptions from signal handlers Pedro Alves
2016-03-18 19:18 ` [PATCH 18/30] Fix inconsistent handling of EINTR in ser-*.c backends Pedro Alves
2016-03-18 19:18 ` [PATCH 01/30] Don't rely on immediate_quit in command_line_input Pedro Alves
2016-03-18 19:19 ` [PATCH 02/30] Inline command_loop in read_command_line Pedro Alves
2016-03-18 19:19 ` [PATCH 29/30] Eliminate immediate_quit Pedro Alves
2016-03-18 19:19 ` [PATCH 22/30] Use target_terminal_ours_for_output in infcmd.c Pedro Alves
2016-03-18 19:19 ` [PATCH 19/30] ada-lang.c: Introduce type_as_string and use it Pedro Alves
2016-03-18 19:19 ` [PATCH 14/30] Don't call clear_quit_flag in captured_main Pedro Alves
2016-03-18 19:19 ` [PATCH 20/30] Use target_terminal_ours_for_output in cp-support.c Pedro Alves
2016-03-18 19:19 ` [PATCH 28/30] target remote: Don't rely on immediate_quit (introduce quit handlers) Pedro Alves
2016-03-18 19:19 ` [PATCH 03/30] TUI: check whether in secondary prompt instead of immediate_quit Pedro Alves
2016-03-18 19:19 ` [PATCH 27/30] TUI: GC tui_target_has_run Pedro Alves
2016-03-18 19:19 ` [PATCH 26/30] Use target_terminal_ours_for_output in MI Pedro Alves
2016-03-18 19:19 ` [PATCH 30/30] Eliminate target_check_pending_interrupt Pedro Alves
2016-03-18 19:24 ` [PATCH 17/30] Pass Ctrl-C to the target in target_terminal_inferior Pedro Alves
2016-03-18 19:24 ` [PATCH 23/30] Use target_terminal_ours_for_output in warning/internal_error Pedro Alves
2016-03-18 19:24 ` Pedro Alves [this message]
2016-03-18 19:25 ` [PATCH 08/30] Fix signal handler/event-loop races Pedro Alves
2016-03-18 19:25 ` [PATCH 04/30] Don't set immediate_quit in prompt_for_continue Pedro Alves
2016-03-18 19:25 ` [PATCH 13/30] Don't call clear_quit_flag in prepare_to_throw_exception Pedro Alves
2016-03-18 19:25 ` [PATCH 12/30] Don't call clear_quit_flag in command_handler Pedro Alves
2016-03-18 19:26 ` [PATCH 24/30] Add missing cleanups to defaulted_query and prompt_for_continue Pedro Alves
2016-03-18 19:26 ` [PATCH 10/30] Make Python use a struct serial event Pedro Alves
2016-03-18 19:26 ` [PATCH 07/30] Introduce a serial interface for select'able events Pedro Alves
2016-03-18 19:27 ` [PATCH 06/30] Remove unused struct serial::name field Pedro Alves
2016-03-18 19:27 ` [PATCH 21/30] Use target_terminal_ours_for_output in exceptions.c Pedro Alves
2016-03-18 19:27 ` [PATCH 15/30] Eliminate clear_quit_flag Pedro Alves
2016-03-18 19:28 ` [PATCH 11/30] Don't call clear_quit_flag after check_quit_flag Pedro Alves
2016-03-18 19:28 ` [PATCH 05/30] Stop remote-fileio.c from throwing from SIGINT handler Pedro Alves
2016-03-18 19:37 ` [PATCH 16/30] Decouple target_interrupt from all-stop/non-stop modes Pedro Alves
2016-03-21 18:21   ` Simon Marchi
2016-03-21 18:24     ` Pedro Alves
2016-03-18 19:37 ` [PATCH 09/30] Introduce interruptible_select Pedro Alves
2016-03-21 17:59   ` Simon Marchi
2016-03-21 18:33     ` Pedro Alves
2016-03-21 19:48   ` Simon Marchi
2016-03-21 19:49     ` Pedro Alves
2016-03-21 19:52 ` [PATCH 00/30] Stop throwing exceptions from signal handlers Simon Marchi
2016-03-31 14:43   ` Pedro Alves
2016-03-31 18:44     ` Pedro Alves
2016-04-12 16:15     ` 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=1458328714-4938-26-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