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

Commit 7c2e7e31 authored by Christopher Ferris's avatar Christopher Ferris
Browse files

Fix fallback paths for dumping threads. 

In the fallback path, if the non-main thread is the target
to be dumped, then no other threads are dumped when creating
a tombstone. Fix this and add unit tests to verify that
this all threads, including the main thread are dumped.

Bug: 234058038

Test: All unit tests pass.
Test: debuggerd -b media.swcodec process
Test: debuggerd media.swcodec process
Change-Id: Ibb75264f7b3847acdbab939a66902d986c0d0e5c
parent ab2d6cdc

debuggerd/debuggerd_test.cpp

+65 −0
Original line number Diff line number Diff line
@@ -1486,6 +1486,37 @@ TEST_F(CrasherTest, seccomp_tombstone_thread_abort) { 
  ASSERT_BACKTRACE_FRAME(result, "abort");

}



TEST_F(CrasherTest, seccomp_tombstone_multiple_threads_abort) {

  int intercept_result;

  unique_fd output_fd;



  static const auto dump_type = kDebuggerdTombstone;

  StartProcess(

      []() {

        std::thread a(foo);

        std::thread b(bar);



        std::this_thread::sleep_for(100ms);



        std::thread abort_thread([] { abort(); });

        abort_thread.join();

      },

      &seccomp_fork);



  StartIntercept(&output_fd, dump_type);

  FinishCrasher();

  AssertDeath(SIGABRT);

  FinishIntercept(&intercept_result);

  ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";



  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ASSERT_BACKTRACE_FRAME(result, "abort");

  ASSERT_BACKTRACE_FRAME(result, "foo");

  ASSERT_BACKTRACE_FRAME(result, "bar");

  ASSERT_BACKTRACE_FRAME(result, "main");

}



TEST_F(CrasherTest, seccomp_backtrace) {

  int intercept_result;

  unique_fd output_fd;
@@ -1516,6 +1547,40 @@ TEST_F(CrasherTest, seccomp_backtrace) { 
  ASSERT_BACKTRACE_FRAME(result, "bar");

}



TEST_F(CrasherTest, seccomp_backtrace_from_thread) {

  int intercept_result;

  unique_fd output_fd;



  static const auto dump_type = kDebuggerdNativeBacktrace;

  StartProcess(

      []() {

        std::thread a(foo);

        std::thread b(bar);



        std::this_thread::sleep_for(100ms);



        std::thread raise_thread([] {

          raise_debugger_signal(dump_type);

          _exit(0);

        });

        raise_thread.join();

      },

      &seccomp_fork);



  StartIntercept(&output_fd, dump_type);

  FinishCrasher();

  AssertDeath(0);

  FinishIntercept(&intercept_result);

  ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";



  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");

  ASSERT_BACKTRACE_FRAME(result, "foo");

  ASSERT_BACKTRACE_FRAME(result, "bar");

  ASSERT_BACKTRACE_FRAME(result, "main");

}



TEST_F(CrasherTest, seccomp_crash_logcat) {

  StartProcess([]() { abort(); }, &seccomp_fork);

  FinishCrasher();

debuggerd/handler/debuggerd_fallback.cpp

+4 −1
Original line number Diff line number Diff line
@@ -210,7 +210,10 @@ static void trace_handler(siginfo_t* info, ucontext_t* ucontext) { 


  // Send a signal to all of our siblings, asking them to dump their stack.

  pid_t current_tid = gettid();

  if (!iterate_tids(current_tid, [&output_fd](pid_t tid) {

  if (!iterate_tids(current_tid, [&output_fd, &current_tid](pid_t tid) {

        if (current_tid == tid) {

          return;

        }

        // Use a pipe, to be able to detect situations where the thread gracefully exits before

        // receiving our signal.

        unique_fd pipe_read, pipe_write;

debuggerd/libdebuggerd/tombstone.cpp

+28 −26
Original line number Diff line number Diff line
@@ -55,15 +55,15 @@ void engrave_tombstone_ucontext(int tombstone_fd, int proto_fd, uint64_t abort_m 
                                siginfo_t* siginfo, ucontext_t* ucontext) {

  pid_t uid = getuid();

  pid_t pid = getpid();

  pid_t tid = gettid();

  pid_t target_tid = gettid();



  log_t log;

  log.current_tid = tid;

  log.crashed_tid = tid;

  log.current_tid = target_tid;

  log.crashed_tid = target_tid;

  log.tfd = tombstone_fd;

  log.amfd_data = nullptr;



  std::string thread_name = get_thread_name(tid);

  std::string thread_name = get_thread_name(target_tid);

  std::vector<std::string> command_line = get_command_line(pid);



  std::unique_ptr<unwindstack::Regs> regs(
@@ -73,19 +73,22 @@ void engrave_tombstone_ucontext(int tombstone_fd, int proto_fd, uint64_t abort_m 
  android::base::ReadFileToString("/proc/self/attr/current", &selinux_label);



  std::map<pid_t, ThreadInfo> threads;

  threads[tid] = ThreadInfo{

    .registers = std::move(regs), .uid = uid, .tid = tid, .thread_name = std::move(thread_name),

    .pid = pid, .command_line = std::move(command_line), .selinux_label = std::move(selinux_label),

    .siginfo = siginfo,

  threads[target_tid] = ThreadInfo {

    .registers = std::move(regs), .uid = uid, .tid = target_tid,

    .thread_name = std::move(thread_name), .pid = pid, .command_line = std::move(command_line),

    .selinux_label = std::move(selinux_label), .siginfo = siginfo,

#if defined(__aarch64__)

    // Only supported on aarch64 for now.

        .tagged_addr_ctrl = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0),

    .pac_enabled_keys = prctl(PR_PAC_GET_ENABLED_KEYS, 0, 0, 0, 0),

#endif

  };

  if (pid == tid) {

  const ThreadInfo& thread = threads[pid];

    if (!iterate_tids(pid, [&threads, &thread](pid_t tid) {

  if (!iterate_tids(pid, [&threads, &thread, &target_tid](pid_t tid) {

        if (target_tid == tid) {

          return;

        }

        async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "Adding thread %d", tid);

        threads[tid] = ThreadInfo{

            .uid = thread.uid,

            .tid = tid,
@@ -99,7 +102,6 @@ void engrave_tombstone_ucontext(int tombstone_fd, int proto_fd, uint64_t abort_m 
    async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to open /proc/%d/task: %s", pid,

                          strerror(errno));

  }

  }



  // Do not use the thread cache here because it will call pthread_key_create

  // which doesn't work in linker code. See b/189803009.
@@ -116,8 +118,8 @@ void engrave_tombstone_ucontext(int tombstone_fd, int proto_fd, uint64_t abort_m 


  ProcessInfo process_info;

  process_info.abort_msg_address = abort_msg_address;

  engrave_tombstone(unique_fd(dup(tombstone_fd)), unique_fd(dup(proto_fd)), &unwinder, threads, tid,

                    process_info, nullptr, nullptr);

  engrave_tombstone(unique_fd(dup(tombstone_fd)), unique_fd(dup(proto_fd)), &unwinder, threads,

                    target_tid, process_info, nullptr, nullptr);

}



void engrave_tombstone(unique_fd output_fd, unique_fd proto_fd,

debuggerd/util.cpp

+1 −3
Original line number Diff line number Diff line
@@ -90,9 +90,7 @@ bool iterate_tids(pid_t pid, std::function<void(pid_t)> callback) { 
    if (tid == 0) {

      continue;

    }

    if (pid != tid) {

    callback(tid);

  }

  }

  return true;

}