Merge pull request #4341 from PastaPastaPasta/backports-0.18-pr14

Backports 0.18 pr14

.travis.yml

@@ -252,33 +252,25 @@ after_success:
         DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1"
         GOAL="install"
         BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-debug CXXFLAGS=\"-g0 -O2\""
-# x86_64 Linux (xenial, no depends, only system libs)
+# x86_64 Linux (xenial, no depends, only system libs, sanitizers: thread (TSAN))
     - stage: test
       env: >-
         HOST=x86_64-unknown-linux-gnu
         DOCKER_NAME_TAG=ubuntu:16.04
-        PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libssl-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 libprotobuf-dev protobuf-compiler libqrencode-dev"
+        PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libssl-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 libprotobuf-dev protobuf-compiler libqrencode-dev"
         NO_DEPENDS=1
+        RUN_FUNCTIONAL_TESTS=false # Disabled for now. TODO identify suppressions or exclude specific tests
         GOAL="install"
-        BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER"
+        BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++"
-# x86_64 Linux (no depends, only system libs)
+# x86_64 Linux (no depends, only system libs, sanitizers: undefined (UBSAN) + integer)
     - stage: test
       env: >-
         HOST=x86_64-unknown-linux-gnu
-        PACKAGES="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 libprotobuf-dev protobuf-compiler libqrencode-dev"
+        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 libprotobuf-dev protobuf-compiler libqrencode-dev"
-        NO_DEPENDS=1
-        GOAL="install"
-        BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER"
-# x86_64 Linux (sanitizers)
-    - stage: test
-      env: >-
-        HOST=x86_64-unknown-linux-gnu
-        PACKAGES="clang 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 libprotobuf-dev protobuf-compiler libqrencode-dev"
         NO_DEPENDS=1
         RUN_BENCH=true
-        RUN_FUNCTIONAL_TESTS=false # Disabled for now, can be combined with the other x86_64 linux NO_DEPENDS job when functional tests pass the sanitizers
         GOAL="install"
-        BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=undefined CC=clang CXX=clang++"
+        BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=integer,undefined CC=clang CXX=clang++"
 # x86_64 Linux, No wallet
     - stage: test
       env: >-

.travis/test_03_before_install.sh

@@ -7,6 +7,8 @@
 export LC_ALL=C.UTF-8
 
 PATH=$(echo $PATH | tr ':' "\n" | sed '/\/opt\/python/d' | tr "\n" ":" | sed "s|::|:|g")
