[libc++] Always build with -fvisibility=hidden

[ldionne]

Summary

This avoids symbols being accidentally exported from the dylib when they
shouldn't. The next step is to use a pragma to apply hidden visibility
to all declarations (unless otherwise specified), which will allow us
to drop the per-declaration hidden visibility attributes we currently
have.

This also has the nice side effect of making sure the dylib exports the
same symbols regardless of the optimization level.

PR38138

[ldionne]

@phosek You may want to take a look too.

[mclow.lists]

This is in response to https://bugs.llvm.org/show_bug.cgi?id=38138, correct?

[ldionne]

In D62868#1529597, @mclow.lists wrote:

This is in response to https://bugs.llvm.org/show_bug.cgi?id=38138, correct?

Yup. I have a note in the commit message.

[smeenai]

<3

[mclow.lists]

I'm fine with this.

[ldionne]

I'll ship this next week once I've tested it on a Linux Docker image (I don't have access to that right now). I'm not sure, but I think I remember trying to make this change a while back and breaking the bots.

[ldionne]

I finally did the test on a docker image mirroring the build bot configuration, and it doesn't appear to break anything. I'll submit this now.

[peter.smith]

Apologies for the late notice, we've just moved one our builders http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux-noexceptions to use gcc rather than clang and we've just picked up some test failures due to this change. In summary when compiling libc++ with GCC the shared object defines a symbol as hidden that clang does not which is causing test failures due to linker errors. I've put the details in https://bugs.llvm.org/show_bug.cgi?id=43140 . It looks like the failure was first seen on http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11/builds/345 (not run by us).

[ldionne]

In D62868#1648991, @peter.smith wrote:

Apologies for the late notice, we've just moved one our builders http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux-noexceptions to use gcc rather than clang and we've just picked up some test failures due to this change. In summary when compiling libc++ with GCC the shared object defines a symbol as hidden that clang does not which is causing test failures due to linker errors. I've put the details in https://bugs.llvm.org/show_bug.cgi?id=43140 . It looks like the failure was first seen on http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11/builds/345 (not run by us).

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

[mgorny]

In D62868#1650892, @ldionne wrote:

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Ping. What's the progress? Unless I'm mistaken, all libc++ failures on our buildbot are related to this: http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22010

[ldionne]

In D62868#1656807, @mgorny wrote:

In D62868#1650892, @ldionne wrote:

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Ping. What's the progress? Unless I'm mistaken, all libc++ failures on our buildbot are related to this: http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22010

https://reviews.llvm.org/D66970 would fix the issue, but I'm waiting on a review. I'll check-in a minimal fix to the problem in the meantime.

[ldionne]

In D62868#1657521, @ldionne wrote:

In D62868#1656807, @mgorny wrote:

In D62868#1650892, @ldionne wrote:

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Ping. What's the progress? Unless I'm mistaken, all libc++ failures on our buildbot are related to this: http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22010

https://reviews.llvm.org/D66970 would fix the issue, but I'm waiting on a review. I'll check-in a minimal fix to the problem in the meantime.

https://reviews.llvm.org/rCXX370926 should fix the problem

[mgorny]

Thanks. It seems to fix the test I used to bisect this. It will take some time before buildbot confirms this but I'm going to presume it's good now. If it fails more, I'll ping you.

libcxx/trunk/include/chrono

 #else
   typedef long long rep;
   typedef nano period;
 #endif
 
   typedef chrono::duration<rep, period> duration;
   typedef chrono::time_point<_FilesystemClock> time_point;
 
+  _LIBCPP_EXPORTED_FROM_ABI
   static _LIBCPP_CONSTEXPR_AFTER_CXX11 const bool is_steady = false;
 
   _LIBCPP_AVAILABILITY_FILESYSTEM _LIBCPP_FUNC_VIS static time_point now() noexcept;
 
   _LIBCPP_INLINE_VISIBILITY
   static time_t to_time_t(const time_point& __t) noexcept {
       typedef chrono::duration<rep> __secs;
       return time_t(

libcxx/trunk/include/filesystem

public:
 
   _LIBCPP_INLINE_VISIBILITY
   bool recursion_pending() const { return __rec_; }
 
   _LIBCPP_INLINE_VISIBILITY
   void disable_recursion_pending() { __rec_ = false; }
 
 private:
+  _LIBCPP_FUNC_VIS
   recursive_directory_iterator(const path& __p, directory_options __opt,
                                error_code* __ec);
 
   _LIBCPP_FUNC_VIS
   const directory_entry& __dereference() const;
 
   _LIBCPP_FUNC_VIS
   bool __try_recursion(error_code* __ec);

libcxx/trunk/src/CMakeLists.txt

 if (LIBCXX_ENABLE_PARALLEL_ALGORITHMS AND NOT TARGET pstl::ParallelSTL)
   message(FATAL_ERROR "Could not find ParallelSTL")
 endif()
 
 function(cxx_set_common_defines name)
   if(LIBCXX_CXX_ABI_HEADER_TARGET)
     add_dependencies(${name} ${LIBCXX_CXX_ABI_HEADER_TARGET})
   endif()
+  target_compile_options(${name} PRIVATE -fvisibility=hidden)
   if(WIN32 AND NOT MINGW)
     target_compile_definitions(${name}
                                PRIVATE
                                  # Ignore the -MSC_VER mismatch, as we may build
                                  # with a different compatibility version.
                                  _ALLOW_MSC_VER_MISMATCH
                                  # Don't check the msvcprt iterator debug levels
                                  # as we will define the iterator types; libc++
set_target_properties(cxx_static
     PROPERTIES
       COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
   )
   cxx_set_common_defines(cxx_static)
 
   if (LIBCXX_HERMETIC_STATIC_LIBRARY)
-    append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
     # If the hermetic library doesn't define the operator new/delete functions
     # then its code shouldn't declare them with hidden visibility.  They might
     # actually be provided by a shared library at link time.
     if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
       append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
     endif()
     target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
     target_compile_definitions(cxx_static PRIVATE _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)

libcxx/trunk/src/valarray.cpp

 //===------------------------ valarray.cpp --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "valarray"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 // These two symbols are part of the v1 ABI but not part of the >=v2 ABI.
 #if _LIBCPP_ABI_VERSION == 1
-template valarray<size_t>::valarray(size_t);
-template valarray<size_t>::~valarray();
+template _LIBCPP_FUNC_VIS valarray<size_t>::valarray(size_t);
+template _LIBCPP_FUNC_VIS valarray<size_t>::~valarray();
 #endif
 
 template void valarray<size_t>::resize(size_t, size_t);
 
 void
 gslice::__init(size_t __start)
 {
     valarray<size_t> __indices(__size_.size());