Skip to content

Commit

Permalink
Fix IPv6 support on lldb-server platform
Browse files Browse the repository at this point in the history
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 [].

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

Patch by António Afonso <antonio.afonso@gmail.com>

llvm-svn: 361079
  • Loading branch information
bulbazord authored and MrSidims committed May 24, 2019
1 parent 0e7d4a0 commit 024247f
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 87 deletions.
2 changes: 1 addition & 1 deletion lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
Expand Up @@ -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();
}
2 changes: 2 additions & 0 deletions lldb/unittests/Host/CMakeLists.txt
Expand Up @@ -9,6 +9,8 @@ set (FILES
SocketAddressTest.cpp
SocketTest.cpp
TaskPoolTest.cpp
SocketTestUtilities.cpp
ConnectionFileDescriptorTest.cpp
)

if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Expand Down
50 changes: 50 additions & 0 deletions 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;
EXPECT_TRUE(UriParser::Parse(connection_file_descriptor.GetURI(), 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"); }
94 changes: 8 additions & 86 deletions lldb/unittests/Host/SocketTest.cpp
Expand Up @@ -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 {
Expand All @@ -33,55 +18,6 @@ class SocketTest : public testing::Test {
}

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) {
Expand Down Expand Up @@ -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());
Expand Down
98 changes: 98 additions & 0 deletions lldb/unittests/Host/SocketTestUtilities.cpp
@@ -0,0 +1,98 @@
//===----------------- 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"
#include <arpa/inet.h>

using namespace lldb_private;

const 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;
}
47 changes: 47 additions & 0 deletions 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

0 comments on commit 024247f

Please sign in to comment.