build: Add CMake-based build system (6 of N) #15

hebasto · 2023-05-01T17:53:11Z

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" commit to the main repo now?

FWIW, we use the same approach for functional tests providing the BITCOIND environment variable when needed.

…ake/-/issues/20652

hebasto · 2023-05-01T17:59:46Z

Friendly ping @fanquake @TheCharlatan @theuni :)

fanquake · 2023-05-02T09:23:07Z

Is make check being run with multiple jobs? I assume not, because this currently takes > 5 minutes to run. With 0e2a1fc:

cmake -S . -B  build
make -C build -j9
make check -C build -j9
...
100% tests passed, 0 tests failed out of 110

Total Test time (real) = 343.94 sec
[100%] Built target check

hebasto · 2023-05-02T13:04:08Z

Is make check being run with multiple jobs? I assume not

That is true, unfortunately.

There are two options to deal with it.

Set the CTEST_PARALLEL_LEVEL environment variable:

export CTEST_PARALLEL_LEVEL=9

Use CMake's way to run tests:

ctest --test-dir build/src/test -j9

This approach will run only tests being added by add_test command, therefore, other make check jobs, i.e., bench_bitcoin, util/test_runner.py and util/rpcauth-test.py, must be converted respectively.

Please let me know which way is preferable for us.

fanquake · 2023-05-02T13:10:58Z

which way is preferable for us.

I don't think using an environment variable is going to work. That would also be completely unintuitive; if I call make check -j9, I'd expect multiple jobs to be run, like every other command, not have to set some environment variable for that to work.

I guess we'll have to do something with ctest, and wrap everything up so you can still just do make check (-jx) and things work as expected.

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

test/CMakeLists.txt

hebasto · 2023-05-02T19:20:06Z

Fixed the source file list for the test_bitcoin target.

hebasto · 2023-05-02T21:43:58Z

Dropped make check support in favour of clean and parallelizable CTest usage:

cmake -S . -B build
cmake --build build -j $(nproc)
ctest --test-dir build -j $(nproc)

cmake -S . -B build -G "Ninja Multi-Config"
cmake --build build -j $(nproc)
ctest --test-dir build -j $(nproc) -C Debug

hebasto · 2023-05-03T08:38:12Z

Fellow reviewers,

It's worth mentioning that I've opened bitcoin#27554 which is related to the testing stuff for the CMake-based build system but not required for this PR particularly.

UPD. bitcoin#27561 as well :)

hebasto · 2023-05-03T11:16:39Z

Reworked to include commits from bitcoin#27561.

hebasto · 2023-05-03T17:36:29Z

The parent PR bitcoin#25797 has been updated on top of this one.

Its CI and Guix builds are OK :)

hebasto · 2023-05-04T13:27:54Z

@fanquake

Is make check being run with multiple jobs? I assume not

In this branch, the top commit implements this functionality.

On Linux and macOS, it allows to build as follows:

cmake -S . -B  build
make -C build -j9
make check -C build -j9

I'll be happy to add it to this branch in case fellow reviewers won't consider it too hackish :)

TheCharlatan

Going to start with the test commits now.

TheCharlatan · 2023-05-04T14:53:15Z

cmake/module/FindBerkeleyDB.cmake

+find_path(BerkeleyDB_INCLUDE_DIR
+  NAMES db.h
+  HINTS ${bdb4_brew_prefix}/include
+  PATH_SUFFIXES 4.8 48 4 db4 5 5.3 db5


Out of curiosity, where did you get these from?

Sources:

github search for FindBerkeleyDB.cmake :)

feedback from @theStack (thanks!)

cmake/module/FindBerkeleyDB.cmake

TheCharlatan · 2023-05-04T16:33:06Z

src/CMakeLists.txt

