Page MenuHomePhabricator

[lldb/gdb-remote] Ignore spurious ACK packets
ClosedPublic

Authored by labath on Nov 24 2021, 2:47 AM.

Details

Summary

Although I cannot find any mention of this in the specification, both
gdb and lldb agree on sending an initial + packet after establishing the
connection.

OTOH, gdbserver and lldb-server behavior is subtly different. While
lldb-server *expects* the initial ack, and drops the connection if it is
not received, gdbserver will just ignore a spurious ack at _any_ point
in the connection.

This patch changes lldb's behavior to match that of gdb. An ACK packet
is ignored at any point in the connection (except when expecting an ACK
packet, of course). This is inline with the "be strict in what you
generate, and lenient in what you accept" philosophy, and also enables
us to remove some special cases from the server code. I've extended the
same handling to NAK (-) packets, mainly because I don't see a reason to
treat them differently here.

(The background here is that we had a stub which was sending spurious
+ packets. This bug has since been fixed, but I think this change makes
sense nonetheless.)

Diff Detail

Event Timeline

labath requested review of this revision.Nov 24 2021, 2:47 AM
labath created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2021, 2:47 AM
mgorny accepted this revision.Nov 24 2021, 3:06 AM
mgorny added inline comments.
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
56

I don't get how this is relevant to the patch in question.

This revision is now accepted and ready to land.Nov 24 2021, 3:06 AM
labath added inline comments.Nov 24 2021, 4:36 AM
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
56

It's not directly related, but in the process of adding comments to the test cases I realized that this test case doesn't make sense (there's no need to escape a ]), so I changed into what the author probably meant to test (escaping a }).

This revision was automatically updated to reflect the committed changes.