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

(cherry picked from commit 206a3b9798e3622c906a3cafdb113c271c1c927c)

Change-Id: If9e7cfeb6eb7b122679a734c1a9eacee8354ef18

8 files changed, 111 insertions, 3 deletions

diff --git a/debuggerd/backtrace.cpp b/debuggerd/backtrace.cpp
index b6916e5b4..32843d860 100644
--- a/debuggerd/backtrace.cpp
+++ b/debuggerd/backtrace.cpp
@@ -91,7 +91,8 @@ static void dump_thread(log_t* log, BacktraceMap* map, pid_t pid, pid_t tid) {
   if (backtrace->Unwind(0)) {
     dump_backtrace_to_log(backtrace.get(), log, "  ");
   } else {
-    ALOGE("Unwind failed: tid = %d", tid);
+    ALOGE("Unwind failed: tid = %d: %s", tid,
+          backtrace->GetErrorString(backtrace->GetError()).c_str());
   }
 }
 
diff --git a/include/backtrace/Backtrace.h b/include/backtrace/Backtrace.h
index f440bd283..c896ab806 100644
--- a/include/backtrace/Backtrace.h
+++ b/include/backtrace/Backtrace.h
@@ -34,6 +34,24 @@ typedef uint64_t word_t;
 typedef uint32_t word_t;
 #endif
 
+enum BacktraceUnwindError : uint32_t {
+  BACKTRACE_UNWIND_NO_ERROR,
+  // Something failed while trying to perform the setup to begin the unwind.
+  BACKTRACE_UNWIND_ERROR_SETUP_FAILED,
+  // There is no map information to use with the unwind.
+  BACKTRACE_UNWIND_ERROR_MAP_MISSING,
+  // An error occurred that indicates a programming error.
+  BACKTRACE_UNWIND_ERROR_INTERNAL,
+  // The thread to unwind has disappeared before the unwind can begin.
+  BACKTRACE_UNWIND_ERROR_THREAD_DOESNT_EXIST,
+  // The thread to unwind has not responded to a signal in a timely manner.
+  BACKTRACE_UNWIND_ERROR_THREAD_TIMEOUT,
+  // Attempt to do an unsupported operation.
+  BACKTRACE_UNWIND_ERROR_UNSUPPORTED_OPERATION,
+  // Attempt to do an offline unwind without a context.
+  BACKTRACE_UNWIND_ERROR_NO_CONTEXT,
+};
+
 struct backtrace_frame_data_t {
   size_t num;             // The current fame number.
   uintptr_t pc;           // The absolute pc.
@@ -127,6 +145,10 @@ public:
 
   BacktraceMap* GetMap() { return map_; }
 
+  BacktraceUnwindError GetError() { return error_; }
+
+  std::string GetErrorString(BacktraceUnwindError error);
+
 protected:
   Backtrace(pid_t pid, pid_t tid, BacktraceMap* map);
 
@@ -145,6 +167,8 @@ protected:
   bool map_shared_;
 
   std::vector<backtrace_frame_data_t> frames_;
+
+  BacktraceUnwindError error_;
 };
 
 #endif // _BACKTRACE_BACKTRACE_H
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 7d829fe74..f6b2591a2 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);
 }
@@ -463,6 +473,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());
 }
@@ -475,6 +486,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());
 }
@@ -516,6 +528,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());
 
@@ -555,14 +568,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);
 
@@ -593,6 +609,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());
 
@@ -719,18 +736,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;
 }
@@ -1309,6 +1329,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));
@@ -1365,6 +1386,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(),
@@ -1388,6 +1410,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"
 
@@ -1399,6 +1429,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();
@@ -1409,6 +1440,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();