+      wallet/wallettool.cpp
+    )
+    target_link_libraries(bitcoin-wallet
+      bitcoin_wallet


The naming and semantics between - and _ constantly trips me up, but I don't think there is really a viable alternative.

hebasto · 2023-05-04T18:20:31Z

Updated bb57453 -> c246c86 (cmake15.04 -> cmake15.05, diff):

addressed @TheCharlatan's comment

theuni · 2023-05-11T19:15:42Z

Sorry for the delay, will review tomorrow.

theuni

Again, sorry for the delay. Here's my first pass review.

I've skipped over some of the rougher-to-review things like the bdb detection and the header generators.

theuni · 2023-05-12T18:07:18Z

cmake/module/CrossPkgConfig.cmake

+function(remove_isystem_from_include_directories_internal target)
+  get_target_property(include_directories ${target} INTERFACE_INCLUDE_DIRECTORIES)
+  if(include_directories)
+    list(REMOVE_ITEM include_directories -isystem)


What happens in the case of CFLAGS=-isystem foo -I bar ?

It is all about how CMake handles the output of the pkg-config --cflags-only-I <package> command. All -I some_directory instances are handled fine.

For example, in a broken combo of CMake 3.16.6 + pkg-config 0.29.2, the value of the INTERFACE_INCLUDE_DIRECTORIES property of the imported PkgConfig::libzmq target is a list as follows:

-isystem;/usr/include/mit-krb5;/usr/include/pgm-5.3;/usr/include/libxml2

Compare to:

$ pkg-config --cflags-only-I libzmq -isystem /usr/include/mit-krb5 -I/usr/include/pgm-5.3 -I/usr/include/libxml2

Therefore, to fix the bug, it is enough to just filter out "-isystem" from the INTERFACE_INCLUDE_DIRECTORIES list.

Could you add a note explaining what's happening here and what it's working around please? That's not clear to me from the code alone.

theuni · 2023-05-12T18:12:46Z

src/CMakeLists.txt

+    bitcoin_cli
+    bitcoin_common
+    bitcoin_util
+    libevent::libevent


crypto is missing here. Is it actually unneeded in autotools?

crypto is missing here.

In CMake, the bitcoin_crypto object library is not a direct dependency of the bitcoin-cli target, rather a dependency of the bitcoin_util static library.

Is it actually unneeded in autotools?

No. It is still needed in Autotools. Otherwise, the libbitcoin_util.a will have undefined references (in src/random.cpp).

theuni · 2023-05-12T18:13:54Z

src/CMakeLists.txt

+  target_link_libraries(bitcoin-tx
+    bitcoin_common
+    bitcoin_util
+    univalue


Same here. No need for crypto upstream? I'll stop asking if I see more as I assume I'm missing something.

Same here.

Indeed :)

No need for crypto upstream?

It is still needed in Autotools.

CMakeLists.txt

src/wallet/CMakeLists.txt

test/functional/test_runner.py

theuni · 2023-05-12T18:37:28Z

test/CMakeLists.txt

+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+set(abs_top_srcdir ${PROJECT_SOURCE_DIR})


I need help understanding.... everything in this file please :)

Could you please give a description of what's going on with the setting/unsetting and file copying?

This is an emulation of Autoconf's AC_CONFIG_FILES([test/config.ini]) macro. The test/config.ini.in file itself exploits the Automake's implementation details of the AM_CONDITIONAL macro. For example, @USE_SQLITE_TRUE@ can be substituted with an empty string or with # character.

Possible to move all this into a function so that it's scoped? That way you could use PARENT_SCOPE as necessary (if at all?) for outputs, and you don't have to worry about unsetting afterwards. Kinda like RAII I suppose.
If possible, I think that would make this much less confusing.

Possible to move all this into a function so that it's scoped?

Sure! Updated.

A note: this file can be significantly simplified after dropping Autotools.

theuni · 2023-05-12T18:55:26Z

src/test/CMakeLists.txt

