diff --git a/contrib/valgrind.supp b/contrib/valgrind.supp index 99e84ab9cb..81b7c25a3d 100644 --- a/contrib/valgrind.supp +++ b/contrib/valgrind.supp @@ -146,14 +146,6 @@ fun:_Znwm fun:_Z11LogInstancev } -{ - Suppress secp256k1_context_create still reachable memory warning - Memcheck:Leak - match-leak-kinds: reachable - fun:malloc - ... - fun:secp256k1_context_create -} { Suppress BCLog::Logger::StartLogging() still reachable memory warning Memcheck:Leak diff --git a/contrib/verifybinaries/README.md b/contrib/verifybinaries/README.md index c50d4bef71..ab831eea28 100644 --- a/contrib/verifybinaries/README.md +++ b/contrib/verifybinaries/README.md @@ -1,16 +1,5 @@ ### Verify Binaries -#### Preparation: - -Make sure you obtain the proper release signing key and verify the fingerprint with several independent sources. - -```sh -$ gpg --fingerprint "Bitcoin Core binary release signing key" -pub 4096R/36C2E964 2015-06-24 [expires: YYYY-MM-DD] - Key fingerprint = 01EA 5486 DE18 A882 D4C2 6845 90C8 019E 36C2 E964 -uid Wladimir J. van der Laan (Bitcoin Core binary release signing key) -``` - #### Usage: This script attempts to download the signature file `SHA256SUMS.asc` from https://bitcoin.org. diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 36c0112534..5ab092de05 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -188,11 +188,14 @@ apt install clang-tidy bear clang Then, pass clang as compiler to configure, and use bear to produce the `compile_commands.json`: ```sh -./autogen.sh && ./configure CC=clang CXX=clang++ -make clean && bear make -j $(nproc) # For bear 2.x -make clean && bear -- make -j $(nproc) # For bear 3.x +./autogen.sh && ./configure CC=clang CXX=clang++ --enable-suppress-external-warnings +make clean && bear --config src/.bear-tidy-config -- make -j $(nproc) ``` +The output is denoised of errors from external dependencies and includes with +`--enable-suppress-external-warnings` and `--config src/.bear-tidy-config`. Both +options may be omitted to view the full list of errors. + To run clang-tidy on all source files: ```sh @@ -513,8 +516,19 @@ address sanitizer, libtsan for the thread sanitizer, and libubsan for the undefined sanitizer. If you are missing required libraries, the configure script will fail with a linker error when testing the sanitizer flags. -The test suite should pass cleanly with the `thread` and `undefined` sanitizers, -but there are a number of known problems when using the `address` sanitizer. The +The test suite should pass cleanly with the `thread` and `undefined` sanitizers. You +may need to use a suppressions file, see `test/sanitizer_suppressions`. They may be +used as follows: +```bash +export LSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/lsan" +export TSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/tsan:halt_on_error=1:second_deadlock_stack=1" +export UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" +``` + +See the CI config for more examples, and upstream documentation for more information +about any additional options. + +There are a number of known problems when using the `address` sanitizer. The address sanitizer is known to fail in [sha256_sse4::Transform](/src/crypto/sha256_sse4.cpp) which makes it unusable unless you also use `--disable-asm` when running configure. We would like to fix diff --git a/src/flatfile.h b/src/flatfile.h index 009984f6b8..9c0a1541ea 100644 --- a/src/flatfile.h +++ b/src/flatfile.h @@ -13,15 +13,15 @@ struct FlatFilePos { - int nFile; - unsigned int nPos; + int nFile{-1}; + unsigned int nPos{0}; SERIALIZE_METHODS(FlatFilePos, obj) { READWRITE(VARINT_MODE(obj.nFile, VarIntMode::NONNEGATIVE_SIGNED), VARINT(obj.nPos)); } - FlatFilePos() : nFile(-1), nPos(0) {} + FlatFilePos() {} FlatFilePos(int nFileIn, unsigned int nPosIn) : nFile(nFileIn), @@ -36,7 +36,6 @@ struct FlatFilePos return !(a == b); } - void SetNull() { nFile = -1; nPos = 0; } bool IsNull() const { return (nFile == -1); } std::string ToString() const; diff --git a/src/index/disktxpos.h b/src/index/disktxpos.h index 3166053226..7718755b78 100644 --- a/src/index/disktxpos.h +++ b/src/index/disktxpos.h @@ -10,7 +10,7 @@ struct CDiskTxPos : public FlatFilePos { - unsigned int nTxOffset; // after header + unsigned int nTxOffset{0}; // after header SERIALIZE_METHODS(CDiskTxPos, obj) { @@ -21,15 +21,7 @@ struct CDiskTxPos : public FlatFilePos CDiskTxPos(const FlatFilePos &blockIn, unsigned int nTxOffsetIn) : FlatFilePos(blockIn.nFile, blockIn.nPos), nTxOffset(nTxOffsetIn) { } - CDiskTxPos() { - SetNull(); - } - - void SetNull() { - FlatFilePos::SetNull(); - nTxOffset = 0; - } + CDiskTxPos() {} }; - #endif // BITCOIN_INDEX_DISKTXPOS_H diff --git a/src/init.cpp b/src/init.cpp index b391f1d29c..1723a2c72e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -564,7 +564,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-externalip=", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-fixedseeds", strprintf("Allow fixed seeds if DNS seeds don't provide peers (default: %u)", DEFAULT_FIXEDSEEDS), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-listen", strprintf("Accept connections from outside (default: %u if no -proxy or -connect)", DEFAULT_LISTEN), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxconnections=", strprintf("Maintain at most connections to peers (temporary service connections excluded) (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxreceivebuffer=", strprintf("Maximum per-connection receive buffer, *1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/psbt.h b/src/psbt.h index a75c958340..1b67a55241 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -442,7 +442,7 @@ struct PartiallySignedTransaction // Make sure that we got an unsigned tx if (!tx) { - throw std::ios_base::failure("No unsigned transcation was provided"); + throw std::ios_base::failure("No unsigned transaction was provided"); } // Read input data diff --git a/src/script/descriptor.h b/src/script/descriptor.h index c10329c484..25d678a527 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -35,7 +35,7 @@ public: /** Retrieve a cached parent xpub * * @param[in] key_exp_pos Position of the key expression within the descriptor - * @param[in] xpub The CExtPubKey to get from cache + * @param[out] xpub The CExtPubKey to get from cache */ bool GetCachedParentExtPubKey(uint32_t key_exp_pos, CExtPubKey& xpub) const; /** Cache an xpub derived at an index @@ -49,7 +49,7 @@ public: * * @param[in] key_exp_pos Position of the key expression within the descriptor * @param[in] der_index Derivation index of the xpub - * @param[in] xpub The CExtPubKey to get from cache + * @param[out] xpub The CExtPubKey to get from cache */ bool GetCachedDerivedExtPubKey(uint32_t key_exp_pos, uint32_t der_index, CExtPubKey& xpub) const; /** Cache a last hardened xpub @@ -61,7 +61,7 @@ public: /** Retrieve a cached last hardened xpub * * @param[in] key_exp_pos Position of the key expression within the descriptor - * @param[in] xpub The CExtPubKey to get from cache + * @param[out] xpub The CExtPubKey to get from cache */ bool GetCachedLastHardenedExtPubKey(uint32_t key_exp_pos, CExtPubKey& xpub) const; diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index edb381042b..14ad4782d7 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -27,6 +27,7 @@ #include #include #endif +#include LockedPoolManager* LockedPoolManager::_instance = nullptr; @@ -43,12 +44,12 @@ static inline size_t align_up(size_t x, size_t align) // Implementation: Arena Arena::Arena(void *base_in, size_t size_in, size_t alignment_in): - base(static_cast(base_in)), end(static_cast(base_in) + size_in), alignment(alignment_in) + base(base_in), end(static_cast(base_in) + size_in), alignment(alignment_in) { // Start with one free chunk that covers the entire arena auto it = size_to_free_chunk.emplace(size_in, base); chunks_free.emplace(base, it); - chunks_free_end.emplace(base + size_in, it); + chunks_free_end.emplace(static_cast(base) + size_in, it); } Arena::~Arena() @@ -74,8 +75,9 @@ void* Arena::alloc(size_t size) // Create the used-chunk, taking its space from the end of the free-chunk const size_t size_remaining = size_ptr_it->first - size; - auto allocated = chunks_used.emplace(size_ptr_it->second + size_remaining, size).first; - chunks_free_end.erase(size_ptr_it->second + size_ptr_it->first); + char* const free_chunk = static_cast(size_ptr_it->second); + auto allocated = chunks_used.emplace(free_chunk + size_remaining, size).first; + chunks_free_end.erase(free_chunk + size_ptr_it->first); if (size_ptr_it->first == size) { // whole chunk is used up chunks_free.erase(size_ptr_it->second); @@ -83,11 +85,11 @@ void* Arena::alloc(size_t size) // still some memory left in the chunk auto it_remaining = size_to_free_chunk.emplace(size_remaining, size_ptr_it->second); chunks_free[size_ptr_it->second] = it_remaining; - chunks_free_end.emplace(size_ptr_it->second + size_remaining, it_remaining); + chunks_free_end.emplace(free_chunk + size_remaining, it_remaining); } size_to_free_chunk.erase(size_ptr_it); - return reinterpret_cast(allocated->first); + return allocated->first; } void Arena::free(void *ptr) @@ -98,11 +100,11 @@ void Arena::free(void *ptr) } // Remove chunk from used map - auto i = chunks_used.find(static_cast(ptr)); + auto i = chunks_used.find(ptr); if (i == chunks_used.end()) { throw std::runtime_error("Arena: invalid or double free"); } - std::pair freed = *i; + auto freed = std::make_pair(static_cast(i->first), i->second); chunks_used.erase(i); // coalesce freed with previous chunk diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h index 03e4e371a3..1aaa76009d 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -89,23 +89,23 @@ public: */ bool addressInArena(void *ptr) const { return ptr >= base && ptr < end; } private: - typedef std::multimap SizeToChunkSortedMap; + typedef std::multimap SizeToChunkSortedMap; /** Map to enable O(log(n)) best-fit allocation, as it's sorted by size */ SizeToChunkSortedMap size_to_free_chunk; - typedef std::unordered_map ChunkToSizeMap; + typedef std::unordered_map ChunkToSizeMap; /** Map from begin of free chunk to its node in size_to_free_chunk */ ChunkToSizeMap chunks_free; /** Map from end of free chunk to its node in size_to_free_chunk */ ChunkToSizeMap chunks_free_end; /** Map from begin of used chunk to its size */ - std::unordered_map chunks_used; + std::unordered_map chunks_used; /** Base address of arena */ - char* base; + void* base; /** End address of arena */ - char* end; + void* end; /** Minimum chunk alignment */ size_t alignment; }; diff --git a/src/test/flatfile_tests.cpp b/src/test/flatfile_tests.cpp index f4bc320f3c..09b94e78fb 100644 --- a/src/test/flatfile_tests.cpp +++ b/src/test/flatfile_tests.cpp @@ -23,6 +23,9 @@ BOOST_AUTO_TEST_CASE(flatfile_filename) FlatFileSeq seq2(data_dir / "a", "b", 16 * 1024); BOOST_CHECK_EQUAL(seq2.FileName(pos), data_dir / "a" / "b00456.dat"); + + // Check default constructor IsNull + assert(FlatFilePos{}.IsNull()); } BOOST_AUTO_TEST_CASE(flatfile_open) diff --git a/src/test/fuzz/flatfile.cpp b/src/test/fuzz/flatfile.cpp index d142e374b1..b5c82c14b4 100644 --- a/src/test/fuzz/flatfile.cpp +++ b/src/test/fuzz/flatfile.cpp @@ -25,6 +25,4 @@ FUZZ_TARGET(flatfile) assert((*flat_file_pos == *another_flat_file_pos) != (*flat_file_pos != *another_flat_file_pos)); } (void)flat_file_pos->ToString(); - flat_file_pos->SetNull(); - assert(flat_file_pos->IsNull()); } diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index 3d7ecd770d..cdf512da64 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(ReadWrite) BOOST_CHECK(values.empty()); BOOST_CHECK(errors.empty()); - // Check duplicate keys not allowed + // Check duplicate keys not allowed and that values returns empty if a duplicate is found. WriteText(path, R"({ "dupe": "string", "dupe": "dupe" @@ -88,6 +88,7 @@ BOOST_AUTO_TEST_CASE(ReadWrite) BOOST_CHECK(!util::ReadSettings(path, values, errors)); std::vector dup_keys = {strprintf("Found duplicate key dupe in settings file %s", fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), dup_keys.begin(), dup_keys.end()); + BOOST_CHECK(values.empty()); // Check non-kv json files not allowed WriteText(path, R"("non-kv")"); diff --git a/src/util/readwritefile.cpp b/src/util/readwritefile.cpp index a45c41d367..2493028f95 100644 --- a/src/util/readwritefile.cpp +++ b/src/util/readwritefile.cpp @@ -3,6 +3,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include + #include #include @@ -10,7 +12,7 @@ #include #include -std::pair ReadBinaryFile(const fs::path &filename, size_t maxsize=std::numeric_limits::max()) +std::pair ReadBinaryFile(const fs::path &filename, size_t maxsize) { FILE *f = fsbridge::fopen(filename, "rb"); if (f == nullptr) diff --git a/src/util/settings.cpp b/src/util/settings.cpp index 64317fd2a0..a2c7f8598b 100644 --- a/src/util/settings.cpp +++ b/src/util/settings.cpp @@ -111,6 +111,8 @@ bool ReadSettings(const fs::path& path, std::map& va auto inserted = values.emplace(in_keys[i], in_values[i]); if (!inserted.second) { errors.emplace_back(strprintf("Found duplicate key %s in settings file %s", in_keys[i], fs::PathToString(path))); + values.clear(); + break; } } return errors.empty(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index aa70eef716..4e68ff19b0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -25,6 +25,7 @@ #include