Trace Redaction - Expand 'scrub ftrace events' test coverage
1. Added integration test that pushes an actual trace through 'scrub ftrace
events' and makes sure the allow-listed events are not affected.
2. Remove the old serialization tests and replaces them with tests that:
a: uses TestConfig's DummyFields to communicate and enfore types
(e.g. field_sfixed32).
b: cover each values full domain (e.g. min int 32 to max int 64).
Bug: 318576499
Change-Id: Ib1c936289ab769f0d104a487f28c251408696464diff --git a/Android.bp b/Android.bp
index 8d476ce..3233f92 100644
--- a/Android.bp
+++ b/Android.bp
@@ -12492,6 +12492,7 @@
srcs: [
"src/trace_redaction/find_package_uid.cc",
"src/trace_redaction/populate_allow_lists.cc",
+ "src/trace_redaction/proto_util.cc",
"src/trace_redaction/prune_package_list.cc",
"src/trace_redaction/scrub_ftrace_events.cc",
"src/trace_redaction/scrub_trace_packet.cc",
@@ -12505,6 +12506,7 @@
name: "perfetto_src_trace_redaction_unittests",
srcs: [
"src/trace_redaction/find_package_uid_unittest.cc",
+ "src/trace_redaction/proto_util_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/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index 7b2f4c5..b662902 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -32,6 +32,8 @@
"find_package_uid.h",
"populate_allow_lists.cc",
"populate_allow_lists.h",
+ "proto_util.cc",
+ "proto_util.h",
"prune_package_list.cc",
"prune_package_list.h",
"scrub_ftrace_events.cc",
@@ -60,7 +62,10 @@
source_set("integrationtests") {
testonly = true
- sources = [ "trace_redactor_integrationtest.cc" ]
+ sources = [
+ "scrub_ftrace_events_integrationtest.cc",
+ "trace_redactor_integrationtest.cc",
+ ]
deps = [
":trace_redaction",
"../../gn:default_deps",
@@ -77,6 +82,7 @@
testonly = true
sources = [
"find_package_uid_unittest.cc",
+ "proto_util_unittest.cc",
"prune_package_list_unittest.cc",
"scrub_ftrace_events_unittest.cc",
"scrub_trace_packet_unittest.cc",
@@ -86,6 +92,8 @@
"../../gn:default_deps",
"../../gn:gtest_and_gmock",
"../../include/perfetto/protozero:protozero",
+ "../../protos/perfetto/config:cpp",
+ "../../protos/perfetto/config:zero",
"../../protos/perfetto/trace:non_minimal_cpp",
"../../protos/perfetto/trace:zero",
"../../protos/perfetto/trace/android:cpp",
diff --git a/src/trace_redaction/proto_util.cc b/src/trace_redaction/proto_util.cc
new file mode 100644
index 0000000..30962d7
--- /dev/null
+++ b/src/trace_redaction/proto_util.cc
@@ -0,0 +1,59 @@
+/*
+ * 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/proto_util.h"
+
+#include "perfetto/protozero/field.h"
+#include "perfetto/protozero/message.h"
+
+namespace perfetto::trace_redaction {
+namespace proto_util {
+
+// This is copied from "src/protozero/field.cc", but was modified to use the
+// serialization methods provided in "perfetto/protozero/message.h".
+void 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 proto_util
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/proto_util.h b/src/trace_redaction/proto_util.h
new file mode 100644
index 0000000..6fac1d8
--- /dev/null
+++ b/src/trace_redaction/proto_util.h
@@ -0,0 +1,36 @@
+/*
+ * 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_PROTO_UTIL_H_
+#define SRC_TRACE_REDACTION_PROTO_UTIL_H_
+
+#include "perfetto/protozero/field.h"
+#include "perfetto/protozero/message.h"
+
+namespace perfetto::trace_redaction {
+
+// This is here, and not in protozero, because field and message are never found
+// together. Because trace redaction is the only user of this function, it is
+// here.
+namespace proto_util {
+
+void AppendField(const protozero::Field& field, protozero::Message* message);
+
+} // namespace proto_util
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_PROTO_UTIL_H_
diff --git a/src/trace_redaction/proto_util_unittest.cc b/src/trace_redaction/proto_util_unittest.cc
new file mode 100644
index 0000000..aa5be3a
--- /dev/null
+++ b/src/trace_redaction/proto_util_unittest.cc
@@ -0,0 +1,270 @@
+/*
+ * 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 "perfetto/protozero/scattered_heap_buffer.h"
+
+#include "src/trace_redaction/proto_util.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/config/test_config.gen.h"
+#include "protos/perfetto/config/test_config.pbzero.h"
+#include "protos/perfetto/trace/trace_packet.gen.h"
+
+namespace perfetto::trace_redaction {
+
+namespace proto_util {
+
+class ProtoUtilTest : public testing::Test {
+ protected:
+ void Reserialize(const protos::gen::TestConfig_DummyFields& fields) {
+ // Serialize the object and then deserialize it with proto decoder. This is
+ // needed to get the fields.
+ auto serialized = fields.SerializeAsString();
+ protozero::ProtoDecoder decoder(serialized);
+
+ protozero::HeapBuffered<protos::pbzero::TestConfig_DummyFields> message;
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ AppendField(field, message.get());
+ }
+
+ auto reserialized = message.SerializeAsString();
+
+ ASSERT_EQ(serialized, reserialized);
+ }
+};
+
+class ProtoUtilUint32Test : public ProtoUtilTest,
+ public testing::WithParamInterface<uint32_t> {};
+class ProtoUtilUint64Test : public ProtoUtilTest,
+ public testing::WithParamInterface<uint64_t> {};
+class ProtoUtilInt32Test : public ProtoUtilTest,
+ public testing::WithParamInterface<int32_t> {};
+class ProtoUtilInt64Test : public ProtoUtilTest,
+ public testing::WithParamInterface<int64_t> {};
+class ProtoUtilFixed32Test : public ProtoUtilTest,
+ public testing::WithParamInterface<uint32_t> {};
+class ProtoUtilFixed64Test : public ProtoUtilTest,
+ public testing::WithParamInterface<uint64_t> {};
+class ProtoUtilSfixed32Test : public ProtoUtilTest,
+ public testing::WithParamInterface<int32_t> {};
+class ProtoUtilSfixed64Test : public ProtoUtilTest,
+ public testing::WithParamInterface<int64_t> {};
+class ProtoUtilDoubleTest : public ProtoUtilTest,
+ public testing::WithParamInterface<double> {};
+class ProtoUtilFloatTest : public ProtoUtilTest,
+ public testing::WithParamInterface<float> {};
+class ProtoUtilSint32Test : public ProtoUtilTest,
+ public testing::WithParamInterface<int32_t> {};
+class ProtoUtilSint64Test : public ProtoUtilTest,
+ public testing::WithParamInterface<int64_t> {};
+class ProtoUtilStringTest : public ProtoUtilTest,
+ public testing::WithParamInterface<std::string> {};
+class ProtoUtilBytesTest : public ProtoUtilTest,
+ public testing::WithParamInterface<std::string> {};
+
+TEST_P(ProtoUtilUint32Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_uint32(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilUint32Test,
+ testing::Values(std::numeric_limits<uint32_t>::min(),
+ 0,
+ 0xFAAAAAAA,
+ std::numeric_limits<uint32_t>::max()));
+
+TEST_P(ProtoUtilUint64Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_uint64(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilUint64Test,
+ testing::Values(std::numeric_limits<uint64_t>::min(),
+ 0,
+ 0xFAAAAAAAAAAAAAAA,
+ std::numeric_limits<uint64_t>::max()));
+
+TEST_P(ProtoUtilInt32Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_int32(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilInt32Test,
+ testing::Values(std::numeric_limits<int32_t>::min(),
+ 0xFAAAAAAA,
+ 0,
+ 0x0AAAAAAA,
+ std::numeric_limits<int32_t>::max()));
+
+TEST_P(ProtoUtilInt64Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_int64(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilInt64Test,
+ testing::Values(std::numeric_limits<int64_t>::min(),
+ 0xFAAAAAAAAAAAAAAA,
+ 0,
+ 0x0AAAAAAAAAAAAAAA,
+ std::numeric_limits<int64_t>::max()));
+
+TEST_P(ProtoUtilFixed32Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_fixed32(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilFixed32Test,
+ testing::Values(std::numeric_limits<uint32_t>::min(),
+ 0,
+ 0xFAAAAAAAAAAAAAAA,
+ std::numeric_limits<uint32_t>::max()));
+
+TEST_P(ProtoUtilSfixed32Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_sfixed32(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilSfixed32Test,
+ testing::Values(std::numeric_limits<int32_t>::min(),
+ 0xFAAAAAAA,
+ 0,
+ 0x0AAAAAAA,
+ std::numeric_limits<int32_t>::max()));
+
+TEST_P(ProtoUtilDoubleTest, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_double(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ Reserialize,
+ ProtoUtilDoubleTest,
+ testing::Values(std::numeric_limits<double>::min(),
+ 0.0,
+ 1.0,
+ std::numeric_limits<double>::infinity(),
+ std::numeric_limits<double>::max()));
+
+TEST_P(ProtoUtilFloatTest, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_float(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilFloatTest,
+ testing::Values(std::numeric_limits<float>::min(),
+ 0.0f,
+ 1.0f,
+ std::numeric_limits<float>::infinity(),
+ std::numeric_limits<float>::max()));
+
+TEST_P(ProtoUtilSint64Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_sint64(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilSint64Test,
+ testing::Values(std::numeric_limits<int64_t>::min(),
+ 0xFAAAAAAAAAAAAAAA,
+ 0,
+ 0x0AAAAAAAAAAAAAAA,
+ std::numeric_limits<int64_t>::max()));
+
+TEST_P(ProtoUtilSint32Test, FullDomain) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_sint32(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilSint32Test,
+ testing::Values(std::numeric_limits<int32_t>::min(),
+ 0xFAAAAAAA,
+ 0,
+ 0x0AAAAAAA,
+ std::numeric_limits<int32_t>::max()));
+
+TEST_P(ProtoUtilStringTest, Various) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_string(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilStringTest,
+ testing::Values("",
+ "a",
+ "abcdefghijklmonpqrstuvwxyz",
+ std::string(1024, 'a')));
+
+TEST_P(ProtoUtilBytesTest, Various) {
+ auto value = GetParam();
+
+ protos::gen::TestConfig_DummyFields fields;
+ fields.set_field_bytes(value);
+ Reserialize(fields);
+}
+
+INSTANTIATE_TEST_SUITE_P(Reserialize,
+ ProtoUtilBytesTest,
+ testing::Values("",
+ "a",
+ "abcdefghijklmonpqrstuvwxyz",
+ std::string(1024, 'a')));
+
+} // namespace proto_util
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/scrub_ftrace_events.cc b/src/trace_redaction/scrub_ftrace_events.cc
index 67585d1..b41fc6e 100644
--- a/src/trace_redaction/scrub_ftrace_events.cc
+++ b/src/trace_redaction/scrub_ftrace_events.cc
@@ -18,9 +18,8 @@
#include <string>
-#include "perfetto/protozero/message.h"
-#include "perfetto/protozero/message_arena.h"
#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "src/trace_redaction/proto_util.h"
#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
#include "protos/perfetto/trace/trace.pbzero.h"
@@ -117,7 +116,7 @@
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());
+ proto_util::AppendField(packet_child_it, packet_msg.get());
continue;
}
@@ -132,13 +131,13 @@
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);
+ proto_util::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);
+ proto_util::AppendField(ftrace_events_it, ftrace_events_msg);
continue;
}
@@ -150,38 +149,4 @@
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
index 9ae6a98..77e6835 100644
--- a/src/trace_redaction/scrub_ftrace_events.h
+++ b/src/trace_redaction/scrub_ftrace_events.h
@@ -17,8 +17,8 @@
#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 <string>
+
#include "src/trace_redaction/trace_redaction_framework.h"
namespace perfetto::trace_redaction {
@@ -51,10 +51,6 @@
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
diff --git a/src/trace_redaction/scrub_ftrace_events_integrationtest.cc b/src/trace_redaction/scrub_ftrace_events_integrationtest.cc
new file mode 100644
index 0000000..7584b0f
--- /dev/null
+++ b/src/trace_redaction/scrub_ftrace_events_integrationtest.cc
@@ -0,0 +1,159 @@
+/*
+ * 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 <string>
+#include <string_view>
+#include <vector>
+
+#include "perfetto/base/status.h"
+#include "perfetto/ext/base/file_utils.h"
+#include "src/base/test/status_matchers.h"
+#include "src/base/test/utils.h"
+#include "src/trace_redaction/scrub_ftrace_events.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/trace//ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/android/packages_list.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;
+using TracePacket = protos::pbzero::TracePacket;
+
+constexpr std::string_view kTracePath =
+ "test/data/trace-redaction-general.pftrace";
+
+// Runs ScrubFtraceEvents over an actual trace, verifying packet integrity when
+// fields are removed.
+class ScrubFtraceEventsIntegrationTest : public testing::Test {
+ public:
+ ScrubFtraceEventsIntegrationTest() = default;
+ ~ScrubFtraceEventsIntegrationTest() override = default;
+
+ protected:
+ void SetUp() override {
+ src_trace_ = base::GetTestDataPath(std::string(kTracePath));
+ context_.ftrace_packet_allow_list.insert(
+ protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber);
+ }
+
+ std::string src_trace_;
+
+ Context context_; // Used for allowlist.
+ ScrubFtraceEvents transform_;
+
+ 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());
+ }
+
+ // Gets spans for `event` messages that contain `sched_switch` messages.
+ static std::vector<protozero::ConstBytes> GetEventsWithSchedSwitch(
+ protos::pbzero::TracePacket::Decoder packet) {
+ std::vector<protozero::ConstBytes> ranges;
+
+ if (!packet.has_ftrace_events()) {
+ return ranges;
+ }
+
+ protos::pbzero::FtraceEventBundle::Decoder bundle(packet.ftrace_events());
+
+ if (!bundle.has_event()) {
+ return ranges;
+ }
+
+ for (auto event_it = bundle.event(); event_it; ++event_it) {
+ protos::pbzero::FtraceEvent::Decoder event(*event_it);
+
+ if (event.has_sched_switch()) {
+ ranges.push_back(*event_it);
+ }
+ }
+
+ return ranges;
+ }
+
+ // Instead of using the allow-list created by PopulateAllowlist, use a simpler
+ // allowlist; an allowlist that contains most value types.
+ //
+ // uint64....FtraceEvent...............timestamp
+ // uint32....FtraceEvent...............pid
+ //
+ // int32.....SchedSwitchFtraceEvent....prev_pid
+ // int64.....SchedSwitchFtraceEvent....prev_state
+ // string....SchedSwitchFtraceEvent....next_comm
+ //
+ // Compare all switch events in each trace. The comparison is only on the
+ // switch packets, not on the data leading up to or around them.
+ static void ComparePackets(protos::pbzero::TracePacket::Decoder left,
+ protos::pbzero::TracePacket::Decoder right) {
+ auto left_switches = GetEventsWithSchedSwitch(std::move(left));
+ auto right_switches = GetEventsWithSchedSwitch(std::move(right));
+
+ ASSERT_EQ(left_switches.size(), right_switches.size());
+
+ auto left_switch_it = left_switches.begin();
+ auto right_switch_it = right_switches.begin();
+
+ while (left_switch_it != left_switches.end() &&
+ right_switch_it != right_switches.end()) {
+ auto left_switch_str = left_switch_it->ToStdString();
+ auto right_switch_str = right_switch_it->ToStdString();
+
+ ASSERT_EQ(left_switch_str, right_switch_str);
+
+ ++left_switch_it;
+ ++right_switch_it;
+ }
+
+ ASSERT_EQ(left_switches.size(), right_switches.size());
+ }
+};
+
+TEST_F(ScrubFtraceEventsIntegrationTest, FindsPackageAndFiltersPackageList) {
+ const auto& src_file = src_trace_;
+
+ auto raw_src_trace = ReadRawTrace(src_file);
+ ASSERT_OK(raw_src_trace);
+
+ protos::pbzero::Trace::Decoder source_trace(raw_src_trace.value());
+
+ for (auto packet_it = source_trace.packet(); packet_it; ++packet_it) {
+ auto packet = packet_it->as_std_string();
+ ASSERT_OK(transform_.Transform(context_, &packet));
+
+ protos::pbzero::TracePacket::Decoder left_packet(*packet_it);
+ protos::pbzero::TracePacket::Decoder right_packet(packet);
+
+ ComparePackets(std::move(left_packet), std::move(right_packet));
+ }
+}
+
+} // namespace
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/scrub_ftrace_events_unittest.cc b/src/trace_redaction/scrub_ftrace_events_unittest.cc
index 179c504..5b5012f 100644
--- a/src/trace_redaction/scrub_ftrace_events_unittest.cc
+++ b/src/trace_redaction/scrub_ftrace_events_unittest.cc
@@ -13,9 +13,9 @@
* 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 "protos/perfetto/trace/ftrace/power.gen.h"
#include "src/base/test/status_matchers.h"
#include "test/gtest_and_gmock.h"
@@ -23,123 +23,42 @@
#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 {
+// Tests which nested messages and fields are removed.
+class ScrubFtraceEventsTest : public testing::Test {
public:
- ScrubFtraceEventsSerializationTest() = default;
- ~ScrubFtraceEventsSerializationTest() override = default;
+ ScrubFtraceEventsTest() = default;
+ ~ScrubFtraceEventsTest() 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());
+ // task_rename should be in the allow-list.
+ static 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);
}
- 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;
+ static 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);
+ }
};
-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) {
+TEST_F(ScrubFtraceEventsTest, ReturnErrorForNullPacket) {
// Have something in the allow-list to avoid that error.
Context context;
context.ftrace_packet_allow_list = {
@@ -149,7 +68,7 @@
ASSERT_FALSE(scrub.Transform(context, nullptr).ok());
}
-TEST(ScrubFtraceEventsTest, ReturnErrorForEmptyPacket) {
+TEST_F(ScrubFtraceEventsTest, ReturnErrorForEmptyPacket) {
// Have something in the allow-list to avoid that error.
Context context;
context.ftrace_packet_allow_list = {
@@ -161,7 +80,7 @@
ASSERT_FALSE(scrub.Transform(context, &packet_str).ok());
}
-TEST(ScrubFtraceEventsTest, ReturnErrorForEmptyAllowList) {
+TEST_F(ScrubFtraceEventsTest, ReturnErrorForEmptyAllowList) {
// The context will have no allow-list entries. ScrubFtraceEvents should fail.
Context context;
@@ -172,7 +91,7 @@
ASSERT_FALSE(scrub.Transform(context, &packet_str).ok());
}
-TEST(ScrubFtraceEventsTest, IgnorePacketWithNoFtraceEvents) {
+TEST_F(ScrubFtraceEventsTest, IgnorePacketWithNoFtraceEvents) {
protos::gen::TracePacket trace_packet;
auto* tree = trace_packet.mutable_process_tree();
@@ -203,7 +122,7 @@
// There are some values in a ftrace event that sits behind the ftrace bundle.
// These values should be retained.
-TEST(ScrubFtraceEventsTest, KeepsFtraceBundleSiblingValues) {
+TEST_F(ScrubFtraceEventsTest, KeepsFtraceBundleSiblingValues) {
protos::gen::TracePacket trace_packet;
auto* ftrace_events = trace_packet.mutable_ftrace_events();
@@ -238,7 +157,7 @@
ASSERT_TRUE(gen_events.event().front().has_task_rename());
}
-TEST(ScrubFtraceEventsTest, KeepsAllowedEvents) {
+TEST_F(ScrubFtraceEventsTest, KeepsAllowedEvents) {
Context context;
context.ftrace_packet_allow_list = {
protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
@@ -284,7 +203,7 @@
}
// Only the specific non-allowed events should be removed from the event list.
-TEST(ScrubFtraceEventsTest, OnlyDropsNotAllowedEvents) {
+TEST_F(ScrubFtraceEventsTest, OnlyDropsNotAllowedEvents) {
// AddTaskRename >> Keep
// AddClockSetRate >> Drop
protos::gen::TracePacket original_packet;
