Re: multi-process remote protocol extensions

[Pedro Alves <pedro at codesourcery dot com>]

Hi Daniel,

Thanks for taking the time to reply.

A Tuesday 03 June 2008 02:27:36, Daniel Jacobowitz wrote:

> One big picture question.  Have you thought about how to communicate
> shared resources from the stub to the host?  e.g. "(all processes|no
> processes|some specific processes) share memory", or "hardware
> breakpoints affect (all processes|all threads in a process|one
> thread|selectable behavior)"?
>
> "Some processes share memory" is interesting for multi-core systems,
> where two cores might be attached to a single RAM (or share a
> software-enforced common MMU configuration), while another core lives
> in its own little world.  Should sound familiar - Cell is a good
> example.
>

Good question.  I haven't planed anything for that, simply
because I had no need for it.  I'm working with a target stub
that has breakpoints support, and there's good chance
gdbserver will gain them too in a not so distant future.

> GDB needs to know whether memory writes affect all processes to
> correctly insert/remove memory breakpoints.  

True.  GDB has to know that so multiple breakpoint shadows
can be kept.  A breakpoint that is set at all attached
processes, where each process has its own code space, looks
similar in principle to breakpoints with multiple locations,
where each location is bound to a process, with its
own shadow.  If the breakpoint target is shared memory, we
can only insert one breakpoint location.

> Though a better solution 
> to this may be requiring the stub to support software breakpoints via
> Z0.  It would be nice to optimize out thread switching when reading
> memory from different threads of the same process.

Yeah.  I don't think we need to require Z0 at the protocol level,
but instead, leave it open.  If someone wants to implement
that knowledge on the GDB side (well, we'll probably get to do
it), it should be possible.

> >  GDB currently identifies threads using 'ptid_t' values.  A ptid_t is
> >  a three-member structure containing a process ID, lightweight process
> >  ID (for the kernel-level thread within the process), and a thread ID
> >  (for the thread library's identifier).  This means that GDB is
> >  already carrying the information needed to find the process to which
> >  a given thread belongs to.
>
> You've extended pid to pid.tid.  Do we need pid.lwp.tid in the
> protocol, for symmetry with ptid_t?  I assume the answer is "no"; just
> making sure the question was considered.  It should be easier to
> extend next time if we need this later.
>

I assumed that stubs that need some form of lwp.tid pair currently
to specify a task, do a stub-tid <-> lwp.tid mapping.

Perhaps we should tag ids?

T.ID.T.ID.T.ID

c.core.p.pid.l.lwp.t.tid
c.1.p.999.l.0.t.123

Tags:
 p - process
 l - lwp
 t - thread
 c - core
 w - whatever

p.999.t.123

That's just 4 characters more.

Or:

pPID.lLWP.TID
oOTHERNAMESPACE.pPID.lLWP.TID

 p999.123
 p999.l1.123

Can only use letters > f then, but that shouldn't be a problem?

I don't think using a letter is ambiguous in any case expecting a
thread id currently.

> >   The multi-process extension allows specifying a (pid.)?tid as n.
> >
> >   E.g.:
> >    `T 05 thread:12.2'
>
> At this point are we talking about an all-stop debug server?  In which
> case we do not need any additional information about what threads have
> stopped.
>

Yes, all-stop.  I don't think you're saying a change is not
needed here.  But for avoidance of doubt, the thread here must
also specify the pid (that's `12.' in the example), otherwise
GDB can't tell which process thread 2 belongs to.

> >  The W and X reply packets have a different problem, they don't
> >  specify any thread or pid currently, so they can't be as easily
> >  extended.  These are the current descriptions:
> >
> >   `W AA'
> >     The process exited, and AA is the exit status. This is only
> >     applicable to certain targets.
> >
> >   `X AA'
> >     The process terminated with signal AA.
> >
> >  In this case, I believe it is better to add a new stop reply packet:
> >
> >   `vExited;why:id(:value)?
> >
> >      The why's currently defined would be the same as the W and X stop
> >      replies, with the extension that the value field isn't limited to
> >      two nibbles.
> >
> >      `vExited;W:pid:exit_code'
> >
> >        W - process pid exited normally with exit status stored in
> >        `exit_code'
> >
> >      `vExited;W:pid:signal'
> >
> >        X - process pid exited signalled, exit signal recorded in
> >        `signal'
>
> The stub can not send vExited unless it has seen the
> qSupported:multiprocess+ from GDB; otherwise GDB won't know what to do
> with it.  So I don't see a strong argument here for a new response.
>
> `W AA [; process:PID]' ?
>

I'm all for it.  I had proposed this first internally, but had
feared adding arguments to existing packets would be unaccepted,
If we can, then I'd prefer it too.  It's very easy for a stub
to not send it if the GDB side doesn't support it.

> There's some question in my mind about thread exits - should they be
> handled the same as process exits.  They're not critical data, so for
> all-stop method I have been thinking we could use some async droppable
> messages to announce them 

Yeah, for non-stop we'll want thread creation too.

> (Jim's progress reporting work - you may 
> remember the discussion of this but it's not finished).

Hmm, I don't seem to remember this.  Was this a public discussion?

> But for non-stop mode we could report them as ordinary events in the same 
way
> as process exit.  So, a hypothetical:
>
> `W AA [; thread:THREAD.ID]'
>

Yes.

Not stricly multi-process related, but while we're at it, two
nibbles `AA' only is unnecessarilly limiting.  That was
the other reason for proposing new status packets.

> > ----------------------------------------------------------------------
> >
> > Part 5 - Interrupting/Stopping threads and processes.
> >
> > In both non-stop and multi-process, interrupting the remote with
> > ^C or BREAK is not sufficiently accurate.  We need to be able to
> > specify a process or thread to interrupt.
> >
> > What would be the preferred way to do this?  Assuming the (pid.)?tid
> > form is accepted, should we extend vCont with a new action, or
> > implement a new packet entirely?
>
> This seems like a problem.  We can't send a packet at this point
> without changing an assumption of the protocol; there's basically a
> "token", meaning only one side is allowed to be sending packets.  ^C
> is not a packet, for that reason.
>
> If we're in all-stop mode, wouldn't C-c mean "interrupt everything"?
> If so, this goes away.  In non-stop mode it'll be clear how to send a
> new packet.
>

Ok, I was thinking ahead, and mixed up.  Let's drop this for now.

> > Part 6 - Detaching and killing processes
> >
> > The detach, and kill packets, described as such in the manual,
> >
> >  `D' Detach GDB from the remote system.
> >
> >    Sent to the remote target before GDB disconnects via the
> >    detach command.
> >
> >  `k' Kill request.
> >
> >     FIXME: There is no description of how to operate when a specific
> >     thread context has been selected (i.e. does 'k' kill only that
> >     thread?).
> >
> >  Both suffer from the problem that they don't specify which
> >  process should be killed or detached.
> >
> >  What is the prefered form to fix this?
> >
> >  I see three options:
> >
> >    a) add new arguments to these packets, that are allowed by
> >       a multi-process aware stub.  If GDB is talking to a
> >       non multi-process aware stub, it doesn't send the extra
> >       arguments.
> >
> >    b) Do not add arguments, but instead specify that the process
> >       to detach from is specified with the extended Hg packet.
> >
> >    c) Specify new `vDetach pid`, and `vKill pid` packets.
> >
> >  Given the FIXME on the description of the `k' packet, I'm somewhat
> >  inclined to specify new packets.
>
> I agree that we should add a new packet to replace 'k'.  I'm not sure
> about 'D'; there's nothing else wrong with it, so I am not inclined to
> deprecate it.

So, do a) for D ?

 D;process:PID

>
> Will vKill have any meaning connected to a non-multiprocess stub?  If
> so we should clearly document it (e.g. CPU reset, single core reset,
> whatever).
>

No reset:

 vKill;PID - kills process PID, in an OS sense.  Get rid of
process PID.

But can be:
 vKill;process:PID - kills process PID.
 vKill;thread:ID - kills thread ID, where ID can be an extended
                   pid.tid (or whatever comes decided) form in
                   multi-process aware stubs.

I'm not sure we should put resets on the same boat as kills.
Killing a process or a thread should have a reply.  It isn't
specified currently if the packets requires a reply or not.
gdbserver doesn't send one, and GDB doesn't expect one.

A reset on the core the stub is running, may have the effect that
it's not always possible for a packet acknowledge to be flushed
on the stub side, and GDB never gets the answer.

Maybe that should be kept a separate packet, similar in vein
to R - restart - vReset/vRestart,

 vReset;core:ID - resets core ID (-1 all cores).  This packet
 has no reply.

> >   - If the stub replies an empty response, to signal that it doesn't
> >     support the packet, GDB retries without the `,tid' field, and
> >     makes sure to not send to the stub the new packet format again.
> >     Everything goes back to what is was.
> >
> >   - If the stub replies an error, to signal that it understood the
> >     packet, but saw a format error, GDB retries without the `,tid'
> >     field, and makes sure to not send to the stub the new packet
> >     format again.  Everything goes back to what is was.
>
> I do not think these are a good idea.  Empty packet means "this entire
> packet is not supported" and should continue to mean that.  And we can
> either require stubs which announce multi-process support to support
> this, or report a separate feature for it - I'm inclined to the latter
> since this is useful even for non-multiprocess stubs.
>

Ok.  The whole idea so it wasn't tied to multiprocess support, and
the notion that if per-thread breakpoints aren't support on the stub
side, the only fallback, is normal process-wide breakpoints, and have
GDB filter them, like it is done currently.

I agree that if we extend Z and announce in some form, then it should
be announced separatelly to multi-process support.

> >   In multi-process, tid would take the (pid.)?tid form, so we can
> >   specify to which process and thread the breakpoint applies to.
> >   pid.-1 to the whole process, pid.tid to a single thread, -1.-1 to
> >   mean all attached processes and threads, and empty to mean all
> >   processes, even the ones not attached.
>
> Is this really the appropriate meaning for an empty argument?  The
> current behavior, with no argument, is either pid.-1 or -1.-1.

Yeah, I take it back.  Making a Z packet with no argument mean all
current attached processes makes more sense.  I'll talk with Vlad about
this vBreak experience.

> >   If you think that extended the breakpoint packets is still not safe,
> >   then I propose a new similar packet:
> >
> >   vBreak type,addr,lenght,tid
> >
> >   ... the the same semantics I just described.
>
> I've no objection to extending the Z packets.  But I've no objection
> to vBreak either - be sure to talk to Vladimir about this, we've got a
> vBreak implementation lying around with a custom type, for a customer
> project last year.

-- 
Pedro Alves