Skip to content

only export interface symbols from shared libraries on non-Windows#82

Merged
coelckers merged 4 commits intomasterfrom
hidden_symbols
Oct 6, 2025
Merged

only export interface symbols from shared libraries on non-Windows#82
coelckers merged 4 commits intomasterfrom
hidden_symbols

Conversation

@coelckers
Copy link
Member

@coelckers coelckers commented Oct 4, 2025

This needs testing on an actual Linux and macOS system to make sure it actually works.

I'd find it preferable if zmusic only exported its public interface and not the innards of the various third party libraries it includes to avoid symbol name clashes.

Attempt to fix #72

@Blzut3
Copy link
Member

Blzut3 commented Oct 4, 2025

This does work, but doesn't do exactly what you set out to accomplish. Since you set visibility for the source directory it only applies to targets defined in the source directory, so the third party libraries are still have default visibility. But ZMusic itself is only exposing the symbols explicitly exported.

Edit: Realized my final sentence may sound contradictory so here's the symbol diff
delta.txt

@Blzut3
Copy link
Member

Blzut3 commented Oct 4, 2025

Two ways to fix the rest of the symbols:

  1. Move the visibility preset to the root CMake so it applies to the thirdparty directory as well. Note that some of the libraries (GME, adlmidi, libxmp, and opnmidi) already support visibility hidden but since we're statically linking them their defines will need to be modified to never set visibility default.
  2. One could do target_link_options(zmusic PRIVATE LINKER:--exclude-libs=ALL) which tells the linker to not export anything in static libraries. HOWEVER, we're using OBJECT libraries in order to allow a static ZMusic to have everything self contained, so this won't just work. One would need to define a variable based on BUILD_SHARED_LIBS to switch the third party targets between OBJECT (BUILD_SHARED_LIBS=OFF) and STATIC (BUILD_SHARED_LIBS=ON).

Probably a good idea to set the linker option from 2 even if doing 1 as insurance. Since it shouldn't cause any crazy deltas option 1 is probably the better way to solve it.

@coelckers
Copy link
Member Author

The preset was supposed to be in the root project file, I must have picked the wrong one - as Visual Studio's file list does not show the paths and with all project files having the same name, it was easy to slip up.

Please try again.

@Blzut3
Copy link
Member

Blzut3 commented Oct 5, 2025

Better, but like I said, you do need to deal with the libraries that set default visibility on their symbols (i.e. BLARGG_EXPORT in gme, but similar in the other libraries I listed).

Also just remembered there's CMAKE_VISIBILITY_INLINES_HIDDEN you'll probably want to turn on.

@coelckers
Copy link
Member Author

Can you give me a list of currently visible symbols? Without anything to check against this is like poking in the dark.
Which libraries still need some work?

@Blzut3
Copy link
Member

Blzut3 commented Oct 6, 2025

Sure: symbols.txt

Based on the way you went about fixing GME, I think we're good here. The others I listed in my second reply happen to not be a problem since (I think) we wrote our own CMakeLists for them so we happen to not define the relevant constant to cause symbols to leak.

It is my understanding that some std:: symbols are unavoidable so don't pay much attention to them. (They're all weak symbols.) Don't know the specific details on the why.

I'd probably still suggest the link option I noted in my second reply for a little extra insurance, but under normal configuration it should make no difference.

@coelckers
Copy link
Member Author

Ok. It looks like this is a deliberate design decision. What's interesting here is that none of the exported symbols is for vectors of custom types, only for vectors of native types and std::string.

I could go ahead and reduce it a bit more by using TArray instead of std::vector but that sounds a bit like overkill with no real gain.
At least this now does what I set out to do, i.e. hide the symbols from the statically linked dependencies. That's good enough.

@coelckers coelckers merged commit d0e7cb0 into master Oct 6, 2025
16 checks passed
#define DLL_EXPORT __declspec(dllexport)
#define DLL_IMPORT __declspec(dllexport) // without this the compiler complains.
#if !defined(ZMUSIC_STATIC)
#if defined(_MSC_VER)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinGW/MinGW-w64 does the absolutely same, so please use _WIN32 here.

@coelckers coelckers deleted the hidden_symbols branch October 6, 2025 06:46
@coelckers
Copy link
Member Author

Done

@jojo989
Copy link

jojo989 commented Oct 18, 2025

amazing

@ZDoom ZDoom deleted a comment from juraisa Dec 19, 2025
@ZDoom ZDoom deleted a comment from Wohlstand Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[non-Windows] Limit exported symbols to ZMusic's public interface

4 participants