From 1ceb5edc1e7383f928252f88c2ab45e3662e9978 Mon Sep 17 00:00:00 2001 From: Tudor Bosman Date: Mon, 16 Dec 2013 16:57:31 -0800 Subject: [PATCH] unw_backtrace is not async-signal-safe 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 --- .../experimental/symbolizer/SignalHandler.cpp | 2 +- folly/experimental/symbolizer/StackTrace.cpp | 54 +++++++++++- folly/experimental/symbolizer/StackTrace.h | 15 +++- folly/experimental/symbolizer/Symbolizer.h | 15 +++- .../symbolizer/test/StackTraceTest.cpp | 83 +++++++++++++++++++ 5 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 folly/experimental/symbolizer/test/StackTraceTest.cpp diff --git a/folly/experimental/symbolizer/SignalHandler.cpp b/folly/experimental/symbolizer/SignalHandler.cpp index 8346e79b..545380f7 100644 --- a/folly/experimental/symbolizer/SignalHandler.cpp +++ b/folly/experimental/symbolizer/SignalHandler.cpp @@ -195,7 +195,7 @@ void dumpStackTrace() { FrameArray addresses; // Skip the getStackTrace frame - if (!getStackTrace(addresses)) { + if (!getStackTraceSafe(addresses)) { print("(error retrieving stack trace)\n"); } else { Symbolizer symbolizer; diff --git a/folly/experimental/symbolizer/StackTrace.cpp b/folly/experimental/symbolizer/StackTrace.cpp index b6664bd8..91f23e76 100644 --- a/folly/experimental/symbolizer/StackTrace.cpp +++ b/folly/experimental/symbolizer/StackTrace.cpp @@ -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(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 diff --git a/folly/experimental/symbolizer/StackTrace.h b/folly/experimental/symbolizer/StackTrace.h index 3f18f5e5..665adf86 100644 --- a/folly/experimental/symbolizer/StackTrace.h +++ b/folly/experimental/symbolizer/StackTrace.h @@ -17,8 +17,8 @@ #ifndef FOLLY_SYMBOLIZER_STACKTRACE_H_ #define FOLLY_SYMBOLIZER_STACKTRACE_H_ -#include #include +#include 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_ */ diff --git a/folly/experimental/symbolizer/Symbolizer.h b/folly/experimental/symbolizer/Symbolizer.h index bef46f3b..02655492 100644 --- a/folly/experimental/symbolizer/Symbolizer.h +++ b/folly/experimental/symbolizer/Symbolizer.h @@ -59,9 +59,9 @@ struct FrameArray { * set frameCount to the actual frame count, which may be > N) and false * on failure. */ +namespace detail { template -bool getStackTrace(FrameArray& fa) { - ssize_t n = getStackTrace(fa.addresses, N); +bool fixFrameArray(FrameArray& 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& fa) { return false; } } +} // namespace detail + +template +bool getStackTrace(FrameArray& fa) { + return detail::fixFrameArray(fa, getStackTrace(fa.addresses, N)); +} + +template +bool getStackTraceSafe(FrameArray& 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 index 00000000..730e68ea --- /dev/null +++ b/folly/experimental/symbolizer/test/StackTraceTest.cpp @@ -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 +#include + +using namespace folly; +using namespace folly::symbolizer; + +void foo1() __attribute__((noinline)); +void foo2() __attribute__((noinline)); + +void verifyStackTraces() { + constexpr size_t kMaxAddresses = 100; + FrameArray fa; + CHECK(getStackTrace(fa)); + + FrameArray 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(); +} + -- 2.34.1