Harden failure signal handler in the face of memory corruptions

Summary:
@mhorowitz got

*** Aborted at 1393267847 (Unix time, try 'date -d @1393267847') ***
*** Signal 11 (SIGSEGV) (0x0) received by PID 12652 (TID 0x7f59cbfff700), stack trace: ***
pure virtual method called
terminate called without an active exception
Entered fatal signal handler recursively. We're in trouble.

in a test, and no stack trace.

The first time we enter recursively (ie. the second time overall), try to dump
without symbolization. The next time, give up.

Test Plan: folly/experimental/symbolizer/test, ran crash with a modified dumpStackTrace to force it to enter recursively

Reviewed By: lucian@fb.com

FB internal diff: D1187942

diff --git a/folly/experimental/symbolizer/SignalHandler.cpp b/folly/experimental/symbolizer/SignalHandler.cpp
index 5c3d56d9cf5f84a23e98179d1f893b16a1409980..02e5ef05219c58fa43d6c7fb3f8c81f40c63fa47 100644 (file)
--- a/folly/experimental/symbolizer/SignalHandler.cpp
+++ b/folly/experimental/symbolizer/SignalHandler.cpp
@@ -196,9 +196,9 @@ constexpr size_t kDefaultCapacity = 500;
 SignalSafeElfCache signalSafeElfCache(kDefaultCapacity);
 }  // namespace
 
-void dumpStackTrace() __attribute__((noinline));
+void dumpStackTrace(bool symbolize) __attribute__((noinline));
 
