Re-landing IPv6 support for LLDB Host

This support was landed in r300579, and reverted in r300669 due to failures on the bots.

The failures were caused by sockets not being properly closed, and this updated version of the patches should resolve that.

Summary from the original change:

This patch adds IPv6 support to LLDB/Host's TCP socket implementation. Supporting IPv6 involved a few significant changes to the implementation of the socket layers, and I have performed some significant code cleanup along the way.

This patch changes the Socket constructors for all types of sockets to not create sockets until first use. This is required for IPv6 support because the socket type will vary based on the address you are connecting to. This also has the benefit of removing code that could have errors from the Socket subclass constructors (which seems like a win to me).

The patch also slightly changes the API and behaviors of the Listen/Accept pattern. Previously both Listen and Accept calls took an address specified as a string. Now only listen does. This change was made because the Listen call can result in opening more than one socket. In order to support listening for both IPv4 and IPv6 connections we need to open one AF_INET socket and one AF_INET6 socket. During the listen call we construct a map of file descriptors to addrin structures which represent the allowable incoming connection address. This map removes the need for taking an address into the Accept call.

This does have a change in functionality. Previously you could Listen for connections based on one address, and Accept connections from a different address. This is no longer supported. I could not find anywhere in LLDB where we actually used the APIs in that way. The new API does still support AnyAddr for allowing incoming connections from any address.

The Listen implementation is implemented using kqueue on FreeBSD and Darwin, WSAPoll on Windows and poll(2) everywhere else.

https://reviews.llvm.org/D31823

llvm-svn: 301492

lldb/cmake/modules/LLDBConfig.cmake

@@ -1,4 +1,7 @@
 include(CheckCXXSymbolExists)
