Don't ever Quit out of resume

If you have a breakpoint command that re-resumes the target, like:

  break foo
  commands
  > c
  > end

and then let the inferior run, hitting the breakpoint, and then press
Ctrl-C at just the right time, between GDB processing the stop at
"foo", and re-resuming the target, you'll hit the QUIT call in
infrun.c:resume.

With this hack, we can reproduce the bad case consistently:

  --- a/gdb/inf-loop.c
  +++ b/gdb/inf-loop.c
  @@ -31,6 +31,8 @@
   #include "top.h"
   #include "observer.h"

  +bool continue_hack;
  +
   /* General function to handle events in the inferior.  */

   void
  @@ -64,6 +66,8 @@ inferior_event_handler (enum inferior_event_type event_type,
	  {
	    check_frame_language_change ();

  +         continue_hack = true;
  +
	    /* Don't propagate breakpoint commands errors.  Either we're
	       stopping or some command resumes the inferior.  The user will
	       be informed.  */
  diff --git a/gdb/infrun.c b/gdb/infrun.c
  index d425664..c74b14c 100644
  --- a/gdb/infrun.c
  +++ b/gdb/infrun.c
  @@ -2403,6 +2403,10 @@ resume (enum gdb_signal sig)
     gdb_assert (!tp->stop_requested);
     gdb_assert (!thread_is_in_step_over_chain (tp));

  +  extern bool continue_hack;
  +
  +  if (continue_hack)
  +    set_quit_flag ();
     QUIT;

The GDB backtrace looks like this:

  (top-gdb) bt
  ...
  #3  0x0000000000612e8b in throw_quit(char const*, ...) (fmt=0xaf84a1 "Quit") at src/gdb/common/common-exceptions.c:408
  #4  0x00000000007fc104 in quit() () at src/gdb/utils.c:748
  #5  0x00000000006a79d2 in default_quit_handler() () at src/gdb/event-top.c:954
  #6  0x00000000007fc134 in maybe_quit() () at src/gdb/utils.c:762
  #7  0x00000000006f66a3 in resume(gdb_signal) (sig=GDB_SIGNAL_0) at src/gdb/infrun.c:2406
  #8  0x0000000000700c3d in keep_going_pass_signal(execution_control_state*) (ecs=0x7ffcf3744e60) at src/gdb/infrun.c:7793
  #9  0x00000000006f5fcd in start_step_over() () at src/gdb/infrun.c:2145
  #10 0x00000000006f7b1f in proceed(unsigned long, gdb_signal) (addr=18446744073709551615, siggnal=GDB_SIGNAL_DEFAULT)
      at src/gdb/infrun.c:3135
  #11 0x00000000006ebdd4 in continue_1(int) (all_threads=0) at src/gdb/infcmd.c:842
  #12 0x00000000006ec097 in continue_command(char*, int) (args=0x0, from_tty=0) at src/gdb/infcmd.c:938
  #13 0x00000000004b5140 in do_cfunc(cmd_list_element*, char*, int) (c=0x2d18570, args=0x0, from_tty=0)
      at src/gdb/cli/cli-decode.c:106
  #14 0x00000000004b8219 in cmd_func(cmd_list_element*, char*, int) (cmd=0x2d18570, args=0x0, from_tty=0)
      at src/gdb/cli/cli-decode.c:1952
  #15 0x00000000007f1532 in execute_command(char*, int) (p=0x7ffcf37452b1 "", from_tty=0) at src/gdb/top.c:608
  #16 0x00000000004bd127 in execute_control_command(command_line*) (cmd=0x3a88ef0) at src/gdb/cli/cli-script.c:485
  #17 0x00000000005cae0c in bpstat_do_actions_1(bpstat*) (bsp=0x37edcf0) at src/gdb/breakpoint.c:4513
  #18 0x00000000005caf67 in bpstat_do_actions() () at src/gdb/breakpoint.c:4563
  #19 0x00000000006e8798 in inferior_event_handler(inferior_event_type, void*) (event_type=INF_EXEC_COMPLETE, client_data=0x0)
      at src/gdb/inf-loop.c:72
  #20 0x00000000006f9447 in fetch_inferior_event(void*) (client_data=0x0) at src/gdb/infrun.c:3970
  #21 0x00000000006e870e in inferior_event_handler(inferior_event_type, void*) (event_type=INF_REG_EVENT, client_data=0x0)
      at src/gdb/inf-loop.c:43
  #22 0x0000000000494d58 in remote_async_serial_handler(serial*, void*) (scb=0x3585ca0, context=0x2cd1b80)
      at src/gdb/remote.c:13820
  #23 0x000000000044d682 in run_async_handler_and_reschedule(serial*) (scb=0x3585ca0) at src/gdb/ser-base.c:137
  #24 0x000000000044d767 in fd_event(int, void*) (error=0, context=0x3585ca0) at src/gdb/ser-base.c:188
  #25 0x00000000006a5686 in handle_file_event(file_handler*, int) (file_ptr=0x45997d0, ready_mask=1)
      at src/gdb/event-loop.c:733
  #26 0x00000000006a5c29 in gdb_wait_for_event(int) (block=1) at src/gdb/event-loop.c:859
  #27 0x00000000006a4aa6 in gdb_do_one_event() () at src/gdb/event-loop.c:347
  #28 0x00000000006a4ade in start_event_loop() () at src/gdb/event-loop.c:371

and when that happens, you end up with GDB's run control in quite a
messed up state.  Something like this:

  thread_function1 (arg=0x1) at threads.c:107
  107             usleep (SLEEP);  /* Loop increment.  */
  Quit
  (gdb) c
  Continuing.
  ** nothing happens, time passes..., press ctrl-c again **
  ^CQuit
  (gdb) info threads
    Id   Target Id         Frame
    1    Thread 1462.1462 "threads" (running)
  * 2    Thread 1462.1466 "threads" (running)
    3    Thread 1462.1465 "function0" (running)
  (gdb) c
  Cannot execute this command while the selected thread is running.
  (gdb)

The first "Quit" above is thrown from within "resume", and cancels run
control while GDB is in the middle of stepping over a breakpoint.
with step_over_info_valid_p() true.  The next "c" didn't actually
resume anything, because GDB throught that the step-over was still in
progress.  It wasn't, because the thread that was supposed to be
stepping over the breakpoint wasn't actually resumed.

So at this point, we press Ctrl-C again, and this time, the default
quit handler is called directly from the event loop
(event-top.c:default_quit_handler -> quit()), because gdb was left
owning the terminal (because the previous resume was cancelled before
we reach target_resume -> target_terminal::inferior()).

Note that the exception called from within resume ends up calling
normal_stop via resume_cleanups.  That's very borked though, because
normal_stop is going to re-handle whatever was the last reported
event, possibly even re-running a hook stop...  I think that the only
sane way to safely cancel the run control state machinery is to push
an event via handle_inferior_event like all other events.

The fix here does two things, and either alone would fix the problem
at hand:

#1 - passes the terminal to the inferior earlier, so that any QUIT
     call from the point we declare the target as running goes to the
     inferior directly, protecting run control from unsafe QUIT calls.

#2 - gets rid of this QUIT call in resume and of its related unsafe
     resume_cleanups.

Aboout #2, the comment describing resume says:

  /* Resume the inferior, but allow a QUIT.  This is useful if the user
     wants to interrupt some lengthy single-stepping operation
     (for child processes, the SIGINT goes to the inferior, and so
     we get a SIGINT random_signal, but for remote debugging and perhaps
     other targets, that's not true).

but that's a really old comment that predates a lot of fixes to Ctrl-C
handling throughout both GDB core and the remote target, that made
sure that a Ctrl-C isn't ever lost.  In any case, if some target
depended on this, a much better fix would be to make the target return
a SIGINT stop out of target_wait the next time that is called.

This was exposed by the new gdb.base/bp-cmds-continue-ctrl-c.exp
testcase added later in the series.

gdb/ChangeLog:
2017-11-16  Pedro Alves  <palves@redhat.com>

	* infrun.c (resume_cleanups): Delete.
	(resume): No longer install a resume_cleanups cleanup nor call
	QUIT.
	(proceed): Pass the terminal to the inferior.
	(keep_going_pass_signal): No longer install a resume_cleanups
	cleanup.

2 files changed, 17 insertions, 35 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 766d153..b55ffdd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2017-11-16  Pedro Alves  <palves@redhat.com>
 
+	* infrun.c (resume_cleanups): Delete.
+	(resume): No longer install a resume_cleanups cleanup nor call
+	QUIT.
+	(proceed): Pass the terminal to the inferior.
+	(keep_going_pass_signal): No longer install a resume_cleanups
+	cleanup.
+
+2017-11-16  Pedro Alves  <palves@redhat.com>
+
 	* inf-loop.c (inferior_event_handler): Don't swallow the exception
 	if the prompt is blocked.
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e2d1248..49f2fc5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -74,8 +74,6 @@ static void sig_print_info (enum gdb_signal);
 
 static void sig_print_header (void);
 
-static void resume_cleanups (void *);
-
 static int follow_fork (void);
 
 static int follow_fork_inferior (int follow_child, int detach_fork);
@@ -2192,17 +2190,6 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
 }
 
 
-/* Resuming.  */
-
-/* Things to clean up if we QUIT out of resume ().  */
-static void
-resume_cleanups (void *ignore)
-{
-  if (!ptid_equal (inferior_ptid, null_ptid))
-    delete_single_step_breakpoints (inferior_thread ());
-
-  normal_stop ();
-}
 
 static const char schedlock_off[] = "off";
 static const char schedlock_on[] = "on";
@@ -2367,17 +2354,12 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig)
   target_commit_resume ();
 }
 
