Skip to content

Conversation

@trofi
Copy link
Contributor

@trofi trofi commented Nov 21, 2021

Detected instability when built flatbuffers-2.0.0 on gcc-12:

[ 75%] Building CXX object CMakeFiles/flattests.dir/tests/test_builder.cpp.o
.../c++/12.0.0/bits/shared_ptr_base.h:397:45: error: 'size' may be used uninitialized [-Werror=maybe-uninitialized]
  397 |       explicit _Sp_ebo_helper(_Tp&& __tp) : _M_tp(std::move(__tp)) { }
      |                                             ^~~~~~~~~~~~~~~~~~~~~~
In file included from flatbuffers/tests/test_builder.cpp:1:
flatbuffers/tests/test_builder.h: In function 'void builder_move_assign_after_releaseraw_test(Builder) [with Builder = flatbuffers::FlatBufferBuilder]':
flatbuffers/tests/test_builder.h:63:10: note: 'size' was declared here
   63 |   size_t size, offset;
      |          ^~~~
...
In file included from flatbuffers/tests/test_builder.cpp:1:
flatbuffers/tests/test_builder.h: In function 'void builder_move_assign_after_releaseraw_test(Builder) [with Builder = GrpcLikeMessageBuilder]':
flatbuffers/tests/test_builder.h:63:10: note: 'size' was declared here
   63 |   size_t size, offset;
      |          ^~~~
cc1plus: all warnings being treated as errors

Here is the relevant bit of test:

template<class Builder>
void builder_move_assign_after_releaseraw_test(Builder b1) {
  auto root_offset1 = populate1(b1);
  b1.Finish(root_offset1);
  size_t size, offset;
  std::shared_ptr<uint8_t> raw(
      b1.ReleaseRaw(size, offset), [size](uint8_t *ptr) {
        flatbuffers::DefaultAllocator::dealloc(ptr, size);
      });

Note how b1.ReleaseRaw(size, offset) is expected to populate size
and [size](uint8_t *ptr) { captures the result. But both are parameters
to the same function call and thus evaluation order is unspecified.

Thank you for submitting a PR!

Please delete this standard text once you've created your own description.

Make sure you include the names of the affected language(s) in your PR title.
This helps us get the correct maintainers to look at your issue.

If you make changes to any of the code generators, be sure to run
cd tests && bash generate_code.sh (or equivalent .bat) and include the generated
code changes in the PR. This allows us to better see the effect of the PR.

If your PR includes C++ code, please adhere to the Google C++ Style Guide,
and don't forget we try to support older compilers (e.g. VS2010, GCC 4.6.3),
so only some C++11 support is available.

For any C++ changes, please make sure to run sh scripts/clang-format-git.sh

Include other details as appropriate.

Thanks!

Detected instability when built `flatbuffers-2.0.0` on `gcc-12`:

    [ 75%] Building CXX object CMakeFiles/flattests.dir/tests/test_builder.cpp.o
    .../c++/12.0.0/bits/shared_ptr_base.h:397:45: error: 'size' may be used uninitialized [-Werror=maybe-uninitialized]
      397 |       explicit _Sp_ebo_helper(_Tp&& __tp) : _M_tp(std::move(__tp)) { }
          |                                             ^~~~~~~~~~~~~~~~~~~~~~
    In file included from flatbuffers/tests/test_builder.cpp:1:
    flatbuffers/tests/test_builder.h: In function 'void builder_move_assign_after_releaseraw_test(Builder) [with Builder = flatbuffers::FlatBufferBuilder]':
    flatbuffers/tests/test_builder.h:63:10: note: 'size' was declared here
       63 |   size_t size, offset;
          |          ^~~~
    ...
    In file included from flatbuffers/tests/test_builder.cpp:1:
    flatbuffers/tests/test_builder.h: In function 'void builder_move_assign_after_releaseraw_test(Builder) [with Builder = GrpcLikeMessageBuilder]':
    flatbuffers/tests/test_builder.h:63:10: note: 'size' was declared here
       63 |   size_t size, offset;
          |          ^~~~
    cc1plus: all warnings being treated as errors

Here is the relevant bit of test:

    template<class Builder>
    void builder_move_assign_after_releaseraw_test(Builder b1) {
      auto root_offset1 = populate1(b1);
      b1.Finish(root_offset1);
      size_t size, offset;
      std::shared_ptr<uint8_t> raw(
          b1.ReleaseRaw(size, offset), [size](uint8_t *ptr) {
            flatbuffers::DefaultAllocator::dealloc(ptr, size);
          });

Note how `b1.ReleaseRaw(size, offset)` is expected to populate `size`
and `[size](uint8_t *ptr) {` captures the result. But both are parameters
to the same function call and thus evaluation order is unspecified.
@github-actions github-actions bot added the c++ label Nov 21, 2021
@aardappel
Copy link
Collaborator

Yup that makes sense, thanks for fixing!

@aardappel aardappel merged commit 85b4eff into google:master Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants