CHECK that Deregister is called before destruction
We were getting use after frees because LogReader::Deregister wasn't
being called. Add a CHECK to catch this situation and enforce it.
Change-Id: Ic5009ac9a6710aa2b33d7f400a9a6b2b900b4098
diff --git a/aos/events/logging/logger.cc b/aos/events/logging/logger.cc
index b3472d7..e767427 100644
--- a/aos/events/logging/logger.cc
+++ b/aos/events/logging/logger.cc
@@ -328,10 +328,21 @@
}
LogReader::~LogReader() {
- Deregister();
+ if (event_loop_factory_unique_ptr_) {
+ Deregister();
+ } else if (event_loop_factory_ != nullptr) {
+ LOG(FATAL) << "Must call Deregister before the SimulatedEventLoopFactory "
+ "is destroyed";
+ }
if (offset_fp_ != nullptr) {
fclose(offset_fp_);
}
+ // Zero out some buffers. It's easy to do use-after-frees on these, so make it
+ // more obvious.
+ if (remapped_configuration_buffer_) {
+ remapped_configuration_buffer_->Wipe();
+ }
+ log_file_header_.Wipe();
}
const Configuration *LogReader::logged_configuration() const {
diff --git a/aos/flatbuffers.h b/aos/flatbuffers.h
index 0a06e00..4aa0ff8 100644
--- a/aos/flatbuffers.h
+++ b/aos/flatbuffers.h
@@ -115,6 +115,10 @@
absl::Span<const uint8_t> span() const {
return absl::Span<const uint8_t>(data(), size());
}
+
+ // Wipes out the data buffer. This is handy to mark an instance as freed, and
+ // make attempts to use it fail more obviously.
+ void Wipe() { memset(data(), 0, size()); }
};
// String backed flatbuffer.