+# Add llvm-symbolizer directory to PATH. Needed to get symbolized stack traces from the sanitizers.
+PATH=$PATH:/usr/lib/llvm-6.0/bin/
 export PATH
 
 BEGIN_FOLD () {

.travis/test_04_install.sh

@@ -7,7 +7,9 @@
 export LC_ALL=C.UTF-8
 
 travis_retry docker pull "$DOCKER_NAME_TAG"
-env | grep -E '^(CCACHE_|WINEDEBUG|LC_ALL|BOOST_TEST_RANDOM|CONFIG_SHELL)' | tee /tmp/env
+export TSAN_OPTIONS="suppressions=${TRAVIS_BUILD_DIR}/test/sanitizer_suppressions/tsan"
+export UBSAN_OPTIONS="suppressions=${TRAVIS_BUILD_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1"
+env | grep -E '^(CCACHE_|WINEDEBUG|LC_ALL|BOOST_TEST_RANDOM|CONFIG_SHELL|(TSAN|UBSAN)_OPTIONS)' | tee /tmp/env
 if [[ $HOST = *-mingw32 ]]; then
   DOCKER_ADMIN="--cap-add SYS_ADMIN"
 elif [[ $BITCOIN_CONFIG = *--with-sanitizers=*address* ]]; then # If ran with (ASan + LSan), Docker needs access to ptrace (https://github.com/google/sanitizers/issues/764)

src/net_processing.h

@@ -75,7 +75,7 @@ public:
     void EvictExtraOutboundPeers(int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 private:
-    int64_t m_stale_tip_check_time; //! Next time to check for stale tip
+    int64_t m_stale_tip_check_time; //!< Next time to check for stale tip
 
     /** Enable BIP61 (sending reject messages) */
     const bool m_enable_bip61;

src/netbase.cpp

@@ -197,10 +197,10 @@ enum SOCKSVersion: uint8_t {
 
 /** Values defined for METHOD in RFC1928 */
 enum SOCKS5Method: uint8_t {
-    NOAUTH = 0x00,        //! No authentication required
+    NOAUTH = 0x00,        //!< No authentication required
-    GSSAPI = 0x01,        //! GSSAPI
+    GSSAPI = 0x01,        //!< GSSAPI
-    USER_PASS = 0x02,     //! Username/password
+    USER_PASS = 0x02,     //!< Username/password
-    NO_ACCEPTABLE = 0xff, //! No acceptable methods
+    NO_ACCEPTABLE = 0xff, //!< No acceptable methods
 };
 
 /** Values defined for CMD in RFC1928 */
@@ -212,15 +212,15 @@ enum SOCKS5Command: uint8_t {
 
 /** Values defined for REP in RFC1928 */
 enum SOCKS5Reply: uint8_t {
-    SUCCEEDED = 0x00,        //! Succeeded
+    SUCCEEDED = 0x00,        //!< Succeeded
-    GENFAILURE = 0x01,       //! General failure
+    GENFAILURE = 0x01,       //!< General failure
-    NOTALLOWED = 0x02,       //! Connection not allowed by ruleset
+    NOTALLOWED = 0x02,       //!< Connection not allowed by ruleset
-    NETUNREACHABLE = 0x03,   //! Network unreachable
+    NETUNREACHABLE = 0x03,   //!< Network unreachable
-    HOSTUNREACHABLE = 0x04,  //! Network unreachable
+    HOSTUNREACHABLE = 0x04,  //!< Network unreachable
-    CONNREFUSED = 0x05,      //! Connection refused
+    CONNREFUSED = 0x05,      //!< Connection refused
-    TTLEXPIRED = 0x06,       //! TTL expired
+    TTLEXPIRED = 0x06,       //!< TTL expired
-    CMDUNSUPPORTED = 0x07,   //! Command not supported
+    CMDUNSUPPORTED = 0x07,   //!< Command not supported
-    ATYPEUNSUPPORTED = 0x08, //! Address type not supported
+    ATYPEUNSUPPORTED = 0x08, //!< Address type not supported
 };
 
 /** Values defined for ATYPE in RFC1928 */

src/policy/fees.h

@@ -50,9 +50,9 @@ std::string StringForFeeReason(FeeReason reason);
 
 /* Used to determine type of fee estimation requested */
 enum class FeeEstimateMode {
-    UNSET,        //! Use default settings based on other criteria
+    UNSET,        //!< Use default settings based on other criteria
-    ECONOMICAL,   //! Force estimateSmartFee to use non-conservative estimates
+    ECONOMICAL,   //!< Force estimateSmartFee to use non-conservative estimates
-    CONSERVATIVE, //! Force estimateSmartFee to use conservative estimates
+    CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
 };
 
 bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode);

src/rpc/util.cpp

@@ -5,7 +5,6 @@
 #include <key_io.h>
 #include <keystore.h>
 #include <pubkey.h>
-#include <rpc/protocol.h>
 #include <rpc/util.h>
 #include <tinyformat.h>
 #include <util/strencodings.h>

src/rpc/util.h

@@ -7,10 +7,10 @@
 
 #include <node/transaction.h>
 #include <pubkey.h>
+#include <rpc/protocol.h>
 #include <script/standard.h>
 #include <univalue.h>
 #include <util/strencodings.h>
-#include <rpc/protocol.h>
 
 #include <string>
 #include <vector>

src/txmempool.h

@@ -352,12 +352,12 @@ struct TxMempoolInfo
  * this is passed to the notification signal.
  */
 enum class MemPoolRemovalReason {
-    UNKNOWN = 0, //! Manually removed or unknown reason
+    UNKNOWN = 0, //!< Manually removed or unknown reason
-    EXPIRY,      //! Expired from mempool
+    EXPIRY,      //!< Expired from mempool
-    SIZELIMIT,   //! Removed in size limiting
+    SIZELIMIT,   //!< Removed in size limiting
-    REORG,       //! Removed for reorganization
+    REORG,       //!< Removed for reorganization
-    BLOCK,       //! Removed for block
+    BLOCK,       //!< Removed for block
-    CONFLICT,    //! Removed for conflict with in-block transaction
+    CONFLICT,    //!< Removed for conflict with in-block transaction
 };
 
 class SaltedTxidHasher

test/sanitizer_suppressions/tsan

@@ -0,0 +1,24 @@
+# ThreadSanitizer suppressions
+# ============================
+
+# fChecked is theoretically racy, practically only in unit tests
+race:CheckBlock
+
+# WalletBatch (unidentified deadlock)
+deadlock:WalletBatch
+
+# Intentional deadlock in tests
+deadlock:TestPotentialDeadLockDetected
+
+# Wildcard for all gui tests, should be replaced with non-wildcard suppressions
+race:src/qt/test/*
+deadlock:src/qt/test/*
+
+# WIP: Unidentified suppressions to run the functional tests
+#race:zmqpublishnotifier.cpp
+#
+#deadlock:CreateWalletFromFile
+#deadlock:importprivkey
+#deadlock:walletdb.h
+#deadlock:walletdb.cpp
+#deadlock:wallet/db.cpp

test/sanitizer_suppressions/ubsan

@@ -0,0 +1,36 @@
+alignment:move.h
+alignment:prevector.h
+bool:wallet/wallet.cpp
+float-divide-by-zero:policy/fees.cpp
+float-divide-by-zero:validation.cpp
+float-divide-by-zero:wallet/wallet.cpp
+nonnull-attribute:support/cleanse.cpp
+unsigned-integer-overflow:arith_uint256.h
+unsigned-integer-overflow:basic_string.h
+unsigned-integer-overflow:bench/bench.h
+unsigned-integer-overflow:bitcoin-tx.cpp
+unsigned-integer-overflow:bloom.cpp
+unsigned-integer-overflow:chain.cpp
+unsigned-integer-overflow:chain.h
+unsigned-integer-overflow:coded_stream.h
+unsigned-integer-overflow:core_write.cpp
+unsigned-integer-overflow:crypto/chacha20.cpp
+unsigned-integer-overflow:crypto/ctaes/ctaes.c
+unsigned-integer-overflow:crypto/ripemd160.cpp
+unsigned-integer-overflow:crypto/sha1.cpp
+unsigned-integer-overflow:crypto/sha256.cpp
+unsigned-integer-overflow:crypto/sha512.cpp
+unsigned-integer-overflow:hash.cpp
+unsigned-integer-overflow:leveldb/db/log_reader.cc
+unsigned-integer-overflow:leveldb/util/bloom.cc
+unsigned-integer-overflow:leveldb/util/crc32c.h
+unsigned-integer-overflow:leveldb/util/hash.cc
+unsigned-integer-overflow:policy/fees.cpp
+unsigned-integer-overflow:prevector.h
+unsigned-integer-overflow:script/interpreter.cpp
+unsigned-integer-overflow:stl_bvector.h
+unsigned-integer-overflow:streams.h
+unsigned-integer-overflow:txmempool.cpp
+unsigned-integer-overflow:util/strencodings.cpp
+unsigned-integer-overflow:validation.cpp
+vptr:fs.cpp