[binutils-gdb] Misc switch_back_to_stepped_thread cleanups

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1afd5965eda86caed3af7f23fd1f8802831360b6

commit 1afd5965eda86caed3af7f23fd1f8802831360b6
Author: Pedro Alves <palves@redhat.com>
Date:   Fri Aug 7 17:23:58 2015 +0100

    Misc switch_back_to_stepped_thread cleanups
    
    Several misc cleanups that prepare the tail end of this function, the
    part that actually re-resumes the stepped thread.
    
    The most non-obvious would be the currently_stepping change, I guess.
    That's because it isn't ever correct to pass step=1 to target_resume
    on software single-step targets, and currently_stepping works at a
    conceptual higher level, it returns step=true even on software step
    targets.  It doesn't really matter on hardware step targets, as the
    breakpoint will be hit immediately, but it's just wrong on software
    step targets.  I tested it against my x86 software single-step branch,
    and it indeed fixes failed assertions (that catch spurious
    PTRACE_SINGLESTEP requests) there.
    
    gdb/ChangeLog:
    2015-08-07  Pedro Alves  <palves@redhat.com>
    
    	* infrun.c (switch_back_to_stepped_thread): Use ecs->ptid instead
    	of inferior_ptid.  If the stepped thread vanished, return 0
    	instead of resuming here.  Use reset_ecs.  Print the prev_pc and
    	the current stop_pc in log message.  Clear trap_expected if the
    	thread advanced.  Don't pass currently_stepping to
    	do_target_resume.

Diff:
---
 gdb/ChangeLog |  9 +++++++++
 gdb/infrun.c  | 33 ++++++++++++++++++---------------
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3a6668a..bd9f969 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-08-07  Pedro Alves  <palves@redhat.com>
 
+	* infrun.c (switch_back_to_stepped_thread): Use ecs->ptid instead
+	of inferior_ptid.  If the stepped thread vanished, return 0
+	instead of resuming here.  Use reset_ecs.  Print the prev_pc and
+	the current stop_pc in log message.  Clear trap_expected if the
+	thread advanced.  Don't pass currently_stepping to
+	do_target_resume.
+
+2015-08-07  Pedro Alves  <palves@redhat.com>
+
 	* gdbthread.h (struct thread_info) <prev_pc>: Extend comment.
 	* infrun.c (struct execution_control_state): Move higher up in the
 	file.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 94075ce..455fc2d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5717,7 +5717,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
         {
 	  /* Ignore threads of processes we're not resuming.  */
 	  if (!sched_multi
-	      && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
+	      && ptid_get_pid (tp->ptid) != ptid_get_pid (ecs->ptid))
 	    continue;
 
 	  /* When stepping over a breakpoint, we lock all threads
@@ -5781,19 +5781,17 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 				    "stepped thread, it has vanished\n");
 
 	      delete_thread (tp->ptid);
-	      keep_going (ecs);
-	      return 1;
+	      return 0;
 	    }
 
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
 				"infrun: switching back to stepped thread\n");
 
-	  ecs->event_thread = tp;
-	  ecs->ptid = tp->ptid;
-	  context_switch (ecs->ptid);
+	  reset_ecs (ecs, tp);
+	  switch_to_thread (tp->ptid);
 
-	  stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
+	  stop_pc = regcache_read_pc (get_thread_regcache (tp->ptid));
 	  frame = get_current_frame ();
 	  gdbarch = get_frame_arch (frame);
 
@@ -5817,23 +5815,28 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 
 	      if (debug_infrun)
 		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: expected thread advanced also\n");
+				    "infrun: expected thread advanced also "
+				    "(%s -> %s)\n",
+				    paddress (target_gdbarch (), tp->prev_pc),
+				    paddress (target_gdbarch (), stop_pc));
 
 	      /* Clear the info of the previous step-over, as it's no
-		 longer valid.  It's what keep_going would do too, if
-		 we called it.  Must do this before trying to insert
-		 the sss breakpoint, otherwise if we were previously
-		 trying to step over this exact address in another
-		 thread, the breakpoint ends up not installed.  */
+		 longer valid (if the thread was trying to step over a
+		 breakpoint, it has already succeeded).  It's what
+		 keep_going would do too, if we called it.  Do this
+		 before trying to insert the sss breakpoint, otherwise
+		 if we were previously trying to step over this exact
+		 address in another thread, the breakpoint is
+		 skipped.  */
 	      clear_step_over_info ();
+	      tp->control.trap_expected = 0;
 
 	      insert_single_step_breakpoint (get_frame_arch (frame),
 					     get_frame_address_space (frame),
 					     stop_pc);
 
 	      resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
-	      do_target_resume (resume_ptid,
-				currently_stepping (tp), GDB_SIGNAL_0);
+	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
 	      prepare_to_wait (ecs);
 	    }
 	  else