backport: bitcoin#17725, #18304, #18392, #18430, #18438, #18441, #18477, #18562, #18569, #18683, #18899, #18912, #19159, #19669, partial #14794 - asan/valgrind CI changes by knst · Pull Request #5379 · dashpay/dash

knst · 2023-05-18T11:24:32Z

Issue being fixed or feature implemented

This PR is the next batch of bitcoin backports, mostly related to CI, address sanitizer and valgrind.

What was done?

There are discovered 2 issues that won't let to enable Asan CI and valgrind CI at the moment.

1. Asan (address sanitizer) generates a lot of failures like that both CI and locally:

********* Finished testing of TrafficGraphDataTests *********
~BitcoinApplication : Stopping thread
~BitcoinApplication : Stopped thread
==28475==WARNING: invalid path to external symbolizer!
==28475==WARNING: Failed to use and restart external symbolizer!
=================================================================
==28475==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 520 byte(s) in 13 object(s) allocated from:
    #0 0x55fc08e2353d  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d)
    #1 0x7f7a7724bd85  (<unknown module>)
Direct leak of 432 byte(s) in 2 object(s) allocated from:
    #0 0x55fc08e2353d  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d)
    #1 0x55fc0dc63b63  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0xc5d1b63)
    #2 0x8604597ff0e8feff  (<unknown module>)
Direct leak of 176 byte(s) in 2 object(s) allocated from:
    #0 0x55fc08e52cad  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x77c0cad)
    #1 0x55fc0d822372  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0xc190372)
Direct leak of 66 byte(s) in 3 object(s) allocated from:
    #0 0x55fc08e2353d  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d)
    #1 0x7f7a77243633  (<unknown module>)
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x55fc08e2353d  (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d)
    #1 0x7f7a77245449  (<unknown module>)

Here's run on CI: https://gitlab.com/dashpay/dash/-/pipelines/883649210
I tried different ways to add more debug infos or check what is these addresses but I haven't localized issue at the moment.
So, ASAN build is disabled at the moment.

2. valgrind fails for each usage of std::optional when it passes as an argument in function params.

==6024== Conditional jump or move depends on uninitialised value(s) 
==6024==    at 0xF6C314: CWallet::ScanForWalletTransactions(uint256 const&, int, std::optional<int>, WalletRescanReserver const&, bool) (wallet.cpp:1976)
==6024==    by 0x8245D8: wallet_tests::scan_for_wallet_transactions::test_method() (wallet_tests.cpp:106)
==6024==    by 0x8267B4: wallet_tests::scan_for_wallet_transactions_invoker() (wallet_tests.cpp:87)
==6024==    by 0x115D4F1: boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) (in DASH/src/test/test_dash)
==6024==    by 0x115BB9C: boost::execution_monitor::catch_signals(boost::function<int ()> const&) (in DASH/src/test/test_dash)
==6024==    by 0x115BC24: boost::execution_monitor::execute(boost::function<int ()> const&) (in DASH/src/test/test_dash)
==6024==    by 0x115BD04: boost::execution_monitor::vexecute(boost::function<void ()> const&) (in DASH/src/test/test_dash)
==6024==    by 0x1122834: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) (in DASH/src/test/test_dash)
==6024==    by 0x10FC18F: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) [clone .isra.0] (in DASH/src/test/test_dash)
==6024==    by 0x10FC489: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) [clone .isra.0] (in DASH/src/test/test_dash)
==6024==    by 0x10FC489: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) [clone .isra.0] (in DASH/src/test/test_dash)
==6024==    by 0x11004D6: boost::unit_test::framework::run(unsigned long, bool) (in DASH/src/test/test_dash)

There's only one example, but it happens for all other places with std::optional too.
valgrind CI is actually removed from bitcoin and super-seeded by address sanitizer, but it doesn't work anyway at the moment for dash's code.

How Has This Been Tested?

Run functional and unit tests + build locally with valgrind/address sanitizer to see a result.
Based on local runs this PR includes several fixes of UB.

Breaking Changes

No breaking changes.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have made corresponding changes to the documentation
I have assigned this pull request to a milestone

github-actions · 2023-05-24T17:40:58Z

