[LLDB] Provide target specific directories to libclang

On Linux some C++ and C include files reside in target specific directories, like /usr/include/x86_64-linux-gnu.
Patch adds them to libclang, so LLDB jitter has more chances to compile expression.

OS Laboratory. Huawei Russian Research Institute. Saint-Petersburg

Reviewed By: teemperor

Differential Revision: https://reviews.llvm.org/D110827

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

@@ -516,7 +516,7 @@ CppModuleConfiguration GetModuleConfig(lldb::LanguageType language,
   // Try to create a configuration from the files. If there is no valid
   // configuration possible with the files, this just returns an invalid
   // configuration.
-  return CppModuleConfiguration(files);
+  return CppModuleConfiguration(files, target->GetArchitecture().GetTriple());
 }
 
 bool ClangUserExpression::PrepareForParsing(

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

@@ -10,6 +10,7 @@
 
 #include "ClangHost.h"
 #include "lldb/Host/FileSystem.h"
+#include "llvm/ADT/Triple.h"
 
 using namespace lldb_private;
 
@@ -30,7 +31,35 @@ bool CppModuleConfiguration::SetOncePath::TrySet(llvm::StringRef path) {
   return false;
 }
 
-bool CppModuleConfiguration::analyzeFile(const FileSpec &f) {
+static llvm::SmallVector<std::string, 2>
+getTargetIncludePaths(const llvm::Triple &triple) {
+  llvm::SmallVector<std::string, 2> paths;
+  if (!triple.str().empty()) {
+    paths.push_back("/usr/include/" + triple.str());
+    if (!triple.getArchName().empty() ||
+        triple.getOSAndEnvironmentName().empty())
+      paths.push_back(("/usr/include/" + triple.getArchName() + "-" +
+                       triple.getOSAndEnvironmentName())
+                          .str());
+  }
+  return paths;
+}
+
+/// Returns the include path matching the given pattern for the given file
+/// path (or None if the path doesn't match the pattern).
+static llvm::Optional<llvm::StringRef>
+guessIncludePath(llvm::StringRef path_to_file, llvm::StringRef pattern) {
+  if (pattern.empty())
+    return llvm::NoneType();
+  size_t pos = path_to_file.find(pattern);
+  if (pos == llvm::StringRef::npos)
+    return llvm::NoneType();
+
+  return path_to_file.substr(0, pos + pattern.size());
+}
+
+bool CppModuleConfiguration::analyzeFile(const FileSpec &f,
+                                         const llvm::Triple &triple) {
   using namespace llvm::sys::path;
   // Convert to slashes to make following operations simpler.
   std::string dir_buffer = convert_to_slash(f.GetDirectory().GetStringRef());
@@ -43,15 +72,25 @@ bool CppModuleConfiguration::analyzeFile(const FileSpec &f) {
   // need to be specified in the header search.
   if (libcpp_regex.match(f.GetPath()) &&
       parent_path(posix_dir, Style::posix).endswith("c++")) {
-    return m_std_inc.TrySet(posix_dir);
+    if (!m_std_inc.TrySet(posix_dir))
+      return false;
+    if (triple.str().empty())
+      return true;
+
+    posix_dir.consume_back("c++/v1");
+    // Check if this is a target-specific libc++ include directory.
+    return m_std_target_inc.TrySet(
+        (posix_dir + triple.str() + "/c++/v1").str());
   }
 
-  // Check for /usr/include. On Linux this might be /usr/include/bits, so
-  // we should remove that '/bits' suffix to get the actual include directory.
-  if (posix_dir.endswith("/usr/include/bits"))
-    posix_dir.consume_back("/bits");
-  if (posix_dir.endswith("/usr/include"))
-    return m_c_inc.TrySet(posix_dir);
+  llvm::Optional<llvm::StringRef> inc_path;
+  // Target specific paths contains /usr/include, so we check them first
+  for (auto &path : getTargetIncludePaths(triple)) {
+    if ((inc_path = guessIncludePath(posix_dir, path)))
+      return m_c_target_inc.TrySet(*inc_path);
+  }
+  if ((inc_path = guessIncludePath(posix_dir, "/usr/include")))
+    return m_c_inc.TrySet(*inc_path);
 
   // File wasn't interesting, continue analyzing.
   return true;
@@ -92,11 +131,11 @@ bool CppModuleConfiguration::hasValidConfig() {
 }
 
 CppModuleConfiguration::CppModuleConfiguration(
-    const FileSpecList &support_files) {
+    const FileSpecList &support_files, const llvm::Triple &triple) {
   // Analyze all files we were given to build the configuration.
   bool error = !llvm::all_of(support_files,
                              std::bind(&CppModuleConfiguration::analyzeFile,
-                                       this, std::placeholders::_1));
+                                       this, std::placeholders::_1, triple));
   // If we have a valid configuration at this point, set the
   // include directories and module list that should be used.
   if (!error && hasValidConfig()) {
@@ -109,6 +148,10 @@ CppModuleConfiguration::CppModuleConfiguration(
     // This order matches the way Clang orders these directories.
     m_include_dirs = {m_std_inc.Get().str(), m_resource_inc,
                       m_c_inc.Get().str()};
+    if (m_c_target_inc.Valid())
+      m_include_dirs.push_back(m_c_target_inc.Get().str());
+    if (m_std_target_inc.Valid())
+      m_include_dirs.push_back(m_std_target_inc.Get().str());
     m_imported_modules = {"std"};
   }
 }

lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h

@@ -42,8 +42,15 @@ class CppModuleConfiguration {
 
   /// If valid, the include path used for the std module.
   SetOncePath m_std_inc;
+  /// If valid, the per-target include path used for the std module.
+  /// This is an optional path only required on some systems.
+  SetOncePath m_std_target_inc;
   /// If valid, the include path to the C library (e.g. /usr/include).
   SetOncePath m_c_inc;
+  /// If valid, the include path to target-specific C library files
+  /// (e.g. /usr/include/x86_64-linux-gnu).
+  /// This is an optional path only required on some systems.
+  SetOncePath m_c_target_inc;
   /// The Clang resource include path for this configuration.
   std::string m_resource_inc;
 
@@ -53,11 +60,13 @@ class CppModuleConfiguration {
   /// Analyze a given source file to build the current configuration.
   /// Returns false iff there was a fatal error that makes analyzing any
   /// further files pointless as the configuration is now invalid.
-  bool analyzeFile(const FileSpec &f);
+  bool analyzeFile(const FileSpec &f, const llvm::Triple &triple);
 
 public:
   /// Creates a configuration by analyzing the given list of used source files.
-  explicit CppModuleConfiguration(const FileSpecList &support_files);
+  /// The triple (if valid) is used to search for target-specific include paths.
+  explicit CppModuleConfiguration(const FileSpecList &support_files,
+                                  const llvm::Triple &triple);
   /// Creates an empty and invalid configuration.
   CppModuleConfiguration() = default;
 

lldb/unittests/Expression/CppModuleConfigurationTest.cpp

@@ -58,7 +58,6 @@ static std::string ResourceInc() {
   return std::string(resource_dir);
 }
 
-
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
 
@@ -69,12 +68,32 @@ TEST_F(CppModuleConfigurationTest, Linux) {
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, LinuxTargetSpecificInclude) {
+  // Test the average Linux configuration.
+
+  std::string usr = "/usr/include";
+  std::string usr_target = "/usr/include/x86_64-linux-gnu";
+  std::string libcpp = "/usr/include/c++/v1";
+  std::string libcpp_target = "/usr/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector<std::string> files = {
+      // C library
+      usr + "/stdio.h", usr_target + "/sys/cdefs.h",
+      // C++ library
+      libcpp + "/vector", libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+                                llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+              testing::ElementsAre(libcpp, ResourceInc(), usr, usr_target,
+                                   libcpp_target));
+}
+
 TEST_F(CppModuleConfigurationTest, Sysroot) {
   // Test that having a sysroot for the whole system works fine.
 
@@ -85,7 +104,7 @@ TEST_F(CppModuleConfigurationTest, Sysroot) {
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -101,7 +120,7 @@ TEST_F(CppModuleConfigurationTest, LinuxLocalLibCpp) {
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -119,12 +138,33 @@ TEST_F(CppModuleConfigurationTest, UnrelatedLibrary) {
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, UnrelatedLibraryWithTargetSpecificInclude) {
+  // Test that having an unrelated library in /usr/include doesn't break.
+
+  std::string usr = "/usr/include";
+  std::string libcpp = "/home/user/llvm-build/include/c++/v1";
+  std::string libcpp_target =
+      "/home/user/llvm-build/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector<std::string> files = {// C library
+                                    usr + "/stdio.h",
+                                    // unrelated library
+                                    usr + "/boost/vector",
+                                    // C++ library
+                                    libcpp + "/vector",
+                                    libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+                                llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+              testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));
+}
+
 TEST_F(CppModuleConfigurationTest, Xcode) {
   // Test detection of libc++ coming from Xcode with generic platform names.
 
@@ -139,7 +179,7 @@ TEST_F(CppModuleConfigurationTest, Xcode) {
       libcpp + "/vector",
       libcpp + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -154,7 +194,7 @@ TEST_F(CppModuleConfigurationTest, LibCppV2) {
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre("/usr/include/c++/v2", ResourceInc(),
@@ -172,7 +212,7 @@ TEST_F(CppModuleConfigurationTest, UnknownLibCppFile) {
                                     libcpp + "/non_existing_file",
                                     libcpp + "/module.modulemap",
                                     libcpp + "/vector"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre("/usr/include/c++/v1", ResourceInc(),
@@ -186,7 +226,7 @@ TEST_F(CppModuleConfigurationTest, MissingUsrInclude) {
   std::vector<std::string> files = {// C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
@@ -199,7 +239,7 @@ TEST_F(CppModuleConfigurationTest, MissingLibCpp) {
       // C library
       usr + "/stdio.h",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
@@ -214,7 +254,7 @@ TEST_F(CppModuleConfigurationTest, IgnoreLibStdCpp) {
       // C++ library
       usr + "/c++/8.0.1/vector",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
@@ -235,7 +275,7 @@ TEST_F(CppModuleConfigurationTest, AmbiguousCLib) {
       libcpp + "/vector",
       libcpp + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
@@ -257,7 +297,7 @@ TEST_F(CppModuleConfigurationTest, AmbiguousLibCpp) {
       libcpp2 + "/vector",
       libcpp2 + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }