https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283
>From 6b2a41ba3d71270e81e24a42d3b4f5dc2f96b939 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH 1/5] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Depends on #101383. Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/tools/lldb-server/lldb-platform.cpp | 347 ++++++++++++++++++++--- 1 file changed, 306 insertions(+), 41 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 7148a1d2a30941..e23237ef574c30 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -22,6 +22,7 @@ #include <optional> #include "llvm/Support/FileSystem.h" +#include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" @@ -32,8 +33,10 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostGetOpt.h" #include "lldb/Host/OptionParser.h" +#include "lldb/Host/Socket.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" using namespace lldb; @@ -60,6 +63,9 @@ static struct option g_long_options[] = { {"max-gdbserver-port", required_argument, nullptr, 'M'}, {"socket-file", required_argument, nullptr, 'f'}, {"server", no_argument, &g_server, 1}, +#if defined(_WIN32) + {"accept", required_argument, nullptr, 'a'}, +#endif {nullptr, 0, nullptr, 0}}; #if defined(__APPLE__) @@ -114,6 +120,249 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static void client_handle(GDBRemoteCommunicationServerPlatform &platform, + const lldb_private::Args &args) { + if (!platform.IsConnected()) + return; + + if (args.GetArgumentCount() > 0) { + lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; + std::optional<uint16_t> port; + std::string socket_name; + Status error = platform.LaunchGDBServer(args, + "", // hostname + pid, port, socket_name); + if (error.Success()) + platform.SetPendingGdbServer(pid, *port, socket_name); + else + fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { + if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, + done) != + GDBRemoteCommunication::PacketResult::Success) + break; + } + + if (error.Fail()) + WithColor::error() << error.AsCString() << '\n'; +} + +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) +static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePortForProcess(pid); +} + +static bool spawn_process_parent(const char *progname, Connection *conn, + uint16_t gdb_port, uint16_t port_offset, + const lldb_private::Args &args, + const std::string &log_file, + const StringRef log_channels) { + Log *log = GetLog(LLDBLog::Platform); + Pipe socket_pipe; + Status error = socket_pipe.CreateNew(true); + if (error.Fail()) { + LLDB_LOGF(log, + "lldb-platform parent: " + "cannot create pipe: %s", + error.AsCString()); + return false; + } + + ProcessLaunchInfo launch_info; + FileSpec self_spec(progname, FileSpec::Style::native); + launch_info.SetExecutableFile(self_spec, true); + Args &self_args = launch_info.GetArguments(); + self_args.AppendArgument(llvm::StringRef("platform")); + self_args.AppendArgument(llvm::StringRef("--accept")); + self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe())); + if (gdb_port) { + self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); + self_args.AppendArgument(llvm::to_string(gdb_port)); + } + if (port_offset > 0) { + self_args.AppendArgument(llvm::StringRef("--port-offset")); + self_args.AppendArgument(llvm::to_string(port_offset)); + } + if (!log_file.empty()) { + self_args.AppendArgument(llvm::StringRef("--log-file")); + self_args.AppendArgument(log_file); + } + if (!log_channels.empty()) { + self_args.AppendArgument(llvm::StringRef("--log-channels")); + self_args.AppendArgument(log_channels); + } + if (args.GetArgumentCount() > 0) { + self_args.AppendArgument("--"); + self_args.AppendArguments(args); + } + + launch_info.SetLaunchInSeparateProcessGroup(false); + launch_info.SetMonitorProcessCallback(&spawn_process_reaped); + + // Copy the current environment. + // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process + // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing + // in the environment. + launch_info.GetEnvironment() = Host::GetEnvironment(); + + launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); + + launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor()); + + // Close STDIN, STDOUT and STDERR. + launch_info.AppendCloseFileAction(STDIN_FILENO); + launch_info.AppendCloseFileAction(STDOUT_FILENO); + launch_info.AppendCloseFileAction(STDERR_FILENO); + + // Redirect STDIN, STDOUT and STDERR to "/dev/null". + launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false); + launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true); + launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true); + + std::string cmd; + self_args.GetCommandString(cmd); + + error = Host::LaunchProcess(launch_info); + if (error.Fail()) { + LLDB_LOGF(log, + "lldb-platform parent: " + "cannot launch child process for connection: %s", + error.AsCString()); + return false; + } + + lldb::pid_t childPid = launch_info.GetProcessID(); + if (childPid == LLDB_INVALID_PROCESS_ID) { + LLDB_LOGF(log, "lldb-platform parent: " + "cannot launch child process for connection: invalid pid"); + return false; + } + LLDB_LOGF(log, + "lldb-platform parent: " + "launched '%s', pid=0x%x", + cmd.c_str(), childPid); + + { + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + gdbserver_portmap.AssociatePortWithProcess(gdb_port, childPid); + } + + if (socket_pipe.CanRead()) + socket_pipe.CloseReadFileDescriptor(); + if (!socket_pipe.CanWrite()) { + LLDB_LOGF(log, "lldb-platform parent: " + "cannot write to socket_pipe"); + Host::Kill(childPid, SIGTERM); + return false; + } + + const TCPSocket &socket = + static_cast<const TCPSocket &>(*conn->GetReadObject()); + NativeSocket nativeSocket = socket.GetNativeSocket(); + + WSAPROTOCOL_INFO protocol_info; + if (::WSADuplicateSocket(nativeSocket, childPid, &protocol_info) == + SOCKET_ERROR) { + LLDB_LOGF(log, + "lldb-platform parent: " + "WSADuplicateSocket() failed, error: %d", + ::WSAGetLastError()); + Host::Kill(childPid, SIGTERM); + return false; + } + + size_t num_bytes; + error = socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(2), num_bytes); + if (error.Fail()) { + LLDB_LOGF(log, + "lldb-platform parent: " + "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %s", + error.AsCString()); + Host::Kill(childPid, SIGTERM); + return false; + } + if (num_bytes != sizeof(protocol_info)) { + LLDB_LOGF(log, + "lldb-platform parent: " + "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); + Host::Kill(childPid, SIGTERM); + return false; + } + + socket_pipe.Close(); + + return true; +} + +static bool spawn_process_child(pipe_t accept_fd, + const std::string &listen_host_port, + uint16_t port_offset, + const lldb_private::Args &args) { + if (accept_fd == INVALID_HANDLE_VALUE) + return false; + + Log *log = GetLog(LLDBLog::Platform); + if (!listen_host_port.empty()) { + LLDB_LOGF(log, "lldb-platform child: " + "ambiguous parameters --listen and --accept"); + exit(SOCKET_ERROR); + } + + Pipe socket_pipe(accept_fd, LLDB_INVALID_PIPE); + + WSAPROTOCOL_INFO protocol_info; + size_t num_bytes; + Status error = + socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(2), num_bytes); + if (error.Fail()) { + LLDB_LOGF(log, + "lldb-platform child: " + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %s", + error.AsCString()); + exit(SOCKET_ERROR); + } + if (num_bytes != sizeof(protocol_info)) { + LLDB_LOGF(log, + "lldb-platform child: " + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); + exit(SOCKET_ERROR); + } + + NativeSocket socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, + FROM_PROTOCOL_INFO, &protocol_info, 0, 0); + if (socket == INVALID_SOCKET) { + LLDB_LOGF(log, + "lldb-platform child: " + "WSASocket(FROM_PROTOCOL_INFO) failed: error %d", + ::WSAGetLastError()); + exit(SOCKET_ERROR); + } + + Connection *conn = + new ConnectionFileDescriptor(new TCPSocket(socket, true, false)); + GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp"); + if (port_offset > 0) + platform.SetPortOffset(port_offset); + platform.SetPortMap(std::move(gdbserver_portmap)); + platform.SetConnection(std::unique_ptr<Connection>(conn)); + client_handle(platform, args); + return true; +} +#endif + // main int main_platform(int argc, char *argv[]) { const char *progname = argv[0]; @@ -123,6 +372,8 @@ int main_platform(int argc, char *argv[]) { #if !defined(_WIN32) signal(SIGPIPE, SIG_IGN); signal(SIGHUP, signal_handler); +#else + pipe_t accept_fd = INVALID_HANDLE_VALUE; #endif int long_option_index = 0; Status error; @@ -133,7 +384,6 @@ int main_platform(int argc, char *argv[]) { StringRef log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" - GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; int min_gdbserver_port = 0; int max_gdbserver_port = 0; uint16_t port_offset = 0; @@ -217,6 +467,17 @@ int main_platform(int argc, char *argv[]) { max_gdbserver_port = portnum; } break; +#if defined(_WIN32) + case 'a': { + uint64_t fd; + if (!llvm::to_integer(optarg, fd)) { + WithColor::error() << "invalid accept fd " << optarg << "\n"; + option_error = 6; + } else + accept_fd = (pipe_t)fd; + } break; +#endif + case 'h': /* fall-through is intentional */ case '?': show_usage = true; @@ -240,7 +501,11 @@ int main_platform(int argc, char *argv[]) { } // Print usage and exit if no listening port is specified. +#if !defined(_WIN32) if (listen_host_port.empty()) +#else + if (listen_host_port.empty() && accept_fd == INVALID_HANDLE_VALUE) +#endif show_usage = true; if (show_usage || option_error) { @@ -254,6 +519,13 @@ int main_platform(int argc, char *argv[]) { lldb_private::Args inferior_arguments; inferior_arguments.SetArguments(argc, const_cast<const char **>(argv)); +#if defined(_WIN32) + // Child process will handle the connection and exit. + if (spawn_process_child(accept_fd, listen_host_port, port_offset, + inferior_arguments)) + return 0; +#endif + const bool children_inherit_listen_socket = false; // the test suite makes many connections in parallel, let's not miss any. // The highest this should get reasonably is a function of the number @@ -303,32 +575,39 @@ int main_platform(int argc, char *argv[]) { ::pid_t waitResult; while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) { // waitResult is the child pid + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); gdbserver_portmap.FreePortForProcess(waitResult); } #endif - // TODO: Clean up portmap for Windows when children die - // See https://github.com/llvm/llvm-project/issues/90923 - // After collecting zombie ports, get the next available - GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; - llvm::Expected<uint16_t> available_port = - gdbserver_portmap.GetNextAvailablePort(); - if (available_port) { - // GetNextAvailablePort() may return 0 if gdbserver_portmap is empty. - if (*available_port) - portmap_for_child.AllowPort(*available_port); - } else { - llvm::consumeError(available_port.takeError()); + std::optional<uint16_t> available_port; + { + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + auto port = gdbserver_portmap.GetNextAvailablePort(); + if (port) + available_port = *port; + else + llvm::consumeError(port.takeError()); + } + if (!available_port) { fprintf(stderr, "no available gdbserver port for connection - dropping...\n"); delete conn; continue; } +#if !defined(_WIN32) + GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; + // GetNextAvailablePort() may return 0 if gdbserver_portmap is empty. + if (*available_port) + portmap_for_child.AllowPort(*available_port); platform.SetPortMap(std::move(portmap_for_child)); auto childPid = fork(); if (childPid) { - gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid); + { + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid); + } // Parent doesn't need a connection to the lldb client delete conn; @@ -340,6 +619,18 @@ int main_platform(int argc, char *argv[]) { // Listening socket is owned by parent process. acceptor_up.release(); } +#else + if (!spawn_process_parent(progname, conn, *available_port, port_offset, + inferior_arguments, log_file, log_channels)) { + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePort(*available_port); + } + // Parent doesn't need a connection to the lldb client + delete conn; + + // Parent will continue to listen for new connections. + continue; +#endif } else { // If not running as a server, this process will not accept // connections while a connection is active. @@ -350,33 +641,7 @@ int main_platform(int argc, char *argv[]) { } platform.SetConnection(std::unique_ptr<Connection>(conn)); - - if (platform.IsConnected()) { - if (inferior_arguments.GetArgumentCount() > 0) { - lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - std::optional<uint16_t> port; - std::string socket_name; - Status error = platform.LaunchGDBServer(inferior_arguments, - "", // hostname - pid, port, socket_name); - if (error.Success()) - platform.SetPendingGdbServer(pid, *port, socket_name); - else - fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); - } - - bool interrupt = false; - bool done = false; - while (!interrupt && !done) { - if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, - done) != - GDBRemoteCommunication::PacketResult::Success) - break; - } - - if (error.Fail()) - WithColor::error() << error.AsCString() << '\n'; - } + client_handle(platform, inferior_arguments); } while (g_server); fprintf(stderr, "lldb-server exiting...\n"); >From b2a00ab77b05289a5536e0692430542588f547d0 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Tue, 6 Aug 2024 16:22:36 +0400 Subject: [PATCH 2/5] [lldb] Updated lldb-server to spawn the child process and share socket `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/tools/lldb-server/lldb-platform.cpp | 325 ++++++++++------------- 1 file changed, 142 insertions(+), 183 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index e23237ef574c30..388a033906f722 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -47,6 +47,14 @@ using namespace llvm; // option descriptors for getopt_long_only() +#ifdef _WIN32 +typedef pipe_t fd_t; +#define kInvalidFD (LLDB_INVALID_PIPE) +#else +typedef NativeSocket fd_t; +#define kInvalidFD (Socket::kInvalidSocketValue) +#endif + static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; @@ -63,9 +71,7 @@ static struct option g_long_options[] = { {"max-gdbserver-port", required_argument, nullptr, 'M'}, {"socket-file", required_argument, nullptr, 'f'}, {"server", no_argument, &g_server, 1}, -#if defined(_WIN32) - {"accept", required_argument, nullptr, 'a'}, -#endif + {"fd", required_argument, nullptr, 'd'}, {nullptr, 0, nullptr, 0}}; #if defined(__APPLE__) @@ -148,42 +154,52 @@ static void client_handle(GDBRemoteCommunicationServerPlatform &platform, break; } - if (error.Fail()) - WithColor::error() << error.AsCString() << '\n'; + printf("Disconnected.\n"); } static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; static std::mutex gdbserver_portmap_mutex; -#if defined(_WIN32) static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); gdbserver_portmap.FreePortForProcess(pid); } -static bool spawn_process_parent(const char *progname, Connection *conn, - uint16_t gdb_port, uint16_t port_offset, - const lldb_private::Args &args, - const std::string &log_file, - const StringRef log_channels) { - Log *log = GetLog(LLDBLog::Platform); - Pipe socket_pipe; - Status error = socket_pipe.CreateNew(true); - if (error.Fail()) { - LLDB_LOGF(log, - "lldb-platform parent: " - "cannot create pipe: %s", - error.AsCString()); - return false; - } +static Status spawn_process(const char *progname, Connection *conn, + uint16_t gdb_port, uint16_t port_offset, + const lldb_private::Args &args, + const std::string &log_file, + const StringRef log_channels) { + Status error; + const TCPSocket &tcpSocket = + static_cast<const TCPSocket &>(*conn->GetReadObject()); + NativeSocket socket = tcpSocket.GetNativeSocket(); ProcessLaunchInfo launch_info; + + fd_t fd; +#ifdef _WIN32 + // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. + Pipe socket_pipe; + error = socket_pipe.CreateNew(true); + if (error.Fail()) + return error; + + // Seems it will not work anyway. ProcessLauncherWindows ignores FileActions. + // And it is necessary to pass HANDLE instead of FD on Widows. + launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor()); + + fd = socket_pipe.GetReadPipe(); +#else + fd = socket; +#endif + FileSpec self_spec(progname, FileSpec::Style::native); launch_info.SetExecutableFile(self_spec, true); Args &self_args = launch_info.GetArguments(); self_args.AppendArgument(llvm::StringRef("platform")); - self_args.AppendArgument(llvm::StringRef("--accept")); - self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe())); + self_args.AppendArgument(llvm::StringRef("--fd")); + self_args.AppendArgument(llvm::to_string(fd)); if (gdb_port) { self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); self_args.AppendArgument(llvm::to_string(gdb_port)); @@ -216,8 +232,6 @@ static bool spawn_process_parent(const char *progname, Connection *conn, launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); - launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor()); - // Close STDIN, STDOUT and STDERR. launch_info.AppendCloseFileAction(STDIN_FILENO); launch_info.AppendCloseFileAction(STDOUT_FILENO); @@ -232,136 +246,54 @@ static bool spawn_process_parent(const char *progname, Connection *conn, self_args.GetCommandString(cmd); error = Host::LaunchProcess(launch_info); - if (error.Fail()) { - LLDB_LOGF(log, - "lldb-platform parent: " - "cannot launch child process for connection: %s", - error.AsCString()); - return false; - } + if (error.Fail()) + return error; - lldb::pid_t childPid = launch_info.GetProcessID(); - if (childPid == LLDB_INVALID_PROCESS_ID) { - LLDB_LOGF(log, "lldb-platform parent: " - "cannot launch child process for connection: invalid pid"); - return false; - } - LLDB_LOGF(log, - "lldb-platform parent: " - "launched '%s', pid=0x%x", - cmd.c_str(), childPid); + lldb::pid_t child_pid = launch_info.GetProcessID(); + if (child_pid == LLDB_INVALID_PROCESS_ID) + return Status("invalid pid"); + + LLDB_LOG(GetLog(LLDBLog::Platform), "lldb-platform launched '{0}', pid={1}", + cmd, child_pid); { std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.AssociatePortWithProcess(gdb_port, childPid); + gdbserver_portmap.AssociatePortWithProcess(gdb_port, child_pid); } +#ifdef _WIN32 + // Transfer WSAPROTOCOL_INFO to the child process. if (socket_pipe.CanRead()) socket_pipe.CloseReadFileDescriptor(); if (!socket_pipe.CanWrite()) { - LLDB_LOGF(log, "lldb-platform parent: " - "cannot write to socket_pipe"); - Host::Kill(childPid, SIGTERM); - return false; + Host::Kill(child_pid, SIGTERM); + return Status("cannot write to socket_pipe"); } - const TCPSocket &socket = - static_cast<const TCPSocket &>(*conn->GetReadObject()); - NativeSocket nativeSocket = socket.GetNativeSocket(); - WSAPROTOCOL_INFO protocol_info; - if (::WSADuplicateSocket(nativeSocket, childPid, &protocol_info) == - SOCKET_ERROR) { - LLDB_LOGF(log, - "lldb-platform parent: " - "WSADuplicateSocket() failed, error: %d", - ::WSAGetLastError()); - Host::Kill(childPid, SIGTERM); - return false; + if (::WSADuplicateSocket(socket, child_pid, &protocol_info) == SOCKET_ERROR) { + int last_error = ::WSAGetLastError(); + Host::Kill(child_pid, SIGTERM); + return Status("WSADuplicateSocket() failed, error: %d", last_error); } size_t num_bytes; error = socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), std::chrono::seconds(2), num_bytes); if (error.Fail()) { - LLDB_LOGF(log, - "lldb-platform parent: " - "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %s", - error.AsCString()); - Host::Kill(childPid, SIGTERM); - return false; + Host::Kill(child_pid, SIGTERM); + return error; } if (num_bytes != sizeof(protocol_info)) { - LLDB_LOGF(log, - "lldb-platform parent: " - "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", - num_bytes); - Host::Kill(childPid, SIGTERM); - return false; - } - - socket_pipe.Close(); - - return true; -} - -static bool spawn_process_child(pipe_t accept_fd, - const std::string &listen_host_port, - uint16_t port_offset, - const lldb_private::Args &args) { - if (accept_fd == INVALID_HANDLE_VALUE) - return false; - - Log *log = GetLog(LLDBLog::Platform); - if (!listen_host_port.empty()) { - LLDB_LOGF(log, "lldb-platform child: " - "ambiguous parameters --listen and --accept"); - exit(SOCKET_ERROR); - } - - Pipe socket_pipe(accept_fd, LLDB_INVALID_PIPE); - - WSAPROTOCOL_INFO protocol_info; - size_t num_bytes; - Status error = - socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(2), num_bytes); - if (error.Fail()) { - LLDB_LOGF(log, - "lldb-platform child: " - "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %s", - error.AsCString()); - exit(SOCKET_ERROR); - } - if (num_bytes != sizeof(protocol_info)) { - LLDB_LOGF(log, - "lldb-platform child: " - "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", - num_bytes); - exit(SOCKET_ERROR); - } - - NativeSocket socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, - FROM_PROTOCOL_INFO, &protocol_info, 0, 0); - if (socket == INVALID_SOCKET) { - LLDB_LOGF(log, - "lldb-platform child: " - "WSASocket(FROM_PROTOCOL_INFO) failed: error %d", - ::WSAGetLastError()); - exit(SOCKET_ERROR); + Host::Kill(child_pid, SIGTERM); + return Status( + "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); } +#endif - Connection *conn = - new ConnectionFileDescriptor(new TCPSocket(socket, true, false)); - GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp"); - if (port_offset > 0) - platform.SetPortOffset(port_offset); - platform.SetPortMap(std::move(gdbserver_portmap)); - platform.SetConnection(std::unique_ptr<Connection>(conn)); - client_handle(platform, args); - return true; + return Status(); } -#endif // main int main_platform(int argc, char *argv[]) { @@ -372,8 +304,6 @@ int main_platform(int argc, char *argv[]) { #if !defined(_WIN32) signal(SIGPIPE, SIG_IGN); signal(SIGHUP, signal_handler); -#else - pipe_t accept_fd = INVALID_HANDLE_VALUE; #endif int long_option_index = 0; Status error; @@ -384,6 +314,8 @@ int main_platform(int argc, char *argv[]) { StringRef log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" + fd_t fd = kInvalidFD; + int min_gdbserver_port = 0; int max_gdbserver_port = 0; uint16_t port_offset = 0; @@ -467,16 +399,14 @@ int main_platform(int argc, char *argv[]) { max_gdbserver_port = portnum; } break; -#if defined(_WIN32) - case 'a': { - uint64_t fd; - if (!llvm::to_integer(optarg, fd)) { - WithColor::error() << "invalid accept fd " << optarg << "\n"; + case 'd': { + uint64_t _fd; + if (!llvm::to_integer(optarg, _fd)) { + WithColor::error() << "invalid fd " << optarg << "\n"; option_error = 6; } else - accept_fd = (pipe_t)fd; + fd = (fd_t)_fd; } break; -#endif case 'h': /* fall-through is intentional */ case '?': @@ -501,11 +431,7 @@ int main_platform(int argc, char *argv[]) { } // Print usage and exit if no listening port is specified. -#if !defined(_WIN32) - if (listen_host_port.empty()) -#else - if (listen_host_port.empty() && accept_fd == INVALID_HANDLE_VALUE) -#endif + if (listen_host_port.empty() && fd == kInvalidFD) show_usage = true; if (show_usage || option_error) { @@ -519,13 +445,65 @@ int main_platform(int argc, char *argv[]) { lldb_private::Args inferior_arguments; inferior_arguments.SetArguments(argc, const_cast<const char **>(argv)); -#if defined(_WIN32) - // Child process will handle the connection and exit. - if (spawn_process_child(accept_fd, listen_host_port, port_offset, - inferior_arguments)) - return 0; + if (fd != kInvalidFD) { + // Child process will handle the connection and exit. + Log *log = GetLog(LLDBLog::Platform); + if (!listen_host_port.empty()) { + LLDB_LOGF(log, "lldb-platform child: " + "ambiguous parameters --listen and --fd"); + return socket_error; + } + + NativeSocket socket; +#ifdef _WIN32 + // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. + WSAPROTOCOL_INFO protocol_info; + { + Pipe socket_pipe(fd, LLDB_INVALID_PIPE); + + size_t num_bytes; + error = socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(2), num_bytes); + if (error.Fail()) { + LLDB_LOGF(log, + "lldb-platform child: " + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %s", + error.AsCString()); + return socket_error; + } + if (num_bytes != sizeof(protocol_info)) { + LLDB_LOGF( + log, + "lldb-platform child: " + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); + return socket_error; + } + } + socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, + FROM_PROTOCOL_INFO, &protocol_info, 0, 0); + if (socket == INVALID_SOCKET) { + LLDB_LOGF(log, + "lldb-platform child: " + "WSASocket(FROM_PROTOCOL_INFO) failed: error %d", + ::WSAGetLastError()); + return socket_error; + } +#else + socket = fd; #endif + Connection *conn = + new ConnectionFileDescriptor(new TCPSocket(socket, true, false)); + GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp"); + if (port_offset > 0) + platform.SetPortOffset(port_offset); + platform.SetPortMap(std::move(gdbserver_portmap)); + platform.SetConnection(std::unique_ptr<Connection>(conn)); + client_handle(platform, inferior_arguments); + return 0; + } + const bool children_inherit_listen_socket = false; // the test suite makes many connections in parallel, let's not miss any. // The highest this should get reasonably is a function of the number @@ -592,45 +570,26 @@ int main_platform(int argc, char *argv[]) { if (!available_port) { fprintf(stderr, "no available gdbserver port for connection - dropping...\n"); - delete conn; - continue; - } -#if !defined(_WIN32) - GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; - // GetNextAvailablePort() may return 0 if gdbserver_portmap is empty. - if (*available_port) - portmap_for_child.AllowPort(*available_port); - platform.SetPortMap(std::move(portmap_for_child)); - - auto childPid = fork(); - if (childPid) { - { - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid); - } - // Parent doesn't need a connection to the lldb client - delete conn; - - // Parent will continue to listen for new connections. - continue; } else { - // Child process will handle the connection and exit. - g_server = 0; - // Listening socket is owned by parent process. - acceptor_up.release(); - } -#else - if (!spawn_process_parent(progname, conn, *available_port, port_offset, - inferior_arguments, log_file, log_channels)) { - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.FreePort(*available_port); + error = spawn_process(progname, conn, *available_port, port_offset, + inferior_arguments, log_file, log_channels); + if (error.Fail()) { + { + + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePort(*available_port); + } + LLDB_LOGF(GetLog(LLDBLog::Platform), "spawn_process failed: %s", + error.AsCString()); + WithColor::error() + << "spawn_process failed: " << error.AsCString() << "\n"; + } } // Parent doesn't need a connection to the lldb client delete conn; // Parent will continue to listen for new connections. continue; -#endif } else { // If not running as a server, this process will not accept // connections while a connection is active. >From 714253b4c0998c276e62b30153fb6381a7a78692 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Wed, 7 Aug 2024 20:32:20 +0400 Subject: [PATCH 3/5] Added the class SharedSocket. --- lldb/tools/lldb-server/lldb-platform.cpp | 204 +++++++++++++---------- 1 file changed, 114 insertions(+), 90 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 388a033906f722..3098e7b5118e44 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -49,12 +49,110 @@ using namespace llvm; #ifdef _WIN32 typedef pipe_t fd_t; -#define kInvalidFD (LLDB_INVALID_PIPE) +const fd_t kInvalidSharedFD = LLDB_INVALID_PIPE; #else typedef NativeSocket fd_t; -#define kInvalidFD (Socket::kInvalidSocketValue) +const fd_t kInvalidSharedFD = Socket::kInvalidSocketValue; #endif +class SharedSocket { +public: + // Used by the parent process. + SharedSocket(Connection *conn, Status &error) { + m_fd = kInvalidSharedFD; + + const Socket *socket = + static_cast<const Socket *>(conn->GetReadObject().get()); + if (socket == nullptr) { + error = Status("invalid conn socket"); + return; + } + +#ifdef _WIN32 + m_socket = socket->GetNativeSocket(); + + // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. + error = m_socket_pipe.CreateNew(true); + if (error.Fail()) + return; + + m_fd = m_socket_pipe.GetReadPipe(); +#else + m_fd = socket->GetNativeSocket(); + error = Status(); +#endif + } + + // Used by the child process. + SharedSocket(fd_t fd) : m_fd(fd) {} + + fd_t GetSendableFD() { return m_fd; } + + Status CompleteSending(lldb::pid_t child_pid) { +#ifdef _WIN32 + // Transfer WSAPROTOCOL_INFO to the child process. + m_socket_pipe.CloseReadFileDescriptor(); + + WSAPROTOCOL_INFO protocol_info; + if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) == + SOCKET_ERROR) { + int last_error = ::WSAGetLastError(); + return Status("WSADuplicateSocket() failed, error: %d", last_error); + } + + size_t num_bytes; + Status error = + m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(10), num_bytes); + if (error.Fail()) + return error; + if (num_bytes != sizeof(protocol_info)) + return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); +#endif + return Status(); + } + + Status GetNativeSocket(NativeSocket &socket) { +#ifdef _WIN32 + socket = Socket::kInvalidSocketValue; + // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. + WSAPROTOCOL_INFO protocol_info; + { + Pipe socket_pipe(m_fd, LLDB_INVALID_PIPE); + size_t num_bytes; + Status error = + socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(10), num_bytes); + if (error.Fail()) + return error; + if (num_bytes != sizeof(protocol_info)) { + return Status( + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes", + num_bytes); + } + } + socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, + FROM_PROTOCOL_INFO, &protocol_info, 0, 0); + if (socket == INVALID_SOCKET) { + return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d", + ::WSAGetLastError()); + } + return Status(); +#else + socket = m_fd; + return Status(); +#endif + } + +private: +#ifdef _WIN32 + Pipe m_socket_pipe; + NativeSocket m_socket; +#endif + fd_t m_fd; +}; + static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; @@ -71,7 +169,7 @@ static struct option g_long_options[] = { {"max-gdbserver-port", required_argument, nullptr, 'M'}, {"socket-file", required_argument, nullptr, 'f'}, {"server", no_argument, &g_server, 1}, - {"fd", required_argument, nullptr, 'd'}, + {"child-platform-fd", required_argument, nullptr, 2}, {nullptr, 0, nullptr, 0}}; #if defined(__APPLE__) @@ -171,35 +269,18 @@ static Status spawn_process(const char *progname, Connection *conn, const std::string &log_file, const StringRef log_channels) { Status error; - const TCPSocket &tcpSocket = - static_cast<const TCPSocket &>(*conn->GetReadObject()); - NativeSocket socket = tcpSocket.GetNativeSocket(); - - ProcessLaunchInfo launch_info; - - fd_t fd; -#ifdef _WIN32 - // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. - Pipe socket_pipe; - error = socket_pipe.CreateNew(true); + SharedSocket shared_socket(conn, error); if (error.Fail()) return error; - // Seems it will not work anyway. ProcessLauncherWindows ignores FileActions. - // And it is necessary to pass HANDLE instead of FD on Widows. - launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor()); - - fd = socket_pipe.GetReadPipe(); -#else - fd = socket; -#endif + ProcessLaunchInfo launch_info; FileSpec self_spec(progname, FileSpec::Style::native); launch_info.SetExecutableFile(self_spec, true); Args &self_args = launch_info.GetArguments(); self_args.AppendArgument(llvm::StringRef("platform")); - self_args.AppendArgument(llvm::StringRef("--fd")); - self_args.AppendArgument(llvm::to_string(fd)); + self_args.AppendArgument(llvm::StringRef("--child-platform-fd")); + self_args.AppendArgument(llvm::to_string(shared_socket.GetSendableFD())); if (gdb_port) { self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); self_args.AppendArgument(llvm::to_string(gdb_port)); @@ -261,36 +342,11 @@ static Status spawn_process(const char *progname, Connection *conn, gdbserver_portmap.AssociatePortWithProcess(gdb_port, child_pid); } -#ifdef _WIN32 - // Transfer WSAPROTOCOL_INFO to the child process. - if (socket_pipe.CanRead()) - socket_pipe.CloseReadFileDescriptor(); - if (!socket_pipe.CanWrite()) { - Host::Kill(child_pid, SIGTERM); - return Status("cannot write to socket_pipe"); - } - - WSAPROTOCOL_INFO protocol_info; - if (::WSADuplicateSocket(socket, child_pid, &protocol_info) == SOCKET_ERROR) { - int last_error = ::WSAGetLastError(); - Host::Kill(child_pid, SIGTERM); - return Status("WSADuplicateSocket() failed, error: %d", last_error); - } - - size_t num_bytes; - error = socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(2), num_bytes); + error = shared_socket.CompleteSending(child_pid); if (error.Fail()) { Host::Kill(child_pid, SIGTERM); return error; } - if (num_bytes != sizeof(protocol_info)) { - Host::Kill(child_pid, SIGTERM); - return Status( - "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", - num_bytes); - } -#endif return Status(); } @@ -314,7 +370,7 @@ int main_platform(int argc, char *argv[]) { StringRef log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" - fd_t fd = kInvalidFD; + fd_t fd = kInvalidSharedFD; int min_gdbserver_port = 0; int max_gdbserver_port = 0; @@ -399,7 +455,7 @@ int main_platform(int argc, char *argv[]) { max_gdbserver_port = portnum; } break; - case 'd': { + case 2: { uint64_t _fd; if (!llvm::to_integer(optarg, _fd)) { WithColor::error() << "invalid fd " << optarg << "\n"; @@ -431,7 +487,7 @@ int main_platform(int argc, char *argv[]) { } // Print usage and exit if no listening port is specified. - if (listen_host_port.empty() && fd == kInvalidFD) + if (listen_host_port.empty() && fd == kInvalidSharedFD) show_usage = true; if (show_usage || option_error) { @@ -445,53 +501,21 @@ int main_platform(int argc, char *argv[]) { lldb_private::Args inferior_arguments; inferior_arguments.SetArguments(argc, const_cast<const char **>(argv)); - if (fd != kInvalidFD) { + if (fd != kInvalidSharedFD) { // Child process will handle the connection and exit. Log *log = GetLog(LLDBLog::Platform); if (!listen_host_port.empty()) { LLDB_LOGF(log, "lldb-platform child: " - "ambiguous parameters --listen and --fd"); + "ambiguous parameters --listen and --child-platform-fd"); return socket_error; } NativeSocket socket; -#ifdef _WIN32 - // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. - WSAPROTOCOL_INFO protocol_info; - { - Pipe socket_pipe(fd, LLDB_INVALID_PIPE); - - size_t num_bytes; - error = socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(2), num_bytes); - if (error.Fail()) { - LLDB_LOGF(log, - "lldb-platform child: " - "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %s", - error.AsCString()); - return socket_error; - } - if (num_bytes != sizeof(protocol_info)) { - LLDB_LOGF( - log, - "lldb-platform child: " - "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", - num_bytes); - return socket_error; - } - } - socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, - FROM_PROTOCOL_INFO, &protocol_info, 0, 0); - if (socket == INVALID_SOCKET) { - LLDB_LOGF(log, - "lldb-platform child: " - "WSASocket(FROM_PROTOCOL_INFO) failed: error %d", - ::WSAGetLastError()); + error = SharedSocket(fd).GetNativeSocket(socket); + if (error.Fail()) { + LLDB_LOGF(log, "lldb-platform child: %s", error.AsCString()); return socket_error; } -#else - socket = fd; -#endif Connection *conn = new ConnectionFileDescriptor(new TCPSocket(socket, true, false)); >From 154e9c48675558adee3efcbb999ce4b17b070d93 Mon Sep 17 00:00:00 2001 From: Dmitry Vassiliev <dvassil...@accesssoftek.com> Date: Thu, 8 Aug 2024 22:16:47 +0400 Subject: [PATCH 4/5] Made GetNativeSocket() a static function. --- lldb/tools/lldb-server/lldb-platform.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 3098e7b5118e44..0615da25691c25 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -57,7 +57,6 @@ const fd_t kInvalidSharedFD = Socket::kInvalidSocketValue; class SharedSocket { public: - // Used by the parent process. SharedSocket(Connection *conn, Status &error) { m_fd = kInvalidSharedFD; @@ -83,9 +82,6 @@ class SharedSocket { #endif } - // Used by the child process. - SharedSocket(fd_t fd) : m_fd(fd) {} - fd_t GetSendableFD() { return m_fd; } Status CompleteSending(lldb::pid_t child_pid) { @@ -113,13 +109,13 @@ class SharedSocket { return Status(); } - Status GetNativeSocket(NativeSocket &socket) { + static Status GetNativeSocket(fd_t fd, NativeSocket &socket) { #ifdef _WIN32 socket = Socket::kInvalidSocketValue; // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. WSAPROTOCOL_INFO protocol_info; { - Pipe socket_pipe(m_fd, LLDB_INVALID_PIPE); + Pipe socket_pipe(fd, LLDB_INVALID_PIPE); size_t num_bytes; Status error = socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), @@ -140,7 +136,7 @@ class SharedSocket { } return Status(); #else - socket = m_fd; + socket = fd; return Status(); #endif } @@ -306,9 +302,6 @@ static Status spawn_process(const char *progname, Connection *conn, launch_info.SetMonitorProcessCallback(&spawn_process_reaped); // Copy the current environment. - // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process - // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing - // in the environment. launch_info.GetEnvironment() = Host::GetEnvironment(); launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); @@ -511,7 +504,7 @@ int main_platform(int argc, char *argv[]) { } NativeSocket socket; - error = SharedSocket(fd).GetNativeSocket(socket); + error = SharedSocket::GetNativeSocket(fd, socket); if (error.Fail()) { LLDB_LOGF(log, "lldb-platform child: %s", error.AsCString()); return socket_error; >From 5988081e0fdcf7c14df4dedb65ba8b7f194b1766 Mon Sep 17 00:00:00 2001 From: Dmitry Vassiliev <dvassil...@accesssoftek.com> Date: Fri, 9 Aug 2024 12:06:25 +0400 Subject: [PATCH 5/5] Removed waitpid(). spawn_process_reaped() does its job. --- lldb/tools/lldb-server/lldb-platform.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 0615da25691c25..d03de235eb5d81 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -565,16 +565,6 @@ int main_platform(int argc, char *argv[]) { printf("Connection established.\n"); if (g_server) { - // Collect child zombie processes. -#if !defined(_WIN32) - ::pid_t waitResult; - while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) { - // waitResult is the child pid - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.FreePortForProcess(waitResult); - } -#endif - // After collecting zombie ports, get the next available std::optional<uint16_t> available_port; { std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits