Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5fd02a8d authored by Josh Gao's avatar Josh Gao
Browse files

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
(cherry picked from commit e06f2a48)
parent 704b27bd

debuggerd/debuggerd_test.cpp

+35 −5
Original line number Diff line number Diff line
@@ -78,6 +78,14 @@ constexpr char kWaitForGdbKey[] = "debug.debuggerd.wait_for_gdb"; 
    }                                                                           \

  } 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 @@ void CrasherTest::AssertDeath(int signo) { 
    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));

  }

  crasher_pid = -1;

}
@@ -336,6 +346,26 @@ TEST_F(CrasherTest, abort_message) { 
  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

+3 −2
Original line number Diff line number Diff line
@@ -389,8 +389,9 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c 


  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;

  }