From f73fce7782dd946f3ac3c25dd87b3bf0d23d4681 Mon Sep 17 00:00:00 2001 From: gabriel-bjg Date: Mon, 10 Jan 2022 19:31:45 +0200 Subject: [PATCH] ci: Enable tsan on linux64 build (#4563) Make fMixing atomic as it has concurrent access Add tsan suppression for zmq namespace Suppress deadlock false positive in ConnectTip Switch ubsan target to linux32 Add new test job for linux64_cxx17 target without any sanitizers Increase rpc time out for block reward reallocation test Fix heap use after free in CConnman::GetExtraOutboundCount() Different builds for linux32 and linux64 tsan and ubsan Increase timeout for llmq_signing functional test --- .gitlab-ci.yml | 36 ++++++++++++++++++++++++++++++++ ci/dash/matrix.sh | 18 ++++++++++++++-- src/coinjoin/client.cpp | 6 ++---- src/coinjoin/client.h | 3 ++- src/net.cpp | 5 +++-- test/sanitizer_suppressions/tsan | 6 ++++++ 6 files changed, 65 insertions(+), 9 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cd3109ff69..1a7617b06e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -234,6 +234,15 @@ linux32-build: variables: BUILD_TARGET: linux32 +linux32_ubsan-build: + extends: + - .build-template + - .skip-in-fast-mode-template + needs: + - i686-pc-linux-gnu + variables: + BUILD_TARGET: linux32_ubsan + linux64-build: extends: .build-template needs: @@ -248,6 +257,15 @@ linux64_cxx20-build: variables: BUILD_TARGET: linux64_cxx20 +linux64_tsan-build: + extends: + - .build-template + - .skip-in-fast-mode-template + needs: + - x86_64-unknown-linux-gnu-debug + variables: + BUILD_TARGET: linux64_tsan + linux64_nowallet-build: extends: - .build-template @@ -286,9 +304,27 @@ linux32-test: variables: BUILD_TARGET: linux32 +linux32_ubsan-test: + extends: + - .test-template + - .skip-in-fast-mode-template + needs: + - linux32_ubsan-build + variables: + BUILD_TARGET: linux32_ubsan + linux64-test: extends: .test-template needs: - linux64-build variables: BUILD_TARGET: linux64 + +linux64_tsan-test: + extends: + - .test-template + - .skip-in-fast-mode-template + needs: + - linux64_tsan-build + variables: + BUILD_TARGET: linux64_tsan diff --git a/ci/dash/matrix.sh b/ci/dash/matrix.sh index ca6c8c9b36..c33658732c 100755 --- a/ci/dash/matrix.sh +++ b/ci/dash/matrix.sh @@ -35,6 +35,9 @@ export MAKEJOBS export RUN_UNITTESTS=true export RUN_INTEGRATIONTESTS=true +# Configure sanitizers options +export TSAN_OPTIONS="suppressions=${SRC_DIR}/test/sanitizer_suppressions/tsan" + if [ "$BUILD_TARGET" = "arm-linux" ]; then export HOST=arm-linux-gnueabihf export CHECK_DOC=1 @@ -53,16 +56,27 @@ elif [ "$BUILD_TARGET" = "linux32" ]; then export BITCOIN_CONFIG="--enable-zmq --disable-bip70 --enable-reduce-exports --enable-crash-hooks" export USE_SHELL="/bin/dash" export PYZMQ=true +elif [ "$BUILD_TARGET" = "linux32_ubsan" ]; then + export HOST=i686-pc-linux-gnu + export BITCOIN_CONFIG="--enable-zmq --disable-bip70 --enable-reduce-exports --enable-crash-hooks --with-sanitizers=undefined" + export USE_SHELL="/bin/dash" + export PYZMQ=true elif [ "$BUILD_TARGET" = "linux64" ]; then export HOST=x86_64-unknown-linux-gnu export DEP_OPTS="NO_UPNP=1 DEBUG=1" - export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=undefined" + export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks" + export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG" + export PYZMQ=true +elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then + export HOST=x86_64-unknown-linux-gnu + export DEP_OPTS="NO_UPNP=1 DEBUG=1" + export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=thread" export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG" export PYZMQ=true elif [ "$BUILD_TARGET" = "linux64_cxx20" ]; then export HOST=x86_64-unknown-linux-gnu export DEP_OPTS="NO_UPNP=1 DEBUG=1" - export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --enable-c++20 --enable-suppress-external-warnings --enable-werror --with-sanitizers=undefined" + export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --enable-c++20 --enable-suppress-external-warnings --enable-werror" export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG" export PYZMQ=true export RUN_INTEGRATIONTESTS=false diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index b3ef3be3db..7029042229 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -235,10 +235,8 @@ void CCoinJoinClientSession::ProcessMessage(CNode* pfrom, const std::string& str } bool CCoinJoinClientManager::StartMixing() { - if (IsMixing()) { - return false; - } - return fMixing = true; + bool expected{false}; + return fMixing.compare_exchange_strong(expected, true); } void CCoinJoinClientManager::StopMixing() { diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index ce795da096..7422cb543a 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -9,6 +9,7 @@ #include #include +#include class CDeterministicMN; using CDeterministicMNCPtr = std::shared_ptr; @@ -178,7 +179,7 @@ private: // TODO: or map ?? std::deque deqSessions GUARDED_BY(cs_deqsessions); - bool fMixing{false}; + std::atomic fMixing{false}; int nCachedLastSuccessBlock{0}; int nMinBlocksToWait{1}; // how many blocks to wait for after one successful mixing tx in non-multisession mode diff --git a/src/net.cpp b/src/net.cpp index 9d9697035a..16a513443d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3218,13 +3218,14 @@ void CConnman::Stop() } // clean up some globals (to help leak detection) - for (CNode *pnode : vNodes) { + std::vector nodes; + WITH_LOCK(cs_vNodes, nodes.swap(vNodes)); + for (CNode *pnode : nodes) { DeleteNode(pnode); } for (CNode *pnode : vNodesDisconnected) { DeleteNode(pnode); } - vNodes.clear(); mapSocketToNode.clear(); { LOCK(cs_vNodes); diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index a5909ee3b0..7e8be7c705 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -1,9 +1,15 @@ # ThreadSanitizer suppressions # ============================ +# Data races from zmq namespace +race:zmq::* + # WalletBatch (unidentified deadlock) deadlock:WalletBatch +# deadlock false positive (see: https://github.com/dashpay/dash/pull/4563) +deadlock:CChainState::ConnectTip + # Intentional deadlock in tests deadlock:sync_tests::potential_deadlock_detected