Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1367: build: Improvements to symbol visi…
Browse files Browse the repository at this point in the history
…bility logic on Windows (attempt 3)

c6cd2b1 ci: Add task for static library on Windows + CMake (Hennadii Stepanov)
020bf69 build: Add extensive docs on visibility issues (Tim Ruffing)
0196e8a build: Introduce `SECP256k1_DLL_EXPORT` macro (Hennadii Stepanov)
9f1b190 refactor: Replace `SECP256K1_API_VAR` with `SECP256K1_API` (Hennadii Stepanov)
ae9db95 build: Introduce `SECP256K1_STATIC` macro for Windows users (Hennadii Stepanov)

Pull request description:

  Previous attempts:
  - bitcoin-core/secp256k1#1346
  - bitcoin-core/secp256k1#1362

  The result is as follows:
  1. Simple, concise and extensively documented code.
  2. Explicitly documented use cases with no ambiguities.
  3. No workarounds for linker warnings.
  4. Solves one item in bitcoin-core/secp256k1#1235.

ACKs for top commit:
  real-or-random:
    utACK c6cd2b1

Tree-SHA512: d58694452d630aefbd047916033249891bc726b7475433aaaa7c3ea2a07ded8f185a598385b67c2ee3440ec5904ff9d9452c97b0961d84dcb2eb2cf46caa171e
  • Loading branch information
real-or-random committed Jul 3, 2023
2 parents 0aacf64 + c6cd2b1 commit 9e6d1b0
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 41 deletions.
7 changes: 6 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,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
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
6 changes: 3 additions & 3 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 0 additions & 6 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions examples/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ target_link_libraries(example INTERFACE
secp256k1
$<$<PLATFORM_ID:Windows>: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)
Expand Down
46 changes: 26 additions & 20 deletions include/secp256k1.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,29 +133,35 @@ 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)
# ifdef SECP256K1_BUILD
# ifdef DLL_EXPORT
# define SECP256K1_API __declspec (dllexport)
# define SECP256K1_API_VAR extern __declspec (dllexport)
/* 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.
* 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
# elif defined _MSC_VER
# define SECP256K1_API
# define SECP256K1_API_VAR extern __declspec (dllimport)
# elif defined DLL_EXPORT
# define SECP256K1_API __declspec (dllimport)
# define SECP256K1_API_VAR extern __declspec (dllimport)
/* 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 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")))
/* Building libsecp256k1 on non-Windows using GCC or compatible. */
# define SECP256K1_API extern __attribute__ ((visibility ("default")))
# else
# define SECP256K1_API
# define SECP256K1_API_VAR extern
/* All cases not captured above. */
# define SECP256K1_API extern
# endif
#endif

Expand Down Expand Up @@ -227,10 +233,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)
Expand Down Expand Up @@ -627,10 +633,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.
*
Expand Down
4 changes: 2 additions & 2 deletions include/secp256k1_ecdh.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
4 changes: 2 additions & 2 deletions include/secp256k1_ellswift.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ 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
* BIP340 tagged hash function with tag "bip324_ellswift_xonly_ecdh". Equivalent
* 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.
*
Expand Down
2 changes: 1 addition & 1 deletion include/secp256k1_schnorrsig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
6 changes: 3 additions & 3 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ 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 $<$<NOT:$<BOOL:${BUILD_SHARED_LIBS}>>:SECP256K1_STATIC>)
endif()

# Object libs don't know if they're being built for a shared or static lib.
Expand Down

0 comments on commit 9e6d1b0

Please sign in to comment.