[libc++] Move the linker script generation step to CMake

[ldionne]

Summary

This allows the linker script generation to query CMake properties
(specifically the dependencies of libc++.so) instead of having to
carry these dependencies around manually in global variables. Notice
the removal of the LIBCXX_INTERFACE_LIBRARIES global variable.

[ldionne]

My goal with this patch is to eliminate some global variables from the CMake sources, which make my life harder while refactoring other parts of the build (I'm working on other, upcoming patches).

I checked that the linker script generated with this patch matched the one generated before this patch on a Docker container running Ubuntu.

[thakis]

libcxx/cmake/Modules/DefineLinkerScript.cmake

47

We recently made it so that the script is silent during normal operation. It looks like this adds the build output back. Could you omit this please? Successful build steps should be silent.

[ldionne]

This comment was removed by ldionne.

libcxx/cmake/Modules/DefineLinkerScript.cmake

47

This build output is only included if the verbose mode of the CMake generator is used.

[ldionne]

Sorry, I sent the previous comment while I was still drafting.

@phosek @thakis I do have one question. Is there a reason why we want the linker script to look like

INPUT(libc++.so.1 -lunwind -lc++abi)

instead of

INPUT(libc++.so.1 -lpthread -lc <etc...> -lunwind -lc++abi)

It seems like it should be OK to include -lpthread and friends in the linker script, since we need to link against those too, no?

libcxx/cmake/Modules/DefineLinkerScript.cmake

47

This build output is only included if the verbose mode of the CMake generator is used.

Scratch that, I was in the process of verifying the claim when I sent out the comment!

So it turns out the comment is only printed when the Makefile generator is used, but I think you guys use Ninja. LMK if you still want me to remove it.

Also, if you really want your build not to output anything when it's successful, there's more you need to do than just suppress these intermediate status updates, since several other tools will have intermediate output too.

[ldionne]

Ping @phosek @thakis

[thakis]

I do not know why the linker script looks like it does.

I don't particularly care about the make build's output. I weakly feel that build steps should be silent (unless they fail) in all generators (http://www.linfo.org/rule_of_silence.html), but cmake's make generator output produces pretty verbose output by default already, so it's just another broken window there.

[ldionne]

It turns out that @EricWF was the one to originally write the script, so maybe he knows why the output is

INPUT(libc++.so.1 -lunwind -lc++abi)

instead of

INPUT(libc++.so.1 -lpthread -lc <etc...> -lunwind -lc++abi)

[phosek]

In D68343#1697634, @ldionne wrote:

It turns out that @EricWF was the one to originally write the script, so maybe he knows why the output is

INPUT(libc++.so.1 -lunwind -lc++abi)

instead of

INPUT(libc++.so.1 -lpthread -lc <etc...> -lunwind -lc++abi)

In case of shared libraries, the libpthread and libc dependency is already encoded in the shared library itself (e.g. in case of ELF it'd be as DT_NEEDED entries), specifying them in the linker script is not necessary and in many cases undesirable (e.g. if your application doesn't use any threading API, you probably don't want to get implicit DT_NEEDED dependency on libpthread just because you're linking libc++). Where it'd make sense is for static libraries, but we don't currently generate linker script for those.

libcxx/cmake/Modules/DefineLinkerScript.cmake

46

Would the file redirect (>) work on Windows as well?

[ldionne]

In D68343#1698655, @phosek wrote:

In case of shared libraries, the libpthread and libc dependency is already encoded in the shared library itself (e.g. in case of ELF it'd be as DT_NEEDED entries), specifying them in the linker script is not necessary and in many cases undesirable (e.g. if your application doesn't use any threading API, you probably don't want to get implicit DT_NEEDED dependency on libpthread just because you're linking libc++). Where it'd make sense is for static libraries, but we don't currently generate linker script for those.

Ok. After r374053 and r374056, the linker script will contain the right dependencies.

I think this is good to go unless we're using a linker script on Windows and the redirection doesn't work, in which case I'll find a workaround.

libcxx/cmake/Modules/DefineLinkerScript.cmake

46

I don't think so, but would you create a linker script on Windows? I thought this was only on Linux?

[thakis]

I don't think linker scripts are a thing on Window. lld-link doesn't support them. Maybe something in MinGW does, I don't know.

[mstorsjo]

In D68343#1700294, @thakis wrote:

I don't think linker scripts are a thing on Window. lld-link doesn't support them. Maybe something in MinGW does, I don't know.

GNU ld does support linker script on COFF (somewhat at least, I think), but lld doesn't.

[ldionne]

I'll go ahead with this since there doesn't seem to be a clear agreement that we need this to work on Windows.

[vitalybuka]

libcxx/cmake/Modules/DefineLinkerScript.cmake

42

https://llvm.org/docs/CMake.html min requirement is 3.4.3
but JOIN is 3.4.12

[vitalybuka]

libcxx/cmake/Modules/DefineLinkerScript.cmake

42

I see a fix r374120

[phosek]

In D68343#1699713, @ldionne wrote:

I think this is good to go unless we're using a linker script on Windows and the redirection doesn't work, in which case I'll find a workaround.

In D68343#1700294, @thakis wrote:

I don't think linker scripts are a thing on Window. lld-link doesn't support them. Maybe something in MinGW does, I don't know.

In D68343#1700312, @mstorsjo wrote:

GNU ld does support linker script on COFF (somewhat at least, I think), but lld doesn't.

It's not about using linker scripts on Windows but cross-compiling for other systems that use linker scripts on Windows, e.g. we use linker scripts in Fuchsia and would like to build Clang toolchain including all runtimes on Windows which requires running this code when cross-compiling runtimes for Fuchsia.

[phosek]

This seems to have broke our build, I checked the generated linker script and it has the following format:

INPUT(libc++.so.2 -lunwind_shared -lcxxabi_shared)

libunwind and libc++abi dependencies are incorrect, it should be:

INPUT(libc++.so.2 -lunwind -lc++abi)

[phosek]

In D68343#1702521, @phosek wrote:

This seems to have broke our build, I checked the generated linker script and it has the following format:

INPUT(libc++.so.2 -lunwind_shared -lcxxabi_shared)

libunwind and libc++abi dependencies are incorrect, it should be:

INPUT(libc++.so.2 -lunwind -lc++abi)

D68791 is a fix.

[ldionne]

In D68343#1702521, @phosek wrote:

This seems to have broke our build, I checked the generated linker script and it has the following format:

INPUT(libc++.so.2 -lunwind_shared -lcxxabi_shared)

libunwind and libc++abi dependencies are incorrect, it should be:

INPUT(libc++.so.2 -lunwind -lc++abi)

How do you configure your build? The paths were certainly correct when I tested this on Linux. I think this may have something to do with the fact that you use the (kind of hacky) HAVE_LIBCXXABI switch. If you let me know how you configure your build, I'll try to repro and fix it.

[ldionne]

In D68343#1703942, @ldionne wrote:

In D68343#1702521, @phosek wrote:

This seems to have broke our build, I checked the generated linker script and it has the following format:

INPUT(libc++.so.2 -lunwind_shared -lcxxabi_shared)

libunwind and libc++abi dependencies are incorrect, it should be:

INPUT(libc++.so.2 -lunwind -lc++abi)

How do you configure your build? The paths were certainly correct when I tested this on Linux. I think this may have something to do with the fact that you use the (kind of hacky) HAVE_LIBCXXABI switch. If you let me know how you configure your build, I'll try to repro and fix it.

Oops, I didn't reload before replying. I'll follow up on https://reviews.llvm.org/D68791.

libcxx/CMakeLists.txt

if (LIBCXX_ENABLE_STATIC AND NOT PYTHONINTERP_FOUND)
     message(FATAL_ERROR "LIBCXX_ENABLE_STATIC_ABI_LIBRARY requires python but it was not found.")
   endif()
 endif()
 
 if (LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
     if (APPLE)
       message(FATAL_ERROR "LIBCXX_ENABLE_ABI_LINKER_SCRIPT cannot be used on APPLE targets")
     endif()
-    if (NOT PYTHONINTERP_FOUND)
-      message(FATAL_ERROR "LIBCXX_ENABLE_ABI_LINKER_SCRIPT requires python but it was not found.")
-    endif()
     if (NOT LIBCXX_ENABLE_SHARED)
       message(FATAL_ERROR "LIBCXX_ENABLE_ABI_LINKER_SCRIPT is only available for shared library builds.")
     endif()
 endif()
 
 if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY AND LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
     message(FATAL_ERROR "Conflicting options given.
         LIBCXX_ENABLE_STATIC_ABI_LIBRARY cannot be specified with

 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR})
 
 # Declare libc++ configuration variables.
 # They are intended for use as follows:
 # LIBCXX_CXX_FLAGS: General flags for both the compiler and linker.
 # LIBCXX_COMPILE_FLAGS: Compile only flags.
 # LIBCXX_LINK_FLAGS: Linker only flags.
 # LIBCXX_LIBRARIES: libraries libc++ is linked to.
-# LIBCXX_INTERFACE_LIBRARIES: Libraries that must be linked when using libc++
-#                             These libraries are exposed in the linker script.
 set(LIBCXX_COMPILE_FLAGS "")
 set(LIBCXX_LINK_FLAGS "")
 set(LIBCXX_LIBRARIES "")
-set(LIBCXX_INTERFACE_LIBRARIES "")
 
 # Include macros for adding and removing libc++ flags.
 include(HandleLibcxxFlags)
 
 # Target flags ================================================================
 # These flags get added to CMAKE_CXX_FLAGS and CMAKE_C_FLAGS so that
 # 'config-ix' use them during feature checks. It also adds them to both
 # 'LIBCXX_COMPILE_FLAGS' and 'LIBCXX_LINK_FLAGS'

libcxx/cmake/Modules/DefineLinkerScript.cmake

+# This function defines a linker script in place of the symlink traditionally
+# created for shared libraries.
+#
+# More specifically, this function goes through the PUBLIC and INTERFACE
+# library dependencies of <target> and gathers them into a linker script,
+# such that those libraries are linked against when the shared library for
+# <target> is linked against.
+#
+# Arguments:
+#   <target>: A target representing a shared library. A linker script will be
+#             created in place of that target's TARGET_LINKER_FILE, which is
+#             the symlink pointing to the actual shared library (usually
+#             libFoo.so pointing to libFoo.so.1, which itself points to
+#             libFoo.so.1.0).
+
+function(define_linker_script target)
+  if (NOT TARGET "${target}")
+    message(FATAL_ERROR "The provided target '${target}' is not actually a target.")
+  endif()
+
+  get_target_property(target_type "${target}" TYPE)
+  if (NOT "${target_type}" STREQUAL "SHARED_LIBRARY")
+    message(FATAL_ERROR "The provided target '${target}' is not a shared library (its type is '${target_type}').")
+  endif()
+
+  set(symlink "$<TARGET_LINKER_FILE:${target}>")
+  set(soname "$<TARGET_SONAME_FILE_NAME:${target}>")
+
+  get_target_property(interface_libs "${target}" INTERFACE_LINK_LIBRARIES)
+
+  set(link_libraries)
+  if (interface_libs)
+    foreach(lib IN LISTS interface_libs)
+      if (TARGET "${lib}")
+        list(APPEND link_libraries "${CMAKE_LINK_LIBRARY_FLAG}$<TARGET_PROPERTY:${lib},OUTPUT_NAME>")
+      else()
+        list(APPEND link_libraries "${CMAKE_LINK_LIBRARY_FLAG}${lib}")
+      endif()
+    endforeach()
+  endif()
+  list(JOIN link_libraries " " link_libraries)
+
+  set(linker_script "INPUT(${soname} ${link_libraries})")
+  add_custom_command(TARGET "${target}" POST_BUILD
+    COMMAND "${CMAKE_COMMAND}" -E remove "${symlink}"
+    COMMAND "${CMAKE_COMMAND}" -E echo "${linker_script}" > "${symlink}"
+    COMMENT "Generating linker script: '${linker_script}' as file ${symlink}"
+    VERBATIM
+  )
+endfunction()

libcxx/src/CMakeLists.txt

PROPERTIES
       DEFINE_SYMBOL ""
   )
   cxx_set_common_defines(cxx_shared)
 
   # Link against LLVM libunwind
   if (LIBCXXABI_USE_LLVM_UNWINDER)
     if (NOT LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
       target_link_libraries(cxx_shared PUBLIC unwind_shared)
-      list(APPEND LIBCXX_INTERFACE_LIBRARIES unwind_shared) # For the linker script
     elseif (LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_static OR HAVE_LIBUNWIND))
       # libunwind is already included in libc++abi
     else()
-      target_link_libraries(cxx_shared PRIVATE unwind)
-      list(APPEND LIBCXX_INTERFACE_LIBRARIES unwind) # For the linker script
+      target_link_libraries(cxx_shared PUBLIC unwind)
     endif()
   endif()
 
   # Link against libc++abi
   if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
     if (APPLE)
       target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}")
     else()
       target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}" "-Wl,-Bdynamic,--no-whole-archive")
     endif()
   else()
     target_link_libraries(cxx_shared PUBLIC "${LIBCXX_CXX_SHARED_ABI_LIBRARY}")
