[compiler-rt] Get rid of PlatformGetListOfModules

[kubamracek]

Summary

Moving the implementation of several functions from sanitizer_symbolizer.cc into sanitizer_symbolizer_libcdep.cc.

[timurrrr]

The change looks good to me assuming you've tested on both Linux and Windows,
but I think Alexey should take a quick look too.

[samsonov]

LGTM

[kubamracek]

Landed in r234472.

lib/sanitizer_common/sanitizer_symbolizer.h

private:
   } module_names_;
 
   /// Platform-specific function for creating a Symbolizer object.
   static Symbolizer *PlatformInit();
 
   bool FindModuleNameAndOffsetForAddress(uptr address, const char **module_name,
                                          uptr *module_offset);
   LoadedModule *FindModuleForAddress(uptr address);
-  // FIXME: get rid of this virtual method, just use GetListOfModules directly.
-  // The only reason we don't do it right away is that GetListOfModules is
-  // currently implemented in a libcdep file.
-  virtual uptr PlatformGetListOfModules(LoadedModule *modules,
-                                        uptr max_modules) {
-    UNIMPLEMENTED();
-  }
   LoadedModule modules_[kMaxNumberOfModules];
   uptr n_modules_;
   // If stale, need to reload the modules before looking up addresses.
   bool modules_fresh_;
 
   // Platform-specific default demangler, must not return nullptr.
   virtual const char *PlatformDemangle(const char *name) { UNIMPLEMENTED(); }
   virtual void PlatformPrepareForSandboxing() { UNIMPLEMENTED(); }

lib/sanitizer_common/sanitizer_symbolizer.cc

if (!internal_strcmp(storage_[i], str)) {
       return last_match_;
     }
   }
   last_match_ = internal_strdup(str);
   storage_.push_back(last_match_);
   return last_match_;
 }
 
-bool Symbolizer::FindModuleNameAndOffsetForAddress(uptr address,
-                                                   const char **module_name,
-                                                   uptr *module_offset) {
-  LoadedModule *module = FindModuleForAddress(address);
-  if (module == 0)
-    return false;
-  *module_name = module->full_name();
-  *module_offset = address - module->base_address();
-  return true;
-}
-
-LoadedModule *Symbolizer::FindModuleForAddress(uptr address) {
-  bool modules_were_reloaded = false;
-  if (!modules_fresh_) {
-    for (uptr i = 0; i < n_modules_; i++)
-      modules_[i].clear();
-    n_modules_ = PlatformGetListOfModules(modules_, kMaxNumberOfModules);
-    CHECK_GT(n_modules_, 0);
-    CHECK_LT(n_modules_, kMaxNumberOfModules);
-    modules_fresh_ = true;
-    modules_were_reloaded = true;
-  }
-  for (uptr i = 0; i < n_modules_; i++) {
-    if (modules_[i].containsAddress(address)) {
-      return &modules_[i];
-    }
-  }
-  // Reload the modules and look up again, if we haven't tried it yet.
-  if (!modules_were_reloaded) {
-    // FIXME: set modules_fresh_ from dlopen()/dlclose() interceptors.
-    // It's too aggressive to reload the list of modules each time we fail
-    // to find a module for a given address.
-    modules_fresh_ = false;
-    return FindModuleForAddress(address);
-  }
-  return 0;
-}
-
 Symbolizer::Symbolizer(IntrusiveList<SymbolizerTool> tools)
     : module_names_(&mu_), n_modules_(0), modules_fresh_(false), tools_(tools),
       start_hook_(0), end_hook_(0) {}
 
 Symbolizer::SymbolizerScope::SymbolizerScope(const Symbolizer *sym)
     : sym_(sym) {
   if (sym_->start_hook_)
     sym_->start_hook_();
 }
 
 Symbolizer::SymbolizerScope::~SymbolizerScope() {
   if (sym_->end_hook_)
     sym_->end_hook_();
 }
 
