From 8df92678a1ea2be2f661daa55b8c2f0472c987d8 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Sat, 16 Feb 2013 22:53:04 +0000 Subject: [PATCH] Fixed a crasher when the ConnectionFileDescriptor was used in a process with over FD_SETSIZE (1024) files open. It would corrupt the stack and cause the stack checker to assert and kill the program. The final fix was to "#define _DARWIN_UNLIMITED_SELECT" at the top of the one and only file that uses select () in the LLDB codebase and then make an array of "fd_set" objects so they can handle more than 1024 file descriptors. The new code can handle as many file descriptors as a process can create. llvm-svn: 175378 --- lldb/source/Core/ConnectionFileDescriptor.cpp | 479 ++++++++++++++++++++++---- 1 file changed, 418 insertions(+), 61 deletions(-) diff --git a/lldb/source/Core/ConnectionFileDescriptor.cpp b/lldb/source/Core/ConnectionFileDescriptor.cpp index 91c2ff3..c5114a00 100644 --- a/lldb/source/Core/ConnectionFileDescriptor.cpp +++ b/lldb/source/Core/ConnectionFileDescriptor.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/Core/ConnectionFileDescriptor.h" // C Includes @@ -25,6 +32,9 @@ // C++ Includes // Other libraries and framework includes +#if defined(__APPLE__) +#include "llvm/ADT/SmallVector.h" +#endif // Project includes #include "lldb/lldb-private-log.h" #include "lldb/Interpreter/Args.h" @@ -464,7 +474,6 @@ ConnectionFileDescriptor::Read (void *dst, return 0; } - //Disconnect (NULL); return 0; } return bytes_read; @@ -590,6 +599,21 @@ ConnectionFileDescriptor::Write (const void *src, size_t src_len, ConnectionStat return bytes_sent; } + + +#if defined(__APPLE__) + +// This ConnectionFileDescriptor::BytesAvailable() uses select(). +// +// PROS: +// - select is consistent across most unix platforms +// - this Apple specific version allows for unlimited fds in the fd_sets by +// setting the _DARWIN_UNLIMITED_SELECT define prior to including the +// required header files. + +// CONS: +// - Darwin only + ConnectionStatus ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_ptr) { @@ -613,90 +637,423 @@ ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_pt tv = time_value.GetAsTimeVal(); tv_ptr = &tv; } - - while (m_fd_recv >= 0) + + // Make a copy of the file descriptors to make sure we don't + // have another thread change these values out from under us + // and cause problems in the loop below where like in FS_SET() + const int data_fd = m_fd_recv; + const int pipe_fd = m_pipe_read; + + if (data_fd >= 0) { - fd_set read_fds; - FD_ZERO (&read_fds); - FD_SET (m_fd_recv, &read_fds); - if (m_pipe_read != -1) - FD_SET (m_pipe_read, &read_fds); - int nfds = std::max(m_fd_recv, m_pipe_read) + 1; + const bool have_pipe_fd = pipe_fd >= 0; - Error error; - + while (data_fd == m_fd_recv) + { + const int nfds = std::max(data_fd, pipe_fd) + 1; + llvm::SmallVector read_fds; + read_fds.resize((nfds/FD_SETSIZE) + 1); + for (size_t i=0; iPrintf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i, %i}, NULL, NULL, timeout=%p)...", + this, nfds, data_fd, pipe_fd, tv_ptr); + else + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i}, NULL, NULL, timeout=%p)...", + this, nfds, data_fd, tv_ptr); + } + + const int num_set_fds = ::select (nfds, read_fds.data(), 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", + this, nfds, data_fd, pipe_fd, 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", + this, nfds, data_fd, tv_ptr, num_set_fds, error.AsCString()); + } + + if (error_ptr) + *error_ptr = error; + + if (error.Fail()) + { + switch (error.GetError()) + { + case EBADF: // One of the descriptor sets specified an invalid descriptor. + return eConnectionStatusLostConnection; + + case EINVAL: // The specified time limit is invalid. One of its components is negative or too large. + default: // Other unknown error + return eConnectionStatusError; + + case EAGAIN: // The kernel was (perhaps temporarily) unable to + // allocate the requested number of file descriptors, + // or we have non-blocking IO + case EINTR: // A signal was delivered before the time limit + // expired and before any of the selected events + // occurred. + break; // Lets keep reading to until we timeout + } + } + else if (num_set_fds == 0) + { + return eConnectionStatusTimedOut; + } + else if (num_set_fds > 0) + { + // FD_ISSET is happy to deal with a something larger than + // a single fd_set. + if (FD_ISSET(data_fd, read_fds.data())) + return eConnectionStatusSuccess; + if (have_pipe_fd && FD_ISSET(pipe_fd, read_fds.data())) + { + // We got a command to exit. Read the data from that pipe: + char buffer[16]; + ssize_t bytes_read; + + do + { + bytes_read = ::read (pipe_fd, buffer, sizeof(buffer)); + } while (bytes_read < 0 && errno == EINTR); + assert (bytes_read == 1 && buffer[0] == 'q'); + + if (log) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() got data: %*s from the command channel.", + this, (int) bytes_read, buffer); + + return eConnectionStatusEndOfFile; + } + } + } + } + + if (error_ptr) + error_ptr->SetErrorString("not connected"); + return eConnectionStatusLostConnection; +} - if (log) - log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds = %i, fd = %i, NULL, NULL, timeout = %p)...", - this, nfds, m_fd_recv, tv_ptr); +#else - const int num_set_fds = ::select (nfds, &read_fds, NULL, NULL, tv_ptr); - if (num_set_fds < 0) - error.SetErrorToErrno(); - else - error.Clear(); +// This ConnectionFileDescriptor::BytesAvailable() uses select(). +// +// PROS: +// - select is consistent across most unix platforms +// CONS: +// - only supports file descriptors up to FD_SETSIZE. This implementation +// will assert if it runs into that hard limit to let users know that +// another ConnectionFileDescriptor::BytesAvailable() should be used +// or a new version of ConnectionFileDescriptor::BytesAvailable() should +// be written for the system that is running into the limitations. MacOSX +// uses kqueues, and there is a poll() based implementation below. - if (log) - log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds = %i, fd = %i, NULL, NULL, timeout = %p) => %d, error = %s", - this, nfds, m_fd_recv, tv_ptr, num_set_fds, error.AsCString()); +ConnectionStatus +ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_ptr) +{ + // Don't need to take the mutex here separately since we are only called from Read. If we + // ever get used more generally we will need to lock here as well. + + LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION)); + if (log) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable (timeout_usec = %u)", this, timeout_usec); + struct timeval *tv_ptr; + struct timeval tv; + if (timeout_usec == UINT32_MAX) + { + // Infinite wait... + tv_ptr = NULL; + } + else + { + TimeValue time_value; + time_value.OffsetWithMicroSeconds (timeout_usec); + tv = time_value.GetAsTimeVal(); + 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 + // and cause problems in the loop below where like in FS_SET() + const int data_fd = m_fd_recv; + const int pipe_fd = m_pipe_read; - if (error_ptr) - *error_ptr = error; + if (data_fd >= 0) + { + // If this assert fires off on MacOSX, we will need to switch to using + // libdispatch to read from file descriptors because poll() is causing + // kernel panics and if we exceed FD_SETSIZE we will have no choice... + assert (data_fd < FD_SETSIZE); + + const bool have_pipe_fd = pipe_fd >= 0; + + if (have_pipe_fd) + { + assert (pipe_fd < FD_SETSIZE); + } - if (error.Fail()) + while (data_fd == m_fd_recv) { - switch (error.GetError()) - { - case EBADF: // One of the descriptor sets specified an invalid descriptor. - return eConnectionStatusLostConnection; + fd_set read_fds; + FD_ZERO (&read_fds); + FD_SET (data_fd, &read_fds); + if (have_pipe_fd) + FD_SET (pipe_fd, &read_fds); - case EINVAL: // The specified time limit is invalid. One of its components is negative or too large. - default: // Other unknown error - return eConnectionStatusError; + const int nfds = std::max(data_fd, pipe_fd) + 1; - case EAGAIN: // The kernel was (perhaps temporarily) unable to - // allocate the requested number of file descriptors, - // or we have non-blocking IO - case EINTR: // A signal was delivered before the time limit - // expired and before any of the selected events - // occurred. - break; // Lets keep reading to until we timeout + Error error; + + if (log) + { + if (have_pipe_fd) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i, %i}, NULL, NULL, timeout=%p)...", + this, nfds, data_fd, pipe_fd, tv_ptr); + else + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds=%i, fds={%i}, NULL, NULL, timeout=%p)...", + this, nfds, data_fd, tv_ptr); + } + + const int num_set_fds = ::select (nfds, &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", + this, nfds, data_fd, pipe_fd, 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", + this, nfds, data_fd, tv_ptr, num_set_fds, error.AsCString()); + } + + if (error_ptr) + *error_ptr = error; + + if (error.Fail()) + { + switch (error.GetError()) + { + case EBADF: // One of the descriptor sets specified an invalid descriptor. + return eConnectionStatusLostConnection; + + case EINVAL: // The specified time limit is invalid. One of its components is negative or too large. + default: // Other unknown error + return eConnectionStatusError; + + case EAGAIN: // The kernel was (perhaps temporarily) unable to + // allocate the requested number of file descriptors, + // or we have non-blocking IO + case EINTR: // A signal was delivered before the time limit + // expired and before any of the selected events + // occurred. + break; // Lets keep reading to until we timeout + } + } + else if (num_set_fds == 0) + { + return eConnectionStatusTimedOut; + } + else if (num_set_fds > 0) + { + if (FD_ISSET(data_fd, &read_fds)) + return eConnectionStatusSuccess; + if (have_pipe_fd && FD_ISSET(pipe_fd, &read_fds)) + { + // We got a command to exit. Read the data from that pipe: + char buffer[16]; + ssize_t bytes_read; + + do + { + bytes_read = ::read (pipe_fd, buffer, sizeof(buffer)); + } while (bytes_read < 0 && errno == EINTR); + assert (bytes_read == 1 && buffer[0] == 'q'); + + if (log) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() got data: %*s from the command channel.", + this, (int) bytes_read, buffer); + + return eConnectionStatusEndOfFile; + } } } - else if (num_set_fds == 0) + } + + if (error_ptr) + error_ptr->SetErrorString("not connected"); + return eConnectionStatusLostConnection; +} + +#endif + +#if 0 +#include + +// This ConnectionFileDescriptor::BytesAvailable() uses poll(). poll() should NOT +// be used on MacOSX as it has all sorts of restrictions on the types of file descriptors +// that it doesn't support. +// +// There may be some systems that properly support poll() that could use this +// implementation. I will let each system opt into this on their own. +// +// PROS: +// - no restrictions on the fd value that is used +// CONS: +// - varies wildly from platform to platform in its implementation restrictions + +ConnectionStatus +ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_ptr) +{ + // Don't need to take the mutex here separately since we are only called from Read. If we + // ever get used more generally we will need to lock here as well. + + LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION)); + if (log) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable (timeout_usec = %u)", this, timeout_usec); + int timeout_msec = 0; + if (timeout_usec == UINT32_MAX) + { + // Infinite wait... + timeout_msec = -1; + } + else if (timeout_usec == 0) + { + // Return immediately, don't wait + timeout_msec = 0; + } + else + { + // Convert usec to msec + timeout_msec = (timeout_usec + 999) / 1000; + } + + // Make a copy of the file descriptors to make sure we don't + // have another thread change these values out from under us + // and cause problems in the loop below where like in FS_SET() + const int data_fd = m_fd_recv; + const int pipe_fd = m_pipe_read; + + // Make sure the file descriptor can be used with select as it + // must be in range + if (data_fd >= 0) + { + const bool have_pipe_fd = pipe_fd >= 0; + struct pollfd fds[2] = { - return eConnectionStatusTimedOut; - } - else if (num_set_fds > 0) + { data_fd, POLLIN, 0 }, + { pipe_fd, POLLIN, 0 } + }; + const int nfds = have_pipe_fd ? 2 : 1; + Error error; + while (data_fd == m_fd_recv) { - if (m_pipe_read != -1 && FD_ISSET(m_pipe_read, &read_fds)) + const int num_set_fds = ::poll (fds, nfds, timeout_msec); + + if (num_set_fds < 0) + error.SetErrorToErrno(); + else + error.Clear(); + + if (error_ptr) + *error_ptr = error; + + if (log) { - // We got a command to exit. Read the data from that pipe: - char buffer[16]; - ssize_t bytes_read; - - do + if (have_pipe_fd) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::poll (fds={{%i,POLLIN},{%i,POLLIN}}, nfds=%i, timeout_ms=%i) => %d, error = %s\n", + this, + data_fd, + pipe_fd, + nfds, + timeout_msec, + num_set_fds, + error.AsCString()); + else + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::poll (fds={{%i,POLLIN}}, nfds=%i, timeout_ms=%i) => %d, error = %s\n", + this, + data_fd, + nfds, + timeout_msec, + num_set_fds, + error.AsCString()); + } + + if (error.Fail()) + { + switch (error.GetError()) { - bytes_read = ::read (m_pipe_read, buffer, sizeof(buffer)); - } while (bytes_read < 0 && errno == EINTR); - assert (bytes_read == 1 && buffer[0] == 'q'); - - if (log) - log->Printf("%p ConnectionFileDescriptor::BytesAvailable() got data: %*s from the command channel.", - this, (int) bytes_read, buffer); - - return eConnectionStatusEndOfFile; + case EBADF: // One of the descriptor sets specified an invalid descriptor. + return eConnectionStatusLostConnection; + + case EINVAL: // The specified time limit is invalid. One of its components is negative or too large. + default: // Other unknown error + return eConnectionStatusError; + + case EAGAIN: // The kernel was (perhaps temporarily) unable to + // allocate the requested number of file descriptors, + // or we have non-blocking IO + case EINTR: // A signal was delivered before the time limit + // expired and before any of the selected events + // occurred. + break; // Lets keep reading to until we timeout + } + } + else if (num_set_fds == 0) + { + return eConnectionStatusTimedOut; + } + else if (num_set_fds > 0) + { + if (fds[0].revents & POLLIN) + return eConnectionStatusSuccess; + if (fds[1].revents & POLLIN) + { + // We got a command to exit. Read the data from that pipe: + char buffer[16]; + ssize_t bytes_read; + + do + { + bytes_read = ::read (pipe_fd, buffer, sizeof(buffer)); + } while (bytes_read < 0 && errno == EINTR); + assert (bytes_read == 1 && buffer[0] == 'q'); + + if (log) + log->Printf("%p ConnectionFileDescriptor::BytesAvailable() got data: %*s from the command channel.", + this, (int) bytes_read, buffer); + + return eConnectionStatusEndOfFile; + } } - else - return eConnectionStatusSuccess; } } - if (error_ptr) error_ptr->SetErrorString("not connected"); return eConnectionStatusLostConnection; } +#endif + ConnectionStatus ConnectionFileDescriptor::Close (int& fd, Error *error_ptr) { -- 2.7.4