Merge "tp: add depth column and super root fn to stdlib memory.heap_graph_dominator_tree" into main
diff --git a/Android.bp b/Android.bp
index 8f7474a..8d476ce 100644
--- a/Android.bp
+++ b/Android.bp
@@ -12493,6 +12493,7 @@
"src/trace_redaction/find_package_uid.cc",
"src/trace_redaction/populate_allow_lists.cc",
"src/trace_redaction/prune_package_list.cc",
+ "src/trace_redaction/scrub_ftrace_events.cc",
"src/trace_redaction/scrub_trace_packet.cc",
"src/trace_redaction/trace_redaction_framework.cc",
"src/trace_redaction/trace_redactor.cc",
@@ -12505,6 +12506,7 @@
srcs: [
"src/trace_redaction/find_package_uid_unittest.cc",
"src/trace_redaction/prune_package_list_unittest.cc",
+ "src/trace_redaction/scrub_ftrace_events_unittest.cc",
"src/trace_redaction/scrub_trace_packet_unittest.cc",
],
}
diff --git a/buildtools/Android.mk b/buildtools/Android.mk
deleted file mode 100644
index e69de29..0000000
--- a/buildtools/Android.mk
+++ /dev/null
diff --git a/protos/perfetto/trace/android/android_input_event.proto b/protos/perfetto/trace/android/android_input_event.proto
index 7364e14..6a91950 100644
--- a/protos/perfetto/trace/android/android_input_event.proto
+++ b/protos/perfetto/trace/android/android_input_event.proto
@@ -123,12 +123,12 @@
oneof event {
// Traces input events received by or generated by InputDispatcher
AndroidMotionEvent dispatcher_motion_event = 1;
- AndroidMotionEvent dispatcher_motion_event_sensitive = 2;
+ AndroidMotionEvent dispatcher_motion_event_redacted = 2;
AndroidKeyEvent dispatcher_key_event = 3;
- AndroidKeyEvent dispatcher_key_event_sensitive = 4;
+ AndroidKeyEvent dispatcher_key_event_redacted = 4;
// Traces an event being dispatched to a window.
AndroidWindowInputDispatchEvent dispatcher_window_dispatch_event = 5;
- AndroidWindowInputDispatchEvent dispatcher_window_dispatch_event_sensitive = 6;
+ AndroidWindowInputDispatchEvent dispatcher_window_dispatch_event_redacted = 6;
}
}
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 8180b44..b8e209d 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -4310,13 +4310,13 @@
oneof event {
// Traces input events received by or generated by InputDispatcher
AndroidMotionEvent dispatcher_motion_event = 1;
- AndroidMotionEvent dispatcher_motion_event_sensitive = 2;
+ AndroidMotionEvent dispatcher_motion_event_redacted = 2;
AndroidKeyEvent dispatcher_key_event = 3;
- AndroidKeyEvent dispatcher_key_event_sensitive = 4;
+ AndroidKeyEvent dispatcher_key_event_redacted = 4;
// Traces an event being dispatched to a window.
AndroidWindowInputDispatchEvent dispatcher_window_dispatch_event = 5;
- AndroidWindowInputDispatchEvent dispatcher_window_dispatch_event_sensitive = 6;
+ AndroidWindowInputDispatchEvent dispatcher_window_dispatch_event_redacted = 6;
}
}
@@ -13801,9 +13801,6 @@
optional Utsname utsname = 1;
optional string android_build_fingerprint = 2;
- // Ticks per second - sysconf(_SC_CLK_TCK).
- optional int64 hz = 3;
-
// The version of traced (the same returned by `traced --version`).
// This is a human readable string with and its format varies depending on
// the build system and the repo (standalone vs AOSP).
@@ -13817,9 +13814,18 @@
// Kernel page size - sysconf(_SC_PAGESIZE).
optional uint32 page_size = 6;
+ // Number of cpus - sysconf(_SC_NPROCESSORS_CONF).
+ // Might be different to the number of online cpus.
+ // Introduced in perfetto v44.
+ optional uint32 num_cpus = 8;
+
// The timezone offset from UTC, as per strftime("%z"), in minutes.
// Introduced in v38 / Android V.
optional int32 timezone_off_mins = 7;
+
+ // Ticks per second - sysconf(_SC_CLK_TCK).
+ // Not serialised as of perfetto v44.
+ optional int64 hz = 3;
}
// End of protos/perfetto/trace/system_info.proto
diff --git a/protos/perfetto/trace/system_info.proto b/protos/perfetto/trace/system_info.proto
index 9a75773..9f38848 100644
--- a/protos/perfetto/trace/system_info.proto
+++ b/protos/perfetto/trace/system_info.proto
@@ -29,9 +29,6 @@
optional Utsname utsname = 1;
optional string android_build_fingerprint = 2;
- // Ticks per second - sysconf(_SC_CLK_TCK).
- optional int64 hz = 3;
-
// The version of traced (the same returned by `traced --version`).
// This is a human readable string with and its format varies depending on
// the build system and the repo (standalone vs AOSP).
@@ -45,7 +42,16 @@
// Kernel page size - sysconf(_SC_PAGESIZE).
optional uint32 page_size = 6;
+ // Number of cpus - sysconf(_SC_NPROCESSORS_CONF).
+ // Might be different to the number of online cpus.
+ // Introduced in perfetto v44.
+ optional uint32 num_cpus = 8;
+
// The timezone offset from UTC, as per strftime("%z"), in minutes.
// Introduced in v38 / Android V.
optional int32 timezone_off_mins = 7;
+
+ // Ticks per second - sysconf(_SC_CLK_TCK).
+ // Not serialised as of perfetto v44.
+ optional int64 hz = 3;
}
diff --git a/src/trace_processor/db/column/string_storage.cc b/src/trace_processor/db/column/string_storage.cc
index 07dfa76..9ab8c1f 100644
--- a/src/trace_processor/db/column/string_storage.cc
+++ b/src/trace_processor/db/column/string_storage.cc
@@ -620,20 +620,51 @@
SortToken* end,
SortDirection direction) const {
switch (direction) {
- case SortDirection::kAscending:
+ case SortDirection::kAscending: {
std::stable_sort(start, end,
- [this](const SortToken& a, const SortToken& b) {
- return string_pool_->Get((*data_)[a.index]) <
- string_pool_->Get((*data_)[b.index]);
+ [this](const SortToken& lhs, const SortToken& rhs) {
+ // If RHS is NULL, we know that LHS is not less than
+ // NULL, as nothing is less then null. This check is
+ // only required to keep the stability of the sort.
+ if ((*data_)[rhs.index] == StringPool::Id::Null()) {
+ return false;
+ }
+
+ // If LHS is NULL, it will always be smaller than any
+ // RHS value.
+ if ((*data_)[lhs.index] == StringPool::Id::Null()) {
+ return true;
+ }
+
+ // If neither LHS or RHS are NULL, we have to simply
+ // check which string is smaller.
+ return string_pool_->Get((*data_)[lhs.index]) <
+ string_pool_->Get((*data_)[rhs.index]);
});
return;
- case SortDirection::kDescending:
+ }
+ case SortDirection::kDescending: {
std::stable_sort(start, end,
- [this](const SortToken& a, const SortToken& b) {
- return string_pool_->Get((*data_)[a.index]) >
- string_pool_->Get((*data_)[b.index]);
+ [this](const SortToken& lhs, const SortToken& rhs) {
+ // If LHS is NULL, we know that it's not greater than
+ // any RHS. This check is only required to keep the
+ // stability of the sort.
+ if ((*data_)[lhs.index] == StringPool::Id::Null()) {
+ return false;
+ }
+
+ // If RHS is NULL, everything will be greater from it.
+ if ((*data_)[rhs.index] == StringPool::Id::Null()) {
+ return true;
+ }
+
+ // If neither LHS or RHS are NULL, we have to simply
+ // check which string is smaller.
+ return string_pool_->Get((*data_)[lhs.index]) >
+ string_pool_->Get((*data_)[rhs.index]);
});
return;
+ }
}
PERFETTO_FATAL("For GCC");
}
diff --git a/src/trace_processor/importers/common/system_info_tracker.cc b/src/trace_processor/importers/common/system_info_tracker.cc
index 89f0baf..3bbed5b 100644
--- a/src/trace_processor/importers/common/system_info_tracker.cc
+++ b/src/trace_processor/importers/common/system_info_tracker.cc
@@ -20,7 +20,7 @@
namespace perfetto {
namespace trace_processor {
-SystemInfoTracker::SystemInfoTracker() {}
+SystemInfoTracker::SystemInfoTracker() = default;
SystemInfoTracker::~SystemInfoTracker() = default;
void SystemInfoTracker::SetKernelVersion(base::StringView name,
diff --git a/src/trace_processor/importers/common/system_info_tracker.h b/src/trace_processor/importers/common/system_info_tracker.h
index d9544d1..ec46e80 100644
--- a/src/trace_processor/importers/common/system_info_tracker.h
+++ b/src/trace_processor/importers/common/system_info_tracker.h
@@ -42,13 +42,16 @@
}
void SetKernelVersion(base::StringView name, base::StringView release);
+ void SetNumCpus(uint32_t num_cpus) { num_cpus_ = num_cpus; }
- std::optional<VersionNumber> GetKernelVersion() { return version_; }
+ std::optional<VersionNumber> GetKernelVersion() const { return version_; }
+ std::optional<uint32_t> GetNumCpus() const { return num_cpus_; }
private:
explicit SystemInfoTracker();
std::optional<VersionNumber> version_;
+ std::optional<uint32_t> num_cpus_;
};
} // namespace trace_processor
} // namespace perfetto
diff --git a/src/trace_processor/importers/proto/system_probes_parser.cc b/src/trace_processor/importers/proto/system_probes_parser.cc
index 09adb4a..46e6166 100644
--- a/src/trace_processor/importers/proto/system_probes_parser.cc
+++ b/src/trace_processor/importers/proto/system_probes_parser.cc
@@ -652,6 +652,8 @@
void SystemProbesParser::ParseSystemInfo(ConstBytes blob) {
protos::pbzero::SystemInfo::Decoder packet(blob.data, blob.size);
+ SystemInfoTracker* system_info_tracker =
+ SystemInfoTracker::GetOrCreate(context_);
if (packet.has_utsname()) {
ConstBytes utsname_blob = packet.utsname();
protos::pbzero::Utsname::Decoder utsname(utsname_blob.data,
@@ -666,8 +668,6 @@
machine.ToStdString().c_str());
}
- SystemInfoTracker* system_info_tracker =
- SystemInfoTracker::GetOrCreate(context_);
system_info_tracker->SetKernelVersion(utsname.sysname(), utsname.release());
StringPool::Id sysname_id =
@@ -717,13 +717,14 @@
metadata::android_sdk_version, Variadic::Integer(*opt_sdk_version));
}
- int64_t hz = packet.hz();
- if (hz > 0)
- ms_per_tick_ = 1000u / static_cast<uint64_t>(hz);
-
page_size_ = packet.page_size();
- if (!page_size_)
+ if (!page_size_) {
page_size_ = 4096;
+ }
+
+ if (packet.has_num_cpus()) {
+ system_info_tracker->SetNumCpus(packet.num_cpus());
+ }
}
void SystemProbesParser::ParseCpuInfo(ConstBytes blob) {
diff --git a/src/trace_processor/importers/proto/system_probes_parser.h b/src/trace_processor/importers/proto/system_probes_parser.h
index 0378128..776b2c6 100644
--- a/src/trace_processor/importers/proto/system_probes_parser.h
+++ b/src/trace_processor/importers/proto/system_probes_parser.h
@@ -18,7 +18,6 @@
#define SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_SYSTEM_PROBES_PARSER_H_
#include <array>
-#include <set>
#include <vector>
#include "perfetto/protozero/field.h"
@@ -79,7 +78,6 @@
std::array<StringId, protos::pbzero::SysStats_PsiSample_PsiResource_MAX + 1>
sys_stats_psi_resource_names_{};
- uint64_t ms_per_tick_ = 0;
uint32_t page_size_ = 0;
int64_t prev_read_amount = -1;
diff --git a/src/trace_processor/perfetto_sql/stdlib/graphs/search.sql b/src/trace_processor/perfetto_sql/stdlib/graphs/search.sql
index 4d2eea8..4bba25a 100644
--- a/src/trace_processor/perfetto_sql/stdlib/graphs/search.sql
+++ b/src/trace_processor/perfetto_sql/stdlib/graphs/search.sql
@@ -73,7 +73,7 @@
-- The order of the next sibling is undefined if the |sort_key| is not unique.
--
-- Example usage:
---
+-- ```
-- -- Compute the next sibling:
-- SELECT *
-- FROM graph_next_sibling!(
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index f65c588..7b2f4c5 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -34,6 +34,8 @@
"populate_allow_lists.h",
"prune_package_list.cc",
"prune_package_list.h",
+ "scrub_ftrace_events.cc",
+ "scrub_ftrace_events.h",
"scrub_trace_packet.cc",
"scrub_trace_packet.h",
"trace_redaction_framework.cc",
@@ -51,6 +53,7 @@
"../../protos/perfetto/trace:non_minimal_zero",
"../../protos/perfetto/trace/android:cpp",
"../../protos/perfetto/trace/android:zero",
+ "../../protos/perfetto/trace/ftrace:zero",
"../trace_processor:storage_minimal",
]
}
@@ -65,6 +68,7 @@
"../../include/perfetto/ext/base",
"../../protos/perfetto/trace:non_minimal_zero",
"../../protos/perfetto/trace/android:zero",
+ "../../protos/perfetto/trace/ftrace:zero",
"../base:test_support",
]
}
@@ -74,15 +78,19 @@
sources = [
"find_package_uid_unittest.cc",
"prune_package_list_unittest.cc",
+ "scrub_ftrace_events_unittest.cc",
"scrub_trace_packet_unittest.cc",
]
deps = [
":trace_redaction",
"../../gn:default_deps",
"../../gn:gtest_and_gmock",
+ "../../include/perfetto/protozero:protozero",
"../../protos/perfetto/trace:non_minimal_cpp",
"../../protos/perfetto/trace:zero",
"../../protos/perfetto/trace/android:cpp",
+ "../../protos/perfetto/trace/ftrace:cpp",
+ "../../protos/perfetto/trace/ftrace:zero",
"../../protos/perfetto/trace/ps:cpp",
"../../protos/perfetto/trace/ps:zero",
"../base:test_support",
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc
index 24ad116..153e437 100644
--- a/src/trace_redaction/main.cc
+++ b/src/trace_redaction/main.cc
@@ -18,6 +18,8 @@
#include "perfetto/base/status.h"
#include "src/trace_redaction/find_package_uid.h"
#include "src/trace_redaction/prune_package_list.h"
+#include "src/trace_redaction/scrub_ftrace_events.h"
+#include "src/trace_redaction/scrub_trace_packet.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "src/trace_redaction/trace_redactor.h"
@@ -36,6 +38,8 @@
// Add all transforms.
redactor.transformers()->emplace_back(new PrunePackageList());
+ redactor.transformers()->emplace_back(new ScrubTracePacket());
+ redactor.transformers()->emplace_back(new ScrubFtraceEvents());
Context context;
context.package_name = package_name;
diff --git a/src/trace_redaction/populate_allow_lists.cc b/src/trace_redaction/populate_allow_lists.cc
index 1ccaa00..fbfe279 100644
--- a/src/trace_redaction/populate_allow_lists.cc
+++ b/src/trace_redaction/populate_allow_lists.cc
@@ -19,6 +19,7 @@
#include "perfetto/base/status.h"
#include "src/trace_redaction/trace_redaction_framework.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
#include "protos/perfetto/trace/trace_packet.pbzero.h"
namespace perfetto::trace_redaction {
@@ -50,6 +51,41 @@
protos::pbzero::TracePacket::kPackagesListFieldNumber,
};
+ context->ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber,
+ protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber,
+ protos::pbzero::FtraceEvent::kCpuIdleFieldNumber,
+ protos::pbzero::FtraceEvent::kSchedBlockedReasonFieldNumber,
+ protos::pbzero::FtraceEvent::kSchedWakingFieldNumber,
+ protos::pbzero::FtraceEvent::kSuspendResumeFieldNumber,
+ protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber,
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
+ protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber,
+ protos::pbzero::FtraceEvent::kRssStatFieldNumber,
+ protos::pbzero::FtraceEvent::kIonHeapShrinkFieldNumber,
+ protos::pbzero::FtraceEvent::kIonHeapGrowFieldNumber,
+ protos::pbzero::FtraceEvent::kIonStatFieldNumber,
+ protos::pbzero::FtraceEvent::kIonBufferCreateFieldNumber,
+ protos::pbzero::FtraceEvent::kIonBufferDestroyFieldNumber,
+ protos::pbzero::FtraceEvent::kDmaHeapStatFieldNumber,
+ protos::pbzero::FtraceEvent::kRssStatThrottledFieldNumber,
+ };
+
+ // TODO: Some ftrace fields should be retained, but they carry too much risk
+ // without additional redaction. This list should be configured in a build
+ // primitive so that they can be optionally included.
+ //
+ // TODO: Some fields will create new packets (e.g. binder calls may create
+ // new spans. This is currently not supported (generated packets still
+ // need to be redacted).
+ //
+ // protos::pbzero::FtraceEvent::kPrintFieldNumber,
+ // protos::pbzero::FtraceEvent::kBinderTransactionFieldNumber,
+ // protos::pbzero::FtraceEvent::kBinderTransactionReceivedFieldNumber,
+ // protos::pbzero::FtraceEvent::kBinderSetPriorityFieldNumber,
+ // protos::pbzero::FtraceEvent::kBinderLockedFieldNumber,
+ // protos::pbzero::FtraceEvent::kBinderUnlockFieldNumber,
+
return base::OkStatus();
}
diff --git a/src/trace_redaction/scrub_ftrace_events.cc b/src/trace_redaction/scrub_ftrace_events.cc
new file mode 100644
index 0000000..67585d1
--- /dev/null
+++ b/src/trace_redaction/scrub_ftrace_events.cc
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_redaction/scrub_ftrace_events.h"
+
+#include <string>
+
+#include "perfetto/protozero/message.h"
+#include "perfetto/protozero/message_arena.h"
+#include "perfetto/protozero/scattered_heap_buffer.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
+#include "protos/perfetto/trace/trace.pbzero.h"
+
+namespace perfetto::trace_redaction {
+namespace {
+
+constexpr auto kFtraceEventsFieldNumber =
+ protos::pbzero::TracePacket::kFtraceEventsFieldNumber;
+
+constexpr auto kEventFieldNumber =
+ protos::pbzero::FtraceEventBundle::kEventFieldNumber;
+
+enum class Redact : uint8_t {
+ // Some resources in the target need to be redacted.
+ kSomething = 0,
+
+ // No resources in the target need to be redacted.
+ kNothing = 1,
+};
+
+// Return kSomething if an event will change after redaction . If a packet
+// will not change, then the packet should skip redaction and be appended
+// to the output.
+//
+// Event packets have few packets (e.g. timestamp, pid, the event payload).
+// because of this, it is relatively cheap to test a packet.
+//
+// event {
+// timestamp: 6702095044306682
+// pid: 0
+// sched_switch {
+// prev_comm: "swapper/2"
+// prev_pid: 0
+// prev_prio: 120
+// prev_state: 0
+// next_comm: "surfaceflinger"
+// next_pid: 819
+// next_prio: 120
+// }
+// }
+Redact ProbeEvent(const Context& context, const protozero::Field& event) {
+ if (event.id() != kEventFieldNumber) {
+ PERFETTO_FATAL("Invalid proto field. Expected kEventFieldNumber.");
+ }
+
+ protozero::ProtoDecoder decoder(event.data(), event.size());
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (context.ftrace_packet_allow_list.count(field.id()) != 0) {
+ return Redact::kNothing;
+ }
+ }
+
+ return Redact::kSomething;
+}
+
+} // namespace
+
+// packet {
+// ftrace_events {
+// event { <-- This is where we test the allow-list
+// timestamp: 6702095044299807
+// pid: 0
+// cpu_idle { <-- This is the event data (allow-list)
+// state: 4294967295
+// cpu_id: 2
+// }
+// }
+// }
+// }
+base::Status ScrubFtraceEvents::Transform(const Context& context,
+ std::string* packet) const {
+ if (packet == nullptr || packet->empty()) {
+ return base::ErrStatus("Cannot scrub null or empty trace packet.");
+ }
+
+ if (context.ftrace_packet_allow_list.empty()) {
+ return base::ErrStatus("Cannot scrub ftrace packets, missing allow-list.");
+ }
+
+ // If the packet has no ftrace events, skip it, leaving it unmodified.
+ protos::pbzero::TracePacket::Decoder query(*packet);
+ if (!query.has_ftrace_events()) {
+ return base::OkStatus();
+ }
+
+ protozero::HeapBuffered<protos::pbzero::TracePacket> packet_msg;
+
+ // packet.foreach_child.foreach( ... )
+ protozero::ProtoDecoder d_packet(*packet);
+ for (auto packet_child_it = d_packet.ReadField(); packet_child_it.valid();
+ packet_child_it = d_packet.ReadField()) {
+ // packet.child_not<ftrace_events>( ).do ( ... )
+ if (packet_child_it.id() != kFtraceEventsFieldNumber) {
+ AppendField(packet_child_it, packet_msg.get());
+ continue;
+ }
+
+ // To clarify, "ftrace_events" is the field name and "FtraceEventBundle" is
+ // the field type. The terms are often used interchangeably.
+ auto* ftrace_events_msg = packet_msg->set_ftrace_events();
+
+ // packet.child<ftrace_events>( ).foreach_child( ... )
+ protozero::ProtoDecoder ftrace_events(packet_child_it.as_bytes());
+ for (auto ftrace_events_it = ftrace_events.ReadField();
+ ftrace_events_it.valid();
+ ftrace_events_it = ftrace_events.ReadField()) {
+ // packet.child<ftrace_events>( ).child_not<event>( ).do ( ... )
+ if (ftrace_events_it.id() != kEventFieldNumber) {
+ AppendField(ftrace_events_it, ftrace_events_msg);
+ continue;
+ }
+
+ // packet.child<ftrace_events>( ).child_is<event>( ).do ( ... )
+ if (ProbeEvent(context, ftrace_events_it) == Redact::kNothing) {
+ AppendField(ftrace_events_it, ftrace_events_msg);
+ continue;
+ }
+
+ // Dropping packet = "is event" and "is redacted"
+ }
+ }
+
+ packet->assign(packet_msg.SerializeAsString());
+ return base::OkStatus();
+}
+
+// This is copied from "src/protozero/field.cc", but was modified to use the
+// serialization methods provided in "perfetto/protozero/message.h".
+void ScrubFtraceEvents::AppendField(const protozero::Field& field,
+ protozero::Message* message) {
+ auto id = field.id();
+ auto type = field.type();
+
+ switch (type) {
+ case protozero::proto_utils::ProtoWireType::kVarInt: {
+ message->AppendVarInt(id, field.raw_int_value());
+ return;
+ }
+
+ case protozero::proto_utils::ProtoWireType::kFixed32: {
+ message->AppendFixed(id, field.as_uint32());
+ return;
+ }
+
+ case protozero::proto_utils::ProtoWireType::kFixed64: {
+ message->AppendFixed(id, field.as_uint64());
+ return;
+ }
+
+ case protozero::proto_utils::ProtoWireType::kLengthDelimited: {
+ message->AppendBytes(id, field.data(), field.size());
+ return;
+ }
+ }
+
+ // A switch-statement would be preferred, but when using a switch statement,
+ // it complains that about case coverage.
+ PERFETTO_FATAL("Unknown field type %u", static_cast<uint8_t>(type));
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/scrub_ftrace_events.h b/src/trace_redaction/scrub_ftrace_events.h
new file mode 100644
index 0000000..9ae6a98
--- /dev/null
+++ b/src/trace_redaction/scrub_ftrace_events.h
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_TRACE_REDACTION_SCRUB_FTRACE_EVENTS_H_
+#define SRC_TRACE_REDACTION_SCRUB_FTRACE_EVENTS_H_
+
+#include "perfetto/protozero/field.h"
+#include "perfetto/protozero/message.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+namespace perfetto::trace_redaction {
+
+// Assumptions:
+// 1. This is a hot path (a lot of ftrace packets)
+// 2. Allocations are slower than CPU cycles.
+//
+// Overview:
+// To limit allocations pbzero protos are used to build a new packet. These
+// protos are append-only, so data is not removed from the packet. Instead,
+// data is optionally added to a new packet.
+//
+// To limit allocations, the goal is to add data as large chucks rather than
+// small fragments. To do this, a reactive strategy is used. All operations
+// follow a probe-than-act pattern. Before any action can be taken, the
+// input data must be queries to determine the scope. For example:
+//
+// [------A------][---B---][------C------]
+// [---][-D-][---]
+//
+// Assume that A and B don't need any work, they can be appended to the
+// output as two large blocks.
+//
+// Block C is different, there is a block D that falls within block C.
+// Block D contains sensitive information and should be dropped. When C
+// is probed, it will come back saying that C needs additional redaction.
+
+class ScrubFtraceEvents final : public TransformPrimitive {
+ public:
+ base::Status Transform(const Context& context,
+ std::string* packet) const override;
+
+ // Used by `Transform()`. Only exposed for conformance testing.
+ static void AppendField(const protozero::Field& field,
+ protozero::Message* message);
+};
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_SCRUB_FTRACE_EVENTS_H_
diff --git a/src/trace_redaction/scrub_ftrace_events_unittest.cc b/src/trace_redaction/scrub_ftrace_events_unittest.cc
new file mode 100644
index 0000000..179c504
--- /dev/null
+++ b/src/trace_redaction/scrub_ftrace_events_unittest.cc
@@ -0,0 +1,329 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "src/trace_redaction/scrub_ftrace_events.h"
+#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "protos/perfetto/trace/ftrace/power.pbzero.h"
+#include "src/base/test/status_matchers.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace_event.gen.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.gen.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
+#include "protos/perfetto/trace/ftrace/power.gen.h"
+#include "protos/perfetto/trace/ftrace/task.gen.h"
+#include "protos/perfetto/trace/ps/process_tree.gen.h"
+#include "protos/perfetto/trace/trace_packet.gen.h"
+
+namespace perfetto::trace_redaction {
+
+namespace {
+// task_rename should be in the allow-list.
+void AddTaskRename(protos::gen::FtraceEventBundle* bundle,
+ int32_t pid,
+ const std::string& old_comm,
+ const std::string& new_comm) {
+ auto* e = bundle->add_event();
+ e->mutable_task_rename()->set_pid(pid);
+ e->mutable_task_rename()->set_oldcomm(old_comm);
+ e->mutable_task_rename()->set_newcomm(new_comm);
+}
+
+void AddClockSetRate(protos::gen::FtraceEventBundle* bundle,
+ uint64_t cpu,
+ const std::string& name,
+ uint64_t state) {
+ auto* e = bundle->add_event();
+ e->mutable_clock_set_rate()->set_cpu_id(cpu);
+ e->mutable_clock_set_rate()->set_name(name);
+ e->mutable_clock_set_rate()->set_state(state);
+}
+
+} // namespace
+
+class ScrubFtraceEventsSerializationTest : public testing::Test {
+ public:
+ ScrubFtraceEventsSerializationTest() = default;
+ ~ScrubFtraceEventsSerializationTest() override = default;
+
+ protected:
+ void SetUp() override {
+ std::array bytes = {
+ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+ 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+ };
+
+ protozero::HeapBuffered<protozero::Message> msg;
+ msg->AppendFixed<uint32_t>(field_id_fixed_32, 7);
+ msg->AppendFixed<uint64_t>(field_id_fixed_64, 9);
+ msg->AppendVarInt(field_id_var_int, 723);
+ msg->AppendBytes(field_id_bytes, bytes.data(), bytes.size());
+
+ message_.assign(msg.SerializeAsString());
+ }
+
+ std::string message_;
+
+ static constexpr uint32_t field_id_fixed_32 = 0;
+ static constexpr uint32_t field_id_fixed_64 = 1;
+ static constexpr uint32_t field_id_var_int = 2;
+ static constexpr uint32_t field_id_bytes = 3;
+};
+
+TEST_F(ScrubFtraceEventsSerializationTest, AppendFixedUint32) {
+ protozero::ProtoDecoder decoder(message_);
+
+ const auto& field = decoder.FindField(field_id_fixed_32);
+
+ std::string expected = "";
+ field.SerializeAndAppendTo(&expected);
+
+ protozero::HeapBuffered<protozero::Message> actual;
+ ScrubFtraceEvents::AppendField(field, actual.get());
+
+ ASSERT_EQ(actual.SerializeAsString(), expected);
+}
+
+TEST_F(ScrubFtraceEventsSerializationTest, AppendFixedUint64) {
+ protozero::ProtoDecoder decoder(message_);
+
+ const auto& field = decoder.FindField(field_id_fixed_64);
+
+ std::string expected = "";
+ field.SerializeAndAppendTo(&expected);
+
+ protozero::HeapBuffered<protozero::Message> actual;
+ ScrubFtraceEvents::AppendField(field, actual.get());
+
+ ASSERT_EQ(actual.SerializeAsString(), expected);
+}
+
+TEST_F(ScrubFtraceEventsSerializationTest, AppendBytes) {
+ protozero::ProtoDecoder decoder(message_);
+
+ const auto& field = decoder.FindField(field_id_bytes);
+
+ std::string expected = "";
+ field.SerializeAndAppendTo(&expected);
+
+ protozero::HeapBuffered<protozero::Message> actual;
+ ScrubFtraceEvents::AppendField(field, actual.get());
+
+ ASSERT_EQ(actual.SerializeAsString(), expected);
+}
+
+TEST_F(ScrubFtraceEventsSerializationTest, AppendVarInt) {
+ protozero::ProtoDecoder decoder(message_);
+
+ const auto& field = decoder.FindField(field_id_var_int);
+
+ std::string expected = "";
+ field.SerializeAndAppendTo(&expected);
+
+ protozero::HeapBuffered<protozero::Message> actual;
+ ScrubFtraceEvents::AppendField(field, actual.get());
+
+ ASSERT_EQ(actual.SerializeAsString(), expected);
+}
+
+TEST(ScrubFtraceEventsTest, ReturnErrorForNullPacket) {
+ // Have something in the allow-list to avoid that error.
+ Context context;
+ context.ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
+
+ ScrubFtraceEvents scrub;
+ ASSERT_FALSE(scrub.Transform(context, nullptr).ok());
+}
+
+TEST(ScrubFtraceEventsTest, ReturnErrorForEmptyPacket) {
+ // Have something in the allow-list to avoid that error.
+ Context context;
+ context.ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
+
+ std::string packet_str = "";
+
+ ScrubFtraceEvents scrub;
+ ASSERT_FALSE(scrub.Transform(context, &packet_str).ok());
+}
+
+TEST(ScrubFtraceEventsTest, ReturnErrorForEmptyAllowList) {
+ // The context will have no allow-list entries. ScrubFtraceEvents should fail.
+ Context context;
+
+ protos::gen::TracePacket packet;
+ std::string packet_str = packet.SerializeAsString();
+
+ ScrubFtraceEvents scrub;
+ ASSERT_FALSE(scrub.Transform(context, &packet_str).ok());
+}
+
+TEST(ScrubFtraceEventsTest, IgnorePacketWithNoFtraceEvents) {
+ protos::gen::TracePacket trace_packet;
+ auto* tree = trace_packet.mutable_process_tree();
+
+ auto& process = tree->mutable_processes()->emplace_back();
+ process.set_pid(1);
+ process.set_ppid(2);
+ process.set_uid(3);
+
+ auto& thread = tree->mutable_threads()->emplace_back();
+ thread.set_name("hello world");
+ thread.set_tgid(1);
+ thread.set_tid(135);
+
+ auto original_packet = trace_packet.SerializeAsString();
+ auto packet = original_packet;
+
+ Context context;
+ context.ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
+
+ ScrubFtraceEvents transform;
+ ASSERT_OK(transform.Transform(context, &packet));
+
+ // The packet doesn't have any ftrace events. It should not be affected by
+ // this transform.
+ ASSERT_EQ(original_packet, packet);
+}
+
+// There are some values in a ftrace event that sits behind the ftrace bundle.
+// These values should be retained.
+TEST(ScrubFtraceEventsTest, KeepsFtraceBundleSiblingValues) {
+ protos::gen::TracePacket trace_packet;
+ auto* ftrace_events = trace_packet.mutable_ftrace_events();
+
+ ftrace_events->set_cpu(7);
+ AddTaskRename(ftrace_events, 7, "old_comm", "new_comm_7");
+ AddClockSetRate(ftrace_events, 7, "cool cpu name", 1);
+
+ auto original_packet = trace_packet.SerializeAsString();
+ auto packet = original_packet;
+
+ Context context;
+ context.ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
+
+ ScrubFtraceEvents transform;
+ ASSERT_OK(transform.Transform(context, &packet));
+
+ protos::gen::TracePacket gen_packet;
+ gen_packet.ParseFromString(packet);
+
+ ASSERT_TRUE(gen_packet.has_ftrace_events());
+ const auto& gen_events = gen_packet.ftrace_events();
+
+ // Because the CPU sits beside the event list, and not inside the event list,
+ // the CPU value should be retained.
+ ASSERT_TRUE(gen_events.has_cpu());
+ ASSERT_EQ(gen_events.cpu(), 7u);
+
+ // ClockSetRate should be dropped. Only TaskRename should remain.
+ ASSERT_EQ(gen_events.event_size(), 1);
+ ASSERT_FALSE(gen_events.event().front().has_clock_set_rate());
+ ASSERT_TRUE(gen_events.event().front().has_task_rename());
+}
+
+TEST(ScrubFtraceEventsTest, KeepsAllowedEvents) {
+ Context context;
+ context.ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
+ };
+
+ protos::gen::TracePacket before;
+ AddTaskRename(before.mutable_ftrace_events(), 7, "old_comm", "new_comm_7");
+ AddTaskRename(before.mutable_ftrace_events(), 8, "old_comm", "new_comm_8");
+ AddTaskRename(before.mutable_ftrace_events(), 9, "old_comm", "new_comm_9");
+
+ auto before_str = before.SerializeAsString();
+ auto after_str = before_str;
+
+ ScrubFtraceEvents transform;
+ ASSERT_OK(transform.Transform(context, &after_str));
+
+ protos::gen::TracePacket after;
+ after.ParseFromString(after_str);
+
+ // Implementation detail: ScrubFtraceEvents may change entry order. The diff
+ // must be order independent. Sort the events by pid, this will make it easier
+ // to assert values.
+ auto events = after.ftrace_events().event();
+ std::sort(events.begin(), events.end(),
+ [](const auto& l, const auto& r) { return l.pid() < r.pid(); });
+
+ ASSERT_EQ(events.size(), 3u);
+
+ ASSERT_TRUE(events[0].has_task_rename());
+ ASSERT_EQ(events[0].task_rename().pid(), 7);
+ ASSERT_EQ(events[0].task_rename().oldcomm(), "old_comm");
+ ASSERT_EQ(events[0].task_rename().newcomm(), "new_comm_7");
+
+ ASSERT_TRUE(events[1].has_task_rename());
+ ASSERT_EQ(events[1].task_rename().pid(), 8);
+ ASSERT_EQ(events[1].task_rename().oldcomm(), "old_comm");
+ ASSERT_EQ(events[1].task_rename().newcomm(), "new_comm_8");
+
+ ASSERT_TRUE(events[2].has_task_rename());
+ ASSERT_EQ(events[2].task_rename().pid(), 9);
+ ASSERT_EQ(events[2].task_rename().oldcomm(), "old_comm");
+ ASSERT_EQ(events[2].task_rename().newcomm(), "new_comm_9");
+}
+
+// Only the specific non-allowed events should be removed from the event list.
+TEST(ScrubFtraceEventsTest, OnlyDropsNotAllowedEvents) {
+ // AddTaskRename >> Keep
+ // AddClockSetRate >> Drop
+ protos::gen::TracePacket original_packet;
+ AddTaskRename(original_packet.mutable_ftrace_events(), 7, "old_comm",
+ "new_comm_7");
+ AddClockSetRate(original_packet.mutable_ftrace_events(), 0, "cool cpu name",
+ 1);
+ AddTaskRename(original_packet.mutable_ftrace_events(), 8, "old_comm",
+ "new_comm_8");
+ AddTaskRename(original_packet.mutable_ftrace_events(), 9, "old_comm",
+ "new_comm_9");
+ auto packet = original_packet.SerializeAsString();
+
+ Context context;
+ context.ftrace_packet_allow_list = {
+ protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
+
+ ScrubFtraceEvents transform;
+ ASSERT_OK(transform.Transform(context, &packet));
+
+ protos::gen::TracePacket modified_packet;
+ ASSERT_TRUE(modified_packet.ParseFromString(packet));
+
+ // Only the clock set rate event should have been removed (drop 1 of the 4
+ // events).
+ ASSERT_TRUE(modified_packet.has_ftrace_events());
+ ASSERT_EQ(modified_packet.ftrace_events().event_size(), 3);
+
+ // All ftrace events should be rename events.
+ const auto& events = modified_packet.ftrace_events().event();
+
+ ASSERT_TRUE(events.at(0).has_task_rename());
+ ASSERT_EQ(events.at(0).task_rename().pid(), 7);
+
+ ASSERT_TRUE(events.at(1).has_task_rename());
+ ASSERT_EQ(events.at(1).task_rename().pid(), 8);
+
+ ASSERT_TRUE(events.at(2).has_task_rename());
+ ASSERT_EQ(events.at(2).task_rename().pid(), 9);
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/scrub_trace_packet.h b/src/trace_redaction/scrub_trace_packet.h
index fbf89ca..75c5722 100644
--- a/src/trace_redaction/scrub_trace_packet.h
+++ b/src/trace_redaction/scrub_trace_packet.h
@@ -21,8 +21,17 @@
namespace perfetto::trace_redaction {
-// Drops whole trace packets based on an allow-list (e.g. retain ProcessTree
-// packets).
+// TODO(vaage): This primitive DOES NOT handle redacting sched_switch and
+// sched_waking ftrace events. These events contain a large amount of "to be
+// redacted" information AND there are a high quantity of them AND they are
+// large packets. As such, this primitive is not enough and an ADDITIONAL
+// primitive is required.
+
+// Goes through individual ftrace packs and drops the ftrace packets from the
+// trace packet without modifying the surround fields.
+//
+// ScrubTracePacket does not respect field order - i.e. the field order going
+// may not match the field order going out.
class ScrubTracePacket final : public TransformPrimitive {
public:
base::Status Transform(const Context& context,
diff --git a/src/trace_redaction/trace_redaction_framework.h b/src/trace_redaction/trace_redaction_framework.h
index 76b8594..d22ecd2 100644
--- a/src/trace_redaction/trace_redaction_framework.h
+++ b/src/trace_redaction/trace_redaction_framework.h
@@ -117,6 +117,45 @@
// Because "data" is a "one of", if no field in "trace_packet_allow_list" can
// be found, it packet should be removed.
base::FlatSet<uint32_t> trace_packet_allow_list;
+
+ // Ftrace packets contain a "one of" entry called "event". Within the scope of
+ // a ftrace event, the event can be considered the payload and other other
+ // values can be considered metadata (e.g. timestamp and pid).
+ //
+ // A ftrace event should be removed if:
+ //
+ // ... we know it contains too much sensitive information
+ //
+ // ... we know it contains sensitive information and we have some ideas on
+ // to remove it, but don't have the resources to do it right now (e.g.
+ // print).
+ //
+ // ... we don't see value in including it
+ //
+ // "ftrace_packet_allow_list" contains field ids of ftrace packets that we
+ // want to pass onto later transformations. An example would be:
+ //
+ // ... kSchedWakingFieldNumber because it contains cpu activity information
+ //
+ // Compared against track days, the rules around removing ftrace packets are
+ // complicated because...
+ //
+ // packet {
+ // ftrace_packets { <-- ONE-OF (1)
+ // event { <-- REPEATED (2)
+ // cpu_idle { } <-- ONE-OF (3)
+ // }
+ // event { ... }
+ // }
+ // }
+ //
+ // 1. A ftrace packet will populate the one-of slot in the trace packet.
+ //
+ // 2. A ftrace packet can have multiple events
+ //
+ // 3. In this example, a cpu_idle event populates the one-of slot in the
+ // ftrace event
+ base::FlatSet<uint32_t> ftrace_packet_allow_list;
};
// Responsible for extracting low-level data from the trace and storing it in
diff --git a/src/trace_redaction/trace_redactor.cc b/src/trace_redaction/trace_redactor.cc
index 94168e6..49fcf8f 100644
--- a/src/trace_redaction/trace_redactor.cc
+++ b/src/trace_redaction/trace_redactor.cc
@@ -161,17 +161,12 @@
continue;
}
- protozero::HeapBuffered<> serializer;
- auto* packet_message =
- serializer->BeginNestedMessage<TracePacket>(Trace::kPacketFieldNumber);
- packet_message->AppendRawProtoBytes(packet.data(), packet.size());
- packet_message->Finalize();
- serializer->Finalize();
+ protozero::HeapBuffered<protos::pbzero::Trace> serializer;
+ serializer->add_packet()->AppendRawProtoBytes(packet.data(), packet.size());
+ packet.assign(serializer.SerializeAsString());
- auto encoded_packet = serializer.SerializeAsString();
-
- if (const auto exported_data = base::WriteAll(
- dest_fd.get(), encoded_packet.data(), encoded_packet.size());
+ if (const auto exported_data =
+ base::WriteAll(dest_fd.get(), packet.data(), packet.size());
exported_data <= 0) {
return base::ErrStatus("Failed to write redacted trace to disk");
}
diff --git a/src/trace_redaction/trace_redactor_integrationtest.cc b/src/trace_redaction/trace_redactor_integrationtest.cc
index 50074b1..bd7847d 100644
--- a/src/trace_redaction/trace_redactor_integrationtest.cc
+++ b/src/trace_redaction/trace_redactor_integrationtest.cc
@@ -15,28 +15,34 @@
*/
#include <cstdint>
-#include <memory>
#include <string>
#include <string_view>
#include <vector>
#include "perfetto/base/status.h"
#include "perfetto/ext/base/file_utils.h"
-#include "perfetto/ext/base/temp_file.h"
+#include "src/base/test/status_matchers.h"
#include "src/base/test/tmp_dir_tree.h"
#include "src/base/test/utils.h"
#include "src/trace_redaction/find_package_uid.h"
+#include "src/trace_redaction/populate_allow_lists.h"
#include "src/trace_redaction/prune_package_list.h"
+#include "src/trace_redaction/scrub_ftrace_events.h"
+#include "src/trace_redaction/scrub_trace_packet.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "src/trace_redaction/trace_redactor.h"
#include "test/gtest_and_gmock.h"
#include "protos/perfetto/trace/android/packages_list.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
#include "protos/perfetto/trace/trace.pbzero.h"
+#include "protos/perfetto/trace/trace_packet.pbzero.h"
namespace perfetto::trace_redaction {
namespace {
+using FtraceEvent = protos::pbzero::FtraceEvent;
using PackagesList = protos::pbzero::PackagesList;
using PackageInfo = protos::pbzero::PackagesList::PackageInfo;
using Trace = protos::pbzero::Trace;
@@ -55,6 +61,19 @@
protected:
void SetUp() override {
src_trace_ = base::GetTestDataPath(std::string(kTracePath));
+
+ // Add every primitive to the redactor. This should mirror the production
+ // configuration. This configuration may differ to help with verifying the
+ // results.
+ redactor_.collectors()->emplace_back(new FindPackageUid());
+ redactor_.builders()->emplace_back(new PopulateAllowlists());
+ redactor_.transformers()->emplace_back(new PrunePackageList());
+ redactor_.transformers()->emplace_back(new ScrubTracePacket());
+ redactor_.transformers()->emplace_back(new ScrubFtraceEvents());
+
+ // Set the package name to "just some package name". If a specific package
+ // name is needed, it should overwrite this value.
+ context_.package_name = "com.google.omadm.trigger";
}
const std::string& src_trace() const { return src_trace_; }
@@ -77,33 +96,80 @@
return infos;
}
+ static base::StatusOr<std::string> ReadRawTrace(const std::string& path) {
+ std::string redacted_buffer;
+
+ if (base::ReadFile(path, &redacted_buffer)) {
+ return redacted_buffer;
+ }
+
+ return base::ErrStatus("Failed to read %s", path.c_str());
+ }
+
+ // NOTE - this will include fields like "timestamp" and "pid".
+ static void GetEventFields(const Trace::Decoder& trace,
+ base::FlatSet<uint32_t>* set) {
+ for (auto packet_it = trace.packet(); packet_it; ++packet_it) {
+ TracePacket::Decoder packet(*packet_it);
+
+ if (!packet.has_ftrace_events()) {
+ continue;
+ }
+
+ protos::pbzero::FtraceEventBundle::Decoder bundle(packet.ftrace_events());
+
+ if (!bundle.has_event()) {
+ continue;
+ }
+
+ for (auto events_it = bundle.event(); events_it; ++events_it) {
+ protozero::ProtoDecoder event(*events_it);
+
+ for (auto event_it = event.ReadField(); event_it.valid();
+ event_it = event.ReadField()) {
+ set->insert(event_it.id());
+ }
+ }
+ }
+ }
+
+ static base::StatusOr<protozero::ConstBytes> FindFirstFtraceEvents(
+ const Trace::Decoder& trace) {
+ for (auto packet_it = trace.packet(); packet_it; ++packet_it) {
+ TracePacket::Decoder packet(*packet_it);
+
+ if (packet.has_ftrace_events()) {
+ return packet.ftrace_events();
+ }
+ }
+
+ return base::ErrStatus("Failed to find ftrace events");
+ }
+
std::string src_trace_;
base::TmpDirTree tmp_dir_;
+
+ Context context_;
+ TraceRedactor redactor_;
};
TEST_F(TraceRedactorIntegrationTest, FindsPackageAndFiltersPackageList) {
- TraceRedactor redaction;
- redaction.collectors()->emplace_back(new FindPackageUid());
- redaction.transformers()->emplace_back(new PrunePackageList());
+ context_.package_name = "com.Unity.com.unity.multiplayer.samples.coop";
- Context context;
- context.package_name = "com.Unity.com.unity.multiplayer.samples.coop";
-
- auto result = redaction.Redact(
- src_trace(), tmp_dir_.AbsolutePath("dst.pftrace"), &context);
+ auto result = redactor_.Redact(
+ src_trace(), tmp_dir_.AbsolutePath("dst.pftrace"), &context_);
tmp_dir_.TrackFile("dst.pftrace");
- ASSERT_TRUE(result.ok()) << result.message();
+ ASSERT_OK(result);
- std::string redacted_buffer;
- ASSERT_TRUE(
- base::ReadFile(tmp_dir_.AbsolutePath("dst.pftrace"), &redacted_buffer));
+ ASSERT_OK_AND_ASSIGN(auto redacted_buffer,
+ ReadRawTrace(tmp_dir_.AbsolutePath("dst.pftrace")));
Trace::Decoder redacted_trace(redacted_buffer);
std::vector<protozero::ConstBytes> infos = GetPackageInfos(redacted_trace);
- ASSERT_TRUE(context.package_uid.has_value());
- ASSERT_EQ(NormalizeUid(context.package_uid.value()),
+ ASSERT_TRUE(context_.package_uid.has_value());
+ ASSERT_EQ(NormalizeUid(context_.package_uid.value()),
NormalizeUid(kPackageUid));
// It is possible for two packages_list to appear in the trace. The
@@ -132,21 +198,15 @@
// the package list, so there is no way to differentiate these packages (only
// the uid is used later), so each entry should remain.
TEST_F(TraceRedactorIntegrationTest, RetainsAllInstancesOfUid) {
- TraceRedactor redaction;
- redaction.collectors()->emplace_back(new FindPackageUid());
- redaction.transformers()->emplace_back(new PrunePackageList());
+ context_.package_name = "com.google.android.networkstack.tethering";
- Context context;
- context.package_name = "com.google.android.networkstack.tethering";
-
- auto result = redaction.Redact(
- src_trace(), tmp_dir_.AbsolutePath("dst.pftrace"), &context);
+ auto result = redactor_.Redact(
+ src_trace(), tmp_dir_.AbsolutePath("dst.pftrace"), &context_);
tmp_dir_.TrackFile("dst.pftrace");
- ASSERT_TRUE(result.ok()) << result.message();
+ ASSERT_OK(result);
- std::string redacted_buffer;
- ASSERT_TRUE(
- base::ReadFile(tmp_dir_.AbsolutePath("dst.pftrace"), &redacted_buffer));
+ ASSERT_OK_AND_ASSIGN(auto redacted_buffer,
+ ReadRawTrace(tmp_dir_.AbsolutePath("dst.pftrace")));
Trace::Decoder redacted_trace(redacted_buffer);
std::vector<protozero::ConstBytes> infos = GetPackageInfos(redacted_trace);
@@ -174,5 +234,110 @@
ASSERT_EQ(package_names[7], "com.google.android.networkstack.tethering");
}
+// Makes sure all not-allowed ftrace event is removed from a trace.
+TEST_F(TraceRedactorIntegrationTest, RemovesFtraceEvents) {
+ auto pre_redaction_file = src_trace();
+ auto post_redaction_file = tmp_dir_.AbsolutePath("dst.pftrace");
+
+ // We know that there are two oom score updates in the test trace. These
+ // events are not in the allowlist and should be dropped.
+ auto pre_redaction_buffer = ReadRawTrace(pre_redaction_file);
+ ASSERT_OK(pre_redaction_buffer) << pre_redaction_buffer.status().message();
+ Trace::Decoder pre_redaction_trace(*pre_redaction_buffer);
+
+ base::FlatSet<uint32_t> pre_redaction_event_types;
+ GetEventFields(pre_redaction_trace, &pre_redaction_event_types);
+ ASSERT_GT(pre_redaction_event_types.count(
+ FtraceEvent::kOomScoreAdjUpdateFieldNumber),
+ 0u);
+
+ auto result =
+ redactor_.Redact(pre_redaction_file, post_redaction_file, &context_);
+ tmp_dir_.TrackFile("dst.pftrace");
+ ASSERT_OK(result) << result.message();
+
+ auto post_redaction_buffer = ReadRawTrace(post_redaction_file);
+ ASSERT_OK(post_redaction_buffer) << post_redaction_buffer.status().message();
+ Trace::Decoder post_redaction_trace(*post_redaction_buffer);
+
+ base::FlatSet<uint32_t> post_redaction_event_types;
+ GetEventFields(post_redaction_trace, &post_redaction_event_types);
+ ASSERT_EQ(post_redaction_event_types.count(
+ FtraceEvent::kOomScoreAdjUpdateFieldNumber),
+ 0u);
+}
+
+// When a event is dropped from ftrace_events, only that event should be droped,
+// the other events in the ftrace_events should be retained.
+TEST_F(TraceRedactorIntegrationTest,
+ RetainsFtraceEventsWhenRemovingFtraceEvent) {
+ auto pre_redaction_file = src_trace();
+ auto post_redaction_file = tmp_dir_.AbsolutePath("dst.pftrace");
+
+ auto pre_redaction_buffer = ReadRawTrace(pre_redaction_file);
+ ASSERT_OK(pre_redaction_buffer) << pre_redaction_buffer.status().message();
+
+ Trace::Decoder pre_redaction_trace(*pre_redaction_buffer);
+
+ auto pre_redaction_first_events = FindFirstFtraceEvents(pre_redaction_trace);
+ ASSERT_OK(pre_redaction_first_events)
+ << pre_redaction_first_events.status().message();
+
+ auto result =
+ redactor_.Redact(pre_redaction_file, post_redaction_file, &context_);
+ tmp_dir_.TrackFile("dst.pftrace");
+ ASSERT_OK(result) << result.message();
+
+ auto post_redaction_buffer = ReadRawTrace(post_redaction_file);
+ ASSERT_OK(post_redaction_buffer) << post_redaction_buffer.status().message();
+
+ Trace::Decoder post_redaction_trace(*post_redaction_buffer);
+
+ auto post_redaction_ftrace_events =
+ FindFirstFtraceEvents(post_redaction_trace);
+ ASSERT_OK(post_redaction_ftrace_events)
+ << post_redaction_ftrace_events.status().message();
+
+ base::FlatSet<uint32_t> events_before;
+ GetEventFields(pre_redaction_trace, &events_before);
+ ASSERT_EQ(events_before.size(), 14u);
+ ASSERT_TRUE(events_before.count(FtraceEvent::kTimestampFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kPidFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kPrintFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kSchedSwitchFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kCpuFrequencyFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kCpuIdleFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kSchedWakeupFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kSchedWakingFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kSchedWakeupNewFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kTaskNewtaskFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kTaskRenameFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kSchedProcessExitFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kSchedProcessFreeFieldNumber));
+ ASSERT_TRUE(events_before.count(FtraceEvent::kOomScoreAdjUpdateFieldNumber));
+
+ base::FlatSet<uint32_t> events_after;
+ GetEventFields(post_redaction_trace, &events_after);
+ ASSERT_EQ(events_after.size(), 9u);
+
+ // Retained.
+ ASSERT_TRUE(events_after.count(FtraceEvent::kTimestampFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kPidFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kSchedSwitchFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kCpuFrequencyFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kCpuIdleFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kSchedWakingFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kTaskNewtaskFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kTaskRenameFieldNumber));
+ ASSERT_TRUE(events_after.count(FtraceEvent::kSchedProcessFreeFieldNumber));
+
+ // Dropped.
+ ASSERT_FALSE(events_after.count(FtraceEvent::kPrintFieldNumber));
+ ASSERT_FALSE(events_after.count(FtraceEvent::kSchedWakeupFieldNumber));
+ ASSERT_FALSE(events_after.count(FtraceEvent::kSchedWakeupNewFieldNumber));
+ ASSERT_FALSE(events_after.count(FtraceEvent::kSchedProcessExitFieldNumber));
+ ASSERT_FALSE(events_after.count(FtraceEvent::kOomScoreAdjUpdateFieldNumber));
+}
+
} // namespace
} // namespace perfetto::trace_redaction
diff --git a/src/tracing/service/tracing_service_impl.cc b/src/tracing/service/tracing_service_impl.cc
index a72754c..b57f135 100644
--- a/src/tracing/service/tracing_service_impl.cc
+++ b/src/tracing/service/tracing_service_impl.cc
@@ -16,12 +16,11 @@
#include "src/tracing/service/tracing_service_impl.h"
-#include <errno.h>
#include <limits.h>
-#include <stdint.h>
#include <string.h>
#include <cinttypes>
+#include <cstdint>
#include <limits>
#include <optional>
#include <regex>
@@ -59,7 +58,7 @@
#include "perfetto/ext/base/file_utils.h"
#include "perfetto/ext/base/metatrace.h"
#include "perfetto/ext/base/string_utils.h"
-#include "perfetto/ext/base/temp_file.h"
+#include "perfetto/ext/base/string_view.h"
#include "perfetto/ext/base/utils.h"
#include "perfetto/ext/base/uuid.h"
#include "perfetto/ext/base/version.h"
@@ -156,8 +155,8 @@
// Format (by bit range):
// [ 31 ][ 30 ][ 29:20 ][ 19:10 ][ 9:0]
// [unused][has flush id][num chunks to patch][num chunks to move][producer id]
-static int32_t EncodeCommitDataRequest(ProducerID producer_id,
- const CommitDataRequest& req_untrusted) {
+int32_t EncodeCommitDataRequest(ProducerID producer_id,
+ const CommitDataRequest& req_untrusted) {
uint32_t cmov = static_cast<uint32_t>(req_untrusted.chunks_to_move_size());
uint32_t cpatch = static_cast<uint32_t>(req_untrusted.chunks_to_patch_size());
uint32_t has_flush_id = req_untrusted.flush_request_id() != 0;
@@ -1015,7 +1014,7 @@
auto range = data_sources_.equal_range(cfg_data_source.config().name());
for (auto it = range.first; it != range.second; it++) {
TraceConfig::ProducerConfig producer_config;
- for (auto& config : cfg.producers()) {
+ for (const auto& config : cfg.producers()) {
if (GetProducer(it->second.producer_id)->name_ ==
config.producer_name()) {
producer_config = config;
@@ -1184,7 +1183,7 @@
// If it wasn't previously setup, set it up now.
// (The per-producer config is optional).
TraceConfig::ProducerConfig producer_config;
- for (auto& config : tracing_session->config.producers()) {
+ for (const auto& config : tracing_session->config.producers()) {
if (producer->name_ == config.producer_name()) {
producer_config = config;
break;
@@ -1796,13 +1795,13 @@
data_source_instances[producer_id].push_back(ds_inst.instance_id);
}
FlushDataSourceInstances(tracing_session, timeout_ms, data_source_instances,
- callback, flush_flags);
+ std::move(callback), flush_flags);
}
void TracingServiceImpl::FlushDataSourceInstances(
TracingSession* tracing_session,
uint32_t timeout_ms,
- std::map<ProducerID, std::vector<DataSourceInstanceID>>
+ const std::map<ProducerID, std::vector<DataSourceInstanceID>>&
data_source_instances,
ConsumerEndpoint::FlushCallback callback,
FlushFlags flush_flags) {
@@ -2706,7 +2705,7 @@
}
TraceConfig::ProducerConfig producer_config;
- for (auto& config : tracing_session.config.producers()) {
+ for (const auto& config : tracing_session.config.producers()) {
if (producer->name_ == config.producer_name()) {
producer_config = config;
break;
@@ -3519,6 +3518,8 @@
utsname_info->set_machine(uname_info.machine);
utsname_info->set_release(uname_info.release);
}
+ info->set_page_size(static_cast<uint32_t>(sysconf(_SC_PAGESIZE)));
+ info->set_num_cpus(static_cast<uint32_t>(sysconf(_SC_NPROCESSORS_CONF)));
#endif // !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
#if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID)
std::string fingerprint_value = base::GetAndroidProp("ro.build.fingerprint");
@@ -3535,8 +3536,6 @@
} else {
PERFETTO_ELOG("Unable to read ro.build.version.sdk");
}
- info->set_hz(sysconf(_SC_CLK_TCK));
- info->set_page_size(static_cast<uint32_t>(sysconf(_SC_PAGESIZE)));
#endif // PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID)
packet->set_trusted_uid(static_cast<int32_t>(uid_));
packet->set_trusted_packet_sequence_id(kServicePacketSequenceID);
@@ -3573,7 +3572,7 @@
return a.first < b.first;
});
- for (const auto& pair : timestamped_packets)
+ for (auto& pair : timestamped_packets)
SerializeAndAppendPacket(packets, std::move(pair.second));
}
@@ -3608,14 +3607,14 @@
auto* sync_exchange_msg = remote_clock_sync->add_synced_clocks();
auto* client_snapshots = sync_exchange_msg->set_client_clocks();
- for (auto& client_clock : sync_exchange.client_clocks) {
+ for (const auto& client_clock : sync_exchange.client_clocks) {
auto* clock = client_snapshots->add_clocks();
clock->set_clock_id(client_clock.clock_id);
clock->set_timestamp(client_clock.timestamp);
}
auto* host_snapshots = sync_exchange_msg->set_host_clocks();
- for (auto& host_clock : sync_exchange.host_clocks) {
+ for (const auto& host_clock : sync_exchange.host_clocks) {
auto* clock = host_snapshots->add_clocks();
clock->set_clock_id(host_clock.clock_id);
clock->set_timestamp(host_clock.timestamp);
@@ -3805,7 +3804,7 @@
std::map<ProducerID, std::vector<DataSourceInstanceID>>
TracingServiceImpl::GetFlushableDataSourceInstancesForBuffers(
TracingSession* session,
- std::set<BufferID> bufs) {
+ const std::set<BufferID>& bufs) {
std::map<ProducerID, std::vector<DataSourceInstanceID>> data_source_instances;
for (const auto& [producer_id, ds_inst] : session->data_source_instances) {
@@ -3825,7 +3824,7 @@
void TracingServiceImpl::OnFlushDoneForClone(TracingSessionID tsid,
PendingCloneID clone_id,
- std::set<BufferID> buf_ids,
+ const std::set<BufferID>& buf_ids,
bool final_flush_outcome) {
TracingSession* src = GetTracingSession(tsid);
// The session might be gone by the time we try to clone it.
@@ -3881,7 +3880,7 @@
bool TracingServiceImpl::DoCloneBuffers(
TracingSession* src,
- std::set<BufferID> buf_ids,
+ const std::set<BufferID>& buf_ids,
std::vector<std::unique_ptr<TraceBuffer>>* buf_snaps) {
PERFETTO_DCHECK(src->num_buffers() == src->config.buffers().size());
buf_snaps->resize(src->buffers_index.size());
@@ -4273,7 +4272,7 @@
int num_started = 0;
for (const auto& kv : sessions)
num_started += kv.second.state == TracingSession::State::STARTED ? 1 : 0;
- svc_state.set_num_sessions_started(static_cast<int>(num_started));
+ svc_state.set_num_sessions_started(num_started);
for (const auto& kv : service_->producers_) {
if (args.sessions_only)
diff --git a/src/tracing/service/tracing_service_impl.h b/src/tracing/service/tracing_service_impl.h
index 5a583ba..a20fa68 100644
--- a/src/tracing/service/tracing_service_impl.h
+++ b/src/tracing/service/tracing_service_impl.h
@@ -21,7 +21,6 @@
#include <functional>
#include <map>
#include <memory>
-#include <mutex>
#include <optional>
#include <random>
#include <set>
@@ -332,8 +331,8 @@
void ApplyChunkPatches(ProducerID,
const std::vector<CommitDataRequest::ChunkToPatch>&);
void NotifyFlushDoneForProducer(ProducerID, FlushRequestID);
- void NotifyDataSourceStarted(ProducerID, const DataSourceInstanceID);
- void NotifyDataSourceStopped(ProducerID, const DataSourceInstanceID);
+ void NotifyDataSourceStarted(ProducerID, DataSourceInstanceID);
+ void NotifyDataSourceStopped(ProducerID, DataSourceInstanceID);
void ActivateTriggers(ProducerID, const std::vector<std::string>& triggers);
// Called by ConsumerEndpointImpl.
@@ -804,14 +803,14 @@
void FlushDataSourceInstances(
TracingSession*,
uint32_t timeout_ms,
- std::map<ProducerID, std::vector<DataSourceInstanceID>>,
+ const std::map<ProducerID, std::vector<DataSourceInstanceID>>&,
ConsumerEndpoint::FlushCallback,
FlushFlags);
std::map<ProducerID, std::vector<DataSourceInstanceID>>
GetFlushableDataSourceInstancesForBuffers(TracingSession*,
- std::set<BufferID>);
+ const std::set<BufferID>&);
bool DoCloneBuffers(TracingSession*,
- std::set<BufferID>,
+ const std::set<BufferID>&,
std::vector<std::unique_ptr<TraceBuffer>>*);
base::Status FinishCloneSession(ConsumerEndpointImpl*,
TracingSessionID,
@@ -821,7 +820,7 @@
base::Uuid*);
void OnFlushDoneForClone(TracingSessionID src_tsid,
PendingCloneID clone_id,
- std::set<BufferID> buf_ids,
+ const std::set<BufferID>& buf_ids,
bool final_flush_outcome);
// Returns true if `*tracing_session` is waiting for a trigger that hasn't
diff --git a/test/trace_processor/diff_tests/syntax/filtering_tests.py b/test/trace_processor/diff_tests/syntax/filtering_tests.py
index adf8c3e..22b4111 100644
--- a/test/trace_processor/diff_tests/syntax/filtering_tests.py
+++ b/test/trace_processor/diff_tests/syntax/filtering_tests.py
@@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from python.generators.diff_tests.testing import DataPath
+from python.generators.diff_tests.testing import DataPath, TextProto
from python.generators.diff_tests.testing import Csv
from python.generators.diff_tests.testing import DiffTestBlueprint
from python.generators.diff_tests.testing import TestSuite
@@ -189,3 +189,49 @@
"cnt"
0
"""))
+
+ def test_string_null_vs_empty(self):
+ return DiffTestBlueprint(
+ trace=TextProto(""),
+ query="""
+ CREATE PERFETTO TABLE foo AS
+ SELECT 0 as id, NULL AS strings
+ UNION ALL
+ SELECT 1, 'cheese'
+ UNION ALL
+ SELECT 2, NULL
+ UNION ALL
+ SELECT 3, '';
+
+ SELECT * FROM foo ORDER BY strings ASC;
+ """,
+ out=Csv("""
+ "id","strings"
+ 0,"[NULL]"
+ 2,"[NULL]"
+ 3,""
+ 1,"cheese"
+ """))
+
+ def test_string_null_vs_empty_desc(self):
+ return DiffTestBlueprint(
+ trace=TextProto(""),
+ query="""
+ CREATE PERFETTO TABLE foo AS
+ SELECT 0 as id, NULL AS strings
+ UNION ALL
+ SELECT 1, 'cheese'
+ UNION ALL
+ SELECT 2, NULL
+ UNION ALL
+ SELECT 3, '';
+
+ SELECT * FROM foo ORDER BY strings DESC;
+ """,
+ out=Csv("""
+ "id","strings"
+ 1,"cheese"
+ 3,""
+ 0,"[NULL]"
+ 2,"[NULL]"
+ """))
