[lldb] Symlink the Clang resource directory to the LLDB build directory in standalone builds

[teemperor]

Summary

When doing a standalone build (i.e., building just LLDB against an existing LLVM/Clang
installation), LLDB is currently unable to find any Clang resource directory that contains
all the builtin headers we need to parse real source code. This causes several tests that
actually parse source code on disk within the expression parser to fail (most notably nearly
all the import-std-module tests).

The reason why LLDB can't find the resource directory is that we search based on the path
of the LLDB shared library path. We assumed that the Clang resource directory is in the
same prefix and has the same relative path to the LLDB shared library (e.g.,
../clang/10.0.0/include). However for a standalone build where the existing Clang can
be anywhere on the disk, so we can't just rely on the hardcoded relative paths to the
LLDB shared library.

It seems we can either solve this by copying the resource directory to the LLDB installation,
symlinking it there or we pass the path to the Clang installation to the code that is trying to find the resource
directory. When building the LLDB framework we currently copy the resource directory
over to the framework folder (this is why the import-std-module are not failing on the
Green Dragon standalone bot).

This patch symlinks the resource directory of Clang into the LLDB build directory. The
reason for that is simply that this is only needed when running LLDB from the build
directory. Once LLDB and Clang/LLVM are installed the already existing logic can find
the Clang resource directory by searching relative to the LLDB shared library.

[kastiglione]

Nice fix. Every time I see issues with the relative resource directory heuristic, I think "someone should improve that".

lldb/cmake/modules/LLDBConfig.cmake

258–259

I realize that this logic previously existed, but should this if/elseif chain check that these paths exists before committing to them? As it stands now, if Clang_DIR has a value that doesn't exist on disk, and meanwhile LLVM_DIR is also assigned, but does exist on disk, then this logic will take Clang_DIR over LLVM_DIR, when it shouldn't.

273

should this remain set, so that it can be checked at runtime anyway? In other words, could it ever not exist at the time of this check, but will exist by the time it's used at runtime?

lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp

73–77

should a standalone build put its resource dir first in the search path?

[JDevlieghere]

I'm not convinced this is the right solution. First of all I don't think embedding a build path in LLDB in acceptable. Swift uses the standalone builds and with this change the linux toolchain will have some random CI path embedded in LLDB, which based on the comment above it wouldn't even need. The existing code implies that this should work once LLDB is installed. If this issue is indeed limited in scope to the build directory, I think we should either create a symlink or copy over the resource directory. An alternative, more general approach might be to make the source directory configurable through a setting so that this could work wherever LLDB lives and then have the test suite populate that setting based on the location of the resource directory in the clang build dir.

[teemperor]

Having a symlink to the Clang resource dir inside the LLDB *build* dir (but not the install) seems like a reasonable workaround. I'm not sure how valuable this as a setting is as the value should never actually change after LLDB installation and most likely people will just break their install by accident when touching it. But I think having a fully optional CMake parameter would be a good follow up in case people have some setup where the install prefixes don't match.

[kastiglione]

I must have missed something, isn't this embedded the installed path of the clang resource dir? The diff description says "i.e., building just LLDB against an existing LLVM/Clang installation". I agree that embedding a build path is bad, but embedding clang's installed path seems legit.

[teemperor]

In D88581#2304375, @kastiglione wrote:

I must have missed something, isn't this embedded the installed path of the clang resource dir? The diff description says "i.e., building just LLDB against an existing LLVM/Clang installation". I agree that embedding a build path is bad, but embedding clang's installed path seems legit.

You can build both against a LLVM/Clang installation (like in /usr or some other prefix) or just a LLVM/Clang build directory that never went through the actual install step to some prefix (but that acts as if it's an installed Clang).

[teemperor]

• Replaced with symlinking resource dir to LLDB build directory

[JDevlieghere]

lldb/source/API/CMakeLists.txt

210 ↗

(On Diff #295852)

Can you check that this works in a non-framework Xcode build (or any other multi-config generator). I think the generator expressions are resolved too late.

[teemperor]

• Fixed Xcode build.

Yeah it seems that the LIBRARY_OUTPUT_DIRECTORY doesn't actually contain the subdirectory, so moved over to TARGET_FILE_DIR which is anyway closer to the way this should work (as we search relative to the liblldb directory).

[JDevlieghere]

Thanks. LGTM!

[mnovakovic]

lldb/source/API/CMakeLists.txt

210 ↗

(On Diff #295852)

Good catch and this doesn't work in a non-Framework context. I am building standalone LLDB and it fails on the next line: file(MAKE_DIRECTORY "${LLDB_CLANG_RESOURCE_DIR_PARENT}")

That line is not even needed because line 214 builds the directory already, the fix for this could be to just remove like 211 and only build directory in the post_build phase.

[mnovakovic]

lldb/source/API/CMakeLists.txt

210 ↗

(On Diff #295852)

Actually removing the line 211 doesn't solve the issue, the failure is only postponed until later when the line 214 is executed (link stage looks like it) and it fails because ${LLDB_CLANG_RESOURCE_DIR_PARENT} is evaluated to an empty string.

[mnovakovic]

lldb/source/API/CMakeLists.txt

210 ↗

(On Diff #295852)

Looks like I can't change the comment - my bad, looks like the deleting line 211 does the trick

[mgorny]

Gentoo uses plain /usr/lib/clang/X.Y.Z which doesn't really match any of the tested paths. Do you think there's a point in adding yet another case there or should I just override it locally?

lldb/cmake/modules/LLDBConfig.cmake

if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   set_target_properties(lldb-headers PROPERTIES FOLDER "lldb misc")
 
   if (NOT CMAKE_CONFIGURATION_TYPES)
     add_llvm_install_targets(install-lldb-headers
                              COMPONENT lldb-headers)
   endif()
 endif()
 
+
+# If this is building against a prebuilt Clang, then the Clang resource
+# directory that LLDB is using for its embedded Clang instance needs to point
+# to the resource directory of the used Clang installation.
+if (NOT TARGET clang-resource-headers)
+  if(Clang_DIR)
+    set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR ${Clang_DIR}/../..)
+  elseif(LLVM_DIR)
+    set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR ${LLVM_DIR}/../..)
+  elseif(LLVM_LIBRARY_DIRS)
+    set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR ${LLVM_LIBRARY_DIRS})
+  elseif(LLVM_BUILD_LIBRARY_DIR)
+    set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR ${LLVM_BUILD_LIBRARY_DIR})
+  elseif(LLVM_BINARY_DIR)
+    set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+  endif()
+  set(clang_version ${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH})
+  # Build the final resource directory path by appending 'clang/<version number>'.
+  set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}/clang/${clang_version})
+  if(NOT EXISTS ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR})
+    unset(LLDB_EXTERNAL_CLANG_RESOURCE_DIR)
+    message(WARNING "Expected directory for clang-resource headers not found: ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}")
+  endif()
+endif()
+
 # Find Apple-specific libraries or frameworks that may be needed.
 if (APPLE)
   if(NOT APPLE_EMBEDDED)
     find_library(CARBON_LIBRARY Carbon)
     find_library(CORE_SERVICES_LIBRARY CoreServices)
   endif()
   find_library(FOUNDATION_LIBRARY Foundation)
   find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)

