Default visibility of shared library symbols

[fgvanzee]

Thanks to #303, we are very close to having a configure-time option to control the default (or starting) visibility of symbols within shared libraries. Thanks to Isuru's efforts, we have now annotated which function prototypes and global variable declarations correspond to "public" APIs and symbols, with the remaining symbols assumed to be eligible for being "hidden" (for internal use only). That is, by compiling with -fvisibility=hidden, the default visibility of all symbols is hidden, and then our new annotations selectively re-publicize those that should be exported. Similarly, when using -fvisibility=default, the default visibility is "public" (default here is a bit of a misnomer, as explained in the gcc man page), in which case all symbols, public and internal, get exported.

I am working on a new configure option to BLIS, --enable-export-all. This option would cause -fvisibility=default to be used, which would mean all symbols are exported within shared libraries, even those that virtually every user of BLIS would have no need for. --disable-export-all would cause -fvisibility=hidden to be used, which would mean only symbols identified as part of our public APIs would be exported.

My primary question for the community is: what should be the default for this new configure option? (My preference is for --disable-export-all to be the default, meaning that we hide internal-only symbols from shared libraries unless the user asks for them at configure-time.)

Secondarily, I wanted to ask @isuruf (and/or others) what compiler options (if any) should be used or withheld in each of the above cases (--enable-export-all and --disable-export-all) in order to accommodate the AppVeyor build process for Windows binaries.

[isuruf]

For windows builds (like on other OSes), it doesn't matter which option is default. Both should work fine.

[isuruf]

On windows with Clang though, --enable-export-all is going to be a misnomer because it doesn't export all symbols. The only way to enable exporting all would be to introduce another macro for internal API.

[fgvanzee]

On windows with Clang though, --enable-export-all is going to be a misnomer because it doesn't export all symbols. The only way to enable exporting all would be to introduce another macro for internal API.

Thanks for pointing this out. It sounds like what you're saying is that on Windows, we will always export only the annotated symbols, whereas on Linux we have the option via -fvisibility=[default|hidden] option. Is that right?

[isuruf]

Yes. There's an option with gcc on windows (-Wl,--export-all-symbols, -Wl,--enable-auto-import ), but there's no such thing with clang AFAIK.

[fgvanzee]

Understood. I'm totally fine with documenting in the configure --help text that this option has no effect for Windows with clang.

Does AppVeyor inherently use clang when building for Windows?

[isuruf]

Out of the 4 jobs on appveyor, first one uses gcc and the other 3 uses clang.

[fgvanzee]

Okay, here's my plan. I see that we have access CC_VENDOR in the top-level Makefile, via the configure-generated config.mk. I can branch based on that:

• If it's gcc for Windows, I can compile with -Wl,--export-all-symbols, -Wl,--enable-auto-import.
• If it's clang for Windows, I'll compile with no extra flags.
• And for Linux, I'll use -fvisibility=[default|hidden].

What should I use for gcc on Windows if I wanted the equivalent of -fvisibility=hidden?

Let me know if I left anything out.

[isuruf]

-Wl,--exclude-all-symbols for gcc on windows.

[fgvanzee]

Okay, thanks. Once I finish the code modifications, you can comment on the logic and/or suggest changes as needed.

[fgvanzee]

@isuruf Take a look at the changes I made to common.mk in e095926. Let me know if there's anything I got wrong about the symbol visibility flags for the various compilers / OSes. (I confirmed that both gcc and icc on Linux respond appropriately to -fvisibility=.) I'm sure AppVeyor will also barf at me if something is wrong, so I'll be on the lookout for that too.

[fgvanzee]

As you probably noticed, I didn't bother branching for Windows vs non-Windows when using icc, mostly because we already don't make any effort to support Windows builds for anything beyond AppVeyor (gcc and clang).

[fgvanzee]

Made another update in 8093956, this time to add symbols that were missing that were needed by the testsuite when building in AppVeyor and linking to the shared library (this probably affected Linux too, but I just fixed it and didn't confirm).

[isuruf]

Looks good to me. Clang on Linux and OSX also support -fvisibility=hidden/default

[fgvanzee]

@isuruf Thanks for pointing that out. The appropriate change is in 6bfe381.

I think I'm done with this issue for now. I'll leave it open for a while to solicit feedback on whether the default should be --disable-export-all (as it currently is) or --enable-export-all.

I'm also thinking about renaming the option to --export=[public|all]. But this is just syntax pickiness; nothing would change about the option semantically.

[fgvanzee]

As I alluded to, I ended up changing the --enable-export-all option in d202d00 as follows. The option is now --export-shared=[public|all] with public still being the default.

[fgvanzee]

Since this issue has attracted no comments from the community, I'll assume that nobody really objects to --export-shared=public being the default option and close this issue. Please re-open if you have any concerns.