Multithreaded debugging: strange thread switches

Multithreaded debugging: strange thread switches

[Vladimir Prus]

Hello,
I'm observing strange behaviour when debugging with gdb using a custom stub.
I have two threads. After connecting, I say "next" several times and that 
steps in thread 1. Then I say "thread 2" and "next". Gdb then stops again in 
thread 1, not in thread 2 as I'd expected.

In the remote protocol I see "Hc1" packet after last "next" though I'd expect 
"Hc2", and in infrun.c, function prepare_to_proceed, I see this:


 if (!ptid_equal (wait_ptid, minus_one_ptid)
      && !ptid_equal (inferior_ptid, wait_ptid))
    {
      /* Switched over from WAIT_PID.  */
      CORE_ADDR wait_pc = read_pc_pid (wait_ptid);

      if (wait_pc != read_pc ())
	{
	  /* Switch back to WAIT_PID thread.  */
	  inferior_ptid = wait_ptid;

	  /* FIXME: This stuff came from switch_to_thread() in
	     thread.c (which should probably be a public function).  */
	  flush_cached_frames ();
	  registers_changed ();
	  stop_pc = wait_pc;
	  select_frame (get_current_frame ());
	}

Can somebody explain the reason for this explicit switch back to "wait_ptid"?

Thanks,
Volodya

Re: Multithreaded debugging: strange thread switches

[Daniel Jacobowitz]

On Mon, Jan 23, 2006 at 06:20:32PM +0300, Vladimir Prus wrote:
> 
> Hello,
> I'm observing strange behaviour when debugging with gdb using a custom stub.
> I have two threads. After connecting, I say "next" several times and that 
> steps in thread 1. Then I say "thread 2" and "next". Gdb then stops again in 
> thread 1, not in thread 2 as I'd expected.
> 
> In the remote protocol I see "Hc1" packet after last "next" though I'd expect 

This, generally, is part of the problem.  If you want this to work, you
need to implement the vCont packet in the stub.  The Hc1 packet is
supposed to mean "step only thread 1, leaving thread 2 stopped", and
that's not what the gdb "next" command is supposed to map to - that's
"step this thread but leave other threads free-running".

> "Hc2", and in infrun.c, function prepare_to_proceed, I see this:
> 
> 
>  if (!ptid_equal (wait_ptid, minus_one_ptid)
>       && !ptid_equal (inferior_ptid, wait_ptid))
>     {
>       /* Switched over from WAIT_PID.  */
>       CORE_ADDR wait_pc = read_pc_pid (wait_ptid);
> 
>       if (wait_pc != read_pc ())
> 	{
> 	  /* Switch back to WAIT_PID thread.  */
> 	  inferior_ptid = wait_ptid;
> 
> 	  /* FIXME: This stuff came from switch_to_thread() in
> 	     thread.c (which should probably be a public function).  */
> 	  flush_cached_frames ();
> 	  registers_changed ();
> 	  stop_pc = wait_pc;
> 	  select_frame (get_current_frame ());
> 	}
> 
> Can somebody explain the reason for this explicit switch back to "wait_ptid"?

I have no idea, but if you remove it you're going to bust the bit below
that steps over breakpoints, and then GDB will really get unhappy. 
Maybe we can adjust that block of code, though.  See the comment in
proceed():

  /* In a multi-threaded task we may select another thread
     and then continue or step.

     But if the old thread was stopped at a breakpoint, it
     will immediately cause another breakpoint stop without
     any execution (i.e. it will report a breakpoint hit
     incorrectly).  So we must step over it first.

     prepare_to_proceed checks the current thread against the thread
     that reported the most recent event.  If a step-over is required
     it returns TRUE and sets the current thread to the old thread. */

The current code was modified to always select the previous thread
in arch-utils.c revision 1.28 in order to support resuming after
hitting Control-C:
  http://sourceware.org/ml/gdb-patches/2001-05/msg00419.html

I can't tell from Jonathan's post what problem he's trying to fix.
Previously the thread would only be switched if (SIGTRAP && breakpoint
here).  Now it's switched if (SIGTRAP || SIGINT) and then we
single-step if (breakpoint here).  Which doesn't make much sense.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Multithreaded debugging: strange thread switches

[Vladimir Prus]

On Monday 23 January 2006 18:52, Daniel Jacobowitz wrote:

> > I'm observing strange behaviour when debugging with gdb using a custom
> > stub. I have two threads. After connecting, I say "next" several times
> > and that steps in thread 1. Then I say "thread 2" and "next". Gdb then
> > stops again in thread 1, not in thread 2 as I'd expected.
> >
> > In the remote protocol I see "Hc1" packet after last "next" though I'd
> > expect
>
> This, generally, is part of the problem.  If you want this to work, you
> need to implement the vCont packet in the stub.  The Hc1 packet is
> supposed to mean "step only thread 1, leaving thread 2 stopped", and
> that's not what the gdb "next" command is supposed to map to - that's
> "step this thread but leave other threads free-running".

Well, strictly speaking I want "next" to mean "move this thread to next source 
line, then single-step all other threads until there are at the same time as 
the current thread". (The remote side is sumulator, so "same time" is 
well-defined).

I do not think that vCont can exactly express what I want, so there's always 
be some difference between what gdb says over remote connection and what I 
want to be done -- are you sure using vCont will be any better?

> > "Hc2", and in infrun.c, function prepare_to_proceed, I see this:
> >
> >
> >  if (!ptid_equal (wait_ptid, minus_one_ptid)
> >       && !ptid_equal (inferior_ptid, wait_ptid))
> >     {
> >       /* Switched over from WAIT_PID.  */
> >       CORE_ADDR wait_pc = read_pc_pid (wait_ptid);
> >
> >       if (wait_pc != read_pc ())
> > 	{
> > 	  /* Switch back to WAIT_PID thread.  */
> > 	  inferior_ptid = wait_ptid;
> >
> > 	  /* FIXME: This stuff came from switch_to_thread() in
> > 	     thread.c (which should probably be a public function).  */
> > 	  flush_cached_frames ();
> > 	  registers_changed ();
> > 	  stop_pc = wait_pc;
> > 	  select_frame (get_current_frame ());
> > 	}
> >
> > Can somebody explain the reason for this explicit switch back to
> > "wait_ptid"?
>
> I have no idea, but if you remove it you're going to bust the bit below
> that steps over breakpoints, and then GDB will really get unhappy.
> Maybe we can adjust that block of code, though.  See the comment in
> proceed():
>
>   /* In a multi-threaded task we may select another thread
>      and then continue or step.
>
>      But if the old thread was stopped at a breakpoint, it
>      will immediately cause another breakpoint stop without
>      any execution (i.e. it will report a breakpoint hit
>      incorrectly).  So we must step over it first.
>
>      prepare_to_proceed checks the current thread against the thread
>      that reported the most recent event.  If a step-over is required
>      it returns TRUE and sets the current thread to the old thread. */

I see. But after we've single-stepped over breakpoint, will we switch back to 
the thread where "next" was issued? 

> The current code was modified to always select the previous thread
> in arch-utils.c revision 1.28 in order to support resuming after
> hitting Control-C:
>   http://sourceware.org/ml/gdb-patches/2001-05/msg00419.html
>
> I can't tell from Jonathan's post what problem he's trying to fix.

Hmmm, maybe that's a sign that Changelog requirements are suboptimal? Most 
often, Changelog entries contain lots of details about
changes to specific functions, but no general overview of the goal of the 
patch. 

> Previously the thread would only be switched if (SIGTRAP && breakpoint
> here).  Now it's switched if (SIGTRAP || SIGINT) and then we
> single-step if (breakpoint here).  Which doesn't make much sense.

I've tried to revert that patch and rerun the testsuite, and got quite a bunch 
of extra errors. I'll look deeper later.

- Volodya

Re: Multithreaded debugging: strange thread switches

[Daniel Jacobowitz]

On Tue, Jan 24, 2006 at 05:22:47PM +0300, Vladimir Prus wrote:
> On Monday 23 January 2006 18:52, Daniel Jacobowitz wrote:
> 
> > > I'm observing strange behaviour when debugging with gdb using a custom
> > > stub. I have two threads. After connecting, I say "next" several times
> > > and that steps in thread 1. Then I say "thread 2" and "next". Gdb then
> > > stops again in thread 1, not in thread 2 as I'd expected.
> > >
> > > In the remote protocol I see "Hc1" packet after last "next" though I'd
> > > expect
> >
> > This, generally, is part of the problem.  If you want this to work, you
> > need to implement the vCont packet in the stub.  The Hc1 packet is
> > supposed to mean "step only thread 1, leaving thread 2 stopped", and
> > that's not what the gdb "next" command is supposed to map to - that's
> > "step this thread but leave other threads free-running".
> 
> Well, strictly speaking I want "next" to mean "move this thread to next source 
> line, then single-step all other threads until there are at the same time as 
> the current thread". (The remote side is sumulator, so "same time" is 
> well-defined).
> 
> I do not think that vCont can exactly express what I want, so there's always 
> be some difference between what gdb says over remote connection and what I 
> want to be done -- are you sure using vCont will be any better?

Well, GDB doesn't have any command that it expects to have that
behavior.  If you want GDB to do that, I recommend adding such an
interface.  From what I can see, you can get the same effect by "single
step all threads in sync, repeatedly, until this thread reaches the
next source line", which is "vCont;s", or possibly "Hc-1" "s" - not
completely sure about the last one, the documentation suggests that's
right, but I don't know any stub that handles either of these
correctly.

> I see. But after we've single-stepped over breakpoint, will we switch back to 
> the thread where "next" was issued? 

At the moment, no.  This is definitely a bug but it's a pretty nasty
infrun limitation; really that whole subsystem needs some love and
attention.

> > The current code was modified to always select the previous thread
> > in arch-utils.c revision 1.28 in order to support resuming after
> > hitting Control-C:
> >   http://sourceware.org/ml/gdb-patches/2001-05/msg00419.html
> >
> > I can't tell from Jonathan's post what problem he's trying to fix.
> 
> Hmmm, maybe that's a sign that Changelog requirements are suboptimal? Most 
> often, Changelog entries contain lots of details about
> changes to specific functions, but no general overview of the goal of the 
> patch. 

No, the mailing list archives are where this sort of thing is supposed
to live - and the comments in the code.  See the GNU coding standards
and their description of what goes in a changelog entry.  Sometimes
very strange, but that's what we've got to work with.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Multithreaded debugging: strange thread switches

[Vladimir Prus]

On Tuesday 24 January 2006 17:28, Daniel Jacobowitz wrote:

> > Well, strictly speaking I want "next" to mean "move this thread to next
> > source line, then single-step all other threads until there are at the
> > same time as the current thread". (The remote side is sumulator, so "same
> > time" is well-defined).
> >
> > I do not think that vCont can exactly express what I want, so there's
> > always be some difference between what gdb says over remote connection
> > and what I want to be done -- are you sure using vCont will be any
> > better?
>
> Well, GDB doesn't have any command that it expects to have that
> behavior.  If you want GDB to do that, I recommend adding such an
> interface.  From what I can see, you can get the same effect by "single
> step all threads in sync, repeatedly, until this thread reaches the
> next source line", which is "vCont;s", 

Generally speaking if 2 threads execute 10 steps each, they no longer are at 
the same time, because instructions in thread 1 can take one cycle each, 
while instructions in thread can take two cycles each. I'd prefer this 
synchronization logic to be inside my remote part, which is specifialized for 
that task.

> or possibly "Hc-1" "s" - not 
> completely sure about the last one, the documentation suggests that's
> right, but I don't know any stub that handles either of these
> correctly.
>
> > I see. But after we've single-stepped over breakpoint, will we switch
> > back to the thread where "next" was issued?
>
> At the moment, no.  This is definitely a bug but it's a pretty nasty
> infrun limitation; really that whole subsystem needs some love and
> attention.

Is that bug filed in the bug tracker? Should I file it?

> > > The current code was modified to always select the previous thread
> > > in arch-utils.c revision 1.28 in order to support resuming after
> > > hitting Control-C:
> > >   http://sourceware.org/ml/gdb-patches/2001-05/msg00419.html
> > >
> > > I can't tell from Jonathan's post what problem he's trying to fix.
> >
> > Hmmm, maybe that's a sign that Changelog requirements are suboptimal?
> > Most often, Changelog entries contain lots of details about
> > changes to specific functions, but no general overview of the goal of the
> > patch.
>
> No, the mailing list archives are where this sort of thing is supposed
> to live 

But if I'm looking at Changelog entry, how can I guess the subject of email 
that discusses this change?

> - and the comments in the code.   

But if change is across 10 files, a comment in one of them is not likely to 
give the entire pictire.

- Volodya

Re: Multithreaded debugging: strange thread switches

[Daniel Jacobowitz]

On Tue, Jan 24, 2006 at 05:41:44PM +0300, Vladimir Prus wrote:
> > Well, GDB doesn't have any command that it expects to have that
> > behavior.  If you want GDB to do that, I recommend adding such an
> > interface.  From what I can see, you can get the same effect by "single
> > step all threads in sync, repeatedly, until this thread reaches the
> > next source line", which is "vCont;s", 
> 
> Generally speaking if 2 threads execute 10 steps each, they no longer are at 
> the same time, because instructions in thread 1 can take one cycle each, 
> while instructions in thread can take two cycles each. I'd prefer this 
> synchronization logic to be inside my remote part, which is specifialized for 
> that task.

I think that adds weight to my argument that the existing GDB commands
don't correspond to this action :-)

There's a documented (but never implemented, as far as I know) 'i'
packet to step a single cycle.  And the vCont mechanism is extensible
for new continue operations (deliberately).  So you could make GDB do
exactly what you want in at least two ways:

1. vCont;TID:s until at the next line
   query time
   vCont;TID:0;i until times match (where 0 means "stopped" and "i"
   means "one cycle")

2. vCont;i until at the next line

I'd recommend #2 unless you have some reason not to do that.

> > or possibly "Hc-1" "s" - not 
> > completely sure about the last one, the documentation suggests that's
> > right, but I don't know any stub that handles either of these
> > correctly.
> >
> > > I see. But after we've single-stepped over breakpoint, will we switch
> > > back to the thread where "next" was issued?
> >
> > At the moment, no.  This is definitely a bug but it's a pretty nasty
> > infrun limitation; really that whole subsystem needs some love and
> > attention.
> 
> Is that bug filed in the bug tracker? Should I file it?

I don't know; you might as well.  I think the case described above
might actually work, but there are definitely some gotchas in this
area.

> But if I'm looking at Changelog entry, how can I guess the subject of email 
> that discusses this change?

You have the date; in practice, it never takes me more than a couple of
minutes.

Please, take further discussion of the ChangeLog convention to a more
appropriate forum - it's the GNU standard.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Multithreaded debugging: strange thread switches

[Eli Zaretskii]

> From: Vladimir Prus <ghost@cs.msu.su>
> Date: Tue, 24 Jan 2006 17:41:44 +0300
> 
> > > Hmmm, maybe that's a sign that Changelog requirements are suboptimal?
> > > Most often, Changelog entries contain lots of details about
> > > changes to specific functions, but no general overview of the goal of the
> > > patch.
> >
> > No, the mailing list archives are where this sort of thing is supposed
> > to live 
> 
> But if I'm looking at Changelog entry, how can I guess the subject of email 
> that discusses this change?

You can't.  The way to look is to search for relevant keywords and for
symbols from the sources in question.

> > - and the comments in the code.   
> 
> But if change is across 10 files, a comment in one of them is not likely to 
> give the entire pictire.

The entire picture should be either in one of the source files, the
one that has the bulk of the related code, or in gdbint.texinfo.

I know that in practice much of such info is just nowhere, but
ChangeLog is no place for it, either.  The way to make sure the big
picture is not lost is to make a point of explaining it in one of the
two places I mentioned when you submit a patch.  I always ask people
to add information to gdbint.texinfo, but of course much is still left
unexplained.

vCont [was Re: Multithreaded debugging: strange thread switches]

[Nathan J. Williams]

Daniel Jacobowitz <drow@false.org> writes:

> This, generally, is part of the problem.  If you want this to work, you
> need to implement the vCont packet in the stub.  The Hc1 packet is
> supposed to mean "step only thread 1, leaving thread 2 stopped", and
> that's not what the gdb "next" command is supposed to map to - that's
> "step this thread but leave other threads free-running".

Tangentially... I'm working on a stub for a system that doesn't have
hardware single-step; GDB knows that and doesn't try to issue any step
commands. However, the logic in remote.c that analyzes the response to
the vCont? packet refuses to use vCont unless it supports all of s, S,
c, and C. Is the best thing for my stub just to lie about supporting s
and S, and to rely on the knowledge that GDB won't try to use them?

        - Nathan

Re: vCont [was Re: Multithreaded debugging: strange thread switches]

[Daniel Jacobowitz]

On Tue, Jan 24, 2006 at 06:34:38PM -0500, Nathan J. Williams wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
> > This, generally, is part of the problem.  If you want this to work, you
> > need to implement the vCont packet in the stub.  The Hc1 packet is
> > supposed to mean "step only thread 1, leaving thread 2 stopped", and
> > that's not what the gdb "next" command is supposed to map to - that's
> > "step this thread but leave other threads free-running".
> 
> Tangentially... I'm working on a stub for a system that doesn't have
> hardware single-step; GDB knows that and doesn't try to issue any step
> commands. However, the logic in remote.c that analyzes the response to
> the vCont? packet refuses to use vCont unless it supports all of s, S,
> c, and C. Is the best thing for my stub just to lie about supporting s
> and S, and to rely on the knowledge that GDB won't try to use them?

Yeah, I've been meaning to fix that.  A better thing would be to modify
GDB to accept a vCont packet that only indicated continue, and then
issue an error if remote_resume tries to step.

-- 
Daniel Jacobowitz
CodeSourcery