Symbolizer refactoring: Merge common parts of POSIXSymbolizer and WinSymbolizer

Reviewed at http://reviews.llvm.org/D8105

llvm-svn: 231680

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc

@@ -16,7 +16,7 @@
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_libc.h"
 #include "sanitizer_placement_new.h"
-#include "sanitizer_symbolizer.h"
+#include "sanitizer_symbolizer_internal.h"
 
 namespace __sanitizer {
 
@@ -68,13 +68,6 @@ Symbolizer *Symbolizer::symbolizer_;
 StaticSpinMutex Symbolizer::init_mu_;
 LowLevelAllocator Symbolizer::symbolizer_allocator_;
 
-Symbolizer *Symbolizer::Disable() {
-  CHECK_EQ(0, symbolizer_);
-  // Initialize a dummy symbolizer.
-  symbolizer_ = new(symbolizer_allocator_) Symbolizer;
-  return symbolizer_;
-}
-
 void Symbolizer::AddHooks(Symbolizer::StartSymbolizationHook start_hook,
                           Symbolizer::EndSymbolizationHook end_hook) {
   CHECK(start_hook_ == 0 && end_hook_ == 0);
@@ -82,7 +75,8 @@ void Symbolizer::AddHooks(Symbolizer::StartSymbolizationHook start_hook,
   end_hook_ = end_hook;
 }
 
-Symbolizer::Symbolizer() : start_hook_(0), end_hook_(0) {}
+Symbolizer::Symbolizer(IntrusiveList<SymbolizerTool> tools)
+    : tools_(tools), start_hook_(0), end_hook_(0) {}
 
 Symbolizer::SymbolizerScope::SymbolizerScope(const Symbolizer *sym)
     : sym_(sym) {
@@ -95,4 +89,76 @@ Symbolizer::SymbolizerScope::~SymbolizerScope() {
     sym_->end_hook_();
 }
 
+SymbolizedStack *Symbolizer::SymbolizePC(uptr addr) {
+  BlockingMutexLock l(&mu_);
+  const char *module_name;
+  uptr module_offset;
+  SymbolizedStack *res = SymbolizedStack::New(addr);
+  if (!PlatformFindModuleNameAndOffsetForAddress(addr, &module_name,
+                                                 &module_offset))
+    return res;
+  // Always fill data about module name and offset.
+  res->info.FillModuleInfo(module_name, module_offset);
+  for (auto iter = Iterator(&tools_); iter.hasNext();) {
+    auto *tool = iter.next();
+    SymbolizerScope sym_scope(this);
+    if (tool->SymbolizePC(addr, res)) {
+      return res;
+    }
+  }
+  return res;
+}
+
+bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
+  BlockingMutexLock l(&mu_);
+  const char *module_name;
+  uptr module_offset;
+  if (!PlatformFindModuleNameAndOffsetForAddress(addr, &module_name,
+                                                 &module_offset))
+    return false;
+  info->Clear();
+  info->module = internal_strdup(module_name);
+  info->module_offset = module_offset;
+  for (auto iter = Iterator(&tools_); iter.hasNext();) {
+    auto *tool = iter.next();
+    SymbolizerScope sym_scope(this);
+    if (tool->SymbolizeData(addr, info)) {
+      return true;
+    }
+  }
+  return true;
+}
+
+bool Symbolizer::GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
+                                 uptr *module_address) {
+  BlockingMutexLock l(&mu_);
+  return PlatformFindModuleNameAndOffsetForAddress(pc, module_name,
+                                                   module_address);
+}
+
+void Symbolizer::Flush() {
+  BlockingMutexLock l(&mu_);
+  for (auto iter = Iterator(&tools_); iter.hasNext();) {
+    auto *tool = iter.next();
+    SymbolizerScope sym_scope(this);
+    tool->Flush();
+  }
+}
+
+const char *Symbolizer::Demangle(const char *name) {
+  BlockingMutexLock l(&mu_);
+  for (auto iter = Iterator(&tools_); iter.hasNext();) {
+    auto *tool = iter.next();
+    SymbolizerScope sym_scope(this);
+    if (const char *demangled = tool->Demangle(name))
+      return demangled;
+  }
+  return PlatformDemangle(name);
+}
+
+void Symbolizer::PrepareForSandboxing() {
+  BlockingMutexLock l(&mu_);
+  PlatformPrepareForSandboxing();
+}
+
 }  // namespace __sanitizer

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h

@@ -73,6 +73,8 @@ struct DataInfo {
   void Clear();
 };
 
+class SymbolizerTool;
+
 class Symbolizer {
  public:
   /// Initialize and return platform-specific implementation of symbolizer
@@ -80,16 +82,10 @@ class Symbolizer {
   static Symbolizer *GetOrInit();
   // Returns a list of symbolized frames for a given address (containing
   // all inlined functions, if necessary).
-  virtual SymbolizedStack *SymbolizePC(uptr address) {
-    return SymbolizedStack::New(address);
-  }
-  virtual bool SymbolizeData(uptr address, DataInfo *info) {
-    return false;
-  }
-  virtual bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
-                                           uptr *module_address) {
-    return false;
-  }
+  SymbolizedStack *SymbolizePC(uptr address);
+  bool SymbolizeData(uptr address, DataInfo *info);
+  bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
+                                   uptr *module_address);
   const char *GetModuleNameForPc(uptr pc) {
     const char *module_name = 0;
     uptr unused;
@@ -97,16 +93,12 @@ class Symbolizer {
       return module_name;
     return nullptr;
   }
-  virtual bool CanReturnFileLineInfo() {
-    return false;
-  }
+  bool CanReturnFileLineInfo() { return !tools_.empty(); }
   // Release internal caches (if any).
-  virtual void Flush() {}
+  void Flush();
   // Attempts to demangle the provided C++ mangled name.
-  virtual const char *Demangle(const char *name) {
-    return name;
-  }
-  virtual void PrepareForSandboxing() {}
+  const char *Demangle(const char *name);
+  void PrepareForSandboxing();
 
   // Allow user to install hooks that would be called before/after Symbolizer
   // does the actual file/line info fetching. Specific sanitizers may need this
@@ -121,14 +113,28 @@ class Symbolizer {
  private:
   /// Platform-specific function for creating a Symbolizer object.
   static Symbolizer *PlatformInit();
-  /// Initialize the symbolizer in a disabled state.  Not thread safe.
-  static Symbolizer *Disable();
+
+  virtual bool PlatformFindModuleNameAndOffsetForAddress(
+      uptr address, const char **module_name, uptr *module_offset) {
+    UNIMPLEMENTED();
+  }
+  // Platform-specific default demangler, must not return nullptr.
+  virtual const char *PlatformDemangle(const char *name) { UNIMPLEMENTED(); }
+  virtual void PlatformPrepareForSandboxing() { UNIMPLEMENTED(); }
 
   static Symbolizer *symbolizer_;
   static StaticSpinMutex init_mu_;
 
+  // Mutex locked from public methods of |Symbolizer|, so that the internals
+  // (including individual symbolizer tools and platform-specific methods) are
+  // always synchronized.
+  BlockingMutex mu_;
+
+  typedef IntrusiveList<SymbolizerTool>::Iterator Iterator;
+  IntrusiveList<SymbolizerTool> tools_;
+
  protected:
-  Symbolizer();
+  explicit Symbolizer(IntrusiveList<SymbolizerTool> tools);
 
   static LowLevelAllocator symbolizer_allocator_;
 

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h

@@ -31,9 +31,9 @@ const char *ExtractUptr(const char *str, const char *delims, uptr *result);
 // Windows DbgHelp symbolizer, etc.).
 class SymbolizerTool {
  public:
-  // POSIXSymbolizer implements a "fallback chain" of symbolizer tools. In a
-  // request to symbolize an address, if one tool returns false, the next tool
-  // in the chain will be tried.
+  // The main |Symbolizer| class implements a "fallback chain" of symbolizer
+  // tools. In a request to symbolize an address, if one tool returns false,
+  // the next tool in the chain will be tried.
   SymbolizerTool *next;
 
   SymbolizerTool() : next(nullptr) { }
@@ -56,7 +56,7 @@ class SymbolizerTool {
 
   virtual void Flush() {}
 
-  // Return nullptr to fallback to the default __cxxabiv1 demangler.
+  // Return nullptr to fallback to the default platform-specific demangler.
   virtual const char *Demangle(const char *name) {
     return nullptr;
   }

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

@@ -51,9 +51,9 @@ Symbolizer *Symbolizer::GetOrInit() {
   SpinMutexLock l(&init_mu_);
   if (symbolizer_)
     return symbolizer_;
-  if ((symbolizer_ = PlatformInit()))
-    return symbolizer_;
-  return Disable();
+  symbolizer_ = PlatformInit();
+  CHECK(symbolizer_);
+  return symbolizer_;
 }
 
 }  // namespace __sanitizer

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

@@ -351,88 +351,21 @@ class InternalSymbolizer : public SymbolizerTool {
 class POSIXSymbolizer : public Symbolizer {
  public:
   explicit POSIXSymbolizer(IntrusiveList<SymbolizerTool> tools)
-      : Symbolizer(), tools_(tools) {}
+      : Symbolizer(tools) {}
 
-  SymbolizedStack *SymbolizePC(uptr addr) override {
-    BlockingMutexLock l(&mu_);
-    const char *module_name;
-    uptr module_offset;
-    SymbolizedStack *res = SymbolizedStack::New(addr);
-    if (!FindModuleNameAndOffsetForAddress(addr, &module_name, &module_offset))
-      return res;
-    // Always fill data about module name and offset.
-    res->info.FillModuleInfo(module_name, module_offset);
-    for (auto iter = Iterator(&tools_); iter.hasNext();) {
-      auto *tool = iter.next();
-      SymbolizerScope sym_scope(this);
-      if (tool->SymbolizePC(addr, res)) {
-        return 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;
-    for (auto iter = Iterator(&tools_); iter.hasNext();) {
-      auto *tool = iter.next();
-      SymbolizerScope sym_scope(this);
-      if (tool->SymbolizeData(addr, info)) {
-        return true;
-      }
-    }
-    return true;
-  }
-
-  bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
-                                   uptr *module_address) override {
-    BlockingMutexLock l(&mu_);
-    return FindModuleNameAndOffsetForAddress(pc, module_name, module_address);
-  }
-
-  bool CanReturnFileLineInfo() override {
-    return !tools_.empty();
-  }
-
-  void Flush() override {
-    BlockingMutexLock l(&mu_);
-    for (auto iter = Iterator(&tools_); iter.hasNext();) {
-      auto *tool = iter.next();
-      SymbolizerScope sym_scope(this);
-      tool->Flush();
-    }
-  }
-
-  const char *Demangle(const char *name) override {
-    BlockingMutexLock l(&mu_);
-    for (auto iter = Iterator(&tools_); iter.hasNext();) {
-      auto *tool = iter.next();
-      SymbolizerScope sym_scope(this);
-      if (const char *demangled = tool->Demangle(name))
-        return demangled;
-    }
+ private:
+  const char *PlatformDemangle(const char *name) override {
     return DemangleCXXABI(name);
   }
 
-  void PrepareForSandboxing() override {
+  void PlatformPrepareForSandboxing() override {
 #if SANITIZER_LINUX && !SANITIZER_ANDROID
-    BlockingMutexLock l(&mu_);
     // Cache /proc/self/exe on Linux.
     CacheBinaryName();
 #endif
   }
 
- private:
   LoadedModule *FindModuleForAddress(uptr address) {
-    mu_.CheckLocked();
     bool modules_were_reloaded = false;
     if (modules_ == 0 || !modules_fresh_) {
       modules_ = (LoadedModule*)(symbolizer_allocator_.Allocate(
@@ -461,9 +394,9 @@ class POSIXSymbolizer : public Symbolizer {
     return 0;
   }
 
-  bool FindModuleNameAndOffsetForAddress(uptr address, const char **module_name,
-                                         uptr *module_offset) {
-    mu_.CheckLocked();
+  bool PlatformFindModuleNameAndOffsetForAddress(uptr address,
+                                                 const char **module_name,
+                                                 uptr *module_offset) override {
     LoadedModule *module = FindModuleForAddress(address);
     if (module == 0)
       return false;
@@ -478,10 +411,6 @@ class POSIXSymbolizer : public Symbolizer {
   uptr n_modules_;
   // If stale, need to reload the modules before looking up addresses.
   bool modules_fresh_;
-  BlockingMutex mu_;
-
-  typedef IntrusiveList<SymbolizerTool>::Iterator Iterator;
-  IntrusiveList<SymbolizerTool> tools_;
 };
 
 static SymbolizerTool *ChooseSymbolizer(LowLevelAllocator *allocator) {

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_win.cc

@@ -147,42 +147,24 @@ bool FindModuleNameAndOffsetForAddress(uptr addr, const char **module_name,
 // TODO(kuba.brecka): To be merged with POSIXSymbolizer.
 class WinSymbolizer : public Symbolizer {
  public:
-  explicit WinSymbolizer(SymbolizerTool *tool) : Symbolizer(), tool_(tool) {
-    CHECK(tool);
-  }
-  SymbolizedStack *SymbolizePC(uptr addr) override {
-    BlockingMutexLock l(&mu_);
-    const char *module_name;
-    uptr module_offset;
-    SymbolizedStack *res = SymbolizedStack::New(addr);
-    if (FindModuleNameAndOffsetForAddress(addr, &module_name, &module_offset))
-      res->info.FillModuleInfo(module_name, module_offset);
-    tool_->SymbolizePC(addr, res);
-    return res;
-  }
-  bool CanReturnFileLineInfo() override {
-    return true;
-  }
-  const char *Demangle(const char *name) override {
-    BlockingMutexLock l(&mu_);
-    return tool_->Demangle(name);
-  }
-  bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
-                                   uptr *module_address) override {
-    BlockingMutexLock l(&mu_);
-    return FindModuleNameAndOffsetForAddress(pc, module_name, module_address);
-  }
+  explicit WinSymbolizer(IntrusiveList<SymbolizerTool> tools)
+      : Symbolizer(tools) {}
 
-  BlockingMutex mu_;
-  SymbolizerTool *tool_;
+ private:
+  bool PlatformFindModuleNameAndOffsetForAddress(
+      uptr addr, const char **module_name, uptr *module_offset) override {
+    return ::FindModuleNameAndOffsetForAddress(addr, module_name,
+                                               module_offset);
+  }
+  const char *PlatformDemangle(const char *name) override { return name; }
+  void PlatformPrepareForSandboxing() override { }
 };
 
 Symbolizer *Symbolizer::PlatformInit() {
-  static bool called_once = false;
-  CHECK(!called_once && "Shouldn't create more than one symbolizer");
-  called_once = true;
-  SymbolizerTool *tool = new(symbolizer_allocator_) WinSymbolizerTool();
-  return new(symbolizer_allocator_) WinSymbolizer(tool);
+  IntrusiveList<SymbolizerTool> list;
+  list.clear();
+  list.push_back(new(symbolizer_allocator_) WinSymbolizerTool());
+  return new(symbolizer_allocator_) WinSymbolizer(list);
 }
 
 }  // namespace __sanitizer