Bail out of processing stop if hook-stop resumes target / changes context

This patch, relative to a tree with
https://sourceware.org/ml/gdb-patches/2015-08/msg00295.html, fixes
issues/crashes that trigger if something unexpected happens during a
hook-stop.

E.g., if the inferior disappears while running the hook-stop, we hit
failed assertions:

 (gdb) define hook-stop
 Type commands for definition of "hook-stop".
 End with a line saying just "end".
 >kill
 >end
 (gdb) si
 Kill the program being debugged? (y or n) [answered Y; input not from terminal]
 /home/pedro/gdb/mygit/build/../src/gdb/thread.c:88: internal-error: inferior_thread: Assertion `tp' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n)

I noticed that if a hook-stop issues a synchronous execution command,
we print the same stop event twice:

 (gdb) define hook-stop
 Type commands for definition of "hook-stop".
 End with a line saying just "end".
 >si
 >end
 (gdb) si
 0x000000000040074a      42          args[i] = 1; /* Init value.  */  <<<<<<< once
 0x000000000040074a      42          args[i] = 1; /* Init value.  */  <<<<<<< twice
 (gdb)

In MI:

 *stopped,reason="end-stepping-range",frame={addr="0x000000000040074a",func="main",args=[],file="threads.c",fullname="/home/pedro/gdb/tests/threads.c",line="42"},thread-id="1",stopped-threads="all",core="0"
 *stopped,reason="end-stepping-range",frame={addr="0x000000000040074a",func="main",args=[],file="threads.c",fullname="/home/pedro/gdb/tests/threads.c",line="42"},thread-id="1",stopped-threads="all",core="0"
 (gdb)

The fix has GDB stop processing the event if the context changed.  I
don't expect people to be doing crazy things from the hook-stop.
E.g., it gives me headaches to try to come up a proper behavior for
handling a thread change from a hook-stop... (E.g., imagine the
hook-stop does thread N; step, with scheduler-locing on).  I think the
most important bit here is preventing crashes.

The patch adds a new hook-stop.exp test that covers the above and also
merges in the old hook-stop-continue.exp and hook-stop-frame.exp into
the same framework.

gdb/ChangeLog:
2015-09-14  Pedro Alves  <palves@redhat.com>

	* infrun.c (current_stop_id): New global.
	(get_stop_id, new_stop_id): New functions.
	(fetch_inferior_event): Handle normal_stop proceeding the target.
	(struct stop_context): New.
	(save_stop_context, release_stop_context_cleanup)
	(stop_context_changed): New functions.
	(normal_stop): Return true if the hook-stop changes the stop
	context.
	* infrun.h (get_stop_id): Declare.
	(normal_stop): Now returns int.  Add documentation.

gdb/testsuite/ChangeLog:
2015-09-14  Pedro Alves  <palves@redhat.com>

	* gdb.base/hook-stop-continue.c: Delete.
	* gdb.base/hook-stop-continue.exp: Delete.
	* gdb.base/hook-stop-frame.c: Delete.
	* gdb.base/hook-stop-frame.exp: Delete.
	* gdb.base/hook-stop.c: New file.
	* gdb.base/hook-stop.exp: New file.

1 files changed, 135 insertions, 13 deletions

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 84890b4..8175fb1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2702,6 +2702,33 @@ resume (enum gdb_signal sig)
 
 /* Proceeding.  */
 
+/* See infrun.h.  */
+
+/* Counter that tracks number of user visible stops.  This can be used
+   to tell whether a command has proceeded the inferior past the
+   current location.  This allows e.g., inferior function calls in
+   breakpoint commands to not interrupt the command list.  When the
+   call finishes successfully, the inferior is standing at the same
+   breakpoint as if nothing happened (and so we don't call
+   normal_stop).  */
+static ULONGEST current_stop_id;
+
+/* See infrun.h.  */
+
+ULONGEST
+get_stop_id (void)
+{
+  return current_stop_id;
+}
+
+/* Called when we report a user visible stop.  */
+
+static void
+new_stop_id (void)
+{
+  current_stop_id++;
+}
+
 /* Clear out all variables saying what to do when inferior is continued.
    First do this, then set the ones you want, then call `proceed'.  */
 
@@ -3854,12 +3881,17 @@ fetch_inferior_event (void *client_data)
 
 	  if (should_notify_stop)
 	    {
+	      int proceeded = 0;
+
 	      /* We may not find an inferior if this was a process exit.  */
 	      if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
-		normal_stop ();
+		proceeded = normal_stop ();
 
-	      inferior_event_handler (INF_EXEC_COMPLETE, NULL);
-	      cmd_done = 1;
+	      if (!proceeded)
+		{
+		  inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+		  cmd_done = 1;
+		}
 	    }
 	}
     }
@@ -7829,15 +7861,83 @@ maybe_remove_breakpoints (void)
     }
 }
 
-/* Here to return control to GDB when the inferior stops for real.
-   Print appropriate messages, remove breakpoints, give terminal our modes.
+/* The execution context that just caused a normal stop.  */
+
+struct stop_context
+{
+  /* The stop ID.  */
+  ULONGEST stop_id;
 
-   STOP_PRINT_FRAME nonzero means print the executing frame
-   (pc, function, args, file, line number and line text).
-   BREAKPOINTS_FAILED nonzero means stop was due to error
-   attempting to insert breakpoints.  */
+  /* The event PTID.  */
 
-void
+  ptid_t ptid;
+
+  /* If stopp for a thread event, this is the thread that caused the
+     stop.  */
+  struct thread_info *thread;
+
+  /* The inferior that caused the stop.  */
+  int inf_num;
+};
+
+/* Returns a new stop context.  If stopped for a thread event, this
+   takes a strong reference to the thread.  */
+
+static struct stop_context *
+save_stop_context (void)
+{
+  struct stop_context *sc = xmalloc (sizeof (struct stop_context));
+
+  sc->stop_id = get_stop_id ();
+  sc->ptid = inferior_ptid;
+  sc->inf_num = current_inferior ()->num;
+
+  if (!ptid_equal (inferior_ptid, null_ptid))
+    {
+      /* Take a strong reference so that the thread can't be deleted
+	 yet.  */
+      sc->thread = inferior_thread ();
+      sc->thread->refcount++;
+    }
+  else
+    sc->thread = NULL;
+
+  return sc;
+}
+
+/* Release a stop context previously created with save_stop_context.
+   Releases the strong reference to the thread as well. */
+
+static void
+release_stop_context_cleanup (void *arg)
+{
+  struct stop_context *sc = arg;
+
+  if (sc->thread != NULL)
+    sc->thread->refcount--;
+  xfree (sc);
+}
+
+/* Return true if the current context no longer matches the saved stop
+   context.  */
+
+static int
+stop_context_changed (struct stop_context *prev)
+{
+  if (!ptid_equal (prev->ptid, inferior_ptid))
+    return 1;
+  if (prev->inf_num != current_inferior ()->num)
+    return 1;
+  if (prev->thread != NULL && prev->thread->state != THREAD_STOPPED)
+    return 1;
+  if (get_stop_id () != prev->stop_id)
+    return 1;
+  return 0;
+}
+
+/* See infrun.h.  */
+
+int
 normal_stop (void)
 {
   struct target_waitstatus last;
@@ -7847,6 +7947,8 @@ normal_stop (void)
 
   get_last_target_status (&last_ptid, &last);
 
+  new_stop_id ();
+
   /* If an exception is thrown from this point on, make sure to
      propagate GDB's knowledge of the executing state to the
      frontend/user running state.  A QUIT is an easy exception to see
@@ -7966,9 +8068,27 @@ normal_stop (void)
 
   /* Look up the hook_stop and run it (CLI internally handles problem
      of stop_command's pre-hook not existing).  */
-  if (stop_command)
-    catch_errors (hook_stop_stub, stop_command,
-		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
+  if (stop_command != NULL)
+    {
+      struct stop_context *saved_context = save_stop_context ();
+      struct cleanup *old_chain
+	= make_cleanup (release_stop_context_cleanup, saved_context);
+
+      catch_errors (hook_stop_stub, stop_command,
+		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
+
+      /* If the stop hook resumes the target, then there's no point in
+	 trying to notify about the previous stop; its context is
+	 gone.  Likewise if the command switches thread or inferior --
+	 the observers would print a stop for the wrong
+	 thread/inferior.  */
+      if (stop_context_changed (saved_context))
+	{
+	  do_cleanups (old_chain);
+	  return 1;
+	}
+      do_cleanups (old_chain);
+    }
 
   /* Notify observers about the stop.  This is where the interpreters
      print the stop event.  */
@@ -7993,6 +8113,8 @@ normal_stop (void)
      longer needed.  Keeping those around slows down things linearly.
      Note that this never removes the current inferior.  */
   prune_inferiors ();
+
+  return 0;
 }
 
 static int