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

ETLCPP doesn't build with gcc-13 #736

Open
dkogan opened this issue Jul 23, 2023 · 36 comments
Open

ETLCPP doesn't build with gcc-13 #736

dkogan opened this issue Jul 23, 2023 · 36 comments
Assignees

Comments

@dkogan
Copy link

dkogan commented Jul 23, 2023

Hello. I just built and uploaded the latest etlcpp release to Debian. In the process I hit 3 points where the build failed with the gcc-13 compiler. Here they are.

  1. The build system runs this command, with the shown effect:

       g++-13 \
         -DETL_DEBUG                                     \
         -I../../test/../include                         \
         -isystem ../../test/UnitTest++/..               \
         -std=gnu++20                                    \
         -fno-omit-frame-pointer                         \
         -fno-common                                     \
         -Wall                                           \
         -Wextra                                         \
         -Werror                                         \
         -o CMakeFiles/etl_tests.dir/test_optional.cpp.o \
         -c ../../test/test_optional.cpp
    
       ....
    
       ../../test/test_optional.cpp:497:24: error: ‘uint8_t’ is not a member of ‘std’; did you mean ‘wint_t’?
         497 |     etl::optional<std::uint8_t> get_optional_test_bug_634()
             |                        ^~~~~~~
             |                        wint_t
    

    This can be fixed by adding #include <cstdint>, as done in this patch: https://salsa.debian.org/debian/etlcpp/-/blob/master/debian/patches/include-cstdint-to-build-with-gcc-13.patch

  2. g++-13 generates more warnings than earlier versions of gcc, and we're
    building with -Werror, so these are fatal. We get this:

       g++-13                                                 \
         -DETL_DEBUG                                          \
         -I../../test/../include                              \
         -isystem ../../test/UnitTest++/..                    \
         -g                                                   \
         -O2                                                  \
         -fstack-protector-strong                             \
         -Wformat                                             \
         -Werror=format-security                              \
         -Wdate-time                                          \
         -D_FORTIFY_SOURCE=2                                  \
         -std=gnu++20                                         \
         -fno-omit-frame-pointer                              \
         -fno-common                                          \
         -Wall                                                \
         -Wextra                                              \
         -Werror                                              \
         -o CMakeFiles/etl_tests.dir/test_bitset_legacy.cpp.o \
         -c ../../test/test_bitset_legacy.cpp
    
       ....
    
       In file included from ../../test/test_bitset_legacy.cpp:35:
       In member function 'etl::ibitset& etl::ibitset::operator<<=(size_t)',
           inlined from 'etl::bitset<MaxN>& etl::bitset<MaxN>::operator<<=(size_t) [with long unsigned int MaxN = 60]' at ../../test/../include/etl/private/bitset_legacy.h:1420:33,
           inlined from 'etl::bitset<MaxN> etl::bitset<MaxN>::operator<<(size_t) const [with long unsigned int MaxN = 60]' at ../../test/../include/etl/private/bitset_legacy.h:1410:12,
           inlined from 'virtual void {anonymous}::Suitetest_bitset_legacy::Testtest_shift_left_copy_operator::RunImpl() const' at ../../test/test_bitset_legacy.cpp:920:21:
       ../../test/../include/etl/private/bitset_legacy.h:849:13: error: iteration 7 invokes undefined behavior [-Werror=aggressive-loop-optimizations]
         849 |             --dst_index;
             |             ^~~~~~~~~~~
       ../../test/../include/etl/private/bitset_legacy.h:846:28: note: within this loop
         846 |           while (dst_index >= 0)
             |                  ~~~~~~~~~~^~~~
       In member function 'etl::ibitset& etl::ibitset::operator<<=(size_t)',
           inlined from 'etl::bitset<MaxN>& etl::bitset<MaxN>::operator<<=(size_t) [with long unsigned int MaxN = 60]' at ../../test/../include/etl/private/bitset_legacy.h:1420:33,
           inlined from 'etl::bitset<MaxN> etl::bitset<MaxN>::operator<<(size_t) const [with long unsigned int MaxN = 60]' at ../../test/../include/etl/private/bitset_legacy.h:1410:12,
           inlined from 'virtual void {anonymous}::Suitetest_bitset_legacy::Testtest_shift_left_copy_operator::RunImpl() const' at ../../test/test_bitset_legacy.cpp:917:21:
       ../../test/../include/etl/private/bitset_legacy.h:849:13: error: iteration 7 invokes undefined behavior [-Werror=aggressive-loop-optimizations]
         849 |             --dst_index;
             |             ^~~~~~~~~~~
       ../../test/../include/etl/private/bitset_legacy.h:846:28: note: within this loop
         846 |           while (dst_index >= 0)
             |                  ~~~~~~~~~~^~~~
    

    I'm working around this by passing -Wno-aggressive-loop-optimizations

  3. And another similar one:

       g++-13                                                 \
         -DETL_DEBUG                                          \
         -I../../test/../include                              \
         -isystem ../../test/UnitTest++/..                    \
         -g                                                   \
         -O2                                                  \
         -fstack-protector-strong                             \
         -Wformat                                             \
         -Werror=format-security                              \
         -Wdate-time                                          \
         -D_FORTIFY_SOURCE=2                                  \
         -std=gnu++20                                         \
         -fno-omit-frame-pointer                              \
         -fno-common                                          \
         -Wall                                                \
         -Wextra                                              \
         -Werror                                              \
         -o CMakeFiles/etl_tests.dir/test_message_packet.cpp.o \
         -c ../../test/test_message_packet.cpp
    
       ....
    
       ../../test/test_message_packet.cpp: In member function ‘virtual void {anonymous}::Suitetest_message_packet::Testmessage_packet_move_constructor::RunImpl() const’:
       ../../test/test_message_packet.cpp:343:49: error: moving a temporary object prevents copy elision [-Werror=pessimizing-move]
         343 |       Packet packet2(std::move(Packet(message1)));
             |                                                 ^
       ../../test/test_message_packet.cpp:343:49: note: remove ‘std::move’ call
    

    I'm working around this by passing -Wno-pessimizing-move

