Merge "UI: reintroduce "table-layout: fixed" for aggregation details panel"
diff --git a/gn/standalone/libc++/BUILD.gn b/gn/standalone/libc++/BUILD.gn
index 809a448..f6b6131 100644
--- a/gn/standalone/libc++/BUILD.gn
+++ b/gn/standalone/libc++/BUILD.gn
@@ -20,6 +20,13 @@
"_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS",
"_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS",
]
+ if (is_debug) {
+ # libc++ has two levels of debug mode. Setting _LIBCPP_DEBUG to zero
+ # enables most assertions. Setting it to one additionally enables iterator
+ # debugging, but that seems to require some extra link-time dependencies.
+ # See https://libcxx.llvm.org/docs/DesignDocs/DebugMode.html
+ defines += [ "_LIBCPP_DEBUG=0" ]
+ }
cflags_cc = [
"-nostdinc++",
"-isystem" + rebase_path("$libcxx_prefix/include", root_build_dir),
@@ -28,9 +35,7 @@
# Avoid linking both libc++ and libstdc++.
ldflags = [ "-nostdlib++" ]
- libs = [
- "dl", # libdl: dynamic linking.
- ]
+ libs = [ "dl" ] # libdl: dynamic linking.
}
}
diff --git a/src/kallsyms/kernel_symbol_map.cc b/src/kallsyms/kernel_symbol_map.cc
index 920c897..126d6dd 100644
--- a/src/kallsyms/kernel_symbol_map.cc
+++ b/src/kallsyms/kernel_symbol_map.cc
@@ -202,7 +202,7 @@
*(tok_wptr++) = token.at(i) & 0x7f;
}
*(tok_wptr++) = static_cast<char>(token.at(token_size - 1) | 0x80);
- PERFETTO_DCHECK(tok_wptr == &buf_[buf_.size()]);
+ PERFETTO_DCHECK(tok_wptr == buf_.data() + buf_.size());
return id;
}
@@ -394,7 +394,7 @@
uint32_t addr = it->first;
uint32_t off = it->second;
const uint8_t* rdptr = &buf_[off];
- const uint8_t* const buf_end = &buf_[buf_.size()];
+ const uint8_t* const buf_end = buf_.data() + buf_.size();
bool parsing_addr = true;
const uint8_t* next_rdptr = nullptr;
uint64_t sym_start_addr = 0;
diff --git a/src/perfetto_cmd/pbtxt_to_pb.cc b/src/perfetto_cmd/pbtxt_to_pb.cc
index f4f0c00..b78d938 100644
--- a/src/perfetto_cmd/pbtxt_to_pb.cc
+++ b/src/perfetto_cmd/pbtxt_to_pb.cc
@@ -47,12 +47,20 @@
namespace {
+constexpr bool IsOct(char c) {
+ return (c >= '0' && c <= '7');
+}
+
+constexpr bool IsDigit(char c) {
+ return (c >= '0' && c <= '9');
+}
+
constexpr bool IsIdentifierStart(char c) {
return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z') || c == '_';
}
constexpr bool IsIdentifierBody(char c) {
- return IsIdentifierStart(c) || isdigit(c);
+ return IsIdentifierStart(c) || IsDigit(c);
}
const char* FieldToTypeName(const FieldDescriptorProto* field) {
@@ -150,19 +158,22 @@
}
void NumericField(Token key, Token value) {
- const FieldDescriptorProto* field = FindFieldByName(
- key, value,
- {
- FieldDescriptorProto::TYPE_UINT64,
- FieldDescriptorProto::TYPE_UINT32, FieldDescriptorProto::TYPE_INT64,
- FieldDescriptorProto::TYPE_SINT64, FieldDescriptorProto::TYPE_INT32,
- FieldDescriptorProto::TYPE_SINT32,
- FieldDescriptorProto::TYPE_FIXED64,
- FieldDescriptorProto::TYPE_SFIXED64,
- FieldDescriptorProto::TYPE_FIXED32,
- FieldDescriptorProto::TYPE_SFIXED32,
- FieldDescriptorProto::TYPE_DOUBLE, FieldDescriptorProto::TYPE_FLOAT,
- });
+ const FieldDescriptorProto* field =
+ FindFieldByName(key, value,
+ {
+ FieldDescriptorProto::TYPE_UINT64,
+ FieldDescriptorProto::TYPE_UINT32,
+ FieldDescriptorProto::TYPE_INT64,
+ FieldDescriptorProto::TYPE_SINT64,
+ FieldDescriptorProto::TYPE_INT32,
+ FieldDescriptorProto::TYPE_SINT32,
+ FieldDescriptorProto::TYPE_FIXED64,
+ FieldDescriptorProto::TYPE_SFIXED64,
+ FieldDescriptorProto::TYPE_FIXED32,
+ FieldDescriptorProto::TYPE_SFIXED32,
+ FieldDescriptorProto::TYPE_DOUBLE,
+ FieldDescriptorProto::TYPE_FLOAT,
+ });
if (!field)
return;
const auto& field_type = field->type();
@@ -202,11 +213,12 @@
}
void StringField(Token key, Token value) {
- const FieldDescriptorProto* field = FindFieldByName(
- key, value,
- {
- FieldDescriptorProto::TYPE_STRING, FieldDescriptorProto::TYPE_BYTES,
- });
+ const FieldDescriptorProto* field =
+ FindFieldByName(key, value,
+ {
+ FieldDescriptorProto::TYPE_STRING,
+ FieldDescriptorProto::TYPE_BYTES,
+ });
if (!field)
return;
uint32_t field_id = static_cast<uint32_t>(field->number());
@@ -217,15 +229,16 @@
std::unique_ptr<char, base::FreeDeleter> s(
static_cast<char*>(malloc(value.size())));
size_t j = 0;
+ const char* const txt = value.txt.data();
for (size_t i = 0; i < value.size(); i++) {
- char c = value.txt.data()[i];
+ char c = txt[i];
if (c == '\\') {
if (i + 1 >= value.size()) {
// This should be caught by the lexer.
PERFETTO_FATAL("Escape at end of string.");
return;
}
- char next = value.txt.data()[++i];
+ char next = txt[++i];
switch (next) {
case '\\':
case '\'':
@@ -254,6 +267,44 @@
case 'v':
s.get()[j++] = '\v';
break;
+ case '0':
+ case '1':
+ case '2':
+ case '3':
+ case '4':
+ case '5':
+ case '6':
+ case '7':
+ case '8':
+ case '9': {
+ // Cases 8 and 9 are not really required and are only added for the
+ // sake of error reporting.
+ bool oct_err = false;
+ if (i + 2 >= value.size() || !IsOct(txt[i + 1]) ||
+ !IsOct(txt[i + 2])) {
+ oct_err = true;
+ } else {
+ char buf[4]{next, txt[++i], txt[++i], '\0'};
+ auto octval = base::CStringToUInt32(buf, 8);
+ if (!octval.has_value() || *octval > 0xff) {
+ oct_err = true;
+ } else {
+ s.get()[j++] = static_cast<char>(static_cast<uint8_t>(*octval));
+ }
+ }
+ if (oct_err) {
+ AddError(value,
+ "Malformed string escape in $k in proto $n on '$v'. "
+ "\\NNN escapes must be exactly three octal digits <= "
+ "\\377 (0xff).",
+ std::map<std::string, std::string>{
+ {"$k", key.ToStdString()},
+ {"$n", descriptor_name()},
+ {"$v", value.ToStdString()},
+ });
+ }
+ break;
+ }
default:
AddError(value,
"Unknown string escape in $k in "
@@ -273,11 +324,12 @@
}
void IdentifierField(Token key, Token value) {
- const FieldDescriptorProto* field = FindFieldByName(
- key, value,
- {
- FieldDescriptorProto::TYPE_BOOL, FieldDescriptorProto::TYPE_ENUM,
- });
+ const FieldDescriptorProto* field =
+ FindFieldByName(key, value,
+ {
+ FieldDescriptorProto::TYPE_BOOL,
+ FieldDescriptorProto::TYPE_ENUM,
+ });
if (!field)
return;
uint32_t field_id = static_cast<uint32_t>(field->number());
@@ -415,7 +467,8 @@
if (!field_descriptor) {
AddError(key, "No field named \"$n\" in proto $p",
{
- {"$n", field_name}, {"$p", descriptor_name()},
+ {"$n", field_name},
+ {"$p", descriptor_name()},
});
return nullptr;
}
@@ -549,7 +602,7 @@
state = kReadingStringValue;
continue;
}
- if (c == '-' || isdigit(c) || c == '.') {
+ if (c == '-' || IsDigit(c) || c == '.') {
state = kReadingNumericValue;
continue;
}
@@ -576,7 +629,7 @@
delegate->NumericField(key, value);
continue;
}
- if (isdigit(c) || c == '.')
+ if (IsDigit(c) || c == '.')
continue;
break;
diff --git a/src/perfetto_cmd/pbtxt_to_pb_unittest.cc b/src/perfetto_cmd/pbtxt_to_pb_unittest.cc
index 00dab16..251c8f0 100644
--- a/src/perfetto_cmd/pbtxt_to_pb_unittest.cc
+++ b/src/perfetto_cmd/pbtxt_to_pb_unittest.cc
@@ -403,6 +403,7 @@
ftrace_events: "newline\nnewline"
ftrace_events: "\"quoted\""
ftrace_events: "\a\b\f\n\r\t\v\\\'\"\?"
+ ftrace_events: "\0127_\03422.\177"
}
}
}
@@ -417,6 +418,7 @@
EXPECT_THAT(events, Contains("newline\nnewline"));
EXPECT_THAT(events, Contains("\"quoted\""));
EXPECT_THAT(events, Contains("\a\b\f\n\r\t\v\\\'\"\?"));
+ EXPECT_THAT(events, Contains("\0127_\03422.\177"));
}
TEST(PbtxtToPb, UnknownField) {
diff --git a/src/profiling/memory/bookkeeping.h b/src/profiling/memory/bookkeeping.h
index 6aa9086..77a1319 100644
--- a/src/profiling/memory/bookkeeping.h
+++ b/src/profiling/memory/bookkeeping.h
@@ -193,8 +193,9 @@
void ClearFrameCache() { frame_cache_.clear(); }
- uint64_t committed_timestamp() { return committed_timestamp_; }
- uint64_t max_timestamp() { return max_timestamp_; }
+ uint64_t dump_timestamp() {
+ return dump_at_max_mode_ ? max_timestamp_ : committed_timestamp_;
+ }
uint64_t GetSizeForTesting(const std::vector<unwindstack::FrameData>& stack,
std::vector<std::string> build_ids);
diff --git a/src/profiling/memory/bookkeeping_unittest.cc b/src/profiling/memory/bookkeeping_unittest.cc
index 79e0183..4dc16ca 100644
--- a/src/profiling/memory/bookkeeping_unittest.cc
+++ b/src/profiling/memory/bookkeeping_unittest.cc
@@ -145,7 +145,7 @@
sequence_number++;
hd.RecordMalloc(stack2(), DummyBuildIds(stack2().size()), 0x2, 1, 2,
sequence_number, 100 * sequence_number);
- ASSERT_EQ(hd.max_timestamp(), 200u);
+ ASSERT_EQ(hd.dump_timestamp(), 200u);
ASSERT_EQ(hd.GetMaxForTesting(stack(), DummyBuildIds(stack().size())), 5u);
ASSERT_EQ(hd.GetMaxForTesting(stack2(), DummyBuildIds(stack2().size())), 2u);
ASSERT_EQ(hd.GetMaxCountForTesting(stack(), DummyBuildIds(stack().size())),
@@ -171,7 +171,7 @@
sequence_number, 100 * sequence_number);
sequence_number++;
hd.RecordFree(0x2, sequence_number, 100 * sequence_number);
- EXPECT_EQ(hd.max_timestamp(), 400u);
+ EXPECT_EQ(hd.dump_timestamp(), 400u);
EXPECT_EQ(hd.GetMaxForTesting(stack(), DummyBuildIds(stack().size())), 0u);
EXPECT_EQ(hd.GetMaxForTesting(stack2(), DummyBuildIds(stack2().size())), 15u);
EXPECT_EQ(hd.GetMaxForTesting(stack3(), DummyBuildIds(stack3().size())), 15u);
diff --git a/src/profiling/memory/heapprofd_producer.cc b/src/profiling/memory/heapprofd_producer.cc
index dfc6922..9865a47 100644
--- a/src/profiling/memory/heapprofd_producer.cc
+++ b/src/profiling/memory/heapprofd_producer.cc
@@ -699,39 +699,26 @@
bool from_startup = data_source->signaled_pids.find(pid) ==
data_source->signaled_pids.cend();
- uint64_t dump_timestamp;
- if (data_source->config.dump_at_max())
- dump_timestamp = heap_info.heap_tracker.max_timestamp();
- else
- dump_timestamp = heap_info.heap_tracker.committed_timestamp();
- const char* heap_name = nullptr;
- if (!heap_info.heap_name.empty())
- heap_name = heap_info.heap_name.c_str();
- uint64_t sampling_interval = heap_info.sampling_interval;
- uint64_t orig_sampling_interval = heap_info.orig_sampling_interval;
-
- auto new_heapsamples =
- [pid, from_startup, dump_timestamp, process_state, data_source,
- heap_name, sampling_interval,
- orig_sampling_interval](ProfilePacket::ProcessHeapSamples* proto) {
- proto->set_pid(static_cast<uint64_t>(pid));
- proto->set_timestamp(dump_timestamp);
- proto->set_from_startup(from_startup);
- proto->set_disconnected(process_state->disconnected);
- proto->set_buffer_overran(process_state->error_state ==
- SharedRingBuffer::kHitTimeout);
- proto->set_client_error(
- ErrorStateToProto(process_state->error_state));
- proto->set_buffer_corrupted(process_state->buffer_corrupted);
- proto->set_hit_guardrail(data_source->hit_guardrail);
- if (heap_name)
- proto->set_heap_name(heap_name);
- proto->set_sampling_interval_bytes(sampling_interval);
- proto->set_orig_sampling_interval_bytes(orig_sampling_interval);
- auto* stats = proto->set_stats();
- SetStats(stats, *process_state);
- };
+ auto new_heapsamples = [pid, from_startup, process_state, data_source,
+ &heap_info](
+ ProfilePacket::ProcessHeapSamples* proto) {
+ proto->set_pid(static_cast<uint64_t>(pid));
+ proto->set_timestamp(heap_info.heap_tracker.dump_timestamp());
+ proto->set_from_startup(from_startup);
+ proto->set_disconnected(process_state->disconnected);
+ proto->set_buffer_overran(process_state->error_state ==
+ SharedRingBuffer::kHitTimeout);
+ proto->set_client_error(ErrorStateToProto(process_state->error_state));
+ proto->set_buffer_corrupted(process_state->buffer_corrupted);
+ proto->set_hit_guardrail(data_source->hit_guardrail);
+ if (!heap_info.heap_name.empty())
+ proto->set_heap_name(heap_info.heap_name.c_str());
+ proto->set_sampling_interval_bytes(heap_info.sampling_interval);
+ proto->set_orig_sampling_interval_bytes(heap_info.orig_sampling_interval);
+ auto* stats = proto->set_stats();
+ SetStats(stats, *process_state);
+ };
DumpState dump_state(data_source->trace_writer.get(),
std::move(new_heapsamples),
diff --git a/src/profiling/memory/wire_protocol.cc b/src/profiling/memory/wire_protocol.cc
index 29ea042..9d30beb 100644
--- a/src/profiling/memory/wire_protocol.cc
+++ b/src/profiling/memory/wire_protocol.cc
@@ -58,6 +58,10 @@
template <typename F>
int64_t WithBuffer(SharedRingBuffer* shmem, size_t total_size, F fn) {
+ if (total_size > shmem->size()) {
+ errno = EMSGSIZE;
+ return -1;
+ }
SharedRingBuffer::Buffer buf;
{
ScopedSpinlock lock = shmem->AcquireLock(ScopedSpinlock::Mode::Try);
diff --git a/src/protozero/filtering/BUILD.gn b/src/protozero/filtering/BUILD.gn
index b438929..ffd1c8c 100644
--- a/src/protozero/filtering/BUILD.gn
+++ b/src/protozero/filtering/BUILD.gn
@@ -83,7 +83,6 @@
":bytecode_common",
":bytecode_generator",
":bytecode_parser",
- ":filter_util",
":message_filter",
"..:protozero",
"../../../gn:default_deps",
@@ -95,10 +94,19 @@
sources = [
"filter_bytecode_generator_unittest.cc",
"filter_bytecode_parser_unittest.cc",
- "filter_util_unittest.cc",
- "message_filter_unittest.cc",
"message_tokenizer_unittest.cc",
]
+
+ # On chromium component build we cannot have a test target depening boh on
+ # protobuf-full and protobuf-lite. Just skip those targets there.
+ # See also https://crug.com/1210223 .
+ if (perfetto_build_standalone || perfetto_build_with_android) {
+ deps += [ ":filter_util" ]
+ sources += [
+ "filter_util_unittest.cc",
+ "message_filter_unittest.cc",
+ ]
+ }
}
if (enable_perfetto_benchmarks) {
diff --git a/src/protozero/filtering/filter_util.cc b/src/protozero/filtering/filter_util.cc
index c17b620..94d436e 100644
--- a/src/protozero/filtering/filter_util.cc
+++ b/src/protozero/filtering/filter_util.cc
@@ -23,6 +23,7 @@
#include <google/protobuf/compiler/importer.h>
+#include "perfetto/base/build_config.h"
#include "perfetto/ext/base/file_utils.h"
#include "perfetto/ext/base/getopt.h"
#include "perfetto/ext/base/string_utils.h"
@@ -68,13 +69,34 @@
bool FilterUtil::LoadMessageDefinition(const std::string& proto_file,
const std::string& root_message,
const std::string& proto_dir_path) {
+ // The protobuf compiler doesn't like backslashes and prints an error like:
+ // Error C:\it7mjanpw3\perfetto-a16500 -1:0: Backslashes, consecutive slashes,
+ // ".", or ".." are not allowed in the virtual path.
+ // Given that C:\foo\bar is a legit path on windows, fix it at this level
+ // because the problem is really the protobuf compiler being too picky.
+ static auto normalize_for_win = [](const std::string& path) {
+#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+ return perfetto::base::ReplaceAll(path, "\\", "/");
+#else
+ return path;
+#endif
+ };
+
google::protobuf::compiler::DiskSourceTree dst;
- dst.MapPath("/", "/");
- dst.MapPath("", proto_dir_path);
+#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+ // If the path is absolute, maps "C:/" -> "C:/" (without hardcoding 'C').
+ if (proto_file.size() > 3 && proto_file[1] == ':') {
+ char win_drive[4];
+ sprintf(win_drive, "%c:/", proto_file[0]);
+ dst.MapPath(win_drive, win_drive);
+ }
+#endif
+ dst.MapPath("/", "/"); // We might still need this on Win under cygwin.
+ dst.MapPath("", normalize_for_win(proto_dir_path));
MultiFileErrorCollectorImpl mfe;
google::protobuf::compiler::Importer importer(&dst, &mfe);
const google::protobuf::FileDescriptor* root_file =
- importer.Import(proto_file);
+ importer.Import(normalize_for_win(proto_file));
const google::protobuf::Descriptor* root_msg = nullptr;
if (!root_message.empty()) {
root_msg = importer.pool()->FindMessageTypeByName(root_message);
diff --git a/src/trace_processor/metrics/android/android_startup_launches.sql b/src/trace_processor/metrics/android/android_startup_launches.sql
index 2cfb4b9..c5829d7 100644
--- a/src/trace_processor/metrics/android/android_startup_launches.sql
+++ b/src/trace_processor/metrics/android/android_startup_launches.sql
@@ -104,11 +104,11 @@
DROP TABLE IF EXISTS launch_processes;
CREATE TABLE launch_processes(launch_id INT, upid BIG INT);
--- We make the (not always correct) simplification that process == package
INSERT INTO launch_processes
SELECT launches.id, process.upid
FROM launches
- JOIN process ON launches.package = process.name
+ LEFT JOIN package_list ON (launches.package = package_list.package_name)
+ JOIN process ON (launches.package = process.name OR process.uid = package_list.uid)
JOIN thread ON (process.upid = thread.upid AND process.pid = thread.tid)
WHERE (process.start_ts IS NULL OR process.start_ts < launches.ts_end)
AND (thread.end_ts IS NULL OR thread.end_ts > launches.ts_end)
diff --git a/src/trace_processor/metrics/metrics.cc b/src/trace_processor/metrics/metrics.cc
index e8539cc..fe2492c 100644
--- a/src/trace_processor/metrics/metrics.cc
+++ b/src/trace_processor/metrics/metrics.cc
@@ -264,6 +264,14 @@
return util::OkStatus();
}
+ if (size > protozero::proto_utils::kMaxMessageLength) {
+ return util::ErrStatus(
+ "Message passed to field %s in proto message %s has size %zu which is "
+ "larger than the maximum allowed message size %zu",
+ field.name().c_str(), descriptor_->full_name().c_str(), size,
+ protozero::proto_utils::kMaxMessageLength);
+ }
+
protos::pbzero::ProtoBuilderResult::Decoder decoder(ptr, size);
if (decoder.is_repeated()) {
return util::ErrStatus("Cannot handle nested repeated messages in field %s",
@@ -305,6 +313,14 @@
util::Status ProtoBuilder::AppendRepeated(const FieldDescriptor& field,
const uint8_t* ptr,
size_t size) {
+ if (size > protozero::proto_utils::kMaxMessageLength) {
+ return util::ErrStatus(
+ "Message passed to field %s in proto message %s has size %zu which is "
+ "larger than the maximum allowed message size %zu",
+ field.name().c_str(), descriptor_->full_name().c_str(), size,
+ protozero::proto_utils::kMaxMessageLength);
+ }
+
protos::pbzero::ProtoBuilderResult::Decoder decoder(ptr, size);
if (!decoder.is_repeated()) {
return util::ErrStatus(
diff --git a/tools/gen_android_bp b/tools/gen_android_bp
index 9c4f408..5ae4b85 100755
--- a/tools/gen_android_bp
+++ b/tools/gen_android_bp
@@ -166,8 +166,8 @@
# Skip test data files that require GCS. They are only for benchmarks.
# We don't run benchmarks in the android tree.
continue
- if line.endswith('/'):
- yield line + '**/*'
+ if line.endswith('/.'):
+ yield line[:-1] + '**/*'
else:
yield line
diff --git a/tools/test_data.txt b/tools/test_data.txt
index 55ee848..0585217 100644
--- a/tools/test_data.txt
+++ b/tools/test_data.txt
@@ -1,7 +1,10 @@
# List of test deps that should be pushed on the device. Paths are relative
# to the root.
-src/traced/probes/filesystem/testdata/
-src/traced/probes/ftrace/test/data/
+# The trailing /. at the end of a directory is to avoid creating a nesting
+# directory when pushing a 2nd-time. adb push has a slightly different behavior
+# than `cp` on directoriesm, trailing slash is not enough.
+src/traced/probes/filesystem/testdata/.
+src/traced/probes/ftrace/test/data/.
test/data/android_log_ring_buffer_mode.pb
test/data/example_android_trace_30s.pb
test/data/full_trace_filter.bytecode
