Index: include/lldb/Host/PipeBase.h =================================================================== --- include/lldb/Host/PipeBase.h +++ include/lldb/Host/PipeBase.h @@ -41,6 +41,9 @@ virtual bool CanRead() const = 0; virtual bool CanWrite() const = 0; + virtual lldb::pipe_t GetReadPipe() const = 0; + virtual lldb::pipe_t GetWritePipe() const = 0; + virtual int GetReadFileDescriptor() const = 0; virtual int GetWriteFileDescriptor() const = 0; virtual int ReleaseReadFileDescriptor() = 0; Index: include/lldb/Host/posix/PipePosix.h =================================================================== --- include/lldb/Host/posix/PipePosix.h +++ include/lldb/Host/posix/PipePosix.h @@ -27,7 +27,7 @@ static int kInvalidDescriptor; PipePosix(); - PipePosix(int read_fd, int write_fd); + PipePosix(lldb::pipe_t read, lldb::pipe_t write); PipePosix(const PipePosix &) = delete; PipePosix(PipePosix &&pipe_posix); PipePosix &operator=(const PipePosix &) = delete; @@ -49,6 +49,13 @@ bool CanRead() const override; bool CanWrite() const override; + lldb::pipe_t GetReadPipe() const override { + return lldb::pipe_t(GetReadFileDescriptor()); + } + lldb::pipe_t GetWritePipe() const override { + return lldb::pipe_t(GetWriteFileDescriptor()); + } + int GetReadFileDescriptor() const override; int GetWriteFileDescriptor() const override; int ReleaseReadFileDescriptor() override; Index: include/lldb/Host/windows/PipeWindows.h =================================================================== --- include/lldb/Host/windows/PipeWindows.h +++ include/lldb/Host/windows/PipeWindows.h @@ -24,10 +24,18 @@ //---------------------------------------------------------------------- class PipeWindows : public PipeBase { public: + static const int kInvalidDescriptor = -1; + +public: PipeWindows(); + PipeWindows(lldb::pipe_t read, lldb::pipe_t write); ~PipeWindows() override; + // Create an unnamed pipe. Status CreateNew(bool child_process_inherit) override; + + // Create a named pipe. + Status CreateNewNamed(bool child_process_inherit); Status CreateNew(llvm::StringRef name, bool child_process_inherit) override; Status CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, @@ -41,6 +49,9 @@ bool CanRead() const override; bool CanWrite() const override; + lldb::pipe_t GetReadPipe() const { return lldb::pipe_t(m_read); } + lldb::pipe_t GetWritePipe() const { return lldb::pipe_t(m_write); } + int GetReadFileDescriptor() const override; int GetWriteFileDescriptor() const override; int ReleaseReadFileDescriptor() override; Index: include/lldb/lldb-types.h =================================================================== --- include/lldb/lldb-types.h +++ include/lldb/lldb-types.h @@ -47,7 +47,8 @@ typedef void *thread_arg_t; // Host thread argument type typedef unsigned thread_result_t; // Host thread result type typedef thread_result_t (*thread_func_t)(void *); // Host thread function type -} +typedef void *pipe_t; // Host pipe type is HANDLE +} // namespace lldb #else @@ -65,6 +66,7 @@ typedef void *thread_arg_t; // Host thread argument type typedef void *thread_result_t; // Host thread result type typedef void *(*thread_func_t)(void *); // Host thread function type +typedef int pipe_t; // Host pipe type } // namespace lldb #endif @@ -76,10 +78,11 @@ void *baton, const char **argv, lldb_private::CommandReturnObject &result); typedef bool (*ExpressionCancelCallback)(ExpressionEvaluationPhase phase, void *baton); -} +} // namespace lldb #define LLDB_INVALID_PROCESS ((lldb::process_t)-1) #define LLDB_INVALID_HOST_THREAD ((lldb::thread_t)NULL) +#define LLDB_INVALID_PIPE ((lldb::pipe_t)-1) namespace lldb { typedef uint64_t addr_t; @@ -91,6 +94,6 @@ typedef int32_t watch_id_t; typedef void *opaque_compiler_type_t; typedef uint64_t queue_id_t; -} +} // namespace lldb #endif // LLDB_lldb_types_h_ Index: source/Host/posix/PipePosix.cpp =================================================================== --- source/Host/posix/PipePosix.cpp +++ source/Host/posix/PipePosix.cpp @@ -61,12 +61,13 @@ std::chrono::time_point Now() { return std::chrono::steady_clock::now(); } -} +} // namespace PipePosix::PipePosix() : m_fds{PipePosix::kInvalidDescriptor, PipePosix::kInvalidDescriptor} {} -PipePosix::PipePosix(int read_fd, int write_fd) : m_fds{read_fd, write_fd} {} +PipePosix::PipePosix(lldb::pipe_t read, lldb::pipe_t write) + : m_fds{read, write} {} PipePosix::PipePosix(PipePosix &&pipe_posix) : PipeBase{std::move(pipe_posix)}, Index: source/Host/windows/PipeWindows.cpp =================================================================== --- source/Host/windows/PipeWindows.cpp +++ source/Host/windows/PipeWindows.cpp @@ -25,14 +25,41 @@ namespace { std::atomic g_pipe_serial(0); +constexpr llvm::StringLiteral g_pipe_name_prefix = "\\\\.\\Pipe\\"; +} // namespace + +PipeWindows::PipeWindows() + : m_read(INVALID_HANDLE_VALUE), m_write(INVALID_HANDLE_VALUE), + m_read_fd(PipeWindows::kInvalidDescriptor), + m_write_fd(PipeWindows::kInvalidDescriptor) { + ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); } -PipeWindows::PipeWindows() { - m_read = INVALID_HANDLE_VALUE; - m_write = INVALID_HANDLE_VALUE; +PipeWindows::PipeWindows(pipe_t read, pipe_t write) + : m_read((HANDLE)read), m_write((HANDLE)write), + m_read_fd(PipeWindows::kInvalidDescriptor), + m_write_fd(PipeWindows::kInvalidDescriptor) { + assert(read != LLDB_INVALID_PIPE || write != LLDB_INVALID_PIPE); + + // Don't risk in passing file descriptors and getting handles from them by + // _get_osfhandle since the retrieved handles are highly likely unrecognized + // in the current process and usually crashes the program. Pass handles + // instead since the handle can be inherited. + + if (read != LLDB_INVALID_PIPE) { + m_read_fd = _open_osfhandle((intptr_t)read, _O_RDONLY); + // Make sure the fd and native handle are consistent. + if (m_read_fd < 0) + m_read = INVALID_HANDLE_VALUE; + } + + if (write != LLDB_INVALID_PIPE) { + m_write_fd = _open_osfhandle((intptr_t)write, _O_WRONLY); + if (m_write_fd < 0) + m_write = INVALID_HANDLE_VALUE; + } - m_read_fd = -1; - m_write_fd = -1; ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); } @@ -40,6 +67,24 @@ PipeWindows::~PipeWindows() { Close(); } Status PipeWindows::CreateNew(bool child_process_inherit) { + // Create an anonymous pipe with the specified inheritance. + SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0, + child_process_inherit ? TRUE : FALSE}; + BOOL result = ::CreatePipe(&m_read, &m_write, &sa, 1024); + if (result == FALSE) + return Status(::GetLastError(), eErrorTypeWin32); + + m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY); + ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + + m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); + + return Status(); +} + +Status PipeWindows::CreateNewNamed(bool child_process_inherit) { // Even for anonymous pipes, we open a named pipe. This is because you // cannot get overlapped i/o on Windows without using a named pipe. So we // synthesize a unique name. @@ -60,7 +105,7 @@ if (CanRead() || CanWrite()) return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32); - std::string pipe_path = "\\\\.\\Pipe\\"; + std::string pipe_path = g_pipe_name_prefix; pipe_path.append(name); // Always open for overlapped i/o. We implement blocking manually in Read @@ -75,7 +120,8 @@ ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); m_read_overlapped.hEvent = ::CreateEvent(nullptr, TRUE, FALSE, nullptr); - // Open the write end of the pipe. + // Open the write end of the pipe. Note that closing either the read or + // write end of the pipe could directly close the pipe itself. Status result = OpenNamedPipe(name, child_process_inherit, false); if (!result.Success()) { CloseReadFileDescriptor(); @@ -111,7 +157,7 @@ Status PipeWindows::OpenAsReader(llvm::StringRef name, bool child_process_inherit) { - if (CanRead() || CanWrite()) + if (CanRead()) return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32); return OpenNamedPipe(name, child_process_inherit, true); @@ -121,7 +167,7 @@ PipeWindows::OpenAsWriterWithTimeout(llvm::StringRef name, bool child_process_inherit, const std::chrono::microseconds &timeout) { - if (CanRead() || CanWrite()) + if (CanWrite()) return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32); return OpenNamedPipe(name, child_process_inherit, false); @@ -137,7 +183,7 @@ SECURITY_ATTRIBUTES attributes = {}; attributes.bInheritHandle = child_process_inherit; - std::string pipe_path = "\\\\.\\Pipe\\"; + std::string pipe_path = g_pipe_name_prefix; pipe_path.append(name); if (is_read) { @@ -170,9 +216,9 @@ int PipeWindows::ReleaseReadFileDescriptor() { if (!CanRead()) - return -1; + return PipeWindows::kInvalidDescriptor; int result = m_read_fd; - m_read_fd = -1; + m_read_fd = PipeWindows::kInvalidDescriptor; if (m_read_overlapped.hEvent) ::CloseHandle(m_read_overlapped.hEvent); m_read = INVALID_HANDLE_VALUE; @@ -182,9 +228,9 @@ int PipeWindows::ReleaseWriteFileDescriptor() { if (!CanWrite()) - return -1; + return PipeWindows::kInvalidDescriptor; int result = m_write_fd; - m_write_fd = -1; + m_write_fd = PipeWindows::kInvalidDescriptor; m_write = INVALID_HANDLE_VALUE; ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -196,9 +242,10 @@ if (m_read_overlapped.hEvent) ::CloseHandle(m_read_overlapped.hEvent); + _close(m_read_fd); m_read = INVALID_HANDLE_VALUE; - m_read_fd = -1; + m_read_fd = PipeWindows::kInvalidDescriptor; ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); } @@ -208,7 +255,7 @@ _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; - m_write_fd = -1; + m_write_fd = PipeWindows::kInvalidDescriptor; ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); } Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1074,9 +1074,9 @@ __FUNCTION__, error.AsCString()); return error; } - int write_fd = socket_pipe.GetWriteFileDescriptor(); + pipe_t write = socket_pipe.GetWritePipe(); debugserver_args.AppendArgument(llvm::StringRef("--pipe")); - debugserver_args.AppendArgument(llvm::to_string(write_fd)); + debugserver_args.AppendArgument(llvm::to_string(write)); launch_info.AppendCloseFileAction(socket_pipe.GetReadFileDescriptor()); #endif } else { Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -164,9 +164,6 @@ GDBRemoteCommunication::PacketResult GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer( StringExtractorGDBRemote &packet) { -#ifdef _WIN32 - return SendErrorResponse(9); -#else // Spawn a local debugserver as a platform so we can then attach or launch a // process... @@ -217,10 +214,9 @@ PacketResult packet_result = SendPacketNoLock(response.GetString()); if (packet_result != PacketResult::Success) { if (debugserver_pid != LLDB_INVALID_PROCESS_ID) - ::kill(debugserver_pid, SIGINT); + Host::Kill(debugserver_pid, SIGINT); } return packet_result; -#endif } GDBRemoteCommunication::PacketResult Index: tools/lldb-server/lldb-gdbserver.cpp =================================================================== --- tools/lldb-server/lldb-gdbserver.cpp +++ tools/lldb-server/lldb-gdbserver.cpp @@ -218,20 +218,17 @@ return writeSocketIdToPipe(port_name_pipe, socket_id); } -Status writeSocketIdToPipe(int unnamed_pipe_fd, const std::string &socket_id) { -#if defined(_WIN32) - return Status("Unnamed pipes are not supported on Windows."); -#else - Pipe port_pipe{Pipe::kInvalidDescriptor, unnamed_pipe_fd}; +Status writeSocketIdToPipe(lldb::pipe_t unnamed_pipe, + const std::string &socket_id) { + Pipe port_pipe{LLDB_INVALID_PIPE, unnamed_pipe}; return writeSocketIdToPipe(port_pipe, socket_id); -#endif } void ConnectToRemote(MainLoop &mainloop, GDBRemoteCommunicationServerLLGS &gdb_server, bool reverse_connect, const char *const host_and_port, const char *const progname, const char *const subcommand, - const char *const named_pipe_path, int unnamed_pipe_fd, + const char *const named_pipe_path, pipe_t unnamed_pipe, int connection_fd) { Status error; @@ -331,8 +328,8 @@ } // If we have an unnamed pipe to write the socket id back to, do that // now. - else if (unnamed_pipe_fd >= 0) { - error = writeSocketIdToPipe(unnamed_pipe_fd, socket_id); + else if (unnamed_pipe != LLDB_INVALID_PIPE) { + error = writeSocketIdToPipe(unnamed_pipe, socket_id); if (error.Fail()) fprintf(stderr, "failed to write to the unnamed pipe: %s\n", error.AsCString()); @@ -384,7 +381,7 @@ std::string log_file; StringRef log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" - int unnamed_pipe_fd = -1; + lldb::pipe_t unnamed_pipe = LLDB_INVALID_PIPE; bool reverse_connect = false; int connection_fd = -1; @@ -425,7 +422,7 @@ case 'U': // unnamed pipe if (optarg && optarg[0]) - unnamed_pipe_fd = StringConvert::ToUInt32(optarg, -1); + unnamed_pipe = (pipe_t)StringConvert::ToUInt64(optarg, -1); break; case 'r': @@ -528,8 +525,8 @@ printf("%s-%s", LLGS_PROGRAM_NAME, LLGS_VERSION_STR); ConnectToRemote(mainloop, gdb_server, reverse_connect, host_and_port, - progname, subcommand, named_pipe_path.c_str(), - unnamed_pipe_fd, connection_fd); + progname, subcommand, named_pipe_path.c_str(), + unnamed_pipe, connection_fd); if (!gdb_server.IsConnected()) { fprintf(stderr, "no connection information provided, unable to run\n");