fix skipping permanent breakpoints

The gdb.arch/i386-bp_permanent.exp test is currently failing an
assertion recently added:

 (gdb) stepi
 ../../src/gdb/infrun.c:2237: internal-error: resume: Assertion `sig != GDB_SIGNAL_0' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 Quit this debugging session? (y or n)
 FAIL: gdb.arch/i386-bp_permanent.exp: Single stepping past permanent breakpoint. (GDB internal error)

The assertion expects that the only reason we currently need to step a
breakpoint instruction is when we have a signal to deliver.  But when
stepping a permanent breakpoint (with or without a signal) we also
reach this code.

The assertion is correct and the permanent breakpoints skipping code
is wrong.

Consider the case of the user doing "step/stepi" when stopped at a
permanent breakpoint.  GDB's `resume' calls the
gdbarch_skip_permanent_breakpoint hook and then happily continues
stepping:

  /* Normally, by the time we reach `resume', the breakpoints are either
     removed or inserted, as appropriate.  The exception is if we're sitting
     at a permanent breakpoint; we need to step over it, but permanent
     breakpoints can't be removed.  So we have to test for it here.  */
  if (breakpoint_here_p (aspace, pc) == permanent_breakpoint_here)
    {
      gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
    }

But since gdbarch_skip_permanent_breakpoint already advanced the PC
manually, this ends up executing the instruction that is _after_ the
breakpoint instruction.  The user-visible result is that a single-step
steps two instructions.

The gdb.arch/i386-bp_permanent.exp test is actually ensuring that
that's indeed how things work.  It runs to an int3 instruction, does
"stepi", and checks that "leave" was executed with that "stepi".  Like
this:

 (gdb) b *0x0804848c
 Breakpoint 2 at 0x804848c
 (gdb) c
 Continuing.

 Breakpoint 2, 0x0804848c in standard ()
 (gdb) disassemble
 Dump of assembler code for function standard:
    0x08048488 <+0>:     push   %ebp
    0x08048489 <+1>:     mov    %esp,%ebp
    0x0804848b <+3>:     push   %edi
 => 0x0804848c <+4>:     int3
    0x0804848d <+5>:     leave
    0x0804848e <+6>:     ret
    0x0804848f <+7>:     nop
 (gdb) si
 0x0804848e in standard ()
 (gdb) disassemble
 Dump of assembler code for function standard:
    0x08048488 <+0>:     push   %ebp
    0x08048489 <+1>:     mov    %esp,%ebp
    0x0804848b <+3>:     push   %edi
    0x0804848c <+4>:     int3
    0x0804848d <+5>:     leave
 => 0x0804848e <+6>:     ret
    0x0804848f <+7>:     nop
 End of assembler dump.
 (gdb)

One would instead expect that a stepi at 0x0804848c stops at
0x0804848d, _before_ the "leave" is executed.  This commit changes GDB
this way.  Care is taken to make stepping into a signal handler when
the step starts at a permanent breakpoint instruction work correctly.

The patch adjusts gdb.arch/i386-bp_permanent.exp in this direction,
and also makes it work on x86_64 (currently it only works on i*86).

The patch also adds a new gdb.base/bp-permanent.exp test that
exercises many different code paths related to stepping permanent
breakpoints, including the stepping with signals cases.  The test uses
"hack/trick" to make it work on all (or most) platforms -- it doesn't
really hard code a breakpoint instruction.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-11-12  Pedro Alves  <palves@redhat.com>

	* infrun.c (resume): Clear the thread's 'stepped_breakpoint' flag.
	Rewrite stepping over a permanent breakpoint.
	(thread_still_needs_step_over, proceed): Don't set
	stepping_over_breakpoint for permanent breakpoints.
	(handle_signal_stop): Don't clear stepped_breakpoint.  Also pull
	single-step breakpoints out of the target on hardware step
	targets.
	(process_event_stop_test): If stepping a permanent breakpoint
	doesn't hit the step-resume breakpoint, delete the step-resume
	breakpoint.
	(switch_back_to_stepped_thread): Also check if the stepped thread
	has advanced already on hardware step targets.
	(currently_stepping): Return true if the thread stepped a
	breakpoint.

gdb/testsuite/
2014-11-12  Pedro Alves  <palves@redhat.com>

	* gdb.arch/i386-bp_permanent.c: New file.
	* gdb.arch/i386-bp_permanent.exp: Don't skip on x86_64.
	(srcfile): Set to i386-bp_permanent.c.
	(top level): Adjust to work in both 32-bit and 64-bit modes.  Test
	that stepi does not execute the 'leave' instruction, instead of
	testing it does execute.
	* gdb.base/bp-permanent.c: New file.
	* gdb.base/bp-permanent.exp: New file.
This commit is contained in:
Pedro Alves
2014-11-12 10:10:49 +00:00
parent 1a853c5224
commit af48d08f97
7 changed files with 644 additions and 66 deletions

View File

@ -2039,6 +2039,8 @@ resume (int step, enum gdb_signal sig)
applies, it's the callers intention that counts. */
const int entry_step = step;
tp->stepped_breakpoint = 0;
QUIT;
if (current_inferior ()->waiting_for_vfork_done)
@ -2075,7 +2077,81 @@ resume (int step, enum gdb_signal sig)
breakpoints can't be removed. So we have to test for it here. */
if (breakpoint_here_p (aspace, pc) == permanent_breakpoint_here)
{
gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
if (sig != GDB_SIGNAL_0)
{
/* We have a signal to pass to the inferior. The resume
may, or may not take us to the signal handler. If this
is a step, we'll need to stop in the signal handler, if
there's one, (if the target supports stepping into
handlers), or in the next mainline instruction, if
there's no handler. If this is a continue, we need to be
sure to run the handler with all breakpoints inserted.
In all cases, set a breakpoint at the current address
(where the handler returns to), and once that breakpoint
is hit, resume skipping the permanent breakpoint. If
that breakpoint isn't hit, then we've stepped into the
signal handler (or hit some other event). We'll delete
the step-resume breakpoint then. */
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: resume: skipping permanent breakpoint, "
"deliver signal first\n");
clear_step_over_info ();
tp->control.trap_expected = 0;
if (tp->control.step_resume_breakpoint == NULL)
{
/* Set a "high-priority" step-resume, as we don't want
user breakpoints at PC to trigger (again) when this
hits. */
insert_hp_step_resume_breakpoint_at_frame (get_current_frame ());
gdb_assert (tp->control.step_resume_breakpoint->loc->permanent);
tp->step_after_step_resume_breakpoint = step;
}
insert_breakpoints ();
}
else
{
/* There's no signal to pass, we can go ahead and skip the
permanent breakpoint manually. */
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: resume: skipping permanent breakpoint\n");
gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
/* Update pc to reflect the new address from which we will
execute instructions. */
pc = regcache_read_pc (regcache);
if (step)
{
/* We've already advanced the PC, so the stepping part
is done. Now we need to arrange for a trap to be
reported to handle_inferior_event. Set a breakpoint
at the current PC, and run to it. Don't update
prev_pc, because if we end in
switch_back_to_stepping, we want the "expected thread
advanced also" branch to be taken. IOW, we don't
want this thread to step further from PC
(overstep). */
insert_single_step_breakpoint (gdbarch, aspace, pc);
insert_breakpoints ();
tp->suspend.stop_signal = GDB_SIGNAL_0;
/* We're continuing with all breakpoints inserted. It's
safe to let the target bypass signals. */
target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass);
/* ... and safe to let other threads run, according to
schedlock. */
resume_ptid = user_visible_resume_ptid (entry_step);
target_resume (resume_ptid, 0, GDB_SIGNAL_0);
discard_cleanups (old_cleanups);
return;
}
}
}
/* If we have a breakpoint to step over, make sure to do a single
@ -2382,7 +2458,8 @@ thread_still_needs_step_over (struct thread_info *tp)
struct regcache *regcache = get_thread_regcache (tp->ptid);
if (breakpoint_here_p (get_regcache_aspace (regcache),
regcache_read_pc (regcache)))
regcache_read_pc (regcache))
== ordinary_breakpoint_here)
return 1;
tp->stepping_over_breakpoint = 0;
@ -2498,7 +2575,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
if (addr == (CORE_ADDR) -1)
{
if (pc == stop_pc && breakpoint_here_p (aspace, pc)
if (pc == stop_pc
&& breakpoint_here_p (aspace, pc) == ordinary_breakpoint_here
&& execution_direction != EXEC_REVERSE)
/* There is a breakpoint at the address we will resume at,
step one instruction before inserting breakpoints so that
@ -4190,50 +4268,46 @@ handle_signal_stop (struct execution_control_state *ecs)
gdbarch = get_frame_arch (frame);
/* Pull the single step breakpoints out of the target. */
if (gdbarch_software_single_step_p (gdbarch))
if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
{
if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
struct regcache *regcache;
struct address_space *aspace;
CORE_ADDR pc;
regcache = get_thread_regcache (ecs->ptid);
aspace = get_regcache_aspace (regcache);
pc = regcache_read_pc (regcache);
/* However, before doing so, if this single-step breakpoint was
actually for another thread, set this thread up for moving
past it. */
if (!thread_has_single_step_breakpoint_here (ecs->event_thread,
aspace, pc))
{
struct regcache *regcache;
struct address_space *aspace;
CORE_ADDR pc;
regcache = get_thread_regcache (ecs->ptid);
aspace = get_regcache_aspace (regcache);
pc = regcache_read_pc (regcache);
/* However, before doing so, if this single-step breakpoint was
actually for another thread, set this thread up for moving
past it. */
if (!thread_has_single_step_breakpoint_here (ecs->event_thread,
aspace, pc))
{
if (single_step_breakpoint_inserted_here_p (aspace, pc))
{
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog,
"infrun: [%s] hit another thread's "
"single-step breakpoint\n",
target_pid_to_str (ecs->ptid));
}
ecs->hit_singlestep_breakpoint = 1;
}
}
else
if (single_step_breakpoint_inserted_here_p (aspace, pc))
{
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog,
"infrun: [%s] hit its "
"infrun: [%s] hit another thread's "
"single-step breakpoint\n",
target_pid_to_str (ecs->ptid));
}
ecs->hit_singlestep_breakpoint = 1;
}
}
else
{
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog,
"infrun: [%s] hit its "
"single-step breakpoint\n",
target_pid_to_str (ecs->ptid));
}
}
delete_just_stopped_threads_single_step_breakpoints ();
}
delete_just_stopped_threads_single_step_breakpoints ();
if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
&& ecs->event_thread->control.trap_expected
@ -4280,7 +4354,6 @@ handle_signal_stop (struct execution_control_state *ecs)
return;
}
ecs->event_thread->stepped_breakpoint = 0;
ecs->event_thread->stepping_over_breakpoint = 0;
ecs->event_thread->stepping_over_watchpoint = 0;
bpstat_clear (&ecs->event_thread->control.stop_bpstat);
@ -4788,6 +4861,30 @@ process_event_stop_test (struct execution_control_state *ecs)
break;
}
/* If we stepped a permanent breakpoint and we had a high priority
step-resume breakpoint for the address we stepped, but we didn't
hit it, then we must have stepped into the signal handler. The
step-resume was only necessary to catch the case of _not_
stepping into the handler, so delete it, and fall through to
checking whether the step finished. */
if (ecs->event_thread->stepped_breakpoint)
{
struct breakpoint *sr_bp
= ecs->event_thread->control.step_resume_breakpoint;
if (sr_bp->loc->permanent
&& sr_bp->type == bp_hp_step_resume
&& sr_bp->loc->address == ecs->event_thread->prev_pc)
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: stepped permanent breakpoint, stopped in "
"handler\n");
delete_step_resume_breakpoint (ecs->event_thread);
ecs->event_thread->step_after_step_resume_breakpoint = 0;
}
}
/* We come here if we hit a breakpoint but should not stop for it.
Possibly we also were stepping and should stop for that. So fall
through and test for stepping. But, if not stepping, do not
@ -5552,8 +5649,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
breakpoint forward, one instruction at a time,
overstepping. */
if (gdbarch_software_single_step_p (gdbarch)
&& stop_pc != tp->prev_pc)
if (stop_pc != tp->prev_pc)
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
@ -5598,6 +5694,7 @@ currently_stepping (struct thread_info *tp)
return ((tp->control.step_range_end
&& tp->control.step_resume_breakpoint == NULL)
|| tp->control.trap_expected
|| tp->stepped_breakpoint
|| bpstat_should_step ());
}