[Lldb-commits] [PATCH] D126702: [lldb] Fix TCPSocket::Connect when getaddrinfo returns multiple addrs

[Daniele Di Proietto via Phabricator via lldb-commits]

ddiproietto created this revision.
Herald added a project: All.
ddiproietto requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

TCPSocket::Connect() calls SocketAddress::GetAddressInfo() and tries to
connect any of them (in a for loop).

This used to work before commit 4f6d3a376c9f <https://reviews.llvm.org/rG4f6d3a376c9faba93bbdf105966cea7585b0b8e9>("[LLDB] Fix setting of
success in Socket::Close()") https://reviews.llvm.org/D116768.

As a side effect of that commit, TCPSocket can only connect to the first
address returned by SocketAddress::GetAddressInfo().

1. If the attempt to connect to the first address fails, TCPSocket::Connect(), calls CLOSE_SOCKET(GetNativeSocket()), which closes the fd, but DOES NOT set m_socket to kInvalidSocketValue.
2. On the second attempt, TCPSocket::CreateSocket() calls Socket::Close().
3. Socket::Close() proceeds, because IsValid() is true (m_socket was not reset on step 1).
4. Socket::Close() calls ::close(m_socket), which fails
5. Since commit 4f6d3a376c9f <https://reviews.llvm.org/rG4f6d3a376c9faba93bbdf105966cea7585b0b8e9>("[LLDB] Fix setting of success in Socket::Close()"), this is error is detected. Socket::Close() returns an error.
6. TCPSocket::CreateSocket() therefore returns an error.
7. TCPSocket::Connect() detects the error and continues, skipping the second (and the third, fourth...) address.

This commit fixes the problem by changing step 1: by calling
Socket::Close, instead of directly calling close(m_socket), m_socket is
also se to kInvalidSocketValue. On step 3, Socket::Close() is going to
return immediately and, on step 6, TCPSocket::CreateSocket() does not 
fail.

How to reproduce this problem:

On my system, getaddrinfo() resolves "localhost" to "::1" (first) and to
"127.0.0.1" (second).

Start a gdbserver that only listens on 127.0.0.1:

  gdbserver 127.0.0.1:2159 /bin/cat
  Process /bin/cat created; pid = 2146709
  Listening on port 2159

Start lldb and make it connect to "localhost:2159"

  ./bin/lldb
  (lldb) gdb-remote localhost:2159

Before 4f6d3a376c9f <https://reviews.llvm.org/rG4f6d3a376c9faba93bbdf105966cea7585b0b8e9>("[LLDB] Fix setting of success in Socket::Close()"),
this used to work. After that commit, it stopped working. This commit
fixes the problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126702

Files:
  lldb/source/Host/common/TCPSocket.cpp


Index: lldb/source/Host/common/TCPSocket.cpp
===================================================================
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -170,7 +170,7 @@
     if (-1 == llvm::sys::RetryAfterSignal(-1, ::connect, GetNativeSocket(),
                                           &address.sockaddr(),
                                           address.GetLength())) {
-      CLOSE_SOCKET(GetNativeSocket());
+      Close();
       continue;
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126702.433086.patch
Type: text/x-patch
Size: 507 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220531/0fdfcf03/attachment.bin>