[libcxx] Add support for building and testing with an ABI library not along linker paths

[EricWF]

Summary

This patch adds support for building/testing libc++ with an ABI library that the linker would not normally find.

• CMAKE_LIBRARY_PATH is used to specify the list of search directories.
• The ABI library is now found using find_library instead of assuming its along the linker's search path.
• CMAKE_LIBRARY_PATH is passed to our LIT config as library_paths.
• For each path in library_paths the following flags are added -L<path> -Wl,-rpath -Wl,<path>

Some changes in existing behavior were also added:

• target_link_libraries is now passed the ABI library file instead of the library name. Ex target_link_libraries(cxx "/usr/lib/libc++abi.so") vs target_link_libraries(cxx "c++abi").
• -Wl,-rpath -Wl,<path> is now used on OSX to link to libc++ instead of env['DYLD_LIBRARY_PATH'] if use_system_lib=False.

[EricWF]

Added documentation on how to build libc++ against a non-installed ABI library.

Unfortunately not only does the user have to pass -DCMAKE_LIBRARY_PATH=/path/to/lib to CMake, but they also need to set RPATH=/path/to/lib when building libc++.
CMake does not have a mechanism for configuring relative paths via CMake variable.

We could manually add -Wl,-rpath -Wl,<lib-path> for all paths in CMAKE_LIBRARY_PATH but this also seems less than ideal.

[EricWF]

One change I forgot to mention.

On linux when using libcxxrt it seems that we also need to link against -ldl. Support for this was added.

Sorry for the spam.

[danalbert]

@EricWF wrote:

Unfortunately not only does the user have to pass -DCMAKE_LIBRARY_PATH=/path/to/lib to CMake, but they also need to set RPATH=/path/to/lib when building libc++.
CMake does not have a mechanism for configuring relative paths via CMake variable.

find_file() does this. Look at how I select libc++ header paths in the libc++abi CMakeLists.txt.

lib/CMakeLists.txt

46

libcxxrt is the one that should be linking libdl.

For a static libcxxrt we will need this though. Do we have a way to tell if we're linking a static abi lib?

48

What about FreeBSD? If the BSDs need it too, I think you actually just want to remove the second half of this conditional and let append_if work out this detail.

test/lit.cfg

342

[l for l in lpaths if l.strip()]

www/index.html

440 ↗