lldb/cmake/modules/LLDBFramework.cmake

 )
 
 # Copy vendor-specific headers from clang (without staging).
 if(NOT APPLE_EMBEDDED)
   if (TARGET clang-resource-headers)
     add_dependencies(liblldb clang-resource-headers)
     set(clang_resource_headers_dir $<TARGET_PROPERTY:clang-resource-headers,RUNTIME_OUTPUT_DIRECTORY>)
   else()
-    # In standalone builds try the best possible guess
-    if(Clang_DIR)
-      set(clang_lib_dir ${Clang_DIR}/../..)
-    elseif(LLVM_DIR)
-      set(clang_lib_dir ${LLVM_DIR}/../..)
-    elseif(LLVM_LIBRARY_DIRS)
-      set(clang_lib_dir ${LLVM_LIBRARY_DIRS})
-    elseif(LLVM_BUILD_LIBRARY_DIR)
-      set(clang_lib_dir ${LLVM_BUILD_LIBRARY_DIR})
-    elseif(LLVM_BINARY_DIR)
-      set(clang_lib_dir ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
-    endif()
-    set(clang_version ${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH})
-    set(clang_resource_headers_dir ${clang_lib_dir}/clang/${clang_version}/include)
+    set(clang_resource_headers_dir ${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}/include)
     if(NOT EXISTS ${clang_resource_headers_dir})
       message(WARNING "Expected directory for clang-resource headers not found: ${clang_resource_headers_dir}")
     endif()
   endif()
 
   add_custom_command(TARGET liblldb POST_BUILD
     COMMAND ${CMAKE_COMMAND} -E copy_directory
             ${clang_resource_headers_dir}

lldb/include/lldb/Host/Config.h.cmake

 #cmakedefine01 LLDB_ENABLE_PYTHON
 
 #cmakedefine01 LLDB_EMBED_PYTHON_HOME
 
 #cmakedefine LLDB_PYTHON_HOME "${LLDB_PYTHON_HOME}"
 
 #define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
 
+// The full path to the resource directory of the Clang that LLDB was build
+// against (or an empty path if LLDB and Clang are build together).
+#define LLDB_EXTERNAL_CLANG_RESOURCE_DIR "${LLDB_EXTERNAL_CLANG_RESOURCE_DIR}"
+
 #endif // #ifndef LLDB_HOST_CONFIG_H

lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp

static const llvm::StringRef kResourceDirSuffixes[] = {
       // in $install_dir/lib{,64}/clang/$clang_version.
       "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
       // swift-lldb uses the clang resource directory copied from swift, which
       // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
       // it there, so we use LLDB_LIBDIR_SUFFIX.
       "lib" LLDB_LIBDIR_SUFFIX "/lldb/clang",
   };
 
+  std::vector<llvm::SmallString<256>> directories_to_check;
   for (const auto &Suffix : kResourceDirSuffixes) {
     llvm::SmallString<256> clang_dir(parent_dir);
     llvm::SmallString<32> relative_path(Suffix);
     llvm::sys::path::native(relative_path);
     llvm::sys::path::append(clang_dir, relative_path);
+    directories_to_check.push_back(clang_dir);
+  }
+
+  // Check the resource directory if LLDB was build against an existing Clang.
+  // The path is empty if Clang is build alongside LLDB.
+  llvm::SmallString<256> external_clang_dir(LLDB_EXTERNAL_CLANG_RESOURCE_DIR);
+  if (!external_clang_dir.empty())
+    directories_to_check.push_back(external_clang_dir);
+
+  for (const auto &clang_dir : directories_to_check) {
     if (!verify || VerifyClangPath(clang_dir)) {
       LLDB_LOG(log,
                "DefaultComputeClangResourceDir: Setting ClangResourceDir "
                "to \"{0}\", verify = {1}",
                clang_dir.str(), verify ? "true" : "false");
       file_spec.GetDirectory().SetString(clang_dir);
       FileSystem::Instance().Resolve(file_spec);
       return true;