[llvm-jitlink] Add diagnostic output and port executor to getaddrinfo(3) as well

Add diagnostic output for TCP connections on both sides, llvm-jitlink and llvm-jitlink-executor.
Port the executor to use getaddrinfo(3) as well. This makes the code more symmetric and seems to be the recommended way for implementing the server side.

Reviewed By: rzurob

Differential Revision: https://reviews.llvm.org/D98581

Author: weliveindetail

llvm/tools/llvm-jitlink/llvm-jitlink-executor/llvm-jitlink-executor.cpp

@@ -17,11 +17,14 @@
 #include "llvm/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
+#include <cstring>
 #include <sstream>
 
 #ifdef LLVM_ON_UNIX
 
+#include <netdb.h>
 #include <netinet/in.h>
 #include <sys/socket.h>
 
@@ -46,37 +49,58 @@ void printErrorAndExit(Twine ErrMsg) {
   exit(1);
 }
 
-int openListener(std::string Host, int Port) {
+int openListener(std::string Host, std::string PortStr) {
 #ifndef LLVM_ON_UNIX
   // FIXME: Add TCP support for Windows.
   printErrorAndExit("listen option not supported");
   return 0;
 #else
-  int SockFD = socket(PF_INET, SOCK_STREAM, 0);
-  struct sockaddr_in ServerAddr, ClientAddr;
-  socklen_t ClientAddrLen = sizeof(ClientAddr);
-  memset(&ServerAddr, 0, sizeof(ServerAddr));
-  ServerAddr.sin_family = PF_INET;
-  ServerAddr.sin_family = INADDR_ANY;
-  ServerAddr.sin_port = htons(Port);
-
-  {
-    // lose the "Address already in use" error message
-    int Yes = 1;
-    if (setsockopt(SockFD, SOL_SOCKET, SO_REUSEADDR, &Yes, sizeof(int)) == -1) {
-      errs() << "Error calling setsockopt.\n";
-      exit(1);
-    }
+  addrinfo Hints{};
+  Hints.ai_family = AF_INET;
+  Hints.ai_socktype = SOCK_STREAM;
+  Hints.ai_flags = AI_PASSIVE;
+
+  addrinfo *AI;
+  if (int EC = getaddrinfo(nullptr, PortStr.c_str(), &Hints, &AI)) {
+    errs() << "Error setting up bind address: " << gai_strerror(EC) << "\n";
+    exit(1);
+  }
+
+  // Create a socket from first addrinfo structure returned by getaddrinfo.
+  int SockFD;
+  if ((SockFD = socket(AI->ai_family, AI->ai_socktype, AI->ai_protocol)) < 0) {
+    errs() << "Error creating socket: " << std::strerror(errno) << "\n";
+    exit(1);
   }
 
-  if (bind(SockFD, (struct sockaddr *)&ServerAddr, sizeof(ServerAddr)) < 0) {
-    errs() << "Error on binding.\n";
+  // Avoid "Address already in use" errors.
+  const int Yes = 1;
+  if (setsockopt(SockFD, SOL_SOCKET, SO_REUSEADDR, &Yes, sizeof(int)) == -1) {
+    errs() << "Error calling setsockopt: " << std::strerror(errno) << "\n";
     exit(1);
   }
 
-  listen(SockFD, 1);
-  return accept(SockFD, (struct sockaddr *)&ClientAddr, &ClientAddrLen);
+  // Bind the socket to the desired port.
+  if (bind(SockFD, AI->ai_addr, AI->ai_addrlen) < 0) {
+    errs() << "Error on binding: " << std::strerror(errno) << "\n";
+    exit(1);
+  }
+
+  // Listen for incomming connections.
+  static constexpr int ConnectionQueueLen = 1;
+  listen(SockFD, ConnectionQueueLen);
+
+  outs() << "Listening at " << Host << ":" << PortStr << "\n";
+
+#if defined(_AIX)
+  assert(Hi_32(AI->ai_addrlen) == 0 && "Field is a size_t on 64-bit AIX");
+  socklen_t AddrLen = Lo_32(AI->ai_addrlen);
+  return accept(SockFD, AI->ai_addr, &AddrLen);
+#else
+  return accept(SockFD, AI->ai_addr, &AI->ai_addrlen);
 #endif
+
+#endif // LLVM_ON_UNIX
 }
 
 int main(int argc, char *argv[]) {
@@ -105,9 +129,11 @@ int main(int argc, char *argv[]) {
 
       int Port = 0;
       if (PortStr.getAsInteger(10, Port))
-        printErrorAndExit("port" + PortStr + " is not a valid integer");
+        printErrorAndExit("port number '" + PortStr +
+                          "' is not a valid integer");
 
-      InFD = OutFD = openListener(Host.str(), Port);
+      InFD = OutFD = openListener(Host.str(), PortStr.str());
+      outs() << "Connection established. Running OrcRPCTPCServer...\n";
     } else
       printErrorAndExit("invalid specifier type \"" + SpecifierType + "\"");
   }

llvm/tools/llvm-jitlink/llvm-jitlink.cpp

@@ -42,6 +42,7 @@
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Timer.h"
 
+#include <cstring>
 #include <list>
 #include <string>
 
@@ -668,73 +669,82 @@ LLVMJITLinkRemoteTargetProcessControl::LaunchExecutor() {
 #endif
 }
 
-Expected<std::unique_ptr<TargetProcessControl>>
-LLVMJITLinkRemoteTargetProcessControl::ConnectToExecutor() {
-#ifndef LLVM_ON_UNIX
-  // FIXME: Add TCP support for Windows.
-  return make_error<StringError>("-" + OutOfProcessExecutorConnect.ArgStr +
-                                     " not supported on non-unix platforms",
-                                 inconvertibleErrorCode());
-#else
-
-  shared::registerStringError<LLVMJITLinkChannel>();
-
-  StringRef HostNameStr, PortStr;
-  std::tie(HostNameStr, PortStr) =
-      StringRef(OutOfProcessExecutorConnect).split(':');
-
-  if (HostNameStr.empty())
-    return make_error<StringError>("host name for -" +
-                                       OutOfProcessExecutorConnect.ArgStr +
-                                       " can not be empty",
-                                   inconvertibleErrorCode());
-  if (PortStr.empty())
-    return make_error<StringError>(
-        "port for -" + OutOfProcessExecutorConnect.ArgStr + " can not be empty",
-        inconvertibleErrorCode());
-
-  std::string HostName = HostNameStr.str();
-  int Port = 0;
-  if (PortStr.getAsInteger(10, Port))
-    return make_error<StringError>("port number " + PortStr +
-                                       " is not a valid integer",
-                                   inconvertibleErrorCode());
+static Error createTCPSocketError(Twine Details) {
+  return make_error<StringError>(
+      formatv("Failed to connect TCP socket '{0}': {1}",
+              OutOfProcessExecutorConnect, Details),
+      inconvertibleErrorCode());
+}
 
+static Expected<int> connectTCPSocket(std::string Host, std::string PortStr) {
   addrinfo *AI;
   addrinfo Hints{};
   Hints.ai_family = AF_INET;
   Hints.ai_socktype = SOCK_STREAM;
   Hints.ai_flags = AI_NUMERICSERV;
-  if (int EC =
-          getaddrinfo(HostName.c_str(), PortStr.str().c_str(), &Hints, &AI))
-    return make_error<StringError>(formatv("Failed to resolve {0}:{1} ({2})",
-                                           HostName, Port, gai_strerror(EC)),
-                                   inconvertibleErrorCode());
 
-  // getaddrinfo returns a list of address structures.  Go through the list
-  // to find one we can connect to.
+  if (int EC = getaddrinfo(Host.c_str(), PortStr.c_str(), &Hints, &AI))
+    return createTCPSocketError("Address resolution failed (" +
+                                StringRef(gai_strerror(EC)) + ")");
+
+  // Cycle through the returned addrinfo structures and connect to the first
+  // reachable endpoint.
   int SockFD;
-  int ConnectRC = -1;
-  for (addrinfo *Server = AI; Server; Server = Server->ai_next) {
-    // If socket fails, maybe it's because the address family is not supported.
-    // Skip to the next addrinfo structure.
+  addrinfo *Server;
+  for (Server = AI; Server != nullptr; Server = Server->ai_next) {
+    // socket might fail, e.g. if the address family is not supported. Skip to
+    // the next addrinfo structure in such a case.
     if ((SockFD = socket(AI->ai_family, AI->ai_socktype, AI->ai_protocol)) < 0)
       continue;
 
-    ConnectRC = connect(SockFD, Server->ai_addr, Server->ai_addrlen);
-    if (ConnectRC == 0)
+    // If connect returns null, we exit the loop with a working socket.
+    if (connect(SockFD, Server->ai_addr, Server->ai_addrlen) == 0)
       break;
 
     close(SockFD);
   }
   freeaddrinfo(AI);
-  if (ConnectRC == -1)
-    return make_error<StringError>("Failed to connect to " + HostName + ":" +
-                                       Twine(Port),
-                                   inconvertibleErrorCode());
+
+  // If we reached the end of the loop without connecting to a valid endpoint,
+  // dump the last error that was logged in socket() or connect().
+  if (Server == nullptr)
+    return createTCPSocketError(std::strerror(errno));
+
+  return SockFD;
+}
+
+Expected<std::unique_ptr<TargetProcessControl>>
+LLVMJITLinkRemoteTargetProcessControl::ConnectToExecutor() {
+#ifndef LLVM_ON_UNIX
+  // FIXME: Add TCP support for Windows.
+  return make_error<StringError>("-" + OutOfProcessExecutorConnect.ArgStr +
+                                     " not supported on non-unix platforms",
+                                 inconvertibleErrorCode());
+#else
+
+  shared::registerStringError<LLVMJITLinkChannel>();
+
+  StringRef Host, PortStr;
+  std::tie(Host, PortStr) = StringRef(OutOfProcessExecutorConnect).split(':');
+  if (Host.empty())
+    return createTCPSocketError("Host name for -" +
+                                OutOfProcessExecutorConnect.ArgStr +
+                                " can not be empty");
+  if (PortStr.empty())
+    return createTCPSocketError("Port number in -" +
+                                OutOfProcessExecutorConnect.ArgStr +
+                                " can not be empty");
+  int Port = 0;
+  if (PortStr.getAsInteger(10, Port))
+    return createTCPSocketError("Port number '" + PortStr +
+                                "' is not a valid integer");
+
+  Expected<int> SockFD = connectTCPSocket(Host.str(), PortStr.str());
+  if (!SockFD)
+    return SockFD.takeError();
 
   auto SSP = std::make_shared<SymbolStringPool>();
-  auto Channel = std::make_unique<shared::FDRawByteChannel>(SockFD, SockFD);
+  auto Channel = std::make_unique<shared::FDRawByteChannel>(*SockFD, *SockFD);
   auto Endpoint = std::make_unique<LLVMJITLinkRPCEndpoint>(*Channel, true);
 
   auto ReportError = [](Error Err) {