[CMake] Revised RPATH handling

[sgraenitz]

Summary

If we build LLDB.framework, dependant tools need appropriate RPATHs in both locations, the build-tree (for testing) and the install-tree (for deployment). Luckily, CMake can handle it for us: https://gitlab.kitware.com/cmake/community/wikis/doc/cmake/RPATH-handling.

• In the build-tree, tools use the absolute path to the framework's actual output location.
• In the install-tree, tools get a list of RPATHs to look for the framework when deployed.

LLDB_FRAMEWORK_INSTALL_DIR is added to the CMAKE_INSTALL_PREFIX to change the relative location of LLDB.framework in the install-tree.
If it is not empty, it will be added as an additional RPATH to all dependant tools (so they are functional in the install-tree).
If it is empty, LLDB.framework goes to the root and tools will not be functional in the directory structure of the LLVM install-tree.
For historical reasons LLDB_FRAMEWORK_INSTALL_DIR defaults to "Library/Frameworks".

[sgraenitz]

Avoid conflicts: updating diff for recent changes on master

[JDevlieghere]

LGTM with the question answered/addressed. Thanks Stefan, these patches are really great work!

tools/driver/CMakeLists.txt

27

Would it make sense/work to do the check inside this function?

[xiaobai]

Looks good to me overall. You also probably probably also invoke lldb_setup_rpaths_framework for the tools included in the framework (argdumper, darwin-debug, debugserver, lldb-server).

[sgraenitz]

Rename function to setup rpaths and extend comments

[sgraenitz]

In D55330#1323525, @xiaobai wrote:

Looks good to me overall. You also probably probably also invoke lldb_setup_rpaths_framework for the tools included in the framework (argdumper, darwin-debug, debugserver, lldb-server).

Actually, these targets don't need the RPATHs. They are going into the framework bundle, yes, but they are linked entirely statically. So they don't need to find the framework at load time and don't need the RPATH, right?

tools/driver/CMakeLists.txt

27

Yes would be possible and it would save some lines of code, but this way it's more visible that it's a very special use-case and shouldn't be copy/pasted into each and every tool. For the general cases the default RPATHs from LLVM are exactly what is necessary. I have the impression that there was some confusing about RPATHs in this code before.

I thought about the alternative approach to query the dependencies for each target in lldb_add_executable and apply the RPATH settings to those targets that depend on liblldb (if LLDB_BUILD_FRAMEWORK). However, it feels like a bit too much magic under the hood and also it goes a little bit too far in the direction of building our own dependency management in CMake, which is not what CMake is made for.

I made the function name a little bit more explicit and extended the documentation. I would keep it like this now. What do you think?

cmake/modules/AddLLDB.cmake

function(add_lldb_library name)
   # or omit this logic instead.
   if (NOT WIN32)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
   endif()
 
   if (PARAM_OBJECT)
     add_library(${name} ${libkind} ${srcs})
   else()
-    llvm_add_library(${name} ${libkind} ${srcs} LINK_LIBS
-                                ${PARAM_LINK_LIBS}
-                                DEPENDS ${PARAM_DEPENDS})
+    if(LLDB_NO_INSTALL_DEFAULT_RPATH)
+      set(pass_NO_INSTALL_RPATH NO_INSTALL_RPATH)
+    endif()
+
+    llvm_add_library(${name} ${libkind} ${srcs}
+      LINK_LIBS ${PARAM_LINK_LIBS}
+      DEPENDS ${PARAM_DEPENDS}
+      ${pass_NO_INSTALL_RPATH}
+    )
 
     if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "liblldb")
       if (PARAM_SHARED)
         if(${name} STREQUAL "liblldb" AND LLDB_BUILD_FRAMEWORK)
           if(LLDB_FRAMEWORK_INSTALL_DIR)
             set(install_dir ${LLDB_FRAMEWORK_INSTALL_DIR})
           else()
             set(install_dir ".")

 function(add_lldb_executable name)
   cmake_parse_arguments(ARG
     "GENERATE_INSTALL"
     "ENTITLEMENTS"
     "LINK_LIBS;LINK_COMPONENTS"
     ${ARGN}
     )
 
+  if(LLDB_NO_INSTALL_DEFAULT_RPATH)
+    set(pass_NO_INSTALL_RPATH NO_INSTALL_RPATH)
+  endif()
+
   list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
