In certain cases, we export too many symbols:
CFLAGS=-fvisibility=default, there’s a few exported variables : secp256k1_ecmult_gen_prec_table, secp256k1_pre_g, secp256k1_pre_g_128.s assembly files are visible but none should be visible (solved by #1242)
- With
CFLAGS=-fvisibility=default, there’s a few exported variables :secp256k1_ecmult_gen_prec_table,secp256k1_pre_g,secp256k1_pre_g_128
The reason is that these three variables are not in the main translation unit, in which they’re merely declared extern. (We cannot simply add static, because that would imply internal linkage which conflicts which the external linkage that we need due to the use of multiple compilation units.)
There’s no generic C language feature to fix this. But we can fix this in both GCC and clang by any of the following (in descending order of my personal preference).
__attribute__ ((visibility ("hidden"))) to the extern declarationsextern declarations in #pragma GCC visibility push(hidden) … #pragma GCC visibility pop.
-Wl,--exclude-libs,libsecp256k1_precomputed.a to LDFLAGS but that’s kind of pointless to cover the case of default visibility, which typically occurs when the user invokes the compiler manually without even caring about -fvisibility.In any case, -fvisibility=hidden would not be strictly necessary on the command line anymore (except for rather exotic compilers such as icc which understand it but not the attribute or the #pragma. This would then help manual builds of shared libraries.
- With
CFLAGS=-fvisibility=default, there’s a few exported variables :secp256k1_ecmult_gen_prec_table,secp256k1_pre_g,secp256k1_pre_g_128The reason is that these three variables are not in the main translation unit, in which they’re merely declared
extern. (We cannot simply addstatic, because that would imply internal linkage which conflicts which the external linkage that we need due to the use of multiple compilation units.)There’s no generic C language feature to fix this. But we can fix this in both GCC and clang by any of the following (in descending order of my personal preference).
1. Add `__attribute__ ((visibility ("hidden")))` to the `extern` declarations 2. Wrap the `extern` declarations in `#pragma GCC visibility push(hidden)` ... `#pragma GCC visibility pop`.
I’m afraid you’re chasing after some ghosts here because as far as I can tell from my tests it’s perfectly possible to use the export keyword without forcing default visibility despite what some of the docs say.
Notice that these exports aren’t present in the linux shared libraries, only mingw.
The problem as I see it is that mingw is currently using __attribute__ ((visibility ("default"))) to mark its external symbols. However the linker machinery seems to only be activated by __declspec (dllexport). I had always kinda assumed they’re equivalent, but minimal test-cases show that the differences are exactly our problems :)
For me, making that switch (along with #1242) solves all current export problems that I’m aware of.
0diff --git a/include/secp256k1.h b/include/secp256k1.h
1index 325f35eb..ed5bbc9a 100644
2--- a/include/secp256k1.h
3+++ b/include/secp256k1.h
4@@ -148,3 +148,3 @@ typedef int (*secp256k1_nonce_function)(
5 /* Symbol visibility. See libtool manual, section "Windows DLLs". */
6-#if defined(_WIN32) && !defined(__GNUC__)
7+#if defined(_WIN32)
8 # ifdef SECP256K1_BUILD
(Note that I haven’t tested for Darwin though, it’s possible that export visibility is different there)
Hmm, it definitely does for me. The difference is more clear with objdump -p.
Before:
objdump -p .libs/libsecp256k1-2.dll:
... ... [ 60] secp256k1_xonly_pubkey_serialize [ 61] secp256k1_xonly_pubkey_tweak_add [ 62] secp256k1_xonly_pubkey_tweak_add_check
After the patch:
objdump -p .libs/libsecp256k1-2.dll:
... ... [ 57] secp256k1_xonly_pubkey_serialize [ 58] secp256k1_xonly_pubkey_tweak_add [ 59] secp256k1_xonly_pubkey_tweak_add_check
Where the missing 3 are: secp256k1_pre_g_128, secp256k1_pre_g, and secp256k1_ecmult_gen_prec_table.
Hmm, it definitely does for me. The difference is more clear with
objdump -p.
Confirming that it works for me as well. And the -fvisibility=hidden C compiler flag becomes redundant.
For me, making that switch (along with #1242) solves all current export problems that I’m aware of.
Nice. https://gcc.gnu.org/wiki/Visibility#How_to_use_the_new_C.2B-.2B-_visibility_support also suggests this. It would be nice to document this deviation from the libtool recommendation in the comment.
Interestingly the gcc man page says
with -fvisibility=hidden and “attribute ((visibility(“default”)))” instead of “__declspec(dllexport)” you get almost identical semantics with identical syntax.
Yep, almost identical…
I had always kinda assumed they’re equivalent
Heh, I guess this is why:
Interestingly the gcc man page says
with -fvisibility=hidden and “attribute ((visibility(“default”)))” instead of “__declspec(dllexport)” you get almost identical semantics with identical syntax.
Yep, almost identical…
Yeah, it would seem those docs are wrong, given that we’re seeing obvious differences in behavior.
It may be that default visibility is intended to be an alias for dllexport on windows, but if that’s the case and this is a bug I doubt they’d change the behavior now. So I’d guess the best approach would be to submit an upstream PR to remove that line in the docs.
Nice. https://gcc.gnu.org/wiki/Visibility#How_to_use_the_new_C.2B-.2B-_visibility_support also suggests this. It would be nice to document this deviation from the libtool recommendation in the comment.
Will do. I’ll PR this as a fix, but I’ll wait until the current PR onslaught slows down a bit to avoid piling on :)
But we can fix this in both GCC and clang by any of the following (in descending order of my personal preference).
Add
__attribute__ ((visibility ("hidden")))to theexterndeclarationsWrap the
externdeclarations in#pragma GCC visibility push(hidden)…#pragma GCC visibility pop.
FWIW, neither works for macOS’s DYLIB.
Are we happy with a non-macOS solution?
UPD. It seems, there is a solution…
Labels
build