[Lldb-commits] [lldb] r175378 - <rdar://problem/13121056>
Greg Clayton
gclayton at apple.com
Sat Feb 16 14:53:04 PST 2013
Author: gclayton
Date: Sat Feb 16 16:53:04 2013
New Revision: 175378
URL: http://llvm.org/viewvc/llvm-project?rev=175378&view=rev
Log:
<rdar://problem/13121056>
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.
Modified:
lldb/trunk/source/Core/ConnectionFileDescriptor.cpp
Modified: lldb/trunk/source/Core/ConnectionFileDescriptor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ConnectionFileDescriptor.cpp?rev=175378&r1=175377&r2=175378&view=diff
==============================================================================
--- lldb/trunk/source/Core/ConnectionFileDescriptor.cpp (original)
+++ lldb/trunk/source/Core/ConnectionFileDescriptor.cpp Sat Feb 16 16:53:04 2013
@@ -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 *ds
return 0;
}
- //Disconnect (NULL);
return 0;
}
return bytes_read;
@@ -590,6 +599,21 @@ ConnectionFileDescriptor::Write (const v
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
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<int>(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<int>(data_fd, pipe_fd) + 1;
+ llvm::SmallVector<fd_set, 1> 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
+ FD_SET (data_fd, read_fds.data());
+ if (have_pipe_fd)
+ FD_SET (pipe_fd, read_fds.data());
+
+ 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.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;
+}
+#else
- if (log)
- log->Printf("%p ConnectionFileDescriptor::BytesAvailable() ::select (nfds = %i, fd = %i, NULL, NULL, timeout = %p)...",
- this, nfds, m_fd_recv, tv_ptr);
+// 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.
- const int num_set_fds = ::select (nfds, &read_fds, NULL, NULL, tv_ptr);
- if (num_set_fds < 0)
- error.SetErrorToErrno();
- else
- error.Clear();
+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 (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 (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());
+ while (data_fd == m_fd_recv)
+ {
+ fd_set read_fds;
+ FD_ZERO (&read_fds);
+ FD_SET (data_fd, &read_fds);
+ if (have_pipe_fd)
+ FD_SET (pipe_fd, &read_fds);
- if (error_ptr)
- *error_ptr = error;
+ const int nfds = std::max<int>(data_fd, pipe_fd) + 1;
- if (error.Fail())
- {
- switch (error.GetError())
+ Error error;
+
+ if (log)
{
- 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;
+ 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());
+ }
- 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
+ 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 <poll.h>
+
+// 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)
{
More information about the lldb-commits
mailing list