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

build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set #25972

Merged
merged 4 commits into from May 4, 2024

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Sep 1, 2022

Now that CXXFLAGS are back in user control, I don't think there's a
reason to no-longer use our warning flags when CXXFLAGS has been
overriden (this includes, by default, when building from depends).

Anyone can suppress warnings from third-party code by
passing the relevant -Wno- options in CXXFLAGS.

Closes: #18092.

@hebasto
Copy link
Member

hebasto commented Sep 1, 2022

Concept ACK.

@fanquake
Copy link
Member Author

fanquake commented Sep 1, 2022

Looks like the CI failures (due to -Werror) are a mix of (older) compiler bugs, and and some things that can actually be improved in our code.

@jarolrod
Copy link
Member

jarolrod commented Sep 2, 2022

GUIX hashes

x86:

decb8702f105372aca1f9b0f0ddb71a911e5d443b198bcb4042cc269cb4ff852  guix-build-0d813df8bb08/output/aarch64-linux-gnu/SHA256SUMS.part
27ade9862b5d37b1f164b5b2a32d26c5f054cdcb3d96544ebb6c978f6f723636  guix-build-0d813df8bb08/output/aarch64-linux-gnu/bitcoin-0d813df8bb08-aarch64-linux-gnu-debug.tar.gz
030fc357f9baca61122cf190a6b90bf843377aab7e75c0a818e4ba1e4905c7fd  guix-build-0d813df8bb08/output/aarch64-linux-gnu/bitcoin-0d813df8bb08-aarch64-linux-gnu.tar.gz
280e0f611532792b9c2160c7b0ed9055425db034132d2321e36c0307cd8cad18  guix-build-0d813df8bb08/output/arm-linux-gnueabihf/SHA256SUMS.part
4d3f6b629b671d99c827d3baa1318da5b72eb6221d07222918dacab405f3254f  guix-build-0d813df8bb08/output/arm-linux-gnueabihf/bitcoin-0d813df8bb08-arm-linux-gnueabihf-debug.tar.gz
69d7a61b63d12f127f94113261acbfac373fe81d5889dc83ec68d3af5b67105f  guix-build-0d813df8bb08/output/arm-linux-gnueabihf/bitcoin-0d813df8bb08-arm-linux-gnueabihf.tar.gz
5236f466cedc1e1e47e379e59e35f440847062ba4e47cbd57f70c221a0531905  guix-build-0d813df8bb08/output/arm64-apple-darwin/SHA256SUMS.part
b0867f75af65d5dd0d5ce1e181fa014ab5681d892ffeca5618d0dcb9c7c85652  guix-build-0d813df8bb08/output/arm64-apple-darwin/bitcoin-0d813df8bb08-arm64-apple-darwin-unsigned.dmg
e8a986eef824442076f7db6cf91a611b9d069b3bdb7d6defae8864e8f955272f  guix-build-0d813df8bb08/output/arm64-apple-darwin/bitcoin-0d813df8bb08-arm64-apple-darwin-unsigned.tar.gz
8955b1ddee5b8063f84f1313b10bab8c02610b200b7166d256a56b867ae02b01  guix-build-0d813df8bb08/output/arm64-apple-darwin/bitcoin-0d813df8bb08-arm64-apple-darwin.tar.gz
759272f39d4b403f3e77483a43a5f86ab8ff58ef5c598dd52a13474af884070d  guix-build-0d813df8bb08/output/dist-archive/bitcoin-0d813df8bb08.tar.gz
0388d6fae623d574efab893a4df6b45bc5d54fa4875fb56fec5c1e62f9230ac5  guix-build-0d813df8bb08/output/powerpc64-linux-gnu/SHA256SUMS.part
2893f906f203d9c9087b5d7149da217010d27b47795fb0cb6f841aa7b5ebe975  guix-build-0d813df8bb08/output/powerpc64-linux-gnu/bitcoin-0d813df8bb08-powerpc64-linux-gnu-debug.tar.gz
2c00544580672579d9aa5d384172598d3e3e64e79d533baa82a49b9cb0734150  guix-build-0d813df8bb08/output/powerpc64-linux-gnu/bitcoin-0d813df8bb08-powerpc64-linux-gnu.tar.gz
875b87899e9fcebf470dd5ef5cb88f850cd23fbc396da07862390c895591a6a5  guix-build-0d813df8bb08/output/powerpc64le-linux-gnu/SHA256SUMS.part
48fab23a31ba8730fa7ab2a005a6481b1eb49720812cae23b6b23544eb2d0422  guix-build-0d813df8bb08/output/powerpc64le-linux-gnu/bitcoin-0d813df8bb08-powerpc64le-linux-gnu-debug.tar.gz
f10672237509f4f34d431e91f9610a35db9a1cfbe58e79e661ed7e62acf11bae  guix-build-0d813df8bb08/output/powerpc64le-linux-gnu/bitcoin-0d813df8bb08-powerpc64le-linux-gnu.tar.gz
9f2a5e1ccc630ed76958e2e01e8378ccac52614154598363018cf96b8e040100  guix-build-0d813df8bb08/output/riscv64-linux-gnu/SHA256SUMS.part
0e5b394f9734e12a21f8a667cbb126d0277b3c74fe4bb1f1ced8e7437396ef1c  guix-build-0d813df8bb08/output/riscv64-linux-gnu/bitcoin-0d813df8bb08-riscv64-linux-gnu-debug.tar.gz
fcfe5c7e864860d90c67b74903d5d1f48b8445594f7054cd532270a099c601fc  guix-build-0d813df8bb08/output/riscv64-linux-gnu/bitcoin-0d813df8bb08-riscv64-linux-gnu.tar.gz
1dcbf133c9bee14a6fcf56529dd3be53e448b8a587eccd94e9657e9f047baf7d  guix-build-0d813df8bb08/output/x86_64-apple-darwin/SHA256SUMS.part
6aa4ef971d8a2733b7f5a49c60e997d3141ea0512494e1392bc8e3556b83e0df  guix-build-0d813df8bb08/output/x86_64-apple-darwin/bitcoin-0d813df8bb08-x86_64-apple-darwin-unsigned.dmg
2ad9a69d1bf2b2a1bd9249051030f04dfc7455028d07220e5334927485276902  guix-build-0d813df8bb08/output/x86_64-apple-darwin/bitcoin-0d813df8bb08-x86_64-apple-darwin-unsigned.tar.gz
0641782cfa61a90d22ed44580419cfc446a8e2ca63c5469d89d13bc8ab7d5878  guix-build-0d813df8bb08/output/x86_64-apple-darwin/bitcoin-0d813df8bb08-x86_64-apple-darwin.tar.gz
668e4cadcbdaa215d9082a3f762a23cc4ca402703211e931f5c9149c7ddd69a6  guix-build-0d813df8bb08/output/x86_64-linux-gnu/SHA256SUMS.part
df202b4b1f4e45e45f33c93db58c14b3c6a709bd2622a29ba87a3f200662911d  guix-build-0d813df8bb08/output/x86_64-linux-gnu/bitcoin-0d813df8bb08-x86_64-linux-gnu-debug.tar.gz
1709f2b99d7ef9aed33a63e272706cbea6aeb702720b3f68494671c06a231b50  guix-build-0d813df8bb08/output/x86_64-linux-gnu/bitcoin-0d813df8bb08-x86_64-linux-gnu.tar.gz
4f7cc1e902a4afa627ef093327cd41349ac2f03cdf6a9e535187108b4a5cdf12  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/SHA256SUMS.part
a245445900b04bf681d9f428ab066aaee3206d09e5cf75e6b30ac6e09d006d00  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64-debug.zip
8d4579e1e06584f81b2c85ecbea491f462872440d53210388946082ef286ed1e  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64-setup-unsigned.exe
9b25a931b8d2515447cc6ec238a36bac86f74b8550c6b44513a45f6a5242e146  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64-unsigned.tar.gz
5dc6e99f10c522cdf6dd41589017171fd00c20afb9fd2149539e0ea2ba425303  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64.zip

arm64:

decb8702f105372aca1f9b0f0ddb71a911e5d443b198bcb4042cc269cb4ff852  guix-build-0d813df8bb08/output/aarch64-linux-gnu/SHA256SUMS.part
27ade9862b5d37b1f164b5b2a32d26c5f054cdcb3d96544ebb6c978f6f723636  guix-build-0d813df8bb08/output/aarch64-linux-gnu/bitcoin-0d813df8bb08-aarch64-linux-gnu-debug.tar.gz
030fc357f9baca61122cf190a6b90bf843377aab7e75c0a818e4ba1e4905c7fd  guix-build-0d813df8bb08/output/aarch64-linux-gnu/bitcoin-0d813df8bb08-aarch64-linux-gnu.tar.gz
280e0f611532792b9c2160c7b0ed9055425db034132d2321e36c0307cd8cad18  guix-build-0d813df8bb08/output/arm-linux-gnueabihf/SHA256SUMS.part
4d3f6b629b671d99c827d3baa1318da5b72eb6221d07222918dacab405f3254f  guix-build-0d813df8bb08/output/arm-linux-gnueabihf/bitcoin-0d813df8bb08-arm-linux-gnueabihf-debug.tar.gz
69d7a61b63d12f127f94113261acbfac373fe81d5889dc83ec68d3af5b67105f  guix-build-0d813df8bb08/output/arm-linux-gnueabihf/bitcoin-0d813df8bb08-arm-linux-gnueabihf.tar.gz
5236f466cedc1e1e47e379e59e35f440847062ba4e47cbd57f70c221a0531905  guix-build-0d813df8bb08/output/arm64-apple-darwin/SHA256SUMS.part
b0867f75af65d5dd0d5ce1e181fa014ab5681d892ffeca5618d0dcb9c7c85652  guix-build-0d813df8bb08/output/arm64-apple-darwin/bitcoin-0d813df8bb08-arm64-apple-darwin-unsigned.dmg
e8a986eef824442076f7db6cf91a611b9d069b3bdb7d6defae8864e8f955272f  guix-build-0d813df8bb08/output/arm64-apple-darwin/bitcoin-0d813df8bb08-arm64-apple-darwin-unsigned.tar.gz
8955b1ddee5b8063f84f1313b10bab8c02610b200b7166d256a56b867ae02b01  guix-build-0d813df8bb08/output/arm64-apple-darwin/bitcoin-0d813df8bb08-arm64-apple-darwin.tar.gz
759272f39d4b403f3e77483a43a5f86ab8ff58ef5c598dd52a13474af884070d  guix-build-0d813df8bb08/output/dist-archive/bitcoin-0d813df8bb08.tar.gz
0388d6fae623d574efab893a4df6b45bc5d54fa4875fb56fec5c1e62f9230ac5  guix-build-0d813df8bb08/output/powerpc64-linux-gnu/SHA256SUMS.part
2893f906f203d9c9087b5d7149da217010d27b47795fb0cb6f841aa7b5ebe975  guix-build-0d813df8bb08/output/powerpc64-linux-gnu/bitcoin-0d813df8bb08-powerpc64-linux-gnu-debug.tar.gz
2c00544580672579d9aa5d384172598d3e3e64e79d533baa82a49b9cb0734150  guix-build-0d813df8bb08/output/powerpc64-linux-gnu/bitcoin-0d813df8bb08-powerpc64-linux-gnu.tar.gz
875b87899e9fcebf470dd5ef5cb88f850cd23fbc396da07862390c895591a6a5  guix-build-0d813df8bb08/output/powerpc64le-linux-gnu/SHA256SUMS.part
48fab23a31ba8730fa7ab2a005a6481b1eb49720812cae23b6b23544eb2d0422  guix-build-0d813df8bb08/output/powerpc64le-linux-gnu/bitcoin-0d813df8bb08-powerpc64le-linux-gnu-debug.tar.gz
f10672237509f4f34d431e91f9610a35db9a1cfbe58e79e661ed7e62acf11bae  guix-build-0d813df8bb08/output/powerpc64le-linux-gnu/bitcoin-0d813df8bb08-powerpc64le-linux-gnu.tar.gz
9f2a5e1ccc630ed76958e2e01e8378ccac52614154598363018cf96b8e040100  guix-build-0d813df8bb08/output/riscv64-linux-gnu/SHA256SUMS.part
0e5b394f9734e12a21f8a667cbb126d0277b3c74fe4bb1f1ced8e7437396ef1c  guix-build-0d813df8bb08/output/riscv64-linux-gnu/bitcoin-0d813df8bb08-riscv64-linux-gnu-debug.tar.gz
fcfe5c7e864860d90c67b74903d5d1f48b8445594f7054cd532270a099c601fc  guix-build-0d813df8bb08/output/riscv64-linux-gnu/bitcoin-0d813df8bb08-riscv64-linux-gnu.tar.gz
1dcbf133c9bee14a6fcf56529dd3be53e448b8a587eccd94e9657e9f047baf7d  guix-build-0d813df8bb08/output/x86_64-apple-darwin/SHA256SUMS.part
6aa4ef971d8a2733b7f5a49c60e997d3141ea0512494e1392bc8e3556b83e0df  guix-build-0d813df8bb08/output/x86_64-apple-darwin/bitcoin-0d813df8bb08-x86_64-apple-darwin-unsigned.dmg
2ad9a69d1bf2b2a1bd9249051030f04dfc7455028d07220e5334927485276902  guix-build-0d813df8bb08/output/x86_64-apple-darwin/bitcoin-0d813df8bb08-x86_64-apple-darwin-unsigned.tar.gz
0641782cfa61a90d22ed44580419cfc446a8e2ca63c5469d89d13bc8ab7d5878  guix-build-0d813df8bb08/output/x86_64-apple-darwin/bitcoin-0d813df8bb08-x86_64-apple-darwin.tar.gz
668e4cadcbdaa215d9082a3f762a23cc4ca402703211e931f5c9149c7ddd69a6  guix-build-0d813df8bb08/output/x86_64-linux-gnu/SHA256SUMS.part
df202b4b1f4e45e45f33c93db58c14b3c6a709bd2622a29ba87a3f200662911d  guix-build-0d813df8bb08/output/x86_64-linux-gnu/bitcoin-0d813df8bb08-x86_64-linux-gnu-debug.tar.gz
1709f2b99d7ef9aed33a63e272706cbea6aeb702720b3f68494671c06a231b50  guix-build-0d813df8bb08/output/x86_64-linux-gnu/bitcoin-0d813df8bb08-x86_64-linux-gnu.tar.gz
4f7cc1e902a4afa627ef093327cd41349ac2f03cdf6a9e535187108b4a5cdf12  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/SHA256SUMS.part
a245445900b04bf681d9f428ab066aaee3206d09e5cf75e6b30ac6e09d006d00  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64-debug.zip
8d4579e1e06584f81b2c85ecbea491f462872440d53210388946082ef286ed1e  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64-setup-unsigned.exe
9b25a931b8d2515447cc6ec238a36bac86f74b8550c6b44513a45f6a5242e146  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64-unsigned.tar.gz
5dc6e99f10c522cdf6dd41589017171fd00c20afb9fd2149539e0ea2ba425303  guix-build-0d813df8bb08/output/x86_64-w64-mingw32/bitcoin-0d813df8bb08-win64.zip

