cmake: Set the proper rpath in add_llvm_executable and llvm_add_library

[plevine]

Summary

LLVM sets CMAKE_INSTALL_RPATH in its top-level CMakeLists.txt to "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}". When Clang is built with LLVM, this propagates to Clang as well and ensures that even if CMAKE_INSTALL_PREFIX is set, installed binaries always find their dependent libraries.

When compiling clang as a separate build, no such rpath is set. If CMAKE_INSTALL_PREFIX is not set and libraries are installed to standard locations, this is not a problem. But when defining CMAKE_INSTALL_PREFIX, even if the same as that of LLVM, and installing and then executing binaries like clang, it results in

error while loading shared libraries: libLLVMBPFCodeGen.so.4.0: cannot open shared object file: No such file or directory

A fix was proposed to Clang: https://reviews.llvm.org/D25267. It was suggested that this problem be resolved by patching LLVM's 'add_llvm_executable' function and remove the rpath logic from the top-level CMakeLists.txt entirely.

This patch is based on the rpath logic of the top-level CMakeLists.txt. It has been tested by building LLVM with "CMAKE_INSTALL_PREFIX=/usr/lib/llvm-9999" and building Clang separately with "CMAKE_INSTALL_PREFIX=/usr/lib/clang-9999" (the version number is a Gentoo Linux convention). Testing with readelf -d /usr/lib/llvm-9999/bin/llvm-extract | grep 'RUNPATH', for instance, correctly prints:

0x000000000000001d (RUNPATH) Library runpath: [$ORIGIN/../lib64]

and readelf -d /usr/lib/clang-9999/bin/clang++ | grep 'RUNPATH' correctly prints

0x000000000000001d (RUNPATH) Library runpath: [$ORIGIN/../lib64:/usr/lib64/llvm-9999/lib64]

Update: Patch updated based on previous suggestions.

[beanz]

First, the two blocks of code are doing the same thing, so they should be a function, not duplicated. Aside from that I have some feedback on the specific implementation, but I'm only going to write it on one set of the diffs.

cmake/modules/AddLLVM.cmake

509

In OS X 10.4 @loader_path was introduced as a more rubust rpath option, and it is equivolent to @executable_path in trivial situations but also supports bundles (which is important to LLDB), so we should use that here instead of @executable_path. I'm pretty sure nobody cares about deploying to 10.3 anymore.

514

Just for safety sake, please put an extra space at the end of the flag too.

518

This is definately not right. This condition will be false in clang and other sub-projects regardless of whether or not you're building against an installed LLVM. It might be sufficient to compare CMAKE_INSTALL_PREFIX against LLVM_INSTALL_PREFIX. The later is defined by LLVMConfig.cmake which is used by out-of-tree project builds. Writing the compare that way would also result in not having unnecissary extra rpaths.

521

Why are you setting this off? This doesn't seem right to me.

522

This should be semi-colon separated not colon-separated. When CMake interprets INSTALL_RPATH it expects a list of entries that it converts to the correct platform specific linker flags. Colon-separated may be correct for ELF platforms, but it isn't for Mach-o.

[plevine]

cmake/modules/AddLLVM.cmake

521

Whenever building clang with BUILD_WITH_INSTALL_RPATH set to ON, it fails with

warning: libclangLex.so.4.0, needed by lib32/libclangFormat.so.4.0.0, not found (try using -rpath or -rpath-link)
/usr/lib/gcc/x86_64-pc-linux-gnu/6.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: warning: libclangAST.so.4.0, needed by lib32/libclangToolingCore.so.4.0.0, not found (try using -rpath or -rpath-link)

along with undefined reference errors pertaining to clang library symbols. The libraries do include in their rpath "$ORIGIN/../lib32" which does correctly point back to the library directory even at build time so I admit I'm at a loss. Regardless, clang does build correctly with BUILD_WITH_INSTALL_RPATH set to OFF and the correct and necessary rpath exists at runtime. I'm open to any insight you can offer and any changes you would suggest.

[beanz]

cmake/modules/AddLLVM.cmake

521

Unfortunately I'm not sure I'm the right person to help with the Linux issue here. What I can say is that I did not see this issue on Darwin, and if you turn BUILD_WITH_INSTALL_RPATH off on Darwin the binaries get the wrong rpath in the build tree.

[plevine]

cmake/modules/AddLLVM.cmake

521

