commit d71da00cc0e6fdebae28365e649403d39ea022de
author Josh Gao <jmgao@google.com> Thu Apr 27 16:50:38 2017 -0700
committer Josh Gao <jmgao@google.com> Thu Apr 27 17:28:05 2017 -0700
tree 2c11a30e50b1da37b49b058f27194faa24de144a
parent 829d8d0c7711424f3c1e6393aff9005f8062bc58

debuggerd_handler: don't assume that abort message implies fatal.

Applications can set abort messages via android_set_abort_message
without actually aborting. This leads to following non-fatal dumps
printing their output to logcat in the same format as a regular crash.

Bug: http://b/37754992
Test: debuggerd_test
Change-Id: I9c5e942984dfda36448860202b0ff1c2950bdd07

debuggerd/debuggerd_test.cpp

diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index b705e27..568879e 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -78,6 +78,14 @@
     }                                                                           \
   } while (0)
 
+#define ASSERT_NOT_MATCH(str, pattern)                                                      \
+  do {                                                                                      \
+    std::regex r((pattern));                                                                \
+    if (std::regex_search((str), r)) {                                                      \
+      FAIL() << "regex mismatch: expected to not find " << (pattern) << " in: \n" << (str); \
+    }                                                                                       \
+  } while (0)
+
 static void tombstoned_intercept(pid_t target_pid, unique_fd* intercept_fd, unique_fd* output_fd) {
   intercept_fd->reset(socket_local_client(kTombstonedInterceptSocketName,
                                           ANDROID_SOCKET_NAMESPACE_RESERVED, SOCK_SEQPACKET));
@@ -226,12 +234,14 @@
     FAIL() << "failed to wait for crasher: " << strerror(errno);
   }
 
-  if (WIFEXITED(status)) {
-    FAIL() << "crasher failed to exec: " << strerror(WEXITSTATUS(status));
-  } else if (!WIFSIGNALED(status)) {
-    FAIL() << "crasher didn't terminate via a signal";
+  if (signo == 0) {
+    ASSERT_TRUE(WIFEXITED(status));
+    ASSERT_EQ(0, WEXITSTATUS(signo));
+  } else {
+    ASSERT_FALSE(WIFEXITED(status));
+    ASSERT_TRUE(WIFSIGNALED(status)) << "crasher didn't terminate via a signal";
+    ASSERT_EQ(signo, WTERMSIG(status));
   }
-  ASSERT_EQ(signo, WTERMSIG(status));
   crasher_pid = -1;
 }
 
@@ -336,6 +346,26 @@
   ASSERT_MATCH(result, R"(Abort message: 'abort message goes here')");
 }
 
+TEST_F(CrasherTest, abort_message_backtrace) {
+  int intercept_result;
+  unique_fd output_fd;
+  StartProcess([]() {
+    android_set_abort_message("not actually aborting");
+    raise(DEBUGGER_SIGNAL);
+    exit(0);
+  });
+  StartIntercept(&output_fd);
+  FinishCrasher();
+  AssertDeath(0);
+  FinishIntercept(&intercept_result);
+
+  ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
+
+  std::string result;
+  ConsumeFd(std::move(output_fd), &result);
+  ASSERT_NOT_MATCH(result, R"(Abort message:)");
+}
+
 TEST_F(CrasherTest, intercept_timeout) {
   int intercept_result;
   unique_fd output_fd;

debuggerd/handler/debuggerd_handler.cpp

diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp
index cd00dc5..b70554f 100644
--- a/debuggerd/handler/debuggerd_handler.cpp
+++ b/debuggerd/handler/debuggerd_handler.cpp
@@ -389,8 +389,9 @@
 
   log_signal_summary(signal_number, info);
 
-  // Populate si_value with the abort message address, if found.
-  if (abort_message) {
+  // If this was a fatal crash, populate si_value with the abort message address if possible.
+  // Note that applications can set an abort message without aborting.
+  if (abort_message && signal_number != DEBUGGER_SIGNAL) {
     info->si_value.sival_ptr = abort_message;
   }