[compiler-rt] Symbolizer refactoring: Move SymbolizerProcess interface to header

[kubamracek]

Summary

Part of http://reviews.llvm.org/D7827. Moves SymbolizerProcess and ExternalSymbolizerInterface into the header. The plan is to later transform SymbolizerProcess into something that better abstracts the communication with an external process, and turn ExternalSymbolizerInterface into a generic tool interface that all symbolizers will implement.

[samsonov]

lib/sanitizer_common/sanitizer_symbolizer.h

156

Please add override here.

158

Why did you change private: to protected: ?

[kubamracek]

Addressing review comments: Added override, changed protected back to private (I realized I didn't have any good reason for that change).

[samsonov]

LGTM

[kubamracek]

Landed in r230530.

lib/sanitizer_common/sanitizer_symbolizer.h

protected:
    public:
     explicit SymbolizerScope(const Symbolizer *sym);
     ~SymbolizerScope();
    private:
     const Symbolizer *sym_;
   };
 };
 
+class ExternalSymbolizerInterface {
+ public:
+  // Can't declare pure virtual functions in sanitizer runtimes:
+  // __cxa_pure_virtual might be unavailable.
+  virtual char *SendCommand(bool is_data, const char *module_name,
+                            uptr module_offset) {
+    UNIMPLEMENTED();
+  }
+};
+
+// SymbolizerProcess encapsulates communication between the tool and
+// external symbolizer program, running in a different subprocess.
+// SymbolizerProcess may not be used from two threads simultaneously.
+class SymbolizerProcess : public ExternalSymbolizerInterface {
+ public:
+  explicit SymbolizerProcess(const char *path);
+  char *SendCommand(bool is_data, const char *module_name,
+                    uptr module_offset) override;
+
+ private:
+  bool Restart();
+  char *SendCommandImpl(bool is_data, const char *module_name,
+                        uptr module_offset);
+  bool ReadFromSymbolizer(char *buffer, uptr max_length);
+  bool WriteToSymbolizer(const char *buffer, uptr length);
+  bool StartSymbolizerSubprocess();
+
+  virtual bool RenderInputCommand(char *buffer, uptr max_length, bool is_data,
+                                  const char *module_name,
+                                  uptr module_offset) const {
+    UNIMPLEMENTED();
+  }
+
+  virtual bool ReachedEndOfOutput(const char *buffer, uptr length) const {
+    UNIMPLEMENTED();
+  }
+
+  virtual void ExecuteWithDefaultArgs(const char *path_to_binary) const {
+    UNIMPLEMENTED();
+  }
+
+  const char *path_;
+  int input_fd_;
+  int output_fd_;
+
+  static const uptr kBufferSize = 16 * 1024;
+  char buffer_[kBufferSize];
+
+  static const uptr kMaxTimesRestarted = 5;
+  static const int kSymbolizerStartupTimeMillis = 10;
+  uptr times_restarted_;
+  bool failed_to_start_;
+  bool reported_invalid_path_;
+};
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_SYMBOLIZER_H

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

static const char *ExtractUptr(const char *str, const char *delims,
   const char *ret = ExtractToken(str, delims, &buff);
   if (buff != 0) {
     *result = (uptr)internal_atoll(buff);
   }
   InternalFree(buff);
   return ret;
 }
 
-class ExternalSymbolizerInterface {
- public:
-  // Can't declare pure virtual functions in sanitizer runtimes:
-  // __cxa_pure_virtual might be unavailable.
-  virtual char *SendCommand(bool is_data, const char *module_name,
-                            uptr module_offset) {
-    UNIMPLEMENTED();
-  }
-};
-
-// SymbolizerProcess encapsulates communication between the tool and
-// external symbolizer program, running in a different subprocess.
-// SymbolizerProcess may not be used from two threads simultaneously.
-class SymbolizerProcess : public ExternalSymbolizerInterface {
- public:
-  explicit SymbolizerProcess(const char *path)
-      : path_(path),
-        input_fd_(kInvalidFd),
-        output_fd_(kInvalidFd),
-        times_restarted_(0),
-        failed_to_start_(false),
-        reported_invalid_path_(false) {
-    CHECK(path_);
-    CHECK_NE(path_[0], '\0');
-  }
+SymbolizerProcess::SymbolizerProcess(const char *path)
 
-  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
-    for (; times_restarted_ < kMaxTimesRestarted; times_restarted_++) {
-      // Start or restart symbolizer if we failed to send command to it.
-      if (char *res = SendCommandImpl(is_data, module_name, module_offset))
-        return res;
-      Restart();
-    }
-    if (!failed_to_start_) {
-      Report("WARNING: Failed to use and restart external symbolizer!\n");
-      failed_to_start_ = true;
-    }
-    return 0;
-  }
+char *SymbolizerProcess::SendCommand(bool is_data, const char *module_name, uptr module_offset) {
 
- private:
-  bool Restart() {
-    if (input_fd_ != kInvalidFd)
-      internal_close(input_fd_);
-    if (output_fd_ != kInvalidFd)
-      internal_close(output_fd_);
-    return StartSymbolizerSubprocess();
-  }
+bool SymbolizerProcess::Restart() {
 
-  char *SendCommandImpl(bool is_data, const char *module_name,
+char *SymbolizerProcess::SendCommandImpl(bool is_data, const char *module_name,
                                          uptr module_offset) {
-    if (input_fd_ == kInvalidFd || output_fd_ == kInvalidFd)
       return 0;
-    CHECK(module_name);
-    if (!RenderInputCommand(buffer_, kBufferSize, is_data, module_name,
-                            module_offset))
       return 0;
-    if (!writeToSymbolizer(buffer_, internal_strlen(buffer_)))
+  if (!WriteToSymbolizer(buffer_, internal_strlen(buffer_)))
       return 0;
-    if (!readFromSymbolizer(buffer_, kBufferSize))
+  if (!ReadFromSymbolizer(buffer_, kBufferSize))
       return 0;
-    return buffer_;
-  }
 
-  bool readFromSymbolizer(char *buffer, uptr max_length) {
-    if (max_length == 0)
-      return true;
-    uptr read_len = 0;
-    while (true) {
-      uptr just_read = internal_read(input_fd_, buffer + read_len,
-                                     max_length - read_len - 1);
-      // We can't read 0 bytes, as we don't expect external symbolizer to close
-      // its stdout.
-      if (just_read == 0 || just_read == (uptr)-1) {
-        Report("WARNING: Can't read from symbolizer at fd %d\n", input_fd_);
-        return false;
-      }
-      read_len += just_read;
-      if (ReachedEndOfOutput(buffer, read_len))
-        break;
-    }
-    buffer[read_len] = '\0';
-    return true;
-  }
+bool SymbolizerProcess::ReadFromSymbolizer(char *buffer, uptr max_length) {
 
-  bool writeToSymbolizer(const char *buffer, uptr length) {
-    if (length == 0)
-      return true;
-    uptr write_len = internal_write(output_fd_, buffer, length);
-    if (write_len == 0 || write_len == (uptr)-1) {
-      Report("WARNING: Can't write to symbolizer at fd %d\n", output_fd_);
-      return false;
-    }
-    return true;
-  }
+bool SymbolizerProcess::WriteToSymbolizer(const char *buffer, uptr length) {
 
-  bool StartSymbolizerSubprocess() {
-    if (!FileExists(path_)) {
-      if (!reported_invalid_path_) {
-        Report("WARNING: invalid path to external symbolizer!\n");
-        reported_invalid_path_ = true;
-      }
-      return false;
-    }
+bool SymbolizerProcess::StartSymbolizerSubprocess() {
 
-    int *infd = NULL;
-    int *outfd = NULL;
-    // The client program may close its stdin and/or stdout and/or stderr
-    // thus allowing socketpair to reuse file descriptors 0, 1 or 2.
-    // In this case the communication between the forked processes may be
-    // broken if either the parent or the child tries to close or duplicate
-    // these descriptors. The loop below produces two pairs of file
-    // descriptors, each greater than 2 (stderr).
-    int sock_pair[5][2];
-    for (int i = 0; i < 5; i++) {
-      if (pipe(sock_pair[i]) == -1) {
-        for (int j = 0; j < i; j++) {
-          internal_close(sock_pair[j][0]);
-          internal_close(sock_pair[j][1]);
-        }
-        Report("WARNING: Can't create a socket pair to start "
-               "external symbolizer (errno: %d)\n", errno);
-        return false;
-      } else if (sock_pair[i][0] > 2 && sock_pair[i][1] > 2) {
-        if (infd == NULL) {
-          infd = sock_pair[i];
-        } else {
-          outfd = sock_pair[i];
-          for (int j = 0; j < i; j++) {
-            if (sock_pair[j] == infd) continue;
-            internal_close(sock_pair[j][0]);
-            internal_close(sock_pair[j][1]);
-          }
-          break;
-        }
-      }
-    }
-    CHECK(infd);
-    CHECK(outfd);
 
-    // Real fork() may call user callbacks registered with pthread_atfork().
-    int pid = internal_fork();
-    if (pid == -1) {
-      // Fork() failed.
-      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);
-      ExecuteWithDefaultArgs(path_);
-      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) {
-      // Either waitpid failed, or child has already exited.
-      Report("WARNING: external symbolizer didn't start up correctly!\n");
-      return false;
-    }
 
-    return true;
-  }
 
-  virtual bool RenderInputCommand(char *buffer, uptr max_length, bool is_data,
-                                  const char *module_name,
-                                  uptr module_offset) const {
-    UNIMPLEMENTED();
-  }
-
-  virtual bool ReachedEndOfOutput(const char *buffer, uptr length) const {
-    UNIMPLEMENTED();
-  }
-
-  virtual void ExecuteWithDefaultArgs(const char *path_to_binary) const {
-    UNIMPLEMENTED();
-  }
-
-  const char *path_;
-  int input_fd_;
-  int output_fd_;
-
-  static const uptr kBufferSize = 16 * 1024;
-  char buffer_[kBufferSize];
-
-  static const uptr kMaxTimesRestarted = 5;
-  static const int kSymbolizerStartupTimeMillis = 10;
-  uptr times_restarted_;
-  bool failed_to_start_;
-  bool reported_invalid_path_;
-};
-
 // For now we assume the following protocol:
 // For each request of the form
 //   <module_name> <module_offset>
 // passed to STDIN, external symbolizer prints to STDOUT response:
 //   <function_name>
 //   <file_name>:<line_number>:<column_number>
 //   <function_name>
 //   <file_name>:<line_number>:<column_number>