Unbreak Symbolizer with small ElfCache

Test Plan: fbconfig -r folly/experimental/symbolizer && fbmake runtests_opt && fbmake runtests

Reviewed By: simpkins@fb.com

FB internal diff: D1224552

@override-unit-failures

diff --git a/folly/experimental/symbolizer/ElfCache.cpp b/folly/experimental/symbolizer/ElfCache.cpp
index 79652a49f015b00773db96e68075bde56d315619..fd43474ab4a284bfecb3034df7be1cf8ac92db6a 100644 (file)
--- a/folly/experimental/symbolizer/ElfCache.cpp
+++ b/folly/experimental/symbolizer/ElfCache.cpp
@@ -57,11 +57,9 @@ std::shared_ptr<ElfFile> SignalSafeElfCache::getFile(StringPiece p) {
 ElfCache::ElfCache(size_t capacity) : capacity_(capacity) { }
 
 std::shared_ptr<ElfFile> ElfCache::getFile(StringPiece p) {
-  auto path = p.str();
-
   std::lock_guard<std::mutex> lock(mutex_);
 
-  auto pos = files_.find(path);
+  auto pos = files_.find(p);
   if (pos != files_.end()) {
     // Found, move to back (MRU)
     auto& entry = pos->second;
@@ -71,6 +69,8 @@ std::shared_ptr<ElfFile> ElfCache::getFile(StringPiece p) {
   }
 
   auto entry = std::make_shared<Entry>();
+  entry->path = p.str();
+  auto& path = entry->path;
 
   // No negative caching
   if (entry->file.openNoThrow(path.c_str()) == -1) {
@@ -78,11 +78,12 @@ std::shared_ptr<ElfFile> ElfCache::getFile(StringPiece p) {
   }
 
   if (files_.size() == capacity_) {
-    // Evict LRU
+    auto& e = lruList_.front();
     lruList_.pop_front();
+    files_.erase(e.path);
   }
 
-  files_.emplace(std::move(path), entry);
+  files_.emplace(entry->path, entry);
   lruList_.push_back(*entry);
 
   return filePtr(entry);

diff --git a/folly/experimental/symbolizer/ElfCache.h b/folly/experimental/symbolizer/ElfCache.h
index f6581940e6e515474051c3faffc6f6b96cc71b72..a2d8993a44767ff31f1851eebaa60aaaf22c5bd8 100644 (file)
--- a/folly/experimental/symbolizer/ElfCache.h
+++ b/folly/experimental/symbolizer/ElfCache.h
@@ -107,7 +107,9 @@ class ElfCache : public ElfCacheBase {
   std::mutex mutex_;
 
   typedef boost::intrusive::list_member_hook<> LruLink;
+
   struct Entry {
+    std::string path;
     ElfFile file;
     LruLink lruLink;
   };
@@ -115,7 +117,10 @@ class ElfCache : public ElfCacheBase {
   static std::shared_ptr<ElfFile> filePtr(const std::shared_ptr<Entry>& e);
 
   size_t capacity_;
-  std::unordered_map<std::string, std::shared_ptr<Entry>> files_;
+  std::unordered_map<
+    StringPiece,
+    std::shared_ptr<Entry>,
+    StringPieceHash> files_;
 
   typedef boost::intrusive::list<
       Entry,

diff --git a/folly/experimental/symbolizer/Symbolizer.cpp b/folly/experimental/symbolizer/Symbolizer.cpp
index 80601aef18c1ef7cf2dcd303d6f15bce6a268e12..ac30db8c7013ad382199804b78212eee6eee5629 100644 (file)
--- a/folly/experimental/symbolizer/Symbolizer.cpp
+++ b/folly/experimental/symbolizer/Symbolizer.cpp
@@ -160,6 +160,27 @@ ElfCache* defaultElfCache() {
 
 }  // namespace
 
+void SymbolizedFrame::set(const std::shared_ptr<ElfFile>& file,
+                          uintptr_t address) {
+  clear();
+  found = true;
+
+  address += file->getBaseAddress();
+  auto sym = file->getDefinitionByAddress(address);
+  if (!sym.first) {
+    return;
+  }
+
+  file_ = file;
+  auto name = file->getSymbolName(sym);
+  if (name) {
+    this->name = name;
+  }
+
+  Dwarf(file.get()).findAddress(address, location);
+}
+
+
 Symbolizer::Symbolizer(ElfCacheBase* cache)
   : cache_(cache ?: defaultElfCache()) {
 }
@@ -172,8 +193,7 @@ void Symbolizer::symbolize(const uintptr_t* addresses,
     auto& frame = frames[i];
     if (!frame.found) {
       ++remaining;
-      frame.name.clear();
-      frame.location = Dwarf::LocationInfo();
+      frame.clear();
     }
   }
 
@@ -234,17 +254,7 @@ void Symbolizer::symbolize(const uintptr_t* addresses,
       }
 
       // Undo relocation
-      uintptr_t fileAddress = address - from + elfFile->getBaseAddress();
-      auto sym = elfFile->getDefinitionByAddress(fileAddress);
-      if (!sym.first) {
-        continue;
-      }
-      auto name = elfFile->getSymbolName(sym);
-      if (name) {
-        frame.name = name;
-      }
-
-      Dwarf(elfFile.get()).findAddress(fileAddress, frame.location);
+      frame.set(elfFile, address - from);
     }
   }
 

diff --git a/folly/experimental/symbolizer/Symbolizer.h b/folly/experimental/symbolizer/Symbolizer.h
index 5a87f90b6b689c9364e3e82ff0c0467468760f87..b4e9690203e713c45442835825f512366ab4689a 100644 (file)
--- a/folly/experimental/symbolizer/Symbolizer.h
+++ b/folly/experimental/symbolizer/Symbolizer.h
@@ -32,14 +32,17 @@
 namespace folly {
 namespace symbolizer {
 
+class Symbolizer;
+
 /**
  * Frame information: symbol name and location.
- *
- * Note that both name and location are references in the Symbolizer object,
- * which must outlive this SymbolizedFrame object.
  */
 struct SymbolizedFrame {
   SymbolizedFrame() : found(false) { }
+
+  void set(const std::shared_ptr<ElfFile>& file, uintptr_t address);
+  void clear() { *this = SymbolizedFrame(); }
+
   bool isSignalFrame;
   bool found;
   StringPiece name;
@@ -51,6 +54,8 @@ struct SymbolizedFrame {
   fbstring demangledName() const {
     return demangle(name.fbstr().c_str());
   }
+ private:
+  std::shared_ptr<ElfFile> file_;
 };
 
 template <size_t N>