frost/secp256k1-zkp
3
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge bitcoin-core/secp256k1#1696: build: Refactor visibility logic and add override

Browse Source 
c82d84bb86 build: add CMake option for disabling symbol visibility attributes (Cory Fields)
ce7923874f build: Add SECP256K1_NO_API_VISIBILITY_ATTRIBUTES (Tim Ruffing)
e5297f6d79 build: Refactor visibility logic (Tim Ruffing)

Pull request description:

  This is less invasive than #1695. The latter might be the right thing in a new library (and then we'd probably not support autotools in the first place), but any semantic change to this code has the potential to create news bug, or at least breakages for downstream users.

  This is different from #1677 in that it does not set `hidden` explicitly. I agree with the comment in #1677 that setting `hidden` violates the principle of least surprise.

  So this similar in spirit to #1674. So I wonder if this should also include
  3eef7362c4. I'd say no, `fvisibility` should then set by the user. But can you, in CMake, set `CMAKE_C_VISIBILITY_PRESET` from a parent project?

ACKs for top commit:
  hebasto:
    ACK c82d84bb86, I have reviewed the code and it looks OK.

Tree-SHA512: dad36c32a108d813e8d4e1849260af43f79a9aa8fbfb9a42b07d737e0467924a511110df0a2c6761539a1587b617a1b11123610a3db9d4cdf2b985dfb3eb21da
This commit is contained in:
merge-script
2025-07-21 14:55:10 +02:00
parent 73a695958a c82d84bb86
commit 7c3380423c
3 changed files with 57 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
CMakeLists.txt
View File

@@ -42,6 +42,8 @@ endif()
option(SECP256K1_INSTALL "Enable installation." ${PROJECT_IS_TOP_LEVEL}) option(SECP256K1_INSTALL "Enable installation." ${PROJECT_IS_TOP_LEVEL})
option(SECP256K1_ENABLE_API_VISIBILITY_ATTRIBUTES "Enable visibility attributes in the API." ON)
## Modules ## Modules
# We declare all options before processing them, to make sure we can express # We declare all options before processing them, to make sure we can express
@@ -312,6 +314,7 @@ else()
set(cross_status "FALSE") set(cross_status "FALSE")
endif() endif()
message("Cross compiling ....................... ${cross_status}") message("Cross compiling ....................... ${cross_status}")
message("API visibility attributes ............. ${SECP256K1_ENABLE_API_VISIBILITY_ATTRIBUTES}")
message("Valgrind .............................. ${SECP256K1_VALGRIND}") message("Valgrind .............................. ${SECP256K1_VALGRIND}")
get_directory_property(definitions COMPILE_DEFINITIONS) get_directory_property(definitions COMPILE_DEFINITIONS)
string(REPLACE ";" " " definitions "${definitions}") string(REPLACE ";" " " definitions "${definitions}")

88
include/secp256k1.h
View File

@@ -121,45 +121,57 @@ typedef int (*secp256k1_nonce_function)(
#endif #endif
/* Symbol visibility. */ /* Symbol visibility. */
#if defined(_WIN32) #if !defined(SECP256K1_API) && defined(SECP256K1_NO_API_VISIBILITY_ATTRIBUTES)
/* GCC for Windows (e.g., MinGW) accepts the __declspec syntax /* The user has requested that we don't specify visibility attributes in
* for MSVC compatibility. A __declspec declaration implies (but is not * the public API.
* exactly equivalent to) __attribute__ ((visibility("default"))), and so we *
* actually want __declspec even on GCC, see "Microsoft Windows Function * Since all our non-API declarations use the static qualifier, this means
* Attributes" in the GCC manual and the recommendations in * that the user can use -fvisibility=<value> to set the visibility of the
* https://gcc.gnu.org/wiki/Visibility. */ * API symbols. For instance, -fvisibility=hidden can be useful *even for
# if defined(SECP256K1_BUILD) * the API symbols*, e.g., when building a static library which is linked
# if defined(DLL_EXPORT) || defined(SECP256K1_DLL_EXPORT) * into a shared library, and the latter should not re-export the
/* Building libsecp256k1 as a DLL. * libsecp256k1 API.
* 1. If using Libtool, it defines DLL_EXPORT automatically. *
* 2. In other cases, SECP256K1_DLL_EXPORT must be defined. */ * While visibility is a concept that applies only to shared libraries,
# define SECP256K1_API extern __declspec (dllexport) * setting visibility will still make a difference when building a static
# else * library: the visibility settings will be stored in the static library,
/* Building libsecp256k1 as a static library on Windows. * solely for the potential case that the static library will be linked into
* No declspec is needed, and so we would want the non-Windows-specific * a shared library. In that case, the stored visibility settings will
* logic below take care of this case. However, this may result in setting * resurface and be honored for the shared library. */
* __attribute__ ((visibility("default"))), which is supposed to be a noop # define SECP256K1_API extern
* on Windows but may trigger warnings when compiling with -flto due to a
* bug in GCC, see
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116478 . */
# define SECP256K1_API extern
# 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 extern __declspec (dllimport)
# endif
#endif #endif
#ifndef SECP256K1_API #if !defined(SECP256K1_API)
/* All cases not captured by the Windows-specific logic. */ # if defined(SECP256K1_BUILD)
# if defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD) /* On Windows, assume a shared library only if explicitly requested.
/* Building libsecp256k1 using GCC or compatible. */ * 1. If using Libtool, it defines DLL_EXPORT automatically.
# define SECP256K1_API extern __attribute__ ((visibility ("default"))) * 2. In other cases, SECP256K1_DLL_EXPORT must be defined. */
# else # if defined(_WIN32) && (defined(SECP256K1_DLL_EXPORT) || defined(DLL_EXPORT))
/* Fall back to standard C's extern. */ /* GCC for Windows (e.g., MinGW) accepts the __declspec syntax for
# define SECP256K1_API extern * MSVC compatibility. A __declspec declaration implies (but is not
# endif * 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 . */
# define SECP256K1_API extern __declspec(dllexport)
/* Avoid __attribute__ ((visibility("default"))) on Windows to get rid
* of warnings when compiling with -flto due to a bug in GCC, see
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116478 . */
# elif !defined(_WIN32) && defined (__GNUC__) && (__GNUC__ >= 4)
# define SECP256K1_API extern __attribute__ ((visibility("default")))
# else
# define SECP256K1_API extern
# endif
# else
/* On Windows, SECP256K1_STATIC must be defined when consuming
* libsecp256k1 as a static library. Note that SECP256K1_STATIC is a
* "consumer-only" macro, and it has no meaning when building
* libsecp256k1. */
# if defined(_WIN32) && !defined(SECP256K1_STATIC)
# define SECP256K1_API extern __declspec(dllimport)
# else
# define SECP256K1_API extern
# endif
# endif
#endif #endif
/* Warning attributes /* Warning attributes

4
src/CMakeLists.txt
View File

@@ -54,6 +54,10 @@ add_library(secp256k1_precomputed OBJECT EXCLUDE_FROM_ALL
# from being exported. # from being exported.
target_sources(secp256k1 PRIVATE secp256k1.c $<TARGET_OBJECTS:secp256k1_precomputed>) target_sources(secp256k1 PRIVATE secp256k1.c $<TARGET_OBJECTS:secp256k1_precomputed>)
if(NOT SECP256K1_ENABLE_API_VISIBILITY_ATTRIBUTES)
target_compile_definitions(secp256k1 PRIVATE SECP256K1_NO_API_VISIBILITY_ATTRIBUTES)
endif()
# Create a helper lib that parent projects can use to link secp256k1 into a # Create a helper lib that parent projects can use to link secp256k1 into a
# static lib. # static lib.
add_library(secp256k1_objs INTERFACE) add_library(secp256k1_objs INTERFACE)