diff --git a/debuggerd/common/include/dump_type.h b/debuggerd/common/include/dump_type.h index a3e171b25..203269e96 100644 --- a/debuggerd/common/include/dump_type.h +++ b/debuggerd/common/include/dump_type.h @@ -24,8 +24,7 @@ enum DebuggerdDumpType : uint8_t { kDebuggerdNativeBacktrace, kDebuggerdTombstone, kDebuggerdJavaBacktrace, - kDebuggerdAnyIntercept, - kDebuggerdTombstoneProto, + kDebuggerdAnyIntercept }; inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& rhs) { @@ -42,9 +41,6 @@ inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& r case kDebuggerdAnyIntercept: stream << "kDebuggerdAnyIntercept"; break; - case kDebuggerdTombstoneProto: - stream << "kDebuggerdTombstoneProto"; - break; default: stream << "[unknown]"; } diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index b9d66063e..191931da6 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -1425,70 +1425,3 @@ TEST_F(CrasherTest, stack_overflow) { ConsumeFd(std::move(output_fd), &result); ASSERT_MATCH(result, R"(Cause: stack pointer[^\n]*stack overflow.\n)"); } - -TEST(tombstoned, proto) { - const pid_t self = getpid(); - unique_fd tombstoned_socket, text_fd, proto_fd; - ASSERT_TRUE( - tombstoned_connect(self, &tombstoned_socket, &text_fd, &proto_fd, kDebuggerdTombstoneProto)); - - tombstoned_notify_completion(tombstoned_socket.get()); - - ASSERT_NE(-1, text_fd.get()); - ASSERT_NE(-1, proto_fd.get()); - - struct stat text_st; - ASSERT_EQ(0, fstat(text_fd.get(), &text_st)); - - // Give tombstoned some time to link the files into place. - std::this_thread::sleep_for(100ms); - - // Find the tombstone. - std::optional tombstone_index; - for (int i = 0; i < 50; ++i) { - std::string path = android::base::StringPrintf("/data/tombstones/tombstone_%02d", i); - - struct stat st; - if (TEMP_FAILURE_RETRY(stat(path.c_str(), &st)) != 0) { - continue; - } - - if (st.st_dev == text_st.st_dev && st.st_ino == text_st.st_ino) { - tombstone_index = i; - break; - } - } - - ASSERT_TRUE(tombstone_index); - std::string proto_path = - android::base::StringPrintf("/data/tombstones/tombstone_%02d.pb", *tombstone_index); - - struct stat proto_fd_st; - struct stat proto_file_st; - ASSERT_EQ(0, fstat(proto_fd.get(), &proto_fd_st)); - ASSERT_EQ(0, stat(proto_path.c_str(), &proto_file_st)); - - ASSERT_EQ(proto_fd_st.st_dev, proto_file_st.st_dev); - ASSERT_EQ(proto_fd_st.st_ino, proto_file_st.st_ino); -} - -TEST(tombstoned, proto_intercept) { - const pid_t self = getpid(); - unique_fd intercept_fd, output_fd; - InterceptStatus status; - - tombstoned_intercept(self, &intercept_fd, &output_fd, &status, kDebuggerdTombstone); - ASSERT_EQ(InterceptStatus::kRegistered, status); - - unique_fd tombstoned_socket, text_fd, proto_fd; - ASSERT_TRUE( - tombstoned_connect(self, &tombstoned_socket, &text_fd, &proto_fd, kDebuggerdTombstoneProto)); - ASSERT_TRUE(android::base::WriteStringToFd("foo", text_fd.get())); - tombstoned_notify_completion(tombstoned_socket.get()); - - text_fd.reset(); - - std::string output; - ASSERT_TRUE(android::base::ReadFdToString(output_fd, &output)); - ASSERT_EQ("foo", output); -} diff --git a/debuggerd/tombstoned/include/tombstoned/tombstoned.h b/debuggerd/tombstoned/include/tombstoned/tombstoned.h index bff216c93..6403dbec4 100644 --- a/debuggerd/tombstoned/include/tombstoned/tombstoned.h +++ b/debuggerd/tombstoned/include/tombstoned/tombstoned.h @@ -23,10 +23,6 @@ #include "dump_type.h" bool tombstoned_connect(pid_t pid, android::base::unique_fd* tombstoned_socket, - android::base::unique_fd* text_output_fd, - android::base::unique_fd* proto_output_fd, DebuggerdDumpType dump_type); - -bool tombstoned_connect(pid_t pid, android::base::unique_fd* tombstoned_socket, - android::base::unique_fd* text_output_fd, DebuggerdDumpType dump_type); + android::base::unique_fd* output_fd, DebuggerdDumpType dump_type); bool tombstoned_notify_completion(int tombstoned_socket); diff --git a/debuggerd/tombstoned/intercept_manager.cpp b/debuggerd/tombstoned/intercept_manager.cpp index 437639e3c..6cbf12c83 100644 --- a/debuggerd/tombstoned/intercept_manager.cpp +++ b/debuggerd/tombstoned/intercept_manager.cpp @@ -191,18 +191,6 @@ InterceptManager::InterceptManager(event_base* base, int intercept_socket) : bas /* backlog */ -1, intercept_socket); } -bool dump_types_compatible(DebuggerdDumpType interceptor, DebuggerdDumpType dump) { - if (interceptor == dump) { - return true; - } - - if (interceptor == kDebuggerdTombstone && dump == kDebuggerdTombstoneProto) { - return true; - } - - return false; -} - bool InterceptManager::GetIntercept(pid_t pid, DebuggerdDumpType dump_type, android::base::unique_fd* out_fd) { auto it = this->intercepts.find(pid); @@ -213,7 +201,7 @@ bool InterceptManager::GetIntercept(pid_t pid, DebuggerdDumpType dump_type, if (dump_type == kDebuggerdAnyIntercept) { LOG(INFO) << "found registered intercept of type " << it->second->dump_type << " for requested type kDebuggerdAnyIntercept"; - } else if (!dump_types_compatible(it->second->dump_type, dump_type)) { + } else if (it->second->dump_type != dump_type) { LOG(WARNING) << "found non-matching intercept of type " << it->second->dump_type << " for requested type: " << dump_type; return false; diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp index f057260e1..2b74fd489 100644 --- a/debuggerd/tombstoned/tombstoned.cpp +++ b/debuggerd/tombstoned/tombstoned.cpp @@ -62,22 +62,14 @@ enum CrashStatus { struct CrashArtifact { unique_fd fd; std::optional temporary_path; - - static CrashArtifact devnull() { - CrashArtifact result; - result.fd.reset(open("/dev/null", O_WRONLY | O_CLOEXEC)); - return result; - } }; struct CrashArtifactPaths { std::string text; - std::optional proto; }; struct CrashOutput { CrashArtifact text; - std::optional proto; }; // Ownership of Crash is a bit messy. @@ -97,15 +89,14 @@ struct Crash { class CrashQueue { public: CrashQueue(const std::string& dir_path, const std::string& file_name_prefix, size_t max_artifacts, - size_t max_concurrent_dumps, bool supports_proto) + size_t max_concurrent_dumps) : file_name_prefix_(file_name_prefix), dir_path_(dir_path), dir_fd_(open(dir_path.c_str(), O_DIRECTORY | O_RDONLY | O_CLOEXEC)), max_artifacts_(max_artifacts), next_artifact_(0), max_concurrent_dumps_(max_concurrent_dumps), - num_concurrent_dumps_(0), - supports_proto_(supports_proto) { + num_concurrent_dumps_(0) { if (dir_fd_ == -1) { PLOG(FATAL) << "failed to open directory: " << dir_path; } @@ -128,14 +119,14 @@ class CrashQueue { static CrashQueue* for_tombstones() { static CrashQueue queue("/data/tombstones", "tombstone_" /* file_name_prefix */, GetIntProperty("tombstoned.max_tombstone_count", 32), - 1 /* max_concurrent_dumps */, true /* supports_proto */); + 1 /* max_concurrent_dumps */); return &queue; } static CrashQueue* for_anrs() { static CrashQueue queue("/data/anr", "trace_" /* file_name_prefix */, GetIntProperty("tombstoned.max_anr_count", 64), - 4 /* max_concurrent_dumps */, false /* supports_proto */); + 4 /* max_concurrent_dumps */); return &queue; } @@ -169,15 +160,6 @@ class CrashQueue { // Don't generate tombstones for backtrace requests. return {}; - case kDebuggerdTombstoneProto: - if (!supports_proto_) { - LOG(ERROR) << "received kDebuggerdTombstoneProto on a queue that doesn't support proto"; - return {}; - } - result.proto = create_temporary_file(); - result.text = create_temporary_file(); - break; - case kDebuggerdTombstone: result.text = create_temporary_file(); break; @@ -196,10 +178,6 @@ class CrashQueue { CrashArtifactPaths result; result.text = StringPrintf("%s%02d", file_name_prefix_.c_str(), next_artifact_); - if (supports_proto_) { - result.proto = StringPrintf("%s%02d.pb", file_name_prefix_.c_str(), next_artifact_); - } - next_artifact_ = (next_artifact_ + 1) % max_artifacts_; return result; } @@ -265,8 +243,6 @@ class CrashQueue { const size_t max_concurrent_dumps_; size_t num_concurrent_dumps_; - bool supports_proto_; - std::deque> queued_requests_; DISALLOW_COPY_AND_ASSIGN(CrashQueue); @@ -285,11 +261,7 @@ static void perform_request(std::unique_ptr crash) { unique_fd output_fd; bool intercepted = intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd); - if (intercepted) { - if (crash->crash_type == kDebuggerdTombstoneProto) { - crash->output.proto = CrashArtifact::devnull(); - } - } else { + if (!intercepted) { if (auto o = CrashQueue::for_crash(crash.get())->get_output(crash->crash_type); o) { crash->output = std::move(*o); output_fd.reset(dup(crash->output.text.fd)); @@ -301,13 +273,8 @@ static void perform_request(std::unique_ptr crash) { TombstonedCrashPacket response = {.packet_type = CrashPacketType::kPerformDump}; - ssize_t rc = -1; - if (crash->output.proto) { - rc = SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get(), - crash->output.proto->fd.get()); - } else { - rc = SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get()); - } + ssize_t rc = + SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get()); output_fd.reset(); @@ -376,7 +343,7 @@ static void crash_request_cb(evutil_socket_t sockfd, short ev, void* arg) { } crash->crash_type = request.packet.dump_request.dump_type; - if (crash->crash_type < 0 || crash->crash_type > kDebuggerdTombstoneProto) { + if (crash->crash_type < 0 || crash->crash_type > kDebuggerdAnyIntercept) { LOG(WARNING) << "unexpected crash dump type: " << crash->crash_type; return; } @@ -471,14 +438,6 @@ static void crash_completed(borrowed_fd sockfd, std::unique_ptr crash) { } } - if (crash->output.proto && crash->output.proto->fd != -1) { - if (!paths.proto) { - LOG(ERROR) << "missing path for proto tombstone"; - } else if (!link_fd(crash->output.proto->fd, queue->dir_fd(), *paths.proto)) { - LOG(ERROR) << "failed to link proto tombstone"; - } - } - // If we don't have O_TMPFILE, we need to clean up after ourselves. if (crash->output.text.temporary_path) { rc = unlinkat(queue->dir_fd().get(), crash->output.text.temporary_path->c_str(), 0); @@ -486,12 +445,6 @@ static void crash_completed(borrowed_fd sockfd, std::unique_ptr crash) { PLOG(ERROR) << "failed to unlink temporary tombstone at " << paths.text; } } - if (crash->output.proto && crash->output.proto->temporary_path) { - rc = unlinkat(queue->dir_fd().get(), crash->output.proto->temporary_path->c_str(), 0); - if (rc != 0) { - PLOG(ERROR) << "failed to unlink temporary proto tombstone"; - } - } } static void crash_completed_cb(evutil_socket_t sockfd, short ev, void* arg) { diff --git a/debuggerd/tombstoned/tombstoned_client.cpp b/debuggerd/tombstoned/tombstoned_client.cpp index abfafb152..2c23c98f3 100644 --- a/debuggerd/tombstoned/tombstoned_client.cpp +++ b/debuggerd/tombstoned/tombstoned_client.cpp @@ -32,13 +32,8 @@ using android::base::ReceiveFileDescriptors; using android::base::unique_fd; -bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* text_output_fd, +bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* output_fd, DebuggerdDumpType dump_type) { - return tombstoned_connect(pid, tombstoned_socket, text_output_fd, nullptr, dump_type); -} - -bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* text_output_fd, - unique_fd* proto_output_fd, DebuggerdDumpType dump_type) { unique_fd sockfd( socket_local_client((dump_type != kDebuggerdJavaBacktrace ? kTombstonedCrashSocketName : kTombstonedJavaTraceSocketName), @@ -59,15 +54,8 @@ bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* text return false; } - unique_fd tmp_output_fd, tmp_proto_fd; - ssize_t rc = -1; - - if (dump_type == kDebuggerdTombstoneProto) { - rc = ReceiveFileDescriptors(sockfd, &packet, sizeof(packet), &tmp_output_fd, &tmp_proto_fd); - } else { - rc = ReceiveFileDescriptors(sockfd, &packet, sizeof(packet), &tmp_output_fd); - } - + unique_fd tmp_output_fd; + ssize_t rc = ReceiveFileDescriptors(sockfd, &packet, sizeof(packet), &tmp_output_fd); if (rc == -1) { async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to read response to DumpRequest packet: %s", strerror(errno)); @@ -90,10 +78,7 @@ bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* text } *tombstoned_socket = std::move(sockfd); - *text_output_fd = std::move(tmp_output_fd); - if (proto_output_fd) { - *proto_output_fd = std::move(tmp_proto_fd); - } + *output_fd = std::move(tmp_output_fd); return true; }