08eb5f1b67 ci: document that -Wreturn-type has been fixed upstream (Windows) (fanquake)
Pull request description:
`noreturn` attributes have been added to the mingw-w64 headers, 1690994f51, meaning that [from 11.0.0 onwards](https://www.mingw-w64.org/changelog/), you'll no-longer see `-Wreturn-type` warnings when using `assert(false)`.
Add -Wno-return-type to the Windows CI, where is should have been all
along, and document why it's required. This can be dropped when we are
using the fixed version of the mingw-w64 headers there.
Drop the -Werror -Wno-return-type special case from our build system.
-Wreturn-type is on by default in Clang and GCC.
The new mingw-w64 header behaviour can be checked on Ubuntu mantic, [which ships with 11.0.0](https://packages.ubuntu.com/mantic/mingw-w64), using:
```cpp
#include <cassert>
int f(){ assert(false); }
int main() {
return 0;
}
```
On Mantic (with 11.0.0):
```bash
x86_64-w64-mingw32-g++ test.cpp -Wreturn-type
# nada
```
On Lunar ([with 10.0.0](https://packages.ubuntu.com/lunar/mingw-w64)):
```bash
x86_64-w64-mingw32-g++ test.cpp -Wreturn-type
test.cpp: In function 'int f()':
test.cpp:3:25: warning: no return statement in function returning non-void [-Wreturn-type]
3 | int f(){ assert(false); }
| ^
```
ACKs for top commit:
TheCharlatan:
ACK 08eb5f1b67
Tree-SHA512: 9cd4310a96abd87bf8ceb37949ad0259fe4adee3367c604f4c4ad521a0cf09bdcc5dd305db19a0f45ce74c85178b0d739e2fca5ad0fc841ac935523a23b28a7f
This is needed for the next commit.
This also requires dropping CI_RETRY from the docker build step, which
is fine, because CI_RETRY should be called inside the build script, not
outside.
Also, fix a doc typo.
(Similar to the doc comment in ci/lint_imagefile)
Also, rename docker-entrypoint.sh to container-entrypoint.sh
Also, add copyright header to touched files.
`noreturn` attributes have been added to the mingw-w64 headers, meaning
that from 11.0.0 onwards, you'll no-longer see `-Wreturn-type` warnings
when using assert(false):
1690994f51.
Add -Wno-return-type to the Windows CI, where is should have been all
along, and document why it's required. This can be dropped when we are
using the fixed version of the mingw-w64 headers there.
Drop the -Werror -Wno-return-type special case from our build system.
-Wreturn-type is on by default in Clang and GCC.
6c97757a48 script: appease spelling linter (Jon Atack)
1316119ce7 script: update ignored-words.txt (Jon Atack)
146c861da2 script: update linter dependencies (Jon Atack)
92408224a4 test: fix PEP484 no implicit optional argument types errors (Jon Atack)
f86a301433 script, test: add missing python type annotations (Jon Atack)
Pull request description:
With these updates, `./test/lint/lint-python.py` and `./test/lint/lint-spelling.py` should be green again for developers using relatively recent Python dependencies, in particular mypy 0.991 (released 11/2022) and later. Please see the commit messages for details.
ACKs for top commit:
fanquake:
ACK 6c97757a48
Tree-SHA512: 8a46a4d36d5978affdcecf4f2ace20ca1b52d483e098304911a2169afe60ccb9b042fa90c04b762d94f3ce53d2cafe6f24476ae839867a770c7f31e7e7242d99
These work for me now. If they still don't work in other setups,
maybe we can better document the issues.
```bash
time FILE_ENV="./ci/test/00_setup_env_s390x.sh" ./ci/test_run_all.sh
...
Running tests: coins_tests from test/coins_tests.cpp
PASS: qt/test/test_bitcoin-qt
Running tests: coinstatsindex_tests from test/coinstatsindex_tests.cpp
...
Stop and remove CI container by ID
+ docker container kill 617bef8accb87530e5fbb03ff07b3b9f0aa9e3030d4da424c9612d153ab98dbf
617bef8accb87530e5fbb03ff07b3b9f0aa9e3030d4da424c9612d153ab98dbf
real 51m37.809s
```
fae7c50d20 test: Run fuzz tests on macOS (MarcoFalke)
Pull request description:
Any reason not to?
ACKs for top commit:
jamesob:
Github ACK fae7c50d20
dergoegge:
utACK fae7c50d20
Tree-SHA512: e45122d73fafb17cea312258314b826cb0745e08daadd28465f687ec02d4c127d2f8cbe20179a9fff5712038850c02c968abb4838fa088b7555e28709317d3a3
32e2ffc393 Remove the syscall sandbox (fanquake)
Pull request description:
After initially being merged in #20487, it's no-longer clear that an internal syscall sandboxing mechanism is something that Bitcoin Core should have/maintain, especially when compared to better maintained/supported alterantives, i.e [firejail](https://github.com/netblue30/firejail).
There is more related discussion in #24771.
Note that given where it's used, the sandbox also gets dragged into the kernel.
If it's removed, this should not require any sort of deprecation, as this was only ever an opt-in, experimental feature.
Closes#24771.
ACKs for top commit:
davidgumberg:
crACK 32e2ffc393
achow101:
ACK 32e2ffc393
dergoegge:
ACK 32e2ffc393
Tree-SHA512: 8cf71c5623bb642cb515531d4a2545d806e503b9d57bfc15a996597632b06103d60d985fd7f843a3c1da6528bc38d0298d6b8bcf0be6f851795a8040d71faf16
Also, fix a few bugs:
* Error: RPC command "enumeratesigners" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update test/fuzz/rpc.cpp.
* in run_once: ...format(" ".join(result.args), ... TypeError: sequence item 2: expected str instance, PosixPath found
cbee1d7091 depends: modernize clang flags (Cory Fields)
2a85857ce5 ci: disable false-positive warnings for now (Cory Fields)
Pull request description:
This is a cleaner and simpler alternative to #25098. Inspired by [this conversation](https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1562543301). The diff is large but the change itself is quite small.
Fixes builds with llvm >= 11 in guix by working around the problem. As a bonus, this is much cleaner and more maintainable than what we had before.
See the updated comment for more info. At a high level: rather than playing tricks and trying to work around clang's default includes, disable them and re-add what we want.
ACKs for top commit:
fanquake:
ACK cbee1d7091 - tested Guix and the depends cross-compile. Would like to move this along, to unblock #27676, which itself might be a blocker for #27897. Note that macOS might seem somewhat in flux for the moment, but once we finish the migration to LLVM Clang + LLD, things will be must simpler, and ultimately more maintainable.
TheCharlatan:
ACK cbee1d7091
Tree-SHA512: 5a8300be528f550e15ab23d869e77df7a62201c6d40c0384795a9eecee38118a676e0b79b2b76c5e597597181443caada54a01b75a544dbcde76da1deba8e3a4
0000f55293 ci: Run fuzz target even if input folder is empty (MarcoFalke)
Pull request description:
This should catch trivial integer sanitizer bugs if the author and all reviewers forget to look for them.
ACKs for top commit:
brunoerg:
reACK 0000f55293
dergoegge:
reACK 0000f55293
Tree-SHA512: f139b9d56f0cf1aae339c2890721c77c88d1fea77b73d492c1386ec99b4f393c5b664029919ff4a22e4e8a2929f085699a148c6acc2cc3e40df8a72fd39ff474
clang <=17 warns on -nostdlibinc, which causes an error on our -Werror builds.
Note that this breaks the "-fPIE" check in configure because it relies on
catching warnings, but that is not a problem for macOS.
After initially being merged in #20487, it's no-longer clear that an
internal syscall sandboxing mechanism is something that Bitcoin Core
should have/maintain, especially when compared to better
maintained/supported alterantives, i.e firejail.
Note that given where it's used, the sandbox also gets dragged into the
kernel.
There is some related discussion in #24771.
This should not require any sort of deprecation, as this was only ever
an opt-in, experimental feature.
Closes#24771.
5763b232e6 ci: return to using Ubuntu 22.04 in MSAN jobs (fanquake)
d3cbcbf626 ci: compile clang and compiler-rt in MSAN jobs (fanquake)
796bd1d0d1 ci: use LLVM 16.0.4 in MSAN jobs (fanquake)
883bc9f561 ci: remove extra CC & CXX from MSAN jobs (fanquake)
2d4f4b8f29 ci: standardize custom libc++ usage in MSAN jobs (fanquake)
Pull request description:
This reworks the MSAN CIs, to first compile Clang and compiler-rt (using GCC 12), and then, compile an MSAN instrumented libc++ using the just-built Clang 16. This fixes the `native_fuzz_with_msan` job, working around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341, by not using the Debian provided Clang/LLVM.
Also included are changes to streamline how we use our "custom libc++", according to upstream: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc, as well as other minor cleanups in the CI configs.
An example job is currently running in the qa-assets repo: https://github.com/bitcoin-core/qa-assets/pull/129 (https://cirrus-ci.com/task/4632561431871488).
ACKs for top commit:
dergoegge:
utACK 5763b232e6
Tree-SHA512: 4f2a6e0b796bb1830b8346dd1e55eaa86a79037b8b4f16a336c1e29f4fc460acca2ecba076635459370bcbb4009333cb79d27ef1521c1fb5db7599cd5bdf558c
fa3ab45203 ci: Enable float-divide-by-zero check (MarcoFalke)
Pull request description:
Enable it, because
* It is enabled on OSS-Fuzz, so to be able to catch bugs earlier, enable it here as well.
* It makes sense to enable, because when a float is divided by zero, it may be a logic bug in our code, so it should be suppressed in the suppressions file.
ACKs for top commit:
willcl-ark:
utACK fa3ab45203
dergoegge:
ACK fa3ab45203
Tree-SHA512: 2c2c025af4fe3ec267b3cfa38f25495e9da678cf6c529a6438ec923ef09a06ad37fa4503c30cbacc83578ac2856a7f729ef70a24befffd61d10ec075132d1ee0
fa123077bc ci: Use podman for persistent workers (MarcoFalke)
fa9c65a74c ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN (MarcoFalke)
Pull request description:
This should prevent the persistent workers from running out of disk space. Containers are already removed, but not images. This is required since CI images are built and cached.
ACKs for top commit:
hebasto:
ACK fa123077bc
Tree-SHA512: 07c4faec57d659d1762e4e6d776c882ee48d4bac6ce6d438d56d9ab13277be3e39d6aa38816165a5a3e0938ac5d47674ee2921b6e115a4bb54e3e4910b34c4b6
015cc5e588 lint: stop ignoring LIEF imports (fanquake)
Pull request description:
Type stubs are now available as of 0.13.0.
See https://github.com/lief-project/LIEF/issues/650.
ACKs for top commit:
TheCharlatan:
ACK 015cc5e588
Tree-SHA512: ebb754f293c2a61a0ef64c3552f7c700ceb3054b50fd3f1573e4a9e87773ddeba47bd9875f6ab055043012dbc20aeb71e4d76cd3da535c76651dfb1fbfc66e89
59c8944749 build: disable boost multi index safe mode (willcl-ark)
Pull request description:
Fixes#27586
Disable boost multi index safe mode by default when configuring with
--enable-debug.
This option can cause transactions to take a long time to be accepted
into the mempool under certain conditions; iterator destruction takes
O(n) time vs O(1) as they are stored in a singly linked list. See
27586 and the [boost docs](https://www.boost.org/doc/libs/1_58_0/boost/multi_index/detail/safe_mode.hpp) for more information.
Re-enable it on the CI builds which previously had it enabled.
Re-enable it on the msan fuzz task so that we have fuzz tasks testing
with it enabled and disabled in this repo.
ACKs for top commit:
hebasto:
~ACK 59c89447499bd9d6202269879555b8bc37373aa2~
fanquake:
ACK 59c8944749
Tree-SHA512: ed654f63dbebdd02e4414d1f81147d92a4d490dbb5a2e0376858e3129097645f3a2df45191d6b40c410a76e803b0d28796d1a01c1d2fd995b94e8b7eb3949027
1f97572b9c Fix `#include`s in `src/wallet` (Hennadii Stepanov)
Pull request description:
This PR is a minimum required changes to fix https://github.com/bitcoin/bitcoin/pull/27571#discussion_r1195497290.
ACKs for top commit:
MarcoFalke:
lgtm ACK 1f97572b9c
Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
