[compiler-rt] Symbolizer refactoring: Abstract SymbolizerProcess better

[kubamracek]

Summary

Part of http://reviews.llvm.org/D7827. Makes SymbolizerProcess operate on strings and not on modules and offsets. Removes the formatting of input commands from SymbolizerProcess and moves it to individual symbolizers.

[samsonov]

Looks like a step in right direction.

lib/sanitizer_common/sanitizer_symbolizer.h

143–144

This should also return const char *

156

This function should probably return const char * instead.

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

374

Add

CHECK(module_name);

here

420–421

add override here

439

ARRAY_SIZE(buffer_), or use named constant.

[kubamracek]

Addressing review comments.

[samsonov]

LGTM

[kubamracek]

Landed in r230749.

lib/sanitizer_common/sanitizer_symbolizer.h

protected:
    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,
+  virtual const 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 {
+class SymbolizerProcess {
  public:
   explicit SymbolizerProcess(const char *path);
-  char *SendCommand(bool is_data, const char *module_name,
-                    uptr module_offset) override;
+  const char *SendCommand(const char *command);
 
  private:
   bool Restart();
-  char *SendCommandImpl(bool is_data, const char *module_name,
-                        uptr module_offset);
+  const char *SendCommandImpl(const char *command);
   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();
   }
 

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

: path_(path),
       output_fd_(kInvalidFd),
       times_restarted_(0),
       failed_to_start_(false),
       reported_invalid_path_(false) {
   CHECK(path_);
   CHECK_NE(path_[0], '\0');
 }
 
-char *SymbolizerProcess::SendCommand(bool is_data, const char *module_name,
-                                     uptr module_offset) {
+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 (char *res = SendCommandImpl(is_data, module_name, module_offset))
+    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();
 }
 
-char *SymbolizerProcess::SendCommandImpl(bool is_data, const char *module_name,
-                                         uptr module_offset) {
+const char *SymbolizerProcess::SendCommandImpl(const char *command) {
   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(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)

 //   <file_name>:<line_number>:<column_number>
 //   ...
 //   <empty line>
 class LLVMSymbolizerProcess : public SymbolizerProcess {
  public:
   explicit LLVMSymbolizerProcess(const char *path) : SymbolizerProcess(path) {}
 
  private:
-  bool RenderInputCommand(char *buffer, uptr max_length, bool is_data,
-                          const char *module_name, uptr module_offset) const {
-    internal_snprintf(buffer, max_length, "%s\"%s\" 0x%zx\n",
-                      is_data ? "DATA " : "", module_name, module_offset);
-    return true;
-  }
-
   bool ReachedEndOfOutput(const char *buffer, uptr length) const {
     // Empty line marks the end of llvm-symbolizer output.
     return length >= 2 && buffer[length - 1] == '\n' &&
            buffer[length - 2] == '\n';
   }
 
   void ExecuteWithDefaultArgs(const char *path_to_binary) const {
 #if defined(__x86_64__)
#endif
     const char *const inline_flag = common_flags()->symbolize_inline_frames
                                         ? "--inlining=true"
                                         : "--inlining=false";
     execl(path_to_binary, path_to_binary, inline_flag, kSymbolizerArch,
           (char *)0);
   }
 };
 
+class LLVMSymbolizer : public ExternalSymbolizerInterface {
+ public:
+  explicit LLVMSymbolizer(const char *path, LowLevelAllocator *allocator)
+      : symbolizer_process_(new(*allocator) LLVMSymbolizerProcess(path)) {}
+
+  const char *SendCommand(bool is_data, const char *module_name,
+                          uptr module_offset) override {
+    CHECK(module_name);
+    internal_snprintf(buffer_, kBufferSize, "%s\"%s\" 0x%zx\n",
+                      is_data ? "DATA " : "", module_name, module_offset);
+    return symbolizer_process_->SendCommand(buffer_);
+  }
+
+ private:
+  LLVMSymbolizerProcess *symbolizer_process_;
+  static const uptr kBufferSize = 16 * 1024;
+  char buffer_[kBufferSize];
+};
+
 class Addr2LineProcess : public SymbolizerProcess {
  public:
   Addr2LineProcess(const char *path, const char *module_name)
       : SymbolizerProcess(path), module_name_(internal_strdup(module_name)) {}
 
   const char *module_name() const { return module_name_; }
 
  private:
-  bool RenderInputCommand(char *buffer, uptr max_length, bool is_data,
-                          const char *module_name, uptr module_offset) const {
-    if (is_data)
-      return false;
-    CHECK_EQ(0, internal_strcmp(module_name, module_name_));
-    internal_snprintf(buffer, max_length, "0x%zx\n", module_offset);
-    return true;
-  }
-
   bool ReachedEndOfOutput(const char *buffer, uptr length) const {
     // Output should consist of two lines.
     int num_lines = 0;
     for (uptr i = 0; i < length; ++i) {
       if (buffer[i] == '\n')
         num_lines++;
       if (num_lines >= 2)
         return true;

 
 class Addr2LinePool : public ExternalSymbolizerInterface {
  public:
   explicit Addr2LinePool(const char *addr2line_path,
                          LowLevelAllocator *allocator)
       : addr2line_path_(addr2line_path), allocator_(allocator),
         addr2line_pool_(16) {}
 
-  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
+  const char *SendCommand(bool is_data, const char *module_name,
+                          uptr module_offset) override {
     if (is_data)
       return 0;
     Addr2LineProcess *addr2line = 0;
     for (uptr i = 0; i < addr2line_pool_.size(); ++i) {
       if (0 ==
           internal_strcmp(module_name, addr2line_pool_[i]->module_name())) {
         addr2line = addr2line_pool_[i];
         break;
       }
     }
     if (!addr2line) {
       addr2line =
           new(*allocator_) Addr2LineProcess(addr2line_path_, module_name);
       addr2line_pool_.push_back(addr2line);
     }
-    return addr2line->SendCommand(is_data, module_name, module_offset);
+    CHECK_EQ(0, internal_strcmp(module_name, addr2line->module_name()));
+    char buffer_[kBufferSize];
+    internal_snprintf(buffer_, kBufferSize, "0x%zx\n", module_offset);
+    return addr2line->SendCommand(buffer_);
   }
 
  private:
+  static const uptr kBufferSize = 32;
   const char *addr2line_path_;
   LowLevelAllocator *allocator_;
   InternalMmapVector<Addr2LineProcess*> addr2line_pool_;
 };
 
 #if SANITIZER_SUPPORTS_WEAK_HOOKS
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE

 #if SANITIZER_LINUX && !SANITIZER_ANDROID
     BlockingMutexLock l(&mu_);
     // Cache /proc/self/exe on Linux.
     CacheBinaryName();
 #endif
   }
 
  private:
-  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
+  const char *SendCommand(bool is_data, const char *module_name,
+                          uptr module_offset) {
     mu_.CheckLocked();
     // First, try to use internal symbolizer.
     if (internal_symbolizer_) {
       SymbolizerScope sym_scope(this);
       return internal_symbolizer_->SendCommand(is_data, module_name,
                                                module_offset);
     }
     // Otherwise, fall back to external symbolizer.
if (!libbacktrace_symbolizer) {
       if (path_to_external && path_to_external[0] == '\0') {
         // External symbolizer is explicitly disabled. Do nothing.
       } else {
         // Find path to llvm-symbolizer if it's not provided.
         if (!path_to_external)
           path_to_external = FindPathToBinary("llvm-symbolizer");
         if (path_to_external) {
           external_symbolizer = new(symbolizer_allocator_)
-              LLVMSymbolizerProcess(path_to_external);
+              LLVMSymbolizer(path_to_external, &symbolizer_allocator_);
         } else if (common_flags()->allow_addr2line) {
           // If llvm-symbolizer is not found, try to use addr2line.
           if (const char *addr2line_path = FindPathToBinary("addr2line")) {
             external_symbolizer = new(symbolizer_allocator_)
                 Addr2LinePool(addr2line_path, &symbolizer_allocator_);
           }
         }
       }