[all-commits] [llvm/llvm-project] a6fedb: [lldb/gdb-remote] Remove initial pipe-draining wor...

[Pavel Labath via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: a6fedbf20c8f87061f333169120790e2c6f22806
      https://github.com/llvm/llvm-project/commit/a6fedbf20c8f87061f333169120790e2c6f22806
  Author: Pavel Labath <pavel at labath.sk>
  Date:   2021-11-25 (Thu, 25 Nov 2021)

  Changed paths:
    M lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

  Log Message:
  -----------
  [lldb/gdb-remote] Remove initial pipe-draining workaround

This code, added in rL197579 (Dec 2013) is supposed to work around what
was presumably a qemu bug, where it would send unsolicited stop-reply
packets after the initial connection.

At present, qemu does not exhibit such behavior. Also, the 10ms delay
introduced by this code is sufficient to mask bugs in other stubs, but
it is not sufficient to *reliably* mask those bugs. This resulted in
flakyness in one of our stubs, which was (incorrectly) sending a +
packet at the start of the connection, resulting in a small-but-annoying
number of dropped connections.

Differential Revision: https://reviews.llvm.org/D114529


  Commit: 165545c7a431aa3682fd9468014771c1c5228226
      https://github.com/llvm/llvm-project/commit/165545c7a431aa3682fd9468014771c1c5228226
  Author: Pavel Labath <pavel at labath.sk>
  Date:   2021-11-25 (Thu, 25 Nov 2021)

  Changed paths:
    M lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
    M lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
    M lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
    M lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
    M lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
    M lldb/tools/lldb-server/lldb-platform.cpp
    M lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
    M lldb/unittests/Process/gdb-remote/GDBRemoteTestUtils.h

  Log Message:
  -----------
  [lldb/gdb-remote] Ignore spurious ACK packets

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.)

Differential Revision: https://reviews.llvm.org/D114520


Compare: https://github.com/llvm/llvm-project/compare/63b1e58f0738...165545c7a431