-    list(APPEND LIBCXX_INTERFACE_LIBRARIES "${LIBCXX_CXX_SHARED_ABI_LIBRARY}") # For the linker script
   endif()
 
   # Maybe re-export symbols from libc++abi
   if (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
                  LIBCXX_CXX_ABI_LIBNAME STREQUAL "default")
             AND NOT DEFINED LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS)
     set(LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS ON)
   endif()
target_link_libraries(cxx_shared PRIVATE
       "-Wl,-force_symbols_not_weak_list,${CMAKE_CURRENT_SOURCE_DIR}/../lib/notweak.exp"
       "-Wl,-force_symbols_weak_list,${CMAKE_CURRENT_SOURCE_DIR}/../lib/weak.exp")
 
     if (NOT LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
       target_link_libraries(cxx_shared PRIVATE "-Wl,-reexported_symbols_list,${CMAKE_CURRENT_SOURCE_DIR}/../lib/libc++abi-new-delete.exp")
     endif()
   endif()
 
-  # Generate a linker script in place of a libc++.so symlink. Rerun this command
-  # after cxx builds.
+  # Generate a linker script in place of a libc++.so symlink.
   if (LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
-    # Get the name of the ABI library and handle the case where CXXABI_LIBNAME
-    # is a target name and not a library. Ex cxxabi_shared.
-    set(LIBCXX_INTERFACE_LIBRARY_NAMES)
-    foreach(lib ${LIBCXX_INTERFACE_LIBRARIES})
-      # FIXME: Handle cxxabi_static and unwind_static.
-      if (TARGET ${lib} OR
-          (${lib} MATCHES "cxxabi(_static|_shared)?" AND HAVE_LIBCXXABI) OR
-          (${lib} MATCHES "unwind(_static|_shared)?" AND HAVE_LIBUNWIND))
-        list(APPEND LIBCXX_INTERFACE_LIBRARY_NAMES "$<TARGET_PROPERTY:${lib},OUTPUT_NAME>")
-      else()
-        list(APPEND LIBCXX_INTERFACE_LIBRARY_NAMES "${lib}")
-      endif()
-    endforeach()
-    add_custom_command(TARGET cxx_shared POST_BUILD
-      COMMAND
-        ${PYTHON_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/gen_link_script.py
-      ARGS
-        --input "$<TARGET_SONAME_FILE:cxx_shared>"
-        --output "$<TARGET_LINKER_FILE:cxx_shared>"
-        ${LIBCXX_INTERFACE_LIBRARY_NAMES}
-      WORKING_DIRECTORY ${LIBCXX_BUILD_DIR}
-    )
+      include(DefineLinkerScript)
+      define_linker_script(cxx_shared)
   endif()
 
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_shared")
   if(WIN32 AND NOT MINGW AND NOT "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows")
     # Since we most likely do not have a mt.exe replacement, disable the
     # manifest bundling.  This allows a normal cmake invocation to pass which
     # will attempt to use the manifest tool to generate the bundled manifest
     set_target_properties(cxx_shared PROPERTIES

libcxx/utils/gen_link_script.py

-#!/usr/bin/env python
-#===----------------------------------------------------------------------===##
-#
-# 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
-#
-#===----------------------------------------------------------------------===##
-
-"""
-Generate a linker script that links libc++ to the proper ABI library.
-An example script for c++abi would look like "INPUT(libc++.so.1 -lc++abi)".
-"""
-
-import argparse
-import os
-import sys
-
-
-def main():
-    parser = argparse.ArgumentParser(description=__doc__)
-    parser.add_argument("--dryrun", help="Don't write any output",
-                        action="store_true", default=False)
-    parser.add_argument("--rename", action="store_true", default=False,
-                        help="Rename the output as input so we can replace it")
-    parser.add_argument("--input", help="Path to libc++ library", required=True)
-    parser.add_argument("--output", help="Path to libc++ linker script",
-                        required=True)
-    parser.add_argument("libraries", nargs="+",
-                        help="List of libraries libc++ depends on")
-    args = parser.parse_args()
-
-    # Use the relative path for the libc++ library.
-    libcxx = os.path.relpath(args.input, os.path.dirname(args.output))
-
-    # Prepare the list of public libraries to link.
-    public_libs = ['-l%s' % l for l in args.libraries]
-
-    # Generate the linker script contents.
-    contents = "INPUT(%s)" % ' '.join([libcxx] + public_libs)
-
-    if args.dryrun:
-        print("GENERATING SCRIPT: '%s' as file %s" % (contents, args.output))
-        return 0
-
-    # Remove the existing libc++ symlink if it exists.
-    if os.path.islink(args.output):
-        os.unlink(args.output)
-
-    # Replace it with the linker script.
-    with open(args.output, 'w') as f:
-        f.write(contents + "\n")
-
-    return 0
-
-
-if __name__ == '__main__':
-    sys.exit(main())