search

supranational / blst

Multilingual BLS12-381 signature library
Apache License 2.0
454 stars 171 forks source link

Support `-fvisibility=hidden`, use explicit visibility attributes for both private and public symbols #179

Closed arnetheduck closed 11 months ago

arnetheduck commented 11 months ago

Currently, symbol exports are controlled via a linker script - linker scripts are notoriously hard to work with in custom build systems.

Using visibility attributes / dllimport / dllexport makes the API easier to consumer for all participants and avoids the need to maintain compiler-specific linker scripts and other forms of unnecessary duplication - https://github.com/bitcoin-core/secp256k1/blob/acf5c55ae6a94e5ca847e07def40427547876101/include/secp256k1.h#L137 is a good example.

In addition to this, private symbols such as __blst_platform_cap should ideally be marked with hidden visibility so that the library can easily be compiled without needing linker scripts nor -fvisibility=hidden flags.

arnetheduck commented 11 months ago

Regarding the last point, without a linker script or -fvisibility=hidden, the code can not be linked into a pic/pie executable due to lack of the right kind of relocations necessary for default-linkage symbols, making the build fail on __blst_platform_cap requires dynamic R_X86_64_PC32 reloc against....

dot-asm commented 11 months ago

Currently, symbol exports are controlled via a linker script

Not anymore:-) But even if it still was, you can rely on the fact that private symbols originating from C are all static and those coming from assembly module - hidden. As build.sh does now.

__blst_platform_cap should ideally be marked with hidden visibility

Done. [There also were hidden directives missing in src/asm/ct_* modules.] Thanks!

arnetheduck commented 11 months ago

you can rely on the fact that private symbols originating from C are all static

cap was not, which caused a fair bit of headscratching ;) thanks for looking into it.

dot-asm commented 11 months ago

can not be linked into a pic/pie executable due to lack of the right kind of relocations necessary for default-linkage symbols

-Bsymbolic is the answer to this question. It's argued that the option is more than appropriate, even necessary, for security libraries. Point is that symbols in non-Bsymbolic modules are easily overridden by other modules leaving you not knowing what's going on.

arnetheduck commented 11 months ago

thanks, lgtm! closing, was able to integrate successfully

arnetheduck commented 11 months ago

-Bsymbolic - I would assume that for hidden symbols, this wouldn't be a problem?

dot-asm commented 11 months ago

-Bsymbolic - I would assume that for hidden symbols, this wouldn't be a problem?

Correct. Linking to .hidden symbols is equivalent to -Bsymbolic. -Bsymbolic is still appropriate though, it helps in cases you both refer to a symbol internally and export it.