[sanitizer_common] Fix SIGPIPE when symbolizer started by StartSubprocess dies

[ndrewh]

It's possible for the sanitized process to get SIGPIPE if the symbolizer dies. To prevent this from happening, #170809 added a field to SymbolizerProcess that can hold the read end of the child's stdin. This keeps the read end of the stdin pipe open for the lifetime of the SymbolizerProcess, which prevents us from getting SIGPIPE.

#170809 only uses this field on the posix_spawn path. This PR makes the change to the StartSubprocess path as well.

Closes #120915

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Andrew Haberlandt (ndrewh)

Changes

It's possible for the sanitized process to get SIGPIPE if the symbolizer dies. To prevent this from happening, #170809 added a field to SymbolizerProcess that can hold the read end of the child's stdin.

#170809 only uses this field on the posix_spawn path. This PR makes the change to the StartSubprocess path as well.

Closes #120915

---

Full diff: https://github.com/llvm/llvm-project/pull/171511.diff

2 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp (-3)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp (+2-4)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index 8e5e87938c372..2fdc093a726e8 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -523,9 +523,6 @@ pid_t StartSubprocess(const char *program, const char *const argv[],
                       const char *const envp[], 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);
     }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
index ab6aee7c9fba7..2c90a77a8e014 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -176,10 +176,6 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() {
       internal_close(outfd[1]);
       return false;
     }
-
-    // We intentionally hold on to the read-end so that we don't get a SIGPIPE
-    child_stdin_fd_ = outfd[0];
-
 #  else   // SANITIZER_APPLE
     UNIMPLEMENTED();
 #  endif  // SANITIZER_APPLE
@@ -195,6 +191,8 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() {
 
   input_fd_ = infd[0];
   output_fd_ = outfd[1];
+  // We intentionally hold on to the read-end so that we don't get a SIGPIPE
+  child_stdin_fd_ = outfd[0];
 
   CHECK_GT(pid, 0);
 

[ndrewh]

I'm not able to adequately test this change since we don't use this path on Darwin, so will need others to test before merging.