7f5ef60532
TL;DR - if we step an instruction that is as long as
decr_pc_after_break (1-byte on x86) right after removing the
breakpoint at PC, in non-stop mode, adjust_pc_after_break adjusts the
PC, but it shouldn't.
In non-stop mode, when a breakpoint is removed, it is moved to the
"moribund locations" list. This is because other threads that are
running may have tripped on that breakpoint as well, and we haven't
heard about it. When a trap is reported, we check if perhaps it was
such a deleted breakpoint that caused the trap. If so, we also need
to adjust the PC (decr_pc_after_break).
Now, say that, on x86:
- a breakpoint was placed at an address where we have an instruction
of the same length as decr_pc_after_break on this arch (1 on x86).
- the breakpoint is removed, and thus put on the moribund locations
list.
- the thread is single-stepped.
As there's no breakpoint inserted at PC anymore, the single-step
actually executes the 1-byte instruction normally. GDB should _not_
adjust the PC for the resulting SIGTRAP. But, adjust_pc_after_break
confuses the step SIGTRAP reported for this single-step as being a
SIGTRAP for the moribund location of the breakpoint that used to be at
the previous PC, and so infrun applies the decr_pc_after_break
adjustment incorrectly.
The confusion comes from the special case mentioned in the comment:
static void
adjust_pc_after_break (struct execution_control_state *ecs)
{
...
As a special case, we could have hardware single-stepped a
software breakpoint. In this case (prev_pc == breakpoint_pc),
we also need to back up to the breakpoint address. */
if (thread_has_single_step_breakpoints_set (ecs->event_thread)
|| !ptid_equal (ecs->ptid, inferior_ptid)
|| !currently_stepping (ecs->event_thread)
|| (ecs->event_thread->stepped_breakpoint
&& ecs->event_thread->prev_pc == breakpoint_pc))
regcache_write_pc (regcache, breakpoint_pc);
The condition that incorrectly triggers is the
"ecs->event_thread->prev_pc == breakpoint_pc" one.
Afterwards, the next resume resume re-executes an instruction that had
already executed, which if you're lucky, results in the inferior
crashing. If you're unlucky, you'll get silent bad behavior...
The fix is to remember that we stepped a breakpoint. Turns out the
only case we step a breakpoint instruction today isn't covered by the
testsuite. It's the case of a 'handle nostop" signal arriving while a
step is in progress _and_ we have a software watchpoint, which forces
always single-stepping. This commit extends sigstep.exp to cover
that, and adds a new test for the adjust_pc_after_break issue.
Tested on x86_64 Fedora 20, native and gdbserver.
gdb/
2014-10-28 Pedro Alves <palves@redhat.com>
PR gdb/12623
* gdbthread.h (struct thread_info) <stepped_breakpoint>: New
field.
* infrun.c (resume) <stepping breakpoint instruction>: Set the
thread's stepped_breakpoint field. Skip if reverse debugging.
Add comment.
(init_thread_stepping_state, handle_signal_stop): Clear the
thread's stepped_breakpoint field.
gdb/testsuite/
2014-10-28 Pedro Alves <palves@redhat.com>
PR gdb/12623
* gdb.base/sigstep.c (no_handler): New global.
(main): If 'no_handler is true, set the signal handlers to
SIG_IGN.
* gdb.base/sigstep.exp (breakpoint_over_handler): Add
with_sw_watch and no_handler parameters. Handle them.
(top level) <stepping over handler when stopped at a breakpoint
test>: Add a test axis for testing with a software watchpoint, and
another for testing with the signal handler set to SIG_IGN.
* gdb.base/step-sw-breakpoint-adjust-pc.c: New file.
* gdb.base/step-sw-breakpoint-adjust-pc.exp: New file.
91 lines
2.6 KiB
C
91 lines
2.6 KiB
C
/* This testcase is part of GDB, the GNU debugger.
|
|
|
|
Copyright 2004-2014 Free Software Foundation, Inc.
|
|
|
|
This program is free software; you can redistribute it and/or modify
|
|
it under the terms of the GNU General Public License as published by
|
|
the Free Software Foundation; either version 3 of the License, or
|
|
(at your option) any later version.
|
|
|
|
This program is distributed in the hope that it will be useful,
|
|
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
GNU General Public License for more details.
|
|
|
|
You should have received a copy of the GNU General Public License
|
|
along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
|
|
*/
|
|
|
|
#include <stdio.h>
|
|
#include <string.h>
|
|
#include <signal.h>
|
|
#include <sys/time.h>
|
|
#include <errno.h>
|
|
|
|
static volatile int done;
|
|
static volatile int dummy;
|
|
static volatile int no_handler;
|
|
|
|
static void
|
|
handler (int sig)
|
|
{
|
|
/* This is more than one write so that the breakpoint location below
|
|
is more than one instruction away. */
|
|
done = 1;
|
|
done = 1;
|
|
done = 1;
|
|
done = 1; /* other handler location */
|
|
} /* handler */
|
|
|
|
struct itimerval itime;
|
|
struct sigaction action;
|
|
|
|
/* The enum is so that GDB can easily see these macro values. */
|
|
enum {
|
|
itimer_real = ITIMER_REAL,
|
|
itimer_virtual = ITIMER_VIRTUAL
|
|
} itimer = ITIMER_VIRTUAL;
|
|
|
|
int
|
|
main ()
|
|
{
|
|
int res;
|
|
|
|
/* Set up the signal handler. */
|
|
memset (&action, 0, sizeof (action));
|
|
action.sa_handler = no_handler ? SIG_IGN : handler;
|
|
sigaction (SIGVTALRM, &action, NULL);
|
|
sigaction (SIGALRM, &action, NULL);
|
|
|
|
/* The values needed for the itimer. This needs to be at least long
|
|
enough for the setitimer() call to return. */
|
|
memset (&itime, 0, sizeof (itime));
|
|
itime.it_value.tv_usec = 250 * 1000;
|
|
|
|
/* Loop for ever, constantly taking an interrupt. */
|
|
while (1)
|
|
{
|
|
/* Set up a one-off timer. A timer, rather than SIGSEGV, is
|
|
used as after a timer handler finishes the interrupted code
|
|
can safely resume. */
|
|
res = setitimer (itimer, &itime, NULL);
|
|
if (res == -1)
|
|
{
|
|
printf ("First call to setitimer failed, errno = %d\r\n",errno);
|
|
itimer = ITIMER_REAL;
|
|
res = setitimer (itimer, &itime, NULL);
|
|
if (res == -1)
|
|
{
|
|
printf ("Second call to setitimer failed, errno = %d\r\n",errno);
|
|
return 1;
|
|
}
|
|
}
|
|
/* Wait. Issue a couple writes to a dummy volatile var to be
|
|
reasonably sure our simple "get-next-pc" logic doesn't
|
|
stumble on branches. */
|
|
dummy = 0; dummy = 0; while (!done);
|
|
done = 0;
|
|
}
|
|
return 0;
|
|
}
|