From 85aecf463c1d5c7167152d0af8f7e32a7ec98540 Mon Sep 17 00:00:00 2001 From: Tim Ruffing <crypto@timruffing.de> Date: Tue, 27 Jun 2023 15:04:13 +0200 Subject: [PATCH 1/5] build: Use dllexport also for Cygwin builds As recommended by https://gcc.gnu.org/wiki/Visibility --- include/secp256k1.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 01d18cff09..56e9b580bf 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -135,7 +135,7 @@ typedef int (*secp256k1_nonce_function)( /* Symbol visibility. See https://gcc.gnu.org/wiki/Visibility */ /* DLL_EXPORT is defined internally for shared builds */ -#if defined(_WIN32) +#if defined(_WIN32) || defined(__CYGWIN__) # ifdef SECP256K1_BUILD # ifdef DLL_EXPORT # define SECP256K1_API __declspec (dllexport) From 297c85ac06fdc612049a0c6236b98850eeb56acd Mon Sep 17 00:00:00 2001 From: Tim Ruffing <crypto@timruffing.de> Date: Tue, 27 Jun 2023 15:20:06 +0200 Subject: [PATCH 2/5] build: Add #ifdefs for requesting import as DLL vs static explicitly --- include/secp256k1.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/secp256k1.h b/include/secp256k1.h index 56e9b580bf..62ec74e99c 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -136,11 +136,20 @@ typedef int (*secp256k1_nonce_function)( /* Symbol visibility. See https://gcc.gnu.org/wiki/Visibility */ /* DLL_EXPORT is defined internally for shared builds */ #if defined(_WIN32) || defined(__CYGWIN__) +# if defined(SECP256K1_STATICLIB) && defined(SECP256K1_DLL) +# error "At most one of SECP256K1_STATICLIB and SECP256K1_DLL must be defined." +# endif # ifdef SECP256K1_BUILD # ifdef DLL_EXPORT # define SECP256K1_API __declspec (dllexport) # define SECP256K1_API_VAR extern __declspec (dllexport) # endif +# elif defined(SECP256K1_STATICLIB) +# define SECP256K1_API +# define SECP256K1_API_VAR extern +# elif defined(SECP256K1_DLL) +# define SECP256K1_API __declspec (dllimport) +# define SECP256K1_API_VAR extern __declspec (dllimport) # elif defined _MSC_VER # define SECP256K1_API # define SECP256K1_API_VAR extern __declspec (dllimport) From d2117f08b27a1b1451733e7b97695f0ccd272ac4 Mon Sep 17 00:00:00 2001 From: Tim Ruffing <crypto@timruffing.de> Date: Tue, 27 Jun 2023 15:22:44 +0200 Subject: [PATCH 3/5] build: Add extensive docs on visibility issues --- include/secp256k1.h | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 62ec74e99c..4e0c04ca21 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -133,36 +133,56 @@ typedef int (*secp256k1_nonce_function)( # define SECP256K1_NO_BUILD #endif -/* Symbol visibility. See https://gcc.gnu.org/wiki/Visibility */ -/* DLL_EXPORT is defined internally for shared builds */ +/* Symbol visibility. */ #if defined(_WIN32) || defined(__CYGWIN__) # if defined(SECP256K1_STATICLIB) && defined(SECP256K1_DLL) # error "At most one of SECP256K1_STATICLIB and SECP256K1_DLL must be defined." # endif -# ifdef SECP256K1_BUILD -# ifdef DLL_EXPORT + /* GCC for Windows (e.g., MinGW) and for Cygwin accept 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) + /* Building libsecp256k1 as a DLL. (DLL_EXPORT is a libtool convention.) */ # define SECP256K1_API __declspec (dllexport) # define SECP256K1_API_VAR extern __declspec (dllexport) # endif # elif defined(SECP256K1_STATICLIB) + /* Linking against static libsecp256k1 requested explicitly. */ # define SECP256K1_API # define SECP256K1_API_VAR extern # elif defined(SECP256K1_DLL) + /* Linking against a libsecp256k1 DLL requested explicitly. */ # define SECP256K1_API __declspec (dllimport) # define SECP256K1_API_VAR extern __declspec (dllimport) -# elif defined _MSC_VER +# elif defined(_MSC_VER) + /* No method requested explicitly. The following works on MSVC for both + * static and dynamic linking, as long as if at least one function is + * imported (i.e., not only variables are imported), which should be the case + * for any meaningful program that uses the libsecp256k1 API. The drawback of + * the following is that it may provoke linker warnings LNK4217 and LNK4286. + * See "Windows DLLs" in the libtool manual. */ # define SECP256K1_API # define SECP256K1_API_VAR extern __declspec (dllimport) -# elif defined DLL_EXPORT +# elif defined(DLL_EXPORT) + /* No method requested explicitly and we're not on MSVC. We make an educated + * guess based on libtool's DLL_EXPORT convention: If the importing program + * is itself a DLL, then it is likely that it also wants to consume + * libsecp256k1 as a DLL. See "Windows DLLs" in the libtool manual. */ # define SECP256K1_API __declspec (dllimport) # define SECP256K1_API_VAR extern __declspec (dllimport) # endif #endif #ifndef SECP256K1_API # if defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD) + /* Building libsecp256k1 on non-Windows using GCC or compatible. */ # define SECP256K1_API __attribute__ ((visibility ("default"))) # define SECP256K1_API_VAR extern __attribute__ ((visibility ("default"))) # else + /* All cases not captured above. */ # define SECP256K1_API # define SECP256K1_API_VAR extern # endif From 69918b823f8679c59a4eae78b191c041758a03bf Mon Sep 17 00:00:00 2001 From: Tim Ruffing <crypto@timruffing.de> Date: Wed, 28 Jun 2023 11:29:30 +0200 Subject: [PATCH 4/5] build: Accept SECP256K1_DLL in addition to DLL_EXPORT in lib builds Addresses one item in #1235. --- include/secp256k1.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 4e0c04ca21..cd9f3766d2 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -145,7 +145,7 @@ typedef int (*secp256k1_nonce_function)( * Attributes" in the GCC manual and the recommendations in * https://gcc.gnu.org/wiki/Visibility. */ # if defined(SECP256K1_BUILD) -# if defined(DLL_EXPORT) +# if defined(SECP256K1_DLL) || defined(DLL_EXPORT) /* Building libsecp256k1 as a DLL. (DLL_EXPORT is a libtool convention.) */ # define SECP256K1_API __declspec (dllexport) # define SECP256K1_API_VAR extern __declspec (dllexport) From 5267c8b5ad2b1fb3879074ac6a5b01a250945685 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:49:19 +0100 Subject: [PATCH 5/5] build: Use `SECP256K1_STATICLIB` macro instead of warning suppressions This makes uses of the freshly introduced `SECP256K1_STATICLIB` macro instead of ignoring MSVC linker warnings LNK4217 and LNK4286. Co-authored-by: Tim Ruffing <crypto@timruffing.de> --- Makefile.am | 6 +++--- configure.ac | 6 ------ examples/CMakeLists.txt | 2 +- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Makefile.am b/Makefile.am index ee14ac4509..a30bad81f0 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_STATICLIB 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_STATICLIB 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_STATICLIB schnorr_example_LDADD = libsecp256k1.la schnorr_example_LDFLAGS = -static if BUILD_WINDOWS diff --git a/configure.ac b/configure.ac index 82cf95132d..a502d1304a 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 e095b7f84f..a314c1787c 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -7,7 +7,7 @@ target_link_libraries(example INTERFACE $<$<PLATFORM_ID:Windows>:bcrypt> ) if(NOT BUILD_SHARED_LIBS AND MSVC) - target_link_options(example INTERFACE /IGNORE:4217) + target_compile_definitions(example INTERFACE SECP256K1_STATICLIB) endif() add_executable(ecdsa_example ecdsa.c)