search

eclipse-ecal / ecal

📦 eCAL - enhanced Communication Abstraction Layer. A high performance publish-subscribe, client-server cross-plattform middleware.
https://ecal.io
Apache License 2.0
848 stars 178 forks source link

[core] force symbol hiding for ecal_core(_c).so. #1787

Closed KerstinKeller closed 2 weeks ago

KerstinKeller commented 2 weeks ago

Description

When a library exports all symbols, this can lead to problems. In eCAL this mainly stems from compiling in symbols from thirdparty libraries, especially nanopb. Hiding the symbols solves the issue that at runtime, internal symbols are visible over .so file boarders. We've actively exported symbols for Windows dlls, so we can just hide symbols on non-msvc platforms.

Currently, this leads to problems with respect to classes. On MSVC, vtables are exported, even when not the complete class is exported. For e.g. gcc this is not the case. Hence we introduce new macros to a) export the whole class for non-msvc compilers and b) export only the member functions for msvc compilers.

DownerCase commented 2 weeks ago

Ah, good luck with this. I had a crack at something like this earlier this year but had trouble with the difference in how MSVC vs gcc/clang handles exporting the run time type info for classes. https://github.com/DownerCase/ecal/commit/1003c3d07bc03f59c7c2f63aa5850b58f1f42348

Pulling from the CI logs this looks like it was undefined reference to typeinfo for eCAL::CServiceClient'. So for gcc, the classes must be marked with a default visibility attribute. However, MSVC won't let you mark the entire class and specific functions.

I found this as a useful reference, as it's the same problem: https://github.com/PixarAnimationStudios/OpenUSD/issues/665

So I believe the two options would be to:

Hope my rambling was at least remotely helpful! 😅

KerstinKeller commented 2 weeks ago

Hi @DownerCase, yes you summed up the problem pretty nicely. Not sure which one will be the "better" approach. If I do export the whole class on both Unix and Windows, using std::shared_ptr as private member for pimpl will give warnings, so I'd have to move that back to raw pointers, which I don't like.

So I'll probably go with different macros, making the whole class visible on Unix only.