From 24438aa4886dc32de58fef0fe18321caf619ed28 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 24 Jun 2025 12:39:24 +0200 Subject: [PATCH] [lldb] Use Socket::CreatePair for launching debugserver (#145017) 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 #145015. --- .../Process/gdb-remote/ProcessGDBRemote.cpp | 145 +++++++----------- 1 file changed, 55 insertions(+), 90 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index f18bdd5175f2..4e3569a5e798 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 this_wp = - std::static_pointer_cast(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 this_wp = + std::static_pointer_cast(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(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(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; - } + llvm::Expected socket_pair = Socket::CreatePair(); + if (!socket_pair) + return Status::FromError(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(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; + LLDB_LOGF(log, "failed to start debugserver process: %s", + error.AsCString()); + return error; + } - 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(our_socket, true)); -#endif - StartAsyncThread(); - } + m_debugserver_pid = debugserver_launch_info.GetProcessID(); + shared_socket.CompleteSending(m_debugserver_pid); - if (error.Fail()) { - Log *log = GetLog(GDBRLog::Process); + // 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( + socket_pair->second.release())); + StartAsyncThread(); - LLDB_LOGF(log, "failed to start debugserver process: %s", - error.AsCString()); - return error; - } - - 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; }