This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Make continuations per-thread.
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: drow at false dot org (Daniel Jacobowitz)
- Cc: vladimir at codesourcery dot com (Vladimir Prus), pedro at codesourcery dot com (Pedro Alves), gdb-patches at sourceware dot org
- Date: Fri, 2 May 2008 17:19:06 +0200 (CEST)
- Subject: Re: [RFA] Make continuations per-thread.
Daniel Jacobowitz wrote:
> On Fri, May 02, 2008 at 05:30:32PM +0400, Vladimir Prus wrote:
> > I think the sensible behaviour is the same as for "next" -- abort
> > whatever the operation we were doing. This means that we have to wipe
> > continuation inside 'proceed'. I can adjust the patch this way, but
> > does it make sense to you?
>
> It makes sense, but I'm wondering how much work it would be to do
> better than that (for all-stop, I mean - clearly you're planning to
> do better for non-stop). In the example in my last message, there's
> a point where thread 1 is stopped in the middle of a finish. Info
> threads could show "(finishing)". Should step / next clear that,
> or should we be able to step a bit and then print the return value
> when we get there?
Note that current GDB is really broken when it comes to handling
step / next operations that are interrupted by an event in some
other thread. There is some code to "context switch" the stepping
status, but this appears to be incomplete, and leads to inconsistent
results; e.g. GDB can run into internal assertion failures like
"Thread Event Breakpoint: gdb should not stop!" (see PR 2250).
I'm currently carrying the following patch in our GDB builds that
fixes the issue be ensuring:
- If the step operation is interrupted by an *internal* breakpoint
that is handled transparently, the operation continues in a
transparent and consistent manner after the breakpoint was handled.
- If the step operation is interrupted by an *explicit* breakpoint
that breaks to a user prompt, it is completely cancelled. It is
then up to the user how to continue from the prompt.
Now it looks like the second point is in conflict to the goal you
are proposing here (namely, that even after a user prompt, the
interrupted step/next/finish still continues in some manner).
I'm not sure how this could be accomplished ...
In any case, see below for the patch I was referring to.
(In addition, I'm also using a patch that to some extent reduces
potential user confusion by always preferring to report an event
in the "current" thread the user is working on over events in some
other threads, if those events happen "simultaneously". This will
cause e.g. single-stepping to basically always remain in the
current thread.)
Bye,
Ulrich
diff -urNp gdb-orig/gdb/gdbthread.h gdb-head/gdb/gdbthread.h
--- src.orig/gdb/gdbthread.h 2008-04-01 16:36:33.000000000 +0200
+++ src/gdb/gdbthread.h 2008-04-01 16:29:54.000000000 +0200
@@ -43,12 +43,6 @@ struct thread_info
int num; /* Convenient handle (GDB thread id) */
/* State from wait_for_inferior */
CORE_ADDR prev_pc;
- struct breakpoint *step_resume_breakpoint;
- CORE_ADDR step_range_start;
- CORE_ADDR step_range_end;
- struct frame_id step_frame_id;
- int current_line;
- struct symtab *current_symtab;
int trap_expected;
int handling_longjmp;
int stepping_over_breakpoint;
@@ -119,32 +113,20 @@ extern struct thread_info *iterate_over_
extern void save_infrun_state (ptid_t ptid,
CORE_ADDR prev_pc,
int trap_expected,
- struct breakpoint *step_resume_breakpoint,
- CORE_ADDR step_range_start,
- CORE_ADDR step_range_end,
- const struct frame_id *step_frame_id,
int handling_longjmp,
int another_trap,
int stepping_through_solib_after_catch,
- bpstat stepping_through_solib_catchpoints,
- int current_line,
- struct symtab *current_symtab);
+ bpstat stepping_through_solib_catchpoints);
/* infrun context switch: load the debugger state previously saved
for the given thread. */
extern void load_infrun_state (ptid_t ptid,
CORE_ADDR *prev_pc,
int *trap_expected,
- struct breakpoint **step_resume_breakpoint,
- CORE_ADDR *step_range_start,
- CORE_ADDR *step_range_end,
- struct frame_id *step_frame_id,
int *handling_longjmp,
int *another_trap,
int *stepping_through_solib_affter_catch,
- bpstat *stepping_through_solib_catchpoints,
- int *current_line,
- struct symtab **current_symtab);
+ bpstat *stepping_through_solib_catchpoints);
/* Switch from one thread to another. */
extern void switch_to_thread (ptid_t ptid);
diff -urNp gdb-orig/gdb/infrun.c gdb-head/gdb/infrun.c
--- src.orig/gdb/infrun.c 2008-04-01 16:36:33.000000000 +0200
+++ src/gdb/infrun.c 2008-04-01 21:14:12.923081149 +0200
@@ -207,6 +207,12 @@ static struct cmd_list_element *stop_com
static struct symbol *step_start_function;
+/* Current thread, line, and symtab while stepping. */
+
+static ptid_t step_ptid;
+static int step_current_line;
+static struct symtab *step_current_symtab;
+
/* Nonzero if we are presently stepping over a breakpoint.
If we hit a breakpoint or watchpoint, and then continue,
@@ -341,6 +347,10 @@ follow_inferior_reset_breakpoints (void)
if (step_resume_breakpoint)
breakpoint_re_set_thread (step_resume_breakpoint);
+ /* We also need to reset step_ptid for the same reason. */
+
+ step_ptid = inferior_ptid;
+
/* Reinsert all breakpoints in the child. The user may have set
breakpoints after catching the fork, in which case those
were never set in the child, but only in the parent. This makes
@@ -385,6 +395,9 @@ follow_exec (int pid, char *execd_pathna
step_resume_breakpoint = NULL;
step_range_start = 0;
step_range_end = 0;
+ step_current_line = 0;
+ step_current_symtab = NULL;
+ step_ptid = null_ptid;
/* What is this a.out's name? */
printf_unfiltered (_("Executing new program: %s\n"), execd_pathname);
@@ -747,7 +760,13 @@ proceed (CORE_ADDR addr, enum target_sig
int oneproc = 0;
if (step > 0)
- step_start_function = find_pc_function (pc);
+ {
+ struct symtab_and_line sal = find_pc_line (pc, 0);
+ step_current_line = sal.line;
+ step_current_symtab = sal.symtab;
+ step_ptid = inferior_ptid;
+ step_start_function = find_pc_function (pc);
+ }
if (step < 0)
stop_after_trap = 1;
@@ -949,8 +968,6 @@ struct execution_control_state
CORE_ADDR stop_func_end;
char *stop_func_name;
struct symtab_and_line sal;
- int current_line;
- struct symtab *current_symtab;
int handling_longjmp; /* FIXME */
ptid_t ptid;
ptid_t saved_inferior_ptid;
@@ -1125,9 +1142,6 @@ init_execution_control_state (struct exe
ecs->handling_longjmp = 0; /* FIXME */
ecs->stepping_through_solib_after_catch = 0;
ecs->stepping_through_solib_catchpoints = NULL;
- ecs->sal = find_pc_line (prev_pc, 0);
- ecs->current_line = ecs->sal.line;
- ecs->current_symtab = ecs->sal.symtab;
ecs->infwait_state = infwait_normal_state;
ecs->waiton_ptid = pid_to_ptid (-1);
ecs->wp = &(ecs->ws);
@@ -1173,24 +1187,16 @@ context_switch (struct execution_control
if (in_thread_list (inferior_ptid) && in_thread_list (ecs->ptid))
{ /* Perform infrun state context switch: */
/* Save infrun state for the old thread. */
- save_infrun_state (inferior_ptid, prev_pc,
- stepping_over_breakpoint, step_resume_breakpoint,
- step_range_start,
- step_range_end, &step_frame_id,
+ save_infrun_state (inferior_ptid, prev_pc, stepping_over_breakpoint,
ecs->handling_longjmp, ecs->stepping_over_breakpoint,
ecs->stepping_through_solib_after_catch,
- ecs->stepping_through_solib_catchpoints,
- ecs->current_line, ecs->current_symtab);
+ ecs->stepping_through_solib_catchpoints);
/* Load infrun state for the new thread. */
- load_infrun_state (ecs->ptid, &prev_pc,
- &stepping_over_breakpoint, &step_resume_breakpoint,
- &step_range_start,
- &step_range_end, &step_frame_id,
+ load_infrun_state (ecs->ptid, &prev_pc, &stepping_over_breakpoint,
&ecs->handling_longjmp, &ecs->stepping_over_breakpoint,
&ecs->stepping_through_solib_after_catch,
- &ecs->stepping_through_solib_catchpoints,
- &ecs->current_line, &ecs->current_symtab);
+ &ecs->stepping_through_solib_catchpoints);
}
switch_to_thread (ecs->ptid);
@@ -1998,7 +2004,8 @@ handle_inferior_event (struct execution_
ecs->random_signal
= !(bpstat_explains_signal (stop_bpstat)
|| stepping_over_breakpoint
- || (step_range_end && step_resume_breakpoint == NULL));
+ || (ptid_equal (step_ptid, ecs->ptid)
+ && step_range_end && step_resume_breakpoint == NULL));
else
{
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
@@ -2048,7 +2055,8 @@ process_event_stop_test:
if (signal_program[stop_signal] == 0)
stop_signal = TARGET_SIGNAL_0;
- if (prev_pc == read_pc ()
+ if (ptid_equal (step_ptid, ecs->ptid)
+ && prev_pc == read_pc ()
&& breakpoint_here_p (read_pc ())
&& !breakpoint_inserted_here_p (read_pc ())
&& step_resume_breakpoint == NULL)
@@ -2070,7 +2078,8 @@ process_event_stop_test:
return;
}
- if (step_range_end != 0
+ if (ptid_equal (step_ptid, ecs->ptid)
+ && step_range_end != 0
&& stop_signal != TARGET_SIGNAL_0
&& stop_pc >= step_range_start && stop_pc < step_range_end
&& frame_id_eq (get_frame_id (get_current_frame ()),
@@ -2355,24 +2364,46 @@ process_event_stop_test:
return;
}
- if (step_resume_breakpoint)
+
+ /* At this point, if we are not stepping, just continue. */
+ if (step_range_end == 0)
{
if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: step-resume breakpoint is inserted\n");
+ fprintf_unfiltered (gdb_stdlog, "infrun: no stepping, continue\n");
+ keep_going (ecs);
+ return;
+ }
- /* Having a step-resume breakpoint overrides anything
- else having to do with stepping commands until
- that breakpoint is reached. */
+ /* If we're currently stepping, but have stopped in some other thread,
+ we need to switch back to the stepped thread. However, if the current
+ thread is blocked on some internal breakpoint, and we simply need to
+ step over that breakpoint to get it going again, do that first. */
+ if (!ptid_equal (step_ptid, ecs->ptid))
+ {
+ if (ecs->stepping_over_breakpoint)
+ {
+ keep_going (ecs);
+ return;
+ }
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun: switching back to stepped thread\n");
+
+ ecs->ptid = step_ptid;
+ context_switch (ecs);
keep_going (ecs);
return;
}
- if (step_range_end == 0)
+ if (step_resume_breakpoint)
{
if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: no stepping, continue\n");
- /* Likewise if we aren't even stepping. */
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: step-resume breakpoint is inserted\n");
+
+ /* Having a step-resume breakpoint overrides anything
+ else having to do with stepping commands until
+ that breakpoint is reached. */
keep_going (ecs);
return;
}
@@ -2652,8 +2683,8 @@ process_event_stop_test:
}
if ((stop_pc == ecs->sal.pc)
- && (ecs->current_line != ecs->sal.line
- || ecs->current_symtab != ecs->sal.symtab))
+ && (step_current_line != ecs->sal.line
+ || step_current_symtab != ecs->sal.symtab))
{
/* We are at the start of a different line. So stop. Note that
we don't stop if we step into the middle of a different line.
@@ -2677,8 +2708,8 @@ process_event_stop_test:
step_range_start = ecs->sal.pc;
step_range_end = ecs->sal.end;
step_frame_id = get_frame_id (get_current_frame ());
- ecs->current_line = ecs->sal.line;
- ecs->current_symtab = ecs->sal.symtab;
+ step_current_line = ecs->sal.line;
+ step_current_symtab = ecs->sal.symtab;
/* In the case where we just stepped out of a function into the
middle of a line of the caller, continue stepping, but
diff -urNp gdb-orig/gdb/thread.c gdb-head/gdb/thread.c
--- src.orig/gdb/thread.c 2008-04-01 16:36:33.000000000 +0200
+++ src/gdb/thread.c 2008-04-01 16:29:54.000000000 +0200
@@ -66,15 +66,10 @@ void
delete_step_resume_breakpoint (void *arg)
{
struct breakpoint **breakpointp = (struct breakpoint **) arg;
- struct thread_info *tp;
if (*breakpointp != NULL)
{
delete_breakpoint (*breakpointp);
- for (tp = thread_list; tp; tp = tp->next)
- if (tp->step_resume_breakpoint == *breakpointp)
- tp->step_resume_breakpoint = NULL;
-
*breakpointp = NULL;
}
}
@@ -82,13 +77,6 @@ delete_step_resume_breakpoint (void *arg
static void
free_thread (struct thread_info *tp)
{
- /* NOTE: this will take care of any left-over step_resume breakpoints,
- 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)
- tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
-
/* FIXME: do I ever need to call the back-end to give it a
chance at this private data before deleting the thread? */
if (tp->private)
@@ -312,16 +300,10 @@ void
load_infrun_state (ptid_t ptid,
CORE_ADDR *prev_pc,
int *trap_expected,
- struct breakpoint **step_resume_breakpoint,
- CORE_ADDR *step_range_start,
- CORE_ADDR *step_range_end,
- struct frame_id *step_frame_id,
int *handling_longjmp,
int *stepping_over_breakpoint,
int *stepping_through_solib_after_catch,
- bpstat *stepping_through_solib_catchpoints,
- int *current_line,
- struct symtab **current_symtab)
+ bpstat *stepping_through_solib_catchpoints)
{
struct thread_info *tp;
@@ -333,18 +315,12 @@ load_infrun_state (ptid_t ptid,
*prev_pc = tp->prev_pc;
*trap_expected = tp->trap_expected;
- *step_resume_breakpoint = tp->step_resume_breakpoint;
- *step_range_start = tp->step_range_start;
- *step_range_end = tp->step_range_end;
- *step_frame_id = tp->step_frame_id;
*handling_longjmp = tp->handling_longjmp;
*stepping_over_breakpoint = tp->stepping_over_breakpoint;
*stepping_through_solib_after_catch =
tp->stepping_through_solib_after_catch;
*stepping_through_solib_catchpoints =
tp->stepping_through_solib_catchpoints;
- *current_line = tp->current_line;
- *current_symtab = tp->current_symtab;
}
/* Save infrun state for the thread PID. */
@@ -353,16 +329,10 @@ void
save_infrun_state (ptid_t ptid,
CORE_ADDR prev_pc,
int trap_expected,
- struct breakpoint *step_resume_breakpoint,
- CORE_ADDR step_range_start,
- CORE_ADDR step_range_end,
- const struct frame_id *step_frame_id,
int handling_longjmp,
int stepping_over_breakpoint,
int stepping_through_solib_after_catch,
- bpstat stepping_through_solib_catchpoints,
- int current_line,
- struct symtab *current_symtab)
+ bpstat stepping_through_solib_catchpoints)
{
struct thread_info *tp;
@@ -374,16 +344,10 @@ save_infrun_state (ptid_t ptid,
tp->prev_pc = prev_pc;
tp->trap_expected = trap_expected;
- tp->step_resume_breakpoint = step_resume_breakpoint;
- tp->step_range_start = step_range_start;
- tp->step_range_end = step_range_end;
- tp->step_frame_id = (*step_frame_id);
tp->handling_longjmp = handling_longjmp;
tp->stepping_over_breakpoint = stepping_over_breakpoint;
tp->stepping_through_solib_after_catch = stepping_through_solib_after_catch;
tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
- tp->current_line = current_line;
- tp->current_symtab = current_symtab;
}
/* Return true if TP is an active thread. */
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com