Merge "ipc: Fix dontreply logic when using derived Deferred messages"
diff --git a/gn/BUILD.gn b/gn/BUILD.gn
index ba42365..bce1986 100644
--- a/gn/BUILD.gn
+++ b/gn/BUILD.gn
@@ -134,9 +134,5 @@
}
config("fuzzer_config") {
- cflags = [ "-fsanitize=fuzzer" ]
- ldflags = [
- "-fsanitize=fuzzer",
- "-fsanitize-coverage=trace-pc-guard",
- ]
+ ldflags = [ "-fsanitize=fuzzer" ]
}
diff --git a/gn/perfetto.gni b/gn/perfetto.gni
index d000a5d..6b94451 100644
--- a/gn/perfetto.gni
+++ b/gn/perfetto.gni
@@ -17,7 +17,6 @@
declare_args() {
# The Android blueprint file generator overrides this to true.
build_with_android = false
- use_libfuzzer = false
}
if (!build_with_chromium) {
diff --git a/gn/standalone/sanitizers/BUILD.gn b/gn/standalone/sanitizers/BUILD.gn
index 0fec24a..3d7eeef 100644
--- a/gn/standalone/sanitizers/BUILD.gn
+++ b/gn/standalone/sanitizers/BUILD.gn
@@ -86,6 +86,9 @@
]
defines += [ "UNDEFINED_SANITIZER" ]
}
+ if (use_libfuzzer) {
+ cflags += [ "-fsanitize=fuzzer" ]
+ }
}
config("sanitizer_options_link_helper") {
diff --git a/gn/standalone/sanitizers/vars.gni b/gn/standalone/sanitizers/vars.gni
index 7ad54ab..d1f2760 100644
--- a/gn/standalone/sanitizers/vars.gni
+++ b/gn/standalone/sanitizers/vars.gni
@@ -27,10 +27,14 @@
# Undefined Behaviour Sanitizer.
is_ubsan = false
+
+ # # Compile for fuzzing with LLVM LibFuzzer.
+ use_libfuzzer = false
}
declare_args() {
- using_sanitizer = is_asan || is_lsan || is_tsan || is_msan || is_ubsan
+ using_sanitizer =
+ is_asan || is_lsan || is_tsan || is_msan || is_ubsan || use_libfuzzer
}
assert(!using_sanitizer || is_clang, "is_*san requires is_clang=true'")
diff --git a/src/ipc/BUILD.gn b/src/ipc/BUILD.gn
index 878a039..8eda6b0 100644
--- a/src/ipc/BUILD.gn
+++ b/src/ipc/BUILD.gn
@@ -16,6 +16,13 @@
import("../../gn/ipc_library.gni")
import("../../gn/proto_library.gni")
+# For use_libfuzzer.
+if (!build_with_chromium) {
+ import("//gn/standalone/sanitizers/vars.gni")
+} else {
+ import("//build/config/sanitizers/sanitizers.gni")
+}
+
source_set("ipc") {
public_configs = [ "../../gn:default_config" ]
public_deps = [
diff --git a/src/tracing/core/shared_memory_abi.cc b/src/tracing/core/shared_memory_abi.cc
index 268a647..f1149c3 100644
--- a/src/tracing/core/shared_memory_abi.cc
+++ b/src/tracing/core/shared_memory_abi.cc
@@ -228,6 +228,10 @@
PageHeader* phdr = page_header(page_idx);
uint32_t layout = phdr->layout.load(std::memory_order_relaxed);
const size_t page_chunk_size = GetChunkSizeForLayout(layout);
+
+ // TODO(primiano): this should not be a CHECK, because a malicious producer
+ // could crash us by putting the chunk in an invalid state. This should
+ // gracefully fail. Keep a CHECK until then.
PERFETTO_CHECK(chunk.size() == page_chunk_size);
const uint32_t chunk_state =
((layout >> (chunk_idx * kChunkShift)) & kChunkMask);
@@ -247,6 +251,8 @@
}
const size_t num_chunks = GetNumChunksForLayout(layout);
all_chunks_state &= (1 << (num_chunks * kChunkShift)) - 1;
+
+ // TODO(primiano): should not be a CHECK (same rationale of comment above).
PERFETTO_CHECK(chunk_state == expected_chunk_state);
uint32_t next_layout = layout;
next_layout &= ~(kChunkMask << (chunk_idx * kChunkShift));
diff --git a/src/tracing/core/trace_writer_impl.cc b/src/tracing/core/trace_writer_impl.cc
index 392cd10..f8c2223 100644
--- a/src/tracing/core/trace_writer_impl.cc
+++ b/src/tracing/core/trace_writer_impl.cc
@@ -56,7 +56,10 @@
}
TraceWriterImpl::~TraceWriterImpl() {
- // TODO(primiano): this should also return the current chunk. Add tests.
+ if (cur_chunk_.is_valid()) {
+ cur_packet_->Finalize();
+ shmem_arbiter_->ReturnCompletedChunk(std::move(cur_chunk_));
+ }
shmem_arbiter_->ReleaseWriterID(id_);
}
diff --git a/src/tracing/core/trace_writer_impl_unittest.cc b/src/tracing/core/trace_writer_impl_unittest.cc
index 48828ed..a1a042f 100644
--- a/src/tracing/core/trace_writer_impl_unittest.cc
+++ b/src/tracing/core/trace_writer_impl_unittest.cc
@@ -56,15 +56,18 @@
::testing::ValuesIn(kPageSizes));
TEST_P(TraceWriterImplTest, SingleWriter) {
- const BufferID tgt_buf_id = 42;
- std::unique_ptr<TraceWriter> writer = arbiter_->CreateTraceWriter(tgt_buf_id);
- for (int i = 0; i < 32; i++) {
+ const BufferID kBufId = 42;
+ std::unique_ptr<TraceWriter> writer = arbiter_->CreateTraceWriter(kBufId);
+ const size_t kNumPackets = 32;
+ for (size_t i = 0; i < kNumPackets; i++) {
auto packet = writer->NewTracePacket();
char str[16];
- sprintf(str, "foobar %d", i);
+ sprintf(str, "foobar %zu", i);
packet->set_test(str);
}
- writer->NewTracePacket()->set_test("workaround for returing the last chunk");
+
+ // Destroying the TraceWriteImpl should cause the last packet to be finalized
+ // and the chunk to be put back in the kChunkComplete state.
writer.reset();
SharedMemoryABI* abi = arbiter_->shmem_abi_for_testing();
@@ -73,14 +76,16 @@
uint32_t page_layout = abi->page_layout_dbg(page_idx);
size_t num_chunks = SharedMemoryABI::GetNumChunksForLayout(page_layout);
for (size_t chunk_idx = 0; chunk_idx < num_chunks; chunk_idx++) {
- auto chunk =
- abi->TryAcquireChunkForReading(page_idx, chunk_idx, tgt_buf_id);
+ auto chunk_state = abi->GetChunkState(page_idx, chunk_idx);
+ ASSERT_TRUE(chunk_state == SharedMemoryABI::kChunkFree ||
+ chunk_state == SharedMemoryABI::kChunkComplete);
+ auto chunk = abi->TryAcquireChunkForReading(page_idx, chunk_idx, kBufId);
if (!chunk.is_valid())
continue;
packets_count += chunk.header()->packets_state.load().count;
}
}
- EXPECT_EQ(32u, packets_count);
+ EXPECT_EQ(kNumPackets, packets_count);
// TODO(primiano): check also the content of the packets decoding the protos.
}
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 8870555..5672e5e 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -15,6 +15,13 @@
import("../gn/perfetto.gni")
import("//build_overrides/build.gni")
+# For use_libfuzzer.
+if (!build_with_chromium) {
+ import("//gn/standalone/sanitizers/vars.gni")
+} else {
+ import("//build/config/sanitizers/sanitizers.gni")
+}
+
source_set("end_to_end_integrationtests") {
testonly = true
deps = [
diff --git a/test/end_to_end_shared_memory_fuzzer.cc b/test/end_to_end_shared_memory_fuzzer.cc
index cca7023..f8267c4 100644
--- a/test/end_to_end_shared_memory_fuzzer.cc
+++ b/test/end_to_end_shared_memory_fuzzer.cc
@@ -138,8 +138,8 @@
// Setup the TraceConfig for the consumer.
TraceConfig trace_config;
- trace_config.add_buffers()->set_size_kb(4096 * 10);
- trace_config.set_duration_ms(200);
+ trace_config.add_buffers()->set_size_kb(4);
+ trace_config.set_duration_ms(10);
// Create the buffer for ftrace.
auto* ds_config = trace_config.add_data_sources()->mutable_config();