[PATCH] preventing resuming of threads in gdbserver

[PATCH] preventing resuming of threads in gdbserver

[Amit Kale]

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

Hi,

gdb lets other threads continue execution during single stepping when doing a 
single step in remote mode. This behavior causes thread switches during step 
or next commands. Native mode behavior is opposite of it. Attached patch 
changes it and makes it similar to native mode.

-Amit

[-- Attachment #2: gdb-noresumeallthreads.patch --]
[-- Type: text/x-diff, Size: 784 bytes --]

Index: gdb-cvs/gdb/remote.c
===================================================================
--- gdb-cvs.orig/gdb/remote.c	2006-01-03 11:24:35.000000000 +0530
+++ gdb-cvs/gdb/remote.c	2006-01-10 18:00:05.000000000 +0530
@@ -2519,10 +2519,10 @@
     {
       /* Resume all threads, with preference for INFERIOR_PTID.  */
       if (step && siggnal != TARGET_SIGNAL_0)
-	outbuf = xstrprintf ("vCont;S%02x:%x;c", siggnal,
+	outbuf = xstrprintf ("vCont;S%02x:%x", siggnal,
 			     PIDGET (inferior_ptid));
       else if (step)
-	outbuf = xstrprintf ("vCont;s:%x;c", PIDGET (inferior_ptid));
+	outbuf = xstrprintf ("vCont;s:%x", PIDGET (inferior_ptid));
       else if (siggnal != TARGET_SIGNAL_0)
 	outbuf = xstrprintf ("vCont;C%02x:%x;c", siggnal,
 			     PIDGET (inferior_ptid));

Re: [PATCH] preventing resuming of threads in gdbserver

[Michael Snyder]

Amit Kale wrote:
> Hi,
> 
> gdb lets other threads continue execution during single stepping when doing a 
> single step in remote mode. This behavior causes thread switches during step 
> or next commands. Native mode behavior is opposite of it. Attached patch 
> changes it and makes it similar to native mode.

Actually, letting other threads continue during single stepping
is the norm.  It happens on almost all multi-thread gdb targets.

Indeed, if you don't allow it to happen, you're risking deadlock,
and certainly changing the program behavior.

Moreover, a patch that changes the behavior of *all* remote targets
is going to be challenging to get approved.



On the other hand, there is a user-setable mode variable
called "scheduler-locking", which is meant to have the exact
effect you are looking for.  If you wanted to re-do your patch
so that it made this change conditionally, under the control of
that variable, it might be more acceptable.


> ------------------------------------------------------------------------
> 
> Index: gdb-cvs/gdb/remote.c
> ===================================================================
> --- gdb-cvs.orig/gdb/remote.c	2006-01-03 11:24:35.000000000 +0530
> +++ gdb-cvs/gdb/remote.c	2006-01-10 18:00:05.000000000 +0530
> @@ -2519,10 +2519,10 @@
>      {
>        /* Resume all threads, with preference for INFERIOR_PTID.  */
>        if (step && siggnal != TARGET_SIGNAL_0)
> -	outbuf = xstrprintf ("vCont;S%02x:%x;c", siggnal,
> +	outbuf = xstrprintf ("vCont;S%02x:%x", siggnal,
>  			     PIDGET (inferior_ptid));
>        else if (step)
> -	outbuf = xstrprintf ("vCont;s:%x;c", PIDGET (inferior_ptid));
> +	outbuf = xstrprintf ("vCont;s:%x", PIDGET (inferior_ptid));
>        else if (siggnal != TARGET_SIGNAL_0)
>  	outbuf = xstrprintf ("vCont;C%02x:%x;c", siggnal,
>  			     PIDGET (inferior_ptid));

Re: [PATCH] preventing resuming of threads in gdbserver

[Daniel Jacobowitz]

On Tue, Jan 10, 2006 at 01:19:19PM -0800, Michael Snyder wrote:
> Amit Kale wrote:
> >Hi,
> >
> >gdb lets other threads continue execution during single stepping when 
> >doing a single step in remote mode. This behavior causes thread switches 
> >during step or next commands. Native mode behavior is opposite of it. 
> >Attached patch changes it and makes it similar to native mode.
> 
> Actually, letting other threads continue during single stepping
> is the norm.  It happens on almost all multi-thread gdb targets.

Michael's right.  It sometimes does not obviously manifest on native
targets, depending on the implementation of the native system's
scheduler.

> Indeed, if you don't allow it to happen, you're risking deadlock,
> and certainly changing the program behavior.
> 
> Moreover, a patch that changes the behavior of *all* remote targets
> is going to be challenging to get approved.
> 
> 
> 
> On the other hand, there is a user-setable mode variable
> called "scheduler-locking", which is meant to have the exact
> effect you are looking for.  If you wanted to re-do your patch
> so that it made this change conditionally, under the control of
> that variable, it might be more acceptable.

Take a look at the implementation of scheduler-locking; this should
already work for gdbserver - and there's a testcase for it!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] preventing resuming of threads in gdbserver

[Amit Kale]

On Wednesday 11 Jan 2006 3:09 am, Daniel Jacobowitz wrote:
> On Tue, Jan 10, 2006 at 01:19:19PM -0800, Michael Snyder wrote:
> > Amit Kale wrote:
> > >Hi,
> > >
> > >gdb lets other threads continue execution during single stepping when
> > >doing a single step in remote mode. This behavior causes thread switches
> > >during step or next commands. Native mode behavior is opposite of it.
> > >Attached patch changes it and makes it similar to native mode.
> >
> > Actually, letting other threads continue during single stepping
> > is the norm.  It happens on almost all multi-thread gdb targets.
>
> Michael's right.  It sometimes does not obviously manifest on native
> targets, depending on the implementation of the native system's
> scheduler.

Yes I am convinced that it's just my setup that never shows a thread switch 
during a single step. It might occur in some setup.

> > On the other hand, there is a user-setable mode variable
> > called "scheduler-locking", which is meant to have the exact
> > effect you are looking for.  If you wanted to re-do your patch
> > so that it made this change conditionally, under the control of
> > that variable, it might be more acceptable.
>
> Take a look at the implementation of scheduler-locking; this should
> already work for gdbserver - and there's a testcase for it!

Right. schduler-locking works now.

Thanks.
-Amit

Re: [PATCH] preventing resuming of threads in gdbserver

[Amit Kale]

On Wednesday 11 Jan 2006 2:49 am, Michael Snyder wrote:
> Amit Kale wrote:
> > Hi,
> >
> > gdb lets other threads continue execution during single stepping when
> > doing a single step in remote mode. This behavior causes thread switches
> > during step or next commands. Native mode behavior is opposite of it.
> > Attached patch changes it and makes it similar to native mode.
>
> Actually, letting other threads continue during single stepping
> is the norm.  It happens on almost all multi-thread gdb targets.
> Indeed, if you don't allow it to happen, you're risking deadlock,
> and certainly changing the program behavior.

Thanks.
I wasn't aware that this was by design. Native gdb doesn't seem to switch 
threads during a "next", which made me think that it's the other way around.

Digging further, following code in resume() does handle scheduler_mode 
correctly.

      if ((scheduler_mode == schedlock_on)
	  || (scheduler_mode == schedlock_step
	      && (step || singlestep_breakpoints_inserted_p)))
	{
	  /* User-settable 'scheduler' mode requires solo thread resume. */
	  resume_ptid = inferior_ptid;
	}

-Amit

>
> Moreover, a patch that changes the behavior of *all* remote targets
> is going to be challenging to get approved.
>
>
>
> On the other hand, there is a user-setable mode variable
> called "scheduler-locking", which is meant to have the exact
> effect you are looking for.  If you wanted to re-do your patch
> so that it made this change conditionally, under the control of
> that variable, it might be more acceptable.
>
> > ------------------------------------------------------------------------
> >
> > Index: gdb-cvs/gdb/remote.c
> > ===================================================================
> > --- gdb-cvs.orig/gdb/remote.c	2006-01-03 11:24:35.000000000 +0530
> > +++ gdb-cvs/gdb/remote.c	2006-01-10 18:00:05.000000000 +0530
> > @@ -2519,10 +2519,10 @@
> >      {
> >        /* Resume all threads, with preference for INFERIOR_PTID.  */
> >        if (step && siggnal != TARGET_SIGNAL_0)
> > -	outbuf = xstrprintf ("vCont;S%02x:%x;c", siggnal,
> > +	outbuf = xstrprintf ("vCont;S%02x:%x", siggnal,
> >  			     PIDGET (inferior_ptid));
> >        else if (step)
> > -	outbuf = xstrprintf ("vCont;s:%x;c", PIDGET (inferior_ptid));
> > +	outbuf = xstrprintf ("vCont;s:%x", PIDGET (inferior_ptid));
> >        else if (siggnal != TARGET_SIGNAL_0)
> >  	outbuf = xstrprintf ("vCont;C%02x:%x;c", siggnal,
> >  			     PIDGET (inferior_ptid));