@fanquake
Copy link
Member Author

cc @theuni. See: https://github.com/bitcoin/bitcoin/pull/25972/checks?check_run_id=8137548994, for an example of unsuppressed sdt.h warnings.

@theuni
Copy link
Member

theuni commented Sep 13, 2022

As @fanquake mentioned, some of these are only warnings in older compilers. I'm going through them to double-check that they are indeed non-issues, as opposed to missed warnings in newer compilers.

(I'm ignoring the sdt.h warnings for now and focusing on what's in our code. I will look at that header next.)

I've pushed commits that implement the following suggestions here: https://github.com/theuni/bitcoin/commits/use_cxxflags_with_depends

For this one:

script/descriptor.cpp:1555:21: error: loop variable 'keyspan' of type 'const Span' creates a copy from type 'const Span' [-Werror,-Wrange-loop-analysis]
for (const auto keyspan : match->second) {

This was a warning in clang-9 but not clang-10. The warning was removed here: https://reviews.llvm.org/D72212

tl;dr: copies are allowed for POD types less than 64bytes.

We can confirm this by adding a dummy char foo[65] to Span, which causes newer clang to warn as expected.

That seems reasonable enough, but I think we may as well just fix it up to take a const ref to remove any ambiguity.

for this one:

script/descriptor.cpp: In function ‘std::unique_ptr<{anonymous}::DescriptorImpl> {anonymous}::InferScript(const CScript&, {anonymous}::ParseScriptContext, const SigningProvider&)’:
script/descriptor.cpp:1262:17: error: ‘’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
return {};

I can reproduce this with gcc9 but not gcc10. My best guess is that it's related to: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

Though I'm having trouble recreating a minimal test-case that fails with gcc9 and succeeds with gcc10.

Changing these from return {} to return std::nullopt avoids the problem and makes the code more clear too, so again, I suggest simply appeasing the dumb compiler because there's not really a reason not to.

For this one:

fs.cpp: In member function ‘bool fsbridge::FileLock::TryLock()’:
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Werror=missing-field-initializers]
129 | _OVERLAPPED overlapped = {0};

This one looks legitimate. Suggest fixing by initializing to {}.

For this one:

test/fuzz/float.cpp: In function ‘void float_fuzz_target(FuzzBufferType)’:
test/fuzz/float.cpp:50:40: error: ‘tmp’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
uint64_t encoded = EncodeDouble(d);

(This applies to x86_64 as well)

Again, gcc10 is smart enough to figure this out, but gcc9 complains. Initializing to zero is harmless, so again I suggest just initializing to 0 to quiet the old compilers.

@theuni
Copy link
Member

theuni commented Sep 13, 2022

I also pushed a patch for systemtap in depends that gets rid of those compiler warnings. From what I can tell, the variadic is completely unused there, but I'm not at all confident in that change yet.

@theuni
Copy link
Member

theuni commented Sep 13, 2022

Concept ACK, btw :)

@fanquake
Copy link
Member Author

This one looks legitimate. Suggest fixing by initializing to {}.

I had opened #26006, to see if anyone would jump in with the change, but at this point will just PR your commit separate to this.

For the remaining of the changes, I've rebased on top of your branch I think the changes are fairly uncontreversial (aside from sdt.h), and are better than adding -Wno-x for older compilers / CI jobs, which is better again than going the #pragma in our source code route. Although given they are changes to code, they can also be split out of here if wanted.

I also pushed a patch for systemtap in depends that gets rid of those compiler warnings. From what I can tell, the variadic is completely unused there, but I'm not at all confident in that change yet.

Maybe @0xB10C can weigh in?

@fanquake
Copy link
Member Author

With the _OVERLAPPED fix applied, the Win64 job fails differently:

/usr/bin/ccache x86_64-w64-mingw32-g++-posix -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config  -fmacro-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-w64-mingw32=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -DHAVE_BUILD_INFO -D_MT -DWIN32 -D_WINDOWS -D_WIN32_WINNT=0x0601 -D_WIN32_IE=0x0501 -DWIN32_LEAN_AND_MEAN -DNOMINMAX -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -I/tmp/cirrus-ci-build/depends/x86_64-w64-mingw32/include/  -fdebug-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-w64-mingw32=. -fstack-reuse=none -Wstack-protector -fstack-protector-all -fcf-protection=full -Wall -Wextra -Wformat -Wformat-security -Wvla -Wredundant-decls -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wno-unused-parameter -Werror -Wno-error=return-type    -fno-extended-identifiers -fvisibility=hidden -fPIE -pipe -std=c++17 -O2  -c -o script/libbitcoin_consensus_a-interpreter.o `test -f 'script/interpreter.cpp' || echo './'`script/interpreter.cpp
script/interpreter.cpp: In function ‘bool SignatureHashSchnorr(uint256&, ScriptExecutionData&, const T&, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData&, MissingDataBehavior) [with T = CTransaction]’:
script/interpreter.cpp:1530:42: error: ‘ext_flag’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1530 |     const uint8_t spend_type = (ext_flag << 1) + (have_annex ? 1 : 0); // The low bit indicates whether an annex is present.
      |                                ~~~~~~~~~~^~~~~
script/interpreter.cpp: In function ‘bool SignatureHashSchnorr(uint256&, ScriptExecutionData&, const T&, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData&, MissingDataBehavior) [with T = CMutableTransaction]’:
script/interpreter.cpp:1530:42: error: ‘ext_flag’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1530 |     const uint8_t spend_type = (ext_flag << 1) + (have_annex ? 1 : 0); // The low bit indicates whether an annex is present.
      |                                ~~~~~~~~~~^~~~~

This looks like another false-positive, as ext_flag should always be either 0 or 1, or we would have asserted:

uint8_t ext_flag, key_version;
.

@0xB10C
Copy link
Contributor

0xB10C commented Sep 14, 2022

I also pushed a patch for systemtap in depends that gets rid of those compiler warnings. From what I can tell, the variadic is completely unused there, but I'm not at all confident in that change yet.

Maybe @0xB10C can weigh in?

I too think that we don't use the variadic and it's Ok to drop it in the patch:

We use, for example, the DTRACE_PROBE6(context, event, a, b, c, d, e, f) macro which expands to STAP_PROBE6(provider,probe,parm1,parm2,parm3,parm4,parm5,parm6) expanding too _SDT_PROBE(provider, name, 6, (arg1, arg2, arg3, arg4, arg5, arg6)) and again expanding too (_SDT_ASM_BODY(provider, name, _SDT_ASM_ARGS, (n)) which is defined as _SDT_ASM_BODY(provider, name, pack_args, args, ...) containing the variadic. We only fill in the provider, name, pack_args, args and don't use the variadic.

See e.g. this warning in https://cirrus-ci.com/task/4790719054348288?logs=ci#L2368.

...
usr/bin/ccache clang++ -m32 -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config  -DDEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -DRPC_DOC_CHECK -DABORT_ON_FAILED_ASSUME -fmacro-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu=.  -DHAVE_BUILD_INFO -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -isystem /tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DBOOST_NO_CXX98_FUNCTION_BASE -DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -I./leveldb/include   -isystem /tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include -pthread -I/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include  -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -I/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/  -O0 -g3 -ftrapv -fdebug-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wunreachable-code-loop-increment -Wimplicit-fallthrough -Wdocumentation -Wno-unused-parameter -Wno-self-assign -Werror     -fPIE -pipe -std=c++17 -O1  -c -o libbitcoin_node_a-net.o `test -f 'net.cpp' || echo './'`net.cpp
net.cpp:2767:5: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments]
    TRACE6(net, outbound_message,
    ^
./util/trace.h:18:50: note: expanded from macro 'TRACE6'
#define TRACE6(context, event, a, b, c, d, e, f) DTRACE_PROBE6(context, event, a, b, c, d, e, f)
                                                 ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/sys/sdt.h:496:3: note: expanded from macro 'DTRACE_PROBE6'
  STAP_PROBE6(provider,probe,parm1,parm2,parm3,parm4,parm5,parm6)
  ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/sys/sdt.h:382:3: note: expanded from macro 'STAP_PROBE6'
  _SDT_PROBE(provider, name, 6, (arg1, arg2, arg3, arg4, arg5, arg6))
  ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/sys/sdt.h:78:75: note: expanded from macro '_SDT_PROBE'
    __asm__ __volatile__ (_SDT_ASM_BODY(provider, name, _SDT_ASM_ARGS, (n)) \
                                                                          ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/sys/sdt.h:285:9: note: macro '_SDT_ASM_BODY' defined here
#define _SDT_ASM_BODY(provider, name, pack_args, args, ...)                   \
        ^
1 error generated.
make[2]: *** [Makefile:9699: libbitcoin_node_a-net.o] Error 1
make[2]: Leaving directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/src'
make[1]: *** [Makefile:18924: install-recursive] Error 1
make[1]: Leaving directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/src'
make: *** [Makefile:821: install-recursive] Error 1

The the tracepoint tests in this task succeed. Also, for reference, this also came up in #25528 (comment) and the variadic was only recently re-added in sytemtap@ecab2afe. If needed, we could also have our own sdt.h at some point with everything we don't use thrown out.

@fanquake
Copy link
Member Author

I've opened an upstream issue: chaincodelabs/libmultiprocess#77, for the warnings in the multiprocess CI job:

/usr/bin/ccache clang++ -m32 -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config  -DDEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -DRPC_DOC_CHECK -DABORT_ON_FAILED_ASSUME -fmacro-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu=.  -DHAVE_BUILD_INFO -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -I/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/  -O0 -g3 -ftrapv -fdebug-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu=. -Wstack-protector -fstack-protector-all -fcf-protection=full -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wunreachable-code-loop-increment -Wimplicit-fallthrough -Wdocumentation -Wno-unused-parameter -Wno-self-assign -Werror     -fPIE -std=c++17 -pthread -I/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include -pipe -std=c++17 -O1  -c -o ipc/capnp/libbitcoin_ipc_a-protocol.o `test -f 'ipc/capnp/protocol.cpp' || echo './'`ipc/capnp/protocol.cpp
In file included from ipc/capnp/protocol.cpp:14:
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/mp/proxy-io.h:33:37: error: parameter 'connection' shadows member inherited from type 'InvokeContext' [-Werror,-Wshadow-field]
    ClientInvokeContext(Connection& connection, ThreadContext& thread_context)
                                    ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/mp/proxy-io.h:27:17: note: declared here
    Connection& connection;
                ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/mp/proxy-io.h:160:16: error: moving a local object in a return statement prevents copy elision [-Werror,-Wpessimizing-move]
        return std::move(logger);
               ^
/tmp/cirrus-ci-build/depends/i686-pc-linux-gnu/include/mp/proxy-io.h:160:16: note: remove std::move call here
        return std::move(logger);
               ^~~~~~~~~~      ~
2 errors generated.

as they are from the multiprocess code:

@theuni
Copy link
Member

theuni commented Sep 14, 2022

With the _OVERLAPPED fix applied, the Win64 job fails differently:

/usr/bin/ccache x86_64-w64-mingw32-g++-posix -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config  -fmacro-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-w64-mingw32=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -DHAVE_BUILD_INFO -D_MT -DWIN32 -D_WINDOWS -D_WIN32_WINNT=0x0601 -D_WIN32_IE=0x0501 -DWIN32_LEAN_AND_MEAN -DNOMINMAX -D_FILE_OFFSET_BITS=64 -DPROVIDE_FUZZ_MAIN_FUNCTION -I. -I./minisketch/include -I./secp256k1/include -I./univalue/include -I./leveldb/include -I/tmp/cirrus-ci-build/depends/x86_64-w64-mingw32/include/  -fdebug-prefix-map=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-w64-mingw32=. -fstack-reuse=none -Wstack-protector -fstack-protector-all -fcf-protection=full -Wall -Wextra -Wformat -Wformat-security -Wvla -Wredundant-decls -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wno-unused-parameter -Werror -Wno-error=return-type    -fno-extended-identifiers -fvisibility=hidden -fPIE -pipe -std=c++17 -O2  -c -o script/libbitcoin_consensus_a-interpreter.o `test -f 'script/interpreter.cpp' || echo './'`script/interpreter.cpp
script/interpreter.cpp: In function ‘bool SignatureHashSchnorr(uint256&, ScriptExecutionData&, const T&, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData&, MissingDataBehavior) [with T = CTransaction]’:
script/interpreter.cpp:1530:42: error: ‘ext_flag’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1530 |     const uint8_t spend_type = (ext_flag << 1) + (have_annex ? 1 : 0); // The low bit indicates whether an annex is present.
      |                                ~~~~~~~~~~^~~~~
script/interpreter.cpp: In function ‘bool SignatureHashSchnorr(uint256&, ScriptExecutionData&, const T&, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData&, MissingDataBehavior) [with T = CMutableTransaction]’:
script/interpreter.cpp:1530:42: error: ‘ext_flag’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1530 |     const uint8_t spend_type = (ext_flag << 1) + (have_annex ? 1 : 0); // The low bit indicates whether an annex is present.
      |                                ~~~~~~~~~~^~~~~

This looks like another false-positive, as ext_flag should always be either 0 or 1, or we would have asserted:

uint8_t ext_flag, key_version;

.

Bumping #25065 (comment) again.

assert(0) doesn't prevent this code from being hit in -DNDEBUG builds, so I believe this warning is correct.

Long-term, I think we could either drop the -DDEBUG requirement and:

  • return false instead of asserting and teach the callers to understand the meaning
  • templatize the sigversion

Short-term, the simple/obvious solution is just to initialize to zero, which wouldn't affect the meaning of the code.

@maflcko
Copy link
Member

maflcko commented Sep 14, 2022

assert(0) doesn't prevent this code from being hit in -DNDEBUG builds, so I believe this warning is correct.

There is also Assert(0), which shouldn't have this static analysis problem.

Edit: Ok, maybe it does. Though annotating assertion_fail with [[noreturn]] might fix that. Otherwise a macro could help, see #24812 (comment) ?

@theuni
Copy link
Member

theuni commented Sep 14, 2022

Here's a slightly more invasive change that eliminates the possibility of undefined behavior by filtering out script versions that can't be consumed by these functions: https://github.com/theuni/bitcoin/commits/explicit-v1-scriptver

The key idea is here: theuni@5f26e4a#diff-da9625bc8e6928c0ec3239921e739c32b9d0abdfbca9773c550a3cdd818230b8R196

The second commit is just a small cleanup.

I think it's a nice improvement. Will PR if there's any interest.

@theuni
Copy link
Member

theuni commented Sep 14, 2022

I too think that we don't use the variadic and it's Ok to drop it in the patch:

We use, for example, the DTRACE_PROBE6(context, event, a, b, c, d, e, f) macro which expands to STAP_PROBE6(provider,probe,parm1,parm2,parm3,parm4,parm5,parm6) expanding too _SDT_PROBE(provider, name, 6, (arg1, arg2, arg3, arg4, arg5, arg6)) and again expanding too (_SDT_ASM_BODY(provider, name, _SDT_ASM_ARGS, (n)) which is defined as _SDT_ASM_BODY(provider, name, pack_args, args, ...) containing the variadic. We only fill in the provider, name, pack_args, args and don't use the variadic.

See e.g. this warning in https://cirrus-ci.com/task/4790719054348288?logs=ci#L2368.

The the tracepoint tests in this task succeed. Also, for reference, this also came up in #25528 (comment) and the variadic was only recently re-added in sytemtap@ecab2afe. If needed, we could also have our own sdt.h at some point with everything we don't use thrown out.

Note that not only do we do not use the variadic, it's not even hooked up to anything. _SDT_ASM_BODY doesn't use __VA_ARGS__ at all, so any extra args passed in would just be discarded anyway. I'm assuming this is a placeholder for some later upstream feature (or maybe something someone forgot to remove), but for now it's doing nothing but triggering a warning.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 14, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK maflcko, hebasto, theuni, TheCharlatan
Concept ACK mruddy

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

@maflcko
Copy link
Member

maflcko commented Sep 15, 2022

Here's a slightly more invasive change that eliminates the possibility of undefined behavior

Strictly speaking, this will keep the UB. Recall that an enum class is allowed to hold any integral value as long as it fits in the underlying type (even if the value is not named in the enum). So you'd have to re-add the assert to avoid UB (an uninitialized value), in which case the warning would likely re-appear.

@theuni
Copy link
Member

theuni commented Sep 15, 2022

Here's a slightly more invasive change that eliminates the possibility of undefined behavior

Strictly speaking, this will keep the UB. Recall that an enum class is allowed to hold any integral value as long as it fits in the underlying type (even if the value is not named in the enum). So you'd have to re-add the assert to avoid UB (an uninitialized value), in which case the warning would likely re-appear.

Heh, I suppose I invited the pedantry there.

Correct, it wouldn't eliminate UB potential entirely here, but it would put it on par with every other switch/case over an fully-handled enum class without a default in our codebase. I don't think you're suggesting adding default: assert(0) for the others? :)

As I see it it'd be strictly an improvement (though admittedly not bulletproof) as the intent is better communicated to callers, and would cleanly eliminate this warning. Would you NACK it as a PR?

fanquake added a commit that referenced this pull request Sep 15, 2022
02c9e56 fs: fully initialize _OVERLAPPED for win32 (Cory Fields)

Pull request description:

  ```bash
  fs.cpp: In member function ‘bool fsbridge::FileLock::TryLock()’:
  fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Werror=missing-field-initializers]
    129 |     _OVERLAPPED overlapped = {0};
        |                                ^
  fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Werror=missing-field-initializers]
  fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Werror=missing-field-initializers]
  ```

  Came up in #25972. That PR is now rebased on this change.

  Closes: #26006

ACKs for top commit:
  sipsorcery:
    tACK 02c9e56.
  hebasto:
    ACK 02c9e56, tested on Linux x86_64:

Tree-SHA512: 6a0495c34bd952b2bb8c994a1450da7d3eee61225bb4ff0ce009c013f5e29dba94bb1c3ecef9989dc18c939909fdc8eba690a38f96da431ae9d64c23656de7d0
@maflcko
Copy link
Member

maflcko commented Sep 15, 2022

I don't think you're suggesting adding default: assert(0) for the others? :)

No, but generally we still put an assert in the fallthrough case. Putting the assert after the switch is equivalent, assuming the cases return. See for example

case(CoinStatsHashType::NONE): {
return ComputeUTXOStats(view, stats, nullptr, interruption_point);
}
} // no default case, so the compiler can warn about missing cases
assert(false);

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 15, 2022
02c9e56 fs: fully initialize _OVERLAPPED for win32 (Cory Fields)

Pull request description:

  ```bash
  fs.cpp: In member function ‘bool fsbridge::FileLock::TryLock()’:
  fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Werror=missing-field-initializers]
    129 |     _OVERLAPPED overlapped = {0};
        |                                ^
  fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Werror=missing-field-initializers]
  fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Werror=missing-field-initializers]
  ```

  Came up in bitcoin#25972. That PR is now rebased on this change.

  Closes: bitcoin#26006

ACKs for top commit:
  sipsorcery:
    tACK 02c9e56.
  hebasto:
    ACK 02c9e56, tested on Linux x86_64:

Tree-SHA512: 6a0495c34bd952b2bb8c994a1450da7d3eee61225bb4ff0ce009c013f5e29dba94bb1c3ecef9989dc18c939909fdc8eba690a38f96da431ae9d64c23656de7d0
@DrahtBot
Copy link
Contributor

DrahtBot commented May 2, 2024

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/24511515887

@fanquake fanquake marked this pull request as ready for review May 2, 2024 14:31
@DrahtBot DrahtBot removed the CI failed label May 2, 2024
This produces false positives, such as:
```bash
torcontrol.cpp: In static member function ‘static void TorControlConnection::readcb(bufferevent*, void*)’:
torcontrol.cpp:94:28: error: ‘result’ may be used uninitialized [-Werror=maybe-uninitialized]
   94 |         self->message.code = ToIntegral<int>(s.substr(0, 3)).value_or(0);
      |         ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./netaddress.h:18,
                 from ./torcontrol.h:11,
                 from torcontrol.cpp:6:
./util/strencodings.h:184:7: note: ‘result’ was declared here
  184 |     T result;
      |       ^~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:11088: libbitcoin_node_a-torcontrol.o] Error 1
```
These warnings are coming from leveldb, and appear to be fixed starting
with GCC 13.
The issues here are coming from Boost Test code.
Now that CXXFLAGS are back in user control, I don't think there's a
reason to no-longer use our warning flags when CXXFLAGS has been
overriden (this includes when building from depends).

Anyone can suppress warnings from third-party code by
passing the relevant `-Wno-` options in CXXFLAGS.

Fixes: bitcoin#18092.
@maflcko
Copy link
Member

maflcko commented May 3, 2024

Would it make sense to mention that -Wno-error=... should be passed, when needed, in the compile docs?

utACK f0e22be 🍡

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: utACK f0e22be69a15248c42964d57f44ce8c37a36081d 🍡
vBxoHilXIAccIn8maseZr4k9PlbsNC+EF53i//yO3OEBMcPEVz9reyJkwVIrCLY4DaAwvqIwXqgWBZt0N+XZBA==

@DrahtBot DrahtBot requested a review from hebasto May 3, 2024 09:39
@fanquake fanquake requested a review from TheCharlatan May 3, 2024 10:30
@fanquake
Copy link
Member Author

fanquake commented May 3, 2024

Guix Build (aarch64):

a416b91cfcc2ed18250021e67b5130e46d407998a642cdaed49996c0be79dd08  guix-build-f0e22be69a15/output/aarch64-linux-gnu/SHA256SUMS.part
569dc4cf491fe4f07009ce8f9d202c05f29a18eec6963fa3e7286a944dd93a82  guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu-debug.tar.gz
126a3b9712bb7685e89e84f6b0d18cce6389bf580e3e66136a976e1f2f50f76f  guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu.tar.gz
90a78669828d32025c50fce357131f009d3a3fb887d84385dd94d82e8d973bdf  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/SHA256SUMS.part
56988aa072006b96fdde476c4a32ed54acc908504b9d858c5e230d43612f4833  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/bitcoin-f0e22be69a15-arm-linux-gnueabihf-debug.tar.gz
1b8a9b3bfbfad88eb10f787623a4ad60f1f87803c0d4040bbe1bf24c4da1e308  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/bitcoin-f0e22be69a15-arm-linux-gnueabihf.tar.gz
5cb8e8f0a4b9aa8a4334623225fd4d329842769f273ec46075dd6079cc283a0d  guix-build-f0e22be69a15/output/arm64-apple-darwin/SHA256SUMS.part
fece504ef26647e11bdf3adc747b895e1dc69576e17c6dc17c61d3471bf810d7  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin-unsigned.tar.gz
1d34f9419cc66d5143d19bbcb1a24d14381cb1d32a9e0599b529a93e4bd7d6fd  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin-unsigned.zip
59144930e85db21845a6b3887405b75a6106657b18aa3ede4ab12e99d9fcffdb  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin.tar.gz
10eb6357756af723a8d204e0bc4604ee10f91004d7a65c57a6259cfb01d02f34  guix-build-f0e22be69a15/output/dist-archive/bitcoin-f0e22be69a15.tar.gz
aa34b3c18d342c8bad11fafbc92628c74aee33a9e1261f31f62449bc05e4a4f9  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/SHA256SUMS.part
1719ecffad40d62beb6e94e65f6b9956ed8e0eb68d5e94b7a2a812ea7d3241da  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/bitcoin-f0e22be69a15-powerpc64-linux-gnu-debug.tar.gz
cbcad735319c6838813617cdc12ff14bde2d81b5f8c35cf6e57c8a9ebe00dbf6  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/bitcoin-f0e22be69a15-powerpc64-linux-gnu.tar.gz
6c644fc29cd8e0fb2cd7bcd3a397f8c9d06ae8701e26096f93f6659b10d66a86  guix-build-f0e22be69a15/output/riscv64-linux-gnu/SHA256SUMS.part
70e2cd91d72e6d1e1835cfbc3d7cb4e3c38af906195e65636ad7706f1f959f11  guix-build-f0e22be69a15/output/riscv64-linux-gnu/bitcoin-f0e22be69a15-riscv64-linux-gnu-debug.tar.gz
fac50f4a7eeed00a8a2fafb3dcaead6ffb14a8e2fe5fda95dd2c76527e3fd612  guix-build-f0e22be69a15/output/riscv64-linux-gnu/bitcoin-f0e22be69a15-riscv64-linux-gnu.tar.gz
2232949e9fea2f17fe7ebdf219db6892a4448988271114fbe6f21015048a9b13  guix-build-f0e22be69a15/output/x86_64-apple-darwin/SHA256SUMS.part
4c307c6f96d13e2521b1ed2a5c8faa1c11b246074c8b22a8e53b00e60f72ff1b  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin-unsigned.tar.gz
f268a0de8a7a45aa2b625e089af1ce83097bc0a079b46578d4000f78c4547b90  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin-unsigned.zip
d248cc1686997b73ade7cb7353a23cb9a6a1d19c0718a4a014e27480d0d7c356  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin.tar.gz
417921fe8a58abca99111e21da7663eba019183cdc2a132dab05cf020d374227  guix-build-f0e22be69a15/output/x86_64-linux-gnu/SHA256SUMS.part
f9cd999dd1e9cb149718a70b720b5514aceddf8e059cde75358ae9e0a3eb72ae  guix-build-f0e22be69a15/output/x86_64-linux-gnu/bitcoin-f0e22be69a15-x86_64-linux-gnu-debug.tar.gz
e95cc49e5467f49b5afa63b9358ef0b2afea3df90256982f6a1dfc49ffde9a33  guix-build-f0e22be69a15/output/x86_64-linux-gnu/bitcoin-f0e22be69a15-x86_64-linux-gnu.tar.gz
8c3d88b8437feefd651fd55b03da15114f7d7eba72a674de1a1af4c3bc341208  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/SHA256SUMS.part
06fe5199d3da1b8cb305a652402f18ef586e3a3c4dc1267a512e921284b6561f  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-debug.zip
a5ffc57c3cd232ab644052ee071b6e49ffe1b29f624879632b6af57c8ea3a8e7  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-setup-unsigned.exe
90a0bd945c60f88d8fef06d11c0b01810e7247988aa937cdc4a09fd7cc214941  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-unsigned.tar.gz
03d46a89b69a93b395f8bb47826c6f0e98b060f086a6afa14ed9d6c8b61e1517  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64.zip

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f0e22be.

My Guix builds:

x86_64
a416b91cfcc2ed18250021e67b5130e46d407998a642cdaed49996c0be79dd08  guix-build-f0e22be69a15/output/aarch64-linux-gnu/SHA256SUMS.part
569dc4cf491fe4f07009ce8f9d202c05f29a18eec6963fa3e7286a944dd93a82  guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu-debug.tar.gz
126a3b9712bb7685e89e84f6b0d18cce6389bf580e3e66136a976e1f2f50f76f  guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu.tar.gz
90a78669828d32025c50fce357131f009d3a3fb887d84385dd94d82e8d973bdf  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/SHA256SUMS.part
56988aa072006b96fdde476c4a32ed54acc908504b9d858c5e230d43612f4833  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/bitcoin-f0e22be69a15-arm-linux-gnueabihf-debug.tar.gz
1b8a9b3bfbfad88eb10f787623a4ad60f1f87803c0d4040bbe1bf24c4da1e308  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/bitcoin-f0e22be69a15-arm-linux-gnueabihf.tar.gz
5cb8e8f0a4b9aa8a4334623225fd4d329842769f273ec46075dd6079cc283a0d  guix-build-f0e22be69a15/output/arm64-apple-darwin/SHA256SUMS.part
fece504ef26647e11bdf3adc747b895e1dc69576e17c6dc17c61d3471bf810d7  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin-unsigned.tar.gz
1d34f9419cc66d5143d19bbcb1a24d14381cb1d32a9e0599b529a93e4bd7d6fd  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin-unsigned.zip
59144930e85db21845a6b3887405b75a6106657b18aa3ede4ab12e99d9fcffdb  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin.tar.gz
10eb6357756af723a8d204e0bc4604ee10f91004d7a65c57a6259cfb01d02f34  guix-build-f0e22be69a15/output/dist-archive/bitcoin-f0e22be69a15.tar.gz
aa34b3c18d342c8bad11fafbc92628c74aee33a9e1261f31f62449bc05e4a4f9  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/SHA256SUMS.part
1719ecffad40d62beb6e94e65f6b9956ed8e0eb68d5e94b7a2a812ea7d3241da  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/bitcoin-f0e22be69a15-powerpc64-linux-gnu-debug.tar.gz
cbcad735319c6838813617cdc12ff14bde2d81b5f8c35cf6e57c8a9ebe00dbf6  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/bitcoin-f0e22be69a15-powerpc64-linux-gnu.tar.gz
6c644fc29cd8e0fb2cd7bcd3a397f8c9d06ae8701e26096f93f6659b10d66a86  guix-build-f0e22be69a15/output/riscv64-linux-gnu/SHA256SUMS.part
70e2cd91d72e6d1e1835cfbc3d7cb4e3c38af906195e65636ad7706f1f959f11  guix-build-f0e22be69a15/output/riscv64-linux-gnu/bitcoin-f0e22be69a15-riscv64-linux-gnu-debug.tar.gz
fac50f4a7eeed00a8a2fafb3dcaead6ffb14a8e2fe5fda95dd2c76527e3fd612  guix-build-f0e22be69a15/output/riscv64-linux-gnu/bitcoin-f0e22be69a15-riscv64-linux-gnu.tar.gz
2232949e9fea2f17fe7ebdf219db6892a4448988271114fbe6f21015048a9b13  guix-build-f0e22be69a15/output/x86_64-apple-darwin/SHA256SUMS.part
4c307c6f96d13e2521b1ed2a5c8faa1c11b246074c8b22a8e53b00e60f72ff1b  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin-unsigned.tar.gz
f268a0de8a7a45aa2b625e089af1ce83097bc0a079b46578d4000f78c4547b90  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin-unsigned.zip
d248cc1686997b73ade7cb7353a23cb9a6a1d19c0718a4a014e27480d0d7c356  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin.tar.gz
417921fe8a58abca99111e21da7663eba019183cdc2a132dab05cf020d374227  guix-build-f0e22be69a15/output/x86_64-linux-gnu/SHA256SUMS.part
f9cd999dd1e9cb149718a70b720b5514aceddf8e059cde75358ae9e0a3eb72ae  guix-build-f0e22be69a15/output/x86_64-linux-gnu/bitcoin-f0e22be69a15-x86_64-linux-gnu-debug.tar.gz
e95cc49e5467f49b5afa63b9358ef0b2afea3df90256982f6a1dfc49ffde9a33  guix-build-f0e22be69a15/output/x86_64-linux-gnu/bitcoin-f0e22be69a15-x86_64-linux-gnu.tar.gz
8c3d88b8437feefd651fd55b03da15114f7d7eba72a674de1a1af4c3bc341208  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/SHA256SUMS.part
06fe5199d3da1b8cb305a652402f18ef586e3a3c4dc1267a512e921284b6561f  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-debug.zip
a5ffc57c3cd232ab644052ee071b6e49ffe1b29f624879632b6af57c8ea3a8e7  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-setup-unsigned.exe
90a0bd945c60f88d8fef06d11c0b01810e7247988aa937cdc4a09fd7cc214941  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-unsigned.tar.gz
03d46a89b69a93b395f8bb47826c6f0e98b060f086a6afa14ed9d6c8b61e1517  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64.zip

@TheCharlatan
Copy link
Contributor

Guix builds (aarch64)

a416b91cfcc2ed18250021e67b5130e46d407998a642cdaed49996c0be79dd08  guix-build-f0e22be69a15/output/aarch64-linux-gnu/SHA256SUMS.part
569dc4cf491fe4f07009ce8f9d202c05f29a18eec6963fa3e7286a944dd93a82  guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu-debug.tar.gz
126a3b9712bb7685e89e84f6b0d18cce6389bf580e3e66136a976e1f2f50f76f  guix-build-f0e22be69a15/output/aarch64-linux-gnu/bitcoin-f0e22be69a15-aarch64-linux-gnu.tar.gz
90a78669828d32025c50fce357131f009d3a3fb887d84385dd94d82e8d973bdf  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/SHA256SUMS.part
56988aa072006b96fdde476c4a32ed54acc908504b9d858c5e230d43612f4833  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/bitcoin-f0e22be69a15-arm-linux-gnueabihf-debug.tar.gz
1b8a9b3bfbfad88eb10f787623a4ad60f1f87803c0d4040bbe1bf24c4da1e308  guix-build-f0e22be69a15/output/arm-linux-gnueabihf/bitcoin-f0e22be69a15-arm-linux-gnueabihf.tar.gz
5cb8e8f0a4b9aa8a4334623225fd4d329842769f273ec46075dd6079cc283a0d  guix-build-f0e22be69a15/output/arm64-apple-darwin/SHA256SUMS.part
fece504ef26647e11bdf3adc747b895e1dc69576e17c6dc17c61d3471bf810d7  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin-unsigned.tar.gz
1d34f9419cc66d5143d19bbcb1a24d14381cb1d32a9e0599b529a93e4bd7d6fd  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin-unsigned.zip
59144930e85db21845a6b3887405b75a6106657b18aa3ede4ab12e99d9fcffdb  guix-build-f0e22be69a15/output/arm64-apple-darwin/bitcoin-f0e22be69a15-arm64-apple-darwin.tar.gz
10eb6357756af723a8d204e0bc4604ee10f91004d7a65c57a6259cfb01d02f34  guix-build-f0e22be69a15/output/dist-archive/bitcoin-f0e22be69a15.tar.gz
aa34b3c18d342c8bad11fafbc92628c74aee33a9e1261f31f62449bc05e4a4f9  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/SHA256SUMS.part
1719ecffad40d62beb6e94e65f6b9956ed8e0eb68d5e94b7a2a812ea7d3241da  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/bitcoin-f0e22be69a15-powerpc64-linux-gnu-debug.tar.gz
cbcad735319c6838813617cdc12ff14bde2d81b5f8c35cf6e57c8a9ebe00dbf6  guix-build-f0e22be69a15/output/powerpc64-linux-gnu/bitcoin-f0e22be69a15-powerpc64-linux-gnu.tar.gz
6c644fc29cd8e0fb2cd7bcd3a397f8c9d06ae8701e26096f93f6659b10d66a86  guix-build-f0e22be69a15/output/riscv64-linux-gnu/SHA256SUMS.part
70e2cd91d72e6d1e1835cfbc3d7cb4e3c38af906195e65636ad7706f1f959f11  guix-build-f0e22be69a15/output/riscv64-linux-gnu/bitcoin-f0e22be69a15-riscv64-linux-gnu-debug.tar.gz
fac50f4a7eeed00a8a2fafb3dcaead6ffb14a8e2fe5fda95dd2c76527e3fd612  guix-build-f0e22be69a15/output/riscv64-linux-gnu/bitcoin-f0e22be69a15-riscv64-linux-gnu.tar.gz
2232949e9fea2f17fe7ebdf219db6892a4448988271114fbe6f21015048a9b13  guix-build-f0e22be69a15/output/x86_64-apple-darwin/SHA256SUMS.part
4c307c6f96d13e2521b1ed2a5c8faa1c11b246074c8b22a8e53b00e60f72ff1b  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin-unsigned.tar.gz
f268a0de8a7a45aa2b625e089af1ce83097bc0a079b46578d4000f78c4547b90  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin-unsigned.zip
d248cc1686997b73ade7cb7353a23cb9a6a1d19c0718a4a014e27480d0d7c356  guix-build-f0e22be69a15/output/x86_64-apple-darwin/bitcoin-f0e22be69a15-x86_64-apple-darwin.tar.gz
417921fe8a58abca99111e21da7663eba019183cdc2a132dab05cf020d374227  guix-build-f0e22be69a15/output/x86_64-linux-gnu/SHA256SUMS.part
f9cd999dd1e9cb149718a70b720b5514aceddf8e059cde75358ae9e0a3eb72ae  guix-build-f0e22be69a15/output/x86_64-linux-gnu/bitcoin-f0e22be69a15-x86_64-linux-gnu-debug.tar.gz
e95cc49e5467f49b5afa63b9358ef0b2afea3df90256982f6a1dfc49ffde9a33  guix-build-f0e22be69a15/output/x86_64-linux-gnu/bitcoin-f0e22be69a15-x86_64-linux-gnu.tar.gz
8c3d88b8437feefd651fd55b03da15114f7d7eba72a674de1a1af4c3bc341208  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/SHA256SUMS.part
06fe5199d3da1b8cb305a652402f18ef586e3a3c4dc1267a512e921284b6561f  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-debug.zip
a5ffc57c3cd232ab644052ee071b6e49ffe1b29f624879632b6af57c8ea3a8e7  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-setup-unsigned.exe
90a0bd945c60f88d8fef06d11c0b01810e7247988aa937cdc4a09fd7cc214941  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64-unsigned.tar.gz
03d46a89b69a93b395f8bb47826c6f0e98b060f086a6afa14ed9d6c8b61e1517  guix-build-f0e22be69a15/output/x86_64-w64-mingw32/bitcoin-f0e22be69a15-win64.zip

@theuni
Copy link
Member

theuni commented May 3, 2024

I can't repro the -Wmaybe-uninitialized issue. Might it be possible to just add the initializations (even if they're unnecessary) rather than disabling?

