llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) <details> <summary>Changes</summary> Replaced `int connection_fd = -1` with `shared_fd_t connection_fd = SharedSocket::kInvalidFD`. This is prerequisite for #<!-- -->104238. --- Full diff: https://github.com/llvm/llvm-project/pull/107553.diff 7 Files Affected: - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (+10-7) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (+3-2) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+1-1) - (modified) lldb/tools/lldb-server/lldb-gdbserver.cpp (+23-8) - (modified) lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp (-4) - (modified) lldb/unittests/tools/lldb-server/tests/TestClient.cpp (-4) - (modified) lldb/unittests/tools/lldb-server/tests/TestClient.h (+4) ``````````diff diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 04f0f525e3ebba..ca517c2f3025d4 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -962,16 +962,19 @@ Status GDBRemoteCommunication::StartDebugserverProcess( #endif // If a url is supplied then use it - if (url) + if (url && url[0]) debugserver_args.AppendArgument(llvm::StringRef(url)); - if (pass_comm_fd >= 0) { + if (pass_comm_fd != SharedSocket::kInvalidFD) { StreamString fd_arg; - fd_arg.Printf("--fd=%i", pass_comm_fd); + fd_arg.Printf("--fd=%" PRIi64, (int64_t)pass_comm_fd); debugserver_args.AppendArgument(fd_arg.GetString()); // Send "pass_comm_fd" down to the inferior so it can use it to - // communicate back with this process - launch_info.AppendDuplicateFileAction(pass_comm_fd, pass_comm_fd); + // communicate back with this process. Ignored on Windows. +#ifndef _WIN32 + launch_info.AppendDuplicateFileAction((int)pass_comm_fd, + (int)pass_comm_fd); +#endif } // use native registers, not the GDB registers @@ -991,7 +994,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess( // port is null when debug server should listen on domain socket - we're // not interested in port value but rather waiting for debug server to // become available. - if (pass_comm_fd == -1) { + if (pass_comm_fd == SharedSocket::kInvalidFD) { if (url) { // Create a temporary file to get the stdout/stderr and redirect the output of // the command into this file. We will later read this file if all goes well @@ -1137,7 +1140,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (error.Success() && (launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) && - pass_comm_fd == -1) { + pass_comm_fd == SharedSocket::kInvalidFD) { if (named_pipe_path.size() > 0) { error = socket_pipe.OpenAsReader(named_pipe_path, false); if (error.Fail()) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index a182291f6c8594..754797ba7f5045 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -21,6 +21,7 @@ #include "lldb/Core/Communication.h" #include "lldb/Host/Config.h" #include "lldb/Host/HostThread.h" +#include "lldb/Host/Socket.h" #include "lldb/Utility/Args.h" #include "lldb/Utility/Listener.h" #include "lldb/Utility/Predicate.h" @@ -156,8 +157,8 @@ class GDBRemoteCommunication : public Communication { Platform *platform, // If non nullptr, then check with the platform for // the GDB server binary if it can't be located ProcessLaunchInfo &launch_info, uint16_t *port, const Args *inferior_args, - int pass_comm_fd); // Communication file descriptor to pass during - // fork/exec to avoid having to connect/accept + shared_fd_t pass_comm_fd); // Communication file descriptor to pass during + // fork/exec to avoid having to connect/accept void DumpHistory(Stream &strm); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 65b1b3a0f26863..5eaf9ce2a302aa 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3447,7 +3447,7 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver( } #endif - int communication_fd = -1; + 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. diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp index 563284730bc705..3b80e922be73a9 100644 --- a/lldb/tools/lldb-server/lldb-gdbserver.cpp +++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp @@ -24,8 +24,8 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Pipe.h" -#include "lldb/Host/Socket.h" #include "lldb/Host/common/NativeProcessProtocol.h" +#include "lldb/Host/common/TCPSocket.h" #include "lldb/Target/Process.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" @@ -195,18 +195,31 @@ void ConnectToRemote(MainLoop &mainloop, bool reverse_connect, llvm::StringRef host_and_port, const char *const progname, const char *const subcommand, const char *const named_pipe_path, pipe_t unnamed_pipe, - int connection_fd) { + shared_fd_t connection_fd) { Status error; std::unique_ptr<Connection> connection_up; std::string url; - if (connection_fd != -1) { + if (connection_fd != SharedSocket::kInvalidFD) { +#ifdef _WIN32 + NativeSocket sockfd; + error = SharedSocket::GetNativeSocket(connection_fd, sockfd); + if (error.Fail()) { + llvm::errs() << llvm::formatv("error: GetNativeSocket failed: {0}\n", + error.AsCString()); + exit(-1); + } + connection_up = + std::unique_ptr<Connection>(new ConnectionFileDescriptor(new TCPSocket( + sockfd, /*should_close=*/true, /*child_processes_inherit=*/false))); +#else url = llvm::formatv("fd://{0}", connection_fd).str(); // Create the connection. -#if LLDB_ENABLE_POSIX && !defined _WIN32 +#if LLDB_ENABLE_POSIX ::fcntl(connection_fd, F_SETFD, FD_CLOEXEC); +#endif #endif } else if (!host_and_port.empty()) { llvm::Expected<std::string> url_exp = @@ -338,7 +351,7 @@ int main_gdbserver(int argc, char *argv[]) { log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" lldb::pipe_t unnamed_pipe = LLDB_INVALID_PIPE; bool reverse_connect = false; - int connection_fd = -1; + shared_fd_t connection_fd = SharedSocket::kInvalidFD; // ProcessLaunchInfo launch_info; ProcessAttachInfo attach_info; @@ -404,10 +417,12 @@ int main_gdbserver(int argc, char *argv[]) { unnamed_pipe = (pipe_t)Arg; } if (Args.hasArg(OPT_fd)) { - if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), connection_fd)) { + int64_t fd; + if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), fd)) { WithColor::error() << "invalid '--fd' argument\n" << HelpText; return 1; } + connection_fd = (shared_fd_t)fd; } if (!LLDBServerUtilities::SetupLogging( @@ -423,7 +438,7 @@ int main_gdbserver(int argc, char *argv[]) { for (const char *Val : Arg->getValues()) Inputs.push_back(Val); } - if (Inputs.empty() && connection_fd == -1) { + if (Inputs.empty() && connection_fd == SharedSocket::kInvalidFD) { WithColor::error() << "no connection arguments\n" << HelpText; return 1; } @@ -432,7 +447,7 @@ int main_gdbserver(int argc, char *argv[]) { GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager); llvm::StringRef host_and_port; - if (!Inputs.empty()) { + if (!Inputs.empty() && connection_fd == SharedSocket::kInvalidFD) { host_and_port = Inputs.front(); Inputs.erase(Inputs.begin()); } diff --git a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp index 66e92415911afb..34f8d23e94e4c9 100644 --- a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp +++ b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp @@ -14,10 +14,6 @@ using namespace llgs_tests; using namespace lldb_private; using namespace llvm; -#ifdef SendMessage -#undef SendMessage -#endif - // Disable this test on Windows as it appears to have a race condition // that causes lldb-server not to exit after the inferior hangs up. #if !defined(_WIN32) diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp index 2d7ce36bacfaa3..a6f2dc32c6d0c0 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp @@ -26,10 +26,6 @@ using namespace lldb_private; using namespace llvm; using namespace llgs_tests; -#ifdef SendMessage -#undef SendMessage -#endif - TestClient::TestClient(std::unique_ptr<Connection> Conn) { SetConnection(std::move(Conn)); SetPacketTimeout(std::chrono::seconds(10)); diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.h b/lldb/unittests/tools/lldb-server/tests/TestClient.h index deb6802d0da707..5d1eacaf6198e8 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.h +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.h @@ -20,6 +20,10 @@ #include <optional> #include <string> +#ifdef SendMessage +#undef SendMessage +#endif + #if LLDB_SERVER_IS_DEBUGSERVER #define LLGS_TEST(x) DISABLED_ ## x #define DS_TEST(x) x `````````` </details> https://github.com/llvm/llvm-project/pull/107553 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
