[libcxx] [test] Include more libraries that normally are linked automatically

[mstorsjo]

Summary

As the libcxx tests link with -nostdlib, libraries that normally
are added by default by the compiler driver has to be added
manually.

The "oldnames" library is automatically added when driving linking
with clang-cl. When linking with the plain clang driver, as the
libcxx tests do, the clang driver does the same but only since Clang
12.0). But when linking with -nostdlib, like the libcxx tests do,
the driver defaults aren't added at all, and we need to specify the
defaults manually.

This allows removing a TODO from the Windows CI setup; it turns out
that upgrading to Clang 12.0 didn't help here as expected, sorry about
that mixup.

[curdeius]

LGTM. Is there any other option on clang-cl to avoid linking MSVCRT but linking other stuff? Is there going to be any change in clang 13?

[mstorsjo]

In D101434#2722122, @curdeius wrote:

LGTM. Is there any other option on clang-cl to avoid linking MSVCRT but linking other stuff? Is there going to be any change in clang 13?

I don't know of any change wrt this in clang 13, and oldnames.lib is actually an extension to the regular msvcrt library (it mainly consists of aliases from functions like getcwd to the form _getcwd which exists in the import libraries, so we don't need to reroute calls to the underscored form in the test code), so I don't think it's practically doable to omit one of them but not the other.

[Mordante]

This SGTM, but I see the Runtimes build fails. Can you verify this has nothing to do with this patch?

[mstorsjo]

In D101434#2723346, @Mordante wrote:

This SGTM, but I see the Runtimes build fails. Can you verify this has nothing to do with this patch?

It's pretty certainly unrelated, as this patch only touches windows specific codepaths, and there were similar spurious breakage in D101437 (both ran on top of the same llvm-project commit). I can of course put it another round through CI before pushing...

[mstorsjo]

Rerun CI to hopefully get rid of spurious failure.

[Mordante]

Thanks for the return! LGTM!

libcxx/utils/ci/run-buildbot

generic-win)
     # TODO: Clang-cl in MSVC configurations don't have access to compiler_rt
     # builtins helpers for int128 division. See
     # https://reviews.llvm.org/D91139#2429595 for a comment about longterm
     # intent for handling the issue. In the meantime, define
     # -D_LIBCPP_HAS_NO_INT128 (both when building the library itself and
     # when building tests) to allow enabling filesystem for running tests,
     # even if it uses a non-permanent ABI.
 
-    # TODO: The CI runner currently uses Clang 11, which doesn't implicitly
-    # link in oldnames.lib (which is needed for some tests) when compiling
-    # with the plain "clang" driver, as the tests do (as opposed to using
-    # the "clang-cl" driver for compiling). When the CI runner runs
-    # Clang 12, the "-loldnames" option can be dropped.
-
     # TODO: Currently, building with the experimental library breaks running
     # tests (the test linking look for the c++experimental library with the
     # wrong name, and the statically linked c++experimental can't be linked
     # correctly when libc++ visibility attributes indicate dllimport linkage
     # anyway), thus just disable the experimental library. Remove this
     # setting when cmake and the test driver does the right thing automatically.
 
     echo "--- Generating CMake"
     cmake -S "${MONOREPO_ROOT}/libcxx" \
           -B "${BUILD_DIR}" \
           -GNinja -DCMAKE_MAKE_PROGRAM="${NINJA}" \
           -DCMAKE_BUILD_TYPE=RelWithDebInfo \
           -DCMAKE_C_COMPILER=clang-cl \
           -DCMAKE_CXX_COMPILER=clang-cl \
           -DLLVM_LIT_ARGS="-sv --show-unsupported --xunit-xml-output test-results.xml" \
           -DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=NO \
           -DLIBCXX_ENABLE_FILESYSTEM=YES \
           -DCMAKE_CXX_FLAGS="-D_LIBCPP_HAS_NO_INT128" \
-          -DLIBCXX_TEST_COMPILER_FLAGS="-D_LIBCPP_HAS_NO_INT128" \
-          -DLIBCXX_TEST_LINKER_FLAGS="-loldnames"
+          -DLIBCXX_TEST_COMPILER_FLAGS="-D_LIBCPP_HAS_NO_INT128"
     echo "+++ Running the libc++ tests"
     ${NINJA} -vC "${BUILD_DIR}" check-cxx
 ;;
 *)
     echo "${BUILDER} is not a known configuration"
     exit 1
 ;;
 esac

libcxx/utils/libcxx/test/config.py

def configure_link_flags_abi_library(self):
         elif cxx_abi == 'libcxxrt':
             self.cxx.link_flags += ['-lcxxrt']
         elif cxx_abi == 'vcruntime':
             debug_suffix = 'd' if self.debug_build else ''
             # This matches the set of libraries linked in the toplevel
             # libcxx CMakeLists.txt if building targeting msvc.
             self.cxx.link_flags += ['-l%s%s' % (lib, debug_suffix) for lib in
                                     ['vcruntime', 'ucrt', 'msvcrt', 'msvcprt']]
+            # The compiler normally links in oldnames.lib too, but we've
+            # specified -nostdlib above, so we need to specify it manually.
+            self.cxx.link_flags += ['-loldnames']
         elif cxx_abi == 'none' or cxx_abi == 'default':
             if self.target_info.is_windows():
                 debug_suffix = 'd' if self.debug_build else ''
                 self.cxx.link_flags += ['-lmsvcrt%s' % debug_suffix]
         else:
             self.lit_config.fatal(
                 'C++ ABI setting %s unsupported for tests' % cxx_abi)