diff --git a/include/flatbuffers/flatbuffer_builder.h b/include/flatbuffers/flatbuffer_builder.h index a33c8966976..f3ffeb0d70a 100644 --- a/include/flatbuffers/flatbuffer_builder.h +++ b/include/flatbuffers/flatbuffer_builder.h @@ -913,8 +913,7 @@ template class FlatBufferBuilderImpl { typedef typename VectorT::size_type LenT; typedef typename OffsetT>::offset_type offset_type; - StartVector(len * sizeof(T) / AlignOf(), sizeof(T), - AlignOf()); + StartVector(len, sizeof(T), AlignOf()); if (len > 0) { PushBytes(reinterpret_cast(v), sizeof(T) * len); } @@ -1384,8 +1383,7 @@ template class FlatBufferBuilderImpl { // Must be completed with EndVectorOfStructs(). template class OffsetT = Offset> T *StartVectorOfStructs(size_t vector_size) { - StartVector(vector_size * sizeof(T) / AlignOf(), sizeof(T), - AlignOf()); + StartVector(vector_size, sizeof(T), AlignOf()); return reinterpret_cast(buf_.make_space(vector_size * sizeof(T))); } diff --git a/tests/alignment_test.cpp b/tests/alignment_test.cpp index 731c328666d..1d981e10ed4 100644 --- a/tests/alignment_test.cpp +++ b/tests/alignment_test.cpp @@ -2,6 +2,7 @@ #include "tests/alignment_test_generated.h" #include "flatbuffers/flatbuffer_builder.h" +#include "flatbuffers/util.h" #include "test_assert.h" namespace flatbuffers { @@ -24,7 +25,51 @@ void AlignmentTest() { builder.Finish(root); Verifier verifier(builder.GetBufferPointer(), builder.GetSize()); - TEST_ASSERT(VerifyBadAlignmentRootBuffer(verifier)); + TEST_ASSERT(verifier.VerifyBuffer(nullptr)); + + + // ============= Test Small Structs Vector misalignment ======== + + builder.Clear(); + + // creating 5 structs with 2 bytes each + // 10 bytes in total for Vector data is needed + std::vector small_vector = { + { 2, 1 }, { 3, 1 }, { 4, 1 } + }; + // CreateVectorOfStructs is used in the generated CreateSmallStructsDirect() + // method, but we test it directly + Offset> small_structs_offset = + builder.CreateVectorOfStructs(small_vector); + Offset small_structs_root = + CreateSmallStructs(builder, small_structs_offset); + + builder.Finish(small_structs_root); + + // Save the binary that we later can annotate with `flatc --annotate` command + // NOTE: the conversion of the JSON data to --binary via `flatc --binary` + // command is not changed with that fix and was always producing the + // correct binary data. + // SaveFile("alignment_test_{before,after}_fix.bin", + // reinterpret_cast(builder.GetBufferPointer()), + // builder.GetSize(), true); + + Verifier verifier_small_structs(builder.GetBufferPointer(), + builder.GetSize()); + TEST_ASSERT(verifier_small_structs.VerifyBuffer(nullptr)); + + // Reading SmallStructs vector values back and compare with original + auto root_msg = + flatbuffers::GetRoot(builder.GetBufferPointer()); + + TEST_EQ(root_msg->small_structs()->size(), small_vector.size()); + for (flatbuffers::uoffset_t i = 0; i < root_msg->small_structs()->size(); + ++i) { + TEST_EQ(small_vector[i].var_0(), + root_msg->small_structs()->Get(i)->var_0()); + TEST_EQ(small_vector[i].var_1(), + root_msg->small_structs()->Get(i)->var_1()); + } } } // namespace tests diff --git a/tests/alignment_test.fbs b/tests/alignment_test.fbs index 3fc08bd7ff5..11f5eb79c72 100644 --- a/tests/alignment_test.fbs +++ b/tests/alignment_test.fbs @@ -21,4 +21,15 @@ table BadAlignmentRoot { small: [BadAlignmentSmall]; } -root_type BadAlignmentRoot; +// sizeof(JustSmallStruct) == 2 +// alignof(JustSmallStruct) == 1 +struct JustSmallStruct { + var_0: uint8; + var_1: uint8; +} + +table SmallStructs { + small_structs: [JustSmallStruct]; +} + +root_type SmallStructs; diff --git a/tests/alignment_test.json b/tests/alignment_test.json new file mode 100644 index 00000000000..7831d15e854 --- /dev/null +++ b/tests/alignment_test.json @@ -0,0 +1,16 @@ +{ + "small_structs": [ + { + "var_0": 2, + "var_1": 1 + }, + { + "var_0": 3, + "var_1": 1 + }, + { + "var_0": 4, + "var_1": 1 + } + ] +} \ No newline at end of file diff --git a/tests/alignment_test_after_fix.afb b/tests/alignment_test_after_fix.afb new file mode 100644 index 00000000000..b8312e0ed43 --- /dev/null +++ b/tests/alignment_test_after_fix.afb @@ -0,0 +1,31 @@ +// Annotated Flatbuffer Binary +// +// Schema file: alignment_test.fbs +// Binary file: alignment_test_after_fix.bin + +header: + +0x00 | 0C 00 00 00 | UOffset32 | 0x0000000C (12) Loc: 0x0C | offset to root table `SmallStructs` + +padding: + +0x04 | 00 00 | uint8_t[2] | .. | padding + +vtable (SmallStructs): + +0x06 | 06 00 | uint16_t | 0x0006 (6) | size of this vtable + +0x08 | 08 00 | uint16_t | 0x0008 (8) | size of referring table + +0x0A | 04 00 | VOffset16 | 0x0004 (4) | offset to field `small_structs` (id: 0) + +root_table (SmallStructs): + +0x0C | 06 00 00 00 | SOffset32 | 0x00000006 (6) Loc: 0x06 | offset to vtable + +0x10 | 04 00 00 00 | UOffset32 | 0x00000004 (4) Loc: 0x14 | offset to field `small_structs` (vector) + +vector (SmallStructs.small_structs): + +0x14 | 03 00 00 00 | uint32_t | 0x00000003 (3) | length of vector (# items) + +0x18 | 02 | uint8_t | 0x02 (2) | struct field `[0].var_0` of 'JustSmallStruct' (UByte) + +0x19 | 01 | uint8_t | 0x01 (1) | struct field `[0].var_1` of 'JustSmallStruct' (UByte) + +0x1A | 03 | uint8_t | 0x03 (3) | struct field `[1].var_0` of 'JustSmallStruct' (UByte) + +0x1B | 01 | uint8_t | 0x01 (1) | struct field `[1].var_1` of 'JustSmallStruct' (UByte) + +0x1C | 04 | uint8_t | 0x04 (4) | struct field `[2].var_0` of 'JustSmallStruct' (UByte) + +0x1D | 01 | uint8_t | 0x01 (1) | struct field `[2].var_1` of 'JustSmallStruct' (UByte) + +padding: + +0x1E | 00 00 | uint8_t[2] | .. | padding diff --git a/tests/alignment_test_after_fix.bin b/tests/alignment_test_after_fix.bin new file mode 100644 index 00000000000..e9c1ffd0892 Binary files /dev/null and b/tests/alignment_test_after_fix.bin differ diff --git a/tests/alignment_test_before_fix.afb b/tests/alignment_test_before_fix.afb new file mode 100644 index 00000000000..c2665310ae1 --- /dev/null +++ b/tests/alignment_test_before_fix.afb @@ -0,0 +1,31 @@ +// Annotated Flatbuffer Binary +// +// Schema file: alignment_test.fbs +// Binary file: alignment_test_before_fix.bin + +header: + +0x00 | 0C 00 00 00 | UOffset32 | 0x0000000C (12) Loc: 0x0C | offset to root table `SmallStructs` + +padding: + +0x04 | 00 00 | uint8_t[2] | .. | padding + +vtable (SmallStructs): + +0x06 | 06 00 | uint16_t | 0x0006 (6) | size of this vtable + +0x08 | 08 00 | uint16_t | 0x0008 (8) | size of referring table + +0x0A | 04 00 | VOffset16 | 0x0004 (4) | offset to field `small_structs` (id: 0) + +root_table (SmallStructs): + +0x0C | 06 00 00 00 | SOffset32 | 0x00000006 (6) Loc: 0x06 | offset to vtable + +0x10 | 04 00 00 00 | UOffset32 | 0x00000004 (4) Loc: 0x14 | offset to field `small_structs` (vector) + +vector (SmallStructs.small_structs): + +0x14 | 03 00 00 00 | uint32_t | 0x00000003 (3) | length of vector (# items) + +0x18 | 00 | uint8_t | 0x00 (0) | struct field `[0].var_0` of 'JustSmallStruct' (UByte) + +0x19 | 00 | uint8_t | 0x00 (0) | struct field `[0].var_1` of 'JustSmallStruct' (UByte) + +0x1A | 02 | uint8_t | 0x02 (2) | struct field `[1].var_0` of 'JustSmallStruct' (UByte) + +0x1B | 01 | uint8_t | 0x01 (1) | struct field `[1].var_1` of 'JustSmallStruct' (UByte) + +0x1C | 03 | uint8_t | 0x03 (3) | struct field `[2].var_0` of 'JustSmallStruct' (UByte) + +0x1D | 01 | uint8_t | 0x01 (1) | struct field `[2].var_1` of 'JustSmallStruct' (UByte) + +unknown (no known references): + +0x1E | 04 01 | ?uint8_t[2] | .. | WARN: could be corrupted padding region. diff --git a/tests/alignment_test_before_fix.bin b/tests/alignment_test_before_fix.bin new file mode 100644 index 00000000000..d914f1880df Binary files /dev/null and b/tests/alignment_test_before_fix.bin differ