[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

34

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

34

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 ↗

(On Diff #321762)

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

libcxx/cmake/config-ix.cmake

27–29

Remove "The"

57

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–19

Remove "The"

libunwind/cmake/config-ix.cmake

19–21

Remove "The"

[zibi]

libcxx/cmake/config-ix.cmake

57

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/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()
 
 # The libc++ is using -nostdlib++ at the link step when available,
-# otherwise -nostdlib++ is used. We want all our checks to also
+# 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 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)
if (LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG)
   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)
     list(APPEND CMAKE_REQUIRED_LIBRARIES gcc)
   elseif (LIBCXX_HAS_GCC_S_LIB)
     list(APPEND CMAKE_REQUIRED_LIBRARIES gcc_s)
   endif ()
   if (MINGW)
     # Mingw64 requires quite a few "C" runtime libraries in order for basic
     # programs to link successfully with -nodefaultlibs.
     if (LIBCXX_USE_COMPILER_RT)
       set(MINGW_RUNTIME ${LIBCXX_BUILTINS_LIBRARY})
     else ()
       set(MINGW_RUNTIME gcc_s gcc)

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 ()
 
 # The libc++abi is using -nostdlib++ at the link step when available,
-# otherwise -nostdlib++ is used. We want all our checks to also
+# 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(-nostdlib++ LIBCXXABI_SUPPORTS_NOSTDLIBXX_FLAG)
 if (LIBCXXABI_SUPPORTS_NOSTDLIBXX_FLAG)

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()
 
 # The libunwind is using -nostdlib++ at the link step when available,
-# otherwise -nostdlib++ is used. We want all our checks to also
+# 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 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++ 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_NODEFAULTLIBS_FLAG)
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
   if (LIBUNWIND_HAS_C_LIB)
     list(APPEND CMAKE_REQUIRED_LIBRARIES c)
   endif ()