Check stack pointer to be inside stack when unwinding.

base/debug/stack_trace.cc

Index: base/debug/stack_trace.cc
diff --git a/base/debug/stack_trace.cc b/base/debug/stack_trace.cc
index 1c96a569d9795544231714f0d0b1f9b0da036355..fa73131b13f746371a53b80dd739b1240799e3eb 100644
--- a/base/debug/stack_trace.cc
+++ b/base/debug/stack_trace.cc
@@ -11,6 +11,12 @@
 
 #include "base/macros.h"
 
+#if HAVE_TRACE_STACK_FRAME_POINTERS && defined(OS_ANDROID)
+#include <pthread.h>
+#include "base/process/process_handle.h"
+#include "base/threading/platform_thread.h"
+#endif
+
 namespace base {
 namespace debug {
 
@@ -41,6 +47,44 @@ std::string StackTrace::ToString() const {
 
 #if HAVE_TRACE_STACK_FRAME_POINTERS
 
+#if defined(OS_ANDROID)
+
+static uintptr_t GetStackEnd() {
+  // Both glibc and bionic read proc/maps on every call to pthread_getattr_np()
+  // when called from the main thread. So we need to cache end of stack in that
+  // case to get acceptable performance.
+  // For all other threads pthread_getattr_np() is fast enough as it just reads
+  // values from its pthread_t argument.
+  static uintptr_t main_stack_end = 0;
+
+  bool is_main_thread = GetCurrentProcId() == PlatformThread::CurrentId();
+
+  if (is_main_thread && main_stack_end) {
+    return main_stack_end;
+  }
+
+  uintptr_t stack_begin;
+  size_t stack_size;
+  pthread_attr_t attributes;
+  int error = pthread_getattr_np(pthread_self(), &attributes);
+  if (!error) {
+    error = pthread_attr_getstack(
+        &attributes,
+        reinterpret_cast<void**>(&stack_begin),
+        &stack_size);
+    pthread_attr_destroy(&attributes);
+  }
+  DCHECK(!error);
+
+  uintptr_t stack_end = stack_begin + stack_size;
+  if (is_main_thread) {
+    main_stack_end = stack_end;
+  }
+  return stack_end;
+}
+
+#endif  // defined(OS_ANDROID)
+
 size_t TraceStackFramePointers(const void** out_trace,
                                size_t max_depth,
                                size_t skip_initial) {
@@ -49,6 +93,10 @@ size_t TraceStackFramePointers(const void** out_trace,
   // be valid.
   uintptr_t sp = reinterpret_cast<uintptr_t>(__builtin_frame_address(0));
 
+#if defined(OS_ANDROID)
+  uintptr_t stack_end = GetStackEnd();

[Nico, 2016/05/24 20:29:48]

hm, should we do this on linux too? seems nicer if chrome/linux and
chrome/android behave the same way here

[Dmitry Skiba, 2016/05/24 23:53:08]

On Linux heuristic check that we have (>100000) is enough, I've never seen it
crash. And on Android we're crashing because we get that magic 0xfff2f2ee value,
which is not a pointer, but something else. It just so happens that its close to
the stack and passes 100000 test. The  value is always the same, and we could
just hardcode a check for it, but this solution is more generic. I'll enable it
on Linux too if it starts crashing there.

+#endif
+
   size_t depth = 0;
   while (depth < max_depth) {
 #if defined(__arm__) && defined(__GNUC__) && !defined(__clang__)
@@ -58,6 +106,13 @@ size_t TraceStackFramePointers(const void** out_trace,
     sp -= sizeof(uintptr_t);
 #endif
 
+#if defined(OS_ANDROID)
+    // Both sp[0] and s[1] must be valid.
+    if (sp + 2 * sizeof(uintptr_t) > stack_end) {
+      break;
+    }
+#endif
+
     if (skip_initial != 0) {
       skip_initial--;
     } else {