Implement basic stack traces on Android and reenable unit tests.

base/debug/stack_trace_android.cc

Index: base/debug/stack_trace_android.cc
diff --git a/base/debug/stack_trace_android.cc b/base/debug/stack_trace_android.cc
index 050a2d8c501904b17e3bee5cf13993fafd639026..c5f6de50fffa8a753d3eae999c3681c690eaecc2 100644
--- a/base/debug/stack_trace_android.cc
+++ b/base/debug/stack_trace_android.cc
@@ -4,17 +4,11 @@
 
 #include "base/debug/stack_trace.h"
 
-#include <signal.h>
-#include <sys/types.h>
-#include <unistd.h>
+#include <android/log.h>
 #include <unwind.h>  // TODO(dmikurube): Remove.  See http://crbug.com/236855.
 
-#include "base/logging.h"
-
-#ifdef __MIPSEL__
-// SIGSTKFLT is not defined for MIPS.
-#define SIGSTKFLT SIGSEGV
-#endif
+#include "base/debug/proc_maps_linux.h"
+#include "base/strings/stringprintf.h"
 
 // TODO(dmikurube): Remove when Bionic's get_backtrace() gets popular.
 // See http://crbug.com/236855.
@@ -93,32 +87,52 @@ StackTrace::StackTrace() {
   // 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 {
-  // 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);
+  std::string backtrace = ToString();
+  __android_log_write(ANDROID_LOG_ERROR, "chromium", backtrace.c_str());
 }
 
+// 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();
+  std::string proc_maps;
+  std::vector<MappedMemoryRegion> regions;
+  if (!ReadProcMaps(&proc_maps)) {
+    __android_log_write(
+        ANDROID_LOG_ERROR, "chromium", "Failed to read /proc/self/maps");
+  } else if (!ParseProcMaps(proc_maps, &regions)) {
+    __android_log_write(
+        ANDROID_LOG_ERROR, "chromium", "Failed to parse /proc/self/maps");
+  }
+
+  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

[Mark Mentovai, 2013/07/03 14:03:30]

Nit: “subtract one,” not “subtract by one.”

[scherkus (not reviewing), 2013/07/03 19:43:01]

Done.

+    // function when a function is annotated as noreturn.

[Mark Mentovai, 2013/07/03 14:03:30]

This comment only tells half the story. If someone goes and disassembles things,
it’s nice for the instruction pointers to line up with the disassembly.

Will this code run only on ARM? If so, and thumb instructions are irrelevant,
you can reliably subtract 4 to land on the actual instruction. But I think you
can’t exclude the possibility of thumb, and even so, with Atom-based Androids,
you can’t really infer anything about instruction length.

[scherkus (not reviewing), 2013/07/03 19:43:01]

I fully admit that this is vestigial code from when I attempted to implement the
android stack trace code using the posix code. This code + comment is lifted
from stack_trace_posix.cc:136

I notice build/common.gypi will enable -mthumb when building arm_version==7, but
I'm not aware of a #define I could check for. I'm also not sure we want to go
down the path of assuming instruction lengths based on build configs :\

I'm not an expert in this area so let me know if you've got any ideas. Happy to
experiment!

+    uintptr_t address = reinterpret_cast<uintptr_t>(trace_[i]) - 1;
+
+    std::vector<MappedMemoryRegion>::iterator iter = regions.begin();
+    while (iter != regions.end()) {
+      if (address >= iter->start && address < iter->end &&
+          (iter->permissions & MappedMemoryRegion::READ) &&

[Mark Mentovai, 2013/07/03 14:03:30]

Why check the protection bits? If the address is in the range, it’s a winner.
Checking the bits just means that some bad actor can mess your stack trace up by
calling mprotect.

On the other hand, since you’re using this to figure out if you have a region
that belongs to a file, maybe you want to check that the device (and inode?)
numbers are zero and that there actually is a nonempty filename.

[scherkus (not reviewing), 2013/07/03 19:43:01]

do you mean non-zero inode/device?

considering we're printing out the path I opted for checking the path

[Mark Mentovai, 2013/07/04 01:25:49]

scherkus wrote:
Oops, yeah, I meant nonzero.

+          (iter->permissions & MappedMemoryRegion::EXECUTE)) {
+        break;
+      }
+      ++iter;
+    }
+
+    if (iter != regions.end()) {
+      uintptr_t rel_pc = address - iter->start + iter->offset;
+      const char* path = iter->path.c_str();
+      *os << base::StringPrintf("pc %08x %s", rel_pc, path);

[Mark Mentovai, 2013/07/03 14:03:30]

Maybe it’s just me, but this format seems kind of weird. I would expect
something like “path + offset_into_path”.

[scherkus (not reviewing), 2013/07/03 19:43:01]

Was attempting to emulate the Android stack dump style so these stacks could
integrate with their tools, but that's no longer a requirement and I don't even
like their stack trace style.

Changed to the following style:

#01 0x1234abcd /path/to/lib.so+0x0004012a
#02 0x1234abc0 <unknown>

Here's some sample output from running the StackTraceTest.DebugOutputToStream
test:

#00 0x692b0ed1
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x00065ed1
#01 0x6941bc01
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d0c01
#02 0x6941d7c5
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d27c5
#03 0x6941d875
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d2875
#04 0x6941d939
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d2939
#05 0x6941daf3
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d2af3
#06 0x6941bc01
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d0c01
#07 0x6941bc2f
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d0c2f
#08 0x69421fe3
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001d6fe3
#09 0x69415321
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001ca321
#10 0x69425f7f
/data/app-lib/org.chromium.native_test-1/libbase_unittests.cr.so+0x001daf7f
#11 0x40929c4f /system/lib/libdvm.so+0x0001dc4f
#12 0x40959ed1 /system/lib/libdvm.so+0x0004ded1
#13 0x40933063 /system/lib/libdvm.so+0x00027063
#14 0x409375ef /system/lib/libdvm.so+0x0002b5ef
#15 0x4096c1e1 /system/lib/libdvm.so+0x000601e1
#16 0x40973de1 /system/lib/libdvm.so+0x00067de1
#17 0x40933063 /system/lib/libdvm.so+0x00027063
#18 0x409375ef /system/lib/libdvm.so+0x0002b5ef
#19 0x4096bf23 /system/lib/libdvm.so+0x0005ff23
#20 0x40955b69 /system/lib/libdvm.so+0x00049b69
#21 0x4024c5ff /system/lib/libandroid_runtime.so+0x0004b5ff
#22 0x4024d291 /system/lib/libandroid_runtime.so+0x0004c291
#23 0x400be05d /system/bin/app_process+0x0000105d
#24 0x40165b4f /system/lib/libc.so+0x0000db4f

+    } else {
+      *os << base::StringPrintf("<unknown> 0x%08x", address);

[Mark Mentovai, 2013/07/03 14:03:30]

kinda like the format you used here.

But why’d you put a 0x on the address this time and not if the file was known?

[scherkus (not reviewing), 2013/07/03 19:43:01]

Done.

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