[SystemZ][ZOS] Prefer -nostdlib++ as opposed to -nodefaultlibs when building c++ libraries

[zibi]

Summary

Let's use -nostdlib++ rather than -nodefaultlibs when building libc++/libc++abi/libunwind libraries. The default is -nostdlib++ if supported by a build compiler like it is the case with clang, otherwise -nodefaultlibs is used as before.

This change is needed to avoid additional changes at the link step and not to increase the maintenance costs. If clang with -nodefaultlibs is used all the libraries which are removed but required would have to be manually added in. This set of libraries are unique and will send out.

The propose change will allow to make the link step simple for other platforms as well.

[ldionne]

The general direction looks alright to me. Can you please use arc diff to upload patches so that the proper metadata is added to the Phabricator revision and CI can run?

libcxx/cmake/config-ix.cmake

28

typo: otherwise -nodefaultlibs

[zibi]

pached created by arc diff

[zibi]

typo + patch with arc diff

[ldionne]

Please wait for CI to pass, and then feel free to ship it. Thanks!

[ldionne]

Well, it looks like there are issues to resolve before this can be shipped, as shown by the CI.

[zibi]

Default libunwind to be build with -nodefaultlibs on MINGW.

[zibi]

On MINGW we need to build libunwind with -nodefaultlibs on b/c LIBUNWIND_SUPPORTS_FUNWIND_TABLES_FLAG is not set,
otherwise we issue this error:

if (LIBUNWIND_ENABLE_SHARED AND
    NOT (LIBUNWIND_SUPPORTS_FNO_EXCEPTIONS_FLAG AND
         LIBUNWIND_SUPPORTS_FUNWIND_TABLES_FLAG))
  message(FATAL_ERROR
          "Compiler doesn't support generation of unwind tables if exception "
          "support is disabled.  Building libunwind DSO with runtime dependency "
          "on C++ ABI library is not supported.")
endif()

[mstorsjo]

libunwind/cmake/config-ix.cmake

33

Doesn't this end up adding -nodefaultlibs twice to CMAKE_REQUIRED_FLAGS (the second time below on line 39)?

[zibi]

libunwind/cmake/config-ix.cmake

33

Doesn't this end up adding -nodefaultlibs twice to CMAKE_REQUIRED_FLAGS (the second time below on line 39)?

Thank you for caching this. Line 39 should be deleted but let me wait first for CI to see if the original regression is gone.

[zibi]

reverse previous fix and try something new to test CI

[zibi]

adding none c++ libraries back even for -nostdlib++ for libcxx/libcxxabi/libunwind to check if this will make CI clean

[zibi]

Hopefully this time CI will be clean, the previous one never completed.

[zibi]

@ldionne Can you check why the last 3 builds always hang in Apple tests?
Here is the link to the latest build.

[ldionne]

In D95875#2542722, @zibi wrote:

@ldionne Can you check why the last 3 builds always hang in Apple tests?
Here is the link to the latest build.

The number of builders on mac is very limited. They're running on a few laptops sitting right beside me. You have to be a bit patient. Re-uploading patches to try and shake the CI is actually making things worse, as our BuildKite setup isn't smart enough to cancel jobs that have already been submitted when you re-upload a newer patch.

[zibi]

In D95875#2542734, @ldionne wrote:

In D95875#2542722, @zibi wrote:

@ldionne Can you check why the last 3 builds always hang in Apple tests?
Here is the link to the latest build.

The number of builders on mac is very limited. They're running on a few laptops sitting right beside me. You have to be a bit patient. Re-uploading patches to try and shake the CI is actually making things worse, as our BuildKite setup isn't smart enough to cancel jobs that have already been submitted when you re-upload a newer patch.

It's good to know, thank you. I was suspicious because quite a few builds completed which were submitted AFTER mine so I was warring that they might loop or hang. Now I actually see that the 1st of my builds just completed. It must be some kind of priority I guess.

[ldionne]

In D95875#2542790, @zibi wrote:

In D95875#2542734, @ldionne wrote:

In D95875#2542722, @zibi wrote:

@ldionne Can you check why the last 3 builds always hang in Apple tests?
Here is the link to the latest build.

The number of builders on mac is very limited. They're running on a few laptops sitting right beside me. You have to be a bit patient. Re-uploading patches to try and shake the CI is actually making things worse, as our BuildKite setup isn't smart enough to cancel jobs that have already been submitted when you re-upload a newer patch.

It's good to know, thank you. I was suspicious because quite a few builds completed which were submitted AFTER mine so I was warring that they might loop or hang. Now I actually see that the 1st of my builds just completed. It must be some kind of priority I guess.

Hmm, actually you're right, newer builds seem to be prioritized over older ones. That's exactly the opposite of what I would expect. That's weird. I'll work on making sure that remaining mac jobs don't run if any other job failed - I think that'll help reduce the load on the mac builders.

[ldionne]

In D95875#2545103, @ldionne wrote:

Hmm, actually you're right, newer builds seem to be prioritized over older ones. That's exactly the opposite of what I would expect. That's weird. I'll work on making sure that remaining mac jobs don't run if any other job failed - I think that'll help reduce the load on the mac builders.

Ah, I just figured out what was wrong. I changed the queue of the mac builders and your CI was targeting an old queue that doesn't exist. Please rebase onto main and resubmit, your build should go through normally. Sorry about that, I've been wresting with the infrastructure this week.

[zibi]

• resubmitting to get clean CI, thank you Louis for finding the issue with previous builds

[zibi]

@ldionne You might also cancel these 2 builds:
https://buildkite.com/llvm-project/libcxx-ci/builds/1246
https://buildkite.com/llvm-project/libcxx-ci/builds/1239

[zibi]

FYI, @ldionne I'm looking for you approval.

The only delta since the last approval is to run the logic of adding additional libraries in both -nostdlib++ and -nodefaultlibs cases for all 3 libraries. Originally I had the logic of adding additional libraries apply to -nodefaultlibs case only which caused regressions. The configuration for all libcxx, libcxxabi and libunwind libraries are now the same.

[ldionne]

libcxx/CMakeLists.txt

729

"In order to remove just libc++ from the link step"

libcxx/cmake/config-ix.cmake

27

Remove "The"

46

I would have expected that this conditional be if (NOT LIBCXX_SUPPORTS_NOSTDLIBXX_FLAG AND LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG) instead. Indeed, if we use -nostdlib++, we shouldn't need to add these system libraries manually, right?

libcxxabi/cmake/config-ix.cmake

17

Remove "The"

libunwind/cmake/config-ix.cmake

19

Remove "The"

[zibi]

libcxx/cmake/config-ix.cmake

46

That would cause cmake test for -nostdinc++ to fail and eventually it would lead to CI failure. See this failure. The root of the problem is that -nostdinc++ was missing from the compile command because cmake thought this option was not supported.

[zibi]

NFC: typos

[zibi]

ping

[ldionne]

I think this is good for libunwind too - I feel confident enough to LGTM this CMake change even though I'm not an owner.

[hvdijk]

In D95875#2565670, @ldionne wrote:

I think this is good for libunwind too

It's not, depending on how clang is configured this can cause the libunwind build to fail because clang now tries to link libunwind against itself. -nodefaultlibs prevents that, -nostdlib++ does not.

/usr/bin/clang  -m64 -fPIC -O2 --target=x86_64-unknown-linux-gnu -DNDEBUG --target=x86_64-unknown-linux-gnu -rtlib=compiler-rt -nostdlib++ -Wl,-O1 -Wl,--as-needed -s -shared -Wl,-soname,libunwind.so.1 -o ../lib/x86_64-linux-gnu/libunwind.so.1.0 CMakeFiles/unwind_shared.dir/libunwind.cpp.o CMakeFiles/unwind_shared.dir/Unwind-EHABI.cpp.o CMakeFiles/unwind_shared.dir/Unwind-seh.cpp.o CMakeFiles/unwind_shared.dir/UnwindLevel1.c.o CMakeFiles/unwind_shared.dir/UnwindLevel1-gcc-ext.c.o CMakeFiles/unwind_shared.dir/Unwind-sjlj.c.o CMakeFiles/unwind_shared.dir/UnwindRegistersRestore.S.o CMakeFiles/unwind_shared.dir/UnwindRegistersSave.S.o  -lc /usr/lib/clang/13.0.0/lib/linux/libclang_rt.builtins-x86_64.a -ldl -lpthread 
ld.lld: error: unable to find library -l:libunwind.so
ld.lld: error: unable to find library -l:libunwind.so
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Would there be any objections to reverting this change for libunwind, or do you prefer to fix this some other way?

[mstorsjo]

In D95875#3068943, @hvdijk wrote:

Would there be any objections to reverting this change for libunwind, or do you prefer to fix this some other way?

I would prefer to not revert it. The generally more specific way of disabling things with -nostdlibc++ is nicer as it doesn’t need to hardcode/guess what other platform specific libs are linked by default, that has to be specified manually.

However, should we try to use -unwindlib=none at the same time as testing for -nostdlibc++? I think that would take care of this issue, right?

[hvdijk]

In D95875#3069114, @mstorsjo wrote:

However, should we try to use -unwindlib=none at the same time as testing for -nostdlibc++? I think that would take care of this issue, right?

That should work too, sure, and sounds good to me.

libcxx/CMakeLists.txt

 endif()
 get_sanitizer_flags(SANITIZER_FLAGS "${LLVM_USE_SANITIZER}")
 if (LIBCXX_STANDALONE_BUILD AND SANITIZER_FLAGS)
   add_flags(${SANITIZER_FLAGS})
 endif()
 
 # Link system libraries =======================================================
 function(cxx_link_system_libraries target)
+
+# In order to remove just a c++ library from the libc++ link step
+# we need to use -nostdlib++ whenever it is supported.
+# Unfortunately this cannot be used universally because for example g++ supports
+# only -nodefaultlibs in which case all libraries will be removed and
+# all libraries but c++ have to be added in manually.
+  if (LIBCXX_SUPPORTS_NOSTDLIBXX_FLAG)
+    target_add_link_flags_if_supported(${target} PRIVATE "-nostdlib++")
+  else()
     target_add_link_flags_if_supported(${target} PRIVATE "-nodefaultlibs")
     target_add_compile_flags_if_supported(${target} PRIVATE "/Zl")
     target_add_link_flags_if_supported(${target} PRIVATE "/nodefaultlib")
+  endif()
 
   if (LIBCXX_HAS_SYSTEM_LIB)
     target_link_libraries(${target} PRIVATE System)
   endif()
 
   if (LIBCXX_HAS_PTHREAD_LIB)
     target_link_libraries(${target} PRIVATE pthread)
   endif()

libcxx/cmake/config-ix.cmake

else()
     if(ANDROID)
       check_library_exists(gcc __gcc_personality_v0 "" LIBCXX_HAS_GCC_LIB)
     else()
       check_library_exists(gcc_s __gcc_personality_v0 "" LIBCXX_HAS_GCC_S_LIB)
     endif()
   endif()
 endif()
 
-# libc++ is built with -nodefaultlibs, so we want all our checks to also
-# use this option, otherwise we may end up with an inconsistency between
+# The libc++ is using -nostdlib++ at the link step when available,
+# otherwise -nodefaultlibs is used. We want all our checks to also
+# use one of these options, otherwise we may end up with an inconsistency between
 # the flags we think we require during configuration (if the checks are
-# performed without -nodefaultlibs) and the flags that are actually
-# required during compilation (which has the -nodefaultlibs). libc is
+# performed without one of those options) and the flags that are actually
+# required during compilation (which has the -nostdlib++ or -nodefaultlibs). libc is
 # required for the link to go through. We remove sanitizers from the
 # configuration checks to avoid spurious link errors.
+
+check_c_compiler_flag(-nostdlib++ LIBCXX_SUPPORTS_NOSTDLIBXX_FLAG)
+if (LIBCXX_SUPPORTS_NOSTDLIBXX_FLAG)
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
+else()
   check_c_compiler_flag(-nodefaultlibs LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG)
   if (LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG)
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
+  endif()
+endif()
+
+if (LIBCXX_SUPPORTS_NOSTDLIBXX_FLAG OR LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG)
   if (LIBCXX_HAS_C_LIB)
     list(APPEND CMAKE_REQUIRED_LIBRARIES c)
   endif ()
   if (LIBCXX_USE_COMPILER_RT)
     list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
     find_compiler_rt_library(builtins LIBCXX_BUILTINS_LIBRARY)
     list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBCXX_BUILTINS_LIBRARY}")
   elseif (LIBCXX_HAS_GCC_LIB)

libcxxabi/cmake/config-ix.cmake

 include(CMakePushCheckState)
 include(CheckLibraryExists)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckCSourceCompiles)
 
 check_library_exists(c fopen "" LIBCXXABI_HAS_C_LIB)
 if (NOT LIBCXXABI_USE_COMPILER_RT)
   if (ANDROID)
     check_library_exists(gcc __gcc_personality_v0 "" LIBCXXABI_HAS_GCC_LIB)
   else ()
     check_library_exists(gcc_s __gcc_personality_v0 "" LIBCXXABI_HAS_GCC_S_LIB)
     check_library_exists(gcc __aeabi_uldivmod "" LIBCXXABI_HAS_GCC_LIB)
   endif ()
 endif ()
 
-# libc++abi is built with -nodefaultlibs, so we want all our checks to also
-# use this option, otherwise we may end up with an inconsistency between
+# The libc++abi is using -nostdlib++ at the link step when available,
+# otherwise -nodefaultlibs is used. We want all our checks to also
+# use one of these options, otherwise we may end up with an inconsistency between
 # the flags we think we require during configuration (if the checks are
 # performed without -nodefaultlibs) and the flags that are actually
 # required during compilation (which has the -nodefaultlibs). libc is
 # required for the link to go through. We remove sanitizers from the
 # configuration checks to avoid spurious link errors.
-check_c_compiler_flag(-nodefaultlibs LIBCXXABI_HAS_NODEFAULTLIBS_FLAG)
-if (LIBCXXABI_HAS_NODEFAULTLIBS_FLAG)
+
+check_c_compiler_flag(-nostdlib++ LIBCXXABI_SUPPORTS_NOSTDLIBXX_FLAG)
+if (LIBCXXABI_SUPPORTS_NOSTDLIBXX_FLAG)
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
+else()
+  check_c_compiler_flag(-nodefaultlibs LIBCXXABI_SUPPORTS_NODEFAULTLIBS_FLAG)
+  if (LIBCXXABI_SUPPORTS_NODEFAULTLIBS_FLAG)
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
+  endif()
+endif()
+
+if (LIBCXXABI_SUPPORTS_NOSTDLIBXX_FLAG OR LIBCXXABI_SUPPORTS_NODEFAULTLIBS_FLAG)
   if (LIBCXXABI_HAS_C_LIB)
     list(APPEND CMAKE_REQUIRED_LIBRARIES c)
   endif ()
   if (LIBCXXABI_USE_COMPILER_RT)
     list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
     find_compiler_rt_library(builtins LIBCXXABI_BUILTINS_LIBRARY)
     list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBCXXABI_BUILTINS_LIBRARY}")
   else ()

libcxxabi/src/CMakeLists.txt

if (ANDROID AND ANDROID_PLATFORM_LEVEL LESS 21)
   list(APPEND LIBCXXABI_LIBRARIES android_support)
 endif()
 
 if (NOT LIBCXXABI_USE_COMPILER_RT)
   add_library_flags_if(LIBCXXABI_HAS_GCC_LIB gcc)
 endif ()
 
 # Setup flags.
