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.
This commit is contained in:
Pedro Alves 2014-05-29 22:17:20 +01:00
parent bdc36728ee
commit a09dd4413d
2 changed files with 21 additions and 7 deletions

View File

@ -1,3 +1,11 @@
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> 2014-05-29 Pedro Alves <palves@redhat.com>
* infrun.c (process_event_stop_test, handle_step_into_function) * infrun.c (process_event_stop_test, handle_step_into_function)

View File

@ -1769,7 +1769,13 @@ resume (int step, enum gdb_signal sig)
CORE_ADDR pc = regcache_read_pc (regcache); CORE_ADDR pc = regcache_read_pc (regcache);
struct address_space *aspace = get_regcache_aspace (regcache); struct address_space *aspace = get_regcache_aspace (regcache);
ptid_t resume_ptid; 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; QUIT;
@ -1789,7 +1795,7 @@ resume (int step, enum gdb_signal sig)
if (debug_infrun) if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, fprintf_unfiltered (gdb_stdlog,
"infrun: resume : clear step\n"); "infrun: resume : clear step\n");
hw_step = 0; step = 0;
} }
if (debug_infrun) if (debug_infrun)
@ -1834,7 +1840,7 @@ a command like `return' or `jump' to continue execution."));
step software breakpoint. */ step software breakpoint. */
if (use_displaced_stepping (gdbarch) if (use_displaced_stepping (gdbarch)
&& (tp->control.trap_expected && (tp->control.trap_expected
|| (hw_step && gdbarch_software_single_step_p (gdbarch))) || (step && gdbarch_software_single_step_p (gdbarch)))
&& sig == GDB_SIGNAL_0 && sig == GDB_SIGNAL_0
&& !current_inferior ()->waiting_for_vfork_done) && !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 Unless we're calling an inferior function, as in that
case we pretend the inferior doesn't run at all. */ case we pretend the inferior doesn't run at all. */
if (!tp->control.in_infcall) 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); discard_cleanups (old_cleanups);
return; return;
} }
@ -1861,8 +1867,8 @@ a command like `return' or `jump' to continue execution."));
pc = regcache_read_pc (get_thread_regcache (inferior_ptid)); pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid)); displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
hw_step = gdbarch_displaced_step_hw_singlestep (gdbarch, step = gdbarch_displaced_step_hw_singlestep (gdbarch,
displaced->step_closure); displaced->step_closure);
} }
/* Do we need to do it the hard way, w/temp breakpoints? */ /* 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 /* Decide the set of threads to ask the target to resume. Start
by assuming everything will be resumed, than narrow the set by assuming everything will be resumed, than narrow the set
by applying increasingly restricting conditions. */ 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 /* 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 (e.g., we might need to step over a breakpoint), from the