GTTF: Make Linux stack dump signal handler async-signal safe.

base/debug/stack_trace_posix.cc

Index: base/debug/stack_trace_posix.cc
diff --git a/base/debug/stack_trace_posix.cc b/base/debug/stack_trace_posix.cc
index ff756fc0eece78622d703162432726d517b198fa..74111b950a324a918c59466657765edad06c667b 100644
--- a/base/debug/stack_trace_posix.cc
+++ b/base/debug/stack_trace_posix.cc
@@ -15,24 +15,13 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include <string>
-#include <vector>
-
-#if defined(__GLIBCXX__)
-#include <cxxabi.h>
-#endif
-
-#if defined(OS_MACOSX)
-#include <AvailabilityMacros.h>
-#endif
+#include <ostream>
 
 #include "base/basictypes.h"
+#include "base/debug/debugger.h"
 #include "base/eintr_wrapper.h"
 #include "base/logging.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/safe_strerror_posix.h"
-#include "base/string_piece.h"
-#include "base/stringprintf.h"
+#include "base/string_number_conversions.h"
 
 #if defined(USE_SYMBOLIZE)
 #include "base/third_party/symbolize/symbolize.h"
@@ -43,124 +32,142 @@ namespace debug {
 
 namespace {
 
-// The prefix used for mangled symbols, per the Itanium C++ ABI:
-// http://www.codesourcery.com/cxx-abi/abi.html#mangling
-const char kMangledSymbolPrefix[] = "_Z";
+class BacktraceOutputHandler {
+ public:
+  virtual void HandleOutput(const char* output) = 0;
 
-// Characters that can be used for symbols, generated by Ruby:
-// (('a'..'z').to_a+('A'..'Z').to_a+('0'..'9').to_a + ['_']).join
-const char kSymbolCharacters[] =
-    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_";
+ protected:
+  virtual ~BacktraceOutputHandler() {}
+};
 
-#if !defined(USE_SYMBOLIZE)
-// Demangles C++ symbols in the given text. Example:
+// POSIX doesn't define any async-signal safe function for converting
+// an integer to ASCII. We'll have to define our own version.
+// itoa_r() converts a (signed) integer to ASCII. It returns "buf", if the
+// conversion was successful or NULL otherwise. It never writes more than "sz"
+// bytes. Output will be truncated as needed, and a NUL character is always
+// appended.
 //
-// "out/Debug/base_unittests(_ZN10StackTraceC1Ev+0x20) [0x817778c]"
-// =>
-// "out/Debug/base_unittests(StackTrace::StackTrace()+0x20) [0x817778c]"
-void DemangleSymbols(std::string* text) {
-#if defined(__GLIBCXX__)
-
-  std::string::size_type search_from = 0;
-  while (search_from < text->size()) {
-    // Look for the start of a mangled symbol, from search_from.
-    std::string::size_type mangled_start =
-        text->find(kMangledSymbolPrefix, search_from);
-    if (mangled_start == std::string::npos) {
-      break;  // Mangled symbol not found.
+// NOTE: code from sandbox/linux/seccomp-bpf/demo.cc.
+static char *itoa_r(intptr_t i, char *buf, size_t sz, int base) {
+  // Make sure we can write at least one NUL byte.
+  size_t n = 1;
+  if (n > sz) {
+    return NULL;
+  }
+
+  char *start = buf;
+  int minint = 0;
+
+  if (base == 10) {
+    // Handle negative numbers.
+    if (i < 0) {
+      // Make sure we can write the '-' character.
+      if (++n > sz) {
+        *start = '\000';
+        return NULL;
+      }
+      *start++ = '-';
+
+      // Turn our number positive.
+      if (i == -i) {
+        // The lowest-most negative integer needs special treatment.
+        minint = 1;
+        i = -(i + 1);
+      } else {
+        // "Normal" negative numbers are easy.
+        i = -i;

[jar (doing other things), 2012/11/06 20:40:03]

You don't actually need the minint game, now that you convert it to unsigned in
line 83.

In the weird/problematic case where we are exactly something like:
10000000000B
This is indeed the most negative number...  and it is the case that when we
negate it (take complement, and add one) we do indeed get back the same negative
number.  The good news is that this number *is* is range when we convert it to
unsigned, and hence we don't need to subtract one here, and then add one on line
96.

The code just works ;-).

The trick is that the range of unsigned (on the positive end) is twice as large
as signed, so we have no trouble representing the maximally negative number in
an unsigned variable.

+      }
     }
+  }
+
+  uintptr_t j = i;
 
-    // Look for the end of the mangled symbol.
-    std::string::size_type mangled_end =
-        text->find_first_not_of(kSymbolCharacters, mangled_start);
-    if (mangled_end == std::string::npos) {
-      mangled_end = text->size();
+  // Loop until we have converted the entire number. Output at least one
+  // character (i.e. '0').
+  char *ptr = start;
+  do {
+    // Make sure there is still enough space left in our output buffer.
+    if (++n > sz) {
+      buf = NULL;
+      break;
     }
-    std::string mangled_symbol =
-        text->substr(mangled_start, mangled_end - mangled_start);
-
-    // Try to demangle the mangled symbol candidate.
-    int status = 0;
-    scoped_ptr_malloc<char> demangled_symbol(
-        abi::__cxa_demangle(mangled_symbol.c_str(), NULL, 0, &status));
-    if (status == 0) {  // Demangling is successful.
-      // Remove the mangled symbol.
-      text->erase(mangled_start, mangled_end - mangled_start);
-      // Insert the demangled symbol.
-      text->insert(mangled_start, demangled_symbol.get());
-      // Next time, we'll start right after the demangled symbol we inserted.
-      search_from = mangled_start + strlen(demangled_symbol.get());
-    } else {
-      // Failed to demangle.  Retry after the "_Z" we just found.
-      search_from = mangled_start + 2;
+
+    // Output the next digit.
+    *ptr = "0123456789abcdef"[j % base] + minint;
+    if (base == 10) {

[jar (doing other things), 2012/11/06 20:40:03]

nit: Don't bother to test here.  Minint will only be set to 1 if there was a
problem, and it was base 10.  The old code on lines 94 was fine (with j instead
of i now).

+      // If necessary compensate for the lowest-most negative integer needing
+      // special treatment. This works because, no matter the bit width of the
+      // integer, the lowest-most integer always ends in 2, 4, 6, or 8.
+      *ptr += minint;
     }
+    ptr++;
+    minint = 0;
+    j /= base;
+  } while (j);
+
+  // Terminate the output with a NUL character.
+  *ptr = '\000';
+
+  // Conversion to ASCII actually resulted in the digits being in reverse
+  // order. We can't easily generate them in forward order, as we can't tell
+  // the number of characters needed until we are done converting.
+  // So, now, we reverse the string (except for the possible "-" sign).
+  while (--ptr > start) {
+    char ch = *ptr;
+    *ptr = *start;
+    *start++ = ch;
   }
-
-#endif  // defined(__GLIBCXX__)
+  return buf;
 }
-#endif  // !defined(USE_SYMBOLIZE)
 
-// Gets the backtrace as a vector of strings. If possible, resolve symbol
-// names and attach these. Otherwise just use raw addresses. Returns true
-// if any symbol name is resolved.  Returns false on error and *may* fill
-// in |error_message| if an error message is available.
-bool GetBacktraceStrings(void *const *trace, int size,
-                         std::vector<std::string>* trace_strings,
-                         std::string* error_message) {
-  bool symbolized = false;
+void ProcessBacktrace(void *const *trace,
+                      int size,
+                      BacktraceOutputHandler* handler) {
+  // NOTE: This code MUST be async-signal safe (it's used by in-process
+  // stack dumping signal handler). NO malloc or stdio is allowed here.
 
-#if defined(USE_SYMBOLIZE)
   for (int i = 0; i < size; ++i) {
-    char symbol[1024];
+    handler->HandleOutput("\t");
+
+    char buf[1024] = { '\0' };
+
+#if defined(USE_SYMBOLIZE)
     // Subtract by one as return address of function may be in the next
     // function when a function is annotated as noreturn.
-    if (google::Symbolize(static_cast<char *>(trace[i]) - 1,
-                          symbol, sizeof(symbol))) {
-      // Don't call DemangleSymbols() here as the symbol is demangled by
-      // google::Symbolize().
-      trace_strings->push_back(
-          base::StringPrintf("%s [%p]", symbol, trace[i]));
-      symbolized = true;
-    } else {
-      trace_strings->push_back(base::StringPrintf("%p", trace[i]));
-    }
-  }
-#else
-  scoped_ptr_malloc<char*> trace_symbols(backtrace_symbols(trace, size));
-  if (trace_symbols.get()) {
-    for (int i = 0; i < size; ++i) {
-      std::string trace_symbol = trace_symbols.get()[i];
-      DemangleSymbols(&trace_symbol);
-      trace_strings->push_back(trace_symbol);
-    }
-    symbolized = true;
-  } else {
-    if (error_message)
-      *error_message = safe_strerror(errno);
-    for (int i = 0; i < size; ++i) {
-      trace_strings->push_back(base::StringPrintf("%p", trace[i]));
-    }
-  }
+    void* address = static_cast<char*>(trace[i]) - 1;
+    if (google::Symbolize(address, buf, sizeof(buf)))
+      handler->HandleOutput(buf);
+    else
+      handler->HandleOutput("<unknown>");
+
+    handler->HandleOutput(" ");
 #endif  // defined(USE_SYMBOLIZE)
 
-  return symbolized;
+    handler->HandleOutput("[0x");
+    itoa_r(reinterpret_cast<intptr_t>(trace[i]), buf, sizeof(buf), 16);
+    handler->HandleOutput(buf);
+    handler->HandleOutput("]\n");
+  }
 }
 
 void StackDumpSignalHandler(int signal, siginfo_t* info, ucontext_t* context) {
+  // NOTE: This code MUST be async-signal safe.
+  // NO malloc or stdio is allowed here.
+
   if (BeingDebugged())
     BreakDebugger();
 
-#if defined(OS_MACOSX)
-  // TODO(phajdan.jr): Fix async-signal non-safety (http://crbug.com/101155).
-  DLOG(ERROR) << "Received signal " << signal;
-  StackTrace().PrintBacktrace();
-#endif
+  char buf[1024] = { '\0' };
+  strncat(buf, "Received signal ", sizeof(buf) - 1);
+  itoa_r(signal, buf + strlen(buf), sizeof(buf) - strlen(buf), 10);
+  RAW_LOG(ERROR, buf);
+
+  debug::StackTrace().PrintBacktrace();
 
   // TODO(shess): Port to Linux.
 #if defined(OS_MACOSX)
   // TODO(shess): Port to 64-bit.
 #if ARCH_CPU_X86_FAMILY && ARCH_CPU_32_BITS
-  char buf[1024];
   size_t len;
 
   // NOTE: Even |snprintf()| is not on the approved list for signal
@@ -201,6 +208,68 @@ void StackDumpSignalHandler(int signal, siginfo_t* info, ucontext_t* context) {
   _exit(1);
 }
 
+class PrintBacktraceOutputHandler : public BacktraceOutputHandler {
+ public:
+  PrintBacktraceOutputHandler() {}
+
+  virtual void HandleOutput(const char* output) {
+    // NOTE: This code MUST be async-signal safe (it's used by in-process
+    // stack dumping signal handler). NO malloc or stdio is allowed here.
+    HANDLE_EINTR(write(STDERR_FILENO, output, strlen(output)));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PrintBacktraceOutputHandler);
+};
+
+class StreamBacktraceOutputHandler : public BacktraceOutputHandler {
+ public:
+  StreamBacktraceOutputHandler(std::ostream* os) : os_(os) {
+  }
+
+  virtual void HandleOutput(const char* output) {
+    (*os_) << output;
+  }
+
+ private:
+  std::ostream* os_;
+
+  DISALLOW_COPY_AND_ASSIGN(StreamBacktraceOutputHandler);
+};
+
+void WarmUpBacktrace() {
+  // Warm up stack trace infrastructure. It turns out that on the first
+  // call glibc initializes some internal data structures using pthread_once,
+  // and even backtrace() can call malloc(), leading to hangs.
+  //
+  // Example stack trace snippet (with tcmalloc):
+  //
+  // #8  0x0000000000a173b5 in tc_malloc
+  //             at ./third_party/tcmalloc/chromium/src/debugallocation.cc:1161
+  // #9  0x00007ffff7de7900 in _dl_map_object_deps at dl-deps.c:517
+  // #10 0x00007ffff7ded8a9 in dl_open_worker at dl-open.c:262
+  // #11 0x00007ffff7de9176 in _dl_catch_error at dl-error.c:178
+  // #12 0x00007ffff7ded31a in _dl_open (file=0x7ffff625e298 "libgcc_s.so.1")
+  //             at dl-open.c:639
+  // #13 0x00007ffff6215602 in do_dlopen at dl-libc.c:89
+  // #14 0x00007ffff7de9176 in _dl_catch_error at dl-error.c:178
+  // #15 0x00007ffff62156c4 in dlerror_run at dl-libc.c:48
+  // #16 __GI___libc_dlopen_mode at dl-libc.c:165
+  // #17 0x00007ffff61ef8f5 in init
+  //             at ../sysdeps/x86_64/../ia64/backtrace.c:53
+  // #18 0x00007ffff6aad400 in pthread_once
+  //             at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_once.S:104
+  // #19 0x00007ffff61efa14 in __GI___backtrace
+  //             at ../sysdeps/x86_64/../ia64/backtrace.c:104
+  // #20 0x0000000000752a54 in base::debug::StackTrace::StackTrace
+  //             at base/debug/stack_trace_posix.cc:175
+  // #21 0x00000000007a4ae5 in
+  //             base::(anonymous namespace)::StackDumpSignalHandler
+  //             at base/process_util_posix.cc:172
+  // #22 <signal handler called>
+  StackTrace stack_trace;
+}
+
 }  // namespace
 
 #if !defined(OS_IOS)
@@ -214,6 +283,9 @@ bool EnableInProcessStackDumping() {
   sigemptyset(&action.sa_mask);
   bool success = (sigaction(SIGPIPE, &action, NULL) == 0);
 
+  // Avoid hangs during backtrace initialization, see above.
+  WarmUpBacktrace();
+
   sig_t handler = reinterpret_cast<sig_t>(&StackDumpSignalHandler);
   success &= (signal(SIGILL, handler) != SIG_ERR);
   success &= (signal(SIGABRT, handler) != SIG_ERR);
@@ -227,35 +299,25 @@ bool EnableInProcessStackDumping() {
 #endif  // !defined(OS_IOS)
 
 StackTrace::StackTrace() {
+  // NOTE: This code MUST be async-signal safe (it's used by in-process
+  // stack dumping signal handler). NO malloc or stdio is allowed here.
+
   // Though the backtrace API man page does not list any possible negative
   // return values, we take no chance.
   count_ = std::max(backtrace(trace_, arraysize(trace_)), 0);
 }
 
 void StackTrace::PrintBacktrace() const {
-  fflush(stderr);
-  std::vector<std::string> trace_strings;
-  GetBacktraceStrings(trace_, count_, &trace_strings, NULL);
-  for (size_t i = 0; i < trace_strings.size(); ++i) {
-    fprintf(stderr, "\t%s\n", trace_strings[i].c_str());
-  }
+  // NOTE: This code MUST be async-signal safe (it's used by in-process
+  // stack dumping signal handler). NO malloc or stdio is allowed here.
+
+  PrintBacktraceOutputHandler handler;
+  ProcessBacktrace(trace_, count_, &handler);
 }
 
 void StackTrace::OutputToStream(std::ostream* os) const {
-  std::vector<std::string> trace_strings;
-  std::string error_message;
-  if (GetBacktraceStrings(trace_, count_, &trace_strings, &error_message)) {
-    (*os) << "Backtrace:\n";
-  } else {
-    if (!error_message.empty())
-      error_message = " (" + error_message + ")";
-    (*os) << "Unable to get symbols for backtrace" << error_message << ". "
-          << "Dumping raw addresses in trace:\n";
-  }
-
-  for (size_t i = 0; i < trace_strings.size(); ++i) {
-    (*os) << "\t" << trace_strings[i] << "\n";
-  }
+  StreamBacktraceOutputHandler handler(os);
+  ProcessBacktrace(trace_, count_, &handler);
 }
 
 }  // namespace debug