Compare commits

..
Author SHA1 Message Date
Artyom Palvelev 5eb190f000 rust: fix 64-bit scalar wire-format alignment on 32-bit targets in Push::alignment (#9465)
In the FlatBuffers binary wire format specification, every scalar field is
aligned to its own byte size (`1`, `2`, `4`, or `8` bytes), regardless of the
host CPU architecture. Both the `flatc` schema compiler (when computing table
`max_align` and struct field offsets) and the C++ `FlatBufferBuilder::Align` /
`PushElement` implementation align 64-bit scalars (`u64`, `i64`, `f64`) to 8
bytes on all architectures.

However, in the Rust `flatbuffers` crate (`rust/flatbuffers/src/push.rs`), the
`Push` trait provides a default implementation:

```rust
fn alignment() -> PushAlignment {
    PushAlignment::new(align_of::<Self::Output>())
}
```

And `impl_push_for_endian_scalar!` did not override `Push::alignment()`,
inheriting `core::mem::align_of::<T>()` for all primitive scalar types (`bool`,
`u8`, `i8`, `u16`, `i16`, `u32`, `i32`, `u64`, `i64`, `f32`, `f64`).

On 64-bit targets (`x86_64`, `aarch64`) and 32-bit ARM (`armv7`),
`align_of::<u64>() == 8`, so `align_of::<T>()` happens to equal `size_of::<T>()`.
On 32-bit x86 (`i686`, System V i386 ABI), however, the host ABI alignment of
64-bit scalars (`u64`, `i64`, `f64`) is **4 bytes** (`align_of::<u64>() == 4`),
while their FlatBuffer wire-format size and required wire alignment is **8
bytes** (`size_of::<u64>() == 8`).

As a consequence, on 32-bit x86 (`i686`):
1. `FlatBufferBuilder::push::<u64 / i64 / f64>` only aligns the builder's cursor
   and tracks `min_align` to 4 bytes instead of 8 bytes when writing 64-bit
   table fields or vectors of 64-bit scalars.
2. When a table containing a 64-bit scalar field (`uint64`, `int64`, `double`)
   is serialized on `i686`, the field can be placed at a buffer offset that is
   4-byte aligned but not 8-byte aligned.
3. When the resulting FlatBuffer is verified by C++ `flatbuffers::Verifier`
   (which checks `VerifyField<uint64_t>` with `sizeof(uint64_t) == 8` alignment
   when `FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE` / alignment checks are active)
   or transferred to a 64-bit reader, verification fails or unaligned 64-bit
   reads occur.

Fix `impl_push_for_endian_scalar!` to explicitly implement `Push::alignment()`
using `size_of::<$ty>()` so that all endian scalar types always use their
wire-format byte size as their FlatBuffer alignment across all target
architectures.
2026-11-06 15:15:33 -08:00
10 changed files with 48 additions and 0 deletions
+1
View File
@@ -504,6 +504,7 @@ grpc::string GenerateInterface(grpc_generator::File* file,
const grpc::string& filename) {
grpc::string output;
std::set<grpc::string> generated_functions;
std::map<grpc::string, grpc::string> vars;
vars["PATH"] = file->package();
+25
View File
@@ -83,6 +83,11 @@ macro_rules! impl_push_for_endian_scalar {
unsafe fn push(&self, dst: &mut [u8], _written_len: usize) {
emplace_scalar::<$ty>(dst, *self);
}
#[inline]
fn alignment() -> PushAlignment {
PushAlignment::new(size_of::<$ty>())
}
}
};
}
@@ -98,3 +103,23 @@ impl_push_for_endian_scalar!(u64);
impl_push_for_endian_scalar!(i64);
impl_push_for_endian_scalar!(f32);
impl_push_for_endian_scalar!(f64);
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn endian_scalar_push_alignment_matches_wire_size() {
assert_eq!(<bool as Push>::alignment().value(), 1);
assert_eq!(<u8 as Push>::alignment().value(), 1);
assert_eq!(<i8 as Push>::alignment().value(), 1);
assert_eq!(<u16 as Push>::alignment().value(), 2);
assert_eq!(<i16 as Push>::alignment().value(), 2);
assert_eq!(<u32 as Push>::alignment().value(), 4);
assert_eq!(<i32 as Push>::alignment().value(), 4);
assert_eq!(<f32 as Push>::alignment().value(), 4);
assert_eq!(<u64 as Push>::alignment().value(), 8);
assert_eq!(<i64 as Push>::alignment().value(), 8);
assert_eq!(<f64 as Push>::alignment().value(), 8);
}
}
+2
View File
@@ -722,6 +722,8 @@ void BinaryAnnotator::BuildTable(const uint64_t table_offset,
BinaryRegionComment offset_field_comment;
offset_field_comment.type = BinaryRegionCommentType::TableOffsetField;
offset_field_comment.name = field->name()->c_str();
const std::string offset_prefix =
"offset to field `" + std::string(field->name()->c_str()) + "`";
// Validate any field that isn't inline (i.e., non-structs).
if (!IsInlineField(field)) {
+2
View File
@@ -2737,6 +2737,7 @@ class CppGenerator : public BaseGenerator {
if (!field.IsScalarOptional()) {
const bool is_scalar = IsScalar(type.base_type);
std::string accessor;
std::string offset_size = "";
if (is_scalar) {
accessor = "GetField<";
} else if (IsStruct(type)) {
@@ -2814,6 +2815,7 @@ class CppGenerator : public BaseGenerator {
void GenTableFieldType(const FieldDef& field) {
const auto& type = field.value.type;
const auto offset_str = GenFieldOffsetName(field);
if (!field.IsScalarOptional()) {
std::string afterptr = " *" + NullableExtension();
code_.SetValue("FIELD_TYPE",
+2
View File
@@ -1149,6 +1149,8 @@ static bool GenerateDart(const Parser& parser, const std::string& path,
static std::string DartMakeRule(const Parser& parser, const std::string& path,
const std::string& file_name) {
auto filebase =
flatbuffers::StripPath(flatbuffers::StripExtension(file_name));
dart::DartGenerator generator(parser, path, file_name);
auto make_rule = generator.Filename("") + ": ";
+2
View File
@@ -966,6 +966,8 @@ class KotlinGenerator : public BaseGenerator {
auto bbgetter = ByteBufferGetter(field.value.type, "bb");
auto ucast = CastToUsigned(field);
auto offset_val = NumToString(field.value.offset);
auto offset_prefix =
"val o = __offset(" + offset_val + "); return o != 0 ? ";
auto value_base_type = field.value.type.base_type;
// Most field accessors need to retrieve and test the field offset
// first, this is the offset value for that:
+5
View File
@@ -146,6 +146,7 @@ class KotlinKMPGenerator : public BaseGenerator {
std::string code =
"// " + std::string(FlatBuffersGeneratedWarning()) + "\n\n";
auto qualified = ns.GetFullyQualifiedName("");
std::string namespace_name = namer_.Namespace(ns);
if (!namespace_name.empty()) {
code += "package " + namespace_name;
@@ -340,6 +341,7 @@ class KotlinKMPGenerator : public BaseGenerator {
std::string GenDefaultBufferValue(const FieldDef& field) const {
auto& value = field.value;
auto base_type = value.type.base_type;
auto field_name = field.name;
std::string suffix = IsScalar(base_type) ? LiteralSuffix(value.type) : "";
if (field.IsScalarOptional()) {
return "null";
@@ -371,6 +373,7 @@ class KotlinKMPGenerator : public BaseGenerator {
std::string GenDefaultValue(const FieldDef& field) const {
auto& value = field.value;
auto base_type = value.type.base_type;
auto field_name = field.name;
std::string suffix = LiteralSuffix(value.type);
if (field.IsScalarOptional()) {
return "null";
@@ -1079,6 +1082,8 @@ class KotlinKMPGenerator : public BaseGenerator {
auto return_type = GetterReturnType(field);
auto bbgetter = ByteBufferGetter(field.value.type, "bb");
auto offset_val = NumToString(field.value.offset);
auto offset_prefix =
"val o = offset(" + offset_val + "); return o != 0 ? ";
auto value_base_type = field.value.type.base_type;
// Most field accessors need to retrieve and test the field offset
// first, this is the offset value for that:
+2
View File
@@ -2010,6 +2010,8 @@ class PythonGenerator : public BaseGenerator {
void InitializeFromPackedBuf(const StructDef& struct_def,
std::string* code_ptr) const {
auto& code = *code_ptr;
const auto struct_var = namer_.Variable(struct_def);
const auto struct_type = namer_.Type(struct_def);
code += GenIndents(1) + "@classmethod";
code += GenIndents(1) + "def InitFromPackedBuf(cls, buf, pos=0):";
+2
View File
@@ -787,6 +787,7 @@ class SwiftGenerator : public BaseGenerator {
}
void GenTableReaderFields(const FieldDef& field) {
const auto offset = NumToString(field.value.offset);
const auto field_field = namer_.Field(field);
const auto type = GenType(field.value.type);
code_.SetValue("FIELDVAR", namer_.Variable(field));
@@ -1381,6 +1382,7 @@ class SwiftGenerator : public BaseGenerator {
const auto& field = **it;
if (field.deprecated) continue;
const auto type = GenType(field.value.type);
code_.SetValue("FIELDVAR", namer_.Variable(field));
if (IsArray(field.value.type)) {
code_.SetValue(
+5
View File
@@ -196,6 +196,7 @@ class TsGenerator : public BaseGenerator {
SkipDir::OutputPathAndTrailingPathSeparator);
symbolic_name = definition.defined_namespace->components.back();
} else {
auto def_mod_name = namer_.File(definition, SkipFile::SuffixAndExtension);
symbolic_name = file_name_;
filepath = path_ + symbolic_name + ".ts";
}
@@ -285,10 +286,12 @@ class TsGenerator : public BaseGenerator {
auto base_name =
namer_.Directories(it.second.ns->components, SkipDir::OutputPath) +
base_file_name;
auto ts_file_path = base_name + ".ts";
auto base_name_rel = std::string("./");
base_name_rel +=
namer_.Directories(rel_components, SkipDir::OutputPath);
base_name_rel += base_file_name;
auto ts_file_path_rel = base_name_rel + ".ts";
auto type_name = def.first;
auto fully_qualified_type_name =
it.second.ns->GetFullyQualifiedName(type_name);
@@ -308,6 +311,7 @@ class TsGenerator : public BaseGenerator {
const auto child_ns_level = it.second.ns->components.size() + 1;
for (const auto& it2 : ns_defs_) {
if (it2.second.ns->components.size() != child_ns_level) continue;
auto ts_file_path = it2.second.path + ".ts";
code += "export * as " + it2.second.symbolic_name + " from './";
int count = it2.second.ns->components.size() > 1 ? 2 : 1;
std::vector<std::string> rel_path;
@@ -1172,6 +1176,7 @@ class TsGenerator : public BaseGenerator {
if (union_type.enum_def) {
const auto& enum_def = *union_type.enum_def;
const auto enum_type = AddImport(imports, dependent, enum_def).name;
const std::string union_accessor = "this." + field_name;
const auto union_has_string = UnionHasStringType(enum_def);
const auto field_binded_method = "this." + field_name + ".bind(this)";