Re-organize the way to produce stacktrace.

Since we introduced the API to set signal handler and print a
stacktrace, we should avoid glibc's backtrace, which may call malloc.
Basically, we choose the way to produce a stacktrace as same as
perftools.

Also, I removed GetStackFrames, which is not used and not implemented
with glibc.


git-svn-id: https://google-glog.googlecode.com/svn/trunk@16 eb4d4688-79bd-11dd-afb4-1d65580434c0

Makefile.am

@@ -40,6 +40,9 @@ dist_doc_DATA = AUTHORS COPYING ChangeLog INSTALL NEWS README \
 ## The libraries (.so's) you want to install
 lib_LTLIBRARIES =
 
+# The libraries libglog depends on.
+COMMON_LIBS=$(PTHREAD_LIBS) $(GFLAGS_LIBS) $(UNWIND_LIBS)
+
 ## unittests you want to run when people type 'make check'.
 ## TESTS is for binary unittests, check_SCRIPTS for script-based unittests.
 ## TESTS_ENVIRONMENT sets environment variables for when you run unittest,
@@ -57,7 +60,7 @@ logging_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
                             src/logging_unittest.cc
 logging_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
 logging_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-logging_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+logging_unittest_LDADD = libglog.la $(COMMON_LIBS)
 
 check_SCRIPTS += logging_striplog_test_sh
 noinst_SCRIPTS += src/logging_striplog_test.sh
@@ -82,63 +85,63 @@ logging_striptest0_SOURCES = $(gloginclude_HEADERS) src/config.h \
                              src/logging_striptest_main.cc
 logging_striptest0_CXXFLAGS = $(PTHREAD_CFLAGS)
 logging_striptest0_LDFLAGS = $(PTHREAD_CFLAGS)
-logging_striptest0_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+logging_striptest0_LDADD = libglog.la $(COMMON_LIBS)
 
 TEST_BINARIES += logging_striptest2
 logging_striptest2_SOURCES = $(gloginclude_HEADERS) src/config.h \
                              src/logging_striptest2.cc
 logging_striptest2_CXXFLAGS = $(PTHREAD_CFLAGS)
 logging_striptest2_LDFLAGS = $(PTHREAD_CFLAGS)
-logging_striptest2_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+logging_striptest2_LDADD = libglog.la $(COMMON_LIBS)
 
 TEST_BINARIES += logging_striptest10
 logging_striptest10_SOURCES = $(gloginclude_HEADERS) src/config.h \
                               src/logging_striptest10.cc
 logging_striptest10_CXXFLAGS = $(PTHREAD_CFLAGS)
 logging_striptest10_LDFLAGS = $(PTHREAD_CFLAGS)
-logging_striptest10_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+logging_striptest10_LDADD = libglog.la $(COMMON_LIBS)
 
 TESTS += demangle_unittest
 demangle_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
                             src/demangle_unittest.cc
 demangle_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
 demangle_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-demangle_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+demangle_unittest_LDADD = libglog.la $(COMMON_LIBS)
 
 TESTS += stacktrace_unittest
 stacktrace_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
                               src/stacktrace_unittest.cc
 stacktrace_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
 stacktrace_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-stacktrace_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+stacktrace_unittest_LDADD = libglog.la $(COMMON_LIBS)
 
 TESTS += symbolize_unittest
 symbolize_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
                               src/symbolize_unittest.cc
 symbolize_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
 symbolize_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-symbolize_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
-
-TESTS += stacktrace_framesizes_unittest
-stacktrace_framesizes_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
-                                         src/stacktrace_framesizes_unittest.cc
-stacktrace_framesizes_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
-stacktrace_framesizes_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-stacktrace_framesizes_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+symbolize_unittest_LDADD = libglog.la $(COMMON_LIBS)
 
 TESTS += stl_logging_unittest
 stl_logging_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
                                src/stl_logging_unittest.cc
 stl_logging_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
 stl_logging_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-stl_logging_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+stl_logging_unittest_LDADD = libglog.la $(COMMON_LIBS)
 
 TEST_BINARIES += signalhandler_unittest
 signalhandler_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
                                src/signalhandler_unittest.cc
 signalhandler_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
 signalhandler_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
-signalhandler_unittest_LDADD = libglog.la $(PTHREAD_LIBS) $(GFLAGS_LIBS)
+signalhandler_unittest_LDADD = libglog.la $(COMMON_LIBS)
+
+TESTS += utilities_unittest
+utilities_unittest_SOURCES = $(gloginclude_HEADERS) src/config.h \
+                             src/utilities_unittest.cc
+utilities_unittest_CXXFLAGS = $(PTHREAD_CFLAGS)
+utilities_unittest_LDFLAGS = $(PTHREAD_CFLAGS)
+utilities_unittest_LDADD = libglog.la $(COMMON_LIBS)
 
 ## vvvv RULES TO MAKE THE LIBRARIES, BINARIES, AND UNITTESTS
 
@@ -147,7 +150,7 @@ libglog_la_SOURCES = $(gloginclude_HEADERS) src/config.h \
                        src/logging.cc src/raw_logging.cc src/vlog_is_on.cc \
                        src/utilities.cc src/utilities.h \
 	 	       src/demangle.cc src/demangle.h \
-                       src/stacktrace.cc src/stacktrace.h \
+                       src/stacktrace.h \
                        src/stacktrace_generic-inl.h \
                        src/stacktrace_libunwind-inl.h \
                        src/stacktrace_powerpc-inl.h \
@@ -160,7 +163,7 @@ libglog_la_SOURCES = $(gloginclude_HEADERS) src/config.h \
 
 libglog_la_CXXFLAGS = $(PTRHEAD_CFLAGS) -DNDEBUG
 libglog_la_LDFLAGS = $(PTRHEAD_CFLAGS)
-libglog_la_LIBADD = $(PTHREAD_LIBS)
+libglog_la_LIBADD = $(COMMON_LIBS)
 
 ## ^^^^ END OF RULES TO MAKE THE LIBRARIES, BINARIES, AND UNITTESTS
 

configure.ac

@@ -31,8 +31,9 @@ AC_CHECK_HEADER(inttypes.h, ac_cv_have_inttypes_h=1, ac_cv_have_inttypes_h=0)
 AC_CHECK_HEADERS(syscall.h)
 AC_CHECK_HEADERS(sys/syscall.h)
 # For backtrace with glibc.
-# TODO(hamaji): Support other platforms using libgcc.
 AC_CHECK_HEADERS(execinfo.h)
+# For backtrace with libunwind.
+AC_CHECK_HEADERS(libunwind.h, ac_cv_have_libunwind_h=1, ac_cv_have_libunwind_h=0)
 AC_CHECK_HEADERS(ucontext.h)
 
 AC_CHECK_SIZEOF(void *)
@@ -92,6 +93,19 @@ else
   GFLAGS_LIBS=
 fi
 
+# We want to link in libunwind if it exists
+UNWIND_LIBS=
+# Unfortunately, we need to check the header file in addition to the
+# lib file to check if libunwind is available since libunwind-0.98
+# doesn't install all necessary header files.
+if test x"$ac_cv_have_libunwind_h" = x"1"; then
+ AC_CHECK_LIB(unwind, backtrace, UNWIND_LIBS=-lunwind)
+fi
+AC_SUBST(UNWIND_LIBS)
+if test x"$UNWIND_LIBS" != x""; then
+  AC_DEFINE(HAVE_LIB_UNWIND, 1, [define if you have libunwind])
+fi
+
 # We'd like to use read/write locks in several places in the code.
 # See if our pthreads support extends to that.  Note: for linux, it
 # does as long as you define _XOPEN_SOURCE appropriately.

src/config.h.in

@@ -15,9 +15,15 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if you have the <libunwind.h> header file. */
+#undef HAVE_LIBUNWIND_H
+
 /* define if you have google gflags library */
 #undef HAVE_LIB_GFLAGS
 
+/* define if you have libunwind */
+#undef HAVE_LIB_UNWIND
+
 /* Define to 1 if you have the <memory.h> header file. */
 #undef HAVE_MEMORY_H
 

src/signalhandler.cc

@@ -22,30 +22,6 @@ _START_GOOGLE_NAMESPACE_
 
 namespace {
 
-// Wrapper of __sync_val_compare_and_swap. If the GCC extension isn't
-// defined, we try the CPU specific logics (we only support x86 and
-// x86_64 for now) first, then use a naive implementation, which has a
-// race condition.
-template<typename T>
-inline T* sync_val_compare_and_swap(T** ptr, T* oldval, T* newval) {
-#if defined(HAVE___SYNC_VAL_COMPARE_AND_SWAP)
-  return __sync_val_compare_and_swap(ptr, oldval, newval);
-#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
-  T* ret;
-  __asm__ __volatile__("lock; cmpxchg %1, (%2);"
-                       :"=a"(ret)
-                       :"r"(newval), "r"(ptr), "a"(oldval)
-                       :"memory", "cc");
-  return ret;
-#else
-  T* ret = *ptr;
-  if (ret == oldval) {
-    *ptr = newval;
-  }
-  return ret;
-#endif
-}
-
 // We'll install the failure signal handler for these signals.  We could
 // use strsignal() to get signal names, but we don't use it to avoid
 // introducing yet another #ifdef complication.
@@ -262,7 +238,8 @@ void FailureSignalHandler(int signal_number,
   // old value (value returned from __sync_val_compare_and_swap) is
   // different from the original value (in this case NULL).
   pthread_t* old_thread_id_pointer =
-      sync_val_compare_and_swap(&g_entered_thread_id_pointer,
+      glog_internal_namespace_::sync_val_compare_and_swap(
+          &g_entered_thread_id_pointer,
           static_cast<pthread_t*>(NULL),
           &my_thread_id);
   if (old_thread_id_pointer != NULL) {

src/signalhandler_unittest.cc

@@ -27,7 +27,7 @@ void WriteToStdout(const char* data, int size) {
 }
 
 int main(int argc, char **argv) {
-#ifdef HAVE_STACKTRACE
+#if defined(HAVE_STACKTRACE) && defined(HAVE_SYMBOLIZE)
   InitGoogleLogging(argv[0]);
   InstallFailureSignalHandler();
   const std::string command = argc > 1 ? argv[1] : "none";

src/signalhandler_unittest.sh

@@ -35,6 +35,13 @@ if [ x`$BINARY` != 'xOK' ]; then
   exit 0
 fi
 
+# The PC cannot be obtained in signal handlers on PowerPC correctly.
+# We just skip the test for PowerPC.
+if [ x`uname -p` = x"powerpc" ]; then
+  echo "PASS (We don't test the signal handler on PowerPC.)"
+  exit 0
+fi
+
 # Test for a case the program kills itself by SIGSEGV.
 $BINARY segv 2> signalhandler.out1
 for pattern in SIGSEGV 0xdead main "Aborted at [0-9]"; do

src/stacktrace.h

@@ -11,40 +11,6 @@
 
 _START_GOOGLE_NAMESPACE_
 
-// Skips the most recent "skip_count" stack frames (also skips the
-// frame generated for the "GetStackFrames" routine itself), and then
-// records the pc values for up to the next "max_depth" frames in
-// "pcs", and the corresponding stack frame sizes in "sizes".  Returns
-// the number of values recorded in "pcs"/"sizes".
-//
-// Example:
-//      main() { foo(); }
-//      foo() { bar(); }
-//      bar() {
-//        void* pcs[10];
-//        int sizes[10];
-//        int depth = GetStackFrames(pcs, sizes, 10, 1);
-//      }
-//
-// The GetStackFrames call will skip the frame for "bar".  It will
-// return 2 and will produce pc values that map to the following
-// procedures:
-//      pcs[0]       foo
-//      pcs[1]       main
-// (Actually, there may be a few more entries after "main" to account for
-// startup procedures.)
-// And corresponding stack frame sizes will also be recorded:
-//    sizes[0]       16
-//    sizes[1]       16
-// (Stack frame sizes of 16 above are just for illustration purposes.)
-// Stack frame sizes of 0 or less indicate that those frame sizes couldn't
-// be identified.
-//
-// This routine may return fewer stack frame entries than are
-// available. Also note that "pcs" and "sizes" must both be non-NULL.
-extern int GetStackFrames(void** pcs, int* sizes, int max_depth,
-                          int skip_count);
-
 // This is similar to the GetStackFrames routine, except that it returns
 // the stack trace only, and not the stack frame sizes as well.
 // Example:

src/stacktrace_framesizes_unittest.cc

-// Copyright 2007 Google Inc. All Rights Reserved.
-// Author: Raksit Ashok
-//
-// Unit test for the GetStackFrames function in stacktrace.cc.
-
-#include <stdio.h>
-#include "glog/logging.h"
-#include "base/commandlineflags.h"
-#include "stacktrace.h"
-#include "config.h"
-#include "utilities.h"
-
-using std::min;
-using namespace GOOGLE_NAMESPACE;
-
-#ifdef HAVE_STACKTRACE
-
-// Obtain a backtrace of the stack frame sizes, verify that they look sane.
-
-//-----------------------------------------------------------------------//
-int CheckFrameSizesLeaf(int32* i);  // 8KB frame size.
-int CheckFrameSizes2(int32* i);     // 4KB
-int CheckFrameSizes1(int32* i);     // 2KB
-int CheckFrameSizes(int32* i);      // 1KB
-//-----------------------------------------------------------------------//
-
-// The expected frame-sizes in the backtrace.
-const int BACKTRACE_STEPS = 4;
-int expected_frame_sizes[BACKTRACE_STEPS] = {
-  1 << 13,
-  1 << 12,
-  1 << 11,
-  1 << 10,
-};
-
-//-----------------------------------------------------------------------//
-
-void CheckFrameSizeIsOk(int actual_frame_size, int ref_frame_size) {
-  // Assume upto 512 bytes of miscellaneous stuff in CheckFrameSizes* frames.
-  const int misc_frame_size = 512;
-  CHECK_GE(actual_frame_size, ref_frame_size);
-  CHECK_LE(actual_frame_size, ref_frame_size + misc_frame_size);
-}
-
-//-----------------------------------------------------------------------//
-
-int ATTRIBUTE_NOINLINE CheckFrameSizesLeaf(int32 *i) {
-  const int DEPTH = 10;
-  void* pcs[DEPTH];
-  int frame_sizes[DEPTH];
-  int size;
-  int32 j[2048];  // 8KB.
-  for (int k = 0; k < 2048; k++) j[k] = k + i[k % 1024];
-
-  for (int depth = 0; depth < DEPTH; depth++) {
-    size = GetStackFrames(pcs, frame_sizes, depth, 0);
-    printf("--- GetStackFrames(..., %d, 0) = %d\n", depth, size);
-    CHECK_LE(size, depth);
-    CHECK_GE(size, min(depth, BACKTRACE_STEPS));
-
-    for (int k = 0; k < size; k++) {
-      if (k < BACKTRACE_STEPS)
-    // GetStackFrames doesn't work correctly if we are using glibc's backtrace.
-#ifndef HAVE_EXECINFO_H
-        CheckFrameSizeIsOk(frame_sizes[k], expected_frame_sizes[k]);
-#endif
-      printf("frame_sizes[%d] = %d\n", k, frame_sizes[k]);
-    }
-  }
-
-  int sum = 0;
-  for (int k = 0; k < 2048; k++) sum += j[k];
-  return sum;
-}
-
-//-----------------------------------------------------------------------//
-
-/* Dummy functions to make the frame-size backtrace more interesting. */
-int ATTRIBUTE_NOINLINE CheckFrameSizes2(int32* i) {
-  int32 j[1024];  // 4KB.
-  for (int k = 0; k < 1024; k++) j[k] = k + i[k % 512];
-  return CheckFrameSizesLeaf(j) + j[512];
-}
-
-int ATTRIBUTE_NOINLINE CheckFrameSizes1(int32* i) {
-  int32 j[512];  // 2KB.
-  for (int k = 0; k < 512; k++) j[k] = k + i[k % 256];
-  return CheckFrameSizes2(j) + j[256];
-}
-
-int ATTRIBUTE_NOINLINE CheckFrameSizes(int32* i)  {
-  int32 j[256];  // 1KB.
-  for (int k = 0; k < 256; k++) j[k] = k + i[k];
-  return CheckFrameSizes1(j) + j[128];
-}
-
-//-----------------------------------------------------------------------//
-
-int main(int argc, char ** argv) {
-  FLAGS_logtostderr = true;
-  InitGoogleLogging(argv[0]);
-
-  int32 i[256];  // 1KB.
-  for (int j = 0; j < 256; j++) i[j] = j;
-  
-  int ret = CheckFrameSizes(i);
-
-  printf("CheckFrameSizes returned: %d\n", ret);
-
-  printf("PASS\n");
-  return 0;
-}
-
-#else
-int main() {
-  printf("PASS (no stacktrace support)\n");
-  return 0;
-}
-#endif  // HAVE_STACKTRACE

src/stacktrace_generic-inl.h

@@ -30,52 +30,4 @@ int GetStackTrace(void** result, int max_depth, int skip_count) {
   return result_count;
 }
 
-// If you change this function, also change GetStackTrace above:
-//
-// This GetStackFrames routine shares a lot of code with GetStackTrace
-// above. This code could have been refactored into a common routine,
-// and then both GetStackTrace/GetStackFrames could call that routine.
-// There are two problems with that:
-//
-// (1) The performance of the refactored-code suffers substantially - the
-//     refactored needs to be able to record the stack trace when called
-//     from GetStackTrace, and both the stack trace and stack frame sizes,
-//     when called from GetStackFrames - this introduces enough new
-//     conditionals that GetStackTrace performance can degrade by as much
-//     as 50%.
-//
-// (2) Whether the refactored routine gets inlined into GetStackTrace and
-//     GetStackFrames depends on the compiler, and we can't guarantee the
-//     behavior either-way, even with "__attribute__ ((always_inline))"
-//     or "__attribute__ ((noinline))". But we need this guarantee or the
-//     frame counts may be off by one.
-//
-// Both (1) and (2) can be addressed without this code duplication, by
-// clever use of template functions, and by defining GetStackTrace and
-// GetStackFrames as macros that expand to these template functions.
-// However, this approach comes with its own set of problems - namely,
-// macros and  preprocessor trouble - for example,  if GetStackTrace
-// and/or GetStackFrames is ever defined as a member functions in some
-// class, we are in trouble.
-int GetStackFrames(void** pcs, int* sizes, int max_depth, int skip_count) {
-  static const int kStackLength = 64;
-  void * stack[kStackLength];
-  int size;
-
-  size = backtrace(stack, kStackLength);
-  skip_count++;  // we want to skip the current frame as well
-  int result_count = size - skip_count;
-  if (result_count < 0)
-    result_count = 0;
-  if (result_count > max_depth)
-    result_count = max_depth;
-  for (int i = 0; i < result_count; i++)
-    pcs[i] = stack[i + skip_count];
-
-  // No implementation for finding out the stack frame sizes yet.
-  memset(sizes, 0, sizeof(*sizes) * result_count);
-
-  return result_count;
-}
-
 _END_GOOGLE_NAMESPACE_

src/stacktrace_libunwind-inl.h

@@ -5,23 +5,25 @@
 //
 // Produce stack trace using libunwind
 
+#include "utilities.h"
+
 extern "C" {
+#define UNW_LOCAL_ONLY
 #include <libunwind.h>
 }
-#include "base/stacktrace.h"
-#include "base/raw_logging.h"
-#include "base/spinlock.h"
+#include "glog/raw_logging.h"
+#include "stacktrace.h"
 
 _START_GOOGLE_NAMESPACE_
 
 // Sometimes, we can try to get a stack trace from within a stack
-// trace, because libunwind can call mmap/sbrk (maybe indirectly via
-// malloc), and that mmap gets trapped and causes a stack-trace
-// request.  If were to try to honor that recursive request, we'd end
-// up with infinite recursion or deadlock.  Luckily, it's safe to
-// ignore those subsequent traces.  In such cases, we return 0 to
-// indicate the situation.
-static SpinLock libunwind_lock(SpinLock::LINKER_INITIALIZED);
+// trace, because libunwind can call mmap (maybe indirectly via an
+// internal mmap based memory allocator), and that mmap gets trapped
+// and causes a stack-trace request.  If were to try to honor that
+// recursive request, we'd end up with infinite recursion or deadlock.
+// Luckily, it's safe to gnore those subsequent traces.  In such cases,
+// we return 0 to indicate the situation.
+static bool g_now_entering = false;
 
 // If you change this function, also change GetStackFrames below.
 int GetStackTrace(void** result, int max_depth, int skip_count) {
@@ -30,7 +32,7 @@ int GetStackTrace(void** result, int max_depth, int skip_count) {
   unw_cursor_t cursor;
   unw_context_t uc;
 
-  if (!libunwind_lock.TryLock()) {
+  if (sync_val_compare_and_swap(&g_now_entering, false, true)) {
     return 0;
   }
 
@@ -52,75 +54,7 @@ int GetStackTrace(void** result, int max_depth, int skip_count) {
       break;
   }
 
-  libunwind_lock.Unlock();
-  return n;
-}
-
-// If you change this function, also change GetStackTrace above:
-//
-// This GetStackFrames routine shares a lot of code with GetStackTrace
-// above. This code could have been refactored into a common routine,
-// and then both GetStackTrace/GetStackFrames could call that routine.
-// There are two problems with that:
-//
-// (1) The performance of the refactored-code suffers substantially - the
-//     refactored needs to be able to record the stack trace when called
-//     from GetStackTrace, and both the stack trace and stack frame sizes,
-//     when called from GetStackFrames - this introduces enough new
-//     conditionals that GetStackTrace performance can degrade by as much
-//     as 50%.
-//
-// (2) Whether the refactored routine gets inlined into GetStackTrace and
-//     GetStackFrames depends on the compiler, and we can't guarantee the
-//     behavior either-way, even with "__attribute__ ((always_inline))"
-//     or "__attribute__ ((noinline))". But we need this guarantee or the
-//     frame counts may be off by one.
-//
-// Both (1) and (2) can be addressed without this code duplication, by
-// clever use of template functions, and by defining GetStackTrace and
-// GetStackFrames as macros that expand to these template functions.
-// However, this approach comes with its own set of problems - namely,
-// macros and  preprocessor trouble - for example,  if GetStackTrace
-// and/or GetStackFrames is ever defined as a member functions in some
-// class, we are in trouble.
-int GetStackFrames(void** pcs, int* sizes, int max_depth, int skip_count) {
-  void *ip;
-  int n = 0;
-  unw_cursor_t cursor;
-  unw_context_t uc;
-  unw_word_t sp = 0, next_sp = 0;
-
-  if (!libunwind_lock.TryLock()) {
-    return 0;
-  }
-
-  unw_getcontext(&uc);
-  RAW_CHECK(unw_init_local(&cursor, &uc) >= 0, "unw_init_local failed");
-  skip_count++;         // Do not include the "GetStackFrames" frame
-
-  while (skip_count--) {
-    if (unw_step(&cursor) <= 0 ||
-        unw_get_reg(&cursor, UNW_REG_SP, &next_sp) < 0) {
-      goto out;
-    }
-  }
-  while (n < max_depth) {
-    sp = next_sp;
-    if (unw_get_reg(&cursor, UNW_REG_IP, (unw_word_t *) &ip) < 0)
-      break;
-    if (unw_step(&cursor) <= 0 ||
-        unw_get_reg(&cursor, UNW_REG_SP, &next_sp)) {
-      // We couldn't step any further (possibly because we reached _start).
-      // Provide the last good PC we've got, and get out.
-      sizes[n] = 0;
-      pcs[n++] = ip;
-      break;
-    }
-    sizes[n] = next_sp - sp;
-    pcs[n++] = ip;
-  }
- out:
-  libunwind_lock.Unlock();
+  g_now_entering = false;
   return n;
 }
 

src/stacktrace_powerpc-inl.h

@@ -101,82 +101,4 @@ int GetStackTrace(void** result, int max_depth, int skip_count) {
   return n;
 }
 
-// If you change this function, also change GetStackTrace above:
-//
-// This GetStackFrames routine shares a lot of code with GetStackTrace
-// above. This code could have been refactored into a common routine,
-// and then both GetStackTrace/GetStackFrames could call that routine.
-// There are two problems with that:
-//
-// (1) The performance of the refactored-code suffers substantially - the
-//     refactored needs to be able to record the stack trace when called
-//     from GetStackTrace, and both the stack trace and stack frame sizes,
-//     when called from GetStackFrames - this introduces enough new
-//     conditionals that GetStackTrace performance can degrade by as much
-//     as 50%.
-//
-// (2) Whether the refactored routine gets inlined into GetStackTrace and
-//     GetStackFrames depends on the compiler, and we can't guarantee the
-//     behavior either-way, even with "__attribute__ ((always_inline))"
-//     or "__attribute__ ((noinline))". But we need this guarantee or the
-//     frame counts may be off by one.
-//
-// Both (1) and (2) can be addressed without this code duplication, by
-// clever use of template functions, and by defining GetStackTrace and
-// GetStackFrames as macros that expand to these template functions.
-// However, this approach comes with its own set of problems - namely,
-// macros and  preprocessor trouble - for example,  if GetStackTrace
-// and/or GetStackFrames is ever defined as a member functions in some
-// class, we are in trouble.
-int GetStackFrames(void** pcs, int *sizes, int max_depth, int skip_count) {
-  void **sp;
-#ifdef __APPLE__
-  __asm__ volatile ("mr %0,r1" : "=r" (sp));
-#else
-  __asm__ volatile ("mr %0,1" : "=r" (sp));
-#endif
-
-  StacktracePowerPCDummyFunction();
-  // Note we do *not* increment skip_count here for the SYSV ABI.  If
-  // we did, the list of stack frames wouldn't properly match up with
-  // the list of return addresses.  Note this means the top pc entry
-  // is probably bogus for linux/ppc (and other SYSV-ABI systems).
-
-  int n = 0;
-  while (sp && n < max_depth) {
-    // The GetStackFrames routine is called when we are in some
-    // informational context (the failure signal handler for example).
-    // Use the non-strict unwinding rules to produce a stack trace
-    // that is as complete as possible (even if it contains a few bogus
-    // entries in some rare cases).
-    void **next_sp = NextStackFrame<false>(sp);
-    if (skip_count > 0) {
-      skip_count--;
-    } else {
-#if defined(_CALL_AIX) || defined(_CALL_DARWIN)
-      pcs[n++] = *(sp+2);
-#elif defined(_CALL_SYSV)
-      pcs[n++] = *(sp+1);
-#elif defined(__APPLE__) || (defined(__linux) && defined(__PPC64__))
-      // This check is in case the compiler doesn't define _CALL_AIX/etc.
-      pcs[n++] = *(sp+2);
-#elif defined(__linux)
-      // This check is in case the compiler doesn't define _CALL_SYSV.
-      pcs[n++] = *(sp+1);
-#else
-#error Need to specify the PPC ABI for your archiecture.
-#endif
-      if (next_sp > sp) {
-        sizes[n] = (uintptr_t)next_sp - (uintptr_t)sp;
-      } else {
-        // A frame-size of 0 is used to indicate unknown frame size.
-        sizes[n] = 0;
-      }
-      n++;
-    }
-    sp = next_sp;
-  }
-  return n;
-}
-
 _END_GOOGLE_NAMESPACE_

src/stacktrace_unittest.cc

@@ -93,6 +93,9 @@ void CheckStackTraceLeaf(void) {
     CheckRetAddrIsInFunction(stack[i], expected_stack[i]);
     printf("OK\n");
   }
+
+  // Check if the second stacktrace returns the same size.
+  CHECK_EQ(size, GetStackTrace(stack, STACK_LEN, 0));
 }
 
 //-----------------------------------------------------------------------//

src/stacktrace_x86-inl.h

@@ -110,88 +110,4 @@ int GetStackTrace(void** result, int max_depth, int skip_count) {
   return n;
 }
 
-// If you change this function, also change GetStackTrace above:
-//
-// This GetStackFrames routine shares a lot of code with GetStackTrace
-// above. This code could have been refactored into a common routine,
-// and then both GetStackTrace/GetStackFrames could call that routine.
-// There are two problems with that:
-//
-// (1) The performance of the refactored-code suffers substantially - the
-//     refactored needs to be able to record the stack trace when called
-//     from GetStackTrace, and both the stack trace and stack frame sizes,
-//     when called from GetStackFrames - this introduces enough new
-//     conditionals that GetStackTrace performance can degrade by as much
-//     as 50%.
-//
-// (2) Whether the refactored routine gets inlined into GetStackTrace and
-//     GetStackFrames depends on the compiler, and we can't guarantee the
-//     behavior either-way, even with "__attribute__ ((always_inline))"
-//     or "__attribute__ ((noinline))". But we need this guarantee or the
-//     frame counts may be off by one.
-//
-// Both (1) and (2) can be addressed without this code duplication, by
-// clever use of template functions, and by defining GetStackTrace and
-// GetStackFrames as macros that expand to these template functions.
-// However, this approach comes with its own set of problems - namely,
-// macros and  preprocessor trouble - for example,  if GetStackTrace
-// and/or GetStackFrames is ever defined as a member functions in some
-// class, we are in trouble.
-int GetStackFrames(void** pcs, int* sizes, int max_depth, int skip_count) {
-  void **sp;
-#ifdef __i386__
-  // Stack frame format:
-  //    sp[0]   pointer to previous frame
-  //    sp[1]   caller address
-  //    sp[2]   first argument
-  //    ...
-  sp = (void **)&pcs - 2;
-#endif
-
-#ifdef __x86_64__
-  // __builtin_frame_address(0) can return the wrong address on gcc-4.1.0-k8
-  unsigned long rbp;
-  // Move the value of the register %rbp into the local variable rbp.
-  // We need 'volatile' to prevent this instruction from getting moved
-  // around during optimization to before function prologue is done.
-  // An alternative way to achieve this
-  // would be (before this __asm__ instruction) to call Noop() defined as
-  //   static void Noop() __attribute__ ((noinline));  // prevent inlining
-  //   static void Noop() { asm(""); }  // prevent optimizing-away
-  __asm__ volatile ("mov %%rbp, %0" : "=r" (rbp));
-  // Arguments are passed in registers on x86-64, so we can't just
-  // offset from &pcs
-  sp = (void **) rbp;
-#endif
-
-  int n = 0;
-  while (sp && n < max_depth) {
-    if (*(sp+1) == (void *)0) {
-      // In 64-bit code, we often see a frame that
-      // points to itself and has a return address of 0.
-      break;
-    }
-    // The GetStackFrames routine is called when we are in some
-    // informational context (the failure signal handler for example).
-    // Use the non-strict unwinding rules to produce a stack trace
-    // that is as complete as possible (even if it contains a few bogus
-    // entries in some rare cases).
-    void **next_sp = NextStackFrame<false>(sp);
-    if (skip_count > 0) {
-      skip_count--;
-    } else {
-      pcs[n] = *(sp+1);
-      if (next_sp > sp) {
-        sizes[n] = (uintptr_t)next_sp - (uintptr_t)sp;
-      } else {
-        // A frame-size of 0 is used to indicate unknown frame size.
-        sizes[n] = 0;
-      }
-      n++;
-    }
-    sp = next_sp;
-  }
-  return n;
-}
-
 _END_GOOGLE_NAMESPACE_

src/stacktrace_x86_64-inl.h

@@ -76,52 +76,4 @@ int GetStackTrace(void** result, int max_depth, int skip_count) {
   return targ.count;
 }
 
-// If you change this function, also change GetStackTrace above:
-//
-// This GetStackFrames routine shares a lot of code with GetStackTrace
-// above. This code could have been refactored into a common routine,
-// and then both GetStackTrace/GetStackFrames could call that routine.
-// There are two problems with that:
-//
-// (1) The performance of the refactored-code suffers substantially - the
-//     refactored needs to be able to record the stack trace when called
-//     from GetStackTrace, and both the stack trace and stack frame sizes,
-//     when called from GetStackFrames - this introduces enough new
-//     conditionals that GetStackTrace performance can degrade by as much
-//     as 50%.
-//
-// (2) Whether the refactored routine gets inlined into GetStackTrace and
-//     GetStackFrames depends on the compiler, and we can't guarantee the
-//     behavior either-way, even with "__attribute__ ((always_inline))"
-//     or "__attribute__ ((noinline))". But we need this guarantee or the
-//     frame counts may be off by one.
-//
-// Both (1) and (2) can be addressed without this code duplication, by
-// clever use of template functions, and by defining GetStackTrace and
-// GetStackFrames as macros that expand to these template functions.
-// However, this approach comes with its own set of problems - namely,
-// macros and  preprocessor trouble - for example,  if GetStackTrace
-// and/or GetStackFrames is ever defined as a member functions in some
-// class, we are in trouble.
-int GetStackFrames(void** pcs, int* sizes, int max_depth, int skip_count) {
-  if (!ready_to_run)
-    return 0;
-
-  trace_arg_t targ;
-
-  skip_count += 1;         // Do not include the "GetStackFrames" frame
-
-  targ.result = pcs;
-  targ.max_depth = max_depth;
-  targ.skip_count = skip_count;
-  targ.count = 0;
-
-  _Unwind_Backtrace(GetOneFrame, &targ);
-
-  // No implementation for finding out the stack frame sizes yet.
-  memset(sizes, 0, sizeof(*sizes) * targ.count);
-
-  return targ.count;
-}
-
 _END_GOOGLE_NAMESPACE_

src/symbolize_unittest.cc

@@ -3,6 +3,8 @@
 //
 // Unit tests for functions in symbolize.cc.
 
+#include "utilities.h"
+
 #include <signal.h>
 #include <iostream>
 
@@ -10,7 +12,6 @@
 #include "symbolize.h"
 #include "googletest.h"
 #include "config.h"
-#include "utilities.h"
 
 using namespace std;
 using namespace GOOGLE_NAMESPACE;
@@ -307,19 +308,17 @@ void ATTRIBUTE_NOINLINE TestWithReturnAddress() {
 int main(int argc, char **argv) {
   FLAGS_logtostderr = true;
   InitGoogleLogging(argv[0]);
+#ifdef HAVE_SYMBOLIZE
   // We don't want to get affected by the callback interface, that may be
   // used to install some callback function at InitGoogle() time.
   InstallSymbolizeCallback(NULL);
 
-  // Symbolize() now only supports ELF binaries.
-  // The test makes sense only if __ELF__ is defined.
-#ifndef __ELF__
-  return 0;
-#else
   TestWithPCInsideInlineFunction();
   TestWithPCInsideNonInlineFunction();
   TestWithReturnAddress();
   return RUN_ALL_TESTS();
+#else
+  return 0;
 #endif
 }
 

src/utilities.cc

+// Copyright 2008 Google Inc. All Rights Reserved.
+// Author: hamaji@google.com (Shinichiro Hamaji)
+
 #include "utilities.h"
 
 #include <signal.h>
@@ -105,7 +108,8 @@ static void DumpStackTraceAndExit() {
 
   abort();
 }
-#endif
+
+#endif  // HAVE_STACKTRACE
 
 namespace glog_internal_namespace_ {
 
@@ -179,3 +183,8 @@ void InitGoogleLogging(const char* argv0) {
 }
 
 _END_GOOGLE_NAMESPACE_
+
+// Make an implementation of stacktrace compiled.
+#ifdef STACKTRACE_H
+# include STACKTRACE_H
+#endif

src/utilities.h

+// Copyright 2008 Google Inc. All Rights Reserved.
+// Author: hamaji@google.com (Shinichiro Hamaji)
+//
 // Define utilties for glog internal usage.
 
 #ifndef UTILITIES_H__
@@ -40,13 +43,48 @@
 #include "config.h"
 #include "glog/logging.h"
 
-#if defined(HAVE_EXECINFO_H)
-# define HAVE_STACKTRACE
-#elif defined(STACKTRACE_WITH_FRAME_POINTER)
+// There are three different ways we can try to get the stack trace:
+//
+// 1) The libunwind library.  This is still in development, and as a
+//    separate library adds a new dependency, but doesn't need a frame
+//    pointer.  It also doesn't call malloc.
+//
+// 2) Our hand-coded stack-unwinder.  This depends on a certain stack
+//    layout, which is used by gcc (and those systems using a
+//    gcc-compatible ABI) on x86 systems, at least since gcc 2.95.
+//    It uses the frame pointer to do its work.
+//
+// 3) The gdb unwinder -- also the one used by the c++ exception code.
+//    It's obviously well-tested, but has a fatal flaw: it can call
+//    malloc() from the unwinder.  This is a problem because we're
+//    trying to use the unwinder to instrument malloc().
+//
+// Note: if you add a new implementation here, make sure it works
+// correctly when GetStackTrace() is called with max_depth == 0.
+// Some code may do that.
+
+#if defined(HAVE_LIB_UNWIND)
+# define STACKTRACE_H "stacktrace_libunwind-inl.h"
+#elif !defined(NO_FRAME_POINTER)
+# if defined(__i386__) && __GNUC__ >= 2
+#  define STACKTRACE_H "stacktrace_x86-inl.h"
+# elif defined(__x86_64__) && __GNUC__ >= 2
+#  define STACKTRACE_H "stacktrace_x86_64-inl.h"
+# elif ((__ppc__) || defined(__PPC__)) && __GNUC__ >= 2
+#  define STACKTRACE_H "stacktrace_powerpc-inl.h"
+# endif
+#endif
+
+#if !defined(STACKTRACE_H) && defined(HAVE_EXECINFO_H)
+# define STACKTRACE_H "stacktrace_generic-inl.h"
+#endif
+
+#if defined(STACKTRACE_H)
 # define HAVE_STACKTRACE
 #endif
 
-#if defined(__ELF__)  // defined by gcc on Linux
+// defined by gcc
+#if defined(__ELF__) && defined(OS_LINUX)
 # define HAVE_SYMBOLIZE
 #elif defined(OS_MACOSX) && defined(HAVE_DLADDR)
 // Use dladdr to symbolize.
@@ -78,7 +116,36 @@ int32 GetMainThreadPid();
 
 const std::string& MyUserName();
 
+// Wrapper of __sync_val_compare_and_swap. If the GCC extension isn't
+// defined, we try the CPU specific logics (we only support x86 and
+// x86_64 for now) first, then use a naive implementation, which has a
+// race condition.
+template<typename T>
+inline T sync_val_compare_and_swap(T* ptr, T oldval, T newval) {
+#if defined(HAVE___SYNC_VAL_COMPARE_AND_SWAP)
+  return __sync_val_compare_and_swap(ptr, oldval, newval);
+#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
+  T ret;
+  __asm__ __volatile__("lock; cmpxchg %1, (%2);"
+                       :"=a"(ret)
+                        // GCC may produces %sil or %dil for
+                        // constraint "r", but some of apple's gas
+                        // dosn't know the 8 bit registers.
+                        // We use "q" to avoid these registers.
+                       :"q"(newval), "q"(ptr), "a"(oldval)
+                       :"memory", "cc");
+  return ret;
+#else
+  T ret = *ptr;
+  if (ret == oldval) {
+    *ptr = newval;
+  }
+  return ret;
+#endif
 }
+
+}  // namespace glog_internal_namespace_
+
 _END_GOOGLE_NAMESPACE_
 
 using namespace GOOGLE_NAMESPACE::glog_internal_namespace_;

src/utilities_unittest.cc

+// Copyright 2008 Google Inc. All Rights Reserved.
+// Author: hamaji@google.com (Shinichiro Hamaji)
+
+#include "utilities.h"
+#include "googletest.h"
+#include "glog/logging.h"
+
+using namespace GOOGLE_NAMESPACE;
+
+TEST(sync_val_compare_and_swap, utilities) {
+  bool now_entering = false;
+  EXPECT_FALSE(sync_val_compare_and_swap(&now_entering, false, true));
+  EXPECT_TRUE(sync_val_compare_and_swap(&now_entering, false, true));
+  EXPECT_TRUE(sync_val_compare_and_swap(&now_entering, false, true));
+}
+
+int main(int argc, char **argv) {
+  InitGoogleLogging(argv[0]);
+
+  CHECK_EQ(RUN_ALL_TESTS(), 0);
+}