This pull request has conflicts, please rebase.

github-actions · 2023-05-31T16:09:38Z

This pull request has conflicts, please rebase.

UdjinM6 · 2023-05-31T08:29:42Z

18683: NOTE: this will have no effect for us (no travis and wrong repo)

UdjinM6

utACK

PastaPastaPasta

utACK for merging via merge commit

facb416 ci: Add valgrind run (MarcoFalke) Pull request description: Fixes bitcoin#17460 ACKs for top commit: practicalswift: ACK facb416 Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d

4444edc ci: Enable all functional tests in valgrind (MarcoFalke) Pull request description: The travis timeout for our repo has been bumped to 2h, so we can run all tests in valgrind now ACKs for top commit: practicalswift: ACK 4444edc -- regarding the three disabled cases (`feature_abortnode`, `feature_block` and `rpc_bind`): not a big deal since MSan will take care of those once bitcoin#18288 is merged. More is more :) Tree-SHA512: ea2f798112911b6d1f3d88cfcdf0a7cdb698687248343703d6fe55da144542c961c15d888bffb41672c10aa76765615cb7c7ff93d468bfad3c51f962f24e7abb

fa92af5 ci: Run feature_block and feature_abortnode in valgrind (MarcoFalke) fa01feb test: Remove ci timeout restriction in test_runner (MarcoFalke) Pull request description: Also revert commit 0a4912e, because some tests take too long for this to be useful anymore. Top commit has no ACKs. Tree-SHA512: 363f14766e1f4a5860ab668a516b41acebc6fbdf11d8defb3a95a772dbf82304ca1f5f14b1dbad97f2029503e03d92e8c69df0466a8872409c20665838f617ed

fab7d95 test: Make valgrind.supp work on aarch64 (MarcoFalke) Pull request description: Was easy to fix by simply removing a line ACKs for top commit: practicalswift: ACK fab7d95 -- diff looks correct Tree-SHA512: d2d7c6cac453a3177c20e256ec50a03066f8dbf5ae45299077ccf4a2b45a3a40252b1b5fcaf9224a59bb5c3df5bd90ac58af27eb0f47dc87c2640df5b2b460ca

fa082d0 travis: Remove valgrind (MarcoFalke) Pull request description: When the valgrind run was added, it took 2 hours. Travis kindly raised the timeout limit to the maximum possible of 3 hours. Today, a full build of Bitcoin Core with all tests takes more than three hours. Thus, it is impossible to run all tests on travis. Moreover, the feedback loop for developers that create a pull request takes at least 2 hours, but in some cases (when the travis queue is full) until the next day. This is unacceptable. Fix both issues by removing the build from travis. Please note that the `ci/test/` configurations are *not* removed. They will stay in the repo and can be executed anywhere (just not on travis). ACKs for top commit: jamesob: ACK bitcoin@fa082d0 jnewbery: utACK fa082d0 Tree-SHA512: 9acaa0e2d3926014fadb7dd2e86c4e01df382e9399f6ae99f989fa609da66a77bdd1b75d6ff42d2686f38f730b8564e6dc722aa597a473290c9d30c2abe7ef0f

fae1e99 ci: Only clone bitcoin-core/qa-assets when fuzzing (MarcoFalke) Pull request description: Currently the only content of that repo are some seeds, so we can speed up some ci builds ACKs for top commit: laanwj: ACK fae1e99 (provided this passes travis) Tree-SHA512: ed813738e7f24bb56a2f12aa3b398e414eb4f0ba98379836a33ff3e5602cbf42a28e89aad10e346468191ecddc03e60d5b236097112e27c07cb1c2293533ea58

25c8b73 ci: Use Homebrew addon on native macOS (Hennadii Stepanov) 596c627 ci: Fix brew in Travis (Hennadii Stepanov) Pull request description: Recently almost every macOS image update on Travis breaks our builds: - bitcoin#17848 - bitcoin#18436 This PR: - fixes the error caused by the recent [update](https://changelog.travis-ci.com/xcode-11-3-1-xcode-11-2-1-xcode-11-1-and-xcode11-images-updated-142286) from 10.14.4 (18E226) to 10.14.6 (18G3020) on March 25 - leverages [Homebrew addon](https://config.travis-ci.com/ref/job/addons/homebrew) to install packages Homebrew is not told to install `automake` and `pkg-config` packages, as the [docs](https://docs.travis-ci.com/user/reference/osx/#compilers-and-build-toolchain) states that they are pre-installed: > - automake 1.16.1 > - pkg-config 0.29.2 Top commit has no ACKs. Tree-SHA512: 1a70c06468fbe162503081b03dcf54614d67abf8ff0ce07d118b5bb50bbb92c182a76f769bea586c691aa82b9281a29cdef88091acc16895817a2e7cddafec6e

…scalar e41e46c ci: Remove misplaced comments from folded block scalar (Hennadii Stepanov) Pull request description: On master (f3a91ab) the [build log for ARM](https://travis-ci.org/github/bitcoin/bitcoin/jobs/667247758) contains: ``` $ export FILE_ENV="./ci/test/00_setup_env_arm.sh" $ export QEMU_USER_CMD="" $ export Can= $ export run= $ export the= $ export tests= $ export natively= $ export without= $ export qemu= Setting up build cache ... ``` and ![Screenshot from 2020-03-26 18-58-50](https://user-images.githubusercontent.com/32963518/77674223-e28d2d00-6f93-11ea-82a1-7f805ccb678d.png) With this PR: ``` $ export FILE_ENV="./ci/test/00_setup_env_arm.sh" $ export QEMU_USER_CMD="" Setting up build cache ... ``` and ![Screenshot from 2020-03-26 19-01-56](https://user-images.githubusercontent.com/32963518/77674456-4d3e6880-6f94-11ea-9c83-546c81f1cdf7.png) --- Also Travis [build config validation](https://docs.travis-ci.com/user/build-config-validation) added: ![Screenshot from 2020-03-26 19-04-19](https://user-images.githubusercontent.com/32963518/77674686-9ee6f300-6f94-11ea-900f-fdef8a673113.png) Top commit has no ACKs. Tree-SHA512: a3ea05d543a4d46c65fffd7c67df9bbd4ca8d426fa215c5ce26653de34145edd419a54884da0416608a2c90bf5b421cab3ec32290c8916f10399354b44ddeb99

faa9491 ci: Use Focal for fuzzers (MarcoFalke) Pull request description: This gives us access to clang-10, as well as a newer version of valgrind ACKs for top commit: fanquake: ACK faa9491 - [Clang 10](https://packages.ubuntu.com/focal/clang) and [valgrind 3.15](https://packages.ubuntu.com/focal/valgrind). practicalswift: ACK faa9491 -- diff looks correct & contemporary clang is better than vintage clang Tree-SHA512: 0e67232673434c0309db79c1054e3e981115083585945967e346f4d58792635832100f89911428aab928155e44e5f401207a023681ae008fdb5280cf02c4d427

fa7af33 ci: Run unit tests sequential once (MarcoFalke) fa68a3e appveyor: Enable minimal unit test logging to aid debugging (MarcoFalke) Pull request description: Fixes bitcoin#16976 Top commit has no ACKs. Tree-SHA512: 1f1ee8776a67afa8c1c5a16ef170c9975b6a486c087c8eba12e97d23382befd1c2801622ec70ca8e4cd1fbedce1dec46be67677ceaf07f35f1d3f3bead0200f0

6136a96 ci: Rename RUN_CI_ON_HOST to DANGER_RUN_CI_ON_HOST (Hennadii Stepanov) 97ba77a ci: Add native s390x (Hennadii Stepanov) Pull request description: Unlike the Docker wrapped solution (bitcoin#17591) this PR suggests running on host system directly. This approach makes builds quick and stable (see: bitcoin#18106). The excerpt from the Travis log: ``` ... Running on host system without docker wrapper ... Byte Order: Big Endian ... ``` ACKs for top commit: MarcoFalke: ACK 6136a96 Tree-SHA512: 1b591de13e38d10a35217e1de11cbd648a359d18d16eed166fac18ea5788b58cc9fc6d407086ed342b99e57e479efd951a0ea693710177e500eb116316b9a788

…epos to avoid timeouts faceeae ci: Disable valgrind functionl tests on forked repos to avoid timeouts (MarcoFalke) Pull request description: Allows people to fork our repo and run the tests again Also print more cache stats ACKs for top commit: hebasto: ACK faceeae, tested on my own repo: https://travis-ci.org/github/hebasto/bitcoin/jobs/676257500 Tree-SHA512: 50e44edf94fcb997438eeaf7308b2b58a0854141ecb1fbb0ba7bf5ed662f19b60899f966f579cca90ad5e789234d0e90122d8c2c854da70339058adc3a475fa6

…-assets) under valgrind to catch memory errors 3f686d1 ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind to catch memory errors (practicalswift) Pull request description: Re-introduce the Travis valgrind fuzzing job which was removed by PR bitcoin#18899. The removal seems to have been made by accident since the removed job does not appear to be the source of the problem the PR set out to fix. --- Run fuzz testing [test cases (bitcoin-core/qa-assets)](https://github.com/bitcoin-core/qa-assets) under `valgrind`. This would have caught `util: Avoid potential uninitialized read in FormatISO8601DateTime(int64_t) by checking gmtime_s/gmtime_r return value` (bitcoin#18162) and similar cases. This fuzzing job was introduced in bitcoin#18166. Top commit has no ACKs. Tree-SHA512: 6e2681eb0ade6af465c5ea91ac163a337465d2130ec9880ba57a36d9af7c25682734586a32977dc25972d4f78483f339d680ea48c0ae13cf1dfa52b617aae401

fa5288c contrib: Fixup valgrind suppressions file (MarcoFalke) Pull request description: I am observing this one on bionic with system boost::fs: ``` { <insert_a_suppression_name_here> Memcheck:Cond fun:__wcsnlen_avx2 fun:wcsnrtombs fun:_ZNKSt7codecvtIwc11__mbstate_tE6do_outERS0_PKwS4_RS4_PcS6_RS6_ fun:_ZN5boost10filesystem11path_traits7convertEPKwS3_RNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKSt7codecvtIwc11__mbstate_tE fun:_ZN5boost10filesystem6detail11unique_pathERKNS0_4pathEPNS_6system10error_codeE ... ACKs for top commit: practicalswift: ACK fa5288c -- patch looks correct Tree-SHA512: 067e10e932a7f5b13e516134e0cfd3030265c1b582cdfde1cea97042e31399aa40c4590710a39429854c68ad703a0ae9f0b06e9af4cdd81e3cacb042939a84b6

``` ********* Finished testing of TrafficGraphDataTests ********* ~BitcoinApplication : Stopping thread ~BitcoinApplication : Stopped thread ==28475==WARNING: invalid path to external symbolizer! ==28475==WARNING: Failed to use and restart external symbolizer! ================================================================= ==28475==ERROR: LeakSanitizer: detected memory leaks Direct leak of 520 byte(s) in 13 object(s) allocated from: #0 0x55fc08e2353d (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d) #1 0x7f7a7724bd85 (<unknown module>) Direct leak of 432 byte(s) in 2 object(s) allocated from: #0 0x55fc08e2353d (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d) #1 0x55fc0dc63b63 (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0xc5d1b63) #2 0x8604597ff0e8feff (<unknown module>) Direct leak of 176 byte(s) in 2 object(s) allocated from: #0 0x55fc08e52cad (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x77c0cad) #1 0x55fc0d822372 (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0xc190372) Direct leak of 66 byte(s) in 3 object(s) allocated from: #0 0x55fc08e2353d (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d) #1 0x7f7a77243633 (<unknown module>) Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x55fc08e2353d (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d) #1 0x7f7a77245449 (<unknown module>) ```

Mainstream it has been done in bitcoin#14673 but not properly backported so far as we hadn't use travis Sanitizers that cannot find it print raw addresses instead of source locations, and a suppression file cannot match a stack of raw addresses. Current CI image registers update-alternatives slaves for llvm-ar, llvm-cov, llvm-nm, llvm-objdump, llvm-ranlib and llvm-strip, but not llvm-symbolizer, so /usr/bin/llvm-symbolizer does not exist. That fixes issue discovered in dashpay#5379: ********* Finished testing of TrafficGraphDataTests ********* ~BitcoinApplication : Stopping thread ~BitcoinApplication : Stopped thread ==28475==WARNING: invalid path to external symbolizer! ==28475==WARNING: Failed to use and restart external symbolizer! ================================================================= ==28475==ERROR: LeakSanitizer: detected memory leaks Direct leak of 520 byte(s) in 13 object(s) allocated from: #0 0x55fc08e2353d (/builds/dashpay/dash/build-ci/dashcore-linux64_asan/src/qt/test/test_dash-qt+0x779153d) #1 0x7f7a7724bd85 (<unknown module>)

knst force-pushed the bc-bp-valgrind-1 branch 2 times, most recently from 56e8596 to bd7b5ed Compare May 19, 2023 15:07

github-actions Bot added the needs rebase label May 24, 2023

knst force-pushed the bc-bp-valgrind-1 branch from f0000da to f51288d Compare May 25, 2023 12:26

github-actions Bot removed the needs rebase label May 25, 2023

knst force-pushed the bc-bp-valgrind-1 branch from f51288d to 75b0f91 Compare May 30, 2023 18:04

knst changed the title ~~backport: - add asan sanitizer (CI), valgrind and various CI changes~~ backport: bitcoin#14794, #17725, #18304, #18392, #18430, #18438, #18441, #18477, #18562, #18569, #18683, #18899, #18912, #19159, #19669 - asan/valgrind CI changes May 30, 2023

knst added this to the 20 milestone May 30, 2023

knst force-pushed the bc-bp-valgrind-1 branch from 8fe5328 to 844e017 Compare May 30, 2023 19:34

knst marked this pull request as ready for review May 31, 2023 07:49

knst requested review from PastaPastaPasta and UdjinM6 May 31, 2023 07:49

github-actions Bot added the needs rebase label May 31, 2023

UdjinM6 reviewed May 31, 2023

View reviewed changes

knst force-pushed the bc-bp-valgrind-1 branch from ff02a26 to e92819e Compare June 1, 2023 09:26

github-actions Bot removed the needs rebase label Jun 1, 2023

knst requested a review from UdjinM6 June 1, 2023 09:27

UdjinM6 approved these changes Jun 1, 2023

View reviewed changes

PastaPastaPasta approved these changes Jun 7, 2023

View reviewed changes

MarcoFalke and others added 8 commits June 7, 2023 01:50

fanquake and others added 11 commits June 7, 2023 01:50

partial merge bitcoin#14794: Add AddressSanitizer (ASan) Travis build

a3c59cc

fix: add supression for integer conversion in crc32 library

028281a

fix: rid of UB in qt/rpcconsole.cpp

6b87c21

fix: get rid UB from integer overflow in transactionview

c357460

PastaPastaPasta force-pushed the bc-bp-valgrind-1 branch from e92819e to 844ede2 Compare June 7, 2023 06:51

PastaPastaPasta merged commit ae9abb9 into dashpay:develop Jun 7, 2023

Navigation Menu

Uh oh!

backport: bitcoin#17725, #18304, #18392, #18430, #18438, #18441, #18477, #18562, #18569, #18683, #18899, #18912, #19159, #19669, partial #14794 - asan/valgrind CI changes - #5379

backport: bitcoin#17725, #18304, #18392, #18430, #18438, #18441, #18477, #18562, #18569, #18683, #18899, #18912, #19159, #19669, partial #14794 - asan/valgrind CI changes#5379
PastaPastaPasta merged 19 commits into
dashpay:developfrom
knst:bc-bp-valgrind-1

knst commented May 18, 2023 •

edited

Loading

Uh oh!

github-actions Bot commented May 24, 2023

Uh oh!

github-actions Bot commented May 31, 2023

Uh oh!

Uh oh!

UdjinM6 May 31, 2023

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Uh oh!

PastaPastaPasta left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

knst commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

github-actions Bot commented May 24, 2023

Uh oh!

github-actions Bot commented May 31, 2023

Uh oh!

Uh oh!

UdjinM6 May 31, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

knst commented May 18, 2023 •

edited

Loading