(On Diff #12877)

Then why are we doing this?

Assuming we do want to do this, should explain why this isn't recommended.

447 ↗

(On Diff #12877)

Aren't "configuring CMake" and "building libc++" the same thing from the (cmake) user's perspective?

More importantly, I'm pretty sure we only need one of these. See my comment about find_file().

452 ↗

(On Diff #12877)

Drop the -G part. It's the default.

456 ↗

(On Diff #12877)

Is this how we phrase this elsewhere in the page? I thought we did path/to/libcxx.

[emaste]

lib/CMakeLists.txt

48

libdl isn't needed on FreeBSD, but that case is already handled by append_if, is it not?

[EricWF]

In D5038#9, @danalbert wrote:

@EricWF wrote:

Unfortunately not only does the user have to pass -DCMAKE_LIBRARY_PATH=/path/to/lib to CMake, but they also need to set RPATH=/path/to/lib when building libc++.
CMake does not have a mechanism for configuring relative paths via CMake variable.

find_file() does this. Look at how I select libc++ header paths in the libc++abi CMakeLists.txt.

Sorry, I could have been more clear. I'm using find_library but if the library is not along a standard path you need to specify CMAKE_LIBRARY_PATH. I incorrectly said relative paths but I meant runtime paths. Let me know if I got anything wrong or am missing anything.

I've respondeded to the inline comments and the changes will follow shortly after.

lib/CMakeLists.txt

46

Huh, Thanks for the correction. I didn't think of that. I don't know if we can tell the difference. I don't think i'll support that for now, but I'll strip this part out.

48

It's not handled by the append if, but the outer conditional which is only true on linux.

test/lit.cfg

342

*bows*. That is such a better way to write it.

www/index.html

440 ↗

(On Diff #12877)

Great note. I'll make the change. The reason we are doing this is

1. Testing.
2. The abililty to build and test on machines where libraries cannot be installed.

452 ↗

(On Diff #12877)

Will do.

456 ↗

(On Diff #12877)

I'll double check and make the change.

[EricWF]

www/index.html

447 ↗

(On Diff #12877)

Hi Dan,

Just wanted to be more clear. The difference between configuring and building is simply the difference between the cmake and the make command. For CMake we only need the CMAKE_LIBRARY_PATH to make find_library search along that path. Then we need RPATH when building so that the linker configures libc++.so to look for the ABI library along RPATH at runtime.

I can't actually find any instances of find_file under libcxxabi. Did you mean find_program?
Please let me know if I have misunderstood anything.

[EricWF]

First, Sorry for the spam.

Second. @danalbert was right. RPATH is not needed at all. CMake correctly handles linking after only providing CMAKE_LIBRARY_PATH.

[EricWF]

Ok, really really really really sorry for all the spam. I've actually updated the code this time.

[emaste]

lib/CMakeLists.txt

48

But LIBCXX_HAS_DL_LIB should be false on FreeBSD (and other systems that don't have libdl)

[EricWF]

Update and merge the patch with changes already upstream.

[danalbert]

Just a few small changes.

test/lit.cfg

380

I don't think you meant to make this one a tuple.

www/index.html

442 ↗

(On Diff #14981)

Indentation mismatch.

451 ↗

(On Diff #14981)

linker's

[danalbert]

test/lit.cfg

380

Oh, generator, not a tuple. In that case, I don't think you can directly concatenate a generator to a list. Use a list comprehension.

[danalbert]

test/lit.cfg

380

Apparently += is allowed, + isn't. Carry on...

[EricWF]

Addressed danalbert's comments.

[EricWF]

Accepting before commit.

[danalbert]

LGTM.

CMakeLists.txt

   )
 set(LIBCXX_TARGET_TRIPLE ${LIBCXX_TARGET_TRIPLE} CACHE STRING "Target triple.")
 
-#===============================================================================
-# Add an ABI library if appropriate
-#===============================================================================
-
-#
-# _setup_abi: Set up the build to use an ABI library
-#
-# Parameters:
-#   abidefines: A list of defines needed to compile libc++ with the ABI library
-#   abilibs   : A list of libraries to link against
-#   abifiles  : A list of files (which may be relative paths) to copy into the
-#               libc++ build tree for the build.  These files will also be
-#               installed alongside the libc++ headers.
-#   abidirs   : A list of relative paths to create under an include directory
-#               in the libc++ build directory.
-#
-macro(setup_abi_lib abipathvar abidefines abilibs abifiles abidirs)
-  list(APPEND LIBCXX_CXX_FEATURE_FLAGS ${abidefines})
-  set(${abipathvar} "${${abipathvar}}"
-    CACHE PATH
-    "Paths to C++ ABI header directories separated by ';'." FORCE
-    )
-  set(LIBCXX_CXX_ABI_LIBRARIES ${abilibs})
-  set(LIBCXX_ABILIB_FILES ${abifiles})
-
-  file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/include")
-  foreach(_d ${abidirs})
-    file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/include/${_d}")
-  endforeach()
-
-  foreach(fpath ${LIBCXX_ABILIB_FILES})
-    set(found FALSE)
-    foreach(incpath ${${abipathvar}})
-      if (EXISTS "${incpath}/${fpath}")
-        set(found TRUE)
-        get_filename_component(dstdir ${fpath} PATH)
-        get_filename_component(ifile ${fpath} NAME)
-        file(COPY "${incpath}/${fpath}"
-          DESTINATION "${CMAKE_BINARY_DIR}/include/${dstdir}"
-          )
-        list(APPEND abilib_headers "${CMAKE_BINARY_DIR}/include/${fpath}")
-      endif()
-    endforeach()
-    if (NOT found)
-      message(FATAL_ERROR "Failed to find ${fpath}")
-    endif()
-  endforeach()
-
-  add_custom_target(LIBCXX_CXX_ABI_DEPS DEPENDS ${abilib_headers})
-  include_directories("${CMAKE_BINARY_DIR}/include")
-
-  install(FILES ${abilib_headers}
-    DESTINATION include/c++/v1
-    PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-    )
-endmacro()
-
-if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libstdc++" OR
-    "${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libsupc++")
-  set(_LIBSUPCXX_INCLUDE_FILES
-    cxxabi.h bits/c++config.h bits/os_defines.h bits/cpu_defines.h
-    bits/cxxabi_tweaks.h bits/cxxabi_forced.h
-    )
-  if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libstdc++")
-    set(_LIBSUPCXX_DEFINES "-DLIBSTDCXX")
-    set(_LIBSUPCXX_LIBNAME stdc++)
-  else()
-    set(_LIBSUPCXX_DEFINES "")
-    set(_LIBSUPCXX_LIBNAME supc++)
-  endif()
-  setup_abi_lib("LIBCXX_LIBSUPCXX_INCLUDE_PATHS"
-    "-D__GLIBCXX__ ${_LIBSUPCXX_DEFINES}"
-    "${_LIBSUPCXX_LIBNAME}" "${_LIBSUPCXX_INCLUDE_FILES}" "bits"
-    )
-elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxabi")
-  if (LIBCXX_CXX_ABI_INTREE)
-    # Link against just-built "cxxabi" target.
-    set(CXXABI_LIBNAME cxxabi)
-  else()
-    # Assume c++abi is installed in the system, rely on -lc++abi link flag.
-    set(CXXABI_LIBNAME "c++abi")
-  endif()
-  setup_abi_lib("LIBCXX_LIBCXXABI_INCLUDE_PATHS" ""
-    ${CXXABI_LIBNAME} "cxxabi.h" ""
-    )
-elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxrt")
-  setup_abi_lib("LIBCXX_LIBCXXRT_INCLUDE_PATHS" "-DLIBCXXRT"
-    "cxxrt" "cxxabi.h;unwind.h;unwind-arm.h;unwind-itanium.h" ""
-    )
-elseif (NOT "${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "none")
-  message(FATAL_ERROR
-    "Currently libstdc++, libsupc++, libcxxabi, libcxxrt and none are "
-    "supported for c++ abi."
-    )
-endif ()
 
 # Configure compiler.
 include(config-ix)
+# Configure ABI
+include(HandleLibCXXABI)
 
 #===============================================================================
 # Setup Compiler Flags

cmake/HandleLibCXXABI.cmake

+
+message(STATUS "PATH: ${CMAKE_LIBRARY_PATH}")
+#===============================================================================
+# Add an ABI library if appropriate
+#===============================================================================
+# _setup_abi: Set up the build to use an ABI library
+#
+# Parameters:
+#   abidefines: A list of defines needed to compile libc++ with the ABI library
+#   abilibs   : A list of libraries to link against
+#   abifiles  : A list of files (which may be relative paths) to copy into the
+#               libc++ build tree for the build.  These files will also be
+#               installed alongside the libc++ headers.
+#   abidirs   : A list of relative paths to create under an include directory
+#               in the libc++ build directory.
+#
+macro(setup_abi_lib abipathvar abidefines abilibs abifiles abidirs)
+  list(APPEND LIBCXX_CXX_FEATURE_FLAGS ${abidefines})
+  set(${abipathvar} "${${abipathvar}}"
+    CACHE PATH
+    "Paths to C++ ABI header directories separated by ';'." FORCE
+    )
+
+  # To allow for libraries installed along non-default paths we use find_library
+  # to locate the ABI libraries we want. Making sure to clean the cache before
+  # each run of find_library.
+  set(LIBCXX_CXX_ABI_LIBRARIES "")
+  foreach(alib ${abilibs})
+    unset(_Res CACHE)
+    find_library(_Res ${alib})
+    if (${_Res} STREQUAL "_Res-NOTFOUND")
+     message(FATAL_ERROR "Failed to find ABI library: ${alib}")
+    else()
+      message(STATUS "Adding ABI library: ${_Res}")
+      list(APPEND LIBCXX_CXX_ABI_LIBRARIES ${_Res})
+    endif()
+  endforeach()
+  set(LIBCXX_ABILIB_FILES ${abifiles})
+
+  file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/include")
+  foreach(_d ${abidirs})
+    file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/include/${_d}")
+  endforeach()
+
+  foreach(fpath ${LIBCXX_ABILIB_FILES})
+    set(found FALSE)
+    foreach(incpath ${${abipathvar}})
+      if (EXISTS "${incpath}/${fpath}")
+        set(found TRUE)
+        get_filename_component(dstdir ${fpath} PATH)
+        get_filename_component(ifile ${fpath} NAME)
+        file(COPY "${incpath}/${fpath}"
+          DESTINATION "${CMAKE_BINARY_DIR}/include/${dstdir}"
+          )
+        list(APPEND abilib_headers "${CMAKE_BINARY_DIR}/include/${fpath}")
+      endif()
+    endforeach()
+    if (NOT found)
+      message(FATAL_ERROR "Failed to find ${fpath}")
+    endif()
+  endforeach()
+
+  add_custom_target(LIBCXX_CXX_ABI_DEPS DEPENDS ${abilib_headers})
+  include_directories("${CMAKE_BINARY_DIR}/include")
+
+  install(FILES ${abilib_headers}
+    DESTINATION include/c++/v1
+    PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
+    )
+endmacro()
+
+if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libstdc++" OR
+    "${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libsupc++")
+  set(_LIBSUPCXX_INCLUDE_FILES
+    cxxabi.h bits/c++config.h bits/os_defines.h bits/cpu_defines.h
+    bits/cxxabi_tweaks.h bits/cxxabi_forced.h
+    )
+  if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libstdc++")
+    set(_LIBSUPCXX_DEFINES "-DLIBSTDCXX")
+    set(_LIBSUPCXX_LIBNAME stdc++)
+  else()
+    set(_LIBSUPCXX_DEFINES "")
+    set(_LIBSUPCXX_LIBNAME supc++)
+  endif()
+  setup_abi_lib("LIBCXX_LIBSUPCXX_INCLUDE_PATHS"
+    "-D__GLIBCXX__ ${_LIBSUPCXX_DEFINES}"
+    "${_LIBSUPCXX_LIBNAME}" "${_LIBSUPCXX_INCLUDE_FILES}" "bits"
+    )
+elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxabi")
+  if (LIBCXX_CXX_ABI_INTREE)
+    # Link against just-built "cxxabi" target.
+    set(CXXABI_LIBNAME cxxabi)
+  else()
+    # Assume c++abi is installed in the system, rely on -lc++abi link flag.
+    set(CXXABI_LIBNAME "c++abi")
+  endif()
+  setup_abi_lib("LIBCXX_LIBCXXABI_INCLUDE_PATHS" ""
+    ${CXXABI_LIBNAME} "cxxabi.h" ""
+    )
+elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxrt")
+  setup_abi_lib("LIBCXX_LIBCXXRT_INCLUDE_PATHS" "-DLIBCXXRT"
+    "cxxrt" "cxxabi.h;unwind.h;unwind-arm.h;unwind-itanium.h" ""
+    )
+elseif (NOT "${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "none")
+  message(FATAL_ERROR
+    "Currently libstdc++, libsupc++, libcxxabi, libcxxrt and none are "
+    "supported for c++ abi."
+    )
+endif ()
 No newline at end of file

cmake/config-ix.cmake

 check_library_exists(m ccos "" LIBCXX_HAS_M_LIB)
 check_library_exists(rt clock_gettime "" LIBCXX_HAS_RT_LIB)
 check_library_exists(gcc_s __gcc_personality_v0 "" LIBCXX_HAS_GCC_S_LIB)
-
+check_library_exists(dl dladdr "" LIBCXX_HAS_DL_LIB)

lib/CMakeLists.txt

 append_if(libraries LIBCXX_HAS_RT_LIB rt)
 append_if(libraries LIBCXX_HAS_GCC_S_LIB gcc_s)
 
+# Linking against libcxxrt requires libdl on linux.
+if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxrt" AND
+    "${CMAKE_SYSTEM}" MATCHES "Linux")
+  append_if(libraries LIBCXX_HAS_DL_LIB dl)
+endif()
+
+
 target_link_libraries(cxx ${libraries})
 
 # Setup flags.

test/lit.cfg

         self.obj_root = None
         self.env = {}
         self.compile_flags = []
+        self.library_paths = []
         self.link_flags = []
         self.use_system_lib = False
 

         self.configure_src_root()
         self.configure_obj_root()
         self.configure_use_system_lib()
-        self.configure_env()
         self.configure_std_flag()
         self.configure_compile_flags()
         self.configure_link_flags()

         # Configure extra compiler flags.
         self.compile_flags += ['-I' + self.src_root + '/include',
                                '-I' + self.src_root + '/test/support']
+        if sys.platform == 'linux2':
+            self.compile_flags += ['-D__STDC_FORMAT_MACROS',
+                                   '-D__STDC_LIMIT_MACROS',
+                                   '-D__STDC_CONSTANT_MACROS']
 
     def configure_link_flags(self):
-        self.link_flags += ['-L' + self.obj_root + '/lib', '-lc++']
+        # Configure library search paths
+        lpaths = self.get_lit_conf('library_paths', '').split(';')
+        lpaths = list((l for l in lpaths if l.strip()))
+        self.link_flags += ['-L' + self.obj_root + '/lib']
+        self.link_flags += ('-L' + l for l in lpaths)
+        # Configure libraries
+        self.link_flags += ['-lc++']
         link_flags_str = self.get_lit_conf('link_flags')
         if link_flags_str is None:
             cxx_abi = self.get_lit_conf('cxx_abi', 'libcxxabi')

             elif sys.platform.startswith('freebsd'):
                 self.link_flags += ['-lc', '-lm', '-pthread', '-lgcc_s']
             else:
-                self.lit_config.fatal("unrecognized system")
+                self.lit_config.fatal("unrecognized system: %r" % sys.platform)
 
             self.lit_config.note(
                 "inferred link_flags as: %r" % self.link_flags)
         if link_flags_str:
             self.link_flags += shlex.split(link_flags_str)
 
-        if sys.platform == 'linux2':
-            if not self.use_system_lib:
-                self.link_flags += ['-Wl,-R', self.obj_root + '/lib']
-            self.compile_flags += ['-D__STDC_FORMAT_MACROS',
-                                   '-D__STDC_LIMIT_MACROS',
-                                   '-D__STDC_CONSTANT_MACROS']
-        elif sys.platform.startswith('freebsd'):
-            if not self.use_system_lib:
-                self.link_flags += ['-Wl,-R', self.obj_root + '/lib']
+        # Configure library runtime search paths
+            self.link_flags += ['-Wl,-rpath', '-Wl,' + self.obj_root + '/lib']
+        for l in lpaths:
+            self.link_flags += ['-Wl,-rpath', '-Wl,' + l]
+
 
     def configure_std_flag(self):
         # Try and get the std version from the command line. Fall back to

             self.lit_config.note(
                 "inferred target_triple as: %r" % self.config.target_triple)
 
-    def configure_env(self):
-        # Configure extra linker parameters.
-        if sys.platform == 'darwin':
-            if not self.use_system_lib:
-                self.env['DYLD_LIBRARY_PATH'] = os.path.join(self.obj_root,
-                                                             'lib')
-
-
 # name: The name of this test suite.
 config.name = 'libc++'
 

test/lit.site.cfg.in

 config.enable_shared         = @LIBCXX_ENABLE_SHARED@
 config.cxx_abi               = "@LIBCXX_CXX_ABI_LIBNAME@"
 config.llvm_use_sanitizer    = "@LLVM_USE_SANITIZER@"
+config.library_paths         = "@CMAKE_LIBRARY_PATH@"
 
 # Let the main config do the real work.
 lit_config.load_config(config, "@LIBCXX_SOURCE_DIR@/test/lit.cfg")