Use protozero in IPC framing protocol
This CL switches the root layer of the IPC to protozero.
Note that this CL doesn't fully remove the libprotobuf
dep from the IPC layer. The auto-generated .ipc.{cc,h}
classes, in fact, still refer to the .pb.h headers for
now (the next CL will sort that out).
This CL takes care only of the raw IPC machinery that
deals with framing (BufferedFrameDeserializer).
The major change introduced is making varint decoding
resilient against ints larger than 64 bits. The IPC
layer needs to defend against too-large frames.
Also failing varint decoding on numbers > 2**64
seems generally a nice property.
I manually inspected all other uses of ParseVarInt
and adapted the only other affected one (ProtoDecoder)
to this new behavior.
Bug: 132880619
Change-Id: I2c8b64ee7cf175a07f70845db04f0639bf3291afdiff --git a/Android.bp b/Android.bp
index 94ff35d..2c498d1 100644
--- a/Android.bp
+++ b/Android.bp
@@ -95,7 +95,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_trace_android_zero_gen",
":perfetto_protos_perfetto_trace_chrome_zero_gen",
":perfetto_protos_perfetto_trace_filesystem_zero_gen",
@@ -166,7 +167,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_zero_gen_headers",
"perfetto_protos_perfetto_trace_filesystem_zero_gen_headers",
@@ -300,7 +302,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_trace_android_zero_gen",
":perfetto_protos_perfetto_trace_chrome_zero_gen",
":perfetto_protos_perfetto_trace_filesystem_zero_gen",
@@ -380,7 +383,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_zero_gen_headers",
"perfetto_protos_perfetto_trace_filesystem_zero_gen_headers",
@@ -484,7 +488,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_trace_android_zero_gen",
":perfetto_protos_perfetto_trace_chrome_zero_gen",
":perfetto_protos_perfetto_trace_filesystem_zero_gen",
@@ -548,7 +553,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_zero_gen_headers",
"perfetto_protos_perfetto_trace_filesystem_zero_gen_headers",
@@ -596,7 +602,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_zero_gen_headers",
"perfetto_protos_perfetto_trace_filesystem_zero_gen_headers",
@@ -665,7 +672,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_trace_android_zero_gen",
":perfetto_protos_perfetto_trace_chrome_zero_gen",
":perfetto_protos_perfetto_trace_filesystem_zero_gen",
@@ -731,7 +739,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_zero_gen_headers",
"perfetto_protos_perfetto_trace_filesystem_zero_gen_headers",
@@ -914,7 +923,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_trace_android_lite_gen",
":perfetto_protos_perfetto_trace_android_zero_gen",
":perfetto_protos_perfetto_trace_chrome_lite_gen",
@@ -1025,7 +1035,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_lite_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_lite_gen_headers",
@@ -2336,33 +2347,71 @@
],
}
-// GN: //protos/perfetto/ipc:wire_protocol
+// GN: //protos/perfetto/ipc:wire_protocol_cpp
genrule {
- name: "perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ name: "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
srcs: [
"protos/perfetto/ipc/wire_protocol.proto",
],
tools: [
"aprotoc",
+ "perfetto_src_protozero_protoc_plugin_cppgen_plugin",
],
- cmd: "mkdir -p $(genDir)/external/perfetto/ && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/ --proto_path=external/perfetto $(in)",
+ cmd: "mkdir -p $(genDir)/external/perfetto/ && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/ --proto_path=external/perfetto --plugin=protoc-gen-plugin=$(location perfetto_src_protozero_protoc_plugin_cppgen_plugin) --plugin_out=:$(genDir)/external/perfetto/ $(in)",
out: [
- "external/perfetto/protos/perfetto/ipc/wire_protocol.pb.cc",
+ "external/perfetto/protos/perfetto/ipc/wire_protocol.gen.cc",
],
}
-// GN: //protos/perfetto/ipc:wire_protocol
+// GN: //protos/perfetto/ipc:wire_protocol_cpp
genrule {
- name: "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ name: "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
srcs: [
"protos/perfetto/ipc/wire_protocol.proto",
],
tools: [
"aprotoc",
+ "perfetto_src_protozero_protoc_plugin_cppgen_plugin",
],
- cmd: "mkdir -p $(genDir)/external/perfetto/ && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/ --proto_path=external/perfetto $(in)",
+ cmd: "mkdir -p $(genDir)/external/perfetto/ && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/ --proto_path=external/perfetto --plugin=protoc-gen-plugin=$(location perfetto_src_protozero_protoc_plugin_cppgen_plugin) --plugin_out=:$(genDir)/external/perfetto/ $(in)",
out: [
- "external/perfetto/protos/perfetto/ipc/wire_protocol.pb.h",
+ "external/perfetto/protos/perfetto/ipc/wire_protocol.gen.h",
+ ],
+ export_include_dirs: [
+ ".",
+ "protos",
+ ],
+}
+
+// GN: //protos/perfetto/ipc:wire_protocol_zero
+genrule {
+ name: "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
+ srcs: [
+ "protos/perfetto/ipc/wire_protocol.proto",
+ ],
+ tools: [
+ "aprotoc",
+ "protozero_plugin",
+ ],
+ cmd: "mkdir -p $(genDir)/external/perfetto/ && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/ --proto_path=external/perfetto --plugin=protoc-gen-plugin=$(location protozero_plugin) --plugin_out=wrapper_namespace=pbzero:$(genDir)/external/perfetto/ $(in)",
+ out: [
+ "external/perfetto/protos/perfetto/ipc/wire_protocol.pbzero.cc",
+ ],
+}
+
+// GN: //protos/perfetto/ipc:wire_protocol_zero
+genrule {
+ name: "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
+ srcs: [
+ "protos/perfetto/ipc/wire_protocol.proto",
+ ],
+ tools: [
+ "aprotoc",
+ "protozero_plugin",
+ ],
+ cmd: "mkdir -p $(genDir)/external/perfetto/ && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/ --proto_path=external/perfetto --plugin=protoc-gen-plugin=$(location protozero_plugin) --plugin_out=wrapper_namespace=pbzero:$(genDir)/external/perfetto/ $(in)",
+ out: [
+ "external/perfetto/protos/perfetto/ipc/wire_protocol.pbzero.h",
],
export_include_dirs: [
".",
@@ -5364,7 +5413,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_metrics_android_zero_gen",
":perfetto_protos_perfetto_metrics_zero_gen",
":perfetto_protos_perfetto_trace_android_lite_gen",
@@ -5509,7 +5559,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_metrics_android_zero_gen_headers",
"perfetto_protos_perfetto_metrics_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_lite_gen_headers",
@@ -5931,7 +5982,8 @@
":perfetto_protos_perfetto_config_sys_stats_zero_gen",
":perfetto_protos_perfetto_config_zero_gen",
":perfetto_protos_perfetto_ipc_ipc_gen",
- ":perfetto_protos_perfetto_ipc_wire_protocol_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen",
+ ":perfetto_protos_perfetto_ipc_wire_protocol_zero_gen",
":perfetto_protos_perfetto_trace_android_zero_gen",
":perfetto_protos_perfetto_trace_chrome_zero_gen",
":perfetto_protos_perfetto_trace_filesystem_zero_gen",
@@ -5994,7 +6046,8 @@
"perfetto_protos_perfetto_config_sys_stats_zero_gen_headers",
"perfetto_protos_perfetto_config_zero_gen_headers",
"perfetto_protos_perfetto_ipc_ipc_gen_headers",
- "perfetto_protos_perfetto_ipc_wire_protocol_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_cpp_gen_headers",
+ "perfetto_protos_perfetto_ipc_wire_protocol_zero_gen_headers",
"perfetto_protos_perfetto_trace_android_zero_gen_headers",
"perfetto_protos_perfetto_trace_chrome_zero_gen_headers",
"perfetto_protos_perfetto_trace_filesystem_zero_gen_headers",
diff --git a/BUILD b/BUILD
index d6d38ab..6616dd3 100644
--- a/BUILD
+++ b/BUILD
@@ -66,7 +66,8 @@
":include_perfetto_ext_ipc_ipc",
],
deps = [
- ":protos_perfetto_ipc_wire_protocol",
+ ":protos_perfetto_ipc_wire_protocol_cpp",
+ ":protos_perfetto_ipc_wire_protocol_zero",
] + PERFETTO_CONFIG.deps.protobuf_lite,
)
@@ -182,7 +183,8 @@
":protos_perfetto_config_sys_stats_zero",
":protos_perfetto_config_zero",
":protos_perfetto_ipc_ipc",
- ":protos_perfetto_ipc_wire_protocol",
+ ":protos_perfetto_ipc_wire_protocol_cpp",
+ ":protos_perfetto_ipc_wire_protocol_zero",
":protos_perfetto_trace_android_zero",
":protos_perfetto_trace_chrome_zero",
":protos_perfetto_trace_filesystem_zero",
@@ -1581,18 +1583,20 @@
":protos_perfetto_config_protos",
":protos_perfetto_config_sys_stats_protos",
":protos_perfetto_ipc_wire_protocol_protos",
- ],
-)
-
-# GN target: //protos/perfetto/ipc:wire_protocol
-perfetto_cc_proto_library(
- name = "protos_perfetto_ipc_wire_protocol",
- deps = [
":protos_perfetto_ipc_wire_protocol_protos",
],
)
-# GN target: //protos/perfetto/ipc:wire_protocol
+# GN target: //protos/perfetto/ipc:wire_protocol_cpp
+perfetto_cc_protocpp_library(
+ name = "protos_perfetto_ipc_wire_protocol_cpp",
+ deps = [
+ ":protos_perfetto_ipc_wire_protocol_protos",
+ ":protos_perfetto_ipc_wire_protocol_zero",
+ ],
+)
+
+# GN target: //protos/perfetto/ipc:wire_protocol_zero
perfetto_proto_library(
name = "protos_perfetto_ipc_wire_protocol_protos",
srcs = [
@@ -1600,6 +1604,14 @@
],
)
+# GN target: //protos/perfetto/ipc:wire_protocol_zero
+perfetto_cc_protozero_library(
+ name = "protos_perfetto_ipc_wire_protocol_zero",
+ deps = [
+ ":protos_perfetto_ipc_wire_protocol_protos",
+ ],
+)
+
# GN target: //protos/perfetto/metrics/android:lite
perfetto_cc_proto_library(
name = "protos_perfetto_metrics_android_lite",
@@ -2251,7 +2263,8 @@
":protos_perfetto_config_sys_stats_zero",
":protos_perfetto_config_zero",
":protos_perfetto_ipc_ipc",
- ":protos_perfetto_ipc_wire_protocol",
+ ":protos_perfetto_ipc_wire_protocol_cpp",
+ ":protos_perfetto_ipc_wire_protocol_zero",
":protos_perfetto_trace_android_zero",
":protos_perfetto_trace_chrome_zero",
":protos_perfetto_trace_filesystem_zero",
@@ -2330,7 +2343,8 @@
":protos_perfetto_config_sys_stats_zero",
":protos_perfetto_config_zero",
":protos_perfetto_ipc_ipc",
- ":protos_perfetto_ipc_wire_protocol",
+ ":protos_perfetto_ipc_wire_protocol_cpp",
+ ":protos_perfetto_ipc_wire_protocol_zero",
":protos_perfetto_trace_android_zero",
":protos_perfetto_trace_chrome_zero",
":protos_perfetto_trace_filesystem_zero",
diff --git a/include/perfetto/protozero/proto_utils.h b/include/perfetto/protozero/proto_utils.h
index 8a93951..d8f0592 100644
--- a/include/perfetto/protozero/proto_utils.h
+++ b/include/perfetto/protozero/proto_utils.h
@@ -225,20 +225,22 @@
// buffer.
inline const uint8_t* ParseVarInt(const uint8_t* start,
const uint8_t* end,
- uint64_t* value) {
+ uint64_t* out_value) {
const uint8_t* pos = start;
- uint64_t shift = 0;
- *value = 0;
- do {
- if (PERFETTO_UNLIKELY(pos >= end)) {
- *value = 0;
- return start;
+ uint64_t value = 0;
+ for (uint32_t shift = 0; pos < end && shift < 64u; shift += 7) {
+ // Cache *pos into |cur_byte| to prevent that the compiler dereferences the
+ // pointer twice (here and in the if() below) due to char* aliasing rules.
+ uint8_t cur_byte = *pos++;
+ value |= static_cast<uint64_t>(cur_byte & 0x7f) << shift;
+ if ((cur_byte & 0x80) == 0) {
+ // In valid cases we get here.
+ *out_value = value;
+ return pos;
}
- PERFETTO_DCHECK(shift < 64ull);
- *value |= static_cast<uint64_t>(*pos & 0x7f) << shift;
- shift += 7;
- } while (*pos++ & 0x80);
- return pos;
+ }
+ *out_value = 0;
+ return start;
}
} // namespace proto_utils
diff --git a/protos/perfetto/ipc/BUILD.gn b/protos/perfetto/ipc/BUILD.gn
index 48f54a1..c65b9f4 100644
--- a/protos/perfetto/ipc/BUILD.gn
+++ b/protos/perfetto/ipc/BUILD.gn
@@ -30,8 +30,11 @@
]
}
-perfetto_proto_library("wire_protocol") {
- proto_generators = [ "lite" ]
+perfetto_proto_library("wire_protocol_@TYPE@") {
+ proto_generators = [
+ "zero",
+ "cpp",
+ ]
sources = [
"wire_protocol.proto",
]
diff --git a/src/ipc/BUILD.gn b/src/ipc/BUILD.gn
index ed0ecd2..bf2b1af 100644
--- a/src/ipc/BUILD.gn
+++ b/src/ipc/BUILD.gn
@@ -31,7 +31,7 @@
deps = [
"../../gn:default_deps",
"../../gn:protobuf_lite",
- "../../protos/perfetto/ipc:wire_protocol",
+ "../../protos/perfetto/ipc:wire_protocol_cpp",
"../base",
]
sources = [
@@ -54,7 +54,7 @@
deps = [
":ipc",
"../../gn:default_deps",
- "../../protos/perfetto/ipc:wire_protocol",
+ "../../protos/perfetto/ipc:wire_protocol_cpp",
]
}
@@ -65,7 +65,7 @@
":test_messages",
"../../gn:default_deps",
"../../gn:gtest_and_gmock",
- "../../protos/perfetto/ipc:wire_protocol",
+ "../../protos/perfetto/ipc:wire_protocol_cpp",
"../base",
"../base:test_support",
]
diff --git a/src/ipc/buffered_frame_deserializer.cc b/src/ipc/buffered_frame_deserializer.cc
index dc107f7..9ef22eb 100644
--- a/src/ipc/buffered_frame_deserializer.cc
+++ b/src/ipc/buffered_frame_deserializer.cc
@@ -25,7 +25,7 @@
#include "perfetto/base/logging.h"
#include "perfetto/ext/base/utils.h"
-#include "protos/perfetto/ipc/wire_protocol.pb.h"
+#include "protos/perfetto/ipc/wire_protocol.gen.h"
namespace perfetto {
namespace ipc {
@@ -166,23 +166,18 @@
if (size == 0)
return;
std::unique_ptr<Frame> frame(new Frame);
- if (frame->ParseFromArray(data, static_cast<int>(size)))
+ if (frame->ParseFromArray(data, size))
decoded_frames_.push_back(std::move(frame));
}
// static
std::string BufferedFrameDeserializer::Serialize(const Frame& frame) {
+ std::vector<uint8_t> payload = frame.SerializeAsArray();
+ const uint32_t payload_size = static_cast<uint32_t>(payload.size());
std::string buf;
- buf.reserve(1024); // Just an educated guess to avoid trivial expansions.
- buf.insert(0, kHeaderSize, 0); // Reserve the space for the header.
- frame.AppendToString(&buf);
- const uint32_t payload_size = static_cast<uint32_t>(buf.size() - kHeaderSize);
- PERFETTO_DCHECK(payload_size == static_cast<uint32_t>(frame.GetCachedSize()));
- // Don't send messages larger than what the receiver can handle.
- PERFETTO_DCHECK(kHeaderSize + payload_size <= kIPCBufferSize);
- char header[kHeaderSize];
- memcpy(header, base::AssumeLittleEndian(&payload_size), kHeaderSize);
- buf.replace(0, kHeaderSize, header, kHeaderSize);
+ buf.resize(kHeaderSize + payload_size);
+ memcpy(&buf[0], base::AssumeLittleEndian(&payload_size), kHeaderSize);
+ memcpy(&buf[kHeaderSize], payload.data(), payload.size());
return buf;
}
diff --git a/src/ipc/buffered_frame_deserializer.h b/src/ipc/buffered_frame_deserializer.h
index 3aeee1f..92988e6 100644
--- a/src/ipc/buffered_frame_deserializer.h
+++ b/src/ipc/buffered_frame_deserializer.h
@@ -31,7 +31,7 @@
namespace perfetto {
namespace ipc {
-class Frame; // Defined in the protobuf autogenerated wire_protocol.pb.h.
+class Frame; // Defined in the protobuf autogenerated wire_protocol.gen.h.
// Deserializes incoming frames, taking care of buffering and tokenization.
// Used by both host and client to decode incoming frames.
diff --git a/src/ipc/buffered_frame_deserializer_unittest.cc b/src/ipc/buffered_frame_deserializer_unittest.cc
index 792365b..9ddf8d8 100644
--- a/src/ipc/buffered_frame_deserializer_unittest.cc
+++ b/src/ipc/buffered_frame_deserializer_unittest.cc
@@ -23,7 +23,7 @@
#include "perfetto/ext/base/utils.h"
#include "test/gtest_and_gmock.h"
-#include "protos/perfetto/ipc/wire_protocol.pb.h"
+#include "protos/perfetto/ipc/wire_protocol.gen.h"
namespace perfetto {
namespace ipc {
@@ -60,15 +60,16 @@
padding_char = padding_char == 'z' ? '0' : padding_char + 1;
padding[i] = padding_char;
}
- frame.add_data_for_testing(padding.data(), padding_size);
+ frame.add_data_for_testing(std::string(padding.data(), padding_size));
}
- PERFETTO_CHECK(frame.ByteSize() == static_cast<int>(payload_size));
+ PERFETTO_CHECK(frame.SerializeAsString().size() == payload_size);
std::vector<char> encoded_frame;
encoded_frame.resize(size);
char* enc_buf = encoded_frame.data();
- PERFETTO_CHECK(frame.SerializeToArray(enc_buf + kHeaderSize,
- static_cast<int>(payload_size)));
+
+ std::string payload = frame.SerializeAsString();
memcpy(enc_buf, base::AssumeLittleEndian(&payload_size), kHeaderSize);
+ memcpy(enc_buf + kHeaderSize, payload.data(), payload.size());
PERFETTO_CHECK(encoded_frame.size() == size);
return encoded_frame;
}
@@ -108,8 +109,7 @@
// Excactly one frame should be decoded, with no leftover buffer.
auto decoded_frame = bfd.PopNextFrame();
ASSERT_TRUE(decoded_frame);
- ASSERT_EQ(static_cast<int32_t>(size - kHeaderSize),
- decoded_frame->ByteSize());
+ ASSERT_EQ(size - kHeaderSize, decoded_frame->SerializeAsString().size());
ASSERT_FALSE(bfd.PopNextFrame());
ASSERT_EQ(0u, bfd.size());
}
@@ -132,11 +132,11 @@
method->set_id(0x424242);
method->set_name("foo");
std::vector<char> serialized_frame;
- uint32_t payload_size = static_cast<uint32_t>(frame.ByteSize());
+ std::string payload = frame.SerializeAsString();
+ uint32_t payload_size = static_cast<uint32_t>(payload.size());
serialized_frame.resize(kHeaderSize + payload_size);
- ASSERT_TRUE(frame.SerializeToArray(serialized_frame.data() + kHeaderSize,
- static_cast<int>(payload_size)));
+ memcpy(serialized_frame.data() + kHeaderSize, payload.data(), payload_size);
memcpy(serialized_frame.data(), base::AssumeLittleEndian(&payload_size),
kHeaderSize);
@@ -163,8 +163,8 @@
// Validate the received frame2.
std::unique_ptr<Frame> decoded_simple_frame = bfd.PopNextFrame();
ASSERT_TRUE(decoded_simple_frame);
- ASSERT_EQ(static_cast<int32_t>(simple_frame.size() - kHeaderSize),
- decoded_simple_frame->ByteSize());
+ ASSERT_EQ(simple_frame.size() - kHeaderSize,
+ decoded_simple_frame->SerializeAsString().size());
std::unique_ptr<Frame> decoded_frame = bfd.PopNextFrame();
ASSERT_TRUE(decoded_frame);
@@ -238,8 +238,7 @@
for (size_t expected_size : batch) {
auto frame = bfd.PopNextFrame();
ASSERT_TRUE(frame);
- ASSERT_EQ(static_cast<int32_t>(expected_size - kHeaderSize),
- frame->ByteSize());
+ ASSERT_EQ(expected_size - kHeaderSize, frame->SerializeAsString().size());
}
ASSERT_FALSE(bfd.PopNextFrame());
ASSERT_EQ(0u, bfd.size());
@@ -301,8 +300,7 @@
ASSERT_FALSE(decoded_frame);
} else {
ASSERT_TRUE(decoded_frame);
- ASSERT_EQ(static_cast<int32_t>(size - kHeaderSize),
- decoded_frame->ByteSize());
+ ASSERT_EQ(size - kHeaderSize, decoded_frame->SerializeAsString().size());
}
ASSERT_EQ(0u, bfd.size());
}
diff --git a/src/ipc/client_impl.h b/src/ipc/client_impl.h
index 71c211f..35dea50 100644
--- a/src/ipc/client_impl.h
+++ b/src/ipc/client_impl.h
@@ -23,7 +23,7 @@
#include "perfetto/ext/ipc/client.h"
#include "src/ipc/buffered_frame_deserializer.h"
-#include "protos/perfetto/ipc/wire_protocol.pb.h"
+#include "protos/perfetto/ipc/wire_protocol.gen.h"
#include <list>
#include <map>
diff --git a/src/ipc/host_impl.cc b/src/ipc/host_impl.cc
index 1158fb8..7784305 100644
--- a/src/ipc/host_impl.cc
+++ b/src/ipc/host_impl.cc
@@ -26,7 +26,7 @@
#include "perfetto/ext/ipc/service.h"
#include "perfetto/ext/ipc/service_descriptor.h"
-#include "protos/perfetto/ipc/wire_protocol.pb.h"
+#include "protos/perfetto/ipc/wire_protocol.gen.h"
// TODO(primiano): put limits on #connections/uid and req. queue (b/69093705).
diff --git a/src/ipc/host_impl_unittest.cc b/src/ipc/host_impl_unittest.cc
index 4e9216b..61a77dd 100644
--- a/src/ipc/host_impl_unittest.cc
+++ b/src/ipc/host_impl_unittest.cc
@@ -30,7 +30,7 @@
#include "src/ipc/test/test_socket.h"
#include "test/gtest_and_gmock.h"
-#include "protos/perfetto/ipc/wire_protocol.pb.h"
+#include "protos/perfetto/ipc/wire_protocol.gen.h"
#include "src/ipc/test/client_unittest_messages.pb.h"
namespace perfetto {
diff --git a/src/protozero/proto_decoder.cc b/src/protozero/proto_decoder.cc
index a5f1b95..9bf9ca5 100644
--- a/src/protozero/proto_decoder.cc
+++ b/src/protozero/proto_decoder.cc
@@ -60,7 +60,10 @@
if (PERFETTO_LIKELY(*pos < 0x80)) { // Fastpath for fields with ID < 16.
preamble = *(pos++);
} else {
- pos = ParseVarInt(pos, end, &preamble);
+ const uint8_t* next = ParseVarInt(pos, end, &preamble);
+ if (PERFETTO_UNLIKELY(pos == next))
+ return res;
+ pos = next;
}
uint32_t field_id = static_cast<uint32_t>(preamble >> kFieldTypeNumBits);
diff --git a/src/protozero/proto_utils_unittest.cc b/src/protozero/proto_utils_unittest.cc
index a724189..9124490 100644
--- a/src/protozero/proto_utils_unittest.cc
+++ b/src/protozero/proto_utils_unittest.cc
@@ -194,6 +194,8 @@
}
}
+// ParseVarInt() must fail gracefully if we hit the |end| without seeing the
+// MSB == 0 (i.e. end-of-sequence).
TEST(ProtoUtilsTest, VarIntDecodingOutOfBounds) {
uint8_t buf[] = {0xff, 0xff, 0xff, 0xff};
for (size_t i = 0; i < 5; i++) {
@@ -204,6 +206,27 @@
}
}
+// Even if we see a valid end-of-sequence, ParseVarInt() must fail if the number
+// is larger than 10 bytes. That would cause subtl bugs when trying to shift
+// left by more than 64 bits.
+TEST(ProtoUtilsTest, RejectVarIntTooBig) {
+ // This is the biggest valid varint we support (2**64 - 1).
+ uint8_t good[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01};
+
+ // Parsing this value must succeed.
+ uint64_t value = static_cast<uint64_t>(-1);
+ const uint8_t* res = ParseVarInt(&good[0], &good[sizeof(good)], &value);
+ EXPECT_EQ(&good[sizeof(good)], res);
+ EXPECT_EQ(value, static_cast<uint64_t>(-1ULL));
+
+ uint8_t bad[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0x01};
+ value = static_cast<uint64_t>(-1);
+ res = ParseVarInt(&bad[0], &bad[sizeof(bad)], &value);
+ EXPECT_EQ(&bad[0], res);
+ EXPECT_EQ(0u, value);
+}
+
} // namespace
} // namespace proto_utils
} // namespace protozero
