[RFA] Make continuations per-thread.

[RFA] Make continuations per-thread.

[Vladimir Prus]

Right now, continuations are global. This means that if we're doing
'finish' in one thread, then we cannot do 'finish' or anything that
also uses continuations, in any other thread. This seems unnecessary
limitation, and this patch makes continuations per-thread.

Further into non-stop series, it really allows me to do 'finish' or 'step'
in several threads independently.

OK?

- Volodya

---
 gdb/gdbthread.h |   15 +++++++++++++--
 gdb/infrun.c    |    8 ++++++--
 gdb/thread.c    |   16 ++++++++++++++--
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 6cfb1f9..56cbd51 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -64,6 +64,11 @@ struct thread_info
      when we finally do stop stepping.  */
   bpstat stepping_through_solib_catchpoints;
 
+  struct continuation *continuations;
+  struct continuation *intermediate_continations;
+
+  int proceed_to_finish;
+
   /* Private data used by the target vector implementation.  */
   struct private_thread_info *private;
 };
@@ -130,7 +135,10 @@ extern void save_infrun_state (ptid_t ptid,
 			       int       stepping_through_solib_after_catch,
 			       bpstat    stepping_through_solib_catchpoints,
 			       int       current_line,
-			       struct symtab *current_symtab);
+			       struct symtab *current_symtab,
+			       struct continuation *continuations,
+			       struct continuation *intermediate_continations,
+			       int proceed_to_finish);
 
 /* infrun context switch: load the debugger state previously saved
    for the given thread.  */
@@ -146,7 +154,10 @@ extern void load_infrun_state (ptid_t ptid,
 			       int       *stepping_through_solib_affter_catch,
 			       bpstat    *stepping_through_solib_catchpoints,
 			       int       *current_line,
-			       struct symtab **current_symtab);
+			       struct symtab **current_symtab,
+			       struct continuation **continuations,
+			       struct continuation **intermediate_continations,
+			       int *proceed_to_finish);
 
 /* Switch from one thread to another.  */
 extern void switch_to_thread (ptid_t ptid);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d68b7a5..d6e2c62 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1518,7 +1518,9 @@ context_switch (struct execution_control_state *ecs)
 			 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->current_line, ecs->current_symtab,
+			 cmd_continuation, intermediate_continuation,
+			 proceed_to_finish);
 
       /* Load infrun state for the new thread.  */
       load_infrun_state (ecs->ptid, &prev_pc,
@@ -1528,7 +1530,9 @@ context_switch (struct execution_control_state *ecs)
 			 &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->current_line, &ecs->current_symtab,
+			 &cmd_continuation, &intermediate_continuation,
+			 &proceed_to_finish);
     }
 
   switch_to_thread (ecs->ptid);
