build: add option for sanitizers

.github/workflows/ci.yml

@@ -23,6 +23,11 @@ jobs:
                     script-id: native
                   - name: x86_64 Linux  [GOAL install]  [GUI]  [bionic]  [no depends]
                     script-id: native_old
+                  - name: x86_64 Linux  [ASan]  [LSan]  [UBSan]  [integer]  [jammy]  [no depends]
+                    script-id: native_asan
+                  # FIXME: depends is unable to compile Qt with clang.
+                  # - name: x86_64 Linux  [TSan]  [GUI]  [jammy]
+                  #  script-id: native_tsan
                   - name: macOS 10.14  [GOAL deploy]  [GUI]  [no tests]  [focal]
                     script-id: mac
         env:

ci/test/00_setup_env_native_asan.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2019-2022 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/licenses/mit-license.php.
+
+export LC_ALL=C.UTF-8
+
+export CONTAINER_NAME=ci_native_asan
+export PACKAGES="clang llvm libqt5gui5 libqt5core5a qtbase5-dev libqt5dbus5 qttools5-dev qttools5-dev-tools libssl-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-iostreams-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libqrencode-dev libzip-dev zlib1g zlib1g-dev libcurl4 libcurl4-openssl-dev"
+export DOCKER_NAME_TAG=ubuntu:22.04
+export NO_DEPENDS=1
+export GOAL="install"
+export GRIDCOIN_CONFIG="--with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++"

ci/test/00_setup_env_native_tsan.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2019-2022 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/licenses/mit-license.php.
+
+export LC_ALL=C.UTF-8
+
+export CONTAINER_NAME=ci_native_tsan
+export DOCKER_NAME_TAG=ubuntu:22.04
+export PACKAGES="clang-13 llvm-13 libc++abi-13-dev libc++-13-dev"
+export DEP_OPTS="CC=clang-13 CXX='clang++-13 -stdlib=libc++'"
+export GOAL="install"
+export GRIDCOIN_CONFIG="CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' CXXFLAGS='-g' --with-sanitizers=thread CC=clang-13 CXX='clang++-13 -stdlib=libc++'"

ci/test/04_install.sh

@@ -17,6 +17,11 @@ fi
 mkdir -p "${CCACHE_DIR}"
 mkdir -p "${PREVIOUS_RELEASES_DIR}"
 
+export ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1"
+export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan"
+export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1:log_path=${BASE_SCRATCH_DIR}/sanitizer-output/tsan"
+export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1"
+
 env | grep -E '^(GRIDCOIN_CONFIG|BASE_|QEMU_|CCACHE_|LC_ALL|BOOST_TEST_RANDOM|DEBIAN_FRONTEND|CONFIG_SHELL|(ASAN|LSAN|TSAN|UBSAN)_OPTIONS|PREVIOUS_RELEASES_DIR)' | tee /tmp/env
 if [[ $HOST = *-mingw32 ]]; then
   DOCKER_ADMIN="--cap-add SYS_ADMIN"

configure.ac

@@ -209,6 +209,12 @@ AC_ARG_ENABLE([debug],
     [enable_debug=$enableval],
     [enable_debug=no])
 