-  add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS} ENTITLEMENTS ${ARG_ENTITLEMENTS})
+  add_llvm_executable(${name} ${ARG_UNPARSED_ARGUMENTS}
+    ENTITLEMENTS ${ARG_ENTITLEMENTS}
+    ${pass_NO_INSTALL_RPATH}
+  )
 
   target_link_libraries(${name} PRIVATE ${ARG_LINK_LIBS})
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 
   if(ARG_GENERATE_INSTALL)
     install(TARGETS ${name}
             COMPONENT ${name}
             RUNTIME DESTINATION bin)
function(lldb_append_link_flags target_name new_link_flags)
   # If we had any linker flags, include them first in the new linker flags.
   if(current_link_flags)
     set(new_link_flags "${current_link_flags} ${new_link_flags}")
   endif()
 
   # Now set them onto the target.
   set_target_properties(${target_name} PROPERTIES LINK_FLAGS ${new_link_flags})
 endfunction()
+
+# Account for different directory structures of build-tree and install-tree with
+# LLDB_BUILD_FRAMEWORK, replacing default RPATH settings (see llvm_setup_rpath),
+# so that we emit correct load commands for tools that dynamically link to the
+# framework bundle.
+function(lldb_setup_rpaths_framework name)
+  # In the build-tree, we know the exact path to the binary in the framework.
+  set(rpath_build_tree "$<TARGET_FILE:liblldb>")
+
+  # The installed framework is relocatable and can be in different locations.
+  set(rpaths_install_tree "@loader_path/../../../SharedFrameworks")
+  list(APPEND rpaths_install_tree "@loader_path/../../System/Library/PrivateFrameworks")
+  list(APPEND rpaths_install_tree "@loader_path/../../Library/PrivateFrameworks")
+
+  if(LLDB_FRAMEWORK_INSTALL_DIR)
+    set(rpaths_install_tree "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
+  endif()
+
+  set_target_properties(${name} PROPERTIES
+    BUILD_WITH_INSTALL_RPATH OFF
+    BUILD_RPATH "${rpath_build_tree}"
+    INSTALL_RPATH "${rpaths_install_tree}"
+  )
+
+  add_dependencies(${name} lldb-framework)
+endfunction()

cmake/modules/LLDBConfig.cmake

 endif()
 
 option(LLDB_DISABLE_PYTHON "Disable Python scripting integration." ${default_disable_python})
 option(LLDB_DISABLE_CURSES "Disable Curses integration." ${default_disable_curses})
 option(LLDB_DISABLE_LIBEDIT "Disable the use of editline." ${default_disable_libedit})
 option(LLDB_RELOCATABLE_PYTHON "Use the PYTHONHOME environment variable to locate Python." OFF)
 option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install a copy of it" OFF)
 option(LLDB_BUILD_FRAMEWORK "Build LLDB.framework (Darwin only)" OFF)
+option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in binaries" OFF)
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
 
 set(LLDB_CODESIGN_IDENTITY lldb_codesign CACHE STRING
     "Identity for code signing debugserver (Darwin only)")
 
 if(LLDB_CODESIGN_IDENTITY)
   set(LLVM_CODESIGNING_IDENTITY ${LLDB_CODESIGN_IDENTITY} CACHE STRING "" FORCE)
 endif()

tools/driver/CMakeLists.txt

 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   add_definitions( -DIMPORT_LIBLLDB )
 endif()
 
 add_dependencies(lldb
   LLDBOptionsTableGen
   ${tablegen_deps}
 )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_setup_rpaths_framework(lldb)
+endif()

tools/lldb-mi/CMakeLists.txt

add_lldb_tool(lldb-mi
   LINK_LIBS
     liblldb
     ${host_lib}
     ${extra_libs}
 
   LINK_COMPONENTS
     Support
   )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_setup_rpaths_framework(lldb-mi)
+endif()

tools/lldb-vscode/CMakeLists.txt

add_lldb_tool(lldb-vscode
   LINK_LIBS
     liblldb
     ${host_lib}
     ${extra_libs}
 
   LINK_COMPONENTS
     Support
   )
+
+if(LLDB_BUILD_FRAMEWORK)
+  lldb_setup_rpaths_framework(lldb-vscode)
+endif()