Page MenuHomePhabricator

[build] Make all warnings into errors with a whitelist
AbandonedPublic

Authored by majcosta on May 19 2020, 00:37.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

Currently our ENABLE_WERROR build promotes a few warnings to errors,
this change enables us to use the more restrictive Werror options while
retaining granularity to preserve warnings as needed

Test Plan

remove whitelisted warnings from CMakeLists.txt file

ninja

see build fail due to warnings promoted to errors

Diff Detail

Event Timeline

Snippet of first build failure:

[00:38:21] :	 [Step 1/2] -- Check size of __int128
[00:38:21] :	 [Step 1/2] -- Check size of __int128 - done
[00:38:21] :	 [Step 1/2] -- Found OpenSSL: /usr/lib/x86_64-linux-gnu/libcrypto.so (found version "1.1.1d") found components:  Crypto 
[00:38:22] :	 [Step 1/2] -- Boost version: 1.67.0
[00:38:22] :	 [Step 1/2] -- Found the following Boost libraries:
[00:38:22] :	 [Step 1/2] --   chrono
[00:38:22] :	 [Step 1/2] --   filesystem
[00:38:22] :	 [Step 1/2] --   thread
[00:38:22] :	 [Step 1/2] --   system
[00:38:22] :	 [Step 1/2] --   date_time
[00:38:22] :	 [Step 1/2] --   atomic
[00:38:22] :	 [Step 1/2] -- Found PkgConfig: /usr/bin/pkg-config (found version "0.29") 
[00:38:22] :	 [Step 1/2] -- Found Event component event: /usr/lib/x86_64-linux-gnu/libevent.so
[00:38:22] :	 [Step 1/2] -- Found Event: /usr/include (found suitable version "2.1.8-stable", minimum required is "2.0.22") found components:  event 
[00:38:22] :	 [Step 1/2] -- Found Event component pthreads: /usr/lib/x86_64-linux-gnu/libevent_pthreads.so
[00:38:22] :	 [Step 1/2] -- Found Event: /usr/include (found suitable version "2.1.8-stable", minimum required is "2.0.22") found components:  pthreads 
[00:38:22] :	 [Step 1/2] -- Boost version: 1.67.0
[00:38:22] :	 [Step 1/2] -- Found the following Boost libraries:
[00:38:22] :	 [Step 1/2] --   unit_test_framework
[00:38:22] :	 [Step 1/2] -- Performing Test BOOST_TEST_DYN_LINK
[00:38:22] :	 [Step 1/2] -- Performing Test BOOST_TEST_DYN_LINK - Success
[00:38:22] :	 [Step 1/2] -- Boost version: 1.67.0
[00:38:22] :	 [Step 1/2] -- Found the following Boost libraries:
[00:38:22] :	 [Step 1/2] --   unit_test_framework
[00:38:22] :	 [Step 1/2] -- Configuring done
[00:38:23] :	 [Step 1/2] -- Generating done
[00:38:23] :	 [Step 1/2] -- Build files have been written to: /work/build/native
[00:38:23] :	 [Step 1/2] [6/455] Building CXX object src/crypto/CMakeFiles/crypto_shani.dir/sha256_shani.cpp.o
[00:38:23] :	 [Step 1/2] FAILED: src/crypto/CMakeFiles/crypto_shani.dir/sha256_shani.cpp.o 
[00:38:23] :	 [Step 1/2] /usr/bin/ccache /usr/bin/clang++  -DENABLE_SHANI -I../src/crypto/.. -g -O2 -fPIC -fvisibility=hidden   -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wvla -Wformat-security -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety-analysis -Wshadow -Wrange-loop-analysis -Wredundant-decls -Wredundant-move -Wno-unused-parameter -Wno-implicit-fallthrough -Werror -Wno-error=deprecated-declarations -msse4 -msha -std=gnu++14 -MD -MT src/crypto/CMakeFiles/crypto_shani.dir/sha256_shani.cpp.o -MF src/crypto/CMakeFiles/crypto_shani.dir/sha256_shani.cpp.o.d -o src/crypto/CMakeFiles/crypto_shani.dir/sha256_shani.cpp.o -c ../src/crypto/sha256_shani.cpp
[00:38:23] :	 [Step 1/2] ../src/crypto/sha256_shani.cpp:74:45: error: cast from 'const uint8_t *' (aka 'const unsigned char *') to 'const __m128i *' increases required alignment from 1 to 16 [-Werror,-Wcast-align]
[00:38:23] :	 [Step 1/2]     return _mm_shuffle_epi8(_mm_loadu_si128((const __m128i *)in), MASK);
[00:38:23] :	 [Step 1/2]                                             ^~~~~~~~~~~~~~~~~~~
[00:38:23] :	 [Step 1/2] ../src/crypto/sha256_shani.cpp:78:22: error: cast from 'uint8_t *' (aka 'unsigned char *') to '__m128i *' increases required alignment from 1 to 16 [-Werror,-Wcast-align]
[00:38:23] :	 [Step 1/2]     _mm_storeu_si128((__m128i *)out, _mm_shuffle_epi8(s, MASK));
[00:38:23] :	 [Step 1/2]                      ^~~~~~~~~~~~~~
[00:38:23] :	 [Step 1/2] ../src/crypto/sha256_shani.cpp:87:26: error: cast from 'uint32_t *' (aka 'unsigned int *') to 'const __m128i *' increases required alignment from 4 to 16 [-Werror,-Wcast-align]
[00:38:23] :	 [Step 1/2]     s0 = _mm_loadu_si128((const __m128i *)s);
[00:38:23] :	 [Step 1/2]                          ^~~~~~~~~~~~~~~~~~
[00:38:23] :	 [Step 1/2] ../src/crypto/sha256_shani.cpp:88:26: error: cast from 'uint32_t *' (aka 'unsigned int *') to 'const __m128i *' increases required alignment from 4 to 16 [-Werror,-Wcast-align]
[00:38:23] :	 [Step 1/2]     s1 = _mm_loadu_si128((const __m128i *)(s + 4));
[00:38:23] :	 [Step 1/2]                          ^~~~~~~~~~~~~~~~~~~~~~~~
[00:38:23] :	 [Step 1/2] ../src/crypto/sha256_shani.cpp:141:22: error: cast from 'uint32_t *' (aka 'unsigned int *') to '__m128i *' increases required alignment from 4 to 16 [-Werror,-Wcast-align]
[00:38:23] :	 [Step 1/2]     _mm_storeu_si128((__m128i *)s, s0);
[00:38:23] :	 [Step 1/2]                      ^~~~~~~~~~~~
[00:38:23]W:	 [Step 1/2] ++ print_sanitizers_log
[00:38:23] :	 [Step 1/2] ../src/crypto/sha256_shani.cpp:142:22: error: cast from 'uint32_t *' (aka 'unsigned int *') to '__m128i *' increases required alignment from 4 to 16 [-Werror,-Wcast-align]
[00:38:23]W:	 [Step 1/2] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[00:38:23] :	 [Step 1/2]     _mm_storeu_si128((__m128i *)(s + 4), s1);
[00:38:23] :	 [Step 1/2]                      ^~~~~~~~~~~~~~~~~~
[00:38:23]W:	 [Step 1/2] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[00:38:23] :	 [Step 1/2] 6 errors generated.
[00:38:23] :	 [Step 1/2] [18/455] Building CXX object src/crypto/CMakeFiles/crypto_sse4.1.dir/sha256_sse41.cpp.o
[00:38:23]W:	 [Step 1/2] ++ cat '/tmp/sanitizer_logs/*.log.*'
[00:38:23] :	 [Step 1/2] ninja: build stopped: subcommand failed.
[00:38:23]W:	 [Step 1/2] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[00:38:23] :	 [Step 1/2] *** Output of /tmp/sanitizer_logs/*.log.* ***
[00:38:23]W:	 [Step 1/2] cp: cannot stat '/work/ibd/debug.log': No such file or directory
[00:38:27]W:	 [Step 1/2] Process exited with code 1
[00:38:27]E:	 [Step 1/2] Process exited with code 1 (Step: Command Line)

well that is embarrassing...

fixed clang build, added conditional to prevent __FORTIFY_SOURCE warnings in '-O0' builds

deadalnix requested changes to this revision.May 19 2020, 17:15
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/CMakeLists.txt
138

You will need to explain what the problem is an how that make sense, because from where I'm sitting, it doesn't.

This revision now requires changes to proceed.May 19 2020, 17:15
majcosta added inline comments.
src/CMakeLists.txt
138

enabling these flags without optimization (as is the case of our Debug configuration that uses -O0 rather than -Og raises a warning, that gets promoted to error under WERROR_ENABLE.

so the intent of that change is to disable _FORTIFY_SOURCE only for Debug builds. however, @Fabien has brought to my attention that the method above isn't good since it doesn't work with all generators, so I'm working on something that's correct

copying previous inline comment that I suspect is about to be deleted:

enabling these flags without optimization (as is the case of our Debug configuration that uses -O0 rather than -Og raises a warning, that gets promoted to error under WERROR_ENABLE.

so the intent of that change is to disable _FORTIFY_SOURCE only for Debug builds. however, @Fabien has brought to my attention that the method above isn't good since it doesn't work with all generators, so I'm working on something that's correct

this method should be robust to detecting build configuration and only exclude _FORTIFY_SOURCE=2 from the Debug build h/t @Fabien

removed a spurious newline I added to CMakeLists.txt