Merge pull request #4748 from PastaPastaPasta/develop-trivial-2022-04-03

trivial backports 2022 04 03

.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: >-

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
+        -m, --min=secs                   Exponential Backoff: minimum sleep amount (seconds): Default 0.3
-        -x, --max=secs                   Exponenetial Backoff: maximum sleep amount (seconds): Default 60
+        -x, --max=secs                   Exponential Backoff: maximum sleep amount (seconds): Default 60
         -f, --fail="script +cmds"        Fail Script: run in case of final failure
 
 ### Examples

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
+    -m, --min=secs                   Exponential Backoff: minimum sleep amount (seconds): Default 0.3
-    -x, --max=secs                   Exponenetial Backoff: maximum sleep amount (seconds): Default 60
+    -x, --max=secs                   Exponential Backoff: maximum sleep amount (seconds): Default 60
     -f, --fail="script +cmds"        Fail Script: run in case of final failure
 EOF
   }

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 --"}

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

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\)

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"

configure.ac

@@ -1244,6 +1244,22 @@ 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 <cstdint>
+      #include <cstddef>
+      extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; }
+      /* comment to remove the main function ...
+     ]],[[
+      */ int not_main() {
+     ]])])
 else
     BITCOIN_QT_INIT
 

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).

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<int>&)` 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<const int> data);
+
+std::vector<int> 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.

doc/reduce-memory.md

@@ -24,7 +24,13 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa
 
 ## Number of peers
 
-- `-maxconnections=<n>` - 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=<n>` - 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
 

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
+Of the 10 outbound peers, there can be 8 full-relay connections and 2
-the -blocksonly mode turned on. You can therefore, have at most 115 inbound connections.
+block-relay-only ones.
 
 The default settings can result in relatively significant traffic consumption.
 

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)
     {

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();
         }

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<int> vec{1,2,3,4};
+ *       Span<int> 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<const int> sp(std::vector<int>{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<const int> arg);
+ *
+ *       Foo(std::vector<int>{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<int> arg);
+ *
+ *       FooMut(std::vector<int>{1, 2, 3}); // Does not compile
+ *       std::vector<int> 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<int>& arg);
+ *       FooVec(std::vector<int>{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<typename C>
 class Span

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
+  // See the implementation below (after the class definition) for more verbose
-  // |num_bytes| of data remain, returns a shorter std::vector containing all
+  // comments for each of the methods.
-  // of the data that's left. Can be used with any byte sized type, such as
-  // char, unsigned char, uint8_t, etc.
-  template <typename T> std::vector<T> ConsumeBytes(size_t num_bytes) {
-    num_bytes = std::min(num_bytes, remaining_bytes_);
-    return ConsumeBytes<T>(num_bytes, num_bytes);
-  }
 
-  // Similar to |ConsumeBytes|, but also appends the terminator value at the end
+  // Methods returning std::vector of bytes. These are the most popular choice
-  // of the resulting vector. Useful, when a mutable null-terminated C-string is
+  // when splitting fuzzing input into pieces, as every piece is put into a
-  // needed, for example. But that is a rare case. Better avoid it, if possible,
+  // separate buffer (i.e. ASan would catch any under-/overflow) and the memory
-  // and prefer using |ConsumeBytes| or |ConsumeBytesAsString| methods.
+  // will be released automatically.
+  template <typename T> std::vector<T> ConsumeBytes(size_t num_bytes);
   template <typename T>
-  std::vector<T> ConsumeBytesWithTerminator(size_t num_bytes,
+  std::vector<T> ConsumeBytesWithTerminator(size_t num_bytes, T terminator = 0);
-                                            T terminator = 0) {
+  template <typename T> std::vector<T> ConsumeRemainingBytes();
-    num_bytes = std::min(num_bytes, remaining_bytes_);
-    std::vector<T> result = ConsumeBytes<T>(num_bytes + 1, num_bytes);
-    result.back() = terminator;
-    return result;
-  }
 
-  // Returns a std::string containing |num_bytes| of input data. Using this and
+  // Methods returning strings. Use only when you need a std::string or a null
-  // |.c_str()| on the resulting string is the best way to get an immutable
+  // terminated C-string. Otherwise, prefer the methods returning std::vector.
-  // null-terminated C string. If fewer than |num_bytes| of data remain, returns
+  std::string ConsumeBytesAsString(size_t num_bytes);
-  // a shorter std::string containing all of the data that's left.
+  std::string ConsumeRandomLengthString(size_t max_length);
-  std::string ConsumeBytesAsString(size_t num_bytes) {
+  std::string ConsumeRandomLengthString();
-    static_assert(sizeof(std::string::value_type) == sizeof(uint8_t),
+  std::string ConsumeRemainingBytesAsString();
-                  "ConsumeBytesAsString cannot convert the data to a string.");
 
-    num_bytes = std::min(num_bytes, remaining_bytes_);
+  // Methods returning integer values.
-    std::string result(
+  template <typename T> T ConsumeIntegral();
-        reinterpret_cast<const std::string::value_type *>(data_ptr_),
+  template <typename T> T ConsumeIntegralInRange(T min, T max);
-        num_bytes);
-    Advance(num_bytes);
-    return result;
-  }
 
-  // Returns a number in the range [min, max] by consuming bytes from the
+  // Methods returning floating point values.
-  // input data. The value might not be uniformly distributed in the given
+  template <typename T> T ConsumeFloatingPoint();
-  // range. If there's no input data left, always returns |min|. |min| must
+  template <typename T> T ConsumeFloatingPointInRange(T min, T max);
-  // be less than or equal to |max|.
-  template <typename T> T ConsumeIntegralInRange(T min, T max) {
-    static_assert(std::is_integral<T>::value, "An integral type is required.");
-    static_assert(sizeof(T) <= sizeof(uint64_t), "Unsupported integral type.");
 
-    if (min > max)
+  // 0 <= return value <= 1.
-      abort();
+  template <typename T> T ConsumeProbability();
 
-    // Use the biggest type possible to hold the range and the result.
+  bool ConsumeBool();
-    uint64_t range = static_cast<uint64_t>(max) - min;
-    uint64_t result = 0;
-    size_t offset = 0;
 
-    while (offset < sizeof(T) * CHAR_BIT && (range >> offset) > 0 &&
+  // Returns a value chosen from the given enum.
-           remaining_bytes_ != 0) {
+  template <typename T> T ConsumeEnum();
-      // 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.
+  // Returns a value from the given array.
-    if (range != std::numeric_limits<decltype(range)>::max())
+  template <typename T, size_t size> T PickValueInArray(const T (&array)[size]);
-      result = result % (range + 1);
+  template <typename T> T PickValueInArray(std::initializer_list<const T> list);
 
-    return static_cast<T>(min + result);
+  // Writes data to the given destination and returns number of bytes written.
-  }
+  size_t ConsumeData(void *destination, size_t num_bytes);
-
-  // 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<char>(data_ptr_[0]);
-      Advance(1);
-      if (next == '\\' && remaining_bytes_ != 0) {
-        next = ConvertUnsignedToSigned<char>(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 <typename T> std::vector<T> ConsumeRemainingBytes() {
-    return ConsumeBytes<T>(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 <typename T> T ConsumeIntegral() {
-    return ConsumeIntegralInRange(std::numeric_limits<T>::min(),
-                                  std::numeric_limits<T>::max());
-  }
-
-  // Reads one byte and returns a bool, or false when no data remains.
-  bool ConsumeBool() { return 1 & ConsumeIntegral<uint8_t>(); }
-
-  // Returns a copy of the value selected from the given fixed-size |array|.
-  template <typename T, size_t size>
-  T PickValueInArray(const T (&array)[size]) {
-    static_assert(size > 0, "The array must be non empty.");
-    return array[ConsumeIntegralInRange<size_t>(0, size - 1)];
-  }
-
-  template <typename T>
-  T PickValueInArray(std::initializer_list<const T> list) {
-    // TODO(Dor1s): switch to static_assert once C++14 is allowed.
-    if (!list.size())
-      abort();
-
-    return *(list.begin() + ConsumeIntegralInRange<size_t>(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 <typename T> T ConsumeEnum() {
-    static_assert(std::is_enum<T>::value, "|T| must be an enum type.");
-    return static_cast<T>(ConsumeIntegralInRange<uint32_t>(
-        0, static_cast<uint32_t>(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 <typename T> T ConsumeProbability() {
-    static_assert(std::is_floating_point<T>::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<T>(ConsumeIntegral<IntegralType>());
-    result /= static_cast<T>(std::numeric_limits<IntegralType>::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 <typename T> T ConsumeFloatingPoint() {
-    return ConsumeFloatingPointInRange<T>(std::numeric_limits<T>::lowest(),
-                                          std::numeric_limits<T>::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 <typename T> 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<T>::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<T>();
-  }
 
   // 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) {
+  void CopyAndAdvance(void *destination, size_t num_bytes);
-    if (num_bytes > remaining_bytes_)
-      abort();
 
-    data_ptr_ += num_bytes;
+  void Advance(size_t num_bytes);
-    remaining_bytes_ -= num_bytes;
-  }
 
   template <typename T>
-  std::vector<T> ConsumeBytes(size_t size, size_t num_bytes_to_consume) {
+  std::vector<T> 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
+  template <typename TS, typename TU> TS ConvertUnsignedToSigned(TU value);
-    // 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<T> 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 <typename TS, typename TU> TS ConvertUnsignedToSigned(TU value) {
-    static_assert(sizeof(TS) == sizeof(TU), "Incompatible data types.");
-    static_assert(!std::numeric_limits<TU>::is_signed,
-                  "Source type must be unsigned.");
-
-    // TODO(Dor1s): change to `if constexpr` once C++17 becomes mainstream.
-    if (std::numeric_limits<TS>::is_modulo)
-      return static_cast<TS>(value);
-
-    // Avoid using implementation-defined unsigned to signer conversions.
-    // To learn more, see https://stackoverflow.com/questions/13150449.
-    if (value <= std::numeric_limits<TS>::max()) {
-      return static_cast<TS>(value);
-    } else {
-      constexpr auto TS_min = std::numeric_limits<TS>::min();
-      return TS_min + static_cast<char>(value - TS_min);
-    }
-  }
 
   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 <typename T>
+std::vector<T> FuzzedDataProvider::ConsumeBytes(size_t num_bytes) {
+  num_bytes = std::min(num_bytes, remaining_bytes_);
+  return ConsumeBytes<T>(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 <typename T>
+std::vector<T> FuzzedDataProvider::ConsumeBytesWithTerminator(size_t num_bytes,
+                                                              T terminator) {
+  num_bytes = std::min(num_bytes, remaining_bytes_);
+  std::vector<T> result = ConsumeBytes<T>(num_bytes + 1, num_bytes);
+  result.back() = terminator;
+  return result;
+}
+
+// Returns a std::vector containing all remaining bytes of the input data.
+template <typename T>
+std::vector<T> FuzzedDataProvider::ConsumeRemainingBytes() {
+  return ConsumeBytes<T>(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<const std::string::value_type *>(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<char>(data_ptr_[0]);
+    Advance(1);
+    if (next == '\\' && remaining_bytes_ != 0) {
+      next = ConvertUnsignedToSigned<char>(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 <typename T> T FuzzedDataProvider::ConsumeIntegral() {
+  return ConsumeIntegralInRange(std::numeric_limits<T>::min(),
+                                std::numeric_limits<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 <typename T>
+T FuzzedDataProvider::ConsumeIntegralInRange(T min, T max) {
+  static_assert(std::is_integral<T>::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<uint64_t>(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<decltype(range)>::max())
+    result = result % (range + 1);
+
+  return static_cast<T>(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 <typename T> T FuzzedDataProvider::ConsumeFloatingPoint() {
+  return ConsumeFloatingPointInRange<T>(std::numeric_limits<T>::lowest(),
+                                        std::numeric_limits<T>::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 <typename T>
+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<T>::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<T>();
+}
+
+// Returns a floating point number in the range [0.0, 1.0]. If there's no
+// input data left, always returns 0.
+template <typename T> T FuzzedDataProvider::ConsumeProbability() {
+  static_assert(std::is_floating_point<T>::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<T>(ConsumeIntegral<IntegralType>());
+  result /= static_cast<T>(std::numeric_limits<IntegralType>::max());
+  return result;
+}
+
+// Reads one byte and returns a bool, or false when no data remains.
+inline bool FuzzedDataProvider::ConsumeBool() {
+  return 1 & ConsumeIntegral<uint8_t>();
+}
+
+// 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 <typename T> T FuzzedDataProvider::ConsumeEnum() {
+  static_assert(std::is_enum<T>::value, "|T| must be an enum type.");
+  return static_cast<T>(
+      ConsumeIntegralInRange<uint32_t>(0, static_cast<uint32_t>(T::kMaxValue)));
+}
+
+// Returns a copy of the value selected from the given fixed-size |array|.
+template <typename T, size_t size>
+T FuzzedDataProvider::PickValueInArray(const T (&array)[size]) {
+  static_assert(size > 0, "The array must be non empty.");
+  return array[ConsumeIntegralInRange<size_t>(0, size - 1)];
+}
+
+template <typename T>
+T FuzzedDataProvider::PickValueInArray(std::initializer_list<const T> list) {
+  // TODO(Dor1s): switch to static_assert once C++14 is allowed.
+  if (!list.size())
+    abort();
+
+  return *(list.begin() + ConsumeIntegralInRange<size_t>(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 <typename T>
+std::vector<T> 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<T> 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 <typename TS, typename TU>
+TS FuzzedDataProvider::ConvertUnsignedToSigned(TU value) {
+  static_assert(sizeof(TS) == sizeof(TU), "Incompatible data types.");
+  static_assert(!std::numeric_limits<TU>::is_signed,
+                "Source type must be unsigned.");
+
+  // TODO(Dor1s): change to `if constexpr` once C++17 becomes mainstream.
+  if (std::numeric_limits<TS>::is_modulo)
+    return static_cast<TS>(value);
+
+  // Avoid using implementation-defined unsigned to signed conversions.
+  // To learn more, see https://stackoverflow.com/questions/13150449.
+  if (value <= std::numeric_limits<TS>::max()) {
+    return static_cast<TS>(value);
+  } else {
+    constexpr auto TS_min = std::numeric_limits<TS>::min();
+    return TS_min + static_cast<char>(value - TS_min);
+  }
+}
+
 #endif // LLVM_FUZZER_FUZZED_DATA_PROVIDER_H_

src/test/fuzz/fuzz.cpp

@@ -12,6 +12,7 @@
 
 const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
+#if defined(PROVIDE_MAIN_FUNCTION)
 static bool read_stdin(std::vector<uint8_t>& data)
 {
     uint8_t buffer[1024];
@@ -21,6 +22,7 @@ static bool read_stdin(std::vector<uint8_t>& data)
     }
     return length == 0;
 }
+#endif
 
 // Default initialization: Override using a non-weak initialize().
 __attribute__((weak)) void initialize()
@@ -42,8 +44,7 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv)
     return 0;
 }
 
-// Declare main(...) "weak" to allow for libFuzzer linking. libFuzzer provides
+#if defined(PROVIDE_MAIN_FUNCTION)
-// the main(...) function.
 __attribute__((weak)) int main(int argc, char** argv)
 {
     initialize();
@@ -72,3 +73,4 @@ __attribute__((weak)) int main(int argc, char** argv)
 #endif
     return 0;
 }
+#endif

src/test/fuzz/parse_iso8601.cpp

@@ -15,7 +15,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
-    const int64_t random_time = fuzzed_data_provider.ConsumeIntegral<int64_t>();
+    const int64_t random_time = fuzzed_data_provider.ConsumeIntegral<int32_t>();
     const std::string random_string = fuzzed_data_provider.ConsumeRemainingBytesAsString();
 
     const std::string iso8601_datetime = FormatISO8601DateTime(random_time);

src/test/fuzz/strprintf.cpp

@@ -8,7 +8,6 @@
 #include <util/strencodings.h>
 
 #include <algorithm>
-#include <cassert>
 #include <cstdint>
 #include <string>
 #include <vector>
@@ -27,7 +26,7 @@ void test_one_input(const std::vector<uint8_t>& 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<uint8_t>& 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<uint8_t>& 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<uint8_t>& 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<float>());
             break;
-        case 7:
+        case 1:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
             break;
-        case 8:
+        case 2:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
             break;
-        case 9:
+        case 3:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
             break;
-        case 10:
+        case 4:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
             break;
-        case 11:
+        case 5:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
             break;
-        case 12:
+        case 6:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
             break;
-        case 13:
+        case 7:
             (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
             break;
-        default:
-            assert(false);
         }
     } catch (const tinyformat::format_error&) {
     }

test/lint/lint-locale-dependence.sh

@@ -124,6 +124,7 @@ LOCALE_DEPENDENT_FUNCTIONS=(
     snprintf
     sprintf
     sscanf
+    std::locale::global
     std::to_string
     stod
     stof