[compiler-rt] Symbolizer refactoring: Move internals to separate files

[kubamracek]

Summary

Part of http://reviews.llvm.org/D7827. Let's introduce a sanitizer_symbolizer_internal.h header file that will contain definitions and classes that are used by symbolizers, and keep sanitizer_symbolizer.h as the public interface for the sanitizers. This also moves the implementation of SymbolizerProcess into a separate file, sanitizer_symbolizer_process_libcdep.cc.

[samsonov]

LGTM

[kubamracek]

Landed in r231014.

lib/sanitizer_common/CMakeLists.txt

set(SANITIZER_LIBCDEP_SOURCES
   sanitizer_coverage_libcdep.cc
   sanitizer_coverage_mapping_libcdep.cc
   sanitizer_linux_libcdep.cc
   sanitizer_posix_libcdep.cc
   sanitizer_stacktrace_libcdep.cc
   sanitizer_stoptheworld_linux_libcdep.cc
   sanitizer_symbolizer_libcdep.cc
   sanitizer_symbolizer_posix_libcdep.cc
+  sanitizer_symbolizer_process_libcdep.cc
   sanitizer_unwind_posix_libcdep.cc)
 
 # Explicitly list all sanitizer_common headers. Not all of these are
 # included in sanitizer_common source files, but we need to depend on
 # headers when building our custom unit tests.
 set(SANITIZER_HEADERS
   sanitizer_addrhashmap.h
   sanitizer_allocator.h
set(SANITIZER_HEADERS
   sanitizer_report_decorator.h
   sanitizer_stackdepot.h
   sanitizer_stackdepotbase.h
   sanitizer_stacktrace.h
   sanitizer_stacktrace_printer.h
   sanitizer_stoptheworld.h
   sanitizer_suppressions.h
   sanitizer_symbolizer.h
+  sanitizer_symbolizer_internal.h
   sanitizer_symbolizer_libbacktrace.h
   sanitizer_syscall_generic.inc
   sanitizer_syscall_linux_x86_64.inc
   sanitizer_thread_registry.h)
 
 set(SANITIZER_COMMON_DEFINITIONS)
 
 if(MSVC)

lib/sanitizer_common/sanitizer_symbolizer.h

protected:
    public:
     explicit SymbolizerScope(const Symbolizer *sym);
     ~SymbolizerScope();
    private:
     const Symbolizer *sym_;
   };
 };
 
-class SymbolizerTool {
- public:
-  // Can't declare pure virtual functions in sanitizer runtimes:
-  // __cxa_pure_virtual might be unavailable.
-
-  // The |stack| parameter is inout. It is pre-filled with the address,
-  // module base and module offset values and is to be used to construct
-  // other stack frames.
-  virtual bool SymbolizePC(uptr addr, SymbolizedStack *stack) {
-    UNIMPLEMENTED();
-  }
-
-  // The |info| parameter is inout. It is pre-filled with the module base
-  // and module offset values.
-  virtual bool SymbolizeData(uptr addr, DataInfo *info) {
-    UNIMPLEMENTED();
-  }
-
-  virtual void Flush() {}
-
-  virtual const char *Demangle(const char *name) {
-    return name;
-  }
-};
-
-// 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:
-  explicit SymbolizerProcess(const char *path);
-  const char *SendCommand(const char *command);
-
- private:
-  bool Restart();
-  const char *SendCommandImpl(const char *command);
-  bool ReadFromSymbolizer(char *buffer, uptr max_length);
-  bool WriteToSymbolizer(const char *buffer, uptr length);
-  bool StartSymbolizerSubprocess();
-
-  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_internal.h

