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