Page MenuHomePhabricator
Differential D10113

build: Enable some commonly enabled compiler diagnostics
ClosedPublic
Actions

Authored by PiRK on Sep 14 2021, 14:47.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC5c2868f3d51d: build: Enable some commonly enabled compiler diagnostics
Summary

as discussed in #17344

There is a large overlap between this list and Jason Turner's list of recommended compiler diagnostics in the Collaborative Collection of C++ Best Practices (cppbestpractices) project. There is also an overlap with the recommendations given in the C++ Core Guidelines (with editors Bjarne Stroustrup and Herb Sutter).

This is a backport of core#19015

Disable -Wduplicated-branches for secp256k1, because of some exotic intentional use of ternary operator duplicated branches in test_inverse_scalar.

Test Plan
cmake .. -GNinja -DCMAKE_CXX_FLAGS=-Werror -DCMAKE_C_FLAGS=-Werror
ninja && ninja check && ninja check-secp256k1
rm -Rf *
cmake .. -GNinja  -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS=-Werror  -DCMAKE_C_FLAGS=-Werror
ninja && ninja check && ninja check-secp256k1

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK created this revision.Sep 14 2021, 14:47
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 14 2021, 14:47
PiRK requested review of this revision.Sep 14 2021, 14:47
teamcity edited the summary of this revision. (Show Details)Sep 14 2021, 14:47
PiRK edited the summary of this revision. (Show Details)Sep 14 2021, 14:48
Harbormaster completed remote builds in B16644: Diff 29948.Sep 14 2021, 14:55
teamcity added a comment.Sep 14 2021, 14:55
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Tail of the build log:

wallet_groups.py                        | ○ Skipped | 0 s
wallet_hd.py                            | ○ Skipped | 0 s
wallet_import_rescan.py                 | ○ Skipped | 0 s
wallet_import_with_label.py             | ○ Skipped | 0 s
wallet_importdescriptors.py             | ○ Skipped | 0 s
wallet_importmulti.py                   | ○ Skipped | 0 s
wallet_importprunedfunds.py             | ○ Skipped | 0 s
wallet_keypool.py                       | ○ Skipped | 0 s
wallet_keypool_topup.py                 | ○ Skipped | 0 s
wallet_labels.py                        | ○ Skipped | 0 s
wallet_listreceivedby.py                | ○ Skipped | 0 s
wallet_listsinceblock.py                | ○ Skipped | 0 s
wallet_listtransactions.py              | ○ Skipped | 0 s
wallet_multiwallet.py --usecli          | ○ Skipped | 0 s
wallet_reorgsrestore.py                 | ○ Skipped | 0 s
wallet_resendwallettransactions.py      | ○ Skipped | 0 s
wallet_txn_clone.py                     | ○ Skipped | 0 s
wallet_txn_clone.py --mineblock         | ○ Skipped | 0 s
wallet_txn_doublespend.py               | ○ Skipped | 0 s
wallet_txn_doublespend.py --mineblock   | ○ Skipped | 0 s
wallet_watchonly.py                     | ○ Skipped | 0 s
wallet_watchonly.py --usecli            | ○ Skipped | 0 s
wallet_zapwallettxes.py                 | ○ Skipped | 0 s

ALL                                     | ✓ Passed  | 447 s (accumulated) 
Runtime: 90 s

----------------------------------------------------------------------
Ran 5 tests in 0.003s

OK

[162/413] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

OK
[163/413] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[171/413] Running pow test suite
PASSED: pow test suite
[182/413] Running seeder test suite
PASSED: seeder test suite
[188/413] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[189/413] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from /usr/include/boost/test/unit_test.hpp:19,
                 from ../../src/test/script_tests.cpp:30:
