unw_backtrace is not async-signal-safe
authorTudor Bosman <tudorb@fb.com>
Tue, 17 Dec 2013 00:57:31 +0000 (16:57 -0800)
committerJordan DeLong <jdelong@fb.com>
Fri, 20 Dec 2013 21:07:40 +0000 (13:07 -0800)
Summary:
Even though it should be according to the docs; tdep_trace allocates memory on
first use from each thread.

Wrote a slow loop that we can use from the signal handler. The exception tracer
still uses the fast version.

Test Plan: fbconfig -r folly/experimental/symbolizer folly/experimental/exception_tracer && fbmake runtests_opt

Reviewed By: philipp@fb.com

FB internal diff: D1101095

folly/experimental/symbolizer/SignalHandler.cpp
folly/experimental/symbolizer/StackTrace.cpp
folly/experimental/symbolizer/StackTrace.h
folly/experimental/symbolizer/Symbolizer.h
folly/experimental/symbolizer/test/StackTraceTest.cpp [new file with mode: 0644]

index 8346e79..545380f 100644 (file)
@@ -195,7 +195,7 @@ void dumpStackTrace() {
   FrameArray<kMaxStackTraceDepth> addresses;
 
   // Skip the getStackTrace frame
-  if (!getStackTrace(addresses)) {
+  if (!getStackTraceSafe(addresses)) {
     print("(error retrieving stack trace)\n");
   } else {
     Symbolizer symbolizer;
index b6664bd..91f23e7 100644 (file)
@@ -25,9 +25,61 @@ 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");
+  // The libunwind documentation says that unw_backtrace is async-signal-safe
+  // but, as of libunwind 1.0.1, it isn't (tdep_trace allocates memory on
+  // x86_64)
   int r = unw_backtrace(reinterpret_cast<void**>(addresses), maxAddresses);
   return r < 0 ? -1 : r;
 }
 
-}}  // namespaces
+namespace {
+inline bool getFrameInfo(unw_cursor_t* cursor, uintptr_t& ip) {
+  unw_word_t uip;
+  if (unw_get_reg(cursor, UNW_REG_IP, &uip) < 0) {
+    return false;
+  }
+  int r = unw_is_signal_frame(cursor);
+  if (r < 0) {
+    return false;
+  }
+  // Use previous instruction in normal (call) frames (because the
+  // return address might not be in the same function for noreturn functions)
+  // but not in signal frames.
+  ip = uip - (r == 0);
+  return true;
+}
+}  // namespace
 
+ssize_t getStackTraceSafe(uintptr_t* addresses, size_t maxAddresses) {
+  if (maxAddresses == 0) {
+    return 0;
+  }
+  unw_context_t context;
+  if (unw_getcontext(&context) < 0) {
+    return -1;
+  }
+  unw_cursor_t cursor;
+  if (unw_init_local(&cursor, &context) < 0) {
+    return -1;
+  }
+  if (!getFrameInfo(&cursor, *addresses)) {
+    return -1;
+  }
+  ++addresses;
+  ssize_t count = 1;
+  for (; count != maxAddresses; ++count, ++addresses) {
+    int r = unw_step(&cursor);
+    if (r < 0) {
+      return -1;
+    }
+    if (r == 0) {
+      break;
+    }
+    if (!getFrameInfo(&cursor, *addresses)) {
+      return -1;
+    }
+  }
+  return count;
+}
+
+}}  // namespaces
index 3f18f5e..665adf8 100644 (file)
@@ -17,8 +17,8 @@
 #ifndef FOLLY_SYMBOLIZER_STACKTRACE_H_
 #define FOLLY_SYMBOLIZER_STACKTRACE_H_
 
-#include <cstddef>
 #include <cstdint>
+#include <cstdlib>
 
 namespace folly { namespace symbolizer {
 
@@ -28,9 +28,22 @@ namespace folly { namespace symbolizer {
  *
  * Returns the number of frames written in the array.
  * Returns -1 on failure.
+ *
+ * NOT async-signal-safe, but fast.
  */
 ssize_t getStackTrace(uintptr_t* addresses, size_t maxAddresses);
 
+/**
+ * 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.
+ *
+ * Async-signal-safe, but likely slower.
+ */
+ssize_t getStackTraceSafe(uintptr_t* addresses, size_t maxAddresses);
+
 }}  // namespaces
 
 #endif /* FOLLY_SYMBOLIZER_STACKTRACE_H_ */
index bef46f3..0265549 100644 (file)
@@ -59,9 +59,9 @@ struct FrameArray {
  * set frameCount to the actual frame count, which may be > N) and false
  * on failure.
  */
+namespace detail {
 template <size_t N>
-bool getStackTrace(FrameArray<N>& fa) {
-  ssize_t n = getStackTrace(fa.addresses, N);
+bool fixFrameArray(FrameArray<N>& fa, ssize_t n) {
   if (n != -1) {
     fa.frameCount = n;
     for (size_t i = 0; i < fa.frameCount; ++i) {
@@ -73,6 +73,17 @@ bool getStackTrace(FrameArray<N>& fa) {
     return false;
   }
 }
+}  // namespace detail
+
+template <size_t N>
+bool getStackTrace(FrameArray<N>& fa) {
+  return detail::fixFrameArray(fa, getStackTrace(fa.addresses, N));
+}
+
+template <size_t N>
+bool getStackTraceSafe(FrameArray<N>& fa) {
+  return detail::fixFrameArray(fa, getStackTraceSafe(fa.addresses, N));
+}
 
 class Symbolizer {
  public:
diff --git a/folly/experimental/symbolizer/test/StackTraceTest.cpp b/folly/experimental/symbolizer/test/StackTraceTest.cpp
new file mode 100644 (file)
index 0000000..730e68e
--- /dev/null
@@ -0,0 +1,83 @@
+/*
+ * 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/symbolizer/StackTrace.h"
+#include "folly/experimental/symbolizer/Symbolizer.h"
+
+#include <glog/logging.h>
+#include <gtest/gtest.h>
+
+using namespace folly;
+using namespace folly::symbolizer;
+
+void foo1() __attribute__((noinline));
+void foo2() __attribute__((noinline));
+
+void verifyStackTraces() {
+  constexpr size_t kMaxAddresses = 100;
+  FrameArray<kMaxAddresses> fa;
+  CHECK(getStackTrace(fa));
+
+  FrameArray<kMaxAddresses> faSafe;
+  CHECK(getStackTraceSafe(faSafe));
+
+  CHECK_EQ(fa.frameCount, faSafe.frameCount);
+
+  // Other than the top 2 frames (this one and getStackTrace /
+  // getStackTraceSafe), the stack traces should be identical
+  for (size_t i = 2; i < fa.frameCount; ++i) {
+    VLOG(1) << "i=" << i << " " << std::hex << fa.addresses[i] << " "
+            << faSafe.addresses[i];
+    CHECK_EQ(fa.addresses[i], faSafe.addresses[i]);
+  }
+}
+
+void foo1() {
+  foo2();
+}
+
+void foo2() {
+  verifyStackTraces();
+}
+
+volatile bool handled = false;
+void handler(int num, siginfo_t* info, void* ctx) {
+  // Yes, getStackTrace and VLOG aren't async-signal-safe, but signals
+  // raised with raise() aren't "async" signals.
+  foo1();
+  handled = true;
+}
+
+TEST(StackTraceTest, Simple) {
+  foo1();
+}
+
+TEST(StackTraceTest, Signal) {
+  struct sigaction sa;
+  memset(&sa, 0, sizeof(sa));
+  sa.sa_sigaction = handler;
+  sa.sa_flags = SA_RESETHAND | SA_SIGINFO;
+  CHECK_ERR(sigaction(SIGUSR1, &sa, nullptr));
+  raise(SIGUSR1);
+  CHECK(handled);
+}
+
+int main(int argc, char *argv[]) {
+  testing::InitGoogleTest(&argc, argv);
+  google::ParseCommandLineFlags(&argc, &argv, true);
+  return RUN_ALL_TESTS();
+}
+