From 4d9662580cdcc559d9a557a9333e28bf311dd3ca Mon Sep 17 00:00:00 2001
From: Tudor Bosman <tudorb@fb.com>
Date: Mon, 17 Mar 2014 13:24:07 -0700
Subject: [PATCH] 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
---
 folly/experimental/symbolizer/ElfCache.cpp   | 11 +++---
 folly/experimental/symbolizer/ElfCache.h     |  7 +++-
 folly/experimental/symbolizer/Symbolizer.cpp | 36 +++++++++++++-------
 folly/experimental/symbolizer/Symbolizer.h   | 11 ++++--
 4 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/folly/experimental/symbolizer/ElfCache.cpp b/folly/experimental/symbolizer/ElfCache.cpp
index 79652a49..fd43474a 100644
--- 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 f6581940..a2d8993a 100644
--- 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 80601aef..ac30db8c 100644
--- 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 5a87f90b..b4e96902 100644
--- 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>
-- 
2.34.1