mapbox / protozero

Minimalist protocol buffer decoder and encoder in C++
BSD 2-Clause "Simplified" License
292 stars 78 forks source link

array-bounds error in nested_testcase.pb.cc #98

Closed chazanov closed 4 years ago

chazanov commented 5 years ago

I'm trying to build protozero 1.6.8 with protobuf 3.7.0 and gcc 9.1.0. The prior version 1.6.7 has had worked, this one fails at compilation:

In member function ‘void TestNested::Sub::SharedCtor()’,
    inlined from ‘TestNested::Sub::Sub()’ at /build/protozero/src/protozero-1.6.8/build/test/t/nested/nested_testcase.pb.cc:412:13,
    inlined from ‘void InitDefaultsSub_nested_5ftestcase_2eproto()’ at /build/protozero/src/protozero-1.6.8/build/test/t/nested/nested_testcase.pb.cc:410:1:
/build/protozero/src/protozero-1.6.8/build/test/t/nested/nested_testcase.pb.cc:432:11: error: ‘void* memset(void*, int, size_t)’ offset [33, 36] from the object at ‘TestNested::_Sub_default_instance_’ is out of the bounds of referenced subobject ‘TestNested::Sub::subsub_’ with type ‘TestNested::SubSub*’ at offset 24 [-Werror=array-bounds]
  432 |   ::memset(&subsub_, 0, static_cast<size_t>(
      |   ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  433 |       reinterpret_cast<char*>(&i_) -
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  434 |       reinterpret_cast<char*>(&subsub_)) + sizeof(i_));
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In member function ‘void TestNested::Test::SharedCtor()’,
    inlined from ‘TestNested::Test::Test()’ at /build/protozero/src/protozero-1.6.8/build/test/t/nested/nested_testcase.pb.cc:720:13,
    inlined from ‘void InitDefaultsTest_nested_5ftestcase_2eproto()’ at /build/protozero/src/protozero-1.6.8/build/test/t/nested/nested_testcase.pb.cc:718:1:
/build/protozero/src/protozero-1.6.8/build/test/t/nested/nested_testcase.pb.cc:740:11: error: ‘void* memset(void*, int, size_t)’ offset [33, 36] from the object at ‘TestNested::_Test_default_instance_’ is out of the bounds of referenced subobject ‘TestNested::Test::sub_’ with type ‘TestNested::Sub*’ at offset 24 [-Werror=array-bounds]
  740 |   ::memset(&sub_, 0, static_cast<size_t>(
      |   ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  741 |       reinterpret_cast<char*>(&i_) -
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  742 |       reinterpret_cast<char*>(&sub_)) + sizeof(i_));
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
At global scope:
cc1plus: error: unrecognized command line option ‘-Wno-covered-switch-default’ [-Werror]
cc1plus: all warnings being treated as errors
make[2]: *** [test/CMakeFiles/writer_tests.dir/build.make:387: test/CMakeFiles/writer_tests.dir/t/nested/nested_testcase.pb.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:167: test/CMakeFiles/writer_tests.dir/all] Error 2
make: *** [Makefile:141: all] Error 2
joto commented 5 years ago

I can not reproduce this (but I have slightly different GCC and protobuf versions). Looks like it might be something in the Google protobuf library that your specific compiler version doesn't like.

I don't see anything that changed in Protozero that could have triggered this. Could you recheck that there is a difference here between 1.6.7 and 1.6.8 and maybe do a git bisect to find which exact commit triggers this?

chazanov commented 5 years ago

nested_testcase.pb.h seems to be generated. When I switch to protobuf 3.9.0 I can provoke the following errors:

#error This file was generated by an older version of protoc which is
#error incompatible with your Protocol Buffer headers. Please
#error regenerate this file with a newer version of protoc.

protobuf 3.9.0 has introduced incompabilities with old generated code, but they say it was fixed with 3.9.1: https://github.com/protocolbuffers/protobuf/issues/6379

Which protobuf version do you advise?

joto commented 5 years ago

Yes, the *.pb.h/cc files are generated. If you switch versions of Google's protobuf library this can lead to problems in which case you should do a new clean build. It shouldn't matter which protobuf library you use, they should all work.

Anyway: All of this doesn't matter for the use of Protozero. Protozero only uses the Google libraries to run its tests, making sure it stays compatible with them. For normal use, Google's libraries are not used at all. So chances are this is only due to some confusion with the Google libraries which don't matter really.

chazanov commented 5 years ago

When I try to rebuild ArchLinux's official protobuf protobuf's own tests fail. This even happens in a clean chroot. So probably something with my config is very broken, but not only for me.

How can I disable all tests in protozero? Not forever - just for a quick test.

joto commented 5 years ago

I don't understand. If you don't want the tests, just don't run them.

chazanov commented 5 years ago

I just run

  cmake -DCMAKE_INSTALL_PREFIX=/usr ..
  make

and don't run ctest. Nevertheless the writer_tests from test/CMakeFiles/writer_tests.dir/ are executed.

joto commented 5 years ago

The test should not run if you don't call ctest. I can't imagine why that would happen. But your problem isn't that the test run fails but the compilation fails and the compilation is, of course, done when you call make. But you don't need to call make, there isn't anything to make except the tests really. Well, there are tools and docs, but you can compile them separately if needed. To use the library you don't have to call make, because it is header-only.