llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) <details> <summary>Changes</summary> `lldb-server --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). 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. --- Patch is 26.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101283.diff 6 Files Affected: - (modified) lldb/include/lldb/Host/windows/PipeWindows.h (+3) - (modified) lldb/source/Host/windows/PipeWindows.cpp (+89-39) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp (+34) - (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h (+1-1) - (modified) lldb/tools/lldb-server/LLDBServerUtilities.cpp (+2) - (modified) lldb/tools/lldb-server/lldb-platform.cpp (+275-41) ``````````diff diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..c99cf52be46c1 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -61,6 +61,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; Status Write(const void *buf, size_t size, size_t &bytes_written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds &timeout, + size_t &bytes_written); Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_read) override; diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index c82c919607b5b..318d5abbed74d 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write) } ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } PipeWindows::~PipeWindows() { Close(); } @@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) { m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); return Status(); } @@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } return Status(); @@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() { return PipeWindows::kInvalidDescriptor; int result = m_write_fd; m_write_fd = PipeWindows::kInvalidDescriptor; + if (m_write_overlapped.hEvent) + ::CloseHandle(m_write_overlapped.hEvent); m_write = INVALID_HANDLE_VALUE; ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() { if (!CanWrite()) return; + if (m_write_overlapped.hEvent) + ::CloseHandle(m_write_overlapped.hEvent); + _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; m_write_fd = PipeWindows::kInvalidDescriptor; @@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read, &m_read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) - return Status(::GetLastError(), eErrorTypeWin32); - - DWORD timeout = (duration == std::chrono::microseconds::zero()) - ? INFINITE - : duration.count() * 1000; - DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout); - if (wait_result != WAIT_OBJECT_0) { - // The operation probably failed. However, if it timed out, we need to - // cancel the I/O. Between the time we returned from WaitForSingleObject - // and the time we call CancelIoEx, the operation may complete. If that - // hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that - // happens, the original operation should be considered to have been - // successful. - bool failed = true; - DWORD failure_error = ::GetLastError(); - if (wait_result == WAIT_TIMEOUT) { - BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped); - if (!cancel_result && GetLastError() == ERROR_NOT_FOUND) - failed = false; + if (!result) { + if (GetLastError() != ERROR_IO_PENDING) + return Status(::GetLastError(), eErrorTypeWin32); + else { + DWORD timeout = (duration == std::chrono::microseconds::zero()) + ? INFINITE + : duration.count() * 1000; + DWORD wait_result = + ::WaitForSingleObject(m_read_overlapped.hEvent, timeout); + if (wait_result != WAIT_OBJECT_0) { + // The operation probably failed. However, if it timed out, we need to + // cancel the I/O. Between the time we returned from WaitForSingleObject + // and the time we call CancelIoEx, the operation may complete. If that + // hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that + // happens, the original operation should be considered to have been + // successful. + bool failed = true; + DWORD failure_error = ::GetLastError(); + if (wait_result == WAIT_TIMEOUT) { + BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped); + if (!cancel_result && GetLastError() == ERROR_NOT_FOUND) + failed = false; + } + if (failed) + return Status(failure_error, eErrorTypeWin32); + } + + // Now we call GetOverlappedResult setting bWait to false, since we've + // already waited as long as we're willing to. + if (!GetOverlappedResult(m_read, &m_read_overlapped, &sys_bytes_read, + FALSE)) + return Status(::GetLastError(), eErrorTypeWin32); } - if (failed) - return Status(failure_error, eErrorTypeWin32); } - - // Now we call GetOverlappedResult setting bWait to false, since we've - // already waited as long as we're willing to. - if (!GetOverlappedResult(m_read, &m_read_overlapped, &sys_bytes_read, FALSE)) - return Status(::GetLastError(), eErrorTypeWin32); - bytes_read = sys_bytes_read; return Status(); } -Status PipeWindows::Write(const void *buf, size_t num_bytes, - size_t &bytes_written) { +Status PipeWindows::WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds &duration, + size_t &bytes_written) { if (!CanWrite()) return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32); - DWORD sys_bytes_written = 0; - BOOL write_result = ::WriteFile(m_write, buf, num_bytes, &sys_bytes_written, - &m_write_overlapped); - if (!write_result && GetLastError() != ERROR_IO_PENDING) - return Status(::GetLastError(), eErrorTypeWin32); + bytes_written = 0; + DWORD sys_bytes_write = size; + BOOL result = ::WriteFile(m_write, buf, sys_bytes_write, &sys_bytes_write, + &m_write_overlapped); + if (!result) { + if (GetLastError() != ERROR_IO_PENDING) + return Status(::GetLastError(), eErrorTypeWin32); + else { + DWORD timeout = (duration == std::chrono::microseconds::zero()) + ? INFINITE + : duration.count() * 1000; + DWORD wait_result = + ::WaitForSingleObject(m_write_overlapped.hEvent, timeout); + if (wait_result != WAIT_OBJECT_0) { + // The operation probably failed. However, if it timed out, we need to + // cancel the I/O. Between the time we returned from WaitForSingleObject + // and the time we call CancelIoEx, the operation may complete. If that + // hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that + // happens, the original operation should be considered to have been + // successful. + bool failed = true; + DWORD failure_error = ::GetLastError(); + if (wait_result == WAIT_TIMEOUT) { + BOOL cancel_result = CancelIoEx(m_write, &m_write_overlapped); + if (!cancel_result && GetLastError() == ERROR_NOT_FOUND) + failed = false; + } + if (failed) + return Status(failure_error, eErrorTypeWin32); + } + + // Now we call GetOverlappedResult setting bWait to false, since we've + // already waited as long as we're willing to. + if (!GetOverlappedResult(m_write, &m_write_overlapped, &sys_bytes_write, + FALSE)) + return Status(::GetLastError(), eErrorTypeWin32); + } + } - BOOL result = GetOverlappedResult(m_write, &m_write_overlapped, - &sys_bytes_written, TRUE); - if (!result) - return Status(::GetLastError(), eErrorTypeWin32); + bytes_written = sys_bytes_write; return Status(); } + +Status PipeWindows::Write(const void *buf, size_t size, size_t &bytes_written) { + return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(), + bytes_written); +} diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index 65f1cc12ba307..18824a6cb08ca 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -159,6 +159,40 @@ GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() = default; +void GDBRemoteCommunicationServerPlatform::Proc( + const lldb_private::Args &args) { + if (!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 = LaunchGDBServer(args, + "", // hostname + pid, port, socket_name); + if (error.Success()) + SetPendingGdbServer(pid, *port, socket_name); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { + if (GetPacketAndSendResponse(std::nullopt, error, interrupt, done) != + GDBRemoteCommunication::PacketResult::Success) + break; + } + + if (error.Fail()) { + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, + "GDBRemoteCommunicationServerPlatform::%s() " + "GetPacketAndSendResponse: %s", + __FUNCTION__, error.AsCString()); + } +} + Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid, std::optional<uint16_t> &port, std::string &socket_name) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h index 1853025466cf0..2f3f313c917c4 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -85,7 +85,7 @@ class GDBRemoteCommunicationServerPlatform void SetPortOffset(uint16_t port_offset); - void SetInferiorArguments(const lldb_private::Args &args); + void Proc(const lldb_private::Args &args); // Set port if you want to use a specific port number. // Otherwise port will be set to the port that was chosen for you. diff --git a/lldb/tools/lldb-server/LLDBServerUtilities.cpp b/lldb/tools/lldb-server/LLDBServerUtilities.cpp index c3a8df19e969e..5facfbf3105e9 100644 --- a/lldb/tools/lldb-server/LLDBServerUtilities.cpp +++ b/lldb/tools/lldb-server/LLDBServerUtilities.cpp @@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler { : m_stream_sp(stream_sp) {} void Emit(llvm::StringRef message) override { + std::lock_guard<std::mutex> guard(m_mutex); (*m_stream_sp) << message; m_stream_sp->flush(); } private: + std::mutex m_mutex; std::shared_ptr<raw_ostream> m_stream_sp; }; diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 7148a1d2a3094..f14b90b1c28fc 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,218 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) +static void SpawnProcessReaped(lldb::pid_t pid, int signal, int status) { + std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePortForProcess(pid); +} + +static bool SpawnProcessParent(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(&SpawnProcessReaped); + + // 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 SpawnProcessChild(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(0x%x).ReadWithTimeout(WSAPROTOCOL_INFO) failed: %s", + accept_fd, error.AsCString()); + exit(SOCKET_ERROR); + } + if (num_bytes != sizeof(protocol_info)) { + LLDB_LOGF(log, + "lldb-platform child: " + "socket_pipe(0x%x).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)); + GDBRemoteCommunicationServ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/101283 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
