SymbolizerProcess should hold on to the read end of the stdin pipe so we don't get SIGPIPE

Author: ndrewh

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

@@ -285,10 +285,14 @@ bool internal_spawn(const char* argv[], const char* envp[], pid_t* pid,
                     fd_t fd_stdin, fd_t fd_stdout) {
   // NOTE: Caller ensures that fd_stdin and fd_stdout are not 0, 1, or 2, since
   // this can break communication.
+  //
+  // NOTE: Caller is responsible for closing fd_stdin after the process has
+  // died.
 
   int res;
   auto fd_closer = at_scope_exit([&] {
-    internal_close(fd_stdin);
+    // NOTE: We intentionally do not close fd_stdin since this can
+    // cause us to receive a fatal SIGPIPE if the process dies.
     internal_close(fd_stdout);
   });
 

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h

@@ -83,7 +83,7 @@ class SymbolizerProcess {
   const char *SendCommand(const char *command);
 
  protected:
-  ~SymbolizerProcess() {}
+  ~SymbolizerProcess();
 
   /// The maximum number of arguments required to invoke a tool process.
   static const unsigned kArgVMax = 16;
@@ -114,6 +114,10 @@ class SymbolizerProcess {
   fd_t input_fd_;
   fd_t output_fd_;
 
+  // We hold on to the child's stdin fd (the read end of the pipe)
+  // so that when we write to it, we don't get a SIGPIPE
+  fd_t child_stdin_fd_;
+
   InternalMmapVector<char> buffer_;
 
   static const uptr kMaxTimesRestarted = 5;

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp

@@ -476,10 +476,11 @@ const char *LLVMSymbolizer::FormatAndSendCommand(const char *command_prefix,
   return symbolizer_process_->SendCommand(buffer_);
 }
 
-SymbolizerProcess::SymbolizerProcess(const char *path, bool use_posix_spawn)
+SymbolizerProcess::SymbolizerProcess(const char* path, bool use_posix_spawn)
     : path_(path),
       input_fd_(kInvalidFd),
       output_fd_(kInvalidFd),
+      child_stdin_fd_(kInvalidFd),
       times_restarted_(0),
       failed_to_start_(false),
       reported_invalid_path_(false),
@@ -488,6 +489,11 @@ SymbolizerProcess::SymbolizerProcess(const char *path, bool use_posix_spawn)
   CHECK_NE(path_[0], '\0');
 }
 
+SymbolizerProcess::~SymbolizerProcess() {
+  if (child_stdin_fd_ != kInvalidFd)
+    CloseFile(child_stdin_fd_);
+}
+
 static bool IsSameModule(const char *path) {
   if (const char *ProcessName = GetProcessName()) {
     if (const char *SymbolizerName = StripModuleName(path)) {
@@ -533,6 +539,8 @@ bool SymbolizerProcess::Restart() {
     CloseFile(input_fd_);
   if (output_fd_ != kInvalidFd)
     CloseFile(output_fd_);
+  if (child_stdin_fd_ != kInvalidFd)
+    CloseFile(output_fd_);
   return StartSymbolizerSubprocess();
 }
 

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp

@@ -192,6 +192,9 @@ 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);
 
   // Check that symbolizer subprocess started successfully.