Skip to content

[C++] Rare bad buffer content alignment if sizeof(T) != alignof(T) #7516

@Naios

Description

@Naios
  • f124e41
  • C++ API
  • MSVC 2019 (16.11.17) is probably applicable to Clang and GCC as well

I'm encountering a rare issue where a completely valid buffer gets generated into badly aligned memory locations.
This behavior is hard to reproduce through a minimalistic test case, but I will explain in detail what probably happens here.

To trigger the issue we need two structs, one with a large size and small alignment (BadAlignmentSmall) and another one with a smaller size and large alignment (BadAlignmentLarge) that are somehow written to our flatbuffer:

struct BadAlignmentSmall {
  var_0: uint;
  var_1: uint;
  var_2: uint;
};

// sizeof(BadAlignmentSmall) == 12
// alignof(BadAlignmentSmall) == 4

struct BadAlignmentLarge {
  var_0: ulong;
};

// sizeof(BadAlignmentLarge) == 8
// alignof(BadAlignmentLarge) == 8

Regardless of the buffer_minalign variable in vector_downward, the final flatbuffers buffer gets aligned by the alignment tracked through FlatBufferBuilder::minalign_ which is recorded through TrackMinAlign (by using the upper bound max of its input value and the current minalign_).

  void Align(size_t elem_size) {
    TrackMinAlign(elem_size);
    buf_.fill(PaddingBytes(buf_.size(), elem_size));
  }

In the flatbuffers codebase we find the following calls to Align:

  • Align(sizeof(T));
  • 2x Align(AlignOf<T>());

Because Align is called with the size and the alignment in different places it can happen that minalign_ is finally 12 which is incompatible with our real required alignment of 8.
This causes the verifier with enabled alignment checks to fail because the buffer gets misaligned on the final call to Finish().

To fix this we have two options:

  • replace Align(sizeof(T)) by Align(alignof(T)) (or Align(AlignOf<T>()) respectively), if this was a typo
  • Use a better function to union two alignments (e.g. max common power of two), max should not be used for that in case you pass the size to Align (size 12 & alignment 8 should result in alignment 16, not 12) - assuming 16 would be a valid real-world alignment.
  void TrackMinAlign(size_t elem_size) {
    if (elem_size > minalign_)  minalign_ = elem_size;
  }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions