From 3ba178b5e9e4de4d4540d6010d139fe4c86d6c19 Mon Sep 17 00:00:00 2001
From: Philip Pronin <philipp@fb.com>
Date: Sun, 26 Feb 2017 15:30:35 -0800
Subject: [PATCH] reduce consumed stack size in symbolizer

Summary:
It currently takes ~32kB of stack trace to run symbolizer, which is
very close to ASan alt stack size (32 kB).

If we exclude `demangle` (which can use unbound stack size in extreme cases),
the heaviest path in symbolizer includes `FrameArray<100>` and three arrays of
`PATH_MAX` (4 kB) size.  This diff removes the former and one of the latters,
reducing this code path from 32 kB to ~10 kB.

Reviewed By: ot, yfeldblum

Differential Revision: D4618467

fbshipit-source-id: e6a53b61b3d5f6e8b892216d2e9b839ed8430d0e
---
 folly/experimental/symbolizer/ElfCache.cpp   |  8 ++++----
 folly/experimental/symbolizer/ElfCache.h     | 13 +++++++++++-
 folly/experimental/symbolizer/Symbolizer.cpp | 21 ++++++++++----------
 folly/experimental/symbolizer/Symbolizer.h   |  4 ++++
 4 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/folly/experimental/symbolizer/ElfCache.cpp b/folly/experimental/symbolizer/ElfCache.cpp
index 8e703016..5bdd0756 100644
--- a/folly/experimental/symbolizer/ElfCache.cpp
+++ b/folly/experimental/symbolizer/ElfCache.cpp
@@ -64,8 +64,8 @@ std::shared_ptr<ElfFile> SignalSafeElfCache::getFile(StringPiece p) {
     return nullptr;
   }
 
-  Path path(p);
-  auto pos = map_.find(path);
+  scratchpad_.assign(p);
+  auto pos = map_.find(scratchpad_);
   if (pos != map_.end()) {
     return slots_[pos->second];
   }
@@ -79,12 +79,12 @@ std::shared_ptr<ElfFile> SignalSafeElfCache::getFile(StringPiece p) {
   auto& f = slots_[n];
 
   const char* msg = "";
-  int r = f->openAndFollow(path.data(), true, &msg);
+  int r = f->openAndFollow(scratchpad_.data(), true, &msg);
   if (r != ElfFile::kSuccess) {
     return nullptr;
   }
 
-  map_[path] = n;
+  map_[scratchpad_] = n;
   return f;
 }
 
diff --git a/folly/experimental/symbolizer/ElfCache.h b/folly/experimental/symbolizer/ElfCache.h
index 709babe9..135b6e9a 100644
--- a/folly/experimental/symbolizer/ElfCache.h
+++ b/folly/experimental/symbolizer/ElfCache.h
@@ -70,9 +70,19 @@ class SignalSafeElfCache : public ElfCacheBase {
   // own wrapper around a fixed-size, null-terminated string.
   class Path : private boost::totally_ordered<Path> {
    public:
+    Path() {
+      assign(folly::StringPiece());
+    }
+
     explicit Path(StringPiece s) {
+      assign(s);
+    }
+
+    void assign(StringPiece s) {
       DCHECK_LE(s.size(), kMaxSize);
-      memcpy(data_, s.data(), s.size());
+      if (!s.empty()) {
+        memcpy(data_, s.data(), s.size());
+      }
       data_[s.size()] = '\0';
     }
 
@@ -94,6 +104,7 @@ class SignalSafeElfCache : public ElfCacheBase {
     char data_[kMaxSize + 1];
   };
 
+  Path scratchpad_; // Preallocated key for map_ lookups.
   boost::container::flat_map<Path, int> map_;
   std::vector<std::shared_ptr<ElfFile>> slots_;
 };
diff --git a/folly/experimental/symbolizer/Symbolizer.cpp b/folly/experimental/symbolizer/Symbolizer.cpp
index 8adb134f..f199594a 100644
--- a/folly/experimental/symbolizer/Symbolizer.cpp
+++ b/folly/experimental/symbolizer/Symbolizer.cpp
@@ -30,6 +30,7 @@
 
 #include <folly/Conv.h>
 #include <folly/FileUtil.h>
+#include <folly/Memory.h>
 #include <folly/ScopeGuard.h>
 #include <folly/String.h>
 
@@ -383,8 +384,8 @@ StackTracePrinter::StackTracePrinter(size_t minSignalSafeElfCacheSize, int fd)
       printer_(
           fd,
           SymbolizePrinter::COLOR_IF_TTY,
-          size_t(64) << 10) // 64KiB
-{}
+          size_t(64) << 10), // 64KiB
+      addresses_(make_unique<FrameArray<kMaxStackTraceDepth>>()) {}
 
 void StackTracePrinter::flush() {
   printer_.flush();
@@ -395,33 +396,31 @@ void StackTracePrinter::printStackTrace(bool symbolize) {
   SCOPE_EXIT {
     flush();
   };
-  // Get and symbolize stack trace
-  constexpr size_t kMaxStackTraceDepth = 100;
-  FrameArray<kMaxStackTraceDepth> addresses;
 
   // Skip the getStackTrace frame
-  if (!getStackTraceSafe(addresses)) {
+  if (!getStackTraceSafe(*addresses_)) {
     print("(error retrieving stack trace)\n");
   } else if (symbolize) {
     // Do our best to populate location info, process is going to terminate,
     // so performance isn't critical.
     Symbolizer symbolizer(&elfCache_, Dwarf::LocationInfoMode::FULL);
-    symbolizer.symbolize(addresses);
+    symbolizer.symbolize(*addresses_);
 
     // Skip the top 2 frames:
     // getStackTraceSafe
     // StackTracePrinter::printStackTrace (here)
     //
     // Leaving signalHandler on the stack for clarity, I think.
-    printer_.println(addresses, 2);
+    printer_.println(*addresses_, 2);
   } else {
     print("(safe mode, symbolizer not available)\n");
     AddressFormatter formatter;
-    for (size_t i = 0; i < addresses.frameCount; ++i) {
-      print(formatter.format(addresses.addresses[i]));
+    for (size_t i = 0; i < addresses_->frameCount; ++i) {
+      print(formatter.format(addresses_->addresses[i]));
       print("\n");
     }
   }
 }
-} // namespace symbolizer
+
+}  // namespace symbolizer
 }  // namespace folly
diff --git a/folly/experimental/symbolizer/Symbolizer.h b/folly/experimental/symbolizer/Symbolizer.h
index 2cb8a9b7..677e0a8d 100644
--- a/folly/experimental/symbolizer/Symbolizer.h
+++ b/folly/experimental/symbolizer/Symbolizer.h
@@ -324,6 +324,7 @@ class StringSymbolizePrinter : public SymbolizePrinter {
 class StackTracePrinter {
  public:
   static constexpr size_t kDefaultMinSignalSafeElfCacheSize = 500;
+
   explicit StackTracePrinter(
       size_t minSignalSafeElfCacheSize = kDefaultMinSignalSafeElfCacheSize,
       int fd = STDERR_FILENO);
@@ -343,9 +344,12 @@ class StackTracePrinter {
   void flush();
 
  private:
+  static constexpr size_t kMaxStackTraceDepth = 100;
+
   int fd_;
   SignalSafeElfCache elfCache_;
   FDSymbolizePrinter printer_;
+  std::unique_ptr<FrameArray<kMaxStackTraceDepth>> addresses_;
 };
 
 }  // namespace symbolizer
-- 
2.34.1