This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch 3/3] Fix stale tp->step_resume_breakpoint
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: gdb-patches at sourceware dot org
- Cc: Pedro Alves <pedro at codesourcery dot com>
- Date: Wed, 24 Nov 2010 01:50:34 +0100
- Subject: [patch 3/3] Fix stale tp->step_resume_breakpoint
Hi,
this is a follow-up to:
Re: [patch] Fix stale tp->step_resume_breakpoint
http://sourceware.org/ml/gdb-patches/2010-11/msg00011.html
On Tue, 02 Nov 2010 02:05:18 +0100, Pedro Alves wrote:
> I think this raises an obvious question, and hints at
> a larger issue: if you find you you need to tuck away step_resume_breakpoint,
> then, how come you don't need to do the same for all the other execution
> command state? (step_range_start, step_range_end, step_frame_id,
> continuations, etc.).
Some of them were already saved, just not into `struct inferior_thread_state'
but into `struct inferior_status'. So extended now further the latter.
I find these two redundant, they could be merged? I did not try to.
This could be a different patch.
> I'd assume that in the use case you trip on step_resume_breakpoint
> troubles, you'd also be losing thread stepping state (or state
> for any other execution command), thus your thread would end up
> running free, forgetting about the previous command that was
> going on before the infcall. Is that not the case?
Tried to fix the fields I thought should be saved. Some fields I am not sure.
For example `stop_signal' IMO should be saved+reset+restored but
gdb.base/call-signal-resume.exp tests it is persisent across infcalls so
I kept it that way.
Thanks,
Jan
gdb/
2010-11-24 Jan Kratochvil <jan.kratochvil@redhat.com>
* defs.h (discard_all_specified_continuations): New prototype.
* infrun.c (struct inferior_status) <step_resume_breakpoint)
(tp_continuations, intermediate_continuations, step_multi)
(inf_continuations): New fields.
(save_inferior_status): Save step_resume_breakpoint, tp_continuations,
intermediate_continuations, step_multi and inf_continuations.
(restore_inferior_status): Restore step_resume_breakpoint,
tp_continuations, intermediate_continuations, step_multi and
inf_continuations.
(discard_inferior_status): Discard step_resume_breakpoint,
tp_continuations, intermediate_continuations and inf_continuations.
* utils.c (discard_all_specified_continuations): New function.
(discard_all_continuations_thread_callback)
(discard_all_intermediate_continuations_thread_callback): Use it.
gdb/testsuite/
2010-11-24 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/step-resume-infcall.exp: New file.
* gdb.base/step-resume-infcall.c: New file.
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -734,6 +734,8 @@ extern void add_continuation (struct thread_info *,
void (*)(void *));
extern void do_all_continuations (void);
extern void do_all_continuations_thread (struct thread_info *);
+extern void
+ discard_all_specified_continuations (struct continuation **continuationsp);
extern void discard_all_continuations (void);
extern void discard_all_continuations_thread (struct thread_info *);
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6167,19 +6167,26 @@ get_inferior_thread_state_regcache (struct inferior_thread_state *inf_state)
struct inferior_status
{
/* Direct copies of the struct thread_info fields: */
+ struct breakpoint *step_resume_breakpoint;
CORE_ADDR step_range_start;
CORE_ADDR step_range_end;
struct frame_id step_frame_id;
struct frame_id step_stack_frame_id;
int trap_expected;
int proceed_to_finish;
+ struct continuation *tp_continuations;
+ struct continuation *intermediate_continuations;
int in_infcall;
enum step_over_calls_kind step_over_calls;
int stop_step;
+ int step_multi;
+ /* STOP_SIGNAL should not be saved/restored as tested by:
+ gdb.base/call-signal-resume.exp */
bpstat stop_bpstat;
/* Direct copies of the struct inferior fields: */
int stop_soon;
+ struct continuation *inf_continuations;
/* Other fields: */
enum stop_stack_kind stop_stack_dummy;
@@ -6201,15 +6208,23 @@ save_inferior_status (void)
struct inferior *inf = current_inferior ();
/* Direct copies of the struct thread_info fields: */
+ inf_status->step_resume_breakpoint = tp->step_resume_breakpoint;
+ tp->step_resume_breakpoint = NULL;
inf_status->step_range_start = tp->step_range_start;
inf_status->step_range_end = tp->step_range_end;
inf_status->step_frame_id = tp->step_frame_id;
inf_status->step_stack_frame_id = tp->step_stack_frame_id;
inf_status->trap_expected = tp->trap_expected;
inf_status->proceed_to_finish = tp->proceed_to_finish;
+ inf_status->tp_continuations = tp->continuations;
+ tp->continuations = NULL;
+ inf_status->intermediate_continuations = tp->intermediate_continuations;
+ tp->intermediate_continuations = NULL;
inf_status->in_infcall = tp->in_infcall;
inf_status->step_over_calls = tp->step_over_calls;
inf_status->stop_step = tp->stop_step;
+ inf_status->step_multi = tp->step_multi;
+ tp->step_multi = 0;
/* Save original bpstat chain here; replace it with copy of chain.
If caller's caller is walking the chain, they'll be happier if we
@@ -6220,6 +6235,8 @@ save_inferior_status (void)
/* Direct copies of the struct inferior fields: */
inf_status->stop_soon = inf->stop_soon;
+ inf_status->inf_continuations = inf->continuations;
+ inf->continuations = NULL;
/* Other fields: */
inf_status->stop_stack_dummy = stop_stack_dummy;
@@ -6261,15 +6278,23 @@ restore_inferior_status (struct inferior_status *inf_status)
struct inferior *inf = current_inferior ();
/* Direct copies of the struct thread_info fields: */
+ if (tp->step_resume_breakpoint)
+ tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+ tp->step_resume_breakpoint = inf_status->step_resume_breakpoint;
tp->step_range_start = inf_status->step_range_start;
tp->step_range_end = inf_status->step_range_end;
tp->step_frame_id = inf_status->step_frame_id;
tp->step_stack_frame_id = inf_status->step_stack_frame_id;
tp->trap_expected = inf_status->trap_expected;
tp->proceed_to_finish = inf_status->proceed_to_finish;
+ discard_all_continuations_thread (tp);
+ tp->continuations = inf_status->tp_continuations;
+ discard_all_intermediate_continuations_thread (tp);
+ tp->intermediate_continuations = inf_status->intermediate_continuations;
tp->in_infcall = inf_status->in_infcall;
tp->step_over_calls = inf_status->step_over_calls;
tp->stop_step = inf_status->stop_step;
+ tp->step_multi = inf_status->step_multi;
/* Handle the bpstat_copy of the chain. */
bpstat_clear (&tp->stop_bpstat);
@@ -6278,6 +6303,8 @@ restore_inferior_status (struct inferior_status *inf_status)
/* Direct copies of the struct inferior fields: */
inf->stop_soon = inf_status->stop_soon;
+ discard_all_inferior_continuations (inf);
+ inf->continuations = inf_status->inf_continuations;
/* Other fields: */
stop_stack_dummy = inf_status->stop_stack_dummy;
@@ -6316,8 +6343,19 @@ make_cleanup_restore_inferior_status (struct inferior_status *inf_status)
void
discard_inferior_status (struct inferior_status *inf_status)
{
+ /* Direct copies of the struct thread_info fields: */
+ if (inf_status->step_resume_breakpoint)
+ inf_status->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+
+ discard_all_specified_continuations (&inf_status->tp_continuations);
+ discard_all_specified_continuations (&inf_status->intermediate_continuations);
+
+ /* Direct copies of the struct inferior fields: */
+ discard_all_specified_continuations (&inf_status->inf_continuations);
+
/* See save_inferior_status for info on stop_bpstat. */
bpstat_clear (&inf_status->stop_bpstat);
+
xfree (inf_status);
}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-resume-infcall.c
@@ -0,0 +1,45 @@
+/* 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>
+
+int
+cond (void)
+{
+ puts ("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,49 @@
+# 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" ".*"
+
+# `cond-hit' is now printed twice; but it may not be in the future.
+gdb_test "next" "cond-hit.*\r\nBreakpoint $caller_bp, .* call-func .*"
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -840,15 +840,22 @@ do_all_continuations (void)
iterate_over_threads (do_all_continuations_thread_callback, NULL);
}
+/* Get rid of all the continuations tracked by *CONTINUATIONSP. */
+void
+discard_all_specified_continuations (struct continuation **continuationsp)
+{
+ struct cleanup *continuation_ptr = &(*continuationsp)->base;
+
+ discard_my_cleanups (&continuation_ptr, NULL);
+ *continuationsp = NULL;
+}
+
/* Callback for iterate over threads. */
static int
discard_all_continuations_thread_callback (struct thread_info *thread,
void *data)
{
- struct cleanup *continuation_ptr = &thread->continuations->base;
-
- discard_my_cleanups (&continuation_ptr, NULL);
- thread->continuations = NULL;
+ discard_all_specified_continuations (&thread->continuations);
return 0;
}
@@ -922,10 +929,7 @@ static int
discard_all_intermediate_continuations_thread_callback (struct thread_info *thread,
void *data)
{
- struct cleanup *continuation_ptr = &thread->intermediate_continuations->base;
-
- discard_my_cleanups (&continuation_ptr, NULL);
- thread->intermediate_continuations = NULL;
+ discard_all_specified_continuations (&thread->intermediate_continuations);
return 0;
}