ci: Bump lint imagefile FROM base #6
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Miniscript code contains assertions to prevent ending up in an insane state or prevent UB, but also to enforce logical invariants. For the latter it is not necessary to crash the program if they are broken. Raising an exception suffices, especially as this code is often called through the RPC interface which can in turn handle the exception and the user can report it to developers. This is based on previous work from Pieter Wuille.
…solation` Rename the `_randomize_credentials` parameter to Proxy's constructor to `tor_stream_isolation` to make it more clear, and more specific what its purpose is. Also change all call sites to use a named parameter.
git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: 35944ffd23fa26652b82210351d50e896ce16c8f
Bump libmultiprocess library to include MP_INCLUDE_DIR definition from bitcoin-core/libmultiprocess#168 which can simplify the IPC cmake build as described bitcoin#31741 (comment) This update brings in the following changes: bitcoin-core/libmultiprocess#166 doc: rename from chaincodelabs to bitcoin-core bitcoin-core/libmultiprocess#168 Switch `MP_INCLUDE_DIR` to global property
Rename WITH_MULTIPROCESS to ENABLE_IPC, because ENABLE_IPC is a more accurate name for the feature. It controls whether the src/ipc/ directory is built and whether IPC features like -ipcbind, -ipcconnect, and -ipcfd are available. It does NOT currently enable multiprocess features which are implemented in bitcoin#10102 building on top of the IPC features. It will also no longer (as of the next commit), control whether a find_package call is made so the "WITH_" prefix is also inappropriate. -BEGIN VERIFY SCRIPT- git grep -l WITH_MULTIPROCESS | xargs sed -i s/WITH_MULTIPROCESS/ENABLE_IPC/g -END VERIFY SCRIPT-
When ENABLE_IPC option is on, build with libmultiprocess subtree and `add_subdirectory(src/ipc/libmultiprocess)` instead of external package and `find_package(Libmultiprocess)` by default. Behavior can be toggled with `WITH_EXTERNAL_LIBMULTIPROCESS` option. Using a subtree should be more convenient for most bitcoin developers, but using an external package is more convenient for developing in the libmultiprocess repository. The `WITH_EXTERNAL_LIBMULTIPROCESS` option is also used to avoid needing to changing the depends build here. But in later commits, the depends build is switched to use the add_subdirectory build as well. Co-authored-by: Cory Fields <cory-nospam-@coryfields.com> Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
This change is technically not needed to add libmultiprocess as a subtree, but it avoids a CI failure in followup PR bitcoin#30975 which enables multiprocess build option in more CI jobs. In that PR, several jobs fail due to the mptest executable not being built by default, as reported bitcoin#30975 (comment) Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
This change is technically not needed to add libmultiprocess as a subtree, but it avoids a CI failure in followup PR bitcoin#30975 which enables multiprocess build option in more CI jobs. In that PR, the "macOS 14 native no depends job" fails due to warnings in boost headers treated as errors, reported in bitcoin#30975 (comment) and bitcoin-core/libmultiprocess#138
This change is technically not needed to add libmultiprocess as a subtree, but it avoids a CI failure in followup PR bitcoin#30975 which enables multiprocess build option in more CI jobs. In that PR, clang-tidy job fails due to missing generated example files as reported bitcoin#30975 (comment) Different fixes were suggested bitcoin#30975 (comment) and bitcoin#30975 (comment) Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
Without this change linter produces errors about: - Use of std::filesystem the libmultiprocess example program. - Use of locale-dependent functions in example program, in the build time code generator, and in the runtime library for debug logging. - Include guards not beginning with BITCOIN_
Move parts of the int_get_build_id into a new int_get_build_properties function. There is no change in behavior. This just organizes assignments better so some build properties can be used to help compute build ids in the next commit.
With newly introduced libmultiprocess subtree, there's no need for depends system to download and track changes to the upstream repository. Note that adding the libmultiprocess subtree does not allow dropping libmultiprocess packages from the depends build, because libmultiprocess includes a code generation tool called mpgen, and in cross-compiled builds, bitcoin core's cmake build system doesn't have access to a native toolchain and can't build mpgen itself, so the depends system (or the native environment if not using depends) needs to supply it.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Add a class TorsStreamIsolationCredentialsGenerator that generates unique credentials based on a randomly generated session prefix and an atomic counter. This makes sure that different launches of the application won't share the same credentials, and thus circuits, even in edge cases. Example with `-debug=proxy`: ``` 2025-03-31T16:30:27Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-0:0afb2da441f5c105-0 2025-03-31T16:30:31Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-1:0afb2da441f5c105-1 ``` Thanks to hodlinator for the idea.
Replace `uint256::SetHexDeprecated()` calls with `Txid::FromHex()` in four locations: - TransactionTableModel::updateTransaction - TransactionView::contextualMenu - TransactionView::abandonTx - TransactionView::bumpFee The input strings are generally expected to be valid hex strings from `GetHex()`. However, due to the potentially unpredictable return value of `.data(TransactionTableModel::TxHashRole)`, check the `Txid::FromHex` result in `contextualMenu` and return early if the transaction hash is invalid. The other two functions, `abandonTx` and `bumpFee` will only be called if the context menu is enabled.
After replacing all instances of `SetHexDeprecated` in the GUI, remove it entirely and reimplement the behavior in `FromHex`.
When iterating over all fuzz input files in a folder, the order should not matter. However, shuffling may be useful to detect non-determinism. Thus, shuffle in fuzz.cpp, when using neither libFuzzer, nor AFL. Also, shuffle in the deterministic-fuzz-coverage tool, when using libFuzzer.
This avoids non-determistic code paths. Without this patch, the tool would report a diff: cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32 ... - 5371| 393| peer.m_next_send_feefilter = current_time + m_rng.randrange<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY); - 5372| 393| } + 5371| 396| peer.m_next_send_feefilter = current_time + m_rng.randrange<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY); + 5372| 396| } 5373| 16.2k|} ...
The global m_headers_presync_stats is not reset in ResetAndInitialize.
This may lead to non-determinism.
Fix it by incrementing the global node id counter instead.
Without this patch, the tool would report a diff:
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32
...
2587| 3.73k| if (best_it == m_headers_presync_stats.end()) {
------------------
- | Branch (2587:17): [True: 80, False: 3.65k]
+ | Branch (2587:17): [True: 73, False: 3.66k]
------------------
...
… helper This refactor clarifies that the MockableSteadyClock::mock_time_point has millisecond precision by defining a type an using it. Moreover, a ElapseSteady helper is added which can be re-used easily.
This allows the clock to be mockable in tests. Also, replace cs_main
with GetMutex() while touching this function.
Also, use the ElapseSteady test helper in the p2p_headers_presync fuzz
target to make it more deterministic.
The m_last_presync_update variable is a global that is not reset in
ResetAndInitialize. However, it is only used for logging, so completely
disable it for now.
Without this patch, the tool would report a diff:
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32
...
4468| 81| auto now = std::chrono::steady_clock::now();
4469| 81| if (now < m_last_presync_update + std::chrono::milliseconds{250}) return;
- ^80
+ ^79
...
It should be sufficient to set it once. Especially, if the dynamic value is only used by ResetAndInitialize. This also avoids non-determistic code paths, when ResetAndInitialize may re-initialize m_next_inv_to_inbounds. Without this patch, the tool would report a diff: cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32 ... - 1126| 3| m_next_inv_to_inbounds = now + m_rng.rand_exp_duration(average_interval); - 1127| 3| } + 1126| 10| m_next_inv_to_inbounds = now + m_rng.rand_exp_duration(average_interval); + 1127| 10| } 1128| 491| return m_next_inv_to_inbounds; ...
…s_presync This may also avoid non-determinism in the scheduler thread.
This should avoid the remaining non-determistic code coverage paths. Without this patch, the tool would report a diff (only when running without libFuzzer): cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32
868816d refactor: Remove SetHexDeprecated (marcofleon) 6b63218 qt: Update SetHexDeprecated to FromHex (marcofleon) Pull request description: This is part of bitcoin#32189. I'm separating this out because it's not immediately obvious that it's just a refactor. `SetHexDeprecated()` doesn't do any correctness checks on the input, while `FromHex()` does, so it's theoretically possible that there's a behavior change. Replaces `uint256::SetHexDeprecated()` calls with `Txid::FromHex()` in four locations: - `TransactionTableModel::updateTransaction` - `TransactionView::contextualMenu` - `TransactionView::abandonTx` - `TransactionView::bumpFee` The input strings in these cases aren't user input, so they should only be valid hex strings from `GetHex()` (through `TransactionRecord::getTxHash()`). These conversions should be safe without additional checks. ACKs for top commit: laanwj: Code review ACK 868816d w0xlt: Code review ACK bitcoin@868816d BrandonOdiwuor: Code Review ACK 868816d TheCharlatan: ACK 868816d hebasto: ACK 868816d, I have reviewed the code and it looks OK. Tree-SHA512: 121f149dcc7358231d0327cb3212ec96486a88410174d3c74ab8cbd61bad35185bc0a9740d534492b714811f72a6736bc7ac6eeae590c0ea1365c61cc791da37
faa3ce3 fuzz: Avoid influence on the global RNG from peerman m_rng (MarcoFalke) faf4c1b fuzz: Disable unused validation interface and scheduler in p2p_headers_presync (MarcoFalke) fafaca6 fuzz: Avoid setting the mock-time twice (MarcoFalke) fad2214 refactor: Use MockableSteadyClock in ReportHeadersPresync (MarcoFalke) fa9c387 test: Introduce MockableSteadyClock::mock_time_point and ElapseSteady helper (MarcoFalke) faf2d51 fuzz: Move global node id counter along with other global state (MarcoFalke) fa98455 fuzz: Set ignore_incoming_txs in p2p_headers_presync (MarcoFalke) faf2e23 fuzz: Shuffle files before testing them (MarcoFalke) Pull request description: This should make the `p2p_headers_presync` fuzz target more deterministic. Tracking issue: bitcoin#29018. The first commits adds an `ElapseSteady` helper and type aliases. The second commit uses those helpers in `ReportHeadersPresync` and in the fuzz target to increase determinism. ### Testing It can be tested via (setting 32 parallel threads): ``` cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ p2p_headers_presync 32 ``` The failing diff is contained in the commit messages, if applicable. ACKs for top commit: Crypt-iQ: tACK faa3ce3 janb84: Re-ACK [faa3ce3](bitcoin@faa3ce3) marcofleon: ACK faa3ce3 Tree-SHA512: 7e2e0ddf3b4e818300373d6906384df57a87f1eeb507fa43de1ba88cf03c8e6752a26b6e91bfb3ee26a21efcaf1d0d9eaf70d311d1637b671965ef4cb96e6b59
The current version of the doc does not explain how to reproduce a recent fuzzing CI failure (not yet part of the corpora). Add instructions on how to manually create a crash file based on a report.
…ng Tor stream isolation ec81a72 net: Add randomized prefix to Tor stream isolation credentials (laanwj) c47f81e net: Rename `_randomize_credentials` Proxy parameter to `tor_stream_isolation` (laanwj) Pull request description: Add a class TorsStreamIsolationCredentialsGenerator that generates unique credentials based on a randomly generated session prefix and an atomic counter. Use this in `ConnectThroughProxy` instead of a simple atomic int counter. This makes sure that different launches of the application won't share the same credentials, and thus circuits, even in edge cases. Example with `-debug=proxy`: ``` 2025-03-31T16:30:27Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-0:0afb2da441f5c105-0 2025-03-31T16:30:31Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-1:0afb2da441f5c105-1 ``` Thanks to hodlinator in bitcoin#32166 (comment) for the idea. ACKs for top commit: hodlinator: re-ACK ec81a72 jonatack: ACK ec81a72 danielabrozzoni: tACK ec81a72 Tree-SHA512: 195f5885fade77545977b91bdc41394234ae575679cb61631341df443fd8482cd74650104e323c7dbfff7826b10ad61692cca1284d6810f84500a3488f46597a
…K_NONFATAL ff0194a miniscript: convert non-critical asserts to CHECK_NONFATAL (Antoine Poinsot) Pull request description: The Miniscript code contains assertions to prevent ending up in an insane state or prevent UB, but also to enforce logical invariants. For the latter it is not necessary to crash the program if they are broken. Raising an exception suffices, especially as this code is often called through the RPC interface which can in turn handle the exception and the user can report it to developers. This revives bitcoin#28678 from Pieter Wuille. ACKs for top commit: hodlinator: ACK ff0194a TheCharlatan: ACK ff0194a brunoerg: code review ACK ff0194a Tree-SHA512: 8ed8f7b494e46ecf7cdebe75120cd0ffe543b6bc289bf882dac631fe2ec2cae590d5f7bc2316e52db085791694b136dffbc71c40c1e16886fa53ab00bd8cabd0
babb9f5 depends: remove non-native libmultiprocess build (Cory Fields) 5d105fb depends: Switch libmultiprocess packages to use local git subtree (Ryan Ofsky) 9b35518 depends, moveonly: split up int_get_build_id function (Ryan Ofsky) 2d373e2 lint: Add exclusions for libmultiprocess subtree (Ryan Ofsky) e88ab39 doc: Update documentation to explain libmultiprocess subtree (Ryan Ofsky) d4bc563 cmake: Fix clang-tidy "no input files" errors (Ryan Ofsky) abdf3cb cmake: Fix warnings from boost headers (Ryan Ofsky) 8532fcb cmake: Fix ctest mptest "Unable to find executable" errors (Ryan Ofsky) d597ab1 cmake: Support building with libmultiprocess subtree (Ryan Ofsky) 69f0d4a scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake (Ryan Ofsky) a2f28e4 Squashed 'src/ipc/libmultiprocess/' content from commit 35944ffd23fa (Ryan Ofsky) d6244f8 depends: Update libmultiprocess library to simplify cmake subtree build (Ryan Ofsky) Pull request description: This adds the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library and code generator as a subtree in `src/ipc/libmultiprocess` and allows it to be built with the cmake `-DENABLE_IPC` option, which is disabled by default. This PR does not entirely remove the depends system [libmultiprocess package](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/native_libmultiprocess.mk) because the package is useful when cross compiling. (A cross-compiling cmake build cannot easily build and run a native code generation tool.) However, it does update the depends package to build from the new git subtree, instead of being downloaded separately from github, so the same sources are used to build both the runtime library and the code generator. This PR includes the following manual changes (not created automatically with `git subtree add`) which just update the build system and documentation: - [`d6244f85c509` depends: Update libmultiprocess library to simplify cmake subtree build](bitcoin@d6244f8) - [`69f0d4adb72c` scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake](bitcoin@69f0d4a) - [`d597ab1dee6b` cmake: Support building with libmultiprocess subtree](bitcoin@d597ab1) - [`8532fcb1c30d` cmake: Fix ctest mptest "Unable to find executable" errors](bitcoin@8532fcb) - [`abdf3cb6456f` cmake: Fix warnings from boost headers](bitcoin@abdf3cb) - [`d4bc5639829f` cmake: Fix clang-tidy "no input files" errors](bitcoin@d4bc563) - [`e88ab394c163` doc: Update documentation to explain libmultiprocess subtree](bitcoin@e88ab39) - [`2d373e27071f` lint: Add exclusions for libmultiprocess subtree](bitcoin@2d373e2) - [`9b35518d2f3f` depends, moveonly: split up int_get_build_id function](bitcoin@9b35518) - [`5d105fb8c3ff` depends: Switch libmultiprocess packages to use local git subtree](bitcoin@5d105fb) - [`babb9f5db641` depends: remove non-native libmultiprocess build](bitcoin@babb9f5) --- Previous minisketch subtree PR bitcoin#23114 may be useful for comparison Instructions for subtree verification can be found: - https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees - https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh TL&DR: ```sh git remote add --fetch libmultiprocess https://github.com/chaincodelabs/libmultiprocess.git test/lint/git-subtree-check.sh -r src/ipc/libmultiprocess ``` --- This PR is part of the [process separation project](bitcoin#28722). ACKs for top commit: Sjors: re-ACK babb9f5 TheCharlatan: tACK babb9f5 vasild: ACK babb9f5 Tree-SHA512: 43d4eecca5aab63e55c613de935965666eaced327f9fe859a0e9c9b85f7685dc16c5c8d6e03e09ca998628c5d468633f4f743529930b037049abe8e0101e0143
…cally 8fe001d doc: Updates how to reproduce fuzz CI failure locally (Sergi Delgado Segura) Pull request description: The current version of the doc does not explain how to reproduce a recent fuzzing CI failure (not yet part of the corpora). Add instructions on how to manually create a crash file based on a report. ACKs for top commit: maflcko: lgtm ACK 8fe001d glozow: ACK 8fe001d Tree-SHA512: 7436d71a30bbbffc34770027f1deeacca2de528d8d1b333431d6070c2ba779ecfcdaf25dc791d2154ba4dd37824d06aed2695a8412d7ca1f29e5bd1796d42aeb
According to https://wiki.debian.org/DebianReleases#Production_Releases, the current image will be EOL in about one year, so it seems fine to slowly bump it to something more recent.
Currently, the lint_test_runner is built and installed into the lint CI image. This is problematic, because it triggers a full image build on every change to its source code. Doing a build of the lint test_runner on every run is easier and faster.
|
Lint task isn't run here :/ |
l0rinc
pushed a commit
that referenced
this pull request
Jun 25, 2025
Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and:
```bash
export CC=clang
export CXX=clang++
cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address
cmake --build build
export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan"
ctest --test-dir build
```
```bash
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms
********* Finished testing of AddressBookTests *********
=================================================================
==21869==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 88 byte(s) in 1 object(s) allocated from:
#0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
#1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
#2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa)
#3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18
#4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5
#5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75
#6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13
#7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5
#8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5
#9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
#10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7)
#11 0xffff8cf57c54 (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#12 0xffff8cf5fa18 (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#13 0xffff8cf6067c (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#14 0xffff8cf610a4 (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd)
#17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30
#18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
#19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
#20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872)
```
This happens when building using depends:
```bash
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#1 0xfbff97f8c164 (<unknown module>)
#2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5)
#14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o
#15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7)
SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s).
```
l0rinc
pushed a commit
that referenced
this pull request
Jun 25, 2025
5be31b2 lsan: add more Qt suppressions (fanquake) Pull request description: Using Clang clang version 20.1.6 (Fedora 20.1.6-9.fc43) and: ```bash export CC=clang export CXX=clang++ cmake -B build -DBUILD_GUI=ON -DSANITIZERS=address cmake --build build export LSAN_OPTIONS="suppressions=/root/bitcoin/test/sanitizer_suppressions/lsan" ctest --test-dir build ``` ```bash Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 1589ms ********* Finished testing of AddressBookTests ********* ================================================================= ==21869==ERROR: LeakSanitizer: detected memory leaks Direct leak of 88 byte(s) in 1 object(s) allocated from: #0 0xaaaab5d5af40 in operator new(unsigned long) (/root/bitcoin/build/bin/test_bitcoin-qt+0x39af40) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) #1 0xffff8c8f56cc in QLayoutPrivate::createWidgetItem(QLayout const*, QWidget*) (/lib64/libQt6Widgets.so.6+0x1a56cc) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #2 0xffff8c8d2f90 in QBoxLayout::insertWidget(int, QWidget*, int, QFlags<Qt::AlignmentFlag>) (/lib64/libQt6Widgets.so.6+0x182f90) (BuildId: 8b7b9e470f4d4cd920282a4f963abb01225814fa) #3 0xaaaab5fc7188 in SendCoinsDialog::addEntry() /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:596:18 #4 0xaaaab5fc4eec in SendCoinsDialog::SendCoinsDialog(PlatformStyle const*, QWidget*) /root/bitcoin/build/src/qt/./qt/sendcoinsdialog.cpp:84:5 #5 0xaaaab5da67ac in (anonymous namespace)::MiniGUI::MiniGUI(interfaces::Node&, PlatformStyle const*) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:235:75 #6 0xaaaab5da2000 in (anonymous namespace)::TestGUI(interfaces::Node&, std::shared_ptr<wallet::CWallet> const&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:270:13 #7 0xaaaab5d9ebc8 in (anonymous namespace)::TestGUI(interfaces::Node&) /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:453:5 #8 0xaaaab5d9ebc8 in WalletTests::walletTests() /root/bitcoin/build/src/qt/test/./qt/test/wallettests.cpp:475:5 #9 0xffff8b1c5314 in QMetaMethodInvoker::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195314) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) #10 0xffff8b1c5dc8 in QMetaMethod::invokeImpl(QMetaMethod, void*, Qt::ConnectionType, long long, void const* const*, char const* const*, QtPrivate::QMetaTypeInterface const* const*) (/lib64/libQt6Core.so.6+0x195dc8) (BuildId: eacb2d1228362560e5df1a1ce496c99ad61960e7) #11 0xffff8cf57c54 (/lib64/libQt6Test.so.6+0x27c54) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #12 0xffff8cf5fa18 (/lib64/libQt6Test.so.6+0x2fa18) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #13 0xffff8cf6067c (/lib64/libQt6Test.so.6+0x3067c) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #14 0xffff8cf610a4 (/lib64/libQt6Test.so.6+0x310a4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #15 0xffff8cf61aa4 in QTest::qRun() (/lib64/libQt6Test.so.6+0x31aa4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #16 0xffff8cf61eb4 in QTest::qExec(QObject*, int, char**) (/lib64/libQt6Test.so.6+0x31eb4) (BuildId: 96bb1cdeead53af0ced36d7970cf9cd79c4c4ccd) #17 0xaaaab5d7d77c in main /root/bitcoin/build/src/qt/test/./qt/test/test_main.cpp:95:30 #18 0xffff8aad6398 in __libc_start_call_main (/lib64/libc.so.6+0x26398) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) #19 0xffff8aad6478 in __libc_start_main@GLIBC_2.17 (/lib64/libc.so.6+0x26478) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) #20 0xaaaab5c74cac in _start (/root/bitcoin/build/bin/test_bitcoin-qt+0x2b4cac) (BuildId: c0e038f1c507ea6860d1cfd499ac54ad83359872) ``` This happens when building using depends: ```bash Indirect leak of 24 byte(s) in 1 object(s) allocated from: #0 0xaaaabdbe86f8 in malloc (/root/bitcoin/build/bin/test_bitcoin-qt+0x4386f8) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #1 0xfbff97f8c164 (<unknown module>) #2 0xaaaabf0cfaa4 in QDBusConnectionPrivate::QDBusConnectionPrivate() (/root/bitcoin/build/bin/test_bitcoin-qt+0x191faa4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #3 0xaaaabf0c9e30 in QDBusConnectionManager::doConnectToStandardBus(QDBusConnection::BusType, QString const&, bool) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919e30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #4 0xaaaabf0cb0e4 in QtPrivate::QCallableObject<QDBusConnectionPrivate* (QDBusConnectionManager::*)(QDBusConnection::BusType, QString const&, bool), QtPrivate::List<QDBusConnection::BusType&, QString const&, bool&>, QDBusConnectionPrivate*>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x191b0e4) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #5 0xaaaabf5cbaf0 in QObject::event(QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e1baf0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #6 0xaaaabf5a4ce0 in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df4ce0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #7 0xaaaabf5a486c in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df486c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #8 0xaaaabf5a575c in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df575c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #9 0xaaaabf66b858 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1ebb858) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #10 0xaaaabf5a9e3c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1df9e3c) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #11 0xaaaabf632a44 in QThread::exec() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1e82a44) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #12 0xaaaabf0c9bd0 in QDBusConnectionManager::run() (/root/bitcoin/build/bin/test_bitcoin-qt+0x1919bd0) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #13 0xaaaabf669c30 in QThreadPrivate::start(void*) (/root/bitcoin/build/bin/test_bitcoin-qt+0x1eb9c30) (BuildId: dd54811dc11325890f7bac3e3a49d38f5a7ffef5) #14 0xaaaabdbe5f2c in asan_thread_start(void*) asan_interceptors.cpp.o #15 0xffff99538608 in thread_start (/lib64/libc.so.6+0xf8608) (BuildId: 627f878dd454ee3cc1dfdbd347bb565f1ffb53e7) SUMMARY: AddressSanitizer: 3592 byte(s) leaked in 37 allocation(s). ``` ACKs for top commit: maflcko: lgtm ACK 5be31b2 Tree-SHA512: 0c33661c7ec83ea9b874c1ee4ee2de513131690287363e216a88560dfb31a59ef563a50af756c86a991583aa64a600a74e20fd5d6a104cf4c0a27532de8d2211
l0rinc
pushed a commit
that referenced
this pull request
Jul 28, 2025
…xec in RunCommandJSON" faa1c3e Revert "Merge bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON" (MarcoFalke) Pull request description: After a fork() in a multithreaded program, the child can safely call only async-signal-safe functions (see [signal-safety(7)](https://www.man7.org/linux/man-pages/man7/signal-safety.7.html)) until such time as it calls execv. The standard library (`std` namespace) is not async-signal-safe. Also, `throw`, isn't. There was an alternative implementation using `readdir` (bitcoin#32529), but that isn't async-signal-safe either, and that implementation was still using `throw`. So temporarily revert this feature. A follow-up in the future can add it back, using only async-signal-safe functions, or by using a different approach. Fixes bitcoin#32524 Fixes bitcoin#33015 Fixes bitcoin#32855 For reference, a failure can manifest in the GCC debug mode: * While `fork`ing, a debug mode mutex is held (by any other thread). * The `fork`ed child tries to use the stdard libary before `execv` and deadlocks. This may look like the following: ``` (gdb) thread apply all bt Thread 1 (Thread 0xf58f4b40 (LWP 774911) "b-httpworker.2"): #0 0xf7f4f589 in __kernel_vsyscall () #1 0xf79e467e in ?? () from /lib32/libc.so.6 #2 0xf79eb582 in pthread_mutex_lock () from /lib32/libc.so.6 #3 0xf7d93bf2 in ?? () from /lib32/libstdc++.so.6 #4 0xf7d93f36 in __gnu_debug::_Safe_iterator_base::_M_attach(__gnu_debug::_Safe_sequence_base*, bool) () from /lib32/libstdc++.so.6 #5 0x5668810a in __gnu_debug::_Safe_iterator_base::_Safe_iterator_base (this=0xf58f13ac, __seq=0xf58f13f8, __constant=false) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_base.h:91 #6 0x56ddfb50 in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::forward_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:162 #7 0x56ddfacb in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::bidirectional_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:539 #8 0x56ddfa5b in __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int, std::allocator<int> >, std::random_access_iterator_tag>::_Safe_iterator (this=0xf58f13a8, __i=3, __seq=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/safe_iterator.h:687 #9 0x56ddd3f6 in std::__debug::vector<int, std::allocator<int> >::begin (this=0xf58f13f8) at /bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/debug/vector:300 #10 0x57d83701 in subprocess::detail::Child::execute_child (this=0xf58f156c) at ./util/subprocess.h:1372 #11 0x57d80a7c in subprocess::Popen::execute_process (this=0xf58f1cd8) at ./util/subprocess.h:1231 #12 0x57d6d2b4 in subprocess::Popen::Popen<subprocess::input, subprocess::output, subprocess::error, subprocess::close_fds> (this=0xf58f1cd8, cmd_args="fake.py enumerate", args=..., args=..., args=..., args=...) at ./util/subprocess.h:964 #13 0x57d6b597 in RunCommandParseJSON (str_command="fake.py enumerate", str_std_in="") at ./common/run_command.cpp:27 #14 0x57a90547 in ExternalSigner::Enumerate (command="fake.py", signers=std::__debug::vector of length 0, capacity 0, chain="regtest") at ./external_signer.cpp:28 #15 0x56defdab in enumeratesigners()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const (this=0xf58f2ba0, self=..., request=...) at ./rpc/external_signer.cpp:51 ... (truncated, only one thread exists) ``` ACKs for top commit: fanquake: ACK faa1c3e darosior: ACK faa1c3e Tree-SHA512: 602da5f2eba08d7fe01ba19baf411e287ae27fe2d4b82f41734e05b7b1d938ce94cc0041e86ba677284fa92838e96ebee687023ff28047e2b036fd9a53567e0a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SHELLCHECK_VERSION=v0.10.0