Implement basic stack traces on Android and reenable unit tests.

base/debug/stack_trace_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/debug/stack_trace.h"
 
 #include <signal.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <unwind.h>  // TODO(dmikurube): Remove.  See http://crbug.com/236855.
 
-#include "base/logging.h"
+#include "base/stringprintf.h"
+#include "base/third_party/symbolize/symbolize.h"
 
 #ifdef __MIPSEL__
 // SIGSTKFLT is not defined for MIPS.
 #define SIGSTKFLT SIGSEGV
 #endif
 
 // TODO(dmikurube): Remove when Bionic's get_backtrace() gets popular.
 // See http://crbug.com/236855.
 namespace {
 
@@ skipping 66 matching lines @@
   // See http://crbug.com/236855.
   stack_crawl_state_t state(reinterpret_cast<uintptr_t*>(trace_), kMaxTraces);
   _Unwind_Backtrace(tracer, &state);
   count_ = state.frame_count;
   // TODO(dmikurube): Symbolize in Chrome.
 }
 
 // Sends fake SIGSTKFLT signals to let the Android linker and debuggerd dump
 // stack. See inlined comments and Android bionic/linker/debugger.c and
 // system/core/debuggerd/debuggerd.c for details.
+//
+// TODO(scherkus): This should probably use OutputToStream() and print to
+// stderr. Instead it crashes the program http://crbug.com/248775

[digit, 2013/06/13 08:17:25]

Small clarification here: outputting to stderr will only work if you run the
program as a standalone executable. Otherwise, everything will be lost. For
regular APKs (like most Chrome unittests on Android these day), you should
really send this to the log (with __android_log_print, for example).

An exception would be a unit tests that reads the stderr() output, but that
would probably a bad idea anyway.

It's possible to test for this condition by looking at the username with
getpwnam(), if it is "root" or "shell", the program is being run from the shell
and output can go to stderr. Otherwise, send it to the log.

 void StackTrace::PrintBacktrace() const {
   // Get the current handler of SIGSTKFLT for later use.
   sighandler_t sig_handler = signal(SIGSTKFLT, SIG_DFL);
   signal(SIGSTKFLT, sig_handler);
 
   // The Android linker will handle this signal and send a stack dumping request
   // to debuggerd which will ptrace_attach this process. Before returning from
   // the signal handler, the linker sets the signal handler to SIG_IGN.
   kill(gettid(), SIGSTKFLT);

[digit, 2013/06/13 08:17:25]

Note: I know you didn't modify that part, but I'm not sure doing this is such a
good idea. The implementation of the default signal handlers (setup by the
linker) and debuggerd is bound to change between platform releases (i.e.
certainly not stable). This code is likely to not work everywhere as expected,
which could result in flaky tests. :-(

[scherkus (not reviewing), 2013/06/13 17:35:05]

You know what? I'm going to rip out this code and put in much simpler logging of
the stack trace. I've had coworkers complain they've been bitten by the current
crashing behaviour a couple times (myself included!)

 
   // Because debuggerd will wait for the process to be stopped by the actual
   // signal in crashing scenarios, signal is sent again to met the expectation.
   // Debuggerd will dump stack into the system log and /data/tombstones/ files.
   // NOTE: If this process runs in the interactive shell, it will be put
   // in the background. To resume it in the foreground, use 'fg' command.
   kill(gettid(), SIGSTKFLT);
 
   // Restore the signal handler so that this method can work the next time.
   signal(SIGSTKFLT, sig_handler);

[digit, 2013/06/13 08:17:25]

Note: Oops, we should never use signal() to save/restore a signal handler. This
code should use sigaction() instead to ensure none of the sa_flags are lost
(SA_SIGINFO, more importantly).

 }
 
+// NOTE: Native libraries in APKs are stripped before installing. Print out the
+// relocatable address and library names so host computers can use tools to
+// symbolize and demangle (e.g., addr2line, c++filt).
 void StackTrace::OutputToStream(std::ostream* os) const {
-  NOTIMPLEMENTED();
+  for (size_t i = 0; i < count_; ++i) {
+    *os << base::StringPrintf("#%02d ", i);
+
+    // Subtract by one as return address of function may be in the next
+    // function when a function is annotated as noreturn.
+    void* address = static_cast<char*>(trace_[i]) - 1;
+
+    char path[1024];
+    uint64 start_address = 0;
+    if (google::FindObjectFileNameContainingPcAndGetStartAddress(
+            address, path, sizeof(path), &start_address)) {
+      uintptr_t rel_pc = reinterpret_cast<uintptr_t>(address) - start_address;
+      *os << base::StringPrintf("pc %08x %s", rel_pc, path);

[satorux1, 2013/06/13 03:34:25]

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=64-bit...

%08x -> %08"PRIx64"

who said printf was easier to read? :)

[digit, 2013/06/13 08:17:25]

nit: That would be very wrong, uintptr_t is always 32-bit on Android. Your
proposal would make base::StringPrintf generate garbage unless rel_pc was
explicitely cast to a int64_t.
Compare to what exactly ? :-)

[scherkus (not reviewing), 2013/06/13 17:35:05]

Open to ideas ... I tried using PRIxPTR but got silly compiler failures:
../../base/debug/stack_trace_android.cc:111:69: error: format '%lx' expects
argument of type 'long unsigned int', but argument 2 has type 'uintptr_t {aka
unsigned int}' [-Werror=format]

FWIW a similar warning would happen if we ever compiled with uintptr_t being
64-bit.

[satorux1, 2013/06/14 01:45:41]

Ah, I misunderstood that we were handling uint64 here. Using %08x sounds good if
it's guaranteed to be always 32-bit. Adding a comment about it would be nice.
streams.

+    } else {
+      *os << "<unknown>";

[digit, 2013/06/13 08:17:25]

I'd suggest printing the address value as well, just in case this makes
diagnosing this kind of situation easier. Note that this will happen for
renderer processes on JB 4.1 and higher, since they run in a additional
sandboxed mode that prevent them from accessing /proc.

[scherkus (not reviewing), 2013/06/13 17:35:05]

Done.

+    }
+
+    *os << "\n";
+  }
 }
 
 }  // namespace debug
 }  // namespace base