diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b358dd71805..9901d164ba7 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2014-10-28 Pedro Alves + + PR gdb/12623 + * gdbthread.h (struct thread_info) : New + field. + * infrun.c (resume) : 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. + 2014-10-27 Pedro Alves * remote.c (remote_thread_alive): New, factored out from ... diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index fb47baebda7..4fd5f69283f 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -201,6 +201,11 @@ struct thread_info SIGTRAP from a breakpoint SIGTRAP. */ CORE_ADDR prev_pc; + /* Did we set the thread stepping a breakpoint instruction? This is + used in conjunction with PREV_PC to decide whether to adjust the + PC. */ + int stepped_breakpoint; + /* Should we step over breakpoint next time keep_going is called? */ int stepping_over_breakpoint; diff --git a/gdb/infrun.c b/gdb/infrun.c index df053e26ffe..7af5e0f58c3 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2213,12 +2213,35 @@ a command like `return' or `jump' to continue execution.")); resume_ptid = inferior_ptid; } - if (gdbarch_cannot_step_breakpoint (gdbarch)) + if (execution_direction != EXEC_REVERSE + && step && breakpoint_inserted_here_p (aspace, pc)) { + /* The only case we currently need to step a breakpoint + instruction is when we have a signal to deliver. See + handle_signal_stop where we handle random signals that could + take out us out of the stepping range. Normally, in that + case we end up continuing (instead of stepping) over the + signal handler with a breakpoint at PC, but there are cases + where we should _always_ single-step, even if we have a + step-resume breakpoint, like when a software watchpoint is + set. Assuming single-stepping and delivering a signal at the + same time would takes us to the signal handler, then we could + have removed the breakpoint at PC to step over it. However, + some hardware step targets (like e.g., Mac OS) can't step + into signal handlers, and for those, we need to leave the + breakpoint at PC inserted, as otherwise if the handler + recurses and executes PC again, it'll miss the breakpoint. + So we leave the breakpoint inserted anyway, but we need to + record that we tried to step a breakpoint instruction, so + that adjust_pc_after_break doesn't end up confused. */ + gdb_assert (sig != GDB_SIGNAL_0); + + tp->stepped_breakpoint = 1; + /* Most targets can step a breakpoint instruction, thus executing it normally. But if this one cannot, just continue and we will hit it anyway. */ - if (step && breakpoint_inserted_here_p (aspace, pc)) + if (gdbarch_cannot_step_breakpoint (gdbarch)) step = 0; } @@ -3221,6 +3244,7 @@ set_step_info (struct frame_info *frame, struct symtab_and_line sal) void init_thread_stepping_state (struct thread_info *tss) { + tss->stepped_breakpoint = 0; tss->stepping_over_breakpoint = 0; tss->stepping_over_watchpoint = 0; tss->step_after_step_resume_breakpoint = 0; @@ -3385,7 +3409,8 @@ adjust_pc_after_break (struct execution_control_state *ecs) if (thread_has_single_step_breakpoints_set (ecs->event_thread) || !ptid_equal (ecs->ptid, inferior_ptid) || !currently_stepping (ecs->event_thread) - || ecs->event_thread->prev_pc == breakpoint_pc) + || (ecs->event_thread->stepped_breakpoint + && ecs->event_thread->prev_pc == breakpoint_pc)) regcache_write_pc (regcache, breakpoint_pc); do_cleanups (old_cleanups); @@ -4241,6 +4266,7 @@ 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); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index cada90d15ea..5d2d265393e 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,17 @@ +2014-10-28 Pedro Alves + + 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) : 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. + 2014-10-28 Pedro Alves PR gdb/17511 diff --git a/gdb/testsuite/gdb.base/sigstep.c b/gdb/testsuite/gdb.base/sigstep.c index 38dd1fb1c2e..bc0f67b307f 100644 --- a/gdb/testsuite/gdb.base/sigstep.c +++ b/gdb/testsuite/gdb.base/sigstep.c @@ -25,6 +25,7 @@ static volatile int done; static volatile int dummy; +static volatile int no_handler; static void handler (int sig) @@ -49,11 +50,11 @@ enum { int main () { - int res; + /* Set up the signal handler. */ memset (&action, 0, sizeof (action)); - action.sa_handler = handler; + action.sa_handler = no_handler ? SIG_IGN : handler; sigaction (SIGVTALRM, &action, NULL); sigaction (SIGALRM, &action, NULL); diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp index 08ad828f121..c4580a7a672 100644 --- a/gdb/testsuite/gdb.base/sigstep.exp +++ b/gdb/testsuite/gdb.base/sigstep.exp @@ -476,18 +476,37 @@ foreach cmd {"step" "next" "continue"} { # Try stepping when there's a signal pending, and a pre-existing # breakpoint at the current instruction, and no breakpoint in the -# handler. Should advance to the next line/instruction. +# handler. Should advance to the next line/instruction. If SW_WATCH +# is true, set a software watchpoint, which exercises stepping the +# breakpoint instruction while delivering a signal at the same time. +# If NO_HANDLER, arrange for the signal's handler be SIG_IGN, thus +# when the software watchpoint is also set, testing stepping a +# breakpoint instruction and immediately triggering the breakpoint +# (exercises adjust_pc_after_break logic). -proc breakpoint_over_handler { cmd } { +proc breakpoint_over_handler { cmd with_sw_watch no_handler } { global infinite_loop global clear_done - with_test_prefix "$cmd on breakpoint, skip handler" { + set prefix "$cmd on breakpoint, skip handler" + if { $with_sw_watch } { + append prefix ", with sw-watchpoint" + } + if { $no_handler } { + append prefix ", no handler" + } + + with_test_prefix "$prefix" { restart # Use the real-time itimer, as otherwize the process never gets # enough time to expire the timer. gdb_test_no_output "set itimer = itimer_real" + if {$no_handler} { + gdb_test "print no_handler = 1" " = 1" \ + "set no_handler" + } + gdb_test "break $infinite_loop" ".*" "break infinite loop" gdb_test "break $clear_done" ".*" "break clear done" @@ -498,10 +517,26 @@ proc breakpoint_over_handler { cmd } { # Make the signal pending sleep 1 + if { $with_sw_watch } { + # A watchpoint on a convenience variable is always a + # software watchpoint. + gdb_test "watch \$convenience" "Watchpoint .*: \\\$convenience" + } + + if {$no_handler} { + # With no handler, we need to set the global ourselves + # manually. + gdb_test "print done = 1" " = 1" "set done" + } + test_skip_handler $cmd } } foreach cmd {"stepi" "nexti" "step" "next" "continue"} { - breakpoint_over_handler $cmd + foreach with_sw_watch {0 1} { + foreach no_handler {0 1} { + breakpoint_over_handler $cmd $with_sw_watch $no_handler + } + } } diff --git a/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.c b/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.c new file mode 100644 index 00000000000..f7b6e70a697 --- /dev/null +++ b/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.c @@ -0,0 +1,50 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 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 . */ + +/* An instruction with the same length as decr_pc_after_break. This + is 1-byte on x86. */ +#define INSN asm ("nop") + +void +test_user_bp (void) +{ + INSN; /* break for user-bp test here */ + INSN; /* insn1 */ + INSN; /* insn2 */ + INSN; /* insn3 */ +} + +void +foo (void) +{ +} + +void +test_step_resume (void) +{ + foo (); /* break for step-resume test here */ + INSN; /* insn1 */ + INSN; /* insn2 */ +} + +int +main (void) +{ + test_user_bp (); + test_step_resume (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp b/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp new file mode 100644 index 00000000000..4d08a4418ed --- /dev/null +++ b/gdb/testsuite/gdb.base/step-sw-breakpoint-adjust-pc.exp @@ -0,0 +1,94 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 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 . + +# Test tstepping an instruction just as long as decr_pc_after_break +# after removing a breakpoint at PC. GDB used to get confused with +# this in non-stop mode, and adjust the PC incorrectly. PR gdb/12623. + +standard_testfile + +if [build_executable "failed to build" ${testfile} ${srcfile} {debug}] { + return -1 +} + +set linenum_for_user_bp [gdb_get_line_number "break for user-bp test here"] +set linenum_for_step_resume [gdb_get_line_number "break for step-resume test here"] + +proc test {non_stop displaced always_inserted} { + global binfile + global linenum_for_user_bp + global linenum_for_step_resume + + clean_restart $binfile + + gdb_test_no_output "set non-stop $non_stop" + gdb_test_no_output "set displaced-stepping $displaced" + gdb_test_no_output "set breakpoint always-inserted $always_inserted" + + if ![runto_main] { + return -1 + } + + with_test_prefix "user bp" { + delete_breakpoints + + gdb_breakpoint $linenum_for_user_bp + gdb_continue_to_breakpoint "continue to breakpoint" + + # If breakpoint always-inserted is on, this makes the location + # moribund. + delete_breakpoints + + gdb_test "si" "INSN.*insn1.*" "si advances" + } + + with_test_prefix "step-resume" { + delete_breakpoints + + gdb_breakpoint $linenum_for_step_resume + gdb_continue_to_breakpoint "continue to breakpoint" + + gdb_test "next" "insn1.*" + + # We're now stopped where the step-resume breakpoint for the + # previous "next" was. That breakpoint was removed and is now + # on the moribund locations list. + gdb_test "si" "INSN.*insn2.*" "si advances" + + delete_breakpoints + } +} + +# Wrapper for foreach that calls with_test_prefix on each iteration, +# including the iterator's current value in the prefix. + +proc foreach_with_prefix {var list body} { + upvar 1 $var myvar + foreach myvar $list { + with_test_prefix "$var=$myvar" { + uplevel 1 $body + } + } +} + +foreach_with_prefix non_stop { "off" "on" } { + foreach_with_prefix displaced_step { "off" "on" } { + foreach_with_prefix always_inserted { "off" "on" } { + test $non_stop $displaced_step $always_inserted + } + } +}