Do target_terminal_ours in query & friends instead of in all callers
authorPedro Alves <palves@redhat.com>
Tue, 12 Apr 2016 15:49:32 +0000 (16:49 +0100)
committerPedro Alves <palves@redhat.com>
Tue, 12 Apr 2016 16:00:01 +0000 (17:00 +0100)
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:
2016-04-12  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/ChangeLog
gdb/gnu-nat.c
gdb/i386-tdep.c
gdb/linux-record.c
gdb/nto-procfs.c
gdb/record-full.c
gdb/remote.c
gdb/utils.c

index 83235c390da49f5fe082b477153e892951a4c712..8f30df17ee353b3c88e50e4c4cc4d539a658beb0 100644 (file)
@@ -1,3 +1,29 @@
+2016-04-12  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.
+
 2016-04-12  Pedro Alves  <palves@redhat.com>
 
        * utils.c (defaulted_query, prompt_for_continue): Free temporary
index b0b870c2066124de232653f21ed620fba32fca2a..c268732b3bdc4ccf31065e41d969639b3eb87d52 100644 (file)
@@ -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;
index 4c66edf9dbc1b8098ac4dadbd26852cad45d58cb..a328c189b969d1c9ae6a376591a9168076ef734b 100644 (file)
@@ -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;
                     }
                }
index bf2041996e119eef3cbf2df8da82e07279e863a1..fda7ada5499ec125a3836ec38c2955a0bf341aa1 100644 (file)
@@ -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:
index 992de2967c65c25fd5e7c7d8b447e0e70ee11404..eb7dcfeb089abde9b59aee30fb50896387978765 100644 (file)
@@ -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.  */
index d8aa89c8727f8d78b808c5c1eb6aa30b92d6b886..f307b4876fcae111e3c2ae282ac7263be8240e00 100644 (file)
@@ -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;
index 0b2d25fde37729832228cc5f3f417d5ebfe7de4a..7f5d7bac7600bbfc4d1e385a8bfc245984793da1 100644 (file)
@@ -5902,10 +5902,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)
     {
@@ -5922,8 +5918,6 @@ interrupt_query (void)
                   "Give up waiting? ")))
        quit ();
     }
-
-  do_cleanups (old_chain);
 }
 
 /* Enable/disable target terminal ownership.  Most targets can use
index 760c37654e59d7b70ca80458bdf80e9f97a997e8..6c2d1d48240b6149f8cc7eef8b2f45d9d3daaa28 100644 (file)
@@ -1274,12 +1274,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);
 
@@ -1288,15 +1291,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);
@@ -1310,6 +1316,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;
@@ -1881,6 +1890,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