Change symbolizer cache to support frames with inline functions.

Summary:
The general idea is to change cache to a mapping between an address and array of frames instead of just one frame.

Also changed Dwarf to directly fill frames array with inline function calls, instead of allocating extra LocationInfo array.

Moved shared data structure like LocationInfo and SymbolizedFrame into a separate file so that it can used in both Dwarf.cpp and Symbolizer.cpp.

Reviewed By: luciang

Differential Revision: D18825831

fbshipit-source-id: fe963a1adbbc6f0055c466b91708eee732abc141

folly/experimental/exception_tracer/ExceptionTracer.cpp

@@ -78,7 +78,7 @@ void printExceptionInfo(
 
       Symbolizer symbolizer(
           (options & SymbolizePrinter::NO_FILE_AND_LINE)
-              ? Dwarf::LocationInfoMode::DISABLED
+              ? LocationInfoMode::DISABLED
               : Symbolizer::kDefaultLocationInfoMode);
       symbolizer.symbolize(addresses, frames.data(), frameCount);
 

folly/experimental/symbolizer/Dwarf.cpp

@@ -335,55 +335,6 @@ void readCompilationUnitAbbrs(
   }
 }
 
-// Simplify a path -- as much as we can while not moving data around...
-void simplifyPath(folly::StringPiece& sp) {
-  // Strip leading slashes and useless patterns (./), leaving one initial
-  // slash.
-  for (;;) {
-    if (sp.empty()) {
-      return;
-    }
-
-    // Strip leading slashes, leaving one.
-    while (sp.startsWith("//")) {
-      sp.advance(1);
-    }
-
-    if (sp.startsWith("/./")) {
-      // Note 2, not 3, to keep it absolute
-      sp.advance(2);
-      continue;
-    }
-
-    if (sp.removePrefix("./")) {
-      // Also remove any subsequent slashes to avoid making this path absolute.
-      while (sp.startsWith('/')) {
-        sp.advance(1);
-      }
-      continue;
-    }
-
-    break;
-  }
-
-  // Strip trailing slashes and useless patterns (/.).
-  for (;;) {
-    if (sp.empty()) {
-      return;
-    }
-
-    // Strip trailing slashes, except when this is the root path.
-    while (sp.size() > 1 && sp.removeSuffix('/')) {
-    }
-
-    if (sp.removeSuffix("/.")) {
-      continue;
-    }
-
-    break;
-  }
-}
-
 } // namespace
 
 Dwarf::Dwarf(const ElfFile* elf)
@@ -403,122 +354,6 @@ Dwarf::Dwarf(const ElfFile* elf)
 
 Dwarf::Section::Section(folly::StringPiece d) : is64Bit_(false), data_(d) {}
 
-Dwarf::Path::Path(
-    folly::StringPiece baseDir,
-    folly::StringPiece subDir,
-    folly::StringPiece file)
-    : baseDir_(baseDir), subDir_(subDir), file_(file) {
-  using std::swap;
-
-  // Normalize
-  if (file_.empty()) {
-    baseDir_.clear();
-    subDir_.clear();
-    return;
-  }
-
-  if (file_[0] == '/') {
-    // file_ is absolute
-    baseDir_.clear();
-    subDir_.clear();
-  }
-
-  if (!subDir_.empty() && subDir_[0] == '/') {
-    baseDir_.clear(); // subDir_ is absolute
-  }
-
-  simplifyPath(baseDir_);
-  simplifyPath(subDir_);
-  simplifyPath(file_);
-
-  // Make sure it's never the case that baseDir_ is empty, but subDir_ isn't.
-  if (baseDir_.empty()) {
-    swap(baseDir_, subDir_);
-  }
-}
-
-size_t Dwarf::Path::size() const {
-  size_t size = 0;
-  bool needsSlash = false;
-
-  if (!baseDir_.empty()) {
-    size += baseDir_.size();
-    needsSlash = !baseDir_.endsWith('/');
-  }
-
-  if (!subDir_.empty()) {
-    size += needsSlash;
-    size += subDir_.size();
-    needsSlash = !subDir_.endsWith('/');
-  }
-
-  if (!file_.empty()) {
-    size += needsSlash;
-    size += file_.size();
-  }
-
-  return size;
-}
-
-size_t Dwarf::Path::toBuffer(char* buf, size_t bufSize) const {
-  size_t totalSize = 0;
-  bool needsSlash = false;
-
-  auto append = [&](folly::StringPiece sp) {
-    if (bufSize >= 2) {
-      size_t toCopy = std::min(sp.size(), bufSize - 1);
-      memcpy(buf, sp.data(), toCopy);
-      buf += toCopy;
-      bufSize -= toCopy;
-    }
-    totalSize += sp.size();
-  };
-
-  if (!baseDir_.empty()) {
-    append(baseDir_);
-    needsSlash = !baseDir_.endsWith('/');
-  }
-  if (!subDir_.empty()) {
-    if (needsSlash) {
-      append("/");
-    }
-    append(subDir_);
-    needsSlash = !subDir_.endsWith('/');
-  }
-  if (!file_.empty()) {
-    if (needsSlash) {
-      append("/");
-    }
-    append(file_);
-  }
-  if (bufSize) {
-    *buf = '\0';
-  }
-  assert(totalSize == size());
-  return totalSize;
-}
-
-void Dwarf::Path::toString(std::string& dest) const {
-  size_t initialSize = dest.size();
-  dest.reserve(initialSize + size());
-  if (!baseDir_.empty()) {
-    dest.append(baseDir_.begin(), baseDir_.end());
-  }
-  if (!subDir_.empty()) {
-    if (!dest.empty() && dest.back() != '/') {
-      dest.push_back('/');
-    }
-    dest.append(subDir_.begin(), subDir_.end());
-  }
-  if (!file_.empty()) {
-    if (!dest.empty() && dest.back() != '/') {
-      dest.push_back('/');
-    }
-    dest.append(file_.begin(), file_.end());
-  }
-  assert(dest.size() == initialSize + size());
-}
-
 // Next chunk in section
 bool Dwarf::Section::next(folly::StringPiece& chunk) {
   chunk = data_;
@@ -619,7 +454,7 @@ bool Dwarf::findLocation(
     const LocationInfoMode mode,
     detail::CompilationUnit& cu,
     LocationInfo& locationInfo,
-    folly::Range<Dwarf::LocationInfo*> inlineLocationInfo) const {
+    folly::Range<SymbolizedFrame*> inlineFrames) const {
   detail::Die die = getDieAtOffset(cu, cu.firstDie);
   FOLLY_SAFE_CHECK(
       die.abbr.tag == DW_TAG_compile_unit, "expecting compile unit entry");
@@ -670,8 +505,8 @@ bool Dwarf::findLocation(
       lineVM.findAddress(address, locationInfo.file, locationInfo.line);
 
   // Look up whether inline function.
-  if (mode == Dwarf::LocationInfoMode::FULL_WITH_INLINE &&
-      !inlineLocationInfo.empty() && locationInfo.hasFileAndLine) {
+  if (mode == LocationInfoMode::FULL_WITH_INLINE && !inlineFrames.empty() &&
+      locationInfo.hasFileAndLine) {
     // Re-get the compilation unit with abbreviation cached.
     std::array<detail::DIEAbbreviation, kMaxAbbreviationEntries> abbrs;
     cu.abbrCache = folly::range(abbrs);
@@ -681,24 +516,32 @@ bool Dwarf::findLocation(
     // Subprogram is the DIE of caller function.
     if (subprogram.abbr.hasChildren) {
       size_t size = std::min<size_t>(
-          Dwarf::kMaxInlineLocationInfoPerFrame, inlineLocationInfo.size());
-      detail::CodeLocation isrLocs[size];
-      auto isrLocsRange = folly::Range<detail::CodeLocation*>(isrLocs, size);
-      findInlinedSubroutineDieForAddress(cu, subprogram, address, isrLocsRange);
-      uint32_t numFound = isrLocsRange.data() - isrLocs;
-      auto* prevLocation = &locationInfo;
-      auto* nextLocation = inlineLocationInfo.data();
-      auto* isrLoc = isrLocs;
+          Dwarf::kMaxInlineLocationInfoPerFrame, inlineFrames.size());
+      detail::CodeLocation inlineLocs[size];
+      auto inlineLocsRange =
+          folly::Range<detail::CodeLocation*>(inlineLocs, size);
+      findInlinedSubroutineDieForAddress(
+          cu, subprogram, address, inlineLocsRange);
+      size_t numFound = inlineLocsRange.data() - inlineLocs;
+
       // The line in locationInfo is the line in the deepest inline functions,
       // and the lines in inlineLocationInfo are the line of callers in call
       // sequence. A shuffle is needed to match lines with their function name.
       uint64_t callerLine = locationInfo.line;
-      while (numFound > 0) {
-        prevLocation->line = isrLoc->line;
+      inlineLocsRange =
+          folly::Range<detail::CodeLocation*>(inlineLocs, numFound);
+      // Put inline frames from inside to outside (callee is before caller).
+      std::reverse(inlineLocsRange.begin(), inlineLocsRange.end());
+      auto inlineFrameIter = inlineFrames.begin();
+      for (auto inlineLocIter = inlineLocsRange.begin();
+           inlineLocIter != inlineLocsRange.end();
+           inlineLocIter++, inlineFrameIter++) {
+        inlineFrameIter->location.line = callerLine;
+        callerLine = inlineLocIter->line;
         folly::Optional<folly::StringPiece> name;
         folly::Optional<uint64_t> file;
-
-        forEachAttribute(cu, isrLoc->die, [&](const detail::Attribute& attr) {
+        forEachAttribute(
+            cu, inlineLocIter->die, [&](const detail::Attribute& attr) {
               switch (attr.spec.name) {
                 case DW_AT_name:
                   name = boost::get<StringPiece>(attr.attrValue);
@@ -713,15 +556,15 @@ bool Dwarf::findLocation(
         if (!name.has_value() || !file.has_value()) {
           break;
         }
-        nextLocation->hasFileAndLine = true;
-        nextLocation->name = name.value();
-        nextLocation->file = lineVM.getFullFileName(file.value());
-        prevLocation = nextLocation;
-        nextLocation++;
-        isrLoc++;
-        numFound--;
+
+        inlineFrameIter->found = true;
+        inlineFrameIter->addr = address;
+        inlineFrameIter->name = name.value().data();
+        inlineFrameIter->location.hasFileAndLine = true;
+        inlineFrameIter->location.name = name.value();
+        inlineFrameIter->location.file = lineVM.getFullFileName(file.value());
       }
-      prevLocation->line = callerLine;
+      locationInfo.line = callerLine;
     }
   }
 
@@ -732,7 +575,7 @@ bool Dwarf::findAddress(
     uintptr_t address,
     LocationInfoMode mode,
     LocationInfo& locationInfo,
-    folly::Range<Dwarf::LocationInfo*> inlineLocationInfo) const {
+    folly::Range<SymbolizedFrame*> inlineFrames) const {
   if (mode == LocationInfoMode::DISABLED) {
     return false;
   }
@@ -748,7 +591,7 @@ bool Dwarf::findAddress(
     if (findDebugInfoOffset(address, debugAranges_, offset)) {
       // Read compilation unit header from .debug_info
       auto unit = getCompilationUnit(debugInfo_, offset);
-      findLocation(address, mode, unit, locationInfo, inlineLocationInfo);
+      findLocation(address, mode, unit, locationInfo, inlineFrames);
       return locationInfo.hasFileAndLine;
     } else if (mode == LocationInfoMode::FAST) {
       // NOTE: Clang (when using -gdwarf-aranges) doesn't generate entries
@@ -771,7 +614,7 @@ bool Dwarf::findAddress(
   while (offset < debugInfo_.size() && !locationInfo.hasFileAndLine) {
     auto unit = getCompilationUnit(debugInfo_, offset);
     offset += unit.size;
-    findLocation(address, mode, unit, locationInfo, inlineLocationInfo);
+    findLocation(address, mode, unit, locationInfo, inlineFrames);
   }
   return locationInfo.hasFileAndLine;
 }

folly/experimental/symbolizer/Dwarf.h

@@ -23,6 +23,7 @@
 #include <folly/Function.h>
 #include <folly/Range.h>
 #include <folly/experimental/symbolizer/Elf.h>
+#include <folly/experimental/symbolizer/SymbolizedFrame.h>
 
 namespace folly {
 namespace symbolizer {
@@ -80,39 +81,18 @@ class Dwarf {
   /** Create a DWARF parser around an ELF file. */
   explicit Dwarf(const ElfFile* elf);
 
-  /**
-   * Represent a file path a s collection of three parts (base directory,
-   * subdirectory, and file).
-   */
-  class Path;
-
-  enum class LocationInfoMode {
-    // Don't resolve location info.
-    DISABLED,
-    // Perform CU lookup using .debug_aranges (might be incomplete).
-    FAST,
-    // Scan all CU in .debug_info (slow!) on .debug_aranges lookup failure.
-    FULL,
-    // Scan .debug_info (super slower, use with caution) for inline functions in
-    // addition to FULL.
-    FULL_WITH_INLINE,
-  };
-
   /**
    * More than one location info may exist if current frame is an inline
    * function call.
    */
   static const uint32_t kMaxInlineLocationInfoPerFrame = 3;
 
-  /** Contains location info like file name, line number, etc. */
-  struct LocationInfo;
-
   /** Find the file and line number information corresponding to address. */
   bool findAddress(
       uintptr_t address,
       LocationInfoMode mode,
       LocationInfo& info,
-      folly::Range<Dwarf::LocationInfo*> inlineInfo = {}) const;
+      folly::Range<SymbolizedFrame*> inlineFrames = {}) const;
 
  private:
   using AttributeValue = boost::variant<uint64_t, folly::StringPiece>;
@@ -137,7 +117,7 @@ class Dwarf {
       const LocationInfoMode mode,
       detail::CompilationUnit& unit,
       LocationInfo& info,
-      folly::Range<Dwarf::LocationInfo*> inlineInfo = {}) const;
+      folly::Range<SymbolizedFrame*> inlineFrames = {}) const;
 
   /**
    * Finds a subprogram debugging info entry that contains a given address among
@@ -227,61 +207,6 @@ class Dwarf::Section {
   folly::StringPiece data_;
 };
 
-class Dwarf::Path {
- public:
-  Path() {}
-
-  Path(
-      folly::StringPiece baseDir,
-      folly::StringPiece subDir,
-      folly::StringPiece file);
-
-  folly::StringPiece baseDir() const {
-    return baseDir_;
-  }
-  folly::StringPiece subDir() const {
-    return subDir_;
-  }
-  folly::StringPiece file() const {
-    return file_;
-  }
-
-  size_t size() const;
-
-  /**
-   * Copy the Path to a buffer of size bufSize.
-   *
-   * toBuffer behaves like snprintf: It will always null-terminate the
-   * buffer (so it will copy at most bufSize-1 bytes), and it will return
-   * the number of bytes that would have been written if there had been
-   * enough room, so, if toBuffer returns a value >= bufSize, the output
-   * was truncated.
-   */
-  size_t toBuffer(char* buf, size_t bufSize) const;
-
-  void toString(std::string& dest) const;
-  std::string toString() const {
-    std::string s;
-    toString(s);
-    return s;
-  }
-
- private:
-  folly::StringPiece baseDir_;
-  folly::StringPiece subDir_;
-  folly::StringPiece file_;
-};
-
-struct Dwarf::LocationInfo {
-  bool hasFileAndLine = false;
-  bool hasMainFile = false;
-  // Function name in call stack.
-  folly::StringPiece name;
-  Path mainFile;
-  Path file;
-  uint64_t line = 0;
-};
-
 class Dwarf::LineNumberVM {
  public:
   LineNumberVM(
@@ -370,9 +295,5 @@ class Dwarf::LineNumberVM {
   uint64_t discriminator_;
 };
 
-inline std::ostream& operator<<(std::ostream& out, const Dwarf::Path& path) {
-  return out << path.toString();
-}
-
 } // namespace symbolizer
 } // namespace folly

folly/experimental/symbolizer/SymbolizedFrame.cpp

+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/experimental/symbolizer/SymbolizedFrame.h>
+
+namespace folly {
+namespace symbolizer {
+
+namespace {
+
+// Simplify a path -- as much as we can while not moving data around...
+void simplifyPath(StringPiece& sp) {
+  // Strip leading slashes and useless patterns (./), leaving one initial
+  // slash.
+  for (;;) {
+    if (sp.empty()) {
+      return;
+    }
+
+    // Strip leading slashes, leaving one.
+    while (sp.startsWith("//")) {
+      sp.advance(1);
+    }
+
+    if (sp.startsWith("/./")) {
+      // Note 2, not 3, to keep it absolute
+      sp.advance(2);
+      continue;
+    }
+
+    if (sp.removePrefix("./")) {
+      // Also remove any subsequent slashes to avoid making this path absolute.
+      while (sp.startsWith('/')) {
+        sp.advance(1);
+      }
+      continue;
+    }
+
+    break;
+  }
+
+  // Strip trailing slashes and useless patterns (/.).
+  for (;;) {
+    if (sp.empty()) {
+      return;
+    }
+
+    // Strip trailing slashes, except when this is the root path.
+    while (sp.size() > 1 && sp.removeSuffix('/')) {
+    }
+
+    if (sp.removeSuffix("/.")) {
+      continue;
+    }
+
+    break;
+  }
+}
+} // namespace
+
+Path::Path(StringPiece baseDir, StringPiece subDir, StringPiece file)
+    : baseDir_(baseDir), subDir_(subDir), file_(file) {
+  using std::swap;
+
+  // Normalize
+  if (file_.empty()) {
+    baseDir_.clear();
+    subDir_.clear();
+    return;
+  }
+
+  if (file_[0] == '/') {
+    // file_ is absolute
+    baseDir_.clear();
+    subDir_.clear();
+  }
+
+  if (!subDir_.empty() && subDir_[0] == '/') {
+    baseDir_.clear(); // subDir_ is absolute
+  }
+
+  simplifyPath(baseDir_);
+  simplifyPath(subDir_);
+  simplifyPath(file_);
+
+  // Make sure it's never the case that baseDir_ is empty, but subDir_ isn't.
+  if (baseDir_.empty()) {
+    swap(baseDir_, subDir_);
+  }
+}
+
+size_t Path::size() const {
+  size_t size = 0;
+  bool needsSlash = false;
+
+  if (!baseDir_.empty()) {
+    size += baseDir_.size();
+    needsSlash = !baseDir_.endsWith('/');
+  }
+
+  if (!subDir_.empty()) {
+    size += needsSlash;
+    size += subDir_.size();
+    needsSlash = !subDir_.endsWith('/');
+  }
+
+  if (!file_.empty()) {
+    size += needsSlash;
+    size += file_.size();
+  }
+
+  return size;
+}
+
+size_t Path::toBuffer(char* buf, size_t bufSize) const {
+  size_t totalSize = 0;
+  bool needsSlash = false;
+
+  auto append = [&](StringPiece sp) {
+    if (bufSize >= 2) {
+      size_t toCopy = std::min(sp.size(), bufSize - 1);
+      memcpy(buf, sp.data(), toCopy);
+      buf += toCopy;
+      bufSize -= toCopy;
+    }
+    totalSize += sp.size();
+  };
+
+  if (!baseDir_.empty()) {
+    append(baseDir_);
+    needsSlash = !baseDir_.endsWith('/');
+  }
+  if (!subDir_.empty()) {
+    if (needsSlash) {
+      append("/");
+    }
+    append(subDir_);
+    needsSlash = !subDir_.endsWith('/');
+  }
+  if (!file_.empty()) {
+    if (needsSlash) {
+      append("/");
+    }
+    append(file_);
+  }
+  if (bufSize) {
+    *buf = '\0';
+  }
+  assert(totalSize == size());
+  return totalSize;
+}
+
+void Path::toString(std::string& dest) const {
+  size_t initialSize = dest.size();
+  dest.reserve(initialSize + size());
+  if (!baseDir_.empty()) {
+    dest.append(baseDir_.begin(), baseDir_.end());
+  }
+  if (!subDir_.empty()) {
+    if (!dest.empty() && dest.back() != '/') {
+      dest.push_back('/');
+    }
+    dest.append(subDir_.begin(), subDir_.end());
+  }
+  if (!file_.empty()) {
+    if (!dest.empty() && dest.back() != '/') {
+      dest.push_back('/');
+    }
+    dest.append(file_.begin(), file_.end());
+  }
+  assert(dest.size() == initialSize + size());
+}
+
+} // namespace symbolizer
+} // namespace folly

folly/experimental/symbolizer/SymbolizedFrame.h

+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <array>
+#include <cstdint>
+#include <memory>
+#include <string>
+
+#include <folly/Range.h>
+#include <folly/experimental/symbolizer/Elf.h>
+
+namespace folly {
+namespace symbolizer {
+
+/**
+ * Represent a file path a s collection of three parts (base directory,
+ * subdirectory, and file).
+ */
+class Path {
+ public:
+  Path() {}
+
+  Path(
+      folly::StringPiece baseDir,
+      folly::StringPiece subDir,
+      folly::StringPiece file);
+
+  folly::StringPiece baseDir() const {
+    return baseDir_;
+  }
+  folly::StringPiece subDir() const {
+    return subDir_;
+  }
+  folly::StringPiece file() const {
+    return file_;
+  }
+
+  size_t size() const;
+
+  /**
+   * Copy the Path to a buffer of size bufSize.
+   *
+   * toBuffer behaves like snprintf: It will always null-terminate the
+   * buffer (so it will copy at most bufSize-1 bytes), and it will return
+   * the number of bytes that would have been written if there had been
+   * enough room, so, if toBuffer returns a value >= bufSize, the output
+   * was truncated.
+   */
+  size_t toBuffer(char* buf, size_t bufSize) const;
+
+  void toString(std::string& dest) const;
+  std::string toString() const {
+    std::string s;
+    toString(s);
+    return s;
+  }
+
+ private:
+  folly::StringPiece baseDir_;
+  folly::StringPiece subDir_;
+  folly::StringPiece file_;
+};
+
+inline std::ostream& operator<<(std::ostream& out, const Path& path) {
+  return out << path.toString();
+}
+
+enum class LocationInfoMode {
+  // Don't resolve location info.
+  DISABLED,
+  // Perform CU lookup using .debug_aranges (might be incomplete).
+  FAST,
+  // Scan all CU in .debug_info (slow!) on .debug_aranges lookup failure.
+  FULL,
+  // Scan .debug_info (super slower, use with caution) for inline functions in
+  // addition to FULL.
+  FULL_WITH_INLINE,
+};
+
+/**
+ * Contains location info like file name, line number, etc.
+ */
+struct LocationInfo {
+  bool hasFileAndLine = false;
+  bool hasMainFile = false;
+  // Function name in call stack.
+  folly::StringPiece name;
+  Path mainFile;
+  Path file;
+  uint64_t line = 0;
+};
+
+/**
+ * Frame information: symbol name and location.
+ */
+struct SymbolizedFrame {
+  bool found = false;
+  uintptr_t addr = 0;
+  // Mangled symbol name. Use `folly::demangle()` to demangle it.
+  const char* name = nullptr;
+  LocationInfo location;
+  std::shared_ptr<ElfFile> file;
+
+  void clear() {
+    *this = SymbolizedFrame();
+  }
+};
+
+template <size_t N>
+struct FrameArray {
+  FrameArray() {}
+
+  size_t frameCount = 0;
+  uintptr_t addresses[N];
+  SymbolizedFrame frames[N];
+};
+
+} // namespace symbolizer
+} // namespace folly

folly/experimental/symbolizer/Symbolizer.cpp

@@ -60,30 +60,33 @@ ElfCache* defaultElfCache() {
   return cache;
 }
 
-} // namespace
-
-void SymbolizedFrame::set(
+void setSymbolizedFrame(
+    SymbolizedFrame& frame,
     const std::shared_ptr<ElfFile>& file,
     uintptr_t address,
-    Dwarf::LocationInfoMode mode,
-    folly::Range<Dwarf::LocationInfo*> inlineLocations) {
-  clear();
-  found = true;
+    LocationInfoMode mode,
+    folly::Range<SymbolizedFrame*> extraInlineFrames = {}) {
+  frame.clear();
+  frame.found = true;
 
   auto sym = file->getDefinitionByAddress(address);
   if (!sym.first) {
     return;
   }
 
-  file_ = file;
-  name = file->getSymbolName(sym);
+  frame.addr = address;
+  frame.file = file;
+  frame.name = file->getSymbolName(sym);
 
-  Dwarf(file.get()).findAddress(address, mode, location, inlineLocations);
+  Dwarf(file.get())
+      .findAddress(address, mode, frame.location, extraInlineFrames);
 }
 
+} // namespace
+
 Symbolizer::Symbolizer(
     ElfCacheBase* cache,
-    Dwarf::LocationInfoMode mode,
+    LocationInfoMode mode,
     size_t symbolCacheSize)
     : cache_(cache ? cache : defaultElfCache()), mode_(mode) {
   if (symbolCacheSize > 0) {
@@ -125,6 +128,15 @@ void Symbolizer::symbolize(
     frames[i].addr = addrs[i];
   }
 
+  // Find out how many frames were filled in.
+  auto countFrames = [](folly::Range<SymbolizedFrame*> framesRange) {
+    return std::distance(
+        framesRange.begin(),
+        std::find_if(framesRange.begin(), framesRange.end(), [&](auto frame) {
+          return !frame.found;
+        }));
+  };
+
   for (auto lmap = _r_debug.r_map; lmap != nullptr && remaining != 0;
        lmap = lmap->l_next) {
     // The empty string is used in place of the filename for the link_map
@@ -146,16 +158,31 @@ void Symbolizer::symbolize(
       }
 
       auto const addr = frame.addr;
-      // Mode FULL_WITH_INLINE (may have multiple frames for an address) is not
-      // supported in symbolCache_.
-      if (symbolCache_ && mode_ != Dwarf::LocationInfoMode::FULL_WITH_INLINE) {
+      if (symbolCache_) {
         // Need a write lock, because EvictingCacheMap brings found item to
         // front of eviction list.
         auto lockedSymbolCache = symbolCache_->wlock();
 
         auto const iter = lockedSymbolCache->find(addr);
         if (iter != lockedSymbolCache->end()) {
-          frame = iter->second;
+          size_t numCachedFrames = countFrames(folly::range(iter->second));
+          // 1 entry in cache is the non-inlined function call and that one
+          // already has space reserved at `frames[i]`
+          auto numInlineFrames = numCachedFrames - 1;
+          if (numInlineFrames <= frameCount - addrCount) {
+            // Move the rest of the frames to make space for inlined frames.
+            std::move_backward(
+                frames.begin() + i + 1,
+                frames.begin() + addrCount,
+                frames.begin() + addrCount + numInlineFrames);
+            // Overwrite frames[i] too (the non-inlined function call entry).
+            std::copy(
+                iter->second.begin(),
+                iter->second.begin() + numInlineFrames + 1,
+                frames.begin() + i);
+            i += numInlineFrames;
+            addrCount += numInlineFrames;
+          }
           continue;
         }
       }
@@ -163,55 +190,43 @@ void Symbolizer::symbolize(
       // Get the unrelocated, ELF-relative address by normalizing via the
       // address at which the object is loaded.
       auto const adjusted = addr - lmap->l_addr;
-
+      size_t numInlined = 0;
       if (elfFile->getSectionContainingAddress(adjusted)) {
-        if (mode_ == Dwarf::LocationInfoMode::FULL_WITH_INLINE &&
+        if (mode_ == LocationInfoMode::FULL_WITH_INLINE &&
             frameCount > addrCount) {
           size_t maxInline = std::min<size_t>(
               Dwarf::kMaxInlineLocationInfoPerFrame, frameCount - addrCount);
-          Dwarf::LocationInfo inlineLocations[maxInline];
-          folly::Range<Dwarf::LocationInfo*> inlineLocRange(
-              inlineLocations, maxInline);
-          frame.set(elfFile, adjusted, mode_, inlineLocRange);
-
-          // Find out how many LocationInfo were filled in.
-          size_t numInlined = std::distance(
-              inlineLocRange.begin(),
-              std::find_if(
-                  inlineLocRange.begin(), inlineLocRange.end(), [&](auto loc) {
-                    return !loc.hasFileAndLine;
-                  }));
+          // First use the trailing empty frames (index starting from addrCount)
+          // to get the inline call stack, then rotate these inline functions
+          // before the caller at `frame[i]`.
+          folly::Range<SymbolizedFrame*> inlineFrameRange(
+              frames.begin() + addrCount,
+              frames.begin() + addrCount + maxInline);
+          setSymbolizedFrame(frame, elfFile, adjusted, mode_, inlineFrameRange);
 
-          // Move the rest of the frames to make space for inlined location info
-          std::move_backward(
+          numInlined = countFrames(inlineFrameRange);
+          // Rotate inline frames right before its caller frame.
+          std::rotate(
               frames.begin() + i,
               frames.begin() + addrCount,
               frames.begin() + addrCount + numInlined);
-
-          // Insert inlined location info in the space we opened up above. The
-          // inlineLocations contains inline functions in call sequence, and
-          // here we store them reversely in frames (caller in later frame).
-          for (size_t k = 0; k < numInlined; k++) {
-            frames[i + k].found = true;
-            frames[i + k].addr = addr;
-            frames[i + k].location = inlineLocations[numInlined - k - 1];
-            frames[i + k].name =
-                inlineLocations[numInlined - k - 1].name.data();
-          }
-
-          // Skip over the newly added inlined items.
-          i += numInlined;
           addrCount += numInlined;
         } else {
-          frame.set(elfFile, adjusted, mode_);
+          setSymbolizedFrame(frame, elfFile, adjusted, mode_);
         }
         --remaining;
-        if (symbolCache_ &&
-            mode_ != Dwarf::LocationInfoMode::FULL_WITH_INLINE) {
+        if (symbolCache_) {
           // frame may already have been set here.  That's ok, we'll just
           // overwrite, which doesn't cause a correctness problem.
-          symbolCache_->wlock()->set(addr, frame);
+          CachedSymbolizedFrames cacheFrames;
+          std::copy(
+              frames.begin() + i,
+              frames.begin() + i + std::min(numInlined + 1, cacheFrames.size()),
+              cacheFrames.begin());
+          symbolCache_->wlock()->set(addr, cacheFrames);
         }
+        // Skip over the newly added inlined items.
+        i += numInlined;
       }
     }
   }
@@ -466,7 +481,7 @@ void SafeStackTracePrinter::printSymbolizedStackTrace() {
 
   // Do our best to populate location info, process is going to terminate,
   // so performance isn't critical.
-  Symbolizer symbolizer(&elfCache_, Dwarf::LocationInfoMode::FULL);
+  Symbolizer symbolizer(&elfCache_, LocationInfoMode::FULL);
   symbolizer.symbolize(*addresses_);
 
   // Skip the top 2 frames captured by printStackTrace:
@@ -508,7 +523,7 @@ FastStackTracePrinter::FastStackTracePrinter(
       printer_(std::move(printer)),
       symbolizer_(
           elfCache_ ? elfCache_.get() : defaultElfCache(),
-          Dwarf::LocationInfoMode::FULL,
+          LocationInfoMode::FULL,
           symbolCacheSize) {}
 
 FastStackTracePrinter::~FastStackTracePrinter() {}

folly/experimental/symbolizer/Symbolizer.h

@@ -31,6 +31,7 @@
 #include <folly/experimental/symbolizer/Elf.h>
 #include <folly/experimental/symbolizer/ElfCache.h>
 #include <folly/experimental/symbolizer/StackTrace.h>
+#include <folly/experimental/symbolizer/SymbolizedFrame.h>
 #include <folly/io/IOBuf.h>
 
 namespace folly {
@@ -38,47 +39,6 @@ namespace symbolizer {
 
 class Symbolizer;
 
-/**
- * Frame information: symbol name and location.
- */
-struct SymbolizedFrame {
-  SymbolizedFrame() {}
-
-  void set(
-      const std::shared_ptr<ElfFile>& file,
-      uintptr_t address,
-      Dwarf::LocationInfoMode mode,
-      folly::Range<Dwarf::LocationInfo*> inlineLocations = {});
-
-  void clear() {
-    *this = SymbolizedFrame();
-  }
-
-  bool found = false;
-  uintptr_t addr = 0;
-  const char* name = nullptr;
-  Dwarf::LocationInfo location;
-
-  /**
-   * Demangle the name and return it. Not async-signal-safe; allocates memory.
-   */
-  fbstring demangledName() const {
-    return name ? demangle(name) : fbstring();
-  }
-
- private:
-  std::shared_ptr<ElfFile> file_;
-};
-
-template <size_t N>
-struct FrameArray {
-  FrameArray() {}
-
-  size_t frameCount = 0;
-  uintptr_t addresses[N];
-  SymbolizedFrame frames[N];
-};
-
 /**
  * Get stack trace into a given FrameArray, return true on success (and
  * set frameCount to the actual frame count, which may be > N) and false
@@ -119,15 +79,14 @@ inline bool getStackTraceSafe(FrameArray<N>& fa) {
 
 class Symbolizer {
  public:
-  static constexpr Dwarf::LocationInfoMode kDefaultLocationInfoMode =
-      Dwarf::LocationInfoMode::FAST;
+  static constexpr auto kDefaultLocationInfoMode = LocationInfoMode::FAST;
 
-  explicit Symbolizer(Dwarf::LocationInfoMode mode = kDefaultLocationInfoMode)
+  explicit Symbolizer(LocationInfoMode mode = kDefaultLocationInfoMode)
       : Symbolizer(nullptr, mode) {}
 
   explicit Symbolizer(
       ElfCacheBase* cache,
-      Dwarf::LocationInfoMode mode = kDefaultLocationInfoMode,
+      LocationInfoMode mode = kDefaultLocationInfoMode,
       size_t symbolCacheSize = 0);
 
   /**
@@ -173,9 +132,14 @@ class Symbolizer {
 
  private:
   ElfCacheBase* const cache_;
-  const Dwarf::LocationInfoMode mode_;
-
-  using SymbolCache = EvictingCacheMap<uintptr_t, SymbolizedFrame>;
+  const LocationInfoMode mode_;
+
+  // SymbolCache contains mapping between an address and its frames. The first
+  // frame is the normal function call, and the following are stacked inline
+  // function calls if any.
+  using CachedSymbolizedFrames =
+      std::array<SymbolizedFrame, 1 + Dwarf::kMaxInlineLocationInfoPerFrame>;
+  using SymbolCache = EvictingCacheMap<uintptr_t, CachedSymbolizedFrames>;
   folly::Optional<Synchronized<SymbolCache>> symbolCache_;
 };
 

folly/experimental/symbolizer/test/DwarfBenchmark.cpp

@@ -17,6 +17,7 @@
 #include <folly/Benchmark.h>
 #include <folly/Range.h>
 #include <folly/experimental/symbolizer/Dwarf.h>
+#include <folly/experimental/symbolizer/SymbolizedFrame.h>
 #include <folly/portability/GFlags.h>
 
 void dummy() {}
@@ -25,7 +26,7 @@ namespace {
 
 using namespace folly::symbolizer;
 
-void run(Dwarf::LocationInfoMode mode, size_t n) {
+void run(LocationInfoMode mode, size_t n) {
   folly::BenchmarkSuspender suspender;
   // NOTE: Using '/proc/self/exe' only works if the code for @dummy is
   // statically linked into the binary.
@@ -33,25 +34,26 @@ void run(Dwarf::LocationInfoMode mode, size_t n) {
   Dwarf dwarf(&elf);
   suspender.dismiss();
   for (size_t i = 0; i < n; i++) {
-    Dwarf::LocationInfo info;
-    auto inlineInfo = std::
-        array<Dwarf::LocationInfo, Dwarf::kMaxInlineLocationInfoPerFrame>();
-    dwarf.findAddress(uintptr_t(&dummy), mode, info, folly::range(inlineInfo));
+    LocationInfo info;
+    auto inlineFrames =
+        std::array<SymbolizedFrame, Dwarf::kMaxInlineLocationInfoPerFrame>();
+    dwarf.findAddress(
+        uintptr_t(&dummy), mode, info, folly::range(inlineFrames));
   }
 }
 
 } // namespace
 
 BENCHMARK(DwarfFindAddressFast, n) {
-  run(folly::symbolizer::Dwarf::LocationInfoMode::FAST, n);
+  run(folly::symbolizer::LocationInfoMode::FAST, n);
 }
 
 BENCHMARK(DwarfFindAddressFull, n) {
-  run(folly::symbolizer::Dwarf::LocationInfoMode::FULL, n);
+  run(folly::symbolizer::LocationInfoMode::FULL, n);
 }
 
 BENCHMARK(DwarfFindAddressFullWithInline, n) {
-  run(folly::symbolizer::Dwarf::LocationInfoMode::FULL_WITH_INLINE, n);
+  run(folly::symbolizer::LocationInfoMode::FULL_WITH_INLINE, n);
 }
 
 int main(int argc, char* argv[]) {

folly/experimental/symbolizer/test/DwarfTests.cpp → folly/experimental/symbolizer/test/SymbolizedFrameTest.cpp

@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-#include <folly/experimental/symbolizer/Dwarf.h>
+#include <folly/experimental/symbolizer/SymbolizedFrame.h>
 
 #include <glog/logging.h>
 
 #include <folly/portability/GTest.h>
 
-using folly::symbolizer::Dwarf;
+using namespace folly::symbolizer;
 
 void checkPath(
     std::string expectedPath,
@@ -30,7 +30,7 @@ void checkPath(
     std::string rawBaseDir,
     std::string rawSubDir,
     std::string rawFile) {
-  Dwarf::Path path(rawBaseDir, rawSubDir, rawFile);
+  Path path(rawBaseDir, rawSubDir, rawFile);
 
   CHECK_EQ(expectedBaseDir, path.baseDir())
       << "Path(" << rawBaseDir << ", " << rawSubDir << ", " << rawFile << ")";
@@ -48,7 +48,7 @@ void checkPath(
   CHECK_EQ(expectedPath, std::string(buf, len));
 }
 
-TEST(Dwarf, Path) {
+TEST(SymbolizedFrame, Path) {
   checkPath("hello.cpp", "", "", "hello.cpp", "", "", "hello.cpp");
   checkPath("foo/hello.cpp", "foo", "", "hello.cpp", "foo", "", "hello.cpp");
   checkPath("foo/hello.cpp", "foo", "", "hello.cpp", "", "foo", "hello.cpp");

folly/experimental/symbolizer/test/SymbolizerTest.cpp

@@ -19,8 +19,10 @@
 #include <array>
 #include <cstdlib>
 
+#include <folly/Demangle.h>
 #include <folly/Range.h>
 #include <folly/String.h>
+#include <folly/experimental/symbolizer/SymbolizedFrame.h>
 #include <folly/portability/GTest.h>
 
 namespace folly {
@@ -33,10 +35,10 @@ TEST(Symbolizer, Single) {
   // It looks like we could only use .debug_aranges with "-g2", with
   // "-g1 -gdwarf-aranges", the code has to fallback to line-tables to
   // get the file name.
-  Symbolizer symbolizer(Dwarf::LocationInfoMode::FULL);
+  Symbolizer symbolizer(LocationInfoMode::FULL);
   SymbolizedFrame a;
   ASSERT_TRUE(symbolizer.symbolize(reinterpret_cast<uintptr_t>(foo), a));
-  EXPECT_EQ("folly::symbolizer::test::foo()", a.demangledName());
+  EXPECT_EQ("folly::symbolizer::test::foo()", folly::demangle(a.name));
 
   auto path = a.location.file.toString();
   folly::StringPiece basename(path);
@@ -115,7 +117,7 @@ TEST_F(ElfCacheTest, SignalSafeElfCache) {
 }
 
 TEST(SymbolizerTest, SymbolCache) {
-  Symbolizer symbolizer(nullptr, Dwarf::LocationInfoMode::FULL, 100);
+  Symbolizer symbolizer(nullptr, LocationInfoMode::FULL, 100);
 
   FrameArray<100> frames;
   bar(frames);
@@ -131,40 +133,61 @@ TEST(SymbolizerTest, SymbolCache) {
 
 namespace {
 
+size_t kQsortCallLineNo = 0;
+size_t kFooCallLineNo = 0;
+
 template <size_t kNumFrames = 100>
 FOLLY_ALWAYS_INLINE void inlineFoo(FrameArray<kNumFrames>& frames) {
   framesToFill = &frames;
   std::array<int, 2> a = {1, 2};
   // Use qsort, which is in a different library
+  kQsortCallLineNo = __LINE__ + 1;
   qsort(a.data(), 2, sizeof(int), comparator<kNumFrames>);
   framesToFill = nullptr;
 }
 
 template <size_t kNumFrames = 100>
 FOLLY_ALWAYS_INLINE void inlineBar(FrameArray<kNumFrames>& frames) {
+  kFooCallLineNo = __LINE__ + 1;
   inlineFoo(frames);
 }
 
 } // namespace
 
 TEST(SymbolizerTest, InlineFunctionBasic) {
-  Symbolizer symbolizer(
-      nullptr, Dwarf::LocationInfoMode::FULL_WITH_INLINE, 100);
+  Symbolizer symbolizer(nullptr, LocationInfoMode::FULL_WITH_INLINE, 0);
 
   FrameArray<100> frames;
   inlineBar<100>(frames);
   symbolizer.symbolize(frames);
 
+  // clang-fromat off
+  // Expected full stack trace:
+  //  Frame:
+  //  Frame: getStackTrace<100>
+  //  Frame: _ZN5folly10symbolizer4test10comparatorILm100EEEiPKvS4_
+  //  Frame: msort_with_tmp.part.0
+  //  Frame: __GI___qsort_r
+  //  Frame: inlineFoo<100>
+  //  Frame: inlineBar<100>
+  //  Frame: _ZN5folly10symbolizer4test43SymbolizerTest_InlineFunctionWithCache_Test8TestBodyEv
+  //  Frame: _ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc
+  //  Frame: _ZN7testing4Test3RunEv 2490 Frame: _ZN7testing8TestInfo3RunEv
+  //  Frame: _ZN7testing8TestCase3RunEv
+  //  Frame: _ZN7testing8internal12UnitTestImpl11RunAllTestsEv
+  //  Frame: _ZN7testing8UnitTest3RunEv 2473 Frame: _Z13RUN_ALL_TESTSv
+  // clang-fromat on
+  EXPECT_EQ(15, frames.frameCount);
   EXPECT_EQ("inlineFoo<100>", std::string(frames.frames[5].name));
   EXPECT_EQ(
       "folly/experimental/symbolizer/test/SymbolizerTest.cpp",
       std::string(frames.frames[5].location.file.toString()));
-  EXPECT_EQ(139, frames.frames[5].location.line);
+  EXPECT_EQ(kQsortCallLineNo, frames.frames[5].location.line);
   EXPECT_EQ("inlineBar<100>", std::string(frames.frames[6].name));
   EXPECT_EQ(
       "folly/experimental/symbolizer/test/SymbolizerTest.cpp",
       std::string(frames.frames[6].location.file.toString()));
-  EXPECT_EQ(145, frames.frames[6].location.line);
+  EXPECT_EQ(kFooCallLineNo, frames.frames[6].location.line);
 
   FrameArray<100> frames2;
   inlineBar<100>(frames2);
@@ -176,13 +199,12 @@ TEST(SymbolizerTest, InlineFunctionBasic) {
 
 // No inline frames should be filled because of no extra frames.
 TEST(SymbolizerTest, InlineFunctionBasicNoExtraFrames) {
-  Symbolizer symbolizer(
-      nullptr, Dwarf::LocationInfoMode::FULL_WITH_INLINE, 100);
+  Symbolizer symbolizer(nullptr, LocationInfoMode::FULL_WITH_INLINE, 100);
   FrameArray<8> frames;
   inlineBar<8>(frames);
   symbolizer.symbolize(frames);
 
-  Symbolizer symbolizer2(nullptr, Dwarf::LocationInfoMode::FULL, 100);
+  Symbolizer symbolizer2(nullptr, LocationInfoMode::FULL, 100);
   FrameArray<8> frames2;
   inlineBar<8>(frames2);
   symbolizer2.symbolize(frames2);
@@ -192,6 +214,33 @@ TEST(SymbolizerTest, InlineFunctionBasicNoExtraFrames) {
   }
 }
 
+TEST(SymbolizerTest, InlineFunctionWithCache) {
+  Symbolizer symbolizer(nullptr, LocationInfoMode::FULL_WITH_INLINE, 100);
+
+  FrameArray<100> frames;
+  inlineBar<100>(frames);
+  symbolizer.symbolize(frames);
+
+  EXPECT_EQ(15, frames.frameCount);
+  EXPECT_EQ("inlineFoo<100>", std::string(frames.frames[5].name));
+  EXPECT_EQ(
+      "folly/experimental/symbolizer/test/SymbolizerTest.cpp",
+      std::string(frames.frames[5].location.file.toString()));
+  EXPECT_EQ(kQsortCallLineNo, frames.frames[5].location.line);
+  EXPECT_EQ("inlineBar<100>", std::string(frames.frames[6].name));
+  EXPECT_EQ(
+      "folly/experimental/symbolizer/test/SymbolizerTest.cpp",
+      std::string(frames.frames[6].location.file.toString()));
+  EXPECT_EQ(kFooCallLineNo, frames.frames[6].location.line);
+
+  FrameArray<100> frames2;
+  inlineBar<100>(frames2);
+  symbolizer.symbolize(frames2);
+  for (size_t i = 0; i < frames.frameCount; i++) {
+    EXPECT_STREQ(frames.frames[i].name, frames2.frames[i].name);
+  }
+}
+
 } // namespace test
 } // namespace symbolizer
 } // namespace folly