traced_perf: support wildcards in /proc/pid/cmdline matching
This changes the semantics of the |target_cmdline| and |exclude_cmdline|
config fields. Normally we should've introduced a separate proto field
for this, but the change is compatible with vast majority of existing
configs, so it's less hassle for both us and users to reuse the field.
See go/perfetto-profiler-globs.
There are two compatibility cases to consider:
(a) old config expecting canonicalization + new profiler binary
(b) new config with a wildcard + old profiler binary
For (a), the primary difference is how a pattern with a fully qualified
path is treated. "/bin/echo" would previously canonicalize to "echo",
and match against anything with that suffix in argv0. Now we'll match
only against that particular fully qualified path. For (b), the old
implementation will try to do exact string comparisons, so patterns with
wildcards will effectively match nothing.
Configs that want to remain compatible with both versions of the
profiler can specify several versions of the same target_cmdline, as the
field is repeated. It'll potentially match more than before, but only in
corner cases.
Bug: 201273553
Change-Id: I48991992fb9456cfe11cab7ac7a92d7fff4ac71c
diff --git a/Android.bp b/Android.bp
index 0526976..cd592dc 100644
--- a/Android.bp
+++ b/Android.bp
@@ -10525,6 +10525,7 @@
":perfetto_src_profiling_common_callstack_trie",
":perfetto_src_profiling_common_interner",
":perfetto_src_profiling_common_interning_output",
+ ":perfetto_src_profiling_common_proc_cmdline",
":perfetto_src_profiling_common_proc_utils",
":perfetto_src_profiling_common_producer_support",
":perfetto_src_profiling_common_profiler_guardrails",
diff --git a/protos/perfetto/config/perfetto_config.proto b/protos/perfetto/config/perfetto_config.proto
index 0e0a58d..10372d4 100644
--- a/protos/perfetto/config/perfetto_config.proto
+++ b/protos/perfetto/config/perfetto_config.proto
@@ -1184,18 +1184,34 @@
// process.
repeated int32 target_pid = 1;
- // Command line allowlist, matched against the
- // /proc/<pid>/cmdline (not the comm string), with both sides being
- // "normalized". Normalization is as follows: (1) trim everything beyond the
- // first null or "@" byte; (2) if the string contains forward slashes, trim
- // everything up to and including the last one.
+ // Command line allowlist, matched against the /proc/<pid>/cmdline (not the
+ // comm string). The semantics of this field were changed since its original
+ // introduction.
+ //
+ // On Android T+ (13+), this field can specify a single wildcard (*), and
+ // the profiler will attempt to match it in two possible ways:
+ // * if the pattern starts with a '/', then it is matched against the first
+ // segment of the cmdline (i.e. argv0). For example "/bin/e*" would match
+ // "/bin/echo".
+ // * otherwise the pattern is matched against the part of argv0
+ // corresponding to the binary name (this is unrelated to /proc/pid/exe).
+ // For example "echo" would match "/bin/echo".
+ //
+ // On Android S (12) and below, both this pattern and /proc/pid/cmdline get
+ // normalized prior to an exact string comparison. Normalization is as
+ // follows: (1) trim everything beyond the first null or "@" byte; (2) if
+ // the string contains forward slashes, trim everything up to and including
+ // the last one.
+ //
+ // Implementation note: in either case, at most 511 characters of cmdline
+ // are considered.
repeated string target_cmdline = 2;
// List of excluded pids.
repeated int32 exclude_pid = 3;
- // List of excluded cmdlines. Normalized in the same way as
- // |target_cmdline|.
+ // List of excluded cmdlines. See description of |target_cmdline| for how
+ // this is handled.
repeated string exclude_cmdline = 4;
// Number of additional command lines to sample. Only those which are
diff --git a/protos/perfetto/config/profiling/perf_event_config.proto b/protos/perfetto/config/profiling/perf_event_config.proto
index a936b5b..a440464 100644
--- a/protos/perfetto/config/profiling/perf_event_config.proto
+++ b/protos/perfetto/config/profiling/perf_event_config.proto
@@ -148,18 +148,34 @@
// process.
repeated int32 target_pid = 1;
- // Command line allowlist, matched against the
- // /proc/<pid>/cmdline (not the comm string), with both sides being
- // "normalized". Normalization is as follows: (1) trim everything beyond the
- // first null or "@" byte; (2) if the string contains forward slashes, trim
- // everything up to and including the last one.
+ // Command line allowlist, matched against the /proc/<pid>/cmdline (not the
+ // comm string). The semantics of this field were changed since its original
+ // introduction.
+ //
+ // On Android T+ (13+), this field can specify a single wildcard (*), and
+ // the profiler will attempt to match it in two possible ways:
+ // * if the pattern starts with a '/', then it is matched against the first
+ // segment of the cmdline (i.e. argv0). For example "/bin/e*" would match
+ // "/bin/echo".
+ // * otherwise the pattern is matched against the part of argv0
+ // corresponding to the binary name (this is unrelated to /proc/pid/exe).
+ // For example "echo" would match "/bin/echo".
+ //
+ // On Android S (12) and below, both this pattern and /proc/pid/cmdline get
+ // normalized prior to an exact string comparison. Normalization is as
+ // follows: (1) trim everything beyond the first null or "@" byte; (2) if
+ // the string contains forward slashes, trim everything up to and including
+ // the last one.
+ //
+ // Implementation note: in either case, at most 511 characters of cmdline
+ // are considered.
repeated string target_cmdline = 2;
// List of excluded pids.
repeated int32 exclude_pid = 3;
- // List of excluded cmdlines. Normalized in the same way as
- // |target_cmdline|.
+ // List of excluded cmdlines. See description of |target_cmdline| for how
+ // this is handled.
repeated string exclude_cmdline = 4;
// Number of additional command lines to sample. Only those which are
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 8dafad0..35983c6 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -1184,18 +1184,34 @@
// process.
repeated int32 target_pid = 1;
- // Command line allowlist, matched against the
- // /proc/<pid>/cmdline (not the comm string), with both sides being
- // "normalized". Normalization is as follows: (1) trim everything beyond the
- // first null or "@" byte; (2) if the string contains forward slashes, trim
- // everything up to and including the last one.
+ // Command line allowlist, matched against the /proc/<pid>/cmdline (not the
+ // comm string). The semantics of this field were changed since its original
+ // introduction.
+ //
+ // On Android T+ (13+), this field can specify a single wildcard (*), and
+ // the profiler will attempt to match it in two possible ways:
+ // * if the pattern starts with a '/', then it is matched against the first
+ // segment of the cmdline (i.e. argv0). For example "/bin/e*" would match
+ // "/bin/echo".
+ // * otherwise the pattern is matched against the part of argv0
+ // corresponding to the binary name (this is unrelated to /proc/pid/exe).
+ // For example "echo" would match "/bin/echo".
+ //
+ // On Android S (12) and below, both this pattern and /proc/pid/cmdline get
+ // normalized prior to an exact string comparison. Normalization is as
+ // follows: (1) trim everything beyond the first null or "@" byte; (2) if
+ // the string contains forward slashes, trim everything up to and including
+ // the last one.
+ //
+ // Implementation note: in either case, at most 511 characters of cmdline
+ // are considered.
repeated string target_cmdline = 2;
// List of excluded pids.
repeated int32 exclude_pid = 3;
- // List of excluded cmdlines. Normalized in the same way as
- // |target_cmdline|.
+ // List of excluded cmdlines. See description of |target_cmdline| for how
+ // this is handled.
repeated string exclude_cmdline = 4;
// Number of additional command lines to sample. Only those which are
diff --git a/src/profiling/perf/BUILD.gn b/src/profiling/perf/BUILD.gn
index aec5549..341bd3a 100644
--- a/src/profiling/perf/BUILD.gn
+++ b/src/profiling/perf/BUILD.gn
@@ -64,7 +64,7 @@
"../common:callstack_trie",
"../common:interner",
"../common:interning_output",
- "../common:proc_utils",
+ "../common:proc_cmdline",
"../common:producer_support",
"../common:profiler_guardrails",
]
diff --git a/src/profiling/perf/event_config.cc b/src/profiling/perf/event_config.cc
index b9aada7..8506a1c 100644
--- a/src/profiling/perf/event_config.cc
+++ b/src/profiling/perf/event_config.cc
@@ -25,7 +25,6 @@
#include "perfetto/base/flat_set.h"
#include "perfetto/ext/base/optional.h"
#include "perfetto/ext/base/utils.h"
-#include "src/profiling/common/proc_utils.h"
#include "src/profiling/perf/regs_parsing.h"
#include "protos/perfetto/common/perf_events.gen.h"
@@ -40,23 +39,6 @@
constexpr uint32_t kDefaultReadTickPeriodMs = 100;
constexpr uint32_t kDefaultRemoteDescriptorTimeoutMs = 100;
-base::Optional<std::string> Normalize(const std::string& src) {
- // Construct a null-terminated string that will be mutated by the normalizer.
- std::vector<char> base(src.size() + 1);
- memcpy(base.data(), src.data(), src.size());
- base[src.size()] = '\0';
-
- char* new_start = base.data();
- ssize_t new_sz = NormalizeCmdLine(&new_start, base.size());
- if (new_sz < 0) {
- PERFETTO_ELOG("Failed to normalize config cmdline [%s], aborting",
- base.data());
- return base::nullopt;
- }
- return base::make_optional<std::string>(new_start,
- static_cast<size_t>(new_sz));
-}
-
// Acceptable forms: "sched/sched_switch" or "sched:sched_switch".
std::pair<std::string, std::string> SplitTracepointString(
const std::string& input) {
@@ -100,40 +82,29 @@
return base::make_optional(tracepoint_id);
}
-// Returns |base::nullopt| if any of the input cmdlines couldn't be normalized.
// |T| is either gen::PerfEventConfig or gen::PerfEventConfig::Scope.
+// Note: the semantics of target_cmdline and exclude_cmdline were changed since
+// their original introduction. They used to be put through a canonicalization
+// function that simplified them to the binary name alone. We no longer do this,
+// regardless of whether we're parsing an old-style config. The overall outcome
+// shouldn't change for almost all existing uses.
template <typename T>
-base::Optional<TargetFilter> ParseTargetFilter(const T& cfg) {
+TargetFilter ParseTargetFilter(const T& cfg) {
TargetFilter filter;
for (const auto& str : cfg.target_cmdline()) {
- base::Optional<std::string> opt = Normalize(str);
- if (!opt.has_value()) {
- PERFETTO_ELOG("Failure normalizing cmdline: [%s]", str.c_str());
- return base::nullopt;
- }
- filter.cmdlines.insert(std::move(opt.value()));
+ filter.cmdlines.push_back(str);
}
-
for (const auto& str : cfg.exclude_cmdline()) {
- base::Optional<std::string> opt = Normalize(str);
- if (!opt.has_value()) {
- PERFETTO_ELOG("Failure normalizing cmdline: [%s]", str.c_str());
- return base::nullopt;
- }
- filter.exclude_cmdlines.insert(std::move(opt.value()));
+ filter.exclude_cmdlines.push_back(str);
}
-
for (const int32_t pid : cfg.target_pid()) {
filter.pids.insert(pid);
}
-
for (const int32_t pid : cfg.exclude_pid()) {
filter.exclude_pids.insert(pid);
}
-
filter.additional_cmdline_count = cfg.additional_cmdline_count();
-
- return base::make_optional(std::move(filter));
+ return filter;
}
constexpr bool IsPowerOfTwo(size_t v) {
@@ -389,14 +360,10 @@
sample_callstacks = true;
// Process scoping.
- auto maybe_filter =
+ target_filter =
pb_config.callstack_sampling().has_scope()
? ParseTargetFilter(pb_config.callstack_sampling().scope())
: ParseTargetFilter(pb_config); // backwards compatibility
- if (!maybe_filter.has_value())
- return base::nullopt;
-
- target_filter = std::move(maybe_filter.value());
// Inclusion of kernel callchains.
kernel_frames = pb_config.callstack_sampling().kernel_frames() ||
diff --git a/src/profiling/perf/event_config.h b/src/profiling/perf/event_config.h
index 782b05d..cb84168 100644
--- a/src/profiling/perf/event_config.h
+++ b/src/profiling/perf/event_config.h
@@ -44,8 +44,8 @@
// Parsed allow/deny-list for filtering samples.
// An empty filter set means that all targets are allowed.
struct TargetFilter {
- base::FlatSet<std::string> cmdlines;
- base::FlatSet<std::string> exclude_cmdlines;
+ std::vector<std::string> cmdlines;
+ std::vector<std::string> exclude_cmdlines;
base::FlatSet<pid_t> pids;
base::FlatSet<pid_t> exclude_pids;
uint32_t additional_cmdline_count = 0;
diff --git a/src/profiling/perf/perf_producer.cc b/src/profiling/perf/perf_producer.cc
index d49c558..22c4a6d 100644
--- a/src/profiling/perf/perf_producer.cc
+++ b/src/profiling/perf/perf_producer.cc
@@ -36,7 +36,7 @@
#include "perfetto/tracing/core/data_source_config.h"
#include "perfetto/tracing/core/data_source_descriptor.h"
#include "src/profiling/common/callstack_trie.h"
-#include "src/profiling/common/proc_utils.h"
+#include "src/profiling/common/proc_cmdline.h"
#include "src/profiling/common/producer_support.h"
#include "src/profiling/common/profiler_guardrails.h"
#include "src/profiling/common/unwind_support.h"
@@ -180,14 +180,27 @@
base::FlatSet<std::string>* additional_cmdlines,
const TargetFilter& filter) {
PERFETTO_CHECK(additional_cmdlines);
+
std::string cmdline;
- bool have_cmdline = GetCmdlineForPID(pid, &cmdline); // normalized form
- if (!have_cmdline) {
- PERFETTO_DLOG("Failed to look up cmdline for pid [%d]",
- static_cast<int>(pid));
+ bool have_cmdline = glob_aware::ReadProcCmdlineForPID(pid, &cmdline);
+
+ const char* binname = "";
+ if (have_cmdline) {
+ binname = glob_aware::FindBinaryName(cmdline.c_str(), cmdline.size());
}
- if (have_cmdline && filter.exclude_cmdlines.count(cmdline)) {
+ auto has_matching_pattern = [](const std::vector<std::string>& patterns,
+ const char* cmd, const char* name) {
+ for (const std::string& pattern : patterns) {
+ if (glob_aware::MatchGlobPattern(pattern.c_str(), cmd, name)) {
+ return true;
+ }
+ }
+ return false;
+ };
+
+ if (have_cmdline &&
+ has_matching_pattern(filter.exclude_cmdlines, cmdline.c_str(), binname)) {
PERFETTO_DLOG("Explicitly rejecting samples for pid [%d] due to cmdline",
static_cast<int>(pid));
return true;
@@ -198,20 +211,21 @@
return true;
}
- if (have_cmdline && filter.cmdlines.count(cmdline)) {
+ if (have_cmdline &&
+ has_matching_pattern(filter.cmdlines, cmdline.c_str(), binname)) {
return false;
}
if (filter.pids.count(pid)) {
return false;
}
+
+ // Empty allow filter means keep everything that isn't explicitly excluded.
if (filter.cmdlines.empty() && filter.pids.empty() &&
!filter.additional_cmdline_count) {
- // If no filters are set allow everything.
return false;
}
- // If we didn't read the command line that's a good prediction we will not be
- // able to profile either.
+ // Config option that allows to profile just the N first seen cmdlines.
if (have_cmdline) {
if (additional_cmdlines->count(cmdline)) {
return false;