This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch 2/2] Fix stale tp->step_resume_breakpoint
On Wed, 24 Nov 2010 13:42:53 +0100, Pedro Alves wrote:
> > +++ b/gdb/testsuite/gdb.base/step-resume-infcall.c
> > @@ -0,0 +1,45 @@
> (...)
> > +#include <stdio.h>
> > +
> > +int
> > +cond (void)
> > +{
> > + puts ("cond-hit");
>
> Can you make the test not rely on stdio, please?
Done, didn't realize that before. (gdbserver case FAILs->PASSes as linux-nat)
No regressions on {x86_64,x86_64-m32,i686}-fedora14-linux-gnu.
Dropped the other fields save/restore, I was not able to reproduce problems
due to them and it is now very clear/easy to extend the save/restore set.
(Waiting on the fields moving cleanup approval.)
Thanks,
Jan
gdb/
2010-11-26 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix step_resume_breakpoint unsaved during an infcall.
* gdbthread.h (struct thread_control_state): Move here field
step_resume_breakpoint ...
(struct thread_info): ... from here.
* infrun.c (save_infcall_control_state): Reset
control.step_resume_breakpoint to NULL.
(restore_infcall_control_state, discard_infcall_control_state): Delete
control.step_resume_breakpoint.
* arm-linux-tdep.c, infrun.c, thread.c: Update all the references to
the moved field.
gdb/testsuite/
2010-11-26 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/step-resume-infcall.exp: New file.
* gdb.base/step-resume-infcall.c: New file.
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -791,7 +791,8 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to,
fprintf_unfiltered (gdb_stdlog, "displaced: unwind pc = %lx. "
"Setting momentary breakpoint.\n", (unsigned long) return_to);
- gdb_assert (inferior_thread ()->step_resume_breakpoint == NULL);
+ gdb_assert (inferior_thread ()->control.step_resume_breakpoint
+ == NULL);
sal = find_pc_line (return_to, 0);
sal.pc = return_to;
@@ -802,7 +803,7 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to,
if (frame)
{
- inferior_thread ()->step_resume_breakpoint
+ inferior_thread ()->control.step_resume_breakpoint
= set_momentary_breakpoint (gdbarch, sal, get_frame_id (frame),
bp_step_resume);
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -37,6 +37,9 @@ struct thread_control_state
{
/* User/external stepping state. */
+ /* Step-resume or longjmp-resume breakpoint. */
+ struct breakpoint *step_resume_breakpoint;
+
/* Range to single step within.
If this is nonzero, respond to a single-step signal by continuing
@@ -151,11 +154,6 @@ struct thread_info
call. See `struct thread_suspend_state'. */
struct thread_suspend_state suspend;
- /* User/external stepping state. */
-
- /* Step-resume or longjmp-resume breakpoint. */
- struct breakpoint *step_resume_breakpoint;
-
int current_line;
struct symtab *current_symtab;
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -424,8 +424,8 @@ follow_fork (void)
preserve the stepping state in the fork child. */
if (follow_child && should_resume)
{
- step_resume_breakpoint
- = clone_momentary_breakpoint (tp->step_resume_breakpoint);
+ step_resume_breakpoint = clone_momentary_breakpoint
+ (tp->control.step_resume_breakpoint);
step_range_start = tp->control.step_range_start;
step_range_end = tp->control.step_range_end;
step_frame_id = tp->control.step_frame_id;
@@ -476,7 +476,8 @@ follow_fork (void)
if (should_resume)
{
tp = inferior_thread ();
- tp->step_resume_breakpoint = step_resume_breakpoint;
+ tp->control.step_resume_breakpoint
+ = step_resume_breakpoint;
tp->control.step_range_start = step_range_start;
tp->control.step_range_end = step_range_end;
tp->control.step_frame_id = step_frame_id;
@@ -530,8 +531,8 @@ follow_inferior_reset_breakpoints (void)
"threads". We must update the bp's notion of which thread
it is for, or it'll be ignored when it triggers. */
- if (tp->step_resume_breakpoint)
- breakpoint_re_set_thread (tp->step_resume_breakpoint);
+ if (tp->control.step_resume_breakpoint)
+ breakpoint_re_set_thread (tp->control.step_resume_breakpoint);
/* Reinsert all breakpoints in the child. The user may have set
breakpoints after catching the fork, in which case those
@@ -769,7 +770,7 @@ follow_exec (ptid_t pid, char *execd_pathname)
/* If there was one, it's gone now. We cannot truly step-to-next
statement through an exec(). */
- th->step_resume_breakpoint = NULL;
+ th->control.step_resume_breakpoint = NULL;
th->control.step_range_start = 0;
th->control.step_range_end = 0;
@@ -3951,7 +3952,8 @@ infrun: no user watchpoint explains watchpoint SIGTRAP, ignoring\n");
|| stopped_by_watchpoint
|| ecs->event_thread->control.trap_expected
|| (ecs->event_thread->control.step_range_end
- && ecs->event_thread->step_resume_breakpoint == NULL));
+ && (ecs->event_thread->control.step_resume_breakpoint
+ == NULL)));
else
{
ecs->random_signal = !bpstat_explains_signal
@@ -4019,7 +4021,7 @@ process_event_stop_test:
if (ecs->event_thread->prev_pc == stop_pc
&& ecs->event_thread->control.trap_expected
- && ecs->event_thread->step_resume_breakpoint == NULL)
+ && ecs->event_thread->control.step_resume_breakpoint == NULL)
{
/* We were just starting a new sequence, attempting to
single-step off of a breakpoint and expecting a SIGTRAP.
@@ -4048,7 +4050,7 @@ process_event_stop_test:
&& stop_pc < ecs->event_thread->control.step_range_end)
&& frame_id_eq (get_stack_frame_id (frame),
ecs->event_thread->control.step_stack_frame_id)
- && ecs->event_thread->step_resume_breakpoint == NULL)
+ && ecs->event_thread->control.step_resume_breakpoint == NULL)
{
/* The inferior is about to take a signal that will take it
out of the single step range. Set a breakpoint at the
@@ -4135,7 +4137,8 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
fprintf_unfiltered (gdb_stdlog,
"infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
- gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
+ gdb_assert (ecs->event_thread->control.step_resume_breakpoint
+ != NULL);
delete_step_resume_breakpoint (ecs->event_thread);
ecs->event_thread->control.stop_step = 1;
@@ -4311,7 +4314,7 @@ infrun: not switching back to stepped thread, it has vanished\n");
return;
}
- if (ecs->event_thread->step_resume_breakpoint)
+ if (ecs->event_thread->control.step_resume_breakpoint)
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
@@ -4862,10 +4865,11 @@ infrun: not switching back to stepped thread, it has vanished\n");
static int
currently_stepping (struct thread_info *tp)
{
- return ((tp->control.step_range_end && tp->step_resume_breakpoint == NULL)
- || tp->control.trap_expected
- || tp->stepping_through_solib_after_catch
- || bpstat_should_step ());
+ return ((tp->control.step_range_end
+ && tp->control.step_resume_breakpoint == NULL)
+ || tp->control.trap_expected
+ || tp->stepping_through_solib_after_catch
+ || bpstat_should_step ());
}
/* Returns true if any thread *but* the one passed in "data" is in the
@@ -5010,14 +5014,14 @@ insert_step_resume_breakpoint_at_sal (struct gdbarch *gdbarch,
/* There should never be more than one step-resume or longjmp-resume
breakpoint per thread, so we should never be setting a new
step_resume_breakpoint when one is already active. */
- gdb_assert (inferior_thread ()->step_resume_breakpoint == NULL);
+ gdb_assert (inferior_thread ()->control.step_resume_breakpoint == NULL);
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: inserting step-resume breakpoint at %s\n",
paddress (gdbarch, sr_sal.pc));
- inferior_thread ()->step_resume_breakpoint
+ inferior_thread ()->control.step_resume_breakpoint
= set_momentary_breakpoint (gdbarch, sr_sal, sr_id, bp_step_resume);
}
@@ -5094,14 +5098,14 @@ insert_longjmp_resume_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pc)
/* There should never be more than one step-resume or longjmp-resume
breakpoint per thread, so we should never be setting a new
longjmp_resume_breakpoint when one is already active. */
- gdb_assert (inferior_thread ()->step_resume_breakpoint == NULL);
+ gdb_assert (inferior_thread ()->control.step_resume_breakpoint == NULL);
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: inserting longjmp-resume breakpoint at %s\n",
paddress (gdbarch, pc));
- inferior_thread ()->step_resume_breakpoint =
+ inferior_thread ()->control.step_resume_breakpoint =
set_momentary_breakpoint_at_pc (gdbarch, pc, bp_longjmp_resume);
}
@@ -6212,6 +6216,8 @@ save_infcall_control_state (void)
inf_status->thread_control = tp->control;
inf_status->inferior_control = inf->control;
+ tp->control.step_resume_breakpoint = NULL;
+
/* Save original bpstat chain to INF_STATUS; replace it in TP with copy of
chain. If caller's caller is walking the chain, they'll be happier if we
hand them back the original chain when restore_infcall_control_state is
@@ -6257,6 +6263,9 @@ restore_infcall_control_state (struct infcall_control_state *inf_status)
struct thread_info *tp = inferior_thread ();
struct inferior *inf = current_inferior ();
+ if (tp->control.step_resume_breakpoint)
+ tp->control.step_resume_breakpoint->disposition = disp_del_at_next_stop;
+
/* Handle the bpstat_copy of the chain. */
bpstat_clear (&tp->control.stop_bpstat);
@@ -6301,8 +6310,13 @@ make_cleanup_restore_infcall_control_state
void
discard_infcall_control_state (struct infcall_control_state *inf_status)
{
+ if (inf_status->thread_control.step_resume_breakpoint)
+ inf_status->thread_control.step_resume_breakpoint->disposition
+ = disp_del_at_next_stop;
+
/* See save_infcall_control_state for info on stop_bpstat. */
bpstat_clear (&inf_status->thread_control.stop_bpstat);
+
xfree (inf_status);
}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-resume-infcall.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2005, 2007, 2008, 2009, 2010 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>
+
+volatile int cond_hit;
+
+int
+cond (void)
+{
+ cond_hit++;
+
+ return 1;
+}
+
+int
+func (void)
+{
+ return 0; /* in-func */
+}
+
+int
+main (void)
+{
+ /* Variable is used so that there is always at least one instruction on the
+ same line after FUNC returns. */
+ int i = func (); /* call-func */
+
+ /* Dummy calls. */
+ cond ();
+ func ();
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-resume-infcall.exp
@@ -0,0 +1,52 @@
+# Copyright (C) 2010 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/>.
+
+set testfile "step-resume-infcall"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_test "step" " in-func .*"
+gdb_test "up" " call-func .*"
+gdb_test_no_output {set $b=$pc}
+
+if ![runto_main] {
+ return -1
+}
+
+gdb_breakpoint {*$b if cond ()}
+
+set test {print $bpnum}
+gdb_test_multiple $test $test {
+ -re " = (\[0-9\]+)\r\n$gdb_prompt $" {
+ set caller_bp $expect_out(1,string)
+ pass $test
+ }
+}
+
+# Debug only:
+gdb_test "disass/m" ".*"
+gdb_test "info breakpoints" ".*"
+
+gdb_test "next" "Breakpoint $caller_bp, .* call-func .*"
+
+# `cond-hit' is now hit twice; but it may not be in the future. It is
+# currently not a bug so it is not KFAILed.
+gdb_test "p cond_hit" { = [12]}
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -83,10 +83,10 @@ inferior_thread (void)
void
delete_step_resume_breakpoint (struct thread_info *tp)
{
- if (tp && tp->step_resume_breakpoint)
+ if (tp && tp->control.step_resume_breakpoint)
{
- delete_breakpoint (tp->step_resume_breakpoint);
- tp->step_resume_breakpoint = NULL;
+ delete_breakpoint (tp->control.step_resume_breakpoint);
+ tp->control.step_resume_breakpoint = NULL;
}
}
@@ -97,10 +97,10 @@ clear_thread_inferior_resources (struct thread_info *tp)
but not any user-specified thread-specific breakpoints. We can not
delete the breakpoint straight-off, because the inferior might not
be stopped at the moment. */
- if (tp->step_resume_breakpoint)
+ if (tp->control.step_resume_breakpoint)
{
- tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
- tp->step_resume_breakpoint = NULL;
+ tp->control.step_resume_breakpoint->disposition = disp_del_at_next_stop;
+ tp->control.step_resume_breakpoint = NULL;
}
bpstat_clear (&tp->control.stop_bpstat);