[PATCH v2 10/23] PPC64: Fix step-over-trips-on-watchpoint.exp with displaced stepping on

[Pedro Alves <palves@redhat.com>]

PPC64 current fails this test like:

 FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: no thread-specific bp: step: step
 FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: no thread-specific bp: next: next
 FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: no thread-specific bp: continue: continue (the program exited)
 FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: with thread-specific bp: step: step
 FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: with thread-specific bp: next: next
 FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: with thread-specific bp: continue: continue (the program exited)

The problem is that PPC is a non-continuable watchpoints architecture
and the displaced stepping code isn't coping with that correctly.  On
such targets/architectures, a watchpoint traps _before_ the
instruction executes/completes.  On a watchpoint trap, the PC points
at the instruction that triggers the watchpoint (side effects haven't
happened yet).  In order to move past the watchpoint, GDB needs to
remove the watchpoint, single-step, and reinsert the watchpoint, just
like when stepping past a breakpoint.

The trouble is that if GDB is stepping over a breakpoint with
displaced stepping, and the instruction under the breakpoint triggers
a watchpoint, we get the watchpoint SIGTRAP, expecting a finished
(hard or software) step trap.  Even though the thread's PC hasn't
advanced yet (must remove watchpoint for that), since we get a
SIGTRAP, displaced_step_fixup thinks the single-step finished
successfuly anyway, and calls gdbarch_displaced_step_fixup, which then
adjusts the thread's registers incorrectly.

The fix is to cancel the displaced step if we trip on a watchpoint.
handle_inferior_event then processes the watchpoint event, and starts
a new step-over, but this time, since we have a watchpoint to step
over, watchpoints are removed from the target, so the step-over
succeeds.

The keep_going/resume changes are necessary because if we have a
watchpoint to step over, we need to remove it from the target -
displaced stepping doesn't help, the copy of the instruction in the
scratch pad reads/writes to the same addresses, thus triggers the
watchpoint too...  So without those changes we keep triggering the
watchpoint forever, never making progress.  With non-stop that means
we need to pause all threads momentarily.  We could avoid that by
removing the watchpoint _only_ from the thread that is moving past the
watchpoint, but GDB is not prepared for that today.  For remote
targets, that would need new packets, so good to have this as fallback
anyway.

gdb/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* infrun.c (displaced_step_fixup): Switch to the event ptid
	earlier.  If the thread stopped for a watchpoint and the
	target/arch has non-continuable watchpoints, cancel the displaced
	step.
	(resume): Don't start a displaced step if in-line step-over info
	is valid.
---
 gdb/infrun.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 937a29d..fac0beb 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1792,6 +1792,8 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
   struct cleanup *old_cleanups;
   struct displaced_step_inferior_state *displaced
     = get_displaced_stepping_state (ptid_get_pid (event_ptid));
+  struct regcache *regcache;
+  struct gdbarch *gdbarch;
 
   /* Was any thread of this process doing a displaced step?  */
   if (displaced == NULL)
@@ -1806,13 +1808,20 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
 
   displaced_step_restore (displaced, displaced->step_ptid);
 
+  /* Fixup may need to read memory/registers.  Switch to the thread
+     that we're fixing up.  Also, target_stopped_by_watchpoint checks
+     the current thread.  */
+  switch_to_thread (event_ptid);
+
+  regcache = get_thread_regcache (event_ptid);
+  gdbarch  = get_regcache_arch (regcache);
+
   /* Did the instruction complete successfully?  */
-  if (signal == GDB_SIGNAL_TRAP)
+  if (signal == GDB_SIGNAL_TRAP
+      && !(target_stopped_by_watchpoint ()
+	   && (target_have_steppable_watchpoint
+	       || gdbarch_have_nonsteppable_watchpoint (gdbarch))))
     {
-      /* Fixup may need to read memory/registers.  Switch to the
-	 thread that we're fixing up.  */
-      switch_to_thread (event_ptid);
-
       /* Fix up the resulting state.  */
       gdbarch_displaced_step_fixup (displaced->step_gdbarch,
                                     displaced->step_closure,
@@ -1824,7 +1833,6 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
     {
       /* Since the instruction didn't complete, all we can do is
          relocate the PC.  */
-      struct regcache *regcache = get_thread_regcache (event_ptid);
       CORE_ADDR pc = regcache_read_pc (regcache);
 
       pc = displaced->step_original + (pc - displaced->step_copy);
@@ -2261,7 +2269,9 @@ resume (enum gdb_signal sig)
 
   /* If enabled, step over breakpoints by executing a copy of the
      instruction at a different address.  */
-  if (tp->control.trap_expected && use_displaced_stepping_now_p (gdbarch, sig))
+  if (tp->control.trap_expected
+      && !step_over_info_valid_p ()
+      && use_displaced_stepping_now_p (gdbarch, sig))
     {
       struct displaced_step_inferior_state *displaced;
 
@@ -2405,6 +2415,7 @@ resume (enum gdb_signal sig)
 
   if (debug_displaced
       && tp->control.trap_expected
+      && !step_over_info_valid_p ()
       && use_displaced_stepping_now_p (gdbarch, sig))
     {
       struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
@@ -6351,15 +6362,18 @@ keep_going (struct execution_control_state *ecs)
 		   || (step_what & STEP_OVER_BREAKPOINT));
       remove_wps = (step_what & STEP_OVER_WATCHPOINT);
 
-      if (remove_bp
-	  && !use_displaced_stepping_now_p (get_regcache_arch (regcache),
-					    signo))
+      if (remove_wps && remove_bp)
+	set_step_over_info (get_regcache_aspace (regcache),
+			    regcache_read_pc (regcache), remove_wps);
+      else if (remove_wps)
+	set_step_over_info (NULL, 0, remove_wps);
+      else if (remove_bp
+	       && !use_displaced_stepping_now_p (get_regcache_arch (regcache),
+						 signo))
 	{
 	  set_step_over_info (get_regcache_aspace (regcache),
 			      regcache_read_pc (regcache), remove_wps);
 	}
-      else if (remove_wps)
-	set_step_over_info (NULL, 0, remove_wps);
       else
 	clear_step_over_info ();
 
-- 
1.9.3