-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: Add CMake-based build system (3 of N) #7
Conversation
aa480d5
to
4f66c81
Compare
ECMULT_WINDOW_SIZE=15 | ||
ENABLE_MODULE_RECOVERY | ||
ENABLE_MODULE_SCHNORRSIG | ||
ENABLE_MODULE_EXTRAKEYS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic for USE_ASM_X86_64
isn't hooked up here. Looks like we'll need a quick port of SECP_64BIT_ASM_CHECK
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic for
USE_ASM_X86_64
isn't hooked up here. Looks like we'll need a quick port ofSECP_64BIT_ASM_CHECK
.
Thanks! Updated.
132f3c8
to
f8e38ad
Compare
Added
CMake's analogue of |
f8e38ad
to
2acea53
Compare
@theuni's #7 (comment) has been addressed. |
0b9e302
to
61aa6e2
Compare
Undrafted. Although, some comments need to be addressed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass review
# GCC 8.x (libstdc++) requires -lstdc++fs | ||
# Clang 8.x (libc++) requires -lc++fs | ||
|
||
function(check_std_filesystem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very nice 👍
) | ||
|
||
if(HAVE_CLMUL) | ||
add_library(minisketch_clmul OBJECT EXCLUDE_FROM_ALL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all the trouble of another library? For others so far, we've been changing source properties to append necessary compiler flags. I think that'd be easier here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both approaches are essentially the same. This one seems more concise and maintainable because source files are mentioned once only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all the trouble of another library?
The point is that an OBJECT
"library" is not a real one :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not convinced that this is cleaner, but I'm also not opposed at all. Point taken that I need to get my head out of autotools mode, heh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm used to seeing these object libraries in cases where the compiled objects are used more than once by some project internal libraries. If these object libraries are to be used instead of defining properties on a GLOB'ed collection of source files I'd like to know when to use either, since this is setting a precedent. Can you comment on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still considering it only as a style difference; therefore, more DRYer and readable option should be chosen.
@@ -38,6 +38,7 @@ list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/module) | |||
# Configurable options. | |||
# When adding a new option, end the <help_text> with a full stop for consistency. | |||
include(CMakeDependentOption) | |||
option(ASM "Use assembly routines." ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to split this part out as a dependency of the secp256k1 asm detection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading this correctly this ASM option has no influence on secp256k1 asm usage and neither does its analog in the current autotools build system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this has always been handled awkwardly, and agreed that behavior doesn't seem to be changing here.
# Distributed under the MIT software license, see the accompanying | ||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
||
if(ASM AND NOT MSVC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why no msvc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It requires bitcoin#24773 or similar.
if(ASM AND NOT MSVC) | ||
# Check for SSE4.1 intrinsics. | ||
set(SSE41_CXXFLAGS -msse4.1) | ||
check_cxx_source_compiles_with_flags("${SSE41_CXXFLAGS}" " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, we need to make a decision about where to add these checks.
It's fine here, and it's fine in introspection, but I don't like the idea of them being arbitrarily in one place or another. Is there some rule you're going by?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
introspection.cmake
contains project-wide checks. This one is libcrypto
-specific only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So why not put the HAVE_POSIX_FALLOCATE
/HAVE_SYS_GETRANDOM
/HAVE_POSIX_FALLOCATE
/etc.
checks in bitcoin_util
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Updated.
CMakeLists.txt
Outdated
$<$<CONFIG:Debug>:BOOST_MULTI_INDEX_ENABLE_SAFE_MODE> | ||
) | ||
|
||
pkg_check_modules(libevent REQUIRED libevent>=2.1.8 IMPORTED_TARGET) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need a way to use pkg-config --static
in order to pick up on dependency dependencies. Our current hack is here: https://github.com/bitcoin/bitcoin/blob/master/depends/config.site.in#L85
Edit: Actually, the next line (almost) wouldn't be needed if we were invoking with pkg-config --static
, as it would pick up ws2_32.
It SHOULD list all necessary libs including iphlpapi, but it seems that's a bug in the upstream .pc file :(
src/CMakeLists.txt
Outdated
common/bloom.cpp | ||
common/interfaces.cpp | ||
common/run_command.cpp | ||
$<$<TARGET_EXISTS:PkgConfig::libevent>:common/url.cpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a sign that something is broken. But sure enough, it exists in autotools.
I really don't like the idea of helper libs with conditional functionality. Obviously this is fine for now because it matches what we currently have, but I'd like to fix this up later.
threadnames.cpp | ||
time.cpp | ||
tokenpipe.cpp | ||
../chainparamsbase.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, awkward. Motivation to move things to their proper homes? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, awkward. Motivation to move things to their proper homes? :)
Exactly :)
I'd really like to encourage building out of depends rather than system/brew/vcpkg. We're going to start running into issues with testing because we're seeing different things. So.. requesting a hookup to depends in the next PR please :) |
I'm asking for a piece of advice from you. At the moment, this PR branch and the staging branch itself are conflicting with https://github.com/bitcoin/bitcoin/tree/master/ due to a few merges which touched the current Autotools-based build system. For example, bitcoin#25862. How am I supposed to resolve such conflicts? |
@hebasto Hmm, again I'd point to the Dictator and Lieutenants Workflow. Ideally you'd merge from bitcoin/master now and then (as a PR), and deal with whatever fix-ups at that time. The downside is that you then have a non-linear history, which isn't going to go over well when it comes time to PR it into core. I suppose at that point you'd have to manually linearize before PR'ing. An alternative would be to just to rebase now and then and show us what changed. Pinging @sipa who has had some experience with this flow. Any recommendations? Edit: I guess one interesting thing about this work in particular is that the CMake files are mostly all new, so there will be very few actual git conflicts. I suppose it would be possible to do something like a round of weekly "fixup" commits which (for ex) move some files around between libraries. You could then (cleanly) rebase on master. Ofc that all falls apart if we start getting actual merge conflicts, which I'm not sure about. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested building on OpenBSD 7.2 (amd64) via
$ cmake -S . -B build
$ cmake --build build --config Release -j 8
which worked flawlessly, spitting out a ~30MB bitcoind binary.
Is there any plan to suppport a debug config? Building with --config Debug
(rather than Release
) seems to use the exact same compile parameters. Or would this be needed to specify at the first stage already?
Note that I don't know much about build systems and can't comment much on the commits, but very happy to receive further testing ideas if you have any :)
Thanks for testing!
Just to clarify, the
Sure. Going to consider to pull the relevant commits from bitcoin#25797 into this PR. |
# Check for SSE4.1 intrinsics. | ||
set(SSE41_CXXFLAGS -msse4.1) | ||
check_cxx_source_compiles_with_flags("${SSE41_CXXFLAGS}" " | ||
#include <immintrin.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For anybody else stumbling over this, removing stdint.h
here in comparison to what's in the configure.ac
is the right thing to do.
src/CMakeLists.txt
Outdated
univalue | ||
secp256k1 | ||
Boost::boost | ||
Threads::Threads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the threads flags / libs are included when linking the final binary. I'm guessing it is out of necessity that they are linked into the libraries now. Is there a performance/size downside to statically linking them to both the common and util library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh? Catching up, but why is Boost even in here. We don't link any Boost libs, and shouldn't be linking any ever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh? Catching up, but why is Boost even in here. We don't link any Boost libs, and shouldn't be linking any ever
Tee target_link_libraries()
command also propagates usage requirements from the listed targets.
For the Boost::boost
target, which is a headers only dependency, only an include directory is a such usage requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an alternative here would be maintaining boost header propagation ourselves, by requiring that clients of libs using boost headers also opt-in to boost headers via target_include_directories
.
I don't have a problem with keeping this as-is though, as well as it's well-behaved and thoroughly commented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the threads flags / libs are included when linking the final binary. I'm guessing it is out of necessity that they are linked into the libraries now.
The idea is to provide direct dependencies for each build target/artifact. Such an approach makes the usage of those artifacts much cleaner. Consider target_link_libraries(bitcoind bitcoin_node)
.
Is there a performance/size downside to statically linking them to both the common and util library?
Tbh, I didn't made such a comparison.
61aa6e2
to
a2b5411
Compare
a2b5411
to
5931a38
Compare
9c83f07
to
16b6c2f
Compare
Addressed @theuni's #7 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a few last comments/questions.
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_7bytes.cpp | ||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_8bytes.cpp | ||
) | ||
target_compile_definitions(minisketch_clmul PUBLIC HAVE_CLMUL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why PUBLIC
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To propagate this usage requirement to the minisketch
target.
Comes from
bitcoin/src/Makefile.minisketch.include
Line 27 in 74981aa
minisketch_libminisketch_a_CPPFLAGS = $(AM_CPPFLAGS) $(LIBMINISKETCH_CPPFLAGS) |
Fixed s/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK a80fb45
Want to take a look here? |
ACK a80fb45 I just left one comment for clarification of the ASM option. Neat:
|
b0351e3 [FIXUP] cmake: Rework PIC/PIE logic (Hennadii Stepanov) a8917ab cmake: Add cross-compiling support (Hennadii Stepanov) e5e6bd3 build: Generate `share/toolchain.cmake` in depends (Hennadii Stepanov) d37c9a2 [FIXUP] cmake: Ensure C language is disabled for FindThreads (Hennadii Stepanov) 3bb3448 [FIXUP] cmake: Enable C language for libsecp256k1 (Hennadii Stepanov) 75b9b6c [FIXUP] cmake: Drop C language from the global project (Hennadii Stepanov) Pull request description: #7 (comment): > So.. requesting a hookup to depends in the next PR please :) Delivered :) --- The parent PR: bitcoin#25797. The previous PRs in the staging branch: #5, #6, #7. --- EXAMPLES: Cross-compiling for Windows: ``` make -C depends HOST=x86_64-w64-mingw32 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1 cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=depends/x86_64-w64-mingw32/share/toolchain.cmake cmake --build build ``` Cross-compiling for macOS, arm64: ``` make -C depends HOST=arm64-apple-darwin NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1 cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=depends/arm64-apple-darwin/share/toolchain.cmake cmake --build build ``` ACKs for top commit: theuni: ACK b0351e3, with the caveat that I hope we can make _significant_ changes to the `toolchain.cmake.in`. But I'm ok calling that a work-in-progress while the rest of the work is introduced to keep it from holding up review further. Tree-SHA512: 41e9925fb30766c61eca3506eed4b8a75701d0ba84c3077317266f73771eca50b14b40102e473b15d5f3f8eac7d87a19c5b27889cb6f350d089b3231b57c0991
8ee7537 cmake: Add `systemtap-sdt` optional package support (Hennadii Stepanov) 697c397 cmake: Add `libzmq` optional package support (Hennadii Stepanov) b195c9c cmake: Add `libminiupnpc` optional package support (Hennadii Stepanov) 9587ed7 cmake: Add `libnatpmp` optional package support (Hennadii Stepanov) ba50cad [FIXUP] cmake: Fix `check_evhttp_connection_get_peer` macro (Hennadii Stepanov) 9412a2e [FIXUP] cmake: Cleanup `AddBoostIfNeeded` module (Hennadii Stepanov) 15bcb8e cmake: Add `ccache` support (Hennadii Stepanov) 480c8cb cmake: Add `TristateOption` module (Hennadii Stepanov) Pull request description: It was [suggested](#10 (comment)): > It might be helpful to get some more of the depends interactions hooked up as a next step Done in this PR. --- The parent PR: bitcoin#25797. The previous PRs in the staging branch: #5, #6, #7, #10. --- EXAMPLES: 1. Cross-compiling for Windows: ``` make -C depends HOST=x86_64-w64-mingw32 NO_QT=1 NO_WALLET=1 cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=depends/x86_64-w64-mingw32/share/toolchain.cmake cmake --build build ``` 2. Cross-compiling for macOS (`arm64`, `x86_64`): ``` make -C depends HOST=arm64-apple-darwin NO_QT=1 NO_WALLET=1 cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=depends/arm64-apple-darwin/share/toolchain.cmake cmake --build build ``` 3. Building on macOS, arm64: - essential build tools: ``` brew install cmake pkg-config ``` - optional build tools: ``` brew install ccache ``` - essential dependencies: ``` brew install boost libevent ``` - optional dependencies: ``` brew install libnatpmp miniupnpc zeromq ``` - configure and build: ``` cmake -S . -B build cmake --build build ``` 4. Building on Windows: - `ccache` is available [here](https://community.chocolatey.org/packages/ccache) or [here](https://ccache.dev/download.html) (also see a [note](#13 (comment))): ``` choco install ccache --version=4.7.4 ``` - the [`vcpkg`](https://vcpkg.io) package manager is used to provide other dependencies: ``` vcpkg --triplet=x64-windows-static install boost-multi-index boost-signals2 libevent miniupnpc zeromq ``` - configure and build: ``` cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake -DVCPKG_TARGET_TRIPLET=x64-windows-static cmake --build build -- -property:UseMultiToolTask=true -maxCpuCount ``` --- **NOTE** As always, make sure your build tree is clean :) ACKs for top commit: vasild: ACK 8ee7537 theuni: ACK 8ee7537 Tree-SHA512: 36ebc63e73d507d86b154121b9bc5b8216554fd63f31808f257cefff7a1a104ce066f2b2cf9fdfc1828066d33cdc0b9ede4a81d8e6a7932de3adcb1f14674796
f952e67 ci: remove usage of untrusted bpfcc-tools (fanquake) 1232c2f ci: use LLVM/clang-16 in native_asan job (fanquake) Pull request description: Similar to bitcoin#27298. Working for me on `x86_64` and solves the issue I currently see with TSAN on `aarch64` with master (6882828): ```bash crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0xffff84400406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment 0xffff84400406: note: pointer points here b9 c5 22 00 01 01 1a 6c 65 76 65 6c 64 62 2e 42 79 74 65 77 69 73 65 43 6f 6d 70 61 72 61 74 6f ^ #0 0xaaaaaddaf0b4 in crc32c::ExtendArm64(unsigned int, unsigned char const*, unsigned long) src/./src/crc32c/src/crc32c_arm64.cc:101:26 #1 0xaaaaadd2c838 in leveldb::crc32c::Value(char const*, unsigned long) src/./leveldb/util/crc32c.h:20:60 #2 0xaaaaadd2c838 in leveldb::log::Reader::ReadPhysicalRecord(leveldb::Slice*) src/./src/leveldb/db/log_reader.cc:246:29 #3 0xaaaaadd2ba9c in leveldb::log::Reader::ReadRecord(leveldb::Slice*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) src/./src/leveldb/db/log_reader.cc:72:38 #4 0xaaaaadd41710 in leveldb::VersionSet::Recover(bool*) src/./src/leveldb/db/version_set.cc:910:19 #5 0xaaaaadcf9fec in leveldb::DBImpl::Recover(leveldb::VersionEdit*, bool*) src/./src/leveldb/db/db_impl.cc:320:18 #6 0xaaaaadd12068 in leveldb::DB::Open(leveldb::Options const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, leveldb::DB**) src/./src/leveldb/db/db_impl.cc:1487:20 #7 0xaaaaad314e80 in CDBWrapper::CDBWrapper(DBParams const&) src/./src/dbwrapper.cpp:156:30 #8 0xaaaaace94880 in CBlockTreeDB::CBlockTreeDB(DBParams const&) src/./txdb.h:89:23 #9 0xaaaaace94880 in std::_MakeUniq<CBlockTreeDB>::__single_object std::make_unique<CBlockTreeDB, DBParams>(DBParams&&) /usr/bin/../lib/gcc/aarch64-linux-gnu/11/../../../../include/c++/11/bits/unique_ptr.h:962:34 #10 0xaaaaace94880 in ChainTestingSetup::ChainTestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&) src/./src/test/util/setup_common.cpp:188:51 #11 0xaaaaace95da0 in TestingSetup::TestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&, bool, bool) src/./src/test/util/setup_common.cpp:243:7 #12 0xaaaaace96730 in TestChain100Setup::TestChain100Setup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&, bool, bool) src/./src/test/util/setup_common.cpp:274:7 #13 0xaaaaac1ddbc8 in blockfilter_index_tests::BuildChainTestingSetup::BuildChainTestingSetup() src/./src/test/blockfilter_index_tests.cpp:26:8 #14 0xaaaaac1ddbc8 in blockfilter_index_tests::blockfilter_index_initial_sync::blockfilter_index_initial_sync() src/./src/test/blockfilter_index_tests.cpp:112:1 #15 0xaaaaac1ddbc8 in blockfilter_index_tests::blockfilter_index_initial_sync_invoker() src/./src/test/blockfilter_index_tests.cpp:112:1 #16 0xaaaaabf08f7c in boost::function0<void>::operator()() const /usr/include/boost/function/function_template.hpp:763:14 #17 0xaaaaabf95468 in boost::detail::forward::operator()() /usr/include/boost/test/impl/execution_monitor.ipp:1388:32 #18 0xaaaaabf95468 in boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:137:18 #19 0xaaaaabf8e12c in boost::function0<int>::operator()() const /usr/include/boost/function/function_template.hpp:763:14 #20 0xaaaaabe7be14 in boost::execution_monitor::catch_signals(boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:903:16 #21 0xaaaaabe7c1c0 in boost::execution_monitor::execute(boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:1301:16 #22 0xaaaaabe6f47c in boost::execution_monitor::vexecute(boost::function<void ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:1397:5 #23 0xaaaaabe75124 in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /usr/include/boost/test/impl/unit_test_monitor.ipp:49:9 #24 0xaaaaabed19fc in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:815:44 #25 0xaaaaabed0f6c in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:784:58 #26 0xaaaaabed0f6c in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:784:58 #27 0xaaaaabe73878 in boost::unit_test::framework::run(unsigned long, bool) /usr/include/boost/test/impl/framework.ipp:1721:29 #28 0xaaaaabe9d244 in boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /usr/include/boost/test/impl/unit_test_main.ipp:250:9 #29 0xffff8f0773f8 (/lib/aarch64-linux-gnu/libc.so.6+0x273f8) (BuildId: f37f3aa07c797e333fd106472898d361f71798f5) #30 0xffff8f0774c8 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x274c8) (BuildId: f37f3aa07c797e333fd106472898d361f71798f5) #31 0xaaaaabda55ac in _start (/home/fedora/ci_scratch/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/test_bitcoin+0x10e55ac) (BuildId: b7909adaefd9db6cd6a7c4d3d40207cf6bdaf4b3) SUMMARY: UndefinedBehaviorSanitizer: misaligned-pointer-use crc32c/src/crc32c_arm64.cc:101:26 in ``` ACKs for top commit: dergoegge: utACK f952e67 MarcoFalke: lgtm ACK f952e67 Tree-SHA512: 9dee2abf73d3f23bb9979bfb453b48e39f0b7a5f58d43824ecf053a53e9800ed413b915382b274d1a84baf2999683e3b485463e377e0455b3f0ead65ed1d1916
682274a ci: install llvm-symbolizer in MSAN jobs (fanquake) 96527cd ci: use LLVM 16.0.6 in MSAN jobs (fanquake) Pull request description: Fixes: bitcoin#27737 (comment). Tested (locally) with bitcoin#27495 that it produces a symbolized backtrace: ```bash 2023-06-20T17:5Uninitialized bytes in __interceptor_strlen at offset 113 inside [0x719000006908, 114) ==35429==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x56060fae8c4b in sqlite3Strlen30 /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:32670:28 #1 0x56060fb0fcf4 in sqlite3PagerOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:57953:17 #2 0x56060fb0f48b in sqlite3BtreeOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:68679:10 #3 0x56060fb01384 in openDatabase /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:171911:8 #4 0x56060fb016ca in sqlite3_open_v2 /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:172034:10 #5 0x56060e8a94db in wallet::SQLiteDatabase::Open() src/wallet/sqlite.cpp:250:19 #6 0x56060e8a30fd in wallet::SQLiteDatabase::SQLiteDatabase(fs::path const&, fs::path const&, wallet::DatabaseOptions const&, bool) src/wallet/sqlite.cpp:133:9 #7 0x56060e8b78f5 in std::__1::__unique_if<wallet::SQLiteDatabase>::__unique_single std::__1::make_unique[abi:v160006]<wallet::SQLiteDatabase, std::__1::__fs::filesystem::path, fs::path&, wallet::DatabaseOptions const&>(std::__1::__fs::filesystem::path&&, fs::path&, wallet::DatabaseOptions const&) /home/ubuntu/ci_scratch/ci/scratch/msan/cxx_build/include/c++/v1/__memory/unique_ptr.h:686:30 #8 0x56060e8b5240 in wallet::MakeSQLiteDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/sqlite.cpp:641:19 #9 0x56060e83560b in wallet::MakeDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/walletdb.cpp:1261:16 #10 0x56060e7546e9 in wallet::MakeWalletDatabase(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/wallet.cpp:2905:12 #11 0x56060e4bc03f in wallet::TestLoadWallet(wallet::WalletContext&) src/wallet/test/util.cpp:68:21 #12 0x56060e349ad4 in wallet::wallet_tests::ZapSelectTx::test_method() src/wallet/test/wallet_tests.cpp:897:19 #13 0x56060e348598 in wallet::wallet_tests::ZapSelectTx_invoker() src/wallet/test/wallet_tests.cpp:891:1 #14 0x56060cfec325 in boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 #15 0x56060ced3a7e in boost::function0<void>::operator()() const /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 #16 0x56060ced3a7e in boost::detail::forward::operator()() /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 #17 0x56060ced3a7e in boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 #18 0x56060cda71c2 in boost::function0<int>::operator()() const /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 #19 0x56060cda71c2 in int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()>>(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:301:30 #20 0x56060cda71c2 in boost::execution_monitor::catch_signals(boost::function<int ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:903:16 #21 0x56060cda784a in boost::execution_monitor::execute(boost::function<int ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1301:16 #22 0x56060cd9ec3a in boost::execution_monitor::vexecute(boost::function<void ()> const&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1397:5 #23 0x56060cd9ec3a in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 #24 0x56060ce1a07b in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:815:44 #25 0x56060ce1ad8b in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 #26 0x56060ce1ad8b in boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 #27 0x56060cd9b8de in boost::unit_test::framework::run(unsigned long, bool) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1722:29 #28 0x56060cdd4fac in boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:250:9 #29 0x56060cdd6094 in main /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:306:12 #30 0x7f7379691d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d) #31 0x7f7379691e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d) #32 0x56060cce2e24 in _start (/home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x188e24) Uninitialized value was created by a heap allocation #0 0x56060cd163f2 in malloc /ci_base_install/ci/scratch/msan/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:934:3 #1 0x56060fc10069 in sqlite3MemMalloc /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:25163:7 #2 0x56060fb063bc in mallocWithAlarm /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:28846:7 #3 0x56060fae4eb9 in sqlite3Malloc /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:28876:5 #4 0x56060faf9e19 in sqlite3DbMallocRaw /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:29176:7 #5 0x56060fb0fc67 in sqlite3PagerOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:57938:17 #6 0x56060fb0f48b in sqlite3BtreeOpen /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:68679:10 #7 0x56060fb01384 in openDatabase /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:171911:8 #8 0x56060fb016ca in sqlite3_open_v2 /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:172034:10 #9 0x56060e8a94db in wallet::SQLiteDatabase::Open() src/wallet/sqlite.cpp:250:19 #10 0x56060e8a30fd in wallet::SQLiteDatabase::SQLiteDatabase(fs::path const&, fs::path const&, wallet::DatabaseOptions const&, bool) src/wallet/sqlite.cpp:133:9 #11 0x56060e8b78f5 in std::__1::__unique_if<wallet::SQLiteDatabase>::__unique_single std::__1::make_unique[abi:v160006]<wallet::SQLiteDatabase, std::__1::__fs::filesystem::path, fs::path&, wallet::DatabaseOptions const&>(std::__1::__fs::filesystem::path&&, fs::path&, wallet::DatabaseOptions const&) /home/ubuntu/ci_scratch/ci/scratch/msan/cxx_build/include/c++/v1/__memory/unique_ptr.h:686:30 #12 0x56060e8b5240 in wallet::MakeSQLiteDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/sqlite.cpp:641:19 #13 0x56060e83560b in wallet::MakeDatabase(fs::path const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/walletdb.cpp:1261:16 #14 0x56060e7546e9 in wallet::MakeWalletDatabase(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, wallet::DatabaseOptions const&, wallet::DatabaseStatus&, bilingual_str&) src/wallet/wallet.cpp:2905:12 #15 0x56060e4bc03f in wallet::TestLoadWallet(wallet::WalletContext&) src/wallet/test/util.cpp:68:21 #16 0x56060e349ad4 in wallet::wallet_tests::ZapSelectTx::test_method() src/wallet/test/wallet_tests.cpp:897:19 #17 0x56060e348598 in wallet::wallet_tests::ZapSelectTx_invoker() src/wallet/test/wallet_tests.cpp:891:1 #18 0x56060cfec325 in boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 #19 0x56060ced3a7e in boost::function0<void>::operator()() const /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 #20 0x56060ced3a7e in boost::detail::forward::operator()() /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 #21 0x56060ced3a7e in boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /home/ubuntu/ci_scratch/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ubuntu/ci_scratch/depends/work/build/x86_64-pc-linux-gnu/sqlite/3380500-f816a3e2d52/sqlite3.c:32670:28 in sqlite3Strlen30 ``` as opposed to unsymbolized: https://cirrus-ci.com/task/6005512018329600?logs=ci#L3245. ACKs for top commit: MarcoFalke: lgtm ACK 682274a Tree-SHA512: 8f3e7636761c956537a472989bf07529f5afbd988c5e7e1f07ece8b2599608fa4fe9e1efdc6e302cf0f7f44dec3cf9a3c1e68b758af81a8a8b476a43d3220807
43123cf FIXUP: Same as PR27458 (Hennadii Stepanov) 2d8930e FIXUP: Same as PR27656 (Hennadii Stepanov) a112470 cmake: Include CTest (Hennadii Stepanov) cb19814 cmake: Build `test_bitcoin` executable (Hennadii Stepanov) 751453f cmake: Build `bench_bitcoin` executable (Hennadii Stepanov) a2c3493 cmake: Add test config and runners (Hennadii Stepanov) cb7dc94 test: Explicitly specify directory where to search tests for (Hennadii Stepanov) 2fd303f test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX` (Hennadii Stepanov) 2e3721e cmake: Add wallet functionality (Hennadii Stepanov) f944ccd cmake: Build `bitcoin-util` executable (Hennadii Stepanov) d1c319d cmake: Build `bitcoin-tx` executable (Hennadii Stepanov) 1934755 cmake: Build `bitcoin-cli` executable (Hennadii Stepanov) 5fc2cee [FIXUP] cmake: Add workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/20652 (Hennadii Stepanov) b2bea9f [FIXUP] cmake: Consider `ASM` option when checking for `HAVE_64BIT_ASM` (Hennadii Stepanov) Pull request description: The parent PR: bitcoin#25797. The previous PRs in the staging branch: #5, #6, #7, #10, #13. --- What is NEW: - `bitcoin-cli` - `bitcoin-tx` - `bitcoin-util` - wallet functionality and `bitcoin-wallet` - `bench_bitcoin` - `test_bitcoin` EXAMPLES: ``` cmake -S . -B build cd build cmake --build . -j $(nproc) ctest -j $(nproc) ./test/functional/test_runner.py -j $(nproc) ``` Using a multi-configuration generator (CMake 3.17+ is required): ``` cmake -S . -B build -G "Ninja Multi-Config" cd build cmake --build . -j $(nproc) ctest -j $(nproc) -C Debug ``` --- What to test: - old CMake versions, for example v3.13 - multi-config generators, for example `-G "Ninja Multi-Config"` --- What to consider additionally: - is it worth to pull the "[test: Make util/test_runner.py honor BITCOINUTIL and BITCOINTX](268aca3)" commit to the main repo now? FWIW, we use the same approach for functional tests providing the `BITCOIND` environment variable when needed. ACKs for top commit: TheCharlatan: ACK 43123cf Tree-SHA512: a04cfca266bcde780528c915cb01f69189f36a0e1beb521fe75e4816ca4f9ab9440646529bbf2cbdfe76275debc5107ee2433e1027405094d3e8a74ec0d1d077
a933ddf [FIXUP] Better document a workaround (Hennadii Stepanov) 769633e [FIXUP] for "cmake: Add wallet functionality" (Hennadii Stepanov) 31e4e62 [FIXUP] for "cmake: Build `test_bitcoin` executable" (Hennadii Stepanov) f2dbb17 [FIXUP] for "cmake: Build `test_bitcoin` executable" (Hennadii Stepanov) 91d7327 [FIXUP] Boost (Hennadii Stepanov) Pull request description: The parent PR: bitcoin#25797. The previous PRs in the staging branch: #5, #6, #7, #10, #13, #15. This PR consists of fixups only to make reviewing easier :) ACKs for top commit: theuni: ACK a933ddf Tree-SHA512: 270869a3bf9b9f2d56b75d169f25032385fa2d1297951a23c0d1900d9668a40b153c7a5d9b51fa87596eec681107c40da8e55f405d28a7ecd2ec0f72f3550bbf
ac7bc5b [FIXUP] Rename CCACHE_EXECUTABLE --> CCACHE_COMMAND for consistency (Hennadii Stepanov) 0476509 [FIXUP] Learn to work with recent ccache in MSVC builds (Hennadii Stepanov) 2fd67c7 [FIXUP] Do not disable `TrackFileAccess` in MSVC builds (Hennadii Stepanov) a53ae12 [FIXUP] Use Multi-ToolTask in MSVC builds by default (Hennadii Stepanov) Pull request description: The parent PR: bitcoin#25797. The previous PRs in the staging branch: #5, #6, #7, #10, #13, #15, #17, #18. This PR consists of fixups related to using [Ccache](https://ccache.dev/) in MSVC builds. Top commit has no ACKs. Tree-SHA512: dc01202b054aaeb5b46607bc93126bee0df523df3b4f2df54b566b2e2d6306d784a723fd4a999d0d84fdf31e926a72304790f94b9d57034db0c112ee9f52070e
a65da0d cmake: Redefine configuration flags (Hennadii Stepanov) 1a1dda5 [FIXUP] Evaluate flags set in depends _after_ config-specific flags (Hennadii Stepanov) 482e844 cmake: Warn about not encapsulated build properties (Hennadii Stepanov) 3736470 cmake: Add platform-specific flags (Hennadii Stepanov) e0621e9 cmake: Add `TryAppendLinkerFlag` module (Hennadii Stepanov) ae430cf cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov) 7903bd5 [FIXUP] Encapsulate common build flags into `core` interface library (Hennadii Stepanov) Pull request description: The parent PR: bitcoin#25797. The previous PRs in the staging branch: #5, #6, #7, #10, #13, #15, #17, #19. --- What is NEW: - functions for checking compiler and linker flags - managing flags for different build types (configurations) EXAMPLES of configuration output on Ubuntu 22.04: - for a single-config generator: ``` $ cmake .. -G "Unix Makefiles" ... Cross compiling ....................... FALSE Preprocessor defined macros ........... C compiler ............................ /usr/bin/cc CFLAGS ................................ C++ compiler .......................... /usr/bin/c++ CXXFLAGS .............................. Common compile options ................ Common link options ................... Linker flags for executables .......... Linker flags for shared libraries ..... Build type (configuration): - CMAKE_BUILD_TYPE ................... RelWithDebInfo - Preprocessor defined macros ........ - CFLAGS ............................. -O2 -g - CXXFLAGS ........................... -O2 -g - LDFLAGS for executables ............ - LDFLAGS for shared libraries ....... Use assembly routines ................. ON Use ccache for compiling .............. ON ... ``` - for a multi-config generator: ``` $ cmake .. -G "Ninja Multi-Config" ... Cross compiling ....................... FALSE Preprocessor defined macros ........... C compiler ............................ /usr/bin/cc CFLAGS ................................ C++ compiler .......................... /usr/bin/c++ CXXFLAGS .............................. Common compile options ................ Common link options ................... Linker flags for executables .......... Linker flags for shared libraries ..... Available build types (configurations) RelWithDebInfo Debug Release 'RelWithDebInfo' build type (configuration): - Preprocessor defined macros ........ - CFLAGS ............................. -O2 -g - CXXFLAGS ........................... -O2 -g - LDFLAGS for executables ............ - LDFLAGS for shared libraries ....... 'Debug' build type (configuration): - Preprocessor defined macros ........ DEBUG DEBUG_LOCKORDER DEBUG_LOCKCONTENTION RPC_DOC_CHECK ABORT_ON_FAILED_ASSUME - CFLAGS ............................. -O0 -g3 - CXXFLAGS ........................... -O0 -g3 -ftrapv - LDFLAGS for executables ............ - LDFLAGS for shared libraries ....... 'Release' build type (configuration): - Preprocessor defined macros ........ - CFLAGS ............................. -O2 - CXXFLAGS ........................... -O2 - LDFLAGS for executables ............ - LDFLAGS for shared libraries ....... Use assembly routines ................. ON Use ccache for compiling .............. ON ... ``` - cross-compiling for Windows: ``` $ make -C depends HOST=x86_64-w64-mingw32 DEBUG=1 NO_QT=1 $ cmake -B build --toolchain depends/x86_64-w64-mingw32/share/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug ... Cross compiling ....................... TRUE, for Windows, x86_64 Preprocessor defined macros ........... _WIN32_WINNT=0x0601 _WIN32_IE=0x0501 WIN32_LEAN_AND_MEAN NOMINMAX WIN32 _WINDOWS _MT _GLIBCXX_DEBUG _GLIBCXX_DEBUG_PEDANTIC C compiler ............................ /usr/bin/x86_64-w64-mingw32-gcc CFLAGS ................................ -pipe -std=c11 -O1 C++ compiler .......................... /usr/bin/x86_64-w64-mingw32-g++-posix CXXFLAGS .............................. -pipe -std=c++17 -O1 Common compile options ................ Common link options ................... -Wl,--major-subsystem-version,6 -Wl,--minor-subsystem-version,1 Linker flags for executables .......... -static Linker flags for shared libraries ..... Build type (configuration): - CMAKE_BUILD_TYPE ................... Debug - Preprocessor defined macros ........ DEBUG DEBUG_LOCKORDER DEBUG_LOCKCONTENTION RPC_DOC_CHECK ABORT_ON_FAILED_ASSUME - CFLAGS ............................. -O0 -g3 - CXXFLAGS ........................... -O0 -g3 -ftrapv - LDFLAGS for executables ............ - LDFLAGS for shared libraries ....... Use assembly routines ................. ON Use ccache for compiling .............. ON ... ``` **A cross-project note.** The `ProcessConfigurations.cmake` is based on the same module that was suggested in bitcoin-core/secp256k1#1291. So, cross-reviewing is welcome :) ACKs for top commit: theuni: ACK a65da0d to keep this moving. This has been sitting for too long :( Tree-SHA512: 57c5e91ddf9675c6a2b56c0cb70fd3f045af8076bee74c49390de38b8d514e130d2086fde6d83d2d1278b437d0a10cc721f0aa44934698110aeadb3a1aef9e64
…BlockTx suppression fa9dc92 test: Add missing CBlockPolicyEstimator::processBlockTx suppression (MarcoFalke) Pull request description: Fixes bitcoin#28865 (comment) ``` # FUZZ=policy_estimator UBSAN_OPTIONS="suppressions=/root/fuzz_dir/scratch/fuzz_gen/code/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/test/fuzz/fuzz /tmp/crash-154b42214e70781a9c1ad72d3f2693913dcf8c06 ... policy/fees.cpp:632:27: runtime error: implicit conversion from type 'unsigned int' of value 4294574080 (32-bit, unsigned) to type 'int' changed the value to -393216 (32-bit, signed) #0 0x55cbbe10daee in CBlockPolicyEstimator::processBlockTx(unsigned int, CTxMemPoolEntry const*) src/policy/fees.cpp:632:27 #1 0x55cbbe10e361 in CBlockPolicyEstimator::processBlock(unsigned int, std::vector<CTxMemPoolEntry const*, std::allocator<CTxMemPoolEntry const*>>&) src/policy/fees.cpp:680:13 #2 0x55cbbd84af48 in policy_estimator_fuzz_target(Span<unsigned char const>)::$_1::operator()() const src/test/fuzz/policy_estimator.cpp:69:40 #3 0x55cbbd84af48 in unsigned long CallOneOf<policy_estimator_fuzz_target(Span<unsigned char const>)::$_0, policy_estimator_fuzz_target(Span<unsigned char const>)::$_1, policy_estimator_fuzz_target(Span<unsigned char const>)::$_2, policy_estimator_fuzz_target(Span<unsigned char const>)::$_3>(FuzzedDataProvider&, policy_estimator_fuzz_target(Span<unsigned char const>)::$_0, policy_estimator_fuzz_target(Span<unsigned char const>)::$_1, policy_estimator_fuzz_target(Span<unsigned char const>)::$_2, policy_estimator_fuzz_target(Span<unsigned char const>)::$_3) src/./test/fuzz/util.h:43:27 #4 0x55cbbd84af48 in policy_estimator_fuzz_target(Span<unsigned char const>) src/test/fuzz/policy_estimator.cpp:38:9 #5 0x55cbbda1cc18 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9 #6 0x55cbbda1cc18 in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:178:5 #7 0x55cbbd26a944 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x190e944) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d) #8 0x55cbbd253916 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x18f7916) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d) #9 0x55cbbd25945a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x18fd45a) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d) #10 0x55cbbd284026 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x1928026) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d) #11 0x7fe4aa8280cf (/lib/x86_64-linux-gnu/libc.so.6+0x280cf) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89) #12 0x7fe4aa828188 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28188) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89) #13 0x55cbbd24e494 in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x18f2494) (BuildId: ffb89e0b86c093ca3bdeae6f85537737a4e3b42d) SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change policy/fees.cpp:632:27 in ``` ``` # base64 /tmp/crash-154b42214e70781a9c1ad72d3f2693913dcf8c06 AQEAAAAAADkFlVwAAQEAAAAAADkFlZVcACTDSSsP3746IAZrH48khwMAAQEB/QEALQAACwAAAAAA FgAAAAAAAQAABgAAAAAAAAAAAAAAAAAAACcQAAAAAAAAAAAAAAAAAAAAAAD6AAAAOQWVXAABAQAA AAAAOQWVlVwAIMNJKw/fvjogBmsfjySHAwABAQH9AQAtAAALAAAAAAAAAAABAAAGAAAAAAAAAAAA AAAAAAAAJxAAAAAAAAAAAAAAAAAAAAAAAPr/AAAAAAAAAAAAAAQAAAAA/wAAAAAAAAAAAAAEAAAA AAEBAeAIAVwBXAAA/jbSBvwBKABSKBwBYgEB2wAEkvXInHYAAAAAAAAAvgAAAAAA/9//6v8e/xIk MgAlAiUAOw== ACKs for top commit: fanquake: ACK fa9dc92 dergoegge: utACK fa9dc92 Tree-SHA512: 3898c17c928ecc2bcc8c7086359e9ae00da2197b4d8e10c7bf6d12415326c9bca3ef6e1d8d3b83172ccfa604ce7e7371415262ba705225f9ea4da8b1a7eb0306
…tifications fuzz target fab164f fuzz: Avoid signed-integer-overflow in wallet_notifications fuzz target (MarcoFalke) Pull request description: Should avoid ``` policy/feerate.cpp:29:63: runtime error: signed integer overflow: 77600710321911316 * 149 cannot be represented in type 'int64_t' (aka 'long') #0 0x563a1775ed66 in CFeeRate::GetFee(unsigned int) const src/policy/feerate.cpp:29:63 #1 0x563a15913a69 in wallet::COutput::COutput(COutPoint const&, CTxOut const&, int, int, bool, bool, bool, long, bool, std::optional<CFeeRate>) src/./wallet/coinselection.h:91:57 #2 0x563a16fa6a6d in wallet::FetchSelectedInputs(wallet::CWallet const&, wallet::CCoinControl const&, wallet::CoinSelectionParams const&) src/wallet/spend.cpp:297:17 #3 0x563a16fc4512 in wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, int, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1105:33 #4 0x563a16fbec74 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, int, wallet::CCoinControl const&, bool) src/wallet/spend.cpp:1291:16 #5 0x563a16fcf6df in wallet::FundTransaction(wallet::CWallet&, CMutableTransaction&, long&, int&, bilingual_str&, bool, std::set<int, std::less<int>, std::allocator<int>> const&, wallet::CCoinControl) src/wallet/spend.cpp:1361:16 #6 0x563a1597b7b9 in wallet::(anonymous namespace)::FuzzedWallet::FundTx(FuzzedDataProvider&, CMutableTransaction) src/wallet/test/fuzz/notifications.cpp:162:15 #7 0x563a15958240 in wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0::operator()() const src/wallet/test/fuzz/notifications.cpp:228:23 #8 0x563a15958240 in unsigned long CallOneOf<wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_1>(FuzzedDataProvider&, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_0, wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>)::$_1) src/./test/fuzz/util.h:43:27 #9 0x563a15958240 in wallet::(anonymous namespace)::wallet_notifications_fuzz_target(Span<unsigned char const>) src/wallet/test/fuzz/notifications.cpp:196:9 #10 0x563a15fdef0c in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9 #11 0x563a15fdef0c in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:178:5 #12 0x563a158032a4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19822a4) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) #13 0x563a15802999 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1981999) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) #14 0x563a15804586 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1983586) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) #15 0x563a15804aa7 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1983aa7) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) #16 0x563a157f21fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19711fb) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) #17 0x563a1581c766 in main (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x199b766) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) #18 0x7f499e17b0cf (/lib/x86_64-linux-gnu/libc.so.6+0x280cf) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89) #19 0x7f499e17b188 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28188) (BuildId: 96ab1a8f3b2c9a2ed37c7388615e6a726d037e89) #20 0x563a157e70c4 in _start (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x19660c4) (BuildId: 8acb42ad599d7f6d25b6f93e18fd564d80df7c06) SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow policy/feerate.cpp:29:63 in MS: 0 ; base unit: 0000000000000000000000000000000000000000 0x3f,0x0,0x2f,0x5f,0x5f,0x5f,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0x7d,0xff,0xff,0xff,0xff,0xff,0x53,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x0,0x0,0x0,0x0,0x13,0x5e,0x5f,0x5f,0x8,0x25,0x0,0x5f,0x5f,0x5f,0x5f,0x5f,0x5f,0x8,0x25,0xca,0x7f,0x5f,0x5f,0x5f,0x13,0x13,0x5f,0x5f,0x5f,0x2,0xdb,0xca,0x0,0x0,0xe7,0xe6,0x66,0x65,0x0,0x0,0x0,0x0,0x44,0x3f,0xa,0xa,0xff,0xff,0xff,0xff,0xff,0x61,0x76,0x6f,0x69,0x0,0xb5,0x15, ?\000/___}}}}}}}}}}}}}}}}}}}}\377\377\377\377\377S\377\377\377\377\377\000\000\000\000\000\000\023^__\010%\000______\010%\312\177___\023\023___\002\333\312\000\000\347\346fe\000\000\000\000D?\012\012\377\377\377\377\377avoi\000\265\025 artifact_prefix='./'; Test unit written to ./crash-4d3bac8a64d4e58b2f0943e6d28e6e1f16328d7d Base64: PwAvX19ffX19fX19fX19fX19fX19fX19fX3//////1P//////wAAAAAAABNeX18IJQBfX19fX18IJcp/X19fExNfX18C28oAAOfmZmUAAAAARD8KCv//////YXZvaQC1FQ== ACKs for top commit: dergoegge: ACK fab164f brunoerg: ACK fab164f Tree-SHA512: f416828f4394aa7303ee437f141e9bbd23c0e0f1b830e4ef3932338858249ba68a811b9837c5b7ad8c6ab871b6354996434183597c1a910a8d8e8d829693e4b2
The previous commit added a test which would fail the unsigned-integer-overflow sanitizer. The warning is harmless and can be triggered on any commit, since the code was introduced. For reference, the warning would happen when the separator `-` was not present. For example: GET /rest/getutxos/6a297bfa5cb8dd976ab0207a767d6cbfaa5e876f30081127ec8674c8c52b16c0_+1.json would result in: rest.cpp:792:77: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'size_type' (aka 'unsigned long') #0 0x55ad42c16931 in rest_getutxos(std::any const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) src/rest.cpp:792:77 #1 0x55ad4319e3c0 in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9 #2 0x55ad4319e3c0 in HTTPWorkItem::operator()() src/httpserver.cpp:59:9 #3 0x55ad431a3eea in WorkQueue<HTTPClosure>::Run() src/httpserver.cpp:114:13 #4 0x55ad4318f961 in HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) src/httpserver.cpp:403:12 #5 0x7f078ebcbbb3 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xeabb3) (BuildId: 40b9b0d17fdeebfb57331304da2b7f85e1396ef2) #6 0x55ad4277e01c in asan_thread_start(void*) asan_interceptors.cpp.o #7 0x7f078e840a93 (/lib/x86_64-linux-gnu/libc.so.6+0x9ca93) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3) #8 0x7f078e8cdc3b (/lib/x86_64-linux-gnu/libc.so.6+0x129c3b) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3) SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow rest.cpp:792:77
…et_create_transaction 5a26cf7 fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction (brunoerg) Pull request description: This PR limites the value of `m_confirm_target` to avoid `implicit-integer-sign-change`: ``` /ci_container_base/src/wallet/fees.cpp:58:58: runtime error: implicit conversion from type 'unsigned int' of value 4294967292 (32-bit, unsigned) to type 'int' changed the value to -4 (32-bit, signed) #0 0x55b6fd26c021 in wallet::GetMinimumFeeRate(wallet::CWallet const&, wallet::CCoinControl const&, FeeCalculation*) ci/scratch/build-x86_64-pc-linux-gnu/src/wallet/./src/wallet/fees.cpp:58:58 #1 0x55b6fd3ef5ca in wallet::CreateTransactionInternal(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) ci/scratch/build-x86_64-pc-linux-gnu/src/wallet/./src/wallet/spend.cpp:1101:49 #2 0x55b6fd3ebea5 in wallet::CreateTransaction(wallet::CWallet&, std::vector<wallet::CRecipient, std::allocator<wallet::CRecipient>> const&, std::optional<unsigned int>, wallet::CCoinControl const&, bool) ci/scratch/build-x86_64-pc-linux-gnu/src/wallet/./src/wallet/spend.cpp:1382:16 #3 0x55b6fccbc154 in wallet::(anonymous namespace)::wallet_create_transaction_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/./src/wallet/test/fuzz/spend.cpp:99:11 #4 0x55b6fccda45d in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9 #5 0x55b6fccda45d in LLVMFuzzerTestOneInput ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/util/./src/test/fuzz/fuzz.cpp:211:5 #6 0x55b6fc368484 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c8a484) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) #7 0x55b6fc367b79 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c89b79) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) #8 0x55b6fc369796 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c8b796) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) #9 0x55b6fc369ca7 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c8bca7) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) #10 0x55b6fc35719f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c7919f) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) #11 0x55b6fc381826 in main (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1ca3826) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) #12 0x7f934c6661c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6) #13 0x7f934c66628a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6) #14 0x55b6fc34c184 in _start (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c6e184) (BuildId: d11f8692b05f02b5a14b6e7579598b426e3144c5) SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change /ci_container_base/src/wallet/fees.cpp:58:58 MS: 0 ; base unit: 0000000000000000000000000000000000000000 0x2e,0x1,0xb0,0xb8,0x0,0xff,0xff,0xff,0xff,0x60,0x14,0x22,0xff,0xff,0xff,0xff,0xff,0xfd,0xff,0xff,0xff,0xff,0xff,0x7e,0xf9,0x41,0x8,0x2b,0x17,0x58,0xb,0x17,0xfc,0xff,0xff,0xff,0xff,0xff,0xff,0x7e,0xf9,0x41,0x8,0x2b,0x17,0x58,0xb, .\001\260\270\000\377\377\377\377`\024\"\377\377\377\377\377\375\377\377\377\377\377~\371A\010+\027X\013\027\374\377\377\377\377\377\377~\371A\010+\027X\013 artifact_prefix='./'; Test unit written to ./crash-5627f57ffba7568a500f8379f62c3338978b43f2 Base64: LgGwuAD/////YBQi///////9//////9++UEIKxdYCxf8////////fvlBCCsXWAs= ``` ACKs for top commit: maflcko: lgtm ACK 5a26cf7 dergoegge: utACK 5a26cf7 Tree-SHA512: a1b129d81d42350cf85ff6fb95cd6982b6aac88467a526ee4b3c9b3313af2f7952c5dfa9886f455756faba399d8356b6c318d7ab2d6318e08fea838bee90b2fe
The parent PR: bitcoin#25797.
The previous PRs in the staging branch: #5, #6.
This PR adds the
bitcoind
build target with its dependencies.To get comparable binaries, one can use the following commands:
Windows-specific notes for reviewers
Windows MSVC builds use dependencies provided by the vcpkg package manager. To install them, run:
To configure, a toolchain file and a triplet must be specified:
cmake -G "Visual Studio 17 2022" -A x64 --toolchain C:/vcpkg/scripts/buildsystems/vcpkg.cmake -DVCPKG_TARGET_TRIPLET=x64-windows-static -S . -B build
Then, build the
all
target as usual: