Fix missing breakpoint/watchpoint hits, eliminate deferred_step_ptid.
authorPedro Alves <palves@redhat.com>
Thu, 20 Mar 2014 13:26:31 +0000 (13:26 +0000)
committerPedro Alves <palves@redhat.com>
Thu, 20 Mar 2014 13:26:31 +0000 (13:26 +0000)
commitb9f437de50bcca478359c4c2ec0da50c29ddc512
treec0204ee7a78d0926ddee22c75f8a5e21bde2b623
parentbcf83b2a66f0d968b51af8357f1543523ef83470
Fix missing breakpoint/watchpoint hits, eliminate deferred_step_ptid.

Consider the case of the user doing "step" in thread 2, while thread 1
had previously stopped for a breakpoint.  In order to make progress,
GDB makes thread 1 step over its breakpoint first (with all other
threads stopped), and once that is over, thread 2 then starts stepping
(with thread 1 and all others running free, by default).  If GDB
didn't do that, thread 1 would just trip on the same breakpoint
immediately again.  This is what the prepare_to_proceed /
deferred_step_ptid code is all about.

However, deferred_step_ptid code resumes the target with:

  resume (1, GDB_SIGNAL_0);
  prepare_to_wait (ecs);
  return;

Recall we were just stepping over a breakpoint when we get here.  That
means that _nothing_ had installed breakpoints yet!  If there's
another breakpoint just after the breakpoint that was just stepped,
we'll miss it.  The fix for that would be to use keep_going instead.

However, there are more problems.  What if the instruction that was
just single-stepped triggers a watchpoint?  Currently, GDB just
happily resumes the thread, losing that too...

Missed watchpoints will need yet further fixes, but we should keep
those in mind.

So the fix must be to let the trap fall through the regular bpstat
handling, and only if no breakpoint, watchpoint, etc. claims the trap,
shall we switch back to the stepped thread.

Now, nowadays, we have code at the tail end of trap handling that does
exactly that -- switch back to the stepped thread
(switch_back_to_the_stepped_thread).

So the deferred_step_ptid code is just standing in the way, and can
simply be eliminated, fixing bugs in the process.  Sweet.

The comment about spurious "Switching to ..." made me pause, but is
actually stale nowadays.  That isn't needed anymore.
previous_inferior_ptid used to be re-set at each (internal) event, but
now it's only touched in proceed and normal stop.

The two tests added by this patch fail without the fix.

Tested on x86_64 Fedora 17 (also against my software single-stepping
on x86 branch).

gdb/
2014-03-20  Pedro Alves  <palves@redhat.com>

* infrun.c (previous_inferior_ptid): Adjust comment.
(deferred_step_ptid): Delete.
(infrun_thread_ptid_changed, prepare_to_proceed)
(init_wait_for_inferior): Adjust.
(handle_signal_stop): Delete deferred_step_ptid handling.

gdb/testsuite/
2014-03-20  Pedro Alves  <palves@redhat.com>

* gdb.threads/step-over-lands-on-breakpoint.c: New file.
* gdb.threads/step-over-lands-on-breakpoint.exp: New file.
gdb/ChangeLog
gdb/infrun.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c [new file with mode: 0644]
gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp [new file with mode: 0644]
This page took 0.025181 seconds and 4 git commands to generate.