+dnl Enable different -fsanitize options
+AC_ARG_WITH([sanitizers],
+    [AS_HELP_STRING([--with-sanitizers],
+                    [comma separated list of extra sanitizers to build with (default is none enabled)])],
+    [use_sanitizers=$withval])
+
 # Turn warnings into errors
 AC_ARG_ENABLE([werror],
     [AS_HELP_STRING([--enable-werror],
@@ -230,6 +236,33 @@ if test "$enable_debug" = "yes"; then
     fi
 fi
 
+if test "$use_sanitizers" != ""; then
+  dnl First check if the compiler accepts flags. If an incompatible pair like
+  dnl -fsanitize=address,thread is used here, this check will fail. This will also
+  dnl fail if a bad argument is passed, e.g. -fsanitize=undfeined
+  AX_CHECK_COMPILE_FLAG(
+    [-fsanitize=$use_sanitizers],
+    [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers"],
+    [AC_MSG_ERROR([compiler did not accept requested flags])])
+
+  dnl Some compilers (e.g. GCC) require additional libraries like libasan,
+  dnl libtsan, libubsan, etc. Make sure linking still works with the sanitize
+  dnl flag. This is a separate check so we can give a better error message when
+  dnl the sanitize flags are supported by the compiler but the actual sanitizer
+  dnl libs are missing.
+  AX_CHECK_LINK_FLAG(
+    [-fsanitize=$use_sanitizers],
+    [SANITIZER_LDFLAGS="-fsanitize=$use_sanitizers"],
+    [AC_MSG_ERROR([linker did not accept requested flags, you are missing required libraries])],
+    [],
+    [AC_LANG_PROGRAM([[
+    #include <cstdint>
+    #include <cstddef>
+    extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; }
+    __attribute__((weak)) // allow for libFuzzer linking
+    ]],[[]])])
+fi
+
 ERROR_CXXFLAGS=
 if test "$enable_werror" = "yes"; then
   if test "$CXXFLAG_WERROR" = ""; then
@@ -1292,6 +1325,8 @@ AC_SUBST(HARDENED_CPPFLAGS)
 AC_SUBST(HARDENED_LDFLAGS)
 AC_SUBST(PIC_FLAGS)
 AC_SUBST(PIE_FLAGS)
+AC_SUBST(SANITIZER_CXXFLAGS)
+AC_SUBST(SANITIZER_LDFLAGS)
 AC_SUBST(SSE42_CXXFLAGS)
 AC_SUBST(SSE41_CXXFLAGS)
 AC_SUBST(AVX2_CXXFLAGS)
@@ -1398,6 +1433,7 @@ echo "  with zmq      = $use_zmq"
 echo "  with test     = $use_tests"
 echo "  with bench    = $use_bench"
 echo "  with upnp     = $use_upnp"
+echo "  sanitizers    = $use_sanitizers"
 echo "  debug enabled = $enable_debug"
 echo "  werror        = $enable_werror"
 echo

src/Makefile.am

@@ -8,8 +8,8 @@ print-%: FORCE
 
 DIST_SUBDIRS = univalue secp256k1
 
-AM_LDFLAGS = ${libcurl_LIBS} $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS)
-AM_CXXFLAGS = $(HARDENED_CXXFLAGS) $(ERROR_CXXFLAGS)
+AM_LDFLAGS = ${libcurl_LIBS} $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(SANITIZER_LDFLAGS)
+AM_CXXFLAGS = $(HARDENED_CXXFLAGS) $(ERROR_CXXFLAGS) $(SANITIZER_CXXFLAGS)
 AM_CPPFLAGS = ${libcurl_CFLAGS} $(HARDENED_CPPFLAGS) -DSTATICLIB -DCURL_STATICLIB -DMINIUPNP_STATICLIB -DZIP_STATIC -DNN_STATIC_LIB
 EXTRA_LIBRARIES =
 

src/gridcoin/cpid.h

@@ -454,9 +454,9 @@ namespace std {
 //! This enables the use of GRC::Cpid as a key in a std::unordered_map object.
 //!
 //! CONSENSUS: Don't use the hash produced by this routine (or by any std::hash
-//! specialization) in protocol-specific implementations. It ignores endianness
-//! and outputs a value with a chance of collision probably too great for usage
-//! besides the intended local look-up functionality.
+//! specialization) in protocol-specific implementations. It outputs a value
+//! with a chance of collision probably too great for usage besides the intended
+//! local look-up functionality.
 //!
 template<>
 struct hash<GRC::Cpid>
@@ -473,8 +473,16 @@ struct hash<GRC::Cpid>
         // Just convert the CPID into a value that we can store in a size_t
         // object. CPIDs are already unique identifiers.
         //
-        return *reinterpret_cast<const uint64_t*>(cpid.Raw().data())
-            + *reinterpret_cast<const uint64_t*>(cpid.Raw().data() + 8);
+        const auto& data = cpid.Raw();
+        size_t ret = ((size_t)(data[0] & 255)       | (size_t)(data[1] & 255) << 8 |
+                      (size_t)(data[2] & 255) << 16 | (size_t)(data[3] & 255) << 24);
+
+        if (sizeof(size_t) == 8) {
+            ret |= ((size_t)(data[4] & 255) << 32 | (size_t)(data[5] & 255) << 40 |
+                    (size_t)(data[6] & 255) << 48 | (size_t)(data[7] & 255) << 56);
+        }
+
+        return ret;
     }
 };
 }

src/gridcoin/mrc.h

@@ -125,14 +125,14 @@ class MRC : public IContractPayload
     //! incoming reward claims and can index those calculated values without
     //! this field. It can be considered informational.
     //!
-    CAmount m_research_subsidy;
+    CAmount m_research_subsidy = 0;
 
     //!
     //! \brief The value of the fees charged to the MRC claimant. These will be
     //! subtracted from the research subsidy and distributed to the staker and
     //! the foundation according to protocol rules encapsulated in ComputeMRCFee().
     //!
-    CAmount m_fee;
+    CAmount m_fee = 0;
 
     //!
     //! \brief The researcher magnitude value from the superblock at the time
@@ -145,14 +145,14 @@ class MRC : public IContractPayload
     //!
     //! Previous protocol versions used the magnitude in reward calculations.
     //!
-    uint16_t m_magnitude;
+    uint16_t m_magnitude = 0;
 
     //!
     //! \brief The magnitude ratio of the network at the time of the claim.
     //!
     //! Informational.
     //!
-    double m_magnitude_unit;
+    double m_magnitude_unit = 0.0;
 
     //!
     //! \brief The hash of the last block (head of the chain) for the MRC

src/gridcoin/scraper/scraper_net.cpp

@@ -139,9 +139,7 @@ int CSplitBlob::addPartData(CDataStream&& vData)
     {
         /* missing data; use the supplied data */
         /* prevent calling the Complete callback FIXME: make this look better */
-        cntPartsRcvd--;
         CSplitBlob::RecvPart(nullptr, vData);
-        cntPartsRcvd++;
     }
     return n;
 }

src/main.h

@@ -478,7 +478,7 @@ class CBlockIndex
     int nHeight;
 
     unsigned int nFlags;  // ppcoin: block index flags
-    enum
+    enum : uint32_t
     {
         BLOCK_PROOF_OF_STAKE = (1 << 0), // is proof-of-stake block
         BLOCK_STAKE_ENTROPY  = (1 << 1), // entropy bit for stake modifier

src/miner.cpp

@@ -310,8 +310,8 @@ bool CreateRestOfTheBlock(CBlock &block, CBlockIndex* pindexPrev,
     int nHeight = pindexPrev->nHeight + 1;
 
     // This is specifically for BlockValidateContracts, and only the nHeight is filled in.
-    CBlockIndex* pindex_contract_validate = new CBlockIndex();
-    pindex_contract_validate->nHeight = nHeight;
+    CBlockIndex pindex_contract_validate;
+    pindex_contract_validate.nHeight = nHeight;
 
     // Create coinbase tx
     CTransaction &CoinBase = block.vtx[0];
@@ -385,7 +385,7 @@ bool CreateRestOfTheBlock(CBlock &block, CBlockIndex* pindexPrev,
             // pindex_contract_validate only has the block height filled out.
             //
             int DoS = 0; // Unused here.
-            if (!tx.GetContracts().empty() && !GRC::BlockValidateContracts(pindex_contract_validate, tx, DoS)) {
+            if (!tx.GetContracts().empty() && !GRC::BlockValidateContracts(&pindex_contract_validate, tx, DoS)) {
                 LogPrint(BCLog::LogFlags::MINER,
                     "%s: contract failed contextual validation. Skipped tx %s",
                     __func__,

src/support/lockedpool.cpp

@@ -24,6 +24,10 @@
 
 #include <algorithm>
 #include <stdexcept>
+#ifdef ARENA_DEBUG
+#include <iomanip>
+#include <iostream>
+#endif
 
 LockedPoolManager* LockedPoolManager::_instance = nullptr;
 std::once_flag LockedPoolManager::init_flag;
@@ -138,7 +142,7 @@ Arena::Stats Arena::stats() const
 }
 
 #ifdef ARENA_DEBUG
-static void printchunk(char* base, size_t sz, bool used) {
+static void printchunk(void* base, size_t sz, bool used) {
     std::cout <<
         "0x" << std::hex << std::setw(16) << std::setfill('0') << base <<
         " 0x" << std::hex << std::setw(16) << std::setfill('0') << sz <<
@@ -150,7 +154,7 @@ void Arena::walk() const
         printchunk(chunk.first, chunk.second, true);
     std::cout << std::endl;
     for (const auto& chunk: chunks_free)
-        printchunk(chunk.first, chunk.second, false);
+        printchunk(chunk.first, chunk.second->first, false);
     std::cout << std::endl;
 }
 #endif

src/test/accounting_tests.cpp

@@ -78,13 +78,13 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
 
 
     wtx.mapValue["comment"] = "y";
-    --wtx.nLockTime;  // Just to change the hash :)
+    ++wtx.nLockTime;  // Just to change the hash :)
     pwalletMain->AddToWallet(wtx, &walletdb);
     vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]);
     vpwtx[1]->nTimeReceived = (unsigned int)1333333336;
 
     wtx.mapValue["comment"] = "x";
-    --wtx.nLockTime;  // Just to change the hash :)
+    ++wtx.nLockTime;  // Just to change the hash :)
     pwalletMain->AddToWallet(wtx, &walletdb);
     vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]);
     vpwtx[2]->nTimeReceived = (unsigned int)1333333329;

src/test/gridcoin/beacon_tests.cpp

@@ -1268,10 +1268,10 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test)
     tx1.nTime = int64_t {1};
     uint256 tx1_hash = tx1.GetHash();
 
-    CBlockIndex* pindex1 = new CBlockIndex;
-    pindex1->nVersion = 13;
-    pindex1->nHeight = 1;
-    pindex1->nTime = tx1.nTime;
+    CBlockIndex pindex1 {};
+    pindex1.nVersion = 13;
+    pindex1.nHeight = 1;
+    pindex1.nTime = tx1.nTime;
 
     GRC::Beacon beacon1 {TestKey::Public(), tx1.nTime, tx1_hash};
     beacon1.m_cpid = TestKey::Cpid();
@@ -1280,7 +1280,7 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test)
     beacon_payload1.m_signature = TestKey::Signature(beacon_payload1);
 
     GRC::Contract contract1 = GRC::MakeContract<GRC::BeaconPayload>(3, GRC::ContractAction::ADD, beacon_payload1);
-    GRC::ContractContext ctx1 {contract1, tx1, pindex1};
+    GRC::ContractContext ctx1 {contract1, tx1, &pindex1};
 
     BOOST_CHECK(ctx1.m_contract.CopyPayloadAs<GRC::BeaconPayload>().m_cpid == TestKey::Cpid());
     BOOST_CHECK(ctx1.m_contract.CopyPayloadAs<GRC::BeaconPayload>().m_beacon.m_status == GRC::BeaconStatusForStorage::PENDING);
@@ -1305,18 +1305,18 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test)
     }
 
     // Activation
-    CBlockIndex* pindex2 = new CBlockIndex;
-    pindex2->nVersion = 13;
-    pindex2->nHeight = 2;
-    pindex2->nTime = int64_t {2};
-    uint256* block2_phash = new uint256 {rng.rand256()};
-    pindex2->phashBlock = block2_phash;
+    CBlockIndex pindex2 {};
+    pindex2.nVersion = 13;
+    pindex2.nHeight = 2;
+    pindex2.nTime = int64_t {2};
+    uint256 block2_phash = rng.rand256();
+    pindex2.phashBlock = &block2_phash;
 
     std::vector<uint160> beacon_ids {TestKey::Public().GetID()};
 