+add_executable(test_bitcoin
+  main.cpp
+  $<TARGET_OBJECTS:bitcoin_consensus>
+  ${CMAKE_CURRENT_BINARY_DIR}/data/asmap.raw.h


These don't need to be compiled as part of test_bitcoin, they're dependencies of the sources of test_bitcoin.

For example: test/script_tests.cpp
depends on data/script_tests.json.h
depends on data/script_tests.json

So, ideally this would be coded in terms of dependencies rather than a "build all the headers" approach. But I'm not sure if that's expressible via CMAKE.

But at minimum, the sources need to depend on the generated headers, and those shouldn't be listed here.

The OBJECT_DEPENDS target property:

was originally introduced for this purpose...

But

... it is no longer necessary.

~~Reworked.~~

CMake's way is to handle such dependencies at a target level.

So, ideally this would be coded in terms of dependencies rather than a "build all the headers" approach.

It is coded in terms of dependencies. No header will be generated if it is not required for a target being built.

src/test/CMakeLists.txt

CMakeLists.txt

hebasto · 2023-05-13T18:23:45Z

Updated c246c86 -> f8131bf (cmake15.05 -> cmake15.06, diff):

fixed the source file list for the bench_bitcoin target
used the recent version of commits from test: Explicitly specify directory where to search tests for bitcoin/bitcoin#27561
addressed @theuni's comments:

hebasto · 2023-05-22T10:48:58Z

The 2fd303f commit has been pulled into the main repository -- bitcoin#27717.

theuni · 2023-05-22T12:18:41Z

I spent Friday getting macOS + XCode setup and working with this PR. It required...

🥁🥁🥁

No changes! Everything just worked.

I'm new to XCode and don't really know what I'm doing yet, but the all target worked, as well as bitcoind and any other that I picked.

So that's really good to know. I'll likely test now and then with XCode, but it seems pretty happy with what CMake produces.

hebasto · 2023-05-22T13:04:23Z

I'm new to XCode and don't really know what I'm doing yet...

Using Xcode generator?

theuni · 2023-05-22T13:10:36Z

I'm new to XCode and don't really know what I'm doing yet...

Using Xcode generator?

Yes. Using that branch with cmake built from depends, I used:
/Users/cory/dev/bitcoin/depends/aarch64-apple-darwin22.5.0/native/bin/cmake -B . -S .. -DCMAKE_TOOLCHAIN_FILE:FILEPATH=/Users/cory/dev/bitcoin/depends/aarch64-apple-darwin22.5.0/share/toolchain.cmake -GXcode (plus a few config tweaks)

I then opened the resultingBitcoin Core.xcodeproj in XCode with no issues.

TheCharlatan · 2023-05-23T07:10:22Z

test/CMakeLists.txt

+  set_configure_variable(BUILD_WALLET_TOOL BUILD_BITCOIN_WALLET)
+  set_configure_variable(BUILD_DAEMON BUILD_BITCOIND_TRUE)
+  set_configure_variable(WITH_ZMQ ENABLE_ZMQ)
+  set_configure_variable(ENABLE_EXTERNAL_SIGNER ENABLE_EXTERNAL_SIGNER)


Afaict these last three are still missing, right?

The ENABLE_EXTERNAL_SIGNER an USE_SYSCALL_SANDBOX variables are not introduced in this PR. As for ENABLE_TRACING, see

bitcoin/cmake/optional.cmake

Lines 90 to 108 in 2d8930e

if(WITH_USDT)

check_cxx_source_compiles("

#include <sys/sdt.h>

int main()

{

DTRACE_PROBE(\"context\", \"event\");

}

" HAVE_USDT_H

)

if(HAVE_USDT_H)

set(ENABLE_TRACING TRUE)

set(WITH_USDT ON)

elseif(WITH_USDT STREQUAL "AUTO")

set(WITH_USDT OFF)

else()

message(FATAL_ERROR "sys/sdt.h requested, but not found.")

endif()

endif()

Anyway, the processing of the test/config.ini.in file expects all three variables set:

ENABLE_EXTERNAL_SIGNER_TRUE

ENABLE_SYSCALL_SANDBOX_TRUE

ENABLE_USDT_TRACEPOINTS_TRUE

Right, all good then :)

hebasto · 2023-05-23T08:03:14Z

bitcoin#27458 has been backported into this branch.

TheCharlatan

ACK 43123cf

Regarding the multi-config generators, I tried running cmake --build . -j 24 --config Release, but got a lot of build errors. This will be handled in some future pull request. cmake --build . -j 24 --config Debug runs just fine.

TheCharlatan · 2023-05-23T07:50:23Z

src/test/util/CMakeLists.txt

+if(ENABLE_WALLET)
+  target_sources(test_util
+    PRIVATE
+      ../../wallet/test/util.cpp


Nit: Is there a particular reason these are relative paths? Otherwise, I'd prefer an absolute path for these (also in the following commits).

We can improve it later, after dropping Autotools, by moving source files into better directories.

TheCharlatan · 2023-05-23T08:07:33Z

test/CMakeLists.txt

+  set_configure_variable(BUILD_WALLET_TOOL BUILD_BITCOIN_WALLET)
+  set_configure_variable(BUILD_DAEMON BUILD_BITCOIND_TRUE)
+  set_configure_variable(WITH_ZMQ ENABLE_ZMQ)
+  set_configure_variable(ENABLE_EXTERNAL_SIGNER ENABLE_EXTERNAL_SIGNER)


Right, all good then :)

…BITCOINUTIL` and `BITCOINTX` 4f2f615 test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX` (Hennadii Stepanov) Pull request description: This PR is a continuation of changes to our testing frameworks (bitcoin/bitcoin#27554, bitcoin/bitcoin#27561) that allow them to work correctly in a multi-config build environment that is possible for [upcoming](bitcoin/bitcoin#25797) CMake-based build system. That means that built for different configurations binaries (e.g., "Debug" and "Release") can coexist in separated directories. The commit has been pulled from hebasto/bitcoin#15 and it seems [useful](hebasto/bitcoin#15 (comment)) by itself as: > I believe the rationale for allowing to drop in the executables via env var is to allow to test the guix-produced, or other third-party-produced executables... The current implementation of the `test/functional/test_framework/test_framework.py` script uses the same approach: https://github.com/bitcoin/bitcoin/blob/09351f51d279612973ecd76811dc075dff08209f/test/functional/test_framework/test_framework.py#L231-L246 ACKs for top commit: MarcoFalke: lgtm ACK 4f2f615 TheCharlatan: ACK 4f2f615 stickies-v: ACK 4f2f615 Tree-SHA512: 99ee9a727b266700649d8f2ec528dfaeb04a1e48f0cb1d4eeaece65917165be647c10c4548429a9e8b30d094597f67e860c1db03ac689ebb409b223ce1b63aa9

hebasto · 2023-05-23T12:26:55Z

The 2fd303f commit has been pulled into the main repository -- bitcoin#27717.

bitcoin#27717 has just been merged to the main repository.

…ch tests for c44f3f2 test: Explicitly specify directory where to search tests for (Hennadii Stepanov) Pull request description: For out-of-source builds, the `test/functional/test_runner.py` is supposed to be run from the build directory which allows it to pick the `test/config.ini` file generated by the build system. Currently, it works accidently for the following reasons: - on POSIX systems, when running a created by Autoconf symlink to the `test/functional/test_runner.py` in the source directory, it actually has the source directory location in the `sys.path`. - on Windows (the `build_msvc` directory) VS project puts and copies every build artifact into the source tree (which is wrong and ugly). This PR makes `test/functional/test_runner.py` work from a build directory in any form (a symbolic link, a hard link, a copy) on _all_ supported platforms, which is highly desirable in the upcoming [CMake-based build system](bitcoin#25797). For the current master branch, this PR has no behaviour change. Required for hebasto#15. --- **Steps to reproduce the issue** While the issue is mostly specific to Windows and CMake builds, it is still possible to reproduce it on the current master branch. 1. Make an out-of-source build: ``` $ ./autogen.sh $ mkdir ../build && cd ../build $ ../bitcoin/configure $ make ``` 2. Note that Autoconf created a symbolic link `test/functional/test_runner.py` in the `../build` directory: ``` $ ls -l test/functional/test_runner.py lrwxrwxrwx 1 hebasto hebasto 47 May 5 17:40 test/functional/test_runner.py -> ../../../bitcoin/test/functional/test_runner.py ``` which works flawlessly. 3. However, replacing this symbolic link with a hard link or a copy of `test/functional/test_runner.py` from the source tree will cause the following error: ``` $ cp ../bitcoin/test/functional/test_runner.py test/functional/test_runner.py $ ls -l test/functional/test_runner.py $ ./test/functional/test_runner.py Temporary test directory at /tmp/test_runner_₿_🏃_20230505_175104 Running Unit Tests for Test Framework Modules E ====================================================================== ERROR: test_framework (unittest.loader._FailedTest) ---------------------------------------------------------------------- ImportError: Failed to import test module: test_framework Traceback (most recent call last): File "/usr/lib/python3.10/unittest/loader.py", line 154, in loadTestsFromName module = __import__(module_name) ModuleNotFoundError: No module named 'test_framework' ---------------------------------------------------------------------- Ran 1 test in 0.000s FAILED (errors=1) Early exiting after failure in TestFramework unit tests ``` ACKs for top commit: stickies-v: re-ACK bitcoin@c44f3f2 MarcoFalke: lgtm ACK c44f3f2 💸 Tree-SHA512: 622ff629080a55f76dd4c1dab6699de0e9f06b75da3315cd3b31b972ef4bde746458bf3e8a95e879b3c6a63be2368af70005a83f6a3c85c4f1ba5be51e91a61d

…TIL` and `BITCOINTX` 4f2f615 test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX` (Hennadii Stepanov) Pull request description: This PR is a continuation of changes to our testing frameworks (bitcoin#27554, bitcoin#27561) that allow them to work correctly in a multi-config build environment that is possible for [upcoming](bitcoin#25797) CMake-based build system. That means that built for different configurations binaries (e.g., "Debug" and "Release") can coexist in separated directories. The commit has been pulled from hebasto#15 and it seems [useful](hebasto#15 (comment)) by itself as: > I believe the rationale for allowing to drop in the executables via env var is to allow to test the guix-produced, or other third-party-produced executables... The current implementation of the `test/functional/test_framework/test_framework.py` script uses the same approach: https://github.com/bitcoin/bitcoin/blob/09351f51d279612973ecd76811dc075dff08209f/test/functional/test_framework/test_framework.py#L231-L246 ACKs for top commit: MarcoFalke: lgtm ACK 4f2f615 TheCharlatan: ACK 4f2f615 stickies-v: ACK 4f2f615 Tree-SHA512: 99ee9a727b266700649d8f2ec528dfaeb04a1e48f0cb1d4eeaece65917165be647c10c4548429a9e8b30d094597f67e860c1db03ac689ebb409b223ce1b63aa9

hebasto · 2023-05-30T08:50:30Z

FWIW, the parent PR, i.e., bitcoin#25797, has been rebased recently using this branch.

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

theuni

See one nit about a request for a comment.

ACK to keep things going.

I guess the plan is to rebase on master after merging this, and that will de-dupe the changes that have already been upstreamed?

theuni · 2023-07-06T17:24:41Z

cmake/module/CrossPkgConfig.cmake

+function(remove_isystem_from_include_directories_internal target)
+  get_target_property(include_directories ${target} INTERFACE_INCLUDE_DIRECTORIES)
+  if(include_directories)
+    list(REMOVE_ITEM include_directories -isystem)


Could you add a note explaining what's happening here and what it's working around please? That's not clear to me from the code alone.

hebasto · 2023-07-07T14:39:16Z

I guess the plan is to rebase on master after merging this, and that will de-dupe the changes that have already been upstreamed?

Exactly :)

See: #16.

hebasto · 2023-07-07T14:46:36Z

re https://github.com/hebasto/bitcoin/pull/15/files#r1254726487

Could you add a note explaining what's happening here and what it's working around please? That's not clear to me from the code alone.

I'll do it in the next non-rebasing PR as a FIXUP.

hebasto · 2023-07-10T14:39:05Z

@theuni

re #15 (files)

Could you add a note explaining what's happening here and what it's working around please? That's not clear to me from the code alone.

I'll do it in the next non-rebasing PR as a FIXUP.

See #17.

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

…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

…ch tests for c44f3f2 test: Explicitly specify directory where to search tests for (Hennadii Stepanov) Pull request description: For out-of-source builds, the `test/functional/test_runner.py` is supposed to be run from the build directory which allows it to pick the `test/config.ini` file generated by the build system. Currently, it works accidently for the following reasons: - on POSIX systems, when running a created by Autoconf symlink to the `test/functional/test_runner.py` in the source directory, it actually has the source directory location in the `sys.path`. - on Windows (the `build_msvc` directory) VS project puts and copies every build artifact into the source tree (which is wrong and ugly). This PR makes `test/functional/test_runner.py` work from a build directory in any form (a symbolic link, a hard link, a copy) on _all_ supported platforms, which is highly desirable in the upcoming [CMake-based build system](bitcoin#25797). For the current master branch, this PR has no behaviour change. Required for hebasto#15. --- **Steps to reproduce the issue** While the issue is mostly specific to Windows and CMake builds, it is still possible to reproduce it on the current master branch. 1. Make an out-of-source build: ``` $ ./autogen.sh $ mkdir ../build && cd ../build $ ../bitcoin/configure $ make ``` 2. Note that Autoconf created a symbolic link `test/functional/test_runner.py` in the `../build` directory: ``` $ ls -l test/functional/test_runner.py lrwxrwxrwx 1 hebasto hebasto 47 May 5 17:40 test/functional/test_runner.py -> ../../../bitcoin/test/functional/test_runner.py ``` which works flawlessly. 3. However, replacing this symbolic link with a hard link or a copy of `test/functional/test_runner.py` from the source tree will cause the following error: ``` $ cp ../bitcoin/test/functional/test_runner.py test/functional/test_runner.py $ ls -l test/functional/test_runner.py $ ./test/functional/test_runner.py Temporary test directory at /tmp/test_runner_₿_🏃_20230505_175104 Running Unit Tests for Test Framework Modules E ====================================================================== ERROR: test_framework (unittest.loader._FailedTest) ---------------------------------------------------------------------- ImportError: Failed to import test module: test_framework Traceback (most recent call last): File "/usr/lib/python3.10/unittest/loader.py", line 154, in loadTestsFromName module = __import__(module_name) ModuleNotFoundError: No module named 'test_framework' ---------------------------------------------------------------------- Ran 1 test in 0.000s FAILED (errors=1) Early exiting after failure in TestFramework unit tests ``` ACKs for top commit: stickies-v: re-ACK bitcoin@c44f3f2 MarcoFalke: lgtm ACK c44f3f2 💸 Tree-SHA512: 622ff629080a55f76dd4c1dab6699de0e9f06b75da3315cd3b31b972ef4bde746458bf3e8a95e879b3c6a63be2368af70005a83f6a3c85c4f1ba5be51e91a61d

hebasto added 5 commits May 1, 2023 08:29

[FIXUP] cmake: Consider ASM option when checking for HAVE_64BIT_ASM

b2bea9f

[FIXUP] cmake: Add workaround for https://gitlab.kitware.com/cmake/cm…

5fc2cee

…ake/-/issues/20652

cmake: Build bitcoin-cli executable

1934755

cmake: Build bitcoin-tx executable

d1c319d

cmake: Build bitcoin-util executable

f944ccd

hebasto commented May 2, 2023

View reviewed changes

test/CMakeLists.txt Outdated Show resolved Hide resolved

hebasto force-pushed the 230501-cmake-F branch from 0e2a1fc to 284ece9 Compare May 2, 2023 19:19

hebasto force-pushed the 230501-cmake-F branch from 284ece9 to 9f4ff31 Compare May 2, 2023 21:38

hebasto force-pushed the 230501-cmake-F branch from 9f4ff31 to bb57453 Compare May 3, 2023 11:15

hebasto mentioned this pull request May 3, 2023

test: Explicitly specify directory where to search tests for bitcoin/bitcoin#27561

Merged

hebasto mentioned this pull request May 3, 2023

build: Add CMake-based build system bitcoin/bitcoin#25797

Closed

16 tasks

hebasto mentioned this pull request May 4, 2023

cmake: Emulate GNU Autotools 'make check -jN' bitcoin-core/secp256k1#1294

Closed

TheCharlatan reviewed May 4, 2023

View reviewed changes

hebasto force-pushed the 230501-cmake-F branch from bb57453 to c246c86 Compare May 4, 2023 18:16

theuni reviewed May 12, 2023

View reviewed changes

hebasto force-pushed the 230501-cmake-F branch from c246c86 to f8131bf Compare May 13, 2023 18:14

TheCharlatan reviewed May 23, 2023

View reviewed changes

FIXUP: Same as PR27458

43123cf

TheCharlatan approved these changes May 23, 2023

View reviewed changes

theuni approved these changes Jul 6, 2023

View reviewed changes

hebasto merged commit 3a1fe30 into cmake-staging Jul 7, 2023

hebasto mentioned this pull request Jul 10, 2023

build: Add CMake-based build system (7 of N) #17

Merged

This was referenced Jul 12, 2023

build: Add CMake-based build system (8 of N) #18

Merged

build: Add CMake-based build system (9 of N) #19

Merged

hebasto mentioned this pull request Oct 6, 2023

CMake-based build system tracking issue bitcoin/bitcoin#28607

Closed

16 tasks

	if(WITH_USDT)
	check_cxx_source_compiles("
	#include <sys/sdt.h>

	int main()
	{
	DTRACE_PROBE(\"context\", \"event\");
	}
	" HAVE_USDT_H
	)
	if(HAVE_USDT_H)
	set(ENABLE_TRACING TRUE)
	set(WITH_USDT ON)
	elseif(WITH_USDT STREQUAL "AUTO")
	set(WITH_USDT OFF)
	else()
	message(FATAL_ERROR "sys/sdt.h requested, but not found.")
	endif()
	endif()

build: Add CMake-based build system (6 of N) #15

build: Add CMake-based build system (6 of N) #15

Conversation

hebasto commented May 1, 2023 • edited Loading

hebasto commented May 1, 2023

fanquake commented May 2, 2023

hebasto commented May 2, 2023 • edited Loading

fanquake commented May 2, 2023 • edited Loading

hebasto commented May 2, 2023

hebasto commented May 2, 2023 • edited Loading

hebasto commented May 3, 2023 • edited Loading

hebasto commented May 3, 2023

hebasto commented May 3, 2023 • edited Loading

hebasto commented May 4, 2023

TheCharlatan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented May 4, 2023

theuni commented May 11, 2023

theuni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theuni May 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto May 13, 2023 • edited Loading

Choose a reason for hiding this comment

hebasto commented May 13, 2023 • edited Loading

hebasto commented May 22, 2023

theuni commented May 22, 2023

hebasto commented May 22, 2023

theuni commented May 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented May 23, 2023

TheCharlatan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented May 23, 2023

hebasto commented May 30, 2023

theuni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented Jul 7, 2023

hebasto commented Jul 7, 2023

hebasto commented Jul 10, 2023

hebasto commented May 1, 2023 •

edited

Loading

hebasto commented May 2, 2023 •

edited

Loading

fanquake commented May 2, 2023 •

edited

Loading

hebasto commented May 2, 2023 •

edited

Loading

hebasto commented May 3, 2023 •

edited

Loading

hebasto commented May 3, 2023 •

edited

Loading

theuni May 19, 2023 •

edited

Loading

hebasto May 13, 2023 •

edited

Loading

hebasto commented May 13, 2023 •

edited

Loading