Like for the example provided, making it:

T result{};

Edit: nm, I see there are other false-positives as well.

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f0e22be. It'll be nice to have this fixed.

@maflcko
Copy link
Member

maflcko commented May 3, 2024

I can't repro the -Wmaybe-uninitialized issue.

It should repro in the CI env, at least. Or you can grep the CI log: https://cirrus-ci.com/task/6169505322237952?logs=ci#L3768

@DrahtBot
Copy link
Contributor

DrahtBot commented May 3, 2024

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File commit 99d7538
(master)
commit d8d9671
(master and this pull)
SHA256SUMS.part 2d37ee006e42acbb... 14b39fcddbe53b60...
*-aarch64-linux-gnu-debug.tar.gz 2fc5a4c0d2994de9... f7a7763ccb552c8a...
*-aarch64-linux-gnu.tar.gz b05b39619c69080e... 600622f681e128c7...
*-arm-linux-gnueabihf-debug.tar.gz c34f642cd99b82c6... 730dc69ae58bf1aa...
*-arm-linux-gnueabihf.tar.gz 76fbe6ef414cc8ff... 3d7bd879fb77f965...
*-arm64-apple-darwin-unsigned.tar.gz ae6edc9bc96874a3... d08513bb136812fa...
*-arm64-apple-darwin-unsigned.zip 2401cd650040cf60... 70add25fbaa1f1ca...
*-arm64-apple-darwin.tar.gz 5315c7233fb206a5... 9342de9b3a32c46e...
*-powerpc64-linux-gnu-debug.tar.gz 63a1c68ee85d0e80... 4816d075a5c9e0f9...
*-powerpc64-linux-gnu.tar.gz 1f70493198c2b2e1... 36a8499e2b375eeb...
*-riscv64-linux-gnu-debug.tar.gz e5c8ae0919d4faa3... 898472569ad18d03...
*-riscv64-linux-gnu.tar.gz 56bb869ec1166776... b622721d43d399da...
*-x86_64-apple-darwin-unsigned.tar.gz 77020cf43485ad4e... bea425a4fc43fa34...
*-x86_64-apple-darwin-unsigned.zip bfb96ca11469209a... 02e984df1abea15f...
*-x86_64-apple-darwin.tar.gz 6670c292aef914b9... 0e5cf4dd8926f4bb...
*-x86_64-linux-gnu-debug.tar.gz 21e71d0b876b2efb... 2413a3a26e5c1ecb...
*-x86_64-linux-gnu.tar.gz acc2c6ef070804ef... d8b117ddd17c4aa6...
*.tar.gz 502b24f208bb3556... 62e6518439272b98...
guix_build.log 032c73808c40d082... 982802f94c631bda...
guix_build.log.diff e67731a6b2fa1c45...

