Add error reporting mechanism for failing Unwind.

Remove the logging of an error if a thread disappears before the unwind
can begin. This can happen, so allow the caller to determine if this
is really a problem worth logging.

Bug: 27449879
Change-Id: Ie81718d53fb0e519fa0a7db9fd5f314b72bfa431

6 files changed, 85 insertions, 2 deletions

diff --git a/libbacktrace/Backtrace.cpp b/libbacktrace/Backtrace.cpp
index baa3d0f70..995abc02c 100644
--- a/libbacktrace/Backtrace.cpp
+++ b/libbacktrace/Backtrace.cpp
@@ -148,3 +148,24 @@ Backtrace* Backtrace::Create(pid_t pid, pid_t tid, BacktraceMap* map) {
     return new UnwindPtrace(pid, tid, map);
   }
 }
+
+std::string Backtrace::GetErrorString(BacktraceUnwindError error) {
+  switch (error) {
+  case BACKTRACE_UNWIND_NO_ERROR:
+    return "No error";
+  case BACKTRACE_UNWIND_ERROR_SETUP_FAILED:
+    return "Setup failed";
+  case BACKTRACE_UNWIND_ERROR_MAP_MISSING:
+    return "No map found";
+  case BACKTRACE_UNWIND_ERROR_INTERNAL:
+    return "Internal libbacktrace error, please submit a bugreport";
+  case BACKTRACE_UNWIND_ERROR_THREAD_DOESNT_EXIST:
+    return "Thread doesn't exist";
+  case BACKTRACE_UNWIND_ERROR_THREAD_TIMEOUT:
+    return "Thread has not repsonded to signal in time";
+  case BACKTRACE_UNWIND_ERROR_UNSUPPORTED_OPERATION:
+    return "Attempt to use an unsupported feature";
+  case BACKTRACE_UNWIND_ERROR_NO_CONTEXT:
+    return "Attempt to do an offline unwind without a context";
+  }
+}
diff --git a/libbacktrace/BacktraceCurrent.cpp b/libbacktrace/BacktraceCurrent.cpp
index 8e223668e..5173e2cc9 100644
--- a/libbacktrace/BacktraceCurrent.cpp
+++ b/libbacktrace/BacktraceCurrent.cpp
@@ -24,6 +24,8 @@
 #include <ucontext.h>
 #include <unistd.h>
 
+#include <stdlib.h>
+
 #include <string>
 
 #include <backtrace/Backtrace.h>
@@ -65,9 +67,11 @@ size_t BacktraceCurrent::Read(uintptr_t addr, uint8_t* buffer, size_t bytes) {
 bool BacktraceCurrent::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) {
   if (GetMap() == nullptr) {
     // Without a map object, we can't do anything.
+    error_ = BACKTRACE_UNWIND_ERROR_MAP_MISSING;
     return false;
   }
 
+  error_ = BACKTRACE_UNWIND_NO_ERROR;
   if (ucontext) {
     return UnwindFromContext(num_ignore_frames, ucontext);
   }
@@ -138,11 +142,19 @@ bool BacktraceCurrent::UnwindThread(size_t num_ignore_frames) {
     BACK_LOGE("sigaction failed: %s", strerror(errno));
     ThreadEntry::Remove(entry);
     pthread_mutex_unlock(&g_sigaction_mutex);
+    error_ = BACKTRACE_UNWIND_ERROR_INTERNAL;
     return false;
   }
 
   if (tgkill(Pid(), Tid(), THREAD_SIGNAL) != 0) {
-    BACK_LOGE("tgkill %d failed: %s", Tid(), strerror(errno));
+    // Do not emit an error message, this might be expected. Set the
+    // error and let the caller decide.
+    if (errno == ESRCH) {
+      error_ = BACKTRACE_UNWIND_ERROR_THREAD_DOESNT_EXIST;
+    } else {
+      error_ = BACKTRACE_UNWIND_ERROR_INTERNAL;
+    }
+
     sigaction(THREAD_SIGNAL, &oldact, nullptr);
     ThreadEntry::Remove(entry);
     pthread_mutex_unlock(&g_sigaction_mutex);
@@ -183,7 +195,13 @@ bool BacktraceCurrent::UnwindThread(size_t num_ignore_frames) {
       BACK_LOGW("Timed out waiting for signal handler to indicate it finished.");
     }
   } else {
-    BACK_LOGE("Timed out waiting for signal handler to get ucontext data.");
+    // Check to see if the thread has disappeared.
+    if (tgkill(Pid(), Tid(), 0) == -1 && errno == ESRCH) {
+      error_ = BACKTRACE_UNWIND_ERROR_THREAD_DOESNT_EXIST;
+    } else {
+      error_ = BACKTRACE_UNWIND_ERROR_THREAD_TIMEOUT;
+      BACK_LOGE("Timed out waiting for signal handler to get ucontext data.");
+    }
   }
 
   ThreadEntry::Remove(entry);
diff --git a/libbacktrace/BacktraceOffline.cpp b/libbacktrace/BacktraceOffline.cpp
index e84ae74d1..ac1404602 100644
--- a/libbacktrace/BacktraceOffline.cpp
+++ b/libbacktrace/BacktraceOffline.cpp
@@ -129,9 +129,11 @@ static unw_accessors_t accessors = {
 bool BacktraceOffline::Unwind(size_t num_ignore_frames, ucontext_t* context) {
   if (context == nullptr) {
     BACK_LOGW("The context is needed for offline backtracing.");
+    error_ = BACKTRACE_UNWIND_ERROR_NO_CONTEXT;
     return false;
   }
   context_ = context;
+  error_ = BACKTRACE_UNWIND_NO_ERROR;
 
   unw_addr_space_t addr_space = unw_create_addr_space(&accessors, 0);
   unw_cursor_t cursor;
@@ -139,6 +141,7 @@ bool BacktraceOffline::Unwind(size_t num_ignore_frames, ucontext_t* context) {
   if (ret != 0) {
     BACK_LOGW("unw_init_remote failed %d", ret);
     unw_destroy_addr_space(addr_space);
+    error_ = BACKTRACE_UNWIND_ERROR_SETUP_FAILED;
     return false;
   }
   size_t num_frames = 0;
diff --git a/libbacktrace/UnwindCurrent.cpp b/libbacktrace/UnwindCurrent.cpp
index 67e583f08..666c481ce 100644
--- a/libbacktrace/UnwindCurrent.cpp
+++ b/libbacktrace/UnwindCurrent.cpp
@@ -70,6 +70,7 @@ bool UnwindCurrent::UnwindFromContext(size_t num_ignore_frames, ucontext_t* ucon
     int ret = unw_getcontext(&context_);
     if (ret < 0) {
       BACK_LOGW("unw_getcontext failed %d", ret);
+      error_ = BACKTRACE_UNWIND_ERROR_SETUP_FAILED;
       return false;
     }
   } else {
@@ -81,6 +82,7 @@ bool UnwindCurrent::UnwindFromContext(size_t num_ignore_frames, ucontext_t* ucon
   int ret = unw_init_local(cursor.get(), &context_);
   if (ret < 0) {
     BACK_LOGW("unw_init_local failed %d", ret);
+    error_ = BACKTRACE_UNWIND_ERROR_SETUP_FAILED;
     return false;
   }
 
diff --git a/libbacktrace/UnwindPtrace.cpp b/libbacktrace/UnwindPtrace.cpp
index 07c243054..306d2acf0 100644
--- a/libbacktrace/UnwindPtrace.cpp
+++ b/libbacktrace/UnwindPtrace.cpp
@@ -50,17 +50,22 @@ UnwindPtrace::~UnwindPtrace() {
 bool UnwindPtrace::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) {
   if (GetMap() == nullptr) {
     // Without a map object, we can't do anything.
+    error_ = BACKTRACE_UNWIND_ERROR_MAP_MISSING;
     return false;
   }
 
+  error_ = BACKTRACE_UNWIND_NO_ERROR;
+
   if (ucontext) {
     BACK_LOGW("Unwinding from a specified context not supported yet.");
+    error_ = BACKTRACE_UNWIND_ERROR_UNSUPPORTED_OPERATION;
     return false;
   }
 
   addr_space_ = unw_create_addr_space(&_UPT_accessors, 0);
   if (!addr_space_) {
     BACK_LOGW("unw_create_addr_space failed.");
+    error_ = BACKTRACE_UNWIND_ERROR_SETUP_FAILED;
     return false;
   }
 
@@ -70,6 +75,7 @@ bool UnwindPtrace::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) {
   upt_info_ = reinterpret_cast<struct UPT_info*>(_UPT_create(Tid()));
   if (!upt_info_) {
     BACK_LOGW("Failed to create upt info.");
+    error_ = BACKTRACE_UNWIND_ERROR_SETUP_FAILED;
     return false;
   }
 
@@ -77,6 +83,7 @@ bool UnwindPtrace::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) {
   int ret = unw_init_remote(&cursor, addr_space_, upt_info_);
   if (ret < 0) {
     BACK_LOGW("unw_init_remote failed %d", ret);
+    error_ = BACKTRACE_UNWIND_ERROR_SETUP_FAILED;
     return false;
   }
 
diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp
index ab09564c7..df6c6c1fc 100644
--- a/libbacktrace/backtrace_test.cpp
+++ b/libbacktrace/backtrace_test.cpp
@@ -162,6 +162,7 @@ void VerifyLevelBacktrace(void*) {
       Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyLevelDump(backtrace.get());
 }
@@ -183,6 +184,7 @@ void VerifyMaxBacktrace(void*) {
       Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyMaxDump(backtrace.get());
 }
@@ -200,6 +202,7 @@ void VerifyThreadTest(pid_t tid, void (*VerifyFunc)(Backtrace*)) {
   std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), tid));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyFunc(backtrace.get());
 }
@@ -220,6 +223,7 @@ TEST(libbacktrace, local_no_unwind_frames) {
   std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), getpid()));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   ASSERT_TRUE(backtrace->NumFrames() != 0);
   for (const auto& frame : *backtrace ) {
@@ -267,16 +271,19 @@ void VerifyLevelIgnoreFrames(void*) {
       Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(all.get() != nullptr);
   ASSERT_TRUE(all->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, all->GetError());
 
   std::unique_ptr<Backtrace> ign1(
       Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(ign1.get() != nullptr);
   ASSERT_TRUE(ign1->Unwind(1));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, ign1->GetError());
 
   std::unique_ptr<Backtrace> ign2(
       Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(ign2.get() != nullptr);
   ASSERT_TRUE(ign2->Unwind(2));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, ign2->GetError());
 
   VerifyIgnoreFrames(all.get(), ign1.get(), ign2.get(), "VerifyLevelIgnoreFrames");
 }
@@ -314,6 +321,7 @@ void VerifyProcTest(pid_t pid, pid_t tid, bool share_map,
       std::unique_ptr<Backtrace> backtrace(Backtrace::Create(pid, tid, map.get()));
       ASSERT_TRUE(backtrace.get() != nullptr);
       ASSERT_TRUE(backtrace->Unwind(0));
+      ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
       if (ReadyFunc(backtrace.get())) {
         VerifyFunc(backtrace.get());
         verified = true;
@@ -372,10 +380,12 @@ void VerifyProcessIgnoreFrames(Backtrace* bt_all) {
   std::unique_ptr<Backtrace> ign1(Backtrace::Create(bt_all->Pid(), BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(ign1.get() != nullptr);
   ASSERT_TRUE(ign1->Unwind(1));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, ign1->GetError());
 
   std::unique_ptr<Backtrace> ign2(Backtrace::Create(bt_all->Pid(), BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(ign2.get() != nullptr);
   ASSERT_TRUE(ign2->Unwind(2));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, ign2->GetError());
 
   VerifyIgnoreFrames(bt_all, ign1.get(), ign2.get(), nullptr);
 }
@@ -462,6 +472,7 @@ void VerifyLevelThread(void*) {
   std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), gettid()));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyLevelDump(backtrace.get());
 }
@@ -474,6 +485,7 @@ void VerifyMaxThread(void*) {
   std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), gettid()));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyMaxDump(backtrace.get());
 }
@@ -515,6 +527,7 @@ TEST(libbacktrace, thread_level_trace) {
   std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), thread_data.tid));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyLevelDump(backtrace.get());
 
