From 0b9d3eefdbf2c9bb50e139c5272b413e8227d842 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Wed, 17 Dec 2014 18:02:19 +0000 Subject: [PATCH] Enhance the Pipe interface for better portability. This patch makes a number of improvements to the Pipe interface. 1) An interface (PipeBase) is provided which exposes pure virtual methods for any implementation of Pipe to override. While not strictly necessary, this helps catch errors where the interfaces are out of sync. 2) All methods return lldb_private::Error instead of returning bool or void. This allows richer error information to be propagated up to LLDB. 3) A new ReadWithTimeout() method is exposed in the base class and implemented on Windows. 4) Support for both named and anonymous pipes is exposed through the base interface and implemented on Windows. For creating a new pipe, both named and anonymous pipes are supported, and for opening an existing pipe, only named pipes are supported. New methods described in points #3 and #4 are stubbed out on posix, but fully implemented on Windows. These should be implemented by someone on the linux / mac / bsd side. Reviewed by: Greg Clayton, Oleksiy Vyalov Differential Revision: http://reviews.llvm.org/D6686 llvm-svn: 224442 --- lldb/include/lldb/Host/Pipe.h | 8 + lldb/include/lldb/Host/PipeBase.h | 48 +++ lldb/include/lldb/Host/posix/PipePosix.h | 68 ++-- lldb/include/lldb/Host/windows/PipeWindows.h | 59 ++-- lldb/lldb.xcodeproj/project.pbxproj | 2 + .../posix/ConnectionFileDescriptorPosix.cpp | 19 +- lldb/source/Host/posix/PipePosix.cpp | 187 +++++++---- lldb/source/Host/windows/PipeWindows.cpp | 301 +++++++++++------- .../Interpreter/ScriptInterpreterPython.cpp | 10 +- lldb/source/Target/Process.cpp | 16 +- 10 files changed, 452 insertions(+), 266 deletions(-) create mode 100644 lldb/include/lldb/Host/PipeBase.h diff --git a/lldb/include/lldb/Host/Pipe.h b/lldb/include/lldb/Host/Pipe.h index e92d613f7e6a7..a5502efa78ed2 100644 --- a/lldb/include/lldb/Host/Pipe.h +++ b/lldb/include/lldb/Host/Pipe.h @@ -12,8 +12,16 @@ #if defined(_WIN32) #include "lldb/Host/windows/PipeWindows.h" +namespace lldb_private +{ +typedef PipeWindows Pipe; +} #else #include "lldb/Host/posix/PipePosix.h" +namespace lldb_private +{ +typedef PipePosix Pipe; +} #endif #endif // liblldb_Host_Pipe_h_ diff --git a/lldb/include/lldb/Host/PipeBase.h b/lldb/include/lldb/Host/PipeBase.h new file mode 100644 index 0000000000000..366e20c8eec95 --- /dev/null +++ b/lldb/include/lldb/Host/PipeBase.h @@ -0,0 +1,48 @@ +//===-- PipeBase.h -----------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef liblldb_Host_PipeBase_h_ +#define liblldb_Host_PipeBase_h_ + +#include +#include + +#include "lldb/Core/Error.h" +#include "llvm/ADT/StringRef.h" + +namespace lldb_private +{ +class PipeBase +{ + public: + virtual ~PipeBase() {} + + virtual Error CreateNew(bool child_process_inherit) = 0; + virtual Error CreateNew(llvm::StringRef name, bool child_process_inherit) = 0; + virtual Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) = 0; + virtual Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit) = 0; + + virtual bool CanRead() const = 0; + virtual bool CanWrite() const = 0; + + virtual int GetReadFileDescriptor() const = 0; + virtual int GetWriteFileDescriptor() const = 0; + virtual int ReleaseReadFileDescriptor() = 0; + virtual int ReleaseWriteFileDescriptor() = 0; + + // Close both descriptors + virtual void Close() = 0; + + virtual Error Write(const void *buf, size_t size, size_t &bytes_written) = 0; + virtual Error Read(void *buf, size_t size, size_t &bytes_read) = 0; + virtual Error ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &timeout, size_t &bytes_read) = 0; +}; +} + +#endif diff --git a/lldb/include/lldb/Host/posix/PipePosix.h b/lldb/include/lldb/Host/posix/PipePosix.h index 254edc60fafe1..41e25fa9d0e81 100644 --- a/lldb/include/lldb/Host/posix/PipePosix.h +++ b/lldb/include/lldb/Host/posix/PipePosix.h @@ -11,70 +11,50 @@ #define liblldb_Host_posix_PipePosix_h_ #if defined(__cplusplus) -#include -#include -#include - -#include "lldb/lldb-private.h" +#include "lldb/Host/PipeBase.h" namespace lldb_private { //---------------------------------------------------------------------- -/// @class Pipe Pipe.h "lldb/Host/posix/PipePosix.h" +/// @class PipePosix PipePosix .h "lldb/Host/posix/PipePosix.h" /// @brief A posix-based implementation of Pipe, a class that abtracts /// unix style pipes. /// /// A class that abstracts the LLDB core from host pipe functionality. //---------------------------------------------------------------------- -class Pipe +class PipePosix : public PipeBase { public: static int kInvalidDescriptor; - - Pipe(); - - ~Pipe(); - - bool - Open(bool child_processes_inherit = false); - bool - IsValid() const; - - bool - ReadDescriptorIsValid() const; + PipePosix(); - bool - WriteDescriptorIsValid() const; + ~PipePosix() override; - int - GetReadFileDescriptor() const; - - int - GetWriteFileDescriptor() const; - - // Close both descriptors - void - Close(); + Error CreateNew(bool child_process_inherit) override; + Error CreateNew(llvm::StringRef name, bool child_process_inherit) override; + Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) override; + Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit) override; - bool - CloseReadFileDescriptor(); - - bool - CloseWriteFileDescriptor(); + bool CanRead() const override; + bool CanWrite() const override; - int - ReleaseReadFileDescriptor(); - - int - ReleaseWriteFileDescriptor(); + int GetReadFileDescriptor() const override; + int GetWriteFileDescriptor() const override; + int ReleaseReadFileDescriptor() override; + int ReleaseWriteFileDescriptor() override; - size_t - Read (void *buf, size_t size); + // Close both descriptors + void Close() override; + + Error Write(const void *buf, size_t size, size_t &bytes_written) override; + Error Read(void *buf, size_t size, size_t &bytes_read) override; + Error ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &timeout, size_t &bytes_read) override; - size_t - Write (const void *buf, size_t size); private: + void CloseReadFileDescriptor(); + void CloseWriteFileDescriptor(); + int m_fds[2]; }; diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 1a22a3ca9c626..b521e063592b3 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -10,6 +10,7 @@ #ifndef liblldb_Host_windows_PipeWindows_h_ #define liblldb_Host_windows_PipeWindows_h_ +#include "lldb/Host/PipeBase.h" #include "lldb/Host/windows/windows.h" namespace lldb_private @@ -22,55 +23,49 @@ namespace lldb_private /// /// A class that abstracts the LLDB core from host pipe functionality. //---------------------------------------------------------------------- -class Pipe +class PipeWindows : public PipeBase { public: - Pipe(); + PipeWindows(); + ~PipeWindows() override; - ~Pipe(); + Error CreateNew(bool child_process_inherit) override; + Error CreateNew(llvm::StringRef name, bool child_process_inherit) override; + Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) override; + Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit) override; - bool Open(bool read_overlapped = false, bool write_overlapped = false); + bool CanRead() const override; + bool CanWrite() const override; - bool IsValid() const; + int GetReadFileDescriptor() const override; + int GetWriteFileDescriptor() const override; + int ReleaseReadFileDescriptor() override; + int ReleaseWriteFileDescriptor() override; - bool ReadDescriptorIsValid() const; + void Close() override; - bool WriteDescriptorIsValid() const; + Error Write(const void *buf, size_t size, size_t &bytes_written) override; + Error Read(void *buf, size_t size, size_t &bytes_read) override; + Error ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &timeout, size_t &bytes_read) override; - int GetReadFileDescriptor() const; + // PipeWindows specific methods. These allow access to the underlying OS handle. + HANDLE GetReadNativeHandle(); + HANDLE GetWriteNativeHandle(); - int GetWriteFileDescriptor() const; - - // Close both descriptors - void Close(); - - bool CloseReadFileDescriptor(); - - bool CloseWriteFileDescriptor(); - - int ReleaseReadFileDescriptor(); - - int ReleaseWriteFileDescriptor(); - - HANDLE - GetReadNativeHandle(); - - HANDLE - GetWriteNativeHandle(); - - size_t Read(void *buf, size_t size); + private: + Error OpenNamedPipe(llvm::StringRef name, bool child_process_inherit, bool is_read); - size_t Write(const void *buf, size_t size); + void CloseReadEndpoint(); + void CloseWriteEndpoint(); - private: HANDLE m_read; HANDLE m_write; int m_read_fd; int m_write_fd; - OVERLAPPED *m_read_overlapped; - OVERLAPPED *m_write_overlapped; + OVERLAPPED m_read_overlapped; + OVERLAPPED m_write_overlapped; }; } // namespace lldb_private diff --git a/lldb/lldb.xcodeproj/project.pbxproj b/lldb/lldb.xcodeproj/project.pbxproj index b1424aa1ef08d..aa81d2c91f979 100644 --- a/lldb/lldb.xcodeproj/project.pbxproj +++ b/lldb/lldb.xcodeproj/project.pbxproj @@ -1794,6 +1794,7 @@ 26FFC19614FC072100087D58 /* DYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DYLDRendezvous.h; sourceTree = ""; }; 26FFC19714FC072100087D58 /* DynamicLoaderPOSIXDYLD.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DynamicLoaderPOSIXDYLD.cpp; sourceTree = ""; }; 26FFC19814FC072100087D58 /* DynamicLoaderPOSIXDYLD.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DynamicLoaderPOSIXDYLD.h; sourceTree = ""; }; + 3F5E8AF31A40D4A500A73232 /* PipeBase.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = PipeBase.h; path = include/lldb/Host/PipeBase.h; sourceTree = ""; }; 3FDFD6C3199C396E009756A7 /* FileAction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = FileAction.h; path = include/lldb/Target/FileAction.h; sourceTree = ""; }; 3FDFDDBC199C3A06009756A7 /* FileAction.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = FileAction.cpp; path = source/Target/FileAction.cpp; sourceTree = ""; }; 3FDFDDBE199D345E009756A7 /* FileCache.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = FileCache.cpp; path = source/Host/common/FileCache.cpp; sourceTree = ""; }; @@ -3700,6 +3701,7 @@ 232CB612191E00CC00EF39FC /* NativeThreadProtocol.h */, A36FF33D17D8E98800244D40 /* OptionParser.h */, 260A39A519647A3A004B4130 /* Pipe.h */, + 3F5E8AF31A40D4A500A73232 /* PipeBase.h */, 26BC7DD610F1B7D500F91463 /* Predicate.h */, 3FDFED2219BA6D55009756A7 /* ProcessRunLock.h */, 236124A71986B50E004EFC37 /* Socket.h */, diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index c0f13f6024090..38ddc0a492201 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -96,11 +96,12 @@ ConnectionFileDescriptor::OpenCommandPipe() Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); // Make the command file descriptor here: - if (!m_pipe.Open(m_child_processes_inherit)) + Error result = m_pipe.CreateNew(m_child_processes_inherit); + if (!result.Success()) { if (log) log->Printf("%p ConnectionFileDescriptor::OpenCommandPipe () - could not make pipe: %s", static_cast(this), - strerror(errno)); + result.AsCString()); } else { @@ -291,7 +292,9 @@ ConnectionFileDescriptor::Connect(const char *s, Error *error_ptr) bool ConnectionFileDescriptor::InterruptRead() { - return m_pipe.Write("i", 1) == 1; + size_t bytes_written = 0; + Error result = m_pipe.Write("i", 1, bytes_written); + return result.Success(); } ConnectionStatus @@ -325,13 +328,13 @@ ConnectionFileDescriptor::Disconnect(Error *error_ptr) if (!got_lock) { - if (m_pipe.WriteDescriptorIsValid()) + if (m_pipe.CanWrite()) { - int result; - result = m_pipe.Write("q", 1) == 1; + size_t bytes_written = 0; + Error result = m_pipe.Write("q", 1, bytes_written); if (log) - log->Printf("%p ConnectionFileDescriptor::Disconnect(): Couldn't get the lock, sent 'q' to %d, result = %d.", - static_cast(this), m_pipe.GetWriteFileDescriptor(), result); + log->Printf("%p ConnectionFileDescriptor::Disconnect(): Couldn't get the lock, sent 'q' to %d, error = '%s'.", + static_cast(this), m_pipe.GetWriteFileDescriptor(), result.AsCString()); } else if (log) { diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp index bf6863c541112..1dbba96b9b123 100644 --- a/lldb/source/Host/posix/PipePosix.cpp +++ b/lldb/source/Host/posix/PipePosix.cpp @@ -9,18 +9,25 @@ #include "lldb/Host/posix/PipePosix.h" -#include +#include #include +#include +#include +using namespace lldb; using namespace lldb_private; -int Pipe::kInvalidDescriptor = -1; +int PipePosix::kInvalidDescriptor = -1; enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE // pipe2 is supported by Linux, FreeBSD v10 and higher. // TODO: Add more platforms that support pipe2. -#define PIPE2_SUPPORTED defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD__ >= 10) +#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD__ >= 10) +#define PIPE2_SUPPORTED 1 +#else +#define PIPE2_SUPPORTED 0 +#endif namespace { @@ -37,26 +44,33 @@ bool SetCloexecFlag(int fd) } -Pipe::Pipe() +PipePosix::PipePosix() { - m_fds[READ] = Pipe::kInvalidDescriptor; - m_fds[WRITE] = Pipe::kInvalidDescriptor; + m_fds[READ] = PipePosix::kInvalidDescriptor; + m_fds[WRITE] = PipePosix::kInvalidDescriptor; } -Pipe::~Pipe() +PipePosix::~PipePosix() { Close(); } -bool -Pipe::Open(bool child_processes_inherit) +Error +PipePosix::CreateNew(bool child_processes_inherit) { - if (IsValid()) - return true; + Error error; + if (CanRead() || CanWrite()) + { + error.SetError(EINVAL, eErrorTypePOSIX); + return error; + } #if PIPE2_SUPPORTED if (::pipe2(m_fds, (child_processes_inherit) ? 0 : O_CLOEXEC) == 0) - return true; + { + error.SetErrorToErrno(); + return error; + } #else if (::pipe(m_fds) == 0) { @@ -65,118 +79,177 @@ Pipe::Open(bool child_processes_inherit) { if (!SetCloexecFlag(m_fds[0]) || !SetCloexecFlag(m_fds[1])) { + error.SetErrorToErrno(); Close(); - return false; + return error; } } #endif - return true; + return error; } #endif - m_fds[READ] = Pipe::kInvalidDescriptor; - m_fds[WRITE] = Pipe::kInvalidDescriptor; - return false; + m_fds[READ] = PipePosix::kInvalidDescriptor; + m_fds[WRITE] = PipePosix::kInvalidDescriptor; + error.SetErrorToErrno(); + return error; +} + +Error +PipePosix::CreateNew(llvm::StringRef name, bool child_process_inherit) +{ + Error error; + if (CanRead() || CanWrite()) + error.SetErrorString("Pipe is already opened"); + else if (name.empty()) + error.SetErrorString("Cannot create named pipe with empty name."); + else + error.SetErrorString("Not implemented"); + return error; +} + +Error +PipePosix::OpenAsReader(llvm::StringRef name, bool child_process_inherit) +{ + Error error; + if (CanRead() || CanWrite()) + error.SetErrorString("Pipe is already opened"); + else if (name.empty()) + error.SetErrorString("Cannot open named pipe with empty name."); + else + error.SetErrorString("Not implemented"); + return error; +} + +Error +PipePosix::OpenAsWriter(llvm::StringRef name, bool child_process_inherit) +{ + Error error; + if (CanRead() || CanWrite()) + error.SetErrorString("Pipe is already opened"); + else if (name.empty()) + error.SetErrorString("Cannot create named pipe with empty name."); + else + error.SetErrorString("Not implemented"); + return error; } int -Pipe::GetReadFileDescriptor() const +PipePosix::GetReadFileDescriptor() const { return m_fds[READ]; } int -Pipe::GetWriteFileDescriptor() const +PipePosix::GetWriteFileDescriptor() const { return m_fds[WRITE]; } int -Pipe::ReleaseReadFileDescriptor() +PipePosix::ReleaseReadFileDescriptor() { const int fd = m_fds[READ]; - m_fds[READ] = Pipe::kInvalidDescriptor; + m_fds[READ] = PipePosix::kInvalidDescriptor; return fd; } int -Pipe::ReleaseWriteFileDescriptor() +PipePosix::ReleaseWriteFileDescriptor() { const int fd = m_fds[WRITE]; - m_fds[WRITE] = Pipe::kInvalidDescriptor; + m_fds[WRITE] = PipePosix::kInvalidDescriptor; return fd; } void -Pipe::Close() +PipePosix::Close() { CloseReadFileDescriptor(); CloseWriteFileDescriptor(); } bool -Pipe::ReadDescriptorIsValid() const -{ - return m_fds[READ] != Pipe::kInvalidDescriptor; -} - -bool -Pipe::WriteDescriptorIsValid() const +PipePosix::CanRead() const { - return m_fds[WRITE] != Pipe::kInvalidDescriptor; + return m_fds[READ] != PipePosix::kInvalidDescriptor; } bool -Pipe::IsValid() const +PipePosix::CanWrite() const { - return ReadDescriptorIsValid() && WriteDescriptorIsValid(); + return m_fds[WRITE] != PipePosix::kInvalidDescriptor; } -bool -Pipe::CloseReadFileDescriptor() +void +PipePosix::CloseReadFileDescriptor() { - if (ReadDescriptorIsValid()) + if (CanRead()) { int err; err = close(m_fds[READ]); - m_fds[READ] = Pipe::kInvalidDescriptor; - return err == 0; + m_fds[READ] = PipePosix::kInvalidDescriptor; } - return true; } -bool -Pipe::CloseWriteFileDescriptor() +void +PipePosix::CloseWriteFileDescriptor() { - if (WriteDescriptorIsValid()) + if (CanWrite()) { int err; err = close(m_fds[WRITE]); - m_fds[WRITE] = Pipe::kInvalidDescriptor; - return err == 0; + m_fds[WRITE] = PipePosix::kInvalidDescriptor; } - return true; } - -size_t -Pipe::Read (void *buf, size_t num_bytes) +Error +PipePosix::Read(void *buf, size_t num_bytes, size_t &bytes_read) { - if (ReadDescriptorIsValid()) + bytes_read = 0; + Error error; + + if (CanRead()) { const int fd = GetReadFileDescriptor(); - return read (fd, buf, num_bytes); + int result = read(fd, buf, num_bytes); + if (result >= 0) + bytes_read = result; + else + error.SetErrorToErrno(); } - return 0; // Return 0 since errno won't be set if we didn't call read + else + error.SetError(EINVAL, eErrorTypePOSIX); + + return error; +} + +Error +PipePosix::ReadWithTimeout(void *buf, size_t num_bytes, const std::chrono::milliseconds &duration, size_t &bytes_read) +{ + bytes_read = 0; + Error error; + error.SetErrorString("Not implemented"); + return error; } -size_t -Pipe::Write (const void *buf, size_t num_bytes) +Error +PipePosix::Write(const void *buf, size_t num_bytes, size_t &bytes_written) { - if (WriteDescriptorIsValid()) + bytes_written = 0; + Error error; + + if (CanWrite()) { const int fd = GetWriteFileDescriptor(); - return write (fd, buf, num_bytes); + int result = write(fd, buf, num_bytes); + if (result >= 0) + bytes_written = result; + else + error.SetErrorToErrno(); } - return 0; // Return 0 since errno won't be set if we didn't call write + else + error.SetError(EINVAL, eErrorTypePOSIX); + + return error; } diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index 572be87840919..e49bbe026f08d 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -17,6 +17,7 @@ #include #include +using namespace lldb; using namespace lldb_private; namespace @@ -24,208 +25,282 @@ namespace std::atomic g_pipe_serial(0); } -Pipe::Pipe() +PipeWindows::PipeWindows() { m_read = INVALID_HANDLE_VALUE; m_write = INVALID_HANDLE_VALUE; m_read_fd = -1; m_write_fd = -1; - - m_read_overlapped = nullptr; - m_write_overlapped = nullptr; + ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); } -Pipe::~Pipe() +PipeWindows::~PipeWindows() { Close(); } -bool -Pipe::Open(bool read_overlapped, bool write_overlapped) +Error +PipeWindows::CreateNew(bool child_process_inherit) { - if (IsValid()) - return true; - + // 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. uint32_t serial = g_pipe_serial.fetch_add(1); std::string pipe_name; llvm::raw_string_ostream pipe_name_stream(pipe_name); - pipe_name_stream << "\\\\.\\Pipe\\lldb.pipe." << ::GetCurrentProcessId() << "." << serial; + pipe_name_stream << "lldb.pipe." << ::GetCurrentProcessId() << "." << serial; pipe_name_stream.flush(); - DWORD read_mode = 0; - DWORD write_mode = 0; - if (read_overlapped) - read_mode |= FILE_FLAG_OVERLAPPED; - if (write_overlapped) - write_mode |= FILE_FLAG_OVERLAPPED; + return CreateNew(pipe_name.c_str(), child_process_inherit); +} + +Error +PipeWindows::CreateNew(llvm::StringRef name, bool child_process_inherit) +{ + if (name.empty()) + return Error(ERROR_INVALID_PARAMETER, eErrorTypeWin32); + + if (CanRead() || CanWrite()) + return Error(ERROR_ALREADY_EXISTS, eErrorTypeWin32); + + std::string pipe_path = "\\\\.\\Pipe\\"; + pipe_path.append(name); + + // Always open for overlapped i/o. We implement blocking manually in Read and Write. + DWORD read_mode = FILE_FLAG_OVERLAPPED; m_read = - ::CreateNamedPipe(pipe_name.c_str(), PIPE_ACCESS_INBOUND | read_mode, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 1024, 1024, 120 * 1000, NULL); + ::CreateNamedPipe(pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 1024, 1024, 120 * 1000, NULL); if (INVALID_HANDLE_VALUE == m_read) - return false; - m_write = ::CreateFile(pipe_name.c_str(), GENERIC_WRITE, 0, NULL, OPEN_EXISTING, write_mode, NULL); - if (INVALID_HANDLE_VALUE == m_write) + return Error(::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 = ::CreateEvent(nullptr, TRUE, FALSE, nullptr); + + // Open the write end of the pipe. + Error result = OpenNamedPipe(name, child_process_inherit, false); + if (!result.Success()) { - ::CloseHandle(m_read); - m_read = INVALID_HANDLE_VALUE; - return false; + CloseReadEndpoint(); + return result; } - m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY); - m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); + return result; +} + +Error +PipeWindows::OpenAsReader(llvm::StringRef name, bool child_process_inherit) +{ + if (CanRead() || CanWrite()) + return Error(ERROR_ALREADY_EXISTS, eErrorTypeWin32); + + return OpenNamedPipe(name, child_process_inherit, true); +} - if (read_overlapped) +Error +PipeWindows::OpenAsWriter(llvm::StringRef name, bool child_process_inherit) +{ + if (CanRead() || CanWrite()) + return Error(ERROR_ALREADY_EXISTS, eErrorTypeWin32); + + return OpenNamedPipe(name, child_process_inherit, false); +} + +Error +PipeWindows::OpenNamedPipe(llvm::StringRef name, bool child_process_inherit, bool is_read) +{ + if (name.empty()) + return Error(ERROR_INVALID_PARAMETER, eErrorTypeWin32); + + assert(is_read ? !CanRead() : !CanWrite()); + + SECURITY_ATTRIBUTES attributes = {0}; + attributes.bInheritHandle = child_process_inherit; + + std::string pipe_path = "\\\\.\\Pipe\\"; + pipe_path.append(name); + + if (is_read) { - m_read_overlapped = new OVERLAPPED; - ZeroMemory(m_read_overlapped, sizeof(OVERLAPPED)); + m_read = ::CreateFile(pipe_path.c_str(), GENERIC_READ, 0, &attributes, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); + if (INVALID_HANDLE_VALUE == m_read) + return Error(::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 = ::CreateEvent(nullptr, TRUE, FALSE, nullptr); } - if (write_overlapped) + else { - m_write_overlapped = new OVERLAPPED; - ZeroMemory(m_write_overlapped, sizeof(OVERLAPPED)); + m_write = ::CreateFile(pipe_path.c_str(), GENERIC_WRITE, 0, &attributes, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); + if (INVALID_HANDLE_VALUE == m_write) + return Error(::GetLastError(), eErrorTypeWin32); + + m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); + + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); } - return true; + + return Error(); } int -Pipe::GetReadFileDescriptor() const +PipeWindows::GetReadFileDescriptor() const { return m_read_fd; } int -Pipe::GetWriteFileDescriptor() const +PipeWindows::GetWriteFileDescriptor() const { return m_write_fd; } int -Pipe::ReleaseReadFileDescriptor() +PipeWindows::ReleaseReadFileDescriptor() { + if (!CanRead()) + return -1; int result = m_read_fd; m_read_fd = -1; + if (m_read_overlapped.hEvent) + ::CloseHandle(m_read_overlapped.hEvent); m_read = INVALID_HANDLE_VALUE; - if (m_read_overlapped) - { - delete m_read_overlapped; - m_read_overlapped = nullptr; - } + ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); return result; } int -Pipe::ReleaseWriteFileDescriptor() +PipeWindows::ReleaseWriteFileDescriptor() { + if (!CanWrite()) + return -1; int result = m_write_fd; m_write_fd = -1; m_write = INVALID_HANDLE_VALUE; - if (m_write_overlapped) - { - delete m_write_overlapped; - m_write_overlapped = nullptr; - } + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); return result; } void -Pipe::Close() +PipeWindows::CloseReadEndpoint() { - CloseReadFileDescriptor(); - CloseWriteFileDescriptor(); -} + if (!CanRead()) + return; -bool -Pipe::ReadDescriptorIsValid() const -{ - return m_read_fd != -1; + if (m_read_overlapped.hEvent) + ::CloseHandle(m_read_overlapped.hEvent); + _close(m_read_fd); + m_read = INVALID_HANDLE_VALUE; + m_read_fd = -1; + ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped)); } -bool -Pipe::WriteDescriptorIsValid() const +void +PipeWindows::CloseWriteEndpoint() { - return m_write_fd != -1; + if (!CanWrite()) + return; + + _close(m_write_fd); + m_write = INVALID_HANDLE_VALUE; + m_write_fd = -1; + ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped)); } -bool -Pipe::IsValid() const +void +PipeWindows::Close() { - return ReadDescriptorIsValid() && WriteDescriptorIsValid(); + CloseReadEndpoint(); + CloseWriteEndpoint(); } bool -Pipe::CloseReadFileDescriptor() +PipeWindows::CanRead() const { - if (ReadDescriptorIsValid()) - { - int err; - err = _close(m_read_fd); - m_read_fd = -1; - m_read = INVALID_HANDLE_VALUE; - if (m_read_overlapped) - { - delete m_read_overlapped; - m_read_overlapped = nullptr; - } - return err == 0; - } - return true; + return (m_read != INVALID_HANDLE_VALUE); } bool -Pipe::CloseWriteFileDescriptor() +PipeWindows::CanWrite() const { - if (WriteDescriptorIsValid()) - { - int err; - err = _close(m_write_fd); - m_write_fd = -1; - m_write = INVALID_HANDLE_VALUE; - if (m_write_overlapped) - { - delete m_write_overlapped; - m_write_overlapped = nullptr; - } - return err == 0; - } - return true; + return (m_write != INVALID_HANDLE_VALUE); } HANDLE -Pipe::GetReadNativeHandle() +PipeWindows::GetReadNativeHandle() { return m_read; } HANDLE -Pipe::GetWriteNativeHandle() +PipeWindows::GetWriteNativeHandle() { return m_write; } -size_t -Pipe::Read(void *buf, size_t num_bytes) +Error +PipeWindows::Read(void *buf, size_t size, size_t &bytes_read) { - if (ReadDescriptorIsValid()) - { - DWORD bytes_read = 0; - ::ReadFile(m_read, buf, num_bytes, &bytes_read, m_read_overlapped); - if (m_read_overlapped) - GetOverlappedResult(m_read, m_read_overlapped, &bytes_read, TRUE); - return bytes_read; - } - return 0; // Return 0 since errno won't be set if we didn't call read + return ReadWithTimeout(buf, size, std::chrono::milliseconds::zero(), bytes_read); } -size_t -Pipe::Write(const void *buf, size_t num_bytes) +Error +PipeWindows::ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &duration, size_t &bytes_read) { - if (WriteDescriptorIsValid()) + if (!CanRead()) + return Error(ERROR_INVALID_HANDLE, eErrorTypeWin32); + + bytes_read = 0; + 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 Error(::GetLastError(), eErrorTypeWin32); + + DWORD timeout = (duration == std::chrono::milliseconds::zero()) ? INFINITE : duration.count(); + DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout); + if (wait_result != WAIT_OBJECT_0) { - DWORD bytes_written = 0; - ::WriteFile(m_write, buf, num_bytes, &bytes_written, m_read_overlapped); - if (m_write_overlapped) - GetOverlappedResult(m_write, m_write_overlapped, &bytes_written, TRUE); - return bytes_written; + // 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 Error(failure_error, eErrorTypeWin32); } - return 0; // Return 0 since errno won't be set if we didn't call write + + // 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 Error(::GetLastError(), eErrorTypeWin32); + + bytes_read = sys_bytes_read; + return Error(); +} + +Error +PipeWindows::Write(const void *buf, size_t num_bytes, size_t &bytes_written) +{ + if (!CanWrite()) + return Error(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 Error(::GetLastError(), eErrorTypeWin32); + + BOOL result = GetOverlappedResult(m_write, &m_write_overlapped, &sys_bytes_written, TRUE); + if (!result) + return Error(::GetLastError(), eErrorTypeWin32); + return Error(); } diff --git a/lldb/source/Interpreter/ScriptInterpreterPython.cpp b/lldb/source/Interpreter/ScriptInterpreterPython.cpp index d53a16bc82007..cf7a3019268c8 100644 --- a/lldb/source/Interpreter/ScriptInterpreterPython.cpp +++ b/lldb/source/Interpreter/ScriptInterpreterPython.cpp @@ -603,18 +603,14 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec // Set output to a temporary file so we can forward the results on to the result object Pipe pipe; -#if defined(_WIN32) - // By default Windows does not create a pipe object that can be used for a non-blocking read. - // We must explicitly request it. Furthermore, we can't use an fd for non-blocking read - // operations, and must use the native os HANDLE. - if (pipe.Open(true, false)) + Error pipe_result = pipe.CreateNew(false); + if (pipe_result.Success()) { +#if defined(_WIN32) lldb::file_t read_file = pipe.GetReadNativeHandle(); pipe.ReleaseReadFileDescriptor(); std::unique_ptr conn_ap(new ConnectionGenericFile(read_file, true)); #else - if (pipe.Open()) - { std::unique_ptr conn_ap(new ConnectionFileDescriptor(pipe.ReleaseReadFileDescriptor(), true)); #endif if (conn_ap->IsConnected()) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 207f07015a4cb..936e2aaff311e 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4927,9 +4927,10 @@ class IOHandlerProcessSTDIO : bool OpenPipes () { - if (m_pipe.IsValid()) + if (m_pipe.CanRead() && m_pipe.CanWrite()) return true; - return m_pipe.Open(); + Error result = m_pipe.CreateNew(false); + return result.Success(); } void @@ -4990,8 +4991,10 @@ class IOHandlerProcessSTDIO : } if (FD_ISSET (pipe_read_fd, &read_fdset)) { + size_t bytes_read; // Consume the interrupt byte - if (m_pipe.Read (&ch, 1) == 1) + Error error = m_pipe.Read(&ch, 1, bytes_read); + if (error.Success()) { switch (ch) { @@ -5039,7 +5042,8 @@ class IOHandlerProcessSTDIO : Cancel () { char ch = 'q'; // Send 'q' for quit - m_pipe.Write (&ch, 1); + size_t bytes_written = 0; + m_pipe.Write(&ch, 1, bytes_written); } virtual bool @@ -5053,7 +5057,9 @@ class IOHandlerProcessSTDIO : if (m_active) { char ch = 'i'; // Send 'i' for interrupt - return m_pipe.Write (&ch, 1) == 1; + size_t bytes_written = 0; + Error result = m_pipe.Write(&ch, 1, bytes_written); + return result.Success(); } else {