darkkirb/old-cross-binutils
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

PPC64: Fix step-over-trips-on-watchpoint.exp with displaced stepping on

Browse source 
PPC64 currently 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, here:

...
      /* At this point, we are stopped at an instruction which has
         attempted to write to a piece of memory under control of
         a watchpoint.  The instruction hasn't actually executed
         yet.  If we were to evaluate the watchpoint expression
         now, we would get the old value, and therefore no change
         would seem to have occurred.
...
      ecs->event_thread->stepping_over_watchpoint = 1;
      keep_going (ecs);
      return;
...

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're stepping
over a watchpoint, 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'll need
to pause all threads momentarily, which we can't today.  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
either.  For remote targets, that would need new packets, so good to
be able to step over it in-line as fallback anyway.

gdb/ChangeLog:
2015-04-10  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.
This commit is contained in:
Pedro Alves 2015-04-10 13:08:32 +01:00
parent c79d856c88
commit cb71640d03
2 changed files with 28 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
gdb/ChangeLog
View file

@ -1,3 +1,12 @@
2015-04-10 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.
2015-04-10 Pedro Alves <palves@redhat.com>
* infrun.c (displaced_step_in_progress): New function.

27
gdb/infrun.c
View file

@ -1810,13 +1810,17 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
displaced_step_restore (displaced, displaced->step_ptid);
/* Did the instruction complete successfully? */
if (signal == GDB_SIGNAL_TRAP)
{
/* Fixup may need to read memory/registers. Switch to the
thread that we're fixing up. */
switch_to_thread (event_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);
/* Did the instruction complete successfully? */
if (signal == GDB_SIGNAL_TRAP
&& !(target_stopped_by_watchpoint ()
&& (gdbarch_have_nonsteppable_watchpoint (displaced->step_gdbarch)
|| target_have_steppable_watchpoint)))
{
/* Fix up the resulting state. */
gdbarch_displaced_step_fixup (displaced->step_gdbarch,
displaced->step_closure,
@ -2247,6 +2251,7 @@ resume (enum gdb_signal sig)
step software breakpoint. */
if (use_displaced_stepping (gdbarch)
&& tp->control.trap_expected
&& !step_over_info_valid_p ()
&& sig == GDB_SIGNAL_0
&& !current_inferior ()->waiting_for_vfork_done)
{
@ -2392,7 +2397,8 @@ resume (enum gdb_signal sig)
if (debug_displaced
&& use_displaced_stepping (gdbarch)
&& tp->control.trap_expected)
&& tp->control.trap_expected
&& !step_over_info_valid_p ())
{
struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
@ -6271,7 +6277,12 @@ keep_going (struct execution_control_state *ecs)
remove_wps = (ecs->event_thread->stepping_over_watchpoint
&& !target_have_steppable_watchpoint);
if (remove_bp && !use_displaced_stepping (get_regcache_arch (regcache)))
/* We can't use displaced stepping if we need to step past a
watchpoint. The instruction copied to the scratch pad would
still trigger the watchpoint. */
if (remove_bp
&& (remove_wps
|| !use_displaced_stepping (get_regcache_arch (regcache))))
{
set_step_over_info (get_regcache_aspace (regcache),
regcache_read_pc (regcache), remove_wps);