../../src/test/script_tests.cpp: In member function ‘void script_tests::script_build::test_method()’:
../../src/test/script_tests.cpp:540:22: note: variable tracking size limit exceeded with -fvar-tracking-assignments, retrying without
 BOOST_AUTO_TEST_CASE(script_build) {
                      ^~~~~~~~~~~~
[190/413] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util_tests.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1
teamcity added a comment.Sep 14 2021, 14:57
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

wallet_createwallet.py                  | ✓ Passed  | 3 s
wallet_createwallet.py --usecli         | ✓ Passed  | 4 s
wallet_descriptor.py                    | ✓ Passed  | 9 s
wallet_disable.py                       | ✓ Passed  | 1 s
wallet_dump.py                          | ✓ Passed  | 6 s
wallet_encryption.py                    | ✓ Passed  | 5 s
wallet_groups.py                        | ✓ Passed  | 85 s
wallet_hd.py                            | ✓ Passed  | 9 s
wallet_import_rescan.py                 | ✓ Passed  | 10 s
wallet_import_with_label.py             | ✓ Passed  | 1 s
wallet_importdescriptors.py             | ✓ Passed  | 6 s
wallet_importmulti.py                   | ✓ Passed  | 6 s
wallet_importprunedfunds.py             | ✓ Passed  | 2 s
wallet_keypool.py                       | ✓ Passed  | 3 s
wallet_keypool_topup.py                 | ✓ Passed  | 4 s
wallet_labels.py                        | ✓ Passed  | 2 s
wallet_listreceivedby.py                | ✓ Passed  | 30 s
wallet_listsinceblock.py                | ✓ Passed  | 5 s
wallet_listtransactions.py              | ✓ Passed  | 12 s
wallet_multiwallet.py --usecli          | ✓ Passed  | 18 s
wallet_reorgsrestore.py                 | ✓ Passed  | 4 s
wallet_resendwallettransactions.py      | ✓ Passed  | 3 s
wallet_txn_clone.py                     | ✓ Passed  | 3 s
wallet_txn_clone.py --mineblock         | ✓ Passed  | 4 s
wallet_txn_doublespend.py               | ✓ Passed  | 3 s
wallet_txn_doublespend.py --mineblock   | ✓ Passed  | 4 s
wallet_watchonly.py                     | ✓ Passed  | 1 s
wallet_watchonly.py --usecli            | ✓ Passed  | 1 s
wallet_zapwallettxes.py                 | ✓ Passed  | 6 s

ALL                                     | ✓ Passed  | 1376 s (accumulated) 
Runtime: 276 s

----------------------------------------------------------------------
Ran 5 tests in 0.003s

OK

[170/453] Running avalanche test suite
PASSED: avalanche test suite
[190/453] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.004s

OK
[191/453] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[194/453] Running seeder test suite
PASSED: seeder test suite
[206/453] Running pow test suite
PASSED: pow test suite
[209/453] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1
teamcity added a comment.Sep 14 2021, 15:00
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

rpc_preciousblock.py                    | ✓ Passed  | 1 s
rpc_psbt.py                             | ✓ Passed  | 36 s
rpc_rawtransaction.py                   | ✓ Passed  | 13 s
rpc_scantxoutset.py                     | ✓ Passed  | 3 s
rpc_setban.py                           | ✓ Passed  | 2 s
rpc_signmessage.py                      | ✓ Passed  | 1 s
rpc_signrawtransaction.py               | ✓ Passed  | 1 s
rpc_txoutproof.py                       | ✓ Passed  | 2 s
rpc_uptime.py                           | ✓ Passed  | 1 s
rpc_users.py                            | ✓ Passed  | 5 s
rpc_whitelist.py                        | ✓ Passed  | 1 s
tool_wallet.py                          | ✓ Passed  | 4 s
wallet_abandonconflict.py               | ✓ Passed  | 5 s
wallet_address_types.py                 | ✓ Passed  | 13 s
wallet_avoidreuse.py                    | ✓ Passed  | 4 s
wallet_backup.py                        | ✓ Passed  | 22 s
wallet_balance.py                       | ✓ Passed  | 11 s
wallet_basic.py                         | ✓ Passed  | 23 s
wallet_coinbase_category.py             | ✓ Passed  | 1 s
wallet_create_tx.py                     | ✓ Passed  | 5 s
wallet_createwallet.py                  | ✓ Passed  | 2 s
wallet_createwallet.py --usecli         | ✓ Passed  | 3 s
wallet_descriptor.py                    | ✓ Passed  | 6 s
wallet_disable.py                       | ✓ Passed  | 1 s
wallet_dump.py                          | ✓ Passed  | 5 s
wallet_encryption.py                    | ✓ Passed  | 5 s
wallet_groups.py                        | ✓ Passed  | 54 s
wallet_hd.py                            | ✓ Passed  | 6 s
wallet_import_rescan.py                 | ✓ Passed  | 4 s
wallet_import_with_label.py             | ✓ Passed  | 1 s
wallet_importdescriptors.py             | ✓ Passed  | 4 s
wallet_importmulti.py                   | ✓ Passed  | 3 s
wallet_importprunedfunds.py             | ✓ Passed  | 2 s
wallet_keypool.py                       | ✓ Passed  | 3 s
wallet_keypool_topup.py                 | ✓ Passed  | 2 s
wallet_labels.py                        | ✓ Passed  | 1 s
wallet_listreceivedby.py                | ✓ Passed  | 16 s
wallet_listsinceblock.py                | ✓ Passed  | 3 s
wallet_listtransactions.py              | ✓ Passed  | 6 s
wallet_multiwallet.py --usecli          | ✓ Passed  | 11 s
wallet_reorgsrestore.py                 | ✓ Passed  | 3 s
wallet_resendwallettransactions.py      | ✓ Passed  | 1 s
wallet_txn_clone.py                     | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock         | ✓ Passed  | 3 s
wallet_txn_doublespend.py               | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock   | ✓ Passed  | 3 s
wallet_watchonly.py                     | ✓ Passed  | 1 s
wallet_watchonly.py --usecli            | ✓ Passed  | 1 s
wallet_zapwallettxes.py                 | ✓ Passed  | 3 s

ALL                                     | ✓ Passed  | 903 s (accumulated) 
Runtime: 181 s

----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
PiRK planned changes to this revision.Sep 14 2021, 15:32

Fixes required related to -Woverloaded-virtual, and test plan must be updated to better exercise the new flags.

PiRK updated this revision to Diff 29952.Sep 14 2021, 16:33
PiRK edited the test plan for this revision. (Show Details)

suppress a [-Werror=overloaded-virtual] in rpc_server_tests.cpp, and edit the Test plan to enable -Werror on compilation.

Harbormaster completed remote builds in B16648: Diff 29952.Sep 14 2021, 16:40
teamcity added a comment.Sep 14 2021, 16:41
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

wallet_listreceivedby.py                | ✓ Passed  | 16 s
wallet_listsinceblock.py                | ✓ Passed  | 6 s
wallet_listtransactions.py              | ✓ Passed  | 19 s
wallet_multiwallet.py --usecli          | ✓ Passed  | 17 s
wallet_reorgsrestore.py                 | ✓ Passed  | 4 s
wallet_resendwallettransactions.py      | ✓ Passed  | 2 s
wallet_txn_clone.py                     | ✓ Passed  | 3 s
wallet_txn_clone.py --mineblock         | ✓ Passed  | 4 s
wallet_txn_doublespend.py               | ✓ Passed  | 3 s
wallet_txn_doublespend.py --mineblock   | ✓ Passed  | 4 s
wallet_watchonly.py                     | ✓ Passed  | 2 s
wallet_watchonly.py --usecli            | ✓ Passed  | 2 s
wallet_zapwallettxes.py                 | ✓ Passed  | 7 s

ALL                                     | ✓ Passed  | 1279 s (accumulated) 
Runtime: 256 s

----------------------------------------------------------------------
Ran 5 tests in 0.003s

OK

[29/453] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.004s

OK
[31/453] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[186/453] Running seeder test suite
PASSED: seeder test suite
[191/453] Building C object src/secp256k1/CMakeFiles/secp256k1-tests.dir/src/tests.c.o
FAILED: src/secp256k1/CMakeFiles/secp256k1-tests.dir/src/tests.c.o 
/usr/bin/ccache /usr/bin/cc -DABORT_ON_FAILED_ASSUME -DDEBUG -DDEBUG_LOCKORDER -DHAVE_CONFIG_H -DSECP256K1_BUILD -DVERIFY -I../../src/secp256k1/. -I../../src/secp256k1/src -Isrc/secp256k1/src -I../../src/secp256k1/include -isystem /usr/include/jemalloc -Werror -O2 -fPIE -fvisibility=hidden   -g3 -ftrapv -fstack-reuse=none -U_FORTIFY_SOURCE -Wnested-externs -Wstrict-prototypes -Wall -Wextra -Wformat -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wredundant-decls -Wsign-compare -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Wformat-security -Wno-unused-parameter -Wno-implicit-fallthrough -pedantic -Wshadow -Wundef -Wno-unused-function -Wno-overlength-strings -std=c89 -Wno-long-long -pthread -MD -MT src/secp256k1/CMakeFiles/secp256k1-tests.dir/src/tests.c.o -MF src/secp256k1/CMakeFiles/secp256k1-tests.dir/src/tests.c.o.d -o src/secp256k1/CMakeFiles/secp256k1-tests.dir/src/tests.c.o   -c ../../src/secp256k1/src/tests.c
../../src/secp256k1/src/tests.c: In function ‘test_inverse_scalar’:
../../src/secp256k1/src/tests.c:2122:41: error: this condition has identical branches [-Werror=duplicated-branches]
     (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&l, x);  /* l = 1/x */
                                         ^
../../src/secp256k1/src/tests.c:2132:41: error: this condition has identical branches [-Werror=duplicated-branches]
     (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&r, &r); /* r = 1/(x-1) */
                                         ^
../../src/secp256k1/src/tests.c:2134:41: error: this condition has identical branches [-Werror=duplicated-branches]
     (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&l, &l); /* l = 1/(1/x-1) */
                                         ^
cc1: all warnings being treated as errors
[203/453] Running avalanche test suite
PASSED: avalanche test suite
[423/453] Running pow test suite
PASSED: pow test suite
[425/453] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[449/453] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1
teamcity added a comment.Sep 14 2021, 16:47
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

rpc_preciousblock.py                    | ✓ Passed  | 1 s
rpc_psbt.py                             | ✓ Passed  | 19 s
rpc_rawtransaction.py                   | ✓ Passed  | 14 s
rpc_scantxoutset.py                     | ✓ Passed  | 4 s
rpc_setban.py                           | ✓ Passed  | 2 s
rpc_signmessage.py                      | ✓ Passed  | 1 s
rpc_signrawtransaction.py               | ✓ Passed  | 1 s
rpc_txoutproof.py                       | ✓ Passed  | 2 s
rpc_uptime.py                           | ✓ Passed  | 1 s
rpc_users.py                            | ✓ Passed  | 5 s
rpc_whitelist.py                        | ✓ Passed  | 1 s
tool_wallet.py                          | ✓ Passed  | 4 s
wallet_abandonconflict.py               | ✓ Passed  | 14 s
wallet_address_types.py                 | ✓ Passed  | 16 s
wallet_avoidreuse.py                    | ✓ Passed  | 4 s
wallet_backup.py                        | ✓ Passed  | 33 s
wallet_balance.py                       | ✓ Passed  | 12 s
wallet_basic.py                         | ✓ Passed  | 28 s
wallet_coinbase_category.py             | ✓ Passed  | 1 s
wallet_create_tx.py                     | ✓ Passed  | 5 s
wallet_createwallet.py                  | ✓ Passed  | 2 s
wallet_createwallet.py --usecli         | ✓ Passed  | 3 s
wallet_descriptor.py                    | ✓ Passed  | 7 s
wallet_disable.py                       | ✓ Passed  | 1 s
wallet_dump.py                          | ✓ Passed  | 4 s
wallet_encryption.py                    | ✓ Passed  | 5 s
wallet_groups.py                        | ✓ Passed  | 53 s
wallet_hd.py                            | ✓ Passed  | 6 s
wallet_import_rescan.py                 | ✓ Passed  | 4 s
wallet_import_with_label.py             | ✓ Passed  | 1 s
wallet_importdescriptors.py             | ✓ Passed  | 5 s
wallet_importmulti.py                   | ✓ Passed  | 3 s
wallet_importprunedfunds.py             | ✓ Passed  | 2 s
wallet_keypool.py                       | ✓ Passed  | 3 s
wallet_keypool_topup.py                 | ✓ Passed  | 2 s
wallet_labels.py                        | ✓ Passed  | 2 s
wallet_listreceivedby.py                | ✓ Passed  | 14 s
wallet_listsinceblock.py                | ✓ Passed  | 3 s
wallet_listtransactions.py              | ✓ Passed  | 11 s
wallet_multiwallet.py --usecli          | ✓ Passed  | 15 s
wallet_reorgsrestore.py                 | ✓ Passed  | 3 s
wallet_resendwallettransactions.py      | ✓ Passed  | 8 s
wallet_txn_clone.py                     | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock         | ✓ Passed  | 3 s
wallet_txn_doublespend.py               | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock   | ✓ Passed  | 3 s
wallet_watchonly.py                     | ✓ Passed  | 1 s
wallet_watchonly.py --usecli            | ✓ Passed  | 1 s
wallet_zapwallettxes.py                 | ✓ Passed  | 4 s

ALL                                     | ✓ Passed  | 996 s (accumulated) 
Runtime: 199 s

----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
PiRK updated this revision to Diff 29957.Sep 14 2021, 18:47
PiRK edited the summary of this revision. (Show Details)
PiRK edited the test plan for this revision. (Show Details)

Disable -Wno-duplicated-branches for secp256k1, update the description and test plan accordingly.

Harbormaster completed remote builds in B16649: Diff 29957.Sep 14 2021, 19:03
Fabien added a subscriber: Fabien.Sep 15 2021, 05:58

@bot build-fuzzer

teamcity awarded a token.Sep 15 2021, 05:59
Fabien accepted this revision.Sep 15 2021, 06:14
This revision is now accepted and ready to land.Sep 15 2021, 06:14
Closed by commit rABC5c2868f3d51d: build: Enable some commonly enabled compiler diagnostics (authored by PiRK). · Explain WhySep 15 2021, 06:53
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC5c2868f3d51d: build: Enable some commonly enabled compiler diagnostics.

Revision Contents
Changeset List

PathSize
src/
4 lines
secp256k1/
1 line
test/
3 lines

Diff 29964

View Options

src/CMakeLists.txt

Loading...
View Options

src/secp256k1/CMakeLists.txt

Loading...
View Options

src/test/rpc_server_tests.cpp

Loading...