-void dumpStackTrace() {
+void dumpStackTrace(bool symbolize) {
   SCOPE_EXIT { fsyncNoInt(STDERR_FILENO); };
   // Get and symbolize stack trace
   constexpr size_t kMaxStackTraceDepth = 100;
@@ -207,7 +207,7 @@ void dumpStackTrace() {
   // Skip the getStackTrace frame
   if (!getStackTraceSafe(addresses)) {
     print("(error retrieving stack trace)\n");
-  } else {
+  } else if (symbolize) {
     Symbolizer symbolizer(&signalSafeElfCache);
     symbolizer.symbolize(addresses);
 
@@ -219,6 +219,13 @@ void dumpStackTrace() {
     //
     // Leaving signalHandler on the stack for clarity, I think.
     printer.println(addresses, 2);
+  } else {
+    print("(safe mode, symbolizer not available)\n");
+    AddressFormatter formatter;
+    for (ssize_t i = 0; i < addresses.frameCount; ++i) {
+      print(formatter.format(addresses.addresses[i]));
+      print("\n");
+    }
   }
 }
 
@@ -229,6 +236,7 @@ void dumpStackTrace() {
 constexpr pthread_t kInvalidThreadId = 0;
 
 std::atomic<pthread_t> gSignalThread(kInvalidThreadId);
+std::atomic<bool> gInRecursiveSignalHandler(false);
 
 // Here be dragons.
 void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
@@ -238,7 +246,13 @@ void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
   pthread_t prevSignalThread = kInvalidThreadId;
   while (!gSignalThread.compare_exchange_strong(prevSignalThread, myId)) {
     if (pthread_equal(prevSignalThread, myId)) {
-      print("Entered fatal signal handler recursively. We're in trouble.\n");
+      // First time here. Try to dump the stack trace without symbolization.
+      // If we still fail, well, we're mightily screwed, so we do nothing the
+      // next time around.
+      if (!gInRecursiveSignalHandler.exchange(true)) {
+        print("Entered fatal signal handler recursively. We're in trouble.\n");
+        dumpStackTrace(false);  // no symbolization
+      }
       return;
     }
 
@@ -253,7 +267,7 @@ void innerSignalHandler(int signum, siginfo_t* info, void* uctx) {
 
   dumpTimeInfo();
   dumpSignalInfo(signum, info);
-  dumpStackTrace();
+  dumpStackTrace(true);  // with symbolization
 
   // Run user callbacks
   gFatalSignalCallbackRegistry->run();

diff --git a/folly/experimental/symbolizer/Symbolizer.cpp b/folly/experimental/symbolizer/Symbolizer.cpp
index 2498b7d9f026226a7bdc88eb203839a6fdef30cc..e2e090adfa11b31ccb71f2320c162c103fec4cf9 100644 (file)
--- a/folly/experimental/symbolizer/Symbolizer.cpp
+++ b/folly/experimental/symbolizer/Symbolizer.cpp
@@ -260,6 +260,26 @@ const SymbolizePrinter::Color kFunctionColor = SymbolizePrinter::Color::PURPLE;
 const SymbolizePrinter::Color kFileColor = SymbolizePrinter::Color::DEFAULT;
 }  // namespace
 
+constexpr char AddressFormatter::bufTemplate[];
+
+AddressFormatter::AddressFormatter() {
+  memcpy(buf_, bufTemplate, sizeof(buf_));
+}
+
+folly::StringPiece AddressFormatter::format(uintptr_t address) {
+  // Can't use sprintf, not async-signal-safe
+  static_assert(sizeof(uintptr_t) <= 8, "huge uintptr_t?");
+  char* end = buf_ + sizeof(buf_) - 1 - (16 - 2 * sizeof(uintptr_t));
+  char* p = end;
+  *p-- = '\0';
+  while (address != 0) {
+    *p-- = kHexChars[address & 0xf];
+    address >>= 4;
+  }
+
+  return folly::StringPiece(buf_, end);
+}
+
 void SymbolizePrinter::print(uintptr_t address, const SymbolizedFrame& frame) {
   if (options_ & TERSE) {
     printTerse(address, frame);
@@ -269,20 +289,13 @@ void SymbolizePrinter::print(uintptr_t address, const SymbolizedFrame& frame) {
   SCOPE_EXIT { color(Color::DEFAULT); };
 
   color(kAddressColor);
-  // Can't use sprintf, not async-signal-safe
-  static_assert(sizeof(uintptr_t) <= 8, "huge uintptr_t?");
-  char buf[] = "    @ 0000000000000000";
-  char* end = buf + sizeof(buf) - 1 - (16 - 2 * sizeof(uintptr_t));
+
+  AddressFormatter formatter;
+  doPrint(formatter.format(address));
+
   const char padBuf[] = "                       ";
   folly::StringPiece pad(padBuf,
                          sizeof(padBuf) - 1 - (16 - 2 * sizeof(uintptr_t)));
-  char* p = end;
-  *p-- = '\0';
-  while (address != 0) {
-    *p-- = kHexChars[address & 0xf];
-    address >>= 4;
-  }
-  doPrint(folly::StringPiece(buf, end));
 
   color(kFunctionColor);
   char mangledBuf[1024];

diff --git a/folly/experimental/symbolizer/Symbolizer.h b/folly/experimental/symbolizer/Symbolizer.h
index 5595a53b67994270c9588de1561899ee7b41db1f..5a87f90b6b689c9364e3e82ff0c0467468760f87 100644 (file)
--- a/folly/experimental/symbolizer/Symbolizer.h
+++ b/folly/experimental/symbolizer/Symbolizer.h
@@ -128,6 +128,24 @@ class Symbolizer {
   ElfCacheBase* cache_;
 };
 
+/**
+ * Format one address in the way it's usually printer by SymbolizePrinter.
+ * Async-signal-safe.
+ */
+class AddressFormatter {
+ public:
+  AddressFormatter();
+
+  /**
+   * Format the address. Returns an internal buffer.
+   */
+  StringPiece format(uintptr_t address);
+
+ private:
+  static constexpr char bufTemplate[] = "    @ 0000000000000000";
+  char buf_[sizeof(bufTemplate)];
+};
+
 /**
  * Print a list of symbolized addresses. Base class.
  */