[PATCH 3/6] Fix missing breakpoing/watchpoint hits, eliminate deferred_step_ptid.

[Pedro Alves <palves at redhat dot com>]

Consider the case of the user doing "step" in thread 2, while thread 1
had previously stopped for a breakpoint.  In order to make progress,
GDB makes thread 1 step over its breakpoint, first, and once that is
over, thread 2 then starts stepping (with thread 1 and all others
running free, by default).  If GDB didn't do that, thread 1 would just
trip on the same breakpoint immediately again.  This is what the
prepare_to_proceed / deferred_step_ptid code is all about.

However, the current implementation has at least two issues:

First, the deferred_step_ptid code resumes the target with:

	  resume (1, GDB_SIGNAL_0);
	  prepare_to_wait (ecs);
	  return;

Recall we were just stepping over a breakpoint when we get here.  That
means that _nothing_ had installed breakpoints yet!  If there's
another breakpoint just after the breakpoint that was just stepped,
we'll miss it.  The fix for that would be to use keep_going instead.

But, there are more problems.  What if the instruction that was just
single-stepped triggers a watchpoint?  Currently, GDB just happily
resumes the thread, losing that to...

So the fix must be to let the trap fall through the regular bpstat
handling, and only if no breakpoint, watchpoint, etc. claims the trap,
shall we switch back to the stepped thread.

Now, nowadays, we have code at the tail end of trap handling that does
exactly that -- switch back to the stepped thread
(switch_back_to_the_stepped_thread).

So the deferred_step_ptid code is just standing in the way, and can
simply be eliminated, fixing bugs in the process.  Sweet.

The comment about spurious "Switching to ..." made me pause, but is
actually stale nowadays.  That isn't needed anymore.
previous_inferior_ptid used to be re-set at each (internal) event, but
now it's only touched in proceed and normal stop.

The two tests added by this patch fail without the fix.

Tested on x86_64 Fedora 17 (also against my software single-stepping
on x86 branch).

gdb/
2014-02-25  Pedro Alves  <palves@redhat.com>

	* infrun.c (previous_inferior_ptid): Adjust comment.
	(deferred_step_ptid): Delete.
	(infrun_thread_ptid_changed, prepare_to_proceed)
	(init_wait_for_inferior): Adjust.
	(handle_signal_stop): Delete deferred_step_ptid handling.

gdb/testsuite/
2014-02-25  Pedro Alves  <palves@redhat.com>

	* gdb.threads/step-over-lands-on-breakpoint.c: New file.
	* gdb.threads/step-over-lands-on-breakpoint.exp: New file.
	* gdb.threads/step-over-trips-on-watchpoint.c: New file.
	* gdb.threads/step-over-trips-on-watchpoint.exp: New file.
---
 gdb/infrun.c                                       | 62 +------------------
 .../gdb.threads/step-over-lands-on-breakpoint.c    | 65 ++++++++++++++++++++
 .../gdb.threads/step-over-lands-on-breakpoint.exp  | 64 ++++++++++++++++++++
 .../gdb.threads/step-over-trips-on-watchpoint.c    | 65 ++++++++++++++++++++
 .../gdb.threads/step-over-trips-on-watchpoint.exp  | 70 ++++++++++++++++++++++
 5 files changed, 267 insertions(+), 59 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c
 create mode 100644 gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp
 create mode 100644 gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 25b40fb..8bfce34 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -127,9 +127,9 @@ show_step_stop_if_no_debug (struct ui_file *file, int from_tty,
 
 int sync_execution = 0;
 
-/* wait_for_inferior and normal_stop use this to notify the user
-   when the inferior stopped in a different thread than it had been
-   running in.  */
+/* proceed and normal_stop use this to notify the user when the
+   inferior stopped in a different thread than it had been running
+   in.  */
 
 static ptid_t previous_inferior_ptid;
 
@@ -977,15 +977,6 @@ static CORE_ADDR singlestep_pc;
 static ptid_t saved_singlestep_ptid;
 static int stepping_past_singlestep_breakpoint;
 
-/* If not equal to null_ptid, this means that after stepping over breakpoint
-   is finished, we need to switch to deferred_step_ptid, and step it.
-
-   The use case is when one thread has hit a breakpoint, and then the user 
-   has switched to another thread and issued 'step'.  We need to step over
-   breakpoint in the thread which hit the breakpoint, but then continue
-   stepping the thread user has selected.  */
-static ptid_t deferred_step_ptid;
-
 /* If stepping over a breakpoint (not displaced stepping), this is the
    address (and address space) where that breakpoint is inserted.
    When not stepping over a breakpoint, STEP_OVER_ASPACE is NULL.
@@ -1615,9 +1606,6 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
   if (ptid_equal (singlestep_ptid, old_ptid))
     singlestep_ptid = new_ptid;
 
-  if (ptid_equal (deferred_step_ptid, old_ptid))
-    deferred_step_ptid = new_ptid;
-
   for (displaced = displaced_step_inferior_states;
        displaced;
        displaced = displaced->next)
@@ -2138,10 +2126,6 @@ prepare_to_proceed (int step)
       if (breakpoint_here_p (get_regcache_aspace (regcache),
 			     regcache_read_pc (regcache)))
 	{
-	  /* If stepping, remember current thread to switch back to.  */
-	  if (step)
-	    deferred_step_ptid = inferior_ptid;
-
 	  /* Switch back to WAIT_PID thread.  */
 	  switch_to_thread (wait_ptid);
 
@@ -2419,7 +2403,6 @@ init_wait_for_inferior (void)
   clear_proceed_status ();
 
   stepping_past_singlestep_breakpoint = 0;
-  deferred_step_ptid = null_ptid;
 
   target_last_wait_ptid = minus_one_ptid;
 
@@ -3959,45 +3942,6 @@ handle_signal_stop (struct execution_control_state *ecs)
 	}
     }
 
-  if (!ptid_equal (deferred_step_ptid, null_ptid))
-    {
-      /* In non-stop mode, there's never a deferred_step_ptid set.  */
-      gdb_assert (!non_stop);
-
-      /* If we stopped for some other reason than single-stepping, ignore
-	 the fact that we were supposed to switch back.  */
-      if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
-	{
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog,
-				"infrun: handling deferred step\n");
-
-	  /* Pull the single step breakpoints out of the target.  */
-	  if (singlestep_breakpoints_inserted_p)
-	    {
-	      if (!ptid_equal (ecs->ptid, inferior_ptid))
-		context_switch (ecs->ptid);
-	      remove_single_step_breakpoints ();
-	      singlestep_breakpoints_inserted_p = 0;
-	    }
-
-	  ecs->event_thread->control.trap_expected = 0;
-	  step_over_aspace = NULL;
-	  step_over_address = 0;
-
-	  context_switch (deferred_step_ptid);
-	  deferred_step_ptid = null_ptid;
-	  /* Suppress spurious "Switching to ..." message.  */
-	  previous_inferior_ptid = inferior_ptid;
-
-	  resume (1, GDB_SIGNAL_0);
-	  prepare_to_wait (ecs);
-	  return;
-	}
-
-      deferred_step_ptid = null_ptid;
-    }
-
   /* See if a thread hit a thread-specific breakpoint that was meant for
      another thread.  If so, then step that thread past the breakpoint,
      and continue it.  */