Here's the problem. The build-time linker doesn't respect $ORIGIN whether with -rpath or --rpath-link. When testing on the command line in the build directory, by changing $ORIGIN/../lib32 to ./lib32 it links fine.

[plevine]

cmake/modules/AddLLVM.cmake

518

It might be sufficient to compare CMAKE_INSTALL_PREFIX against LLVM_INSTALL_PREFIX

This doesn't appear to be working.

[beanz]

cmake/modules/AddLLVM.cmake

316

Please rename this to follow the conventions in this file (i.e. llvm_set_rpath).

317

Can you refactor this to reduce the number of places where set_target_properties is called?

This should be able to have a single call site something like:

set_target_properties(${name} PROPERTIES
          BUILD_WITH_INSTALL_RPATH On
          INSTALL_RPATH "${_install_rpath}"
          ${_install_name_dir})

Where _install_rpath is the rpath setting, and _install_name_dir is only defined on Apple to set(_install_name_dir INSTALL_NAME_DIR "@rpath").

327

As convention we don't put the condition inside endif blocks.

471

Rather than adding a condition block for this can you hoist it up under the add_library call above in the elseif(ARG_SHARED) block?

521

This seems wrong to me. In general rpaths are intended to be consumed by the loader, not the static linker. They get encoded into the final binary. Rather than disabling building with the install rpath, maybe this should be fixed by adding linker search paths.

Disabling the install rpath means that binaries need to be re-linked before installing them because the binaries aren't in their final linked form in the build directory.

[mgorny]

@plevine, are you still working on this? If not, I may need to take it over. I just noticed that the lack of this is breaking tests in clang stand-alone build, making them use clang libraries from previous install.

[mgorny]

Also, testing this patch I get the following output:

... -Wl,-rpath,/var/tmp/portage/sys-devel/llvm-9999/work/llvm-9999-abi_x86_32.x86/lib32:::::::::::::::::::

This doesn't look correct to me (esp. the dozen of empty paths).

[beanz]

@mgorny and I worked together on a patch that I've landed in rL285714, which should address this issue. If you're still having problems please let us know.

[plevine]

Yep. rL285714 works for me.

CMakeLists.txt

configure_file(
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support/DataTypes.h.cmake
   ${LLVM_INCLUDE_DIR}/llvm/Support/DataTypes.h)
 
 # They are not referenced. See set_output_directory().
 set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/bin )
 set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
 set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
 
-set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
-if (APPLE)
-  set(CMAKE_INSTALL_NAME_DIR "@rpath")
-  set(CMAKE_INSTALL_RPATH "@executable_path/../lib")
-else(UNIX)
-  if(NOT DEFINED CMAKE_INSTALL_RPATH)
-    set(CMAKE_INSTALL_RPATH "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}")
-    if(${CMAKE_SYSTEM_NAME} MATCHES "(FreeBSD|DragonFly)")
-      set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-z,origin")
-      set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-z,origin")
-    endif()
-  endif(NOT DEFINED CMAKE_INSTALL_RPATH)
-endif()
-
 if(APPLE AND DARWIN_LTO_LIBRARY)
   set(CMAKE_EXE_LINKER_FLAGS
     "${CMAKE_EXE_LINKER_FLAGS} -Wl,-lto_library -Wl,${DARWIN_LTO_LIBRARY}")
   set(CMAKE_SHARED_LINKER_FLAGS
     "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-lto_library -Wl,${DARWIN_LTO_LIBRARY}")
   set(CMAKE_MODULE_LINKER_FLAGS
     "${CMAKE_MODULE_LINKER_FLAGS} -Wl,-lto_library -Wl,${DARWIN_LTO_LIBRARY}")
 endif()

cmake/modules/AddLLVM.cmake

set_property(SOURCE ${resource_file}
                "RC_VERSION_FIELD_3=${ARG_VERSION_PATCHLEVEL}"
                "RC_VERSION_FIELD_4=0"
                "RC_FILE_VERSION=\"${ARG_VERSION_STRING}\""
                "RC_INTERNAL_NAME=\"${name}\""
                "RC_PRODUCT_NAME=\"${ARG_PRODUCT_NAME}\""
                "RC_PRODUCT_VERSION=\"${ARG_VERSION_STRING}\"")
 endfunction(set_windows_version_resource_properties)
 
