Validate flatbuffers before printing in aos_dump, log_cat, and simulated_event_loop
We had a corrupted flatbuffer being logged, and the error was very poor
when it was being printed. Let's try to catch this a lot earlier.
Fix 1: log_cat should validate before printing. That'll prevent us
from wasting time debugging why it is crashing and point the finger a
lot better.
Fix 2: aos_dump should do the same. That'll save us from nasty crashes
there too and focus attention a lot better.
Fix 3: SimulatedEventLoop sender Send should also validate in debug
mode. This will avoid too big a performance hit in the fast path, but
will catch corrupted flatbuffers in any simulations we do, or any log
replay that gets done with debug turned on.
This caught a couple of places where we were missing schemas, so add
those in too.
Change-Id: I1873ddd592d33fe4e64210a2e08aa9b937d61ab8
Signed-off-by: Austin Schuh <austin.schuh@bluerivertech.com>
diff --git a/aos/aos_dump.cc b/aos/aos_dump.cc
index 45168b2..e2467b3 100644
--- a/aos/aos_dump.cc
+++ b/aos/aos_dump.cc
@@ -31,6 +31,14 @@
// information on stderr.
builder->Reset();
+
+ CHECK(flatbuffers::Verify(*channel->schema(),
+ *channel->schema()->root_table(),
+ static_cast<const uint8_t *>(context.data),
+ static_cast<size_t>(context.size)))
+ << ": Corrupted flatbuffer on " << channel->name()->c_str() << " "
+ << channel->type()->c_str();
+
aos::FlatbufferToJson(
builder, channel->schema(), static_cast<const uint8_t *>(context.data),
{FLAGS_pretty, static_cast<size_t>(FLAGS_max_vector_size)});
diff --git a/aos/configuration.cc b/aos/configuration.cc
index b388bd3..faafcfc 100644
--- a/aos/configuration.cc
+++ b/aos/configuration.cc
@@ -915,6 +915,14 @@
}
} // namespace
+aos::FlatbufferDetachedBuffer<aos::Configuration> AddSchema(
+ std::string_view json,
+ const std::vector<aos::FlatbufferVector<reflection::Schema>> &schemas) {
+ FlatbufferDetachedBuffer<Configuration> addition =
+ JsonToFlatbuffer(json, Configuration::MiniReflectTypeTable());
+ return MergeConfiguration(addition, schemas);
+}
+
int GetNodeIndex(const Configuration *config, const Node *node) {
if (!MultiNode(config)) {
return 0;
diff --git a/aos/configuration.h b/aos/configuration.h
index c6d25df..c22a1cf 100644
--- a/aos/configuration.h
+++ b/aos/configuration.h
@@ -40,6 +40,12 @@
FlatbufferDetachedBuffer<Configuration> MergeWithConfig(
const Configuration *config, const Flatbuffer<Configuration> &addition);
+// Adds the list of schemas to the provide config json. This should mostly be
+// used for testing and in conjunction with MergeWithConfig.
+FlatbufferDetachedBuffer<aos::Configuration> AddSchema(
+ std::string_view json,
+ const std::vector<FlatbufferVector<reflection::Schema>> &schemas);
+
// Returns the resolved Channel for a name, type, and application name. Returns
// nullptr if none is found.
//
diff --git a/aos/controls/control_loop_test.h b/aos/controls/control_loop_test.h
index 7fbc94a..2a892fa 100644
--- a/aos/controls/control_loop_test.h
+++ b/aos/controls/control_loop_test.h
@@ -24,16 +24,6 @@
template <typename TestBaseClass>
class ControlLoopTestTemplated : public TestBaseClass {
public:
- // Builds a control loop test with the provided configuration. Note: this
- // merges and sorts the config to reduce user errors.
- ControlLoopTestTemplated(std::string_view configuration,
- ::std::chrono::nanoseconds dt = kTimeTick)
- : ControlLoopTestTemplated(
- configuration::MergeConfiguration(
- aos::FlatbufferDetachedBuffer<Configuration>(JsonToFlatbuffer(
- configuration, Configuration::MiniReflectTypeTable()))),
- dt) {}
-
ControlLoopTestTemplated(
FlatbufferDetachedBuffer<Configuration> configuration,
::std::chrono::nanoseconds dt = kTimeTick)
diff --git a/aos/events/BUILD b/aos/events/BUILD
index aab764b..4194514 100644
--- a/aos/events/BUILD
+++ b/aos/events/BUILD
@@ -1,4 +1,5 @@
load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library", "flatbuffer_ts_library")
+load("//aos:flatbuffers.bzl", "cc_static_flatbuffer")
load("//aos:config.bzl", "aos_config")
package(default_visibility = ["//visibility:public"])
@@ -10,6 +11,12 @@
target_compatible_with = ["@platforms//os:linux"],
)
+cc_static_flatbuffer(
+ name = "test_message_schema",
+ function = "aos::TestMessageSchema",
+ target = ":test_message_fbs_reflection_out",
+)
+
flatbuffer_cc_library(
name = "event_loop_fbs",
srcs = ["event_loop.fbs"],
@@ -20,6 +27,12 @@
target_compatible_with = ["@platforms//os:linux"],
)
+cc_static_flatbuffer(
+ name = "timing_report_schema",
+ function = "aos::timing::ReportSchema",
+ target = ":event_loop_fbs_reflection_out",
+)
+
flatbuffer_cc_library(
name = "ping_fbs",
srcs = ["ping.fbs"],
@@ -293,7 +306,13 @@
deps = [
":event_loop",
":test_message_fbs",
+ ":test_message_schema",
+ ":timing_report_schema",
"//aos:realtime",
+ "//aos/logging:log_message_schema",
+ "//aos/network:message_bridge_client_schema",
+ "//aos/network:message_bridge_server_schema",
+ "//aos/network:timestamp_schema",
"//aos/testing:googletest",
],
)
diff --git a/aos/events/event_loop_param_test.cc b/aos/events/event_loop_param_test.cc
index ee77b8a..0fd1078 100644
--- a/aos/events/event_loop_param_test.cc
+++ b/aos/events/event_loop_param_test.cc
@@ -1801,7 +1801,8 @@
auto loop2 = MakePrimary();
auto loop3 = Make();
- const std::string kData("971 is the best");
+ const FlatbufferDetachedBuffer<TestMessage> kMessage =
+ JsonToFlatbuffer<TestMessage>("{}");
std::unique_ptr<aos::RawSender> sender =
loop1->MakeRawSender(configuration::GetChannel(
@@ -1811,29 +1812,29 @@
loop3->MakeRawFetcher(configuration::GetChannel(
loop3->configuration(), "/test", "aos.TestMessage", "", nullptr));
- loop2->OnRun(
- [&]() { EXPECT_TRUE(sender->Send(kData.data(), kData.size())); });
+ loop2->OnRun([&]() {
+ EXPECT_TRUE(sender->Send(kMessage.span().data(), kMessage.span().size()));
+ });
bool happened = false;
loop2->MakeRawWatcher(
configuration::GetChannel(loop2->configuration(), "/test",
"aos.TestMessage", "", nullptr),
- [this, &kData, &fetcher, &happened](const Context &context,
- const void *message) {
+ [this, &kMessage, &fetcher, &happened](const Context &context,
+ const void *message) {
happened = true;
- EXPECT_EQ(std::string_view(kData),
- std::string_view(reinterpret_cast<const char *>(message),
- context.size));
- EXPECT_EQ(std::string_view(kData),
- std::string_view(reinterpret_cast<const char *>(context.data),
- context.size));
+ EXPECT_EQ(
+ kMessage.span(),
+ absl::Span<const uint8_t>(
+ reinterpret_cast<const uint8_t *>(message), context.size));
+ EXPECT_EQ(message, context.data);
ASSERT_TRUE(fetcher->Fetch());
- EXPECT_EQ(std::string_view(kData),
- std::string_view(
- reinterpret_cast<const char *>(fetcher->context().data),
- fetcher->context().size));
+ EXPECT_EQ(kMessage.span(),
+ absl::Span<const uint8_t>(reinterpret_cast<const uint8_t *>(
+ fetcher->context().data),
+ fetcher->context().size));
this->Exit();
});
@@ -1850,7 +1851,8 @@
auto loop2 = MakePrimary();
auto loop3 = Make();
- const std::string kData("971 is the best");
+ const FlatbufferDetachedBuffer<TestMessage> kMessage =
+ JsonToFlatbuffer<TestMessage>("{}");
const aos::monotonic_clock::time_point monotonic_remote_time =
aos::monotonic_clock::time_point(chrono::seconds(1501));
@@ -1868,9 +1870,9 @@
loop3->configuration(), "/test", "aos.TestMessage", "", nullptr));
loop2->OnRun([&]() {
- EXPECT_TRUE(sender->Send(kData.data(), kData.size(), monotonic_remote_time,
- realtime_remote_time, remote_queue_index,
- remote_boot_uuid));
+ EXPECT_TRUE(sender->Send(kMessage.span().data(), kMessage.span().size(),
+ monotonic_remote_time, realtime_remote_time,
+ remote_queue_index, remote_boot_uuid));
});
bool happened = false;
@@ -1904,7 +1906,8 @@
TEST_P(AbstractEventLoopTest, RawSenderSentData) {
auto loop1 = MakePrimary();
- const std::string kData("971 is the best");
+ const FlatbufferDetachedBuffer<TestMessage> kMessage =
+ JsonToFlatbuffer<TestMessage>("{}");
std::unique_ptr<aos::RawSender> sender =
loop1->MakeRawSender(configuration::GetChannel(
@@ -1913,7 +1916,7 @@
const aos::monotonic_clock::time_point monotonic_now = loop1->monotonic_now();
const aos::realtime_clock::time_point realtime_now = loop1->realtime_now();
- EXPECT_TRUE(sender->Send(kData.data(), kData.size()));
+ EXPECT_TRUE(sender->Send(kMessage.span().data(), kMessage.span().size()));
EXPECT_GE(sender->monotonic_sent_time(), monotonic_now);
EXPECT_LE(sender->monotonic_sent_time(),
@@ -1923,7 +1926,7 @@
realtime_now + chrono::milliseconds(100));
EXPECT_EQ(sender->sent_queue_index(), 0u);
- EXPECT_TRUE(sender->Send(kData.data(), kData.size()));
+ EXPECT_TRUE(sender->Send(kMessage.span().data(), kMessage.span().size()));
EXPECT_GE(sender->monotonic_sent_time(), monotonic_now);
EXPECT_LE(sender->monotonic_sent_time(),
diff --git a/aos/events/event_loop_param_test.h b/aos/events/event_loop_param_test.h
index 0cbb5fe..3138c51 100644
--- a/aos/events/event_loop_param_test.h
+++ b/aos/events/event_loop_param_test.h
@@ -7,8 +7,14 @@
#include "aos/events/event_loop.h"
#include "aos/events/test_message_generated.h"
+#include "aos/events/test_message_schema.h"
+#include "aos/events/timing_report_schema.h"
#include "aos/flatbuffers.h"
#include "aos/json_to_flatbuffer.h"
+#include "aos/logging/log_message_schema.h"
+#include "aos/network/message_bridge_client_schema.h"
+#include "aos/network/message_bridge_server_schema.h"
+#include "aos/network/timestamp_schema.h"
#include "gtest/gtest.h"
namespace aos {
@@ -17,7 +23,8 @@
class EventLoopTestFactory {
public:
EventLoopTestFactory()
- : flatbuffer_(JsonToFlatbuffer<Configuration>(R"config({
+ : flatbuffer_(configuration::AddSchema(
+ R"config({
"channels": [
{
"name": "/aos",
@@ -40,7 +47,11 @@
"type": "aos.TestMessage"
}
]
-})config")) {}
+})config",
+ {aos::FlatbufferSpan<reflection::Schema>(
+ logging::LogMessageFbsSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(timing::ReportSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(TestMessageSchema())})) {}
virtual ~EventLoopTestFactory() {}
@@ -61,7 +72,8 @@
// Sets the config to a config with a max size with an invalid alignment.
void InvalidChannelAlignment() {
- flatbuffer_ = JsonToFlatbuffer<Configuration>(R"config({
+ flatbuffer_ = configuration::AddSchema(
+ R"config({
"channels": [
{
"name": "/aos",
@@ -85,7 +97,11 @@
"type": "aos.TestMessage"
}
]
-})config");
+})config",
+ {aos::FlatbufferSpan<reflection::Schema>(
+ logging::LogMessageFbsSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(timing::ReportSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(TestMessageSchema())});
}
void PinReads() {
@@ -124,8 +140,11 @@
]
})config";
- flatbuffer_ = FlatbufferDetachedBuffer<Configuration>(
- JsonToFlatbuffer(kJson, Configuration::MiniReflectTypeTable()));
+ flatbuffer_ = configuration::AddSchema(
+ kJson, {aos::FlatbufferSpan<reflection::Schema>(
+ logging::LogMessageFbsSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(timing::ReportSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(TestMessageSchema())});
}
void EnableNodes(std::string_view my_node) {
@@ -239,8 +258,19 @@
})config";
flatbuffer_ = configuration::MergeConfiguration(
- FlatbufferDetachedBuffer<Configuration>(
- JsonToFlatbuffer(kJson, Configuration::MiniReflectTypeTable())));
+ configuration::MergeConfiguration(
+ aos::FlatbufferDetachedBuffer<Configuration>(
+ JsonToFlatbuffer<Configuration>(kJson))),
+ {aos::FlatbufferSpan<reflection::Schema>(
+ logging::LogMessageFbsSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(timing::ReportSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(TestMessageSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(
+ message_bridge::ClientStatisticsSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(
+ message_bridge::ServerStatisticsSchema()),
+ aos::FlatbufferSpan<reflection::Schema>(
+ message_bridge::TimestampSchema())});
my_node_ = configuration::GetNode(&flatbuffer_.message(), my_node);
}
diff --git a/aos/events/logging/log_cat.cc b/aos/events/logging/log_cat.cc
index 2032f9f..3469674 100644
--- a/aos/events/logging/log_cat.cc
+++ b/aos/events/logging/log_cat.cc
@@ -44,6 +44,13 @@
const aos::Context &context,
aos::FastStringBuilder *builder) {
builder->Reset();
+ CHECK(flatbuffers::Verify(*channel->schema(),
+ *channel->schema()->root_table(),
+ static_cast<const uint8_t *>(context.data),
+ static_cast<size_t>(context.size)))
+ << ": Corrupted flatbuffer on " << channel->name()->c_str() << " "
+ << channel->type()->c_str();
+
aos::FlatbufferToJson(
builder, channel->schema(), static_cast<const uint8_t *>(context.data),
{FLAGS_pretty, static_cast<size_t>(FLAGS_max_vector_size)});
@@ -111,6 +118,17 @@
CHECK_LT(channel_index, channels->size());
const aos::Channel *const channel = channels->Get(channel_index);
+ if (message.value().message().data() != nullptr) {
+ CHECK(channel->has_schema());
+
+ CHECK(flatbuffers::Verify(*channel->schema(),
+ *channel->schema()->root_table(),
+ message.value().message().data()->data(),
+ message.value().message().data()->size()))
+ << ": Corrupted flatbuffer on " << channel->name()->c_str() << " "
+ << channel->type()->c_str();
+ }
+
if (FLAGS_format_raw && message.value().message().data() != nullptr) {
std::cout << aos::configuration::StrippedChannelToString(channel) << " "
<< aos::FlatbufferToJson(
diff --git a/aos/events/simulated_event_loop.cc b/aos/events/simulated_event_loop.cc
index ef34839..51021b6 100644
--- a/aos/events/simulated_event_loop.cc
+++ b/aos/events/simulated_event_loop.cc
@@ -806,6 +806,17 @@
message->context.queue_index = queue_index;
message->context.data = message->data(channel()->max_size()) +
channel()->max_size() - message->context.size;
+
+ DCHECK(channel()->has_schema())
+ << ": Missing schema for channel "
+ << configuration::StrippedChannelToString(channel());
+ DCHECK(flatbuffers::Verify(
+ *channel()->schema(), *channel()->schema()->root_table(),
+ static_cast<const uint8_t *>(message->context.data),
+ message->context.size))
+ << ": Corrupted flatbuffer on " << channel()->name()->c_str() << " "
+ << channel()->type()->c_str();
+
next_queue_index_ = next_queue_index_.Increment();
latest_message_ = message;
diff --git a/aos/flatbuffers.bzl b/aos/flatbuffers.bzl
index eb19beb..3db4ca0 100644
--- a/aos/flatbuffers.bzl
+++ b/aos/flatbuffers.bzl
@@ -1,4 +1,4 @@
-def cc_static_flatbuffer(name, target, function):
+def cc_static_flatbuffer(name, target, function, visibility = None):
"""Creates a cc_library which encodes a file as a Span.
args:
@@ -7,10 +7,10 @@
"""
native.genrule(
name = name + "_gen",
- tools = ["//aos:flatbuffers_static"],
+ tools = ["@org_frc971//aos:flatbuffers_static"],
srcs = [target],
outs = [name + ".h"],
- cmd = "$(location //aos:flatbuffers_static) $(SRCS) $(OUTS) '" + function + "'",
+ cmd = "$(location @org_frc971//aos:flatbuffers_static) $(SRCS) $(OUTS) '" + function + "'",
)
native.cc_library(
@@ -19,4 +19,5 @@
deps = [
"@com_google_absl//absl/types:span",
],
+ visibility = visibility,
)
diff --git a/aos/logging/BUILD b/aos/logging/BUILD
index cf0c370..30fd0d1 100644
--- a/aos/logging/BUILD
+++ b/aos/logging/BUILD
@@ -1,4 +1,5 @@
load("@com_github_google_flatbuffers//:build_defs.bzl", "flatbuffer_cc_library")
+load("//aos:flatbuffers.bzl", "cc_static_flatbuffer")
# The primary client logging interface.
cc_library(
@@ -95,3 +96,10 @@
target_compatible_with = ["@platforms//os:linux"],
visibility = ["//visibility:public"],
)
+
+cc_static_flatbuffer(
+ name = "log_message_schema",
+ function = "aos::logging::LogMessageFbsSchema",
+ target = ":log_message_fbs_reflection_out",
+ visibility = ["//visibility:public"],
+)
diff --git a/aos/network/BUILD b/aos/network/BUILD
index 4980748..e6ff5af 100644
--- a/aos/network/BUILD
+++ b/aos/network/BUILD
@@ -53,6 +53,13 @@
target_compatible_with = ["@platforms//os:linux"],
)
+cc_static_flatbuffer(
+ name = "timestamp_schema",
+ function = "aos::message_bridge::TimestampSchema",
+ target = ":timestamp_fbs_reflection_out",
+ visibility = ["//visibility:public"],
+)
+
flatbuffer_cc_library(
name = "message_bridge_client_fbs",
srcs = ["message_bridge_client.fbs"],
@@ -64,6 +71,13 @@
target_compatible_with = ["@platforms//os:linux"],
)
+cc_static_flatbuffer(
+ name = "message_bridge_client_schema",
+ function = "aos::message_bridge::ClientStatisticsSchema",
+ target = ":message_bridge_client_fbs_reflection_out",
+ visibility = ["//visibility:public"],
+)
+
flatbuffer_cc_library(
name = "message_bridge_server_fbs",
srcs = ["message_bridge_server.fbs"],
@@ -74,6 +88,13 @@
target_compatible_with = ["@platforms//os:linux"],
)
+cc_static_flatbuffer(
+ name = "message_bridge_server_schema",
+ function = "aos::message_bridge::ServerStatisticsSchema",
+ target = ":message_bridge_server_fbs_reflection_out",
+ visibility = ["//visibility:public"],
+)
+
cc_library(
name = "team_number",
srcs = [