https://github.com/labath created
https://github.com/llvm/llvm-project/pull/145293
Originally added for reproducers, it is now only used for test code.
While we could make it a test helper, I think that after #145015 it is simple
enough to not be needed.
Also squeeze in a change to make ConnectionFileDescriptor accept a
unique_ptr<Socket>.
>From a1c32e3bcdf9355f6564754ed9e77880002289d5 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Mon, 23 Jun 2025 10:53:15 +0200
Subject: [PATCH] [lldb] Remove GDBRemoteCommunication::ConnectLocally
Originally added for reproducers, it is now only used for test code.
While we could make it a test helper, I think that after #145015 it is
simple enough to not be needed.
Also squeeze in a change to make ConnectionFileDescriptor accept a
unique_ptr<Socket>.
---
.../Host/posix/ConnectionFileDescriptorPosix.h | 7 +------
.../posix/ConnectionFileDescriptorPosix.cpp | 13 +++++--------
.../gdb-remote/GDBRemoteCommunication.cpp | 18 ++----------------
.../gdb-remote/GDBRemoteCommunication.h | 3 ---
lldb/tools/lldb-server/lldb-platform.cpp | 4 ++--
lldb/unittests/Core/CommunicationTest.cpp | 4 ++--
.../Host/ConnectionFileDescriptorTest.cpp | 6 +++---
.../gdb-remote/GDBRemoteClientBaseTest.cpp | 9 +++++++--
.../GDBRemoteCommunicationClientTest.cpp | 9 +++++++--
.../gdb-remote/GDBRemoteCommunicationTest.cpp | 10 ++++++++--
.../tools/lldb-server/tests/TestClient.cpp | 3 ++-
11 files changed, 39 insertions(+), 47 deletions(-)
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index b771f9c3f45a2..df0e196fea688 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -18,13 +18,10 @@
#include "lldb/Host/Pipe.h"
#include "lldb/Host/Socket.h"
#include "lldb/Utility/Connection.h"
-#include "lldb/Utility/IOObject.h"
namespace lldb_private {
class Status;
-class Socket;
-class SocketAddress;
class ConnectionFileDescriptor : public Connection {
public:
@@ -35,7 +32,7 @@ class ConnectionFileDescriptor : public Connection {
ConnectionFileDescriptor(int fd, bool owns_fd);
- ConnectionFileDescriptor(Socket *socket);
+ ConnectionFileDescriptor(std::unique_ptr<Socket> socket_up);
~ConnectionFileDescriptor() override;
@@ -136,8 +133,6 @@ class ConnectionFileDescriptor : public Connection {
std::string m_uri;
private:
- void InitializeSocket(Socket *socket);
-
ConnectionFileDescriptor(const ConnectionFileDescriptor &) = delete;
const ConnectionFileDescriptor &
operator=(const ConnectionFileDescriptor &) = delete;
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index ae935dda5af4e..57dce44812c89 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -72,9 +72,11 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(int fd,
bool owns_fd)
OpenCommandPipe();
}
-ConnectionFileDescriptor::ConnectionFileDescriptor(Socket *socket)
- : Connection(), m_pipe(), m_mutex(), m_shutting_down(false) {
- InitializeSocket(socket);
+ConnectionFileDescriptor::ConnectionFileDescriptor(
+ std::unique_ptr<Socket> socket_up)
+ : m_shutting_down(false) {
+ m_uri = socket_up->GetRemoteConnectionURI();
+ m_io_sp = std::move(socket_up);
}
ConnectionFileDescriptor::~ConnectionFileDescriptor() {
@@ -796,8 +798,3 @@ ConnectionStatus
ConnectionFileDescriptor::ConnectSerialPort(
#endif // LLDB_ENABLE_POSIX
llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
}
-
-void ConnectionFileDescriptor::InitializeSocket(Socket *socket) {
- m_io_sp.reset(socket);
- m_uri = socket->GetRemoteConnectionURI();
-}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index d1f57cc22d8bd..0d3ead840b080 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1128,8 +1128,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
Socket *accepted_socket = nullptr;
error = sock_up->Accept(/*timeout=*/std::nullopt, accepted_socket);
if (accepted_socket) {
- SetConnection(
- std::make_unique<ConnectionFileDescriptor>(accepted_socket));
+ SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ std::unique_ptr<Socket>(accepted_socket)));
}
}
@@ -1138,20 +1138,6 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm);
}
-llvm::Error
-GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
- GDBRemoteCommunication &server) {
- llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
- if (!pair)
- return pair.takeError();
-
- client.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->first.release()));
- server.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->second.release()));
- return llvm::Error::success();
-}
-
GDBRemoteCommunication::ScopedTimeout::ScopedTimeout(
GDBRemoteCommunication &gdb_comm, std::chrono::seconds timeout)
: m_gdb_comm(gdb_comm), m_saved_timeout(0), m_timeout_modified(false) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index ee87629d9077b..fc86f801f0d8a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -149,9 +149,6 @@ class GDBRemoteCommunication : public Communication {
void DumpHistory(Stream &strm);
- static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
- GDBRemoteCommunication &server);
-
/// Expand GDB run-length encoding.
static std::optional<std::string> ExpandRLE(std::string);
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 37ec8455c63a7..3c79dc001f65e 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -485,8 +485,8 @@ int main_platform(int argc, char *argv[]) {
GDBRemoteCommunicationServerPlatform platform(socket->GetSocketProtocol(),
gdbserver_port);
- platform.SetConnection(std::unique_ptr<Connection>(
- new ConnectionFileDescriptor(socket.release())));
+ platform.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(socket)));
client_handle(platform, inferior_arguments);
return 0;
}
diff --git a/lldb/unittests/Core/CommunicationTest.cpp
b/lldb/unittests/Core/CommunicationTest.cpp
index fd07b4da9f8c1..51e218af8baf4 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -41,7 +41,7 @@ static void CommunicationReadTest(bool use_read_thread) {
ThreadedCommunication comm("test");
comm.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->second.release()));
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
comm.SetCloseOnEOF(true);
if (use_read_thread) {
@@ -126,7 +126,7 @@ TEST_F(CommunicationTest, SynchronizeWhileClosing) {
ThreadedCommunication comm("test");
comm.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(pair->second.release()));
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
comm.SetCloseOnEOF(true);
ASSERT_TRUE(comm.StartReadThread());
diff --git a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
index 250ed6fe8fd37..171481acca01b 100644
--- a/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
+++ b/lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
@@ -22,11 +22,11 @@ class ConnectionFileDescriptorTest : public testing::Test {
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
CreateTCPConnectedSockets(ip, &socket_a_up, &socket_b_up);
- auto *socket = socket_a_up.release();
- ConnectionFileDescriptor connection_file_descriptor(socket);
+ uint16_t socket_a_remote_port = socket_a_up->GetRemotePortNumber();
+ ConnectionFileDescriptor
connection_file_descriptor(std::move(socket_a_up));
std::string uri(connection_file_descriptor.GetURI());
- EXPECT_EQ((URI{"connect", ip, socket->GetRemotePortNumber(), "/"}),
+ EXPECT_EQ((URI{"connect", ip, socket_a_remote_port, "/"}),
*URI::Parse(uri));
}
};
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
index ed77e86ac3701..b28b8013bcdc7 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -10,6 +10,7 @@
#include "GDBRemoteTestUtils.h"
#include "Plugins/Process/Utility/LinuxSignals.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Utility/GDBRemote.h"
#include "lldb/Utility/Listener.h"
#include "llvm/ADT/StringRef.h"
@@ -51,8 +52,12 @@ struct TestClient : public GDBRemoteClientBase {
class GDBRemoteClientBaseTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
- llvm::Succeeded());
+ llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(pair, llvm::Succeeded());
+ client.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->first)));
+ server.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
ASSERT_EQ(TestClient::eBroadcastBitRunPacketSent,
listener_sp->StartListeningForEvents(
&client, TestClient::eBroadcastBitRunPacketSent));
diff --git
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 96f377ec7dfac..f940229985887 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -8,6 +8,7 @@
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
#include "GDBRemoteTestUtils.h"
#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/XML.h"
#include "lldb/Target/MemoryRegionInfo.h"
#include "lldb/Utility/DataBuffer.h"
@@ -63,8 +64,12 @@ std::string one_register_hex = "41424344";
class GDBRemoteCommunicationClientTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
- llvm::Succeeded());
+ llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(pair, llvm::Succeeded());
+ client.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->first)));
+ server.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
}
protected:
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
index 3da1f0a718fc5..e96d587b10e25 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
@@ -5,7 +5,9 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
+
#include "GDBRemoteTestUtils.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
#include "llvm/Testing/Support/Error.h"
using namespace lldb_private::process_gdb_remote;
@@ -28,8 +30,12 @@ class TestClient : public GDBRemoteCommunication {
class GDBRemoteCommunicationTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
- llvm::Succeeded());
+ llvm::Expected<Socket::Pair> pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(pair, llvm::Succeeded());
+ client.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->first)));
+ server.SetConnection(
+ std::make_unique<ConnectionFileDescriptor>(std::move(pair->second)));
}
protected:
diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
index 48e089bc0fcff..f3510cad22e7f 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -125,7 +125,8 @@ TestClient::launchCustom(StringRef Log, bool disable_stdio,
listen_socket.Accept(2 * GetDefaultTimeout(), accept_socket)
.takeError())
return E;
- auto Conn = std::make_unique<ConnectionFileDescriptor>(accept_socket);
+ auto Conn = std::make_unique<ConnectionFileDescriptor>(
+ std::unique_ptr<Socket>(accept_socket));
auto Client = std::unique_ptr<TestClient>(new TestClient(std::move(Conn)));
if (Error E = Client->initializeConnection())
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