diff --git a/gdb/thread.c b/gdb/thread.c
index 431d88f..bba32e8 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -333,7 +333,10 @@ load_infrun_state (ptid_t ptid,
 		   int *stepping_through_solib_after_catch,
 		   bpstat *stepping_through_solib_catchpoints,
 		   int *current_line,
-		   struct symtab **current_symtab)
+		   struct symtab **current_symtab,
+		   struct continuation **continuations,
+		   struct continuation **intermediate_continations,
+		   int *proceed_to_finish)
 {
   struct thread_info *tp;
 
@@ -357,6 +360,9 @@ load_infrun_state (ptid_t ptid,
     tp->stepping_through_solib_catchpoints;
   *current_line = tp->current_line;
   *current_symtab = tp->current_symtab;
+  *continuations = tp->continuations;
+  *intermediate_continations = tp->intermediate_continations;
+  *proceed_to_finish = tp->proceed_to_finish;
 }
 
 /* Save infrun state for the thread PID.  */
@@ -374,7 +380,10 @@ save_infrun_state (ptid_t ptid,
 		   int stepping_through_solib_after_catch,
 		   bpstat stepping_through_solib_catchpoints,
 		   int current_line,
-		   struct symtab *current_symtab)
+		   struct symtab *current_symtab,
+		   struct continuation *continuations,
+		   struct continuation *intermediate_continations,
+		   int proceed_to_finish)
 {
   struct thread_info *tp;
 
@@ -396,6 +405,9 @@ save_infrun_state (ptid_t ptid,
   tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
   tp->current_line = current_line;
   tp->current_symtab = current_symtab;
+  tp->continuations = continuations;
+  tp->intermediate_continations = intermediate_continations;
+  tp->proceed_to_finish = proceed_to_finish;
 }
 
 /* Return true if TP is an active thread. */
-- 
1.5.3.5

Re: [RFA] Make continuations per-thread.

[Daniel Jacobowitz]

On Thu, Apr 24, 2008 at 07:45:38PM +0300, Vladimir Prus wrote:
> 
> Right now, continuations are global. This means that if we're doing
> 'finish' in one thread, then we cannot do 'finish' or anything that
> also uses continuations, in any other thread. This seems unnecessary
> limitation, and this patch makes continuations per-thread.
> 
> Further into non-stop series, it really allows me to do 'finish' or 'step'
> in several threads independently.
> 
> OK?

Could you explain why this is safe?  We will do continuations for the
thread which reports an event only.  So it seems like continuations
for other threads will linger in their struct thread.

For example, if we finish from one thread and hit a breakpoint in
another thread before the finish returns.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Make continuations per-thread.

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

A Friday 02 May 2008 04:00:12, Daniel Jacobowitz wrote:
> On Thu, Apr 24, 2008 at 07:45:38PM +0300, Vladimir Prus wrote:
> > Right now, continuations are global. This means that if we're doing
> > 'finish' in one thread, then we cannot do 'finish' or anything that
> > also uses continuations, in any other thread. This seems unnecessary
> > limitation, and this patch makes continuations per-thread.
> >
> > Further into non-stop series, it really allows me to do 'finish' or
> > 'step' in several threads independently.
> >
> > OK?
>
> Could you explain why this is safe?  We will do continuations for the
> thread which reports an event only.  So it seems like continuations
> for other threads will linger in their struct thread.
>
> For example, if we finish from one thread and hit a breakpoint in
> another thread before the finish returns.

That's true.  Attached is what we have next on the non-stop
series to fix that.  I'm not thrilled by it, but there are intermediate
context switches in handle_inferior_event that make it much uglier to try
to not make it centralized.  This is one of the things that gets much
better looking when we switch completelly to always-a-thread, and
get rid of context-switching.  I'm introducing another variable, instead of
using previous_inferior_ptid, which would be a good candidate, but I have
other plans for it.

-- 
Pedro Alves

[-- Attachment #2: switch_context_for_intermediate_continuations.patch --]
[-- Type: text/x-diff, Size: 3689 bytes --]

2008-05-02  Pedro Alves  <pedro@codesourcery.com>

	* inferior.h (step_ptid): Declare variable.
	* infcmd.c (step_1): Set step_ptid.
	* infrun.c (step_ptid): New variable.
	(fetch_inferior_event): Run the intermediate continuations in the
	right context, and clear step_ptid.

---
 gdb/infcmd.c   |    1 +
 gdb/inferior.h |    4 ++++
 gdb/infrun.c   |   36 +++++++++++++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletion(-)

Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h	2008-05-02 12:32:10.000000000 +0100
+++ src/gdb/inferior.h	2008-05-02 12:32:11.000000000 +0100
@@ -371,6 +371,10 @@ enum stop_kind
 
 extern enum stop_kind stop_soon;
 
+/* The thread we are executing a `step n'-like operation on.  Used in
+   all-stop mode to run the intermediate-continuations.  */
+extern ptid_t step_ptid;
+
 /* Nonzero if proceed is being used for a "finish" command or a similar
    situation when stop_registers should be saved.  */
 
Index: src/gdb/infcmd.c
===================================================================
--- src.orig/gdb/infcmd.c	2008-05-02 12:32:10.000000000 +0100
+++ src/gdb/infcmd.c	2008-05-02 12:32:11.000000000 +0100
@@ -789,6 +789,7 @@ which has no line number information.\n"
      and handle them one at the time, through step_once(). */
   else
     {
+      step_ptid = inferior_ptid;
       step_once (skip_subroutines, single_inst, count);
       /* We are running, and the contination is installed.  It will
 	 disable the longjmp breakpoint as appropriate.  */
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-05-02 12:32:10.000000000 +0100
+++ src/gdb/infrun.c	2008-05-02 12:32:11.000000000 +0100
@@ -74,6 +74,8 @@ static void set_schedlock_func (char *ar
 
 struct execution_control_state;
 
+static void context_switch (struct execution_control_state *ecs);
+
 static int currently_stepping (struct execution_control_state *ecs);
 
 static void xdb_handle_command (char *args, int from_tty);
@@ -447,6 +449,10 @@ follow_exec (int pid, char *execd_pathna
    until after the `wait' in `wait_for_inferior'.  */
 static int singlestep_breakpoints_inserted_p = 0;
 
+/* The thread we are executing a `step n'-like operation on.  Used in
+   all-stop mode to run the intermediate-continuations.  */
+ptid_t step_ptid;
+
 /* The thread we inserted single-step breakpoints for.  */
 static ptid_t singlestep_ptid;
 
@@ -1521,11 +1527,39 @@ fetch_inferior_event (void *client_data)
     {
       delete_step_resume_breakpoint (&step_resume_breakpoint);
 
+      /* In all-stop mode, if an event in another thread stops a
+	 requested `step n'-like operation, we must still run the
+	 intermediate continuations.  Since continuations are per
+	 thread, we must context switch back to the stepping thread to
+	 run them.  */
+      if (!ptid_equal (step_ptid, null_ptid)
+	  && !ptid_equal (step_ptid, inferior_ptid)
+	  && in_thread_list (step_ptid)
+	  && in_thread_list (inferior_ptid))
+	{
+	  volatile struct gdb_exception e;
+	  ptid_t current_ptid = inferior_ptid;
+
+	  async_ecs->ptid = step_ptid;
+	  context_switch (async_ecs);
+
+	  TRY_CATCH (e, RETURN_MASK_ERROR)
+	    {
+	      do_all_intermediate_continuations (0);
+	    }
+
+	  async_ecs->ptid = current_ptid;
+	  context_switch (async_ecs);
+	}
+
       normal_stop ();
       if (step_multi && stop_step)
 	inferior_event_handler (INF_EXEC_CONTINUE, NULL);
       else
-	inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+	{
+	  inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+	  step_ptid = null_ptid;
+	}
     }
 }
 

Re: [RFA] Make continuations per-thread.

[Pedro Alves]

A Friday 02 May 2008 12:34:11, Pedro Alves wrote:
> A Friday 02 May 2008 04:00:12, Daniel Jacobowitz wrote:
> > On Thu, Apr 24, 2008 at 07:45:38PM +0300, Vladimir Prus wrote:
> > > Right now, continuations are global. This means that if we're doing
> > > 'finish' in one thread, then we cannot do 'finish' or anything that
> > > also uses continuations, in any other thread. This seems unnecessary
> > > limitation, and this patch makes continuations per-thread.
> > >
> > > Further into non-stop series, it really allows me to do 'finish' or
> > > 'step' in several threads independently.
> > >
> > > OK?
> >
> > Could you explain why this is safe?  We will do continuations for the
> > thread which reports an event only.  So it seems like continuations
> > for other threads will linger in their struct thread.
> >
> > For example, if we finish from one thread and hit a breakpoint in
> > another thread before the finish returns.
>
> That's true.  Attached is what we have next on the non-stop
> series to fix that.  I'm not thrilled by it, but there are intermediate
> context switches in handle_inferior_event that make it much uglier to try
> to not make it centralized.  This is one of the things that gets much
> better looking when we switch completelly to always-a-thread, and
> get rid of context-switching.  I'm introducing another variable, instead of
> using previous_inferior_ptid, which would be a good candidate, but I have
> other plans for it.

Err, it doesn't fix all issues you mentioned.  I'll have to do a more than
this.

-- 
Pedro Alves

Re: [RFA] Make continuations per-thread.

[Vladimir Prus]

On Friday 02 May 2008 15:34:11 Pedro Alves wrote:
> A Friday 02 May 2008 04:00:12, Daniel Jacobowitz wrote:
> > On Thu, Apr 24, 2008 at 07:45:38PM +0300, Vladimir Prus wrote:
> > > Right now, continuations are global. This means that if we're doing
> > > 'finish' in one thread, then we cannot do 'finish' or anything that
> > > also uses continuations, in any other thread. This seems unnecessary
> > > limitation, and this patch makes continuations per-thread.
> > >
> > > Further into non-stop series, it really allows me to do 'finish' or
> > > 'step' in several threads independently.
> > >
> > > OK?
> >
> > Could you explain why this is safe?  We will do continuations for the
> > thread which reports an event only.  So it seems like continuations
> > for other threads will linger in their struct thread.
> >
> > For example, if we finish from one thread and hit a breakpoint in
> > another thread before the finish returns.
> 
> That's true.  Attached is what we have next on the non-stop
> series to fix that.  I'm not thrilled by it, but there are intermediate
> context switches in handle_inferior_event that make it much uglier to try
> to not make it centralized.  This is one of the things that gets much
> better looking when we switch completelly to always-a-thread, and
> get rid of context-switching.  I'm introducing another variable, instead of
> using previous_inferior_ptid, which would be a good candidate, but I have
> other plans for it.

This is only for intermediate continations. For ordinary continuations, not
running them when we hit a breakpoint in another thread is desirable. Why should
a breakpoint in some other thread abort "finish"? Note that in current gdb,
hitting a breakpoint in unrelated thread does not abort "next" -- say we
did next, inserted step resume breakpoint, and then hit breakpoint in some other
thread. Then, the step resume breakpoint will not be removed. If we decide to
continue the program, we'll eventually hit it. 

I don't see any problem with continuations been kept for a given thread
for a long time. It's not an unbounded amount of continuations -- if we get an
event in this thread, continuation will run, and if we don't get an event,
we won't add any futher continuations.

- Volodya

Re: [RFA] Make continuations per-thread.

[Daniel Jacobowitz]

On Fri, May 02, 2008 at 12:34:11PM +0100, Pedro Alves wrote:
> to not make it centralized.  This is one of the things that gets much
> better looking when we switch completelly to always-a-thread, and
> get rid of context-switching.  I'm introducing another variable, instead of

So maybe we should do that in the FSF tree before the attached patch -
is that feasible?

On Fri, May 02, 2008 at 03:51:10PM +0400, Vladimir Prus wrote:
> This is only for intermediate continations. For ordinary continuations, not
> running them when we hit a breakpoint in another thread is desirable. Why should
> a breakpoint in some other thread abort "finish"? Note that in current gdb,
> hitting a breakpoint in unrelated thread does not abort "next" -- say we
> did next, inserted step resume breakpoint, and then hit breakpoint in some other
> thread. Then, the step resume breakpoint will not be removed. If we decide to
> continue the program, we'll eventually hit it. 
> 
> I don't see any problem with continuations been kept for a given thread
> for a long time. It's not an unbounded amount of continuations -- if we get an
> event in this thread, continuation will run, and if we don't get an event,
> we won't add any futher continuations.

In non-stop mode, the continuation will run the first time that thread
stops because threads only stop when there is an event.  But in
all-stop mode the thread will be stopped with its continuations not
yet run.

[Current thread is 1]
finish
[Switching to thread 2]
Breakpoint at....
thread 1
finish

Now thread 1 has two finish continuations and they're at different
threads... is it going to do something sensible?  What's sensible?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Make continuations per-thread.

[Vladimir Prus]

On Friday 02 May 2008 17:23:37 Daniel Jacobowitz wrote:
> On Fri, May 02, 2008 at 12:34:11PM +0100, Pedro Alves wrote:
> > to not make it centralized.  This is one of the things that gets much
> > better looking when we switch completelly to always-a-thread, and
> > get rid of context-switching.  I'm introducing another variable, instead of
> 
> So maybe we should do that in the FSF tree before the attached patch -
> is that feasible?
> 
> On Fri, May 02, 2008 at 03:51:10PM +0400, Vladimir Prus wrote:
> > This is only for intermediate continations. For ordinary continuations, not
> > running them when we hit a breakpoint in another thread is desirable. Why should
> > a breakpoint in some other thread abort "finish"? Note that in current gdb,
> > hitting a breakpoint in unrelated thread does not abort "next" -- say we
> > did next, inserted step resume breakpoint, and then hit breakpoint in some other
> > thread. Then, the step resume breakpoint will not be removed. If we decide to
> > continue the program, we'll eventually hit it. 
> > 
> > I don't see any problem with continuations been kept for a given thread
> > for a long time. It's not an unbounded amount of continuations -- if we get an
> > event in this thread, continuation will run, and if we don't get an event,
> > we won't add any futher continuations.
> 
> In non-stop mode, the continuation will run the first time that thread
> stops because threads only stop when there is an event.  But in
> all-stop mode the thread will be stopped with its continuations not
> yet run.
> 
> [Current thread is 1]
> finish
> [Switching to thread 2]
> Breakpoint at....
> thread 1
> finish
> 
> Now thread 1 has two finish continuations and they're at different
> threads... is it going to do something sensible?  What's sensible?

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?

- Volodya

Re: [RFA] Make continuations per-thread.

[Daniel Jacobowitz]

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?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Make continuations per-thread.

[Ulrich Weigand]

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

Re: [RFA] Make continuations per-thread.

[Pedro Alves]

A Friday 02 May 2008 16:19:06, Ulrich Weigand wrote:
> 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:
>

Thanks for showing us that.

> - 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.
>

A agree that this behaviour is sensible in all-stop
(what we have now) mode.  I tried your patch, and the bahaviour
makes sense to me.

Note, however, that in non-stop mode, we need to retain the
stepping state per-thread (and actually make a
few more things per-thread), so we'd like to keep the variables
you're removing from struct thread_info in some form.

http://sourceware.org/ml/gdb-patches/2008-05/msg00232.html

> 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 ...
>

Our major goal is to have that in non-stop mode.  It seemed
we would win some of it for free in all-stop mode, but it
turned out it isn't so.

Thanks!

-- 
Pedro Alves

Re: [RFA] Make continuations per-thread.

[Pedro Alves]

A Friday 02 May 2008 14:30:32, Vladimir Prus escreveu:
> On Friday 02 May 2008 17:23:37 Daniel Jacobowitz wrote:
> > On Fri, May 02, 2008 at 12:34:11PM +0100, Pedro Alves wrote:
> > > to not make it centralized.  This is one of the things that gets much
> > > better looking when we switch completelly to always-a-thread, and
> > > get rid of context-switching.  I'm introducing another variable,
> > > instead of
> >
> > So maybe we should do that in the FSF tree before the attached patch -
> > is that feasible?
> >
> > On Fri, May 02, 2008 at 03:51:10PM +0400, Vladimir Prus wrote:
> > > This is only for intermediate continations. For ordinary continuations,
> > > not running them when we hit a breakpoint in another thread is
> > > desirable. Why should a breakpoint in some other thread abort "finish"?
> > > Note that in current gdb, hitting a breakpoint in unrelated thread does
> > > not abort "next" -- say we did next, inserted step resume breakpoint,
> > > and then hit breakpoint in some other thread. Then, the step resume
> > > breakpoint will not be removed. If we decide to continue the program,
> > > we'll eventually hit it.
> > >
> > > I don't see any problem with continuations been kept for a given thread
> > > for a long time. It's not an unbounded amount of continuations -- if we
> > > get an event in this thread, continuation will run, and if we don't get
> > > an event, we won't add any futher continuations.
> >
> > In non-stop mode, the continuation will run the first time that thread
> > stops because threads only stop when there is an event.  But in
> > all-stop mode the thread will be stopped with its continuations not
> > yet run.
> >
> > [Current thread is 1]
> > finish
> > [Switching to thread 2]
> > Breakpoint at....
> > thread 1
> > finish
> >
> > Now thread 1 has two finish continuations and they're at different
> > threads... is it going to do something sensible?  What's sensible?
>

Yes, this seems like a problem.  The second finish command installs
the continuations in the last even thread.  In non-stop, this is
fixed by making the thread command context_switch instead of just
switch_to_thread.  Maybe that should be done in all-stop too?
We'd context-switch to the last event thread when resuming, so
the right context is set to step over breakpoints etc.

Then the question would be:

 Now thread 1 has two finish continuations in the queue.  Shouldn't
 the previous one be canceled?

> 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?
>

Isn't that too late?  When you get to proceed, the new finish
continuation is already installed.

-- 
Pedro Alves

Re: [RFA] Make continuations per-thread.

[Vladimir Prus]

On Friday 02 May 2008 17:45:51 Pedro Alves wrote:
> A Friday 02 May 2008 14:30:32, Vladimir Prus escreveu:
> > On Friday 02 May 2008 17:23:37 Daniel Jacobowitz wrote:
> > > On Fri, May 02, 2008 at 12:34:11PM +0100, Pedro Alves wrote:
> > > > to not make it centralized.  This is one of the things that gets much
> > > > better looking when we switch completelly to always-a-thread, and
> > > > get rid of context-switching.  I'm introducing another variable,
> > > > instead of
> > >
> > > So maybe we should do that in the FSF tree before the attached patch -
> > > is that feasible?
> > >
> > > On Fri, May 02, 2008 at 03:51:10PM +0400, Vladimir Prus wrote:
> > > > This is only for intermediate continations. For ordinary continuations,
> > > > not running them when we hit a breakpoint in another thread is
> > > > desirable. Why should a breakpoint in some other thread abort "finish"?
> > > > Note that in current gdb, hitting a breakpoint in unrelated thread does
> > > > not abort "next" -- say we did next, inserted step resume breakpoint,
> > > > and then hit breakpoint in some other thread. Then, the step resume
> > > > breakpoint will not be removed. If we decide to continue the program,
> > > > we'll eventually hit it.
> > > >
> > > > I don't see any problem with continuations been kept for a given thread
> > > > for a long time. It's not an unbounded amount of continuations -- if we
> > > > get an event in this thread, continuation will run, and if we don't get
> > > > an event, we won't add any futher continuations.
> > >
> > > In non-stop mode, the continuation will run the first time that thread
> > > stops because threads only stop when there is an event.  But in
> > > all-stop mode the thread will be stopped with its continuations not
> > > yet run.
> > >
> > > [Current thread is 1]
> > > finish
> > > [Switching to thread 2]
> > > Breakpoint at....
> > > thread 1
> > > finish
> > >
> > > Now thread 1 has two finish continuations and they're at different
> > > threads... is it going to do something sensible?  What's sensible?
> >
> 
> Yes, this seems like a problem.  The second finish command installs
> the continuations in the last even thread.  In non-stop, this is
> fixed by making the thread command context_switch instead of just
> switch_to_thread.  Maybe that should be done in all-stop too?

This sounds like a reasonable idea.

> We'd context-switch to the last event thread when resuming, so
> the right context is set to step over breakpoints etc.
> 
> Then the question would be:
> 
>  Now thread 1 has two finish continuations in the queue.  Shouldn't
>  the previous one be canceled?
> 
> > 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?
> >
> 
> Isn't that too late?  When you get to proceed, the new finish
> continuation is already installed.

This is true. The clean solution, probably, is to add a check in each
command that eventually calls proceed. If the per-thread state of
the current thread records we're doing some operation, we should ask
the user if he wants to actually abort that operation. I think we
can actually find all the commands that proceed the targets, so this is
doable, but is a bit of work, unfortunately.

- Volodya