Thanks.

@dkogan
Copy link
Author

dkogan commented Jul 23, 2023

Forgot to mention:

  • amd64 architecture
  • Bleeding-edge Debian GNU/Linux
  • gcc 13.1.0
  • latest etlcpp tag: 20.37.2

@jwellbelove
Copy link
Contributor

Regarding the message_packet_move_constructor warning.
The test code disables that warning for the particular line in the unit test.
I'm not sure why your build raising it.

#include "etl/private/diagnostic_pessimizing_move_push.h"
      Packet packet2(std::move(Packet(message1)));
#include "etl/private/diagnostic_pop.h"

@jwellbelove
Copy link
Contributor

OK, I know why. The diagnostic push does not disable it for GCC.
I'll have to check if GCC12 supports this.

@jwellbelove
Copy link
Contributor

Possible fix. Can you test?
736-aa64f42-Possible fix.patch

@dkogan
Copy link
Author

dkogan commented Jul 24, 2023

Hello. Thanks for looking. I applied this patch in lieu of the other workarounds. I now get this. No idea if it's related to the others, or if it is a new issue that somehow I didn't notice before:

   g++-13                                                 \
     -DETL_DEBUG                                          \
     -I../../test/../include                              \
     -isystem ../../test/UnitTest++/..                    \
     -g                                                   \
     -O2                                                  \
     -fstack-protector-strong                             \
     -Wformat                                             \
     -Werror=format-security                              \
     -Wdate-time                                          \
     -D_FORTIFY_SOURCE=2                                  \
     -std=gnu++20                                         \
     -fno-omit-frame-pointer                              \
     -fno-common                                          \
     -Wall                                                \
     -Wextra                                              \
     -Werror                                              \
     -o CMakeFiles/etl_tests.dir/test_bit_stream.cpp.o \
     -c ../../test/test_bit_stream.cpp

   ....


In file included from ../../test/UnitTest++/CheckMacros.h:6,
                 from ../../test/UnitTest++/UnitTestPP.h:6,
                 from ../../test/UnitTest++/UnitTest++.h:1,
                 from ../../test/unit_test_framework.h:32,
                 from ../../test/test_bit_stream.cpp:29:
In function ‘bool UnitTest::AreClose(const Expected&, const Actual&, const Tolerance&) [with Expected = float; Actual = double; Tolerance = float]’,
    inlined from ‘void UnitTest::CheckClose(TestResults&, const Expected&, const Actual&, const Tolerance&, const TestDetails&) [with Expected = float; Actual = double; Tolerance = float]’ at ../../test/UnitTest++/Checks.h:239:20,
    inlined from ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_float::RunImpl() const’ at ../../test/test_bit_stream.cpp:1068:7:
../../test/UnitTest++/Checks.h:232:49: error: ‘rd’ may be used uninitialized [-Werror=maybe-uninitialized]
  232 |       return (actual >= (expected - tolerance)) && (actual <= (expected + tolerance));
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../test/test_bit_stream.cpp: In member function ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_float::RunImpl() const’:
../../test/test_bit_stream.cpp:1062:14: note: ‘rd’ was declared here
 1062 |       double rd;
      |              ^~
In file included from ../../test/test_bit_stream.cpp:31:
In member function ‘typename etl::enable_if<etl::is_integral<T>::value, bool>::type etl::bit_stream::get(T&, uint_least8_t) [with T = int]’,
    inlined from ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’ at ../../test/test_bit_stream.cpp:998:7:
../../test/../include/etl/bit_stream.h:255:41: error: ‘ri1’ may be used uninitialized [-Werror=maybe-uninitialized]
  255 |         value = etl::sign_extend<ST, ST>(value, bits);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
../../test/test_bit_stream.cpp: In member function ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’:
../../test/test_bit_stream.cpp:985:15: note: ‘ri1’ declared here
  985 |       int32_t ri1;
      |               ^~~
In member function ‘typename etl::enable_if<etl::is_integral<T>::value, bool>::type etl::bit_stream::get(T&, uint_least8_t) [with T = int]’,
    inlined from ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’ at ../../test/test_bit_stream.cpp:1004:7:
../../test/../include/etl/bit_stream.h:255:41: error: ‘ri2’ may be used uninitialized [-Werror=maybe-uninitialized]
  255 |         value = etl::sign_extend<ST, ST>(value, bits);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
../../test/test_bit_stream.cpp: In member function ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’:
../../test/test_bit_stream.cpp:986:15: note: ‘ri2’ declared here
  986 |       int32_t ri2;
      |               ^~~
In file included from ../../test/../include/etl/endianness.h:36,
                 from ../../test/../include/etl/bit_stream.h:32:
In function ‘constexpr TReturn etl::sign_extend(TValue, size_t) [with TReturn = signed char; TValue = signed char]’,
    inlined from ‘typename etl::enable_if<etl::is_integral<T>::value, bool>::type etl::bit_stream::get(T&, uint_least8_t) [with T = char]’ at ../../test/../include/etl/bit_stream.h:255:41,
    inlined from ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’ at ../../test/test_bit_stream.cpp:1013:7:
../../test/../include/etl/binary.h:321:11: error: ‘rc2’ may be used uninitialized [-Werror=maybe-uninitialized]
  321 |     value = value & TValue((TValue(1) << NBITS) - 1);
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../test/test_bit_stream.cpp: In member function ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’:
../../test/test_bit_stream.cpp:982:12: note: ‘rc2’ was declared here
  982 |       char rc2;
      |            ^~~
In function ‘constexpr TReturn etl::sign_extend(TValue, size_t) [with TReturn = long int; TValue = long int]’,
    inlined from ‘typename etl::enable_if<etl::is_integral<T>::value, bool>::type etl::bit_stream::get(T&, uint_least8_t) [with T = long int]’ at ../../test/../include/etl/bit_stream.h:255:41,
    inlined from ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’ at ../../test/test_bit_stream.cpp:1016:7:
../../test/../include/etl/binary.h:321:11: error: ‘rll’ may be used uninitialized [-Werror=maybe-uninitialized]
  321 |     value = value & TValue((TValue(1) << NBITS) - 1);
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../test/test_bit_stream.cpp: In member function ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_variable_size::RunImpl() const’:
../../test/test_bit_stream.cpp:989:15: note: ‘rll’ was declared here
  989 |       int64_t rll;
      |               ^~~
In file included from ../../test/UnitTest++/TestMacros.h:6,
                 from ../../test/UnitTest++/UnitTestPP.h:5:
../../test/test_bit_stream.cpp: In member function ‘virtual void {anonymous}::Suitetest_bit_stream::Testput_get_multiple_full_size::RunImpl() const’:
../../test/test_bit_stream.cpp:922:28: error: ‘rc2’ may be used uninitialized [-Werror=maybe-uninitialized]
  922 |       CHECK_EQUAL(int(c2), int(rc2));
      |                            ^~~~~~~~
../../test/UnitTest++/ExceptionMacros.h:7:37: note: in definition of macro ‘UNITTEST_IMPL_TRY’
    7 |    #define UNITTEST_IMPL_TRY(x) try x
      |                                     ^
../../test/UnitTest++/CheckMacros.h:248:27: note: in expansion of macro ‘UNITTEST_CHECK_EQUAL’
  248 |       #define CHECK_EQUAL UNITTEST_CHECK_EQUAL
      |                           ^~~~~~~~~~~~~~~~~~~~
../../test/test_bit_stream.cpp:922:7: note: in expansion of macro ‘CHECK_EQUAL’
  922 |       CHECK_EQUAL(int(c2), int(rc2));
      |       ^~~~~~~~~~~
../../test/test_bit_stream.cpp:891:12: note: ‘rc2’ was declared here
  891 |       char rc2;
      |            ^~~
cc1plus: all warnings being treated as errors

@jwellbelove
Copy link
Contributor

Another patch for the above. 'maybe uninitialized' in unit tests.
736-b04c4da-Possible fix.patch

@dkogan
Copy link
Author

dkogan commented Jul 26, 2023

Hello. With that patch I get the pessimizing-move warning above.

@jwellbelove
Copy link
Contributor

@dkogan
Copy link
Author

dkogan commented Jul 26, 2023

cd obj-x86_64-linux-gnu/test &&     \
g++-13                              \
  -DETL_DEBUG                       \
  -I../../test/../include           \
  -isystem ../../test/UnitTest++/.. \
  -g                                \
  -O2                               \
  -fstack-protector-strong          \
  -Wformat                          \
  -Werror=format-security           \
  -Wdate-time                       \
  -D_FORTIFY_SOURCE=2               \
  -std=gnu++20                      \
  -fno-omit-frame-pointer           \
  -fno-common                       \
  -Wall                             \
  -Wextra                           \
  -Werror                           \
  -o /tmp/tst.o                     \
  -c ../../test/test_message_packet.cpp


In file included from /usr/include/c++/13/string:54,
                 from /usr/include/c++/13/bits/locale_classes.h:40,
                 from /usr/include/c++/13/bits/ios_base.h:41,
                 from /usr/include/c++/13/ios:44,
                 from /usr/include/c++/13/istream:40,
                 from /usr/include/c++/13/sstream:40,
                 from ../../test/UnitTest++/MemoryOutStream.h:9,
                 from ../../test/UnitTest++/ExecuteTest.h:8,
                 from ../../test/UnitTest++/TestMacros.h:7,
                 from ../../test/UnitTest++/UnitTestPP.h:5,
                 from ../../test/UnitTest++/UnitTest++.h:1,
                 from ../../test/unit_test_framework.h:32,
                 from ../../test/test_message_packet.cpp:29:
In member function ‘constexpr std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::length() const [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’,
    inlined from ‘constexpr std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’ at /usr/include/c++/13/bits/basic_string.h:541:62,
    inlined from ‘{anonymous}::Message3::Message3({anonymous}::Message3&&)’ at ../../test/test_message_packet.cpp:153:9,
    inlined from ‘bool etl::message_packet<TMessageTypes>::add_new_message_type(etl::imessage&&) [with TType = {anonymous}::Message3; TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:312:9,
    inlined from ‘void etl::message_packet<TMessageTypes>::add_new_message(etl::imessage&&) [with TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:275:43,
    inlined from ‘void etl::message_packet<TMessageTypes>::add_new_message(etl::imessage&&) [with TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:273:10,
    inlined from ‘etl::message_packet<TMessageTypes>& etl::message_packet<TMessageTypes>::operator=(etl::message_packet<TMessageTypes>&&) [with TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:173:24,
    inlined from ‘virtual void {anonymous}::Suitetest_message_packet::Testmessage_packet_move_assignment::RunImpl() const’ at ../../test/test_message_packet.cpp:394:34:
/usr/include/c++/13/bits/basic_string.h:1067:16: error: ‘*(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*)((char*)&packet1 + offsetof(etl::Packet, etl::message_packet<<unnamed>::Message1, <unnamed>::Message2, <unnamed>::Message3>::data.etl::aligned_storage<48, 8>::type::data[8])).std::__cxx11::basic_string<char>::_M_string_length’ may be used uninitialized [-Werror=maybe-uninitialized]
 1067 |       { return _M_string_length; }
      |                ^~~~~~~~~~~~~~~~
../../test/test_message_packet.cpp: In member function ‘virtual void {anonymous}::Suitetest_message_packet::Testmessage_packet_move_assignment::RunImpl() const’:
../../test/test_message_packet.cpp:391:14: note: ‘packet1’ declared here
  391 |       Packet packet1(message1);
      |              ^~~~~~~
In member function ‘constexpr std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::length() const [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’,
    inlined from ‘constexpr std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’ at /usr/include/c++/13/bits/basic_string.h:541:62,
    inlined from ‘{anonymous}::Message3::Message3(const {anonymous}::Message3&)’ at ../../test/test_message_packet.cpp:146:9,
    inlined from ‘bool etl::message_packet<TMessageTypes>::add_new_message_type(const etl::imessage&) [with TType = {anonymous}::Message3; TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:296:9,
    inlined from ‘void etl::message_packet<TMessageTypes>::add_new_message(const etl::imessage&) [with TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:269:43,
    inlined from ‘void etl::message_packet<TMessageTypes>::add_new_message(const etl::imessage&) [with TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:267:10,
    inlined from ‘etl::message_packet<TMessageTypes>& etl::message_packet<TMessageTypes>::operator=(const etl::message_packet<TMessageTypes>&) [with TMessageTypes = {{anonymous}::Message1, {anonymous}::Message2, {anonymous}::Message3}]’ at ../../test/../include/etl/message_packet.h:160:24,
    inlined from ‘virtual void {anonymous}::Suitetest_message_packet::Testmessage_packet_assignment::RunImpl() const’ at ../../test/test_message_packet.cpp:369:17:
/usr/include/c++/13/bits/basic_string.h:1067:16: error: ‘*(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*)((char*)&packet1 + offsetof(etl::Packet, etl::message_packet<<unnamed>::Message1, <unnamed>::Message2, <unnamed>::Message3>::data.etl::aligned_storage<48, 8>::type::data[8])).std::__cxx11::basic_string<char>::_M_string_length’ may be used uninitialized [-Werror=maybe-uninitialized]
 1067 |       { return _M_string_length; }
      |                ^~~~~~~~~~~~~~~~
../../test/test_message_packet.cpp: In member function ‘virtual void {anonymous}::Suitetest_message_packet::Testmessage_packet_assignment::RunImpl() const’:
../../test/test_message_packet.cpp:366:14: note: ‘packet1’ declared here
  366 |       Packet packet1(message1);
      |              ^~~~~~~

@jwellbelove
Copy link
Contributor

That one is a bit tricky to decipher.
I'm wondering if it is because the move constructors don't move the arguments.
736-e0bbdc9-Add move to test message move constructor and assignment.patch

@dkogan
Copy link
Author

dkogan commented Jul 28, 2023 via email

@jwellbelove
Copy link
Contributor

I don't have GCC 13 on anything here.
I'm running Ubuntu 22.04 on WSL2.
GCC 13 isn't available to install. I tried Debian 12, but it isn't available there either.

@dkogan
Copy link
Author

dkogan commented Jul 28, 2023 via email

@jwellbelove
Copy link
Contributor

debian@John-PC:/etc/apt$ sudo apt install g++-13
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
 binutils-x86-64-linux-gnu : Depends: libgprofng (>= 2.40.90) but it is not installable
E: Unable to correct problems, you have held broken packages.

@jwellbelove
Copy link
Contributor

sources.list

deb http://deb.debian.org/debian bookworm main
deb http://deb.debian.org/debian bookworm-updates main
deb http://security.debian.org/debian-security bookworm-security main
deb http://ftp.debian.org/debian bookworm-backports main
deb http://httpredir.debian.org/debian unstable main contrib non-free

@jwellbelove
Copy link
Contributor

debian@John-PC:~$ sudo apt install build-essential
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
 binutils-x86-64-linux-gnu : Depends: libgprofng (>= 2.40.90) but it is not installable
E: Unable to correct problems, you have held broken packages.

@dkogan
Copy link
Author

dkogan commented Jul 30, 2023 via email

@dkogan
Copy link
Author

dkogan commented Jul 30, 2023 via email

@jwellbelove
Copy link
Contributor

OK, the updates are working now.
I'm able to install GCC-13

@jwellbelove
Copy link
Contributor

jwellbelove commented Jul 30, 2023

I've tried compiling the unit tests for test_message_packet.cpp only for commit 2464e9732fb62fb8dacbca1612eea84d7fcf126d

The CMakeLists.txt contains these compiler flags.

	target_compile_options(etl_tests
			PRIVATE
			-g
			-O2
			-fstack-protector-strong
			-Wformat
			-Werror=format-security
			-Wdate-time
			-D_FORTIFY_SOURCE=2
			-std=gnu++20
			-fno-omit-frame-pointer
			-fno-common
			-Wall
			-Wextra
			-Werror
			)

Running the unit test script for GCC-13 + STL, I get no compilation warnings or errors.

gcc (Debian 13.1.0-9) 13.1.0
-- The CXX compiler identification is GNU 13.1.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Compiling for C++20
-- Compiling for STL
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Configuring done (8.8s)
-- Generating done (0.8s)
-- Build files have been written to: /mnt/d/Users/John/Programming/GitHub/etl/test/build-make
[  7%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/AssertException.cpp.o
[ 11%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/Checks.cpp.o
[ 11%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/CompositeTestReporter.cpp.o
[ 19%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/DeferredTestReporter.cpp.o
[ 19%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/CurrentTest.cpp.o
[ 23%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/DeferredTestResult.cpp.o
[ 26%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/MemoryOutStream.cpp.o
[ 30%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/ReportAssert.cpp.o
[ 34%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/RequiredCheckException.cpp.o
[ 42%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/RequiredCheckTestReporter.cpp.o
[ 42%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/Test.cpp.o
[ 46%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TestDetails.cpp.o
[ 53%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TestList.cpp.o
[ 53%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TestReporter.cpp.o
[ 61%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TestReporterStdout.cpp.o
[ 61%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TestResults.cpp.o
[ 65%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TestRunner.cpp.o
[ 69%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/ThrowingTestReporter.cpp.o
[ 73%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/TimeConstraint.cpp.o
[ 76%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/XmlTestReporter.cpp.o
[ 80%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/Posix/SignalTranslator.cpp.o
[ 84%] Building CXX object UnitTest++/CMakeFiles/UnitTestpp.dir/Posix/TimeHelpers.cpp.o
[ 88%] Linking CXX static library libUnitTestpp.a
[ 88%] Built target UnitTestpp
[ 96%] Building CXX object CMakeFiles/etl_tests.dir/main.cpp.o
[ 96%] Building CXX object CMakeFiles/etl_tests.dir/test_message_packet.cpp.o
[100%] Linking CXX executable etl_tests
[100%] Built target etl_tests

-----------------------------------------------
 Passed Compilation - GCC - STL
-----------------------------------------------

Success: 10 tests passed.
Test time: 0.01 seconds.

-----------------------------------------------
 Passed Tests - GCC - STL
-----------------------------------------------


-----------------------------------------------
 All Tests Completed OK
-----------------------------------------------

Have I missed something?

@dkogan
Copy link
Author

dkogan commented Jul 30, 2023

Can you see what compile commands you're ending up with? The above tells you what you're telling cmake, but not what the compiler gets. VERBOSE=1 make ...

Also, you can paste the compile commands as they appear above in this report to reproduce the specific complaints. If THAT isn't reproducible, we should figure out why.

@dkogan
Copy link
Author

dkogan commented Jul 30, 2023

Actually, I bet I know what's going on. You installed g++-13, but this didn't replace the default: this is still g++, which is still version 12. What does g++ --version say?

You should tell cmake to compile with g++-13

@jwellbelove
Copy link
Contributor

gcc --version
gcc (Debian 13.1.0-9) 13.1.0

g++ --version
g++ (Debian 13.1.0-9) 13.1.0

@jwellbelove
Copy link
Contributor

I get the errors if I use your command line parameters, but not with the cmake route.

@jwellbelove
Copy link
Contributor

If I set the cmake file to 'verbose' mode I get this for the compilation of test_message_packet.cpp.

/usr/bin/g++ 
-DETL_DEBUG 
-I/mnt/d/Users/John/Programming/GitHub/etl/test/../include 
-isystem /mnt/d/Users/John/Programming/GitHub/etl/test/UnitTest++/.. 
-O2 
-std=gnu++20 
-g 
-O2 
-fstack-protector-strong 
-Wformat 
-Werror=format-security 
-Wdate-time 
-D_FORTIFY_SOURCE=2 
-std=gnu++20 
-fno-omit-frame-pointer 
-fno-common 
-Wall 
-Wextra 
-Werror 
-fsanitize=address,undefined,bounds 
-MD 
-MT 
CMakeFiles/etl_tests.dir/test_message_packet.cpp.o 
-MF 
CMakeFiles/etl_tests.dir/test_message_packet.cpp.o.d 
-o 
CMakeFiles/etl_tests.dir/test_message_packet.cpp.o 
-c 
/mnt/d/Users/John/Programming/GitHub/etl/test/test_message_packet.cpp

@dkogan
Copy link
Author

dkogan commented Jul 31, 2023

OK. I suppose you should compare the failing and succeeding commands to see how they differ. What is /usr/bin/g++ --version ?

@jwellbelove
Copy link
Contributor

/usr/bin/g++ --version
g++ (Debian 13.1.0-9) 13.1.0

I apologise that I'm not able to spend a lot of time on this at the moment, as I'm in the middle of organising a major kitchen refit.

@jwellbelove
Copy link
Contributor

I've just tried this on the command line and get no errors!?

g++-13                              \
  -DETL_DEBUG                       \
  -I../include                      \
  -isystem UnitTest++/..            \
  -g                                \
  -O2                               \
  -fstack-protector-strong          \
  -Wformat                          \
  -Werror=format-security           \
  -Wdate-time                       \
  -D_FORTIFY_SOURCE=2               \
  -std=gnu++20                      \
  -fno-omit-frame-pointer           \
  -fno-common                       \
  -Wall                             \
  -Wextra                           \
  -Werror                           \
  -o build/test_bit_stream.cpp.o    \
  -c test_bit_stream.cpp

@jwellbelove
Copy link
Contributor

I am now on gcc (Debian 13.2.0-1) 13.2.0

@dkogan
Copy link
Author

dkogan commented Jul 31, 2023

Hi. Let's clarify who's testing what. I'm on the 20.37.2 tag + some debian patches that shouldn't matter + these two patches from this bug:

  • 736-b04c4da-Possible.fix.patch
  • 736-2464e97-Added.pessimizing-move.warning.disable.for.GCC.patch

Without the patches I see test_bit_stream.cpp fail, but the patches fix it.

test_message_packet.cpp fails with and without the patches.

Above you said that test_bit_stream.cpp is building fine for you. This is witout any of the fixes? What code, exactly are you building?

Similarly, for test_message_packet.cpp you're saying you see successful builds or not? If you ARE seeing successful builds, which code are you looking at?

Thanks

@jwellbelove
Copy link
Contributor

OK, I copied the wrong unit test file name from the previous posts (I'm hopping back an forth between the kitchen refit and Github)
I meant to test test_message_packet.cpp.
When I do, I get the error.

When I use the ETL's unit test CMakeLists.txt, I don't

@jwellbelove
Copy link
Contributor

This compiles with the error.

g++-13                              \
  -DETL_DEBUG                       \
  -I../include                      \
  -isystem UnitTest++/..            \
  -g                                \
  -O2                               \
  -fstack-protector-strong          \
  -Wformat                          \
  -Werror=format-security           \
  -Wdate-time                       \
  -D_FORTIFY_SOURCE=2               \
  -std=gnu++20                      \
  -fno-omit-frame-pointer           \
  -fno-common                       \
  -Wall                             \
  -Wextra                           \
  -Werror                           \
  -o build/test_message_packet.cpp.o    \
  -c test_message_packet.cpp

This doesn't

g++-13 \
	-DETL_DEBUG \
	-I../include                      \
	-isystem UnitTest++/..            \
	-g \
	-O2 \
	-fstack-protector-strong \
	-Wformat \
	-Werror=format-security \
	-Wdate-time \
	-D_FORTIFY_SOURCE=2 \
	-std=gnu++20 \
	-fno-omit-frame-pointer \
	-fno-common \
	-Wall \
	-Wextra \
	-Werror \
	-fsanitize=address,undefined,bounds \
	-o build/test_message_packet.cpp.o    \
	-c test_message_packet.cpp

The only difference appears to be this line...
-fsanitize=address,undefined,bounds

@dkogan
Copy link
Author

dkogan commented Aug 3, 2023

Hello. I see the failures both ways. I'm at the 20.37.2 tag, which probably isn't where you are. And you're saying that with the sanitizers you see no warnings at all? I don't know enough about the sanitizers to know why that would be the case, but it's an issue for your build regardless: the sanitizers are extra diagnostic instrumentation, and any canonical build doesn't use them. I disabled these for Debian because turning them on can make the build fail due to external factors (this was discussed in an earlier etlcpp bug report).

@jwellbelove
Copy link
Contributor

Looks like I'm going to have to run my CI with optional inclusion of the sanitizers.
Some users of the ETL do run them as a safety check, and I've had bug reports where valid issues have been highlighted.

I'm on 20.37.2 + the patches that I sent you.

@jwellbelove jwellbelove self-assigned this Aug 3, 2023
@jwellbelove jwellbelove added the bug label Aug 3, 2023
@jwellbelove
Copy link
Contributor

I've got rid of most of the warnings/errors, but I'm currently stuck with a -Werror=array-bounds for __builtin_memmove(__result - _Num, __first, sizeof(_Tp) * _Num); reported by GCC-13 for string insert.

offset [-22, -5] is out of the bounds [0, 24] of object ‘buffer’ with type ‘{anonymous}::Suitetest_string_char::TextBuffer’ {aka ‘std::array<char16_t, 12>

test_string_char16_t_external_buffer::test_insert_size_t_position_string

This only occurs when the STL std::copy_backward is used.
The ETL's non-STL loop implementation is fine, as is a direct call of __builtin_memmove.

At the moment I can't see how GCC's std::copy_backward is coming up with offsets of -22 and -5.

@dkogan
Copy link
Author

dkogan commented Aug 8, 2023

Thanks for looking at this. I don't know anything about your specific case, but as with any software, gcc has bugs. It's certainly possible that you stumbled on one. If so, reporting it would be good. You should run the preprocessor, and minimize the reproducing program (something like creduce does this well) to the bare minimum. And then you can submit the report here: https://gcc.gnu.org/bugs/

And if it's not really a bug, then trying to isolate it should clarify exactly why it's complaining.

Thanks

@jwellbelove jwellbelove removed the bug label Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants