This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFA [PATCH v4] Implement 'catch syscall' for gdbserver (was Re: RFA [PATCH v3] Implement 'catch syscall' for gdbserver)


On 10/04/2013 07:55 PM, Stan Shebs wrote:
> On 10/4/13 10:40 AM, Pedro Alves wrote:
> 
>> [...]  Unless perhaps we make GDB
>> send a unique id along as well... I think the RSP used to always send
>> a sequence number with each packet, and that has been removed a long
>> time ago.  I wish I know why it was removed.  It would solve these
>> issues.  Maybe we should add it back.
> 
> I researched this a bit, since I had the same vague memory, but the
> situation is odd.  In 1994, Jim Kingdon documented a $cc:<restofpacket>
> at the top of remote.c, as an option that stubs accept, and to this
> day the example stubs have their bit of code, but I don't see any
> evidence that anything was done in GDB before or since.  (The rationale
> may simply have been that the stubs needed to be extended first, this
> being before the days of qSupported.)

Indeed, in gdb 4.13:

/* Remote communication protocol.

   A debug packet whose contents are <data>
   is encapsulated for transmission in the form:

	$ <data> # CSUM1 CSUM2

	<data> must be ASCII alphanumeric and cannot include characters
	'$' or '#'.  If <data> starts with two characters followed by
	':', then the existing stubs interpret this as a sequence number.

	CSUM1 and CSUM2 are ascii hex representation of an 8-bit
	checksum of <data>, the most significant nibble is sent first.
	the hex digits 0-9,a-f are used.


> 
> Sequence numbers don't seem like a great idea, though, potentially
> introducing brittleness of various sorts.  

Hmm, not sure what brittleness would that be?  The RSP, with its
ack/nak and retransmission support, thinking in network protocol
terms, is really an application layer protocol that also tries to be
a transport layer protocol.  But then I'd say it's not really good
at either.  :-) The way the ack/nak/timeout process is designed,
if RSP is being pushed on top of an unreliable network protocol/link
(like serial/RS232, or udp), then it sometimes happens that GDB
and the server get out of sync.  A sequence number is just the
basic thing that reliable transport protocols (such as TCP) have
to sort out that issue, so that the receiver of an ack can tell
what is being acked.  Currently, if acks are lost, havoc
usually ensues.  The implemention in 1994 does look too naive though.

It just feels to me that the RSP should either have had ack/naks
and retransmissions + sequence numbers, or none of all that at all.

> Although we didn't really
> think about it explicitly at the outset, the remote protocol has
> generally evolved in the direction of being stateless; 

I think it's useful to consider the transport and application
layers independently here, even though the RSP really mixes/messes
thing up there.  If we think at the transport level, if GDB can
assume the transport is reliable, it'll disable its own transport
layer features -- that is, the ack/naks.  That is the QStartNoAckMode
stuff (which gdbserver makes use of, when the connection is through
tcp/ip, but not when it's serial).  This was something that was added
not that long ago.

Now, if we move on past that to the RSP commands/packets
themselves, we start thinking in terms of application layer.
At that conceptual level, both client and server should
assume that packets always get to the other side in sequence.
GDB should be able to be sure that the "OK" it just pulled
out of the  packet stream is the reply to the command it just
sent, not for the previous command that happened to fail.
Today, in scenarios around packet loss, it can't.

As a result of the lame ack/retransmission design, any
problem at the communication level needs to result in force closing
the connection.  If the user wants to keep debugging, she
should reconnect.  The disconnect might well delete useful state
from GDB that might be painful to re-setup.

> for instance,
> we don't send a half-dozen vCont packets that must all be applied
> together, we send one packet that includes everything.

True that we've been evolving in that direction, though we have
several packets that aren't like that.

For example qfThreadInfo/qmThreadInfo (get first thread; get next thread;
get next thread; etc.).  The server needs to know which thread to
return next.  Another example is downloading tracepoints.  We first
create the tracepoint (QTDP:...) , and then augment the tracepoint
with its actions, one by one, by sending (QTDP:-...).
Or even for more recent examples, how sometimes when we
fetch an xml off the target, it needs to first take a snapshot
of whatever it's going to return when GDB first reads offset 0,
and then subsequent reads are served from that cached snapshot.
E.g., gdbserver/server.c:handle_qxfer_threads.

I don't see a conceptual issue with preparing a request
in chunks, and then issuing a final "commit", if it makes sense
for the particulars of the action being performed.

The main reasons we give when designing packets and pushing in the
direction of sending one big packet, are simplicity, and minimizing
roundtrips.  The reverse side of the coin, is that then we need
to care whether we're overflowing the maximum packet buffer size.
Actually, the packet proposed in this thread has code exactly for
that, because a request for catching syscalls
lists all the syscalls GDB is interested in.

But, this is talking about the semantics of the commands/packets,
IOW, talking at a higher conceptual level than the acks/naks or
sequence numbers.

My main complain here is with the bad mixup of transport and application
layers, which ends up manifesting in things like the Z/z packets having
this requirement:

 "To avoid potential problems with duplicate packets, the operations
 should be implemented in an idempotent way."

(that's talking about Z vs z).

And this meant we ended up e.g., merging all multiple breakpoints
with target-assisted conditions, and breakpoints with target-side
commands all as a single breakpoint on the target, as long as their
at the same address.

[And then that's the root cause of breakpoints/15180.
If we try extending the Z packets to support target-side
thread-specific breakpoint handling, such merging by address gets
even less clearly a good idea to me.]

With all that in mind, and given we do have plenty of sequences
in the protocol that do require state, let me clarify my original
concern:

> There's also the issue with the fact that
> for Z packets, the RSP specifies that a second Z packet seen for
> the same address replaces the previous packet, because it might
> have happened that GDB and the server lost sync for a bit, and the
> second packet was actually a retransmission.  Making QInsertCatchpoint
> return a reference conflicts with that.

... by saying that IMO, this "potential problems with duplicate packets"
concern in the Z/z packets is IMO solved in the wrong place, and
that issue IMO should not prevent coming up with a breakpoint/catchpoint
packet that returns a target-generated token/reference/handle for
the created resource.  A better solution for that would IMO be done
at the "transport" side of the protocol (at the level of ack/naks),
and that probably would mean introducing sequence numbers.

> Another well-known stateless protocol is HTTP, needed due to its
> own unpredictabilities (as we all experience every day :-) ).  Now
> there are situations where statefulness is useful, which they solve
> by using cookies.  In the remote protocol, our analogous situation
> is where the target needs to be more like an agent, such as with
> tracing, and in those cases we do have identifiers that are
> coordinated between host and target, though in a somewhat adhoc
> way right now.

Right.

> I think we could develop the id concept into more of a generic mechanism
> that is available for target-side commands, actionpoints in general,
> and so forth, perhaps with everything using a single numeric (or
> textual?) space, so that "231" can be unambiguously a catchpoint,
> rather than either a catchpoint or a trace state variable, depending
> on context.

Yeah, good idea.  That might came in handy.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]