diff --git a/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c
new file mode 100644
index 0000000..882ae82
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c
@@ -0,0 +1,65 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+pthread_barrier_t barrier;
+pthread_t child_thread;
+
+volatile unsigned int counter = 1;
+
+void *
+child_function (void *arg)
+{
+  pthread_barrier_wait (&barrier);
+
+  while (counter > 0)
+    {
+      counter++;
+
+      asm ("	nop"); /* set breakpoint child here */
+      asm ("	nop"); /* set breakpoint after step-over here */
+      usleep (1);
+    }
+
+  pthread_exit (NULL);
+}
+
+static int
+wait_threads (void)
+{
+  return 1; /* in wait_threads */
+}
+
+int
+main ()
+{
+  int res;
+  long i;
+
+  pthread_barrier_init (&barrier, NULL, 2);
+
+  res = pthread_create (&child_thread, NULL, child_function, NULL);
+  pthread_barrier_wait (&barrier);
+  wait_threads (); /* set wait-thread breakpoint here */
+
+  pthread_join (child_thread, NULL);
+
+  exit (EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp
new file mode 100644
index 0000000..c781570
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp
@@ -0,0 +1,64 @@
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that when a step-over lands on a breakpoint, that breakpoint
+# hit is reported.
+
+standard_testfile
+set executable ${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	 executable [list debug "incdir=${objdir}"]] != "" } {
+    return -1
+}
+
+# Cover both stepping and non-stepping execution commands.
+foreach command {"step" "next" "continue" } {
+    with_test_prefix $command {
+	clean_restart $executable
+
+	if ![runto_main] {
+	    continue
+	}
+
+	gdb_breakpoint [gdb_get_line_number "set wait-thread breakpoint here"]
+	gdb_continue_to_breakpoint "run to wait-thread breakpoint"
+	gdb_test "info threads" "2 .*\\\* 1.*" "info threads shows all threads"
+
+	gdb_test "set scheduler-locking on"
+
+	delete_breakpoints
+
+	gdb_breakpoint [gdb_get_line_number "set breakpoint child here"]
+	gdb_test "thread 2" "Switching to .*"
+	gdb_continue_to_breakpoint "run to breakpoint in thread 2"
+	gdb_test "p counter = 0" " = 0" "unbreak loop in thread 2"
+
+	# Set a breakpoint exactly where the step-over will land.
+	gdb_breakpoint [gdb_get_line_number "breakpoint after step-over here"]
+
+	# Switch back to thread 1 and disable scheduler locking.
+	gdb_test "thread 1" "Switching to .*"
+	gdb_test "set scheduler-locking off"
+	gdb_test "info breakpoints"
+	gdb_test "set debug infrun 1"
+
+	# Thread 2 is still stopped at a breakpoint that needs to be
+	# stepped over before proceeding thread 1.  However, right
+	# where the step-over lands there's another breakpoint
+	# installed, which should trap and be reported to the user.
+	gdb_test "$command" "step-over here.*"
+    }
+}
diff --git a/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c
new file mode 100644
index 0000000..b72e238
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c
@@ -0,0 +1,65 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+pthread_barrier_t barrier;
+pthread_t child_thread;
+
+volatile unsigned int counter = 1;
+volatile unsigned int watch_me;
+
+void *
+child_function (void *arg)
+{
+  pthread_barrier_wait (&barrier);
+
+  while (counter > 0)
+    {
+      counter++;
+
+      watch_me = 1; /* set breakpoint child here */
+      usleep (1);
+    }
+
+  pthread_exit (NULL);
+}
+
+static int
+wait_threads (void)
+{
+  return 1; /* in wait_threads */
+}
+
+int
+main ()
+{
+  int res;
+  long i;
+
+  pthread_barrier_init (&barrier, NULL, 2);
+
+  res = pthread_create (&child_thread, NULL, child_function, NULL);
+  pthread_barrier_wait (&barrier);
+  wait_threads (); /* set wait-thread breakpoint here */
+
+  pthread_join (child_thread, NULL);
+
+  exit (EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp
new file mode 100644
index 0000000..1e8fe40
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp
@@ -0,0 +1,70 @@
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that when a step-over trips on a watchpoint, that watchpoint is
+# reported.
+
+standard_testfile
+set executable ${testfile}
+
+# This test verifies that a watchpoint is detected in a multithreaded
+# program so the test is only meaningful on a system with hardware
+# watchpoints.
+if {[skip_hw_watchpoint_tests]} {
+    return 0
+}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	 executable [list debug "incdir=${objdir}"]] != "" } {
+    return -1
+}
+
+# Cover both stepping and non-stepping execution commands.
+foreach command {"step" "next" "continue" } {
+    with_test_prefix $command {
+	clean_restart $executable
+
+	if ![runto_main] {
+	    continue
+	}
+
+	gdb_breakpoint [gdb_get_line_number "set wait-thread breakpoint here"]
+	gdb_continue_to_breakpoint "run to wait-thread breakpoint"
+	gdb_test "info threads" "2 .*\\\* 1.*" "info threads shows all threads"
+
+	gdb_test "set scheduler-locking on"
+
+	delete_breakpoints
+
+	gdb_breakpoint [gdb_get_line_number "set breakpoint child here"]
+	gdb_test "thread 2" "Switching to .*"
+	gdb_continue_to_breakpoint "run to breakpoint in thread 2"
+	gdb_test "p counter = 0" " = 0" "unbreak loop in thread 2"
+	gdb_test "p watch_me = 0" " = 0" "clear watch_me"
+	gdb_test "watch watch_me" "Hardware watchpoint .*"
+
+	# Switch back to thread 1 and disable scheduler locking.
+	gdb_test "thread 1" "Switching to .*"
+	gdb_test "set scheduler-locking off"
+	gdb_test "info breakpoints"
+	gdb_test "set debug infrun 1"
+
+	# Thread 2 is still stopped at a breakpoint that needs to be
+	# stepped over before proceeding thread 1.  However, right
+	# where the step-over lands there's another breakpoint
+	# installed, which should trap and be reported to the user.
+	gdb_test "$command" "Hardware watchpoint.*: watch_me.*New value = 1.*"
+    }
+}
-- 
1.7.11.7