[compiler-rt] r258849 - [sanitizers] extracted process management functions

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 26 14:48:36 PST 2016


Hi Mike,

I believe this commit broke at least one bot:
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA_build/10010/consoleFull#-158682280549ba4694-19c4-4d7e-bec5-911270d8a58c

Could you fix or revert please?

Thanks,
-Quentin
> On Jan 26, 2016, at 12:10 PM, Mike Aizatsky via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: aizatsky
> Date: Tue Jan 26 14:10:01 2016
> New Revision: 258849
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=258849&view=rev
> Log:
> [sanitizers] extracted process management functions
> 
> Differential Revision: http://reviews.llvm.org/D16546
> 
> Modified:
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
>    compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc
> 
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc?rev=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc Tue Jan 26 14:10:01 2016
> @@ -423,6 +423,10 @@ bool TemplateMatch(const char *templ, co
> static const char kPathSeparator = SANITIZER_WINDOWS ? ';' : ':';
> 
> char *FindPathToBinary(const char *name) {
> +  if (FileExists(name)) {
> +    return internal_strdup(name);
> +  }
> +
>   const char *path = GetEnv("PATH");
>   if (!path)
>     return nullptr;
> 
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Tue Jan 26 14:10:01 2016
> @@ -279,6 +279,17 @@ const char *GetPwd();
> char *FindPathToBinary(const char *name);
> bool IsPathSeparator(const char c);
> bool IsAbsolutePath(const char *path);
> +// Starts a subprocess and returs its pid.
> +// If *_fd parameters are not kInvalidFd their corresponding input/output
> +// streams will be redirect to the file. The files will always be closed
> +// in parent process even in case of an error.
> +// The child process will close all fds after STDERR_FILENO
> +// before passing control to a program.
> +pid_t StartSubprocess(const char *filename, const char *const argv[],
> +                      fd_t stdin_fd = kInvalidFd, fd_t stdout_fd = kInvalidFd,
> +                      fd_t stderr_fd = kInvalidFd);
> +// Checks if specified process is still running
> +bool IsProcessRunning(pid_t pid);
> 
> u32 GetUid();
> void ReExec();
> @@ -748,6 +759,23 @@ void GetPcSpBp(void *context, uptr *pc,
> void DisableReexec();
> void MaybeReexec();
> 
> +template <typename Fn>
> +class RunOnDestruction {
> + public:
> +  explicit RunOnDestruction(Fn fn) : fn_(fn) {}
> +  ~RunOnDestruction() { fn_(); }
> +
> + private:
> +  Fn fn_;
> +};
> +
> +// A simple scope guard. Usage:
> +// auto cleanup = at_scope_exit([]{ do_cleanup; });
> +template <typename Fn>
> +RunOnDestruction<Fn> at_scope_exit(Fn fn) {
> +  return RunOnDestruction<Fn>(fn);
> +}
> +
> }  // namespace __sanitizer
> 
> inline void *operator new(__sanitizer::operator_new_size_type size,
> 
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h?rev=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h Tue Jan 26 14:10:01 2016
> @@ -89,6 +89,7 @@ typedef unsigned error_t;
> typedef int fd_t;
> typedef int error_t;
> #endif
> +typedef int pid_t;
> 
> // WARNING: OFF_T may be different from OS type off_t, depending on the value of
> // _FILE_OFFSET_BITS. This definition of OFF_T matches the ABI of system calls
> 
> 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=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix.h Tue Jan 26 14:10:01 2016
> @@ -81,6 +81,8 @@ int my_pthread_attr_getstack(void *attr,
> int internal_sigaction(int signum, const void *act, void *oldact);
> void internal_sigfillset(__sanitizer_sigset_t *set);
> 
> +uptr internal_execve(const char *filename, char *const argv[],
> +                     char *const envp[]);
> }  // namespace __sanitizer
> 
> #endif  // SANITIZER_POSIX_H
> 
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc?rev=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc Tue Jan 26 14:10:01 2016
> @@ -34,6 +34,7 @@
> #include <sys/stat.h>
> #include <sys/time.h>
> #include <sys/types.h>
> +#include <sys/wait.h>
> #include <unistd.h>
> 
> #if SANITIZER_FREEBSD
> @@ -320,6 +321,68 @@ void AdjustStackSize(void *attr_) {
> }
> #endif // !SANITIZER_GO
> 
> +pid_t StartSubprocess(const char *program, const char *const argv[],
> +                      fd_t stdin_fd, fd_t stdout_fd, fd_t stderr_fd) {
> +  auto file_closer = at_scope_exit([&] {
> +    if (stdin_fd != kInvalidFd) {
> +      internal_close(stdin_fd);
> +    }
> +    if (stdout_fd != kInvalidFd) {
> +      internal_close(stdout_fd);
> +    }
> +    if (stderr_fd != kInvalidFd) {
> +      internal_close(stderr_fd);
> +    }
> +  });
> +
> +  int pid = internal_fork();
> +
> +  if (pid < 0) {
> +    int rverrno;
> +    if (internal_iserror(pid, &rverrno)) {
> +      Report("WARNING: failed to fork (errno %d)\n", rverrno);
> +    }
> +    return pid;
> +  }
> +
> +  if (pid == 0) {
> +    // Child subprocess
> +    if (stdin_fd != kInvalidFd) {
> +      internal_close(STDIN_FILENO);
> +      internal_dup2(stdin_fd, STDIN_FILENO);
> +      internal_close(stdin_fd);
> +    }
> +    if (stdout_fd != kInvalidFd) {
> +      internal_close(STDOUT_FILENO);
> +      internal_dup2(stdout_fd, STDOUT_FILENO);
> +      internal_close(stdout_fd);
> +    }
> +    if (stderr_fd != kInvalidFd) {
> +      internal_close(STDERR_FILENO);
> +      internal_dup2(stderr_fd, STDERR_FILENO);
> +      internal_close(stderr_fd);
> +    }
> +
> +    for (int fd = sysconf(_SC_OPEN_MAX); fd > 2; fd--) internal_close(fd);
> +
> +    internal_execve(program, const_cast<char **>(&argv[0]), nullptr);
> +    internal__exit(1);
> +  }
> +
> +  return pid;
> +}
> +
> +bool IsProcessRunning(pid_t pid) {
> +  int process_status;
> +  uptr waitpid_status = internal_waitpid(pid, &process_status, WNOHANG);
> +  int local_errno;
> +  if (internal_iserror(waitpid_status, &local_errno)) {
> +    VReport(1, "Waiting on the process failed (errno %d).\n", local_errno);
> +    return false;
> +  }
> +  return waitpid_status == 0;
> +}
> +
> } // namespace __sanitizer
> 
> #endif // SANITIZER_POSIX
> 
> 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=258849&r1=258848&r2=258849&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 Tue Jan 26 14:10:01 2016
> @@ -137,47 +137,23 @@ bool SymbolizerProcess::StartSymbolizerS
>     CHECK(infd);
>     CHECK(outfd);
> 
> -    // Real fork() may call user callbacks registered with pthread_atfork().
> -    pid = internal_fork();
> -    if (pid == -1) {
> -      // Fork() failed.
> +    const char *argv[kArgVMax];
> +    GetArgV(path_, argv);
> +    pid = StartSubprocess(path_, argv, /* stdin */ outfd[0],
> +                          /* stdout */ infd[1]);
> +    if (pid < 0) {
>       internal_close(infd[0]);
> -      internal_close(infd[1]);
> -      internal_close(outfd[0]);
>       internal_close(outfd[1]);
> -      Report("WARNING: failed to fork external symbolizer "
> -             " (errno: %d)\n", errno);
>       return false;
> -    } else if (pid == 0) {
> -      // Child subprocess.
> -      internal_close(STDOUT_FILENO);
> -      internal_close(STDIN_FILENO);
> -      internal_dup2(outfd[0], STDIN_FILENO);
> -      internal_dup2(infd[1], STDOUT_FILENO);
> -      internal_close(outfd[0]);
> -      internal_close(outfd[1]);
> -      internal_close(infd[0]);
> -      internal_close(infd[1]);
> -      for (int fd = sysconf(_SC_OPEN_MAX); fd > 2; fd--)
> -        internal_close(fd);
> -      const char *argv[kArgVMax];
> -      GetArgV(path_, argv);
> -      execv(path_, const_cast<char **>(&argv[0]));
> -      internal__exit(1);
>     }
> 
> -    // Continue execution in parent process.
> -    internal_close(outfd[0]);
> -    internal_close(infd[1]);
>     input_fd_ = infd[0];
>     output_fd_ = outfd[1];
>   }
> 
>   // Check that symbolizer subprocess started successfully.
> -  int pid_status;
>   SleepForMillis(kSymbolizerStartupTimeMillis);
> -  int exited_pid = waitpid(pid, &pid_status, WNOHANG);
> -  if (exited_pid != 0) {
> +  if (!IsProcessRunning(pid)) {
>     // Either waitpid failed, or child has already exited.
>     Report("WARNING: external symbolizer didn't start up correctly!\n");
>     return false;
> 
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc?rev=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc Tue Jan 26 14:10:01 2016
> @@ -775,6 +775,20 @@ char **GetArgv() {
>   return 0;
> }
> 
> +pid_t StartSubprocess(const char *program, const char *const argv[],
> +                      fd_t stdin_fd, fd_t stdout_fd, fd_t stderr_fd) {
> +  // FIXME: implement on this platform
> +  // Should be implemented based on
> +  // SymbolizerProcess::StarAtSymbolizerSubprocess
> +  // from lib/sanitizer_common/sanitizer_symbolizer_win.cc.
> +  return -1;
> +}
> +
> +bool IsProcessRunning(pid_t pid) {
> +  // FIXME: implement on this platform.
> +  return false;
> +}
> +
> }  // namespace __sanitizer
> 
> #endif  // _WIN32
> 
> Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc?rev=258849&r1=258848&r2=258849&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_linux_test.cc Tue Jan 26 14:10:01 2016
> @@ -263,6 +263,36 @@ TEST(SanitizerLinux, ThreadSelfTest) {
> }
> #endif
> 
> +TEST(SanitizerCommon, StartSubprocessTest) {
> +  int pipe_fds[2];
> +  ASSERT_EQ(0, pipe(pipe_fds));
> +  const char *argv[] = {"/bin/sh", "-c", "echo -n 'hello'"};
> +  int pid = StartSubprocess("/bin/sh", const_cast<char **>(&argv[0]),
> +                            kInvalidFd /* stdin */, pipe_fds[1] /* stdout */);
> +  ASSERT_GT(pid, 0);
> +
> +  // wait for process to finish.
> +  while (IsProcessRunning(pid)) {
> +  }
> +  ASSERT_FALSE(IsProcessRunning(pid));
> +
> +  char buffer[256];
> +  {
> +    char *ptr = buffer;
> +    uptr bytes_read;
> +    while (ReadFromFile(pipe_fds[0], ptr, 256, &bytes_read)) {
> +      if (!bytes_read) {
> +        break;
> +      }
> +      ptr += bytes_read;
> +    }
> +    ASSERT_EQ(5, ptr - buffer);
> +    *ptr = 0;
> +  }
> +  ASSERT_EQ(0, strcmp(buffer, "hello")) << "Buffer: " << buffer;
> +  internal_close(pipe_fds[0]);
> +}
> +
> }  // namespace __sanitizer
> 
> #endif  // SANITIZER_LINUX
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list