-SymbolizedStack *Symbolizer::SymbolizePC(uptr addr) {
-  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 Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
-  BlockingMutexLock l(&mu_);
-  const char *module_name;
-  uptr module_offset;
-  if (!FindModuleNameAndOffsetForAddress(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_);
-  const char *internal_module_name = nullptr;
-  if (!FindModuleNameAndOffsetForAddress(pc, &internal_module_name,
-                                         module_address))
-    return false;
-
-  if (module_name)
-    *module_name = module_names_.GetOwnedCopy(internal_module_name);
-  return true;
-}
-
-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

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

const char *ExtractTokenUpToDelimiter(const char *str, const char *delimiter,
   *result = (char *)InternalAlloc(prefix_len + 1);
   internal_memcpy(*result, str, prefix_len);
   (*result)[prefix_len] = '\0';
   const char *prefix_end = str + prefix_len;
   if (*prefix_end != '\0') prefix_end += internal_strlen(delimiter);
   return prefix_end;
 }
 
+SymbolizedStack *Symbolizer::SymbolizePC(uptr addr) {
+  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 Symbolizer::SymbolizeData(uptr addr, DataInfo *info) {
+  BlockingMutexLock l(&mu_);
+  const char *module_name;
+  uptr module_offset;
+  if (!FindModuleNameAndOffsetForAddress(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_);
+  const char *internal_module_name = nullptr;
+  if (!FindModuleNameAndOffsetForAddress(pc, &internal_module_name,
+                                         module_address))
+    return false;
+
+  if (module_name)
+    *module_name = module_names_.GetOwnedCopy(internal_module_name);
+  return true;
+}
+
+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();
+}
+
+bool Symbolizer::FindModuleNameAndOffsetForAddress(uptr address,
+                                                   const char **module_name,
+                                                   uptr *module_offset) {
+  LoadedModule *module = FindModuleForAddress(address);
+  if (module == 0)
+    return false;
+  *module_name = module->full_name();
+  *module_offset = address - module->base_address();
+  return true;
+}
+
+LoadedModule *Symbolizer::FindModuleForAddress(uptr address) {
+  bool modules_were_reloaded = false;
+  if (!modules_fresh_) {
+    for (uptr i = 0; i < n_modules_; i++)
+      modules_[i].clear();
+    n_modules_ =
+        GetListOfModules(modules_, kMaxNumberOfModules, /* filter */ nullptr);
+    CHECK_GT(n_modules_, 0);
+    CHECK_LT(n_modules_, kMaxNumberOfModules);
+    modules_fresh_ = true;
+    modules_were_reloaded = true;
+  }
+  for (uptr i = 0; i < n_modules_; i++) {
+    if (modules_[i].containsAddress(address)) {
+      return &modules_[i];
+    }
+  }
+  // Reload the modules and look up again, if we haven't tried it yet.
+  if (!modules_were_reloaded) {
+    // FIXME: set modules_fresh_ from dlopen()/dlclose() interceptors.
+    // It's too aggressive to reload the list of modules each time we fail
+    // to find a module for a given address.
+    modules_fresh_ = false;
+    return FindModuleForAddress(address);
+  }
+  return 0;
+}
+
 Symbolizer *Symbolizer::GetOrInit() {
   SpinMutexLock l(&init_mu_);
   if (symbolizer_)
     return symbolizer_;
   symbolizer_ = PlatformInit();
   CHECK(symbolizer_);
   return symbolizer_;
 }
 
 }  // namespace __sanitizer

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

 #endif  // SANITIZER_SUPPORTS_WEAK_HOOKS
 
 class POSIXSymbolizer : public Symbolizer {
  public:
   explicit POSIXSymbolizer(IntrusiveList<SymbolizerTool> tools)
       : Symbolizer(tools) {}
 
  private:
-  uptr PlatformGetListOfModules(LoadedModule *modules,
-                                uptr max_modules) override {
-    return ::GetListOfModules(modules, max_modules, /* filter */ nullptr);
-  }
-
   const char *PlatformDemangle(const char *name) override {
     return DemangleCXXABI(name);
   }
 
   void PlatformPrepareForSandboxing() override {
 #if SANITIZER_LINUX && !SANITIZER_ANDROID
     // Cache /proc/self/exe on Linux.
     CacheBinaryName();

lib/sanitizer_common/sanitizer_symbolizer_win.cc

 
 // TODO(kuba.brecka): To be merged with POSIXSymbolizer.
 class WinSymbolizer : public Symbolizer {
  public:
   explicit WinSymbolizer(IntrusiveList<SymbolizerTool> tools)
       : Symbolizer(tools) {}
 
  private:
-  uptr PlatformGetListOfModules(LoadedModule *modules,
-                                uptr max_modules) override {
-    return ::GetListOfModules(modules, max_modules, /* filter */ nullptr);
-  }
   const char *PlatformDemangle(const char *name) override { return name; }
   void PlatformPrepareForSandboxing() override { }
 };
 
 Symbolizer *Symbolizer::PlatformInit() {
   IntrusiveList<SymbolizerTool> list;
   list.clear();
   list.push_back(new(symbolizer_allocator_) WinSymbolizerTool());
   return new(symbolizer_allocator_) WinSymbolizer(list);
 }
 
 }  // namespace __sanitizer
 
 #endif  // _WIN32