+if (LIBCXXABI_SUPPORTS_NOSTDLIBXX_FLAG)
+  add_link_flags_if_supported(-nostdlib++)
+else()
   add_link_flags_if_supported(-nodefaultlibs)
+endif()
 
 if ( APPLE )
   if (LLVM_USE_SANITIZER)
     if (("${LLVM_USE_SANITIZER}" STREQUAL "Address") OR
         ("${LLVM_USE_SANITIZER}" STREQUAL "Address;Undefined") OR
         ("${LLVM_USE_SANITIZER}" STREQUAL "Undefined;Address"))
       set(LIBFILE "libclang_rt.asan_osx_dynamic.dylib")
     elseif("${LLVM_USE_SANITIZER}" STREQUAL "Undefined")

libunwind/cmake/config-ix.cmake

if (NOT LIBUNWIND_USE_COMPILER_RT)
   if (ANDROID)
     check_library_exists(gcc __gcc_personality_v0 "" LIBUNWIND_HAS_GCC_LIB)
   else ()
     check_library_exists(gcc_s __gcc_personality_v0 "" LIBUNWIND_HAS_GCC_S_LIB)
     check_library_exists(gcc __absvdi2 "" LIBUNWIND_HAS_GCC_LIB)
   endif ()
 endif()
 
-# libunwind is built with -nodefaultlibs, so we want all our checks to also
-# use this option, otherwise we may end up with an inconsistency between
+# The libunwind is using -nostdlib++ at the link step when available,
+# otherwise -nodefaultlibs is used. We want all our checks to also
+# use one of these options, otherwise we may end up with an inconsistency between
 # the flags we think we require during configuration (if the checks are
-# performed without -nodefaultlibs) and the flags that are actually
-# required during compilation (which has the -nodefaultlibs). libc is
+# performed without one of those options) and the flags that are actually
+# required during compilation (which has the -nostdlib++ or -nodefaultlibs). libc is
 # required for the link to go through. We remove sanitizers from the
 # configuration checks to avoid spurious link errors.
-check_c_compiler_flag(-nodefaultlibs LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
-if (LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
+
+check_c_compiler_flag(-nostdlib++ LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
+if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
+  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
+else()
+  check_c_compiler_flag(-nodefaultlibs LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
+  if (LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
     set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
+  endif()
+endif()
+
+if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG OR LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
   if (LIBUNWIND_HAS_C_LIB)
     list(APPEND CMAKE_REQUIRED_LIBRARIES c)
   endif ()
   if (LIBUNWIND_USE_COMPILER_RT)
     find_compiler_rt_library(builtins LIBUNWIND_BUILTINS_LIBRARY)
     list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBUNWIND_BUILTINS_LIBRARY}")
   else ()
     if (LIBUNWIND_HAS_GCC_S_LIB)

libunwind/src/CMakeLists.txt

 endif()
 add_library_flags_if(LIBUNWIND_HAS_DL_LIB dl)
 if (LIBUNWIND_ENABLE_THREADS)
   add_library_flags_if(LIBUNWIND_HAS_PTHREAD_LIB pthread)
   add_compile_flags_if(LIBUNWIND_WEAK_PTHREAD_LIB -DLIBUNWIND_USE_WEAK_PTHREAD=1)
 endif()
 
 # Setup flags.
+if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
+  add_link_flags_if_supported(-nostdlib++)
+else()
   add_link_flags_if_supported(-nodefaultlibs)
+endif()
 
 # MINGW_LIBRARIES is defined in config-ix.cmake
 add_library_flags_if(MINGW "${MINGW_LIBRARIES}")
 
 if (LIBUNWIND_ENABLE_SHARED AND
     NOT (LIBUNWIND_SUPPORTS_FNO_EXCEPTIONS_FLAG AND
          LIBUNWIND_SUPPORTS_FUNWIND_TABLES_FLAG))
   message(FATAL_ERROR