Revert "tombstoned: support for protobuf fds."
Revert "Let crash_dump read /proc/$PID."
Revert submission 1556807-tombstone_proto
Reason for revert: b/178455196, Broken test: android.seccomp.cts.SeccompHostJUnit4DeviceTest#testAppZygoteSyscalls on git_master on cf_x86_64_phone-userdebug
Reverted Changes:
Ide6811297:tombstoned: switch from goto to RAII.
I8d285c4b4:tombstoned: make it easier to add more types of ou...
Id0f0fa285:tombstoned: support for protobuf fds.
I6be6082ab:Let crash_dump read /proc/$PID.
Id812ca390:Make protobuf vendor_ramdisk_available.
Ieeece6e6d:libdebuggerd: add protobuf implementation.
Change-Id: I0c4f3a17e8b06d6c65255388c571ebf11d371dbb
diff --git a/debuggerd/common/include/dump_type.h b/debuggerd/common/include/dump_type.h
index a3e171b..203269e 100644
--- a/debuggerd/common/include/dump_type.h
+++ b/debuggerd/common/include/dump_type.h
@@ -24,8 +24,7 @@
kDebuggerdNativeBacktrace,
kDebuggerdTombstone,
kDebuggerdJavaBacktrace,
- kDebuggerdAnyIntercept,
- kDebuggerdTombstoneProto,
+ kDebuggerdAnyIntercept
};
inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& rhs) {
@@ -42,9 +41,6 @@
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 b9d6606..191931d 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -1425,70 +1425,3 @@
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<int> 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 bff216c..6403dbe 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 437639e..6cbf12c 100644
--- a/debuggerd/tombstoned/intercept_manager.cpp
+++ b/debuggerd/tombstoned/intercept_manager.cpp
@@ -191,18 +191,6 @@
/* 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 @@
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 f057260..2b74fd4 100644
--- a/debuggerd/tombstoned/tombstoned.cpp
+++ b/debuggerd/tombstoned/tombstoned.cpp
@@ -62,22 +62,14 @@
struct CrashArtifact {
unique_fd fd;
std::optional<std::string> 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<std::string> proto;
};
struct CrashOutput {
CrashArtifact text;
- std::optional<CrashArtifact> proto;
};
// Ownership of Crash is a bit messy.
@@ -97,15 +89,14 @@
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 @@
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 @@
// 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 @@
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 @@
const size_t max_concurrent_dumps_;
size_t num_concurrent_dumps_;
- bool supports_proto_;
-
std::deque<std::unique_ptr<Crash>> queued_requests_;
DISALLOW_COPY_AND_ASSIGN(CrashQueue);
@@ -285,11 +261,7 @@
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 @@
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 @@
}
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 @@
}
}
- 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 @@
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 abfafb1..2c23c98 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 @@
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 @@
}
*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;
}