Remove warning suppressions and fix many casting bugs
This change removes most warning suppressions, in particular
from production code. In the past we inherit a bunch of -Wno-xxx
suppressions required to build gtest and libprotobuf headers.
Doing so, however, caused the suppressions to propagate back to
the translation units that were including any protobuf header
or any auto-generated .pb.h stub.
This change moves the gtest and probobuf header to be a
system include (-isystem vs -I). Doing so implicitly blacklists
any compiler warning on the headers.
This CL then re-enables warnings and deals with the fall out of
fixes that came out of this.
Bug: 77316877
Test: pefetto_unittests / perfetto_integrationtests
Change-Id: I3a01852ebf7d0b9bf19658ddf117209d129c70be
diff --git a/src/tracing/core/packet_stream_validator.cc b/src/tracing/core/packet_stream_validator.cc
index 3887f60..8f6f21e 100644
--- a/src/tracing/core/packet_stream_validator.cc
+++ b/src/tracing/core/packet_stream_validator.cc
@@ -33,7 +33,7 @@
size += slice.size;
protos::TrustedPacket packet;
- if (!packet.ParseFromBoundedZeroCopyStream(&stream, size))
+ if (!packet.ParseFromBoundedZeroCopyStream(&stream, static_cast<int>(size)))
return false;
// Only the service is allowed to fill in the trusted uid.
diff --git a/src/tracing/core/service_impl.cc b/src/tracing/core/service_impl.cc
index def0f55..3bb107f 100644
--- a/src/tracing/core/service_impl.cc
+++ b/src/tracing/core/service_impl.cc
@@ -267,8 +267,9 @@
// corresponding BufferID, which is a global ID namespace for the service and
// all producers.
size_t total_buf_size_kb = 0;
- tracing_session->buffers_index.reserve(cfg.buffers_size());
- for (int i = 0; i < cfg.buffers_size(); i++) {
+ const size_t num_buffers = static_cast<size_t>(cfg.buffers_size());
+ tracing_session->buffers_index.reserve(num_buffers);
+ for (size_t i = 0; i < num_buffers; i++) {
const TraceConfig::BufferConfig& buffer_cfg = cfg.buffers()[i];
BufferID global_id = buffer_ids_.Allocate();
if (!global_id) {
@@ -325,14 +326,15 @@
}
// Trigger delayed task if the trace is time limited.
- if (cfg.duration_ms()) {
+ const uint32_t trace_duration_ms = cfg.duration_ms();
+ if (trace_duration_ms > 0) {
auto weak_this = weak_ptr_factory_.GetWeakPtr();
task_runner_->PostDelayedTask(
[weak_this, tsid] {
if (weak_this)
weak_this->FlushAndDisableTracing(tsid);
},
- cfg.duration_ms());
+ trace_duration_ms);
}
// Start the periodic drain tasks if we should to save the trace into a file.
@@ -347,10 +349,11 @@
}
tracing_session->tracing_enabled = true;
- PERFETTO_LOG("Enabled tracing, #sources:%zu, duration:%" PRIu32
- " ms, #buffers:%d, total buffer size:%zu KB, total sessions:%zu",
- cfg.data_sources().size(), cfg.duration_ms(), cfg.buffers_size(),
- total_buf_size_kb, tracing_sessions_.size());
+ PERFETTO_LOG(
+ "Enabled tracing, #sources:%zu, duration:%d ms, #buffers:%d, total "
+ "buffer size:%zu KB, total sessions:%zu",
+ cfg.data_sources().size(), trace_duration_ms, cfg.buffers_size(),
+ total_buf_size_kb, tracing_sessions_.size());
return true;
}
@@ -393,7 +396,7 @@
}
void ServiceImpl::Flush(TracingSessionID tsid,
- int timeout_ms,
+ uint32_t timeout_ms,
ConsumerEndpoint::FlushCallback callback) {
PERFETTO_DCHECK_THREAD(thread_checker_);
TracingSession* tracing_session = GetTracingSession(tsid);
@@ -557,10 +560,10 @@
tbuf.BeginRead();
while (!did_hit_threshold) {
TracePacket packet;
- uid_t producer_uid = -1;
+ uid_t producer_uid = kInvalidUid;
if (!tbuf.ReadNextTracePacket(&packet, &producer_uid))
break;
- PERFETTO_DCHECK(producer_uid != static_cast<uid_t>(-1));
+ PERFETTO_DCHECK(producer_uid != kInvalidUid);
PERFETTO_DCHECK(packet.size() > 0);
if (!PacketStreamValidator::Validate(packet.slices())) {
PERFETTO_DLOG("Dropping invalid packet");
@@ -576,7 +579,7 @@
// the producer can't give us a partial packet (e.g., a truncated
// string) which only becomes valid when the UID is appended here.
protos::TrustedPacket trusted_packet;
- trusted_packet.set_trusted_uid(producer_uid);
+ trusted_packet.set_trusted_uid(static_cast<int32_t>(producer_uid));
static constexpr size_t kTrustedBufSize = 16;
Slice slice = Slice::Allocate(kTrustedBufSize);
PERFETTO_CHECK(
@@ -598,9 +601,9 @@
// |write_into_file| == true in the trace config, drain the packets read
// (if any) into the given file descriptor.
if (tracing_session->write_into_file) {
- const size_t max_size = tracing_session->max_file_size_bytes
- ? tracing_session->max_file_size_bytes
- : std::numeric_limits<size_t>::max();
+ const uint64_t max_size = tracing_session->max_file_size_bytes
+ ? tracing_session->max_file_size_bytes
+ : std::numeric_limits<size_t>::max();
// When writing into a file, the file should look like a root trace.proto
// message. Each packet should be prepended with a proto preamble stating
@@ -611,7 +614,7 @@
bool stop_writing_into_file = tracing_session->write_period_ms == 0;
std::unique_ptr<struct iovec[]> iovecs(new struct iovec[max_iovecs]);
size_t num_iovecs_at_last_packet = 0;
- size_t bytes_about_to_be_written = 0;
+ uint64_t bytes_about_to_be_written = 0;
for (TracePacket& packet : packets) {
std::tie(iovecs[num_iovecs].iov_base, iovecs[num_iovecs].iov_len) =
packet.GetProtoPreamble();
@@ -639,25 +642,25 @@
PERFETTO_DCHECK(num_iovecs <= max_iovecs);
int fd = *tracing_session->write_into_file;
- size_t total_wr_size = 0;
+ uint64_t total_wr_size = 0;
// writev() can take at most IOV_MAX entries per call. Batch them.
constexpr size_t kIOVMax = IOV_MAX;
for (size_t i = 0; i < num_iovecs; i += kIOVMax) {
- size_t iov_batch_size = std::min(num_iovecs - i, kIOVMax);
+ int iov_batch_size = static_cast<int>(std::min(num_iovecs - i, kIOVMax));
ssize_t wr_size = PERFETTO_EINTR(writev(fd, &iovecs[i], iov_batch_size));
if (wr_size <= 0) {
PERFETTO_PLOG("writev() failed");
stop_writing_into_file = true;
break;
}
- total_wr_size += wr_size;
+ total_wr_size += static_cast<size_t>(wr_size);
}
tracing_session->bytes_written_into_file += total_wr_size;
- PERFETTO_DLOG("Draining into file, written: %zu, stop: %d", total_wr_size,
- stop_writing_into_file);
+ PERFETTO_DLOG("Draining into file, written: %" PRIu64 " KB, stop: %d",
+ (total_wr_size + 1023) / 1024, stop_writing_into_file);
if (stop_writing_into_file) {
tracing_session->write_into_file.reset();
tracing_session->write_period_ms = 0;
@@ -1041,16 +1044,17 @@
for (auto& clock : clocks) {
protos::ClockSnapshot::Clock* c = clock_snapshot->add_clocks();
c->set_type(clock.type);
- c->set_timestamp(base::FromPosixTimespec(clock.ts).count());
+ c->set_timestamp(
+ static_cast<uint64_t>(base::FromPosixTimespec(clock.ts).count()));
}
#else // !PERFETTO_BUILDFLAG(PERFETTO_OS_MACOSX)
protos::ClockSnapshot::Clock* c = clock_snapshot->add_clocks();
c->set_type(protos::ClockSnapshot::Clock::MONOTONIC);
- c->set_timestamp(base::GetWallTimeNs().count());
+ c->set_timestamp(static_cast<uint64_t>(base::GetWallTimeNs().count()));
#endif // !PERFETTO_BUILDFLAG(PERFETTO_OS_MACOSX)
- packet.set_trusted_uid(getuid());
- Slice slice = Slice::Allocate(packet.ByteSize());
+ packet.set_trusted_uid(static_cast<int32_t>(getuid()));
+ Slice slice = Slice::Allocate(static_cast<size_t>(packet.ByteSize()));
PERFETTO_CHECK(packet.SerializeWithCachedSizesToArray(slice.own_data()));
packets->emplace_back();
packets->back().AddSlice(std::move(slice));
@@ -1063,8 +1067,8 @@
tracing_session->did_emit_config = true;
protos::TrustedPacket packet;
tracing_session->config.ToProto(packet.mutable_trace_config());
- packet.set_trusted_uid(getuid());
- Slice slice = Slice::Allocate(packet.ByteSize());
+ packet.set_trusted_uid(static_cast<int32_t>(getuid()));
+ Slice slice = Slice::Allocate(static_cast<size_t>(packet.ByteSize()));
PERFETTO_CHECK(packet.SerializeWithCachedSizesToArray(slice.own_data()));
packets->emplace_back();
packets->back().AddSlice(std::move(slice));
@@ -1132,7 +1136,7 @@
tracing_session_id_ = 0;
}
-void ServiceImpl::ConsumerEndpointImpl::Flush(int timeout_ms,
+void ServiceImpl::ConsumerEndpointImpl::Flush(uint32_t timeout_ms,
FlushCallback callback) {
PERFETTO_DCHECK_THREAD(thread_checker_);
if (!tracing_session_id_) {
diff --git a/src/tracing/core/service_impl.h b/src/tracing/core/service_impl.h
index 046e983..c6f7936 100644
--- a/src/tracing/core/service_impl.h
+++ b/src/tracing/core/service_impl.h
@@ -121,7 +121,7 @@
void DisableTracing() override;
void ReadBuffers() override;
void FreeBuffers() override;
- void Flush(int timeout_ms, FlushCallback) override;
+ void Flush(uint32_t timeout_ms, FlushCallback) override;
private:
friend class ServiceImpl;
@@ -164,7 +164,7 @@
base::ScopedFile);
void DisableTracing(TracingSessionID);
void Flush(TracingSessionID tsid,
- int timeout_ms,
+ uint32_t timeout_ms,
ConsumerEndpoint::FlushCallback);
void FlushAndDisableTracing(TracingSessionID);
void ReadBuffers(TracingSessionID, ConsumerEndpointImpl*);
@@ -211,7 +211,7 @@
size_t num_buffers() const { return buffers_index.size(); }
- int delay_to_next_write_period_ms() const {
+ uint32_t delay_to_next_write_period_ms() const {
PERFETTO_DCHECK(write_period_ms > 0);
return write_period_ms -
(base::GetWallTimeMs().count() % write_period_ms);
@@ -250,9 +250,9 @@
// trace packets into, rather than returning it to the consumer via
// OnTraceData().
base::ScopedFile write_into_file;
- int write_period_ms = 0;
- size_t max_file_size_bytes = 0;
- size_t bytes_written_into_file = 0;
+ uint32_t write_period_ms = 0;
+ uint64_t max_file_size_bytes = 0;
+ uint64_t bytes_written_into_file = 0;
};
ServiceImpl(const ServiceImpl&) = delete;
diff --git a/src/tracing/core/service_impl_unittest.cc b/src/tracing/core/service_impl_unittest.cc
index 33fe88d..59e42ac 100644
--- a/src/tracing/core/service_impl_unittest.cc
+++ b/src/tracing/core/service_impl_unittest.cc
@@ -372,8 +372,9 @@
for (size_t i = 0; i < kNumProducers; i++) {
auto* producer_config = trace_config.add_producers();
producer_config->set_producer_name("mock_producer_" + std::to_string(i));
- producer_config->set_shm_size_kb(kConfigSizesKb[i]);
- producer_config->set_page_size_kb(kConfigPageSizesKb[i]);
+ producer_config->set_shm_size_kb(static_cast<uint32_t>(kConfigSizesKb[i]));
+ producer_config->set_page_size_kb(
+ static_cast<uint32_t>(kConfigPageSizesKb[i]));
}
consumer->EnableTracing(trace_config);
diff --git a/src/tracing/core/shared_memory_abi.cc b/src/tracing/core/shared_memory_abi.cc
index 9d2964a..11ef1a1 100644
--- a/src/tracing/core/shared_memory_abi.cc
+++ b/src/tracing/core/shared_memory_abi.cc
@@ -30,7 +30,7 @@
if (attempt < kRetryAttempts / 2) {
std::this_thread::yield();
} else {
- usleep((attempt / 10) * 1000);
+ usleep((useconds_t(attempt) / 10) * 1000);
}
}
@@ -59,7 +59,7 @@
} // namespace
// static
-constexpr size_t SharedMemoryABI::kNumChunksForLayout[];
+constexpr uint32_t SharedMemoryABI::kNumChunksForLayout[];
constexpr const char* SharedMemoryABI::kChunkStateStr[];
constexpr const size_t SharedMemoryABI::kInvalidPageIdx;
constexpr const size_t SharedMemoryABI::kMaxPageSize;
@@ -115,11 +115,11 @@
static_assert(sizeof(ChunkHeader::writer_id) == sizeof(WriterID),
"WriterID size");
ChunkHeader chunk_header{};
- chunk_header.chunk_id = -1;
- PERFETTO_CHECK(chunk_header.chunk_id == kMaxChunkID);
+ chunk_header.chunk_id.store(static_cast<uint32_t>(-1));
+ PERFETTO_CHECK(chunk_header.chunk_id.load() == kMaxChunkID);
- chunk_header.writer_id = -1;
- PERFETTO_CHECK(kMaxWriterID <= chunk_header.writer_id);
+ chunk_header.writer_id.store(static_cast<uint16_t>(-1));
+ PERFETTO_CHECK(kMaxWriterID <= chunk_header.writer_id.load());
PERFETTO_CHECK(page_size >= base::kPageSize);
PERFETTO_CHECK(page_size <= kMaxPageSize);
@@ -203,12 +203,12 @@
return true;
}
-size_t SharedMemoryABI::GetFreeChunks(size_t page_idx) {
+uint32_t SharedMemoryABI::GetFreeChunks(size_t page_idx) {
uint32_t layout =
page_header(page_idx)->layout.load(std::memory_order_relaxed);
- const size_t num_chunks = GetNumChunksForLayout(layout);
- size_t res = 0;
- for (size_t i = 0; i < num_chunks; i++) {
+ const uint32_t num_chunks = GetNumChunksForLayout(layout);
+ uint32_t res = 0;
+ for (uint32_t i = 0; i < num_chunks; i++) {
res |= ((layout & kChunkMask) == kChunkFree) ? (1 << i) : 0;
layout >>= kChunkShift;
}
@@ -273,7 +273,7 @@
bool SharedMemoryABI::TryAcquireAllChunksForReading(size_t page_idx) {
PageHeader* phdr = page_header(page_idx);
uint32_t layout = phdr->layout.load(std::memory_order_relaxed);
- const size_t num_chunks = GetNumChunksForLayout(layout);
+ const uint32_t num_chunks = GetNumChunksForLayout(layout);
if (num_chunks == 0)
return false;
uint32_t next_layout = layout & kLayoutMask;
@@ -338,7 +338,7 @@
// TODO(primiano): The divisions below could be avoided if we cached
// |page_shift_|.
- const uintptr_t rel_addr = chunk.begin() - start_;
+ const uintptr_t rel_addr = static_cast<uintptr_t>(chunk.begin() - start_);
const size_t page_idx = rel_addr / page_size_;
const size_t offset = rel_addr % page_size_;
PERFETTO_DCHECK(offset >= sizeof(PageHeader));
diff --git a/src/tracing/core/trace_buffer.cc b/src/tracing/core/trace_buffer.cc
index 891aed0..8ec272f 100644
--- a/src/tracing/core/trace_buffer.cc
+++ b/src/tracing/core/trace_buffer.cc
@@ -244,7 +244,7 @@
PERFETTO_CHECK(next_chunk_ptr <= end());
}
PERFETTO_DCHECK(next_chunk_ptr >= search_end && next_chunk_ptr <= end());
- return next_chunk_ptr - search_end;
+ return static_cast<size_t>(next_chunk_ptr - search_end);
}
void TraceBuffer::AddPaddingRecord(size_t size) {
@@ -387,7 +387,10 @@
// - return the first patched+complete packet in the next sequence, if any.
// - return false if none of the above is found.
TRACE_BUFFER_DLOG("ReadNextTracePacket()");
- *producer_uid = -1; // Just in case we forget to initialize it below.
+
+ // Just in case we forget to initialize it below.
+ *producer_uid = kInvalidUid;
+
#if PERFETTO_DCHECK_IS_ON()
PERFETTO_DCHECK(!changed_since_last_read_);
#endif
diff --git a/src/tracing/core/trace_buffer.h b/src/tracing/core/trace_buffer.h
index a66a4db..1998be5 100644
--- a/src/tracing/core/trace_buffer.h
+++ b/src/tracing/core/trace_buffer.h
@@ -491,7 +491,7 @@
uint8_t* begin() const { return reinterpret_cast<uint8_t*>(data_.get()); }
uint8_t* end() const { return begin() + size_; }
- size_t size_to_end() const { return end() - wptr_; }
+ size_t size_to_end() const { return static_cast<size_t>(end() - wptr_); }
base::PageAllocator::UniquePtr data_;
size_t size_ = 0; // Size in bytes of |data_|.
diff --git a/src/tracing/core/trace_buffer_unittest.cc b/src/tracing/core/trace_buffer_unittest.cc
index 36e0ae6..b1ae927 100644
--- a/src/tracing/core/trace_buffer_unittest.cc
+++ b/src/tracing/core/trace_buffer_unittest.cc
@@ -163,9 +163,9 @@
}
TEST_F(TraceBufferTest, ReadWrite_OneChunkPerWriter) {
- for (uint8_t num_writers = 1; num_writers <= 10; num_writers++) {
+ for (int8_t num_writers = 1; num_writers <= 10; num_writers++) {
ResetBuffer(4096);
- for (uint8_t i = 1; i <= num_writers; i++) {
+ for (char i = 1; i <= num_writers; i++) {
ASSERT_EQ(32u, CreateChunk(ProducerID(i), WriterID(i), ChunkID(i))
.AddPacket(32 - 16, i)
.CopyIntoTraceBuffer());
@@ -173,7 +173,7 @@
// The expected read sequence now is: c3, c4, c5.
trace_buffer()->BeginRead();
- for (uint8_t i = 1; i <= num_writers; i++)
+ for (char i = 1; i <= num_writers; i++)
ASSERT_THAT(ReadPacket(), ElementsAre(FakePacketFragment(32 - 16, i)));
ASSERT_THAT(ReadPacket(), IsEmpty());
} // for(num_writers)
@@ -297,7 +297,7 @@
}
TEST_F(TraceBufferTest, ReadWrite_RandomChunksNoWrapping) {
- for (int seed = 1; seed <= 32; seed++) {
+ for (unsigned int seed = 1; seed <= 32; seed++) {
std::minstd_rand0 rnd_engine(seed);
ResetBuffer(4096 * (1 + rnd_engine() % 32));
std::uniform_int_distribution<size_t> size_dist(18, 4096);
@@ -399,7 +399,7 @@
ResetBuffer(4096);
// [c0: 512][c1: 512][c2: 512][c3: 512][c4: 512][c5: 512][c6: 512][c7: 512]
- for (uint8_t i = 0; i < 8; i++) {
+ for (char i = 0; i < 8; i++) {
ASSERT_EQ(512u, CreateChunk(ProducerID(1), WriterID(1), ChunkID(i))
.AddPacket(512 - 16, 'a' + i)
.CopyIntoTraceBuffer());
@@ -596,7 +596,7 @@
ResetBuffer(4096);
std::vector<FakePacketFragment> expected_fragments;
- for (ChunkID chunk_id = -2; chunk_id <= 2; chunk_id++) {
+ for (ChunkID chunk_id = static_cast<ChunkID>(-2); chunk_id <= 2; chunk_id++) {
char prefix = static_cast<char>('c' + chunk_id);
expected_fragments.emplace_back(10 + chunk_id, prefix);
CreateChunk(ProducerID(1), WriterID(1), chunk_id)
@@ -626,7 +626,7 @@
.SetUID(11)
.CopyIntoTraceBuffer();
trace_buffer()->BeginRead();
- uid_t uid = -1;
+ uid_t uid = kInvalidUid;
ASSERT_THAT(ReadPacket(&uid), ElementsAre(FakePacketFragment(10, 'a')));
ASSERT_EQ(11u, uid);
@@ -1071,7 +1071,9 @@
TEST_F(TraceBufferTest, Iterator_ManyStreamsWrapping) {
ResetBuffer(64 * 1024);
- auto Neg = [](int x) { return kMaxChunkID + x; };
+ auto Neg = [](int x) -> ChunkID {
+ return kMaxChunkID + static_cast<ChunkID>(x);
+ };
AppendChunks({
{ProducerID(1), WriterID(1), ChunkID(Neg(-4))},
{ProducerID(1), WriterID(1), ChunkID(Neg(-3))},
diff --git a/src/tracing/core/trace_packet_unittest.cc b/src/tracing/core/trace_packet_unittest.cc
index e8ffd58..8aab213 100644
--- a/src/tracing/core/trace_packet_unittest.cc
+++ b/src/tracing/core/trace_packet_unittest.cc
@@ -132,7 +132,8 @@
ASSERT_EQ(1u, tp.slices().size());
memcpy(&buf[preamble_size], tp.slices()[0].start, tp.slices()[0].size);
protos::Trace trace;
- ASSERT_TRUE(trace.ParseFromArray(buf, preamble_size + tp.size()));
+ ASSERT_TRUE(
+ trace.ParseFromArray(buf, static_cast<int>(preamble_size + tp.size())));
ASSERT_EQ(1, trace.packet_size());
ASSERT_EQ(payload, trace.packet(0).for_testing().str());
}
diff --git a/src/tracing/core/virtual_destructors.cc b/src/tracing/core/virtual_destructors.cc
new file mode 100644
index 0000000..24c7c7c
--- /dev/null
+++ b/src/tracing/core/virtual_destructors.cc
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2018 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/tracing/core/consumer.h"
+#include "perfetto/tracing/core/producer.h"
+#include "perfetto/tracing/core/service.h"
+#include "perfetto/tracing/core/shared_memory.h"
+#include "perfetto/tracing/core/shared_memory_arbiter.h"
+
+// This translation unit contains the definitions for the destructor of pure
+// virtual interfaces for the current build target. The alternative would be
+// introducing a one-liner .cc file for each pure virtual interface, which is
+// overkill. This is for compliance with -Wweak-vtables.
+
+namespace perfetto {
+
+Consumer::~Consumer() = default;
+Producer::~Producer() = default;
+Service::~Service() = default;
+Service::ConsumerEndpoint::~ConsumerEndpoint() = default;
+Service::ProducerEndpoint::~ProducerEndpoint() = default;
+SharedMemory::~SharedMemory() = default;
+SharedMemory::Factory::~Factory() = default;
+SharedMemoryArbiter::~SharedMemoryArbiter() = default;
+
+} // namespace perfetto