+//===-- sanitizer_symbolizer_internal.h -------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Header for internal classes and functions to be used by implementations of
+// symbolizers.
+//
+//===----------------------------------------------------------------------===//
+#ifndef SANITIZER_SYMBOLIZER_INTERNAL_H
+#define SANITIZER_SYMBOLIZER_INTERNAL_H
+
+#include "sanitizer_symbolizer.h"
+
+namespace __sanitizer {
+
+class SymbolizerTool {
+ public:
+  // Can't declare pure virtual functions in sanitizer runtimes:
+  // __cxa_pure_virtual might be unavailable.
+
+  // The |stack| parameter is inout. It is pre-filled with the address,
+  // module base and module offset values and is to be used to construct
+  // other stack frames.
+  virtual bool SymbolizePC(uptr addr, SymbolizedStack *stack) {
+    UNIMPLEMENTED();
+  }
+
+  // The |info| parameter is inout. It is pre-filled with the module base
+  // and module offset values.
+  virtual bool SymbolizeData(uptr addr, DataInfo *info) {
+    UNIMPLEMENTED();
+  }
+
+  virtual void Flush() {}
+
+  virtual const char *Demangle(const char *name) {
+    return name;
+  }
+};
+
+// 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:
+  explicit SymbolizerProcess(const char *path);
+  const char *SendCommand(const char *command);
+
+ private:
+  bool Restart();
+  const char *SendCommandImpl(const char *command);
+  bool ReadFromSymbolizer(char *buffer, uptr max_length);
+  bool WriteToSymbolizer(const char *buffer, uptr length);
+  bool StartSymbolizerSubprocess();
+
+  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_INTERNAL_H

lib/sanitizer_common/sanitizer_symbolizer_libbacktrace.h

 // run-time libraries.
 // Header for libbacktrace symbolizer.
 //===----------------------------------------------------------------------===//
 #ifndef SANITIZER_SYMBOLIZER_LIBBACKTRACE_H
 #define SANITIZER_SYMBOLIZER_LIBBACKTRACE_H
 
 #include "sanitizer_platform.h"
 #include "sanitizer_common.h"
-#include "sanitizer_symbolizer.h"
+#include "sanitizer_symbolizer_internal.h"
 
 #ifndef SANITIZER_LIBBACKTRACE
 # define SANITIZER_LIBBACKTRACE 0
 #endif
 
 #ifndef SANITIZER_CP_DEMANGLE
 # define SANITIZER_CP_DEMANGLE 0
 #endif

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

 //===-- sanitizer_symbolizer_libcdep.cc -----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is shared between AddressSanitizer and ThreadSanitizer
 // run-time libraries.
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_internal_defs.h"
-#include "sanitizer_symbolizer.h"
+#include "sanitizer_symbolizer_internal.h"
 
 namespace __sanitizer {
 
 Symbolizer *Symbolizer::GetOrInit() {
   SpinMutexLock l(&init_mu_);
   if (symbolizer_)
     return symbolizer_;
   if ((symbolizer_ = PlatformInit()))
     return symbolizer_;
   return Disable();
 }
 
 }  // namespace __sanitizer

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

 #if SANITIZER_POSIX
 #include "sanitizer_allocator_internal.h"
 #include "sanitizer_common.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_linux.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
-#include "sanitizer_symbolizer.h"
+#include "sanitizer_symbolizer_internal.h"
 #include "sanitizer_symbolizer_libbacktrace.h"
 
-#include <errno.h>
-#include <stdlib.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 // C++ demangling function, as required by Itanium C++ ABI. This is weak,
 // because we do not require a C++ ABI library to be linked to a program
 // using sanitizers; if it's not present, we'll just use the mangled name.
 namespace __cxxabiv1 {
   extern "C" SANITIZER_WEAK_ATTRIBUTE
   char *__cxa_demangle(const char *mangled, char *buffer,
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;
 }
 
-SymbolizerProcess::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');
-}
-
-const char *SymbolizerProcess::SendCommand(const char *command) {
-  for (; times_restarted_ < kMaxTimesRestarted; times_restarted_++) {
-    // Start or restart symbolizer if we failed to send command to it.
-    if (const char *res = SendCommandImpl(command))
-      return res;
-    Restart();
-  }
-  if (!failed_to_start_) {
-    Report("WARNING: Failed to use and restart external symbolizer!\n");
-    failed_to_start_ = true;
-  }
-  return 0;
-}
-
-bool SymbolizerProcess::Restart() {
-  if (input_fd_ != kInvalidFd)
-    internal_close(input_fd_);
-  if (output_fd_ != kInvalidFd)
-    internal_close(output_fd_);
-  return StartSymbolizerSubprocess();
-}
-
-const char *SymbolizerProcess::SendCommandImpl(const char *command) {
-  if (input_fd_ == kInvalidFd || output_fd_ == kInvalidFd)
-      return 0;
-  if (!WriteToSymbolizer(command, internal_strlen(command)))
-      return 0;
-  if (!ReadFromSymbolizer(buffer_, kBufferSize))
-      return 0;
-  return buffer_;
-}
-
-bool SymbolizerProcess::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::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::StartSymbolizerSubprocess() {
-  if (!FileExists(path_)) {
-    if (!reported_invalid_path_) {
-      Report("WARNING: invalid path to external symbolizer!\n");
-      reported_invalid_path_ = true;
-    }
-    return false;
-  }
-
-  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;
-}
-
-
 // Parses one or more two-line strings in the following format:
 //   <function_name>
 //   <file_name>:<line_number>[:<column_number>]
 // Used by LLVMSymbolizer, Addr2LinePool and InternalSymbolizer, since all of
 // them use the same output format.
 static void ParseSymbolizePCOutput(const char *str, SymbolizedStack *res) {
   bool top_frame = true;
   SymbolizedStack *last = res;

lib/sanitizer_common/sanitizer_symbolizer_process_libcdep.cc

+//===-- sanitizer_symbolizer_process_libcdep.cc ---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Implementation of SymbolizerProcess used by external symbolizers.
+//
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_platform.h"
+#if SANITIZER_POSIX
+#include "sanitizer_symbolizer_internal.h"
+
+#include <errno.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+namespace __sanitizer {
+
+SymbolizerProcess::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');
+}
+
+const char *SymbolizerProcess::SendCommand(const char *command) {
+  for (; times_restarted_ < kMaxTimesRestarted; times_restarted_++) {
+    // Start or restart symbolizer if we failed to send command to it.
+    if (const char *res = SendCommandImpl(command))
+      return res;
+    Restart();
+  }
+  if (!failed_to_start_) {
+    Report("WARNING: Failed to use and restart external symbolizer!\n");
+    failed_to_start_ = true;
+  }
+  return 0;
+}
+
+bool SymbolizerProcess::Restart() {
+  if (input_fd_ != kInvalidFd)
+    internal_close(input_fd_);
+  if (output_fd_ != kInvalidFd)
+    internal_close(output_fd_);
+  return StartSymbolizerSubprocess();
+}
+
+const char *SymbolizerProcess::SendCommandImpl(const char *command) {
+  if (input_fd_ == kInvalidFd || output_fd_ == kInvalidFd)
+      return 0;
+  if (!WriteToSymbolizer(command, internal_strlen(command)))
+      return 0;
+  if (!ReadFromSymbolizer(buffer_, kBufferSize))
+      return 0;
+  return buffer_;
+}
+
+bool SymbolizerProcess::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::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::StartSymbolizerSubprocess() {
+  if (!FileExists(path_)) {
+    if (!reported_invalid_path_) {
+      Report("WARNING: invalid path to external symbolizer!\n");
+      reported_invalid_path_ = true;
+    }
+    return false;
+  }
+
+  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;
+}
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_POSIX