[compiler-rt] r233687 - [Sanitizer RT] Put the Symbolizer module name string ownership in order

[Timur Iskhodzhanov]

Author: timurrrr
Date: Tue Mar 31 07:50:05 2015
New Revision: 233687

URL: http://llvm.org/viewvc/llvm-project?rev=233687&view=rev
Log:
[Sanitizer RT] Put the Symbolizer module name string ownership in order

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


Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc?rev=233687&r1=233686&r2=233687&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep.cc Tue Mar 31 07:50:05 2015
@@ -338,9 +338,8 @@ void CoverageData::UpdateModuleNameVec(u
   const char *module_name = sym->GetModuleNameForPc(caller_pc);
   if (!module_name) return;
   if (module_name_vec.empty() ||
-      internal_strcmp(module_name_vec.back().copied_module_name, module_name))
-    module_name_vec.push_back(
-        {internal_strdup(module_name), range_beg, range_end});
+      module_name_vec.back().copied_module_name != module_name)
+    module_name_vec.push_back({module_name, range_beg, range_end});
   else
     module_name_vec.back().end = range_end;
 }
@@ -751,9 +750,8 @@ void CoverageData::DumpOffsets() {
       uptr pc = UnbundlePc(pc_array[i]);
       uptr counter = UnbundleCounter(pc_array[i]);
       if (!pc) continue; // Not visited.
-      const char *unused;
       uptr offset = 0;
-      sym->GetModuleNameAndOffsetForPC(pc, &unused, &offset);
+      sym->GetModuleNameAndOffsetForPC(pc, nullptr, &offset);
       offsets.push_back(BundlePcAndCounter(offset, counter));
     }
 

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc?rev=233687&r1=233686&r2=233687&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc Tue Mar 31 07:50:05 2015
@@ -75,8 +75,28 @@ void Symbolizer::AddHooks(Symbolizer::St
   end_hook_ = end_hook;
 }
 
+const char *Symbolizer::ModuleNameOwner::GetOwnedCopy(const char *str) {
+  mu_->CheckLocked();
+
+  // 'str' will be the same string multiple times in a row, optimize this case.
+  if (last_match_ && !internal_strcmp(last_match_, str))
+    return last_match_;
+
+  // FIXME: this is linear search.
+  // We should optimize this further if this turns out to be a bottleneck later.
+  for (uptr i = 0; i < storage_.size(); ++i) {
+    if (!internal_strcmp(storage_[i], str)) {
+      last_match_ = storage_[i];
+      return last_match_;
+    }
+  }
+  last_match_ = internal_strdup(str);
+  storage_.push_back(last_match_);
+  return last_match_;
+}
+
 Symbolizer::Symbolizer(IntrusiveList<SymbolizerTool> tools)
-    : tools_(tools), start_hook_(0), end_hook_(0) {}
+    : module_names_(&mu_), tools_(tools), start_hook_(0), end_hook_(0) {}
 
 Symbolizer::SymbolizerScope::SymbolizerScope(const Symbolizer *sym)
     : sym_(sym) {
@@ -130,10 +150,16 @@ bool Symbolizer::SymbolizeData(uptr addr
 }
 
 bool Symbolizer::GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
-                                 uptr *module_address) {
+                                             uptr *module_address) {
   BlockingMutexLock l(&mu_);
-  return PlatformFindModuleNameAndOffsetForAddress(pc, module_name,
-                                                   module_address);
+  const char *internal_module_name = nullptr;
+  if (!PlatformFindModuleNameAndOffsetForAddress(pc, &internal_module_name,
+                                                 module_address))
+    return false;
+
+  if (module_name)
+    *module_name = module_names_.GetOwnedCopy(internal_module_name);
+  return true;
 }
 
 void Symbolizer::Flush() {

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h?rev=233687&r1=233686&r2=233687&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h Tue Mar 31 07:50:05 2015
@@ -84,15 +84,19 @@ class Symbolizer {
   // all inlined functions, if necessary).
   SymbolizedStack *SymbolizePC(uptr address);
   bool SymbolizeData(uptr address, DataInfo *info);
+
+  // The module names Symbolizer returns are stable and unique for every given
+  // module.  It is safe to store and compare them as pointers.
   bool GetModuleNameAndOffsetForPC(uptr pc, const char **module_name,
                                    uptr *module_address);
   const char *GetModuleNameForPc(uptr pc) {
-    const char *module_name = 0;
+    const char *module_name = nullptr;
     uptr unused;
     if (GetModuleNameAndOffsetForPC(pc, &module_name, &unused))
       return module_name;
     return nullptr;
   }
+
   // Release internal caches (if any).
   void Flush();
   // Attempts to demangle the provided C++ mangled name.
@@ -110,6 +114,26 @@ class Symbolizer {
                 EndSymbolizationHook end_hook);
 
  private:
+  // GetModuleNameAndOffsetForPC has to return a string to the caller.
+  // Since the corresponding module might get unloaded later, we should create
+  // our owned copies of the strings that we can safely return.
+  // ModuleNameOwner does not provide any synchronization, thus calls to
+  // its method should be protected by |mu_|.
+  class ModuleNameOwner {
+   public:
+    explicit ModuleNameOwner(BlockingMutex *synchronized_by)
+        : storage_(kInitialCapacity), last_match_(nullptr),
+          mu_(synchronized_by) {}
+    const char *GetOwnedCopy(const char *str);
+
+   private:
+    static const uptr kInitialCapacity = 1000;
+    InternalMmapVector<const char*> storage_;
+    const char *last_match_;
+
+    BlockingMutex *mu_;
+  } module_names_;
+
   /// Platform-specific function for creating a Symbolizer object.
   static Symbolizer *PlatformInit();