Change symbolize function to return the actual used frame count.

Summary:
If inline functions are enabled, actual frames used can be larger than the
address count.

Reviewed By: luciang

Differential Revision: D21371112

fbshipit-source-id: c1e602b62b6af427f11701bf25894a99f554ec47

folly/experimental/symbolizer/Symbolizer.cpp

@@ -34,10 +34,10 @@
 #include <folly/Memory.h>
 #include <folly/ScopeGuard.h>
 #include <folly/String.h>
-
 #include <folly/experimental/symbolizer/Dwarf.h>
 #include <folly/experimental/symbolizer/Elf.h>
 #include <folly/experimental/symbolizer/LineReader.h>
+#include <folly/lang/SafeAssert.h>
 #include <folly/portability/SysMman.h>
 #include <folly/portability/Unistd.h>
 
@@ -94,33 +94,23 @@ Symbolizer::Symbolizer(
   }
 }
 
-void Symbolizer::symbolize(
+size_t Symbolizer::symbolize(
     folly::Range<const uintptr_t*> addrs,
     folly::Range<SymbolizedFrame*> frames) {
   size_t addrCount = addrs.size();
   size_t frameCount = frames.size();
-  size_t remaining = 0;
-  for (size_t i = 0; i < addrCount; ++i) {
-    auto& frame = frames[i];
-    if (!frame.found) {
-      ++remaining;
-      frame.clear();
-    }
-  }
-
-  if (remaining == 0) { // we're done
-    return;
-  }
+  FOLLY_SAFE_CHECK(addrCount <= frameCount, "Not enough frames.");
+  size_t remaining = addrCount;
 
   if (_r_debug.r_version != 1) {
-    return;
+    return 0;
   }
 
   char selfPath[PATH_MAX + 8];
   ssize_t selfSize;
   if ((selfSize = readlink("/proc/self/exe", selfPath, PATH_MAX + 1)) == -1) {
     // Something has gone terribly wrong.
-    return;
+    return 0;
   }
   selfPath[selfSize] = '\0';
 
@@ -230,6 +220,8 @@ void Symbolizer::symbolize(
       }
     }
   }
+
+  return addrCount;
 }
 
 namespace {

folly/experimental/symbolizer/Symbolizer.h

@@ -98,32 +98,32 @@ class Symbolizer {
       size_t symbolCacheSize = 0);
 
   /**
-   *  Symbolize given addresses.
+   *  Symbolize given addresses and return the number of @frames filled:
    *
    * - all entries in @addrs will be symbolized (if possible, e.g. if they're
-   *   valid code addresses)
+   *   valid code addresses and if frames.size() >= addrs.size())
    *
    * - if `mode_ == FULL_WITH_INLINE` and `frames.size() > addrs.size()` then at
    *   most `frames.size() - addrs.size()` additional inlined functions will
    *   also be symbolized (at most `kMaxInlineLocationInfoPerFrame` per @addr
    *   entry).
    */
-  void symbolize(
+  size_t symbolize(
       folly::Range<const uintptr_t*> addrs,
       folly::Range<SymbolizedFrame*> frames);
 
-  void symbolize(
+  size_t symbolize(
       const uintptr_t* addresses,
       SymbolizedFrame* frames,
       size_t frameCount) {
-    symbolize(
+    return symbolize(
         folly::Range<const uintptr_t*>(addresses, frameCount),
         folly::Range<SymbolizedFrame*>(frames, frameCount));
   }
 
   template <size_t N>
-  void symbolize(FrameArray<N>& fa) {
-    symbolize(
+  size_t symbolize(FrameArray<N>& fa) {
+    return symbolize(
         folly::Range<const uintptr_t*>(fa.addresses, fa.frameCount),
         folly::Range<SymbolizedFrame*>(fa.frames, N));
   }

folly/experimental/symbolizer/test/SymbolizerTest.cpp

@@ -134,6 +134,7 @@ FOLLY_NOINLINE void lexicalBlockBar(FrameArray<kNumFrames>& frames) try {
 }
 
 void verifyStackTrace(
+    Symbolizer& symbolizer,
     const FrameArray<100>& frames,
     const std::string& barName,
     size_t barLine,
@@ -149,6 +150,12 @@ void verifyStackTrace(
   EXPECT_EQ(barName, std::string(folly::demangle(frames.frames[6].name)));
   EXPECT_EQ(barFile, std::string(frames.frames[6].location.file.toString()));
   EXPECT_EQ(barLine, frames.frames[6].location.line);
+
+  FrameArray<10> singleAddressFrames;
+  singleAddressFrames.frameCount = 1;
+  singleAddressFrames.addresses[0] = frames.frames[7].addr;
+  // Two inline function calls are added into frames.
+  EXPECT_EQ(3, symbolizer.symbolize(singleAddressFrames));
 }
 
 template <size_t kNumFrames = 100>
@@ -208,6 +215,7 @@ TEST(SymbolizerTest, InlineFunctionBasic) {
   //  Frame: _ZN7testing8internal12UnitTestImpl11RunAllTestsEv
   // clang-format on
   verifyStackTrace(
+      symbolizer,
       frames,
       "void folly::symbolizer::test::inlineBar<100ul>("
       "folly::symbolizer::FrameArray<100ul>&)",
@@ -261,6 +269,7 @@ TEST(SymbolizerTest, InlineFunctionInLexicalBlock) {
   symbolizer.symbolize(frames);
 
   verifyStackTrace(
+      symbolizer,
       frames,
       "void folly::symbolizer::test::inlineBar<100ul>("
       "folly::symbolizer::FrameArray<100ul>&)",
@@ -304,6 +313,7 @@ TEST(SymbolizerTest, InlineClassMemberFunction) {
   symbolizer.symbolize(frames);
 
   verifyStackTrace(
+      symbolizer,
       frames,
       "folly::symbolizer::test::ClassWithInlineFunctions::inlineBar("
       "folly::symbolizer::FrameArray<100ul>&) const",
@@ -319,6 +329,7 @@ TEST(SymbolizerTest, StaticInlineClassMemberFunction) {
   symbolizer.symbolize(frames);
 
   verifyStackTrace(
+      symbolizer,
       frames,
       "folly::symbolizer::test::ClassWithInlineFunctions::staticInlineBar("
       "folly::symbolizer::FrameArray<100ul>&)",
@@ -335,6 +346,7 @@ TEST(SymbolizerTest, InlineClassMemberFunctionInDifferentFile) {
   symbolizer.symbolize(frames);
 
   verifyStackTrace(
+      symbolizer,
       frames,
       "folly::symbolizer::test::InlineFunctionsWrapper::inlineBar("
       "folly::symbolizer::FrameArray<100ul>&) const",
@@ -350,6 +362,7 @@ TEST(SymbolizerTest, StaticInlineClassMemberFunctionInDifferentFile) {
   symbolizer.symbolize(frames);
 
   verifyStackTrace(
+      symbolizer,
       frames,
       "folly::symbolizer::test::InlineFunctionsWrapper::staticInlineBar("
       "folly::symbolizer::FrameArray<100ul>&)",
@@ -380,6 +393,7 @@ TEST(SymbolizerTest, InlineFunctionWithCache) {
   symbolizer.symbolize(frames);
 
   verifyStackTrace(
+      symbolizer,
       frames,
       "void folly::symbolizer::test::inlineBar<100ul>("
       "folly::symbolizer::FrameArray<100ul>&)",