[libc] exposes internal implementations in symbol table

[nickdesaulniers]

running llvm-readelf -sW on libc.a shows that the namespaced symbols are exported with STB_GLOBAL binding and STV_DEFAULT visbility:

$ llvm-readelf -sW projects/libc/lib/libc.a | llvm-cxxfilt | grep select
File: projects/libc/lib/libc.a(select.cpp.o)
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS select.cpp
     2: 0000000000000000    16 OBJECT  LOCAL  DEFAULT    5 .L__const.select.pss
     3: 0000000000000000   187 FUNC    GLOBAL DEFAULT    3 select
     6: 0000000000000000   187 FUNC    GLOBAL DEFAULT    3 __llvm_libc_18_0_0_git::select(int, fd_set*, fd_set*, fd_set*, timeval*)

ideally _ZN22__llvm_libc_18_0_0_git6selectEiP6fd_setS1_S1_P7timeval would not be accessible to folks linking against libc.a.

diff --git a/libc/src/__support/common.h b/libc/src/__support/common.h
index 53951dc131c2..e7902f96b617 100644
--- a/libc/src/__support/common.h
+++ b/libc/src/__support/common.h
@@ -22,10 +22,10 @@
 
 // MacOS needs to be excluded because it does not support aliasing.
 #if defined(LIBC_COPT_PUBLIC_PACKAGING) && (!defined(__APPLE__))
-#define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                           \
-  LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                       \
-      __##name##_impl__ __asm__(#name);                                        \
-  decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name)]];                   \
+#define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                                    \
+  LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                                \
+      __##name##_impl__ __asm__(#name);                                                 \
+  decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name),gnu::visibility("hidden")]];  \
   type __##name##_impl__ arglist
 #else
 #define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist) type name arglist

will at least produce

     6: 0000000000000000   187 FUNC    GLOBAL HIDDEN     3 __llvm_libc_18_0_0_git::select(int, fd_set*, fd_set*, fd_set*, timeval*)

but I think even better would be:

     6: 0000000000000000   187 FUNC    LOCAL DEFAULT     3 __llvm_libc_18_0_0_git::select(int, fd_set*, fd_set*, fd_set*, timeval*)

[llvmbot]

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

running `llvm-readelf -sW` on `libc.a` shows that the namespaced symbols are exported with STB_GLOBAL binding and STV_DEFAULT visbility:

$ llvm-readelf -sW projects/libc/lib/libc.a | llvm-cxxfilt | grep select
File: projects/libc/lib/libc.a(select.cpp.o)
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS select.cpp
     2: 0000000000000000    16 OBJECT  LOCAL  DEFAULT    5 .L__const.select.pss
     3: 0000000000000000   187 FUNC    GLOBAL DEFAULT    3 select
     6: 0000000000000000   187 FUNC    GLOBAL DEFAULT    3 __llvm_libc_18_0_0_git::select(int, fd_set*, fd_set*, fd_set*, timeval*)

ideally _ZN22__llvm_libc_18_0_0_git6selectEiP6fd_setS1_S1_P7timeval would not be accessible to folks linking against libc.a.

diff --git a/libc/src/__support/common.h b/libc/src/__support/common.h
index 53951dc131c2..e7902f96b617 100644
--- a/libc/src/__support/common.h
+++ b/libc/src/__support/common.h
@@ -22,10 +22,10 @@
 
 // MacOS needs to be excluded because it does not support aliasing.
 #if defined(LIBC_COPT_PUBLIC_PACKAGING) && (!defined(__APPLE__))
-#define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                           \
-  LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                       \
-      __##name##_impl__ __asm__(#name);                                        \
-  decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name)]];                   \
+#define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                                    \
+  LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                                \
+      __##name##_impl__ __asm__(#name);                                                 \
+  decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name),gnu::visibility("hidden")]];  \
   type __##name##_impl__ arglist
 #else
 #define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist) type name arglist

will at least produce

     6: 0000000000000000   187 FUNC    GLOBAL HIDDEN     3 __llvm_libc_18_0_0_git::select(int, fd_set*, fd_set*, fd_set*, timeval*)

