Clean up base/debug/stack_trace_android.cc.

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 <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
 
   // The first stack frame is this function itself.  Skip it.
   if (ip != 0 && !state->have_skipped_self) {
     state->have_skipped_self = true;
     return _URC_NO_REASON;
   }
 
   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
 
 namespace base {
 namespace debug {
 
 bool EnableInProcessStackDumping() {
   // When running in an application, our code typically expects SIGPIPE
   // to be ignored.  Therefore, when testing that same code, it should run
   // with SIGPIPE ignored as well.
   // TODO(phajdan.jr): De-duplicate this SIGPIPE code.
   struct sigaction action;
   memset(&action, 0, sizeof(action));
   action.sa_handler = SIG_IGN;
   sigemptyset(&action.sa_mask);
   return (sigaction(SIGPIPE, &action, NULL) == 0);
 }
 
 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 {
   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).
@@ skipping 31 matching lines @@
     } else {
       *os << "<unknown>";
     }
 
     *os << "\n";
   }
 }
 
 }  // namespace debug
 }  // namespace base