Clean up base/debug/stack_trace_android.cc.

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 4c11a244d6ee2f5513c560f67702de21ab924c5c..388ce4de2d9a1ed11ca8138f02aff7423911c844 100644
--- a/base/debug/stack_trace_android.cc
+++ b/base/debug/stack_trace_android.cc
@@ -5,45 +5,35 @@
 #include "base/debug/stack_trace.h"
 
 #include <android/log.h>
-#include <unwind.h>  // TODO(dmikurube): Remove.  See http://crbug.com/236855.
+#include <unwind.h>
 
 #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.
 namespace {
 
-/* depends how the system includes define this */
-#ifdef HAVE_UNWIND_CONTEXT_STRUCT
-typedef struct _Unwind_Context __unwind_context;
-#else
-typedef _Unwind_Context __unwind_context;
-#endif
+struct StackCrawlState {
+  StackCrawlState(uintptr_t* frames, size_t max_depth)
+      : frames(frames),
+        frame_count(0),
+        max_depth(max_depth),
+        have_skipped_self(false) {}
 
-struct stack_crawl_state_t {
   uintptr_t* frames;
   size_t frame_count;
   size_t max_depth;
   bool have_skipped_self;
-
-  stack_crawl_state_t(uintptr_t* frames, size_t max_depth)
-      : frames(frames),
-        frame_count(0),
-        max_depth(max_depth),
-        have_skipped_self(false) {
-  }
 };
 
-static _Unwind_Reason_Code tracer(__unwind_context* context, void* arg) {
-  stack_crawl_state_t* state = static_cast<stack_crawl_state_t*>(arg);
+_Unwind_Reason_Code TraceStackFrame(_Unwind_Context* context, void* arg) {
+  StackCrawlState* state = static_cast<StackCrawlState*>(arg);
 
+// Clang's unwind.h doesn't provide _Unwind_GetIP on ARM, refer to

[Mark Mentovai, 2013/07/09 20:17:27]

Tiny nit: comment ought to be indented +2.

[scherkus (not reviewing), 2013/07/09 20:31:10]

Done.

+// http://llvm.org/bugs/show_bug.cgi?id=16564 for details.

[scherkus (not reviewing), 2013/07/08 20:37:27]

FYI: filed bug upstream with llvm to figure out what the deal is

 #if defined(__clang__)
-  // Vanilla Clang's unwind.h doesn't have _Unwind_GetIP for ARM.
-  // See http://crbug.com/236855, too.
   uintptr_t ip = 0;
   _Unwind_VRS_Get(context, _UVRSC_CORE, 15, _UVRSD_UINT32, &ip);
-  ip &= ~(uintptr_t)0x1;  // remove thumb mode bit
+  ip &= ~(uintptr_t)0x1;  // Remove thumb mode bit.

[Mark Mentovai, 2013/07/09 20:17:27]

Maybe it’s better to refactor the comment and these three lines (plus a return)
out into their own function guarded by the #ifdef, so that you can write this
function without any #ifdefs.

[scherkus (not reviewing), 2013/07/09 20:31:10]

Ended up defining a local _Unwind_GetIP() guarded by __clang__ ifdef (hopefully
that's what you meant!)

 #else
   uintptr_t ip = _Unwind_GetIP(context);
 #endif
@@ -57,8 +47,7 @@ static _Unwind_Reason_Code tracer(__unwind_context* context, void* arg) {
   state->frames[state->frame_count++] = ip;
   if (state->frame_count >= state->max_depth)
     return _URC_END_OF_STACK;
-  else
-    return _URC_NO_REASON;
+  return _URC_NO_REASON;
 }
 
 }  // namespace
@@ -79,12 +68,9 @@ bool EnableInProcessStackDumping() {
 }
 
 StackTrace::StackTrace() {
-  // TODO(dmikurube): Replace it with Bionic's get_backtrace().
-  // See http://crbug.com/236855.
-  stack_crawl_state_t state(reinterpret_cast<uintptr_t*>(trace_), kMaxTraces);
-  _Unwind_Backtrace(tracer, &state);
+  StackCrawlState state(reinterpret_cast<uintptr_t*>(trace_), kMaxTraces);
+  _Unwind_Backtrace(&TraceStackFrame, &state);
   count_ = state.frame_count;
-  // TODO(dmikurube): Symbolize in Chrome.
 }
 
 void StackTrace::PrintBacktrace() const {