but I think even better would be:

     6: 0000000000000000   187 FUNC    LOCAL DEFAULT     3 __llvm_libc_18_0_0_git::select(int, fd_set*, fd_set*, fd_set*, timeval*)

[nickdesaulniers]

@lntue points out that the tests appear to be calling the namespaced variants.

[nickdesaulniers]

LIBC_NAMESPACE_DECL was a recent addition which addresses this issue (via hidden visibility).

I was also surprised when debugging a different issue that our external symbols also had hidden visibility. Had I integrated llvm-libc into bionic sooner, the recent commit to llvm-libc to add LIBC_NAMESPACE_DECL would have broken the bionic build.

665efe8 introduced LIBC_NAMESPACE_DECL, which was then used in 5ff3ff3. If I revert 5ff3ff3 then do another overlay build, I get default vis for both symbols below.

example:

$ cd llvm-project
$ mkdir build_foo
$ cd !$
$ cmake ../llvm -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_ENABLE_PROJECTS=libc
$ ninja libc
$ llvm-readelf -s projects/libc/lib/libllvmlibc.a | grep bsearch
File: projects/libc/lib/libllvmlibc.a(bsearch.cpp.o)
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT   ABS bsearch.cpp
     2: 0000000000000000   147 FUNC    GLOBAL HIDDEN      3 bsearch
     3: 0000000000000000   147 FUNC    GLOBAL HIDDEN      3 _ZN22__llvm_libc_19_0_0_git7bsearchEPKvS1_mmPFiS1_S1_E

I can understand _ZN22__llvm_libc_19_0_0_git7bsearchEPKvS1_mmPFiS1_S1_E having hidden visibility, but not bsearch itself. @gburgessiv tried to work around this in https://android-review.googlesource.com/c/platform/external/llvm-libc/+/3322522, but I don't think the workaround should be used; I suspect something else is wrong with 5ff3ff3.

cc @petrhosek @PiJoules @gburgessiv

[nickdesaulniers]

Changing the definition of LIBC_NAMESPACE_DECL via:

diff --git a/libc/src/__support/macros/config.h b/libc/src/__support/macros/config.h
index 2ab0fba095e6..ac278412acaf 100644
--- a/libc/src/__support/macros/config.h
+++ b/libc/src/__support/macros/config.h
@@ -27,7 +27,7 @@
 #define LIBC_HAS_FEATURE(f) 0
 #endif
 
-#ifdef __clang__
+#if defined (__clang__) && defined (LLVM_LIBC_FULL_BUILD)
 // Declare a LIBC_NAMESPACE with hidden visibility. `namespace
 // LIBC_NAMESPACE_DECL {` should be used around all declarations and definitions
 // for libc internals as opposed to just `namespace LIBC_NAMESPACE {`. This

so that we only use hidden visibility when using full build mode gives us:

$ llvm-readelf -s projects/libc/lib/libllvmlibc.a | grep bsearch
File: projects/libc/lib/libllvmlibc.a(bsearch.cpp.o)
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT   ABS bsearch.cpp
     2: 0000000000000000   147 FUNC    GLOBAL DEFAULT     3 bsearch
     3: 0000000000000000   147 FUNC    GLOBAL DEFAULT     3 _ZN22__llvm_libc_19_0_0_git7bsearchEPKvS1_mmPFiS1_S1_E

but I suspect that ultimately what we want for overlay mode is:

 llvm-readelf -s projects/libc/lib/libllvmlibc.a | grep bsearch
 File: projects/libc/lib/libllvmlibc.a(bsearch.cpp.o)
      1: 0000000000000000     0 FILE    LOCAL  DEFAULT   ABS bsearch.cpp
      2: 0000000000000000   147 FUNC    GLOBAL DEFAULT     3 bsearch
-     3: 0000000000000000   147 FUNC    GLOBAL DEFAULT     3 _ZN22__llvm_libc_19_0_0_git7bsearchEPKvS1_mmPFiS1_S1_E
+     3: 0000000000000000   147 FUNC    GLOBAL HIDDEN     3 _ZN22__llvm_libc_19_0_0_git7bsearchEPKvS1_mmPFiS1_S1_E

Actually, that's probably what we want for BOTH overlay and full build mode?

---

EDIT: ah Petr has a patch!