From ae9db95ceaa2605138fac9c237c640acea3f3bd6 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 30 Jun 2023 09:26:48 +0100 Subject: [PATCH 1/5] build: Introduce `SECP256K1_STATIC` macro for Windows users It is a non-Libtool-specific way to explicitly specify the user's intention to consume a static `libseck256k1`. This change allows to get rid of MSVC linker warnings LNK4217 and LNK4286. Also, it makes possible to merge the `SECP256K1_API` and `SECP256K1_API_VAR` into one. --- CHANGELOG.md | 3 +++ Makefile.am | 6 +++--- configure.ac | 6 ------ examples/CMakeLists.txt | 3 --- include/secp256k1.h | 8 ++++---- src/CMakeLists.txt | 1 + 6 files changed, 11 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7f2053ea756c..dacb0a3f48c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Document `doc/ellswift.md` which explains the mathematical background of the scheme. - The [paper](https://eprint.iacr.org/2022/759) on which the scheme is based. +#### Changed + - When consuming libsecp256k1 as a static library on Windows, the user must now define the `SECP256K1_STATIC` macro before including `secp256k1.h`. + ## [0.3.2] - 2023-05-13 We strongly recommend updating to 0.3.2 if you use or plan to use GCC >=13 to compile libsecp256k1. When in doubt, check the GCC version using `gcc -v`. diff --git a/Makefile.am b/Makefile.am index ee14ac45095eb..32bc729a41a29 100644 --- a/Makefile.am +++ b/Makefile.am @@ -153,7 +153,7 @@ endif if USE_EXAMPLES noinst_PROGRAMS += ecdsa_example ecdsa_example_SOURCES = examples/ecdsa.c -ecdsa_example_CPPFLAGS = -I$(top_srcdir)/include +ecdsa_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC ecdsa_example_LDADD = libsecp256k1.la ecdsa_example_LDFLAGS = -static if BUILD_WINDOWS @@ -163,7 +163,7 @@ TESTS += ecdsa_example if ENABLE_MODULE_ECDH noinst_PROGRAMS += ecdh_example ecdh_example_SOURCES = examples/ecdh.c -ecdh_example_CPPFLAGS = -I$(top_srcdir)/include +ecdh_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC ecdh_example_LDADD = libsecp256k1.la ecdh_example_LDFLAGS = -static if BUILD_WINDOWS @@ -174,7 +174,7 @@ endif if ENABLE_MODULE_SCHNORRSIG noinst_PROGRAMS += schnorr_example schnorr_example_SOURCES = examples/schnorr.c -schnorr_example_CPPFLAGS = -I$(top_srcdir)/include +schnorr_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC schnorr_example_LDADD = libsecp256k1.la schnorr_example_LDFLAGS = -static if BUILD_WINDOWS diff --git a/configure.ac b/configure.ac index 82cf95132d6f1..a502d1304a7b2 100644 --- a/configure.ac +++ b/configure.ac @@ -127,12 +127,6 @@ AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [ SECP_TRY_APPEND_CFLAGS([-wd4267], $1) # Disable warning C4267 "'var' : conversion from 'size_t' to 'type', possible loss of data". # Eliminate deprecation warnings for the older, less secure functions. CPPFLAGS="-D_CRT_SECURE_NO_WARNINGS $CPPFLAGS" - # We pass -ignore:4217 to the MSVC linker to suppress warning 4217 when - # importing variables from a statically linked secp256k1. - # (See the libtool manual, section "Windows DLLs" for background.) - # Unfortunately, libtool tries to be too clever and strips "-Xlinker arg" - # into "arg", so this will be " -Xlinker -ignore:4217" after stripping. - LDFLAGS="-Xlinker -Xlinker -Xlinker -ignore:4217 $LDFLAGS" fi ]) SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS) diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index e095b7f84fd40..e2ea47300854b 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -6,9 +6,6 @@ target_link_libraries(example INTERFACE secp256k1 $<$:bcrypt> ) -if(NOT BUILD_SHARED_LIBS AND MSVC) - target_link_options(example INTERFACE /IGNORE:4217) -endif() add_executable(ecdsa_example ecdsa.c) target_link_libraries(ecdsa_example example) diff --git a/include/secp256k1.h b/include/secp256k1.h index 01d18cff09c89..4429bd34b5f94 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -141,10 +141,10 @@ typedef int (*secp256k1_nonce_function)( # define SECP256K1_API __declspec (dllexport) # define SECP256K1_API_VAR extern __declspec (dllexport) # endif -# elif defined _MSC_VER -# define SECP256K1_API -# define SECP256K1_API_VAR extern __declspec (dllimport) -# elif defined DLL_EXPORT + /* The user must define SECP256K1_STATIC when consuming libsecp256k1 as a static + * library on Windows. */ +# elif !defined(SECP256K1_STATIC) + /* Consuming libsecp256k1 as a DLL. */ # define SECP256K1_API __declspec (dllimport) # define SECP256K1_API_VAR extern __declspec (dllimport) # endif diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0bba19982ad53..fd7c48fcf17dc 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -24,6 +24,7 @@ endif() # This matches libtool's usage of DLL_EXPORT if(WIN32) set_target_properties(secp256k1 PROPERTIES DEFINE_SYMBOL "DLL_EXPORT") + target_compile_definitions(secp256k1 INTERFACE $<$>:SECP256K1_STATIC>) endif() # Object libs don't know if they're being built for a shared or static lib. From 9f1b1904a358e4ce7248c6542e8c7ac143ba0e3f Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 29 Jun 2023 17:07:50 +0100 Subject: [PATCH 2/5] refactor: Replace `SECP256K1_API_VAR` with `SECP256K1_API` --- include/secp256k1.h | 20 ++++++++------------ include/secp256k1_ecdh.h | 4 ++-- include/secp256k1_ellswift.h | 4 ++-- include/secp256k1_schnorrsig.h | 2 +- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 4429bd34b5f94..9370906c31638 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -138,24 +138,20 @@ typedef int (*secp256k1_nonce_function)( #if defined(_WIN32) # ifdef SECP256K1_BUILD # ifdef DLL_EXPORT -# define SECP256K1_API __declspec (dllexport) -# define SECP256K1_API_VAR extern __declspec (dllexport) +# define SECP256K1_API extern __declspec (dllexport) # endif /* The user must define SECP256K1_STATIC when consuming libsecp256k1 as a static * library on Windows. */ # elif !defined(SECP256K1_STATIC) /* Consuming libsecp256k1 as a DLL. */ -# define SECP256K1_API __declspec (dllimport) -# define SECP256K1_API_VAR extern __declspec (dllimport) +# define SECP256K1_API extern __declspec (dllimport) # endif #endif #ifndef SECP256K1_API # if defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD) -# define SECP256K1_API __attribute__ ((visibility ("default"))) -# define SECP256K1_API_VAR extern __attribute__ ((visibility ("default"))) +# define SECP256K1_API extern __attribute__ ((visibility ("default"))) # else -# define SECP256K1_API -# define SECP256K1_API_VAR extern +# define SECP256K1_API extern # endif #endif @@ -227,10 +223,10 @@ typedef int (*secp256k1_nonce_function)( * * It is highly recommended to call secp256k1_selftest before using this context. */ -SECP256K1_API_VAR const secp256k1_context *secp256k1_context_static; +SECP256K1_API const secp256k1_context *secp256k1_context_static; /** Deprecated alias for secp256k1_context_static. */ -SECP256K1_API_VAR const secp256k1_context *secp256k1_context_no_precomp +SECP256K1_API const secp256k1_context *secp256k1_context_no_precomp SECP256K1_DEPRECATED("Use secp256k1_context_static instead"); /** Perform basic self tests (to be used in conjunction with secp256k1_context_static) @@ -627,10 +623,10 @@ SECP256K1_API int secp256k1_ecdsa_signature_normalize( * If a data pointer is passed, it is assumed to be a pointer to 32 bytes of * extra entropy. */ -SECP256K1_API_VAR const secp256k1_nonce_function secp256k1_nonce_function_rfc6979; +SECP256K1_API const secp256k1_nonce_function secp256k1_nonce_function_rfc6979; /** A default safe nonce generation function (currently equal to secp256k1_nonce_function_rfc6979). */ -SECP256K1_API_VAR const secp256k1_nonce_function secp256k1_nonce_function_default; +SECP256K1_API const secp256k1_nonce_function secp256k1_nonce_function_default; /** Create an ECDSA signature. * diff --git a/include/secp256k1_ecdh.h b/include/secp256k1_ecdh.h index 837ae2abe5052..515e17429986b 100644 --- a/include/secp256k1_ecdh.h +++ b/include/secp256k1_ecdh.h @@ -27,11 +27,11 @@ typedef int (*secp256k1_ecdh_hash_function)( /** An implementation of SHA256 hash function that applies to compressed public key. * Populates the output parameter with 32 bytes. */ -SECP256K1_API_VAR const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_sha256; +SECP256K1_API const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_sha256; /** A default ECDH hash function (currently equal to secp256k1_ecdh_hash_function_sha256). * Populates the output parameter with 32 bytes. */ -SECP256K1_API_VAR const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_default; +SECP256K1_API const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_default; /** Compute an EC Diffie-Hellman secret in constant time * diff --git a/include/secp256k1_ellswift.h b/include/secp256k1_ellswift.h index c0b898713ceaf..2b7e403c497fb 100644 --- a/include/secp256k1_ellswift.h +++ b/include/secp256k1_ellswift.h @@ -72,7 +72,7 @@ typedef int (*secp256k1_ellswift_xdh_hash_function)( /** An implementation of an secp256k1_ellswift_xdh_hash_function which uses * SHA256(prefix64 || ell_a64 || ell_b64 || x32), where prefix64 is the 64-byte * array pointed to by data. */ -SECP256K1_API_VAR const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_prefix; +SECP256K1_API const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_prefix; /** An implementation of an secp256k1_ellswift_xdh_hash_function compatible with * BIP324. It returns H_tag(ell_a64 || ell_b64 || x32), where H_tag is the @@ -80,7 +80,7 @@ SECP256K1_API_VAR const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_ * to secp256k1_ellswift_xdh_hash_function_prefix with prefix64 set to * SHA256("bip324_ellswift_xonly_ecdh")||SHA256("bip324_ellswift_xonly_ecdh"). * The data argument is ignored. */ -SECP256K1_API_VAR const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_bip324; +SECP256K1_API const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_bip324; /** Construct a 64-byte ElligatorSwift encoding of a given pubkey. * diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index 1ee665fd19acb..26358533f67fb 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -61,7 +61,7 @@ typedef int (*secp256k1_nonce_function_hardened)( * Therefore, to create BIP-340 compliant signatures, algo must be set to * "BIP0340/nonce" and algolen to 13. */ -SECP256K1_API_VAR const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; +SECP256K1_API const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; /** Data structure that contains additional arguments for schnorrsig_sign_custom. * From 0196e8ade16e2b2d8efadac01d8520205553ee39 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 29 Jun 2023 17:14:03 +0100 Subject: [PATCH 3/5] build: Introduce `SECP256k1_DLL_EXPORT` macro This change provides a way to build a shared library that is not tired to the Libtool-specific `DLL_EXPORT` macro. --- include/secp256k1.h | 8 +++++--- src/CMakeLists.txt | 5 ++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 9370906c31638..88b68df7b0e7d 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -134,10 +134,12 @@ typedef int (*secp256k1_nonce_function)( #endif /* Symbol visibility. See https://gcc.gnu.org/wiki/Visibility */ -/* DLL_EXPORT is defined internally for shared builds */ #if defined(_WIN32) -# ifdef SECP256K1_BUILD -# ifdef DLL_EXPORT +# if defined(SECP256K1_BUILD) +# if defined(DLL_EXPORT) || defined(SECP256K1_DLL_EXPORT) + /* Building libsecp256k1 as a DLL. + * 1. If using Libtool, it defines DLL_EXPORT automatically. + * 2. In other cases, SECP256K1_DLL_EXPORT must be defined. */ # define SECP256K1_API extern __declspec (dllexport) # endif /* The user must define SECP256K1_STATIC when consuming libsecp256k1 as a static diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index fd7c48fcf17dc..b305751b08da7 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -20,10 +20,9 @@ if(SECP256K1_ASM STREQUAL "arm32") target_link_libraries(secp256k1_asm INTERFACE secp256k1_asm_arm) endif() -# Define our export symbol only for Win32 and only for shared libs. -# This matches libtool's usage of DLL_EXPORT if(WIN32) - set_target_properties(secp256k1 PROPERTIES DEFINE_SYMBOL "DLL_EXPORT") + # Define our export symbol only for shared libs. + set_target_properties(secp256k1 PROPERTIES DEFINE_SYMBOL SECP256K1_DLL_EXPORT) target_compile_definitions(secp256k1 INTERFACE $<$>:SECP256K1_STATIC>) endif() From 020bf69a44ba700624d09de0c18ceb867369d24e Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 27 Jun 2023 15:22:44 +0200 Subject: [PATCH 4/5] build: Add extensive docs on visibility issues --- include/secp256k1.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 88b68df7b0e7d..ce2145ae3d276 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -133,8 +133,14 @@ typedef int (*secp256k1_nonce_function)( # define SECP256K1_NO_BUILD #endif -/* Symbol visibility. See https://gcc.gnu.org/wiki/Visibility */ +/* Symbol visibility. */ #if defined(_WIN32) + /* GCC for Windows (e.g., MinGW) accepts the __declspec syntax + * for MSVC compatibility. A __declspec declaration implies (but is not + * exactly equivalent to) __attribute__ ((visibility("default"))), and so we + * actually want __declspec even on GCC, see "Microsoft Windows Function + * Attributes" in the GCC manual and the recommendations in + * https://gcc.gnu.org/wiki/Visibility. */ # if defined(SECP256K1_BUILD) # if defined(DLL_EXPORT) || defined(SECP256K1_DLL_EXPORT) /* Building libsecp256k1 as a DLL. @@ -151,8 +157,10 @@ typedef int (*secp256k1_nonce_function)( #endif #ifndef SECP256K1_API # if defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD) + /* Building libsecp256k1 on non-Windows using GCC or compatible. */ # define SECP256K1_API extern __attribute__ ((visibility ("default"))) # else + /* All cases not captured above. */ # define SECP256K1_API extern # endif #endif From c6cd2b15a007ad0a2d5c4656ae641ba442d8b2fe Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 30 Jun 2023 09:32:57 +0100 Subject: [PATCH 5/5] ci: Add task for static library on Windows + CMake --- .cirrus.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index cec5ad8fffb4a..835b6b227df44 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -383,12 +383,17 @@ task: # Ignore MSBuild warning MSB8029. # See: https://learn.microsoft.com/en-us/visualstudio/msbuild/errors/msb8029?view=vs-2022 IgnoreWarnIntDirInTempDetected: 'true' + matrix: + - env: + BUILD_SHARED_LIBS: ON + - env: + BUILD_SHARED_LIBS: OFF git_show_script: # Print commit to allow reproducing the job outside of CI. - git show --no-patch configure_script: - '%x64_NATIVE_TOOLS%' - - cmake -E env CFLAGS="/WX" cmake -G "Visual Studio 17 2022" -A x64 -S . -B build -DSECP256K1_ENABLE_MODULE_RECOVERY=ON -DSECP256K1_BUILD_EXAMPLES=ON + - cmake -E env CFLAGS="/WX" cmake -A x64 -B build -DSECP256K1_ENABLE_MODULE_RECOVERY=ON -DSECP256K1_BUILD_EXAMPLES=ON -DBUILD_SHARED_LIBS=%BUILD_SHARED_LIBS% build_script: - '%x64_NATIVE_TOOLS%' - cmake --build build --config RelWithDebInfo -- -property:UseMultiToolTask=true;CL_MPcount=5