From f0aec78d9377e84db7cca0b5cb5d67204ee4e216 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 28 Jun 2024 12:22:51 +0100 Subject: [PATCH 1/8] cmake: Switch to libsecp256k1 upstream build system --- CMakeLists.txt | 1 - cmake/secp256k1.cmake | 62 ------------------------------------------- src/CMakeLists.txt | 20 ++++++++++++-- 3 files changed, 18 insertions(+), 65 deletions(-) delete mode 100644 cmake/secp256k1.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 4583af6d5efaa..5067151257786 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -369,7 +369,6 @@ include(cmake/ccache.cmake) include(cmake/crc32c.cmake) include(cmake/leveldb.cmake) include(cmake/minisketch.cmake) -include(cmake/secp256k1.cmake) add_library(warn_interface INTERFACE) target_link_libraries(core_interface INTERFACE warn_interface) diff --git a/cmake/secp256k1.cmake b/cmake/secp256k1.cmake deleted file mode 100644 index 22139e519b4a8..0000000000000 --- a/cmake/secp256k1.cmake +++ /dev/null @@ -1,62 +0,0 @@ -# Copyright (c) 2023-present The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or https://opensource.org/license/mit/. - -# This file is part of the transition from Autotools to CMake. Once CMake -# support has been merged we should switch to using the upstream CMake -# buildsystem. - -enable_language(C) -set(CMAKE_C_STANDARD 90) -set(CMAKE_C_EXTENSIONS OFF) -string(APPEND CMAKE_C_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CFLAGS}") - -include(CheckCSourceCompiles) -check_c_source_compiles(" - #include - - int main() - { - uint64_t a = 11, tmp; - __asm__ __volatile__(\"movq $0x100000000,%1; mulq %%rsi\" : \"+a\"(a) : \"S\"(tmp) : \"cc\", \"%rdx\"); - } - " HAVE_64BIT_ASM -) - -add_library(secp256k1 STATIC EXCLUDE_FROM_ALL - ${PROJECT_SOURCE_DIR}/src/secp256k1/src/secp256k1.c - ${PROJECT_SOURCE_DIR}/src/secp256k1/src/precomputed_ecmult.c - ${PROJECT_SOURCE_DIR}/src/secp256k1/src/precomputed_ecmult_gen.c -) - -target_compile_definitions(secp256k1 - PRIVATE - ECMULT_WINDOW_SIZE=15 - ENABLE_MODULE_RECOVERY - ENABLE_MODULE_SCHNORRSIG - ENABLE_MODULE_EXTRAKEYS - ENABLE_MODULE_ELLSWIFT - # COMB_* definitions for SECP256K1_ECMULT_GEN_KB=86 - COMB_BLOCKS=43 - COMB_TEETH=6 - $<$:USE_ASM_X86_64=1> - INTERFACE - $<$:SECP256K1_STATIC> -) - -target_include_directories(secp256k1 - PUBLIC - $ -) - -if(MSVC) - target_compile_options(secp256k1 - PRIVATE - /wd4146 - /wd4244 - /wd4267 - ) -endif() - -target_link_libraries(secp256k1 PRIVATE core_base_interface) -set_target_properties(secp256k1 PROPERTIES EXPORT_COMPILE_COMMANDS OFF) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6116b817bc403..39477ebeb3751 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,6 +1,6 @@ -# Copyright (c) 2023 The Bitcoin Core developers +# Copyright (c) 2023-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. +# file COPYING or https://opensource.org/license/mit/. include(GNUInstallDirs) include(AddWindowsResources) @@ -38,6 +38,22 @@ if(WITH_MULTIPROCESS) add_subdirectory(ipc) endif() +#============================= +# secp256k1 subtree +#============================= +message("") +message("Configuring secp256k1 subtree...") +set(SECP256K1_DISABLE_SHARED ON CACHE BOOL "" FORCE) +set(SECP256K1_ENABLE_MODULE_ECDH OFF CACHE BOOL "" FORCE) +set(SECP256K1_ENABLE_MODULE_RECOVERY ON CACHE BOOL "" FORCE) +set(SECP256K1_ECMULT_GEN_KB 86 CACHE STRING "" FORCE) +include(GetTargetInterface) +get_target_interface(core_c_flags "" core_base_interface COMPILE_OPTIONS) +set(SECP256K1_LATE_CFLAGS ${core_c_flags} CACHE STRING "" FORCE) +unset(core_c_flags) +add_subdirectory(secp256k1 EXCLUDE_FROM_ALL) +set_target_properties(secp256k1 PROPERTIES EXPORT_COMPILE_COMMANDS OFF) +string(APPEND CMAKE_C_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CFLAGS}") # Stable, backwards-compatible consensus functionality. add_library(bitcoin_consensus STATIC EXCLUDE_FROM_ALL From c3134b05d2fed18739a0bb2d854d3216db3add6a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 25 May 2024 10:15:03 +0100 Subject: [PATCH 2/8] cmake: Let libsecp256k1 manage config-specific C flags --- CMakeLists.txt | 24 ++------------------ cmake/module/ProcessConfigurations.cmake | 28 ------------------------ 2 files changed, 2 insertions(+), 50 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5067151257786..23de7b4d64b98 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -428,15 +428,12 @@ target_compile_definitions(core_interface_debug INTERFACE ) # We leave assertions on. if(MSVC) - remove_c_flag_from_all_configs(/DNDEBUG) remove_cxx_flag_from_all_configs(/DNDEBUG) else() - remove_c_flag_from_all_configs(-DNDEBUG) remove_cxx_flag_from_all_configs(-DNDEBUG) - # Adjust flags used by the C/CXX compiler during RELEASE builds. + # Adjust flags used by the CXX compiler during RELEASE builds. # Prefer -O2 optimization level. (-O3 is CMake's default for Release for many compilers.) - replace_c_flag_in_config(Release -O3 -O2) replace_cxx_flag_in_config(Release -O3 -O2) are_flags_overridden(CMAKE_CXX_FLAGS_DEBUG cxx_flags_debug_overridden) @@ -446,6 +443,7 @@ else() if(compiler_supports_g3) replace_cxx_flag_in_config(Debug -g -g3) endif() + unset(compiler_supports_g3) try_append_cxx_flags("-ftrapv" RESULT_VAR compiler_supports_ftrapv) if(compiler_supports_ftrapv) @@ -462,24 +460,6 @@ else() ) endif() unset(cxx_flags_debug_overridden) - - are_flags_overridden(CMAKE_C_FLAGS_DEBUG c_flags_debug_overridden) - if(NOT c_flags_debug_overridden) - # Redefine flags used by the C compiler during DEBUG builds. - if(compiler_supports_g3) - replace_c_flag_in_config(Debug -g -g3) - endif() - - string(PREPEND CMAKE_C_FLAGS_DEBUG "-O0 ") - - set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}" - CACHE STRING - "Flags used by the C compiler during DEBUG builds." - FORCE - ) - endif() - unset(compiler_supports_g3) - unset(c_flags_debug_overridden) endif() set(CMAKE_CXX_FLAGS_COVERAGE "-Og --coverage") diff --git a/cmake/module/ProcessConfigurations.cmake b/cmake/module/ProcessConfigurations.cmake index d5895497deb26..e53df8c14f112 100644 --- a/cmake/module/ProcessConfigurations.cmake +++ b/cmake/module/ProcessConfigurations.cmake @@ -84,23 +84,6 @@ function(set_default_config config) endif() endfunction() -function(remove_c_flag_from_all_configs flag) - get_all_configs(all_configs) - foreach(config IN LISTS all_configs) - string(TOUPPER "${config}" config_uppercase) - set(flags "${CMAKE_C_FLAGS_${config_uppercase}}") - separate_arguments(flags) - list(FILTER flags EXCLUDE REGEX "${flag}") - list(JOIN flags " " new_flags) - set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}" PARENT_SCOPE) - set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}" - CACHE STRING - "Flags used by the C compiler during ${config_uppercase} builds." - FORCE - ) - endforeach() -endfunction() - function(remove_cxx_flag_from_all_configs flag) get_all_configs(all_configs) foreach(config IN LISTS all_configs) @@ -118,17 +101,6 @@ function(remove_cxx_flag_from_all_configs flag) endforeach() endfunction() -function(replace_c_flag_in_config config old_flag new_flag) - string(TOUPPER "${config}" config_uppercase) - string(REGEX REPLACE "(^| )${old_flag}( |$)" "\\1${new_flag}\\2" new_flags "${CMAKE_C_FLAGS_${config_uppercase}}") - set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}" PARENT_SCOPE) - set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}" - CACHE STRING - "Flags used by the C compiler during ${config_uppercase} builds." - FORCE - ) -endfunction() - function(replace_cxx_flag_in_config config old_flag new_flag) string(TOUPPER "${config}" config_uppercase) string(REGEX REPLACE "(^| )${old_flag}( |$)" "\\1${new_flag}\\2" new_flags "${CMAKE_CXX_FLAGS_${config_uppercase}}") From 469e1ade67406aaf63f2845acb7dff5b2e223a93 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 25 May 2024 10:15:03 +0100 Subject: [PATCH 3/8] cmake: Remove C stuff from summary --- CMakeLists.txt | 1 - cmake/module/FlagsSummary.cmake | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 23de7b4d64b98..88993778398ca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -643,7 +643,6 @@ else() set(cross_status "FALSE") endif() message("Cross compiling ....................... ${cross_status}") -message("C compiler ............................ ${CMAKE_C_COMPILER_ID} ${CMAKE_C_COMPILER_VERSION}, ${CMAKE_C_COMPILER}") message("C++ compiler .......................... ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}, ${CMAKE_CXX_COMPILER}") include(FlagsSummary) flags_summary() diff --git a/cmake/module/FlagsSummary.cmake b/cmake/module/FlagsSummary.cmake index b3e9a5ed65910..563146716cb8e 100644 --- a/cmake/module/FlagsSummary.cmake +++ b/cmake/module/FlagsSummary.cmake @@ -22,17 +22,6 @@ function(print_flags_per_config config indent_num) get_target_interface(definitions ${config} core_interface COMPILE_DEFINITIONS) indent_message("Preprocessor defined macros ..........." "${definitions}" ${indent_num}) - string(STRIP "${CMAKE_C_FLAGS} ${CMAKE_C_FLAGS_${config_uppercase}}" combined_c_flags) - string(STRIP "${combined_c_flags} ${CMAKE_C${CMAKE_C_STANDARD}_STANDARD_COMPILE_OPTION}" combined_c_flags) - if(CMAKE_POSITION_INDEPENDENT_CODE) - string(JOIN " " combined_c_flags ${combined_c_flags} ${CMAKE_C_COMPILE_OPTIONS_PIC}) - endif() - get_target_interface(core_c_flags ${config} core_base_interface COMPILE_OPTIONS) - string(STRIP "${combined_c_flags} ${core_c_flags}" combined_c_flags) - string(STRIP "${combined_c_flags} ${APPEND_CPPFLAGS}" combined_c_flags) - string(STRIP "${combined_c_flags} ${APPEND_CFLAGS}" combined_c_flags) - indent_message("C compiler flags ......................" "${combined_c_flags}" ${indent_num}) - string(STRIP "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${config_uppercase}}" combined_cxx_flags) string(STRIP "${combined_cxx_flags} ${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION}" combined_cxx_flags) if(CMAKE_POSITION_INDEPENDENT_CODE) From 65f15d127ca33d69cd3ca3acc20a25d179703822 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 25 Jun 2024 13:48:14 +0100 Subject: [PATCH 4/8] cmake: Pass to secp256k1 build system only sanitizer flags This change mirrors behavior in the master branch. --- CMakeLists.txt | 6 ++++-- src/CMakeLists.txt | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 88993778398ca..3872c812067a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -322,11 +322,13 @@ target_link_libraries(core_base_interface INTERFACE Threads::Threads ) +add_library(sanitize_interface INTERFACE) +target_link_libraries(core_base_interface INTERFACE sanitize_interface) if(SANITIZERS) # First check if the compiler accepts flags. If an incompatible pair like # -fsanitize=address,thread is used here, this check will fail. This will also # fail if a bad argument is passed, e.g. -fsanitize=undfeined - try_append_cxx_flags("-fsanitize=${SANITIZERS}" TARGET core_base_interface + try_append_cxx_flags("-fsanitize=${SANITIZERS}" TARGET sanitize_interface RESULT_VAR cxx_supports_sanitizers SKIP_LINK ) @@ -353,7 +355,7 @@ if(SANITIZERS) message(FATAL_ERROR "Linker did not accept requested flags, you are missing required libraries.") endif() endif() -target_link_options(core_base_interface INTERFACE ${SANITIZER_LDFLAGS}) +target_link_options(sanitize_interface INTERFACE ${SANITIZER_LDFLAGS}) include(AddBoostIfNeeded) add_boost_if_needed() diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 39477ebeb3751..902b0248cddbe 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -48,9 +48,11 @@ set(SECP256K1_ENABLE_MODULE_ECDH OFF CACHE BOOL "" FORCE) set(SECP256K1_ENABLE_MODULE_RECOVERY ON CACHE BOOL "" FORCE) set(SECP256K1_ECMULT_GEN_KB 86 CACHE STRING "" FORCE) include(GetTargetInterface) -get_target_interface(core_c_flags "" core_base_interface COMPILE_OPTIONS) -set(SECP256K1_LATE_CFLAGS ${core_c_flags} CACHE STRING "" FORCE) -unset(core_c_flags) +# -fsanitize and related flags apply to both C++ and C, +# so we can pass them down to libsecp256k1 as CFLAGS. +get_target_interface(core_sanitizer_cxx_flags "" sanitize_interface COMPILE_OPTIONS) +set(SECP256K1_LATE_CFLAGS ${core_sanitizer_cxx_flags} CACHE STRING "" FORCE) +unset(core_sanitizer_cxx_flags) add_subdirectory(secp256k1 EXCLUDE_FROM_ALL) set_target_properties(secp256k1 PROPERTIES EXPORT_COMPILE_COMMANDS OFF) string(APPEND CMAKE_C_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CFLAGS}") From cdd843a9e703076287bad54bebb800c4507ff753 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 22 Jun 2024 16:00:06 +0100 Subject: [PATCH 5/8] cmake: Override build configurations for libsecp256k1 --- src/CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 902b0248cddbe..9558674fe3f56 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -53,6 +53,15 @@ include(GetTargetInterface) get_target_interface(core_sanitizer_cxx_flags "" sanitize_interface COMPILE_OPTIONS) set(SECP256K1_LATE_CFLAGS ${core_sanitizer_cxx_flags} CACHE STRING "" FORCE) unset(core_sanitizer_cxx_flags) +# We want to build libsecp256k1 with the most tested RelWithDebInfo configuration. +enable_language(C) +foreach(config IN LISTS CMAKE_BUILD_TYPE CMAKE_CONFIGURATION_TYPES) + if(config STREQUAL "") + continue() + endif() + string(TOUPPER "${config}" config) + set(CMAKE_C_FLAGS_${config} "${CMAKE_C_FLAGS_RELWITHDEBINFO}") +endforeach() add_subdirectory(secp256k1 EXCLUDE_FROM_ALL) set_target_properties(secp256k1 PROPERTIES EXPORT_COMPILE_COMMANDS OFF) string(APPEND CMAKE_C_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CFLAGS}") From 70ec2d938616f3a740670ddea216e105bcd28be7 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 25 Jun 2024 10:47:09 +0100 Subject: [PATCH 6/8] cmake: Reorder subdirectories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change ensures that both summaries—secp256k1's and Bitcoin Core's— are displayed without interleaving of the `find_package(Doxygen)` output from the `doc` subdirectory. --- CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3872c812067a8..af07c719a75a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -588,10 +588,11 @@ endif() # - https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833 set(CMAKE_SKIP_BUILD_RPATH TRUE) set(CMAKE_SKIP_INSTALL_RPATH TRUE) -add_subdirectory(src) add_subdirectory(test) add_subdirectory(doc) +add_subdirectory(src) + include(cmake/tests.cmake) include(Maintenance) From bfe7c4d556f5e5a5b8cf4497444c270eaeaf99f3 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 26 Jun 2024 21:32:37 +0100 Subject: [PATCH 7/8] [FIXUP] cmake: Enable tests before adding subdirectories --- CMakeLists.txt | 3 +++ cmake/tests.cmake | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index af07c719a75a5..1dd2a42085e9e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -580,6 +580,9 @@ if(DEFINED ENV{LDFLAGS}) deduplicate_flags(CMAKE_EXE_LINKER_FLAGS) endif() +if(BUILD_TESTS) + enable_testing() +endif() # TODO: The `CMAKE_SKIP_BUILD_RPATH` variable setting can be deleted # in the future after reordering Guix script commands to # perform binary checks after the installation step. diff --git a/cmake/tests.cmake b/cmake/tests.cmake index 0abccf5999f1e..7f597262c82d0 100644 --- a/cmake/tests.cmake +++ b/cmake/tests.cmake @@ -2,8 +2,6 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or https://opensource.org/license/mit/. -include(CTest) - if(TARGET bitcoin-util AND TARGET bitcoin-tx AND PYTHON_COMMAND) add_test(NAME util_test_runner COMMAND ${CMAKE_COMMAND} -E env BITCOINUTIL=$ BITCOINTX=$ ${PYTHON_COMMAND} ${CMAKE_BINARY_DIR}/test/util/test_runner.py From 458d54c57de3f4708a5e1b3e7b6ef1aac9ea4247 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 26 Jun 2024 22:14:16 +0100 Subject: [PATCH 8/8] cmake: Run secp256k1 tests --- src/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9558674fe3f56..74ee36c6054c2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -47,6 +47,11 @@ set(SECP256K1_DISABLE_SHARED ON CACHE BOOL "" FORCE) set(SECP256K1_ENABLE_MODULE_ECDH OFF CACHE BOOL "" FORCE) set(SECP256K1_ENABLE_MODULE_RECOVERY ON CACHE BOOL "" FORCE) set(SECP256K1_ECMULT_GEN_KB 86 CACHE STRING "" FORCE) +set(SECP256K1_BUILD_BENCHMARK OFF CACHE BOOL "" FORCE) +set(SECP256K1_BUILD_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE) +set(SECP256K1_BUILD_EXHAUSTIVE_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE) +set(SECP256K1_BUILD_CTIME_TESTS OFF CACHE BOOL "" FORCE) +set(SECP256K1_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE) include(GetTargetInterface) # -fsanitize and related flags apply to both C++ and C, # so we can pass them down to libsecp256k1 as CFLAGS. @@ -62,7 +67,7 @@ foreach(config IN LISTS CMAKE_BUILD_TYPE CMAKE_CONFIGURATION_TYPES) string(TOUPPER "${config}" config) set(CMAKE_C_FLAGS_${config} "${CMAKE_C_FLAGS_RELWITHDEBINFO}") endforeach() -add_subdirectory(secp256k1 EXCLUDE_FROM_ALL) +add_subdirectory(secp256k1) set_target_properties(secp256k1 PROPERTIES EXPORT_COMPILE_COMMANDS OFF) string(APPEND CMAKE_C_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CFLAGS}")