Copy link
Contributor

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f0e22be

@fanquake
Copy link
Member Author

fanquake commented May 4, 2024

Would it make sense to mention that -Wno-error=... should be passed, when needed, in the compile docs?

Maybe into the CI or dev docs, given we'll never use -Werror by default for compilation?

@fanquake fanquake merged commit bd597c3 into bitcoin:master May 4, 2024
16 checks passed
@fanquake fanquake deleted the use_cxxflags_with_depends branch May 4, 2024 00:47
@maflcko
Copy link
Member

maflcko commented May 4, 2024

Maybe into the CI or dev docs, given we'll never use -Werror by default for compilation?

Oh I see. --enable-werror is only set in the CI in this project. Seems fine to leave as-is.

@hebasto
Copy link
Member

hebasto commented May 4, 2024

Ported to the CMake-based build system in hebasto#188.

hebasto added a commit to hebasto/bitcoin that referenced this pull request May 8, 2024
1689b48 fixup! ci: Test CMake edge cases (Hennadii Stepanov)
b72aeba fixup! ci: Test CMake edge cases (Hennadii Stepanov)
f38ec56 fixup! cmake: Add compiler diagnostic flags (Hennadii Stepanov)

Pull request description:

  This PR ports bitcoin#25972 after the recent sync/rebase [PR](#186).

ACKs for top commit:
  vasild:
    ACK 1689b48

Tree-SHA512: 0cc7a6a1ad0c9e99e0dddcd69bf00ae182119b33ecc0f21d22446e207ce756b8fa7d865585dfd2fb43458c44db00194e9088f7ea01a1c7cc226f3117a20374d7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build: CXXFLAGS with depends
9 participants