rnpgp / sexpp

C++ Library for SEXP (S-expressions)
Other
7 stars 4 forks source link

Consider symbol visibility for shared library #41

Open dkg opened 1 year ago

dkg commented 1 year ago

The current shared object produced with BUILD_SHARED_LIBS=on exposes many different C++ symbols, and appears to make no attempt to filter what is explicitly exposed.

Does every symbol need to be exposed in the shared object? I'm not enough of a C++ person to be able to tell what the library's symbol table should really look like, but i do observe that there are symbols exposed that are not part of either the sexp namespace or the ext_key_format namespace.

For example, when i try to build it i see the following symbols:

(there are many more, but the list above gives a flavor of what i'm seeing)

maxirmx commented 1 year ago

Thank you, @dkg

It was designed as a private static library. Now it needs some reasonable public API

dkg commented 1 year ago

i don't think the symbol cleanup was as effective as intended here. the only exported symbol removed from the library between 0.8.5 and 0.8.7 was _ZSt30__lexicographical_compare_implIN9__gnu_cxx17__normal_iteratorIPKcNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEESA_NS0_5__ops15_Iter_comp_iterIZNK14ext_key_format22extended_private_key_t7ci_lessclERKS9_SH_EUlccE_EEEbT_SK_T0_SL_T1_ (which de-mangles to bool std::__lexicographical_compare_impl<__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, __gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, __gnu_cxx::__ops::_Iter_comp_iter<ext_key_format::extended_private_key_t::ci_less::operator()(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const::{lambda(char, char)#1}> >(__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, __gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, __gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, __gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, __gnu_cxx::__ops::_Iter_comp_iter<ext_key_format::extended_private_key_t::ci_less::operator()(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const::{lambda(char, char)#1}>))

That is, it's good that this symbol was removed, but there are still many other symbols of dubious value to a clean API exposed (at least when building with gcc and the toolchain available in debian testing/unstable).

ni4 commented 1 year ago

@dkg Thanks for checking this. @maxirmx I has some nice time with symbol visibility for rnp library. Please see this snippet from the rnp/lib/CMakeLists.txt:

 # Limit symbols export only to rnp_* functions.
  if (APPLE)
    # use -export_symbols_list on Apple OSs
    target_link_options(librnp PRIVATE -Wl,-exported_symbols_list "${CMAKE_CURRENT_SOURCE_DIR}/librnp.symbols")
    set_target_properties(librnp PROPERTIES LINK_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/librnp.symbols")
  elseif(NOT WIN32)
    target_link_options(librnp PRIVATE "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/librnp.vsc")
    set_target_properties(librnp PROPERTIES LINK_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/librnp.vsc")
  endif()
maxirmx commented 1 year ago

sexpp is C++ library that uses C++ standard library heavily.

C++ standard library is a template library to the great extent. Template function (or class) is essentially code generator and an entity that is linked(loaded) from another module. Practically it means that when a toolset needs to instatiate a template it indirectly creates a body for function(s), compiles it and places in a current module. This approach means that multiple modules (executables and shared libraries) may have a body for the same instantiated function. This conflict is resolved with weak symbols. When a weak defined symbol is linked with a normal defined symbol, the normal defined symbol is used with no error. When a weak undefined symbol is linked and the symbol is not defined, the value of the weak symbol becomes zero with no error.

If I look at the questioned symbols: _ZNKSt5ctypeIcE8do_widenEc (unmangled: std::ctype<char>::do_widen(char) const) -- function template https://en.cppreference.com/w/cpp/locale/ctype/widen (it is not on the export table on my Ubuntu box though)

_ZNSt15_Sp_counted_ptrIDnLN9__gnu_cxx12_Lock_policyE2EE10_M_disposeEv (unmangled: std::_Sp_counted_ptr<decltype(nullptr), (__gnu_cxx::_Lock_policy)2>::_M_dispose()) this is a template behind std::shared_ptr. I have it marked 'W' -- the symbol is a weak symbol that has not been specifically tagged as a weak object symbol and has a default value specified.

_ZZNSt8__detail18__to_chars_10_implIjEEvPcjT_E8__digits (unmangled: std::__detail::__to_chars_10_impl<unsigned int>(char*, unsigned int, unsigned int)::__digits) https://gcc.gnu.org/onlinedocs/gcc-13.1.0/libstdc++/api/a01663.html#a74745386dd30ed18bc932a26a766d147 I do not know what this template does, but it is also marked 'W'

Simply speaking, the when a library exports classes/functions that use C++ standard library template types as bases/parameters it also exports all member functions for instatiated templates and their underlying templates recursively as weak symbols.

It is is very different from managing export symbols for C-style libraries.

If you know how to apply a different method of linkage to C++ standart library please advise.

dkg commented 1 year ago

@maxirmx, i think you probably have a better understanding of it than i do, unfortunately. You describe some aspects of the problem in ways that make me think more deeply about it, at least ☺

That said, i don't understand how or when C++ applications are supposed to use these library interfaces. For example, consider two libraries that take the same approach, which will both have weak symbols for the templated classes. how does an application decide which one of them to use? What happens if the memory image of the object imagined by one implementation disagrees with the memory image from another implementation?

If the answer is that an application won't use either of them, and instead relies on its own instantiation, then why do the libraries need to export the functions in the first place?

Anyway, i'm happy to leave this open as a question in the hopes that someone else will come along and offer more insight. thanks for considering it!