[lldb] Use Socket::CreatePair for launching debugserver #145017
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesThis lets get rid of platform-specific code in ProcessGDBRemote and use the Depends on #145015. (This PR consists of two commits, the first of which is equivalent to #145015. For reviewing, I recommend only looking at the second commit (if you have comments on the first commit, please put them on the first PR.) Full diff: https://github.com/llvm/llvm-project/pull/145017.diff 10 Files Affected:
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index c313aa4f6d26b..14a9660ed30b7 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -106,6 +106,10 @@ class Socket : public IOObject {
static std::unique_ptr<Socket> Create(const SocketProtocol protocol,
Status &error);
+ static llvm::Expected<
+ std::pair<std::unique_ptr<Socket>, std::unique_ptr<Socket>>>
+ CreatePair(std::optional<SocketProtocol> protocol = std::nullopt);
+
virtual Status Connect(llvm::StringRef name) = 0;
virtual Status Listen(llvm::StringRef name, int backlog) = 0;
diff --git a/lldb/include/lldb/Host/common/TCPSocket.h b/lldb/include/lldb/Host/common/TCPSocket.h
index cb950c0015ea6..e81cb82dbcba1 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -23,6 +23,10 @@ class TCPSocket : public Socket {
TCPSocket(NativeSocket socket, bool should_close);
~TCPSocket() override;
+ static llvm::Expected<
+ std::pair<std::unique_ptr<TCPSocket>, std::unique_ptr<TCPSocket>>>
+ CreatePair();
+
// returns port number or 0 if error
uint16_t GetLocalPortNumber() const;
diff --git a/lldb/include/lldb/Host/posix/DomainSocket.h b/lldb/include/lldb/Host/posix/DomainSocket.h
index a840d474429ec..c3a6a64bbdef8 100644
--- a/lldb/include/lldb/Host/posix/DomainSocket.h
+++ b/lldb/include/lldb/Host/posix/DomainSocket.h
@@ -19,6 +19,10 @@ class DomainSocket : public Socket {
DomainSocket(NativeSocket socket, bool should_close);
explicit DomainSocket(bool should_close);
+ static llvm::Expected<
+ std::pair<std::unique_ptr<DomainSocket>, std::unique_ptr<DomainSocket>>>
+ CreatePair();
+
Status Connect(llvm::StringRef name) override;
Status Listen(llvm::StringRef name, int backlog) override;
diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp
index 5c5cd653c3d9e..c9dec0f8ea22a 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -234,6 +234,23 @@ std::unique_ptr<Socket> Socket::Create(const SocketProtocol protocol,
return socket_up;
}
+llvm::Expected<std::pair<std::unique_ptr<Socket>, std::unique_ptr<Socket>>>
+Socket::CreatePair(std::optional<SocketProtocol> protocol) {
+ constexpr SocketProtocol kBestProtocol =
+ LLDB_ENABLE_POSIX ? ProtocolUnixDomain : ProtocolTcp;
+ switch (protocol.value_or(kBestProtocol)) {
+ case ProtocolTcp:
+ return TCPSocket::CreatePair();
+#if LLDB_ENABLE_POSIX
+ case ProtocolUnixDomain:
+ case ProtocolUnixAbstract:
+ return DomainSocket::CreatePair();
+#endif
+ default:
+ return llvm::createStringError("Unsupported protocol");
+ }
+}
+
llvm::Expected<std::unique_ptr<Socket>>
Socket::TcpConnect(llvm::StringRef host_and_port) {
Log *log = GetLog(LLDBLog::Connection);
diff --git a/lldb/source/Host/common/TCPSocket.cpp b/lldb/source/Host/common/TCPSocket.cpp
index 3d0dea1c61dd6..34f249746149e 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -52,6 +52,34 @@ TCPSocket::TCPSocket(NativeSocket socket, bool should_close)
TCPSocket::~TCPSocket() { CloseListenSockets(); }
+llvm::Expected<
+ std::pair<std::unique_ptr<TCPSocket>, std::unique_ptr<TCPSocket>>>
+TCPSocket::CreatePair() {
+ auto listen_socket_up = std::make_unique<TCPSocket>(true);
+ if (Status error = listen_socket_up->Listen("localhost:0", 5); error.Fail())
+ return error.takeError();
+
+ std::string connect_address =
+ llvm::StringRef(listen_socket_up->GetListeningConnectionURI()[0])
+ .split("://")
+ .second.str();
+
+ auto connect_socket_up = std::make_unique<TCPSocket>(true);
+ if (Status error = connect_socket_up->Connect(connect_address); error.Fail())
+ return error.takeError();
+
+ // Connection has already been made above, so a short timeout is sufficient.
+ Socket *accept_socket;
+ if (Status error =
+ listen_socket_up->Accept(std::chrono::seconds(1), accept_socket);
+ error.Fail())
+ return error.takeError();
+
+ return std::make_pair(
+ std::move(connect_socket_up),
+ std::unique_ptr<TCPSocket>(static_cast<TCPSocket *>(accept_socket)));
+}
+
bool TCPSocket::IsValid() const {
return m_socket != kInvalidSocketValue || m_listen_sockets.size() != 0;
}
diff --git a/lldb/source/Host/posix/DomainSocket.cpp b/lldb/source/Host/posix/DomainSocket.cpp
index 4f76e0c16d4c7..0202dea45a8e1 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -13,9 +13,11 @@
#endif
#include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include <cstddef>
+#include <fcntl.h>
#include <memory>
#include <sys/socket.h>
#include <sys/un.h>
@@ -76,6 +78,33 @@ DomainSocket::DomainSocket(SocketProtocol protocol, NativeSocket socket,
m_socket = socket;
}
+llvm::Expected<
+ std::pair<std::unique_ptr<DomainSocket>, std::unique_ptr<DomainSocket>>>
+DomainSocket::CreatePair() {
+ int sockets[2];
+ int type = SOCK_STREAM;
+#ifdef SOCK_CLOEXEC
+ type |= SOCK_CLOEXEC;
+#endif
+ if (socketpair(AF_UNIX, type, 0, sockets) == -1)
+ return llvm::errorCodeToError(llvm::errnoAsErrorCode());
+
+#ifndef SOCK_CLOEXEC
+ for (int s : sockets) {
+ int r = fcntl(s, F_SETFD, FD_CLOEXEC | fcntl(s, F_GETFD));
+ assert(r == 0);
+ (void)r;
+ }
+#endif
+
+ return std::make_pair(std::unique_ptr<DomainSocket>(
+ new DomainSocket(ProtocolUnixDomain, sockets[0],
+ /*should_close=*/true)),
+ std::unique_ptr<DomainSocket>(
+ new DomainSocket(ProtocolUnixDomain, sockets[1],
+ /*should_close=*/true)));
+}
+
Status DomainSocket::Connect(llvm::StringRef name) {
sockaddr_un saddr_un;
socklen_t saddr_un_len;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 2aea7c6b781d7..590d78e98f9a0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1141,34 +1141,14 @@ void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm); }
llvm::Error
GDBRemoteCommunication::ConnectLocally(GDBRemoteCommunication &client,
GDBRemoteCommunication &server) {
- const int backlog = 5;
- TCPSocket listen_socket(true);
- if (llvm::Error error =
- listen_socket.Listen("localhost:0", backlog).ToError())
- return error;
-
- llvm::SmallString<32> remote_addr;
- llvm::raw_svector_ostream(remote_addr)
- << "connect://localhost:" << listen_socket.GetLocalPortNumber();
-
- std::unique_ptr<ConnectionFileDescriptor> conn_up(
- new ConnectionFileDescriptor());
- Status status;
- if (conn_up->Connect(remote_addr, &status) != lldb::eConnectionStatusSuccess)
- return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "Unable to connect: %s", status.AsCString());
-
- // The connection was already established above, so a short timeout is
- // sufficient.
- Socket *accept_socket = nullptr;
- if (Status accept_status =
- listen_socket.Accept(std::chrono::seconds(1), accept_socket);
- accept_status.Fail())
- return accept_status.takeError();
-
- client.SetConnection(std::move(conn_up));
- server.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(accept_socket));
+ auto expected_socket_pair = Socket::CreatePair();
+ if (!expected_socket_pair)
+ return expected_socket_pair.takeError();
+
+ client.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ expected_socket_pair->first.release()));
+ server.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ expected_socket_pair->second.release()));
return llvm::Error::success();
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index f18bdd5175f2e..ba806767f78da 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3447,115 +3447,80 @@ ProcessGDBRemote::EstablishConnectionIfNeeded(const ProcessInfo &process_info) {
}
return error;
}
-#if !defined(_WIN32)
-#define USE_SOCKETPAIR_FOR_LOCAL_CONNECTION 1
-#endif
-
-#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
-static bool SetCloexecFlag(int fd) {
-#if defined(FD_CLOEXEC)
- int flags = ::fcntl(fd, F_GETFD);
- if (flags == -1)
- return false;
- return (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == 0);
-#else
- return false;
-#endif
-}
-#endif
Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
const ProcessInfo &process_info) {
using namespace std::placeholders; // For _1, _2, etc.
- Status error;
- if (m_debugserver_pid == LLDB_INVALID_PROCESS_ID) {
- // If we locate debugserver, keep that located version around
- static FileSpec g_debugserver_file_spec;
+ if (m_debugserver_pid != LLDB_INVALID_PROCESS_ID)
+ return Status();
- ProcessLaunchInfo debugserver_launch_info;
- // Make debugserver run in its own session so signals generated by special
- // terminal key sequences (^C) don't affect debugserver.
- debugserver_launch_info.SetLaunchInSeparateProcessGroup(true);
+ ProcessLaunchInfo debugserver_launch_info;
+ // Make debugserver run in its own session so signals generated by special
+ // terminal key sequences (^C) don't affect debugserver.
+ debugserver_launch_info.SetLaunchInSeparateProcessGroup(true);
- const std::weak_ptr<ProcessGDBRemote> this_wp =
- std::static_pointer_cast<ProcessGDBRemote>(shared_from_this());
- debugserver_launch_info.SetMonitorProcessCallback(
- std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3));
- debugserver_launch_info.SetUserID(process_info.GetUserID());
+ const std::weak_ptr<ProcessGDBRemote> this_wp =
+ std::static_pointer_cast<ProcessGDBRemote>(shared_from_this());
+ debugserver_launch_info.SetMonitorProcessCallback(
+ std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3));
+ debugserver_launch_info.SetUserID(process_info.GetUserID());
#if defined(__APPLE__)
- // On macOS 11, we need to support x86_64 applications translated to
- // arm64. We check whether a binary is translated and spawn the correct
- // debugserver accordingly.
- int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
- static_cast<int>(process_info.GetProcessID()) };
- struct kinfo_proc processInfo;
- size_t bufsize = sizeof(processInfo);
- if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), &processInfo,
- &bufsize, NULL, 0) == 0 && bufsize > 0) {
- if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
- FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
- debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
- }
+ // On macOS 11, we need to support x86_64 applications translated to
+ // arm64. We check whether a binary is translated and spawn the correct
+ // debugserver accordingly.
+ int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID,
+ static_cast<int>(process_info.GetProcessID())};
+ struct kinfo_proc processInfo;
+ size_t bufsize = sizeof(processInfo);
+ if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), &processInfo, &bufsize,
+ NULL, 0) == 0 &&
+ bufsize > 0) {
+ if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
+ FileSpec rosetta_debugserver(
+ "/Library/Apple/usr/libexec/oah/debugserver");
+ debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
}
+ }
#endif
- shared_fd_t communication_fd = SharedSocket::kInvalidFD;
-#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
- // Use a socketpair on non-Windows systems for security and performance
- // reasons.
- int sockets[2]; /* the pair of socket descriptors */
- if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) {
- error = Status::FromErrno();
- return error;
- }
+ auto expected_socket_pair = Socket::CreatePair();
+ if (!expected_socket_pair)
+ return Status::FromError(expected_socket_pair.takeError());
- int our_socket = sockets[0];
- int gdb_socket = sockets[1];
- auto cleanup_our = llvm::make_scope_exit([&]() { close(our_socket); });
- auto cleanup_gdb = llvm::make_scope_exit([&]() { close(gdb_socket); });
+ Status error;
+ SharedSocket shared_socket(expected_socket_pair->first.get(), error);
+ if (error.Fail())
+ return error;
- // Don't let any child processes inherit our communication socket
- SetCloexecFlag(our_socket);
- communication_fd = gdb_socket;
-#endif
+ error = m_gdb_comm.StartDebugserverProcess(
+ nullptr, GetTarget().GetPlatform().get(), debugserver_launch_info,
+ nullptr, nullptr, shared_socket.GetSendableFD());
- error = m_gdb_comm.StartDebugserverProcess(
- nullptr, GetTarget().GetPlatform().get(), debugserver_launch_info,
- nullptr, nullptr, communication_fd);
+ if (error.Fail()) {
+ Log *log = GetLog(GDBRLog::Process);
- if (error.Success())
- m_debugserver_pid = debugserver_launch_info.GetProcessID();
- else
- m_debugserver_pid = LLDB_INVALID_PROCESS_ID;
-
- if (m_debugserver_pid != LLDB_INVALID_PROCESS_ID) {
-#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
- // Our process spawned correctly, we can now set our connection to use
- // our end of the socket pair
- cleanup_our.release();
- m_gdb_comm.SetConnection(
- std::make_unique<ConnectionFileDescriptor>(our_socket, true));
-#endif
- StartAsyncThread();
- }
+ LLDB_LOGF(log, "failed to start debugserver process: %s",
+ error.AsCString());
+ return error;
+ }
- if (error.Fail()) {
- Log *log = GetLog(GDBRLog::Process);
+ m_debugserver_pid = debugserver_launch_info.GetProcessID();
+ shared_socket.CompleteSending(m_debugserver_pid);
- LLDB_LOGF(log, "failed to start debugserver process: %s",
- error.AsCString());
- return error;
- }
+ // Our process spawned correctly, we can now set our connection to use
+ // our end of the socket pair
+ m_gdb_comm.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ expected_socket_pair->second.release()));
+ StartAsyncThread();
- if (m_gdb_comm.IsConnected()) {
- // Finish the connection process by doing the handshake without
- // connecting (send NULL URL)
- error = ConnectToDebugserver("");
- } else {
- error = Status::FromErrorString("connection failed");
- }
+ if (m_gdb_comm.IsConnected()) {
+ // Finish the connection process by doing the handshake without
+ // connecting (send NULL URL)
+ error = ConnectToDebugserver("");
+ } else {
+ error = Status::FromErrorString("connection failed");
}
return error;
}
diff --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp
index df9ff089a0d73..369d1534ae32a 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -7,14 +7,14 @@
//===----------------------------------------------------------------------===//
#include "lldb/Core/Communication.h"
+#include "TestingSupport/SubsystemRAII.h"
#include "lldb/Core/ThreadedCommunication.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/Pipe.h"
+#include "lldb/Host/Socket.h"
#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
-#include "TestingSupport/Host/SocketTestUtilities.h"
-#include "TestingSupport/SubsystemRAII.h"
#include <chrono>
#include <thread>
@@ -31,15 +31,17 @@ class CommunicationTest : public testing::Test {
};
static void CommunicationReadTest(bool use_read_thread) {
- std::unique_ptr<TCPSocket> a, b;
- ASSERT_TRUE(CreateTCPConnectedSockets("localhost", &a, &b));
+ auto maybe_socket_pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(maybe_socket_pair, llvm::Succeeded());
+ Socket &a = *maybe_socket_pair->first;
size_t num_bytes = 4;
- ASSERT_THAT_ERROR(a->Write("test", num_bytes).ToError(), llvm::Succeeded());
+ ASSERT_THAT_ERROR(a.Write("test", num_bytes).ToError(), llvm::Succeeded());
ASSERT_EQ(num_bytes, 4U);
ThreadedCommunication comm("test");
- comm.SetConnection(std::make_unique<ConnectionFileDescriptor>(b.release()));
+ comm.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ maybe_socket_pair->second.release()));
comm.SetCloseOnEOF(true);
if (use_read_thread) {
@@ -73,7 +75,7 @@ static void CommunicationReadTest(bool use_read_thread) {
EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
// This read should return EOF.
- ASSERT_THAT_ERROR(a->Close().ToError(), llvm::Succeeded());
+ ASSERT_THAT_ERROR(a.Close().ToError(), llvm::Succeeded());
error.Clear();
EXPECT_EQ(
comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U);
@@ -118,17 +120,19 @@ TEST_F(CommunicationTest, ReadThread) {
}
TEST_F(CommunicationTest, SynchronizeWhileClosing) {
- std::unique_ptr<TCPSocket> a, b;
- ASSERT_TRUE(CreateTCPConnectedSockets("localhost", &a, &b));
+ auto maybe_socket_pair = Socket::CreatePair();
+ ASSERT_THAT_EXPECTED(maybe_socket_pair, llvm::Succeeded());
+ Socket &a = *maybe_socket_pair->first;
ThreadedCommunication comm("test");
- comm.SetConnection(std::make_unique<ConnectionFileDescriptor>(b.release()));
+ comm.SetConnection(std::make_unique<ConnectionFileDescriptor>(
+ maybe_socket_pair->second.release()));
comm.SetCloseOnEOF(true);
ASSERT_TRUE(comm.StartReadThread());
// Ensure that we can safely synchronize with the read thread while it is
// closing the read end (in response to us closing the write end).
- ASSERT_THAT_ERROR(a->Close().ToError(), llvm::Succeeded());
+ ASSERT_THAT_ERROR(a.Close().ToError(), llvm::Succeeded());
comm.SynchronizeWithReadThread();
ASSERT_TRUE(comm.StopReadThread());
diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
index 77366593f05f8..3630b63242707 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -74,6 +74,41 @@ TEST_F(SocketTest, DecodeHostAndPort) {
llvm::HasValue(Socket::HostAndPort{"abcd:12fg:AF58::1", 12345}));
}
+TEST_F(SocketTest, CreatePair) {
+ std::vector<std::optional<Socket::SocketProtocol>> functional_protocols = {
+ std::nullopt,
+ Socket::ProtocolTcp,
+#if LLDB_ENABLE_POSIX
+ Socket::ProtocolUnixDomain,
+ Socket::ProtocolUnixAbstract,
+#endif
+ };
+ for (auto p : functional_protocols) {
+ auto expected_socket_pair = Socket::CreatePair(p);
+ ASSERT_THAT_EXPECTED(expected_socket_pair, llvm::Succeeded());
+ Socket &a = *expected_socket_pair->first;
+ Socket &b = *expected_socket_pair->second;
+ size_t num_bytes = 1;
+ ASSERT_THAT_ERROR(a.Write("a", num_bytes).takeError(), llvm::Succeeded());
+ ASSERT_EQ(num_bytes, 1);
+ char c;
+ ASSERT_THAT_ERROR(b.Read(&c, num_bytes).takeError(), llvm::Succeeded());
+ ASSERT_EQ(num_bytes, 1);
+ ASSERT_EQ(c, 'a');
+ }
+
+ std::vector<Socket::SocketProtocol> erroring_protocols = {
+#if !LLDB_ENABLE_POSIX
+ Socket::ProtocolUnixDomain,
+ Socket::ProtocolUnixAbstract,
+#endif
+ };
+ for (auto p : erroring_protocols) {
+ ASSERT_THAT_EXPECTED(Socket::CreatePair(p),
+ llvm::FailedWithMessage("Unsupported protocol"));
+ }
+}
+
#if LLDB_ENABLE_POSIX
TEST_F(SocketTest, DomainListenConnectAccept) {
llvm::SmallString<64> Path;
|
labath
added a commit
to labath/llvm-project
that referenced
this pull request
Jun 20, 2025
The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the "null url *and* null comm_fd" which is not used as of llvm#145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object.
slydiman
approved these changes
Jun 21, 2025
This lets get rid of platform-specific code in ProcessGDBRemote and use the same code path (module differences in socket types) everywhere. It also unlocks further cleanups in the debugserver launching code.
Thanks. Good point, updated the PR description. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/25174 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/181/builds/22355 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/21490 Here is the relevant piece of the build log for the reference |
labath
added a commit
that referenced
this pull request
Jun 24, 2025
Mid-flight collision with #145293.
labath
added a commit
to labath/llvm-project
that referenced
this pull request
Jun 24, 2025
The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the "null url *and* null comm_fd" which is not used as of llvm#145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/31036 Here is the relevant piece of the build log for the reference |
DrSergei
pushed a commit
to DrSergei/llvm-project
that referenced
this pull request
Jun 24, 2025
This lets get rid of platform-specific code in ProcessGDBRemote and use the same code path (module differences in socket types) everywhere. It also unlocks further cleanups in the debugserver launching code. The main effect of this change is that lldb on windows will now use the `--fd` lldb-server argument for "local remote" debug sessions instead of having lldb-server connect back to lldb. This is the same method used by lldb on non-windows platforms (for many years) and "lldb-server platform" on windows for truly remote debug sessions (for ~one year). Depends on llvm#145015.
DrSergei
pushed a commit
to DrSergei/llvm-project
that referenced
this pull request
Jun 24, 2025
Mid-flight collision with llvm#145293.
labath
added a commit
that referenced
this pull request
Jun 25, 2025
) The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the (surprising) "null url *and* null comm_fd" code path which is not used as of #145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object. Depends on #145017
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 25, 2025
…ocess (#145021) The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the (surprising) "null url *and* null comm_fd" code path which is not used as of llvm/llvm-project#145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object. Depends on #145017
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
This lets get rid of platform-specific code in ProcessGDBRemote and use the same code path (module differences in socket types) everywhere. It also unlocks further cleanups in the debugserver launching code. The main effect of this change is that lldb on windows will now use the `--fd` lldb-server argument for "local remote" debug sessions instead of having lldb-server connect back to lldb. This is the same method used by lldb on non-windows platforms (for many years) and "lldb-server platform" on windows for truly remote debug sessions (for ~one year). Depends on llvm#145015.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
Mid-flight collision with llvm#145293.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
…#145021) The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the (surprising) "null url *and* null comm_fd" code path which is not used as of llvm#145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object. Depends on llvm#145017
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
…#145021) The function was extremely messy in that it, depending on the set of arguments, it could either modify the Connection object in `this` or not. It had a lot of arguments, with each call site passing a different combination of null values. This PR: - packs "url" and "comm_fd" arguments into a variant as they are mutually exclusive - removes the (surprising) "null url *and* null comm_fd" code path which is not used as of llvm#145017 - marks the function as `static` to make it clear it (now) does not operate on the `this` object. Depends on llvm#145017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This lets get rid of platform-specific code in ProcessGDBRemote and use the
same code path (module differences in socket types) everywhere. It also unlocks
further cleanups in the debugserver launching code.
The main effect of this change is that lldb on windows will now use the
--fdlldb-server argument for "local remote" debug sessions instead of having lldb-server connect back to lldb. This is the same method used by lldb on non-windows platforms (for many years) and "lldb-server platform" on windows for truly remote debug sessions (for ~one year).Depends on #145015.