Use fixed size stack traces; unify getStackTrace

Summary:
Also, switch to the simpler unw_backtrace(), which has the nice advantage of
actually doing IP adjustment (-1 in certain frames) correctly, unlike me :)

This is in preparation for the faster backtrace in libunwind 1.1.

Test Plan: folly/experimental/exception_tracer, folly/experimental/symbolizer, admarket/lib/util:memory_tracker_test

Reviewed By: lucian@fb.com

FB internal diff: D1088357

folly/experimental/exception_tracer/ExceptionTracer.cpp

@@ -29,17 +29,18 @@
 
 namespace {
 
+using namespace ::folly::exception_tracer;
+using namespace ::folly::symbolizer;
+using namespace __cxxabiv1;
+
 extern "C" {
-const StackTraceStack* getExceptionStackTraceStack(void) __attribute__((weak));
-typedef const StackTraceStack* (*GetExceptionStackTraceStackType)(void);
+StackTraceStack* getExceptionStackTraceStack(void) __attribute__((weak));
+typedef StackTraceStack* (*GetExceptionStackTraceStackType)(void);
 GetExceptionStackTraceStackType getExceptionStackTraceStackFn;
 }
 
 }  // namespace
 
-using namespace ::folly::symbolizer;
-using namespace __cxxabiv1;
-
 namespace folly {
 namespace exception_tracer {
 
@@ -54,19 +55,23 @@ std::ostream& operator<<(std::ostream& out, const ExceptionInfo& info) {
       << (info.frames.size() == 1 ? " frame" : " frames")
       << ")\n";
   try {
-    std::vector<FrameInfo> addresses;
-    addresses.reserve(info.frames.size());
-    for (auto ip : info.frames) {
-      // Symbolize the previous address because the IP might be in the
-      // next function, per glog/src/signalhandler.cc
-      addresses.emplace_back(ip - 1);
-    }
+    ssize_t frameCount = info.frames.size();
+    // Skip our own internal frames
+    static constexpr size_t skip = 3;
+
+    if (frameCount > skip) {
+      auto addresses = info.frames.data() + skip;
+      frameCount -= skip;
+
+      std::vector<SymbolizedFrame> frames;
+      frames.resize(frameCount);
 
       Symbolizer symbolizer;
-    symbolizer.symbolize(addresses.data(), addresses.size());
+      symbolizer.symbolize(addresses, frames.data(), frameCount);
 
       OStreamSymbolizePrinter osp(out);
-    osp.print(addresses.data(), addresses.size(), addresses.size());
+      osp.print(addresses, frames.data(), frameCount);
+    }
   } catch (const std::exception& e) {
     out << "\n !! caught " << folly::exceptionStr(e) << "\n";
   } catch (...) {
@@ -118,8 +123,7 @@ std::vector<ExceptionInfo> getCurrentExceptions() {
     return exceptions;
   }
 
-  bool hasTraceStack = false;
-  const StackTraceStack* traceStack = nullptr;
+  StackTraceStack* traceStack = nullptr;
   if (!getExceptionStackTraceStackFn) {
     static bool logged = false;
     if (!logged) {
@@ -134,10 +138,9 @@ std::vector<ExceptionInfo> getCurrentExceptions() {
         << "Exception stack trace invalid, stack traces not available";
       logged = true;
     }
-  } else {
-    hasTraceStack = true;
   }
 
+  StackTrace* trace = traceStack ? traceStack->top() : nullptr;
   while (currentException) {
     ExceptionInfo info;
     // Dependent exceptions (thrown via std::rethrow_exception) aren't
@@ -148,18 +151,16 @@ std::vector<ExceptionInfo> getCurrentExceptions() {
       isAbiCppException(currentException) ?
       currentException->exceptionType :
       nullptr;
-    if (hasTraceStack) {
-      CHECK(traceStack) << "Invalid trace stack!";
-      info.frames.assign(
-          traceStack->trace.frameIPs,
-          traceStack->trace.frameIPs + traceStack->trace.frameCount);
-      traceStack = traceStack->next;
+    if (traceStack) {
+      CHECK(trace) << "Invalid trace stack!";
+      info.frames.assign(trace->addresses,
+                         trace->addresses + trace->frameCount);
+      trace = traceStack->next(trace);
     }
     currentException = currentException->nextException;
     exceptions.push_back(std::move(info));
   }
-
-  CHECK(!traceStack) << "Invalid trace stack!";
+  CHECK(!trace) << "Invalid trace stack!";
 
   return exceptions;
 }

folly/experimental/exception_tracer/ExceptionTracerLib.cpp

@@ -24,6 +24,7 @@
 #include "folly/experimental/exception_tracer/StackTrace.h"
 #include "folly/experimental/exception_tracer/ExceptionAbi.h"
 #include "folly/experimental/exception_tracer/ExceptionTracer.h"
+#include "folly/experimental/symbolizer/Symbolizer.h"
 
 namespace __cxxabiv1 {
 
@@ -37,11 +38,13 @@ void __cxa_end_catch(void);
 
 }  // namespace __cxxabiv1
 
+using namespace folly::exception_tracer;
+
 namespace {
 
 __thread bool invalid;
-__thread StackTraceStack* activeExceptions;
-__thread StackTraceStack* caughtExceptions;
+__thread StackTraceStack activeExceptions;
+__thread StackTraceStack caughtExceptions;
 pthread_once_t initialized = PTHREAD_ONCE_INIT;
 
 extern "C" {
@@ -86,34 +89,34 @@ void initialize() {
 }  // namespace
 
 // This function is exported and may be found via dlsym(RTLD_NEXT, ...)
-extern "C" const StackTraceStack* getExceptionStackTraceStack() {
-  return caughtExceptions;
+extern "C" StackTraceStack* getExceptionStackTraceStack() {
+  return invalid ? nullptr : &caughtExceptions;
 }
 
 namespace {
-// Make sure we're counting stack frames correctly for the "skip" argument to
-// pushCurrentStackTrace, don't inline.
+
+// Make sure we're counting stack frames correctly, don't inline.
 void addActiveException() __attribute__((noinline));
 
 void addActiveException() {
   pthread_once(&initialized, initialize);
   // Capture stack trace
   if (!invalid) {
-    if (pushCurrentStackTrace(3, &activeExceptions) != 0) {
-      clearStack(&activeExceptions);
-      clearStack(&caughtExceptions);
+    if (!activeExceptions.pushCurrent()) {
+      activeExceptions.clear();
+      caughtExceptions.clear();
       invalid = true;
     }
   }
 }
 
-void moveTopException(StackTraceStack** from, StackTraceStack** to) {
+void moveTopException(StackTraceStack& from, StackTraceStack& to) {
   if (invalid) {
     return;
   }
-  if (moveTop(from, to) != 0) {
-    clearStack(from);
-    clearStack(to);
+  if (!to.moveTopFrom(from)) {
+    from.clear();
+    to.clear();
     invalid = true;
   }
 }
@@ -134,13 +137,13 @@ void __cxa_rethrow() {
   // we'll implement something simpler (and slower): we pop the exception from
   // the caught stack, and push it back onto the active stack; this way, our
   // implementation of __cxa_begin_catch doesn't have to do anything special.
-  moveTopException(&caughtExceptions, &activeExceptions);
+  moveTopException(caughtExceptions, activeExceptions);
   orig_cxa_rethrow();
 }
 
 void* __cxa_begin_catch(void *excObj) {
   // excObj is a pointer to the unwindHeader in __cxa_exception
-  moveTopException(&activeExceptions, &caughtExceptions);
+  moveTopException(activeExceptions, caughtExceptions);
   return orig_cxa_begin_catch(excObj);
 }
 
@@ -153,7 +156,10 @@ void __cxa_end_catch() {
     // In the rethrow case, we've already popped the exception off the
     // caught stack, so we don't do anything here.
     if (top->handlerCount == 1) {
-      popStackTrace(&caughtExceptions);
+      if (!caughtExceptions.pop()) {
+        activeExceptions.clear();
+        invalid = true;
+      }
     }
   }
   orig_cxa_end_catch();

folly/experimental/exception_tracer/StackTrace.cpp

+/*
+ * Copyright 2013 Facebook, Inc.
+ *
+ * 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/exception_tracer/StackTrace.h"
+
+#include <cassert>
+#include <cstdlib>
+#include <new>
+#include "folly/experimental/symbolizer/StackTrace.h"
+
+namespace folly { namespace exception_tracer {
+
+class StackTraceStack::Node : public StackTrace {
+ public:
+  static Node* allocate();
+  void deallocate();
+
+  Node* next;
+
+ private:
+  Node() : next(nullptr) { }
+  ~Node() { }
+};
+
+auto StackTraceStack::Node::allocate() -> Node* {
+  // Null pointer on error, please.
+  return new (std::nothrow) Node();
+}
+
+void StackTraceStack::Node::deallocate() {
+  delete this;
+}
+
+bool StackTraceStack::pushCurrent() {
+  checkGuard();
+  auto node = Node::allocate();
+  if (!node) {
+    // cannot allocate memory
+    return false;
+  }
+
+  ssize_t n = folly::symbolizer::getStackTrace(node->addresses, kMaxFrames);
+  if (n == -1) {
+    node->deallocate();
+    return false;
+  }
+  node->frameCount = n;
+
+  node->next = top_;
+  top_ = node;
+  return true;
+}
+
+bool StackTraceStack::pop() {
+  checkGuard();
+  if (!top_) {
+    return false;
+  }
+
+  auto node = top_;
+  top_ = node->next;
+  node->deallocate();
+  return true;
+}
+
+bool StackTraceStack::moveTopFrom(StackTraceStack& other) {
+  checkGuard();
+  if (!other.top_) {
+    return false;
+  }
+
+  auto node = other.top_;
+  other.top_ = node->next;
+  node->next = top_;
+  top_ = node;
+  return true;
+}
+
+void StackTraceStack::clear() {
+  checkGuard();
+  while (top_) {
+    pop();
+  }
+}
+
+StackTrace* StackTraceStack::top() {
+  checkGuard();
+  return top_;
+}
+
+StackTrace* StackTraceStack::next(StackTrace* p) {
+  checkGuard();
+  assert(p);
+  return static_cast<Node*>(p)->next;
+}
+
+}}  // namespaces
+

folly/experimental/exception_tracer/StackTrace.h

@@ -18,66 +18,86 @@
 #ifndef FOLLY_EXPERIMENTAL_EXCEPTION_TRACER_STACKTRACE_H_
 #define FOLLY_EXPERIMENTAL_EXCEPTION_TRACER_STACKTRACE_H_
 
-#include <stddef.h>
-#include <stdint.h>
+#include <cassert>
+#include <cstddef>
+#include <cstdint>
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+namespace folly { namespace exception_tracer {
+
+constexpr size_t kMaxFrames = 500;
+
+struct StackTrace {
+  StackTrace() : frameCount(0) { }
 
-typedef struct StackTrace {
-  uintptr_t* frameIPs;  /* allocated with malloc() */
   size_t frameCount;
-} StackTrace;
+  uintptr_t addresses[kMaxFrames];
+};
 
-/**
- * Get the current stack trace, allocating trace->frameIPs using malloc().
- * Skip the topmost "skip" frames.
- * Return 0 on success, a negative value on error.
- * On error, trace->frameIPs is NULL.
+// note: no constructor so this can be __thread.
+// A StackTraceStack MUST be placed in zero-initialized memory.
+class StackTraceStack {
+  class Node;
+ public:
+  /**
+   * Push the current stack trace onto the stack.
+   * Returns false on failure (not enough memory, getting stack trace failed),
+   * true on success.
    */
-int getCurrentStackTrace(size_t skip, StackTrace* trace);
+  bool pushCurrent();
 
-/**
- * Free data allocated in a StackTrace object.
+  /**
+   * Pop the top stack trace from the stack.
+   * Returns true on success, false on failure (stack was empty).
    */
-void destroyStackTrace(StackTrace* trace);
+  bool pop();
 
-/**
- * A stack of stack traces.
+  /**
+   * Move the top stack trace from other onto this.
+   * Returns true on success, false on failure (other was empty).
    */
-typedef struct StackTraceStack {
-  StackTrace trace;
-  struct StackTraceStack* next;
-} StackTraceStack;
+  bool moveTopFrom(StackTraceStack& other);
 
-/**
- * Push the current stack trace onto the stack.
- * Return 0 on success, a negative value on error.
- * On error, the stack is unchanged.
+  /**
+   * Clear the stack.
    */
-int pushCurrentStackTrace(size_t skip, StackTraceStack** head);
 
-/**
- * Pop (and destroy) the top stack trace from the stack.
+  void clear();
+
+  /**
+   * Is the stack empty?
    */
-void popStackTrace(StackTraceStack** head);
+  bool empty() const { return !top_; }
 
-/**
- * Completely empty the stack, destroying everything.
+  /**
+   * Return the top stack trace, or nullptr if the stack is empty.
    */
-void clearStack(StackTraceStack** head);
+  StackTrace* top();
 
-/**
- * Move the top stack trace from one stack to another.
- * Return 0 on success, a negative value on error (if the source stack is
- * empty)
+  /**
+   * Return the stack trace following p, or nullptr if p is the bottom of
+   * the stack.
    */
-int moveTop(StackTraceStack** from, StackTraceStack** to);
+  StackTrace* next(StackTrace* p);
+
+ private:
+  // In debug mode, we assert that we're in zero-initialized memory by
+  // checking that the two guards around top_ are zero.
+  void checkGuard() const {
+#ifndef NDEBUG
+    assert(guard1_ == 0 && guard2_ == 0);
+#endif
+  }
 
-#ifdef __cplusplus
-}  /* extern "C" */
+#ifndef NDEBUG
+  uintptr_t guard1_;
 #endif
+  Node* top_;
+#ifndef NDEBUG
+  uintptr_t guard2_;
+#endif
+};
+
+}}  // namespaces
 
 #endif /* FOLLY_EXPERIMENTAL_EXCEPTION_TRACER_STACKTRACE_H_ */
 

folly/experimental/symbolizer/SignalHandler.cpp

@@ -44,30 +44,28 @@ namespace {
  */
 class FatalSignalCallbackRegistry {
  public:
-  typedef std::function<void()> Func;
-
   FatalSignalCallbackRegistry();
 
-  void add(Func func);
+  void add(SignalCallback func);
   void markInstalled();
   void run();
 
  private:
   std::atomic<bool> installed_;
   std::mutex mutex_;
-  std::vector<Func> handlers_;
+  std::vector<SignalCallback> handlers_;
 };
 
 FatalSignalCallbackRegistry::FatalSignalCallbackRegistry()
   : installed_(false) {
 }
 
-void FatalSignalCallbackRegistry::add(Func func) {
+void FatalSignalCallbackRegistry::add(SignalCallback func) {
   std::lock_guard<std::mutex> lock(mutex_);
   CHECK(!installed_)
     << "FatalSignalCallbackRegistry::add may not be used "
        "after installing the signal handlers.";
-  handlers_.push_back(std::move(func));
+  handlers_.push_back(func);
 }
 
 void FatalSignalCallbackRegistry::markInstalled() {
@@ -240,12 +238,7 @@ void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
 
 void signalHandler(int signum, siginfo_t* info, void* uctx) {
   SCOPE_EXIT { fsyncNoInt(STDERR_FILENO); };
-  try {
   innerSignalHandler(signum, info, uctx);
-  } catch (...) {
-    // Ignore any exceptions. What? Exceptions?
-    print("Exception in innerSignalHandler!\n");
-  }
 
   gSignalThread = nullptr;
   // Kill ourselves with the previous handler.
@@ -254,8 +247,8 @@ void signalHandler(int signum, siginfo_t* info, void* uctx) {
 
 }  // namespace
 
-void addFatalSignalCallback(std::function<void()> handler) {
-  gFatalSignalCallbackRegistry->add(std::move(handler));
+void addFatalSignalCallback(SignalCallback cb) {
+  gFatalSignalCallbackRegistry->add(cb);
 }
 
 namespace {

folly/experimental/symbolizer/SignalHandler.h

@@ -41,7 +41,8 @@ void installFatalSignalHandler();
  * All these fatal callback must be added before calling
  * installFatalSignalHandler().
  */
-void addFatalSignalCallback(std::function<void()> callback);
+typedef void (*SignalCallback)(void);
+void addFatalSignalCallback(SignalCallback callback);
 
 
 }}  // namespaces

folly/experimental/symbolizer/StackTrace.cpp

+/*
+ * Copyright 2013 Facebook, Inc.
+ *
+ * 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.
+ */
+
+// Must be first to ensure that UNW_LOCAL_ONLY is defined
+#define UNW_LOCAL_ONLY 1
+#include <libunwind.h>
+
+#include "folly/experimental/symbolizer/StackTrace.h"
+
+namespace folly { namespace symbolizer {
+
+ssize_t getStackTrace(uintptr_t* addresses, size_t maxAddresses) {
+  static_assert(sizeof(uintptr_t) == sizeof(void*),
+                "uinptr_t / pointer size mismatch");
+  int r = unw_backtrace(reinterpret_cast<void**>(addresses), maxAddresses);
+  return r < 0 ? -1 : r;
+}
+
+}}  // namespaces
+

folly/experimental/symbolizer/StackTrace.h

+/*
+ * Copyright 2013 Facebook, Inc.
+ *
+ * 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.
+ */
+
+#ifndef FOLLY_SYMBOLIZER_STACKTRACE_H_
+#define FOLLY_SYMBOLIZER_STACKTRACE_H_
+
+#include <cstddef>
+#include <cstdint>
+
+namespace folly { namespace symbolizer {
+
+/**
+ * Get the current stack trace into addresses, which has room for at least
+ * maxAddresses frames.
+ *
+ * Returns the number of frames written in the array.
+ * Returns -1 on failure.
+ */
+ssize_t getStackTrace(uintptr_t* addresses, size_t maxAddresses);
+
+}}  // namespaces
+
+#endif /* FOLLY_SYMBOLIZER_STACKTRACE_H_ */
+

folly/experimental/symbolizer/Symbolizer.cpp

@@ -14,10 +14,6 @@
  * limitations under the License.
  */
 
-// Must be first to ensure that UNW_LOCAL_ONLY is defined
-#define UNW_LOCAL_ONLY 1
-#include <libunwind.h>
-
 #include "folly/experimental/symbolizer/Symbolizer.h"
 
 #include <limits.h>
@@ -148,62 +144,16 @@ bool parseProcMapsLine(StringPiece line,
 
 }  // namespace
 
-ssize_t getStackTrace(FrameInfo* addresses,
-                      size_t maxAddresses,
-                      size_t skip) {
-  unw_context_t uctx;
-  int r = unw_getcontext(&uctx);
-  if (r < 0) {
-    return -1;
-  }
-
-  unw_cursor_t cursor;
-  size_t idx = 0;
-  bool first = true;
-  for (;;) {
-    if (first) {
-      first = false;
-      r = unw_init_local(&cursor, &uctx);
-    } else {
-      r = unw_step(&cursor);
-      if (r == 0) {
-        break;
-      }
-    }
-    if (r < 0) {
-      return -1;
-    }
-
-    if (skip != 0) {
-      --skip;
-      continue;
-    }
-
-    if (idx < maxAddresses) {
-      unw_word_t ip;
-      int rr = unw_get_reg(&cursor, UNW_REG_IP, &ip);
-      if (rr < 0) {
-        return -1;
-      }
-
-      // If error, assume not a signal frame
-      rr = unw_is_signal_frame(&cursor);
-      addresses[idx] = FrameInfo(ip, (rr > 0));
-    }
-    ++idx;
-  }
-
-  return idx;
-}
-
-void Symbolizer::symbolize(FrameInfo* addresses, size_t addressCount) {
+void Symbolizer::symbolize(const uintptr_t* addresses,
+                           SymbolizedFrame* frames,
+                           size_t addressCount) {
   size_t remaining = 0;
   for (size_t i = 0; i < addressCount; ++i) {
-    auto& ainfo = addresses[i];
-    if (!ainfo.found) {
+    auto& frame = frames[i];
+    if (!frame.found) {
       ++remaining;
-      ainfo.name.clear();
-      ainfo.location = Dwarf::LocationInfo();
+      frame.name.clear();
+      frame.location = Dwarf::LocationInfo();
     }
   }
 
@@ -240,27 +190,19 @@ void Symbolizer::symbolize(FrameInfo* addresses, size_t addressCount) {
 
     // See if any addresses are here
     for (size_t i = 0; i < addressCount; ++i) {
-      auto& ainfo = addresses[i];
-      if (ainfo.found) {
+      auto& frame = frames[i];
+      if (frame.found) {
         continue;
       }
 
-      uintptr_t address = ainfo.address;
-
-      // If the next address (closer to the top of the stack) was a signal
-      // frame, then this is the *resume* address, which is the address
-      // after the location where the signal was caught. This might be in
-      // the next function, so subtract 1 before symbolizing.
-      if (i != 0 && addresses[i-1].isSignalFrame) {
-        --address;
-      }
+      uintptr_t address = addresses[i];
 
       if (from > address || address >= to) {
         continue;
       }
 
       // Found
-      ainfo.found = true;
+      frame.found = true;
       --remaining;
 
       // Open the file on first use
@@ -290,10 +232,10 @@ void Symbolizer::symbolize(FrameInfo* addresses, size_t addressCount) {
       }
       auto name = elfFile->getSymbolName(sym);
       if (name) {
-        ainfo.name = name;
+        frame.name = name;
       }
 
-      Dwarf(elfFile).findAddress(fileAddress, ainfo.location);
+      Dwarf(elfFile).findAddress(fileAddress, frame.location);
     }
   }
 
@@ -304,8 +246,7 @@ namespace {
 const char kHexChars[] = "0123456789abcdef";
 }  // namespace
 
-void SymbolizePrinter::print(const FrameInfo& ainfo) {
-  uintptr_t address = ainfo.address;
+void SymbolizePrinter::print(uintptr_t address, const SymbolizedFrame& frame) {
   // Can't use sprintf, not async-signal-safe
   static_assert(sizeof(uintptr_t) <= 8, "huge uintptr_t?");
   char buf[] = "    @ 0000000000000000";
@@ -322,20 +263,20 @@ void SymbolizePrinter::print(const FrameInfo& ainfo) {
   doPrint(folly::StringPiece(buf, end));
 
   char mangledBuf[1024];
-  if (!ainfo.found) {
+  if (!frame.found) {
     doPrint(" (not found)\n");
     return;
   }
 
-  if (ainfo.name.empty()) {
+  if (frame.name.empty()) {
     doPrint(" (unknown)\n");
-  } else if (ainfo.name.size() >= sizeof(mangledBuf)) {
+  } else if (frame.name.size() >= sizeof(mangledBuf)) {
     doPrint(" ");
-    doPrint(ainfo.name);
+    doPrint(frame.name);
     doPrint("\n");
   } else {
-    memcpy(mangledBuf, ainfo.name.data(), ainfo.name.size());
-    mangledBuf[ainfo.name.size()] = '\0';
+    memcpy(mangledBuf, frame.name.data(), frame.name.size());
+    mangledBuf[frame.name.size()] = '\0';
 
     char demangledBuf[1024];
     demangle(mangledBuf, demangledBuf, sizeof(demangledBuf));
@@ -346,23 +287,23 @@ void SymbolizePrinter::print(const FrameInfo& ainfo) {
 
   char fileBuf[PATH_MAX];
   fileBuf[0] = '\0';
-  if (ainfo.location.hasFileAndLine) {
-    ainfo.location.file.toBuffer(fileBuf, sizeof(fileBuf));
+  if (frame.location.hasFileAndLine) {
+    frame.location.file.toBuffer(fileBuf, sizeof(fileBuf));
     doPrint(pad);
     doPrint(fileBuf);
 
     char buf[22];
-    uint32_t n = uint64ToBufferUnsafe(ainfo.location.line, buf);
+    uint32_t n = uint64ToBufferUnsafe(frame.location.line, buf);
     doPrint(":");
     doPrint(StringPiece(buf, n));
     doPrint("\n");
   }
 
-  if (ainfo.location.hasMainFile) {
+  if (frame.location.hasMainFile) {
     char mainFileBuf[PATH_MAX];
     mainFileBuf[0] = '\0';
-    ainfo.location.mainFile.toBuffer(mainFileBuf, sizeof(mainFileBuf));
-    if (!ainfo.location.hasFileAndLine || strcmp(fileBuf, mainFileBuf)) {
+    frame.location.mainFile.toBuffer(mainFileBuf, sizeof(mainFileBuf));
+    if (!frame.location.hasFileAndLine || strcmp(fileBuf, mainFileBuf)) {
       doPrint(pad);
       doPrint("-> ");
       doPrint(mainFileBuf);
@@ -371,21 +312,11 @@ void SymbolizePrinter::print(const FrameInfo& ainfo) {
   }
 }
 
-void SymbolizePrinter::print(const FrameInfo* addresses,
-                             size_t addressesSize,
+void SymbolizePrinter::print(const uintptr_t* addresses,
+                             const SymbolizedFrame* frames,
                              size_t frameCount) {
-  for (size_t i = 0; i < std::min(addressesSize, frameCount); ++i) {
-    auto& ainfo = addresses[i];
-    print(ainfo);
-  }
-
-  // Indicate the number of frames that we couldn't log due to space
-  if (frameCount > addressesSize) {
-    char buf[22];
-    uint32_t n = uint64ToBufferUnsafe(frameCount - addressesSize, buf);
-    doPrint("    (");
-    doPrint(StringPiece(buf, n));
-    doPrint(" omitted, max buffer size reached)\n");
+  for (size_t i = 0; i < frameCount; ++i) {
+    print(addresses[i], frames[i]);
   }
 }
 
@@ -397,11 +328,5 @@ void FDSymbolizePrinter::doPrint(StringPiece sp) {
   writeFull(fd_, sp.data(), sp.size());
 }
 
-std::ostream& operator<<(std::ostream& out, const FrameInfo& ainfo) {
-  OStreamSymbolizePrinter osp(out);
-  osp.print(ainfo);
-  return out;
-}
-
 }  // namespace symbolizer
 }  // namespace folly

folly/experimental/symbolizer/Symbolizer.h

@@ -25,6 +25,7 @@
 #include "folly/Range.h"
 #include "folly/experimental/symbolizer/Elf.h"
 #include "folly/experimental/symbolizer/Dwarf.h"
+#include "folly/experimental/symbolizer/StackTrace.h"
 
 namespace folly {
 namespace symbolizer {
@@ -33,14 +34,10 @@ namespace symbolizer {
  * Frame information: symbol name and location.
  *
  * Note that both name and location are references in the Symbolizer object,
- * which must outlive this FrameInfo object.
+ * which must outlive this SymbolizedFrame object.
  */
-struct FrameInfo {
-  /* implicit */ FrameInfo(uintptr_t a=0, bool sf=false)
-    : address(a),
-      isSignalFrame(sf),
-      found(false) { }
-  uintptr_t address;
+struct SymbolizedFrame {
+  SymbolizedFrame() : found(false) { }
   bool isSignalFrame;
   bool found;
   StringPiece name;
@@ -52,34 +49,23 @@ struct FrameArray {
   FrameArray() : frameCount(0) { }
 
   size_t frameCount;
-  FrameInfo frames[N];
+  uintptr_t addresses[N];
+  SymbolizedFrame frames[N];
 };
 
-/**
- * Get the current stack trace into addresses, which has room for at least
- * maxAddresses frames. Skip the first (topmost) skip entries.
- *
- * Returns the number of frames in the stack trace. Just like snprintf,
- * if the number of frames is greater than maxAddresses, it will return
- * the actual number of frames, so the stack trace was truncated iff
- * the return value > maxAddresses.
- *
- * Returns -1 on failure.
- */
-ssize_t getStackTrace(FrameInfo* addresses,
-                      size_t maxAddresses,
-                      size_t skip=0);
-
 /**
  * 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
  * on failure.
  */
 template <size_t N>
-bool getStackTrace(FrameArray<N>& fa, size_t skip=0) {
-  ssize_t n = getStackTrace(fa.frames, N, skip);
+bool getStackTrace(FrameArray<N>& fa) {
+  ssize_t n = getStackTrace(fa.addresses, N);
   if (n != -1) {
     fa.frameCount = n;
+    for (size_t i = 0; i < fa.frameCount; ++i) {
+      fa.frames[i].found = false;
+    }
     return true;
   } else {
     fa.frameCount = 0;
@@ -94,19 +80,21 @@ class Symbolizer {
   /**
    * Symbolize given addresses.
    */
-  void symbolize(FrameInfo* addresses, size_t addressCount);
+  void symbolize(const uintptr_t* addresses,
+                 SymbolizedFrame* frames,
+                 size_t frameCount);
 
   template <size_t N>
   void symbolize(FrameArray<N>& fa) {
-    symbolize(fa.frames, std::min(fa.frameCount, N));
+    symbolize(fa.addresses, fa.frames, fa.frameCount);
   }
 
   /**
    * Shortcut to symbolize one address.
    */
-  bool symbolize(FrameInfo& address) {
-    symbolize(&address, 1);
-    return address.found;
+  bool symbolize(uintptr_t address, SymbolizedFrame& frame) {
+    symbolize(&address, &frame, 1);
+    return frame.found;
   }
 
  private:
@@ -122,14 +110,16 @@ class Symbolizer {
  */
 class SymbolizePrinter {
  public:
-  void print(const FrameInfo& ainfo);
-  void print(const FrameInfo* addresses,
-             size_t addressesSize,
+  void print(uintptr_t address, const SymbolizedFrame& frame);
+  void print(const uintptr_t* addresses,
+             const SymbolizedFrame* frames,
              size_t frameCount);
 
   template <size_t N>
-  void print(const FrameArray<N>& fa) {
-    print(fa.frames, N, fa.frameCount);
+  void print(const FrameArray<N>& fa, size_t skip=0) {
+    if (skip < fa.frameCount) {
+      print(fa.addresses + skip, fa.frames + skip, fa.frameCount - skip);
+    }
   }
 
   virtual ~SymbolizePrinter() { }
@@ -161,14 +151,6 @@ class FDSymbolizePrinter : public SymbolizePrinter {
   int fd_;
 };
 
-/**
- * Print an FrameInfo to a stream. Note that the Symbolizer that
- * symbolized the address must outlive the FrameInfo. Just like
- * OStreamSymbolizePrinter (which it uses internally), this is not
- * reentrant; do not use from signal handling code.
- */
-std::ostream& operator<<(std::ostream& out, const FrameInfo& ainfo);
-
 }  // namespace symbolizer
 }  // namespace folly
 

folly/experimental/symbolizer/test/SymbolizerTest.cpp

@@ -27,10 +27,11 @@ void foo() {
 }
 
 TEST(Symbolizer, Single) {
-  FrameInfo a(reinterpret_cast<uintptr_t>(foo));
   Symbolizer symbolizer;
-  ASSERT_TRUE(symbolizer.symbolize(a));
-  EXPECT_EQ("folly::symbolizer::test::foo()", demangle(a.name.str().c_str()));
+  SymbolizedFrame a;
+  ASSERT_TRUE(symbolizer.symbolize(reinterpret_cast<uintptr_t>(foo), a));
+  EXPECT_EQ("folly::symbolizer::test::foo()",
+            demangle(a.name.str().c_str()));
 
   auto path = a.location.file.toString();
   folly::StringPiece basename(path);