This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[pushed] Fix confusion in infrun.c:resume


Running the current tree against my software-single-step-on-x86_64
branch showed some extra assertions I have in place triggering.  Turns
out my previous change to 'resume' was incomplete, and we mishandle
the 'hw_step' / 'step' variable pair.  (I swear I had fixed this, but
I guess I lost that in some local branch...)

Tested on x86_64 Fedora 20.

gdb/
2014-05-29  Pedro Alves  <palves@redhat.com>

	* infrun.c (resume): Rename local 'hw_step' to 'entry_step'
	and make it const.  When a single-step decays to a continue,
	clear 'step', not 'hw_step'.  Pass whether the caller wanted
	to step to user_visible_resume_ptid, not what we ask the
	target to do.
---
 gdb/ChangeLog |  8 ++++++++
 gdb/infrun.c  | 20 +++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 156b957..b86f1c7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2014-05-29  Pedro Alves  <palves@redhat.com>
 
+	* infrun.c (resume): Rename local 'hw_step' to 'entry_step'
+	and make it const.  When a single-step decays to a continue,
+	clear 'step', not 'hw_step'.  Pass whether the caller wanted
+	to step to user_visible_resume_ptid, not what we ask the
+	target to do.
+
+2014-05-29  Pedro Alves  <palves@redhat.com>
+
 	* infrun.c (process_event_stop_test, handle_step_into_function)
 	(handle_step_into_function_backward): Adjust.
 	Don't set the even thread's stop_step and call stop_waiting before
diff --git a/gdb/infrun.c b/gdb/infrun.c
index fb0bd54..47604c7 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1769,7 +1769,13 @@ resume (int step, enum gdb_signal sig)
   CORE_ADDR pc = regcache_read_pc (regcache);
   struct address_space *aspace = get_regcache_aspace (regcache);
   ptid_t resume_ptid;
-  int hw_step = step;
+  /* From here on, this represents the caller's step vs continue
+     request, while STEP represents what we'll actually request the
+     target to do.  STEP can decay from a step to a continue, if e.g.,
+     we need to implement single-stepping with breakpoints (software
+     single-step).  When deciding whether "set scheduler-locking step"
+     applies, it's the callers intention that counts.  */
+  const int entry_step = step;
 
   QUIT;
 
@@ -1789,7 +1795,7 @@ resume (int step, enum gdb_signal sig)
       if (debug_infrun)
 	fprintf_unfiltered (gdb_stdlog,
 			    "infrun: resume : clear step\n");
-      hw_step = 0;
+      step = 0;
     }
 
   if (debug_infrun)
@@ -1834,7 +1840,7 @@ a command like `return' or `jump' to continue execution."));
      step software breakpoint.  */
   if (use_displaced_stepping (gdbarch)
       && (tp->control.trap_expected
-	  || (hw_step && gdbarch_software_single_step_p (gdbarch)))
+	  || (step && gdbarch_software_single_step_p (gdbarch)))
       && sig == GDB_SIGNAL_0
       && !current_inferior ()->waiting_for_vfork_done)
     {
@@ -1851,7 +1857,7 @@ a command like `return' or `jump' to continue execution."));
 	     Unless we're calling an inferior function, as in that
 	     case we pretend the inferior doesn't run at all.  */
 	  if (!tp->control.in_infcall)
-	    set_running (user_visible_resume_ptid (step), 1);
+	    set_running (user_visible_resume_ptid (entry_step), 1);
 	  discard_cleanups (old_cleanups);
 	  return;
 	}
@@ -1861,8 +1867,8 @@ a command like `return' or `jump' to continue execution."));
       pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
 
       displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
-      hw_step = gdbarch_displaced_step_hw_singlestep (gdbarch,
-						      displaced->step_closure);
+      step = gdbarch_displaced_step_hw_singlestep (gdbarch,
+						   displaced->step_closure);
     }
 
   /* Do we need to do it the hard way, w/temp breakpoints?  */
@@ -1924,7 +1930,7 @@ a command like `return' or `jump' to continue execution."));
   /* Decide the set of threads to ask the target to resume.  Start
      by assuming everything will be resumed, than narrow the set
      by applying increasingly restricting conditions.  */
-  resume_ptid = user_visible_resume_ptid (step);
+  resume_ptid = user_visible_resume_ptid (entry_step);
 
   /* Even if RESUME_PTID is a wildcard, and we end up resuming less
      (e.g., we might need to step over a breakpoint), from the
-- 
1.9.0


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]