[libc-commits] [libc] [libc] Set default visibility for LLVM functions (PR #116686)

[Petr Hosek via libc-commits]

petrhosek wrote:

> > FWIW LLVM_LIBC_FUNCTION_ATTR is used to set the default visibility in the default Bazel configuration as well:
> 
> So now that's three cases where downstream users are using `LLVM_LIBC_FUNCTION_ATTR` just to set the non-namespaced symbols to default visibility! 🤮
> 
> 1. [Fuchsia](https://cs.opensource.google/fuchsia/fuchsia/+/main:zircon/system/ulib/c/BUILD.gn;l=184;drc=4cd8bba05cb37d30da9eb365b369ed0a0bb43267)
> 2. [Android](https://cs.android.com/android/platform/superproject/main/+/main:external/llvm-libc/Android.bp;l=41)
> 3. [Bazel](https://github.com/llvm/llvm-project/blob/6626ed6f9fae79d35aba504f50bac4375686a03b/utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl#L114-L120)
> 
> (Bazel is also using LLVM_LIBC_FUNCTION_ATTR to add weak linkage attributes, but I think that can be better solved by @lntue's #116160).
>
> (There's a few other mentions of `LLVM_LIBC_FUNCTION_ATTR` in comments/docs to clean up, too. utils/bazel/llvm-project-overlay/libc/libc_build_rules.bzl would have to get cleaned up, too).
> 
> Thoughts?

This is a flawed analysis because it ignores all users that don't set `LLVM_LIBC_FUNCTION_ATTR` because they want hidden visibility. Even in Fuchsia, we build LLVM libc several times for several different use cases, and only one of those is using `[[gnu::visibility("default")]]`, that is when building `libc.so`. In general, I think `libc.so` is probably the only case where you want `[[gnu::visibility("default")]]`, for all other cases `[[gnu::visibility("hidden")]]` is preferable.

I agree with @jhuber6, I think we should keep `LLVM_LIBC_FUNCTION_ATTR` and introduce a new `config.json` option to let you set the default (and if unset we should default to hidden).

https://github.com/llvm/llvm-project/pull/116686