@@ -554,14 +567,17 @@ TEST(libbacktrace, thread_ignore_frames) {
   std::unique_ptr<Backtrace> all(Backtrace::Create(getpid(), thread_data.tid));
   ASSERT_TRUE(all.get() != nullptr);
   ASSERT_TRUE(all->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, all->GetError());
 
   std::unique_ptr<Backtrace> ign1(Backtrace::Create(getpid(), thread_data.tid));
   ASSERT_TRUE(ign1.get() != nullptr);
   ASSERT_TRUE(ign1->Unwind(1));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, ign1->GetError());
 
   std::unique_ptr<Backtrace> ign2(Backtrace::Create(getpid(), thread_data.tid));
   ASSERT_TRUE(ign2.get() != nullptr);
   ASSERT_TRUE(ign2->Unwind(2));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, ign2->GetError());
 
   VerifyIgnoreFrames(all.get(), ign1.get(), ign2.get(), nullptr);
 
@@ -592,6 +608,7 @@ TEST(libbacktrace, thread_max_trace) {
   std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), thread_data.tid));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   VerifyMaxDump(backtrace.get());
 
@@ -718,18 +735,21 @@ TEST(libbacktrace, simultaneous_maps) {
   Backtrace* back1 = Backtrace::Create(getpid(), BACKTRACE_CURRENT_THREAD, map1);
   ASSERT_TRUE(back1 != nullptr);
   EXPECT_TRUE(back1->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, back1->GetError());
   delete back1;
   delete map1;
 
   Backtrace* back2 = Backtrace::Create(getpid(), BACKTRACE_CURRENT_THREAD, map2);
   ASSERT_TRUE(back2 != nullptr);
   EXPECT_TRUE(back2->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, back2->GetError());
   delete back2;
   delete map2;
 
   Backtrace* back3 = Backtrace::Create(getpid(), BACKTRACE_CURRENT_THREAD, map3);
   ASSERT_TRUE(back3 != nullptr);
   EXPECT_TRUE(back3->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, back3->GetError());
   delete back3;
   delete map3;
 }