+# Set the proper rpath for shared libraries and executables
+function(set_rpath name)
+  set_target_properties( ${name} PROPERTIES BUILD_WITH_INSTALL_RPATH ON )
+  if (APPLE)
+    set_target_properties( ${name} PROPERTIES INSTALL_NAME_DIR "@rpath" )
+    set_target_properties( ${name} PROPERTIES INSTALL_RPATH "@loader_path/../lib" )
+  else (UNIX)
+    if(NOT DEFINED CMAKE_INSTALL_RPATH)
+      set_target_properties( ${name} PROPERTIES INSTALL_RPATH "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}" )
+      if(${CMAKE_SYSTEM_NAME} MATCHES "(FreeBSD|DragonFly)")
+        set_property(TARGET ${name} APPEND_STRING PROPERTY LINK_FLAGS " -Wl,-z,origin ")
+      endif()
+    endif(NOT DEFINED CMAKE_INSTALL_RPATH)
+  endif()
+
+  if ( NOT "${CMAKE_INSTALL_PREFIX}" STREQUAL "${LLVM_INSTALL_PREFIX}" )
+    set_property(TARGET ${name} APPEND_STRING PROPERTY INSTALL_RPATH ";${LLVM_LIBRARY_DIR}")
+    if (UNIX)
+      set_target_properties( ${name} PROPERTIES BUILD_WITH_INSTALL_RPATH OFF )
+    endif()
+  endif()
+endfunction()
+
 # llvm_add_library(name sources...
 #   SHARED;STATIC
 #     STATIC by default w/o BUILD_SHARED_LIBS.
 #     SHARED by default w/  BUILD_SHARED_LIBS.
 #   OBJECT
 #     Also create an OBJECT library target. Default if STATIC && SHARED.
 #   MODULE
 #     Target ${name} might not be created on unsupported platforms.
function(llvm_add_library name)
   if(ARG_MODULE)
     set_target_properties(${name} PROPERTIES
       PREFIX ""
       SUFFIX ${LLVM_PLUGIN_EXT}
       )
   endif()
 
   if(ARG_SHARED)
+    set_rpath( ${name} )
+  endif()
+
+  if(ARG_SHARED)
     if(WIN32)
       set_target_properties(${name} PROPERTIES
         PREFIX ""
         )
     endif()
 
     # Set SOVERSION on shared libraries that lack explicit SONAME
     # specifier, on *nix systems that are not Darwin.
if(ARG_MODULE OR ARG_SHARED)
     if (LLVM_EXPORTED_SYMBOL_FILE)
       add_llvm_symbol_exports( ${name} ${LLVM_EXPORTED_SYMBOL_FILE} )
     endif()
   endif()
 
   if(ARG_SHARED AND UNIX)
     if(NOT APPLE AND ARG_SONAME)
       get_target_property(output_name ${name} OUTPUT_NAME)
       if(${output_name} STREQUAL "output_name-NOTFOUND")
         set(output_name ${name})
       endif()
       set(library_name ${output_name}-${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}${LLVM_VERSION_SUFFIX})
       set(api_name ${output_name}-${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}${LLVM_VERSION_SUFFIX})
       set_target_properties(${name} PROPERTIES OUTPUT_NAME ${library_name})
       llvm_install_library_symlink(${api_name} ${library_name} SHARED
         COMPONENT ${name}
         ALWAYS_GENERATE)
       llvm_install_library_symlink(${output_name} ${library_name} SHARED
         COMPONENT ${name}
         ALWAYS_GENERATE)
     endif()
   endif()
 
   if(ARG_MODULE AND LLVM_EXPORT_SYMBOLS_FOR_PLUGINS AND ARG_PLUGIN_TOOL AND (WIN32 OR CYGWIN))
     # On DLL platforms symbols are imported from the tool by linking against it.
     set(llvm_libs ${ARG_PLUGIN_TOOL})
   elseif (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS)
     if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB)
       set(llvm_libs LLVM)
     else()
macro(add_llvm_executable name)
   add_link_opts( ${name} )
 
   # Do not add -Dname_EXPORTS to the command-line when building files in this
   # target. Doing so is actively harmful for the modules build because it
   # creates extra module variants, and not useful because we don't use these
   # macros.
   set_target_properties( ${name} PROPERTIES DEFINE_SYMBOL "" )
 
+  set_rpath( ${name} )
+
   if (LLVM_EXPORTED_SYMBOL_FILE)
     add_llvm_symbol_exports( ${name} ${LLVM_EXPORTED_SYMBOL_FILE} )
   endif(LLVM_EXPORTED_SYMBOL_FILE)
 
   if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB)
     set(USE_SHARED USE_SHARED)
   endif()