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 <iomanip>
 
-#include "base/logging.h"
+#include "base/third_party/symbolize/demangle.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.
 void StackTrace::PrintBacktrace() const {

[satorux1, 2013/06/12 05:43:42]

PrintBacktrace() and OutputToStream() will behave very differently. Is it a good
thing? In strack_trace_posix.cc, the two functions behave consistently.

[scherkus (not reviewing), 2013/06/13 01:27:44]

I noticed this too -- I filed a bug and am trying to track down the owner

   // 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);
 
   // 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);
 }
 
+static int OnSymbolizeCallback(int fd,
+                               void* pc,
+                               char* out,
+                               size_t out_size,
+                               uint64 relocation) {
+  // Maximum string length of an int is 10 characters.
+  char path[32];
+  int size = snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+  if (size < 0 || static_cast<size_t>(size) >= sizeof(path))
+    return -1;
+
+  char buf[1024];
+  size = readlink(path, buf, sizeof(buf));

[scherkus (not reviewing), 2013/06/12 00:43:43]

satorux: this is also kind of unnecessary since symbolize has the filename -- it
would be nice to simply get that bit of information :)

[satorux1, 2013/06/12 05:43:42]

I think it's easy to change symbolize.cc to provide the file name to this
callback. I think it's fine to make local modifications like that.

+  if (size < 0 || static_cast<size_t>(size) >= sizeof(buf))
+    return -1;
+  buf[size] = '\0';  // readlink() doesn't null-terminate strings.
+
+  uintptr_t rel_pc = reinterpret_cast<uintptr_t>(pc) - relocation;
+  size = snprintf(out, out_size, "pc %08x %s ", rel_pc, buf);
+  return size;
+}
+
 void StackTrace::OutputToStream(std::ostream* os) const {
-  NOTIMPLEMENTED();
+  // TODO(scherkus): HACK - is this thread/async safe and/or proper usage?
+  google::InstallSymbolizeCallback(&OnSymbolizeCallback);

[scherkus (not reviewing), 2013/06/12 00:43:43]

satorux: is this intended usage? I couldn't find any existing usage in the
Chromium code :\

[satorux1, 2013/06/12 05:43:42]

IIRC, this awkward callback hook was introduced to decouple some dependency (we
wanted to inject a debug info reader through this callback). This is not
intended usage, but it should be ok to use this way.

+
+  for (size_t i = 0; i < count_; ++i) {
+    // 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).
+    char buf[1024];
+    strncpy(buf, "<unknown>", sizeof(buf));
+
+    // 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;
+    google::Symbolize(address, buf, sizeof(buf));
+
+    // TODO(scherkus): HACK - we blindly print contents of |buf| knowing that

[scherkus (not reviewing), 2013/06/12 00:43:43]

satorux: this is also really gross

I want to use symbolize's /proc/self/map mapping code + symbolizing + demanging
instead of duplicating the code from Android's bionic. Knowing that on Android
we'll fail to symbolize most of the time due to using stripped binaries, my goal
is to get the file name + relocatable address

any ideas?

[satorux1, 2013/06/12 05:43:42]

If Symbolize() fails most of the time, using this function only to get extra
bits of information (pc, relocation, fd) sounds rather awkward. Maybe:

1) Copy the code for reading /proc/self/maps from symbolize.cc. Copying code is
not desirable in general, but it's not a big amount of code.

2) Refactor symbolize.cc so you can achieve your goal cleanly. This will make it
hard to follow the upstream, but this may not be a big deal, as we don't seem to
be following the upstream anyway.

3) Write your own minimal /proc/self/maps reader here, that suits your purpose
perfectly.

4) Write a generic /proc/self/maps reader and make it a separate base library
like base/debug/proc_maps_reader.h

[scherkus (not reviewing), 2013/06/13 01:27:44]

I went with (2) -- let me know if you think it's too much of a divergence

+    // it's initialized to "<unknown>" and that our callback will be executed to
+    // fill the contents with the executable path and offset.
+
+    *os << "  #" << std::setfill('0') << std::setw(2) << i << " " << buf
+        << "\n";
+  }
+
+  google::InstallSymbolizeCallback(NULL);
 }
 
 }  // namespace debug
 }  // namespace base