Fix IPv6 support on lldb-server platform

Summary:
This is a general fix for the ConnectionFileDescriptor class but my main motivation was to make lldb-server working with IPv6.
The connect URI can use square brackets ([]) to wrap the interface part of the URI (e.g.: <scheme>://[<interface>]:<port>). For IPv6 addresses this is a must since its ip can include colons and it will overlap with the port colon otherwise. The URIParser class parses the square brackets correctly but the ConnectionFileDescriptor doesn't generate them for IPv6 addresses making it impossible to connect to the gdb server when using this protocol.

How to reproduce the issue:
```
$ lldb-server p --server --listen [::1]:8080
...
$ lldb
(lldb) platform select remote-macosx
(lldb) platform connect connect://[::1]:8080
(lldb) platform process -p <pid>
error: unable to launch a GDB server on 'computer'
```

The server was actually launched we were just not able to connect to it. With this fix lldb will correctly connect. I fixed this by wrapping the ip portion with [].

Reviewers: labath

Reviewed By: labath

Subscribers: xiaobai, mgorny, jfb, lldb-commits, labath

Tags: #lldb

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

llvm-svn: 361898

diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 167569dca69e971d1045466e5f87a355cdf4527f..237d11acb418f5f07d553a859a6f7b8f657848b8 100644 (file)
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -764,7 +764,7 @@ void ConnectionFileDescriptor::InitializeSocket(Socket *socket) {
   m_write_sp.reset(socket);
   m_read_sp = m_write_sp;
   StreamString strm;
-  strm.Printf("connect://%s:%u", tcp_socket->GetRemoteIPAddress().c_str(),
+  strm.Printf("connect://[%s]:%u", tcp_socket->GetRemoteIPAddress().c_str(),
               tcp_socket->GetRemotePortNumber());
   m_uri = strm.GetString();
 }

diff --git a/lldb/unittests/Host/CMakeLists.txt b/lldb/unittests/Host/CMakeLists.txt
index 8c79bcfdc8fefa15018fc0b8615c532fb3cffec5..cf7c7cacfe2bbf8a1e5751167d1dd5bb31fbdfc2 100644 (file)
--- a/lldb/unittests/Host/CMakeLists.txt
+++ b/lldb/unittests/Host/CMakeLists.txt
@@ -1,4 +1,5 @@
 set (FILES
+  ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
   HostInfoTest.cpp
@@ -8,6 +9,7 @@ set (FILES
   ProcessLaunchInfoTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
+  SocketTestUtilities.cpp
   TaskPoolTest.cpp
 )
 

diff --git a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
new file mode 100644 (file)
index 0000000..229e0ee
--- /dev/null
+++ b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
@@ -0,0 +1,50 @@
+//===-- ConnectionFileDescriptorTest.cpp ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SocketTestUtilities.h"
+#include "gtest/gtest.h"
+
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
+#include "lldb/Utility/UriParser.h"
+
+using namespace lldb_private;
+
+class ConnectionFileDescriptorTest : public testing::Test {
+public:
+  void SetUp() override {
+    ASSERT_THAT_ERROR(Socket::Initialize(), llvm::Succeeded());
+  }
+
+  void TearDown() override { Socket::Terminate(); }
+
+  void TestGetURI(std::string ip) {
+    std::unique_ptr<TCPSocket> socket_a_up;
+    std::unique_ptr<TCPSocket> socket_b_up;
+    if (!IsAddressFamilySupported(ip)) {
+      GTEST_LOG_(WARNING) << "Skipping test due to missing IPv"
+                          << (IsIPv4(ip) ? "4" : "6") << " support.";
+      return;
+    }
+    CreateTCPConnectedSockets(ip, &socket_a_up, &socket_b_up);
+    auto socket = socket_a_up.release();
+    ConnectionFileDescriptor connection_file_descriptor(socket);
+
+    llvm::StringRef scheme;
+    llvm::StringRef hostname;
+    int port;
+    llvm::StringRef path;
+    std::string uri(connection_file_descriptor.GetURI());
+    EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
+    EXPECT_EQ(ip, hostname);
+    EXPECT_EQ(socket->GetRemotePortNumber(), port);
+  }
+};
+
+TEST_F(ConnectionFileDescriptorTest, TCPGetURIv4) { TestGetURI("127.0.0.1"); }
+
+TEST_F(ConnectionFileDescriptorTest, TCPGetURIv6) { TestGetURI("::1"); }
\ No newline at end of file

diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
index 1192fa65f875e90d0f72792a21317c44187078e0..26a8bd765c6efbec7c9843e1fa7d10b122291bad 100644 (file)
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -6,24 +6,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <cstdio>
-#include <functional>
-#include <thread>
-
+#include "SocketTestUtilities.h"
 #include "gtest/gtest.h"
 
-#include "lldb/Host/Config.h"
-#include "lldb/Host/Socket.h"
-#include "lldb/Host/common/TCPSocket.h"
-#include "lldb/Host/common/UDPSocket.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
-#include "llvm/Testing/Support/Error.h"
-
-#ifndef LLDB_DISABLE_POSIX
-#include "lldb/Host/posix/DomainSocket.h"
-#endif
-
 using namespace lldb_private;
 
 class SocketTest : public testing::Test {
@@ -33,55 +18,6 @@ public:
   }
 
   void TearDown() override { Socket::Terminate(); }
-
-protected:
-  static void AcceptThread(Socket *listen_socket,
-                           bool child_processes_inherit, Socket **accept_socket,
-                           Status *error) {
-    *error = listen_socket->Accept(*accept_socket);
-  }
-
-  template <typename SocketType>
-  void CreateConnectedSockets(
-      llvm::StringRef listen_remote_address,
-      const std::function<std::string(const SocketType &)> &get_connect_addr,
-      std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up) {
-    bool child_processes_inherit = false;
-    Status error;
-    std::unique_ptr<SocketType> listen_socket_up(
-        new SocketType(true, child_processes_inherit));
-    EXPECT_FALSE(error.Fail());
-    error = listen_socket_up->Listen(listen_remote_address, 5);
-    EXPECT_FALSE(error.Fail());
-    EXPECT_TRUE(listen_socket_up->IsValid());
-
-    Status accept_error;
-    Socket *accept_socket;
-    std::thread accept_thread(AcceptThread, listen_socket_up.get(),
-                              child_processes_inherit, &accept_socket,
-                              &accept_error);
-
-    std::string connect_remote_address = get_connect_addr(*listen_socket_up);
-    std::unique_ptr<SocketType> connect_socket_up(
-        new SocketType(true, child_processes_inherit));
-    EXPECT_FALSE(error.Fail());
-    error = connect_socket_up->Connect(connect_remote_address);
-    EXPECT_FALSE(error.Fail());
-    EXPECT_TRUE(connect_socket_up->IsValid());
-
-    a_up->swap(connect_socket_up);
-    EXPECT_TRUE(error.Success());
-    EXPECT_NE(nullptr, a_up->get());
-    EXPECT_TRUE((*a_up)->IsValid());
-
-    accept_thread.join();
-    b_up->reset(static_cast<SocketType *>(accept_socket));
-    EXPECT_TRUE(accept_error.Success());
-    EXPECT_NE(nullptr, b_up->get());
-    EXPECT_TRUE((*b_up)->IsValid());
-
-    listen_socket_up.reset();
-  }
 };
 
 TEST_F(SocketTest, DecodeHostAndPort) {
@@ -159,38 +95,24 @@ TEST_F(SocketTest, DomainListenConnectAccept) {
 
   std::unique_ptr<DomainSocket> socket_a_up;
   std::unique_ptr<DomainSocket> socket_b_up;
-  CreateConnectedSockets<DomainSocket>(
-      Path, [=](const DomainSocket &) { return Path.str().str(); },
-      &socket_a_up, &socket_b_up);
+  CreateDomainConnectedSockets(Path, &socket_a_up, &socket_b_up);
 }
 #endif
 
 TEST_F(SocketTest, TCPListen0ConnectAccept) {
   std::unique_ptr<TCPSocket> socket_a_up;
   std::unique_ptr<TCPSocket> socket_b_up;
-  CreateConnectedSockets<TCPSocket>(
-      "127.0.0.1:0",
-      [=](const TCPSocket &s) {
-        char connect_remote_address[64];
-        snprintf(connect_remote_address, sizeof(connect_remote_address),
-                 "127.0.0.1:%u", s.GetLocalPortNumber());
-        return std::string(connect_remote_address);
-      },
-      &socket_a_up, &socket_b_up);
+  CreateTCPConnectedSockets("127.0.0.1", &socket_a_up, &socket_b_up);
 }
 
 TEST_F(SocketTest, TCPGetAddress) {
   std::unique_ptr<TCPSocket> socket_a_up;
   std::unique_ptr<TCPSocket> socket_b_up;
-  CreateConnectedSockets<TCPSocket>(
-      "127.0.0.1:0",
-      [=](const TCPSocket &s) {
-        char connect_remote_address[64];
-        snprintf(connect_remote_address, sizeof(connect_remote_address),
-                 "127.0.0.1:%u", s.GetLocalPortNumber());
-        return std::string(connect_remote_address);
-      },
-      &socket_a_up, &socket_b_up);
+  if (!IsAddressFamilySupported("127.0.0.1")) {
+    GTEST_LOG_(WARNING) << "Skipping test due to missing IPv4 support.";
+    return;
+  }
+  CreateTCPConnectedSockets("127.0.0.1", &socket_a_up, &socket_b_up);
 
   EXPECT_EQ(socket_a_up->GetLocalPortNumber(),
             socket_b_up->GetRemotePortNumber());

diff --git a/lldb/unittests/Host/SocketTestUtilities.cpp b/lldb/unittests/Host/SocketTestUtilities.cpp
new file mode 100644 (file)
index 0000000..660aba0
--- /dev/null
+++ b/lldb/unittests/Host/SocketTestUtilities.cpp
@@ -0,0 +1,104 @@
+//===----------------- SocketTestUtilities.cpp ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SocketTestUtilities.h"
+#include "lldb/Utility/StreamString.h"
+
+#ifdef _WIN32
+#include <winsock2.h>
+#include <ws2tcpip.h>
+#else
+#include <arpa/inet.h>
+#endif
+
+using namespace lldb_private;
+
+static void AcceptThread(Socket *listen_socket, bool child_processes_inherit,
+                         Socket **accept_socket, Status *error) {
+  *error = listen_socket->Accept(*accept_socket);
+}
+
+template <typename SocketType>
+void lldb_private::CreateConnectedSockets(
+    llvm::StringRef listen_remote_address,
+    const std::function<std::string(const SocketType &)> &get_connect_addr,
+    std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up) {
+  bool child_processes_inherit = false;
+  Status error;
+  std::unique_ptr<SocketType> listen_socket_up(
+      new SocketType(true, child_processes_inherit));
+  EXPECT_FALSE(error.Fail());
+  error = listen_socket_up->Listen(listen_remote_address, 5);
+  EXPECT_FALSE(error.Fail());
+  EXPECT_TRUE(listen_socket_up->IsValid());
+
+  Status accept_error;
+  Socket *accept_socket;
+  std::thread accept_thread(AcceptThread, listen_socket_up.get(),
+                            child_processes_inherit, &accept_socket,
+                            &accept_error);
+
+  std::string connect_remote_address = get_connect_addr(*listen_socket_up);
+  std::unique_ptr<SocketType> connect_socket_up(
+      new SocketType(true, child_processes_inherit));
+  EXPECT_FALSE(error.Fail());
+  error = connect_socket_up->Connect(connect_remote_address);
+  EXPECT_FALSE(error.Fail());
+  EXPECT_TRUE(connect_socket_up->IsValid());
+
+  a_up->swap(connect_socket_up);
+  EXPECT_TRUE(error.Success());
+  EXPECT_NE(nullptr, a_up->get());
+  EXPECT_TRUE((*a_up)->IsValid());
+
+  accept_thread.join();
+  b_up->reset(static_cast<SocketType *>(accept_socket));
+  EXPECT_TRUE(accept_error.Success());
+  EXPECT_NE(nullptr, b_up->get());
+  EXPECT_TRUE((*b_up)->IsValid());
+
+  listen_socket_up.reset();
+}
+
+bool lldb_private::CreateTCPConnectedSockets(
+    std::string listen_remote_ip, std::unique_ptr<TCPSocket> *socket_a_up,
+    std::unique_ptr<TCPSocket> *socket_b_up) {
+  StreamString strm;
+  strm.Printf("[%s]:0", listen_remote_ip.c_str());
+  CreateConnectedSockets<TCPSocket>(
+      strm.GetString(),
+      [=](const TCPSocket &s) {
+        char connect_remote_address[64];
+        snprintf(connect_remote_address, sizeof(connect_remote_address),
+                 "[%s]:%u", listen_remote_ip.c_str(), s.GetLocalPortNumber());
+        return std::string(connect_remote_address);
+      },
+      socket_a_up, socket_b_up);
+  return true;
+}
+
+#ifndef LLDB_DISABLE_POSIX
+void lldb_private::CreateDomainConnectedSockets(
+    llvm::StringRef path, std::unique_ptr<DomainSocket> *socket_a_up,
+    std::unique_ptr<DomainSocket> *socket_b_up) {
+  return CreateConnectedSockets<DomainSocket>(
+      path, [=](const DomainSocket &) { return path.str(); }, socket_a_up,
+      socket_b_up);
+}
+#endif
+
+bool lldb_private::IsAddressFamilySupported(std::string ip) {
+  auto addresses = lldb_private::SocketAddress::GetAddressInfo(
+      ip.c_str(), NULL, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+  return addresses.size() > 0;
+}
+
+bool lldb_private::IsIPv4(std::string ip) {
+  struct sockaddr_in sock_addr;
+  return inet_pton(AF_INET, ip.c_str(), &(sock_addr.sin_addr)) != 0;
+}

diff --git a/lldb/unittests/Host/SocketTestUtilities.h b/lldb/unittests/Host/SocketTestUtilities.h
new file mode 100644 (file)
index 0000000..4e51be9
--- /dev/null
+++ b/lldb/unittests/Host/SocketTestUtilities.h
@@ -0,0 +1,47 @@
+//===--------------------- SocketTestUtilities.h ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UNITTESTS_HOST_SOCKETTESTUTILITIES_H
+#define LLDB_UNITTESTS_HOST_SOCKETTESTUTILITIES_H
+
+#include <cstdio>
+#include <functional>
+#include <thread>
+
+#include "lldb/Host/Config.h"
+#include "lldb/Host/Socket.h"
+#include "lldb/Host/common/TCPSocket.h"
+#include "lldb/Host/common/UDPSocket.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
+
+#ifndef LLDB_DISABLE_POSIX
+#include "lldb/Host/posix/DomainSocket.h"
+#endif
+
+namespace lldb_private {
+template <typename SocketType>
+void CreateConnectedSockets(
+    llvm::StringRef listen_remote_address,
+    const std::function<std::string(const SocketType &)> &get_connect_addr,
+    std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up);
+bool CreateTCPConnectedSockets(std::string listen_remote_ip,
+                               std::unique_ptr<TCPSocket> *a_up,
+                               std::unique_ptr<TCPSocket> *b_up);
+#ifndef LLDB_DISABLE_POSIX
+void CreateDomainConnectedSockets(llvm::StringRef path,
+                                  std::unique_ptr<DomainSocket> *a_up,
+                                  std::unique_ptr<DomainSocket> *b_up);
+#endif
+
+bool IsAddressFamilySupported(std::string ip);
+bool IsIPv4(std::string ip);
+} // namespace lldb_private
+
+#endif
\ No newline at end of file