Skip to content
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.

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

Use ELF visibility to export only public symbols from the library #560

Open
vadz opened this issue Jul 20, 2017 · 3 comments
Open

Use ELF visibility to export only public symbols from the library #560

vadz opened this issue Jul 20, 2017 · 3 comments

Comments

@vadz
Copy link
Member

vadz commented Jul 20, 2017

We should really define SOCI_DLL appropriately not only in Win32 builds, but also when ELF visibility is supported for all the usual reasons (more efficient shared library loading; consistency between platforms).

As usual, I don't know how to handle this at CMake level, but if anyone can take care of this, I could do the required (trivial) changes to the sources.

@phetdam
Copy link

phetdam commented Jan 4, 2025

Very late but this is something I've had to personally work around on Windows when using the SOCI DLLs.

Take for example use of the ODBC backend DLL. The ODBC backend docs show use of soci::odbc, declared as:

extern SOCI_ODBC_DECL odbc_backend_factory const odbc;

However, without SOCI_DLL defined during compilation, trying to access the soci::odbc symbol while using SOCI::soci_odbc in a target_link_libraries call will just result in a linker error with MSVC since SOCI_DECL_IMPORT, and thus SOCI_ODBC_DECL, is just defined to nothing. To work around this, I made my interface targets for my own project, e.g.

# SOCI core library, propagating SOCI_DLL when using the shared target
add_library(myproj::soci_core INTERFACE IMPORTED)
target_link_libraries(myproj::soci_core INTERFACE SOCI::soci_core)
target_compile_definitions(myproj::soci_core INTERFACE SOCI_DLL)
# assume something similar for a myproj::soci_odbc target

Linking against these targets now works seamlessly because the SOCI_DLL macro definition is propagated. Something similar can be done for SOCI's own targets, e.g. target_compile_options(${SOCI_CORE_TARGET} PUBLIC SOCI_DLL).

@phetdam
Copy link

phetdam commented Jan 4, 2025

By the way, I think DEFINE_SYMBOL and SOCI_SOURCE are being used incorrectly. SOCI_SOURCE should control whether SOCI_DECL (and SOCI_ODBC_DECL, etc.) resolves to SOCI_DECL_EXPORT or SOCI_DECL_IMPORT. This is what DEFINE_SYMBOL is supposed to be used for; I was surprised to see SOCI_DLL being used for DEFINE_SYMBOL instead and SOCI_SOURCE being manually defined in .cpp sources. Ideally, we want the following to be done for SOCI shared libraries:

  1. Define SOCI_DLL during shared library compilation and propagate this requirement to downstream targets
  2. Define SOCI_SOURCE only during shared library compilation

This can be pretty easily achieved with something like the following (e.g. for the core library):

# already called add_library(${SOCI_CORE_TARGET} SHARED ...)
# ensure SOCI_DLL is defined when using a shared library
target_compile_options(${SOCI_CORE_TARGET} PUBLIC SOCI_DLL)
# ensure when compiling that annotated symbols get exported
set_target_properties(${SOCI_CORE_TARGET} PROPERTIES DEFINE_SYMBOL SOCI_SOURCE)

To this end, the #define SOCI_SOURCE lines should all be removed from the .cpp files. Also, for cross-platform consistency in terms of symbol export, soci-platform.h should be tweaked a little bit:

#elif defined(SOCI_HAVE_VISIBILITY_SUPPORT)
// note: public visibility only applied when SOCI_DLL is defined. current build
// config will use -fvisibility=hidden or equivalent for unexported symbols
# ifdef SOCI_DLL
#  define SOCI_DECL_EXPORT __attribute__ (( visibility("default") ))
#  define SOCI_DECL_IMPORT __attribute__ (( visibility("default") ))
# endif
#endif

I see in the CMakeLists.txt that CMAKE_CXX_VISIBILITY_PRESET will be set to hidden if SOCI_HAVE_VISIBILITY_SUPPORT is defined, so in this case, static SOCI libraries should all have hidden symbols across both Windows/POSIX. Currently, there is no extra SOCI_DLL check, so the SOCI_DECL annotated symbols will still have default (public) visibility.

@phetdam
Copy link

phetdam commented Jan 5, 2025

Ended up making a feature branch in my SOCI fork that ensures that SOCI_DLL is part of the interface compile definitions for the SOCI::soci_core and SOCI backend shared libraries. The soci_backend CMake macro will now use SOCI_${NAMEU}_SOURCE as the DEFINE_SYMBOL for each backend library, e.g. for the Firebird backend library SOCI_FIREBIRD_SOURCE is defined during compilation. Of course, the SOCI core library uses SOCI_SOURCE as its DEFINE_SYMBOL.

In the course of my work I was forced to also do the following:

  1. Fix FindMySQL.cmake to not set CMAKE_REQUIRED_INCLUDES if MySQL is not found
  2. Remove SOCI_DECL annotation of failover_callback as it is 100% defined inline (otherwise linker is confused)
  3. Remove the manual SOCI_SOURCE and SOCI_<backend>_SOURCE macro definitions in source files
  4. Ensure separate soci_tests_common and soci_tests_common_static static libraries with testing helpers

The reason 4. is necessary is because since SOCI_DLL is required for use of SOCI shared libraries, the export visibility of the declarations will change depending on whether SOCI_DLL is defined or not. This is particularly noticeable on Windows.

So far things seem to working well in terms of building the library. I was able to test MySQL, ODBC, SQLite3, Firebird, and PostgreSQL backend compilation on WSL Ubuntu (ODBC and SQLite3 only for native Windows).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants