heapprofd: fork mode: pass cmdline as a flag to heapprofd
Client now reads (and normalizes) /proc/own_pid/cmdline, passing it as a
separate flag to heapprofd. The forked heapprofd is otherwise unable to access
the target's cmdline entry due to hidepid.
Note also that this is making the client give up if it's unable to parse the
cmdline. Previously we would still try to carry on just using the pid (so
by-cmdline configs wouldn't work, but by-pid would).
Bug: 120185246
Change-Id: I849c4c18cd68e290077fe2633c851eb4fefd9fef
diff --git a/Android.bp b/Android.bp
index 77bcbbd..ec143a5 100644
--- a/Android.bp
+++ b/Android.bp
@@ -161,6 +161,7 @@
"src/base/watchdog_posix.cc",
"src/profiling/memory/client.cc",
"src/profiling/memory/malloc_hooks.cc",
+ "src/profiling/memory/proc_utils.cc",
"src/profiling/memory/sampler.cc",
"src/profiling/memory/wire_protocol.cc",
],
diff --git a/src/profiling/memory/BUILD.gn b/src/profiling/memory/BUILD.gn
index b17fde0..3268b4b 100644
--- a/src/profiling/memory/BUILD.gn
+++ b/src/profiling/memory/BUILD.gn
@@ -37,9 +37,21 @@
]
}
+source_set("proc_utils") {
+ deps = [
+ "../../../gn:default_deps",
+ "../../base",
+ ]
+ sources = [
+ "proc_utils.cc",
+ "proc_utils.h",
+ ]
+}
+
source_set("daemon") {
public_configs = [ "../../../buildtools:libunwindstack_config" ]
deps = [
+ ":proc_utils",
":wire_protocol",
"../../../buildtools:libunwindstack",
"../../../gn:default_deps",
@@ -58,8 +70,6 @@
"heapprofd_producer.cc",
"heapprofd_producer.h",
"interner.h",
- "proc_utils.cc",
- "proc_utils.h",
"process_matcher.cc",
"process_matcher.h",
"queue_messages.h",
@@ -77,6 +87,7 @@
source_set("client") {
public_configs = [ "../../../buildtools:libunwindstack_config" ]
deps = [
+ ":proc_utils",
":wire_protocol",
"../../../buildtools:libunwindstack",
"../../../gn:default_deps",
@@ -97,6 +108,7 @@
deps = [
":client",
":daemon",
+ ":proc_utils",
":wire_protocol",
"../../../gn:default_deps",
"../../../gn:gtest_deps",
@@ -145,6 +157,7 @@
source_set("malloc_hooks") {
deps = [
":client",
+ ":proc_utils",
":wire_protocol",
"../../../gn:default_deps",
"../../base",
diff --git a/src/profiling/memory/heapprofd_producer.cc b/src/profiling/memory/heapprofd_producer.cc
index 5b8070b..e4c6e30 100644
--- a/src/profiling/memory/heapprofd_producer.cc
+++ b/src/profiling/memory/heapprofd_producer.cc
@@ -100,15 +100,10 @@
}
}
-void HeapprofdProducer::SetTargetProcess(pid_t target_pid) {
+void HeapprofdProducer::SetTargetProcess(pid_t target_pid,
+ std::string target_cmdline) {
target_pid_ = target_pid;
- target_cmdline_.clear();
- if (!GetCmdlineForPID(target_pid, &target_cmdline_)) {
- PERFETTO_ELOG(
- "Failed to look up cmdline for target pid [%d], cmdline-based "
- "configurations will not work. Proceeding.",
- static_cast<int>(target_pid));
- }
+ target_cmdline_ = target_cmdline;
}
bool HeapprofdProducer::SourceMatchesTarget(const HeapprofdConfig& cfg) {
@@ -145,7 +140,13 @@
base::ScopedFile(fd), &weak_producer->socket_listener_,
weak_producer->task_runner_, base::SockType::kStream);
- weak_producer->socket_listener_.HandleClientConnection(std::move(socket));
+ // The forked heapprofd will not normally be able to read the target's
+ // cmdline under procfs, so pass peer's description explicitly.
+ Process process{weak_producer->target_pid_,
+ weak_producer->target_cmdline_};
+
+ weak_producer->socket_listener_.HandleClientConnection(
+ std::move(socket), std::move(process));
});
}
}
diff --git a/src/profiling/memory/heapprofd_producer.h b/src/profiling/memory/heapprofd_producer.h
index a21a78c..d00486d 100644
--- a/src/profiling/memory/heapprofd_producer.h
+++ b/src/profiling/memory/heapprofd_producer.h
@@ -66,7 +66,7 @@
void AdoptConnectedSockets(std::vector<base::ScopedFile> inherited_sockets);
// Valid only if mode_ == kChild.
- void SetTargetProcess(pid_t target_pid);
+ void SetTargetProcess(pid_t target_pid, std::string target_cmdline);
private:
// TODO(fmayer): Delete once we have generic reconnect logic.
diff --git a/src/profiling/memory/main.cc b/src/profiling/memory/main.cc
index 7a8bb62..8aea48c 100644
--- a/src/profiling/memory/main.cc
+++ b/src/profiling/memory/main.cc
@@ -39,6 +39,7 @@
namespace {
int StartChildHeapprofd(pid_t target_pid,
+ std::string target_cmdline,
std::vector<base::ScopedFile> inherited_sock_fds);
int StartCentralHeapprofd();
@@ -47,12 +48,14 @@
int HeapprofdMain(int argc, char** argv) {
bool cleanup_crash = false;
pid_t target_pid = base::kInvalidPid;
+ std::string target_cmdline;
std::vector<base::ScopedFile> inherited_sock_fds;
- enum { kCleanupCrash = 256, kTargetPid, kInheritFd };
+ enum { kCleanupCrash = 256, kTargetPid, kTargetCmd, kInheritFd };
static struct option long_options[] = {
{"cleanup-after-crash", no_argument, nullptr, kCleanupCrash},
{"exclusive-for-pid", required_argument, nullptr, kTargetPid},
+ {"exclusive-for-cmdline", required_argument, nullptr, kTargetCmd},
{"inherit-socket-fd", required_argument, nullptr, kInheritFd},
{nullptr, 0, nullptr, 0}};
int option_index;
@@ -65,6 +68,9 @@
case kTargetPid:
target_pid = static_cast<pid_t>(atoi(optarg));
break;
+ case kTargetCmd: // assumed to be already normalized
+ target_cmdline = std::string(optarg);
+ break;
case kInheritFd: // repetition supported
inherited_sock_fds.emplace_back(atoi(optarg));
break;
@@ -82,17 +88,19 @@
// If |target_pid| is given, we're supposed to be operating as a private
// heapprofd for that process. Note that we might not be a direct child due to
// reparenting.
- bool ppid_set = target_pid != base::kInvalidPid;
- bool fds_set = inherited_sock_fds.size() > 0;
- if (ppid_set || fds_set) {
- if (!ppid_set || !fds_set) {
+ bool tpid_set = target_pid != base::kInvalidPid;
+ bool tcmd_set = !target_cmdline.empty();
+ bool fds_set = !inherited_sock_fds.empty();
+ if (tpid_set || tcmd_set || fds_set) {
+ if (!tpid_set || !tcmd_set || !fds_set) {
PERFETTO_ELOG(
- "If starting in child mode, requires both --exclusive-for-pid and "
- "--inherit_socket_fd");
+ "If starting in child mode, requires all of: {--exclusive-for-pid, "
+ "--exclusive-for-cmdline, --inherit_socket_fd}");
return 1;
}
- return StartChildHeapprofd(target_pid, std::move(inherited_sock_fds));
+ return StartChildHeapprofd(target_pid, target_cmdline,
+ std::move(inherited_sock_fds));
}
// Otherwise start as a central daemon.
@@ -100,10 +108,11 @@
}
int StartChildHeapprofd(pid_t target_pid,
+ std::string target_cmdline,
std::vector<base::ScopedFile> inherited_sock_fds) {
base::UnixTaskRunner task_runner;
HeapprofdProducer producer(HeapprofdMode::kChild, &task_runner);
- producer.SetTargetProcess(target_pid);
+ producer.SetTargetProcess(target_pid, target_cmdline);
producer.ConnectWithRetries(GetProducerSocket());
producer.AdoptConnectedSockets(std::move(inherited_sock_fds));
task_runner.Run();
diff --git a/src/profiling/memory/malloc_hooks.cc b/src/profiling/memory/malloc_hooks.cc
index 6b0c4b7..28142d8 100644
--- a/src/profiling/memory/malloc_hooks.cc
+++ b/src/profiling/memory/malloc_hooks.cc
@@ -34,6 +34,7 @@
#include "perfetto/base/unix_socket.h"
#include "perfetto/base/utils.h"
#include "src/profiling/memory/client.h"
+#include "src/profiling/memory/proc_utils.h"
#include "src/profiling/memory/wire_protocol.h"
// The real malloc function pointers we get in initialize.
@@ -174,7 +175,14 @@
child_sock.RetainOnExec();
+ // Record own pid and cmdline, to pass down to the forked heapprofd.
pid_t target_pid = getpid();
+ std::string target_cmdline;
+ if (!perfetto::profiling::GetCmdlineForPID(target_pid, &target_cmdline)) {
+ PERFETTO_ELOG("Failed to read own cmdline.");
+ return nullptr;
+ }
+
pid_t fork_pid = fork();
if (fork_pid == -1) {
PERFETTO_PLOG("Failed to fork.");
@@ -190,10 +198,12 @@
}
std::string pid_arg =
std::string("--exclusive-for-pid=") + std::to_string(target_pid);
+ std::string cmd_arg =
+ std::string("--exclusive-for-cmdline=") + target_cmdline;
std::string fd_arg =
std::string("--inherit-socket-fd=") + std::to_string(child_sock.fd());
- const char* argv[] = {kHeapprofdBinPath, pid_arg.c_str(), fd_arg.c_str(),
- nullptr};
+ const char* argv[] = {kHeapprofdBinPath, pid_arg.c_str(), cmd_arg.c_str(),
+ fd_arg.c_str(), nullptr};
execv(kHeapprofdBinPath, const_cast<char**>(argv));
PERFETTO_PLOG("Failed to execute private heapprofd.");
diff --git a/src/profiling/memory/socket_listener.cc b/src/profiling/memory/socket_listener.cc
index c85f8bf..568b663 100644
--- a/src/profiling/memory/socket_listener.cc
+++ b/src/profiling/memory/socket_listener.cc
@@ -39,11 +39,7 @@
} // namespace
-SocketListener::ProcessInfo::ProcessInfo(pid_t pid) {
- process.pid = pid;
- if (!GetCmdlineForPID(pid, &process.cmdline))
- PERFETTO_ELOG("Failed to get cmdline for %d", pid);
-}
+SocketListener::ProcessInfo::ProcessInfo(Process p) : process(std::move(p)) {}
void SocketListener::ProcessInfo::Connected(
ProcessMatcher* process_matcher,
@@ -98,16 +94,24 @@
void SocketListener::OnNewIncomingConnection(
base::UnixSocket*,
std::unique_ptr<base::UnixSocket> new_connection) {
- HandleClientConnection(std::move(new_connection));
+ Process peer_process;
+ peer_process.pid = new_connection->peer_pid();
+ if (!GetCmdlineForPID(peer_process.pid, &peer_process.cmdline))
+ PERFETTO_ELOG("Failed to get cmdline for %d", peer_process.pid);
+
+ HandleClientConnection(std::move(new_connection), peer_process);
}
void SocketListener::HandleClientConnection(
- std::unique_ptr<base::UnixSocket> new_connection) {
- pid_t peer_pid = new_connection->peer_pid();
+ std::unique_ptr<base::UnixSocket> new_connection,
+ Process peer_process) {
+ PERFETTO_DCHECK(peer_process.pid == new_connection->peer_pid());
+
base::UnixSocket* new_connection_raw = new_connection.get();
decltype(process_info_)::iterator it;
- std::tie(it, std::ignore) = process_info_.emplace(peer_pid, peer_pid);
+ std::tie(it, std::ignore) =
+ process_info_.emplace(peer_process.pid, peer_process);
ProcessInfo& process_info = it->second;
process_info.Connected(&process_matcher_, bookkeeping_thread_);
process_info.sockets.emplace(new_connection_raw, std::move(new_connection));
diff --git a/src/profiling/memory/socket_listener.h b/src/profiling/memory/socket_listener.h
index d577995..37484e0 100644
--- a/src/profiling/memory/socket_listener.h
+++ b/src/profiling/memory/socket_listener.h
@@ -51,7 +51,8 @@
void Disconnect(pid_t pid) override;
// Delegate for OnNewIncomingConnection.
- void HandleClientConnection(std::unique_ptr<base::UnixSocket> new_connection);
+ void HandleClientConnection(std::unique_ptr<base::UnixSocket> new_connection,
+ Process peer_process);
ProcessMatcher& process_matcher() { return process_matcher_; }
@@ -64,7 +65,7 @@
};
struct ProcessInfo {
- ProcessInfo(pid_t pid);
+ ProcessInfo(Process p);
void Connected(ProcessMatcher* process_matcher,
BookkeepingThread* bookkeeping_thread);