Consecutive step-overs trigger internal error.

If a thread trips on a breakpoint that needs stepping over just after
finishing a step over, GDB currently fails an assertion.  This is a
regression caused by the "Handle multiple step-overs." patch
(99619beac6) at
https://sourceware.org/ml/gdb-patches/2014-02/msg00765.html.

 (gdb) x /4i $pc
 => 0x400540 <main+4>:   movl   $0x0,0x2003da(%rip)        # 0x600924 <i>
    0x40054a <main+14>:  movl   $0x1,0x2003d0(%rip)        # 0x600924 <i>
    0x400554 <main+24>:  movl   $0x2,0x2003c6(%rip)        # 0x600924 <i>
    0x40055e <main+34>:  movl   $0x3,0x2003bc(%rip)        # 0x600924 <i>
 (gdb) PASS: gdb.base/consecutive-step-over.exp: get breakpoint addresses
 break *0x40054a
 Breakpoint 2 at 0x40054a: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 23.
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 1: set breakpoint
 condition $bpnum condition
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 1: set condition
 break *0x400554
 Breakpoint 3 at 0x400554: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 24.
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 2: set breakpoint
 condition $bpnum condition
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 2: set condition
 break *0x40055e
 Breakpoint 4 at 0x40055e: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 25.
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 3: set breakpoint
 condition $bpnum condition
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 3: set condition
 break 27
 Breakpoint 5 at 0x400568: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 27.
 (gdb) continue
 Continuing.
 ../../src/gdb/infrun.c:5200: internal-error: switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
 A problem internal to GDB has been detected,
 further debugging may prove unreliable.
 FAIL: gdb.base/consecutive-step-over.exp: continue to breakpoint: break here (GDB internal error)

The assertion fails, because the code is not expecting that the event
thread itself might need another step over.  IOW, not expecting that
TP in:

     tp = find_thread_needs_step_over (stepping_thread != NULL,
                                      stepping_thread);

could be the event thread.

A small fix for this would be to clear the event thread's
trap_expected earlier, before asserting.  But looking deeper, although
currently_stepping_or_nexting_callback's intention is finding the
thread that is doing a step/next, it also returns the thread that is
doing a step-over dance, with trap_expected set.  If there ever was a
reason for that (it was I who added
currently_stepping_or_nexting_callback , but I can't recall why I put
trap_expected there in the first place), the only remaining reason
nowadays is to aid in implementing switch_back_to_stepped_thread's
assertion that is now triggering, by piggybacking on the walk over all
threads, thus avoiding a separate walk.  This is quite obscure, and I
think we can do even better, by merging the walks that look for the
stepping thread, and the walk that looks for some thread that might
need a step over.

Tested on x86_64 Fedora 17, native and gdbserver, and also native on
top of my "software single-step on x86_64" series.

gdb/
2014-04-22  Pedro Alves  <palves@redhat.com>

	* infrun.c (schedlock_applies): New function, factored out from
	find_thread_needs_step_over.
	(find_thread_needs_step_over): Use it.
	(switch_back_to_stepped_thread): Always clear trap_expected if the
	step over is finished.  Return early if scheduler locking applies.
	Look for the stepping thread and a potential step-over thread with
	a single loop.
	(currently_stepping_or_nexting_callback): Delete.

2014-04-22  Pedro Alves  <palves@redhat.com>

	* gdb.base/consecutive-step-over.c: New file.
	* gdb.base/consecutive-step-over.exp: New file.
This commit is contained in:
Pedro Alves 2014-04-22 15:00:56 +01:00
parent 06d9754365
commit 483805cf9e
5 changed files with 195 additions and 44 deletions

View File

@ -1,3 +1,14 @@
2014-04-22 Pedro Alves <palves@redhat.com>
* infrun.c (schedlock_applies): New function, factored out from
find_thread_needs_step_over.
(find_thread_needs_step_over): Use it.
(switch_back_to_stepped_thread): Always clear trap_expected if the
step over is finished. Return early if scheduler locking applies.
Look for the stepping thread and a potential step-over thread with
a single loop.
(currently_stepping_or_nexting_callback): Delete.
2014-04-22 Nick Clifton <nickc@redhat.com>
* NEWS: Mention that ARM sim now supports tracing.

View File

@ -85,9 +85,6 @@ static void set_schedlock_func (char *args, int from_tty,
static int currently_stepping (struct thread_info *tp);
static int currently_stepping_or_nexting_callback (struct thread_info *tp,
void *data);
static void xdb_handle_command (char *args, int from_tty);
static void print_exited_reason (int exitstatus);
@ -2107,6 +2104,17 @@ thread_still_needs_step_over (struct thread_info *tp)
return 0;
}
/* Returns true if scheduler locking applies. STEP indicates whether
we're about to do a step/next-like command to a thread. */
static int
schedlock_applies (int step)
{
return (scheduler_mode == schedlock_on
|| (scheduler_mode == schedlock_step
&& step));
}
/* Look a thread other than EXCEPT that has previously reported a
breakpoint event, and thus needs a step-over in order to make
progress. Returns NULL is none is found. STEP indicates whether
@ -2116,21 +2124,16 @@ thread_still_needs_step_over (struct thread_info *tp)
static struct thread_info *
find_thread_needs_step_over (int step, struct thread_info *except)
{
int schedlock_enabled;
struct thread_info *tp, *current;
/* With non-stop mode on, threads are always handled individually. */
gdb_assert (! non_stop);
schedlock_enabled = (scheduler_mode == schedlock_on
|| (scheduler_mode == schedlock_step
&& step));
current = inferior_thread ();
/* If scheduler locking applies, we can avoid iterating over all
threads. */
if (schedlock_enabled)
if (schedlock_applies (step))
{
if (except != current
&& thread_still_needs_step_over (current))
@ -5137,6 +5140,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
{
struct thread_info *tp;
struct thread_info *stepping_thread;
struct thread_info *step_over;
/* If any thread is blocked on some internal breakpoint, and we
simply need to step over that breakpoint to get it going
@ -5179,17 +5183,72 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
return 1;
}
stepping_thread
= iterate_over_threads (currently_stepping_or_nexting_callback,
ecs->event_thread);
/* Otherwise, we no longer expect a trap in the current thread.
Clear the trap_expected flag before switching back -- this is
what keep_going does as well, if we call it. */
ecs->event_thread->control.trap_expected = 0;
/* Check if any other thread except the stepping thread that
needs to start a step-over. Do that before actually
proceeding with step/next/etc. */
tp = find_thread_needs_step_over (stepping_thread != NULL,
stepping_thread);
if (tp != NULL)
/* If scheduler locking applies even if not stepping, there's no
need to walk over threads. Above we've checked whether the
current thread is stepping. If some other thread not the
event thread is stepping, then it must be that scheduler
locking is not in effect. */
if (schedlock_applies (0))
return 0;
/* Look for the stepping/nexting thread, and check if any other
thread other than the stepping thread needs to start a
step-over. Do all step-overs before actually proceeding with
step/next/etc. */
stepping_thread = NULL;
step_over = NULL;
ALL_THREADS (tp)
{
/* Ignore threads of processes we're not resuming. */
if (!sched_multi
&& ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
continue;
/* When stepping over a breakpoint, we lock all threads
except the one that needs to move past the breakpoint.
If a non-event thread has this set, the "incomplete
step-over" check above should have caught it earlier. */
gdb_assert (!tp->control.trap_expected);
/* Did we find the stepping thread? */
if (tp->control.step_range_end)
{
/* Yep. There should only one though. */
gdb_assert (stepping_thread == NULL);
/* The event thread is handled at the top, before we
enter this loop. */
gdb_assert (tp != ecs->event_thread);
/* If some thread other than the event thread is
stepping, then scheduler locking can't be in effect,
otherwise we wouldn't have resumed the current event
thread in the first place. */
gdb_assert (!schedlock_applies (1));
stepping_thread = tp;
}
else if (thread_still_needs_step_over (tp))
{
step_over = tp;
/* At the top we've returned early if the event thread
is stepping. If some other thread not the event
thread is stepping, then scheduler locking can't be
in effect, and we can resume this thread. No need to
keep looking for the stepping thread then. */
break;
}
}
if (step_over != NULL)
{
tp = step_over;
if (debug_infrun)
{
fprintf_unfiltered (gdb_stdlog,
@ -5197,14 +5256,9 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
target_pid_to_str (tp->ptid));
}
gdb_assert (!tp->control.trap_expected);
/* Only the stepping thread should have this set. */
gdb_assert (tp->control.step_range_end == 0);
/* We no longer expect a trap in the current thread. Clear
the trap_expected flag before switching. This is what
keep_going would do as well, if we called it. */
ecs->event_thread->control.trap_expected = 0;
ecs->ptid = tp->ptid;
ecs->event_thread = tp;
switch_to_thread (ecs->ptid);
@ -5212,12 +5266,13 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
return 1;
}
tp = stepping_thread;
if (tp != NULL)
if (stepping_thread != NULL)
{
struct frame_info *frame;
struct gdbarch *gdbarch;
tp = stepping_thread;
/* If the stepping thread exited, then don't try to switch
back and resume it, which could fail in several different
ways depending on the target. Instead, just keep going.
@ -5250,11 +5305,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
return 1;
}
/* Otherwise, we no longer expect a trap in the current thread.
Clear the trap_expected flag before switching back -- this is
what keep_going would do as well, if we called it. */
ecs->event_thread->control.trap_expected = 0;
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: switching back to stepped thread\n");
@ -5325,19 +5375,6 @@ currently_stepping (struct thread_info *tp)
|| bpstat_should_step ());
}
/* Returns true if any thread *but* the one passed in "data" is in the
middle of stepping or of handling a "next". */
static int
currently_stepping_or_nexting_callback (struct thread_info *tp, void *data)
{
if (tp == data)
return 0;
return (tp->control.step_range_end
|| tp->control.trap_expected);
}
/* Inferior has stepped into a subroutine call with source code that
we should not step over. Do step to the first line of code in
it. */

View File

@ -1,3 +1,8 @@
2014-04-22 Pedro Alves <palves@redhat.com>
* gdb.base/consecutive-step-over.c: New file.
* gdb.base/consecutive-step-over.exp: New file.
2014-04-22 Pedro Alves <palves@redhat.com>
* lib/gdb.exp (gdb_continue_to_breakpoint): Use gdb_test_multiple

View File

@ -0,0 +1,28 @@
/* 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 <http://www.gnu.org/licenses/>. */
volatile int i;
volatile int condition;
int
main (void)
{
i = 0;
i = 1;
i = 2;
i = 3;
return 0; /* break here */
}

View File

@ -0,0 +1,70 @@
# 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 <http://www.gnu.org/licenses/>.
#
# Regression test for a bug where GDB would internal error if a thread
# runs into a breakpoint that needs stepping over, just after stepping
# over another breakpoint, without a user visible stop in between.
#
standard_testfile
if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
return -1
}
if ![runto_main] then {
fail "Can't run to main"
return 0
}
# Make sure the target doesn't hide the breakpoint hits (that don't
# cause a user visible stop) from GDB.
gdb_test_no_output "set breakpoint condition-evaluation host"
set up_to_nl "\[^\r\n\]+\[\r\n\]+"
# Number of consecutive breakpoints in a row to try.
set n_insns 3
# Extract addresses of a few consecutive instructions.
set test "get breakpoint addresses"
if { [gdb_test_multiple "x /[expr $n_insns + 1]i \$pc" $test {
-re "=> $hex${up_to_nl} ($hex)${up_to_nl} ($hex)${up_to_nl} ($hex)${up_to_nl}$gdb_prompt $" {
for {set i 1} {$i <= $n_insns} {incr i} {
set bp_addrs($i) $expect_out($i,string)
}
pass $test
}
}] != 0 } {
# No use proceeding if bp_addrs wasn't set.
return
}
for {set i 1} {$i <= $n_insns} {incr i} {
with_test_prefix "insn $i" {
gdb_test "break \*$bp_addrs($i)" \
"Breakpoint $decimal at $bp_addrs($i): file .*" \
"set breakpoint"
# Give the breakpoint a condition that always fails, so that
# the thread is immediately re-resumed.
gdb_test_no_output "condition \$bpnum condition" \
"set condition"
}
}
set lineno [gdb_get_line_number "break here"]
gdb_breakpoint $lineno
gdb_continue_to_breakpoint "break here" ".*break here.*"