-/* Resume the inferior, but allow a QUIT.  This is useful if the user
-   wants to interrupt some lengthy single-stepping operation
-   (for child processes, the SIGINT goes to the inferior, and so
-   we get a SIGINT random_signal, but for remote debugging and perhaps
-   other targets, that's not true).
+/* Resume the inferior.  SIG is the signal to give the inferior
+   (GDB_SIGNAL_0 for none).  */
 
-   SIG is the signal to give the inferior (zero for none).  */
 void
 resume (enum gdb_signal sig)
 {
-  struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = regcache->arch ();
   struct thread_info *tp = inferior_thread ();
@@ -2397,8 +2379,6 @@ resume (enum gdb_signal sig)
   gdb_assert (!tp->stop_requested);
   gdb_assert (!thread_is_in_step_over_chain (tp));
 
-  QUIT;
-
   if (tp->suspend.waitstatus_pending_p)
     {
       if (debug_infrun)
@@ -2425,7 +2405,6 @@ resume (enum gdb_signal sig)
 	}
 
       tp->suspend.stop_signal = GDB_SIGNAL_0;
-      discard_cleanups (old_cleanups);
 
       if (target_can_async_p ())
 	target_async (1);
@@ -2537,7 +2516,6 @@ resume (enum gdb_signal sig)
 
 	      resume_ptid = internal_resume_ptid (user_step);
 	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
-	      discard_cleanups (old_cleanups);
 	      tp->resumed = 1;
 	      return;
 	    }
