Index: include/lldb/Host/File.h =================================================================== --- include/lldb/Host/File.h +++ include/lldb/Host/File.h @@ -11,11 +11,12 @@ #define liblldb_File_h_ // C Includes -// C++ Includes #include #include #include - +// C++ Includes +#include +#include // Other libraries and framework includes // Project includes #include "lldb/lldb-private.h" @@ -438,6 +439,68 @@ Error Write (const void *src, size_t &num_bytes, off_t &offset); + class SelectInfo + { + public: + + // Defaults to infinite wait for select unless you call SetTimeout() + SelectInfo(); + + // Call SetTimeout() before calling SelectInfo::Select() to set the + // timeout based on the current time + the timeout. This allows multiple + // calls to SelectInfo::Select() without having to worry about the + // absolute timeout as this class manages to set the relative timeout + // correctly. + void SetTimeout(const std::chrono::microseconds &timeout); + + // Call the Check*() functions before calling SelectInfo::Select() to + // set the file descriptors that we will watch for when calling + // select + void CheckRead(int fd); + void CheckWrite(int fd); + void CheckError(int fd); + // Call the *IsReady() functions after calling SelectInfo::Select() + // to check which file descriptors are ready for read/write/error + bool ReadIsReady(int fd) const; + bool WriteIsReady(int fd) const; + bool ErrorIsReady(int fd) const; + + // Call the system's select() to wait for descriptors using + // timeout provided in a call the SelectInfo::SetTimeout(), + // or infinite wait if no timeout was set. + Error Select(); + protected: + struct FDInfo + { + FDInfo() : + read_check(false), + write_check(false), + error_check(false), + read_ready(false), + write_ready(false), + error_ready(false) + { + } + + void + PrepareForSelect() + { + read_ready = false; + write_ready = false; + error_ready = false; + } + + bool read_check; + bool write_check; + bool error_check; + bool read_ready; + bool write_ready; + bool error_ready; + }; + std::map m_fd_map; + std::unique_ptr m_end_time_ap; + }; + //------------------------------------------------------------------ /// Flush the current stream /// Index: source/Host/common/Editline.cpp =================================================================== --- source/Host/common/Editline.cpp +++ source/Host/common/Editline.cpp @@ -16,6 +16,7 @@ #include "lldb/Core/Error.h" #include "lldb/Core/StringList.h" #include "lldb/Core/StreamString.h" +#include "lldb/Host/File.h" #include "lldb/Host/FileSpec.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Host.h" @@ -155,11 +156,10 @@ // instead use some kind of yet-to-be-created abstraction that select-like functionality on // non-socket objects. const int fd = fileno (file); - fd_set fds; - FD_ZERO (&fds); - FD_SET (fd, &fds); - timeval timeout = { 0, 0 }; - return select (fd + 1, &fds, NULL, NULL, &timeout); + File::SelectInfo select_info; + select_info.SetTimeout(std::chrono::microseconds(0)); + select_info.CheckRead(fd); + return select_info.Select().Success(); } namespace lldb_private Index: source/Host/common/File.cpp =================================================================== --- source/Host/common/File.cpp +++ source/Host/common/File.cpp @@ -7,6 +7,13 @@ // //===----------------------------------------------------------------------===// +#if defined(__APPLE__) +// Enable this special support for Apple builds where we can have unlimited +// select bounds. We tried switching to poll() and kqueue and we were panicing +// the kernel, so we have to stick with select for now. +#define _DARWIN_UNLIMITED_SELECT +#endif + #include "lldb/Host/File.h" #include @@ -21,6 +28,9 @@ #include #endif +#include + +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/Process.h" // for llvm::sys::Process::FileDescriptorHasColors() @@ -30,6 +40,7 @@ #include "lldb/Host/Config.h" #include "lldb/Host/FileSpec.h" #include "lldb/Host/FileSystem.h" +#include "lldb/Utility/LLDBAssert.h" using namespace lldb; using namespace lldb_private; @@ -934,6 +945,264 @@ return error; } +File::SelectInfo::SelectInfo() : + m_fd_map(), + m_end_time_ap() // Infinite timeout unless File::SelectInfo::SetTimeout() gets called +{ +} + +void +File::SelectInfo::SetTimeout(const std::chrono::microseconds &timeout) +{ + using namespace std::chrono; + m_end_time_ap.reset(new steady_clock::time_point(steady_clock::now() + timeout)); +} + +void +File::SelectInfo::CheckRead(int fd) +{ + m_fd_map[fd].read_check = true; +} + +void +File::SelectInfo::CheckWrite(int fd) +{ + m_fd_map[fd].write_check = true; +} + +void +File::SelectInfo::CheckError(int fd) +{ + m_fd_map[fd].error_check = true; +} + +bool +File::SelectInfo::ReadIsReady(int fd) const +{ + auto pos = m_fd_map.find(fd); + if (pos != m_fd_map.end()) + return pos->second.read_ready; + else + return false; +} + +bool +File::SelectInfo::WriteIsReady(int fd) const +{ + auto pos = m_fd_map.find(fd); + if (pos != m_fd_map.end()) + return pos->second.write_ready; + else + return false; +} + +bool +File::SelectInfo::ErrorIsReady(int fd) const +{ + auto pos = m_fd_map.find(fd); + if (pos != m_fd_map.end()) + return pos->second.error_ready; + else + return false; +} + +Error +File::SelectInfo::Select() +{ + Error error; + + int max_read_fd = -1; + int max_write_fd = -1; + int max_error_fd = -1; + int max_fd = -1; + for (auto &pair : m_fd_map) + { + pair.second.PrepareForSelect(); + const int fd = pair.first; +#if !defined(__APPLE__) + lldbassert(fd < FD_SETSIZE); + if (fd >= FD_SETSIZE) + { + error.SetErrorStringWithFormat("%i is too large for select()", fd); + return error; + } +#endif + if (pair.second.read_check) + { + max_read_fd = std::max(fd, max_read_fd); + max_fd = std::max(fd, max_fd); + } + if (pair.second.write_check) + { + max_write_fd = std::max(fd, max_write_fd); + max_fd = std::max(fd, max_fd); + } + if (pair.second.error_check) + { + max_error_fd = std::max(fd, max_error_fd); + max_fd = std::max(fd, max_fd); + } + } + + if (max_fd == -1) + { + error.SetErrorString("no valid file descriptors"); + return error; + } + + const int nfds = max_fd + 1; + fd_set *read_fdset_ptr = nullptr; + fd_set *write_fdset_ptr = nullptr; + fd_set *error_fdset_ptr = nullptr; + //---------------------------------------------------------------------- + // Initialize and zero out the fdsets + //---------------------------------------------------------------------- +#if defined(__APPLE__) + llvm::SmallVector read_fdset; + llvm::SmallVector write_fdset; + llvm::SmallVector error_fdset; + + if (max_read_fd >= 0) + { + read_fdset.resize((nfds / FD_SETSIZE) + 1); + read_fdset_ptr = read_fdset.data(); + } + if (max_write_fd >= 0) + { + write_fdset.resize((nfds / FD_SETSIZE) + 1); + write_fdset_ptr = write_fdset.data(); + } + if (max_error_fd >= 0) + { + error_fdset.resize((nfds / FD_SETSIZE) + 1); + error_fdset_ptr = error_fdset.data(); + } + for (auto &fd_set : read_fdset) + FD_ZERO(&fd_set); + for (auto &fd_set : write_fdset) + FD_ZERO(&fd_set); + for (auto &fd_set : error_fdset) + FD_ZERO(&fd_set); +#else + fd_set read_fdset; + fd_set write_fdset; + fd_set error_fdset; + + if (max_read_fd >= 0) + { + FD_ZERO(&read_fdset); + read_fdset_ptr = &read_fdset; + } + if (max_write_fd >= 0) + { + FD_ZERO(&write_fdset); + write_fdset_ptr = &write_fdset; + } + if (max_error_fd >= 0) + { + FD_ZERO(&error_fdset); + error_fdset_ptr = &error_fdset; + } +#endif + //---------------------------------------------------------------------- + // Set the FD bits in the fdsets for read/write/error + //---------------------------------------------------------------------- + for (auto &pair : m_fd_map) + { + const int fd = pair.first; + + if (pair.second.read_check) + FD_SET(fd, read_fdset_ptr); + + if (pair.second.write_check) + FD_SET(fd, write_fdset_ptr); + + if (pair.second.error_check) + FD_SET(fd, error_fdset_ptr); + } + + //---------------------------------------------------------------------- + // Setup our timeout time value if needed + //---------------------------------------------------------------------- + struct timeval *tv_ptr = nullptr; + struct timeval tv = {0, 0}; + + while (1) + { + using namespace std::chrono; + //------------------------------------------------------------------ + // Setup out relative timeout based on the end time if we have one + //------------------------------------------------------------------ + if (m_end_time_ap) + { + tv_ptr = &tv; + const auto remaining_dur = duration_cast(*m_end_time_ap - steady_clock::now()); + if (remaining_dur.count() > 0) + { + // Wait for a specific amount of time + const auto dur_secs = duration_cast(remaining_dur); + const auto dur_usecs = remaining_dur % seconds(1); + tv.tv_sec = dur_secs.count(); + tv.tv_usec = dur_usecs.count(); + } + else + { + // Just poll once with no timeout + tv.tv_sec = 0; + tv.tv_usec = 0; + } + } + const int num_set_fds = ::select(nfds, read_fdset_ptr, write_fdset_ptr, error_fdset_ptr, tv_ptr); + if (num_set_fds < 0) + { + // We got an error + error.SetErrorToErrno(); + if (error.GetError() == EINTR) + { + error.Clear(); + continue; // Keep calling select if we get EINTR + } + else + return error; + } + else if (num_set_fds == 0) + { + // Timeout + error.SetError(ETIMEDOUT, eErrorTypePOSIX); + error.SetErrorString("timed out"); + return error; + } + else + { + // One or more descriptors were set, update the FDInfo::select_ready mask + // so users can ask the SelectInfo class so clients can call one of: + + for (auto &pair : m_fd_map) + { + const int fd = pair.first; + + if (pair.second.read_check) + { + if (FD_ISSET(fd, read_fdset_ptr)) + pair.second.read_ready = true; + } + if (pair.second.write_check) + { + if (FD_ISSET(fd, write_fdset_ptr)) + pair.second.write_ready = true; + } + if (pair.second.error_check) + { + if (FD_ISSET(fd, error_fdset_ptr)) + pair.second.error_ready = true; + } + } + break; + } + } + return error; +} + //------------------------------------------------------------------ // Print some formatted output to the stream. //------------------------------------------------------------------ Index: source/Host/posix/ConnectionFileDescriptorPosix.cpp =================================================================== --- source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -16,6 +16,7 @@ #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h" #include "lldb/Host/Config.h" +#include "lldb/Host/File.h" #include "lldb/Host/IOObject.h" #include "lldb/Host/SocketAddress.h" #include "lldb/Host/Socket.h" @@ -572,7 +573,7 @@ return m_uri; } -// This ConnectionFileDescriptor::BytesAvailable() uses select(). +// This ConnectionFileDescriptor::BytesAvailable() uses select() (in File::SelectInfo) // // PROS: // - select is consistent across most unix platforms @@ -586,11 +587,6 @@ // be used or a new version of ConnectionFileDescriptor::BytesAvailable() // should be written for the system that is running into the limitations. -#if defined(__APPLE__) -#define FD_SET_DATA(fds) fds.data() -#else -#define FD_SET_DATA(fds) &fds -#endif ConnectionStatus ConnectionFileDescriptor::BytesAvailable(uint32_t timeout_usec, Error *error_ptr) @@ -602,21 +598,6 @@ if (log) log->Printf("%p ConnectionFileDescriptor::BytesAvailable (timeout_usec = %u)", static_cast(this), timeout_usec); - struct timeval *tv_ptr; - struct timeval tv; - if (timeout_usec == UINT32_MAX) - { - // Infinite wait... - tv_ptr = nullptr; - } - else - { - TimeValue time_value; - time_value.OffsetWithMicroSeconds(timeout_usec); - tv.tv_sec = time_value.seconds(); - tv.tv_usec = time_value.microseconds(); - tv_ptr = &tv; - } // Make a copy of the file descriptors to make sure we don't // have another thread change these values out from under us @@ -624,8 +605,14 @@ const IOObject::WaitableHandle handle = m_read_sp->GetWaitableHandle(); const int pipe_fd = m_pipe.GetReadFileDescriptor(); + if (handle != IOObject::kInvalidHandleValue) { + File::SelectInfo select_info; + if (timeout_usec != UINT32_MAX) + select_info.SetTimeout(std::chrono::microseconds(timeout_usec)); + + select_info.CheckRead(handle); #if defined(_MSC_VER) // select() won't accept pipes on Windows. The entire Windows codepath needs to be // converted over to using WaitForMultipleObjects and event HANDLEs, but for now at least @@ -633,63 +620,15 @@ const bool have_pipe_fd = false; #else const bool have_pipe_fd = pipe_fd >= 0; -#if !defined(__APPLE__) - assert(handle < FD_SETSIZE); +#endif if (have_pipe_fd) - assert(pipe_fd < FD_SETSIZE); -#endif -#endif + select_info.CheckRead(pipe_fd); + while (handle == m_read_sp->GetWaitableHandle()) { - const int nfds = std::max(handle, pipe_fd) + 1; -#if defined(__APPLE__) - llvm::SmallVector read_fds; - read_fds.resize((nfds / FD_SETSIZE) + 1); - for (size_t i = 0; i < read_fds.size(); ++i) - FD_ZERO(&read_fds[i]); -// FD_SET doesn't bounds check, it just happily walks off the end -// but we have taken care of making the extra storage with our -// SmallVector of fd_set objects -#else - fd_set read_fds; - FD_ZERO(&read_fds); -#endif - FD_SET(handle, FD_SET_DATA(read_fds)); - if (have_pipe_fd) - FD_SET(pipe_fd, FD_SET_DATA(read_fds)); - Error error; + Error error = select_info.Select(); - if (log) - { - if (have_pipe_fd) - log->Printf( - "%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i, %i}, NULL, NULL, timeout=%p)...", - static_cast(this), nfds, handle, pipe_fd, static_cast(tv_ptr)); - else - log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i}, NULL, NULL, timeout=%p)...", - static_cast(this), nfds, handle, static_cast(tv_ptr)); - } - - const int num_set_fds = ::select(nfds, FD_SET_DATA(read_fds), NULL, NULL, tv_ptr); - if (num_set_fds < 0) - error.SetErrorToErrno(); - else - error.Clear(); - - if (log) - { - if (have_pipe_fd) - log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i, %i}, NULL, NULL, timeout=%p) " - "=> %d, error = %s", - static_cast(this), nfds, handle, pipe_fd, static_cast(tv_ptr), num_set_fds, - error.AsCString()); - else - log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i}, NULL, NULL, timeout=%p) => " - "%d, error = %s", - static_cast(this), nfds, handle, static_cast(tv_ptr), num_set_fds, error.AsCString()); - } - if (error_ptr) *error_ptr = error; @@ -704,6 +643,9 @@ default: // Other unknown error return eConnectionStatusError; + case ETIMEDOUT: + return eConnectionStatusTimedOut; + case EAGAIN: // The kernel was (perhaps temporarily) unable to // allocate the requested number of file descriptors, // or we have non-blocking IO @@ -713,15 +655,12 @@ break; // Lets keep reading to until we timeout } } - else if (num_set_fds == 0) + else { - return eConnectionStatusTimedOut; - } - else if (num_set_fds > 0) - { - if (FD_ISSET(handle, FD_SET_DATA(read_fds))) + if (select_info.ReadIsReady(handle)) return eConnectionStatusSuccess; - if (have_pipe_fd && FD_ISSET(pipe_fd, FD_SET_DATA(read_fds))) + + if (select_info.ReadIsReady(pipe_fd)) { // There is an interrupt or exit command in the command pipe // Read the data from that pipe: Index: source/Host/posix/PipePosix.cpp =================================================================== --- source/Host/posix/PipePosix.cpp +++ source/Host/posix/PipePosix.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Host/posix/PipePosix.h" +#include "lldb/Host/File.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "llvm/ADT/SmallString.h" @@ -65,75 +66,8 @@ return std::chrono::steady_clock::now(); } -Error -SelectIO(int handle, bool is_read, const std::function &io_handler, const std::chrono::microseconds &timeout) -{ - Error error; - fd_set fds; - bool done = false; - - using namespace std::chrono; - - const auto finish_time = Now() + timeout; - - while (!done) - { - struct timeval tv = {0, 0}; - if (timeout != microseconds::zero()) - { - const auto remaining_dur = duration_cast(finish_time - Now()); - if (remaining_dur.count() <= 0) - { - error.SetErrorString("timeout exceeded"); - break; - } - const auto dur_secs = duration_cast(remaining_dur); - const auto dur_usecs = remaining_dur % seconds(1); - - tv.tv_sec = dur_secs.count(); - tv.tv_usec = dur_usecs.count(); - } - else - tv.tv_sec = 1; - - FD_ZERO(&fds); - FD_SET(handle, &fds); - - const auto retval = ::select(handle + 1, - (is_read) ? &fds : nullptr, - (is_read) ? nullptr : &fds, - nullptr, &tv); - if (retval == -1) - { - if (errno == EINTR) - continue; - error.SetErrorToErrno(); - break; - } - if (retval == 0) - { - error.SetErrorString("timeout exceeded"); - break; - } - if (!FD_ISSET(handle, &fds)) - { - error.SetErrorString("invalid state"); - break; - } - - error = io_handler(done); - if (error.Fail()) - { - if (error.GetError() == EINTR) - continue; - break; - } - } - return error; } -} - PipePosix::PipePosix() : m_fds{ PipePosix::kInvalidDescriptor, @@ -383,27 +317,33 @@ if (!CanRead()) return Error(EINVAL, eErrorTypePOSIX); - auto handle = GetReadFileDescriptor(); - return SelectIO(handle, - true, - [=, &bytes_read](bool &done) - { - Error error; - auto result = ::read(handle, - reinterpret_cast(buf) + bytes_read, - size - bytes_read); - if (result != -1) - { - bytes_read += result; - if (bytes_read == size || result == 0) - done = true; - } - else - error.SetErrorToErrno(); + const int fd = GetReadFileDescriptor(); - return error; - }, - timeout); + File::SelectInfo select_info; + select_info.SetTimeout(timeout); + select_info.CheckRead(fd); + + Error error; + while (error.Success()) + { + error = select_info.Select(); + if (error.Success()) + { + auto result = ::read(fd, reinterpret_cast(buf) + bytes_read, size - bytes_read); + if (result != -1) + { + bytes_read += result; + if (bytes_read == size || result == 0) + break; + } + else + { + error.SetErrorToErrno(); + break; + } + } + } + return error; } Error @@ -413,25 +353,29 @@ if (!CanWrite()) return Error(EINVAL, eErrorTypePOSIX); - auto handle = GetWriteFileDescriptor(); - return SelectIO(handle, - false, - [=, &bytes_written](bool &done) - { - Error error; - auto result = ::write(handle, - reinterpret_cast(buf) + bytes_written, - size - bytes_written); - if (result != -1) - { - bytes_written += result; - if (bytes_written == size) - done = true; - } - else - error.SetErrorToErrno(); + const int fd = GetWriteFileDescriptor(); + File::SelectInfo select_info; + select_info.SetTimeout(std::chrono::seconds(0)); + select_info.CheckWrite(fd); - return error; - }, - std::chrono::microseconds::zero()); + Error error; + while (error.Success()) + { + error = select_info.Select(); + if (error.Success()) + { + auto result = ::write(fd, reinterpret_cast(buf) + bytes_written, size - bytes_written); + if (result != -1) + { + bytes_written += result; + if (bytes_written == size) + break; + } + else + { + error.SetErrorToErrno(); + } + } + } + return error; } Index: source/Target/Process.cpp =================================================================== --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -29,6 +29,7 @@ #include "lldb/Expression/IRDynamicChecks.h" #include "lldb/Expression/UserExpression.h" #include "lldb/Host/ConnectionFileDescriptor.h" +#include "lldb/Host/File.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Host.h" #include "lldb/Host/HostInfo.h" @@ -4925,25 +4926,20 @@ m_is_running = true; while (!GetIsDone()) { - fd_set read_fdset; - FD_ZERO (&read_fdset); - FD_SET (read_fd, &read_fdset); - FD_SET (pipe_read_fd, &read_fdset); - const int nfds = std::max(read_fd, pipe_read_fd) + 1; - int num_set_fds = select(nfds, &read_fdset, nullptr, nullptr, nullptr); + File::SelectInfo select_info; + select_info.CheckRead(read_fd); + select_info.CheckRead(pipe_read_fd); + Error error = select_info.Select(); - if (num_set_fds < 0) + if (error.Fail()) { - const int select_errno = errno; - - if (select_errno != EINTR) - SetIsDone(true); + SetIsDone(true); } - else if (num_set_fds > 0) + else { char ch = 0; size_t n; - if (FD_ISSET (read_fd, &read_fdset)) + if (select_info.ReadIsReady(read_fd)) { n = 1; if (m_read_file.Read(&ch, n).Success() && n == 1) @@ -4954,7 +4950,7 @@ else SetIsDone(true); } - if (FD_ISSET (pipe_read_fd, &read_fdset)) + if (select_info.ReadIsReady(pipe_read_fd)) { size_t bytes_read; // Consume the interrupt byte