[compiler-rt] Symbolizer refactoring: SymbolizerTool and better interface

[kubamracek]

Summary

Part of http://reviews.llvm.org/D7827. This patch transforms ExternalSymbolizerInterface into SymbolizerTool and makes it a more high-level interface that (eventually) all symbolizer tools should implement. This makes both the formatting of the input and parsing of the output be the responsibility of the individual tool. For now, we make LLVMSymbolizer, Addr2LinePool and InternalSymbolizer be the implementations of this interface (I'll do LibbacktraceSymbolizer in a next step).

[samsonov]

LGTM, but please address all the comments below before committing (there's a bug introduced in InteranalSymbolizer, so be careful). Keep an eye on WinSymbolizer - looks like we will be able to inherit it from SymbolizerTool as well, make our current "POSIXSymbolizer" class the only class implementing "Symbolizer" interface, and get rid of the latter.

Thanks for doing this!

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

373

if (const char *res = SendCommand(...)) {
  ParseSymbolizePCOutput(res, stack);
  return true;
}
return false;

380

same here

435

same here

498

Here you should call __sanitizer_symbolize_code

505

And here - __sanitizer_symbolize_data

577

if (SymbolizerTool *tool = GetSymbolizerTool()) {
  ...
}

601

same here

[kubamracek]

Landed in r230842.

In D7936#131397, @samsonov wrote:

Keep an eye on WinSymbolizer - looks like we will be able to inherit it from SymbolizerTool as well, make our current "POSIXSymbolizer" class the only class implementing "Symbolizer" interface, and get rid of the latter.

Yes, that's my plan.

lib/sanitizer_common/sanitizer_symbolizer.h

protected:
    public:
     explicit SymbolizerScope(const Symbolizer *sym);
     ~SymbolizerScope();
    private:
     const Symbolizer *sym_;
   };
 };
 