@@ -2575,7 +2553,6 @@ resume (enum gdb_signal sig)
 				"Got placed in step-over queue\n");
 
 	  tp->control.trap_expected = 0;
-	  discard_cleanups (old_cleanups);
 	  return;
 	}
       else if (prepared < 0)
@@ -2752,7 +2729,6 @@ resume (enum gdb_signal sig)
 
   do_target_resume (resume_ptid, step, sig);
   tp->resumed = 1;
-  discard_cleanups (old_cleanups);
 }
 
 /* Proceeding.  */
@@ -3067,6 +3043,12 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
      inferior.  */
   gdb_flush (gdb_stdout);
 
+  /* Since we've marked the inferior running, give it the terminal.  A
+     QUIT/Ctrl-C from here on is forwarded to the target (which can
+     still detect attempts to unblock a stuck connection with repeated
+     Ctrl-C from within target_pass_ctrlc).  */
+  target_terminal::inferior ();
+
   /* In a multi-threaded task we may select another thread and
      then continue or step.
 
@@ -7657,10 +7639,6 @@ stop_waiting (struct execution_control_state *ecs)
 static void
 keep_going_pass_signal (struct execution_control_state *ecs)
 {
-  /* Make sure normal_stop is called if we get a QUIT handled before
-     reaching resume.  */
-  struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
-
   gdb_assert (ptid_equal (ecs->event_thread->ptid, inferior_ptid));
   gdb_assert (!ecs->event_thread->resumed);
 
@@ -7682,7 +7660,6 @@ keep_going_pass_signal (struct execution_control_state *ecs)
 	 non-signal event (e.g., a fork); or took a signal which we
 	 are supposed to pass through to the inferior.  Simply
 	 continue.  */
-      discard_cleanups (old_cleanups);
       resume (ecs->event_thread->suspend.stop_signal);
     }
   else if (step_over_info_valid_p ())
@@ -7710,8 +7687,6 @@ keep_going_pass_signal (struct execution_control_state *ecs)
 				"resume of %s deferred\n",
 				target_pid_to_str (tp->ptid));
 	}
-
-      discard_cleanups (old_cleanups);
     }
   else
     {
@@ -7775,14 +7750,12 @@ keep_going_pass_signal (struct execution_control_state *ecs)
 	{
 	  exception_print (gdb_stderr, e);
 	  stop_waiting (ecs);
-	  discard_cleanups (old_cleanups);
 	  return;
 	}
       END_CATCH
 
       ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
 
-      discard_cleanups (old_cleanups);
       resume (ecs->event_thread->suspend.stop_signal);
     }