@@ -1308,6 +1328,7 @@ void VerifyUnreadableElfBacktrace(uintptr_t test_func) {
                                                          BACKTRACE_CURRENT_THREAD));
   ASSERT_TRUE(backtrace.get() != nullptr);
   ASSERT_TRUE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
   size_t frame_num;
   ASSERT_TRUE(FindFuncFrameInBacktrace(backtrace.get(), test_func, &frame_num));
@@ -1364,6 +1385,7 @@ TEST(libbacktrace, unwind_through_unreadable_elf_remote) {
     std::unique_ptr<Backtrace> backtrace(Backtrace::Create(pid, BACKTRACE_CURRENT_THREAD));
     ASSERT_TRUE(backtrace.get() != nullptr);
     ASSERT_TRUE(backtrace->Unwind(0));
+    ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
 
     size_t frame_num;
     if (FindFuncFrameInBacktrace(backtrace.get(),
@@ -1387,6 +1409,14 @@ TEST(libbacktrace, unwind_through_unreadable_elf_remote) {
   ASSERT_TRUE(done) << "Test function never found in unwind.";
 }
 
+TEST(libbacktrace, unwind_thread_doesnt_exist) {
+  std::unique_ptr<Backtrace> backtrace(
+      Backtrace::Create(BACKTRACE_CURRENT_PROCESS, 99999999));
+  ASSERT_TRUE(backtrace.get() != nullptr);
+  ASSERT_FALSE(backtrace->Unwind(0));
+  ASSERT_EQ(BACKTRACE_UNWIND_ERROR_THREAD_DOESNT_EXIST, backtrace->GetError());
+}
+
 #if defined(ENABLE_PSS_TESTS)
 #include "GetPss.h"
 
@@ -1398,6 +1428,7 @@ void CheckForLeak(pid_t pid, pid_t tid) {
     Backtrace* backtrace = Backtrace::Create(pid, tid);
     ASSERT_TRUE(backtrace != nullptr);
     ASSERT_TRUE(backtrace->Unwind(0));
+    ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
     delete backtrace;
   }
   size_t stable_pss = GetPssBytes();
@@ -1408,6 +1439,7 @@ void CheckForLeak(pid_t pid, pid_t tid) {
     Backtrace* backtrace = Backtrace::Create(pid, tid);
     ASSERT_TRUE(backtrace != nullptr);
     ASSERT_TRUE(backtrace->Unwind(0));
+    ASSERT_EQ(BACKTRACE_UNWIND_NO_ERROR, backtrace->GetError());
     delete backtrace;
   }
   size_t new_pss = GetPssBytes();