Merge "Always use main thread pid for manual dumping." am: d874398501 (7e91d810) · Commits · e / os / android_system_core

debuggerd/debuggerd.cpp

+18 −4

Original line number	Diff line number	Diff line
		@@ -93,8 +93,18 @@ int main(int argc, char* argv[]) {
		errx(1, "process %d is a zombie", pid);
		}

		if (kill(pid, 0) != 0) {
		// Send a signal to the main thread pid, not a side thread. The signal
		// handler always sets the crashing tid to the main thread pid when sent this
		// signal. This is to avoid a problem where the signal is sent to a process,
		// but happens on a side thread and the intercept mismatches since it
		// is looking for the main thread pid, not the tid of this random thread.
		// See b/194346289 for extra details.
		if (kill(proc_info.pid, 0) != 0) {
		if (pid == proc_info.pid) {
		err(1, "cannot send signal to process %d", pid);
		} else {
		err(1, "cannot send signal to main thread %d (requested thread %d)", proc_info.pid, pid);
		}
		}

		unique_fd piperead, pipewrite;
		@@ -103,9 +113,13 @@ int main(int argc, char* argv[]) {
		}

		std::thread redirect_thread = spawn_redirect_thread(std::move(piperead));
		if (!debuggerd_trigger_dump(pid, dump_type, 0, std::move(pipewrite))) {
		if (!debuggerd_trigger_dump(proc_info.pid, dump_type, 0, std::move(pipewrite))) {
		redirect_thread.join();
		if (pid == proc_info.pid) {
		errx(1, "failed to dump process %d", pid);
		} else {
		errx(1, "failed to dump main thread %d (requested thread %d)", proc_info.pid, pid);
		}
		}

		redirect_thread.join();

debuggerd/debuggerd_test.cpp

+26 −0

Original line number	Diff line number	Diff line
		@@ -1825,3 +1825,29 @@ TEST(tombstoned, proto_intercept) {
		ASSERT_TRUE(android::base::ReadFdToString(output_fd, &output));
		ASSERT_EQ("foo", output);
		}

		// Verify that when an intercept is present for the main thread, and the signal
		// is received on a different thread, the intercept still works.
		TEST_F(CrasherTest, intercept_for_main_thread_signal_on_side_thread) {
		StartProcess([]() {
		std::thread thread([]() {
		// Raise the signal on the side thread.
		raise_debugger_signal(kDebuggerdNativeBacktrace);
		});
		thread.join();
		_exit(0);
		});

		unique_fd output_fd;
		StartIntercept(&output_fd, kDebuggerdNativeBacktrace);
		FinishCrasher();
		AssertDeath(0);

		int intercept_result;
		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");
		}

debuggerd/handler/debuggerd_handler.cpp

+18 −13

Original line number	Diff line number	Diff line
		@@ -155,18 +155,14 @@ static bool get_main_thread_name(char* buf, size_t len) {
		* could allocate memory or hold a lock.
		*/
		static void log_signal_summary(const siginfo_t* info) {
		char thread_name[MAX_TASK_NAME_LEN + 1]; // one more for termination
		if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(thread_name), 0, 0, 0) != 0) {
		strcpy(thread_name, "<name unknown>");
		} else {
		// short names are null terminated by prctl, but the man page
		// implies that 16 byte names are not.
		thread_name[MAX_TASK_NAME_LEN] = 0;
		char main_thread_name[MAX_TASK_NAME_LEN + 1];
		if (!get_main_thread_name(main_thread_name, sizeof(main_thread_name))) {
		strncpy(main_thread_name, "<unknown>", sizeof(main_thread_name));
		}

		if (info->si_signo == BIONIC_SIGNAL_DEBUGGER) {
		async_safe_format_log(ANDROID_LOG_INFO, "libc", "Requested dump for tid %d (%s)", __gettid(),
		thread_name);
		async_safe_format_log(ANDROID_LOG_INFO, "libc", "Requested dump for pid %d (%s)", __getpid(),
		main_thread_name);
		return;
		}

		@@ -181,9 +177,13 @@ static void log_signal_summary(const siginfo_t* info) {
		get_signal_sender(sender_desc, sizeof(sender_desc), info);
		}

		char main_thread_name[MAX_TASK_NAME_LEN + 1];
		if (!get_main_thread_name(main_thread_name, sizeof(main_thread_name))) {
		strncpy(main_thread_name, "<unknown>", sizeof(main_thread_name));
		char thread_name[MAX_TASK_NAME_LEN + 1]; // one more for termination
		if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(thread_name), 0, 0, 0) != 0) {
		strcpy(thread_name, "<name unknown>");
		} else {
		// short names are null terminated by prctl, but the man page
		// implies that 16 byte names are not.
		thread_name[MAX_TASK_NAME_LEN] = 0;
		}

		async_safe_format_log(ANDROID_LOG_FATAL, "libc",
		@@ -532,8 +532,13 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c

		log_signal_summary(info);

		// If we got here due to the signal BIONIC_SIGNAL_DEBUGGER, it's possible
		// this is not the main thread, which can cause the intercept logic to fail
		// since the intercept is only looking for the main thread. In this case,
		// setting crashing_tid to pid instead of the current thread's tid avoids
		// the problem.
		debugger_thread_info thread_info = {
		.crashing_tid = __gettid(),
		.crashing_tid = (signal_number == BIONIC_SIGNAL_DEBUGGER) ? __getpid() : __gettid(),
		.pseudothread_tid = -1,
		.siginfo = info,
		.ucontext = context,