-    registry.ActivatePending(beacon_ids, pindex2->nTime, *pindex2->phashBlock, pindex2->nHeight);
+    registry.ActivatePending(beacon_ids, pindex2.nTime, *pindex2.phashBlock, pindex2.nHeight);
 
-    uint256 activated_beacon_hash = Hash(*block2_phash, pending_beacons[0]->m_hash);
+    uint256 activated_beacon_hash = Hash(block2_phash, pending_beacons[0]->m_hash);
 
     BOOST_CHECK(registry.GetBeaconDB().size() == 2);
 
@@ -1475,10 +1475,10 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test_2)
     tx1.nTime = int64_t {1};
     uint256 tx1_hash = tx1.GetHash();
 
-    CBlockIndex* pindex1 = new CBlockIndex;
-    pindex1->nVersion = 13;
-    pindex1->nHeight = 1;
-    pindex1->nTime = tx1.nTime;
+    CBlockIndex pindex1 {};
+    pindex1.nVersion = 13;
+    pindex1.nHeight = 1;
+    pindex1.nTime = tx1.nTime;
 
     GRC::Beacon beacon1 {TestKey::Public(), tx1.nTime, tx1_hash};
     beacon1.m_cpid = TestKey::Cpid();
@@ -1487,7 +1487,7 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test_2)
     beacon_payload1.m_signature = TestKey::Signature(beacon_payload1);
 
     GRC::Contract contract1 = GRC::MakeContract<GRC::BeaconPayload>(3, GRC::ContractAction::ADD, beacon_payload1);
-    GRC::ContractContext ctx1 {contract1, tx1, pindex1};
+    GRC::ContractContext ctx1 {contract1, tx1, &pindex1};
 
     BOOST_CHECK(ctx1.m_contract.CopyPayloadAs<GRC::BeaconPayload>().m_cpid == TestKey::Cpid());
     BOOST_CHECK(ctx1.m_contract.CopyPayloadAs<GRC::BeaconPayload>().m_beacon.m_status == GRC::BeaconStatusForStorage::PENDING);
@@ -1512,18 +1512,18 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test_2)
     }
 
     // Activation
-    CBlockIndex* pindex2 = new CBlockIndex;
-    pindex2->nVersion = 13;
-    pindex2->nHeight = 2;
-    pindex2->nTime = int64_t {2};
-    uint256* block2_phash = new uint256 {rng.rand256()};
-    pindex2->phashBlock = block2_phash;
+    CBlockIndex pindex2 {};
+    pindex2.nVersion = 13;
+    pindex2.nHeight = 2;
+    pindex2.nTime = int64_t {2};
+    uint256 block2_phash = rng.rand256();
+    pindex2.phashBlock = &block2_phash;
 
     std::vector<uint160> beacon_ids {TestKey::Public().GetID()};
 
-    registry.ActivatePending(beacon_ids, pindex2->nTime, *pindex2->phashBlock, pindex2->nHeight);
+    registry.ActivatePending(beacon_ids, pindex2.nTime, *pindex2.phashBlock, pindex2.nHeight);
 
-    uint256 activated_beacon_hash = Hash(*block2_phash, pending_beacons[0]->m_hash);
+    uint256 activated_beacon_hash = Hash(block2_phash, pending_beacons[0]->m_hash);
 
     BOOST_CHECK(registry.GetBeaconDB().size() == 2);
 

src/test/gridcoin/cpid_tests.cpp

@@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(it_is_hashable_to_key_a_lookup_map)
     std::hash<GRC::Cpid> hasher;
 
     // CPID halves, little endian
-    const size_t expected = static_cast<size_t>(0x0706050403020100ull + 0x1514131211100908ull);
+    const size_t expected = static_cast<size_t>(0x0706050403020100ull);
 
     BOOST_CHECK_EQUAL(hasher(cpid), expected);
 }

src/test/gridcoin/mrc_tests.cpp

@@ -103,6 +103,7 @@ struct Setup {
 
         mapBlockIndex.erase(pindexGenesisBlock->GetBlockHash());
         delete pindexGenesisBlock->phashBlock;
+        delete wallet;
     }
 };
 } // Anonymous namespace