Skip to content

build: Add SECP256K1_FORCE_HIDDEN_VISIBILITY #1677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

build: Add SECP256K1_FORCE_HIDDEN_VISIBILITY #1677

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions include/secp256k1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,37 @@ typedef int (*secp256k1_nonce_function)(
# endif
#endif
#ifndef SECP256K1_API
/* All cases not captured by the Windows-specific logic. */
# if defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD)
/* Building libsecp256k1 using GCC or compatible. */
# define SECP256K1_API extern __attribute__ ((visibility ("default")))
# else
/* Fall back to standard C's extern. */
# define SECP256K1_API extern
# if defined(__GNUC__) && (__GNUC__ == 4) && defined(SECP256K1_BUILD)
/* Building libsecp256k1 using GCC or compatible on non-Windows. */
# if defined(SECP256K1_FORCE_HIDDEN_VISIBILITY)
/* Hidden visibility is requested explicitly.
*
* Forcing hidden visibility can be useful, e.g., when building a static
* library which is linked into a shared library, and the latter should not
* reexport the libsecp256k1 API. Since this will create an unusable shared
* library, SECP256K1_FORCE_HIDDEN_VISIBILITY should only be used when
* building a static library. (You want to ./configure with --disable-shared
* if using Autotools.)
*
* While visibility is a concept that applies only to shared libraries,
* setting visibility will still make a difference when building a static
* library: the visibility settings will be stored in the static library,
* solely for the potential case that the static library will be linked into
* a shared library. In that case, the stored visibility settings will
* resurface and be honored for the shared library. */
# define SECP256K1_API extern __attribute__ ((visibility("hidden")))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Win32 needs this branch too, no? Otherwise it'll still require -fvisibility=hidden for static libs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe #if defined(SECP256K1_FORCE_HIDDEN_VISIBILITY) could just go at the top, and keep with the #ifndef(SECP256K1_API) pattern below it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Win32 needs this branch too, no? Otherwise it'll still require -fvisibility=hidden for static libs?

I don't think so (but I haven't tested this so far). As far as I understand, gcc defaults to hidden on Windows as soon as it sees an explicit __declspec(dllexport) annotation (and MSVC anyway exports only annotated symbols). At least this is what the libtool manual claims:

It should be noted that the GNU auto-export feature is turned off when an explicit __declspec(dllexport) is seen. The GNU tools do this to not make more symbols visible for projects that have already taken the trouble to decorate symbols.

https://www.gnu.org/software/libtool/manual/html_node/Windows-DLLs.html

Actually, maybe #if defined(SECP256K1_FORCE_HIDDEN_VISIBILITY) could just go at the top, and keep with the #ifndef(SECP256K1_API) pattern below it?

Not sure if I can follow. If you still think we'll need this for Windows, can you suggest/sketch a patch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries.

# else
/* Set default visibility explicit. This is to not break the build for users
* who pass `-fvisibility=hidden` in the expection that this is a
* visibility-aware library. */
# define SECP256K1_API extern __attribute__ ((visibility("default")))
# endif
# endif
#endif
#ifndef SECP256K1_API
/* Fall back to standard C's extern. */
# define SECP256K1_API extern
#endif

/* Warning attributes
* NONNULL is not used if SECP256K1_BUILD is set to avoid the compiler optimizing out
Expand Down