+include(CheckSymbolExists)
+include(CheckIncludeFile)
+include(CheckIncludeFiles)
 
 set(LLDB_PROJECT_ROOT ${CMAKE_CURRENT_SOURCE_DIR})
 set(LLDB_SOURCE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/source")
@@ -426,10 +429,14 @@ if ((CMAKE_SYSTEM_NAME MATCHES "Android") AND LLVM_BUILD_STATIC AND
 endif()
 
 find_package(Backtrace)
+set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
+check_symbol_exists(ppoll poll.h HAVE_PPOLL)
+set(CMAKE_REQUIRED_DEFINITIONS)
+check_symbol_exists(sigaction signal.h HAVE_SIGACTION)
 
 include(CheckIncludeFile)
 check_include_file(termios.h HAVE_TERMIOS_H)
-check_include_file(sys/event.h HAVE_SYS_EVENT_H)
+check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
 
 # These checks exist in LLVM's configuration, so I want to match the LLVM names
 # so that the check isn't duplicated, but we translate them into the LLDB names

lldb/include/lldb/Host/Config.h

@@ -18,6 +18,10 @@
 
 #define HAVE_SYS_EVENT_H 1
 
+#define HAVE_PPOLL 0
+
+#define HAVE_SIGACTION 1
+
 #else
 
 #error This file is only used by the Xcode build.

lldb/include/lldb/Host/Config.h.cmake

@@ -16,4 +16,8 @@
 
 #cmakedefine01 HAVE_SYS_EVENT_H
 
+#cmakedefine01 HAVE_PPOLL
+
+#cmakedefine01 HAVE_SIGACTION
+
 #endif // #ifndef LLDB_HOST_CONFIG_H

lldb/include/lldb/Host/MainLoop.h

@@ -10,16 +10,95 @@
 #ifndef lldb_Host_MainLoop_h_
 #define lldb_Host_MainLoop_h_
 
-#ifdef _WIN32
+#include "lldb/Host/Config.h"
 #include "lldb/Host/MainLoopBase.h"
+
+#include "llvm/ADT/DenseMap.h"
+
+#if !HAVE_PPOLL && !HAVE_SYS_EVENT_H
+#define SIGNAL_POLLING_UNSUPPORTED 1
+#endif
+
 namespace lldb_private {
-typedef MainLoopBase MainLoop;
-}
-#else
-#include "lldb/Host/posix/MainLoopPosix.h"
-namespace lldb_private {
-typedef MainLoopPosix MainLoop;
-}
+
+// Implementation of the MainLoopBase class. It can monitor file descriptors for
+// readability using ppoll, kqueue, poll or WSAPoll. On Windows it only supports
+// polling sockets, and will not work on generic file handles or pipes. On
+// systems without kqueue or ppoll handling singnals is not supported. In
+// addition to the common base, this class provides the ability to invoke a
+// given handler when a signal is received.
+//
+// Since this class is primarily intended to be used for single-threaded
+// processing, it does not attempt to perform any internal synchronisation and
+// any concurrent accesses must be protected  externally. However, it is
+// perfectly legitimate to have more than one instance of this class running on
+// separate threads, or even a single thread (with some limitations on signal
+// monitoring).
+// TODO: Add locking if this class is to be used in a multi-threaded context.
+class MainLoop : public MainLoopBase {
+private:
+  class SignalHandle;
+
+public:
+  typedef std::unique_ptr<SignalHandle> SignalHandleUP;
+
+  ~MainLoop() override;
+
+  ReadHandleUP RegisterReadObject(const lldb::IOObjectSP &object_sp,
+                                  const Callback &callback,
+                                  Error &error) override;
+
+  // Listening for signals from multiple MainLoop instances is perfectly safe as
+  // long as they don't try to listen for the same signal. The callback function
+  // is invoked when the control returns to the Run() function, not when the
+  // hander is executed. This mean that you can treat the callback as a normal
+  // function and perform things which would not be safe in a signal handler.
+  // However, since the callback is not invoked synchronously, you cannot use
+  // this mechanism to handle SIGSEGV and the like.
+  SignalHandleUP RegisterSignal(int signo, const Callback &callback,
+                                Error &error);
+
+  Error Run() override;
+
+  // This should only be performed from a callback. Do not attempt to terminate
+  // the processing from another thread.
+  // TODO: Add synchronization if we want to be terminated from another thread.
+  void RequestTermination() override { m_terminate_request = true; }
+
+protected:
+  void UnregisterReadObject(IOObject::WaitableHandle handle) override;
+
+  void UnregisterSignal(int signo);
+
+private:
+  class SignalHandle {
+  public:
+    ~SignalHandle() { m_mainloop.UnregisterSignal(m_signo); }
+
+  private:
+    SignalHandle(MainLoop &mainloop, int signo)
+        : m_mainloop(mainloop), m_signo(signo) {}
+
+    MainLoop &m_mainloop;
+    int m_signo;
+
+    friend class MainLoop;
+    DISALLOW_COPY_AND_ASSIGN(SignalHandle);
+  };
+
+  struct SignalInfo {
+    Callback callback;
+#if HAVE_SIGACTION
+    struct sigaction old_action;
 #endif
+    bool was_blocked : 1;
+  };
+
+  llvm::DenseMap<IOObject::WaitableHandle, Callback> m_read_fds;
+  llvm::DenseMap<int, SignalInfo> m_signals;
+  bool m_terminate_request : 1;
+};
+
+} // namespace lldb_private
 
 #endif // lldb_Host_MainLoop_h_

lldb/include/lldb/Host/Socket.h

@@ -57,8 +57,7 @@ class Socket : public IOObject {
 
   virtual Error Connect(llvm::StringRef name) = 0;
   virtual Error Listen(llvm::StringRef name, int backlog) = 0;
-  virtual Error Accept(llvm::StringRef name, bool child_processes_inherit,
-                       Socket *&socket) = 0;
+  virtual Error Accept(Socket *&socket) = 0;
 
   // Initialize a Tcp Socket object in listening mode.  listen and accept are
   // implemented
@@ -103,7 +102,8 @@ class Socket : public IOObject {
                                 int32_t &port, Error *error_ptr);
 
 protected:
-  Socket(NativeSocket socket, SocketProtocol protocol, bool should_close);
+  Socket(SocketProtocol protocol, bool should_close,
+         bool m_child_process_inherit);
 
   virtual size_t Send(const void *buf, const size_t num_bytes);
 
@@ -117,6 +117,7 @@ class Socket : public IOObject {
 
   SocketProtocol m_protocol;
   NativeSocket m_socket;
+  bool m_child_processes_inherit;
 };
 
 } // namespace lldb_private

lldb/include/lldb/Host/common/TCPSocket.h

@@ -11,12 +11,16 @@
 #define liblldb_TCPSocket_h_
 
 #include "lldb/Host/Socket.h"
+#include "lldb/Host/SocketAddress.h"
+#include <map>
 
 namespace lldb_private {
 class TCPSocket : public Socket {
 public:
-  TCPSocket(NativeSocket socket, bool should_close);
-  TCPSocket(bool child_processes_inherit, Error &error);
+  TCPSocket(bool should_close, bool child_processes_inherit);
+  TCPSocket(NativeSocket socket, bool should_close,
+            bool child_processes_inherit);
+  ~TCPSocket() override;
 
   // returns port number or 0 if error
   uint16_t GetLocalPortNumber() const;
@@ -37,8 +41,18 @@ class TCPSocket : public Socket {
 
   Error Connect(llvm::StringRef name) override;
   Error Listen(llvm::StringRef name, int backlog) override;
-  Error Accept(llvm::StringRef name, bool child_processes_inherit,
-               Socket *&conn_socket) override;
+  Error Accept(Socket *&conn_socket) override;
+
+  Error CreateSocket(int domain);
+
+  bool IsValid() const override;
+
+private:
+  TCPSocket(NativeSocket socket, const TCPSocket &listen_socket);
+
+  void CloseListenSockets();
+
+  std::map<int, SocketAddress> m_listen_sockets;
 };
 }
 

lldb/include/lldb/Host/common/UDPSocket.h

@@ -15,19 +15,20 @@
 namespace lldb_private {
 class UDPSocket : public Socket {
 public:
-  UDPSocket(bool child_processes_inherit, Error &error);
+  UDPSocket(bool should_close, bool child_processes_inherit);
 
   static Error Connect(llvm::StringRef name, bool child_processes_inherit,
                        Socket *&socket);
 
 private:
-  UDPSocket(NativeSocket socket);
+  UDPSocket(NativeSocket socket, const UDPSocket &listen_socket);
 
   size_t Send(const void *buf, const size_t num_bytes) override;
   Error Connect(llvm::StringRef name) override;
   Error Listen(llvm::StringRef name, int backlog) override;
-  Error Accept(llvm::StringRef name, bool child_processes_inherit,
-               Socket *&socket) override;
+  Error Accept(Socket *&socket) override;
+
+  Error CreateSocket();
 
   SocketAddress m_sockaddr;
 };

lldb/include/lldb/Host/linux/AbstractSocket.h

@@ -15,7 +15,7 @@
 namespace lldb_private {
 class AbstractSocket : public DomainSocket {
 public:
-  AbstractSocket(bool child_processes_inherit, Error &error);
+  AbstractSocket(bool child_processes_inherit);
 
 protected:
   size_t GetNameOffset() const override;

lldb/include/lldb/Host/posix/DomainSocket.h

@@ -15,22 +15,20 @@
 namespace lldb_private {
 class DomainSocket : public Socket {
 public:
-  DomainSocket(bool child_processes_inherit, Error &error);
+  DomainSocket(bool should_close, bool child_processes_inherit);
 
   Error Connect(llvm::StringRef name) override;
   Error Listen(llvm::StringRef name, int backlog) override;
-  Error Accept(llvm::StringRef name, bool child_processes_inherit,
-               Socket *&socket) override;
+  Error Accept(Socket *&socket) override;
 
 protected:
-  DomainSocket(SocketProtocol protocol, bool child_processes_inherit,
-               Error &error);
+  DomainSocket(SocketProtocol protocol, bool child_processes_inherit);
 
   virtual size_t GetNameOffset() const;
   virtual void DeleteSocketFile(llvm::StringRef name);
 
 private:
-  DomainSocket(NativeSocket socket);
+  DomainSocket(NativeSocket socket, const DomainSocket &listen_socket);
 };
 }