From 8f5d07ec825091719f27bb9ffc778064ff3a0570 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 1 Nov 2019 13:17:24 -0400 Subject: [PATCH 01/17] Merge #17284: build: update retry to current version 58d0393bec7680933702b76cd3f1d1e33030ed16 build: update retry to current version (randymcmillann) Pull request description: This commit eliminates spelling and white space errors that are flagged in the linting process ACKs for top commit: practicalswift: ACK 58d0393bec7680933702b76cd3f1d1e33030ed16 Tree-SHA512: c241ed0775026c890dd29d1f7231c5540e9c9285867a99844605753a3007d08f0bd4f7a59f078e4c65b741301ff7fa8a871e2e3c64b9a9fe47b3ea74c4228498 --- ci/retry/README.md | 4 ++-- ci/retry/retry | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ci/retry/README.md b/ci/retry/README.md index 983a498070..1b03c652db 100644 --- a/ci/retry/README.md +++ b/ci/retry/README.md @@ -32,8 +32,8 @@ Help: -v, --verbose Verbose output -t, --tries=# Set max retries: Default 10 -s, --sleep=secs Constant sleep amount (seconds) - -m, --min=secs Exponenetial Backoff: minimum sleep amount (seconds): Default 0.3 - -x, --max=secs Exponenetial Backoff: maximum sleep amount (seconds): Default 60 + -m, --min=secs Exponential Backoff: minimum sleep amount (seconds): Default 0.3 + -x, --max=secs Exponential Backoff: maximum sleep amount (seconds): Default 60 -f, --fail="script +cmds" Fail Script: run in case of final failure ### Examples diff --git a/ci/retry/retry b/ci/retry/retry index 0e5f6e9701..3c06519dbd 100755 --- a/ci/retry/retry +++ b/ci/retry/retry @@ -17,7 +17,7 @@ __log_out() { echo "$1" 1>&2 } -# Paramters: max_tries min_sleep max_sleep constant_sleep fail_script EXECUTION_COMMAND +# Parameters: max_tries min_sleep max_sleep constant_sleep fail_script EXECUTION_COMMAND retry() { local max_tries="$1"; shift @@ -83,8 +83,8 @@ Usage: $retry [options] -- execute command -v, --verbose Verbose output -t, --tries=# Set max retries: Default 10 -s, --sleep=secs Constant sleep amount (seconds) - -m, --min=secs Exponenetial Backoff: minimum sleep amount (seconds): Default 0.3 - -x, --max=secs Exponenetial Backoff: maximum sleep amount (seconds): Default 60 + -m, --min=secs Exponential Backoff: minimum sleep amount (seconds): Default 0.3 + -x, --max=secs Exponential Backoff: maximum sleep amount (seconds): Default 60 -f, --fail="script +cmds" Fail Script: run in case of final failure EOF } From baecb5f73d54db9d5dd50ca5e9aa8f3e30f3d717 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 18 Nov 2019 09:35:04 -0500 Subject: [PATCH 02/17] Merge #17503: doc: Remove bitness from bitcoin-qt help message and manpage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit e161bc74d24a381c313aecb950d3b8411e0ed19d doc: Remove bitness from bitcoin-qt help message and manpage (Wladimir J. van der Laan) Pull request description: Remove the `(64-bit)` from the bitcoin-qt help message. Since removing the Windows 32-bit builds, it is no longer information that is often useful for troubleshooting. This never worked for other architectures than x86, and the only 32-bit x86 build left is the Linux one. Linux users tend to know what architecture they are using. It also accidentally ends up in the bitcoin-qt manpage (if you happen to be generating them on a x86 machine), which gets checked in. See for example https://github.com/bitcoin/bitcoin/commit/1bc9988993ee84bc814e5a7f33cc90f670a19f6a#diff-e4b84be382c8ea33b83203ceb8c85296 ACKs for top commit: practicalswift: ACK e161bc74d24a381c313aecb950d3b8411e0ed19d -- rationale makes sense and diff looks correct :) MarcoFalke: Tested ACK e161bc74d24a381c313aecb950d3b8411e0ed19d 🔮 Tree-SHA512: d38754903252896dc86fac6c12ad6615d322c2744db7c02b18574a08c69e8876b2c905e1f09b324002236b111ee93479f89769c562e7b3b2e6eb2992d76464ef --- src/qt/utilitydialog.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index a5928612d2..5b56092928 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -41,14 +41,6 @@ HelpMessageDialog::HelpMessageDialog(interfaces::Node& node, QWidget *parent, He GUIUtil::updateFonts(); QString version = QString{PACKAGE_NAME} + " " + tr("version") + " " + QString::fromStdString(FormatFullVersion()); - /* On x86 add a bit specifier to the version so that users can distinguish between - * 32 and 64 bit builds. On other architectures, 32/64 bit may be more ambiguous. - */ -#if defined(__x86_64__) - version += " " + tr("(%1-bit)").arg(64); -#elif defined(__i386__ ) - version += " " + tr("(%1-bit)").arg(32); -#endif if (helpMode == about) { From 3f4b5f9fa12b8e4e2bfcb3f5d83832ce13d91f1f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 25 Nov 2019 14:57:37 -0500 Subject: [PATCH 03/17] Merge #17591: ci: Add big endian platform - s390x da1f153e5e260f1744ee1bf4f24ca3a74ffea465 Add s390x tests to travis (Elichai Turkel) 2fa65e0de94f01d502e8ace89be3c5dc963dd764 Add ci script to install on s390x (Elichai Turkel) Pull request description: Discovered this as part of #17402 and a conversation with gmaxwell. You can see here that the platform is indeed BE: https://travis-ci.org/elichai/bitcoin/jobs/616656410#L36 This closes https://github.com/bitcoin/bitcoin/issues/6466 ACKs for top commit: MarcoFalke: ACK da1f153e5e260f1744ee1bf4f24ca3a74ffea465 Tree-SHA512: e7e94e54e220257d91b24fddc79eab2bcaaadf0b2d1e7e6872d9757808ab2541728f00b1f3ab7e343305c0e7d91bb48a17a3f9621f6fff6c9fe6cde6682de408 --- .travis.yml | 6 ++++++ ci/test/00_setup_env_s390x.sh | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 ci/test/00_setup_env_s390x.sh diff --git a/.travis.yml b/.travis.yml index 36db188232..bd5f2d7968 100644 --- a/.travis.yml +++ b/.travis.yml @@ -229,6 +229,12 @@ after_success: env: >- FILE_ENV="./ci/test/00_setup_env_arm.sh" + - stage: test + name: 'S390x [GOAL: install] [unit tests, functional tests]' + arch: s390x + env: >- + FILE_ENV="./ci/test/00_setup_env_s390x.sh" + - stage: test name: 'Win64 [GOAL: deploy] [unit tests, no gui, no functional tests]' env: >- diff --git a/ci/test/00_setup_env_s390x.sh b/ci/test/00_setup_env_s390x.sh new file mode 100644 index 0000000000..b41d44c61a --- /dev/null +++ b/ci/test/00_setup_env_s390x.sh @@ -0,0 +1,18 @@ +#!/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 HOST=s390x-unknown-linux-gnu +export DOCKER_NAME_TAG=s390x/ubuntu:18.04 +export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" +export NO_DEPENDS=1 +export RUN_UNIT_TESTS=true +export RUN_FUNCTIONAL_TESTS=false +export GOAL="install" +export BITCOIN_CONFIG="--enable-reduce-exports --with-incompatible-bdb" + +lscpu From 0e898ee80d097baef0122e2fd3eadad71838610b Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 29 Jan 2020 20:37:13 +0800 Subject: [PATCH 04/17] Merge #18008: test: only declare a main() when fuzzing with AFL b35567fe0ba3e6f8d8f9525088eb8ee62065ad01 test: only declare a main() when fuzzing with AFL (fanquake) Pull request description: This fixes fuzzing using [libFuzzer](https://llvm.org/docs/LibFuzzer.html) on macOS, which caused a few issues during the recent review club. macOS users could only fuzz using afl, or inside a VM. It seems that the `__attribute__((weak))` marking is not quite enough to properly mark `main()` as weak on macOS. See Apples docs on [Frameworks and Weak Linking](https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html#//apple_ref/doc/uid/20002378-107262-CJBJAEID). Have tested fuzzing using libFuzzer and AFL with this patch. ACKs for top commit: MarcoFalke: ACK b35567fe0ba3e6f8d8f9525088eb8ee62065ad01 fjahr: ACK b35567f Tree-SHA512: b881fdd98c7e1587fcf44debd31f5e7a52df938059ab91c41d0785077b3329b793e051a2bf2eee64488b9f6029d9288c911052ec23ab3ab8c0561a2be1682dae --- src/test/fuzz/fuzz.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index 3984f68116..9d8a89ce01 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -12,6 +12,7 @@ const std::function G_TEST_LOG_FUN{}; +#if defined(__AFL_COMPILER) static bool read_stdin(std::vector& data) { uint8_t buffer[1024]; @@ -21,6 +22,7 @@ static bool read_stdin(std::vector& data) } return length == 0; } +#endif // Default initialization: Override using a non-weak initialize(). __attribute__((weak)) void initialize() @@ -42,9 +44,9 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv) return 0; } -// Declare main(...) "weak" to allow for libFuzzer linking. libFuzzer provides -// the main(...) function. -__attribute__((weak)) int main(int argc, char** argv) +// Generally, the fuzzer will provide main(), except for AFL +#if defined(__AFL_COMPILER) +int main(int argc, char** argv) { initialize(); #ifdef __AFL_INIT @@ -72,3 +74,4 @@ __attribute__((weak)) int main(int argc, char** argv) #endif return 0; } +#endif From 2388159c5bec43329e2925b0a180dbae96455d47 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 5 Mar 2020 15:08:18 -0500 Subject: [PATCH 05/17] Merge #18109: tests: Avoid hitting some known minor tinyformat issues when fuzzing strprintf(...) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 470e2ac602ed2d6e62e5c80f27cd0a60c7cf6bce tests: Avoid hitting some known minor tinyformat issues when fuzzing strprintf(...) (practicalswift) Pull request description: Avoid hitting some known minor tinyformat issues when fuzzing `strprintf(...)`. These can be removed when the issues have been resolved upstreams :) Note to reviewers: The `%c` and `%*` issues are also present for `%c` and `%*`. That is why simply matching on `"%c"` or `"%*"` is not enough. Note that the intentionally trivial skipping logic overshoots somewhat (`c[…]%` is filtered in addition to `%[…]c`). Top commit has no ACKs. Tree-SHA512: 2b002981e8b3f2ee021c3013f1260654ac7e158699313849c9e9660462bb8cd521544935799bb8daa74925959dc04d63440e647495e0b008cfe1b8a8b2202d40 --- src/test/fuzz/strprintf.cpp | 47 ++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/test/fuzz/strprintf.cpp b/src/test/fuzz/strprintf.cpp index 0de21f0e7c..d5be1070bd 100644 --- a/src/test/fuzz/strprintf.cpp +++ b/src/test/fuzz/strprintf.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -27,7 +26,7 @@ void test_one_input(const std::vector& buffer) // * strprintf("%.222222200000000$", 1.1); // // Upstream bug report: https://github.com/c42f/tinyformat/issues/70 - if (format_string.find("%") != std::string::npos && digits_in_format_specifier >= 7) { + if (format_string.find('%') != std::string::npos && digits_in_format_specifier >= 7) { return; } @@ -35,7 +34,7 @@ void test_one_input(const std::vector& buffer) // * strprintf("%1$*1$*", -11111111); // // Upstream bug report: https://github.com/c42f/tinyformat/issues/70 - if (format_string.find("%") != std::string::npos && format_string.find("$") != std::string::npos && format_string.find("*") != std::string::npos && digits_in_format_specifier > 0) { + if (format_string.find('%') != std::string::npos && format_string.find('$') != std::string::npos && format_string.find('*') != std::string::npos && digits_in_format_specifier > 0) { return; } @@ -96,7 +95,7 @@ void test_one_input(const std::vector& buffer) } try { - switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 13)) { + switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 5)) { case 0: (void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32)); break; @@ -115,32 +114,52 @@ void test_one_input(const std::vector& buffer) case 5: (void)strprintf(format_string, fuzzed_data_provider.ConsumeBool()); break; - case 6: + } + } catch (const tinyformat::format_error&) { + } + + if (format_string.find('%') != std::string::npos && format_string.find('c') != std::string::npos) { + // Avoid triggering the following: + // * strprintf("%c", 1.31783e+38); + // tinyformat.h:244:36: runtime error: 1.31783e+38 is outside the range of representable values of type 'char' + return; + } + + if (format_string.find('%') != std::string::npos && format_string.find('*') != std::string::npos) { + // Avoid triggering the following: + // * strprintf("%*", -2.33527e+38); + // tinyformat.h:283:65: runtime error: -2.33527e+38 is outside the range of representable values of type 'int' + // * strprintf("%*", -2147483648); + // tinyformat.h:763:25: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself + return; + } + + try { + switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 7)) { + case 0: (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint()); break; - case 7: + case 1: (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint()); break; - case 8: + case 2: (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral()); break; - case 9: + case 3: (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral()); break; - case 10: + case 4: (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral()); break; - case 11: + case 5: (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral()); break; - case 12: + case 6: (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral()); break; - case 13: + case 7: (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral()); break; - default: - assert(false); } } catch (const tinyformat::format_error&) { } From 68cecee5580cf41848cc3320dd8a1b21934ae6d1 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 3 Apr 2020 05:06:09 +0800 Subject: [PATCH 06/17] Merge #18464: doc: block-relay-only vs blocksonly fa6e01f2a163511a735088895ab02232b150801b doc: block-relay-only is not blocksonly (MarcoFalke) Pull request description: Those are different concepts, see https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.19.0.1.md#p2p-changes for the block-relay-only nodes. ACKs for top commit: jonatack: ACK fa6e01f hebasto: ACK fa6e01f2a163511a735088895ab02232b150801b Tree-SHA512: 6de2c81201b62ed59e504a3a6f164068600182e1bbf63eda7f9db3160507bdba091c13882ee0e75e713f0832bfaf5973a86eba3b94588d5b72196f05ae0a9c9a --- doc/reduce-memory.md | 4 +++- doc/reduce-traffic.md | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/reduce-memory.md b/doc/reduce-memory.md index 18988c1d8d..3c71416434 100644 --- a/doc/reduce-memory.md +++ b/doc/reduce-memory.md @@ -24,7 +24,9 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa ## Number of peers -- `-maxconnections=` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming connections are enabled, otherwise the number of connections will never be more than `10`. Of the 10 outbound peers, there can be 8 full outgoing connections and 2 -blocksonly peers, in which case they are block/addr peers, but not tx peers. +- `-maxconnections=` - the maximum number of connections, this defaults to 125. Each active connection takes up some + memory. This option applies only if incoming connections are enabled, otherwise the number of connections will never + be more than 10. Of the 10 outbound peers, there can be 8 full-relay connections and 2 block-relay-only ones. ## Thread configuration diff --git a/doc/reduce-traffic.md b/doc/reduce-traffic.md index fd0f44358a..b9527dfbd1 100644 --- a/doc/reduce-traffic.md +++ b/doc/reduce-traffic.md @@ -5,8 +5,8 @@ Some node operators need to deal with bandwidth caps imposed by their ISPs. By default, Dash Core allows up to 125 connections to different peers, 10 of which are outbound. You can therefore, have at most 115 inbound connections. -Of the 10 outbound peers, there can be 8 full outgoing connections and 2 with -the -blocksonly mode turned on. You can therefore, have at most 115 inbound connections. +Of the 10 outbound peers, there can be 8 full-relay connections and 2 +block-relay-only ones. The default settings can result in relatively significant traffic consumption. From 27a96eedb1438ec2dce2327e7604924511adc404 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 4 Apr 2020 03:07:57 +0800 Subject: [PATCH 07/17] Merge #18508: RPC: Fix more formatting nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f32ab443a98e622afa601372454310aef1f380be Bugfix: RPC: JSON null is not "None" (Luke Dashjr) 26dcf3958187cbdc9ffc9438a5eebfcaf607f7e9 Bugfix: RPC: Don't use a continuation elipsis after an elision elipsis (Luke Dashjr) eca65caadcddf43b2dace111bd7e4b0a2a9556c2 Bugfix: RPC: Add missing commas and correct indentation of explicit ELISION (Luke Dashjr) Pull request description: 1. listsinceblock had a double ellipsis (elision + continuation); this looks ugly, just one is needed. 2. Elision ellipsis wasn't getting a comma, so was truncated to `".."` by comma-removal code. 3. Elision ellipsis was indented incorrectly (as if it was a subitem). 4. Similarly, type "none" would get truncated to `"Non"`, when it should really be `"null"` anyway. ACKs for top commit: MarcoFalke: ACK f32ab443a98e622afa601372454310aef1f380be 🐰 Tree-SHA512: 34e1c72673790ed11cdee838d64ea5e0ac498de19258df99d54b5322e003060123c65ad27ac2fd4729a1dfe52066a0629602a132b1ef85d4154affd99a065a3f --- src/rpc/util.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 87c5c62bbd..ade02f36ad 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -618,11 +618,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const switch (m_type) { case Type::ELISION: { // If the inner result is empty, use three dots for elision - sections.PushSection({indent_next + "...", m_description}); + sections.PushSection({indent + "..." + maybe_separator, m_description}); return; } case Type::NONE: { - sections.PushSection({indent + "None", Description("json null")}); + sections.PushSection({indent + "null" + maybe_separator, Description("json null")}); return; } case Type::STR: { @@ -655,10 +655,10 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const for (const auto& i : m_inner) { i.ToSections(sections, OuterType::ARR, current_indent + 2); } - if (m_type == Type::ARR) { + CHECK_NONFATAL(!m_inner.empty()); + if (m_type == Type::ARR && m_inner.back().m_type != Type::ELISION) { sections.PushSection({indent_next + "...", ""}); } else { - CHECK_NONFATAL(!m_inner.empty()); // Remove final comma, which would be invalid JSON sections.m_sections.back().m_left.pop_back(); } @@ -671,11 +671,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const for (const auto& i : m_inner) { i.ToSections(sections, OuterType::OBJ, current_indent + 2); } - if (m_type == Type::OBJ_DYN) { + CHECK_NONFATAL(!m_inner.empty()); + if (m_type == Type::OBJ_DYN && m_inner.back().m_type != Type::ELISION) { // If the dictionary keys are dynamic, use three dots for continuation sections.PushSection({indent_next + "...", ""}); } else { - CHECK_NONFATAL(!m_inner.empty()); // Remove final comma, which would be invalid JSON sections.m_sections.back().m_left.pop_back(); } From bd85d875f3662e489cd96794055ce59ad7d11f8e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 29 Apr 2020 11:14:10 -0400 Subject: [PATCH 08/17] Merge #18798: ci: Fix default retry script usage 45615de26caa4c8ffeacc558143aaf6887cbb314 ci: Fix default retry script usage (Hennadii Stepanov) Pull request description: On master (5352d14b3796d9e672a20ada8f7613a70fe448f4) `CI_RETRY_EXE=${CI_RETRY_EXE:retry}` works as a [Substring Expansion](https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html), and that is wrong. If `CI_RETRY_EXE` variable was unset initially, its new value becomes an empty string, but not "retry" as one could expect. Consequently, the `${CI_RETRY_EXE} ...` command does _not_ use `ci/retry/retry` script. This PR makes for `CI_RETRY_EXE` variable a usual parameter expansion, i.e., `${parameter:-word}`. Reference: https://github.com/bitcoin/bitcoin/pull/18735#issuecomment-620095489 Top commit has no ACKs. Tree-SHA512: 108173f6b2677979b9ddf2f9b9df4a6c56f5efa81c36543a1816bb3b984e42984bf3c83fe413ea3a5ca1e2317c4efb02fea7180a6b44863af7cfe6202e9cf94d --- ci/test/00_setup_env.sh | 2 +- ci/test/04_install.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index 8bc50da2c3..1c7428f863 100755 --- a/ci/test/00_setup_env.sh +++ b/ci/test/00_setup_env.sh @@ -45,4 +45,4 @@ export DOCKER_PACKAGES=${DOCKER_PACKAGES:-build-essential libtool autotools-dev export GOAL=${GOAL:-install} export DIR_QA_ASSETS=${DIR_QA_ASSETS:-${BASE_BUILD_DIR}/qa-assets} export PATH=${BASE_ROOT_DIR}/ci/retry:$PATH -export CI_RETRY_EXE=${CI_RETRY_EXE:retry} +export CI_RETRY_EXE=${CI_RETRY_EXE:-"retry --"} diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 4e200ba1d8..3359c82b6c 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -39,6 +39,7 @@ else bash -c "export PATH=$BASE_SCRATCH_DIR/bins/:\$PATH && cd $PWD && $*" } fi +export -f DOCKER_EXEC DOCKER_EXEC free -m -h DOCKER_EXEC echo "Number of CPUs \(nproc\):" \$\(nproc\) From 50d20fe78871106eaedf4cb345356cbc2522b830 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Jun 2020 14:38:34 -0400 Subject: [PATCH 09/17] Merge #19366: tests: Provide main(...) function in fuzzer. Allow building uninstrumented harnesses with --enable-fuzz. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1087807b2bc56b9c7e7a5471c83f6ecfae79b048 tests: Provide main(...) function in fuzzer (practicalswift) Pull request description: Provide `main(...)` function in fuzzer. Allow building uninstrumented harnesses with only `--enable-fuzz`. This PR restores the behaviour to how things worked prior to #18008. #18008 worked around an macOS specific issue but did it in a way which unnecessarily affected platforms not in need of the workaround :) Before this patch: ``` # Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation) $ ./configure --enable-fuzz $ make CXXLD test/fuzz/span /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/Scrt1.o: In function `_start': (.text+0x20): undefined reference to `main' collect2: error: ld returned 1 exit status Makefile:7244: recipe for target 'test/fuzz/span' failed make[2]: *** [test/fuzz/span] Error 1 make[2]: *** Waiting for unfinished jobs.... $ ``` After this patch: ``` # Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation) $ ./configure --enable-fuzz $ make $ echo foo | src/test/fuzz/span $ ``` The examples above show the change in non-macOS functionality. macOS functionality is unaffected by this patch. ACKs for top commit: MarcoFalke: ACK 1087807b2bc56b9c7e7a5471c83f6ecfae79b048 Tree-SHA512: 9c16ea32ffd378057c4fae9d9124636d11e3769374d340f68a1b761b9e3e3b8a33579e60425293c96b8911405d8b96ac3ed378e669ea4c47836af06892aca73d --- src/test/fuzz/fuzz.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index 9d8a89ce01..f5fbe87bad 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -12,7 +12,16 @@ const std::function G_TEST_LOG_FUN{}; -#if defined(__AFL_COMPILER) +// Decide if main(...) should be provided: +// * AFL needs main(...) regardless of platform. +// * macOS handles __attribute__((weak)) main(...) poorly when linking +// against libFuzzer. See https://github.com/bitcoin/bitcoin/pull/18008 +// for details. +#if defined(__AFL_COMPILER) || !defined(MAC_OSX) +#define PROVIDE_MAIN_FUNCTION +#endif + +#if defined(PROVIDE_MAIN_FUNCTION) static bool read_stdin(std::vector& data) { uint8_t buffer[1024]; @@ -44,9 +53,8 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv) return 0; } -// Generally, the fuzzer will provide main(), except for AFL -#if defined(__AFL_COMPILER) -int main(int argc, char** argv) +#if defined(PROVIDE_MAIN_FUNCTION) +__attribute__((weak)) int main(int argc, char** argv) { initialize(); #ifdef __AFL_INIT From a61b615218f4487f503698cb83c70c9348a8a9eb Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 29 Jun 2020 15:17:38 +0200 Subject: [PATCH 10/17] Merge #19367: doc: Span pitfalls fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 doc: Mention Span in developer-notes.md (Pieter Wuille) 3502a60418858a8281ddf2f9cd59daa8f01d2fa8 doc: Document Span pitfalls (Pieter Wuille) Pull request description: This is an attempt to document pitfalls with the use of `Span`, following up on comments like https://github.com/bitcoin/bitcoin/pull/18468#issuecomment-622846597 and https://github.com/bitcoin/bitcoin/pull/18468#discussion_r442998211 ACKs for top commit: laanwj: ACK fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 Tree-SHA512: 8f6f277d6d88921852334853c2b7ced97e83d3222ce40c9fe12dfef508945f26269b90ae091439ebffddf03f939797cb28126b2387f77959069ef8909c25ab53 --- doc/developer-notes.md | 13 ++++++++++ src/span.h | 56 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 34933ec09d..87e4292697 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -582,6 +582,19 @@ class A int. If the signed int is some negative `N`, it'll become `INT_MAX - N` which might cause unexpected consequences. +- Use `Span` as function argument when it can operate on any range-like container. + + - *Rationale*: Compared to `Foo(const vector&)` this avoids the need for a (potentially expensive) + conversion to vector if the caller happens to have the input stored in another type of container. + However, be aware of the pitfalls documented in [span.h](../src/span.h). + +```cpp +void Foo(Span data); + +std::vector vec{1,2,3}; +Foo(vec); +``` + - Prefer `enum class` (scoped enumerations) over `enum` (traditional enumerations) where possible. - *Rationale*: Scoped enumerations avoid two potential pitfalls/problems with traditional C++ enumerations: implicit conversions to `int`, and name clashes due to enumerators being exported to the surrounding scope. diff --git a/src/span.h b/src/span.h index 2134b4f6cf..4dbdd3161f 100644 --- a/src/span.h +++ b/src/span.h @@ -31,6 +31,62 @@ /** A Span is an object that can refer to a contiguous sequence of objects. * * It implements a subset of C++20's std::span. + * + * Things to be aware of when writing code that deals with Spans: + * + * - Similar to references themselves, Spans are subject to reference lifetime + * issues. The user is responsible for making sure the objects pointed to by + * a Span live as long as the Span is used. For example: + * + * std::vector vec{1,2,3,4}; + * Span sp(vec); + * vec.push_back(5); + * printf("%i\n", sp.front()); // UB! + * + * may exhibit undefined behavior, as increasing the size of a vector may + * invalidate references. + * + * - One particular pitfall is that Spans can be constructed from temporaries, + * but this is unsafe when the Span is stored in a variable, outliving the + * temporary. For example, this will compile, but exhibits undefined behavior: + * + * Span sp(std::vector{1, 2, 3}); + * printf("%i\n", sp.front()); // UB! + * + * The lifetime of the vector ends when the statement it is created in ends. + * Thus the Span is left with a dangling reference, and using it is undefined. + * + * - Due to Span's automatic creation from range-like objects (arrays, and data + * types that expose a data() and size() member function), functions that + * accept a Span as input parameter can be called with any compatible + * range-like object. For example, this works: +* + * void Foo(Span arg); + * + * Foo(std::vector{1, 2, 3}); // Works + * + * This is very useful in cases where a function truly does not care about the + * container, and only about having exactly a range of elements. However it + * may also be surprising to see automatic conversions in this case. + * + * When a function accepts a Span with a mutable element type, it will not + * accept temporaries; only variables or other references. For example: + * + * void FooMut(Span arg); + * + * FooMut(std::vector{1, 2, 3}); // Does not compile + * std::vector baz{1, 2, 3}; + * FooMut(baz); // Works + * + * This is similar to how functions that take (non-const) lvalue references + * as input cannot accept temporaries. This does not work either: + * + * void FooVec(std::vector& arg); + * FooVec(std::vector{1, 2, 3}); // Does not compile + * + * The idea is that if a function accepts a mutable reference, a meaningful + * result will be present in that variable after the call. Passing a temporary + * is useless in that context. */ template class Span From ca5cb456dd76502b7df0854eeba2c7ac560673f9 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 3 Jul 2020 10:17:52 +0800 Subject: [PATCH 11/17] Merge #18649: tests: Add std::locale::global to list of locale dependent functions in lint-locale-dependence.sh 54b5eb2b149a1f2a4a1dbdb9e0648c5a390d8e22 tests: Add std::locale::global to list of locale dependent functions in lint-locale-dependence.sh (practicalswift) Pull request description: Add `std::locale::global` to list of locale dependent functions in `lint-locale-dependence.sh`. We currently flag `setlocale(...)` as locale dependent, but prior to this commit we didn't flag `std::locale::global(...)` as such. In addition to setting the global C++ locale `std::locale::global(...)` also does the equivalent of `std::setlocale(LC_ALL, ...);`. Thus the functionality of `std::locale::global(...)` is a superset of `setlocale(...)` :) ACKs for top commit: MarcoFalke: ACK 54b5eb2b149a1f2a4a1dbdb9e0648c5a390d8e22, fine with me Tree-SHA512: bcf2f1c765add6ed09c3debca968b75eeea81602503f109c0f76ec98635911d453f4834a39e741703c3d470f123178e8952191a9b1a3429394b99c07765dcf1f --- test/lint/lint-locale-dependence.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index accf459c89..5fafdaa4c4 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -124,6 +124,7 @@ LOCALE_DEPENDENT_FUNCTIONS=( snprintf sprintf sscanf + std::locale::global std::to_string stod stof From c4567ce9d41f301643c82cf5ea8c08403cd530fc Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 8 Oct 2020 13:17:25 +0200 Subject: [PATCH 12/17] Merge #20065: fuzz: Configure check for main function fae7a1c18803675e70b9bf66575e1e0a6e01f6f6 fuzz: Configure check for main function (MarcoFalke) Pull request description: Instead of the PP jungle, use a proper configure check Fixes https://github.com/google/honggfuzz/issues/336#issuecomment-702972138 ACKs for top commit: practicalswift: ACK fae7a1c18803675e70b9bf66575e1e0a6e01f6f6 Tree-SHA512: 2e55457d01f9ac598bb1e119d8b49dca55a28f88ec164cee6b5f071c29e9791f5a46cc8ee2b801b3a3faf906348da964ce32e7254da981c1104b9210a3508100 --- configure.ac | 14 ++++++++++++++ src/test/fuzz/fuzz.cpp | 9 --------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 4a86f8faf6..acfabf7f6c 100644 --- a/configure.ac +++ b/configure.ac @@ -1244,6 +1244,20 @@ if test "x$enable_fuzz" = "xyes"; then use_upnp=no use_natpmp=no use_zmq=no + + AC_MSG_CHECKING([whether main function is needed]) + AX_CHECK_LINK_FLAG( + [[-fsanitize=$use_sanitizers]], + [AC_MSG_RESULT([no])], + [AC_MSG_RESULT([yes]) + CPPFLAGS="$CPPFLAGS -DPROVIDE_MAIN_FUNCTION"], + [], + [AC_LANG_PROGRAM([[ + #include + #include + extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; } + /* unterminated comment to remove the main function ... + ]],[[]])]) else BITCOIN_QT_INIT diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index f5fbe87bad..bead6ca070 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -12,15 +12,6 @@ const std::function G_TEST_LOG_FUN{}; -// Decide if main(...) should be provided: -// * AFL needs main(...) regardless of platform. -// * macOS handles __attribute__((weak)) main(...) poorly when linking -// against libFuzzer. See https://github.com/bitcoin/bitcoin/pull/18008 -// for details. -#if defined(__AFL_COMPILER) || !defined(MAC_OSX) -#define PROVIDE_MAIN_FUNCTION -#endif - #if defined(PROVIDE_MAIN_FUNCTION) static bool read_stdin(std::vector& data) { From c2c35795c539cf7dc12f10c969e1fbd55f2d0c4f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 22 Dec 2020 12:12:04 +0100 Subject: [PATCH 13/17] Merge #20740: fuzz: Update FuzzedDataProvider.h from upstream (LLVM) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit e3d2ba7c70b13a2165020e45abf02373a1e953f7 fuzz: Update FuzzedDataProvider.h from upstream (LLVM) (practicalswift) Pull request description: Update `FuzzedDataProvider.h` from upstream (LLVM). Upstream revision: https://github.com/llvm/llvm-project/blob/6d0488f75bb2f37bcfe93fc8f59f6e78c9a0c939/compiler-rt/include/fuzzer/FuzzedDataProvider.h Changes since last update: * [[compiler-rt] FuzzedDataProvider: add ConsumeData and method.](https://github.com/llvm/llvm-project/commit/20a604d3f5bc8cdba161e7e12f12f3f5260aad57) * [[compiler-rt] Fix a typo in a comment in FuzzedDataProvider.h.](https://github.com/llvm/llvm-project/commit/5517d3b80b136b4afc7097147397d03bb840403a) * [[compiler-rt] Add ConsumeRandomLengthString() version without arguments.](https://github.com/llvm/llvm-project/commit/2136d17d8dec4b5bd47d2ba2b48866c34c55e56d) * [[compiler-rt] Refactor FuzzedDataProvider for better readability.](https://github.com/llvm/llvm-project/commit/1262db1b6a99d1ed79735e5ef377b1d2c23da3e7) * [[compiler-rt] FuzzedDataProvider: make linter happy.](https://github.com/llvm/llvm-project/commit/1e65209e045598c3ca5833331215606a02267229) * [[compiler-rt] Mark FDP non-template methods inline to avoid ODR violations.](https://github.com/llvm/llvm-project/commit/6d0488f75bb2f37bcfe93fc8f59f6e78c9a0c939) ACKs for top commit: MarcoFalke: ACK e3d2ba7c70b13a2165020e45abf02373a1e953f7 🌛 Tree-SHA512: 62cb27906f08fd07983f4a8fbbd381c12ed185617a58f1ebc8564c87e638086f952417f4f6481fbd91b9a313aff00e944215393734566c219c074512991f8057 --- src/test/fuzz/FuzzedDataProvider.h | 564 +++++++++++++++++------------ 1 file changed, 323 insertions(+), 241 deletions(-) diff --git a/src/test/fuzz/FuzzedDataProvider.h b/src/test/fuzz/FuzzedDataProvider.h index 07fe78b2f4..cb3ace3b94 100644 --- a/src/test/fuzz/FuzzedDataProvider.h +++ b/src/test/fuzz/FuzzedDataProvider.h @@ -35,208 +35,47 @@ class FuzzedDataProvider { : data_ptr_(data), remaining_bytes_(size) {} ~FuzzedDataProvider() = default; - // Returns a std::vector containing |num_bytes| of input data. If fewer than - // |num_bytes| of data remain, returns a shorter std::vector containing all - // of the data that's left. Can be used with any byte sized type, such as - // char, unsigned char, uint8_t, etc. - template std::vector ConsumeBytes(size_t num_bytes) { - num_bytes = std::min(num_bytes, remaining_bytes_); - return ConsumeBytes(num_bytes, num_bytes); - } + // See the implementation below (after the class definition) for more verbose + // comments for each of the methods. - // Similar to |ConsumeBytes|, but also appends the terminator value at the end - // of the resulting vector. Useful, when a mutable null-terminated C-string is - // needed, for example. But that is a rare case. Better avoid it, if possible, - // and prefer using |ConsumeBytes| or |ConsumeBytesAsString| methods. + // Methods returning std::vector of bytes. These are the most popular choice + // when splitting fuzzing input into pieces, as every piece is put into a + // separate buffer (i.e. ASan would catch any under-/overflow) and the memory + // will be released automatically. + template std::vector ConsumeBytes(size_t num_bytes); template - std::vector ConsumeBytesWithTerminator(size_t num_bytes, - T terminator = 0) { - num_bytes = std::min(num_bytes, remaining_bytes_); - std::vector result = ConsumeBytes(num_bytes + 1, num_bytes); - result.back() = terminator; - return result; - } + std::vector ConsumeBytesWithTerminator(size_t num_bytes, T terminator = 0); + template std::vector ConsumeRemainingBytes(); - // Returns a std::string containing |num_bytes| of input data. Using this and - // |.c_str()| on the resulting string is the best way to get an immutable - // null-terminated C string. If fewer than |num_bytes| of data remain, returns - // a shorter std::string containing all of the data that's left. - std::string ConsumeBytesAsString(size_t num_bytes) { - static_assert(sizeof(std::string::value_type) == sizeof(uint8_t), - "ConsumeBytesAsString cannot convert the data to a string."); + // Methods returning strings. Use only when you need a std::string or a null + // terminated C-string. Otherwise, prefer the methods returning std::vector. + std::string ConsumeBytesAsString(size_t num_bytes); + std::string ConsumeRandomLengthString(size_t max_length); + std::string ConsumeRandomLengthString(); + std::string ConsumeRemainingBytesAsString(); - num_bytes = std::min(num_bytes, remaining_bytes_); - std::string result( - reinterpret_cast(data_ptr_), - num_bytes); - Advance(num_bytes); - return result; - } + // Methods returning integer values. + template T ConsumeIntegral(); + template T ConsumeIntegralInRange(T min, T max); - // Returns a number in the range [min, max] by consuming bytes from the - // input data. The value might not be uniformly distributed in the given - // range. If there's no input data left, always returns |min|. |min| must - // be less than or equal to |max|. - template T ConsumeIntegralInRange(T min, T max) { - static_assert(std::is_integral::value, "An integral type is required."); - static_assert(sizeof(T) <= sizeof(uint64_t), "Unsupported integral type."); + // Methods returning floating point values. + template T ConsumeFloatingPoint(); + template T ConsumeFloatingPointInRange(T min, T max); - if (min > max) - abort(); + // 0 <= return value <= 1. + template T ConsumeProbability(); - // Use the biggest type possible to hold the range and the result. - uint64_t range = static_cast(max) - min; - uint64_t result = 0; - size_t offset = 0; + bool ConsumeBool(); - while (offset < sizeof(T) * CHAR_BIT && (range >> offset) > 0 && - remaining_bytes_ != 0) { - // Pull bytes off the end of the seed data. Experimentally, this seems to - // allow the fuzzer to more easily explore the input space. This makes - // sense, since it works by modifying inputs that caused new code to run, - // and this data is often used to encode length of data read by - // |ConsumeBytes|. Separating out read lengths makes it easier modify the - // contents of the data that is actually read. - --remaining_bytes_; - result = (result << CHAR_BIT) | data_ptr_[remaining_bytes_]; - offset += CHAR_BIT; - } + // Returns a value chosen from the given enum. + template T ConsumeEnum(); - // Avoid division by 0, in case |range + 1| results in overflow. - if (range != std::numeric_limits::max()) - result = result % (range + 1); + // Returns a value from the given array. + template T PickValueInArray(const T (&array)[size]); + template T PickValueInArray(std::initializer_list list); - return static_cast(min + result); - } - - // Returns a std::string of length from 0 to |max_length|. When it runs out of - // input data, returns what remains of the input. Designed to be more stable - // with respect to a fuzzer inserting characters than just picking a random - // length and then consuming that many bytes with |ConsumeBytes|. - std::string ConsumeRandomLengthString(size_t max_length) { - // Reads bytes from the start of |data_ptr_|. Maps "\\" to "\", and maps "\" - // followed by anything else to the end of the string. As a result of this - // logic, a fuzzer can insert characters into the string, and the string - // will be lengthened to include those new characters, resulting in a more - // stable fuzzer than picking the length of a string independently from - // picking its contents. - std::string result; - - // Reserve the anticipated capaticity to prevent several reallocations. - result.reserve(std::min(max_length, remaining_bytes_)); - for (size_t i = 0; i < max_length && remaining_bytes_ != 0; ++i) { - char next = ConvertUnsignedToSigned(data_ptr_[0]); - Advance(1); - if (next == '\\' && remaining_bytes_ != 0) { - next = ConvertUnsignedToSigned(data_ptr_[0]); - Advance(1); - if (next != '\\') - break; - } - result += next; - } - - result.shrink_to_fit(); - return result; - } - - // Returns a std::vector containing all remaining bytes of the input data. - template std::vector ConsumeRemainingBytes() { - return ConsumeBytes(remaining_bytes_); - } - - // Returns a std::string containing all remaining bytes of the input data. - // Prefer using |ConsumeRemainingBytes| unless you actually need a std::string - // object. - std::string ConsumeRemainingBytesAsString() { - return ConsumeBytesAsString(remaining_bytes_); - } - - // Returns a number in the range [Type's min, Type's max]. The value might - // not be uniformly distributed in the given range. If there's no input data - // left, always returns |min|. - template T ConsumeIntegral() { - return ConsumeIntegralInRange(std::numeric_limits::min(), - std::numeric_limits::max()); - } - - // Reads one byte and returns a bool, or false when no data remains. - bool ConsumeBool() { return 1 & ConsumeIntegral(); } - - // Returns a copy of the value selected from the given fixed-size |array|. - template - T PickValueInArray(const T (&array)[size]) { - static_assert(size > 0, "The array must be non empty."); - return array[ConsumeIntegralInRange(0, size - 1)]; - } - - template - T PickValueInArray(std::initializer_list list) { - // TODO(Dor1s): switch to static_assert once C++14 is allowed. - if (!list.size()) - abort(); - - return *(list.begin() + ConsumeIntegralInRange(0, list.size() - 1)); - } - - // Returns an enum value. The enum must start at 0 and be contiguous. It must - // also contain |kMaxValue| aliased to its largest (inclusive) value. Such as: - // enum class Foo { SomeValue, OtherValue, kMaxValue = OtherValue }; - template T ConsumeEnum() { - static_assert(std::is_enum::value, "|T| must be an enum type."); - return static_cast(ConsumeIntegralInRange( - 0, static_cast(T::kMaxValue))); - } - - // Returns a floating point number in the range [0.0, 1.0]. If there's no - // input data left, always returns 0. - template T ConsumeProbability() { - static_assert(std::is_floating_point::value, - "A floating point type is required."); - - // Use different integral types for different floating point types in order - // to provide better density of the resulting values. - using IntegralType = - typename std::conditional<(sizeof(T) <= sizeof(uint32_t)), uint32_t, - uint64_t>::type; - - T result = static_cast(ConsumeIntegral()); - result /= static_cast(std::numeric_limits::max()); - return result; - } - - // Returns a floating point value in the range [Type's lowest, Type's max] by - // consuming bytes from the input data. If there's no input data left, always - // returns approximately 0. - template T ConsumeFloatingPoint() { - return ConsumeFloatingPointInRange(std::numeric_limits::lowest(), - std::numeric_limits::max()); - } - - // Returns a floating point value in the given range by consuming bytes from - // the input data. If there's no input data left, returns |min|. Note that - // |min| must be less than or equal to |max|. - template T ConsumeFloatingPointInRange(T min, T max) { - if (min > max) - abort(); - - T range = .0; - T result = min; - constexpr T zero(.0); - if (max > zero && min < zero && max > min + std::numeric_limits::max()) { - // The diff |max - min| would overflow the given floating point type. Use - // the half of the diff as the range and consume a bool to decide whether - // the result is in the first of the second part of the diff. - range = (max / 2.0) - (min / 2.0); - if (ConsumeBool()) { - result += range; - } - } else { - range = max - min; - } - - return result + range * ConsumeProbability(); - } + // Writes data to the given destination and returns number of bytes written. + size_t ConsumeData(void *destination, size_t num_bytes); // Reports the remaining bytes available for fuzzed input. size_t remaining_bytes() { return remaining_bytes_; } @@ -245,62 +84,305 @@ class FuzzedDataProvider { FuzzedDataProvider(const FuzzedDataProvider &) = delete; FuzzedDataProvider &operator=(const FuzzedDataProvider &) = delete; - void Advance(size_t num_bytes) { - if (num_bytes > remaining_bytes_) - abort(); + void CopyAndAdvance(void *destination, size_t num_bytes); - data_ptr_ += num_bytes; - remaining_bytes_ -= num_bytes; - } + void Advance(size_t num_bytes); template - std::vector ConsumeBytes(size_t size, size_t num_bytes_to_consume) { - static_assert(sizeof(T) == sizeof(uint8_t), "Incompatible data type."); + std::vector ConsumeBytes(size_t size, size_t num_bytes); - // The point of using the size-based constructor below is to increase the - // odds of having a vector object with capacity being equal to the length. - // That part is always implementation specific, but at least both libc++ and - // libstdc++ allocate the requested number of bytes in that constructor, - // which seems to be a natural choice for other implementations as well. - // To increase the odds even more, we also call |shrink_to_fit| below. - std::vector result(size); - if (size == 0) { - if (num_bytes_to_consume != 0) - abort(); - return result; - } - - std::memcpy(result.data(), data_ptr_, num_bytes_to_consume); - Advance(num_bytes_to_consume); - - // Even though |shrink_to_fit| is also implementation specific, we expect it - // to provide an additional assurance in case vector's constructor allocated - // a buffer which is larger than the actual amount of data we put inside it. - result.shrink_to_fit(); - return result; - } - - template TS ConvertUnsignedToSigned(TU value) { - static_assert(sizeof(TS) == sizeof(TU), "Incompatible data types."); - static_assert(!std::numeric_limits::is_signed, - "Source type must be unsigned."); - - // TODO(Dor1s): change to `if constexpr` once C++17 becomes mainstream. - if (std::numeric_limits::is_modulo) - return static_cast(value); - - // Avoid using implementation-defined unsigned to signer conversions. - // To learn more, see https://stackoverflow.com/questions/13150449. - if (value <= std::numeric_limits::max()) { - return static_cast(value); - } else { - constexpr auto TS_min = std::numeric_limits::min(); - return TS_min + static_cast(value - TS_min); - } - } + template TS ConvertUnsignedToSigned(TU value); const uint8_t *data_ptr_; size_t remaining_bytes_; }; +// Returns a std::vector containing |num_bytes| of input data. If fewer than +// |num_bytes| of data remain, returns a shorter std::vector containing all +// of the data that's left. Can be used with any byte sized type, such as +// char, unsigned char, uint8_t, etc. +template +std::vector FuzzedDataProvider::ConsumeBytes(size_t num_bytes) { + num_bytes = std::min(num_bytes, remaining_bytes_); + return ConsumeBytes(num_bytes, num_bytes); +} + +// Similar to |ConsumeBytes|, but also appends the terminator value at the end +// of the resulting vector. Useful, when a mutable null-terminated C-string is +// needed, for example. But that is a rare case. Better avoid it, if possible, +// and prefer using |ConsumeBytes| or |ConsumeBytesAsString| methods. +template +std::vector FuzzedDataProvider::ConsumeBytesWithTerminator(size_t num_bytes, + T terminator) { + num_bytes = std::min(num_bytes, remaining_bytes_); + std::vector result = ConsumeBytes(num_bytes + 1, num_bytes); + result.back() = terminator; + return result; +} + +// Returns a std::vector containing all remaining bytes of the input data. +template +std::vector FuzzedDataProvider::ConsumeRemainingBytes() { + return ConsumeBytes(remaining_bytes_); +} + +// Returns a std::string containing |num_bytes| of input data. Using this and +// |.c_str()| on the resulting string is the best way to get an immutable +// null-terminated C string. If fewer than |num_bytes| of data remain, returns +// a shorter std::string containing all of the data that's left. +inline std::string FuzzedDataProvider::ConsumeBytesAsString(size_t num_bytes) { + static_assert(sizeof(std::string::value_type) == sizeof(uint8_t), + "ConsumeBytesAsString cannot convert the data to a string."); + + num_bytes = std::min(num_bytes, remaining_bytes_); + std::string result( + reinterpret_cast(data_ptr_), num_bytes); + Advance(num_bytes); + return result; +} + +// Returns a std::string of length from 0 to |max_length|. When it runs out of +// input data, returns what remains of the input. Designed to be more stable +// with respect to a fuzzer inserting characters than just picking a random +// length and then consuming that many bytes with |ConsumeBytes|. +inline std::string +FuzzedDataProvider::ConsumeRandomLengthString(size_t max_length) { + // Reads bytes from the start of |data_ptr_|. Maps "\\" to "\", and maps "\" + // followed by anything else to the end of the string. As a result of this + // logic, a fuzzer can insert characters into the string, and the string + // will be lengthened to include those new characters, resulting in a more + // stable fuzzer than picking the length of a string independently from + // picking its contents. + std::string result; + + // Reserve the anticipated capaticity to prevent several reallocations. + result.reserve(std::min(max_length, remaining_bytes_)); + for (size_t i = 0; i < max_length && remaining_bytes_ != 0; ++i) { + char next = ConvertUnsignedToSigned(data_ptr_[0]); + Advance(1); + if (next == '\\' && remaining_bytes_ != 0) { + next = ConvertUnsignedToSigned(data_ptr_[0]); + Advance(1); + if (next != '\\') + break; + } + result += next; + } + + result.shrink_to_fit(); + return result; +} + +// Returns a std::string of length from 0 to |remaining_bytes_|. +inline std::string FuzzedDataProvider::ConsumeRandomLengthString() { + return ConsumeRandomLengthString(remaining_bytes_); +} + +// Returns a std::string containing all remaining bytes of the input data. +// Prefer using |ConsumeRemainingBytes| unless you actually need a std::string +// object. +inline std::string FuzzedDataProvider::ConsumeRemainingBytesAsString() { + return ConsumeBytesAsString(remaining_bytes_); +} + +// Returns a number in the range [Type's min, Type's max]. The value might +// not be uniformly distributed in the given range. If there's no input data +// left, always returns |min|. +template T FuzzedDataProvider::ConsumeIntegral() { + return ConsumeIntegralInRange(std::numeric_limits::min(), + std::numeric_limits::max()); +} + +// Returns a number in the range [min, max] by consuming bytes from the +// input data. The value might not be uniformly distributed in the given +// range. If there's no input data left, always returns |min|. |min| must +// be less than or equal to |max|. +template +T FuzzedDataProvider::ConsumeIntegralInRange(T min, T max) { + static_assert(std::is_integral::value, "An integral type is required."); + static_assert(sizeof(T) <= sizeof(uint64_t), "Unsupported integral type."); + + if (min > max) + abort(); + + // Use the biggest type possible to hold the range and the result. + uint64_t range = static_cast(max) - min; + uint64_t result = 0; + size_t offset = 0; + + while (offset < sizeof(T) * CHAR_BIT && (range >> offset) > 0 && + remaining_bytes_ != 0) { + // Pull bytes off the end of the seed data. Experimentally, this seems to + // allow the fuzzer to more easily explore the input space. This makes + // sense, since it works by modifying inputs that caused new code to run, + // and this data is often used to encode length of data read by + // |ConsumeBytes|. Separating out read lengths makes it easier modify the + // contents of the data that is actually read. + --remaining_bytes_; + result = (result << CHAR_BIT) | data_ptr_[remaining_bytes_]; + offset += CHAR_BIT; + } + + // Avoid division by 0, in case |range + 1| results in overflow. + if (range != std::numeric_limits::max()) + result = result % (range + 1); + + return static_cast(min + result); +} + +// Returns a floating point value in the range [Type's lowest, Type's max] by +// consuming bytes from the input data. If there's no input data left, always +// returns approximately 0. +template T FuzzedDataProvider::ConsumeFloatingPoint() { + return ConsumeFloatingPointInRange(std::numeric_limits::lowest(), + std::numeric_limits::max()); +} + +// Returns a floating point value in the given range by consuming bytes from +// the input data. If there's no input data left, returns |min|. Note that +// |min| must be less than or equal to |max|. +template +T FuzzedDataProvider::ConsumeFloatingPointInRange(T min, T max) { + if (min > max) + abort(); + + T range = .0; + T result = min; + constexpr T zero(.0); + if (max > zero && min < zero && max > min + std::numeric_limits::max()) { + // The diff |max - min| would overflow the given floating point type. Use + // the half of the diff as the range and consume a bool to decide whether + // the result is in the first of the second part of the diff. + range = (max / 2.0) - (min / 2.0); + if (ConsumeBool()) { + result += range; + } + } else { + range = max - min; + } + + return result + range * ConsumeProbability(); +} + +// Returns a floating point number in the range [0.0, 1.0]. If there's no +// input data left, always returns 0. +template T FuzzedDataProvider::ConsumeProbability() { + static_assert(std::is_floating_point::value, + "A floating point type is required."); + + // Use different integral types for different floating point types in order + // to provide better density of the resulting values. + using IntegralType = + typename std::conditional<(sizeof(T) <= sizeof(uint32_t)), uint32_t, + uint64_t>::type; + + T result = static_cast(ConsumeIntegral()); + result /= static_cast(std::numeric_limits::max()); + return result; +} + +// Reads one byte and returns a bool, or false when no data remains. +inline bool FuzzedDataProvider::ConsumeBool() { + return 1 & ConsumeIntegral(); +} + +// Returns an enum value. The enum must start at 0 and be contiguous. It must +// also contain |kMaxValue| aliased to its largest (inclusive) value. Such as: +// enum class Foo { SomeValue, OtherValue, kMaxValue = OtherValue }; +template T FuzzedDataProvider::ConsumeEnum() { + static_assert(std::is_enum::value, "|T| must be an enum type."); + return static_cast( + ConsumeIntegralInRange(0, static_cast(T::kMaxValue))); +} + +// Returns a copy of the value selected from the given fixed-size |array|. +template +T FuzzedDataProvider::PickValueInArray(const T (&array)[size]) { + static_assert(size > 0, "The array must be non empty."); + return array[ConsumeIntegralInRange(0, size - 1)]; +} + +template +T FuzzedDataProvider::PickValueInArray(std::initializer_list list) { + // TODO(Dor1s): switch to static_assert once C++14 is allowed. + if (!list.size()) + abort(); + + return *(list.begin() + ConsumeIntegralInRange(0, list.size() - 1)); +} + +// Writes |num_bytes| of input data to the given destination pointer. If there +// is not enough data left, writes all remaining bytes. Return value is the +// number of bytes written. +// In general, it's better to avoid using this function, but it may be useful +// in cases when it's necessary to fill a certain buffer or object with +// fuzzing data. +inline size_t FuzzedDataProvider::ConsumeData(void *destination, + size_t num_bytes) { + num_bytes = std::min(num_bytes, remaining_bytes_); + CopyAndAdvance(destination, num_bytes); + return num_bytes; +} + +// Private methods. +inline void FuzzedDataProvider::CopyAndAdvance(void *destination, + size_t num_bytes) { + std::memcpy(destination, data_ptr_, num_bytes); + Advance(num_bytes); +} + +inline void FuzzedDataProvider::Advance(size_t num_bytes) { + if (num_bytes > remaining_bytes_) + abort(); + + data_ptr_ += num_bytes; + remaining_bytes_ -= num_bytes; +} + +template +std::vector FuzzedDataProvider::ConsumeBytes(size_t size, size_t num_bytes) { + static_assert(sizeof(T) == sizeof(uint8_t), "Incompatible data type."); + + // The point of using the size-based constructor below is to increase the + // odds of having a vector object with capacity being equal to the length. + // That part is always implementation specific, but at least both libc++ and + // libstdc++ allocate the requested number of bytes in that constructor, + // which seems to be a natural choice for other implementations as well. + // To increase the odds even more, we also call |shrink_to_fit| below. + std::vector result(size); + if (size == 0) { + if (num_bytes != 0) + abort(); + return result; + } + + CopyAndAdvance(result.data(), num_bytes); + + // Even though |shrink_to_fit| is also implementation specific, we expect it + // to provide an additional assurance in case vector's constructor allocated + // a buffer which is larger than the actual amount of data we put inside it. + result.shrink_to_fit(); + return result; +} + +template +TS FuzzedDataProvider::ConvertUnsignedToSigned(TU value) { + static_assert(sizeof(TS) == sizeof(TU), "Incompatible data types."); + static_assert(!std::numeric_limits::is_signed, + "Source type must be unsigned."); + + // TODO(Dor1s): change to `if constexpr` once C++17 becomes mainstream. + if (std::numeric_limits::is_modulo) + return static_cast(value); + + // Avoid using implementation-defined unsigned to signed conversions. + // To learn more, see https://stackoverflow.com/questions/13150449. + if (value <= std::numeric_limits::max()) { + return static_cast(value); + } else { + constexpr auto TS_min = std::numeric_limits::min(); + return TS_min + static_cast(value - TS_min); + } +} + #endif // LLVM_FUZZER_FUZZED_DATA_PROVIDER_H_ From 353dc403f534862d9afe9914b7797864234d5f9f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 5 Feb 2021 10:04:50 +0100 Subject: [PATCH 14/17] Merge #21080: fuzz: Configure check for main function (take 2) fac4be30482c21ac330e09ef8756c49e37faa6fa fuzz: Configure check for main function (take 2) (MarcoFalke) Pull request description: Actually fix https://github.com/google/honggfuzz/issues/336#issuecomment-702972138 Follow-up to #20065 Steps to test: `honggfuzz` section in doc/fuzzing.md ACKs for top commit: practicalswift: cr ACK fac4be30482c21ac330e09ef8756c49e37faa6fa: patch looks correct! Tree-SHA512: 893768c80439fe5d90b883ade89dc02f5bb80e27637916cf5575b6a9ed0b1c04942ff851342f5bbabb8666e6696715427feeb98f5301ad23c7b87b09e5872f97 --- configure.ac | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index acfabf7f6c..3935fd2757 100644 --- a/configure.ac +++ b/configure.ac @@ -1256,8 +1256,10 @@ if test "x$enable_fuzz" = "xyes"; then #include #include extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; } - /* unterminated comment to remove the main function ... - ]],[[]])]) + /* comment to remove the main function ... + ]],[[ + */ int not_main() { + ]])]) else BITCOIN_QT_INIT From 3b76f34edc53107929d710907b8fd366ecf8dd9e Mon Sep 17 00:00:00 2001 From: fanquake Date: Sun, 21 Feb 2021 10:09:29 +0800 Subject: [PATCH 15/17] Merge #21243: ci: Avoid invoking curl on the host fa330d8fed5a02349440be170af3b443c1321b4b ci: Avoid invoking curl on the host (MarcoFalke) Pull request description: The only requirement for the ci system are the programs `docker.io` and `bash`. However, the mac cross build invokes `curl` on the host. Fix that. Before: ``` $ FILE_ENV="./ci/test/00_setup_env_mac.sh" ./ci/test_run_all.sh ... ./ci/test/05_before_script.sh: line 22: curl: command not found ``` After: ``` ... (command passes) ACKs for top commit: promag: ACK fa330d8fed5a02349440be170af3b443c1321b4b. Tree-SHA512: 49120fd671a48a6599dd6c34f6d3502a6e9f84b4476061cab06f55cba374d8188f53b9b41363e90f5fafb0074767b581f30bd2545c0b6934580a7eccfa1ef5c4 --- ci/test/05_before_script.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/05_before_script.sh b/ci/test/05_before_script.sh index 30a22a063f..eac3dfcc69 100755 --- a/ci/test/05_before_script.sh +++ b/ci/test/05_before_script.sh @@ -13,7 +13,7 @@ OSX_SDK_PATH="depends/sdk-sources/${OSX_SDK_BASENAME}" mkdir -p depends/SDKs depends/sdk-sources if [ -n "$XCODE_VERSION" ] && [ ! -f "$OSX_SDK_PATH" ]; then - curl --location --fail "${SDK_URL}/${OSX_SDK_BASENAME}" -o "$OSX_SDK_PATH" + DOCKER_EXEC curl --location --fail "${SDK_URL}/${OSX_SDK_BASENAME}" -o "$OSX_SDK_PATH" fi if [ -n "$XCODE_VERSION" ] && [ -f "$OSX_SDK_PATH" ]; then DOCKER_EXEC tar -C "depends/SDKs" -xf "$OSX_SDK_PATH" From 6525ca37a75f1950dc7edaee28ec1d0385cec55e Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 5 May 2021 16:10:21 +0200 Subject: [PATCH 16/17] Merge bitcoin/bitcoin#21709: doc: update reduce-memory.md and bitcoin.conf -maxconnections info 300234ab6698277fb261a87be72f89ef94d3697a doc: update bitcoin.conf maxconnections info (Jon Atack) 926827065ffa56b75e9261f63d49b924d4bced0f doc: update reduce-memory.md peer connections info (Jon Atack) Pull request description: This patch updates the documentation in `doc/reduce-memory.md` and `share/examples/bitcoin.conf` regarding the peer connections limits and `-maxconnections` ACKs for top commit: jarolrod: re-ACK 300234ab6698277fb261a87be72f89ef94d3697a laanwj: ACK 300234ab6698277fb261a87be72f89ef94d3697a prayank23: ACK https://github.com/bitcoin/bitcoin/pull/21709/commits/300234ab6698277fb261a87be72f89ef94d3697a Tree-SHA512: 90f53626124afb50706e6a3b644bc7bb800bb7cf41ae7062c20c17b3d9bdf4a8d73b4cf188faec9113d772596f7e4bc36a4a69481cacb92cc55d5956181d0c31 --- contrib/debian/examples/dash.conf | 7 ++++++- doc/reduce-memory.md | 10 +++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/contrib/debian/examples/dash.conf b/contrib/debian/examples/dash.conf index 5a06980200..57cbf5f57f 100644 --- a/contrib/debian/examples/dash.conf +++ b/contrib/debian/examples/dash.conf @@ -60,7 +60,12 @@ # Port on which to listen for connections (default: 9999, testnet: 19999, regtest: 19899) #port= -# Maximum number of inbound+outbound connections. +# Maximum number of inbound + outbound connections (default: 125). This option +# applies only if inbound connections are enabled; otherwise, the number of connections +# will not be more than 11: 8 full-relay connections, 2 block-relay-only ones, and +# occasionally 1 short-lived feeler or extra outbound block-relay-only connection. +# These limits do not apply to connections added manually with the -addnode +# configuration option or the addnode RPC, which have a separate limit of 8 connections. #maxconnections= # Maximum upload bandwidth target in MiB per day (e.g. 'maxuploadtarget=1024' is 1 GiB per day). diff --git a/doc/reduce-memory.md b/doc/reduce-memory.md index 3c71416434..6b5ada7e22 100644 --- a/doc/reduce-memory.md +++ b/doc/reduce-memory.md @@ -24,9 +24,13 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa ## Number of peers -- `-maxconnections=` - the maximum number of connections, this defaults to 125. Each active connection takes up some - memory. This option applies only if incoming connections are enabled, otherwise the number of connections will never - be more than 10. Of the 10 outbound peers, there can be 8 full-relay connections and 2 block-relay-only ones. +- `-maxconnections=` - the maximum number of connections, which defaults to 125. Each active connection takes up some + memory. This option applies only if inbound connections are enabled; otherwise, the number of connections will not + be more than 11. Of the 11 outbound peers, there can be 8 full-relay connections, 2 block-relay-only ones, + and occasionally 1 short-lived feeler or extra outbound block-relay-only connection. + +- These limits do not apply to connections added manually with the `-addnode` configuration option or + the `addnode` RPC, which have a separate limit of 8 connections. ## Thread configuration From e711f02684fdda7ad6e33b9c6b07d30041e7ddac Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 10 May 2021 14:18:35 +0800 Subject: [PATCH 17/17] Merge bitcoin/bitcoin#21890: fuzz: Limit ParseISO8601DateTime fuzzing to 32-bit fa1aa6c571f406a2c40282664487aca4aff9dc9d fuzz: Limit ParseISO8601DateTime fuzzing to 32-bit (MarcoFalke) Pull request description: 2038 is more than 10 years in the future, so no need for us to waste time fuzzing a 3rd party lib that will be EOL by then. Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34092 ACKs for top commit: theStack: Concept and code review ACK fa1aa6c571f406a2c40282664487aca4aff9dc9d Tree-SHA512: fdd2fbc7b5c7ce33ad23b2e5431bb97eaf6ae8c2d2a55990a3ab73be79282c584b704dcd1471ba288de75283732970c70c9a03ddad059b97b66ba8b3de39effe --- src/test/fuzz/parse_iso8601.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/parse_iso8601.cpp b/src/test/fuzz/parse_iso8601.cpp index c86f8a853e..7ac644d752 100644 --- a/src/test/fuzz/parse_iso8601.cpp +++ b/src/test/fuzz/parse_iso8601.cpp @@ -15,7 +15,7 @@ void test_one_input(const std::vector& buffer) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - const int64_t random_time = fuzzed_data_provider.ConsumeIntegral(); + const int64_t random_time = fuzzed_data_provider.ConsumeIntegral(); const std::string random_string = fuzzed_data_provider.ConsumeRemainingBytesAsString(); const std::string iso8601_datetime = FormatISO8601DateTime(random_time);