From a450681a8f75a9924705fc8481ad65a2316dc8e6 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 6 Dec 2019 23:32:16 -0500 Subject: [PATCH 01/13] Merge #17635: ci: Add CentOS 7 build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 711e0449cf4a0f15cabe0d64094e3add24ad44b0 ci: Remove trusty build (Hennadii Stepanov) 7f3ae224685efaeb6fe714de90e8871d12e55f34 ci: Add CentOS 7 build (Hennadii Stepanov) Pull request description: Arguably, CentOS is the most conservative distro of all the popular ones. Thus, it could be a good way to check the Bitcoin Core compatibility with aged dependencies. Currently, CentOS 7 has: - Berkeley DB == 4.8.30 - Boost == 1.53.0 - GCC == 4.8.5 - libevent == 2.0.21 < minimum required [2.0.22](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md), but tests passed - MiniUPnPc == 2.0 - Python == 3.6.8 - qrencode == 3.4.1 - Qt == 5.9.7 - ZeroMQ == 4.1.4 ~Please note that this PR is based on the bugfix #17634.~ Also trusty build has been removed for the following reasons: - https://github.com/bitcoin/bitcoin/issues/17628#issuecomment-559448201: > Maybe it'd make sense to replace Ubuntu Trusty with Centos 7 as the "check ancient backward compatibililty" Travis run. It's supported until 2024, apparently. - https://github.com/bitcoin/bitcoin/pull/17635#discussion_r354811792: > Our travis is currently running at its limit and this doesn't seem like it is adding a lot new coverage compared to the other builds. Close #17628 ACKs for top commit: MarcoFalke: ACK 711e0449cf4a0f15cabe0d64094e3add24ad44b0 🚠 Tree-SHA512: 614ec8394943f482a5867067f7119bffd052924a51e32ffda9a08e10c392c4a955a3539e2f8907cb65bfd9347dadf0ba62f6d1530bbc49927c347360a5a7f73c --- .travis.yml | 10 +++++----- ci/test/00_setup_env_native_centos.sh | 14 ++++++++++++++ ci/test/04_install.sh | 9 ++++++++- 3 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 ci/test/00_setup_env_native_centos.sh diff --git a/.travis.yml b/.travis.yml index 17b8773ae2..4037ffac7f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -233,16 +233,16 @@ after_success: env: >- FILE_ENV="./ci/test/00_setup_env_i686.sh" + - stage: test + name: 'x86_64 Linux [GOAL: install] [CentOS 7] [no depends, only system libs]' + env: >- + FILE_ENV="./ci/test/00_setup_env_native_centos.sh" + - stage: test name: 'x86_64 Linux [GOAL: install] [bionic] [uses qt5 dev package instead of depends Qt to speed up build and avoid timeout] [unsigned char]' env: >- FILE_ENV="./ci/test/00_setup_env_native_qt5.sh" # x86_64 Linux (xenial, no depends, only system libs, sanitizers: thread (TSan)) - - stage: test - name: 'x86_64 Linux [GOAL: install] [trusty] [no functional tests, no depends, only system libs]' - env: >- - FILE_ENV="./ci/test/00_setup_env_native_trusty.sh" - - stage: test name: 'x86_64 Linux [GOAL: install] [xenial] [no depends, only system libs, sanitizers: thread (TSan), no wallet]' env: >- diff --git a/ci/test/00_setup_env_native_centos.sh b/ci/test/00_setup_env_native_centos.sh new file mode 100644 index 0000000000..56b915b6c7 --- /dev/null +++ b/ci/test/00_setup_env_native_centos.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +export LC_ALL=C.UTF-8 + +export DOCKER_NAME_TAG=centos:7 +export DOCKER_PACKAGES="gcc-c++ libtool make git python3 python36-zmq" +export PACKAGES="boost-devel libevent-devel libdb4-devel libdb4-cxx-devel miniupnpc-devel zeromq-devel qt5-qtbase-devel qt5-qttools-devel qrencode-devel" +export NO_DEPENDS=1 +export GOAL="install" +export BITCOIN_CONFIG="--enable-reduce-exports" diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 5a87cf06d9..21df74f483 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -6,6 +6,10 @@ export LC_ALL=C.UTF-8 +if [[ $DOCKER_NAME_TAG == centos* ]]; then + export LC_ALL=en_US.utf8 +fi + if [ "$TRAVIS_OS_NAME" == "osx" ]; then set +o errexit pushd /usr/local/Homebrew || exit 1 @@ -81,7 +85,10 @@ if [ -n "$DPKG_ADD_ARCH" ]; then DOCKER_EXEC dpkg --add-architecture "$DPKG_ADD_ARCH" fi -if [ "$TRAVIS_OS_NAME" != "osx" ]; then +if [[ $DOCKER_NAME_TAG == centos* ]]; then + ${CI_RETRY_EXE} DOCKER_EXEC yum -y install epel-release + ${CI_RETRY_EXE} DOCKER_EXEC yum -y install $DOCKER_PACKAGES $PACKAGES +elif [ "$TRAVIS_OS_NAME" != "osx" ]; then ${CI_RETRY_EXE} DOCKER_EXEC apt-get update ${CI_RETRY_EXE} DOCKER_EXEC apt-get install --no-install-recommends --no-upgrade -y $PACKAGES $DOCKER_PACKAGES fi From c550648bd076bec9ce724a9e81b87641c434c613 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 2 Jan 2020 13:51:30 -0500 Subject: [PATCH 02/13] Merge #17849: ci: Fix brew python link 87744b16b02cb9e4f6e97509facf6cc781e60b98 ci: Fix brew python link (Hennadii Stepanov) Pull request description: During the native macOS build on Travis brew-version python update from 3.7.5 to 3.7.6_1 causes link failure: ``` ==> Upgrading python3 ==> Downloading https://homebrew.bintray.com/bottles/python-3.7.6_1.mojave.bottl ==> Downloading from https://akamai.bintray.com/64/643d627c2b4fc03a3286c397d2992 ######################################################################## 100.0% ==> Pouring python-3.7.6_1.mojave.bottle.tar.gz Error: The `brew link` step did not complete successfully The formula built, but is not symlinked into /usr/local ``` Close #17848 Top commit has no ACKs. Tree-SHA512: 09164805c557e3bd21df2d0765a1c6815e786040e9ec0e81a916b2df6c4f03974cf92c31eca999b997f8c4ed0998bdd6e35c3de7ccbaaed3bf131521ecc637dc --- ci/test/04_install.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 21df74f483..f81aabfa51 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -16,6 +16,7 @@ if [ "$TRAVIS_OS_NAME" == "osx" ]; then git reset --hard origin/master popd || exit 1 set -o errexit + ${CI_RETRY_EXE} brew unlink python@2 ${CI_RETRY_EXE} brew update # brew upgrade returns an error if any of the packages is already up to date # Failure is safe to ignore, unless we really need an update. From be35f1bc6716089280ed6cbd7609463a1183ec76 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 5 Feb 2020 10:41:36 +0000 Subject: [PATCH 03/13] Merge #18069: test: replace 'regtest' leftovers by self.chain eca56f89293b74f11ca631ff2a0793e970e65841 test: replace 'regtest' leftovers by self.chain (Sebastian Falbesoner) Pull request description: This is a follow-up PR to #16681 (fixes #18068), replacing all remaining hardcoded `"regtest"` strings in functional tests by `self.chain`. Top commit has no ACKs. Tree-SHA512: 96524649b33164938e5a95215991103ed7855ebab55ef788d4816b3fa5cbc03d8f3b0d39f2247a87522f289fd7f4daf25e059900b8462b5127eb154bbee89054 --- test/functional/feature_config_args.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index dd33a4bfb9..18d7a13edb 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -94,8 +94,8 @@ class ConfArgsTest(BitcoinTestFramework): 'Command-line arg: rpcpassword=****', 'Command-line arg: rpcuser=****', 'Command-line arg: torpassword=****', - 'Config file arg: regtest="1"', - 'Config file arg: [regtest] server="1"', + 'Config file arg: %s="1"' % self.chain, + 'Config file arg: [%s] server="1"' % self.chain, ], unexpected_msgs=[ 'alice:f7efda5c189b999524f151318c0c86$d5b51b3beffbc0', From 87f1f84068ea04cfa5c664fff02a83ed397ea2bf Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 8 Jun 2020 19:35:57 +0800 Subject: [PATCH 04/13] Merge #19194: util: Don't reference errno when pthread fails. cb38b069b0f41b1a26264784b1c1303c8ac6ab08 util: Don't reference errno when pthread fails. (MIZUTA Takeshi) Pull request description: Pthread library does not set errno. Pthread library's errno is returned by return value. ACKs for top commit: practicalswift: ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08 -- patch looks correct MarcoFalke: review ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08 hebasto: ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08, only squashed commits since the [previous](https://github.com/bitcoin/bitcoin/pull/19194#pullrequestreview-425831739) review. Tree-SHA512: e6c950e30726e5031db97a7b84c8a9215da5ad3e5d233bcc349f812ad15957ddfe378e26d18339b9e0a5dcac2f50b47a687b87a6a6beaf6139df84f31531321e --- src/util/system.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index 5eef0abf75..387b27a187 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1344,8 +1344,9 @@ void ScheduleBatchPriority() { #ifdef SCHED_BATCH const static sched_param param{}; - if (pthread_setschedparam(pthread_self(), SCHED_BATCH, ¶m) != 0) { - LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(errno)); + const int rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, ¶m); + if (rc != 0) { + LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(rc)); } #endif } From 77ab447b7f00882de70c17eb9ab4269c459821de Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 10 Jul 2020 22:51:52 +0200 Subject: [PATCH 05/13] Merge #19140: tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 20d31bdd92cc2ad9b8d26ed80da73bbcd6016144 tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift) Pull request description: Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from #19074 as suggested by MarcoFalke. The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness. The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path. `" http:// HTTP/1.1\n"` was a crashing input prior to this workaround. Before this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input AddressSanitizer:DEADLYSIGNAL ================================================================= ==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0) ==27905==The signal is caused by a READ memory access. ==27905==Hint: address points to the zero page. #0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37 #1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7 #2 0x55a1687f624e in test_one_input(std::vector > const&) src/test/fuzz/http_request.cpp:51:9 … $ echo $? 1 ``` After this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input Executed input in 0 ms *** *** NOTE: fuzzing was not performed, you have only *** executed the target code on a fixed set of inputs. *** $ echo $? 0 ``` See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) Top commit has no ACKs. Tree-SHA512: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304 --- src/test/fuzz/http_request.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index 4104c5574d..e588757abc 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -31,7 +32,14 @@ void test_one_input(const std::vector& buffer) assert(evbuf != nullptr); const std::vector http_buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, 4096); evbuffer_add(evbuf, http_buffer.data(), http_buffer.size()); - if (evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) { + // Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering + // a nullptr dereference. The dereference (req->evcon->http_server) takes place in evhttp_parse_request_line + // and is a consequence of our hacky but necessary use of the internal function evhttp_parse_firstline_ in + // this fuzzing harness. The workaround is not aesthetically pleasing, but it successfully avoids the troublesome + // code path. " http:// HTTP/1.1\n" was a crashing input prior to this workaround. + const std::string http_buffer_str = ToLower({http_buffer.begin(), http_buffer.end()}); + if (http_buffer_str.find(" http://") != std::string::npos || http_buffer_str.find(" https://") != std::string::npos || + evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) { evbuffer_free(evbuf); evhttp_request_free(evreq); return; From 23539206625936d9aa7ed73c79034777e9fa4f1d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 21 Jul 2020 09:37:20 +0200 Subject: [PATCH 06/13] Merge #16878: Fix non-deterministic coverage of test DoS_mapOrphans 4455949d6f0218b40d33d7fe6de6555f8f62192f Make test DoS_mapOrphans deterministic (David Reikher) Pull request description: This pull request proposes a solution to make the test `DoS_mapOrphans` in denialofservice_tests.cpp have deterministic coverage. The `RandomOrphan` function in denialofservice_tests.cpp and the implicitly called function `ecdsa_signature_parse_der_lax` in pubkey.cpp were causing the non-deterministic test coverage. In the former, if a random orphan was selected the index of which is bigger than the max. orphan index in `mapOrphanTransactions`, the last orphan was returned from `RandomOrphan`. If the random number generated was never large enough, this condition would not be fulfilled and the corresponding branch wouldn't run. The proposed solution is to force one of the 50 dependant orphans to depend on the last orphan in `mapOrphanTransactions` using the newly introduced function `OrphanByIndex` (and passing it a large uint256), forcing this branch to run at least once. In the latter, if values for ECDSA `R` or `S` (or both) had no leading zeros, some code would not be executed. The solution was to find a constant signature that would be comprised of `R` and `S` values with leading zeros and calling `CPubKey::Verify` at the end of the test with this signature forcing this code to always run at least once at the end even if it hadn't throughout the test. To test that the coverage is (at least highly likely) deterministic, I ran `contrib/devtools/test_deterministic_coverage.sh denialofservice_tests/DoS_mapOrphans 1000` and the result was deterministic coverage across 1000 runs. Also - removed denialofservice_tests test entry from the list of non-deterministic tests in the coverage script. ACKs for top commit: MarcoFalke: ACK 4455949d6f0218b40d33d7fe6de6555f8f62192f Tree-SHA512: 987eb1f94b80d5bec4d4944e91ef43b9b8603055750362d4b4665b7f011be27045808aa9f4c6ccf8ae009b61405f9a1b8671d65a843c3328e5b8acce1f1c00a6 --- .../devtools/test_deterministic_coverage.sh | 1 - src/test/denialofservice_tests.cpp | 20 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/contrib/devtools/test_deterministic_coverage.sh b/contrib/devtools/test_deterministic_coverage.sh index 1418ebb140..0ce0d3261b 100755 --- a/contrib/devtools/test_deterministic_coverage.sh +++ b/contrib/devtools/test_deterministic_coverage.sh @@ -17,7 +17,6 @@ GCOV_EXECUTABLE="gcov" NON_DETERMINISTIC_TESTS=( "blockfilter_index_tests/blockfilter_index_initial_sync" # src/checkqueue.h: In CCheckQueue::Loop(): while (queue.empty()) { ... } "coinselector_tests/knapsack_solver_test" # coinselector_tests.cpp: if (equal_sets(setCoinsRet, setCoinsRet2)) - "denialofservice_tests/DoS_mapOrphans" # denialofservice_tests.cpp: it = mapOrphanTransactions.lower_bound(InsecureRand256()); "fs_tests/fsbridge_fstream" # deterministic test failure? "miner_tests/CreateNewBlock_validity" # validation.cpp: if (GetMainSignals().CallbacksPending() > 10) "scheduler_tests/manythreads" # scheduler.cpp: CScheduler::serviceQueue() diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 7c524d60fe..385032aa97 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -4,10 +4,12 @@ // Unit tests for denial-of-service detection/prevention code +#include #include #include #include #include +#include #include