Skip to content

Commit 26f438f

Browse files
Merge dashpay#6637: build: merge bitcoin#28092, bitcoin#28999, bitcoin#27872, bitcoin#29486, bitcoin#25972 (ensure WARN_CXXFLAGS are populated to ensure expected --enable-werror behavior)
af14f23 ci: don't build using `-Werror` when using Clang (Kittywhiskers Van Gogh) 3b837c8 ci: don't build using `-Werror` when using GCC (Kittywhiskers Van Gogh) 04e036e revert: make fuzzing builds stricter by enabling -Werror by default (Kittywhiskers Van Gogh) 29090a0 merge bitcoin#25972: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set (Kittywhiskers Van Gogh) d0548f8 merge bitcoin#29486: remove -Wdocumentation conditional (Kittywhiskers Van Gogh) 0f4812f merge bitcoin#27872: suppress external warnings by default (Kittywhiskers Van Gogh) 7100780 merge bitcoin#28999: Enable -Wunreachable-code (Kittywhiskers Van Gogh) 5471c58 merge bitcoin#28092: document that -Wreturn-type has been fixed upstream (mingw-w64) (Kittywhiskers Van Gogh) 9098c9c build: always enable -Wsuggest-override (Kittywhiskers Van Gogh) Pull request description: ## Motivation While working on [dash#6633](dashpay#6633), I had built `develop` (5e4a892) but the build _failed_ locally due to a `-Wthread-safety` warning (see below) that was introduced by [bitcoin#25337](bitcoin#25337) ([commit](dashpay@a7d4127)). It was caught because I use additional `CXXFLAGS` on my local system but it _should_ have been caught by CI, especially since [bitcoin#20182](bitcoin#20182) ([commit](dashpay@14a67ee)) made `--enable-werror` the default for CI and the sole exception (the Windows build) was remedied with [bitcoin#20586](bitcoin#20586) ([commit](dashpay@750447e)), so every build variant should've run with `-Werror`. <details> <summary>Compile error:</summary> ``` CXX wallet/libbitcoin_wallet_a-sqlite.o wallet/rpcwallet.cpp:1038:36: error: calling function 'IsMine' requires holding mutex 'wallet.cs_wallet' exclusively [-Werror,-Wthread-safety-analysis] 1038 | isminefilter mine = wallet.IsMine(address); | ^ ``` </details> But that didn't happen. Till [bitcoin#23149](dashpay@70ed6b4), there were a separate set of warnings (overridable by `CXXFLAGS`) and errors (overridable by `CXXFLAG_WERROR`). _Before_ the backport, coverage was as expected ([build](https://gitlab.com/dashpay/dash/-/jobs/9221165750#L786), search for `-Werror=thread-safety`) but _after_ the backport, `thread-safety` (and other expected warnings) were no longer being evaluated ([build](https://gitlab.com/dashpay/dash/-/jobs/9238308844#L740), search for `-Werror` and `-Wthread-safety`, only the former is present). Expected `CXXFLAGS`: ``` CXXFLAGS = -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=. -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wreorder -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wunreachable-code-loop-increment -Wimplicit-fallthrough -Wno-unused-parameter -Wno-self-assign -Werror -pipe -std=c++20 -O2 -O0 -g0 ``` Actual `CXXFLAGS`: ``` CXXFLAGS = -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=. -Werror -pipe -std=c++20 -O2 -O0 -g0 ``` This happened because `CXXFLAGS` are overridden by default which results in none of the warnings making it to the final `CXXFLAGS`, which reduced the effectiveness of `-Werror` substantially (while `CXXFLAG_WERROR` was left undisturbed, which allowed pre-backport builds to include coverage). This is remedied by backporting [bitcoin#25972](bitcoin#25972) (done by this PR), which will ensure that `WARN_CXXFLAGS` are included _even if `CXXFLAGS` are overridden_ and is possible because [bitcoin#24391](bitcoin#24391) ([commit](dashpay@11323c3)) is already in `develop`. ## Additional Information * Dependency for dashpay#6638 * Dependency for dashpay#6639 * Because the warnings (converted to errors with `-Werror`) cast a wider net than the older set of error flags, `develop` in its current state does not compile. To allow CI to bless this PR, `-Werror` for both Clang and GCC-based builds have been **tentatively** disabled and will be re-enabled by the dependent PRs listed above. It is recommended to read the pull request description for both dependents while reviewing this PR. * `-Wsuggest-override` was made unconditional in [bitcoin#28348](bitcoin#28348) ([commit](dashpay@c71e3df)) **but** there were two such conditional checks, they were deduplicated in [bitcoin#23149](bitcoin#23149) but the former was merged before the latter (i.e. out-of-order) and one conditional check lingered around. This lingering check has been removed as we don't support GCC 9.2. * `CXXFLAGS` set for the fuzz build ([commit](dashpay@184bd60)) that enabled `-Werror` are made redundant with [bitcoin#20182](bitcoin#20182) and therefore, have been removed. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK af14f23 Tree-SHA512: ccdaf71cf79eb3aec2468c4c1eaa696cd120c03e9665a3c4b56da6ef17cca9585ef8c66ac1625f2ba243c7f80f15e92a336c0bd90b5f11969fabb3adde3c8125
2 parents e053c8b + af14f23 commit 26f438f

16 files changed

+60
-61
lines changed

ci/dash/build_src.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ if [ -n "$CONFIG_SHELL" ]; then
3333
export CONFIG_SHELL="$CONFIG_SHELL"
3434
fi
3535

36-
BITCOIN_CONFIG_ALL="--enable-suppress-external-warnings --disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib"
36+
BITCOIN_CONFIG_ALL="--disable-dependency-tracking --prefix=$DEPENDS_DIR/$HOST --bindir=$BASE_OUTDIR/bin --libdir=$BASE_OUTDIR/lib"
3737
if [ -z "$NO_WERROR" ]; then
3838
BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} --enable-werror"
3939
fi

ci/test/00_setup_env_arm.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,5 @@ export RUN_FUNCTIONAL_TESTS=false
2525
export GOAL="install"
2626
# -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1"
2727
# This could be removed once the ABI change warning does not show up by default
28-
export BITCOIN_CONFIG="--enable-reduce-exports --enable-suppress-external-warnings CXXFLAGS=-Wno-psabi --with-boost-process"
28+
export BITCOIN_CONFIG="--enable-reduce-exports CXXFLAGS=-Wno-psabi --with-boost-process"
29+
export NO_WERROR=1

ci/test/00_setup_env_mac.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ export RUN_UNIT_TESTS=false
1515
export RUN_FUNCTIONAL_TESTS=false
1616
export GOAL="all deploy"
1717
export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --disable-miner --with-boost-process"
18+
export NO_WERROR=1

ci/test/00_setup_env_native_fuzz.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ export CONTAINER_NAME=ci_native_fuzz
1010
export PACKAGES="clang llvm python3 libevent-dev bsdmainutils libboost-dev"
1111
export DEP_OPTS="NO_UPNP=1 DEBUG=1"
1212
export CPPFLAGS="-DDEBUG_LOCKORDER -DARENA_DEBUG"
13-
export CXXFLAGS="-Werror -Wno-unused-command-line-argument -Wno-unused-value -Wno-deprecated-builtins -Wno-deprecated-volatile"
1413
export PYZMQ=true
1514
export RUN_UNIT_TESTS=false
1615
export RUN_FUNCTIONAL_TESTS=false
1716
export RUN_FUZZ_TESTS=true
1817
export GOAL="install"
19-
export BITCOIN_CONFIG="--enable-zmq --disable-ccache --enable-fuzz --with-sanitizers=fuzzer,address,undefined,integer --enable-suppress-external-warnings CC='clang-18 -ftrivial-auto-var-init=pattern' CXX='clang++-18 -ftrivial-auto-var-init=pattern' --with-boost-process"
18+
export BITCOIN_CONFIG="--enable-zmq --disable-ccache --enable-fuzz --with-sanitizers=fuzzer,address,undefined,integer CC='clang-18 -ftrivial-auto-var-init=pattern' CXX='clang++-18 -ftrivial-auto-var-init=pattern' --with-boost-process"
19+
export NO_WERROR=1

ci/test/00_setup_env_native_fuzz_with_valgrind.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ export RUN_FUNCTIONAL_TESTS=false
1414
export RUN_FUZZ_TESTS=true
1515
export FUZZ_TESTS_CONFIG="--valgrind"
1616
export GOAL="install"
17-
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer --enable-suppress-external-warnings CC=clang-18 CXX=clang++-18"
17+
export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang-18 CXX=clang++-18"

ci/test/00_setup_env_native_multiprocess.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,6 @@ export GOAL="install"
1515
export TEST_RUNNER_EXTRA="--v2transport"
1616
export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang-18 CXX=clang++-18" # Use clang to avoid OOM
1717
# Additional flags for RUN_TIDY
18-
export BITCOIN_CONFIG="${BITCOIN_CONFIG} --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'"
18+
export BITCOIN_CONFIG="${BITCOIN_CONFIG} --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -Wno-error=documentation'"
1919
export BITCOIND=dash-node # Used in functional tests
20+
export NO_WERROR=1

ci/test/00_setup_env_native_nowallet.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ export PACKAGES="python3-zmq"
1212
export DEP_OPTS="NO_WALLET=1 CC=gcc-14 CXX=g++-14"
1313
export GOAL="install"
1414
export BITCOIN_CONFIG="--enable-reduce-exports --with-boost-process CC=gcc-14 CXX=g++-14"
15+
export NO_WERROR=1

ci/test/00_setup_env_native_qt5.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ export RUN_UNIT_TESTS="false"
1616
export GOAL="install"
1717
export PREVIOUS_RELEASES_TO_DOWNLOAD="v0.15.0.0 v0.16.1.1 v0.17.0.3 v18.2.2 v19.3.0 v20.0.1"
1818
export BITCOIN_CONFIG="--enable-zmq --with-libs=no --enable-reduce-exports --disable-fuzz-binary LDFLAGS=-static-libstdc++ --with-boost-process"
19+
export NO_WERROR=1

ci/test/00_setup_env_native_sqlite.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libhar
1111
export DEP_OPTS="NO_BDB=1 NO_UPNP=1 DEBUG=1"
1212
export GOAL="install"
1313
export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --with-sqlite --without-bdb CC=gcc-11 CXX=g++-11"
14+
export NO_WERROR=1

ci/test/00_setup_env_native_tsan.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ export GOAL="install"
1515
export BITCOIN_CONFIG="--enable-zmq --with-sanitizers=thread CC=clang-18 CXX=clang++-18 CXXFLAGS='-g' --with-boost-process"
1616
export CPPFLAGS="-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION"
1717
export PYZMQ=true
18+
export NO_WERROR=1

0 commit comments

Comments
 (0)