Actually enforce the size in ChannelPreallocatedAllocator
We were ignoring the size when being asked to allocate. Turns out
Flatbuffers rounds alignment up, so a 300 byte request becomes 304
bytes, corrupting the redzone. Add a bunch of CHECKs here to catch it,
teach EventLoop to catch it, and catch it in the compiler.
Change-Id: I10488a2f96eeb7a955c6da436e6f9de1fcebbd14
diff --git a/aos/config_flattener.cc b/aos/config_flattener.cc
index d39cd50..83959a1 100644
--- a/aos/config_flattener.cc
+++ b/aos/config_flattener.cc
@@ -25,6 +25,15 @@
FlatbufferDetachedBuffer<Configuration> config =
configuration::ReadConfig(config_path, {bazel_outs_directory});
+ for (const Channel *channel : *config.message().channels()) {
+ if (channel->max_size() % alignof(flatbuffers::largest_scalar_t) != 0) {
+ LOG(FATAL) << "max_size() (" << channel->max_size()
+ << ") is not a multiple of alignment ("
+ << alignof(flatbuffers::largest_scalar_t) << ") for channel "
+ << configuration::CleanedChannelToString(channel) << ".";
+ }
+ }
+
std::vector<aos::FlatbufferString<reflection::Schema>> schemas;
for (int i = 5; i < argc; ++i) {
diff --git a/aos/events/channel_preallocated_allocator.h b/aos/events/channel_preallocated_allocator.h
index 5ca370b..c4f0eca 100644
--- a/aos/events/channel_preallocated_allocator.h
+++ b/aos/events/channel_preallocated_allocator.h
@@ -34,17 +34,30 @@
~ChannelPreallocatedAllocator() override { CHECK(!is_allocated_); }
// TODO(austin): Read the contract for these.
- uint8_t *allocate(size_t /*size*/) override {
+ uint8_t *allocate(size_t size) override {
if (is_allocated_) {
- LOG(FATAL) << "Can't allocate more memory with a fixed size allocator. "
- "Increase the memory reserved.";
+ LOG(FATAL) << "Can't allocate more memory with a fixed size allocator on "
+ "channel "
+ << configuration::CleanedChannelToString(channel_);
}
+ CHECK_LE(size, size_)
+ << ": Tried to allocate more space than available on channel "
+ << configuration::CleanedChannelToString(channel_);
+
is_allocated_ = true;
return data_;
}
- void deallocate(uint8_t *, size_t) override { is_allocated_ = false; }
+ void deallocate(uint8_t *data, size_t size) override {
+ CHECK_EQ(data, data_)
+ << ": Deallocating data not allocated here on channel "
+ << configuration::CleanedChannelToString(channel_);
+ CHECK_LE(size, size_)
+ << ": Tried to deallocate more space than available on channel "
+ << configuration::CleanedChannelToString(channel_);
+ is_allocated_ = false;
+ }
uint8_t *reallocate_downward(uint8_t * /*old_p*/, size_t /*old_size*/,
size_t new_size, size_t /*in_use_back*/,
diff --git a/aos/events/event_loop.cc b/aos/events/event_loop.cc
index 5c7e49d..639e337 100644
--- a/aos/events/event_loop.cc
+++ b/aos/events/event_loop.cc
@@ -10,6 +10,16 @@
"Period in milliseconds to publish timing reports at.");
namespace aos {
+namespace {
+void CheckAlignment(const Channel *channel) {
+ if (channel->max_size() % alignof(flatbuffers::largest_scalar_t) != 0) {
+ LOG(FATAL) << "max_size() (" << channel->max_size()
+ << ") is not a multiple of alignment ("
+ << alignof(flatbuffers::largest_scalar_t) << ") for channel "
+ << configuration::CleanedChannelToString(channel) << ".";
+ }
+}
+} // namespace
RawSender::RawSender(EventLoop *event_loop, const Channel *channel)
: event_loop_(event_loop),
@@ -120,6 +130,8 @@
}
void EventLoop::NewFetcher(RawFetcher *fetcher) {
+ CheckAlignment(fetcher->channel());
+
fetchers_.emplace_back(fetcher);
UpdateTimingReport();
}
@@ -144,6 +156,8 @@
CHECK(!is_running()) << ": Cannot add new objects while running.";
ChannelIndex(channel);
+ CheckAlignment(channel);
+
CHECK(taken_senders_.find(channel) == taken_senders_.end())
<< ": " << configuration::CleanedChannelToString(channel)
<< " is already being used.";
@@ -163,6 +177,8 @@
CHECK(!is_running()) << ": Cannot add new objects while running.";
ChannelIndex(channel);
+ CheckAlignment(channel);
+
CHECK(taken_watchers_.find(channel) == taken_watchers_.end())
<< ": Channel " << configuration::CleanedChannelToString(channel)
<< " is already being used.";
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index 993011f..979de74 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -1158,6 +1158,35 @@
"Channel pointer not found in configuration\\(\\)->channels\\(\\)");
}
+// Verifies that the event loop implementations detect when Channel has an
+// invalid alignment.
+TEST_P(AbstractEventLoopDeathTest, InvalidChannelAlignment) {
+ const char *const kError = "multiple of alignment";
+ InvalidChannelAlignment();
+
+ auto loop = MakePrimary();
+
+ const Channel *channel = configuration::GetChannel(
+ loop->configuration(), "/test", "aos.TestMessage", "", nullptr);
+
+ EXPECT_DEATH({ loop->MakeRawSender(channel); }, kError);
+ EXPECT_DEATH({ loop->MakeSender<TestMessage>("/test"); }, kError);
+
+ EXPECT_DEATH({ loop->MakeRawFetcher(channel); }, kError);
+ EXPECT_DEATH({ loop->MakeFetcher<TestMessage>("/test"); }, kError);
+
+ EXPECT_DEATH(
+ { loop->MakeRawWatcher(channel, [](const Context &, const void *) {}); },
+ kError);
+ EXPECT_DEATH({ loop->MakeRawNoArgWatcher(channel, [](const Context &) {}); },
+ kError);
+
+ EXPECT_DEATH({ loop->MakeNoArgWatcher<TestMessage>("/test", []() {}); },
+ kError);
+ EXPECT_DEATH({ loop->MakeWatcher("/test", [](const TestMessage &) {}); },
+ kError);
+}
+
// Verify that the send time on a message is roughly right when using a watcher.
TEST_P(AbstractEventLoopTest, MessageSendTime) {
auto loop1 = MakePrimary();
diff --git a/aos/events/event_loop_param_test.h b/aos/events/event_loop_param_test.h
index 57d0525..85e802b 100644
--- a/aos/events/event_loop_param_test.h
+++ b/aos/events/event_loop_param_test.h
@@ -59,6 +59,35 @@
// Advances time by sleeping. Can't be called from inside a loop.
virtual void SleepFor(::std::chrono::nanoseconds duration) = 0;
+ // Sets the config to a config with a max size with an invalid alignment.
+ void InvalidChannelAlignment() {
+ flatbuffer_ = JsonToFlatbuffer<Configuration>(R"config({
+ "channels": [
+ {
+ "name": "/aos",
+ "type": "aos.logging.LogMessageFbs"
+ },
+ {
+ "name": "/aos",
+ "type": "aos.timing.Report"
+ },
+ {
+ "name": "/test",
+ "type": "aos.TestMessage",
+ "max_size": 13
+ },
+ {
+ "name": "/test1",
+ "type": "aos.TestMessage"
+ },
+ {
+ "name": "/test2",
+ "type": "aos.TestMessage"
+ }
+ ]
+})config");
+ }
+
void PinReads() {
static const std::string kJson = R"config({
"channels": [
@@ -245,6 +274,8 @@
return factory_->MakePrimary(name);
}
+ void InvalidChannelAlignment() { factory_->InvalidChannelAlignment(); }
+
void EnableNodes(std::string_view my_node) { factory_->EnableNodes(my_node); }
void Run() { return factory_->Run(); }
diff --git a/aos/network/www/test_config_file.json b/aos/network/www/test_config_file.json
index 09b22dd..39b8ef1 100644
--- a/aos/network/www/test_config_file.json
+++ b/aos/network/www/test_config_file.json
@@ -10,7 +10,7 @@
"name": "/test",
"type": "aos.Configuration",
"source_node": "roborio",
- "max_size": 1678,
+ "max_size": 2056,
"frequency": 200
}
],