Incorrect values added by CreateVectorOfStructs() when the vector parameter has an odd size [C++, clang 15, OS X, 22.9.24 and later]

[nolen777]

When I call FlatBufferBuilder::CreateVectorOfStructs() passing in a vector with an odd number of entries, the created vector always has zero values in the first entry.

I believe this is a regression in 72aa85a

I've created a small repro case at https://github.com/nolen777/flatbuffers-test

To reproduce:

1. Clone https://github.com/nolen777/flatbuffers-test
2. bazel test --test_output=errors //src/main/cpp:test

The test is just checking that the entries in the FB are the same as the entries in the original vector.

If you change the argument vector to have an even number of entries (comment out line 22 and un-comment line 19 in src/main/cpp/test.cpp), the test passes. If you change the WORKSPACE file to the previous commit, the test passes. But on commit 72aa85a or later with an odd number of entries, the test fails.

[mikemccarty-vertex]

I also encountered this same bug and came to the same conclusion. To reproduce it, I riffed off the unit test added to the commit referenced above:

// sizeof(BadAlignmentSmall) == 3
// alignof(BadAlignmentSmall) == 1
struct BadAlignmentSmall {
  var_0: uint8;
  var_1: uint8;
  var_2: uint8;
}

// sizeof(BadAlignmentLarge) == 8
// alignof(BadAlignmentLarge) == 8
struct BadAlignmentLarge {
  var_0: ulong;
}

table OuterLarge {
  large: BadAlignmentLarge;
}

table BadAlignmentRoot {
  large: OuterLarge;
  small: [BadAlignmentSmall];
}

root_type BadAlignmentRoot;

The unit test (also riffing off the original unit test):

{
    using namespace flatbuffers;
    FlatBufferBuilder builder;

    BadAlignmentLarge large;
    Offset<OuterLarge> outer_large = CreateOuterLarge(builder, &large);

    std::vector<BadAlignmentSmall> small;
    small.emplace_back(1, 2, 3);
    auto small_offset = builder.CreateVectorOfStructs(small);

    auto root = CreateBadAlignmentRoot(builder, outer_large, small_offset);

    builder.Finish(root);

    auto rt = UnPackBadAlignmentRoot(builder.GetBufferPointer());
    ASSERT_EQ(rt->small.size(), 1);
    ASSERT_EQ(rt->small[0].var_0(), 1);
    ASSERT_EQ(rt->small[0].var_1(), 2);
    ASSERT_EQ(rt->small[0].var_2(), 3);
}

yields a failure:

Failure
Expected equality of these values:
  rt->small[0].var_0()
    Which is: '\0'
  1

This is a pretty significant regression.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

[mikemccarty-vertex]

I repeat: this is a pretty significant regression. Generated code leads to corrupted data.

[nolen777]

So my test repo above passes if I point to commit fb9afba, which I think is the last that passed tests? I'm trying to get tests to run locally to try @mikemccarty-vertex 's unit test.

[nolen777]

I'm fairly confident this is fixed now.

I added another field to the JustSmallStruct test in the latest master alignment_test.fbs and adjusted the test populate and use all three fields; patch attached. That passes. I then pulled that version of the test back on top of 72aa85a and the test fails with the unexpected 0 values.

8117.patch

I'll wait for @mikemccarty-vertex to comment before closing.

[mikemccarty-vertex]

Thanks @nolen777 for working through this. I admit I didn't do my due diligence on this since the last time I checked into it. It looks like you've been very thorough.

My only remaining question: Is there now a unit test in the repo that exercises this scenario? This is subtle enough that it's not hard imagining this sort of problem someday cropping up again.

[nolen777]

I don't believe there is a test that catches it. I'll try back-porting the existing test to the failing commit to verify that, and then I could probably adjust my patch to add to the existing unit tests instead of replacing one.

I won't be able to get to it for a few days, and it's not clear this repo is accepting outside contributions, but I'll give it a shot.

[nolen777]

I think #8363 should do it