Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misalignment of small structs in a Vector (C++) #8024

Closed
bexcite opened this issue Jul 5, 2023 · 0 comments · Fixed by #7883
Closed

Misalignment of small structs in a Vector (C++) #8024

bexcite opened this issue Jul 5, 2023 · 0 comments · Fixed by #7883

Comments

@bexcite
Copy link
Contributor

bexcite commented Jul 5, 2023

C++, generated headers create small struct vectors with incorrect alignment. Seen in AMD/x84 and Mac M1 architectures.

I've traced the bug and it was introduced by this PR: #7520

Also, I've made a fix (#7883), but it seems lost in PRs without issues, so I'm posting the issue to give it more weight.

The issue:
Small structs that are less than uoffset_t in size without any special alignment rules may result in a broken Vector layout that reads back with different values.

Example of the flatbuffers:

// sizeof(JustSmallStruct) == 2
// alignof(JustSmallStruct) == 1
struct JustSmallStruct {
  var_0: uint8;
  var_1: uint8;
}

table SmallStructs {
  small_structs: [JustSmallStruct];
}

now if a Vector of just 3 values of JustSmallStruct: { 2, 1 }, { 3, 1 }, { 4, 1 } is created with FlatBufferBuilder::CreateVectorOfStructs<T>() we get this binary Vector that is when read back get zeros in the first JustSmallStruct item:

03 00 00 00 00 00 02 01 03 01 04 01
          ^    ^
          |    |
  vector len   |
               |
         followed by two zero bytes of an alignment
         that was added because uoffset_t required
         4 bytes alignment!?

More confusing is that FlatBufferBuilder::CreateUninitializedVectorOfStructs<T>() method doesn't have this behavior and not adding "2 bytes zeros" before writing the vector len element.

PR with a fix and tests and annotated Flatbuffers that shows the issue is here: #7883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant