[compiler-rt] r324849 - Revert r324847, there's bot failures.

Kuba Mracek via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 11 12:44:04 PST 2018


Author: kuba.brecka
Date: Sun Feb 11 12:44:04 2018
New Revision: 324849

URL: http://llvm.org/viewvc/llvm-project?rev=324849&view=rev
Log:
Revert r324847, there's bot failures.


Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_internal.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_mac.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc?rev=324849&r1=324848&r2=324849&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc Sun Feb 11 12:44:04 2018
@@ -64,9 +64,7 @@ extern "C" {
 #include <pthread.h>
 #include <sched.h>
 #include <signal.h>
-#include <spawn.h>
 #include <stdlib.h>
-#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <sys/resource.h>
 #include <sys/stat.h>
@@ -208,62 +206,27 @@ int internal_fork() {
   return fork();
 }
 
-fd_t internal_spawn(const char *argv[], pid_t *pid) {
-  int master_fd = kInvalidFd;
-  int slave_fd = kInvalidFd;
-  char **env = GetEnviron();
-  int res = 0;
-
-  // We need a new pseudoterminal to avoid bufferring problems. The 'atos' tool
-  // in particular detects when it's talking to a pipe and forgets to flush the
-  // output stream after sending a response.
-  master_fd = posix_openpt(O_RDWR);
-  if (master_fd == kInvalidFd) goto cleanup;
-  res = grantpt(master_fd);
-  if (res != 0) goto cleanup;
-  res = unlockpt(master_fd);
-  if (res != 0) goto cleanup;
-
-  // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems.
-  char slave_pty_name[128];
-  res = ioctl(master_fd, TIOCPTYGNAME, slave_pty_name);
-  if (res == -1) goto cleanup;
-
-  slave_fd = internal_open(slave_pty_name, O_RDWR);
-  if (slave_fd == kInvalidFd) goto cleanup;
-
-  posix_spawn_file_actions_t actions;
-  res = posix_spawn_file_actions_init(&actions);
-  if (res != 0) goto cleanup;
-  res = posix_spawn_file_actions_adddup2(&actions, slave_fd, STDIN_FILENO);
-  if (res != 0) goto cleanup;
-  res = posix_spawn_file_actions_adddup2(&actions, slave_fd, STDOUT_FILENO);
-  if (res != 0) goto cleanup;
-  res = posix_spawn_file_actions_addclose(&actions, slave_fd);
-  if (res != 0) goto cleanup;
-  res = posix_spawn_file_actions_addclose(&actions, master_fd);
-  if (res != 0) goto cleanup;
-
-  res = posix_spawnp(pid, argv[0], &actions, NULL, const_cast<char **>(argv),
-                     env);
-  if (res != 0) goto cleanup;
-
-  internal_close(slave_fd);
-  slave_fd = kInvalidFd;
-
-  // Disable echo in the new terminal, disable CR.
-  struct termios termflags;
-  tcgetattr(master_fd, &termflags);
-  termflags.c_oflag &= ~ONLCR;
-  termflags.c_lflag &= ~ECHO;
-  tcsetattr(master_fd, TCSANOW, &termflags);
-
-  return master_fd;
-
- cleanup:
-  if (master_fd != kInvalidFd) internal_close(master_fd);
-  if (slave_fd != kInvalidFd) internal_close(slave_fd);
-  return kInvalidFd;
+int internal_forkpty(int *amaster) {
+  int master, slave;
+  if (openpty(&master, &slave, nullptr, nullptr, nullptr) == -1) return -1;
+  int pid = internal_fork();
+  if (pid == -1) {
+    close(master);
+    close(slave);
+    return -1;
+  }
+  if (pid == 0) {
+    close(master);
+    if (login_tty(slave) != 0) {
+      // We already forked, there's not much we can do.  Let's quit.
+      Report("login_tty failed (errno %d)\n", errno);
+      internal__exit(1);
+    }
+  } else {
+    *amaster = master;
+    close(slave);
+  }
+  return pid;
 }
 
 uptr internal_rename(const char *oldpath, const char *newpath) {

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h?rev=324849&r1=324848&r2=324849&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h Sun Feb 11 12:44:04 2018
@@ -57,7 +57,7 @@ uptr internal_ptrace(int request, int pi
 uptr internal_waitpid(int pid, int *status, int options);
 
 int internal_fork();
-fd_t internal_spawn(const char *argv[], pid_t *pid);
+int internal_forkpty(int *amaster);
 
 // These functions call appropriate pthread_ functions directly, bypassing
 // the interceptor. They are weak and may not be present in some tools.

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_internal.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_internal.h?rev=324849&r1=324848&r2=324849&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_internal.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_internal.h Sun Feb 11 12:44:04 2018
@@ -72,7 +72,7 @@ class SymbolizerTool {
 // SymbolizerProcess may not be used from two threads simultaneously.
 class SymbolizerProcess {
  public:
-  explicit SymbolizerProcess(const char *path, bool use_posix_spawn = false);
+  explicit SymbolizerProcess(const char *path, bool use_forkpty = false);
   const char *SendCommand(const char *command);
 
  protected:
@@ -109,7 +109,7 @@ class SymbolizerProcess {
   uptr times_restarted_;
   bool failed_to_start_;
   bool reported_invalid_path_;
-  bool use_posix_spawn_;
+  bool use_forkpty_;
 };
 
 class LLVMSymbolizerProcess;

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc?rev=324849&r1=324848&r2=324849&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc Sun Feb 11 12:44:04 2018
@@ -395,14 +395,14 @@ const char *LLVMSymbolizer::FormatAndSen
   return symbolizer_process_->SendCommand(buffer_);
 }
 
-SymbolizerProcess::SymbolizerProcess(const char *path, bool use_posix_spawn)
+SymbolizerProcess::SymbolizerProcess(const char *path, bool use_forkpty)
     : path_(path),
       input_fd_(kInvalidFd),
       output_fd_(kInvalidFd),
       times_restarted_(0),
       failed_to_start_(false),
       reported_invalid_path_(false),
-      use_posix_spawn_(use_posix_spawn) {
+      use_forkpty_(use_forkpty) {
   CHECK(path_);
   CHECK_NE(path_[0], '\0');
 }

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_mac.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_mac.cc?rev=324849&r1=324848&r2=324849&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_mac.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_mac.cc Sun Feb 11 12:44:04 2018
@@ -51,7 +51,7 @@ bool DlAddrSymbolizer::SymbolizeData(upt
 class AtosSymbolizerProcess : public SymbolizerProcess {
  public:
   explicit AtosSymbolizerProcess(const char *path, pid_t parent_pid)
-      : SymbolizerProcess(path, /*use_posix_spawn*/ true) {
+      : SymbolizerProcess(path, /*use_forkpty*/ true) {
     // Put the string command line argument in the object so that it outlives
     // the call to GetArgV.
     internal_snprintf(pid_str_, sizeof(pid_str_), "%d", parent_pid);

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc?rev=324849&r1=324848&r2=324849&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc Sun Feb 11 12:44:04 2018
@@ -34,6 +34,10 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+#if SANITIZER_MAC
+#include <util.h>  // for forkpty()
+#endif  // SANITIZER_MAC
+
 // C++ demangling function, as required by Itanium C++ ABI. This is weak,
 // because we do not require a C++ ABI library to be linked to a program
 // using sanitizers; if it's not present, we'll just use the mangled name.
@@ -148,36 +152,80 @@ bool SymbolizerProcess::StartSymbolizerS
     return false;
   }
 
-  pid_t pid = -1;
+  int pid = -1;
 
-  const char *argv[kArgVMax];
-  GetArgV(path_, argv);
+  int infd[2];
+  internal_memset(&infd, 0, sizeof(infd));
+  int outfd[2];
+  internal_memset(&outfd, 0, sizeof(outfd));
+  if (!CreateTwoHighNumberedPipes(infd, outfd)) {
+    Report("WARNING: Can't create a socket pair to start "
+           "external symbolizer (errno: %d)\n", errno);
+    return false;
+  }
 
-  if (use_posix_spawn_) {
+  if (use_forkpty_) {
 #if SANITIZER_MAC
-    fd_t fd = internal_spawn(argv, &pid);
-    if (fd == kInvalidFd) {
-      Report("WARNING: failed to spawn external symbolizer (errno: %d)\n",
+    fd_t fd = kInvalidFd;
+
+    // forkpty redirects stdout and stderr into a single stream, so we would
+    // receive error messages as standard replies. To avoid that, let's dup
+    // stderr and restore it in the child.
+    int saved_stderr = dup(STDERR_FILENO);
+    CHECK_GE(saved_stderr, 0);
+
+    // We only need one pipe, for stdin of the child.
+    close(outfd[0]);
+    close(outfd[1]);
+
+    // Use forkpty to disable buffering in the new terminal.
+    pid = internal_forkpty(&fd);
+    if (pid == -1) {
+      // forkpty() failed.
+      Report("WARNING: failed to fork external symbolizer (errno: %d)\n",
              errno);
       return false;
+    } else if (pid == 0) {
+      // Child subprocess.
+
+      // infd[0] is the child's reading end.
+      close(infd[1]);
+
+      // Set up stdin to read from the pipe.
+      CHECK_GE(dup2(infd[0], STDIN_FILENO), 0);
+      close(infd[0]);
+
+      // Restore stderr.
+      CHECK_GE(dup2(saved_stderr, STDERR_FILENO), 0);
+      close(saved_stderr);
+
+      const char *argv[kArgVMax];
+      GetArgV(path_, argv);
+      execv(path_, const_cast<char **>(&argv[0]));
+      internal__exit(1);
     }
 
+    // Input for the child, infd[1] is our writing end.
+    output_fd_ = infd[1];
+    close(infd[0]);
+
+    // Continue execution in parent process.
     input_fd_ = fd;
-    output_fd_ = fd;
+
+    close(saved_stderr);
+
+    // Disable echo in the new terminal, disable CR.
+    struct termios termflags;
+    tcgetattr(fd, &termflags);
+    termflags.c_oflag &= ~ONLCR;
+    termflags.c_lflag &= ~ECHO;
+    tcsetattr(fd, TCSANOW, &termflags);
 #else  // SANITIZER_MAC
     UNIMPLEMENTED();
 #endif  // SANITIZER_MAC
   } else {
-    int infd[2];
-    internal_memset(&infd, 0, sizeof(infd));
-    int outfd[2];
-    internal_memset(&outfd, 0, sizeof(outfd));
-    if (!CreateTwoHighNumberedPipes(infd, outfd)) {
-      Report("WARNING: Can't create a socket pair to start "
-             "external symbolizer (errno: %d)\n", errno);
-      return false;
-    }
-
+    const char *argv[kArgVMax];
+    GetArgV(path_, argv);
     pid = StartSubprocess(path_, argv, /* stdin */ outfd[0],
                           /* stdout */ infd[1]);
     if (pid < 0) {




More information about the llvm-commits mailing list