-class ExternalSymbolizerInterface {
+class SymbolizerTool {
  public:
   // Can't declare pure virtual functions in sanitizer runtimes:
   // __cxa_pure_virtual might be unavailable.
-  virtual const char *SendCommand(bool is_data, const char *module_name,
-                                  uptr module_offset) {
+
+  // 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);

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

#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 {
+class LLVMSymbolizer : public SymbolizerTool {
  public:
   explicit LLVMSymbolizer(const char *path, LowLevelAllocator *allocator)
       : symbolizer_process_(new(*allocator) LLVMSymbolizerProcess(path)) {}
 
+  bool SymbolizePC(uptr addr, SymbolizedStack *stack) override {
+    const char *buf = SendCommand(/*is_data*/ false, stack->info.module,
+                                  stack->info.module_offset);
+    if (buf) ParseSymbolizePCOutput(buf, stack);
+    return buf != nullptr;
+  }
+
+  bool SymbolizeData(uptr addr, DataInfo *info) override {
+    const char *buf =
+        SendCommand(/*is_data*/ false, info->module, info->module_offset);
+    if (buf) ParseSymbolizeDataOutput(buf, info);
+    return buf != nullptr;
+  }
+
+ private:
   const char *SendCommand(bool is_data, const char *module_name,
-                          uptr module_offset) override {
+                          uptr module_offset) {
     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)
private:
 
   void ExecuteWithDefaultArgs(const char *path_to_binary) const {
     execl(path_to_binary, path_to_binary, "-Cfe", module_name_, (char *)0);
   }
 
   const char *module_name_;  // Owned, leaked.
 };
 
-class Addr2LinePool : public ExternalSymbolizerInterface {
+class Addr2LinePool : public SymbolizerTool {
  public:
   explicit Addr2LinePool(const char *addr2line_path,
                          LowLevelAllocator *allocator)
       : addr2line_path_(addr2line_path), allocator_(allocator),
         addr2line_pool_(16) {}
 
-  const char *SendCommand(bool is_data, const char *module_name,
-                          uptr module_offset) override {
-    if (is_data)
-      return 0;
+  bool SymbolizePC(uptr addr, SymbolizedStack *stack) override {
+    const char *buf =
+        SendCommand(stack->info.module, stack->info.module_offset);
+    if (buf) ParseSymbolizePCOutput(buf, stack);
+    return buf != nullptr;
+  }
+
+  bool SymbolizeData(uptr addr, DataInfo *info) override {
+    return false;
+  }
+
+ private:
+  const char *SendCommand(const char *module_name, uptr module_offset) {
     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);
     }
     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
 bool __sanitizer_symbolize_code(const char *ModuleName, u64 ModuleOffset,
                                 char *Buffer, int MaxLength);
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 bool __sanitizer_symbolize_data(const char *ModuleName, u64 ModuleOffset,
                                 char *Buffer, int MaxLength);
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 void __sanitizer_symbolize_flush();
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 int __sanitizer_symbolize_demangle(const char *Name, char *Buffer,
                                    int MaxLength);
 }  // extern "C"
 
-class InternalSymbolizer {
+class InternalSymbolizer : public SymbolizerTool {
  public:
-  typedef bool (*SanitizerSymbolizeFn)(const char*, u64, char*, int);
-
   static InternalSymbolizer *get(LowLevelAllocator *alloc) {
     if (__sanitizer_symbolize_code != 0 &&
         __sanitizer_symbolize_data != 0) {
       return new(*alloc) InternalSymbolizer();
     }
     return 0;
   }
 
-  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
-    SanitizerSymbolizeFn symbolize_fn = is_data ? __sanitizer_symbolize_data
-                                                : __sanitizer_symbolize_code;
-    if (symbolize_fn(module_name, module_offset, buffer_, kBufferSize))
-      return buffer_;
-    return 0;
+  bool SymbolizePC(uptr addr, SymbolizedStack *stack) override {
+    bool result = __sanitizer_symbolize_data(
+        stack->info.module, stack->info.module_offset, buffer_, kBufferSize);
+    if (result) ParseSymbolizePCOutput(buffer_, stack);
+    return result;
   }
 
-  void Flush() {
+  bool SymbolizeData(uptr addr, DataInfo *info) override {
+    bool result = __sanitizer_symbolize_code(info->module, info->module_offset,
+                                             buffer_, kBufferSize);
+    if (result) ParseSymbolizeDataOutput(buffer_, info);
+    return result;
+  }
+
+  void Flush() override {
     if (__sanitizer_symbolize_flush)
       __sanitizer_symbolize_flush();
   }
 
-  const char *Demangle(const char *name) {
+  const char *Demangle(const char *name) override {
     if (__sanitizer_symbolize_demangle) {
       for (uptr res_length = 1024;
            res_length <= InternalSizeClassMap::kMaxSize;) {
         char *res_buff = static_cast<char*>(InternalAlloc(res_length));
         uptr req_length =
             __sanitizer_symbolize_demangle(name, res_buff, res_length);
         if (req_length > res_length) {
           res_length = req_length + 1;
private:
   InternalSymbolizer() { }
 
   static const int kBufferSize = 16 * 1024;
   static const int kMaxDemangledNameSize = 1024;
   char buffer_[kBufferSize];
 };
 #else  // SANITIZER_SUPPORTS_WEAK_HOOKS
 
-class InternalSymbolizer {
+class InternalSymbolizer : public SymbolizerTool {
  public:
   static InternalSymbolizer *get(LowLevelAllocator *alloc) { return 0; }
-  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
-    return 0;
-  }
-  void Flush() { }
-  const char *Demangle(const char *name) { return name; }
 };
 
 #endif  // SANITIZER_SUPPORTS_WEAK_HOOKS
 
 class POSIXSymbolizer : public Symbolizer {
  public:
-  POSIXSymbolizer(ExternalSymbolizerInterface *external_symbolizer,
-                  InternalSymbolizer *internal_symbolizer,
+  POSIXSymbolizer(SymbolizerTool *external_symbolizer,
+                  SymbolizerTool *internal_symbolizer,
                   LibbacktraceSymbolizer *libbacktrace_symbolizer)
       : Symbolizer(),
         external_symbolizer_(external_symbolizer),
         internal_symbolizer_(internal_symbolizer),
         libbacktrace_symbolizer_(libbacktrace_symbolizer) {}
 
   SymbolizedStack *SymbolizePC(uptr addr) override {
     BlockingMutexLock l(&mu_);
     const char *module_name;
     uptr module_offset;
     if (!FindModuleNameAndOffsetForAddress(addr, &module_name, &module_offset))
       return SymbolizedStack::New(addr);
     // First, try to use libbacktrace symbolizer (if it's available).
     if (libbacktrace_symbolizer_ != 0) {
       mu_.CheckLocked();
       if (SymbolizedStack *res = libbacktrace_symbolizer_->SymbolizeCode(
               addr, module_name, module_offset))
         return res;
     }
     // Always fill data about module name and offset.
     SymbolizedStack *res = SymbolizedStack::New(addr);
     res->info.FillAddressAndModuleInfo(addr, module_name, module_offset);
 
-    const char *str = SendCommand(false, module_name, module_offset);
-    if (str == 0) {
-      // Symbolizer was not initialized or failed.
-      return res;
+    SymbolizerTool *tool = GetSymbolizerTool();
+    if (tool) {
+      SymbolizerScope sym_scope(this);
+      tool->SymbolizePC(addr, res);
     }
-
-    ParseSymbolizePCOutput(str, res);
     return res;
   }
 
   bool SymbolizeData(uptr addr, DataInfo *info) override {
     BlockingMutexLock l(&mu_);
     LoadedModule *module = FindModuleForAddress(addr);
     if (module == 0)
       return false;
     const char *module_name = module->full_name();
     uptr module_offset = addr - module->base_address();
     info->Clear();
     info->module = internal_strdup(module_name);
     info->module_offset = module_offset;
     // First, try to use libbacktrace symbolizer (if it's available).
     if (libbacktrace_symbolizer_ != 0) {
       mu_.CheckLocked();
       if (libbacktrace_symbolizer_->SymbolizeData(addr, info))
         return true;
     }
-    const char *str = SendCommand(true, module_name, module_offset);
-    if (str == 0)
-      return true;
-    ParseSymbolizeDataOutput(str, info);
+    SymbolizerTool *tool = GetSymbolizerTool();
+    if (tool) {
+      SymbolizerScope sym_scope(this);
+      tool->SymbolizeData(addr, info);
+    }
     info->start += module->base_address();
     return true;
   }
 
   bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
                                    uptr *module_address) override {
     BlockingMutexLock l(&mu_);
     return FindModuleNameAndOffsetForAddress(pc, module_name, module_address);

 #if SANITIZER_LINUX && !SANITIZER_ANDROID
     BlockingMutexLock l(&mu_);
     // Cache /proc/self/exe on Linux.
     CacheBinaryName();
 #endif
   }
 
  private:
-  const char *SendCommand(bool is_data, const char *module_name,
-                          uptr module_offset) {
+  SymbolizerTool *GetSymbolizerTool() {
     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 (external_symbolizer_) {
-      SymbolizerScope sym_scope(this);
-      return external_symbolizer_->SendCommand(is_data, module_name,
-                                               module_offset);
-    }
-    return 0;
+    if (internal_symbolizer_) return internal_symbolizer_;
+    if (external_symbolizer_) return external_symbolizer_;
+    return nullptr;
   }
 
   LoadedModule *FindModuleForAddress(uptr address) {
     mu_.CheckLocked();
     bool modules_were_reloaded = false;
     if (modules_ == 0 || !modules_fresh_) {
       modules_ = (LoadedModule*)(symbolizer_allocator_.Allocate(
           kMaxNumberOfModuleContexts * sizeof(LoadedModule)));
private:
   // 16K loaded modules should be enough for everyone.
   static const uptr kMaxNumberOfModuleContexts = 1 << 14;
   LoadedModule *modules_;  // Array of module descriptions is leaked.
   uptr n_modules_;
   // If stale, need to reload the modules before looking up addresses.
   bool modules_fresh_;
   BlockingMutex mu_;
 
-  ExternalSymbolizerInterface *external_symbolizer_;  // Leaked.
-  InternalSymbolizer *const internal_symbolizer_;     // Leaked.
+  SymbolizerTool *const external_symbolizer_;         // Leaked.
+  SymbolizerTool *const internal_symbolizer_;         // Leaked.
   LibbacktraceSymbolizer *libbacktrace_symbolizer_;   // Leaked.
 };
 
 Symbolizer *Symbolizer::PlatformInit() {
   if (!common_flags()->symbolize) {
     return new(symbolizer_allocator_) POSIXSymbolizer(0, 0, 0);
   }
   InternalSymbolizer* internal_symbolizer =
       InternalSymbolizer::get(&symbolizer_allocator_);
-  ExternalSymbolizerInterface *external_symbolizer = 0;
+  SymbolizerTool *external_symbolizer = 0;
   LibbacktraceSymbolizer *libbacktrace_symbolizer = 0;
 
   if (!internal_symbolizer) {
     libbacktrace_symbolizer =
         LibbacktraceSymbolizer::get(&symbolizer_allocator_);
     if (!libbacktrace_symbolizer) {
       const char *path_to_external = common_flags()->external_symbolizer_path;
       if (path_to_external && path_to_external[0] == '\0') {